diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bd6cbb..18fcf42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,16 @@ observable behavior and compatibility, not every internal refactor. ### Fixed +- Portable HTML finding-bound fixes now record against the HTML report contract + without requiring Qoder's `canvas.json`, and refresh `findings.json`, + `report.md`, and `report.html` to the same repair revision. Qoder split reports + retain their Canvas-sidecar validation boundary. + +- Root CLI delegation failures now keep machine mode parseable: spawn errors, + signal termination, and output-buffer exhaustion each emit one stable JSON + error document, while normal child stdout, stderr, and numeric exit status + remain capability-owned. + - Checkup plan/apply is provider-aware: only `provider=qoder` can emit or execute `qodercli` disable mutations. Other hosts keep candidates as `manual-review` until a provider-native apply contract exists. `provider-home` source diff --git a/docs/specs/2026-08-01-html-finding-fix-recording.md b/docs/specs/2026-08-01-html-finding-fix-recording.md new file mode 100644 index 0000000..7a53507 --- /dev/null +++ b/docs/specs/2026-08-01-html-finding-fix-recording.md @@ -0,0 +1,114 @@ +# Keep Portable HTML Fix Records Consistent + +## Traceability + +- Spec ID: `html-finding-fix-recording` +- Status: Implemented +- Review context: `QoderAI/better-harness#39` + +## Intent + +Complete the finding-bound repair flow introduced for portable HTML reports. +The fix recorder must recognize a valid HTML report without requiring Qoder's +Canvas sidecar, validate the correct report contract, and publish the updated +`findings.json`, `report.md`, and `report.html` as one consistent revision. +Qoder Canvas reports must retain their stricter split-artifact validation. + +## Acceptance Scenarios + +- **HFR-AC-1 (report-family resolution):** a compact findings document with + `report.md` and `report.html`, and without Canvas artifacts, is recognized as + an HTML report. A split Canvas report is recognized only through its Canvas + artifacts. Mixed, incomplete, or otherwise ambiguous artifact sets fail + closed with a stable report-context error. +- **HFR-AC-2 (HTML recording):** a valid result for an HTML finding records + `actualOutput`, its AI-authored Assignment Summary, and its independent repair + review without reading or creating `canvas.json`; the finding revision and + report repair progress advance exactly once. +- **HFR-AC-3 (three-artifact consistency):** after a successful HTML record, + `findings.json`, `report.md`, and `report.html` are regenerated from the same + updated report data. The HTML action callback carries the new expected + revision, and the report directory still contains only those three durable + report artifacts. +- **HFR-AC-4 (failure safety):** stale revisions, invalid result payloads, + invalid output paths, render-validation failures, and ambiguous report + families do not change any durable report artifact and do not consume the + result file. +- **HFR-AC-5 (Canvas isolation):** existing Qoder split reports still require + and validate their Canvas sidecar, and a fabricated or missing sidecar cannot + make an HTML report pass through the Canvas contract. +- **HFR-AC-6 (result lifecycle):** workflow guidance keeps the temporary result + outside the renderer-owned report directory and consumes it only after the + recorder returns `status: "pass"`. + +## Non-goals + +- Changing Loop Effectiveness scores or treating a repair review as delayed + outcome evidence. +- Changing the callback transport contract already shipped by PR #39. +- Redesigning Qoder Canvas rendering, durable Canvas ownership, or its split + report schema. +- Reconstructing or mutating historical reports without an explicit + finding-bound record operation. +- Guaranteeing cross-file atomicity across an operating-system or power loss; + the publish order instead keeps `findings.json` as the final commit marker. + +## Plan and Tasks + +1. Resolve report family from the durable sibling artifact signature before + validation: full findings, portable HTML, or Qoder Canvas. Reject mixed and + incomplete compact contexts. +2. Validate portable HTML findings with the compact task-loop contract and + preserve the existing full and Canvas validation paths. +3. Apply the result to an isolated clone, validate outputs, render Markdown and + HTML, and run the HTML evaluator before writing durable files. +4. Publish HTML and Markdown first and `findings.json` last so the revision file + remains the final commit marker for prepared artifacts. +5. Update the finding-bound-fix workflow to place temporary result payloads + outside the report directory and consume them only on recorder success. +6. Add focused tests using an actual three-artifact HTML report, plus failure + and Canvas-isolation coverage. + +## Test and Review Evidence + +- **HFR-AC-1/HFR-AC-2/HFR-AC-3:** add focused tests to + `test/task-loop-report.test.mjs` that render an HTML report, record one fix, + parse the resulting findings, and verify Markdown, HTML, callback revision, + repair progress, and durable artifact names. +- **HFR-AC-4:** snapshot all three artifacts before representative failures + and assert byte identity afterward; assert the temporary result remains. +- **HFR-AC-5:** retain existing split-report tests and add explicit missing and + mixed-sidecar rejection cases. +- **HFR-AC-6:** update and validate the shipped skill reference and package. +- **Regression:** run the focused report and CLI tests, `npm run pack:verify`, + and the full `npm test` suite. +- **Documentation integrity:** regenerate the Better Harness doc routing graph + and run `node --test test/doc-link-graph.test.mjs`. +- **Review readiness:** run `git diff --check`, inspect staged and unstaged + changes, and record changed modules, generated files, risks, and AI evidence. +- **Risk:** an interrupted multi-file publish can leave prepared Markdown or + HTML ahead of the old findings revision. Publishing `findings.json` last + keeps the old callback revision authoritative until all prepared views have + been installed; normal validation failures occur before any publish begins. + +## Observed Validation Evidence + +- The 16 focused `record-fix-output` tests passed, including the new portable + HTML success, pre-publish failure safety, and Qoder Canvas isolation cases. +- The HTML fixture proves that a successful record updates all three artifacts, + increments the embedded callback revision, advances repair progress, leaves + only `findings.json`, `report.md`, and `report.html`, and consumes an external + result only after success. +- The original three-artifact Codex report was exercised without mutation by + deliberately supplying a stale expected revision. The recorder returned + `STALE_FIX_OUTPUT_REVISION` instead of `ENOENT`, proving that the valid compact + HTML contract no longer attempts to read `canvas.json`. +- The combined report, renderer, Skill, CLI, and compatibility run passed 166 + tests with one supported Windows symlink skip; its only failure was the same + pre-existing unguarded Windows symlink-permission case reproduced on the + unchanged baseline. +- The full repository run reached 1,044 passes and 6 supported skips. Its four + failures were all pre-existing Windows `EPERM` symlink-creation cases and did + not execute the changed recorder or HTML publication path. +- `npm run pack:verify` passed with 369 npm entries and 392 runtime-zip entries; + the regenerated doc graph and all six doc-link tests passed. diff --git a/docs/specs/2026-08-01-root-cli-machine-failure-contract.md b/docs/specs/2026-08-01-root-cli-machine-failure-contract.md new file mode 100644 index 0000000..3080006 --- /dev/null +++ b/docs/specs/2026-08-01-root-cli-machine-failure-contract.md @@ -0,0 +1,108 @@ +# Keep Root CLI Machine Failures Parseable + +## Traceability + +- Spec ID: `root-cli-machine-failure-contract` +- Status: Implemented + +## Intent + +Keep automation able to classify failures that occur at the root CLI's +delegated-process boundary. After the root recognizes machine mode, a child +spawn failure or signal termination must produce one existing versioned error +envelope with a stable code and safe recovery hint. Human invocations should +retain short diagnostics, while successfully started child commands continue +to own their normal output bytes and exit status. + +## Acceptance Scenarios + +- **RCM-AC-1 (spawn failure):** when a pre-passthrough `--json` token selects + machine mode and the delegated process cannot start, stdout contains exactly + one parseable `format_version: "1.0"` error document with code + `DELEGATED_COMMAND_SPAWN_FAILED`, stderr contains no second envelope, and the + process exits with status 1. +- **RCM-AC-2 (signal termination):** when a delegated process terminates by + signal in machine mode, any incomplete child stdout or stderr is not relayed; + stdout instead contains exactly one parseable `format_version: "1.0"` error + document with code `DELEGATED_COMMAND_SIGNAL_TERMINATED`, a safe next step, + and exit status 1. +- **RCM-AC-3 (human diagnostics):** the same two failures without recognized + machine mode write no stdout and emit one concise diagnostic on stderr. A + spawn diagnostic may retain the operating-system error detail; a signal + diagnostic names the terminating signal. +- **RCM-AC-4 (normal child ownership):** delegated commands that return a + numeric status keep that status, stdout, and stderr byte-for-byte. The root + CLI adds no success or child-business-failure envelope. +- **RCM-AC-5 (output overflow):** when a delegated process starts, runs, and + exceeds the machine-mode output buffer, stdout contains exactly one parseable + `format_version: "1.0"` error document with code + `DELEGATED_COMMAND_OUTPUT_OVERFLOW` and a hint that narrows scope or drops + `--json`. Truncated child bytes and the raw `ENOBUFS` diagnostic are not + relayed, and the envelope is never confused with a start failure. + +## Non-goals + +- Changing capability-owned output schemas, messages, or business-failure + exit codes. +- Replacing synchronous argv-array dispatch, changing the command registry, or + adding shell-string execution. +- Activating the ADR's proposed `command-contract.v1` shape beyond the root + CLI's existing `format_version: "1.0"` envelope. +- Normalizing malformed argument bytes or redesigning all global-option + parsing. + +## Plan and Tasks + +1. Keep dispatch resolution in `scripts/better-harness-cli/cli.mjs`, and add a + single result-normalization path that receives the already-recognized + machine-mode state. +2. In machine mode, capture delegated stdout and stderr until `spawnSync` + returns. Relay both unchanged for numeric child statuses, but replace + incomplete output with the root error envelope for spawn and signal + failures. +3. Reuse the root `errorPayload`/`jsonDocument` writer for versioning and add a + narrow process-runner/output-writer seam so focused tests can deterministically + supply cross-platform spawn and signal results. +4. Add focused root CLI tests for spawn failure, signal termination, human + diagnostics, absence of a stderr envelope, and normal byte preservation. + +## Test and Review Evidence + +- **RCM-AC-1/RCM-AC-2:** run + `node --test test/better-harness-cli.test.mjs`; focused injected-result tests + must parse all of machine stdout with `JSON.parse`, assert the stable error + code and hint, and assert empty stderr even when the simulated child result + contains partial output. +- **RCM-AC-3:** the same focused tests assert empty stdout and bounded stderr in + human mode for spawn and signal failures. +- **RCM-AC-4:** compare delegated root CLI output with direct capability output + and assert identical stdout, stderr, and numeric exit status. +- **RCM-AC-5:** drive a real `spawnSync` overflow with a small `maxBuffer`, then + assert the resulting `ENOBUFS` shape produces the overflow envelope instead of + the spawn-failure envelope in machine mode and a bounded stderr diagnostic in + human mode. +- **Documentation integrity:** run + `node scripts/doc-link-graph/cli.mjs skills/better-harness` and + `node --test test/doc-link-graph.test.mjs` after adding this Markdown file. +- **Review readiness:** run `git diff --check` and inspect staged and unstaged + changes separately. +- **Risk:** machine-mode buffering delays delegated output until the child + exits and is bounded by Node's synchronous child-process buffer. Keep the + buffer explicit and large enough for current machine documents; buffer + exhaustion stays a stable machine envelope with its own overflow code rather + than leaking a raw diagnostic or truncated child bytes into machine stdout. + +## Observed Validation Evidence + +- `node --test test/better-harness-cli.test.mjs` passed 36 tests with one + pre-existing Windows symlink-permission skip. The focused injected-result + tests parse the complete spawn/signal stdout as one JSON document, assert + empty machine stderr, and prove partial child envelopes are discarded. +- The combined CLI, report, Skill, renderer, and compatibility run retained all + upstream leaf-command behavior and passed every root CLI and + `scripts-refactor-contract` assertion. +- The full repository run reached 1,044 passes and 6 supported skips. Its four + failures were pre-existing Windows `EPERM` symlink-creation cases; the same + failure was reproduced from the unchanged baseline and does not execute the + changed CLI normalization path. +- `npm run pack:verify`, the six doc-link tests, and `git diff --check` passed. diff --git a/scripts/better-harness-cli/cli.mjs b/scripts/better-harness-cli/cli.mjs index b85f840..d6125c3 100644 --- a/scripts/better-harness-cli/cli.mjs +++ b/scripts/better-harness-cli/cli.mjs @@ -19,6 +19,7 @@ import { createStyle, formatRows, shouldUseColor } from "./format.mjs"; const scriptsRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const repoRoot = path.resolve(scriptsRoot, ".."); const programName = "better-harness"; +const MACHINE_OUTPUT_MAX_BUFFER = 64 * 1024 * 1024; const HELP_GROUPS = [ { @@ -255,7 +256,11 @@ function hasHelpFlag(argv) { } function hasJsonFlag(argv) { - return argv.includes("--json"); + for (const value of argv) { + if (value === "--") return false; + if (value === "--json") return true; + } + return false; } function optionValue(argv, name) { @@ -492,26 +497,94 @@ export function resolveDispatch(argv = []) { }; } -export function main(argv = process.argv.slice(2)) { - const dispatch = resolveDispatch(argv); - if (dispatch.kind === "help" || dispatch.kind === "json") { - process.stdout.write(dispatch.text); - return dispatch.exitCode; +function emitRootResult(result, { stdout, stderr }) { + if (result.kind === "help" || result.kind === "json") { + stdout.write(result.text); + return result.exitCode; } - if (dispatch.kind === "error") { - process.stderr.write(dispatch.message); - return dispatch.exitCode; + stderr.write(result.message); + return result.exitCode; +} + +function writeChildOutput(stream, value) { + if (value !== null && value !== undefined && value.length > 0) { + stream.write(value); } +} - const result = spawnSync(process.execPath, [dispatch.script, ...dispatch.args], { - cwd: process.cwd(), - stdio: "inherit", - windowsHide: true, - }); +function signalName(value) { + return typeof value === "string" && /^[A-Z0-9]+$/u.test(value) + ? value + : "UNKNOWN"; +} + +function normalizeDispatchResult(result, { + machine, + stdout = process.stdout, + stderr = process.stderr, +} = {}) { + if (result.error?.code === "ENOBUFS") { + return emitRootResult(rootError({ + code: "DELEGATED_COMMAND_OUTPUT_OVERFLOW", + message: "The delegated command produced more output than machine mode can buffer.", + hint: machine + ? "Narrow the command scope, or rerun without `--json` to stream the full output." + : undefined, + machine, + }), { stdout, stderr }); + } if (result.error) { - process.stderr.write(`${result.error.message}\n`); - return 1; + return emitRootResult(rootError({ + code: "DELEGATED_COMMAND_SPAWN_FAILED", + message: machine + ? "Failed to start the delegated command." + : `Failed to start the delegated command: ${result.error.message}`, + hint: machine + ? "Verify the Better Harness installation, then retry the command." + : undefined, + machine, + }), { stdout, stderr }); + } + + if (result.signal) { + const signal = signalName(result.signal); + return emitRootResult(rootError({ + code: "DELEGATED_COMMAND_SIGNAL_TERMINATED", + message: `The delegated command terminated with signal ${signal}.`, + hint: machine + ? "Retry the command; if it repeats, inspect the delegated capability diagnostics." + : undefined, + machine, + }), { stdout, stderr }); + } + + if (machine) { + writeChildOutput(stdout, result.stdout); + writeChildOutput(stderr, result.stderr); } return result.status ?? 1; } + +export function main(argv = process.argv.slice(2), runtime = {}) { + const { + spawn = spawnSync, + stdout = process.stdout, + stderr = process.stderr, + cwd = process.cwd(), + } = runtime; + const dispatch = resolveDispatch(argv); + if (dispatch.kind !== "dispatch") { + return emitRootResult(dispatch, { stdout, stderr }); + } + + const machine = hasJsonFlag(argv); + const result = spawn(process.execPath, [dispatch.script, ...dispatch.args], { + cwd, + stdio: machine ? ["inherit", "pipe", "pipe"] : "inherit", + windowsHide: true, + ...(machine ? { maxBuffer: MACHINE_OUTPUT_MAX_BUFFER } : {}), + }); + + return normalizeDispatchResult(result, { machine, stdout, stderr }); +} diff --git a/scripts/harness-analysis/record-fix-output.mjs b/scripts/harness-analysis/record-fix-output.mjs index af98db8..2a9bc5e 100644 --- a/scripts/harness-analysis/record-fix-output.mjs +++ b/scripts/harness-analysis/record-fix-output.mjs @@ -10,9 +10,13 @@ import { assignmentSummariesFromFindings, isFullTaskLoopFindings, repairProgressFromFindings, + validateCompactTaskLoopFindings, validateTaskLoopCanvasSplit, validateTaskLoopFindings, } from "./task-loop-report.mjs"; +import { normalizeReportData } from "./report-data-schema.mjs"; +import { evaluateHtmlReport, renderHtml } from "./renderers/html.mjs"; +import { renderMarkdown } from "./renderers/markdown.mjs"; import { findingTargetErrors, resolveWorkspaceTopology, @@ -90,14 +94,14 @@ async function atomicReplace(filePath, content) { } } -function reportErrors(findings, canvas) { - return isFullTaskLoopFindings(findings) - ? validateTaskLoopFindings(findings) - : validateTaskLoopCanvasSplit(findings, canvas); +function reportErrors(context) { + if (context.findingsShape === "full") return validateTaskLoopFindings(context.findings); + if (context.family === "html") return validateCompactTaskLoopFindings(context.findings); + return validateTaskLoopCanvasSplit(context.findings, context.canvas); } -function assertValidReport(findings, canvas, phase) { - const errors = reportErrors(findings, canvas); +function assertValidReport(context, phase) { + const errors = reportErrors(context); if (errors.length === 0) return; throw Object.assign(new Error(`${phase}: ${errors.join("; ")}`), { code: "INVALID_TASK_LOOP_FINDINGS", @@ -155,10 +159,79 @@ async function assertOutputTargets(actualOutput, workspacePath, { async function readReportContext(findingsPath) { const findings = JSON.parse(await readFile(findingsPath, "utf8")); - if (isFullTaskLoopFindings(findings)) return { findings, canvas: undefined }; - const canvasPath = path.join(path.dirname(findingsPath), "canvas.json"); - const canvas = JSON.parse(await readFile(canvasPath, "utf8")); - return { findings, canvas }; + const findingsShape = isFullTaskLoopFindings(findings) ? "full" : "compact"; + const reportDir = path.dirname(findingsPath); + const artifactExists = async (name) => { + try { + return (await stat(path.join(reportDir, name))).isFile(); + } catch (error) { + if (error?.code === "ENOENT") return false; + throw error; + } + }; + const [hasMarkdown, hasHtml, hasCanvasData, hasCanvasModule] = await Promise.all([ + artifactExists("report.md"), + artifactExists("report.html"), + artifactExists("canvas.json"), + artifactExists("report.canvas.tsx"), + ]); + const hasHtmlArtifact = hasMarkdown || hasHtml; + const hasCanvasArtifact = hasCanvasData || hasCanvasModule; + const invalidContext = (message) => { + throw Object.assign(new Error(message), { code: "INVALID_REPORT_CONTEXT" }); + }; + + if (hasHtmlArtifact && hasCanvasArtifact) { + invalidContext("compact report context mixes portable HTML and Qoder Canvas artifacts"); + } + if (hasHtmlArtifact) { + if (!hasMarkdown || !hasHtml) { + invalidContext("portable HTML report context requires both report.md and report.html"); + } + return { + family: "html", + findingsShape, + findings, + markdownPath: path.join(reportDir, "report.md"), + htmlPath: path.join(reportDir, "report.html"), + }; + } + if (hasCanvasArtifact) { + if (findingsShape === "full") { + invalidContext("full findings cannot be combined with Qoder Canvas split artifacts"); + } + if (!hasCanvasData) { + invalidContext("Qoder Canvas report context requires canvas.json"); + } + return { + family: "qoder-canvas", + findingsShape, + findings, + canvas: JSON.parse(await readFile(path.join(reportDir, "canvas.json"), "utf8")), + }; + } + if (findingsShape === "full") return { family: "full", findingsShape, findings }; + invalidContext("compact report context is missing portable HTML or Qoder Canvas artifacts"); +} + +function prepareHtmlArtifacts(context, { findingsPath, workspacePath }) { + const reportData = normalizeReportData(context.findings, { + mode: "html", + language: context.findings.summary?.locale, + target: workspacePath, + dataPath: findingsPath, + }); + const actionContext = { findingsPath }; + const markdown = renderMarkdown(reportData); + const html = renderHtml(reportData, actionContext); + const htmlCheck = evaluateHtmlReport(html, reportData, actionContext); + if (htmlCheck.status === "fail") { + throw Object.assign(new Error(`Updated HTML report validation failed: ${htmlCheck.errors.join("; ")}`), { + code: "INVALID_UPDATED_HTML_REPORT", + errors: htmlCheck.errors, + }); + } + return { markdown, html }; } export async function recordFixOutput({ @@ -170,6 +243,7 @@ export async function recordFixOutput({ consumeResult = false, topology, resolveTopology = resolveWorkspaceTopology, + prepareHtmlReport = prepareHtmlArtifacts, } = {}) { const workspacePath = path.resolve(String(workspace ?? "")); const findingsPath = path.resolve(String(findingsOption ?? "")); @@ -189,7 +263,7 @@ export async function recordFixOutput({ let payload; try { const context = await readReportContext(findingsPath); - assertValidReport(context.findings, context.canvas, "Current findings validation failed"); + assertValidReport(context, "Current findings validation failed"); const matches = context.findings.findings.filter((finding) => finding?.id === findingId); if (matches.length !== 1) throw new Error(`finding id must match exactly one row: ${findingId}`); const finding = matches[0]; @@ -227,11 +301,18 @@ export async function recordFixOutput({ else finding.postFixRepairReview = JSON.parse(JSON.stringify(resultPayload.postFixRepairReview)); delete finding.postFixScoreReview; context.findings.summary.assignmentSummaries = assignmentSummariesFromFindings(context.findings.findings); - assertValidReport(context.findings, context.canvas, "Updated findings validation failed"); + assertValidReport(context, "Updated findings validation failed"); await assertOutputTargets(finding.actualOutput, workspacePath, { findingTarget: finding.target, topology: repairTopology, }); + const htmlArtifacts = context.family === "html" + ? prepareHtmlReport(context, { findingsPath: findingsRealPath, workspacePath: workspaceRealPath }) + : null; + if (htmlArtifacts) { + await atomicReplace(context.markdownPath, htmlArtifacts.markdown); + await atomicReplace(context.htmlPath, htmlArtifacts.html); + } await atomicReplace(findingsPath, `${JSON.stringify(context.findings, null, 2)}\n`); payload = { kind: "harness-fix-output-record", @@ -248,6 +329,7 @@ export async function recordFixOutput({ reason: resultPayload.postFixScoreReview === undefined ? "deferred-outcome-window" : "legacy-review-deferred", dimensions: [], }, + reportFamily: context.family, resultConsumed: false, }; } finally { diff --git a/skills/better-harness/references/finding-bound-fix.md b/skills/better-harness/references/finding-bound-fix.md index ddf5527..d67984b 100644 --- a/skills/better-harness/references/finding-bound-fix.md +++ b/skills/better-harness/references/finding-bound-fix.md @@ -92,7 +92,10 @@ workspace is a member package. This keeps an ancestor-owned result such as Author one standalone `assignmentSummary` in the report's exact locale. Its title and body explain the finding-level verified outcome and validation boundary; do not build it by joining artifact summaries. Write both fields to a -temporary result object. +temporary result object outside the directory that owns the callback's +`findings.json`. Use an operating-system temporary directory or another +workspace-controlled scratch location; never add the result object as a fourth +durable report artifact beside `findings.json`, `report.md`, or `report.html`. ### Reassess the Repair Independently @@ -145,5 +148,5 @@ Report success only when the writer returns `status: pass` and the next revision. Reuse the recorded Assignment Summary and report `repairProgress`; `scoreRefresh` must remain unchanged for the current outcome window. If target validation fails, no material change exists, or the writer fails, do not edit -`findings.json`; preserve the temporary result for diagnosis and surface the -exact blocker. +the report artifacts; preserve the temporary result outside the report +directory for diagnosis and surface the exact blocker. diff --git a/test/better-harness-cli.test.mjs b/test/better-harness-cli.test.mjs index 11cb621..debc59b 100644 --- a/test/better-harness-cli.test.mjs +++ b/test/better-harness-cli.test.mjs @@ -5,7 +5,7 @@ import os from "node:os"; import path from "node:path"; import test from "node:test"; import { pathToFileURL } from "node:url"; -import { resolveDispatch } from "../scripts/better-harness-cli/cli.mjs"; +import { main, resolveDispatch } from "../scripts/better-harness-cli/cli.mjs"; import { commandInventory } from "../scripts/better-harness-cli/registry.mjs"; const cliPath = path.join(process.cwd(), "scripts", "better-harness.mjs"); @@ -19,6 +19,27 @@ function runBetterHarness(args, options = {}) { }); } +function runMainWithResult(args, childResult) { + const calls = []; + const stdout = []; + const stderr = []; + const exitCode = main(args, { + cwd: process.cwd(), + spawn(command, childArgs, options) { + calls.push({ command, childArgs, options }); + return childResult; + }, + stdout: { write: (value) => stdout.push(Buffer.from(value).toString("utf8")) }, + stderr: { write: (value) => stderr.push(Buffer.from(value).toString("utf8")) }, + }); + return { + calls, + exitCode, + stdout: stdout.join(""), + stderr: stderr.join(""), + }; +} + function helpGuardEnvironment(expectedOwner, expectedOwnerArgs) { const inheritedOptions = process.env.NODE_OPTIONS?.trim(); return { @@ -679,6 +700,114 @@ test("better-harness CLI emits JSON root errors in machine mode", () => { assert.doesNotMatch(payload.error.hint, /Usage:/); }); +test("better-harness CLI normalizes delegated spawn errors in human and machine modes", () => { + const childResult = { + error: new Error("spawn ENOENT at a private installation path"), + signal: null, + status: null, + stdout: Buffer.from('{"child":"partial"}\n'), + stderr: Buffer.from('{"second":"envelope"}\n'), + }; + const machine = runMainWithResult(["cloc", "--json"], childResult); + + assert.equal(machine.exitCode, 1); + assert.equal(machine.stderr, ""); + const payload = JSON.parse(machine.stdout); + assert.equal(payload.ok, false); + assert.equal(payload.format_version, "1.0"); + assert.equal(payload.error.code, "DELEGATED_COMMAND_SPAWN_FAILED"); + assert.match(payload.error.hint, /Verify the Better Harness installation/u); + assert.doesNotMatch(machine.stdout, /private installation path/u); + assert.doesNotMatch(machine.stdout, /child|second/u); + assert.deepEqual(machine.calls[0].options.stdio, ["inherit", "pipe", "pipe"]); + + const human = runMainWithResult(["cloc"], childResult); + assert.equal(human.exitCode, 1); + assert.equal(human.stdout, ""); + assert.match(human.stderr, /^Failed to start the delegated command: spawn ENOENT/u); + assert.equal(human.calls[0].options.stdio, "inherit"); +}); + +test("better-harness CLI replaces signalled child fragments with one machine envelope", () => { + const machine = runMainWithResult(["cloc", "--json"], { + error: undefined, + signal: "SIGTERM", + status: null, + stdout: Buffer.from('{"child":"partial"}\n'), + stderr: Buffer.from('{"second":"envelope"}\n'), + }); + + assert.equal(machine.exitCode, 1); + assert.equal(machine.stderr, ""); + const payload = JSON.parse(machine.stdout); + assert.equal(payload.ok, false); + assert.equal(payload.format_version, "1.0"); + assert.equal(payload.error.code, "DELEGATED_COMMAND_SIGNAL_TERMINATED"); + assert.equal(payload.error.message, "The delegated command terminated with signal SIGTERM."); + assert.match(payload.error.hint, /Retry the command/u); + assert.doesNotMatch(machine.stdout, /child|second/u); + + const human = runMainWithResult(["cloc"], { + error: undefined, + signal: "SIGTERM", + status: null, + stdout: null, + stderr: null, + }); + assert.equal(human.exitCode, 1); + assert.equal(human.stdout, ""); + assert.equal(human.stderr, "The delegated command terminated with signal SIGTERM.\n"); + + const passthrough = runMainWithResult(["cloc", "--", "--json"], { + error: undefined, + signal: "SIGTERM", + status: null, + stdout: null, + stderr: null, + }); + assert.equal(passthrough.stdout, ""); + assert.equal(passthrough.stderr, "The delegated command terminated with signal SIGTERM.\n"); + assert.equal(passthrough.calls[0].options.stdio, "inherit"); +}); + +test("better-harness CLI preserves normal machine child output and status byte-for-byte", () => { + const childStdout = Buffer.from('{"owner":"child"}\r\n'); + const childStderr = Buffer.from("child diagnostic\r\n"); + const result = runMainWithResult(["cloc", "--json"], { + error: undefined, + signal: null, + status: 7, + stdout: childStdout, + stderr: childStderr, + }); + + assert.equal(result.exitCode, 7); + assert.equal(result.stdout, childStdout.toString("utf8")); + assert.equal(result.stderr, childStderr.toString("utf8")); +}); + +test("better-harness CLI separates machine output overflow from spawn failure", () => { + const overflow = spawnSync(process.execPath, [ + "-e", + "process.stdout.write('x'.repeat(1024 * 1024)); process.exit(0);", + ], { stdio: ["inherit", "pipe", "pipe"], maxBuffer: 1024 }); + assert.equal(overflow.error?.code, "ENOBUFS"); + + const machine = runMainWithResult(["cloc", "--json"], overflow); + assert.equal(machine.exitCode, 1); + assert.equal(machine.stderr, ""); + const payload = JSON.parse(machine.stdout); + assert.equal(payload.ok, false); + assert.equal(payload.format_version, "1.0"); + assert.equal(payload.error.code, "DELEGATED_COMMAND_OUTPUT_OVERFLOW"); + assert.match(payload.error.hint, /rerun without `--json`/u); + assert.doesNotMatch(machine.stdout, /ENOBUFS|xxxx/u); + + const human = runMainWithResult(["cloc"], overflow); + assert.equal(human.stdout, ""); + assert.equal(human.stderr, "The delegated command produced more output than machine mode can buffer.\n"); +}); + test("better-harness CLI runs through a package-bin symlink", async (t) => { const root = await mkdtemp(path.join(os.tmpdir(), "better-harness-cli-bin-")); const linkPath = path.join(root, "better-harness"); diff --git a/test/task-loop-report.test.mjs b/test/task-loop-report.test.mjs index 57d402e..556f440 100644 --- a/test/task-loop-report.test.mjs +++ b/test/task-loop-report.test.mjs @@ -1,7 +1,7 @@ import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; import test from "node:test"; -import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readFile, readdir, rm, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; @@ -10,6 +10,7 @@ import { buildCheckupScan } from "../scripts/coding-agent-practices/checkup/scan import { createHarnessReportSource, validateHarnessReportSource } from "../scripts/harness-analysis/report-source.mjs"; import { normalizeReportData } from "../scripts/harness-analysis/report-data-schema.mjs"; import { evaluateBetterHarnessArtifacts, renderBetterHarnessReportCanvas } from "../scripts/harness-analysis/renderers/better-harness.mjs"; +import { evaluateHtmlReport, renderHtml } from "../scripts/harness-analysis/renderers/html.mjs"; import { renderMarkdown } from "../scripts/harness-analysis/renderers/markdown.mjs"; import { mergeTaskLoopCanvasData, @@ -19,6 +20,7 @@ import { taskLoopCanvasFromSummaryFacts, reconcileTaskLoopFindingLinks, splitTaskLoopFindings, + validateCompactTaskLoopFindings, validateTaskLoopCanvasSplit, validateTaskLoopFindings, } from "../scripts/harness-analysis/task-loop-report.mjs"; @@ -2861,6 +2863,216 @@ test("practice findings preserve root and package owners through final projectio assert.match(validateTaskLoopFindings(invalid).join("; "), /target must be an object/u); }); +async function htmlFixFixture(root) { + const workspace = path.join(root, "workspace"); + const runDir = path.join(workspace, ".codex", "better-harness", "run"); + const findingsPath = path.join(runDir, "findings.json"); + const markdownPath = path.join(runDir, "report.md"); + const htmlPath = path.join(runDir, "report.html"); + const resultPath = path.join(root, "result.json"); + const targetPath = "fix-output/owner.md"; + const split = splitTaskLoopFindings(projectTaskLoopFindings(reportSource())); + const finding = split.findings.findings[0]; + const reportData = normalizeReportData(split.findings, { + mode: "html", + language: split.findings.summary.locale, + target: workspace, + dataPath: findingsPath, + }); + const resultPayload = { + actualOutput: [{ + action: "updated", + artifact: finding.expectedArtifact, + name: `${finding.expectedArtifact} owner`, + scope: "Project", + path: targetPath, + summary: "Updated the finding owner and retained its focused validation result.", + }], + assignmentSummary: assignmentSummary({ + title: "The portable report now shows the verified fix", + body: "The finding owner passed focused validation, and the portable report now presents the exact recorded result.", + }), + postFixRepairReview: { + modelId: split.findings.summary.modelId, + findingId: finding.id, + status: "verified", + summary: "The focused repair passed its target-owned validation.", + reason: "An independent review confirmed the actual output while reserving outcome scores for a later task window.", + confidence: "medium", + evidenceRefs: [ + { kind: "fix-validation", id: "portable-html-owner-check" }, + { kind: "asset-integrity", id: "portable-html-integrity-check" }, + ], + }, + }; + await mkdir(path.join(workspace, path.dirname(targetPath)), { recursive: true }); + await mkdir(runDir, { recursive: true }); + await writeFile(path.join(workspace, targetPath), "# Owner\n"); + await writeFile(findingsPath, `${JSON.stringify(split.findings, null, 2)}\n`); + await writeFile(markdownPath, renderMarkdown(reportData)); + await writeFile(htmlPath, renderHtml(reportData, { findingsPath })); + await writeFile(resultPath, JSON.stringify(resultPayload)); + return { + workspace, + runDir, + findingsPath, + markdownPath, + htmlPath, + resultPath, + resultPayload, + finding, + }; +} + +async function htmlArtifactSnapshot(fixture) { + return Object.fromEntries(await Promise.all([ + ["findings.json", fixture.findingsPath], + ["report.md", fixture.markdownPath], + ["report.html", fixture.htmlPath], + ].map(async ([name, filePath]) => [name, await readFile(filePath, "utf8")]))); +} + +test("record-fix-output records portable HTML fixes and refreshes all three artifacts", async () => { + await withTempDir(async (root) => { + const fixture = await htmlFixFixture(root); + const before = await htmlArtifactSnapshot(fixture); + + const recorded = await recordFixOutput({ + workspace: fixture.workspace, + findings: fixture.findingsPath, + findingId: fixture.finding.id, + expectedRevision: 0, + result: fixture.resultPath, + consumeResult: true, + }); + + assert.equal(recorded.status, "pass"); + assert.equal(recorded.reportFamily, "html"); + assert.equal(recorded.revision, 1); + assert.equal(recorded.repairProgress.verifiedFindingCount, 1); + assert.equal(recorded.resultConsumed, true); + await assert.rejects(readFile(fixture.resultPath, "utf8"), { code: "ENOENT" }); + assert.deepEqual((await readdir(fixture.runDir)).sort(), ["findings.json", "report.html", "report.md"]); + + const after = await htmlArtifactSnapshot(fixture); + assert.notEqual(after["findings.json"], before["findings.json"]); + assert.notEqual(after["report.md"], before["report.md"]); + assert.notEqual(after["report.html"], before["report.html"]); + const updated = JSON.parse(after["findings.json"]); + assert.deepEqual(validateCompactTaskLoopFindings(updated), []); + assert.equal(updated.findings[0].actualOutputRevision, 1); + assert.equal(updated.findings[0].postFixRepairReview.status, "verified"); + assert.match(after["report.md"], /Asset Health \/ Repair Progress: 100\/100 \(1 verified, 0 partial, 0 pending\)/u); + assert.match(after["report.html"], new RegExp(`${recorded.repairProgress.score}\\s*\\/\\s*100`, "u")); + + const actionPayload = JSON.parse(after["report.html"].match( + /([\s\S]*?)<\/script>/iu, + )[1]); + assert.equal(actionPayload.findings.find((row) => row.id === fixture.finding.id).expectedRevision, 1); + const reportData = normalizeReportData(updated, { + mode: "html", + language: updated.summary.locale, + target: fixture.workspace, + dataPath: fixture.findingsPath, + }); + assert.equal(evaluateHtmlReport(after["report.html"], reportData, { + findingsPath: fixture.findingsPath, + }).status, "pass"); + }); +}); + +test("record-fix-output leaves portable HTML artifacts unchanged on pre-publish failures", async () => { + await withTempDir(async (root) => { + const fixture = await htmlFixFixture(root); + const before = await htmlArtifactSnapshot(fixture); + + await assert.rejects(recordFixOutput({ + workspace: fixture.workspace, + findings: fixture.findingsPath, + findingId: fixture.finding.id, + expectedRevision: 1, + result: fixture.resultPath, + consumeResult: true, + }), (error) => error?.code === "STALE_FIX_OUTPUT_REVISION"); + assert.deepEqual(await htmlArtifactSnapshot(fixture), before); + assert.ok((await readFile(fixture.resultPath, "utf8")).length > 0); + + await assert.rejects(recordFixOutput({ + workspace: fixture.workspace, + findings: fixture.findingsPath, + findingId: fixture.finding.id, + expectedRevision: 0, + result: fixture.resultPath, + consumeResult: true, + prepareHtmlReport() { + throw Object.assign(new Error("simulated HTML validation failure"), { + code: "INVALID_UPDATED_HTML_REPORT", + }); + }, + }), (error) => error?.code === "INVALID_UPDATED_HTML_REPORT"); + assert.deepEqual(await htmlArtifactSnapshot(fixture), before); + assert.ok((await readFile(fixture.resultPath, "utf8")).length > 0); + + await writeFile(path.join(fixture.runDir, "canvas.json"), "{}\n"); + await assert.rejects(recordFixOutput({ + workspace: fixture.workspace, + findings: fixture.findingsPath, + findingId: fixture.finding.id, + expectedRevision: 0, + result: fixture.resultPath, + }), (error) => error?.code === "INVALID_REPORT_CONTEXT"); + assert.deepEqual(await htmlArtifactSnapshot(fixture), before); + }); +}); + +test("record-fix-output keeps compact Qoder reports bound to canvas.json", async () => { + await withTempDir(async (root) => { + const workspace = path.join(root, "workspace"); + const runDir = path.join(workspace, ".qoder", "better-harness", "run"); + const findingsPath = path.join(runDir, "findings.json"); + const resultPath = path.join(root, "result.json"); + const targetPath = "fix-output/owner.md"; + const split = splitTaskLoopFindings(projectTaskLoopFindings(reportSource())); + const finding = split.findings.findings[0]; + await mkdir(path.join(workspace, path.dirname(targetPath)), { recursive: true }); + await mkdir(runDir, { recursive: true }); + await writeFile(path.join(workspace, targetPath), "# Owner\n"); + await writeFile(findingsPath, `${JSON.stringify(split.findings, null, 2)}\n`); + await writeFile(path.join(runDir, "report.canvas.tsx"), "export default function Report() { return null; }\n"); + await writeFile(resultPath, JSON.stringify({ + actualOutput: [{ + action: "updated", + artifact: finding.expectedArtifact, + name: `${finding.expectedArtifact} owner`, + scope: "Project", + path: targetPath, + summary: "Updated the verified Qoder report owner.", + }], + assignmentSummary: assignmentSummary(), + })); + const before = await readFile(findingsPath, "utf8"); + + await assert.rejects(recordFixOutput({ + workspace, + findings: findingsPath, + findingId: finding.id, + expectedRevision: 0, + result: resultPath, + }), (error) => error?.code === "INVALID_REPORT_CONTEXT" && /requires canvas\.json/u.test(error.message)); + assert.equal(await readFile(findingsPath, "utf8"), before); + + await writeFile(path.join(runDir, "canvas.json"), "{}\n"); + await assert.rejects(recordFixOutput({ + workspace, + findings: findingsPath, + findingId: finding.id, + expectedRevision: 0, + result: resultPath, + }), (error) => error?.code === "INVALID_TASK_LOOP_FINDINGS"); + assert.equal(await readFile(findingsPath, "utf8"), before); + }); +}); + test("record-fix-output rejects a finding bound to a sibling package", async () => { await withTempDir(async (root) => { const workspace = path.join(root, "packages", "b");