From 3c428eeb27865096a189c666d50a06d85a0c1eb7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 21:05:45 +0000 Subject: [PATCH 1/5] wip(lint): one-line verdicts for the approval-approvers, data-model, agent-authoring, view-ref and chart-binding rules Claude-Session: https://claude.ai/code/session_01S3aAf11JjbW1mSGL1EhfFj Co-authored-by: Claude --- packages/lint/src/data-model-rules.ts | 89 +++-- packages/lint/src/index.ts | 6 + packages/lint/src/lint-view-refs.ts | 32 +- packages/lint/src/rule-explanations.ts | 353 ++++++++++++++++++ .../lint/src/validate-ai-agent-authoring.ts | 32 +- .../lint/src/validate-approval-approvers.ts | 45 ++- packages/lint/src/validate-chart-bindings.ts | 69 ++-- 7 files changed, 509 insertions(+), 117 deletions(-) diff --git a/packages/lint/src/data-model-rules.ts b/packages/lint/src/data-model-rules.ts index 7cd2a99f4d5..f9652215756 100644 --- a/packages/lint/src/data-model-rules.ts +++ b/packages/lint/src/data-model-rules.ts @@ -253,7 +253,22 @@ function refOf(def: any): string | undefined { // ─── R2 under controlled_by_parent ────────────────────────────────── /** The rule id R2 reports under, on both tiers. */ -const MASTER_DETAIL_REQUIRED = 'relationship/master-detail-required'; +export const RELATIONSHIP_MASTER_DETAIL_REQUIRED = 'relationship/master-detail-required'; + +/** The rule id R3 reports under (`master_detail` without `deleteBehavior`). */ +export const RELATIONSHIP_DELETE_BEHAVIOR = 'relationship/delete-behavior'; + +/** The rule id R13 reports under (a `min`/`max` roll-up over a non-numeric child). */ +export const ROLLUP_NON_NUMERIC_AGGREGAND = 'rollup/non-numeric-aggregand'; + +/** + * [#22161] The consequence every `controlled_by_parent` arm of R2 ends with — + * one wording, so the three shapes cannot drift. Why such a record is + * readable by nobody, and why `readonly` / `system` skip the required check, + * is the `relationship/master-detail-required` explanation (`os explain`). + */ +const CBP_ORPHAN_CONSEQUENCE = + 'a controlled_by_parent detail saved without its master is readable by nobody'; /** * R2's `error` tier — a `master_detail` reference on a @@ -326,23 +341,19 @@ function cbpMasterReferenceFinding( if (!missingRequired && flags.length === 0) return undefined; const subject = `master_detail "${objectName}.${fieldName}" → ${parent}`; - const derivation = - `"${objectName}" is controlled_by_parent, so its record access is derived through this ` + - `reference — a record saved without its master is readable by nobody (the derived read filter ` + - `never matches an empty master) and refused on every later write`; - const flagWords = flags.map((flag) => `\`${flag}: true\``).join(' and '); + // [#22161] One verdict per shape, ending in the one shared consequence; the + // derivation (why the record is unreadable, why the flags skip the check) + // is `os explain` text. const message = missingRequired ? flags.length === 0 - ? `${subject} must be required: ${derivation}` - : `${subject} must be required and must not be ${flagWords}: ${derivation}; record validation ` + - `also skips readonly and system fields before its required check` - : `${subject} is required but also ${flagWords}: record validation skips readonly and system ` + - `fields before its required check, so \`required: true\` is never enforced on it — and ` + - `${derivation}`; + ? `${subject} must be required: ${CBP_ORPHAN_CONSEQUENCE}` + : `${subject} must be required and not marked ${flags.join(' or ')}: ${CBP_ORPHAN_CONSEQUENCE}` + : `${subject} is marked ${flags.join(' and ')}, which skips its required check: ` + + CBP_ORPHAN_CONSEQUENCE; return { severity: 'error', - rule: MASTER_DETAIL_REQUIRED, + rule: RELATIONSHIP_MASTER_DETAIL_REQUIRED, message, path: `${fieldPath}.${missingRequired ? 'required' : flags[0]}`, fix: [ @@ -447,11 +458,11 @@ export function lintUnscopedDeclaredIndexes(objects: any[]): LocatedLintIssue[] severity: 'error', rule: UNIQUE_UNSCOPED_DECLARED_INDEX, where: indexWhere(obj, idx, j, colList), + // [#22161] One verdict; the two refusal channels and the stored-row + // conversion are `os explain` text. message: - `"${obj.name}" declares index${indexLabel} [${cols}] with bare \`unique: true\` — a unique index whose scope is ` + - `unstated (ADR-0120). Protocol 18 refuses this spelling on a declared index: it built the index over ` + - `exactly its \`fields\`, i.e. installation-wide, while reading like "unique per organization". ` + - `Stored metadata that still carries it converts to \`unique: 'global'\`, which builds the same physical index.`, + `"${obj.name}" index${indexLabel} [${cols}] has bare \`unique: true\`, an unstated scope protocol 18 ` + + `refuses (ADR-0120): it built the index installation-wide`, path: `objects[${i}].indexes[${j}]`, fix: `State the scope: \`unique: 'global'\` (installation-wide — the exact index bare \`true\` built) or ` + @@ -530,15 +541,16 @@ export function lintUniqueDeclarations(objects: any[]): LocatedLintIssue[] { const fieldSpelling = `\`unique: ${typeof def.unique === 'string' ? `'${def.unique}'` : def.unique}\``; const indexSpelling = `\`unique: ${typeof idx.unique === 'string' ? `'${idx.unique}'` : idx.unique}\``; + // [#22161] One verdict per quadrant; the scope matrix itself is + // `os explain` text. let message: string; let fix: string; if (fScope === iScope) { // Same scope on both sides — the same index declared twice. const boundary = fScope === 'global' ? 'installation-wide' : 'per-organization'; message = - `"${obj.name}.${name}" declares field-level ${fieldSpelling} AND a single-column unique index${indexLabel} ` + - `(${indexSpelling}) on the same column. Both ask for the same ${boundary} boundary — the same unique ` + - `index declared twice (ADR-0120 D5b). Redundant, not contradictory: drop one so the intent has a single home.`; + `"${obj.name}.${name}" field ${fieldSpelling} and index${indexLabel} ${indexSpelling} both ask ` + + `for ${boundary} uniqueness: the same index declared twice`; fix = fScope === 'global' ? `Keep ONE spelling of installation-wide uniqueness: \`unique: 'global'\` on '${name}', or the declared index — not both.` @@ -546,13 +558,9 @@ export function lintUniqueDeclarations(objects: any[]): LocatedLintIssue[] { } else { // Different scopes — the installation-wide side is physically stricter // and wins; the per-organization intent is dead on arrival. - const globalSide = fScope === 'global' ? `field-level ${fieldSpelling}` : `declared index${indexLabel} (${indexSpelling})`; - const orgSide = fScope === 'global' ? `declared index${indexLabel} (${indexSpelling})` : `field-level ${fieldSpelling}`; message = - `"${obj.name}.${name}" declares an installation-wide unique (${globalSide}) AND a per-organization unique ` + - `(${orgSide}) on the same column — the two scopes CONTRADICT (ADR-0120 D5b). The installation-wide index is ` + - `physically stricter and wins; the per-organization constraint can never be tripped, so one of the two ` + - `intents you wrote is silently dead.`; + `"${obj.name}.${name}" field ${fieldSpelling} and index${indexLabel} ${indexSpelling} CONTRADICT: ` + + `the installation-wide one wins, so the per-organization intent is silently dead`; fix = `Pick ONE scope and say it once: for installation-wide uniqueness keep \`unique: 'global'\` and drop the ` + `per-organization declaration; for per-organization uniqueness set \`unique: 'organization'\` (field-level on ` + @@ -624,18 +632,16 @@ export function lintLegacyOrganizationComposites(objects: any[]): LocatedLintIss if (!cols.includes(tenantColumn)) continue; const indexLabel = typeof idx?.name === 'string' && idx.name.trim() ? ` '${idx.name.trim()}'` : ''; - const spelling = `\`unique: ${typeof idx.unique === 'string' ? `'${idx.unique}'` : idx.unique}\``; const rest = cols.filter((c: string) => c !== tenantColumn); issues.push({ severity: 'warning', rule: UNIQUE_LEGACY_ORGANIZATION_COMPOSITE, where: indexWhere(obj, idx, j, cols), + // [#22161] One verdict; what the composite is, and what the + // respelling costs, is `os explain` text. message: - `"${obj.name}" declares index${indexLabel} [${cols.join(', ')}] with ${spelling} and lists the organization ` + - `column '${tenantColumn}' itself — the hand-written per-organization composite that predates the scope ` + - `vocabulary (ADR-0120 S6). It reads as "unique per organization" but materializes as a plain composite, and ` + - `SQL UNIQUE is NULL-distinct: on every row whose '${tenantColumn}' is NULL it enforces nothing — which ` + - `on a single-organization deployment is every row.`, + `"${obj.name}" index${indexLabel} [${cols.join(', ')}] lists the organization column, and SQL ` + + `UNIQUE is NULL-distinct: on every row whose '${tenantColumn}' is NULL it enforces nothing`, path: `objects[${i}].indexes[${j}]`, fix: `State the scope instead: \`unique: 'organization'\` on this index (keep \`fields\` exactly as they are — the ` + @@ -818,14 +824,13 @@ export function lintDataModel(objects: any[]): LintIssue[] { if (typeof childType === 'string' && !summaryRollupAnswerFitsColumn(childType)) { issues.push({ severity: 'error', - rule: 'rollup/non-numeric-aggregand', + rule: ROLLUP_NON_NUMERIC_AGGREGAND, + // [#22161] One verdict; the value contract and why the two + // aggregate tables differ is `os explain` text. message: `summary field "${obj.name}.${fieldName}" rolls up ` + - `${fn}(${childName}.${childFieldName}), but "${childName}.${childFieldName}" is ` + - `a ${childType} field — ${fn} answers with a value of the CHILD field's own type, ` + - `while a summary field's value contract is a finite number (it is a member of the ` + - `spec's NUMERIC_VALUE_TYPES class), so the answer does not fit the column the ` + - `roll-up is stored in`, + `${fn}(${childName}.${childFieldName}), a ${childType} field: ${fn} answers in the ` + + `child field's own type, but a summary field stores a finite number`, path: `${fieldPath}.summaryOperations.field`, fix: `Aggregate a numeric or boolean child field instead (min/max over those answer ` + @@ -866,7 +871,7 @@ export function lintDataModel(objects: any[]): LintIssue[] { } else if (def.required !== true) { issues.push({ severity: 'warning', - rule: MASTER_DETAIL_REQUIRED, + rule: RELATIONSHIP_MASTER_DETAIL_REQUIRED, message: `master_detail "${obj.name}.${fieldName}" → ${parent} should be required (a detail record cannot exist without its master)`, path: `${fieldPath}.required`, fix: 'required: true', @@ -876,8 +881,10 @@ export function lintDataModel(objects: any[]): LintIssue[] { if (def.deleteBehavior === undefined) { issues.push({ severity: 'suggestion', - rule: 'relationship/delete-behavior', - message: `master_detail "${obj.name}.${fieldName}" → ${parent} should declare deleteBehavior (cascade/restrict — set_null is not honored on master_detail; use a lookup field if children must survive the parent)`, + rule: RELATIONSHIP_DELETE_BEHAVIOR, + // [#22161] One verdict; why `set_null` is not a choice here and + // what to use when children must survive is `os explain` text. + message: `master_detail "${obj.name}.${fieldName}" → ${parent} should declare deleteBehavior (cascade/restrict): left unset, deleting the master deletes its details`, path: `${fieldPath}.deleteBehavior`, fix: "deleteBehavior: 'cascade'", }); diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 4c9d3d3a54f..d9da1c69995 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -940,6 +940,12 @@ export { UNIQUE_DOUBLE_DECLARATION, UNIQUE_UNSCOPED_DECLARED_INDEX, UNIQUE_LEGACY_ORGANIZATION_COMPOSITE, + // [#22161] Three `lintDataModel` ids that had no constant, published so a + // `RULE_EXPLANATIONS` key can be held to one (`rule-explanations.test.ts`) + // and a consumer can compare `f.rule` against a name, not a retyped slug. + RELATIONSHIP_MASTER_DETAIL_REQUIRED, + RELATIONSHIP_DELETE_BEHAVIOR, + ROLLUP_NON_NUMERIC_AGGREGAND, } from './data-model-rules.js'; export type { LintIssue, LocatedLintIssue, Severity } from './data-model-rules.js'; diff --git a/packages/lint/src/lint-view-refs.ts b/packages/lint/src/lint-view-refs.ts index bc12f791211..ed505922f05 100644 --- a/packages/lint/src/lint-view-refs.ts +++ b/packages/lint/src/lint-view-refs.ts @@ -80,7 +80,7 @@ */ import { expandViewContainerWithDiagnostics, isAggregatedViewContainer } from '@objectstack/spec'; -import { listNames, recordsOf, suggestName } from './object-graph.js'; +import { recordsOf, suggestName } from './object-graph.js'; export interface ViewRefFinding { where: string; @@ -122,6 +122,17 @@ function resolvesViewId(requested: string, ids: ReadonlySet, object: str const shortViewName = (id: string, object: string): string => id.startsWith(`${object}.`) ? id.slice(object.length + 1) : id; +/** + * [#22161] An object's list views for a one-line verdict: sorted, at most + * three, then `(and N more)` — the `fix:` line's `Did you mean` names the + * nearest one wherever it sorts. + */ +function viewRoster(names: readonly string[]): string { + const sorted = [...names].sort(); + const shown = sorted.slice(0, 3).join(', '); + return sorted.length > 3 ? `${shown} (and ${sorted.length - 3} more)` : shown; +} + /** Pull the view-container slots out of an object definition (ADR-0017 nested * "Object has-many View"). Absent slots stay undefined — the expander ignores * them. */ @@ -215,10 +226,11 @@ export function lintViewRefs(stack: AnyRec): ViewRefFinding[] { for (const col of collisions) { findings.push({ where: `object '${object}' · view key '${col.key}'`, + // [#22161] One verdict; the shared namespace and which references + // resolve through it are `os explain` text. message: - `View key collision: the ${col.viewKind} view '${col.requested}' clashes with another view ` + - `in the same container and was renamed to '${col.renamedTo}'. Anything referencing ` + - `'${col.requested}' (a form action target, a navigation viewName) resolves to the OTHER view, not this one.`, + `View key collision: the ${col.viewKind} view '${col.requested}' was renamed to ` + + `'${col.renamedTo}', so every reference to '${col.requested}' resolves to the OTHER view`, hint: `Give the ${col.viewKind} view a unique key — the default list implicitly claims '.default'. ` + `Renaming key '${col.key}' fixes both this collision and any reference that targets it.`, @@ -325,15 +337,13 @@ export function lintViewRefs(stack: AnyRec): ViewRefFinding[] { findings.push({ where: `app '${appName}' · nav '${navId}'`, + // [#22161] One verdict; how the name is matched and why the fallback is + // silent are `os explain` text. The list views stay: they are the fix. message: `Navigation entry opens view '${viewName}' on object '${objectName}', which declares no such ` + - `list view. ` + - (isFormView - ? `The name resolves to a FORM view of that object, which the object's view switcher never offers. ` - : '') + - `At runtime the name does not resolve and the entry falls back to the object's default view, ` + - `keeping its authored label and icon — so the sidebar still reads correctly while opening the ` + - `wrong view. List views on '${objectName}': ${listNames(available)}.`, + `list view` + + (isFormView ? ` (the name resolves to a FORM view)` : '') + + `, so it opens the default view under its own label. List views: ${viewRoster(available)}`, hint: `Correct the name, declare '${viewName}' in the object's \`listViews\`, or drop \`viewName\` ` + `to open the default view.` + diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index c52e66aa6f8..92bc8bbf661 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -2033,6 +2033,344 @@ const FILTER_PRESET_COMPARAND_EXPLANATION: RuleExplanation = { ], }; +// ── Approval approvers (`validate-approval-approvers.ts`) ─────────────────── + +const APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY_EXPLANATION: RuleExplanation = { + rule: 'approval-approvers-may-resolve-empty', + covers: 'why an all-group or all-manager slate can stall', + paragraphs: [ + 'An approval node opens its request on the people its approvers expand to at runtime. When ' + + 'EVERY approver on the node routes to a group whose membership is runtime data — a ' + + '`position`, a `team` or a `department` — and none is staffed, the request opens on an empty ' + + '`pending_approvers` slate: no user can act, so it waits forever, and under the default ' + + '`lockRecord: true` the record stays locked with it, with no in-product recovery. An approver ' + + 'of any other type on the node is a non-group route, so a mixed slate is not judged.', + 'The `manager` arm is the same dead end with a cause the product cannot repair. ' + + '`{ type: \'manager\' }` resolves from `sys_user.manager_id` of the user the record names ' + + '(the field `value` names, else the owner), and returns nobody where that column is unset. A ' + + 'static check cannot read that column, so this does not assert the slate IS empty; it reports ' + + 'that nothing else on the node can approve if it is. The arm fires only when the whole slate ' + + 'is `manager` rungs, so no node draws both arms, and a slate mixing groups and `manager` is ' + + 'silent.', + 'One fact in the stack silences the `manager` arm: a seed row of `sys_user` carrying a ' + + 'non-empty `manager_id`, which shows the stack populates the column. Declaring ' + + '`onEmptyApprovers: \'fallback\'` does not silence it, because a `fallbackApprovers` list can ' + + 'itself resolve to nobody, which a static check cannot see either. The runtime publish gate ' + + 'carries no seeds, so a Studio publish of a manager-only flow draws the advisory however the ' + + 'users are wired.', + 'Both arms are `info`: staffing and the manager column are runtime data a linter cannot see, so ' + + 'the rule flags the SHAPE, and an `info` finding never blocks a build or a publish.', + ], +}; + +const APPROVAL_APPROVER_NOT_MEMBERSHIP_TIER_EXPLANATION: RuleExplanation = { + rule: 'approval-approver-not-membership-tier', + covers: 'what an org_membership_level approver resolves against', + paragraphs: [ + '`org_membership_level` — and `role`, its deprecated spelling — resolves against the ' + + 'better-auth org-membership tier a member holds (`sys_member.role`), never against positions. ' + + 'The vocabulary is closed and framework-owned (ADR-0108): the tiers the verdict lists are ' + + 'read from `@objectstack/spec`\'s `BUILTIN_MEMBERSHIP_ROLES`, and a value is compared ' + + 'case-insensitively.', + 'After ADR-0090 D3 renamed `sys_role` to `sys_position`, an approver authored as ' + + '`{ type: \'role\', value: \'sales_manager\' }` finds no member row: the expansion falls back ' + + 'to the `role:sales_manager` literal, and the request waits on an approver that can never ' + + 'act. A business role is always a position, resolved through `sys_user_position`.', + 'A warning, not an error: the runtime keeps its literal fallback rather than refusing the node. ' + + 'When the value is not a tier, this finding is reported instead of ' + + '`approval-approver-type-deprecated` on the same approver: the bad value is the more serious ' + + 'defect, and its fix (`position`) differs from the deprecation\'s (`org_membership_level`).', + ], +}; + +// ── Data-model rules (`data-model-rules.ts`) ─────────────────────────────── +// The three ADR-0120 uniqueness ids run on `os validate` and `os build` as +// registry entries and on `os lint` through `lintDataModel`; the other three +// are `lintDataModel`'s own sweep. + +/** The ADR-0120 scope words, as the two uniqueness spellings read them (both write it out). */ +const UNIQUE_SCOPE_WORDS = + 'ADR-0120 gives uniqueness two scope words: `\'organization\'`, one holder per organization, and ' + + '`\'global\'`, installation-wide. On a FIELD, bare `unique: true` is the positional synonym of ' + + '`\'organization\'` and stays valid; on a declared INDEX the same spelling states no scope, and ' + + 'is judged as `\'global\'`.'; + +/** Where the three `lintDataModel` sweep ids are reported (all three write it out). */ +const DATA_MODEL_SWEEP_REACH = + 'Reach: `os lint` and the metadata-generation rubric run this rule, and nothing else does — it ' + + 'is not an `os validate` / `os build` rule and does not run at the metadata save door, so its ' + + 'severity moves `os lint`\'s exit code and the rubric\'s score, never a publish verdict.'; + +const UNIQUE_LEGACY_ORGANIZATION_COMPOSITE_EXPLANATION: RuleExplanation = { + rule: 'unique/legacy-organization-composite', + covers: 'what a hand-written organization composite enforces', + paragraphs: [ + 'A declared unique index that lists the organization column itself — `organization_id`, or the ' + + 'object\'s `tenancy.tenantField` — is the hand-written per-organization composite that ' + + 'predates the scope vocabulary (ADR-0120 S6). It reads as "unique per organization" but ' + + 'materializes as a plain composite, and SQL UNIQUE is NULL-distinct: it enforces nothing on a ' + + 'row whose organization column is NULL, which on a single-organization deployment is every ' + + 'row.', + 'Respelled `unique: \'organization\'`, the NULL rows become one platform bucket ' + + '(`COALESCE(organization_id, \'__global__\')`) that is unique among themselves, which is what ' + + 'closes the hole.', + 'Advisory, and never auto-fixed (ADR-0120 D5c): the legacy spelling stays valid indefinitely ' + + 'and forces no drift, and opting in is a real physical tightening that goes through the D4 ' + + 'ceremony, because rows the void constraint admitted may still be there. Not reported: ' + + '`unique: \'organization\'` itself, and a unique on the organization column alone, which is ' + + 'not a composite.', + ], +}; + +const UNIQUE_UNSCOPED_DECLARED_INDEX_EXPLANATION: RuleExplanation = { + rule: 'unique/unscoped-declared-index', + covers: 'why a declared unique index must state its scope', + paragraphs: [ + 'Bare `unique: true` on a declared index states no scope (ADR-0120 D5a). It built the index ' + + 'over exactly its `fields` — installation-wide — while reading like "unique per ' + + 'organization", so on an organization-scoped object a value one organization holds refused ' + + 'the same value in every other. The rule fires on the spelling alone, with no tenancy ' + + 'inference: `organization_id` is kernel-injected at registration, so a guess from the ' + + 'authored fields would be wrong half the time.', + UNIQUE_SCOPE_WORDS, + 'Protocol 18 refuses the spelling (ADR-0120 D7) through two channels. The schema ' + + '(`IndexSchema.unique`) refuses it at every door that parses — `os validate`, `os build`, ' + + '`ObjectSchema.create` and the runtime save door — and this rule refuses it where nothing ' + + 'parses: `os lint`, which judges the normalized stack. Stored metadata that still carries it ' + + 'converts to `unique: \'global\'`, which builds the same physical index.', + ], +}; + +const UNIQUE_DOUBLE_DECLARATION_EXPLANATION: RuleExplanation = { + rule: 'unique/double-declaration', + covers: 'how a field unique and an index unique combine', + paragraphs: [ + 'One column carries both a field-level `unique` and a declared single-column unique index, and ' + + 'the two are judged in the scope vocabulary (ADR-0120 D5b), from the two spellings alone with ' + + 'no tenancy inference.', + UNIQUE_SCOPE_WORDS, + 'Different scopes CONTRADICT: the installation-wide index is physically stricter and wins, so ' + + 'the per-organization constraint can never be tripped and one of the two declared intents is ' + + 'silently dead. The same scope on both sides is REDUNDANT — the same index declared twice — ' + + 'and dropping one gives the intent a single home.', + 'A composite declared index is not judged here: listing the organization column is the legacy ' + + 'organization spelling, `unique/legacy-organization-composite`\'s question. Advisory: the ' + + 'stack is well-defined, and the cost is an intent that never takes effect.', + ], +}; + +const RELATIONSHIP_MASTER_DETAIL_REQUIRED_EXPLANATION: RuleExplanation = { + rule: 'relationship/master-detail-required', + covers: 'why a master_detail reference must be required', + paragraphs: [ + 'A detail record cannot exist without its master, so a `master_detail` reference should be ' + + '`required: true`. On most objects nothing at runtime refuses a detail saved without one, so ' + + 'there the rule is a warning.', + 'On a `sharingModel: \'controlled_by_parent\'` object it is an error. Such an object derives ALL ' + + 'of its record access through the master reference (ADR-0055), and the derived read filter — ' + + '`masterFK IN (accessible master ids)` — never matches an empty master, so a detail saved ' + + 'without one is readable by nobody and refused on every later write. Record validation never ' + + 'checks a field that is not `required`, and skips `readonly` and `system` fields before its ' + + 'required check, so on the three shapes reported — `required` absent or false, ' + + '`required: true` with `readonly: true`, and `required: true` with `system: true` — the ' + + 'security gate (`assertControlledByParentWrite`) is the only thing refusing such an insert.', + 'Every `master_detail` field of such an object is judged, not only the one the runtime resolves ' + + 'as the master: that is the scope `ObjectSchema.create()` enforces when it forces ' + + '`required: true` and refuses an explicit `required: false`. The builder never inspects ' + + '`readonly` or `system`, and a plain object literal or a raw `.parse()` of stored metadata ' + + 'never runs it, so this rule is where all three shapes meet a refusal at authoring time. One ' + + 'finding per field, located at its first defect.', + DATA_MODEL_SWEEP_REACH, + ], +}; + +const ROLLUP_NON_NUMERIC_AGGREGAND_EXPLANATION: RuleExplanation = { + rule: 'rollup/non-numeric-aggregand', + covers: 'why a min/max roll-up needs a numeric child field', + paragraphs: [ + 'A `summary` field persists its roll-up\'s answer, and its value contract is a finite number: ' + + '`summary` is a member of the spec\'s `NUMERIC_VALUE_TYPES`. `min` and `max` answer with a ' + + 'value of the CHILD field\'s own type, and the engine stores the driver\'s answer verbatim, so ' + + 'an ordinary "latest shipment" roll-up — `max` over a `datetime` child — computes an instant ' + + 'into a number column.', + 'Accepted children are the numeric value types and the boolean ones, which aggregate as numbers ' + + 'on every backend. This is deliberately not the analytics aggregate table, which accepts a ' + + 'temporal `min` / `max` because a dataset measure RETURNS its answer to the caller instead of ' + + 'storing it.', + 'Judged: `min` and `max` only — `count` reads no value off the field, and `sum` / `avg` over a ' + + 'non-numeric child is a different shape. Silent when the child object or field does not ' + + 'resolve in this stack (another package, a partial load): a rule that cannot answer does not ' + + 'block.', + DATA_MODEL_SWEEP_REACH, + ], +}; + +const RELATIONSHIP_DELETE_BEHAVIOR_EXPLANATION: RuleExplanation = { + rule: 'relationship/delete-behavior', + covers: 'what deleting a master does to its details', + paragraphs: [ + 'On a `master_detail` field the engine resolves every `deleteBehavior` except `restrict` — an ' + + 'unset one included — to `cascade`: deleting the master deletes its details. Declaring the ' + + 'value states that choice where a reader can see it: `cascade` accepts it, and `restrict` ' + + 'refuses to delete a master that still has details.', + '`set_null` is not a choice here. A detail cannot outlive its master — a nulled master reference ' + + 'would orphan it — so an authored `deleteBehavior: \'set_null\'` on a `master_detail` is ' + + 'refused at the parse. Where children must survive their parent, the relationship is a ' + + '`lookup`, not a `master_detail`.', + DATA_MODEL_SWEEP_REACH, + ], +}; + +// ── AI agent authoring (`validate-ai-agent-authoring.ts`) ────────────────── + +/** + * The platform agent roster and its aliases (all three ids). The aliases are + * held to the rule's own table by its tests, which run the rule on each. + */ +const PLATFORM_AGENT_ROSTER = + 'The kernel ships exactly two agents, `ask` and `build`, and the surface the user is in binds one ' + + '(ADR-0063 §2). Their retired spellings — `data_chat` for `ask`, `metadata_assistant` for ' + + '`build` — are registered one way in the alias registry at plugin init: resolution only, kept ' + + 'for old bookmarks and persisted `agent_id`s, never separate records, and the agent catalog ' + + 'shows each agent once under its canonical name.'; + +const DEFAULT_AGENT_LEGACY_ALIAS_EXPLANATION: RuleExplanation = { + rule: 'default-agent-legacy-alias', + covers: 'why a retired agent alias is the wrong spelling', + paragraphs: [ + PLATFORM_AGENT_ROSTER, + 'So an aliased `defaultAgent` is not broken: it resolves, and the app gets the agent it meant. ' + + 'What is wrong is the spelling in the artifact, and it is also the weaker pin: resolution ' + + 'depends on the owning package\'s in-process alias registration having run, which the ' + + 'canonical id does not. A warning for that reason, and an id of its own beside ' + + '`default-agent-outside-roster`, because an alias resolves and an unknown name does not.', + ], +}; + +const DEFAULT_AGENT_OUTSIDE_ROSTER_EXPLANATION: RuleExplanation = { + rule: 'default-agent-outside-roster', + covers: 'what an app\'s defaultAgent resolves against', + paragraphs: [ + PLATFORM_AGENT_ROSTER, + '`app.defaultAgent` is resolved against those two names and their aliases only. An unrecognized ' + + 'name is not rejected: it silently falls back to the platform default at runtime (ADR-0063 ' + + '§1), so the pin has no effect and the value drifts from what actually serves the app.', + 'The key is a plain snake_case identifier in the schema, so any value parses, validates and ' + + 'builds. The check is a warning rather than a schema enum on purpose: narrowing the key is a ' + + 'breaking authoring change ADR-0063 already walked back once.', + ], +}; + +const AGENT_AUTHORING_WITHDRAWN_EXPLANATION: RuleExplanation = { + rule: 'agent-authoring-withdrawn', + covers: 'why a stack-declared agent never runs', + paragraphs: [ + PLATFORM_AGENT_ROSTER, + 'ADR-0063 §2 withdrew tenant and app-package custom agents: third parties extend the platform ' + + 'by authoring skills. The runtime enforces it on both paths — `listAgents()` filters ' + + 'non-platform records out of the catalog and `loadAgent()` refuses them — so a stack-authored ' + + 'agent 404s on chat and cannot be pinned through `app.defaultAgent`: it parses, validates and ' + + 'ships as inert metadata.', + 'A declaration named after a platform agent — `ask`, `build`, or an alias of one — is a ' + + 'different case: it shadows the platform\'s record, which the runtime serves for that name ' + + 'while ignoring this one, so the declaration has no effect and drifts from the platform\'s ' + + 'definition.', + 'A warning, not an error: the platform\'s own packages legitimately author agent records, and ' + + 'the rule cannot tell a platform package from an app package by reading the stack alone — the ' + + 'runtime is what gates. Not a schema refinement either: an existing stack must keep parsing ' + + '(ADR-0078).', + ], +}; + +// ── View references (`lint-view-refs.ts`) ────────────────────────────────── + +const VIEW_REF_NAV_VIEW_MISSING_EXPLANATION: RuleExplanation = { + rule: 'view-ref-nav-view-missing', + covers: 'how a navigation viewName is resolved', + paragraphs: [ + 'An app navigation entry\'s `viewName` names a list view of its object, and an unresolvable name ' + + 'does not fail: the console falls back to the object\'s default view, else its first list ' + + 'view, and keeps the entry\'s authored label and icon. The sidebar reads correctly while ' + + 'opening the wrong view, `os validate` and `os build` stay green, and renaming a list view ' + + 'silently degrades every entry that points at it; the console\'s only signal is a ' + + 'browser-console warning.', + 'The name is matched the way the console\'s `resolveViewId` matches it, in all three ' + + 'directions: the exact id, a short name retried as `OBJECT.NAME`, and a qualified name ' + + 'retried with the `OBJECT.` prefix stripped. `all` is not special-cased: it resolves only ' + + 'when the object declares it. A name that resolves to a FORM view of the object is still ' + + 'reported, because the object\'s view switcher never offers one. The verdict quotes at most ' + + 'three of the object\'s list views.', + 'An error, because it fires only when this stack declared a non-empty list-view namespace for ' + + 'the object. Skipped: an object this stack does not declare or contributes no expandable list ' + + 'view for, an entry carrying `requiresObject` (another package provides the object), an ' + + 'interpolated name, and an entry with `recordId` (the schema ignores `viewName` there). A view ' + + 'saved at runtime is invisible to any author-time pass.', + ], +}; + +const VIEW_KEY_COLLISION_EXPLANATION: RuleExplanation = { + rule: 'view-key-collision', + covers: 'what a renamed view key does to its references', + paragraphs: [ + 'List and form views share one `OBJECT.KEY` namespace when a view container is expanded. A ' + + 'colliding key is renamed — `OBJECT.KEY` becomes `OBJECT.KEY_2` — so the registry key stays ' + + 'unique, and every reference to the requested name resolves to the OTHER view: a ' + + '`type: \'form\'` action target and a navigation `viewName` alike.', + 'A warning, not an error: the rename alone breaks something only when the name is referenced, ' + + 'and a reference that lands on the wrong kind of view is reported on its own, by ' + + '`view-ref-form-target-kind` or `view-ref-nav-view-missing`.', + ], +}; + +// ── Chart bindings (`validate-chart-bindings.ts`) ────────────────────────── + +/** Which chart surfaces the two ids judge, and which they leave alone (both write it out). */ +const CHART_BINDING_SURFACES = + 'Judged on the chart surfaces the dashboard rule does not reach: a report\'s own selection ' + + '(`rows`, `columns`, `values`) and its chart, a list-view chart, and a dataset-bound page chart ' + + 'component. Not judged: the react `ObjectChart` block, which is object-bound and keyed by raw ' + + 'field names (`validate-react-page-props` judges it), and the chart of a `joined` report or of a ' + + 'report block, which no renderer draws (their own selections are judged).'; + +const CHART_MEASURE_UNKNOWN_EXPLANATION: RuleExplanation = { + rule: 'chart-measure-unknown', + covers: 'which chart positions bind a measure, per surface', + paragraphs: [ + 'Post-ADR-0021 a dataset query\'s result rows are keyed by MEASURE NAME (`sum_amount`), not by ' + + 'the base field (`amount`). A position that names a raw field instead of a declared measure ' + + 'still renders, and the series it feeds comes back empty.', + 'At a QUERY position the name is a binding, and an unknown one is an error: a report\'s ' + + '`values` and its chart\'s `yAxis` (the embedded report chart queries `chart.xAxis` × ' + + '`chart.yAxis` itself), a list-view chart\'s `values`, and a page chart\'s `values`.', + 'At a PRESENTATION position the renderer does not bind the name, so an unknown one is a ' + + 'warning: a report `chart.series[]` entry is a per-measure display-name override, paired with ' + + 'the derived series whose key it equals and ignored when it names no declared measure; a page ' + + 'chart\'s `series[]` is replaced wholesale by one derived entry per selected measure; and a ' + + 'page chart\'s `yAxis[].field` keeps its slot (the count turns on a secondary axis) and its ' + + 'scale, while the plotted columns come from `values`.', + CHART_BINDING_SURFACES, + ], +}; + +const CHART_AXIS_NOT_SELECTED_EXPLANATION: RuleExplanation = { + rule: 'chart-axis-not-selected', + covers: 'which selection a chart position is measured against', + paragraphs: [ + 'The entry sits at a PRESENTATION position — a report `chart.series[]` override, or a page ' + + 'chart\'s `series[]` or `yAxis[].field` — and names a measure the dataset declares but the ' + + 'chart does not select. No series is derived for it, so the entry lands on nothing and the ' + + 'chart drawn is unaffected. Advisory: the selection may legitimately be widened at runtime. A ' + + 'query position is never reported: it IS the selection, or the query itself.', + 'The selection is per surface. On a page chart it is `values`, the measure set the query asks ' + + 'for. On a report it is the chart\'s own `chart.yAxis`: the embedded chart queries ' + + 'exactly one dimension × one measure and derives ONE series, while `report.values` is the ' + + 'selection of the table beneath it. So a report\'s `chart.yAxis` is never reported — a chart ' + + 'cannot fail to select what it queries — and a `chart.series[].name` override lands only when ' + + 'it names `chart.yAxis`. The verdict quotes at most three names of the selection.', + CHART_BINDING_SURFACES, + ], +}; + /** * Every rule explanation this package ships, keyed by rule id. `os explain @@ -2126,6 +2464,21 @@ export const RULE_EXPLANATIONS: Readonly> = Obje [ACTION_DISPATCH_CONTRACT_MISMATCH_EXPLANATION.rule]: ACTION_DISPATCH_CONTRACT_MISMATCH_EXPLANATION, [COMPONENT_TYPE_UNKNOWN_EXPLANATION.rule]: COMPONENT_TYPE_UNKNOWN_EXPLANATION, [FILTER_PRESET_COMPARAND_EXPLANATION.rule]: FILTER_PRESET_COMPARAND_EXPLANATION, + [APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY_EXPLANATION.rule]: APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY_EXPLANATION, + [APPROVAL_APPROVER_NOT_MEMBERSHIP_TIER_EXPLANATION.rule]: APPROVAL_APPROVER_NOT_MEMBERSHIP_TIER_EXPLANATION, + [UNIQUE_LEGACY_ORGANIZATION_COMPOSITE_EXPLANATION.rule]: UNIQUE_LEGACY_ORGANIZATION_COMPOSITE_EXPLANATION, + [UNIQUE_UNSCOPED_DECLARED_INDEX_EXPLANATION.rule]: UNIQUE_UNSCOPED_DECLARED_INDEX_EXPLANATION, + [UNIQUE_DOUBLE_DECLARATION_EXPLANATION.rule]: UNIQUE_DOUBLE_DECLARATION_EXPLANATION, + [RELATIONSHIP_MASTER_DETAIL_REQUIRED_EXPLANATION.rule]: RELATIONSHIP_MASTER_DETAIL_REQUIRED_EXPLANATION, + [ROLLUP_NON_NUMERIC_AGGREGAND_EXPLANATION.rule]: ROLLUP_NON_NUMERIC_AGGREGAND_EXPLANATION, + [RELATIONSHIP_DELETE_BEHAVIOR_EXPLANATION.rule]: RELATIONSHIP_DELETE_BEHAVIOR_EXPLANATION, + [DEFAULT_AGENT_LEGACY_ALIAS_EXPLANATION.rule]: DEFAULT_AGENT_LEGACY_ALIAS_EXPLANATION, + [DEFAULT_AGENT_OUTSIDE_ROSTER_EXPLANATION.rule]: DEFAULT_AGENT_OUTSIDE_ROSTER_EXPLANATION, + [AGENT_AUTHORING_WITHDRAWN_EXPLANATION.rule]: AGENT_AUTHORING_WITHDRAWN_EXPLANATION, + [VIEW_REF_NAV_VIEW_MISSING_EXPLANATION.rule]: VIEW_REF_NAV_VIEW_MISSING_EXPLANATION, + [VIEW_KEY_COLLISION_EXPLANATION.rule]: VIEW_KEY_COLLISION_EXPLANATION, + [CHART_MEASURE_UNKNOWN_EXPLANATION.rule]: CHART_MEASURE_UNKNOWN_EXPLANATION, + [CHART_AXIS_NOT_SELECTED_EXPLANATION.rule]: CHART_AXIS_NOT_SELECTED_EXPLANATION, }); /** The explanation for `rule`, or `undefined` when the rule has none. Exact id match. */ diff --git a/packages/lint/src/validate-ai-agent-authoring.ts b/packages/lint/src/validate-ai-agent-authoring.ts index 3bb8a9509ff..99799d54bd0 100644 --- a/packages/lint/src/validate-ai-agent-authoring.ts +++ b/packages/lint/src/validate-ai-agent-authoring.ts @@ -160,15 +160,14 @@ export function validateAiAgentAuthoring(stack: AnyRec): AiAgentAuthoringFinding rule: AGENT_AUTHORING_WITHDRAWN, where: `agent "${name}"`, path: `agents[${ai}]`, + // [#22161] One verdict per arm; the withdrawal, how the runtime enforces + // it and why this is a warning are `os explain` text. message: isPlatformName - ? `This stack declares an agent named "${name}", which is a PLATFORM agent id. The ` + - `runtime serves its own record for that name and ignores this one — the declaration ` + - `has no effect and will drift from the platform's definition.` - : `This stack declares the agent "${name}", but tenant/app-package agents were withdrawn ` + - `(ADR-0063 §2): the kernel ships exactly two agents (\`ask\`, \`build\`) and the surface ` + - `the user is in binds one. The runtime filters this record out of the agent catalog and ` + - `refuses to load it, so it never runs — it parses, validates, and ships as inert ` + - `metadata.`, + ? `This stack declares an agent named "${name}", which is a PLATFORM agent id, so the ` + + `runtime serves its own record for that name and this declaration has no effect` + : `This stack declares the agent "${name}", but app-package agents were withdrawn ` + + `(ADR-0063 §2): the runtime filters it out of the agent catalog and refuses to load it, ` + + `so it never runs`, hint: isPlatformName ? `Remove the declaration; the platform owns "${name}". Extend it with skills instead.` : `Delete the agent and express its capability as skills. Everything an agent carried ` + @@ -202,13 +201,11 @@ export function validateAiAgentAuthoring(stack: AnyRec): AiAgentAuthoringFinding rule: DEFAULT_AGENT_LEGACY_ALIAS, where: `app "${appName}".defaultAgent`, path: `apps[${appIdx}].defaultAgent`, + // [#22161] One verdict; what the alias registry is for and why the + // aliased pin is the weaker one are `os explain` text. message: - `app "${appName}" pins \`defaultAgent\` to "${defaultAgent}", the RETIRED alias of the ` + - `platform agent "${canonical}". It still resolves — the alias registry maps legacy ` + - `names to canonical ones for old bookmarks and persisted \`agent_id\`s (ADR-0063 §2) — ` + - `so nothing is broken at runtime; what is wrong is the spelling in the artifact. It is ` + - `also the weaker pin: resolution depends on the owning package's in-process alias ` + - `registration having run, which the canonical id does not.`, + `app "${appName}" pins \`defaultAgent\` to "${defaultAgent}", the RETIRED alias of ` + + `platform agent "${canonical}": it still resolves, but only through the alias registry`, hint: `Write \`defaultAgent: '${canonical}'\`. The aliases are back-compat resolution, not ` + `authoring vocabulary — always author the canonical id (${roster}).`, @@ -221,12 +218,11 @@ export function validateAiAgentAuthoring(stack: AnyRec): AiAgentAuthoringFinding rule: DEFAULT_AGENT_OUTSIDE_ROSTER, where: `app "${appName}".defaultAgent`, path: `apps[${appIdx}].defaultAgent`, + // [#22161] One verdict; how the key resolves and why it is not a schema + // enum are `os explain` text. message: `app "${appName}" pins \`defaultAgent\` to "${defaultAgent}", which is not a platform ` + - `agent (${roster}). The kernel ships exactly two agents (ADR-0063 §2) and resolves this ` + - `key against them and their legacy aliases only — an unrecognized name is not rejected, ` + - `it silently falls back to the platform default at runtime, so the pin has no effect and ` + - `the value drifts from what actually serves the app.`, + `agent (${roster}), so it silently falls back to the platform default and the pin has no effect`, hint: `Set \`defaultAgent\` to one of the platform agent names: ${roster}. If the goal is a ` + `dedicated persona or capability, express it as skills instead — they attach to "ask" ` + diff --git a/packages/lint/src/validate-approval-approvers.ts b/packages/lint/src/validate-approval-approvers.ts index 143561c356c..19f0419c54f 100644 --- a/packages/lint/src/validate-approval-approvers.ts +++ b/packages/lint/src/validate-approval-approvers.ts @@ -97,6 +97,14 @@ const RESERVED_OUTPUT_KEYS = new Set(['decision', 'requestId']); */ const GROUP_ROUTED_TYPES = new Set(['position', 'team', 'department']); +/** + * [#22161] The closing clause both empty-slate verdicts share under the + * default `lockRecord: true` — one wording, so the two arms cannot drift. What + * recovers such a record is the `approval-approvers-may-resolve-empty` + * explanation's text (`os explain`), not the verdict's. + */ +const STAYS_LOCKED = ', and (lockRecord) the record stays locked'; + /** * How an operator actually populates `sys_user.manager_id` (#16748 / #16678). * @@ -272,8 +280,9 @@ const TYPE_FIX: Record = { * non-empty `manager_id` demonstrably knows about the column and populates it, * so the advisory below would be noise there. A stack that seeds none has given * the linter nothing, which is not the same as the column being unset at - * runtime — hence the advisory's wording, which flags the SHAPE and explicitly - * does not assert the slate is empty. + * runtime — hence the advisory's wording, which flags the SHAPE and is + * conditional ("where … is unset"): it does not assert the slate is empty, and + * its `os explain` entry says why. * * ⛔ This is deliberately NOT "every seeded user has a manager": the top of any * real reporting chain legitimately has none, so an all-rows test would fire on @@ -443,10 +452,11 @@ export function validateApprovalApprovers(stack: AnyRec): ApprovalApproverFindin rule: APPROVAL_APPROVER_NOT_MEMBERSHIP_TIER, where, path: `${path}.value`, + // [#22161] One verdict; what the type resolves against and why a + // business role is a position is `os explain` text. message: - `approver { type: '${type}', value: '${value}' } resolves against the better-auth ` + - `org-membership tier (sys_member.role: ${MEMBERSHIP_TIER_LIST}) — '${value}' is not ` + - `a membership tier, so this approver matches nobody and the request stalls.`, + `approver { type: '${type}', value: '${value}' } names no org-membership tier ` + + `(${MEMBERSHIP_TIER_LIST}), so it matches nobody and the request stalls`, hint: `If '${value}' is an org position, author { type: 'position', value: '${value}' } ` + `(resolved via sys_user_position, ADR-0090 D3). Keep type 'org_membership_level' ` + @@ -533,11 +543,12 @@ export function validateApprovalApprovers(stack: AnyRec): ApprovalApproverFindin rule: APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY, where, path: `${nodePath}.config.approvers`, + // [#22161] One verdict sentence; why staffing is unreadable here and + // what recovers a stuck request is `os explain` text. message: - `every approver on this node routes to a group (position/team/department) whose ` + - `members are runtime data — if none is staffed, the request resolves to an empty ` + - `slate and waits forever` + - (locks ? `, and (lockRecord) the record stays locked with no in-product recovery.` : `.`), + `every approver on this node routes to a group (position/team/department), so if ` + + `none is staffed the request opens on an empty slate and waits forever` + + (locks ? STAYS_LOCKED : ''), hint: `Make sure at least one target is always staffed, or add a guaranteed-staffed ` + `approver entry, e.g. { type: 'org_membership_level', value: 'owner' }, or declare ` + @@ -568,9 +579,9 @@ export function validateApprovalApprovers(stack: AnyRec): ApprovalApproverFindin // verdict, mixed slates included, exactly as it was. // // Advisory (`info`), the same tier as its sibling: this reads the SHAPE. - // It does not read row data, and the message says so rather than + // It does not read row data, so the message is conditional rather than // asserting a slate is empty — a lint rule must not claim a runtime fact - // it did not read. `stackWiresManagerChain` is the one manager-chain fact + // it did not read (the explanation states that limit in full). `stackWiresManagerChain` is the one manager-chain fact // a stack CAN put in front of it, and it silences the advisory. if ( !managerChainWired && @@ -583,13 +594,13 @@ export function validateApprovalApprovers(stack: AnyRec): ApprovalApproverFindin rule: APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY, where, path: `${nodePath}.config.approvers`, + // [#22161] Conditional on purpose — the rule cannot read the column, + // so the verdict says what happens WHERE it is unset and asserts no + // runtime fact; that reasoning is `os explain` text. message: - `every approver on this node is { type: 'manager' }, resolved at runtime from ` + - `sys_user.manager_id of the record's owner — a static check cannot read that column, ` + - `so this does not assert the slate IS empty; it reports that nothing else on the node ` + - `can approve if it is. Where manager_id is unset the expansion returns nobody, the ` + - `request resolves to an empty slate and waits forever` + - (locks ? `, and (lockRecord) the record stays locked with no in-product recovery.` : `.`), + `every approver on this node is { type: 'manager' }, so where sys_user.manager_id ` + + `is unset the request opens on an empty slate and waits forever` + + (locks ? STAYS_LOCKED : ''), hint: `${MANAGER_ONLY_REMEDY} ${MANAGER_ONLY_ROUTES} Populate it for everyone who submits ` + `this request, or take the other escape that needs none of that: add a second approver ` + diff --git a/packages/lint/src/validate-chart-bindings.ts b/packages/lint/src/validate-chart-bindings.ts index 9ececb06e68..c1b134f3e1c 100644 --- a/packages/lint/src/validate-chart-bindings.ts +++ b/packages/lint/src/validate-chart-bindings.ts @@ -318,23 +318,22 @@ type ResolvedBinding = Omit; */ type MeasurePosition = 'query' | 'report-series' | 'page-series' | 'page-axis'; -/** Presentation position → what actually happens when the name resolves to nothing. */ +/** + * Presentation position → what actually happens when the name resolves to + * nothing, as the verdict's one clause. [#22161] Why each position is + * presentation on its surface is the `chart-measure-unknown` explanation + * (`os explain`), not this text. + */ const UNKNOWN_CONSEQUENCE: Record, string> = { 'report-series': - 'On this surface `chart.series[]` is a per-measure DISPLAY-NAME override, not a ' - + 'binding: the renderer derives the series from the chart\'s own `xAxis`/`yAxis` ' - + 'query and pairs an authored entry with the derived series whose key it EQUALS, so ' - + 'an entry naming no declared measure is ignored. The override lands on nothing — ' - + 'the series is still drawn from the dataset selection.', + 'this `chart.series[]` entry is a DISPLAY-NAME override, so it lands on nothing and the ' + + 'series is still drawn', 'page-series': - 'On this surface `series[]` is presentation, not a binding: the renderer derives one ' - + 'series per selected measure and REPLACES the authored array with the derived one, ' - + 'so this entry never reaches the chart at all. It lands on nothing — the chart is ' - + 'still drawn from `dimensions`/`values`.', + 'the renderer REPLACES the authored array with the derived series, so this entry never ' + + 'reaches the chart', 'page-axis': - 'On this surface `yAxis[].field` is axis PRESENTATION, not a binding: the entry keeps ' - + 'its slot (the count is what turns on a secondary axis) and its scale/chrome, while ' - + 'the plotted columns come from `values`. The key re-points nothing.', + 'this `yAxis[].field` is axis PRESENTATION and the plotted columns come from `values`, so ' + + 'it re-points nothing', }; /** Presentation position → the shape sentence in the hint. */ @@ -350,21 +349,29 @@ const SHAPE_HINT: Record, string> = { + 'plotted columns come from `values`.', }; -/** Position → the consequence of naming a DECLARED measure outside the selection. */ +/** + * Position → the consequence of naming a DECLARED measure outside the + * selection, as the verdict's closing clause. [#22161] Which selection each + * surface measures against is the `chart-axis-not-selected` explanation. + */ const UNSELECTED_CONSEQUENCE: Record = { - query: 'the query does not return it, so the series plots nothing.', + query: 'the query does not return it, so the series plots nothing', 'report-series': - 'this entry is a display-name override matched against the ONE series the chart ' - + 'derives (from its own `chart.yAxis` query), so unless it names that measure the ' - + 'override lands on nothing. The series drawn is unaffected.', - 'page-series': - 'the series are derived from `values`, so none is derived for it and this entry ' - + 'lands on nothing. The chart drawn is unaffected.', - 'page-axis': - 'the plotted columns come from `values`, so the axis entry re-points nothing. The ' - + 'chart drawn is unaffected.', + 'this display-name override pairs only with the series the chart derives, so it lands on nothing', + 'page-series': 'the series are derived from `values`, so this entry lands on nothing', + 'page-axis': 'the plotted columns come from `values`, so the axis entry re-points nothing', }; +/** + * [#22161] A chart's selection for a one-line verdict: sorted, at most three, + * then `(and N more)`. + */ +function selectionRoster(names: Iterable): string { + const all = [...names].sort(); + const shown = all.slice(0, 3).join(', '); + return all.length > 3 ? `${shown} (and ${all.length - 3} more)` : shown; +} + export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] { const findings: ChartBindingFinding[] = []; if (!stack || typeof stack !== 'object') return findings; @@ -438,12 +445,14 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] { rule: CHART_MEASURE_UNKNOWN, where: binding.where, path, + // [#22161] One verdict per position; the per-surface reading of + // which positions bind is `os explain` text. message: - `"${name}" is not a measure declared by dataset "${dsName}". ` + + `"${name}" is not a measure declared by dataset "${dsName}"` + (position === 'query' - ? `Post-ADR-0021 result rows are keyed by MEASURE NAME (e.g. "sum_amount"), ` + - `not the base field (e.g. "amount"), so this series comes back empty.` - : UNKNOWN_CONSEQUENCE[position]), + ? `, so this series comes back empty (result rows are keyed by measure name, ` + + `not the base field)` + : `: ${UNKNOWN_CONSEQUENCE[position]}`), hint: `Dataset measures: ${list(ds.measures)}.${suggestName(name, ds.measures)} ` + (position === 'query' @@ -465,8 +474,8 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] { where: binding.where, path, message: - `"${name}" is a declared measure of "${dsName}" but is not in this chart's ` + - `selected values (${list(selected)}) — ${UNSELECTED_CONSEQUENCE[position]}`, + `"${name}" is a declared measure of "${dsName}" outside this chart's ` + + `selected values (${selectionRoster(selected)}): ${UNSELECTED_CONSEQUENCE[position]}`, hint: position === 'report-series' ? `Point the entry at the measure this chart plots (\`chart.yAxis\`), or drop it.` From d9f3e019c87ed2a65946747a9abc6363c1d809a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 21:19:22 +0000 Subject: [PATCH 2/5] test(lint): pin the one-line verdicts and their explanations; changeset Claude-Session: https://claude.ai/code/session_01S3aAf11JjbW1mSGL1EhfFj Co-authored-by: Claude --- .changeset/22161-lint-slice-7-one-line.md | 21 +++ ...model-rules.master-detail-required.test.ts | 94 ++++++++++- .../data-model-rules.summary-rollup.test.ts | 55 +++++- .../data-model-rules.unique-verdicts.test.ts | 158 ++++++++++++++++++ packages/lint/src/lint-view-refs.test.ts | 98 ++++++++++- .../src/validate-ai-agent-authoring.test.ts | 81 ++++++++- .../src/validate-approval-approvers.test.ts | 97 ++++++++++- .../lint/src/validate-chart-bindings.test.ts | 123 +++++++++++++- packages/lint/src/validate-chart-bindings.ts | 4 +- 9 files changed, 721 insertions(+), 10 deletions(-) create mode 100644 .changeset/22161-lint-slice-7-one-line.md create mode 100644 packages/lint/src/data-model-rules.unique-verdicts.test.ts diff --git a/.changeset/22161-lint-slice-7-one-line.md b/.changeset/22161-lint-slice-7-one-line.md new file mode 100644 index 00000000000..e74f39c827e --- /dev/null +++ b/.changeset/22161-lint-slice-7-one-line.md @@ -0,0 +1,21 @@ +--- +"@objectstack/lint": patch +--- + +fix(lint): the approval-approver, data-model, AI agent-authoring, view-reference and chart-binding findings print one verdict line, and `os explain ` carries their reasoning + +Clause-②: no + +- **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 200 characters; before, the longest of each ran from 199 to 478 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 ` 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 ` 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). diff --git a/packages/lint/src/data-model-rules.master-detail-required.test.ts b/packages/lint/src/data-model-rules.master-detail-required.test.ts index 3488b722465..0df5641f6c1 100644 --- a/packages/lint/src/data-model-rules.master-detail-required.test.ts +++ b/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) => + 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 = { + [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); + } + }); +}); diff --git a/packages/lint/src/data-model-rules.summary-rollup.test.ts b/packages/lint/src/data-model-rules.summary-rollup.test.ts index aec6893e22e..33e5d41c9c6 100644 --- a/packages/lint/src/data-model-rules.summary-rollup.test.ts +++ b/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.)`, and a child declaring that field as `childType`. */ const model = (childType: string | undefined, fn = 'max', overrides: Record = {}) => [ { @@ -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); + } + }); +}); diff --git a/packages/lint/src/data-model-rules.unique-verdicts.test.ts b/packages/lint/src/data-model-rules.unique-verdicts.test.ts new file mode 100644 index 00000000000..48902987526 --- /dev/null +++ b/packages/lint/src/data-model-rules.unique-verdicts.test.ts @@ -0,0 +1,158 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +// [#22161] The three ADR-0120 uniqueness rules print one verdict sentence per +// finding; the reasoning they used to carry is each id's `os explain` entry. +// Their behaviour is pinned in `packages/cli/test/data-model-rules.test.ts` and +// their `where` slot in `data-model-rule-where-slot.test.ts`; this file pins the +// verdict SHAPE over every arm the three rules have — each spelling, a named +// and an unnamed index, a declared tenant column, and all four scope quadrants +// of the double declaration — and the explanation that took the rest. + +import { describe, expect, it } from 'vitest'; + +import { + lintLegacyOrganizationComposites, + lintUniqueDeclarations, + lintUnscopedDeclaredIndexes, + UNIQUE_DOUBLE_DECLARATION, + UNIQUE_LEGACY_ORGANIZATION_COMPOSITE, + UNIQUE_UNSCOPED_DECLARED_INDEX, +} from './data-model-rules.js'; +import { explainRule } from './rule-explanations.js'; + +const CONVERTED = [UNIQUE_UNSCOPED_DECLARED_INDEX, UNIQUE_DOUBLE_DECLARATION, UNIQUE_LEGACY_ORGANIZATION_COMPOSITE]; + +const fired: Array<{ rule: string; message: string }> = []; +const record = (issues: T[]): T[] => { + fired.push(...issues); + return issues; +}; + +/** One object whose single column carries a field-level `unique` and a one-column unique index. */ +const twice = (fieldUnique: unknown, indexUnique: unknown, indexName?: string) => [ + { + name: 'crm_product', + fields: { sku: { type: 'text', unique: fieldUnique } }, + indexes: [{ ...(indexName ? { name: indexName } : {}), fields: ['sku'], unique: indexUnique }], + }, +]; + +describe('[#22161] one-line verdicts — the ADR-0120 uniqueness rules', () => { + it('unique/unscoped-declared-index: the index, the spelling, the refusal and what it built', () => { + const issues = record( + lintUnscopedDeclaredIndexes([ + { + name: 'sys_account', + fields: {}, + indexes: [ + { name: 'uniq_org_email', fields: ['organization_id', 'email'], unique: true }, + { fields: ['provider_id', 'account_id'], unique: true }, + ], + }, + ]), + ); + expect(issues.map((i) => i.message)).toEqual([ + '"sys_account" index \'uniq_org_email\' [organization_id, email] has bare `unique: true`, an unstated ' + + 'scope protocol 18 refuses (ADR-0120): it built the index installation-wide', + '"sys_account" index [provider_id, account_id] has bare `unique: true`, an unstated scope protocol 18 ' + + 'refuses (ADR-0120): it built the index installation-wide', + ]); + }); + + it('unique/legacy-organization-composite: the index and the NULL rows it does not hold', () => { + const issues = record( + lintLegacyOrganizationComposites([ + { + name: 'sys_team', + fields: {}, + indexes: [ + { name: 'uk_team', fields: ['name', 'organization_id'], unique: true }, + { fields: ['code', 'organization_id'], unique: 'global' }, + ], + }, + { + name: 'legacy_thing', + tenancy: { tenantField: 'tenant_ref' }, + fields: {}, + indexes: [{ fields: ['code', 'tenant_ref'], unique: true }], + }, + ]), + ); + expect(issues.map((i) => i.message)).toEqual([ + '"sys_team" index \'uk_team\' [name, organization_id] lists the organization column, and SQL UNIQUE is ' + + "NULL-distinct: on every row whose 'organization_id' is NULL it enforces nothing", + '"sys_team" index [code, organization_id] lists the organization column, and SQL UNIQUE is ' + + "NULL-distinct: on every row whose 'organization_id' is NULL it enforces nothing", + '"legacy_thing" index [code, tenant_ref] lists the organization column, and SQL UNIQUE is ' + + "NULL-distinct: on every row whose 'tenant_ref' is NULL it enforces nothing", + ]); + }); + + it('unique/double-declaration: all four scope quadrants, both spellings named', () => { + const verdict = (fieldUnique: unknown, indexUnique: unknown, name?: string) => + record(lintUniqueDeclarations(twice(fieldUnique, indexUnique, name)))[0]!.message; + expect(verdict(true, 'global', 'uniq_product_sku')).toBe( + '"crm_product.sku" field `unique: true` and index \'uniq_product_sku\' `unique: \'global\'` CONTRADICT: ' + + 'the installation-wide one wins, so the per-organization intent is silently dead', + ); + expect(verdict('global', 'organization')).toBe( + '"crm_product.sku" field `unique: \'global\'` and index `unique: \'organization\'` CONTRADICT: the ' + + 'installation-wide one wins, so the per-organization intent is silently dead', + ); + expect(verdict('organization', 'organization')).toBe( + '"crm_product.sku" field `unique: \'organization\'` and index `unique: \'organization\'` both ask for ' + + 'per-organization uniqueness: the same index declared twice', + ); + expect(verdict('global', 'global')).toBe( + '"crm_product.sku" field `unique: \'global\'` and index `unique: \'global\'` both ask for ' + + 'installation-wide uniqueness: the same index declared twice', + ); + }); + + it('every verdict the cases above fired is one line of at most 200 characters', () => { + // The coverage control first: all three ids fired, so the shape assertion + // below cannot pass over an empty or partial record. + expect([...new Set(fired.map((f) => f.rule))].sort()).toEqual([...CONVERTED].sort()); + for (const f of fired) { + 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 = { + [UNIQUE_UNSCOPED_DECLARED_INDEX]: [ + 'ADR-0120 D5a', + 'no tenancy inference', + 'ADR-0120 D7', + '`IndexSchema.unique`', + '`os lint`, which judges the normalized stack', + "converts to `unique: 'global'`", + ], + [UNIQUE_DOUBLE_DECLARATION]: [ + 'ADR-0120 D5b', + 'physically stricter and wins', + 'REDUNDANT', + '`unique/legacy-organization-composite`', + ], + [UNIQUE_LEGACY_ORGANIZATION_COMPOSITE]: [ + 'ADR-0120 S6', + '`tenancy.tenantField`', + 'single-organization deployment is every row', + "`COALESCE(organization_id, '__global__')`", + 'ADR-0120 D5c', + 'D4 ceremony', + ], + }; + 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); + } + // The scope words are one paragraph, printed under both spellings it explains. + const scopeWords = (rule: string) => explainRule(rule)!.paragraphs.find((p) => p.startsWith('ADR-0120 gives')); + expect(scopeWords(UNIQUE_UNSCOPED_DECLARED_INDEX)).toBeDefined(); + expect(scopeWords(UNIQUE_DOUBLE_DECLARATION)).toBe(scopeWords(UNIQUE_UNSCOPED_DECLARED_INDEX)); + }); +}); diff --git a/packages/lint/src/lint-view-refs.test.ts b/packages/lint/src/lint-view-refs.test.ts index db93b1d2e02..b232ae67839 100644 --- a/packages/lint/src/lint-view-refs.test.ts +++ b/packages/lint/src/lint-view-refs.test.ts @@ -2,13 +2,27 @@ import { describe, it, expect } from 'vitest'; import { - lintViewRefs, + lintViewRefs as lintViewRefsUnrecorded, VIEW_KEY_COLLISION, VIEW_REF_FORM_TARGET_MISSING, VIEW_REF_FORM_TARGET_KIND, VIEW_REF_NAV_VIEW_MISSING, } from './lint-view-refs.js'; import { runAuthoringRules, splitBySeverity } from './authoring-rules.js'; +import { explainRule } from './rule-explanations.js'; + +// [#22161] Each finding of the two converted ids is one verdict sentence; the +// reasoning it used to carry is the id's `os explain` entry. Every direct call +// below records what it fired, and the last cases in this file hold each +// recorded verdict of those 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 }> = []; +const lintViewRefs: typeof lintViewRefsUnrecorded = (...args) => { + const findings = lintViewRefsUnrecorded(...args); + fired.push(...findings); + return findings; +}; const listView = (object: string) => ({ type: 'grid', @@ -406,3 +420,85 @@ describe('#14108 acceptance — a nav viewName miss gates `validate` AND `build` }); } }); + +describe('[#22161] one-line verdicts — view-ref-nav-view-missing, view-key-collision', () => { + const CONVERTED = [VIEW_REF_NAV_VIEW_MISSING, VIEW_KEY_COLLISION]; + + it('every verdict the cases above fired is one line of at most 200 characters', () => { + const converted = fired.filter((f) => CONVERTED.includes(f.rule)); + // The coverage control first: both ids fired, and the nav id on both arms. + expect([...new Set(converted.map((f) => f.rule))].sort()).toEqual([...CONVERTED].sort()); + const nav = converted.filter((f) => f.rule === VIEW_REF_NAV_VIEW_MISSING); + expect(nav.some((f) => f.message.includes('resolves to a FORM view'))).toBe(true); + expect(nav.some((f) => !f.message.includes('resolves to a FORM view'))).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('reads as one sentence, and keeps the list views — they are the fix', () => { + const [miss] = navFindings( + navStack({ id: 'nav_schedule', type: 'object', objectName: 'duly_task', viewName: 'A4_no_such_view', label: 'S' }), + ); + expect(miss!.message).toBe( + "Navigation entry opens view 'A4_no_such_view' on object 'duly_task', which declares no such list " + + 'view, so it opens the default view under its own label. List views: board, default, schedule', + ); + const [form] = navFindings(navStack({ id: 'n', type: 'object', objectName: 'duly_task', viewName: 'edit', label: 'E' })); + expect(form!.message).toBe( + "Navigation entry opens view 'edit' on object 'duly_task', which declares no such list view (the " + + 'name resolves to a FORM view), so it opens the default view under its own label. List views: ' + + 'board, default, schedule', + ); + const [collision] = lintViewRefs({ + views: [{ name: 'task', list: listView('task'), formViews: { default: formView('task') } }], + }).filter((f) => f.rule === VIEW_KEY_COLLISION); + expect(collision!.message).toBe( + "View key collision: the form view 'task.default' was renamed to 'task.default_2', so every " + + "reference to 'task.default' resolves to the OTHER view", + ); + }); + + it('quotes at most three list views, then counts the rest', () => { + const many = { + ...NAV_OBJECT, + listViews: { + ...NAV_OBJECT.listViews, + archive: navListView('duly_task', 'Archive', 'title'), + mine: navListView('duly_task', 'Mine', 'title'), + }, + }; + const [miss] = navFindings({ + objects: [many], + apps: [{ name: 'duly', navigation: [{ id: 'n', type: 'object', objectName: 'duly_task', viewName: 'nope', label: 'N' }] }], + }); + expect(miss!.message).toMatch(/List views: archive, board, default \(and 2 more\)$/); + }); + + it('`os explain` carries what the verdicts no longer say', () => { + const facts: Record = { + [VIEW_REF_NAV_VIEW_MISSING]: [ + 'authored label and icon', + 'browser-console warning', + '`resolveViewId`', + '`all` is not special-cased', + 'view switcher never offers one', + '`requiresObject`', + '`recordId`', + ], + [VIEW_KEY_COLLISION]: [ + '`OBJECT.KEY_2`', + "`type: 'form'` action target", + 'navigation `viewName`', + '`view-ref-form-target-kind`', + ], + }; + 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); + } + }); +}); diff --git a/packages/lint/src/validate-ai-agent-authoring.test.ts b/packages/lint/src/validate-ai-agent-authoring.test.ts index 8955be1b089..c60e137aeae 100644 --- a/packages/lint/src/validate-ai-agent-authoring.test.ts +++ b/packages/lint/src/validate-ai-agent-authoring.test.ts @@ -2,11 +2,24 @@ import { describe, it, expect } from 'vitest'; import { - validateAiAgentAuthoring, + validateAiAgentAuthoring as validateAiAgentAuthoringUnrecorded, AGENT_AUTHORING_WITHDRAWN, DEFAULT_AGENT_OUTSIDE_ROSTER, DEFAULT_AGENT_LEGACY_ALIAS, } from './validate-ai-agent-authoring.js'; +import { explainRule } from './rule-explanations.js'; + +// [#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 to one line of at most +// 200 characters — every firing variant this suite exercises, not a chosen few. +// Run the whole file: those cases read what the cases above fired. +const fired: Array<{ rule: string; message: string }> = []; +const validateAiAgentAuthoring: typeof validateAiAgentAuthoringUnrecorded = (...args) => { + const findings = validateAiAgentAuthoringUnrecorded(...args); + fired.push(...findings); + return findings; +}; describe('validate-ai-agent-authoring', () => { it('is silent for the stack every app package should be — no agents at all', () => { @@ -201,3 +214,69 @@ describe('validate-ai-agent-authoring', () => { }); }); }); + +describe('[#22161] one-line verdicts — the three agent-authoring ids', () => { + const IDS = [AGENT_AUTHORING_WITHDRAWN, DEFAULT_AGENT_OUTSIDE_ROSTER, DEFAULT_AGENT_LEGACY_ALIAS]; + + it('every verdict the cases above fired is one line of at most 200 characters', () => { + // The coverage control first: all three ids fired, and both arms of the + // declaration id, so the shape assertion cannot pass over a partial record. + expect([...new Set(fired.map((f) => f.rule))].sort()).toEqual([...IDS].sort()); + const declared = fired.filter((f) => f.rule === AGENT_AUTHORING_WITHDRAWN); + expect(declared.some((f) => f.message.includes('PLATFORM agent id'))).toBe(true); + expect(declared.some((f) => f.message.includes('were withdrawn'))).toBe(true); + for (const f of fired) { + expect(f.message, f.rule).not.toContain('\n'); + expect(f.message.length, `${f.rule}: ${f.message}`).toBeLessThanOrEqual(200); + } + }); + + it('reads as one sentence per arm', () => { + const findings = validateAiAgentAuthoring({ + agents: [{ name: 'sales_copilot' }, { name: 'build' }], + apps: [ + { name: 'studio', defaultAgent: 'metadata_assistant' }, + { name: 'crm', defaultAgent: 'sales_copilot' }, + ], + }); + expect(findings.map((f) => f.message)).toEqual([ + 'This stack declares the agent "sales_copilot", but app-package agents were withdrawn (ADR-0063 §2): ' + + 'the runtime filters it out of the agent catalog and refuses to load it, so it never runs', + 'This stack declares an agent named "build", which is a PLATFORM agent id, so the runtime serves ' + + 'its own record for that name and this declaration has no effect', + 'app "studio" pins `defaultAgent` to "metadata_assistant", the RETIRED alias of platform agent ' + + '"build": it still resolves, but only through the alias registry', + 'app "crm" pins `defaultAgent` to "sales_copilot", which is not a platform agent (ask, build), so ' + + 'it silently falls back to the platform default and the pin has no effect', + ]); + }); + + it('`os explain` carries what the verdicts no longer say', () => { + const facts: Record = { + [AGENT_AUTHORING_WITHDRAWN]: ['`listAgents()`', '`loadAgent()`', '404s on chat', 'inert metadata', 'ADR-0078'], + [DEFAULT_AGENT_OUTSIDE_ROSTER]: ['ADR-0063 §1', 'snake_case identifier', 'walked back once'], + [DEFAULT_AGENT_LEGACY_ALIAS]: ['persisted `agent_id`s', 'the weaker pin', 'in-process alias registration'], + }; + 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); + } + }); + + it("the roster paragraph's aliases are the ones the rule resolves", () => { + // The explanation module imports nothing, so it spells the aliases out; + // this holds each spelling to the rule by running it. + const roster = explainRule(DEFAULT_AGENT_LEGACY_ALIAS)!.paragraphs[0]!; + for (const [alias, canonical] of [['data_chat', 'ask'], ['metadata_assistant', 'build']] as const) { + expect(roster).toContain(`\`${alias}\` for \`${canonical}\``); + const [finding] = validateAiAgentAuthoringUnrecorded({ apps: [{ name: 'a', defaultAgent: alias }] }); + expect(finding).toMatchObject({ rule: DEFAULT_AGENT_LEGACY_ALIAS }); + expect(finding!.message).toContain(`platform agent "${canonical}"`); + } + // One paragraph, printed under all three ids it explains. + expect(explainRule(DEFAULT_AGENT_OUTSIDE_ROSTER)!.paragraphs[0]).toBe(roster); + expect(explainRule(AGENT_AUTHORING_WITHDRAWN)!.paragraphs[0]).toBe(roster); + }); +}); diff --git a/packages/lint/src/validate-approval-approvers.test.ts b/packages/lint/src/validate-approval-approvers.test.ts index a12dfda79bf..9fa294e12ed 100644 --- a/packages/lint/src/validate-approval-approvers.test.ts +++ b/packages/lint/src/validate-approval-approvers.test.ts @@ -1,8 +1,9 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; +import { BUILTIN_MEMBERSHIP_ROLES } from '@objectstack/spec'; import { - validateApprovalApprovers, + validateApprovalApprovers as validateApprovalApproversUnrecorded, APPROVAL_APPROVER_NOT_MEMBERSHIP_TIER, APPROVAL_APPROVER_TYPE_DEPRECATED, APPROVAL_APPROVER_TYPE_UNKNOWN, @@ -14,6 +15,20 @@ import { APPROVAL_DECISION_OUTPUTS_RESERVED, APPROVAL_APPROVER_CROSS_ORG_UNSUPPORTED, } from './validate-approval-approvers.js'; +import { explainRule } from './rule-explanations.js'; + +// [#22161] Each finding of the two converted ids 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 those ids to one line of at most 200 characters — so the pin +// covers every firing variant this suite exercises, not a chosen few. Run the +// whole file: those cases read what the cases above fired. +const fired: Array<{ rule: string; message: string }> = []; +const validateApprovalApprovers: typeof validateApprovalApproversUnrecorded = (...args) => { + const findings = validateApprovalApproversUnrecorded(...args); + fired.push(...findings); + return findings; +}; function stackWithApprovers(approvers: unknown[]): Record { return { @@ -348,8 +363,12 @@ describe('unset-manager dead-end (#16748)', () => { it('does not claim a runtime fact it did not read', () => { const [finding] = validateApprovalApprovers(managerOnly()); - expect(finding.message).toContain('a static check cannot read that column'); - expect(finding.message).toContain('does not assert the slate IS empty'); + // [#22161] The verdict is CONDITIONAL — it says what happens where the + // column is unset, never that it is — and the explanation states the limit. + expect(finding.message).toContain('where sys_user.manager_id is unset'); + const text = explainRule(APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY)!.paragraphs.join('\n'); + expect(text).toContain('a static check cannot read that column'); + expect(text).toContain('does not assert the slate IS empty'); }); it("names the node-level escape — onEmptyApprovers: 'fallback' with fallbackApprovers", () => { @@ -693,3 +712,75 @@ describe('nested regions', () => { expect(findings.some((f) => f.rule === APPROVAL_APPROVER_TYPE_UNKNOWN)).toBe(true); }); }); + +describe('[#22161] one-line verdicts — approval-approvers-may-resolve-empty, approval-approver-not-membership-tier', () => { + const CONVERTED = [APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY, APPROVAL_APPROVER_NOT_MEMBERSHIP_TIER]; + + it('every verdict the cases above fired is one line of at most 200 characters', () => { + const converted = fired.filter((f) => CONVERTED.includes(f.rule)); + // The coverage control first: both ids fired, and both arms of the + // empty-slate id with and without the lock clause, so the shape assertion + // below cannot pass over an empty or partial record. + expect([...new Set(converted.map((f) => f.rule))].sort()).toEqual([...CONVERTED].sort()); + const emptySlate = converted.filter((f) => f.rule === APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY); + expect(emptySlate.some((f) => f.message.includes('routes to a group'))).toBe(true); + expect(emptySlate.some((f) => f.message.includes("{ type: 'manager' }"))).toBe(true); + expect(emptySlate.some((f) => f.message.includes('locked'))).toBe(true); + expect(emptySlate.some((f) => !f.message.includes('locked'))).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('the two empty-slate arms read as one sentence each, sharing the lock clause', () => { + const [group] = validateApprovalApprovers(stackWithApprovers([{ type: 'position', value: 'exec' }])); + expect(group.message).toBe( + 'every approver on this node routes to a group (position/team/department), so if none is ' + + 'staffed the request opens on an empty slate and waits forever, and (lockRecord) the record ' + + 'stays locked', + ); + const [manager] = validateApprovalApprovers(stackWithApprovers([{ type: 'manager' }])); + expect(manager.message).toBe( + "every approver on this node is { type: 'manager' }, so where sys_user.manager_id is unset the " + + 'request opens on an empty slate and waits forever, and (lockRecord) the record stays locked', + ); + }); + + it('the membership-tier verdict names the tiers from the spec, so it cannot contradict them', () => { + const [finding] = validateApprovalApprovers( + stackWithApprovers([{ type: 'org_membership_level', value: 'sales_manager' }]), + ); + expect(finding.message).toBe( + "approver { type: 'org_membership_level', value: 'sales_manager' } names no org-membership tier " + + `(${BUILTIN_MEMBERSHIP_ROLES.join('/')}), so it matches nobody and the request stalls`, + ); + }); + + it('`os explain` carries what the verdicts no longer say', () => { + const facts: Record = { + [APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY]: [ + '`pending_approvers`', + 'no in-product recovery', + '`sys_user.manager_id`', + 'a seed row of `sys_user`', + "`onEmptyApprovers: 'fallback'` does not silence it", + 'The runtime publish gate carries no seeds', + ], + [APPROVAL_APPROVER_NOT_MEMBERSHIP_TIER]: [ + '`sys_member.role`', + 'ADR-0108', + '`BUILTIN_MEMBERSHIP_ROLES`', + 'ADR-0090 D3', + '`sys_user_position`', + '`approval-approver-type-deprecated`', + ], + }; + 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); + } + }); +}); diff --git a/packages/lint/src/validate-chart-bindings.test.ts b/packages/lint/src/validate-chart-bindings.test.ts index 29e7fa389dc..a368dacf949 100644 --- a/packages/lint/src/validate-chart-bindings.test.ts +++ b/packages/lint/src/validate-chart-bindings.test.ts @@ -2,12 +2,26 @@ import { describe, it, expect } from 'vitest'; import { - validateChartBindings, + validateChartBindings as validateChartBindingsUnrecorded, CHART_DIMENSION_UNKNOWN, CHART_MEASURE_UNKNOWN, CHART_DATASET_UNKNOWN, CHART_AXIS_NOT_SELECTED, } from './validate-chart-bindings.js'; +import { explainRule } from './rule-explanations.js'; + +// [#22161] Each finding of the two converted ids 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 those ids to one line of at most 200 characters — every position +// and surface this suite exercises, not a chosen few. Run the whole file: those +// cases read what the cases above fired. +const fired: Array<{ rule: string; message: string; path: string }> = []; +const validateChartBindings: typeof validateChartBindingsUnrecorded = (...args) => { + const findings = validateChartBindingsUnrecorded(...args); + fired.push(...findings); + return findings; +}; /** A dataset whose measure names deliberately differ from the base fields. */ const baseStack = () => ({ @@ -1040,3 +1054,110 @@ describe('validateChartBindings — floor', () => { expect(findings).toEqual([]); }); }); + +describe('[#22161] one-line verdicts — chart-measure-unknown, chart-axis-not-selected', () => { + const CONVERTED = [CHART_MEASURE_UNKNOWN, CHART_AXIS_NOT_SELECTED]; + + it('every verdict the cases above fired is one line of at most 200 characters', () => { + const converted = fired.filter((f) => CONVERTED.includes(f.rule)); + // The coverage control first: both ids fired, the unknown-measure id at a + // query position and at all three presentation positions, so the shape + // assertion below cannot pass over a partial record. + expect([...new Set(converted.map((f) => f.rule))].sort()).toEqual([...CONVERTED].sort()); + const unknown = converted.filter((f) => f.rule === CHART_MEASURE_UNKNOWN); + for (const marker of ['comes back empty', 'DISPLAY-NAME override', 'REPLACES the authored array', 'axis PRESENTATION']) { + expect(unknown.some((f) => f.message.includes(marker)), marker).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('reads as one sentence per position', () => { + const [query] = validateChartBindings({ + ...baseStack(), + reports: [{ name: 'r', dataset: 'task_metrics', values: ['task_count'], chart: { type: 'bar', xAxis: 'status', yAxis: 'estimate_hours' } }], + }); + expect(query!.message).toBe( + '"estimate_hours" is not a measure declared by dataset "task_metrics", so this series comes back ' + + 'empty (result rows are keyed by measure name, not the base field)', + ); + const [series] = validateChartBindings({ + ...baseStack(), + reports: [ + { + name: 'r', + dataset: 'task_metrics', + values: ['task_count'], + chart: { type: 'bar', xAxis: 'status', yAxis: 'task_count', series: [{ name: 'est_hours' }] }, + }, + ], + }); + expect(series!.message).toBe( + '"est_hours" is a declared measure of "task_metrics" outside this chart\'s selected values ' + + '(task_count): this display-name override pairs only with the series the chart derives, so it ' + + 'lands on nothing', + ); + }); + + it('quotes at most three names of the selection, then counts the rest', () => { + const stack = { + datasets: [ + { + name: 'm', + object: 'o', + dimensions: [{ name: 'd', field: 'd' }], + measures: ['a', 'b', 'c', 'e', 'f', 'z'].map((name) => ({ name, aggregate: 'count' })), + }, + ], + pages: [ + { + name: 'p', + regions: [ + { + name: 'main', + components: [ + { type: 'object-chart', properties: { dataset: 'm', dimensions: ['d'], values: ['f', 'e', 'c', 'b', 'a'], series: [{ name: 'z' }] } }, + ], + }, + ], + }, + ], + }; + const [finding] = validateChartBindings(stack); + expect(finding).toMatchObject({ rule: CHART_AXIS_NOT_SELECTED }); + expect(finding!.message).toContain('selected values (a, b, c and 2 more)'); + }); + + it('`os explain` carries what the verdicts no longer say', () => { + const facts: Record = { + [CHART_MEASURE_UNKNOWN]: [ + 'ADR-0021', + '`sum_amount`', + '`chart.xAxis` × `chart.yAxis`', + 'per-measure display-name override', + 'replaced wholesale', + 'turns on a secondary axis', + '`ObjectChart`', + '`joined` report', + ], + [CHART_AXIS_NOT_SELECTED]: [ + 'PRESENTATION position', + 'widened at runtime', + 'A query position is never reported', + '`report.values` is the selection of the table beneath it', + 'a `chart.series[].name` override lands only when it names `chart.yAxis`', + ], + }; + 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); + } + // The surface paragraph is one text, printed under both ids it explains. + const last = (rule: string) => explainRule(rule)!.paragraphs.at(-1); + expect(last(CHART_AXIS_NOT_SELECTED)).toBe(last(CHART_MEASURE_UNKNOWN)); + }); +}); diff --git a/packages/lint/src/validate-chart-bindings.ts b/packages/lint/src/validate-chart-bindings.ts index c1b134f3e1c..2c26d0f1116 100644 --- a/packages/lint/src/validate-chart-bindings.ts +++ b/packages/lint/src/validate-chart-bindings.ts @@ -364,12 +364,12 @@ const UNSELECTED_CONSEQUENCE: Record = { /** * [#22161] A chart's selection for a one-line verdict: sorted, at most three, - * then `(and N more)`. + * then `and N more` (the verdict already quotes it in parentheses). */ function selectionRoster(names: Iterable): string { const all = [...names].sort(); const shown = all.slice(0, 3).join(', '); - return all.length > 3 ? `${shown} (and ${all.length - 3} more)` : shown; + return all.length > 3 ? `${shown} and ${all.length - 3} more` : shown; } export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] { From 0ed57745b0babdc38e3df400bb880dcf020e9a63 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 21:59:28 +0000 Subject: [PATCH 3/5] fix(lint): tighten the nav-view and report-series verdicts under the 200 bound Claude-Session: https://claude.ai/code/session_01S3aAf11JjbW1mSGL1EhfFj Co-authored-by: Claude --- packages/lint/src/lint-view-refs.test.ts | 10 +++++----- packages/lint/src/lint-view-refs.ts | 5 ++--- packages/lint/src/rule-explanations.ts | 4 ++-- packages/lint/src/validate-chart-bindings.test.ts | 3 +-- packages/lint/src/validate-chart-bindings.ts | 3 +-- 5 files changed, 11 insertions(+), 14 deletions(-) diff --git a/packages/lint/src/lint-view-refs.test.ts b/packages/lint/src/lint-view-refs.test.ts index b232ae67839..8b54bce6b2a 100644 --- a/packages/lint/src/lint-view-refs.test.ts +++ b/packages/lint/src/lint-view-refs.test.ts @@ -442,14 +442,14 @@ describe('[#22161] one-line verdicts — view-ref-nav-view-missing, view-key-col navStack({ id: 'nav_schedule', type: 'object', objectName: 'duly_task', viewName: 'A4_no_such_view', label: 'S' }), ); expect(miss!.message).toBe( - "Navigation entry opens view 'A4_no_such_view' on object 'duly_task', which declares no such list " + - 'view, so it opens the default view under its own label. List views: board, default, schedule', + "Navigation entry opens view 'A4_no_such_view' on object 'duly_task', which has no such list view, " + + 'so it silently opens the default view. List views: board, default, schedule', ); const [form] = navFindings(navStack({ id: 'n', type: 'object', objectName: 'duly_task', viewName: 'edit', label: 'E' })); expect(form!.message).toBe( - "Navigation entry opens view 'edit' on object 'duly_task', which declares no such list view (the " + - 'name resolves to a FORM view), so it opens the default view under its own label. List views: ' + - 'board, default, schedule', + "Navigation entry opens view 'edit' on object 'duly_task', which has no such list view (the name " + + 'resolves to a FORM view), so it silently opens the default view. List views: board, default, ' + + 'schedule', ); const [collision] = lintViewRefs({ views: [{ name: 'task', list: listView('task'), formViews: { default: formView('task') } }], diff --git a/packages/lint/src/lint-view-refs.ts b/packages/lint/src/lint-view-refs.ts index ed505922f05..483ffe09850 100644 --- a/packages/lint/src/lint-view-refs.ts +++ b/packages/lint/src/lint-view-refs.ts @@ -340,10 +340,9 @@ export function lintViewRefs(stack: AnyRec): ViewRefFinding[] { // [#22161] One verdict; how the name is matched and why the fallback is // silent are `os explain` text. The list views stay: they are the fix. message: - `Navigation entry opens view '${viewName}' on object '${objectName}', which declares no such ` + - `list view` + + `Navigation entry opens view '${viewName}' on object '${objectName}', which has no such list view` + (isFormView ? ` (the name resolves to a FORM view)` : '') + - `, so it opens the default view under its own label. List views: ${viewRoster(available)}`, + `, so it silently opens the default view. List views: ${viewRoster(available)}`, hint: `Correct the name, declare '${viewName}' in the object's \`listViews\`, or drop \`viewName\` ` + `to open the default view.` + diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index 92bc8bbf661..e366db5ca98 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -2047,8 +2047,8 @@ const APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY_EXPLANATION: RuleExplanation = { 'of any other type on the node is a non-group route, so a mixed slate is not judged.', 'The `manager` arm is the same dead end with a cause the product cannot repair. ' + '`{ type: \'manager\' }` resolves from `sys_user.manager_id` of the user the record names ' + - '(the field `value` names, else the owner), and returns nobody where that column is unset. A ' + - 'static check cannot read that column, so this does not assert the slate IS empty; it reports ' + + '(the field `value` names, else the owner), and returns nobody where that column is unset; a ' + + 'static check cannot read that column, so this does not assert the slate IS empty — it reports ' + 'that nothing else on the node can approve if it is. The arm fires only when the whole slate ' + 'is `manager` rungs, so no node draws both arms, and a slate mixing groups and `manager` is ' + 'silent.', diff --git a/packages/lint/src/validate-chart-bindings.test.ts b/packages/lint/src/validate-chart-bindings.test.ts index a368dacf949..4ca39c9f831 100644 --- a/packages/lint/src/validate-chart-bindings.test.ts +++ b/packages/lint/src/validate-chart-bindings.test.ts @@ -1096,8 +1096,7 @@ describe('[#22161] one-line verdicts — chart-measure-unknown, chart-axis-not-s }); expect(series!.message).toBe( '"est_hours" is a declared measure of "task_metrics" outside this chart\'s selected values ' + - '(task_count): this display-name override pairs only with the series the chart derives, so it ' + - 'lands on nothing', + '(task_count): this display-name override pairs only with `chart.yAxis`, so it lands on nothing', ); }); diff --git a/packages/lint/src/validate-chart-bindings.ts b/packages/lint/src/validate-chart-bindings.ts index 2c26d0f1116..c712f13ce11 100644 --- a/packages/lint/src/validate-chart-bindings.ts +++ b/packages/lint/src/validate-chart-bindings.ts @@ -356,8 +356,7 @@ const SHAPE_HINT: Record, string> = { */ const UNSELECTED_CONSEQUENCE: Record = { query: 'the query does not return it, so the series plots nothing', - 'report-series': - 'this display-name override pairs only with the series the chart derives, so it lands on nothing', + 'report-series': 'this display-name override pairs only with `chart.yAxis`, so it lands on nothing', 'page-series': 'the series are derived from `values`, so this entry lands on nothing', 'page-axis': 'the plotted columns come from `values`, so the axis entry re-points nothing', }; From 4077426704367f5391c4f1258a725d86ebfa280f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 22:43:42 +0000 Subject: [PATCH 4/5] docs(changeset): the measured verdict bounds per suite Claude-Session: https://claude.ai/code/session_01S3aAf11JjbW1mSGL1EhfFj Co-authored-by: Claude --- .changeset/22161-lint-slice-7-one-line.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/22161-lint-slice-7-one-line.md b/.changeset/22161-lint-slice-7-one-line.md index e74f39c827e..2651381a6e9 100644 --- a/.changeset/22161-lint-slice-7-one-line.md +++ b/.changeset/22161-lint-slice-7-one-line.md @@ -6,7 +6,7 @@ fix(lint): the approval-approver, data-model, AI agent-authoring, view-reference Clause-②: no -- **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 200 characters; before, the longest of each ran from 199 to 478 characters. The ids: +- **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`; From 4bd706494beae8a54e0f0874aafffba2b9a12680 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 23:13:19 +0000 Subject: [PATCH 5/5] =?UTF-8?q?docs(changeset):=20slice=207=20is=20minor,?= =?UTF-8?q?=20Clause-=E2=91=A1=20yes=20(widening):=20three=20rule=20id=20c?= =?UTF-8?q?onstants=20join=20the=20barrel?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_01S3aAf11JjbW1mSGL1EhfFj Co-authored-by: Claude --- .changeset/22161-lint-slice-7-one-line.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/22161-lint-slice-7-one-line.md b/.changeset/22161-lint-slice-7-one-line.md index 2651381a6e9..7d3962556bd 100644 --- a/.changeset/22161-lint-slice-7-one-line.md +++ b/.changeset/22161-lint-slice-7-one-line.md @@ -1,10 +1,10 @@ --- -"@objectstack/lint": patch +"@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 ` carries their reasoning -Clause-②: no +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`;