diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a1dfea0..9d60853 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,18 +5,29 @@ on: branches: ["**"] pull_request: +# CI needs to read the repo, nothing more. +permissions: + contents: read + jobs: build: - name: typecheck · test · build + name: typecheck · lint · test · build (node ${{ matrix.node }}) runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + # engines says >=20 and the release workflow builds on 24 — test what + # users actually run, not only the floor. + node: ["20", "22", "24"] steps: - uses: actions/checkout@v4 - uses: actions/setup-node@v4 with: - node-version: "20" + node-version: ${{ matrix.node }} cache: npm - run: npm ci - run: npm run typecheck + - run: npm run lint - run: npm test - run: npm run build diff --git a/.github/workflows/dogfood.yml b/.github/workflows/dogfood.yml index 6afdc15..0cebb4c 100644 --- a/.github/workflows/dogfood.yml +++ b/.github/workflows/dogfood.yml @@ -11,6 +11,10 @@ on: branches: ["**"] pull_request: +# CI needs to read the repo, nothing more. +permissions: + contents: read + jobs: coverage: name: self-coverage (proofkeeper verifies proofkeeper) @@ -29,11 +33,16 @@ jobs: # install it as an external CLI from rac-core (its distribution is # "requirements-as-code"; the console script is `rac`). Proofkeeper consumes # only its published `rac export --graph` JSON output. + # + # Pinned to a commit: an unpinned default-branch install would execute + # whatever lands on rac-core main in this CI on every push — and make the + # gate non-reproducible. Bump the SHA deliberately when adopting a newer + # contract. - uses: actions/setup-python@v5 with: python-version: "3.11" - name: Install the rac engine (provides the `rac` CLI) - run: pip install "git+https://github.com/itsthelore/rac-core.git" + run: pip install "git+https://github.com/itsthelore/rac-core.git@76e8feff9ae508cfbd4bc9a861c408901920da1d" - name: Proofkeeper coverage over its own corpus (fails on any unverified) run: node dist/cli.js coverage --corpus lore-proofkeeper/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 98e1ac1..f8024e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,16 @@ match exactly, or the workflow fails the publish. summary), and `--verbose` streams a per-turn audit trail (tool calls, errors, model latency) to stderr. +- **Hardened delivery.** The Dogfood gate installs the rac engine pinned to a + commit (reproducible CI, no default-branch supply-chain exposure); CI runs + at least privilege on Node 20/22/24 with a new lint gate (Biome). Config + typos now fail at parse time with the offender named (duplicate capability + ids, unknown environments/personas/defaultTarget) instead of silently + driving the wrong URL. Write-back re-runs are idempotent: an existing head + branch is re-pointed, an already-open PR is returned instead of a 422, and + the marked PR comment is found past one hundred comments. `## Verified By` + paths are always written POSIX-style. + ## 2026.07.1 — the "any model" release The release that makes **bring-your-own-model** mean *any* model — and proves Proofkeeper on itself. Everything since the first cut: diff --git a/biome.json b/biome.json new file mode 100644 index 0000000..b00a4ac --- /dev/null +++ b/biome.json @@ -0,0 +1,42 @@ +{ + "$schema": "https://biomejs.dev/schemas/2.5.2/schema.json", + "vcs": { + "enabled": true, + "clientKind": "git", + "useIgnoreFile": true + }, + "files": { + "includes": ["src/**", "tests/**", "!tests/fixtures/**"] + }, + "formatter": { + "enabled": false + }, + "linter": { + "enabled": true, + "rules": { + "preset": "recommended", + "complexity": { + "useLiteralKeys": "off" + }, + "style": { + "noNonNullAssertion": "off", + "useTemplate": "off" + } + } + }, + "assist": { + "enabled": false + }, + "overrides": [ + { + "includes": ["tests/compiler-emit.test.ts"], + "linter": { + "rules": { + "suspicious": { + "noTemplateCurlyInString": "off" + } + } + } + } + ] +} diff --git a/lore-proofkeeper/designs/design-delivery-hygiene.md b/lore-proofkeeper/designs/design-delivery-hygiene.md new file mode 100644 index 0000000..941004d --- /dev/null +++ b/lore-proofkeeper/designs/design-delivery-hygiene.md @@ -0,0 +1,105 @@ +--- +schema_version: 1 +id: PK-KWFX3KXWBRKD +type: design +--- +# Delivery Hygiene — Pinning, Permissions, Lint, and Idempotent Re-Runs + +## Context + +The final batch of a four-part review response: the first three hardened the +agent loop and its verdicts; this one hardens how Proofkeeper itself is built, +checked, and re-run. Everything here is process and edge hardening — no drive +or verdict semantics change. + +## User Need + +A maintainer must be able to trust that CI results are reproducible (not a +function of another repository's default branch), that the published package +works on the Node versions users actually run, that re-running a write-back +cannot fail or spam, and that a config typo is caught before a drive targets +the wrong URL. + +## Design + +- **Supply chain.** `dogfood.yml` installs rac-core at a commit SHA with a + comment stating the bump policy; both CI workflows gain + `permissions: contents: read`. +- **Matrix + lint.** The build job fans out over Node 20/22/24 and gains a + `npm run lint` step. Lint is Biome (single pinned dev dependency, + lint-only — the formatter stays off). Two recommended rules are disabled as + deliberate house style: bracket access on `Record` + (`useLiteralKeys`) and post-validation non-null assertions + (`noNonNullAssertion`); `useTemplate` is off to avoid churn. The remaining + recommended set is enforced at zero diagnostics. +- **Config cross-validation.** `parseConfig` ends with a `validateReferences` + pass: duplicate capability ids, unknown `defaultTarget`, unknown capability + `environment`, and unknown `persona` references all raise `ConfigParseError` + naming the offender. `resolveTarget`'s fallback remains for + programmatically built configs, but a parsed config can no longer reach it + with a typo. +- **Idempotent GitHub layer.** `createBranch` catches the 422 "already + exists" and force-updates the existing head ref to the fresh base; + `openPullRequest` catches the 422 and returns the already-open pull request + for that head; `listComments` pages until a short page. All three keep + their error behavior for every other failure. +- **Portable references.** `linksFromResults` POSIX-normalizes spec and trace + paths before they become corpus `## Verified By` content. +- `parseCoverageArgs` uses the same `requireValue` guard as every other + parser. + +## Constraints + +- The lint baseline must hold at zero diagnostics — rules are configured off + explicitly rather than tolerated as warnings. +- Idempotency handling matches on GitHub's documented 422 semantics only; + unknown errors still throw. +- No behavior change for configs and write-backs that were already + well-formed. + +## Rationale + +Pinning by SHA (not tag) makes the dogfood gate exactly reproducible and +makes adopting a new contract an explicit, reviewable diff. Biome over +ESLint: one pinned binary, no plugin graph — a smaller supply-chain surface in +the same PR that pins the other one. Parse-time reference validation follows +the engine's own philosophy: refuse with a named offender rather than degrade +silently. + +## Alternatives + +- **Renovate/Dependabot-style automated pin bumps.** Deferred: process + machinery beyond this change's scope; the manual bump comment suffices at + current cadence. +- **ESLint + typescript-eslint.** Rejected for now: larger dependency + surface; the strict tsconfig already covers most type-adjacent rules. +- **Deleting stale head branches instead of re-pointing.** Rejected: + re-pointing preserves an open pull request and its review thread. + +## Accessibility + +Not applicable — CI configuration and internal validation; user surface is +error text that names the fix. + +## Style Guidance + +Validation errors name the artifact and the offending reference +("capability 'A' references undefined environment 'prod'") so the fix needs no +searching. + +## Open Questions + +- Whether the dogfood pin should move to a rac-core release tag once CalVer + releases are cut there regularly. + +## Related Requirements + +- req-delivery-hygiene + +## Related Roadmaps + +- autonomous-qa-enhancements + +## Status + +Accepted diff --git a/lore-proofkeeper/requirements/req-delivery-hygiene.md b/lore-proofkeeper/requirements/req-delivery-hygiene.md new file mode 100644 index 0000000..aa3f75e --- /dev/null +++ b/lore-proofkeeper/requirements/req-delivery-hygiene.md @@ -0,0 +1,67 @@ +--- +schema_version: 1 +id: PK-KWFX3K4WPAXN +type: requirement +--- +# Delivery Hygiene — Supply Chain, Lint, Matrix, Idempotent Write-Back + +## Problem + +The delivery pipeline had hardening gaps a review surfaced. The dogfood gate +installed rac-core from its default branch unpinned, executing whatever landed +there in this repository's CI on every push — non-reproducible and a supply +chain exposure. CI workflows ran with the default token scope instead of +least privilege, on a single Node version while the release built on a newer +one. No lint tooling guarded style regressions the type checker cannot see. +Config typos degraded silently at run time (an unknown environment fell back +to the default URL; a duplicate capability id was accepted). And a re-run of +the write-back was fatal rather than idempotent: an existing head branch made +branch creation throw 422, a busy pull request's marked comment beyond the +first hundred was missed and duplicated, and Windows path separators could be +written into corpus `## Verified By` references. + +## Requirements + +- [REQ-001] The dogfood gate installs rac-core pinned to a commit SHA, bumped deliberately; CI workflows carry an explicit least-privilege `permissions` block. +- [REQ-002] The build job runs on every Node major the package supports (20, 22, 24), not only the floor. +- [REQ-003] A lint gate (`npm run lint`) runs in CI alongside typecheck/test/build. +- [REQ-004] Config cross-references fail at parse time with the offender named: duplicate capability ids, a `defaultTarget` or capability `environment` naming no defined environment, and a capability persona naming no defined persona. +- [REQ-005] Write-back re-runs are idempotent at the GitHub layer: an existing head branch is re-pointed, an already-open pull request for the head is returned rather than failed, and comment listing paginates past one hundred. +- [REQ-006] Corpus verifier references are written with POSIX separators regardless of the authoring platform. + +## Success Metrics + +- The dogfood job's install line names a SHA; changing rac-core main does not + change this repository's CI behavior until the pin is bumped. +- `npm run lint` exits 0 locally and in CI; a lint regression fails the build. +- A config with a typo'd environment name is rejected at parse with the + capability and name in the error. +- Running the write-back twice for the same capability yields one branch and + one pull request, updated in place. + +## Risks + +- A pinned rac can drift behind the contract. Mitigation: the pin is visible + in the workflow with a comment saying when to bump; contract changes arrive + as deliberate updates. +- Parse-time reference validation rejects configs that previously "worked" by + fallback. Mitigation: the errors name the fix, and silent wrong-target runs + were the worse failure. + +## Assumptions + +- GitHub's 422 messages for existing refs and pull requests keep containing + "already exists" (the strings matched for idempotent handling). +- Biome's recommended rule set, with the two house-style exceptions recorded + in `biome.json`, is a stable lint baseline. + +## Related Roadmaps + +- autonomous-qa-enhancements + +## Verified By + +- `tests/scope-config.test.ts` +- `tests/github-rest-gateway.test.ts` +- `tests/writeback-proposer.test.ts` +- `tests/qa-command.test.ts` diff --git a/lore-proofkeeper/roadmaps/autonomous-qa-enhancements.md b/lore-proofkeeper/roadmaps/autonomous-qa-enhancements.md index 5ce9587..d6b3bc4 100644 --- a/lore-proofkeeper/roadmaps/autonomous-qa-enhancements.md +++ b/lore-proofkeeper/roadmaps/autonomous-qa-enhancements.md @@ -94,6 +94,13 @@ failure from its siblings, cap observation growth, and make token spend and per-turn conduct visible. Serves the fast-reliable-scoped-QA outcome under real-world failure. +### Delivery hygiene + +Pin the dogfood engine install to a commit, run CI at least privilege across +every supported Node major with a lint gate, fail config typos at parse time +with the offender named, and make write-back re-runs idempotent at the GitHub +layer. Serves the trust outcome for the pipeline itself. + ## Success Measures - A pull request shows exactly one Proofkeeper QA comment regardless of how many diff --git a/package-lock.json b/package-lock.json index 31c091c..762ee5a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@itsthelore/proofkeeper", - "version": "0.0.1", + "version": "2026.07.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@itsthelore/proofkeeper", - "version": "0.0.1", + "version": "2026.07.1", "license": "Apache-2.0", "dependencies": { "@playwright/test": "^1.45.0" @@ -16,6 +16,7 @@ }, "devDependencies": { "@anthropic-ai/sdk": "^0.105.0", + "@biomejs/biome": "2.5.2", "@types/node": "^20.14.0", "tsx": "^4.16.0", "typescript": "^5.5.0", @@ -23,6 +24,14 @@ }, "engines": { "node": ">=20" + }, + "peerDependencies": { + "@anthropic-ai/sdk": ">=0.40.0" + }, + "peerDependenciesMeta": { + "@anthropic-ai/sdk": { + "optional": true + } } }, "node_modules/@anthropic-ai/sdk": { @@ -57,6 +66,169 @@ "node": ">=6.9.0" } }, + "node_modules/@biomejs/biome": { + "version": "2.5.2", + "resolved": "https://registry.npmjs.org/@biomejs/biome/-/biome-2.5.2.tgz", + "integrity": "sha512-VQ3RCqr7JmDIX+w6stWYl+g/3bYofN3q2wDBHUKKc/c7i5QWrFKFBZYCYPWTE6agsUPMIZZe6/CMmVUfUAhkKA==", + "dev": true, + "license": "MIT OR Apache-2.0", + "bin": { + "biome": "bin/biome" + }, + "engines": { + "node": ">=14.21.3" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/biome" + }, + "optionalDependencies": { + "@biomejs/cli-darwin-arm64": "2.5.2", + "@biomejs/cli-darwin-x64": "2.5.2", + "@biomejs/cli-linux-arm64": "2.5.2", + "@biomejs/cli-linux-arm64-musl": "2.5.2", + "@biomejs/cli-linux-x64": "2.5.2", + "@biomejs/cli-linux-x64-musl": "2.5.2", + "@biomejs/cli-win32-arm64": "2.5.2", + "@biomejs/cli-win32-x64": "2.5.2" + } + }, + "node_modules/@biomejs/cli-darwin-arm64": { + "version": "2.5.2", + "resolved": "https://registry.npmjs.org/@biomejs/cli-darwin-arm64/-/cli-darwin-arm64-2.5.2.tgz", + "integrity": "sha512-e7P3P7EkwFc/KiX2AHw4YDLIBOMfG9CPCAwy52k5Bp0dfhkozx9hf6wCmIr2QeXy2XeccJ3V/Sg+hDmzYEqxSg==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-darwin-x64": { + "version": "2.5.2", + "resolved": "https://registry.npmjs.org/@biomejs/cli-darwin-x64/-/cli-darwin-x64-2.5.2.tgz", + "integrity": "sha512-ymzMvjC1Jg0b9K0D26ZdARqFQXs7MocfLC5FOCGfkC0Ss+ACUJkX5364ZM5nT4NLZanHRZNVrZEy+Ibwcvux/g==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-linux-arm64": { + "version": "2.5.2", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-arm64/-/cli-linux-arm64-2.5.2.tgz", + "integrity": "sha512-t7sseOmqND57uUWTwlawU6BYj+J06T/9EkydzBhkrgw/FK3QVhjU2wsJR0frljrKZ0/I8A/rYw7284QgqjQfIQ==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-linux-arm64-musl": { + "version": "2.5.2", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-arm64-musl/-/cli-linux-arm64-musl-2.5.2.tgz", + "integrity": "sha512-w+ANG0ZvTu9IeEg9QnstoOnk6L0fpwJifW6aHR18+cb5Z39bkANItYjAfMrnvce5tmMK+IQ6nPX7/kQFdam5iw==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-linux-x64": { + "version": "2.5.2", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-x64/-/cli-linux-x64-2.5.2.tgz", + "integrity": "sha512-M/lOZrewzTCRDINbjhQ1gYYru37KlD3kJBQwwKCG0ckz5E9IZwIoJ3X0wBwRXA+yBDIwWUuPBHS67HzJY4dTfA==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-linux-x64-musl": { + "version": "2.5.2", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-x64-musl/-/cli-linux-x64-musl-2.5.2.tgz", + "integrity": "sha512-VArNLAzND063tF+XY0yPyM+DyahpzOMzOAvb7qs259nhjJWRjvjZdssuA+Rfl+l07+NOesKZ0Xu2yFrXyBMtzw==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-win32-arm64": { + "version": "2.5.2", + "resolved": "https://registry.npmjs.org/@biomejs/cli-win32-arm64/-/cli-win32-arm64-2.5.2.tgz", + "integrity": "sha512-kbjFFKyZlzYnAuw7sRy5qDoFG6zrP40UK08oPQsWK0ct3NMnGSt+Bs1iviEEyEIP57N5MrykGXdO/wRiaR4lww==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-win32-x64": { + "version": "2.5.2", + "resolved": "https://registry.npmjs.org/@biomejs/cli-win32-x64/-/cli-win32-x64-2.5.2.tgz", + "integrity": "sha512-4InchVpdVmdkkkgjQqKpgvyu+VPnoF/7RPSw5YATgEVpt2j72wcCAeV5TwaE9ZGJUZWZn7v2CwSAj6CrMJEx8A==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">=14.21.3" + } + }, "node_modules/@esbuild/aix-ppc64": { "version": "0.28.1", "resolved": "https://registry.npmjs.org/@esbuild/aix-ppc64/-/aix-ppc64-0.28.1.tgz", diff --git a/package.json b/package.json index f66d177..f616d79 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,8 @@ "typecheck": "tsc -p tsconfig.json --noEmit", "test": "vitest run", "test:watch": "vitest", - "proofkeeper": "node --import tsx ./src/cli.ts" + "proofkeeper": "node --import tsx ./src/cli.ts", + "lint": "biome lint src tests" }, "keywords": [ "lore", @@ -43,6 +44,7 @@ }, "devDependencies": { "@anthropic-ai/sdk": "^0.105.0", + "@biomejs/biome": "2.5.2", "@types/node": "^20.14.0", "tsx": "^4.16.0", "typescript": "^5.5.0", diff --git a/src/cli.ts b/src/cli.ts index 1140b7a..fcad1a6 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -136,16 +136,17 @@ interface CoverageArgs { json: boolean; } -function parseCoverageArgs(argv: string[]): CoverageArgs { +/** Parse `coverage` arguments. Pure and exported so it is unit-testable. */ +export function parseCoverageArgs(argv: string[]): CoverageArgs { const args: CoverageArgs = { json: false }; for (let i = 0; i < argv.length; i++) { const arg = argv[i]; switch (arg) { case "--graph-file": - args.graphFile = argv[++i]; + args.graphFile = requireValue(argv[++i], "--graph-file"); break; case "--corpus": - args.corpus = argv[++i]; + args.corpus = requireValue(argv[++i], "--corpus"); break; case "--json": args.json = true; @@ -160,9 +161,6 @@ function parseCoverageArgs(argv: string[]): CoverageArgs { if (args.graphFile && args.corpus) { throw new UsageError("pass only one of --graph-file or --corpus"); } - if ((args.graphFile !== undefined && !args.graphFile) || (args.corpus !== undefined && !args.corpus)) { - throw new UsageError("missing value for --graph-file/--corpus"); - } return args; } @@ -346,7 +344,8 @@ export function parseQaArgs(argv: string[]): QaArgs { raw.allowShell = true; break; case "--allow-host": - (raw.allowedHosts ??= []).push(requireValue(argv[++i], "--allow-host")); + raw.allowedHosts ??= []; + raw.allowedHosts.push(requireValue(argv[++i], "--allow-host")); break; case "--verbose": raw.verbose = true; diff --git a/src/scope/config.ts b/src/scope/config.ts index f5be473..ed492dd 100644 --- a/src/scope/config.ts +++ b/src/scope/config.ts @@ -203,9 +203,47 @@ export function parseConfig(json: string): ProofkeeperConfig { } config.failureLearning = raw["failureLearning"] !== undefined ? parseFailureLearning(raw["failureLearning"]) : "suggest_in_report"; + validateReferences(config); return config; } +/** + * Cross-reference validation: a typo'd environment or persona name must fail + * at parse time with the artifact named, not silently fall back to a default + * URL (or fail mid-drive) at run time. + */ +function validateReferences(config: ProofkeeperConfig): void { + const seen = new Set(); + for (const cap of config.capabilities) { + if (seen.has(cap.id)) { + throw new ConfigParseError( + `duplicate capability id '${cap.id}' — each capability must appear once`, + ); + } + seen.add(cap.id); + } + + const environments = config.environments ?? {}; + if (config.defaultTarget !== undefined && environments[config.defaultTarget] === undefined) { + throw new ConfigParseError( + `defaultTarget '${config.defaultTarget}' is not a defined environment`, + ); + } + const personas = new Set((config.personas ?? []).map((p) => p.name)); + for (const cap of config.capabilities) { + if (cap.environment !== undefined && environments[cap.environment] === undefined) { + throw new ConfigParseError( + `capability '${cap.id}' references undefined environment '${cap.environment}'`, + ); + } + if (cap.persona !== undefined && !personas.has(cap.persona)) { + throw new ConfigParseError( + `capability '${cap.id}' references undefined persona '${cap.persona}'`, + ); + } + } +} + function parseFailureLearning(raw: unknown): FailureLearningStrategy { if (typeof raw !== "string" || !FAILURE_LEARNING_STRATEGIES.includes(raw as FailureLearningStrategy)) { throw new ConfigParseError(`failureLearning must be one of: ${FAILURE_LEARNING_STRATEGIES.join(", ")}`); diff --git a/src/writeback/comment.ts b/src/writeback/comment.ts index 28c3b2d..13d8515 100644 --- a/src/writeback/comment.ts +++ b/src/writeback/comment.ts @@ -62,7 +62,9 @@ export function renderWriteBackComment(input: { } if (input.steps && input.steps.length > 0) { lines.push("", "Steps exercised:"); - input.steps.forEach((s, i) => lines.push(`${i + 1}. ${s}`)); + input.steps.forEach((s, i) => { + lines.push(`${i + 1}. ${s}`); + }); } const trace = input.links.find((l) => l.trace)?.trace; if (trace) { diff --git a/src/writeback/gateways/github-rest.ts b/src/writeback/gateways/github-rest.ts index fd67fee..616a333 100644 --- a/src/writeback/gateways/github-rest.ts +++ b/src/writeback/gateways/github-rest.ts @@ -80,10 +80,21 @@ export class GitHubRestGateway implements RepoGateway { const ref = (await this.request("GET", this.repoPath(`/git/ref/heads/${fromRef}`))) as { object: { sha: string }; }; - await this.request("POST", this.repoPath("/git/refs"), { - ref: `refs/heads/${name}`, - sha: ref.object.sha, - }); + try { + await this.request("POST", this.repoPath("/git/refs"), { + ref: `refs/heads/${name}`, + sha: ref.object.sha, + }); + } catch (err) { + // A prior write-back run for the same capability may have left the head + // branch behind (its PR still open, or closed unmerged). Re-point it at + // the fresh base instead of failing the whole run with a 422. + if (!(err as Error).message.includes("already exists")) throw err; + await this.request("PATCH", this.repoPath(`/git/refs/heads/${name}`), { + sha: ref.object.sha, + force: true, + }); + } } async commitFile(input: { branch: string; path: string; content: string; message: string }): Promise { @@ -110,13 +121,27 @@ export class GitHubRestGateway implements RepoGateway { url: string; number: number; }> { - const pr = (await this.request("POST", this.repoPath("/pulls"), { - title: input.title, - head: input.head, - base: input.base, - body: input.body, - })) as { html_url: string; number: number }; - return { url: pr.html_url, number: pr.number }; + try { + const pr = (await this.request("POST", this.repoPath("/pulls"), { + title: input.title, + head: input.head, + base: input.base, + body: input.body, + })) as { html_url: string; number: number }; + return { url: pr.html_url, number: pr.number }; + } catch (err) { + // A prior run's PR for this head branch may still be open — the fresh + // commit is already on the branch, so return the existing PR instead of + // failing (re-runs are idempotent, not fatal). + if (!(err as Error).message.includes("already exists")) throw err; + const open = (await this.request( + "GET", + this.repoPath(`/pulls?head=${encodeURIComponent(`${this.owner}:${input.head}`)}&state=open&per_page=1`), + )) as { html_url: string; number: number }[]; + const existing = open[0]; + if (!existing) throw err; + return { url: existing.html_url, number: existing.number }; + } } async commentOnPullRequest(input: { number: number; body: string }): Promise<{ url: string }> { @@ -128,11 +153,17 @@ export class GitHubRestGateway implements RepoGateway { } async listComments(prNumber: number): Promise<{ id: number; body: string }[]> { - const data = (await this.request( - "GET", - this.repoPath(`/issues/${prNumber}/comments?per_page=100`), - )) as { id: number; body?: string }[]; - return data.map((c) => ({ id: c.id, body: c.body ?? "" })); + // Paginate: on a busy pull request the marked comment can sit past the + // first page, and missing it would post a duplicate instead of updating. + const all: { id: number; body: string }[] = []; + for (let page = 1; ; page++) { + const data = (await this.request( + "GET", + this.repoPath(`/issues/${prNumber}/comments?per_page=100&page=${page}`), + )) as { id: number; body?: string }[]; + all.push(...data.map((c) => ({ id: c.id, body: c.body ?? "" }))); + if (data.length < 100) return all; + } } async updateComment(commentId: number, body: string): Promise<{ url: string }> { diff --git a/src/writeback/proposal.ts b/src/writeback/proposal.ts index 7eae6dc..4712c53 100644 --- a/src/writeback/proposal.ts +++ b/src/writeback/proposal.ts @@ -83,7 +83,9 @@ function proposalBody(input: { } if (input.steps && input.steps.length > 0) { lines.push("", "Steps exercised:"); - input.steps.forEach((s, i) => lines.push(`${i + 1}. ${s}`)); + input.steps.forEach((s, i) => { + lines.push(`${i + 1}. ${s}`); + }); } const trace = input.links.find((l) => l.trace)?.trace; if (trace) { @@ -117,14 +119,20 @@ export function buildProposal(input: BuildProposalInput): WriteBackProposal { }; } +/** Normalize a path to POSIX separators — corpus references must be portable. */ +function posixPath(p: string): string { + return p.replaceAll("\\", "/"); +} + /** * Derive verification links from a compiled test and its run results: the * committed spec (the corpus verifier) plus the first replayable trace produced - * for it (surfaced in the PR). + * for it (surfaced in the PR). Paths are POSIX-normalized: a `## Verified By` + * reference written from Windows must still resolve for every other consumer. */ export function linksFromResults(candidate: CandidateTest, results: RunResult[]): VerificationLink[] { const trace = results.find((r) => r.testId === candidate.id && r.tracePath)?.tracePath; - const link: VerificationLink = { test: candidate.specPath }; - if (trace) link.trace = trace; + const link: VerificationLink = { test: posixPath(candidate.specPath) }; + if (trace) link.trace = posixPath(trace); return [link]; } diff --git a/tests/github-rest-gateway.test.ts b/tests/github-rest-gateway.test.ts index d270c24..99b4405 100644 --- a/tests/github-rest-gateway.test.ts +++ b/tests/github-rest-gateway.test.ts @@ -89,7 +89,7 @@ describe("GitHubRestGateway", () => { it("lists PR comments (id + body) via the issues comments endpoint", async () => { const { impl } = fakeFetch({ - "GET /repos/o/r/issues/12/comments?per_page=100": () => ({ + "GET /repos/o/r/issues/12/comments?per_page=100&page=1": () => ({ json: [ { id: 1, body: "hi" }, { id: 2, body: "\nstatus" }, @@ -123,3 +123,42 @@ describe("GitHubRestGateway", () => { await expect(gateway.getFileContent("x", "main")).rejects.toThrow(/403/); }); }); + +describe("GitHubRestGateway — re-run idempotency", () => { + it("re-points an already-existing head branch instead of failing with 422", async () => { + const { impl, calls } = fakeFetch({ + "GET /repos/o/r/git/ref/heads/main": () => ({ json: { object: { sha: "abc" } } }), + "POST /repos/o/r/git/refs": () => ({ status: 422, json: { message: "Reference already exists" } }), + "PATCH /repos/o/r/git/refs/heads/proofkeeper/verified-by/req-b": () => ({ json: {} }), + }); + const gateway = new GitHubRestGateway({ owner: "o", repo: "r", token: "t", fetch: impl }); + await gateway.createBranch("proofkeeper/verified-by/req-b", "main"); + expect(calls.some((c) => c.method === "PATCH" && c.url.includes("/git/refs/heads/"))).toBe(true); + }); + + it("returns the existing open PR when one is already open for the head branch", async () => { + const { impl } = fakeFetch({ + "POST /repos/o/r/pulls": () => ({ status: 422, json: { message: "A pull request already exists for o:h." } }), + "GET /repos/o/r/pulls?head=o%3Ah&state=open&per_page=1": () => ({ + json: [{ html_url: "https://gh/pull/7", number: 7 }], + }), + }); + const gateway = new GitHubRestGateway({ owner: "o", repo: "r", token: "t", fetch: impl }); + const pr = await gateway.openPullRequest({ base: "main", head: "h", title: "t", body: "b" }); + expect(pr).toEqual({ url: "https://gh/pull/7", number: 7 }); + }); + + it("paginates comment listing past the first hundred", async () => { + const first = Array.from({ length: 100 }, (_, i) => ({ id: i + 1, body: `c${i + 1}` })); + const { impl } = fakeFetch({ + "GET /repos/o/r/issues/9/comments?per_page=100&page=1": () => ({ json: first }), + "GET /repos/o/r/issues/9/comments?per_page=100&page=2": () => ({ + json: [{ id: 101, body: "\nmarked" }], + }), + }); + const gateway = new GitHubRestGateway({ owner: "o", repo: "r", token: "t", fetch: impl }); + const comments = await gateway.listComments(9); + expect(comments).toHaveLength(101); + expect(comments.at(-1)?.body).toContain("proofkeeper:scoped-qa"); + }); +}); diff --git a/tests/observe.test.ts b/tests/observe.test.ts index 005de6b..95da702 100644 --- a/tests/observe.test.ts +++ b/tests/observe.test.ts @@ -29,7 +29,8 @@ function fakePage() { const handlers: Record void>> = {}; const page = { on: (event: string, handler: (arg: unknown) => void) => { - (handlers[event] ??= []).push(handler); + handlers[event] ??= []; + handlers[event].push(handler); }, off: (event: string, handler: (arg: unknown) => void) => { handlers[event] = (handlers[event] ?? []).filter((h) => h !== handler); @@ -37,7 +38,9 @@ function fakePage() { } as unknown as Page; return { page, - emit: (event: string, arg: unknown) => (handlers[event] ?? []).forEach((h) => h(arg)), + emit: (event: string, arg: unknown) => { + for (const h of handlers[event] ?? []) h(arg); + }, count: (event: string) => (handlers[event] ?? []).length, }; } diff --git a/tests/qa-command.test.ts b/tests/qa-command.test.ts index 0ff2902..6458ba6 100644 --- a/tests/qa-command.test.ts +++ b/tests/qa-command.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; -import { parseQaArgs } from "../src/cli.js"; +import { parseQaArgs, parseCoverageArgs } from "../src/cli.js"; import { runQa, selectCapability, defaultGoal, type QaDeps } from "../src/qa/run-qa.js"; import { InMemoryLearningStore } from "../src/learning/store.js"; import type { Graph } from "../src/coverage/graph.js"; @@ -379,3 +379,10 @@ describe("parseQaArgs", () => { expect(() => parseQaArgs(["--graph-file", "g.json", "--url", "http://x/", "--bogus"])).toThrow(/unknown option/); }); }); + +describe("parseCoverageArgs value guard", () => { + it("rejects a missing value for --graph-file/--corpus like every other parser", () => { + expect(() => parseCoverageArgs(["--graph-file"])).toThrow(/missing value for --graph-file/); + expect(() => parseCoverageArgs(["--corpus", ""])).toThrow(/missing value for --corpus/); + }); +}); diff --git a/tests/scope-config.test.ts b/tests/scope-config.test.ts index d47c5fc..88d4183 100644 --- a/tests/scope-config.test.ts +++ b/tests/scope-config.test.ts @@ -128,7 +128,6 @@ const PERSONA_CFG = parseConfig( capabilities: [ { id: "REQ-ADMIN", paths: ["x"], url: "http://x/", persona: "admin" }, { id: "REQ-NONE", paths: ["y"], url: "http://y/" }, - { id: "REQ-BAD", paths: ["z"], url: "http://z/", persona: "ghost" }, ], personas: [ { name: "admin", testFocus: ["settings", "billing"], cannotDo: [] }, @@ -159,8 +158,19 @@ describe("personaContext", () => { expect(personaContext(PERSONA_CFG, cap("REQ-NONE"))).toBeUndefined(); }); - it("throws when the named persona is not defined", () => { - expect(() => personaContext(PERSONA_CFG, cap("REQ-BAD"))).toThrow(/undefined persona 'ghost'/); + it("rejects an undefined persona reference at parse time", () => { + expect(() => + parseConfig( + JSON.stringify({ + capabilities: [{ id: "REQ-BAD", paths: ["z"], url: "http://z/", persona: "ghost" }], + personas: [{ name: "admin" }], + }), + ), + ).toThrow(/undefined persona 'ghost'/); + // The runtime guard still holds for programmatically built configs. + expect(() => + personaContext({ capabilities: [] }, { id: "X", paths: ["x"], persona: "ghost" }), + ).toThrow(/undefined persona 'ghost'/); }); it("rejects a malformed persona block at parse time", () => { @@ -245,3 +255,49 @@ describe("trust boundary (allowShell / allowedHosts)", () => { ).toThrow(/allowedHosts/); }); }); + +describe("cross-reference validation at parse time", () => { + it("rejects duplicate capability ids", () => { + expect(() => + parseConfig( + JSON.stringify({ + capabilities: [ + { id: "REQ-A", paths: ["a"] }, + { id: "REQ-A", paths: ["b"] }, + ], + }), + ), + ).toThrow(/duplicate capability id 'REQ-A'/); + }); + + it("rejects a defaultTarget that names no defined environment", () => { + expect(() => + parseConfig( + JSON.stringify({ capabilities: [{ id: "A", paths: ["x"] }], defaultTarget: "staging" }), + ), + ).toThrow(/defaultTarget 'staging' is not a defined environment/); + }); + + it("rejects a capability environment that names no defined environment", () => { + expect(() => + parseConfig( + JSON.stringify({ + capabilities: [{ id: "A", paths: ["x"], environment: "prod" }], + environments: { dev: { url: "http://dev/" } }, + }), + ), + ).toThrow(/capability 'A' references undefined environment 'prod'/); + }); + + it("accepts a fully consistent config", () => { + const cfg = parseConfig( + JSON.stringify({ + capabilities: [{ id: "A", paths: ["x"], environment: "dev", persona: "admin" }], + environments: { dev: { url: "http://dev/" } }, + defaultTarget: "dev", + personas: [{ name: "admin" }], + }), + ); + expect(cfg.capabilities).toHaveLength(1); + }); +}); diff --git a/tests/writeback-proposer.test.ts b/tests/writeback-proposer.test.ts index 5799de3..a68339c 100644 --- a/tests/writeback-proposer.test.ts +++ b/tests/writeback-proposer.test.ts @@ -163,3 +163,13 @@ describe("GitHubWriteBackProposer", () => { expect(gateway.prs).toEqual([]); }); }); + +describe("linksFromResults — portable paths", () => { + it("POSIX-normalizes Windows separators in spec and trace paths", () => { + const links = linksFromResults( + { id: "c", specPath: "tests\\generated\\req-b.spec.ts" }, + [{ testId: "c", target: "local", status: "passed", durationMs: 1, tracePath: "test-results\\c\\trace.zip" }], + ); + expect(links).toEqual([{ test: "tests/generated/req-b.spec.ts", trace: "test-results/c/trace.zip" }]); + }); +});