From 2c1b5ffd1ba56ff7736fd2c703c4ff238cf8aa3c Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Sun, 25 Oct 2026 16:42:57 +0000 Subject: [PATCH 01/10] feat(ci): make the PR report say what each finding means and what to do The CI report now says, under its counts, whether there is anything to fix, and that findings do not fail the check and can come from files the pull request did not change. Each section with a finding says what it means and how to fix it, in plain words. The spec gap section names the adoption baseline in plain words, and the two least clear spec problems (an invariant bullet with no name, a cited commit no branch has) now say how to fix them. Co-Authored-By: Claude Opus 5.5 Generated-By: PostHog Desktop Task-Id: 4189fcb8-6fd5-428d-8c5f-4314d28e4dac --- .../runs/pr-report-words.e2255a45.jsonl | 3 ++ src/lifecycle/Lifecycle.spec.md | 6 +++ src/lifecycle/ci-report.ts | 16 ++++--- src/lifecycle/ci.test.ts | 45 +++++++++++++++++-- src/spec/grammar.ts | 2 +- src/spec/practice.test.ts | 2 +- src/spec/practices.ts | 2 +- 7 files changed, 63 insertions(+), 13 deletions(-) create mode 100644 .coherence/runs/pr-report-words.e2255a45.jsonl diff --git a/.coherence/runs/pr-report-words.e2255a45.jsonl b/.coherence/runs/pr-report-words.e2255a45.jsonl new file mode 100644 index 00000000..9574cf9b --- /dev/null +++ b/.coherence/runs/pr-report-words.e2255a45.jsonl @@ -0,0 +1,3 @@ +{"at":"2026-10-25T16:41:59.995Z","session":"pr-report-words","agent":"claude","binding":"none: this session owns no active work order","commit":"b91e4ad","dirty":true,"instrument":{"language":"typescript","server":"none"},"latency":30585,"load":{"average":2.44,"cores":8},"batch":{"ms":30056},"invariants":[{"component":"src/lifecycle","name":"the CI report says what each finding means and what to do","form":"totality oracle","verdict":"fail","refutation":"missing","bypasses":[],"testReferences":0,"files":[],"latency":30056,"reason":"2 of 5 tests under \"ci report says what each finding means and what to do\" failed: ci report says what each finding means and what to do a spec gap and one the baseline holds; ci report says what each finding means and what to do","testMs":64,"mode":"batched","state":"requirement","enforces":["ci report says what each finding means and what to do"]}]} +{"kind":"refutation","at":"2026-10-25T16:42:00.472Z","session":"pr-report-words","agent":"claude","component":"src/lifecycle","name":"the CI report says what each finding means and what to do","form":"totality oracle","broke":"put back the old spec gap words in ci-report.ts","verdict":"fail","reason":"2 of 5 tests under \"ci report says what each finding means and what to do\" failed: ci report says what each finding means and what to do a spec gap and one the baseline holds; ci report says what each finding means and what to do","commit":"b91e4ad","dirty":true} +{"at":"2026-10-25T16:42:36.857Z","session":"pr-report-words","agent":"claude","binding":"none: this session owns no active work order","commit":"b91e4ad","dirty":true,"instrument":{"language":"typescript","server":"none"},"latency":31960,"load":{"average":2.12,"cores":8},"batch":{"ms":31440},"invariants":[{"component":"src/lifecycle","name":"the CI report says what each finding means and what to do","form":"totality oracle","verdict":"pass","refutation":"witnessed","bypasses":[],"testReferences":0,"files":[],"latency":31440,"reason":"5 tests under \"ci report says what each finding means and what to do\" passed in one invocation of node --disable-warning=ExperimentalWarning --import=./src/enforcement/leak-guard.ts --test --test-concurrency=1 --test-reporter=./src/enforcement/node-test-reporter.ts --test-reporter-destination= --test-name-pattern=<1 names> src/**/*.test.ts src/*.e2e.ts","testMs":76,"mode":"batched","state":"invariant","enforces":["ci report says what each finding means and what to do"]}]} diff --git a/src/lifecycle/Lifecycle.spec.md b/src/lifecycle/Lifecycle.spec.md index 1227ac87..68def7ce 100644 --- a/src/lifecycle/Lifecycle.spec.md +++ b/src/lifecycle/Lifecycle.spec.md @@ -1283,6 +1283,12 @@ The gyroscope delivered through harness events: orient at start, regulate at sto because: in the comment mode, findings are reported and never fail the job, while a run that never happened is never mistaken for a clean one; a report read from the latest verdicts instead of the session's run would show a pass from another branch refuted: buildReport read every run in each project instead of the CI session's, so another session's later pass hid the failing verdict -> "ci report carries each project's problems, chokepoint verdicts, spec gaps and guard failures as Markdown, and fails only when Coherence did not finish" went red in ci.test.ts on its own assertion; restored byte for byte, green batched and alone (2026-10-09) kinds: none +- the CI report says what each finding means and what to do: Under its counts the report says in plain words whether there is anything to fix, and that findings do not fail the check and can come from files the pull request did not change; each section with a finding says what that kind of finding means and how to fix it, and a spec gap section says how many more the adoption baseline holds and why they are not listed. + over: a report with nothing to fix, a spec problem, a failing chokepoint, and an open spec gap beside one the adoption baseline holds + via: ci report says what each finding means and what to do + because: an adopter read a report of spec problems, spec gaps, traced controls and an adoption baseline and could not tell what any of it meant, whether the pull request caused it, or what to change; a report a reader cannot act on is noise on every pull request + refuted: put back the old spec gap words in ci-report.ts, "no traced control" for what the gap means -> "ci report says what each finding means and what to do" went red in ci.test.ts on the spec gap case; restored byte for byte, green (2026-10-25) + kinds: none - a closure mirror is named at the edit that breaks it: At PostToolUse, a write to a file in the value-import closure a declared mirror names is read again, and each file the closure now reaches that the list does not name is added to the edit's context as advice, never a block, naming the file that imports it and the list's file and line; a type-only import adds nothing, and the file the closure grew by is watched from then on. over: a fixture of the b37627e shape: a list of the page's modules, the closure of page.ts, a type-only import added to a closure file, then a value import of a file the list omits via: an edit that gives a closure file a value import its mirror omits names the file and the list's line, as advice diff --git a/src/lifecycle/ci-report.ts b/src/lifecycle/ci-report.ts index 0e81f3be..81e52177 100644 --- a/src/lifecycle/ci-report.ts +++ b/src/lifecycle/ci-report.ts @@ -97,10 +97,10 @@ function projectSection(p: ProjectReport, session: string, heading: boolean): { const where = p.name === "." ? "" : ` in ${p.name}`; const c = p.model.counts; if (heading) lines.push(`### ${cell(p.name)}`, ""); - lines.push(`${plural(c.components, "component")}, ${plural(c.bullets, "bullet")} (${plural(c.invariants, "invariant")}, ${plural(c.requirements, "requirement")}), ${plural(c.problems, "problem")}`, ""); + lines.push(`Spec size: ${plural(c.components, "component")}, ${plural(c.bullets, "bullet")} (${plural(c.invariants, "invariant")}, ${plural(c.requirements, "requirement")})`, ""); if (p.model.problems.length > 0) { - lines.push(`**Spec problems** (spec --check${where})`, ""); + lines.push(`**Spec problems** (spec --check${where}): lines Coherence cannot read as written. Fix each one, then run \`coherence spec --check\` until it finds 0 problems.`, ""); for (const problem of p.model.problems) lines.push(`- \`${problem.file}:${problem.line}\` ${cell(problem.message)}`); lines.push(""); } @@ -115,6 +115,7 @@ function projectSection(p: ProjectReport, session: string, heading: boolean): { const fails = entries.filter((e) => e.verdict === "fail").length; const notRun = entries.filter((e) => e.verdict === "not run"); lines.push(`**Chokepoints:** ${entries.length - fails - notRun.length} pass, ${fails} fail, ${notRun.length} not run`, ""); + if (fails > 0) lines.push("A chokepoint fails when code reaches what it protects without going through it. Send each bypass below through the chokepoint, or fix the spec if the rule is wrong.", ""); for (const e of [...entries].sort((a, b) => order(a) - order(b))) { lines.push(`- ${MARK[e.verdict]} \`${bullet(e)}\` ${e.grade ?? e.verdict}: ${cell(e.reason)}`); for (const b of e.bypasses.slice(0, BYPASSES)) lines.push(` - bypass \`${b.file}:${b.line}\` in ${cell(b.symbol)}`); @@ -129,15 +130,17 @@ function projectSection(p: ProjectReport, session: string, heading: boolean): { } else { const open = p.gaps.gaps.filter((g) => !p.baselined.has(`${g.component}\u0000${g.name}`)); const held = p.gaps.gaps.length - open.length; - lines.push(`**Spec gaps:** ${plural(open.length, "entrance")} with outside or unknown trust and no traced control${held === 0 ? "" : `, and ${held} more held by the adoption baseline`}`, ""); - for (const g of open) lines.push(`- \`${g.specPath}\` entrance ${cell(g.name)} (trust ${cell(g.trust.join(", ") || "unknown")}): ${cell(g.route.stops.join(" -> "))}`); - if (open.length > 0) lines.push("", "Close one with `guard: `, an invariant that names it on its `entrances:` line, or `control: none — `; `scaffold control` proposes it."); + const takes = open.length === 1 ? "takes" : "take"; + const was = held === 1 ? "was" : "were"; + lines.push(`**Spec gaps:** ${plural(open.length, "entrance")} ${takes} input from outside this code (trust outside or unknown), and the spec names no check on that input.${held === 0 ? "" : ` ${held} more ${was} already open when Coherence was adopted, so ${held === 1 ? "it is" : "they are"} not listed.`}`, ""); + for (const g of open) lines.push(`- \`${g.specPath}\` entrance **${cell(g.name)}** (trust ${cell(g.trust.join(", ") || "unknown")}), route: ${cell(g.route.stops.join(" -> "))}`); + if (open.length > 0) lines.push("", "To close a gap, add one of these to its spec: `guard: ` when a verified chokepoint checks the input; the entrance's name on the `entrances:` line of an invariant that covers it; or `control: none — ` when it needs no check. `coherence scaffold control \"\"` proposes the line."); lines.push(""); } const failures = p.model.defects?.guardFailures ?? []; if (failures.length > 0) { - lines.push(`**Guard failures:** ${failures.length}`, ""); + lines.push(`**Guard failures:** ${failures.length}`, "", "A defect came back in a class that a guard was meant to stop. Check why each guard below let it through.", ""); for (const g of failures) lines.push(`- ${g.id} in class ${cell(g.class)}, which ${cell(g.guard)} has guarded since ${g.resolution} closed ${g.guardedBy}`); lines.push(""); } @@ -167,6 +170,7 @@ export function renderReport(projects: readonly ProjectReport[], session: string const guards = projects.reduce((n, p) => n + (p.model.defects?.guardFailures.length ?? 0), 0); const head = [REPORT_MARKER, "## Coherence", ""]; head.push(`**${plural(problems, "spec problem")} · ${plural(fails, "failing chokepoint")} · ${plural(gaps, "spec gap")} · ${plural(guards, "guard failure")}**${projects.length > 1 ? ` across ${plural(projects.length, "project")}` : ""}`, ""); + head.push(problems + fails + gaps + guards === 0 ? "Nothing to fix." : "These findings do not fail this check. Some can come from files this pull request did not change.", ""); if (unfinished.length > 0) { head.push("> [!WARNING]", "> Coherence did not finish, so this report is incomplete and the check fails:"); for (const line of unfinished) head.push(`> - ${cell(line)}`); diff --git a/src/lifecycle/ci.test.ts b/src/lifecycle/ci.test.ts index 4ed85392..41228467 100644 --- a/src/lifecycle/ci.test.ts +++ b/src/lifecycle/ci.test.ts @@ -11,10 +11,11 @@ import { sessionFiles } from "../journal/shard-fixture.ts"; import { existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, realpathSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; -import { after, test } from "node:test"; +import { after, describe, test } from "node:test"; import { appendRun, type RunEntry, type RunRecord } from "../enforcement/record.ts"; import { stopWarmServers } from "../enforcement/server-fixture.ts"; import type { Io } from "../journal/cli.ts"; +import type { Gap } from "../readings/scope/gaps.ts"; import { loadSpecModel } from "../spec/model.ts"; import { buildReport, ciReportCommand, renderReport, type GapReader } from "./ci-report.ts"; import { cli } from "./cli-fixture.ts"; @@ -280,9 +281,9 @@ test("ci report carries each project's problems, chokepoint verdicts, spec gaps assert.ok(md.startsWith(`${REPORT_MARKER}\n## Coherence\n`), "the marker the workflow finds its comment by comes first"); assert.match(md, /\*\*1 spec problem · 1 failing chokepoint · 1 spec gap · 0 guard failures\*\* across 2 projects/); assert.match(md, /### products\/a[\s\S]*✓ `a door` reference-choked|### products\/a[\s\S]*✓ `a door` pass/); - assert.match(md, /### products\/b[\s\S]*\*\*Spec problems\*\* \(spec --check in products\/b\)\n\n- `Extra\.spec\.md:1` a folder holds one spec/); + assert.match(md, /### products\/b[\s\S]*\*\*Spec problems\*\* \(spec --check in products\/b\): [^\n]+\n\n- `Extra\.spec\.md:1` a folder holds one spec/); assert.match(md, /✕ `b door` broken: 1 reference to ROWS outside door: src\/leak\.ts:3 in leak\n {2}- bypass `src\/leak\.ts:3` in leak/); - assert.match(md, /\*\*Spec gaps:\*\* 1 entrance with outside or unknown trust and no traced control\n\n- `A\.spec\.md` entrance webhook \(trust outside\): \. -\\> store/); + assert.match(md, /\*\*Spec gaps:\*\* 1 entrance takes input from outside this code [^\n]+\n\n- `A\.spec\.md` entrance \*\*webhook\*\* \(trust outside\), route: \. -\\> store/); assert.match(md, /\*\*Spec gaps:\*\* not read: the language server never answered\./, "a gap reading that failed is said, never shown as none"); assert.deepEqual(report.unfinished, [], "findings alone leave the report finished"); assert.doesNotMatch(md, /someone-else|\[!WARNING\]/, "only the CI session's run is read"); @@ -303,10 +304,46 @@ test("ci report carries each project's problems, chokepoint verdicts, spec gaps model.defects = { recorded: 2, closed: 1, guarded: 1, decided: 0, unguarded: [], undeclared: [], guardFailures: [{ id: "df-2", class: "bypass", guard: "products/a/a door", guardedBy: "df-1", resolution: "rs-1" }] }; const guarded = renderReport([{ name: ".", model, runs: [record("ci-7", [{ name: "a door" }])], gaps: { gaps: [] }, baselined: new Set() }], "ci-7", undefined); assert.match(guarded.markdown, /0 spec gaps · 1 guard failure\*\*/); - assert.match(guarded.markdown, /\*\*Guard failures:\*\* 1\n\n- df-2 in class bypass, which products\/a\/a door has guarded since rs-1 closed df-1/); + assert.match(guarded.markdown, /\*\*Guard failures:\*\* 1\n\n[^\n]+\n\n- df-2 in class bypass, which products\/a\/a door has guarded since rs-1 closed df-1/); assert.doesNotMatch(guarded.markdown, /^### /m, "one project at the root needs no heading"); }); +const UNNAMED = "# a\n\nThe a product.\n\n## invariants\n- ROWS leave only through door.\n"; +const WEBHOOK: Gap = { component: ".", name: "webhook", specPath: "A.spec.md", handler: "hook in src/hook.ts", file: "src/hook.ts", trust: ["outside"], route: { id: "r", first: "webhook", entrances: 1, stops: [".", "store"] } }; + +describe("ci report says what each finding means and what to do", () => { + for (const { finding, spec, runs, gaps, baselined, says } of [ + { finding: "nothing", spec: SPEC("a"), runs: [{ name: "a door" }], gaps: [], baselined: [], says: [/\*\*\n\nNothing to fix\.\n/] }, + { + finding: "a spec problem", + spec: UNNAMED, + runs: [], + gaps: [], + baselined: [], + says: [/\n\nThese findings do not fail this check\. Some can come from files this pull request did not change\.\n/, /\*\*Spec problems\*\* \(spec --check\): lines Coherence cannot read as written\. Fix each one, then run `coherence spec --check` until it finds 0 problems\.\n\n- `A\.spec\.md:6` an invariant bullet starts with a short name and a colon, as in "- \\: \\"\n/], + }, + { finding: "a failing chokepoint", spec: SPEC("a"), runs: [{ name: "a door", verdict: "fail", grade: "broken" }], gaps: [], baselined: [], says: [/\*\*Chokepoints:\*\* 0 pass, 1 fail, 0 not run\n\nA chokepoint fails when code reaches what it protects without going through it\. Send each bypass below through the chokepoint, or fix the spec if the rule is wrong\.\n/] }, + { + finding: "a spec gap and one the baseline holds", + spec: SPEC("a"), + runs: [{ name: "a door" }], + gaps: [WEBHOOK, { ...WEBHOOK, name: "old" }], + baselined: ["old"], + says: [ + /\*\*Spec gaps:\*\* 1 entrance takes input from outside this code \(trust outside or unknown\), and the spec names no check on that input\. 1 more was already open when Coherence was adopted, so it is not listed\.\n\n- `A\.spec\.md` entrance \*\*webhook\*\* \(trust outside\), route: \. -\\> store\n/, + /\nTo close a gap, add one of these to its spec: `guard: ` when a verified chokepoint checks the input; the entrance's name on the `entrances:` line of an invariant that covers it; or `control: none — ` when it needs no check\. `coherence scaffold control ""` proposes the line\.\n/, + ], + }, + ] satisfies { finding: string; spec: string; runs: Partial[]; gaps: Gap[]; baselined: string[]; says: RegExp[] }[]) { + test(finding, () => { + const root = repository({ "A.spec.md": spec }); + const model = loadSpecModel(root); + const report = renderReport([{ name: ".", model, runs: [record("ci-7", runs)], gaps: { gaps }, baselined: new Set(baselined.map((n) => `.\u0000${n}`)) }], "ci-7", undefined); + for (const said of says) assert.match(report.markdown, said); + }); + } +}); + test("run at a registry's top runs each listed project from its own folder, its records in each", async () => { const top = repository({ "coherence.config.json": JSON.stringify({ projects: ["products/a", "products/b"], language: "typescript" }), diff --git a/src/spec/grammar.ts b/src/spec/grammar.ts index 77cf1f2b..0188cbb3 100644 --- a/src/spec/grammar.ts +++ b/src/spec/grammar.ts @@ -604,7 +604,7 @@ export function parseSpec(text: string, file: string, options: ParseOptions = {} const head = bullet[1]!; const colon = head.indexOf(":"); if (colon <= 0) { - problem(line, "an invariant bullet starts with : "); + problem(line, "an invariant bullet starts with a short name and a colon, as in \"- : \""); current = { name: head.trim(), sentence: "", line, fields: [] }; return; } diff --git a/src/spec/practice.test.ts b/src/spec/practice.test.ts index 88acfb71..00e9bc20 100644 --- a/src/spec/practice.test.ts +++ b/src/spec/practice.test.ts @@ -199,7 +199,7 @@ describe("a cited commit is no commit only where git can tell", () => { git(tmpdir(), "clone", "-q", "--depth", "1", `file://${origin}`, root); } const messages = loadSpecModel(root, { runs: false }).problems.map((p) => p.message); - assert.equal(messages.some((m) => /which is no commit in this repository/.test(m)), problem, messages.join("\n")); + assert.equal(messages.some((m) => /^practice "[^"]+" cites \S+, but no branch in this repository has that commit\. If its pull request was squash-merged, cite the merge commit instead\.$/.test(m)), problem, messages.join("\n")); }); } }); diff --git a/src/spec/practices.ts b/src/spec/practices.ts index 80dcacae..be67ef35 100644 --- a/src/spec/practices.ts +++ b/src/spec/practices.ts @@ -141,7 +141,7 @@ export function practiceProblems(checks: PracticeChecks): Problem[] { if (isRecord(c)) { if (!known.has(c) && !elsewhere.has(c)) problems.push({ file: p.file, line, message: `practice ${p.name} cites ${c}, which no journal or work record has, here or on another branch` }); } else if (absent !== undefined && absent.has(c)) { - problems.push({ file: p.file, line, message: `practice ${p.name} cites ${c}, which is no commit in this repository` }); + problems.push({ file: p.file, line, message: `practice "${p.name}" cites ${c}, but no branch in this repository has that commit. If its pull request was squash-merged, cite the merge commit instead.` }); } } From 60ed988622f31f2dff434d9dbada1a1a31a5f537 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Sun, 25 Oct 2026 16:56:34 +0000 Subject: [PATCH 02/10] feat(ci): mark the report's findings that are in files the pull request changed ci report takes --base . With it, the report says how many findings are in files changed since that commit and marks each one, or says why it could not tell. The workflow ci install writes passes the pull request's base sha. Co-Authored-By: Claude Opus 5.5 Generated-By: PostHog Desktop Task-Id: 4189fcb8-6fd5-428d-8c5f-4314d28e4dac --- .../runs/pr-report-words.0f6f8353.jsonl | 3 + README.md | 2 +- src/cli.ts | 2 +- src/lifecycle/Lifecycle.spec.md | 5 +- src/lifecycle/ci-report.ts | 59 ++++++++++++++++--- src/lifecycle/ci.test.ts | 37 ++++++++++-- src/lifecycle/ci.ts | 6 +- 7 files changed, 94 insertions(+), 20 deletions(-) create mode 100644 .coherence/runs/pr-report-words.0f6f8353.jsonl diff --git a/.coherence/runs/pr-report-words.0f6f8353.jsonl b/.coherence/runs/pr-report-words.0f6f8353.jsonl new file mode 100644 index 00000000..e8abff38 --- /dev/null +++ b/.coherence/runs/pr-report-words.0f6f8353.jsonl @@ -0,0 +1,3 @@ +{"at":"2026-10-25T16:54:51.679Z","session":"pr-report-words","agent":"claude","binding":"none: this session owns no active work order","commit":"2c1b5ff","dirty":true,"instrument":{"language":"typescript","server":"none"},"latency":31332,"load":{"average":0.59,"cores":8},"batch":{"ms":30836},"invariants":[{"component":"src/lifecycle","name":"the CI report says what each finding means and what to do","form":"totality oracle","verdict":"fail","refutation":"witnessed","bypasses":[],"testReferences":0,"files":[],"latency":30836,"reason":"4 of 10 tests under \"ci report says what each finding means and what to do\" failed: ci report says what each finding means and what to do a spec problem in a file the pull request changed; ci report says what each finding means and what to do a spec gap whose handler the pull request changed; ci report says what each finding means and what to do ci report --base reads the files changed since that commit","testMs":195,"mode":"batched","state":"structural defect","enforces":["ci report says what each finding means and what to do"]}]} +{"kind":"refutation","at":"2026-10-25T16:54:52.163Z","session":"pr-report-words","agent":"claude","component":"src/lifecycle","name":"the CI report says what each finding means and what to do","form":"totality oracle","broke":"made the changed-file mark in ci-report.ts never mark a finding","verdict":"fail","reason":"4 of 10 tests under \"ci report says what each finding means and what to do\" failed: ci report says what each finding means and what to do a spec problem in a file the pull request changed; ci report says what each finding means and what to do a spec gap whose handler the pull request changed; ci report says what each finding means and what to do ci report --base reads the files changed since that commit","commit":"2c1b5ff","dirty":true} +{"at":"2026-10-25T16:55:23.787Z","session":"pr-report-words","agent":"claude","binding":"none: this session owns no active work order","commit":"2c1b5ff","dirty":true,"instrument":{"language":"typescript","server":"none"},"latency":30980,"load":{"average":0.93,"cores":8},"batch":{"ms":30453},"invariants":[{"component":"src/lifecycle","name":"the CI report says what each finding means and what to do","form":"totality oracle","verdict":"pass","refutation":"witnessed","bypasses":[],"testReferences":0,"files":[],"latency":30453,"reason":"10 tests under \"ci report says what each finding means and what to do\" passed in one invocation of node --disable-warning=ExperimentalWarning --import=./src/enforcement/leak-guard.ts --test --test-concurrency=1 --test-reporter=./src/enforcement/node-test-reporter.ts --test-reporter-destination= --test-name-pattern=<1 names> src/**/*.test.ts src/*.e2e.ts","testMs":197,"mode":"batched","state":"invariant","enforces":["ci report says what each finding means and what to do"]}]} diff --git a/README.md b/README.md index e8a7d390..5e0e1ac8 100644 --- a/README.md +++ b/README.md @@ -531,7 +531,7 @@ node src/cli.ts hooks uninstall --host claude|codex # removes only Coherence's node src/cli.ts hooks --check --host claude|codex # exit 1 naming each event missing, stale, or extra against what install would write node src/cli.ts hooks status # the wiring per agent host, and what each event delivers for this project node src/cli.ts ci install --host github --mode fail|comment [--write] # the workflow that runs Coherence on every pull request; never over a file already there -node src/cli.ts ci report --session # the pull request report as Markdown, from that session's run +node src/cli.ts ci report --session [--base ] # the pull request report as Markdown, from that session's run; --base marks findings in files changed since that commit npm run spec:check # the spec check alone npm test # typecheck, tests, vocabulary check, spec check; any one failing fails the tree ``` diff --git a/src/cli.ts b/src/cli.ts index 838f40d1..aa8c21a8 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -25,7 +25,7 @@ * coherence hooks --check --host exit 1 with each event's drift from what install would write * coherence hooks status the wiring per agent host and what each event delivers for this project * coherence ci install --host github --mode fail|comment [--write] the workflow that runs Coherence on every pull request (ci.ts) - * coherence ci report --session the pull request report as Markdown (ci-report.ts) + * coherence ci report --session [--base ] the pull request report as Markdown (ci-report.ts) * coherence telemetry on | off | status | show | reset-id opt-in fleet telemetry (telemetry.ts) * coherence version (--version, -v) the running version, where it is, and the newest published one (version.ts) * coherence help (--help, -h) this usage, on stdout diff --git a/src/lifecycle/Lifecycle.spec.md b/src/lifecycle/Lifecycle.spec.md index 68def7ce..ad6f2498 100644 --- a/src/lifecycle/Lifecycle.spec.md +++ b/src/lifecycle/Lifecycle.spec.md @@ -1283,11 +1283,12 @@ The gyroscope delivered through harness events: orient at start, regulate at sto because: in the comment mode, findings are reported and never fail the job, while a run that never happened is never mistaken for a clean one; a report read from the latest verdicts instead of the session's run would show a pass from another branch refuted: buildReport read every run in each project instead of the CI session's, so another session's later pass hid the failing verdict -> "ci report carries each project's problems, chokepoint verdicts, spec gaps and guard failures as Markdown, and fails only when Coherence did not finish" went red in ci.test.ts on its own assertion; restored byte for byte, green batched and alone (2026-10-09) kinds: none -- the CI report says what each finding means and what to do: Under its counts the report says in plain words whether there is anything to fix, and that findings do not fail the check and can come from files the pull request did not change; each section with a finding says what that kind of finding means and how to fix it, and a spec gap section says how many more the adoption baseline holds and why they are not listed. - over: a report with nothing to fix, a spec problem, a failing chokepoint, and an open spec gap beside one the adoption baseline holds +- the CI report says what each finding means and what to do: Under its counts the report says in plain words whether there is anything to fix, and that findings do not fail the check and can come from files the pull request did not change; each section with a finding says what that kind of finding means and how to fix it, and a spec gap section says how many more the adoption baseline holds and why they are not listed. Given --base, it says how many findings are in files changed since that commit, or why it could not tell, and marks each one; the workflow passes the pull request's base. + over: a report with nothing to fix, a spec problem, a failing chokepoint, an open spec gap beside one the adoption baseline holds, a spec problem and a gap in files the pull request changed, findings in none it changed, a base git cannot diff against, and --base read from a real repository via: ci report says what each finding means and what to do because: an adopter read a report of spec problems, spec gaps, traced controls and an adoption baseline and could not tell what any of it meant, whether the pull request caused it, or what to change; a report a reader cannot act on is noise on every pull request refuted: put back the old spec gap words in ci-report.ts, "no traced control" for what the gap means -> "ci report says what each finding means and what to do" went red in ci.test.ts on the spec gap case; restored byte for byte, green (2026-10-25) + refuted: made the changed-file mark in ci-report.ts never mark a finding -> "ci report says what each finding means and what to do" went red in ci.test.ts, 4 of 10 cases; restored byte for byte, green (2026-10-25) kinds: none - a closure mirror is named at the edit that breaks it: At PostToolUse, a write to a file in the value-import closure a declared mirror names is read again, and each file the closure now reaches that the list does not name is added to the edit's context as advice, never a block, naming the file that imports it and the list's file and line; a type-only import adds nothing, and the file the closure grew by is watched from then on. over: a fixture of the b37627e shape: a list of the page's modules, the closure of page.ts, a type-only import added to a closure file, then a value import of a file the list omits diff --git a/src/lifecycle/ci-report.ts b/src/lifecycle/ci-report.ts index 81e52177..b676dd77 100644 --- a/src/lifecycle/ci-report.ts +++ b/src/lifecycle/ci-report.ts @@ -19,6 +19,7 @@ import type { Io } from "../journal/cli.ts"; import { readComponentInterfaces } from "../readings/scope/component-interfaces.ts"; import { currentGaps, gapsOf, readAndRecord, readGapBaseline, structureState, type Gap } from "../readings/scope/gaps.ts"; import { flowOf } from "../readings/scope/structure-flow.ts"; +import { changedSince } from "../observation/observed.ts"; import { loadSpecModel, type SpecModel } from "../spec/model.ts"; import { CI_USAGE, REPORT_MARKER } from "./ci.ts"; import { mayHaveGaps } from "./hook.ts"; @@ -26,6 +27,9 @@ import { mayHaveGaps } from "./hook.ts"; /** The spec gaps of one project, or why they are not known. */ export type GapsRead = { gaps: Gap[] } | { unread: string }; +/** The files a pull request changed, relative to the project, or why they are not known. */ +export type ChangedRead = { files: Set } | { unread: string }; + /** How the report learns a project's gaps; a test hands in one that needs no language server. */ export type GapReader = (root: string, model: SpecModel) => Promise; @@ -51,6 +55,8 @@ export interface ProjectReport { gaps: GapsRead; /** Gap keys the adoption baseline holds. */ baselined: Set; + /** Undefined when the report was given no base to diff against. */ + changed?: ChangedRead | undefined; } export interface Report { @@ -90,10 +96,20 @@ function bullet(e: RunEntry): string { const MARK: Record = { pass: "✓", fail: "✕", "not run": "○" }; +const CHANGED_HERE = " · **changed in this pull request**"; + + /** One project's section, and why its part did not finish. */ -function projectSection(p: ProjectReport, session: string, heading: boolean): { lines: string[]; unfinished: string[] } { +function projectSection(p: ProjectReport, session: string, heading: boolean): { lines: string[]; unfinished: string[]; touched: number } { const lines: string[] = []; const unfinished: string[] = []; + let touched = 0; + const changed = p.changed !== undefined && "files" in p.changed ? p.changed.files : new Set(); + const mark = (...files: (string | undefined)[]): string => { + if (!files.some((f) => f !== undefined && changed.has(f))) return ""; + touched++; + return CHANGED_HERE; + }; const where = p.name === "." ? "" : ` in ${p.name}`; const c = p.model.counts; if (heading) lines.push(`### ${cell(p.name)}`, ""); @@ -101,7 +117,7 @@ function projectSection(p: ProjectReport, session: string, heading: boolean): { if (p.model.problems.length > 0) { lines.push(`**Spec problems** (spec --check${where}): lines Coherence cannot read as written. Fix each one, then run \`coherence spec --check\` until it finds 0 problems.`, ""); - for (const problem of p.model.problems) lines.push(`- \`${problem.file}:${problem.line}\` ${cell(problem.message)}`); + for (const problem of p.model.problems) lines.push(`- \`${problem.file}:${problem.line}\` ${cell(problem.message)}${mark(problem.file)}`); lines.push(""); } @@ -117,7 +133,7 @@ function projectSection(p: ProjectReport, session: string, heading: boolean): { lines.push(`**Chokepoints:** ${entries.length - fails - notRun.length} pass, ${fails} fail, ${notRun.length} not run`, ""); if (fails > 0) lines.push("A chokepoint fails when code reaches what it protects without going through it. Send each bypass below through the chokepoint, or fix the spec if the rule is wrong.", ""); for (const e of [...entries].sort((a, b) => order(a) - order(b))) { - lines.push(`- ${MARK[e.verdict]} \`${bullet(e)}\` ${e.grade ?? e.verdict}: ${cell(e.reason)}`); + lines.push(`- ${MARK[e.verdict]} \`${bullet(e)}\` ${e.grade ?? e.verdict}: ${cell(e.reason)}${e.verdict === "fail" ? mark(...e.bypasses.map((b) => b.file)) : ""}`); for (const b of e.bypasses.slice(0, BYPASSES)) lines.push(` - bypass \`${b.file}:${b.line}\` in ${cell(b.symbol)}`); if (e.bypasses.length > BYPASSES) lines.push(` - and ${e.bypasses.length - BYPASSES} more; run --form chokepoint names them all`); } @@ -133,7 +149,7 @@ function projectSection(p: ProjectReport, session: string, heading: boolean): { const takes = open.length === 1 ? "takes" : "take"; const was = held === 1 ? "was" : "were"; lines.push(`**Spec gaps:** ${plural(open.length, "entrance")} ${takes} input from outside this code (trust outside or unknown), and the spec names no check on that input.${held === 0 ? "" : ` ${held} more ${was} already open when Coherence was adopted, so ${held === 1 ? "it is" : "they are"} not listed.`}`, ""); - for (const g of open) lines.push(`- \`${g.specPath}\` entrance **${cell(g.name)}** (trust ${cell(g.trust.join(", ") || "unknown")}), route: ${cell(g.route.stops.join(" -> "))}`); + for (const g of open) lines.push(`- \`${g.specPath}\` entrance **${cell(g.name)}** (trust ${cell(g.trust.join(", ") || "unknown")}), route: ${cell(g.route.stops.join(" -> "))}${mark(g.file, g.specPath)}`); if (open.length > 0) lines.push("", "To close a gap, add one of these to its spec: `guard: ` when a verified chokepoint checks the input; the entrance's name on the `entrances:` line of an invariant that covers it; or `control: none — ` when it needs no check. `coherence scaffold control \"\"` proposes the line."); lines.push(""); } @@ -144,7 +160,7 @@ function projectSection(p: ProjectReport, session: string, heading: boolean): { for (const g of failures) lines.push(`- ${g.id} in class ${cell(g.class)}, which ${cell(g.guard)} has guarded since ${g.resolution} closed ${g.guardedBy}`); lines.push(""); } - return { lines, unfinished }; + return { lines, unfinished, touched }; } /** How many bypass sites one verdict lists before it counts the rest. */ @@ -155,14 +171,31 @@ function order(e: RunEntry): number { return e.verdict === "fail" ? 0 : e.verdict === "not run" ? 1 : 2; } +/** Whether there is anything to fix, and which findings this pull request's own files hold, when the report knows them. */ +function findingsLead(projects: readonly ProjectReport[], findings: number, touched: number): string[] { + if (findings === 0) return ["Nothing to fix.", ""]; + const changed = projects.filter((p) => p.changed !== undefined); + if (changed.length === 0) return ["These findings do not fail this check. Some can come from files this pull request did not change.", ""]; + const lines = ["These findings do not fail this check.", ""]; + for (const p of changed) if ("unread" in p.changed!) lines.push(`Coherence could not tell which files this pull request changed${p.name === "." ? "" : ` in ${cell(p.name)}`}: ${cell(p.changed.unread)}.`, ""); + if (changed.some((p) => "files" in p.changed!)) { + if (touched === 0) lines.push("None of these findings are in files this pull request changed.", ""); + else if (touched === 1) lines.push("1 of these findings is in a file this pull request changed. It is marked below.", ""); + else lines.push(`${touched} of these findings are in files this pull request changed. They are marked below.`, ""); + } + return lines; +} + /** The report over the projects, with the registry's own problems when the report ran at its top. */ export function renderReport(projects: readonly ProjectReport[], session: string, registry: { problems: { file: string; message: string }[] } | undefined): Report { const unfinished: string[] = []; const sections: string[] = []; + let touched = 0; for (const p of projects) { const section = projectSection(p, session, projects.length > 1 || p.name !== "."); sections.push(...section.lines); unfinished.push(...section.unfinished); + touched += section.touched; } const problems = projects.reduce((n, p) => n + p.model.problems.length, 0) + (registry?.problems.length ?? 0); const fails = projects.reduce((n, p) => n + chokepointEntries(p.runs).filter((e) => e.verdict === "fail").length, 0); @@ -170,7 +203,7 @@ export function renderReport(projects: readonly ProjectReport[], session: string const guards = projects.reduce((n, p) => n + (p.model.defects?.guardFailures.length ?? 0), 0); const head = [REPORT_MARKER, "## Coherence", ""]; head.push(`**${plural(problems, "spec problem")} · ${plural(fails, "failing chokepoint")} · ${plural(gaps, "spec gap")} · ${plural(guards, "guard failure")}**${projects.length > 1 ? ` across ${plural(projects.length, "project")}` : ""}`, ""); - head.push(problems + fails + gaps + guards === 0 ? "Nothing to fix." : "These findings do not fail this check. Some can come from files this pull request did not change.", ""); + head.push(...findingsLead(projects, problems + fails + gaps + guards, touched)); if (unfinished.length > 0) { head.push("> [!WARNING]", "> Coherence did not finish, so this report is incomplete and the check fails:"); for (const line of unfinished) head.push(`> - ${cell(line)}`); @@ -186,7 +219,7 @@ export function renderReport(projects: readonly ProjectReport[], session: string } /** Build the report for `root`: every project a registry lists when `root` is its top, else `root` itself. */ -export async function buildReport(given: string, session: string, readGapsOf: GapReader = readGaps): Promise { +export async function buildReport(given: string, session: string, readGapsOf: GapReader = readGaps, base?: string): Promise { const root = realOr(given); const registry = registryOf(root); // At a registry's top, which is no project of its own, the report covers every listed project, as spec --check and run do there. @@ -203,18 +236,26 @@ export async function buildReport(given: string, session: string, readGapsOf: Ga runs, gaps: await readGapsOf(project, model), baselined: baseline?.entrances ?? new Set(), + changed: base === undefined ? undefined : changedRead(changedSince(project, base)), }); } return renderReport(projects, session, atTop ? { problems: registryProblems(registry.top) } : undefined); } -/** ci report --session : the Markdown on stdout; exit 1 when Coherence did not finish. */ +function changedRead(files: string[] | string): ChangedRead { + return typeof files === "string" ? { unread: files } : { files: new Set(files) }; +} + +/** ci report --session [--base ]: the Markdown on stdout; exit 1 when Coherence did not finish. */ export async function ciReportCommand(argv: string[], io: Io, readGapsOf: GapReader = readGaps): Promise { let session: string | undefined; + let base: string | undefined; for (let i = 0; i < argv.length; i++) { const arg = argv[i]!; if (arg === "--session") session = argv[++i]; else if (arg.startsWith("--session=")) session = arg.slice("--session=".length); + else if (arg === "--base") base = argv[++i]; + else if (arg.startsWith("--base=")) base = arg.slice("--base=".length); else { io.err(`ci report: unexpected ${arg}\n${CI_USAGE}`); return 64; @@ -224,7 +265,7 @@ export async function ciReportCommand(argv: string[], io: Io, readGapsOf: GapRea io.err(`ci report: --session names the run's session, as the chokepoint run recorded it\n${CI_USAGE}`); return 64; } - const report = await buildReport(io.cwd, session, readGapsOf); + const report = await buildReport(io.cwd, session, readGapsOf, base); io.out(report.markdown.trimEnd()); for (const line of report.unfinished) io.err(`ci report: ${line}`); return report.unfinished.length > 0 ? 1 : 0; diff --git a/src/lifecycle/ci.test.ts b/src/lifecycle/ci.test.ts index 41228467..3cd4c469 100644 --- a/src/lifecycle/ci.test.ts +++ b/src/lifecycle/ci.test.ts @@ -7,6 +7,7 @@ */ import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; import { sessionFiles } from "../journal/shard-fixture.ts"; import { existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, realpathSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; @@ -113,7 +114,7 @@ test("ci install writes the comment mode for pnpm: findings never fail the job, assert.equal(steps[0], "pnpm install --frozen-lockfile"); assert.equal(steps[1], "pnpm exec coherence spec --check || [ $? -eq 1 ]", "a spec problem is a finding; any other exit breaks the job"); assert.equal(steps[2], 'pnpm exec coherence run --form chokepoint --no-server --session "ci-${{ github.run_id }}" --agent ci || [ $? -eq 1 ]'); - assert.match(steps[3]!, /pnpm exec coherence ci report --session "ci-\$\{\{ github.run_id \}\}" > "\$RUNNER_TEMP\/coherence-report.md" \|\| status=\$\?/); + assert.match(steps[3]!, /pnpm exec coherence ci report --session "ci-\$\{\{ github.run_id \}\}" --base "\$\{\{ github.event.pull_request.base.sha \}\}" > "\$RUNNER_TEMP\/coherence-report.md" \|\| status=\$\?/); assert.match(steps[3]!, />> "\$GITHUB_STEP_SUMMARY"\nexit "\$status"$/, "the report reaches the job summary, then its exit stands"); assert.ok(steps[4]!.includes(`startswith("${REPORT_MARKER}")`), "the comment is found by the report's marker"); assert.match(steps[4]!, /gh api --method PATCH "repos\/\$REPO\/issues\/comments\/\$id" -F body=@"\$body"/, "an existing comment is updated in place"); @@ -312,7 +313,7 @@ const UNNAMED = "# a\n\nThe a product.\n\n## invariants\n- ROWS leave only throu const WEBHOOK: Gap = { component: ".", name: "webhook", specPath: "A.spec.md", handler: "hook in src/hook.ts", file: "src/hook.ts", trust: ["outside"], route: { id: "r", first: "webhook", entrances: 1, stops: [".", "store"] } }; describe("ci report says what each finding means and what to do", () => { - for (const { finding, spec, runs, gaps, baselined, says } of [ + for (const { finding, spec, runs, gaps, baselined, changed, says } of [ { finding: "nothing", spec: SPEC("a"), runs: [{ name: "a door" }], gaps: [], baselined: [], says: [/\*\*\n\nNothing to fix\.\n/] }, { finding: "a spec problem", @@ -334,14 +335,42 @@ describe("ci report says what each finding means and what to do", () => { /\nTo close a gap, add one of these to its spec: `guard: ` when a verified chokepoint checks the input; the entrance's name on the `entrances:` line of an invariant that covers it; or `control: none — ` when it needs no check\. `coherence scaffold control ""` proposes the line\.\n/, ], }, - ] satisfies { finding: string; spec: string; runs: Partial[]; gaps: Gap[]; baselined: string[]; says: RegExp[] }[]) { + { + finding: "a spec problem in a file the pull request changed", + spec: UNNAMED, + runs: [], + gaps: [], + baselined: [], + changed: ["A.spec.md"], + says: [/\n\n1 of these findings is in a file this pull request changed\. It is marked below\.\n/, /- `A\.spec\.md:6` [^\n]+ · \*\*changed in this pull request\*\*\n/], + }, + { finding: "a spec gap whose handler the pull request changed", spec: SPEC("a"), runs: [{ name: "a door" }], gaps: [WEBHOOK], baselined: [], changed: ["src/hook.ts"], says: [/route: \. -\\> store · \*\*changed in this pull request\*\*\n/] }, + { finding: "findings in no file the pull request changed", spec: UNNAMED, runs: [], gaps: [WEBHOOK], baselined: [], changed: ["src/other.ts"], says: [/\n\nNone of these findings are in files this pull request changed\.\n/] }, + { finding: "changed files git could not list", spec: UNNAMED, runs: [], gaps: [], baselined: [], changed: "git diff main failed: bad revision", says: [/\n\nCoherence could not tell which files this pull request changed: git diff main failed: bad revision\.\n/] }, + ] satisfies { finding: string; spec: string; runs: Partial[]; gaps: Gap[]; baselined: string[]; changed?: string[] | string; says: RegExp[] }[]) { test(finding, () => { const root = repository({ "A.spec.md": spec }); const model = loadSpecModel(root); - const report = renderReport([{ name: ".", model, runs: [record("ci-7", runs)], gaps: { gaps }, baselined: new Set(baselined.map((n) => `.\u0000${n}`)) }], "ci-7", undefined); + const report = renderReport([{ name: ".", model, runs: [record("ci-7", runs)], gaps: { gaps }, baselined: new Set(baselined.map((n) => `.\u0000${n}`)), changed: changed === undefined ? undefined : typeof changed === "string" ? { unread: changed } : { files: new Set(changed) } }], "ci-7", undefined); for (const said of says) assert.match(report.markdown, said); }); } + + test("ci report --base reads the files changed since that commit", async () => { + const root = repository({ "A.spec.md": SPEC("a") }); + const git = (...args: string[]): string => execFileSync("git", ["-c", "user.name=t", "-c", "user.email=t@t", ...args], { cwd: root, encoding: "utf8" }).trim(); + rmSync(join(root, ".git"), { recursive: true }); + git("init", "-q"); + git("add", "-A"); + git("commit", "-q", "-m", "base"); + const base = git("rev-parse", "HEAD"); + writeFileSync(join(root, "A.spec.md"), UNNAMED); + git("commit", "-q", "-am", "the change"); + appendRun(root, record("ci-7", [])); + const said = capture(root); + assert.equal(await ciReportCommand(["--session", "ci-7", "--base", base], said.io, async () => ({ gaps: [] })), 0); + assert.match(said.out.join("\n"), /1 of these findings is in a file this pull request changed[\s\S]*- `A\.spec\.md:6` [^\n]+ · \*\*changed in this pull request\*\*/); + }); }); test("run at a registry's top runs each listed project from its own folder, its records in each", async () => { diff --git a/src/lifecycle/ci.ts b/src/lifecycle/ci.ts index ad30ddde..0b2fbb6d 100644 --- a/src/lifecycle/ci.ts +++ b/src/lifecycle/ci.ts @@ -6,7 +6,7 @@ * updated in place on every push (ci-report.ts builds it). * * coherence ci install --host github --mode fail|comment [--write] - * coherence ci report --session + * coherence ci report --session [--base ] * * Install prints the workflow it would write, and with --write writes * .github/workflows/coherence.yml at the repository top; a file already @@ -47,7 +47,7 @@ export const MODE_TEXT: Record = { export const CI_USAGE = [ ` ci install --host <${CI_HOSTS.join("|")}> --mode <${CI_MODES.join("|")}> [--write] the workflow that runs Coherence on every pull request: --mode fail ${MODE_TEXT.fail}; --mode comment ${MODE_TEXT.comment}; printed, and with --write written to ${WORKFLOW_PATH.split(sep).join("/")}, never over a file already there`, - " ci report --session the pull request report as Markdown: problems, chokepoint verdicts, spec gaps and guard failures for every project; exit 1 when the session's run is missing or a chokepoint was not run", + " ci report --session [--base ] the pull request report as Markdown: problems, chokepoint verdicts, spec gaps and guard failures for every project, marking those in files changed since --base; exit 1 when the session's run is missing or a chokepoint was not run", ].join("\n"); /** The actions the workflow uses, pinned by commit; the comment names the release. */ @@ -347,7 +347,7 @@ export function workflowText(plan: CiPlan): string { ...where, " run: |", " status=0", - ` ${cli} ci report --session "ci-\${{ github.run_id }}" > "$RUNNER_TEMP/coherence-report.md" || status=$?`, + ` ${cli} ci report --session "ci-\${{ github.run_id }}" --base "\${{ github.event.pull_request.base.sha }}" > "$RUNNER_TEMP/coherence-report.md" || status=$?`, ' cat "$RUNNER_TEMP/coherence-report.md" >> "$GITHUB_STEP_SUMMARY"', ' exit "$status"', "", From 068487786247aacc627cb249fb56fd003ce939d3 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Sun, 25 Oct 2026 18:18:17 +0000 Subject: [PATCH 03/10] feat(ci): default the report's base to the pull request's base branch; release 1.10.0 Without --base, ci report on a GitHub pull request run compares against origin/$GITHUB_BASE_REF, so an adopter's existing workflow marks changed findings after a package upgrade alone. The generated workflow no longer passes --base. Bumps the version to 1.10.0 so merging releases it. Co-Authored-By: Claude Opus 5.5 Generated-By: PostHog Desktop Task-Id: 4189fcb8-6fd5-428d-8c5f-4314d28e4dac --- .coherence/runs/pr-report-words.656f4efb.jsonl | 3 +++ README.md | 2 +- mods/coherence/.claude-plugin/plugin.json | 2 +- package-lock.json | 4 ++-- package.json | 2 +- src/lifecycle/Lifecycle.spec.md | 5 +++-- src/lifecycle/ci-report.ts | 8 ++++++-- src/lifecycle/ci.test.ts | 16 +++++++++++++--- src/lifecycle/ci.ts | 4 ++-- 9 files changed, 32 insertions(+), 14 deletions(-) create mode 100644 .coherence/runs/pr-report-words.656f4efb.jsonl diff --git a/.coherence/runs/pr-report-words.656f4efb.jsonl b/.coherence/runs/pr-report-words.656f4efb.jsonl new file mode 100644 index 00000000..ee8570cc --- /dev/null +++ b/.coherence/runs/pr-report-words.656f4efb.jsonl @@ -0,0 +1,3 @@ +{"at":"2026-10-25T18:17:11.736Z","session":"pr-report-words","agent":"claude","binding":"none: this session owns no active work order","commit":"60ed988","dirty":true,"instrument":{"language":"typescript","server":"none"},"latency":35697,"load":{"average":6.43,"cores":8},"batch":{"ms":35171},"invariants":[{"component":"src/lifecycle","name":"the CI report says what each finding means and what to do","form":"totality oracle","verdict":"fail","refutation":"witnessed","bypasses":[],"testReferences":0,"files":[],"latency":35171,"reason":"2 of 10 tests under \"ci report says what each finding means and what to do\" failed: ci report says what each finding means and what to do ci report --base, or a pull request's base branch on GitHub Actions, reads the files changed since that commit; ci report says what each finding means and what to do","testMs":218,"mode":"batched","state":"structural defect","enforces":["ci report says what each finding means and what to do"]}]} +{"kind":"refutation","at":"2026-10-25T18:17:12.240Z","session":"pr-report-words","agent":"claude","component":"src/lifecycle","name":"the CI report says what each finding means and what to do","form":"totality oracle","broke":"dropped the GITHUB_BASE_REF default in ci-report.ts","verdict":"fail","reason":"2 of 10 tests under \"ci report says what each finding means and what to do\" failed: ci report says what each finding means and what to do ci report --base, or a pull request's base branch on GitHub Actions, reads the files changed since that commit; ci report says what each finding means and what to do","commit":"60ed988","dirty":true} +{"at":"2026-10-25T18:17:48.024Z","session":"pr-report-words","agent":"claude","binding":"none: this session owns no active work order","commit":"60ed988","dirty":true,"instrument":{"language":"typescript","server":"none"},"latency":35134,"load":{"average":4.33,"cores":8},"batch":{"ms":34605},"invariants":[{"component":"src/lifecycle","name":"the CI report says what each finding means and what to do","form":"totality oracle","verdict":"pass","refutation":"witnessed","bypasses":[],"testReferences":0,"files":[],"latency":34605,"reason":"10 tests under \"ci report says what each finding means and what to do\" passed in one invocation of node --disable-warning=ExperimentalWarning --import=./src/enforcement/leak-guard.ts --test --test-concurrency=1 --test-reporter=./src/enforcement/node-test-reporter.ts --test-reporter-destination= --test-name-pattern=<1 names> src/**/*.test.ts src/*.e2e.ts","testMs":242,"mode":"batched","state":"invariant","enforces":["ci report says what each finding means and what to do"]}]} diff --git a/README.md b/README.md index 5e0e1ac8..072db8ab 100644 --- a/README.md +++ b/README.md @@ -531,7 +531,7 @@ node src/cli.ts hooks uninstall --host claude|codex # removes only Coherence's node src/cli.ts hooks --check --host claude|codex # exit 1 naming each event missing, stale, or extra against what install would write node src/cli.ts hooks status # the wiring per agent host, and what each event delivers for this project node src/cli.ts ci install --host github --mode fail|comment [--write] # the workflow that runs Coherence on every pull request; never over a file already there -node src/cli.ts ci report --session [--base ] # the pull request report as Markdown, from that session's run; --base marks findings in files changed since that commit +node src/cli.ts ci report --session [--base ] # the pull request report as Markdown, from that session's run; --base marks findings in files changed since that commit (on GitHub Actions, the pull request's base branch by default) npm run spec:check # the spec check alone npm test # typecheck, tests, vocabulary check, spec check; any one failing fails the tree ``` diff --git a/mods/coherence/.claude-plugin/plugin.json b/mods/coherence/.claude-plugin/plugin.json index 10f51956..c1b76ba8 100644 --- a/mods/coherence/.claude-plugin/plugin.json +++ b/mods/coherence/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "coherence", - "version": "1.9.0", + "version": "1.10.0", "description": "Coherence's hooks through one warm process per session, with its verdicts where the user reads them", "author": { "name": "PostHog" }, "types": "./types/index.d.ts" diff --git a/package-lock.json b/package-lock.json index 7fe2b6cb..ffaca231 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@posthog/coherence", - "version": "1.9.0", + "version": "1.10.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@posthog/coherence", - "version": "1.9.0", + "version": "1.10.0", "license": "MIT", "os": [ "darwin", diff --git a/package.json b/package.json index b3e6a88e..6aa5b739 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@posthog/coherence", - "version": "1.9.0", + "version": "1.10.0", "description": "A prosthetic for proprioception: specs, invariants, and the readings that keep an agent balanced.", "type": "module", "license": "MIT", diff --git a/src/lifecycle/Lifecycle.spec.md b/src/lifecycle/Lifecycle.spec.md index ad6f2498..4e764408 100644 --- a/src/lifecycle/Lifecycle.spec.md +++ b/src/lifecycle/Lifecycle.spec.md @@ -1283,12 +1283,13 @@ The gyroscope delivered through harness events: orient at start, regulate at sto because: in the comment mode, findings are reported and never fail the job, while a run that never happened is never mistaken for a clean one; a report read from the latest verdicts instead of the session's run would show a pass from another branch refuted: buildReport read every run in each project instead of the CI session's, so another session's later pass hid the failing verdict -> "ci report carries each project's problems, chokepoint verdicts, spec gaps and guard failures as Markdown, and fails only when Coherence did not finish" went red in ci.test.ts on its own assertion; restored byte for byte, green batched and alone (2026-10-09) kinds: none -- the CI report says what each finding means and what to do: Under its counts the report says in plain words whether there is anything to fix, and that findings do not fail the check and can come from files the pull request did not change; each section with a finding says what that kind of finding means and how to fix it, and a spec gap section says how many more the adoption baseline holds and why they are not listed. Given --base, it says how many findings are in files changed since that commit, or why it could not tell, and marks each one; the workflow passes the pull request's base. - over: a report with nothing to fix, a spec problem, a failing chokepoint, an open spec gap beside one the adoption baseline holds, a spec problem and a gap in files the pull request changed, findings in none it changed, a base git cannot diff against, and --base read from a real repository +- the CI report says what each finding means and what to do: Under its counts the report says in plain words whether there is anything to fix, and that findings do not fail the check and can come from files the pull request did not change; each section with a finding says what that kind of finding means and how to fix it, and a spec gap section says how many more the adoption baseline holds and why they are not listed. Given --base, or else in a GitHub pull request run the base branch GITHUB_BASE_REF names, it says how many findings are in files changed since that commit, or why it could not tell, and marks each one. + over: a report with nothing to fix, a spec problem, a failing chokepoint, an open spec gap beside one the adoption baseline holds, a spec problem and a gap in files the pull request changed, findings in none it changed, a base git cannot diff against, and --base, GITHUB_BASE_REF and neither read from a real repository via: ci report says what each finding means and what to do because: an adopter read a report of spec problems, spec gaps, traced controls and an adoption baseline and could not tell what any of it meant, whether the pull request caused it, or what to change; a report a reader cannot act on is noise on every pull request refuted: put back the old spec gap words in ci-report.ts, "no traced control" for what the gap means -> "ci report says what each finding means and what to do" went red in ci.test.ts on the spec gap case; restored byte for byte, green (2026-10-25) refuted: made the changed-file mark in ci-report.ts never mark a finding -> "ci report says what each finding means and what to do" went red in ci.test.ts, 4 of 10 cases; restored byte for byte, green (2026-10-25) + refuted: dropped the GITHUB_BASE_REF default in ci-report.ts -> "ci report says what each finding means and what to do" went red in ci.test.ts on the base branch case; restored byte for byte, green (2026-10-25) kinds: none - a closure mirror is named at the edit that breaks it: At PostToolUse, a write to a file in the value-import closure a declared mirror names is read again, and each file the closure now reaches that the list does not name is added to the edit's context as advice, never a block, naming the file that imports it and the list's file and line; a type-only import adds nothing, and the file the closure grew by is watched from then on. over: a fixture of the b37627e shape: a list of the page's modules, the closure of page.ts, a type-only import added to a closure file, then a value import of a file the list omits diff --git a/src/lifecycle/ci-report.ts b/src/lifecycle/ci-report.ts index b676dd77..2f9a38e7 100644 --- a/src/lifecycle/ci-report.ts +++ b/src/lifecycle/ci-report.ts @@ -246,8 +246,11 @@ function changedRead(files: string[] | string): ChangedRead { return typeof files === "string" ? { unread: files } : { files: new Set(files) }; } -/** ci report --session [--base ]: the Markdown on stdout; exit 1 when Coherence did not finish. */ -export async function ciReportCommand(argv: string[], io: Io, readGapsOf: GapReader = readGaps): Promise { +/** + * ci report --session [--base ]: the Markdown on stdout; exit 1 when Coherence did not finish. + * Without --base, a pull request's run on GitHub Actions compares against its base branch, which GITHUB_BASE_REF names. + */ +export async function ciReportCommand(argv: string[], io: Io, readGapsOf: GapReader = readGaps, env: NodeJS.ProcessEnv = process.env): Promise { let session: string | undefined; let base: string | undefined; for (let i = 0; i < argv.length; i++) { @@ -265,6 +268,7 @@ export async function ciReportCommand(argv: string[], io: Io, readGapsOf: GapRea io.err(`ci report: --session names the run's session, as the chokepoint run recorded it\n${CI_USAGE}`); return 64; } + if (base === undefined && env.GITHUB_BASE_REF) base = `origin/${env.GITHUB_BASE_REF}`; const report = await buildReport(io.cwd, session, readGapsOf, base); io.out(report.markdown.trimEnd()); for (const line of report.unfinished) io.err(`ci report: ${line}`); diff --git a/src/lifecycle/ci.test.ts b/src/lifecycle/ci.test.ts index 3cd4c469..c301b898 100644 --- a/src/lifecycle/ci.test.ts +++ b/src/lifecycle/ci.test.ts @@ -114,7 +114,7 @@ test("ci install writes the comment mode for pnpm: findings never fail the job, assert.equal(steps[0], "pnpm install --frozen-lockfile"); assert.equal(steps[1], "pnpm exec coherence spec --check || [ $? -eq 1 ]", "a spec problem is a finding; any other exit breaks the job"); assert.equal(steps[2], 'pnpm exec coherence run --form chokepoint --no-server --session "ci-${{ github.run_id }}" --agent ci || [ $? -eq 1 ]'); - assert.match(steps[3]!, /pnpm exec coherence ci report --session "ci-\$\{\{ github.run_id \}\}" --base "\$\{\{ github.event.pull_request.base.sha \}\}" > "\$RUNNER_TEMP\/coherence-report.md" \|\| status=\$\?/); + assert.match(steps[3]!, /pnpm exec coherence ci report --session "ci-\$\{\{ github.run_id \}\}" > "\$RUNNER_TEMP\/coherence-report.md" \|\| status=\$\?/); assert.match(steps[3]!, />> "\$GITHUB_STEP_SUMMARY"\nexit "\$status"$/, "the report reaches the job summary, then its exit stands"); assert.ok(steps[4]!.includes(`startswith("${REPORT_MARKER}")`), "the comment is found by the report's marker"); assert.match(steps[4]!, /gh api --method PATCH "repos\/\$REPO\/issues\/comments\/\$id" -F body=@"\$body"/, "an existing comment is updated in place"); @@ -356,7 +356,7 @@ describe("ci report says what each finding means and what to do", () => { }); } - test("ci report --base reads the files changed since that commit", async () => { + test("ci report --base, or a pull request's base branch on GitHub Actions, reads the files changed since that commit", async () => { const root = repository({ "A.spec.md": SPEC("a") }); const git = (...args: string[]): string => execFileSync("git", ["-c", "user.name=t", "-c", "user.email=t@t", ...args], { cwd: root, encoding: "utf8" }).trim(); rmSync(join(root, ".git"), { recursive: true }); @@ -369,7 +369,17 @@ describe("ci report says what each finding means and what to do", () => { appendRun(root, record("ci-7", [])); const said = capture(root); assert.equal(await ciReportCommand(["--session", "ci-7", "--base", base], said.io, async () => ({ gaps: [] })), 0); - assert.match(said.out.join("\n"), /1 of these findings is in a file this pull request changed[\s\S]*- `A\.spec\.md:6` [^\n]+ · \*\*changed in this pull request\*\*/); + const marked = /1 of these findings is in a file this pull request changed[\s\S]*- `A\.spec\.md:6` [^\n]+ · \*\*changed in this pull request\*\*/; + assert.match(said.out.join("\n"), marked); + + git("update-ref", "refs/remotes/origin/main", base); + const onActions = capture(root); + assert.equal(await ciReportCommand(["--session", "ci-7"], onActions.io, async () => ({ gaps: [] }), { GITHUB_BASE_REF: "main" }), 0); + assert.match(onActions.out.join("\n"), marked, "GITHUB_BASE_REF names the base when --base does not"); + + const elsewhere = capture(root); + assert.equal(await ciReportCommand(["--session", "ci-7"], elsewhere.io, async () => ({ gaps: [] }), {}), 0); + assert.match(elsewhere.out.join("\n"), /These findings do not fail this check\. Some can come from files this pull request did not change\./, "with neither, no files are compared"); }); }); diff --git a/src/lifecycle/ci.ts b/src/lifecycle/ci.ts index 0b2fbb6d..620dbb58 100644 --- a/src/lifecycle/ci.ts +++ b/src/lifecycle/ci.ts @@ -47,7 +47,7 @@ export const MODE_TEXT: Record = { export const CI_USAGE = [ ` ci install --host <${CI_HOSTS.join("|")}> --mode <${CI_MODES.join("|")}> [--write] the workflow that runs Coherence on every pull request: --mode fail ${MODE_TEXT.fail}; --mode comment ${MODE_TEXT.comment}; printed, and with --write written to ${WORKFLOW_PATH.split(sep).join("/")}, never over a file already there`, - " ci report --session [--base ] the pull request report as Markdown: problems, chokepoint verdicts, spec gaps and guard failures for every project, marking those in files changed since --base; exit 1 when the session's run is missing or a chokepoint was not run", + " ci report --session [--base ] the pull request report as Markdown: problems, chokepoint verdicts, spec gaps and guard failures for every project, marking those in files changed since --base (on GitHub Actions, the pull request's base branch by default); exit 1 when the session's run is missing or a chokepoint was not run", ].join("\n"); /** The actions the workflow uses, pinned by commit; the comment names the release. */ @@ -347,7 +347,7 @@ export function workflowText(plan: CiPlan): string { ...where, " run: |", " status=0", - ` ${cli} ci report --session "ci-\${{ github.run_id }}" --base "\${{ github.event.pull_request.base.sha }}" > "$RUNNER_TEMP/coherence-report.md" || status=$?`, + ` ${cli} ci report --session "ci-\${{ github.run_id }}" > "$RUNNER_TEMP/coherence-report.md" || status=$?`, ' cat "$RUNNER_TEMP/coherence-report.md" >> "$GITHUB_STEP_SUMMARY"', ' exit "$status"', "", From 68cdd1a1e6da859dc42ca7434702521741535488 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Sun, 25 Oct 2026 18:23:22 +0000 Subject: [PATCH 04/10] chore(release): bump the bundled plugins to 1.10.0 and name them in the release steps The declared, python, rust and typescript plugins ship at the package's version, and bundled.e2e.ts holds them equal. The README's release steps named only the mod's plugin.json. Co-Authored-By: Claude Opus 5.5 Generated-By: PostHog Desktop Task-Id: 4189fcb8-6fd5-428d-8c5f-4314d28e4dac --- README.md | 5 +++-- plugins/declared/plugin.json | 2 +- plugins/python/plugin.json | 2 +- plugins/rust/plugin.json | 2 +- plugins/typescript/plugin.json | 2 +- 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 072db8ab..e81c4948 100644 --- a/README.md +++ b/README.md @@ -624,8 +624,9 @@ repository all build. `node-llama-cpp` is an optional peer: a project that wants the local embedding pass installs it beside Coherence. To release, merge a change that bumps `version` in package.json, and the same -`version` in `mods/coherence/.claude-plugin/plugin.json` (a test holds them -equal), into main. +`version` in `mods/coherence/.claude-plugin/plugin.json` and in each bundled +plugin that ships at the package's version (`plugins/declared`, `python`, +`rust` and `typescript`; tests hold them equal), into main. The Release workflow sees a version npm does not have, publishes it through npm trusted publishing from `.github/workflows/release.yml` in the `Release` environment, with provenance and no stored token, and tags it as a GitHub diff --git a/plugins/declared/plugin.json b/plugins/declared/plugin.json index 5005f6a0..d651cb8a 100644 --- a/plugins/declared/plugin.json +++ b/plugins/declared/plugin.json @@ -1,6 +1,6 @@ { "name": "declared", - "version": "1.9.0", + "version": "1.10.0", "pluginApi": 2, "provides": { "declaredLanguages": true diff --git a/plugins/python/plugin.json b/plugins/python/plugin.json index e105cefa..c8ee2d67 100644 --- a/plugins/python/plugin.json +++ b/plugins/python/plugin.json @@ -1,6 +1,6 @@ { "name": "python", - "version": "1.9.0", + "version": "1.10.0", "pluginApi": 2, "provides": { "language": "python" diff --git a/plugins/rust/plugin.json b/plugins/rust/plugin.json index f67111ec..bb2ffeb4 100644 --- a/plugins/rust/plugin.json +++ b/plugins/rust/plugin.json @@ -1,6 +1,6 @@ { "name": "rust", - "version": "1.9.0", + "version": "1.10.0", "pluginApi": 2, "provides": { "language": "rust" diff --git a/plugins/typescript/plugin.json b/plugins/typescript/plugin.json index 1b67f530..d6b107e1 100644 --- a/plugins/typescript/plugin.json +++ b/plugins/typescript/plugin.json @@ -1,6 +1,6 @@ { "name": "typescript", - "version": "1.9.0", + "version": "1.10.0", "pluginApi": 2, "provides": { "language": "typescript" From 1697f9a7ada8c30054d392b55f9ed63866438053 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Sun, 25 Oct 2026 18:48:23 +0000 Subject: [PATCH 05/10] fix: address review-triage findings on the CI report Fixes several bot and QA Swarm review findings on src/lifecycle/ci-report.ts (PR #154): - never says "Nothing to fix" when Coherence did not finish or a project's spec gaps were not read - never says "None of these findings are in files this pull request changed" when some projects' changed files could not be read - attributes a failing chokepoint using its recorded files, not only its bypasses - marks a registry problem when the pull request changed the file it is about - compares against the pull request's fixed base commit from GITHUB_EVENT_PATH on GitHub Actions, falling back to origin/$GITHUB_BASE_REF only when the event carries none, so a moving target branch no longer hides or fakes findings - reuses CLOSE_WAYS from gaps.ts instead of restating the three ways to close a gap - plainer guard-failure wording, a note that guard failures carry no changed-file mark, and a worked example for what a chokepoint failure means - removes a stray double blank line Updates the matching Lifecycle.spec.md bullet and adds test cases for each behavior. Co-Authored-By: Claude Opus 5.5 Generated-By: PostHog Desktop Task-Id: 4189fcb8-6fd5-428d-8c5f-4314d28e4dac --- src/lifecycle/Lifecycle.spec.md | 4 +- src/lifecycle/ci-report.ts | 71 ++++++++++++++++++++++------- src/lifecycle/ci.test.ts | 80 +++++++++++++++++++++++++++++++-- src/lifecycle/ci.ts | 2 +- 4 files changed, 134 insertions(+), 23 deletions(-) diff --git a/src/lifecycle/Lifecycle.spec.md b/src/lifecycle/Lifecycle.spec.md index 4e764408..05542ad5 100644 --- a/src/lifecycle/Lifecycle.spec.md +++ b/src/lifecycle/Lifecycle.spec.md @@ -1283,8 +1283,8 @@ The gyroscope delivered through harness events: orient at start, regulate at sto because: in the comment mode, findings are reported and never fail the job, while a run that never happened is never mistaken for a clean one; a report read from the latest verdicts instead of the session's run would show a pass from another branch refuted: buildReport read every run in each project instead of the CI session's, so another session's later pass hid the failing verdict -> "ci report carries each project's problems, chokepoint verdicts, spec gaps and guard failures as Markdown, and fails only when Coherence did not finish" went red in ci.test.ts on its own assertion; restored byte for byte, green batched and alone (2026-10-09) kinds: none -- the CI report says what each finding means and what to do: Under its counts the report says in plain words whether there is anything to fix, and that findings do not fail the check and can come from files the pull request did not change; each section with a finding says what that kind of finding means and how to fix it, and a spec gap section says how many more the adoption baseline holds and why they are not listed. Given --base, or else in a GitHub pull request run the base branch GITHUB_BASE_REF names, it says how many findings are in files changed since that commit, or why it could not tell, and marks each one. - over: a report with nothing to fix, a spec problem, a failing chokepoint, an open spec gap beside one the adoption baseline holds, a spec problem and a gap in files the pull request changed, findings in none it changed, a base git cannot diff against, and --base, GITHUB_BASE_REF and neither read from a real repository +- the CI report says what each finding means and what to do: Under its counts the report says in plain words whether there is anything to fix, never claiming that when Coherence did not finish or a project's spec gaps were not read; it says that findings do not fail the check and can come from files the pull request did not change; each section with a finding says what that kind of finding means and how to fix it, a spec gap section says how many more the adoption baseline holds and why they are not listed, and a guard failure section says it carries no changed-file mark. Given --base, or else on GitHub Actions the pull request's fixed base commit from the triggering event, falling back to the base branch GITHUB_BASE_REF names, it says how many findings are in files changed since that commit, or why it could not tell — saying so without claiming none are when only some projects' changes were read — and marks each one, including a registry problem in a changed top-level config and a failing chokepoint attributed by its recorded files as well as its bypasses. + over: a report with nothing to fix, one Coherence could not finish or read every gap for, a spec problem, a failing chokepoint with a bypass and one attributed only by its own recorded file, a guard failure, an open spec gap beside one the adoption baseline holds, a spec problem and a gap in files the pull request changed, a registry problem in a changed config, findings in none it changed, a project read beside one that was not, a base git cannot diff against, and --base, an event's fixed base commit, GITHUB_BASE_REF and neither read from a real repository via: ci report says what each finding means and what to do because: an adopter read a report of spec problems, spec gaps, traced controls and an adoption baseline and could not tell what any of it meant, whether the pull request caused it, or what to change; a report a reader cannot act on is noise on every pull request refuted: put back the old spec gap words in ci-report.ts, "no traced control" for what the gap means -> "ci report says what each finding means and what to do" went red in ci.test.ts on the spec gap case; restored byte for byte, green (2026-10-25) diff --git a/src/lifecycle/ci-report.ts b/src/lifecycle/ci-report.ts index 2f9a38e7..5b60b1a6 100644 --- a/src/lifecycle/ci-report.ts +++ b/src/lifecycle/ci-report.ts @@ -11,13 +11,13 @@ * chokepoint the run could not check. Either way the document says so. */ -import { realpathSync } from "node:fs"; +import { readFileSync, realpathSync } from "node:fs"; import { relative, resolve, sep } from "node:path"; import { registryOf, registryProblems } from "../adapters/project-config.ts"; import { loadRuns, type RunEntry, type RunRecord } from "../enforcement/record.ts"; import type { Io } from "../journal/cli.ts"; import { readComponentInterfaces } from "../readings/scope/component-interfaces.ts"; -import { currentGaps, gapsOf, readAndRecord, readGapBaseline, structureState, type Gap } from "../readings/scope/gaps.ts"; +import { CLOSE_WAYS, currentGaps, gapsOf, readAndRecord, readGapBaseline, structureState, type Gap } from "../readings/scope/gaps.ts"; import { flowOf } from "../readings/scope/structure-flow.ts"; import { changedSince } from "../observation/observed.ts"; import { loadSpecModel, type SpecModel } from "../spec/model.ts"; @@ -98,7 +98,6 @@ const MARK: Record = { pass: "✓", fail: "✕", "not run": "○ const CHANGED_HERE = " · **changed in this pull request**"; - /** One project's section, and why its part did not finish. */ function projectSection(p: ProjectReport, session: string, heading: boolean): { lines: string[]; unfinished: string[]; touched: number } { const lines: string[] = []; @@ -131,9 +130,13 @@ function projectSection(p: ProjectReport, session: string, heading: boolean): { const fails = entries.filter((e) => e.verdict === "fail").length; const notRun = entries.filter((e) => e.verdict === "not run"); lines.push(`**Chokepoints:** ${entries.length - fails - notRun.length} pass, ${fails} fail, ${notRun.length} not run`, ""); - if (fails > 0) lines.push("A chokepoint fails when code reaches what it protects without going through it. Send each bypass below through the chokepoint, or fix the spec if the rule is wrong.", ""); + if (fails > 0) + lines.push( + "A chokepoint fails when code reaches what it protects without going through it — for example, code that reads a table directly instead of going through the one function meant to guard it. Send each bypass below through the chokepoint, or fix the spec if the rule is wrong.", + "", + ); for (const e of [...entries].sort((a, b) => order(a) - order(b))) { - lines.push(`- ${MARK[e.verdict]} \`${bullet(e)}\` ${e.grade ?? e.verdict}: ${cell(e.reason)}${e.verdict === "fail" ? mark(...e.bypasses.map((b) => b.file)) : ""}`); + lines.push(`- ${MARK[e.verdict]} \`${bullet(e)}\` ${e.grade ?? e.verdict}: ${cell(e.reason)}${e.verdict === "fail" ? mark(...e.files, ...e.bypasses.map((b) => b.file)) : ""}`); for (const b of e.bypasses.slice(0, BYPASSES)) lines.push(` - bypass \`${b.file}:${b.line}\` in ${cell(b.symbol)}`); if (e.bypasses.length > BYPASSES) lines.push(` - and ${e.bypasses.length - BYPASSES} more; run --form chokepoint names them all`); } @@ -150,13 +153,20 @@ function projectSection(p: ProjectReport, session: string, heading: boolean): { const was = held === 1 ? "was" : "were"; lines.push(`**Spec gaps:** ${plural(open.length, "entrance")} ${takes} input from outside this code (trust outside or unknown), and the spec names no check on that input.${held === 0 ? "" : ` ${held} more ${was} already open when Coherence was adopted, so ${held === 1 ? "it is" : "they are"} not listed.`}`, ""); for (const g of open) lines.push(`- \`${g.specPath}\` entrance **${cell(g.name)}** (trust ${cell(g.trust.join(", ") || "unknown")}), route: ${cell(g.route.stops.join(" -> "))}${mark(g.file, g.specPath)}`); - if (open.length > 0) lines.push("", "To close a gap, add one of these to its spec: `guard: ` when a verified chokepoint checks the input; the entrance's name on the `entrances:` line of an invariant that covers it; or `control: none — ` when it needs no check. `coherence scaffold control \"\"` proposes the line."); + if (open.length > 0) lines.push("", `To close a gap, ${CLOSE_WAYS}. \`coherence scaffold control ""\` proposes the line.`); lines.push(""); } const failures = p.model.defects?.guardFailures ?? []; if (failures.length > 0) { - lines.push(`**Guard failures:** ${failures.length}`, "", "A defect came back in a class that a guard was meant to stop. Check why each guard below let it through.", ""); + lines.push( + `**Guard failures:** ${failures.length}`, + "", + "A kind of bug came back, even though a guard was meant to stop it. Check why each guard below let it through.", + "", + "These have no file to check against this pull request's changes, so none of them ever carry the changed-in-this-pull-request mark.", + "", + ); for (const g of failures) lines.push(`- ${g.id} in class ${cell(g.class)}, which ${cell(g.guard)} has guarded since ${g.resolution} closed ${g.guardedBy}`); lines.push(""); } @@ -172,22 +182,35 @@ function order(e: RunEntry): number { } /** Whether there is anything to fix, and which findings this pull request's own files hold, when the report knows them. */ -function findingsLead(projects: readonly ProjectReport[], findings: number, touched: number): string[] { - if (findings === 0) return ["Nothing to fix.", ""]; +function findingsLead(projects: readonly ProjectReport[], findings: number, touched: number, unfinished: readonly string[]): string[] { + if (findings === 0) { + if (unfinished.length > 0 || projects.some((p) => "unread" in p.gaps)) return ["Coherence could not check everything, so this might not list every finding.", ""]; + return ["Nothing to fix.", ""]; + } const changed = projects.filter((p) => p.changed !== undefined); if (changed.length === 0) return ["These findings do not fail this check. Some can come from files this pull request did not change.", ""]; const lines = ["These findings do not fail this check.", ""]; + const allRead = changed.every((p) => "files" in p.changed!); for (const p of changed) if ("unread" in p.changed!) lines.push(`Coherence could not tell which files this pull request changed${p.name === "." ? "" : ` in ${cell(p.name)}`}: ${cell(p.changed.unread)}.`, ""); if (changed.some((p) => "files" in p.changed!)) { - if (touched === 0) lines.push("None of these findings are in files this pull request changed.", ""); + if (touched === 0) lines.push(allRead ? "None of these findings are in files this pull request changed." : "None of the findings Coherence could check are in files this pull request changed.", ""); else if (touched === 1) lines.push("1 of these findings is in a file this pull request changed. It is marked below.", ""); else lines.push(`${touched} of these findings are in files this pull request changed. They are marked below.`, ""); } return lines; } +/** Whether a registry problem's file is one this pull request changed. */ +function registryMark(problem: { file: string }, changed: ChangedRead | undefined): string { + return changed !== undefined && "files" in changed && changed.files.has(problem.file) ? CHANGED_HERE : ""; +} + /** The report over the projects, with the registry's own problems when the report ran at its top. */ -export function renderReport(projects: readonly ProjectReport[], session: string, registry: { problems: { file: string; message: string }[] } | undefined): Report { +export function renderReport( + projects: readonly ProjectReport[], + session: string, + registry: { problems: { file: string; message: string }[]; changed: ChangedRead | undefined } | undefined, +): Report { const unfinished: string[] = []; const sections: string[] = []; let touched = 0; @@ -197,13 +220,15 @@ export function renderReport(projects: readonly ProjectReport[], session: string unfinished.push(...section.unfinished); touched += section.touched; } + const registryMarks = (registry?.problems ?? []).map((problem) => registryMark(problem, registry?.changed)); + touched += registryMarks.filter((m) => m !== "").length; const problems = projects.reduce((n, p) => n + p.model.problems.length, 0) + (registry?.problems.length ?? 0); const fails = projects.reduce((n, p) => n + chokepointEntries(p.runs).filter((e) => e.verdict === "fail").length, 0); const gaps = projects.reduce((n, p) => n + ("gaps" in p.gaps ? p.gaps.gaps.filter((g) => !p.baselined.has(`${g.component}\u0000${g.name}`)).length : 0), 0); const guards = projects.reduce((n, p) => n + (p.model.defects?.guardFailures.length ?? 0), 0); const head = [REPORT_MARKER, "## Coherence", ""]; head.push(`**${plural(problems, "spec problem")} · ${plural(fails, "failing chokepoint")} · ${plural(gaps, "spec gap")} · ${plural(guards, "guard failure")}**${projects.length > 1 ? ` across ${plural(projects.length, "project")}` : ""}`, ""); - head.push(...findingsLead(projects, problems + fails + gaps + guards, touched)); + head.push(...findingsLead(projects, problems + fails + gaps + guards, touched, unfinished)); if (unfinished.length > 0) { head.push("> [!WARNING]", "> Coherence did not finish, so this report is incomplete and the check fails:"); for (const line of unfinished) head.push(`> - ${cell(line)}`); @@ -211,7 +236,7 @@ export function renderReport(projects: readonly ProjectReport[], session: string } if (registry !== undefined && registry.problems.length > 0) { head.push("**Registry problems**", ""); - for (const problem of registry.problems) head.push(`- \`${problem.file}\` ${cell(problem.message)}`); + registry.problems.forEach((problem, i) => head.push(`- \`${problem.file}\` ${cell(problem.message)}${registryMarks[i]}`)); head.push(""); } const markdown = [...head, ...sections].join("\n").replace(/\n{3,}/g, "\n\n").trimEnd() + "\n"; @@ -239,16 +264,30 @@ export async function buildReport(given: string, session: string, readGapsOf: Ga changed: base === undefined ? undefined : changedRead(changedSince(project, base)), }); } - return renderReport(projects, session, atTop ? { problems: registryProblems(registry.top) } : undefined); + return renderReport(projects, session, atTop ? { problems: registryProblems(registry.top), changed: base === undefined ? undefined : changedRead(changedSince(root, base)) } : undefined); } function changedRead(files: string[] | string): ChangedRead { return typeof files === "string" ? { unread: files } : { files: new Set(files) }; } +/** The pull request's base commit as GitHub Actions recorded it at the event that started the run, fixed even when the base branch moves on while the run is in flight. */ +function eventBaseSha(env: NodeJS.ProcessEnv): string | undefined { + if (!env.GITHUB_EVENT_PATH) return undefined; + try { + const event = JSON.parse(readFileSync(env.GITHUB_EVENT_PATH, "utf8")) as { pull_request?: { base?: { sha?: string } } }; + const sha = event.pull_request?.base?.sha; + return sha === undefined || sha === "" ? undefined : sha; + } catch { + return undefined; + } +} + /** * ci report --session [--base ]: the Markdown on stdout; exit 1 when Coherence did not finish. - * Without --base, a pull request's run on GitHub Actions compares against its base branch, which GITHUB_BASE_REF names. + * Without --base, a pull request's run on GitHub Actions compares against the event's fixed base commit, falling + * back to the base branch GITHUB_BASE_REF names when the event carries none — the base branch itself can move on + * while the run is still going, which --base must not follow. */ export async function ciReportCommand(argv: string[], io: Io, readGapsOf: GapReader = readGaps, env: NodeJS.ProcessEnv = process.env): Promise { let session: string | undefined; @@ -268,7 +307,7 @@ export async function ciReportCommand(argv: string[], io: Io, readGapsOf: GapRea io.err(`ci report: --session names the run's session, as the chokepoint run recorded it\n${CI_USAGE}`); return 64; } - if (base === undefined && env.GITHUB_BASE_REF) base = `origin/${env.GITHUB_BASE_REF}`; + if (base === undefined) base = eventBaseSha(env) ?? (env.GITHUB_BASE_REF ? `origin/${env.GITHUB_BASE_REF}` : undefined); const report = await buildReport(io.cwd, session, readGapsOf, base); io.out(report.markdown.trimEnd()); for (const line of report.unfinished) io.err(`ci report: ${line}`); diff --git a/src/lifecycle/ci.test.ts b/src/lifecycle/ci.test.ts index c301b898..d9d7d6a2 100644 --- a/src/lifecycle/ci.test.ts +++ b/src/lifecycle/ci.test.ts @@ -18,7 +18,7 @@ import { stopWarmServers } from "../enforcement/server-fixture.ts"; import type { Io } from "../journal/cli.ts"; import type { Gap } from "../readings/scope/gaps.ts"; import { loadSpecModel } from "../spec/model.ts"; -import { buildReport, ciReportCommand, renderReport, type GapReader } from "./ci-report.ts"; +import { buildReport, ciReportCommand, renderReport, type GapReader, type ProjectReport } from "./ci-report.ts"; import { cli } from "./cli-fixture.ts"; import { ciInstallCommand, ciSuggestion, CiError, planWorkflow, REPORT_MARKER, WORKFLOW_PATH, workflowText } from "./ci.ts"; @@ -305,7 +305,7 @@ test("ci report carries each project's problems, chokepoint verdicts, spec gaps model.defects = { recorded: 2, closed: 1, guarded: 1, decided: 0, unguarded: [], undeclared: [], guardFailures: [{ id: "df-2", class: "bypass", guard: "products/a/a door", guardedBy: "df-1", resolution: "rs-1" }] }; const guarded = renderReport([{ name: ".", model, runs: [record("ci-7", [{ name: "a door" }])], gaps: { gaps: [] }, baselined: new Set() }], "ci-7", undefined); assert.match(guarded.markdown, /0 spec gaps · 1 guard failure\*\*/); - assert.match(guarded.markdown, /\*\*Guard failures:\*\* 1\n\n[^\n]+\n\n- df-2 in class bypass, which products\/a\/a door has guarded since rs-1 closed df-1/); + assert.match(guarded.markdown, /\*\*Guard failures:\*\* 1\n\n[^\n]+\n\n[^\n]+\n\n- df-2 in class bypass, which products\/a\/a door has guarded since rs-1 closed df-1/); assert.doesNotMatch(guarded.markdown, /^### /m, "one project at the root needs no heading"); }); @@ -323,7 +323,25 @@ describe("ci report says what each finding means and what to do", () => { baselined: [], says: [/\n\nThese findings do not fail this check\. Some can come from files this pull request did not change\.\n/, /\*\*Spec problems\*\* \(spec --check\): lines Coherence cannot read as written\. Fix each one, then run `coherence spec --check` until it finds 0 problems\.\n\n- `A\.spec\.md:6` an invariant bullet starts with a short name and a colon, as in "- \\: \\"\n/], }, - { finding: "a failing chokepoint", spec: SPEC("a"), runs: [{ name: "a door", verdict: "fail", grade: "broken" }], gaps: [], baselined: [], says: [/\*\*Chokepoints:\*\* 0 pass, 1 fail, 0 not run\n\nA chokepoint fails when code reaches what it protects without going through it\. Send each bypass below through the chokepoint, or fix the spec if the rule is wrong\.\n/] }, + { + finding: "a failing chokepoint", + spec: SPEC("a"), + runs: [{ name: "a door", verdict: "fail", grade: "broken" }], + gaps: [], + baselined: [], + says: [ + /\*\*Chokepoints:\*\* 0 pass, 1 fail, 0 not run\n\nA chokepoint fails when code reaches what it protects without going through it — for example, code that reads a table directly instead of going through the one function meant to guard it\. Send each bypass below through the chokepoint, or fix the spec if the rule is wrong\.\n/, + ], + }, + { + finding: "a failing chokepoint recorded against its own file, with no bypass", + spec: SPEC("a"), + runs: [{ name: "a door", verdict: "fail", grade: "broken", files: ["src/door.ts"] }], + gaps: [], + baselined: [], + changed: ["src/door.ts"], + says: [/✕ `a door` broken: [^\n]+ · \*\*changed in this pull request\*\*\n/], + }, { finding: "a spec gap and one the baseline holds", spec: SPEC("a"), @@ -332,7 +350,7 @@ describe("ci report says what each finding means and what to do", () => { baselined: ["old"], says: [ /\*\*Spec gaps:\*\* 1 entrance takes input from outside this code \(trust outside or unknown\), and the spec names no check on that input\. 1 more was already open when Coherence was adopted, so it is not listed\.\n\n- `A\.spec\.md` entrance \*\*webhook\*\* \(trust outside\), route: \. -\\> store\n/, - /\nTo close a gap, add one of these to its spec: `guard: ` when a verified chokepoint checks the input; the entrance's name on the `entrances:` line of an invariant that covers it; or `control: none — ` when it needs no check\. `coherence scaffold control ""` proposes the line\.\n/, + /\nTo close a gap, declare guard: where a verified chokepoint wraps it, name it on the entrances: line of an invariant whose crossing enters from its trust, or record control: none — \. `coherence scaffold control ""` proposes the line\.\n/, ], }, { @@ -347,6 +365,14 @@ describe("ci report says what each finding means and what to do", () => { { finding: "a spec gap whose handler the pull request changed", spec: SPEC("a"), runs: [{ name: "a door" }], gaps: [WEBHOOK], baselined: [], changed: ["src/hook.ts"], says: [/route: \. -\\> store · \*\*changed in this pull request\*\*\n/] }, { finding: "findings in no file the pull request changed", spec: UNNAMED, runs: [], gaps: [WEBHOOK], baselined: [], changed: ["src/other.ts"], says: [/\n\nNone of these findings are in files this pull request changed\.\n/] }, { finding: "changed files git could not list", spec: UNNAMED, runs: [], gaps: [], baselined: [], changed: "git diff main failed: bad revision", says: [/\n\nCoherence could not tell which files this pull request changed: git diff main failed: bad revision\.\n/] }, + { + finding: "no chokepoint run, so there is nothing to fix only in appearance", + spec: SPEC("a"), + runs: [{ name: "a door", verdict: "not run", reason: "the instrument never answered" }], + gaps: [], + baselined: [], + says: [/\*\*\n\nCoherence could not check everything, so this might not list every finding\.\n/], + }, ] satisfies { finding: string; spec: string; runs: Partial[]; gaps: Gap[]; baselined: string[]; changed?: string[] | string; says: RegExp[] }[]) { test(finding, () => { const root = repository({ "A.spec.md": spec }); @@ -356,6 +382,40 @@ describe("ci report says what each finding means and what to do", () => { }); } + test("spec gaps Coherence could not read keep the lead from claiming there is nothing to fix", () => { + const root = repository({ "A.spec.md": SPEC("a") }); + const model = loadSpecModel(root); + const report = renderReport([{ name: ".", model, runs: [record("ci-7", [{ name: "a door" }])], gaps: { unread: "the language server never answered" }, baselined: new Set() }], "ci-7", undefined); + assert.match(report.markdown, /\*\*\n\nCoherence could not check everything, so this might not list every finding\.\n/); + }); + + test("a project whose changed files were read beside one that was not keeps the lead from claiming none of the findings changed", () => { + const root = repository({ "A.spec.md": SPEC("a") }); + const model = loadSpecModel(root); + const read: ProjectReport = { name: "read", model, runs: [record("ci-7", [{ name: "a door" }])], gaps: { gaps: [WEBHOOK] }, baselined: new Set(), changed: { files: new Set(["src/other.ts"]) } }; + const unread: ProjectReport = { name: "unread", model, runs: [record("ci-7", [{ name: "a door" }])], gaps: { gaps: [] }, baselined: new Set(), changed: { unread: "git diff main failed: bad revision" } }; + const report = renderReport([read, unread], "ci-7", undefined); + assert.match(report.markdown, /None of the findings Coherence could check are in files this pull request changed\./); + assert.doesNotMatch(report.markdown, /None of these findings are in files this pull request changed\./); + }); + + test("registry problems are marked when the pull request changed the file they are about", () => { + const root = repository({ "A.spec.md": SPEC("a") }); + const model = loadSpecModel(root); + const changedReport = renderReport( + [{ name: ".", model, runs: [record("ci-7", [{ name: "a door" }])], gaps: { gaps: [] }, baselined: new Set() }], + "ci-7", + { problems: [{ file: "coherence.config.json", message: "projects is a list of folders relative to the repository top" }], changed: { files: new Set(["coherence.config.json"]) } }, + ); + assert.match(changedReport.markdown, /- `coherence\.config\.json` projects is a list of folders relative to the repository top · \*\*changed in this pull request\*\*\n/); + const unchangedReport = renderReport( + [{ name: ".", model, runs: [record("ci-7", [{ name: "a door" }])], gaps: { gaps: [] }, baselined: new Set() }], + "ci-7", + { problems: [{ file: "coherence.config.json", message: "projects is a list of folders relative to the repository top" }], changed: { files: new Set(["src/other.ts"]) } }, + ); + assert.match(unchangedReport.markdown, /- `coherence\.config\.json` projects is a list of folders relative to the repository top\n/); + }); + test("ci report --base, or a pull request's base branch on GitHub Actions, reads the files changed since that commit", async () => { const root = repository({ "A.spec.md": SPEC("a") }); const git = (...args: string[]): string => execFileSync("git", ["-c", "user.name=t", "-c", "user.email=t@t", ...args], { cwd: root, encoding: "utf8" }).trim(); @@ -377,6 +437,18 @@ describe("ci report says what each finding means and what to do", () => { assert.equal(await ciReportCommand(["--session", "ci-7"], onActions.io, async () => ({ gaps: [] }), { GITHUB_BASE_REF: "main" }), 0); assert.match(onActions.out.join("\n"), marked, "GITHUB_BASE_REF names the base when --base does not"); + const head = git("rev-parse", "HEAD"); + git("update-ref", "refs/remotes/origin/main", head); + const eventPath = join(root, "event.json"); + writeFileSync(eventPath, JSON.stringify({ pull_request: { base: { sha: base } } })); + const fixedBase = capture(root); + assert.equal(await ciReportCommand(["--session", "ci-7"], fixedBase.io, async () => ({ gaps: [] }), { GITHUB_BASE_REF: "main", GITHUB_EVENT_PATH: eventPath }), 0); + assert.match(fixedBase.out.join("\n"), marked, "the event's fixed base commit is used even once origin/main has moved past it"); + + const noEvent = capture(root); + assert.equal(await ciReportCommand(["--session", "ci-7"], noEvent.io, async () => ({ gaps: [] }), { GITHUB_BASE_REF: "main", GITHUB_EVENT_PATH: join(root, "missing-event.json") }), 0); + assert.doesNotMatch(noEvent.out.join("\n"), marked, "a missing event file falls back to the moved base branch, which no longer shows the change"); + const elsewhere = capture(root); assert.equal(await ciReportCommand(["--session", "ci-7"], elsewhere.io, async () => ({ gaps: [] }), {}), 0); assert.match(elsewhere.out.join("\n"), /These findings do not fail this check\. Some can come from files this pull request did not change\./, "with neither, no files are compared"); diff --git a/src/lifecycle/ci.ts b/src/lifecycle/ci.ts index 620dbb58..a79f51f4 100644 --- a/src/lifecycle/ci.ts +++ b/src/lifecycle/ci.ts @@ -47,7 +47,7 @@ export const MODE_TEXT: Record = { export const CI_USAGE = [ ` ci install --host <${CI_HOSTS.join("|")}> --mode <${CI_MODES.join("|")}> [--write] the workflow that runs Coherence on every pull request: --mode fail ${MODE_TEXT.fail}; --mode comment ${MODE_TEXT.comment}; printed, and with --write written to ${WORKFLOW_PATH.split(sep).join("/")}, never over a file already there`, - " ci report --session [--base ] the pull request report as Markdown: problems, chokepoint verdicts, spec gaps and guard failures for every project, marking those in files changed since --base (on GitHub Actions, the pull request's base branch by default); exit 1 when the session's run is missing or a chokepoint was not run", + " ci report --session [--base ] the pull request report as Markdown: problems, chokepoint verdicts, spec gaps and guard failures for every project, marking those in files changed since --base (on GitHub Actions, the pull request's fixed base commit by default, or its base branch when that is not known); exit 1 when the session's run is missing or a chokepoint was not run", ].join("\n"); /** The actions the workflow uses, pinned by commit; the comment names the release. */ From 5d52973046de727a8815c785a2e9d5f6b3f9d71a Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Sun, 25 Oct 2026 18:50:48 +0000 Subject: [PATCH 06/10] fix(ci): lead the gap advice with a plain sentence and keep the lexicon clean Reusing CLOSE_WAYS kept one source for the three closing moves but put spec jargon back in front of PR authors; a plain sentence now leads it. The spec bullet no longer uses a noun the lexicon does not know. Co-Authored-By: Claude Opus 5.5 Generated-By: PostHog Desktop Task-Id: 4189fcb8-6fd5-428d-8c5f-4314d28e4dac --- src/lifecycle/Lifecycle.spec.md | 2 +- src/lifecycle/ci-report.ts | 2 +- src/lifecycle/ci.test.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lifecycle/Lifecycle.spec.md b/src/lifecycle/Lifecycle.spec.md index 05542ad5..20b857f8 100644 --- a/src/lifecycle/Lifecycle.spec.md +++ b/src/lifecycle/Lifecycle.spec.md @@ -1283,7 +1283,7 @@ The gyroscope delivered through harness events: orient at start, regulate at sto because: in the comment mode, findings are reported and never fail the job, while a run that never happened is never mistaken for a clean one; a report read from the latest verdicts instead of the session's run would show a pass from another branch refuted: buildReport read every run in each project instead of the CI session's, so another session's later pass hid the failing verdict -> "ci report carries each project's problems, chokepoint verdicts, spec gaps and guard failures as Markdown, and fails only when Coherence did not finish" went red in ci.test.ts on its own assertion; restored byte for byte, green batched and alone (2026-10-09) kinds: none -- the CI report says what each finding means and what to do: Under its counts the report says in plain words whether there is anything to fix, never claiming that when Coherence did not finish or a project's spec gaps were not read; it says that findings do not fail the check and can come from files the pull request did not change; each section with a finding says what that kind of finding means and how to fix it, a spec gap section says how many more the adoption baseline holds and why they are not listed, and a guard failure section says it carries no changed-file mark. Given --base, or else on GitHub Actions the pull request's fixed base commit from the triggering event, falling back to the base branch GITHUB_BASE_REF names, it says how many findings are in files changed since that commit, or why it could not tell — saying so without claiming none are when only some projects' changes were read — and marks each one, including a registry problem in a changed top-level config and a failing chokepoint attributed by its recorded files as well as its bypasses. +- the CI report says what each finding means and what to do: Under its counts the report says in plain words whether there is anything to fix, never claiming that when Coherence did not finish or a project's spec gaps were not read; it says that findings do not fail the check and can come from files the pull request did not change; each section with a finding says what that kind of finding means and how to fix it, a spec gap section says how many more the adoption baseline holds and why they are not listed, and a guard failure section says it carries no changed-file mark. Given --base, or else in a GitHub pull request run the pull request's fixed base commit from the triggering event, falling back to the base branch GITHUB_BASE_REF names, it says how many findings are in files changed since that commit, or why it could not tell — saying so without claiming none are when only some projects' changes were read — and marks each one, including a registry problem in a changed top-level config and a failing chokepoint attributed by its recorded files as well as its bypasses. over: a report with nothing to fix, one Coherence could not finish or read every gap for, a spec problem, a failing chokepoint with a bypass and one attributed only by its own recorded file, a guard failure, an open spec gap beside one the adoption baseline holds, a spec problem and a gap in files the pull request changed, a registry problem in a changed config, findings in none it changed, a project read beside one that was not, a base git cannot diff against, and --base, an event's fixed base commit, GITHUB_BASE_REF and neither read from a real repository via: ci report says what each finding means and what to do because: an adopter read a report of spec problems, spec gaps, traced controls and an adoption baseline and could not tell what any of it meant, whether the pull request caused it, or what to change; a report a reader cannot act on is noise on every pull request diff --git a/src/lifecycle/ci-report.ts b/src/lifecycle/ci-report.ts index 5b60b1a6..86e49033 100644 --- a/src/lifecycle/ci-report.ts +++ b/src/lifecycle/ci-report.ts @@ -153,7 +153,7 @@ function projectSection(p: ProjectReport, session: string, heading: boolean): { const was = held === 1 ? "was" : "were"; lines.push(`**Spec gaps:** ${plural(open.length, "entrance")} ${takes} input from outside this code (trust outside or unknown), and the spec names no check on that input.${held === 0 ? "" : ` ${held} more ${was} already open when Coherence was adopted, so ${held === 1 ? "it is" : "they are"} not listed.`}`, ""); for (const g of open) lines.push(`- \`${g.specPath}\` entrance **${cell(g.name)}** (trust ${cell(g.trust.join(", ") || "unknown")}), route: ${cell(g.route.stops.join(" -> "))}${mark(g.file, g.specPath)}`); - if (open.length > 0) lines.push("", `To close a gap, ${CLOSE_WAYS}. \`coherence scaffold control ""\` proposes the line.`); + if (open.length > 0) lines.push("", `To close a gap, write in its spec what checks that input. In spec terms: ${CLOSE_WAYS}. \`coherence scaffold control ""\` proposes the line.`); lines.push(""); } diff --git a/src/lifecycle/ci.test.ts b/src/lifecycle/ci.test.ts index d9d7d6a2..691c9e8a 100644 --- a/src/lifecycle/ci.test.ts +++ b/src/lifecycle/ci.test.ts @@ -350,7 +350,7 @@ describe("ci report says what each finding means and what to do", () => { baselined: ["old"], says: [ /\*\*Spec gaps:\*\* 1 entrance takes input from outside this code \(trust outside or unknown\), and the spec names no check on that input\. 1 more was already open when Coherence was adopted, so it is not listed\.\n\n- `A\.spec\.md` entrance \*\*webhook\*\* \(trust outside\), route: \. -\\> store\n/, - /\nTo close a gap, declare guard: where a verified chokepoint wraps it, name it on the entrances: line of an invariant whose crossing enters from its trust, or record control: none — \. `coherence scaffold control ""` proposes the line\.\n/, + /\nTo close a gap, write in its spec what checks that input\. In spec terms: declare guard: where a verified chokepoint wraps it, name it on the entrances: line of an invariant whose crossing enters from its trust, or record control: none — \. `coherence scaffold control ""` proposes the line\.\n/, ], }, { From 5207b59928d543e3d37ff107c62d90b4d6fe5e1e Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Sun, 25 Oct 2026 18:52:53 +0000 Subject: [PATCH 07/10] refactor: apply simplify pass One changedFiles helper reads the changed files of a project or the registry, in place of two copies of the same check. Co-Authored-By: Claude Opus 5.5 Generated-By: PostHog Desktop Task-Id: 4189fcb8-6fd5-428d-8c5f-4314d28e4dac --- src/lifecycle/ci-report.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/lifecycle/ci-report.ts b/src/lifecycle/ci-report.ts index 86e49033..e03f49c5 100644 --- a/src/lifecycle/ci-report.ts +++ b/src/lifecycle/ci-report.ts @@ -30,6 +30,11 @@ export type GapsRead = { gaps: Gap[] } | { unread: string }; /** The files a pull request changed, relative to the project, or why they are not known. */ export type ChangedRead = { files: Set } | { unread: string }; +/** The changed files a read holds, or none when there is no read or it failed. */ +function changedFiles(changed: ChangedRead | undefined): Set { + return changed !== undefined && "files" in changed ? changed.files : new Set(); +} + /** How the report learns a project's gaps; a test hands in one that needs no language server. */ export type GapReader = (root: string, model: SpecModel) => Promise; @@ -103,7 +108,7 @@ function projectSection(p: ProjectReport, session: string, heading: boolean): { const lines: string[] = []; const unfinished: string[] = []; let touched = 0; - const changed = p.changed !== undefined && "files" in p.changed ? p.changed.files : new Set(); + const changed = changedFiles(p.changed); const mark = (...files: (string | undefined)[]): string => { if (!files.some((f) => f !== undefined && changed.has(f))) return ""; touched++; @@ -202,7 +207,7 @@ function findingsLead(projects: readonly ProjectReport[], findings: number, touc /** Whether a registry problem's file is one this pull request changed. */ function registryMark(problem: { file: string }, changed: ChangedRead | undefined): string { - return changed !== undefined && "files" in changed && changed.files.has(problem.file) ? CHANGED_HERE : ""; + return changedFiles(changed).has(problem.file) ? CHANGED_HERE : ""; } /** The report over the projects, with the registry's own problems when the report ran at its top. */ From 7e9f108a3c8ad5b3cadd3c8ca3f59bb0ef8ce5ed Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Sun, 25 Oct 2026 18:56:52 +0000 Subject: [PATCH 08/10] fix(ci): keep spec placeholders, match quoted file names, and say when the registry diff failed CLOSE_WAYS goes through cell() so GitHub does not strip and as HTML. changedSince asks git for unquoted paths, so a non-ASCII file name matches its finding. A registry top whose changed files git could not list is named in the lead line, as a project's is. Co-Authored-By: Claude Opus 5.5 Generated-By: PostHog Desktop Task-Id: 4189fcb8-6fd5-428d-8c5f-4314d28e4dac --- src/lifecycle/Lifecycle.spec.md | 2 +- src/lifecycle/ci-report.ts | 22 +++++++++++++++------- src/lifecycle/ci.test.ts | 27 ++++++++++++++++++++++++++- src/observation/observed.ts | 4 ++-- 4 files changed, 44 insertions(+), 11 deletions(-) diff --git a/src/lifecycle/Lifecycle.spec.md b/src/lifecycle/Lifecycle.spec.md index 20b857f8..36ee5b99 100644 --- a/src/lifecycle/Lifecycle.spec.md +++ b/src/lifecycle/Lifecycle.spec.md @@ -1284,7 +1284,7 @@ The gyroscope delivered through harness events: orient at start, regulate at sto refuted: buildReport read every run in each project instead of the CI session's, so another session's later pass hid the failing verdict -> "ci report carries each project's problems, chokepoint verdicts, spec gaps and guard failures as Markdown, and fails only when Coherence did not finish" went red in ci.test.ts on its own assertion; restored byte for byte, green batched and alone (2026-10-09) kinds: none - the CI report says what each finding means and what to do: Under its counts the report says in plain words whether there is anything to fix, never claiming that when Coherence did not finish or a project's spec gaps were not read; it says that findings do not fail the check and can come from files the pull request did not change; each section with a finding says what that kind of finding means and how to fix it, a spec gap section says how many more the adoption baseline holds and why they are not listed, and a guard failure section says it carries no changed-file mark. Given --base, or else in a GitHub pull request run the pull request's fixed base commit from the triggering event, falling back to the base branch GITHUB_BASE_REF names, it says how many findings are in files changed since that commit, or why it could not tell — saying so without claiming none are when only some projects' changes were read — and marks each one, including a registry problem in a changed top-level config and a failing chokepoint attributed by its recorded files as well as its bypasses. - over: a report with nothing to fix, one Coherence could not finish or read every gap for, a spec problem, a failing chokepoint with a bypass and one attributed only by its own recorded file, a guard failure, an open spec gap beside one the adoption baseline holds, a spec problem and a gap in files the pull request changed, a registry problem in a changed config, findings in none it changed, a project read beside one that was not, a base git cannot diff against, and --base, an event's fixed base commit, GITHUB_BASE_REF and neither read from a real repository + over: a report with nothing to fix, one Coherence could not finish or read every gap for, a spec problem, a failing chokepoint with a bypass and one attributed only by its own recorded file, a guard failure, an open spec gap beside one the adoption baseline holds, a spec problem and a gap in files the pull request changed, a registry problem in a changed config, findings in none it changed, a project read beside one that was not, a base git cannot diff against at a project or at the registry top, a file name git would quote, and --base, an event's fixed base commit, GITHUB_BASE_REF and neither read from a real repository via: ci report says what each finding means and what to do because: an adopter read a report of spec problems, spec gaps, traced controls and an adoption baseline and could not tell what any of it meant, whether the pull request caused it, or what to change; a report a reader cannot act on is noise on every pull request refuted: put back the old spec gap words in ci-report.ts, "no traced control" for what the gap means -> "ci report says what each finding means and what to do" went red in ci.test.ts on the spec gap case; restored byte for byte, green (2026-10-25) diff --git a/src/lifecycle/ci-report.ts b/src/lifecycle/ci-report.ts index e03f49c5..9b31144b 100644 --- a/src/lifecycle/ci-report.ts +++ b/src/lifecycle/ci-report.ts @@ -158,7 +158,7 @@ function projectSection(p: ProjectReport, session: string, heading: boolean): { const was = held === 1 ? "was" : "were"; lines.push(`**Spec gaps:** ${plural(open.length, "entrance")} ${takes} input from outside this code (trust outside or unknown), and the spec names no check on that input.${held === 0 ? "" : ` ${held} more ${was} already open when Coherence was adopted, so ${held === 1 ? "it is" : "they are"} not listed.`}`, ""); for (const g of open) lines.push(`- \`${g.specPath}\` entrance **${cell(g.name)}** (trust ${cell(g.trust.join(", ") || "unknown")}), route: ${cell(g.route.stops.join(" -> "))}${mark(g.file, g.specPath)}`); - if (open.length > 0) lines.push("", `To close a gap, write in its spec what checks that input. In spec terms: ${CLOSE_WAYS}. \`coherence scaffold control ""\` proposes the line.`); + if (open.length > 0) lines.push("", `To close a gap, write in its spec what checks that input. In spec terms: ${cell(CLOSE_WAYS)}. \`coherence scaffold control ""\` proposes the line.`); lines.push(""); } @@ -187,17 +187,23 @@ function order(e: RunEntry): number { } /** Whether there is anything to fix, and which findings this pull request's own files hold, when the report knows them. */ -function findingsLead(projects: readonly ProjectReport[], findings: number, touched: number, unfinished: readonly string[]): string[] { +/** Where the report read changed files: each project, and the registry top when it has problems of its own. */ +interface ChangedWhere { + where: string; + changed: ChangedRead | undefined; +} + +function findingsLead(projects: readonly ProjectReport[], reads: readonly ChangedWhere[], findings: number, touched: number, unfinished: readonly string[]): string[] { if (findings === 0) { if (unfinished.length > 0 || projects.some((p) => "unread" in p.gaps)) return ["Coherence could not check everything, so this might not list every finding.", ""]; return ["Nothing to fix.", ""]; } - const changed = projects.filter((p) => p.changed !== undefined); + const changed = reads.filter((r) => r.changed !== undefined); if (changed.length === 0) return ["These findings do not fail this check. Some can come from files this pull request did not change.", ""]; const lines = ["These findings do not fail this check.", ""]; - const allRead = changed.every((p) => "files" in p.changed!); - for (const p of changed) if ("unread" in p.changed!) lines.push(`Coherence could not tell which files this pull request changed${p.name === "." ? "" : ` in ${cell(p.name)}`}: ${cell(p.changed.unread)}.`, ""); - if (changed.some((p) => "files" in p.changed!)) { + const allRead = changed.every((r) => "files" in r.changed!); + for (const r of changed) if ("unread" in r.changed!) lines.push(`Coherence could not tell which files this pull request changed${r.where}: ${cell(r.changed.unread)}.`, ""); + if (changed.some((r) => "files" in r.changed!)) { if (touched === 0) lines.push(allRead ? "None of these findings are in files this pull request changed." : "None of the findings Coherence could check are in files this pull request changed.", ""); else if (touched === 1) lines.push("1 of these findings is in a file this pull request changed. It is marked below.", ""); else lines.push(`${touched} of these findings are in files this pull request changed. They are marked below.`, ""); @@ -233,7 +239,9 @@ export function renderReport( const guards = projects.reduce((n, p) => n + (p.model.defects?.guardFailures.length ?? 0), 0); const head = [REPORT_MARKER, "## Coherence", ""]; head.push(`**${plural(problems, "spec problem")} · ${plural(fails, "failing chokepoint")} · ${plural(gaps, "spec gap")} · ${plural(guards, "guard failure")}**${projects.length > 1 ? ` across ${plural(projects.length, "project")}` : ""}`, ""); - head.push(...findingsLead(projects, problems + fails + gaps + guards, touched, unfinished)); + const reads: ChangedWhere[] = projects.map((p) => ({ where: p.name === "." ? "" : ` in ${cell(p.name)}`, changed: p.changed })); + if (registry !== undefined && registry.problems.length > 0) reads.push({ where: " at the registry top", changed: registry.changed }); + head.push(...findingsLead(projects, reads, problems + fails + gaps + guards, touched, unfinished)); if (unfinished.length > 0) { head.push("> [!WARNING]", "> Coherence did not finish, so this report is incomplete and the check fails:"); for (const line of unfinished) head.push(`> - ${cell(line)}`); diff --git a/src/lifecycle/ci.test.ts b/src/lifecycle/ci.test.ts index 691c9e8a..76c60e72 100644 --- a/src/lifecycle/ci.test.ts +++ b/src/lifecycle/ci.test.ts @@ -350,7 +350,7 @@ describe("ci report says what each finding means and what to do", () => { baselined: ["old"], says: [ /\*\*Spec gaps:\*\* 1 entrance takes input from outside this code \(trust outside or unknown\), and the spec names no check on that input\. 1 more was already open when Coherence was adopted, so it is not listed\.\n\n- `A\.spec\.md` entrance \*\*webhook\*\* \(trust outside\), route: \. -\\> store\n/, - /\nTo close a gap, write in its spec what checks that input\. In spec terms: declare guard: where a verified chokepoint wraps it, name it on the entrances: line of an invariant whose crossing enters from its trust, or record control: none — \. `coherence scaffold control ""` proposes the line\.\n/, + /\nTo close a gap, write in its spec what checks that input\. In spec terms: declare guard: \\ where a verified chokepoint wraps it, name it on the entrances: line of an invariant whose crossing enters from its trust, or record control: none — \\\. `coherence scaffold control ""` proposes the line\.\n/, ], }, { @@ -416,6 +416,31 @@ describe("ci report says what each finding means and what to do", () => { assert.match(unchangedReport.markdown, /- `coherence\.config\.json` projects is a list of folders relative to the repository top\n/); }); + test("a registry top whose changed files git could not list says so", () => { + const root = repository({ "A.spec.md": SPEC("a") }); + const project = { name: "products/a", model: loadSpecModel(root), runs: [record("ci-7", [{ name: "a door" }])], gaps: { gaps: [] }, baselined: new Set(), changed: { files: new Set() } }; + const report = renderReport([project], "ci-7", { problems: [{ file: "coherence.config.json", message: "lists a folder that is not there" }], changed: { unread: "git diff main failed" } }); + assert.match(report.markdown, /\n\nCoherence could not tell which files this pull request changed at the registry top: git diff main failed\.\n/); + assert.match(report.markdown, /\n\nNone of the findings Coherence could check are in files this pull request changed\.\n/); + }); + + test("a file name git would quote still matches its finding", async () => { + const root = repository({ "A.spec.md": SPEC("a") }); + const git = (...args: string[]): string => execFileSync("git", ["-c", "user.name=t", "-c", "user.email=t@t", ...args], { cwd: root, encoding: "utf8" }).trim(); + rmSync(join(root, ".git"), { recursive: true }); + git("init", "-q"); + git("add", "-A"); + git("commit", "-q", "-m", "base"); + const base = git("rev-parse", "HEAD"); + writeFileSync(join(root, "Café.spec.md"), "# café\n"); + git("add", "-A"); + git("commit", "-q", "-m", "a second spec"); + appendRun(root, record("ci-7", [{ name: "a door" }])); + const said = capture(root); + assert.equal(await ciReportCommand(["--session", "ci-7", "--base", base], said.io, async () => ({ gaps: [] }), {}), 0); + assert.match(said.out.join("\n"), /- `Café\.spec\.md:1` [^\n]+ · \*\*changed in this pull request\*\*/); + }); + test("ci report --base, or a pull request's base branch on GitHub Actions, reads the files changed since that commit", async () => { const root = repository({ "A.spec.md": SPEC("a") }); const git = (...args: string[]): string => execFileSync("git", ["-c", "user.name=t", "-c", "user.email=t@t", ...args], { cwd: root, encoding: "utf8" }).trim(); diff --git a/src/observation/observed.ts b/src/observation/observed.ts index 7c03d058..4ce9e3da 100644 --- a/src/observation/observed.ts +++ b/src/observation/observed.ts @@ -121,9 +121,9 @@ export function answerFailures(records: readonly ObservationRecord[], head: stri /** Files changed since a commit: the diff to the working tree, and untracked files. */ export function changedSince(root: string, commit: string): string[] | string { - const diff = spawnSync("git", ["diff", "--name-only", "--relative", commit, "--"], { cwd: root, encoding: "utf8" }); + const diff = spawnSync("git", ["-c", "core.quotePath=false", "diff", "--name-only", "--relative", commit, "--"], { cwd: root, encoding: "utf8" }); if (diff.status !== 0) return `git diff ${commit} failed: ${diff.stderr.trim()}`; - const untracked = spawnSync("git", ["ls-files", "--others", "--exclude-standard"], { cwd: root, encoding: "utf8" }); + const untracked = spawnSync("git", ["-c", "core.quotePath=false", "ls-files", "--others", "--exclude-standard"], { cwd: root, encoding: "utf8" }); return [...new Set([...diff.stdout.split("\n"), ...(untracked.status === 0 ? untracked.stdout.split("\n") : [])].filter((l) => l.trim() !== ""))].sort(); } From 60770e20e481e851a6e3fc7ca446a7bdc3cce207 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Sun, 25 Oct 2026 19:27:50 +0000 Subject: [PATCH 09/10] fix(adapters): a broken pipe to the language server fails the client, not the process JsonRpcClient listened for errors on the child process but not on its stdin. When the server had already gone, the TypeScript adapter's exit notification hit EPIPE on a stream with no error listener, an uncaught exception that failed server.test.ts in CI at random. Exit, process errors and stdin errors now share one fail() path. Co-Authored-By: Claude Opus 5.5 Generated-By: PostHog Desktop Task-Id: 4189fcb8-6fd5-428d-8c5f-4314d28e4dac --- .../runs/pr-report-words.fca8b5cf.jsonl | 3 +++ src/adapters/Adapters.spec.md | 7 ++++++ src/adapters/jsonrpc.test.ts | 23 +++++++++++++++++++ src/adapters/jsonrpc.ts | 19 ++++++++------- 4 files changed, 42 insertions(+), 10 deletions(-) create mode 100644 .coherence/runs/pr-report-words.fca8b5cf.jsonl create mode 100644 src/adapters/jsonrpc.test.ts diff --git a/.coherence/runs/pr-report-words.fca8b5cf.jsonl b/.coherence/runs/pr-report-words.fca8b5cf.jsonl new file mode 100644 index 00000000..7b245b71 --- /dev/null +++ b/.coherence/runs/pr-report-words.fca8b5cf.jsonl @@ -0,0 +1,3 @@ +{"at":"2026-10-25T19:27:00.086Z","session":"pr-report-words","agent":"claude","binding":"none: this session owns no active work order","commit":"7e9f108","dirty":true,"instrument":{"language":"typescript","server":"none"},"latency":34242,"load":{"average":0.32,"cores":8},"batch":{"ms":33764},"invariants":[{"component":"src/adapters","name":"a broken pipe is a server gone","form":"totality oracle","verdict":"fail","refutation":"missing","bypasses":[],"testReferences":0,"files":[],"latency":33764,"reason":"1 of 1 tests under \"a write to a server that closed its input fails the client, never the process\" failed: a write to a server that closed its input fails the client, never the process","testMs":308,"mode":"batched","state":"requirement","enforces":["a write to a server that closed its input fails the client, never the process"]}]} +{"kind":"refutation","at":"2026-10-25T19:27:00.541Z","session":"pr-report-words","agent":"claude","component":"src/adapters","name":"a broken pipe is a server gone","form":"totality oracle","broke":"removed the stdin error listener in jsonrpc.ts","verdict":"fail","reason":"1 of 1 tests under \"a write to a server that closed its input fails the client, never the process\" failed: a write to a server that closed its input fails the client, never the process","commit":"7e9f108","dirty":true} +{"at":"2026-10-25T19:27:34.683Z","session":"pr-report-words","agent":"claude","binding":"none: this session owns no active work order","commit":"7e9f108","dirty":true,"instrument":{"language":"typescript","server":"none"},"latency":33538,"load":{"average":0.76,"cores":8},"batch":{"ms":33045},"invariants":[{"component":"src/adapters","name":"a broken pipe is a server gone","form":"totality oracle","verdict":"pass","refutation":"witnessed","bypasses":[],"testReferences":0,"files":[],"latency":33045,"reason":"1 test under \"a write to a server that closed its input fails the client, never the process\" passed in one invocation of node --disable-warning=ExperimentalWarning --import=./src/enforcement/leak-guard.ts --test --test-concurrency=1 --test-reporter=./src/enforcement/node-test-reporter.ts --test-reporter-destination= --test-name-pattern=<1 names> src/**/*.test.ts src/*.e2e.ts","testMs":608,"mode":"batched","state":"invariant","enforces":["a write to a server that closed its input fails the client, never the process"]}]} diff --git a/src/adapters/Adapters.spec.md b/src/adapters/Adapters.spec.md index d07f9a72..f60488ec 100644 --- a/src/adapters/Adapters.spec.md +++ b/src/adapters/Adapters.spec.md @@ -19,6 +19,13 @@ The language adapter seam: how to ask a language's server for definitions, refer crossing: project-source -> instrument refuted: left an open document to the file watcher on forget, as the adapter did before -> "the check reads the current disk text after a forget, with the instrument's watcher blind to the file: a loop of edit-then-check yields zero wrong verdicts" went red in enforcement.test.ts with two of six verdicts answered from pre-edit text; restored, green (2026-09-18) kinds: none +- a broken pipe is a server gone: A write the language server can no longer read fails the client the way the server exiting does, rejecting every pending request and refusing new ones, and never raises an uncaught error in the process that talks to it. + over: a server that closes its input while it still runs, then notifications and a request sent to it + via: a write to a server that closed its input fails the client, never the process + because: the TypeScript adapter's close sends exit after shutdown; when the server had already gone, the write's EPIPE went to a stream with no error listener and crashed a CI test run in code the run was not about + crossing: instrument -> instrument + refuted: removed the stdin error listener in jsonrpc.ts -> "a write to a server that closed its input fails the client, never the process" went red in jsonrpc.test.ts with an uncaught EPIPE; restored byte for byte, green (2026-10-25) + kinds: none - every site is a reference: Every site the language server reports for the protected thing is a reference, an import or re-export specifier included; the adapter reports the syntactic form it read at the site and never exempts one, and the check alone decides what the form means. over: every reference site either adapter reports, import and export specifiers included via: every site the language server reports is a reference: a bypass beneath a semicolon-less bare import is a bypass, and an import outside the chokepoint is one too diff --git a/src/adapters/jsonrpc.test.ts b/src/adapters/jsonrpc.test.ts new file mode 100644 index 00000000..cf2ddc90 --- /dev/null +++ b/src/adapters/jsonrpc.test.ts @@ -0,0 +1,23 @@ +/** + * The JSON-RPC client over a language server's stdio: a server that stops + * reading is a server gone, never an uncaught error in the process that + * talks to it. + */ + +import assert from "node:assert/strict"; +import { tmpdir } from "node:os"; +import { test } from "node:test"; +import { JsonRpcClient } from "./jsonrpc.ts"; + +test("a write to a server that closed its input fails the client, never the process", async () => { + const client = JsonRpcClient.spawn(process.execPath, ["-e", "require(\"node:fs\").closeSync(0); setInterval(() => {}, 1000);"], tmpdir()); + try { + await new Promise((resolve) => setTimeout(resolve, 300)); + for (let i = 0; i < 5; i++) client.notify("exit", null); + await new Promise((resolve) => setTimeout(resolve, 300)); + await assert.rejects(client.request("shutdown", null, 1_000), /EPIPE|exited|write after end/); + assert.equal(client.alive, false, "a broken pipe is a server gone"); + } finally { + client.kill(); + } +}); diff --git a/src/adapters/jsonrpc.ts b/src/adapters/jsonrpc.ts index 77b6a266..dd5db50d 100644 --- a/src/adapters/jsonrpc.ts +++ b/src/adapters/jsonrpc.ts @@ -41,16 +41,15 @@ export class JsonRpcClient { constructor(child: ChildProcess) { this.child = child; child.stdout!.on("data", (chunk: Buffer) => this.receive(chunk)); - child.on("exit", (code, signal) => { - this.exited = new Error(`language server exited (${code ?? signal})`); - for (const p of this.pending.values()) p.reject(this.exited); - this.pending.clear(); - }); - child.on("error", (error) => { - this.exited = error; - for (const p of this.pending.values()) p.reject(error); - this.pending.clear(); - }); + child.on("exit", (code, signal) => this.fail(new Error(`language server exited (${code ?? signal})`))); + child.on("error", (error) => this.fail(error)); + child.stdin!.on("error", (error) => this.fail(error)); + } + + private fail(error: Error): void { + this.exited ??= error; + for (const p of this.pending.values()) p.reject(error); + this.pending.clear(); } static spawn(command: string, args: string[], cwd: string, env?: Record): JsonRpcClient { From 9f9e5d0204d4bab92b0d4a6f91d3f8f1e4106c74 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Sun, 25 Oct 2026 19:38:26 +0000 Subject: [PATCH 10/10] fix(adapters): end the language server when its input pipe fails A pipe error marks the client not alive, and callers such as the Rust adapter's close() and the restart paths kill only a live client, so a server that closed its input but kept running was left behind. The client now kills the child when its input pipe fails. Co-Authored-By: Claude Opus 5.5 Generated-By: PostHog Desktop Task-Id: 4189fcb8-6fd5-428d-8c5f-4314d28e4dac --- .coherence/runs/pr-report-words.e0c74a81.jsonl | 3 +++ src/adapters/Adapters.spec.md | 7 ++++--- src/adapters/jsonrpc.test.ts | 5 ++++- src/adapters/jsonrpc.ts | 5 ++++- 4 files changed, 15 insertions(+), 5 deletions(-) create mode 100644 .coherence/runs/pr-report-words.e0c74a81.jsonl diff --git a/.coherence/runs/pr-report-words.e0c74a81.jsonl b/.coherence/runs/pr-report-words.e0c74a81.jsonl new file mode 100644 index 00000000..ba17a631 --- /dev/null +++ b/.coherence/runs/pr-report-words.e0c74a81.jsonl @@ -0,0 +1,3 @@ +{"at":"2026-10-25T19:37:40.228Z","session":"pr-report-words","agent":"claude","binding":"none: this session owns no active work order","commit":"60770e2","dirty":true,"instrument":{"language":"typescript","server":"none"},"latency":34883,"load":{"average":0.37,"cores":8},"batch":{"ms":34386},"invariants":[{"component":"src/adapters","name":"a broken pipe is a server gone","form":"totality oracle","verdict":"fail","refutation":"witnessed","bypasses":[],"testReferences":0,"files":[],"latency":34386,"reason":"1 of 1 tests under \"a write to a server that closed its input fails the client and ends the server, never the process\" failed: a write to a server that closed its input fails the client and ends the server, never the process","testMs":911,"mode":"batched","state":"structural defect","enforces":["a write to a server that closed its input fails the client and ends the server, never the process"]}]} +{"kind":"refutation","at":"2026-10-25T19:37:40.696Z","session":"pr-report-words","agent":"claude","component":"src/adapters","name":"a broken pipe is a server gone","form":"totality oracle","broke":"left the server running after its input pipe failed in jsonrpc.ts","verdict":"fail","reason":"1 of 1 tests under \"a write to a server that closed its input fails the client and ends the server, never the process\" failed: a write to a server that closed its input fails the client and ends the server, never the process","commit":"60770e2","dirty":true} +{"at":"2026-10-25T19:38:16.389Z","session":"pr-report-words","agent":"claude","binding":"none: this session owns no active work order","commit":"60770e2","dirty":true,"instrument":{"language":"typescript","server":"none"},"latency":35073,"load":{"average":0.85,"cores":8},"batch":{"ms":34558},"invariants":[{"component":"src/adapters","name":"a broken pipe is a server gone","form":"totality oracle","verdict":"pass","refutation":"witnessed","bypasses":[],"testReferences":0,"files":[],"latency":34558,"reason":"1 test under \"a write to a server that closed its input fails the client and ends the server, never the process\" passed in one invocation of node --disable-warning=ExperimentalWarning --import=./src/enforcement/leak-guard.ts --test --test-concurrency=1 --test-reporter=./src/enforcement/node-test-reporter.ts --test-reporter-destination= --test-name-pattern=<1 names> src/**/*.test.ts src/*.e2e.ts","testMs":909,"mode":"batched","state":"invariant","enforces":["a write to a server that closed its input fails the client and ends the server, never the process"]}]} diff --git a/src/adapters/Adapters.spec.md b/src/adapters/Adapters.spec.md index f60488ec..41204701 100644 --- a/src/adapters/Adapters.spec.md +++ b/src/adapters/Adapters.spec.md @@ -19,12 +19,13 @@ The language adapter seam: how to ask a language's server for definitions, refer crossing: project-source -> instrument refuted: left an open document to the file watcher on forget, as the adapter did before -> "the check reads the current disk text after a forget, with the instrument's watcher blind to the file: a loop of edit-then-check yields zero wrong verdicts" went red in enforcement.test.ts with two of six verdicts answered from pre-edit text; restored, green (2026-09-18) kinds: none -- a broken pipe is a server gone: A write the language server can no longer read fails the client the way the server exiting does, rejecting every pending request and refusing new ones, and never raises an uncaught error in the process that talks to it. +- a broken pipe is a server gone: A write the language server can no longer read fails the client the way the server exiting does, rejecting every pending request and refusing new ones, ends the server process, since no caller kills a client that is no longer alive, and never raises an uncaught error in the process that talks to it. over: a server that closes its input while it still runs, then notifications and a request sent to it - via: a write to a server that closed its input fails the client, never the process + via: a write to a server that closed its input fails the client and ends the server, never the process because: the TypeScript adapter's close sends exit after shutdown; when the server had already gone, the write's EPIPE went to a stream with no error listener and crashed a CI test run in code the run was not about crossing: instrument -> instrument - refuted: removed the stdin error listener in jsonrpc.ts -> "a write to a server that closed its input fails the client, never the process" went red in jsonrpc.test.ts with an uncaught EPIPE; restored byte for byte, green (2026-10-25) + refuted: removed the stdin error listener in jsonrpc.ts -> the test, then named "a write to a server that closed its input fails the client, never the process", went red in jsonrpc.test.ts with an uncaught EPIPE; restored byte for byte, green (2026-10-25) + refuted: left the server running after its input pipe failed in jsonrpc.ts -> "a write to a server that closed its input fails the client and ends the server, never the process" went red in jsonrpc.test.ts, the process still there; restored byte for byte, green (2026-10-25) kinds: none - every site is a reference: Every site the language server reports for the protected thing is a reference, an import or re-export specifier included; the adapter reports the syntactic form it read at the site and never exempts one, and the check alone decides what the form means. over: every reference site either adapter reports, import and export specifiers included diff --git a/src/adapters/jsonrpc.test.ts b/src/adapters/jsonrpc.test.ts index cf2ddc90..60d19c6b 100644 --- a/src/adapters/jsonrpc.test.ts +++ b/src/adapters/jsonrpc.test.ts @@ -9,7 +9,7 @@ import { tmpdir } from "node:os"; import { test } from "node:test"; import { JsonRpcClient } from "./jsonrpc.ts"; -test("a write to a server that closed its input fails the client, never the process", async () => { +test("a write to a server that closed its input fails the client and ends the server, never the process", async () => { const client = JsonRpcClient.spawn(process.execPath, ["-e", "require(\"node:fs\").closeSync(0); setInterval(() => {}, 1000);"], tmpdir()); try { await new Promise((resolve) => setTimeout(resolve, 300)); @@ -17,6 +17,9 @@ test("a write to a server that closed its input fails the client, never the proc await new Promise((resolve) => setTimeout(resolve, 300)); await assert.rejects(client.request("shutdown", null, 1_000), /EPIPE|exited|write after end/); assert.equal(client.alive, false, "a broken pipe is a server gone"); + const pid = client.pid!; + await new Promise((resolve) => setTimeout(resolve, 300)); + assert.throws(() => process.kill(pid, 0), /ESRCH/, "the server whose input broke is ended, since no caller will kill a client that is not alive"); } finally { client.kill(); } diff --git a/src/adapters/jsonrpc.ts b/src/adapters/jsonrpc.ts index dd5db50d..930236e9 100644 --- a/src/adapters/jsonrpc.ts +++ b/src/adapters/jsonrpc.ts @@ -43,7 +43,10 @@ export class JsonRpcClient { child.stdout!.on("data", (chunk: Buffer) => this.receive(chunk)); child.on("exit", (code, signal) => this.fail(new Error(`language server exited (${code ?? signal})`))); child.on("error", (error) => this.fail(error)); - child.stdin!.on("error", (error) => this.fail(error)); + child.stdin!.on("error", (error) => { + this.fail(error); + this.kill(); + }); } private fail(error: Error): void {