feat(codex): add multi-account Codex home aggregation#870
Open
jackcpku wants to merge 4 commits intoryoppippi:mainfrom
Open
feat(codex): add multi-account Codex home aggregation#870jackcpku wants to merge 4 commits intoryoppippi:mainfrom
jackcpku wants to merge 4 commits intoryoppippi:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds multi-account Codex support: new CLI flags (--codex-home, --by-account), session-source resolution, account-aware data loading/types, account-grouped daily/monthly/session reports, a reusable responsive usage table factory, and docs/README updates. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as Command Handler
participant Resolver as resolveSessionSources
participant Loader as loadTokenUsageEvents
participant Builder as Report Builder
participant Table as createUsageResponsiveTable / Renderer
User->>CLI: Run command (--codex-home, optional --by-account)
CLI->>Resolver: resolveSessionSources(codexHomeArg)
Resolver->>Resolver: Parse entries, expand ~, assign unique accounts
Resolver-->>CLI: SessionSource[] {account,directory}
CLI->>Loader: loadTokenUsageEvents({sessionSources})
loop per SessionSource
Loader->>Loader: Read sessions in directory
Loader-->>CLI: Emit TokenUsageEvent (with account)
end
CLI->>Builder: buildReport(events, {byAccount})
Builder->>Builder: Group by period (+ account if byAccount)
Builder-->>CLI: ReportRows[] (include account)
CLI->>Table: createUsageResponsiveTable({mode, includeAccountColumn})
Table-->>CLI: table, tableColumnCount
CLI->>Table: Render rows and totals (account column if present)
Table-->>User: Terminal table output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/codex/src/session-sources.ts (1)
105-107: IsolateCODEX_HOMEenv mutation in tests.This test deletes
process.env[CODEX_HOME_ENV]and does not restore it, which can leak state into later tests in the same process.✅ Suggested test hardening
if (import.meta.vitest != null) { describe('resolveSessionSources', () => { + const originalCodexHome = process.env[CODEX_HOME_ENV]; + afterEach(() => { + if (originalCodexHome == null) { + delete process.env[CODEX_HOME_ENV]; + return; + } + process.env[CODEX_HOME_ENV] = originalCodexHome; + }); + it('uses default CODEX_HOME when no override is provided', () => { delete process.env[CODEX_HOME_ENV]; const sources = resolveSessionSources();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/codex/src/session-sources.ts` around lines 105 - 107, The test mutates process.env[CODEX_HOME_ENV] without restoring it; update the test around resolveSessionSources (the 'uses default CODEX_HOME when no override is provided' test) to snapshot the original value of process.env[CODEX_HOME_ENV] before deleting it, perform the delete and run resolveSessionSources, and then restore the original value (or undefined) after the assertion (or use beforeEach/afterEach to manage setup/teardown) so no global env state leaks to other tests.apps/codex/src/commands/daily.ts (1)
126-165: Extract table schema construction into a shared helper to reduce drift.Line [127] onward mirrors near-identical header/alignment/compact setup in
apps/codex/src/commands/monthly.tsandapps/codex/src/commands/session.ts. Centralizing this avoids future column-order mismatches.♻️ Suggested direction
+// e.g. in a shared command table helper module +export function buildUsageTableConfig( + mode: "daily" | "monthly" | "session", + includeAccountColumn: boolean, +): { + head: string[]; + colAligns: TableCellAlign[]; + compactHead: string[]; + compactColAligns: TableCellAlign[]; +} { + // centralize schema selection by mode + account flag +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/codex/src/commands/daily.ts` around lines 126 - 165, Extract the repeated table schema construction (head, colAligns, compactHead, compactColAligns, tableColumnCount and ResponsiveTable init) into a shared helper function (e.g., buildCostTableSchema or createResponsiveTableConfig) and call it from daily.ts, monthly.ts, and session.ts; the helper should accept a boolean byAccount and return the head, colAligns, compactHead, compactColAligns, compactThreshold and tableColumnCount (or even return a preconfigured ResponsiveTable instance) so you remove the duplicated blocks surrounding the includeAccountColumn logic and the ResponsiveTable constructor while preserving existing symbols like ResponsiveTable, head, colAligns, compactHead, compactColAligns, compactThreshold and tableColumnCount.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/codex/src/data-loader.ts`:
- Around line 199-208: The derived account labels from options.sessionDirs use
only the basename (variable basename) which can produce collisions (e.g., two
different paths both named "sessions"); change the mapping logic that builds {
account, directory } (the options.sessionDirs.map block) to ensure uniqueness by
disambiguating duplicate account names — for example, compute accountCandidate =
allDirs.length <= 1 ? 'default' : (basename === '' ? fallbackAccount :
basename), then if accountCandidate already exists append a suffix (like
`-${index}`) or a short hash of path.resolve(dir) to form account, or
deduplicate in a post-pass that renames collisions consistently; update the code
which uses account (the account variable returned in the mapping) accordingly so
account labels are guaranteed unique.
In `@apps/codex/src/session-sources.ts`:
- Around line 70-73: The code passes codexHome into path.resolve without
expanding a leading tilde, so inputs like "~/.codex-work" are treated literally;
update the code that uses codexHome (the occurrences that compute baseName via
path.resolve(codexHome) and the other resolve at the other occurrence around
line 98) to first expand a leading "~" to the user's home directory (e.g.
codexHome = codexHome.replace(/^~(?=$|[\\/])/, os.homedir())), ensure you
import/require the os module, then call path.resolve on the expanded codexHome
before computing path.basename or other path operations.
In `@docs/guide/codex/index.md`:
- Line 70: The description for LOG_LEVEL contains a typo: replace the word
"consola" with "console" in the LOG_LEVEL table entry (the row referencing
`LOG_LEVEL`) so it reads "Adjust console verbosity (0 silent … 5 trace)"; update
only that string in the docs/guide/codex/index.md content where the `LOG_LEVEL`
table row is defined.
---
Nitpick comments:
In `@apps/codex/src/commands/daily.ts`:
- Around line 126-165: Extract the repeated table schema construction (head,
colAligns, compactHead, compactColAligns, tableColumnCount and ResponsiveTable
init) into a shared helper function (e.g., buildCostTableSchema or
createResponsiveTableConfig) and call it from daily.ts, monthly.ts, and
session.ts; the helper should accept a boolean byAccount and return the head,
colAligns, compactHead, compactColAligns, compactThreshold and tableColumnCount
(or even return a preconfigured ResponsiveTable instance) so you remove the
duplicated blocks surrounding the includeAccountColumn logic and the
ResponsiveTable constructor while preserving existing symbols like
ResponsiveTable, head, colAligns, compactHead, compactColAligns,
compactThreshold and tableColumnCount.
In `@apps/codex/src/session-sources.ts`:
- Around line 105-107: The test mutates process.env[CODEX_HOME_ENV] without
restoring it; update the test around resolveSessionSources (the 'uses default
CODEX_HOME when no override is provided' test) to snapshot the original value of
process.env[CODEX_HOME_ENV] before deleting it, perform the delete and run
resolveSessionSources, and then restore the original value (or undefined) after
the assertion (or use beforeEach/afterEach to manage setup/teardown) so no
global env state leaks to other tests.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/codex/README.mdapps/codex/src/_shared-args.tsapps/codex/src/_types.tsapps/codex/src/commands/daily.tsapps/codex/src/commands/monthly.tsapps/codex/src/commands/session.tsapps/codex/src/daily-report.tsapps/codex/src/data-loader.tsapps/codex/src/monthly-report.tsapps/codex/src/session-report.tsapps/codex/src/session-sources.tsdocs/guide/codex/daily.mddocs/guide/codex/index.mddocs/guide/codex/monthly.mddocs/guide/codex/session.md
Contributor
Author
|
Addressed the review feedback in commit
Validation run:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/codex/src/data-loader.ts`:
- Around line 236-241: The fallback logic for computing sessionSources builds a
single path from CODEX_HOME (using CODEX_HOME_ENV, DEFAULT_CODEX_DIR,
DEFAULT_SESSION_SUBDIR) and therefore skips the multi-account parsing used
elsewhere; update this to delegate to the same resolver used by the rest of the
code (e.g., call resolveSessionSources or the existing session-parsing utility)
so comma-separated and label=path entries in CODEX_HOME are parsed the same way.
Concretely, when providedSources and providedDirs are both undefined, compute
the fallback input as the resolved CODEX_HOME (defaulting to DEFAULT_CODEX_DIR)
+ DEFAULT_SESSION_SUBDIR and pass that through resolveSessionSources (or the
same parsing function) to produce sessionSources, instead of building a single
[{ account: 'default', directory: ... }] entry. Ensure you reference
CODEX_HOME_ENV, DEFAULT_CODEX_DIR, DEFAULT_SESSION_SUBDIR,
resolveSessionSources, and loadTokenUsageEvents to locate the change.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/codex/README.mdapps/codex/src/commands/daily.tsapps/codex/src/commands/monthly.tsapps/codex/src/commands/session.tsapps/codex/src/commands/usage-table.tsapps/codex/src/data-loader.tsapps/codex/src/session-sources.tsdocs/guide/codex/index.md
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/codex/src/commands/monthly.ts
Contributor
Author
|
Addressed the additional CodeRabbit finding in commit
Validation:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/codex/src/data-loader.ts`:
- Around line 179-187: The current makeUniqueAccountLabel(base, usedAccounts)
can produce labels that collide with existing explicit labels (e.g., "work-2");
change the logic to generate a candidate label and loop while
usedAccounts.has(candidate) incrementing a counter (start at 1 or 2 as
appropriate) until you find a candidate not present in usedAccounts, then set
usedAccounts.set(candidate, 1) and also update the base's counter
(usedAccounts.set(base, newCount)) so future generations continue from the
correct number; apply the same fix pattern to the analogous dedup
functions/blocks at the other ranges referenced (lines ~203-209 and ~218-226)
using the same candidate loop and usedAccounts.has(...) checks.
Contributor
Author
|
Addressed the new account-label collision finding in commit Root cause:
What changed:
Regression tests added:
Validation:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/codex/src/data-loader.ts (1)
174-201: Extract account-label deduplication into a shared helper.
makeUniqueAccountLabelsis now effectively duplicated in bothapps/codex/src/data-loader.tsandapps/codex/src/session-sources.ts. Centralizing this logic would reduce drift risk in future suffix-collision fixes.Also applies to: 214-245
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/codex/src/data-loader.ts` around lines 174 - 201, Extract the duplicated account-label deduplication into a single exported helper and replace both copies with imports: create a shared function file that exports makeUniqueAccountLabels (keeping the existing reservedLabels/usedLabels and suffix logic) and optionally export normalizeAccountLabel if also duplicated, then update callers in the two locations to import and use the shared makeUniqueAccountLabels (and normalizeAccountLabel) instead of their local implementations; ensure the exported names match (makeUniqueAccountLabels, normalizeAccountLabel) and existing behavior/tests remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/codex/src/data-loader.ts`:
- Around line 174-201: Extract the duplicated account-label deduplication into a
single exported helper and replace both copies with imports: create a shared
function file that exports makeUniqueAccountLabels (keeping the existing
reservedLabels/usedLabels and suffix logic) and optionally export
normalizeAccountLabel if also duplicated, then update callers in the two
locations to import and use the shared makeUniqueAccountLabels (and
normalizeAccountLabel) instead of their local implementations; ensure the
exported names match (makeUniqueAccountLabels, normalizeAccountLabel) and
existing behavior/tests remain unchanged.
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
@ccusage/codexvia--codex-home(supports comma-separated paths andlabel=path)--by-accountreport grouping for daily/monthly and account-aware session rendering(account, sessionId)Implementation details
session-sources.tsparser resolves sources from CLI orCODEX_HOMEloadTokenUsageEventsnow acceptssessionSourcesand annotates each event withaccountValidation
pnpm --filter @ccusage/codex formatpnpm --filter @ccusage/codex testpnpm --filter @ccusage/codex typecheckRelated: #839
Summary by CodeRabbit
New Features
Documentation
Tests