fix(codex): engage the monitor bridge on codex 0.141 (ws transport + loaded-thread discovery)#174
Merged
Merged
Conversation
…loaded-thread discovery) codex 0.141 rejects `--remote unix://` (ws-only) so the TUI exited before the bridge could attach, and it no longer exposes the thread id to hooks (no CODEX_THREAD_ID, no rollout for --remote), so the launcher never started the bridge. Three changes make the monitor bridge engage again: - codex-bridge.js: the WebSocket app-server client connects over either a unix socket or a TCP host/port, so --app-server accepts ws://host:port. Add --thread loaded, which discovers the live TUI thread via thread/loaded/list instead of relying on a hook-resolved id. - codex-monitor.sh: run the shared app-server on ws://127.0.0.1:<port> (recorded per project for reuse) and connect the TUI with --remote ws://... - codex-bridge-launcher.sh: resolve the project's single codex identity itself and start the bridge with --thread loaded; a request file with a real thread id (older codex) still takes precedence. Tests: ws://host:port connect, thread/loaded/list discovery, and the no-loaded timeout; the prior 'rejects unsupported endpoints' test now uses http:// since ws:// is supported. Closes #170
The ws migration picked the loopback port with a node -e one-liner, which made codex-monitor.sh itself fail (and take down the Codex TUI) when Node was not on PATH — e.g. an nvm-only Node in a non-interactive spawn shell. Let the app-server pick the port (--listen ws://127.0.0.1:0) and parse the reported 'listening on' line instead. codex-monitor.sh now needs no Node; only the bridge does, and it degrades on its own if Node is missing rather than aborting the launch.
The bridge (codex-bridge.js) is launched via its env-node shebang, but an nvm/fnm/volta Node is only on PATH in interactive shells — so a bridge started from a non-interactive context (a spawn boot script) cannot find Node and Codex monitor silently never starts, even though Node is installed. Add lib/node.sh::agmsg_resolve_node — PATH, then AGMSG_NODE, then the newest nvm/fnm node, then volta/homebrew/usr-local — and launch the bridge with the resolved binary from codex-bridge-launcher.sh and session-start.sh. Falls back to bare "node" so behaviour is never worse than relying on PATH (the existing delivery.sh preflight still warns when no Node is found).
…as-is The Node-resolution change launched the bridge as `$node $bridge_cmd`, which broke AGMSG_CODEX_BRIDGE_CMD overrides that point at a non-Node runnable (the delivery/session-start tests use a bash stub) — Node tried to parse the stub and no bridge started. Run an explicit AGMSG_CODEX_BRIDGE_CMD as-is; only the default codex-bridge.js goes through the resolved Node. Fixes the 3 session-start codex tests that regressed on CI.
…p-server reuse Addresses review notes on #174: - delivery.sh Node preflight now resolves via lib/node.sh (the same path the launcher uses), so it no longer false-warns when only a version-manager Node is present, and it accepts AGMSG_NODE (canonical) as well as AGMSG_CODEX_NODE. An explicit override is returned verbatim so a bogus value still warns. - codex-monitor.sh reuses an existing app-server only when its recorded SERVER_PID is alive AND the port answers, so a foreign process that grabbed the same port after ours died is not mistaken for the bridge app-server.
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
On codex 0.141 the Codex monitor bridge never engaged, so a registered
codexagent received no pushed messages while idle (#170). Two breakages:codex --remote <addr>now accepts onlyws://host:port(rejectsunix://), but the bridge used aunix://socket — the TUI exited before the bridge could attach.--remote, codex no longer exposes the thread id to hooks (CODEX_THREAD_IDunset, no rollout under~/.codex/sessions/), so the launcher never learned which thread to attach to.Changes
codex-bridge.js— the WebSocket app-server client now connects over either a unix socket ({ path }) or a TCP host/port ({ host, port }), so--app-serveracceptsws://host:portas well asunix://PATH(handshake/framing unchanged). New--thread loadeddiscovers the live TUI thread viathread/loaded/listinstead of a hook-resolved id.codex-monitor.sh— runs the shared app-server onws://127.0.0.1:<port>(port recorded per project for reuse) and connects the TUI with--remote ws://127.0.0.1:<port>.codex-bridge-launcher.sh— resolves the project's single codex identity itself and starts the bridge with--thread loaded. A request file carrying a real thread id (older codex, hook path) still takes precedence, preserving backward compatibility.Verification
tests/test_codex_bridge.bats(16 cases, all green locally): addedws://host:portconnect,thread/loaded/listdiscovery, and the no-loaded-thread timeout. The prior "rejects unsupported endpoints" case now useshttp://sincews://is supported.codex app-server --listendocumentsws://IP:PORT, andgenerate-json-schemaexposesthread/loaded/list→{ data: string[] }.Closes #170