From 953091937ffc300f7c6146c7a868ae9888c7128c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 13:20:56 +0000 Subject: [PATCH 1/3] feat(cli): make the silent hook-body downgrade a lint verdict 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 Claude-Session: https://claude.ai/code/session_01UngCYXF98BVpYA9hfz6NYk --- .changeset/cli-hook-body-lowering-loud.md | 45 ++++ packages/cli/src/commands/lint.ts | 14 ++ .../cli/src/lint/hook-body-lowering.test.ts | 215 ++++++++++++++++++ packages/cli/src/lint/hook-body-lowering.ts | 211 +++++++++++++++++ packages/cli/src/utils/extract-hook-body.ts | 85 ++++++- .../src/utils/hook-body-refusal-kind.test.ts | 116 ++++++++++ packages/cli/src/utils/lower-callables.ts | 43 +++- 7 files changed, 721 insertions(+), 8 deletions(-) create mode 100644 .changeset/cli-hook-body-lowering-loud.md create mode 100644 packages/cli/src/lint/hook-body-lowering.test.ts create mode 100644 packages/cli/src/lint/hook-body-lowering.ts create mode 100644 packages/cli/src/utils/hook-body-refusal-kind.test.ts diff --git a/.changeset/cli-hook-body-lowering-loud.md b/.changeset/cli-hook-body-lowering-loud.md new file mode 100644 index 0000000000..c8d570c03b --- /dev/null +++ b/.changeset/cli-hook-body-lowering-loud.md @@ -0,0 +1,45 @@ +--- +'@objectstack/cli': minor +--- + +feat(cli): `os lint` refuses a hook body that silently stopped being metadata (#13651) + +An L2 hook handler is lowered to a metadata-only `body.source` and evaluated in +QuickJS with no module scope. When the handler reaches out of that scope, +`extractHookBody` refuses — and `lowerCallables` caught the refusal, recorded it, +and shipped the callable through the back-compat `.mjs` bundle instead. `os build` +exited 0. The hook kept working locally. What changed silently was the +**deployment shape**: the app had stopped being shippable as pure metadata. + +The refusal was never the missing piece — `extractHookBody` had already computed +the exact free-identifier list. What was missing is that nothing said no to the +recorded array. + +**The refusal now carries its classification.** `HookBodyExtractionError` (new +export, with `HookBodyRefusalKind`) names which rule refused — +`free-identifiers` / `forbidden-token` / `unparseable` — and `lowerCallables` +carries it plus the identifier list on each `bodyExtractionWarnings` entry +(also new in `os build --json`). That classification was computed at the throw +and flattened into a message string; it is now structure a consumer can act on. + +**`os lint` is the first consumer, and it tells the two classes apart.** They +used to share one catch, so they shared one fate: + +- **accidental** (`free-identifiers`) — the handler *is* expressible as a + metadata body; it merely names a module-scope const, helper or import. Now a + lint **`error`**, so a gate can fail on it. `os lint` exits 1. +- **structural** (`forbidden-token`) — `fetch`/`require`/`process`/… are + capabilities the sandbox does not have, so writing one *is* choosing a bundled + closure and the bundle is the designed answer. Reported as a **`warning`**, + never fatal. + +An author who deliberately wants a bundled closure keeps two channels that +already existed and are still silent: give the hook an explicit `body`, or move +the function into the top-level `functions:` map and reference it by name. + +**What did NOT change: what `os build` accepts.** The catch in `lowerCallables` +stays, both classes still fall back to bundling, and the build still exits 0 — +verified over a real spawned `os build`. Flipping that default is a separate +contract decision. The new rule runs the *same* `extractHookBody` the build +runs, so the lint verdict cannot drift from what the build would do to the same +handler. diff --git a/packages/cli/src/commands/lint.ts b/packages/cli/src/commands/lint.ts index 74fddc7657..9a8bd99ecc 100644 --- a/packages/cli/src/commands/lint.ts +++ b/packages/cli/src/commands/lint.ts @@ -11,6 +11,7 @@ import { lintDataModel, runAuthoringRules } from '@objectstack/lint'; import { resolveSduiManifest } from '../utils/sdui-manifest.js'; import { collectAndLintDocs } from '../utils/collect-docs.js'; import { scoreMetadata } from '../lint/score.js'; +import { checkHookBodyLowering } from '../lint/hook-body-lowering.js'; import { runMetadataEval } from '../lint/metadata-eval.js'; import { DEFAULT_METADATA_EVAL_CORPUS } from '../lint/corpus.js'; import { @@ -400,6 +401,19 @@ export function lintConfig(config: any, opts: LintConfigOptions = {}): LintIssue } } + // ── Hook/action bodies that cannot be lowered to metadata (#13651) ── + // `os build` catches every extraction refusal, warns, and bundles the closure + // at exit 0 — so an app can stop being shippable as pure metadata with nothing + // red anywhere. This rule is the "no" to that recorded array. It runs the SAME + // `extractHookBody` the build runs, so the two cannot disagree, and it splits + // the accidental class (an `error`, which a gate can fail on) from the + // structural one (a `warning`, because bundling is its designed answer). It + // does NOT move what `os build` accepts; see the rule module's header. + // + // Reads FUNCTION values, so it must run on the normalized input before any + // Zod parse — which is where `lintConfig` already sits. + issues.push(...checkHookBodyLowering(config as Record)); + // ── Data-model best practices (relationships / master-detail / roll-ups) ── // Cross-object rules that encode the conventions in ADR-0035 and the // objectstack-data/-ui skills. These double as the eval rubric (see score.ts). diff --git a/packages/cli/src/lint/hook-body-lowering.test.ts b/packages/cli/src/lint/hook-body-lowering.test.ts new file mode 100644 index 0000000000..cb8efdda4d --- /dev/null +++ b/packages/cli/src/lint/hook-body-lowering.test.ts @@ -0,0 +1,215 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #13651 — the silent downgrade becomes a lint verdict. + * + * The pins here are about the DISTINCTION, not just the noise: an accidental + * scope leak must be an `error` (so a gate can fail on it) while the structural + * refusal must stay a `warning` (so the legitimate fallback-to-bundling path is + * not punished). A test that only asserted "something was reported" would pass + * on the change this card explicitly forbids — deleting the catch. + */ + +import { describe, it, expect } from 'vitest'; +import { + checkHookBodyLowering, + NOT_LOWERABLE_RULE, + BUNDLED_FALLBACK_RULE, +} from './hook-body-lowering.js'; +import { lowerCallables } from '../utils/lower-callables.js'; + +// Module scope — exactly what a lowered body cannot reach. +const SLA_MATRIX = { high: 4, low: 48 }; + +const freeIdentifierHook = { + name: 'case_sla', + object: 'case', + events: ['beforeInsert'], + handler: (ctx: any) => { + ctx.input.sla_hours = SLA_MATRIX[ctx.input.priority]; + }, +}; + +const forbiddenTokenHook = { + name: 'enrich_lead', + object: 'lead', + events: ['beforeInsert'], + handler: async (ctx: any) => { + const res = await fetch('https://example.invalid/enrich'); + ctx.input.score = res.status; + }, +}; + +const selfContainedHook = { + name: 'normalize_name', + object: 'account', + events: ['beforeInsert'], + handler: (ctx: any) => { + ctx.input.name = String(ctx.input.name).trim(); + }, +}; + +describe('checkHookBodyLowering', () => { + it('reports an accidental scope leak as an ERROR a gate can fail on', () => { + const issues = checkHookBodyLowering({ hooks: [freeIdentifierHook] }); + + expect(issues).toHaveLength(1); + expect(issues[0].severity).toBe('error'); + expect(issues[0].rule).toBe(NOT_LOWERABLE_RULE); + expect(issues[0].path).toBe('hooks[0].handler'); + // Names the callable and the identifier that caused it — the diagnostic + // `extractHookBody` had already computed and the build dropped into a log. + expect(issues[0].message).toContain("hook 'case_sla'"); + expect(issues[0].message).toContain('SLA_MATRIX'); + // Says what actually changed. "no behavior change" is true of behaviour and + // false of deployment shape, which is the whole defect. + expect(issues[0].message).toContain('deployment shape'); + }); + + it('keeps the STRUCTURAL refusal a warning — the bundle is its designed answer', () => { + const issues = checkHookBodyLowering({ hooks: [forbiddenTokenHook] }); + + expect(issues).toHaveLength(1); + expect(issues[0].severity).toBe('warning'); + expect(issues[0].rule).toBe(BUNDLED_FALLBACK_RULE); + expect(issues[0].message).toContain("hook 'enrich_lead'"); + // Reported, but never fatal: `os lint` exits 1 on `error` only, so a + // legitimate `fetch()` handler still lints clean-enough to ship. + expect(issues.some((i) => i.severity === 'error')).toBe(false); + }); + + it('says nothing about a handler that really does ship as metadata', () => { + expect(checkHookBodyLowering({ hooks: [selfContainedHook] })).toEqual([]); + }); + + it('truncates the multi-line offending-source dump to one line', () => { + const [issue] = checkHookBodyLowering({ hooks: [forbiddenTokenHook] }); + expect(issue.message).not.toContain('--- offending body source ---'); + expect(issue.message.split('\n')).toHaveLength(1); + }); + + describe('the two ways an author declares "bundle this deliberately"', () => { + it('says nothing about a string handler (already a bundle reference)', () => { + const issues = checkHookBodyLowering({ + hooks: [{ name: 'h', object: 'o', events: ['beforeInsert'], handler: 'some_bundled_fn' }], + }); + expect(issues).toEqual([]); + }); + + it('says nothing when the author supplied an explicit `body`', () => { + // `lowerCallables` only extracts `if (!hook.body)`; this rule mirrors that + // skip, so the opt-out is the same one the build already honours. + const issues = checkHookBodyLowering({ + hooks: [{ ...freeIdentifierHook, body: { language: 'js', source: 'return 1;' } }], + }); + expect(issues).toEqual([]); + }); + + it('never judges a top-level `functions:` entry — that path is never lowered', () => { + const issues = checkHookBodyLowering({ + functions: { my_fn: (ctx: any) => ctx.input.x = SLA_MATRIX.high }, + }); + expect(issues).toEqual([]); + }); + }); + + describe('actions', () => { + it('judges an object action `target` and names its path', () => { + const issues = checkHookBodyLowering({ + objects: [ + { + name: 'case', + actions: [ + { name: 'escalate', target: (ctx: any) => { ctx.out = SLA_MATRIX.high; } }, + ], + }, + ], + }); + expect(issues).toHaveLength(1); + expect(issues[0].severity).toBe('error'); + expect(issues[0].path).toBe('objects[0].actions[0].target'); + expect(issues[0].message).toContain("action 'case_escalate'"); + }); + + it('judges a top-level action `target`', () => { + const issues = checkHookBodyLowering({ + actions: [{ name: 'sweep', target: (ctx: any) => { ctx.out = SLA_MATRIX.low; } }], + }); + expect(issues).toHaveLength(1); + expect(issues[0].path).toBe('actions[0].target'); + expect(issues[0].message).toContain("action 'global_sweep'"); + }); + }); + + /** + * The #3782 class: two surfaces disagreeing about what an author is told. + * This rule and the build both call `extractHookBody` on the same normalized + * input, so the agreement is by construction — this pins that it stays so, + * and would fail the moment someone re-implements the analysis here. + */ + it('judges exactly the callables the build records as extraction warnings', () => { + const config = { + hooks: [ + freeIdentifierHook, + forbiddenTokenHook, + selfContainedHook, + { name: 'string_ref', object: 'o', events: ['beforeInsert'], handler: 'bundled' }, + { ...selfContainedHook, name: 'with_body', body: { language: 'js', source: 'return 1;' } }, + ], + objects: [ + { + name: 'case', + actions: [{ name: 'escalate', target: (ctx: any) => { ctx.out = SLA_MATRIX.high; } }], + }, + ], + }; + + // What the BUILD records (and then bundles anyway, at exit 0). + const lowering = lowerCallables(structuredClone_(config)); + const buildSaw = lowering.bodyExtractionWarnings + .map((w) => `${w.origin}|${w.kind}`) + .sort(); + + // What LINT reports, mapped back through the rule -> kind correspondence. + const kindOfRule: Record = { + [NOT_LOWERABLE_RULE]: 'free-identifiers', + [BUNDLED_FALLBACK_RULE]: 'forbidden-token', + }; + const lintSaw = checkHookBodyLowering(structuredClone_(config)) + .map((i) => { + const origin = /^((?:hook|action) '[^']+')/.exec(i.message)?.[1]; + return `${origin}|${kindOfRule[i.rule]}`; + }) + .sort(); + + expect(lintSaw).toEqual(buildSaw); + // And the population is the real one, not an empty set agreeing with itself. + expect(buildSaw).toEqual([ + "action 'case_escalate'|free-identifiers", + "hook 'case_sla'|free-identifiers", + "hook 'enrich_lead'|forbidden-token", + ]); + }); +}); + +/** + * `structuredClone` cannot carry functions, and `lowerCallables` mutates only + * shallow clones of what it is handed — so the two passes above must each get a + * fresh object graph without losing the callables. A shallow-enough hand clone + * is exactly that. + */ +function structuredClone_>(v: T): T { + return { + ...v, + ...(Array.isArray(v.hooks) ? { hooks: v.hooks.map((h: any) => ({ ...h })) } : {}), + ...(Array.isArray(v.objects) + ? { + objects: v.objects.map((o: any) => ({ + ...o, + ...(Array.isArray(o.actions) ? { actions: o.actions.map((a: any) => ({ ...a })) } : {}), + })), + } + : {}), + ...(Array.isArray(v.actions) ? { actions: v.actions.map((a: any) => ({ ...a })) } : {}), + }; +} diff --git a/packages/cli/src/lint/hook-body-lowering.ts b/packages/cli/src/lint/hook-body-lowering.ts new file mode 100644 index 0000000000..b47714f401 --- /dev/null +++ b/packages/cli/src/lint/hook-body-lowering.ts @@ -0,0 +1,211 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os lint` rule: a registered handler that cannot be lowered to a + * metadata-only body (#13651). + * + * ## The defect this closes + * + * An L2 hook handler is lowered to a metadata-only `body.source` and evaluated + * in QuickJS with no module scope. When the handler reaches out of that scope, + * `extractHookBody` refuses — and `lowerCallables` catches the refusal, records + * it, and ships the callable through the back-compat `.mjs` bundle instead. + * `os build` exits 0. The hook keeps working locally. What changed silently is + * the DEPLOYMENT SHAPE: the app has stopped being shippable as pure metadata. + * + * The refusal was never the missing piece — `extractHookBody` had already + * computed the exact free-identifier list. What was missing is that nothing + * said NO to the recorded array. This rule is that "no". + * + * ## Why a lint `error` and not a build failure + * + * `os lint` exits 1 on an `error`, so a gate can fail on this — which is what + * the card asked for — WITHOUT changing what `os build` accepts. That + * separation is deliberate and load-bearing: + * + * - `os build`'s warn-and-bundle is a published behaviour that downstream + * apps this repo cannot measure depend on. Flipping its default to a hard + * failure is a contract act, and the in-tree note at `compile.ts` step 2c + * already records that position. + * - `os lint`'s own rubric, by contrast, is explicitly "a lint verdict, not a + * publish gate" (see `lintConfig`), which is exactly the tier this belongs + * in. The shared authoring-rule registry could not host it either way: its + * `gating` tier must be run by all three commands (so it WOULD move the + * build's accept set) and its `advisory` tier can never emit an `error`. + * + * ## What separates an author's mistake from a deliberate bundle + * + * Today both arrive through one catch, so they share one fate. They are not + * the same event, and the refusal kind already tells them apart: + * + * `free-identifiers` ACCIDENTAL. The handler IS expressible as a metadata + * body; it merely names a module-scope const/helper/ + * import. The author wrote something that reads + * self-contained, and the platform quietly re-shaped the + * deployment behind them — its own message even says "no + * behavior change", which is true of behaviour and false + * of shape. Remedy is local: inline the value. => `error`. + * `forbidden-token` STRUCTURAL. `fetch`/`require`/`process`/`eval`/… are + * capabilities the sandbox does not have, so the handler + * can NEVER be a metadata body. Writing one IS choosing a + * bundled closure, and the bundle is the designed answer + * (the refusal text says "declare a Connector recipe + * instead"). Reporting it is right; failing on it would + * punish the legitimate path. => `warning`. + * `unparseable` An instrument limit, not a verdict about the author. + * => `warning`. + * + * An author who deliberately wants a bundled closure for the ACCIDENTAL class + * is not cornered: two declarative channels already exist and are already + * silent here, and neither needs a new spec key. + * + * 1. Give the hook an explicit `body` — extraction is skipped entirely + * (`lowerCallables` only extracts `if (!hook.body)`). + * 2. Move the function into the top-level `functions:` map and reference it + * from the hook by NAME (`handler` accepts a string). That path registers + * the callable for bundling and never attempts extraction at all — which + * is measurably how three callables in this repo's own examples already + * ship, warning-free. + * + * 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". That + * distinction already existed in the authoring surface; nothing was reading it. + * + * ## Parity + * + * This rule calls the SAME `extractHookBody` that `lowerCallables` calls, on + * the same normalized input. It therefore cannot drift from what `os build` + * would do to the same handler — the #3782 class (two surfaces disagreeing + * about what an author is told) is closed by construction here, not by a + * second implementation kept in sync by hand. + */ + +import { extractHookBody, HookBodyExtractionError } from '../utils/extract-hook-body.js'; + +/** Mirrors `LintIssue` in `../commands/lint.ts` (structurally compatible). */ +export interface HookBodyLintIssue { + severity: 'error' | 'warning' | 'suggestion'; + rule: string; + message: string; + path: string; + fix?: string; +} + +/** The rule name a gate greps for when the handler could have been metadata. */ +export const NOT_LOWERABLE_RULE = 'hook-body/not-lowerable'; +/** The rule name for a refusal whose designed answer is the bundle. */ +export const BUNDLED_FALLBACK_RULE = 'hook-body/bundled-fallback'; + +const DELIBERATE_BUNDLE_REMEDY = + 'If a bundled closure is what you want, say so: give the hook an explicit `body`, ' + + 'or move the function into the top-level `functions:` map and reference it by name ' + + '(`handler: \'\'`) — neither is lowered to a metadata body.'; + +const isPlainObject = (v: unknown): v is Record => + typeof v === 'object' && v !== null && !Array.isArray(v); + +type AnyFn = (...args: unknown[]) => unknown; + +/** + * Run `extractHookBody` over one callable and turn a refusal into an issue. + * Returns `null` when the body extracts cleanly — i.e. the callable really does + * ship as metadata. + */ +function judge(fn: AnyFn, originLabel: string, path: string): HookBodyLintIssue | null { + try { + extractHookBody(fn, originLabel); + return null; + } catch (err: unknown) { + const kind = err instanceof HookBodyExtractionError ? err.kind : 'unknown'; + const free = err instanceof HookBodyExtractionError ? err.freeIdentifiers : []; + // Only the first line: the refusal text carries a multi-line offending-source + // dump for `--strict-body`'s per-callable diagnostic, which would swamp a + // lint report. `os build --strict-body` remains the place to read it whole. + const firstLine = String((err as Error)?.message ?? err).split('\n')[0]; + + if (kind === 'free-identifiers') { + return { + severity: 'error', + rule: NOT_LOWERABLE_RULE, + message: + `${originLabel} cannot be lowered to a metadata-only body: it references ` + + `${free.length === 1 ? 'the identifier' : 'identifiers'} ${free.join(', ')}, which ` + + `${free.length === 1 ? 'is' : 'are'} not in scope inside the sandbox. The handler is ` + + `BUNDLED instead, so this app is no longer shippable as pure metadata — a change of ` + + `deployment shape, not of behaviour. Inline the value(s) into the handler, or reach ` + + `them through \`ctx\`. ${DELIBERATE_BUNDLE_REMEDY}`, + path, + }; + } + + return { + severity: 'warning', + rule: BUNDLED_FALLBACK_RULE, + message: + `${originLabel} is bundled rather than shipped as a metadata-only body. ${firstLine} ` + + `This is the designed fallback — the body uses something the sandbox cannot provide — ` + + `but the app is not pure metadata while it is here.`, + path, + }; + } +} + +/** + * Every registered callable `lowerCallables` would attempt to extract, judged. + * + * Deliberately mirrors that function's walk (hooks, object actions, top-level + * actions) and its skip conditions (a string handler is already a bundle + * reference; an explicit `body` opts out of extraction). A callable this rule + * does not visit is one `os build` never attempts to lower either. + */ +export function checkHookBodyLowering(config: Record): HookBodyLintIssue[] { + const issues: HookBodyLintIssue[] = []; + + if (Array.isArray(config.hooks)) { + config.hooks.forEach((raw, i) => { + if (!isPlainObject(raw)) return; + if (typeof raw.handler !== 'function') return; // string ref = deliberate bundle + if (raw.body) return; // author supplied the body themselves + const name = + typeof raw.name === 'string' && raw.name.length > 0 ? raw.name : 'anon_hook'; + const issue = judge(raw.handler as AnyFn, `hook '${name}'`, `hooks[${i}].handler`); + if (issue) issues.push(issue); + }); + } + + const judgeActions = (actions: unknown[], ownerLabel: string, pathPrefix: string): void => { + actions.forEach((raw, i) => { + if (!isPlainObject(raw)) return; + if (typeof raw.target !== 'function') return; + if (raw.body) return; + const baseName = + typeof raw.name === 'string' && raw.name.length > 0 + ? `${ownerLabel}_${raw.name}` + : `${ownerLabel}_anon_action`; + const issue = judge( + raw.target as AnyFn, + `action '${baseName}'`, + `${pathPrefix}[${i}].target`, + ); + if (issue) issues.push(issue); + }); + }; + + if (Array.isArray(config.objects)) { + config.objects.forEach((rawObj, oi) => { + if (!isPlainObject(rawObj)) return; + if (!Array.isArray(rawObj.actions)) return; + judgeActions( + rawObj.actions, + String(rawObj.name ?? 'object'), + `objects[${oi}].actions`, + ); + }); + } + + if (Array.isArray(config.actions)) { + judgeActions(config.actions, 'global', 'actions'); + } + + return issues; +} diff --git a/packages/cli/src/utils/extract-hook-body.ts b/packages/cli/src/utils/extract-hook-body.ts index 68d5665359..5416d2304d 100644 --- a/packages/cli/src/utils/extract-hook-body.ts +++ b/packages/cli/src/utils/extract-hook-body.ts @@ -29,11 +29,18 @@ * until #10678, which is the whole defect that card names. * `--strict-body` the same recorded warnings become a hard failure (exit 1) * with a per-callable diagnostic, and nothing is bundled. + * `os lint` (#13651) reads the REFUSAL KIND, not the exit code, and + * gives the two classes different verdicts: an accidental + * scope leak (`free-identifiers`) is a lint `error`, so a + * gate can fail on it; a structural one (`forbidden-token`) + * stays a warning, because bundling is its designed answer. + * `os lint` calls THIS function, so its verdict cannot + * drift from what `os build` would do to the same handler. * * So the allow-list gates what may become `body.source`; it does not (yet) gate * what may build. Closing the L3 `.mjs` path is `--strict-body`'s job today and * Phase 3's later — not this list's. Docs `hook-bodies.mdx` describe the same - * two outcomes; when this header and that page disagree, they are both wrong + * outcomes; when this header and that page disagree, they are both wrong * until one of them is measured over a real `os build`. * * Capability inference: we scan the body for known `ctx.api.*`, `ctx.log.*`, @@ -68,6 +75,69 @@ import { detectFreeIdentifiers } from './detect-free-identifiers.js'; +/** + * WHY a refusal carries a machine-readable kind (#13651). + * + * Every refusal below already KNOWS which rule refused — the rule is what + * produced the sentence. Until this type existed, that knowledge was flattened + * into the message string at the `throw` and never recovered: `lowerCallables` + * caught the error, kept `err.message`, and every consumer downstream had a + * paragraph of English where it needed a category. So the two refusals that + * mean OPPOSITE things to an author shared one undifferentiated fate: + * + * `free-identifiers` the handler IS expressible as a metadata-only body. + * It references a module-scope helper/import/const, so + * the deployment shape silently changed from metadata to + * bundled closure — against what the author wrote. The + * remedy is local and mechanical (inline the value). + * `forbidden-token` the handler is NOT expressible as a metadata-only body + * at ALL. `fetch`/`require`/`process`/… are capabilities + * the QuickJS sandbox does not have, so writing one IS + * choosing a bundled closure. Falling back to the bundle + * is the designed answer, not a degradation to report as + * an error. + * `unparseable` the extractor could not find a body to peel. An + * instrument limit, not an author verdict. + * + * ⛔ The kind is NOT a license to change what `os build` accepts. Both classes + * still fall back to bundling and still exit 0 — see `lowerCallables`, whose + * catch is deliberately kept. What the kind buys is that a consumer can now + * treat the accidental class differently from the structural one; `os lint` is + * the first to do so. + */ +export type HookBodyRefusalKind = 'unparseable' | 'forbidden-token' | 'free-identifiers'; + +/** + * A refusal from {@link extractHookBody}, carrying the classification the + * refusing rule already had. + * + * The `message` is deliberately byte-identical to what this function threw + * before the class existed: `os build`'s warn-and-bundle line, `--strict-body`'s + * per-callable diagnostic and `content/docs/automation/hook-bodies.mdx` all + * quote those sentences, and a refusal that reads differently would be a + * documentation break wearing a refactor's clothes. The class ADDS structure + * beside the prose; it does not restate it. + */ +export class HookBodyExtractionError extends Error { + readonly kind: HookBodyRefusalKind; + readonly originLabel: string; + /** Names the handler referenced but does not bind — `free-identifiers` only. */ + readonly freeIdentifiers: readonly string[]; + + constructor( + kind: HookBodyRefusalKind, + originLabel: string, + message: string, + freeIdentifiers: readonly string[] = [], + ) { + super(message); + this.name = 'HookBodyExtractionError'; + this.kind = kind; + this.originLabel = originLabel; + this.freeIdentifiers = freeIdentifiers; + } +} + const FORBIDDEN_PATTERNS: Array<{ rx: RegExp; reason: string }> = [ { rx: /\bimport\s*[\(\*\{]/, reason: 'dynamic `import()` and ES imports are not allowed in hook/action bodies — declare a Connector recipe instead' }, // Both spellings, one reason (#10678). A TypeScript config is loaded through @@ -142,7 +212,9 @@ export function extractHookBody(fn: (...a: unknown[]) => unknown, originLabel: s // result is a pure block body suitable for `new Function('ctx', body)`. const block = peelToBlockBody(raw); if (!block) { - throw new Error( + throw new HookBodyExtractionError( + 'unparseable', + originLabel, `[hook-body-extract] could not parse the body of ${originLabel}; ` + `please rewrite the handler as a single arrow function or named function expression`, ); @@ -151,7 +223,9 @@ export function extractHookBody(fn: (...a: unknown[]) => unknown, originLabel: s // Reject any forbidden token before we ship the source as metadata. for (const { rx, reason } of FORBIDDEN_PATTERNS) { if (rx.test(block.source)) { - throw new Error( + throw new HookBodyExtractionError( + 'forbidden-token', + originLabel, `[hook-body-extract] ${originLabel}: ${reason}\n` + `--- offending body source ---\n${block.source.slice(0, 400)}${block.source.length > 400 ? '…' : ''}`, ); @@ -167,11 +241,14 @@ export function extractHookBody(fn: (...a: unknown[]) => unknown, originLabel: s // included) is analyzed so parameters are correctly in scope. const { free, unparsed } = detectFreeIdentifiers(raw); if (!unparsed && free.length > 0) { - throw new Error( + throw new HookBodyExtractionError( + 'free-identifiers', + originLabel, `[hook-body-extract] ${originLabel}: handler references identifier(s) not in scope at runtime: ` + `${free.join(', ')}. Module-scope helpers/imports aren't shipped with a metadata-only body, so ` + `this handler will be BUNDLED instead (no behavior change). To make it body-only, inline the ` + `helper(s) into the handler or move the logic behind \`ctx\` (e.g. \`ctx.api\`).`, + free, ); } diff --git a/packages/cli/src/utils/hook-body-refusal-kind.test.ts b/packages/cli/src/utils/hook-body-refusal-kind.test.ts new file mode 100644 index 0000000000..4b3cab3271 --- /dev/null +++ b/packages/cli/src/utils/hook-body-refusal-kind.test.ts @@ -0,0 +1,116 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #13651 — the refusal carries its classification, and the messages did not move. + * + * Two halves, and the second matters as much as the first: `os build`'s + * warn-and-bundle line, `--strict-body`'s per-callable diagnostic and + * `content/docs/automation/hook-bodies.mdx` all quote these sentences, so a + * refactor that "only" reworded them would be a documentation break with a + * green test suite. + */ + +import { describe, it, expect } from 'vitest'; +import { extractHookBody, HookBodyExtractionError } from './extract-hook-body.js'; +import { lowerCallables } from './lower-callables.js'; + +const TERRITORY = { US: 'na', DE: 'eu' }; + +describe('HookBodyExtractionError', () => { + it('classifies a module-scope reference as free-identifiers and names them', () => { + let caught: unknown; + try { + extractHookBody(((ctx: any) => { + ctx.input.territory = TERRITORY[ctx.input.country]; + }) as any, "hook 'x'"); + } catch (err) { + caught = err; + } + + expect(caught).toBeInstanceOf(HookBodyExtractionError); + const err = caught as HookBodyExtractionError; + expect(err.kind).toBe('free-identifiers'); + expect(err.originLabel).toBe("hook 'x'"); + expect(err.freeIdentifiers).toContain('TERRITORY'); + // Message unchanged — the docs quote this sentence. + expect(err.message).toContain('references identifier(s) not in scope at runtime'); + }); + + it('classifies a sandbox-impossible token as forbidden-token', () => { + let caught: unknown; + try { + extractHookBody((async (ctx: any) => { + ctx.out = await fetch('https://example.invalid'); + }) as any, "hook 'y'"); + } catch (err) { + caught = err; + } + + const err = caught as HookBodyExtractionError; + expect(err.kind).toBe('forbidden-token'); + expect(err.freeIdentifiers).toEqual([]); + expect(err.message).toContain('`fetch()` is not allowed in hook/action bodies'); + // The offending-source dump the `--strict-body` diagnostic prints. + expect(err.message).toContain('--- offending body source ---'); + }); +}); + +describe('lowerCallables carries the classification without changing what it does', () => { + const config = () => ({ + hooks: [ + { + name: 'territory', + object: 'account', + events: ['beforeInsert'], + handler: (ctx: any) => { + ctx.input.territory = TERRITORY[ctx.input.country]; + }, + }, + ], + }); + + it('records the kind and the identifiers beside the unchanged reason', () => { + const out = lowerCallables(config()); + + expect(out.bodyExtractionWarnings).toHaveLength(1); + const w = out.bodyExtractionWarnings[0]; + expect(w.origin).toBe("hook 'territory'"); + expect(w.kind).toBe('free-identifiers'); + expect(w.freeIdentifiers).toContain('TERRITORY'); + expect(w.reason).toContain('references identifier(s) not in scope at runtime'); + }); + + it('STILL bundles the callable and still emits no body — the fallback is intact', () => { + // The catch must survive this card. If a later change deletes it, the + // legitimate "author deliberately wants a bundled closure" path dies with + // the accidental one, and this assertion is what notices. + const out = lowerCallables(config()); + + expect(out.count).toBe(1); + expect(out.bodyExtracted).toBe(0); + expect(out.functions.territory).toBeTypeOf('function'); + expect((out.lowered.hooks as any[])[0].handler).toBe('territory'); + expect((out.lowered.hooks as any[])[0].body).toBeUndefined(); + }); + + it('reports kind "unknown" when the extractor itself throws a bare Error', () => { + // Not folded into `unparseable`: an instrument failure must stay + // distinguishable from a verdict about the author's handler. + const exploding = () => { + throw new Error('boom'); + }; + Object.defineProperty(exploding, 'toString', { + value: () => { + throw new Error('cannot stringify'); + }, + }); + + const out = lowerCallables({ + hooks: [{ name: 'boom', object: 'o', events: ['beforeInsert'], handler: exploding }], + }); + + expect(out.bodyExtractionWarnings).toHaveLength(1); + expect(out.bodyExtractionWarnings[0].kind).toBe('unknown'); + expect(out.bodyExtractionWarnings[0].freeIdentifiers).toEqual([]); + }); +}); diff --git a/packages/cli/src/utils/lower-callables.ts b/packages/cli/src/utils/lower-callables.ts index 8d96f8e043..933a484fb9 100644 --- a/packages/cli/src/utils/lower-callables.ts +++ b/packages/cli/src/utils/lower-callables.ts @@ -21,7 +21,32 @@ * emitting it. */ -import { extractHookBody } from './extract-hook-body.js'; +import { extractHookBody, HookBodyExtractionError, type HookBodyRefusalKind } from './extract-hook-body.js'; + +/** + * One recorded extraction refusal. + * + * `origin` and `reason` are unchanged — `os build`'s warn-and-bundle line, + * `--strict-body`'s diagnostic and the `--json` payload all read them. + * `kind` (#13651) is the classification the refusing rule already had and used + * to throw away at the catch below; `freeIdentifiers` is the list + * `detectFreeIdentifiers` had already computed. Both are ADDITIVE: nothing here + * changes which callables are bundled, or the exit code. + * + * `kind` is `'unknown'` only for a throw that is not a + * {@link HookBodyExtractionError} — i.e. a bug in the extractor rather than a + * verdict about the author's handler. It is deliberately NOT folded into + * `'unparseable'`: a consumer must be able to tell "the extractor refused this + * handler" from "the extractor itself fell over", and silently filing the + * second as the first is how an instrument failure gets read as an author + * verdict. + */ +export interface BodyExtractionWarning { + origin: string; + reason: string; + kind: HookBodyRefusalKind | 'unknown'; + freeIdentifiers: readonly string[]; +} export interface LoweringResult { /** A deep-cloned, JSON-safe copy of the stack with handlers replaced by strings. */ @@ -33,7 +58,7 @@ export interface LoweringResult { /** Number of handlers that successfully emitted a metadata-only `body`. */ bodyExtracted: number; /** Per-extraction failures (still emit handler ref + bundle, but warn). */ - bodyExtractionWarnings: Array<{ origin: string; reason: string }>; + bodyExtractionWarnings: BodyExtractionWarning[]; } type AnyFn = (...args: unknown[]) => unknown; @@ -57,7 +82,7 @@ function uniqueName(base: string, taken: Set): string { export function lowerCallables(input: Record): LoweringResult { const functions: Record = {}; const taken = new Set(); - const warnings: Array<{ origin: string; reason: string }> = []; + const warnings: BodyExtractionWarning[] = []; let bodyExtracted = 0; // Try to extract a metadata-only body from a callable. Returns null if the @@ -72,7 +97,17 @@ export function lowerCallables(input: Record): LoweringResult { bodyExtracted += 1; return { language: 'js', source: ext.source, capabilities: ext.capabilities }; } catch (err: any) { - warnings.push({ origin: originLabel, reason: err?.message ?? String(err) }); + // ⛔ The catch STAYS. Deleting it would make every refusal fatal and take + // the LEGITIMATE fallback-to-bundling path down with the accidental one — + // the two share this catch, which is exactly why the refusal now arrives + // classified. Telling them apart is the consumer's job (`os lint`), not + // this function's: lowering keeps bundling both, at exit 0, unchanged. + warnings.push({ + origin: originLabel, + reason: err?.message ?? String(err), + kind: err instanceof HookBodyExtractionError ? err.kind : 'unknown', + freeIdentifiers: err instanceof HookBodyExtractionError ? err.freeIdentifiers : [], + }); return null; } } From f7924e98c0f414777fa7fd5ef4b4fa812b54d2af Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 13:56:46 +0000 Subject: [PATCH 2/3] fix(cli): type the two test fixtures' index maps so tsc reads them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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` 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 Claude-Session: https://claude.ai/code/session_01UngCYXF98BVpYA9hfz6NYk --- packages/cli/src/lint/hook-body-lowering.test.ts | 2 +- packages/cli/src/utils/hook-body-refusal-kind.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/lint/hook-body-lowering.test.ts b/packages/cli/src/lint/hook-body-lowering.test.ts index cb8efdda4d..e49ad45b71 100644 --- a/packages/cli/src/lint/hook-body-lowering.test.ts +++ b/packages/cli/src/lint/hook-body-lowering.test.ts @@ -19,7 +19,7 @@ import { import { lowerCallables } from '../utils/lower-callables.js'; // Module scope — exactly what a lowered body cannot reach. -const SLA_MATRIX = { high: 4, low: 48 }; +const SLA_MATRIX: Record = { high: 4, low: 48 }; const freeIdentifierHook = { name: 'case_sla', diff --git a/packages/cli/src/utils/hook-body-refusal-kind.test.ts b/packages/cli/src/utils/hook-body-refusal-kind.test.ts index 4b3cab3271..e2d37543e9 100644 --- a/packages/cli/src/utils/hook-body-refusal-kind.test.ts +++ b/packages/cli/src/utils/hook-body-refusal-kind.test.ts @@ -14,7 +14,7 @@ import { describe, it, expect } from 'vitest'; import { extractHookBody, HookBodyExtractionError } from './extract-hook-body.js'; import { lowerCallables } from './lower-callables.js'; -const TERRITORY = { US: 'na', DE: 'eu' }; +const TERRITORY: Record = { US: 'na', DE: 'eu' }; describe('HookBodyExtractionError', () => { it('classifies a module-scope reference as free-identifiers and names them', () => { From 87d595fc2dc9787039fc4c9bd3104dc59ea556cf Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 00:00:23 +0000 Subject: [PATCH 3/3] fix(cli): keep instrument failures distinct from author verdicts in os lint (#13834 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01UngCYXF98BVpYA9hfz6NYk --- .changeset/cli-hook-body-lowering-loud.md | 27 +++++-- .../cli/src/lint/hook-body-lowering.test.ts | 79 ++++++++++++++++++- packages/cli/src/lint/hook-body-lowering.ts | 41 +++++++++- 3 files changed, 137 insertions(+), 10 deletions(-) diff --git a/.changeset/cli-hook-body-lowering-loud.md b/.changeset/cli-hook-body-lowering-loud.md index c8d570c03b..2216914f46 100644 --- a/.changeset/cli-hook-body-lowering-loud.md +++ b/.changeset/cli-hook-body-lowering-loud.md @@ -15,14 +15,21 @@ The refusal was never the missing piece — `extractHookBody` had already comput the exact free-identifier list. What was missing is that nothing said no to the recorded array. -**The refusal now carries its classification.** `HookBodyExtractionError` (new -export, with `HookBodyRefusalKind`) names which rule refused — -`free-identifiers` / `forbidden-token` / `unparseable` — and `lowerCallables` -carries it plus the identifier list on each `bodyExtractionWarnings` entry -(also new in `os build --json`). That classification was computed at the throw -and flattened into a message string; it is now structure a consumer can act on. - -**`os lint` is the first consumer, and it tells the two classes apart.** They +**The refusal now carries its classification — and what that publishes rides +on two CLI surfaces, not on new API exports.** Internally, +`HookBodyExtractionError` (with `HookBodyRefusalKind`) names which rule +refused — `free-identifiers` / `forbidden-token` / `unparseable` — and +`lowerCallables` records it beside the identifier list on each +`bodyExtractionWarnings` entry. Those types are module-internal: the package +`exports` map exposes only `.` and `./console`, and `src/index.ts` re-exports +none of them. What this release actually publishes is: + +- **`os lint`'s exit contract** — the new `hook-body/not-lowerable` rule is an + `error`, so `os lint` can now exit 1 where it previously exited 0. +- **`os build --json`** — each `bodyExtractionWarnings` entry now carries + `kind` and `freeIdentifiers` fields beside the unchanged `origin`/`reason`. + +**`os lint` is the first consumer, and it tells the classes apart.** They used to share one catch, so they shared one fate: - **accidental** (`free-identifiers`) — the handler *is* expressible as a @@ -32,6 +39,10 @@ used to share one catch, so they shared one fate: capabilities the sandbox does not have, so writing one *is* choosing a bundled closure and the bundle is the designed answer. Reported as a **`warning`**, never fatal. +- **instrument** (`unparseable`, or a failure of the extractor itself) — the + tool could not judge the body at all. Reported as a **`warning`** under its + own rule, with prose that names the instrument — never as if the author chose + a bundle, because an instrument failure is not a verdict about the author. An author who deliberately wants a bundled closure keeps two channels that already existed and are still silent: give the hook an explicit `body`, or move diff --git a/packages/cli/src/lint/hook-body-lowering.test.ts b/packages/cli/src/lint/hook-body-lowering.test.ts index e49ad45b71..5af4fdea76 100644 --- a/packages/cli/src/lint/hook-body-lowering.test.ts +++ b/packages/cli/src/lint/hook-body-lowering.test.ts @@ -15,6 +15,8 @@ import { checkHookBodyLowering, NOT_LOWERABLE_RULE, BUNDLED_FALLBACK_RULE, + UNPARSEABLE_BODY_RULE, + EXTRACTION_FAILED_RULE, } from './hook-body-lowering.js'; import { lowerCallables } from '../utils/lower-callables.js'; @@ -49,6 +51,29 @@ const selfContainedHook = { }, }; +// `unparseable`: `String(fn)` yields something `peelToBlockBody` cannot peel — +// an instrument LIMIT, not anything the author chose. +const unparseableHook = (() => { + const fn = (ctx: any) => { + ctx.input.x = 1; + }; + Object.defineProperty(fn, 'toString', { value: () => '???' }); + return { name: 'opaque', object: 'o', events: ['beforeInsert'], handler: fn }; +})(); + +// `unknown`: the extractor itself throws a bare Error (not a +// `HookBodyExtractionError`) — an instrument FAILURE. Same fixture shape the +// build-side pin in `hook-body-refusal-kind.test.ts` uses. +const explodingHook = (() => { + const fn = () => undefined; + Object.defineProperty(fn, 'toString', { + value: () => { + throw new Error('cannot stringify'); + }, + }); + return { name: 'exploding', object: 'o', events: ['beforeInsert'], handler: fn }; +})(); + describe('checkHookBodyLowering', () => { it('reports an accidental scope leak as an ERROR a gate can fail on', () => { const issues = checkHookBodyLowering({ hooks: [freeIdentifierHook] }); @@ -88,6 +113,47 @@ describe('checkHookBodyLowering', () => { expect(issue.message.split('\n')).toHaveLength(1); }); + describe('an instrument failure is NOT an author verdict', () => { + // The whole PR exists because a silent downgrade was misattributed. Telling + // an author "you chose a bundled closure" when in fact the TOOL failed is + // the same wrong verdict wearing the fix's clothes — so `unparseable` and + // `unknown` must never land in the `bundled-fallback` arm, whose prose + // asserts "the body uses something the sandbox cannot provide". + + it('reports an unparseable body under its own rule, naming the instrument', () => { + const issues = checkHookBodyLowering({ hooks: [{ ...unparseableHook }] }); + + expect(issues).toHaveLength(1); + expect(issues[0].rule).toBe(UNPARSEABLE_BODY_RULE); + // Never fatal: an instrument limit must not move the exit contract. + expect(issues[0].severity).toBe('warning'); + expect(issues[0].message).toContain('not a verdict about the handler'); + // And never the author-verdict prose of the deliberate-bundle arm. + expect(issues[0].rule).not.toBe(BUNDLED_FALLBACK_RULE); + expect(issues[0].message).not.toContain('the body uses something the sandbox cannot provide'); + }); + + it('reports an extractor throw (kind `unknown`) under its own rule, not as a chosen bundle', () => { + const issues = checkHookBodyLowering({ hooks: [{ ...explodingHook }] }); + + expect(issues).toHaveLength(1); + expect(issues[0].rule).toBe(EXTRACTION_FAILED_RULE); + expect(issues[0].severity).toBe('warning'); + expect(issues[0].message).toContain('the extraction instrument itself failed'); + expect(issues[0].message).toContain('not a verdict about the handler'); + expect(issues[0].rule).not.toBe(BUNDLED_FALLBACK_RULE); + expect(issues[0].message).not.toContain('the body uses something the sandbox cannot provide'); + }); + + it('keeps the two instrument kinds distinct from each other, not just from the verdict arms', () => { + // `unknown` is deliberately not folded into `unparseable` (#13651): a + // broken instrument and a limited instrument are different events. + const [unparseable] = checkHookBodyLowering({ hooks: [{ ...unparseableHook }] }); + const [unknown] = checkHookBodyLowering({ hooks: [{ ...explodingHook }] }); + expect(unparseable.rule).not.toBe(unknown.rule); + }); + }); + describe('the two ways an author declares "bundle this deliberately"', () => { it('says nothing about a string handler (already a bundle reference)', () => { const issues = checkHookBodyLowering({ @@ -153,6 +219,8 @@ describe('checkHookBodyLowering', () => { freeIdentifierHook, forbiddenTokenHook, selfContainedHook, + unparseableHook, + explodingHook, { name: 'string_ref', object: 'o', events: ['beforeInsert'], handler: 'bundled' }, { ...selfContainedHook, name: 'with_body', body: { language: 'js', source: 'return 1;' } }, ], @@ -171,9 +239,15 @@ describe('checkHookBodyLowering', () => { .sort(); // What LINT reports, mapped back through the rule -> kind correspondence. + // Total over all four kinds on purpose: a kind with no row here is a kind + // whose divergent labeling this pin could never catch — which is exactly + // how the `unknown`-folded-into-"designed fallback" defect survived to + // review the first time. const kindOfRule: Record = { [NOT_LOWERABLE_RULE]: 'free-identifiers', [BUNDLED_FALLBACK_RULE]: 'forbidden-token', + [UNPARSEABLE_BODY_RULE]: 'unparseable', + [EXTRACTION_FAILED_RULE]: 'unknown', }; const lintSaw = checkHookBodyLowering(structuredClone_(config)) .map((i) => { @@ -183,11 +257,14 @@ describe('checkHookBodyLowering', () => { .sort(); expect(lintSaw).toEqual(buildSaw); - // And the population is the real one, not an empty set agreeing with itself. + // And the population is the real one, not an empty set agreeing with itself + // — all four kinds present, the two instrument kinds included. expect(buildSaw).toEqual([ "action 'case_escalate'|free-identifiers", "hook 'case_sla'|free-identifiers", "hook 'enrich_lead'|forbidden-token", + "hook 'exploding'|unknown", + "hook 'opaque'|unparseable", ]); }); }); diff --git a/packages/cli/src/lint/hook-body-lowering.ts b/packages/cli/src/lint/hook-body-lowering.ts index b47714f401..009c396043 100644 --- a/packages/cli/src/lint/hook-body-lowering.ts +++ b/packages/cli/src/lint/hook-body-lowering.ts @@ -52,8 +52,15 @@ * (the refusal text says "declare a Connector recipe * instead"). Reporting it is right; failing on it would * punish the legitimate path. => `warning`. - * `unparseable` An instrument limit, not a verdict about the author. + * `unparseable` An instrument LIMIT: the extractor found no body it + * could judge. Not a verdict about the author — reported + * under its own rule, with prose that names the + * instrument, never as the author choosing a bundle. * => `warning`. + * `unknown` An instrument FAILURE (a non-`HookBodyExtractionError` + * throw — the extractor itself broke). Same discipline, + * its own rule; kept distinct from `unparseable` on + * purpose. => `warning`. * * An author who deliberately wants a bundled closure for the ACCIDENTAL class * is not cornered: two declarative channels already exist and are already @@ -95,6 +102,10 @@ export interface HookBodyLintIssue { export const NOT_LOWERABLE_RULE = 'hook-body/not-lowerable'; /** The rule name for a refusal whose designed answer is the bundle. */ export const BUNDLED_FALLBACK_RULE = 'hook-body/bundled-fallback'; +/** The rule name when the extractor found no body it could judge (`unparseable`). */ +export const UNPARSEABLE_BODY_RULE = 'hook-body/unparseable'; +/** The rule name when the extraction instrument itself threw (kind `unknown`). */ +export const EXTRACTION_FAILED_RULE = 'hook-body/extraction-failed'; const DELIBERATE_BUNDLE_REMEDY = 'If a bundled closure is what you want, say so: give the hook an explicit `body`, ' + @@ -138,6 +149,34 @@ function judge(fn: AnyFn, originLabel: string, path: string): HookBodyLintIssue }; } + if (kind === 'unparseable' || kind === 'unknown') { + // An instrument LIMIT (`unparseable`: the extractor found no body it + // could judge) or an instrument FAILURE (`unknown`: the extractor itself + // threw something that is not a refusal). In both cases the tool is what + // fell short — the event says nothing about what the author chose, so it + // must not borrow the bundled-fallback prose below ("the body uses + // something the sandbox cannot provide"), which asserts a verdict about + // the handler. Kept distinct on purpose (#13651): an instrument failure + // must not read as a verdict about the author. + const [rule, instrument] = + kind === 'unparseable' + ? ([UNPARSEABLE_BODY_RULE, 'the extractor found no function body it could judge'] as const) + : ([EXTRACTION_FAILED_RULE, 'the extraction instrument itself failed'] as const); + return { + severity: 'warning', + rule, + message: + `${originLabel} could not be analysed: ${instrument}. ${firstLine} ` + + `This is a limit of the instrument, not a verdict about the handler — it does NOT mean ` + + `a bundled closure was chosen; whether this body could ship as pure metadata is ` + + `undetermined. \`os build\` still bundles it via the designed fallback.`, + path, + }; + } + + // `forbidden-token` — the only kind left, and the only one whose prose may + // assert the author's choice: the sandbox genuinely cannot provide what the + // body uses, so the bundle is the designed answer. return { severity: 'warning', rule: BUNDLED_FALLBACK_RULE,