Record old Security review findings privately

[JacobStephens2]

Summary

Old vulnerabilities from a guidance Security review now become private records while introduced findings retain their Self-merge hold.

Guidance Security review (one session, pinned merge base)
  introduced findings → PR Security outcome → Self-merge hold
  old findings → private report under logs/security-reviews/
    match fingerprint → reuse existing record and Day-shift grade
    new fingerprint → draft advisory / private security-finding issue
      keep scored reproduction → later Security run may fix when allowed
  Delivery → push → ready PR → Repair → eligible Self-merge

Closes #552

Evidence

• Before: cargo test --test security_review an_old_finding_is_recorded_privately_without_holding_self_merge -- --exact failed: the guidance prompt had no private report destination and the Merge run ended incomplete.
  After: The same test passes: one draft advisory, private proof-of-concept evidence retained, no old-finding details in the PR, and successful Self-merge even with security-fix enabled.
• Before: cargo test --test security_review review_uses_the_published_base_when_the_local_base_is_stale -- --exact failed because classification used stale local main.
  After: The same test passes against the pinned merge base from origin/main.
• Reviewer regressions reproduced before fixes and retained: the next Security run now delivers a fix PR for both storage types; a checkout changed before/during Security review is rejected before recording.
• Scenario coverage: mixed old/introduced findings on Claude Code and Codex; duplicate fingerprints within and across reviews; open/closed private issues and draft/published/closed advisories; preserving Day-shift grades; missing, malformed and inconsistent reports.

Validation: cargo test: 1,628 passed, 0 failed, 1 ignored, including all 17 Security-review tests; cargo fmt --check, cargo check --all-targets, and cargo clippy --all-targets -- -D warnings passed.

Seams under test: The Spec's end-to-end scenario harness: the real binary and Git, with fake GitHub and Harness CLIs. Observe private records, PR body/readiness, Self-merge, the report contract in Session prompts, and confidentiality in command output.

Merge Danger

Door: two-way

Blast Radius: Delivery

Enabled guidance reviews now require a private JSON report and pre_existing_count in their final line. A missing or invalid report makes the review incomplete and holds Self-merge. Private records persist on GitHub; reverting the code does not remove them.

Review

Reviewed with thirdshift-code-review against the published issue-427 commit 422d954bfad171bca25c2d40e2fde846660becf8.

• Standards: 1 finding, fixed. Worktree observations now retain acquired-instance checks, including a check after the Security session.
• Spec: 2 findings, fixed. New private records include the canonical reproduced outcome, severity and fix size needed by later Security runs; the merge base uses the full remote ref to avoid local-tag shadowing.
• Changed files left unread: none under Standards or Spec; both final files-read lists cover all 10 changed files. Reports are saved as .thirdshift-review-igjmf8/standards.md and .thirdshift-review-igjmf8/spec.md in the Run worktree.

Unaddressed findings

Standards

None. Every reported regression failed as written; the findings are fixed and their tests are retained.

Spec

None. Every reported regression failed as written; the findings are fixed and their tests are retained.

Built with codex · gpt-6.1-sol · xhigh