Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions .changeset/cli-hook-body-lowering-loud.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
---
'@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 — 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
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.
- **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
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.
14 changes: 14 additions & 0 deletions packages/cli/src/commands/lint.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 {
Expand DownExpand Up@@ -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<string, unknown>));

// ── 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).
Expand Down
292 changes: 292 additions & 0 deletions packages/cli/src/lint/hook-body-lowering.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,292 @@
// 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,
UNPARSEABLE_BODY_RULE,
EXTRACTION_FAILED_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: Record<string, number> = { 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();
},
};

// `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] });

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('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({
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,
unparseableHook,
explodingHook,
{ 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.
// 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<string, string> = {
[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) => {
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
// — 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",
]);
});
});

/**
* `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_<T extends Record<string, any>>(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 })) } : {}),
};
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
feat(cli): make the silent hook-body downgrade loud — `os lint` refuses an accidental scope leak (ask 1) by os-steve · Pull Request #13834 · objectstack-ai/objectstack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions .changeset/cli-hook-body-lowering-loud.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
---
'@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 — 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
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.
- **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
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.
14 changes: 14 additions & 0 deletions packages/cli/src/commands/lint.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 {
Expand DownExpand Up@@ -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<string, unknown>));

// ── 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).
Expand Down
292 changes: 292 additions & 0 deletions packages/cli/src/lint/hook-body-lowering.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,292 @@
// 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,
UNPARSEABLE_BODY_RULE,
EXTRACTION_FAILED_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: Record<string, number> = { 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();
},
};

// `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] });

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('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({
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,
unparseableHook,
explodingHook,
{ 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.
// 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<string, string> = {
[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) => {
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
// — 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",
]);
});
});

/**
* `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_<T extends Record<string, any>>(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 })) } : {}),
};
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' feat(cli): make the silent hook-body downgrade loud — `os lint` refuses an accidental scope leak (ask 1) by os-steve · Pull Request #13834 · objectstack-ai/objectstack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions .changeset/cli-hook-body-lowering-loud.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
---
'@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 — 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
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.
- **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
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.
14 changes: 14 additions & 0 deletions packages/cli/src/commands/lint.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 {
Expand DownExpand Up@@ -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<string, unknown>));

// ── 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).
Expand Down
292 changes: 292 additions & 0 deletions packages/cli/src/lint/hook-body-lowering.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,292 @@
// 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,
UNPARSEABLE_BODY_RULE,
EXTRACTION_FAILED_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: Record<string, number> = { 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();
},
};

// `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] });

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('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({
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,
unparseableHook,
explodingHook,
{ 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.
// 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<string, string> = {
[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) => {
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
// — 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",
]);
});
});

/**
* `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_<T extends Record<string, any>>(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 })) } : {}),
};
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' feat(cli): make the silent hook-body downgrade loud — `os lint` refuses an accidental scope leak (ask 1) by os-steve · Pull Request #13834 · objectstack-ai/objectstack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions .changeset/cli-hook-body-lowering-loud.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
---
'@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 — 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
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.
- **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
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.
14 changes: 14 additions & 0 deletions packages/cli/src/commands/lint.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 {
Expand DownExpand Up@@ -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<string, unknown>));

// ── 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).
Expand Down
292 changes: 292 additions & 0 deletions packages/cli/src/lint/hook-body-lowering.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,292 @@
// 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,
UNPARSEABLE_BODY_RULE,
EXTRACTION_FAILED_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: Record<string, number> = { 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();
},
};

// `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] });

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('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({
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,
unparseableHook,
explodingHook,
{ 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.
// 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<string, string> = {
[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) => {
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
// — 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",
]);
});
});

/**
* `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_<T extends Record<string, any>>(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 })) } : {}),
};
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' feat(cli): make the silent hook-body downgrade loud — `os lint` refuses an accidental scope leak (ask 1) by os-steve · Pull Request #13834 · objectstack-ai/objectstack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions .changeset/cli-hook-body-lowering-loud.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
---
'@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 — 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
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.
- **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
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.
14 changes: 14 additions & 0 deletions packages/cli/src/commands/lint.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 {
Expand DownExpand Up@@ -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<string, unknown>));

// ── 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).
Expand Down
292 changes: 292 additions & 0 deletions packages/cli/src/lint/hook-body-lowering.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,292 @@
// 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,
UNPARSEABLE_BODY_RULE,
EXTRACTION_FAILED_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: Record<string, number> = { 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();
},
};

// `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] });

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('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({
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,
unparseableHook,
explodingHook,
{ 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.
// 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<string, string> = {
[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) => {
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
// — 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",
]);
});
});

/**
* `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_<T extends Record<string, any>>(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 })) } : {}),
};
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' feat(cli): make the silent hook-body downgrade loud — `os lint` refuses an accidental scope leak (ask 1) by os-steve · Pull Request #13834 · objectstack-ai/objectstack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions .changeset/cli-hook-body-lowering-loud.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
---
'@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 — 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
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.
- **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
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.
14 changes: 14 additions & 0 deletions packages/cli/src/commands/lint.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 {
Expand DownExpand Up@@ -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<string, unknown>));

// ── 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).
Expand Down
292 changes: 292 additions & 0 deletions packages/cli/src/lint/hook-body-lowering.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,292 @@
// 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,
UNPARSEABLE_BODY_RULE,
EXTRACTION_FAILED_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: Record<string, number> = { 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();
},
};

// `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] });

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('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({
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,
unparseableHook,
explodingHook,
{ 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.
// 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<string, string> = {
[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) => {
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
// — 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",
]);
});
});

/**
* `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_<T extends Record<string, any>>(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 })) } : {}),
};
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' feat(cli): make the silent hook-body downgrade loud — `os lint` refuses an accidental scope leak (ask 1) by os-steve · Pull Request #13834 · objectstack-ai/objectstack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions .changeset/cli-hook-body-lowering-loud.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
---
'@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 — 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
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.
- **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
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.
14 changes: 14 additions & 0 deletions packages/cli/src/commands/lint.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 {
Expand DownExpand Up@@ -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<string, unknown>));

// ── 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).
Expand Down
292 changes: 292 additions & 0 deletions packages/cli/src/lint/hook-body-lowering.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,292 @@
// 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,
UNPARSEABLE_BODY_RULE,
EXTRACTION_FAILED_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: Record<string, number> = { 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();
},
};

// `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] });

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('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({
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,
unparseableHook,
explodingHook,
{ 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.
// 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<string, string> = {
[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) => {
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
// — 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",
]);
});
});

/**
* `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_<T extends Record<string, any>>(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 })) } : {}),
};
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); feat(cli): make the silent hook-body downgrade loud — `os lint` refuses an accidental scope leak (ask 1) by os-steve · Pull Request #13834 · objectstack-ai/objectstack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions .changeset/cli-hook-body-lowering-loud.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
---
'@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 — 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
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.
- **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
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.
14 changes: 14 additions & 0 deletions packages/cli/src/commands/lint.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 {
Expand DownExpand Up@@ -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<string, unknown>));

// ── 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).
Expand Down
292 changes: 292 additions & 0 deletions packages/cli/src/lint/hook-body-lowering.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,292 @@
// 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,
UNPARSEABLE_BODY_RULE,
EXTRACTION_FAILED_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: Record<string, number> = { 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();
},
};

// `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] });

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('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({
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,
unparseableHook,
explodingHook,
{ 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.
// 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<string, string> = {
[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) => {
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
// — 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",
]);
});
});

/**
* `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_<T extends Record<string, any>>(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 })) } : {}),
};
}
Loading
Loading