Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
clawsweeper
Bot
added
P2
|
Codex review: blocked before merge. Reviewed September 17, 2026, 2:22 AM ET / 06:22 UTC (Revision 10). ClawSweeper reviewWhat this changesAdds default-off, manually refreshed SSH collection that combines verified local and remote Codex histories in the native cost card and chart while preserving local Priority pricing. Merge readiness⛔ Blocked before merge - 3 items remain This remains useful, distinct work for a demonstrated cross-device reporting gap. The earlier findings are addressed, and the supplied runtime evidence supports the revised implementation; product and privacy acceptance remain unresolved. Priority: P2 Review scores
Verification
How this fits togetherCodexBar derives token and estimated-cost history from native Codex session logs. This change brings one selected server’s logs through temporary private storage into the existing scanner, then presents a deduplicated snapshot in the native interface. flowchart LR
A[Manual refresh and consent] --> B[SSH collection]
C[Server session logs] --> B
B --> D[Private temporary storage]
E[Local logs and pricing evidence] --> F[Verified combined scan]
D --> F
F --> G[Cleanup before publication]
G --> H[Cost card and daily chart]
Decision needed
Why: VISION.md reserves new features and privacy/storage behavior for sign-off, and the earlier owner discussion did not approve this expanded native scope. Before merge
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep a single explicitly refreshed native-log scope with verified deduplication, frozen pricing, local fallback, and an owner-approved raw-log retention contract. Do we have a high-confidence way to reproduce the issue? Not applicable to an existing-behavior bug: this adds cross-device reporting. Supplied real SSH receipts demonstrate the new behavior with known synthetic inputs. Is this the best way to solve the issue? Unclear until scope approval: the joint scanner solves copied-session deduplication that separate numeric reports cannot, but requires accepting temporary raw-log transfer. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against b6e65a83dc47. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (9 earlier review cycles; latest 8 shown)
|
|
@clawsweeper re-review Added the requested captured runtime evidence in The tested implementation remains |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
proof: sufficient
…-native-manual # Conflicts: # CHANGELOG.md
A-l-an
changed the title
clawsweeper
Bot
added
the
merge-risk: 🚨 security-boundary
A-l-an
changed the title
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cac2b9298d
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| else exit 44 | ||
| fi | ||
| done | ||
| ' sh "$home" \#(limits.fileBytes) {} + || exit 46 |
There was a problem hiding this comment.
When a nested entry is unreadable or unsafe, or a JSONL file exceeds the per-file limit, the inner shell exits with 43, 44, or 45, but find ... || exit 46 converts every such failure to 46. CodexRemoteLogMirrorProcess.failure consequently reports these permanent conditions as unstableSource, telling users to retry instead of identifying inaccessible, unsafe, or oversized logs; propagate the validation failure rather than unconditionally replacing its status.
Useful? React with 👍 / 👎.
| calendar: calendar, | ||
| day: calendar.startOfDay(for: now), | ||
| pricingRevision: revision, | ||
| sshRevision: CodexRemoteLogMirror.configurationFingerprint(environment: self.environmentBase), |
There was a problem hiding this comment.
When the SSH-cost option is enabled, every call to codexRemoteCostPresentation or codexCostPresentationSnapshot rebuilds this context on UsageStore's main actor. A single menu construction invokes those selectors repeatedly, and each invocation synchronously rereads the pricing files and calls configurationFingerprint, which may open and hash up to 4,096 included SSH configs totaling 8 MiB; a large but accepted Include glob therefore causes repeated filesystem scans and visible menu/UI stalls. Cache or asynchronously compute the revision rather than performing the full fingerprint on each presentation lookup.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🦐 gold shrimp
clawsweeper
Bot
added
status: ⏳ waiting on author
clawsweeper
Bot
added
rating: 🐚 platinum hermit
1 participant
Summary
Adds Manual SSH cost statistics to Settings → Providers → Codex. Enter one SSH destination, click Refresh server statistics, and see this Mac's and the server's native Codex usage together in the existing Today/history cost card and daily chart.
~/.codex).rsync, GNUfind,stat, andsha256sum; a remote CodexBar CLI and Agent Sessions are not required.Try it: enable the switch, enter an alias such as
research-server, confirm the remote Codex home, and click Refresh server statistics. If a managed-account scope is selected, first choose Use this Mac’s native history. The shared native-log scope receives the server contribution once; account quota bars and managed-account rows retain their existing scopes.Why this matters
When a MacBook is used to SSH into a Linux development host, Codex work performed on that host is recorded there. CodexBar running on the Mac only sees the Mac’s local history, so the server’s tokens and estimated cost are missing from that view. Account-wide quota bars do not fill this gap: they answer a different question from historical tokens and API-equivalent cost.
Issue #3036 describes this Mac + Linux workflow. Subsequent users independently report the same gap: a MacBook connected to an SSH desktop, specifically requesting menu visibility and copied-session deduplication, and macOS + a Linux VM. This PR makes that concrete workflow reviewable in the existing cost card and daily chart.
Live UI proof
Freshly built App, using the production settings, cost-card and chart views in an isolated native window, connected over real SSH to a Debian 12 container hosted on a remote Intel Mac. These are synthetic sessions with known expected results. Tested implementation:
8f1df3474591fd4f4838a6b618b80c00e3cace1d.Configured SSH source, combined Today/history values, and a daily chart containing the remote-only date:
The fixture includes a copied session prefix plus additional remote usage:
The chart also gains September 13: 55,000 tokens / $0.183, present only on the server. The GUI rounds dollar values to cents. Deduplication produces $2.379 over seven days; simply adding the two machines would incorrectly show $3.477.
First-refresh disclosure, privacy masking, and failure/cancellation views
The first refresh explains that raw logs may contain conversations and project paths:
Hide Personal Info masks both inputs and the displayed source without changing the totals:
An unreachable endpoint retains the original local card and chart with a readable error:
Cancelling an active transfer returns to local statistics after temporary data is removed:
The normal installed menu-bar instance and live-account login/quota flows were not used for this acceptance run; the proof window therefore shows “Not fetched yet” for the unprobed quota header. Full receipts and isolation limits are linked under Verification.
Accounting and refresh behavior
sessionsandarchived_sessionsJSONL, then scans a canonical local/remote union once. Exact copies count once; verified complete ordered prefixes retain their longer suffix; different session identities remain independent.Privacy and boundaries
The first refresh explicitly discloses that temporary raw JSONL can contain conversations and project paths. Private request directories and files are removed before publication; failed cleanup is visible and retryable. A foreground guardian retains the activity lock until transfer writers have stopped, including if the App exits unexpectedly. Startup cleanup only considers valid owned inactive requests.
SSH is noninteractive with strict host-key checks. The Linux server needs GNU
find,stat,sha256sumandrsync; it does not need a CodexBar CLI. Budgets and supported log/configuration shapes are documented in the native SSH guide. The 512 MiB received-data threshold is monitored, not a hard filesystem quota; temporary scan SQLite storage is additional. Estimates do not claim invoice parity or every fork/export shape.Priority/Fast fix and current verification
The joint scan previously rejected any retained local Priority tokens or relevant local trace evidence, making ordinary Fast histories fall back to Only this Mac. It now preserves row-owned pricing through canonicalization, scopes trace evidence to the local session and turn, and rebuilds the native Standard/Priority breakdown using the frozen pricing context. Conflicting trace owners, unmatched retained rows and unsupported inline tier evidence still fail closed.
Validation for
81da9ad7f5fb4be2e1d647a208d00b8fd7666b02:make check: passed with zero SwiftLint violations.make test: 108/108 groups passed; test summaries report 11,247 tests, with 0 failures, retries or timeouts.action_required, awaiting maintainer approval; this is not a passing CI result.Priority/Fast fix evidence
Earlier validation remains available: review fixes at
76a3ee2dcover the main-actor fingerprint cache, nested manifest error propagation, upstream conflict resolution, full Mac/Linux tests, and protected native UI checks. Native acceptance at8f1df347documents disclosure, manual refresh, chart/card presentation, privacy masking, recovery and cancellation. Receiving-permissions follow-up covers macOS system openrsync.Decision needed
This PR still requires maintainer product sign-off. The earlier discussion favored a narrow one-shot CLI direction; it did not approve this expanded native UI or temporary raw-log privacy contract. Maintainer approval of those boundaries is still required under
VISION.md. The linked issue demonstrates concrete user demand, not an approval or a claim that this closes the whole request.Related to #1566, #3036, #3538 and #3545. This implementation builds on CodexBar's existing native scanner, pricing context and process-group helpers; existing upstream/vendored attribution is retained.