Skip to content

Improve Codex multi-account dashboard visibility - #3552

Open
xxchan wants to merge 1 commit into
steipete:mainfrom
xxchan:feat/dashboard-codex-accounts
Open

Improve Codex multi-account dashboard visibility#3552
xxchan wants to merge 1 commit into
steipete:mainfrom
xxchan:feat/dashboard-codex-accounts

Conversation

@xxchan

@xxchan xxchan commented Sep 11, 2026

Copy link
Copy Markdown

Summary

Expose Codex multi-account details consistently in the dashboard and menu bar Overview while preserving privacy and existing provider-level metrics.

Changes

  • Include all visible Codex accounts in the dashboard snapshot and render them as separate account cards.
  • Carry the actual active-account selection through the usage payload instead of assuming the first sorted account is active.
  • Redact account labels and expose opaque account IDs when dashboard identity redaction is enabled.
  • Add per-account credit balances, reset-credit counts, and expiry data; the WebUI shows the earliest expiry while retaining the provider-level financial summary.
  • Add provider-scoped WebUI display settings for hiding Spark windows and reset-credit counts.
  • Expand Codex stacked accounts into one row per account in CommandBar Overview, with safe activation IDs and no misleading provider-level history submenu.
  • Add multi-account snapshot coverage for active selection, redaction, opaque IDs, credits, and reset details.

Validation

  • swift build --product CodexBar
  • swift build --product CodexBarCLI
  • make check (passed on the initial revision; source changes are format-safe under git diff --check)
  • Focused dashboard snapshot assertions added for the multi-account projection.

The full test target was started but its first complete rebuild exceeded the local run window; no test failure was reported before it was stopped. The two production products compile successfully after the review fixes.

@clawsweeper

clawsweeper Bot commented Sep 11, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-11T03:45:27.760686Z f34bb12 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@clawsweeper

clawsweeper Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed September 11, 2026, 3:14 AM ET / 07:14 UTC (Revision 2).

ClawSweeper review

What this changes

Adds separate Codex account cards to the browser dashboard and native Overview, with account credits, reset details, and persisted browser display settings.

Regression provenance

Possible regression — suspected (reviewed change). No predecessor PR is attributed.

Merge readiness

Blocked before merge - 6 items remain

Several earlier findings are resolved, but account failure handling and grouped rendering still have concrete defects. The contribution remains useful and distinct from main; real behavior proof is still outstanding.

Priority: P2
Reviewed head: e90a46d399b5e8c7dc93d0639a2349f052365b3b

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) Useful work and several successful review fixes remain limited by three presentation defects and absent runtime proof.
Proof confidence 🦪 silver shellfish (2/6) Needs real behavior proof before merge: The captured evidence covers builds and snapshot assertions, not an observed run of the changed dashboard projection, persisted browser settings, or native Overview. Provide after-fix response output and screenshots or recordings from a real setup, including redaction and fresh/existing browser profiles; redact emails, tokens, IPs, and private endpoints. Updating the PR body should trigger re-review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🦐 gold shrimp (3/6) 3 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The captured evidence covers builds and snapshot assertions, not an observed run of the changed dashboard projection, persisted browser settings, or native Overview. Provide after-fix response output and screenshots or recordings from a real setup, including redaction and fresh/existing browser profiles; redact emails, tokens, IPs, and private endpoints. Updating the PR body should trigger re-review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 10 items Policy and review boundary: Read the complete root AGENTS.md; no additional ancestor policies or maintainer notes were found for the changed paths. Applied provider isolation, focused validation, and UI-proof guidance. No builds or tests were executed under the read-only review contract.
Pinned introduction: The verified merge-base-to-head delta changes nine files, with 373 additions and 37 deletions. Release metadata differences belong to base drift, not this PR.
Current-main and release necessity: Current main still maps each usage payload directly to a provider row without the new Codex account projection. The supplied latest release, v0.59.0, still sets includeAllCodexAccounts to false for dashboard collection; the requested dashboard behavior is therefore not already covered.
Findings 3 actionable findings [P2] [P2] Preserve active-account metadata when a fetch fails
[P2] [P2] Preserve shared cost history and failures in account groups
[P2] [P2] Use the disambiguated label for Codex account titles
Security None None.

How this fits together

CodexBar collects usage from locally configured accounts and projects it into an authenticated dashboard response and native menu cards. The browser combines that response with provider cost history to display account usage and shared spending.

flowchart LR
  A[Visible Codex accounts] --> B[Usage collection]
  B --> C[Dashboard account projection]
  D[Provider cost history] --> E[Browser dashboard]
  C --> E
  F[Browser display preferences] --> E
  A --> G[Native Overview rows]
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The captured evidence covers builds and snapshot assertions, not an observed run of the changed dashboard projection, persisted browser settings, or native Overview. Provide after-fix response output and screenshots or recordings from a real setup, including redaction and fresh/existing browser profiles; redact emails, tokens, IPs, and private endpoints. Updating the PR body should trigger re-review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • [P2] Preserve active-account metadata when a fetch fails (P2) - The new active flag is populated only on successful usage fetches. Error payloads leave it nil, so this fallback labels the first alphabetically sorted failed account active even when another account is selected; a selected account that fails later in the list loses its active badge. This leaves the earlier active-selection finding partially unresolved. Carry the known selection flag through failure outputs instead of deriving it from array position.
  • [P2] Preserve shared cost history and failures in account groups (P2) - Emitting Codex accounts now routes existing multi-account dashboards through this group instead of renderProvider. The restored numeric summary omits the daily-spend chart and provider.error: available /cost history disappears, and a failed cost collection leaves missing totals without the diagnostic previously shown. Preserve the shared financial section once per group, including its chart and failure message.
  • [P2] Use the disambiguated label for Codex account titles (P2) - The new Codex account projection supplies workspace-aware labels, but this existing renderer prefers identity.accountEmail whenever a fetch succeeds. Two workspaces sharing an email and plan therefore receive indistinguishable titles despite distinct labels and opaque IDs. Prefer the projected Codex label, which already follows redaction policy, so users can identify which workspace each usage card represents.
  • Resolve merge risk (P1) - Existing multi-account dashboard users would lose daily-spend charts and cost-failure diagnostics when the new account grouping replaces provider-card rendering.
  • Complete next step (P2) - Resolve the three findings, complete the revised-head test and lint checks, and add real browser and native Overview proof before merge.

Findings

  • [P2] [P2] Preserve active-account metadata when a fetch fails — Sources/CodexBarCLI/DashboardSnapshotBuilder.swift:65-68
  • [P2] [P2] Preserve shared cost history and failures in account groups — Sources/CodexBarCLI/CLIServeWebUI+HTML.swift:1158-1164
  • [P2] [P2] Use the disambiguated label for Codex account titles — Sources/CodexBarCLI/CLIServeWebUI+HTML.swift:978-979
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta Production +295/-36 lines; tests +78/-1 lines The production growth supports two account views and browser preferences, while added tests focus on snapshot projection.

Merge-risk options

Maintainer options:

  1. Preserve shared financial presentation (recommended)
    Render the existing provider cost chart and cost diagnostics once above the Codex account grid, with regression coverage for multi-account dashboards.

Technical review

Best possible solution:

Keep account identity and selection independent of fetch success, display disambiguated account labels, and reuse shared financial rendering alongside account cards.

Do we have a high-confidence way to reproduce the issue?

Yes, source establishes deterministic patch-level cases: a failed account fetch loses selection metadata, shared-email workspaces lose their distinguishing labels, and grouped Codex cards bypass financial rendering. These cases were not executed.

Is this the best way to solve the issue?

Not yet: reusing the existing account collector and snapshot contract is appropriate, but the projection and browser rendering must preserve account distinctions and existing provider information.

Full review comments:

  • [P2] [P2] Preserve active-account metadata when a fetch fails — Sources/CodexBarCLI/DashboardSnapshotBuilder.swift:65-68
    The new active flag is populated only on successful usage fetches. Error payloads leave it nil, so this fallback labels the first alphabetically sorted failed account active even when another account is selected; a selected account that fails later in the list loses its active badge. This leaves the earlier active-selection finding partially unresolved. Carry the known selection flag through failure outputs instead of deriving it from array position.
    Confidence: 0.99
  • [P2] [P2] Preserve shared cost history and failures in account groups — Sources/CodexBarCLI/CLIServeWebUI+HTML.swift:1158-1164
    Emitting Codex accounts now routes existing multi-account dashboards through this group instead of renderProvider. The restored numeric summary omits the daily-spend chart and provider.error: available /cost history disappears, and a failed cost collection leaves missing totals without the diagnostic previously shown. Preserve the shared financial section once per group, including its chart and failure message.
    Confidence: 0.98
  • [P2] [P2] Use the disambiguated label for Codex account titles — Sources/CodexBarCLI/CLIServeWebUI+HTML.swift:978-979
    The new Codex account projection supplies workspace-aware labels, but this existing renderer prefers identity.accountEmail whenever a fetch succeeds. Two workspaces sharing an email and plan therefore receive indistinguishable titles despite distinct labels and opaque IDs. Prefer the projected Codex label, which already follows redaction policy, so users can identify which workspace each usage card represents.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against f277e7ee6afd.

Labels

Label changes:

  • add P2: This is a bounded account-visibility improvement with display defects rather than an urgent runtime outage.
  • add merge-risk: 🚨 compatibility: Existing Codex dashboards with multiple accounts switch rendering paths and lose their shared cost chart and failure diagnostics.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The captured evidence covers builds and snapshot assertions, not an observed run of the changed dashboard projection, persisted browser settings, or native Overview. Provide after-fix response output and screenshots or recordings from a real setup, including redaction and fresh/existing browser profiles; redact emails, tokens, IPs, and private endpoints. Updating the PR body should trigger re-review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: This is a bounded account-visibility improvement with display defects rather than an urgent runtime outage.
  • merge-risk: 🚨 compatibility: Existing Codex dashboards with multiple accounts switch rendering paths and lose their shared cost chart and failure diagnostics.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The captured evidence covers builds and snapshot assertions, not an observed run of the changed dashboard projection, persisted browser settings, or native Overview. Provide after-fix response output and screenshots or recordings from a real setup, including redaction and fresh/existing browser profiles; redact emails, tokens, IPs, and private endpoints. Updating the PR body should trigger re-review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

  • Policy and review boundary: Read the complete root AGENTS.md; no additional ancestor policies or maintainer notes were found for the changed paths. Applied provider isolation, focused validation, and UI-proof guidance. No builds or tests were executed under the read-only review contract. (AGENTS.md:1, e90a46d399b5)
  • Pinned introduction: The verified merge-base-to-head delta changes nine files, with 373 additions and 37 deletions. Release metadata differences belong to base drift, not this PR. (e90a46d399b5)
  • Current-main and release necessity: Current main still maps each usage payload directly to a provider row without the new Codex account projection. The supplied latest release, v0.59.0, still sets includeAllCodexAccounts to false for dashboard collection; the requested dashboard behavior is therefore not already covered. (Sources/CodexBarCLI/CLIServeCommand.swift:1039, dca9c5f4b7a2)
  • Active metadata is success-only: Successful fetches carry codexVisibleAccount.isActive, but the failure branch uses makeProviderErrorPayload without that metadata. The new projection substitutes accountIndex == 0 for nil, although visible accounts are sorted by email rather than active selection. (Sources/CodexBarCLI/CLIUsageCommand.swift:583, e90a46d399b5)
  • Grouped financial rendering: The new group summary restores numeric totals, but only renderProvider renders state.costHistories and provider.error. Codex providers with multiple account payloads now bypass that renderer. (Sources/CodexBarCLI/CLIServeWebUI+HTML.swift:1161, e90a46d399b5)
  • Workspace distinction is discarded: CodexVisibleAccount.menuDisplayName includes workspace labels and collision discriminators, and CLIUsageCommand preserves it in payload.account. The browser instead prefers identity.accountEmail over the projected label, hiding those distinctions on successful cards. (Sources/CodexBarCLI/CLIServeWebUI+HTML.swift:978, e90a46d399b5)

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • sf-jin-ku: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Preserve active selection on failed account fetches and add mixed-success regression coverage.
  • Retain workspace labels, shared cost charts, and cost diagnostics in grouped rendering.
  • Provide real browser/native evidence and complete make test and make check on the revised head.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)
  • reviewed 2026-09-11T03:46:47.627Z sha f34bb12 :: needs real behavior proof before merge. :: [P1] [P1] Apply privacy mode to account labels and identifiers | [P2] [P2] Carry the actual active account into the dashboard | [P1] [P1] Retain the financial summary when showing account cards | [P2] [P2] Scope Overview history submenus to their account row | [P2] [P2] Keep account row identifiers compatible with activation | [P2] [P2] Exercise multiple payloads in the account-credit test

@xxchan
xxchan force-pushed the feat/dashboard-codex-accounts branch from f34bb12 to e90a46d Compare September 11, 2026 05:16
@xxchan

xxchan commented Sep 11, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Sep 11, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant