fix(objectql): decide the insert-side runtime-owned strip by hook-write provenance - #14472

Merged
os-musk merged 7 commits into
mainfrom
claude/issue-14259-hook-provenance-sibling-seams
Sep 2, 2026
Merged

fix(objectql): decide the insert-side runtime-owned strip by hook-write provenance#14472
os-musk merged 7 commits into
mainfrom
claude/issue-14259-hook-provenance-sibling-seams

Conversation

@os-musk

Copy link
Copy Markdown
Collaborator

Part of #14259

⚠️Part of, not a closing keyword, and that is the headline.#14259 names TWO sibling seams. One is here and finished; the other is WITHHELD pending a maintainer ruling, because implementing it was measured to violate the card's own binding constraint. Merging this must not close the card.

Verified on git rev-parse --short HEAD = 25619ebb7 (origin/main merged immediately before opening).


What ships: seam 2 — stripRuntimeOwnedFields, the INSERT-side strip

#14088 replaced Object.is(payload[k], supplied[k]) inside stripReadonlyFields with a recording of the keys the before-phase hook chain actually assigned (recordHookPayloadWrites). Its argument was never about null: value equality cannot separate

  • the hook deliberately wrote the value the caller also sent, from
  • the hook never touched the key at all,

and the two demand opposite verdicts. stripRuntimeOwnedFields was left on the comparison that argument retired, and #6339's own prose is the finding: it argued a key SET made the contract true "only BY ACCIDENT" and moved to VALUES, which is accidental in the identical way. So #6339's own measured hook — a beforeInsert that re-issues or normalises a record number — still loses its write to the one caller that submitted the same value.

The change.engine.insert arms one recordHookPayloadWritesper row at hook-context construction and seals each immediately after that row's beforeInsert chain; stripRuntimeOwnedFields gains the same optional hookWrittenKeys#14088 gave stripReadonlyFields, consulted before the value test, with the same no-record fallback.

engine.ts regions touched — declared, and nothing else

packages/objectql/src/engine.ts is a hot file, so the touch is region-bounded and the regions are named here as they were in the dev claim:

regionwhat
the insert path's rowHookContexts constructionarm one recording per row over defaultedData
immediately after the beforeInsert dispatch loopseal each row, restore the raw payload into input.data
the stripRuntimeOwnedFields call sitepass that row's hookWrittenKeys
import type beside the existing recordHookPayloadWrites importone line

No other engine.ts hunk. The two update-path readonlyWhen call sites named in the dev claim are not touched (see the withheld seam below). At the time of opening, no other open PR touches packages/objectql/src/validation/rule-validator.ts or these regions.

The forgery boundary the NEW recording owes

The card carries it verbatim — "The record is safe only because it is armed after the caller's payload has arrived and sealed before any engine-owned pass touches it, and because it records that an assignment ran rather than anything about the payload's contents. Any producer of a hook-written key set owes the same proof." The insert-side recording is new, so here is that proof, each property measured rather than asserted:

  1. Armed after the caller's payload has arrived. The arm sits on defaultedData — after suppliedPerRow has taken its explicit shallow copy of the caller's rows, and after the engine's own applyFieldDefaults / initializeSummaryFields. So neither a caller key nor an engine default can enter the record. Pinned: FORGERY BOUNDARY: the recording is armed AFTER the caller payload arrives.
  2. Sealed before any engine-owned pass touches it. Measured between the beforeInsert dispatch and the strip: the post-hook declared-field door (undeclaredWriteFieldErrors, reads only), resolveSystemInsertOrganization (measured read-only over rows — it consults row[tenantField] and writes nothing), buildDriverOptions (writes input.options, never input.data), then refuseEmptyPasswordFields and encryptSecretFields — the last of which does write to the row and is exactly what the recorder's own header names as the escalation risk. The seal is placed ahead of all of them, immediately after the dispatch loop, so the only code inside the window is before-phase hook code. Pinned: FORGERY BOUNDARY: the recording is SEALED before any engine-owned pass.
  3. Records that an assignment ran, not the contents. Pinned in both directions: a hook assigning the caller's own value and a hook assigning a different one reach the same verdict.

Two further properties the update path's single recording does not have to carry:

  • Per row, never per call. A batch arms one recorder per row, so a hook stamping row 0 confers nothing on row 1 — pinned, including that row 1's caller seed is still stripped even when the value is the very one row 0's hook legitimately wrote.
  • No recording view reaches a driver. The seal puts the RAW row back in input.data for every row, including one the declared-field door culled (that row ran no hook, so its record is legitimately empty).

⛔ Not a relaxation of #5503 — the negative controls

The accept set for callers does not move. A caller-seeded record number that no hook assigned is still stripped, still warns with the same text, and still reports through both observability seams. Asserted through the documented surfaces rather than by toEqual on the result:

  • the warning line still contains Field 'code' on 'prov_ticket', runtime-owned, COMMITTED WITHOUT IT and hook-written keys are not caller-supplied;
  • onFieldsDropped still fires with reason: 'readonly' naming code;
  • strictReadonlyWrites still REFUSES — asserted as an envelope (nameReadonlyFieldRejectedError, codeERR_READONLY_FIELD_REJECTED, fields containing code, operationinsert) plus zero rows reaching the driver, never as a bare toThrow(). ⚠️status is deliberately not asserted: this class carries code and name only, the HTTP mapping living at the protocol layer, so pinning a status here would pin a member the class does not have.

isSystem and preserveAudit are untouched. A hook that REPLACES the payload object leaves no attributable record and falls back to the pre-existing value test, which over-strips — pinned, because reading a replacement's keys as hook-owned would launder a caller's forgery, and keeping the old bug is the only safe direction.


What is WITHHELD: seam 1 — isCallerSuppliedValue, behind the two readonlyWhen strips

Implemented, measured, then reverted out of this PR. The card's own fork clause fired.

Threading the sealed record into isCallerSuppliedValue turns the existing #9107 pin LOCK 3b red. Measured, exact:

FAIL engine-readonly-when-derived-writes.test.ts
> LOCK 3b — a hook that writes the caller value BACK is the caller value, and goes
AssertionError: expected '1999-01-01' to be null

The hook in that pin is ctx.input.data.closed_note = ctx.input.data.closed_note — a self-assignment that computes nothing. Under Object.is it "has written nothing". Under provenance it is a set on the recorded object, so the key becomes hook-owned and the caller's forged value survives a TRUE readonlyWhen predicate on a record.status == 'closed' state lock. The pin's own prose says why it exists: "Pinned because 'a hook touched this key' is exactly the weaker rule that WOULD open a laundering path."

That is the fork clause verbatim — a case where seam 1 under provenance lets a caller value survive a TRUE predicate — so it goes to the decision inbox and is not resolved here, in either direction. Nothing about the readonlyWhen seam's behaviour changes in this PR, and no existing pin is relaxed.

The tension is real and not a bug in either half: the mechanism the card mandates records that an assignment ran and is deliberately blind to the value, which makes "a hook deliberately wrote the value the caller also sent" (must be KEPT, per the inherited ruling) and "a hook echoed the caller's value onto a locked column" (must be STRIPPED, per LOCK 3b) mechanically indistinguishable. A "value differs from what was there" filter does not separate them either — it would re-break #14088's own measured completed_at: null row, where the hook's assignment is value-identical to what already stands on the key.

⚠️ The same shape is measured and pinned on the seam that ships, so the asymmetry is visible rather than discovered later: a lone self-assigning beforeInsert hook does leave the caller's seeded record number on the key. It ships here because stripRuntimeOwnedFields guards a runtime-owned COLUMN (#5503) — the same class of protection #14088 already moved to provenance on the update side, and the class whose hook exemption runtimeOwnedStripWarning promises authors in prose — not a STATE lock whose whole purpose is that no caller write survives a TRUE predicate. INSERT is exempt from readonlyWhen entirely, so no lock of that class exists on this path to open. A ruling that self-assignment must not count would move that pin and #14088's seam together, deliberately.

Seam 3 from the card (arming the recorder before the middleware chain) is out of scope by triage and was not implemented; no middleware stamp needing it was measured.


Verification

All readings on 25619ebb7 unless stated. Every exit code captured after a redirect, never through a pipe; each verdict quoted from the gate's own line.

Tests.pnpm --filter @objectstack/objectql testTest Files 258 passed (258) · Tests 4470 passed (4470). pnpm --filter @objectstack/objectql typecheck → exit 0, check:test-typecheck: OK (44 files / 242 errors / 69 pinned signatures held — the ledger did not move).

⚠️ The new pin file is IN the typecheck program, not merely un-flagged: tsc --noEmit -p tsconfig.test.json --listFiles names it (1 hit, not 0).

New pin filepackages/objectql/src/engine-hook-provenance-sibling-seams.test.ts — 13 cases, all green, alongside every neighbouring pin (engine-readonly-strip-caller-values, engine-readonly-when-derived-writes, engine-insert-runtime-owned-strip, hook-write-provenance, validation/rule-validator, validation/skip-provenance): 7 files / 268 tests green together.

Ablation, direction predicted before the run and matched: reverting the hookWrittenKeys?.has(name) branch in stripRuntimeOwnedFields turns 5 red / 8 green — every DEFECT and consequence case red, every negative control green. No rebuild leg applies: the pin imports ./engine.js, a relative specifier inside the same package's src/, so vitest resolves the subject from source and never from dist/ — the ablation going red with no build in between is itself the evidence.

The mutation was proven on disk before the run (exact occurrence counts 1 -> 0 deleted, 0 -> 1 injected, counted in Python because the anchor is multi-line and grep -c would count either half of it). ⚠️ Worth recording: the first attempt's marker matched 2 occurrences — the bare branch line is byte-identical to #14088's inside stripReadonlyFields two functions up — and the on-disk guard refused the run rather than let a reading contaminated by ablating #14088's seam through. The anchor was narrowed to the preceding comment. Restore was proven by observed state, not exit code: git hash-object equal to the HEAD blob, git diff HEAD empty, git status --porcelain empty, with the restore path pinned to an absolute repo root inside the trap.

Lint. The whole population, not a narrowing: eslint . --no-inline-config --format json (the exact rule set pnpm lint runs) → 5702 files, 0 errors, 0 warnings, exit 0.

Gates. Re-derived after the last commit with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands (no path list — the script takes its own change set), then run as a union on this HEAD: 60 green, 0 red, 4 NOT MEASURED. The four are exit 3 prerequisite refusals, recorded in the gates' own words and never as passes:

  • check-test-completeness — needs a saved turbo run test log this gate cannot produce; CI tees one.
  • check-half-states — needs repo-scoped GitHub reads this container's egress refuses.
  • check:dual-build-cjs-loadsPREREQUISITE NOT MET, reads built output for 56 unbuilt packages.
  • check:type-check-debtPREREQUISITE NOT MET, --re-measure needs 41 dependency closures built.

check:skill-examples initially refused for the same reason (packages/client-react/dist unbuilt, and its own text says a verdict then would be a FALSE GREEN); the closure was built and it now reports 263 prose examples type-check across 3 surfaces, exit 0.

Gate scaffolding, regenerated and never hand-edited.check-system-context-census went red on pure line rot — the insert-path arm/seal shifts every later engine.ts line — and was repaired with node scripts/check-system-context-census.mjs --fix (15 anchors rewritten in content/docs/permissions/system-context.mdx; the gate then reports OK — 109 elevation read sites ... all anchored). check:objectql-double-limit flagged the new pin's driver double for ignoring the caller's bound; the double now applies limit after the filter, by presence, and the gate is green. check:engine-double-contract was green throughout and needed no ledger row.

⚠️ One measurement trap worth recording, because it read exactly like a regression: immediately after merging origin/main, engine-filter-array-lowering.test.ts failed 6 cases. It was stale dist/, not code — the merge brought packages/spec changes my worktree's build predated. Clean origin/main in a comparison worktree ran the same file 60/60 green, and rebuilding this worktree's dependency closure made the failure vanish. Nothing was "fixed"; the instrument was.

Scope

Changeset: @objectstack/objectqlpatch. packages/objectql-internal throughout — none of the four functions is re-exported from index.ts or core.ts, and every mention outside rule-validator.ts / engine.ts is prose in a comment (packages/spec, packages/metadata-protocol, three packages/lint files, one showcase object) or a test in this package. The new option is optional and every positional signature is unchanged, so no existing call site moves.

🤖 Generated with Claude Code

https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68


Generated by Claude Code

… runtime-owned strips
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…te provenance
Withholds the readonlyWhen sibling seam: threading the record into
isCallerSuppliedValue was measured to let a caller value survive a TRUE
readonlyWhen predicate (#9107 pin LOCK 3b), which #14259's fork clause
routes to the decision inbox rather than to a unilateral choice.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…le; re-anchor the system-context census
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql, touching 2 documentable anchor(s).

18 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aaf.

1 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 15 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aafpackageMentionDocs.

Which tree this was computed on

This run read content/docs from afdbce2baed0487c867d978b8956d531e297b5cf — the merge of head 25619ebb76b68f80ef5489dddb101255bdc1d802 into base 18d816a504a5795a13c609304eab5825cf923aaf, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin afdbce2baed0487c867d978b8956d531e297b5cf && git checkout afdbce2baed0487c867d978b8956d531e297b5cf
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 18d816a504a5795a13c609304eab5825cf923aaf 25619ebb76b68f80ef5489dddb101255bdc1d802 && git checkout -B drift-repro 18d816a504a5795a13c609304eab5825cf923aaf && git merge --no-ff 25619ebb76b68f80ef5489dddb101255bdc1d802
node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aaf

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 18d816a504a5795a13c609304eab5825cf923aaf → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-muskClaude

Copy link
Copy Markdown
CollaboratorAuthor

Landing provenance — engine execution seat (session session_0112hMx9hjJ9BgB28X97DS68).

ACCEPT on the card: comment 5506811038 (#14259 — seam 2 ships here as Part of; seam 1 is in the decision box, four-facet block 5506819337). Flip pre-checks on head 25619ebb7: every one of the 34 check runs completed with conclusion success or skipped (Lint & Repo Gates completed 08:50:47Z; Part-of PR must not also close its card green); governed-surface test on the final 5-path file list: 0 of 5 path(s) hit the register ⇒ ordinary queue landing; closing-keyword two-read done at ACCEPT (Part of #14259, no closing keyword — correct, the card stays open as the seam-1 carrier). Clause-② self-reading no (package-internal functions, optional parameter, no public-entry export), so no contract-review carrier applied.

Action: draft: false then auto-merge (squash) — the merge queue takes it from here. At MERGED the card flips pm:dispatchedneeds-user-decision in one stroke.


Generated by Claude Code

@os-musk
os-musk enabled auto-merge September 2, 2026 08:53
@os-musk
os-musk added this pull request to the merge queueSep 2, 2026
Merged via the queue into main with commit 00ff228Sep 2, 2026
36 checks passed
@os-musk
os-musk deleted the claude/issue-14259-hook-provenance-sibling-seams branch September 2, 2026 09:18
os-musk pushed a commit that referenced this pull request Sep 2, 2026
…th faces
`isCallerSuppliedValue`'s docblock claimed the readonlyWhen seam runs "the
identical two-part test `stripReadonlyFields` applies" and is "textually
parallel" with it. Neither has held since #14088 moved the static face onto a
hook-write record and #14472 did the same to the insert-side twin. The
divergence is deliberate and lock-motivated, so it is now stated where the test
lives, with the reason each face guards a different thing.
Adds the static face's missing measurement pin (a lone self-assigning hook
leaves the CALLER value on the key) beside #14088's own suite, and cross-links
it with `LOCK 3b`, which pins the opposite verdict for the identical hook
spelling. Comment- and test-only; no behaviour moves.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@os-musk@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(objectql): decide the insert-side runtime-owned strip by hook-write provenance - #14472

Merged
os-musk merged 7 commits into
mainfrom
claude/issue-14259-hook-provenance-sibling-seams
Sep 2, 2026
Merged

fix(objectql): decide the insert-side runtime-owned strip by hook-write provenance#14472
os-musk merged 7 commits into
mainfrom
claude/issue-14259-hook-provenance-sibling-seams

Conversation

@os-musk

Copy link
Copy Markdown
Collaborator

Part of #14259

⚠️Part of, not a closing keyword, and that is the headline.#14259 names TWO sibling seams. One is here and finished; the other is WITHHELD pending a maintainer ruling, because implementing it was measured to violate the card's own binding constraint. Merging this must not close the card.

Verified on git rev-parse --short HEAD = 25619ebb7 (origin/main merged immediately before opening).


What ships: seam 2 — stripRuntimeOwnedFields, the INSERT-side strip

#14088 replaced Object.is(payload[k], supplied[k]) inside stripReadonlyFields with a recording of the keys the before-phase hook chain actually assigned (recordHookPayloadWrites). Its argument was never about null: value equality cannot separate

  • the hook deliberately wrote the value the caller also sent, from
  • the hook never touched the key at all,

and the two demand opposite verdicts. stripRuntimeOwnedFields was left on the comparison that argument retired, and #6339's own prose is the finding: it argued a key SET made the contract true "only BY ACCIDENT" and moved to VALUES, which is accidental in the identical way. So #6339's own measured hook — a beforeInsert that re-issues or normalises a record number — still loses its write to the one caller that submitted the same value.

The change.engine.insert arms one recordHookPayloadWritesper row at hook-context construction and seals each immediately after that row's beforeInsert chain; stripRuntimeOwnedFields gains the same optional hookWrittenKeys#14088 gave stripReadonlyFields, consulted before the value test, with the same no-record fallback.

engine.ts regions touched — declared, and nothing else

packages/objectql/src/engine.ts is a hot file, so the touch is region-bounded and the regions are named here as they were in the dev claim:

regionwhat
the insert path's rowHookContexts constructionarm one recording per row over defaultedData
immediately after the beforeInsert dispatch loopseal each row, restore the raw payload into input.data
the stripRuntimeOwnedFields call sitepass that row's hookWrittenKeys
import type beside the existing recordHookPayloadWrites importone line

No other engine.ts hunk. The two update-path readonlyWhen call sites named in the dev claim are not touched (see the withheld seam below). At the time of opening, no other open PR touches packages/objectql/src/validation/rule-validator.ts or these regions.

The forgery boundary the NEW recording owes

The card carries it verbatim — "The record is safe only because it is armed after the caller's payload has arrived and sealed before any engine-owned pass touches it, and because it records that an assignment ran rather than anything about the payload's contents. Any producer of a hook-written key set owes the same proof." The insert-side recording is new, so here is that proof, each property measured rather than asserted:

  1. Armed after the caller's payload has arrived. The arm sits on defaultedData — after suppliedPerRow has taken its explicit shallow copy of the caller's rows, and after the engine's own applyFieldDefaults / initializeSummaryFields. So neither a caller key nor an engine default can enter the record. Pinned: FORGERY BOUNDARY: the recording is armed AFTER the caller payload arrives.
  2. Sealed before any engine-owned pass touches it. Measured between the beforeInsert dispatch and the strip: the post-hook declared-field door (undeclaredWriteFieldErrors, reads only), resolveSystemInsertOrganization (measured read-only over rows — it consults row[tenantField] and writes nothing), buildDriverOptions (writes input.options, never input.data), then refuseEmptyPasswordFields and encryptSecretFields — the last of which does write to the row and is exactly what the recorder's own header names as the escalation risk. The seal is placed ahead of all of them, immediately after the dispatch loop, so the only code inside the window is before-phase hook code. Pinned: FORGERY BOUNDARY: the recording is SEALED before any engine-owned pass.
  3. Records that an assignment ran, not the contents. Pinned in both directions: a hook assigning the caller's own value and a hook assigning a different one reach the same verdict.

Two further properties the update path's single recording does not have to carry:

  • Per row, never per call. A batch arms one recorder per row, so a hook stamping row 0 confers nothing on row 1 — pinned, including that row 1's caller seed is still stripped even when the value is the very one row 0's hook legitimately wrote.
  • No recording view reaches a driver. The seal puts the RAW row back in input.data for every row, including one the declared-field door culled (that row ran no hook, so its record is legitimately empty).

⛔ Not a relaxation of #5503 — the negative controls

The accept set for callers does not move. A caller-seeded record number that no hook assigned is still stripped, still warns with the same text, and still reports through both observability seams. Asserted through the documented surfaces rather than by toEqual on the result:

  • the warning line still contains Field 'code' on 'prov_ticket', runtime-owned, COMMITTED WITHOUT IT and hook-written keys are not caller-supplied;
  • onFieldsDropped still fires with reason: 'readonly' naming code;
  • strictReadonlyWrites still REFUSES — asserted as an envelope (nameReadonlyFieldRejectedError, codeERR_READONLY_FIELD_REJECTED, fields containing code, operationinsert) plus zero rows reaching the driver, never as a bare toThrow(). ⚠️status is deliberately not asserted: this class carries code and name only, the HTTP mapping living at the protocol layer, so pinning a status here would pin a member the class does not have.

isSystem and preserveAudit are untouched. A hook that REPLACES the payload object leaves no attributable record and falls back to the pre-existing value test, which over-strips — pinned, because reading a replacement's keys as hook-owned would launder a caller's forgery, and keeping the old bug is the only safe direction.


What is WITHHELD: seam 1 — isCallerSuppliedValue, behind the two readonlyWhen strips

Implemented, measured, then reverted out of this PR. The card's own fork clause fired.

Threading the sealed record into isCallerSuppliedValue turns the existing #9107 pin LOCK 3b red. Measured, exact:

FAIL engine-readonly-when-derived-writes.test.ts
> LOCK 3b — a hook that writes the caller value BACK is the caller value, and goes
AssertionError: expected '1999-01-01' to be null

The hook in that pin is ctx.input.data.closed_note = ctx.input.data.closed_note — a self-assignment that computes nothing. Under Object.is it "has written nothing". Under provenance it is a set on the recorded object, so the key becomes hook-owned and the caller's forged value survives a TRUE readonlyWhen predicate on a record.status == 'closed' state lock. The pin's own prose says why it exists: "Pinned because 'a hook touched this key' is exactly the weaker rule that WOULD open a laundering path."

That is the fork clause verbatim — a case where seam 1 under provenance lets a caller value survive a TRUE predicate — so it goes to the decision inbox and is not resolved here, in either direction. Nothing about the readonlyWhen seam's behaviour changes in this PR, and no existing pin is relaxed.

The tension is real and not a bug in either half: the mechanism the card mandates records that an assignment ran and is deliberately blind to the value, which makes "a hook deliberately wrote the value the caller also sent" (must be KEPT, per the inherited ruling) and "a hook echoed the caller's value onto a locked column" (must be STRIPPED, per LOCK 3b) mechanically indistinguishable. A "value differs from what was there" filter does not separate them either — it would re-break #14088's own measured completed_at: null row, where the hook's assignment is value-identical to what already stands on the key.

⚠️ The same shape is measured and pinned on the seam that ships, so the asymmetry is visible rather than discovered later: a lone self-assigning beforeInsert hook does leave the caller's seeded record number on the key. It ships here because stripRuntimeOwnedFields guards a runtime-owned COLUMN (#5503) — the same class of protection #14088 already moved to provenance on the update side, and the class whose hook exemption runtimeOwnedStripWarning promises authors in prose — not a STATE lock whose whole purpose is that no caller write survives a TRUE predicate. INSERT is exempt from readonlyWhen entirely, so no lock of that class exists on this path to open. A ruling that self-assignment must not count would move that pin and #14088's seam together, deliberately.

Seam 3 from the card (arming the recorder before the middleware chain) is out of scope by triage and was not implemented; no middleware stamp needing it was measured.


Verification

All readings on 25619ebb7 unless stated. Every exit code captured after a redirect, never through a pipe; each verdict quoted from the gate's own line.

Tests.pnpm --filter @objectstack/objectql testTest Files 258 passed (258) · Tests 4470 passed (4470). pnpm --filter @objectstack/objectql typecheck → exit 0, check:test-typecheck: OK (44 files / 242 errors / 69 pinned signatures held — the ledger did not move).

⚠️ The new pin file is IN the typecheck program, not merely un-flagged: tsc --noEmit -p tsconfig.test.json --listFiles names it (1 hit, not 0).

New pin filepackages/objectql/src/engine-hook-provenance-sibling-seams.test.ts — 13 cases, all green, alongside every neighbouring pin (engine-readonly-strip-caller-values, engine-readonly-when-derived-writes, engine-insert-runtime-owned-strip, hook-write-provenance, validation/rule-validator, validation/skip-provenance): 7 files / 268 tests green together.

Ablation, direction predicted before the run and matched: reverting the hookWrittenKeys?.has(name) branch in stripRuntimeOwnedFields turns 5 red / 8 green — every DEFECT and consequence case red, every negative control green. No rebuild leg applies: the pin imports ./engine.js, a relative specifier inside the same package's src/, so vitest resolves the subject from source and never from dist/ — the ablation going red with no build in between is itself the evidence.

The mutation was proven on disk before the run (exact occurrence counts 1 -> 0 deleted, 0 -> 1 injected, counted in Python because the anchor is multi-line and grep -c would count either half of it). ⚠️ Worth recording: the first attempt's marker matched 2 occurrences — the bare branch line is byte-identical to #14088's inside stripReadonlyFields two functions up — and the on-disk guard refused the run rather than let a reading contaminated by ablating #14088's seam through. The anchor was narrowed to the preceding comment. Restore was proven by observed state, not exit code: git hash-object equal to the HEAD blob, git diff HEAD empty, git status --porcelain empty, with the restore path pinned to an absolute repo root inside the trap.

Lint. The whole population, not a narrowing: eslint . --no-inline-config --format json (the exact rule set pnpm lint runs) → 5702 files, 0 errors, 0 warnings, exit 0.

Gates. Re-derived after the last commit with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands (no path list — the script takes its own change set), then run as a union on this HEAD: 60 green, 0 red, 4 NOT MEASURED. The four are exit 3 prerequisite refusals, recorded in the gates' own words and never as passes:

  • check-test-completeness — needs a saved turbo run test log this gate cannot produce; CI tees one.
  • check-half-states — needs repo-scoped GitHub reads this container's egress refuses.
  • check:dual-build-cjs-loadsPREREQUISITE NOT MET, reads built output for 56 unbuilt packages.
  • check:type-check-debtPREREQUISITE NOT MET, --re-measure needs 41 dependency closures built.

check:skill-examples initially refused for the same reason (packages/client-react/dist unbuilt, and its own text says a verdict then would be a FALSE GREEN); the closure was built and it now reports 263 prose examples type-check across 3 surfaces, exit 0.

Gate scaffolding, regenerated and never hand-edited.check-system-context-census went red on pure line rot — the insert-path arm/seal shifts every later engine.ts line — and was repaired with node scripts/check-system-context-census.mjs --fix (15 anchors rewritten in content/docs/permissions/system-context.mdx; the gate then reports OK — 109 elevation read sites ... all anchored). check:objectql-double-limit flagged the new pin's driver double for ignoring the caller's bound; the double now applies limit after the filter, by presence, and the gate is green. check:engine-double-contract was green throughout and needed no ledger row.

⚠️ One measurement trap worth recording, because it read exactly like a regression: immediately after merging origin/main, engine-filter-array-lowering.test.ts failed 6 cases. It was stale dist/, not code — the merge brought packages/spec changes my worktree's build predated. Clean origin/main in a comparison worktree ran the same file 60/60 green, and rebuilding this worktree's dependency closure made the failure vanish. Nothing was "fixed"; the instrument was.

Scope

Changeset: @objectstack/objectqlpatch. packages/objectql-internal throughout — none of the four functions is re-exported from index.ts or core.ts, and every mention outside rule-validator.ts / engine.ts is prose in a comment (packages/spec, packages/metadata-protocol, three packages/lint files, one showcase object) or a test in this package. The new option is optional and every positional signature is unchanged, so no existing call site moves.

🤖 Generated with Claude Code

https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68


Generated by Claude Code

… runtime-owned strips
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…te provenance
Withholds the readonlyWhen sibling seam: threading the record into
isCallerSuppliedValue was measured to let a caller value survive a TRUE
readonlyWhen predicate (#9107 pin LOCK 3b), which #14259's fork clause
routes to the decision inbox rather than to a unilateral choice.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…le; re-anchor the system-context census
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql, touching 2 documentable anchor(s).

18 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aaf.

1 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 15 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aafpackageMentionDocs.

Which tree this was computed on

This run read content/docs from afdbce2baed0487c867d978b8956d531e297b5cf — the merge of head 25619ebb76b68f80ef5489dddb101255bdc1d802 into base 18d816a504a5795a13c609304eab5825cf923aaf, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin afdbce2baed0487c867d978b8956d531e297b5cf && git checkout afdbce2baed0487c867d978b8956d531e297b5cf
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 18d816a504a5795a13c609304eab5825cf923aaf 25619ebb76b68f80ef5489dddb101255bdc1d802 && git checkout -B drift-repro 18d816a504a5795a13c609304eab5825cf923aaf && git merge --no-ff 25619ebb76b68f80ef5489dddb101255bdc1d802
node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aaf

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 18d816a504a5795a13c609304eab5825cf923aaf → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-muskClaude

Copy link
Copy Markdown
CollaboratorAuthor

Landing provenance — engine execution seat (session session_0112hMx9hjJ9BgB28X97DS68).

ACCEPT on the card: comment 5506811038 (#14259 — seam 2 ships here as Part of; seam 1 is in the decision box, four-facet block 5506819337). Flip pre-checks on head 25619ebb7: every one of the 34 check runs completed with conclusion success or skipped (Lint & Repo Gates completed 08:50:47Z; Part-of PR must not also close its card green); governed-surface test on the final 5-path file list: 0 of 5 path(s) hit the register ⇒ ordinary queue landing; closing-keyword two-read done at ACCEPT (Part of #14259, no closing keyword — correct, the card stays open as the seam-1 carrier). Clause-② self-reading no (package-internal functions, optional parameter, no public-entry export), so no contract-review carrier applied.

Action: draft: false then auto-merge (squash) — the merge queue takes it from here. At MERGED the card flips pm:dispatchedneeds-user-decision in one stroke.


Generated by Claude Code

@os-musk
os-musk enabled auto-merge September 2, 2026 08:53
@os-musk
os-musk added this pull request to the merge queueSep 2, 2026
Merged via the queue into main with commit 00ff228Sep 2, 2026
36 checks passed
@os-musk
os-musk deleted the claude/issue-14259-hook-provenance-sibling-seams branch September 2, 2026 09:18
os-musk pushed a commit that referenced this pull request Sep 2, 2026
…th faces
`isCallerSuppliedValue`'s docblock claimed the readonlyWhen seam runs "the
identical two-part test `stripReadonlyFields` applies" and is "textually
parallel" with it. Neither has held since #14088 moved the static face onto a
hook-write record and #14472 did the same to the insert-side twin. The
divergence is deliberate and lock-motivated, so it is now stated where the test
lives, with the reason each face guards a different thing.
Adds the static face's missing measurement pin (a lone self-assigning hook
leaves the CALLER value on the key) beside #14088's own suite, and cross-links
it with `LOCK 3b`, which pins the opposite verdict for the identical hook
spelling. Comment- and test-only; no behaviour moves.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@os-musk@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(objectql): decide the insert-side runtime-owned strip by hook-write provenance - #14472

Merged
os-musk merged 7 commits into
mainfrom
claude/issue-14259-hook-provenance-sibling-seams
Sep 2, 2026
Merged

fix(objectql): decide the insert-side runtime-owned strip by hook-write provenance#14472
os-musk merged 7 commits into
mainfrom
claude/issue-14259-hook-provenance-sibling-seams

Conversation

@os-musk

Copy link
Copy Markdown
Collaborator

Part of #14259

⚠️Part of, not a closing keyword, and that is the headline.#14259 names TWO sibling seams. One is here and finished; the other is WITHHELD pending a maintainer ruling, because implementing it was measured to violate the card's own binding constraint. Merging this must not close the card.

Verified on git rev-parse --short HEAD = 25619ebb7 (origin/main merged immediately before opening).


What ships: seam 2 — stripRuntimeOwnedFields, the INSERT-side strip

#14088 replaced Object.is(payload[k], supplied[k]) inside stripReadonlyFields with a recording of the keys the before-phase hook chain actually assigned (recordHookPayloadWrites). Its argument was never about null: value equality cannot separate

  • the hook deliberately wrote the value the caller also sent, from
  • the hook never touched the key at all,

and the two demand opposite verdicts. stripRuntimeOwnedFields was left on the comparison that argument retired, and #6339's own prose is the finding: it argued a key SET made the contract true "only BY ACCIDENT" and moved to VALUES, which is accidental in the identical way. So #6339's own measured hook — a beforeInsert that re-issues or normalises a record number — still loses its write to the one caller that submitted the same value.

The change.engine.insert arms one recordHookPayloadWritesper row at hook-context construction and seals each immediately after that row's beforeInsert chain; stripRuntimeOwnedFields gains the same optional hookWrittenKeys#14088 gave stripReadonlyFields, consulted before the value test, with the same no-record fallback.

engine.ts regions touched — declared, and nothing else

packages/objectql/src/engine.ts is a hot file, so the touch is region-bounded and the regions are named here as they were in the dev claim:

regionwhat
the insert path's rowHookContexts constructionarm one recording per row over defaultedData
immediately after the beforeInsert dispatch loopseal each row, restore the raw payload into input.data
the stripRuntimeOwnedFields call sitepass that row's hookWrittenKeys
import type beside the existing recordHookPayloadWrites importone line

No other engine.ts hunk. The two update-path readonlyWhen call sites named in the dev claim are not touched (see the withheld seam below). At the time of opening, no other open PR touches packages/objectql/src/validation/rule-validator.ts or these regions.

The forgery boundary the NEW recording owes

The card carries it verbatim — "The record is safe only because it is armed after the caller's payload has arrived and sealed before any engine-owned pass touches it, and because it records that an assignment ran rather than anything about the payload's contents. Any producer of a hook-written key set owes the same proof." The insert-side recording is new, so here is that proof, each property measured rather than asserted:

  1. Armed after the caller's payload has arrived. The arm sits on defaultedData — after suppliedPerRow has taken its explicit shallow copy of the caller's rows, and after the engine's own applyFieldDefaults / initializeSummaryFields. So neither a caller key nor an engine default can enter the record. Pinned: FORGERY BOUNDARY: the recording is armed AFTER the caller payload arrives.
  2. Sealed before any engine-owned pass touches it. Measured between the beforeInsert dispatch and the strip: the post-hook declared-field door (undeclaredWriteFieldErrors, reads only), resolveSystemInsertOrganization (measured read-only over rows — it consults row[tenantField] and writes nothing), buildDriverOptions (writes input.options, never input.data), then refuseEmptyPasswordFields and encryptSecretFields — the last of which does write to the row and is exactly what the recorder's own header names as the escalation risk. The seal is placed ahead of all of them, immediately after the dispatch loop, so the only code inside the window is before-phase hook code. Pinned: FORGERY BOUNDARY: the recording is SEALED before any engine-owned pass.
  3. Records that an assignment ran, not the contents. Pinned in both directions: a hook assigning the caller's own value and a hook assigning a different one reach the same verdict.

Two further properties the update path's single recording does not have to carry:

  • Per row, never per call. A batch arms one recorder per row, so a hook stamping row 0 confers nothing on row 1 — pinned, including that row 1's caller seed is still stripped even when the value is the very one row 0's hook legitimately wrote.
  • No recording view reaches a driver. The seal puts the RAW row back in input.data for every row, including one the declared-field door culled (that row ran no hook, so its record is legitimately empty).

⛔ Not a relaxation of #5503 — the negative controls

The accept set for callers does not move. A caller-seeded record number that no hook assigned is still stripped, still warns with the same text, and still reports through both observability seams. Asserted through the documented surfaces rather than by toEqual on the result:

  • the warning line still contains Field 'code' on 'prov_ticket', runtime-owned, COMMITTED WITHOUT IT and hook-written keys are not caller-supplied;
  • onFieldsDropped still fires with reason: 'readonly' naming code;
  • strictReadonlyWrites still REFUSES — asserted as an envelope (nameReadonlyFieldRejectedError, codeERR_READONLY_FIELD_REJECTED, fields containing code, operationinsert) plus zero rows reaching the driver, never as a bare toThrow(). ⚠️status is deliberately not asserted: this class carries code and name only, the HTTP mapping living at the protocol layer, so pinning a status here would pin a member the class does not have.

isSystem and preserveAudit are untouched. A hook that REPLACES the payload object leaves no attributable record and falls back to the pre-existing value test, which over-strips — pinned, because reading a replacement's keys as hook-owned would launder a caller's forgery, and keeping the old bug is the only safe direction.


What is WITHHELD: seam 1 — isCallerSuppliedValue, behind the two readonlyWhen strips

Implemented, measured, then reverted out of this PR. The card's own fork clause fired.

Threading the sealed record into isCallerSuppliedValue turns the existing #9107 pin LOCK 3b red. Measured, exact:

FAIL engine-readonly-when-derived-writes.test.ts
> LOCK 3b — a hook that writes the caller value BACK is the caller value, and goes
AssertionError: expected '1999-01-01' to be null

The hook in that pin is ctx.input.data.closed_note = ctx.input.data.closed_note — a self-assignment that computes nothing. Under Object.is it "has written nothing". Under provenance it is a set on the recorded object, so the key becomes hook-owned and the caller's forged value survives a TRUE readonlyWhen predicate on a record.status == 'closed' state lock. The pin's own prose says why it exists: "Pinned because 'a hook touched this key' is exactly the weaker rule that WOULD open a laundering path."

That is the fork clause verbatim — a case where seam 1 under provenance lets a caller value survive a TRUE predicate — so it goes to the decision inbox and is not resolved here, in either direction. Nothing about the readonlyWhen seam's behaviour changes in this PR, and no existing pin is relaxed.

The tension is real and not a bug in either half: the mechanism the card mandates records that an assignment ran and is deliberately blind to the value, which makes "a hook deliberately wrote the value the caller also sent" (must be KEPT, per the inherited ruling) and "a hook echoed the caller's value onto a locked column" (must be STRIPPED, per LOCK 3b) mechanically indistinguishable. A "value differs from what was there" filter does not separate them either — it would re-break #14088's own measured completed_at: null row, where the hook's assignment is value-identical to what already stands on the key.

⚠️ The same shape is measured and pinned on the seam that ships, so the asymmetry is visible rather than discovered later: a lone self-assigning beforeInsert hook does leave the caller's seeded record number on the key. It ships here because stripRuntimeOwnedFields guards a runtime-owned COLUMN (#5503) — the same class of protection #14088 already moved to provenance on the update side, and the class whose hook exemption runtimeOwnedStripWarning promises authors in prose — not a STATE lock whose whole purpose is that no caller write survives a TRUE predicate. INSERT is exempt from readonlyWhen entirely, so no lock of that class exists on this path to open. A ruling that self-assignment must not count would move that pin and #14088's seam together, deliberately.

Seam 3 from the card (arming the recorder before the middleware chain) is out of scope by triage and was not implemented; no middleware stamp needing it was measured.


Verification

All readings on 25619ebb7 unless stated. Every exit code captured after a redirect, never through a pipe; each verdict quoted from the gate's own line.

Tests.pnpm --filter @objectstack/objectql testTest Files 258 passed (258) · Tests 4470 passed (4470). pnpm --filter @objectstack/objectql typecheck → exit 0, check:test-typecheck: OK (44 files / 242 errors / 69 pinned signatures held — the ledger did not move).

⚠️ The new pin file is IN the typecheck program, not merely un-flagged: tsc --noEmit -p tsconfig.test.json --listFiles names it (1 hit, not 0).

New pin filepackages/objectql/src/engine-hook-provenance-sibling-seams.test.ts — 13 cases, all green, alongside every neighbouring pin (engine-readonly-strip-caller-values, engine-readonly-when-derived-writes, engine-insert-runtime-owned-strip, hook-write-provenance, validation/rule-validator, validation/skip-provenance): 7 files / 268 tests green together.

Ablation, direction predicted before the run and matched: reverting the hookWrittenKeys?.has(name) branch in stripRuntimeOwnedFields turns 5 red / 8 green — every DEFECT and consequence case red, every negative control green. No rebuild leg applies: the pin imports ./engine.js, a relative specifier inside the same package's src/, so vitest resolves the subject from source and never from dist/ — the ablation going red with no build in between is itself the evidence.

The mutation was proven on disk before the run (exact occurrence counts 1 -> 0 deleted, 0 -> 1 injected, counted in Python because the anchor is multi-line and grep -c would count either half of it). ⚠️ Worth recording: the first attempt's marker matched 2 occurrences — the bare branch line is byte-identical to #14088's inside stripReadonlyFields two functions up — and the on-disk guard refused the run rather than let a reading contaminated by ablating #14088's seam through. The anchor was narrowed to the preceding comment. Restore was proven by observed state, not exit code: git hash-object equal to the HEAD blob, git diff HEAD empty, git status --porcelain empty, with the restore path pinned to an absolute repo root inside the trap.

Lint. The whole population, not a narrowing: eslint . --no-inline-config --format json (the exact rule set pnpm lint runs) → 5702 files, 0 errors, 0 warnings, exit 0.

Gates. Re-derived after the last commit with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands (no path list — the script takes its own change set), then run as a union on this HEAD: 60 green, 0 red, 4 NOT MEASURED. The four are exit 3 prerequisite refusals, recorded in the gates' own words and never as passes:

  • check-test-completeness — needs a saved turbo run test log this gate cannot produce; CI tees one.
  • check-half-states — needs repo-scoped GitHub reads this container's egress refuses.
  • check:dual-build-cjs-loadsPREREQUISITE NOT MET, reads built output for 56 unbuilt packages.
  • check:type-check-debtPREREQUISITE NOT MET, --re-measure needs 41 dependency closures built.

check:skill-examples initially refused for the same reason (packages/client-react/dist unbuilt, and its own text says a verdict then would be a FALSE GREEN); the closure was built and it now reports 263 prose examples type-check across 3 surfaces, exit 0.

Gate scaffolding, regenerated and never hand-edited.check-system-context-census went red on pure line rot — the insert-path arm/seal shifts every later engine.ts line — and was repaired with node scripts/check-system-context-census.mjs --fix (15 anchors rewritten in content/docs/permissions/system-context.mdx; the gate then reports OK — 109 elevation read sites ... all anchored). check:objectql-double-limit flagged the new pin's driver double for ignoring the caller's bound; the double now applies limit after the filter, by presence, and the gate is green. check:engine-double-contract was green throughout and needed no ledger row.

⚠️ One measurement trap worth recording, because it read exactly like a regression: immediately after merging origin/main, engine-filter-array-lowering.test.ts failed 6 cases. It was stale dist/, not code — the merge brought packages/spec changes my worktree's build predated. Clean origin/main in a comparison worktree ran the same file 60/60 green, and rebuilding this worktree's dependency closure made the failure vanish. Nothing was "fixed"; the instrument was.

Scope

Changeset: @objectstack/objectqlpatch. packages/objectql-internal throughout — none of the four functions is re-exported from index.ts or core.ts, and every mention outside rule-validator.ts / engine.ts is prose in a comment (packages/spec, packages/metadata-protocol, three packages/lint files, one showcase object) or a test in this package. The new option is optional and every positional signature is unchanged, so no existing call site moves.

🤖 Generated with Claude Code

https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68


Generated by Claude Code

… runtime-owned strips
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…te provenance
Withholds the readonlyWhen sibling seam: threading the record into
isCallerSuppliedValue was measured to let a caller value survive a TRUE
readonlyWhen predicate (#9107 pin LOCK 3b), which #14259's fork clause
routes to the decision inbox rather than to a unilateral choice.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…le; re-anchor the system-context census
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql, touching 2 documentable anchor(s).

18 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aaf.

1 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 15 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aafpackageMentionDocs.

Which tree this was computed on

This run read content/docs from afdbce2baed0487c867d978b8956d531e297b5cf — the merge of head 25619ebb76b68f80ef5489dddb101255bdc1d802 into base 18d816a504a5795a13c609304eab5825cf923aaf, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin afdbce2baed0487c867d978b8956d531e297b5cf && git checkout afdbce2baed0487c867d978b8956d531e297b5cf
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 18d816a504a5795a13c609304eab5825cf923aaf 25619ebb76b68f80ef5489dddb101255bdc1d802 && git checkout -B drift-repro 18d816a504a5795a13c609304eab5825cf923aaf && git merge --no-ff 25619ebb76b68f80ef5489dddb101255bdc1d802
node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aaf

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 18d816a504a5795a13c609304eab5825cf923aaf → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-muskClaude

Copy link
Copy Markdown
CollaboratorAuthor

Landing provenance — engine execution seat (session session_0112hMx9hjJ9BgB28X97DS68).

ACCEPT on the card: comment 5506811038 (#14259 — seam 2 ships here as Part of; seam 1 is in the decision box, four-facet block 5506819337). Flip pre-checks on head 25619ebb7: every one of the 34 check runs completed with conclusion success or skipped (Lint & Repo Gates completed 08:50:47Z; Part-of PR must not also close its card green); governed-surface test on the final 5-path file list: 0 of 5 path(s) hit the register ⇒ ordinary queue landing; closing-keyword two-read done at ACCEPT (Part of #14259, no closing keyword — correct, the card stays open as the seam-1 carrier). Clause-② self-reading no (package-internal functions, optional parameter, no public-entry export), so no contract-review carrier applied.

Action: draft: false then auto-merge (squash) — the merge queue takes it from here. At MERGED the card flips pm:dispatchedneeds-user-decision in one stroke.


Generated by Claude Code

@os-musk
os-musk enabled auto-merge September 2, 2026 08:53
@os-musk
os-musk added this pull request to the merge queueSep 2, 2026
Merged via the queue into main with commit 00ff228Sep 2, 2026
36 checks passed
@os-musk
os-musk deleted the claude/issue-14259-hook-provenance-sibling-seams branch September 2, 2026 09:18
os-musk pushed a commit that referenced this pull request Sep 2, 2026
…th faces
`isCallerSuppliedValue`'s docblock claimed the readonlyWhen seam runs "the
identical two-part test `stripReadonlyFields` applies" and is "textually
parallel" with it. Neither has held since #14088 moved the static face onto a
hook-write record and #14472 did the same to the insert-side twin. The
divergence is deliberate and lock-motivated, so it is now stated where the test
lives, with the reason each face guards a different thing.
Adds the static face's missing measurement pin (a lone self-assigning hook
leaves the CALLER value on the key) beside #14088's own suite, and cross-links
it with `LOCK 3b`, which pins the opposite verdict for the identical hook
spelling. Comment- and test-only; no behaviour moves.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@os-musk@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(objectql): decide the insert-side runtime-owned strip by hook-write provenance - #14472

Merged
os-musk merged 7 commits into
mainfrom
claude/issue-14259-hook-provenance-sibling-seams
Sep 2, 2026
Merged

fix(objectql): decide the insert-side runtime-owned strip by hook-write provenance#14472
os-musk merged 7 commits into
mainfrom
claude/issue-14259-hook-provenance-sibling-seams

Conversation

@os-musk

Copy link
Copy Markdown
Collaborator

Part of #14259

⚠️Part of, not a closing keyword, and that is the headline.#14259 names TWO sibling seams. One is here and finished; the other is WITHHELD pending a maintainer ruling, because implementing it was measured to violate the card's own binding constraint. Merging this must not close the card.

Verified on git rev-parse --short HEAD = 25619ebb7 (origin/main merged immediately before opening).


What ships: seam 2 — stripRuntimeOwnedFields, the INSERT-side strip

#14088 replaced Object.is(payload[k], supplied[k]) inside stripReadonlyFields with a recording of the keys the before-phase hook chain actually assigned (recordHookPayloadWrites). Its argument was never about null: value equality cannot separate

  • the hook deliberately wrote the value the caller also sent, from
  • the hook never touched the key at all,

and the two demand opposite verdicts. stripRuntimeOwnedFields was left on the comparison that argument retired, and #6339's own prose is the finding: it argued a key SET made the contract true "only BY ACCIDENT" and moved to VALUES, which is accidental in the identical way. So #6339's own measured hook — a beforeInsert that re-issues or normalises a record number — still loses its write to the one caller that submitted the same value.

The change.engine.insert arms one recordHookPayloadWritesper row at hook-context construction and seals each immediately after that row's beforeInsert chain; stripRuntimeOwnedFields gains the same optional hookWrittenKeys#14088 gave stripReadonlyFields, consulted before the value test, with the same no-record fallback.

engine.ts regions touched — declared, and nothing else

packages/objectql/src/engine.ts is a hot file, so the touch is region-bounded and the regions are named here as they were in the dev claim:

regionwhat
the insert path's rowHookContexts constructionarm one recording per row over defaultedData
immediately after the beforeInsert dispatch loopseal each row, restore the raw payload into input.data
the stripRuntimeOwnedFields call sitepass that row's hookWrittenKeys
import type beside the existing recordHookPayloadWrites importone line

No other engine.ts hunk. The two update-path readonlyWhen call sites named in the dev claim are not touched (see the withheld seam below). At the time of opening, no other open PR touches packages/objectql/src/validation/rule-validator.ts or these regions.

The forgery boundary the NEW recording owes

The card carries it verbatim — "The record is safe only because it is armed after the caller's payload has arrived and sealed before any engine-owned pass touches it, and because it records that an assignment ran rather than anything about the payload's contents. Any producer of a hook-written key set owes the same proof." The insert-side recording is new, so here is that proof, each property measured rather than asserted:

  1. Armed after the caller's payload has arrived. The arm sits on defaultedData — after suppliedPerRow has taken its explicit shallow copy of the caller's rows, and after the engine's own applyFieldDefaults / initializeSummaryFields. So neither a caller key nor an engine default can enter the record. Pinned: FORGERY BOUNDARY: the recording is armed AFTER the caller payload arrives.
  2. Sealed before any engine-owned pass touches it. Measured between the beforeInsert dispatch and the strip: the post-hook declared-field door (undeclaredWriteFieldErrors, reads only), resolveSystemInsertOrganization (measured read-only over rows — it consults row[tenantField] and writes nothing), buildDriverOptions (writes input.options, never input.data), then refuseEmptyPasswordFields and encryptSecretFields — the last of which does write to the row and is exactly what the recorder's own header names as the escalation risk. The seal is placed ahead of all of them, immediately after the dispatch loop, so the only code inside the window is before-phase hook code. Pinned: FORGERY BOUNDARY: the recording is SEALED before any engine-owned pass.
  3. Records that an assignment ran, not the contents. Pinned in both directions: a hook assigning the caller's own value and a hook assigning a different one reach the same verdict.

Two further properties the update path's single recording does not have to carry:

  • Per row, never per call. A batch arms one recorder per row, so a hook stamping row 0 confers nothing on row 1 — pinned, including that row 1's caller seed is still stripped even when the value is the very one row 0's hook legitimately wrote.
  • No recording view reaches a driver. The seal puts the RAW row back in input.data for every row, including one the declared-field door culled (that row ran no hook, so its record is legitimately empty).

⛔ Not a relaxation of #5503 — the negative controls

The accept set for callers does not move. A caller-seeded record number that no hook assigned is still stripped, still warns with the same text, and still reports through both observability seams. Asserted through the documented surfaces rather than by toEqual on the result:

  • the warning line still contains Field 'code' on 'prov_ticket', runtime-owned, COMMITTED WITHOUT IT and hook-written keys are not caller-supplied;
  • onFieldsDropped still fires with reason: 'readonly' naming code;
  • strictReadonlyWrites still REFUSES — asserted as an envelope (nameReadonlyFieldRejectedError, codeERR_READONLY_FIELD_REJECTED, fields containing code, operationinsert) plus zero rows reaching the driver, never as a bare toThrow(). ⚠️status is deliberately not asserted: this class carries code and name only, the HTTP mapping living at the protocol layer, so pinning a status here would pin a member the class does not have.

isSystem and preserveAudit are untouched. A hook that REPLACES the payload object leaves no attributable record and falls back to the pre-existing value test, which over-strips — pinned, because reading a replacement's keys as hook-owned would launder a caller's forgery, and keeping the old bug is the only safe direction.


What is WITHHELD: seam 1 — isCallerSuppliedValue, behind the two readonlyWhen strips

Implemented, measured, then reverted out of this PR. The card's own fork clause fired.

Threading the sealed record into isCallerSuppliedValue turns the existing #9107 pin LOCK 3b red. Measured, exact:

FAIL engine-readonly-when-derived-writes.test.ts
> LOCK 3b — a hook that writes the caller value BACK is the caller value, and goes
AssertionError: expected '1999-01-01' to be null

The hook in that pin is ctx.input.data.closed_note = ctx.input.data.closed_note — a self-assignment that computes nothing. Under Object.is it "has written nothing". Under provenance it is a set on the recorded object, so the key becomes hook-owned and the caller's forged value survives a TRUE readonlyWhen predicate on a record.status == 'closed' state lock. The pin's own prose says why it exists: "Pinned because 'a hook touched this key' is exactly the weaker rule that WOULD open a laundering path."

That is the fork clause verbatim — a case where seam 1 under provenance lets a caller value survive a TRUE predicate — so it goes to the decision inbox and is not resolved here, in either direction. Nothing about the readonlyWhen seam's behaviour changes in this PR, and no existing pin is relaxed.

The tension is real and not a bug in either half: the mechanism the card mandates records that an assignment ran and is deliberately blind to the value, which makes "a hook deliberately wrote the value the caller also sent" (must be KEPT, per the inherited ruling) and "a hook echoed the caller's value onto a locked column" (must be STRIPPED, per LOCK 3b) mechanically indistinguishable. A "value differs from what was there" filter does not separate them either — it would re-break #14088's own measured completed_at: null row, where the hook's assignment is value-identical to what already stands on the key.

⚠️ The same shape is measured and pinned on the seam that ships, so the asymmetry is visible rather than discovered later: a lone self-assigning beforeInsert hook does leave the caller's seeded record number on the key. It ships here because stripRuntimeOwnedFields guards a runtime-owned COLUMN (#5503) — the same class of protection #14088 already moved to provenance on the update side, and the class whose hook exemption runtimeOwnedStripWarning promises authors in prose — not a STATE lock whose whole purpose is that no caller write survives a TRUE predicate. INSERT is exempt from readonlyWhen entirely, so no lock of that class exists on this path to open. A ruling that self-assignment must not count would move that pin and #14088's seam together, deliberately.

Seam 3 from the card (arming the recorder before the middleware chain) is out of scope by triage and was not implemented; no middleware stamp needing it was measured.


Verification

All readings on 25619ebb7 unless stated. Every exit code captured after a redirect, never through a pipe; each verdict quoted from the gate's own line.

Tests.pnpm --filter @objectstack/objectql testTest Files 258 passed (258) · Tests 4470 passed (4470). pnpm --filter @objectstack/objectql typecheck → exit 0, check:test-typecheck: OK (44 files / 242 errors / 69 pinned signatures held — the ledger did not move).

⚠️ The new pin file is IN the typecheck program, not merely un-flagged: tsc --noEmit -p tsconfig.test.json --listFiles names it (1 hit, not 0).

New pin filepackages/objectql/src/engine-hook-provenance-sibling-seams.test.ts — 13 cases, all green, alongside every neighbouring pin (engine-readonly-strip-caller-values, engine-readonly-when-derived-writes, engine-insert-runtime-owned-strip, hook-write-provenance, validation/rule-validator, validation/skip-provenance): 7 files / 268 tests green together.

Ablation, direction predicted before the run and matched: reverting the hookWrittenKeys?.has(name) branch in stripRuntimeOwnedFields turns 5 red / 8 green — every DEFECT and consequence case red, every negative control green. No rebuild leg applies: the pin imports ./engine.js, a relative specifier inside the same package's src/, so vitest resolves the subject from source and never from dist/ — the ablation going red with no build in between is itself the evidence.

The mutation was proven on disk before the run (exact occurrence counts 1 -> 0 deleted, 0 -> 1 injected, counted in Python because the anchor is multi-line and grep -c would count either half of it). ⚠️ Worth recording: the first attempt's marker matched 2 occurrences — the bare branch line is byte-identical to #14088's inside stripReadonlyFields two functions up — and the on-disk guard refused the run rather than let a reading contaminated by ablating #14088's seam through. The anchor was narrowed to the preceding comment. Restore was proven by observed state, not exit code: git hash-object equal to the HEAD blob, git diff HEAD empty, git status --porcelain empty, with the restore path pinned to an absolute repo root inside the trap.

Lint. The whole population, not a narrowing: eslint . --no-inline-config --format json (the exact rule set pnpm lint runs) → 5702 files, 0 errors, 0 warnings, exit 0.

Gates. Re-derived after the last commit with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands (no path list — the script takes its own change set), then run as a union on this HEAD: 60 green, 0 red, 4 NOT MEASURED. The four are exit 3 prerequisite refusals, recorded in the gates' own words and never as passes:

  • check-test-completeness — needs a saved turbo run test log this gate cannot produce; CI tees one.
  • check-half-states — needs repo-scoped GitHub reads this container's egress refuses.
  • check:dual-build-cjs-loadsPREREQUISITE NOT MET, reads built output for 56 unbuilt packages.
  • check:type-check-debtPREREQUISITE NOT MET, --re-measure needs 41 dependency closures built.

check:skill-examples initially refused for the same reason (packages/client-react/dist unbuilt, and its own text says a verdict then would be a FALSE GREEN); the closure was built and it now reports 263 prose examples type-check across 3 surfaces, exit 0.

Gate scaffolding, regenerated and never hand-edited.check-system-context-census went red on pure line rot — the insert-path arm/seal shifts every later engine.ts line — and was repaired with node scripts/check-system-context-census.mjs --fix (15 anchors rewritten in content/docs/permissions/system-context.mdx; the gate then reports OK — 109 elevation read sites ... all anchored). check:objectql-double-limit flagged the new pin's driver double for ignoring the caller's bound; the double now applies limit after the filter, by presence, and the gate is green. check:engine-double-contract was green throughout and needed no ledger row.

⚠️ One measurement trap worth recording, because it read exactly like a regression: immediately after merging origin/main, engine-filter-array-lowering.test.ts failed 6 cases. It was stale dist/, not code — the merge brought packages/spec changes my worktree's build predated. Clean origin/main in a comparison worktree ran the same file 60/60 green, and rebuilding this worktree's dependency closure made the failure vanish. Nothing was "fixed"; the instrument was.

Scope

Changeset: @objectstack/objectqlpatch. packages/objectql-internal throughout — none of the four functions is re-exported from index.ts or core.ts, and every mention outside rule-validator.ts / engine.ts is prose in a comment (packages/spec, packages/metadata-protocol, three packages/lint files, one showcase object) or a test in this package. The new option is optional and every positional signature is unchanged, so no existing call site moves.

🤖 Generated with Claude Code

https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68


Generated by Claude Code

… runtime-owned strips
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…te provenance
Withholds the readonlyWhen sibling seam: threading the record into
isCallerSuppliedValue was measured to let a caller value survive a TRUE
readonlyWhen predicate (#9107 pin LOCK 3b), which #14259's fork clause
routes to the decision inbox rather than to a unilateral choice.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…le; re-anchor the system-context census
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql, touching 2 documentable anchor(s).

18 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aaf.

1 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 15 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aafpackageMentionDocs.

Which tree this was computed on

This run read content/docs from afdbce2baed0487c867d978b8956d531e297b5cf — the merge of head 25619ebb76b68f80ef5489dddb101255bdc1d802 into base 18d816a504a5795a13c609304eab5825cf923aaf, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin afdbce2baed0487c867d978b8956d531e297b5cf && git checkout afdbce2baed0487c867d978b8956d531e297b5cf
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 18d816a504a5795a13c609304eab5825cf923aaf 25619ebb76b68f80ef5489dddb101255bdc1d802 && git checkout -B drift-repro 18d816a504a5795a13c609304eab5825cf923aaf && git merge --no-ff 25619ebb76b68f80ef5489dddb101255bdc1d802
node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aaf

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 18d816a504a5795a13c609304eab5825cf923aaf → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-muskClaude

Copy link
Copy Markdown
CollaboratorAuthor

Landing provenance — engine execution seat (session session_0112hMx9hjJ9BgB28X97DS68).

ACCEPT on the card: comment 5506811038 (#14259 — seam 2 ships here as Part of; seam 1 is in the decision box, four-facet block 5506819337). Flip pre-checks on head 25619ebb7: every one of the 34 check runs completed with conclusion success or skipped (Lint & Repo Gates completed 08:50:47Z; Part-of PR must not also close its card green); governed-surface test on the final 5-path file list: 0 of 5 path(s) hit the register ⇒ ordinary queue landing; closing-keyword two-read done at ACCEPT (Part of #14259, no closing keyword — correct, the card stays open as the seam-1 carrier). Clause-② self-reading no (package-internal functions, optional parameter, no public-entry export), so no contract-review carrier applied.

Action: draft: false then auto-merge (squash) — the merge queue takes it from here. At MERGED the card flips pm:dispatchedneeds-user-decision in one stroke.


Generated by Claude Code

@os-musk
os-musk enabled auto-merge September 2, 2026 08:53
@os-musk
os-musk added this pull request to the merge queueSep 2, 2026
Merged via the queue into main with commit 00ff228Sep 2, 2026
36 checks passed
@os-musk
os-musk deleted the claude/issue-14259-hook-provenance-sibling-seams branch September 2, 2026 09:18
os-musk pushed a commit that referenced this pull request Sep 2, 2026
…th faces
`isCallerSuppliedValue`'s docblock claimed the readonlyWhen seam runs "the
identical two-part test `stripReadonlyFields` applies" and is "textually
parallel" with it. Neither has held since #14088 moved the static face onto a
hook-write record and #14472 did the same to the insert-side twin. The
divergence is deliberate and lock-motivated, so it is now stated where the test
lives, with the reason each face guards a different thing.
Adds the static face's missing measurement pin (a lone self-assigning hook
leaves the CALLER value on the key) beside #14088's own suite, and cross-links
it with `LOCK 3b`, which pins the opposite verdict for the identical hook
spelling. Comment- and test-only; no behaviour moves.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@os-musk@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(objectql): decide the insert-side runtime-owned strip by hook-write provenance - #14472

Merged
os-musk merged 7 commits into
mainfrom
claude/issue-14259-hook-provenance-sibling-seams
Sep 2, 2026
Merged

fix(objectql): decide the insert-side runtime-owned strip by hook-write provenance#14472
os-musk merged 7 commits into
mainfrom
claude/issue-14259-hook-provenance-sibling-seams

Conversation

@os-musk

Copy link
Copy Markdown
Collaborator

Part of #14259

⚠️Part of, not a closing keyword, and that is the headline.#14259 names TWO sibling seams. One is here and finished; the other is WITHHELD pending a maintainer ruling, because implementing it was measured to violate the card's own binding constraint. Merging this must not close the card.

Verified on git rev-parse --short HEAD = 25619ebb7 (origin/main merged immediately before opening).


What ships: seam 2 — stripRuntimeOwnedFields, the INSERT-side strip

#14088 replaced Object.is(payload[k], supplied[k]) inside stripReadonlyFields with a recording of the keys the before-phase hook chain actually assigned (recordHookPayloadWrites). Its argument was never about null: value equality cannot separate

  • the hook deliberately wrote the value the caller also sent, from
  • the hook never touched the key at all,

and the two demand opposite verdicts. stripRuntimeOwnedFields was left on the comparison that argument retired, and #6339's own prose is the finding: it argued a key SET made the contract true "only BY ACCIDENT" and moved to VALUES, which is accidental in the identical way. So #6339's own measured hook — a beforeInsert that re-issues or normalises a record number — still loses its write to the one caller that submitted the same value.

The change.engine.insert arms one recordHookPayloadWritesper row at hook-context construction and seals each immediately after that row's beforeInsert chain; stripRuntimeOwnedFields gains the same optional hookWrittenKeys#14088 gave stripReadonlyFields, consulted before the value test, with the same no-record fallback.

engine.ts regions touched — declared, and nothing else

packages/objectql/src/engine.ts is a hot file, so the touch is region-bounded and the regions are named here as they were in the dev claim:

regionwhat
the insert path's rowHookContexts constructionarm one recording per row over defaultedData
immediately after the beforeInsert dispatch loopseal each row, restore the raw payload into input.data
the stripRuntimeOwnedFields call sitepass that row's hookWrittenKeys
import type beside the existing recordHookPayloadWrites importone line

No other engine.ts hunk. The two update-path readonlyWhen call sites named in the dev claim are not touched (see the withheld seam below). At the time of opening, no other open PR touches packages/objectql/src/validation/rule-validator.ts or these regions.

The forgery boundary the NEW recording owes

The card carries it verbatim — "The record is safe only because it is armed after the caller's payload has arrived and sealed before any engine-owned pass touches it, and because it records that an assignment ran rather than anything about the payload's contents. Any producer of a hook-written key set owes the same proof." The insert-side recording is new, so here is that proof, each property measured rather than asserted:

  1. Armed after the caller's payload has arrived. The arm sits on defaultedData — after suppliedPerRow has taken its explicit shallow copy of the caller's rows, and after the engine's own applyFieldDefaults / initializeSummaryFields. So neither a caller key nor an engine default can enter the record. Pinned: FORGERY BOUNDARY: the recording is armed AFTER the caller payload arrives.
  2. Sealed before any engine-owned pass touches it. Measured between the beforeInsert dispatch and the strip: the post-hook declared-field door (undeclaredWriteFieldErrors, reads only), resolveSystemInsertOrganization (measured read-only over rows — it consults row[tenantField] and writes nothing), buildDriverOptions (writes input.options, never input.data), then refuseEmptyPasswordFields and encryptSecretFields — the last of which does write to the row and is exactly what the recorder's own header names as the escalation risk. The seal is placed ahead of all of them, immediately after the dispatch loop, so the only code inside the window is before-phase hook code. Pinned: FORGERY BOUNDARY: the recording is SEALED before any engine-owned pass.
  3. Records that an assignment ran, not the contents. Pinned in both directions: a hook assigning the caller's own value and a hook assigning a different one reach the same verdict.

Two further properties the update path's single recording does not have to carry:

  • Per row, never per call. A batch arms one recorder per row, so a hook stamping row 0 confers nothing on row 1 — pinned, including that row 1's caller seed is still stripped even when the value is the very one row 0's hook legitimately wrote.
  • No recording view reaches a driver. The seal puts the RAW row back in input.data for every row, including one the declared-field door culled (that row ran no hook, so its record is legitimately empty).

⛔ Not a relaxation of #5503 — the negative controls

The accept set for callers does not move. A caller-seeded record number that no hook assigned is still stripped, still warns with the same text, and still reports through both observability seams. Asserted through the documented surfaces rather than by toEqual on the result:

  • the warning line still contains Field 'code' on 'prov_ticket', runtime-owned, COMMITTED WITHOUT IT and hook-written keys are not caller-supplied;
  • onFieldsDropped still fires with reason: 'readonly' naming code;
  • strictReadonlyWrites still REFUSES — asserted as an envelope (nameReadonlyFieldRejectedError, codeERR_READONLY_FIELD_REJECTED, fields containing code, operationinsert) plus zero rows reaching the driver, never as a bare toThrow(). ⚠️status is deliberately not asserted: this class carries code and name only, the HTTP mapping living at the protocol layer, so pinning a status here would pin a member the class does not have.

isSystem and preserveAudit are untouched. A hook that REPLACES the payload object leaves no attributable record and falls back to the pre-existing value test, which over-strips — pinned, because reading a replacement's keys as hook-owned would launder a caller's forgery, and keeping the old bug is the only safe direction.


What is WITHHELD: seam 1 — isCallerSuppliedValue, behind the two readonlyWhen strips

Implemented, measured, then reverted out of this PR. The card's own fork clause fired.

Threading the sealed record into isCallerSuppliedValue turns the existing #9107 pin LOCK 3b red. Measured, exact:

FAIL engine-readonly-when-derived-writes.test.ts
> LOCK 3b — a hook that writes the caller value BACK is the caller value, and goes
AssertionError: expected '1999-01-01' to be null

The hook in that pin is ctx.input.data.closed_note = ctx.input.data.closed_note — a self-assignment that computes nothing. Under Object.is it "has written nothing". Under provenance it is a set on the recorded object, so the key becomes hook-owned and the caller's forged value survives a TRUE readonlyWhen predicate on a record.status == 'closed' state lock. The pin's own prose says why it exists: "Pinned because 'a hook touched this key' is exactly the weaker rule that WOULD open a laundering path."

That is the fork clause verbatim — a case where seam 1 under provenance lets a caller value survive a TRUE predicate — so it goes to the decision inbox and is not resolved here, in either direction. Nothing about the readonlyWhen seam's behaviour changes in this PR, and no existing pin is relaxed.

The tension is real and not a bug in either half: the mechanism the card mandates records that an assignment ran and is deliberately blind to the value, which makes "a hook deliberately wrote the value the caller also sent" (must be KEPT, per the inherited ruling) and "a hook echoed the caller's value onto a locked column" (must be STRIPPED, per LOCK 3b) mechanically indistinguishable. A "value differs from what was there" filter does not separate them either — it would re-break #14088's own measured completed_at: null row, where the hook's assignment is value-identical to what already stands on the key.

⚠️ The same shape is measured and pinned on the seam that ships, so the asymmetry is visible rather than discovered later: a lone self-assigning beforeInsert hook does leave the caller's seeded record number on the key. It ships here because stripRuntimeOwnedFields guards a runtime-owned COLUMN (#5503) — the same class of protection #14088 already moved to provenance on the update side, and the class whose hook exemption runtimeOwnedStripWarning promises authors in prose — not a STATE lock whose whole purpose is that no caller write survives a TRUE predicate. INSERT is exempt from readonlyWhen entirely, so no lock of that class exists on this path to open. A ruling that self-assignment must not count would move that pin and #14088's seam together, deliberately.

Seam 3 from the card (arming the recorder before the middleware chain) is out of scope by triage and was not implemented; no middleware stamp needing it was measured.


Verification

All readings on 25619ebb7 unless stated. Every exit code captured after a redirect, never through a pipe; each verdict quoted from the gate's own line.

Tests.pnpm --filter @objectstack/objectql testTest Files 258 passed (258) · Tests 4470 passed (4470). pnpm --filter @objectstack/objectql typecheck → exit 0, check:test-typecheck: OK (44 files / 242 errors / 69 pinned signatures held — the ledger did not move).

⚠️ The new pin file is IN the typecheck program, not merely un-flagged: tsc --noEmit -p tsconfig.test.json --listFiles names it (1 hit, not 0).

New pin filepackages/objectql/src/engine-hook-provenance-sibling-seams.test.ts — 13 cases, all green, alongside every neighbouring pin (engine-readonly-strip-caller-values, engine-readonly-when-derived-writes, engine-insert-runtime-owned-strip, hook-write-provenance, validation/rule-validator, validation/skip-provenance): 7 files / 268 tests green together.

Ablation, direction predicted before the run and matched: reverting the hookWrittenKeys?.has(name) branch in stripRuntimeOwnedFields turns 5 red / 8 green — every DEFECT and consequence case red, every negative control green. No rebuild leg applies: the pin imports ./engine.js, a relative specifier inside the same package's src/, so vitest resolves the subject from source and never from dist/ — the ablation going red with no build in between is itself the evidence.

The mutation was proven on disk before the run (exact occurrence counts 1 -> 0 deleted, 0 -> 1 injected, counted in Python because the anchor is multi-line and grep -c would count either half of it). ⚠️ Worth recording: the first attempt's marker matched 2 occurrences — the bare branch line is byte-identical to #14088's inside stripReadonlyFields two functions up — and the on-disk guard refused the run rather than let a reading contaminated by ablating #14088's seam through. The anchor was narrowed to the preceding comment. Restore was proven by observed state, not exit code: git hash-object equal to the HEAD blob, git diff HEAD empty, git status --porcelain empty, with the restore path pinned to an absolute repo root inside the trap.

Lint. The whole population, not a narrowing: eslint . --no-inline-config --format json (the exact rule set pnpm lint runs) → 5702 files, 0 errors, 0 warnings, exit 0.

Gates. Re-derived after the last commit with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands (no path list — the script takes its own change set), then run as a union on this HEAD: 60 green, 0 red, 4 NOT MEASURED. The four are exit 3 prerequisite refusals, recorded in the gates' own words and never as passes:

  • check-test-completeness — needs a saved turbo run test log this gate cannot produce; CI tees one.
  • check-half-states — needs repo-scoped GitHub reads this container's egress refuses.
  • check:dual-build-cjs-loadsPREREQUISITE NOT MET, reads built output for 56 unbuilt packages.
  • check:type-check-debtPREREQUISITE NOT MET, --re-measure needs 41 dependency closures built.

check:skill-examples initially refused for the same reason (packages/client-react/dist unbuilt, and its own text says a verdict then would be a FALSE GREEN); the closure was built and it now reports 263 prose examples type-check across 3 surfaces, exit 0.

Gate scaffolding, regenerated and never hand-edited.check-system-context-census went red on pure line rot — the insert-path arm/seal shifts every later engine.ts line — and was repaired with node scripts/check-system-context-census.mjs --fix (15 anchors rewritten in content/docs/permissions/system-context.mdx; the gate then reports OK — 109 elevation read sites ... all anchored). check:objectql-double-limit flagged the new pin's driver double for ignoring the caller's bound; the double now applies limit after the filter, by presence, and the gate is green. check:engine-double-contract was green throughout and needed no ledger row.

⚠️ One measurement trap worth recording, because it read exactly like a regression: immediately after merging origin/main, engine-filter-array-lowering.test.ts failed 6 cases. It was stale dist/, not code — the merge brought packages/spec changes my worktree's build predated. Clean origin/main in a comparison worktree ran the same file 60/60 green, and rebuilding this worktree's dependency closure made the failure vanish. Nothing was "fixed"; the instrument was.

Scope

Changeset: @objectstack/objectqlpatch. packages/objectql-internal throughout — none of the four functions is re-exported from index.ts or core.ts, and every mention outside rule-validator.ts / engine.ts is prose in a comment (packages/spec, packages/metadata-protocol, three packages/lint files, one showcase object) or a test in this package. The new option is optional and every positional signature is unchanged, so no existing call site moves.

🤖 Generated with Claude Code

https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68


Generated by Claude Code

… runtime-owned strips
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…te provenance
Withholds the readonlyWhen sibling seam: threading the record into
isCallerSuppliedValue was measured to let a caller value survive a TRUE
readonlyWhen predicate (#9107 pin LOCK 3b), which #14259's fork clause
routes to the decision inbox rather than to a unilateral choice.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…le; re-anchor the system-context census
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql, touching 2 documentable anchor(s).

18 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aaf.

1 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 15 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aafpackageMentionDocs.

Which tree this was computed on

This run read content/docs from afdbce2baed0487c867d978b8956d531e297b5cf — the merge of head 25619ebb76b68f80ef5489dddb101255bdc1d802 into base 18d816a504a5795a13c609304eab5825cf923aaf, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin afdbce2baed0487c867d978b8956d531e297b5cf && git checkout afdbce2baed0487c867d978b8956d531e297b5cf
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 18d816a504a5795a13c609304eab5825cf923aaf 25619ebb76b68f80ef5489dddb101255bdc1d802 && git checkout -B drift-repro 18d816a504a5795a13c609304eab5825cf923aaf && git merge --no-ff 25619ebb76b68f80ef5489dddb101255bdc1d802
node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aaf

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 18d816a504a5795a13c609304eab5825cf923aaf → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-muskClaude

Copy link
Copy Markdown
CollaboratorAuthor

Landing provenance — engine execution seat (session session_0112hMx9hjJ9BgB28X97DS68).

ACCEPT on the card: comment 5506811038 (#14259 — seam 2 ships here as Part of; seam 1 is in the decision box, four-facet block 5506819337). Flip pre-checks on head 25619ebb7: every one of the 34 check runs completed with conclusion success or skipped (Lint & Repo Gates completed 08:50:47Z; Part-of PR must not also close its card green); governed-surface test on the final 5-path file list: 0 of 5 path(s) hit the register ⇒ ordinary queue landing; closing-keyword two-read done at ACCEPT (Part of #14259, no closing keyword — correct, the card stays open as the seam-1 carrier). Clause-② self-reading no (package-internal functions, optional parameter, no public-entry export), so no contract-review carrier applied.

Action: draft: false then auto-merge (squash) — the merge queue takes it from here. At MERGED the card flips pm:dispatchedneeds-user-decision in one stroke.


Generated by Claude Code

@os-musk
os-musk enabled auto-merge September 2, 2026 08:53
@os-musk
os-musk added this pull request to the merge queueSep 2, 2026
Merged via the queue into main with commit 00ff228Sep 2, 2026
36 checks passed
@os-musk
os-musk deleted the claude/issue-14259-hook-provenance-sibling-seams branch September 2, 2026 09:18
os-musk pushed a commit that referenced this pull request Sep 2, 2026
…th faces
`isCallerSuppliedValue`'s docblock claimed the readonlyWhen seam runs "the
identical two-part test `stripReadonlyFields` applies" and is "textually
parallel" with it. Neither has held since #14088 moved the static face onto a
hook-write record and #14472 did the same to the insert-side twin. The
divergence is deliberate and lock-motivated, so it is now stated where the test
lives, with the reason each face guards a different thing.
Adds the static face's missing measurement pin (a lone self-assigning hook
leaves the CALLER value on the key) beside #14088's own suite, and cross-links
it with `LOCK 3b`, which pins the opposite verdict for the identical hook
spelling. Comment- and test-only; no behaviour moves.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@os-musk@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(objectql): decide the insert-side runtime-owned strip by hook-write provenance - #14472

Merged
os-musk merged 7 commits into
mainfrom
claude/issue-14259-hook-provenance-sibling-seams
Sep 2, 2026
Merged

fix(objectql): decide the insert-side runtime-owned strip by hook-write provenance#14472
os-musk merged 7 commits into
mainfrom
claude/issue-14259-hook-provenance-sibling-seams

Conversation

@os-musk

Copy link
Copy Markdown
Collaborator

Part of #14259

⚠️Part of, not a closing keyword, and that is the headline.#14259 names TWO sibling seams. One is here and finished; the other is WITHHELD pending a maintainer ruling, because implementing it was measured to violate the card's own binding constraint. Merging this must not close the card.

Verified on git rev-parse --short HEAD = 25619ebb7 (origin/main merged immediately before opening).


What ships: seam 2 — stripRuntimeOwnedFields, the INSERT-side strip

#14088 replaced Object.is(payload[k], supplied[k]) inside stripReadonlyFields with a recording of the keys the before-phase hook chain actually assigned (recordHookPayloadWrites). Its argument was never about null: value equality cannot separate

  • the hook deliberately wrote the value the caller also sent, from
  • the hook never touched the key at all,

and the two demand opposite verdicts. stripRuntimeOwnedFields was left on the comparison that argument retired, and #6339's own prose is the finding: it argued a key SET made the contract true "only BY ACCIDENT" and moved to VALUES, which is accidental in the identical way. So #6339's own measured hook — a beforeInsert that re-issues or normalises a record number — still loses its write to the one caller that submitted the same value.

The change.engine.insert arms one recordHookPayloadWritesper row at hook-context construction and seals each immediately after that row's beforeInsert chain; stripRuntimeOwnedFields gains the same optional hookWrittenKeys#14088 gave stripReadonlyFields, consulted before the value test, with the same no-record fallback.

engine.ts regions touched — declared, and nothing else

packages/objectql/src/engine.ts is a hot file, so the touch is region-bounded and the regions are named here as they were in the dev claim:

regionwhat
the insert path's rowHookContexts constructionarm one recording per row over defaultedData
immediately after the beforeInsert dispatch loopseal each row, restore the raw payload into input.data
the stripRuntimeOwnedFields call sitepass that row's hookWrittenKeys
import type beside the existing recordHookPayloadWrites importone line

No other engine.ts hunk. The two update-path readonlyWhen call sites named in the dev claim are not touched (see the withheld seam below). At the time of opening, no other open PR touches packages/objectql/src/validation/rule-validator.ts or these regions.

The forgery boundary the NEW recording owes

The card carries it verbatim — "The record is safe only because it is armed after the caller's payload has arrived and sealed before any engine-owned pass touches it, and because it records that an assignment ran rather than anything about the payload's contents. Any producer of a hook-written key set owes the same proof." The insert-side recording is new, so here is that proof, each property measured rather than asserted:

  1. Armed after the caller's payload has arrived. The arm sits on defaultedData — after suppliedPerRow has taken its explicit shallow copy of the caller's rows, and after the engine's own applyFieldDefaults / initializeSummaryFields. So neither a caller key nor an engine default can enter the record. Pinned: FORGERY BOUNDARY: the recording is armed AFTER the caller payload arrives.
  2. Sealed before any engine-owned pass touches it. Measured between the beforeInsert dispatch and the strip: the post-hook declared-field door (undeclaredWriteFieldErrors, reads only), resolveSystemInsertOrganization (measured read-only over rows — it consults row[tenantField] and writes nothing), buildDriverOptions (writes input.options, never input.data), then refuseEmptyPasswordFields and encryptSecretFields — the last of which does write to the row and is exactly what the recorder's own header names as the escalation risk. The seal is placed ahead of all of them, immediately after the dispatch loop, so the only code inside the window is before-phase hook code. Pinned: FORGERY BOUNDARY: the recording is SEALED before any engine-owned pass.
  3. Records that an assignment ran, not the contents. Pinned in both directions: a hook assigning the caller's own value and a hook assigning a different one reach the same verdict.

Two further properties the update path's single recording does not have to carry:

  • Per row, never per call. A batch arms one recorder per row, so a hook stamping row 0 confers nothing on row 1 — pinned, including that row 1's caller seed is still stripped even when the value is the very one row 0's hook legitimately wrote.
  • No recording view reaches a driver. The seal puts the RAW row back in input.data for every row, including one the declared-field door culled (that row ran no hook, so its record is legitimately empty).

⛔ Not a relaxation of #5503 — the negative controls

The accept set for callers does not move. A caller-seeded record number that no hook assigned is still stripped, still warns with the same text, and still reports through both observability seams. Asserted through the documented surfaces rather than by toEqual on the result:

  • the warning line still contains Field 'code' on 'prov_ticket', runtime-owned, COMMITTED WITHOUT IT and hook-written keys are not caller-supplied;
  • onFieldsDropped still fires with reason: 'readonly' naming code;
  • strictReadonlyWrites still REFUSES — asserted as an envelope (nameReadonlyFieldRejectedError, codeERR_READONLY_FIELD_REJECTED, fields containing code, operationinsert) plus zero rows reaching the driver, never as a bare toThrow(). ⚠️status is deliberately not asserted: this class carries code and name only, the HTTP mapping living at the protocol layer, so pinning a status here would pin a member the class does not have.

isSystem and preserveAudit are untouched. A hook that REPLACES the payload object leaves no attributable record and falls back to the pre-existing value test, which over-strips — pinned, because reading a replacement's keys as hook-owned would launder a caller's forgery, and keeping the old bug is the only safe direction.


What is WITHHELD: seam 1 — isCallerSuppliedValue, behind the two readonlyWhen strips

Implemented, measured, then reverted out of this PR. The card's own fork clause fired.

Threading the sealed record into isCallerSuppliedValue turns the existing #9107 pin LOCK 3b red. Measured, exact:

FAIL engine-readonly-when-derived-writes.test.ts
> LOCK 3b — a hook that writes the caller value BACK is the caller value, and goes
AssertionError: expected '1999-01-01' to be null

The hook in that pin is ctx.input.data.closed_note = ctx.input.data.closed_note — a self-assignment that computes nothing. Under Object.is it "has written nothing". Under provenance it is a set on the recorded object, so the key becomes hook-owned and the caller's forged value survives a TRUE readonlyWhen predicate on a record.status == 'closed' state lock. The pin's own prose says why it exists: "Pinned because 'a hook touched this key' is exactly the weaker rule that WOULD open a laundering path."

That is the fork clause verbatim — a case where seam 1 under provenance lets a caller value survive a TRUE predicate — so it goes to the decision inbox and is not resolved here, in either direction. Nothing about the readonlyWhen seam's behaviour changes in this PR, and no existing pin is relaxed.

The tension is real and not a bug in either half: the mechanism the card mandates records that an assignment ran and is deliberately blind to the value, which makes "a hook deliberately wrote the value the caller also sent" (must be KEPT, per the inherited ruling) and "a hook echoed the caller's value onto a locked column" (must be STRIPPED, per LOCK 3b) mechanically indistinguishable. A "value differs from what was there" filter does not separate them either — it would re-break #14088's own measured completed_at: null row, where the hook's assignment is value-identical to what already stands on the key.

⚠️ The same shape is measured and pinned on the seam that ships, so the asymmetry is visible rather than discovered later: a lone self-assigning beforeInsert hook does leave the caller's seeded record number on the key. It ships here because stripRuntimeOwnedFields guards a runtime-owned COLUMN (#5503) — the same class of protection #14088 already moved to provenance on the update side, and the class whose hook exemption runtimeOwnedStripWarning promises authors in prose — not a STATE lock whose whole purpose is that no caller write survives a TRUE predicate. INSERT is exempt from readonlyWhen entirely, so no lock of that class exists on this path to open. A ruling that self-assignment must not count would move that pin and #14088's seam together, deliberately.

Seam 3 from the card (arming the recorder before the middleware chain) is out of scope by triage and was not implemented; no middleware stamp needing it was measured.


Verification

All readings on 25619ebb7 unless stated. Every exit code captured after a redirect, never through a pipe; each verdict quoted from the gate's own line.

Tests.pnpm --filter @objectstack/objectql testTest Files 258 passed (258) · Tests 4470 passed (4470). pnpm --filter @objectstack/objectql typecheck → exit 0, check:test-typecheck: OK (44 files / 242 errors / 69 pinned signatures held — the ledger did not move).

⚠️ The new pin file is IN the typecheck program, not merely un-flagged: tsc --noEmit -p tsconfig.test.json --listFiles names it (1 hit, not 0).

New pin filepackages/objectql/src/engine-hook-provenance-sibling-seams.test.ts — 13 cases, all green, alongside every neighbouring pin (engine-readonly-strip-caller-values, engine-readonly-when-derived-writes, engine-insert-runtime-owned-strip, hook-write-provenance, validation/rule-validator, validation/skip-provenance): 7 files / 268 tests green together.

Ablation, direction predicted before the run and matched: reverting the hookWrittenKeys?.has(name) branch in stripRuntimeOwnedFields turns 5 red / 8 green — every DEFECT and consequence case red, every negative control green. No rebuild leg applies: the pin imports ./engine.js, a relative specifier inside the same package's src/, so vitest resolves the subject from source and never from dist/ — the ablation going red with no build in between is itself the evidence.

The mutation was proven on disk before the run (exact occurrence counts 1 -> 0 deleted, 0 -> 1 injected, counted in Python because the anchor is multi-line and grep -c would count either half of it). ⚠️ Worth recording: the first attempt's marker matched 2 occurrences — the bare branch line is byte-identical to #14088's inside stripReadonlyFields two functions up — and the on-disk guard refused the run rather than let a reading contaminated by ablating #14088's seam through. The anchor was narrowed to the preceding comment. Restore was proven by observed state, not exit code: git hash-object equal to the HEAD blob, git diff HEAD empty, git status --porcelain empty, with the restore path pinned to an absolute repo root inside the trap.

Lint. The whole population, not a narrowing: eslint . --no-inline-config --format json (the exact rule set pnpm lint runs) → 5702 files, 0 errors, 0 warnings, exit 0.

Gates. Re-derived after the last commit with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands (no path list — the script takes its own change set), then run as a union on this HEAD: 60 green, 0 red, 4 NOT MEASURED. The four are exit 3 prerequisite refusals, recorded in the gates' own words and never as passes:

  • check-test-completeness — needs a saved turbo run test log this gate cannot produce; CI tees one.
  • check-half-states — needs repo-scoped GitHub reads this container's egress refuses.
  • check:dual-build-cjs-loadsPREREQUISITE NOT MET, reads built output for 56 unbuilt packages.
  • check:type-check-debtPREREQUISITE NOT MET, --re-measure needs 41 dependency closures built.

check:skill-examples initially refused for the same reason (packages/client-react/dist unbuilt, and its own text says a verdict then would be a FALSE GREEN); the closure was built and it now reports 263 prose examples type-check across 3 surfaces, exit 0.

Gate scaffolding, regenerated and never hand-edited.check-system-context-census went red on pure line rot — the insert-path arm/seal shifts every later engine.ts line — and was repaired with node scripts/check-system-context-census.mjs --fix (15 anchors rewritten in content/docs/permissions/system-context.mdx; the gate then reports OK — 109 elevation read sites ... all anchored). check:objectql-double-limit flagged the new pin's driver double for ignoring the caller's bound; the double now applies limit after the filter, by presence, and the gate is green. check:engine-double-contract was green throughout and needed no ledger row.

⚠️ One measurement trap worth recording, because it read exactly like a regression: immediately after merging origin/main, engine-filter-array-lowering.test.ts failed 6 cases. It was stale dist/, not code — the merge brought packages/spec changes my worktree's build predated. Clean origin/main in a comparison worktree ran the same file 60/60 green, and rebuilding this worktree's dependency closure made the failure vanish. Nothing was "fixed"; the instrument was.

Scope

Changeset: @objectstack/objectqlpatch. packages/objectql-internal throughout — none of the four functions is re-exported from index.ts or core.ts, and every mention outside rule-validator.ts / engine.ts is prose in a comment (packages/spec, packages/metadata-protocol, three packages/lint files, one showcase object) or a test in this package. The new option is optional and every positional signature is unchanged, so no existing call site moves.

🤖 Generated with Claude Code

https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68


Generated by Claude Code

… runtime-owned strips
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…te provenance
Withholds the readonlyWhen sibling seam: threading the record into
isCallerSuppliedValue was measured to let a caller value survive a TRUE
readonlyWhen predicate (#9107 pin LOCK 3b), which #14259's fork clause
routes to the decision inbox rather than to a unilateral choice.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…le; re-anchor the system-context census
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql, touching 2 documentable anchor(s).

18 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aaf.

1 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 15 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aafpackageMentionDocs.

Which tree this was computed on

This run read content/docs from afdbce2baed0487c867d978b8956d531e297b5cf — the merge of head 25619ebb76b68f80ef5489dddb101255bdc1d802 into base 18d816a504a5795a13c609304eab5825cf923aaf, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin afdbce2baed0487c867d978b8956d531e297b5cf && git checkout afdbce2baed0487c867d978b8956d531e297b5cf
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 18d816a504a5795a13c609304eab5825cf923aaf 25619ebb76b68f80ef5489dddb101255bdc1d802 && git checkout -B drift-repro 18d816a504a5795a13c609304eab5825cf923aaf && git merge --no-ff 25619ebb76b68f80ef5489dddb101255bdc1d802
node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aaf

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 18d816a504a5795a13c609304eab5825cf923aaf → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-muskClaude

Copy link
Copy Markdown
CollaboratorAuthor

Landing provenance — engine execution seat (session session_0112hMx9hjJ9BgB28X97DS68).

ACCEPT on the card: comment 5506811038 (#14259 — seam 2 ships here as Part of; seam 1 is in the decision box, four-facet block 5506819337). Flip pre-checks on head 25619ebb7: every one of the 34 check runs completed with conclusion success or skipped (Lint & Repo Gates completed 08:50:47Z; Part-of PR must not also close its card green); governed-surface test on the final 5-path file list: 0 of 5 path(s) hit the register ⇒ ordinary queue landing; closing-keyword two-read done at ACCEPT (Part of #14259, no closing keyword — correct, the card stays open as the seam-1 carrier). Clause-② self-reading no (package-internal functions, optional parameter, no public-entry export), so no contract-review carrier applied.

Action: draft: false then auto-merge (squash) — the merge queue takes it from here. At MERGED the card flips pm:dispatchedneeds-user-decision in one stroke.


Generated by Claude Code

@os-musk
os-musk enabled auto-merge September 2, 2026 08:53
@os-musk
os-musk added this pull request to the merge queueSep 2, 2026
Merged via the queue into main with commit 00ff228Sep 2, 2026
36 checks passed
@os-musk
os-musk deleted the claude/issue-14259-hook-provenance-sibling-seams branch September 2, 2026 09:18
os-musk pushed a commit that referenced this pull request Sep 2, 2026
…th faces
`isCallerSuppliedValue`'s docblock claimed the readonlyWhen seam runs "the
identical two-part test `stripReadonlyFields` applies" and is "textually
parallel" with it. Neither has held since #14088 moved the static face onto a
hook-write record and #14472 did the same to the insert-side twin. The
divergence is deliberate and lock-motivated, so it is now stated where the test
lives, with the reason each face guards a different thing.
Adds the static face's missing measurement pin (a lone self-assigning hook
leaves the CALLER value on the key) beside #14088's own suite, and cross-links
it with `LOCK 3b`, which pins the opposite verdict for the identical hook
spelling. Comment- and test-only; no behaviour moves.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@os-musk@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(objectql): decide the insert-side runtime-owned strip by hook-write provenance - #14472

Merged
os-musk merged 7 commits into
mainfrom
claude/issue-14259-hook-provenance-sibling-seams
Sep 2, 2026
Merged

fix(objectql): decide the insert-side runtime-owned strip by hook-write provenance#14472
os-musk merged 7 commits into
mainfrom
claude/issue-14259-hook-provenance-sibling-seams

Conversation

@os-musk

Copy link
Copy Markdown
Collaborator

Part of #14259

⚠️Part of, not a closing keyword, and that is the headline.#14259 names TWO sibling seams. One is here and finished; the other is WITHHELD pending a maintainer ruling, because implementing it was measured to violate the card's own binding constraint. Merging this must not close the card.

Verified on git rev-parse --short HEAD = 25619ebb7 (origin/main merged immediately before opening).


What ships: seam 2 — stripRuntimeOwnedFields, the INSERT-side strip

#14088 replaced Object.is(payload[k], supplied[k]) inside stripReadonlyFields with a recording of the keys the before-phase hook chain actually assigned (recordHookPayloadWrites). Its argument was never about null: value equality cannot separate

  • the hook deliberately wrote the value the caller also sent, from
  • the hook never touched the key at all,

and the two demand opposite verdicts. stripRuntimeOwnedFields was left on the comparison that argument retired, and #6339's own prose is the finding: it argued a key SET made the contract true "only BY ACCIDENT" and moved to VALUES, which is accidental in the identical way. So #6339's own measured hook — a beforeInsert that re-issues or normalises a record number — still loses its write to the one caller that submitted the same value.

The change.engine.insert arms one recordHookPayloadWritesper row at hook-context construction and seals each immediately after that row's beforeInsert chain; stripRuntimeOwnedFields gains the same optional hookWrittenKeys#14088 gave stripReadonlyFields, consulted before the value test, with the same no-record fallback.

engine.ts regions touched — declared, and nothing else

packages/objectql/src/engine.ts is a hot file, so the touch is region-bounded and the regions are named here as they were in the dev claim:

regionwhat
the insert path's rowHookContexts constructionarm one recording per row over defaultedData
immediately after the beforeInsert dispatch loopseal each row, restore the raw payload into input.data
the stripRuntimeOwnedFields call sitepass that row's hookWrittenKeys
import type beside the existing recordHookPayloadWrites importone line

No other engine.ts hunk. The two update-path readonlyWhen call sites named in the dev claim are not touched (see the withheld seam below). At the time of opening, no other open PR touches packages/objectql/src/validation/rule-validator.ts or these regions.

The forgery boundary the NEW recording owes

The card carries it verbatim — "The record is safe only because it is armed after the caller's payload has arrived and sealed before any engine-owned pass touches it, and because it records that an assignment ran rather than anything about the payload's contents. Any producer of a hook-written key set owes the same proof." The insert-side recording is new, so here is that proof, each property measured rather than asserted:

  1. Armed after the caller's payload has arrived. The arm sits on defaultedData — after suppliedPerRow has taken its explicit shallow copy of the caller's rows, and after the engine's own applyFieldDefaults / initializeSummaryFields. So neither a caller key nor an engine default can enter the record. Pinned: FORGERY BOUNDARY: the recording is armed AFTER the caller payload arrives.
  2. Sealed before any engine-owned pass touches it. Measured between the beforeInsert dispatch and the strip: the post-hook declared-field door (undeclaredWriteFieldErrors, reads only), resolveSystemInsertOrganization (measured read-only over rows — it consults row[tenantField] and writes nothing), buildDriverOptions (writes input.options, never input.data), then refuseEmptyPasswordFields and encryptSecretFields — the last of which does write to the row and is exactly what the recorder's own header names as the escalation risk. The seal is placed ahead of all of them, immediately after the dispatch loop, so the only code inside the window is before-phase hook code. Pinned: FORGERY BOUNDARY: the recording is SEALED before any engine-owned pass.
  3. Records that an assignment ran, not the contents. Pinned in both directions: a hook assigning the caller's own value and a hook assigning a different one reach the same verdict.

Two further properties the update path's single recording does not have to carry:

  • Per row, never per call. A batch arms one recorder per row, so a hook stamping row 0 confers nothing on row 1 — pinned, including that row 1's caller seed is still stripped even when the value is the very one row 0's hook legitimately wrote.
  • No recording view reaches a driver. The seal puts the RAW row back in input.data for every row, including one the declared-field door culled (that row ran no hook, so its record is legitimately empty).

⛔ Not a relaxation of #5503 — the negative controls

The accept set for callers does not move. A caller-seeded record number that no hook assigned is still stripped, still warns with the same text, and still reports through both observability seams. Asserted through the documented surfaces rather than by toEqual on the result:

  • the warning line still contains Field 'code' on 'prov_ticket', runtime-owned, COMMITTED WITHOUT IT and hook-written keys are not caller-supplied;
  • onFieldsDropped still fires with reason: 'readonly' naming code;
  • strictReadonlyWrites still REFUSES — asserted as an envelope (nameReadonlyFieldRejectedError, codeERR_READONLY_FIELD_REJECTED, fields containing code, operationinsert) plus zero rows reaching the driver, never as a bare toThrow(). ⚠️status is deliberately not asserted: this class carries code and name only, the HTTP mapping living at the protocol layer, so pinning a status here would pin a member the class does not have.

isSystem and preserveAudit are untouched. A hook that REPLACES the payload object leaves no attributable record and falls back to the pre-existing value test, which over-strips — pinned, because reading a replacement's keys as hook-owned would launder a caller's forgery, and keeping the old bug is the only safe direction.


What is WITHHELD: seam 1 — isCallerSuppliedValue, behind the two readonlyWhen strips

Implemented, measured, then reverted out of this PR. The card's own fork clause fired.

Threading the sealed record into isCallerSuppliedValue turns the existing #9107 pin LOCK 3b red. Measured, exact:

FAIL engine-readonly-when-derived-writes.test.ts
> LOCK 3b — a hook that writes the caller value BACK is the caller value, and goes
AssertionError: expected '1999-01-01' to be null

The hook in that pin is ctx.input.data.closed_note = ctx.input.data.closed_note — a self-assignment that computes nothing. Under Object.is it "has written nothing". Under provenance it is a set on the recorded object, so the key becomes hook-owned and the caller's forged value survives a TRUE readonlyWhen predicate on a record.status == 'closed' state lock. The pin's own prose says why it exists: "Pinned because 'a hook touched this key' is exactly the weaker rule that WOULD open a laundering path."

That is the fork clause verbatim — a case where seam 1 under provenance lets a caller value survive a TRUE predicate — so it goes to the decision inbox and is not resolved here, in either direction. Nothing about the readonlyWhen seam's behaviour changes in this PR, and no existing pin is relaxed.

The tension is real and not a bug in either half: the mechanism the card mandates records that an assignment ran and is deliberately blind to the value, which makes "a hook deliberately wrote the value the caller also sent" (must be KEPT, per the inherited ruling) and "a hook echoed the caller's value onto a locked column" (must be STRIPPED, per LOCK 3b) mechanically indistinguishable. A "value differs from what was there" filter does not separate them either — it would re-break #14088's own measured completed_at: null row, where the hook's assignment is value-identical to what already stands on the key.

⚠️ The same shape is measured and pinned on the seam that ships, so the asymmetry is visible rather than discovered later: a lone self-assigning beforeInsert hook does leave the caller's seeded record number on the key. It ships here because stripRuntimeOwnedFields guards a runtime-owned COLUMN (#5503) — the same class of protection #14088 already moved to provenance on the update side, and the class whose hook exemption runtimeOwnedStripWarning promises authors in prose — not a STATE lock whose whole purpose is that no caller write survives a TRUE predicate. INSERT is exempt from readonlyWhen entirely, so no lock of that class exists on this path to open. A ruling that self-assignment must not count would move that pin and #14088's seam together, deliberately.

Seam 3 from the card (arming the recorder before the middleware chain) is out of scope by triage and was not implemented; no middleware stamp needing it was measured.


Verification

All readings on 25619ebb7 unless stated. Every exit code captured after a redirect, never through a pipe; each verdict quoted from the gate's own line.

Tests.pnpm --filter @objectstack/objectql testTest Files 258 passed (258) · Tests 4470 passed (4470). pnpm --filter @objectstack/objectql typecheck → exit 0, check:test-typecheck: OK (44 files / 242 errors / 69 pinned signatures held — the ledger did not move).

⚠️ The new pin file is IN the typecheck program, not merely un-flagged: tsc --noEmit -p tsconfig.test.json --listFiles names it (1 hit, not 0).

New pin filepackages/objectql/src/engine-hook-provenance-sibling-seams.test.ts — 13 cases, all green, alongside every neighbouring pin (engine-readonly-strip-caller-values, engine-readonly-when-derived-writes, engine-insert-runtime-owned-strip, hook-write-provenance, validation/rule-validator, validation/skip-provenance): 7 files / 268 tests green together.

Ablation, direction predicted before the run and matched: reverting the hookWrittenKeys?.has(name) branch in stripRuntimeOwnedFields turns 5 red / 8 green — every DEFECT and consequence case red, every negative control green. No rebuild leg applies: the pin imports ./engine.js, a relative specifier inside the same package's src/, so vitest resolves the subject from source and never from dist/ — the ablation going red with no build in between is itself the evidence.

The mutation was proven on disk before the run (exact occurrence counts 1 -> 0 deleted, 0 -> 1 injected, counted in Python because the anchor is multi-line and grep -c would count either half of it). ⚠️ Worth recording: the first attempt's marker matched 2 occurrences — the bare branch line is byte-identical to #14088's inside stripReadonlyFields two functions up — and the on-disk guard refused the run rather than let a reading contaminated by ablating #14088's seam through. The anchor was narrowed to the preceding comment. Restore was proven by observed state, not exit code: git hash-object equal to the HEAD blob, git diff HEAD empty, git status --porcelain empty, with the restore path pinned to an absolute repo root inside the trap.

Lint. The whole population, not a narrowing: eslint . --no-inline-config --format json (the exact rule set pnpm lint runs) → 5702 files, 0 errors, 0 warnings, exit 0.

Gates. Re-derived after the last commit with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands (no path list — the script takes its own change set), then run as a union on this HEAD: 60 green, 0 red, 4 NOT MEASURED. The four are exit 3 prerequisite refusals, recorded in the gates' own words and never as passes:

  • check-test-completeness — needs a saved turbo run test log this gate cannot produce; CI tees one.
  • check-half-states — needs repo-scoped GitHub reads this container's egress refuses.
  • check:dual-build-cjs-loadsPREREQUISITE NOT MET, reads built output for 56 unbuilt packages.
  • check:type-check-debtPREREQUISITE NOT MET, --re-measure needs 41 dependency closures built.

check:skill-examples initially refused for the same reason (packages/client-react/dist unbuilt, and its own text says a verdict then would be a FALSE GREEN); the closure was built and it now reports 263 prose examples type-check across 3 surfaces, exit 0.

Gate scaffolding, regenerated and never hand-edited.check-system-context-census went red on pure line rot — the insert-path arm/seal shifts every later engine.ts line — and was repaired with node scripts/check-system-context-census.mjs --fix (15 anchors rewritten in content/docs/permissions/system-context.mdx; the gate then reports OK — 109 elevation read sites ... all anchored). check:objectql-double-limit flagged the new pin's driver double for ignoring the caller's bound; the double now applies limit after the filter, by presence, and the gate is green. check:engine-double-contract was green throughout and needed no ledger row.

⚠️ One measurement trap worth recording, because it read exactly like a regression: immediately after merging origin/main, engine-filter-array-lowering.test.ts failed 6 cases. It was stale dist/, not code — the merge brought packages/spec changes my worktree's build predated. Clean origin/main in a comparison worktree ran the same file 60/60 green, and rebuilding this worktree's dependency closure made the failure vanish. Nothing was "fixed"; the instrument was.

Scope

Changeset: @objectstack/objectqlpatch. packages/objectql-internal throughout — none of the four functions is re-exported from index.ts or core.ts, and every mention outside rule-validator.ts / engine.ts is prose in a comment (packages/spec, packages/metadata-protocol, three packages/lint files, one showcase object) or a test in this package. The new option is optional and every positional signature is unchanged, so no existing call site moves.

🤖 Generated with Claude Code

https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68


Generated by Claude Code

… runtime-owned strips
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…te provenance
Withholds the readonlyWhen sibling seam: threading the record into
isCallerSuppliedValue was measured to let a caller value survive a TRUE
readonlyWhen predicate (#9107 pin LOCK 3b), which #14259's fork clause
routes to the decision inbox rather than to a unilateral choice.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…le; re-anchor the system-context census
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql, touching 2 documentable anchor(s).

18 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aaf.

1 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 15 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aafpackageMentionDocs.

Which tree this was computed on

This run read content/docs from afdbce2baed0487c867d978b8956d531e297b5cf — the merge of head 25619ebb76b68f80ef5489dddb101255bdc1d802 into base 18d816a504a5795a13c609304eab5825cf923aaf, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin afdbce2baed0487c867d978b8956d531e297b5cf && git checkout afdbce2baed0487c867d978b8956d531e297b5cf
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 18d816a504a5795a13c609304eab5825cf923aaf 25619ebb76b68f80ef5489dddb101255bdc1d802 && git checkout -B drift-repro 18d816a504a5795a13c609304eab5825cf923aaf && git merge --no-ff 25619ebb76b68f80ef5489dddb101255bdc1d802
node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aaf

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 18d816a504a5795a13c609304eab5825cf923aaf → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-muskClaude

Copy link
Copy Markdown
CollaboratorAuthor

Landing provenance — engine execution seat (session session_0112hMx9hjJ9BgB28X97DS68).

ACCEPT on the card: comment 5506811038 (#14259 — seam 2 ships here as Part of; seam 1 is in the decision box, four-facet block 5506819337). Flip pre-checks on head 25619ebb7: every one of the 34 check runs completed with conclusion success or skipped (Lint & Repo Gates completed 08:50:47Z; Part-of PR must not also close its card green); governed-surface test on the final 5-path file list: 0 of 5 path(s) hit the register ⇒ ordinary queue landing; closing-keyword two-read done at ACCEPT (Part of #14259, no closing keyword — correct, the card stays open as the seam-1 carrier). Clause-② self-reading no (package-internal functions, optional parameter, no public-entry export), so no contract-review carrier applied.

Action: draft: false then auto-merge (squash) — the merge queue takes it from here. At MERGED the card flips pm:dispatchedneeds-user-decision in one stroke.


Generated by Claude Code

@os-musk
os-musk enabled auto-merge September 2, 2026 08:53
@os-musk
os-musk added this pull request to the merge queueSep 2, 2026
Merged via the queue into main with commit 00ff228Sep 2, 2026
36 checks passed
@os-musk
os-musk deleted the claude/issue-14259-hook-provenance-sibling-seams branch September 2, 2026 09:18
os-musk pushed a commit that referenced this pull request Sep 2, 2026
…th faces
`isCallerSuppliedValue`'s docblock claimed the readonlyWhen seam runs "the
identical two-part test `stripReadonlyFields` applies" and is "textually
parallel" with it. Neither has held since #14088 moved the static face onto a
hook-write record and #14472 did the same to the insert-side twin. The
divergence is deliberate and lock-motivated, so it is now stated where the test
lives, with the reason each face guards a different thing.
Adds the static face's missing measurement pin (a lone self-assigning hook
leaves the CALLER value on the key) beside #14088's own suite, and cross-links
it with `LOCK 3b`, which pins the opposite verdict for the identical hook
spelling. Comment- and test-only; no behaviour moves.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@os-musk@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(objectql): decide the insert-side runtime-owned strip by hook-write provenance - #14472

Merged
os-musk merged 7 commits into
mainfrom
claude/issue-14259-hook-provenance-sibling-seams
Sep 2, 2026
Merged

fix(objectql): decide the insert-side runtime-owned strip by hook-write provenance#14472
os-musk merged 7 commits into
mainfrom
claude/issue-14259-hook-provenance-sibling-seams

Conversation

@os-musk

Copy link
Copy Markdown
Collaborator

Part of #14259

⚠️Part of, not a closing keyword, and that is the headline.#14259 names TWO sibling seams. One is here and finished; the other is WITHHELD pending a maintainer ruling, because implementing it was measured to violate the card's own binding constraint. Merging this must not close the card.

Verified on git rev-parse --short HEAD = 25619ebb7 (origin/main merged immediately before opening).


What ships: seam 2 — stripRuntimeOwnedFields, the INSERT-side strip

#14088 replaced Object.is(payload[k], supplied[k]) inside stripReadonlyFields with a recording of the keys the before-phase hook chain actually assigned (recordHookPayloadWrites). Its argument was never about null: value equality cannot separate

  • the hook deliberately wrote the value the caller also sent, from
  • the hook never touched the key at all,

and the two demand opposite verdicts. stripRuntimeOwnedFields was left on the comparison that argument retired, and #6339's own prose is the finding: it argued a key SET made the contract true "only BY ACCIDENT" and moved to VALUES, which is accidental in the identical way. So #6339's own measured hook — a beforeInsert that re-issues or normalises a record number — still loses its write to the one caller that submitted the same value.

The change.engine.insert arms one recordHookPayloadWritesper row at hook-context construction and seals each immediately after that row's beforeInsert chain; stripRuntimeOwnedFields gains the same optional hookWrittenKeys#14088 gave stripReadonlyFields, consulted before the value test, with the same no-record fallback.

engine.ts regions touched — declared, and nothing else

packages/objectql/src/engine.ts is a hot file, so the touch is region-bounded and the regions are named here as they were in the dev claim:

regionwhat
the insert path's rowHookContexts constructionarm one recording per row over defaultedData
immediately after the beforeInsert dispatch loopseal each row, restore the raw payload into input.data
the stripRuntimeOwnedFields call sitepass that row's hookWrittenKeys
import type beside the existing recordHookPayloadWrites importone line

No other engine.ts hunk. The two update-path readonlyWhen call sites named in the dev claim are not touched (see the withheld seam below). At the time of opening, no other open PR touches packages/objectql/src/validation/rule-validator.ts or these regions.

The forgery boundary the NEW recording owes

The card carries it verbatim — "The record is safe only because it is armed after the caller's payload has arrived and sealed before any engine-owned pass touches it, and because it records that an assignment ran rather than anything about the payload's contents. Any producer of a hook-written key set owes the same proof." The insert-side recording is new, so here is that proof, each property measured rather than asserted:

  1. Armed after the caller's payload has arrived. The arm sits on defaultedData — after suppliedPerRow has taken its explicit shallow copy of the caller's rows, and after the engine's own applyFieldDefaults / initializeSummaryFields. So neither a caller key nor an engine default can enter the record. Pinned: FORGERY BOUNDARY: the recording is armed AFTER the caller payload arrives.
  2. Sealed before any engine-owned pass touches it. Measured between the beforeInsert dispatch and the strip: the post-hook declared-field door (undeclaredWriteFieldErrors, reads only), resolveSystemInsertOrganization (measured read-only over rows — it consults row[tenantField] and writes nothing), buildDriverOptions (writes input.options, never input.data), then refuseEmptyPasswordFields and encryptSecretFields — the last of which does write to the row and is exactly what the recorder's own header names as the escalation risk. The seal is placed ahead of all of them, immediately after the dispatch loop, so the only code inside the window is before-phase hook code. Pinned: FORGERY BOUNDARY: the recording is SEALED before any engine-owned pass.
  3. Records that an assignment ran, not the contents. Pinned in both directions: a hook assigning the caller's own value and a hook assigning a different one reach the same verdict.

Two further properties the update path's single recording does not have to carry:

  • Per row, never per call. A batch arms one recorder per row, so a hook stamping row 0 confers nothing on row 1 — pinned, including that row 1's caller seed is still stripped even when the value is the very one row 0's hook legitimately wrote.
  • No recording view reaches a driver. The seal puts the RAW row back in input.data for every row, including one the declared-field door culled (that row ran no hook, so its record is legitimately empty).

⛔ Not a relaxation of #5503 — the negative controls

The accept set for callers does not move. A caller-seeded record number that no hook assigned is still stripped, still warns with the same text, and still reports through both observability seams. Asserted through the documented surfaces rather than by toEqual on the result:

  • the warning line still contains Field 'code' on 'prov_ticket', runtime-owned, COMMITTED WITHOUT IT and hook-written keys are not caller-supplied;
  • onFieldsDropped still fires with reason: 'readonly' naming code;
  • strictReadonlyWrites still REFUSES — asserted as an envelope (nameReadonlyFieldRejectedError, codeERR_READONLY_FIELD_REJECTED, fields containing code, operationinsert) plus zero rows reaching the driver, never as a bare toThrow(). ⚠️status is deliberately not asserted: this class carries code and name only, the HTTP mapping living at the protocol layer, so pinning a status here would pin a member the class does not have.

isSystem and preserveAudit are untouched. A hook that REPLACES the payload object leaves no attributable record and falls back to the pre-existing value test, which over-strips — pinned, because reading a replacement's keys as hook-owned would launder a caller's forgery, and keeping the old bug is the only safe direction.


What is WITHHELD: seam 1 — isCallerSuppliedValue, behind the two readonlyWhen strips

Implemented, measured, then reverted out of this PR. The card's own fork clause fired.

Threading the sealed record into isCallerSuppliedValue turns the existing #9107 pin LOCK 3b red. Measured, exact:

FAIL engine-readonly-when-derived-writes.test.ts
> LOCK 3b — a hook that writes the caller value BACK is the caller value, and goes
AssertionError: expected '1999-01-01' to be null

The hook in that pin is ctx.input.data.closed_note = ctx.input.data.closed_note — a self-assignment that computes nothing. Under Object.is it "has written nothing". Under provenance it is a set on the recorded object, so the key becomes hook-owned and the caller's forged value survives a TRUE readonlyWhen predicate on a record.status == 'closed' state lock. The pin's own prose says why it exists: "Pinned because 'a hook touched this key' is exactly the weaker rule that WOULD open a laundering path."

That is the fork clause verbatim — a case where seam 1 under provenance lets a caller value survive a TRUE predicate — so it goes to the decision inbox and is not resolved here, in either direction. Nothing about the readonlyWhen seam's behaviour changes in this PR, and no existing pin is relaxed.

The tension is real and not a bug in either half: the mechanism the card mandates records that an assignment ran and is deliberately blind to the value, which makes "a hook deliberately wrote the value the caller also sent" (must be KEPT, per the inherited ruling) and "a hook echoed the caller's value onto a locked column" (must be STRIPPED, per LOCK 3b) mechanically indistinguishable. A "value differs from what was there" filter does not separate them either — it would re-break #14088's own measured completed_at: null row, where the hook's assignment is value-identical to what already stands on the key.

⚠️ The same shape is measured and pinned on the seam that ships, so the asymmetry is visible rather than discovered later: a lone self-assigning beforeInsert hook does leave the caller's seeded record number on the key. It ships here because stripRuntimeOwnedFields guards a runtime-owned COLUMN (#5503) — the same class of protection #14088 already moved to provenance on the update side, and the class whose hook exemption runtimeOwnedStripWarning promises authors in prose — not a STATE lock whose whole purpose is that no caller write survives a TRUE predicate. INSERT is exempt from readonlyWhen entirely, so no lock of that class exists on this path to open. A ruling that self-assignment must not count would move that pin and #14088's seam together, deliberately.

Seam 3 from the card (arming the recorder before the middleware chain) is out of scope by triage and was not implemented; no middleware stamp needing it was measured.


Verification

All readings on 25619ebb7 unless stated. Every exit code captured after a redirect, never through a pipe; each verdict quoted from the gate's own line.

Tests.pnpm --filter @objectstack/objectql testTest Files 258 passed (258) · Tests 4470 passed (4470). pnpm --filter @objectstack/objectql typecheck → exit 0, check:test-typecheck: OK (44 files / 242 errors / 69 pinned signatures held — the ledger did not move).

⚠️ The new pin file is IN the typecheck program, not merely un-flagged: tsc --noEmit -p tsconfig.test.json --listFiles names it (1 hit, not 0).

New pin filepackages/objectql/src/engine-hook-provenance-sibling-seams.test.ts — 13 cases, all green, alongside every neighbouring pin (engine-readonly-strip-caller-values, engine-readonly-when-derived-writes, engine-insert-runtime-owned-strip, hook-write-provenance, validation/rule-validator, validation/skip-provenance): 7 files / 268 tests green together.

Ablation, direction predicted before the run and matched: reverting the hookWrittenKeys?.has(name) branch in stripRuntimeOwnedFields turns 5 red / 8 green — every DEFECT and consequence case red, every negative control green. No rebuild leg applies: the pin imports ./engine.js, a relative specifier inside the same package's src/, so vitest resolves the subject from source and never from dist/ — the ablation going red with no build in between is itself the evidence.

The mutation was proven on disk before the run (exact occurrence counts 1 -> 0 deleted, 0 -> 1 injected, counted in Python because the anchor is multi-line and grep -c would count either half of it). ⚠️ Worth recording: the first attempt's marker matched 2 occurrences — the bare branch line is byte-identical to #14088's inside stripReadonlyFields two functions up — and the on-disk guard refused the run rather than let a reading contaminated by ablating #14088's seam through. The anchor was narrowed to the preceding comment. Restore was proven by observed state, not exit code: git hash-object equal to the HEAD blob, git diff HEAD empty, git status --porcelain empty, with the restore path pinned to an absolute repo root inside the trap.

Lint. The whole population, not a narrowing: eslint . --no-inline-config --format json (the exact rule set pnpm lint runs) → 5702 files, 0 errors, 0 warnings, exit 0.

Gates. Re-derived after the last commit with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands (no path list — the script takes its own change set), then run as a union on this HEAD: 60 green, 0 red, 4 NOT MEASURED. The four are exit 3 prerequisite refusals, recorded in the gates' own words and never as passes:

  • check-test-completeness — needs a saved turbo run test log this gate cannot produce; CI tees one.
  • check-half-states — needs repo-scoped GitHub reads this container's egress refuses.
  • check:dual-build-cjs-loadsPREREQUISITE NOT MET, reads built output for 56 unbuilt packages.
  • check:type-check-debtPREREQUISITE NOT MET, --re-measure needs 41 dependency closures built.

check:skill-examples initially refused for the same reason (packages/client-react/dist unbuilt, and its own text says a verdict then would be a FALSE GREEN); the closure was built and it now reports 263 prose examples type-check across 3 surfaces, exit 0.

Gate scaffolding, regenerated and never hand-edited.check-system-context-census went red on pure line rot — the insert-path arm/seal shifts every later engine.ts line — and was repaired with node scripts/check-system-context-census.mjs --fix (15 anchors rewritten in content/docs/permissions/system-context.mdx; the gate then reports OK — 109 elevation read sites ... all anchored). check:objectql-double-limit flagged the new pin's driver double for ignoring the caller's bound; the double now applies limit after the filter, by presence, and the gate is green. check:engine-double-contract was green throughout and needed no ledger row.

⚠️ One measurement trap worth recording, because it read exactly like a regression: immediately after merging origin/main, engine-filter-array-lowering.test.ts failed 6 cases. It was stale dist/, not code — the merge brought packages/spec changes my worktree's build predated. Clean origin/main in a comparison worktree ran the same file 60/60 green, and rebuilding this worktree's dependency closure made the failure vanish. Nothing was "fixed"; the instrument was.

Scope

Changeset: @objectstack/objectqlpatch. packages/objectql-internal throughout — none of the four functions is re-exported from index.ts or core.ts, and every mention outside rule-validator.ts / engine.ts is prose in a comment (packages/spec, packages/metadata-protocol, three packages/lint files, one showcase object) or a test in this package. The new option is optional and every positional signature is unchanged, so no existing call site moves.

🤖 Generated with Claude Code

https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68


Generated by Claude Code

… runtime-owned strips
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…te provenance
Withholds the readonlyWhen sibling seam: threading the record into
isCallerSuppliedValue was measured to let a caller value survive a TRUE
readonlyWhen predicate (#9107 pin LOCK 3b), which #14259's fork clause
routes to the decision inbox rather than to a unilateral choice.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…le; re-anchor the system-context census
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql, touching 2 documentable anchor(s).

18 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aaf.

1 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 15 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aafpackageMentionDocs.

Which tree this was computed on

This run read content/docs from afdbce2baed0487c867d978b8956d531e297b5cf — the merge of head 25619ebb76b68f80ef5489dddb101255bdc1d802 into base 18d816a504a5795a13c609304eab5825cf923aaf, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin afdbce2baed0487c867d978b8956d531e297b5cf && git checkout afdbce2baed0487c867d978b8956d531e297b5cf
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 18d816a504a5795a13c609304eab5825cf923aaf 25619ebb76b68f80ef5489dddb101255bdc1d802 && git checkout -B drift-repro 18d816a504a5795a13c609304eab5825cf923aaf && git merge --no-ff 25619ebb76b68f80ef5489dddb101255bdc1d802
node scripts/docs-audit/affected-docs.mjs --json 18d816a504a5795a13c609304eab5825cf923aaf

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 18d816a504a5795a13c609304eab5825cf923aaf → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-muskClaude

Copy link
Copy Markdown
CollaboratorAuthor

Landing provenance — engine execution seat (session session_0112hMx9hjJ9BgB28X97DS68).

ACCEPT on the card: comment 5506811038 (#14259 — seam 2 ships here as Part of; seam 1 is in the decision box, four-facet block 5506819337). Flip pre-checks on head 25619ebb7: every one of the 34 check runs completed with conclusion success or skipped (Lint & Repo Gates completed 08:50:47Z; Part-of PR must not also close its card green); governed-surface test on the final 5-path file list: 0 of 5 path(s) hit the register ⇒ ordinary queue landing; closing-keyword two-read done at ACCEPT (Part of #14259, no closing keyword — correct, the card stays open as the seam-1 carrier). Clause-② self-reading no (package-internal functions, optional parameter, no public-entry export), so no contract-review carrier applied.

Action: draft: false then auto-merge (squash) — the merge queue takes it from here. At MERGED the card flips pm:dispatchedneeds-user-decision in one stroke.


Generated by Claude Code

@os-musk
os-musk enabled auto-merge September 2, 2026 08:53
@os-musk
os-musk added this pull request to the merge queueSep 2, 2026
Merged via the queue into main with commit 00ff228Sep 2, 2026
36 checks passed
@os-musk
os-musk deleted the claude/issue-14259-hook-provenance-sibling-seams branch September 2, 2026 09:18
os-musk pushed a commit that referenced this pull request Sep 2, 2026
…th faces
`isCallerSuppliedValue`'s docblock claimed the readonlyWhen seam runs "the
identical two-part test `stripReadonlyFields` applies" and is "textually
parallel" with it. Neither has held since #14088 moved the static face onto a
hook-write record and #14472 did the same to the insert-side twin. The
divergence is deliberate and lock-motivated, so it is now stated where the test
lives, with the reason each face guards a different thing.
Adds the static face's missing measurement pin (a lone self-assigning hook
leaves the CALLER value on the key) beside #14088's own suite, and cross-links
it with `LOCK 3b`, which pins the opposite verdict for the identical hook
spelling. Comment- and test-only; no behaviour moves.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@os-musk@claude