Prevent duplicate Codex resume launch - #228
Conversation
|
Follow-up from the independent Codex review: this PR now also closes the confirmed P1/P2 findings. Codex daemon readiness requires the actual Codex command in the zmx task row; the app waits for that daemon-owned session before attaching, preventing a bare attach shell from blocking launch. Remote status now reads zmx ls task state rather than stale ZMX_TASK_COMPLETED history. Focused AttachedSessionBriefingTests, ZmxSessionLauncherTests, and RemoteLoopSurvivalTests pass. |
The status probe matched the session name with grep -F over a pattern holding a literal backslash-t, which matches no zmx ls row — every remote probe reported an existing session absent. The tab is a real one now (grep -F never interprets escapes). The app's waiting attach polled zmx ls at 10 Hz forever when the daemon never created the session; it now gives up after a minute with a message. Also formats the new daemon-check code to the restored swift-format gate.
scgopi
left a comment
scgopi
left a comment
There was a problem hiding this comment.
Review of the diff, three findings — all fixed and pushed as db669f3 (focused suites: 63 tests pass; swift format + swiftlint clean).
P1 — remote status probe can never match a session. remoteStatusInvocation built the grep -F pattern from a Swift \\t, i.e. the two characters backslash-t. grep -F never interprets escapes, and zmx ls rows are real-tab-separated (name=…\tpid=…\tcmd=…), so gc_row was always empty and every remote probe reported an existing session absent — remote loops would all read as dead, sends would stage instead of delivering. The pattern now carries a real tab; pinned by assertions in RemoteLoopSurvivalTests (present real tab, absent literal backslash-t).
P2 — the waiting attach polls forever. waitingAttachCommand's until loop had no cap: if the daemon never creates the session (failed launch, loop deleted mid-wait), the pane spawns two zmx ls pipelines ten times a second indefinitely. Now capped at 60s, then prints a message and exits 1; the success path is unchanged (exec attach once cmd=.*codex appears). Pinned in appWaitsForTheDaemonBeforeAttachingCodex.
P3 — PR fails the restored swift-format gate. make check failed on the branch: three [AddLines] errors in the new daemonReadyCheckCommand/waitingAttachCommand bodies (swift format format --in-place applied; diff confined to those lines).
Verified not findings: the sed \\t in the probe works on both BSD and GNU sed (checked on macOS); cmd=.*codex is correct because the daemon launches via zmx run <name> -d codex …, so daemon-owned rows carry the executable, while a bare attach husk correctly fails the check; reordering defersCodexLaunchToDaemon ahead of localResumeOrFreshCommand is the intended fix for the duplicate-resume race.
1 participant
Summary
This is a follow-up to merged PR #219. Claude launch behavior is unchanged.
Verification
make test