Fix #1691: surface tower owner-guard refusal in the CLI, not a 30s timeout - #1692
Merged
Merged
Conversation
…meout `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).
- 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.
…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.
amrmelsayed
added a commit
that referenced
this pull request
Sep 17, 2026
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
afx tower startdaemonizes the tower-server and waited only on the port for readiness, so when the #1629 owner-lock guard refuses (the daemon logs its teaching error and exits within ~1s) the launcher burned the full 30s budget and printed a generic "failed to respond within 30000ms" — indistinguishable from a genuine hang (#1685 class), with the guard's teaching error reaching onlytower.log. The readiness wait now watches the spawned daemon's liveness, distinguishes three outcomes, and on a fast-exit surfaces the daemon's refusal verbatim.Fixes #1691
Refs #1690 (item 1 of the owner-lock robustness follow-up; items 2+3 remain there)
Root Cause
waitForServer(port)inpackages/codev/src/agent-farm/commands/tower.tspolled/api/statusfor up toSTARTUP_TIMEOUT_MS(30s) with no awareness of the spawned daemon's liveness. WhenbootSequence(tower-server.ts) fails the owner-lock claim it callslog('ERROR', ownershipConflictMessage(...))thenprocess.exit(1); the port never comes up, so the loop ran the full timeout and then emitted a generic message. The teaching error was only ever written totower.log.Fix
packages/codev/src/agent-farm/commands/tower.ts(+89/-11):TowerStartupOutcome = 'started' | 'exited' | 'timeout'andwaitForServerOutcome(isReady, isDaemonAlive, opts)replacing the booleanwaitForServer. It short-circuits toexitedthe instant the daemon is seen dead (with a final readiness re-probe to close the benign same-tick race), so a refusal no longer burns the 30s budget.towerStartregistersserverProcess.on('exit')and captures thetower.logbyte offset right after its own pre-spawn writes. Onexitedit reads everything the daemon appended for this run (readLogSince) and prints it verbatim on stderr (the owner-guard teaching error, or any early-boot failure), then exits non-zero. The three outcomes are now distinguishable in output: started / exited (refused-with-reason) / timeout (still running, status unknown).No skeleton twin: the Tower launcher is product code, not a shipped template.
Test Plan
bugfix-1691-tower-start-surface-refusal.test.ts, 6 tests): the outcome logic incl. the "no 30s burn" timing guarantee and the benign same-tick race; and twotowerStarttests (mocked spawn/http/shell) proving the teaching error is surfaced verbatim on stderr with a non-zero exit within seconds, plus the empty-log fallback.pnpm build)porch check;tsc --noEmitclean)