Fix #274: Reorder Tower startup to prevent architect terminal loss#275
Merged
waleedkadous merged 3 commits intomainfrom Feb 15, 2026
Merged
Conversation
The architect terminal was lost on Tower restart due to a race condition in the startup sequence. initInstances() was called before reconcileTerminalSessions(), which allowed dashboard polls arriving during reconciliation to trigger on-the-fly shellper reconnection via getTerminalsForProject(). Both paths competed for the same shellper socket — the shellper's single-connection model replaced the first client, triggering removeDeadSession() which deleted the socket file and corrupted the session. Builder terminals were unaffected because getInstances() skips /.builders/ paths. Fix: Move reconcileTerminalSessions() before initInstances() so getInstances() returns [] during reconciliation (since _deps is null), preventing any on-the-fly reconnection from interfering.
CMAP review (Codex) identified that /project/<path>/api/state calls getTerminalsForProject() directly, bypassing the getInstances() guard. A browser tab left open from before the restart could trigger on-the-fly shellper reconnection during reconciliation via this path. Fix: Add _reconciling flag set during reconcileTerminalSessions() that blocks on-the-fly reconnection in getTerminalsForProject(). This closes both race paths: getInstances (startup ordering) and direct API access (_reconciling guard).
waleedkadous
deleted the
builder/bugfix-274-architect-terminal-should-surv
branch
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.
Summary
Architect terminals were lost on Tower restart due to a startup race condition between terminal reconciliation and API request processing. Reorders startup so reconciliation completes before the instance module enables request handling.
Fixes #274
Root Cause
In Tower's startup sequence,
initInstances()was called beforereconcileTerminalSessions(). This enabled dashboard polls (getInstances()→getTerminalsForProject()) to arrive during reconciliation. BothgetTerminalsForProject()'s on-the-fly reconnection andreconcileTerminalSessions()would attempt to connect to the same shellper socket. The shellper's single-connection model (new connection replaces old) caused the first client to be disconnected, triggeringremoveDeadSession()which deleted the socket file and corrupted the session — permanently losing the architect terminal.Builder terminals were unaffected because
getInstances()explicitly skips/.builders/paths, so theirgetTerminalsForProject()was never called during the race window.Fix
Moved
reconcileTerminalSessions()to run beforeinitInstances()intower-server.ts. During reconciliation,getInstances()returns[](since_depsis null), preventing any on-the-fly reconnection from interfering.Test Plan
bugfix-274-architect-persistence.test.ts)