From 5f6db2ac9ded292800b1bbe03c66fd2b6a5afbee Mon Sep 17 00:00:00 2001 From: Lio Lunesu Date: Tue, 4 Aug 2026 15:13:45 +0000 Subject: [PATCH 1/3] fix(codex): the pane re-authenticates itself when the token is dead MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A codex Remote Control pane on a box whose ChatGPT token had been invalidated printed the raw pairing error — HTTP status, cf-ray, JSON body — and offered "Press Enter to try again", which could not work: `codex login status` is a LOCAL check, so it kept reporting "Logged in using ChatGPT", the sign-in guard kept (correctly) declining, and every Enter reprinted the same 401. Only typing the undocumented `login` word got out. Pairing is where the rejection surfaces, so classify its failure instead of retrying blind: on an auth-shaped error (`token_invalidated`, `HTTP 401`, `invalid_grant`, "unauthorized", "sign in again") pair() returns 2 at once rather than burning three attempts over six seconds, and onboard() drops the dead credentials and re-runs the device flow by itself, printing the server's own message and not the transport guts. Details worth knowing: - The automatic sign-in happens once per cycle, guarded by a flag: `logout` is destructive, and a 401 a fresh token cannot cure (wrong account, revoked access) would otherwise spin the pane through logout/device-auth forever. The second rejection says so and stops; the flag clears on a successful pairing, so a token that expires hours later still gets one automatic recovery. - Failures that are NOT about auth keep the old behaviour exactly — the cold-start enrollment race and a dead network are retried three times, reported raw, and never cost the box its working credentials. - Shell globs, not grep: the agent unit's PATH carries coreutils but no gnugrep, and `grep: command not found` would have read as "not an auth failure" and silently restored the dead end. Same reason the reason-string extraction is parameter expansion. - `login` stays as a typed word: signing in as the wrong ACCOUNT produces no error to detect, and only the user knows about it. - relogin() now clears was_signed_in when the box ends up signed out (a device flow the user walked away from), so the health loop still notices sign-in whenever it does complete. The transition check keeps its short-circuit, so a signed-in pane spawns no `login status` every 5s. tests/sessions.nix drives the supervisor wrapper against a stub codex — a real server-side rejection is not producible in the sandbox — and asserts the three outcomes: invalidated token recovers with no keystroke and no HTTP status on screen, a still-rejected fresh sign-in reports once without looping, and a network failure retries without logging out. Fixes #187 (first item; the remotable-vs-local question there is separate). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FY7YhQDw21A83rdQtkmtAi --- README.md | 18 ++++++-- modules/agent-box.nix | 97 ++++++++++++++++++++++++++++++++++++---- modules/agent-box.nix.in | 97 ++++++++++++++++++++++++++++++++++++---- tests/sessions.nix | 97 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 287 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 63cb97e..a94ea7f 100644 --- a/README.md +++ b/README.md @@ -225,10 +225,20 @@ itself and prints the URL plus the one-time code (valid 15 min); open the link on any device, enter the code, and the pane goes straight on to print the pairing code for the Codex desktop/mobile app. Pairing codes are short-lived - **press Enter in the pane for a fresh one**. Enter also retries sign-in if it -was abandoned. Typing `login` + Enter drops the stored credentials and signs in -again - the recovery path when a box holds a stale token or the wrong account, -where `codex login status` still succeeds but pairing keeps failing. Those two -keys are all the pane's keyboard does; it is not a codex prompt. +was abandoned. Those two keys are all the pane's keyboard does; it is not a +codex prompt. + +Credentials the ChatGPT backend has invalidated (password change, revoked +session, expired refresh token) need no keys at all: `codex login status` is a +local check and keeps reporting "logged in", so pairing is what discovers the +`401 token_invalidated`, and the pane then drops the dead credentials and re-runs +the device flow by itself - printing the server's own explanation, not the HTTP +transcript. That happens once per cycle; if a *fresh* sign-in is rejected too, +the account is the problem (wrong account, no Codex access) and the pane says so +instead of looping. Typing `login` + Enter forces the same logout-and-sign-in by +hand - still worth having, because signing in as the wrong account produces no +error to detect. Pairing failures that are *not* about auth (an enrollment race +just after boot, no network) are retried and never drop working credentials. Sign-in has to be device auth: plain `codex login` starts a callback server on `localhost:1455`, which the browser on your laptop cannot reach. Until sign-in diff --git a/modules/agent-box.nix b/modules/agent-box.nix index 7501387..faaf78f 100644 --- a/modules/agent-box.nix +++ b/modules/agent-box.nix @@ -386,6 +386,40 @@ let EOF "$codex" login --device-auth } + # `login status` is a LOCAL check: it reports how ~/.codex/auth.json was + # minted, not whether the backend still honours it. Credentials the server + # has invalidated (password change, revoked session, expired refresh token) + # keep reporting "Logged in using ChatGPT", so signed_in() stays true, + # device_login() correctly declines, and PAIRING is the first thing that + # notices — with an HTTP 401 whose body names the reason. Recognise that + # shape so the pane can re-authenticate itself instead of printing the + # transport guts and waiting for a user who would have to know the + # undocumented `login` word to get out of it (issue 187). Deliberately + # narrow: enrollment races and network failures must NOT match, because + # retrying is the right answer for those and dropping credentials is not. + auth_rejected() { + # Shell globs, not grep: the agent unit's PATH is curated (coreutils, but + # no gnugrep), and a `grep: command not found` here would silently read as + # "not an auth failure" and quietly restore the old dead end. + case "$1" in + *token_invalidated*|*invalid_grant*|*"HTTP 401"*|*[Uu]nauthorized* \ + |*"sign in again"*|*"signing in again"*) return 0 ;; + esac + return 1 + } + # The only part of the JSON-RPC blob worth showing a human is the server's + # own message; the URL, cf-ray and status code belong in a bug report. + auth_reason() { + reason=$1 + case "$reason" in + *'"message":"'*) + reason=''${reason#*'"message":"'} + reason=''${reason%%'"'*} + ;; + *) reason="the stored credentials were rejected" ;; + esac + printf '%s' "$reason" + } # A pairing code is what the Codex desktop/mobile apps ask for to adopt this # box, so mint it here too. The first attempt after a cold daemon start # races enrollment and fails with either "timed out waiting for @@ -394,6 +428,9 @@ EOF # the error to the user. The last attempt re-enables Remote Control first: # enrollment cannot complete while logged out, so a daemon that came up # before sign-in needs the nudge once credentials exist. + # + # Exit status: 0 paired, 2 the credentials were rejected (caller + # re-authenticates), 1 anything else (already reported, retryable). pair() { attempt=0 while [ "$attempt" -lt 3 ]; do @@ -410,6 +447,9 @@ EOF EOF return 0 fi + # Retrying a token the backend has rejected just prints the same 401 + # three times over six seconds; hand it to the caller at once. + if auth_rejected "$pairout"; then return 2; fi sleep 2 done printf '\n ✗ Could not mint a pairing code:\n' >&2 @@ -421,10 +461,36 @@ EOF # on startup and on every Enter, so a session that is already paired, one # waiting on sign-in, and one whose code just expired all respond to the # same keystroke. + # + # Rejected credentials are the one failure Enter cannot fix on its own, so + # onboard fixes it: sign in again automatically. Guarded by a flag rather + # than run unconditionally — `logout` is destructive, and a backend + # answering 401 for some reason a fresh token won't cure would otherwise + # put the pane in a logout/device-auth spin. The flag clears on a + # successful pairing, so a token that expires later in the same pane's life + # still gets one automatic recovery. + relogin_tried=false onboard() { device_login || true if signed_in; then - pair || true + pair; pairrc=$? + if [ "$pairrc" -eq 0 ]; then + relogin_tried=false + elif [ "$pairrc" -eq 2 ] && [ "$relogin_tried" = false ]; then + relogin_tried=true + # No "signing in again" line here: device_login's own banner says + # exactly that, and says it right before the URL and code. + printf '\n ✗ ChatGPT rejected this box'"'"'s stored credentials:\n' + printf ' %s\n' "$(auth_reason "$pairout")" + relogin + elif [ "$pairrc" -eq 2 ]; then + # Already re-authenticated once this cycle and still rejected: the + # account itself is the problem (wrong account, revoked access), which + # no amount of retrying here can tell apart. Say so instead of looping. + printf '\n ✗ ChatGPT still rejects the credentials: %s\n' "$(auth_reason "$pairout")" >&2 + printf '%s\n' " Sign-in was already retried once. Type: login to try again," >&2 + printf '%s\n' " or check that the account you signed in with has Codex access." >&2 + fi else cat </dev/null 2>&1 || true onboard + # The logout may be ALL that happened — a device flow the user walked away + # from leaves the box signed out. Say so, so the health loop's transition + # check notices whenever sign-in does complete. + signed_in || was_signed_in=false } daemon_failed() { cat >&2 <&2 @@ -417,10 +457,36 @@ EOF # on startup and on every Enter, so a session that is already paired, one # waiting on sign-in, and one whose code just expired all respond to the # same keystroke. + # + # Rejected credentials are the one failure Enter cannot fix on its own, so + # onboard fixes it: sign in again automatically. Guarded by a flag rather + # than run unconditionally — `logout` is destructive, and a backend + # answering 401 for some reason a fresh token won't cure would otherwise + # put the pane in a logout/device-auth spin. The flag clears on a + # successful pairing, so a token that expires later in the same pane's life + # still gets one automatic recovery. + relogin_tried=false onboard() { device_login || true if signed_in; then - pair || true + pair; pairrc=$? + if [ "$pairrc" -eq 0 ]; then + relogin_tried=false + elif [ "$pairrc" -eq 2 ] && [ "$relogin_tried" = false ]; then + relogin_tried=true + # No "signing in again" line here: device_login's own banner says + # exactly that, and says it right before the URL and code. + printf '\n ✗ ChatGPT rejected this box'"'"'s stored credentials:\n' + printf ' %s\n' "$(auth_reason "$pairout")" + relogin + elif [ "$pairrc" -eq 2 ]; then + # Already re-authenticated once this cycle and still rejected: the + # account itself is the problem (wrong account, revoked access), which + # no amount of retrying here can tell apart. Say so instead of looping. + printf '\n ✗ ChatGPT still rejects the credentials: %s\n' "$(auth_reason "$pairout")" >&2 + printf '%s\n' " Sign-in was already retried once. Type: login to try again," >&2 + printf '%s\n' " or check that the account you signed in with has Codex access." >&2 + fi else cat </dev/null 2>&1 || true onboard + # The logout may be ALL that happened — a device flow the user walked away + # from leaves the box signed out. Say so, so the health loop's transition + # check notices whenever sign-in does complete. + signed_in || was_signed_in=false } daemon_failed() { cat >&2 < /tmp/stub-codex <<'EOF'\n" + "#!/bin/sh\n" + 'printf "%s\\n" "$*" >> /tmp/stub/log\n' + 'case "$*" in\n' + '"app-server daemon version") exit 1 ;;\n' + '"login status") test -f /tmp/stub/loggedin ;;\n' + '"login --device-auth")\n' + " touch /tmp/stub/loggedin\n" + " test -f /tmp/stub/persist || rm -f /tmp/stub/stale\n" + ' echo "Successfully logged in" ;;\n' + '"logout") rm -f /tmp/stub/loggedin ;;\n' + '"remote-control pair")\n' + ' test -f /tmp/stub/loggedin || { echo "not signed in" >&2; exit 1; }\n' + # Single-quoted printf, not echo: the real error embeds a JSON body, and + # whether `echo` eats the backslashes of an escaped one is shell- + # dependent — the pane greps that JSON for the message it shows. + " if test -f /tmp/stub/stale; then\n" + " printf '%s\\n' 'Error: remoteControl/pairing/start failed: remote" + " control server refresh failed: HTTP 401 Unauthorized, cf-ray: test," + ' body: {"error":{"message":"Your authentication token has been' + ' invalidated. Please try signing in again.","type":' + '"invalid_request_error","code":"token_invalidated"},"status":401}' + "' >&2\n" + " exit 1\n" + " fi\n" + " if test -f /tmp/stub/offline; then\n" + " printf '%s\\n' 'Error: remoteControl/pairing/start failed: error" + " sending request: tcp connect error: Connection refused (os error" + " 111)' >&2\n" + " exit 1\n" + " fi\n" + ' echo "Pairing code: TEST-PAIR" ;;\n' + "esac\n" + "EOF" + ) + machine.succeed("chmod 0755 /tmp/stub-codex") + + def run_pane(*markers): + machine.succeed("rm -rf /tmp/stub && install -d -m 0777 /tmp/stub") + for marker in markers: + machine.succeed(f"touch /tmp/stub/{marker}") + machine.succeed( + as_agent(f"{wrapper} '' /tmp/stub-codex > /tmp/stub/out 2>&1 || true") + ) + return ( + machine.succeed("cat /tmp/stub/out"), + machine.succeed("cat /tmp/stub/log"), + ) + + # Invalidated token: the pane explains itself in the server's own words, + # drops the dead credentials, runs the device flow and pairs — all without a + # keystroke, and without showing anyone an HTTP status line. + stale_out, stale_calls = run_pane("loggedin", "stale") + assert "rejected this box's stored credentials" in stale_out, stale_out + assert "authentication token has been invalidated" in stale_out, stale_out + assert "HTTP 401" not in stale_out, stale_out + assert "Press Enter to try again" not in stale_out, stale_out + assert "Pairing code: TEST-PAIR" in stale_out, stale_out + assert "logout" in stale_calls, stale_calls + assert "login --device-auth" in stale_calls, stale_calls + # A rejection short-circuits the enrollment-race retries: one failed pair, + # then one that succeeds after signing in. Not three failures in six seconds. + assert stale_calls.count("remote-control pair") == 2, stale_calls + + # Credentials that stay rejected after a fresh sign-in are an account + # problem, so the pane says so ONCE and stops — a logout/device-auth spin + # would make the box unusable. + hopeless_out, hopeless_calls = run_pane("loggedin", "stale", "persist") + assert "still rejects the credentials" in hopeless_out, hopeless_out + assert hopeless_calls.count("logout") == 1, hopeless_calls + assert hopeless_calls.count("login --device-auth") == 1, hopeless_calls + + # A pairing failure that is NOT about auth keeps the old behaviour: retry + # (enrollment races clear on their own), report, offer Enter — and never + # drop working credentials. + offline_out, offline_calls = run_pane("loggedin", "offline") + assert "Could not mint a pairing code" in offline_out, offline_out + assert "Press Enter to try again" in offline_out, offline_out + assert "logout" not in offline_calls, offline_calls + assert offline_calls.count("remote-control pair") == 3, offline_calls + # Re-adding an existing name errors out and must not clobber the stored # config (issue 100): helper keeps its codex agent. machine.fail( From 698f2879fdbf89698d2f64b81ea8c653bb69d7c7 Mon Sep 17 00:00:00 2001 From: Lio Lunesu Date: Tue, 4 Aug 2026 15:19:51 +0000 Subject: [PATCH 2/3] fix(test): the empty host-label argument ended the Nix string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `as_agent(f"{wrapper} '' /tmp/stub-codex ...")` passed the empty host label the way shell wants it — but a bare pair of single quotes CLOSES a Nix indented string, so tests/sessions.nix stopped parsing and every VM test in the run died at evaluation. Double quotes are an equally empty argument and are inert in Nix. (The first attempt to explain that in a comment used the same two characters, and broke it again.) Caught by `nix-instantiate --parse tests/*.nix modules/*.nix flake.nix` plus `nix eval .#checks.x86_64-linux.sessions.drvPath` — the test derivation now EVALUATES natively on aarch64 even though running its VM needs x86, which is the cheap check that was missing here. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FY7YhQDw21A83rdQtkmtAi --- tests/sessions.nix | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/sessions.nix b/tests/sessions.nix index 9b62c39..13df50c 100644 --- a/tests/sessions.nix +++ b/tests/sessions.nix @@ -324,7 +324,9 @@ for marker in markers: machine.succeed(f"touch /tmp/stub/{marker}") machine.succeed( - as_agent(f"{wrapper} '' /tmp/stub-codex > /tmp/stub/out 2>&1 || true") + # The empty first argument is written with DOUBLE quotes: a bare + # pair of single quotes would end this Nix indented string here. + as_agent(f'{wrapper} "" /tmp/stub-codex > /tmp/stub/out 2>&1 || true') ) return ( machine.succeed("cat /tmp/stub/out"), From d6e2aa1d7f4f99302fc40edc8b2b963375872900 Mon Sep 17 00:00:00 2001 From: Lio Lunesu Date: Tue, 4 Aug 2026 15:33:12 +0000 Subject: [PATCH 3/3] fix(test): the driver type-checks testScript, so narrow the Match MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `re.search(...).group(0)` is a `Match[str] | None`, and the nixos test driver runs `ty check` (plus `ruff check --select F`) over testScript as part of BUILDING the driver — so this failed the whole sessions test before any VM booted, with "Attribute `group` is not defined on `None`". Assign the match, assert it, then read it. Both gates now run natively here against the extracted script, with the driver-supplied names (machine, client, start_all, subtest) stubbed: `ty` and `ruff` are aarch64-available from the pinned nixpkgs even though the x86 driver is not buildable. Reintroducing the bare `.group(0)` reproduces CI's diagnostic exactly, and the committed file passes both. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FY7YhQDw21A83rdQtkmtAi --- tests/sessions.nix | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/sessions.nix b/tests/sessions.nix index 13df50c..2bd2833 100644 --- a/tests/sessions.nix +++ b/tests/sessions.nix @@ -278,9 +278,13 @@ # `app-server daemon version` fails in the stub, which ends the wrapper's # health loop instead of blocking the test; the "" first argument skips the # UTS re-exec, which is not what this asserts. - wrapper = re.search( + # Assigned and asserted, not `re.search(...).group(0)`: the driver + # type-checks testScript, and a Match|None cannot be subscripted there. + wrapper_match = re.search( r"/nix/store/\S*agent-box-codex-remote-control", helper_cmdline - ).group(0) + ) + assert wrapper_match, helper_cmdline + wrapper = wrapper_match.group(0) machine.succeed( "cat > /tmp/stub-codex <<'EOF'\n" "#!/bin/sh\n"