fix(windows): publish account usage in background - #559
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (8)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughChangesBackground sync flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🔵 Low · up to This PR enables periodic Windows background publication of account usage through the local service. A bounded shutdown or timeout window could allow synchronization state to clear before the underlying operation fully exits, potentially delaying a retry or causing duplicate work; the change is otherwise mergeable with explicit owner awareness. Sequence Diagram(s)sequenceDiagram
participant ServerManager
participant LocalSyncPublisher
participant LocalAPI
participant TrackerSync
ServerManager->>LocalSyncPublisher: RunBackgroundSyncAsync
LocalSyncPublisher->>LocalAPI: GET /api/local-auth
LocalAPI-->>LocalSyncPublisher: Return token
LocalSyncPublisher->>LocalAPI: POST authenticated background payload
LocalAPI->>TrackerSync: Spawn sync with native-only WSL environment
TrackerSync-->>LocalAPI: Complete sync
LocalAPI-->>LocalSyncPublisher: Return publish response
LocalSyncPublisher-->>ServerManager: Complete background sync
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 7 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
xiufengsun
left a comment
xiufengsun
left a comment
There was a problem hiding this comment.
The Windows loopback publisher and its tests look coherent on exact head e6ab6994b59c7fbbccb661dda1460d673fa53a80, but the account publication still cannot succeed reliably against current main.
drainQueueToCloud() currently sources the InsForge anonymous key only from TOKENTRACKER_INSFORGE_ANON_KEY. The native background request launches a child without that environment key; the configured runtime key available in the local server is not forwarded into the uploader. As a result, this PR schedules publication every five minutes but normally sends ingest without the required apikey header.
Please either incorporate a concurrency-safe runtime-key forwarding fix or rework this flow so the child receives the local server's current anonymous key. Do not copy #551 as-is: its current head can delete a newly written key during a concurrent device login. Add an end-to-end regression that starts from the native loopback request with no environment key and verifies the ingest request carries the runtime key.
2 participants
Summary
Keep Windows account usage current while the dashboard is hidden by routing the five-minute native background sync through the authenticated local publication endpoint.
Scope
src/)dashboard/)TokenTrackerBar/)TokenTrackerWin/)Why
On Windows v0.94.1, the raw session files, V2 parser cursor, local queue, and local usage API could all be current while the signed-in account view remained several days behind. The native host log showed the five-minute timer repeatedly exiting successfully with:
sync --auto --backgroundThat path intentionally performs local parsing only. Account publication requires
--publish-accountplus an ephemeral device token, so a tray-only Windows session never advanced its cloud upload offset. Opening the dashboard could make one bounded upload attempt, but it was not a periodic publisher and a large backlog stayed stale.This is independent of #551: that PR fixes the ingest anon-key value after publication starts; this PR fixes the missing Windows publication trigger itself.
What changed
GET /api/local-authfollowed by an authenticatedPOST /functions/tokentracker-local-sync.auto,background,allLocalSources,publishAccount, andnativeOnlyWslas strict booleans.nativeOnlyWslonly for a real background request and passesTOKENTRACKER_WSL_MODE=native-onlyto the child.LocalSyncPublisheris covered by executable xUnit tests rather than source-text assertions alone.Checklist
npm testpassesdashboard/src/content/copy.csv(no new strings)fix(windows): ...)Verification
node --test test/windows-background-sync-source.test.js test/windows-background-sync-args.test.js test/local-api-background.test.js test/local-api-security.test.js— 37 passednpm run validate:copy— passed (existing unused-key warnings only)npm run validate:locale— passednpm run validate:ui-hardcode— passednpm run validate:guardrails— passednpm run validate:versions— passednode --test test/architecture-guardrails.test.js— 4 passednpm --prefix dashboard run build— passednpm testwas attempted locally on Windows/Node 25; unrelated host- and timing-dependent baseline tests fail in this environment. The changed-scope tests above are green.Risk layer addendum
Risk layer triggers
Rules / invariants
Boundary matrix
publishAccounteven though the native request asks for it.nativeOnlyWsl: ignored; no environment override is injected.Codex review context
LocalSyncPublisher; replaced a too-short client timeout with server-owned lifetime plus shutdown cancellation.Most likely regression surface
Windows native sync lifecycle, loopback local-auth exchange, shutdown cancellation, and WSL background isolation.
Verification method
Focused Node contract tests, executable xUnit HTTP protocol tests in CI, Windows Release build in CI, and a privacy-preserving live data-flow reproduction.
Uncovered scope
Local .NET compilation is pending CI because no SDK is installed on the reproducing machine.
Summary by CodeRabbit
New Features
Bug Fixes
Tests