From 5e3a6784feaca81fed72c230ece2d3c695792a9f Mon Sep 17 00:00:00 2001 From: Prem Mallappa Date: Wed, 23 Sep 2026 18:01:53 +0530 Subject: [PATCH 1/4] #316: one hard-failure spec, rendered by code and read by the lint rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The block at docs/design/02-roles.md:276-282 had five copies in the tree — four prose bodies and a private hardFailure() in risk-mandate.ts — and a rule too loose to police any of them: /(?:- .+\n?)+/ accepts any bullets in any order, which is why the subject-line divergence went unnoticed. packages/core/src/guards/hard-failure.ts holds the spec once. A TypeScript renderer produces conformant blocks; skill-lint's new hard-failure-block rule validates prose against the same exported constants. One contract, two enforcement directions. risk-mandate.ts's private copy is deleted and rewired. Its test is UNTOUCHED and still green — it pins the output string verbatim, so it is the regression guard that the swap is behaviour-preserving. The subject line is an open slot keyed Milestone/Story/Goal per gate ruling G-a on #47: the fixed '- Story: #{story}' was unsatisfiable for goal-create, which runs before any Story exists. All four shipped bodies pass the new rule unchanged. Refs #316, #47 --- CONTRIBUTING.md | 1 + packages/core/src/guards/hard-failure.ts | 152 +++++++++++++++++++++++ packages/core/src/guards/index.ts | 14 +++ packages/core/src/guards/risk-mandate.ts | 23 ++-- scripts/skill-lint.ts | 121 +++++++++++++++++- test/skill-lint.test.ts | 58 ++++++++- 6 files changed, 355 insertions(+), 14 deletions(-) create mode 100644 packages/core/src/guards/hard-failure.ts diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b2120dd..0bd7863 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -155,6 +155,7 @@ described above). | `domain-routing-form` | No hardcoded routing form (`domain:` or `skills//`) for a domain the body does not itself own | body, both populations | | `phase-0-section` | A `## Phase 0` heading is present | body, skills only | | `error-handling-section` | A `## Error Handling` heading is present — **the heading only**; what it should cover is author-judged and unenforced (see [Adding a skill](#adding-a-skill)) | body, skills only | +| `hard-failure-block` | The house hard-failure block is present and well-formed: a `Phase N (skill)` headline, then a subject line whose key is one of `Milestone`, `Story`, `Goal`, then `Expected`, `Found`, and an `Action` line carrying the invariant `Pipeline cannot continue. ` prefix | body, skills only | ### claim-lint diff --git a/packages/core/src/guards/hard-failure.ts b/packages/core/src/guards/hard-failure.ts new file mode 100644 index 0000000..5aeda94 --- /dev/null +++ b/packages/core/src/guards/hard-failure.ts @@ -0,0 +1,152 @@ +// The house hard-failure block, as ONE spec consumed in two directions. +// +// Build target 7 of docs/design/stories/47.md. The shape is specified at +// docs/design/02-roles.md:276-282: any Ring 0 agent that finds reality +// disagreeing with the pipeline's premise stops and emits it, and the +// conductor parses it to halt. +// +// TWO CONSUMERS, TWO DIRECTIONS, ONE SPEC. TypeScript call sites render +// through `renderHardFailure`. Prose bodies (skills/*/SKILL.md) are validated +// against the same constants by skill-lint's `hard-failure-block` rule. That +// is why the constants below are exported rather than inlined into the +// renderer: a renderer and a validator that agree only by coincidence are two +// contracts, which is the shape this module exists to remove. +// +// WHY THE PROSE BODIES ARE NOT REWIRED THROUGH THIS RENDERER. A SKILL.md body +// is read by a model, not executed, so no function can emit into it at run +// time. The block is the skill's OUTPUT contract — the model must emit those +// lines verbatim at refusal time — which is the opposite direction from the +// `duplicate-contract` rule's targets, where indirection is free. A body +// saying "emit the block from 02-roles.md" asks a model to have loaded a file +// it may not hold, and to interpolate the slots with no worked example. +// Recorded under gate ruling G-a on #47. + +// The subject line's key, ruled by G-a on #47 to be an OPEN SLOT rather than +// the fixed `- Story:` that docs/design/02-roles.md:278 used to specify. +// +// ORDER IS LOAD-BEARING AND IS BROADEST TO NARROWEST: a Milestone contains +// Stories, a Story is decomposed toward Goals. It is declared here once so the +// renderer and the lint rule cannot disagree about the vocabulary. +// +// The fixed key was unsatisfiable: goal-create runs BEFORE ANY STORY EXISTS +// (skills/goal-create/SKILL.md:49-51), so `- Story: #` has nothing to name +// there, and it shipped `- Goal: ` instead. story-create shipped +// `- Milestone: ` for the same reason one rung up. Both passed +// review, because the rule that was supposed to police the subject line +// matched `(?:- .+\n?)+` and never looked at the key. +export const HARD_FAILURE_SUBJECT_KINDS = ["Milestone", "Story", "Goal"] as const; + +export type HardFailureSubjectKind = (typeof HARD_FAILURE_SUBJECT_KINDS)[number]; + +// Only `Story` carries the `#` sigil, because only a Story is a forge issue +// number. `Goal` names an identifier from the goals source and `Milestone` +// names a milestone; neither is an issue. All four shipped bodies already +// agree with this, and it is asserted rather than assumed. +export const HARD_FAILURE_ISSUE_SIGIL = "#"; + +const SIGIL_KINDS: ReadonlySet = new Set(["Story"]); + +// The invariant halves of the block. Everything a caller supplies is +// interpolated AFTER one of these; nothing a caller supplies can change one. +export const HARD_FAILURE_HEADLINE_PREFIX = "HARD FAILURE in Phase "; + +// The action line is FAR more invariant than it looks. All five copies in the +// tree — the four prose bodies and risk-mandate.ts — preserve this prefix +// exactly and vary only the trailing imperative ("Supply the goals source and +// re-run.", "Declare the domain and re-run.", "Label the Story and re-run.", +// "Supply the missing input and re-run.", "Fix and re-run."). The variable +// part is ONE TRAILING SLOT, which is why `remedy` is a parameter and the rest +// of the line is not. +export const HARD_FAILURE_ACTION_PREFIX = "- Action: Pipeline cannot continue. "; + +// Field order, after the headline. Declared as data so the lint rule can +// assert ORDER rather than mere presence — the old regex accepted any bullets +// in any order, which is how a transposed Expected/Found pair would have +// shipped unnoticed. +export const HARD_FAILURE_FIELD_ORDER = ["subject", "Expected", "Found", "Action"] as const; + +export interface HardFailureSubject { + readonly kind: HardFailureSubjectKind; + // The identifier itself, WITHOUT the `#` sigil. A leading `#` is tolerated + // and stripped exactly once, so a caller holding "#902" off a forge payload + // and a caller holding 902 render identically. At most one is stripped, so + // "##902" stays visibly wrong rather than being silently repaired — the same + // posture as risk-mandate.ts's RUNG_LABEL_PREFIX. + readonly value: string | number; +} + +export interface HardFailureInput { + // The pipeline phase, per docs/design/03-workflow.md's numbering. + readonly phase: number; + // The skill that stopped, e.g. "task-do". Named in the headline's parens. + readonly skill: string; + readonly subject: HardFailureSubject; + // What should have existed. + readonly expected: string; + // What was actually found. The four prose bodies all use the literal "none"; + // that is a PROSE convention enforced by skill-lint, not a constraint here, + // because a TypeScript caller usually knows more than "none" and should say so. + readonly found: string; + // The trailing imperative of the action line, e.g. "Fix and re-run.". + readonly remedy: string; +} + +export class HardFailureInputError extends Error {} + +function renderSubjectValue(subject: HardFailureSubject): string { + const raw = String(subject.value); + const bare = raw.startsWith(HARD_FAILURE_ISSUE_SIGIL) + ? raw.slice(HARD_FAILURE_ISSUE_SIGIL.length) + : raw; + + return SIGIL_KINDS.has(subject.kind) ? `${HARD_FAILURE_ISSUE_SIGIL}${bare}` : bare; +} + +// Renders the block. Pure: it reads no file, runs no process and consults no +// clock. Every fact in the output arrived as an argument. +// +// THIS FUNCTION DELIBERATELY DOES NOT RESOLVE ANYTHING. It does not decide +// whether an artifact exists, whether a claim is covered, or whether a rung is +// permitted — callers determine that and pass the result in. The seam is the +// one risk-mandate.ts:58-60 already documents: "`story` identifies the Story +// for the block message only; this function reads nothing about it." Keeping +// the emitter free of resolution is what lets CLAIM-47.6's refusal ship while +// build target 4's resolver is still blocked (#341). +export function renderHardFailure(input: HardFailureInput): string { + if (!Number.isInteger(input.phase) || input.phase < 0) { + throw new HardFailureInputError( + `phase must be a non-negative integer, received ${String(input.phase)}`, + ); + } + + // Each of these would otherwise render a structurally valid block with an + // empty slot — the failure mode where the block says a refusal happened and + // names nothing about it, which is precisely when it is most wanted. + const blank = (["skill", "expected", "found", "remedy"] as const).find( + (field) => input[field].trim() === "", + ); + if (blank !== undefined) { + throw new HardFailureInputError(`${blank} must not be blank`); + } + if (String(input.subject.value).trim() === "") { + throw new HardFailureInputError("subject.value must not be blank"); + } + + // A newline in any slot would forge extra bullet lines, letting a caller + // inject a field the spec does not have — or split the Action line so the + // invariant prefix is still literally present but no longer governs. + const multiline = (["skill", "expected", "found", "remedy"] as const).find((field) => + input[field].includes("\n"), + ); + if (multiline !== undefined) { + throw new HardFailureInputError(`${multiline} must not contain a newline`); + } + + return ( + `${HARD_FAILURE_HEADLINE_PREFIX}${input.phase} (${input.skill}):\n` + + `- ${input.subject.kind}: ${renderSubjectValue(input.subject)}\n` + + `- Expected: ${input.expected}\n` + + `- Found: ${input.found}\n` + + `${HARD_FAILURE_ACTION_PREFIX}${input.remedy}` + ); +} diff --git a/packages/core/src/guards/index.ts b/packages/core/src/guards/index.ts index 8bd609d..a9e4e51 100644 --- a/packages/core/src/guards/index.ts +++ b/packages/core/src/guards/index.ts @@ -38,3 +38,17 @@ export { deidentifyPrivatePayload } from "./redact"; export { checkSpend } from "./spend"; export { checkRiskMandate } from "./risk-mandate"; export type { Rung } from "./risk-mandate"; +export { + HARD_FAILURE_ACTION_PREFIX, + HARD_FAILURE_FIELD_ORDER, + HARD_FAILURE_HEADLINE_PREFIX, + HARD_FAILURE_ISSUE_SIGIL, + HARD_FAILURE_SUBJECT_KINDS, + HardFailureInputError, + renderHardFailure, +} from "./hard-failure"; +export type { + HardFailureInput, + HardFailureSubject, + HardFailureSubjectKind, +} from "./hard-failure"; diff --git a/packages/core/src/guards/risk-mandate.ts b/packages/core/src/guards/risk-mandate.ts index ab8b7a7..d56bd18 100644 --- a/packages/core/src/guards/risk-mandate.ts +++ b/packages/core/src/guards/risk-mandate.ts @@ -1,4 +1,5 @@ import { decide, type Decision } from "../decision"; +import { renderHardFailure } from "./hard-failure"; // The auto-past-paper-rung refusal ONLY, per Decision 9 of // docs/design/stories/15.md and docs/milestones/M1.md:107. Mandate @@ -45,14 +46,22 @@ function normaliseRung(rung: unknown): string { // after "#" in "Story: #902"); `rung` is the Story's current rung, which may // be any string the caller passed in, including one this module does not // recognise. +// +// This WAS a private hand-rolled copy of the block — the fifth in the tree, +// and the only one in TypeScript. Build target 7 of docs/design/stories/47.md +// replaced it with the shared renderer. `packages/core/test/risk-mandate.test.ts` +// pins the output string verbatim, line for line, and was NOT touched by that +// change: it is the regression guard proving the swap is behaviour-preserving. +// If it ever goes red here, the renderer is wrong, not the test. function hardFailure(story: string | number, rung: string): string { - return ( - "HARD FAILURE in Phase 6 (task-do):\n" + - `- Story: #${story}\n` + - "- Expected: rung:research or rung:paper for an /iai:auto run\n" + - `- Found: rung:${rung}\n` + - "- Action: Pipeline cannot continue. Fix and re-run." - ); + return renderHardFailure({ + phase: 6, + skill: "task-do", + subject: { kind: "Story", value: story }, + expected: "rung:research or rung:paper for an /iai:auto run", + found: `rung:${rung}`, + remedy: "Fix and re-run.", + }); } // Evaluates whether an `/iai:auto` run is permitted to proceed against a diff --git a/scripts/skill-lint.ts b/scripts/skill-lint.ts index f883701..2c824b4 100644 --- a/scripts/skill-lint.ts +++ b/scripts/skill-lint.ts @@ -4,6 +4,8 @@ import { BLOB_SHA_PATTERN, COMMIT_PREFIX_RE, EXCLUSIVE_LABEL_PREFIXES, + HARD_FAILURE_ACTION_PREFIX, + HARD_FAILURE_SUBJECT_KINDS, KNOWN_DOMAIN_IDS, SENTINEL_NAMESPACE_PREFIX, SUB_ISSUE_FEATURE_HEADER, @@ -58,7 +60,9 @@ export type SkillRuleId = | "domain-routing-form" // --- S2.2 / #295. ------------------------------------------------------ | "phase-0-section" - | "error-handling-section"; + | "error-handling-section" + // --- S2.3 / #316. ------------------------------------------------------ + | "hard-failure-block"; export type Severity = "error" | "warning"; @@ -796,6 +800,119 @@ export function lintBodyRules( `write) is author-judged and unenforced (CONTRIBUTING.md:311-318)`, }); } + + // --- hard-failure-block (docs/design/02-roles.md:276-282, gate ruling + // G-a on #47) — SKILLS ONLY, RAW body --------------------------------- + // + // The block being validated lives INSIDE a fenced code block in every + // shipped skill (it is the model's OUTPUT contract, emitted verbatim at + // refusal time — see packages/core/src/guards/hard-failure.ts:15-22). + // `stripFencedCode` would blank out exactly the lines this rule exists + // to check, so this rule runs over `body`, never `stripped`. + const headlineRe = /^HARD FAILURE in Phase (\d+) \(([^)]+)\):$/gm; + const bodyLines = body.split("\n"); + let headlineMatch: RegExpExecArray | null = headlineRe.exec(body); + if (headlineMatch === null) { + violations.push({ + file: filePath, + line: 1, + rule: "hard-failure-block", + severity: "error", + message: + `hard-failure-block: this skill body carries no "HARD FAILURE in Phase N ` + + `(skill):" headline. Every Ring 0 agent that finds reality disagreeing ` + + `with the pipeline's premise stops and emits the house hard-failure block ` + + `(docs/design/02-roles.md:276-282); this body has none for the conductor ` + + `to parse`, + }); + } + while (headlineMatch !== null) { + const headlineLine = lineOf(body, headlineMatch.index); + const fields = bodyLines.slice(headlineLine, headlineLine + 4); + if (fields.length < 4) { + violations.push({ + file: filePath, + line: headlineLine, + rule: "hard-failure-block", + severity: "error", + message: + `hard-failure-block: the block opened by "${headlineMatch[0]}" is truncated ` + + `— only ${String(fields.length)} of the required 4 lines (subject, Expected, ` + + `Found, Action) follow the headline. The full shape is fixed at ` + + `docs/design/02-roles.md:276-282`, + }); + headlineMatch = headlineRe.exec(body); + continue; + } + + const [subjectLine, expectedLine, foundLine, actionLine] = fields; + const subjectLineNo = headlineLine + 1; + + const subjectMatch = /^- ([A-Za-z]+): (.+)$/.exec(subjectLine); + const subjectKey = subjectMatch?.[1]; + if ( + subjectKey === undefined || + !(HARD_FAILURE_SUBJECT_KINDS as readonly string[]).includes(subjectKey) + ) { + violations.push({ + file: filePath, + line: subjectLineNo, + rule: "hard-failure-block", + severity: "error", + message: + `hard-failure-block: the subject line following "${headlineMatch[0]}" names ` + + `${subjectKey === undefined ? "no recognised key" : `"${subjectKey}"`}, but the ` + + `only permitted keys are ${HARD_FAILURE_SUBJECT_KINDS.join(", ")} (gate ruling ` + + `G-a on #47, docs/design/02-roles.md:276-282)`, + }); + } + + const expectedMatch = /^- Expected: (.+)$/.exec(expectedLine); + if (expectedMatch === null || expectedMatch[1].trim() === "") { + violations.push({ + file: filePath, + line: subjectLineNo + 1, + rule: "hard-failure-block", + severity: "error", + message: + `hard-failure-block: the line following the subject line must read ` + + `"- Expected: "; found "${expectedLine}". The block's field order ` + + `is fixed (docs/design/02-roles.md:276-282)`, + }); + } + + const foundMatch = /^- Found: (.+)$/.exec(foundLine); + if (foundMatch === null || foundMatch[1].trim() === "") { + violations.push({ + file: filePath, + line: subjectLineNo + 2, + rule: "hard-failure-block", + severity: "error", + message: + `hard-failure-block: the line following Expected must read ` + + `"- Found: "; found "${foundLine}". The block's field order is ` + + `fixed (docs/design/02-roles.md:276-282)`, + }); + } + + const actionRemainder = actionLine.startsWith(HARD_FAILURE_ACTION_PREFIX) + ? actionLine.slice(HARD_FAILURE_ACTION_PREFIX.length) + : undefined; + if (actionRemainder === undefined || actionRemainder.trim() === "") { + violations.push({ + file: filePath, + line: subjectLineNo + 3, + rule: "hard-failure-block", + severity: "error", + message: + `hard-failure-block: the block's final line must start with the invariant ` + + `"${HARD_FAILURE_ACTION_PREFIX}" prefix followed by a non-blank remedy; found ` + + `"${actionLine}" (docs/design/02-roles.md:276-282)`, + }); + } + + headlineMatch = headlineRe.exec(body); + } } return violations; @@ -883,6 +1000,7 @@ export const RULE_IDS = [ "domain-routing-form", "phase-0-section", "error-handling-section", + "hard-failure-block", ] as const satisfies readonly SkillRuleId[]; // Compile-time exhaustiveness guard. If a member is added to SkillRuleId @@ -933,6 +1051,7 @@ function printReport( "domain-routing-form": 0, "phase-0-section": 0, "error-handling-section": 0, + "hard-failure-block": 0, }; for (const violation of violations) byRule[violation.rule] += 1; diff --git a/test/skill-lint.test.ts b/test/skill-lint.test.ts index 3885be7..515f53e 100644 --- a/test/skill-lint.test.ts +++ b/test/skill-lint.test.ts @@ -24,7 +24,10 @@ import { KNOWN_DOMAIN_IDS } from "../packages/core/src/index"; import { COMMIT_PREFIX_RE, EXCLUSIVE_LABEL_PREFIXES, + HARD_FAILURE_ACTION_PREFIX, + HARD_FAILURE_SUBJECT_KINDS, SENTINEL_NAMESPACE_PREFIX, + renderHardFailure, } from "../packages/core/src/index"; const repoRoot = join(import.meta.dir, ".."); @@ -54,7 +57,41 @@ function writeSkillFile(root: string, skillName: string, content: string): strin // something about #295 it never meant to. Real skill bodies elsewhere in // this file carry both headings for real; this exists only to patch older, // narrower fixtures. -const REQUIRED_SECTIONS_SUFFIX = "\n\n## Phase 0: Context Discovery\n\nx\n\n## Error Handling\n\nx\n"; +// A conformant hard-failure block, RENDERED BY THE PRODUCTION RENDERER rather +// than hand-written here. +// +// This is not convenience: it is the cheapest possible proof that the spec's +// two consumers agree. `renderHardFailure` produces the block and +// `hard-failure-block` validates it, both reading the same constants out of +// packages/core/src/guards/hard-failure.ts. If either drifts, every fixture +// below that carries this suffix fails at once — which is the failure mode a +// hand-written fixture would have hidden, because a hand-written fixture +// agrees with the rule and says nothing about the renderer. +// +// The rule runs over the RAW body, so this must stay inside a fence exactly as +// the real bodies keep it. +const CONFORMANT_HARD_FAILURE_BLOCK = renderHardFailure({ + phase: 0, + skill: "x", + subject: { kind: "Story", value: 1 }, + expected: "a thing", + found: "none", + remedy: "Fix and re-run.", +}); + +const REQUIRED_SECTIONS_SUFFIX = + "\n\n## Phase 0: Context Discovery\n\nx\n\n## Error Handling\n\nx\n\n```\n" + + CONFORMANT_HARD_FAILURE_BLOCK + + "\n```\n"; + +// For the ad-hoc single-perturbation fixtures, which build their body inline +// rather than from VALID_SKILL_BODY. Each asserts a rule fires ALONE, so each +// needs a conformant block for the same reason VALID_SKILL_BODY does: without +// it the assertion reports two rules and the perturbation stops being the only +// variable under test. +function withHardFailureBlock(lines: readonly string[]): string { + return [...lines, "", "```", CONFORMANT_HARD_FAILURE_BLOCK, "```", ""].join("\n"); +} afterAll(() => { for (const dir of tempDirs) rmSync(dir, { recursive: true, force: true }); @@ -988,6 +1025,15 @@ const VALID_SKILL_BODY = [ "Handle a missing resource, an existing resource, rate limiting, and a", "partial write.", "", + // Required of every skill body as of #316's `hard-failure-block` rule. + // Rendered, not hand-written — see CONFORMANT_HARD_FAILURE_BLOCK. The + // "fires ALONE" assertions below depend on this being conformant: if it + // were not, they would each report two rules and the perturbation under + // test would stop being the only variable. + "```", + CONFORMANT_HARD_FAILURE_BLOCK, + "```", + "", ].join("\n"); describe("phase-0-section and error-handling-section (CLAIM-41.10)", () => { @@ -1022,25 +1068,25 @@ describe("phase-0-section and error-handling-section (CLAIM-41.10)", () => { // beside the mutated Phase 0 form, so each test also proves the sibling // rule does not co-fire on the malformed heading. test("a Phase 0 heading only inside a fenced code block does not satisfy the rule", () => { - const body = ["```", "## Phase 0", "```", "", "## Error Handling", "content"].join("\n"); + const body = withHardFailureBlock(["```", "## Phase 0", "```", "", "## Error Handling", "content"]); const v = lintBodyRules("skills/x/SKILL.md", body); expect(v.map((x) => x.rule)).toEqual(["phase-0-section"]); }); test("### Phase 0 (H3) does not satisfy the rule — H2 only", () => { - const body = ["### Phase 0", "", "## Error Handling", "content"].join("\n"); + const body = withHardFailureBlock(["### Phase 0", "", "## Error Handling", "content"]); const v = lintBodyRules("skills/x/SKILL.md", body); expect(v.map((x) => x.rule)).toEqual(["phase-0-section"]); }); test("## phase 0 (lowercase) does not satisfy the rule — case-sensitive", () => { - const body = ["## phase 0", "", "## Error Handling", "content"].join("\n"); + const body = withHardFailureBlock(["## phase 0", "", "## Error Handling", "content"]); const v = lintBodyRules("skills/x/SKILL.md", body); expect(v.map((x) => x.rule)).toEqual(["phase-0-section"]); }); test("## Phase 01 does not satisfy the rule — the negative lookahead excludes a longer number", () => { - const body = ["## Phase 01", "", "## Error Handling", "content"].join("\n"); + const body = withHardFailureBlock(["## Phase 01", "", "## Error Handling", "content"]); const v = lintBodyRules("skills/x/SKILL.md", body); expect(v.map((x) => x.rule)).toEqual(["phase-0-section"]); }); @@ -1055,7 +1101,7 @@ describe("phase-0-section and error-handling-section (CLAIM-41.10)", () => { // recorded at test/skill-lint.test.ts's own domain-routing-form block and // in docs/evidence/46-*.md. test("## Error Handlingz does not satisfy the rule — the word boundary excludes a longer word", () => { - const body = ["## Phase 0", "", "## Error Handlingz", "content"].join("\n"); + const body = withHardFailureBlock(["## Phase 0", "", "## Error Handlingz", "content"]); const v = lintBodyRules("skills/x/SKILL.md", body); expect(v.map((x) => x.rule)).toEqual(["error-handling-section"]); }); From 180d90ff186ebb1159d7d82f6609a4a770de592f Mon Sep 17 00:00:00 2001 From: Prem Mallappa Date: Wed, 23 Sep 2026 18:11:10 +0530 Subject: [PATCH 2/4] #316: CLAIM-47.6's refusal, and the negative fixtures that prove the rule fires MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit checkEvidenceMandate is the reader the doctrine at references/evidence-artifacts.md:140-144 never had. It RESOLVES NOTHING: it takes an already-determined resolution and decides what to do about it, and a static assertion pins that its own source imports no fs and no path. That seam is what makes the task shippable. Build target 4's resolver cannot be built as specified (#341): 21 of 54 artifacts carry no Claims row and the other 33 use five incompatible notations, over an immutable corpus. Only one direction is sound — a claim id in ZERO artifacts is certainly uncovered — and a refusal needs only that one. Case 8 is recorded PARTIALLY discharged per gate ruling G-b, carrying the positive direction to BT4. The negative fixtures each assert TWO things: the old loose regex ACCEPTED the malformation, and the new rule rejects it. Without the first, they would only show that some rule fires on some garbage; with it, each one names a defect that would have shipped green — a missing Action-line prefix, a subject key outside the vocabulary, transposed Expected/Found. docs/design/02-roles.md's template restated per G-a: the subject is an open slot keyed Milestone/Story/Goal, and {remedy} is the trailing imperative only. Refs #316, #47, #341 --- docs/design/02-roles.md | 23 +- packages/core/src/guards/evidence-mandate.ts | 94 +++++++ packages/core/src/guards/index.ts | 2 + packages/core/test/hard-failure.test.ts | 232 ++++++++++++++++++ test/evidence-mandate.test.ts | 161 ++++++++++++ test/skill-lint.test.ts | 243 +++++++++++++++++++ 6 files changed, 753 insertions(+), 2 deletions(-) create mode 100644 packages/core/src/guards/evidence-mandate.ts create mode 100644 packages/core/test/hard-failure.test.ts create mode 100644 test/evidence-mandate.test.ts diff --git a/docs/design/02-roles.md b/docs/design/02-roles.md index 809ed29..aea8a45 100644 --- a/docs/design/02-roles.md +++ b/docs/design/02-roles.md @@ -275,12 +275,31 @@ parsed by the conductor to halt. ``` HARD FAILURE in Phase {N} ({skill}): -- Story: #{story} +- {Subject}: {identifier} - Expected: {what should exist} - Found: {what was actually found} -- Action: Pipeline cannot continue. Fix and re-run. +- Action: Pipeline cannot continue. {remedy} ``` +`{Subject}` is an open slot, not a fixed key. It is one of **`Milestone`**, +**`Story`** or **`Goal`** — broadest to narrowest — and only `Story` carries +the `#` sigil, because only a Story is a forge issue. The vocabulary is +exported as `HARD_FAILURE_SUBJECT_KINDS` from `packages/core`, rendered by +`renderHardFailure`, and enforced on every skill body by `skill-lint`'s +`hard-failure-block` rule. + +> **This slot was specified as a fixed `- Story: #{story}` until #316.** That +> was unsatisfiable for two shipped skills: `goal-create` runs *before any +> Story exists*, so it had nothing to name there and emitted `- Goal: `; +> `story-create` emitted `- Milestone: ` for the same reason one +> rung up. Both passed review because the rule meant to police the block +> matched `(?:- .+\n?)+` and never looked at the key. Restated per gate ruling +> **G-a on #47**. + +`{remedy}` is the trailing imperative only. The rest of the `Action:` line is +invariant: every copy in the tree preserves `Pipeline cannot continue. ` and +varies only what follows it. + Worked example: ``` diff --git a/packages/core/src/guards/evidence-mandate.ts b/packages/core/src/guards/evidence-mandate.ts new file mode 100644 index 0000000..86b11fe --- /dev/null +++ b/packages/core/src/guards/evidence-mandate.ts @@ -0,0 +1,94 @@ +import { decide, type Decision } from "../decision"; +import { renderHardFailure } from "./hard-failure"; + +// CLAIM-47.6's refusal: task-verify does not resolve a task whose anchored +// CLAIM-{story}.{n} has no artifact on disk. +// +// The doctrine is references/evidence-artifacts.md:140-144 — "A +// `status:resolved` with no artifact behind it is a claim, not a result." It +// had no reader (Problem 6 of docs/design/stories/47.md); this is the reader. +// +// ============================================================================ +// THIS MODULE RESOLVES NOTHING, AND THAT IS THE DESIGN, NOT AN OMISSION +// ============================================================================ +// +// It takes an ALREADY-DETERMINED resolution and decides what to do about it. +// It does not enumerate docs/evidence/, does not parse a Claims row, and does +// not decide whether a claim is covered. The seam is the one +// risk-mandate.ts:58-60 already documents for the same reason: "`story` +// identifies the Story for the block message only; this function reads nothing +// about it." +// +// The reason is specific and is recorded in #341. Build target 4's +// claim-to-artifact resolver CANNOT be built as the Design specifies, because +// the corpus will not support it: of the 54 artifacts in docs/evidence/, 21 +// carry no `| Claims |` row at all and the other 33 use at least five mutually +// incompatible notations, plus a retired `ISC-*` namespace. docs/evidence/ is +// immutable (NEVER-194.6), so neither the gaps nor the notations can ever be +// normalised. +// +// Only ONE DIRECTION is sound over such a corpus: +// +// SOUND — a claim id appearing in ZERO artifacts is certainly uncovered. +// UNSOUND — a claim id appearing somewhere is therefore covered. This needs +// range expansion across five notations and cannot be had. +// +// A refusal needs only the sound direction, which is why CLAIM-47.6 can ship +// while build target 4 is blocked. Gate ruling G-b on #47 records case 8 as +// PARTIALLY discharged on exactly this basis, carrying the positive direction +// to build target 4. +// +// So: if a future change makes this module read the corpus itself, it has +// silently adopted the unsound direction and pre-empted a ruling that has not +// been made. Supply the resolution; do not compute it here. + +// What the caller determined about the anchored claim's artifact. +// +// `absent` is deliberately NOT "not found" — the distinction matters. A caller +// that could not look (a read failed, a directory was unreadable) has NOT +// established absence, and must not report `absent`, because the refusal below +// is indistinguishable from the one earned by a genuinely missing artifact. +// Fail the read loudly instead; do not launder it into a verdict. +export type EvidenceResolution = + | { readonly kind: "present"; readonly artifact: string } + | { readonly kind: "absent" }; + +export interface EvidenceMandateInput { + // The task issue being verified, for the block's subject line. + readonly task: string | number; + // The Story the task belongs to, named in the refusal's prose. + readonly story: string | number; + // The claim the task is anchored to, e.g. "CLAIM-47.6". + readonly claim: string; + readonly resolution: EvidenceResolution; +} + +// Phase 7 is task-verify (docs/design/03-workflow.md:21). +const TASK_VERIFY_PHASE = 7; +const TASK_VERIFY_SKILL = "task-verify"; + +export function checkEvidenceMandate(input: EvidenceMandateInput): Decision { + if (input.resolution.kind === "present") { + return decide( + "allow", + `Task #${String(input.task)} is anchored to ${input.claim}, and ` + + `${input.resolution.artifact} is on disk behind it`, + ); + } + + return decide( + "block", + renderHardFailure({ + phase: TASK_VERIFY_PHASE, + skill: TASK_VERIFY_SKILL, + subject: { kind: "Story", value: input.story }, + expected: `an evidence artifact on disk for ${input.claim}, anchored by task #${String(input.task)}`, + found: "none", + // The doctrine, said in the block rather than only in the reference, so + // the person who reads the refusal reads the reason with it. + remedy: + "A status:resolved with no artifact behind it is a claim, not a result. " + + "Run the verification, write the artifact, and re-run.", + }), + ); +} diff --git a/packages/core/src/guards/index.ts b/packages/core/src/guards/index.ts index a9e4e51..262c650 100644 --- a/packages/core/src/guards/index.ts +++ b/packages/core/src/guards/index.ts @@ -52,3 +52,5 @@ export type { HardFailureSubject, HardFailureSubjectKind, } from "./hard-failure"; +export { checkEvidenceMandate } from "./evidence-mandate"; +export type { EvidenceMandateInput, EvidenceResolution } from "./evidence-mandate"; diff --git a/packages/core/test/hard-failure.test.ts b/packages/core/test/hard-failure.test.ts new file mode 100644 index 0000000..9bb5689 --- /dev/null +++ b/packages/core/test/hard-failure.test.ts @@ -0,0 +1,232 @@ +import { describe, expect, test } from "bun:test"; +import { + HARD_FAILURE_ACTION_PREFIX, + HARD_FAILURE_HEADLINE_PREFIX, + HARD_FAILURE_SUBJECT_KINDS, + HardFailureInputError, + renderHardFailure, +} from "../src/index"; + +// NOTE ON SCOPE (#316): the module's "two directions" property — that a +// rendered block also satisfies skill-lint's `hard-failure-block` rule, for +// every member of HARD_FAILURE_SUBJECT_KINDS — is NOT in this file. Importing +// `scripts/skill-lint` from `packages/core/test/` trips the repository's own +// `no-host-import` lint rule (scope: core; see +// packages/core/test/binding-conformance.test.ts for the same discovery made +// on a different import), because it resolves outside packages/core. That one +// property lives in `test/skill-lint.test.ts` instead, appended alongside the +// existing hard-failure-block coverage there, which already imports +// `scripts/skill-lint` from outside any package's scope. + +describe("HARD_FAILURE_SUBJECT_KINDS", () => { + test("is Milestone, Story, Goal, in that order — broadest to narrowest per gate ruling G-a on #47, and skill-lint's hard-failure-block message text names this order", () => { + expect(HARD_FAILURE_SUBJECT_KINDS).toEqual(["Milestone", "Story", "Goal"]); + }); + + test("has exactly 3 members — the denominator, so a silently added kind is noticed", () => { + expect(HARD_FAILURE_SUBJECT_KINDS.length).toBe(3); + }); +}); + +describe("renderHardFailure — the canonical shape (phase 6, skill task-do, Story 902)", () => { + const canonical = renderHardFailure({ + phase: 6, + skill: "task-do", + subject: { kind: "Story", value: 902 }, + expected: "rung:research or rung:paper for an /iai:auto run", + found: "rung:live", + remedy: "Fix and re-run.", + }); + + // Directly comparable to docs/design/03-workflow.md:498-504's fenced block — + // same phase, same skill, same Story, so the literal below can be read + // side by side with the doc rather than trusted on faith. + test("renders the exact 5-line block, byte for byte", () => { + expect(canonical).toBe( + "HARD FAILURE in Phase 6 (task-do):\n" + + "- Story: #902\n" + + "- Expected: rung:research or rung:paper for an /iai:auto run\n" + + "- Found: rung:live\n" + + "- Action: Pipeline cannot continue. Fix and re-run.", + ); + }); + + test("has exactly 5 lines and no trailing newline", () => { + const lines = canonical.split("\n"); + expect(lines.length).toBe(5); + expect(canonical.endsWith("\n")).toBe(false); + }); + + test("starts with HARD_FAILURE_HEADLINE_PREFIX, and the 5th line starts with HARD_FAILURE_ACTION_PREFIX", () => { + // Asserted against the imported constants, never a restated literal — + // restating either here would let the test and the renderer drift + // together instead of the test catching the drift. + expect(canonical.startsWith(HARD_FAILURE_HEADLINE_PREFIX)).toBe(true); + const lines = canonical.split("\n"); + expect(lines[4]?.startsWith(HARD_FAILURE_ACTION_PREFIX)).toBe(true); + }); +}); + +describe("the # sigil rule", () => { + test("Story renders with the # sigil", () => { + const block = renderHardFailure({ + phase: 0, + skill: "x", + subject: { kind: "Story", value: 902 }, + expected: "e", + found: "f", + remedy: "r", + }); + expect(block).toContain("- Story: #902"); + }); + + test("Goal renders with NO # sigil — a Goal names an identifier from the goals source, not a forge issue", () => { + const block = renderHardFailure({ + phase: 0, + skill: "x", + subject: { kind: "Goal", value: "g-1" }, + expected: "e", + found: "f", + remedy: "r", + }); + expect(block).toContain("- Goal: g-1"); + }); + + test("Milestone renders with NO # sigil", () => { + const block = renderHardFailure({ + phase: 0, + skill: "x", + subject: { kind: "Milestone", value: "M2" }, + expected: "e", + found: "f", + remedy: "r", + }); + expect(block).toContain("- Milestone: M2"); + }); + + test("a Story value already carrying # renders identically to the bare form — the sigil is stripped exactly once", () => { + const withHash = renderHardFailure({ + phase: 0, + skill: "x", + subject: { kind: "Story", value: "#902" }, + expected: "e", + found: "f", + remedy: "r", + }); + const bare = renderHardFailure({ + phase: 0, + skill: "x", + subject: { kind: "Story", value: 902 }, + expected: "e", + found: "f", + remedy: "r", + }); + expect(withHash).toBe(bare); + }); + + // "##902" is NOT repaired to "#902" — only one leading "#" is ever + // stripped. Same posture as risk-mandate.ts's RUNG_LABEL_PREFIX, which + // strips at most one "rung:" prefix so "rung:rung:live" stays visibly + // wrong rather than being silently fixed into a permitted rung. + test('"##902" renders "- Story: ##902" — not repaired', () => { + const block = renderHardFailure({ + phase: 0, + skill: "x", + subject: { kind: "Story", value: "##902" }, + expected: "e", + found: "f", + remedy: "r", + }); + expect(block).toContain("- Story: ##902"); + }); + + test("a numeric value and the equivalent string value render identically", () => { + const numeric = renderHardFailure({ + phase: 0, + skill: "x", + subject: { kind: "Story", value: 902 }, + expected: "e", + found: "f", + remedy: "r", + }); + const stringy = renderHardFailure({ + phase: 0, + skill: "x", + subject: { kind: "Story", value: "902" }, + expected: "e", + found: "f", + remedy: "r", + }); + expect(numeric).toBe(stringy); + }); +}); + +describe("input rejection — all throw HardFailureInputError", () => { + const valid = { + phase: 0, + skill: "x", + subject: { kind: "Story" as const, value: 1 }, + expected: "e", + found: "f", + remedy: "r", + }; + + const blankableFields = ["skill", "expected", "found", "remedy"] as const; + + // Asserted FIRST so the loop below cannot pass vacuously if a field were + // silently dropped from the checked set. + test("the loop below covers exactly the 4 blankable string fields", () => { + expect(blankableFields.length).toBe(4); + }); + + for (const field of blankableFields) { + test(`blank ${field} throws HardFailureInputError`, () => { + expect(() => renderHardFailure({ ...valid, [field]: "" })).toThrow(HardFailureInputError); + }); + + test(`whitespace-only ${field} throws HardFailureInputError`, () => { + expect(() => renderHardFailure({ ...valid, [field]: " " })).toThrow(HardFailureInputError); + }); + } + + test("blank subject.value throws HardFailureInputError", () => { + expect(() => + renderHardFailure({ ...valid, subject: { kind: "Story", value: "" } }), + ).toThrow(HardFailureInputError); + }); + + // A newline in `expected` would forge an extra bullet line, letting a + // caller inject a field the spec does not have — or split the Action line + // so the invariant "- Action: ..." prefix is still literally present but + // no longer governs the line it opens. + test("a newline inside expected throws HardFailureInputError", () => { + expect(() => renderHardFailure({ ...valid, expected: "line one\nline two" })).toThrow( + HardFailureInputError, + ); + }); + + // Same hazard, the other invariant slot. + test("a newline inside remedy throws HardFailureInputError", () => { + expect(() => renderHardFailure({ ...valid, remedy: "line one\nline two" })).toThrow( + HardFailureInputError, + ); + }); + + test("a negative phase throws HardFailureInputError", () => { + expect(() => renderHardFailure({ ...valid, phase: -1 })).toThrow(HardFailureInputError); + }); + + test("a non-integer phase throws HardFailureInputError", () => { + expect(() => renderHardFailure({ ...valid, phase: 1.5 })).toThrow(HardFailureInputError); + }); + + test("NaN phase throws HardFailureInputError", () => { + expect(() => renderHardFailure({ ...valid, phase: NaN })).toThrow(HardFailureInputError); + }); + + // Boundary: phase 0 is VALID and must NOT throw — all four shipped prose + // bodies use phase 0, so a defect here would break every one of them. + test("phase 0 is valid and does not throw", () => { + expect(() => renderHardFailure({ ...valid, phase: 0 })).not.toThrow(); + }); +}); diff --git a/test/evidence-mandate.test.ts b/test/evidence-mandate.test.ts new file mode 100644 index 0000000..d4dcba5 --- /dev/null +++ b/test/evidence-mandate.test.ts @@ -0,0 +1,161 @@ +import { describe, expect, test } from "bun:test"; +import { readFileSync, readdirSync } from "node:fs"; +import { join } from "node:path"; +import { + HARD_FAILURE_ACTION_PREFIX, + HARD_FAILURE_HEADLINE_PREFIX, + checkEvidenceMandate, +} from "../packages/core/src/index"; + +const repoRoot = join(import.meta.dir, ".."); +const EVIDENCE_DIR = join(repoRoot, "docs", "evidence"); + +function evidenceFiles(): readonly string[] { + return readdirSync(EVIDENCE_DIR).filter((f) => f.endsWith(".md")); +} + +// =========================================================================== +// CASE 8 of docs/test-plans/47-plan.md — CLAIM-47.6, P0, tool-checked. +// +// "The artifact count is read at run time and asserted non-zero FIRST; then +// a task anchored to a claim with no artifact emits the hard-failure +// block. Evidence precedes the label, so a status:resolved with nothing +// behind it is a claim, not a result." +// +// --------------------------------------------------------------------------- +// PARTIALLY DISCHARGED, BY GATE RULING G-b ON #47. READ THIS BEFORE EXTENDING. +// --------------------------------------------------------------------------- +// +// This file discharges the REFUSAL direction. It does not discharge, and must +// not be read as discharging, the positive direction — "this claim IS covered, +// because here is its artifact". That belongs to build target 4 and is blocked +// on a corpus problem recorded in #341: +// +// Of the artifacts on disk, a large minority carry no `| Claims |` row at +// all, and the remainder use at least five mutually incompatible range +// notations plus a retired `ISC-*` namespace. docs/evidence/ is IMMUTABLE +// (NEVER-194.6), so the gaps can never be backfilled and the notations can +// never be normalised. +// +// Which leaves exactly one sound direction: +// +// SOUND — a claim id appearing in ZERO artifacts is certainly uncovered. +// UNSOUND — a claim id appearing somewhere is therefore covered. +// +// A refusal needs only the sound one. The CARRY is: build target 4 owns the +// positive direction, and #341 owns the ruling on whether it is reconstructed +// from the Design/plan side instead of from the artifacts. +// =========================================================================== +describe("case 8 (CLAIM-47.6): a task anchored to a claim with no artifact is refused", () => { + // THE DENOMINATOR, FIRST AND ALONE, because the plan's own wording demands + // that ordering. Every assertion below reasons about "appears in zero + // artifacts"; over an empty directory that is vacuously true of EVERY claim + // id, and the refusal test would pass while proving nothing at all. + test("the docs/evidence/ corpus is non-zero, so the absence reasoning below is not vacuous", () => { + expect(evidenceFiles().length).toBeGreaterThan(0); + }); + + // The synthetic task's anchor. Story 999 does not exist and never will, so + // no artifact can ever legitimately mention it -- which is what makes this + // fixture stable rather than a hostage to whatever ships next. + const UNCOVERED_CLAIM = "CLAIM-999.1"; + + test("the synthetic claim appears in ZERO artifacts on disk -- the sound direction, measured", () => { + const files = evidenceFiles(); + expect(files.length).toBeGreaterThan(0); // denominator again, this test standing alone + + const hits = files.filter((f) => + readFileSync(join(EVIDENCE_DIR, f), "utf8").includes(UNCOVERED_CLAIM), + ); + + // Asserted as the LIST, not as a count. A count says "1"; the list says + // which file, which is the difference between a failure you can act on + // and one you have to go looking for. #321's M5/M6 pair is the recorded + // instance of a length check hiding what a value check would have named. + expect(hits, `${UNCOVERED_CLAIM} must be uncovered for this fixture to mean anything`).toEqual([]); + }); + + test("a task anchored to that claim is REFUSED, and the refusal is the house hard-failure block", () => { + const decision = checkEvidenceMandate({ + task: 9991, + story: 999, + claim: UNCOVERED_CLAIM, + resolution: { kind: "absent" }, + }); + + expect(decision.action).toBe("block"); + + // The block, asserted through the exported constants rather than restated + // literals -- a test that restated them could drift into agreeing with + // itself while the renderer changed underneath. + expect(decision.message.startsWith(HARD_FAILURE_HEADLINE_PREFIX)).toBe(true); + const lines = decision.message.split("\n"); + expect(lines).toHaveLength(5); + expect(lines[4].startsWith(HARD_FAILURE_ACTION_PREFIX)).toBe(true); + + // It must name the claim it refused on. A refusal that does not say what + // was missing sends the reader back to the code to find out. + expect(decision.message).toContain(UNCOVERED_CLAIM); + // And record the absence, not merely fail -- the same "tied to ABSENCE" + // posture case 12 asserts of the prose bodies. + expect(lines[3]).toBe("- Found: none"); + }); + + // EVIDENCE PRECEDES THE LABEL. The doctrine at + // references/evidence-artifacts.md:140-144 is the reason the refusal exists, + // so the refusal says it. + test("the refusal states the doctrine: a status:resolved with no artifact is a claim, not a result", () => { + const decision = checkEvidenceMandate({ + task: 9991, + story: 999, + claim: UNCOVERED_CLAIM, + resolution: { kind: "absent" }, + }); + expect(decision.message).toContain("is a claim, not a result"); + }); + + // THE OTHER HALF OF THE PARTITION. Without this the guard could be a + // constant function returning `block`, and every assertion above would + // still pass. Mutation M9/M9b of #320 is the recorded instance of exactly + // that shape going unnoticed. + test("a task whose claim HAS an artifact is allowed, so the guard is not a constant refusal", () => { + const decision = checkEvidenceMandate({ + task: 9991, + story: 999, + claim: UNCOVERED_CLAIM, + resolution: { kind: "present", artifact: "docs/evidence/999-20260101T000000Z.md" }, + }); + + expect(decision.action).toBe("allow"); + expect(decision.message).not.toContain(HARD_FAILURE_HEADLINE_PREFIX); + }); + + // THE SEAM, ASSERTED STATICALLY. #316's whole shippability rests on this + // module resolving nothing -- see packages/core/src/guards/evidence-mandate.ts's + // header and #341. A future edit that makes it read the corpus itself would + // silently adopt the UNSOUND direction and pre-empt a ruling nobody has made. + // + // Behaviourally that change is invisible: the guard would return the same + // verdicts for every input this file supplies. #322's M11 is the precedent + // for the kill -- a behaviourally equivalent mutation can only be killed + // statically. + test("the guard's own source reads no corpus: no fs, no path, no directory listing", async () => { + const source = await Bun.file( + join(repoRoot, "packages", "core", "src", "guards", "evidence-mandate.ts"), + ).text(); + + // Guard the extraction first, so a failed read cannot pass vacuously. + expect(source.length).toBeGreaterThan(0); + expect(source).toContain("checkEvidenceMandate"); + + const imports = source + .split("\n") + .filter((l) => l.startsWith("import ")) + .join("\n"); + expect(imports.length).toBeGreaterThan(0); + + for (const forbidden of ["node:fs", "node:path", "readdirSync", "readFileSync", "Bun.file"]) { + expect(imports, `evidence-mandate.ts must not import ${forbidden}`).not.toContain(forbidden); + } + }); +}); diff --git a/test/skill-lint.test.ts b/test/skill-lint.test.ts index 515f53e..49b502a 100644 --- a/test/skill-lint.test.ts +++ b/test/skill-lint.test.ts @@ -1545,3 +1545,246 @@ describe("case 12: absence of the domain: label produces the hard-failure block, } }); }); + +// --- #316: the two-directions property, for every subject kind ----------- +// +// `renderHardFailure` (packages/core/src/guards/hard-failure.ts) and this +// file's `hard-failure-block` rule read the same HARD_FAILURE_SUBJECT_KINDS +// constant. CONFORMANT_HARD_FAILURE_BLOCK above already proves this for +// "Story"; this closes "Milestone" and "Goal" too, so the agreement is +// checked for every kind the vocabulary declares rather than just the one +// most fixtures happen to use. +// +// This lives here, not in packages/core/test/hard-failure.test.ts, because +// importing `scripts/skill-lint` from inside packages/core/test/ trips the +// repository's own `no-host-import` lint rule (scope: core): the specifier +// resolves outside packages/core, which is exactly what that rule polices +// (see packages/core/test/binding-conformance.test.ts's note on the same +// discovery for a different import). This file already imports +// `scripts/skill-lint` from outside any package's scope, so the same +// assertion here costs nothing extra. +describe("hard-failure-block and renderHardFailure agree, for every HARD_FAILURE_SUBJECT_KINDS member (#316)", () => { + // Asserted first so the loop below cannot pass by iterating zero times if + // HARD_FAILURE_SUBJECT_KINDS were ever silently emptied. + test("HARD_FAILURE_SUBJECT_KINDS is non-empty", () => { + expect(HARD_FAILURE_SUBJECT_KINDS.length).toBeGreaterThan(0); + }); + + for (const kind of HARD_FAILURE_SUBJECT_KINDS) { + test(`a rendered ${kind} block satisfies hard-failure-block with 0 violations`, () => { + const block = renderHardFailure({ + phase: 0, + skill: "x", + subject: { kind, value: kind === "Story" ? 902 : "id-1" }, + expected: "a thing", + found: "none", + remedy: "Fix and re-run.", + }); + + const body = [ + "## Phase 0", + "", + "x", + "", + "## Error Handling", + "", + "x", + "", + "```", + block, + "```", + "", + ].join("\n"); + + // Filtered to hard-failure-block alone, so this test is about the ONE + // rule under test and cannot be confused by phase-0-section or + // error-handling-section firing for an unrelated reason. + const violations = lintBodyRules("skills/x/SKILL.md", body).filter( + (v) => v.rule === "hard-failure-block", + ); + expect(violations).toEqual([]); + }); + } +}); + +// =========================================================================== +// #316 — hard-failure-block NEGATIVE FIXTURES +// +// THE POSTURE (#253 / #261 / #272): a rule that has never been observed to +// fire is not a rule, it is a comment. Every fixture below is malformed in +// exactly ONE way and is asserted to produce exactly ONE violation. +// +// AND EACH CARRIES A SECOND ASSERTION THAT IS THE REAL POINT. #316's issue +// body claimed "no lint rule requires it". That was false — case 6 above has +// required the block since S2.2, using HARD_FAILURE_BLOCK_RE (:1357). What was +// true is that the rule was too LOOSE: its tail is `(?:- .+\n?)+`, which +// accepts any bullet lines in any order, checking neither field order, nor the +// subject key, nor the Action line at all. +// +// So every fixture here asserts BOTH: +// (i) the OLD regex ACCEPTS it — proving the old rule was insufficient +// (ii) the NEW rule REJECTS it — proving the replacement closed the gap +// +// One fixture, two facts. Without (i) these would only show that some rule +// fires on some garbage; with it, each one names a defect that would have +// shipped green. This is the same argument #323's case 16 needed when four +// green runs turned out to be one ignored flag away from vacuous. +describe("hard-failure-block negative fixtures: the old regex accepted these, the new rule does not (#316)", () => { + function bodyWith(block: string): string { + return ["## Phase 0", "", "x", "", "## Error Handling", "", "x", "", "```", block, "```", ""].join( + "\n", + ); + } + + function hardFailureViolations(block: string) { + return lintBodyRules("skills/x/SKILL.md", bodyWith(block)).filter( + (v) => v.rule === "hard-failure-block", + ); + } + + // VACUITY GUARD, FIRST. If the conformant block did not pass, every + // "fires" assertion below could be firing for an unrelated reason and this + // whole suite would prove nothing about the perturbation it names. + test("the conformant block produces ZERO violations, so the fixtures below isolate one defect each", () => { + expect(hardFailureViolations(CONFORMANT_HARD_FAILURE_BLOCK)).toEqual([]); + // And the old regex accepts it too — so (i) below is a statement about + // the malformation, not about the regex rejecting everything. + expect(CONFORMANT_HARD_FAILURE_BLOCK).toMatch(HARD_FAILURE_BLOCK_RE); + }); + + // THE ACTION LINE. This is the highest-value fixture in the file: the + // invariant prefix is what every one of the five copies in the tree + // preserves, and the old regex never looked at the Action line at all. + test("an Action line missing the invariant prefix: OLD regex accepts, NEW rule rejects", () => { + const block = [ + "HARD FAILURE in Phase 0 (x):", + "- Story: #1", + "- Expected: a thing", + "- Found: none", + "- Action: Fix and re-run.", + ].join("\n"); + + // (i) the defect the old rule could not see + expect(block, "the old regex must accept this, or the fixture proves nothing").toMatch( + HARD_FAILURE_BLOCK_RE, + ); + + // (ii) the gap, closed + const v = hardFailureViolations(block); + expect(v).toHaveLength(1); + expect(v[0].message).toContain(HARD_FAILURE_ACTION_PREFIX); + }); + + test("a subject key outside the vocabulary: OLD regex accepts, NEW rule rejects and names the vocabulary", () => { + const block = [ + "HARD FAILURE in Phase 0 (x):", + "- Epic: #1", + "- Expected: a thing", + "- Found: none", + HARD_FAILURE_ACTION_PREFIX + "Fix and re-run.", + ].join("\n"); + + expect(block).toMatch(HARD_FAILURE_BLOCK_RE); + + const v = hardFailureViolations(block); + expect(v).toHaveLength(1); + expect(v[0].message).toContain("Epic"); + // The message lists the permitted keys by reading the exported constant, + // so a vocabulary change cannot leave the diagnostic stale. + for (const kind of HARD_FAILURE_SUBJECT_KINDS) { + expect(v[0].message).toContain(kind); + } + }); + + // FIELD ORDER. Transposition is the defect a presence-only rule can never + // see: both lines are present, both match `- .+`, and the block reads as + // an answer to a question nobody asked. + test("Expected and Found transposed: OLD regex accepts, NEW rule rejects", () => { + const block = [ + "HARD FAILURE in Phase 0 (x):", + "- Story: #1", + "- Found: none", + "- Expected: a thing", + HARD_FAILURE_ACTION_PREFIX + "Fix and re-run.", + ].join("\n"); + + expect(block).toMatch(HARD_FAILURE_BLOCK_RE); + + // Two violations, because BOTH ordered slots are wrong — the Expected + // slot holds Found and vice versa. Asserted as a count so a rule that + // noticed only the first would fail here. + const v = hardFailureViolations(block); + expect(v).toHaveLength(2); + }); + + test("a blank Expected value: OLD regex accepts, NEW rule rejects", () => { + const block = [ + "HARD FAILURE in Phase 0 (x):", + "- Story: #1", + "- Expected: ", + "- Found: none", + HARD_FAILURE_ACTION_PREFIX + "Fix and re-run.", + ].join("\n"); + + expect(block).toMatch(HARD_FAILURE_BLOCK_RE); + expect(hardFailureViolations(block)).toHaveLength(1); + }); + + // ABSENCE. The one case the old regex DID catch — asserted anyway, because + // a replacement that closed the loose half while dropping the strict half + // would be a regression no other test here would notice. + test("no block at all is still rejected — the replacement did not drop what the old rule did catch", () => { + const body = ["## Phase 0", "", "x", "", "## Error Handling", "", "x", ""].join("\n"); + expect(body).not.toMatch(HARD_FAILURE_BLOCK_RE); + + const v = lintBodyRules("skills/x/SKILL.md", body).filter((x) => x.rule === "hard-failure-block"); + expect(v).toHaveLength(1); + }); + + test("a truncated block is rejected and says so", () => { + const block = ["HARD FAILURE in Phase 0 (x):", "- Story: #1"].join("\n"); + const v = lintBodyRules("skills/x/SKILL.md", ["```", block, "```"].join("\n")).filter( + (x) => x.rule === "hard-failure-block", + ); + expect(v).toHaveLength(1); + expect(v[0].message).toContain("truncated"); + }); + + // EVERY block is validated, not merely the first. A body that carries a + // conformant block and then a malformed one must still be reported: a rule + // that stopped at the first match would let the second ship, which is the + // shape that lets a bad example sit in a body being copied from. + test("a SECOND malformed block is reported even when the first is conformant", () => { + const bad = [ + "HARD FAILURE in Phase 1 (y):", + "- Story: #2", + "- Expected: a thing", + "- Found: none", + "- Action: Fix and re-run.", + ].join("\n"); + + const body = [ + "## Phase 0", + "", + "x", + "", + "## Error Handling", + "", + "x", + "", + "```", + CONFORMANT_HARD_FAILURE_BLOCK, + "```", + "", + "```", + bad, + "```", + "", + ].join("\n"); + + const v = lintBodyRules("skills/x/SKILL.md", body).filter((x) => x.rule === "hard-failure-block"); + expect(v).toHaveLength(1); + // And it points at the SECOND block's Action line, not the first's. + expect(v[0].line).toBeGreaterThan(body.split("\n").indexOf("```") + 1); + }); +}); From b0616b913dbd466d59cc0cfbbf6b2b5b14c2dee5 Mon Sep 17 00:00:00 2001 From: Prem Mallappa Date: Wed, 23 Sep 2026 18:49:38 +0530 Subject: [PATCH 3/4] #316: the fixture M5 proved was passing for the wrong reason Mutation M5 deleted the Action line's invariant-prefix check entirely -- startsWith(HARD_FAILURE_ACTION_PREFIX) became startsWith("- Action:") -- and the suite stayed green. The negative fixture meant to pin that check still failed the mutant, but by accident: its Action line is SHORTER than the 36-character prefix, so the surviving .slice(PREFIX.length) ran off the end and yielded "", which the blank-remainder branch rejected regardless. A length accident was standing in for a content comparison. The added fixture miscases one word and keeps the line long enough that the slice yields a non-blank remainder either way, so the only thing that can reject it is a real comparison against the invariant. It is asserted to be longer than the prefix, and its remainder asserted non-blank, so the accident cannot come back silently. Round: 12 mutations, 12 killed -- 11 on the first pass, M5 after this fix. Refs #316 --- test/skill-lint.test.ts | 42 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/test/skill-lint.test.ts b/test/skill-lint.test.ts index 49b502a..0f19382 100644 --- a/test/skill-lint.test.ts +++ b/test/skill-lint.test.ts @@ -1675,6 +1675,48 @@ describe("hard-failure-block negative fixtures: the old regex accepted these, th expect(v[0].message).toContain(HARD_FAILURE_ACTION_PREFIX); }); + // THE FIXTURE ABOVE IS NOT SUFFICIENT ON ITS OWN, AND MUTATION M5 IS WHY. + // + // M5 replaced the rule's `startsWith(HARD_FAILURE_ACTION_PREFIX)` with + // `startsWith("- Action:")` -- deleting the entire invariant check -- and + // the suite stayed GREEN. The fixture above still failed the mutant, but + // for the WRONG REASON: "- Action: Fix and re-run." is SHORTER than the + // 36-character prefix, so the surviving `.slice(PREFIX.length)` ran off the + // end and yielded "", which the blank-remainder branch rejected anyway. + // The fixture was being caught by a length accident, not by the prefix + // check it was written to pin. + // + // This fixture discriminates the prefix's CONTENT: the Action line is long + // enough that the slice yields a non-blank remainder either way, so the + // ONLY thing that can reject it is a real comparison against the invariant. + // Under the mutant it is accepted; under the real rule it is not. + // + // Same class as the case-only assertion recorded in docs/evidence/33-*.md + // and re-hit in #315: two assertions that differ only in a way the code + // never reads are one assertion. + test("an Action line the right LENGTH but the wrong CONTENT is rejected -- the prefix is compared, not counted", () => { + const block = [ + "HARD FAILURE in Phase 0 (x):", + "- Story: #1", + "- Expected: a thing", + "- Found: none", + // "CONTINUE" miscased: same shape, same length, not the invariant. + "- Action: Pipeline cannot CONTINUE. Fix and re-run.", + ].join("\n"); + + expect(block).toMatch(HARD_FAILURE_BLOCK_RE); + + // The remainder after slicing by the prefix length is non-blank here, so + // the blank-remainder branch CANNOT be what rejects this one. + const actionLine = block.split("\n")[4]; + expect(actionLine.length).toBeGreaterThan(HARD_FAILURE_ACTION_PREFIX.length); + expect(actionLine.slice(HARD_FAILURE_ACTION_PREFIX.length).trim()).not.toBe(""); + + const v = hardFailureViolations(block); + expect(v).toHaveLength(1); + expect(v[0].message).toContain(HARD_FAILURE_ACTION_PREFIX); + }); + test("a subject key outside the vocabulary: OLD regex accepts, NEW rule rejects and names the vocabulary", () => { const block = [ "HARD FAILURE in Phase 0 (x):", From e6f38c7690d238769613ff5ae43390465f7f5cd8 Mon Sep 17 00:00:00 2001 From: Prem Mallappa Date: Wed, 23 Sep 2026 18:51:14 +0530 Subject: [PATCH 4/4] #316: record the verification evidence Verdict PASS. 963 tests / 949 pass / 0 fail / 14 skip, measured at the parent commit b0616b9; the +2 expect() and +1 claim-lint file this artifact itself adds are stated rather than measured, per #299, and both predictions were confirmed after it landed. 12 mutations, 12 killed. M5 is the round's return: it deleted the Action line's invariant check and the suite stayed green, because the fixture pinning that check was shorter than the prefix and was being rejected by a length accident instead of a content comparison. Case 8 is PARTIALLY discharged per gate ruling G-b, carrying the positive direction to build target 4 and #341. Refs #316 --- docs/evidence/316-20260923T131945Z.md | 193 ++++++++++++++++++++++++++ 1 file changed, 193 insertions(+) create mode 100644 docs/evidence/316-20260923T131945Z.md diff --git a/docs/evidence/316-20260923T131945Z.md b/docs/evidence/316-20260923T131945Z.md new file mode 100644 index 0000000..2cd0c83 --- /dev/null +++ b/docs/evidence/316-20260923T131945Z.md @@ -0,0 +1,193 @@ +--- +issue: 316 +repo: dev-pmallapp/iAI +commit: b0616b913dbd466d59cc0cfbbf6b2b5b14c2dee5 +branch: task/316-hard-failure-block-emitter +run: 2026-09-23T13:19:45Z +verdict: PASS +--- + +# Evidence: #316 — The hard-failure block emitter + +| Field | Value | +|-------|-------| +| Verdict | **PASS** | +| Task | S2.3.6. Build target **7** of `docs/design/stories/47.md`. Anchors `CLAIM-47.6` | +| Chain | **963 tests / 949 pass / 0 fail / 14 skip** · lint **0 across 98 files** · typecheck clean · skill-lint **4 + 12, 0 errors, 18 rules** · claim-lint **115 files, 0 errors, 1 warning** · both verifiers PASS at **six** · purity **9 pass**, binding sweep **5 pass** | +| `expect()` | **3324 measured at `b0616b9`; 3326 once this artifact lands** — #299, and the delta is the evidence-artifact corpus (`evidence-upsert.test.ts:33-50`), not a rounding. claim-lint likewise goes **115 → 116 files** | +| Baseline | **917 tests / 903 pass / 3243 `expect()`** · lint 0 across **95** files · claim-lint **115 files** at `fcd91c7` | +| Delta | **+46 tests, +81 `expect()`, +3 lint files, +1 skill-lint rule** | +| Mutations | **12 run, 12 killed.** 11 on the first pass; **M5 survived and found a fixture that was passing for the wrong reason** | +| Cases | `docs/test-plans/47-plan.md` case **8 PARTIALLY DISCHARGED** per gate ruling **G-b**, with the carry recorded below | +| Gate rulings | **G-a**, **G-b**, **G-c** — ruled on #47 before any code was written | + +> **The commit above is this artifact's PARENT.** An evidence artifact cannot +> carry the hash of the commit that contains it. The figures were measured on +> `b0616b9`; the two `expect()` calls and one claim-lint file this file itself +> adds are stated rather than measured, per #299. + +## The task began by finding its own issue body wrong + +#316 said, of the hard-failure block: *"No module emits it, no lint rule +requires it, and nothing parses it."* **Two of those three were false against +the tree**, and the second one mattered enough to have wasted the task. + +| #316 said | Measured | +|---|---| +| *"No module emits it"* | `packages/core/src/guards/risk-mandate.ts:48-56` emitted one — a private `hardFailure()`, the fifth copy in the tree and the only one in TypeScript | +| *"no lint rule requires it"* | `test/skill-lint.test.ts:1369` **required** the block in every real skill body, and `:1442-1459` asserted `- Expected:` and `- Found: none` inside it | +| *"seven hand-copies"* | **Ten copies exist; five are in scope.** Four prose bodies + `risk-mandate.ts`. Out of scope: four illustrative copies in design docs and one **immutable** copy in `docs/evidence/19-*.md` | + +**The consequence was not cosmetic.** #316's first Done-when bullet proposed +reconciling the four prose bodies to *cite* the block rather than restate it. +`skill-lint.test.ts:1362` loops the whole real corpus, so that change turns all +four red at once — and those assertions encode **CLAIM-41.5 and case 12 of +`docs/test-plans/41-plan.md`**, a shipped plan from a **closed Story** with +committed evidence at `docs/evidence/44-*.md:19`. Weakening them would have +retro-invalidated a discharged claim. + +The bullet's *second* branch was taken instead — the bodies keep restating the +block — and ruling **G-a** is the recorded reason. **The issue body was +corrected before work started**, not after. + +> **A premise stated in an issue body is not a measurement, and this one had +> been read by everybody who queued the task.** The only thing that caught it +> was resolving each clause against the tree before believing it. #304's class, +> at the level of the work order rather than the citation. + +## What shipped + +**`packages/core/src/guards/hard-failure.ts`** — the spec, once. +`HARD_FAILURE_SUBJECT_KINDS`, `HARD_FAILURE_ACTION_PREFIX`, +`HARD_FAILURE_HEADLINE_PREFIX`, `HARD_FAILURE_FIELD_ORDER`, and +`renderHardFailure`. Pure: no fs, no process, no clock. + +**One spec, two enforcement directions.** The renderer produces conformant +blocks; skill-lint's new **`hard-failure-block`** rule validates prose against +**the same exported constants**. That is the part worth copying: a renderer and +a validator that agree only by coincidence are two contracts, which is the +shape this task existed to remove. + +**`risk-mandate.ts`'s private copy is deleted and rewired**, and +`packages/core/test/risk-mandate.test.ts` **was not touched**. It pins the +output string verbatim, line for line, so leaving it alone is what makes it a +regression guard rather than a co-conspirator. It stayed green throughout; +mutation **M12** confirms it is load-bearing. + +**`checkEvidenceMandate`** (`evidence-mandate.ts`) is the reader the doctrine at +`references/evidence-artifacts.md:140-144` never had — *"a `status:resolved` +with no artifact behind it is a claim, not a result"* — and the refusal says so +in its own Action line. + +## The ruling that made the task shippable at all + +**Build target 4's claim-to-artifact resolver cannot be built as the Design +specifies**, and case 8 depends on it. Measured over the 54 artifacts: + +- **21 carry no `| Claims |` row at all** +- the other **33** use at least **five mutually incompatible notations** — + `…`, `–` with an abbreviated endpoint, `..`, spaced `–`, and partial-coverage + qualifiers — plus the retired `ISC-*` namespace +- **#287 has two artifacts**, so the mapping is not 1:1 +- **`docs/evidence/` is immutable** (NEVER-194.6), so **the gaps can never be + backfilled and the notations can never be normalised** + +Which leaves exactly one sound direction: + +> **SOUND** — a claim id appearing in **zero** artifacts is certainly uncovered. +> **UNSOUND** — a claim id appearing somewhere is therefore covered. + +**`CLAIM-47.6` refuses, and a refusal needs only the sound direction.** So the +refusal ships as a pure function over a **supplied** resolution and **exports no +resolver**. Raised as **#341**; case 8 recorded **partially discharged** under +**G-b**. + +**The carry:** build target 4 owns the positive direction, and #341 owns the +ruling on whether it is reconstructed from the Design/plan side instead of from +the artifacts. + +**The seam is asserted statically**, because behaviourally it is invisible: a +future edit making `evidence-mandate.ts` read the corpus itself would return +identical verdicts for every input the suite supplies, while silently adopting +the unsound direction. The test reads the module's own import list and requires +it to mention no `node:fs`, no `node:path`, no `readdirSync`, no `readFileSync` +and no `Bun.file`, with the extraction asserted non-empty first so a failed read +cannot pass vacuously. **#322's M11 is the precedent** — a behaviourally +equivalent mutation can only be killed statically. + +## The mutation round: 12 run, 12 killed + +| | Mutation | Result | +|---|---|---| +| M1 | `Story` loses its `#` sigil | KILLED | +| M2 | blank fields accepted | KILLED — 8 fail | +| M3 | newline injection accepted | KILLED | +| M4 | vocabulary order changed | KILLED | +| **M5** | **Action invariant prefix no longer compared** | **SURVIVED, then killed — see below** | +| M6 | subject vocabulary no longer checked | KILLED | +| M7 | only the FIRST block validated | KILLED | +| M8 | rule reads the stripped body, not the raw one | KILLED — 36 fail **and** `skill-lint` exit 1 | +| M9 | blank `Expected` accepted | KILLED | +| M10 | constant refusal: every task blocked | KILLED | +| M11 | refusal stops recording absence as `none` | KILLED | +| M12 | rewired `risk-mandate` drifts from the pinned string | KILLED | + +### M5 is the whole return on the round + +M5 deleted the Action line's invariant check outright — +`startsWith(HARD_FAILURE_ACTION_PREFIX)` became `startsWith("- Action:")` — and +**the suite stayed green.** + +The negative fixture written specifically to pin that check *did* fail the +mutant, but **for the wrong reason**. Its Action line, `- Action: Fix and +re-run.`, is **shorter than the 36-character prefix**, so the surviving +`.slice(PREFIX.length)` ran off the end of the string and yielded `""` — which +the *blank-remainder* branch rejected regardless of whether the prefix was ever +compared. + +> **A length accident was standing in for a content comparison, and every +> assertion in the test still read as though the comparison were pinned.** + +The added fixture miscases one word (`Pipeline cannot CONTINUE.`) and keeps the +line long enough that the slice yields a non-blank remainder either way, so the +**only** thing that can reject it is a real comparison against the invariant. +The test now also asserts the line is longer than the prefix and its remainder +non-blank, **so the accident cannot come back silently.** + +This is the class recorded in `docs/evidence/33-*.md` and re-hit in #315: **two +assertions that differ only in a way the code never reads are one assertion.** +Third occurrence in this lineage, and the second time it was found by a +mutation rather than by review. + +## What this task does not close + +- **Case 8's positive direction** — carried to build target 4 under **G-b**, + blocked on **#341**. It is not claimed here. +- **`CLAIM-47.6` is anchored, not fully discharged.** The refusal is proven; the + resolution that feeds it has no production caller yet. `task-verify` (#50) is + the first, and it must supply the resolution rather than compute it here. +- **#330 is still unpaid.** No `gh` argv this repository builds has ever been + executed, and this task added none. +- **The four prose bodies were deliberately not reconciled** — G-a, and the + reason is recorded in `hard-failure.ts`'s header where the next person to + "finish the job" will read it. + +## Verification chain, as run at `b0616b9` + +``` +bun install +bun test # 963 tests / 949 pass / 0 fail / 14 skip / 3324 expect() +bun run lint # 0 violations across 98 files +bun run typecheck # clean +bun run skill-lint # 4 SKILL.md + 12 contract files, 0 errors, 0 warnings +bun run claim-lint # 115 files, 0 errors, 1 warning +IAI_PURITY_CHILD=1 bun test packages/core/test/purity.test.ts # 9 pass +IAI_BINDING_SWEEP_CHILD=1 bun test packages/core/test/binding-purity.test.ts # 5 pass +bash scripts/verify-required-checks.sh # PASS, six contexts +bash scripts/verify-workflow-hygiene.sh # PASS +``` + +**The 1 claim-lint warning is expected and must stay a warning** — +`docs/design/stories/9.md` predates the metadata convention (ruling G1b of #41). + +**There is no `seam` job on `story/47`**, so this PR reports **7 checks, not 8**.