diff --git a/.changeset/spec-refusal-message-issue-ids.md b/.changeset/spec-refusal-message-issue-ids.md new file mode 100644 index 0000000000..3d4159315e --- /dev/null +++ b/.changeset/spec-refusal-message-issue-ids.md @@ -0,0 +1,39 @@ +--- +"@objectstack/spec": patch +--- + +Strip internal tracker ids from the refusal messages an author actually reads (#12124) + +Fifteen zod refusal messages across nine `packages/spec/src` files ended a sentence with +an internal issue id. Those strings are printed **at** the author, verbatim, the moment +their metadata is refused — by `os validate`, by a publish gate, by a parse — and the +reader has no tracker to open. A `#NNNN` there is a citation-shaped token that resolves to +nothing, in the one place the sentence most needs to be actionable. + +```text +before: A field condition's keys are field names, never $-prefixed operators (#7711). +after: A field condition's keys are field names, never $-prefixed operators. + +before: ... refused at authoring time because the query path refuses it too + (400 INVALID_FILTER, #5869). +after: ... refused at authoring time because the query path refuses it too + (400 INVALID_FILTER). +``` + +Where a customer-resolvable anchor already carried the meaning it was kept and the id +dropped beside it: the second example above keeps `400 INVALID_FILTER`, which is the token +an author can actually match their query-path error against. Where the reference is +load-bearing for an *internal* reader only, it moved to an adjacent `//` comment (four +sites: the endpoint publish gate's two `#5040` section pointers, the summary-field rule's +founding incident, and the interim renderer precedence behind the doubled-redirect +refusal). Elsewhere it is simply gone — git history keeps the anchor. + +Text only. **No accept/reject behaviour changes**: the same inputs are refused on the same +schemas with the same issue `code`, `path` and error shape; only the sentence changes. +Test twins that pinned the old wording now pin the new text plus a negative assertion that +the message carries no issue id at all. + +The convention is held mechanically from here — `check:doc-authoring` gained a third rule +that parses `packages/spec/src` and reds on an id in any refusal-message string. It parses +rather than scanning lines because refusal prose here is written as multi-line string +concatenation: a single-line `message:.*#[0-9]{3,5}` grep sees 1 of the 16 literals. diff --git a/packages/spec/src/ai/skill-trigger-condition-value-shape.test.ts b/packages/spec/src/ai/skill-trigger-condition-value-shape.test.ts index 48542d4092..9c8a88f8c4 100644 --- a/packages/spec/src/ai/skill-trigger-condition-value-shape.test.ts +++ b/packages/spec/src/ai/skill-trigger-condition-value-shape.test.ts @@ -65,7 +65,10 @@ describe('#7113 — the reported shape is refused at authoring time', () => { it('names the consumer-side coercion as the thing being replaced', () => { const issue = valueIssue(parse({ field: 'userRole', operator: 'not_in', value: 'admin' })); expect(issue.message).toContain('coerces the scalar today'); - expect(issue.message).toContain('#7113'); + // The claim is carried by the SENTENCE, not by a tracker id: this string is + // printed at an author who has no tracker to open. + expect(issue.message).toContain('the contract never declared that spelling'); + expect(issue.message).not.toMatch(/(? z.record(z.string(), FieldOperatorsSchema).refine( (condition) => !Object.keys(condition).some((key) => key.startsWith('$')), { - message: 'A field condition\'s keys are field names, never $-prefixed operators (#7711).', + message: 'A field condition\'s keys are field names, never $-prefixed operators.', // `abort` so this branch cannot become the union's spokesman. Measured on // zod 4.4.3: a union whose other options all abort returns a lone // CONTINUABLE failure verbatim, which made `{ $not: { c: } }` — a diff --git a/packages/spec/src/integration/connector.zod.ts b/packages/spec/src/integration/connector.zod.ts index 2ab4536817..d1593d5089 100644 --- a/packages/spec/src/integration/connector.zod.ts +++ b/packages/spec/src/integration/connector.zod.ts @@ -935,7 +935,7 @@ export const DeclarativeConnectorEntrySchema = lazySchema(() => path: ['authentication'], message: isInstance ? `Provider-bound connector instance '${entry.name}' must not inline secrets via \`authentication\`; reference credentials with \`auth: { type, credentialRef }\` instead (ADR-0097 §3).` - : `Connector '${entry.name}' must not inline secrets via \`authentication\` — a published connector row is stored whole in \`sys_metadata\`, so the credential would land in cleartext (#7990). A catalog descriptor holds no live credentials: drop \`authentication\` (or set \`{ type: 'none' }\`) and describe the auth scheme in \`description\`. A dispatchable instance declares \`provider\` and references its credential with \`auth: { type, credentialRef }\` (ADR-0097 §3).`, + : `Connector '${entry.name}' must not inline secrets via \`authentication\` — a published connector row is stored whole in \`sys_metadata\`, so the credential would land in cleartext. A catalog descriptor holds no live credentials: drop \`authentication\` (or set \`{ type: 'none' }\`) and describe the auth scheme in \`description\`. A dispatchable instance declares \`provider\` and references its credential with \`auth: { type, credentialRef }\` (ADR-0097 §3).`, }); } if (!isInstance) { diff --git a/packages/spec/src/kernel/functional-completeness.ts b/packages/spec/src/kernel/functional-completeness.ts index cc2d7abb52..623c24506a 100644 --- a/packages/spec/src/kernel/functional-completeness.ts +++ b/packages/spec/src/kernel/functional-completeness.ts @@ -121,6 +121,9 @@ export function checkFieldCompleteness(def: unknown): CompletenessFinding[] { const out: CompletenessFinding[] = []; if (type === 'summary' && !isRec(def.summaryOperations)) { + // Internal anchor: the founding incident for this rule is cloud#687. It sits + // here rather than in the message — the message is printed to a customer who + // has neither repo's tracker; ADR-0078 is the reference that travels. out.push({ rule: FIELD_SUMMARY_WITHOUT_OPERATIONS, severity: 'error', @@ -129,7 +132,7 @@ export function checkFieldCompleteness(def: unknown): CompletenessFinding[] { 'A `summary` field with no `summaryOperations` computes nothing: the engine\'s ' + 'summary index skips it (`engine.ts` — `if (!d.summaryOperations) continue`), so it ' + 'reads 0/null everywhere and anything derived from it is stuck at 0 — while every ' - + 'authoring surface reports success. This is the cloud#687 shape ADR-0078 was written for.', + + 'authoring surface reports success. This is the shape ADR-0078 was written for.', fix: "summaryOperations: { object: '', field: '', function: 'sum' }", }); } @@ -278,7 +281,7 @@ export function checkWebhookCompleteness(webhook: unknown): CompletenessFinding[ message: 'A webhook with no `triggers` never fires on any path. The auto-enqueuer drops it while ' + 'building its subscription cache (`auto-enqueuer.ts` — `if (triggers.size === 0) … return ' - + 'null`), and there is no manual fire path to reach it either: `webhook.zod.ts` (#3196) ' + + 'null`), and there is no manual fire path to reach it either: `webhook.zod.ts` ' + 'records that the `api` trigger was removed because "no manual fire path exists — the only ' + 'webhook HTTP surface re-queues already-failed deliveries". The webhook materializes into ' + '`sys_webhook`, looks armed in Setup, and delivers nothing. To disable a webhook use ' diff --git a/packages/spec/src/system/auth-config.zod.ts b/packages/spec/src/system/auth-config.zod.ts index 6078a225fc..981e2ff968 100644 --- a/packages/spec/src/system/auth-config.zod.ts +++ b/packages/spec/src/system/auth-config.zod.ts @@ -417,7 +417,7 @@ export const AudienceConfigSchema = lazySchema(() => z.object({ message: `posture '${posture}' permits self-registration, so the permission set a self-registrant receives ` + 'must be DECLARED (selfRegistrationPermissionSet) — the implicit member_default fallback is retired ' + - '(#11739; declaring member_default explicitly is allowed).', + '(declaring member_default explicitly is allowed).', }); } else if (value.selfRegistrationPermissionSet === 'admin_full_access') { ctx.addIssue({ diff --git a/packages/spec/src/ui/action-doubled-redirect.test.ts b/packages/spec/src/ui/action-doubled-redirect.test.ts index 7514f73f4f..c3ba709843 100644 --- a/packages/spec/src/ui/action-doubled-redirect.test.ts +++ b/packages/spec/src/ui/action-doubled-redirect.test.ts @@ -47,9 +47,14 @@ describe('ActionSchema — doubled post-success navigation (#11519)', () => { // The remedy: one destination, declared in one place. expect(msg).toMatch(/drop|remove|keep/i); // The interim renderer precedence this refusal supersedes at authoring - // time (declared wins, objectui#5933) is recorded so an author hitting - // the error understands what happens to metadata published before it. - expect(msg).toContain('objectui#5933'); + // time is recorded so an author hitting the error understands what + // happens to metadata published before it. Pinned as the SUBSTANCE — + // which channel wins and which is dropped — rather than as the tracker id + // that used to stand in for it: the id resolved to nothing for the author + // this message is printed at, while the sentence tells them the outcome. + expect(msg).toContain('interim precedence'); + expect(msg).toContain('silently ignored'); + expect(msg).not.toMatch(/(? { diff --git a/packages/spec/src/ui/action.zod.ts b/packages/spec/src/ui/action.zod.ts index fea685cefd..73fa164ff5 100644 --- a/packages/spec/src/ui/action.zod.ts +++ b/packages/spec/src/ui/action.zod.ts @@ -1509,8 +1509,8 @@ export const ActionSchema = lazySchema(() => actionObject().refine((data) => { 'An action that declares `confirmText` beside a non-empty `params` shows the user TWO dialogs ' + 'for one decision — the confirm, then the param prompt, with nothing sent until the second. ' + "Carry the confirm question in the action's top-level `description` instead (it renders under " - + 'the param dialog\'s title) and drop `confirmText`: one condition, one wording, one dialog ' - + '(#7278). Not `ai.description` — that is the LLM-facing tool contract. `confirmText` stays ' + + 'the param dialog\'s title) and drop `confirmText`: one condition, one wording, one dialog. ' + + 'Not `ai.description` — that is the LLM-facing tool contract. `confirmText` stays ' + 'correct for a param-LESS action, where the confirm IS the only dialog, and for a view\'s ' + '`bulkActionDefs`, where the pair renders one dialog by that schema\'s own contract.', path: ['confirmText'], @@ -1555,6 +1555,10 @@ export const ActionSchema = lazySchema(() => actionObject().refine((data) => { // Scoped to `type: 'script'` — the ruled sentence. `opensInNewTab: false` // is not the marker (it declares the channel is NOT in use). The corpus was // measured at zero doubled producers (#11519), so nothing legal breaks. + // + // Internal anchor for the "interim precedence" the message names: objectui#5933 + // is where the renderer-side precedence was ruled. It lives here rather than in + // the message, which is printed to a customer who has neither repo's tracker. if (data.type === 'script' && data.onSuccess && data.opensInNewTab === true) { return false; } @@ -1564,9 +1568,9 @@ export const ActionSchema = lazySchema(() => actionObject().refine((data) => { "A `type: 'script'` action declaring BOTH `onSuccess` and `opensInNewTab: true` carries two " + 'post-success destinations for one success: `opensInNewTab` pre-opens a tab for the ' + 'HANDLER-RETURNED `{ redirectUrl }`, while `onSuccess.navigate` declares the hop in ' - + 'metadata. A renderer can perform only one — under the interim precedence (objectui#5933) ' + + 'metadata. A renderer can perform only one — under the interim precedence ' + "the declared `onSuccess` wins and the handler's `redirectUrl` is silently ignored — so the " - + 'doubled declaration is refused at authoring time (#11519). Keep `onSuccess` and drop ' + + 'doubled declaration is refused at authoring time. Keep `onSuccess` and drop ' + '`opensInNewTab` (and stop returning `redirectUrl` from the handler), or keep ' + '`opensInNewTab` + the handler redirect and drop `onSuccess`. There is no `precedence` ' + 'field, by ruling: one destination, declared in one place.', @@ -1602,7 +1606,7 @@ export const ActionSchema = lazySchema(() => actionObject().refine((data) => { + 'otherwise, so without the flag it would parse clean and never run (ADR-0078). If a ' + 'pre-opened tab is intended, add `opensInNewTab: true`; otherwise drop `newTabUrl` ' + '(behavior is unchanged — the lone key was never read). For a STATIC url action, new-tab ' - + 'behavior is `openIn: "new-tab"`, not this pair (#11842).', + + 'behavior is `openIn: "new-tab"`, not this pair.', path: ['newTabUrl'], }).transform((data, ctx) => lowerRequiresFeature(data, ctx))); diff --git a/packages/spec/src/ui/i18n.test.ts b/packages/spec/src/ui/i18n.test.ts index 8748c25cf1..8ad2835fca 100644 --- a/packages/spec/src/ui/i18n.test.ts +++ b/packages/spec/src/ui/i18n.test.ts @@ -102,7 +102,10 @@ describe('I18nLabelSchema', () => { const issues = JSON.stringify(r.error?.issues); expect(issues).toContain('invalid_key'); expect(issues).toContain('never by `key`/`defaultValue`'); - expect(issues).toContain('#5055'); + // The retired form is named in words. It used to be named by a tracker id + // as well, which resolved to nothing for the author reading the refusal. + expect(issues).toContain('the retired key-reference form'); + expect(issues).not.toMatch(/(? { diff --git a/packages/spec/src/ui/i18n.zod.ts b/packages/spec/src/ui/i18n.zod.ts index ffac1951d5..66cd1a7dbe 100644 --- a/packages/spec/src/ui/i18n.zod.ts +++ b/packages/spec/src/ui/i18n.zod.ts @@ -195,7 +195,7 @@ export const InlineLocaleMapSchema: z.ZodType< z.string().regex( INLINE_LOCALE_KEY, 'an inline label map is keyed by BCP-47 locale tags (`en`, `zh-CN`, …) or `default` — ' - + 'never by `key`/`defaultValue`, the retired key-reference form (#5055): nothing looks the key up, ' + + 'never by `key`/`defaultValue`, the retired key-reference form: nothing looks the key up, ' + 'so both resolvers fall through to the first string value and the raw key is rendered on screen', ), z.string(), diff --git a/packages/spec/src/ui/view-filter-rule-value-shape.test.ts b/packages/spec/src/ui/view-filter-rule-value-shape.test.ts index 71b768cb71..c8dd7a4b5b 100644 --- a/packages/spec/src/ui/view-filter-rule-value-shape.test.ts +++ b/packages/spec/src/ui/view-filter-rule-value-shape.test.ts @@ -60,7 +60,16 @@ describe('#6227 — the reported shape is refused at authoring time', () => { it('names the runtime twin, so the two moments are traceable to one rule', () => { const issue = valueIssue(parse({ field: 'stage', operator: 'in', value: 'won' })); - expect(issue.message).toContain('400 INVALID_FILTER, #5869'); + // The traceable token is the runtime's ERROR CODE, not a tracker id — the + // code is what an author sees on the query path and can match this refusal + // against. The id that used to ride beside it resolved to nothing for the + // customer this string is printed to. + expect(issue.message).toContain('400 INVALID_FILTER'); + }); + + it('carries no internal tracker id — the reader of this string cannot open one', () => { + const issue = valueIssue(parse({ field: 'stage', operator: 'in', value: 'won' })); + expect(issue.message).not.toMatch(/(? { diff --git a/packages/spec/src/ui/view.zod.ts b/packages/spec/src/ui/view.zod.ts index 8cc3e9b859..3097068e8d 100644 --- a/packages/spec/src/ui/view.zod.ts +++ b/packages/spec/src/ui/view.zod.ts @@ -527,7 +527,7 @@ function checkViewFilterRuleValueShape( + `${value === undefined ? '["…"]' : previewFilterValue([value])} for a single value, ` + `or use ${operator === 'in' ? '"equals"' : '"not_equals"'} to compare against it. ` + `An empty list [] is allowed and is a real predicate. This is refused at authoring ` - + `time because the query path refuses it too (400 INVALID_FILTER, #5869).`, + + `time because the query path refuses it too (400 INVALID_FILTER).`, }); return; } @@ -541,7 +541,7 @@ function checkViewFilterRuleValueShape( `Operator "${operator}" on field "${field}" requires a [min, max] value array. ` + `Received ${describeFilterValue(value)} (${previewFilterValue(value)}). ` + `A range needs exactly two bounds, in order. This is refused at authoring time ` - + `because the query path refuses it too (400 INVALID_FILTER, #5869).`, + + `because the query path refuses it too (400 INVALID_FILTER).`, }); } diff --git a/scripts/check-doc-authoring.mjs b/scripts/check-doc-authoring.mjs index f1512e1c66..aab5c8a71f 100644 --- a/scripts/check-doc-authoring.mjs +++ b/scripts/check-doc-authoring.mjs @@ -88,7 +88,14 @@ import { mkdirSync, mkdtempSync, readdirSync, readFileSync, renameSync, rmSync, statSync, writeFileSync, } from 'node:fs'; import { tmpdir } from 'node:os'; +import { createRequire } from 'node:module'; import { dirname, join, sep } from 'node:path'; +import { parseSourceFile } from './ts-parse.mjs'; + +// `typescript` is resolved rather than imported at module top so this gate's +// two Markdown rules keep working in a checkout where it is absent; Rule 3 asks +// for it at the moment it scans, and says so by name if it cannot be had. +const requireFromHere = createRequire(import.meta.url); const ROOTS = ['.claude', 'docs', 'skills', 'content']; const SKIP_DIRS = new Set(['node_modules', '.git', 'dist', 'references']); @@ -325,6 +332,77 @@ const INTERNAL_ID_SOURCE = String.raw`(? 0": + * an open rule would flag `.default('draft')`, `.catch('')` and every other + * VALUE argument, and a gate that reports values as prose is one authors route + * around. Reaching for a spelling that is not here? Add it, and add a self-test + * case in the same edit — an unrecognised spelling produces no flag, silently. + */ +const POSITIONAL_MESSAGE_CALLS = new Set([ + 'refine', 'superRefine', 'check', 'regex', 'min', 'max', 'length', + 'startsWith', 'endsWith', 'includes', 'email', 'url', 'uuid', 'int', + 'positive', 'nonnegative', 'multipleOf', 'nonempty', 'gt', 'gte', 'lt', 'lte', +]); + const posix = (p) => p.split(sep).join('/'); function walk(dir, out) { @@ -472,6 +550,116 @@ function findIdViolations(source, file) { return out; } +/** + * Every non-test TypeScript source in the spec package. + * + * Test bodies are excluded on purpose and it is the one exclusion here: a test + * asserting "this refusal names #5869" is read only by someone who has the + * tracker, and the pin has to be able to quote whatever the message says. The + * ban follows the audience, which is the same sentence Rule 2 is scoped by. + * + * @throws {DeadRootError} the root is not a directory. + * @throws {EmptyRootError} the root yielded no source file — "the spec is + * clean" and "the spec was never opened" are otherwise the same output and + * the same exit code (#4932, one population over). + */ +function collectSpecSourceFiles(root = SPEC_SOURCE_ROOT) { + assertRootsResolvable([root]); + const files = []; + (function descend(dir) { + for (const e of readdirSync(dir)) { + if (e === 'node_modules' || e === '.git' || e === 'dist') continue; + const p = join(dir, e); + if (statSync(p).isDirectory()) descend(p); + else if (/\.m?ts$/.test(e) && !/\.(test|spec|bench)\.m?ts$/.test(e)) files.push(posix(p)); + } + })(root); + if (files.length === 0) throw new EmptyRootError([root], 0); + return files.sort(); +} + +/** + * Does this string literal sit in a refusal-message position? + * + * Climbs OUT through `+` concatenation, parentheses, conditionals and template + * spans before asking — the whole reason this rule is an AST walk. Returns the + * position's name (for the failure text) or `undefined`. + */ +function messagePosition(node, ts) { + let cur = node; + // A bound, not a belief: refusal prose in this tree reaches ~14 concatenated + // operands, and an unbounded climb would walk to the SourceFile and start + // reporting whole modules as messages. + for (let hops = 0; cur.parent && hops < 60; hops++) { + const p = cur.parent; + if ( + (ts.isBinaryExpression(p) && p.operatorToken.kind === ts.SyntaxKind.PlusToken) + || ts.isParenthesizedExpression(p) + || ts.isConditionalExpression(p) + || ts.isTemplateSpan(p) + || ts.isTemplateExpression(p) + || ts.isAsExpression(p) + || ts.isSatisfiesExpression(p) + ) { cur = p; continue; } + + if (ts.isPropertyAssignment(p) && p.initializer === cur) { + return p.name.getText() === 'message' ? 'message:' : undefined; + } + if (ts.isCallExpression(p)) { + const callee = p.expression; + const name = ts.isPropertyAccessExpression(callee) ? callee.name.getText() : callee.getText(); + return POSITIONAL_MESSAGE_CALLS.has(name) && p.arguments.indexOf(cur) > 0 + ? `.${name}(…, message)` + : undefined; + } + if (ts.isVariableDeclaration(p) || ts.isReturnStatement(p) || ts.isArrowFunction(p)) return undefined; + cur = p; + } + return undefined; +} + +/** + * Refusal messages in one spec source, and how many message strings were seen + * at all. + * + * The second number is not decoration. This rule's population is expected to be + * EMPTY in the steady state, so "no violations" is the same output as "the + * detector no longer recognises how messages are spelled" — the failure this + * whole file is a monument to. `seen` is what {@link main} asserts against, so + * a detector that has gone blind reds instead of congratulating itself. + */ +function findMessageIdViolations(source, file, ts) { + const out = []; + let seen = 0; + const sf = parseSourceFile(file, source); + const visit = (node) => { + if ( + ts.isStringLiteral(node) + || ts.isNoSubstitutionTemplateLiteral(node) + || ts.isTemplateExpression(node) + ) { + const where = messagePosition(node, ts); + if (where) { + seen += 1; + const text = node.getText(sf); + const ids = text.match(INTERNAL_ID); + if (ids) { + out.push({ + file: posix(file), + line: sf.getLineAndCharacterOfPosition(node.getStart(sf)).line + 1, + ids, + where, + text: text.length > 120 ? `${text.slice(0, 120)}…` : text, + }); + } + } + } + ts.forEachChild(node, visit); + }; + ts.forEachChild(sf, visit); + return { violations: out, seen }; +} + /** Bare metadata literals inside ts/tsx fenced blocks of one file's source. */ function findViolations(source, file) { const out = []; @@ -777,6 +965,8 @@ function selfTest() { process.chdir(dir); rmSync(idDir, { recursive: true, force: true }); } + + selfTestRule3(expect); } finally { process.chdir(cwd); rmSync(dir, { recursive: true, force: true }); @@ -835,7 +1025,173 @@ function selfTest() { console.error(`\n✗ check-doc-authoring self-test failed:\n${failures.join('\n')}\n`); process.exit(1); } - console.log('✓ check-doc-authoring self-test: scope wiring (.claude and the live docs/ corpus in, .claude/worktrees and docs/{audits,handoff,plans} out), detection, the dead-root hard error (red when a ROOT is renamed, green when restored), the empty-scan hard error (red when a root yields nothing and when the whole scan does, green when restored), the published-catalog internal-id rule (red on a planted id in prose, in a fenced comment and in the repo#NNNN spelling, green when removed; hex colours, version numbers, HTTP codes, array indices and the "#1" ordinal all pass; references/ reached, generated artifacts and the internal roots out; the `#` placeholder passes while the concrete ids it replaced stay red, with no exemption to reach for) and the dispatch-gates declaration (every separator-less ROOT declared as a subtree, nothing declared this gate does not walk, the over-claim bounded to SKIP_PATHS) all hold.'); + console.log('✓ check-doc-authoring self-test: scope wiring (.claude and the live docs/ corpus in, .claude/worktrees and docs/{audits,handoff,plans} out), detection, the dead-root hard error (red when a ROOT is renamed, green when restored), the empty-scan hard error (red when a root yields nothing and when the whole scan does, green when restored), the published-catalog internal-id rule (red on a planted id in prose, in a fenced comment and in the repo#NNNN spelling, green when removed; hex colours, version numbers, HTTP codes, array indices and the "#1" ordinal all pass; references/ reached, generated artifacts and the internal roots out; the `#` placeholder passes while the concrete ids it replaced stay red, with no exemption to reach for), the spec refusal-message internal-id rule (red on an id planted on a LATER line of a concatenated message — the shape a line-oriented census cannot see, proven here — and in a template chain, a positional validator message and the repo#NNNN spelling; green when removed; a `.default()` VALUE and a `.describe()` do not fire; test bodies out, and a tree with no recognised message string reports seen=0 so a blinded detector reds instead of passing) and the dispatch-gates declaration (every separator-less ROOT declared as a subtree, nothing declared this gate does not walk, the over-claim bounded to SKIP_PATHS) all hold.'); +} + +/** + * Rule 3's red/green battery, over a real temporary `packages/spec/src` tree. + * + * Same discipline as Rule 2's: green on a clean tree proves nothing on its own, + * because a rule that CANNOT fire looks identical. Every claim below is a pair — + * plant the id, require red; remove it, require green — and the multi-line + * concatenation case is first because it is the one the commissioning card's + * own census command could not see. + */ +function selfTestRule3(expect) { + const ts = requireFromHere('typescript'); + const cwd = process.cwd(); + const dir = mkdtempSync(join(tmpdir(), 'doc-authoring-selftest-msg-')); + try { + const write = (rel, body) => { + const full = join(dir, ...rel.split('/')); + mkdirSync(dirname(full), { recursive: true }); + writeFileSync(full, body); + return full; + }; + const CLEAN = [ + "import { z } from 'zod';", + 'export const S = z.object({ a: z.string() }).refine((v) => !!v.a, {', + " message: 'a is required — declare it or drop the block.',", + '});', + ].join('\n'); + // A test body carrying an id: in the tree, out of the population. + write('packages/spec/src/ui/pin.test.ts', + "expect(issue.message).toContain('400 INVALID_FILTER, #5869');"); + // A TSDoc / `.describe()` id: a different population, deliberately untouched + // by this rule. If it ever fires here, the scope has silently widened. + write('packages/spec/src/data/doc.ts', [ + "import { z } from 'zod';", + '/** Removed in protocol 17 (#4286). */', + "export const D = z.string().describe('A machine name (#4286).');", + ].join('\n')); + const target = write('packages/spec/src/ui/action.zod.ts', CLEAN); + + const scan = () => { + let out = []; + let seen = 0; + for (const f of collectSpecSourceFiles()) { + const r = findMessageIdViolations(readFileSync(f, 'utf8'), f, ts); + out = out.concat(r.violations); + seen += r.seen; + } + return { violations: out, seen }; + }; + + process.chdir(dir); + + // GREEN, and the detector is demonstrably NOT blind: it saw the clean + // message. Reporting both numbers is the point — "0 violations" and "0 + // messages found" are the same line to a reader who only checks the first. + let r = scan(); + expect('a clean spec tree is green', r.violations.length, 0); + expect('the detector actually recognised a message string', r.seen >= 1, true); + + // Scope, both directions, before any red: a test body is out and a + // `.describe()` is out — each is satisfied by a wrong scope in the other + // direction if asserted alone. + const scanned = collectSpecSourceFiles(); + expect('test bodies are not scanned', scanned.includes('packages/spec/src/ui/pin.test.ts'), false); + expect('ordinary sources are scanned', scanned.includes('packages/spec/src/data/doc.ts'), true); + expect('a `.describe()` id is NOT this rule\'s population', + r.violations.some((v) => v.file === 'packages/spec/src/data/doc.ts'), false); + + // RED #1 — the founding shape: `message:` and the id on DIFFERENT lines of a + // `+` chain. This is what `git grep "message:.*#[0-9]{3,5}"` cannot see, and + // the whole reason this rule parses instead of scanning lines. + writeFileSync(target, [ + "import { z } from 'zod';", + 'export const S = z.object({ a: z.string() }).refine((v) => !!v.a, {', + ' message:', + " 'This pair declares two destinations for one success, so the doubled '", + " + 'declaration is refused at authoring time (#11519). Keep `onSuccess`.',", + '});', + ].join('\n')); + r = scan(); + expect('an id on a later line of a concatenated message is RED', r.violations.length, 1); + expect('the red names the file', r.violations[0]?.file, 'packages/spec/src/ui/action.zod.ts'); + expect('the red names the id', r.violations[0]?.ids?.join(','), '#11519'); + expect('the red names the position', r.violations[0]?.where, 'message:'); + // ...and the single-line grep the card shipped really cannot: proven here so + // the claim in this file's header is a measurement, not a recollection. + expect('the line-oriented census command misses it', + /message:.*#[0-9]{3,5}/.test(readFileSync(target, 'utf8')), false); + + // RED #2 — a template literal, the other half of the real population. + writeFileSync(target, [ + "import { z } from 'zod';", + 'export const S = z.object({ a: z.string() }).superRefine((v, ctx) => {', + ' ctx.addIssue({', + " code: 'custom',", + ' message:', + ' `Operator "${v.a}" needs an ARRAY. `', + ' + `The query path refuses it too (400 INVALID_FILTER, #5869).`,', + ' });', + '});', + ].join('\n')); + r = scan(); + expect('an id in a concatenated TEMPLATE message is RED', r.violations.length, 1); + expect('the template red names the id', r.violations[0]?.ids?.join(','), '#5869'); + + // RED #3 — the POSITIONAL spelling. One member of the founding population + // was written this way, so a `message:`-only matcher under-reports by + // exactly the shape it exists to catch. + writeFileSync(target, [ + "import { z } from 'zod';", + 'export const S = z.record(z.string().regex(', + ' /^[a-z]+$/,', + " 'keyed by BCP-47 locale tags — never by `key`, the retired form (#5055)',", + '), z.string());', + ].join('\n')); + r = scan(); + expect('an id in a positional validator message is RED', r.violations.length, 1); + expect('the positional red names the position', r.violations[0]?.where, '.regex(…, message)'); + + // RED #4 — the cross-repo spelling, which really occurs in this population + // (`objectui#5933`, `cloud#687`). + writeFileSync(target, [ + "import { z } from 'zod';", + 'export const S = z.object({ a: z.string() }).refine((v) => !!v.a, {', + " message: 'under the interim precedence (objectui#5933) the declared hop wins.',", + '});', + ].join('\n')); + r = scan(); + expect('the `repo#NNNN` spelling is RED here too', r.violations.length, 1); + + // Precision — a validator's VALUE argument is not prose. `.min(3, …)` takes + // a message; `.default('#4286')` does not, and an open "any string after + // position 0" rule would report it. + writeFileSync(target, [ + "import { z } from 'zod';", + "export const S = z.object({ a: z.string().default('#4286') });", + ].join('\n')); + expect('precision — a `.default()` VALUE is not a message', scan().violations.length, 0); + + // GREEN again from the same scan, so every red above was the id and nothing + // else about the tree. + writeFileSync(target, CLEAN); + r = scan(); + expect('stripping the id makes it green again', r.violations.length, 0); + expect('and the detector is still not blind', r.seen >= 1, true); + + // The blindness assertion itself must be able to fire: a tree whose only + // sources declare no message at all is `seen === 0`, which main() reds on. + writeFileSync(target, "export const S = 1;\n"); + write('packages/spec/src/data/doc.ts', "export const D = 2;\n"); + write('packages/spec/src/ui/pin.test.ts', "export const T = 3;\n"); + expect('a tree with no recognised message string reports seen=0 (main reds on it)', + scan().seen, 0); + + // Empty is a hard error, not a pass — same discipline as the other two rules. + rmSync(join(dir, 'packages', 'spec', 'src'), { recursive: true, force: true }); + mkdirSync(join(dir, 'packages', 'spec', 'src'), { recursive: true }); + let emptyErr = null; + try { collectSpecSourceFiles(); } catch (err) { emptyErr = err; } + expect('an empty spec source root is red, not "0 files clean"', + emptyErr instanceof EmptyRootError, true); + } finally { + process.chdir(cwd); + rmSync(dir, { recursive: true, force: true }); + } } function main() { @@ -894,6 +1250,27 @@ function main() { } const idViolations = published.flatMap((file) => findIdViolations(readFileSync(file, 'utf8'), file)); + const ts = requireFromHere('typescript'); + let specSources; + try { + specSources = collectSpecSourceFiles(); + } catch (err) { + console.error( + `\n✗ doc authoring guard: the spec source root (${SPEC_SOURCE_ROOT}/) could not be` + + `\nscanned for internal issue-id references in refusal messages, so this run cannot` + + `\nvouch for it:\n\n ${err.message}\n`, + ); + process.exit(1); + return; + } + const messageIdViolations = []; + let messageStringsSeen = 0; + for (const file of specSources) { + const r = findMessageIdViolations(readFileSync(file, 'utf8'), file, ts); + messageIdViolations.push(...r.violations); + messageStringsSeen += r.seen; + } + let failed = false; if (violations.length > 0) { @@ -933,10 +1310,54 @@ function main() { ); } + if (messageStringsSeen === 0) { + failed = true; + console.error( + `\n✗ doc authoring guard: ${specSources.length} spec source(s) were parsed and NOT ONE` + + `\nrefusal-message string was recognised, so "no violations" below would be a verdict on` + + `\na population this run never located.` + + `\n\nThat is the dormant-gate shape, not a clean tree: the spec really does declare` + + `\nrefusal prose, so a zero here means the DETECTOR stopped matching how it is spelled —` + + `\nan options-object key renamed away from \`message\`, a new validator helper, a wrapper` + + `\nthat builds the string somewhere \`messagePosition()\` does not climb to.` + + `\n\nFix \`messagePosition()\` / POSITIONAL_MESSAGE_CALLS in scripts/check-doc-authoring.mjs` + + `\nand add the new spelling to --self-test in the same edit. Do NOT delete this assertion:` + + `\nit is the only thing standing between this rule and a permanent green.\n`, + ); + } + + if (messageIdViolations.length > 0) { + failed = true; + console.error(`\n✗ Internal issue-id reference(s) in CUSTOMER-FACING spec refusal messages:\n`); + for (const v of messageIdViolations) { + console.error(` ${v.file}:${v.line} ${v.ids.join(' ')} [${v.where}]`); + console.error(` ${v.text}`); + } + console.error( + `\n${messageIdViolations.length} message string(s). These are printed AT the customer, verbatim,` + + `\nthe moment their metadata is refused — by \`os validate\`, by a publish gate, by a parse.` + + `\nThat reader has no tracker, no \`git log\` and no ADRs, so \`#NNNN\` is a citation-shaped` + + `\ntoken resolving to nothing in the one place they most need the sentence to be actionable.` + + `\n\nStrip the id from the string. Where the reference is genuinely load-bearing for an` + + `\nINTERNAL reader, move it to an adjacent \`//\` comment; otherwise just remove it — git` + + `\nhistory keeps the anchor. Prefer a customer-resolvable anchor where one exists: an ADR` + + `\nnumber, a protocol version, an error code (\`400 INVALID_FILTER\` traces the runtime twin` + + `\nfar better than the id that used to ride beside it).` + + `\n\nA test twin pinning the old wording moves WITH the string — keep it pinning the new` + + `\ntext, and add the negative pin (the message must not match an issue id).` + + `\n\nThere is no per-message exemption to reach for, by design.` + + `\n\nMaintainer ruling 2026-08-12, verbatim: 「处理 issue 时犯的错应该总结成经验,保留 issue id没有意义」\n`, + ); + } + if (failed) process.exit(1); console.log(`✓ doc authoring guard: ${files.length} files clean — no bare metadata literals.`); console.log(`✓ doc authoring guard: ${published.length} published skill files clean — no internal issue-id references.`); + console.log( + `✓ doc authoring guard: ${messageStringsSeen} refusal-message string(s) across ` + + `${specSources.length} spec sources clean — no internal issue-id references.`, + ); } main(); diff --git a/scripts/pm/dispatch-gates.mjs b/scripts/pm/dispatch-gates.mjs index cdd0d0b8d0..a3b3b8052c 100644 --- a/scripts/pm/dispatch-gates.mjs +++ b/scripts/pm/dispatch-gates.mjs @@ -5527,10 +5527,18 @@ function selfTest() { t('and the agent operating manual it took in for the same reason', docAuthoringHints.some((h) => hintCovers(h, '.claude/agents/os-dev.md'))); t('and the published skills catalog', docAuthoringHints.some((h) => hintCovers(h, 'skills/objectstack-upgrade/SKILL.md'))); t('and the content tree', docAuthoringHints.some((h) => hintCovers(h, 'content/docs/deployment/cli.mdx'))); - // The negative half, load-bearing for a declaration spanning four roots: a - // gate named on EVERY card is the louder version of naming none. These are - // the three biggest trees in the repo and none of them is corpus. - t('and claims nothing under packages/', !docAuthoringHints.some((h) => hintCovers(h, 'packages/spec/src/index.ts'))); + // Rule 3's root, added when the gate took in the spec's customer-facing zod + // refusal messages. It is NOT one of ROOTS — the Markdown rules never walk it + // — so the gate's own self-test (which derives its declaration from ROOTS) + // cannot pin it and this case is the only place that does. + t('and the spec refusal-message population Rule 3 walks', docAuthoringHints.some((h) => hintCovers(h, 'packages/spec/src/ui/action.zod.ts'))); + // The negative half, load-bearing for a declaration spanning five roots: a + // gate named on EVERY card is the louder version of naming none. `packages/` + // is now claimed in ONE place and must stay that narrow — the whole tree is + // 78 packages and none of the other 77 is corpus, nor is spec's own build + // output, which the walk skips. + t('and claims nothing elsewhere under packages/', !docAuthoringHints.some((h) => hintCovers(h, 'packages/runtime/src/index.ts'))); + t('nor spec outside its source tree', !docAuthoringHints.some((h) => hintCovers(h, 'packages/spec/package.json'))); t('nor under apps/', !docAuthoringHints.some((h) => hintCovers(h, 'apps/console/src/main.tsx'))); t('nor under examples/', !docAuthoringHints.some((h) => hintCovers(h, 'examples/crm/objects/account.object.ts')));