Uh oh!
There was an error while loading. Please reload this page.
feat(cli): make the silent hook-body downgrade loud — os lint refuses an accidental scope leak (ask 1) - #13834
Conversation
An L2 hook handler that reaches out of the sandbox's scope is refused by `extractHookBody`; `lowerCallables` caught the refusal, recorded it, and bundled the closure anyway at exit 0. The deployment shape changed from metadata to bundle with nothing red. The refusal now carries the classification the refusing rule already had (`HookBodyExtractionError` / `HookBodyRefusalKind`), and `lowerCallables` carries it plus the free-identifier list on each warning. `os lint` reads the kind and splits the accidental class (an `error`, so a gate can fail on it) from the structural one (a `warning`, because bundling is its designed answer). The catch stays: what `os build` accepts is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UngCYXF98BVpYA9hfz6NYk
…ok-body-lowering-loud
The package's `tsconfig.json` says `include: ["src"]`, so these two new test files ARE in the package's own tsc program — and they carried 3 TS7053 (indexing a typed object with an `any` key). Measured, and fully attributed: removing exactly these two files returned TEST_DEBT['@objectstack/cli'] to its recorded 144 with no other entry moving, so the +3 was entirely theirs. Fixed at source. ⛔ The ledger entry is not raised — it is shrink-only and maintainer-only by the gate's own text. `Record<string, …>` changes nothing about the fixtures being module-scope free identifiers, which is what they are in the tests for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UngCYXF98BVpYA9hfz6NYk
…ok-body-lowering-loud
📓 Docs Drift CheckThis PR changes 1 package(s): 20 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin eaafd10ea0286503974721290172055cee499e6d && git checkout eaafd10ea0286503974721290172055cee499e6d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 3b2a3f640c7ed41a5c4851b9f3bf24b7c42158f3 f46ff142523b010756a6add78e249bfe9dc22b7e && git checkout -B drift-repro 3b2a3f640c7ed41a5c4851b9f3bf24b7c42158f3 && git merge --no-ff f46ff142523b010756a6add78e249bfe9dc22b7e
node scripts/docs-audit/affected-docs.mjs --json 3b2a3f640c7ed41a5c4851b9f3bf24b7c42158f3
|
os-warren
commented
Aug 31, 2026
Contract review (Clause ②) — REWORKReviewed at head Carrier action
What to change — three items, all small, none architectural
The substance is confirmed sound: the content limb is real and honestly self-declared, Generated by Claude Code |
os-steve
commented
Aug 31, 2026
At-tier contract review (clause ②) — REQUEST CHANGESI am the at-tier contract reviewer for this PR. Standing verified first, by symbol: Verdict: REQUEST CHANGES. The contract substance is sound on every axis I judged — the reject-set move is justified at What I measured (own instrument, base |
…s lint (#13834 review) Contract-review items 1 and 2 on PR #13834: - judge() no longer folds `unparseable`/`unknown` into the hook-body/bundled-fallback arm, whose prose asserts the author chose a bundled closure. An instrument limit (unparseable) and an instrument failure (unknown) now report under their own rules — hook-body/unparseable and hook-body/extraction-failed — as warnings whose prose names the instrument, not the author. Severities are unchanged (warning), so the lint exit contract does not move. - Parity pins now cover all four kinds: the rule->kind map gains rows for unparseable and unknown, the parity fixture population includes both, and three unit pins assert the instrument kinds never borrow the designed-fallback prose. - Changeset: the false 'new exports' sentence is replaced by the real published carriers — os lint's exit contract (0 -> 1) and the kind / freeIdentifiers fields on os build --json bodyExtractionWarnings. Grade stays minor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UngCYXF98BVpYA9hfz6NYk
…ok-body-lowering-loud
os-steve
commented
Sep 1, 2026
At-tier contract review, follow-up — APPROVE; |
os-steve
commented
Sep 1, 2026
Park LIFTED by maintainer ruling. Contract review cleared on both carriers. Proceeding to ready + arm.
The rulingMaintainer, 2026-09-01, live PM chat, verbatim option: 「合并,类别另立决策卡」 — merge this PR now, and file the clause-② class question as its own decision card. I put the question to the maintainer rather than lifting my own hold by argument. My park's exit predicate was written as "CI completes, and the maintainer answers this card's own clause-② question" (PM review on #13651), and quietly reasoning my way past a predicate I had published would have been a seat clearing its own gate. The predicate is now satisfied as written. ⇒ The class question is filed as #14047 ( The contract-review hold, discharged separately and earlier
Tier verified before adoption, with a discriminating control — the reviewer cannot self-certify, since its
⇒ The probe returns a different value on the control, so it is a reading rather than a tautology. What the follow-up review established, on the new head
|
Fixes#13651
Scope is ask 1 only (fail loudly). The constant-surface half is #13718 and is not addressed here — no half of it is stubbed in, out of scope: #13718 remains open.
Clause-②: yes
This changes what
os lintrejects: a config whose hook handler leaks module scope now produces a linterror, soos lintexits 1 where it previously exited 0.It does not change what
os buildaccepts. That was a deliberate choice, not an omission — see the measurement below. The catch inlowerCallablesstays, both refusal classes still fall back to bundling, and the build still exits 0; the threee2esuites that spawn a realos buildpass unchanged.PR stays draft. Not armed, not queued, not flipped ready.
The measured population — the reading #13718 needs to size itself
Measured on the real build path, not by re-implementing it:
loadConfig(bundle-require/esbuild, exactly whatos builduses) thennormalizeStackInputthenlowerCallables, reading the reason strings the realextractHookBodyproduced.objectstack-ai/hotcrm(the reference app,aff9cb2f)examples/app-showcaseexamples/app-todoexamples/app-crmplugin-auth,plugin-security,service-i18ncreate-objectstackblank templateThe existing population on the silent-downgrade path is zero. Nothing in this repo or in the reference app currently takes it.
Two things that number does not mean:
tryExtractBodyswallowsextractHookBody's refusal into a warnings array and bundles the closure — make it loud (ask 1; the constant-surface half is split out) #13651 documents, and it is what Give a lowered hook body a declared constant surface — every shared constant is currently hand-copied into each handler and pinned by a bespoke drift test (8 rows in the reference app) #13718 is sizing.os build. Zero here says nothing about apps outside this tree, andos buildis a published CLI.The 3 callables that produced no body and no warning are not this class: they are top-level
functions:map entries (app-showcase2,app-todo1), a path that never attempts extraction by design.Positive control (a zero-hit is not a reading without one). The same instrument over a fixture with three known-outcome hooks reported exactly:
hook_freetofree-identifiers,hook_forbiddentoforbidden-token,hook_cleanextracted. The instrument yields positives in both classes, so its zeros are real.Which gate shape, and why
The card offered two. Chosen: the
lintdiagnostic a gate can fail on — the card's own steadier intermediate — made class-aware.Against making
os buildfail outright: the population is zero, so that option reddens nothing today, but it changes the accept set of a published CLI for apps that cannot be measured from here, and it would take the legitimate fallback down with the accidental case.compile.tsstep 2c already records that position in-tree.Why
os lint's own rubric rather than the shared authoring-rule registry: the registry'sgatingtier must be run by all three commands (so it would move the build's accept set), and itsadvisorytier can never emit anerror.lintConfigis the only tier whoseerrorfailsos lintalone — its own comment calls it "a lint verdict, not a publish gate".Telling an author's mistake from a deliberate bundle
The refusal already knew which rule refused; the throw flattened it into prose and the catch kept only the sentence. It now carries
kind:free-identifiers) — the handler is expressible as a metadata body; it merely names a module-scope const, helper or import. The deployment shape changed against what the author wrote. Gives aerror, rulehook-body/not-lowerable.forbidden-token) —fetch/require/process/evalare capabilities the sandbox does not have, so writing one is choosing a bundled closure and the bundle is the designed answer. Gives awarning, rulehook-body/bundled-fallback.unparseablegives a warning.unknown(a non-HookBodyExtractionErrorthrow) is kept distinct fromunparseableon purpose: an instrument failure must not read as a verdict about the author.An author who deliberately wants a bundled closure keeps two channels that already existed, are already honoured, and stay silent — neither needs a new spec key: give the hook an explicit
body, or move the function into the top-levelfunctions:map and reference it by name. So the inline-function form means "I intend this to be a hook body" and the named-functions:form means "I intend this to be bundled code" — a distinction the authoring surface already had and nothing was reading.Parity by construction: the rule calls the same
extractHookBodythe build calls, so the lint verdict cannot drift from what the build would do to the same handler. Pinned by a test that asserts the two see exactly the same callables.Ablation
Direction predicted in writing before running: mutating the
kind === 'free-identifiers'branch so the rule stops consulting the classification should turn the accidental class into a warning, reddening the error-severity and parity pins, while the refusal-kind suite stays green.Observed: mutation proven on disk (blob
b47714f4tof71b0286, anchor count 1 to 0, injected 0 to 1); 3 tests failed inhook-body-lowering.test.ts— the two predicted plus the action-path pin, which is the same accidental-class assertion applied to actions, so wider in extent than predicted, same direction;hook-body-refusal-kind.test.tsstayed green as predicted. Restore proven three ways: blob hash equal to the HEAD blob, emptygit diff HEAD, cleangit status --porcelain.No rebuild leg was needed, and the ablation itself is the proof: mutating
src/changed the outcome with no build, so these tests read source, notdist.Verification
Gate union derived with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commandsafter mergingorigin/mainand after the last edit, harvested with--commands(never grepped), each exit code captured before any pipe.comm -23andcomm -13are both empty (exact comparison, no arithmetic on a hand-kept counter). 35 path-derived plus convention-triggered families, pluspnpm lint, which the derivation never names.pnpm lint(eslint . --no-inline-config): run repo-wide, exit 0. No narrowing to declare.pnpm --filter @objectstack/cli typecheck: exit 0.e2esuites that spawn a realos build.59e77df0d7.Two families exited non-zero during the sweep; both are resolved:
node scripts/check-test-completeness.mjs— exit 3, NOT MEASURED, in the gate's own words: "PREREQUISITE NOT MET — this gate grades a savedturbo run testlog, and no log was named. … the local reading for this gate is NOT MEASURED. It is not a red, and there is nothing here to fix."pnpm check:type-check-debt— exit 1, a real red that is fixed at source.TEST_DEBT['@objectstack/cli']recorded 144,tsc --noEmitreported 147 (+3).packages/cli/tsconfig.jsonsaysinclude: ["src"], so the two new test files sit inside the package program and carried 3 TS7053. Attributed mechanically: removing exactly those two files returned the ledger to green with no other entry moving, so the +3 was entirely theirs. Fixed by typing the two fixture maps. The entry was not raised — shrink-only and maintainer-only by the gate's own text. Re-run on the merged head: exit 0, "29 ledger entr(ies) re-measured … none above its recorded number."Changeset
@objectstack/cli: minor. What this publishes rides on two CLI surfaces, not on new API exports — corrected in the contract-review patch round:HookBodyExtractionError/HookBodyRefusalKind/BodyExtractionWarningare module-internal (the packageexportsmap exposes only.and./console, andsrc/index.tsre-exports none of them). The published carriers are theos lintexit contract (a newerrorrule, soos lintcan newly exit 1 where it exited 0) and thekind/freeIdentifiersfields onos build --json'sbodyExtractionWarnings. Nothing removed,os build's accept set untouched, so not major. Precedent: the launch-window narrowing-as-minorconvention stated in.changeset/form-view-option-default-narrowed.md; the earlier citation (cli-i18n-flow-screen-bucket) is not on point — its diagnostics error only under--i18n-strict.Patch round — at-tier contract review (2026-09-01, head
f46ff14252)Acting on the REQUEST CHANGES review (items 1 and 2; item 3 — the verbatim
Clause-②: yesline on the #13651 claim comment — is the PM seat's, not addressed here). Both premises re-derived against the tree before editing:packages/cli/src/index.tsnames none of the three types (grep exit 1, noexport *), the packageexportsmap is.and./consoleonly; andjudge()'s catch branched only onfree-identifiers, sounparseable/unknownfell into thebundled-fallbackarm whose message asserts "This is the designed fallback — the body uses something the sandbox cannot provide.".changeset/cli-hook-body-lowering-loud.md): the "new exports" sentence is replaced by the real published carriers — theos lintexit contract (0 → 1) and thekind/freeIdentifiersfields onos build --json'sbodyExtractionWarnings. Grade unchanged:minor.judge()keeps instrument failures distinct from author verdicts:unparseableandunknownno longer borrow thebundled-fallbackprose; they report under their own rules —hook-body/unparseableandhook-body/extraction-failed— as warnings whose prose says "not a verdict about the handler". Severities unchanged (warning), so this patch moves no exit contract. The parity pin's rule→kind map is now total over all four kinds and its fixture population includes both instrument kinds, so build and lint are pinned to label them identically; three unit pins additionally forbid the instrument kinds from ever reading as a chosen bundle.009c3960→d0b07871, unique anchor "the extraction instrument itself failed" count 1 → 0); restore proven (blob hash equal, emptygit diff HEAD, cleangit status --porcelain); trap in the same process as the measurement.Verification on the merged head
f46ff14252: gate union re-derived post-merge (the pre-merge derivation self-reported STALE TREE), named 36 (35 derived +pnpm lint, which the derivation never names), ran 36, unreconciled 0 —comm -23andcomm -13both empty, exact string comparison, every exit code captured before any pipe. 31 gates green first pass; 4 families (check:dual-build-cjs-loads,check:i18n,check:i18n-coverage,check:type-check-debt) first reported PREREQUISITE NOT MET (no builtdist/— NOT MEASURED, not treated as pass or red), so the closure was built exactly as each gate's text instructs (turbo run build --filter='./packages/*' --filter='./packages/*/*', 70/70) and all 4 re-ran exit 0, measured — type-check-debt: "29 ledger entr(ies) re-measured … none above its recorded number" (the new pins add zero debt; the ledger was not touched).check-test-completenessexit 3 stays NOT MEASURED in the gate's own words ("this gate grades a savedturbo run testlog … running the family locally, record this gate as NOT MEASURED"); the in-lieu reading: full@objectstack/clivitest suite on the built closure — 223 files, 2553 tests, 0 failures, exit 0. PR stays draft; labels untouched.Generated by Claude Code