Repository navigation
fix(objectql)!: a system write's readonly value is judged for its shape — a seed's malformed readonly datetime is refused, never stored (#21663) - #21695
Conversation
…d for its shape
The static readonly strip exempts a system write (seed replay, migration,
hook stamps), and the record validator skipped every readonly field on the
premise that the strip had removed anything a caller sent. So under
isSystem a readonly value reached the driver unjudged: a seed's 'yesterday'
on a readonly datetime, or an unresolved cel envelope, was stored verbatim
while the same value on a non-readonly field was refused.
The strip keeps its system exemption. validateRecord gains a
readonlyValues scope ('skip' | 'include' | 'only'): the engine judges each
readonly value's SHAPE wherever the payload is final - in the same call on
insert and in the dry run (both after their strips), and in a second pass
after the strip on both update paths. A readonly value reaches the type's
shape arms only, with the non-readonly sentence: never required, and never
an author-declared constraint (option membership, bounds, valueDomain,
formats), which keeps the open-vocabulary ruling on sys_activity.type.
Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi
Co-authored-by: Claude <noreply@anthropic.com>
… all four write seams Triage's three pins on the real SeedLoaderService and the engine's own seed context: 'yesterday' on a readonly datetime is refused and counted as a seed error with the non-readonly sentence (insert, replay update, by-id and predicate update, dry run; VALIDATION_FAILED / 400 at the boundary); a valid ISO value - authored, evaluated from cel, or on created_at - is kept; the non-readonly path, the non-system strip and the readonly constraint arms (option membership, bounds) are unchanged. Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
engine-insert-static-readonly-strip: the isSystem 'strict adds no second policy' case used the placeholder 'x' in a readonly datetime. Its subject is strictReadonlyWrites, not value shape, so it now writes a valid instant like its isSystem sibling (a respelling, the case still pins what it pinned). record-validator.number-value: the normalizer's exclusion list pinned 'not a readonly field'. The number arm now judges a readonly value, so its numeric string is written as its number - #20309's own invariant. The case moves the readonly field to the rewritten side and keeps every other exclusion. Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
…ope is engine-internal The readonly scope is a fact only the engine's write path knows (whether its payload stands before or after the readonly strip), so it moves off the public ValidateRecordOptions onto a module-internal validateRecordInScope. validateRecord keeps its signature and behaviour byte for byte, and every engine seam now names its scope explicitly: 'skip' before the update strip, 'only' after it, 'include' on insert and in the dry run. Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
…em write's readonly value is judged for its shape Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
…med 'code'
check:error-code-casing reads a lowercase literal under a 'code' key as an
ADR-0112 error code. The pins' external-id field was named 'code', so every
row key ('bad', 'iso', ...) read as one. The field is arbitrary; renamed.
Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi
Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift Check9 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. What this run could not see
Coarse fallback — 17 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin b53f69d504a6cf47efacced0da6ea187160fe9ed && git checkout b53f69d504a6cf47efacced0da6ea187160fe9ed
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 38bef8cf95f1d0e3d2dc268eeef5e6b30718eed1 5c58fabb6e04be85379f504be4f248e84844382b && git checkout -B drift-repro 38bef8cf95f1d0e3d2dc268eeef5e6b30718eed1 && git merge --no-ff 5c58fabb6e04be85379f504be4f248e84844382b
node scripts/docs-audit/affected-docs.mjs --json 38bef8cf95f1d0e3d2dc268eeef5e6b30718eed1 |
ACCEPT — PR #21695 at head
|
…dateRecord again ADR-0020 anchors packages/objectql/src/engine.ts#validateRecord and quotes the update path's call, validateRecord(schema, hookContext.input.data, 'update'), as the one that sees only the PATCH payload. Spelling that call as validateRecordInScope(..., 'skip', ...) left the anchor unresolved (check:adr-symbol-anchors). The public validateRecord IS that scope, so the two pre-strip calls use it again: no behaviour change, the anchor resolves on the call it names, and the ADR text stays true as written. Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
ACCEPT addendum — PR #21695, patch round 1, head
|
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,
isSystemplugin 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 underisSystema malformed readonly value reached the driver unjudged.After this PR:
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.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 at72f3c74d60: the strip lives there (stripReadonlyFields), but the shape check and its readonly skip live inpackages/objectql/src/validation/record-validator.ts(validateRecord,if (def.system || def.readonly) continueon both walks). The engine (packages/objectql/src/engine.ts) runs the strip and the validator at different points:72f3c74d60validateRecordObjectQL.validate)validateRecordvalidateRecord, then the stripSo the fix lands in the producer's own file, with the engine choosing the scope at each seam:
record-validator.ts: aReadonlyValueScope('skip' | 'include' | 'only') and a module-internalvalidateRecordInScope. The publishedvalidateRecordkeeps 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 afterassertNoStrictDrops(), where the payload is final.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:
date/datetime/timethe 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).maxLength/minLength,valueDomain,min/max/scale/precision, the email / url / phone formats, andrequired.Option membership is the load-bearing exclusion.
sys_activity.typeis a readonlyselectwhose options are the built-in set of an open vocabulary. The maintainer ruling recorded at commit88b9d749abinds 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. Readonlyurlfields 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.
isSystem, readonly, at72f3c74d60isSystem, readonly, afterisSystem, non-readonly (unchanged)'yesterday'invalid_datecelenvelopeinvalid_date'yesterday'invalid_date'noon'invalid_time'abc'invalid_number'maybe'invalid_booleaninvalid_type{ $in: [...] }invalid_typemax: 5, value 9maxLength: 3, 6 charscelenvelope'nowhere'H3, H4, H5
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, acelvalue the loader evaluates, and an authoredcreated_atare kept. That last case pairs with the arm A seed row's explicitcreated_atis overwritten with the boot instant on INSERT (seed context sets nopreserveAudit), yet written on the upsert UPDATE of a later boot — seeds cannot backdate creation time consistently #21646 landed.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'sseed()(packages/verify/src/handle.ts). A rawcelenvelope on a readonly datetime is now refused on their call shapes (single-row and array insert underSEED_WRITE_EXECUTION_CONTEXT, pinned). No example app or test newly fails.runtime(4554 tests) andverify(131) are green. The only readonly field seeded withcelinexamples/iscreated_at, in 10app-showcasetask rows, and every one of those rows also seeds a non-readonlydue_datewithcel. 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-existingwarn-level per-row loss is in the Acceptance notes.plugin-audit621,plugin-pinyin-search21,plugin-security3527,plugin-auth2494,plugin-approvals875,service-automation2098,metadata-protocol3463,runtime4554,verify131. Inside objectql, two fixtures turned red and were re-judged, not relaxed:engine-insert-static-readonly-strip.test.ts: anisSystemcase used the placeholder'x'in a readonly datetime, in a test aboutstrictReadonlyWrites. It is respelled to a valid instant, like itsisSystemsibling.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 realSeedLoaderService. Each refusal assertscodeandstatus(ADR-0112) throughresolveThrownHttpError, the boundary's own reading. The engine'sValidationErrorcarries nostatusby design.'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) asVALIDATION_FAILED/ 400 with the non-readonly field envelope, for a rawcelenvelope, and for a malformed authoredcreated_at.cel, and oncreated_at, on insert and on replay.Reverse verification (committed first, at
196b217829). The split was reverted at its one predicate inrecord-validator.ts, throughscripts/ablation-replace.mjsunder a shell trap: anchorif (def.readonly === true) return scope !== 'skip';went from 1 to 0 hits, the replacementreturn falsefrom 0 to 1, and the blob fromd57cbd3078to3768d5668f. 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, andthe write must be refusedtwice); pin 2 and the three pin-3 tests stayed green. Restore wasgit checkout HEAD -- PATH: blob back tod57cbd3078(the HEAD blob),git diff HEAD0 bytes,git status --porcelainempty. No dist rebuild per leg was needed: the pin imports./engine.js/./plugin.jsfrom src by relative path.Tests
Head of record:
b73f58e396(after mergingorigin/mainat251a7dd4b4).pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=2 src/seed-readonly-value-shape.test.tsgivesTests 8 passed (8)atb73f58e396. With A seed row's explicitcreated_atis overwritten with the boot instant on INSERT (seed context sets nopreserveAudit), yet written on the upsert UPDATE of a later boot — seeds cannot backdate creation time consistently #21646's pin file beside it earlier:Tests 14 passed (14).vitest run --project local --maxWorkers=2givesTest Files 372 passed (372) / Tests 7455 passed (7455)and--project repogivesTests 5 passed (5), both ate6b5281680.pnpm --filter @objectstack/objectql run typecheckexits 0, withcheck:test-typecheck: OK … 40 file(s) / 234 error(s) / 65 pinned signature(s) held(ledger unchanged).tsc -p tsconfig.test.json --listFilesOnlylists the new pin file. The only change aftere6b5281680is the pin file's row-key rename (rerun green above).pnpm --filter PKG run test, against objectql's rebuiltdist/, at45804afc28): 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 ofmainwith no objectql overlap, and the pin rename.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackwith no paths derives 68 commands atb73f58e396, from 7 paths against merge base251a7dd4b. All 68 ran, each exit code captured before any pipe: 68 × exit 0.--ranreports68 derived, 68 run, 0 NOT-MEASURED, 0 UNRUN. Two readings came from the first union ate6b5281680:check:error-code-casingwas red. It read the pins' row key, a field namedcode, as a lowercase error code. The key is renamed toref, and the gate is green fromb73f58e396.check:dual-build-cjs-loadsexited 3 (PREREQUISITE NOT MET). After a fullturbo run build(72/72) it exits 0.b73f58e396): 50 exit 0.check-closing-target-claim,check-partof-closing-keywordandcheck-single-claim-pathsreport NOT WIRED with no PR context (exit 2, not a verdict).check-partof-closing-keywordwas then run with this body asPR_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.check-changeset-no-major,check-adr-0087-registration(1 declared-breaking changeset, carrying its disposition) andcheck-empty-changesetall exit 0.pnpm lintis CI-owned and was not run.Acceptance notes
owner_idkeeps the full skip. It issystembut notreadonly, 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.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 toonAdmittedValueShapeViolation, never stored silently. Theos migrate value-shapesscan population is unchanged.AppPlugin's fallback seeders log a refused row atwarnand then report "Data seeding complete", whileSeedLoaderServicelogs the same loss aterror. This is pre-existing and not caused here. carrier: none.validateRecordskips readonly fields" (plugin-audit sources and tests, and two ADR-0087 semantic entries inpackages/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.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