diff --git a/.coherence/runs/pr-report-words.0f6f8353.jsonl b/.coherence/runs/pr-report-words.0f6f8353.jsonl new file mode 100644 index 0000000..e8abff3 --- /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/.coherence/runs/pr-report-words.656f4efb.jsonl b/.coherence/runs/pr-report-words.656f4efb.jsonl new file mode 100644 index 0000000..ee8570c --- /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/.coherence/runs/pr-report-words.e0c74a81.jsonl b/.coherence/runs/pr-report-words.e0c74a81.jsonl new file mode 100644 index 0000000..ba17a63 --- /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/.coherence/runs/pr-report-words.e2255a45.jsonl b/.coherence/runs/pr-report-words.e2255a45.jsonl new file mode 100644 index 0000000..9574cf9 --- /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/.coherence/runs/pr-report-words.fca8b5cf.jsonl b/.coherence/runs/pr-report-words.fca8b5cf.jsonl new file mode 100644 index 0000000..7b245b7 --- /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/README.md b/README.md index e8a7d39..e81c494 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 (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 ``` @@ -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/mods/coherence/.claude-plugin/plugin.json b/mods/coherence/.claude-plugin/plugin.json index 10f5195..c1b76ba 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 7fe2b6c..ffaca23 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 b3e6a88..6aa5b73 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/plugins/declared/plugin.json b/plugins/declared/plugin.json index 5005f6a..d651cb8 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 e105cef..c8ee2d6 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 f67111e..bb2ffeb 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 1b67f53..d6b107e 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" diff --git a/src/adapters/Adapters.spec.md b/src/adapters/Adapters.spec.md index d07f9a7..4120470 100644 --- a/src/adapters/Adapters.spec.md +++ b/src/adapters/Adapters.spec.md @@ -19,6 +19,14 @@ 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, 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 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 -> 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 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 0000000..60d19c6 --- /dev/null +++ b/src/adapters/jsonrpc.test.ts @@ -0,0 +1,26 @@ +/** + * 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 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)); + 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"); + 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 77b6a26..930236e 100644 --- a/src/adapters/jsonrpc.ts +++ b/src/adapters/jsonrpc.ts @@ -41,18 +41,20 @@ 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); + this.kill(); }); } + 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 { const child = spawn(command, args, { cwd, stdio: ["pipe", "pipe", "ignore"], ...(env === undefined ? {} : { env: { ...process.env, ...env } }) }); return new JsonRpcClient(child); diff --git a/src/cli.ts b/src/cli.ts index 838f40d..aa8c21a 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 1227ac8..36ee5b9 100644 --- a/src/lifecycle/Lifecycle.spec.md +++ b/src/lifecycle/Lifecycle.spec.md @@ -1283,6 +1283,14 @@ 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 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 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) + 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 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 0e81f3b..9b31144 100644 --- a/src/lifecycle/ci-report.ts +++ b/src/lifecycle/ci-report.ts @@ -11,14 +11,15 @@ * 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"; import { CI_USAGE, REPORT_MARKER } from "./ci.ts"; import { mayHaveGaps } from "./hook.ts"; @@ -26,6 +27,14 @@ 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 }; + +/** 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; @@ -51,6 +60,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,18 +101,27 @@ 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 = changedFiles(p.changed); + 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)}`, ""); - 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})`, ""); - for (const problem of p.model.problems) lines.push(`- \`${problem.file}:${problem.line}\` ${cell(problem.message)}`); + 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)}${mark(problem.file)}`); lines.push(""); } @@ -115,8 +135,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 — 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)}`); + 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`); } @@ -129,19 +154,28 @@ 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(" -> "))}${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: ${cell(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}`, ""); + 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(""); } - return { lines, unfinished }; + return { lines, unfinished, touched }; } /** How many bypass sites one verdict lists before it counts the rest. */ @@ -152,21 +186,62 @@ 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. */ +/** 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 = 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((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.`, ""); + } + return lines; +} + +/** Whether a registry problem's file is one this pull request changed. */ +function registryMark(problem: { file: string }, changed: ChangedRead | undefined): string { + 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. */ -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; 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 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")}` : ""}`, ""); + 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)}`); @@ -174,7 +249,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"; @@ -182,7 +257,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. @@ -199,18 +274,43 @@ 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); + 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 : 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 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; + 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; @@ -220,7 +320,8 @@ 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); + 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}`); return report.unfinished.length > 0 ? 1 : 0; diff --git a/src/lifecycle/ci.test.ts b/src/lifecycle/ci.test.ts index 4ed8539..76c60e7 100644 --- a/src/lifecycle/ci.test.ts +++ b/src/lifecycle/ci.test.ts @@ -7,16 +7,18 @@ */ 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"; 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 { 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"; @@ -280,9 +282,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 +305,181 @@ 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[^\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, changed, 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 — 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"), + 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, 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/, + ], + }, + { + 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/] }, + { + 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 }); + const model = loadSpecModel(root); + 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("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("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(); + 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); + 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 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"); + }); +}); + 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/lifecycle/ci.ts b/src/lifecycle/ci.ts index ad30ddd..a79f51f 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 (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. */ diff --git a/src/observation/observed.ts b/src/observation/observed.ts index 7c03d05..4ce9e3d 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(); } diff --git a/src/spec/grammar.ts b/src/spec/grammar.ts index 77cf1f2..0188cbb 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 88acfb7..00e9bc2 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 80dcaca..be67ef3 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.` }); } }