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
18 changes: 18 additions & 0 deletions .changeset/22161-lint-slice-3-one-line.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
"@objectstack/lint": patch
---

fix(lint): 14 more author-time findings print one verdict line, and `os explain <rule-id>` carries their reasoning

Clause-②: no

- **Shorter verdicts.** Each finding of these 14 rule ids now prints a `message` of one verdict sentence. Every finding the rules' own test suites fire, and the CLI suite's handler-hook variants, is at most 200 characters; before, the longest of each ran from 212 to 792 characters. The ids:
- hook bodies: `hook-body-write-unknown-field`, `hook-body-write-unprovisioned-anchor`, `hook-body-source-unparseable`;
- action bodies: `action-body-write-unknown-field`, `action-body-write-unprovisioned-anchor`, `action-record-write-discarded`, `action-body-source-unparseable`;
- flow nodes: `flow-node-write-unknown-field`, `flow-node-write-unprovisioned-anchor`;
- readonly writes: `flow-update-readonly-field`, `flow-update-readonly-when-field`, `hook-api-update-readonly-field`, `hook-api-update-readonly-when-field`, `action-api-update-readonly-when-field`.

The three write surfaces now share one wording per question: an undeclared field ends on "so the write is refused (INVALID_FIELD / 400)", an unprovisioned anchor reads "'FIELD' is an injected column with no storage on external object 'OBJECT', so … can never land", an unparseable hook or action body reads "L2 body did not parse (…), so writes in its unread part go unchecked", and a readonly write says it is "silently stripped" (a `readonlyWhen` field "where its predicate is TRUE"). The values the author wrote still close each verdict — the field, the object, the `ctx.api` call, the run identity — so a verdict over a long name grows with it, and a hook lowered from an inline `handler` keeps its " (judged on the metadata body lowered from the inline handler)" suffix. The `fix` (the CLI's `fix:` line, the runtime issue's `hint`), every rule id, severity and `path`, and what each rule accepts or refuses are unchanged. A tool that matched the old message text should match on `rule` and `path` instead.
- **`os explain <rule-id>` takes these 14 ids**, for example `os explain hook-body-write-unknown-field`. It prints the reasoning the verdicts no longer carry: how each write channel reaches the engine's declared-field door and what the refusal takes down with it, why a write to an unprovisioned anchor on an external object passes the validator and never lands, what a partially parsed body leaves unchecked, why an action's `ctx.record` is never written back, and which readonly strip a system context waives and which it does not. The paragraphs the three surfaces share are one text, printed under every id they explain. The `rule:` line under each of these findings now ends with `` — `os explain <rule-id>` for … ``. The no-argument listing and its `--json` `rules` array list the 14 ids, and so does the unknown-id error's `Rules with an explanation:` line. `RULE_EXPLANATIONS` in `@objectstack/lint` gains the 14 entries.
- **Where the new text prints.** On the CLI, `os validate`, `os build` (and `os compile`, which `os dev` runs on every compile), `os lint`, `os verify` and `os init`'s scaffold check print the new `message` on the text face, and `os validate --json` and `os build --json` carry it in their `warnings` and author-time `issues`. At the runtime publish gate (Studio, REST `/meta`, MCP), a `flow` write carries the four flow ids: `flow-node-write-unknown-field` and `flow-update-readonly-field` are errors, so the 422 issue's `message` and the refusal log line under `OS_ALLOW_UNLINTED_METADATA_WRITES` change; `flow-node-write-unprovisioned-anchor` and `flow-update-readonly-when-field` are warnings, so the `message` in the 2xx response's `advisories` and the deduped `[Protocol] authoring advisory` server log line change. Each issue's `hint` is unchanged.
- **Never at the runtime gate:** the ten hook and action ids. A `hook` or `action` write does not dispatch these rules (they parse authored JavaScript, which the publish path never loads), and a `flow` write's snapshot carries no hook or action body for them to read; they speak only on the CLI doors above.
302 changes: 302 additions & 0 deletions packages/lint/src/rule-explanations.ts

Large diffs are not rendered by default.

96 changes: 80 additions & 16 deletions packages/lint/src/validate-action-body-writes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import { describe, it, expect } from 'vitest';
import {
validateActionBodyWrites,
validateActionBodyWrites as validateActionBodyWritesUnrecorded,
ACTION_BODY_WRITE_PATTERNS,
ACTION_BODY_WRITE_PATTERN_IDS,
ACTION_RECORD_WRITE_PATTERNS,
Expand All @@ -18,6 +18,30 @@ import {
HOOK_BODY_WRITE_PATTERNS,
IMPLICIT_FIELDS,
} from './validate-hook-body-writes.js';
import { explainRule } from './rule-explanations.js';

// [#22161] Each finding of the rule ids this file's rule shortened is one
// verdict sentence; the reasoning it used to carry is the id's `os explain`
// entry. Every call below records what it fired, and the last case in this
// file holds each recorded verdict of those ids to one line of at most 200
// characters — so the pin covers every firing variant this suite exercises,
// not a chosen few. Run the whole file: that case reads what the cases above
// fired.
const SHORTENED_RULE_IDS: readonly string[] = [
ACTION_BODY_WRITE_UNPROVISIONED_ANCHOR,
ACTION_BODY_WRITE_UNKNOWN_FIELD,
ACTION_RECORD_WRITE_DISCARDED,
ACTION_BODY_SOURCE_UNPARSEABLE,
];
const firedShortened: Array<{ rule: string; message: string }> = [];
const validateActionBodyWrites: typeof validateActionBodyWritesUnrecorded = (...args) => {
const findings = validateActionBodyWritesUnrecorded(...args);
for (const f of findings) if (SHORTENED_RULE_IDS.includes(f.rule)) firedShortened.push(f);
return findings;
};

/** The `os explain` text of `rule`, one string. */
const explanationOf = (rule: string): string => explainRule(rule)?.paragraphs.join('\n') ?? '';

// Target objects: array-shaped and map-shaped `fields`, so both authoring
// shapes are resolved.
Expand Down Expand Up @@ -163,7 +187,7 @@ describe('validateActionBodyWrites — ctx.api writes', () => {
expect(findings[0].path).toBe('actions[0].body.source');
expect(findings[0].message).toContain('discont_total');
expect(findings[0].message).toContain('crm_deal');
expect(findings[0].message).toContain('INVALID_FIELD / 400, identically on every driver');
expect(findings[0].message).toContain('INVALID_FIELD / 400');
expect(findings[0].hint).toContain("'discount_total'");
});

Expand All @@ -176,24 +200,30 @@ describe('validateActionBodyWrites — ctx.api writes', () => {
// The old text promised a driver-level error on SQL and a persisted stray
// key on schemaless — neither happens on this path, and has not since
// #8682/#8738 put the declared-field door ahead of any statement.
//
// [#22161] The verdict names the refusal; the rest of the measured account
// is `os explain action-body-write-unknown-field`, so it is pinned there.
it('states the measured refusal — INVALID_FIELD / 400 on every driver — and no driver split', () => {
const [finding] = validateActionBodyWrites(
stackWith("await ctx.api.object('crm_deal').update({ discont_total: 0 });"),
);
const explanation = explanationOf(ACTION_BODY_WRITE_UNKNOWN_FIELD);

expect(finding.message).toContain('INVALID_FIELD / 400');
expect(finding.message).toContain('identically on every driver');
expect(finding.message).toContain('before any statement is built');
expect(explanation).toContain('identically on every driver');
expect(explanation).toContain('before any statement is built');
// The reason the door — not a driver — is what answers.
expect(finding.message).toContain('ordinary CALLER write');
// The action-side blast radius, the one word that differs from the hook
// sibling's sentence. Pinned so a future sweep cannot flatten the two.
expect(finding.message).toContain('fails the action');

expect(finding.message).not.toMatch(/driver-level error/);
expect(finding.message).not.toMatch(/schemaless/);
expect(finding.message).not.toMatch(/is persisted/);
expect(finding.message).not.toMatch(/write-path validator skips/);
expect(explanation).toContain('ordinary CALLER write');
// The action-side blast radius, the one sentence that differs from the
// hook sibling's explanation. Pinned so a future sweep cannot flatten the two.
expect(explanation).toContain('fails the action');

for (const text of [finding.message, explanation]) {
expect(text).not.toMatch(/driver-level error/);
expect(text).not.toMatch(/schemaless/);
expect(text).not.toMatch(/is persisted/);
expect(text).not.toMatch(/write-path validator skips/);
}
});

it('checks insert/update payloads (argument 0) and updateById at argument 1', () => {
Expand All @@ -203,7 +233,7 @@ describe('validateActionBodyWrites — ctx.api writes', () => {
"await ctx.api.object('crm_deal').updateById(ctx.recordId, { stag: 'won' });",
),
);
expect(findings.map((f) => f.message.match(/writing '(\w+)'/)?.[1])).toEqual(['emial', 'stag']);
expect(findings.map((f) => f.message.match(/writing undeclared field '(\w+)'/)?.[1])).toEqual(['emial', 'stag']);
expect(findings[0].message).toContain("ctx.api.object('crm_contact').insert");
expect(findings[1].message).toContain('updateById');
});
Expand Down Expand Up @@ -313,7 +343,10 @@ describe('validateActionBodyWrites — discarded ctx.record writes (#4345)', ()
expect(findings[0].where).toBe('action "close_deal" › body');
expect(findings[0].path).toBe('actions[0].body.source');
expect(findings[0].message).toContain('ctx.record.stage');
expect(findings[0].message).toContain("The snapshot stays read-only by design: an action's write channel is ctx.api.");
// [#22161] Why the snapshot is not a write surface is the id's `os explain` text.
expect(explanationOf(ACTION_RECORD_WRITE_DISCARDED)).toContain(
"The snapshot stays read-only by design: an action's write channel is `ctx.api`.",
);
expect(findings[0].hint).toContain('updateById');
});

Expand Down Expand Up @@ -528,7 +561,7 @@ describe('[#8663] validateActionBodyWrites — unprovisioned anchor writes', ()
expect(findings[0].where).toBe('action "stamp_owner" › body');
expect(findings[0].path).toBe('actions[0].body.source');
expect(findings[0].message).toContain("'owner_id'");
expect(findings[0].message).toContain('external object (ADR-0015)');
expect(findings[0].message).toContain("external object 'wh_order'");
expect(findings[0].message).toContain('can never land');
});

Expand Down Expand Up @@ -592,3 +625,34 @@ describe('an unparseable action body is reported, not scored clean (#10653)', ()
}
});
});

describe('[#22161] one-line verdicts — the rule ids this file shortened', () => {
it('every verdict the cases above fired for those ids is one line of at most 200 characters', () => {
// The coverage control first: each shortened id fired at least once, so
// the shape assertion below cannot pass over an empty record.
expect([...new Set(firedShortened.map((f) => f.rule))].sort()).toEqual([...SHORTENED_RULE_IDS].sort());
for (const f of firedShortened) {
expect(f.message, f.rule).not.toContain('\n');
expect(f.message.length, `${f.rule}: ${f.message}`).toBeLessThanOrEqual(200);
}
});

// What each verdict stopped saying, which `os explain RULE_ID` now prints.
const MOVED: Record<string, readonly string[]> = {
[ACTION_BODY_WRITE_UNPROVISIONED_ANCHOR]: ['external object', 'ADR-0015', 'PAST the write-path validator', 'no such column', 'schemaless remote'],
[ACTION_BODY_WRITE_UNKNOWN_FIELD]: ['scoped handle on the running engine', 'ordinary CALLER write', 'before any statement is built', 'fails the action'],
[ACTION_RECORD_WRITE_DISCARDED]: ['whether or not NAME is a declared field', 'read-only by design', 'provably dead'],
[ACTION_BODY_SOURCE_UNPARSEABLE]: ['partially recovered', 'judged by no rule', 'action-api-update-readonly-when-field'],
};

it('covers exactly the shortened ids', () => {
expect(Object.keys(MOVED).sort()).toEqual([...SHORTENED_RULE_IDS].sort());
});

it.each([...SHORTENED_RULE_IDS])('`os explain %s` carries what its verdict no longer says', (rule) => {
const explanation = explainRule(rule);
expect(explanation, `no \`os explain ${rule}\` entry`).toBeDefined();
const text = explanation!.paragraphs.join('\n');
for (const fact of MOVED[rule]) expect(text, `${rule} explanation names ${fact}`).toContain(fact);
});
});
47 changes: 20 additions & 27 deletions packages/lint/src/validate-action-body-writes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,18 +83,16 @@

import { findClosestMatches, formatSuggestion } from '@objectstack/spec/shared';

import { describeParseFailure, PARSE_FAILURE_HINT } from './checked-parse.js';
import {
indexUnprovisionedAnchors,
unprovisionedAnchorCause,
unprovisionedAnchorHint,
} from './system-fields.js';
import { PARSE_FAILURE_HINT } from './checked-parse.js';
import { indexUnprovisionedAnchors, unprovisionedAnchorHint } from './system-fields.js';
import {
bodyParseFailureVerdict,
extractHookBodyWriteSet,
indexObjectFields,
judgeableFieldsOf,
IMPLICIT_FIELDS,
unprovisionedAnchorWriteConsequence,
undeclaredApiWriteVerdict,
unprovisionedAnchorWriteVerdict,
HOOK_BODY_WRITE_PATTERNS,
type BodyWritePatternExclusion,
type HookBodyWritePattern,
Expand Down Expand Up @@ -341,9 +339,8 @@ export function validateActionBodyWrites(stack: AnyRec): ActionBodyWriteFinding[
rule: ACTION_BODY_SOURCE_UNPARSEABLE,
where,
path: site.path,
message:
`L2 body did not parse (${describeParseFailure(parseFailure)}), so its write set was read from a ` +
`partially recovered tree — an undeclared field write in the unread part is not reported.`,
// [#22161] The hook twin's sentence, from the one function both use.
message: bodyParseFailureVerdict(parseFailure),
hint: PARSE_FAILURE_HINT,
});
}
Expand All @@ -365,11 +362,13 @@ export function validateActionBodyWrites(stack: AnyRec): ActionBodyWriteFinding[
rule: ACTION_RECORD_WRITE_DISCARDED,
where,
path: site.path,
// [#22161] One verdict sentence; that the snapshot is read-only by
// design whether or not the field is declared, and why only a
// provably dead write is reported, is
// `os explain action-record-write-discarded`.
message:
`body assigns ctx.record.${w.field}, but an action's ctx.record is a plain snapshot the runtime ` +
`never writes back — the action returns success and the assignment is discarded, whether or not ` +
`'${w.field}' is a declared field. The snapshot stays read-only by design: an action's ` +
`write channel is ctx.api.`,
`body assigns ctx.record.${w.field}, but an action's ctx.record is a snapshot the runtime never ` +
`writes back, so the assignment is discarded while the action returns success`,
hint:
`To persist it, write through the API: ctx.api.object('<object>').updateById(ctx.recordId, ` +
`{ ${w.field}: … }). Reported only because ctx.record is never passed anywhere in this body — ` +
Expand Down Expand Up @@ -408,9 +407,7 @@ export function validateActionBodyWrites(stack: AnyRec): ActionBodyWriteFinding[
rule: ACTION_BODY_WRITE_UNPROVISIONED_ANCHOR,
where,
path: site.path,
message:
`body calls ctx.api.object('${w.object}').${w.method ?? 'update'}(…) writing '${w.field}', and ` +
`${unprovisionedAnchorCause(w.object, w.field)} — ${unprovisionedAnchorWriteConsequence()}`,
message: unprovisionedAnchorWriteVerdict(w.object, w.field, `the body's ${w.method ?? 'update'}(…) write`),
hint: unprovisionedAnchorHint(w.object, w.field),
});
continue;
Expand All @@ -422,16 +419,12 @@ export function validateActionBodyWrites(stack: AnyRec): ActionBodyWriteFinding[
rule: ACTION_BODY_WRITE_UNKNOWN_FIELD,
where,
path: site.path,
message:
`body calls ctx.api.object('${w.object}').${w.method ?? 'update'}(…) writing '${w.field}', but ` +
// [#13858] Same door, same measurement as the hook sibling — ctx.api
// is a ScopedContext over the running engine, so this payload is
// CALLER-supplied and #8682/#8738 refuse it before any driver.
`object '${w.object}' declares no such field. ctx.api is a scoped handle on the running ` +
`engine, so the payload arrives as an ordinary CALLER write and the declared-field door ` +
`REFUSES it at run time — INVALID_FIELD / 400, identically on every driver, before ` +
`any statement is built. The write lands nothing, and the refusal escapes the body and ` +
`fails the action.`,
// [#13858] Same door, same measurement as the hook sibling — ctx.api
// is a ScopedContext over the running engine, so this payload is
// CALLER-supplied and #8682/#8738 refuse it before any driver.
// [#22161] The hook sibling's verdict, from the one function both use;
// the reasoning is `os explain action-body-write-unknown-field`.
message: undeclaredApiWriteVerdict(w.object, w.method ?? 'update', w.field),
hint: fixHint(w.field, [...known]),
});
}
Expand Down
Loading
Loading