Skip to content

Block stale and unstable merge states#24

Merged
mberman84 merged 1 commit into
Forward-Future:mainfrom
coygeek:droid/issue-18-merge-state-readiness
Jun 22, 2026
Merged

Block stale and unstable merge states#24
mberman84 merged 1 commit into
Forward-Future:mainfrom
coygeek:droid/issue-18-merge-state-readiness

Conversation

@coygeek

@coygeek coygeek commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • classify GitHub BEHIND merge states as blocked with an out-of-date head ref reason
  • classify GitHub UNSTABLE merge states as blocked with a non-passing commit status reason
  • keep CLEAN and HAS_HOOKS ready-state coverage in separate regression tests

Closes #18

Validation

  • PYTHONPATH=src python3 -m pytest tests/test_cli.py -q
  • PYTHONPATH=src python3 -m pytest -q
  • ruff check src tests
  • autoreview --mode local: clean, no accepted/actionable findings

@mberman84 mberman84 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes because the blanket UNSTABLE classification regresses the exact-main integration path. Merging this head onto current main fails test_integration_snapshot_uses_exact_commit_check_fallback (blocked instead of ready). Preserve the useful BEHIND handling, but derive failed vs waiting from configured required-check states and do not override successful exact-commit evidence. GitHub CI is still action_required; re-request review after updating the head.

Comment thread src/agent_merge_queue/cli.py Outdated
blocked.append(
f"GitHub reports the pull request merge state as {self.merge_state}"
)
elif self.merge_state == "UNSTABLE":

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UNSTABLE means a non-passing commit status, which can be pending or an optional failure. Treating it unconditionally as blocked turns ordinary CI into a repair escalation and also blocks current main's integration fallback after exact required CI has passed. Use the reduced required-check states to distinguish failed from waiting, and allow exact successful evidence to remain ready.

@coygeek
coygeek force-pushed the droid/issue-18-merge-state-readiness branch from 767ead0 to 375a9a2 Compare June 21, 2026 16:28
@coygeek

coygeek commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the review feedback in the updated head. BEHIND remains blocked as an out-of-date head ref, while UNSTABLE no longer blocks unconditionally: it waits only when DeployBot cannot prove the configured required checks have passed, preserving the exact-check integration fallback.

Verification: PYTHONPATH=src uv run --with pytest --with ruff python -m pytest tests/test_cli.py -q and uv run --with ruff ruff check src tests both pass.

Treat GitHub BEHIND and UNSTABLE merge states as non-ready so the queue reports the real blocker instead of ready-to-merge status.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
@mberman84
mberman84 force-pushed the droid/issue-18-merge-state-readiness branch from 375a9a2 to bb50ac3 Compare June 22, 2026 14:25

@mberman84 mberman84 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed on the latest main. BEHIND fails closed; UNSTABLE waits only without exact required-check proof and preserves exact-success integration behavior. Full suite: 227 passed, 3 skipped, 202 subtests; Ruff and structured autoreview clean.

@mberman84
mberman84 merged commit 57ae51f into Forward-Future:main Jun 22, 2026
3 checks passed
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.

[bug] BEHIND and UNSTABLE merge states are treated as ready

2 participants