Uh oh!
There was an error while loading. Please reload this page.
test(objectql): declare the recorded-by fixture's lookup with the canonical reference key - #14633
Merged
Merged
Conversation
…onical `reference` key
`protocol-recorded-by-null.test.ts` declared `recorded_by` as
`{ type: 'lookup', referenceTo: 'sys_user', readonly: true }` under a comment
claiming it is "the real declaration". `referenceTo` is an alias `FieldSchema`
refuses by name, and `referenceTargetOf` — the single arbiter the #4441
write-path referential check resolves through — does not read it, so the lookup
presented as target-less.
Measured on the fixture's own write path before renaming anything, by counting
the guard's target probe: with `referenceTo` spelled, no probe ran even with the
`readonly` exemption removed; with `reference` spelled, it runs. So the
exemption the file's second half claims to exercise had never admitted these
writes — the target-less skip did.
Renames the key and adds the two pins that keep the header's claim honest: the
declaration resolves through `referenceTargetOf`, and an actor id no `sys_user`
row matches is still admitted (which is the exemption, and is now red if the
exemption is deleted).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68…corded-by-reference-spelling
Contributor
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
os-musk
marked this pull request as ready for review
September 2, 2026 16:12
os-musk
enabled auto-merge
September 2, 2026 16:12
os-musk
commented
Sep 2, 2026
CollaboratorAuthor
Landing provenance (engine execution seat, session
Generated by Claude Code |
This was referenced Sep 2, 2026
Uh oh!
There was an error while loading. Please reload this page.
os-musk
deleted the
claude/issue-14535-recorded-by-reference-spelling
branch
September 2, 2026 18:06
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#14535
protocol-recorded-by-null.test.tsdeclaredrecorded_byas{ type: 'lookup', referenceTo: 'sys_user', readonly: true }under a comment claiming it is "the real declaration". The rename is the last step of this change, not its point: the card ruled that what the file's second half has been proving had to be established first, with an instrument rather than a reading.Step 1 — the instrument, and what it read
referenceTargetOf(packages/spec/src/data/field-value.zod.ts:141) is the single arbiter the #4441 write-path guard resolves a lookup's target through:assertReferencesResolvecalls it atpackages/objectql/src/engine.ts:5814and skips the field atif (!target) continuewhen it answers nothing. It reads thereferencekey only (plus the implicit target oftype: 'user').Instrument. A temporary probe wrapped the engine's own
assertReferencesResolveandreferenceExistson this fixture's real protocol write path, and counted the target probe the guard performs. Control: the fourth row below — the same instrument does register a probe when a target resolves, so an empty count is a real absence rather than a blind instrument.Reading, over the fixture's own writes (
saveMetaItemwith an actor and without one):referenceTo+readonly: true(as written)reference+readonly: true(after the rename)referenceTo,readonlyremovedreference,readonlyremovedreferenceExists('sys_user', 'usr_alice')and directly:
referenceTargetOfanswersundefinedfor thereferenceTospelling and'sys_user'forreference.So the guard did run here — this is a non-system write, and
recorded_bywas present both in the caller-supplied payload and in the normalized data. It simply never looked at the field's target, because to the arbiter there was none. Thereadonlyexemption the file's second half claims to exercise is not what admitted these writes; the target-less skip is. Row three is the proof: with the alias spelled, deleting the exemption changes nothing at all.That is the triage's step 1 answered in its own terms — the exemption has been exercised against a field with no target, not against the case the file says it covers.
Steps 2 and 3 — the rename, and the assertions that had to change with it
Because the branch was never reached, this is not a rename. Two pins are added so the file proves what its header says:
referenceTargetOf(sysMetadataHistoryObject.fields.recorded_by)is'sys_user'. This is the fidelity claim as an assertion instead of prose. A raw object literal handed to the registry is never parsed byFieldSchema, so the alias could never be refused where it was written ([finding] docs vs driver disagree on whether lookup columns get a real FOREIGN KEY — and the driver branch keys onreference_to, notreference#11567), and nothing in the suite could notice.sys_userrow matches is still admitted — the data: a lookup accepts an id that does not exist in the referenced object — including the RBAC permission-set link tables #4441 exemption, at the protocol layer where the regression was found. This is not thesys_metadata_history.recorded_by是lookup('sys_user')却存哨兵字符串'system'——声明的类型与实际存的值不是一回事 #4556 sentinel returning:'system'was a string the platform minted for every actor-less write, which this suite still refuses; an actor the caller named is the caller's own value, and data: a lookup accepts an id that does not exist in the referenced object — including the RBAC permission-set link tables #4441 deliberately does not police areadonlylookup (the residual is reported by theisSystem写入仍可产生悬空 lookup 引用——需要一条只报告不拦截的巡检(#4441 残留) #4551 audit instead).The file header now records the measurement in place of the claim it could not support.
Reverse verification — the 2x2
Each cell is the whole suite re-run; mutations were applied with an anchored edit whose landing was proved by counting the injected and the removed text, restored from
HEADunder atrap, and both files verified byte-identical to theirHEADblobs afterwards (git hash-objectcompared togit rev-parse HEAD:path).readonlyexemption inengine.tsreference(this PR)referenceTo(rename reverted)reference(this PR)referenceToCells 3 and 4 are the same engine ablation with opposite outcomes, and the only difference between them is the spelling. That is what the rename buys: the exemption becomes load-bearing for this fixture. Cell 2 is the reverse verification for pin 1.
The
engine.tsmutation was a measurement only and is not part of this diff, which is one test file. The exemption's engine-layer coverage already exists and is spelled canonically —engine-lookup-referential-integrity.test.ts, "a READONLY lookup is not the caller's to answer for"; this file's protocol-layer half was the vacuous one.Tests
All at
64f4383eb, the head of this branch. Heavy runs went throughscripts/pm/os-verify-lock.sh; exit codes captured after a redirect, never through a pipe.pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=2 src/protocol-recorded-by-null.test.ts— exit 0,Test Files 1 passed (1) / Tests 7 passed (7).pnpm --filter @objectstack/objectql typecheck— exit 0, includingcheck:test-typecheck: OK — @objectstack/objectql's test layer compiles under packages/objectql/tsconfig.test.json; 44 file(s) / 242 error(s) / 69 pinned signature(s) held(the exact per-file ratchet: the new test file adds none).pnpm lint(whole repo,eslint . --no-inline-config) — exit 0, no output. Not narrowed.pnpm check:nul-bytes—OK (scanned 7986 text file(s) ... no raw ASCII control bytes).pnpm check:error-status-conformance— exit 0.node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands(identical to the dispatch seed): 25 of 28 green, quoted in the report comment on the card.check-test-completeness.mjs("running the family locally, record this gate as NOT MEASURED" — it grades a savedturbo run testlog CI tees),check:dual-build-cjs-loadsandcheck:type-check-debt(both need the whole built closure — "Runpnpm buildfirst. This is NOT a pass: nothing was measured"). All three measure the built world, which a test-only diff does not move; CI runs them after the closure build.node scripts/check-changeset-no-major.mjs— exit 0.skip-changeset: the diff is one test file, publishes nothing, andcheck:published-filesis green with tests excluded from every whitelist.🤖 Generated with Claude Code
https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
Generated by Claude Code