v0.1.2: codex CLI subcommand shape + inbox poller activation + launchd PATH
Three production bugs caught by the codex agent live-testing the daemon
end-to-end against codex-cli 0.128 + a real LaunchAgent install:
1. Codex CLI 0.6+ moved `--resume` from a flag on `exec` to a
`resume` subcommand. The daemon was sending
`codex exec --skip-git-repo-check --resume <sid> <prompt>`, which
parses on 0.5.x but fails on 0.6.x+. Fixed to:
fresh: codex exec --skip-git-repo-check <prompt>
resume: codex exec resume --skip-git-repo-check <sid> <prompt>
Verified on codex-cli 0.128 with a live binary; the 0.5.x behavior
is preserved by the fake-codex test fixture, which now SystemExits
if it sees the legacy `--resume` flag (regression gate).
2. wait_for_message() never returned anything because nothing in the
daemon ever called molecule_runtime.inbox.activate() or started
the poller thread. The wheel ships those primitives but expects
the embedding runtime to wire them up — the workspace runtime
does this in start.py, but a standalone daemon embedding the
tools must do it itself. Added the activation + poller-thread
start in _RealTools.__init__.
3. The codex binary is a `#!/usr/bin/env node` shim. Under launchd /
stripped systemd units, the parent process PATH is `/usr/bin:/bin`
and `env node` 127s out silently. CodexRunner now prepends the
directory of the resolved codex binary to the subprocess PATH at
spawn time — Node lives next to codex under nvm / brew /
pnpm-global, so this restores the discovery without operators
having to thread PATH through their LaunchAgent / unit file.
README updated with a note on the launchd/systemd interaction.
Test:
- 31 passed (was 28). Three new regression gates: subcommand-shape,
PATH-prepend (launchd-default PATH stripped), and PATH-no-double
(idempotent when codex_bin_dir already present). Verified the two
behavioural new tests FAIL on the old codex_runner.py by stashing
the source change only and re-running.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
9abf6bfa79
commit
0f194d4507
@ -55,6 +55,14 @@ codex-channel-molecule
|
||||
|
||||
The daemon runs in the foreground; logs go to stderr. For systemd hosts, register a unit; for one-off use, `nohup ... &` plus a log file works.
|
||||
|
||||
### Running under launchd / systemd (Node-on-PATH note)
|
||||
|
||||
The codex binary is a `#!/usr/bin/env node` shim, so spawning it requires `node` to be discoverable on PATH. Under an interactive shell that's automatic; under `launchd` (macOS) and stripped-down `systemd` units PATH defaults to `/usr/bin:/bin`, and `env node` will 127-out silently.
|
||||
|
||||
Since 0.1.2 the daemon resolves `codex` to its absolute path and prepends that directory to the subprocess PATH automatically — Node lives next to `codex` under nvm / brew / pnpm-global, so the shim's `env node` finds it. Operators typically don't need any PATH plumbing in their LaunchAgent / unit file beyond the three required env vars.
|
||||
|
||||
If you're on an unusual install layout where `codex` and `node` live in different directories, set the LaunchAgent / unit `PATH` explicitly to include both.
|
||||
|
||||
## Deprecation path
|
||||
|
||||
When [`openai/codex#17543`](https://github.com/openai/codex/issues/17543) lands upstream — a generic path for handling MCP custom notifications in codex and forwarding them into the active session as user submissions — this daemon becomes redundant. Codex itself will accept inbound molecule messages as `Op::UserInput` directly through the MCP server already wired in `~/.codex/config.toml`. Until then, this is the operator-facing answer.
|
||||
|
||||
@ -64,8 +64,13 @@ class _RealTools:
|
||||
"""
|
||||
|
||||
def __init__(self) -> None:
|
||||
import molecule_runtime.inbox as inbox
|
||||
from molecule_runtime.a2a_client import PLATFORM_URL, WORKSPACE_ID
|
||||
from molecule_runtime import a2a_tools
|
||||
|
||||
state = inbox.InboxState(cursor_path=inbox.default_cursor_path())
|
||||
inbox.activate(state)
|
||||
inbox.start_poller_thread(state, PLATFORM_URL, WORKSPACE_ID)
|
||||
self._tools = a2a_tools
|
||||
|
||||
async def wait_for_message(self, timeout_secs: float) -> str:
|
||||
@ -253,6 +258,16 @@ async def _handle_one(
|
||||
)
|
||||
|
||||
result: CodexResult = await runner.run(message=body, session_id=session_id)
|
||||
logger.info(
|
||||
"codex result for %s: exit=%s session=%s stdout=%d stderr_tail=%d",
|
||||
activity_id,
|
||||
result.exit_code,
|
||||
result.session_id or "none",
|
||||
len(result.text),
|
||||
len(result.stderr_tail),
|
||||
)
|
||||
if result.exit_code != 0:
|
||||
logger.warning("codex stderr tail for %s: %s", activity_id, result.stderr_tail)
|
||||
if result.session_id and result.session_id != session_id:
|
||||
store.set(chat_id, result.session_id)
|
||||
|
||||
|
||||
@ -69,6 +69,31 @@ class CodexRunner:
|
||||
)
|
||||
self._codex_bin = resolved
|
||||
self._timeout_secs = timeout_secs
|
||||
# The codex binary is a `#!/usr/bin/env node` shim, so spawning
|
||||
# it requires `node` to be discoverable via the subprocess PATH.
|
||||
# Under a shell launch this is fine (operator's interactive PATH
|
||||
# has Node), but launchd and most systemd unit defaults strip
|
||||
# PATH down to /usr/bin:/bin — `env node` then 127s out and the
|
||||
# daemon silently fails on every turn.
|
||||
#
|
||||
# Fix: remember the directory containing the resolved codex
|
||||
# binary. Node lives next to it under nvm / brew / pnpm
|
||||
# global installs. Subprocesses get that dir prepended to their
|
||||
# PATH (see _build_env). Operators don't have to thread PATH
|
||||
# through their LaunchAgent / unit file — just $WORKSPACE_ID,
|
||||
# $PLATFORM_URL, $MOLECULE_WORKSPACE_TOKEN per the install
|
||||
# contract.
|
||||
self._codex_bin_dir = os.path.dirname(resolved)
|
||||
|
||||
def _build_env(self) -> dict[str, str]:
|
||||
env = os.environ.copy()
|
||||
existing_path = env.get("PATH", "")
|
||||
if self._codex_bin_dir not in existing_path.split(os.pathsep):
|
||||
env["PATH"] = (
|
||||
self._codex_bin_dir
|
||||
+ (os.pathsep + existing_path if existing_path else "")
|
||||
)
|
||||
return env
|
||||
|
||||
async def run(
|
||||
self,
|
||||
@ -81,16 +106,23 @@ class CodexRunner:
|
||||
that session. When None, codex starts a fresh session and assigns
|
||||
a new id (returned in CodexResult.session_id).
|
||||
"""
|
||||
args: list[str] = [self._codex_bin, "exec", "--skip-git-repo-check"]
|
||||
if session_id:
|
||||
args.extend(["--resume", session_id])
|
||||
args.append(message)
|
||||
args = [
|
||||
self._codex_bin,
|
||||
"exec",
|
||||
"resume",
|
||||
"--skip-git-repo-check",
|
||||
session_id,
|
||||
message,
|
||||
]
|
||||
else:
|
||||
args = [self._codex_bin, "exec", "--skip-git-repo-check", message]
|
||||
|
||||
proc = await asyncio.create_subprocess_exec(
|
||||
*args,
|
||||
stdout=asyncio.subprocess.PIPE,
|
||||
stderr=asyncio.subprocess.PIPE,
|
||||
env=os.environ.copy(),
|
||||
env=self._build_env(),
|
||||
)
|
||||
|
||||
try:
|
||||
|
||||
@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
|
||||
|
||||
[project]
|
||||
name = "codex-channel-molecule"
|
||||
version = "0.1.1"
|
||||
version = "0.1.2"
|
||||
description = "Bridge daemon for codex CLI ↔ Molecule platform — long-polls the platform inbox, runs `codex exec --resume <session>` per inbound message, replies via send_message_to_user MCP tool. Counterpart to hermes-channel-molecule."
|
||||
readme = "README.md"
|
||||
requires-python = ">=3.11"
|
||||
|
||||
@ -25,7 +25,8 @@ _FAKE_CODEX_SCRIPT = textwrap.dedent("""\
|
||||
\"\"\"Fake codex CLI for tests.
|
||||
|
||||
Behaviors keyed on argv shape and env:
|
||||
argv: codex exec [--skip-git-repo-check] [--resume <sid>] <message>
|
||||
argv: codex exec [--skip-git-repo-check] <message>
|
||||
argv: codex exec resume [--skip-git-repo-check] <sid> <message>
|
||||
|
||||
Echoes a banner to stderr (\"session: <uuid>\") and the input message
|
||||
to stdout. Honors FAKE_EXIT_CODE for failure-path tests.
|
||||
@ -35,17 +36,27 @@ _FAKE_CODEX_SCRIPT = textwrap.dedent("""\
|
||||
args = sys.argv[1:]
|
||||
assert args[0] == \"exec\", f\"unexpected first arg: {args[0]!r}\"
|
||||
|
||||
if \"--resume\" in args:
|
||||
raise SystemExit(\"--resume is not supported\")
|
||||
|
||||
resume_id = None
|
||||
i = 1
|
||||
while i < len(args):
|
||||
if args[i] == \"--resume\":
|
||||
resume_id = args[i + 1]
|
||||
i += 2
|
||||
elif args[i].startswith(\"--\"):
|
||||
i += 1 # skip unrecognized flag
|
||||
else:
|
||||
break
|
||||
msg = args[i] if i < len(args) else \"\"
|
||||
if len(args) > 1 and args[1] == \"resume\":
|
||||
i = 2
|
||||
while i < len(args):
|
||||
if args[i].startswith(\"--\"):
|
||||
i += 1
|
||||
else:
|
||||
break
|
||||
resume_id = args[i]
|
||||
msg = args[i + 1] if i + 1 < len(args) else \"\"
|
||||
else:
|
||||
i = 1
|
||||
while i < len(args):
|
||||
if args[i].startswith(\"--\"):
|
||||
i += 1 # skip unrecognized flag
|
||||
else:
|
||||
break
|
||||
msg = args[i] if i < len(args) else \"\"
|
||||
|
||||
sid = resume_id or os.environ.get(\"FAKE_NEW_SESSION_ID\", \"a1b2c3d4-1111-2222-3333-444455556666\")
|
||||
sys.stderr.write(f\"session: {sid}\\n\")
|
||||
@ -89,6 +100,8 @@ async def test_run_resumes_existing_session(fake_codex):
|
||||
result = await runner.run(message="follow up", session_id=given)
|
||||
# Resume → no new session id is captured (input is echoed back as-is).
|
||||
assert result.session_id == given
|
||||
assert result.exit_code == 0
|
||||
assert result.text == "echo: follow up"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@ -134,5 +147,103 @@ def test_extract_session_id_matches_alternate_banner_shape():
|
||||
) == "12345678-aaaa-bbbb-cccc-dddddddddddd"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_uses_exec_resume_subcommand_not_legacy_flag(fake_codex):
|
||||
"""Codex CLI 0.6+ rejects ``codex exec --resume <sid>``; the correct
|
||||
invocation is ``codex exec resume [--skip-git-repo-check] <sid> <prompt>``.
|
||||
The fake-codex fixture raises SystemExit if it sees ``--resume`` in
|
||||
argv, so the assertion is that the call SUCCEEDS — proving the
|
||||
runner moved off the legacy flag.
|
||||
"""
|
||||
runner = CodexRunner(codex_bin=str(fake_codex), timeout_secs=10.0)
|
||||
sid = "deadbeef-1111-2222-3333-444455556666"
|
||||
result = await runner.run(message="follow", session_id=sid)
|
||||
# Subprocess returned 0 because the fake didn't see --resume. If the
|
||||
# runner ever regresses to the old shape, the fake exits non-zero
|
||||
# with "--resume is not supported" → result.exit_code != 0.
|
||||
assert result.exit_code == 0, (
|
||||
f"Expected exec resume subcommand shape, got "
|
||||
f"exit_code={result.exit_code} stderr_tail={result.stderr_tail!r}"
|
||||
)
|
||||
assert result.session_id == sid
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_prepends_codex_bin_dir_to_subprocess_path(tmp_path):
|
||||
"""The codex binary is a `#!/usr/bin/env node` shim. Under launchd /
|
||||
systemd, the parent process's PATH is stripped to /usr/bin:/bin, so
|
||||
the shebang's `env node` lookup fails and codex 127s out silently.
|
||||
The runner mitigates this by prepending the codex binary's own
|
||||
directory (where node lives in nvm/brew/pnpm-global layouts) to the
|
||||
subprocess PATH. Pin: subprocess sees that dir first in PATH.
|
||||
"""
|
||||
bin_dir = tmp_path / "fakebin"
|
||||
bin_dir.mkdir()
|
||||
fake = bin_dir / "codex"
|
||||
# Echo the resolved subprocess PATH so the test can inspect it.
|
||||
fake.write_text(textwrap.dedent("""\
|
||||
#!/usr/bin/env python3
|
||||
import os, sys
|
||||
sys.stderr.write(f"session: a1b2c3d4-1111-2222-3333-444455556666\\n")
|
||||
sys.stdout.write(os.environ.get("PATH", ""))
|
||||
"""))
|
||||
fake.chmod(0o755)
|
||||
|
||||
runner = CodexRunner(codex_bin=str(fake), timeout_secs=10.0)
|
||||
# Strip PATH down to the launchd-default minimum to prove the
|
||||
# runner's own injection puts bin_dir back.
|
||||
import os as _os
|
||||
saved = _os.environ.get("PATH", "")
|
||||
_os.environ["PATH"] = "/usr/bin:/bin"
|
||||
try:
|
||||
result = await runner.run(message="probe")
|
||||
finally:
|
||||
_os.environ["PATH"] = saved
|
||||
|
||||
assert result.exit_code == 0
|
||||
seen_path = result.text
|
||||
# bin_dir must appear FIRST so `env node` finds it before falling
|
||||
# back to the (empty for launchd) system PATH.
|
||||
assert seen_path.startswith(str(bin_dir) + os.pathsep), (
|
||||
f"expected codex_bin_dir prefix in subprocess PATH, got: {seen_path!r}"
|
||||
)
|
||||
assert "/usr/bin" in seen_path, (
|
||||
f"expected existing PATH preserved after prefix, got: {seen_path!r}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_does_not_duplicate_codex_bin_dir_in_path(tmp_path):
|
||||
"""If the operator's PATH already includes the codex bin dir (the
|
||||
shell-launched case), don't double it — keeps logs and downstream
|
||||
tools sane.
|
||||
"""
|
||||
bin_dir = tmp_path / "alreadythere"
|
||||
bin_dir.mkdir()
|
||||
fake = bin_dir / "codex"
|
||||
fake.write_text(textwrap.dedent("""\
|
||||
#!/usr/bin/env python3
|
||||
import os, sys
|
||||
sys.stderr.write(f"session: a1b2c3d4-1111-2222-3333-444455556666\\n")
|
||||
sys.stdout.write(os.environ.get("PATH", ""))
|
||||
"""))
|
||||
fake.chmod(0o755)
|
||||
|
||||
runner = CodexRunner(codex_bin=str(fake), timeout_secs=10.0)
|
||||
import os as _os
|
||||
saved = _os.environ.get("PATH", "")
|
||||
_os.environ["PATH"] = f"{bin_dir}:/usr/bin:/bin"
|
||||
try:
|
||||
result = await runner.run(message="probe")
|
||||
finally:
|
||||
_os.environ["PATH"] = saved
|
||||
|
||||
seen_path = result.text
|
||||
occurrences = seen_path.split(os.pathsep).count(str(bin_dir))
|
||||
assert occurrences == 1, (
|
||||
f"expected codex_bin_dir to appear exactly once, got {occurrences}: {seen_path!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_extract_session_id_returns_none_on_no_match():
|
||||
assert _extract_session_id("nothing relevant here") is None
|
||||
|
||||
Loading…
Reference in New Issue
Block a user