Repository navigation
Commit 8843505
Fixes #21663
Clause-②: no (narrowing)
## What this changes
A system writer is exempt from the readonly **strip**, never from the
value-**shape** check (triage's ruling on the card, comment 5975978206).
The static readonly strip drops a non-system caller's readonly value and
exempts a system write (seed replay, migration, `isSystem` plugin code,
a hook's stamp). The record validator skipped every readonly field
outright, on the premise that the strip had already removed anything a
caller sent. That premise is false for exactly the writers the strip
exempts, so under `isSystem` a malformed readonly value reached the
driver unjudged.
After this PR:
- The strip is untouched. It keeps its system exemption, and a
non-system caller's readonly value is still dropped, never refused.
- The value the exemption keeps is judged for its SHAPE wherever the
payload is final. A malformed value is refused with `VALIDATION_FAILED`
(400 at the HTTP boundary), with the same field code and the same
sentence a non-readonly field gets (`Run At must be a valid datetime
(ISO-8601)`). A seed counts the row as a seed error.
- ⛔ No silent coercion. Nothing malformed is rewritten into something
else.
## Where the fix lives (the order's H1 file location did not hold)
The dispatch expected the branch in
`packages/objectql/src/validation/rule-validator.ts`. Measured at
`72f3c74d60`: the strip lives there (`stripReadonlyFields`), but the
shape check and its readonly skip live in
**`packages/objectql/src/validation/record-validator.ts`**
(`validateRecord`, `if (def.system || def.readonly) continue` on both
walks). The engine (`packages/objectql/src/engine.ts`) runs the strip
and the validator at different points:
| path | order at `72f3c74d60` |
|---|---|
| insert | strip, then `validateRecord` |
| dry run (`ObjectQL.validate`) | strips, then `validateRecord` |
| update, by id and by predicate | `validateRecord`, **then** the strip
|
So the fix lands in the producer's own file, with the engine choosing
the scope at each seam:
- `record-validator.ts`: a `ReadonlyValueScope` (`'skip' | 'include' |
'only'`) and a module-internal `validateRecordInScope`. The published
`validateRecord` keeps its signature and behaviour byte for byte, so
nothing on the package's public surface widens (the claim's Clause-②
line holds).
- `engine.ts`: insert and dry run judge with `'include'` (post-strip).
Both update paths keep their first call at `'skip'` and add a second
pass at `'only'` right after `assertNoStrictDrops()`, where the payload
is final.
- Why not judge readonly values in the update path's first call: that
call runs before the strip, so a readonly value there may be a caller's
that the strip is about to drop. A whole-record write-back that echoes a
legacy malformed stored value would turn from a save into a refusal. Pin
3 holds this.
- `rule-validator.ts`: docs only. The strip's docblock now says a system
write skips the strip and nothing else.
## The boundary: shape, never a constraint
A readonly value reaches the type's shape arms only:
- **refused:** a `date` / `datetime` / `time` the platform does not
read; a non-number on a number-typed field; a non-boolean; a non-array
on a multi-value field; a filter-operator object; and the ADR-0104
reference / media / structured-JSON shape under the object's own posture
(warn-first, exactly as on a non-readonly field).
- **not checked, as before:** option membership, `maxLength` /
`minLength`, `valueDomain`, `min` / `max` / `scale` / `precision`, the
email / url / phone formats, and `required`.
Option membership is the load-bearing exclusion. `sys_activity.type` is
a readonly `select` whose options are the built-in set of an open
vocabulary. The maintainer ruling recorded at commit `88b9d749a` binds
that an author-contributed value is stored, and its object file says "Do
not fix this by enforcing the enum on system-owned writes". The email /
url / phone formats stay out because the spec's stored shape for those
types (`valueSchemaFor`) is a plain string. Readonly `url` fields that
platform code writes (`sys_activity.url`,
`sys_activity.actor_avatar_url`, `sys_organization.logo`) are why that
matters.
For the same reason "judged" equals "stored": a numeric string on a
readonly number field is now written as its number
(`normalizeNumericStringValues`, at the door, ahead of the caller
snapshot, so the strip still drops a non-system caller's key), and a
lone scalar on a readonly multi-value field is wrapped post-strip, as on
any other field.
## H2: which shape checks a system write skipped (measured)
A throwaway probe (deleted, never committed) inserted one malformed
value per type into a readonly field and into its non-readonly twin.
| field (malformed value) | `isSystem`, readonly, at `72f3c74d60` |
`isSystem`, readonly, after | `isSystem`, non-readonly (unchanged) |
non-system, readonly (unchanged) |
|---|---|---|---|---|
| datetime `'yesterday'` | stored | refused `invalid_date` | refused |
dropped |
| datetime, raw `cel` envelope | stored | refused `invalid_date` |
refused | dropped |
| date `'yesterday'` | stored | refused `invalid_date` | refused |
dropped |
| time `'noon'` | stored | refused `invalid_time` | refused | dropped |
| number / currency / percent `'abc'` | stored | refused
`invalid_number` | refused | dropped |
| boolean `'maybe'` | stored | refused `invalid_boolean` | refused |
dropped |
| multiselect, an object | stored | refused `invalid_type` | refused |
dropped |
| text, `{ $in: [...] }` | stored | refused `invalid_type` | refused |
dropped |
| number `max: 5`, value 9 | stored | stored (constraint) | refused |
dropped |
| select, undeclared option | stored | stored (constraint) | refused |
dropped |
| text `maxLength: 3`, 6 chars | stored | stored (constraint) | refused
| dropped |
| email / url / phone, malformed | stored | stored (format) | refused |
dropped |
| lookup, `cel` envelope | stored | stored with the ADR-0104 warning
(warn-first) | stored with the warning | dropped |
| location `'nowhere'` | stored | stored with the ADR-0104 warning
(warn-first) | stored with the warning | dropped |
## H3, H4, H5
- **H3, the seed path:** measured through the real `SeedLoaderService`
(pins 1 and 2). `'yesterday'` on a readonly datetime is refused and
counted (`summary.totalErrored`), with the non-readonly sentence, on the
fresh-boot insert and on the replay update. A valid ISO value, a `cel`
value the loader evaluates, and an authored `created_at` are kept. That
last case pairs with the arm #21646 landed.
- **H4, the seeders that skip `resolveSeedRecord`:** `AppPlugin`'s two
fallback inserts (`packages/runtime/src/app-plugin.ts`, the
no-metadata-service branch and the loader-threw branch) and
`@objectstack/verify`'s `seed()` (`packages/verify/src/handle.ts`). A
raw `cel` envelope on a readonly datetime is now refused on their call
shapes (single-row and array insert under
`SEED_WRITE_EXECUTION_CONTEXT`, pinned). **No example app or test newly
fails.** `runtime` (4554 tests) and `verify` (131) are green. The only
readonly field seeded with `cel` in `examples/` is `created_at`, in 10
`app-showcase` task rows, and every one of those rows also seeds a
non-readonly `due_date` with `cel`. So on the fallback path those rows
were already refused before this change, and on the normal path the
loader evaluates them. No cross-lane fix is needed for this change. The
fallback's pre-existing `warn`-level per-row loss is in the Acceptance
notes.
- **H5, other system writers:** measured through the platform's own
suites. None writes a malformed readonly value. Green: `plugin-audit`
621, `plugin-pinyin-search` 21, `plugin-security` 3527, `plugin-auth`
2494, `plugin-approvals` 875, `service-automation` 2098,
`metadata-protocol` 3463, `runtime` 4554, `verify` 131. Inside objectql,
two fixtures turned red and were re-judged, not relaxed:
- `engine-insert-static-readonly-strip.test.ts`: an `isSystem` case used
the placeholder `'x'` in a readonly datetime, in a test about
`strictReadonlyWrites`. It is respelled to a valid instant, like its
`isSystem` sibling.
- `record-validator.number-value.test.ts`: it pinned "the numeric
normalizer skips a readonly field". The number arm now judges a readonly
value, so by the normalizer's own invariant (what the arm judges is what
the driver stores) the readonly field moves to the rewritten side.
## Pins
`packages/objectql/src/seed-readonly-value-shape.test.ts`, on the real
kernel (`ObjectKernel` + `ObjectQLPlugin`) and the real
`SeedLoaderService`. Each refusal asserts `code` and `status` (ADR-0112)
through `resolveThrownHttpError`, the boundary's own reading. The
engine's `ValidationError` carries no `status` by design.
1. `'yesterday'` on a readonly datetime in a seed is refused and
counted, with the same sentence the non-readonly twin gets. The same
holds on the replay, on all four write seams (insert, update by id,
update by predicate, dry run) as `VALIDATION_FAILED` / 400 with the
non-readonly field envelope, for a raw `cel` envelope, and for a
malformed authored `created_at`.
2. A valid ISO value on a readonly field under the seed context is kept:
authored, evaluated from `cel`, and on `created_at`, on insert and on
replay.
3. The non-readonly path is unchanged. A non-system caller's readonly
value is still dropped, never refused, on insert and on a whole-record
write-back echoing a legacy malformed value. A readonly undeclared
option and an out-of-bound number are stored, while the non-readonly
twin refuses both.
**Reverse verification** (committed first, at `196b217829`). The split
was reverted at its one predicate in `record-validator.ts`, through
`scripts/ablation-replace.mjs` under a shell trap: anchor `if
(def.readonly === true) return scope !== 'skip';` went from 1 to 0 hits,
the replacement `return false` from 0 to 1, and the blob from
`d57cbd3078` to `3768d5668f`. Result: `Tests 4 failed | 4 passed (8)`.
The four red tests are exactly pin 1 (`expected 1 to be 2`, `expected +0
to be 1`, and `the write must be refused` twice); pin 2 and the three
pin-3 tests stayed green. Restore was `git checkout HEAD -- PATH`: blob
back to `d57cbd3078` (the HEAD blob), `git diff HEAD` 0 bytes, `git
status --porcelain` empty. No dist rebuild per leg was needed: the pin
imports `./engine.js` / `./plugin.js` from src by relative path.
## Tests
Head of record: `b73f58e396` (after merging `origin/main` at
`251a7dd4b4`).
- **Pins:** `pnpm --filter @objectstack/objectql exec vitest run
--maxWorkers=2 src/seed-readonly-value-shape.test.ts` gives `Tests 8
passed (8)` at `b73f58e396`. With #21646's pin file beside it earlier:
`Tests 14 passed (14)`.
- **objectql:** `vitest run --project local --maxWorkers=2` gives `Test
Files 372 passed (372) / Tests 7455 passed (7455)` and `--project repo`
gives `Tests 5 passed (5)`, both at `e6b5281680`. `pnpm --filter
@objectstack/objectql run typecheck` exits 0, with
`check:test-typecheck: OK … 40 file(s) / 234 error(s) / 65 pinned
signature(s) held` (ledger unchanged). `tsc -p tsconfig.test.json
--listFilesOnly` lists the new pin file. The only change after
`e6b5281680` is the pin file's row-key rename (rerun green above).
- **H5 consumer suites** (`pnpm --filter PKG run test`, against
objectql's rebuilt `dist/`, at `45804afc28`): every one green, with the
counts in the H5 section. The commits after it are behaviour-identical
for these suites: the engine-internal entry refactor, the changeset, a
merge of `main` with no objectql overlap, and the pin rename.
- **Gates:** `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` with no paths derives 68 commands at
`b73f58e396`, from 7 paths against merge base `251a7dd4b`. All 68 ran,
each exit code captured before any pipe: **68 × exit 0**. `--ran`
reports `68 derived, 68 run, 0 NOT-MEASURED, 0 UNRUN`. Two readings came
from the first union at `e6b5281680`:
- `check:error-code-casing` was red. It read the pins' row key, a field
named `code`, as a lowercase error code. The key is renamed to `ref`,
and the gate is green from `b73f58e396`.
- `check:dual-build-cjs-loads` exited 3 (PREREQUISITE NOT MET). After a
full `turbo run build` (72/72) it exits 0.
- **Artifact-roster block** (53 families outside the derived total, all
run at `b73f58e396`): 50 exit 0. `check-closing-target-claim`,
`check-partof-closing-keyword` and `check-single-claim-paths` report NOT
WIRED with no PR context (exit 2, not a verdict).
`check-partof-closing-keyword` was then run with this body as `PR_BODY`:
exit 0, "no Part-of/closing-keyword contradiction". The other two need
the PR number, and their results go in the os-dev-report on the card.
- **Changeset gates:** `check-changeset-no-major`,
`check-adr-0087-registration` (1 declared-breaking changeset, carrying
its disposition) and `check-empty-changeset` all exit 0.
- **NOT MEASURED** (CI-only, no local invocation): shard attestation and
test-completeness, the Test Core / Temporal Conformance / Dogfood /
Dogfood Verify / Build Core jobs, the workspace and consumer type-check
lanes, and the 11 declared wide-population families. Repository-wide
`pnpm lint` is CI-owned and was not run.
## Acceptance notes
- **`owner_id` keeps the full skip.** It is `system` but not `readonly`,
so the split does not reach it (no strip exemption is involved). It is
caller-writable and its value shape is still never judged. This is
read-only inference, not measured through a door.
- **The ADR-0104 dormancy test still excludes readonly columns**
(`isScannableValueShapeField`). Widening it would make every object
non-dormant through its injected readonly lookups. So an object whose
only covered fields are readonly stays warn-first for them: a malformed
readonly reference is admitted, logged and reported to
`onAdmittedValueShapeViolation`, never stored silently. The `os migrate
value-shapes` scan population is unchanged.
- **`AppPlugin`'s fallback seeders** log a refused row at `warn` and
then report "Data seeding complete", while `SeedLoaderService` logs the
same loss at `error`. This is pre-existing and not caused here. carrier:
none.
- **Comments elsewhere still say "`validateRecord` skips readonly
fields"** (plugin-audit sources and tests, and two ADR-0087 semantic
entries in `packages/spec/src/migrations/`). What they rely on, that a
readonly option set is not enforced, stays true by the boundary above.
The stated reason is now imprecise. Not edited here. carrier: none.
- **The dry run never applies `normalizeMultiValueFields`**, for any
field, so a scalar on a multi-value field previews as invalid while the
write wraps and accepts it. This is pre-existing, and readonly fields
now behave the same as the rest. carrier: none.
---
_Generated by [Claude
Code](https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
1 parent a2aadab commit 8843505
7 files changed
Lines changed: 672 additions & 40 deletions
File tree
- .changeset
- packages/objectql/src
- validation
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
Lines changed: 5 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
382 | 382 | | |
383 | 383 | | |
384 | 384 | | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
385 | 389 | | |
386 | | - | |
| 390 | + | |
387 | 391 | | |
388 | 392 | | |
389 | 393 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
262 | 262 | | |
263 | 263 | | |
264 | 264 | | |
265 | | - | |
| 265 | + | |
266 | 266 | | |
267 | 267 | | |
268 | 268 | | |
| |||
6063 | 6063 | | |
6064 | 6064 | | |
6065 | 6065 | | |
6066 | | - | |
6067 | | - | |
6068 | | - | |
| 6066 | + | |
| 6067 | + | |
| 6068 | + | |
| 6069 | + | |
6069 | 6070 | | |
6070 | 6071 | | |
6071 | 6072 | | |
| |||
9884 | 9885 | | |
9885 | 9886 | | |
9886 | 9887 | | |
9887 | | - | |
9888 | | - | |
9889 | | - | |
9890 | | - | |
9891 | | - | |
9892 | | - | |
| 9888 | + | |
| 9889 | + | |
| 9890 | + | |
| 9891 | + | |
| 9892 | + | |
| 9893 | + | |
| 9894 | + | |
| 9895 | + | |
| 9896 | + | |
| 9897 | + | |
| 9898 | + | |
| 9899 | + | |
| 9900 | + | |
9893 | 9901 | | |
9894 | 9902 | | |
9895 | 9903 | | |
| |||
12469 | 12477 | | |
12470 | 12478 | | |
12471 | 12479 | | |
12472 | | - | |
12473 | | - | |
12474 | | - | |
| 12480 | + | |
| 12481 | + | |
| 12482 | + | |
| 12483 | + | |
| 12484 | + | |
12475 | 12485 | | |
12476 | 12486 | | |
12477 | 12487 | | |
| |||
12724 | 12734 | | |
12725 | 12735 | | |
12726 | 12736 | | |
12727 | | - | |
| 12737 | + | |
| 12738 | + | |
| 12739 | + | |
| 12740 | + | |
| 12741 | + | |
12728 | 12742 | | |
12729 | 12743 | | |
12730 | 12744 | | |
| |||
13515 | 13529 | | |
13516 | 13530 | | |
13517 | 13531 | | |
13518 | | - | |
13519 | | - | |
| 13532 | + | |
| 13533 | + | |
| 13534 | + | |
| 13535 | + | |
| 13536 | + | |
| 13537 | + | |
| 13538 | + | |
13520 | 13539 | | |
13521 | 13540 | | |
13522 | 13541 | | |
| |||
14866 | 14885 | | |
14867 | 14886 | | |
14868 | 14887 | | |
14869 | | - | |
| 14888 | + | |
| 14889 | + | |
| 14890 | + | |
| 14891 | + | |
| 14892 | + | |
| 14893 | + | |
| 14894 | + | |
14870 | 14895 | | |
14871 | 14896 | | |
14872 | 14897 | | |
| |||
15051 | 15076 | | |
15052 | 15077 | | |
15053 | 15078 | | |
| 15079 | + | |
| 15080 | + | |
| 15081 | + | |
| 15082 | + | |
| 15083 | + | |
| 15084 | + | |
| 15085 | + | |
| 15086 | + | |
| 15087 | + | |
15054 | 15088 | | |
15055 | 15089 | | |
15056 | 15090 | | |
| |||
15186 | 15220 | | |
15187 | 15221 | | |
15188 | 15222 | | |
15189 | | - | |
| 15223 | + | |
| 15224 | + | |
| 15225 | + | |
| 15226 | + | |
| 15227 | + | |
| 15228 | + | |
| 15229 | + | |
15190 | 15230 | | |
15191 | 15231 | | |
15192 | 15232 | | |
| |||
15305 | 15345 | | |
15306 | 15346 | | |
15307 | 15347 | | |
| 15348 | + | |
| 15349 | + | |
| 15350 | + | |
| 15351 | + | |
| 15352 | + | |
| 15353 | + | |
15308 | 15354 | | |
15309 | 15355 | | |
15310 | 15356 | | |
| |||
0 commit comments