Repository navigation
fix(agent-setup): ask before a sign-in changes the selected account - #9181
tsavo-at-pieces wants to merge 4 commits into
Conversation
In a workspace terminal the codex and claude wrappers point every launch at the account selected for new agents. That included `codex login`, `codex logout`, `claude auth login` and `claude auth logout`. A bare `codex login` signed in to the selected home instead of ~/.codex, and so did `env -u CODEX_HOME codex login`, because the resolver filled an unset CODEX_HOME from the pointer file. Current Codex releases log out the target home's stored login before the browser step, so this ended the selected account's login even when the sign-in was cancelled. buildDefaultAccountResolver now takes each wrapper's sign-in commands and matches them against the leading words of the arguments. For those commands an unset value stays unset. When the value is the injected selection (it equals its SUPERSET_DEFAULT_* twin and, for Codex, is not the ambient home), the wrapper names the home and asks on a terminal. Without a terminal it exits 2 and prints how to run the command on that account or without it. Agent runs, `codex login status`, --help, explicit homes and logins with no selection are unchanged. Fixes ParadevAI#9172 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Resolver options and credential guard packages/agent-setup/src/agent-wrappers-common.ts |
The resolver accepts ambient-home and command options. It matches configured credential commands, excludes read-only commands, and prompts or refuses when a selected account would be used. |
Wrapper configuration and behavior checks packages/agent-setup/src/agent-wrappers-claude-codex-opencode.ts, packages/agent-setup/src/default-account-resolver.test.ts, packages/agent-setup/src/agent-wrappers.test.ts, docs/agent-tooling.md |
The Claude and Codex wrappers configure credential commands. Tests cover command parsing, account selection, terminal confirmation, refusal, and piped input. The documentation describes the wrapper behavior. |
System-default Codex API re-sign
| Layer / File(s) | Summary |
|---|---|
Ambient-home resolution and checks apps/desktop/src/renderer/routes/_authenticated/settings/usage/components/UsageView/utils/switchSignInCommand/*, apps/desktop/src/renderer/routes/_authenticated/settings/usage/components/UsageView/utils/apiBilling/fixtures/shell-checks.ts |
The system-default API-key command checks SUPERSET_AMBIENT_CODEX_HOME before CODEX_HOME and $HOME/.codex. Tests verify the selected home and billing-marker location. |
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Bug fix · Severity of issue fixed: Medium
Sequence Diagram(s)
sequenceDiagram
participant CodexWrapper
participant DefaultAccountResolver
participant Terminal
participant CodexCLI
CodexWrapper->>DefaultAccountResolver: Resolve account and inspect credential command
alt Selected account would be used
DefaultAccountResolver->>Terminal: Request confirmation
Terminal-->>DefaultAccountResolver: Return yes or no
alt Confirmation accepted
DefaultAccountResolver->>CodexCLI: Continue credential command
else Confirmation declined
DefaultAccountResolver-->>CodexWrapper: Exit with status 1
end
else No terminal is available
DefaultAccountResolver-->>CodexWrapper: Exit with status 2 without running command
else Account variable is unset or account is not selected
DefaultAccountResolver->>CodexCLI: Run command with resolved account environment
end
Possibly related issues
#9173: The changes resolve the system-default Codex home for API-key re-sign commands, but do not change the Usage card’s status classification or signed-out card behavior.
Suggested reviewers: kitenite
Merge Risk: 
da6e8
-
The change makes Codex and Claude sign-in and sign-out commands ask for confirmation, or refuse without a terminal, before touching the selected account. No concrete merge-blocking defect was found in the reviewed changes.
Pre-merge checks: 
-
Check name Status Explanation Resolution Linked Issues check Issue #9172has coding requirements. The PR meets most of them.buildDefaultAccountResolver(agent-wrappers-common.ts) matches whole argument words for Codexlogin/logoutand Claudeauth login…Make the non-terminal refusal in buildCredentialCommandGuardprint how to name the home explicitly, as#9172requires. Use the same form as the declined-answer hint, for exampleCODEX_HOME=<selected home> codex …or `env -u CLAUDE_CONFI…Docstring Coverage Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title uses conventional commit format and clearly describes the main change: prompting before sign-in changes the selected account. Description check The description includes the required What & why, How I tested it, and Checklist sections. It explains the behavior, scope, tests, known environment limitations, and incomplete repository-wide typeche… Out of Scope Changes check The reviewed changes stay within #9172. The resolver, wrapper callers, and tests inpackages/agent-setupimplement the credential-command guard.docs/agent-tooling.mddescribes that guard. The des…Full details: Description check
-
Explanation
The description includes the required What & why, How I tested it, and Checklist sections. It explains the behavior, scope, tests, known environment limitations, and incomplete repository-wide typecheck status. The unchecked checklist item accurately notes that CI must run the full repository checks.
Full details: Out of Scope Changes check
-
Explanation
The reviewed changes stay within
#9172. The resolver, wrapper callers, and tests inpackages/agent-setupimplement the credential-command guard.docs/agent-tooling.mddescribes that guard. The desktop change inswitchSignInCommand.tsand its test and fixture files alters the system-default Codex API re-sign target. The issue names this path: the Usage card's advised command in a workspace terminal re-signs the selected home. The PR description and objectives now include this change. Its test files cover the change. No unrelated product behavior appears in the summary.
-
Full details: Linked Issues check
-
Explanation
Issue
#9172has coding requirements. The PR meets most of them.buildDefaultAccountResolver(agent-wrappers-common.ts) matches whole argument words for Codexlogin/logoutand Claudeauth login/auth logout, with root options allowed before the subcommand.login statusand help launches are excluded throughreadOnlyCommandsand the help check. An unsetCODEX_HOME/CLAUDE_CONFIG_DIRstays unset for credential commands. The guard compares against theSUPERSET_DEFAULT_*twin. On a terminal it asks[y/N]. Onlyy/yescontinues. A declined answer prints the system-default command. The desktop API re-sign now usesSUPERSET_AMBIENT_CODEX_HOMEfirst. The unmet requirement:#9172requires that, without a terminal, the wrapper refuse "with instructions for explicitly naming the home" (issue body: "it refuses and says how to name the home explicitly"). The current refusal names the home but prints no command, and says only that a person must confirm in a terminal. The earlier head printed the explicit-home command. The incremental commit that removed it makes this requirement unmet.docs/agent-tooling.mddocuments an override for scripts, but the refusal output itself does not give the instructions. I checked source diffs only and did not run the tests.Resolution
Make the non-terminal refusal in
buildCredentialCommandGuardprint how to name the home explicitly, as#9172requires. Use the same form as the declined-answer hint, for exampleCODEX_HOME=<selected home> codex …orenv -u CLAUDE_CONFIG_DIR claude …for the system default, or the form the issue owner accepts. Update the tests that assert the refusal prints no command. If the author keeps the no-command design,#9172must first be revised by its owner to drop that requirement. The PR cannot change the requirement alone.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 9 functions across 7 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
 [Medium impact] The change is limited to credential commands in the Codex and Claude wrappers. Fix the leading-option bypass before merging so credential commands cannot silently change the selected account. FindingsFix with agent prompt### Issue 1
packages/agent-setup/src/agent-wrappers-common.ts:207-209
With an account selected, `codex -c 'model="gpt-5"' logout` stops this scan at `-c`, leaving `superset_credential_command` empty. Logout then changes the selected account without asking. With `env -u CODEX_HOME`, the missed match also lets the resolver fill the selected home back in.
The Codex wrapper already passes global options before subcommands, so this is a supported argument order. Skip supported global options and their values before checking the command.
### Issue 2
packages/agent-setup/src/agent-wrappers-common.ts:211-215
Joining arguments with spaces makes `claude "auth login"` indistinguishable from `claude auth login`. The first command passes one ordinary prompt; it does not sign in. With an injected selection, the wrapper now asks for confirmation or exits 2 instead of starting that chat.
Match separate arguments rather than a joined string, and add a test for the quoted prompt.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryAdds confirmation before Codex and Claude sign-in commands change the selected account. Commands without a terminal are refused, and recognized credential commands leave an unset home unset.
Acknowledged by Reviews (1) · Last reviewed commit: "fix(agent-setup): ask before a sign-in c..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| case "$superset_arg" in | ||
| -h|--help) superset_command_words=""; break ;; | ||
| -*) break ;; |
There was a problem hiding this comment.
With an account selected, codex -c 'model="gpt-5"' logout stops this scan at -c, leaving superset_credential_command empty. Logout then changes the selected account without asking. With env -u CODEX_HOME, the missed match also lets the resolver fill the selected home back in.
The Codex wrapper already passes global options before subcommands, so this is a supported argument order. Skip supported global options and their values before checking the command.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/agent-setup/src/agent-wrappers-common.ts
Line: 207-209
Comment:
**Leading options bypass confirmation**
With an account selected, `codex -c 'model="gpt-5"' logout` stops this scan at `-c`, leaving `superset_credential_command` empty. Logout then changes the selected account without asking. With `env -u CODEX_HOME`, the missed match also lets the resolver fill the selected home back in.
The Codex wrapper already passes global options before subcommands, so this is a supported argument order. Skip supported global options and their values before checking the command.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| superset_command_words="\${superset_command_words:+$superset_command_words }$superset_arg" | ||
| done | ||
| superset_credential_command="" | ||
| case "$superset_command_words" in | ||
| ${pattern}) superset_credential_command="\${0##*/} $superset_command_words" ;; |
There was a problem hiding this comment.
Joining arguments with spaces makes claude "auth login" indistinguishable from claude auth login. The first command passes one ordinary prompt; it does not sign in. With an injected selection, the wrapper now asks for confirmation or exits 2 instead of starting that chat.
Match separate arguments rather than a joined string, and add a test for the quoted prompt.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/agent-setup/src/agent-wrappers-common.ts
Line: 211-215
Comment:
**Quoted prompts get refused**
Joining arguments with spaces makes `claude "auth login"` indistinguishable from `claude auth login`. The first command passes one ordinary prompt; it does not sign in. With an injected selection, the wrapper now asks for confirmation or exits 2 instead of starting that chat.
Match separate arguments rather than a joined string, and add a test for the quoted prompt.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.The match stopped at the first argument that starts with "-", so a sign-in written with a root option before the subcommand (`codex -c key=value login`, `codex --enable hooks logout`, `claude --verbose auth login`) was not recognised. It ran on the selected account without asking, and `env -u CODEX_HOME codex -c key=value login` got CODEX_HOME filled in from the pointer file again. Codex takes its global options before any subcommand, and the wrapper's own exec line puts some there. The wrappers now look for the command words as whole arguments anywhere before `--`: `login` or `logout` for Codex, `auth` and then `login` or `logout` for Claude. `codex login status` stays read-only, and a launch that asks for help (-h, --help, or help first) is left alone. Because only whole arguments count, a one-argument prompt such as `claude "auth login"` starts a chat instead of stopping at the guard. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…v/tty Without a terminal the refusal printed two runnable lines: one ran the same sign-in on the selected account with no confirmation, the other sent it to the CLI's default home. In a workspace most callers without a terminal are agents' tool calls, so an agent could follow either line and end the login the guard is there to protect. The refusal now names the home and says a person has to confirm in a terminal, and prints no command. docs/agent-tooling.md describes the override for scripts. The question also needed stdin to be a terminal, so a sign-in that pipes its input was refused inside an interactive terminal. That includes the Usage card's re-sign command for an API-billed Codex profile, which pipes the key into `codex login --with-api-key`. The wrapper now asks whenever stderr is a terminal, and reads the answer from /dev/tty when stdin is not one. After a "no", the hint points at the system default: for Codex that is the preserved ambient home rather than the CLI's own ~/.codex. The hint keeps the original arguments, so `--with-api-key` and `--device-auth` are no longer dropped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Usage card's API re-sign command for the system-default Codex login
pointed at "${CODEX_HOME:-$HOME/.codex}". In a Paradev terminal with an
account selected, CODEX_HOME holds that account, so the command signed
the selected profile in with the key and marked it API-billed. With the
wrapper guard it stopped at a question about the selected home instead.
It now prefers the ambient home such a terminal preserves
(SUPERSET_AMBIENT_CODEX_HOME), which is the home the Usage tab calls the
system default, and behaves as before in any other shell.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What & why
Fixes #9172.
Once an account is the default for new agents (Settings → Usage), the
codexandclaudewrappers point every launch at it. That includedcodex login,codex logout,claude auth loginandclaude auth logout. A barecodex loginin a workspace terminal signed in to the selected home rather than~/.codex, and so didenv -u CODEX_HOME codex login, because the resolver filled an unset value from the pointer file. Current Codex releases log out the target home's stored login before the browser step, so the selected account lost its login even when the sign-in was cancelled. Another tool that manages logins in the same home lost its login too.buildDefaultAccountResolvernow takes each wrapper's sign-in commands and looks for their words as whole arguments anywhere before--:loginorlogoutfor Codex,authand thenloginorlogoutfor Claude. Root options in front of the subcommand (codex -c key=value login,codex --enable hooks logout,claude --verbose auth login) do not hide it, and a one-argument prompt such asclaude "auth login"does not count.codex login statusand launches that ask for help (-h,--help, orhelpfirst) are left alone. For the sign-in commands:CODEX_HOME/CLAUDE_CONFIG_DIRstays unset, soenv -u CODEX_HOME codex loginreaches the CLI's own default, as it does outside Paradev.SUPERSET_DEFAULT_*twin and, for Codex, is notSUPERSET_AMBIENT_CODEX_HOME), the wrapper names the home and asksContinue? [y/N]if stderr is a terminal. The answer comes from stdin, or from/dev/ttywhen stdin carries the CLI's input, as when an API key is piped intocodex login --with-api-key. Onlyyoryescontinues. Any other answer exits 1 and prints the same command aimed at the system default (CODEX_HOME=<ambient home> codex …, orenv -u CLAUDE_CONFIG_DIR claude …).SUPERSET_DEFAULT_CODEX_HOME= CODEX_HOME=<home> codex login);docs/agent-tooling.mddescribes this.A pseudo-terminal counts as a terminal, so an agent whose shell tool runs commands in a PTY can still answer the question. The guard catches a sign-in typed out of habit and an agent's ordinary tool call; it does not stop a caller that is set on signing in to that home.
The guard checks the
SUPERSET_DEFAULT_*twin, notSUPERSET_TERMINAL_ID, so it also covers chat agents, whose environment carries the selection without a terminal id. Matching errs toward asking: an unquoted prompt word (codex exec login) andcodex mcp login <server>ask too.One desktop change: the Usage card's API-key re-sign for the system-default Codex login pointed at
"${CODEX_HOME:-$HOME/.codex}". In a workspace terminal with an account selected, that is the selected account, so the command signed the selected profile in with the key and marked it API-billed. It now uses"${SUPERSET_AMBIENT_CODEX_HOME:-${CODEX_HOME:-$HOME/.codex}}", the home the Usage tab calls the system default, and is unchanged in any other shell.Unchanged: agent runs,
codex login status,--help, a login into a home the user named that is not the selection (the Add account command, Switch sign-in for another profile), logins when nothing is selected, and the Usage dialog's own terminal, which has noSUPERSET_DEFAULT_*variables. Paradev's own commands that point at the selected home (the chat's sign-in command, and the Usage card's re-sign command for that profile, the API-key one included) work after one confirmation.The third parameter of
buildDefaultAccountResolveris now an options object (ambientEnvVar,credentialCommands,readOnlyCommands). Both callers are inagent-setup.docs/agent-tooling.mddescribes the guard, the override for scripts, and its reach: launches through the wrappers only, not the real binary run by path or/loginand/logoutinside a running session.Not in this PR: the Usage card still suggests a bare
codex login/claude auth loginwhen the system-default login has expired. In a workspace terminal with an account selected, that command stops at the question, which names the selected home; onnthe wrapper prints the system-default form. Changing that card's command would be a separate change in the desktop app.How I tested it
Everything ran in Docker on
oven/bun:1.4.3, the version.bun-versionpins: one container at a time, the tree streamed in withgit archive, dependencies frombun install --frozen --ignore-scripts, and no network for tests, typecheck or lint. The i18n catalogs were compiled first (bun run compileinpackages/i18n), as turbo's^builddoes beforetestandtypecheck.Tests first. The final test files, run against the first commit of this PR (
643ba76e), fail 11 tests inpackages/agent-setupand 2 in the desktop app (one is the runner of the shell checks), each on its assertion: root options before the subcommand (Codex and Claude, refused and kept unset, end to end through the generated wrappers), the one-argumentclaude "auth login"prompt, the refusal printing no command, the piped API-key login asking through/dev/tty(resolver and generated wrapper), the hint after a "no", and the system-default Codex API re-sign targeting the ambient home.With the change,
bun testinpackages/agent-setup: 403 pass, 37 fail. Unchangedmainin the same container: 380 pass, 37 fail. The 37 are the same tests by name in both runs: notify-hook, shell-wrapper fallback and Codex session-log tests that needcurl,jq,python3orps, which the slim image does not have. None of them touches the resolver. The two touched test files pass at each of the three new commits, apart from the same three session-log tests.Desktop: the two touched test files,
switchSignInCommand.test.tsandapiBilling.test.ts(which runs the shell checks, two of them new, in a separate process), 11 pass.The prompt tests run on a real pseudo-terminal (
Bun.spawnwithterminal, which makes it the controlling terminal):ycontinues;nand a bare Enter exit 1 and the CLI does not run. The piped tests feedprintf '%s' sk-test | … login --with-api-keyand check that the answer came from the terminal and the key reached the CLI.Mutation check. 22 small changes to the new shell and to the wrapper options, each run against the two test files: 21 make a test fail. The one that survives drops the
[ -t 2 ]check, which only matters when stderr is redirected (codex login 2>/dev/nullon a terminal), where the command is refused without a visible message.bun run typecheck:packages/agent-setupno errors;apps/desktopno errors (unchangedmain: no errors either).biome check .over the whole repo (whatscripts/lint.shruns), Biome 2.4.2: no diagnostics.bash -non the generatedcodexandclaudewrappers. The new shell uses only what bash 3.2 (macOS/bin/bash) has (case,[ -t ],read -r,printf %q, a subshell redirect from/dev/tty); I did not run it on macOS.Flows with the generated wrappers and the commands the desktop code itself generates (
addAccountCommand,switchSignInCommand,agentLoginCommand), stub CLIs that print what they got, accounts selected:ysigns in with the key and writes the marker,nchanges nothing. The system-default API re-sign goes to the ambient home with no question and leaves the selected profile alone.codex login,codex login --with-api-key,codex -c model=x logout,claude auth login --console,claude --verbose auth logout, andcodex --config foo=bar loginin a chat agent's environment all exit 2 without printing a command. The documented override runs.No screenshots, because no UI changed. These are the generated wrappers in the container with
/home/u/.codex-workand/home/u/.claude-workselected, over stub CLIs that print the home and the arguments they got (REAL codex …):Checklist
type(scope): subject)bun run lintandbun run typecheckpass (CI fails on lint warnings too). Lint passes over the whole repo. Typecheck passes forpackages/agent-setupandapps/desktop, the two packages changed; the repo-wide run is left to CI.🤖 Generated with Claude Code
Summary by CodeRabbit