Skip to content

release: v2.2.0 stable + post-audit hardening - #504

Merged
ndycode merged 2 commits into
mainfrom
fix/post-merge-audit-lows
Jun 2, 2026
Merged

release: v2.2.0 stable + post-audit hardening#504
ndycode merged 2 commits into
mainfrom
fix/post-merge-audit-lows

Conversation

@ndycode

@ndycode ndycode commented Jun 2, 2026

Copy link
Copy Markdown
Owner

Summary

Cuts the v2.2.0 stable release (promoting the v2.1.13-beta line) and folds in the remaining LOW findings from the deep post-merge audit of main.

Per the maintainer decision, this is a minor stable release → npm latest (drops the -beta prerelease tag).

Release

Fixes (deep-audit LOW findings)

  • styleAccountDetailText (codex-manager.ts): test failed|error (danger) before unavailable|not available (warning) in both the suffix and compact paths — a real failure whose text contains "not available" now renders red, not a soft yellow warning.
  • resolvePath (storage/paths.ts): reject a NUL-byte path up front (defense in depth) instead of letting it reach the fs layer.
  • switch <index>: require a strict integer — 1.5 / 2abc no longer silently truncate to a valid account; they error.

Tests: strict-index rejection for switch, NUL-byte rejection for resolvePath.

Deliberately NOT changed

The quota-probe transient-vs-unsupported precedence (a CodeRabbit LOW suggestion) was left as-is: existing tests ("does not throw CodexUnavailableError when a non-unsupported failure is mixed in" / "does not mask an instruction-fetch failure") intentionally keep a real transient surfacing rather than masking a possible outage behind the friendly "Codex unavailable" note. Changing it would regress that safety contract.

Audit confidence (this release)

Deep multi-arm audit of merged main (the basis for 2.2.0):

Verification

typecheck + lint + audit:ci clean · full suite 4278 passed / 2 skipped.

🤖 Generated with Claude Code

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

promotes the v2.1.13-beta line to a stable 2.2.0 release and folds in the remaining LOW audit findings: tone-precedence fix in styleAccountDetailText, NUL-byte rejection in resolvePath, and strict-integer guard in switch. all three fixes ship with targeted vitest coverage (the previously-noted gap for the tone fix is now closed by test/codex-manager-detail-tone.test.ts).

  • styleAccountDetailText (lib/codex-manager.ts): failed|error now checked before unavailable|not available on both the compact path and the quota-suffix path, so a 5xx "service not available" renders danger (red) instead of warning (yellow). exported for direct unit testing.
  • resolvePath (lib/storage/paths.ts): NUL-byte (\x00) check added as the very first guard, before tilde expansion or path.resolve, giving a clear error rather than a deep fs-layer throw on windows or posix.
  • switch <index> (lib/codex-manager/commands/switch.ts): ^\d+$ regex rejects floats (1.5), scientific notation (1e0), hex (0x2), and sign-prefixed strings (+1, -1) before parseInt is called, preventing silent truncation to a valid account index.

Confidence Score: 5/5

all three code changes are narrow, well-scoped hardening fixes with direct vitest coverage; version bump and docs are mechanical.

the tone-precedence fix, NUL-byte guard, and strict-integer check each have no side-effects beyond the intended behavior change, and each is pinned by new tests. the previous audit gap (missing tone test) is now closed. no concurrency-sensitive paths are touched, no token or filesystem safety regressions are introduced.

no files require special attention.

Important Files Changed

Filename Overview
lib/codex-manager.ts exports styleAccountDetailText and reorders tone precedence — danger before warning — on both the compact and quota-suffix paths; logic is correct and now covered by the new tone test.
lib/codex-manager/commands/switch.ts adds ^\d+$ guard before parseInt to reject floats, hex, scientific-notation, and sign-prefixed strings; early return prevents loadAccounts and persistAndSyncSelectedAccount from ever being called on invalid input.
lib/storage/paths.ts NUL-byte rejection added as the very first check in resolvePath, before any tilde expansion or path.resolve call; correct on both POSIX and Windows.
test/codex-manager-detail-tone.test.ts new test file covering both the compact path and quota-suffix path for tone precedence; uses isTTY override and v2Enabled:false to make ANSI color assertions deterministic.
test/codex-manager-switch-command.test.ts adds it.each over seven non-integer inputs verifying exit code 1, correct error message, and no side-effects (persistAndSyncSelectedAccount not called).
test/paths.test.ts adds two NUL-byte rejection cases for resolvePath matching /NUL byte/i; straightforward and sufficient.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["switch index arg"] --> B{"^\\d+$ test on trimmed arg"}
    B -- fail --> E["logError: Invalid index\nreturn 1"]
    B -- pass --> C{"isFinite && >= 1"}
    C -- fail --> E
    C -- pass --> F["loadAccounts()"]
    F --> G{"targetIndex in range?"}
    G -- no --> H["logError: out of range\nreturn 1"]
    G -- yes --> I["persistAndSyncSelectedAccount()"]
    I --> J["return 0"]

    R["resolvePath(filePath)"] --> R1{"contains NUL byte?"}
    R1 -- yes --> R2["throw: Invalid path: contains a NUL byte"]
    R1 -- no --> R3["tilde expand / path.resolve"]
    R3 --> R4["lookalike-sibling + root checks"]
    R4 --> R5["return resolved path"]

    S["styleAccountDetailText(detail)"] --> S1{"quota pattern match?"}
    S1 -- yes --> S2{"suffix: /failed|error/?"}
    S2 -- yes --> SD["danger (red)"]
    S2 -- no --> S3{"suffix: unavailable/stale?"}
    S3 -- yes --> SW["warning (yellow)"]
    S3 -- no --> SM["muted"]
    S1 -- no --> S4{"compact: /rate-limited/?"}
    S4 -- yes --> SD2["danger"]
    S4 -- no --> S5{"compact: /failed|error/?"}
    S5 -- yes --> SD3["danger"]
    S5 -- no --> S6{"compact: unavailable/stale?"}
    S6 -- yes --> SW2["warning"]
    S6 -- no --> SF["fallback tone"]
Loading

Comments Outside Diff (1)

  1. lib/codex-manager.ts, line 424-448 (link)

    P2 missing vitest coverage for styleAccountDetailText priority fix

    the priority-swap is the core correctness fix in this PR, but there's no unit test verifying the new ordering. styleAccountDetailText is only referenced in tests via a pass-through mock in repair-commands.test.ts — so a regression (e.g. the suffix branch reverting to the old order) would pass the full suite undetected. a targeted test for a suffix like "service not available – error" asserting "danger" output would close this gap.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: lib/codex-manager.ts
    Line: 424-448
    
    Comment:
    **missing vitest coverage for `styleAccountDetailText` priority fix**
    
    the priority-swap is the core correctness fix in this PR, but there's no unit test verifying the new ordering. `styleAccountDetailText` is only referenced in tests via a pass-through mock in `repair-commands.test.ts` — so a regression (e.g. the suffix branch reverting to the old order) would pass the full suite undetected. a targeted test for a suffix like `"service not available – error"` asserting `"danger"` output would close this gap.
    
    How can I resolve this? If you propose a fix, please make it concise.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

    Fix in Codex

Reviews (2): Last reviewed commit: "test(codex-manager): pin danger>warning ..." | Re-trigger Greptile

Promote the v2.1.13-beta line to a stable 2.2.0 release and fold in the
remaining LOW findings from the deep post-merge audit.

Release:
- Version 2.1.13-beta.3 -> 2.2.0 across package.json, .codex-plugin/plugin.json,
  AGENTS.md, package-lock.json.
- Rename docs/releases/v2.1.13-beta.3.md -> v2.2.0.md, rewrite as a STABLE
  release (install `npm i -g codex-multi-auth`, no @beta), add a #501/#502 quota
  section, and document the post-audit hardening. Update the docs-portal + root
  README pointers from "current prerelease (beta)" to "current stable" v2.2.0.

Fixes (from the deep audit's LOW findings):
- codex-manager.ts styleAccountDetailText: test `failed|error` (danger) BEFORE
  the `unavailable|not available` warning keywords in both the suffix and compact
  paths, so a real failure whose text contains "not available" renders red, not a
  soft yellow warning.
- storage/paths.ts resolvePath: reject a path containing a NUL byte up front
  (defense in depth) rather than letting it reach the fs layer.
- codex-manager/commands/switch.ts: require a strict integer index; "1.5"/"2abc"
  no longer silently truncate to a valid account — they error.

Tests: strict-index rejection cases for `switch`, and a NUL-byte rejection case
for `resolvePath`.

Not changed: the quota-probe transient-vs-unsupported precedence (CodeRabbit
LOW-2) was intentionally left as-is — existing tests (quota-probe "does not throw
CodexUnavailableError when a non-unsupported failure is mixed in" / "does not mask
an instruction-fetch failure") deliberately keep a real transient surfacing
rather than masking an outage behind the friendly note.

typecheck + lint + audit:ci clean; full suite 4278 passed / 2 skipped.
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@ndycode

ndycode commented Jun 2, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Full review triggered.

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

v2.2.0 promoted to stable. adds nul-byte rejection in lib/storage/paths.ts:445, strict digits-only switch index validation in lib/codex-manager/commands/switch.ts:39, and an error-tone precedence fix in lib/codex-manager.ts:425,442. docs and version metadata updated.

Changes

release and input validation hardening

Layer / File(s) Summary
version metadata and release documentation
package.json, .codex-plugin/plugin.json, AGENTS.md, README.md, docs/README.md, docs/releases/v2.2.0.md, .gitignore
version bumped to 2.2.0 across manifests and docs; release notes promoted to stable; .omc/ added to .gitignore.
nul-byte path rejection
lib/storage/paths.ts:445, test/paths.test.ts:775
resolvePath now throws on NUL bytes before any path normalization or containment checks; tests assert errors for NUL-containing inputs (see lib/storage/paths.ts:445 and test/paths.test.ts:775).
switch index strict integer parsing
lib/codex-manager/commands/switch.ts:39, test/codex-manager-switch-command.test.ts:70
runSwitchCommand rejects non-digit index inputs via /^\d+$/ after trim, logs Invalid index: <value>, and returns exit code 1; tests cover decimals, hex, signs, scientific notation (lib/codex-manager/commands/switch.ts:39, test/codex-manager-switch-command.test.ts:70).
error/failure tone precedence in account status
lib/codex-manager.ts:425,442, test/codex-manager-detail-tone.test.ts:1-71
styleAccountDetailText exported and reordered so `/failed

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~18 minutes

  • missing regression tests: confirm lib/storage/paths.ts:445 behavior on windows path APIs is covered. current test/paths.test.ts:775 asserts NUL rejection but may not exercise platform-specific path handling.
  • windows edge cases: explicitly verify NUL-byte behavior on lib/storage/paths.ts:445 under windows semantics.
  • concurrency risks: review lib/codex-manager/commands/switch.ts:39 interactions with persistence (persistAndSyncSelectedAccount) for concurrent switch operations.

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Title follows conventional commits format with type 'release', has appropriate scope context, and clearly summarizes the main change (v2.2.0 stable + post-audit hardening) at 45 characters.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed PR description is comprehensive, well-structured, and covers all required sections with clear detail on changes, audit findings, and deliberate non-changes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/post-merge-audit-lows
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch fix/post-merge-audit-lows

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/codex-manager.ts`:
- Around line 442-444: Add a focused unit test for styleAccountDetailText to
assert that when a detail string contains overlapping keywords (e.g., both
"error"/"failed" and "unavailable"/"not available") the tone precedence yields
"danger" (matching the compact-path change that checks /failed|error/i before
the unavailable regex). Locate the function under test styleAccountDetailText in
lib/codex-manager.ts and write a vitest unit test (e.g., alongside
test/codex-manager-cli.test.ts or a new file under test/) that passes a string
containing both keywords (and normalized whitespace/newlines) and asserts the
returned/styled output uses the danger style (same outcome as stylePromptText
when matching /failed|error/i); make the assertion mirror existing tests that
check CODEX_UNAVAILABLE_PROBE_NOTE but expect "danger" instead of "warning".
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6914fa6e-7439-4edb-a594-5d012950f99a

📥 Commits

Reviewing files that changed from the base of the PR and between 1317fe9 and d8f714c.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (12)
  • .codex-plugin/plugin.json
  • .gitignore
  • AGENTS.md
  • README.md
  • docs/README.md
  • docs/releases/v2.2.0.md
  • lib/codex-manager.ts
  • lib/codex-manager/commands/switch.ts
  • lib/storage/paths.ts
  • package.json
  • test/codex-manager-switch-command.test.ts
  • test/paths.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (17)
package.json

📄 CodeRabbit inference engine (SECURITY.md)

package.json: Pin hono to 4.12.18 or higher (but below 4.12.0-4.12.1) to avoid GHSA-xh87-mx6m-69f3 authentication bypass vulnerability
Pin rollup to ^4.59.0 or higher to avoid vulnerable versions below 4.59.0 in Vite and Vitest transitive dependencies

Files:

  • package.json
docs/{README.md,docs/**/*.md}

📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)

docs/{README.md,docs/**/*.md}: Use canonical package name codex-multi-auth in all documentation
Verify internal links in documentation are valid before merge
Ensure no conflicting guidance exists between README, docs, and governance files before merge

Files:

  • docs/README.md
docs/{README.md,docs/getting-started.md}

📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)

Update README.md and docs/getting-started.md first when runtime behavior changes

Files:

  • docs/README.md
docs/{README.md,docs/getting-started.md,docs/configuration.md,docs/troubleshooting.md,docs/reference/**/*.md}

📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)

Verify every documented command is executable as written before merge

Files:

  • docs/README.md
docs/{docs/**/*.md,README.md}

📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)

Verify paths in documentation match runtime modules (lib/runtime-paths.ts, lib/storage.ts, lib/config.ts)

Files:

  • docs/README.md
docs/{README.md,docs/index.md,docs/README.md}

📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)

Use accurate discoverability terms in public landing pages without keyword stuffing or ranking promises

Root README and docs landing pages should naturally include Codex CLI, multi-account OAuth, account switching, health checks, runtime rotation, diagnostics, and recovery when those topics are in scope

Files:

  • docs/README.md
docs/**/*.md

📄 CodeRabbit inference engine (docs/STYLE_GUIDE.md)

docs/**/*.md: User-facing documentation should follow the page template: Title and one-line lead, Quick path commands, Core operational workflow, Troubleshooting or failure handling, and Related links
Use short sections and scan-friendly tables in documentation where they improve clarity
Prefer direct, actionable language in documentation
Use runnable command examples in documentation
Explain expected outcomes after critical commands in documentation
Keep terminology consistent with runtime names in documentation
Avoid speculative language when behavior is deterministic in documentation
Put the user problem in the first paragraph before implementation detail
Use descriptive page titles such as codex-multi-auth Features instead of generic titles on public docs
Do not repeat keyword lists in every section; search terms should appear only where they help a developer understand the page
Canonical command family is codex-multi-auth ...
Canonical runtime root is ~/.codex/multi-auth
Runtime rotation must be described as default-on unless the release policy changes
Legacy command/path references belong only in migration contexts in documentation
Compatibility aliases (codex multi auth, codex multi-auth, codex multiauth) belong only in command reference, troubleshooting, or migration contexts
Keep command flags aligned with runtime usage text in documentation
Avoid non-runnable command snippets in documentation
Avoid conflicting path guidance across documentation
Avoid legacy-first onboarding language in documentation

Maintain complete documentation including: getting-started guide, features overview, configuration reference, troubleshooting guide, command reference, public API contract, error contracts, settings reference, storage paths, upgrade guide, privacy policy, and release notes

Files:

  • docs/README.md
  • docs/releases/v2.2.0.md
docs/{README.md,package.json,docs/development/GITHUB_DISCOVERABILITY.md}

📄 CodeRabbit inference engine (docs/STYLE_GUIDE.md)

Keep the repository description, package description, README lead, and docs/development/GITHUB_DISCOVERABILITY.md aligned

Files:

  • docs/README.md
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/README.md
  • docs/releases/v2.2.0.md
lib/**/*.ts

📄 CodeRabbit inference engine (lib/AGENTS.md)

lib/**/*.ts: All public exports should flow through lib/index.ts or documented package subpaths
Never import from dist/ in source tests or library code
Never suppress type errors

Files:

  • lib/storage/paths.ts
  • lib/codex-manager.ts
  • lib/codex-manager/commands/switch.ts
lib/storage/**/*.ts

📄 CodeRabbit inference engine (lib/AGENTS.md)

lib/storage/**/*.ts: Worktree storage uses resolveProjectStorageIdentityRoot; never derive project pools from raw worktree paths
Never use bare recursive cleanup in Windows-sensitive paths without retry handling

Files:

  • lib/storage/paths.ts
**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Do not use as any, @ts-ignore, or @ts-expect-error type assertions

Do not bypass the official Codex CLI by reimplementing general Codex commands in the wrapper

Do not patch official Codex app binaries; use app bind or launcher helpers for desktop app integration

Do not expose account emails or tokens in runtime proxy client response headers or logs

Do not key project storage by worktree path; use resolveProjectStorageIdentityRoot for storage path resolution

Use ESM only with "type": "module"; Node >= 18 is required

Email dedup must be case-insensitive via normalizeEmailKey() function (trim + lowercase)

Files:

  • lib/storage/paths.ts
  • test/codex-manager-switch-command.test.ts
  • lib/codex-manager.ts
  • lib/codex-manager/commands/switch.ts
  • test/paths.test.ts
**/*.{ts,tsx,js,mjs,cjs}

📄 CodeRabbit inference engine (README.md)

Install and use codex-multi-auth as a global npm package with npm i -g codex-multi-auth to manage multi-account OAuth for Codex CLI

Implement OAuth account credential handling for personal development use only; include clear documentation that this is not an official OpenAI product and users are responsible for policy compliance

Implement stateless store=false routing as default; enable stateful store=true background response mode only via explicit CODEX_AUTH_BACKGROUND_RESPONSES=1 or settings configuration for callers that send background: true

Implement Responses request/prompt compatibility with strict runtime handling and documented error contracts; support configurable timeout overrides via CODEX_AUTH_FETCH_TIMEOUT_MS and CODEX_AUTH_STREAM_STALL_TIMEOUT_MS

Files:

  • lib/storage/paths.ts
  • test/codex-manager-switch-command.test.ts
  • lib/codex-manager.ts
  • lib/codex-manager/commands/switch.ts
  • test/paths.test.ts
lib/**

⚙️ CodeRabbit configuration file

focus on auth rotation, windows filesystem IO, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios. check for logging that leaks tokens or emails.

Files:

  • lib/storage/paths.ts
  • lib/codex-manager.ts
  • lib/codex-manager/commands/switch.ts
test/**/*.test.ts

📄 CodeRabbit inference engine (test/AGENTS.md)

test/**/*.test.ts: Vitest globals (describe, it, expect) are enabled and should be used without explicit imports
Maintain 80% coverage threshold across statements, branches, functions, and lines
Use removeWithRetry for Windows filesystem cleanup instead of bare fs.rm to handle EBUSY/EPERM/ENOTEMPTY backoff
Use source files in tests, not compiled dist/ files; test the source directly
Do not skip tests without justification; include rationale if a test must be skipped
Relax ESLint rules for test files as specified in eslint.config.js

Files:

  • test/codex-manager-switch-command.test.ts
  • test/paths.test.ts
test/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Use Vitest for test suites; include property tests and chaos tests for core functionality

Files:

  • test/codex-manager-switch-command.test.ts
  • test/paths.test.ts
test/**

⚙️ CodeRabbit configuration file

tests must stay deterministic and use vitest. demand regression cases that reproduce concurrency bugs, token refresh races, and windows filesystem behavior. reject changes that mock real secrets or skip assertions.

Files:

  • test/codex-manager-switch-command.test.ts
  • test/paths.test.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T11:45:09.291Z
Learning: Runtime rotation is default-on through `codexRuntimeRotationProxy`; users can opt out with `codex-multi-auth rotation disable` or `CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY=0`
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T11:45:09.291Z
Learning: Keep runtime rotation default-on behavior aligned with explicit release and migration documentation
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T11:45:09.291Z
Learning: OAuth callback port remains 1455
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T11:45:09.291Z
Learning: The runtime proxy is loopback-only and uses a per-process client token; it forwards only Responses API and model discovery requests
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T11:45:09.291Z
Learning: The persistent desktop app bind is reversible and edits user config/startup metadata, not official app binaries
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T11:45:09.291Z
Learning: The package does not publish a global `codex` bin; `codex-multi-auth-codex` is the explicit wrapper with auth commands running locally and non-auth commands forwarding to official Codex
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T11:45:09.291Z
Learning: Settings Q hotkey = cancel without save; theme live-preview restores baseline on cancel
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T11:45:40.404Z
Learning: Keep `codex` binary owned by the official OpenAI install path; use `codex-multi-auth-codex` wrapper only when intentionally choosing wrapper-launched sessions instead of direct Codex CLI usage
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T11:45:40.404Z
Learning: Design for personal development workflows where credentials stay local, runtime rotation is loopback-only, and account state remains visible and recoverable
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T11:45:40.404Z
Learning: Ensure that experimental features in Settings menu are non-destructive by default: sync previews before apply, preserve destination-only accounts, and fail safely on filename collisions
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T11:45:49.834Z
Learning: Documentation files should be organized by category: Start Here, Daily Use, Release History, Repair, Reference, Maintainer Docs, and Governance
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T11:45:49.834Z
Learning: Documentation governance is defined in DOCUMENTATION.md and establishes the contract for documentation standards and practices
🔇 Additional comments (12)
.codex-plugin/plugin.json (1)

3-3: LGTM!

package.json (1)

3-3: LGTM!

AGENTS.md (1)

6-6: LGTM!

README.md (1)

386-388: LGTM!

docs/README.md (1)

35-36: LGTM!

docs/releases/v2.2.0.md (1)

1-9: LGTM!

Also applies to: 14-14, 95-110

.gitignore (1)

36-37: LGTM!

lib/storage/paths.ts (1)

445-450: LGTM!

test/paths.test.ts (1)

775-781: LGTM!

lib/codex-manager/commands/switch.ts (1)

39-45: LGTM!

test/codex-manager-switch-command.test.ts (1)

70-80: LGTM!

lib/codex-manager.ts (1)

419-432: LGTM!

Comment thread lib/codex-manager.ts
…ntDetailText

CodeRabbit (#504) flagged missing coverage for the tone-precedence fix: a
failure detail containing both /failed|error/i and an unavailable keyword must
render danger (red), not be downgraded to warning (yellow).

- Export styleAccountDetailText (behavior-neutral) for direct unit testing
- Add test/codex-manager-detail-tone.test.ts: 5 cases covering compact path,
  'not available' variant, whitespace/newline normalization, warning-only
  control, and the quota-suffix branch

Forces legacy ANSI (v2 off) + TTY so danger=red / warning=yellow are
deterministic. Full suite: 4283 passed.
@ndycode

ndycode commented Jun 2, 2026

Copy link
Copy Markdown
Owner Author

Addressed the actionable review comment in 51d9a06.

Finding: missing coverage for styleAccountDetailText tone precedence (danger before warning).

Fix: exported styleAccountDetailText (behavior-neutral) and added test/codex-manager-detail-tone.test.ts with 5 cases:

  • compact path: refresh failed: service unavailable → danger (red), not warning
  • not available variant → danger
  • whitespace/newline normalization → danger
  • warning-only control (Codex not available for this account) → warning (yellow), confirming the regex still works when no failure keyword is present
  • quota-suffix branch: acct (12%) — refresh failed, now unavailable → suffix renders danger

Forces legacy ANSI (v2 off) + TTY so danger=red / warning=yellow are deterministic. Full suite: 4283 passed, 2 skipped, 0 failed; typecheck + lint clean.

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the new test file
fd 'codex-manager-detail-tone.test.ts' --exec cat {}

# Check the export change in lib/codex-manager.ts around styleAccountDetailText
rg -n 'styleAccountDetailText' --type ts -C 3

Length of output: 11602



@ndycode — fix looks solid. quick notes from the inspection:

  • lib/codex-manager.ts:409 — export is additive, signature unchanged, comment clearly documents the "why" for future maintainers. no behavior delta.
  • test/codex-manager-detail-tone.test.ts:18-37 — TTY override + v2Enabled: false setup/teardown is correct. stdoutIsTTYDescriptor captured at describe scope before any mutation, so restore is accurate. no concurrency risk (block is not concurrent).
  • 5 cases pin both paths (compact and quota-suffix) and include the warning-only control — that's the right regression surface.
  • minor gap, non-blocking: no case for "error" alone without any unavailable keyword (pure danger, no conflation). test/codex-manager-detail-tone.test.ts:47 covers error + not available together, which is the security-relevant case. a standalone "error" case would round it out but isn't required for the stated invariant.
  • test/repair-commands.test.ts:124 stub is passthrough — unaffected by the export change, confirmed.

full review triggered for the updated branch.

✅ Actions performed

Full review triggered.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/codex-manager.ts`:
- Around line 406-409: The export styleAccountDetailText is intentionally
exposed for unit tests; add an explicit JSDoc `@internal` tag above its
declaration to signal it’s test-only (keep the existing explanatory comment
about tone precedence/security). Locate the exported function named
styleAccountDetailText and prepend a JSDoc block containing `@internal` (and a
brief one-line summary if desired) so the intent is clear while preserving the
current comment and behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b8e8cfff-eca7-43ff-a4a1-3f6b2b932e96

📥 Commits

Reviewing files that changed from the base of the PR and between d8f714c and 51d9a06.

📒 Files selected for processing (2)
  • lib/codex-manager.ts
  • test/codex-manager-detail-tone.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (7)
test/**/*.test.ts

📄 CodeRabbit inference engine (test/AGENTS.md)

test/**/*.test.ts: Vitest globals (describe, it, expect) are enabled and should be used without explicit imports
Maintain 80% coverage threshold across statements, branches, functions, and lines
Use removeWithRetry for Windows filesystem cleanup instead of bare fs.rm to handle EBUSY/EPERM/ENOTEMPTY backoff
Use source files in tests, not compiled dist/ files; test the source directly
Do not skip tests without justification; include rationale if a test must be skipped
Relax ESLint rules for test files as specified in eslint.config.js

Files:

  • test/codex-manager-detail-tone.test.ts
**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Do not use as any, @ts-ignore, or @ts-expect-error - maintain proper type safety

Files:

  • test/codex-manager-detail-tone.test.ts
  • lib/codex-manager.ts
**/*.{ts,js}

📄 CodeRabbit inference engine (AGENTS.md)

Use ESM only ("type": "module"), target Node >= 18

Files:

  • test/codex-manager-detail-tone.test.ts
  • lib/codex-manager.ts
test/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Use Vitest for test suites with support for property tests and chaos tests

Files:

  • test/codex-manager-detail-tone.test.ts
test/**

⚙️ CodeRabbit configuration file

tests must stay deterministic and use vitest. demand regression cases that reproduce concurrency bugs, token refresh races, and windows filesystem behavior. reject changes that mock real secrets or skip assertions.

Files:

  • test/codex-manager-detail-tone.test.ts
lib/**/*.ts

📄 CodeRabbit inference engine (lib/AGENTS.md)

lib/**/*.ts: All public exports should flow through lib/index.ts or documented package subpaths
Never import from dist/ in source tests or library code
Never suppress type errors

Store core runtime logic following the lib/ directory structure: auth/, runtime/, request/, storage/, codex-cli/, codex-manager/, prompts/, recovery/, tools/, ui/

Files:

  • lib/codex-manager.ts
lib/**

⚙️ CodeRabbit configuration file

focus on auth rotation, windows filesystem IO, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios. check for logging that leaks tokens or emails.

Files:

  • lib/codex-manager.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T12:08:50.198Z
Learning: Canonical package name is `codex-multi-auth` and canonical command family is `codex-multi-auth ...`
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T12:08:50.198Z
Learning: The package does not publish a global `codex` bin; `codex-multi-auth-codex` is the explicit wrapper: auth commands run locally, non-auth commands forward to official Codex
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T12:08:50.198Z
Learning: Runtime rotation is default-on through `codexRuntimeRotationProxy`; users can opt out with `codex-multi-auth rotation disable` or `CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY=0`
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T12:08:50.198Z
Learning: The runtime proxy is loopback-only and uses a per-process client token, forwarding only Responses API and model discovery requests
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T12:08:50.198Z
Learning: The persistent desktop app bind is reversible and edits user config/startup metadata, not official app binaries
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T12:08:50.198Z
Learning: Do not bypass the official Codex CLI by reimplementing general Codex commands in the wrapper
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T12:08:50.198Z
Learning: Keep runtime rotation default-on behavior aligned with explicit release and migration documentation
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T12:08:50.198Z
Learning: Do not expose account emails or tokens in runtime proxy client response headers or logs
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T12:09:02.503Z
Learning: Store accounts in JSON format at `~/.codex/multi-auth/openai-codex-accounts.json` or in project-scoped paths under `~/.codex/multi-auth/projects/<project-key>/openai-codex-accounts.json`
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T12:09:02.503Z
Learning: Store settings in JSON format at `~/.codex/multi-auth/settings.json` or at the path specified by `CODEX_MULTI_AUTH_CONFIG_PATH`
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T12:09:02.503Z
Learning: Use `CODEX_MULTI_AUTH_DIR` environment variable to allow override of settings and accounts root directory
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T12:09:02.503Z
Learning: Implement health-aware account selection that considers quota state, cooldown state, and runtime metrics before rotating to a different account
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T12:09:02.503Z
Learning: Use bounded outbound request budgets to prevent a single prompt from exhausting the entire account pool during rotation
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T12:09:02.503Z
Learning: Keep credentials local and ensure runtime rotation is loopback-only with no external request forwarding outside the local machine
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T12:09:02.503Z
Learning: Use JSON format for all configuration files including settings, accounts, quota cache, policies, routing profiles, and budget guards
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T12:09:02.503Z
Learning: Implement OAuth callback handling on port `1455` for the default login flow, with fallback to device-auth and manual callback-paste flows when port is unavailable
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T12:09:02.503Z
Learning: Log all activity to `~/.codex/multi-auth/logs/codex-plugin/` with structured logging that supports runtime observability and diagnostics
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T12:09:02.503Z
Learning: Respect `CODEX_MODE`, `CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY`, `CODEX_MULTI_AUTH_APP_ROTATION_IDLE_MS`, and other feature-gate environment variables to allow runtime configuration of package behavior
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T12:09:02.503Z
Learning: Provide machine-readable JSON output for diagnostic commands like `status`, `report`, `check`, and `doctor` to support automation and tooling integration
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T12:09:35.895Z
Learning: Maintain a comprehensive test suite (4,200+ tests) with green results and pass all security audits (`npm run audit:ci`), typecheck, and lint validation
Learnt from: CR
Repo: ndycode/codex-multi-auth

Timestamp: 2026-06-02T12:09:35.895Z
Learning: Apply defensive security hardening across all layers (runtime, storage, prompt caching, path handling) to prevent common attack vectors like shell injection, path traversal, token exposure, and unauthorized network access
🔇 Additional comments (2)
lib/codex-manager.ts (1)

410-451: LGTM!

test/codex-manager-detail-tone.test.ts (1)

6-72: test coverage for tone precedence is solid.

the suite pins the danger-before-warning invariant in both compact (lib/codex-manager.ts:444-447) and quota-suffix (lib/codex-manager.ts:427-435) branches. forcing isTTY=true and v2Enabled=false on lines 24-28 makes ansi assertions deterministic.

five cases cover the key scenarios:

  • "failed" + "unavailable" → danger (test 1)
  • "error" + "not available" → danger (test 2)
  • whitespace/newline normalization → danger (test 3)
  • "unavailable" alone → warning, not danger (test 4, control)
  • quota-suffix with both keywords → danger (test 5)

this directly resolves the past review comment on lib/codex-manager.ts:445-447 that flagged missing regression coverage for overlapping keywords.

Comment thread lib/codex-manager.ts
@ndycode

ndycode commented Jun 2, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai the only actionable comment (tone-precedence test for styleAccountDetailText) is addressed in 51d9a06 and you confirmed it ("✅ Addressed in commits d8f714c to 51d9a06"). Resolved the thread. Please re-evaluate and clear the stale CHANGES_REQUESTED from the prior commit.

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

@ndycode — confirmed. the only blocking item (tone-precedence coverage for styleAccountDetailText) was addressed in 51d9a06, pinned across both the compact path (lib/codex-manager.ts compact branch) and the quota-suffix branch via test/codex-manager-detail-tone.test.ts:18-72. suite is clean at 4,283 passed. approving now to clear the stale status.

✅ Actions performed

Comments resolved and changes approved.

@ndycode
ndycode merged commit 29c8b4f into main Jun 2, 2026
2 checks passed
@ndycode
ndycode deleted the fix/post-merge-audit-lows branch June 2, 2026 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant