Daemon concurrency: non-blocking mailboxes, sharded per task - #33
Merged
Conversation
The daemon runs one actor with one mailbox, and every command is awaited inline. That was right at the original scale and is now the user-visible bottleneck: starting a task stalls the conversation, and approving a tool call queues behind unrelated work in another task. Record what the code actually does today (blocking SQLite on every streamed chunk, a second queue in the WebSocket read loop, inline git and filesystem work, a latent self-send deadlock), the target — non-blocking handlers, write-behind persistence, reads off the mailbox, state sharded per task — and the alternatives rejected along the way. The invariants section is the load-bearing part: each entry is a way this regresses quietly, including the ones that look local and harmless in review. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDLFbFWS5sR5zxixK9uVRk
Every state change wrote to SQLite from inside the actor loop, including one INSERT per streamed chunk of agent output. Those are blocking disk writes on a tokio worker, so while an agent streamed, the mailbox could not accept anything queued behind it — a tool approval in an unrelated task waited for the agent to stop typing. Writes now go to a dedicated persistence thread that drains its queue into one transaction. Batching only, never coalescing: the resume replay guard compares persisted history against the agent's replay chunk for chunk, so merging two AgentText rows would double a turn's output on resume. Write-behind makes reading the transcript back off disk wrong as well as slow — the rows a caller needs are usually still queued — so the actor keeps the transcript in memory and answers from there. That covers the replay guard, the duplicate-suppression check, and the workflow engine's stage-output parsing, which would otherwise read its own truncated text and mis-parse a verdict. Snapshot history folds from the same in-memory copy; the fold moved into a shared helper so the store's existing tests still cover it. Account edits and task deletion stay awaited: they report failure to the user, and dropping the error would leave a silently unsaved account or a task that reappears on the next start. Shutdown flushes the queue. Without that, exiting drops the tail of every transcript written since the last batch. Known trade-off: the in-memory transcript is not bounded, so a very long session holds more than the previous trimmed snapshot did. Bounding it belongs with the read-path work, where the projection gets designed as one thing. Refs docs/adr/0002-daemon-concurrency.md Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDLFbFWS5sR5zxixK9uVRk
diff::search_files is a synchronous walk that reads every file in the project, and it ran inline in the command handler. On a large repo that froze the daemon for the length of the search — including the mention picker's own follow-up requests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDLFbFWS5sR5zxixK9uVRk
GetDiff, GetFileContents, ListFiles, GitBranches and GitPushInfo each awaited git subprocesses inline in the command handler, so every one of them sat between the mailbox and whatever came next. GetDiff is the expensive case: the changes panel polls it, so an open task meant a steady stream of git pairs blocking approvals and agent updates. Each resolves its repo path from actor state — the only part that needs the actor — then hands the git work to a task that answers the caller directly. None of them mutate state on completion, so there is nothing to route back and no stale-result window. Git operations that write (commit, push, merge, rebase, branch switch) still run inline: they update task state when they finish, which needs the epoch handling ADR 0002 describes. They are also user-initiated rather than polled, so they cost a click, not a drip. Refs docs/adr/0002-daemon-concurrency.md Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDLFbFWS5sR5zxixK9uVRk
The WebSocket read loop awaited dispatch inline, so one connection served one request at a time: a tool approval was not even read off the socket until the file search ahead of it returned. Reads now answer off the read loop through a shared writer, bounded by a per-connection semaphore so a client cannot fan out unbounded git and filesystem work. The eligible set is its own list rather than the negation of method_is_mutation. That one exists to decide what the update gate holds back and counts the LSP methods as non-mutating, but LSP is an ordered protocol — dispatching LspSend concurrently would reorder a language server's inbox. Mutations stay serialized, so their ordering is unchanged. Two fixes fell out of testing this: diff::list_files stats every file in the project synchronously inside an async fn, holding a runtime worker; it now uses the blocking pool like search does. Accepted connections get TCP_NODELAY. Replies are small frames, and without it each one waits on an ACK for the previous — paired with the peer's delayed ACK that is tens of milliseconds added to an answer the daemon already had ready. It was also what made the new test look like a failure at first: the second request was sitting in the client's send buffer, not queued in the daemon. The test asserts the cheap request sent second is answered first, and was checked to fail with the concurrency removed. Refs docs/adr/0002-daemon-concurrency.md Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDLFbFWS5sR5zxixK9uVRk
Creating a task with a worktree ran `git worktree add` inline in the command handler, so every other task's messages and approvals waited on the new task's checkout. This is the symptom that started the whole thread: starting a task stalled the conversation. The task is now created and emitted immediately, the checkout runs off the loop, and Command::WorktreeReady attaches it and starts the session. WorktreeManager gained detached constructors so the git work no longer needs a borrow of the actor; the manager records the result afterwards. This is the first spawned work that mutates state on completion, so it needs the stale-result guard ADR 0002 describes. The pending session start is that token: cancel and delete remove it, and a checkout landing afterwards finds nothing to start. The worktree itself is recorded either way — it exists on disk regardless, and one the manager does not know about is one nothing can clean up. Both tests were checked against the mutation they describe: the first fails if the checkout blocks creation again, the second fails if cancel stops clearing the token. Refs docs/adr/0002-daemon-concurrency.md Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDLFbFWS5sR5zxixK9uVRk
text.generate spawns an agent process with a two-minute ceiling, and it was dispatched on the connection's read loop — so while a task's title was being written the daemon read nothing else from that client. Starting a task therefore stalled the conversation it was starting, whether or not a worktree was involved. agents.install and languageServers.install had the same shape and shell out to a package manager, so they could hold the connection far longer. The previous split asked whether a request writes, which is the wrong question. What matters is whether it must be ordered against the others on the connection: reads are independent, and so is a one-shot job whose result depends on nothing else in flight. Ordered work — LSP's streaming protocol, git writes that only mean what they mean in sequence — stays serial. Concurrent mutations keep the update gate, applied inside the spawned task, so moving a method between the lists cannot quietly let it run during a daemon handover. Refs docs/adr/0002-daemon-concurrency.md Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDLFbFWS5sR5zxixK9uVRk
Branching a conversation looked up the source task's *worktree* to find what to inherit. A source task running in the project checkout has none, so the lookup came back empty and the branch was created on a clean HEAD — the uncommitted change it was supposed to continue from was silently absent. Reported from a real branch whose source had a modified file. The source is now wherever the task actually works: its worktree when it has one, the project checkout otherwise. The base branch still comes from a source worktree when there is one, and falls back to HEAD. That exposed a second bug, latent until the project checkout could be a copy source. Worktrees live at `.worktrees/<task>` inside the project, and `git ls-files --others` reports a nested checkout as a single directory entry — so copying the working state from the project checkout tried to copy a worktree directory as a file and failed the whole branch. Only plain files are copied now. Worktree failures also log their context chain. The top-level message alone said "failed to copy working state" and named no git step, which is what made this take a run to find. Both halves were checked against the test: it fails with either the source lookup or the file check reverted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDLFbFWS5sR5zxixK9uVRk
Replace the actor's full in-memory copy of every task's session history (HashMap<String, Vec<SessionUpdate>>, loaded entirely at startup, never released) with bounded projections: - last_session_update (O(1) per task) for duplicate detection - a current-turn buffer, cleared on each user message, for stage text - resume replay guards, snapshots, and finished-turn output now read from the store off the actor loop, after a persist flush (ADR 0002): the resume guard lands as ResumeReplayReady before the session starts, and turn output comes back as TaskOutputReady. - startup loads only tool-call start timestamps via a targeted query. No in-memory structure grows with session length; handlers still never await store I/O.
The transcript projections landed with the finished-turn output assembled unconditionally: every turn end flushed the write queue and read the task's whole history back out of SQLite. Both consumers are no-ops for an ordinary task — the orchestrator hook needs the tag, the inbox delivery needs a parent — so for most tasks that work was discarded, and it grew with the conversation. That trades the memory the projections save for disk they never needed to touch. Ask for the output only when a consumer exists. load_tool_call_starts kept the last timestamp per tool call where the folding it replaced kept the first. Frames of one call normally repeat the same value, but a daemon restart mid-call can assign a new one, and this map exists so a call's start time does not move under the user. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDLFbFWS5sR5zxixK9uVRk
A stage whose child session ended called workflow_finalize, and Failed is terminal — is_active() is false and resume answers "the pipeline is not paused". Reported from a real run: an agent was killed mid-stage by something unrelated to the work, the user re-prompted the task, the session reconnected and finished the implementation. The work was done and reviewable; the pipeline was dead and the only way on was a new task. Losing the agent process is an infrastructure failure. The stage never got to say whether the work was good, so there is no verdict — but no reason to conclude the run failed either. It now parks at the existing pause barrier, so resume re-runs the stage. Same for a review round where every reviewer's agent died: an absent verdict, not a rejection. The re-run gets the partial-work warning restore_workflow_runs already gives a stage interrupted by a daemon restart, since the working copy may hold the dead attempt's edits. Reusing the pause barrier keeps this to the daemon: no protocol addition, no desktop work. ADR 0003 records what that trades away — the case where the work was already finished still re-runs the stage. Checked against the mutation: the test fails with the finalize restored. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDLFbFWS5sR5zxixK9uVRk
Daemon teardown swept the project's whole port range with lsof and killed every listener in it — TERM, then KILL. The range holds whatever happens to be there, so this killed processes warpforge never started: a developer's own server on 4001, and, because every test that shuts a daemon down runs the same teardown, `cargo test` in this repo killed the agents of the warpforge running the tests. That is what has been breaking the OpenCode connection on every test run, and what killed the workflow stage in the reported pipeline failure. At teardown the daemon knows exactly which ports it handed out — they are in the allocation map — so it sweeps those. Orphans of its own services are still caught; strangers are not touched. The startup sweep in serve() keeps the range scan: a fresh process has an empty allocation map, and cleaning up after a previous daemon's crash is the whole point of it. It does not run in tests. The user-initiated sweeps (stop project, stop runtime, authorized project removal) are left alone — one of them already carries a comment about reaching untracked listeners, so narrowing them is a separate decision. Checked against the mutation: the test fails with the range sweep restored. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDLFbFWS5sR5zxixK9uVRk
Commit, push, merge, rebase, branch create/rename/delete/switch, update, create-PR, file save and hunk reject all awaited git or the filesystem inline in their command handlers, so each one held the mailbox for the length of a subprocess. Reads moved off the loop earlier; these are the writes, and they were left because they mutate task state when they finish and so needed somewhere to report back to. Command::GitOpFinished is that: the operation runs on its own task and sends back what it changed — HEAD moved, a commit landed, a hunk went away. No token is needed here, unlike the worktree checkout: an effect describes something that already happened on disk, so applying it late is still correct as long as the task exists, which the handler checks. Store reads that had been left running synchronously inside spawned tasks now go through runtime::store_read on the blocking pool. SQLite blocks, and doing it on a runtime worker stalls that worker outright whenever the persistence thread holds the lock for a batch commit. The one remaining direct lock is startup-only and says so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDLFbFWS5sR5zxixK9uVRk
Merging ran two git commands and a checkout removal inline in the handler, so a merge held the mailbox for all of it. It is split the same way the checkout was: detached merge and remove functions that need no manager borrow, with WorktreeManager::forget for the bookkeeping, and a Command::WorktreeMerged carrying the outcome back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDLFbFWS5sR5zxixK9uVRk
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
2 participants
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.
Summary
Refactors the daemon's single-threaded actor model to eliminate head-of-line blocking. Introduces a write-behind persistence layer, concurrent request handling per connection, and offloads blocking I/O (git, filesystem, subprocess) to dedicated threads. Fixes workflow agent loss handling and improves responsiveness across the board.
Key Changes
Concurrency Architecture (ADR 0002)
daemon/runtime/persist.rs: Write-behind persistence thread that batches SQLite writes into single transactions. Prevents blocking the actor on every streamed chunk from agents.daemon/server.rs:daemon/actor.rs: Delegates blocking I/O to runtime workers instead of awaiting inlineBlocking I/O Offloading
daemon/diff.rs: File listing and diff operations moved to blocking pool (both stat every file synchronously)daemon/worktree.rs: Extractedcreate_detached()andremove_detached()functions for off-actor git operations; manager now tracks worktrees created externallyWorkflow Agent Loss Handling (ADR 0003)
RunState::FailedtoRunState::Pausedwhen a stage's agent process diesworkflow_resumeto reconnect the agent and continue the pipeline instead of treating it as terminal failureStore & Persistence
daemon/store.rs: Addedwrite_batch()for transactional batching of multiple writesPort Management
service.rs: Newkill_listeners_on_ports()to kill only specific ports instead of sweeping entire rangesports.rs: Newallocated_in_ranges()to identify which ports warpforge actually allocatedTesting & Fixtures
shutdown_does_not_kill_listeners_it_did_not_start()test to prevent regressionmock-acp-workflow.mjs: Added "die" command to simulate agent process loss mid-turnImplementation Details
Writeenum variants don't report back to actor; only user-initiated operations (Askenum) await confirmationcreate_detached()runs off-actor, thenadopt()registers it with the managerChangesets
Multiple user-facing improvements documented in
.changeset/: