feat(loop): merge-poll Done-edge fallback + parallel drives (max_concurrent>1) - #6
Merged
Merged
Conversation
…urrent>1)
Two autonomy improvements to the orchestration loop, both off-by-default-safe:
1. Merge-poll fallback for the single Done edge. The merge webhook stays the
primary path, but many deployments (local / NAT / tailnet-only agents) have no
public URL GitHub can post to, so a merged feature would sit in_review forever.
The loop now polls each in_review PR via `gh pr view ... --json state` and runs
the same idempotent record_merge for any that merged. On by default; rate-limited
to merge_poll_interval_s (60s); only probes in_review PRs, so it's cheap.
- worktree.pr_is_merged() — the probe (a gh failure → False, next poll retries).
- worktree.reap_feature_worktree() — shared id→worktree/branch reap; the webhook
reap in api.py now uses it too (DRY).
2. Lift the max_concurrent cap. The loop drove exactly one feature at a time; it now
drives up to max_concurrent concurrently, each in its own worktree (the puller
tops up to the cap each tick, re-checking promptly while busy). Per-feature
in-flight worktrees are tracked (self._inflight) so shutdown cancels every drive
and reaps every orphaned worktree. Default stays 1 (serial) — fully backward
compatible.
Config: max_concurrent (now honoured; floors at 1), merge_poll (default true),
merge_poll_interval_s (default 60). Manifest + README updated; version → 0.3.0.
Tests: +11 (72 total) — pr_is_merged, reap_feature_worktree, _spawn_ready cap +
claim accounting, _poll_merges (merged-only + reap), poll rate-limit + off paths,
and the _active→_inflight tracking.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
👀 Quinn is reviewing — verdict (PASS / WARN / FAIL) + findings to follow. |
There was a problem hiding this comment.
QA Audit — PR #6 | feat(loop): merge-poll Done-edge fallback + parallel drives (max_concurrent>1)
VERDICT: WARN
CI Status
- test: queued ⏳
Diff Review
- Clean DRY refactor:
api.pywebhook reap delegates to sharedworktree.reap_feature_worktree() - Concurrency model (
_active→_drivesset +_inflightdict) is well-structured;add_done_callback(self._drives.discard)is an elegant auto-clean pattern stop()properly cancels all drives, gathers withreturn_exceptions=True, then reaps every inflight worktree — no orphan risk- Config parsing is defensive:
max(1, int(...)),bool(merge_poll), adaptive sleep (idle=interval / busy=3s) _spawn_ready()is sync (correct —claim_next_readyis not async) and tops up to cap cleanly
Observations
- LOW: clawpatch structural review unavailable — repo not in project registry; 547-line diff reviewed via diff_summary only
- LOW: diff truncated at ~200/547 lines —
_maybe_poll_merges()body,worktree.pr_is_merged(),worktree.reap_feature_worktree(), and all 11 new tests are not visible to this review pass - LOW: CI still queued — tests unverified; formal PASS/FAIL deferred to a re-review once CI is terminal
— Quinn, QA Engineer
|
Submitted COMMENT review on |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two autonomy improvements to the orchestration loop — the genuine feature gaps left after the test/cleanup PRs.
1. Merge-poll fallback for the Done edge
The merge webhook (
/webhook/pr) stays the primary path, but many deployments — local agents, NAT'd, tailnet-only — have no public URL GitHub can post to, so a merged feature would sit inin_reviewforever. The loop now polls eachin_reviewPR viagh pr view … --json stateand runs the same idempotentrecord_mergefor any that merged.merge_poll_interval_s(60s); only probesin_reviewPRs → cheap.worktree.pr_is_merged()— the probe (aghfailure returnsFalse, so the next poll just retries; never raises into the loop).worktree.reap_feature_worktree()— a sharedid → worktree/branchreap; the webhook reap inapi.pynow uses it too (DRY).2. Lift the
max_concurrentcapThe loop drove exactly one feature at a time (the config knob existed but was ignored). It now drives up to
max_concurrentfeatures concurrently, each in its own worktree — the puller tops up to the cap each tick and re-checks promptly while busy. Per-feature in-flight worktrees are tracked (self._inflight) so shutdown cancels every drive and reaps every orphaned worktree. Default stays 1 (serial) — fully backward compatible.Config (manifest + README updated)
Version → 0.3.0 (manifest + pyproject, kept in lockstep).
Tests
+11 (72 total), all green:
pr_is_merged(merged/open/closed/gh-fail),reap_feature_worktree,_spawn_readyconcurrency cap + claim accounting,_poll_merges(merged-only + reap), poll rate-limit +merge_poll: false, and the_active → _inflighttracking.ruff check .+ruff format --check .clean.🤖 Generated with Claude Code