Skip to content

Run repository-specific validation in Review runs - #1722

Merged
JacobStephens2 merged 9 commits into
issue-706from
issue-1704
Nov 4, 2026
Merged

JacobStephens2 merged 9 commits into
issue-706from
issue-1704

Conversation

@JacobStephens2

@JacobStephens2 JacobStephens2 commented Nov 4, 2026 •

Copy link
Copy Markdown
Owner

Summary

 Review run
   review and address findings
+  run repository final validation and review.commands_by_repo commands
   optional Security review
 Delivery
+  repeat validation if a later editing Step changed the head
+  withhold that report when it copies a private Security report
+  report each command's pass, failure or reason it could not run
+  discard disposable validation output after successful publication

The User config's [review.commands_by_repo] maps case-insensitive owner/repo names to ordered shell commands. With no matching commands, repository agent-instruction final validation still runs. Suites invalid in worktrees are reported not run against the branch, never passing. Recommend-only validates too. The manual, three hand-written site pages and generated prompt catalogue describe this behavior.

Merged the updated issue-706, preserving its optional Security review and adding validation of the final head after later fixes.

Closes #1704

Evidence

  • Before: the command-routing regression failed with unknown section [review]; the exact Spec reproductions failed for suppressed artifact comments, a blocked second review, and validation of a pre-Security commit.
    After: all three reproductions are retained and pass. extra_validation_covers_the_commit_after_security_fixes executes the configured command and compares its checked SHA with the pushed SHA.
  • Security: before the withhold, validation_does_not_publish_private_security_report_text posted a harmless local sentinel from the private report (exit 101). That publication did not occur on merge base 6bd88433 (exit 0). After the fix the test passes and the comment uses the withheld sentence.
  • Focused checks pass: 27 active Review/Review-security/validation tests; 51 Setup unit and 35 Setup integration tests; 41 config tests; 13 generated-prompt checks; docs regeneration check; cargo clippy --all-targets -- -D warnings. The worktree regression suite passed 175 tests before reconciliation.
  • CI exposed complete-config Setup fixtures missing the new section. Their focused failures were reproduced and their expectations repaired; the complete Setup unit and integration files pass.
  • Final local scripts/affected-tests origin/issue-706: all 61 site tests passed in 91.87s; the full Rust selection stopped after 377.69s at three Console account-revocation cases because the shared user's OS file-watch quota was exhausted. All three exact cases pass under an isolated user without changing host limits. This limitation is not counted as a passing full suite.
  • Full validation of 428f6d88 passed in CI: 3,663 Rust tests passed, 78 skipped, across 82 binaries. The cargo nextest run step took 17m 10s including 1m 01s compilation (test execution 968.174s). Site and Console CI jobs also passed. Successful CI on this exact head is reused instead of repeating the full suite on the quota-limited local host.

Test seams

  • thirdshift review <PR URL> with real Git and fake GitHub/Harness adapters: command handoff, reporting, both modes, cleanup, repeated reviews and validation after Security fixes.
  • UserConfig::parse: strict repository routing, defaults and shell-text preservation.
  • thirdshift setup: complete configuration and preservation of configured commands.

Review coverage

The initial review used issue-706 at 0474c388. After upstream reconciliation, fresh independent Standards and Spec reviewers used issue-706 at 6bd8843335d5e815ae4e3fd245b53b4af8530aa4, including the final fix at 428f6d88.

Both axes read all 26 changed files, including generated output. Changed files left unread: none. Generated-output exclusions: none. Reports and counts are saved in .thirdshift-review-825hFR/standards.md, spec.md and summary.json.

Unaddressed findings

Standards

None. The original P3 duplication heuristic was addressed by shared repository-key validation. The reconciliation review raised no additional Standards findings.

Spec

None. Both P2 findings were reproduced with their exact commands, fixed and kept as regressions: validation-artifact lifecycle and validation after later Security fixes.

Security

None. Delivery withholds a later validation report when it contains private Security review material, covered by validation_does_not_publish_private_security_report_text.

Security review outcome

No unaddressed introduced findings.

Merge Danger

Door: two-way

Blast Radius: Review

Repository-specific validation, final-head reporting and successful Review-run cleanup. Tracked edits, unpushed commits and failed-run work retain their preservation checks.

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

Delivery posts the validation session's final message after a Security review has written its private report. Keep that message off the pull request when it contains a stored report path or identifying field.
Preserve Review validation alongside the author Review Pass in the domain
index and hand-written site pages. Regenerate the manual from merged sources.

Validation:
- cargo clippy --all-targets -- -D warnings and cargo fmt --all -- --check pass.
- scripts/affected-tests origin/issue-706: all 61 site tests pass; Rust stops
  after the shared user's file-watch quota prevents three Console tests.
- cargo nextest run --no-fail-fast, three threads and Console tests under a
  separate UID: 3668 passed, 7 failed, 79 skipped. Wall time 3602.03s includes
  the 0.75s cached test build. All seven failures pass exact targeted reruns:
  six file-watch cases under the separate UID and one systemd scope collision
  under the normal user. The full run is not counted as an unqualified pass.
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