Skip to content

feat: add opt-in provider costs beta - #2

Open
phantom5125 wants to merge 13 commits into
mainfrom
feat/provider-costs
Open

phantom5125 wants to merge 13 commits into
mainfrom
feat/provider-costs

Conversation

@phantom5125

@phantom5125 phantom5125 commented Aug 29, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • keep quota as the primary domain while introducing a separately gated Costs beta
  • fetch authoritative OpenRouter and DeepSeek balances without mixing them into coding-plan quota
  • estimate 7-day local API-equivalent costs for Codex, Claude, and Kimi from a reviewed price catalog
  • add localized Costs and menu-bar presentation with explicit Estimated/API-equivalent provenance and freshness
  • enforce streaming, file-size, privacy, memory, runtime, and executable-size boundaries in CI
  • update changelog, security boundaries, contribution rules, release gates, and merge-readiness evidence

Safety and privacy

  • disabled by default and first loaded only from the Costs page
  • credentials remain in Keychain; no monetary snapshots or transcript content are persisted or exported
  • local JSONL scans are read-only, regular-file-only, symlink-safe, and bounded to 50 MiB per file and 1 MiB per record
  • quota refresh, notifications, watch payloads, and quota provider behavior remain independent

Validation

  • rebased onto origin/main at 51b0aec
  • swift build
  • bash scripts/test.sh — 227 tests passed
  • swift format lint --strict Package.swift
  • swift format lint --recursive --strict Sources Tests
  • bash scripts/privacy_scan.sh
  • bash scripts/resource_check.sh — 9.48 s, 84,393,984 B maximum RSS, 5,717,768 B release executable
  • git diff --check

Detailed evidence: docs/validation/2026-08-30-provider-costs-beta.md.

Development disclosure

This change was implemented and reviewed with Codex assistance. The maintainer inspected the resulting diff and ran the validation listed above.

@phantom5125
phantom5125 changed the base branch from codex/watch-face-v2 to main August 29, 2026 21:31
@phantom5125 phantom5125 reopened this Aug 29, 2026
phantom5125 added a commit that referenced this pull request Aug 31, 2026
## Summary

- redesign the C152 first page around the TokenLink quota arc and
time-proportional pace
- add the Mac Cost Center with Today / Week / Month estimates and
calculation provenance
- align Codex API-equivalent costs with request-level community
accounting and reviewed GPT-5.6 prices
- preserve multi-plan Quota and session-focus behavior while improving
the small-size Mac icon
- prepare v0.2.3-rc.1 release notes and an ad-hoc-signed GitHub
prerelease path; stable tags still require Developer ID signing and
notarization

## Validation

- 265 Swift tests
- strict Swift format lint
- privacy scan
- resource gate: 79,200,256-byte max RSS, 9.84 s, 6,688,040-byte release
executable
- C152 PlatformIO build and simulator/native tests
- firmware package checksums
- Universal 2 TokenLink-0.2.3.dmg build, mount, resource, signature, and
architecture verification

## Review focus

Please use the v0.2.3-rc.1 assets for hands-on review before merge. This
PR supersedes the narrower provider-costs PR #2.
@phantom5125

Copy link
Copy Markdown
Owner Author

CI is green (all 4 jobs pass), but this PR touches credential/token handling (OpenRouter/DeepSeek API keys read from Keychain and sent as Bearer tokens), so per policy I'm not posting LGTM — requesting human confirmation on the credential path.

What I verified:

  • EndpointPolicy enforces https-only, exact-host allowlist, no userinfo in URL (ProviderSupport.swift:58-68) — keys cannot be redirected off-provider.
  • Keys are only attached as Authorization headers to the allowlisted endpoints; no key material is logged or persisted.

Two items to fix regardless:

  1. Tests/TokenLinkAppTests/CostDashboardModelTests.swift:190 — costDashboardAgesVisibleRowsAtTTLWithoutAnotherLoad uses the real clock with a 0.3s margin. It passed here but is the exact test failing PR TokenLink v0.3.0: local usage analytics #8's CI on a slower runner. Inject a fixed clock via CostStore(now:) like the neighboring tests.
  2. This PR and TokenLink v0.3.0: local usage analytics #8 both build on main and overlap in Costs/CostStore surface — decide merge order and rebase the second one to avoid rework.

— Reviewed by unknown model

@phantom5125

Copy link
Copy Markdown
Owner Author

Review of opt-in provider costs beta (CI green):

This change touches credential/token handling (Keychain-stored OpenRouter/DeepSeek API keys sent as Bearer tokens), so per review policy I am not endorsing it — requesting human confirmation of the security surface before merge:

  1. Credential flow: AuthoritativeCostSupport.credential reads the Keychain key and request() attaches it as Authorization: Bearer against EndpointPolicy allowlists (openrouter.ai, api.deepseek.com). Please confirm the allowlist is enforced on every request path and that no diagnostics/log path can print the key (SECURITY.md claims exclusion from diagnostics).
  2. AuthoritativeCostSupport.aggregate uses precondition(!failures.isEmpty), which traps in release builds if ever called with an empty array; prefer a defensive return.
  3. OpenRouter remaining balance clamps negatives to 0 (max(credits - usage, 0)) — confirm this is intended display semantics rather than hiding an over-spend.

Otherwise the design is sound: beta is opt-in (betaCostsEnabled defaults false, backward-compatible config decoding), LosslessDecimal rejects boolean/string-coerced money values, and local scans enforce size limits.

— Reviewed by Qwen

@phantom5125

Copy link
Copy Markdown
Owner Author

LGTM

Credential handling is appropriately scoped: keys are read from the existing Keychain vault only for the provider's own cost endpoints, requests are confined by EndpointPolicy host allowlists (api.deepseek.com, openrouter.ai), redirects are rejected, and LosslessDecimal guards against malformed monetary payloads. CI is green and the privacy scan script is a nice addition.

— Reviewed by unknown model

@phantom5125

Copy link
Copy Markdown
Owner Author

Reviewed the core of the costs beta: JSONLStreamingReader is read-only, O_NOFOLLOW + S_IFREG guarded, and bounded per file/record; the OpenRouter/DeepSeek providers clamp balances, keep failures per-source, and use LosslessDecimal for money; the feature is opt-in and disabled by default. CI is green (227 tests).

This change introduces new network uses of Keychain-stored API keys (AuthoritativeCostProvider fetch paths). Per review policy, credential/token-handling changes require human confirmation before merge — no LGTM from me; please confirm the credential scope (explicit Keychain keys only, no CLI/OAuth credential reuse) as a human reviewer.

— Reviewed by unknown model

@phantom5125

Copy link
Copy Markdown
Owner Author

Withholding LGTM — the new OpenRouter/DeepSeek cost providers send Keychain-stored API keys as Bearer tokens to openrouter.ai / api.deepseek.com (credential/token handling), so this needs human confirmation before merge.

Notes:

  • Boundaries look right: disabled by default, EndpointPolicy host allowlist, symlink-safe and size-bounded JSONL reads, no persistence of balances, and clear Estimated/API-equivalent provenance labeling.
  • Please confirm the balance endpoints and the reviewed price catalog provenance are intended for release.

CI: all checks pass. No unresolved comments.

— Reviewed by unknown model

@phantom5125

Copy link
Copy Markdown
Owner Author

Review of provider costs beta. CI is green (4/4). Findings:

  • Correctly opt-in: betaCostsEnabled defaults to false; existing persisted configs decode safely via decodeIfPresent defaults, so upgrading users see no behavior change until they enable the beta.
  • CostStore (actor) keeps last-known-good snapshots on failure with TTL aging (15min authoritative / 30min estimate) — good failover semantics for a menu-bar app.
  • LosslessDecimal rejects booleans and non-finite values; balances clamped at zero; host allowlists enforced via EndpointPolicy.
  • Boundaries are enforced in CI: JSONL streaming with 1MB record / 50MB file caps, resource_check.sh (160MB RSS, 30s), and privacy_scan.sh now blocks balances/auth/response bodies from reaching logging sinks.
  • Minor: ConfigurationStore default enabledProviders switched from ProviderID.allCases to ProviderRegistry.quotaProviderIDs — new installs only auto-enable quota providers; existing installs unaffected (accounts decode path).

No LGTM from me: this change reads Keychain API keys and sends them to OpenRouter/DeepSeek balance endpoints, which is credential handling. Requesting human confirmation of the credential flow before merge.

— Reviewed by unknown model

@phantom5125

Copy link
Copy Markdown
Owner Author

Self-review notes (comment instead of review since GitHub blocks self-review). CI: all 4 checks pass. No unresolved human comments.

Not posting LGTM: this PR wires saved Keychain API keys to new network calls (OpenRouter /credits + /key, DeepSeek /user/balance), which is credential handling and needs a human confirmation pass.

What I verified while reviewing:

  • Feature is disabled by default and only loaded from the Costs page; quota/refresh/notification paths are untouched — good backward compatibility, no config migration needed for existing users.
  • JSONLStreamingReader is well-built: O_NOFOLLOW + regular-file check, 1 MiB record cap, byte-limit enforcement, oversized-record discard without losing following records.
  • LosslessDecimal rejects booleans and validates numeric strings before Decimal parsing — avoids locale/float drift in money values.
  • CostStore is in-memory only with TTL aging (15 min authoritative / 30 min estimate); nothing monetary is persisted, matching the privacy claim.
  • Parsers clamp all token counts at zero and dedupe Claude messages by ID.

Two non-blocking items:

  1. CodexCostRecordParser.consumeCumulative (LocalUsageReaders.swift): the first total_token_usage event is emitted as full usage. Correct for transcripts parsed from the start, but if a scan begins mid-session (file truncated/rotated) the pre-scan cumulative total is double-attributed. Consider requiring an explicit baseline before emitting.
  2. AuthoritativeCostSupport.aggregate uses precondition(!failures.isEmpty) — safe today since the only caller guards on non-empty failures, but a defensive failures[0] fallback would avoid a crash if a future caller misses the guard.

— Reviewed by unknown model

@phantom5125

Copy link
Copy Markdown
Owner Author

Verdict: not LGTM yet — human confirmation required.

This PR introduces credential-backed balance fetching (Keychain keys sent as Bearer tokens to openrouter.ai and api.deepseek.com), so per review policy it needs a human sign-off on the credential path before merge. CI is green and the implementation is careful: explicit keychain accounts only (no browser/CLI credential reuse), EndpointPolicy host allowlists, LosslessDecimal rejecting booleans/non-finite values, and partial-source warnings instead of invented data.

One code item:

  • AuthoritativeCostSupport.aggregate starts with precondition(!failures.isEmpty) (Sources/TokenLinkProviders/Costs/AuthoritativeCostProvider.swift). Today's call sites guarantee non-empty, but a precondition crash is a harsh failure mode for a UI-facing provider layer; prefer returning a generic .network failure on the empty case.

— Reviewed by unknown model

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant