From 61aa7b3f0c83e7ec8c7ec8ad4f0b7cf7d25ad0bc Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 22:36:27 +0000 Subject: [PATCH 1/2] fix(lint): one-line fix lines for slice 15's rule ids (WIP) The hint of 17 more author-time rule ids in eight rule files is one remedy line; what the lines explained moves into their RULE_EXPLANATIONS entries. Claude-Session: https://claude.ai/code/session_019G5ZnRrTMiwcHG6iZM19zf Co-authored-by: Claude --- packages/lint/src/rule-explanations.ts | 44 ++++++++- .../src/validate-action-body-writes.test.ts | 43 ++++++++- .../lint/src/validate-action-body-writes.ts | 32 ++++--- .../src/validate-ai-agent-authoring.test.ts | 47 +++++++++- .../lint/src/validate-ai-agent-authoring.ts | 25 +++--- .../src/validate-flow-node-writes.test.ts | 37 +++++++- .../lint/src/validate-flow-node-writes.ts | 13 ++- .../src/validate-managed-api-methods.test.ts | 37 +++++++- .../lint/src/validate-managed-api-methods.ts | 14 +-- .../validate-nav-object-servability.test.ts | 26 +++++- .../src/validate-nav-object-servability.ts | 14 +-- .../src/validate-react-page-props.test.ts | 90 ++++++++++++++++++- .../lint/src/validate-react-page-props.ts | 46 +++++++--- .../src/validate-rule-compilability.test.ts | 55 +++++++++++- .../lint/src/validate-rule-compilability.ts | 22 +++-- .../validate-translatable-sections.test.ts | 42 +++++++-- .../src/validate-translatable-sections.ts | 10 +-- 17 files changed, 493 insertions(+), 104 deletions(-) diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index 8af3928cea5..e8b47936983 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -383,8 +383,9 @@ const UNPROVISIONED_ANCHOR_CAUSE = /** * The three ways out of an unprovisioned anchor, which the one-line `fix:` of * the converted anchor ids (`unprovisionedAnchorFix()`) names without the how: - * `sort-field-unprovisioned`, `searchable-field-unprovisioned` and - * `hook-body-write-unprovisioned-anchor` write it out. + * `sort-field-unprovisioned`, `searchable-field-unprovisioned`, + * `react-chart-field-unprovisioned` and the hook, action and flow-node + * write-anchor ids write it out. */ const UNPROVISIONED_ANCHOR_REMEDY = 'To fix it, check the remote table. If it really carries the column, declare it in the object\'s ' + @@ -951,6 +952,11 @@ const ACTION_BODY_WRITE_UNKNOWN_FIELD_EXPLANATION: RuleExplanation = { 'The nested write lands nothing, and its refusal escapes the body and fails the action. Hook bodies ' + 'get the same check (`hook-body-write-unknown-field`) through the same extractor, so the two rules ' + 'judge the same write shape the same way.', + 'Only the literal write shapes in `ACTION_BODY_WRITE_PATTERNS` are checked — today the ' + + '`ctx.api.object(NAME)` call with a literal payload — so a clean run does not prove every write ' + + 'lands, and the `ctx.input` shapes are excluded because an action\'s input is its params bag. The ' + + 'warning never blocks a build. The `fix:` line suggests the nearest declared or system column when ' + + 'one is close.', ], }; @@ -969,7 +975,8 @@ const ACTION_RECORD_WRITE_DISCARDED_EXPLANATION: RuleExplanation = { 'It is reported only when the write is provably dead: `ctx.record` never leaves the body as a ' + 'value. Handed to anything — an argument, an assignment, a spread, a return — the snapshot may be ' + 'a payload under construction, so every write in that body is skipped. An alias ' + - '(`const r = ctx.record`) counts as an escape; a property read does not.', + '(`const r = ctx.record`) counts as an escape; a property read does not. The warning never blocks ' + + 'a build.', ], }; @@ -992,6 +999,7 @@ const ACTION_BODY_WRITE_UNPROVISIONED_ANCHOR_EXPLANATION: RuleExplanation = { 'On an action body only the `ctx.api.object(NAME)` write is judged: an action\'s `ctx.input` is its ' + 'params bag, not a record, and its `ctx.record` is a snapshot the runtime never writes back ' + '(`action-record-write-discarded`).', + UNPROVISIONED_ANCHOR_REMEDY, ], }; @@ -1023,6 +1031,7 @@ const FLOW_NODE_WRITE_UNPROVISIONED_ANCHOR_EXPLANATION: RuleExplanation = { 'node\'s literal object name. This finding is a warning where the node\'s undeclared-field finding ' + 'is an error: that one is a certainty about this stack, this one is a claim about a remote schema ' + 'the build cannot see.', + UNPROVISIONED_ANCHOR_REMEDY, ], }; @@ -1723,6 +1732,11 @@ const VALIDATION_RULE_REGEX_UNCOMPILABLE_EXPLANATION: RuleExplanation = { 'The pattern is compiled exactly as `checkFormat` compiles it — `new RegExp(source)`, no flags — and ' + 'the verdict quotes the engine\'s reason; the `fix:` line quotes the source. The source is a STRING, ' + 'so a backslash in it is written twice in TypeScript.', + 'To fix it, escape a literal `(`, `[` or `\\`: in TypeScript source that is `\\\\(`, `\\\\[` and ' + + '`\\\\\\\\`, and a pattern such as `^\\d{2}-\\d{7}$` is written `\'^\\\\d{2}-\\\\d{7}$\'`. A rule that only ' + + 'checks a common shape can drop `regex` and name a `format` instead: ' + + // Held equal to `FormatValidationSchema`'s `format` enum by the rule's test. + '`email`, `url`, `phone` or `json`.', ], }; @@ -1738,6 +1752,10 @@ const VALIDATION_RULE_JSON_SCHEMA_UNCOMPILABLE_EXPLANATION: RuleExplanation = { 'outcome that depends on which schema the runtime happened to compile first — is not judged here. ' + 'A misspelled `format` name compiles in both environments; that is ' + '`validation-rule-json-schema-unknown-format`\'s verdict, not this one\'s.', + 'To fix it, correct the keyword the verdict names. The usual offenders: `type` must be one of ' + + '`null`, `boolean`, `object`, `array`, `number`, `string` or `integer` (or an array of those), ' + + '`required` an array of strings, and every `$ref` must resolve. A vendor keyword is fine, because ' + + 'the runtime runs `strict: false`; a MALFORMED standard keyword is not.', ], }; @@ -1915,6 +1933,7 @@ const REACT_CHART_FIELD_UNPROVISIONED_EXPLANATION: RuleExplanation = { 'name maps a remote column the author vouches for and is never reported. The finding is a warning ' + 'because the rule cannot see the remote table, only that the platform provisions no storage for ' + 'the anchor.', + UNPROVISIONED_ANCHOR_REMEDY, ], }; @@ -1930,6 +1949,10 @@ const REACT_BLOCK_NEEDS_RECORD_CONTEXT_EXPLANATION: RuleExplanation = { 'binding makes the block render. A component the page declares itself under the same name ' + 'shadows the injected one and is not judged. The `fix:` line names the block\'s react-tier ' + 'replacement, or a record page.', + 'On a react page the record is ordinary React state, so the replacement binds it itself: ' + + '`` by its own `objectName` / `recordId` props, `` with the parent binding ' + + 'as an ordinary filter, and a strip or a stage bar by reading the record with ' + + '`useAdapter().findOne` and laying it out in JSX.', ], }; @@ -2528,6 +2551,12 @@ const PLATFORM_AGENT_ROSTER = 'for old bookmarks and persisted `agent_id`s, never separate records, and the agent catalog ' + 'shows each agent once under its canonical name.'; +/** How an app reaches the platform agents without an agent of its own — the two agent ids it fixes. */ +const SKILLS_ATTACH_BY_SURFACE = + 'A capability or persona an app wants from an agent is authored as skills: a skill attaches to the ' + + 'platform agent (`ask` or `build`) by `surface` affinity, not through a custom agent or a custom ' + + '`defaultAgent` value, so the skills a stack declares reach the agent that serves their surface.'; + const DEFAULT_AGENT_LEGACY_ALIAS_EXPLANATION: RuleExplanation = { rule: 'default-agent-legacy-alias', covers: 'why a retired agent alias is the wrong spelling', @@ -2552,6 +2581,7 @@ const DEFAULT_AGENT_OUTSIDE_ROSTER_EXPLANATION: RuleExplanation = { 'The key is a plain snake_case identifier in the schema, so any value parses, validates and ' + 'builds. The check is a warning rather than a schema enum on purpose: narrowing the key is a ' + 'breaking authoring change ADR-0063 already walked back once.', + SKILLS_ATTACH_BY_SURFACE, ], }; @@ -2573,6 +2603,10 @@ const AGENT_AUTHORING_WITHDRAWN_EXPLANATION: RuleExplanation = { 'the rule cannot tell a platform package from an app package by reading the stack alone — the ' + 'runtime is what gates. Not a schema refinement either: an existing stack must keep parsing ' + '(ADR-0078).', + SKILLS_ATTACH_BY_SURFACE, + 'Everything an agent carries that a skill does not is persona text, so the useful parts of its ' + + '`instructions` move into the skills\' own instructions. The skills the agent references ' + + 'already carry its capability, so nothing is lost by dropping the persona.', ], }; @@ -2947,7 +2981,9 @@ const TRANSLATION_SECTION_NAME_MISSING_EXPLANATION: RuleExplanation = { 'Why the renderer does not fall back to a key slugified from the label: that would fossilise ' + 'a second contract (`_sections.SLUG_OF_LABEL`) beside the declared one, and the slug would ' + 'move the day anyone edits the heading text. The defect is at the producer, so this rule is ' + - 'where the producer hears about it.', + 'where the producer hears about it. For the same reason the name the `fix:` line suggests is a ' + + 'suggestion to write down, never a key derived from the label: the renderers look the heading ' + + 'up by name only, so renaming the heading later cannot break the lookup.', 'A warning: the section is real and renders, and only its heading stays in the source locale. ' + 'Judged only on an object that some `stack.translations` bundle carries a node for (a ' + 'monolingual stack owes nobody a section name), and only on the array form of `sections`: a ' + diff --git a/packages/lint/src/validate-action-body-writes.test.ts b/packages/lint/src/validate-action-body-writes.test.ts index 169c15b777d..08eff809664 100644 --- a/packages/lint/src/validate-action-body-writes.test.ts +++ b/packages/lint/src/validate-action-body-writes.test.ts @@ -33,7 +33,7 @@ const SHORTENED_RULE_IDS: readonly string[] = [ ACTION_RECORD_WRITE_DISCARDED, ACTION_BODY_SOURCE_UNPARSEABLE, ]; -const firedShortened: Array<{ rule: string; message: string }> = []; +const firedShortened: Array<{ rule: string; message: string; hint: 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); @@ -656,3 +656,44 @@ describe('[#22161] one-line verdicts — the rule ids this file shortened', () = for (const fact of MOVED[rule]) expect(text, `${rule} explanation names ${fact}`).toContain(fact); }); }); + +// [#22161] Slice 15: the `fix:` line of the four ids. Each names its remedy in +// one line, the hook sibling's where there is one; which write shapes are +// checked and that the warning never blocks a build, how to vouch for or opt +// out of an unprovisioned anchor, why only a provably dead record write is +// reported, and what a partially recovered tree leaves unchecked are the +// explanations'. +describe('[#22161] one-line fixes — the rule ids this file shortened', () => { + it('an unknown field with no near miss gets the remedy without a suggestion', () => { + const findings = validateActionBodyWrites( + stackWith("await ctx.api.object('crm_deal').update({ totally_unrelated_column: 1 });"), + ); + expect(findings).toHaveLength(1); + expect(findings[0].hint).toBe("Fix the field name, or declare 'totally_unrelated_column' on the object"); + }); + + it('every `fix:` line the cases above fired is one line of at most 200 characters', () => { + // Coverage control: every id fired, the unknown-field line with and + // without a suggestion. + expect([...new Set(firedShortened.map((f) => f.rule))].sort()).toEqual([...SHORTENED_RULE_IDS].sort()); + const unknown = firedShortened.filter((f) => f.rule === ACTION_BODY_WRITE_UNKNOWN_FIELD); + expect(unknown.some((f) => f.hint.startsWith('Did you mean'))).toBe(true); + expect(unknown.some((f) => f.hint.startsWith('Fix the field name'))).toBe(true); + for (const f of firedShortened) { + expect(f.hint, f.rule).not.toContain('\n'); + expect(f.hint.length, `${f.rule}: ${f.hint}`).toBeLessThanOrEqual(200); + } + }); + + const MOVED_FROM_FIX: Record = { + [ACTION_BODY_WRITE_UNPROVISIONED_ANCHOR]: ['`columnMap`', "`ownership: 'none'`", '`systemFields: { audit: false }`'], + [ACTION_BODY_WRITE_UNKNOWN_FIELD]: ['`ACTION_BODY_WRITE_PATTERNS`', 'params bag', 'never blocks a build'], + [ACTION_RECORD_WRITE_DISCARDED]: ['provably dead', 'Handed to anything', 'never blocks a build'], + [ACTION_BODY_SOURCE_UNPARSEABLE]: ['never executed', 'partially recovered'], + }; + + it.each([...SHORTENED_RULE_IDS])('`os explain %s` carries what its `fix:` line no longer says', (rule) => { + const text = explanationOf(rule); + for (const fact of MOVED_FROM_FIX[rule]) expect(text, `${rule} explanation names ${fact}`).toContain(fact); + }); +}); diff --git a/packages/lint/src/validate-action-body-writes.ts b/packages/lint/src/validate-action-body-writes.ts index 134c72868e9..47f1a6942b7 100644 --- a/packages/lint/src/validate-action-body-writes.ts +++ b/packages/lint/src/validate-action-body-writes.ts @@ -83,8 +83,8 @@ import { findClosestMatches, formatSuggestion } from '@objectstack/spec/shared'; -import { PARSE_FAILURE_HINT } from './checked-parse.js'; -import { indexUnprovisionedAnchors, unprovisionedAnchorHint } from './system-fields.js'; +import { PARSE_FAILURE_FIX } from './checked-parse.js'; +import { indexUnprovisionedAnchors, unprovisionedAnchorFix } from './system-fields.js'; import { bodyParseFailureVerdict, extractHookBodyWriteSet, @@ -339,9 +339,10 @@ export function validateActionBodyWrites(stack: AnyRec): ActionBodyWriteFinding[ rule: ACTION_BODY_SOURCE_UNPARSEABLE, where, path: site.path, - // [#22161] The hook twin's sentence, from the one function both use. + // [#22161] The hook twin's sentence, from the one function both use; + // so is the one-line fix. message: bodyParseFailureVerdict(parseFailure), - hint: PARSE_FAILURE_HINT, + hint: PARSE_FAILURE_FIX, }); } @@ -362,18 +363,17 @@ 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 + // [#22161] One verdict sentence and one remedy line; that the + // snapshot is read-only by design whether or not the field is + // declared, why only a provably dead write is reported, and that the + // warning never blocks a build, are // `os explain action-record-write-discarded`. message: `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('').updateById(ctx.recordId, ` + - `{ ${w.field}: … }). Reported only because ctx.record is never passed anywhere in this body — ` + - `mutating the snapshot and then handing it to an API write is a live payload and is not flagged. ` + - `This warning never blocks a build.`, + `{ ${w.field}: … })`, }); } } @@ -408,7 +408,7 @@ export function validateActionBodyWrites(stack: AnyRec): ActionBodyWriteFinding[ where, path: site.path, message: unprovisionedAnchorWriteVerdict(w.object, w.field, `the body's ${w.method ?? 'update'}(…) write`), - hint: unprovisionedAnchorHint(w.object, w.field), + hint: unprovisionedAnchorFix(w.object, w.field), }); continue; } @@ -436,10 +436,8 @@ export function validateActionBodyWrites(stack: AnyRec): ActionBodyWriteFinding[ /** Did-you-mean (declared + system columns as candidates) plus the fix. */ function fixHint(field: string, declared: string[]): string { const suggestion = formatSuggestion(findClosestMatches(field, [...declared, ...IMPLICIT_FIELDS])); - return ( - (suggestion ? `${suggestion} ` : '') + - `Fix the field name, or declare '${field}' on the object. Only the literal write patterns in ` + - `ACTION_BODY_WRITE_PATTERNS are checked — an action's ctx.input is its params bag, so it is not a ` + - `record-write surface and is never resolved against fields — and this warning never blocks a build.` - ); + // [#22161] One remedy line, the hook sibling's; which write shapes are + // checked, why `ctx.input` is not one, and that the warning never blocks a + // build, are `os explain action-body-write-unknown-field`. + return `${suggestion ? `${suggestion} ` : ''}Fix the field name, or declare '${field}' on the object`; } diff --git a/packages/lint/src/validate-ai-agent-authoring.test.ts b/packages/lint/src/validate-ai-agent-authoring.test.ts index c60e137aeae..5d8f45fd16f 100644 --- a/packages/lint/src/validate-ai-agent-authoring.test.ts +++ b/packages/lint/src/validate-ai-agent-authoring.test.ts @@ -14,7 +14,7 @@ import { explainRule } from './rule-explanations.js'; // the last cases in this file hold each recorded verdict to one line of at most // 200 characters — every firing variant this suite exercises, not a chosen few. // Run the whole file: those cases read what the cases above fired. -const fired: Array<{ rule: string; message: string }> = []; +const fired: Array<{ rule: string; message: string; hint: string }> = []; const validateAiAgentAuthoring: typeof validateAiAgentAuthoringUnrecorded = (...args) => { const findings = validateAiAgentAuthoringUnrecorded(...args); fired.push(...findings); @@ -54,7 +54,7 @@ describe('validate-ai-agent-authoring', () => { it('omits the skill sentence when the agent references none', () => { const findings = validateAiAgentAuthoring({ agents: [{ name: 'lonely' }] }); expect(findings).toHaveLength(1); - expect(findings[0].hint).not.toContain('skills this agent references'); + expect(findings[0].hint).not.toContain('it references'); }); it('uses distinct wording when a stack shadows a platform agent id', () => { @@ -280,3 +280,46 @@ describe('[#22161] one-line verdicts — the three agent-authoring ids', () => { expect(explainRule(AGENT_AUTHORING_WITHDRAWN)!.paragraphs[0]).toBe(roster); }); }); + +// [#22161] Slice 15: the `fix:` line of the two ids whose line ran long. Each +// names its remedy in one line; that skills attach to the platform agents by +// `surface` affinity, and that dropping the persona loses nothing a skill +// carries, are the explanations'. +describe('[#22161] one-line fixes — agent-authoring-withdrawn and default-agent-outside-roster', () => { + const SHORTENED = [AGENT_AUTHORING_WITHDRAWN, DEFAULT_AGENT_OUTSIDE_ROSTER]; + + it('counts one referenced skill in the singular', () => { + const [finding] = validateAiAgentAuthoring({ agents: [{ name: 'solo', skills: ['live_data'] }] }); + expect(finding!.hint).toBe( + 'Delete the agent and express its capability as skills, moving the useful parts of its `instructions` ' + + 'into theirs; the 1 skill it references already carries it', + ); + }); + + it('every `fix:` line the cases above fired is one line of at most 200 characters', () => { + // Coverage control: both ids fired, the declaration id on both arms and + // with and without referenced skills. + expect([...new Set(fired.map((f) => f.rule).filter((r) => SHORTENED.includes(r)))].sort()).toEqual( + [...SHORTENED].sort(), + ); + const declared = fired.filter((f) => f.rule === AGENT_AUTHORING_WITHDRAWN); + expect(declared.some((f) => f.hint.startsWith('Remove the declaration'))).toBe(true); + expect(declared.some((f) => f.hint.endsWith('already carry it'))).toBe(true); + expect(declared.some((f) => f.hint.endsWith('into theirs'))).toBe(true); + for (const f of fired.filter((x) => SHORTENED.includes(x.rule))) { + expect(f.hint, f.rule).not.toContain('\n'); + expect(f.hint.length, `${f.rule}: ${f.hint}`).toBeLessThanOrEqual(200); + } + }); + + it('`os explain` carries what the `fix:` lines no longer say', () => { + const facts: Record = { + [AGENT_AUTHORING_WITHDRAWN]: ['by `surface` affinity', 'persona text', 'nothing is lost by dropping the persona'], + [DEFAULT_AGENT_OUTSIDE_ROSTER]: ['by `surface` affinity', 'not through a custom agent or a custom `defaultAgent` value'], + }; + for (const [rule, list] of Object.entries(facts)) { + const text = explainRule(rule)?.paragraphs.join('\n') ?? ''; + for (const fact of list) expect(text, `${rule} explanation names ${fact}`).toContain(fact); + } + }); +}); diff --git a/packages/lint/src/validate-ai-agent-authoring.ts b/packages/lint/src/validate-ai-agent-authoring.ts index 70a0f8b4992..c662147533b 100644 --- a/packages/lint/src/validate-ai-agent-authoring.ts +++ b/packages/lint/src/validate-ai-agent-authoring.ts @@ -160,8 +160,9 @@ export function validateAiAgentAuthoring(stack: AnyRec): AiAgentAuthoringFinding rule: AGENT_AUTHORING_WITHDRAWN, where: `agent "${name}"`, path: `agents[${ai}]`, - // [#22161] One verdict per arm; the withdrawal, how the runtime enforces - // it and why this is a warning are `os explain` text. + // [#22161] One verdict and one remedy line per arm; the withdrawal, how + // the runtime enforces it, why this is a warning, and why dropping the + // persona loses nothing a skill carries, are `os explain` text. message: isPlatformName ? `This stack declares an agent named "${name}", which is a PLATFORM agent id, so the ` + `runtime serves its own record for that name and this declaration has no effect` @@ -170,13 +171,11 @@ export function validateAiAgentAuthoring(stack: AnyRec): AiAgentAuthoringFinding `so it never runs`, hint: isPlatformName ? `Remove the declaration; the platform owns "${name}". Extend it with skills instead.` - : `Delete the agent and express its capability as skills. Everything an agent carried ` + - `that a skill does not is persona text: move the useful parts of \`instructions\` into ` + - `the skills' own instructions.` + + : `Delete the agent and express its capability as skills, moving the useful parts of its ` + + `\`instructions\` into theirs` + (skillCount > 0 - ? ` The ${skillCount} skill${skillCount === 1 ? '' : 's'} this agent references ` + - `already carry the capability — they attach to the platform agent by \`surface\` ` + - `affinity, so nothing is lost by dropping the persona.` + ? `; the ${skillCount} skill${skillCount === 1 ? '' : 's'} it references already ` + + `${skillCount === 1 ? 'carries' : 'carry'} it` : ``), }); } @@ -218,15 +217,15 @@ export function validateAiAgentAuthoring(stack: AnyRec): AiAgentAuthoringFinding rule: DEFAULT_AGENT_OUTSIDE_ROSTER, where: `app "${appName}".defaultAgent`, path: `apps[${appIdx}].defaultAgent`, - // [#22161] One verdict; how the key resolves and why it is not a schema - // enum are `os explain` text. + // [#22161] One verdict and one remedy line; how the key resolves, why it + // is not a schema enum, and how skills reach the platform agents, are + // `os explain` text. message: `app "${appName}" pins \`defaultAgent\` to "${defaultAgent}", which is not a platform ` + `agent (${roster}), so it silently falls back to the platform default and the pin has no effect`, hint: - `Set \`defaultAgent\` to one of the platform agent names: ${roster}. If the goal is a ` + - `dedicated persona or capability, express it as skills instead — they attach to "ask" ` + - `/ "build" by surface affinity, not as a custom \`defaultAgent\` value.`, + `Set \`defaultAgent\` to a platform agent name (${roster}); for a dedicated persona or ` + + `capability, author skills instead`, }); } diff --git a/packages/lint/src/validate-flow-node-writes.test.ts b/packages/lint/src/validate-flow-node-writes.test.ts index f2690e460fb..2e55dca6cf4 100644 --- a/packages/lint/src/validate-flow-node-writes.test.ts +++ b/packages/lint/src/validate-flow-node-writes.test.ts @@ -26,7 +26,7 @@ import { explainRule } from './rule-explanations.js'; // not a chosen few. Run the whole file: that case reads what the cases above // fired. const SHORTENED_RULE_IDS: readonly string[] = [FLOW_NODE_WRITE_UNPROVISIONED_ANCHOR, FLOW_NODE_WRITE_UNKNOWN_FIELD]; -const firedShortened: Array<{ rule: string; message: string }> = []; +const firedShortened: Array<{ rule: string; message: string; hint: string }> = []; const validateFlowNodeWrites: typeof validateFlowNodeWritesUnrecorded = (...args) => { const findings = validateFlowNodeWritesUnrecorded(...args); for (const f of findings) if (SHORTENED_RULE_IDS.includes(f.rule)) firedShortened.push(f); @@ -632,3 +632,38 @@ describe('[#22161] one-line verdicts — the rule ids this file shortened', () = for (const fact of MOVED[rule]) expect(text, `${rule} explanation names ${fact}`).toContain(fact); }); }); + +// [#22161] Slice 15: the `fix:` line of the two ids. Each names its remedy in +// one line; why the unknown-field finding gates where the hook and action body +// rules advise, and how to vouch for or opt out of an unprovisioned anchor, are +// the explanations'. +describe('[#22161] one-line fixes — the rule ids this file shortened', () => { + it('an unknown field with no near miss gets the remedy without a suggestion', () => { + const findings = validateFlowNodeWrites({ objects: [dealObject], flows: [flowWith({ totally_unrelated_column: 1 })] }); + expect(findings).toHaveLength(1); + expect(findings[0].hint).toBe("Fix the field name, or declare 'totally_unrelated_column' on the object"); + }); + + it('every `fix:` line the cases above fired is one line of at most 200 characters', () => { + // Coverage control: every id fired, the unknown-field line with and + // without a suggestion. + expect([...new Set(firedShortened.map((f) => f.rule))].sort()).toEqual([...SHORTENED_RULE_IDS].sort()); + const unknown = firedShortened.filter((f) => f.rule === FLOW_NODE_WRITE_UNKNOWN_FIELD); + expect(unknown.some((f) => f.hint.startsWith('Did you mean'))).toBe(true); + expect(unknown.some((f) => f.hint.startsWith('Fix the field name'))).toBe(true); + for (const f of firedShortened) { + expect(f.hint, f.rule).not.toContain('\n'); + expect(f.hint.length, `${f.rule}: ${f.hint}`).toBeLessThanOrEqual(200); + } + }); + + const MOVED_FROM_FIX: Record = { + [FLOW_NODE_WRITE_UNPROVISIONED_ANCHOR]: ['`columnMap`', "`ownership: 'none'`", '`systemFields: { audit: false }`'], + [FLOW_NODE_WRITE_UNKNOWN_FIELD]: ['This rule gates where the hook and action body rules advise', 'nothing here is parsed'], + }; + + it.each([...SHORTENED_RULE_IDS])('`os explain %s` carries what its `fix:` line no longer says', (rule) => { + const text = explainRule(rule)?.paragraphs.join('\n') ?? ''; + for (const fact of MOVED_FROM_FIX[rule]) expect(text, `${rule} explanation names ${fact}`).toContain(fact); + }); +}); diff --git a/packages/lint/src/validate-flow-node-writes.ts b/packages/lint/src/validate-flow-node-writes.ts index 99582c11f40..084b6a3acba 100644 --- a/packages/lint/src/validate-flow-node-writes.ts +++ b/packages/lint/src/validate-flow-node-writes.ts @@ -95,7 +95,7 @@ import { UNDECLARED_FIELD_WRITE_REFUSAL, unprovisionedAnchorWriteVerdict, } from './validate-hook-body-writes.js'; -import { indexUnprovisionedAnchors, unprovisionedAnchorHint } from './system-fields.js'; +import { indexUnprovisionedAnchors, unprovisionedAnchorFix } from './system-fields.js'; import { walkFlowNodes, flowNodeLabel } from './flow-walk.js'; import { recordsOf } from './object-graph.js'; @@ -264,7 +264,7 @@ export function validateFlowNodeWrites(stack: AnyRec): FlowNodeWriteFinding[] { where: `flow "${flowName}" › ${nodeWhere}`, path: `${nodePath}.config.fields.${fieldName}`, message: unprovisionedAnchorWriteVerdict(objectName, fieldName, `this ${node.type} write`), - hint: unprovisionedAnchorHint(objectName, fieldName), + hint: unprovisionedAnchorFix(objectName, fieldName), }); continue; } @@ -304,10 +304,7 @@ export function validateFlowNodeWrites(stack: AnyRec): FlowNodeWriteFinding[] { /** Did-you-mean (declared + system columns as candidates) plus the fix. */ function fixHint(field: string, declared: string[]): string { const suggestion = formatSuggestion(findClosestMatches(field, [...declared, ...IMPLICIT_FIELDS])); - return ( - (suggestion ? `${suggestion} ` : '') + - `Fix the field name, or declare '${field}' on the object. This gates the build rather than warning: ` + - `the key is literal and so is the object, so unlike the hook/action body rules there is nothing here ` + - `that could have been mis-extracted.` - ); + // [#22161] One remedy line; why this gates where the hook and action body + // rules advise is `os explain flow-node-write-unknown-field`. + return `${suggestion ? `${suggestion} ` : ''}Fix the field name, or declare '${field}' on the object`; } diff --git a/packages/lint/src/validate-managed-api-methods.test.ts b/packages/lint/src/validate-managed-api-methods.test.ts index 2d89b160800..95dbd952bee 100644 --- a/packages/lint/src/validate-managed-api-methods.test.ts +++ b/packages/lint/src/validate-managed-api-methods.test.ts @@ -23,7 +23,7 @@ import { // recorded verdict to one line of at most 200 characters — every firing // variant this suite exercises, not a chosen few. Run the whole file: those // cases read what the cases above fired. -const fired: Array<{ rule: string; message: string }> = []; +const fired: Array<{ rule: string; message: string; hint: string }> = []; const validateManagedApiMethods: typeof validateManagedApiMethodsUnrecorded = (...args) => { const findings = validateManagedApiMethodsUnrecorded(...args); fired.push(...findings); @@ -54,9 +54,12 @@ describe('validateManagedApiMethods — the finding', () => { it('offers both ways out, and names the affordances that would be needed', () => { const [f] = validateManagedApiMethods({ objects: [sysEnvironment] }); expect(f.hint).toContain('userActions: { create: true, edit: true }'); - expect(f.hint).toContain('remove'); + expect(f.hint).toContain('Remove [create, update]'); // ADR-0092 D4 — an author must not open the affordance to silence a lint. - expect(f.hint).toContain('ADR-0092 D4'); + // [#22161] The line keeps the condition ("once a guard enforces the + // write"); the ruling it cites is the explanation's. + expect(f.hint).toContain('once a guard enforces the write'); + expect(explainRule(MANAGED_API_METHOD_UNAFFORDABLE)!.paragraphs.join('\n')).toContain('ADR-0092 D4'); }); it('emits ONE finding per object, not one per offending verb', () => { @@ -175,3 +178,31 @@ describe('[#22161] one-line verdicts — object/managed-api-method-unaffordable' } }); }); + +// [#22161] Slice 15: the `fix:` line names both ways out in one line, removal +// first; that an affordance opens only for a write a user context may really +// perform, once its guard exists (ADR-0092 D4), is the explanation's (pinned +// above). +describe('[#22161] one-line fix — object/managed-api-method-unaffordable', () => { + it('the longest line — every write verb, every affordance — stays within 200 characters', () => { + const [f] = validateManagedApiMethods({ + objects: [{ name: 'sys_locked', managedBy: 'better-auth', enable: { apiMethods: Object.keys(MANAGED_WRITE_VERB_AFFORDANCE) } }], + }); + expect(f!.hint).toBe( + 'Remove [create, update, upsert, delete, purge] from `enable.apiMethods`, or add ' + + '`userActions: { create: true, edit: true, delete: true }` once a guard enforces the write', + ); + }); + + it('every `fix:` line the cases in this file fired is one line of at most 200 characters', () => { + const converted = fired.filter((f) => f.rule === MANAGED_API_METHOD_UNAFFORDABLE); + // Coverage control: one verb, several verbs and every verb fired. + expect(converted.some((f) => f.hint.includes('[create, update]'))).toBe(true); + expect(converted.some((f) => f.hint.includes('[create, update, delete]'))).toBe(true); + expect(converted.some((f) => f.hint.includes('[create, update, upsert, delete, purge]'))).toBe(true); + for (const f of converted) { + expect(f.hint, f.rule).not.toContain('\n'); + expect(f.hint.length, `${f.rule}: ${f.hint}`).toBeLessThanOrEqual(200); + } + }); +}); diff --git a/packages/lint/src/validate-managed-api-methods.ts b/packages/lint/src/validate-managed-api-methods.ts index 6099a92d224..dc654f6cf7b 100644 --- a/packages/lint/src/validate-managed-api-methods.ts +++ b/packages/lint/src/validate-managed-api-methods.ts @@ -101,17 +101,17 @@ export function validateManagedApiMethods(stack: unknown): ManagedApiMethodFindi rule: MANAGED_API_METHOD_UNAFFORDABLE, where: `object "${obj.name}"`, path: `objects[${oi}].enable.apiMethods`, - // [#22161] One verdict sentence; `where` names the object. The verb → - // affordance table, the boot-time strip it predicts and why this - // refuses are `os explain object/managed-api-method-unaffordable`. + // [#22161] One verdict sentence and one remedy line; `where` names the + // object. The verb → affordance table, the boot-time strip it predicts, + // why this refuses, and that an affordance opens only for a write a user + // context may really perform (ADR-0092 D4) are + // `os explain object/managed-api-method-unaffordable`. message: `\`enable.apiMethods\` advertises [${verbs}], which \`managedBy: '${String(obj.def.managedBy)}'\` ` + `affordances refuse (needs userActions ${flags.join('/')}), so the registry strips them`, hint: - `Either add \`userActions: { ${flags.map((f) => `${f}: true`).join(', ')} }\` to the object ` + - `— only if the write is genuinely one a user context may perform, and only once the guard ` + - `enforcing it exists (ADR-0092 D4: affordance never ships ahead of the guard) — or remove ` + - `[${verbs}] from \`enable.apiMethods\`, which is what the runtime does for you today.`, + `Remove [${verbs}] from \`enable.apiMethods\`, or add ` + + `\`userActions: { ${flags.map((f) => `${f}: true`).join(', ')} }\` once a guard enforces the write`, }); } diff --git a/packages/lint/src/validate-nav-object-servability.test.ts b/packages/lint/src/validate-nav-object-servability.test.ts index e91cf2878f4..88f87cd4375 100644 --- a/packages/lint/src/validate-nav-object-servability.test.ts +++ b/packages/lint/src/validate-nav-object-servability.test.ts @@ -15,7 +15,7 @@ import { explainRule } from './rule-explanations.js'; // exercises, not a chosen few. Run the whole file: those cases read what the // cases above fired. Before this file the rule's only pin was the suite's // "every member actually runs" case in `reference-integrity-suite.test.ts`. -const fired: Array<{ rule: string; message: string }> = []; +const fired: Array<{ rule: string; message: string; hint: string }> = []; const validateNavObjectServability: typeof validateNavObjectServabilityUnrecorded = (...args) => { const findings = validateNavObjectServabilityUnrecorded(...args); fired.push(...findings); @@ -140,3 +140,27 @@ describe('[#22161] one-line verdicts — nav-object-unservable', () => { } }); }); + +// [#22161] Slice 15: the `fix:` line names both ways out and the exception in +// one line; why an object API-disabled on purpose makes the entry the mistake +// (credential material) is the explanation's (pinned above). +describe('[#22161] one-line fix — nav-object-unservable', () => { + it('reads as one remedy', () => { + const [f] = validateNavObjectServability(stackWith({ navigation: [objectNav('nav_tokens', 'crm_secret_token')] })); + expect(f!.hint).toBe( + 'Remove the nav entry, or make "crm_secret_token" listable (`enable.apiEnabled: true` and `list` in ' + + '`enable.apiMethods`) unless it is API-disabled on purpose', + ); + }); + + it('every `fix:` line the cases in this file fired is one line of at most 200 characters', () => { + const converted = fired.filter((f) => f.rule === NAV_OBJECT_UNSERVABLE); + // Coverage control: both arms fired. + expect(converted.some((f) => f.message.includes('is false'))).toBe(true); + expect(converted.some((f) => f.message.includes('lacks `list`'))).toBe(true); + for (const f of converted) { + expect(f.hint, f.rule).not.toContain('\n'); + expect(f.hint.length, `${f.rule}: ${f.hint}`).toBeLessThanOrEqual(200); + } + }); +}); diff --git a/packages/lint/src/validate-nav-object-servability.ts b/packages/lint/src/validate-nav-object-servability.ts index 9857b265597..1862f69f680 100644 --- a/packages/lint/src/validate-nav-object-servability.ts +++ b/packages/lint/src/validate-nav-object-servability.ts @@ -149,19 +149,19 @@ export function validateNavObjectServability(stack: unknown): NavObjectServabili // key that condemns it is named in the message, because the fix // may belong at either end. path: `${navPath}.objectName`, - // [#22161] One verdict sentence naming the editable key; the two - // refusal envelopes, why no permission on the entry can rescue - // it and what the served `/meta` does with it is + // [#22161] One verdict sentence naming the editable key, and one + // remedy line; the two refusal envelopes, why no permission on + // the entry can rescue it, what the served `/meta` does with it, + // and why an object API-disabled on purpose (credential + // material) makes the entry the mistake, are // `os explain nav-object-unservable`. message: `Navigation targets object "${target}", which cannot serve a list ` + `(\`${offendingKey}\` ${apiDisabled ? 'is false' : 'lacks `list`'}), so the server prunes ` + `the entry: its list answers ${apiDisabled ? '404' : '405'} for every user`, hint: - `Remove the nav entry, or make "${target}" listable by setting \`enable.apiEnabled: true\` ` - + `and granting \`list\` in \`enable.apiMethods\`. ⛔ Do NOT open the API on an object that ` - + `is disabled on purpose — several platform objects hold credential material and are ` - + `API-disabled deliberately; for those the entry is the mistake, not the \`enable\` block.`, + `Remove the nav entry, or make "${target}" listable (\`enable.apiEnabled: true\` and \`list\` ` + + `in \`enable.apiMethods\`) unless it is API-disabled on purpose`, }); } } diff --git a/packages/lint/src/validate-react-page-props.test.ts b/packages/lint/src/validate-react-page-props.test.ts index 7d54c6cbb95..b388ebeec9e 100644 --- a/packages/lint/src/validate-react-page-props.test.ts +++ b/packages/lint/src/validate-react-page-props.test.ts @@ -25,7 +25,13 @@ import { PAGE_FIELD_UNKNOWN, PAGE_FIELD_UNPROVISIONED } from './validate-page-fi // The gate PARSES `ChartAggregateSchema` since #5020, so the function // vocabulary lives in the schema and nowhere in the rule. Imported here to pin // the test table against it — see the `aggregate` block near the bottom. -import { ChartAggregateFunctionSchema, ChartAggregateSchema, ChartDrillDownSchema } from '@objectstack/spec/ui'; +import { + ChartAggregateFunctionSchema, + ChartAggregateSchema, + ChartDrillDownSchema, + REACT_BLOCKS, + REACT_RECORD_BLOCK_ALTERNATIVES, +} from '@objectstack/spec/ui'; import { explainRule } from './rule-explanations.js'; // [#22161] Each finding of the five ids below is one verdict sentence; the @@ -1747,3 +1753,85 @@ describe('[#22161] one-line fixes — react-chart-drilldown-invalid, react-chart ); }); }); + +// [#22161] Slice 15: the `fix:` line of four more ids. The anchor and parse +// ids converge on the shared one-line forms (`unprovisionedAnchorFix()`, +// `PARSE_FAILURE_FIX`); the record-block line is the spec's replacement behind +// a one-word lead-in; a missing required binding quotes the first sentence of +// the contract's description, which names the binding and its spelling. How to +// vouch for or opt out of an anchor, what a partially recovered tree leaves +// unchecked, and why the replacement binds the record itself are the +// explanations'. +describe('[#22161] one-line fixes — four more react-page ids', () => { + const MISSING_REQUIRED = 'react-prop-missing-required'; + const FIX_IDS: readonly string[] = [ + REACT_CHART_FIELD_UNPROVISIONED, + MISSING_REQUIRED, + REACT_BLOCK_NEEDS_RECORD_CONTEXT, + REACT_PAGE_SOURCE_UNPARSEABLE, + ]; + + it('every `fix:` line of the four ids the cases above fired is one line of at most 200 characters', () => { + const converted = fired.filter((f) => FIX_IDS.includes(f.rule)); + // Coverage control: every id fired, the record-block id with a named + // replacement and with the generic record-page advice. + for (const id of FIX_IDS) expect(converted.some((f) => f.rule === id), `${id} fired`).toBe(true); + const record = converted.filter((f) => f.rule === REACT_BLOCK_NEEDS_RECORD_CONTEXT); + expect(record.some((f) => f.hint.startsWith('Instead: <'))).toBe(true); + expect(record.some((f) => f.hint.startsWith("Instead: author the page as `type:'record'`"))).toBe(true); + for (const f of converted) { + expect(f.hint, f.rule).not.toContain('\n'); + expect(f.hint.length, `${f.rule}: ${f.hint}`).toBeLessThanOrEqual(200); + } + }); + + it("every replacement the spec names for a record block fits the line, behind the lead-in", () => { + // The spec's table is the source: a replacement it grows past the bound + // turns this red rather than printing a second line. + const types = Object.keys(REACT_RECORD_BLOCK_ALTERNATIVES); + expect(types.length).toBeGreaterThan(0); + for (const type of types) { + const [f] = validateReactPagePropsUnrecorded(page(`function Page(){ return ; }`)); + expect(f?.rule, type).toBe(REACT_BLOCK_NEEDS_RECORD_CONTEXT); + expect(f!.hint, type).toBe(`Instead: ${REACT_RECORD_BLOCK_ALTERNATIVES[type]}`); + expect(f!.hint.length, `${type}: ${f!.hint}`).toBeLessThanOrEqual(200); + } + }); + + it("every required binding the contract publishes gets a one-line `fix:`, led by the binding's spelling", () => { + // The contract (`REACT_BLOCKS`) is the source: each block with a required + // binding is rendered with no props at all, and every missing-binding line + // is held to the bound. A description whose first sentence grows past it + // turns this red. + const required = (REACT_BLOCKS as Array<{ tag: string; interactions: Array<{ name: string; required?: boolean }> }>) + .flatMap((b) => b.interactions.filter((i) => i.required).map((i) => [b.tag, i.name] as const)); + expect(required.length).toBeGreaterThan(0); + for (const [tag, prop] of required) { + const lines = validateReactPagePropsUnrecorded(page(`function Page(){ return <${tag} />; }`)).filter( + (f) => f.rule === MISSING_REQUIRED && f.message.includes(`"${prop}"`), + ); + expect(lines, `<${tag}> ${prop}`).toHaveLength(1); + expect(lines[0]!.hint, `<${tag}> ${prop}`).toMatch(new RegExp(`^Pass ${prop}=\\{…\\}`)); + expect(lines[0]!.hint.length, `<${tag}> ${prop}: ${lines[0]!.hint}`).toBeLessThanOrEqual(200); + } + }); + + it("a description of several sentences contributes its first, which names the spelling", () => { + const [f] = validateReactPagePropsUnrecorded(page(`function Page(){ return ; }`)); + expect(f!.hint).toBe( + "Pass data={…}: The object this block binds to, written as the metadata-tier data source: " + + "data={{ provider: 'object', object: '…' }}", + ); + }); + + const MOVED_FROM_FIX: Record = { + [REACT_CHART_FIELD_UNPROVISIONED]: ['`columnMap`', "`ownership: 'none'`", '`systemFields: { audit: false }`'], + [REACT_BLOCK_NEEDS_RECORD_CONTEXT]: ['the replacement binds it itself', 'ordinary React state'], + [REACT_PAGE_SOURCE_UNPARSEABLE]: ['never executed', 'partially recovered'], + }; + + it.each(Object.keys(MOVED_FROM_FIX))('`os explain %s` carries what its `fix:` line no longer says', (rule) => { + const text = explanationOf(rule); + for (const fact of MOVED_FROM_FIX[rule]) expect(text, `${rule} explanation names ${fact}`).toContain(fact); + }); +}); diff --git a/packages/lint/src/validate-react-page-props.ts b/packages/lint/src/validate-react-page-props.ts index 5e00b833203..700d05f7d53 100644 --- a/packages/lint/src/validate-react-page-props.ts +++ b/packages/lint/src/validate-react-page-props.ts @@ -63,7 +63,7 @@ import { // #5020's zod-rejection renderer, shared with the SDUI component-props gate // since #5068 — see `zod-issue-format.ts` for why one copy matters here. import { describeIssue, type LintZodIssue } from './zod-issue-format.js'; -import { createSourceFileChecked, describeParseFailure, PARSE_FAILURE_HINT } from './checked-parse.js'; +import { createSourceFileChecked, describeParseFailure, PARSE_FAILURE_FIX } from './checked-parse.js'; // [#22161] The verdict half of a forwarded `@objectstack/spec` refusal — one // cut for every rule that forwards the same producer's words. import { schemaRefusalHead } from './validate-flow-trigger-readiness.js'; @@ -71,7 +71,7 @@ import { schemaRefusalHead } from './validate-flow-trigger-readiness.js'; import { SYSTEM_FIELDS, indexUnprovisionedAnchors, - unprovisionedAnchorHint, + unprovisionedAnchorFix, unprovisionedAnchorVerdict, } from './system-fields.js'; import { collectionEntries } from './collection-entries.js'; @@ -167,6 +167,18 @@ const BLOCKS: Map = new Map( ]), ); +/** + * [#22161] The first sentence of a contract description, for a one-line + * `fix:`: the cut `schemaRefusalHead` makes for a forwarded spec refusal (the + * text up to the first `. `), without the rename question a refusal can carry. + * A description of one sentence is quoted whole. + */ +function descriptionHead(description: string): string { + const text = description.replace(/\s+/g, ' ').trim(); + const cut = text.indexOf('. '); + return cut === -1 ? text : text.slice(0, cut); +} + function editDistance(a: string, b: string, cap = 2): number { if (Math.abs(a.length - b.length) > cap) return cap + 1; const dp = Array.from({ length: a.length + 1 }, (_, i) => i); @@ -606,14 +618,15 @@ function checkObjectChart( // error, so the chart renders and says nothing true. if (anchors?.has(name)) { // [#22161] One verdict sentence, in the shared short form of the - // anchor cause; ADR-0015's federation and why the query does not fail - // are `os explain react-chart-field-unprovisioned`. + // anchor cause, and the shared one-line fix; ADR-0015's federation, + // why the query does not fail and how to vouch for or opt out of the + // anchor are `os explain react-chart-field-unprovisioned`. push( 'warning', REACT_CHART_FIELD_UNPROVISIONED, `aggregate.${prop} "${name}": ${unprovisionedAnchorVerdict(objectName, name)}, so the chart ` + (prop === 'groupBy' ? 'groups everything into one empty bucket' : 'aggregates nothing'), - unprovisionedAnchorHint(objectName, name), + unprovisionedAnchorFix(objectName, name), ); } return; @@ -1063,13 +1076,13 @@ function recordContextFinding( where, path, // [#22161] One verdict sentence; why binding the block does not help is - // `os explain react-block-needs-record-context`. + // `os explain react-block-needs-record-context`. The `fix:` line is the + // spec's replacement for the block, as one line: that the replacement + // binds the record itself is the explanation's, so the lead-in is a word. message: `<${tag}> renders "${schemaType}", which reads its record from a record page's context; a ` + `kind:'react' page mounts none, so the block renders empty`, - hint: - `On a react page bind the record yourself: ` + - `${REACT_RECORD_BLOCK_ALTERNATIVES[schemaType] ?? RECORD_BLOCK_GENERIC_FIX}`, + hint: `Instead: ${REACT_RECORD_BLOCK_ALTERNATIVES[schemaType] ?? RECORD_BLOCK_GENERIC_FIX}`, }; } @@ -1138,12 +1151,13 @@ export function validateReactPageProps(stack: AnyRec): ReactPropFinding[] { rule: REACT_PAGE_SOURCE_UNPARSEABLE, where: `page "${name}"`, path: `${pagePath}.source`, - // [#22161] One verdict sentence; what a partially recovered tree means - // for the checks is `os explain react-page-source-unparseable`. + // [#22161] One verdict sentence and the shared one-line fix; what a + // partially recovered tree means for the checks is + // `os explain react-page-source-unparseable`. message: `kind:'react' source did not parse (${describeParseFailure(failure)}), so the ` + `component-contract checks may have missed problems in it`, - hint: PARSE_FAILURE_HINT, + hint: PARSE_FAILURE_FIX, }); } @@ -1217,12 +1231,16 @@ export function validateReactPageProps(stack: AnyRec): ReactPropFinding[] { // write it — the author gets the spelling, not a pointer. // [#22161] `hint` is the CLI's `fix:` line, and a description // alone ("The registered component type to render.") is not a - // fix: the instruction leads, the description follows it. + // fix: the instruction leads, the description follows it — + // its first sentence, which names the binding and its spelling. + // What a description says after that (which providers are + // accepted, why the prop is required) is the contract's, and + // the published react-blocks reference carries it whole. hint: dep ? dep.note : `Pass ${req}={…}` + (block.requiredDescriptions.has(req) - ? `: ${block.requiredDescriptions.get(req)}` + ? `: ${descriptionHead(block.requiredDescriptions.get(req)!)}` : '. See the react-tier component contract.'), }); } diff --git a/packages/lint/src/validate-rule-compilability.test.ts b/packages/lint/src/validate-rule-compilability.test.ts index b558f31713e..35eac92da25 100644 --- a/packages/lint/src/validate-rule-compilability.test.ts +++ b/packages/lint/src/validate-rule-compilability.test.ts @@ -20,7 +20,7 @@ import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { describe, it, expect } from 'vitest'; import { ObjectStackSchema } from '@objectstack/spec'; -import { ObjectSchema } from '@objectstack/spec/data'; +import { FormatValidationSchema, ObjectSchema } from '@objectstack/spec/data'; import { validateRuleCompilability as validateRuleCompilabilityUnrecorded, @@ -39,7 +39,7 @@ import { explainRule } from './rule-explanations.js'; // this suite exercises, not a chosen few. Run the whole file: those cases read // what the cases above fired. const COMPILE_IDS: readonly string[] = [VALIDATION_RULE_REGEX_UNCOMPILABLE, VALIDATION_RULE_SCHEMA_UNCOMPILABLE]; -const fired: Array<{ rule: string; message: string }> = []; +const fired: Array<{ rule: string; message: string; hint: string }> = []; const validateRuleCompilability: typeof validateRuleCompilabilityUnrecorded = (...args) => { const findings = validateRuleCompilabilityUnrecorded(...args); fired.push(...findings); @@ -1007,3 +1007,54 @@ describe('[#22161] one-line verdicts — the two compile ids', () => { for (const fact of MOVED[rule]) expect(text, `${rule} explanation names ${fact}`).toContain(fact); }); }); + +// [#22161] Slice 15: the `fix:` line of the two ids. Each names its remedy in +// one line; the escaped spellings, the named formats, and the shapes the common +// JSON Schema keywords take are the explanations'. +describe('[#22161] one-line fixes — the two compile ids', () => { + it('every `fix:` line the cases above fired is one line of at most 200 characters', () => { + // Coverage control: both ids fired. + expect([...new Set(fired.map((f) => f.rule))].sort()).toEqual([...COMPILE_IDS].sort()); + for (const f of fired) { + expect(f.hint, f.rule).not.toContain('\n'); + expect(f.hint, f.rule).not.toMatch(/\babove\b/); + expect(f.hint.length, `${f.rule}: ${f.hint}`).toBeLessThanOrEqual(200); + } + }); + + it('the regex line quotes the source and names both ways out', () => { + const [f] = validateRuleCompilabilityUnrecorded( + objectWith({ type: 'format', name: 'fmt', field: 'tax_id', regex: '([', message: 'm' }), + ); + expect(f.hint).toBe( + "Fix the pattern so `new RegExp('([')` compiles (escape a literal `(`, `[` or `\\`), or drop `regex` and " + + 'use a named `format`', + ); + }); + + const MOVED_FROM_FIX: Record = { + [VALIDATION_RULE_REGEX_UNCOMPILABLE]: ['`\\\\(`, `\\\\[` and `\\\\\\\\`', "`'^\\\\d{2}-\\\\d{7}$'`"], + [VALIDATION_RULE_SCHEMA_UNCOMPILABLE]: ['`required` an array of strings', 'every `$ref` must resolve', 'MALFORMED standard keyword'], + }; + + it.each([...COMPILE_IDS])('`os explain %s` carries what its `fix:` line no longer says', (rule) => { + const text = explanationOf(rule); + for (const fact of MOVED_FROM_FIX[rule]) expect(text, `${rule} explanation names ${fact}`).toContain(fact); + }); + + it("the regex explanation's named formats are the schema's `format` enum, in order", () => { + // The explanation module imports nothing, so it spells the list out; this + // holds it to `FormatValidationSchema`. + const formats = (FormatValidationSchema.shape.format.unwrap() as { options: readonly string[] }).options; + expect(formats.length).toBeGreaterThan(0); + const spelled = `${formats.slice(0, -1).map((f) => `\`${f}\``).join(', ')} or \`${formats[formats.length - 1]}\`.`; + expect(explanationOf(VALIDATION_RULE_REGEX_UNCOMPILABLE)).toContain(`name a \`format\` instead: ${spelled}`); + }); + + it('the json_schema explanation names every JSON Schema primitive type', () => { + const text = explanationOf(VALIDATION_RULE_SCHEMA_UNCOMPILABLE); + for (const t of ['null', 'boolean', 'object', 'array', 'number', 'string', 'integer']) { + expect(text, t).toContain(`\`${t}\``); + } + }); +}); diff --git a/packages/lint/src/validate-rule-compilability.ts b/packages/lint/src/validate-rule-compilability.ts index 4114a3d4973..eddb8285afd 100644 --- a/packages/lint/src/validate-rule-compilability.ts +++ b/packages/lint/src/validate-rule-compilability.ts @@ -463,15 +463,14 @@ export function validateRuleCompilability(stack: unknown): RuleCompilabilityFind rule: VALIDATION_RULE_REGEX_UNCOMPILABLE, where, path: `${basePath}.regex`, - // [#22161] One verdict sentence; `where` names the rule and its - // object. How the write path compiles it and why it skips is + // [#22161] One verdict sentence and one remedy line; `where` names + // the rule and its object. How the write path compiles it and why it + // skips, the escaped spellings and the named formats are // `os explain validation-rule-regex-uncompilable`. message: `\`regex\` does not compile (${regexRefusal(rule.regex, err)}), ${SKIPPED_ON_EVERY_WRITE}`, hint: - `Fix the pattern so \`new RegExp('${rule.regex}')\` compiles — a literal \`(\`, \`[\` or \`\\\` ` + - `must be escaped (\`\\\\(\`, \`\\\\[\`, \`\\\\\\\\\`), and the source is a STRING, so a backslash ` + - `is written twice in TypeScript ('^\\\\d{2}-\\\\d{7}$'). Or drop \`regex\` and use a named ` + - `\`format\` ('email' | 'url' | 'phone' | 'json').`, + `Fix the pattern so \`new RegExp('${rule.regex}')\` compiles (escape a literal \`(\`, \`[\` or ` + + `\`\\\`), or drop \`regex\` and use a named \`format\``, }); } } @@ -486,15 +485,14 @@ export function validateRuleCompilability(stack: unknown): RuleCompilabilityFind rule: VALIDATION_RULE_SCHEMA_UNCOMPILABLE, where, path: `${basePath}.schema`, - // [#22161] One verdict sentence; the runtime's ajv environment and - // why the write path skips is + // [#22161] One verdict sentence and one remedy line; the runtime's + // ajv environment, why the write path skips, and the shapes the + // common keywords take are // `os explain validation-rule-json-schema-uncompilable`. message: `ajv cannot compile \`schema\` (${schemaRefusal(err)}), ${SKIPPED_ON_EVERY_WRITE}`, hint: - `Correct the schema so ajv compiles it — the message above names the offending keyword. ` + - `\`type\` must be one of null|boolean|object|array|number|string|integer (or an array of ` + - `those), \`required\` an array of strings, and every \`$ref\` must resolve. Vendor keywords ` + - `are fine (the runtime runs \`strict: false\`); a MALFORMED standard keyword is not.`, + `Correct the keyword the verdict names so ajv compiles the schema (a vendor keyword is fine; ` + + `a malformed standard one is not)`, }); } } diff --git a/packages/lint/src/validate-translatable-sections.test.ts b/packages/lint/src/validate-translatable-sections.test.ts index a118acd2653..c7d262a4480 100644 --- a/packages/lint/src/validate-translatable-sections.test.ts +++ b/packages/lint/src/validate-translatable-sections.test.ts @@ -27,7 +27,7 @@ import { // fired, and the last cases in this file hold each recorded verdict to one line // of at most 200 characters — every firing variant this suite exercises, not a // chosen few. Run the whole file: those cases read what the cases above fired. -const fired: Array<{ rule: string; message: string; path: string }> = []; +const fired: Array<{ rule: string; message: string; path: string; hint: string }> = []; const validateTranslatableSections: typeof validateTranslatableSectionsUnrecorded = (...args) => { const findings = validateTranslatableSectionsUnrecorded(...args); fired.push(...findings); @@ -116,10 +116,13 @@ describe('validateTranslatableSections — the HotCRM shape (#5417)', () => { expect(findings[0].hint).toContain("`name: 'case'`"); expect(findings[0].hint).toContain('objects.crm_case._sections.case.label'); expect(findings[2].hint).toContain("`name: 'resolution'`"); - // The renderer resolves by name only. Saying so in the hint is what keeps an - // author (or an AI author) from assuming the label text is itself a key — - // the slug-fishing fallback objectui#3373 pinned as WRONG. - expect(findings[0].hint).toContain('never a key derived from the label'); + // The renderer resolves by name only. Saying so is what keeps an author (or + // an AI author) from assuming the label text is itself a key — the + // slug-fishing fallback objectui#3373 pinned as WRONG. [#22161] Said in the + // explanation now, which `os explain` prints; the `fix:` line is the remedy. + expect(explainRule(TRANSLATION_SECTION_NAME_MISSING)!.paragraphs.join('\n')).toContain( + 'never a key derived from the label', + ); }); it('says nothing once the sections carry names', () => { @@ -570,3 +573,32 @@ describe('[#22161] one-line verdict — translation-section-name-missing', () => } }); }); + +// [#22161] Slice 15: the `fix:` line names the name to write and the key to +// translate, in one line; that the name is a suggestion to write down and never +// a key derived from the label is the explanation's (pinned above). +describe('[#22161] one-line fix — translation-section-name-missing', () => { + it('every `fix:` line the cases above fired is one line of at most 200 characters', () => { + const converted = fired.filter((f) => f.rule === TRANSLATION_SECTION_NAME_MISSING); + // Coverage control: the id fired, on a view's form sections and on a + // page's `record:details` sections. + expect(converted.some((f) => f.path.startsWith('views['))).toBe(true); + expect(converted.some((f) => f.path.includes('.properties.sections['))).toBe(true); + for (const f of converted) { + expect(f.hint, f.rule).not.toContain('\n'); + expect(f.hint.length, `${f.rule}: ${f.hint}`).toBeLessThanOrEqual(200); + } + }); + + it('reads as one remedy', () => { + const [first] = validateTranslatableSections({ + objects: [crmCase], + views: [{ name: 'case_views', formViews: { edit: { type: 'simple', data, sections: hotcrmSections } } }], + translations: caseTranslated, + }); + expect(first!.hint).toBe( + "Give the section a stable `name` (snake_case), e.g. `name: 'case'`, then translate it as " + + '`objects.crm_case._sections.case.label` in each locale bundle', + ); + }); +}); diff --git a/packages/lint/src/validate-translatable-sections.ts b/packages/lint/src/validate-translatable-sections.ts index 1da105c527b..7e44ab7fd5b 100644 --- a/packages/lint/src/validate-translatable-sections.ts +++ b/packages/lint/src/validate-translatable-sections.ts @@ -314,9 +314,9 @@ export function validateTranslatableSections(stack: AnyRec): TranslatableSection rule: TRANSLATION_SECTION_NAME_MISSING, where: joinWhere(`object "${objectName}"`, site.surface, `section "${heading}"`), path: `${site.path}[${i}]`, - // [#22161] One verdict sentence; why no gate sees the hole, and why - // the fix is a written name and never a derived key, are - // `os explain translation-section-name-missing`. + // [#22161] One verdict sentence and one remedy line; why no gate sees + // the hole, and why the fix is a written name and never a derived key, + // are `os explain translation-section-name-missing`. message: `Section "${heading}" declares a label but no \`name\`, so no ` + `\`objects.${objectName}._sections..label\` key can address it and its heading ` + @@ -325,9 +325,7 @@ export function validateTranslatableSections(stack: AnyRec): TranslatableSection `Give the section a stable \`name\` (snake_case)` + (slug ? `, e.g. \`name: '${slug}'\`` : '') + `, then translate it as \`objects.${objectName}._sections.${slug ?? ''}.label\` in ` + - `each locale bundle. The renderers look the heading up by name only — the name above ` + - `is a suggestion to write down, never a key derived from the label, so renaming the ` + - `heading later cannot break the lookup.`, + `each locale bundle`, }); } } From 5aaa606c9e25b74c60a5695b6d2acbab623a6f58 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 22:52:48 +0000 Subject: [PATCH 2/2] chore(changeset): @objectstack/lint patch for slice 15's one-line fix lines (WIP) Claude-Session: https://claude.ai/code/session_019G5ZnRrTMiwcHG6iZM19zf Co-authored-by: Claude --- .../22161-lint-slice-15-one-line-fix.md | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 .changeset/22161-lint-slice-15-one-line-fix.md diff --git a/.changeset/22161-lint-slice-15-one-line-fix.md b/.changeset/22161-lint-slice-15-one-line-fix.md new file mode 100644 index 00000000000..c5e024afe21 --- /dev/null +++ b/.changeset/22161-lint-slice-15-one-line-fix.md @@ -0,0 +1,28 @@ +--- +"@objectstack/lint": patch +--- + +fix(lint): 17 more author-time rule ids print a `fix:` line of one remedy sentence, and `os explain RULE_ID` carries what those lines used to explain + +Clause-②: no + +- **Shorter fix lines.** Each finding of these 17 rule ids now carries a `hint` (the CLI's `fix:` line, the runtime issue's `hint`) of one remedy sentence of at most 200 characters on every variant the lint suite fires (the lengths here are the `hint` alone; the CLI prints it after `fix: `). The longest of each, before → after: + - react pages: `react-chart-field-unprovisioned` (362 → 175), `react-prop-missing-required` (277 → 131), `react-page-source-unparseable` (254 → 105) and `react-block-needs-record-context` (228 → 195); + - action bodies: `action-body-write-unprovisioned-anchor` (358 → 171), `action-body-write-unknown-field` (323 → 91), `action-record-write-discarded` (309 → 104) and `action-body-source-unparseable` (254 → 105); + - agents: `agent-authoring-withdrawn` (363 → 158) and `default-agent-outside-roster` (237 → 118); + - flow nodes: `flow-node-write-unprovisioned-anchor` (358 → 171) and `flow-node-write-unknown-field` (289 → 110); + - object validation rules: `validation-rule-json-schema-uncompilable` (341 → 124) and `validation-rule-regex-uncompilable` (300 → 135); + - translations: `translation-section-name-missing` (359 → 172); + - managed objects: `object/managed-api-method-unaffordable` (357 → 169); + - app navigation: `nav-object-unservable` (348 → 156). +- **One line, one remedy.** Where a fix line explained its remedy, the explanation moved: how to vouch for an unprovisioned anchor (`columnMap`) or opt the object out of the injection (`ownership: 'none'`, `systemFields: { audit: false }`), now on the react-chart, action-body and flow-node anchor ids too; what a partially parsed source leaves unchecked; which action-body write shapes are checked and that the warning never blocks a build; why the flow-node unknown-field finding gates where the body rules advise; that skills attach to the platform agents by `surface` affinity, so dropping a custom agent loses nothing; the escaped regex spellings and the named `format`s; the shapes the common JSON Schema keywords take; that a suggested section name is never a key derived from the label; and that a record block's react-tier replacement binds the record itself. +- **Shared one-line forms.** The anchor and parse ids of the react-page, action-body and flow-node rules print the same one-line forms the hook-body ids print (`If the remote table carries '…', declare it in …'s own fields (mapped through columnMap); otherwise drop the reference or opt the object out of the injection`, and `Fix the syntax error and re-run: until the source parses, the checks cover only what the parser recovered`). +- **Quoted from the contract, cut to one sentence.** A missing required react-tier binding still prints the contract's own description of it, now its first sentence, which names the binding and its spelling (`Pass data={…}: The object this block binds to, written as the metadata-tier data source: data={{ provider: 'object', object: '…' }}`); the rest of the description is in the published react-blocks reference. A record block's line is the spec's named replacement behind the lead-in `Instead:`. +- **`os explain RULE_ID` carries the rest.** What the fix lines stopped saying is each id's `RULE_EXPLANATIONS` entry in `@objectstack/lint`, which `os explain RULE_ID` prints; eleven entries gain text. The named formats the regex entry spells out are held to the spec's `FormatValidationSchema` by the rule's test. `react-prop-missing-required` has no entry, as before, so its `rule:` line carries no pointer. +- **At the runtime publish gate** (Studio, REST `PUT /api/v1/meta`, and the MCP and AI authors that write through that door), 5 of the 17 fire; the `hint` FROM → TO, measured at the door: + - an `object` write: `object/managed-api-method-unaffordable` (422 `issues[].hint`, two refused verbs / every write verb 335 / 372 → 132 / 169), `validation-rule-regex-uncompilable` (422, the pattern `([` / a 28-character pattern 288 / 314 → 123 / 149) and `validation-rule-json-schema-uncompilable` (422, 341 → 124); + - a `flow` write: `flow-node-write-unknown-field` (422, with / without a near miss 271 / 250 → 92 / 71) and `flow-node-write-unprovisioned-anchor` (2xx `advisories[].hint`, 361 → 174). + + Studio's save-advisory toast prints an advisory's `hint` after its message, so the flow-node anchor toast shortens; Studio renders no 422 issue's `hint`. No Studio, REST or MCP reader in this repository or in objectui matches on `hint` text; a tool that did should match on `rule` and `path` instead. +- **Never at the runtime gate:** the four react-page ids, the four action-body ids, the two agent ids, `translation-section-name-missing` and `nav-object-unservable` run on the CLI only. +- **Unchanged:** every `message` (the verdict), rule id, severity and `path`, and what each rule accepts or refuses.