Skip to content

fix(cli)!: objectstack build --strict-body counts each callable once, and --json issues lists callables only - #22849

Merged
objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-22839-strict-body-count
Oct 11, 2026
Merged

objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-22839-strict-body-count

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #22839

Clause-②: no (narrowing)

What was wrong (measured on origin/main 1eff3224d)

compile.ts's --strict-body block built issues as the body-extraction warnings plus one placeholder row whenever count - bodyExtracted > 0. The placeholder's origin was the word aggregate in angle brackets, and its reason was "N callable(s) lowered to handler ref but produced no body". The block then printed issues.length as the number of callables. At the door (bin/run-dev.js build --strict-body, the reverse leg below), one refused handler printed --strict-body: 2 callable(s) lack a metadata body and four printed 5. --json issues carried the placeholder as an extra row beside the real ones.

Premise check: are missingBody and the warnings the same callables? Not always

Measured with a tsx probe that runs lowerCallables and the old arithmetic over nine shapes:

shape old header new header new rows
A. CONTROL: 1 clean hook 0 0 none
B. 1 refused hook 2 1 hook 'refused_1'
C. 4 refused hooks 5 4 the 4 hooks
D. 1 functions map entry only 1 (placeholder row only) 1 function 'score_lead'
E. 1 functions array entry + 1 refused hook 2 2 the hook, function 'arr_fn'
F. 1 hook with an authored body beside an inline handler 1 (placeholder row only) 1 hook 'clean_1'
G. same refused hook at top level AND in packages[0].manifest 3 1 the hook, once
H. same clean hook at both levels + 1 functions entry 0 (passed) 1 function 'score_lead'
I. same clean + refused hook at both levels 2 1 the refused hook, once
J. one handler shared by two clean hooks + 1 functions entry (one walk) 0 (passed) 1 function 'score_lead'

So the populations differ in three ways:

  1. A functions entry has no body slot. It is never extracted, so it leaves no warning, and only the placeholder row counted it (D, E).
  2. An inline function beside an authored body is never extracted either, so it also leaves no warning (F).
  3. A stack carrying the same callable at the top level and inside a packages[i].manifest body is walked twice. count dedups by function identity, while extraction runs once per walk. Each refusal is recorded twice and each lowered body is counted twice (G, I). That hid the functions entry in H, so the old gate passed it.

The fix

  • lowerCallables (packages/cli/src/utils/lower-callables.ts) returns bodylessCallables. It holds one row per registered callable that no slot lowered into a body, each once, keyed by ref, in registration order. This is exactly the population the placeholder row counted, itemized. A refused handler's row is the extractor's refusal, unchanged (origin, reason, kind, freeIdentifiers). A functions entry, or an inline function beside an authored body, gets an { origin, reason } row naming it (function 'name', hook 'name', action 'name').
  • compile.ts: issues = lowering.bodylessCallables. The header's wording is unchanged and its number is issues.length. The --json exit keeps the same error string and the same keys.
  • Landing point. The claim's file surface named compile.ts, its tests and the changeset. The producer of the count is lower-callables.ts in the same package, so the fix lands there (producer side). extract-hook-body.ts is untouched.

Why F stays a row. Two definitions were possible. One is "a slot runs it from the runtime module", under which F is not a row, since the hook binder prefers body. The other is "no slot lowered it", under which F is a row. The second is the placeholder's own population, and F's function is still bundled (needsBundle reads count - bodyExtracted, which this PR does not touch). Under it, every stack in which each callable is bound at one slot gets exactly the verdict it got before. There bodyExtracted counts each lowered callable once, so count - bodyExtracted is this list's length, and the old gate refused iff that exceeded 0, which is iff this list is non-empty. The first definition would have let --strict-body pass F while the build still emitted objectstack-runtime.*.mjs beside it, so it would have widened the accept set. Verdicts move only from pass to refuse, and only where a callable lowered at two or more slots masked a callable that was never lowered. That is H, the same callable at the top level and in a package body, reachable only through the two-level hand-written shape. It is also J, one function bound as the handler of two or more hooks or actions in a single walk, beside a functions entry or an authored-body slot. Both are narrowings.

Consumers of --json issues (measured, before the shape moved)

git grep over the tree at 1eff3224d (packages, scripts, .github, examples, packages/qa, content/docs):

  • The placeholder origin and its reason text: one hit, compile.ts:340 (the producer). No reader.
  • strict-body: missing body: build-json-failure-warnings.e2e.test.ts:437, which reads error and warnings, not issues. truncation-remainder-notices.test.ts:411 is a source-text pin on the payload literal, unchanged here. docs/qa/platform-checklist/areas/cli.json:1344 is a manual step that captures the payload and reads no row.
  • payload.issues reads: build-json-failure-warnings.e2e.test.ts:358 and :394, both on other exits (docs validation, capability preflight).
  • No --strict-body invocation in .github/workflows, scripts or examples.

No reader of the placeholder row, so it is removed outright and no summary field is added.

Docs

content/docs/automation/hook-bodies.mdx:173 ("objectstack build --strict-body fails on it") and :345 ("turn extraction warnings into a hard build failure (exit 1) with a per-callable diagnostic") were re-read, and both are still true. No other content/docs page describes --strict-body's output. No docs edit.

Tests (all at 11d4d8c6e, the head of this PR)

  • packages/cli/test/build-strict-body-count.test.ts (new, integration project: it spawns the CLI) pins the triage's door pins. CONTROL with 0 refused: exit 0 and no header. 1 refused: the header reads 1 and lists 1 hook; --json issues has 1 entry. 4 refused: the header reads 4 and lists 4 hooks; --json issues has 4 entries, each kind: 'free-identifiers'. A functions entry beside a refused hook gives issues two origins: the hook, then function 'sbc_score'.
  • packages/cli/test/lower-callables.test.ts (unit): 10 new cases over bodylessCallables. They cover the control, 1 and 4 refused, a refused action, functions entries (map, declared, array), an authored body beside an inline function (hook and action), a single walk whose list length equals count - bodyExtracted, a callable lowered at any slot, and the two double-walk cases. The divergence cases assert the old answer beside the new one.
vitest --project integration test/build-strict-body-count.test.ts   Tests 6 passed (6)          @ 11d4d8c6e
vitest --project unit (whole @objectstack/cli unit tier)             281 files, 4195 tests passed @ 11d4d8c6e
pnpm --filter @objectstack/cli build && typecheck                    exit 0 (tsc --noEmit + check:test-typecheck; both new test files are in tsconfig.test.json's program, by --listFilesOnly)

The integration tier is otherwise declared to CI. Locally only the new integration file was run, because the diff adds it.

Reverse verification (committed fix, ablation-replace WRAP mode, restore proven)

compile.ts loads from src/ in the spawned child (bin/run-dev.js via tsx; packages/cli/dist was absent for the first leg A run and for legs B-prime and C; the final leg A re-run ran with dist built and still went red, so the child loads src — corrected by the seat from the dev's report), and the unit file imports ../src. So no rebuild leg applies. Each mutation was proven on disk (anchor 1 to 0, blob changed), and each restore was proven (blob equals HEAD, git diff HEAD empty).

  • Leg A (compile.ts put back to warnings + placeholder) at 11d4d8c6e: the integration file reds 5 of 6 with Expected "1" Received "2" and Expected "4" Received "5", and the control stays green. Same result at 50fd36bbf.
  • Leg B′ (bodylessCallables becomes the raw warnings): the unit file reds 5 of 19 (the functions entry, authored body, single-walk equality and both double-walk cases).
  • Leg C (the lowered-at-any-slot filter dropped): the unit file reds 1 of 19.

Gates

node scripts/pm/dispatch-gates.mjs --commands re-derived from this diff gives 65 families, the 54 named at dispatch plus 11 changeset-family gates the diff adds. All 65 were run, each exit code captured before any pipe, and --ran reconciles: 65 derived, 65 run, 0 NOT-MEASURED, all codes recorded. check:dual-build-cjs-loads and check:i18n-coverage first exited 3 (PREREQUISITE NOT MET: no dist/ for packages outside the cli closure). After a full turbo run build they exit 0.

Lint (proven narrowing, not pnpm lint). First, ESLint's own API (isPathIgnored + calculateConfigForFile) says 4 of the 5 changed paths are linted; the changeset .md is not. Second, eslint --no-inline-config --format json on those 4 files at 11d4d8c6e lists 4 files, 0 errors and 0 warnings. Third, invariance: eslint.config.mjs never enables type-aware linting (its own comment says no parserOptions.project and no typed rules), has no cross-file import rules, and loads only two baseline JSONs that this diff does not touch. So this diff cannot move the verdict on any untouched file. The repo-wide pnpm lint is CI's.

Acceptance notes

  • Sibling defect, reported to the seat for filing (not fixed here). needsBundle in compile.ts still reads count - bodyExtracted. On a stack carrying the same hook at the top level and in packages[0].manifest plus a functions entry, objectstack build (default) printed Skipping legacy runtime bundle (all 2 callables are body-only), exited 0 and wrote no objectstack-runtime.*.mjs. The functions entry therefore ships with no implementation (measured on a scratch app). Fixing it changes which builds emit the bundle, so it is out of this card's scope.
  • composeStacks(…, { manifest: 'preserve' }) on main emits only manifest and packages; measured, its top-level keys are exactly those two. So the double walk is reachable only through a hand-written stack carrying both levels, the shape build-multi-package-artifact.e2e.test.ts builds. That file's comment says the shape is "the way composeStacks … writes it", which no longer holds.
  • Triage's grade wrote Clause-②: yes, and the claim wrote Clause-②: no (narrowing). This PR keeps the claim's line: it widens nothing (no single-walk verdict moves, no field is added) and narrows the --json issues shape.

Generated by Claude Code

…ble once

The header printed `issues.length` while `issues` was the extraction
warnings plus one `<aggregate>` placeholder row, so one refused handler
read 2 and four read 5, and `--json` `issues` carried a row naming no
callable. `lowerCallables` now returns `bodylessCallables`: one row per
callable that ships with no metadata body, keyed by ref, so `functions`
entries are named, authored bodies are not counted, and an ADR-0130 D4
package callable walked twice is counted once.

Claude-Session: https://claude.ai/code/session_01B5CHJNXuuqzChM4w6hkTN4
Co-authored-by: Claude <noreply@anthropic.com>
A callable is bodyless when no slot lowered it into a body: the
population the old placeholder row counted, itemized and deduped by
ref. An inline function beside an authored body stays a row (it still
rides the runtime module), so no single-walk `--strict-body` verdict
moves; only the count and the rows do.

Claude-Session: https://claude.ai/code/session_01B5CHJNXuuqzChM4w6hkTN4
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 11, 2026
@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 8 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 12b9daf7493f30ebb5faf47e7b40eb99d2bda267.

⛔ 5 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 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 28 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 12b9daf7493f30ebb5faf47e7b40eb99d2bda267 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 12b9daf7493f30ebb5faf47e7b40eb99d2bda267

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 11d4d8c6ee1ba9b802108064670d1362e5ce87bb
Local-runs: none

Inputs read: card #22839 (body and all three comments: triage 6109249693, claim 6109361423, os-dev-report 6109800087), PR #22849 (body, five-file list, net diff against main), the check-runs on the head (polled last at 2026-10-11T14:10:08Z), and origin/main (d48931871) for packages/cli/src/commands/compile.ts, packages/cli/src/utils/lower-callables.ts, packages/cli/package.json, src/index.ts, src/hook-body.ts, src/console.ts, the --json consumers under packages, scripts, .github/workflows, packages/qa, content/docs, docs/qa, and the changeset gates.

① Derived judgments

  1. Accept set of objectstack build --strict-body — a narrowing only; the code is right, the changeset's scoping of it is not (② item 4). Traced on the diff against the walk at origin/main: every registered ref ends in loweredRefs (some slot extracted it) or in unlowered (refused, an authored-body slot, or a functions entry), and bodylessCallables is unlowered minus loweredRefs. The old gate refused iff a warning existed or count − bodyExtracted exceeded 0. Refuse → pass cannot happen: a refused ref is never in loweredRefs (extractHookBody is a function of the handler's source alone; originLabel only words the message), and count above bodyExtracted leaves at least one ref outside loweredRefs, which is then a row. Pass → refuse happens exactly when no refusal exists and a never-tried callable is masked by a callable extracted more than once (bodyExtracted counts per slot, count per ref). Two reachable shapes: (a) the same callable at the top level and in a packages[i].manifest body (the PR's probe H, the changeset's third BREAKING bullet); (b) one function object bound as the handler of two or more bodyless hooks or actions in ONE walk, beside a functions entry or an authored-body slot — a handler shared by two objects plus a functions entry is ordinary authoring. For (b) the old arithmetic read count 2, bodyExtracted 2, no warning, so the gate passed (and, by the same arithmetic, needsBundle emitted no bundle, so the functions entry shipped with no implementation — the finding: objectstack build skips the runtime module ("all N callables are body-only") on a stack that carries a callable at the top level and in a packages[i].manifest, so a functions entry ships with no implementation, at exit 0 #22853 family, single-walk member); the new list carries one row naming the functions entry, so the build is refused, correctly. On every refusal the exit code (1) and the error string (strict-body: missing body) are byte-identical to before; the header's wording is unchanged and its number drops by the placeholder row (and by the duplicates on a double-walked stack). Verdict on the direction question: narrowing only — right.

  2. bodylessCallables population — right. It is exactly "each registered callable that no slot lowered into a body, once": unlowered is a Map keyed by ref and noteUnlowered keeps the first row, the filter drops every ref some slot lowered, and the order is registration order for every surviving row (a surviving ref was lowered nowhere, so its first slot both registered and noted it). A refused handler's row is the extractor's warning object itself (identity pinned in the unit file). The authored-body-beside-inline-function case belongs in the list: the old gate already refused it (count exceeds bodyExtracted by one for it), needsBundle (stillNeeded above 0) emits the runtime module for it and the bundle carries every registered ref, and the flag's stated purpose in compile.ts is to enforce body-only before bundle emission goes mandatory-off. The rejected definition ("a slot runs it from the runtime module") would have moved that stack refuse → pass while --no-runtime-bundle kept refusing it: a widening, and the dev was right to refuse it. Both reason constants state true facts about where the callable runs.

  3. Public surface — internal only; right. @objectstack/cli's exports map is ., ./console, ./hook-body and ./package.json. src/index.ts re-exports command classes only; src/hook-body.ts re-exports extractHookBody, HookBodyExtractionError and two types from extract-hook-body.ts; src/console.ts re-exports console utilities. lower-callables.ts is unreachable from any published entry, so bodylessCallables, BodylessCallable, FUNCTIONS_ENTRY_NO_BODY_REASON and AUTHORED_BODY_BESIDE_FUNCTION_REASON widen no published surface. LoweringResult gains one required field, additive for every in-tree reader (compile.ts, lint.ts, validate.ts, author-time-rules.ts, scaffold-validate.ts, and packages/qa/dogfood/test/build-shaped-artifact.ts, which imports by workspace-relative path, its own comment recording that the deep dist import was retired); no in-tree literal constructs a LoweringResult (the only other mentions are two comments). The four Type Check runs are green on this head. The changeset's "no spec key, export, type or stored shape moves" holds for the published surface.

  4. --json issues shape — right. The placeholder row leaves; a refused handler's row is unchanged (origin, reason, kind, freeIdentifiers); a never-tried callable gets { origin, reason }, the shape the placeholder already had, so no key is new on the payload (success, error, issues, warnings, conversions as before). Consumers re-measured by me on origin/main d48931871 (later than the dev's 1eff3224d), same result: the aggregate placeholder origin and its reason text occur only at the producer (compile.ts:340); strict-body: missing body is read by build-json-failure-warnings.e2e.test.ts:437 (error and warnings, not issues), anchored as source text by truncation-remainder-notices.test.ts:411 (the anchor survives in the new compile.ts), captured by a manual step in docs/qa/platform-checklist/areas/cli.json:1344 that reads no row, and quoted in CHANGELOG.md; every payload.issues read in the cli tests is on another exit; no --strict-body invocation exists in .github/workflows, scripts or examples. Removing the row outright with no summary field is the right call; the triage's "or a separate summary field if a consumer needs it" found no consumer.

  5. The rest of the Unchanged paragraph — right. The compile.ts hunk is the --strict-body block alone. The walk's outputs (lowered, functions, count, bodyExtracted, bodyExtractionWarnings, loweredHookRefs) are untouched; only bookkeeping is added. The default build, its 2c warning lines, bodyExtractionWarnings in the success payload and needsBundle do not move. Every source-text anchor in truncation-remainder-notices.test.ts (the header template, the list issues, the printBulletList call with limit: 20, the error: 'strict-body: missing body', issues, payload line, the two 2c precedent lines) is present in the new file, so that pin needed no edit.

  6. Pins — each can fail; right. Leg A (compile.ts back to warnings plus placeholder): the header reads 2 and 5 against the pinned 1 and 4, and the with-function case lists the hook then the aggregate placeholder origin instead of function 'sbc_score'; the control is a control and stays green by design. Leg B′ (list equals the raw warnings): the functions-entry cases, the authored-body case, the single-walk equality (3 rows pinned, 1 produced) and both ADR-0130 D4 cases (two rows for one refused hook; no row for the shared functions entry) go red. Leg C (the loweredRefs filter dropped): the hook-plus-functions-entry-sharing-one-function case produces a row where [] is pinned. The bullet regex matches printBulletList's two-space-bullet format, the refusal kind free-identifiers and freeIdentifiers: ['slugOf'] are what the extractor emits for a module-scope identifier, and both helpers (defineStackSourceFromLiteral, linkSpec, childEnv) exist on main. Tier: the new file spawns the CLI (child_process, run-dev.js, the tsx binary), so the behaviour predicate in vitest-tiers.ts puts it in the integration project, and its queue-tier name keeps it in the per-PR run; Test Core and all six shards are green on this head, so the door pins ran in CI. One gap, carried into ②: shape (b) of item 1 has no pin; the adjacent "lowered at ANY slot" case is the non-moving variant of it.

  7. Docs — right. hook-bodies.mdx:173 and :345 remain true (the flag still fails on a refused body, exit 1, per-callable diagnostic); "Flag docs affected by code changes" is green.

② Semver level

  • Changeset: @objectstack/cli: minor, conventional ! summary, a BREAKING banner, Clause-②: no (narrowing), one adr-0087 marker comment reading not-required (no-migration-prescription) with its why.
  • Clause-②: no (narrowing) — right. Clause ② is a new key on a published payload (check-changeset-no-major.mjs's own definition); this diff adds none, and the { origin, reason } row shape pre-existed as the placeholder's. The arm is (narrowing) because the accept set narrows (① item 1) and the issues list narrows (the placeholder leaves). The triage's Clause-②: yes was a grade shape, not a ruling, and would have been wrong as a reading (nothing is added); the claim corrected it before dispatch and the PR's third acceptance note says why. Per AGENTS.md the (narrowing) arm IS breaking, so the ! summary, the banner and the ADR-0087 marker are required and present; minor is the launch-window grade for a breaking change (major is refused by check-changeset-no-major). Consistent. "Check Changeset", the job that runs check-empty-changeset, check-adr-0087-registration and check-changeset-no-major on the merge base, is green on this head.
  • ADR-0087 not-required (no-migration-prescription) — consistent. No authorable key, published export or stored shape moves and no stored row is read or rewritten, so objectstack migrate meta has nothing to derive; the marker closes the other categories on facts (published package, no existing id, a build diagnostic rather than a runtime interface or type surface). The body's "one-line fix" sentence is a note for a script reading a CLI row, not a metadata FROM → TO prescription; the gate's detector did not read it as one and the gate is green.
  • The Unchanged paragraph's first sentence — WRONG, and this is the finding that decides the verdict. It says: for a stack that is walked once, --strict-body refuses exactly the stacks it refused before. ① item 1 shape (b) is a stack walked once that moves from pass to refuse: one function object as the handler of two or more bodyless hooks or actions, beside a functions entry or an authored-body slot. The third BREAKING bullet attributes the masking to the packages double walk alone, so the moved set is under-described by one reachable shape, in the text that ships to consumers as CHANGELOG.md. The PR body's "every stack walked once gets exactly the verdict it got before: the old gate refused iff count − bodyExtracted exceeds 0, which is iff this list is non-empty" carries the same over-claim (the "iff" fails in that shape), and the probe table has no row for it. The bodylessCallables docblock in the diff already states the true condition ("only when every callable is walked once" — walked at one slot), so the fix is a wording alignment, not a code change. Remedy: (1) reword the Unchanged sentence to the docblock's condition (a stack in which each callable is bound at one slot), (2) add shape (b) to the third BREAKING bullet or as a fourth, and (3) one unit case pinning shape (b): old count − bodyExtracted equal to 0, new list of one row naming the functions entry. Nothing else in the changeset or the code needs to move.
  • Everything else the changeset states (FROM → TO for the row and the header, the refusal's unchanged fields, exit code and error string, default build, warning lines, bodyExtractionWarnings, runtime-bundle decision) holds against the diff.

③ Boundary flags

Implemented-by: claude/issue-22839-strict-body-count
Reviewed-by: session_01B5CHJNXuuqzChM4w6hkTN4

VERDICT: FAIL


Generated by Claude Code

One function bound as the handler of two or more hooks or actions,
beside a `functions` entry or an authored-`body` slot, moves from pass
to refuse under `--strict-body` in a single walk too. The changeset's
Unchanged sentence now states the true condition (each callable bound
at one slot), the shape gets its own BREAKING bullet, and one unit case
pins it.

Claude-Session: https://claude.ai/code/session_01B5CHJNXuuqzChM4w6hkTN4
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: e583a93803453deba054fe3feaae417f2ef0e86d
Local-runs: none

Inputs read: card #22839 (body and all five comments: triage 6109249693, claim 6109361423, the three os-dev-reports 6109800087, 6110061229 and 6110082477), PR #22849 (body, five-file list, net diff against main, the docs-drift comment 6109788096, and the earlier ## Contract review record 6109936970, which judged the previous head 11d4d8c FAIL on its ② item 4), the check-runs on this head (polled last at 2026-10-11T14:42:32Z), and origin/main at its current tip 12b9daf for packages/cli/src/commands/compile.ts, packages/cli/src/utils/lower-callables.ts, packages/cli/src/utils/extract-hook-body.ts, packages/cli/package.json with its three entry modules, every --strict-body and payload.issues reader under packages, scripts, .github, examples, content/docs and docs/qa, and packages/cli/test/truncation-remainder-notices.test.ts. Re-review on a new head: read through the contents API, the blob shas of compile.ts, lower-callables.ts and build-strict-body-count.test.ts are identical at 11d4d8c and at this head, so the code the earlier record traced is the code judged here; what moved since is the changeset (two sentences and one bullet) and one unit case in lower-callables.test.ts.

① Derived judgments

  1. The three remedies the earlier record named — each met on this head. (1) The changeset's Unchanged sentence now reads "For a stack in which each callable is bound at one slot, --strict-body refuses exactly the stacks it refused before" — the condition under which it is true (item 3). (2) A fourth BREAKING bullet names shape (b): one function bound as the handler of two or more hooks or actions, beside a functions entry or a hook or action with an authored body, in a single walk, moving from pass to refuse, with the mechanism stated (the shared function is lowered at each of its slots, each slot counts as one lowered body, the function counts as one callable, the surplus cancelled the never-lowered callable, no warning, the old count read 0). Traced against the code: register dedups by function identity (refByFn), tryExtractBody increments bodyExtracted once per call, count is the size of the ref map, and noteUnlowered records the functions entry or the authored-body slot under its own ref, which no slot lowers, so that row survives the loweredRefs filter. The wording holds for hooks, for actions (lowerActionCallable registers and extracts under the same ref) and for a hook and an action sharing one function. With the shared function at k slots and m never-lowered callables beside it, the old number was m minus (k minus 1), so the pass-to-refuse move is the case the bullet describes (m at most k minus 1); with more never-lowered callables than the surplus, the old build already refused and only the header's number moves, which the second bullet covers. (3) The unit case "one walk: a handler shared by two hooks no longer hides a functions entry from the count" builds two bodyless hooks on one function object beside functions: { score_lead }, asserts the old reading (count - bodyExtracted is 0, no warning) and the new list as an exact deep equality of one row naming function 'score_lead' with FUNCTIONS_ENTRY_NO_BODY_REASON. It can fail: against the old arithmetic (the warnings plus a placeholder only when count - bodyExtracted is above 0) the list is empty and the equality reds — the dev's leg D measured exactly that (9 of 20 red, this case among them) — and a duplicated or mis-named row reds it too. With the loweredRefs filter dropped it stays green, because the shared ref is never noted unlowered, so this pin and the "lowered at ANY slot" pin discriminate different mutations, as the dev reported. It is a unit-project file importing ../src; of the six Test Core shards, four are green and two pending at my last poll (③).

  2. Accept set of objectstack build --strict-body — a narrowing only, now described in full. Re-traced on this head's lower-callables.ts: every registered ref ends in loweredRefs (some slot extracted it) or in unlowered (refused, an authored-body slot, or a functions entry), and bodylessCallables is unlowered minus loweredRefs, in first-registration order. Refuse-to-pass cannot happen: extractHookBody's verdict is a function of the handler's own source — originLabel only words the message, read at extract-hook-body.ts lines 432 to 471 — so a refused function is refused at every slot and never enters loweredRefs; and count above bodyExtracted leaves at least one ref outside loweredRefs, which is then a row. Pass-to-refuse happens exactly when no refusal exists and a never-lowered callable coexists with a callable extracted at two or more slots: across the ADR-0130 D4 double walk (the third BREAKING bullet, probe H) or within one walk through a shared handler (the fourth, probe J). No other shape raises bodyExtracted above the number of lowered refs. On every refusal the exit code (1) and the error string are byte-identical to before; the header's wording is unchanged and its number is the row count. Right.

  3. The Unchanged sentence — right. When each callable is bound at one slot, each ref is registered once and either extracted once (one increment of bodyExtracted, one entry in loweredRefs) or noted once in unlowered, so count - bodyExtracted equals the list's length and the extraction warnings are a subset of the list; the old gate refused iff a warning existed or that difference was above 0, which is iff the list is non-empty. The sentence is a sufficient condition, not a necessary one: a function bound as a lowered hook handler and again as a functions entry is bound at two slots and does not move (the "lowered at ANY slot" pin), which the sentence does not claim either way.

  4. The reworded population sentence — right in substance, flat at one edge; not a finding. "It is the population the extra row's number was meant to count, itemized: in the two shapes below that number undercounted it." The number, count - bodyExtracted, can never exceed the list's length, since bodyExtracted is at least the size of loweredRefs; it falls short exactly when a callable is extracted at two or more slots, which is the two shapes the third and fourth bullets describe. In the sub-case of the third bullet where every double-walked callable is refused (probe G), the number was exact (1) and only the list (the refusal twice) and the header (3) were inflated — the bullet itself names that sub-case. So "undercounted" names the only direction the number ever errs in and the shapes in which it errs; "could undercount" would be exact. Nothing in it misstates a verdict, a row or a field.

  5. Public surface — internal only; right. @objectstack/cli's exports map on the current tip is ., ./console, ./hook-body and ./package.json; src/index.ts re-exports command classes, src/hook-body.ts the extractor's two values and two types, src/console.ts three console helpers; none reaches lower-callables.ts, so bodylessCallables, BodylessCallable and the two reason constants widen no published surface. LoweringResult gains one required field; its only in-tree constructor is lowerCallables itself (every other mention is a comment), and TypeScript Type Check with its four runs and Build Core are green on this head. The changeset's "no spec key, export, type or stored shape moves" holds for the published surface.

  6. --json issues shape and its consumers — right, re-measured on the current tip. The placeholder origin and reason text occur only at the producer (compile.ts:340 on main). Every test that passes --strict-body: build-json-failure-warnings.e2e.test.ts:434 reads error and warnings; build-json-failure-conversions.e2e.test.ts:316 (not named in the PR body's census) reads error, warnings and conversions; hook-body-build-reach.e2e.test.ts:253 reads the text face for the require() reason and __require, which is the refused row's own reason, unchanged; truncation-remainder-notices.test.ts:411 is a source-text anchor on the payload line, which survives. Every payload.issues read in the cli and qa tests is on another exit (docs rules, capability preflight, expression rules, object references, the lint and jsx notices). No --strict-body invocation in .github, scripts or examples; docs/qa/platform-checklist/areas/cli.json:1344 captures the payload and reads no row. Removing the row outright with no summary field is right; the refused row keeps origin, reason, kind and freeIdentifiers, the new rows are { origin, reason }, the shape the placeholder already had, and the payload's keys are the same five.

  7. The rest of the Unchanged paragraph — right. The compile.ts hunk is the --strict-body block alone; the 2c warning lines, bodyExtractionWarnings in the success payload (compile.ts:1156 on main) and needsBundle (:1022 to :1023, still count - bodyExtracted) do not move. The five source-text anchors in truncation-remainder-notices.test.ts (the header template, the list issues, the printBulletList call with limit: 20, the payload line and the lowering.bodyExtractionWarnings slice) are all present in the new file.

  8. Door pins — unchanged file, unchanged judgment. build-strict-body-count.test.ts is byte-identical to the previous head; the earlier record's leg analysis stands (header 2 and 5 against the pinned 1 and 4, the aggregate placeholder origin in place of function 'sbc_score', the control green by design). Dogfood Verify CLI is green on this head.

  9. Docs — right. hook-bodies.mdx:173 and :345 remain true; "Flag docs affected by code changes" is green.

② Semver level

  • Changeset: @objectstack/cli: minor, conventional ! summary, a BREAKING banner with four bullets, Clause-②: no (narrowing), one adr-0087 marker reading not-required (no-migration-prescription) with its why. The PR body carries the same Clause-②: no (narrowing) line. "Check Changeset" (check-empty-changeset, check-adr-0087-registration, check-changeset-no-major on the merge base) is green on this head.
  • Clause-②: no (narrowing) — right. Clause ② is a new key on a published payload (the gate's own definition); this diff adds none — the { origin, reason } row shape pre-existed as the placeholder's and the payload's keys are unchanged. The (narrowing) arm is right because the accept set narrows (① item 2) and the issues list narrows. Per AGENTS.md the (narrowing) arm is breaking, so the ! summary, the banner and the marker are required and present; minor is the launch-window grade for a breaking change. Consistent. The triage's yes was a grade shape the claim corrected before dispatch; the PR's third acceptance note says why.
  • ADR-0087 not-required (no-migration-prescription) — consistent. No authorable key, published export or stored shape moves and no stored row is read or rewritten, so objectstack migrate meta has nothing to derive; the marker closes the other categories on facts. The "one-line fix" sentence is a note for a script reading a CLI row, not a metadata FROM → TO prescription.
  • Every sentence re-checked against the diff — each holds. The first paragraph (what was printed, 2 and 5, the extra row and its reason, the header counting it); the second (one row per never-lowered callable, each once; the population sentence per ① item 4; the header is the row count); BREAKING bullets one to four (the FROM → TO for issues, the refused row's four unchanged fields, the two new row kinds and their origins, the header, the double walk with its duplicated refusal and double-counted body and the hidden functions entry, the shared handler in one walk); the one-line fix; the Unchanged paragraph (① items 3, 5 and 7); the marker. The earlier record's ② finding is closed: the moved set is described in full, and the sentence that over-claimed is scoped to the condition under which it is true.

③ Boundary flags

  • open_questions: none in the three reports; none found.
  • Round-0 deviations (landing point in lower-callables.ts, pins in two new files, heavy runs through the lock, the gate record's re-run exits, model-free commit trailers, "no role-file conflict"): as the earlier record judged them; nothing moved.
  • Round-1 deviation "only leg D can red shape (b); leg C was run as well": right on the mechanics (① item 1), both outcomes reported. "Worktree recreated on the existing branch; origin/main not merged in": the merge base stays 1eff322; GitHub reads the PR mergeable, and its blocked state is the draft flag plus pending checks, not a conflict. "Stray /r2-unit.pid at the dev container's filesystem root": outside the repo; the diff is the five files and nothing else. Round 1b: changeset prose only.
  • PR body: the "Why F stays a row" paragraph carries the two replacement sentences the dev supplied and the probe table has row J; both are right against the diff (① items 2 and 3). Nits left unapplied from the dev's optional list, none of which ships to consumers: "over nine shapes" above a ten-row table; "three ways" above a four-bullet moved set; the Tests section's "10 new cases" where the unit file carries 11, stamped "all at 11d4d8c" though the eleventh landed later; the reverse-verification section lacks round 1's legs D and C. The seat may apply the dev's replacements; no verdict effect.
  • Code-comment nit: the bodylessCallables docblock's two sub-bullets name the no-warning callables and the double walk, not the one-walk shared handler; the changeset's fourth bullet and the unit case carry that shape. No contract effect.
  • out_of_scope_findings: the needsBundle double-walk defect is finding: objectstack build skips the runtime module ("all N callables are body-only") on a stack that carries a callable at the top level and in a packages[i].manifest, so a functions entry ships with no implementation, at exit 0 #22853; the earlier record's escalation (shape (b) is that family's single-walk member) is the seat's to carry onto that card, as the round-1 report says; the stale comment in build-multi-package-artifact.e2e.test.ts is comment-only.
  • Acceptance note 3 (triage yes versus claim no (narrowing)): ruled in ②; the PR's line stands.
  • Check-runs on the head, latest run per name, at 2026-10-11T14:42:32Z: completed with success — Build Core, TypeScript Type Check and its four Type Check runs, Test Core (1/6), (2/6), (5/6) and (6/6), Dogfood Regression Gate and its three shards, Dogfood Verify CLI, Temporal Conformance (live PG + MySQL), Check Changeset, Governed Surface Queue Guard, the claim, single-writer and part-of guards, Check Documentation Links, Check PR Size, Flag docs affected by code changes, Auto Label, filter; skipped — Build Docs, Console Pin Gate, Packed-tarball smoke; no completed run concluded other than success or skipped. Still in_progress, read as pending and never as green: Lint & Repo Gates, Test Core (3/6) and Test Core (4/6) — two of the seven required contexts are among them, and Lint & Repo Gates carries the repo-wide pnpm lint and every check:* gate, which the dev's lint-narrowing argument covers for the four linted files only. The PASS below is the contract verdict on this head; it lands nothing by itself, since the Tier S landing rule needs every check green beside the record, and a red among those runs is read afresh, not argued around.
  • No maintainer ruling on the card; this is the first record naming this head. It supersedes 6109936970 for the PR's current head only; that record stays the verdict on 11d4d8c.

Implemented-by: claude/issue-22839-strict-body-count
Reviewed-by: session_01B5CHJNXuuqzChM4w6hkTN4

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 11, 2026 14:50
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 11, 2026 14:50
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 11, 2026
Merged via the queue into main with commit 6364375 Oct 11, 2026
36 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-22839-strict-body-count branch October 11, 2026 15:18
marchtian pushed a commit that referenced this pull request Oct 11, 2026
Resolves the four lower-callables.ts hunks PR #22849 shares with this
branch by carrying both parameters, ref then face: tryExtractBody's
signature, the hook call site, lowerActionCallable's tryExtract type, and
the action call site. The bodylessCallables / noteUnlowered logic is
main's, byte for byte.

Claude-Session: https://claude.ai/code/session_01B5CHJNXuuqzChM4w6hkTN4
Co-authored-by: Claude <noreply@anthropic.com>
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