From 9ea262109843865a50c9aaf354815c0f64ddb91f Mon Sep 17 00:00:00 2001 From: M Waleed Kadous Date: Sun, 26 Jul 2026 14:08:00 -0700 Subject: [PATCH 1/8] chore(porch): bugfix-1261 init bugfix --- .../status.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 codev/projects/bugfix-1261-tower-serves-api-requests-befo/status.yaml diff --git a/codev/projects/bugfix-1261-tower-serves-api-requests-befo/status.yaml b/codev/projects/bugfix-1261-tower-serves-api-requests-befo/status.yaml new file mode 100644 index 000000000..3ad898938 --- /dev/null +++ b/codev/projects/bugfix-1261-tower-serves-api-requests-befo/status.yaml @@ -0,0 +1,14 @@ +id: bugfix-1261 +title: tower-serves-api-requests-befo +protocol: bugfix +phase: investigate +plan_phases: [] +current_plan_phase: null +gates: + pr: + status: pending +iteration: 1 +build_complete: false +history: [] +started_at: '2026-07-26T21:08:00.200Z' +updated_at: '2026-07-26T21:08:00.200Z' From 4fd9d5a12c3992148ac74518960db1e6b5a82c14 Mon Sep 17 00:00:00 2001 From: M Waleed Kadous Date: Sun, 26 Jul 2026 14:10:59 -0700 Subject: [PATCH 2/8] chore(porch): bugfix-1261 fix phase-transition --- .../bugfix-1261-tower-serves-api-requests-befo/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/bugfix-1261-tower-serves-api-requests-befo/status.yaml b/codev/projects/bugfix-1261-tower-serves-api-requests-befo/status.yaml index 3ad898938..77408510c 100644 --- a/codev/projects/bugfix-1261-tower-serves-api-requests-befo/status.yaml +++ b/codev/projects/bugfix-1261-tower-serves-api-requests-befo/status.yaml @@ -1,7 +1,7 @@ id: bugfix-1261 title: tower-serves-api-requests-befo protocol: bugfix -phase: investigate +phase: fix plan_phases: [] current_plan_phase: null gates: @@ -11,4 +11,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-07-26T21:08:00.200Z' -updated_at: '2026-07-26T21:08:00.200Z' +updated_at: '2026-07-26T21:10:59.180Z' From 5e80345918df908da35488aa74559f31cef15e98 Mon Sep 17 00:00:00 2001 From: M Waleed Kadous Date: Sun, 26 Jul 2026 14:19:28 -0700 Subject: [PATCH 3/8] [Bugfix #1261] Fix: hold Tower API requests until boot wiring completes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tower bound its port and started serving before initInstances() had wired up tower-instances._deps, so DELETE /api/terminals/:id returned 404 for a terminal that plainly existed while GET on the same id returned 200. The window scaled with disk state — the #1227 husk sweep and the #1238 session-log retention scan both ran between the bind and the wiring — so a log-heavy machine failed deterministically while CI stayed green. - Hold each request in http.createServer until bootSequence() opens the readiness gate; a boot that exceeds 20s answers 503 + Retry-After rather than hanging clients. The port still binds first: it is the single-Tower mutex and what every readiness probe connects to. - Move initInstances()/initCron() ahead of the two maintenance sweeps and initTunnel(), which now run after readiness. Gating local readiness on a remote tunnel connect would make an unreachable cloud endpoint look like a broken Tower. - Add instancesReady() so the terminal-delete paths can answer 503 instead of 404 (or a lying 204) if the gate is ever bypassed. Also makes 'afx tower start' honest: it polls /api/status for a 200, which the pre-fix Tower returned during the window because getInstances() returns [] when _deps is null. --- codev/resources/arch.md | 25 ++- .../src/agent-farm/servers/tower-instances.ts | 15 ++ .../src/agent-farm/servers/tower-routes.ts | 33 ++++ .../src/agent-farm/servers/tower-server.ts | 154 +++++++++++++++--- 4 files changed, 197 insertions(+), 30 deletions(-) diff --git a/codev/resources/arch.md b/codev/resources/arch.md index fcd47c618..edcad92c0 100644 --- a/codev/resources/arch.md +++ b/codev/resources/arch.md @@ -240,6 +240,22 @@ Tower binds its port and starts serving immediately, but `reconcileTerminalSessi **Invariant for new Tower-startup work**: any endpoint that reads `workspaceTerminals` to build a response should route through `getRehydratedTerminalsEntry` so it inherits the gate, rather than reading the map directly. +#### Boot Readiness Gate (#1261) + +The #997 barrier gates one dependency (reconcile's output) for the readers that name it. Every *other* boot dependency was still unguarded, and the largest of them was `initInstances()` — which sets `tower-instances.ts`'s `_deps` and was the last step of the boot sequence. Requests landing before it got whatever a half-wired Tower could produce: `DELETE /api/terminals/:id` returned **404 for a terminal that existed**, because `killTerminalWithShellper()` returns a bare `false` when `_deps` is null and the route reads that as "not found". The window scaled with disk state — the #1227 husk sweep and #1238 log sweep ran inside it — so a log-heavy machine failed deterministically while CI stayed green. + +The fix inverts the default from "serve whatever we have" to "serve nothing until wired": + +- The port still binds first. It is the single-Tower mutex (a second `afx tower start` needs `EADDRINUSE`) and what every readiness probe connects to. +- `tower-server.ts` holds each request in `http.createServer` until `bootSequence()` calls `markBootComplete()`; held requests get 503 + `Retry-After` if boot exceeds `BOOT_READY_TIMEOUT_MS` (20s), so a hung boot fails loud instead of hanging clients forever. +- `markBootComplete()` fires as soon as the *dependencies* are wired (through `initCron()`). Maintenance and background services — husk sweep, log-retention sweep, `initTunnel()` — deliberately run **after** it. Gating readiness on `initTunnel()` in particular would make an unreachable cloud endpoint look like a broken local Tower. + +This also makes `afx tower start`'s readiness signal honest: it polls `/api/status` for a 200, which the pre-fix Tower returned during the window (`getInstances()` returns `[]` when `_deps` is null). + +**Invariant for new Tower-startup work**: anything a request handler depends on must be wired *before* `markBootComplete()`; anything else (sweeps, timers, remote connections) goes after it. Do not add work between `server.listen()` and the gate. + +**Second line of defence**: `instancesReady()` lets routes distinguish "not wired yet" from "no such thing" — the `_deps`-dependent terminal-delete paths answer 503 rather than 404 or a lying 204. Unreachable while the gate holds, and deliberately so: the failure mode if it is ever bypassed should be honest. + #### Wire Protocol Binary frame format: `[1-byte type] [4-byte big-endian length] [payload]` @@ -1713,7 +1729,7 @@ The startup ordering is critical — race conditions have caused real bugs when | Step | Operation | Why this order | |------|-----------|----------------| -| 1 | HTTP server binds to `localhost:port` | Must be listening before anything registers routes | +| 1 | HTTP server binds to `localhost:port` | Single-Tower mutex + what readiness probes connect to. **Requests are held, not served, until step 9** (#1261) | | 2 | SessionManager init + stale socket cleanup | Prepares shellper infrastructure | | 3 | `initTerminals()` | Terminal management module ready | | 4 | `startSendBuffer()` | Typing-aware message delivery ready | @@ -1721,12 +1737,15 @@ The startup ordering is critical — race conditions have caused real bugs when | 6 | `killOrphanedShellpers()` | **MUST run after step 5** — avoids killing sessions that were just reconnected | | 7 | `initInstances()` | Enables workspace API handlers — triggers dashboard polling | | 8 | `initCron()` | Scheduler starts after instances ready | -| 9 | `initTunnel()` | Cloud tunnel connects last | -| 10 | WebSocket upgrade handler installed | Terminal connections accepted | +| 9 | **`markBootComplete()`** | **Readiness gate opens** — held requests are released and Tower starts serving (#1261) | +| 10 | Husk sweep (#1227) + session-log sweep (#1238) | Maintenance: scales with disk state, so it must not gate the API | +| 11 | `initTunnel()` | Cloud tunnel connects last — a remote endpoint must never gate local readiness | +| 12 | WebSocket upgrade handler installed | Terminal connections accepted (installed at module load; not gated — see #997 barrier) | **Known ordering bugs**: - **Bugfix #274**: `initInstances()` before `reconcileTerminalSessions()` allowed dashboard polls to race with reconciliation, corrupting shellper sessions - **Bugfix #341**: Killing orphaned shellpers before reconciliation killed sessions that were about to be reconnected +- **Bugfix #1261**: `initInstances()` ran last, *after* the two disk-scaling sweeps, so every `_deps`-dependent route was broken for as long as those scans took — `DELETE /api/terminals/:id` 404'd for a terminal that existed. Fixed by moving the sweeps after readiness and holding requests until step 9 **Defense in depth**: During startup, `getTerminalsForWorkspace()` skips on-the-fly shellper reconnection (via `_reconciling` guard) to prevent races through alternate code paths. diff --git a/packages/codev/src/agent-farm/servers/tower-instances.ts b/packages/codev/src/agent-farm/servers/tower-instances.ts index dd4157d9a..c82738005 100644 --- a/packages/codev/src/agent-farm/servers/tower-instances.ts +++ b/packages/codev/src/agent-farm/servers/tower-instances.ts @@ -168,6 +168,21 @@ export function shutdownInstances(): void { _deps = null; } +/** + * Whether the module has been wired up. + * + * Issue #1261: routes that call into this module need to tell "not wired yet" + * apart from "no such thing", because those deserve different answers — 503 + * "try again" versus 404 "it isn't here". `killTerminalWithShellper()` returns + * a bare boolean and cannot express the difference, so the DELETE route lands + * on 404 for a terminal that exists. Tower's readiness gate now holds requests + * until boot completes, so this should never be false at request time; it is + * kept as a second line of defence. + */ +export function instancesReady(): boolean { + return _deps !== null; +} + // ============================================================================ // Known workspace registration // ============================================================================ diff --git a/packages/codev/src/agent-farm/servers/tower-routes.ts b/packages/codev/src/agent-farm/servers/tower-routes.ts index d597d5166..f102bce7f 100644 --- a/packages/codev/src/agent-farm/servers/tower-routes.ts +++ b/packages/codev/src/agent-farm/servers/tower-routes.ts @@ -58,6 +58,7 @@ import { getDirectorySuggestions, launchInstance, killTerminalWithShellper, + instancesReady, stopInstance, addArchitect, removeArchitect, @@ -195,6 +196,25 @@ const ROUTES: Record = { 'GET /index.html': (_req, res, _url, ctx) => handleDashboard(res, ctx), }; +/** + * Issue #1261: tell "Tower isn't wired up yet" apart from "no such thing". + * A route that needs the instances module must not report a missing terminal + * (404) or a successful kill (204) when the truth is that it could not act at + * all. 503 + Retry-After says so, and matches what `stopInstance()` already + * returns in the same situation. + * + * Tower's readiness gate holds requests until boot completes, so these guards + * should be unreachable in practice — they exist so the failure mode, if the + * gate is ever bypassed, is honest rather than misleading. + */ +function respondStartingUp(res: http.ServerResponse): void { + res.writeHead(503, { 'Content-Type': 'application/json', 'Retry-After': '1' }); + res.end(JSON.stringify({ + error: 'STARTING_UP', + message: 'Tower is still starting up. Try again shortly.', + })); +} + // ============================================================================ // Main request handler // ============================================================================ @@ -840,6 +860,12 @@ async function handleTerminalRoutes( // DELETE /api/terminals/:id - Kill terminal (disable shellper auto-restart if applicable) if (req.method === 'DELETE' && (!subpath || subpath === '')) { + // Issue #1261: without this, a not-yet-wired Tower answers 404 for a + // terminal that exists — the exact symptom the issue reports. + if (!instancesReady()) { + respondStartingUp(res); + return; + } if (!(await killTerminalWithShellper(manager, terminalId))) { res.writeHead(404, { 'Content-Type': 'application/json' }); res.end(JSON.stringify({ error: 'NOT_FOUND', message: `Session ${terminalId} not found` })); @@ -2531,6 +2557,13 @@ async function handleWorkspaceTabDelete( } if (terminalId) { + // Issue #1261: this path discards the kill result and answers 204 either + // way, so a not-yet-wired Tower would report a successful close for a + // terminal it never touched. Refuse instead. + if (!instancesReady()) { + respondStartingUp(res); + return; + } // Disable shellper auto-restart if applicable, then kill the PtySession await killTerminalWithShellper(manager, terminalId); diff --git a/packages/codev/src/agent-farm/servers/tower-server.ts b/packages/codev/src/agent-farm/servers/tower-server.ts index e4c6680c3..c685a85ec 100644 --- a/packages/codev/src/agent-farm/servers/tower-server.ts +++ b/packages/codev/src/agent-farm/servers/tower-server.ts @@ -362,17 +362,76 @@ const routeCtx: RouteContext = { // route handler (/api/worktree-config, /api/activity-hooks) on first request. setCodevConfigNotifier(broadcastNotification); +// ============================================================================ +// Readiness gate (Issue #1261) +// ============================================================================ +// +// The port must bind first — it is the single-Tower mutex (a second +// `afx tower start` detects EADDRINUSE) and the thing every readiness check +// probes. But binding used to mean "serving", and the boot sequence that wires +// up the request handlers' dependencies ran *after* the bind. Requests landing +// in that window were answered with whatever a half-wired Tower could produce: +// `DELETE /api/terminals/:id` returned 404 for a terminal that plainly existed, +// because `killTerminalWithShellper()` bails when `_deps` is null. +// +// So: bind immediately, but hold requests until `bootSequence()` finishes. +// Readiness becomes deterministic instead of a function of how long the boot's +// disk work happens to take on this machine. + +// Boot is bounded by the maintenance work that stays *inside* it; anything +// slower than this is a hang, and a held request should not hang with it. +const BOOT_READY_TIMEOUT_MS = 20_000; + +const bootStartedAtMs = Date.now(); +let bootComplete = false; +const bootWaiters: Array<() => void> = []; + +/** Open the gate: release every held request and admit all future ones. */ +function markBootComplete(): void { + if (bootComplete) return; + bootComplete = true; + log('INFO', `Tower ready — serving API requests (${Date.now() - bootStartedAtMs}ms after process start)`); + for (const release of bootWaiters.splice(0)) release(); +} + +/** Resolves true once boot completes, or false if it takes longer than `timeoutMs`. */ +function whenBootComplete(timeoutMs: number): Promise { + if (bootComplete) return Promise.resolve(true); + return new Promise((resolve) => { + const timer = setTimeout(() => { + const index = bootWaiters.indexOf(release); + if (index !== -1) bootWaiters.splice(index, 1); + resolve(false); + }, timeoutMs); + const release = (): void => { + clearTimeout(timer); + resolve(true); + }; + bootWaiters.push(release); + }); +} + // ============================================================================ // Create server — delegates all HTTP handling to tower-routes.ts // ============================================================================ const server = http.createServer(async (req, res) => { + if (!bootComplete) { + if (!(await whenBootComplete(BOOT_READY_TIMEOUT_MS))) { + log('ERROR', `Boot did not complete within ${BOOT_READY_TIMEOUT_MS}ms; rejecting ${req.method} ${req.url} with 503`); + res.writeHead(503, { 'Content-Type': 'application/json', 'Retry-After': '1' }); + res.end(JSON.stringify({ error: 'STARTING_UP', message: 'Tower is starting up. Try again shortly.' })); + return; + } + // The client may have given up while it was held. + if (res.destroyed || res.writableEnded) return; + } await handleRequest(req, res, routeCtx); }); // SECURITY: Bind to configured host (default 127.0.0.1 for localhost-only). // Bridge mode enables non-localhost binding when BRIDGE_MODE=1 is set. -server.listen(port, bindHost, async () => { +server.listen(port, bindHost, () => { if (bridgeMode) { log('WARN', `Bridge mode is ENABLED — Tower is listening on ${bindHost} network interfaces.`); } @@ -380,6 +439,22 @@ server.listen(port, bindHost, async () => { const displayHost = bindHost === '0.0.0.0' ? 'localhost' : bindHost; log('INFO', `Tower server listening at http://${displayHost}:${port}`); + bootSequence().catch((err) => { + // Preserve the previous fail-fast behaviour: before Issue #1261 this body + // was the listen callback, so a throw surfaced as an unhandled rejection + // and the process-level handler exited. Keep exiting rather than serving + // from a half-wired Tower. + log('ERROR', `Tower boot sequence failed: ${(err as Error).message}\n${(err as Error).stack}`); + process.exit(1); + }); +}); + +/** + * Everything that must be wired up before Tower can answer API requests + * correctly. Runs once, immediately after the port binds; requests are held by + * the readiness gate above until it calls markBootComplete(). + */ +async function bootSequence(): Promise { // Initialize shellper session manager for persistent terminals const socketDir = process.env.SHELLPER_SOCKET_DIR || path.join(homedir(), '.codev', 'run'); const shellperScript = path.join(__dirname, '..', '..', 'terminal', 'shellper-main.js'); @@ -530,9 +605,55 @@ server.listen(port, bindHost, async () => { log('INFO', `Killed ${orphansKilled} orphaned shellper process(es)`); } + // Issue #1261 test hook: widen the pre-wiring window to a known duration so + // the startup race is deterministic instead of a function of this machine's + // process table and session-log volume. Never set outside tests. + const bootDelayMs = parseInt(process.env.AF_TEST_BOOT_DELAY_MS || '0', 10); + if (bootDelayMs > 0) { + log('WARN', `AF_TEST_BOOT_DELAY_MS=${bootDelayMs} — delaying dependency wiring (test hook)`); + await new Promise((r) => setTimeout(r, bootDelayMs)); + } + + // Spec 0105 Phase 3: Initialize instance lifecycle module. + // Placed after reconciliation so getInstances() cannot race with it. + // + // Issue #1261: this used to be the *last* step of the boot sequence, after + // the husk sweep and the session-log retention scan — both of which scale + // with disk state. Every route that touches `_deps` was therefore broken for + // as long as those scans took. It only needs to follow reconciliation, so it + // now runs here and the two maintenance sweeps run after readiness instead. + initInstances({ + log, + workspaceTerminals: getWorkspaceTerminals(), + getTerminalManager, + shellperManager, + getWorkspaceTerminalsEntry, + saveTerminalSession, + deleteTerminalSession, + deleteWorkspaceTerminalSessions, + deleteFileTabsForWorkspace, + getTerminalsForWorkspace, + }); + + // Spec 399: Initialize cron scheduler after instances are ready + initCron({ + log, + getKnownWorkspacePaths, + resolveTarget, + getTerminalManager: () => getTerminalManager(), + }); + + // Issue #1261: dependency wiring is complete — open the gate. Everything + // below is maintenance or background service startup: it must not gate the + // API, and none of it is a prerequisite for a correct response. + markBootComplete(); + // Issue #1227: run the stricter husk sweep once at startup too, same // ordering requirement as killOrphanedShellpers (must run after // reconciliation so a reconnected session's shellper is registered). + // Safe to run post-readiness: the sweep's predicate requires an aged + // shellper (1h grace by default), so a session created by an incoming + // request while the sweep runs can never match it. await runHuskSweep(); // Issue #1238: PTY session log retention. Session logs are per-session files @@ -569,36 +690,15 @@ server.listen(port, bindHost, async () => { sessionLogSweepInterval.unref(); } - // Spec 0105 Phase 3: Initialize instance lifecycle module. - // Placed after reconciliation so getInstances() returns [] during startup - // (since _deps is null), preventing race conditions with reconciliation. - initInstances({ - log, - workspaceTerminals: getWorkspaceTerminals(), - getTerminalManager, - shellperManager, - getWorkspaceTerminalsEntry, - saveTerminalSession, - deleteTerminalSession, - deleteWorkspaceTerminalSessions, - deleteFileTabsForWorkspace, - getTerminalsForWorkspace, - }); - - // Spec 399: Initialize cron scheduler after instances are ready - initCron({ - log, - getKnownWorkspacePaths, - resolveTarget, - getTerminalManager: () => getTerminalManager(), - }); - - // Spec 0097 Phase 4 / Spec 0105 Phase 2: Initialize cloud tunnel + // Spec 0097 Phase 4 / Spec 0105 Phase 2: Initialize cloud tunnel. + // Deliberately after markBootComplete(): connectTunnel() talks to a remote + // service, so gating local API readiness on it would make an unreachable + // cloud endpoint look like a broken Tower. await initTunnel( { port, log, workspaceTerminals: getWorkspaceTerminals(), terminalManager: getTerminalManager() }, { getInstances }, ); -}); +} // Initialize terminal WebSocket server (Phase 2 - Spec 0090) terminalWss = new WebSocketServer({ noServer: true }); From 2510d4757125b44d4e3262d9b6f35ab0eeed11b9 Mon Sep 17 00:00:00 2001 From: M Waleed Kadous Date: Sun, 26 Jul 2026 14:19:35 -0700 Subject: [PATCH 4/8] [Bugfix #1261] Test: regression coverage for the startup readiness window The race was invisible to the suite for two reasons, both addressed here: the e2e helper polls the port every 200ms, which usually lands after boot finishes, and the window's width depends on the machine's process table and session-log volume. - waitForPortImmediate() / startTower({returnAtBind: true}) return at the instant of the bind instead of on the next poll tick. - AF_TEST_BOOT_DELAY_MS widens the pre-wiring window to a known duration so the race is deterministic rather than machine-dependent. - tower-startup-readiness.e2e.test.ts asserts DELETE returns 204 (not a spurious 404) for a request issued at bind time, and that /api/status does not answer 200 until wiring is complete. - A tower-routes unit test covers the instancesReady() 503 guard. Verified both e2e tests fail with the gate disabled. --- codev/state/bugfix-1261_thread.md | 90 ++++++++++++++++++ .../__tests__/helpers/tower-test-utils.ts | 35 ++++++- .../agent-farm/__tests__/tower-routes.test.ts | 21 +++++ .../tower-startup-readiness.e2e.test.ts | 92 +++++++++++++++++++ 4 files changed, 236 insertions(+), 2 deletions(-) create mode 100644 codev/state/bugfix-1261_thread.md create mode 100644 packages/codev/src/agent-farm/__tests__/tower-startup-readiness.e2e.test.ts diff --git a/codev/state/bugfix-1261_thread.md b/codev/state/bugfix-1261_thread.md new file mode 100644 index 000000000..2be41c550 --- /dev/null +++ b/codev/state/bugfix-1261_thread.md @@ -0,0 +1,90 @@ +# bugfix-1261 — Tower serves API before internal wiring completes + +## Investigate + +Issue: `DELETE /api/terminals/:id` 404s for an existing terminal during Tower's +startup window, because the whole boot sequence lives inside the +`server.listen()` callback and `initInstances()` (which sets `_deps`) is the +*last* async step. `killTerminalWithShellper()` bails with `false` when +`_deps` is null; the DELETE route maps that to 404. + +Confirmed by reading: +- `tower-server.ts:375` — `server.listen(...)` callback holds the entire boot. +- `tower-server.ts:575` — `initInstances()` is last, after stale-socket cleanup, + consolidation, `reconcileTerminalSessions()`, `killOrphanedShellpers()`, + the #1227 husk sweep, and the #1238 session-log retention scan. +- `tower-instances.ts:800` — `killTerminalWithShellper` → `if (!_deps) return false`. +- `tower-routes.ts:842` — DELETE translates that `false` into 404 NOT_FOUND. + +Extra finding not in the issue: `afx tower start` treats a 200 from +`/api/status` as readiness (`commands/tower.ts:110`), and `/api/status` +answers 200 during the window (`getInstances()` returns `[]` when `_deps` is +null). So the CLI's "Tower started" is a *false* readiness signal — that is +what makes `afx tower start && ` racy. + +### Reproduced + +Wrote a scratchpad repro that tight-loop-connects to the port and fires +create+DELETE the instant it binds (the e2e helper's 200ms poll adds slack that +usually hides it). On this machine, with an *empty* log dir: + +``` +POST /api/terminals -> 201 (269ms after port open) +DELETE /api/terminals/:id -> 404 {"error":"NOT_FOUND", ...} +GET /api/terminals/:id -> 200 <-- terminal exists; the 404 was spurious +``` + +Deterministic, 100% of runs. So the window is not exotic — it just needs a +client that doesn't sleep before its first request. + +Planned fix (three parts, all small): +1. Readiness gate in `tower-server.ts`: `server.listen()` still binds first + (preserves EADDRINUSE detection + the single-Tower mutex), but + `handleRequest` awaits a boot-complete promise before dispatching, with a + bounded timeout → 503 + `Retry-After`. +2. Reorder boot: move `initInstances()` up to right after the ordering- + constrained steps; push the disk-scaling husk sweep + log retention sweep + after it. Shrinks the window from O(disk) to O(process scan). +3. Defense in depth: DELETE (and any `_deps`-dependent route) returns 503 + "Tower is starting up" instead of 404, generalizing what `stopInstance` + already does. + +## Fix + +All three parts implemented. + +- `tower-server.ts`: readiness gate (`bootComplete` + `whenBootComplete`) in + the `http.createServer` handler; the listen callback now just logs and kicks + off a named `bootSequence()`. Boot-throw still exits the process (was an + unhandled rejection before, same net effect). Held requests get 503 + + `Retry-After` if boot exceeds 20s. +- `tower-server.ts`: `initInstances()` + `initCron()` moved up to right after + `killOrphanedShellpers()`; the #1227 husk sweep, the #1238 log-retention + sweep, and `initTunnel()` now run *after* the gate opens. Tunnel especially: + gating local readiness on a remote connect would make an unreachable cloud + endpoint look like a broken Tower. Measured effect on this machine: boot + reaches ready at ~90ms instead of running the disk scans first. +- `tower-instances.ts`: new `instancesReady()`; `tower-routes.ts`: DELETE + `/api/terminals/:id` and the workspace tab-delete path return 503 instead of + 404 / a lying 204 when the module isn't wired. +- Test hook `AF_TEST_BOOT_DELAY_MS` widens the window deterministically — + without it the race depends on this machine's process table and log volume, + which is exactly why CI never saw it. + +### Verification + +Scratchpad repro, same invocation as before the fix: + +``` +POST /api/terminals -> 201 +DELETE /api/terminals/:id -> 204 (was 404) +GET /api/terminals/:id -> 404 (was 200 — i.e. it really is gone now) +``` + +New `tower-startup-readiness.e2e.test.ts` (2 tests) passes. Confirmed it +*fails* with the gate disabled in dist: DELETE 503 (the second-line guard +firing) and the status-hold assertion 2ms vs the required ≥1200ms. + +Known adjacent case left alone: WebSocket upgrades bypass the gate +(`setupUpgradeHandler` attaches its own listener). Not reachable in practice — +every client does an HTTP call first — so out of BUGFIX scope; noted in the PR. diff --git a/packages/codev/src/agent-farm/__tests__/helpers/tower-test-utils.ts b/packages/codev/src/agent-farm/__tests__/helpers/tower-test-utils.ts index fe549a582..d6b28d85c 100644 --- a/packages/codev/src/agent-farm/__tests__/helpers/tower-test-utils.ts +++ b/packages/codev/src/agent-farm/__tests__/helpers/tower-test-utils.ts @@ -80,6 +80,23 @@ export async function waitForPort(port: number, timeoutMs: number): Promise { + const start = Date.now(); + while (Date.now() - start < timeoutMs) { + if (await isPortListening(port)) return true; + } + return false; +} + /** * Find an available port starting from the given port */ @@ -91,11 +108,23 @@ async function findAvailablePort(startPort: number): Promise { throw new Error(`No available port found starting from ${startPort}`); } +export interface StartTowerOptions { + /** + * Return the moment the port accepts a connection rather than on the next + * 200ms poll tick. Issue #1261 tests need to race the bind itself. + */ + returnAtBind?: boolean; +} + /** * Start the tower server for testing. * Creates an isolated socket directory for shellper sessions (Spec 0116). */ -export async function startTower(port?: number, extraEnv?: Record): Promise { +export async function startTower( + port?: number, + extraEnv?: Record, + opts?: StartTowerOptions, +): Promise { const actualPort = port ?? (await findAvailablePort(14100)); // Spec 0116: Create isolated socket dir so tests don't pollute ~/.codev/run/ @@ -120,7 +149,9 @@ export async function startTower(port?: number, extraEnv?: Record (stderr += d.toString())); // Wait for tower to start - const started = await waitForPort(actualPort, TOWER_START_TIMEOUT); + const started = opts?.returnAtBind + ? await waitForPortImmediate(actualPort, TOWER_START_TIMEOUT) + : await waitForPort(actualPort, TOWER_START_TIMEOUT); if (!started) { proc.kill(); try { rmSync(socketDir, { recursive: true, force: true }); } catch { /* ignore */ } diff --git a/packages/codev/src/agent-farm/__tests__/tower-routes.test.ts b/packages/codev/src/agent-farm/__tests__/tower-routes.test.ts index 35f9fa209..c5324dd18 100644 --- a/packages/codev/src/agent-farm/__tests__/tower-routes.test.ts +++ b/packages/codev/src/agent-farm/__tests__/tower-routes.test.ts @@ -63,6 +63,9 @@ vi.mock('../servers/tower-instances.js', () => ({ getDirectorySuggestions: vi.fn(async () => []), launchInstance: vi.fn(async () => ({ success: true })), killTerminalWithShellper: vi.fn(async () => true), + // Issue #1261: routes that need the instances module ask this first, so a + // wired-up Tower is the default for every route test here. + instancesReady: vi.fn(() => true), stopInstance: vi.fn(async () => ({ ok: true })), addArchitect: vi.fn(async () => ({ success: true, name: 'sibling', terminalId: 'term-arch-sibling' })), removeArchitect: vi.fn(async () => ({ success: true })), @@ -978,6 +981,24 @@ describe('tower-routes', () => { expect(deleteTerminalSession).not.toHaveBeenCalled(); expect(removeTerminalFromRegistry).not.toHaveBeenCalled(); }); + + // Issue #1261: "Tower isn't wired up yet" is not "no such terminal". + // Answering 404 sent callers off hunting for a terminal that was there all + // along; 503 + Retry-After tells them to try again instead. + it('returns 503 rather than 404 when the instances module is not wired yet', async () => { + const { instancesReady, killTerminalWithShellper } = await import('../servers/tower-instances.js'); + (instancesReady as any).mockReturnValueOnce(false); + + const req = makeReq('DELETE', `/api/terminals/${terminalId}`); + const { res, statusCode, body, headers } = makeRes(); + await handleRequest(req, res, makeCtx()); + + expect(statusCode()).toBe(503); + expect(headers()['Retry-After']).toBe('1'); + expect(JSON.parse(body()).error).toBe('STARTING_UP'); + // And it must not have tried to kill anything on the way out. + expect(killTerminalWithShellper).not.toHaveBeenCalled(); + }); }); // ========================================================================= diff --git a/packages/codev/src/agent-farm/__tests__/tower-startup-readiness.e2e.test.ts b/packages/codev/src/agent-farm/__tests__/tower-startup-readiness.e2e.test.ts new file mode 100644 index 000000000..9334537ec --- /dev/null +++ b/packages/codev/src/agent-farm/__tests__/tower-startup-readiness.e2e.test.ts @@ -0,0 +1,92 @@ +/** + * Issue #1261: Tower must not serve API requests before its internal + * dependency wiring is complete. + * + * Tower binds its port first — the port is the single-Tower mutex and the + * thing every readiness probe checks — but binding used to mean "serving". + * Requests landing between the bind and `initInstances()` were answered by a + * half-wired Tower: `DELETE /api/terminals/:id` returned 404 for a terminal + * that existed, because `killTerminalWithShellper()` bails when `_deps` is + * null and the route reads that `false` as "not found". + * + * These tests widen that window to a known duration with + * AF_TEST_BOOT_DELAY_MS, so the race is deterministic rather than a function + * of how long the boot's disk work takes on the machine running the suite. + * That is why the bug hid from CI: fresh runners boot fast enough that the + * window closed before the test's first request arrived. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { + startTower, + cleanupAllTerminals, + cleanupTestDb, + type TowerHandle, +} from './helpers/tower-test-utils.js'; + +const TEST_TOWER_PORT = 14107; + +// Long enough that no plausible scheduling delay lets a request slip in after +// wiring completes; short enough to keep the suite quick. +const BOOT_DELAY_MS = 1500; + +let tower: TowerHandle | null = null; + +afterEach(async () => { + if (tower) { + await cleanupAllTerminals(tower.port); + await tower.stop(); + cleanupTestDb(tower.port); + tower = null; + } +}); + +describe('Tower startup readiness (Issue #1261)', () => { + it('does not answer DELETE /api/terminals/:id with a spurious 404 during startup', async () => { + tower = await startTower( + TEST_TOWER_PORT, + { AF_TEST_BOOT_DELAY_MS: String(BOOT_DELAY_MS) }, + { returnAtBind: true }, + ); + + // Both requests are issued inside the delayed window. Before the fix the + // DELETE returned 404 while the terminal was demonstrably still there. + const createRes = await fetch(`http://localhost:${TEST_TOWER_PORT}/api/terminals`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ label: 'readiness-1261' }), + }); + expect(createRes.status).toBe(201); + const created = await createRes.json(); + + const deleteRes = await fetch( + `http://localhost:${TEST_TOWER_PORT}/api/terminals/${created.id}`, + { method: 'DELETE' }, + ); + expect(deleteRes.status).toBe(204); + + // And the delete really happened — a 204 that killed nothing would be a + // different lie with the same status code. + const getRes = await fetch(`http://localhost:${TEST_TOWER_PORT}/api/terminals/${created.id}`); + expect(getRes.status).toBe(404); + }, 30_000); + + it('holds requests issued at bind time until wiring completes', async () => { + tower = await startTower( + TEST_TOWER_PORT, + { AF_TEST_BOOT_DELAY_MS: String(BOOT_DELAY_MS) }, + { returnAtBind: true }, + ); + + // `afx tower start` polls /api/status and treats 200 as "Tower is up". + // That signal was dishonest: /api/status answered 200 during the window + // because getInstances() returns [] when the module isn't wired. The gate + // makes the first 200 mean what the CLI assumes it means. + const started = Date.now(); + const statusRes = await fetch(`http://localhost:${TEST_TOWER_PORT}/api/status`); + const elapsed = Date.now() - started; + + expect(statusRes.status).toBe(200); + expect(elapsed).toBeGreaterThanOrEqual(BOOT_DELAY_MS * 0.8); + }, 30_000); +}); From 09797a54cde1cf8a517fd1f15a868804d8a81c81 Mon Sep 17 00:00:00 2001 From: M Waleed Kadous Date: Sun, 26 Jul 2026 14:20:09 -0700 Subject: [PATCH 5/8] chore(porch): bugfix-1261 pr phase-transition --- .../bugfix-1261-tower-serves-api-requests-befo/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/bugfix-1261-tower-serves-api-requests-befo/status.yaml b/codev/projects/bugfix-1261-tower-serves-api-requests-befo/status.yaml index 77408510c..06a8f70ce 100644 --- a/codev/projects/bugfix-1261-tower-serves-api-requests-befo/status.yaml +++ b/codev/projects/bugfix-1261-tower-serves-api-requests-befo/status.yaml @@ -1,7 +1,7 @@ id: bugfix-1261 title: tower-serves-api-requests-befo protocol: bugfix -phase: fix +phase: pr plan_phases: [] current_plan_phase: null gates: @@ -11,4 +11,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-07-26T21:08:00.200Z' -updated_at: '2026-07-26T21:10:59.180Z' +updated_at: '2026-07-26T21:20:09.623Z' From 1b77ea38449c0e064589e471ce0af77b46132793 Mon Sep 17 00:00:00 2001 From: M Waleed Kadous Date: Sun, 26 Jul 2026 14:23:22 -0700 Subject: [PATCH 6/8] [Bugfix #1261] Record CMAP verdicts in builder thread --- codev/state/bugfix-1261_thread.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/codev/state/bugfix-1261_thread.md b/codev/state/bugfix-1261_thread.md index 2be41c550..a21c768e7 100644 --- a/codev/state/bugfix-1261_thread.md +++ b/codev/state/bugfix-1261_thread.md @@ -88,3 +88,24 @@ firing) and the status-hold assertion 2ms vs the required ≥1200ms. Known adjacent case left alone: WebSocket upgrades bypass the gate (`setupUpgradeHandler` attaches its own listener). Not reachable in practice — every client does an HTTP call first — so out of BUGFIX scope; noted in the PR. + +## PR + +PR #1263 — "[Bugfix #1261] Hold Tower API requests until boot wiring completes". + +CMAP (3-way, `--issue 1261 --project-id bugfix-1261`; note the bare +`consult --protocol bugfix --type pr` form bails with "Multiple projects +found" in this repo — it needs the issue/project flags): + +| Model | Verdict | Confidence | Key issues | +|--------|---------|------------|------------| +| gemini | APPROVE | HIGH | None | +| codex | APPROVE | HIGH | None | +| claude | APPROVE | HIGH | None | + +No REQUEST_CHANGES, so nothing to address or rebut. Claude raised two +non-blocking observations and dismissed both itself: the `waitForPortImmediate` +name, and whether the `elapsed >= BOOT_DELAY_MS * 0.8` assertion is CI-timing +sensitive (it isn't — a slow machine only makes `>=` more true). + +Awaiting the human `pr` gate. From 2e46c3f0a86f9d2e55660d6fe99e24a6c474112c Mon Sep 17 00:00:00 2001 From: M Waleed Kadous Date: Sun, 26 Jul 2026 14:23:32 -0700 Subject: [PATCH 7/8] chore(porch): bugfix-1261 pr gate-requested --- .../bugfix-1261-tower-serves-api-requests-befo/status.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/codev/projects/bugfix-1261-tower-serves-api-requests-befo/status.yaml b/codev/projects/bugfix-1261-tower-serves-api-requests-befo/status.yaml index 06a8f70ce..f678567b1 100644 --- a/codev/projects/bugfix-1261-tower-serves-api-requests-befo/status.yaml +++ b/codev/projects/bugfix-1261-tower-serves-api-requests-befo/status.yaml @@ -7,8 +7,10 @@ current_plan_phase: null gates: pr: status: pending + requested_at: '2026-07-26T21:23:32.875Z' iteration: 1 build_complete: false history: [] started_at: '2026-07-26T21:08:00.200Z' -updated_at: '2026-07-26T21:20:09.623Z' +updated_at: '2026-07-26T21:23:32.876Z' +pr_ready_for_human: true From b775f02fe3e963e86465f2098ef46dedbcfc9526 Mon Sep 17 00:00:00 2001 From: M Waleed Kadous Date: Sun, 26 Jul 2026 22:25:11 -0700 Subject: [PATCH 8/8] chore(porch): bugfix-1261 pr gate-approved --- .../bugfix-1261-tower-serves-api-requests-befo/status.yaml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/codev/projects/bugfix-1261-tower-serves-api-requests-befo/status.yaml b/codev/projects/bugfix-1261-tower-serves-api-requests-befo/status.yaml index f678567b1..96814b225 100644 --- a/codev/projects/bugfix-1261-tower-serves-api-requests-befo/status.yaml +++ b/codev/projects/bugfix-1261-tower-serves-api-requests-befo/status.yaml @@ -6,11 +6,12 @@ plan_phases: [] current_plan_phase: null gates: pr: - status: pending + status: approved requested_at: '2026-07-26T21:23:32.875Z' + approved_at: '2026-07-27T05:25:11.442Z' iteration: 1 build_complete: false history: [] started_at: '2026-07-26T21:08:00.200Z' -updated_at: '2026-07-26T21:23:32.876Z' -pr_ready_for_human: true +updated_at: '2026-07-27T05:25:11.443Z' +pr_ready_for_human: false