diff --git a/docs/better-harness-doc-links.mmd b/docs/better-harness-doc-links.mmd index 05b3131..58b0407 100644 --- a/docs/better-harness-doc-links.mmd +++ b/docs/better-harness-doc-links.mmd @@ -50,6 +50,7 @@ flowchart LR n_skills_better_harness_references_asset_demand_reconciliation_md["asset-demand-reconciliation.md"] n_skills_better_harness_references_finding_bound_fix_md["finding-bound-fix.md"] n_skills_better_harness_references_findings_review_md["findings-review.md"] + n_skills_better_harness_references_manual_direct_fix_md["manual-direct-fix.md"] n_skills_better_harness_references_project_harness_md["project-harness.md"] n_skills_better_harness_references_session_evidence_md["session-evidence.md"] n_skills_better_harness_references_session_repeated_workflows_md["session-repeated-workflows.md"] @@ -69,6 +70,7 @@ flowchart LR n_skills_better_harness_SKILL_md --> n_skills_better_harness_references_asset_demand_reconciliation_md n_skills_better_harness_SKILL_md --> n_skills_better_harness_references_finding_bound_fix_md n_skills_better_harness_SKILL_md --> n_skills_better_harness_references_findings_review_md + n_skills_better_harness_SKILL_md --> n_skills_better_harness_references_manual_direct_fix_md n_skills_better_harness_SKILL_md --> n_skills_better_harness_references_project_harness_md n_skills_better_harness_SKILL_md --> n_skills_better_harness_references_session_evidence_md n_skills_better_harness_SKILL_md --> n_skills_better_harness_references_session_repeated_workflows_md diff --git a/docs/specs/2026-07-30-html-finding-fix-callback.md b/docs/specs/2026-07-30-html-finding-fix-callback.md new file mode 100644 index 0000000..324ee12 --- /dev/null +++ b/docs/specs/2026-07-30-html-finding-fix-callback.md @@ -0,0 +1,167 @@ +# Keep copied finding fixes bound without exposing local paths + +## Traceability + +- Spec ID: html-finding-fix-callback +- Review: QoderAI/better-harness#39 +- Status: Implemented + +## Intent + +Make `Copy AI Fix` from a durable Codex HTML report executable by the +activated Better Harness Skill when the report remains in its generated +workspace-relative location. Preserve callback-free report compatibility and +the report's portable reader surface without embedding renderer-added absolute +workspace or artifact paths. + +The callback is local action transport, not an analysis conclusion or +AI-authored reader field. Rendering must leave the persisted `findings.json`, +paired Markdown report, finding scores, and reviewed `aiFixPrompt` unchanged. + +## Acceptance Scenarios + +- AC-1: A report renders two Copy controls only for each finding whose reviewed + `aiFixPrompt` is a non-empty, non-whitespace string. A finding without a + usable prompt retains its details control but has no action row and no + callback metadata. +- AC-2: A bound report embeds only a normalized workspace-relative POSIX route + to the final `report.html`, plus finding ids and current revisions. It does + not embed renderer-added absolute `workspacePath`, `findingsPath`, + `dataPath`, or `target.path` values in either interaction payload. +- AC-3: When the current document is a `file:` URL whose decoded path ends in + the exact embedded report route, Copy derives the workspace root and sibling + final `findings.json` path locally. It appends exactly one + `` block with contract + `better-harness-fix-output/v1`, the derived exact paths, exact finding id, + and the finding's current `actualOutputRevision` or `0`. +- AC-4: When the renderer has no safe action context, or the report is opened + through HTTP or from a file path that does not match the expected route, all + clipboard and manual-copy paths transport the unchanged reviewed + `aiFixPrompt` without a callback or renderer-added local paths. +- AC-5: An intentionally empty action binding enables only the compatibility + fallback. A present but malformed, incomplete, stale, cross-bound, absolute, + or escaping binding fails deterministic HTML validation and never becomes a + compatibility fallback. +- AC-6: Action transport remains finding-scoped across multiple rows and + produces parseable callback JSON for Windows drive paths, Windows UNC paths, + macOS paths, Linux paths, spaces, Unicode, and punctuation. +- AC-7: The final route is derived from the published run directory, never the + temporary staging directory or source findings path. Existing output-location + allocation and replacement behavior remains unchanged. +- AC-8: The rendered `findings.json`, paired `report.md`, source + `aiFixPrompt`, Qoder Canvas behavior, report scoring, finding eligibility, + and visible details content remain unchanged. + +## Non-goals + +- Automatically submit, approve, or execute the copied fix. +- Add a ChatGPT Desktop host bridge, deep link, network request, sidecar action + file, new persisted finding field, or runtime file read. +- Guarantee a finding-bound callback after the HTML file is moved, downloaded + outside its generated route, shared to another machine, or served over HTTP. + Those states retain the reviewed manual prompt only. +- Change Qoder Canvas handoffs or broaden one finding-bound repair into a new + Harness review. +- Repair previously generated HTML files in place. + +## Plan and Tasks + +1. Project the full report into a minimal reader-safe interaction payload + containing only actionable finding ids and unchanged reviewed prompts. + Remove the full `reportData` object from embedded interaction JSON. + (AC-1, AC-2, AC-8) +2. Build deterministic machine action metadata from the target workspace and + final report location. Emit only a safe workspace-relative report route and + per-finding revision metadata; emit an intentionally empty binding when the + report is not safely beneath the workspace. (AC-1, AC-2, AC-5, AC-7) +3. Resolve file-local bindings inside the self-contained interaction + controller. Require exact route suffix matching, derive platform-native + workspace and sibling findings paths, and append the v1 callback only at + copy time. (AC-3, AC-6) +4. Preserve the raw-prompt compatibility route for intentionally empty + bindings, HTTP previews, and moved file reports. Keep malformed declared + bindings fail-closed. (AC-4, AC-5) +5. Render and validate Copy controls only for usable prompts. Validate exact + interaction and action payload projections plus per-finding action counts. + (AC-1, AC-5) +6. Add unit, renderer, CLI, and browser regression coverage for callback + generation, fallback states, empty prompts, binding tampering, final routes, + and cross-platform URL/path conversion. (AC-1..AC-8) + +## Test and Review Evidence + +- Interaction coverage: + `node --test test/html-report-interactions.test.mjs` must verify original + file binding, callback JSON parsing, Clipboard API/legacy/manual parity, + intentional-empty fallback, HTTP fallback, moved-file fallback, malformed + binding failure, and Windows/macOS/Linux paths. +- Renderer and CLI coverage: + `node --test test/harness-report-render-cli.test.mjs` must verify minimal + embedded interaction data, relative final report routes, actionable finding + counts, no renderer-added absolute paths, no staging routes, exact revisions, + and validator rejection of tampering. +- AC-8: compare rendered `findings.json` and Markdown semantics with the input, + and run the existing Canvas and report-render suites. +- Documentation integrity: + `node scripts/doc-link-graph/cli.mjs skills/better-harness`, then + `node --test test/doc-link-graph.test.mjs`. +- Complete validation: `npm test`, `npm run pack:verify`, and + `git diff --check`. +- Visual validation: serve a generated HTML report through the local HTML + preview, use Playwright to inspect console/page errors, exercise raw-prompt + HTTP fallback and the details dialog, and retain a screenshot. +- Repository preview smoke: run `npm run preview`, then request + `http://localhost:58575/health` and + `http://localhost:58575/canvas-module.js`. If the external Canvas SDK + runtime is unavailable, record the limitation separately rather than + claiming success. +- Risk: route suffix matching could bind a moved report to the wrong workspace. + Accept only safe relative segments and an exact decoded `file:` suffix; + otherwise transport only the reviewed prompt. +- Risk: Windows URL decoding and separator conversion could create invalid JSON + or paths. Generate the callback with `JSON.stringify` and parse it in + cross-platform fixtures. +- Risk: compatibility fallback could hide a corrupted binding. Distinguish an + explicit empty binding from any declared malformed or mismatched binding and + keep the latter fail-closed. + +## Revision Baseline + +- PR #39 review confirmed that callback-free renders expose Copy buttons whose + interaction controller can no longer find the original prompt. +- The same review confirmed that empty prompts currently receive callback-only + actions and that the hand-written Windows callback fixture is invalid JSON. +- The pre-change HTML already embeds absolute `dataPath` and `target.path`; + the first callback implementation additionally embeds `workspacePath` and + `findingsPath`. This revision removes renderer-added absolute paths from both + interaction payloads rather than documenting a local-path disclosure. + +## Implementation Evidence + +- The renderer now embeds only actionable finding ids/prompts and a separate + final workspace-relative report route with ids/revisions. Context-free or + unsafe output locations emit `{ reportRoute: null, findings: [] }`. +- The browser controller validates both minimal payloads, derives native + workspace and sibling `findings.json` paths only for an exact matching + `file:` route, and uses the same computed text for Clipboard API, legacy, and + manual-copy paths. HTTP and moved-file routes preserve the raw reviewed + prompt; malformed declared bindings fail closed. +- Copy controls and binding rows are omitted for empty or whitespace-only + prompts while details remain visible. The HTML validator enforces the minimal + payloads, relative route, revision, finding binding, and actionable counts. +- `node --test test/html-report-interactions.test.mjs` passed 9/9, including + Windows drive, Windows UNC, macOS, Linux Unicode, local-file, HTTP, moved-file, + empty-binding, and malformed-binding cases. All HTML-focused render tests + passed; the enclosing file has one unrelated local symlink-permission case. +- Playwright verified the original local file callback fields and the raw-prompt + HTTP fallback, opened the details dialog, retained a screenshot, and found no + report-script warning or error. The temporary HTTP server produced only a + missing `favicon.ico` 404. +- `npm run pack:verify` passed with 365 npm entries and 388 runtime ZIP entries. + The full `npm test` run passed 1036/1046 with 6 platform skips; the remaining + 4 failures are local Windows `EPERM` errors from tests that require symlink + creation and do not touch this change. +- `npm run preview` could not start because no Canvas SDK runtime or + `CANVAS_SDK_MEDIA_DIR` / `CANVAS_SDK_ROOT` is available, so `/health` and + `/canvas-module.js` were unavailable. The self-contained HTML browser route + has no Canvas runtime dependency. diff --git a/docs/specs/2026-07-30-manual-direct-fix-routing.md b/docs/specs/2026-07-30-manual-direct-fix-routing.md new file mode 100644 index 0000000..d1ea71c --- /dev/null +++ b/docs/specs/2026-07-30-manual-direct-fix-routing.md @@ -0,0 +1,125 @@ +# Route explicit manual fixes without requiring a report callback + +## Traceability + +- Spec ID: manual-direct-fix-routing +- Review: QoderAI/better-harness#39 +- Status: Implemented + +## Intent + +Let ChatGPT Desktop users invoke an explicit command such as +`/better-harness fix this issue` with a concrete problem, bounded change +request, and validation instructions without first generating a Better Harness +report. Preserve the Skill's existing review trigger surface and keep the +manual route separate from report-bound repair and ordinary Harness reviews. + +The manual route may change only the task-local workspace authorized by the +prompt. It must not discover, update, or claim repair progress for a +`findings.json`. + +## Acceptance Scenarios + +- AC-1: A callback-free slash-command request selects Manual Direct Fix only + when its first instruction is an explicit `fix`, `repair`, or `修复` + directive and the prompt identifies a concrete problem or requested outcome. + The agent inspects the smallest relevant owner, applies the bounded change, + and runs the smallest relevant validation without requesting report callback + fields or starting a new Harness review. +- AC-2: A request containing a machine-owned + `` callback takes precedence and continues to use + the existing Finding-bound Fix protocol, including exact path, finding, and + revision validation plus `record-fix-output`. +- AC-3: Requests whose first instruction is review, evaluation, reporting, or + another Harness-review intent continue to use the evidence-bundle, + reconciliation, and durable-report workflow. A mixed request such as + `review my harness and fix issues` does not grant direct mutation authority. +- AC-4: Manual direct fixes never search for a recent report, infer a Finding, + read or update `findings.json`, run `record-fix-output`, or claim Assignment + Summary, Repair Progress, revision, or score changes. +- AC-5: If an explicit manual repair request does not identify a concrete + problem, requested outcome, or target-local scope, the agent asks for the + missing repair brief; it does not ask the user to hand-author machine callback + fields. +- AC-6: The root Skill retains its established review-trigger keywords and + appends only `manual direct fixes` for discoverability. Step 5 continues to + route Finding-bound repair through its reference and preserves the separate + independent post-fix reassessment constraint. + +## Non-goals + +- Treat fuzzy repair intent or a repair clause inside a review request as direct + mutation authority. +- Weaken callback validation or add fallback inference to report-bound repair. +- Convert ordinary `/better-harness` reviews into code-changing tasks. +- Backfill or migrate existing reports. +- Change Qoder Canvas, HTML action transport, report schemas, or renderer + artifacts. + +## Plan and Tasks + +1. Restore the root Skill's original description and append + `manual direct fixes`; restore the Step 5 Finding-bound route and independent + reassessment language without exceeding the root prompt budget. (AC-6) +2. Add a deterministic entry router before the evidence workflow: a valid + callback selects Finding-bound Fix; an explicit leading manual-fix directive + selects Manual Direct Fix; all other invocations retain the review/report + route. (AC-1..AC-3) +3. Tighten the Manual Direct Fix reference around explicit instruction, + task-local authority, minimal owner inspection, validation evidence, and + strict separation from report state. (AC-1, AC-4, AC-5) +4. Add positive and negative routing coverage for English and Chinese explicit + fixes, callback-bound repair, vague fixes, ordinary reviews, and mixed + review-and-fix requests. (AC-1..AC-6) +5. Regenerate the documentation routing graph and verify packaged artifacts. + (AC-1..AC-6) + +## Test and Review Evidence + +- Root contract and route coverage: + `node --test test/better-harness-skill.test.mjs`. +- Fresh-context behavior: + - explicit English `fix` and `repair`, plus Chinese `修复`, select Manual + Direct Fix without callback fields; + - callback-bearing input selects Finding-bound Fix; + - ordinary review and `review my harness and fix issues` retain the + evidence/report route; + - vague explicit fixes request only the missing task-local brief. +- Documentation integrity: + `node scripts/doc-link-graph/cli.mjs skills/better-harness`, then + `node --test test/doc-link-graph.test.mjs`. +- Complete validation: `npm test`, `npm run pack:verify`, and + `git diff --check`. +- Risk: fuzzy routing could broaden mutation authority. The explicit leading + directive and negative mixed-intent AC keep review requests on the read-only + evidence route. +- Risk: root guidance could lose trigger recall or independent repair review. + Preserve the original keywords and reassessment language and enforce both in + the focused Skill contract test. + +## Revision Baseline + +- PR #39 review confirmed that the previous description rewrite removed + established trigger keywords and that the `repair intent` predicate was too + broad. +- The same review confirmed that Step 5 dropped an independent post-fix + reassessment guarantee without matching acceptance coverage. +- Earlier environment-specific backup movement is not acceptance evidence for + this revision. Validation must come from the current worktree and CI. + +## Implementation Evidence + +- The root description retains all established review keywords and appends only + `manual direct fixes`. The root stays within its contract budget at 217 lines + and 11,997 UTF-8 bytes. +- Callback-bearing input routes first to Finding-bound Fix. Without a callback, + only an explicit leading `fix`, `repair`, or Unicode-literal Chinese `fix` + directive routes to Manual Direct Fix; review/evaluation/reporting and mixed + review-and-fix requests remain on Step 1. +- Step 5 again requires the Finding-bound reference and a separate independent + post-fix agent before verified finding state or Repair Progress can change. +- `node --test test/better-harness-skill.test.mjs` passed 13/13 and enforces the + original trigger keywords, explicit positive route, mixed-intent negative + route, report-state boundary, independent reassessment, and prompt budget. +- The regenerated documentation graph passed 6/6 integrity tests. Package and + full-suite results are recorded in the paired HTML callback spec. diff --git a/docs/specs/2026-07-31-windows-file-identity.md b/docs/specs/2026-07-31-windows-file-identity.md new file mode 100644 index 0000000..8d49bfd --- /dev/null +++ b/docs/specs/2026-07-31-windows-file-identity.md @@ -0,0 +1,65 @@ +# Keep secret-scan file identity checks portable + +## Traceability + +- Spec ID: windows-file-identity +- Review: QoderAI/better-harness#39 CI follow-up after rebasing onto main +- Status: Implemented + +## Intent + +Keep the secret scanner's open-time file identity guard fail-closed while +accepting an unchanged regular file on Windows, where `lstat` can expose an +unavailable zero device id and the opened handle can expose the real device id. + +## Acceptance Scenarios + +- AC-1: A clean explicit regular file scans with complete coverage and exit 0 + on Windows, macOS, and Linux when its non-zero inode remains unchanged. +- AC-2: Different non-zero inodes fail identity validation. +- AC-3: When both observations expose non-zero device ids, different devices + fail identity validation; a zero device id on only one observation is treated + as unavailable rather than contradictory. +- AC-4: When inode identity is unavailable, the existing conservative metadata + comparison remains required. +- AC-5: Symlink, containment, no-follow, and coverage-gap behavior remains + unchanged. + +## Non-goals + +- Relax path containment or follow symbolic links. +- Treat mismatched non-zero identity fields as equivalent. +- Change secret detection rules or public diagnostics. + +## Plan and Tasks + +1. Compare non-zero inode identity first and compare device ids only when both + observations provide them. (AC-1..AC-3) +2. Preserve the metadata fallback when inode identity is unavailable. (AC-4) +3. Run the focused secret-scan suite, complete repository suite, and Windows CI. + (AC-1..AC-5) + +## Test and Review Evidence + +- Focused regression: + `node --test test/agent-guardrails-secret-scan.test.mjs` must restore the clean + explicit-file case without changing link and containment expectations. +- Complete validation: `npm test`, `npm run pack:verify`, and `git diff --check`. +- Baseline evidence: on the current Windows host, one unchanged file reported + the same non-zero inode through both observations while `lstat.dev` was `0` + and the opened handle's `stat.dev` was non-zero; the pre-change CLI exited 3 + with `scan target changed while it was being opened`. + +## Implementation Evidence + +- `sameFileIdentity` now rejects different non-zero device ids, then requires + matching inodes whenever either observation exposes one, so a zero inode on + only one side stays contradictory. A zero device id on one observation no + longer contradicts the matching inode. The existing metadata fallback applies + only when neither observation exposes an inode. +- `node --test test/agent-guardrails-secret-scan.test.mjs` passed 18 tests with + 0 failures and 3 symlink-permission skips. The formerly failing clean explicit + file case now reports complete coverage and exit 0. +- The full repository run also passed the clean-file regression; its only 4 + remaining failures are separate tests whose symlink fixtures cannot be + created under this local Windows permission profile. diff --git a/scripts/agent-guardrails/secret-scan.mjs b/scripts/agent-guardrails/secret-scan.mjs index d52ccf1..a20d77c 100644 --- a/scripts/agent-guardrails/secret-scan.mjs +++ b/scripts/agent-guardrails/secret-scan.mjs @@ -727,8 +727,9 @@ function sameFileIdentity(beforeOpen, opened) { const openedIno = Number(opened?.ino ?? 0); const beforeDev = Number(beforeOpen?.dev ?? 0); const openedDev = Number(opened?.dev ?? 0); - if (beforeIno !== 0 || openedIno !== 0 || beforeDev !== 0 || openedDev !== 0) { - return beforeIno === openedIno && beforeDev === openedDev; + if (beforeDev !== 0 && openedDev !== 0 && beforeDev !== openedDev) return false; + if (beforeIno !== 0 || openedIno !== 0) { + return beforeIno === openedIno; } return beforeOpen?.size === opened?.size && beforeOpen?.mtimeMs === opened?.mtimeMs diff --git a/scripts/harness-analysis/render-report.mjs b/scripts/harness-analysis/render-report.mjs index cae9499..5d1fa80 100644 --- a/scripts/harness-analysis/render-report.mjs +++ b/scripts/harness-analysis/render-report.mjs @@ -178,7 +178,10 @@ async function writeArtifacts({ reportData, artifactDir, runDir }) { await writeJson(path.join(artifactDir, "findings.json"), findingsJson); artifacts.push(artifact("findings.json", runDir)); await writeFile(path.join(artifactDir, "report.md"), renderMarkdown(reportData)); - await writeFile(path.join(artifactDir, "report.html"), renderHtml(reportData)); + await writeFile( + path.join(artifactDir, "report.html"), + renderHtml(reportData, { findingsPath: path.join(runDir, "findings.json") }), + ); artifacts.push(artifact("report.md", runDir), artifact("report.html", runDir)); } @@ -240,7 +243,11 @@ async function validateArtifacts({ reportData, artifactDir, runDir, artifacts, o allowStandaloneTaskLoop: true, })); if (reportData.mode === "html") { - checks.push(evaluateHtmlReport(readFileSync(path.join(artifactDir, "report.html"), "utf8"), reportData)); + checks.push(evaluateHtmlReport( + readFileSync(path.join(artifactDir, "report.html"), "utf8"), + reportData, + { findingsPath: path.join(runDir, "findings.json") }, + )); } } diff --git a/scripts/harness-analysis/renderers/html-interactions.mjs b/scripts/harness-analysis/renderers/html-interactions.mjs index 625a13c..0faf02e 100644 --- a/scripts/harness-analysis/renderers/html-interactions.mjs +++ b/scripts/harness-analysis/renderers/html-interactions.mjs @@ -1,19 +1,140 @@ export function installHtmlReportInteractions({ document, navigator, + location, labels, schedule = (callback, delay) => globalThis.setTimeout(callback, delay), }) { document.documentElement.classList.remove("no-js"); - const reportDataElement = document.getElementById("harness-report-data"); - const reportData = reportDataElement - ? JSON.parse(reportDataElement.textContent || "{}") - : {}; - const findings = new Map( - (Array.isArray(reportData.findings) ? reportData.findings : []) - .map((finding) => [String(finding?.id ?? ""), finding]), - ); + function readJson(id) { + const element = document.getElementById(id); + if (!element) return { ok: false, value: null }; + try { + return { ok: true, value: JSON.parse(element.textContent || "{}") }; + } catch { + return { ok: false, value: null }; + } + } + + function hasOnlyKeys(value, keys) { + return value && typeof value === "object" && !Array.isArray(value) + && Object.keys(value).every((key) => keys.includes(key)); + } + + function safeReportRoute(value) { + if (typeof value !== "string" || value.length === 0 || value.includes("\\")) return false; + if (value.startsWith("/") || /^[A-Za-z]:/u.test(value)) return false; + const segments = value.split("/"); + return segments.at(-1) === "report.html" + && segments.every((segment) => segment && segment !== "." && segment !== ".."); + } + + const reportDataState = readJson("harness-report-data"); + const reportActionsState = readJson("harness-report-actions"); + const prompts = new Map(); + const actions = new Map(); + let bindingMode = "invalid"; + let reportRoute = null; + + if (reportDataState.ok + && hasOnlyKeys(reportDataState.value, ["findings"]) + && Array.isArray(reportDataState.value.findings)) { + let validPrompts = true; + for (const finding of reportDataState.value.findings) { + const id = String(finding?.id ?? ""); + if (!hasOnlyKeys(finding, ["id", "aiFixPrompt"]) + || !id + || typeof finding.aiFixPrompt !== "string" + || finding.aiFixPrompt.trim().length === 0 + || prompts.has(id)) { + validPrompts = false; + break; + } + prompts.set(id, finding.aiFixPrompt); + } + + const payload = reportActionsState.value; + if (validPrompts + && reportActionsState.ok + && hasOnlyKeys(payload, ["reportRoute", "findings"]) + && Array.isArray(payload.findings)) { + let validActions = true; + for (const finding of payload.findings) { + const id = String(finding?.id ?? ""); + if (!hasOnlyKeys(finding, ["id", "expectedRevision"]) + || !id + || !Number.isInteger(finding.expectedRevision) + || finding.expectedRevision < 0 + || actions.has(id)) { + validActions = false; + break; + } + actions.set(id, finding); + } + const sameIds = actions.size === prompts.size + && [...prompts.keys()].every((id) => actions.has(id)); + if (validActions && payload.reportRoute === null && actions.size === 0) { + bindingMode = "empty"; + } else if (validActions && safeReportRoute(payload.reportRoute) && sameIds) { + bindingMode = "bound"; + reportRoute = payload.reportRoute; + } + } + } + + function resolveBindingPaths() { + try { + const url = new URL(String(location?.href ?? "")); + if (url.protocol !== "file:") return null; + const pathname = decodeURIComponent(url.pathname); + const windowsDrive = /^\/[A-Za-z]:\//u.test(pathname); + const windowsUnc = Boolean(url.hostname); + const suffix = `/${reportRoute}`; + const comparablePath = windowsDrive || windowsUnc ? pathname.toLowerCase() : pathname; + const comparableSuffix = windowsDrive || windowsUnc ? suffix.toLowerCase() : suffix; + if (!comparablePath.endsWith(comparableSuffix)) return null; + + const workspaceUrlPath = pathname.slice(0, pathname.length - suffix.length); + const reportDirectory = pathname.slice(0, pathname.lastIndexOf("/")); + const findingsUrlPath = `${reportDirectory}/findings.json`; + const toNativePath = (value) => { + if (windowsDrive) return value.slice(1).replace(/\//gu, "\\"); + if (windowsUnc) { + const host = decodeURIComponent(url.hostname); + return `\\\\${host}${value.replace(/\//gu, "\\")}`; + } + return value || "/"; + }; + return { + workspacePath: toNativePath(workspaceUrlPath), + findingsPath: toNativePath(findingsUrlPath), + }; + } catch { + return null; + } + } + + function promptForFinding(findingId) { + if (bindingMode === "invalid") return null; + const id = String(findingId); + const prompt = prompts.get(id); + if (typeof prompt !== "string" || prompt.trim().length === 0) return null; + if (bindingMode === "empty") return prompt; + const paths = resolveBindingPaths(); + if (!paths) return prompt; + const action = actions.get(id); + if (!action) return null; + const callback = { + contract: "better-harness-fix-output/v1", + workspacePath: paths.workspacePath, + findingsPath: paths.findingsPath, + findingId: id, + expectedRevision: action.expectedRevision, + }; + return `${prompt}\n\n\n${JSON.stringify(callback)}\n`; + } + const status = document.getElementById("copy-status"); const manualDialog = document.getElementById("manual-copy-dialog"); const manualText = document.getElementById("manual-copy-text"); @@ -55,7 +176,7 @@ export function installHtmlReportInteractions({ } async function copyFinding(findingId, trigger) { - const prompt = findings.get(String(findingId))?.aiFixPrompt; + const prompt = promptForFinding(findingId); if (typeof prompt !== "string" || prompt.length === 0) { report(labels.missingPrompt, "error"); return false; @@ -155,5 +276,5 @@ export function renderHtmlInteractionScript(language) { manualCopy: "Automatic copy was blocked. Copy the selected prompt manually.", missingPrompt: "No AI Fix prompt is available for this finding.", }; - return ``; + return ``; } diff --git a/scripts/harness-analysis/renderers/html.mjs b/scripts/harness-analysis/renderers/html.mjs index 482a3ee..c909ddd 100644 --- a/scripts/harness-analysis/renderers/html.mjs +++ b/scripts/harness-analysis/renderers/html.mjs @@ -1,3 +1,5 @@ +import path from "node:path"; + import { isAgentWorkLoopReport } from "../fluency-dimensions.mjs"; import { renderHtmlInteractionScript } from "./html-interactions.mjs"; import { renderMarkdown } from "./markdown.mjs"; @@ -67,6 +69,48 @@ function serializeForScript(value) { .replace(/\u2029/gu, "\\u2029"); } +function hasAiFixPrompt(finding) { + return typeof finding?.aiFixPrompt === "string" && finding.aiFixPrompt.trim().length > 0; +} + +export function buildHtmlInteractionData(reportData) { + return { + findings: list(reportData?.findings) + .flatMap((finding, index) => (hasAiFixPrompt(finding) + ? [{ id: String(finding?.id ?? index), aiFixPrompt: finding.aiFixPrompt }] + : [])), + }; +} + +function finalReportRoute(workspacePath, findingsPath) { + if (!path.isAbsolute(workspacePath) || !path.isAbsolute(findingsPath)) return null; + const reportPath = path.join(path.dirname(findingsPath), "report.html"); + const relative = path.relative(workspacePath, reportPath); + if (!relative || path.isAbsolute(relative)) return null; + const segments = relative.split(/[\\/]/u); + if (segments.some((segment) => !segment || segment === "." || segment === "..")) return null; + return segments.join("/"); +} + +export function buildHtmlReportActions(reportData, { findingsPath } = {}) { + const workspacePath = String(reportData?.target?.path ?? ""); + const resolvedFindingsPath = String(findingsPath ?? ""); + const reportRoute = finalReportRoute(workspacePath, resolvedFindingsPath); + if (!reportRoute) return { reportRoute: null, findings: [] }; + return { + reportRoute, + findings: list(reportData?.findings) + .flatMap((finding, index) => (hasAiFixPrompt(finding) + ? [{ + id: String(finding?.id ?? index), + expectedRevision: Number.isInteger(finding?.actualOutputRevision) + ? finding.actualOutputRevision + : 0, + }] + : [])), + }; +} + function findingPromptParts(prompt, language) { const lines = String(prompt ?? "").split(/\r?\n/u).map((line) => line.trim()); const validationIndex = lines.findIndex((line) => /^##\s+Validation$/iu.test(line)); @@ -234,6 +278,7 @@ function renderFindings(reportData, language) { const findings = list(reportData.findings); if (findings.length === 0) return `

${renderVisibleText(copy(language, "No reviewed findings were retained.", "没有保留已复核 finding。"), language)}

`; return `
${findings.map((row, index) => { + const actionable = hasAiFixPrompt(row); const prompt = findingPromptParts(row.aiFixPrompt, language); const expected = textLines(row.expectedOutput ?? row.expectedOutcome); const refs = list(row.dimensionRefs).map((ref) => dimensions.get(ref) ?? ref); @@ -254,7 +299,7 @@ function renderFindings(reportData, language) {

${renderVisibleText(row.reason ?? "", language)}

- + ${actionable ? `` : ""}
@@ -266,7 +311,7 @@ function renderFindings(reportData, language) { ${expected.length ? `
${renderVisibleText(copy(language, "Expected Output", "预期结果"), language)}
    ${expected.map((item) => `
  1. ${renderVisibleText(item, language)}
    2. `).join("")}
` : ""} ${prompt.checks.length ? `
${renderVisibleText(copy(language, "Acceptance Checks", "验收检查"), language)}
    ${prompt.checks.map((item) => `
  • ${renderVisibleText(item, language)}
    • `).join("")}
` : ""}
- + ${actionable ? `` : ""}
@@ -375,9 +420,11 @@ function renderHtmlBody(reportData) { ${section("methodology", copy(language, "05 · Boundary", "05 · 边界"), copy(language, "Evidence and methodology", "证据与方法"), renderEvidence(summary, language), copy(language, "Reader-safe evidence only.", "仅使用读者安全证据。"), language)}`; } -export function renderHtml(reportData) { +export function renderHtml(reportData, actionContext) { const language = reportData.language === "zh-CN" ? "zh-CN" : "en"; const title = copy(reportData.language, "Harness Insights", "Harness 洞察"); + const reportActions = buildHtmlReportActions(reportData, actionContext); + const interactionData = buildHtmlInteractionData(reportData); return ` @@ -443,14 +490,15 @@ ${renderHtmlBody(reportData)} - + + ${renderHtmlInteractionScript(reportData.language)} `; } -export function evaluateHtmlReport(htmlText, reportData) { +export function evaluateHtmlReport(htmlText, reportData, actionContext) { const text = String(htmlText ?? ""); const errors = []; const warnings = []; @@ -459,6 +507,7 @@ export function evaluateHtmlReport(htmlText, reportData) { ["viewport", //iu], + ["embedded report actions", //iu], ["interaction controller", //iu], ["copy status", /id="copy-status"[^>]+role="status"[^>]+aria-live="polite"/iu], ["manual copy fallback", /([\s\S]*?)<\/script>/iu, + )?.[1]; + if (actionPayloadText !== undefined) { + try { + const actualActions = JSON.parse(actionPayloadText); + const expectedActions = buildHtmlReportActions(reportData, actionContext); + if (JSON.stringify(actualActions) !== JSON.stringify(expectedActions)) { + errors.push("report.html finding action payload does not match the exact relative binding metadata"); + } + } catch (error) { + errors.push(`report.html finding action payload is invalid: ${error.message}`); + } + } + const interactionPayloadText = text.match( + /([\s\S]*?)<\/script>/iu, + )?.[1]; + if (interactionPayloadText !== undefined) { + try { + const actualData = JSON.parse(interactionPayloadText); + const expectedData = buildHtmlInteractionData(reportData); + if (JSON.stringify(actualData) !== JSON.stringify(expectedData)) { + errors.push("report.html interaction data does not match the minimal reviewed prompt projection"); + } + } catch (error) { + errors.push(`report.html interaction data is invalid: ${error.message}`); + } + } const interactionController = text.match( /([\s\S]*?)<\/script>/iu, )?.[1] ?? ""; @@ -508,8 +585,9 @@ export function evaluateHtmlReport(htmlText, reportData) { errors.push(`report.html finding dialog count ${findingDialogs} does not match reviewed findings ${list(reportData?.findings).length}`); } const copyActions = (text.match(/data-copy-finding=/gu) ?? []).length; - if (copyActions !== list(reportData?.findings).length * 2) { - errors.push(`report.html copy action count ${copyActions} does not match expected ${list(reportData?.findings).length * 2}`); + const actionableFindings = list(reportData?.findings).filter(hasAiFixPrompt); + if (copyActions !== actionableFindings.length * 2) { + errors.push(`report.html copy action count ${copyActions} does not match expected ${actionableFindings.length * 2}`); } const detailActions = (text.match(/data-view-finding-dialog=/gu) ?? []).length; if (detailActions !== list(reportData?.findings).length) { @@ -521,7 +599,7 @@ export function evaluateHtmlReport(htmlText, reportData) { const bindings = [ ["card", `data-finding-id="${findingId}"`, 1], ["dialog", `data-finding-dialog-id="${findingId}"`, 1], - ["copy action", `data-copy-finding="${findingId}"`, 2], + ["copy action", `data-copy-finding="${findingId}"`, hasAiFixPrompt(finding) ? 2 : 0], ["detail action", `data-view-finding-dialog="${dialogId}"`, 1], ["close action", `data-close-finding-dialog="${dialogId}"`, 1], ]; diff --git a/skills/better-harness/SKILL.md b/skills/better-harness/SKILL.md index 41c9e77..34887d1 100644 --- a/skills/better-harness/SKILL.md +++ b/skills/better-harness/SKILL.md @@ -1,30 +1,31 @@ --- name: better-harness -description: Use when /better-harness reviews the outer coding-agent Harness for lifecycle controls, repeated work, project feedback, agent assets, session outcomes, repair planning, durable reports, or finding-bound fixes. Invoke only via slash command. +description: Use when /better-harness reviews the outer coding-agent Harness for lifecycle controls, repeated work, project feedback, agent assets, session outcomes, repair planning, durable reports, finding-bound fixes, or manual direct fixes. Invoke only via slash command. --- # Better Harness -Review the coding-agent operating system: context, execution, control, feedback, -and learning. Sessions, project, and Agent assets stay independent until joined. +Review the coding-agent system: context, execution, control, feedback, +and learning; keep Sessions, project, and Agent assets independent. ## Step 1: Resolve Scope and Collect the Evidence Bundle -If the prompt contains ``, read -[Finding-bound Fix](references/finding-bound-fix.md) before any probe. +Route: +- ``: [Finding-bound Fix](references/finding-bound-fix.md). +- No callback plus leading `fix`, `repair`, or `\u4fee\u590d`: [Manual Direct Fix](references/manual-direct-fix.md). +- Review/evaluation/reporting or mixed review-and-fix: Step 1. Resolve the Skill path, `` as `../..`, a supported ``, -and `` as -` /scripts/better-harness.mjs`. Stop if any owner is -missing; never select another cache or runtime by search order. +and `` as ` /scripts/better-harness.mjs`. Stop if +any owner is missing; never select another cache or runtime by search order. Resolve the absolute target, decision, acceptance boundary, risks, locale (the user's request language unless explicitly changed), output mode, provider scope, and evidence depth. Quick uses three assets or Episodes and the previous 7 days; normal uses five and the previous 30 days. Default Qoder to durable Canvas and other providers to durable HTML. Only an explicit inline or no-files request writes nothing. -Keep providers separate. Use the current provider by default; include multiple -supported providers only for an explicitly authorized project-wide review. +Keep providers separate. Use the current one unless project-wide review +explicitly authorizes multiple supported providers. Qoder project Memory title metadata is part of the selected workspace baseline. Memory bodies, Codex Memory, Qoder global Memory, user-home, raw Session, installed-plugin, marketplace, and @@ -44,14 +45,13 @@ Memory metadata requires `--include-memories`; user/global or installed-Plugin metadata requires `--include-user-home`. Apply both when both scopes are authorized. Neither flag authorizes Memory bodies. -It freezes resolved topology, provider, window, depth, limit, and authority. -Before delegation, read -`bundle.context.topology.target`; report `kind`, `route`, and `packageRoute` -(`memberRoute` or `null`). Providers must agree. It returns `sessionEvidence`, -`projectHarness`, `agentCustomize`, and the lead envelope. The Agent Customize -lane contains the bounded `lint`, `inventory`, and `integrity` envelopes from -one shared asset snapshot. Keep every lane and stage status and each provider -distinct. Use the individual `session-analysis facts`, `core-change-watch +It freezes topology, provider, window, depth, limit, and authority. Before +delegation, read `bundle.context.topology.target`; report `kind`, `route`, and `packageRoute` +(`memberRoute` or `null`). Providers must agree. It returns +`sessionEvidence`, `projectHarness`, `agentCustomize`, and the lead envelope. +Agent Customize holds bounded `lint`, `inventory`, and `integrity` envelopes +from one shared asset snapshot. Keep lane/stage status and providers distinct. +Use the individual `session-analysis facts`, `core-change-watch evidence-pack`, `coding-agent-practices asset-baseline`, or `harness analyze` command only to diagnose a named unavailable or truncated owner; do not substitute diagnostic output into the bundle or rerun all owners. Counts for @@ -62,9 +62,9 @@ unavailable; do not replace the missing review with an `unobserved` disposition. If the provider discovers or the user supplies a historical insight source, -the lead may inspect only the few explicitly authorized architecture/history -notes. Never assume or search a conventional path. Historical notes cannot -prove current behavior, configured capability, or effectiveness. +the lead may inspect only a few authorized architecture/history notes. +Never assume or search a conventional path; notes cannot prove current behavior, +configured capability, or effectiveness. ## Step 2: Run Three Independent Evidence Passes @@ -198,8 +198,8 @@ bare directory, or an output-file inventory. ## Step 5: Follow Up -- Finding-bound repair uses [Finding-bound Fix](references/finding-bound-fix.md). - A separate independent post-fix agent may update verified finding state and +- Finding-bound repair uses [Finding-bound Fix](references/finding-bound-fix.md); + a separate independent post-fix agent may update verified finding state and Repair Progress; Loop Effectiveness waits for comparable later Task Episodes. - Usage/model questions use `session-analysis usage-summary` once. - Repeated work continues through @@ -209,8 +209,8 @@ bare directory, or an output-file inventory. [Core Change Watch](../../references/project-harness/core-change-watch.md), and [Report Routing](../../templates/reporting/routing.md). -The default durable route authorizes only renderer-owned report artifacts in -the resolved host root. Other creation, activation, mutation, cleanup, -scheduling, external writes, and high-risk access require task-local authority. -When an owner or required value is unresolved, stop with the exact condition to -resume; do not invent a substitute artifact or inspect internal validators. +The durable route authorizes only renderer-owned artifacts in its host +root. Other creation, activation, mutation, cleanup, scheduling, external +writes, and high-risk access require task-local authority. If an owner or value +is unresolved, stop with the condition to resume; do not invent a +substitute artifact or inspect internal validators. diff --git a/skills/better-harness/references/manual-direct-fix.md b/skills/better-harness/references/manual-direct-fix.md new file mode 100644 index 0000000..fdaf9bd --- /dev/null +++ b/skills/better-harness/references/manual-direct-fix.md @@ -0,0 +1,33 @@ +# Manual Direct Fix + +Use this route only when an explicitly invoked `/better-harness` request has no +`` callback and its first instruction is an explicit +`fix`, `repair`, or `\u4fee\u590d` (Chinese `fix`) directive. It is a task-local repair, not a Harness +review or report-bound Finding repair. A review, evaluation, reporting, or +mixed request such as `review my harness and fix issues` remains on the ordinary +Harness review route. + +## Validate the Repair Brief + +Require a concrete problem or requested outcome in the current workspace. +Treat named files, observed behavior, acceptance conditions, and validation +commands as the task boundary. If neither the problem nor requested outcome is +concrete enough to locate the smallest relevant owner, ask for that missing +repair brief. Do not ask the user to provide `workspacePath`, `findingsPath`, +`findingId`, or `expectedRevision`. + +## Apply and Verify + +Follow the target repository's instructions. Diagnose the stated problem, +inspect the smallest relevant owner, apply only the authorized change, and run +the smallest relevant validation. Do not collect a Harness evidence bundle, +launch the three review agents, reconcile findings, or render a new report. + +This route must not search for or discover a recent `findings.json`. It must not read +or update `findings.json` as report state, and must not call +`record-fix-output`. It must not claim an Assignment Summary, Repair Progress, +revision update, score change, or any other report result. + +Report only the concrete files changed and validation actually completed. If +the repair cannot be made safely within the supplied task boundary, stop with +the exact missing target, authority, or validation evidence. diff --git a/templates/reporting/html-visual.md b/templates/reporting/html-visual.md index 233b7ff..d601ac3 100644 --- a/templates/reporting/html-visual.md +++ b/templates/reporting/html-visual.md @@ -63,11 +63,22 @@ raw evidence anchors in an evidence table or appendix. HTML cannot assume a host chat API, native prompt injection, or host-specific deep links. For each shared finding/action row, render compact `Copy AI Fix` -and `View details` controls when an AI fix exists. Copy the exact reviewed -prompt locally, report copy success truthfully, and fall back to selected -manual-copy text when browser clipboard paths fail. Keep the full cause, -expected output, and acceptance checks available in the scoped details dialog -and as readable no-JavaScript and print content. +and `View details` controls only when a non-empty reviewed AI fix exists; keep +`View details` when it does not. Keep `aiFixPrompt` unchanged in +`findings.json`. Embed only the actionable id/prompt projection plus a separate +machine binding containing the final `report.html` workspace-relative POSIX +route, finding id, and current output revision. Do not embed renderer-added +absolute workspace or artifact paths. + +When opened from its original matching `file:` location, derive the workspace +and sibling final `findings.json` paths at copy time and append one +renderer-owned `` callback. For HTTP previews, moved +files, or intentionally context-free renders, copy the unchanged reviewed +prompt without a callback. Keep malformed declared bindings fail-closed. Report +copy success truthfully and use the same computed text for clipboard, legacy, +and selected manual-copy fallbacks. Keep the full cause, expected output, and +acceptance checks in the scoped details dialog and readable no-JavaScript and +print content. ## Markdown Report Addendum diff --git a/test/better-harness-skill.test.mjs b/test/better-harness-skill.test.mjs index dc801ee..7cb40c4 100644 --- a/test/better-harness-skill.test.mjs +++ b/test/better-harness-skill.test.mjs @@ -28,6 +28,17 @@ test("Better Harness keeps one compact five-step owner", () => { assert.match(description, /^Use when /); assert.match(description, /\/better-harness/); assert.match(description, /Invoke only via slash command/); + for (const keyword of [ + "lifecycle controls", + "repeated work", + "project feedback", + "agent assets", + "session outcomes", + "repair planning", + "durable reports", + "finding-bound fixes", + "manual direct fixes", + ]) assert.match(description, new RegExp(keyword)); assert.ok(skill.split("\n").length < 220, "root Skill must remain compact"); assert.ok(Buffer.byteLength(skill) < 12_000, "root Skill must stay below the prompt budget"); @@ -62,6 +73,7 @@ test("Better Harness keeps one compact five-step owner", () => { assert.match(skill, /writes every distinct supported finding and freezes final severity/); assert.match(skill, /shape at most\s+three priority moves/); assert.match(skill, /Repair Progress; Loop Effectiveness waits/); + assert.match(skill, /separate independent post-fix agent/); }); test("non-Qoder providers default to validated durable HTML with an explicit inline opt-out", () => { @@ -385,7 +397,7 @@ test("finding-bound fixes remain slash-command owned and independently reassesse const canvas = read("templates/canvas/better-harness-insights.canvas.tsx"); const html = read("scripts/harness-analysis/renderers/html.mjs"); - assert.match(skill, /prompt contains ``/); + assert.match(skill, /``: \[Finding-bound Fix\]/); assert.doesNotMatch(skill, /legacy fix callbacks/); assert.match(skill, /\[Finding-bound Fix\]\(references\/finding-bound-fix\.md\)/); assert.match(fix, /initiating handoff must explicitly invoke `\/better-harness`/); @@ -411,6 +423,25 @@ test("finding-bound fixes remain slash-command owned and independently reassesse assert.match(html, /Asset Health \/ Repair Progress/); }); +test("manual direct fixes do not require report callbacks or mutate report state", () => { + const skill = read("skills/better-harness/SKILL.md"); + const directFixPath = "skills/better-harness/references/manual-direct-fix.md"; + assertExistingFile(directFixPath); + const directFix = read(directFixPath); + + assert.match(skill, /No callback plus leading `fix`, `repair`, or `\\u4fee\\u590d`:[\s\S]+\[Manual Direct Fix\]\(references\/manual-direct-fix\.md\)/i); + assert.match(skill, /Review\/evaluation\/reporting or mixed review-and-fix: Step 1/i); + assert.match(directFix, /first instruction is an explicit[\s\S]+`fix`, `repair`, or `\\u4fee\\u590d` \(Chinese `fix`\) directive/i); + assert.match(directFix, /`review my harness and fix issues` remains on the ordinary[\s\S]+review route/i); + assert.match(directFix, /concrete problem or requested outcome/i); + assert.match(directFix, /smallest relevant owner/i); + assert.match(directFix, /smallest relevant validation/i); + assert.match(directFix, /must not (?:search for|discover)[\s\S]+`findings\.json`/i); + assert.match(directFix, /must not (?:read|update)[\s\S]+`findings\.json`/i); + assert.match(directFix, /do not ask[\s\S]+`workspacePath`[\s\S]+`findingsPath`[\s\S]+`findingId`[\s\S]+`expectedRevision`/i); + assert.match(directFix, /must not claim[\s\S]+Repair Progress/i); +}); + test("bug-diagnosis examples remain provider-neutral evidence-bound Skill shapes", () => { const exampleRoot = "case-studies/agent-customize/bug-diagnosis-skills"; const frontendPath = `${exampleRoot}/reproduce-frontend-bug/SKILL.md`; diff --git a/test/harness-report-render-cli.test.mjs b/test/harness-report-render-cli.test.mjs index 4c76159..8135f19 100644 --- a/test/harness-report-render-cli.test.mjs +++ b/test/harness-report-render-cli.test.mjs @@ -117,6 +117,15 @@ function sampleFindings() { }; } +function embeddedJson(html, id) { + const payload = html.match(new RegExp( + `