feat(deepseek): Safari web-auth import, balance consumption & per-model cost - #2938
feat(deepseek): Safari web-auth import, balance consumption & per-model cost#2938jky1314 wants to merge 2 commits into
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
merge-risk: 🚨 security-boundary
|
Codex review: needs real behavior proof before merge. Reviewed August 16, 2026, 2:36 AM ET / 06:36 UTC. ClawSweeper reviewWhat this changesThe PR adds opt-in Safari DeepSeek session import, local balance-spend estimates, per-model web cost details, and related menu localization and documentation. Merge readiness⛔ Blocked until real behavior proof from a real setup is added - 8 items remain Keep open: the Safari reader is gated correctly, but a Safari-only user cannot reach that gate because no Safari profile is discoverable before one is already selected. The branch also retains unrelated Traditional Chinese translation removals and needs real after-fix runtime proof. Likely related people: Peter Steinberger (medium confidence). Priority: P2 Review scores
Verification
How this fits togetherCodexBar resolves DeepSeek API keys and browser sessions into usage snapshots, then renders balance and detailed usage in its menu-bar card. This PR extends the browser-session input and adds locally derived and per-model usage outputs. flowchart LR
A[API key or web session] --> B[DeepSeek data fetch]
C[Chrome or Safari storage] --> B
B --> D[Usage normalization]
D --> E[Balance history]
D --> F[Model cost details]
E --> G[Menu-bar usage card]
F --> G
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Add a visible user-initiated Safari discovery choice that preserves Chrome-only background refreshes, restore unrelated zh-Hant entries, isolate the Safari reader behind a test seam, and provide redacted fresh-bundle proof. Do we have a high-confidence way to reproduce the issue? Yes, from source: start with only a Safari DeepSeek session and no saved Is this the best way to solve the issue? No. The privacy gate is appropriate, but it must be preceded by a visible user-initiated Safari discovery path; otherwise the advertised capability is unreachable. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against b9ccac302b28. 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 (6 earlier review cycles)
|
![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: ce266b70fe
ℹ️ 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".
| Self.balanceHistoryStore.record( | ||
| balance: balance.totalBalance, | ||
| currency: balance.currency, | ||
| accountKey: accountKey, | ||
| at: now) |
There was a problem hiding this comment.
After a recharge, every production refresh records the current balance before calculating consumption. For example, after recording a recharge at 200, a later refresh at 195 overwrites that day's lastBalance with 195; consumptionSummary then uses 195 as both the recharge baseline and current balance, so both Today and Total report zero instead of 5. The tests bypass this ordering by calling consumptionSummary without first recording their final balance.
Useful? React with 👍 / 👎.
| let log: @Sendable (String) -> Void = { message in logger?("[deepseek-storage] \(message)") } | ||
|
|
||
| // Safari stores localStorage in WebKit sqlite files; Chromium browsers use leveldb. | ||
| let safariTokens = Self.importSafariTokens(logger: log) |
There was a problem hiding this comment.
Automatic DeepSeek session resolution now reads the protected Safari container before checking Chrome even when the user has not selected Safari. On macOS, an ordinary background refresh can therefore initiate Safari-data/Full Disk Access permission handling; keep Chrome as the default and invoke the Safari reader only through an explicit browser selection or opt-in.
AGENTS.md reference: AGENTS.md:L48-L48
Useful? React with 👍 / 👎.
| guard let apiKey else { return "default" } | ||
| return String(apiKey.prefix(8)) + ":" + String(apiKey.suffix(4)) |
There was a problem hiding this comment.
In web-only mode there is no API key, so every Safari or Chrome DeepSeek profile is assigned the same default history key. If a user switches between two platform profiles, the second account's balance is compared with the first account's snapshots and displayed as a recharge or spend; derive the key from the selected platform profile or token when the API key is absent.
Useful? React with 👍 / 👎.
| /// 90-day window. | ||
| public func record( | ||
| balance: Double, | ||
| currency: String, |
There was a problem hiding this comment.
The currency argument is accepted but never persisted or included in the history key. When the existing balance parser changes its selected wallet—for example, funded USD reaches zero and it falls back to a funded CNY balance—the store subtracts the prior USD amount from the current CNY amount and formats that result with the current symbol. Reset or partition records when the selected currency changes.
Useful? React with 👍 / 👎.
| return try? ProviderDetailSection( | ||
| title: title, | ||
| rows: rows, | ||
| chart: section.chart) |
There was a problem hiding this comment.
The helper adds localized mappings for Daily tokens and tokens, but it passes section.chart through unchanged. Consequently, the newly localized DeepSeek details still render the chart heading and unit in English in zh-Hans; rebuild the chart with mapped title and unit values.
Useful? React with 👍 / 👎.
|
@clawsweeper re-review — addressed P1 (API key now hashed via namespaced SHA-256 digest, with persistence regression tests) and P2 (Safari + Full Disk Access documented in docs/deepseek.md and menu copy). Real behavior proof added to the PR body (digest verification output + fresh test run). |
|
🦞🧹 I asked ClawSweeper to review this item again. |
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
…el cost Squashed from 3 commits: original feature, hashed account keys, credential-safe test fixtures (GitGuardian-clean).
8273bba to
5458091
Compare
… opt-in, profile-scoped history, currency partitioning, chart localization - Calculate consumption BEFORE recording the refresh so recharge baselines are never overwritten by the following refresh (both spend values stay correct) - Gate Safari storage reads behind explicit opt-in: routine refreshes stay Chrome-only; Safari container is read only when a safari: profile is selected - Scope web-only balance history to the selected profile: nil API key no longer collapses every browser account into the shared "default" history key - Partition persisted balance history by currency so USD/CNY wallet switches never subtract unrelated balances or mix symbols - Localize DeepSeek chart title/unit/point labels through the same label map as the detail rows (zh-Hans no longer shows English chart headers) - Sync the 6 new catalog keys across all 21 language directories (en/zh-Hans were updated while the rest drifted, failing the catalog consistency tests) - Regression tests: refresh-after-recharge spend, Safari opt-in flag, profile key isolation, USD/CNY partitioning, chart localization
|
🦞👀 Re-review progress:
|
|
@clawsweeper re-review — all 5 findings addressed: P1-1 consumption order: consumption is now derived BEFORE recording the fresh sample, so a recharge baseline is never overwritten by the following refresh. Regression: P1-2 Safari opt-in: P1-3 web-only history scope: P2-4 currency partitioning: P2-5 chart localization: Also synced the 6 new catalog keys across all 21 language directories (en/zh-Hans had drifted, failing Local verification: 152 related tests pass (DeepSeek + localization + menu-card suites). PR body updated with per-finding behavior proof. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
1 participant
Summary
Three DeepSeek improvements in one PR:
1. Safari web-auth token import
DeepSeekPlatformTokenImporteronly scanned Chromium leveldb (browsers: [.chrome]). This ports Factory's WebKit SQLite3 reader soplatform.deepseek.com'suserTokencan be imported from Safari local storage (macOS only). The Chromium path is kept; candidates from both sources are merged.~/Library/Containers/com.apple.Safari/.../WebsiteData/Default/<origin>/LocalStorage/localstorage.sqlite3(requires Full Disk Access)originfiles (newer Safari uses hashed dirs, not domain-named dirs)safari:profile; routine refreshes stay Chrome-only (privacy opt-in)2. API-key balance consumption tracking
DeepSeek's API has no usage endpoint, so API-key mode can only see the balance. New
DeepSeekBalanceHistoryStorepersists daily first/last balance snapshots (90-day rolling window, per-account isolated):DeepSeekUsageSnapshotrows appendToday −¥x · Total −¥xcom.steipete.codexbar.deepseek-balance-history.v1/.profile.v1), never raw key fragments — mirrors the existingDeepSeekSettingsReader.profileScopepattern3. Per-model cost breakdown (web mode)
The platform
usage/costendpoint returns per-model totals, butDeepSeekUsageSummarydiscarded them. AddedmodelCosts: [DeepSeekModelCost]aggregated inbuildBreakdownsand rendered in the Detailed usage section. Also added app-layer localization for DeepSeek detail rows and chart title/unit (zh-Hans) and Safari-aware setup copy (docs/deepseek.md, menu hints).Real behavior proof
Credential-boundary proof (digest contains no API-key fragments):
Consumption-order proof (fresh test run, recharge baseline survives the next refresh):
Profile-scope proof (web-only history no longer collapses to "default"):
Safari opt-in proof:
Chart localization proof (zh-Hans catalog now covers chart title/unit):
Full suite: 152 related tests pass (DeepSeek + localization + menu-card suites), release build verified locally.
Notes