fix(cli): graceful SIGINT/SIGTERM in team mode; bound --until-empty redispatch - #52
Merged
Merged
Conversation
A team run that ends in a non-complete phase re-queues its board issue as retry with the claim cleared, and FindDispatchableIssue treats unclaimed retry issues as dispatchable. dispatchBoardIssues therefore re-selected a deterministically failing issue forever with zero backoff, spinning `board dispatch --until-empty` (and the root team-execution loop, which shares this function) indefinitely while burning agent runs. The drain loop now tracks attempts per issue and, once an issue exhausts its cap (default 3, tunable via --max-attempts), parks it by setting a sentinel ClaimedBy plus TrackerMeta status and a board comment, then moves on. Parked issues are hidden from FindDispatchableIssue; `board move <id> retry` clears the claim to re-enable dispatch. The loop also checks ctx between iterations so cancellation from the root execution loop is honored mid-drain. Constraint: LocalBoardState has no terminal failed state, so parking must reuse the ClaimedBy skip in FindDispatchableIssue Rejected: sleep backoff between redispatches | delays but does not bound the loop; the cap alone guarantees termination Rejected: persisting attempt counts in TrackerMeta across drains | per-drain cap already terminates every drain, and parking persists on disk across poll cycles Confidence: high Scope-risk: narrow Directive: parked issues rely on ClaimedBy being non-empty; do not add stale-claim recovery that clears parked:max-dispatch-attempts without revisiting this loop Not-tested: concurrent dispatchers racing on the same board directory
…ts to stdout Root team-execution mode returned from run() before the single-agent signal hook was installed, and runTeamExecutionLoop disables the team-level handler, so `contrabass --no-tui` (or --dry-run) with team.execution_mode=team had zero signal handling: Ctrl+C killed the process via the Go default disposition, skipping runner.Close (orphaning tmux panes still running agents) and boardSyncer.Finalize (leaving the issue ClaimedBy="team:<name>", which FindDispatchableIssue skips forever). run() now wraps the team branch in signal.NotifyContext so interruption cancels ctx and shutdown flows through the normal exit path. Also in this commit: - --no-tui promised "log events to stdout" but runHeadless only wrote to the file logger. Headless mode now streams event lines to stdout in addition to the log file, making the help text and README accurate. Emitting to stdout was chosen over rewording the docs because piping events is the useful CI behavior the flag advertises. - runDryRun checked orch.Run's error for DeadlineExceeded, but Run returns nil when its context is done, so the 60s warning was dead code. The timeout is now detected on dryCtx.Err(). - Single-agent mode printed "Web dashboard available" even when the binary lacks embedded dashboard assets (nil dashboardFS serves no "/" route); it now prints an API-only message pointing at the dashboard_dist build tag, mirroring team mode's diagnostic. - Removed dead workflowTimelineProvider (never constructed; cannot even satisfy web.TimelineProvider) and buildTeamTasksFromBoardIssue (only a test called it; the test now pins buildBoardTeamPlan directly). Constraint: bubbletea only intercepts Ctrl+C in TUI mode, so headless/dry-run team paths need a process-level handler Rejected: dedicated handler inside runTeamExecutionApp | run() owns the root ctx and the single-agent branch already installs its handler at the same level Rejected: correcting help text/README to say events go to the log file | stdout emission is what headless CI consumers need and matches the long-documented behavior Confidence: high Scope-risk: narrow Directive: keep signal registration ahead of the TeamExecutionMode switch; moving it after the switch silently restores the default-kill behavior for team mode Not-tested: SIGTERM delivery (regression test exercises SIGINT; both share one NotifyContext registration)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This branch was successfully deployed
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
1 participant
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.
Wave 2/4.
HIGH — no signal handling in root team-execution mode: Ctrl+C hit Go's default disposition, dying before runner.Close and boardSyncer.Finalize — orphaned tmux sessions, board issues left claimed. Now wrapped in signal.NotifyContext so interruption routes through the normal shutdown path.
HIGH —
--until-emptyredispatched failing retry issues forever with zero backoff: now capped per-issue with backoff; exhausted issues are reported and skipped so the loop terminates.Also: headless (--no-tui) events now reach stdout as promised by the help text; 'Web dashboard available' no longer printed when dashboard assets aren't embedded; dry-run help/behavior reconciled; dead code removed.
Adversarial review: approved, 0 blocking.
go test ./cmd/... ./internal/team/...green after rebase.