From 980d32271c77ebd7e64e0fd01e0ea2c5acd56e6a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 17:09:39 +0000 Subject: [PATCH 01/10] feat(lint,cli): one-line rule verdicts with an `os explain ` pointer field-no-consumers and security-owd-unset print one verdict sentence and one fix; their long reasoning moves into rule explanations keyed by rule id (@objectstack/lint, also published as the import-free `@objectstack/lint/rule-explanations` entry). `os explain` resolves a rule id after the schema names, and the CLI's `rule:` line names the command for the rules that have an explanation, from one spelling in utils/format.ts. `os validate` now prints the fix and rule lines under each registry warning, as `os build` does. Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude --- packages/cli/README.md | 2 +- packages/cli/src/commands/explain.ts | 73 +++++++++- packages/cli/src/commands/lint.ts | 6 +- packages/cli/src/commands/validate.ts | 15 ++- packages/cli/src/utils/format.ts | 41 +++++- packages/lint/package.json | 10 ++ packages/lint/src/index.ts | 6 + packages/lint/src/rule-explanations.test.ts | 79 +++++++++++ packages/lint/src/rule-explanations.ts | 126 ++++++++++++++++++ .../lint/src/rule-id-barrel-exports.test.ts | 12 +- .../lint/src/validate-field-consumers.test.ts | 62 +++++++-- packages/lint/src/validate-field-consumers.ts | 41 +++--- .../src/validate-security-posture.test.ts | 28 ++++ .../lint/src/validate-security-posture.ts | 14 +- packages/lint/tsup.config.ts | 8 +- 15 files changed, 464 insertions(+), 59 deletions(-) create mode 100644 packages/lint/src/rule-explanations.test.ts create mode 100644 packages/lint/src/rule-explanations.ts diff --git a/packages/cli/README.md b/packages/cli/README.md index 109a5f38d75..b2f2536adf1 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -163,7 +163,7 @@ The group has no `install`: ADR-0025 records the code-plugin install half (downl | Command | Description | |---------|-------------| -| `os explain [schema]` | Display human-readable explanation of an ObjectStack schema | +| `os explain [schema]` | Display a human-readable explanation of an ObjectStack schema, or of an author-time rule by its id (`os explain field-no-consumers`) | ## Configuration diff --git a/packages/cli/src/commands/explain.ts b/packages/cli/src/commands/explain.ts index 8e4845790bd..f07571ab158 100644 --- a/packages/cli/src/commands/explain.ts +++ b/packages/cli/src/commands/explain.ts @@ -2,6 +2,9 @@ import { Args, Command, Flags } from '@oclif/core'; import chalk from 'chalk'; +// [#22161] The long-form author-time rule explanations, from the data-only +// entry — `os explain object` does not pay for loading the rule engine. +import { RULE_EXPLANATIONS, explainRule, type RuleExplanation } from '@objectstack/lint/rule-explanations'; import { printHeader, printSuccess, @@ -406,13 +409,50 @@ export const SCHEMAS: Record = { }, }; +// ─── Rule explanations ───────────────────────────────────────────── + +/** Word-wrap one paragraph to `width` columns, each line indented by `indent`. */ +function wrapParagraph(text: string, width: number, indent: string): string[] { + const lines: string[] = []; + let line = ''; + for (const word of text.split(/\s+/).filter(Boolean)) { + if (line && indent.length + line.length + 1 + word.length > width) { + lines.push(indent + line); + line = word; + } else { + line = line ? `${line} ${word}` : word; + } + } + if (line) lines.push(indent + line); + return lines; +} + +/** + * [#22161] `os explain `: the reasoning an author-time finding no + * longer carries. A finding prints one verdict and one fix, and its `rule:` + * line points here for the rules `@objectstack/lint` explains. + */ +function printRuleExplanation(explanation: RuleExplanation): void { + printHeader(`Rule: ${explanation.rule}`); + for (const paragraph of explanation.paragraphs) { + console.log(''); + for (const line of wrapParagraph(paragraph, 88, ' ')) console.log(line); + } + console.log(''); +} + // ─── Command ──────────────────────────────────────────────────────── export default class Explain extends Command { - static override description = 'Display human-readable explanation of an ObjectStack schema'; + static override description = + 'Display a human-readable explanation of an ObjectStack schema, or of an author-time rule by its id'; static override args = { - schema: Args.string({ description: 'Schema name (e.g., object, field, view, flow, agent, app)', required: false }), + schema: Args.string({ + description: + 'Schema name (e.g., object, field, view, flow, agent, app) or author-time rule id (e.g., field-no-consumers)', + required: false, + }), }; static override flags = { @@ -423,7 +463,7 @@ export default class Explain extends Command { const { args, flags } = await this.parse(Explain); const schemaName = args.schema; - // ── No argument: list all schemas ── + // ── No argument: list all schemas, then the rules that have an explanation ── if (!schemaName) { if (flags.json) { await emitJson({ @@ -431,6 +471,7 @@ export default class Explain extends Command { name: key, description: s.description, })), + rules: Object.values(RULE_EXPLANATIONS).map((r) => ({ id: r.rule, covers: r.covers })), }); return; } @@ -442,21 +483,39 @@ export default class Explain extends Command { console.log(` ${chalk.bold.cyan(key.padEnd(12))} ${chalk.dim(desc.length > 70 ? desc.slice(0, 70) + '...' : desc)}`); } console.log(''); - printInfo(`Run ${chalk.white('objectstack explain ')} for details.`); + printHeader('Rule Explanations'); + console.log(''); + for (const rule of Object.values(RULE_EXPLANATIONS)) { + console.log(` ${chalk.bold.cyan(rule.rule)} ${chalk.dim(`— ${rule.covers}`)}`); + } + console.log(''); + printInfo(`Run ${chalk.white('objectstack explain ')} or ${chalk.white('objectstack explain ')} for details.`); console.log(''); return; } - // ── Lookup schema ── + // ── Lookup: a schema name first (as before), then an author-time rule id ── + // The two sets are disjoint (pinned in test/explain-rule-id.test.ts), so the + // order decides nothing today; it keeps every schema lookup exactly as it was. const schema = SCHEMAS[schemaName.toLowerCase()]; + const ruleExplanation = schema ? undefined : explainRule(schemaName); + if (!schema && ruleExplanation) { + if (flags.json) { + await emitJson(ruleExplanation); + return; + } + printRuleExplanation(ruleExplanation); + return; + } if (!schema) { if (flags.json) { - await emitJson({ error: `Unknown schema: ${schemaName}` }, 0, { compact: true }); + await emitJson({ error: `Unknown schema or rule id: ${schemaName}` }, 0, { compact: true }); process.exit(1); } - printError(`Unknown schema: "${schemaName}"`); + printError(`Unknown schema or rule id: "${schemaName}"`); console.log(''); printInfo(`Available schemas: ${Object.keys(SCHEMAS).join(', ')}`); + printInfo(`Rules with an explanation: ${Object.keys(RULE_EXPLANATIONS).join(', ')}`); console.log(''); process.exit(1); } diff --git a/packages/cli/src/commands/lint.ts b/packages/cli/src/commands/lint.ts index dad7d827724..0e2c8d22a5e 100644 --- a/packages/cli/src/commands/lint.ts +++ b/packages/cli/src/commands/lint.ts @@ -37,6 +37,7 @@ import { isExitSignal, errorCodeFields, isReportedError, + explainPointer, } from '../utils/format.js'; // ─── Types ────────────────────────────────────────────────────────── @@ -1122,7 +1123,10 @@ export default class Lint extends Command { 'ℹ'; console.log(` ${color(icon)} ${color(issue.message)}`); - console.log(chalk.dim(` ${issue.rule} at ${issue.path}`)); + // [#22161] The same `os explain ` pointer `os validate` and + // `os build` print, from the same one spelling, for the rules that have + // a long-form explanation. + console.log(chalk.dim(` ${issue.rule} at ${issue.path}${explainPointer(issue.rule)}`)); if (flags.fix && issue.fix) { console.log(chalk.green(` → fix: ${issue.fix}`)); } diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index 3f2d706b9f7..fbf91411e09 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -33,6 +33,8 @@ import { printStep, formatConversionNotice, printAuthoringRuleErrors, + authoringFindingDetailLines, + type AuthoringRuleFinding, printDocIssueErrors, JSON_FULL_LIST_REMEDY, createTimer, @@ -846,7 +848,14 @@ export default class Validate extends Command { // before, roughly half were printed inline and invisible to it, so // `--strict` failed or passed depending on which gate happened to raise // the finding — a second, quieter version of the same coverage drift. + // + // [#22161] Each one also remembers its finding, so the text face below + // prints its `fix:` and `rule:` lines (with the `os explain` pointer) the + // way `os build` does — the warning line itself is one verdict sentence + // now, and the rule id is how an author reaches the rest. + const registryFindingAt = new Map(); for (const f of ruleAdvisories) { + registryFindingAt.set(warnings.length, f); warnings.push(`${f.where}: ${f.message}`); } for (const w of docWarnings) { @@ -949,8 +958,12 @@ export default class Validate extends Command { if (warnings.length > 0) { console.log(''); - for (const w of warnings) { + for (const [i, w] of warnings.entries()) { console.log(chalk.yellow(` ⚠ ${w}`)); + const finding = registryFindingAt.get(i); + if (finding) { + for (const line of authoringFindingDetailLines(finding)) console.log(chalk.dim(` ${line}`)); + } } // The text face's half of the `--strict` gate. Its JSON counterpart is // the `CliExitCode` argument at the `emitJson` call above, reading this diff --git a/packages/cli/src/utils/format.ts b/packages/cli/src/utils/format.ts index 9dc1b9d954c..9d90c98c409 100644 --- a/packages/cli/src/utils/format.ts +++ b/packages/cli/src/utils/format.ts @@ -10,6 +10,7 @@ import type { TenancyPosture } from '@objectstack/spec/security'; // through the contract is that the two sides cannot disagree. import type { SeedSettlementSnapshot } from '@objectstack/spec/contracts'; import type { DevLogin } from '@objectstack/spec/system'; +import { explainRule } from '@objectstack/lint/rule-explanations'; import { writeStdoutDirect } from './json-stdout.js'; import { authoringRuleUnionStack } from './stack-collections.js'; import { stripAnsi } from './boot-log-capture.js'; @@ -1949,6 +1950,40 @@ export interface AuthoringAdvisory { hint?: string; } +/** + * [#22161] The pointer the `rule:` line carries for a rule with a long-form + * explanation — an em dash, then `os explain ` in backticks, then + * "for " — or `''` for a rule without one, so the line never + * names a command that has nothing to show. + * + * ⛔ The ONE place the pointer is spelled. A finding carries one verdict and + * one fix; what the rule counts and why it exists is its explanation, which + * `@objectstack/lint` keys by rule id and `os explain ` prints. The + * table is read from the `rule-explanations` entry, which imports nothing, so + * this formatter stays free of the rule engine (see {@link AuthoringAdvisory}). + */ +export function explainPointer(rule: string): string { + const explanation = explainRule(rule); + return explanation ? ` — \`os explain ${rule}\` for ${explanation.covers}` : ''; +} + +/** + * [#22161] The lines printed under an author-time finding's verdict line, in + * order: `fix: ` (when the finding has one) and + * `rule: at ` with the {@link explainPointer}. Every text face that + * prints a registry finding renders these through this function — the build + * and validate advisory lists and every gating-error list — so the three + * commands cannot print one finding three ways. + */ +export function authoringFindingDetailLines( + f: Pick, +): string[] { + const lines: string[] = []; + if (f.hint) lines.push(`fix: ${f.hint}`); + lines.push(`rule: ${f.rule} at ${f.path}${explainPointer(f.rule)}`); + return lines; +} + /** * The pointer a truncation notice offers when — and ONLY when — the command's * own `--json` payload really does carry the list that was cut. @@ -2053,8 +2088,7 @@ export function printAuthoringAdvisories( for (const f of advisories.slice(0, limit)) { printWarning(`${f.where}: ${f.message}`); - if (f.hint) console.log(chalk.dim(` ${f.hint}`)); - console.log(chalk.dim(` rule: ${f.rule} at ${f.path}`)); + for (const line of authoringFindingDetailLines(f)) console.log(chalk.dim(` ${line}`)); } // [#11642] The notice sentence now lives in ONE place. Rendering here is @@ -2105,8 +2139,7 @@ export function printAuthoringRuleErrors( const limit = options.limit ?? DIAGNOSTIC_PRINT_LIMIT; for (const f of errors.slice(0, limit)) { console.log(` • ${f.where}: ${f.message}`); - if (f.hint) console.log(chalk.dim(` ${f.hint}`)); - console.log(chalk.dim(` rule: ${f.rule} at ${f.path}`)); + for (const line of authoringFindingDetailLines(f)) console.log(chalk.dim(` ${line}`)); } printTruncationNotice({ total: errors.length, diff --git a/packages/lint/package.json b/packages/lint/package.json index 8882f4b5d09..d770436a88c 100644 --- a/packages/lint/package.json +++ b/packages/lint/package.json @@ -26,6 +26,16 @@ "types": "./dist/runtime.d.cts", "default": "./dist/runtime.cjs" } + }, + "./rule-explanations": { + "import": { + "types": "./dist/rule-explanations.d.ts", + "default": "./dist/rule-explanations.js" + }, + "require": { + "types": "./dist/rule-explanations.d.cts", + "default": "./dist/rule-explanations.cjs" + } } }, "scripts": { diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index bb4a9dc732d..4c9d3d3a54f 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -699,6 +699,12 @@ export type { FieldConsumerVerdict, } from './validate-field-consumers.js'; +// [#22161] The long-form reasoning behind an author-time rule, keyed by rule +// id — what `os explain ` prints. A finding carries one verdict and +// one fix; the "why" lives here, once (see the module note). +export { RULE_EXPLANATIONS, explainRule } from './rule-explanations.js'; +export type { RuleExplanation } from './rule-explanations.js'; + export { validateTranslationReferences, TRANSLATION_TARGET_UNKNOWN, diff --git a/packages/lint/src/rule-explanations.test.ts b/packages/lint/src/rule-explanations.test.ts new file mode 100644 index 00000000000..c4d8ac14972 --- /dev/null +++ b/packages/lint/src/rule-explanations.test.ts @@ -0,0 +1,79 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +// [#22161] The long-form rule explanations `os explain ` prints. The +// module is published as its own entry and imports nothing (see its header), +// so the two facts it spells as literals — which rule each entry explains and +// the scan roots `field-no-consumers` reads — are held to the rules here. + +import { readFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; + +import * as indexBarrel from './index.js'; +import { RULE_EXPLANATIONS, explainRule } from './rule-explanations.js'; +import { CARRIER_ROOTS, CONSUMER_ROOTS, FIELD_NO_CONSUMERS } from './validate-field-consumers.js'; + +const HERE = dirname(fileURLToPath(import.meta.url)); + +/** Every rule id constant the root barrel exports, by value. */ +function exportedRuleIds(): Set { + const ids = new Set(); + for (const [name, value] of Object.entries(indexBarrel)) { + if (/^[A-Z][A-Z0-9_]*$/.test(name) && typeof value === 'string') ids.add(value); + } + return ids; +} + +describe('rule explanations (#22161)', () => { + it('every entry explains a rule id a rule file exports, under its own key', () => { + const ids = exportedRuleIds(); + expect(Object.keys(RULE_EXPLANATIONS).length).toBeGreaterThan(0); + for (const [key, entry] of Object.entries(RULE_EXPLANATIONS)) { + expect(entry.rule, `entry under "${key}"`).toBe(key); + expect(ids.has(key), `"${key}" is not an exported rule id constant`).toBe(true); + } + }); + + it('each entry completes the rule-line pointer and carries real text', () => { + for (const entry of Object.values(RULE_EXPLANATIONS)) { + // `covers` finishes "`os explain ` for …" on one terminal line. + expect(entry.covers.length).toBeGreaterThan(0); + expect(entry.covers.length).toBeLessThanOrEqual(60); + expect(entry.covers).not.toMatch(/[.\n]/); + expect(entry.paragraphs.length).toBeGreaterThan(0); + for (const p of entry.paragraphs) { + expect(p.trim().length).toBeGreaterThan(0); + expect(p).not.toContain('\n'); + // An author is shown this text: the lesson goes in, a tracker number does not. + expect(p).not.toMatch(/#\d+/); + } + } + }); + + it('the field-no-consumers roots paragraph lists exactly the roots the rule scans', () => { + const text = explainRule(FIELD_NO_CONSUMERS)!.paragraphs.join('\n'); + expect(text).toContain( + `Roots scanned: ${CONSUMER_ROOTS.join(', ')} (consumers) · ${CARRIER_ROOTS.join(', ')} (carriers).`, + ); + }); + + it('explainRule is an exact-id lookup: no prefix, case or prototype match', () => { + expect(explainRule(FIELD_NO_CONSUMERS)?.rule).toBe(FIELD_NO_CONSUMERS); + expect(explainRule('field-no-consumer')).toBeUndefined(); + expect(explainRule('FIELD-NO-CONSUMERS')).toBeUndefined(); + expect(explainRule('toString')).toBeUndefined(); + expect(explainRule('__proto__')).toBeUndefined(); + }); + + it('the root barrel re-exports the same table', () => { + expect(indexBarrel.RULE_EXPLANATIONS).toBe(RULE_EXPLANATIONS); + expect(indexBarrel.explainRule).toBe(explainRule); + }); + + it('the module imports nothing, so its published entry stays free of the rule engine', () => { + const source = readFileSync(join(HERE, 'rule-explanations.ts'), 'utf8'); + expect(source).not.toMatch(/^\s*import\s/m); + expect(source).not.toMatch(/^\s*export\s[^\n]*\sfrom\s/m); + }); +}); diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts new file mode 100644 index 00000000000..7ac9bfdd52a --- /dev/null +++ b/packages/lint/src/rule-explanations.ts @@ -0,0 +1,126 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The full reasoning behind an author-time rule, kept out of its finding. + * + * A finding is printed by `os validate`, `os build` and `os dev` on every run, + * so it carries one verdict sentence (`message`) and one fix (`hint`) and + * nothing more. Everything an author needs only once — what the rule counts, + * what it deliberately does not, why it exists, where its boundaries are — + * lives here instead, keyed by rule id, and `os explain ` prints it. + * The CLI's `rule:` line names that command for exactly the rule ids this + * table holds, so the pointer is never printed for a rule that has nothing to + * show. + * + * ⛔ One copy. The text below is not repeated in the rule's message or hint, + * and a rule's message does not paraphrase it: an author who wants the "why" + * runs the command. + * + * ## Why this module imports nothing + * + * It is published as its own entry, `@objectstack/lint/rule-explanations`, as + * well as from the root barrel, because the CLI's finding printer reads it on + * every command that prints a finding, and that printer is deliberately not a + * rule-engine import: loading the root barrel costs about half a second (every + * rule module and `@objectstack/spec` with it), which `os explain object` and + * every other command would otherwise pay. So the keys are the rule ids + * spelled as literals, and a list a rule computes from data (the roots a scan + * reads) is written out here — `rule-explanations.test.ts` holds each key to a + * rule id constant some rule file exports and each such list to the rule's own + * constant, so neither can drift from the rule it explains. + * + * Adding an entry: key it by the rule's id, write `covers` so that it + * completes the sentence "`os explain ` for …", and move the long text out + * of the rule in the same edit. The id must not equal an `os explain` schema + * name — the command resolves one positional against both, and the CLI pins + * the two sets disjoint. + */ + +/** The long-form explanation of one author-time rule. */ +export interface RuleExplanation { + /** The rule id this explains — the `rule` of every finding it covers. */ + rule: string; + /** + * A short noun phrase that completes "`os explain ` for …" on the CLI's + * `rule:` line, e.g. `what counts as a consumer`. + */ + covers: string; + /** The reasoning, one paragraph per entry, in reading order. */ + paragraphs: readonly string[]; +} + +const FIELD_NO_CONSUMERS_EXPLANATION: RuleExplanation = { + rule: 'field-no-consumers', + covers: 'what counts as a consumer', + paragraphs: [ + 'A declared field is CONSUMED when something in this stack reads or displays it: a view column, ' + + 'an inline grid column, a form section, a page binding, a flow node, a dataset or cube member, a ' + + 'dashboard widget, a formula, a validation, a hook or an action that names it; a declared field ' + + 'group (`group` naming one of the object\'s `fieldGroups`) that places it on the synthesized ' + + 'layout; or a seed or import mapping that matches on it (`externalId` / `upsertKey`). One such ' + + 'site is enough: a field that is only drawn is the ordinary state of most fields, not a defect.', + 'A CARRIER names a field without reading it, so it does not count: a translation label, a seed ' + + 'value, an import-mapping target, a permission grant, a flow that only WRITES it, an ' + + '`inlineColumns` entry on a relationship field that does not set `inlineEdit` (no grid is ' + + 'drawn), or a dataset or cube member path the analytics door refuses (a hop or column that does ' + + 'not resolve, or a join the dataset\'s `include` does not declare).', + 'The verdict on the warning is `inert` when no site of any kind names the field, and ' + + '`carrier-only` when only carriers do — then the warning lists each carrier site, because ' + + 'removing the declaration means cleaning them too. Verdicts are per object: a consumer of the ' + + 'same field name on another object does not cover this declaration.', + 'Never reported, because the platform reads them without an authored consumer: a system column ' + + 'the registry injects (when the object re-declares it), the record\'s title field, and a ' + + '`master_detail` field (the relationship is consumed by being declared). Fields an ' + + '`objectExtensions` entry adds are not judged, and a stack that declares no consumer root at ' + + 'all (objects only) is skipped: its consumers are declared elsewhere.', + 'To fix it, give the field a consumer — a view column, a form section, a page binding, a ' + + 'formula, a validation, a flow node, a dataset dimension, or a `group` naming one of the ' + + 'object\'s declared `fieldGroups` so the synthesized layout draws it — or remove the ' + + 'declaration together with any carrier sites the warning lists.', + 'The rule is advisory and stays a warning: a consumer can legitimately live outside the stack — ' + + 'an API client, a hook or package this stack does not carry, a Studio-authored view. Ignore ' + + 'the warning for such a field.', + // Held equal to `CONSUMER_ROOTS` / `CARRIER_ROOTS` by `rule-explanations.test.ts`. + 'Roots scanned: objects, views, pages, apps, flows, dashboards, reports, datasets, actions, hooks, ' + + 'jobs, emailTemplates, agents, tools, skills, apis, webhooks, sharingRules, analyticsCubes ' + + '(consumers) · translations, data, mappings, permissions (carriers). Test fixtures are never ' + + 'scanned: the rule reads metadata, not a repository, so a field that only a test reads is ' + + 'reported.', + ], +}; + +const SECURITY_OWD_UNSET_EXPLANATION: RuleExplanation = { + rule: 'security-owd-unset', + covers: 'why the baseline must be declared', + paragraphs: [ + 'Every custom object declares its organization-wide default, `sharingModel` (OWD): the ' + + 'record-level baseline that every permission set\'s object grant is read against. An object ' + + 'that declares none still runs — the runtime fails CLOSED to \'private\' (ADR-0090 D1) — and ' + + 'this rule refuses it anyway, because the baseline must be an authored decision, not an ' + + 'accident of a default.', + 'The shape it guards: an object with no `sharingModel`, granted ordinary read/write by a ' + + 'permission set, let that grant read and edit every other user\'s records. Failing closed is ' + + 'the runtime\'s half of the fix; declaring the value is the author\'s half, so the next reader ' + + 'of the object sees the decision instead of inferring it from a default.', + 'The values, for internal users: \'private\' — the owner plus explicit shares (the recommended ' + + 'default); \'public_read\' — everyone reads, the owner writes; \'public_read_write\' — everyone ' + + 'reads and writes; \'controlled_by_parent\' — access derives from the master record (a ' + + 'master-detail child).', + 'System objects (`isSystem: true`, or a name starting with `sys_`) are not judged: the platform ' + + 'owns their posture.', + ], +}; + +/** + * Every rule explanation this package ships, keyed by rule id. `os explain + * ` reads this table and nothing else. + */ +export const RULE_EXPLANATIONS: Readonly> = Object.freeze({ + [FIELD_NO_CONSUMERS_EXPLANATION.rule]: FIELD_NO_CONSUMERS_EXPLANATION, + [SECURITY_OWD_UNSET_EXPLANATION.rule]: SECURITY_OWD_UNSET_EXPLANATION, +}); + +/** The explanation for `rule`, or `undefined` when the rule has none. Exact id match. */ +export function explainRule(rule: string): RuleExplanation | undefined { + return Object.prototype.hasOwnProperty.call(RULE_EXPLANATIONS, rule) ? RULE_EXPLANATIONS[rule] : undefined; +} diff --git a/packages/lint/src/rule-id-barrel-exports.test.ts b/packages/lint/src/rule-id-barrel-exports.test.ts index 7c4317b0ff0..2083b8ddcd4 100644 --- a/packages/lint/src/rule-id-barrel-exports.test.ts +++ b/packages/lint/src/rule-id-barrel-exports.test.ts @@ -9,8 +9,9 @@ // and `suppressWarnings: ['']` in authored metadata. The rule files // export a named constant for exactly that reason: a consumer should compare // against the constant, not retype the slug. But `package.json#exports` opens -// only "." and "./runtime" (and `tsup.config.ts` builds only those two -// entries), so a constant that no barrel re-exports is not merely inconvenient +// only the entries `tsup.config.ts` builds ("." and "./runtime", plus the +// data-only "./rule-explanations" since #22161), so a constant that no barrel +// re-exports is not merely inconvenient // to reach — it is UNREACHABLE. There is no deep path to fall back to, and the // consumer's only remaining option is the string literal the constant existed // to eliminate. @@ -37,11 +38,11 @@ // rule surface` additionally pins a floor on the number of ids found, so a // future edit that breaks the extraction pattern fails loudly instead of // passing over an empty set. -// 2. A barrel entry the check never imported. The two entries are named +// 2. A barrel entry the check never imported. The entries are named // statically below (a fully dynamic import cannot be bundled reliably), but // `barrel entries match the published exports map` proves that list is the // COMPLETE set of published entries by deriving it independently from -// `package.json#exports` and from `tsup.config.ts`. Adding a third entry +// `package.json#exports` and from `tsup.config.ts`. Adding an entry // without registering it here fails rather than going unchecked. // // The reverse direction (`no barrel export names a rule id that no rule file @@ -58,6 +59,7 @@ import { describe, expect, it } from 'vitest'; import * as indexBarrel from './index.js'; import * as runtimeBarrel from './runtime.js'; +import * as ruleExplanationsBarrel from './rule-explanations.js'; const srcDir = dirname(fileURLToPath(import.meta.url)); const pkgDir = join(srcDir, '..'); @@ -72,6 +74,8 @@ const pkgDir = join(srcDir, '..'); const BARREL_ENTRIES: Record> = { index: indexBarrel as unknown as Record, runtime: runtimeBarrel as unknown as Record, + // [#22161] Data only — it declares no rule id constant, so it carries none. + 'rule-explanations': ruleExplanationsBarrel as unknown as Record, }; /** diff --git a/packages/lint/src/validate-field-consumers.test.ts b/packages/lint/src/validate-field-consumers.test.ts index 7f47613122d..1fa47a218a9 100644 --- a/packages/lint/src/validate-field-consumers.test.ts +++ b/packages/lint/src/validate-field-consumers.test.ts @@ -11,6 +11,7 @@ import { validateFieldConsumers, } from './validate-field-consumers.js'; import { AUTHORING_COMMANDS, AUTHORING_RULES, runAuthoringRules } from './authoring-rules.js'; +import { explainRule } from './rule-explanations.js'; type AnyRec = Record; @@ -149,8 +150,7 @@ describe('validateFieldConsumers (#15922)', () => { expect(product.object).toBe('inv_product'); expect(product.field).toBe('tax_rate'); expect(product.verdict).toBe('carrier-only'); - expect(product.message).toContain('"inv_line"'); - expect(product.message).toContain('verdicts are per object'); + expect(product.message).toContain('a consumer of the same name on "inv_line" does not count'); expect(f['objects[1].fields.tax_rate']).toBeUndefined(); }); @@ -170,17 +170,61 @@ describe('validateFieldConsumers (#15922)', () => { const inert = f['objects[0].fields.is_taxable']; expect(inert.verdict).toBe('inert'); expect(inert.carriers).toEqual([]); - expect(inert.message).toContain('Verdict: inert'); - expect(inert.message).not.toContain('The same name'); + expect(inert.message).toBe('declared, but nothing in this stack displays or reads it (inert)'); + expect(inert.message).not.toContain('same name'); }); - it('names the roots it scanned on every finding and in the hint', () => { + // [#22161] The finding is printed on every validate / build / dev run, so it + // carries ONE verdict sentence and ONE fix; the long reasoning lives in the + // rule's explanation (`os explain field-no-consumers`), and the CLI's `rule:` + // line names that command (pinned in packages/cli, where the line is drawn). + describe('the printed shape: a verdict line, a fix line, the reasoning in the explanation', () => { + it('inert: the verdict and the fix, each one line, neither restating the location', () => { + const inert = byPath(validateFieldConsumers(corpus()))['objects[0].fields.is_taxable']; + expect(inert.where).toBe('object "inv_product" · field "is_taxable"'); + expect(inert.message).toBe('declared, but nothing in this stack displays or reads it (inert)'); + expect(inert.hint).toBe('add it to a view column or a form section, or remove the declaration'); + for (const line of [inert.message, inert.hint]) { + expect(line).not.toContain('\n'); + expect(line).not.toContain('inv_product'); + } + }); + + it('carrier-only: the verdict counts the carriers, the fix names each one a removal must clean', () => { + const color = byPath(validateFieldConsumers(corpus()))['objects[0].fields.color']; + expect(color.message).toBe( + 'declared, but nothing in this stack displays or reads it ' + + '(carrier-only: 1 site(s) name it without reading it)', + ); + expect(color.hint).toBe( + 'add it to a view column or a form section, or remove the declaration together with its ' + + '1 carrier site(s): permissions[0].objects.inv_product.fields.color', + ); + }); + + it('the reasoning is in the explanation, not on the finding', () => { + const findings = validateFieldConsumers(corpus()); + const explanation = explainRule(FIELD_NO_CONSUMERS); + expect(explanation?.rule).toBe(FIELD_NO_CONSUMERS); + expect(explanation?.covers).toBe('what counts as a consumer'); + const text = explanation!.paragraphs.join('\n'); + // What the message used to enumerate inline now lives here, once. + expect(text).toContain('A CARRIER names a field without reading it'); + expect(text).toContain('Verdicts are per object'); + expect(text).toContain('Test fixtures are never scanned'); + for (const finding of findings) { + expect(finding.message).not.toContain('carrier, not a consumer'); + expect(finding.hint).not.toContain('Roots scanned'); + } + }); + }); + + it('names the roots it scanned on every finding and in the explanation', () => { const [first] = validateFieldConsumers(corpus()); expect(first.rootsScanned).toEqual([...CONSUMER_ROOTS, ...CARRIER_ROOTS]); - expect(first.hint).toContain('test fixtures are never scanned'); - for (const root of ['views', 'pages', 'flows', 'translations', 'data', 'mappings']) { - expect(first.hint).toContain(root); - } + // Interpolated from the rule's own constants, never retyped. + const text = explainRule(FIELD_NO_CONSUMERS)!.paragraphs.join('\n'); + expect(text).toContain(`Roots scanned: ${CONSUMER_ROOTS.join(', ')} (consumers) · ${CARRIER_ROOTS.join(', ')}`); }); it('carries the positional path on the array-shaped field map', () => { diff --git a/packages/lint/src/validate-field-consumers.ts b/packages/lint/src/validate-field-consumers.ts index 4b4654ff79f..5d5fce81124 100644 --- a/packages/lint/src/validate-field-consumers.ts +++ b/packages/lint/src/validate-field-consumers.ts @@ -1290,15 +1290,21 @@ export function validateFieldConsumers(stack: AnyRec): FieldConsumerFinding[] { const verdict: FieldConsumerVerdict = carriers.length > 0 ? 'carrier-only' : 'inert'; const sharedWith = [...(ledger.objectsByField.get(field) ?? [])].filter((o) => o !== object); - const verdictClause = + // [#22161] One verdict sentence and one fix — the finding is printed on + // every `os validate` / `os build` / `os dev` run. What counts as a + // consumer, what is only a carrier, the exemptions and the roots scanned + // are the rule's long-form explanation (`rule-explanations.ts`), which + // `os explain field-no-consumers` prints and the CLI's `rule:` line names. + // Only what is specific to THIS field stays here: the verdict, the carrier + // sites a removal must clean, and the other objects whose consumers of the + // same name do not count. + const verdictTag = verdict === 'carrier-only' - ? `Verdict: carrier-only — ${carriers.length} carrier site(s) name it without reading it, and a removal ` + - `must clean each: ${listPaths(carriers)}.` - : `Verdict: inert — no site of any kind names it.`; + ? `carrier-only: ${carriers.length} site(s) name it without reading it` + : 'inert'; const sharedClause = sharedWith.length > 0 - ? ` The same name is declared on ${sharedWith.map((o) => `"${o}"`).join(', ')}; verdicts are per ` + - `object, so a consumer there does not cover this declaration.` + ? `; a consumer of the same name on ${sharedWith.map((o) => `"${o}"`).join(', ')} does not count` : ''; findings.push({ @@ -1306,25 +1312,12 @@ export function validateFieldConsumers(stack: AnyRec): FieldConsumerFinding[] { rule: FIELD_NO_CONSUMERS, where: `object "${object}" · field "${field}"`, path, - message: - `field "${field}" on object "${object}" is declared but nothing in this stack reads or displays ` + - `it: no view column, inline grid column, form section, page binding, flow node, dataset or cube ` + - `member, widget, formula, validation, hook or action names it, no declared field group places it on the ` + - `synthesized layout, and no ` + - `seed or import mapping matches on it. A translation label, a seed value, an import-mapping ` + - `target, a permission grant, a flow that only WRITES it, an \`inlineColumns\` entry on a ` + - `relationship field that does not set \`inlineEdit\` (no grid is drawn), or a dataset or cube ` + - `member path the analytics door refuses (a hop or column that does not resolve, or a join the ` + - `dataset's \`include\` does not declare) is a carrier, not a consumer. ` + - `${verdictClause}${sharedClause}`, + message: `declared, but nothing in this stack displays or reads it (${verdictTag}${sharedClause})`, hint: - `Give "${field}" a consumer — a view column, a form section, a page binding, a formula, a ` + - `validation, a flow node, a dataset dimension, or a \`group\` naming one of this object's ` + - `declared \`fieldGroups\` so the synthesized layout draws it — or remove the declaration` + - (carriers.length > 0 ? ` together with its ${carriers.length} carrier site(s) listed above` : '') + - `. Ignore this if the field is read only by an API client, by a hook or package this stack does not ` + - `carry, or by a Studio-authored view. Roots scanned: ${CONSUMER_ROOTS.join(', ')} (consumers) · ` + - `${CARRIER_ROOTS.join(', ')} (carriers); test fixtures are never scanned.`, + `add it to a view column or a form section, or remove the declaration` + + (carriers.length > 0 + ? ` together with its ${carriers.length} carrier site(s): ${listPaths(carriers)}` + : ''), object, field, verdict, diff --git a/packages/lint/src/validate-security-posture.test.ts b/packages/lint/src/validate-security-posture.test.ts index 3daff5be61a..1d49b2c3a5e 100644 --- a/packages/lint/src/validate-security-posture.test.ts +++ b/packages/lint/src/validate-security-posture.test.ts @@ -32,6 +32,7 @@ import { SECURITY_CBP_AMBIGUOUS_RELATION, } from './validate-security-posture.js'; import { lintDataModel } from './data-model-rules.js'; +import { explainRule } from './rule-explanations.js'; const rulesOf = (stack: Record) => validateSecurityPosture(stack).map((f) => f.rule); @@ -84,6 +85,33 @@ describe('validateSecurityPosture (ADR-0090 D7)', () => { expect(findings[0].hint).toContain("'private'"); }); + // [#22161] One verdict line, one fix line; the reasoning (fail-closed is the + // runtime's half, declaring is the author's, and the incident shape) is the + // rule's explanation, which `os explain security-owd-unset` prints and the + // CLI's `rule:` line names (pinned in packages/cli, where the line is drawn). + it('prints as a verdict line and a fix line; the reasoning is the explanation', () => { + const [finding] = validateSecurityPosture({ objects: [{ name: 'leave_request', label: 'Leave Request' }] }); + expect(finding.message).toBe( + "custom object declares no sharingModel (OWD); the runtime falls back to 'private', " + + 'but the baseline must be an authored decision', + ); + expect(finding.hint).toBe( + "declare sharingModel: 'private' (owner + shares; recommended), 'public_read', " + + "'public_read_write', or 'controlled_by_parent' (master-detail children)", + ); + for (const line of [finding.message, finding.hint]) expect(line).not.toContain('\n'); + // The location is the finding's `where`, never restated in the verdict. + expect(finding.message).not.toContain('leave_request'); + + const explanation = explainRule(SECURITY_OWD_UNSET); + expect(explanation?.rule).toBe(SECURITY_OWD_UNSET); + expect(explanation?.covers).toBe('why the baseline must be declared'); + const text = explanation!.paragraphs.join('\n'); + expect(text).toContain('ADR-0090 D1'); + expect(text).toContain("read and edit every other user's records"); + expect(finding.message).not.toContain('incident'); + }); + it('does not flag system objects for unset OWD', () => { expect(rulesOf({ objects: [{ name: 'sys_thing' }, { name: 'custom', isSystem: true }] })).toEqual([]); }); diff --git a/packages/lint/src/validate-security-posture.ts b/packages/lint/src/validate-security-posture.ts index ab4f32f8f0c..f306973b53e 100644 --- a/packages/lint/src/validate-security-posture.ts +++ b/packages/lint/src/validate-security-posture.ts @@ -483,14 +483,16 @@ export function validateSecurityPosture(stack: AnyRec, opts?: { nowMs?: number } rule: SECURITY_OWD_UNSET, where: `object "${objName}"`, path: `${objPath}.sharingModel`, + // [#22161] One verdict sentence and one fix. Why an unset OWD is + // refused although the runtime fails closed — and the incident shape + // it guards — is the rule's long-form explanation + // (`rule-explanations.ts`, `os explain security-owd-unset`). message: - `custom object "${objName}" declares no sharingModel (OWD). The runtime fails ` + - `CLOSED to 'private' (ADR-0090 D1), but the baseline must be an authored decision, ` + - `not an accident — this is the exact shape of the leave_request incident, where an object ` + - `with no sharingModel let an ordinary read/write grant read and edit every other user's records.`, + `custom object declares no sharingModel (OWD); the runtime falls back to 'private', ` + + `but the baseline must be an authored decision`, hint: - `Declare sharingModel explicitly: 'private' (owner + shares; recommended default), ` + - `'public_read', 'public_read_write', or 'controlled_by_parent' (master-detail children).`, + `declare sharingModel: 'private' (owner + shares; recommended), 'public_read', ` + + `'public_read_write', or 'controlled_by_parent' (master-detail children)`, }); } else if (typeof owd === 'string' && OWD_ALIAS_FIX[owd]) { // Reachable ONLY through the unparsed doors (`os lint` on a raw diff --git a/packages/lint/tsup.config.ts b/packages/lint/tsup.config.ts index 76fc9895517..d6608254368 100644 --- a/packages/lint/tsup.config.ts +++ b/packages/lint/tsup.config.ts @@ -3,7 +3,7 @@ import { defineConfig } from 'tsup'; import { dropSourcesContent } from '../../scripts/tsup-drop-sources-content.mjs'; /** - * Package-local config (#4463): `@objectstack/lint` ships TWO entries, so it + * Package-local config (#4463): `@objectstack/lint` ships THREE entries, so it * cannot use the repo-root `tsup.config.ts` (single `src/index.ts`). * * - `index` — the full authoring surface, used by the CLI. @@ -13,9 +13,13 @@ import { dropSourcesContent } from '../../scripts/tsup-drop-sources-content.mjs' * lighter. `splitting: false` emits each entry self-contained, and measured * `dist/runtime.js` is 93.8% of `dist/index.js` and does name the react/jsx * rules' modules. `src/runtime.ts`'s header carries the measurement. + * - `rule-explanations` — the long-form rule explanations alone (#22161), + * which the CLI's finding printer reads on every command that prints a + * finding. That printer is no rule-engine import, and this entry is what + * keeps it one: the module imports nothing (its header says why). */ export default defineConfig({ - entry: ['src/index.ts', 'src/runtime.ts'], + entry: ['src/index.ts', 'src/runtime.ts', 'src/rule-explanations.ts'], splitting: false, sourcemap: true, clean: true, From ed786eb3efcb687f6e8845ccfdd1e75fbabdd205 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 17:21:16 +0000 Subject: [PATCH 02/10] test(lint,cli): pin the verdict / fix / rule-line shape and `os explain ` Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude --- packages/cli/test/explain-rule-id.test.ts | 203 ++++++++++++++++++ .../rule-line-explain-pointer.e2e.test.ts | 109 ++++++++++ .../test/truncation-remainder-notices.test.ts | 3 +- .../test/validate-build-gate-parity.test.ts | 5 + 4 files changed, 319 insertions(+), 1 deletion(-) create mode 100644 packages/cli/test/explain-rule-id.test.ts create mode 100644 packages/cli/test/rule-line-explain-pointer.e2e.test.ts diff --git a/packages/cli/test/explain-rule-id.test.ts b/packages/cli/test/explain-rule-id.test.ts new file mode 100644 index 00000000000..deb3129cf26 --- /dev/null +++ b/packages/cli/test/explain-rule-id.test.ts @@ -0,0 +1,203 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#22161] `os explain ` and the `rule:` line that points at it. + * + * An author-time finding prints one verdict sentence and one fix; the rule's + * long reasoning is its explanation in `@objectstack/lint`, keyed by rule id, + * which `os explain ` prints. The `rule:` line names that command — + * spelled once, in `utils/format.ts` — for exactly the rules that have one. + * + * Pinned here: + * - one positional, two namespaces: a schema name resolves as before, a rule + * id resolves to its explanation, and the two sets are disjoint (so neither + * lookup can shadow the other); + * - every rule id the pointer can name resolves through the command — the + * pointer is never printed for a command that would answer "unknown"; + * - the printed shape of each shortened rule, from the rule's REAL finding: + * a verdict line, a `fix:` line, and the `rule:` line with the pointer. + * + * In-process, no spawn: `test/rule-line-explain-pointer.e2e.test.ts` drives + * the same shape through the real `os validate` / `os build` binaries. + */ + +import { resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import * as lint from '@objectstack/lint'; +import { + FIELD_NO_CONSUMERS, + RULE_EXPLANATIONS, + SECURITY_OWD_UNSET, + validateFieldConsumers, + validateSecurityPosture, +} from '@objectstack/lint'; +import Explain, { SCHEMAS } from '../src/commands/explain'; +import { + authoringFindingDetailLines, + explainPointer, + printAuthoringAdvisories, + printAuthoringRuleErrors, +} from '../src/utils/format'; + +const CLI_ROOT = resolve(fileURLToPath(import.meta.url), '..', '..'); + +/** Everything written to stdout while `fn` runs, ANSI-free. */ +async function captureStdout(fn: () => unknown | Promise): Promise { + const chunks: string[] = []; + const spy = vi.spyOn(process.stdout, 'write').mockImplementation(((chunk: unknown, ...rest: unknown[]) => { + chunks.push(String(chunk)); + const done = rest.find((a) => typeof a === 'function') as ((e?: Error | null) => void) | undefined; + done?.(null); + return true; + }) as never); + try { + await fn(); + } finally { + spy.mockRestore(); + } + // eslint-disable-next-line no-control-regex + return chunks.join('').replace(/\u001b\[[0-9;]*m/g, ''); +} + +/** Every rule id constant `@objectstack/lint` exports, by value. */ +function exportedRuleIds(): string[] { + return Object.entries(lint) + .filter(([name, value]) => /^[A-Z][A-Z0-9_]*$/.test(name) && typeof value === 'string') + .map(([, value]) => value as string); +} + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('os explain — one positional, schema names and rule ids (#22161)', () => { + it('the schema names and the rule ids are disjoint sets', () => { + const schemas = Object.keys(SCHEMAS); + const ruleIds = exportedRuleIds(); + // Anti-vacuity: both sides are populated. + expect(schemas.length).toBeGreaterThan(5); + expect(ruleIds.length).toBeGreaterThan(100); + // The command lowercases a schema lookup, so compare lowercased. + const lowered = new Set(ruleIds.map((id) => id.toLowerCase())); + expect(schemas.filter((s) => lowered.has(s))).toEqual([]); + expect(Object.keys(RULE_EXPLANATIONS).filter((id) => id.toLowerCase() in SCHEMAS)).toEqual([]); + }); + + it('every rule the pointer can name resolves through the command to its own explanation', async () => { + const ids = Object.keys(RULE_EXPLANATIONS); + expect(ids).toEqual(expect.arrayContaining([FIELD_NO_CONSUMERS, SECURITY_OWD_UNSET])); + for (const id of ids) { + expect(explainPointer(id)).toBe(` — \`os explain ${id}\` for ${RULE_EXPLANATIONS[id].covers}`); + const out = await captureStdout(() => Explain.run([id, '--json'], { root: CLI_ROOT })); + expect(JSON.parse(out)).toEqual(JSON.parse(JSON.stringify(RULE_EXPLANATIONS[id]))); + } + }, 60_000); + + it('a rule with no explanation gets no pointer', () => { + expect(explainPointer('liveness-dead-property')).toBe(''); + expect(explainPointer('no-such-rule')).toBe(''); + }); + + it('`os explain field-no-consumers` prints the reasoning the warning no longer carries', async () => { + const out = await captureStdout(() => Explain.run([FIELD_NO_CONSUMERS], { root: CLI_ROOT })); + expect(out).toContain(`Rule: ${FIELD_NO_CONSUMERS}`); + const flat = out.replace(/\s+/g, ' '); + for (const paragraph of RULE_EXPLANATIONS[FIELD_NO_CONSUMERS].paragraphs) { + expect(flat).toContain(paragraph.replace(/\s+/g, ' ')); + } + // Wrapped for a terminal: no printed line runs past the wrap width. + for (const line of out.split('\n')) expect(line.length, line).toBeLessThanOrEqual(88); + }, 60_000); + + it('a schema name still resolves as before', async () => { + const out = await captureStdout(() => Explain.run(['object'], { root: CLI_ROOT })); + expect(out).toContain('Schema: Object'); + expect(out).not.toContain('Rule:'); + }, 60_000); + + it('the no-argument listing names the rule explanations beside the schemas', async () => { + const out = await captureStdout(() => Explain.run(['--json'], { root: CLI_ROOT })); + const payload = JSON.parse(out) as { schemas: { name: string }[]; rules: { id: string; covers: string }[] }; + expect(payload.schemas.map((s) => s.name)).toEqual(Object.keys(SCHEMAS)); + expect(payload.rules).toEqual( + Object.values(RULE_EXPLANATIONS).map((r) => ({ id: r.rule, covers: r.covers })), + ); + }, 60_000); + + it('an id that is neither refuses with exit 1 and names both lists', async () => { + const exit = vi.spyOn(process, 'exit').mockImplementation(((code?: number) => { + throw new Error(`exit:${code}`); + }) as never); + const errors: string[] = []; + vi.spyOn(console, 'error').mockImplementation((...a: unknown[]) => { + errors.push(a.join(' ')); + }); + let thrown: unknown; + const out = await captureStdout(async () => { + try { + await Explain.run(['field-no-consumer'], { root: CLI_ROOT }); + } catch (e) { + thrown = e; + } + }); + expect(String((thrown as Error)?.message)).toBe('exit:1'); + expect(exit).toHaveBeenCalledWith(1); + const text = out + errors.join('\n'); + expect(text).toContain('Unknown schema or rule id: "field-no-consumer"'); + expect(text).toContain(`Rules with an explanation: ${Object.keys(RULE_EXPLANATIONS).join(', ')}`); + }, 60_000); +}); + +describe('the printed shape of each shortened rule — verdict, fix, rule line with the pointer (#22161)', () => { + const fieldFinding = () => { + const [finding] = validateFieldConsumers({ + objects: [ + { + name: 'my_app_ticket', + fields: { title: { type: 'text' }, description: { type: 'textarea' } }, + }, + ], + views: [{ list: { data: { object: 'my_app_ticket' }, columns: [{ field: 'title' }] } }], + }); + return finding; + }; + + it('field-no-consumers', async () => { + const finding = fieldFinding(); + expect(finding.rule).toBe(FIELD_NO_CONSUMERS); + expect(authoringFindingDetailLines(finding)).toEqual([ + 'fix: add it to a view column or a form section, or remove the declaration', + 'rule: field-no-consumers at objects[0].fields.description — ' + + '`os explain field-no-consumers` for what counts as a consumer', + ]); + const out = await captureStdout(() => printAuthoringAdvisories([finding])); + expect(out.split('\n').filter((l) => l.trim() !== '')).toEqual([ + ' ⚠ object "my_app_ticket" · field "description": declared, but nothing in this stack displays or reads it (inert)', + ' fix: add it to a view column or a form section, or remove the declaration', + ' rule: field-no-consumers at objects[0].fields.description — ' + + '`os explain field-no-consumers` for what counts as a consumer', + ]); + }); + + it('security-owd-unset', async () => { + const [finding] = validateSecurityPosture({ objects: [{ name: 'my_app_ticket', label: 'Ticket' }] }); + expect(finding.rule).toBe(SECURITY_OWD_UNSET); + const out = await captureStdout(() => printAuthoringRuleErrors([finding])); + expect(out.split('\n').filter((l) => l.trim() !== '')).toEqual([ + ' • object "my_app_ticket": custom object declares no sharingModel (OWD); the runtime falls back to ' + + "'private', but the baseline must be an authored decision", + " fix: declare sharingModel: 'private' (owner + shares; recommended), 'public_read', " + + "'public_read_write', or 'controlled_by_parent' (master-detail children)", + ' rule: security-owd-unset at objects[0].sharingModel — ' + + '`os explain security-owd-unset` for why the baseline must be declared', + ]); + }); + + it('a rule without an explanation keeps a bare rule line', () => { + expect(authoringFindingDetailLines({ rule: 'liveness-dead-property', path: 'views[0].name', hint: 'Remove it.' })) + .toEqual(['fix: Remove it.', 'rule: liveness-dead-property at views[0].name']); + expect(authoringFindingDetailLines({ rule: 'some-rule', path: 'p' })).toEqual(['rule: some-rule at p']); + }); +}); diff --git a/packages/cli/test/rule-line-explain-pointer.e2e.test.ts b/packages/cli/test/rule-line-explain-pointer.e2e.test.ts new file mode 100644 index 00000000000..c44bebc9fc4 --- /dev/null +++ b/packages/cli/test/rule-line-explain-pointer.e2e.test.ts @@ -0,0 +1,109 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#22161] The `field-no-consumers` warning, as `os validate` and `os build` + * really print it: one verdict line, one `fix:` line, and the `rule:` line that + * names `os explain field-no-consumers` — the command that prints the reasoning + * the warning used to carry inline (one 856-character line at validate, plus a + * second ~700-character paragraph at build, measured on a tutorial-shaped + * project before this change). + * + * `os validate` used to print a registry warning as its `⚠` line alone — no + * fix, no rule id — so this pins that the validate text face now carries the + * same three lines as `os build`, from the same helper. The JSON faces are + * untouched (the `⚠` line itself is unchanged in kind, so + * `validate-json-warning-parity.e2e.test.ts` still pairs it). + * + * Spawns the real CLI from source (`bin/run-dev.js` + tsx, the + * `validate-json-warning-parity.e2e.test.ts` pattern): what is pinned is the + * text two real commands print, not a function's return value. The in-process + * pins for the helper and for `os explain ` are in + * `test/explain-rule-id.test.ts`. + */ + +import { execFile } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +import { childEnv } from './helpers/serve-process.js'; +import { linkSpec } from './helpers/define-stack-fixture.js'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const CLI = resolve(HERE, '../bin/run-dev.js'); +const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); + +/** The tutorial shape: a ticket whose long-text `description` nothing displays. */ +const SOURCE = ` +import { defineStack } from '@objectstack/spec'; + +export default defineStack({ + manifest: { id: 'com.example.my-app', name: 'my_app', version: '0.1.0', type: 'app', namespace: 'my_app' }, + objects: [{ + name: 'my_app_ticket', + label: 'Ticket', + sharingModel: 'private', + fields: { + title: { type: 'text', label: 'Title' }, + description: { type: 'textarea', label: 'Description' }, + }, + }], + views: [{ object: 'my_app_ticket', label: 'Tickets', list: { type: 'grid', label: 'All', columns: ['title'] } }], +}); +`; + +const EXPECTED = [ + '⚠ object "my_app_ticket" · field "description": declared, but nothing in this stack displays or reads it (inert)', + 'fix: add it to a view column or a form section, or remove the declaration', + 'rule: field-no-consumers at objects[0].fields.description — ' + + '`os explain field-no-consumers` for what counts as a consumer', +]; + +function runCli(args: string[], cwd: string): Promise<{ code: number; stdout: string; stderr: string }> { + return new Promise((done) => { + execFile( + TSX, + [CLI, ...args], + { cwd, maxBuffer: 16 * 1024 * 1024, env: childEnv({ NO_COLOR: '1' }) }, + (err, stdout, stderr) => { + const code = err ? (typeof (err as { code?: unknown }).code === 'number' ? (err as { code: number }).code : 1) : 0; + done({ code, stdout: String(stdout), stderr: String(stderr) }); + }, + ); + }); +} + +/** The warning line for the fixture's field, and the two lines printed under it, trimmed. */ +function warningBlock(stdout: string): string[] { + const lines = stdout.split('\n'); + const at = lines.findIndex((l) => l.includes('field "description"') && l.includes('⚠')); + return at === -1 ? [] : lines.slice(at, at + 3).map((l) => l.trim()); +} + +let dir: string; + +beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), 'os-rule-line-pointer-')); + writeFileSync(join(dir, 'objectstack.config.ts'), SOURCE); + linkSpec(dir); +}); + +afterAll(() => { + rmSync(dir, { recursive: true, force: true }); +}); + +describe('the field-no-consumers warning prints as verdict, fix and rule line (#22161)', () => { + for (const command of ['validate', 'build'] as const) { + it(`os ${command}`, async () => { + const run = await runCli([command], dir); + expect(run.code, `${command} failed:\n${run.stdout}\n${run.stderr}`).toBe(0); + expect(warningBlock(run.stdout)).toEqual(EXPECTED); + // Printed once per run, and none of the reasoning rides along. + expect(run.stdout.split('field-no-consumers at').length - 1).toBe(1); + expect(run.stdout).not.toContain('carrier, not a consumer'); + expect(run.stdout).not.toContain('Roots scanned'); + }, 120_000); + } +}); diff --git a/packages/cli/test/truncation-remainder-notices.test.ts b/packages/cli/test/truncation-remainder-notices.test.ts index fc069caee9e..43d6954011f 100644 --- a/packages/cli/test/truncation-remainder-notices.test.ts +++ b/packages/cli/test/truncation-remainder-notices.test.ts @@ -174,7 +174,8 @@ describe('[#11642] printAuthoringRuleErrors — the gating rule failures', () => it('CONTROL — the rows are unchanged: the notice adds, it does not replace', () => { const out = capture(() => printAuthoringRuleErrors(many(80, finding), { remedy: JSON_FULL_LIST_REMEDY })); expect(out).toContain(' • object "obj_0": message 0'); - expect(out).toContain(' hint 0'); + // [#22161] The hint renders as the finding's `fix:` line. + expect(out).toContain(' fix: hint 0'); expect(out).toContain(' rule: rule-0 at objects[0].sharingModel'); // The cap still caps: the 50th row is there and the 51st is not. expect(out).toContain('at objects[49].sharingModel'); diff --git a/packages/cli/test/validate-build-gate-parity.test.ts b/packages/cli/test/validate-build-gate-parity.test.ts index 21f8338b21e..5e44e323d15 100644 --- a/packages/cli/test/validate-build-gate-parity.test.ts +++ b/packages/cli/test/validate-build-gate-parity.test.ts @@ -310,6 +310,11 @@ const NOT_A_GATE: Readonly> = { // keep honest. 'printAdvisoriesOnce', 'printAuthoringRuleErrors', + // [#22161] The `fix:` and `rule:` lines (with the `os explain` pointer) + // under a registry finding the rules already produced — the same helper + // `printAuthoringAdvisories` and `printAuthoringRuleErrors` render with; + // `validate.ts` calls it for its own `⚠` advisory lines. + 'authoringFindingDetailLines', 'printDocIssueErrors', 'printMetadataStats', 'collectMetadataStats', From 315a26618d620eaff268154e124cefb6efabb452 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 17:59:43 +0000 Subject: [PATCH 03/10] chore(changeset): lint + cli minor for one-line rule verdicts; author-time-rules test reads the field from where Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude --- .changeset/22161-rule-message-one-line.md | 21 +++++++++++++++++++ .../cli/src/utils/author-time-rules.test.ts | 5 +++-- 2 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 .changeset/22161-rule-message-one-line.md diff --git a/.changeset/22161-rule-message-one-line.md b/.changeset/22161-rule-message-one-line.md new file mode 100644 index 00000000000..0876572da70 --- /dev/null +++ b/.changeset/22161-rule-message-one-line.md @@ -0,0 +1,21 @@ +--- +"@objectstack/lint": minor +"@objectstack/cli": minor +--- + +feat(lint, cli): one-line author-time rule verdicts, and `os explain ` for the reasoning + +Clause-②: yes (widening) + +- **Shorter warnings.** `field-no-consumers` and `security-owd-unset` now print one verdict sentence and one fix. `os validate`, `os build` and `os dev` used to print `field-no-consumers` as a single line of about 860 characters, and `os build` added a second paragraph of about 700; the warning now reads: + + ```text + ⚠ object "my_app_ticket" · field "description": declared, but nothing in this stack displays or reads it (inert) + fix: add it to a view column or a form section, or remove the declaration + rule: field-no-consumers at objects[1].fields.description — `os explain field-no-consumers` for what counts as a consumer + ``` + + The finding's `message` no longer restates its `where`, and no longer carries the list of consumer and carrier kinds, the exemptions or the scanned roots; `hint` is the fix alone (for a `carrier-only` verdict it still names each carrier site a removal must clean). The `verdict`, `carriers` and `rootsScanned` fields of a `field-no-consumers` finding are unchanged. A tool that matched the old message text should match on `rule`, `where` and `path` instead. +- **`os explain `.** `os explain` takes an author-time rule id as well as a schema name — `os explain field-no-consumers`, `os explain security-owd-unset` — and prints the reasoning the warning no longer carries; `--json` prints `{ rule, covers, paragraphs }`. A schema name resolves exactly as before. With no argument it also lists the rule ids that have an explanation (`--json` adds `rules: [{ id, covers }]`). The `rule:` line names the command only for those rule ids. +- **`os validate` prints the `fix:` and `rule:` lines** under each author-time warning, the way `os build` does, and the hint line under every author-time finding (`os build`, `os validate`, `os verify`, `os init`) is now labelled `fix:`. `os lint` adds the same `os explain` pointer to its rule line. +- **New exports in `@objectstack/lint`:** `RULE_EXPLANATIONS`, `explainRule(ruleId)` and the type `RuleExplanation`, from the root entry and from the import-free `@objectstack/lint/rule-explanations` entry. diff --git a/packages/cli/src/utils/author-time-rules.test.ts b/packages/cli/src/utils/author-time-rules.test.ts index 031b840c35e..7673a63bda6 100644 --- a/packages/cli/src/utils/author-time-rules.test.ts +++ b/packages/cli/src/utils/author-time-rules.test.ts @@ -136,10 +136,11 @@ describe('judgeAuthorTimeRules — the stage os verify runs first (#21323)', () expect(verdict.refusal).toBeNull(); const perPackage = verdict.advisories.filter((f) => PER_PACKAGE_WHERE.test(f.where)); expect(perPackage.map((f) => f.rule)).toContain('field-no-consumers'); - expect(perPackage.some((f) => f.message.includes('industry'))).toBe(true); + // [#22161] The verdict no longer restates the location; the field is named in `where`. + expect(perPackage.some((f) => f.where.includes('field "industry"'))).toBe(true); // ...and the union run did not raise it, so the pass is the only source. const union = verdict.advisories.filter((f) => !PER_PACKAGE_WHERE.test(f.where)); - expect(union.some((f) => f.rule === 'field-no-consumers' && f.message.includes('industry'))).toBe(false); + expect(union.some((f) => f.rule === 'field-no-consumers' && f.where.includes('field "industry"'))).toBe(false); }); it('refuses a stack that does not parse — there is no rule verdict to give about it', () => { From 6d2eb857ce81c6c16b9296198dcb21fd4e99ad07 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 18:52:56 +0000 Subject: [PATCH 04/10] =?UTF-8?q?fix(cli):=20os=20explain=20resolves=20sch?= =?UTF-8?q?ema=20names=20as=20own=20keys=20=E2=80=94=20`constructor`=20/?= =?UTF-8?q?=20`=5F=5Fproto=5F=5F`=20are=20refused,=20not=20crashed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The schema lookup this change restructured read `SCHEMAS[name]` bare, so `os explain constructor` printed "Schema: Object … undefined" and threw `schema.required is not iterable` (exit 1, stack trace). Both lookups are own-key reads now; a prototype key is an unknown id like any other. Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude --- packages/cli/src/commands/explain.ts | 7 +++++-- packages/cli/test/explain-rule-id.test.ts | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/commands/explain.ts b/packages/cli/src/commands/explain.ts index f07571ab158..09b7564b4b6 100644 --- a/packages/cli/src/commands/explain.ts +++ b/packages/cli/src/commands/explain.ts @@ -496,8 +496,11 @@ export default class Explain extends Command { // ── Lookup: a schema name first (as before), then an author-time rule id ── // The two sets are disjoint (pinned in test/explain-rule-id.test.ts), so the - // order decides nothing today; it keeps every schema lookup exactly as it was. - const schema = SCHEMAS[schemaName.toLowerCase()]; + // order decides nothing today. Both are OWN-key lookups: a bare index read + // answered `constructor` / `__proto__` with Object's own members and the + // pretty printer then threw `schema.required is not iterable`. + const schemaKey = schemaName.toLowerCase(); + const schema = Object.prototype.hasOwnProperty.call(SCHEMAS, schemaKey) ? SCHEMAS[schemaKey] : undefined; const ruleExplanation = schema ? undefined : explainRule(schemaName); if (!schema && ruleExplanation) { if (flags.json) { diff --git a/packages/cli/test/explain-rule-id.test.ts b/packages/cli/test/explain-rule-id.test.ts index deb3129cf26..27bb0cfc5f8 100644 --- a/packages/cli/test/explain-rule-id.test.ts +++ b/packages/cli/test/explain-rule-id.test.ts @@ -126,6 +126,26 @@ describe('os explain — one positional, schema names and rule ids (#22161)', () ); }, 60_000); + it('a prototype key is neither a schema nor a rule: refused, not crashed', async () => { + for (const key of ['constructor', '__proto__', 'toString']) { + vi.spyOn(process, 'exit').mockImplementation(((code?: number) => { + throw new Error(`exit:${code}`); + }) as never); + vi.spyOn(console, 'error').mockImplementation(() => {}); + let thrown: unknown; + const out = await captureStdout(async () => { + try { + await Explain.run([key, '--json'], { root: CLI_ROOT }); + } catch (e) { + thrown = e; + } + }); + expect(String((thrown as Error)?.message), key).toBe('exit:1'); + expect(JSON.parse(out), key).toEqual({ error: `Unknown schema or rule id: ${key}` }); + vi.restoreAllMocks(); + } + }, 60_000); + it('an id that is neither refuses with exit 1 and names both lists', async () => { const exit = vi.spyOn(process, 'exit').mockImplementation(((code?: number) => { throw new Error(`exit:${code}`); From 2683575966209355c825cdbb316baea36af26d50 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 19:15:34 +0000 Subject: [PATCH 05/10] chore(changeset): note the os explain own-key lookup fix Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude --- .changeset/22161-rule-message-one-line.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.changeset/22161-rule-message-one-line.md b/.changeset/22161-rule-message-one-line.md index 0876572da70..27f0f7a7b5f 100644 --- a/.changeset/22161-rule-message-one-line.md +++ b/.changeset/22161-rule-message-one-line.md @@ -17,5 +17,6 @@ Clause-②: yes (widening) The finding's `message` no longer restates its `where`, and no longer carries the list of consumer and carrier kinds, the exemptions or the scanned roots; `hint` is the fix alone (for a `carrier-only` verdict it still names each carrier site a removal must clean). The `verdict`, `carriers` and `rootsScanned` fields of a `field-no-consumers` finding are unchanged. A tool that matched the old message text should match on `rule`, `where` and `path` instead. - **`os explain `.** `os explain` takes an author-time rule id as well as a schema name — `os explain field-no-consumers`, `os explain security-owd-unset` — and prints the reasoning the warning no longer carries; `--json` prints `{ rule, covers, paragraphs }`. A schema name resolves exactly as before. With no argument it also lists the rule ids that have an explanation (`--json` adds `rules: [{ id, covers }]`). The `rule:` line names the command only for those rule ids. +- **`os explain constructor` (or `__proto__`, `toString`) is refused as an unknown id** instead of printing `Schema: Object … undefined` and crashing with `schema.required is not iterable`: both lookups read own keys only. - **`os validate` prints the `fix:` and `rule:` lines** under each author-time warning, the way `os build` does, and the hint line under every author-time finding (`os build`, `os validate`, `os verify`, `os init`) is now labelled `fix:`. `os lint` adds the same `os explain` pointer to its rule line. - **New exports in `@objectstack/lint`:** `RULE_EXPLANATIONS`, `explainRule(ruleId)` and the type `RuleExplanation`, from the root entry and from the import-free `@objectstack/lint/rule-explanations` entry. From c76a01bb6b1c3a63ab91925dd15d5052f5f22e68 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 19:15:45 +0000 Subject: [PATCH 06/10] chore(changeset): name only the two prototype keys the old lookup crashed on Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude --- .changeset/22161-rule-message-one-line.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/22161-rule-message-one-line.md b/.changeset/22161-rule-message-one-line.md index 27f0f7a7b5f..b79d9e695d1 100644 --- a/.changeset/22161-rule-message-one-line.md +++ b/.changeset/22161-rule-message-one-line.md @@ -17,6 +17,6 @@ Clause-②: yes (widening) The finding's `message` no longer restates its `where`, and no longer carries the list of consumer and carrier kinds, the exemptions or the scanned roots; `hint` is the fix alone (for a `carrier-only` verdict it still names each carrier site a removal must clean). The `verdict`, `carriers` and `rootsScanned` fields of a `field-no-consumers` finding are unchanged. A tool that matched the old message text should match on `rule`, `where` and `path` instead. - **`os explain `.** `os explain` takes an author-time rule id as well as a schema name — `os explain field-no-consumers`, `os explain security-owd-unset` — and prints the reasoning the warning no longer carries; `--json` prints `{ rule, covers, paragraphs }`. A schema name resolves exactly as before. With no argument it also lists the rule ids that have an explanation (`--json` adds `rules: [{ id, covers }]`). The `rule:` line names the command only for those rule ids. -- **`os explain constructor` (or `__proto__`, `toString`) is refused as an unknown id** instead of printing `Schema: Object … undefined` and crashing with `schema.required is not iterable`: both lookups read own keys only. +- **`os explain constructor` (or `__proto__`) is refused as an unknown id** instead of printing `Schema: Object … undefined` and crashing with `schema.required is not iterable`: both lookups read own keys only. - **`os validate` prints the `fix:` and `rule:` lines** under each author-time warning, the way `os build` does, and the hint line under every author-time finding (`os build`, `os validate`, `os verify`, `os init`) is now labelled `fix:`. `os lint` adds the same `os explain` pointer to its rule line. - **New exports in `@objectstack/lint`:** `RULE_EXPLANATIONS`, `explainRule(ruleId)` and the type `RuleExplanation`, from the root entry and from the import-free `@objectstack/lint/rule-explanations` entry. From e84732425919aaada2f0d89df850213bcc5f72c3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 19:36:44 +0000 Subject: [PATCH 07/10] =?UTF-8?q?fix(lint,cli):=20a=20hint=20is=20a=20fix?= =?UTF-8?q?=20=E2=80=94=20expression-invalid's=20source=20rides=20its=20me?= =?UTF-8?q?ssage;=20four=20context-only=20hints=20state=20their=20fix?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `os validate` / `os build` label every finding's hint `fix:`. Five producers put something else there: expression-invalid quoted the authored source (printed as `fix: source: …`), and component-props-invalid, flow-time-relative-descriptor-invalid, react-prop-missing-required (the contract description branch) and liveness-experimental-property carried context only. The source now ends the expression-invalid message (` — source: …`, the flow engine's spelling) with an empty hint; the other four lead with the instruction. Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude --- packages/cli/src/utils/format.ts | 7 ++-- packages/cli/test/explain-rule-id.test.ts | 42 +++++++++++++++++++ packages/lint/src/authoring-rules.ts | 11 ++++- packages/lint/src/lint-liveness-properties.ts | 6 ++- packages/lint/src/validate-component-props.ts | 15 ++++--- .../src/validate-flow-trigger-readiness.ts | 9 ++-- .../lint/src/validate-react-page-props.ts | 8 +++- 7 files changed, 83 insertions(+), 15 deletions(-) diff --git a/packages/cli/src/utils/format.ts b/packages/cli/src/utils/format.ts index 9d90c98c409..40dcdfee995 100644 --- a/packages/cli/src/utils/format.ts +++ b/packages/cli/src/utils/format.ts @@ -2128,9 +2128,10 @@ export type AuthoringRuleFinding = AuthoringAdvisory; * * The `hint` line is conditional, which is how `printAuthoringAdvisories` and * `init` already rendered it; `compile`/`validate` printed it unconditionally. - * `AuthoringFinding.hint` is a required non-empty string in every rule the - * registry ships (checked: no rule emits an empty one), so the two forms - * differ on no finding this CLI can actually produce. + * [#22161] The condition is now load-bearing: `hint` prints as the `fix:` + * line, and `expression-invalid` emits an empty one — its finding carries no + * fix of its own, and the authored source it used to put there is a quote, + * which rides the message instead. */ export function printAuthoringRuleErrors( errors: readonly AuthoringRuleFinding[], diff --git a/packages/cli/test/explain-rule-id.test.ts b/packages/cli/test/explain-rule-id.test.ts index 27bb0cfc5f8..babdf59cb14 100644 --- a/packages/cli/test/explain-rule-id.test.ts +++ b/packages/cli/test/explain-rule-id.test.ts @@ -27,6 +27,8 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import * as lint from '@objectstack/lint'; import { + AUTHORING_RULES, + EXPRESSION_INVALID, FIELD_NO_CONSUMERS, RULE_EXPLANATIONS, SECURITY_OWD_UNSET, @@ -220,4 +222,44 @@ describe('the printed shape of each shortened rule — verdict, fix, rule line w .toEqual(['fix: Remove it.', 'rule: liveness-dead-property at views[0].name']); expect(authoringFindingDetailLines({ rule: 'some-rule', path: 'p' })).toEqual(['rule: some-rule at p']); }); + + // The `fix:` label is only true of a hint that IS a fix. `expression-invalid` + // used to put the authored source in `hint`, which printed as + // `fix: source: \`status != "resolved"\`` — the expression the author already + // wrote, offered as the fix. The source is a quote: it rides the verdict, so + // it still reaches the text face (and the runtime 422 issue's `message`). + it('expression-invalid: the authored source rides the verdict, and no `fix: source:` line is printed', async () => { + const entry = AUTHORING_RULES.find((r) => r.name === 'validateStackExpressions'); + expect(entry, 'the registry entry that adapts ExprIssue into expression-invalid').toBeDefined(); + const [finding] = entry!.run( + { + objects: [ + { + name: 'my_app_ticket', + fields: { + title: { type: 'text' }, + status: { type: 'select', options: [{ label: 'Open', value: 'open' }, { label: 'Resolved', value: 'resolved' }] }, + }, + }, + ], + actions: [ + { name: 'resolve_ticket', label: 'Resolve', objectName: 'my_app_ticket', type: 'script', visible: 'status != "resolved"' }, + ], + }, + {}, + ); + expect(finding?.rule).toBe(EXPRESSION_INVALID); + expect(finding.message).toContain('Write `record.status`'); + expect(finding.message).toMatch(/ — source: `status != "resolved"`$/); + expect(finding.hint).toBe(''); + + const out = await captureStdout(() => printAuthoringRuleErrors([finding])); + const lines = out.split('\n').filter((l) => l.trim() !== ''); + expect(lines).toHaveLength(2); + expect(lines[0]).toBe(` • ${finding.where}: ${finding.message}`); + expect(lines[0]).toContain('source: `status != "resolved"`'); + expect(lines[1]).toBe(` rule: expression-invalid at ${finding.path}`); + expect(out).not.toMatch(/fix: source:/); + expect(out).not.toMatch(/^\s*fix:/m); + }); }); diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index ae865c04308..8584dbe986e 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -683,8 +683,15 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ rule: EXPRESSION_INVALID, where: i.where, path: i.where, - message: i.message, - hint: `source: \`${i.source}\``, + // [#22161] The authored source is a QUOTE of what the author wrote, not + // a fix, so it rides the verdict — `— source: \`…\`` is the spelling + // the flow engine's runtime refusals already use — and reaches the CLI + // text face and the runtime 422 issue alike. `hint` is the `fix:` line: + // an `ExprIssue` carries no fix of its own (its message prescribes the + // rewrite where there is one, e.g. "Write `record.status`"), so there is + // none to print rather than a quote dressed as one. + message: i.source.trim() ? `${i.message} — source: \`${i.source}\`` : i.message, + hint: '', })), }, // ADR-0053 — `userFilters`/`quickFilters` on an object list view ("views" diff --git a/packages/lint/src/lint-liveness-properties.ts b/packages/lint/src/lint-liveness-properties.ts index c63e2763cc2..64bb353a7b7 100644 --- a/packages/lint/src/lint-liveness-properties.ts +++ b/packages/lint/src/lint-liveness-properties.ts @@ -244,7 +244,11 @@ function describe(entry: LedgerEntry): { kind: string; rule: string; defaultHint return { kind: 'is experimental — declared but NOT enforced at runtime', rule: LIVENESS_EXPERIMENTAL_PROPERTY, - defaultHint: 'It is declared in the spec as an experimental guarantee — not yet enforced at runtime.', + // [#22161] The CLI prints `hint` as the `fix:` line, so it says what to + // do; the fact it used to state alone is the reason. + defaultHint: + 'Do not rely on it as a guarantee: it is declared in the spec as an experimental guarantee and ' + + 'not yet enforced at runtime.', }; } if (entry.status === 'planned') { diff --git a/packages/lint/src/validate-component-props.ts b/packages/lint/src/validate-component-props.ts index c58334f819b..dfcabca527a 100644 --- a/packages/lint/src/validate-component-props.ts +++ b/packages/lint/src/validate-component-props.ts @@ -332,12 +332,17 @@ export function validateComponentProps(stack: AnyRec): ComponentPropsFinding[] { rule: COMPONENT_PROPS_INVALID, where, path: at, - message: `${at.slice(base.length + 1) || 'properties'}: ${describeIssue(issue, props)}`, + // [#22161] `hint` is the CLI's `fix:` line, so it states the fix; the + // advisory posture it used to explain (judged here as a warning, and + // the props bag is not parsed on the storage path) is the verdict's + // consequence and rides the message. + message: + `${at.slice(base.length + 1) || 'properties'}: ${describeIssue(issue, props)} — ` + + 'nothing refuses it today, so the renderer receives the props as written', hint: - `\`${type}\`'s props are declared by ComponentPropsMap (@objectstack/spec/ui) — the ` + - 'rejection above carries the fix. Advisory for now: props are judged here, at the authoring ' + - 'door, as a warning before they become an error, and the props bag is not parsed on the ' + - 'storage path either, so nothing rejects this today.', + `Give \`${type}\` props its schema accepts (ComponentPropsMap, @objectstack/spec/ui) — the ` + + 'rejection above names what it expects — or, if the component really does honour what is ' + + 'written, correct the schema so the declaration and the renderer agree.', }); } } diff --git a/packages/lint/src/validate-flow-trigger-readiness.ts b/packages/lint/src/validate-flow-trigger-readiness.ts index ccbf68406d2..41e4be0bce5 100644 --- a/packages/lint/src/validate-flow-trigger-readiness.ts +++ b/packages/lint/src/validate-flow-trigger-readiness.ts @@ -494,10 +494,13 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines `has a config.timeRelative descriptor the time-relative trigger REFUSES at bind time, so the ` + `sweep is never installed — the flow declares a time-relative trigger and then never runs ` + `(the only trace is one warn in the server log). ${problems}`, + // [#22161] `hint` is the CLI's `fix:` line: the instruction first, the + // reason it is sufficient after it. hint: - `Those messages are TimeRelativeTriggerSchema's own — the same schema the trigger safeParses at ` + - `bind time, so a descriptor that satisfies them binds. An unrecognized key names the declared key ` + - `it was probably meant to be; see content/docs/references/automation/time-relative-trigger.mdx.`, + `Correct config.timeRelative until it satisfies each message above: they are ` + + `TimeRelativeTriggerSchema's own, the same schema the trigger safeParses at bind time, so a ` + + `descriptor that satisfies them binds. An unrecognized key names the declared key it was ` + + `probably meant to be; see content/docs/references/automation/time-relative-trigger.mdx.`, }); } } diff --git a/packages/lint/src/validate-react-page-props.ts b/packages/lint/src/validate-react-page-props.ts index c4bd5a4bb03..23d223d27bb 100644 --- a/packages/lint/src/validate-react-page-props.ts +++ b/packages/lint/src/validate-react-page-props.ts @@ -1163,9 +1163,15 @@ export function validateReactPageProps(stack: AnyRec): ReactPropFinding[] { : `<${tag}> is missing the required prop "${req}".`, // The contract's own description of the binding says how to // 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. hint: dep ? dep.note - : block.requiredDescriptions.get(req) ?? `Pass ${req}={…}. See the react-tier component contract.`, + : `Pass ${req}={…}` + + (block.requiredDescriptions.has(req) + ? `: ${block.requiredDescriptions.get(req)}` + : '. See the react-tier component contract.'), }); } } From 819444f5099171543851b11a7346c54b8281a792 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 19:49:12 +0000 Subject: [PATCH 08/10] docs: re-render the three quoted findings from the printer (fix: label, source in the verdict); changeset line Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude --- .changeset/22161-rule-message-one-line.md | 1 + content/docs/getting-started/build-with-claude-code.mdx | 2 +- content/docs/ui/react-pages.mdx | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.changeset/22161-rule-message-one-line.md b/.changeset/22161-rule-message-one-line.md index b79d9e695d1..c73c1f47487 100644 --- a/.changeset/22161-rule-message-one-line.md +++ b/.changeset/22161-rule-message-one-line.md @@ -19,4 +19,5 @@ Clause-②: yes (widening) - **`os explain `.** `os explain` takes an author-time rule id as well as a schema name — `os explain field-no-consumers`, `os explain security-owd-unset` — and prints the reasoning the warning no longer carries; `--json` prints `{ rule, covers, paragraphs }`. A schema name resolves exactly as before. With no argument it also lists the rule ids that have an explanation (`--json` adds `rules: [{ id, covers }]`). The `rule:` line names the command only for those rule ids. - **`os explain constructor` (or `__proto__`) is refused as an unknown id** instead of printing `Schema: Object … undefined` and crashing with `schema.required is not iterable`: both lookups read own keys only. - **`os validate` prints the `fix:` and `rule:` lines** under each author-time warning, the way `os build` does, and the hint line under every author-time finding (`os build`, `os validate`, `os verify`, `os init`) is now labelled `fix:`. `os lint` adds the same `os explain` pointer to its rule line. +- **The `fix:` line is always a fix.** `expression-invalid` no longer puts the authored source in `hint`, where it printed as `fix: source: …`: the source now ends the finding's `message` as `` — source: `…` `` (the spelling the flow engine's runtime refusals use), so the CLI verdict line and the runtime publish gate's 422 issue `message` (Studio, REST `/meta`, MCP) both still carry it, and its `hint` is empty, so no `fix:` line prints. `component-props-invalid`, `flow-time-relative-descriptor-invalid`, `react-prop-missing-required` (where the component contract describes the binding) and `liveness-experimental-property` carried context in `hint`; each now leads with the instruction, and `component-props-invalid`'s message states its consequence (nothing refuses it today, so the renderer receives the props as written). - **New exports in `@objectstack/lint`:** `RULE_EXPLANATIONS`, `explainRule(ruleId)` and the type `RuleExplanation`, from the root entry and from the import-free `@objectstack/lint/rule-explanations` entry. diff --git a/content/docs/getting-started/build-with-claude-code.mdx b/content/docs/getting-started/build-with-claude-code.mdx index 9da7c9016a1..81253c3c8e8 100644 --- a/content/docs/getting-started/build-with-claude-code.mdx +++ b/content/docs/getting-started/build-with-claude-code.mdx @@ -309,7 +309,7 @@ visible: 'status != "resolved"' formula/validation expression binds the record as the `record` namespace, not at top level, so `status` resolves to nothing and the expression silently evaluates to null. Write `record.status`. - source: `status != "resolved"` + — source: `status != "resolved"` rule: expression-invalid at stack · action 'resolve_ticket' visible ``` diff --git a/content/docs/ui/react-pages.mdx b/content/docs/ui/react-pages.mdx index f0e52ce3db7..bff33aa7ab9 100644 --- a/content/docs/ui/react-pages.mdx +++ b/content/docs/ui/react-pages.mdx @@ -359,7 +359,7 @@ by tag and through `` alike: ``` ✗ Author-time rules failed (1 issue) • page "showcase_renewals_pipeline" › : renders "record:highlights", which reads its record from the record context a record page mounts — a kind:'react' page never mounts one, so the block renders empty no matter how it is bound (its objectName/recordId are not read by the renderer). - On a react page bind the record yourself: , or read the record with useAdapter().findOne and lay the strip out in JSX. + fix: On a react page bind the record yourself: , or read the record with useAdapter().findOne and lay the strip out in JSX. rule: react-block-needs-record-context at pages[27].source ``` @@ -401,7 +401,7 @@ A **missing required binding** fails the build: ``` ✗ Author-time rules failed (1 issue) • page "showcase_renewals_pipeline" › : is missing the required prop "objectName". - Pass objectName={…}. See the react-tier component contract. + fix: Pass objectName={…}: The object this block binds to (server-connected). rule: react-prop-missing-required at pages[27].source ``` From 74bed8f56ca26da15e3b25be7a74402c0fe946c3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 20:37:04 +0000 Subject: [PATCH 09/10] chore(changeset): name the runtime 422 hint text change for the three reworded hints that run at the publish gate Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude --- .changeset/22161-rule-message-one-line.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/22161-rule-message-one-line.md b/.changeset/22161-rule-message-one-line.md index c73c1f47487..32768bd8d5d 100644 --- a/.changeset/22161-rule-message-one-line.md +++ b/.changeset/22161-rule-message-one-line.md @@ -19,5 +19,5 @@ Clause-②: yes (widening) - **`os explain `.** `os explain` takes an author-time rule id as well as a schema name — `os explain field-no-consumers`, `os explain security-owd-unset` — and prints the reasoning the warning no longer carries; `--json` prints `{ rule, covers, paragraphs }`. A schema name resolves exactly as before. With no argument it also lists the rule ids that have an explanation (`--json` adds `rules: [{ id, covers }]`). The `rule:` line names the command only for those rule ids. - **`os explain constructor` (or `__proto__`) is refused as an unknown id** instead of printing `Schema: Object … undefined` and crashing with `schema.required is not iterable`: both lookups read own keys only. - **`os validate` prints the `fix:` and `rule:` lines** under each author-time warning, the way `os build` does, and the hint line under every author-time finding (`os build`, `os validate`, `os verify`, `os init`) is now labelled `fix:`. `os lint` adds the same `os explain` pointer to its rule line. -- **The `fix:` line is always a fix.** `expression-invalid` no longer puts the authored source in `hint`, where it printed as `fix: source: …`: the source now ends the finding's `message` as `` — source: `…` `` (the spelling the flow engine's runtime refusals use), so the CLI verdict line and the runtime publish gate's 422 issue `message` (Studio, REST `/meta`, MCP) both still carry it, and its `hint` is empty, so no `fix:` line prints. `component-props-invalid`, `flow-time-relative-descriptor-invalid`, `react-prop-missing-required` (where the component contract describes the binding) and `liveness-experimental-property` carried context in `hint`; each now leads with the instruction, and `component-props-invalid`'s message states its consequence (nothing refuses it today, so the renderer receives the props as written). +- **The `fix:` line is always a fix.** `expression-invalid` no longer puts the authored source in `hint`, where it printed as `fix: source: …`: the source now ends the finding's `message` as `` — source: `…` `` (the spelling the flow engine's runtime refusals use), so the CLI verdict line and the runtime publish gate's 422 issue `message` (Studio, REST `/meta`, MCP) both still carry it, and its `hint` is empty, so no `fix:` line prints. `component-props-invalid`, `flow-time-relative-descriptor-invalid`, `react-prop-missing-required` (where the component contract describes the binding) and `liveness-experimental-property` carried context in `hint`; each now leads with the instruction (the same text reaches the runtime publish gate's 422 issue `hint` for the three of them that run there: the time-relative, react-prop and liveness rules), and `component-props-invalid`'s message states its consequence (nothing refuses it today, so the renderer receives the props as written). - **New exports in `@objectstack/lint`:** `RULE_EXPLANATIONS`, `explainRule(ruleId)` and the type `RuleExplanation`, from the root entry and from the import-free `@objectstack/lint/rule-explanations` entry. From 1e016895c333ac726d83467331777fa84da91849 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 21:04:01 +0000 Subject: [PATCH 10/10] =?UTF-8?q?chore(changeset):=20state=20the=20runtime?= =?UTF-8?q?-wire=20changes=20exactly=20=E2=80=94=20one=20reworded=20hint?= =?UTF-8?q?=20reaches=20a=20422,=20security-owd-unset=20at=20the=20object?= =?UTF-8?q?=20door,=20the=20os=20explain=20unknown-id=20string?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude --- .changeset/22161-rule-message-one-line.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.changeset/22161-rule-message-one-line.md b/.changeset/22161-rule-message-one-line.md index 32768bd8d5d..0e9fbcf991e 100644 --- a/.changeset/22161-rule-message-one-line.md +++ b/.changeset/22161-rule-message-one-line.md @@ -16,8 +16,10 @@ Clause-②: yes (widening) ``` The finding's `message` no longer restates its `where`, and no longer carries the list of consumer and carrier kinds, the exemptions or the scanned roots; `hint` is the fix alone (for a `carrier-only` verdict it still names each carrier site a removal must clean). The `verdict`, `carriers` and `rootsScanned` fields of a `field-no-consumers` finding are unchanged. A tool that matched the old message text should match on `rule`, `where` and `path` instead. -- **`os explain `.** `os explain` takes an author-time rule id as well as a schema name — `os explain field-no-consumers`, `os explain security-owd-unset` — and prints the reasoning the warning no longer carries; `--json` prints `{ rule, covers, paragraphs }`. A schema name resolves exactly as before. With no argument it also lists the rule ids that have an explanation (`--json` adds `rules: [{ id, covers }]`). The `rule:` line names the command only for those rule ids. + + `security-owd-unset` also changes on the runtime wire. It runs at the metadata write door's publish gate for `object` writes (Studio, REST `/meta`, MCP), and an active write of a custom object (neither `isSystem: true` nor `sys_`-named) with no `sharingModel` is refused with a 422 whose `security-owd-unset` issue now carries the message `custom object declares no sharingModel (OWD); the runtime falls back to 'private', but the baseline must be an authored decision` and the hint `declare sharingModel: 'private' (owner + shares; recommended), 'public_read', 'public_read_write', or 'controlled_by_parent' (master-detail children)`. The old message named the object, which the issue's `where` and `path` still carry, and told the leave_request incident, which `os explain security-owd-unset` now prints. +- **`os explain `.** `os explain` takes an author-time rule id as well as a schema name — `os explain field-no-consumers`, `os explain security-owd-unset` — and prints the reasoning the warning no longer carries; `--json` prints `{ rule, covers, paragraphs }`. A schema name resolves exactly as before. With no argument it also lists the rule ids that have an explanation (`--json` adds `rules: [{ id, covers }]`). The `rule:` line names the command only for those rule ids. An argument that is neither still exits 1, and its error string changes from `Unknown schema: "X"` to `Unknown schema or rule id: "X"`, followed by a second list, `Rules with an explanation: …`; the `--json` `error` field changes the same way, from `Unknown schema: X` to `Unknown schema or rule id: X`. - **`os explain constructor` (or `__proto__`) is refused as an unknown id** instead of printing `Schema: Object … undefined` and crashing with `schema.required is not iterable`: both lookups read own keys only. - **`os validate` prints the `fix:` and `rule:` lines** under each author-time warning, the way `os build` does, and the hint line under every author-time finding (`os build`, `os validate`, `os verify`, `os init`) is now labelled `fix:`. `os lint` adds the same `os explain` pointer to its rule line. -- **The `fix:` line is always a fix.** `expression-invalid` no longer puts the authored source in `hint`, where it printed as `fix: source: …`: the source now ends the finding's `message` as `` — source: `…` `` (the spelling the flow engine's runtime refusals use), so the CLI verdict line and the runtime publish gate's 422 issue `message` (Studio, REST `/meta`, MCP) both still carry it, and its `hint` is empty, so no `fix:` line prints. `component-props-invalid`, `flow-time-relative-descriptor-invalid`, `react-prop-missing-required` (where the component contract describes the binding) and `liveness-experimental-property` carried context in `hint`; each now leads with the instruction (the same text reaches the runtime publish gate's 422 issue `hint` for the three of them that run there: the time-relative, react-prop and liveness rules), and `component-props-invalid`'s message states its consequence (nothing refuses it today, so the renderer receives the props as written). +- **The `fix:` line is always a fix.** `expression-invalid` no longer puts the authored source in `hint`, where it printed as `fix: source: …`: the source now ends the finding's `message` as `` — source: `…` `` (the spelling the flow engine's runtime refusals use), so the CLI verdict line still carries it, and so does the issue `message` at the runtime publish gate for `flow`, `action`, `hook` and `object` writes (Studio, REST `/meta`, MCP): in the 422 for an `error`, in the 2xx `advisories` for a `warning`. Its `hint` is empty, so no `fix:` line prints and the runtime issue's `hint` is `''`. `component-props-invalid`, `flow-time-relative-descriptor-invalid`, `react-prop-missing-required` (where the component contract describes the binding) and `liveness-experimental-property` carried context in `hint`; each now leads with the instruction, and `component-props-invalid`'s message states its consequence (nothing refuses it today, so the renderer receives the props as written). Two of the four run at the runtime publish gate. `flow-time-relative-descriptor-invalid` is an `error` on `flow` writes, so its new hint reaches the 422 issue `hint`. `liveness-experimental-property` runs on `email_template`, `mapping` and `datasource` writes as a `warning`, so its hint would ride the 2xx `advisories`, but none of those three ledgers has an `experimental` row today, so it reaches no runtime response yet. `component-props-invalid` is CLI-only, and `react-prop-missing-required` judges no `page` write at the gate, so their new text reaches the CLI only. - **New exports in `@objectstack/lint`:** `RULE_EXPLANATIONS`, `explainRule(ruleId)` and the type `RuleExplanation`, from the root entry and from the import-free `@objectstack/lint/rule-explanations` entry.