fix(tmux,team): exit-code marker completion detection; stop re-executing stale team state - #50
Merged
Merged
Conversation
…e death
TmuxRunner's only done signal was pane liveness, which is wrong in both
directions: the CLI is launched via send-keys inside the pane's interactive
shell, so the pane stays alive after the command exits (#{pane_dead} never
fires and the coordinator blocks on proc.Done forever, issue #39); and when
a pane actually dies tmux destroys it, so IsPaneDead errors and a normal
stop was reported as a failure.
The bootstrap now wraps the launched command with
"; echo $? > <workspace>/.contrabass/task-exit-<id>" and redirects the
prompt file into stdin for stdin/file prompt modes. The monitor polls the
marker first: exit code 0 completes the task (and kills the now-idle shell
pane), non-zero fails it, and pane death without a marker — including
IsPaneDead erroring because the pane is gone — is a crash, never success.
Server-mode registry entries are switched to run-to-completion commands
rather than given separate completion semantics: "opencode serve" never
exits and never even received the prompt, so a one-shot "opencode run"
fed via stdin is both the smaller change and the only one that makes the
exit marker meaningful. Same for "codex exec -" (codex is routed to
CodexRunner in production, but the registry entry should not hang if ever
used) and oh-my-opencode's "run" instead of its interactive TUI.
Also in this commit:
- session-level tmux targets use the "=" exact-match prefix so operations
on team "alpha" cannot prefix-match team "alpha-2"'s session
- runner worker IDs are namespaced "tmux-worker-<n>" so they cannot collide
with coordinator "worker-<n>" heartbeats and mask a stale worker
Constraint: completion must come from the command's exit, not the pane's
Constraint: pane shells are POSIX-assumed (existing bootstrap already emits export/cd)
Rejected: remain-on-exit + exec launch | changes global window options and loses the shell for post-mortem
Rejected: ReadyPattern polling + HTTP prompt submission for servers | large protocol surface per agent for no gain over run-to-completion CLIs
Confidence: high
Scope-risk: moderate
Directive: registry entries must run to completion; a server entrypoint here will hang the exec phase
Not-tested: real opencode/omx/omc binaries under tmux (mock CommandRunner only); fish-style shells where $? is not the exit status
…ked deps Four related team state-machine fixes: CreateManifest overwrote an existing team's manifest and reset phase-state.json to plan on every `team run`, while leftover task files from the previous run stayed claimable — retrying a failed board issue (deterministic team name) silently re-executed the old run's tasks, and a concurrent second run corrupted the live coordinator's phase machine. CreateManifest now refuses with ErrTeamExists; Coordinator.Initialize wipes the previous state only when its phase is terminal (or phase state is missing) and refuses a non-terminal team. A pending task depending on a failed task bricked the pipeline: workers drain, PhaseGateRule sees a non-terminal pending task, and the run goes straight to PhaseFailed — the fix loop that would reset the failed dependency was unreachable. Transition governance now treats pending tasks transitively blocked by a failed dependency as settled, so exec/verify hand off to the fix loop and MaxFixLoops bounds the retries as designed. ListTasks swallowed every read error with a bare continue, so an unreadable task file (permissions, transient EMFILE) vanished from the board and AllTasksCompleted could report the pipeline done without executing it. It now skips only ENOENT (delete race) and corrupt JSON (logged, matching recovery.go's convention) and propagates real I/O errors. checkForStaleWorkers flagged idle workers forever: heartbeatLoop stops when a task ends but the worker entry stays in c.workers, so its frozen heartbeat mtime tripped the stale check every 10s. Workers without a current task are now skipped. Constraint: recovery and cleanup paths must keep working from on-disk state alone Rejected: wiping task/worker dirs unconditionally in CreateManifest | destroys a live concurrent run's board Rejected: routing exec directly to PhaseFix on blocked-on-failed | verify owns the failure-routing decision today Confidence: high Scope-risk: moderate Directive: re-running a crashed (non-terminal) team now requires removing its state dir; surface this in UX before automating cleanup Not-tested: EMFILE-style transient read errors in ListTasks (only permission errors are simulated)
RecoverStaleLocks removed files ending in ".tmp", but the Store names its crash-leftover temp files "<path>.tmp.<pid>.<unixnano>" — nothing in the repo ever creates a bare ".tmp" file, so the recovery walk was a permanent no-op and temp litter accumulated. Match the ".tmp." infix instead. The test previously hand-created "*.tmp" files the production writer never produces, masking the mismatch; it now uses the real naming scheme. Confidence: high Scope-risk: narrow Not-tested: aging out orphaned ".lock" files (still unreclaimed, out of scope here)
|
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/2 — closes the root cause of #39.
HIGH — completion was equated with tmux pane death, which never happens: the CLI runs inside an interactive shell (pane survives CLI exit), and server-mode registry entries (
opencode serve) never exit at all. The bootstrap now wraps the CLI so its exit code lands in<workspace>/.contrabass/task-exit-<id>; the monitor polls the marker first (0 = complete, non-zero = failed) and treats pane death without a marker — including IsPaneDead erroring because the pane is gone — as a crash, never success. Server-mode registry entries switched to run-to-completion commands (opencode run,codex exec -,oh-my-opencode run).HIGH — CreateManifest overwrote existing team manifest/phase state and leftover task files were re-executed: Store now refuses with ErrTeamExists; Coordinator.Initialize wipes only terminal previous runs and refuses live teams.
Also: exact-match tmux session targeting (
=name, no more alpha/alpha-2 collisions), namespaced tmux worker heartbeat IDs, blocked-on-failed tasks route to the fix loop instead of hard-failing the pipeline, ListTasks surfaces I/O errors, idle workers no longer flagged stale, stale-lock recovery matches real temp-file names.Adversarial review: approved, 0 blocking.
go test ./internal/team/... ./internal/tmux/... ./internal/agent/...green after rebase.