fix: verify subagent launches and retry dropped commands (shell-init race)#59
Draft
elucid wants to merge 3 commits into
Draft
fix: verify subagent launches and retry dropped commands (shell-init race)#59elucid wants to merge 3 commits into
elucid wants to merge 3 commits into
Conversation
Launch commands typed into a freshly split pane race against the pane
shell's own startup: direnv/devenv/zsh init can flush the tty input queue
and silently discard the typeahead (repro'd 100% in a devenv repo with the
default 500ms delay). A fixed post-split delay can never be reliable.
Instead, verify and retry:
- sendLongCommand() now writes launch scripts with an idempotency guard as
the first executable lines: the script exits if '<script>.started' exists
and creates the marker before running the command. Resending the
'bash <script>' line is therefore always a safe no-op once started.
- New runLaunchVerifyLoop(): after the initial send, polls for the marker
on a backoff schedule (2s, 4s, 8s, 15s, 30s + 2s grace) and resends the
launch line while the marker is absent. Bounded retries; abortable via
AbortSignal (respects the /reload cleanup pattern); all state is
per-spawn so N concurrent launches stay independent.
- tmux belt-and-suspenders: before resending, check
'#{pane_current_command}' and skip the resend unless the pane is sitting
at an idle shell (prevents typing into the subagent's TUI editor or a
user-started program). Other backends use the marker check only.
- Verification is opt-in via options.verify with an onResult callback
reporting verified / failed (marker-missing | surface-gone) / aborted.
Part of the subagent spawn race fix (TODO-4b7117b7).
A swallowed launch previously registered the subagent as running forever: the child-side activity reporting (HazAT#35) means a never-started child can never report anything, so the parent emitted 'stalled 1m' steer messages indefinitely while an orphan idle shell sat in the pane. Wire the launch verify loop (previous commit) into all three launch sites (pi path, claude path, subagent_resume): - Each sendLongCommand() now runs the verify/retry loop, abortable via the module-level /reload abort signal, with a per-spawn onResult callback (no shared state across concurrent spawns). - On retry exhaustion, handleLaunchVerifyResult() records launchFailure on the running entry and aborts its watcher. watchSubagent() converts that into a launch-failure result delivered through the existing subagent result steer path: exit code 1, error 'launch-failed', and a summary stating the shell never executed the launch command (N resends over Ss) with remediation hints (surface id + backend, launch script path, manual 'bash <script>' retry line). - The entry is removed from runningSubagents, so the widget entry and stall notifications stop; the pane is intentionally NOT closed so the orphan shell can be inspected post-mortem. - subagent_resume result handling no longer overrides a launch-failure summary with 'exited without new output' from the untouched session. - Launch failure of claude-backed subagents omits the never-created pi session file from the result to avoid a misleading resume hint. Closes TODO-02b64f40 (subagent spawn race fix).
…ce tests
- subagent and subagent_resume immediate tool results now include the mux
surface/pane id in details (launchScriptFile was already there), so
orchestrators can inspect or take over the pane deliberately instead of
guessing which pane belongs to which spawn.
- subagent_result steer details additionally carry error/surface/
launchScriptFile when the result is a launch failure.
- Unit tests (17 new, npm test green at 131):
- buildLaunchScript: guard check + marker creation ordered before the
command, shell-escaped marker paths, preamble preserved
- guarded script executed 3x via bash in a temp dir runs the command
exactly once and leaves the .started marker
- runLaunchVerifyLoop: no resend when marker present; resend-then-verify;
bounded retries with marker-missing failure; shouldResend=false skips
typing into non-idle panes; surface-gone on resend error; AbortSignal
aborts cleanly; default schedule sanity
- isIdleShellCommand accepts shells (zsh/-zsh/bash/fish path) and rejects
node/pi/vim/claude
- handleLaunchVerifyResult: records failure + aborts watcher, marks
verified launches, ignores stale results for untracked entries
- formatLaunchFailureSummary: truthful message with surface id, script
path, manual 'bash <script>' retry hint; surface-gone variant
Also live-smoke-tested against a dedicated tmux session (race-debug-worker,
removed afterwards): an input-swallowing pane recovered via 2 resends
(verified @14s, command ran once); a pane stuck in 'cat' produced 0 resends
and a marker-missing failure (no typing into non-shell panes).
Closes TODO-5f79696d (subagent spawn race fix).
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.
TL;DR
Launch keystrokes can be swallowed by shell init, leaving a "running" subagent that never started. Launches are now verified against a marker file and retried safely until they provably ran — or fail loudly instead of stalling forever.
Problem
Subagent launches type the launch command into a freshly created pane after a fixed delay (default 500 ms,
PI_SUBAGENT_SHELL_READY_DELAY_MS). If the shell is still initializing when the keystrokes arrive — common with direnv/devenv/nix hooks — some init steps flush the tty input queue: the command is echoed to the scrollback but never executed.Result: the pane sits at an idle prompt, the tool has already reported the subagent as running, and the orchestrator receives "stalled" notifications forever. No session is created and no work happens.
Fixed delays can't reliably fix this because shell init time is unbounded (cold caches, disk pressure). In an environment with a heavy direnv hook, a warm-cache init reliably swallowed commands sent up to ~500 ms after the split, and a cold init swallowed them at any plausible delay.
Fix: make the launch closed-loop
1. Idempotency-guarded launch scripts (
df5fa1e) — every generated launch script now begins with:The
.startedmarker is both proof that the launch ran and a guard that makes re-running the script a no-op — which is what makes retrying safe.2. Verify/retry loop — after the initial send, an async loop polls for the marker (2 s, 4 s, 8 s, 15 s, 30 s backoff). If it's missing, the launch line is resent. On tmux, a resend only happens while the pane's current command is still an idle shell, so a retry can never type into the child's TUI. Loops are per-launch (unique script + marker per attempt, including resumes), abortable on
/reload, and safe under concurrent spawns.3. Truthful failure state (
3c84bca) — if the marker never appears after all retries, the subagent is removed from the running set (stall notifications stop) and the orchestrator gets an explicit failure message including the pane/surface id and the launch script path, with the pane kept open for post-mortem. Launch failures surface through the normal result path instead of an eternal "stalled".4. Surface id in tool results (
b1d718b) —subagentandsubagent_resumeresults now include the pane/surface id so orchestrators can inspect or take over a pane deliberately when something goes wrong.Behavior notes
Testing
npm test): guard ordering/escaping, exactly-once execution under repeated runs, resend/backoff/abort logic, idle-shell gating, failure handling.cat) produced the failure message with zero unsafe resends and no lingering stall notifications.Known limitation
Resuming a session that is already running is not detected.