Hook-body build gates: @capabilities override dead through os build; require() pattern dead; default build silently bundles forbidden patterns

[baozhoutao]

Found by QA run #10663 driving cli.hook-body-extraction-gates at framework 79ebb37. The security net is intact throughout — no forbidden body ever shipped as body.source, and every forbidden/free-identifier hook was still refused under --strict-body. Three defects in the reporting/reachability, not the enforcement:

1. // @capabilities override is unreachable through os build (product/docs gap)

A handler authored in objectstack.config.ts with a first line // @capabilities api.read api.write produces artifact capabilities []. The config loader (config.ts:loadConfig → bundle-require → esbuild transformSync{loader:'ts'}) strips // line comments before String(fn) runs, so the override at extract-hook-body.ts:118-131 — documented as working at hook-bodies.mdx:320-327 — never receives the comment. The pin lower-callables.test.ts masks it by feeding raw JS function literals that keep their comment, never exercising the esbuild load path. Fix: move the directive off a // comment (e.g. a real property), or state in docs that it is authorable only in pre-bundled JS; add an os build-level test.

2. require() FORBIDDEN pattern is effectively dead (assertion/coverage gap)

require('node:os') in a TS config is rewritten by esbuild's ESM shim to __require("node:os"), so FORBIDDEN_PATTERNS[/\brequire\s*\(/] (extract-hook-body.ts:35) never matches. The refusal still fires (exit 1) — via the #1876 free-identifier gate naming __require — so enforcement holds, but the require()-specific worded reason the acceptance promises is never emitted for the real authoring path.

3. Default os build silently warn-and-bundles forbidden patterns (header contradiction)

A default (non---strict-body) os build of a hook containing a forbidden pattern exits 0 with no output: lower-callables.ts:63-78 catches the extraction error and falls back to the .mjs bundle; the warning is recorded into bodyExtractionWarnings but compile.ts prints it nowhere and :437 --json warnings carries rule advisories only. This contradicts the extractor header extract-hook-body.ts:14-18 ("makes the build fail … no silent fallback"). Docs hook-bodies.mdx:256 agree with the code, so the header is the outlier. Fix: surface the warning on the default path (at minimum in --json), or correct the header to describe warn-and-bundle.

Clauses that pass

Capability inference (.find→api.read, .update→api.write, const api=ctx.api alias caught), crypto.hash infers nothing (#4391 regression), free-identifier bundle fallback (#1876), all-body-only skip + .mjs cleanup.

QA-source: #10663 · cli.hook-body-extraction-gates · clauses 3, 0, 6

[os-zhuang]

Triage: lands in packages/cli (extract-hook-body.ts, lower-callables.ts, compile.ts) ⇒ pm:queue · domain:cli · type Bug. Rationale: three declared-but-unenforced/mis-reported behaviors on the hook-body gate surface, all measured; the enforcement net itself held, so this is reporting/reachability, not a security hole.

Dispatch notes (per-defect constraints):

1. @capabilities override dead through os build: default to the docs-truth route — document that the //-comment directive only survives pre-bundled JS, and add an os build-level test proving the real path. Introducing a NEW authoring surface (e.g. a real config property for capabilities) expands the public surface → that is a maintainer decision; propose it in the report if you think it's right, don't ship it.
2. require() pattern dead (esbuild rewrites to __require): fix the pattern/wording so the promised require()-specific reason fires on the real authoring path (e.g. also match __require); enforcement already holds via [P0] build-time vs runtime validation parity: objectstack build accepts metadata the runtime rejects #1876, so this is assertion honesty.
3. Default-build silent warn-and-bundle: surface the recorded bodyExtractionWarnings (stdout + --json) and fix the extractor header to describe warn-and-bundle. Flipping the default build to hard-fail would change published os build accept behavior → not without a ruling.

• Keep the passing clauses green — the card lists them; they are your regression set.

Size/model suggestion: M, opus floor.

---

Generated by Claude Code

[os-elon]

Claim: domain:cli lane execution seat
Session: session_019bmVFqoQPq63zhKrxdYG1r
Branch: claude/issue-10678-hook-body-gate-honesty
Worktree: objectstack-issue-10678
Domain: domain:cli
File surface: packages/cli's hook-body gate surface — extract-hook-body.ts, lower-callables.ts, compile.ts — plus their tests and the extractor header. ⛔ Re-derive every anchor at head.
Container & model: M, mode:subagent, model: opus (triage's stated floor)
Clause-②: no — as fenced. ⚠️ This card sits one step from a yes on two of its three defects, and triage fenced both (see below). If your diff would change what os build accepts or what may be authored, you have crossed the line — STOP and report, do not ship it.
Serial constraints cleared: measured — nothing open touches these files. The two cards this lane has in flight elsewhere are serve.ts (#10769) and none other; packages/cli is same-package, which the seat ruling exempts — same-file is the hard serial and there is no overlap.

⛔ Triage has ruled each defect separately — quoting the fences

From comment 5368008541, verbatim:

1. @capabilities override dead through os build: default to the docs-truth route — document that the //-comment directive only survives pre-bundled JS, and add an os build-level test proving the real path. Introducing a NEW authoring surface (e.g. a real config property for capabilities) expands the public surface → that is a maintainer decision; propose it in the report if you think it's right, don't ship it.
2. require() pattern dead (esbuild rewrites to __require): fix the pattern/wording so the promised require()-specific reason fires on the real authoring path (e.g. also match __require); enforcement already holds via [P0] build-time vs runtime validation parity: objectstack build accepts metadata the runtime rejects #1876, so this is assertion honesty.
3. Default-build silent warn-and-bundle: surface the recorded bodyExtractionWarnings (stdout + --json) and fix the extractor header to describe warn-and-bundle. Flipping the default build to hard-fail would change published os build accept behavior → not without a ruling.

⚠️ Defects 1 and 3 both have an obvious "proper" fix that is explicitly not yours to make. Adding a capabilities config property, or making the default build fail on a forbidden pattern, are the changes a careful engineer reaches for — and both are maintainer decisions about the published surface. Propose either in open_questions with your evidence; ⛔ do not ship it.

⭐ What this card is actually about

All three are the same shape, and it is the shape this lane has now closed four times today: a gate that reports something it did not establish.

• @capabilities reports an override that never reaches the real build path;
• the require() branch promises a specific reason that cannot fire, because esbuild has already rewritten it to __require;
• the default build warns and bundles anyway, and records bodyExtractionWarnings that nothing prints.

Triage is explicit that the enforcement net itself held — so this is reporting and reachability, ⛔ not a security hole. Do not describe it as one, and ⛔ do not assess exploitability; severity grading is the maintainer's.

⭐ Defect 3 is the most valuable of the three, for the same reason os datasource validate was: the data already exists (bodyExtractionWarnings is recorded) and simply never reaches a human. Making recorded-but-unprinted state visible is a smaller change than it looks and removes a whole silent class.

Fences

• Keep the passing clauses green — the card lists them and they are your regression set. ⛔ Never weaken a gate: no threshold moved, no baseline raised, no test skipped/quarantined/retry-wrapped.
• For defect 1, the deliverable includes an os build-level test proving the real path — a unit test over the extractor alone would restate the same false confidence the card is about.
• ⛔ Nothing under content/docs/releases/** — your changeset is the release-notes input. Docs that describe the directive's real reach are in scope for defect 1's docs-truth route.

premise_still_valid: false is a legitimate delivery for any individual defect that no longer reproduces at head — report per-defect, not as one verdict.

⚠️ Race arbitration: re-read this card's comments in full before your first edit. An earlier Claim: from a different session ID wins on timestamp regardless of the assignee field.

---

Generated by Claude Code

[os-elon]

os-dev-report

{
 "issue": 10678,
 "status": "done",
 "branch": "claude/issue-10678-hook-body-gate-honesty",
 "pr": "https://github.com/objectstack-ai/objectstack/pull/10912",
 "premise_still_valid": true,
 "summary": "All three defects reproduced at head (7c02a4529c) before any edit, and each was delivered on the route triage fenced for it. Defect 3 (highest value): bodyExtractionWarnings was already recorded and printed nowhere, so a forbidden pattern in a hook body was an exit-0 build with no output at all; compile.ts now prints it on the default path and carries it in --json under a new bodyExtractionWarnings key, separate from the rule-advisory warnings key whose record shape os validate --json shares. Exit code deliberately unchanged. Defect 2: esbuild rewrites require( to __require( before String(fn) runs, so the require()-specific reason could never match on a TS config; the pattern now matches both spellings under one reason that explains the rewrite, with accept behaviour unchanged and tested as such. Defect 1: docs-truth route, and the premise came back STRONGER than triage stated. Triage said the directive survives pre-bundled JS; measured, it survives nothing an app author would write. bundleRequire runs esbuild on every config regardless of extension, so .ts, .js, .mjs and a handler imported from a local ./handlers.js ALL silently drop it and ship inferred capabilities alone. Documenting the triage wording verbatim would have pointed readers at a .js file that also does not work, i.e. a fresh false claim of exactly the class this card is about; the docs now state the measured reach and point at body.capabilities (data, not a comment) as the escape hatch that does survive, itself measured. Per triage, this is reporting and reachability, not a security hole: the enforcement net held throughout and is unchanged. No fenced change was shipped: os build accepts exactly what it accepted before, and nothing new became authorable.",
 "tests": "Repro at head, real CLI spawned against mkdtemp projects (bin/run-dev.js + tsx): defect 1 shipped capabilities [] for a handler asking api.read api.write; defect 2 gave 'handler references identifier(s) not in scope at runtime: __require' instead of the require() reason; defect 3 exited 0 with no output. After the fix: defect 3 default build prints '1 handler could not be lowered to a metadata body' plus the per-hook fetch() reason, still exit 0, and --json carries bodyExtractionWarnings with origin \"hook 'def_probe'\"; defect 2 --strict-body exit 1 naming '`require()` is not allowed in hook/action bodies (esbuild rewrites it to `__require()` ...)' while the default build still exits 0 and still bundles. NEW TEST packages/cli/test/hook-body-build-reach.e2e.test.ts is os build-LEVEL (spawns the CLI, reads the artifact and the shell exit code) because a unit test over the extractor alone would restate the same false confidence: the existing unit tests feed raw JS literals that keep their comments and their require( spelling. Suite: packages/cli full run 147 files / 1622 tests passed, 0 failed; pnpm --filter @objectstack/cli typecheck clean. Gates: 29 green at final head 4edd41f194, derived by 'node scripts/pm/dispatch-gates.mjs' with NO hand-built path list (27 derived families + check:nul-bytes + the two convention-triggered families the residue names for new test files), os-verify-lock VERDICT command-exit 0. Ratchet check:type-check-debt run after turbo run build over the workspace closure (70 successful, 70 total); its own verdict line reads 'check-type-check-coverage: OK - 64/77 workspace packages type-checked (plus the root), 13 in the DEBT ledger', TCDEBT_EXIT=0. ABLATION / REBUILD: both fix legs were reverted on top of the commit and re-run; predicted direction was red and red is what happened, 5 failed / 21 passed, each failing for the right reason (the __require case fell back to the free-identifier message; the defect-3 case showed 'Build complete' with no hook named; the --json control read undefined instead of []). Restore leg re-run to green: 2 files / 26 tests passed. NO REBUILD was needed for either leg and none is claimed - bin/run-dev.js runs the CLI from src/ through tsx (its header says it exists so tests do not depend on packages/cli/dist being built) and the unit test imports from src, so nothing in this ablation resolves through dist. The defect-1 reach test stayed green under ablation, correctly: it pins pre-existing measured behaviour and has no code mutation to be sensitive to. Trees: suite and ratchet saw commit 5b09f67a9e; the amend to 4edd41f194 added one prose line to hook-bodies.mdx, which is in no tsc program and no test, and the five docs families that can see it were re-run on the amended tree.",
 "open_questions": [
 {
 "question": "Defect 1 fence: should the `@capabilities` override get a real authorable surface, or be retired? Measured, it reaches the extractor from NO ordinary authoring shape through os build (.ts, .js, .mjs, and a handler imported from a local ./handlers.js all fail), because bundleRequire runs esbuild on every config and esbuild strips // line comments before the handler is a runtime value. So today a documented feature does nothing on every path an app author would use. This PR takes triage's docs-truth route only.",
 "options": [
 "A - Keep docs-truth as shipped: the directive stays in the docs, documented as not reaching the build, with body.capabilities named as the escape hatch. Zero surface change, but leaves a directive that does nothing and that a reader may still copy.",
 "B - Retire the directive: delete the override branch in extract-hook-body.ts, the docs block, and the two masking unit tests. Removes a dead authorable-looking surface; body.capabilities already covers the need and is measured to survive. This is a REMOVAL from the published surface, so it is a maintainer call.",
 "C - Give it a real surface: a hook-level config property (e.g. capabilities on the hook, alongside body). Makes the intent expressible on the real path, but EXPANDS the public authorable surface - explicitly fenced by triage."
 ],
 "recommendation": "B, and if not B then A. Business need: no measured consumer - the directive is reachable from no authoring path, so nothing in the tree or in any example app can be relying on it today. Long-term soundness: a documented directive that silently does nothing is the exact declared-but-unenforced shape this repo keeps paying for, and A preserves it indefinitely; contract-first says fix it at the producer or remove it. AI-authored metadata: this is the strongest axis for B - an AI writing a metadata app reads the docs, copies the comment, gets capabilities [] with NO error and NO warning, and the failure surfaces later as a sandbox denial far from the cause. Removing it makes that mistake impossible to express; documenting it merely asks the author to have read the caveat. C is the one that fails the startup-scope axis: it adds published surface for a capability nobody has pulled for, and body.capabilities already expresses it as data. Note the ADR-0049 enforce-or-remove framing and the spec-property-retirement playbook look directly applicable if B is chosen."
 },
 {
 "question": "Defect 3 fence: should a forbidden pattern fail the default os build, rather than warn-and-bundle at exit 0? The extractor header claimed it did ('makes the build fail ... no silent fallback'); the code has always warned and bundled, and hook-bodies.mdx agreed with the code. This PR resolved the contradiction toward the CODE (header rewritten, warnings surfaced) because the other direction changes what os build accepts.",
 "options": [
 "A - As shipped: default warns and bundles, now visibly; --strict-body remains the way to make it fatal. No accept-behaviour change.",
 "B - Flip the default to hard-fail, making --strict-body the default and adding an opt-out flag. Matches the header's original promise and closes the L3 .mjs path faster, but CHANGES what os build accepts - every existing app with a forbidden pattern in a hook body starts failing its build.",
 "C - Stage it: keep exit 0 now, add a deprecation notice to the printed warning naming the release in which it becomes fatal, then flip in that release."
 ],
 "recommendation": "A as the immediate state (already shipped), with C as the path if the header's original promise is the real intent. Business need: warn-and-bundle is what deployments actually rely on today, and B would break builds that currently pass with no migration window - the blast radius is every app with a forbidden pattern in a hook body, which is exactly the population that never saw a warning until this PR. Long-term soundness: the .mjs fallback is documented as closing in Phase 3, so B is the eventual destination and C is how to get there without a silent breakage; picking C also means the warning this PR added becomes the migration signal, which is why surfacing it first was the right order. AI-authored metadata: B is strongest here (a hard failure at authoring time is unmissable), which is the real argument for reaching C rather than sitting on A forever. Startup scope: C costs one line in the warning now and defers the accept change to a release boundary. Either way the decision is the maintainer's - I shipped only the reporting half."
 }
 ],
 "out_of_scope_findings": []
}

---

Generated by Claude Code

[os-elon]

pm-dispatch-outcome

Delivered as PR #10912, all three defects on the routes triage fenced. Reviewed by content; ⛔ not enqueued yet — CI will be read once, by name.

⭐ The dev found triage's own premise understated — and following it verbatim would have shipped a NEW instance of this card's defect

Triage said the @capabilities directive "only survives pre-bundled JS". Measured, it survives nothing an app author would write: bundleRequire runs esbuild on every config regardless of extension, so .ts, .js, .mjs, and a handler imported from a local ./handlers.js all silently drop it and ship inferred capabilities alone.

The consequence the dev drew is the best thing in this round:

Documenting the triage wording verbatim would have pointed readers at a .js file that also does not work, i.e. a fresh false claim of exactly the class this card is about.

⭐ That is the trap this card exists to close, re-entered through the fix itself. Obeying the instruction literally would have produced a new documented-but-untrue statement — and the only thing standing between the two was the dev choosing to measure the reach rather than transcribe it. The docs now state the measured reach and point at body.capabilities (data, not a comment) as the escape hatch that does survive — itself measured, not assumed.

Fences held — verified by content, not accepted from the report

• ⛔ No accept-behaviour change. No process.exit / exitCode / new this.error anywhere in the source diff; the only --strict-body mentions are comments and docs describing the flag that already existed. os build accepts exactly what it accepted before.
• ⛔ No new authoring surface. No schema or capabilities property added — the fenced "real config property" was proposed, not shipped.
• ⭐ The --json key is separate. bodyExtractionWarnings is a new key, deliberately not folded into the existing warnings key whose record shape os validate --json shares. Reusing it would have changed a shape another command depends on — a quiet way to break a third party while fixing a second.
• ✅ The new test is genuinely os build-level: execFile spawning the real CLI (bin/run-dev.js + tsx) against a mkdtemp project, reading the artifact and the shell exit code. The dev names why a unit test would not have done: the existing unit tests feed raw JS literals, which keep their comments and their require( spelling — i.e. they reproduce the false confidence rather than test it.
• ✅ Docs touched are content/docs/automation/hook-bodies.mdx, in scope for defect 1's docs-truth route. ⛔ Nothing under content/docs/releases/**.

Defect 3 is the one that removes a whole silent class

bodyExtractionWarnings was already recorded and printed nowhere — so a forbidden pattern in a hook body was an exit-0 build with no output at all. It is now printed on the default path and carried in --json. Exit code deliberately unchanged.

⚠️ Note what this resolved: the extractor header claimed the build fails ("makes the build fail … no silent fallback") while the code has always warned and bundled, and hook-bodies.mdx agreed with the code. The dev resolved the contradiction toward the code and rewrote the header — the right direction, because the other one changes what os build accepts.

Verification

• All three defects reproduced at head before any edit, with the real CLI against mkdtemp projects — defect 1 shipped capabilities []; defect 2 gave the free-identifier message instead of the require() reason; defect 3 exited 0 silently.
• Ablation: both fix legs reverted, 5 failed | 21 passed, each failing for the right reason. ⭐ And the dev states that the defect-1 reach test stayed green under ablation "correctly: it pins pre-existing measured behaviour and has no code mutation to be sensitive to" — explaining which evidence did not move, rather than presenting five reds as five proofs.
• Suite 147 files / 1622 tests; 29 gates green at final head; ratchet run after a full 70-package closure build.
• ⚠️ Tree discrepancy declared: the suite and ratchet saw 5b09f67a9e; the amend to 4edd41f194 added one prose line to hook-bodies.mdx, which is in no tsc program and no test, and the five docs families that can see it were re-run on the amended tree. Correctly disclosed rather than papered over.

⛔ Two decisions escalated to the maintainer — I am not answering either

Both are the fenced questions, and both are genuinely his:

1. Should @capabilities be retired rather than documented? Measured, it reaches the extractor from no ordinary authoring shape. The dev recommends retiring it (ADR-0049 enforce-or-remove; the spec-property-retirement playbook applies), with docs-truth as the fallback. ⭐ His strongest argument is the AI-authoring axis: an agent reads the docs, copies the comment, gets capabilities [] with no error and no warning, and the failure surfaces later as a sandbox denial far from its cause.
2. Should a forbidden pattern fail the default build? Recommends keeping today's warn-and-bundle (now visible), with a staged deprecation as the path to the header's original promise — because flipping it now breaks every app that currently builds with a forbidden pattern, which is exactly the population that never saw a warning until this PR.

⛔ Neither blocks this PR: it ships the reporting half only, and that half is complete on the fenced routes. I will file the retirement question as its own card for grading rather than leaving it in a PR report.

---

Generated by Claude Code