Skip to content

Add a remote serve client, multi-account serve payloads, and durable sync credentials - #3791

Draft
VACInc wants to merge 30 commits into
steipete:mainfrom
VACInc:feature/remote-codexbar-serve
Draft

VACInc wants to merge 30 commits into
steipete:mainfrom
VACInc:feature/remote-codexbar-serve

Conversation

@VACInc

@VACInc VACInc commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Summary

High Level TLDR

CodexBar can already act as a server: codexbar serve publishes a token-protected dashboard snapshot. Nothing could consume it. This adds the client half: a Sync settings pane that points CodexBar at a remote codexbar serve endpoint and renders that machine's providers and accounts in the normal menus, plus an optional remote-only mode that skips local probes entirely.

It also fixes two things the client half exposed: the serve payload only ever described a provider's selected account, and the saved bearer token was unreadable after any change to the app's code signature.

What this changes

Remote client

  • RemoteCodexBarSnapshot fetches and decodes GET /dashboard/v1/snapshot, with schema-version gating, a streaming response size limit, and last-good-snapshot retention for transient failures.
  • A Sync preferences pane configures the endpoint, bearer token, and explicit consent for unencrypted private-network/Tailscale HTTP. Endpoint and token are stored as one atomic Keychain record so an interrupted edit can never pair one server's URL with another server's token.
  • Optional "use every provider from this server only" mode suppresses local provider probes.

Multi-account serve payload

  • serve and the one-shot dashboard command previously collected only the selected Codex/token account, so a provider row could never describe more than one account. They now enumerate every account and fold them into one provider entry carrying accounts[], with the serving machine's active account marked and ordered first.
  • Remote accounts render through the same AccountMenuLayoutPlanner path as local ones, so stacked vs compact layout follows the existing user setting. Remote accounts are non-activatable from the client, so segmented layout goes compact from two accounts up.

Keychain credential durability

  • The credential is stored with kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly and a non-secret tombstone instead of a delete, so a surviving legacy cache item is never silently re-migrated.
  • A code-signature change no longer discards the credential; see Root Cause.

Root Cause

Two defects, both found by running the client against a real serve instance.

1. Single-account serve payload. DashboardSnapshotBuilder was invoked with the selected account only. The snapshot schema had no place to put siblings, so a machine with two signed-in Codex accounts served one and the client faithfully displayed one. Reproduced by diffing the one-shot dashboard JSON from a release build against this branch on the same machine with the same two accounts:

release build:   codex  accounts = 0   (no "accounts" key emitted)
this branch:     codex  accounts = 2
                 ibmbob accounts = 2

2. Credential lost on signature change. The Keychain item's access-control list is bound to the creating binary's designated requirement. When the running binary is not on that list, SecItemCopyMatching reports errSecInteractionNotAllowed rather than a missing item. KeychainRemoteCodexBarTokenStore.loadCredential() collapsed that state into .temporarilyUnavailable, and every query applies KeychainNoUIQuery, so no authorization prompt could ever be presented. The token silently read back empty and Sync showed itself as unconfigured.

The same stale record also blocked recovery by hand: SecItemUpdate against an item the process cannot decrypt fails, so re-entering the token surfaced "could not be saved securely" and the user was wedged with no in-app path out.

Fix: keep .interactionRequired distinct, and add loadCredentialAllowingInteraction(), which reads with Keychain UI allowed and then deletes and re-adds the record so the current binary owns the new ACL and subsequent launches are silent again. Deletion does not decrypt the payload, so it succeeds without the access the process is missing. Saving takes the same re-own path instead of failing. SettingsStore attempts recovery at most once per launch and offers an explicit "Unlock Saved Token" control for a declined prompt. The plain-HTTP consent now mirrors to UserDefaults next to the server URL, so it survives a failed read the same way the endpoint already did.

Real behavior proof

Two machines on a private network, one running codexbar serve bound off-loopback with a bearer token, one running the client.

Multi-account payload, same machine and same two signed-in Codex accounts, release build vs this branch:

$ CodexBarCLI dashboard | jq '.providers[] | {id, accounts: (.accounts|length)}'
release:      {"id":"codex","accounts":0}
this branch:  {"id":"codex","accounts":2}

The client then rendered both Codex accounts in the menu through the existing multi-account layout.

Credential recovery, exercised against a real Keychain item created by a different code signature: the pre-fix binary read back an empty token with no prompt and rejected a re-entered token with "could not be saved securely"; the post-fix binary presented one authorization prompt, recovered the stored token and plain-HTTP consent, and read silently on every later launch of that binary.

What was not tested

  • Only arm64 macOS was exercised. No Intel or CI-matrix hardware run beyond what the workflows cover.
  • The interactive Keychain path is inherently UI-driven and is covered by unit tests through a protocol fake, not by an automated UI test.
  • No performance benchmark. The remote client adds one periodic HTTP fetch on the existing refresh cadence and does not change any local hot path.
  • Long-lived soak of the remote connection (days) was not run.

Verification

  • make test — green, using the repository's suite-splitting harness.
  • make format — clean.
  • New tests: multi-account snapshot projection and remote multi-account menu rendering; credential recovery through one authorization prompt; declined prompt not repeated within a launch.
  • ./Scripts/package_app.sh release — succeeds, including the packaged-app launch smoke check.

Pre-existing on this branch and not introduced here: ProviderArchitectureGatekeeperTests reports stale suppressed-reference anchors in files this PR does not touch, and SwiftLint reports three violations in pre-existing lines (UsageStore.swift file length, one long line, one trailing-closure style). Happy to fold the anchor cleanup in if you would rather have it here than as a follow-up.

In remote-only mode the menu-bar icon serves from
remoteCodexBarPrimarySnapshots, but iconObservationToken only
observed local snapshot state. The status item therefore never
re-rendered when served snapshots arrived, while the menu
(menuObservationToken) did and rendered filled usage bars.

Track remoteCodexBarSnapshots and remoteCodexBarPrimarySnapshots in
iconObservationToken so icon updates fire on remote snapshot changes.
Ad-hoc replacement builds can lose access to a prior build's Keychain
ACL. Previously Connect failed outright even though keeping the bearer
token in process memory is safe for the current session.

Allow an initial connection to proceed session-only when the secure
write fails, while preserving the existing authority pair if a token
replacement cannot be persisted. Explain that the token will be
forgotten on quit and cover the fallback with a regression test.
The remote snapshot emitted one account per provider: the dashboard path
collected only the selected account, and accounts[] was claude-swap only.
Collect every token/Codex account, fold them into one provider row whose
accounts[] carries them all, mark the serving Mac's selection active, and
render remote accounts through the shared multi-account menu layout.
Add remote-only served provider mode shifted PreferencesProvidersPane.swift
by one line, so the three allowlisted Codex account-state guards no longer
matched their recorded anchors and the gatekeeper reported them as
unjustified provider-specific constructs. Point the entries at lines
275/291/304.
The dashboard credential is a Keychain generic password whose access-control
list names the binary that created it. A signature change invalidates that
entry, so the preflight returns .interactionRequired, which loadCredential()
collapsed into .temporarilyUnavailable. Every query is no-UI, so the user never
saw a prompt: the token and the plain-HTTP consent silently went blank and had
to be retyped after each update.

Keep .interactionRequired distinct, and add loadCredentialAllowingInteraction(),
which reads with Keychain UI allowed and then deletes and re-adds the item so
the current binary owns the new record and later launches stay silent. Saving
takes the same recovery path instead of failing, and the plain-HTTP consent now
mirrors to UserDefaults alongside the server URL so it survives a failed read.

SettingsStore attempts recovery once per launch and exposes an "Unlock Saved
Token" control in Preferences for a declined prompt.
@clawsweeper

clawsweeper Bot commented Sep 20, 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.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. 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 20, 2026
@clawsweeper

clawsweeper Bot commented Sep 20, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed September 20, 2026, 5:43 PM ET / 21:43 UTC (Revision 2).

ClawSweeper review

What this changes

Adds a remote usage client in Sync settings, multi-account dashboard responses, and persistent endpoint-bound credentials with Keychain recovery.

Merge readiness

Blocked before merge - 17 items remain

Keep open: this is meaningful work absent from current main, but all six prior findings remain on the unchanged head. The supplied successful-path evidence does not resolve the credential revocation and privacy blockers.

Priority: P2
Reviewed head: 441a40f6fcdb3277781af596b82fa5b01a7c354e
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) Useful implementation and real successful-path evidence are outweighed by six unresolved findings, including credential and privacy defects.
Proof confidence 🦐 gold shrimp (3/6) Needs stronger real behavior proof before merge: Authority-chain proof required: the captured arm64 dashboard output and real Keychain recovery report support successful collection and access, but do not show that Disconnect with a surviving legacy record prevents authenticated requests after restart. After repair, provide redacted transport logs or terminal evidence for that final-I/O boundary and privacy-mode output; screenshots or video can additionally demonstrate the native recovery menu. Remove tokens, private addresses, and endpoints. Updating the PR body should trigger 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 🦪 silver shellfish (2/6) Security review found an item that needs attention.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: Authority-chain proof required: the captured arm64 dashboard output and real Keychain recovery report support successful collection and access, but do not show that Disconnect with a surviving legacy record prevents authenticated requests after restart. After repair, provide redacted transport logs or terminal evidence for that final-I/O boundary and privacy-mode output; screenshots or video can additionally demonstrate the native recovery menu. Remove tokens, private addresses, and endpoints. Updating the PR body should trigger 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 11 items Policy and review scope: Read the full root AGENTS.md. Tracked policy discovery found no nested AGENTS.md or maintainer notes. Applied credential isolation, provider siloing, focused-test, and menu-recovery guidance; no builds, tests, live account probes, or Keychain operations were executed.
Pinned ownership and re-review continuity: Reviewed the introduced merge-base-to-head changes, not endpoint drift. HEAD remains the exact revision reviewed previously; comparison against that revision is empty. All six retained prior findings were checked independently against source. No verified test merge is available, so no main-branch deletion claims are made.
Still necessary on current main and latest release: Current main still builds provider rows without generic multi-account grouping, and its dashboard and serve collectors explicitly select one Codex account. The remote client is absent. The peeled v0.63.0 tag equals fetched main, so the requested implementation is also absent from that release.
Findings 6 actionable findings [P1] Use opaque account IDs in redacted dashboard output
[P1] Distinguish a credential tombstone from an absent record
[P2] Preserve the durable credential until replacement succeeds
Security Needs attention Disconnected credentials can regain authority: A readable tombstone falls through to legacy migration, allowing a surviving credential to restore authenticated network requests after the user disconnected.
Privacy-mode responses expose account identities: New account IDs include raw Codex cache keys containing emails and workspace identifiers, bypassing the selected dashboard identity mode.

How this fits together

CodexBar collects provider usage locally and exposes dashboard snapshots through its CLI server. This change lets another Mac fetch those snapshots with a saved bearer token and display remote accounts in its menus and widgets.

flowchart LR
  A[Provider accounts] --> B[Dashboard server]
  C[Sync settings] --> D[Endpoint and token in Keychain]
  D --> E[Authenticated snapshot client]
  B --> E
  E --> F[Remote account projection]
  F --> G[Menus and widgets]
  C --> H[Local or remote-only mode]
  H --> G
Loading

Decision needed

Question Recommendation
Should existing serve and dashboard invocations automatically export all configured accounts, or should multi-account export require explicit opt-in? Preserve selected-account defaults: Keep existing invocations scoped to the selected account and explicitly enable multi-account export.

Why: The patch expands the data visible to existing dashboard clients; implementation intent alone does not establish acceptance of that upgrade contract.

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: Authority-chain proof required: the captured arm64 dashboard output and real Keychain recovery report support successful collection and access, but do not show that Disconnect with a surviving legacy record prevents authenticated requests after restart. After repair, provide redacted transport logs or terminal evidence for that final-I/O boundary and privacy-mode output; screenshots or video can additionally demonstrate the native recovery menu. Remove tokens, private addresses, and endpoints. Updating the PR body should trigger 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.
  • Use opaque account IDs in redacted dashboard output (P1) - For multiple Codex accounts with workspace identities, usageCacheAccountKey embeds the full email and workspace ID. Copying that key into accounts[].id leaks both through --identity redacted and --identity none, even though the identity object is filtered. Export a stable opaque identifier and verify the complete serialized response in each privacy mode.
  • Distinguish a credential tombstone from an absent record (P1) - Disconnect writes an empty-token tombstone, but this guard returns the same nil used for a missing record. Both load paths then call migrateLegacyCredential(). If best-effort legacy deletion failed and that record later becomes readable, restarting restores the old endpoint and bearer token and resumes authenticated requests after Disconnect. Preserve an explicit tombstone result and allow migration only for a genuinely absent record.
  • Preserve the durable credential until replacement succeeds (P2) - reown deletes the only durable record before adding its replacement. An add failure or interruption therefore loses the credential; interactive recovery additionally suppresses the error and returns a token that works only until exit. The fake store used by the failed-write test cannot expose this delete/add sequence. Use a replacement strategy that preserves recoverable durable state and cover failure between those operations.
  • Keep a recovery menu visible when remote providers disappear (P1) - Remote provider inventory starts empty and is cleared on unauthorized or invalid responses. In remote-only mode this return removes the fallback while enabledProvidersForDisplay() also returns an empty list; updateVisibility() then hides the main status item and removes the provider items. Keep a visible recovery menu so users can open Sync settings, retry, or disconnect when the server supplies no usable providers.
  • Honor remote-only intent while saved credentials are unavailable (P2) - When a saved token is temporarily unavailable or requires authorization, initialization leaves the token empty and this predicate becomes false despite the persisted remote-only preference. Background provider selection then follows the local path and can run local probes the user explicitly disabled. Base suppression on the persisted mode and show an awaiting-credentials state until recovery or explicit disconnect.
  • Realign the architecture anchors shifted by this patch (P2) - The observation additions move the synthetic and warp anchors to lines 105 and 124, but these entries record 104 and 123. They matched the merge base at 103 and 122, so this mismatch is introduced here. The gatekeeper requires exact line text and rejects both entries; shifted SettingsStore and UsageStore entries also need reconciliation. Update the affected anchors and run the focused gatekeeper suite.
  • Resolve security concern: Disconnected credentials can regain authority - A readable tombstone falls through to legacy migration, allowing a surviving credential to restore authenticated network requests after the user disconnected.
  • Resolve security concern: Privacy-mode responses expose account identities - New account IDs include raw Codex cache keys containing emails and workspace identifiers, bypassing the selected dashboard identity mode.
  • Resolve merge risk (P1) - Existing serve/dashboard invocations will expose every configured account instead of only the selected account; this expanded default needs maintainer acceptance or an opt-in compatibility path with fresh-install and upgrade evidence.
  • Resolve merge risk (P1) - Disconnect can resurrect a surviving legacy credential on restart, and privacy-mode dashboard output can disclose account identifiers.
  • Resolve merge risk (P1) - Keychain recovery can erase the durable credential before replacement succeeds, leaving a working session that loses its connection on restart.
  • Complete next step (P2) - Resolve the six findings, obtain the account-export policy decision, add the required real behavior evidence, and reconcile the reported branch conflicts before requesting merge.
  • Improve patch quality - Repair the six source-confirmed findings with focused regressions and passing make test and make check results.
  • Improve patch quality - Add real final-I/O evidence that disconnected credentials stay inactive after restart, plus privacy-mode output and visible recovery-menu proof.
  • Improve patch quality - Resolve the account-export default and demonstrate fresh-install and upgrade behavior.
  • Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

  • [P1] Use opaque account IDs in redacted dashboard output — Sources/CodexBarCLI/DashboardSnapshotBuilder.swift:207-208
  • [P1] Distinguish a credential tombstone from an absent record — Sources/CodexBar/RemoteCodexBarTokenStore.swift:105-106
  • [P2] Preserve the durable credential until replacement succeeds — Sources/CodexBar/RemoteCodexBarTokenStore.swift:115-118
  • [high] Disconnected credentials can regain authority — Sources/CodexBar/RemoteCodexBarTokenStore.swift:106
  • [medium] Privacy-mode responses expose account identities — Sources/CodexBarCLI/DashboardSnapshotBuilder.swift:208
Agent review details

Security

Needs attention: The security pass found credential revocation and dashboard privacy defects; no unrelated dependency or supply-chain execution changes were introduced.

Review metrics

Metric Value Why it matters
Production and test growth Production +1,780/-78 (net +1,702); tests +1,363/-21 (net +1,342) The stated remote-client, credential, and multi-account features explain the production growth, but require review across storage, transport, and UI boundaries.

Merge-risk options

Maintainer options:

  1. Repair credential and privacy invariants (recommended)
    Distinguish tombstones from missing records, preserve credentials through failed replacement, and export opaque account IDs before collecting restart and final-I/O proof.
  2. Preserve existing export scope
    Keep selected-account behavior by default and introduce multi-account export only through an explicitly approved opt-in.

Technical review

Best possible solution:

Provide durable, revocation-safe remote sync with a visible recovery menu, opaque exported account IDs, and an explicit account-export policy that preserves existing deployments unless maintainers approve expansion.

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

Yes for the PR defects: source establishes the tombstone migration, raw account-ID export, destructive replacement, empty-menu, remote-only fallback, and anchor-mismatch paths. These are source findings, not executed reproductions or current-main failures.

Is this the best way to solve the issue?

Partly: reusing the dashboard contract and shared account layout is appropriate, but the credential state machine and export privacy need repair, and the broader default account scope needs an explicit decision.

Full review comments:

  • [P1] Use opaque account IDs in redacted dashboard output — Sources/CodexBarCLI/DashboardSnapshotBuilder.swift:207-208
    For multiple Codex accounts with workspace identities, usageCacheAccountKey embeds the full email and workspace ID. Copying that key into accounts[].id leaks both through --identity redacted and --identity none, even though the identity object is filtered. Export a stable opaque identifier and verify the complete serialized response in each privacy mode.
    Confidence: 0.99
  • [P1] Distinguish a credential tombstone from an absent record — Sources/CodexBar/RemoteCodexBarTokenStore.swift:105-106
    Disconnect writes an empty-token tombstone, but this guard returns the same nil used for a missing record. Both load paths then call migrateLegacyCredential(). If best-effort legacy deletion failed and that record later becomes readable, restarting restores the old endpoint and bearer token and resumes authenticated requests after Disconnect. Preserve an explicit tombstone result and allow migration only for a genuinely absent record.
    Confidence: 0.99
  • [P2] Preserve the durable credential until replacement succeeds — Sources/CodexBar/RemoteCodexBarTokenStore.swift:115-118
    reown deletes the only durable record before adding its replacement. An add failure or interruption therefore loses the credential; interactive recovery additionally suppresses the error and returns a token that works only until exit. The fake store used by the failed-write test cannot expose this delete/add sequence. Use a replacement strategy that preserves recoverable durable state and cover failure between those operations.
    Confidence: 0.98
  • [P1] Keep a recovery menu visible when remote providers disappear — Sources/CodexBar/StatusItemController+MenuTypes.swift:11-14
    Remote provider inventory starts empty and is cleared on unauthorized or invalid responses. In remote-only mode this return removes the fallback while enabledProvidersForDisplay() also returns an empty list; updateVisibility() then hides the main status item and removes the provider items. Keep a visible recovery menu so users can open Sync settings, retry, or disconnect when the server supplies no usable providers.
    Confidence: 0.99
  • [P2] Honor remote-only intent while saved credentials are unavailable — Sources/CodexBar/SettingsStore+RemoteCodexBar.swift:16-18
    When a saved token is temporarily unavailable or requires authorization, initialization leaves the token empty and this predicate becomes false despite the persisted remote-only preference. Background provider selection then follows the local path and can run local probes the user explicitly disabled. Base suppression on the persisted mode and show an awaiting-credentials state until recovery or explicit disconnect.
    Confidence: 0.98
  • [P2] Realign the architecture anchors shifted by this patch — Tests/CodexBarTests/ProviderArchitectureGatekeeperTests.swift:948-951
    The observation additions move the synthetic and warp anchors to lines 105 and 124, but these entries record 104 and 123. They matched the merge base at 103 and 122, so this mismatch is introduced here. The gatekeeper requires exact line text and rejects both entries; shifted SettingsStore and UsageStore entries also need reconciliation. Update the affected anchors and run the focused gatekeeper suite.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

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

Labels

Label justifications:

  • P2: This is a useful optional remote-usage feature with bounded scope, rather than an established urgent production outage.
  • merge-risk: 🚨 compatibility: Existing server invocations gain all-account export, and credential migration and restart behavior are not upgrade-safe yet.
  • merge-risk: 🚨 auth-provider: Credential replacement can lose saved authentication, while unavailable credentials unexpectedly re-enable local provider probes.
  • merge-risk: 🚨 security-boundary: A disconnected credential can be restored from legacy storage and redacted responses can reveal full account identities.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦐 gold shrimp and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Authority-chain proof required: the captured arm64 dashboard output and real Keychain recovery report support successful collection and access, but do not show that Disconnect with a surviving legacy record prevents authenticated requests after restart. After repair, provide redacted transport logs or terminal evidence for that final-I/O boundary and privacy-mode output; screenshots or video can additionally demonstrate the native recovery menu. Remove tokens, private addresses, and endpoints. Updating the PR body should trigger 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

Security concerns:

  • [high] Disconnected credentials can regain authority — Sources/CodexBar/RemoteCodexBarTokenStore.swift:106
    A readable tombstone falls through to legacy migration, allowing a surviving credential to restore authenticated network requests after the user disconnected.
    Confidence: 0.99
  • [medium] Privacy-mode responses expose account identities — Sources/CodexBarCLI/DashboardSnapshotBuilder.swift:208
    New account IDs include raw Codex cache keys containing emails and workspace identifiers, bypassing the selected dashboard identity mode.
    Confidence: 0.99

What I checked:

  • Policy and review scope: Read the full root AGENTS.md. Tracked policy discovery found no nested AGENTS.md or maintainer notes. Applied credential isolation, provider siloing, focused-test, and menu-recovery guidance; no builds, tests, live account probes, or Keychain operations were executed. (AGENTS.md:1, 441a40f6fcdb)
  • Pinned ownership and re-review continuity: Reviewed the introduced merge-base-to-head changes, not endpoint drift. HEAD remains the exact revision reviewed previously; comparison against that revision is empty. All six retained prior findings were checked independently against source. No verified test merge is available, so no main-branch deletion claims are made. (441a40f6fcdb)
  • Still necessary on current main and latest release: Current main still builds provider rows without generic multi-account grouping, and its dashboard and serve collectors explicitly select one Codex account. The remote client is absent. The peeled v0.63.0 tag equals fetched main, so the requested implementation is also absent from that release. (Sources/CodexBarCLI/DashboardSnapshotBuilder.swift:37, f3e718c897d5)
  • Release identity: The annotated v0.63.0 tag resolves to the supplied current-main commit. (f3e718c897d5)
  • Dashboard privacy bypass: The new accounts[].id copies cacheAccountKey without applying identity mode. CLIUsageCommand.swift constructs Codex workspace keys containing the full email and workspace identifier, so redacted output still exposes them. (Sources/CodexBarCLI/DashboardSnapshotBuilder.swift:208, 441a40f6fcdb)
  • Disconnect and durable credential defects: An empty-token tombstone becomes nil, which both load paths interpret as permission to migrate a legacy credential. Legacy cleanup is explicitly best effort and can fail. Separately, reown deletes the durable record before adding its replacement, and interactive recovery suppresses replacement failure. (Sources/CodexBar/RemoteCodexBarTokenStore.swift:106, 441a40f6fcdb)

Likely related people:

  • Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • Artus Krohn-Grimberghe: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

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-20T21:19:06.091Z sha 441a40f :: needs real behavior proof before merge. :: [P1] Use opaque account IDs in redacted dashboard output | [P1] Distinguish a credential tombstone from an absent record | [P2] Preserve the durable credential until replacement succeeds | [P1] Keep a recovery menu visible when remote providers disappear | [P2] Honor remote-only intent while saved credentials are unavailable | [P2] Realign the architecture anchors shifted by this patch

@steipete

Copy link
Copy Markdown
Owner

Thanks @VACInc. This adds the client side of serve and remote account presentation; it does not aggregate cross-device cost history. Keeping it open for the remote-client and credential-handling decision. There is also a source-level privacy concern: DashboardSnapshotBuilder.makeUsageAccounts publishes cacheAccountKey as accounts[].id, while usageCacheAccountKey can contain the full Codex workspace ID and email. Those IDs bypass identity redaction. Account enumeration overlaps #2827; maintainer adoption should use one implementation, opaque IDs, and dashboard-only expansion while preserving generic /usage behavior. Automatic interaction-allowed Keychain recovery needs an explicit decision too. No runtime validation was performed in this survey.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. 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.

2 participants