Skip to content

Add remote codexbar serve snapshots - #3260

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

Add remote codexbar serve snapshots#3260
VACInc wants to merge 17 commits into
steipete:mainfrom
VACInc:feature/remote-codexbar-serve

Conversation

@VACInc

@VACInc VACInc commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a provider-neutral Sync settings pane that keeps the existing iCloud controls and adds a Remote CodexBar connection for a codexbar serve base URL and dashboard bearer token
  • store the token in Keychain, validate HTTPS/private-network/Tailscale HTTP endpoints, require explicit consent before sending it over non-loopback HTTP, and fetch /dashboard/v1/snapshot with bounded schema-v1 decoding
  • project remote provider and account snapshots into the existing fleet-account card UI while preserving local provider behavior
  • refresh remote data alongside normal app refreshes, retain last-good cards only for transient same-connection failures, and clear them after permanent auth/schema failures or connection changes
  • persist the endpoint, token, and cleartext consent as one Keychain credential record, so failed or interrupted replacements cannot recombine authority from different hosts
  • clear the staged token and private-HTTP consent as soon as an existing endpoint draft changes, requiring explicit token re-entry for the new host
  • stop reading oversized snapshot responses at 2 MiB and do not retry an oversized retryable response
  • prefer unique unredacted account identity for local/remote deduplication, then use server-scoped stable IDs for redacted or ambiguous identities so unrelated local accounts do not collide
  • replace ambient provider cards with account cards for multi-account snapshots, and propagate refresh cancellation to the underlying URLSession request
  • hide cached cards synchronously when their bound configuration no longer matches the active endpoint/token pair
  • render the server-provided multi-account label as the authoritative card title while retaining plan and stable identity metadata

This is the display-client counterpart to the authenticated snapshot endpoint added in #2227 and the remote-monitoring request in #319.

Verification

  • focused RemoteCodexBar settings, transport, projection, and menu tests, including failed and interrupted credential replacement, Keychain re-enablement, private-HTTP consent, transient/permanent failure state, local/remote account deduplication, redacted-account identity, streaming response limits, and request cancellation
  • full hosted macOS test shards, Linux CLI builds/tests, static-musl build, SwiftFormat/SwiftLint, and GitGuardian on the exact PR head
  • independent final diff review

Notes

  • public endpoints require HTTPS; loopback HTTP works directly, while private-network and Tailscale/CGNAT HTTP require an explicit cleartext-bearer acknowledgment before the connection can be saved
  • the app never writes the bearer token to UserDefaults
  • the response-limit transport rejects cross-origin redirects so bearer authorization cannot follow a redirect to another host

@clawsweeper

clawsweeper Bot commented Aug 29, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2a6111727e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread Sources/CodexBar/SettingsStore+RemoteCodexBar.swift
Comment thread Sources/CodexBar/RemoteCodexBarSnapshot.swift
@clawsweeper clawsweeper Bot added merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 29, 2026
@clawsweeper

clawsweeper Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed August 29, 2026, 1:50 AM ET / 05:50 UTC.

ClawSweeper review

What this changes

The branch adds a Sync-pane connection to an authenticated remote codexbar serve snapshot endpoint and displays its provider and account data in existing menu cards.

Merge readiness

Blocked until real behavior proof from a real setup is added - 4 items remain

Keep open: the branch adds a distinct remote client feature not present on current main, but an external PR needs real behavior proof and maintainer product sponsorship before merge.

Priority: P2
Reviewed head: 4660db18e52c6316f00e6d7ee2b2b13d5b7d469f
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The implementation is substantial and source-reviewed, but external merge readiness is capped by missing real behavior and authority-boundary proof.
Proof confidence 🧂 unranked krab (1/6) Needs real behavior proof before merge: Authority-chain proof required: SettingsStore creates the endpoint-bound credential, RemoteCodexBarSnapshotClient sends it to the resolved snapshot URL, and UsageStore projects the result into menu cards; available evidence is limited to injected transports, URLProtocol, and in-memory stores, while the contributor confirms no live app install or real authenticated connection was performed. 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) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: Authority-chain proof required: SettingsStore creates the endpoint-bound credential, RemoteCodexBarSnapshotClient sends it to the resolved snapshot URL, and UsageStore projects the result into menu cards; available evidence is limited to injected transports, URLProtocol, and in-memory stores, while the contributor confirms no live app install or real authenticated connection was performed. 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 5 items Credential authority flow: The PR atomically stores the configured endpoint, token, and cleartext-consent value before publishing the new configuration revision; the request client then attaches that token only to the resolved snapshot URL.
Network boundary guards: The introduced transport uses an ephemeral session without cookie, credential, or URL-cache storage, caps streaming responses, and rejects redirects whose scheme, host, or effective port differs from the original request.
Established server counterpart: Current source authorizes the dashboard snapshot route before cache access. The endpoint originates in merged PR 2227, which is contained in v0.44.0 and v0.45.0 tags; this PR adds the separate macOS display client.
Findings None None.
Security None None.

How this fits together

The Sync pane saves an endpoint-bound bearer credential, UsageStore fetches the remote snapshot, and the menu controller combines those cards with local and iCloud account data. The endpoint and bearer token are the authority boundary for the outgoing request.

flowchart LR
  A[Sync settings] --> B[Endpoint-bound credential]
  B --> C[Snapshot client]
  C --> D[Authenticated remote server]
  D --> E[Usage projection]
  E --> F[Menu account cards]
Loading

Decision needed

Question Recommendation
Should CodexBar support a configured remote bearer-authenticated snapshot client after real connection and authority-boundary proof is supplied? Sponsor after proof: Keep the PR open for redacted built-client proof, then accept the remote-monitoring direction if the authority flow is confirmed.

Why: The merged server endpoint establishes a compatible backend contract, but this PR adds a substantial new native settings, credential, and menu-display product surface.

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: Authority-chain proof required: SettingsStore creates the endpoint-bound credential, RemoteCodexBarSnapshotClient sends it to the resolved snapshot URL, and UsageStore projects the result into menu cards; available evidence is limited to injected transports, URLProtocol, and in-memory stores, while the contributor confirms no live app install or real authenticated connection was performed. 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.
  • Resolve merge risk (P1) - Authority-chain proof is unresolved: supplied tests cover endpoint replacement and request behavior with fakes, but do not show a built client reaching a real authorized endpoint or demonstrate that a replaced endpoint cannot receive the former endpoint’s token.
  • Complete next step (P2) - The contributor must add real behavior proof, then a maintainer must explicitly sponsor or decline the new remote-monitoring surface.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production versus test delta production +1,092/-17; tests +775/-18 The feature spans credential storage, network transport, refresh state, settings, menu projection, and focused regression coverage.

Merge-risk options

Maintainer options:

  1. Require real authority-flow proof (recommended)
    Before merge, provide redacted evidence from a built app connected to a real token-gated server, including the resulting account card and replacement-endpoint protection.
  2. Pause the feature
    Close or pause the PR if maintainers do not want remote bearer-token monitoring as a supported application feature.

Technical review

Best possible solution:

Keep the endpoint-bound credential design, obtain redacted real client/server evidence for both the allowed connection and replacement-endpoint rejection, then land only with maintainer sponsorship for the remote-monitoring surface.

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

Not applicable: this PR proposes a new remote-monitoring feature rather than reporting broken existing behavior.

Is this the best way to solve the issue?

Unclear: the design reuses the established authenticated snapshot contract, but source-only and mock coverage cannot establish the real client-to-server authority flow.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 41c53c34dbeb.

Labels

Label changes:

  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🧂 unranked krab, so this older rating label is no longer current.

Label justifications:

  • P2: This is a substantial optional remote-monitoring feature with bounded user impact rather than an urgent regression.
  • merge-risk: 🚨 security-boundary: The branch persists a bearer token and sends it to a configured remote endpoint.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab 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: Authority-chain proof required: SettingsStore creates the endpoint-bound credential, RemoteCodexBarSnapshotClient sends it to the resolved snapshot URL, and UsageStore projects the result into menu cards; available evidence is limited to injected transports, URLProtocol, and in-memory stores, while the contributor confirms no live app install or real authenticated connection was performed. 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:

Likely related people:

  • Peter Steinberger: Merged the server-side dashboard snapshot feature that this client consumes. (role: merger of the authenticated snapshot endpoint; confidence: high; commits: 547b2789e4c5; files: Sources/CodexBarCLI/CLIServeCommand.swift, Sources/CodexBarCLI/DashboardSnapshotBuilder.swift)
  • jethac: Authored the merged server endpoint and schema contract used by this new client. (role: introduced the authenticated snapshot endpoint; confidence: high; commits: 547b2789e4c5; files: Sources/CodexBarCLI/CLIServeCommand.swift, Sources/CodexBarCLI/DashboardSnapshotBuilder.swift)
  • Yuxin Qiao: Current-main blame attributes the fleet-account projection seam that the PR extends to this recent area change. (role: recent fleet-card contributor; confidence: high; commits: f61744bad39f; files: Sources/CodexBar/StatusItemController+FleetAccounts.swift)

Rank-up moves

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

  • Post redacted built-app evidence of a real authenticated snapshot connection and rendered account card, including final-effect evidence that endpoint replacement cannot send the former token to the new host.
  • Obtain maintainer sponsorship for the new remote-monitoring client surface.

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 (14 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-29T04:07:53.295Z sha 76fb484 :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-29T04:18:12.463Z sha 6145082 :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-29T04:27:37.670Z sha e88f18e :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-29T04:39:41.550Z sha 25bc52b :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-29T04:48:15.035Z sha 558b66d :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-29T04:57:20.029Z sha 23f8a8c :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-29T05:09:09.021Z sha 685b784 :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-29T05:19:36.887Z sha 4660db1 :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot removed the merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. label Aug 29, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Aug 29, 2026
@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. P1 Urgent regression or broken agent/channel workflow affecting real users now. labels Aug 29, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Aug 29, 2026
@VACInc

VACInc commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

Current head: 4660db18e52c6316f00e6d7ee2b2b13d5b7d469f

Addressed all actionable code-review findings, including atomic endpoint-bound credentials, streaming response limits, explicit private/Tailscale HTTP consent, account projection/deduplication and labels, permanent-failure cleanup, Keychain recovery, and endpoint/cancellation transition races.

Validation: exact-head CI run https://github.com/steipete/CodexBar/actions/runs/33235749067 is green, including both macOS Swift test shards, Linux CLI builds, musl, lint, and GitGuardian. Independent final diff review found no actionable correctness defects; the only subsequent changes were the CI-required SwiftFormat and test-import corrections.

No live app install was performed.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 29, 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 rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Aug 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. 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