Skip to content

fix(lint): stop the readonlyWhen hints ruling out the working remedy and offering a useless one - #14202

Merged
baozhoutao merged 2 commits into
mainfrom
claude/issue-13832-readonly-when-hints
Sep 1, 2026
Merged

fix(lint): stop the readonlyWhen hints ruling out the working remedy and offering a useless one#14202
baozhoutao merged 2 commits into
mainfrom
claude/issue-13832-readonly-when-hints

Conversation

@baozhoutao

Copy link
Copy Markdown
Contributor

Fixes #13832

Message text only, across the three carriers the card and its follow-up name. Rule ids, severities and match sets are untouched — no finding changes shape, appears, or disappears. But the hint is the whole product of an advisory rule (neither readonlyWhen finding blocks a build), so the sentence is the entire thing the author acts on, and two of these sentences were measured false against the engine.

What was wrong, and in which direction

The defect ran in both directions at once — one hint recommended a remedy that does nothing, and two carriers ruled out the remedy that works.

The false remedyflow-update-readonly-when-field said:

If automation must maintain this field regardless of record state, run the flow runAs:'system'.

It does not. stripReadonlyWhenFields runs on the update path with no isSystem guard at all, unlike the static readonly strip immediately below it, which really is skipped for system callers. packages/objectql/src/engine.ts says so at the call site, in the #9107 note: "isSystem is still NOT an exemption here, unlike the static strip below." So the advice bought the author an elevated run identity, a re-run, the same missing column — and runAs:'system' in the tree with no compensating behaviour. That is what the triage grading called decisive: advice to widen a write's privileges for no effect.

The ruled-out remedy — the hook-api-update-readonly-when-field hint and the matching hook-bodies.mdx bullet both asserted:

readonlyWhen strips even a beforeUpdate-derived value, so an own-hook stamp is NOT a workaround here

That is the behaviour #9107 removed. The conditional strip now judges the caller's entry snapshot, so a value a beforeUpdate hook derives is not caller-supplied and lands even on a locked record.

Premise re-verification — two carriers had already half-moved

Re-checked against origin/main before editing, and reported here because it changes what the card asked for. PR #14044 ("Stop lowering hook handlers that call ctx.api.sudo() into bodies that cannot run it") already removed the elevation half from two of the three carriers:

Carrier Elevation advice "strips a derived value"
validate-readonly-hook-writes.ts already removed by #14044 still false — fixed here
hook-bodies.mdx bullet already removed by #14044 still false — fixed here
validate-readonly-flow-writes.ts still live — fixed here absent (remedy was simply missing)

So the card's title claim ("the hints on the hook and flow rules recommend elevation") is now true only of the flow rule. The defect is real on all three, but the hook and docs halves that survived are the derived-value falsehood, not the elevation one. Nothing here re-litigates #14044 — its correction stands and is preserved verbatim, including its stronger, separate reason that sudo() is a TypeError from a sandboxed body.

The three carriers, before to after

1. packages/lint/src/validate-readonly-hook-writes.tshook-api-update-readonly-when-field

  • before: "readonlyWhen strips even a beforeUpdate-derived value, so an own-hook stamp is NOT a workaround here — and neither is ctx.api.sudo() …"
  • after: names the two working remedies, and refuses elevation on two independent grounds — sudo() is unreachable from a body (Stop lowering hook handlers that call ctx.api.sudo() into bodies that cannot run it #14044's reason, kept), and a system context does not waive the conditional lock in any case. The second reason is what keeps the refusal correct if the first is ever fixed.

2. packages/lint/src/validate-readonly-flow-writes.tsflow-update-readonly-when-field

  • before: "If automation must maintain this field regardless of record state, run the flow runAs:'system'."
  • after: states that the conditional lock is NOT waived by a system context, then names the same two remedies.

3. content/docs/automation/hook-bodies.mdx — the "Writing a readonly field" bullet

Deliberately not flattened

The static-readonly hints and docs rows that recommend elevation are unchanged, because for that strip elevation genuinely is the intended channel. The flow rule's readonly hint still says runAs:'system'; the docs' action paragraph still says a readonly write lands in an elevated action body. The two disagree for a reason, and a new pin in the flow test now holds them apart explicitly, so a future text sweep cannot quietly align them.

One in-file comment correction rides along, named here rather than left silent: validate-readonly-flow-writes.ts's header asserted that a runAs:'system' run makes "the engine skip the strip entirely" — the same false belief that produced the hint, and it would now contradict the corrected hint in its own file. It is narrowed to the static strip, with the engine evidence cited. No match-set change: the rule still skips runAs:'system' flows entirely. That skip is genuinely wider than the conditional lock warrants — a system flow writing a readonlyWhen field is still stripped on a locked record and goes unflagged — which is a behaviour question the triage fenced out of this card; filed as #14201 and noted in the comment.

Verification

Gate families derived from the actual diff via node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (the script reads its own change set; the first derivation reported a STALE TREE, so origin/main was merged and it was re-derived on the merged tree). Exit codes captured by redirect before any pipe.

pnpm --filter @objectstack/lint exec vitest run src/validate-readonly-hook-writes.test.ts src/validate-readonly-flow-writes.test.ts src/validate-readonly-action-writes.test.ts3 files, 79 tests passed.

Ablation (the pins are new, so they had to be shown capable of failing): the old hint text was restored on both rule files, the mutation confirmed on disk by occurrence count in both directions (injected text present, replaced text absent), the suite re-run, and the tree restored HEAD-pinned. Result: exactly the 2 new pins failed, 45 other tests unaffected; restore proven byte-identical to HEAD (blob hashes matched, git diff HEAD empty). Direction as predicted: RED.

Green on the merged tree: check:doc-authoring, check:doc-anchors, check:docs-single-h1, check:docs-redirects, check:docs-audit-scope, check:corpus-claim-drift, check:cross-package-test-inputs, check:changeset-gate-self-tests, check:engine-double-contract, check:objectql-double-limit, check:published-files, check:query-options-erasure, check:role-word, spec check:docs, spec check:skill-examples, lint check:doc-formula-expressions, lint check:doc-security-posture, and the rest of the derived family.

Recorded as NOT MEASURED, not as passes — each exited on a missing prerequisite (exit 3) rather than reaching a verdict: check:dual-build-cjs-loads (needs a full pnpm build), check-test-completeness.mjs (needs a saved turbo run test log; no local reading exists), and scripts/pm/check-half-states.mjs (a GitHub-querying patrol gate; it hung on network locally). Three others — lint check:doc-formula-expressions, lint check:doc-security-posture and spec check:skill-examples — first returned prerequisite-not-met, and are reported green only after building @objectstack/lint and @objectstack/client-react and re-running them.

Generated by Claude Code


Generated by Claude Code

…and offering a useless one

Message text only across three carriers; rule ids, severities and match sets
untouched.

The flow hint recommended runAs:'system'. The conditional strip has no
isSystem guard at all, so that is a privilege widening for no behaviour
change (LOCK 2 pins it). The hook hint and the hook-bodies.mdx bullet
asserted readonlyWhen strips a beforeUpdate-derived value -- the behaviour
#9107 removed -- thereby ruling out the one remedy that works.

All three now name the two measured remedies and refuse elevation, following
the shape action-api-update-readonly-when-field already ships. The
static-readonly hints that recommend elevation are deliberately unchanged;
a new pin holds the two apart.

Fixes #13832

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WLJQhde67SeTccsmnBVarV
@github-actions github-actions Bot added the size/m label Sep 1, 2026
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 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 07cced5ab97a2ec34be6c18e15b8956507885005packageMentionDocs.

Which tree this was computed on

This run read content/docs from 127f4bf4abed84c8f00fdfbb2a19bc9f3c634419 — the merge of head 3bebbecad7cd50ae67545cf4ce1faa3c8dbee883 into base 07cced5ab97a2ec34be6c18e15b8956507885005, 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 127f4bf4abed84c8f00fdfbb2a19bc9f3c634419 && git checkout 127f4bf4abed84c8f00fdfbb2a19bc9f3c634419
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 07cced5ab97a2ec34be6c18e15b8956507885005 3bebbecad7cd50ae67545cf4ce1faa3c8dbee883 && git checkout -B drift-repro 07cced5ab97a2ec34be6c18e15b8956507885005 && git merge --no-ff 3bebbecad7cd50ae67545cf4ce1faa3c8dbee883

node scripts/docs-audit/affected-docs.mjs --json 07cced5ab97a2ec34be6c18e15b8956507885005

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

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/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

lint: the readonlyWhen hints on the hook and flow rules recommend elevation, which the engine's conditional strip does not honour

2 participants