fix(reports): bind copied fixes to findings and route manual fixes without callbacks

[sxyseo]

Summary

This PR now implements the full follow-up solution requested in review, rebased onto current main (92a7e74):

• fix(reports): keep copied finding fixes bound to their source finding without embedding renderer-owned absolute local paths.
• fix(skills): support callback-free manual fixes through a narrow, explicit route while preserving the established review behavior.
• fix(security): keep the latest-main secret scanner's file-identity check portable on Windows.

Changes

• Restore the established Better Harness trigger vocabulary and add manual direct fixes without replacing existing discovery terms.
• Route only requests that begin with an explicit fix, repair, or Chinese fix directive to the callback-free manual path; review and mixed review-and-fix requests continue through the review workflow.
• Preserve the report-state boundary for manual fixes and restore an independent post-fix reassessment.
• Embed only the reviewed actionable prompt data plus a workspace-relative final report route and finding revision.
• Derive the finding callback at copy time only when the report is opened from its exact local file location. HTTP, moved, and context-free reports copy the reviewed raw prompt instead.
• Omit Copy controls for empty prompts and fail closed on malformed or cross-bound action metadata.
• Generate and parse callback JSON in tests across Windows drive, UNC, macOS, Linux, and Unicode paths.
• Treat a zero Windows device id as unavailable only when the non-zero inode still matches, while continuing to reject different non-zero devices or inodes.

Specs

• docs/specs/2026-07-30-html-finding-fix-callback.md
• docs/specs/2026-07-30-manual-direct-fix-routing.md
• docs/specs/2026-07-31-windows-file-identity.md

Validation

• Focused Skill, HTML interaction/render, documentation graph, and secret-scan tests pass.
• npm run pack:verify passes.
• Playwright validates both exact local-file callback copying and HTTP raw-prompt fallback.
• GitHub Actions passes on Ubuntu Node 22, Ubuntu Node 24, macOS Node 22, and Windows Node 22: https://github.com/QoderAI/better-harness/actions/runs/30645033164

[phodal]

Review: scope creep beyond the two specs

The core HTML callback binding work (html.mjs / render-report.mjs and its tests) looks solid. However, several changes go beyond what the two specs authorize, and a few regressions slipped in.

1. SKILL.md description rewrite is out of spec scope

The original description lists eight trigger keywords (lifecycle controls, repeated work, project feedback, agent assets, session outcomes, repair planning, durable reports, finding-bound fixes) — this is the skill's trigger-routing surface. The PR collapses it to "reviews the outer coding-agent Harness or handles an explicitly requested manual or finding-bound fix", which:

• Degrades trigger recall for legitimate review invocations (e.g. "durable reports", "session outcomes").
• Broadens authority: "handles an explicitly requested manual … fix" turns /better-harness into a general-purpose fix command.
• Is covered by no AC in either spec (2026-07-30-manual-direct-fix-routing.md only asks for an entry router before Step 1), and is not mentioned in the PR body — an untraceable behavior change under the repo's change-traceability rules.

Request: revert the description. If manual fixes must be discoverable, append manual direct fixes to the existing keyword list instead of rewriting it.

2. Manual Direct Fix routing predicate is too broad

"If it expresses repair intent" is a fuzzy classifier and contradicts the spec's own non-goal ("Convert ordinary /better-harness reviews into code-changing tasks"). A request like "review my harness and fix issues" would bypass the entire evidence/review workflow and mutate code. Tighten the route to require an explicit fix instruction, and add a matching AC.

3. Step 5 silently drops the independent post-fix reassessment constraint

The PR deletes "Finding-bound repair uses [Finding-bound Fix] … A separate independent post-fix agent may update verified finding state" from Step 5. That is a behavioral guarantee (independent reassessment after repair), not covered by any AC, and the existing tests do not guard it. Please restore it.

4. Interaction regression for callback-less renders

html-interactions.mjs now reads only harness-report-actions. AC-3 claims a render without an action context stays "backward compatible" via an empty action binding — but Copy AI Fix buttons are still rendered unconditionally (2 per finding), and every click now fails with the missing-prompt error, whereas the previous code copied aiFixPrompt from harness-report-data. The validator passes while the interaction is broken, and no test covers this path. Either restore the fallback or add a regression test that defines the intended empty-binding behavior.

5. Smaller issues

• buildHtmlReportActions builds "\n\n<better-harness-fix-output>…" even when aiFixPrompt is empty, bypassing the old empty-prompt guard — users can copy a callback with no repair plan.
• The self-contained, shareable HTML now embeds absolute local paths (workspacePath, findingsPath). Sharing the report leaks local filesystem paths, and the callback is guaranteed to break off-machine.
• In test/html-report-interactions.test.mjs, the Windows fixture callback contains "C:\work\fixture" — \w is an invalid JSON escape, so the fixture's callback is unparseable. The test never parses it, so it passes, but it contradicts AC-3's "escapes Windows paths safely" claim.
• The spec evidence notes npm run preview smoke was skipped and that a packaged plugin backup was moved outside the repository — a task-external environment mutation worth flagging.

Summary of requested changes

1. Revert the description and Step 5 edits (append a keyword instead of rewriting).
2. Tighten the Manual Direct Fix trigger to explicit fix instructions and cover it with an AC.
3. Add a regression test for empty action binding + copy buttons, or restore the fallback.
4. Fix the Windows-path test fixture and the empty-aiFixPrompt guard.

[sxyseo]

Thanks for the detailed review. I checked each point against the current branch and the two specs. I agree that the trigger-surface rewrite and the Step 5 deletion are out of scope, that the current manual-fix predicate is too broad, and that the callback-less interaction is a real regression.

A few facts I verified:

• The branch rewrites the original eight-keyword Skill description instead of only extending it, and it removes the independent post-fix reassessment wording from Step 5. I will revert both changes, append manual direct fixes to the original trigger list, and add regression coverage for the reassessment constraint.
• renderHtml() without action context currently emits { findings: [] }, while the renderer still emits two Copy buttons per finding. Since html-interactions.mjs now reads only harness-report-actions, those buttons cannot copy the original aiFixPrompt. I will restore a narrowly scoped legacy fallback.
• The hand-written Windows callback fixture is indeed invalid JSON after JavaScript string decoding (\w is not a valid JSON escape). I will generate it with JSON.stringify, parse the extracted callback in the test, and assert the exact Windows paths.
• Absolute renderer paths are not entirely new to this PR: before this change, the embedded harness-report-data already contained absolute dataPath and target.path. However, this PR adds an absolute findingsPath and duplicates the workspace path in the action payload, so it does expand the disclosure and makes the off-machine action boundary ambiguous.

I propose the following full fix, with the specs moved back to Draft before implementation:

1. Restore the original Skill description and Step 5 semantics. Add only manual direct fixes to the existing trigger keywords.
2. Make Manual Direct Fix deterministic: it is selected only by an explicit leading fix instruction such as /better-harness fix ..., /better-harness repair ..., or /better-harness 修复 ..., with a concrete task-local brief. Ordinary review requests, including review my harness and fix issues, remain on the evidence/review/report route. I will add positive and negative ACs for these cases.
3. Treat only findings with a non-empty reviewed aiFixPrompt as copy-action findings. Empty prompts produce neither an action row nor Copy buttons; Details remains available.
4. Restore callback-less compatibility only when the entire action binding is intentionally empty. In that state, Copy uses the original reviewed aiFixPrompt. A present but incomplete, stale, or cross-bound action payload remains fail-closed and never falls back.
5. Remove renderer-added absolute paths from the HTML interaction payload. Instead of embedding the full reportData, embed only the minimal reader-safe interaction data needed for fallback (finding id and reviewed aiFixPrompt). For bound actions, embed a workspace-relative expected report route plus finding id and revision, not absolute paths.
6. When opened from disk, the interaction code will accept only a file: URL whose decoded path matches that expected workspace-relative report route. It can then derive the workspace root and sibling findings.json path locally and construct the exact v1 callback at copy time. If the report is moved, downloaded outside the expected route, or served over HTTP, Copy falls back to the reviewed prompt without a callback or local paths. No host bridge, network call, or sidecar is required.
7. Rebuild the Windows/macOS/Linux path tests around parsed callback JSON, add tests for original-file binding, moved/HTTP fallback, empty action binding, malformed non-empty binding, and empty prompts, then run the focused suites, doc-link graph, full npm test, pack:verify, and HTML Playwright checks. I will replace the old external backup-move evidence with fresh validation from the current worktree and report the Canvas SDK preview limitation separately if it remains unavailable.

Could you confirm that this is the intended contract?

• Original generated file: location: Copy produces the exact finding-bound callback.
• No action context, moved/downloaded report, or HTTP preview: Copy produces only the reviewed aiFixPrompt, with no callback and no renderer-added local paths.
• A declared but malformed/mismatched action binding: validation fails; no compatibility fallback.

If that matches your expectation, I will update the two specs first and implement against those ACs.

[sxyseo]

Implemented the full solution from the review and rebased the branch onto current main (92a7e74).

The review points are addressed as follows:

• The Skill description keeps the established trigger vocabulary and only adds manual direct fixes.
• Callback-free manual routing is now limited to an explicit leading fix, repair, or Chinese fix directive. Review/evaluation requests, including mixed review-and-fix requests, remain on the review path.
• Manual direct fixes do not mutate finding/report state, and the independent post-fix reassessment is restored.
• HTML actions now embed only the reviewed actionable prompt plus a workspace-relative final report route and the finding revision. The callback is derived at copy time only for the exact local file: report location.
• HTTP, moved, and context-free reports copy the reviewed raw prompt; empty prompts have no Copy controls; malformed or cross-bound metadata fails closed.
• The hand-written Windows callback fixture was replaced with generated and parsed JSON coverage for Windows drive paths, UNC paths, macOS/Linux paths, and Unicode. Clipboard, fallback, and manual-copy paths use the same computed prompt.
• Rebasing onto the latest main exposed a Windows file-identity mismatch in the new secret-scan guard (dev=0 from one stat observation versus a non-zero device with the same inode). The fix treats the zero device id as unavailable only when the inode matches, while still rejecting different non-zero devices or inodes.

Validation is green in GitHub Actions on Ubuntu Node 22, Ubuntu Node 24, macOS Node 22, and Windows Node 22: https://github.com/QoderAI/better-harness/actions/runs/30645033164. Focused Skill, HTML interaction/render, doc-link, secret-scan, package verification, and local/HTTP Playwright checks also pass.

Could you please confirm that this matches the full approach you requested, particularly the narrow manual route and copy-time callback derivation, and re-review the PR?