Skip to content

Run requested Security reviews during Review runs - #1723

Merged
JacobStephens2 merged 2 commits into
issue-706from
issue-1705
Nov 4, 2026
Merged

JacobStephens2 merged 2 commits into
issue-706from
issue-1705

Conversation

@JacobStephens2

@JacobStephens2 JacobStephens2 commented Nov 4, 2026 •

Copy link
Copy Markdown
Owner

Summary

 Review run
   review-and-address
+  security-review (flag or User config; security.harness)
   delivery
+    push both sets of fixes and include Security findings in the comment

no-security-review overrides the setting. Recommend-only skips the Security review. Old findings use the existing private recording path. The manual, site pages and generated prompt catalogue describe the behavior.

Closes #1705

Evidence

  • Before: cargo test --test review_security security_fixes_follow_addressed_findings_and_reach_the_pr_branch -- --exact failed with unexpected review argument: --security-review.
    After: the regression passes; all 7 new CLI tests pass, including execution order, published fixes, config overrides, Security comments, harness/fallback, recommendations, copied Processes and private records. The 14 existing Review run tests pass.
  • cargo check --bin thirdshift, generated prompt verification (13 checks), npm run docs:check --prefix site, and site tests (61 cases) pass.
  • scripts/affected-tests origin/issue-706: site checks passed; the Rust run stopped on a Console SSE HTTP timeout (274 passed, 1 failed, 3380 not run; 870.74s including compilation). The exact Console test passed alone and reported the host's file-watch limit was reached.
  • The reduced-thread full rerun (cargo nextest run --test-threads 3 --no-fail-fast) had 521 passes and four Console session/stream failures with file-watch-limit warnings; it was stopped after confirming that local limitation (three active tests interrupted, 3127 not run; 980.51s). The full local suite is not green. No Console code changed in this PR. The PR's full Rust CI check is pending; the CI Console and site jobs, formatting/clippy stages, and Linux artifact builds passed. The shared test-helper repair was separately rerun: 7 passed.

Test seams

The thirdshift review <PR URL> CLI with real Git and fake GitHub/Harness processes: Security review selection, execution order, published commits, public comments, harness selection/fallback, and read-only recommendations.

Merge Danger

Door: two-way

Blast Radius: Review

Review runs can start an additional Security session when requested. Publication retains the existing captured-head checks; pre-existing findings remain private.

Unaddressed findings

Standards

None. The review raised one possible Duplicated Code smell in the test scripts; it was addressed by sharing the session dispatch helper and confirmed addressed by the reviewer.

Spec

None. No Spec findings were raised.

Security

None. Guidance review of 0474c3886581c997c3aa72bae07425185cf6d1a5...HEAD found no introduced vulnerability with a concrete trust-boundary failure. The new Review run path publishes only the deterministic Security entry (introduced titles, or a public incomplete reason), keeps pre-existing records on the private recording path, runs only when the flag or [security] review asks, lets no-security-review override that setting, skips the session for --recommend-only, and selects security.harness with the usual fallback.

Security review outcome

No unaddressed introduced findings.

Review coverage

Independent Standards and Spec reviews use issue-706 at 0474c3886581c997c3aa72bae07425185cf6d1a5. Both reviewers read all 17 changed files, including generated output, and checked the test-helper repair. Changed files left unread: none. Generated-output exclusions: none. Reports and finding counts were saved in the Session's .thirdshift-review-ySh40a/ directory.

Built with codex codex-cli 0.164.0 · gpt-6.1-sol · xhigh

@JacobStephens2
JacobStephens2 merged commit 6bd8843 into issue-706 Nov 4, 2026
16 checks passed
@JacobStephens2
JacobStephens2 deleted the issue-1705 branch November 4, 2026 03:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant