From 4840bb33f56c5366dd3a2edab8a3864cbf924658 Mon Sep 17 00:00:00 2001 From: Loongphy Date: Sun, 19 Apr 2026 07:19:32 +0800 Subject: [PATCH 1/3] feat: add api override flags for account commands --- README.md | 43 ++- docs/api-refresh.md | 21 +- src/cli.zig | 216 +++++++++++--- src/format.zig | 29 +- src/main.zig | 588 ++++++++++++++++++++++++++++++++++--- src/tests/cli_bdd_test.zig | 140 ++++++++- src/tests/e2e_cli_test.zig | 389 ++++++++++++++++++++++++ src/tests/main_test.zig | 21 +- 8 files changed, 1335 insertions(+), 112 deletions(-) diff --git a/README.md b/README.md index 50bc9bb..346aa8b 100644 --- a/README.md +++ b/README.md @@ -94,10 +94,10 @@ Remove-Item "$env:LOCALAPPDATA\codex-auth\bin\codex-auth-auto.exe" -Force -Error | Command | Description | |---------|-------------| -| `codex-auth list [--debug]` | List all accounts | +| `codex-auth list [--debug] [--api|--skip-api]` | List all accounts. `--api` forces a live refresh, while `--skip-api` uses only stored local usage and team-name data. | | `codex-auth login [--device-auth]` | Run `codex login` (optionally with `--device-auth`), then add the current account | -| `codex-auth switch []` | Switch active account interactively or by partial match | -| `codex-auth remove` | Remove accounts with interactive multi-select | +| `codex-auth switch [] [--api|--skip-api]` | Switch the active account interactively or by `` (row number, alias, or fuzzy match). `--api` forces a live refresh first; `--skip-api` stays local-only. | +| `codex-auth remove [...] [--all] [--api|--skip-api]` | Remove accounts interactively or by one or more selectors (row number, alias, or fuzzy match). Default behavior is local-only; `--api` forces a live refresh first. | | `codex-auth status` | Show auto-switch, service, and usage status | ### Import @@ -128,34 +128,59 @@ Remove-Item "$env:LOCALAPPDATA\codex-auth\bin\codex-auth-auto.exe" -Force -Error ```shell codex-auth list codex-auth list --debug +codex-auth list --api # force usage/team-name API refresh, even if config api is disabled +codex-auth list --skip-api # use only stored registry data; skip usage/team-name API refresh ``` +`--api` forces the foreground usage and team-name refresh path for this command only. +`--skip-api` keeps the current local snapshot exactly as stored in `registry.json`. +It does not call the usage API or `accounts/check`, so transient live-refresh failures such as `401` and `403` are not preserved in this mode. + ### Switch Account -Interactive: shows email, 5h, weekly, and last activity. +Interactive `switch` shows email, 5h, weekly, and last activity. +Without ``, it follows the configured refresh mode before opening the picker. +With ``, it resolves locally by default. +Use `--api` to force a foreground refresh first, or `--skip-api` to stay on stored local data only. ```shell codex-auth switch +codex-auth switch --api +codex-auth switch --skip-api ``` ![command switch](https://github.com/user-attachments/assets/48a86acf-2a6e-4206-a8c4-591989fdc0df) -Non-interactive: fuzzy match by email or alias. +`` can be a displayed row number, an alias, or a fuzzy email/alias match. +The row number follows the interactive `switch` list, and the same number from `codex-auth list` also works because both commands use the same ordering. ```shell -codex-auth switch john # match any account containing "john" -codex-auth switch john@gmail.com # match by full or partial email -codex-auth switch work # match by alias set during import +codex-auth switch 02 # switch by displayed row number +codex-auth switch john # fuzzy match by email or alias +codex-auth switch work # match by alias set during import +codex-auth switch --api work # force refresh before resolving the selector +codex-auth switch --skip-api 02 # local-only row-number selection ``` -If the keyword matches multiple accounts, the command falls back to interactive selection. Press `q` to quit without switching. +If `` matches multiple accounts, the command falls back to interactive selection. ### Remove Accounts +`remove` is local-only by default and does not refresh from APIs before deleting. +Use `--api` to force a foreground refresh first, or `--skip-api` to make that local-only choice explicit. +Each selector supports the same query forms as `switch`: row number, alias, or fuzzy email/alias match. +The row number follows the interactive `switch` list, and the same number from `codex-auth list` also works because both commands use the same ordering. +You can pass multiple selectors in one command. + ```shell codex-auth remove +codex-auth remove --skip-api 01 03 +codex-auth remove --api work personal +codex-auth remove 01 jane@example.com ``` +If any selector matches multiple accounts, `remove` asks for confirmation in interactive terminals before deleting. + ### Login (Add Account) Add the currently logged-in Codex account: diff --git a/docs/api-refresh.md b/docs/api-refresh.md index 91be5ab..15e06cc 100644 --- a/docs/api-refresh.md +++ b/docs/api-refresh.md @@ -41,12 +41,13 @@ The `accounts/check` response is parsed by `chatgpt_account_id`. `name: null` an - `api.usage = true`: foreground refresh uses the usage API. - `api.usage = false`: foreground refresh reads only the newest local `~/.codex/sessions/**/rollout-*.jsonl`. -- when `api.usage = true`, `list`, interactive `switch`, and `remove` without a query or `--all` refresh all stored accounts before rendering, using stored auth snapshots under `accounts/` with a maximum concurrency of `3` -- when one of those per-account foreground usage requests returns a non-`200` HTTP status, the corresponding `list` / `switch` / `remove` row shows that response status in both usage columns until a later successful refresh replaces it -- when a stored account snapshot cannot make a ChatGPT usage request because it is missing the required ChatGPT auth fields, the corresponding `list` / `switch` / `remove` row shows `MissingAuth` in both usage columns until a later successful refresh replaces it +- when `api.usage = true`, `list` and interactive `switch` refresh all stored accounts before rendering, using stored auth snapshots under `accounts/` with a maximum concurrency of `3` +- when one of those per-account foreground usage requests returns a non-`200` HTTP status, the corresponding `list` / `switch` row shows that response status in both usage columns until a later successful refresh replaces it +- when a stored account snapshot cannot make a ChatGPT usage request because it is missing the required ChatGPT auth fields, the corresponding `list` / `switch` row shows `MissingAuth` in both usage columns until a later successful refresh replaces it - when `api.usage = false`, foreground refresh still uses only the active local rollout data because local session files do not identify the other stored accounts -- `switch ` does not perform a foreground usage refresh before activation or before showing a local multi-match picker -- `remove ` and `remove --all` do not perform a foreground usage refresh before deletion +- `list --api` forces foreground usage refresh for this command even when `api.usage = false`; `list --skip-api` skips foreground usage refresh completely and renders only the stored registry data +- interactive `switch` follows the configured foreground usage mode by default; `switch ` resolves selectors locally from stored data by default; `switch --api` forces foreground usage refresh before selector resolution, while `switch --skip-api` stays local-only +- `remove` is local-only by default; `remove --api` forces foreground usage refresh before selector resolution or the interactive picker; `remove --skip-api` keeps the default local-only behavior explicit - `switch` does not refresh usage again after the new account is activated - the auto-switch daemon refreshes the current active account usage during each cycle when `auto_switch.enabled = true` - the auto-switch daemon may also refresh a small number of non-active candidate accounts from stored snapshots so it can score switch candidates @@ -58,10 +59,10 @@ The `accounts/check` response is parsed by `chatgpt_account_id`. `name: null` an - A usable ChatGPT auth context with both `access_token` and `chatgpt_account_id` is required. If either value is missing, refresh is skipped before any request is sent. - `login` refreshes immediately after the new active auth is ready. - Single-file `import` refreshes immediately for the imported auth context. -- `list` refreshes synchronously before rendering and waits for `accounts/check` when the active user scope qualifies. -- interactive `switch` refreshes synchronously before showing the picker and waits for `accounts/check` when the current active user scope qualifies. -- `switch ` skips foreground account-name refresh and uses stored metadata only. -- `list` and interactive `switch` load the request auth context from the current active `auth.json`. +- `list --api` forces synchronous `accounts/check` refresh for this command even when `api.account = false`; `list --skip-api` skips it and uses stored metadata only. +- interactive `switch` follows the configured account-name refresh mode by default; `switch ` is local-only by default; `switch --api` forces foreground account-name refresh before selector resolution, while `switch --skip-api` uses stored metadata only. +- `remove` is local-only by default; `remove --api` forces foreground account-name refresh before selector resolution or the interactive picker; `remove --skip-api` keeps the default local-only behavior explicit. +- `list`, interactive `switch`, `switch --api`, and `remove --api` load the request auth context from the current active `auth.json` when they do refresh. - the auto-switch daemon still uses a grouped-scope scan during each cycle when `auto_switch.enabled = true`. - daemon refreshes load the request auth context from stored account snapshots under `accounts/` and do not depend on the current `auth.json` belonging to the scope being refreshed. - when multiple stored ChatGPT snapshots exist for one grouped scope, daemon refreshes pick the snapshot with the newest `last_refresh`. @@ -76,7 +77,7 @@ Request failures and unparseable responses are non-fatal and leave stored `accou Grouped account-name refresh always operates on one `chatgpt_user_id` scope at a time. - `login` and single-file `import` start from the just-parsed auth info -- `list` and interactive `switch` start from the current active auth info +- `list` and interactive `switch` start from the current active auth info when foreground refresh is enabled - the auto-switch daemon scans registry-backed grouped scopes and refreshes each qualifying scope independently That scope includes: diff --git a/src/cli.zig b/src/cli.zig index 78296ad..25440d6 100644 --- a/src/cli.zig +++ b/src/cli.zig @@ -33,8 +33,15 @@ fn stderrColorEnabled() bool { return fs.File.stderr().isTty(); } +pub const ApiMode = enum { + default, + force_api, + skip_api, +}; + pub const ListOptions = struct { debug: bool = false, + api_mode: ApiMode = .default, }; pub const LoginOptions = struct { device_auth: bool = false, @@ -46,10 +53,14 @@ pub const ImportOptions = struct { purge: bool, source: ImportSource, }; -pub const SwitchOptions = struct { query: ?[]u8 }; -pub const RemoveOptions = struct { +pub const SwitchOptions = struct { query: ?[]u8, + api_mode: ApiMode = .default, +}; +pub const RemoveOptions = struct { + selectors: [][]const u8, all: bool, + api_mode: ApiMode = .default, }; pub const CleanOptions = struct {}; pub const AutoAction = enum { enable, disable }; @@ -149,6 +160,22 @@ pub fn parseArgs(allocator: std.mem.Allocator, args: []const [:0]const u8) !Pars opts.debug = true; continue; } + if (std.mem.eql(u8, arg, "--api")) { + switch (opts.api_mode) { + .default => opts.api_mode = .force_api, + .force_api => return usageErrorResult(allocator, .list, "duplicate `--api` for `list`.", .{}), + .skip_api => return usageErrorResult(allocator, .list, "`--api` cannot be combined with `--skip-api` for `list`.", .{}), + } + continue; + } + if (std.mem.eql(u8, arg, "--skip-api")) { + switch (opts.api_mode) { + .default => opts.api_mode = .skip_api, + .skip_api => return usageErrorResult(allocator, .list, "duplicate `--skip-api` for `list`.", .{}), + .force_api => return usageErrorResult(allocator, .list, "`--skip-api` cannot be combined with `--api` for `list`.", .{}), + } + continue; + } if (isHelpFlag(arg)) { return usageErrorResult(allocator, .list, "`--help` must be used by itself for `list`.", .{}); } @@ -257,21 +284,49 @@ pub fn parseArgs(allocator: std.mem.Allocator, args: []const [:0]const u8) !Pars return .{ .command = .{ .help = .switch_account } }; } - var query: ?[]u8 = null; + var opts: SwitchOptions = .{ .query = null }; var i: usize = 2; while (i < args.len) : (i += 1) { const arg = std.mem.sliceTo(args[i], 0); + if (std.mem.eql(u8, arg, "--api")) { + switch (opts.api_mode) { + .default => opts.api_mode = .force_api, + .force_api => { + if (opts.query) |query| allocator.free(query); + return usageErrorResult(allocator, .switch_account, "duplicate `--api` for `switch`.", .{}); + }, + .skip_api => { + if (opts.query) |query| allocator.free(query); + return usageErrorResult(allocator, .switch_account, "`--api` cannot be combined with `--skip-api` for `switch`.", .{}); + }, + } + continue; + } + if (std.mem.eql(u8, arg, "--skip-api")) { + switch (opts.api_mode) { + .default => opts.api_mode = .skip_api, + .skip_api => { + if (opts.query) |query| allocator.free(query); + return usageErrorResult(allocator, .switch_account, "duplicate `--skip-api` for `switch`.", .{}); + }, + .force_api => { + if (opts.query) |query| allocator.free(query); + return usageErrorResult(allocator, .switch_account, "`--skip-api` cannot be combined with `--api` for `switch`.", .{}); + }, + } + continue; + } if (std.mem.startsWith(u8, arg, "-")) { - if (query) |e| allocator.free(e); + if (opts.query) |query| allocator.free(query); return usageErrorResult(allocator, .switch_account, "unknown flag `{s}` for `switch`.", .{arg}); } - if (query != null) { - if (query) |e| allocator.free(e); + if (opts.query != null) { + if (opts.query) |query| allocator.free(query); return usageErrorResult(allocator, .switch_account, "unexpected extra query `{s}` for `switch`.", .{arg}); } - query = try allocator.dupe(u8, arg); + opts.query = try allocator.dupe(u8, arg); } - return .{ .command = .{ .switch_account = .{ .query = query } } }; + return .{ .command = .{ .switch_account = opts } }; } if (std.mem.eql(u8, cmd, "remove")) { @@ -279,33 +334,50 @@ pub fn parseArgs(allocator: std.mem.Allocator, args: []const [:0]const u8) !Pars return .{ .command = .{ .help = .remove_account } }; } - var query: ?[]u8 = null; + var selectors = std.ArrayList([]const u8).empty; + errdefer freeOwnedStringList(allocator, selectors.items); + defer selectors.deinit(allocator); var all = false; + var api_mode: ApiMode = .default; var i: usize = 2; while (i < args.len) : (i += 1) { const arg = std.mem.sliceTo(args[i], 0); + if (std.mem.eql(u8, arg, "--api")) { + switch (api_mode) { + .default => api_mode = .force_api, + .force_api => return usageErrorResult(allocator, .remove_account, "duplicate `--api` for `remove`.", .{}), + .skip_api => return usageErrorResult(allocator, .remove_account, "`--api` cannot be combined with `--skip-api` for `remove`.", .{}), + } + continue; + } + if (std.mem.eql(u8, arg, "--skip-api")) { + switch (api_mode) { + .default => api_mode = .skip_api, + .skip_api => return usageErrorResult(allocator, .remove_account, "duplicate `--skip-api` for `remove`.", .{}), + .force_api => return usageErrorResult(allocator, .remove_account, "`--skip-api` cannot be combined with `--api` for `remove`.", .{}), + } + continue; + } if (std.mem.eql(u8, arg, "--all")) { - if (all or query != null) { - if (query) |q| allocator.free(q); + if (all or selectors.items.len != 0) { return usageErrorResult(allocator, .remove_account, "`remove` cannot combine `--all` with another selector.", .{}); } all = true; continue; } if (std.mem.startsWith(u8, arg, "-")) { - if (query) |q| allocator.free(q); return usageErrorResult(allocator, .remove_account, "unknown flag `{s}` for `remove`.", .{arg}); } - if (query != null or all) { - if (query) |q| allocator.free(q); - if (all) { - return usageErrorResult(allocator, .remove_account, "`remove` cannot combine `--all` with another selector.", .{}); - } - return usageErrorResult(allocator, .remove_account, "unexpected extra selector `{s}` for `remove`.", .{arg}); + if (all) { + return usageErrorResult(allocator, .remove_account, "`remove` cannot combine `--all` with another selector.", .{}); } - query = try allocator.dupe(u8, arg); + try selectors.append(allocator, try allocator.dupe(u8, arg)); } - return .{ .command = .{ .remove_account = .{ .query = query, .all = all } } }; + return .{ .command = .{ .remove_account = .{ + .selectors = try selectors.toOwnedSlice(allocator), + .all = all, + .api_mode = api_mode, + } } }; } if (std.mem.eql(u8, cmd, "clean")) { @@ -412,10 +484,11 @@ fn freeCommand(allocator: std.mem.Allocator, cmd: *Command) void { if (opts.alias) |a| allocator.free(a); }, .switch_account => |*opts| { - if (opts.query) |e| allocator.free(e); + if (opts.query) |query| allocator.free(query); }, .remove_account => |*opts| { - if (opts.query) |q| allocator.free(q); + freeOwnedStringList(allocator, opts.selectors); + allocator.free(opts.selectors); }, else => {}, } @@ -488,6 +561,10 @@ fn freeImportOptions(allocator: std.mem.Allocator, auth_path: ?[]u8, alias: ?[]u if (alias) |value| allocator.free(value); } +fn freeOwnedStringList(allocator: std.mem.Allocator, items: []const []const u8) void { + for (items) |item| allocator.free(@constCast(item)); +} + pub fn printHelp(auto_cfg: *const registry.AutoSwitchConfig, api_cfg: *const registry.ApiConfig) !void { var stdout: io_util.Stdout = undefined; stdout.init(); @@ -547,8 +624,8 @@ pub fn writeHelp( .{ .name = "status", .description = "Show auto-switch and usage API status" }, .{ .name = "login", .description = "Login and add the current account" }, .{ .name = "import", .description = "Import auth files or rebuild registry" }, - .{ .name = "switch []", .description = "Switch the active account" }, - .{ .name = "remove [|--all]", .description = "Remove one or more accounts" }, + .{ .name = "switch [] [--api|--skip-api]", .description = "Switch the active account" }, + .{ .name = "remove [...] [--all] [--api|--skip-api]", .description = "Remove one or more accounts" }, .{ .name = "clean", .description = "Delete backup and stale files under accounts/" }, .{ .name = "config", .description = "Manage configuration" }, }; @@ -694,8 +771,8 @@ fn commandDescriptionForTopic(topic: HelpTopic) []const u8 { .status => "Show auto-switch, service, and usage API status.", .login => "Run `codex login` or `codex login --device-auth`, then add the current account.", .import_auth => "Import auth files or rebuild the registry.", - .switch_account => "Switch the active account interactively or by query.", - .remove_account => "Remove one or more accounts.", + .switch_account => "Switch the active account interactively, by query, or by list row number.", + .remove_account => "Remove one or more accounts by query or list row number.", .clean => "Delete backup and stale files under accounts/.", .config => "Manage auto-switch and usage API configuration.", .daemon => "Run the background auto-switch daemon.", @@ -717,7 +794,7 @@ fn writeUsageSection(out: *std.Io.Writer, topic: HelpTopic) !void { try out.writeAll(" codex-auth --help\n"); try out.writeAll(" codex-auth help \n"); }, - .list => try out.writeAll(" codex-auth list [--debug]\n"), + .list => try out.writeAll(" codex-auth list [--debug] [--api|--skip-api]\n"), .status => try out.writeAll(" codex-auth status\n"), .login => { try out.writeAll(" codex-auth login\n"); @@ -729,13 +806,13 @@ fn writeUsageSection(out: *std.Io.Writer, topic: HelpTopic) !void { try out.writeAll(" codex-auth import --purge []\n"); }, .switch_account => { - try out.writeAll(" codex-auth switch\n"); - try out.writeAll(" codex-auth switch \n"); + try out.writeAll(" codex-auth switch [--api|--skip-api]\n"); + try out.writeAll(" codex-auth switch [--api|--skip-api] \n"); }, .remove_account => { - try out.writeAll(" codex-auth remove\n"); - try out.writeAll(" codex-auth remove \n"); - try out.writeAll(" codex-auth remove --all\n"); + try out.writeAll(" codex-auth remove [--api|--skip-api]\n"); + try out.writeAll(" codex-auth remove [--api|--skip-api] [...]\n"); + try out.writeAll(" codex-auth remove [--api|--skip-api] --all\n"); }, .clean => try out.writeAll(" codex-auth clean\n"), .config => { @@ -764,6 +841,8 @@ fn writeExamplesSection(out: *std.Io.Writer, topic: HelpTopic) !void { .list => { try out.writeAll(" codex-auth list\n"); try out.writeAll(" codex-auth list --debug\n"); + try out.writeAll(" codex-auth list --api\n"); + try out.writeAll(" codex-auth list --skip-api\n"); }, .status => try out.writeAll(" codex-auth status\n"), .login => { @@ -777,11 +856,14 @@ fn writeExamplesSection(out: *std.Io.Writer, topic: HelpTopic) !void { }, .switch_account => { try out.writeAll(" codex-auth switch\n"); - try out.writeAll(" codex-auth switch john@example.com\n"); + try out.writeAll(" codex-auth switch work --api\n"); + try out.writeAll(" codex-auth switch 02 --skip-api\n"); }, .remove_account => { try out.writeAll(" codex-auth remove\n"); - try out.writeAll(" codex-auth remove john@example.com\n"); + try out.writeAll(" codex-auth remove --skip-api 01 03\n"); + try out.writeAll(" codex-auth remove --api work\n"); + try out.writeAll(" codex-auth remove john@example.com jane@example.com\n"); try out.writeAll(" codex-auth remove --all\n"); }, .clean => try out.writeAll(" codex-auth clean\n"), @@ -922,7 +1004,7 @@ pub fn printRemoveRequiresTtyError() !void { try writeErrorPrefixTo(out, use_color); try out.writeAll(" interactive remove requires a TTY.\n"); try writeHintPrefixTo(out, use_color); - try out.writeAll(" Use `codex-auth remove ` or `codex-auth remove --all` instead.\n"); + try out.writeAll(" Use `codex-auth remove ...` or `codex-auth remove --all` instead.\n"); try out.flush(); } @@ -1698,7 +1780,13 @@ fn renderSwitchList( const is_selected = selected != null and selected.? == selectable_counter; const is_active = row.is_active; if (use_color) { - if (is_selected) { + if (row.has_error) { + if (is_selected or is_active) { + try out.writeAll(ansi.bold_red); + } else { + try out.writeAll(ansi.red); + } + } else if (is_selected) { try out.writeAll(ansi.bold_green); } else if (is_active) { try out.writeAll(ansi.green); @@ -1777,7 +1865,13 @@ fn renderRemoveList( const is_checked = checked[selectable_counter]; const is_active = row.is_active; if (use_color) { - if (is_cursor) { + if (row.has_error) { + if (is_cursor or is_checked or is_active) { + try out.writeAll(ansi.bold_red); + } else { + try out.writeAll(ansi.red); + } + } else if (is_cursor) { try out.writeAll(ansi.bold_green); } else if (is_checked or is_active) { try out.writeAll(ansi.green); @@ -1871,6 +1965,7 @@ const SwitchRow = struct { last: []u8, depth: u8, is_active: bool, + has_error: bool, is_header: bool, fn deinit(self: *SwitchRow, allocator: std.mem.Allocator) void { @@ -1950,6 +2045,7 @@ fn buildSwitchRowsWithUsageOverrides( .last = last, .depth = display_row.depth, .is_active = display_row.is_active, + .has_error = usage_override != null, .is_header = false, }; widths.email = @max(widths.email, display_row.account_cell.len + (@as(usize, display_row.depth) * 2)); @@ -1967,6 +2063,7 @@ fn buildSwitchRowsWithUsageOverrides( .last = try allocator.dupe(u8, ""), .depth = display_row.depth, .is_active = false, + .has_error = false, .is_header = true, }; widths.email = @max(widths.email, display_row.account_cell.len + (@as(usize, display_row.depth) * 2)); @@ -2024,6 +2121,7 @@ fn buildSwitchRowsFromIndicesWithUsageOverrides( .last = last, .depth = display_row.depth, .is_active = display_row.is_active, + .has_error = usage_override != null, .is_header = false, }; widths.email = @max(widths.email, display_row.account_cell.len + (@as(usize, display_row.depth) * 2)); @@ -2041,6 +2139,7 @@ fn buildSwitchRowsFromIndicesWithUsageOverrides( .last = try allocator.dupe(u8, ""), .depth = display_row.depth, .is_active = false, + .has_error = false, .is_header = true, }; widths.email = @max(widths.email, display_row.account_cell.len + (@as(usize, display_row.depth) * 2)); @@ -2293,6 +2392,49 @@ test "Scenario: Given usage overrides when rendering remove list then failed row try std.testing.expect(std.mem.count(u8, output, "401") >= 2); } +test "Scenario: Given usage overrides when rendering switch list with color then failed rows are highlighted red" { + const gpa = std.testing.allocator; + var reg = makeTestRegistry(); + defer reg.deinit(gpa); + + try appendTestAccount(gpa, ®, "user-1::acc-1", "user@example.com", "", .team); + try appendTestAccount(gpa, ®, "user-1::acc-2", "user@example.com", "", .free); + + const usage_overrides = [_]?[]const u8{ null, "401" }; + var rows = try buildSwitchRowsWithUsageOverrides(gpa, ®, &usage_overrides); + defer rows.deinit(gpa); + + var buffer: [2048]u8 = undefined; + var writer: std.Io.Writer = .fixed(&buffer); + const idx_width = @max(@as(usize, 2), indexWidth(rows.selectable_row_indices.len)); + try renderSwitchList(&writer, ®, rows.items, idx_width, rows.widths, null, true); + + const output = writer.buffered(); + try std.testing.expect(std.mem.indexOf(u8, output, ansi.red) != null); +} + +test "Scenario: Given usage overrides when rendering remove list with color then failed rows are highlighted red" { + const gpa = std.testing.allocator; + var reg = makeTestRegistry(); + defer reg.deinit(gpa); + + try appendTestAccount(gpa, ®, "user-1::acc-1", "user@example.com", "", .team); + try appendTestAccount(gpa, ®, "user-1::acc-2", "user@example.com", "", .free); + + const usage_overrides = [_]?[]const u8{ null, "401" }; + var rows = try buildSwitchRowsWithUsageOverrides(gpa, ®, &usage_overrides); + defer rows.deinit(gpa); + + var checked = [_]bool{ false, false }; + var buffer: [2048]u8 = undefined; + var writer: std.Io.Writer = .fixed(&buffer); + const idx_width = @max(@as(usize, 2), indexWidth(rows.selectable_row_indices.len)); + try renderRemoveList(&writer, ®, rows.items, idx_width, rows.widths, null, &checked, true); + + const output = writer.buffered(); + try std.testing.expect(std.mem.indexOf(u8, output, ansi.red) != null); +} + test "Scenario: Given a usage snapshot plan when building switch rows then the displayed plan prefers it over the stored auth plan" { const gpa = std.testing.allocator; var reg = makeTestRegistry(); diff --git a/src/format.zig b/src/format.zig index f7416e9..3a8ee85 100644 --- a/src/format.zig +++ b/src/format.zig @@ -13,6 +13,8 @@ const c = @cImport({ const ansi = struct { const reset = "\x1b[0m"; const dim = "\x1b[2m"; + const red = "\x1b[31m"; + const bold_red = "\x1b[1;31m"; const green = "\x1b[32m"; }; @@ -180,7 +182,13 @@ fn writeAccountsTableWithUsageOverrides( const last_cell = try truncateAlloc(last, widths[4]); defer std.heap.page_allocator.free(last_cell); if (use_color) { - if (row.is_active) { + if (usage_override != null) { + if (row.is_active) { + try out.writeAll(ansi.bold_red); + } else { + try out.writeAll(ansi.red); + } + } else if (row.is_active) { try out.writeAll(ansi.green); } else { try out.writeAll(ansi.dim); @@ -753,6 +761,25 @@ test "writeAccountsTable shows usage override statuses for failed refreshes" { try std.testing.expect(std.mem.count(u8, output, "403") >= 2); } + +test "writeAccountsTable highlights usage override rows in red when color is enabled" { + const gpa = std.testing.allocator; + var reg = makeTestRegistry(); + defer reg.deinit(gpa); + + try appendTestAccount(gpa, ®, "user-1::acc-1", "user@example.com", "", .team); + try appendTestAccount(gpa, ®, "user-1::acc-2", "user@example.com", "", .free); + + const usage_overrides = [_]?[]const u8{ null, "403" }; + + var buffer: [4096]u8 = undefined; + var writer: std.Io.Writer = .fixed(&buffer); + try writeAccountsTableWithUsageOverrides(&writer, ®, true, &usage_overrides); + + const output = writer.buffered(); + try std.testing.expect(std.mem.indexOf(u8, output, ansi.red) != null); +} + test "writeAccountsTable prefers usage snapshot plan labels over stored auth plan" { const gpa = std.testing.allocator; var reg = makeTestRegistry(); diff --git a/src/main.zig b/src/main.zig index 1a34871..c1903a6 100644 --- a/src/main.zig +++ b/src/main.zig @@ -220,7 +220,19 @@ pub const ForegroundUsageRefreshTarget = enum { }; pub fn shouldRefreshForegroundUsage(target: ForegroundUsageRefreshTarget) bool { - return target == .list or target == .switch_account or target == .remove_account; + return target == .list or target == .switch_account; +} + +fn apiModeUsesApi(default_enabled: bool, api_mode: cli.ApiMode) bool { + return switch (api_mode) { + .default => default_enabled, + .force_api => true, + .skip_api => false, + }; +} + +fn apiModeUsesStoredDataOnly(api_mode: cli.ApiMode) bool { + return api_mode == .skip_api; } fn isAccountNameRefreshOnlyMode() bool { @@ -327,13 +339,14 @@ pub fn refreshForegroundUsageForDisplayWithApiFetcher( reg: *registry.Registry, usage_fetcher: UsageFetchDetailedFn, ) !ForegroundUsageRefreshState { - return refreshForegroundUsageForDisplayWithApiFetcherWithPoolInitAndDebug( + return refreshForegroundUsageForDisplayWithApiFetcherWithPoolInitAndDebugUsingApiEnabled( allocator, codex_home, reg, usage_fetcher, initForegroundUsagePool, null, + reg.api.usage, ); } @@ -344,13 +357,14 @@ pub fn refreshForegroundUsageForDisplayWithApiFetcherWithPoolInit( usage_fetcher: UsageFetchDetailedFn, pool_init: ForegroundUsagePoolInitFn, ) !ForegroundUsageRefreshState { - return refreshForegroundUsageForDisplayWithApiFetcherWithPoolInitAndDebug( + return refreshForegroundUsageForDisplayWithApiFetcherWithPoolInitAndDebugUsingApiEnabled( allocator, codex_home, reg, usage_fetcher, pool_init, null, + reg.api.usage, ); } @@ -361,6 +375,26 @@ pub fn refreshForegroundUsageForDisplayWithApiFetcherWithPoolInitAndDebug( usage_fetcher: UsageFetchDetailedFn, pool_init: ForegroundUsagePoolInitFn, debug_logger: ?*ForegroundUsageDebugLogger, +) !ForegroundUsageRefreshState { + return refreshForegroundUsageForDisplayWithApiFetcherWithPoolInitAndDebugUsingApiEnabled( + allocator, + codex_home, + reg, + usage_fetcher, + pool_init, + debug_logger, + reg.api.usage, + ); +} + +fn refreshForegroundUsageForDisplayWithApiFetcherWithPoolInitAndDebugUsingApiEnabled( + allocator: std.mem.Allocator, + codex_home: []const u8, + reg: *registry.Registry, + usage_fetcher: UsageFetchDetailedFn, + pool_init: ForegroundUsagePoolInitFn, + debug_logger: ?*ForegroundUsageDebugLogger, + usage_api_enabled: bool, ) !ForegroundUsageRefreshState { var state = try initForegroundUsageRefreshState(allocator, reg.accounts.items.len); errdefer state.deinit(allocator); @@ -370,7 +404,7 @@ pub fn refreshForegroundUsageForDisplayWithApiFetcherWithPoolInitAndDebug( var debug_context: ?ForegroundUsageDebugContext = null; - if (!reg.api.usage) { + if (!usage_api_enabled) { state.local_only_mode = true; if (try auto.refreshActiveUsage(allocator, codex_home, reg)) { try registry.saveRegistry(allocator, codex_home, reg); @@ -766,11 +800,40 @@ pub fn maybeRefreshForegroundAccountNames( reg: *registry.Registry, target: ForegroundUsageRefreshTarget, fetcher: AccountFetchFn, +) !void { + return try maybeRefreshForegroundAccountNamesWithAccountApiEnabled( + allocator, + codex_home, + reg, + target, + fetcher, + reg.api.account, + ); +} + +fn maybeRefreshForegroundAccountNamesWithAccountApiEnabled( + allocator: std.mem.Allocator, + codex_home: []const u8, + reg: *registry.Registry, + target: ForegroundUsageRefreshTarget, + fetcher: AccountFetchFn, + account_api_enabled: bool, ) !void { const changed = switch (target) { - .list => try refreshAccountNamesForList(allocator, codex_home, reg, fetcher), - .switch_account => try refreshAccountNamesAfterSwitch(allocator, codex_home, reg, fetcher), - .remove_account => false, + .list, .remove_account => try refreshAccountNamesForListWithAccountApiEnabled( + allocator, + codex_home, + reg, + fetcher, + account_api_enabled, + ), + .switch_account => try refreshAccountNamesAfterSwitchWithAccountApiEnabled( + allocator, + codex_home, + reg, + fetcher, + account_api_enabled, + ), }; if (!changed) return; try registry.saveRegistry(allocator, codex_home, reg); @@ -794,9 +857,25 @@ fn maybeRefreshAccountNamesForAuthInfo( reg: *registry.Registry, info: *const auth.AuthInfo, fetcher: AccountFetchFn, +) !bool { + return try maybeRefreshAccountNamesForAuthInfoWithAccountApiEnabled( + allocator, + reg, + info, + fetcher, + reg.api.account, + ); +} + +fn maybeRefreshAccountNamesForAuthInfoWithAccountApiEnabled( + allocator: std.mem.Allocator, + reg: *registry.Registry, + info: *const auth.AuthInfo, + fetcher: AccountFetchFn, + account_api_enabled: bool, ) !bool { const chatgpt_user_id = info.chatgpt_user_id orelse return false; - if (!shouldRefreshTeamAccountNamesForUserScope(reg, chatgpt_user_id)) return false; + if (!shouldRefreshTeamAccountNamesForUserScopeWithAccountApiEnabled(reg, chatgpt_user_id, account_api_enabled)) return false; const access_token = info.access_token orelse return false; const chatgpt_account_id = info.chatgpt_account_id orelse return false; @@ -829,13 +908,35 @@ fn refreshAccountNamesForActiveAuth( codex_home: []const u8, reg: *registry.Registry, fetcher: AccountFetchFn, +) !bool { + return try refreshAccountNamesForActiveAuthWithAccountApiEnabled( + allocator, + codex_home, + reg, + fetcher, + reg.api.account, + ); +} + +fn refreshAccountNamesForActiveAuthWithAccountApiEnabled( + allocator: std.mem.Allocator, + codex_home: []const u8, + reg: *registry.Registry, + fetcher: AccountFetchFn, + account_api_enabled: bool, ) !bool { const active_user_id = registry.activeChatgptUserId(reg) orelse return false; - if (!shouldRefreshTeamAccountNamesForUserScope(reg, active_user_id)) return false; + if (!shouldRefreshTeamAccountNamesForUserScopeWithAccountApiEnabled(reg, active_user_id, account_api_enabled)) return false; var info = (try loadActiveAuthInfoForAccountRefresh(allocator, codex_home)) orelse return false; defer info.deinit(allocator); - return try maybeRefreshAccountNamesForAuthInfo(allocator, reg, &info, fetcher); + return try maybeRefreshAccountNamesForAuthInfoWithAccountApiEnabled( + allocator, + reg, + &info, + fetcher, + account_api_enabled, + ); } pub fn refreshAccountNamesAfterLogin( @@ -853,7 +954,29 @@ pub fn refreshAccountNamesAfterSwitch( reg: *registry.Registry, fetcher: AccountFetchFn, ) !bool { - return try refreshAccountNamesForActiveAuth(allocator, codex_home, reg, fetcher); + return try refreshAccountNamesAfterSwitchWithAccountApiEnabled( + allocator, + codex_home, + reg, + fetcher, + reg.api.account, + ); +} + +fn refreshAccountNamesAfterSwitchWithAccountApiEnabled( + allocator: std.mem.Allocator, + codex_home: []const u8, + reg: *registry.Registry, + fetcher: AccountFetchFn, + account_api_enabled: bool, +) !bool { + return try refreshAccountNamesForActiveAuthWithAccountApiEnabled( + allocator, + codex_home, + reg, + fetcher, + account_api_enabled, + ); } pub fn refreshAccountNamesForList( @@ -862,11 +985,41 @@ pub fn refreshAccountNamesForList( reg: *registry.Registry, fetcher: AccountFetchFn, ) !bool { - return try refreshAccountNamesForActiveAuth(allocator, codex_home, reg, fetcher); + return try refreshAccountNamesForListWithAccountApiEnabled( + allocator, + codex_home, + reg, + fetcher, + reg.api.account, + ); +} + +fn refreshAccountNamesForListWithAccountApiEnabled( + allocator: std.mem.Allocator, + codex_home: []const u8, + reg: *registry.Registry, + fetcher: AccountFetchFn, + account_api_enabled: bool, +) !bool { + return try refreshAccountNamesForActiveAuthWithAccountApiEnabled( + allocator, + codex_home, + reg, + fetcher, + account_api_enabled, + ); } fn shouldRefreshTeamAccountNamesForUserScope(reg: *registry.Registry, chatgpt_user_id: []const u8) bool { - if (!reg.api.account) return false; + return shouldRefreshTeamAccountNamesForUserScopeWithAccountApiEnabled(reg, chatgpt_user_id, reg.api.account); +} + +fn shouldRefreshTeamAccountNamesForUserScopeWithAccountApiEnabled( + reg: *registry.Registry, + chatgpt_user_id: []const u8, + account_api_enabled: bool, +) bool { + if (!account_api_enabled) return false; return registry.shouldFetchTeamAccountNamesForUser(reg, chatgpt_user_id); } @@ -875,14 +1028,29 @@ fn shouldPreflightNodeForAccountNameRefresh( codex_home: []const u8, reg: *registry.Registry, target: ForegroundUsageRefreshTarget, +) !bool { + return try shouldPreflightNodeForAccountNameRefreshWithAccountApiEnabled( + allocator, + codex_home, + reg, + target, + reg.api.account, + ); +} + +fn shouldPreflightNodeForAccountNameRefreshWithAccountApiEnabled( + allocator: std.mem.Allocator, + codex_home: []const u8, + reg: *registry.Registry, + target: ForegroundUsageRefreshTarget, + account_api_enabled: bool, ) !bool { switch (target) { - .list, .switch_account => {}, - .remove_account => return false, + .list, .switch_account, .remove_account => {}, } const active_user_id = registry.activeChatgptUserId(reg) orelse return false; - if (!shouldRefreshTeamAccountNamesForUserScope(reg, active_user_id)) return false; + if (!shouldRefreshTeamAccountNamesForUserScopeWithAccountApiEnabled(reg, active_user_id, account_api_enabled)) return false; var info = (try loadActiveAuthInfoForAccountRefresh(allocator, codex_home)) orelse return false; defer info.deinit(allocator); @@ -896,8 +1064,32 @@ fn shouldPreflightNodeForForegroundTarget( reg: *registry.Registry, target: ForegroundUsageRefreshTarget, ) !bool { - if (shouldRefreshForegroundUsage(target) and reg.api.usage and reg.accounts.items.len > 0) return true; - return try shouldPreflightNodeForAccountNameRefresh(allocator, codex_home, reg, target); + return try shouldPreflightNodeForForegroundTargetWithApiEnabled( + allocator, + codex_home, + reg, + target, + reg.api.usage, + reg.api.account, + ); +} + +fn shouldPreflightNodeForForegroundTargetWithApiEnabled( + allocator: std.mem.Allocator, + codex_home: []const u8, + reg: *registry.Registry, + target: ForegroundUsageRefreshTarget, + usage_api_enabled: bool, + account_api_enabled: bool, +) !bool { + if (usage_api_enabled and reg.accounts.items.len > 0) return true; + return try shouldPreflightNodeForAccountNameRefreshWithAccountApiEnabled( + allocator, + codex_home, + reg, + target, + account_api_enabled, + ); } fn ensureForegroundNodeAvailable( @@ -906,11 +1098,31 @@ fn ensureForegroundNodeAvailable( reg: *registry.Registry, target: ForegroundUsageRefreshTarget, ) !void { - try ensureForegroundNodeAvailableWithChecker( + try ensureForegroundNodeAvailableWithApiEnabled( allocator, codex_home, reg, target, + reg.api.usage, + reg.api.account, + ); +} + +fn ensureForegroundNodeAvailableWithApiEnabled( + allocator: std.mem.Allocator, + codex_home: []const u8, + reg: *registry.Registry, + target: ForegroundUsageRefreshTarget, + usage_api_enabled: bool, + account_api_enabled: bool, +) !void { + try ensureForegroundNodeAvailableWithCheckerUsingApiEnabled( + allocator, + codex_home, + reg, + target, + usage_api_enabled, + account_api_enabled, chatgpt_http.ensureNodeExecutableAvailable, ); } @@ -922,7 +1134,34 @@ fn ensureForegroundNodeAvailableWithChecker( target: ForegroundUsageRefreshTarget, checker: NodeAvailabilityFn, ) !void { - if (!try shouldPreflightNodeForForegroundTarget(allocator, codex_home, reg, target)) return; + try ensureForegroundNodeAvailableWithCheckerUsingApiEnabled( + allocator, + codex_home, + reg, + target, + reg.api.usage, + reg.api.account, + checker, + ); +} + +fn ensureForegroundNodeAvailableWithCheckerUsingApiEnabled( + allocator: std.mem.Allocator, + codex_home: []const u8, + reg: *registry.Registry, + target: ForegroundUsageRefreshTarget, + usage_api_enabled: bool, + account_api_enabled: bool, + checker: NodeAvailabilityFn, +) !void { + if (!try shouldPreflightNodeForForegroundTargetWithApiEnabled( + allocator, + codex_home, + reg, + target, + usage_api_enabled, + account_api_enabled, + )) return; try checker(allocator); } @@ -1116,7 +1355,22 @@ fn handleList(allocator: std.mem.Allocator, codex_home: []const u8, opts: cli.Li if (try registry.syncActiveAccountFromAuth(allocator, codex_home, ®)) { try registry.saveRegistry(allocator, codex_home, ®); } - try ensureForegroundNodeAvailable(allocator, codex_home, ®, .list); + if (apiModeUsesStoredDataOnly(opts.api_mode)) { + try format.printAccounts(®); + return; + } + + const usage_api_enabled = apiModeUsesApi(reg.api.usage, opts.api_mode); + const account_api_enabled = apiModeUsesApi(reg.api.account, opts.api_mode); + + try ensureForegroundNodeAvailableWithApiEnabled( + allocator, + codex_home, + ®, + .list, + usage_api_enabled, + account_api_enabled, + ); var debug_stdout: io_util.Stdout = undefined; var debug_logger: ?ForegroundUsageDebugLogger = null; if (opts.debug) { @@ -1124,16 +1378,24 @@ fn handleList(allocator: std.mem.Allocator, codex_home: []const u8, opts: cli.Li debug_logger = ForegroundUsageDebugLogger.init(debug_stdout.out()); } - var usage_state = try refreshForegroundUsageForDisplayWithApiFetcherWithPoolInitAndDebug( + var usage_state = try refreshForegroundUsageForDisplayWithApiFetcherWithPoolInitAndDebugUsingApiEnabled( allocator, codex_home, ®, usage_api.fetchUsageForAuthPathDetailed, initForegroundUsagePool, if (debug_logger) |*logger| logger else null, + usage_api_enabled, ); defer usage_state.deinit(allocator); - try maybeRefreshForegroundAccountNames(allocator, codex_home, ®, .list, defaultAccountFetcher); + try maybeRefreshForegroundAccountNamesWithAccountApiEnabled( + allocator, + codex_home, + ®, + .list, + defaultAccountFetcher, + account_api_enabled, + ); try format.printAccountsWithUsageOverrides(®, usage_state.usage_overrides); } @@ -1206,22 +1468,54 @@ fn handleSwitch(allocator: std.mem.Allocator, codex_home: []const u8, opts: cli. try registry.saveRegistry(allocator, codex_home, ®); } if (opts.query) |query| { + const usage_api_enabled = apiModeUsesApi(false, opts.api_mode); + const account_api_enabled = apiModeUsesApi(false, opts.api_mode); + var usage_state: ?ForegroundUsageRefreshState = null; + defer if (usage_state) |*state| state.deinit(allocator); + + if (usage_api_enabled or account_api_enabled) { + try ensureForegroundNodeAvailableWithApiEnabled( + allocator, + codex_home, + ®, + .switch_account, + usage_api_enabled, + account_api_enabled, + ); + usage_state = try refreshForegroundUsageForDisplayWithApiFetcherWithPoolInitAndDebugUsingApiEnabled( + allocator, + codex_home, + ®, + usage_api.fetchUsageForAuthPathDetailed, + initForegroundUsagePool, + null, + usage_api_enabled, + ); + try maybeRefreshForegroundAccountNamesWithAccountApiEnabled( + allocator, + codex_home, + ®, + .switch_account, + defaultAccountFetcher, + account_api_enabled, + ); + } + var resolution = try resolveSwitchQueryLocally(allocator, ®, query); defer resolution.deinit(allocator); + const usage_overrides = if (usage_state) |*state| state.usage_overrides else null; const selected_account_key = switch (resolution) { .not_found => { try cli.printAccountNotFoundError(query); return error.AccountNotFound; }, - // Query-driven switching stays local-only so a direct target does not - // block on usage or account-name API refreshes before activation. .direct => |account_key| account_key, .multiple => |matches| try cli.selectAccountFromIndicesWithUsageOverrides( allocator, ®, matches.items, - null, + usage_overrides, ), }; if (selected_account_key == null) return; @@ -1229,15 +1523,44 @@ fn handleSwitch(allocator: std.mem.Allocator, codex_home: []const u8, opts: cli. try registry.saveRegistry(allocator, codex_home, ®); return; } - try ensureForegroundNodeAvailable(allocator, codex_home, ®, .switch_account); - var usage_state = try refreshForegroundUsageForDisplayWithApiFetcher( + + if (apiModeUsesStoredDataOnly(opts.api_mode)) { + const selected_account_key = try cli.selectAccount(allocator, ®); + if (selected_account_key == null) return; + try registry.activateAccountByKey(allocator, codex_home, ®, selected_account_key.?); + try registry.saveRegistry(allocator, codex_home, ®); + return; + } + + const usage_api_enabled = apiModeUsesApi(reg.api.usage, opts.api_mode); + const account_api_enabled = apiModeUsesApi(reg.api.account, opts.api_mode); + + try ensureForegroundNodeAvailableWithApiEnabled( + allocator, + codex_home, + ®, + .switch_account, + usage_api_enabled, + account_api_enabled, + ); + var usage_state = try refreshForegroundUsageForDisplayWithApiFetcherWithPoolInitAndDebugUsingApiEnabled( allocator, codex_home, ®, usage_api.fetchUsageForAuthPathDetailed, + initForegroundUsagePool, + null, + usage_api_enabled, ); defer usage_state.deinit(allocator); - try maybeRefreshForegroundAccountNames(allocator, codex_home, ®, .switch_account, defaultAccountFetcher); + try maybeRefreshForegroundAccountNamesWithAccountApiEnabled( + allocator, + codex_home, + ®, + .switch_account, + defaultAccountFetcher, + account_api_enabled, + ); const selected_account_key = try cli.selectAccountWithUsageOverrides(allocator, ®, usage_state.usage_overrides); if (selected_account_key == null) return; @@ -1251,6 +1574,10 @@ pub fn resolveSwitchQueryLocally( reg: *registry.Registry, query: []const u8, ) !SwitchQueryResolution { + if (try findAccountIndexByDisplayNumber(allocator, reg, query)) |account_idx| { + return .{ .direct = reg.accounts.items[account_idx].account_key }; + } + var matches = try findMatchingAccounts(allocator, reg, query); if (matches.items.len == 0) { matches.deinit(allocator); @@ -1294,6 +1621,32 @@ pub fn findMatchingAccounts( return matches; } +fn parseDisplayNumber(selector: []const u8) ?usize { + if (selector.len == 0) return null; + for (selector) |ch| { + if (ch < '0' or ch > '9') return null; + } + + const parsed = std.fmt.parseInt(usize, selector, 10) catch return null; + if (parsed == 0) return null; + return parsed; +} + +fn findAccountIndexByDisplayNumber( + allocator: std.mem.Allocator, + reg: *registry.Registry, + selector: []const u8, +) !?usize { + const display_number = parseDisplayNumber(selector) orelse return null; + + var display = try display_rows.buildDisplayRows(allocator, reg, null); + defer display.deinit(allocator); + + if (display_number > display.selectable_row_indices.len) return null; + const row_idx = display.selectable_row_indices[display_number - 1]; + return display.rows[row_idx].account_index; +} + const CurrentAuthState = struct { record_key: ?[]u8, syncable: bool, @@ -1388,35 +1741,74 @@ fn handleRemove(allocator: std.mem.Allocator, codex_home: []const u8, opts: cli. try registry.saveRegistry(allocator, codex_home, ®); } - const needs_selector = !opts.all and opts.query == null; var usage_state: ?ForegroundUsageRefreshState = null; defer if (usage_state) |*state| state.deinit(allocator); - - if (needs_selector) { - try ensureForegroundNodeAvailable(allocator, codex_home, ®, .remove_account); - usage_state = try refreshForegroundUsageForDisplayWithApiFetcher( + if (opts.api_mode == .force_api) { + try ensureForegroundNodeAvailableWithApiEnabled( + allocator, + codex_home, + ®, + .remove_account, + true, + true, + ); + usage_state = try refreshForegroundUsageForDisplayWithApiFetcherWithPoolInitAndDebugUsingApiEnabled( allocator, codex_home, ®, usage_api.fetchUsageForAuthPathDetailed, + initForegroundUsagePool, + null, + true, + ); + try maybeRefreshForegroundAccountNamesWithAccountApiEnabled( + allocator, + codex_home, + ®, + .remove_account, + defaultAccountFetcher, + true, ); } + const usage_overrides = if (usage_state) |*state| state.usage_overrides else null; var selected: ?[]usize = null; if (opts.all) { selected = try allocator.alloc(usize, reg.accounts.items.len); for (selected.?, 0..) |*slot, idx| slot.* = idx; - } else if (opts.query) |query| { - var matches = try findMatchingAccounts(allocator, ®, query); - defer matches.deinit(allocator); + } else if (opts.selectors.len != 0) { + var selected_list = std.ArrayList(usize).empty; + defer selected_list.deinit(allocator); + var requires_confirmation = false; + + for (opts.selectors) |selector| { + if (try findAccountIndexByDisplayNumber(allocator, ®, selector)) |account_idx| { + if (!selectionContainsIndex(selected_list.items, account_idx)) { + try selected_list.append(allocator, account_idx); + } + continue; + } - if (matches.items.len == 0) { - try cli.printAccountNotFoundError(query); - return error.AccountNotFound; + var matches = try findMatchingAccounts(allocator, ®, selector); + defer matches.deinit(allocator); + + if (matches.items.len == 0) { + try cli.printAccountNotFoundError(selector); + return error.AccountNotFound; + } + if (matches.items.len > 1) { + requires_confirmation = true; + } + for (matches.items) |account_idx| { + if (!selectionContainsIndex(selected_list.items, account_idx)) { + try selected_list.append(allocator, account_idx); + } + } } - if (matches.items.len > 1) { - var matched_labels = try cli.buildRemoveLabels(allocator, ®, matches.items); + if (selected_list.items.len == 0) return; + if (requires_confirmation) { + var matched_labels = try cli.buildRemoveLabels(allocator, ®, selected_list.items); defer { freeOwnedStrings(allocator, matched_labels.items); matched_labels.deinit(allocator); @@ -1428,12 +1820,12 @@ fn handleRemove(allocator: std.mem.Allocator, codex_home: []const u8, opts: cli. if (!(try cli.confirmRemoveMatches(matched_labels.items))) return; } - selected = try allocator.dupe(usize, matches.items); + selected = try allocator.dupe(usize, selected_list.items); } else { selected = cli.selectAccountsToRemoveWithUsageOverrides( allocator, ®, - if (usage_state) |*state| state.usage_overrides else null, + usage_overrides, ) catch |err| switch (err) { error.InvalidRemoveSelectionInput => { try cli.printInvalidRemoveSelectionError(); @@ -1733,6 +2125,114 @@ test "foreground node preflight fails fast when account-name refresh needs node" try std.testing.expectEqual(@as(usize, 1), TestState.check_count); } +test "foreground node preflight can be forced on when command api override enables usage refresh" { + const TestState = struct { + var check_count: usize = 0; + + fn missingNode(allocator: std.mem.Allocator) !void { + _ = allocator; + check_count += 1; + return error.NodeJsRequired; + } + }; + + const gpa = std.testing.allocator; + var tmp = fs.tmpDir(.{}); + defer tmp.cleanup(); + + const codex_home = try tmp.dir.realpathAlloc(gpa, "."); + defer gpa.free(codex_home); + + var reg = registry.Registry{ + .schema_version = registry.current_schema_version, + .active_account_key = null, + .active_account_activated_at_ms = null, + .auto_switch = registry.defaultAutoSwitchConfig(), + .api = registry.defaultApiConfig(), + .accounts = std.ArrayList(registry.AccountRecord).empty, + }; + defer reg.deinit(gpa); + reg.api.usage = false; + reg.api.account = false; + + try reg.accounts.append(gpa, .{ + .account_key = try gpa.dupe(u8, "user-1::acct-1"), + .chatgpt_account_id = try gpa.dupe(u8, "acct-1"), + .chatgpt_user_id = try gpa.dupe(u8, "user-1"), + .email = try gpa.dupe(u8, "alpha@example.com"), + .alias = try gpa.dupe(u8, ""), + .account_name = null, + .plan = .plus, + .auth_mode = .chatgpt, + .created_at = 1, + .last_used_at = null, + .last_usage = null, + .last_usage_at = null, + .last_local_rollout = null, + }); + + TestState.check_count = 0; + try std.testing.expectError( + error.NodeJsRequired, + ensureForegroundNodeAvailableWithCheckerUsingApiEnabled(gpa, codex_home, ®, .list, true, false, TestState.missingNode), + ); + try std.testing.expectEqual(@as(usize, 1), TestState.check_count); +} + +test "foreground node preflight can be forced on for remove when command api override enables usage refresh" { + const TestState = struct { + var check_count: usize = 0; + + fn missingNode(allocator: std.mem.Allocator) !void { + _ = allocator; + check_count += 1; + return error.NodeJsRequired; + } + }; + + const gpa = std.testing.allocator; + var tmp = fs.tmpDir(.{}); + defer tmp.cleanup(); + + const codex_home = try tmp.dir.realpathAlloc(gpa, "."); + defer gpa.free(codex_home); + + var reg = registry.Registry{ + .schema_version = registry.current_schema_version, + .active_account_key = null, + .active_account_activated_at_ms = null, + .auto_switch = registry.defaultAutoSwitchConfig(), + .api = registry.defaultApiConfig(), + .accounts = std.ArrayList(registry.AccountRecord).empty, + }; + defer reg.deinit(gpa); + reg.api.usage = false; + reg.api.account = false; + + try reg.accounts.append(gpa, .{ + .account_key = try gpa.dupe(u8, "user-1::acct-1"), + .chatgpt_account_id = try gpa.dupe(u8, "acct-1"), + .chatgpt_user_id = try gpa.dupe(u8, "user-1"), + .email = try gpa.dupe(u8, "alpha@example.com"), + .alias = try gpa.dupe(u8, ""), + .account_name = null, + .plan = .plus, + .auth_mode = .chatgpt, + .created_at = 1, + .last_used_at = null, + .last_usage = null, + .last_usage_at = null, + .last_local_rollout = null, + }); + + TestState.check_count = 0; + try std.testing.expectError( + error.NodeJsRequired, + ensureForegroundNodeAvailableWithCheckerUsingApiEnabled(gpa, codex_home, ®, .remove_account, true, false, TestState.missingNode), + ); + try std.testing.expectEqual(@as(usize, 1), TestState.check_count); +} + test "handled cli errors include missing node" { try std.testing.expect(isHandledCliError(error.NodeJsRequired)); } diff --git a/src/tests/cli_bdd_test.zig b/src/tests/cli_bdd_test.zig index c407779..5126083 100644 --- a/src/tests/cli_bdd_test.zig +++ b/src/tests/cli_bdd_test.zig @@ -182,6 +182,36 @@ test "Scenario: Given list with debug flag when parsing then debug mode is prese } } +test "Scenario: Given list with skip-api flag when parsing then local-only display mode is preserved" { + const gpa = std.testing.allocator; + const args = [_][:0]const u8{ "codex-auth", "list", "--skip-api" }; + var result = try cli.parseArgs(gpa, &args); + defer cli.freeParseResult(gpa, &result); + + switch (result) { + .command => |cmd| switch (cmd) { + .list => |opts| try std.testing.expectEqual(cli.ApiMode.skip_api, opts.api_mode), + else => return error.TestExpectedEqual, + }, + else => return error.TestExpectedEqual, + } +} + +test "Scenario: Given list with api flag when parsing then forced api mode is preserved" { + const gpa = std.testing.allocator; + const args = [_][:0]const u8{ "codex-auth", "list", "--api" }; + var result = try cli.parseArgs(gpa, &args); + defer cli.freeParseResult(gpa, &result); + + switch (result) { + .command => |cmd| switch (cmd) { + .list => |opts| try std.testing.expectEqual(cli.ApiMode.force_api, opts.api_mode), + else => return error.TestExpectedEqual, + }, + else => return error.TestExpectedEqual, + } +} + test "Scenario: Given login with removed no-login flag when parsing then usage error is returned" { const gpa = std.testing.allocator; const args = [_][:0]const u8{ "codex-auth", "login", "--no-login" }; @@ -256,7 +286,7 @@ test "Scenario: Given help when rendering then login and command help notes are try std.testing.expect(std.mem.indexOf(u8, help, "`config api enable` may trigger OpenAI account restrictions or suspension in some environments.") != null); try std.testing.expect(std.mem.indexOf(u8, help, "login") != null); try std.testing.expect(std.mem.indexOf(u8, help, "clean") != null); - try std.testing.expect(std.mem.indexOf(u8, help, "remove [|--all]") != null); + try std.testing.expect(std.mem.indexOf(u8, help, "remove [...] [--all] [--api|--skip-api]") != null); try std.testing.expect(std.mem.indexOf(u8, help, "Delete backup and stale files under accounts/") != null); try std.testing.expect(std.mem.indexOf(u8, help, "status") != null); try std.testing.expect(std.mem.indexOf(u8, help, "config") != null); @@ -279,7 +309,7 @@ test "Scenario: Given simple command help when rendering then examples are omitt const help = aw.written(); try std.testing.expect(std.mem.indexOf(u8, help, "codex-auth list") != null); try std.testing.expect(std.mem.indexOf(u8, help, "List available accounts.") != null); - try std.testing.expect(std.mem.indexOf(u8, help, "Usage:\n codex-auth list [--debug]\n") != null); + try std.testing.expect(std.mem.indexOf(u8, help, "Usage:\n codex-auth list [--debug] [--api|--skip-api]\n") != null); try std.testing.expect(std.mem.indexOf(u8, help, "Examples:") == null); } @@ -674,6 +704,45 @@ test "Scenario: Given switch with positional query when parsing then non-interac .switch_account => |opts| { try std.testing.expect(opts.query != null); try std.testing.expect(std.mem.eql(u8, opts.query.?, "user@example.com")); + try std.testing.expectEqual(cli.ApiMode.default, opts.api_mode); + }, + else => return error.TestExpectedEqual, + }, + else => return error.TestExpectedEqual, + } +} + +test "Scenario: Given switch with skip-api flag when parsing then local-only picker mode is preserved" { + const gpa = std.testing.allocator; + const args = [_][:0]const u8{ "codex-auth", "switch", "--skip-api", "02" }; + var result = try cli.parseArgs(gpa, &args); + defer cli.freeParseResult(gpa, &result); + + switch (result) { + .command => |cmd| switch (cmd) { + .switch_account => |opts| { + try std.testing.expectEqual(cli.ApiMode.skip_api, opts.api_mode); + try std.testing.expect(opts.query != null); + try std.testing.expect(std.mem.eql(u8, opts.query.?, "02")); + }, + else => return error.TestExpectedEqual, + }, + else => return error.TestExpectedEqual, + } +} + +test "Scenario: Given switch with api flag when parsing then forced api mode is preserved" { + const gpa = std.testing.allocator; + const args = [_][:0]const u8{ "codex-auth", "switch", "--api", "02" }; + var result = try cli.parseArgs(gpa, &args); + defer cli.freeParseResult(gpa, &result); + + switch (result) { + .command => |cmd| switch (cmd) { + .switch_account => |opts| { + try std.testing.expectEqual(cli.ApiMode.force_api, opts.api_mode); + try std.testing.expect(opts.query != null); + try std.testing.expect(std.mem.eql(u8, opts.query.?, "02")); }, else => return error.TestExpectedEqual, }, @@ -699,7 +768,7 @@ test "Scenario: Given switch with unexpected flag when parsing then usage error try expectUsageError(result, .switch_account, "unknown flag"); } -test "Scenario: Given remove with positional query when parsing then query mode is preserved" { +test "Scenario: Given remove with positional query when parsing then selector mode is preserved" { const gpa = std.testing.allocator; const args = [_][:0]const u8{ "codex-auth", "remove", "user@example.com" }; var result = try cli.parseArgs(gpa, &args); @@ -708,9 +777,10 @@ test "Scenario: Given remove with positional query when parsing then query mode switch (result) { .command => |cmd| switch (cmd) { .remove_account => |opts| { - try std.testing.expect(opts.query != null); - try std.testing.expect(std.mem.eql(u8, opts.query.?, "user@example.com")); + try std.testing.expectEqual(@as(usize, 1), opts.selectors.len); + try std.testing.expect(std.mem.eql(u8, opts.selectors[0], "user@example.com")); try std.testing.expect(!opts.all); + try std.testing.expectEqual(cli.ApiMode.default, opts.api_mode); }, else => return error.TestExpectedEqual, }, @@ -727,8 +797,31 @@ test "Scenario: Given remove with all flag when parsing then all mode is preserv switch (result) { .command => |cmd| switch (cmd) { .remove_account => |opts| { - try std.testing.expect(opts.query == null); + try std.testing.expectEqual(@as(usize, 0), opts.selectors.len); try std.testing.expect(opts.all); + try std.testing.expectEqual(cli.ApiMode.default, opts.api_mode); + }, + else => return error.TestExpectedEqual, + }, + else => return error.TestExpectedEqual, + } +} + +test "Scenario: Given remove with multiple selectors when parsing then all selectors are preserved" { + const gpa = std.testing.allocator; + const args = [_][:0]const u8{ "codex-auth", "remove", "01", "b@example.com", "03" }; + var result = try cli.parseArgs(gpa, &args); + defer cli.freeParseResult(gpa, &result); + + switch (result) { + .command => |cmd| switch (cmd) { + .remove_account => |opts| { + try std.testing.expectEqual(@as(usize, 3), opts.selectors.len); + try std.testing.expect(std.mem.eql(u8, opts.selectors[0], "01")); + try std.testing.expect(std.mem.eql(u8, opts.selectors[1], "b@example.com")); + try std.testing.expect(std.mem.eql(u8, opts.selectors[2], "03")); + try std.testing.expect(!opts.all); + try std.testing.expectEqual(cli.ApiMode.default, opts.api_mode); }, else => return error.TestExpectedEqual, }, @@ -736,13 +829,42 @@ test "Scenario: Given remove with all flag when parsing then all mode is preserv } } -test "Scenario: Given remove with duplicate targets when parsing then usage error is returned" { +test "Scenario: Given remove with skip-api flag when parsing then explicit local-only mode is preserved" { const gpa = std.testing.allocator; - const args = [_][:0]const u8{ "codex-auth", "remove", "a@example.com", "b@example.com" }; + const args = [_][:0]const u8{ "codex-auth", "remove", "--skip-api", "01" }; var result = try cli.parseArgs(gpa, &args); defer cli.freeParseResult(gpa, &result); - try expectUsageError(result, .remove_account, "unexpected extra selector"); + switch (result) { + .command => |cmd| switch (cmd) { + .remove_account => |opts| { + try std.testing.expectEqual(cli.ApiMode.skip_api, opts.api_mode); + try std.testing.expectEqual(@as(usize, 1), opts.selectors.len); + try std.testing.expect(std.mem.eql(u8, opts.selectors[0], "01")); + }, + else => return error.TestExpectedEqual, + }, + else => return error.TestExpectedEqual, + } +} + +test "Scenario: Given remove with api flag when parsing then forced api mode is preserved" { + const gpa = std.testing.allocator; + const args = [_][:0]const u8{ "codex-auth", "remove", "--api", "work" }; + var result = try cli.parseArgs(gpa, &args); + defer cli.freeParseResult(gpa, &result); + + switch (result) { + .command => |cmd| switch (cmd) { + .remove_account => |opts| { + try std.testing.expectEqual(cli.ApiMode.force_api, opts.api_mode); + try std.testing.expectEqual(@as(usize, 1), opts.selectors.len); + try std.testing.expect(std.mem.eql(u8, opts.selectors[0], "work")); + }, + else => return error.TestExpectedEqual, + }, + else => return error.TestExpectedEqual, + } } test "Scenario: Given remove with unexpected flag when parsing then usage error is returned" { diff --git a/src/tests/e2e_cli_test.zig b/src/tests/e2e_cli_test.zig index 2b8b5e4..bd9f31b 100644 --- a/src/tests/e2e_cli_test.zig +++ b/src/tests/e2e_cli_test.zig @@ -286,6 +286,76 @@ fn runCliWithIsolatedHomeAndPath( return try runCapture(allocator, project_root, &env_map, argv.items); } +fn runCliWithIsolatedHomeAndPathAndStdin( + allocator: std.mem.Allocator, + project_root: []const u8, + home_root: []const u8, + path_override: []const u8, + args: []const []const u8, + stdin_data: []const u8, +) !std.process.RunResult { + const exe_path = try builtCliPathAlloc(allocator, project_root); + defer allocator.free(exe_path); + + var argv = std.ArrayList([]const u8).empty; + defer argv.deinit(allocator); + try argv.append(allocator, exe_path); + try argv.appendSlice(allocator, args); + + var env_map = try process_compat.getEnvMap(allocator); + defer env_map.deinit(); + try env_map.put("HOME", home_root); + try env_map.put("USERPROFILE", home_root); + _ = env_map.swapRemove("CODEX_HOME"); + try env_map.put("PATH", path_override); + try env_map.put("CODEX_AUTH_SKIP_SERVICE_RECONCILE", "1"); + try env_map.put("CODEX_AUTH_DISABLE_BACKGROUND_ACCOUNT_NAME_REFRESH", "1"); + + var child = std.process.spawn(fs.io(), .{ + .argv = argv.items, + .cwd = .{ .path = project_root }, + .environ_map = &env_map, + .stdin = .pipe, + .stdout = .pipe, + .stderr = .pipe, + }) catch |err| switch (err) { + error.OutOfMemory => return error.SkipZigTest, + else => return err, + }; + defer child.kill(fs.io()); + + if (child.stdin) |stdin_pipe| { + try fs.wrapFile(stdin_pipe).writeAll(stdin_data); + fs.wrapFile(stdin_pipe).close(); + child.stdin = null; + } + + var multi_reader_buffer: std.Io.File.MultiReader.Buffer(2) = undefined; + var multi_reader: std.Io.File.MultiReader = undefined; + multi_reader.init(allocator, fs.io(), multi_reader_buffer.toStreams(), &.{ child.stdout.?, child.stderr.? }); + defer multi_reader.deinit(); + + const stdout_reader = multi_reader.reader(0); + const stderr_reader = multi_reader.reader(1); + + while (multi_reader.fill(64, .none)) |_| { + if (stdout_reader.buffered().len > 1024 * 1024) return error.StreamTooLong; + if (stderr_reader.buffered().len > 1024 * 1024) return error.StreamTooLong; + } else |err| switch (err) { + error.EndOfStream => {}, + else => |e| return e, + } + + try multi_reader.checkAnyError(); + const term = try child.wait(fs.io()); + + return .{ + .stdout = try multi_reader.toOwnedSlice(0), + .stderr = try multi_reader.toOwnedSlice(1), + .term = term, + }; +} + fn runCliWithIsolatedHomeAndStdin( allocator: std.mem.Allocator, project_root: []const u8, @@ -417,6 +487,22 @@ fn seedRegistryWithAccounts( try registry.saveRegistry(allocator, codex_home, ®); } +fn setRegistryApiConfig( + allocator: std.mem.Allocator, + home_root: []const u8, + usage_enabled: bool, + account_enabled: bool, +) !void { + const codex_home = try codexHomeAlloc(allocator, home_root); + defer allocator.free(codex_home); + + var reg = try registry.loadRegistry(allocator, codex_home); + defer reg.deinit(allocator); + reg.api.usage = usage_enabled; + reg.api.account = account_enabled; + try registry.saveRegistry(allocator, codex_home, ®); +} + fn appendCustomAccount( allocator: std.mem.Allocator, reg: *registry.Registry, @@ -1264,6 +1350,184 @@ test "Scenario: Given switch query with a direct local match when running switch try std.testing.expect(std.mem.eql(u8, loaded.active_account_key.?, backup_key)); } +test "Scenario: Given list with api override when api config is disabled then it still requires api refresh executables" { + const gpa = std.testing.allocator; + const project_root = try projectRootAlloc(gpa); + defer gpa.free(project_root); + try buildCliBinary(gpa, project_root); + + var tmp = fs.tmpDir(.{}); + defer tmp.cleanup(); + + const home_root = try tmp.dir.realpathAlloc(gpa, "."); + defer gpa.free(home_root); + + try seedRegistryWithAccounts(gpa, home_root, "alpha@example.com", &[_]SeedAccount{ + .{ .email = "alpha@example.com", .alias = "alpha" }, + }); + try setRegistryApiConfig(gpa, home_root, false, false); + + try tmp.dir.makePath("empty-bin"); + const empty_path = try tmp.dir.realpathAlloc(gpa, "empty-bin"); + defer gpa.free(empty_path); + + const result = try runCliWithIsolatedHomeAndPath( + gpa, + project_root, + home_root, + empty_path, + &[_][]const u8{ "list", "--api" }, + ); + defer gpa.free(result.stdout); + defer gpa.free(result.stderr); + + try expectFailure(result); + try std.testing.expect(std.mem.indexOf(u8, result.stderr, "Node.js 22+") != null); +} + +test "Scenario: Given list with skip-api when running list then it does not require api refresh executables" { + const gpa = std.testing.allocator; + const project_root = try projectRootAlloc(gpa); + defer gpa.free(project_root); + try buildCliBinary(gpa, project_root); + + var tmp = fs.tmpDir(.{}); + defer tmp.cleanup(); + + const home_root = try tmp.dir.realpathAlloc(gpa, "."); + defer gpa.free(home_root); + + try seedRegistryWithAccounts(gpa, home_root, "alpha@example.com", &[_]SeedAccount{ + .{ .email = "alpha@example.com", .alias = "alpha" }, + .{ .email = "beta@example.com", .alias = "beta" }, + }); + + try tmp.dir.makePath("empty-bin"); + const empty_path = try tmp.dir.realpathAlloc(gpa, "empty-bin"); + defer gpa.free(empty_path); + + const result = try runCliWithIsolatedHomeAndPath( + gpa, + project_root, + home_root, + empty_path, + &[_][]const u8{ "list", "--skip-api" }, + ); + defer gpa.free(result.stdout); + defer gpa.free(result.stderr); + + try expectSuccess(result); + try std.testing.expect(std.mem.indexOf(u8, result.stdout, "ACCOUNT") != null); + try std.testing.expect(std.mem.indexOf(u8, result.stdout, "alpha@example.com") != null); + try std.testing.expect(std.mem.indexOf(u8, result.stdout, "beta@example.com") != null); + try std.testing.expectEqualStrings("", result.stderr); +} + +test "Scenario: Given switch query with api override when api config is disabled then it still requires api refresh executables" { + const gpa = std.testing.allocator; + const project_root = try projectRootAlloc(gpa); + defer gpa.free(project_root); + try buildCliBinary(gpa, project_root); + + var tmp = fs.tmpDir(.{}); + defer tmp.cleanup(); + + const home_root = try tmp.dir.realpathAlloc(gpa, "."); + defer gpa.free(home_root); + + try seedRegistryWithAccounts(gpa, home_root, "active@example.com", &[_]SeedAccount{ + .{ .email = "active@example.com", .alias = "active" }, + .{ .email = "backup@example.com", .alias = "backup" }, + }); + try setRegistryApiConfig(gpa, home_root, false, false); + + try tmp.dir.makePath("empty-bin"); + const empty_path = try tmp.dir.realpathAlloc(gpa, "empty-bin"); + defer gpa.free(empty_path); + + const result = try runCliWithIsolatedHomeAndPath( + gpa, + project_root, + home_root, + empty_path, + &[_][]const u8{ "switch", "--api", "02" }, + ); + defer gpa.free(result.stdout); + defer gpa.free(result.stderr); + + try expectFailure(result); + try std.testing.expect(std.mem.indexOf(u8, result.stderr, "Node.js 22+") != null); +} + +test "Scenario: Given switch with skip-api when running interactively then it does not require api refresh executables" { + const gpa = std.testing.allocator; + const project_root = try projectRootAlloc(gpa); + defer gpa.free(project_root); + try buildCliBinary(gpa, project_root); + + var tmp = fs.tmpDir(.{}); + defer tmp.cleanup(); + + const home_root = try tmp.dir.realpathAlloc(gpa, "."); + defer gpa.free(home_root); + + try seedRegistryWithAccounts(gpa, home_root, "active@example.com", &[_]SeedAccount{ + .{ .email = "active@example.com", .alias = "active" }, + .{ .email = "backup@example.com", .alias = "backup" }, + }); + + const codex_home = try codexHomeAlloc(gpa, home_root); + defer gpa.free(codex_home); + const active_auth_path = try authJsonPathAlloc(gpa, home_root); + defer gpa.free(active_auth_path); + + const active_key = try bdd.accountKeyForEmailAlloc(gpa, "active@example.com"); + defer gpa.free(active_key); + const backup_key = try bdd.accountKeyForEmailAlloc(gpa, "backup@example.com"); + defer gpa.free(backup_key); + const active_snapshot_path = try registry.accountAuthPath(gpa, codex_home, active_key); + defer gpa.free(active_snapshot_path); + const backup_snapshot_path = try registry.accountAuthPath(gpa, codex_home, backup_key); + defer gpa.free(backup_snapshot_path); + + const active_auth = try bdd.authJsonWithEmailPlan(gpa, "active@example.com", "team"); + defer gpa.free(active_auth); + const backup_auth = try bdd.authJsonWithEmailPlan(gpa, "backup@example.com", "plus"); + defer gpa.free(backup_auth); + + try tmp.dir.writeFile(.{ .sub_path = ".codex/auth.json", .data = active_auth }); + try fs.cwd().writeFile(.{ .sub_path = active_snapshot_path, .data = active_auth }); + try fs.cwd().writeFile(.{ .sub_path = backup_snapshot_path, .data = backup_auth }); + + try tmp.dir.makePath("empty-bin"); + const empty_path = try tmp.dir.realpathAlloc(gpa, "empty-bin"); + defer gpa.free(empty_path); + + const result = try runCliWithIsolatedHomeAndPathAndStdin( + gpa, + project_root, + home_root, + empty_path, + &[_][]const u8{ "switch", "--skip-api" }, + "2\n", + ); + defer gpa.free(result.stdout); + defer gpa.free(result.stderr); + + try expectSuccess(result); + try std.testing.expect(std.mem.indexOf(u8, result.stdout, "Select account to activate:") != null); + try std.testing.expectEqualStrings("", result.stderr); + + const auth_after = try bdd.readFileAlloc(gpa, active_auth_path); + defer gpa.free(auth_after); + try std.testing.expectEqualStrings(backup_auth, auth_after); + + var loaded = try registry.loadRegistry(gpa, codex_home); + defer loaded.deinit(gpa); + try std.testing.expect(loaded.active_account_key != null); + try std.testing.expect(std.mem.eql(u8, loaded.active_account_key.?, backup_key)); +} + test "Scenario: Given remove query with one match when running remove then it deletes immediately and prints a summary" { const gpa = std.testing.allocator; const project_root = try projectRootAlloc(gpa); @@ -1328,6 +1592,131 @@ test "Scenario: Given remove query with one match when running remove then it de keeper_backup.close(); } +test "Scenario: Given remove with multiple selectors when running remove then it deletes all selected accounts" { + const gpa = std.testing.allocator; + const project_root = try projectRootAlloc(gpa); + defer gpa.free(project_root); + try buildCliBinary(gpa, project_root); + + var tmp = fs.tmpDir(.{}); + defer tmp.cleanup(); + + const home_root = try tmp.dir.realpathAlloc(gpa, "."); + defer gpa.free(home_root); + + try seedRegistryWithAccounts(gpa, home_root, "beta@example.com", &[_]SeedAccount{ + .{ .email = "alpha@example.com", .alias = "" }, + .{ .email = "beta@example.com", .alias = "" }, + .{ .email = "keeper@example.com", .alias = "" }, + }); + + const codex_home = try codexHomeAlloc(gpa, home_root); + defer gpa.free(codex_home); + + const result = try runCliWithIsolatedHomeAndStdin( + gpa, + project_root, + home_root, + &[_][]const u8{ "remove", "01", "keeper@example.com" }, + "", + ); + defer gpa.free(result.stdout); + defer gpa.free(result.stderr); + + try expectSuccess(result); + try std.testing.expectEqualStrings( + "Removed 2 account(s): alpha@example.com, keeper@example.com\n", + result.stdout, + ); + try std.testing.expectEqualStrings("", result.stderr); + + var loaded = try registry.loadRegistry(gpa, codex_home); + defer loaded.deinit(gpa); + try std.testing.expectEqual(@as(usize, 1), loaded.accounts.items.len); + try std.testing.expect(std.mem.eql(u8, loaded.accounts.items[0].email, "beta@example.com")); +} + +test "Scenario: Given remove with api override when api config is disabled then it still requires api refresh executables" { + const gpa = std.testing.allocator; + const project_root = try projectRootAlloc(gpa); + defer gpa.free(project_root); + try buildCliBinary(gpa, project_root); + + var tmp = fs.tmpDir(.{}); + defer tmp.cleanup(); + + const home_root = try tmp.dir.realpathAlloc(gpa, "."); + defer gpa.free(home_root); + + try seedRegistryWithAccounts(gpa, home_root, "alpha@example.com", &[_]SeedAccount{ + .{ .email = "alpha@example.com", .alias = "" }, + .{ .email = "beta@example.com", .alias = "" }, + }); + try setRegistryApiConfig(gpa, home_root, false, false); + + try tmp.dir.makePath("empty-bin"); + const empty_path = try tmp.dir.realpathAlloc(gpa, "empty-bin"); + defer gpa.free(empty_path); + + const result = try runCliWithIsolatedHomeAndPath( + gpa, + project_root, + home_root, + empty_path, + &[_][]const u8{ "remove", "--api", "01" }, + ); + defer gpa.free(result.stdout); + defer gpa.free(result.stderr); + + try expectFailure(result); + try std.testing.expect(std.mem.indexOf(u8, result.stderr, "Node.js 22+") != null); +} + +test "Scenario: Given remove without selectors when running remove then it does not require api refresh executables" { + const gpa = std.testing.allocator; + const project_root = try projectRootAlloc(gpa); + defer gpa.free(project_root); + try buildCliBinary(gpa, project_root); + + var tmp = fs.tmpDir(.{}); + defer tmp.cleanup(); + + const home_root = try tmp.dir.realpathAlloc(gpa, "."); + defer gpa.free(home_root); + + try seedRegistryWithAccounts(gpa, home_root, "alpha@example.com", &[_]SeedAccount{ + .{ .email = "alpha@example.com", .alias = "" }, + .{ .email = "beta@example.com", .alias = "" }, + }); + + const codex_home = try codexHomeAlloc(gpa, home_root); + defer gpa.free(codex_home); + + try tmp.dir.makePath("empty-bin"); + const empty_path = try tmp.dir.realpathAlloc(gpa, "empty-bin"); + defer gpa.free(empty_path); + + const result = try runCliWithIsolatedHomeAndPathAndStdin( + gpa, + project_root, + home_root, + empty_path, + &[_][]const u8{"remove"}, + "2\n", + ); + defer gpa.free(result.stdout); + defer gpa.free(result.stderr); + + try expectSuccess(result); + try std.testing.expect(std.mem.indexOf(u8, result.stdout, "Select accounts to delete:") != null); + try std.testing.expectEqualStrings("", result.stderr); + + var loaded = try registry.loadRegistry(gpa, codex_home); + defer loaded.deinit(gpa); + try std.testing.expectEqual(@as(usize, 1), loaded.accounts.items.len); + try std.testing.expect(std.mem.eql(u8, loaded.accounts.items[0].email, "alpha@example.com")); +} + test "Scenario: Given active account removal with a replacement when running remove then it does not recreate a backup for the deleted auth" { const gpa = std.testing.allocator; const project_root = try projectRootAlloc(gpa); diff --git a/src/tests/main_test.zig b/src/tests/main_test.zig index fb48e4c..f36347f 100644 --- a/src/tests/main_test.zig +++ b/src/tests/main_test.zig @@ -331,10 +331,10 @@ test "Scenario: Given foreground commands when checking reconcile policy then co try std.testing.expect(!main_mod.shouldReconcileManagedService(.{ .daemon = .{ .mode = .once } })); } -test "Scenario: Given foreground usage refresh targets when checking refresh policy then list, switch, and remove all refresh" { +test "Scenario: Given foreground usage refresh targets when checking refresh policy then only list and switch refresh" { try std.testing.expect(main_mod.shouldRefreshForegroundUsage(.list)); try std.testing.expect(main_mod.shouldRefreshForegroundUsage(.switch_account)); - try std.testing.expect(main_mod.shouldRefreshForegroundUsage(.remove_account)); + try std.testing.expect(!main_mod.shouldRefreshForegroundUsage(.remove_account)); } test "Scenario: Given switch query with one local match when resolving locally then it returns the target account directly" { @@ -354,6 +354,23 @@ test "Scenario: Given switch query with one local match when resolving locally t } } +test "Scenario: Given switch query with a display number when resolving locally then it uses the list ordering" { + const gpa = std.testing.allocator; + var reg = makeRegistry(); + defer reg.deinit(gpa); + + try appendAccount(gpa, ®, primary_record_key, "alpha@example.com", "alpha", .team); + try appendAccount(gpa, ®, secondary_record_key, "beta@example.com", "beta", .plus); + + var resolution = try main_mod.resolveSwitchQueryLocally(gpa, ®, "02"); + defer resolution.deinit(gpa); + + switch (resolution) { + .direct => |account_key| try std.testing.expectEqualStrings(secondary_record_key, account_key), + else => return error.TestExpectedEqual, + } +} + test "Scenario: Given switch query with multiple local matches when resolving locally then it keeps the local picker set" { const gpa = std.testing.allocator; var reg = makeRegistry(); From a3be5ecba85a315d1303a78da6b5c98ef26642bf Mon Sep 17 00:00:00 2001 From: Loongphy Date: Sun, 19 Apr 2026 08:03:47 +0800 Subject: [PATCH 2/3] fix: report all missing remove selectors --- src/cli.zig | 20 ++++++++++++++++++++ src/main.zig | 10 ++++++++-- src/tests/e2e_cli_test.zig | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/src/cli.zig b/src/cli.zig index 25440d6..9ed4049 100644 --- a/src/cli.zig +++ b/src/cli.zig @@ -996,6 +996,26 @@ pub fn printAccountNotFoundError(query: []const u8) !void { try out.flush(); } +pub fn printAccountNotFoundErrors(queries: []const []const u8) !void { + if (queries.len == 0) return; + if (queries.len == 1) { + return printAccountNotFoundError(queries[0]); + } + + var buffer: [1024]u8 = undefined; + var writer = fs.File.stderr().writer(&buffer); + const out = &writer.interface; + const use_color = stderrColorEnabled(); + try writeErrorPrefixTo(out, use_color); + try out.writeAll(" no account matches: "); + for (queries, 0..) |query, idx| { + if (idx != 0) try out.writeAll(", "); + try out.writeAll(query); + } + try out.writeAll(".\n"); + try out.flush(); +} + pub fn printRemoveRequiresTtyError() !void { var buffer: [512]u8 = undefined; var writer = fs.File.stderr().writer(&buffer); diff --git a/src/main.zig b/src/main.zig index c1903a6..b4edae5 100644 --- a/src/main.zig +++ b/src/main.zig @@ -1779,6 +1779,8 @@ fn handleRemove(allocator: std.mem.Allocator, codex_home: []const u8, opts: cli. } else if (opts.selectors.len != 0) { var selected_list = std.ArrayList(usize).empty; defer selected_list.deinit(allocator); + var missing_selectors = std.ArrayList([]const u8).empty; + defer missing_selectors.deinit(allocator); var requires_confirmation = false; for (opts.selectors) |selector| { @@ -1793,8 +1795,8 @@ fn handleRemove(allocator: std.mem.Allocator, codex_home: []const u8, opts: cli. defer matches.deinit(allocator); if (matches.items.len == 0) { - try cli.printAccountNotFoundError(selector); - return error.AccountNotFound; + try missing_selectors.append(allocator, selector); + continue; } if (matches.items.len > 1) { requires_confirmation = true; @@ -1806,6 +1808,10 @@ fn handleRemove(allocator: std.mem.Allocator, codex_home: []const u8, opts: cli. } } + if (missing_selectors.items.len != 0) { + try cli.printAccountNotFoundErrors(missing_selectors.items); + return error.AccountNotFound; + } if (selected_list.items.len == 0) return; if (requires_confirmation) { var matched_labels = try cli.buildRemoveLabels(allocator, ®, selected_list.items); diff --git a/src/tests/e2e_cli_test.zig b/src/tests/e2e_cli_test.zig index bd9f31b..2f78752 100644 --- a/src/tests/e2e_cli_test.zig +++ b/src/tests/e2e_cli_test.zig @@ -1987,6 +1987,39 @@ test "Scenario: Given remove query with no matches when running remove then it e try std.testing.expect(std.mem.indexOf(u8, result.stderr, "main.zig") == null); } +test "Scenario: Given multiple remove queries with no matches when running remove then it reports all missing selectors together" { + const gpa = std.testing.allocator; + const project_root = try projectRootAlloc(gpa); + defer gpa.free(project_root); + try buildCliBinary(gpa, project_root); + + var tmp = fs.tmpDir(.{}); + defer tmp.cleanup(); + + const home_root = try tmp.dir.realpathAlloc(gpa, "."); + defer gpa.free(home_root); + + try seedRegistryWithAccounts(gpa, home_root, "keeper@example.com", &[_]SeedAccount{ + .{ .email = "keeper@example.com", .alias = "" }, + }); + + const result = try runCliWithIsolatedHomeAndStdin( + gpa, + project_root, + home_root, + &[_][]const u8{ "remove", "112222", "222222" }, + "", + ); + defer gpa.free(result.stdout); + defer gpa.free(result.stderr); + + try expectFailure(result); + try std.testing.expectEqualStrings("", result.stdout); + try std.testing.expectEqualStrings("error: no account matches: 112222, 222222.\n", result.stderr); + try std.testing.expect(std.mem.indexOf(u8, result.stderr, "AccountNotFound") == null); + try std.testing.expect(std.mem.indexOf(u8, result.stderr, "main.zig") == null); +} + test "Scenario: Given non-tty remove with invalid selection input when running remove then it fails without deleting accounts" { const gpa = std.testing.allocator; const project_root = try projectRootAlloc(gpa); From 259639e4ca9213fc52c32db5fd3039adc057cd1d Mon Sep 17 00:00:00 2001 From: Loongphy Date: Sun, 19 Apr 2026 09:01:14 +0800 Subject: [PATCH 3/3] fix: keep selector switch and remove local-only --- README.md | 19 +++++---- docs/api-refresh.md | 10 ++--- src/cli.zig | 60 +++++++++++++-------------- src/main.zig | 67 ++---------------------------- src/tests/cli_bdd_test.zig | 62 +++++----------------------- src/tests/e2e_cli_test.zig | 84 +++++++++++++++++++++++++++++++++++--- 6 files changed, 136 insertions(+), 166 deletions(-) diff --git a/README.md b/README.md index 346aa8b..843eb56 100644 --- a/README.md +++ b/README.md @@ -96,8 +96,10 @@ Remove-Item "$env:LOCALAPPDATA\codex-auth\bin\codex-auth-auto.exe" -Force -Error |---------|-------------| | `codex-auth list [--debug] [--api|--skip-api]` | List all accounts. `--api` forces a live refresh, while `--skip-api` uses only stored local usage and team-name data. | | `codex-auth login [--device-auth]` | Run `codex login` (optionally with `--device-auth`), then add the current account | -| `codex-auth switch [] [--api|--skip-api]` | Switch the active account interactively or by `` (row number, alias, or fuzzy match). `--api` forces a live refresh first; `--skip-api` stays local-only. | -| `codex-auth remove [...] [--all] [--api|--skip-api]` | Remove accounts interactively or by one or more selectors (row number, alias, or fuzzy match). Default behavior is local-only; `--api` forces a live refresh first. | +| `codex-auth switch [--api|--skip-api]` | Switch the active account interactively. `--api` forces a live refresh first; `--skip-api` stays local-only. | +| `codex-auth switch ` | Switch the active account directly by row number, alias, or fuzzy match using stored local data only. | +| `codex-auth remove [...]` | Remove accounts interactively or by one or more selectors (row number, alias, or fuzzy match) using stored local data only. | +| `codex-auth remove --all` | Remove all stored accounts. | | `codex-auth status` | Show auto-switch, service, and usage status | ### Import @@ -140,7 +142,6 @@ It does not call the usage API or `accounts/check`, so transient live-refresh fa Interactive `switch` shows email, 5h, weekly, and last activity. Without ``, it follows the configured refresh mode before opening the picker. -With ``, it resolves locally by default. Use `--api` to force a foreground refresh first, or `--skip-api` to stay on stored local data only. ```shell @@ -153,30 +154,30 @@ codex-auth switch --skip-api `` can be a displayed row number, an alias, or a fuzzy email/alias match. The row number follows the interactive `switch` list, and the same number from `codex-auth list` also works because both commands use the same ordering. +`switch ` always resolves from stored local data and does not accept `--api` or `--skip-api`. ```shell codex-auth switch 02 # switch by displayed row number codex-auth switch john # fuzzy match by email or alias codex-auth switch work # match by alias set during import -codex-auth switch --api work # force refresh before resolving the selector -codex-auth switch --skip-api 02 # local-only row-number selection ``` If `` matches multiple accounts, the command falls back to interactive selection. ### Remove Accounts -`remove` is local-only by default and does not refresh from APIs before deleting. -Use `--api` to force a foreground refresh first, or `--skip-api` to make that local-only choice explicit. +`remove` always uses stored local data and does not refresh from APIs before deleting. Each selector supports the same query forms as `switch`: row number, alias, or fuzzy email/alias match. The row number follows the interactive `switch` list, and the same number from `codex-auth list` also works because both commands use the same ordering. You can pass multiple selectors in one command. +Selector-based `remove` does not accept `--api` or `--skip-api`. ```shell codex-auth remove -codex-auth remove --skip-api 01 03 -codex-auth remove --api work personal +codex-auth remove 01 03 +codex-auth remove work personal codex-auth remove 01 jane@example.com +codex-auth remove --all ``` If any selector matches multiple accounts, `remove` asks for confirmation in interactive terminals before deleting. diff --git a/docs/api-refresh.md b/docs/api-refresh.md index 15e06cc..d96a659 100644 --- a/docs/api-refresh.md +++ b/docs/api-refresh.md @@ -46,8 +46,8 @@ The `accounts/check` response is parsed by `chatgpt_account_id`. `name: null` an - when a stored account snapshot cannot make a ChatGPT usage request because it is missing the required ChatGPT auth fields, the corresponding `list` / `switch` row shows `MissingAuth` in both usage columns until a later successful refresh replaces it - when `api.usage = false`, foreground refresh still uses only the active local rollout data because local session files do not identify the other stored accounts - `list --api` forces foreground usage refresh for this command even when `api.usage = false`; `list --skip-api` skips foreground usage refresh completely and renders only the stored registry data -- interactive `switch` follows the configured foreground usage mode by default; `switch ` resolves selectors locally from stored data by default; `switch --api` forces foreground usage refresh before selector resolution, while `switch --skip-api` stays local-only -- `remove` is local-only by default; `remove --api` forces foreground usage refresh before selector resolution or the interactive picker; `remove --skip-api` keeps the default local-only behavior explicit +- interactive `switch` follows the configured foreground usage mode by default; `switch ` always resolves selectors locally from stored data and does not accept `--api` or `--skip-api` +- `remove` always resolves selectors from stored local data and does not accept `--api` or `--skip-api` - `switch` does not refresh usage again after the new account is activated - the auto-switch daemon refreshes the current active account usage during each cycle when `auto_switch.enabled = true` - the auto-switch daemon may also refresh a small number of non-active candidate accounts from stored snapshots so it can score switch candidates @@ -60,9 +60,9 @@ The `accounts/check` response is parsed by `chatgpt_account_id`. `name: null` an - `login` refreshes immediately after the new active auth is ready. - Single-file `import` refreshes immediately for the imported auth context. - `list --api` forces synchronous `accounts/check` refresh for this command even when `api.account = false`; `list --skip-api` skips it and uses stored metadata only. -- interactive `switch` follows the configured account-name refresh mode by default; `switch ` is local-only by default; `switch --api` forces foreground account-name refresh before selector resolution, while `switch --skip-api` uses stored metadata only. -- `remove` is local-only by default; `remove --api` forces foreground account-name refresh before selector resolution or the interactive picker; `remove --skip-api` keeps the default local-only behavior explicit. -- `list`, interactive `switch`, `switch --api`, and `remove --api` load the request auth context from the current active `auth.json` when they do refresh. +- interactive `switch` follows the configured account-name refresh mode by default; `switch ` always stays local-only and does not accept `--api` or `--skip-api`. +- `remove` always stays local-only and does not accept `--api` or `--skip-api`. +- `list` and interactive `switch` load the request auth context from the current active `auth.json` when they do refresh. - the auto-switch daemon still uses a grouped-scope scan during each cycle when `auto_switch.enabled = true`. - daemon refreshes load the request auth context from stored account snapshots under `accounts/` and do not depend on the current `auth.json` belonging to the scope being refreshed. - when multiple stored ChatGPT snapshots exist for one grouped scope, daemon refreshes pick the snapshot with the newest `last_refresh`. diff --git a/src/cli.zig b/src/cli.zig index 9ed4049..8cb57d3 100644 --- a/src/cli.zig +++ b/src/cli.zig @@ -60,7 +60,6 @@ pub const SwitchOptions = struct { pub const RemoveOptions = struct { selectors: [][]const u8, all: bool, - api_mode: ApiMode = .default, }; pub const CleanOptions = struct {}; pub const AutoAction = enum { enable, disable }; @@ -326,6 +325,15 @@ pub fn parseArgs(allocator: std.mem.Allocator, args: []const [:0]const u8) !Pars } opts.query = try allocator.dupe(u8, arg); } + if (opts.query != null and opts.api_mode != .default) { + if (opts.query) |query| allocator.free(query); + return usageErrorResult( + allocator, + .switch_account, + "`switch ` does not support `--api` or `--skip-api`.", + .{}, + ); + } return .{ .command = .{ .switch_account = opts } }; } @@ -338,25 +346,16 @@ pub fn parseArgs(allocator: std.mem.Allocator, args: []const [:0]const u8) !Pars errdefer freeOwnedStringList(allocator, selectors.items); defer selectors.deinit(allocator); var all = false; - var api_mode: ApiMode = .default; var i: usize = 2; while (i < args.len) : (i += 1) { const arg = std.mem.sliceTo(args[i], 0); - if (std.mem.eql(u8, arg, "--api")) { - switch (api_mode) { - .default => api_mode = .force_api, - .force_api => return usageErrorResult(allocator, .remove_account, "duplicate `--api` for `remove`.", .{}), - .skip_api => return usageErrorResult(allocator, .remove_account, "`--api` cannot be combined with `--skip-api` for `remove`.", .{}), - } - continue; - } - if (std.mem.eql(u8, arg, "--skip-api")) { - switch (api_mode) { - .default => api_mode = .skip_api, - .skip_api => return usageErrorResult(allocator, .remove_account, "duplicate `--skip-api` for `remove`.", .{}), - .force_api => return usageErrorResult(allocator, .remove_account, "`--skip-api` cannot be combined with `--api` for `remove`.", .{}), - } - continue; + if (std.mem.eql(u8, arg, "--api") or std.mem.eql(u8, arg, "--skip-api")) { + return usageErrorResult( + allocator, + .remove_account, + "`remove` does not support `--api` or `--skip-api`.", + .{}, + ); } if (std.mem.eql(u8, arg, "--all")) { if (all or selectors.items.len != 0) { @@ -376,7 +375,6 @@ pub fn parseArgs(allocator: std.mem.Allocator, args: []const [:0]const u8) !Pars return .{ .command = .{ .remove_account = .{ .selectors = try selectors.toOwnedSlice(allocator), .all = all, - .api_mode = api_mode, } } }; } @@ -624,8 +622,8 @@ pub fn writeHelp( .{ .name = "status", .description = "Show auto-switch and usage API status" }, .{ .name = "login", .description = "Login and add the current account" }, .{ .name = "import", .description = "Import auth files or rebuild registry" }, - .{ .name = "switch [] [--api|--skip-api]", .description = "Switch the active account" }, - .{ .name = "remove [...] [--all] [--api|--skip-api]", .description = "Remove one or more accounts" }, + .{ .name = "switch [--api|--skip-api] | switch ", .description = "Switch the active account" }, + .{ .name = "remove [...] | remove --all", .description = "Remove one or more accounts" }, .{ .name = "clean", .description = "Delete backup and stale files under accounts/" }, .{ .name = "config", .description = "Manage configuration" }, }; @@ -771,8 +769,8 @@ fn commandDescriptionForTopic(topic: HelpTopic) []const u8 { .status => "Show auto-switch, service, and usage API status.", .login => "Run `codex login` or `codex login --device-auth`, then add the current account.", .import_auth => "Import auth files or rebuild the registry.", - .switch_account => "Switch the active account interactively, by query, or by list row number.", - .remove_account => "Remove one or more accounts by query or list row number.", + .switch_account => "Switch the active account interactively, or by query using stored local data.", + .remove_account => "Remove one or more accounts by query or list row number using stored local data.", .clean => "Delete backup and stale files under accounts/.", .config => "Manage auto-switch and usage API configuration.", .daemon => "Run the background auto-switch daemon.", @@ -807,12 +805,12 @@ fn writeUsageSection(out: *std.Io.Writer, topic: HelpTopic) !void { }, .switch_account => { try out.writeAll(" codex-auth switch [--api|--skip-api]\n"); - try out.writeAll(" codex-auth switch [--api|--skip-api] \n"); + try out.writeAll(" codex-auth switch \n"); }, .remove_account => { - try out.writeAll(" codex-auth remove [--api|--skip-api]\n"); - try out.writeAll(" codex-auth remove [--api|--skip-api] [...]\n"); - try out.writeAll(" codex-auth remove [--api|--skip-api] --all\n"); + try out.writeAll(" codex-auth remove\n"); + try out.writeAll(" codex-auth remove [...]\n"); + try out.writeAll(" codex-auth remove --all\n"); }, .clean => try out.writeAll(" codex-auth clean\n"), .config => { @@ -856,13 +854,15 @@ fn writeExamplesSection(out: *std.Io.Writer, topic: HelpTopic) !void { }, .switch_account => { try out.writeAll(" codex-auth switch\n"); - try out.writeAll(" codex-auth switch work --api\n"); - try out.writeAll(" codex-auth switch 02 --skip-api\n"); + try out.writeAll(" codex-auth switch --api\n"); + try out.writeAll(" codex-auth switch --skip-api\n"); + try out.writeAll(" codex-auth switch work\n"); + try out.writeAll(" codex-auth switch 02\n"); }, .remove_account => { try out.writeAll(" codex-auth remove\n"); - try out.writeAll(" codex-auth remove --skip-api 01 03\n"); - try out.writeAll(" codex-auth remove --api work\n"); + try out.writeAll(" codex-auth remove 01 03\n"); + try out.writeAll(" codex-auth remove work personal\n"); try out.writeAll(" codex-auth remove john@example.com jane@example.com\n"); try out.writeAll(" codex-auth remove --all\n"); }, diff --git a/src/main.zig b/src/main.zig index b4edae5..a0ac6f4 100644 --- a/src/main.zig +++ b/src/main.zig @@ -1468,43 +1468,11 @@ fn handleSwitch(allocator: std.mem.Allocator, codex_home: []const u8, opts: cli. try registry.saveRegistry(allocator, codex_home, ®); } if (opts.query) |query| { - const usage_api_enabled = apiModeUsesApi(false, opts.api_mode); - const account_api_enabled = apiModeUsesApi(false, opts.api_mode); - var usage_state: ?ForegroundUsageRefreshState = null; - defer if (usage_state) |*state| state.deinit(allocator); - - if (usage_api_enabled or account_api_enabled) { - try ensureForegroundNodeAvailableWithApiEnabled( - allocator, - codex_home, - ®, - .switch_account, - usage_api_enabled, - account_api_enabled, - ); - usage_state = try refreshForegroundUsageForDisplayWithApiFetcherWithPoolInitAndDebugUsingApiEnabled( - allocator, - codex_home, - ®, - usage_api.fetchUsageForAuthPathDetailed, - initForegroundUsagePool, - null, - usage_api_enabled, - ); - try maybeRefreshForegroundAccountNamesWithAccountApiEnabled( - allocator, - codex_home, - ®, - .switch_account, - defaultAccountFetcher, - account_api_enabled, - ); - } + std.debug.assert(opts.api_mode == .default); var resolution = try resolveSwitchQueryLocally(allocator, ®, query); defer resolution.deinit(allocator); - const usage_overrides = if (usage_state) |*state| state.usage_overrides else null; const selected_account_key = switch (resolution) { .not_found => { try cli.printAccountNotFoundError(query); @@ -1515,7 +1483,7 @@ fn handleSwitch(allocator: std.mem.Allocator, codex_home: []const u8, opts: cli. allocator, ®, matches.items, - usage_overrides, + null, ), }; if (selected_account_key == null) return; @@ -1741,36 +1709,7 @@ fn handleRemove(allocator: std.mem.Allocator, codex_home: []const u8, opts: cli. try registry.saveRegistry(allocator, codex_home, ®); } - var usage_state: ?ForegroundUsageRefreshState = null; - defer if (usage_state) |*state| state.deinit(allocator); - if (opts.api_mode == .force_api) { - try ensureForegroundNodeAvailableWithApiEnabled( - allocator, - codex_home, - ®, - .remove_account, - true, - true, - ); - usage_state = try refreshForegroundUsageForDisplayWithApiFetcherWithPoolInitAndDebugUsingApiEnabled( - allocator, - codex_home, - ®, - usage_api.fetchUsageForAuthPathDetailed, - initForegroundUsagePool, - null, - true, - ); - try maybeRefreshForegroundAccountNamesWithAccountApiEnabled( - allocator, - codex_home, - ®, - .remove_account, - defaultAccountFetcher, - true, - ); - } - const usage_overrides = if (usage_state) |*state| state.usage_overrides else null; + const usage_overrides: ?[]const ?[]const u8 = null; var selected: ?[]usize = null; if (opts.all) { diff --git a/src/tests/cli_bdd_test.zig b/src/tests/cli_bdd_test.zig index 5126083..ad1c153 100644 --- a/src/tests/cli_bdd_test.zig +++ b/src/tests/cli_bdd_test.zig @@ -286,7 +286,8 @@ test "Scenario: Given help when rendering then login and command help notes are try std.testing.expect(std.mem.indexOf(u8, help, "`config api enable` may trigger OpenAI account restrictions or suspension in some environments.") != null); try std.testing.expect(std.mem.indexOf(u8, help, "login") != null); try std.testing.expect(std.mem.indexOf(u8, help, "clean") != null); - try std.testing.expect(std.mem.indexOf(u8, help, "remove [...] [--all] [--api|--skip-api]") != null); + try std.testing.expect(std.mem.indexOf(u8, help, "switch [--api|--skip-api] | switch ") != null); + try std.testing.expect(std.mem.indexOf(u8, help, "remove [...] | remove --all") != null); try std.testing.expect(std.mem.indexOf(u8, help, "Delete backup and stale files under accounts/") != null); try std.testing.expect(std.mem.indexOf(u8, help, "status") != null); try std.testing.expect(std.mem.indexOf(u8, help, "config") != null); @@ -712,42 +713,22 @@ test "Scenario: Given switch with positional query when parsing then non-interac } } -test "Scenario: Given switch with skip-api flag when parsing then local-only picker mode is preserved" { +test "Scenario: Given switch query with skip-api flag when parsing then usage error is returned" { const gpa = std.testing.allocator; const args = [_][:0]const u8{ "codex-auth", "switch", "--skip-api", "02" }; var result = try cli.parseArgs(gpa, &args); defer cli.freeParseResult(gpa, &result); - switch (result) { - .command => |cmd| switch (cmd) { - .switch_account => |opts| { - try std.testing.expectEqual(cli.ApiMode.skip_api, opts.api_mode); - try std.testing.expect(opts.query != null); - try std.testing.expect(std.mem.eql(u8, opts.query.?, "02")); - }, - else => return error.TestExpectedEqual, - }, - else => return error.TestExpectedEqual, - } + try expectUsageError(result, .switch_account, "does not support"); } -test "Scenario: Given switch with api flag when parsing then forced api mode is preserved" { +test "Scenario: Given switch query with api flag when parsing then usage error is returned" { const gpa = std.testing.allocator; const args = [_][:0]const u8{ "codex-auth", "switch", "--api", "02" }; var result = try cli.parseArgs(gpa, &args); defer cli.freeParseResult(gpa, &result); - switch (result) { - .command => |cmd| switch (cmd) { - .switch_account => |opts| { - try std.testing.expectEqual(cli.ApiMode.force_api, opts.api_mode); - try std.testing.expect(opts.query != null); - try std.testing.expect(std.mem.eql(u8, opts.query.?, "02")); - }, - else => return error.TestExpectedEqual, - }, - else => return error.TestExpectedEqual, - } + try expectUsageError(result, .switch_account, "does not support"); } test "Scenario: Given switch with duplicate target when parsing then usage error is returned" { @@ -780,7 +761,6 @@ test "Scenario: Given remove with positional query when parsing then selector mo try std.testing.expectEqual(@as(usize, 1), opts.selectors.len); try std.testing.expect(std.mem.eql(u8, opts.selectors[0], "user@example.com")); try std.testing.expect(!opts.all); - try std.testing.expectEqual(cli.ApiMode.default, opts.api_mode); }, else => return error.TestExpectedEqual, }, @@ -799,7 +779,6 @@ test "Scenario: Given remove with all flag when parsing then all mode is preserv .remove_account => |opts| { try std.testing.expectEqual(@as(usize, 0), opts.selectors.len); try std.testing.expect(opts.all); - try std.testing.expectEqual(cli.ApiMode.default, opts.api_mode); }, else => return error.TestExpectedEqual, }, @@ -821,7 +800,6 @@ test "Scenario: Given remove with multiple selectors when parsing then all selec try std.testing.expect(std.mem.eql(u8, opts.selectors[1], "b@example.com")); try std.testing.expect(std.mem.eql(u8, opts.selectors[2], "03")); try std.testing.expect(!opts.all); - try std.testing.expectEqual(cli.ApiMode.default, opts.api_mode); }, else => return error.TestExpectedEqual, }, @@ -829,42 +807,22 @@ test "Scenario: Given remove with multiple selectors when parsing then all selec } } -test "Scenario: Given remove with skip-api flag when parsing then explicit local-only mode is preserved" { +test "Scenario: Given remove with skip-api flag when parsing then usage error is returned" { const gpa = std.testing.allocator; const args = [_][:0]const u8{ "codex-auth", "remove", "--skip-api", "01" }; var result = try cli.parseArgs(gpa, &args); defer cli.freeParseResult(gpa, &result); - switch (result) { - .command => |cmd| switch (cmd) { - .remove_account => |opts| { - try std.testing.expectEqual(cli.ApiMode.skip_api, opts.api_mode); - try std.testing.expectEqual(@as(usize, 1), opts.selectors.len); - try std.testing.expect(std.mem.eql(u8, opts.selectors[0], "01")); - }, - else => return error.TestExpectedEqual, - }, - else => return error.TestExpectedEqual, - } + try expectUsageError(result, .remove_account, "does not support"); } -test "Scenario: Given remove with api flag when parsing then forced api mode is preserved" { +test "Scenario: Given remove with api flag when parsing then usage error is returned" { const gpa = std.testing.allocator; const args = [_][:0]const u8{ "codex-auth", "remove", "--api", "work" }; var result = try cli.parseArgs(gpa, &args); defer cli.freeParseResult(gpa, &result); - switch (result) { - .command => |cmd| switch (cmd) { - .remove_account => |opts| { - try std.testing.expectEqual(cli.ApiMode.force_api, opts.api_mode); - try std.testing.expectEqual(@as(usize, 1), opts.selectors.len); - try std.testing.expect(std.mem.eql(u8, opts.selectors[0], "work")); - }, - else => return error.TestExpectedEqual, - }, - else => return error.TestExpectedEqual, - } + try expectUsageError(result, .remove_account, "does not support"); } test "Scenario: Given remove with unexpected flag when parsing then usage error is returned" { diff --git a/src/tests/e2e_cli_test.zig b/src/tests/e2e_cli_test.zig index 2f78752..7e4e1f2 100644 --- a/src/tests/e2e_cli_test.zig +++ b/src/tests/e2e_cli_test.zig @@ -1423,7 +1423,7 @@ test "Scenario: Given list with skip-api when running list then it does not requ try std.testing.expectEqualStrings("", result.stderr); } -test "Scenario: Given switch query with api override when api config is disabled then it still requires api refresh executables" { +test "Scenario: Given switch query with api flag when running switch then it returns a usage error" { const gpa = std.testing.allocator; const project_root = try projectRootAlloc(gpa); defer gpa.free(project_root); @@ -1439,7 +1439,6 @@ test "Scenario: Given switch query with api override when api config is disabled .{ .email = "active@example.com", .alias = "active" }, .{ .email = "backup@example.com", .alias = "backup" }, }); - try setRegistryApiConfig(gpa, home_root, false, false); try tmp.dir.makePath("empty-bin"); const empty_path = try tmp.dir.realpathAlloc(gpa, "empty-bin"); @@ -1456,7 +1455,44 @@ test "Scenario: Given switch query with api override when api config is disabled defer gpa.free(result.stderr); try expectFailure(result); - try std.testing.expect(std.mem.indexOf(u8, result.stderr, "Node.js 22+") != null); + try std.testing.expectEqualStrings("", result.stdout); + try std.testing.expect(std.mem.indexOf(u8, result.stderr, "does not support `--api` or `--skip-api`") != null); +} + +test "Scenario: Given switch query with skip-api flag when running switch then it returns a usage error" { + const gpa = std.testing.allocator; + const project_root = try projectRootAlloc(gpa); + defer gpa.free(project_root); + try buildCliBinary(gpa, project_root); + + var tmp = fs.tmpDir(.{}); + defer tmp.cleanup(); + + const home_root = try tmp.dir.realpathAlloc(gpa, "."); + defer gpa.free(home_root); + + try seedRegistryWithAccounts(gpa, home_root, "active@example.com", &[_]SeedAccount{ + .{ .email = "active@example.com", .alias = "active" }, + .{ .email = "backup@example.com", .alias = "backup" }, + }); + + try tmp.dir.makePath("empty-bin"); + const empty_path = try tmp.dir.realpathAlloc(gpa, "empty-bin"); + defer gpa.free(empty_path); + + const result = try runCliWithIsolatedHomeAndPath( + gpa, + project_root, + home_root, + empty_path, + &[_][]const u8{ "switch", "--skip-api", "02" }, + ); + defer gpa.free(result.stdout); + defer gpa.free(result.stderr); + + try expectFailure(result); + try std.testing.expectEqualStrings("", result.stdout); + try std.testing.expect(std.mem.indexOf(u8, result.stderr, "does not support `--api` or `--skip-api`") != null); } test "Scenario: Given switch with skip-api when running interactively then it does not require api refresh executables" { @@ -1636,7 +1672,7 @@ test "Scenario: Given remove with multiple selectors when running remove then it try std.testing.expect(std.mem.eql(u8, loaded.accounts.items[0].email, "beta@example.com")); } -test "Scenario: Given remove with api override when api config is disabled then it still requires api refresh executables" { +test "Scenario: Given remove with api flag when running remove then it returns a usage error" { const gpa = std.testing.allocator; const project_root = try projectRootAlloc(gpa); defer gpa.free(project_root); @@ -1652,7 +1688,6 @@ test "Scenario: Given remove with api override when api config is disabled then .{ .email = "alpha@example.com", .alias = "" }, .{ .email = "beta@example.com", .alias = "" }, }); - try setRegistryApiConfig(gpa, home_root, false, false); try tmp.dir.makePath("empty-bin"); const empty_path = try tmp.dir.realpathAlloc(gpa, "empty-bin"); @@ -1669,7 +1704,44 @@ test "Scenario: Given remove with api override when api config is disabled then defer gpa.free(result.stderr); try expectFailure(result); - try std.testing.expect(std.mem.indexOf(u8, result.stderr, "Node.js 22+") != null); + try std.testing.expectEqualStrings("", result.stdout); + try std.testing.expect(std.mem.indexOf(u8, result.stderr, "does not support `--api` or `--skip-api`") != null); +} + +test "Scenario: Given remove with skip-api flag when running remove then it returns a usage error" { + const gpa = std.testing.allocator; + const project_root = try projectRootAlloc(gpa); + defer gpa.free(project_root); + try buildCliBinary(gpa, project_root); + + var tmp = fs.tmpDir(.{}); + defer tmp.cleanup(); + + const home_root = try tmp.dir.realpathAlloc(gpa, "."); + defer gpa.free(home_root); + + try seedRegistryWithAccounts(gpa, home_root, "alpha@example.com", &[_]SeedAccount{ + .{ .email = "alpha@example.com", .alias = "" }, + .{ .email = "beta@example.com", .alias = "" }, + }); + + try tmp.dir.makePath("empty-bin"); + const empty_path = try tmp.dir.realpathAlloc(gpa, "empty-bin"); + defer gpa.free(empty_path); + + const result = try runCliWithIsolatedHomeAndPath( + gpa, + project_root, + home_root, + empty_path, + &[_][]const u8{ "remove", "--skip-api", "01" }, + ); + defer gpa.free(result.stdout); + defer gpa.free(result.stderr); + + try expectFailure(result); + try std.testing.expectEqualStrings("", result.stdout); + try std.testing.expect(std.mem.indexOf(u8, result.stderr, "does not support `--api` or `--skip-api`") != null); } test "Scenario: Given remove without selectors when running remove then it does not require api refresh executables" {