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

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

The write-set lint that #4305 gave L2 hook bodies now covers the other surface
that carries one. An action body is the same artefact: the same
`HookBodySchema` union, parsed by the same `HookBodySchema.safeParse` in
`actionBodyRunnerFactory`, run in the same QuickJS sandbox. So it fails the
same way — `ctx.api.object('crm_deal').update({ stag: 'won' })` inside an
action succeeds, returns success to the caller, and the unknown column simply
never lands. Half the surface was still blind.

**New rule — `action-body-write-unknown-field` (advisory).** Wired into
`REFERENCE_INTEGRITY_RULES`, so `os validate`, `os lint` and `os compile` all
report it; it never blocks a build. Both places the runtime reads actions from
are walked — top-level `actions` and `objects[].actions` — and a
`defineStack`-merged action, which lives in both, is reported once at its
authored path. That dedupe is by VALUE (bound object + name + body source), not
by object identity the way `collectBundleActions` can afford: the suite runs on
the schema-PARSED stack, and parsing rebuilds every node, so the two copies
arrive as distinct objects that are merely equal. An identity check passes a
shared-reference unit fixture and then reports the showcase app's one warning
twice — which is exactly what it did before the end-to-end run caught it.

**Only the `ctx.api` write family carries over, and that is the point.** An
action's `ctx.input` is its PARAMS bag (`input: unwrapProxyToPlain(actionCtx
?.params)`), not a record, so resolving those names against object fields would
flag every correctly-named parameter — a pure false-positive machine, and a
false positive kills an advisory lint. `ctx.record` is not a write surface
either: the runner hands the body a plain snapshot and never writes it back, so
`ctx.record.x = …` is discarded for *declared* and undeclared fields alike —
a different defect from "the unknown column vanishes", and flagging only its
undeclared half would imply the declared half persists.

So the rule ships a declared **partition** of the shared
`HOOK_BODY_WRITE_PATTERNS` rather than a second ledger:
`ACTION_BODY_WRITE_PATTERN_IDS` (today: `api-crud-literal`) and
`ACTION_BODY_WRITE_EXCLUSIONS` (`input-property-assign`,
`input-object-assign`), each exclusion carrying its reason. The two halves are
tested to cover the shared ledger exactly, so a fourth pattern landing on the
hook side fails this rule's test until someone classifies it — silence is not a
decision. Every applicable pattern is additionally proved end-to-end through
the full validator (prefilter, pattern filter and field check included), and
every exclusion is proved to be about applicability rather than an
unextractable shape: the shared extractor still sees it, and this rule still
reports nothing for it.

One extractor, one field index, one implicit-field set, shared with the hook
rule rather than copied. The action rule is the same check on the other body
surface, so a second copy of `IMPLICIT_FIELDS` would drift exactly the way the
five hand-copied system-field lists #4330 collapsed did.

The lint stays off the kernel boot path, and lands one notch tighter than the
hook side: the only applicable pattern is rooted at `ctx.api`, so an action
body that never mentions it does not even parse, let alone load the ~9 MB
TypeScript compiler. Guarded by `lazy-deps.test.ts`.

`@objectstack/spec`: `ScriptBodySchema` and `ActionSchema.body` now point at
the action-side rule and spell out that `ctx.input` (params) and `ctx.record`
(a discarded snapshot) are not record-write surfaces — doc comments only, no
schema or generated-artifact change.
17 changes: 17 additions & 0 deletions packages/lint/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,23 @@ export type {
ExtractedHookBodyWrite,
} from './validate-hook-body-writes.js';

// The same write-set check on action bodies — same schema, same sandbox, same
// silent no-op. Its ledger is a declared partition of HOOK_BODY_WRITE_PATTERNS
// (only the `ctx.api` family survives the context change), so the two rules
// share one extractor rather than growing two.
export {
validateActionBodyWrites,
ACTION_BODY_WRITE_PATTERNS,
ACTION_BODY_WRITE_PATTERN_IDS,
ACTION_BODY_WRITE_EXCLUSIONS,
ACTION_BODY_WRITE_UNKNOWN_FIELD,
} from './validate-action-body-writes.js';
export type {
ActionBodyWriteFinding,
ActionBodyWriteSeverity,
ActionBodyWriteExclusion,
} from './validate-action-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
29 changes: 28 additions & 1 deletion packages/lint/src/lazy-deps.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,21 @@ describe('lazy dependency loading (kernel boot-path contract)', () => {
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 } }] });
const jsAction = (source, language) => ({ objects: [{ name: 'a', fields: { amount: {} } }], actions: [{ name: 'act', label: 'Act', objectName: 'a', 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');
mod.validateActionBodyWrites(jsAction('input.x > 0', 'expression'));
mod.validateActionBodyWrites(jsAction('ctx.input.amout = 1;'));
if (loaded('typescript')) fail('the action-body write gate must not load typescript for a body that never touches ctx.api');
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 actionWrites = mod.validateActionBodyWrites(jsAction("await ctx.api.object('a').update({ amout: 1 });"));
if (!actionWrites.some((f) => f.rule === 'action-body-write-unknown-field')) fail('action-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 @@ -117,7 +123,8 @@ 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, validateHookBodyWrites } = await import('./index.js');
const { validateReactPages, validateReactPageProps, validateHookBodyWrites, validateActionBodyWrites } =
await import('./index.js');

// Stacks without a react-source page never touch either dep.
expect(validateReactPages({ pages: [{ name: 'p', kind: 'object' }] })).toEqual([]);
Expand All @@ -129,6 +136,17 @@ describe('lazy dependency loading (kernel boot-path contract)', () => {
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([]);
// Action bodies narrow it further: the only pattern the action rule carries
// is rooted at `ctx.api`, so even an L2 body that writes params never parses.
const action = (body: unknown) => ({ name: 'act', label: 'Act', objectName: 'a', body });
expect(validateActionBodyWrites({ actions: [action(undefined)] })).toEqual([]);
expect(validateActionBodyWrites({ actions: [action({ language: 'expression', source: 'input.x > 0' })] })).toEqual([]);
expect(
validateActionBodyWrites({
objects: [{ name: 'a', fields: { amount: {} } }],
actions: [action({ language: 'js', source: 'ctx.input.amout = 1; ctx.record.nope = 2;' })],
}),
).toEqual([]);
for (const dep of LAZY_DEPS) {
expect(depLoaded(req.cache, dep), `${dep} loaded before any react-source or L2-body validation`).toBe(false);
}
Expand All @@ -151,6 +169,15 @@ describe('lazy dependency loading (kernel boot-path contract)', () => {
expect(depLoaded(req.cache, 'typescript')).toBe(true);
expect(hookWrites.some((f) => f.rule === 'hook-body-write-unknown-field' && f.severity === 'warning')).toBe(true);

// …and the action gate works on the same terms (#4271 follow-up).
const actionWrites = validateActionBodyWrites({
objects: [{ name: 'a', fields: { amount: {} } }],
actions: [action({ language: 'js', source: "await ctx.api.object('a').update({ amout: 1 });" })],
});
expect(actionWrites.some((f) => f.rule === 'action-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
18 changes: 16 additions & 2 deletions packages/lint/src/reference-integrity-suite.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ describe('reference-integrity suite — membership', () => {
'validateAiToolReferences',
'validateAiAgentAuthoring',
'validateHookBodyWrites',
'validateActionBodyWrites',
]);
});

Expand Down Expand Up @@ -60,6 +61,18 @@ describe('reference-integrity suite — every member actually runs', () => {
actions: [
// validateObjectReferences: a param pointing at an object nothing declares.
{ name: 'assign', label: 'Assign', params: [{ name: 'owner', reference: 'user' }] },
// validateActionBodyWrites: the L2 body persists a field crm_lead does
// not declare — the action returns success and the column never lands
// (#4271, the action half of the hook rule below).
{
name: 'score_now',
label: 'Score Now',
objectName: 'crm_lead',
body: {
language: 'js',
source: "await ctx.api.object('crm_lead').update({ lead_score: 100 });",
},
},
],
views: [
{
Expand Down Expand Up @@ -171,6 +184,7 @@ describe('reference-integrity suite — every member actually runs', () => {
expect(rules).toContain('ai-skill-tool-unresolved');
expect(rules).toContain('agent-authoring-withdrawn');
expect(rules).toContain('hook-body-write-unknown-field');
expect(rules).toContain('action-body-write-unknown-field');
});

it('carries a gating flow-template finding through the suite (#3810)', () => {
Expand All @@ -192,9 +206,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, hook-body writes last.
// Object references run first, action-body writes last.
expect(findings[0].rule).toBe('object-reference-unknown');
expect(findings[findings.length - 1].rule).toBe('hook-body-write-unknown-field');
expect(findings[findings.length - 1].rule).toBe('action-body-write-unknown-field');
});

it('returns nothing for an empty stack', () => {
Expand Down
6 changes: 6 additions & 0 deletions packages/lint/src/reference-integrity-suite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ 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';
import { validateActionBodyWrites } from './validate-action-body-writes.js';

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

Expand Down Expand Up @@ -118,6 +119,11 @@ export const REFERENCE_INTEGRITY_RULES: readonly ReferenceIntegrityRule[] = [
// read-side membership (#4271). Lazy: only a hook that actually carries a
// `language:'js'` body loads the TypeScript parser.
{ name: 'validateHookBodyWrites', run: validateHookBodyWrites },
// The same check on the other surface that carries a `HookBodySchema` body:
// action bodies, run by the same sandbox. Only the `ctx.api` write family
// carries over — an action's `ctx.input` is its params bag, not a record
// (see that module's ledger). Lazy on the same terms.
{ name: 'validateActionBodyWrites', run: validateActionBodyWrites },
];

/**
Expand Down
Loading
Loading