Skip to content

feat(codex): port multi-account Codex domain core (PR 1/5) - #255

Merged
Finesssee merged 3 commits into
nesszer:mainfrom
BlazzzPlay:feat/codex-account-domain
Aug 6, 2026
Merged

feat(codex): port multi-account Codex domain core (PR 1/5)#255
Finesssee merged 3 commits into
nesszer:mainfrom
BlazzzPlay:feat/codex-account-domain

Conversation

@BlazzzPlay

@BlazzzPlay BlazzzPlay commented Aug 5, 2026

Copy link
Copy Markdown

Summary

  • Port the Windows multi-account Codex core from ademisler/codexcontrol (MIT) into rust/src/codex_accounts/ (ADR 0003).
  • Domain only: account manager, quota/usage API, Codex Desktop restart, versioned stores, and models — all unit tested (32 tests).
  • MIT attribution added to NOTICE. No shell, UI, or behavior wiring yet; the ambient single-account Codex path is untouched.

Related issue

ADR 0003 (docs/adr/0003-multi-account-codex.md). No issue number yet.

Affected areas

  • Tray panel
  • Settings UI
  • Config file / settings persistence
  • Provider-specific behavior
  • Installer / release packaging
  • Startup / background behavior
  • Documentation (NOTICE + module docs)
  • Other:

Validation

Hosted PR check runs on Blacksmith Windows when CI_BUDGET_MODE is not off (see .github/workflows/pr-check.yml and CONTEXT.md). Still run the local slice and list commands/results below. If a check is not relevant, say why.

UI / tray proof

For UI, tray, settings, or visual behavior changes, use CUA Driver for visual proof. If CUA Driver cannot be used, explain why and attach equivalent manual proof.

  • Not applicable (domain layer only, no UI/tray surface touched)

Notes for reviewers

  • size:exception: this is a ~3,470-line PR. It is a faithful Rust port of the MIT-licensed ademisler/codexcontrol Windows core (per ADR 0003), not hand-written novel code; the vendor/migration diff does not split cleanly by file without breaking compilation. Review focus: diff against upstream semantics + the unit tests in-module.
  • Delivered as part of a stacked chain (ADR 0003). Follow-up PRs (shell commands, frontend bridge/i18n, settings panel, tray menu) will each be reviewed as separate focused PRs against main.

Chain Context

Field Value
Chain codex-multi-account (ADR 0003)
Tracker PR Not needed (stacked PRs to main)
Position 1 of 5
Base main
Depends on None
Follow-up #2 shell commands + lanes refresh
Review budget 3,474 / 400 (size:exception: faithful MIT port)
Starts at main
Ends with Domain core compiles standalone, 32 unit tests green

Chain Overview

main
 └── 📍 #1 This PR — domain core (rust/src/codex_accounts)
      └── #2 shell commands + multi-lane refresh
           └── #3 frontend bridge + i18n
                └── #4 settings accounts panel
                     └── #5 tray menu flyout

Scope

  • Includes: rust/src/codex_accounts/ domain port, lib.rs module wiring, NOTICE attribution.
  • Excludes: Tauri commands, frontend bridge, UI, multi-lane refresh — all follow-up PRs.

Autonomy

  • CI is expected to pass for this PR branch
  • This PR has one deliverable scope
  • This PR can be rolled back without unrelated changes
  • Tests cover this unit (32 in-module tests)

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@Finesssee

Copy link
Copy Markdown
Collaborator

Thanks for the PR, I will review it ASAP.

@Finesssee

Copy link
Copy Markdown
Collaborator

Thermo-nuclear series review (1/5 — domain) — direction-approved with two required items before merge.

The porting itself reads correctly: thiserror layering, canonicalize/strip-prefix guards on the delete path (remove_managed_files_if_owned refuses the managed-homes root — good), careful child-process bind/clear/cancel handling in CodexLoginRunner, and the ambient .codex-global-state.json creator_id rewrite mirrors the upstream Windows-Python logic faithfully. Discovery semantics (ambient account from ~/.codex/auth.json, managed homes under managed-homes/) and the switch flow (backup ambient auth, copy target, sync global state) follow a sane, test-covered shape.

Two items before this stack merges:

  1. The ADR you cite doesn't exist. The PR body references docs/adr/0003-multi-account-codex.md, but that file is absent on main and on every branch in this stack (feat/codex-account-{domain,shell,frontend-bridge,settings-panel,tray-menu} all 404). This stack adds a parallel account system next to the ambient single-account codex provider — that is exactly the kind of layering decision the repo records in docs/adr/. Please ADD docs/adr/0003-multi-account-codex.md to THIS PR (255) stating: (a) is the ambient rust/src/providers/codex single-account provider planned for replacement by codex_accounts, or permanent coexistence? (b) the identity precedence rule between the ambient slot and managed accounts, and (c) the trigger/timeline for retiring the ambient path if replacement is the goal.

  2. account_manager.rs lands at 1034 lines, crossing the repo's ~1000-line file-size bar in a single addition. The CodexLoginRunner/ManagedLoginProcess/wait_for_child/kill_and_drain block (~250 lines) is a clean seam — split it into rust/src/codex_accounts/login_runner.rs and keep account_manager.rs at ~780 lines of discovery/switch/remove logic. It maps 1:1 onto the upstream account_manager.py structure anyway, so the split costs nothing against the source mapping note in NOTICE.

Non-blocking notes: the CodexLoginOutcome enum + CodexAccountSource discriminant are the right typed shapes (no booleans-as-state); the Windows-specific creator_id rewrite via electron-persisted-atom-state.environment correctly mirrors upstream's observed MSIX session format; NOTICE carries MIT attribution properly.

For the stack mechanics: merge strictly in order 255 → 256 → 258 → 259 → 260 and rebase each subsequent branch after every merge (squash merges rewrite ancestry). FYI the two earlier auto-close comments on PR 5/5 came from our former 4-PR-per-7d interaction limit — today's guard update (#263) raises it to 15, so guard now passes on #260.

blazz and others added 3 commits August 6, 2026 13:16
Port the Windows multi-account Codex core from ademisler/codexcontrol
(MIT) into rust/src/codex_accounts: account manager, quota API, Codex
Desktop restart, versioned stores, and models.

MIT attribution added to NOTICE.
@Finesssee
Finesssee force-pushed the feat/codex-account-domain branch from 9307345 to d3a22c4 Compare August 6, 2026 06:16
@Finesssee
Finesssee merged commit 49e13b1 into nesszer:main Aug 6, 2026
2 checks passed
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.

2 participants