Skip to content

fix: a CR marked rejected is checked when its branch changes anything - #455

Merged
itercharles merged 1 commit into
mainfrom
fix/rejected-cr-is-checked
Oct 21, 2026
Merged

itercharles merged 1 commit into
mainfrom
fix/rejected-cr-is-checked

Conversation

@itercharles

Copy link
Copy Markdown
Owner

Summary

Closes the loophole noted in #454: a CR whose triage_result says rejected skipped the code and impact_analysis checks, and any agent can write that field, so an agent could reject its own CR and change requirements or code under it.

  • design_validation.impact_findings now decides once:
    • rejected, and the branch changes only the CR → skipped as before (only the dependents check runs, as it always did);
    • rejected, and the branch changes another item or any code → a triage_result finding ("a rejected CR changes nothing", naming up to five changes) and every impact check, as for an accepted CR.
  • The rejection test is removed from _unreviewed_code and _check_impact_analysis. triage_rejected, cr_triage_rejection and build plan's early stop on a rejection are unchanged.
  • docs/interface.md: one sentence in the verify impact paragraph. CHANGELOG ### Fixed under Unreleased.

Measure

New seeded omission rejected_to_skip_the_checks: the CR marks itself rejected while the branch also changes src/export.py, which it never accounts for.

  • Before this change: passed every gate.
  • After: stopped by verify impact.

Seeded omissions stopped: 9 of 12 (10 passed, 3 xfailed).

Tests

  • test_verify_impact.py: the "rejected is not checked" test now uses a branch that changes only the CR; new test that a rejected CR whose branch changes items fails with triage_result: plus the normal findings.
  • test_a_changed_file_reaches_its_module.py: new test that a rejected CR whose branch changes only a code file is stopped.
  • Full suite: 1949 passed, 9 skipped, 3 xfailed (before the rebase onto main; the three affected test files re-run after it).

Manual review

  • Drafted by a Haiku subagent. The seed case it was given did not exercise the loophole (it was already stopped by the dependents check); it was replaced and verified before and after the fix.

🤖 Generated with Claude Code

A rejection means no design was generated, yet the mark alone exempted a
CR from the code and impact_analysis checks, and any agent can write it.
`impact_findings` now decides once: a rejected CR whose branch changes
only the CR is skipped as before; one whose branch changes another item
or any code fails with a `triage_result` finding and runs every check.

A new seeded omission (the CR rejects itself while the branch changes
export code it never accounts for) passed every gate before this change
and is now stopped by `verify impact`: 9 of 12 stopped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@itercharles
itercharles merged commit 2f60cd4 into main Oct 21, 2026
11 checks passed
@itercharles
itercharles deleted the fix/rejected-cr-is-checked branch October 21, 2026 20:04
itercharles added a commit that referenced this pull request Oct 21, 2026
The gates stop 9 of 12 seeded omissions: G1 closed (#453), the
triage_result gate removed (#454), and the rejected-CR loophole closed
(#455), with its new case stopped. Mark G1, G3 and the triage gate done
in the iteration 1 inventory, list what is still open (G2, G4, the test
point that only warns), and set iteration 3 as next.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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