Skip to content

fix: deliver first input to a reconnected task via the ready-detection path#78

Merged
kovtcharov merged 6 commits into
mainfrom
fix/reconnect-input-delivery
Jul 2, 2026
Merged

fix: deliver first input to a reconnected task via the ready-detection path#78
kovtcharov merged 6 commits into
mainfrom
fix/reconnect-input-delivery

Conversation

@kovtcharov

Copy link
Copy Markdown
Contributor

Problem

A task that was disconnected (e.g. after a server restart) and then sent a message couldn't receive input — the message went nowhere, or only landed ~15s later. Hit idle worktree tasks especially.

Root cause

writeToTask reconnected the task, then attached a listener waiting for a one-shot idle state transition to deliver the buffered input. But reconnectTask creates the task at state:'idle' and emits taskStateChanged(idle) synchronously — before the listener can attach, so the only idle emit is missed. Delivery then falls to a 15s timeout fallback; and for a quietly-resuming session that never re-emits idle, the input is effectively lost.

This was latent until #69 broadened auto-reconnect eligibility from shouldContinue to wasInterrupted (every task is stamped wasInterrupted=true at shutdown), so idle tasks now traverse this reconnect path for the first time — which is why these tasks "worked before" and broke after a restart.

Fix

Deliver a reconnected task's first message through the same TUI-ready detection + Enter-retry machinery that delivers a new task's initial prompt (already proven by the shouldContinue continuation path):

  • reconnectTask(taskId, pendingInput?) now takes the message, queues it as pendingPrompt, and lets the ready-detection in setupProcessHandlers (+ the existing fallback timer) send it via sendPromptWithRetry once the resumed Claude TUI actually signals readiness.
  • Removes the racy taskStateChanged listener and the separate 15s writeToTask re-entry.
  • Plain background reconnects (auto-reconnect, sleep/wake) with no pending input are unchanged (no second arg → identical behavior).

Also: guard reconnectTask against a missing workspace directory — worktree dirs can be removed after a PR merges, and spawning a PTY into a non-existent cwd threw ENOENT that was swallowed (silently dropping input). Now it fails the reconnect explicitly with a logged error.

Validation

  • npm run build -w backend
  • Full suite + coverage gate: 550 tests ✓
  • Runtime: rebuild backend + restart, then send a message to a disconnected/idle task — it should be typed into the resumed session and submitted as soon as the TUI is ready (watch for Claude ready, sending prompt instead of Fallback: delivering pending write after 15s).

Follow-ups (identified, not in this PR)

  • Frontend TerminalView.tsx restoreInProgress buffer has no timeout fallback — can freeze the terminal if the xterm write callback never fires.
  • worktree:remove/prune active-task guard excludes idle, so removing a merged worktree can orphan an idle task.

…n path

A disconnected task (e.g. after a server restart) that is then sent a message
had its input delivered by listening for a one-shot 'idle' state transition
after reconnect. But reconnectTask emits that idle state synchronously, before
writeToTask can attach the listener, so the transition is missed and the
message stalls behind a 15s fallback — or, for a quietly resuming session that
never re-emits idle, is effectively lost. This hit idle worktree tasks hard
after #69 broadened auto-reconnect eligibility from shouldContinue to
wasInterrupted, routing idle tasks through this path for the first time.

Route the first post-reconnect message through the same TUI-ready detection +
Enter-retry machinery that delivers a new task's initial prompt: reconnectTask
now takes the message as a pendingPrompt and sends it (via sendPromptWithRetry)
only once the resumed Claude TUI signals readiness. Deterministic, and reuses
the proven new-task/continuation delivery path. Plain background reconnects
(auto-reconnect, sleep/wake) with no pending input are unchanged.

Also guard reconnectTask against a missing workspace directory: worktree dirs
can be removed after a PR merges, and spawning a PTY into a non-existent cwd
threw ENOENT that was swallowed, silently dropping the input. Fail the
reconnect explicitly with a logged error instead.
…ir source

A client looping 'task:input: /clear' wiped a task's running session (the
content was only saved because the session pointer happened to survive). Two
hardening changes to writeToTask:

- Rate-limit context-destroying slash commands (/clear, /compact, /reset):
  allow up to 2 to a given task within 10s, then drop further ones and log a
  warning. A runaway client can no longer clear a session in a loop.
- Attribute every write to a source. task:input now passes a per-connection
  client id (web/mobile + remote address + seq); internal callers default to
  'internal'. The 'Writing to PTY' and block logs include source= so the next
  occurrence names exactly which client is responsible.
Current Claude Code rejects '*' as an allow rule ('Wildcard tool name "*"
is not supported in allow rules'), printing a warning at startup and — worse —
leaving the normal permission flow active, so skip-permissions tasks still
prompt for command approval and the TUI churns through permission dialogs.

--dangerously-skip-permissions (already passed when skip-permissions is on)
bypasses all permission checks by itself, so the allowlist is unnecessary in
that mode. Only set the narrow '--allowedTools mcp__*' in the non-skip case.
Fixes both the spurious permission prompts and the associated console thrash on
skip-permissions tasks. Applies to both the initial spawn and reconnect paths.
…repo

When a task running in a worktree spawns a new worktree (e.g. via the MCP
create-task tool), addWorktreeWorkspace was setting worktreeParentId to the
spawning task's workspace — which is itself a worktree. The sidebar only groups
worktrees one level under a top-level workspace, so a worktree nested under
another worktree renders nowhere and the task is invisible in the toolbar
(though it runs fine).

addWorktreeWorkspace now walks the given parent up to the root repo workspace,
so worktrees always nest under a top-level workspace. Adds a regression test.
A wedged TUI floods cursor-position queries (\x1b[6n); xterm answers each with an
8-char cursor-position report (\x1b[r;cR), and every one was logged as 'Writing to
PTY ... (8 chars)', spamming the console. Skip logging bare terminal-response
escape sequences (cursor reports, device attributes, etc.) — they're still written
normally, just not logged.
The /clear-injection source (logged as source=web:<client>) can persist; allowing
2 per 10s still let it wipe sessions. Cap to a single /clear|/compact|/reset per
task per 30s, dropping the rest and logging the source. Legitimate manual use is
rarely repeated within seconds; this limits an injected/looping source to one wipe
per window while the offending client is identified and closed.
@kovtcharov kovtcharov enabled auto-merge (squash) July 1, 2026 22:38
@kovtcharov kovtcharov requested a review from thisislance98 July 1, 2026 22:38
@kovtcharov kovtcharov merged commit 464146f into main Jul 2, 2026
3 checks passed
@kovtcharov kovtcharov deleted the fix/reconnect-input-delivery branch July 2, 2026 00:15
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.

2 participants