Skip to content

feat(lint): gate a hook body's ctx.api write to a readonly field - #13771

Merged
os-project-manager merged 3 commits into
mainfrom
claude/issue-13653-readonly-hook-write-lint
Aug 31, 2026
Merged

feat(lint): gate a hook body's ctx.api write to a readonly field#13771
os-project-manager merged 3 commits into
mainfrom
claude/issue-13653-readonly-hook-write-lint

Conversation

@claude

@claude claude Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Fixes #13653

A hook body writing a readonly field through ctx.api is a silent no-op. ctx.api is a ScopedContext over the triggering operation's execution context (buildHookApi), so ctx.api.object('x').update({ readonlyField }) reaches the engine as an ordinary non-system caller; the update path runs stripReadonlyFields under if (!opCtx.context?.isSystem) and deletes the key. The call returns success and the column stays null for the life of the app.

This is the hook-side completion of the flow-side gate flow-update-readonly-field.

The asymmetry the rule is built around

readonly + a beforeInsert/beforeUpdate stamp is a correct and widely used pairing and is never flagged. The strip drops a key only while it is still Object.is-equal to what the caller supplied, so a ctx.input.<field> = ... stamp writes a server value that survives. The judgement is therefore keyed on the write channel, not on the field:

Body writes Verdict
ctx.input.<field> = ... / Object.assign(ctx.input, ...) never flagged — survives the strip
ctx.api.object('lit').update() / .updateById() flagged
ctx.api.sudo().object(...) never flagged — elevated, the intended channel
ctx.api.object(...).insert() / .create() never flagged — INSERT is engine-exempt

Both directions are pinned by tests, and the GREEN half is proven load-bearing by ablation (below), not merely asserted.

New rule ids

  • hook-api-update-readonly-fielderror
  • hook-api-update-readonly-when-fieldwarning (per-record state)

Wired through REFERENCE_INTEGRITY_RULES, so it runs on os validate, os lint and os compile at once rather than being hand-wired per command. buildReadonlyIndex is now shared from the flow rule instead of copied.

Zone 2 — the PM's assumptions, measured

A2.1 — CONFIRMED. The extractor already separates the two write kinds: an api-crud-literal write carries object + method; ctx.input writes carry neither. Measured with the real extractor, not read off the source.

A2.2 — FALSIFIED, and it shrinks the card. The flow half is not something to build from validate-flow-node-writes.ts: #3425 was closed as delivered, not rejected, and validate-readonly-flow-writes.ts already ships the complete flow-side gate (error for static readonly, warning for readonlyWhen, skipping runAs:'system' and create_record). So this card is purely the hook-side completion, and the right move was to mirror that rule rather than invent a posture.

A2.3 — severity resolved as error, and here is the reasoning rather than a coin-flip. The two candidate precedents disagree because they answer different questions:

  • flow-update-readonly-field (error) is this same judgement one surface over.
  • hook-body-write-unknown-field (warning) asks "does this field exist?", which can be wrong for reasons outside the stack — another package may declare the field.

The readonly judgement is answered entirely from two facts this stack declares: the field's readonly, and the body's literal ctx.api update. That is the flow rule's epistemic position, not the unknown-field rule's — so it gates.

The honest caveat, stated rather than glossed: a hook has no declared run identity. A flow declares runAs, which is what lets its rule call the strip a certainty. A hook inherits its context from whoever triggered the write, so the write is dropped whenever the trigger is non-system (the default, and the only path a user-reachable object can rely on) and lands on a system-triggered one. The residual case is not a stable invariant — nothing declares or enforces it, and the first user-context write silently voids the stamp. Its remedy is the same .sudo() the rule points at, so the flagged code is worth changing under both readings. That is what makes gating defensible here where it would not be for an existence check.

A2.4 — readonlyWhen covered at the sibling's boundary, not widened. The flow rule already grades it warning; this rule mirrors that exactly. Its hint differs on a measured point: readonlyWhen strips a beforeUpdate-derived value too, so the own-hook stamp is not a workaround there and the hint does not offer it.

A2.5 — two-way fixtures built in this repo. The reference-app shape is reproduced locally (crm_account.last_activity_date as the outage field, name_normalized as the correct readonly + before-hook pairing).

A false-positive vector the order did not know about

ScopedContext.sudo() returns a context with isSystem: true, which the strip skips entirely — the hook-side analogue of runAs:'system'. It is already structurally invisible to the extractor (the matcher requires a literal ctx.api receiver; ctx.api.sudo() is a CallExpression), so elevated writes cannot be flagged even by accident. A test pins this, so a future extractor change cannot start gating the one channel the platform recommends.

Also excluded, each for a stated reason: dynamic object names, non-literal payloads, objects/fields this stack does not declare, objects declaring no fields at all, an unparseable body (a gating rule must not fire off a partially recovered tree), and id in an update payload — that key is the row address, and the engine deliberately does not report it.

Verification

Built at 915f9cc5c.

Ablation — the GREEN half is load-bearing. Committed first, then mutated the rule into the naive blanket form (accept every write shape, resolve an object-less write against the hook's own target). Mutation proven on disk before measuring: injected marker 0 -> 2, removed text 1 -> 0, blob hash changed. Result: 7 failures, and they are exactly the GREEN cases — all four ctx.input stamp shapes, both INSERT cases, and the exclusion-ledger drive. The RED cases stayed green. Restore leg verified by state, not exit code: worktree blob hash 4f582d01 equals the HEAD blob, git diff HEAD empty, marker count back to 0. (Source-resolved via the relative import, so there is no dist leg to go stale.)

Field data before choosing to gate. Ran the real extractor over every source: literal in examples/ — 187 body sources, positive controls live (4 ctx.input writes seen). ctx.api update writes in hook bodies: 0. End-to-end, turbo run build over the whole workspace is green (70/70 tasks), which includes os build on all three example apps under the new gating rule.

Tests. @objectstack/lint full suite 2387 passed / 86 files; the new suite is 27 cases.

Gates. Family re-derived on the final commit 915f9cc5c with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (53 commands / 49 families, unchanged by the docs commit) and all 53 re-run on that head: 52 GREEN, 1 NOT MEASURED. The one is scripts/check-test-completeness.mjs, exit 3, which the script itself defines as PREREQUISITE NOT MET — it grades a saved turbo run test log that only CI tees, and its own output says "⛔ It is not a red". Every exit code was captured before any pipe.

pnpm lint (repo-wide eslint, . with --no-inline-config) is GREEN with no narrowing. pnpm --filter @objectstack/lint typecheck is green — with the honest qualifier that --listFiles shows it covers validate-readonly-hook-writes.ts but not validate-readonly-hook-writes.test.ts (this package hides its tests from tsc, pre-existing ledgered debt), so that green says nothing about the test file's types; vitest exercises it.

dispatch-gates also warned the tree is 7 commits behind origin/main with 10 gate-defining files changed in that range — CI runs the authoritative family on the merge.

A stale sentence in the card, stated plainly

The card lists its prior art under "Prior art, none of it open" and concludes "None of them gave the author a signal". That is stale for the flow half, and correcting it changes what a reader should understand this PR to have added.

#3425 was closed as completed, not rejected. Its closing comment records the delivery: validateReadonlyFlowWrites shipped in #3465 (4340f1319), wired into os validate / os build, gating a runAs:'user' update_record write to a static-readonly field as an error, grading readonlyWhen as a warning, and exempting create_record and runAs:'system' — the same carve-outs this rule mirrors. packages/lint/src/validate-readonly-flow-writes.ts is on origin/main today and its header says so.

So this PR is not the first author-time signal of its kind. It is the hook-side sibling of an existing flow-side rule, and the design work was to mirror that rule's boundaries rather than to invent a posture. The card's underlying complaint is still exactly right for the surface it was filed about: the hook side had no signal, which is what this closes.

Docs drift check — the reading

The drift check listed content/docs/api/error-handling-server.mdx via the anchor updateById, a string literal in this diff's STRIP_SUBJECT_METHODS. Read, no change needed, for two independent reasons: its updateById example is an L1 handler hook, and this rule only reads L2 (body.language === 'js') sources, so that code is outside its reach entirely; and the page's other mention states the repository signature — "the record id travels inside data for update" — which is the very fact this rule's address-key exclusion encodes, so the page corroborates the rule rather than being falsified by it.

The check declares two blind spots, and both turned out to matter here — hand-checking them found real staleness the tool could not:

  • packages/lint/src/index.ts yielded no anchor, so pages naming the lint rule roster were not covered by that run. content/docs/automation/hooks.mdx was one: it listed "readonly targets" as a flow-only os validate check and said a hook body's write set is checked "only as an advisory warning". Both went false with this PR; both are corrected here.
  • The check cannot see a page that states a rule by its inputs when the diff changed the emitter. content/docs/automation/hook-bodies.mdx is exactly that shape, so it was re-read by hand: two further over-broad claims ("write side, advisory and literal-only"; "Because the checking is advisory and literal-only") are now scoped, because existence stays advisory for its own reason while writability gates.

That drift run also reports its checkout carried uncommitted changes, so its named commit does not fully identify what it read — its row is quoted above as a pointer, not treated as authoritative over the tree.

Docs

content/docs/automation/hook-bodies.mdx carried the claim that a gating readonly error is something "hooks have no counterpart for" — true before this PR, false after, so it is corrected rather than left to rot, and a new "Writing a readonly field" section documents the four-channel table above. content/docs/automation/hooks.mdx and two further "advisory" claims are corrected for the reasons in the drift-reading section.

Out of scope, filed not folded

#13770 — the action body surface has the identical hole, with a live population of five ctx.api update writes in the shipped showcase app (none currently on a readonly field). Not folded in here: the ruling narrowed this card to the hook side, and the action surface is a separate rule file with its own ledger and tests.

Option 2 of the card (making the strip observable) was not crossed into — it lands in packages/objectql. No packages/spec/src/** change was needed, and no governed-surface file was touched.

Generated by Claude Code


Generated by Claude Code

claude added 2 commits August 31, 2026 09:41
)

A hook's `ctx.api` is a ScopedContext over the TRIGGERING operation's
execution context, so `ctx.api.object('x').update({ readonlyField })`
reaches the engine as an ordinary non-system caller and the update path
strips the key. The call returns success and the column stays null - a
failure only an end-to-end read-back detects. This completes the hook
side of the flow-side gate that shipped as `flow-update-readonly-field`.

The rule keys on the write CHANNEL, not on the field: a beforeInsert /
beforeUpdate body stamping `ctx.input.<field> = ...` writes a server
value that survives the strip (#5591) and is never flagged - `readonly`
plus a before-hook is a correct and widely used pairing. Also skipped,
each for a stated reason: `ctx.api.sudo()` chains (elevated, the
intended channel), insert/create (INSERT is engine-exempt, #3043/#3413),
dynamic object names, non-literal payloads, objects or fields this stack
does not declare, and `id` in an update payload (the row address, #8141).

- `hook-api-update-readonly-field` - error
- `hook-api-update-readonly-when-field` - warning (per-record state)

Wired through REFERENCE_INTEGRITY_RULES so it runs on `os validate`,
`os lint` and `os compile` at once. `buildReadonlyIndex` is now shared
from the flow rule rather than copied.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC
`check:doc-authoring` gates on this: a finding's `message`/`hint` reaches
authors, operators and generated surfaces, none of whom can resolve
`#NNNN`. The four ids (#2948, #3042, #5591, #9107) move to adjacent `//`
comments, where the reader who CAN resolve them already is.

Maintainer ruling 2026-08-12: 「处理 issue 时犯的错应该总结成经验,保留 issue id没有意义」

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/lint, touching 22 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/lint/src/index.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/error-handling-server.mdx (via updateById (literal, a string literal in STRIP_SUBJECT_METHODS))
What this run could not see
  • 1 changed file(s) yielded no anchor (packages/lint/src/index.ts) — pages documenting those are invisible to this run
  • 5 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 — 5 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 8ab4ace4dd9b17b6e75b90b1cc77193685a748d1packageMentionDocs.

Which tree this was computed on

This run read content/docs from a8d7da766886b19c42565013c252be130e8765f5 — the merge of head 915f9cc5cfb9ef72425ff06a92b0e45bd07c2495 into base 8ab4ace4dd9b17b6e75b90b1cc77193685a748d1, 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 a8d7da766886b19c42565013c252be130e8765f5 && git checkout a8d7da766886b19c42565013c252be130e8765f5
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 8ab4ace4dd9b17b6e75b90b1cc77193685a748d1 915f9cc5cfb9ef72425ff06a92b0e45bd07c2495 && git checkout -B drift-repro 8ab4ace4dd9b17b6e75b90b1cc77193685a748d1 && git merge --no-ff 915f9cc5cfb9ef72425ff06a92b0e45bd07c2495

node scripts/docs-audit/affected-docs.mjs --json 8ab4ace4dd9b17b6e75b90b1cc77193685a748d1

⚠️ 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 8ab4ace4dd9b17b6e75b90b1cc77193685a748d1 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Aug 31, 2026
… this rule falsifies

Three statements went stale the moment a hook-body write check began to gate,
and none of them was reachable by the docs drift check: `packages/lint/src/
index.ts` yields no anchor, and a page that states a rule by its INPUTS shares
no token with a diff that changed the EMITTER.

- hooks.mdx listed "readonly targets" as a flow-only `os validate` check and
  said a hook body's write set is checked "only as an advisory warning".
- hook-bodies.mdx called the whole write side "advisory and literal-only".

Existence stays advisory for its own reason (the answer can depend on a package
the build cannot see); writability gates because both halves of that judgement
are declared in the stack being checked. The pages now say which is which.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC
@os-project-manager
os-project-manager marked this pull request as ready for review August 31, 2026 11:47
@os-project-manager
os-project-manager added this pull request to the merge queue Aug 31, 2026
Merged via the queue into main with commit 36d2878 Aug 31, 2026
38 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-13653-readonly-hook-write-lint branch August 31, 2026 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants