fix(opencode): correct stale draft/head-moved test expectation from #1697 - #1709
fix(opencode): correct stale draft/head-moved test expectation from #1697#1709seonghobae wants to merge 1 commit into
Conversation
…1697 test_draft_exemption_fails_closed_when_live_head_moved in tests/test_opencode_live_draft_state_regression.py still asserted the pre-#1697 behavior (exit 1, "head moved while validating live") that PR #1697 deliberately replaced with a graceful exit 0 ("still a draft on the live exact head"). #1697's own PR body verification scope did not include this test file, so the mismatch went uncaught and this test has been failing against origin/main ever since #1697 merged. #1697's reorder is correct: for both draft+head-unchanged and draft+head-moved, the script exits via the identical unconditional draft-check before ever comparing heads and before any consequential action (no receipt-gate fetch, OIDC exchange, dispatch, or polling) -- there is no distinct-case correctness reason to fail one and not the other. Rebased on the real production incident #1697 cites (contextual-orchestrator PR #1000, spurious hard failure at https://github.com/ContextualWisdomLab/contextual-orchestrator/actions/runs/33548447878/job/100066104033). Renamed to test_draft_exemption_survives_when_live_head_moved and rewrote the docstring to cite #1697 and its evidence, matching this file's own pattern of sourcing rationale in docstrings. Kept (not deleted) since this harness uniquely combines a moved live head with event-level stale-draft-conversion metadata, a distinct angle from #1697's own new tests in test_opencode_required_verdict_regression.py. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 3 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| PR #1697 (`fix(opencode): retire stale draft/head dispatches without | ||
| false failure`) deliberately reordered both step bodies to check | ||
| closed/draft *before* head-SHA-match, following a real production | ||
| incident: contextual-orchestrator PR #1000 stayed draft the whole time, | ||
| but a push landed between the event snapshot and this step's live | ||
| re-fetch, and the old head-match-first ordering failed hard with | ||
| `::error::...head moved while validating live review state.` and exit 1 | ||
| even though no review was ever being requested against a stable target | ||
| (https://github.com/ContextualWisdomLab/contextual-orchestrator/actions/runs/33548447878/job/100066104033). |
There was a problem hiding this comment.
🔍 Cross-repository reference breaks traceability
The new docstring uses “contextual-orchestrator PR #1000,” violating the required owner/repo#num format for cross-repository references.
Was this helpful? React with 👍 or 👎 to provide feedback.
| assert result.returncode == 0, result.stderr | ||
| assert "still a draft on the live exact head" in result.stdout | ||
| assert "head moved" not in result.stdout |
There was a problem hiding this comment.
🔍 Moved head labeled exact
With different event and live heads, the asserted live exact head message is false. The test now preserves a misleading operator diagnostic.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Superseded — this exact conflict (test_draft_exemption_fails_closed_when_live_head_moved vs. #1697's deliberate check reorder) was independently investigated and fixed in parallel via #1705, which was merged into #1710 and landed on |
1 participant
Summary
tests/test_opencode_live_draft_state_regression.py::test_draft_exemption_fails_closed_when_live_head_movedstill asserted the pre-fix(opencode): do not fail draft PRs when the head moves mid-check #1697 behavior (exit 1+"head moved while validating live"). PR fix(opencode): do not fail draft PRs when the head moves mid-check #1697 (fix(opencode): retire stale draft/head dispatches without false failure, merged 2026-09-02) deliberately reordered bothopencode-review.ymlstep bodies to check closed/draft before head-SHA-match, exactly to stop failing draft PRs whose head moved mid-check — a real production incident (contextual-orchestrator PR 🎨 Palette: UX check #1000, stilldraft:true, spurious hard failure: https://github.com/ContextualWisdomLab/contextual-orchestrator/actions/runs/33548447878/job/100066104033).tests/test_opencode_agent_contract.py tests/test_opencode_required_verdict_regression.py tests/test_opencode_security_boundaries.py tests/test_required_workflow_queue_contract.py—test_opencode_live_draft_state_regression.pywas not in that list, so this test kept asserting the retired behavior and has been failing againstorigin/mainever since fix(opencode): do not fail draft PRs when the head moves mid-check #1697 merged. This PR is the fix for that gap, not a revert of fix(opencode): do not fail draft PRs when the head moves mid-check #1697.Changes
tests/test_opencode_live_draft_state_regression.py:test_draft_exemption_fails_closed_when_live_head_moved→test_draft_exemption_survives_when_live_head_moved.returncode == 0and"still a draft on the live exact head"in stdout (mirroring the sibling test in this file and fix(opencode): do not fail draft PRs when the head moves mid-check #1697's own new tests intests/test_opencode_required_verdict_regression.py).PR_ACTION="converted_to_draft"), a distinct angle from fix(opencode): do not fail draft PRs when the head moves mid-check #1697's own new coverage, which doesn't vary event action/draft metadata..github/workflows/opencode-review.ymlis untouched — this is a test-only fix; #1697's real, evidence-backed behavior change stands as merged.Test plan
tests/test_opencode_live_draft_state_regression.py+tests/test_opencode_required_verdict_regression.py— 60 passedtests/test_opencode_agent_contract.py tests/test_opencode_security_boundaries.py tests/test_required_workflow_queue_contract.py(part of fix(opencode): do not fail draft PRs when the head moves mid-check #1697's own verification scope, re-run here to confirm no regression) — 170 passedcoverage run -m pytest tests && coverage report --show-missing(100% gate onscripts/ci) — running; will update this PR once confirmedactionlintwas not required.Note on required checks
The org-wide GitHub Actions capacity incident (being addressed separately this session) may delay this PR's own required checks from completing promptly. If that happens, this PR may need a bypass-merge per this org's standing chicken-and-egg authorization for exactly that situation — but I am not merging this myself; that decision is left to a human/maintainer.
🤖 Generated with Claude Code