feat: multi-device support with UUID device id and multi device TUI aggregation#329
feat: multi-device support with UUID device id and multi device TUI aggregation#329blpeng2 wants to merge 13 commits intojunhoyeo:mainfrom
Conversation
|
@blpeng2 is attempting to deploy a commit to the Inevitable Team on Vercel. A member of the Team first needs to authorize it. |
![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: 372dd88e18
ℹ️ 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".
crates/tokscale-cli/src/tui/mod.rs
Outdated
| app.data_source = data_source; | ||
|
|
||
| let (bg_tx, bg_rx) = mpsc::channel::<Result<UsageData>>(); | ||
| let needs_background_load = !has_cached_data || cache_is_stale; |
There was a problem hiding this comment.
When a remote cache hit is used (effective_data comes from remote_cached), this condition still schedules the local background loader whenever the local cache is stale or missing. In that common case, the local loader result later replaces the remote aggregate in the app state (and flips the source to local), so multi-device totals disappear a moment after startup instead of staying synced. This makes the new remote aggregation path unreliable unless the local cache is also fresh.
Useful? React with 👍 / 👎.
| const sourceBreakdown = recalculateClientAggregate({ | ||
| [deviceId]: incomingClientBreakdown, | ||
| }) as DailySourceBreakdown; |
There was a problem hiding this comment.
New-day inserts use the raw deviceId as the map key here, but deviceId is only defaulted for null headers, not empty values. If the client sends X-Device-Id: "" (which can happen when CLI UUID creation falls back to an empty string), rows are stored under devices[""]; later updates normalize to "__legacy__", so the same device can end up split across two buckets and be double-counted.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
17 issues found across 13 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/tokscale-cli/src/device.rs">
<violation number="1" location="crates/tokscale-cli/src/device.rs:36">
P1: Non-atomic device-id initialization allows concurrent first-run processes to assign different IDs on the same machine.</violation>
<violation number="2" location="crates/tokscale-cli/src/device.rs:38">
P2: Existing device-id file content is not validated as a UUID; any non-empty (including partially written/corrupted) value is permanently reused.</violation>
</file>
<file name="packages/frontend/src/app/api/me/stats/route.ts">
<violation number="1" location="packages/frontend/src/app/api/me/stats/route.ts:152">
P2: Legacy flat `sourceBreakdown` rows are not represented in `devices`, causing undercounted/incomplete device totals for legacy or mixed-format histories.</violation>
</file>
<file name="crates/tokscale-cli/src/tui/ui/footer.rs">
<violation number="1" location="crates/tokscale-cli/src/tui/ui/footer.rs:247">
P2: Status row now always prepends a sync badge in a 1-line footer without narrow-width fallback, causing important status text to be truncated on small terminal widths.</violation>
</file>
<file name="crates/tokscale-cli/src/tui/mod.rs">
<violation number="1" location="crates/tokscale-cli/src/tui/mod.rs:159">
P1: Remote cached stats are injected into TUI without applying CLI client/date filters, causing potentially incorrect filtered views.</violation>
<violation number="2" location="crates/tokscale-cli/src/tui/mod.rs:233">
P1: Background remote stats fetch is disconnected from the running TUI: fetched data is discarded and never sent into the UI update channel.</violation>
<violation number="3" location="crates/tokscale-cli/src/tui/mod.rs:305">
P1: When remote cached stats are active, a stale local cache still triggers background local reload, and completion unconditionally switches `data_source` to Local, overriding the remote aggregated view.</violation>
</file>
<file name="packages/frontend/src/app/api/submit/route.ts">
<violation number="1" location="packages/frontend/src/app/api/submit/route.ts:67">
P2: Blank X-Device-Id headers ("" value) won’t fall back to "__legacy__" because `Headers.get()` returns a string even when the header is present but empty. Using `??` means empty string becomes the device key, creating a separate bucket instead of the intended legacy fallback.</violation>
</file>
<file name="packages/frontend/src/lib/db/helpers.ts">
<violation number="1" location="packages/frontend/src/lib/db/helpers.ts:34">
P2: `SourceBreakdown` now mixes client entries with a nested `devices` object, breaking consumers that still iterate it as a flat `Record<string, ClientBreakdownData>`.</violation>
<violation number="2" location="packages/frontend/src/lib/db/helpers.ts:90">
P1: Legacy-to-device migration keeps `__legacy__` data while writing new UUID data to a separate bucket, and aggregate recomputation sums both, causing inflated totals after upgrade.</violation>
</file>
<file name="packages/frontend/__tests__/api/meStats.test.ts">
<violation number="1" location="packages/frontend/__tests__/api/meStats.test.ts:69">
P1: The new `/api/me/stats` tests mock Drizzle in a query-agnostic way and never assert predicate arguments, so user/submission scoping regressions could pass undetected.</violation>
</file>
<file name="crates/tokscale-cli/src/main.rs">
<violation number="1" location="crates/tokscale-cli/src/main.rs:3018">
P1: Device-id errors are silently converted to an empty header value, which bypasses missing-header legacy fallback and can store submissions under an empty device key.</violation>
<violation number="2" location="crates/tokscale-cli/src/main.rs:3025">
P2: Submit success updates only local cache; fresh remote aggregated-stats cache can remain stale and still be preferred by TUI for up to TTL.</violation>
</file>
<file name="crates/tokscale-cli/src/tui/remote.rs">
<violation number="1" location="crates/tokscale-cli/src/tui/remote.rs:55">
P1: `RemoteStats` requires `fetched_at_secs` during API deserialization even though it is local metadata and immediately overwritten, risking hard parse failure when the server omits `fetchedAtSecs`.</violation>
<violation number="2" location="crates/tokscale-cli/src/tui/remote.rs:88">
P2: `fetch_remote_stats` incorrectly fails the whole remote fetch when cache persistence fails, discarding valid freshly fetched stats.</violation>
<violation number="3" location="crates/tokscale-cli/src/tui/remote.rs:92">
P2: Remote stats cache is globally keyed, so cached data can be reused across different accounts/API environments within TTL.</violation>
<violation number="4" location="crates/tokscale-cli/src/tui/remote.rs:116">
P2: Atomic cache writes are race-prone because all writers use the same fixed temp filename.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| ensure_config_dir()?; | ||
| let path = get_device_id_path()?; | ||
|
|
||
| if path.exists() { |
There was a problem hiding this comment.
P1: Non-atomic device-id initialization allows concurrent first-run processes to assign different IDs on the same machine.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/tokscale-cli/src/device.rs, line 36:
<comment>Non-atomic device-id initialization allows concurrent first-run processes to assign different IDs on the same machine.</comment>
<file context>
@@ -0,0 +1,157 @@
+ ensure_config_dir()?;
+ let path = get_device_id_path()?;
+
+ if path.exists() {
+ let id = fs::read_to_string(&path)?.trim().to_string();
+ if !id.is_empty() {
</file context>
| deviceId: string | ||
| ): SourceBreakdown { | ||
| const normalizedDeviceId = deviceId || "__legacy__"; | ||
| const devices = normalizeDevices(existing); |
There was a problem hiding this comment.
P1: Legacy-to-device migration keeps __legacy__ data while writing new UUID data to a separate bucket, and aggregate recomputation sums both, causing inflated totals after upgrade.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/frontend/src/lib/db/helpers.ts, line 90:
<comment>Legacy-to-device migration keeps `__legacy__` data while writing new UUID data to a separate bucket, and aggregate recomputation sums both, causing inflated totals after upgrade.</comment>
<file context>
@@ -70,29 +81,95 @@ export function recalculateDayTotals(
+ deviceId: string
+): SourceBreakdown {
+ const normalizedDeviceId = deviceId || "__legacy__";
+ const devices = normalizeDevices(existing);
+ const deviceClients: DeviceClientData = { ...(devices[normalizedDeviceId] || {}) };
</file context>
| [clientName: string]: ClientBreakdownData; | ||
| } | ||
|
|
||
| export interface SourceBreakdown { |
There was a problem hiding this comment.
P2: SourceBreakdown now mixes client entries with a nested devices object, breaking consumers that still iterate it as a flat Record<string, ClientBreakdownData>.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/frontend/src/lib/db/helpers.ts, line 34:
<comment>`SourceBreakdown` now mixes client entries with a nested `devices` object, breaking consumers that still iterate it as a flat `Record<string, ClientBreakdownData>`.</comment>
<file context>
@@ -27,6 +27,15 @@ export interface ClientBreakdownData {
+ [clientName: string]: ClientBreakdownData;
+}
+
+export interface SourceBreakdown {
+ [clientName: string]: ClientBreakdownData | Record<string, DeviceClientData> | undefined;
+ devices?: Record<string, DeviceClientData>;
</file context>
| .post(format!("{}/api/submit", api_url)) | ||
| .header("Content-Type", "application/json") | ||
| .header("Authorization", format!("Bearer {}", credentials.token)) | ||
| .header("X-Device-Id", &device_id) |
There was a problem hiding this comment.
P2: Submit success updates only local cache; fresh remote aggregated-stats cache can remain stale and still be preferred by TUI for up to TTL.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/tokscale-cli/src/main.rs, line 3025:
<comment>Submit success updates only local cache; fresh remote aggregated-stats cache can remain stale and still be preferred by TUI for up to TTL.</comment>
<file context>
@@ -3014,11 +3015,14 @@ fn run_submit_command(
.post(format!("{}/api/submit", api_url))
.header("Content-Type", "application/json")
.header("Authorization", format!("Bearer {}", credentials.token))
+ .header("X-Device-Id", &device_id)
.json(&submit_payload)
.send()
</file context>
| fs::create_dir_all(dir).context("Failed to create remote stats cache directory")?; | ||
| } | ||
|
|
||
| let temp_path = cache_path.with_extension("json.tmp"); |
There was a problem hiding this comment.
P2: Atomic cache writes are race-prone because all writers use the same fixed temp filename.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/tokscale-cli/src/tui/remote.rs, line 116:
<comment>Atomic cache writes are race-prone because all writers use the same fixed temp filename.</comment>
<file context>
@@ -0,0 +1,143 @@
+ fs::create_dir_all(dir).context("Failed to create remote stats cache directory")?;
+ }
+
+ let temp_path = cache_path.with_extension("json.tmp");
+ let file = File::create(&temp_path).context("Failed to create remote stats temp cache file")?;
+ let writer = BufWriter::new(file);
</file context>
- Add #[serde(default)] to fetched_at_secs to prevent deserialization failure when server response omits this local-only metadata field - Soft-fail cache write errors so a cache save failure no longer discards valid freshly-fetched remote stats - Preserve DataSource::Remote when background local reload completes, preventing the local data source from overriding the remote view - Skip background local reload when remote data is already active - Use || instead of ?? for X-Device-Id fallback to handle empty-string headers correctly alongside helpers.ts which already used ||
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90df8e3f06
ℹ️ 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".
crates/tokscale-cli/src/tui/mod.rs
Outdated
| let remote_cached = remote::load_cached_remote_stats(); | ||
| let (effective_data, data_source) = if let Some(ref remote_stats) = remote_cached { | ||
| (Some(remote_stats_to_usage_data(remote_stats)), DataSource::Remote) | ||
| } else { |
There was a problem hiding this comment.
run() unconditionally prefers remote::load_cached_remote_stats() over local data before checking credentials, and the cache file has no user/token identity attached. If a user logs out or switches API tokens/accounts on the same machine, the next TUI start can show a previous account’s aggregated totals as “synced,” which is both incorrect and a data-leak risk until the cache expires.
Useful? React with 👍 / 👎.
crates/tokscale-cli/src/tui/mod.rs
Outdated
| let Ok(runtime) = tokio::runtime::Runtime::new() else { | ||
| return; | ||
| }; | ||
| let _ = runtime.block_on(remote::fetch_remote_stats(&token, &api_url)); |
There was a problem hiding this comment.
When there is no fresh remote cache, the background thread fetches /api/me/stats but drops the returned value (let _ = ...) and only warms disk cache. In that common first-run/stale-cache path, the current session stays on local-only data and multi-device totals do not appear until a full restart, so the new remote aggregation is not actually usable immediately.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
4 issues found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/frontend/src/app/api/submit/route.ts">
<violation number="1" location="packages/frontend/src/app/api/submit/route.ts:67">
P1: Unvalidated `X-Device-Id` is used as a plain-object key, enabling reserved-key/prototype-key abuse (`__proto__`, etc.) in device maps.</violation>
<violation number="2" location="packages/frontend/src/app/api/submit/route.ts:67">
P1: Using `||` for `X-Device-Id` fallback collapses explicit empty header values into the reserved `__legacy__` device bucket, causing cross-device collisions and possible overwrites of legacy-bucket data during per-device merge.</violation>
</file>
<file name="crates/tokscale-cli/src/tui/mod.rs">
<violation number="1" location="crates/tokscale-cli/src/tui/mod.rs:199">
P1: Remote cached stats bypass CLI/TUI filters and the new background-load condition prevents filtered local data from reloading when remote cache exists.</violation>
<violation number="2" location="crates/tokscale-cli/src/tui/mod.rs:305">
P2: Local reload results can replace displayed data while `data_source` remains `Remote`, causing incorrect remote/synced status in the UI.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| */ | ||
| export async function POST(request: Request) { | ||
| try { | ||
| const deviceId = request.headers.get("X-Device-Id") || "__legacy__"; |
There was a problem hiding this comment.
P1: Unvalidated X-Device-Id is used as a plain-object key, enabling reserved-key/prototype-key abuse (__proto__, etc.) in device maps.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/frontend/src/app/api/submit/route.ts, line 67:
<comment>Unvalidated `X-Device-Id` is used as a plain-object key, enabling reserved-key/prototype-key abuse (`__proto__`, etc.) in device maps.</comment>
<file context>
@@ -64,7 +64,7 @@ function normalizeSubmissionData(data: unknown): void {
export async function POST(request: Request) {
try {
- const deviceId = request.headers.get("X-Device-Id") ?? "__legacy__";
+ const deviceId = request.headers.get("X-Device-Id") || "__legacy__";
// ========================================
</file context>
| */ | ||
| export async function POST(request: Request) { | ||
| try { | ||
| const deviceId = request.headers.get("X-Device-Id") || "__legacy__"; |
There was a problem hiding this comment.
P1: Using || for X-Device-Id fallback collapses explicit empty header values into the reserved __legacy__ device bucket, causing cross-device collisions and possible overwrites of legacy-bucket data during per-device merge.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/frontend/src/app/api/submit/route.ts, line 67:
<comment>Using `||` for `X-Device-Id` fallback collapses explicit empty header values into the reserved `__legacy__` device bucket, causing cross-device collisions and possible overwrites of legacy-bucket data during per-device merge.</comment>
<file context>
@@ -64,7 +64,7 @@ function normalizeSubmissionData(data: unknown): void {
export async function POST(request: Request) {
try {
- const deviceId = request.headers.get("X-Device-Id") ?? "__legacy__";
+ const deviceId = request.headers.get("X-Device-Id") || "__legacy__";
// ========================================
</file context>
| const deviceId = request.headers.get("X-Device-Id") || "__legacy__"; | |
| const deviceId = request.headers.get("X-Device-Id") ?? "__legacy__"; |
…etch to live session - Store cached_for_user in remote stats cache and reject it when the current account differs, preventing a previous account's aggregated totals from appearing as synced after a token or account switch - Load credentials before loading remote cache so the username is available for the scope check - Add a dedicated remote_rx channel so background-fetched remote stats are delivered to the running TUI instead of only warming disk cache, making multi-device aggregation visible immediately in the same session
There was a problem hiding this comment.
3 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/tokscale-cli/src/tui/remote.rs">
<violation number="1" location="crates/tokscale-cli/src/tui/remote.rs:91">
P2: Remote stats cache is keyed only by username, not API base URL, so same usernames across servers can receive stale data from the wrong backend.</violation>
<violation number="2" location="crates/tokscale-cli/src/tui/remote.rs:114">
P1: Account-scoped cache invalidation is bypassed for legacy/unknown-owner cache entries because empty `cached_for_user` is accepted as valid for any user.</violation>
</file>
<file name="crates/tokscale-cli/src/tui/mod.rs">
<violation number="1" location="crates/tokscale-cli/src/tui/mod.rs:339">
P1: Remote mode can be silently overwritten by local reload results, leaving the UI labeled as synced/remote while displaying local data.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18a43c7a8f
ℹ️ 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".
crates/tokscale-cli/src/tui/mod.rs
Outdated
| app.update_data(data); | ||
| if app.data_source != DataSource::Remote { | ||
| app.data_source = DataSource::Local; | ||
| } |
There was a problem hiding this comment.
Fresh evidence: even after the new startup guard, this block still applies local reload results via app.update_data(data) and only conditionally updates data_source. When users press r or enable auto-refresh, remote multi-device aggregates are replaced with local-only data while the footer can still report remote sync, so totals become incorrect and the sync state is misleading.
Useful? React with 👍 / 👎.
| let (effective_data, data_source) = if let Some(ref remote_stats) = remote_cached { | ||
| (Some(remote_stats_to_usage_data(remote_stats)), DataSource::Remote) | ||
| } else { | ||
| (cached_data, DataSource::Local) |
There was a problem hiding this comment.
run() prefers cached remote stats unconditionally in this branch, but the new /api/me/stats endpoint aggregates all days/clients for the account (packages/frontend/src/app/api/me/stats/route.ts reads all daily_breakdown rows without filter predicates). In filtered TUI sessions (--claude, --since, --month, etc.), this causes unfiltered totals to be shown and prevents the requested scope from being honored.
Useful? React with 👍 / 👎.
…ries - Add cached_for_api_url field to RemoteStats for server-scoped caching - Validate API URL in load_cached_remote_stats to prevent cross-server data leaks - Reject cache entries with empty cached_for_user when expected_user is provided - Prevents stale data from wrong backend and legacy cache bypass
- Skip local data update when in Remote mode to preserve authoritative remote state - Ensures UI label (Synced) matches displayed data source - Shows Remote data unchanged status instead of silently overwriting
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/tokscale-cli/src/tui/mod.rs">
<violation number="1" location="crates/tokscale-cli/src/tui/mod.rs:160">
P1: Logged-out TUI sessions can still load and display cached remote account stats because remote cache lookup is not gated on credentials and username scoping is skipped when `None`.</violation>
</file>
<file name="crates/tokscale-cli/src/tui/remote.rs">
<violation number="1" location="crates/tokscale-cli/src/tui/remote.rs:124">
P2: Remote cache scoping compares unnormalized API base URL strings, so equivalent URLs with/without trailing slash incorrectly invalidate fresh cache.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Trim trailing slashes from cached and expected API URLs - Prevents false cache invalidation for equivalent URLs - Fixes mismatch between https://api.example.com and https://api.example.com/
- Skip remote cache lookup when user is logged out - Prevents unauthorized display of cached remote account stats - Ensures remote data is only shown to authenticated users
1 participant
Summary
Adds end-to-end multi-device support so users working across multiple machines can see aggregated token usage stats in the TUI.
This builds on the approach from #252, but replaces the tokenId-based device key with a persistent UUID device-id stored locally. Using tokenId as the device key causes double-counting when the API token is renewed — the same machine gets a new tokenId and resubmits all historical data under a different key. A locally-persisted UUID survives token renewals.
Flow:
Changes
CLI
Introduce persistent UUID device-id (
device.rs)~/.config/tokscale/device-id0o600permissions on Unix for consistency with existing auth storageInclude device identity in submissions
run_submit_command()sendsX-Device-Idheader with each/api/submitrequestAdd remote stats module for TUI
remote.rsimplementsfetch_remote_stats()forGET /api/me/statsload_cached_remote_stats()reads~/.cache/tokscale/remote-stats-cache.jsonAPI
Accept per-device submissions
/api/submitreadsX-Device-Idheader"__legacy__"when header is missing for backward compatibilityAdd cross-device aggregation endpoint
GET /api/me/statsDatabase / Aggregation Logic
Introduce
devices[deviceId]structure for client breakdownsUpdate
mergeClientBreakdowns()to perform per-device replacementAdd
normalizeDevices()to lazily migrate legacy datasourceBreakdownis wrapped underdevices["__legacy__"]Update
recalculateClientAggregate()to recompute totalsTUI
Integrate remote aggregated stats
Startup behavior
Background synchronization
Add
DataSourceenum (Local/Remote)Footer sync indicator
syncedwhen remote data is activelocal onlywhen running without remote statsRelation to #252 and #313
Test Plan
Backward Compatibility
Summary by cubic
Adds multi-device support with a persistent UUID device ID, a cross-device stats endpoint, and TUI aggregation with a synced indicator. Hardens remote stats caching by scoping to account and API server to avoid cross-account/server data.
New Features
Bug Fixes
Written for commit 91cbb99. Summary will update on new commits.