Skip to content

Prefer shell function setup for Codex monitor shim#193

Open
yui-stingray wants to merge 2 commits into
fujibee:mainfrom
yui-stingray:codex/prefer-codex-function-monitor-shim
Open

Prefer shell function setup for Codex monitor shim#193
yui-stingray wants to merge 2 commits into
fujibee:mainfrom
yui-stingray:codex/prefer-codex-function-monitor-shim

Conversation

@yui-stingray

Copy link
Copy Markdown

Summary

  • make codex-shim-install.sh print a shell function by default instead of installing ~/.agents/bin/codex
  • make delivery set monitor codex print that shell function and leave the global PATH shim as an explicit opt-in
  • update Codex monitor docs, the Codex driver template, and tests for the function-first setup

Why

The current Codex monitor beta setup installs a global ~/.agents/bin/codex shim and then asks users to put ~/.agents/bin before the real Codex binary on PATH. That is easy to miss and affects every shell that inherits that PATH.

A shell function keeps the interception local to the user's interactive shell profile while still launching monitored Codex sessions with the normal codex command. The previous global PATH shim remains available via an explicit codex-shim-install.sh install command for users who prefer it.

Validation

  • bash -n scripts/drivers/types/codex/codex-shim-install.sh scripts/drivers/types/codex/_delivery.sh
  • git diff --check
  • bats tests/test_codex_shim.bats
  • bats --filter 'codex' tests/test_delivery.bats
  • bats tests/test_install.bats
  • manual smoke: delivery.sh set monitor codex prints a codex() { ... } function and does not create ~/.agents/bin/codex

Note: a full bats tests/test_delivery.bats run still hits an unrelated existing local failure in delivery set monitor: existing settings with single-quoted hook commands stays valid JSON (#134): SQLite reports Error: stepping, malformed JSON (1) for that test's seeded settings JSON before this patch's Codex-specific assertions are reached.

@yui-stingray yui-stingray force-pushed the codex/prefer-codex-function-monitor-shim branch from f772425 to 248482b Compare June 22, 2026 06:22
@yui-stingray yui-stingray marked this pull request as ready for review June 22, 2026 06:29
@yui-stingray yui-stingray force-pushed the codex/prefer-codex-function-monitor-shim branch 2 times, most recently from e002ba1 to 85de8c3 Compare June 22, 2026 06:57
@fujibee

fujibee commented Jun 23, 2026

Copy link
Copy Markdown
Owner

Thanks for this — the shell-function-first direction is sound and fits where #192 took Windows. It delegates to the existing bash codex-shim.sh rather than reimplementing anything, so it stays WSL-avoiding and path-pinned, and not silently creating a global ~/.agents/bin/codex that intercepts PATH everywhere is a real safety improvement. Keeping an explicit install is good too.

One blocking item — a coexistence bug for existing installs. A user who already has the generated ~/.agents/bin/codex wrapper (and install.sh --update keeps regenerating it while status is installed) and then adds the new shell function will hit a re-entry risk: the function calls codex-shim.sh directly, so AGMSG_CODEX_SHIM_TARGET is unset; resolve_real_codex then scans PATH and can mistake the ~/.agents/bin/codex wrapper for the real codex, looping function -> codex-shim.sh -> wrapper -> codex-shim.sh. The current wrapper sets AGMSG_CODEX_SHIM_TARGET precisely to avoid this. Please either have the emitted function set AGMSG_CODEX_SHIM_TARGET as well, or have codex-shim.sh detect and skip an existing agmsg wrapper found on PATH. A regression test for "function mode with an existing ~/.agents/bin/codex first on PATH still resolves the real codex" would lock it down.

Changing the default subcommand from install to function is a small breaking change for anyone scripting it, but the explicit install remains and the docs/tests follow — that's fine. With the coexistence fix, this is good to take.

@yui-stingray yui-stingray force-pushed the codex/prefer-codex-function-monitor-shim branch from 85de8c3 to fe1aacd Compare June 24, 2026 00:25
@yui-stingray

Copy link
Copy Markdown
Author

Thanks for the careful catch. I reproduced the coexistence case and confirmed the function path could resolve the existing generated ~/.agents/bin/codex wrapper as AGMSG_REAL_CODEX.

Updated in fe1aacd:

  • Hardened codex-shim.sh so resolve_real_codex skips generated agmsg PATH wrappers by sentinel marker, not only by AGMSG_CODEX_SHIM_TARGET. This keeps the invariant local to the resolver: it should never return agmsg's own wrapper as the real Codex binary.
  • Added regression coverage for the migration case: emitted shell function + existing agmsg ~/.agents/bin/codex first on PATH + real Codex behind it.
  • Added a companion guard that a non-agmsg ~/.agents/bin/codex remains eligible, so the resolver does not blindly skip that path.

Focused validation:

  • git diff --check
  • bash -n scripts/drivers/types/codex/codex-shim.sh scripts/drivers/types/codex/codex-shim-install.sh scripts/drivers/types/codex/_delivery.sh scripts/install.sh
  • bats tests/test_codex_shim.bats -> 9/9
  • bats -f 'codex' tests/test_delivery.bats -> 10/10
  • bats --filter 'Codex monitor shim' tests/test_install.bats -> 1/1

I also attempted the full bats tests/ suite. It reached all 390 cases but had unrelated existing/environment failures outside this change area (the known #134 single-quoted fixture, #124 watcher self-clean race, node resolve env, whoami env detection, and watch watermark/relaunch tests), then left test watcher processes running and was terminated. The focused Codex shim/delivery/install coverage above is green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants