Skip to content

fix(cli,lint): refuse a hook/action body calling .create() at lowering, and withdraw the verb from the write-pattern ledger - #16900

Merged
os-project-manager merged 4 commits into
mainfrom
claude/issue-16249-refuse-create-at-lowering
Sep 8, 2026
Merged

fix(cli,lint): refuse a hook/action body calling .create() at lowering, and withdraw the verb from the write-pattern ledger#16900
os-project-manager merged 4 commits into
mainfrom
claude/issue-16249-refuse-create-at-lowering

Conversation

@os-project-manager

Copy link
Copy Markdown
Collaborator

Fixes #16249

Clause-②: no — re-declared from the delivered diff, not inherited from the dispatch. Every move in this PR narrows toward a contract the spec already declares: the build refuses a spelling the sandbox never installed, and the extractor ledger stops advertising it. Nothing published gains a member. The rejected alternative — installing a create leaf in installCtx — would have flipped it to yes, because it would have had the sandbox ratify a verb the contract never declared; it is not taken here.

The premise triage did not verify — verified, and it holds

Triage's ruling rested on an explicitly unchecked premise, so this was the first action on the card:

⚠️ 若认领席发现 spec 契约其实也声明了 create(本席未核 scoped-context.ts:154 的完整成员表),停下回报

Read at origin/main, packages/spec/src/contracts/scoped-context.ts (read-only here — PR #16783 holds that file):

  • IScopedObjectRepository declares exactly find · findOne · count · insert · update · updateById. No create.
  • Its own JSDoc names create in the measured-and-deliberately-excluded list: "create (an insert alias), deleteById, execute — no call site at all outside those fixtures."
  • ObjectRepository.create() in packages/objectql/src/engine.ts is a host-side alias of insert(), carrying the comment /** Alias for insert() — matches @objectql/core convention */.

⇒ The contract declares insert only. The contract-first route holds; the card proceeds as ruled. Nothing to stop and report.

What was wrong

Four layers disagreed about ctx.api.object('x').create({ … }), and the only one that actively told an author how to write it was the one that was wrong.

layer what it said
spec contract declares insert only
QuickJS sandbox (installCtx) installs insert / update / delete / updateMany / deleteMany / upsert — no create leaf
extractor ledger / lint advertised .create({…}) as legal api-crud-literal syntax and graded its payload as a live write
build time said nothing

An author following the ledger got a body that threw TypeError: not a function on its first run, and under a hook's default onError: 'abort' that throw aborted the triggering write, with a message naming no member — the blind message #14010 measured for sudo().

The three moves, together

  1. packages/cli/src/utils/extract-hook-body.ts.create( joins FORBIDDEN_PATTERNS. Shaped on .sudo(, one entry over, with a reason that names .insert({ ... }) as the spelling the sandbox actually has and lists the leaves the VM installs. The header comment above the table keeps up. Behaviour is the ordinary forbidden-token fallback: the callable is still registered and still shipped through the .mjs bundle, so the handler keeps running in-process where the host alias exists — the build merely declines to also emit it as a body that cannot run. Under --strict-body it is a hard failure, correctly.

    The alias question is answered by .sudo(, not re-decided here: receiver-loose, so const repo = ctx.api.object('x'); repo.create(…) is refused too. ⭐ One carve-out .sudo( needed no equivalent of: Object is a real sandbox global (pinned in SANDBOX_GLOBALS), so Object.create(null) is working, lowerable code. A bare receiver-loose rule would refuse it — turning a correct body into a bundled closure, and a hard failure under --strict-body. That is a false refusal, not the safe direction, so the pattern excludes that one receiver by lookbehind and nothing else (myObject.create( still matches, because the word boundary is required). Both directions are pinned.

  2. packages/lint/src/validate-hook-body-writes.tscreate withdrawn from the ledger. Out of the api-crud-literal advertised syntax, and out of API_WRITE_METHODS. The map's docblock now records both absences (upsert, create) with their measured reasons, the way this repo keeps "excluded" tellable from "overlooked". hook-body-write-unknown-field / action-body-write-unknown-field stop grading a call that cannot run — the ledger is shared, so the withdrawal lands on the action surface identically, and for the same reason (an action body runs in the same VM).

  3. packages/lint/src/validate-readonly-hook-writes.tsfinding: the flow and hook readonly rules still carry the superseded "INSERT is engine-exempt" premise as a scan gap — a non-system create_record / ctx.api.insert of a readonly column is now a silent no-op nothing reports at build time #15394's exclusion kept, its reason updated. READONLY_HOOK_METHOD_EXCLUSIONS still excludes create; what changed is why. It is no longer "the call throws, so a silently-dropped finding would be false" — that described a shape which reached this rule because the ledger advertised it. It is now "the shape can no longer get here": the build refuses .create( at lowering, so a handler spelling it is bundled and never becomes a body.source this rule parses. The sandbox reading stays in the text as the why behind the refusal, so the entry still explains itself to someone who never opens the CLI. The file header carries the same correction. ⛔ The old sentence is pinned as not coming back.

  4. The host-side ObjectRepository.create() alias is untouched. The card draws that line: it is reachable by in-process handlers and actions and is not the defect. Only the L2 body surface is.

Pins, and the ablation that proves they can fail

Tier choice: both pins are named into the queue tier (no .e2e / .live in the filename) and land in the unit project by behaviour — they spawn nothing and boot no kernel, they call the extractor and the rule functions directly. Deliberate: a build-time refusal and a ledger partition are pure functions of source text, and the value of the pin is that it runs on every queue build rather than nightly.

Ablation run from the committed state, on fb0af58acd. Verdicts are test counts, never exit codes — vitest --project X naming no collected file exits 1 exactly as a failing suite does.

Leg A — neuter the build-time refusal (packages/cli/src/utils/extract-hook-body.ts)

reading
on-disk proof deleted text 1 -> 0, injected marker 0 -> 1; blob 073ea1c7ead8 -> 462147869c0c
baseline Tests 30 passed (30)
ablated Tests 3 failed | 27 passed (30)
restored git diff HEAD empty; blob back to 073ea1c7ead8; Tests 30 passed (30)

Leg B — re-add create to BOTH ledger halves (packages/lint/src/validate-hook-body-writes.ts)

reading
on-disk proof injected API_WRITE_METHODS row 0 -> 1, injected advertised syntax 0 -> 1; blob f6b39519c027 -> 3d89842454e8
baseline Tests 131 passed (131)
ablated Tests 3 failed | 128 passed (131) — the three named pins, one per surface
restored git diff HEAD empty; blob back to f6b39519c027; Tests 131 passed (131)

The three cases leg B reddens are exactly the requested "a re-added ledger entry reddens too":

  • validate-readonly-hook-writes.test.tspartitions the extractor's ctx.api write verbs exactly
  • validate-hook-body-writes.test.tsdoes not grade a .create() payload
  • validate-action-body-writes.test.tsdoes not grade a .create() payload on the action surface either

Whole-tree restore proof after both legs: git status --porcelain empty, git diff HEAD --stat empty. The ablation script carried an EXIT INT TERM trap restoring with git checkout HEAD -- ABSOLUTE_PATH; an empty git hash-object reading was treated as failure, never as "nothing to compare". Its first run aborted on its own guard — a marker I had chosen was not unique to the syntax line, so the pre-count was 1 where 0 was expected — and it refused to read the run rather than reporting a mutation it had not proven.

Verification

  • Repo-wide pnpm lint (eslint . --no-inline-config) — exit 0, 2m09s, on fb0af58acd. Full scan; no narrowing claimed and none needed.
  • Derived gate familiesnode scripts/pm/dispatch-gates.mjs --commands over the merge-base change set: 86 derived, 86 run, 85 green, then check:type-check-debt green on a re-run once its heap ceiling was raised (it OOMs under --max-old-space-size=4096, which is a prerequisite failure and not a finding). --ran reconciliation: 86 derived familie(s) accounted for — 86 run, 0 NOT-MEASURED.
  • Two real gate findings, both fixed in this PR: check:doc-authoring refused the tracker id I had put inside the runtime reason string (a runtime string reaches authors who cannot resolve #NNNN); the id moved to the adjacent // comment. The first CLI pin also failed on its own assertion regex, which expected the leaf list unquoted while the refusal backticks it.
  • Tests@objectstack/cli --project unit on the touched files: Tests 73 passed (73). @objectstack/lint full suite: Test Files 102 passed (102), Tests 3571 passed (3571). typecheck green on both packages. packages/spec check:generatedAll 15 generated artifacts are up to date.
  • packages/cli tiering: the integration tier is declared to CI. The diff touches no integration-tier file, no spawn entry (bin/, test/helpers/serve-process.ts) and no driver/kernel boot path; test/vitest-tiers-partition.test.ts ran green in the unit project.
  • Control-char sweep beyond check:nul-bytes: grep -naP over the nine delivered files, zero hits, with a positive control proving the query fires.

Docs drift — done on this lane, with the tool's blind spot answered by hand

node scripts/docs-audit/affected-docs.mjs --json origin/main, from a worktree whose git status --porcelain was empty. The tool's own provenance says dirty: false, head fb0af58acd, diffBase afa3a2698f. Its verdict: 0 docs, 0 release-owned docs, 4 symbol anchors (API_WRITE_METHODS, FORBIDDEN_PATTERNS, HOOK_BODY_WRITE_PATTERNS, READONLY_HOOK_METHOD_EXCLUSIONS).

⚠️ That number under-reports, exactly as its own stated blind spot predictsa page that states a rule by its inputs shares no identifier with the emitter. This change is emitter-side, so a hand token sweep was run, and it found a page the derivation could not: content/docs/automation/hook-bodies.mdx states this rule three times by its inputs and names none of the four anchors. Three passages there were falsified and are corrected in this PR — the forbidden-token list, the recognised write-shape table row (insert\|create\|update -> insert\|update), and the .create() exclusion prose. A fourth, the capability-inference table, is left listing create and gains a sentence saying why: that matcher is deliberately over-inclusive (it also names patch, remove, get, list), and create can no longer reach inference at all.

No release-owned page is falsified; none was edited. Every content/docs/releases/*.mdx page was read for a hook-body/sandbox create claim and there is none — a zero paired with a live control (the same corpus and vocabulary returns 5 hits for sandbox / ctx.api).

Other zeros in the sweep, each with its control: no page or skill outside hook-bodies.mdx spells ctx.api.object(...).create (control: the .insert form hits 4 files); the same failing query run at the branch base hits exactly the two lines this PR edits, proving the query is live. skills/objectstack-data/references/data-hooks.md's repository-method and capability tables already list insert / update / delete / upsert / updateMany / deleteMany without create — accurate before and after.

Serial constraints

Re-measured independently of the dispatch, over 21 open PRs, FULL pagination, 705 file rows, 0 empty file lists: every file this PR edits has 0 holders. The reading has a live control — packages/spec/src/contracts/scoped-context.ts correctly reports PR #16783 as its holder, which is the file this PR only reads.

验收备注 — noted, not filed

  • CAPABILITY_PATTERNS' api.write regex in extract-hook-body.ts still lists create. With the refusal running first, .object(...).create( can no longer reach capability inference, so that arm is dead for this spelling. Left alone deliberately: the matcher is documented as over-inclusive and already names three other spellings the VM does not install (patch, remove, plus get/list on the read arm), so removing one of them alone would make the set less coherent, not more. Dead code, not a defect — noted, not filed. Carrier: any PR that revisits that matcher's membership as a set.
  • READONLY_ACTION_INSERT_SILENCE in validate-readonly-action-writes.ts still lists create among its silenced methods. Its reason there is about elevation (an action body's ctx.api is already system-context), which is still true and was never the sandbox argument — so, unlike the hook-side exclusion, nothing it says has become false. What has changed is only that the verb can no longer arrive. Out of this card's declared file surface and left untouched. Carrier: the domain:devx lane, which owns packages/lint and is named in this card's file-surface declaration.
  • The "What the sandbox forbids" list in hook-bodies.mdx did not name .sudo( either, although the build has refused it since A hook cannot elevate, so a hook-written computed column cannot be protected by field-level editable: false — the guard and the writer are the same door #14010. The one bullet this PR had to write is the accurate statement of that list, so it names both members; this is a by-product of writing the row correctly, not a second change.

File surface — declared, including the supplements to the dispatch's four

The dispatch declared four files across two lanes. Delivered, nine:

file lane why
packages/cli/src/utils/extract-hook-body.ts domain:cli declared — the build-time refusal
packages/lint/src/validate-hook-body-writes.ts domain:devx declared — the ledger withdrawal
packages/lint/src/validate-readonly-hook-writes.ts domain:devx declared — the exclusion's reason
packages/cli/test/extract-hook-body.test.ts domain:cli supplement — the refusal pins (acceptance item 5)
packages/lint/src/validate-readonly-hook-writes.test.ts domain:devx supplement — the ledger-withdrawal pin; its verb-partition case is derived from the ledger's own syntax line and had to move with it
packages/lint/src/validate-hook-body-writes.test.ts domain:devx supplement — fixture triage: an existing case carried a .create({ email: 'b' }) line with a valid field, which produced no finding before the withdrawal and none after, so it would have passed while measuring nothing. Replaced with a misspelled-field case that is a real pin
packages/lint/src/validate-action-body-writes.test.ts domain:devx supplement — same fixture triage on the action surface, reached by scanning the rule's consumption radius rather than the edited package
content/docs/automation/hook-bodies.mdx docs supplement — three passages falsified by this change (see Docs drift)
.changeset/refuse-create-at-hook-body-lowering.md patch on both published packages, with the FROM -> TO migration line

packages/runtime/src/sandbox/quickjs-runner.ts and packages/spec/src/contracts/scoped-context.ts were read only, as declared.

Governed surfaces: none. docs/adr/** · .claude/** · skills/** · AGENTS.md · CLAUDE.md are all absent from this diff (checked against GOVERNED_SURFACES, with a control proving the check fires).

Generated by Claude Code


Generated by Claude Code

@github-actions github-actions Bot added the size/m label Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

4 anchor(s) derived from 2 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
  • 3 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 25 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 f2b5e4686b0464fcbc19873c24b9a477fd5a0bfapackageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json f2b5e4686b0464fcbc19873c24b9a477fd5a0bfa

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

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 8, 2026
@os-project-manager
os-project-manager marked this pull request as ready for review September 8, 2026 15:51
@os-project-manager
os-project-manager added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit ed6579b Sep 8, 2026
36 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-16249-refuse-create-at-lowering branch September 8, 2026 16:16
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

2 participants