diff --git a/docs/adr/0005-codex-invocation-scoped-app-server.md b/docs/adr/0005-codex-invocation-scoped-app-server.md new file mode 100644 index 000000000..49b03f1db --- /dev/null +++ b/docs/adr/0005-codex-invocation-scoped-app-server.md @@ -0,0 +1,76 @@ +# ADR 0005: Codex invocation-scoped app-server lifecycle + +**Status:** proposed +**Date:** 2026-08-21 +**Deciders:** @fujibee + +## Context + +The Codex monitor currently reuses one app-server per project, launches its +bridge dispatcher in the background, and then replaces the monitor with the +Codex TUI via `exec`. Remote tool processes inherit the TUI thread working +directory from that shared app-server, so closing one TUI does not provide an +ownership boundary for its workers. The orphan behavior is tracked in +[#149](https://github.com/fujibee/agmsg/issues/149). + +Some callers launch Codex in a disposable Git worktree and need a bounded +lifecycle boundary before deciding whether the worktree is ready for removal. +A project hash, process name, shared parent PID, or logical Codex thread ID +cannot prove OS-process ownership when several sessions share one app-server. + +## Decision + +Add an opt-in `codex-monitor.sh --invocation-scope ` mode. The token is +validated, combined with the canonical project path, and hashed before it is +used as the app-server record key. A scoped launch never reuses an existing +app-server: the monitor acquires an exact scope lease, starts a fresh app-server +as its child, runs the Codex TUI as a supervised foreground child, and on TUI +exit stops and waits for its captured app-server, bridge-launcher, and TUI +processes before returning the TUI status. A direct `TERM` takes the same path +and returns status 143. A live duplicate scope fails closed. Scope-less +launches retain the existing project-shared app-server and `exec` behavior. + +Only app-server lifetime is invocation-scoped. agmsg role seating, the bridge +request, and dispatcher ownership remain project-scoped so concurrent sessions +cannot create duplicate inbox consumers. The scoped server key is inherited +internally as `AGMSG_CODEX_APP_SERVER_KEY`, allowing hook-side code running in +the app-server context to resolve the correct port without exposing the raw +token or falling back to another invocation's project server. + +## Alternatives considered + +- **Classify every shared app-server child as owned.** Rejected because a shared + server can have same-cwd workers from another Codex session. +- **Bind cleanup only to Codex thread IDs.** Rejected because thread APIs do not + enumerate every OS worker or provide a complete PID ownership tree. +- **Disable agmsg monitor for disposable worktrees.** Safe, but removes real-time + delivery from the exact sessions that use multi-agent work most heavily. +- **Make every bridge and role record invocation-scoped.** Rejected because it + would create competing consumers for one project role and is unnecessary for + process ownership. +- **Change all monitor launches to scoped lifetime.** Rejected for compatibility; + current users may rely on project-wide server reuse. + +## Consequences + +- Positive: a scoped monitor captures exact app-server, bridge-launcher, and + TUI processes that it can stop and wait for without inspecting or signalling + foreign project sessions. +- Positive: fresh and resume launches use the same lifecycle contract, preserve + the Codex exit status, and leave the existing no-scope behavior unchanged. +- Positive: no daemon, external dependency, or second cleanup implementation is + introduced. +- Negative: scoped launches pay app-server startup cost on every invocation. +- Negative: the monitor does not prove that any downstream remote or role + descendant has independently exited. They naturally bind to the scoped + app-server lifetime, while the final caller still decides readiness. +- Negative: `SIGKILL` can leave a stale lease; the next same-scope launch fails + closed unless it can prove the recorded owner is dead before reclaiming it. +- Neutral: project role seating remains single-owner/latest-seat behavior. + +## References + +- [Issue #149](https://github.com/fujibee/agmsg/issues/149) +- `scripts/drivers/types/codex/codex-monitor.sh` +- `scripts/drivers/types/codex/codex-bridge-launcher.sh` +- `docs/codex-monitor-beta.md` diff --git a/docs/codex-monitor-beta.md b/docs/codex-monitor-beta.md index 4fa3ba1ef..5d55a9d5f 100644 --- a/docs/codex-monitor-beta.md +++ b/docs/codex-monitor-beta.md @@ -13,9 +13,10 @@ approximates the same experience by launching Codex through an app-server bridge > enabling monitor takes effect only after you **restart Codex and send your > first message** — the SessionStart hook fires on the first turn, not the > moment Codex opens, so the bridge is absent until you interact once; an -> already-running session stays unmonitored until you restart it (#151); the -> bridge is not torn down when you close the TUI (orphans linger until reboot -> or `mode off`/manual kill, see #149). +> already-running session stays unmonitored until you restart it (#151). A +> scope-less launch retains the shared app-server lifetime, so use the opt-in +> invocation scope below when a disposable worktree needs a bounded lifecycle +> (#149). ## Quick Start @@ -110,6 +111,32 @@ codex logout The shim also passes through when the current project is not in Codex monitor mode. +## Invocation-scoped lifetime (opt-in) + +The normal, scope-less monitor keeps its existing behavior: it reuses a live +app-server for the project and `exec`s the Codex TUI. Use an invocation scope +only when a caller needs a disposable-worktree lifecycle boundary: + +```bash +codex-monitor.sh --project "$PWD" --invocation-scope "$opaque_scope" --codex-command codex -- -C "$target" +``` + +`$opaque_scope` is a unique, non-secret token for this invocation. The monitor +combines it with the canonical project path and records only the resulting key. +Scoped launches always start a fresh app-server, so they pay startup cost and +do not reuse the project server. + +The scoped monitor captures its Codex TUI, app-server, and bridge launcher. +When the TUI exits, it stops and waits for those captured processes before +returning the TUI status. A direct `TERM` follows the same cleanup path and +returns status `143`. + +This does not prove that any downstream remote or role descendants have +independently exited. They naturally bind to the scoped app-server lifetime; +the final caller still decides whether its own readiness condition is met. +`SIGKILL` cannot run cleanup and can leave a stale lease. The next launch with +that same scope fails closed unless it can prove the recorded owner is dead. + ## Bridge Mechanics `codex-monitor.sh` starts (or reuses) an agmsg-managed Codex app-server socket diff --git a/scripts/drivers/types/codex/_app-server.sh b/scripts/drivers/types/codex/_app-server.sh index a8ed5658c..8baf5d006 100644 --- a/scripts/drivers/types/codex/_app-server.sh +++ b/scripts/drivers/types/codex/_app-server.sh @@ -26,18 +26,29 @@ # Echo the app-server URL for , or nothing. # -# The environment variable wins when present: it is the value monitor exported -# for this very process, and preferring it keeps every context that already -# worked on exactly the path it used before. +# With no scoped key, the environment variable still wins: it is the value a +# legacy monitor exported for this process. A scoped key identifies the exact +# server instead, so its port record must win over any inherited generic URL. +_agmsg_codex_app_server_record_key() { + local project="$1" + if [ -n "${AGMSG_CODEX_APP_SERVER_KEY:-}" ]; then + printf '%s' "$AGMSG_CODEX_APP_SERVER_KEY" + else + printf '%s' "$project" | agmsg_sha1 + fi +} + _agmsg_codex_app_server_url() { - local project="$1" port_file port + local project="$1" record_key port_file port [ -n "$project" ] || return 0 - if [ -n "${AGMSG_CODEX_BRIDGE_APP_SERVER:-}" ]; then + if [ -z "${AGMSG_CODEX_APP_SERVER_KEY:-}" ] \ + && [ -n "${AGMSG_CODEX_BRIDGE_APP_SERVER:-}" ]; then printf '%s' "$AGMSG_CODEX_BRIDGE_APP_SERVER" return 0 fi command -v agmsg_sha1 >/dev/null 2>&1 || return 0 - port_file="$SKILL_DIR/run/codex-app-server.$(printf '%s' "$project" | agmsg_sha1 2>/dev/null).port" + record_key="$(_agmsg_codex_app_server_record_key "$project")" + port_file="$SKILL_DIR/run/codex-app-server.$record_key.port" port="$(cat "$port_file" 2>/dev/null || true)" # Digits, and a port a TCP stack could have handed out. Digits alone are not # enough on their own — a prefix of a real port (5 of 52962) is all digits and diff --git a/scripts/drivers/types/codex/_session-start.sh b/scripts/drivers/types/codex/_session-start.sh index 1aa55fb9b..eefa51b9d 100644 --- a/scripts/drivers/types/codex/_session-start.sh +++ b/scripts/drivers/types/codex/_session-start.sh @@ -14,6 +14,9 @@ # launcher start the bridge — a hook-launched bridge cannot connect to the unix # socket from inside the Codex sandbox (#41). +# shellcheck source=_app-server.sh +source "$SKILL_DIR/scripts/drivers/types/codex/_app-server.sh" + # Newest-N rollout files under $sessions_dir, sorted by mtime descending. # `ls -t "$dir"/*/*/*/rollout-*.jsonl` is unreliable on Windows/Git Bash -- # reported to intermittently return an empty/truncated list with no @@ -120,21 +123,28 @@ agmsg_session_start() { done <<< "$PAIRS" PAIRS="$safe_pairs" [ -n "$PAIRS" ] || exit 0 - app_server="${AGMSG_CODEX_BRIDGE_APP_SERVER:-}" - if [ -z "$app_server" ]; then - agent_pid=$(agmsg_agent_pid "$TYPE" 2>/dev/null || true) - if [ -n "$agent_pid" ]; then - agent_cmd=$(compat_get_cmdline "$agent_pid" 2>/dev/null || true) - app_server=$(printf '%s\n' "$agent_cmd" \ - | sed -n 's/.*\(unix:\/\/[^[:space:]]*\).*/\1/p' \ - | head -1) + if [ -n "${AGMSG_CODEX_APP_SERVER_KEY:-}" ]; then + app_server="$(_agmsg_codex_app_server_url "$PROJECT")" + else + app_server="${AGMSG_CODEX_BRIDGE_APP_SERVER:-}" + if [ -z "$app_server" ]; then + agent_pid=$(agmsg_agent_pid "$TYPE" 2>/dev/null || true) + if [ -n "$agent_pid" ]; then + agent_cmd=$(compat_get_cmdline "$agent_pid" 2>/dev/null || true) + app_server=$(printf '%s\n' "$agent_cmd" \ + | sed -n 's/.*\(unix:\/\/[^[:space:]]*\).*/\1/p' \ + | head -1) + fi fi - fi - if [ -z "$app_server" ]; then - project_hash=$(printf '%s' "$PROJECT" | agmsg_sha1) - socket_path="$RUN_DIR/codex-app-server.$project_hash.sock" - if [ -S "$socket_path" ] || [ "${AGMSG_TEST_ASSUME_CODEX_SOCKET:-}" = "$socket_path" ]; then - app_server="unix://$socket_path" + if [ -z "$app_server" ]; then + app_server="$(_agmsg_codex_app_server_url "$PROJECT")" + fi + if [ -z "$app_server" ]; then + project_hash=$(printf '%s' "$PROJECT" | agmsg_sha1) + socket_path="$RUN_DIR/codex-app-server.$project_hash.sock" + if [ -S "$socket_path" ] || [ "${AGMSG_TEST_ASSUME_CODEX_SOCKET:-}" = "$socket_path" ]; then + app_server="unix://$socket_path" + fi fi fi [ -n "$app_server" ] || exit 0 diff --git a/scripts/drivers/types/codex/codex-bridge-launcher.sh b/scripts/drivers/types/codex/codex-bridge-launcher.sh index aedcba555..a60d45ea2 100755 --- a/scripts/drivers/types/codex/codex-bridge-launcher.sh +++ b/scripts/drivers/types/codex/codex-bridge-launcher.sh @@ -38,6 +38,8 @@ source "$SCRIPT_DIR/../../../lib/close-fds.sh" agmsg_close_inherited_fds # shellcheck source=../../../lib/hash.sh source "$SCRIPT_DIR/../../../lib/hash.sh" +# shellcheck source=_app-server.sh +source "$SCRIPT_DIR/_app-server.sh" # The liveness helpers. Every lifetime and lock-owner check below goes through # one of them, chosen by where the pid was minted: _agmsg_pid_alive_local for # the ones this shell or codex-monitor.sh produced, _agmsg_pid_alive for the @@ -49,7 +51,8 @@ source "$SCRIPT_DIR/../../../lib/instance-id.sh" PROJECT_HASH="$(printf '%s' "$PROJECT" | agmsg_sha1)" REQUEST_FILE="$RUN_DIR/codex-bridge-request.$PROJECT_HASH" DISPATCHER_LOCK_RESOURCE="codex-dispatcher:$PROJECT_HASH" -SERVER_PID_FILE="$RUN_DIR/codex-app-server.$PROJECT_HASH.pid" +SERVER_RECORD_KEY="$(_agmsg_codex_app_server_record_key "$PROJECT")" +SERVER_PID_FILE="$RUN_DIR/codex-app-server.$SERVER_RECORD_KEY.pid" # shellcheck source=../../../lib/node.sh source "$SCRIPT_DIR/../../../lib/node.sh" @@ -71,12 +74,13 @@ PROJECT_PHYS="$(agmsg_canonical_path "$PROJECT" 2>/dev/null || printf '%s' "$PRO mkdir -p "$RUN_DIR" -# The app-server is shared by every Codex TUI in a project. Bind dispatcher and -# role-child lifetime to that shared process rather than whichever TUI happened -# to start first. Tests/older launchers without the sidecar retain parent-PID -# fallback behavior. +# Scoped launchers must bind to their exact server record; a missing/dead record +# is the end of that scope, never permission to follow a project peer. Legacy +# launchers retain the parent-PID fallback used before scoped servers existed. LIFETIME_PID="$(cat "$SERVER_PID_FILE" 2>/dev/null || true)" -if [ -z "$LIFETIME_PID" ] || ! _agmsg_pid_alive_local "$LIFETIME_PID"; then +if [ -n "${AGMSG_CODEX_APP_SERVER_KEY:-}" ]; then + [ -n "$LIFETIME_PID" ] && _agmsg_pid_alive_local "$LIFETIME_PID" || exit 0 +elif [ -z "$LIFETIME_PID" ] || ! _agmsg_pid_alive_local "$LIFETIME_PID"; then LIFETIME_PID="$PARENT_PID" fi @@ -224,6 +228,32 @@ poll_sleep() { return 0 } +mark_runtime_lock_standby() { + local resource="$1" + [ -n "${AGMSG_TEST_LOCK_STANDBY_BARRIER:-}" ] || return 0 + printf '%s' "$resource" > "$AGMSG_TEST_LOCK_STANDBY_BARRIER.$$" +} + +acquire_runtime_lock_while_alive() { + local resource="$1" lifetime_pid="$2" + while _agmsg_pid_alive_local "$lifetime_pid"; do + if acquire_runtime_lock "$resource"; then + if _agmsg_pid_alive_local "$lifetime_pid"; then + poll_reset + return 0 + fi + # The scope died inside acquisition. Drop only our CAS row; never signal + # the previous owner or any process from another scope. + agmsg_runtime_lock_release "$resource" "$$" || true + HELD_LOCK_RESOURCE="" + return 1 + fi + mark_runtime_lock_standby "$resource" + poll_sleep + done + return 1 +} + # Any change here can change the safe subscription set. Include the request # thread plus each role's recorded session/project, not merely registrations: # actas/resume rewrites a role record without changing identities.sh output. @@ -254,7 +284,15 @@ build_safety_state() { # The parent only dispatches. Every role receives an independent child launcher # and therefore an independent bridge bound to its own recorded thread. if [ -z "$ROLE_PAIR" ]; then - acquire_runtime_lock "$DISPATCHER_LOCK_RESOURCE" || exit 0 + # A second scoped server shares this project dispatcher, but it must not + # disappear just because the first scope currently owns the CAS row. Retry + # only while this launcher's exact app-server lifetime is alive; that lifetime + # is the bound, so a dead scope neither polls forever nor signals its peer. + if [ -n "${AGMSG_CODEX_APP_SERVER_KEY:-}" ]; then + acquire_runtime_lock_while_alive "$DISPATCHER_LOCK_RESOURCE" "$LIFETIME_PID" || exit 0 + else + acquire_runtime_lock "$DISPATCHER_LOCK_RESOURCE" || exit 0 + fi known_pairs="" while agmsg_runtime_lock_verify "$DISPATCHER_LOCK_RESOURCE" "$$" \ && _agmsg_pid_alive_local "$LIFETIME_PID"; do @@ -293,7 +331,11 @@ fi # this lock every dispatcher generation left another full set of children behind. # The lock makes those re-spawns exit on arrival instead of accumulating. CHILD_LOCK_RESOURCE="codex-child:$PROJECT_HASH:$(printf '%s' "$ROLE_PAIR" | agmsg_sha1)" -acquire_runtime_lock "$CHILD_LOCK_RESOURCE" || exit 0 +if [ -n "${AGMSG_CODEX_APP_SERVER_KEY:-}" ]; then + acquire_runtime_lock_while_alive "$CHILD_LOCK_RESOURCE" "$PARENT_PID" || exit 0 +else + acquire_runtime_lock "$CHILD_LOCK_RESOURCE" || exit 0 +fi # Bounded, not open-ended. The dispatcher only spawns a child for a pair it has # already seen registered, so an empty list here is either the brief actas write diff --git a/scripts/drivers/types/codex/codex-monitor.sh b/scripts/drivers/types/codex/codex-monitor.sh index 8f580c917..dd9dcbf1d 100755 --- a/scripts/drivers/types/codex/codex-monitor.sh +++ b/scripts/drivers/types/codex/codex-monitor.sh @@ -29,20 +29,26 @@ source "$SCRIPT_DIR/../../../lib/instance-id.sh" # good one. Only the writer can make that state unobservable. # shellcheck source=../../../lib/registry-lock.sh source "$SCRIPT_DIR/../../../lib/registry-lock.sh" +# shellcheck source=../../../lib/storage.sh +source "$SCRIPT_DIR/../../../lib/storage.sh" PROJECT="$(pwd)" SOCKET_PATH="" CODEX_COMMAND="resume" CODEX_ARGS=() REAL_CODEX="${AGMSG_REAL_CODEX:-codex}" +INVOCATION_SCOPE="" +SCOPE_SEEN=0 usage() { cat <] [--codex-command ] [-- ] +Usage: codex-monitor.sh [--project ] [--invocation-scope TOKEN] [--codex-command ] [-- ] -Starts/reuses an agmsg-managed Codex app-server on a loopback ws:// port, -enables agmsg Codex bridge delivery for this project, then execs: - codex resume --remote ws://127.0.0.1: +Without --invocation-scope, starts/reuses the project's agmsg-managed Codex +app-server, enables bridge delivery, then execs the selected Codex command. +With --invocation-scope TOKEN, starts a dedicated app-server and supervises the +selected Codex command instead of execing it. +Scoped mode waits for its captured TUI, app-server, and bridge launcher processes. (--socket-path is accepted for compatibility but ignored: codex 0.141+ requires a ws:// transport for --remote. See #170.) @@ -67,6 +73,19 @@ while [ "$#" -gt 0 ]; do CODEX_COMMAND="${2:?--codex-command requires codex or resume}" shift 2 ;; + --invocation-scope) + [ "$SCOPE_SEEN" -eq 0 ] || { echo "codex-monitor: --invocation-scope may be given once" >&2; exit 2; } + [ "$#" -ge 2 ] || { echo "codex-monitor: --invocation-scope requires a value" >&2; exit 2; } + INVOCATION_SCOPE="$2" + case "$INVOCATION_SCOPE" in + [A-Za-z0-9]* ) ;; + * ) exit 2 ;; + esac + case "$INVOCATION_SCOPE" in *[!A-Za-z0-9._-]* ) exit 2 ;; esac + [ "${#INVOCATION_SCOPE}" -le 128 ] || exit 2 + SCOPE_SEEN=1 + shift 2 + ;; --) shift CODEX_ARGS=("$@") @@ -87,7 +106,14 @@ case "$CODEX_COMMAND" in ;; esac -PROJECT="$(cd "$PROJECT" && pwd)" +PROJECT="$(cd "$PROJECT" && pwd -P)" + +# A no-scope monitor can be launched from a scoped app-server tool shell. Do not +# let that outer server key/URL turn this launcher's children into scoped peers. +# Explicit scoped mode keeps caller-provided state until it selects its own key. +if [ -z "$INVOCATION_SCOPE" ]; then + unset AGMSG_CODEX_APP_SERVER_KEY AGMSG_CODEX_BRIDGE_APP_SERVER +fi # Fail-open: never let a broken bridge block codex. If the agmsg app-server can't # be brought up — e.g. a codex release changes the app-server interface and the @@ -103,6 +129,16 @@ PROJECT="$(cd "$PROJECT" && pwd)" exec_plain_codex() { echo "agmsg: Codex monitor bridge unavailable - launching plain Codex. Real-time agmsg delivery is OFF this session (messages still queue; check your inbox manually). Likely cause: the Codex app-server interface changed in 0.142+. Fix in progress." >&2 cd "$PROJECT" 2>/dev/null || true + if [ -n "$INVOCATION_SCOPE" ]; then + case "$CODEX_COMMAND" in + codex) "$REAL_CODEX" ${CODEX_ARGS[@]+"${CODEX_ARGS[@]}"} <&0 & ;; + resume) "$REAL_CODEX" resume ${CODEX_ARGS[@]+"${CODEX_ARGS[@]}"} <&0 & ;; + esac + tui_bg=$! + if wait "$tui_bg"; then codex_status=0; else codex_status=$?; fi + cleanup_scoped_invocation + exit "$codex_status" + fi case "$CODEX_COMMAND" in codex) exec "$REAL_CODEX" ${CODEX_ARGS[@]+"${CODEX_ARGS[@]}"} ;; resume) exec "$REAL_CODEX" resume ${CODEX_ARGS[@]+"${CODEX_ARGS[@]}"} ;; @@ -110,15 +146,90 @@ exec_plain_codex() { } PROJECT_HASH="$(printf '%s' "$PROJECT" | agmsg_sha1)" -SERVER_LOG="$RUN_DIR/codex-app-server.$PROJECT_HASH.log" -SERVER_PID="$RUN_DIR/codex-app-server.$PROJECT_HASH.pid" -PORT_FILE="$RUN_DIR/codex-app-server.$PROJECT_HASH.port" +APP_SERVER_KEY="$PROJECT_HASH" +if [ -n "$INVOCATION_SCOPE" ]; then + APP_SERVER_KEY="$(printf '%s\n%s' "$PROJECT" "$INVOCATION_SCOPE" | agmsg_sha1)" + export AGMSG_CODEX_APP_SERVER_KEY="$APP_SERVER_KEY" +fi +SERVER_LOG="$RUN_DIR/codex-app-server.$APP_SERVER_KEY.log" +SERVER_PID="$RUN_DIR/codex-app-server.$APP_SERVER_KEY.pid" +PORT_FILE="$RUN_DIR/codex-app-server.$APP_SERVER_KEY.port" # Records the codex version that launched the reusable app-server. A TUI from a # newer/older codex can't speak to an app-server from a different build, so a # stale server left running across a codex upgrade must not be reused. -VERSION_FILE="$RUN_DIR/codex-app-server.$PROJECT_HASH.version" +VERSION_FILE="$RUN_DIR/codex-app-server.$APP_SERVER_KEY.version" +server_bg="" +launcher_bg="" +tui_bg="" +SCOPED_LEASE_RESOURCE="" +scoped_cleanup_done=0 +scoped_lease_held=0 + +scoped_job_is_running() { + local child_pid="$1" job_pid + [ -n "$child_pid" ] || return 1 + for job_pid in $(jobs -pr); do + [ "$job_pid" = "$child_pid" ] && return 0 + done + return 1 +} + +cleanup_scoped_invocation() { + [ -n "${INVOCATION_SCOPE:-}" ] || return 0 + [ "$scoped_cleanup_done" -eq 0 ] || return 0 + + local child_pid recorded_server_pid + for child_pid in "$server_bg" "$launcher_bg"; do + if scoped_job_is_running "$child_pid"; then + kill "$child_pid" 2>/dev/null || true + fi + done + for child_pid in "$server_bg" "$launcher_bg"; do + [ -n "$child_pid" ] || continue + wait "$child_pid" 2>/dev/null || true + done + + recorded_server_pid="$(cat "$SERVER_PID" 2>/dev/null || true)" + if [ -n "$server_bg" ] && [ "$recorded_server_pid" = "$server_bg" ]; then + rm -f "$SERVER_LOG" "$SERVER_PID" "$PORT_FILE" "$VERSION_FILE" 2>/dev/null || true + fi + + if [ "$scoped_lease_held" -eq 1 ]; then + scoped_lease_held=0 + agmsg_runtime_lock_release "$SCOPED_LEASE_RESOURCE" "$$" || true + fi + scoped_cleanup_done=1 +} + +terminate_scoped_invocation() { + local signal_status="$1" + if scoped_job_is_running "$tui_bg"; then + kill "$tui_bg" 2>/dev/null || true + fi + [ -z "$tui_bg" ] || wait "$tui_bg" 2>/dev/null || true + cleanup_scoped_invocation + exit "$signal_status" +} + CODEX_VERSION="$("$REAL_CODEX" --version 2>/dev/null || true)" +if [ -n "$INVOCATION_SCOPE" ]; then + SCOPED_LEASE_RESOURCE="codex-app-server:$APP_SERVER_KEY" + lock_owner="$(agmsg_runtime_lock_acquire "$SCOPED_LEASE_RESOURCE" "$$" 2>/dev/null || true)" + if [ "$lock_owner" != "$$" ] && [ -n "$lock_owner" ] && ! _agmsg_pid_alive_local "$lock_owner"; then + lock_owner="$(agmsg_runtime_lock_acquire "$SCOPED_LEASE_RESOURCE" "$$" "$lock_owner" 2>/dev/null || true)" + fi + if [ "$lock_owner" != "$$" ]; then + echo "codex-monitor: invocation scope is already active" >&2 + exit 1 + fi + scoped_lease_held=1 + trap cleanup_scoped_invocation EXIT + # A direct TERM to the scoped supervisor is part of the lifecycle contract: + # stop/reap its captured TUI first, drain its other children, and return 143. + trap 'terminate_scoped_invocation 143' TERM +fi + mkdir -p "$RUN_DIR" # codex 0.141+ accepts only ws:// (not unix://) for the TUI's --remote, so the @@ -129,7 +240,11 @@ port_alive() { # $1 = port; succeeds if something is accepting on 127.0.0.1:$1 } PORT="" -if [ -f "$PORT_FILE" ] && [ -f "$SERVER_PID" ]; then +if [ -n "$INVOCATION_SCOPE" ]; then + # Scoped records identify a previous invocation, never a process this launch + # may reuse or signal. A later lifecycle task owns only its captured children. + rm -f "$PORT_FILE" "$SERVER_PID" "$VERSION_FILE" +elif [ -f "$PORT_FILE" ] && [ -f "$SERVER_PID" ]; then existing_port="$(cat "$PORT_FILE" 2>/dev/null || true)" existing_pid="$(cat "$SERVER_PID" 2>/dev/null || true)" existing_version="$(cat "$VERSION_FILE" 2>/dev/null || true)" @@ -211,8 +326,10 @@ if [ -z "$PORT" ]; then if [ -z "$PORT" ]; then echo "codex-monitor: app-server did not report a listening port; starting codex without the agmsg bridge" >&2 echo "codex-monitor: see $SERVER_LOG" >&2 - kill "$server_bg" 2>/dev/null || true - rm -f "$SERVER_PID" "$VERSION_FILE" + if [ -z "$INVOCATION_SCOPE" ]; then + kill "$server_bg" 2>/dev/null || true + rm -f "$SERVER_PID" "$VERSION_FILE" + fi exec_plain_codex fi agmsg_write_atomic "$PORT_FILE" "$PORT" @@ -235,13 +352,28 @@ export AGMSG_CODEX_BRIDGE_APP_SERVER="$SOCKET_URL" export AGMSG_CODEX_BRIDGE_LAUNCHER=1 launcher_cmd="${AGMSG_CODEX_BRIDGE_LAUNCHER_CMD:-$SCRIPT_DIR/codex-bridge-launcher.sh}" -# Same guard: the launcher is detached on purpose and outlives this script, so -# an inherited fd 3 would outlive the test file that started it. +# Same guard: the launcher outlives an unscoped monitor, so an inherited fd 3 +# would outlive the test file that started it. "$launcher_cmd" codex "$PROJECT" "$SOCKET_URL" "$$" >/dev/null 2>&1 3>&- 4>&- & +launcher_bg="$!" cd "$PROJECT" # Guard the array expansion: under bash 3.2 + `set -u`, "${CODEX_ARGS[@]}" on an # empty array errors with "unbound variable" (a no-arg `codex`/`codex resume`). +if [ -n "$INVOCATION_SCOPE" ]; then + case "$CODEX_COMMAND" in + codex) + "$REAL_CODEX" --remote "$SOCKET_URL" ${CODEX_ARGS[@]+"${CODEX_ARGS[@]}"} <&0 3>&- 4>&- & + ;; + resume) + "$REAL_CODEX" resume --remote "$SOCKET_URL" ${CODEX_ARGS[@]+"${CODEX_ARGS[@]}"} <&0 3>&- 4>&- & + ;; + esac + tui_bg=$! + if wait "$tui_bg"; then codex_status=0; else codex_status=$?; fi + cleanup_scoped_invocation + exit "$codex_status" +fi case "$CODEX_COMMAND" in codex) exec "$REAL_CODEX" --remote "$SOCKET_URL" ${CODEX_ARGS[@]+"${CODEX_ARGS[@]}"} diff --git a/tests/test_codex_bridge_launcher.bats b/tests/test_codex_bridge_launcher.bats index f5704472a..ca9ad3a6d 100644 --- a/tests/test_codex_bridge_launcher.bats +++ b/tests/test_codex_bridge_launcher.bats @@ -230,6 +230,206 @@ run_launcher() { wait "$parent_b" 2>/dev/null || true } +@test "launcher: a losing legacy dispatcher exits instead of standing by" { + bash "$SCRIPTS/leave.sh" team alice >/dev/null + local hash dispatcher_resource owner="" gone=0 i + hash=$(printf '%s' "$PROJ" | bash -c 'source "$1"; agmsg_sha1' _ "$SCRIPTS/lib/hash.sh") + dispatcher_resource="codex-dispatcher:$hash" + sleep 6 3>&- & local parent_a=$! + sleep 6 3>&- & local parent_b=$! + + env -u AGMSG_CODEX_APP_SERVER_KEY \ + bash "$LAUNCHER" codex "$PROJ" "ws://127.0.0.1:1" "$parent_a" >/dev/null 2>&1 3>&- & + local launcher_a=$! + for i in {1..50}; do + owner="$(sqlite3 "$TEST_SKILL_DIR/db/messages.db" \ + "SELECT owner_pid FROM locks WHERE resource='$dispatcher_resource';" 2>/dev/null || true)" + [ "$owner" = "$launcher_a" ] && break + sleep 0.1 + done + [ "$owner" = "$launcher_a" ] + + env -u AGMSG_CODEX_APP_SERVER_KEY \ + bash "$LAUNCHER" codex "$PROJ" "ws://127.0.0.1:1" "$parent_b" >/dev/null 2>&1 3>&- & + local launcher_b=$! + for i in {1..20}; do + jobs >/dev/null 2>&1 || true + if _pid_gone "$launcher_b"; then + gone=1 + break + fi + sleep 0.05 + done + + kill "$launcher_a" "$launcher_b" "$parent_a" "$parent_b" 2>/dev/null || true + wait "$launcher_a" 2>/dev/null || true + wait "$launcher_b" 2>/dev/null || true + wait "$parent_a" 2>/dev/null || true + wait "$parent_b" 2>/dev/null || true + [ "$gone" -eq 1 ] +} + +@test "launcher: standby dispatcher takes over after the first scoped server exits" { + # Hold A's bridge open after A's server dies. That keeps A's role child lock + # occupied while B takes the dispatcher and exercises the real handoff race. + export AGMSG_CODEX_BRIDGE_CMD="$SCRIPTS/drivers/types/codex/codex-bridge.js" + export AGMSG_TEST_RELEASE_A="$TEST_SKILL_DIR/release-a" + export AGMSG_TEST_RELEASE_B="$TEST_SKILL_DIR/release-b" + cat > "$AGMSG_CODEX_BRIDGE_CMD" <<'EOF' +#!/usr/bin/env bash +printf '%s\n' "$*" >> "$CAPTURE" +case "$*" in + *ws://127.0.0.1:1111*) release="$AGMSG_TEST_RELEASE_A" ;; + *ws://127.0.0.1:2222*) release="$AGMSG_TEST_RELEASE_B" ;; + *) exit 1 ;; +esac +while [ ! -f "$release" ]; do sleep 0.1; done +EOF + chmod +x "$AGMSG_CODEX_BRIDGE_CMD" + put_record team alice thread-alice "$PROJ" codex + + local hash pair_hash dispatcher_resource child_resource owner i + local bridge_a_pid child_a_pid child_b_pid bridge_b_pid child_barrier candidate + hash=$(printf '%s' "$PROJ" | bash -c 'source "$1"; agmsg_sha1' _ "$SCRIPTS/lib/hash.sh") + pair_hash=$(printf '%s' $'team\talice' | bash -c 'source "$1"; agmsg_sha1' _ "$SCRIPTS/lib/hash.sh") + dispatcher_resource="codex-dispatcher:$hash" + child_resource="codex-child:$hash:$pair_hash" + + sleep 30 3>&- & local parent_a=$! + sleep 30 3>&- & local parent_b=$! + sleep 30 3>&- & local lifetime_a=$! + sleep 30 3>&- & local lifetime_b=$! + printf '%s\n' "$lifetime_a" > "$RUN_DIR/codex-app-server.scope-a.pid" + printf '%s\n' "$lifetime_b" > "$RUN_DIR/codex-app-server.scope-b.pid" + + AGMSG_CODEX_APP_SERVER_KEY=scope-a \ + bash "$LAUNCHER" codex "$PROJ" "ws://127.0.0.1:1111" "$parent_a" >/dev/null 2>&1 3>&- & + local launcher_a=$! + owner="" + for i in {1..80}; do + owner="$(sqlite3 "$TEST_SKILL_DIR/db/messages.db" \ + "SELECT owner_pid FROM locks WHERE resource='$dispatcher_resource';" 2>/dev/null || true)" + [ "$owner" = "$launcher_a" ] && break + sleep 0.1 + done + [ "$owner" = "$launcher_a" ] + for i in {1..80}; do + child_a_pid="$(sqlite3 "$TEST_SKILL_DIR/db/messages.db" \ + "SELECT owner_pid FROM locks WHERE resource='$child_resource';" 2>/dev/null || true)" + [ -n "$child_a_pid" ] && [ -f "$RUN_DIR/codex-bridge.team.alice.pid" ] && break + sleep 0.1 + done + [ -n "$child_a_pid" ] + bridge_a_pid="$(cat "$RUN_DIR/codex-bridge.team.alice.pid")" + kill -0 "$child_a_pid" + kill -0 "$bridge_a_pid" + grep -q -- '--app-server ws://127.0.0.1:1111' "$CAPTURE" + + local standby_barrier="$TEST_SKILL_DIR/standby-observed" + AGMSG_CODEX_APP_SERVER_KEY=scope-b \ + AGMSG_TEST_LOCK_STANDBY_BARRIER="$standby_barrier" \ + bash "$LAUNCHER" codex "$PROJ" "ws://127.0.0.1:2222" "$parent_b" >/dev/null 2>&1 3>&- & + local launcher_b=$! + for i in {1..80}; do + [ -f "$standby_barrier.$launcher_b" ] && break + sleep 0.1 + done + [ "$(cat "$standby_barrier.$launcher_b" 2>/dev/null)" = "$dispatcher_resource" ] + [ "$(sqlite3 "$TEST_SKILL_DIR/db/messages.db" \ + "SELECT owner_pid FROM locks WHERE resource='$dispatcher_resource';")" = "$launcher_a" ] + + kill "$lifetime_a" 2>/dev/null || true + wait "$lifetime_a" 2>/dev/null || true + owner="" + for i in {1..100}; do + owner="$(sqlite3 "$TEST_SKILL_DIR/db/messages.db" \ + "SELECT owner_pid FROM locks WHERE resource='$dispatcher_resource';" 2>/dev/null || true)" + [ "$owner" = "$launcher_b" ] && break + sleep 0.1 + done + [ "$owner" = "$launcher_b" ] + # A's custom bridge is still blocked, so its role child still owns the child + # lock when B has already become dispatcher. + [ "$(sqlite3 "$TEST_SKILL_DIR/db/messages.db" \ + "SELECT owner_pid FROM locks WHERE resource='$child_resource';")" = "$child_a_pid" ] + child_barrier="" + for i in {1..100}; do + for candidate in "$standby_barrier".*; do + [ -f "$candidate" ] || continue + if [ "$(cat "$candidate")" = "$child_resource" ]; then + child_barrier="$candidate" + break 2 + fi + done + sleep 0.1 + done + [ -n "$child_barrier" ] + child_b_pid="${child_barrier##*.}" + kill -0 "$child_b_pid" + + : > "$AGMSG_TEST_RELEASE_A" + for i in {1..120}; do + owner="$(sqlite3 "$TEST_SKILL_DIR/db/messages.db" \ + "SELECT owner_pid FROM locks WHERE resource='$child_resource';" 2>/dev/null || true)" + [ "$owner" = "$child_b_pid" ] && [ -f "$RUN_DIR/codex-bridge.team.alice.pid" ] && break + sleep 0.1 + done + [ "$owner" = "$child_b_pid" ] + bridge_b_pid="$(cat "$RUN_DIR/codex-bridge.team.alice.pid")" + [ "$bridge_b_pid" != "$bridge_a_pid" ] + refute kill -0 "$bridge_a_pid" 2>/dev/null + kill -0 "$bridge_b_pid" + grep -q -- '--app-server ws://127.0.0.1:2222' "$CAPTURE" + # Stability interval: one unchanged B bridge pid and one unchanged B child + # lock owner, not merely a historical capture line. + sleep 1 + [ "$(cat "$RUN_DIR/codex-bridge.team.alice.pid")" = "$bridge_b_pid" ] + kill -0 "$bridge_b_pid" + [ "$(sqlite3 "$TEST_SKILL_DIR/db/messages.db" \ + "SELECT owner_pid FROM locks WHERE resource='$child_resource';")" = "$child_b_pid" ] + + : > "$AGMSG_TEST_RELEASE_B" + kill "$lifetime_b" "$launcher_a" "$launcher_b" "$parent_a" "$parent_b" 2>/dev/null || true + wait "$lifetime_b" 2>/dev/null || true + wait "$launcher_a" 2>/dev/null || true + wait "$launcher_b" 2>/dev/null || true + wait "$parent_a" 2>/dev/null || true + wait "$parent_b" 2>/dev/null || true +} + +@test "launcher: exact scoped server lifetime rejects a missing pid record" { + sleep 5 3>&- & local parent=$! + AGMSG_CODEX_APP_SERVER_KEY=missing-scope \ + bash "$LAUNCHER" codex "$PROJ" "ws://127.0.0.1:2222" "$parent" >/dev/null 2>&1 3>&- & + local launcher=$! i + for i in {1..30}; do + kill -0 "$launcher" 2>/dev/null || break + sleep 0.05 + done + refute kill -0 "$launcher" 2>/dev/null + kill "$parent" 2>/dev/null || true + wait "$launcher" 2>/dev/null || true + wait "$parent" 2>/dev/null || true +} + +@test "launcher: exact scoped server lifetime rejects a dead pid record" { + sleep 0.1 3>&- & local dead=$! + wait "$dead" + printf '%s\n' "$dead" > "$RUN_DIR/codex-app-server.dead-scope.pid" + sleep 5 3>&- & local parent=$! + AGMSG_CODEX_APP_SERVER_KEY=dead-scope \ + bash "$LAUNCHER" codex "$PROJ" "ws://127.0.0.1:2222" "$parent" >/dev/null 2>&1 3>&- & + local launcher=$! i + for i in {1..30}; do + kill -0 "$launcher" 2>/dev/null || break + sleep 0.05 + done + refute kill -0 "$launcher" 2>/dev/null + kill "$parent" 2>/dev/null || true + wait "$launcher" 2>/dev/null || true + wait "$parent" 2>/dev/null || true +} + @test "launcher: stale dispatcher reclamation remains singleton under contention" { put_record team alice thread-alice "$PROJ" codex export MOCK_BRIDGE_SLEEP=8 diff --git a/tests/test_codex_monitor.bats b/tests/test_codex_monitor.bats index ebf4be667..60b3337ae 100644 --- a/tests/test_codex_monitor.bats +++ b/tests/test_codex_monitor.bats @@ -4,8 +4,11 @@ load test_helper setup() { setup_test_env - export TEST_PROJECT="$(mktemp -d)" + export TEST_PROJECT="$(cd "$(mktemp -d)" && pwd -P)" export CALL_LOG="$TEST_PROJECT/calls.log" + export TEST_MONITOR_PID="" + export TEST_TUI_RELEASE_FILE="" + export TEST_TUI_EXIT_FILE="" # Fake codex for codex-monitor tests. # --version -> prints "codex-cli $FAKE_CODEX_VERSION" @@ -28,10 +31,26 @@ case "${1:-}" in echo "error: unexpected argument '--listen' found" >&2 exit 2 fi + [ -z "${AGMSG_TEST_APP_SERVER_KEY_LOG:-}" ] || printf '%s' "${AGMSG_CODEX_APP_SERVER_KEY:-}" > "$AGMSG_TEST_APP_SERVER_KEY_LOG" + [ -z "${AGMSG_TEST_APP_SERVER_INHERITED_URL_LOG:-}" ] || printf '%s' "${AGMSG_CODEX_BRIDGE_APP_SERVER:-}" > "$AGMSG_TEST_APP_SERVER_INHERITED_URL_LOG" + if [ -n "${AGMSG_TEST_APP_SERVER_URL_LOG:-}" ]; then + ( + SKILL_DIR="$TEST_SKILL_DIR" + source "$SKILL_DIR/scripts/lib/hash.sh" + source "$SKILL_DIR/scripts/drivers/types/codex/_app-server.sh" + nested_url="" + for _probe in $(seq 1 100); do + nested_url="$(_agmsg_codex_app_server_url "$AGMSG_TEST_APP_SERVER_PROJECT")" + [ -n "$nested_url" ] && break + sleep 0.05 + done + printf '%s' "$nested_url" > "$AGMSG_TEST_APP_SERVER_URL_LOG" + ) & + fi # Run the listener as a CHILD (no exec) so this script stays the recorded pid; # its argv ("...real-codex app-server --listen") is what codex-monitor's - # cmdline check matches. The child exits when this parent is killed. - python3 - <<'PY' + # cmdline check matches. + python3 - <<'PY' & import socket, sys, os s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) @@ -47,11 +66,33 @@ while True: except Exception: pass PY + child=$! + record_server_term() { + [ -z "${FAKE_TERM_LOG:-}" ] || printf 'server TERM\n' >> "$FAKE_TERM_LOG" + kill "$child" 2>/dev/null || true + wait "$child" 2>/dev/null || true + exit 0 + } + trap record_server_term TERM + [ -z "${FAKE_SERVER_READY_FILE:-}" ] || printf '%s' "$$" > "$FAKE_SERVER_READY_FILE" + wait "$child" ;; *) + record_tui_term() { + [ -z "${FAKE_TUI_TERM_LOG:-}" ] || printf 'tui TERM\n' > "$FAKE_TUI_TERM_LOG" + [ -z "${FAKE_TUI_EXIT_MARKER:-}" ] || : > "$FAKE_TUI_EXIT_MARKER" + exit 143 + } + trap record_tui_term TERM + [ -z "${FAKE_TUI_PID_FILE:-}" ] || printf '%s' "$$" > "$FAKE_TUI_PID_FILE" + [ -z "${AGMSG_TEST_TUI_CWD_LOG:-}" ] || printf '%s\n' "$PWD" >> "$AGMSG_TEST_TUI_CWD_LOG" printf 'plain-codex' >> "$CALL_LOG" for a in "$@"; do printf ' <%s>' "$a" >> "$CALL_LOG"; done printf '\n' >> "$CALL_LOG" + [ -z "${FAKE_TUI_READY_FILE:-}" ] || printf '%s' "$$" > "$FAKE_TUI_READY_FILE" + while [ -n "${FAKE_TUI_GATE:-}" ] && [ ! -e "$FAKE_TUI_GATE" ]; do sleep 0.1; done + [ -z "${FAKE_TUI_EXIT_MARKER:-}" ] || : > "$FAKE_TUI_EXIT_MARKER" + exit "${FAKE_TUI_STATUS:-0}" ;; esac EOF @@ -65,6 +106,12 @@ teardown() { # process inside it is alive, so the rm below fails with "Directory not # empty" and the test reports a failure whose assertions all passed. local pf pid + if [ -n "${TEST_MONITOR_PID:-}" ]; then + kill "$TEST_MONITOR_PID" 2>/dev/null || true + wait "$TEST_MONITOR_PID" 2>/dev/null || true + fi + [ -z "${TEST_TUI_RELEASE_FILE:-}" ] || : > "$TEST_TUI_RELEASE_FILE" + [ -z "${TEST_TUI_EXIT_FILE:-}" ] || wait_for_file "$TEST_TUI_EXIT_FILE" || true for pf in "$TEST_SKILL_DIR"/run/codex-app-server.*.pid; do [ -f "$pf" ] || continue pid="$(cat "$pf" 2>/dev/null)" @@ -76,6 +123,21 @@ teardown() { teardown_test_env } +_make_term_recording_launcher() { + local launcher="$1" + cat > "$launcher" <<'EOF' +#!/usr/bin/env bash +record_launcher_term() { + printf 'launcher TERM\n' > "$FAKE_LAUNCHER_TERM_LOG" + exit 0 +} +trap record_launcher_term TERM +printf '%s' "$$" > "$FAKE_LAUNCHER_READY_FILE" +while :; do sleep 0.1; done +EOF + chmod +x "$launcher" +} + # --- fail-open (A) --- @test "codex-monitor: fails open to plain codex when the app-server won't start (#170)" { @@ -136,6 +198,460 @@ teardown() { [ "$(cat "$pidf")" = "$first_pid" ] } +@test "codex-monitor: invocation scope is consumed before Codex argv" { + local key_log="$TEST_PROJECT/app-server-key" + local inherited_url_log="$TEST_PROJECT/inherited-app-server-url" + run env AGMSG_CODEX_BRIDGE_APP_SERVER=ws://127.0.0.1:3333 \ + AGMSG_TEST_APP_SERVER_KEY_LOG="$key_log" \ + AGMSG_TEST_APP_SERVER_INHERITED_URL_LOG="$inherited_url_log" \ + AGMSG_REAL_CODEX="$FAKE_CODEX" \ + bash "$TYPES/codex/codex-monitor.sh" \ + --project "$TEST_PROJECT" --invocation-scope scope-A --codex-command codex -- --foo + [ "$status" -eq 0 ] + grep -Eq 'plain-codex <--remote> <--foo>' "$CALL_LOG" + refute grep -q -- '--invocation-scope' "$CALL_LOG" + local expected_key + expected_key="$(printf '%s\n%s' "$(cd "$TEST_PROJECT" && pwd)" scope-A | ( . "$SCRIPTS/lib/hash.sh"; agmsg_sha1 ))" + [ "$(cat "$key_log")" = "$expected_key" ] + [ "$(cat "$inherited_url_log")" = "ws://127.0.0.1:3333" ] +} + +@test "codex-monitor: malformed and duplicate scopes fail before launch" { + run env AGMSG_REAL_CODEX="$FAKE_CODEX" bash "$TYPES/codex/codex-monitor.sh" \ + --project "$TEST_PROJECT" --invocation-scope + [ "$status" -eq 2 ] + run env AGMSG_REAL_CODEX="$FAKE_CODEX" bash "$TYPES/codex/codex-monitor.sh" \ + --project "$TEST_PROJECT" --invocation-scope bad/scope --codex-command codex -- + [ "$status" -eq 2 ] + run env AGMSG_REAL_CODEX="$FAKE_CODEX" bash "$TYPES/codex/codex-monitor.sh" \ + --project "$TEST_PROJECT" --invocation-scope one --invocation-scope two \ + --codex-command codex -- + [ "$status" -eq 2 ] + [ ! -e "$CALL_LOG" ] +} + +@test "codex-monitor: help distinguishes legacy reuse from scoped supervision" { + run bash "$TYPES/codex/codex-monitor.sh" --help + [ "$status" -eq 0 ] + grep -Fq -- "--invocation-scope TOKEN" <<< "$output" + grep -Fq -- "Without --invocation-scope, starts/reuses" <<< "$output" + grep -Fq -- "then execs" <<< "$output" + grep -Fq -- "Scoped mode waits for its captured TUI, app-server, and bridge launcher processes." <<< "$output" +} + +@test "codex-monitor: physical project aliases share scoped leases and one dispatcher" { + skip_on_windows "requires POSIX symlink and process semantics" + + local physical="$TEST_PROJECT/physical-project" + local project_alias="$TEST_PROJECT/project-alias" + local gate_a="$TEST_PROJECT/release-a" gate_b="$TEST_PROJECT/release-b" + local cwd_log="$TEST_PROJECT/tui-cwds" + local project_hash key_a key_b pidfile_a pidfile_b first_pidfile first_pid + local second_pid_log dispatcher_log probe_pid probe_status second_pid dispatcher_rows i + mkdir -p "$physical" + mkdir -p "$TEST_SKILL_DIR/run" + ln -s "$physical" "$project_alias" + physical="$(cd "$physical" && pwd -P)" + project_hash="$(printf '%s' "$physical" | ( . "$SCRIPTS/lib/hash.sh"; agmsg_sha1 ))" + key_a="$(printf '%s\n%s' "$physical" scope-A | ( . "$SCRIPTS/lib/hash.sh"; agmsg_sha1 ))" + key_b="$(printf '%s\n%s' "$physical" scope-B | ( . "$SCRIPTS/lib/hash.sh"; agmsg_sha1 ))" + pidfile_a="$TEST_SKILL_DIR/run/codex-app-server.$key_a.pid" + pidfile_b="$TEST_SKILL_DIR/run/codex-app-server.$key_b.pid" + + env FAKE_TUI_GATE="$gate_a" AGMSG_TEST_TUI_CWD_LOG="$cwd_log" \ + AGMSG_REAL_CODEX="$FAKE_CODEX" \ + bash "$TYPES/codex/codex-monitor.sh" --project "$project_alias" \ + --invocation-scope scope-A --codex-command codex -- & + TEST_MONITOR_PID=$! + first_pidfile="" + for i in $(seq 1 100); do + first_pidfile="$(find "$TEST_SKILL_DIR/run" -name 'codex-app-server.*.pid' -type f | head -1)" + [ -n "$first_pidfile" ] && break + sleep 0.1 + done + [ -n "$first_pidfile" ] + first_pid="$(cat "$first_pidfile")" + + run env AGMSG_REAL_CODEX="$FAKE_CODEX" \ + bash "$TYPES/codex/codex-monitor.sh" --project "$physical" \ + --invocation-scope scope-A --codex-command codex -- + if [ "$status" -eq 0 ]; then + echo "same physical project and scope launched twice through alias paths" >&2 + false + fi + [ "$first_pidfile" = "$pidfile_a" ] + [ "$(cat "$pidfile_a")" = "$first_pid" ] + + second_pid_log="$TEST_PROJECT/scope-b-pid" + dispatcher_log="$TEST_PROJECT/dispatcher-rows" + ( + set -e + trap ': > "$gate_b"' EXIT + wait_for_file "$pidfile_b" + second_pid="$(cat "$pidfile_b")" + [ "$first_pid" != "$second_pid" ] + kill -0 "$first_pid" + kill -0 "$second_pid" + dispatcher_rows="" + for i in $(seq 1 100); do + dispatcher_rows="$(sqlite3 "$TEST_SKILL_DIR/db/messages.db" \ + "SELECT resource FROM locks WHERE resource LIKE 'codex-dispatcher:%' ORDER BY resource;" 2>/dev/null || true)" + [ -n "$dispatcher_rows" ] && break + sleep 0.1 + done + printf '%s' "$second_pid" > "$second_pid_log" + printf '%s' "$dispatcher_rows" > "$dispatcher_log" + ) & + probe_pid=$! + run env FAKE_TUI_GATE="$gate_b" AGMSG_TEST_TUI_CWD_LOG="$cwd_log" \ + AGMSG_REAL_CODEX="$FAKE_CODEX" \ + bash "$TYPES/codex/codex-monitor.sh" --project "$physical" \ + --invocation-scope scope-B --codex-command codex -- + [ "$status" -eq 0 ] + if wait "$probe_pid"; then probe_status=0; else probe_status=$?; fi + [ "$probe_status" -eq 0 ] + second_pid="$(cat "$second_pid_log")" + dispatcher_rows="$(cat "$dispatcher_log")" + [ "$first_pid" != "$second_pid" ] + [ "$dispatcher_rows" = "codex-dispatcher:$project_hash" ] + grep -Fq "$physical" "$physical/.codex/hooks.json" + refute grep -Fq "$project_alias" "$physical/.codex/hooks.json" + + for i in $(seq 1 100); do + [ "$(wc -l < "$cwd_log" 2>/dev/null | tr -d ' ')" -eq 2 ] && break + sleep 0.1 + done + [ "$(wc -l < "$cwd_log" | tr -d ' ')" -eq 2 ] + [ "$(sort -u "$cwd_log")" = "$physical" ] + + : > "$gate_a" + wait "$TEST_MONITOR_PID" + TEST_MONITOR_PID="" +} + +@test "codex-monitor: no-scope server ignores inherited scoped routing" { + skip_on_windows "spawns a python socket listener; flaky on the Windows runner" + + local hash base key_log nested_url_log nested_url own_url + hash="$(printf '%s' "$(cd "$TEST_PROJECT" && pwd -P)" | ( . "$SCRIPTS/lib/hash.sh"; agmsg_sha1 ))" + base="$TEST_SKILL_DIR/run/codex-app-server.$hash" + key_log="$TEST_PROJECT/app-server-key" + nested_url_log="$TEST_PROJECT/nested-app-server-url" + mkdir -p "$TEST_SKILL_DIR/run" + printf '1111' > "$TEST_SKILL_DIR/run/codex-app-server.outer-scope.port" + + run env AGMSG_CODEX_APP_SERVER_KEY=outer-scope \ + AGMSG_CODEX_BRIDGE_APP_SERVER=ws://127.0.0.1:3333 \ + AGMSG_TEST_APP_SERVER_KEY_LOG="$key_log" \ + AGMSG_TEST_APP_SERVER_URL_LOG="$nested_url_log" \ + AGMSG_TEST_APP_SERVER_PROJECT="$TEST_PROJECT" \ + AGMSG_REAL_CODEX="$FAKE_CODEX" \ + bash "$TYPES/codex/codex-monitor.sh" --project "$TEST_PROJECT" --codex-command codex -- + [ "$status" -eq 0 ] + wait_for_file "$nested_url_log" + [ -f "$base.pid" ] + [ ! -e "$TEST_SKILL_DIR/run/codex-app-server.outer-scope.pid" ] + nested_url="$(cat "$nested_url_log")" + own_url="ws://127.0.0.1:$(cat "$base.port")" + if [ "$nested_url" != "$own_url" ]; then + echo "nested no-scope URL=$nested_url; own project URL=$own_url" >&2 + false + fi + [ ! -s "$key_log" ] + grep -Fq "<--remote> <$own_url>" "$CALL_LOG" +} + +@test "codex-monitor: no-scope duplicate dispatcher does not inherit scoped standby" { + skip_on_windows "uses POSIX process and lock-owner semantics" + + local hash resource barrier gate owner sentinel_pid probe_log probe_pid probe_status + local barrier_count observed_owner + hash="$(printf '%s' "$(cd "$TEST_PROJECT" && pwd -P)" | ( . "$SCRIPTS/lib/hash.sh"; agmsg_sha1 ))" + resource="codex-dispatcher:$hash" + barrier="$TEST_PROJECT/standby-observed" + gate="$TEST_PROJECT/release-tui" + mkdir -p "$TEST_SKILL_DIR/run" + sleep 30 3>&- & + TEST_MONITOR_PID=$! + sentinel_pid="$TEST_MONITOR_PID" + owner="$(bash -c 'source "$1"; agmsg_runtime_lock_acquire "$2" "$3"' \ + _ "$SCRIPTS/lib/storage.sh" "$resource" "$sentinel_pid")" + [ "$owner" = "$sentinel_pid" ] + printf '%s' "$sentinel_pid" > "$TEST_SKILL_DIR/run/codex-app-server.outer-scope.pid" + + probe_log="$TEST_PROJECT/dispatcher-probe" + ( + set -e + trap ': > "$gate"' EXIT + wait_for_file_contains "$CALL_LOG" plain-codex + sleep 1 + barrier_count="$(find "$TEST_PROJECT" -maxdepth 1 -name 'standby-observed.*' -type f | wc -l | tr -d ' ')" + [ "$barrier_count" -eq 0 ] + observed_owner="$(bash -c 'source "$1"; agmsg_runtime_lock_owner "$2"' \ + _ "$SCRIPTS/lib/storage.sh" "$resource")" + [ "$observed_owner" = "$sentinel_pid" ] + printf '%s\n%s\n' "$barrier_count" "$observed_owner" > "$probe_log" + ) & + probe_pid=$! + run env AGMSG_CODEX_APP_SERVER_KEY=outer-scope \ + AGMSG_CODEX_BRIDGE_APP_SERVER=ws://127.0.0.1:3333 \ + AGMSG_TEST_LOCK_STANDBY_BARRIER="$barrier" FAKE_TUI_GATE="$gate" \ + AGMSG_REAL_CODEX="$FAKE_CODEX" \ + bash "$TYPES/codex/codex-monitor.sh" --project "$TEST_PROJECT" --codex-command codex -- + [ "$status" -eq 0 ] + if wait "$probe_pid"; then probe_status=0; else probe_status=$?; fi + [ "$probe_status" -eq 0 ] + [ "$(sed -n '1p' "$probe_log")" -eq 0 ] + [ "$(sed -n '2p' "$probe_log")" = "$sentinel_pid" ] + + kill "$sentinel_pid" 2>/dev/null || true + wait "$sentinel_pid" 2>/dev/null || true + TEST_MONITOR_PID="" +} + +@test "codex-monitor: scoped TUI exit preserves status and removes exact artifacts" { + run env FAKE_TUI_STATUS=37 AGMSG_REAL_CODEX="$FAKE_CODEX" \ + bash "$TYPES/codex/codex-monitor.sh" --project "$TEST_PROJECT" \ + --invocation-scope scope-exit --codex-command resume -- --last -C "$TEST_PROJECT" + [ "$status" -eq 37 ] + grep -Eq '^plain-codex <--remote> <--last> <-C>' "$CALL_LOG" + [ "$(find "$TEST_SKILL_DIR/run" -name 'codex-app-server.*' -type f | wc -l | tr -d ' ')" -eq 0 ] +} + +@test "codex-monitor: scoped TUI exit sends TERM to its captured server and launcher children" { + local term_log="$TEST_PROJECT/server-term.log" + local launcher_term_log="$TEST_PROJECT/launcher-term.log" + local launcher="$TEST_PROJECT/fake-launcher" + local gate="$TEST_PROJECT/release-tui" + local tui_exit="$TEST_PROJECT/tui-exit" + local server_ready="$TEST_PROJECT/server-ready" + local launcher_ready="$TEST_PROJECT/launcher-ready" + local tui_ready="$TEST_PROJECT/tui-ready" + _make_term_recording_launcher "$launcher" + TEST_TUI_RELEASE_FILE="$gate" + TEST_TUI_EXIT_FILE="$tui_exit" + + env FAKE_TERM_LOG="$term_log" FAKE_SERVER_READY_FILE="$server_ready" \ + FAKE_LAUNCHER_TERM_LOG="$launcher_term_log" FAKE_LAUNCHER_READY_FILE="$launcher_ready" \ + FAKE_TUI_GATE="$gate" FAKE_TUI_READY_FILE="$tui_ready" FAKE_TUI_EXIT_MARKER="$tui_exit" \ + AGMSG_CODEX_BRIDGE_LAUNCHER_CMD="$launcher" AGMSG_REAL_CODEX="$FAKE_CODEX" \ + bash "$TYPES/codex/codex-monitor.sh" --project "$TEST_PROJECT" \ + --invocation-scope scope-term --codex-command codex -- & + TEST_MONITOR_PID=$! + wait_for_file_contains "$server_ready" '[0-9]' + wait_for_file_contains "$launcher_ready" '[0-9]' + wait_for_file_contains "$tui_ready" '[0-9]' + : > "$gate" + wait "$TEST_MONITOR_PID" + TEST_MONITOR_PID="" + + grep -qx 'server TERM' "$term_log" + grep -qx 'launcher TERM' "$launcher_term_log" +} + +@test "codex-monitor: scoped cleanup failure preserves TUI status and releases lease" { + local stubdir="$TEST_PROJECT/stub-bin" + local marker="$TEST_PROJECT/fail-cleanup-rm" + local real_rm key pidfile returned_status server_pid lock_owner + mkdir -p "$stubdir" + real_rm="$(command -v rm)" + cat > "$stubdir/rm" <}" >&2 + fi + [ "$returned_status:$lock_owner" = "37:" ] +} + +@test "codex-monitor: scoped direct TERM reaps TUI and returns signal status" { + skip_on_windows "uses POSIX direct-child signal semantics" + + local gate="$TEST_PROJECT/hold-tui" + local server_ready="$TEST_PROJECT/server-ready" + local launcher_ready="$TEST_PROJECT/launcher-ready" + local tui_ready="$TEST_PROJECT/tui-ready" + local server_term="$TEST_PROJECT/server-term.log" + local launcher_term="$TEST_PROJECT/launcher-term.log" + local tui_term="$TEST_PROJECT/tui-term.log" + local tui_exit="$TEST_PROJECT/tui-exit" + local tui_pid_file="$TEST_PROJECT/tui.pid" + local launcher="$TEST_PROJECT/fake-launcher" + local key pidfile server_pid launcher_pid tui_pid monitor_status tui_gone lock_owner + _make_term_recording_launcher "$launcher" + TEST_TUI_RELEASE_FILE="$gate" + TEST_TUI_EXIT_FILE="$tui_exit" + + key="$(printf '%s\n%s' "$(cd "$TEST_PROJECT" && pwd)" scope-direct-term | ( . "$SCRIPTS/lib/hash.sh"; agmsg_sha1 ))" + pidfile="$TEST_SKILL_DIR/run/codex-app-server.$key.pid" + env FAKE_TERM_LOG="$server_term" FAKE_SERVER_READY_FILE="$server_ready" \ + FAKE_LAUNCHER_TERM_LOG="$launcher_term" FAKE_LAUNCHER_READY_FILE="$launcher_ready" \ + FAKE_TUI_GATE="$gate" FAKE_TUI_READY_FILE="$tui_ready" FAKE_TUI_EXIT_MARKER="$tui_exit" \ + FAKE_TUI_PID_FILE="$tui_pid_file" FAKE_TUI_TERM_LOG="$tui_term" \ + AGMSG_CODEX_BRIDGE_LAUNCHER_CMD="$launcher" AGMSG_REAL_CODEX="$FAKE_CODEX" \ + bash "$TYPES/codex/codex-monitor.sh" --project "$TEST_PROJECT" \ + --invocation-scope scope-direct-term --codex-command codex -- & + TEST_MONITOR_PID=$! + + wait_for_file_contains "$server_ready" '[0-9]' + wait_for_file_contains "$launcher_ready" '[0-9]' + wait_for_file_contains "$tui_ready" '[0-9]' + server_pid="$(cat "$pidfile")" + launcher_pid="$(cat "$launcher_ready")" + tui_pid="$(cat "$tui_pid_file")" + + kill -TERM "$TEST_MONITOR_PID" + if wait "$TEST_MONITOR_PID"; then monitor_status=0; else monitor_status=$?; fi + TEST_MONITOR_PID="" + + tui_gone=1 + if ! wait_for_pid_exit "$tui_pid"; then + tui_gone=0 + : > "$gate" + wait_for_file "$tui_exit" || true + wait_for_pid_exit "$tui_pid" || true + fi + lock_owner="$(. "$SCRIPTS/lib/storage.sh"; agmsg_runtime_lock_owner "codex-app-server:$key")" + + [ "$monitor_status" -eq 143 ] + if [ "$tui_gone" -ne 1 ]; then + echo "signal observation: status=$monitor_status tui_gone=$tui_gone" >&2 + fi + [ "$tui_gone" -eq 1 ] + wait_for_pid_exit "$server_pid" + wait_for_pid_exit "$launcher_pid" + grep -qx 'tui TERM' "$tui_term" + grep -qx 'server TERM' "$server_term" + grep -qx 'launcher TERM' "$launcher_term" + [ "$(find "$TEST_SKILL_DIR/run" -name 'codex-app-server.*' -type f | wc -l | tr -d ' ')" -eq 0 ] + [ -z "$lock_owner" ] +} + +@test "codex-monitor: scoped fail-open exit preserves status and argv" { + run env FAKE_CODEX_MODE=broken FAKE_TUI_STATUS=37 AGMSG_REAL_CODEX="$FAKE_CODEX" \ + bash "$TYPES/codex/codex-monitor.sh" --project "$TEST_PROJECT" \ + --invocation-scope scope-fail-open --codex-command resume -- --last -C "$TEST_PROJECT" + [ "$status" -eq 37 ] + grep -Eq '^plain-codex <--last> <-C>' "$CALL_LOG" + refute grep -q -- '--remote' "$CALL_LOG" + printf '%s\n' "$output" | grep -Fq 'Real-time agmsg delivery is OFF' + [ "$(find "$TEST_SKILL_DIR/run" -name 'codex-app-server.*' -type f | wc -l | tr -d ' ')" -eq 0 ] +} + +@test "codex-monitor: scoped cleanup never signals a foreign pidfile target" { + skip_on_windows "spawns a python socket listener; flaky on the Windows runner" + + local gate="$TEST_PROJECT/release-tui" + local key pidfile server_pid sentinel_pid monitor_pid + key="$(printf '%s\n%s' "$(cd "$TEST_PROJECT" && pwd)" scope-foreign | ( . "$SCRIPTS/lib/hash.sh"; agmsg_sha1 ))" + pidfile="$TEST_SKILL_DIR/run/codex-app-server.$key.pid" + + env FAKE_TUI_GATE="$gate" AGMSG_REAL_CODEX="$FAKE_CODEX" \ + bash "$TYPES/codex/codex-monitor.sh" --project "$TEST_PROJECT" \ + --invocation-scope scope-foreign --codex-command codex -- & + monitor_pid=$! + wait_for_file "$pidfile" + wait_for_file_contains "$CALL_LOG" plain-codex + server_pid="$(cat "$pidfile")" + + sleep 60 & + sentinel_pid=$! + printf '%s' "$sentinel_pid" > "$pidfile" + : > "$gate" + wait "$monitor_pid" + + wait_for_pid_exit "$server_pid" + kill -0 "$sentinel_pid" + [ "$(cat "$pidfile")" = "$sentinel_pid" ] + kill "$sentinel_pid" 2>/dev/null || true + wait "$sentinel_pid" 2>/dev/null || true +} + +@test "codex-monitor: different invocation scopes use different live app-servers" { + skip_on_windows "spawns python socket listeners; flaky on the Windows runner" + + local gate_a="$TEST_PROJECT/release-tui-a" gate_b="$TEST_PROJECT/release-tui-b" + local key_a key_b pidfile_a pidfile_b + key_a="$(printf '%s\n%s' "$(cd "$TEST_PROJECT" && pwd)" scope-A | ( . "$SCRIPTS/lib/hash.sh"; agmsg_sha1 ))" + key_b="$(printf '%s\n%s' "$(cd "$TEST_PROJECT" && pwd)" scope-B | ( . "$SCRIPTS/lib/hash.sh"; agmsg_sha1 ))" + pidfile_a="$TEST_SKILL_DIR/run/codex-app-server.$key_a.pid" + pidfile_b="$TEST_SKILL_DIR/run/codex-app-server.$key_b.pid" + + env FAKE_TUI_GATE="$gate_a" AGMSG_REAL_CODEX="$FAKE_CODEX" \ + bash "$TYPES/codex/codex-monitor.sh" --project "$TEST_PROJECT" \ + --invocation-scope scope-A --codex-command codex -- & + local scope_a_monitor=$! + + wait_for_file "$pidfile_a" + local scope_a_server="$(cat "$pidfile_a")" + + env FAKE_TUI_GATE="$gate_b" AGMSG_REAL_CODEX="$FAKE_CODEX" \ + bash "$TYPES/codex/codex-monitor.sh" --project "$TEST_PROJECT" \ + --invocation-scope scope-B --codex-command codex -- & + local scope_b_monitor=$! + + wait_for_file "$pidfile_b" + local scope_b_server="$(cat "$pidfile_b")" + [ "$scope_a_server" != "$scope_b_server" ] + kill -0 "$scope_a_server" + kill -0 "$scope_b_server" + + : > "$gate_a" + wait "$scope_a_monitor" + wait_for_pid_exit "$scope_a_server" + kill -0 "$scope_b_monitor" + kill -0 "$scope_b_server" + + : > "$gate_b" + wait "$scope_b_monitor" + wait_for_pid_exit "$scope_b_server" +} + +@test "codex-monitor: same live invocation scope fails without changing its server" { + skip_on_windows "spawns a python socket listener; flaky on the Windows runner" + + local gate="$TEST_PROJECT/release-tui" + env FAKE_TUI_GATE="$gate" AGMSG_REAL_CODEX="$FAKE_CODEX" \ + bash "$TYPES/codex/codex-monitor.sh" --project "$TEST_PROJECT" \ + --invocation-scope scope-A --codex-command codex -- & + local first_monitor=$! + + local i pidfiles + for i in $(seq 1 100); do + pidfiles=("$TEST_SKILL_DIR"/run/codex-app-server.*.pid) + [ -f "${pidfiles[0]}" ] && break + sleep 0.1 + done + [ -f "${pidfiles[0]}" ] + local first_pid="$(cat "${pidfiles[0]}")" + + run env AGMSG_REAL_CODEX="$FAKE_CODEX" bash "$TYPES/codex/codex-monitor.sh" \ + --project "$TEST_PROJECT" --invocation-scope scope-A --codex-command codex -- + [ "$status" -ne 0 ] + [ "$(cat "${pidfiles[0]}")" = "$first_pid" ] + + : > "$gate" + wait "$first_monitor" +} + # --- port discovery vs colorized banner (codex 0.144+) --- @test "codex-monitor: discovers the port when codex colorizes the banner (0.144+)" { diff --git a/tests/test_codex_resume.bats b/tests/test_codex_resume.bats index 4bb27060d..a4a612e65 100644 --- a/tests/test_codex_resume.bats +++ b/tests/test_codex_resume.bats @@ -377,6 +377,64 @@ record_with_loaded_via_port_file() { # [ "$(recorded_uuid team alice)" = "thr-unclaimed" ] } +@test "codex record: scoped server key selects only the scoped port file" { + local proj ids hash fake log + proj="$(mktemp -d)" + ids="$TEST_SKILL_DIR/loaded.txt" + fake="$TEST_SKILL_DIR/logging-node" + log="$TEST_SKILL_DIR/logging-node.args" + printf 'thr-scoped\n' > "$ids" + # shellcheck disable=SC1091 + source "$SKILL_DIR/scripts/lib/hash.sh" + hash="$(printf '%s' "$proj" | agmsg_sha1)" + printf '1111' > "$TEST_SKILL_DIR/run/codex-app-server.$hash.port" + printf '2222' > "$TEST_SKILL_DIR/run/codex-app-server.scoped-key.port" + { + printf '#!/usr/bin/env bash\n' + printf 'printf "%%s\\n" "$*" > %q\n' "$log" + printf 'cat %q\n' "$ids" + } > "$fake" + chmod +x "$fake" + + ( unset CODEX_THREAD_ID + AGMSG_CODEX_APP_SERVER_KEY=scoped-key \ + AGMSG_CODEX_BRIDGE_APP_SERVER=ws://127.0.0.1:3333 \ + AGMSG_NODE="$fake" \ + bash "$TYPES/codex/codex-record-session.sh" team alice "$proj" ) + + [ "$(recorded_uuid team alice)" = "thr-scoped" ] + grep -q -- '--app-server ws://127.0.0.1:2222' "$log" + refute grep -q -- 'ws://127.0.0.1:1111' "$log" + ! grep -q -- 'ws://127.0.0.1:3333' "$log" +} + +@test "codex record: no scoped server key keeps explicit app-server URL precedence" { + local proj ids hash fake log + proj="$(mktemp -d)" + ids="$TEST_SKILL_DIR/loaded.txt" + fake="$TEST_SKILL_DIR/logging-node" + log="$TEST_SKILL_DIR/logging-node.args" + printf 'thr-legacy\n' > "$ids" + # shellcheck disable=SC1091 + source "$SKILL_DIR/scripts/lib/hash.sh" + hash="$(printf '%s' "$proj" | agmsg_sha1)" + printf '2222' > "$TEST_SKILL_DIR/run/codex-app-server.$hash.port" + { + printf '#!/usr/bin/env bash\n' + printf 'printf "%%s\\n" "$*" > %q\n' "$log" + printf 'cat %q\n' "$ids" + } > "$fake" + chmod +x "$fake" + + ( unset CODEX_THREAD_ID AGMSG_CODEX_APP_SERVER_KEY + AGMSG_CODEX_BRIDGE_APP_SERVER=ws://127.0.0.1:3333 AGMSG_NODE="$fake" \ + bash "$TYPES/codex/codex-record-session.sh" team alice "$proj" ) + + [ "$(recorded_uuid team alice)" = "thr-legacy" ] + grep -q -- '--app-server ws://127.0.0.1:3333' "$log" + ! grep -q -- 'ws://127.0.0.1:2222' "$log" +} + @test "codex record: no port file and no variable records nothing, it does not guess" { # Fail closed. Without a way to ask, the answer is "could not ask" -- never # "asked and found nothing" -- so no weaker signal may seat a thread here. diff --git a/tests/test_delivery.bats b/tests/test_delivery.bats index d99f562d3..7ad0410b5 100644 --- a/tests/test_delivery.bats +++ b/tests/test_delivery.bats @@ -2146,6 +2146,13 @@ JSON printf '%s\n' "$*" >> "$AGMSG_TEST_LOG" EOF chmod +x "$fake" + # A no-key launch keeps the explicit URL ahead of the legacy project record. + # shellcheck disable=SC1090 + source "$SCRIPTS/lib/hash.sh" + local legacy_hash + legacy_hash="$(printf '%s' "$TEST_PROJECT" | agmsg_sha1)" + mkdir -p "$TEST_SKILL_DIR/run" + printf '2222' > "$TEST_SKILL_DIR/run/codex-app-server.$legacy_hash.port" AGMSG_CODEX_BRIDGE=1 \ AGMSG_STORAGE_PATH="$TEST_SKILL_DIR/custom-store" \ @@ -2153,7 +2160,8 @@ EOF AGMSG_CODEX_BRIDGE_CMD="$fake" \ AGMSG_TEST_LOG="$log" \ CODEX_THREAD_ID="thread-123" \ - bash "$SCRIPTS/session-start.sh" codex "$TEST_PROJECT" >/dev/null + env -u AGMSG_CODEX_APP_SERVER_KEY \ + bash "$SCRIPTS/session-start.sh" codex "$TEST_PROJECT" >/dev/null for _ in {1..20}; do [ -f "$log" ] && break @@ -2168,6 +2176,45 @@ EOF grep -q -- "--inline-inbox" "$log" } +@test "session-start.sh for codex uses the scoped server key port in its request URL" { + bash "$SCRIPTS/join.sh" team alice codex "$TEST_PROJECT" >/dev/null + _seed_role_record team alice thread-scoped "$TEST_PROJECT" codex + # shellcheck disable=SC1090 + source "$SCRIPTS/lib/hash.sh" + local hash request_file request + hash="$(printf '%s' "$TEST_PROJECT" | agmsg_sha1)" + request_file="$TEST_SKILL_DIR/run/codex-bridge-request.$hash" + mkdir -p "$TEST_SKILL_DIR/run" + printf '1111' > "$TEST_SKILL_DIR/run/codex-app-server.$hash.port" + printf '2222' > "$TEST_SKILL_DIR/run/codex-app-server.scoped-key.port" + bash -c 'sleep 10 & wait' codex --remote unix:///tmp/foreign.sock 3>&- & + local foreign_pid=$! + local foreign_cmd i + for i in {1..20}; do + foreign_cmd="$(ps -o args= -p "$foreign_pid" 2>/dev/null || true)" + [[ "$foreign_cmd" == *'unix:///tmp/foreign.sock'* ]] && break + sleep 0.05 + done + printf '%s\n' "$foreign_cmd" | grep -Fq 'unix:///tmp/foreign.sock' + + AGMSG_CODEX_BRIDGE=1 \ + AGMSG_CODEX_BRIDGE_LAUNCHER=1 \ + AGMSG_CODEX_APP_SERVER_KEY=scoped-key \ + AGMSG_CODEX_BRIDGE_APP_SERVER=ws://127.0.0.1:4444 \ + AGMSG_AGENT_PID="$foreign_pid" \ + CODEX_THREAD_ID=thread-scoped \ + bash "$SCRIPTS/session-start.sh" codex "$TEST_PROJECT" >/dev/null + kill "$foreign_pid" 2>/dev/null || true + wait "$foreign_pid" 2>/dev/null || true + + [ -f "$request_file" ] + request="$(cat "$request_file")" + [ "$request" = $'codex\tthread-scoped\tws://127.0.0.1:2222' ] + refute grep -Fq -- 'ws://127.0.0.1:1111' <<<"$request" + refute grep -Fq -- 'ws://127.0.0.1:4444' <<<"$request" + [[ "$request" != *'unix:///tmp/foreign.sock'* ]] +} + @test "session-start.sh for codex stays quiet without monitor launcher env" { bash "$SCRIPTS/join.sh" team alice codex "$TEST_PROJECT" >/dev/null local fake="$TEST_SKILL_DIR/fake-codex-bridge"