From ee87eaec1261d0e67c66f769b98463539969dbe3 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Thu, 17 Sep 2026 09:38:54 +1000 Subject: [PATCH 1/9] chore(porch): bugfix-1691 init bugfix --- .../status.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml diff --git a/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml b/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml new file mode 100644 index 000000000..2fa45520e --- /dev/null +++ b/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml @@ -0,0 +1,14 @@ +id: bugfix-1691 +title: afx-tower-start-surface-the-ow +protocol: bugfix +phase: investigate +plan_phases: [] +current_plan_phase: null +gates: + pr: + status: pending +iteration: 1 +build_complete: false +history: [] +started_at: '2026-09-16T23:38:54.218Z' +updated_at: '2026-09-16T23:38:54.219Z' From e45c29f611ba90c0486f18cdb8f430344652cf3b Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Thu, 17 Sep 2026 09:41:57 +1000 Subject: [PATCH 2/9] chore(porch): bugfix-1691 fix phase-transition --- .../bugfix-1691-afx-tower-start-surface-the-ow/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml b/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml index 2fa45520e..4a9c4d76f 100644 --- a/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml +++ b/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml @@ -1,7 +1,7 @@ id: bugfix-1691 title: afx-tower-start-surface-the-ow 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-09-16T23:38:54.218Z' -updated_at: '2026-09-16T23:38:54.219Z' +updated_at: '2026-09-16T23:41:57.243Z' From 2555dd1c7796c3710010363954e29492c538e9fe Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Thu, 17 Sep 2026 09:48:55 +1000 Subject: [PATCH 3/9] Fix #1691: surface tower owner-guard refusal in the CLI, not a 30s timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `afx tower start` spawns the tower-server daemon detached and polled only its port for readiness. When the #1629 owner-lock guard refuses, the daemon logs its teaching error and exits within ~1s, but the port-only wait burned the full 30s budget and then printed a generic "failed to respond within 30000ms" — the whole value of the guard's teaching error was lost to tower.log, and the outcome was indistinguishable from a genuine hang. The readiness wait now also watches the spawned daemon's liveness and returns one of three outcomes: started / exited / timeout. On a fast-exit it reads back what the daemon appended to tower.log for this run and prints it verbatim on stderr (the owner-guard's refusal, or any early-boot failure), then exits non-zero — the instant the exit is observed, not after 30s. Refs #1690 (item 1 of the owner-lock robustness follow-up; items 2+3 remain there). --- codev/state/bugfix-1691_thread.md | 61 ++++++ ...x-1691-tower-start-surface-refusal.test.ts | 202 ++++++++++++++++++ .../codev/src/agent-farm/commands/tower.ts | 100 ++++++++- 3 files changed, 352 insertions(+), 11 deletions(-) create mode 100644 codev/state/bugfix-1691_thread.md create mode 100644 packages/codev/src/agent-farm/__tests__/bugfix-1691-tower-start-surface-refusal.test.ts diff --git a/codev/state/bugfix-1691_thread.md b/codev/state/bugfix-1691_thread.md new file mode 100644 index 000000000..9ff1d800a --- /dev/null +++ b/codev/state/bugfix-1691_thread.md @@ -0,0 +1,61 @@ +# bugfix-1691 thread + +Issue #1691: `afx tower start` — surface the owner-guard refusal in the CLI instead of the +generic 30s timeout. Split from #1690 item 1; companion to #1689's owner guard (PR #1689, +`tower-owner.ts`). Scope is item 1 ONLY (per main's lane context). Items 2+3 (shutdown overlap, +clear-owner hatch, flock eval) stay in #1690 and are NOT mine. + +Commit discipline (main): no closing keywords for #1690 — use `Refs #1690` if referencing; +`Fix #1691:` is fine. A `Fix #N` commit SUBJECT auto-closes on merge even with a clean PR body +(#1677 lesson). + +## Investigate (done) + +Root cause — `packages/codev/src/agent-farm/commands/tower.ts`: +- `towerStart()` spawns the tower-server daemon detached (`stdio: 'ignore'`, `unref()`), then + `waitForServer(port)` (lines 149-160) polls ONLY `/api/status` for up to `STARTUP_TIMEOUT_MS` + (30s). It has no awareness of the spawned pid's liveness. +- When the #1689 owner guard refuses, `bootSequence()` in `tower-server.ts` (lines 526-536) calls + `log('ERROR', ownershipConflictMessage(...))` then `process.exit(1)`. The daemon dies within + ~1s; the port never comes up. +- So `waitForServer` loops the full 30s, then prints the generic + "Tower server failed to respond within 30000ms" (lines 266-271) — indistinguishable from the + #1685 hang class. The teaching error is only in `tower.log`. +- Daemon log format (`tower-server.ts` `log()`, lines 142-161): `[iso] [ERROR] \n`, + first line prefixed, continuation lines (the multi-line `ownershipConflictMessage`) unprefixed. + +Fix shape (implement phase): +1. Detect fast-exit: track the spawned child's `exit` event; the readiness wait stops immediately + when the daemon dies before the port responds (no more burning 30s). +2. Surface verbatim: capture `tower.log` byte offset right before the daemon can write, then on + fast-exit read everything appended since and print it (the guard's teaching error, or any + other early-boot failure). +3. Three distinguishable outcomes: `started` / `exited` (refused-with-reason) / + `timeout` (timed-out-still-unknown). + +Testable seam: extract an injectable `waitForServerOutcome(probe, daemonAlive, opts)` returning +the discriminated outcome, plus a `readLogSince(offset)` helper. Regression test drives the seam +with fakes (daemon dies while probe stays false → `exited` well before timeout) and asserts the +launcher surfaces the teaching error + exits non-zero on fast-exit. + +## Fix (done) + +Changed one product file: `packages/codev/src/agent-farm/commands/tower.ts` (+89/-11). No +skeleton twin (the Tower launcher is product code, not a shipped template — confirmed no +`tower.ts` under `codev-skeleton/`). + +- New exported `TowerStartupOutcome = 'started' | 'exited' | 'timeout'` and + `waitForServerOutcome(isReady, isDaemonAlive, opts)` replacing the old boolean `waitForServer`. + It short-circuits to `exited` the instant the daemon is seen dead (with a final readiness + re-probe for the benign same-tick race), so a refusal no longer burns the 30s budget. +- `towerStart` registers `serverProcess.on('exit')` → `daemonExited`, and captures the tower.log + byte offset right after the launcher's pre-spawn writes. On `exited` it reads everything the + daemon appended since (`readLogSince`) and prints it verbatim on stderr (the guard's teaching + error), then `exit(1)`. Three distinguishable outcomes: started / exited (refused-with-reason) + / timeout (still-running, status unknown). + +Regression test: `packages/codev/src/agent-farm/__tests__/bugfix-1691-tower-start-surface-refusal.test.ts` +(6 tests). Unit tests pin the outcome logic incl. the "no 30s burn" timing; two towerStart tests +(mocked spawn/http/shell) prove the teaching error is surfaced verbatim + exit(1) within seconds, +and the empty-log fallback. Build clean, `tsc --noEmit` clean, tower-command + 1629 + 1691 suites +green (46 tests). diff --git a/packages/codev/src/agent-farm/__tests__/bugfix-1691-tower-start-surface-refusal.test.ts b/packages/codev/src/agent-farm/__tests__/bugfix-1691-tower-start-surface-refusal.test.ts new file mode 100644 index 000000000..72ebc48c2 --- /dev/null +++ b/packages/codev/src/agent-farm/__tests__/bugfix-1691-tower-start-surface-refusal.test.ts @@ -0,0 +1,202 @@ +/** + * Issue #1691 — `afx tower start` must surface the owner-guard refusal in the CLI, + * not the generic 30s timeout. + * + * `towerStart` spawns the tower-server daemon detached and then polls its port for + * readiness. When the #1629 owner-lock guard refuses, the daemon logs its teaching + * error and `process.exit(1)`s within ~1s — but the old readiness wait only watched + * the port, so it burned the full 30s budget and then printed a generic "failed to + * respond within 30000ms", indistinguishable from a real hang (#1685 class). The + * daemon's refusal reached only tower.log. + * + * The fix teaches the wait to distinguish three outcomes — started / exited / + * timeout — by also watching the spawned daemon's liveness, and on a fast-exit it + * reads back what the daemon appended to tower.log for THIS run and prints it + * verbatim. These tests pin both halves: the pure outcome logic (fast-exit is + * detected immediately, not after the 30s budget) and the launcher wiring (a + * fast-exiting daemon's teaching error is surfaced on stderr and the CLI exits + * non-zero within seconds). + */ + +import { describe, it, expect, vi, beforeEach, afterEach, afterAll } from 'vitest'; +import { EventEmitter } from 'node:events'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +// A throwaway AGENT_FARM_DIR so tower.log (resolved from AGENT_FARM_DIR at module +// load) lives under our control, and a mutable holder for the spawn stub's child — +// both created in a hoisted block so the vi.mock factories below can reference them. +const h = vi.hoisted(() => { + const nodeOs = require('node:os') as typeof import('node:os'); + const nodePath = require('node:path') as typeof import('node:path'); + const nodeFs = require('node:fs') as typeof import('node:fs'); + const dir = nodeFs.mkdtempSync(nodePath.join(nodeOs.tmpdir(), 'af-1691-')); + return { agentFarmDir: dir, spawn: { child: null as EventEmitter & { pid?: number; unref?: () => void } | null } }; +}); + +vi.mock('../lib/tower-client.js', () => ({ + DEFAULT_TOWER_PORT: 4100, + AGENT_FARM_DIR: h.agentFarmDir, +})); + +vi.mock('../utils/config.js', () => ({ + getConfig: () => ({ serversDir: h.agentFarmDir }), +})); + +// isPortInUse() = !isPortAvailable(); "available" keeps towerStart off the +// zombie-cleanup branch and straight onto the spawn path. +vi.mock('../utils/shell.js', () => ({ + isPortAvailable: vi.fn(async () => true), +})); + +// The readiness probe uses http.request; a request that immediately errors makes +// isServerResponding() resolve false (the port never comes up on a refusal). +vi.mock('node:http', () => { + const request = vi.fn(() => { + const req = new EventEmitter() as EventEmitter & { end: () => void; destroy: () => void }; + req.destroy = () => {}; + req.end = () => { + setImmediate(() => req.emit('error', new Error('ECONNREFUSED'))); + }; + return req; + }); + return { default: { request }, request }; +}); + +vi.mock('node:child_process', async () => { + const actual = await vi.importActual('node:child_process'); + return { ...actual, spawn: vi.fn(() => h.spawn.child) }; +}); + +const LOG_FILE = path.join(h.agentFarmDir, 'tower.log'); + +const TEACHING_ERROR = [ + 'Refusing to start: this global.db is already owned by a live Tower.', + ' owner pid 12345 on port 4100 (host testhost)', + ' shared database dir: /Users/test/.agent-farm', + 'A second Tower opening a live global.db hijacks and deletes its shellper sessions (Issue #1629).', + 'If you meant to run an isolated test Tower, set CODEV_AGENT_FARM_DIR (NOT AGENT_FARM_DIR) to a throwaway directory (#1515).', + "Otherwise stop the existing Tower first ('afx tower stop'), or investigate pid 12345 if you believe it is stale.", +].join('\n'); + +let consoleErrorSpy: ReturnType; +let consoleLogSpy: ReturnType; +let exitSpy: ReturnType; + +beforeEach(() => { + // A dummy compiled server so towerStart takes the `node ` branch; spawn + // is stubbed, so the file is never executed. + fs.writeFileSync(path.join(h.agentFarmDir, 'tower-server.js'), '// stub'); + fs.rmSync(LOG_FILE, { force: true }); + h.spawn.child = null; + consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + exitSpy = vi.spyOn(process, 'exit').mockImplementation(((code?: number) => { + throw new Error(`process.exit:${code ?? 0}`); + }) as never); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +afterAll(() => { + fs.rmSync(h.agentFarmDir, { recursive: true, force: true }); +}); + +describe('waitForServerOutcome (#1691)', () => { + it('returns "started" when the port answers readiness', async () => { + const { waitForServerOutcome } = await import('../commands/tower.js'); + const outcome = await waitForServerOutcome(async () => true, () => true, { + timeoutMs: 1000, + intervalMs: 5, + }); + expect(outcome).toBe('started'); + }); + + it('returns "exited" the instant the daemon dies — it does NOT burn the 30s budget', async () => { + const { waitForServerOutcome } = await import('../commands/tower.js'); + let alive = true; + setTimeout(() => { + alive = false; + }, 10); + + const start = Date.now(); + // A 30s budget as in production; the fix must return far sooner than that. + const outcome = await waitForServerOutcome(async () => false, () => alive, { + timeoutMs: 30000, + intervalMs: 10, + }); + const elapsed = Date.now() - start; + + expect(outcome).toBe('exited'); + expect(elapsed).toBeLessThan(1000); + }); + + it('returns "timeout" when the daemon stays alive but never answers', async () => { + const { waitForServerOutcome } = await import('../commands/tower.js'); + const outcome = await waitForServerOutcome(async () => false, () => true, { + timeoutMs: 60, + intervalMs: 10, + }); + expect(outcome).toBe('timeout'); + }); + + it('prefers "started" when the port answers in the same tick the daemon is seen to exit', async () => { + const { waitForServerOutcome } = await import('../commands/tower.js'); + let readyCalls = 0; + // First probe false (loop enters), daemon reported dead, re-probe true → started. + const isReady = async () => ++readyCalls >= 2; + const outcome = await waitForServerOutcome(isReady, () => false, { + timeoutMs: 1000, + intervalMs: 5, + }); + expect(outcome).toBe('started'); + }); +}); + +describe('towerStart fast-exit surfacing (#1691)', () => { + it('surfaces the daemon refusal verbatim and exits non-zero within seconds', async () => { + const { towerStart } = await import('../commands/tower.js'); + + const child = new EventEmitter() as EventEmitter & { pid: number; unref: () => void }; + child.pid = 999999; + child.unref = () => {}; + h.spawn.child = child; + + // Mimic the daemon: append its refusal to tower.log (the guard's format is + // `[iso] [ERROR] `), then exit before the port ever answers. + setTimeout(() => { + fs.appendFileSync(LOG_FILE, `[2026-09-17T00:00:00.000Z] [ERROR] ${TEACHING_ERROR}\n`); + child.emit('exit', 1, null); + }, 30); + + await expect(towerStart({ wait: true })).rejects.toThrow('process.exit:1'); + + const stderr = consoleErrorSpy.mock.calls.map((c) => c.join(' ')).join('\n'); + // The teaching error — not a generic timeout — is what the user sees. + expect(stderr).toContain('Refusing to start: this global.db is already owned by a live Tower'); + expect(stderr).toContain('CODEV_AGENT_FARM_DIR'); + expect(stderr).toContain('exited during startup'); + expect(stderr).not.toContain('failed to respond within'); + }); + + it('reports the fast-exit even when the daemon logged nothing', async () => { + const { towerStart } = await import('../commands/tower.js'); + + const child = new EventEmitter() as EventEmitter & { pid: number; unref: () => void }; + child.pid = 999998; + child.unref = () => {}; + h.spawn.child = child; + + // Exit with no fresh log output. + setTimeout(() => child.emit('exit', 1, null), 30); + + await expect(towerStart({ wait: true })).rejects.toThrow('process.exit:1'); + + const stderr = consoleErrorSpy.mock.calls.map((c) => c.join(' ')).join('\n'); + expect(stderr).toContain('exited during startup'); + expect(stderr).toContain('No output was captured'); + }); +}); diff --git a/packages/codev/src/agent-farm/commands/tower.ts b/packages/codev/src/agent-farm/commands/tower.ts index 0509df402..47c8780ef 100644 --- a/packages/codev/src/agent-farm/commands/tower.ts +++ b/packages/codev/src/agent-farm/commands/tower.ts @@ -3,7 +3,7 @@ */ import { resolve } from 'node:path'; -import { existsSync, mkdirSync, appendFileSync } from 'node:fs'; +import { existsSync, mkdirSync, appendFileSync, statSync, readFileSync } from 'node:fs'; import http from 'node:http'; import { logger, fatal } from '../utils/logger.js'; import { spawn } from 'node:child_process'; @@ -144,19 +144,68 @@ async function isServerResponding(port: number): Promise { } /** - * Wait for the server to start responding + * The three distinguishable outcomes of waiting for a freshly spawned Tower (Issue #1691). + * - `started`: the port answered readiness within the budget. + * - `exited`: the spawned daemon died before the port answered — a fast-exit refusal (the + * #1629 owner-lock guard) or any early boot failure. The launcher stops waiting the instant + * it observes the exit instead of burning the full timeout on a process already gone. + * - `timeout`: the budget elapsed with the daemon still alive but not answering (a real hang). */ -async function waitForServer(port: number): Promise { +export type TowerStartupOutcome = 'started' | 'exited' | 'timeout'; + +/** + * Wait for a freshly spawned Tower to become ready, returning which of the three outcomes + * occurred. `isReady` is the readiness probe (port answering); `isDaemonAlive` reports whether + * the spawned daemon process is still running. Both are injectable so the outcome logic can be + * exercised deterministically without spawning a real server. + * + * The tower-server IS the daemon, so once it exits the port can never come up — hence a dead + * daemon short-circuits to `exited`. A final readiness re-probe closes the benign race where the + * daemon answered readiness in the same tick it was observed to exit. + */ +export async function waitForServerOutcome( + isReady: () => Promise, + isDaemonAlive: () => boolean, + opts: { timeoutMs?: number; intervalMs?: number } = {} +): Promise { + const timeoutMs = opts.timeoutMs ?? STARTUP_TIMEOUT_MS; + const intervalMs = opts.intervalMs ?? STARTUP_CHECK_INTERVAL_MS; const startTime = Date.now(); - while (Date.now() - startTime < STARTUP_TIMEOUT_MS) { - if (await isServerResponding(port)) { - return true; + while (Date.now() - startTime < timeoutMs) { + if (await isReady()) { + return 'started'; + } + if (!isDaemonAlive()) { + return (await isReady()) ? 'started' : 'exited'; } - await new Promise((r) => setTimeout(r, STARTUP_CHECK_INTERVAL_MS)); + await new Promise((r) => setTimeout(r, intervalMs)); + } + + return 'timeout'; +} + +/** Current byte length of the tower log (0 if absent) — the boundary for {@link readLogSince}. */ +function currentLogOffset(): number { + try { + return statSync(LOG_FILE).size; + } catch { + return 0; } +} - return false; +/** + * Read everything appended to the tower log since `offset` bytes. Surfaces a fast-exiting + * daemon's own output (e.g. the owner-lock guard's teaching error) verbatim, without dragging in + * stale lines from previous runs. Returns '' if the file is gone or nothing new was written. + */ +function readLogSince(offset: number): string { + try { + const buf = readFileSync(LOG_FILE); + return buf.subarray(Math.min(offset, buf.length)).toString('utf8').trim(); + } catch { + return ''; + } } export { getProcessesOnPort } from '../utils/port.js'; @@ -256,16 +305,45 @@ export async function towerStart(options: TowerStartOptions = {}): Promise logToFile(`Spawned tower server with PID ${serverProcess.pid}`); + // Issue #1691: track the daemon's liveness and mark where its own log output begins, so the + // readiness wait can distinguish a fast-exit (owner-lock refusal, early boot failure) from a + // genuine hang — and, on a fast-exit, surface the daemon's teaching error verbatim instead of + // a generic timeout. The offset is captured here (after the launcher's own pre-spawn writes) + // so readLogSince() returns only what THIS daemon run appended. + let daemonExited = false; + serverProcess.on('exit', () => { + daemonExited = true; + }); + const logOffsetAtSpawn = currentLogOffset(); + const dashboardUrl = `http://localhost:${port}`; if (wait) { // Wait for server to actually start responding logger.info('Waiting for server to start...'); - const started = await waitForServer(port); + const outcome = await waitForServerOutcome( + () => isServerResponding(port), + () => !daemonExited + ); + + if (outcome === 'exited') { + // The daemon died before the port came up: a refusal (the #1629 owner-lock guard) or an + // early boot failure. Surface its own log output verbatim so the user sees the teaching + // error, not a generic timeout indistinguishable from a real hang (Issue #1691). + const reason = readLogSince(logOffsetAtSpawn); + logToFile('Tower server exited during startup before responding'); + logger.error('Tower server exited during startup before it became ready:'); + if (reason) { + console.error(`\n${reason}\n`); + } else { + logger.error(`No output was captured. Check logs at: ${LOG_FILE}`); + } + process.exit(1); + } - if (!started) { + if (outcome === 'timeout') { logToFile(`Tower server failed to respond within ${STARTUP_TIMEOUT_MS}ms`); - logger.error(`Tower server failed to start within ${STARTUP_TIMEOUT_MS / 1000}s`); + logger.error(`Tower server did not respond within ${STARTUP_TIMEOUT_MS / 1000}s and is still running (status unknown).`); logger.error(`Check logs at: ${LOG_FILE}`); process.exit(1); } From a2635159513cde222bbf281b4d0ef7e91d59dd4c Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Thu, 17 Sep 2026 09:49:36 +1000 Subject: [PATCH 4/9] chore(porch): bugfix-1691 pr phase-transition --- .../bugfix-1691-afx-tower-start-surface-the-ow/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml b/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml index 4a9c4d76f..0f393d552 100644 --- a/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml +++ b/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml @@ -1,7 +1,7 @@ id: bugfix-1691 title: afx-tower-start-surface-the-ow 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-09-16T23:38:54.218Z' -updated_at: '2026-09-16T23:41:57.243Z' +updated_at: '2026-09-16T23:49:36.211Z' From 3d185a1234a5554dad98f34211501cd8ef5df9b9 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Thu, 17 Sep 2026 09:57:27 +1000 Subject: [PATCH 5/9] Fix #1691: harden startup-outcome wait per CMAP feedback - Add a post-loop liveness check in waitForServerOutcome so a daemon that exits as the readiness budget expires is reported as `exited`, not a misleading `timeout` (codex). - Convert the started/exited ternary to if/else per the project's if/else preference (claude). - Assert the fast-exit reaction time explicitly (< 5s) in the towerStart test, and add a unit test pinning the post-loop liveness check. Refs #1690. --- codev/state/bugfix-1691_thread.md | 29 +++++++++++++++++-- ...x-1691-tower-start-surface-refusal.test.ts | 19 ++++++++++++ .../codev/src/agent-farm/commands/tower.ts | 15 +++++++++- 3 files changed, 59 insertions(+), 4 deletions(-) diff --git a/codev/state/bugfix-1691_thread.md b/codev/state/bugfix-1691_thread.md index 9ff1d800a..9fe41051d 100644 --- a/codev/state/bugfix-1691_thread.md +++ b/codev/state/bugfix-1691_thread.md @@ -54,8 +54,31 @@ skeleton twin (the Tower launcher is product code, not a shipped template — co error), then `exit(1)`. Three distinguishable outcomes: started / exited (refused-with-reason) / timeout (still-running, status unknown). -Regression test: `packages/codev/src/agent-farm/__tests__/bugfix-1691-tower-start-surface-refusal.test.ts` -(6 tests). Unit tests pin the outcome logic incl. the "no 30s burn" timing; two towerStart tests +Regression test: `packages/codev/src/agent-farm/__tests__/bugfix-1691-tower-start-surface-refusal.test.ts`. +Unit tests pin the outcome logic incl. the "no 30s burn" timing; two towerStart tests (mocked spawn/http/shell) prove the teaching error is surfaced verbatim + exit(1) within seconds, and the empty-log fallback. Build clean, `tsc --noEmit` clean, tower-command + 1629 + 1691 suites -green (46 tests). +green. + +## PR + CMAP (done) + +PR #1692 (`Fixes #1691`, `Refs #1690` — non-closing form for #1690 per commit discipline). + +CMAP: **claude=APPROVE** (HIGH, verified end-to-end), **codex=COMMENT** (HIGH), **gemini=skipped** +(agy unauthenticated, non-blocking). Addressed feedback in a second commit: +- codex #2 (no final liveness check after the wait loop → exit on the deadline misreports as + `timeout`): added a post-loop `isDaemonAlive` check. New unit test pins it (asserts `exited` + where the old code returned `timeout`). +- claude #2 (ternary vs project if/else preference): converted the `started/exited` ternary to + if/else. +- claude #1 (implicit timing guarantee): added an explicit `< 5000ms` elapsed assertion to the + towerStart fast-exit test so the "no 30s burn" bound is visible, not reliant on vitest's 5s + default. +- codex #1 (capture log offset before spawn): NOT applied. Claude verified the daemon's + boot-to-first-log (~100ms+: module load + bootSequence) far exceeds the sub-ms parent window + between spawn() and offset capture, so the race is not practically reachable, and it degrades + gracefully (the "No output captured" fallback points to the log) if it ever did. Claude + explicitly praised the current placement for keeping the surfaced tail daemon-only (no launcher + lines echoed at the user). Kept the clean placement over a theoretical hardening. + +7 regression tests green; `tsc --noEmit` clean. diff --git a/packages/codev/src/agent-farm/__tests__/bugfix-1691-tower-start-surface-refusal.test.ts b/packages/codev/src/agent-farm/__tests__/bugfix-1691-tower-start-surface-refusal.test.ts index 72ebc48c2..a262a9079 100644 --- a/packages/codev/src/agent-farm/__tests__/bugfix-1691-tower-start-surface-refusal.test.ts +++ b/packages/codev/src/agent-farm/__tests__/bugfix-1691-tower-start-surface-refusal.test.ts @@ -143,6 +143,21 @@ describe('waitForServerOutcome (#1691)', () => { expect(outcome).toBe('timeout'); }); + it('returns "exited" (not "timeout") when the daemon dies as the budget expires', async () => { + const { waitForServerOutcome } = await import('../commands/tower.js'); + let alive = true; + // Dies mid-sleep, after the last in-loop check but before the loop condition re-evaluates — + // caught only by the post-loop liveness check. + setTimeout(() => { + alive = false; + }, 20); + const outcome = await waitForServerOutcome(async () => false, () => alive, { + timeoutMs: 15, + intervalMs: 40, + }); + expect(outcome).toBe('exited'); + }); + it('prefers "started" when the port answers in the same tick the daemon is seen to exit', async () => { const { waitForServerOutcome } = await import('../commands/tower.js'); let readyCalls = 0; @@ -172,7 +187,11 @@ describe('towerStart fast-exit surfacing (#1691)', () => { child.emit('exit', 1, null); }, 30); + const start = Date.now(); await expect(towerStart({ wait: true })).rejects.toThrow('process.exit:1'); + // The whole point of #1691: the CLI reacts to the fast-exit in seconds, not after the 30s + // readiness budget. Asserted explicitly rather than left to vitest's default test timeout. + expect(Date.now() - start).toBeLessThan(5000); const stderr = consoleErrorSpy.mock.calls.map((c) => c.join(' ')).join('\n'); // The teaching error — not a generic timeout — is what the user sees. diff --git a/packages/codev/src/agent-farm/commands/tower.ts b/packages/codev/src/agent-farm/commands/tower.ts index 47c8780ef..ae0d2e1e3 100644 --- a/packages/codev/src/agent-farm/commands/tower.ts +++ b/packages/codev/src/agent-farm/commands/tower.ts @@ -177,11 +177,24 @@ export async function waitForServerOutcome( return 'started'; } if (!isDaemonAlive()) { - return (await isReady()) ? 'started' : 'exited'; + // The daemon is gone before the port answered. Re-probe once to close the benign race + // where it answered readiness in the same tick it was observed to exit. + if (await isReady()) { + return 'started'; + } + return 'exited'; } await new Promise((r) => setTimeout(r, intervalMs)); } + // Budget exhausted. The daemon may have exited during the final sleep — check once more so a + // fast-exit landing on the deadline is reported as `exited`, not a misleading `timeout`. + if (!isDaemonAlive()) { + if (await isReady()) { + return 'started'; + } + return 'exited'; + } return 'timeout'; } From cf041ba4f02d276a7b9bf0b5506691e27d32a6de Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Thu, 17 Sep 2026 09:58:57 +1000 Subject: [PATCH 6/9] chore(porch): bugfix-1691 pr gate-requested --- .../bugfix-1691-afx-tower-start-surface-the-ow/status.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml b/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml index 0f393d552..971837b9c 100644 --- a/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml +++ b/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml @@ -7,8 +7,10 @@ current_plan_phase: null gates: pr: status: pending + requested_at: '2026-09-16T23:58:57.118Z' iteration: 1 build_complete: false history: [] started_at: '2026-09-16T23:38:54.218Z' -updated_at: '2026-09-16T23:49:36.211Z' +updated_at: '2026-09-16T23:58:57.119Z' +pr_ready_for_human: true From d6ffc65d3db115e5ece3dba22fe0974407aa3c80 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Thu, 17 Sep 2026 11:18:53 +1000 Subject: [PATCH 7/9] Fix #1691: drop internal issue numbers from the user-visible refusal message The owner-lock refusal (ownershipConflictMessage) was only a tower.log line until this lane surfaced it at the CLI, so its `(Issue #1629)` / `(#1515)` references now show to end users. Remove them from the message text, keeping the actionable `CODEV_AGENT_FARM_DIR (NOT AGENT_FARM_DIR)` guidance; the incident context moves into the function's JSDoc. Comments carry issue numbers, user-visible messages do not. --- codev/state/bugfix-1691_thread.md | 10 ++++++++++ .../__tests__/bugfix-1629-tower-owner.test.ts | 4 +++- .../bugfix-1691-tower-start-surface-refusal.test.ts | 6 ++++-- packages/codev/src/agent-farm/db/tower-owner.ts | 9 +++++++-- 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/codev/state/bugfix-1691_thread.md b/codev/state/bugfix-1691_thread.md index 9fe41051d..6b56a00e9 100644 --- a/codev/state/bugfix-1691_thread.md +++ b/codev/state/bugfix-1691_thread.md @@ -82,3 +82,13 @@ CMAP: **claude=APPROVE** (HIGH, verified end-to-end), **codex=COMMENT** (HIGH), lines echoed at the user). Kept the clean placement over a theoretical hardening. 7 regression tests green; `tsc --noEmit` clean. + +## Post-gate follow-up (owner-directed) + +Owner reviewed the real end-to-end output and flagged that the refusal message references +internal issue numbers (`#1629`, `#1515`) — fine in comments, not in a user-visible string. This +is coupled to #1691: my fix is what promotes that message from a `tower.log` line to a CLI +message the user reads. Stripped `(Issue #1629)` / `(#1515)` from `ownershipConflictMessage` +(`db/tower-owner.ts`), keeping the actionable `CODEV_AGENT_FARM_DIR (NOT AGENT_FARM_DIR)` guidance; +moved the incident context into the function's JSDoc. Updated the #1629 test (asserts the message +now matches no `#\d+`) and my fixture. 46 tests green, tsc clean. diff --git a/packages/codev/src/agent-farm/__tests__/bugfix-1629-tower-owner.test.ts b/packages/codev/src/agent-farm/__tests__/bugfix-1629-tower-owner.test.ts index 1a049aa43..91b6cc917 100644 --- a/packages/codev/src/agent-farm/__tests__/bugfix-1629-tower-owner.test.ts +++ b/packages/codev/src/agent-farm/__tests__/bugfix-1629-tower-owner.test.ts @@ -269,7 +269,9 @@ describe('ownershipConflictMessage — loud and teaching', () => { expect(msg).toContain('4100'); expect(msg).toContain('/home/u/.agent-farm'); expect(msg).toContain('CODEV_AGENT_FARM_DIR'); - expect(msg).toContain('Issue #1629'); + // User-visible message: carries the actionable env-var fix, but no internal + // issue numbers (they live in the code comment) — Issue #1691. + expect(msg).not.toMatch(/#\d+/); }); }); diff --git a/packages/codev/src/agent-farm/__tests__/bugfix-1691-tower-start-surface-refusal.test.ts b/packages/codev/src/agent-farm/__tests__/bugfix-1691-tower-start-surface-refusal.test.ts index a262a9079..1de7fbcee 100644 --- a/packages/codev/src/agent-farm/__tests__/bugfix-1691-tower-start-surface-refusal.test.ts +++ b/packages/codev/src/agent-farm/__tests__/bugfix-1691-tower-start-surface-refusal.test.ts @@ -71,12 +71,14 @@ vi.mock('node:child_process', async () => { const LOG_FILE = path.join(h.agentFarmDir, 'tower.log'); +// Mirrors ownershipConflictMessage() — the user-visible teaching error, with no +// internal issue numbers in the text. const TEACHING_ERROR = [ 'Refusing to start: this global.db is already owned by a live Tower.', ' owner pid 12345 on port 4100 (host testhost)', ' shared database dir: /Users/test/.agent-farm', - 'A second Tower opening a live global.db hijacks and deletes its shellper sessions (Issue #1629).', - 'If you meant to run an isolated test Tower, set CODEV_AGENT_FARM_DIR (NOT AGENT_FARM_DIR) to a throwaway directory (#1515).', + 'A second Tower opening a live global.db hijacks and deletes its shellper sessions.', + 'If you meant to run an isolated test Tower, set CODEV_AGENT_FARM_DIR (NOT AGENT_FARM_DIR) to a throwaway directory.', "Otherwise stop the existing Tower first ('afx tower stop'), or investigate pid 12345 if you believe it is stale.", ].join('\n'); diff --git a/packages/codev/src/agent-farm/db/tower-owner.ts b/packages/codev/src/agent-farm/db/tower-owner.ts index 5040f1c10..f98b7ba95 100644 --- a/packages/codev/src/agent-farm/db/tower-owner.ts +++ b/packages/codev/src/agent-farm/db/tower-owner.ts @@ -403,14 +403,19 @@ export async function claimGlobalDbOwnership(opts: { * host) and the shared DB, and — because the misconfiguration that triggered the * incident was a mistyped env var — spells out the `AGENT_FARM_DIR` vs * `CODEV_AGENT_FARM_DIR` fix. + * + * This string is user-visible at the CLI (surfaced by the launcher on a fast-exit, + * Issue #1691), so it carries no internal issue numbers — the incident context + * (#1629's shellper hijack, the #1515 mistyped-env-var trigger) stays here in the + * comment, not in the text a user reads. */ export function ownershipConflictMessage(conflict: TowerOwner, dbDir: string): string { return [ `Refusing to start: this global.db is already owned by a live Tower.`, ` owner pid ${conflict.pid} on port ${conflict.port} (host ${conflict.hostname})`, ` shared database dir: ${dbDir}`, - `A second Tower opening a live global.db hijacks and deletes its shellper sessions (Issue #1629).`, - `If you meant to run an isolated test Tower, set CODEV_AGENT_FARM_DIR (NOT AGENT_FARM_DIR) to a throwaway directory (#1515).`, + `A second Tower opening a live global.db hijacks and deletes its shellper sessions.`, + `If you meant to run an isolated test Tower, set CODEV_AGENT_FARM_DIR (NOT AGENT_FARM_DIR) to a throwaway directory.`, `Otherwise stop the existing Tower first ('afx tower stop'), or investigate pid ${conflict.pid} if you believe it is stale.`, ].join('\n'); } From f1fa4cc0bd924af24d942b871fd156e8a720caa4 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Thu, 17 Sep 2026 11:37:06 +1000 Subject: [PATCH 8/9] chore(porch): bugfix-1691 pr gate-approved --- .../bugfix-1691-afx-tower-start-surface-the-ow/status.yaml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml b/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml index 971837b9c..372fdcccb 100644 --- a/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml +++ b/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml @@ -6,11 +6,12 @@ plan_phases: [] current_plan_phase: null gates: pr: - status: pending + status: approved requested_at: '2026-09-16T23:58:57.118Z' + approved_at: '2026-09-17T01:37:06.595Z' iteration: 1 build_complete: false history: [] started_at: '2026-09-16T23:38:54.218Z' -updated_at: '2026-09-16T23:58:57.119Z' -pr_ready_for_human: true +updated_at: '2026-09-17T01:37:06.596Z' +pr_ready_for_human: false From e7a2ffd15eb9b28f50b99ea028c11bbef9501632 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Thu, 17 Sep 2026 11:37:33 +1000 Subject: [PATCH 9/9] chore(porch): bugfix-1691 record PR #1692 --- .../bugfix-1691-afx-tower-start-surface-the-ow/status.yaml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml b/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml index 372fdcccb..4ae1fbdcd 100644 --- a/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml +++ b/codev/projects/bugfix-1691-afx-tower-start-surface-the-ow/status.yaml @@ -13,5 +13,10 @@ iteration: 1 build_complete: false history: [] started_at: '2026-09-16T23:38:54.218Z' -updated_at: '2026-09-17T01:37:06.596Z' +updated_at: '2026-09-17T01:37:33.182Z' pr_ready_for_human: false +pr_history: + - phase: pr + pr_number: 1692 + branch: builder/bugfix-1691 + created_at: '2026-09-17T01:37:33.181Z'