Add remote codexbar serve snapshots - #3260
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
There was a problem hiding this comment.
💡 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".
|
Codex review: needs real behavior proof before merge. Reviewed August 29, 2026, 1:50 AM ET / 05:50 UTC. ClawSweeper reviewWhat this changesThe branch adds a Sync-pane connection to an authenticated remote 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 Review scores
Verification
How this fits togetherThe 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]
Decision needed
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
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest 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. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (14 earlier review cycles; latest 8 shown)
|
|
Current head: 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 |
|
🦞🧹 I asked ClawSweeper to review this item again. |
Summary
codexbar servebase URL and dashboard bearer token/dashboard/v1/snapshotwith bounded schema-v1 decodingThis is the display-client counterpart to the authenticated snapshot endpoint added in #2227 and the remote-monitoring request in #319.
Verification
Notes