This repository was archived by the owner on Jun 8, 2026. It is now read-only.
Conversation
- Add canonical BillingMode enum (src/shared/billing-mode.ts) and a
best-effort detector (src/main/billing-mode-detector.ts) that maps
(harness, env, credentials-file existence) to api / claude_max /
codex_chatgpt_pro / cursor_pro / copilot_seat / opencode / unknown.
Credentials files are existence-checked only — contents are never
read or logged. ANTHROPIC_API_KEY / OPENAI_API_KEY beat any
credentials file.
- Stamp harness + billingMode at every Claude/Codex spawn site
(symphony-loop LLM commit, symphony-interactive streamClaudeChat
and Haiku commit, codex.ts engineer-chat / Codex review / Codex
conversation, chat-providers Codex, symphony-utils bootstrap) via
a new recordSessionSpawn shared helper that writes into the
worktree's launch-metadata.json. Per-harness importers
(cursor/copilot/opencode) stamp a fixed mode on the sessions row
via the new setSessionBillingMode prepared statement; codex
importer defaults to codex_chatgpt_pro (overridden by launch
metadata when desktop spawned it with an API key).
- Schema migration in build-agent-monitor.mjs adds
`billing_mode TEXT NOT NULL DEFAULT 'unknown'` + an index, mirroring
the existing harness migration. The generated db.js carries the
ALTER TABLE + setSessionBillingMode statement; verified by building
the agent-monitor tree.
- Sync contract bumped to v2 with optional billingMode on
SyncedAgentSession. The desktop sync service joins the sessions row
with launch-metadata so the env-detected mode wins over the
importer-stamped row value. Legacy v1 payloads remain decodable —
see the inline note in agent-session-sync-contract.ts; the cloud
relay must accept missing billingMode as 'unknown' until a removal
ticket is filed.
- Sessions.tsx renders two stat tiles ("Metered API spend" /
"Subscription-covered") and a per-row cost cell that says
"Covered · $X equiv." for subscription modes. Subscription cost is
NEVER summed into the headline (it's on its own muted tile).
- Add showSubscriptionEquivalentCost privacy setting (default true)
on SettingsStore + DesktopSettings so users can hide the
subscription dollar figure for screen-sharing.
- Step 7 (quota indicator from anthropic-ratelimit-* headers) is
deferred — no header plumbing currently exists between the Claude
Code subprocess and the desktop; the stream-json output swallows
HTTP headers. The UI silently omits the quota pill, matching the
plan's accepted-for-v1 fallback.
- Bump apps/desktop version 0.15.93 -> 0.15.94.
Testing:
- pnpm -C apps/desktop typecheck: clean.
- pnpm -C apps/desktop lint: clean.
- pnpm -C apps/desktop test: 1834 pass / 0 fail (tsx) + 104 pass / 0
fail (node --test). New tests: 11 billing-mode-detector cases (env,
disk, dispatch, anti-leak guard) + 3 schema-migration cases
(additive, idempotent, index) + 3 sync-service round-trip cases
(column → payload, launch-metadata override, schema version bump).
- pnpm -C apps/desktop build:agent-monitor: clean; generated db.js
shows the new ALTER + setSessionBillingMode statement; all four
non-Claude importers received the stamp; client bundle includes the
two-ledger UI logic.
Risks:
- External contract bump (schema v2). Outbound payloads still
decode under v1 readers — they just omit billingMode — but a
cloud relay that enforces strict v2 requires a coordinated rollout.
Tracking ticket for removing the v1 acceptance path needs to be
filed; the inline comment in agent-session-sync-contract.ts is the
pointer.
- Existing sessions migrate to billing_mode='unknown' on first DB
open; UI shows them under neither ledger (unknown ≠ api ≠
subscription). New rows from the next sidecar startup carry a real
mode.
…ation Codex review of 3ebc393 surfaced three issues; this commit stacks fixes on top. - [P1] Two-ledger UI was reading sessions.billing_mode directly but that column stayed at the importer default (or schema 'unknown') for Claude-hook-created sessions even when launch-metadata carried the real mode. loadSyncedSessions now writes the resolved mode back to the row (only over importer defaults — never a deliberate setting) so local UI matches the cloud relay's view. - [P2] recordSessionSpawn was wired at the LLM-commit helper but NOT at the main handleLoopRequest spawn paths (Plan, Execute, RequestChanges, GeneratePrd, RequestPrdChanges, EvaluatePrd, EvaluateFeature, EvaluatePlan, EvaluateCode, Decompose, Bootstrap). Extract a one-call tagSpawnedSession helper into symphony-utils.ts and invoke it once at the shared spawn-prep block in symphony-loop.ts, plus at the learnings-processing spawn (process-chat-learnings.sh invokes claude internally). - [P2] AgentSessionSyncService did SELECT billing_mode FROM sessions directly; if the sidecar startup ALTER hadn't landed yet, this threw `no such column`. Added an idempotent ensureBillingModeColumn migration (try/SELECT → catch/ALTER) run inside syncOnce so the sync service is self-sufficient regardless of sidecar boot order. Bump apps/desktop version 0.15.94 -> 0.15.95. Testing: - pnpm -C apps/desktop typecheck: clean. - pnpm -C apps/desktop lint: clean. - pnpm -C apps/desktop test: 1842 pass / 0 fail (tsx) + 104 pass / 0 fail (node --test). 8 new tests: 3 writeback / migration cases in agent-session-sync-service.test.ts (writeback fires for importer defaults, NEVER clobbers a non-default value, ensureBillingModeColumn adds the column + index idempotently); 5 tagSpawnedSession cases in symphony-utils.test.ts (claude API, codex API, cursor fixed-default, malformed-worktree resilience, source-level regression guard that asserts symphony-loop.ts imports + invokes tagSpawnedSession). Risks: - The writeback only fires when launch-metadata's billingMode differs from the row column AND the row is a known importer default. The set is hardcoded ({unknown, codex_chatgpt_pro, cursor_pro, copilot_seat, opencode}); a future taxonomy expansion needs to update BILLING_MODE_IMPORTER_DEFAULTS or switch to a priority-rank source field. Inline comment flags this for follow-up. - ensureBillingModeColumn runs on every syncOnce. The probe is one SELECT LIMIT 1 — cheap — but a sidecar with corrupt schema would hit the inner try/catch and silently skip. Acceptable: the sync service already swallows all unexpected DB errors at the syncOnce boundary; this just keeps the column-missing path recoverable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The FEA-1434 review-fix agent flagged that agent-session-sync-contract.ts still says "Tracking ticket TBD — see the report" for removing the v1 decode path. The tracking ticket (FEA-1439) was created out-of-band during the FEA-1434 rollout; this commit just substitutes it into the comment so future readers find it cleanly. No behavior change. No tests touched. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closedloop convention requires a version bump on every commit touching apps/desktop/. 6ea51df edited the contract comment without bumping; adding the bump now as a follow-up commit per the "create new commit rather than amend" rule. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex review of ea40777 surfaced four P2 findings; this commit stacks fixes on top. - [A] loadSyncedSessions UPDATE was `WHERE id = ?` only — between the read loop scheduling a writeback and the batched flush, a concurrent writer (sidecar importer or manual fix) could have replaced the importer default with a deliberate non-default value, and the unconditional UPDATE would clobber it. Extract IMPORTER_DEFAULT_ BILLING_MODES (single source of truth for the read-side Set and the write-side WHERE-IN placeholders), tighten the UPDATE to `WHERE id = ? AND (billing_mode IS NULL OR billing_mode IN (...))`. IS NULL branch mirrors the read-side null allowance — SQL IN does not match NULL. - [B] Bootstrap's upstream tagSpawnedSession stamped `worktreeDir ?? claudeWorkDir`, but the generated bootstrap script `cd`s into each `entry.localPath` before each `$CLAUDE_BIN` so the per-repo sessions' `sessions.cwd` landed at `entry.localPath` (not covered by the upstream stamp). Extract tagBootstrapPerRepoLaunchMetadata helper and invoke it after the manifest is built so launch-metadata is pre-written at every runnable entry.localPath before the bash script spawns. The shell script can't easily call the JS helper inline — static cwds known at spawn time make pre-writing the cleanest fix. - [C] ClaudeProvider.spawn in chat-providers.ts did not stamp launch-metadata; CodexProvider at the sibling site did. Mirror the Codex stamp before delegating to processManager.spawnStreaming — uses recordSessionSpawn with detectBillingModeForHarness("claude", shellEnv). Only stamps when resolveSpawnCwd returned a path (no worktree → no metadata file). - [D] spawnClaudeReview() in codex.ts spawned `claude` for the PR-review flow without stamping. The sibling spawnCodexReviewProcess was already stamped. Hoist the shellEnv into a const, add recordSessionSpawn before spawn(claudeBin, ...). Out of scope (filed as a follow-up FEA below): the billingModeByCwd cache in launch-metadata.ts keeps the first observed metadata for the process lifetime, so a re-tag in the same cwd is masked until the desktop app restarts. Real but minor — restart fixes it. Bump apps/desktop version 0.15.96 -> 0.15.97. Testing: - pnpm -C apps/desktop typecheck: clean. - pnpm -C apps/desktop lint: clean. - pnpm -C apps/desktop test: 1848 pass / 0 fail (tsx) + 104 pass / 0 fail (node --test). +6 new tests vs round 1 baseline (1842+104): agent-session-sync-service.test.ts adds (1) a TOCTOU race test that monkey-patches db.prepare to inject a concurrent UPDATE between prepare() and run() and asserts the deliberate value survives, and (2) a source-level guard asserting the UPDATE SQL contains the importer-default WHERE-clause predicate. symphony-utils.test.ts adds (3) a unit test for tagBootstrapPerRepoLaunchMetadata with two fixture entries, (4) a source-level guard that the Bootstrap branch invokes the per-repo helper, (5) a source-level guard that ClaudeProvider's class body contains recordSessionSpawn (not just somewhere in the file — CodexProvider already had it), and (6) a source-level guard scoped to the spawnClaudeReview function body. Risks: - The race test relies on monkey-patching db.prepare, which is a white-box hack. If the sync service stops caching the prepared statement, or switches to a different SQL builder, the interceptor may not fire — but the source-level guard catches the regression independently. - The Bootstrap per-repo stamp runs once per runnable entry, before the bash script spawns. If a future refactor moves the bootstrap script to clone repos lazily (where entry.localPath is not known at spawn time), the pre-write strategy breaks. The source-level guard would still pass (the call is still there) so the failure mode would be runtime-only; flagged for future review. - ClaudeProvider's stamp uses resolveSpawnCwd to compute the cwd once and reuse it for both the stamp and the spawn. Pre-fix the stamp would have been skipped silently when params.cwd was undefined; post-fix it is still skipped — the cwd-required path is the same as before. Follow-up FEA suggestion (not addressed in this commit, see Out of scope above): "Invalidate billingModeByCwd cache on launch-metadata file change" — the cache in launch-metadata.ts keeps the first observed metadata for the lifetime of the desktop process, so a re-stamp in the same cwd (e.g. after the user fixes their API key in settings without restarting) is masked. Real but minor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… guard Claude review of ee4f7e4 surfaced three findings; this commit stacks fixes on top. - [Finding 1 / M] showSubscriptionEquivalentCost was fully dead: defined in DEFAULT_DESKTOP_SETTINGS + SettingsStore getter/setter + desktop:get-settings IPC, but never read by Sessions.tsx — flipping it had zero UI effect. Two options were considered; Option A (wire it through a new sidecar /api/settings route + iframe polling + conditional render) is >150 LOC across the build-script patch, the sidecar route, a transport mechanism, the client polling, and tests. Picked Option B per review guidance: remove the dead field from contracts.ts and the getter/setter from settings-store.ts, and defer the privacy toggle to FEA-1445 (filed before commit). Regression test in settings-migration.test.ts pins the field-and-method removal so a future merge cannot silently re-add a dead setting. - [Finding 2 / M] Sidecar importer could clobber writeback 'api' mode. The sidecar's setSessionBillingMode UPDATE was COALESCE(billing_mode,'') != ?, so if the desktop sync-service had persisted 'api' (because the user has OPENAI_API_KEY) and the sidecar restarted, codex-import.js would overwrite 'api' → 'codex_chatgpt_pro' until the next sync cycle restored it (~5s window where Sessions UI mis-buckets). Narrow the UPDATE to NOT IN (?, 'api', 'claude_max', 'claude_pro') so importers can still promote 'unknown' but never demote a deliberate non-default value. The exclusion list is sourced from a protectedModes array in build-agent-monitor.mjs (single source of truth — adding a new protected mode is a one-line array edit). All four importers (codex / cursor / copilot / opencode) updated to pass the additional placeholder bindings. - [Finding 3 / L] claude_pro is a dead BillingMode entry today — detectClaudeBillingMode always returns claude_max for OAuth Claude because the credentials file does not distinguish Pro from Max. Per review guidance, leave it in the union (removing it would be a cloud-relay schema breaking change) and document why. Added a multi-line comment in billing-mode.ts explaining the three reasons it stays: reserved for a future signal, cloud-relay contract stability, and the new importer-clobber guard already protects it. Regression test in billing-mode-detector.test.ts pins (a) the type variant, (b) SUBSCRIPTION_MODES membership, and (c) the reserved-for-future-signal comment text so a "dead enum entry" cleanup pass cannot silently drop the variant. Side effect of Finding 2: rebuilt apps/desktop/.generated/agent- monitor/ (gitignored — only the build-script patch source is checked in). The new prepared statement renders as UPDATE sessions SET billing_mode = ? WHERE id = ? AND COALESCE(billing_mode, '') NOT IN (?, ?, ?, ?) verified in the regenerated server/db.js. Filed follow-up FEA-1445 "Subscription cost privacy toggle — wire showSubscriptionEquivalentCost through sidecar" for the Option-A path. Slug: FEA-1445. Bump apps/desktop version 0.15.97 -> 0.15.98. Testing: - pnpm -C apps/desktop typecheck: clean. - pnpm -C apps/desktop lint: clean. - pnpm -C apps/desktop test: 1860 pass / 0 fail (tsx, +5 new tests vs round 2 baseline 1855) and 104 pass / 0 fail (node --test). New tests: 1. settings-migration.test.ts: showSubscriptionEquivalentCost is NOT in DEFAULT_DESKTOP_SETTINGS (deferred to FEA-1445). 2. settings-migration.test.ts: SettingsStore has no get/setShowSubscriptionEquivalentCost method. 3. billing-mode-importer-clobber-guard.test.ts (new file, 7 tests): importer promotes 'unknown' → harness default; cannot clobber 'api', 'claude_max', 'claude_pro'; is a no-op when the row already carries the target mode; can still promote between importer defaults; build-script source pins the protected-mode exclusion list. 4. billing-mode-detector.test.ts: claude_pro is still a valid BillingMode union variant (type-level pin). 5. billing-mode-detector.test.ts: claude_pro remains in SUBSCRIPTION_MODES. 6. billing-mode-detector.test.ts: billing-mode.ts comment still explains why claude_pro is reserved-for-future-use. Risks: - The protected-mode exclusion list lives in two places: the SQL patch in build-agent-monitor.mjs (renders the prepared statement) and the JS arg arrays in each importer (bound at .run() time). Adding a new protected mode requires both edits. The new clobber- guard test pins the build-script side; the importers will throw at runtime on a placeholder/arg-count mismatch. - Option B (Finding 1) is a contract change to DesktopSettings — the field is gone. Per AGENTS.md "Breaking Changes", this is an internal contract (IPC, shipped atomically with the app), so no legacy migration is required. The settings file on disk simply loses the unused key on the next save; downgrade is the inverse — older app versions default the missing field to true. - Finding 3 keeps claude_pro in the union. If a future detectClaudeBillingMode learns to distinguish tiers, the taxonomy is ready; if not, the variant stays dormant with no runtime cost. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Compatibility Smoke Test Results
|
thadeusb
reviewed
May 29, 2026
| const ledgerTotals = sessions.reduce( | ||
| (acc, s) => { | ||
| const cost = s.cost ?? 0; | ||
| if (isApiMetered(s.billing_mode)) { |
Contributor
There was a problem hiding this comment.
This rollup drops anything that isn't 'api' or a subscription mode, so an 'unknown' session lands in neither card. But down at line 462 that same row still renders its cost as a bare dollar amount, identical to an api row. 'unknown' is the schema default for every pre-migration row and for any spawn where detection fell through (no API key env, no creds file), so you'll routinely have rows showing '$3.00' in the column that aren't in the Metered API spend total. The two cards won't add up to what the user sees in the table. Either fold unknown into the api bucket or render it muted so it's clear it's uncounted.
thadeusb
reviewed
May 29, 2026
| // Existence-only check — never read or log the contents of this file. The | ||
| // OAuth token inside the credentials file must not appear in logs or | ||
| // telemetry; see CLAUDE.md "spawn|argv|env|secrets". | ||
| if (existsSync(expandHome(claudeCredentialsPath))) { |
Contributor
There was a problem hiding this comment.
This misses Bedrock/Vertex. A run with CLAUDE_CODE_USE_BEDROCK or CLAUDE_CODE_USE_VERTEX is metered through the cloud provider and has no ANTHROPIC_API_KEY, so if there's a stale ~/.claude/.credentials.json from a past OAuth login it gets tagged claude_max and disappears from the headline. That's the expensive direction to get wrong, real spend rendered as covered. The doc comment only calls out the Pro-vs-Max ambiguity, this case is worth handling too.
thadeusb
reviewed
May 29, 2026
| // canonical enum in src/shared/billing-mode.ts on the desktop side. Inlined | ||
| // here because the agent-monitor client is bundled separately from the | ||
| // desktop main TypeScript tree. | ||
| const SUBSCRIPTION_BILLING_MODES = new Set<string>([ |
Contributor
There was a problem hiding this comment.
This is a hand copy of SUBSCRIPTION_MODES in billing-mode.ts with nothing tying them together. Add a 9th subscription mode there and the importers/detector will stamp it but this UI silently buckets it as uncounted, and no build gate catches the drift. The separate-bundle reason is fair, but worth at least a test that asserts the two sets match.
thadeusb
approved these changes
May 29, 2026
thadeusb
left a comment
thadeusb
left a comment
Contributor
There was a problem hiding this comment.
Ledger split is built well, the importer clobber guard and the v1/v2 contract handling especially. One real thing: 'unknown' sessions show a dollar cost in the table but land in neither rollup card, and 'unknown' is the default for every pre-migration row, so the cards won't reconcile with what's on screen. Couple smaller notes on detection inline.
Contributor
Author
|
Superseded by a clean single-PR rebuild on The FEA-1434 two-ledger / per-session billing-mode architecture is being KEPT — only the pricing foundation underneath it is being rebuilt on @pydantic/genai-prices (PRD-414 v2). This work is being consolidated into one PR rather than three phases. Head commit archived locally as tag archive/pr-254-FEA-1434 for reference during the rebuild. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Parent PRD: PRD-414 — Accurate token cost calculation & reconciliation across multi-vendor agents.
Summary
Phase 2 of the token cost work. Distinguishes API-metered sessions from subscription-covered sessions (Claude Max, ChatGPT Pro, Cursor Pro, Copilot, OpenCode) so a heavy Max user doesn't see "$1,800 spent" on a $200 flat plan.
BillingModetype insrc/shared/billing-mode.ts(api | claude_pro | claude_max | codex_chatgpt_pro | cursor_pro | copilot_seat | opencode | unknown).src/main/billing-mode-detector.ts. File-existence-only probes — token contents are NEVER read or logged.tagSpawnedSessionhelper invoked at every Claude/Codex spawn site (symphony-loop, symphony-interactive, chat-providers ClaudeProvider + CodexProvider, codex.ts both review paths, learnings.ts shell-script spawn).sessionstable gets abilling_modecolumn via additive ALTER migration. The sync-service runs an idempotent migration before any SELECT to handle the sidecar-startup race.Review chain:
Schema-version coordination
This PR bumps `AGENT_SESSION_SYNC_SCHEMA_VERSION` from 1 → 2. Sibling PR (#253) also bumps 1 → 2. Whichever merges first wins; the second needs to rebase to 2 → 3.
Follow-up tickets filed during the work
Test plan
🤖 Generated with Claude Code