diff --git a/.changeset/lint-hook-rules-reach-handler-authored-hooks.md b/.changeset/lint-hook-rules-reach-handler-authored-hooks.md new file mode 100644 index 0000000000..18a9223e30 --- /dev/null +++ b/.changeset/lint-hook-rules-reach-handler-authored-hooks.md @@ -0,0 +1,19 @@ +--- +"@objectstack/cli": minor +"@objectstack/lint": minor +--- + +`objectstack lint` now judges hooks authored as inline `handler` functions with the same write-set rules it already applied to explicit `body` hooks. + +The `hook-body-write-unknown-field`, `hook-body-write-unprovisioned-anchor`, `hook-body-source-unparseable`, `hook-api-update-readonly-field` and `hook-api-update-readonly-when-field` rules open on `body.language === 'js'`. A hook written as `handler: async (ctx) => { … }` carries no `body`, so on the stack `objectstack lint` handed the rule registry the whole family returned before reading anything — while the reference app authors every one of its hooks that way. `objectstack build` never had the gap: it lowers each inline handler to a metadata body before it parses and judges the lowered stack. + +`objectstack lint` now hands the registry's parsed-tier rules that same lowered view (the `lowerCallables` pass the build runs), so a handler-authored hook writing a `readonly` field through `ctx.api` is refused by the pre-flight exactly as the build would refuse it. What this does and does not change: + +- A config whose inline handler writes a `readonly: true` field via `ctx.api.object(...).update()` / `.updateById()` / `.insert()` — and does not declare `runAs: 'system'` — now fails `objectstack lint` with `hook-api-update-readonly-field` (exit 1). It already failed `objectstack build` with the same finding, so nothing that built green fails lint red. +- The warning-severity members of the family (`hook-body-write-unknown-field`, `hook-api-update-readonly-when-field`, …) now report on inline handlers too; they never fail a run without `--strict`. +- Nothing about what `objectstack build` accepts changes, and `objectstack validate` — which parses without lowering — is unchanged and still does not see handler-authored hooks; both are recorded in the rules' headers. +- The lint input is never mutated: rules that read the live function value (`hook-body/not-lowerable` and its siblings) keep seeing it, and a handler the extractor refuses has no body on any command, so no rule guesses about a body that was not produced. + +Measured on this repository's own four example apps (`examples/app-crm`, `app-showcase`, `app-todo`, `app-multi-package`), before and after: **121 findings before, 121 after — row for row identical, and zero at `error` on both sides.** No config that passes today starts failing. Two of the six hooks in that corpus are `handler`-authored and were invisible to this family before; their bodies write nothing the family objects to, which is why the delta is zero rather than the family being unreached. The reach itself is pinned separately, with a body-authored control beside every leg. + +`@objectstack/lint` carries only the header ledger recording which *intakes* reach each hook rule — the call sites of `runAuthoringRules`, which are more numerous than the three commands (the scaffold validator is a fourth, and it has always reached this family). Its behaviour is unchanged. diff --git a/content/docs/automation/hook-bodies.mdx b/content/docs/automation/hook-bodies.mdx index 50226a8c2c..38ec5c08e2 100644 --- a/content/docs/automation/hook-bodies.mdx +++ b/content/docs/automation/hook-bodies.mdx @@ -268,6 +268,8 @@ The dropped case is the dangerous one: nothing fails, the step reports success, - `hook-api-update-readonly-field` — **error**. A body's literal `ctx.api.object('…').update()` / `.updateById()` / `.insert()` writes a field the named object declares `readonly: true`. Since [#15394](https://github.com/objectstack-ai/objectstack/issues/15394) the `insert` row of the table above is reported at build time exactly like the `update` row — same id, same severity, a message naming the verb — unless the hook declares `runAs: 'system'`. Only the static shape is judged on an insert: a `readonlyWhen` field has no prior record to lock on and the engine runs no conditional strip on INSERT, so no warning is produced there. - `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record *state*. The own-hook stamp **is** the workaround here, exactly as it is for static `readonly`: since [#9107](https://github.com/objectstack-ai/objectstack/issues/9107) the conditional strip judges the *caller's* entry payload, so a value a `beforeUpdate` hook **derives** is not caller-supplied and lands even on a locked record. (Deriving is the operative word — a hook that merely echoes the caller's own value back has written nothing the strip can tell from the caller's, and it still goes.) What does **not** help is elevation: unlike the static strip, the conditional lock is **not** waived by a system context, so neither `runAs: 'system'` nor the `sudo()` a body cannot reach makes a caller-supplied value survive. On this shape, confirm the write only targets records whose predicate is `false`, or derive the field in a `beforeUpdate` hook on the target object. +Which hooks these rules can *see* depends on the command, because every rule in this family opens on `body.language === 'js'`. A hook authored as an inline `handler` function carries no `body`, so it is judged only where the command has first lowered the handler to a metadata body: `objectstack build` always has (it lowers before it parses — see [How the build lowers a handler](#build-pipeline)), and since [#16095](https://github.com/objectstack-ai/objectstack/issues/16095) `objectstack lint` judges that same lowered view, so an author who runs only the pre-flight is told the same thing the build would refuse. `objectstack validate` parses without lowering, so there a handler-authored hook is not seen by this family — the explicit-`body` form is. A handler the build cannot lower (a forbidden token, a module-scope identifier) has no body on any command and is reported by the lowering rules instead, never guessed at here. + Only literal object names and literal payload keys are seen; a `sudo()` chain, a dynamic object name and an object this stack does not declare are all skipped, so the rule has no opinion on them. `.create()` is skipped too, for a reason about the **sandbox** rather than the engine: the VM-side `ctx.api.object()` installs `insert` / `update` / `delete` / `updateMany` / `deleteMany` / `upsert` and no `create` leaf, so a body calling `.create()` throws `TypeError: not a function` on its first run — a loud failure, not a silent drop — and the same payload spelled `.insert()` is what the rule judges. The flow surface has carried the same gate as `flow-update-readonly-field` since [#3425](https://github.com/objectstack-ai/objectstack/issues/3425), and since [#15394](https://github.com/objectstack-ai/objectstack/issues/15394) it reports a non-`runAs: 'system'` `create_record` node's static-`readonly` write at the same **error**, again with no conditional finding on a create. The table above is about a **hook** body. An **action** body is the one surface where the answer changes, so read this before you move a body from one to the other: an action body runs **elevated** — its `ctx.api` is built over the caller's envelope with `isSystem` set, which is the same trusted posture that lets an action bypass row and field permissions — and the static strip applies only to non-system callers. So `ctx.api.object('x').update({ someReadonlyField })` **lands** in an action, and there is no finding for it. Elevation does not waive the *conditional* lock, though, so that half does carry across: `action-api-update-readonly-when-field` — a **warning** — on an action body's literal `ctx.api` update to a `readonlyWhen` field ([#13770](https://github.com/objectstack-ai/objectstack/issues/13770)). Net effect when you move a body: a `readonly` write changes behaviour, a `readonlyWhen` write does not. diff --git a/packages/cli/src/commands/lint.ts b/packages/cli/src/commands/lint.ts index 9dcba005df..65828944cc 100644 --- a/packages/cli/src/commands/lint.ts +++ b/packages/cli/src/commands/lint.ts @@ -13,6 +13,7 @@ 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 { lowerCallables } from '../utils/lower-callables.js'; import { runMetadataEval } from '../lint/metadata-eval.js'; import { DEFAULT_METADATA_EVAL_CORPUS } from '../lint/corpus.js'; import { @@ -481,7 +482,40 @@ export function lintConfig(config: any, opts: LintConfigOptions = {}): LintIssue // give), so the registry runs both stack tiers against the normalized input — // which is what this command already did for the reference-integrity suite // and the security linter. - for (const f of runAuthoringRules('lint', { normalized: config, sduiManifest: opts.sduiManifest })) { + // + // ── The `parsed` tier is handed the LOWERED view (#16095) ── + // A hook authored as an inline `handler` function carries no `body`, and the + // `hook-body-*` / `hook-api-update-readonly-*` family opens on + // `body.language === 'js'` — so on the un-lowered stack the whole family + // returned before reading anything, while the reference app authors 39 of + // 39 hooks that way. `os build` never had that gap: it runs `lowerCallables` + // BEFORE its parse and judges the lowered stack, so the same rules fire + // there. This is the same call on the same normalized input, so what the + // family sees here cannot drift from what `os build` sees (the parity + // `checkHookBodyLowering` above already claims for the refusal side). + // + // What this changes and what it does not: + // - `parsed`-tier rules see `body: { language: 'js', source }` on every + // hook/action whose handler extracts, and `handler: ''` in place of + // the function — exactly the stack `os build` parses. Still unparsed: + // no defaults are filled, which is the standing condition of this tier + // under `os lint` and one every rule already tolerates. + // - `normalized`-tier rules keep the un-lowered input, as they do in `os + // build` (which hands them `normalized`, not `lowering.lowered`). + // - `lowerCallables` returns a NEW top-level object and re-maps the slots + // it touches (`hooks`, `objects[*].actions`, `actions`, `functions`, + // `packages[*].manifest`); the caller's stack is never mutated, so the + // function-reading rule above and `scoreMetadata` keep their live + // callables. A handler the extractor refuses is left with no `body` — + // the family stays silent on it and `checkHookBodyLowering` is what + // reports it, so no verdict is ever given about a body that was not + // produced. Nothing here touches what `os build` accepts (#13838). + const { lowered } = lowerCallables(config as Record); + for (const f of runAuthoringRules('lint', { + normalized: config, + parsed: lowered, + sduiManifest: opts.sduiManifest, + })) { issues.push({ severity: f.severity === 'info' ? 'suggestion' : f.severity, rule: f.rule, diff --git a/packages/cli/test/lint-hook-rules-reach-handler-hooks.e2e.test.ts b/packages/cli/test/lint-hook-rules-reach-handler-hooks.e2e.test.ts new file mode 100644 index 0000000000..9802d1b241 --- /dev/null +++ b/packages/cli/test/lint-hook-rules-reach-handler-hooks.e2e.test.ts @@ -0,0 +1,192 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #16095 — which authoring commands can SEE a hook authored as an inline + * `handler` function, measured over the real CLI, one door per leg, with a + * control beside every leg. + * + * The family (`hook-api-update-readonly-*`, `hook-body-*`) opens on + * `body.language === 'js'`. Whether a handler-authored hook reaches it is a + * property of the DOOR — what each command hands the rule registry — not of + * the rule, so a claim measured through one command says nothing about the + * others (#16109's lesson, applied here). The doors: + * + * `os build` lowers inline handlers to a metadata body BEFORE the parse + * (`lowerCallables`) and judges the lowered stack — the family + * reached handler-authored hooks here all along. + * `os lint` used to judge the un-lowered normalized stack; since #16095 + * `lintConfig` hands the registry's `parsed` tier the same + * lowered view `os build` judges. This file's RED leg. + * `os validate` parses the normalized stack WITHOUT lowering, so a + * handler-authored hook carries no body there and the family + * does not fire. Recorded below as a MEASUREMENT of that door, + * not as a contract: an author who runs `os validate` alone is + * not told. Closing it changes what `os validate` refuses and + * is its own decision (see the card's report). + * + * The fixture is the card's own: a readonly `is_escalated` written through + * `ctx.api.object('crm_case').update(…)` from an `afterUpdate` hook — the write + * the engine strips on a non-system context while the call reports success. + * The control authors the identical statement as an explicit `body`, which + * fired on every door before this change. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFile } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { childEnv } from './helpers/serve-process.js'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const CLI = resolve(HERE, '../bin/run-dev.js'); +const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); + +const READONLY_RULE = 'hook-api-update-readonly-field'; + +interface Run { + code: number; + stdout: string; + stderr: string; +} + +function runCli(args: string[], cwd: string): Promise { + return new Promise((resolvePromise) => { + execFile( + TSX, + [CLI, ...args], + { cwd, maxBuffer: 16 * 1024 * 1024, env: childEnv({ NO_COLOR: '1' }) }, + (err, stdout, stderr) => { + resolvePromise({ + code: err ? (typeof (err as { code?: unknown }).code === 'number' ? (err as unknown as { code: number }).code : 1) : 0, + stdout: String(stdout), + stderr: String(stderr), + }); + }, + ); + }); +} + +const OBJECT = `{ + name: 'crm_case', + label: 'Case', + sharingModel: 'private', + fields: { + title: { type: 'text', label: 'Title' }, + is_escalated: { type: 'boolean', label: 'Escalated', readonly: true }, + }, + }`; + +/** INTAKE: the reference app's shape — an inline handler, no `body`. */ +const CONFIG_HANDLER = ` +export default { + manifest: { id: 'com.example.reach_handler', name: 'reach_handler', version: '1.0.0', type: 'app' }, + objects: [${OBJECT}], + hooks: [{ + name: 'escalate', + object: 'crm_case', + events: ['afterUpdate'], + handler: async (ctx: any) => { + await ctx.api.object('crm_case').update({ id: ctx.input.id, is_escalated: true }); + }, + }], +}; +`; + +/** CONTROL: the identical statement authored as an explicit `body`. */ +const CONFIG_BODY = ` +export default { + manifest: { id: 'com.example.reach_body', name: 'reach_body', version: '1.0.0', type: 'app' }, + objects: [${OBJECT}], + hooks: [{ + name: 'escalate', + object: 'crm_case', + events: ['afterUpdate'], + body: { + language: 'js', + source: "await ctx.api.object('crm_case').update({ id: ctx.input.id, is_escalated: true });", + }, + }], +}; +`; + +const dirs: Record = {}; + +function project(key: string, source: string): string { + const dir = mkdtempSync(join(tmpdir(), `os-reach-${key}-`)); + writeFileSync(join(dir, 'objectstack.config.ts'), source); + dirs[key] = dir; + return dir; +} + +beforeAll(() => { + project('handler', CONFIG_HANDLER); + project('body', CONFIG_BODY); +}); + +afterAll(() => { + for (const dir of Object.values(dirs)) rmSync(dir, { recursive: true, force: true }); +}); + +/** Rule ids named anywhere in a `--json` payload's issue/finding lists. */ +function rulesIn(run: Run): string[] { + const json = JSON.parse(run.stdout); + const lists: unknown[] = [json.issues, json.errors, json.warnings, json.findings].filter(Array.isArray); + const ids: string[] = []; + for (const list of lists) { + for (const entry of list as Array>) { + if (typeof entry?.rule === 'string') ids.push(entry.rule); + } + } + return ids; +} + +const label = (run: Run) => `exit ${run.code}\nstdout:\n${run.stdout}\nstderr:\n${run.stderr}`; + +describe('#16095 — door: `os lint`', () => { + it('INTAKE — a handler-authored hook writing a readonly field is refused (error, exit 1)', async () => { + const run = await runCli(['lint', 'objectstack.config.ts', '--json'], dirs.handler); + expect(run.code, label(run)).toBe(1); + expect(rulesIn(run)).toContain(READONLY_RULE); + }, 60_000); + + it('CONTROL — the same statement as an explicit body is refused identically', async () => { + const run = await runCli(['lint', 'objectstack.config.ts', '--json'], dirs.body); + expect(run.code, label(run)).toBe(1); + expect(rulesIn(run)).toContain(READONLY_RULE); + }, 60_000); +}); + +describe('#16095 — door: `os build` (the door that never had the gap)', () => { + it('INTAKE — the lowered handler is refused at build, exit 1 — unchanged by this card', async () => { + const run = await runCli(['build', 'objectstack.config.ts', '--json'], dirs.handler); + expect(run.code, label(run)).toBe(1); + expect(rulesIn(run)).toContain(READONLY_RULE); + }, 90_000); + + it('CONTROL — the explicit body is refused at build identically', async () => { + const run = await runCli(['build', 'objectstack.config.ts', '--json'], dirs.body); + expect(run.code, label(run)).toBe(1); + expect(rulesIn(run)).toContain(READONLY_RULE); + }, 90_000); +}); + +describe('#16095 — door: `os validate` (measured, NOT lowered)', () => { + // A reading of the door as it stands, so a change to it is a change someone + // chose: `os validate` parses the normalized stack without lowering, and the + // handler-authored hook carries no body there. If this leg starts failing + // because `os validate` began lowering, the intake row becomes the control + // row — update the ledger in the file header, do not delete the pin. + it('INTAKE — the handler-authored hook is NOT seen by the family here (exit 0, no finding)', async () => { + const run = await runCli(['validate', 'objectstack.config.ts', '--json'], dirs.handler); + expect(run.code, label(run)).toBe(0); + expect(rulesIn(run)).not.toContain(READONLY_RULE); + }, 60_000); + + it('CONTROL — the explicit body IS refused here, so the silence above is the door, not the rule', async () => { + const run = await runCli(['validate', 'objectstack.config.ts', '--json'], dirs.body); + expect(run.code, label(run)).toBe(1); + expect(rulesIn(run)).toContain(READONLY_RULE); + }, 60_000); +}); diff --git a/packages/cli/test/lint-hook-rules-reach-handler-hooks.test.ts b/packages/cli/test/lint-hook-rules-reach-handler-hooks.test.ts new file mode 100644 index 0000000000..e31b078fd3 --- /dev/null +++ b/packages/cli/test/lint-hook-rules-reach-handler-hooks.test.ts @@ -0,0 +1,242 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #16095 — the `hook-body-*` / `hook-api-update-readonly-*` family reaches a + * hook authored as an inline `handler` function through `os lint`. + * + * Every rule in that family opens on `body.language === 'js'`. A hook written + * as `handler: async (ctx) => { … }` carries no `body`, so on the stack `os + * lint` used to hand the registry the whole family returned before reading + * anything — while the reference app authors 39 of 39 hooks that way. `os + * build` never had the gap: it lowers every inline handler to a metadata body + * (`lowerCallables`) BEFORE the parse, and hands the registry the lowered + * stack, so the same rules fire there. `lintConfig` now hands the registry's + * `parsed` tier that same lowered view, and these pins hold the reach. + * + * Every RED case has a control beside it — the identical statement authored as + * an explicit `body`, which fired on both sides of the change — so a red here + * is a reading about the door, never about the rule. + */ +import { describe, expect, it } from 'vitest'; +import { normalizeStackInput } from '@objectstack/spec'; +import { lintConfig } from '../src/commands/lint'; +import { runScaffoldAuthoringRules } from '../src/utils/scaffold-validate'; + +const READONLY_RULE = 'hook-api-update-readonly-field'; +const READONLY_WHEN_RULE = 'hook-api-update-readonly-when-field'; +const UNKNOWN_FIELD_RULE = 'hook-body-write-unknown-field'; + +/** A `crm_case` whose `is_escalated` nobody may hand-write (the card's fixture). */ +const OBJECTS = [ + { + name: 'crm_case', + label: 'Case', + fields: { + title: { type: 'text', label: 'Title' }, + is_escalated: { type: 'boolean', label: 'Escalated', readonly: true }, + credit_hold: { type: 'boolean', label: 'Credit hold', readonlyWhen: 'status == "closed"' }, + }, + }, +]; + +type Hook = Record; + +const stackWith = (hook: Hook) => + normalizeStackInput({ objects: OBJECTS, hooks: [hook] } as Record); + +const rulesOf = (hook: Hook, rule: string) => + lintConfig(stackWith(hook)).filter((i) => i.rule === rule); + +/** The statement under test, once as source text and once as a live function. */ +const WRITE_READONLY_SOURCE = + "await ctx.api.object('crm_case').update({ id: ctx.input.id, is_escalated: true });"; + +const handlerHook = (name: string, handler: unknown, extra: Hook = {}): Hook => ({ + name, + object: 'crm_case', + events: ['afterUpdate'], + handler, + ...extra, +}); + +const bodyHook = (name: string, source: string, extra: Hook = {}): Hook => ({ + name, + object: 'crm_case', + events: ['afterUpdate'], + body: { language: 'js', source }, + ...extra, +}); + +describe('#16095 — INTAKE: a handler-authored hook reaches the readonly family through lintConfig', () => { + it('RED — `hook-api-update-readonly-field` fires on an inline handler writing a readonly field', () => { + const findings = rulesOf( + handlerHook('escalate', async (ctx: any) => { + await ctx.api.object('crm_case').update({ id: ctx.input.id, is_escalated: true }); + }), + READONLY_RULE, + ); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('error'); + expect(findings[0].message).toContain('is_escalated'); + // The finding is reported on the LOWERED body — the same path `os build` + // reports it on for the same hook, so the two commands agree word for word. + expect(findings[0].path).toBe('hooks[0].body.source'); + }); + + it('CONTROL — the identical statement as an explicit `body` fires the same finding', () => { + const findings = rulesOf(bodyHook('escalate', WRITE_READONLY_SOURCE), READONLY_RULE); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('error'); + expect(findings[0].path).toBe('hooks[0].body.source'); + }); + + it('RED — `hook-api-update-readonly-when-field` (warning) fires on an inline handler too', () => { + const findings = rulesOf( + handlerHook('hold', async (ctx: any) => { + await ctx.api.object('crm_case').update({ id: ctx.input.id, credit_hold: true }); + }), + READONLY_WHEN_RULE, + ); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('warning'); + }); + + it('RED — `hook-body-write-unknown-field` fires on an inline handler writing an undeclared field', () => { + const findings = rulesOf( + handlerHook('typo', async (ctx: any) => { + await ctx.api.object('crm_case').update({ id: ctx.input.id, is_escalatd: true }); + }), + UNKNOWN_FIELD_RULE, + ); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('warning'); + expect(findings[0].message).toContain('is_escalatd'); + }); +}); + +describe('#16095 — NEGATIVE CONTROLS: the lowered view adds no verdict the rule would not give a body', () => { + it('`runAs: "system"` exempts the inline handler exactly as it exempts a body (#14010)', () => { + const viaHandler = rulesOf( + handlerHook( + 'escalate_sys', + async (ctx: any) => { + await ctx.api.object('crm_case').update({ id: ctx.input.id, is_escalated: true }); + }, + { runAs: 'system' }, + ), + READONLY_RULE, + ); + const viaBody = rulesOf(bodyHook('escalate_sys', WRITE_READONLY_SOURCE, { runAs: 'system' }), READONLY_RULE); + expect(viaHandler).toEqual([]); + expect(viaBody).toEqual([]); + }); + + it('a handler the extractor REFUSES gets no write-set verdict — the lowering rule reports it instead', () => { + // `fetch` is a forbidden token: `lowerCallables` records the refusal and + // leaves the hook with no `body`, so the family stays silent on it — the + // author is told by `hook-body/bundled-fallback`, not by a guess about a + // body that was never produced. + const issues = lintConfig( + stackWith( + handlerHook('remote', async (ctx: any) => { + await fetch('https://example.com/x'); + await ctx.api.object('crm_case').update({ id: ctx.input.id, is_escalated: true }); + }), + ), + ); + expect(issues.filter((i) => i.rule === READONLY_RULE)).toEqual([]); + expect(issues.filter((i) => i.rule === 'hook-body/bundled-fallback')).toHaveLength(1); + }); + + it('a hook that already carries an explicit `body` keeps it — the handler beside it is not re-extracted', () => { + // `lowerCallables` only extracts `if (!hook.body)`; an authored body wins, + // so the verdict is about what the author wrote, not about a shadow copy. + const findings = rulesOf( + handlerHook( + 'authored', + async (ctx: any) => { + await ctx.api.object('crm_case').update({ id: ctx.input.id, is_escalated: true }); + }, + { body: { language: 'js', source: 'return ctx.input;' } }, + ), + READONLY_RULE, + ); + expect(findings).toEqual([]); + }); + + it('a string `handler` (a bundle reference) is not lowered and stays outside the family', () => { + expect(rulesOf(handlerHook('legacy', 'legacy_fn'), READONLY_RULE)).toEqual([]); + }); +}); + +describe('#16095 — the lowering is a VIEW for the registry, not a rewrite of the input', () => { + it('lintConfig leaves the caller\'s stack untouched: the handler is still a function, no body appears', () => { + const stack = stackWith( + handlerHook('escalate', async (ctx: any) => { + await ctx.api.object('crm_case').update({ id: ctx.input.id, is_escalated: true }); + }), + ); + const before = JSON.stringify(stack); + lintConfig(stack); + const hook = (stack.hooks as Hook[])[0]; + expect(typeof hook.handler).toBe('function'); + expect(hook.body).toBeUndefined(); + expect(JSON.stringify(stack)).toBe(before); + }); + + it('the function-reading rule (`hook-body/not-lowerable`) still sees the live function on the same run', () => { + // The `normalized` tier and `checkHookBodyLowering` read FUNCTION values; + // handing them the lowered view would blind them. Both halves on one run. + const SLA_HOURS = 4; + const issues = lintConfig( + stackWith( + handlerHook('sla', async (ctx: any) => { + await ctx.api.object('crm_case').update({ id: ctx.input.id, is_escalated: SLA_HOURS > 2 }); + }), + ), + ); + expect(issues.filter((i) => i.rule === 'hook-body/not-lowerable')).toHaveLength(1); + // And because that handler could NOT be lowered, the family has no body to judge. + expect(issues.filter((i) => i.rule === READONLY_RULE)).toEqual([]); + }); +}); + +/** + * The FOURTH door, and the reason it is pinned here rather than left to the + * three `os *` legs in the e2e sibling. + * + * `runScaffoldAuthoringRules` is a separate entry into the same registry — + * `os init` / `dev` drive it over a freshly rendered template — and it is NOT + * one of the three commands. It reaches the family, and always did, because it + * lowers before it parses exactly as `compile.ts` does; nothing in this card + * changed it. It is pinned because an unpinned reached door is the failure the + * ledger in the two rule headers exists to prevent: a reachability claim + * measured through one entry point is a claim about that entry point, not + * about the rule. The control below is what makes the RED leg readable — if + * the scaffold path ever stops lowering, the handler leg goes silent while the + * body leg keeps firing, and that asymmetry is the signal. + */ +describe('#16095 — door: `runScaffoldAuthoringRules` (reached, and not one of the three commands)', () => { + const scaffoldStack = (hook: Hook) => ({ objects: OBJECTS, hooks: [hook] }); + const familyOf = (hook: Hook) => { + const report = runScaffoldAuthoringRules(scaffoldStack(hook)); + // A schema failure returns empty lists, which would read exactly like "no + // finding" — assert the stack actually parsed before reading the verdict. + expect(report.schemaError).toBeNull(); + return [...report.errors, ...report.advisories].filter((f) => f.rule === READONLY_RULE); + }; + + it('INTAKE — the handler-authored hook IS judged here (it lowers before it parses)', () => { + expect( + familyOf( + handlerHook('escalate', async (ctx: any) => { + await ctx.api.object('crm_case').update({ id: ctx.input.id, is_escalated: true }); + }), + ), + ).toHaveLength(1); + }); + + it('CONTROL — the identical statement as an explicit `body` fires the same finding', () => { + expect(familyOf(bodyHook('escalate_body', WRITE_READONLY_SOURCE))).toHaveLength(1); + }); +}); diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index 4580834955..68f22d4490 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -222,6 +222,15 @@ export type AuthoringRuleTier = 'gating' | 'advisory'; * `os validate`'s verdict to give), so it runs BOTH tiers on the normalized * stack. Every rule here is written to tolerate that — it is what `os lint` * already did for the reference-integrity suite and the security linter. + * Since #16095 the `parsed` tier under `os lint` is that same normalized stack + * with its inline `handler` callables LOWERED to a metadata `body` (the + * `lowerCallables` pass `os build` runs before its parse) — still unparsed, + * still the same input, but carrying the `body.source` the `hook-body-*` / + * `hook-api-update-readonly-*` family opens on. Without it the family judged + * only hooks authored with an explicit `body`, which the reference app never + * writes (39 of 39 hooks are `handler` functions), while `os build` judged + * them all along. `normalized`-tier rules keep the un-lowered input on every + * command, so a rule that reads a live function value belongs there. * * ## What `normalized` does NOT buy, measured (#6073) * @@ -1568,8 +1577,10 @@ export interface AuthoringRuleRun extends AuthoringRuleContext { /** `normalizeStackInput` output — pre-Zod-parse. Always required. */ normalized: AnyRec; /** - * Post-Zod-parse stack. Omitted by `os lint`, which does not parse; `parsed` - * rules then read `normalized` (see `AuthoringRuleInputTier`). + * Post-Zod-parse stack. `os lint`, which does not parse, hands this the + * normalized stack with its inline callables lowered to metadata bodies + * (#16095, see `AuthoringRuleInputTier`); omitted, `parsed` rules read + * `normalized`. */ parsed?: AnyRec; } diff --git a/packages/lint/src/validate-hook-body-writes.ts b/packages/lint/src/validate-hook-body-writes.ts index d1c60f0eff..898f15e7d8 100644 --- a/packages/lint/src/validate-hook-body-writes.ts +++ b/packages/lint/src/validate-hook-body-writes.ts @@ -711,6 +711,40 @@ export function extractHookBodyWriteSet(source: string): ExtractedHookBodyWriteS /** * Validate L2 hook-body writes against target-object field declarations. * Pure `(stack) => Finding[]` (ADR-0019); safe on pre- or post-parse stacks. + * + * ## Which intakes reach a hook authored as an inline `handler` (#16095) + * + * This rule opens on `body.language === 'js'`. A hook written as + * `handler: async (ctx) => { … }` carries no `body`, so whether it is judged + * is a property of the DOOR — what each caller hands this function — not of + * the rule. Measured (`packages/cli/test/lint-hook-rules-reach-handler-hooks*`): + * + * Every leg below was measured with the body-authored control beside it, so a + * silent leg is a reading about that door and never about this rule. The doors + * are the call sites of `runAuthoringRules`, enumerated — not the three `os *` + * commands, which are fewer than the doors: + * + * `os build` union `compile.ts` lowers every inline handler to a + * metadata body BEFORE its parse (`lowerCallables`) + * and judges the parsed result — REACHED, always was. + * `os build` per-package same lowered `result.data`, re-entered one package + * manifest at a time — REACHED, always was. + * `os lint` hands the registry's `parsed` tier that same + * lowered view — REACHED since #16095. + * scaffold validate `runScaffoldAuthoringRules` (`os init` / `dev` over + * a rendered template) lowers before it parses too — + * REACHED, always was, and pinned since #16095. + * `os validate` parses the normalized stack WITHOUT lowering — NOT + * reached; the body-authored control fires there. + * Changing that changes what `os validate` refuses + * and is its own decision, not this card's. + * direct call judges exactly the stack it is given — NOT reached + * unless the caller lowers first; measured both ways. + * + * A handler the extractor refuses (forbidden token, free identifier, + * unparseable) is left with no `body` on every door, so this rule stays silent + * on it; the refusal itself is reported by `os lint`'s `hook-body/*` rules and + * by `os build`'s warn-and-bundle line, never guessed at here. */ export function validateHookBodyWrites(stack: AnyRec): HookBodyWriteFinding[] { const findings: HookBodyWriteFinding[] = []; diff --git a/packages/lint/src/validate-readonly-hook-writes.ts b/packages/lint/src/validate-readonly-hook-writes.ts index c3ec36f083..e991d5c4e0 100644 --- a/packages/lint/src/validate-readonly-hook-writes.ts +++ b/packages/lint/src/validate-readonly-hook-writes.ts @@ -276,6 +276,40 @@ function isRec(v: unknown): v is AnyRec { * Validate L2 hook-body `ctx.api` writes against target-object readonly * declarations. Pure `(stack) => Finding[]` (ADR-0019); safe on pre- or * post-parse stacks. + * + * ## Which intakes reach a hook authored as an inline `handler` (#16095) + * + * This rule opens on `body.language === 'js'`. A hook written as + * `handler: async (ctx) => { … }` carries no `body`, so whether it is judged + * is a property of the DOOR — what each caller hands this function — not of + * the rule. Measured (`packages/cli/test/lint-hook-rules-reach-handler-hooks*`): + * + * Every leg below was measured with the body-authored control beside it, so a + * silent leg is a reading about that door and never about this rule. The doors + * are the call sites of `runAuthoringRules`, enumerated — not the three `os *` + * commands, which are fewer than the doors: + * + * `os build` union `compile.ts` lowers every inline handler to a + * metadata body BEFORE its parse (`lowerCallables`) + * and judges the parsed result — REACHED, always was. + * `os build` per-package same lowered `result.data`, re-entered one package + * manifest at a time — REACHED, always was. + * `os lint` hands the registry's `parsed` tier that same + * lowered view — REACHED since #16095. + * scaffold validate `runScaffoldAuthoringRules` (`os init` / `dev` over + * a rendered template) lowers before it parses too — + * REACHED, always was, and pinned since #16095. + * `os validate` parses the normalized stack WITHOUT lowering — NOT + * reached; the body-authored control fires there. + * Changing that changes what `os validate` refuses + * and is its own decision, not this card's. + * direct call judges exactly the stack it is given — NOT reached + * unless the caller lowers first; measured both ways. + * + * A handler the extractor refuses (forbidden token, free identifier, + * unparseable) is left with no `body` on every door, so this rule stays silent + * on it; the refusal itself is reported by `os lint`'s `hook-body/*` rules and + * by `os build`'s warn-and-bundle line, never guessed at here. */ export function validateReadonlyHookWrites(stack: AnyRec): ReadonlyHookWriteFinding[] { const findings: ReadonlyHookWriteFinding[] = [];