feat: full Rust rewrite (17-crate Cargo workspace) - #640
Open
ndycode wants to merge 13 commits into
Open
Conversation
Wave 0 of the Rust rewrite: workspace Cargo.toml, per-crate manifests with frozen dependency sets, complete module trees, and PORT-PENDING stubs for every planned .rs file. cargo check --workspace is green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Full port of the foundation layer: constants, CodexError family, serde models (PluginConfig, AccountStorageV3, flagged, WAL journal, tokens), byte-compatible json_io, fs_retry with Win32 errno mapping, runtime paths probing ladder, masking logger, JWT/token utils, display-width tables, table formatter, clock seam, shutdown registry. Test sandbox + golden byte fixtures generated from the TS reference implementation. 376 tests passing; clippy -D warnings clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rsion Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…r, usage, tui, auth (wave 1) Full ports: plugin config (54 getters, lockfile, mtime-CAS save, explain report), account storage (WAL-first save pipeline, recovery ladder, 4-tier identity matching, transactions, flagged store, named backups), rotation trackers/hybrid selector (never-persisted by construction), codex-cli mirror state/sync/writer, usage ledger with toFixed(8) cost parity, hand-rolled TUI (select engine, themes, dashboard builders), auth (PKCE OAuth, device auth, browser openers, refresh lease/queue, local client tokens). settings.json golden reproduces the TS fixture byte-for-byte. Known gap: auth callback_server.rs still stubbed (implementer connection dropped); follow-up in next commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Port of lib/auth/server.ts: never-rejecting bind on fixed port 1455 (ready:false + bindErrorCode on conflict), first-code-wins capture, 100ms poll / 5-min timeout, frozen route/status/header contract, bundled success page via include_str!. 22 tests incl. live port-1455 serial suite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Port of lib/accounts.ts + sidecar stores: AccountManager (never-persisted runtime state wrap, 4-tier identity hydration, codex-cli token merge, transactional commit_refreshed_auth), selection (round-robin, #509 drain-first no-advance, hybrid delegation, tracker interactions), persistence (build_storage_snapshot as the only storage bridge, #474 strictly-greater pin adoption, 500ms debounced save, 7-day rate-limit clamp with reason->key matrix), session affinity (#474 generation drop), capability/entitlement stores, account policies + routing profiles with golden byte parity. 216 tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Port of the request pipeline: transformer (background-mode gating, fast-session, plan/capability tool sanitization), model map (full alias table + hand-rolled codex-max lookbehind semantics), SSE response handler (incomplete=success, raw-passthrough at original status, synthetic 502 terminal errors), stream failover (pre-first-byte replay, cap 1), fetch helpers (7-day retry-after clamp, quota-404->429), error classification, rate-limit backoff/decision (seconds-vs-ms epoch heuristics), prompts cache (ETag+sha256, SWR, bundled fallback). Implements the ModelCatalog/ModelProfileProvider seams for accounts. 541 tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Port of forecast (pure H5 risk engine), quota probe (6-model chain, header reset-at heuristics, observability hook seam), quota cache with golden byte parity, readiness (RAW >=100 exhaustion), preemptive scheduler (in-memory H4 rules, never persisted), budget guard (UTC windows vs usage ledger), audit JSONL rotation, live account sync (notify watcher + poll fallback + debounce), runtime policy gate. 161 tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Port of the runtime layer: app-bind (byte-exact config.toml rewriting, sha256 gate, detached router spawn, #614 orphan self-heal, R3 router binary swap), first-run O_EXCL claim, rotation modules (choose_account pin>sequential>affinity>hybrid, #474 no-markSwitched-on-pinned, #509 no-advance, #606 stale-state recovery, #623 forced-index), monotonic extend-only token refresh cooldowns, account check engine, oauth flow orchestration, observability snapshot, recovery library (R2), services lifecycle. 281 tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The hard merge: one request pipeline implementing the union of the index.ts loader fetch state machine and the runtime rotation proxy loop (selection, refresh, model fallback chains, 429 mark-before-sleep, stream failover cap 1, token-invalidation no-rotate, synthetic terminal bodies, counters surviving fallback restarts). Proxy HTTP surface with loopback validation, timing-safe client auth, stable error codes, 64MiB cap, header scrub. Local bridge (/health /v1/models /v1/responses). App router binary with R3 idle-exit. Wrapper: forwarding, shadow CODEX_HOME with sync-back, --account force (#623) fail-hard, bin resolver, mcodex monitor/tmux. 166 tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The full CLI: dispatcher (29-command registry, auth-prefix normalization, frozen error strings), all commands with exact flag grammars/exit codes/JSON rules, repair suite (fix never deletes, doctor 15 checks with masked identities, verify-flagged with drift guards), health check (no re-base asymmetry, re-enables working disabled accounts), login dashboard flow + settings hub (11 toggles + 16 numerics, per-path FIFO write queue), formatters, oc-chatgpt import/orchestrator, update notice, integration generators. 436 tests green; workspace clippy-clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Five thin binary mains (codex-multi-auth, codex-multi-auth-codex, mcodex, app-launcher, app-router) wiring manager/wrapper/proxy/runtime, plus the documentation.test.ts analogue asserting the 29-command registry, --json anchors, and stable error-code tables against docs/reference. Workspace fully green: 2976 tests / 0 failures across 55 binaries, clippy -D warnings clean, release build clean. E2E smoke verified sandboxed: status/list/config explain/rotation status parity, golden account pool renders correctly, wrapper forwards to the real codex CLI through a shadow home. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eview 8 subsystem reviewers diffed TS vs Rust line-by-line; 32 candidates were independently verified adversarially, 27 confirmed and all fixed with pinning tests. Highlights: refresh-queue lazy-future wedge (critical, now eagerly spawned), error ladder keyed off mapped status so remapped 404 usage-limit/entitlement responses hit the 429/403 branches (high), streaming forward failures now do account bookkeeping + correct ledger rows (high), Windows shadow-home junctions instead of privileged symlinks (high), stray FORCE_ACCOUNT_INDEX scrubbed (high), atomic 0600 file modes, i64 saturating retry-after math, lease release-before-await, toast debounce gates, TUI numpad/signal/local-date fixes. Full re-gate: 2998 tests / 0 failures, clippy -D warnings clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Important Review skippedToo many files! This PR contains 355 files, which is 255 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (355)
You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
OutcomeThe trigger reproduces the pull request summary but contains no requested change, review finding, CI failure, or other actionable follow-up. The working tree was already clean, so I did not modify files, create an empty commit, or open a redundant pull request. Checks
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| // Best-effort reset-marker unlink (the marker persists across clears | ||
| // until this very unlink — spec 02 §3.4). | ||
| let _ = fs::remove_file(&reset_marker_path); |
There was a problem hiding this comment.
when windows antivirus or another process transiently locks the reset-intent marker, this best-effort removal silently fails after the new account file is committed. the next load treats the stale marker as an intentional reset and returns empty storage, making the newly saved accounts and credentials disappear.
Context Used: speak in lowercase, concise sentences. act like th... (source)
Knowledge Base Used: Account Storage Layer
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/storage/src/save.rs
Line: 229
Comment:
**stale reset intent hides accounts**
when windows antivirus or another process transiently locks the reset-intent marker, this best-effort removal silently fails after the new account file is committed. the next load treats the stale marker as an intentional reset and returns empty storage, making the newly saved accounts and credentials disappear.
**Context Used:** speak in lowercase, concise sentences. act like th... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
**Knowledge Base Used:** [Account Storage Layer](https://app.greptile.com/zeian/-/custom-context/knowledge-base/ndycode/codex-multi-auth/-/docs/storage-accounts.md)
How can I resolve this? If you propose a fix, please make it concise.
| use cma_core::schemas::account_storage::AccountStorageV3; | ||
| use cma_core::schemas::flagged::FlaggedAccountStorageV1; | ||
|
|
||
| static STORAGE_MUTEX: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); |
There was a problem hiding this comment.
when the cli and runtime proxy, or two cli processes, mutate the same account store concurrently, each process acquires its own tokio mutex and independently replaces the shared wal and primary file. a stale read-modify-write transaction can therefore overwrite a newer account selection or credential update.
Context Used: speak in lowercase, concise sentences. act like th... (source)
Knowledge Base Used: Account Storage Layer
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/storage/src/transactions.rs
Line: 33
Comment:
**storage lock excludes other processes**
when the cli and runtime proxy, or two cli processes, mutate the same account store concurrently, each process acquires its own tokio mutex and independently replaces the shared wal and primary file. a stale read-modify-write transaction can therefore overwrite a newer account selection or credential update.
**Context Used:** speak in lowercase, concise sentences. act like th... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
**Knowledge Base Used:** [Account Storage Layer](https://app.greptile.com/zeian/-/custom-context/knowledge-base/ndycode/codex-multi-auth/-/docs/storage-accounts.md)
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+676
to
+678
| fn escape_windows_batch_path(value: &str) -> String { | ||
| value.replace('%', "%%") | ||
| } |
There was a problem hiding this comment.
when a windows installation or user-profile path contains a cmd metacharacter such as &, ^, or |, this helper escapes only percent characters before interpolating the path into the startup .cmd file. cmd.exe alters or splits the command, so the app router does not start and the bound codex desktop app cannot reach its configured provider.
Context Used: speak in lowercase, concise sentences. act like th... (source)
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/runtime/src/app_bind.rs
Line: 676-678
Comment:
**batch paths leave metacharacters active**
when a windows installation or user-profile path contains a cmd metacharacter such as `&`, `^`, or `|`, this helper escapes only percent characters before interpolating the path into the startup `.cmd` file. cmd.exe alters or splits the command, so the app router does not start and the bound codex desktop app cannot reach its configured provider.
**Context Used:** speak in lowercase, concise sentences. act like th... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
**Knowledge Base Used:**
- [Runtime Services](https://app.greptile.com/zeian/-/custom-context/knowledge-base/ndycode/codex-multi-auth/-/docs/runtime-services.md)
- [Codex CLI Integration](https://app.greptile.com/zeian/-/custom-context/knowledge-base/ndycode/codex-multi-auth/-/docs/codex-cli-integration.md)
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+248
to
+256
| fn lock_for(key: &Path) -> Arc<TokioMutex<()>> { | ||
| let mut guard = APP_BIND_LOCKS | ||
| .lock() | ||
| .unwrap_or_else(|poison| poison.into_inner()); | ||
| guard | ||
| .entry(key.to_path_buf()) | ||
| .or_insert_with(|| Arc::new(TokioMutex::new(()))) | ||
| .clone() | ||
| } |
There was a problem hiding this comment.
when two processes run the app-bind command concurrently, each process owns a separate mutex and both rewrite config.toml and the bind-state file. the last writes can combine configuration and state from different router instances, leaving the recorded hash, port, or client key inconsistent and causing routing or unbind failures.
Context Used: speak in lowercase, concise sentences. act like th... (source)
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/runtime/src/app_bind.rs
Line: 248-256
Comment:
**bind lock excludes other processes**
when two processes run the app-bind command concurrently, each process owns a separate mutex and both rewrite `config.toml` and the bind-state file. the last writes can combine configuration and state from different router instances, leaving the recorded hash, port, or client key inconsistent and causing routing or unbind failures.
**Context Used:** speak in lowercase, concise sentences. act like th... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
**Knowledge Base Used:**
- [Runtime Services](https://app.greptile.com/zeian/-/custom-context/knowledge-base/ndycode/codex-multi-auth/-/docs/runtime-services.md)
- [Codex CLI Integration](https://app.greptile.com/zeian/-/custom-context/knowledge-base/ndycode/codex-multi-auth/-/docs/codex-cli-integration.md)
How can I resolve this? If you propose a fix, please make it concise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Full Rust rewrite of codex-multi-auth as a 17-crate Cargo workspace under
crates/, added alongside the existing TypeScript tree (the TS plugin keeps shipping for JS-host users — R1 below).codex-multi-auth,codex-multi-auth-codex,mcodex,codex-multi-auth-app-launcher, and the newcodex-multi-auth-app-router)clippy -D warningsclean, debug + release builds green~/.codex/multi-auth/, same JSON key order/indent/trailing-newline conventions — verified by golden byte fixtures generated from the TS implementation (crates/testkit/goldens/)--jsonstable anchors — pinned by adocumentation.test.ts-analogue docs-parity testpreserve_order, crossterm (hand-rolled TUI port), clap-free hand-parsed command grammarsArchitecture
Key decisions (full rationale in the architecture doc used to drive the port):
index.ts(JS-host plugin) is not ported — its per-request loader state machine is merged with the runtime rotation proxy loop into one pipeline (cma-proxy::pipeline), used by the proxy HTTP surface, the wrapper, and the local bridgecodex-multi-auth-app-routerreplacesscripts/codex-app-router.js; app-bind state keeps its JSON shape (nodePathnow stores the router binary path,routerScriptPathempty; legacy TS binds detected and re-bound)Serializeimpl — the 6.4.0 regression class is now unrepresentablecodex-multi-auth-codex, force specific account / pool #623, 7-day retry-after clamp, WAL-first storage, refresh lease/queue single-flightVerification
dist/buildCODEX_HOME/CODEX_MULTI_AUTH_DIR): usage/status/list/config explain/rotation status parity, golden account pool renders correctly, wrapper forwards to the real installed codex CLI (codex-cli 0.142.5) through a shadow home, resolver-error strings byte-matchscripts/codex.jsCI
.github/workflows/rust.yml: check + clippy + test on ubuntu-latest and windows-latest, Rust 1.96.0 pinned.Not ported (deliberate)
index.ts/ vendored@codex-ai/*shims (JS-host only; logic absorbed — R1)postinstall,preuninstall,repo-hygiene, pack budget)hashlinetools + session-recovery hook ported as library code but not wired (R2; JS-host-only surfaces)Known accepted deviations
Documented in module headers throughout; the notable ones:
Displaytext (fixed prefixes/suffixes frozen)getAccountsSnapshotdeep-clones (TS shared nested arrays with the live pool)🤖 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
this pr adds a full rust workspace alongside the existing typescript implementation.
Confidence Score: 2/5
this pr is not safe to merge until account-store concurrency, app-bind concurrency, stale reset-marker handling, and windows startup path escaping are fixed.
concurrent processes can overwrite shared credential or app-bind state, a transient windows marker-cleanup failure can make saved accounts load as empty, and valid windows paths containing cmd metacharacters can prevent the bound router from starting.
Files Needing Attention: crates/storage/src/save.rs, crates/storage/src/transactions.rs, crates/runtime/src/app_bind.rs
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR cli[cli and wrappers] --> runtime[runtime services] runtime --> auth[oauth and token refresh] runtime --> proxy[local proxy pipeline] proxy --> request[request transform and failover] request --> upstream[chatgpt and codex api] runtime --> rotation[account rotation] rotation --> storage[account storage] auth --> storage storage --> codex[codex cli files] launcher[app launcher] --> router[app router] router --> proxyPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(rust): resolve 27 confirmed parity d..." | Re-trigger Greptile
Context used (6)