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
51 changes: 51 additions & 0 deletions .changeset/hook-body-write-set-lint.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
---
"@objectstack/lint": minor
"@objectstack/spec": patch
---

feat(lint): L2 hook-body writes to undeclared fields warn at author time (#4271)

An L2 (`language:'js'`) hook body that writes a field the target object never
declares — `ctx.input.amout = 0`, `ctx.api.object('deal').update({ stag: … })`
— runs clean in the QuickJS sandbox, reports success, and the unknown column
simply never lands in the stored record. No diagnostic anywhere: the #4001
"silent no-op manufactures false completion" failure mode at the
runtime-expression layer. The read side (`hook.condition`) and the capability
surface were already statically checked; the write side was the one blind face,
and `hook-body.zod.ts` carried it as an **accepted gap**.

**New rule — `hook-body-write-unknown-field` (advisory).** `@objectstack/lint`
now parses each L2 body (TypeScript parser; parsed, never executed, never
type-checked) and resolves its literal writes against the target object's
declared + system fields. An unknown field warns with a did-you-mean. Wired
into `REFERENCE_INTEGRITY_RULES`, so `os validate`, `os lint` and `os compile`
all report it; it never blocks a build.

The recognized write shapes are declared as data — `HOOK_BODY_WRITE_PATTERNS`,
each entry carrying a canonical example that a reconciliation test round-trips
through the real extractor, so a pattern cannot be declared-but-unverified
(#3528's death). v1 ships three:

- `ctx.input.<field> = …` / `ctx.input['<field>'] ⟨op⟩= …` → the hook's own
target object(s); flat-input envelope keys (`id`/`options`/`ast`/`data`) are
never treated as record fields.
- `Object.assign(ctx.input, { <field>: … })` → same target.
- `ctx.api.object('<object>').insert|create|update({…})` / `.updateById(id, {…})`
→ the named object, at the **real** `ObjectRepository` payload positions
(`update(data)` — the payload is argument 0, not `update(id, data)`).

Everything statically unknowable is skipped silently, favouring missed findings
over false ones: computed keys, spreads, non-literal payloads, dynamic object
names, wildcard-target (`object:'*'`) input writes, cross-package targets,
aliased input (`const doc = ctx.input`), and multi-target hooks where the field
exists on *some* target (the body may branch per object — only an
everywhere-miss warns).

The lint stays off the kernel boot path: the TypeScript compiler loads lazily,
only when a hook actually carries a JS body (same contract as the react-page
gates, guarded by `lazy-deps.test.ts`).

`@objectstack/spec`: the `ScriptBodySchema` header's "write-set opacity —
accepted static-analysis gap" note now points at the lint instead, and spells
out what remains opaque so the warning's absence is not read as proof of
correctness.
13 changes: 13 additions & 0 deletions packages/lint/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,19 @@ export type {
AiAgentAuthoringSeverity,
} from './validate-ai-agent-authoring.js';

export {
validateHookBodyWrites,
extractHookBodyWrites,
HOOK_BODY_WRITE_PATTERNS,
HOOK_BODY_WRITE_UNKNOWN_FIELD,
} from './validate-hook-body-writes.js';
export type {
HookBodyWriteFinding,
HookBodyWriteSeverity,
HookBodyWritePattern,
ExtractedHookBodyWrite,
} from './validate-hook-body-writes.js';

// One entry point for the reference-resolution rules above (#3583 §5 D5).
// Adding a rule to `REFERENCE_INTEGRITY_RULES` runs it on `validate`, `lint`
// and `compile` at once — the CLI call sites do not change.
Expand Down
25 changes: 23 additions & 2 deletions packages/lint/src/lazy-deps.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,16 @@ describe('lazy dependency loading (kernel boot-path contract)', () => {
for (const dep of ${JSON.stringify(LAZY_DEPS)}) {
if (loaded(dep)) fail(dep + ' was loaded eagerly, at import time');
}
const jsHook = (source, language) => ({ objects: [{ name: 'a', fields: { amount: {} } }], hooks: [{ name: 'h', object: 'a', events: ['beforeInsert'], body: { language: language ?? 'js', source } }] });
mod.validateHookBodyWrites(jsHook('input.x > 0', 'expression'));
if (loaded('typescript')) fail('the hook-body write gate on an L1-only stack must not load typescript');
const syntax = mod.validateReactPages(${reactStack('function Page(){ return <div>oops; }')});
if (!loaded('sucrase')) fail('sucrase was not loaded by a react-page syntax validation');
if (loaded('typescript')) fail('the syntax gate must not load typescript');
if (!syntax.some((f) => f.rule === 'react-page-syntax')) fail('syntax gate produced no finding');
const hookWrites = mod.validateHookBodyWrites(jsHook('ctx.input.amout = 1;'));
if (!loaded('typescript')) fail('typescript was not loaded by an L2 hook-body write validation');
if (!hookWrites.some((f) => f.rule === 'hook-body-write-unknown-field')) fail('hook-body write gate produced no finding');
const props = mod.validateReactPageProps(${reactStack('function Page(){ return <ObjectForm mode="edit" />; }')});
if (!loaded('typescript')) fail('typescript was not loaded by a react-page props validation');
if (!props.some((f) => f.rule === 'react-prop-missing-required')) fail('props gate produced no finding');
Expand Down Expand Up @@ -111,14 +117,20 @@ describe('lazy dependency loading (kernel boot-path contract)', () => {

it('loads each dep lazily in-process and the gates still work', async () => {
const req = createRequire(import.meta.url);
const { validateReactPages, validateReactPageProps } = await import('./index.js');
const { validateReactPages, validateReactPageProps, validateHookBodyWrites } = await import('./index.js');

// Stacks without a react-source page never touch either dep.
expect(validateReactPages({ pages: [{ name: 'p', kind: 'object' }] })).toEqual([]);
expect(validateReactPageProps({ pages: [{ name: 'p', kind: 'object' }] })).toEqual([]);
expect(validateReactPageProps({ pages: [{ name: 'r', kind: 'react', source: ' ' }] })).toEqual([]);
// Nor do stacks whose hooks carry no L2 JS body — including a JS body that
// never mentions `ctx`/`Object` (the prefilter skips the parse entirely).
const hook = (body: unknown) => ({ name: 'h', object: 'a', events: ['beforeInsert'], body });
expect(validateHookBodyWrites({ hooks: [hook(undefined)] })).toEqual([]);
expect(validateHookBodyWrites({ hooks: [hook({ language: 'expression', source: 'input.x > 0' })] })).toEqual([]);
expect(validateHookBodyWrites({ hooks: [hook({ language: 'js', source: 'return 1;' })] })).toEqual([]);
for (const dep of LAZY_DEPS) {
expect(depLoaded(req.cache, dep), `${dep} loaded before any react-source validation`).toBe(false);
expect(depLoaded(req.cache, dep), `${dep} loaded before any react-source or L2-body validation`).toBe(false);
}

// The first react page with source pays the cost of exactly its own gate's
Expand All @@ -130,6 +142,15 @@ describe('lazy dependency loading (kernel boot-path contract)', () => {
expect(depLoaded(req.cache, 'typescript'), 'the syntax gate must not load typescript').toBe(false);
expect(syntax.some((f) => f.rule === 'react-page-syntax' && f.severity === 'error')).toBe(true);

// The first hook with an L2 JS body pays the typescript load — and the
// write-set gate works (#4271).
const hookWrites = validateHookBodyWrites({
objects: [{ name: 'a', fields: { amount: {} } }],
hooks: [hook({ language: 'js', source: 'ctx.input.amout = 1;' })],
});
expect(depLoaded(req.cache, 'typescript')).toBe(true);
expect(hookWrites.some((f) => f.rule === 'hook-body-write-unknown-field' && f.severity === 'warning')).toBe(true);

const props = validateReactPageProps({
pages: [{ name: 'r', kind: 'react', source: 'function Page(){ return <ObjectForm mode="edit" />; }' }],
});
Expand Down
16 changes: 14 additions & 2 deletions packages/lint/src/reference-integrity-suite.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ describe('reference-integrity suite — membership', () => {
'validateAiSurfaceAffinity',
'validateAiToolReferences',
'validateAiAgentAuthoring',
'validateHookBodyWrites',
]);
});

Expand Down Expand Up @@ -118,6 +119,16 @@ describe('reference-integrity suite — every member actually runs', () => {
// validateAiToolReferences: a tool name nothing declares, registers, or
// materialises (the HotCRM fictional-tool class).
skills: [{ name: 'metadata_authoring', surface: 'build', tools: ['forecast_revenue'] }],
hooks: [
// validateHookBodyWrites: the L2 body writes a field crm_lead does not
// declare — runs clean in the sandbox, never lands in the record (#4271).
{
name: 'score_lead',
object: 'crm_lead',
events: ['beforeInsert'],
body: { language: 'js', source: "ctx.input.lead_score = 100;" },
},
],
flows: [
{
name: 'lead_followup',
Expand Down Expand Up @@ -153,6 +164,7 @@ describe('reference-integrity suite — every member actually runs', () => {
expect(rules).toContain('ai-skill-surface-mismatch');
expect(rules).toContain('ai-skill-tool-unresolved');
expect(rules).toContain('agent-authoring-withdrawn');
expect(rules).toContain('hook-body-write-unknown-field');
});

it('carries a gating flow-template finding through the suite (#3810)', () => {
Expand All @@ -174,9 +186,9 @@ describe('reference-integrity suite — every member actually runs', () => {
expect(typeof f.message).toBe('string');
expect(typeof f.hint).toBe('string');
}
// Object references run first, agent-authoring last.
// Object references run first, hook-body writes last.
expect(findings[0].rule).toBe('object-reference-unknown');
expect(findings[findings.length - 1].rule).toBe('agent-authoring-withdrawn');
expect(findings[findings.length - 1].rule).toBe('hook-body-write-unknown-field');
});

it('returns nothing for an empty stack', () => {
Expand Down
7 changes: 7 additions & 0 deletions packages/lint/src/reference-integrity-suite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import { validateFlowTemplatePaths } from './validate-flow-template-paths.js';
import { validateAiSurfaceAffinity } from './validate-ai-surface-affinity.js';
import { validateAiToolReferences } from './validate-ai-tool-references.js';
import { validateAiAgentAuthoring } from './validate-ai-agent-authoring.js';
import { validateHookBodyWrites } from './validate-hook-body-writes.js';

export type ReferenceIntegritySeverity = 'error' | 'warning';

Expand Down Expand Up @@ -100,6 +101,12 @@ export const REFERENCE_INTEGRITY_RULES: readonly ReferenceIntegrityRule[] = [
{ name: 'validateAiSurfaceAffinity', run: validateAiSurfaceAffinity },
{ name: 'validateAiToolReferences', run: validateAiToolReferences },
{ name: 'validateAiAgentAuthoring', run: validateAiAgentAuthoring },
// Field names WRITTEN by an L2 hook body (`ctx.input.x = …`,
// `ctx.api.object('y').update({ x })`), resolved against the target object's
// declared fields — the write-side counterpart of validateFlowTemplatePaths'
// read-side membership (#4271). Lazy: only a hook that actually carries a
// `language:'js'` body loads the TypeScript parser.
{ name: 'validateHookBodyWrites', run: validateHookBodyWrites },
];

/**
Expand Down
Loading
Loading