fix(lint): one-line verdicts for the approval-approver, data-model, agent-authoring, view-reference and chart-binding rules; os explain RULE_ID carries their reasoning

.changeset/22161-lint-slice-7-one-line.md

@@ -0,0 +1,21 @@
+---
+"@objectstack/lint": minor
+---
+
+fix(lint): the approval-approver, data-model, AI agent-authoring, view-reference and chart-binding findings print one verdict line, and `os explain <rule-id>` carries their reasoning
+
+Clause-②: yes (widening: three rule id constants exported from the barrel)
+
+- **Shorter verdicts.** Each finding of these 15 rule ids now prints a `message` of one verdict sentence. Every finding the rules' own test suites fire is at most 197 characters, the `packages/cli` unit suite's at most 188, and `os validate` on the example apps' at most 191; before, the longest of each ran from 201 to 480 characters. The ids:
+  - approval nodes (`flows[]`): `approval-approvers-may-resolve-empty` (both the group arm and the `manager` arm), `approval-approver-not-membership-tier`;
+  - the ADR-0120 uniqueness rules (`objects[].indexes` and field-level `unique`): `unique/unscoped-declared-index`, `unique/double-declaration`, `unique/legacy-organization-composite`;
+  - `os lint`'s data-model sweep: `relationship/master-detail-required`, `relationship/delete-behavior`, `rollup/non-numeric-aggregand`;
+  - AI agents (`agents[]`, `apps[].defaultAgent`): `agent-authoring-withdrawn`, `default-agent-outside-roster`, `default-agent-legacy-alias`;
+  - view references: `view-ref-nav-view-missing` (app navigation), `view-key-collision` (view containers);
+  - chart bindings (reports, list-view charts, dataset-bound page charts): `chart-measure-unknown`, `chart-axis-not-selected`.
+
+  The `manager` arm of `approval-approvers-may-resolve-empty` stays conditional — it says what happens where `sys_user.manager_id` is unset and asserts nothing about the column, which the rule cannot read. `view-ref-nav-view-missing` still lists the object's list views, at most three and then `(and N more)`, and `chart-axis-not-selected` quotes at most three names of the chart's selection. The `fix` (the CLI's `fix:` line, the runtime issue's `hint`), every rule id, severity and `path`, and what each rule accepts or refuses are unchanged. A tool that matched the old message text should match on `rule` and `path` instead.
+- **Three rule id constants are published** for ids `os lint`'s data-model sweep already emitted without one: `RELATIONSHIP_MASTER_DETAIL_REQUIRED` (`relationship/master-detail-required`), `RELATIONSHIP_DELETE_BEHAVIOR` (`relationship/delete-behavior`) and `ROLLUP_NON_NUMERIC_AGGREGAND` (`rollup/non-numeric-aggregand`), exported from `@objectstack/lint` beside the `UNIQUE_*` constants. The id strings themselves are unchanged.
+- **`os explain <rule-id>` takes these 15 ids**, for example `os explain unique/double-declaration`. It prints the reasoning the verdicts no longer carry: why an all-group or all-`manager` approval slate can stall, what silences the `manager` arm and what does not, and what an `org_membership_level` approver resolves against; the ADR-0120 scope words, the two channels that refuse a bare `unique: true` on a declared index, how a field `unique` and an index `unique` contradict or repeat, and why a hand-written organization composite enforces nothing on NULL rows; why a `master_detail` reference must be required under `controlled_by_parent`, what deleting a master does to its details, and why a `min` / `max` roll-up needs a numeric child; the platform agent roster and its retired aliases, how the runtime enforces the agent withdrawal, and what `defaultAgent` resolves against; how a navigation `viewName` is matched and what a renamed view key does to its references; and which chart positions bind a measure on each surface and which selection each is measured against. Paragraphs shared across ids are one text, printed under every id they explain. The `rule:` line under each of these findings now ends with `` — `os explain <rule-id>` for … ``. The no-argument listing and its `--json` `rules` array list the 15 ids, and so does the unknown-id error's `Rules with an explanation:` line. `RULE_EXPLANATIONS` in `@objectstack/lint` gains the 15 entries.
+- **Where the new text prints.** On the CLI: `os validate`, `os build` (and `os compile`, which `os dev` runs on every compile), `os lint`, `os verify`, and the scaffold check `os init` and `os generate` run print the new `message` on the text face for the approval, agent, view-reference and chart ids and for the three `unique/*` ids, and `os validate --json` and `os build --json` carry it in their `errors` and author-time `issues`. `relationship/master-detail-required`, `relationship/delete-behavior` and `rollup/non-numeric-aggregand` print on `os lint` only (and in the metadata-generation rubric's lint half), as before. At the runtime publish gate (Studio, REST `/meta`, MCP): on a `flow` write, the two approval ids ride the 2xx `advisories` and the `[Protocol] authoring advisory` log line; on a `report` write, `chart-measure-unknown` at a report's `values` or chart `yAxis` changes the 422 issue's `message` and the refusal log line under `OS_ALLOW_UNLINTED_METADATA_WRITES`, and `chart-measure-unknown` at a report chart's `series[]` and `chart-axis-not-selected` ride the 2xx `advisories` and the advisory log line. Each issue's `hint` is unchanged.
+- **Never at the runtime gate:** the three agent ids (the rule runs there only on a `flow` write, whose snapshot carries no agents or apps), the two view-reference ids and the three `unique/*` ids (CLI-only rules), the three data-model sweep ids (`os lint` only), and the chart ids at a list-view or page chart (a report write carries neither surface).

packages/lint/src/data-model-rules.master-detail-required.test.ts

@@ -25,10 +25,28 @@
 
 import { describe, expect, it } from 'vitest';
 import { Field, ObjectSchema } from '@objectstack/spec/data';
-import { lintDataModel } from './data-model-rules.js';
+import {
+  lintDataModel as lintDataModelUnrecorded,
+  RELATIONSHIP_DELETE_BEHAVIOR,
+  RELATIONSHIP_MASTER_DETAIL_REQUIRED,
+} from './data-model-rules.js';
+import { explainRule } from './rule-explanations.js';
 
 const R2 = 'relationship/master-detail-required';
 
+// [#22161] Each R2 / R3 finding is one verdict sentence; the reasoning it used
+// to carry is the id's `os explain` entry. Every call below records what it
+// fired, and the last cases in this file hold each recorded verdict of the two
+// ids to one line of at most 200 characters — every firing variant this suite
+// exercises, not a chosen few. Run the whole file: those cases read what the
+// cases above fired.
+const fired: Array<{ rule: string; message: string; severity: string }> = [];
+const lintDataModel: typeof lintDataModelUnrecorded = (...args) => {
+  const issues = lintDataModelUnrecorded(...args);
+  fired.push(...issues);
+  return issues;
+};
+
 /** The master, plus one detail object carrying exactly the fields under test. */
 const stack = (sharingModel: string | undefined, fields: unknown): unknown[] => [
   { name: 'work_order', sharingModel: 'private', fields: { name: { type: 'text' } } },
@@ -158,3 +176,77 @@ describe('R2 over objects built by ObjectSchema.create()', () => {
     expect(r2([{ name: 'work_order', sharingModel: 'private', fields: {} }, detail])).toEqual([]);
   });
 });
+
+describe('[#22161] one-line verdicts — relationship/master-detail-required, relationship/delete-behavior', () => {
+  it('the published constants spell the ids the findings carry', () => {
+    expect(RELATIONSHIP_MASTER_DETAIL_REQUIRED).toBe(R2);
+    expect(RELATIONSHIP_DELETE_BEHAVIOR).toBe('relationship/delete-behavior');
+  });
+
+  it('each controlled_by_parent shape reads as one sentence ending in the one shared consequence', () => {
+    const verdict = (shape: Record<string, unknown>) =>
+      r2(stack('controlled_by_parent', masterRef(shape)))[0]!.message;
+    const consequence = 'a controlled_by_parent detail saved without its master is readable by nobody';
+    expect(verdict({})).toBe(`master_detail "work_order_item.order" → work_order must be required: ${consequence}`);
+    expect(verdict({ readonly: true, system: true })).toBe(
+      'master_detail "work_order_item.order" → work_order must be required and not marked readonly or ' +
+        `system: ${consequence}`,
+    );
+    expect(verdict({ required: true, readonly: true })).toBe(
+      'master_detail "work_order_item.order" → work_order is marked readonly, which skips its required ' +
+        `check: ${consequence}`,
+    );
+  });
+
+  it('R3 names the declarable set and what an unset value does', () => {
+    const [issue] = lintDataModel([
+      { name: 'work_order', fields: { name: { type: 'text' } } },
+      { name: 'work_order_item', fields: { order: { type: 'master_detail', reference: 'work_order', required: true } } },
+    ]).filter((i) => i.rule === RELATIONSHIP_DELETE_BEHAVIOR);
+    expect(issue!.message).toBe(
+      'master_detail "work_order_item.order" → work_order should declare deleteBehavior ' +
+        '(cascade/restrict): left unset, deleting the master deletes its details',
+    );
+  });
+
+  it('every verdict the cases above fired is one line of at most 200 characters', () => {
+    const converted = fired.filter(
+      (f) => f.rule === RELATIONSHIP_MASTER_DETAIL_REQUIRED || f.rule === RELATIONSHIP_DELETE_BEHAVIOR,
+    );
+    // The coverage control first: both ids fired, R2 at both severities, so the
+    // shape assertion below cannot pass over an empty or partial record.
+    const r2Severities = new Set(converted.filter((f) => f.rule === R2).map((f) => f.severity));
+    expect([...r2Severities].sort()).toEqual(['error', 'warning']);
+    expect(converted.some((f) => f.rule === RELATIONSHIP_DELETE_BEHAVIOR)).toBe(true);
+    for (const f of converted) {
+      expect(f.message, f.rule).not.toContain('\n');
+      expect(f.message.length, `${f.rule}: ${f.message}`).toBeLessThanOrEqual(200);
+    }
+  });
+
+  it('`os explain` carries what the verdicts no longer say', () => {
+    const facts: Record<string, string[]> = {
+      [RELATIONSHIP_MASTER_DETAIL_REQUIRED]: [
+        'ADR-0055',
+        '`masterFK IN (accessible master ids)`',
+        'refused on every later write',
+        'skips `readonly` and `system` fields before its required check',
+        '`assertControlledByParentWrite`',
+        '`ObjectSchema.create()`',
+        'never a publish verdict',
+      ],
+      [RELATIONSHIP_DELETE_BEHAVIOR]: [
+        'an unset one included',
+        '`set_null` is not a choice here',
+        'refused at the parse',
+        'the relationship is a `lookup`',
+      ],
+    };
+    for (const [rule, list] of Object.entries(facts)) {
+      const entry = explainRule(rule);
+      expect(entry, `no \`os explain ${rule}\` entry`).toBeDefined();
+      const text = entry!.paragraphs.join('\n');
+      for (const fact of list) expect(text, `${rule} explanation names ${fact}`).toContain(fact);
+    }
+  });
+});

packages/lint/src/data-model-rules.summary-rollup.test.ts

@@ -28,10 +28,23 @@ import {
   isAggregateCompatibleWithFieldType,
 } from '@objectstack/spec/data';
 
-import { lintDataModel } from './data-model-rules.js';
+import { lintDataModel as lintDataModelUnrecorded, ROLLUP_NON_NUMERIC_AGGREGAND } from './data-model-rules.js';
+import { explainRule } from './rule-explanations.js';
 
 const RULE = 'rollup/non-numeric-aggregand';
 
+// [#22161] Each finding is one verdict sentence; the reasoning it used to carry
+// is the id's `os explain` entry. Every call below records what it fired, and
+// the last cases in this file hold each recorded verdict of the id to one line
+// of at most 200 characters — every child type this suite refuses, not a
+// chosen few. Run the whole file: those cases read what the cases above fired.
+const fired: Array<{ rule: string; message: string }> = [];
+const lintDataModel: typeof lintDataModelUnrecorded = (...args) => {
+  const issues = lintDataModelUnrecorded(...args);
+  fired.push(...issues);
+  return issues;
+};
+
 /** A parent rolling up `fn(child.<field>)`, and a child declaring that field as `childType`. */
 const model = (childType: string | undefined, fn = 'max', overrides: Record<string, unknown> = {}) => [
   {
@@ -184,3 +197,43 @@ describe('rollup/non-numeric-aggregand — stays silent where it cannot resolve,
     expect(findings(objects)).toEqual([]);
   });
 });
+
+describe('[#22161] one-line verdicts — rollup/non-numeric-aggregand', () => {
+  it('the published constant spells the id the findings carry', () => {
+    expect(ROLLUP_NON_NUMERIC_AGGREGAND).toBe(RULE);
+  });
+
+  it('reads as one sentence: the roll-up, the child type, and why the answer does not fit', () => {
+    const [issue] = findings(model('datetime'));
+    expect(issue!.message).toBe(
+      'summary field "invoice.rolled_up" rolls up max(invoice_line.shipped_at), a datetime field: max ' +
+        "answers in the child field's own type, but a summary field stores a finite number",
+    );
+  });
+
+  it('every verdict the cases above fired is one line of at most 200 characters', () => {
+    const converted = fired.filter((f) => f.rule === RULE);
+    // The coverage control first: the id fired, over more than one child type.
+    expect(new Set(converted.map((f) => f.message)).size).toBeGreaterThan(1);
+    for (const f of converted) {
+      expect(f.message, f.rule).not.toContain('\n');
+      expect(f.message.length, `${f.rule}: ${f.message}`).toBeLessThanOrEqual(200);
+    }
+  });
+
+  it('`os explain` carries what the verdict no longer says', () => {
+    const entry = explainRule(RULE);
+    expect(entry, 'no `os explain rollup/non-numeric-aggregand` entry').toBeDefined();
+    const text = entry!.paragraphs.join('\n');
+    for (const fact of [
+      '`NUMERIC_VALUE_TYPES`',
+      'the CHILD field\'s own type',
+      'the boolean ones',
+      'analytics aggregate table',
+      '`count` reads no value',
+      'does not resolve in this stack',
+    ]) {
+      expect(text, `explanation names ${fact}`).toContain(fact);
+    }
+  });
+});