From cc99e5078872091332e7a2026838e6ff03a7f126 Mon Sep 17 00:00:00 2001 From: Niko Savas Date: Sun, 26 Jul 2026 14:40:12 -0700 Subject: [PATCH 1/6] Add design spec for opt-in background scrobbling beta Documents the architecture for a backend that continues large Last.fm imports while the user's browser is closed, offered only when a selection exceeds the 2,700/day limit. Key decisions: - Cloudflare Workers free tier to start, with explicit portability rules and a ~24-concurrent-job ceiling as the trigger to move to a VM. - Auth handoff via a second Last.fm round-trip so the client's session key is never transmitted; server holds its own, deleted on completion. - Duplicate safety by reconciling against Last.fm rather than relying on cursor granularity, since the ambiguity window is irreducible. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2c093209-9749-4d93-990f-c970f26d570e --- ...2026-07-26-background-scrobbling-design.md | 310 ++++++++++++++++++ 1 file changed, 310 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-26-background-scrobbling-design.md diff --git a/docs/superpowers/specs/2026-07-26-background-scrobbling-design.md b/docs/superpowers/specs/2026-07-26-background-scrobbling-design.md new file mode 100644 index 0000000..da1b323 --- /dev/null +++ b/docs/superpowers/specs/2026-07-26-background-scrobbling-design.md @@ -0,0 +1,310 @@ +# Background Scrobbling (Beta) — Design + +**Date:** 2026-07-26 +**Status:** Approved, pending implementation plan + +## Problem + +Last.fm enforces an undocumented daily scrobble limit of roughly 2,800 per user. +Scrobblify paces itself at 2,700/day (`ScrobbleStep.vue:89`) and pauses when it +hits that ceiling. A user importing a 100,000-track Spotify history must +therefore return to the site once a day for about 37 consecutive days. + +Save-and-resume (`src/services/StateManager.ts`) makes returning painless, but it +does not make it unnecessary. Most users with large histories never finish. + +This design adds an opt-in beta backend that continues a user's import while +their browser is closed. + +## Non-goals + +- **Making imports faster.** The 2,700/day per-user limit is imposed by Last.fm + and applies equally to a backend. A 100k import still takes ~37 days. The + feature buys *unattendedness*, not speed. +- **User accounts.** The Last.fm username is the only identity. +- **Replacing the client-side flow.** It remains the default for most users. + +## Scope + +Background mode is offered **only when the selected track count exceeds +`DAILY_LIMIT` (2,700)** — that is, only to users who would otherwise have to +return at least once. Smaller imports finish in one sitting and stay entirely +client-side. + +It is an **opt-in beta**. Users must actively choose it, and the UI must label it +as beta. + +## Architecture + +Three components. + +**1. The SPA** (unchanged deployment: FTP to Namecheap via `.github/workflows/cd.yml`) + +Parses the Spotify ZIP, deduplicates, and lets the user select tracks — all in +the browser, exactly as today. The Spotify ZIP never leaves the browser under +any circumstance. + +**2. API worker** (Cloudflare Worker, HTTP, at `api.savas.ca`) + +Handles the Last.fm auth callback, job creation, track-list upload, and status +queries. + +**3. Scheduler worker** (Cloudflare Worker, Cron Trigger, every minute) + +Selects due jobs, scrobbles a batch for each, commits progress. + +**Storage:** D1 for job rows, cursors, and encrypted session keys. R2 for +track-list blobs. + +### Why Cloudflare, and when to leave + +Cloudflare Workers' free tier costs nothing at idle and requires no ops, which +suits an unproven feature. Its constraints define a hard capacity ceiling: + +| Constraint | Value | Consequence | +| --- | --- | --- | +| Cron ticks/day | 1,440 (1/min) | — | +| Subrequests per invocation (free) | 50 | ~45 scrobbles/tick after D1/R2 overhead | +| **Global throughput** | **~65,000 scrobbles/day** | **~24 concurrent users** | + +Two things follow. First, ~24 concurrent jobs is the migration trigger: when +capacity is consistently saturated, move the worker to an Oracle Cloud Always +Free ARM VM (4 cores, 24GB RAM, 200GB disk, dedicated IP). + +Second, 65,000/day averages **0.75 requests/sec**, so the free tier makes it +impossible to breach Last.fm's documented limit of *5 requests/sec per +originating IP, averaged over 5 minutes*. Compliance holds by construction, which +defuses the main concern with Cloudflare's shared egress IPs. + +### Portability requirements + +The worker must be movable to a plain VM without a rewrite: + +- Plain `fetch` and WebCrypto only in the scrobbling core. Both exist in Workers + and Node 18+. No `node:` imports, no Workers-only APIs. +- Portable SQL only, so D1's SQLite can be swapped for Postgres. +- R2 access behind a minimal get/put-by-key interface, so it can become S3 or + the filesystem. +- **No Durable Objects for scheduling.** This is the principal lock-in trap. + Scheduling stays "SELECT due jobs → process batch → commit", which runs + identically under Workers Cron, a systemd timer, or pg_cron. + +## Authentication + +### The constraint + +Last.fm issues exactly one credential: the session key returned by +`auth.getSession`. It never expires, it is unscoped write access, and it can only +be revoked by the user at last.fm/settings/applications. There is no scoped or +delegated variant. + +The server must therefore hold a permanent write credential. The design cannot +avoid this; it can only minimise how long the credential is held and how much +damage its disclosure would cause. + +### Handoff via a second Last.fm round-trip + +The wizard authenticates at step 1, before the track count is known +(`Scrobblify.vue:28-36`). By the time we can offer background mode (step 3→4), +the browser already holds a session key. + +When the user opts in, the SPA redirects to Last.fm auth again with `cb=` +pointing at **the API worker** rather than the SPA. The worker exchanges the +fresh token for its own independent session key. + +**The client's session key is never transmitted anywhere.** Because the user has +already authorised the application, Last.fm does not re-prompt; the user sees a +redirect, not a second login. After successful handoff the SPA clears its own +session key from `localStorage` — the background job now owns the import. + +The worker then encrypts the session key, creates the job row, sets an httpOnly +cookie, and redirects back to the SPA, which uploads the selected track list as +gzipped NDJSON to R2. + +### The shared secret stays public + +`store.ts:12` hardcodes both the Last.fm API key and shared secret in the client +bundle. Routing *all* authentication through the worker would make the secret +genuinely secret, but it would put Cloudflare on the critical path for the +majority of users who never use background mode. + +This design keeps client-side auth as the default, so **the shared secret remains +public**. This is the existing accepted risk and is not made worse. The API key is +public regardless. + +### Sessions and identity + +- **httpOnly cookie**, scoped `Domain=savas.ca; SameSite=Lax; Secure`. Because + `savas.ca` and `api.savas.ca` share a registrable domain, this is a + first-party cookie and survives Safari's ITP and Firefox's Total Cookie + Protection. This is why the API must not live on `*.workers.dev`. + (`savas.ca` is already served by Cloudflare nameservers, so a Workers custom + domain is the only setup required.) +- **Fallback:** re-authenticate through Last.fm. The username resolves to the + job. Works on any device, from any browser, after any cookie loss. + + This exchange necessarily yields *another* session key. It is used only to + prove identity and **must be discarded immediately, never stored**. The job + continues to use the credential captured at handoff. Storing a second + credential per user would widen the blast radius for no benefit. + +No email address, no password, no PII beyond the Last.fm username. + +### Credential lifecycle + +Non-negotiable, because a free-tier account that is suspended or reclaimed would +otherwise strand other people's permanent write credentials on infrastructure we +do not control: + +- Encrypt the session key at rest with a key held as a Worker secret, never in + D1 alongside the ciphertext. +- **Delete it the instant** the job completes, fails permanently, is cancelled, + or reaches its TTL of 60 days. +- Delete the R2 blob at the same time. Retain only summary statistics. +- **Expose no generic Last.fm proxy.** The worker may only scrobble tracks + already committed to that job's immutable blob. A stolen cookie therefore + grants nothing beyond reading progress. + +## Progress and status + +`Scrobblify.vue` already checks `hasSavedState()` on mount and renders a "Resume +previous session?" alert. Background jobs reuse that pattern: a parallel +`GET /api/job` renders a banner for a live job, opening a status view in place of +step 4's local scrobbler. + +The status view shows completed/total, current state (running, waiting on the +daily limit, rate-limited, needs re-auth, failed), failed-track count, and an +**estimated completion date**. The estimate is essential: without "expect to +finish around 1 September", a user watching a 37-day job will conclude it is +broken and re-import. + +Controls: pause, cancel-and-delete-my-data, export progress (see Escape hatches), +and a link to last.fm/settings/applications. + +Polling is a fetch on load plus a 60-second poll while the tab is visible. The +100,000/day invocation budget is shared with the scheduler, and a job advancing +at 2,700/day does not warrant more. + +Completed job rows are retained (without the session key) for 30 days so a +returning user sees "done — 94,203 scrobbled, 112 failed" with a downloadable +failure list, rather than "no job found". + +## Scheduler + +**Fairness is round-robin, not FIFO.** Due jobs are selected ordered by +`last_run_at` ascending and each gets a slice of the tick's budget. FIFO would +let a single 100k import starve every job behind it for a month. + +**Per-user limits reuse the existing constants.** `BURST_LIMIT` (950, 10-minute +cooldown) and `DAILY_LIMIT` (2,700) currently live in `ScrobbleStep.vue:88-89`, +and `LastFm.isRateLimitError` / `isNetworkError` in `src/api/LastFm.ts`. These +move into a `shared/` module imported by both the SPA and the worker. Divergence +between client and server pacing would be a slow-to-surface bug. + +### Error handling + +| Condition | Response | +| --- | --- | +| Error 29 (rate limit) | Exponential backoff for the job **and** trip a global circuit breaker — on shared egress, a 29 may mean the whole IP is throttled. Log every occurrence with timestamp and active-job count. | +| **Error 9 (invalid session key)** | The user revoked access. Delete the credential immediately, mark the job `needs_reauth`, stop. **Never retry.** | +| Network error | Backoff and retry; the track was not consumed. | +| Per-track failure | Record and continue, matching current client behaviour. | +| 10 consecutive failures | Pause the job, mark `needs_attention`, surface on the status page. | + +Error 9 is new. The client never encounters it because its session key is created +seconds before use; a job running for weeks certainly will. + +The error-29 log is also the dataset that answers whether Last.fm's binding limit +is per-API-key or per-IP — currently unknown, and the deciding factor in whether +migrating to a dedicated IP would help at all. + +## Correctness under change + +Jobs run for weeks, so bugs **will** be fixed while jobs are in flight. The +design must make that safe. + +### Duplicate scrobbles: reconcile, don't prevent + +**The ambiguity window is irreducible.** Even committing the cursor after every +single track, the worker can crash between Last.fm accepting a scrobble and D1 +recording it. No cursor granularity eliminates this. + +Therefore the worker **reconciles against Last.fm** using the existing +`getAllScrobblesInRange`, fetching the user's actual recent scrobbles and +skipping any already present. The duplicate-detection logic in the upload step +already performs this comparison. + +An **unclean tick** is precisely: a job whose `locked_until` lease expired +without the tick having committed a final cursor. The next tick to pick up that +job reconciles before scrobbling anything. + +The **reconciliation range** runs from the timestamp of the earliest track in the +uncommitted cursor range to the present. Tracks found already on the user's +profile are marked complete and skipped; the cursor advances past them. + +Once reconciliation exists, batch size is purely a throughput tuning knob rather +than a safety mechanism. Last.fm's own deduplication is not relied upon. + +Re-tagged listens carry today's timestamp, so they fall inside the reconciliation +window like any other recent scrobble. + +### Safe-deploy mechanics + +- **Immutable job payload.** The R2 blob is written once at job creation and + never mutated. No code change can alter what an existing job will scrobble. +- **Lease-based locking.** Each job carries a `locked_until` timestamp. Two ticks + can never process one job concurrently, and a tick killed mid-batch by a deploy + simply has its lease expire; the next tick resumes from the last committed + cursor and reconciles. +- **Additive-only migrations.** Never drop or rename a column a running job + depends on. Job rows carry a `schema_version` so the worker can handle rows + created by older code. +- **Global kill switch.** A `paused` flag in D1, checked first on every tick. If + a bug is spotted, flip it, jobs freeze safely mid-flight, fix, unflip. +- **Batch audit log.** Every batch records job ID, cursor range, timestamp, and + outcome, so any incident can be diagnosed and its blast radius established. + +### Escape hatches + +Because this is a beta that may be withdrawn: + +- The status page can **export progress in the existing `StateManager` JSON + format**. A user whose job is cancelled — or whose beta access ends — imports + that file and resumes client-side through the existing flow. The format and + both code paths already exist (`StateManager.exportToFile` / + `importFromFile`), so this costs almost nothing and guarantees progress is + never stranded server-side. +- **Concurrency cap.** Background mode admits a bounded number of concurrent jobs + — initially **20**, deliberately below the ~24 capacity ceiling so that status + polling and retries retain headroom. When full, the UI offers the normal + client-side flow and reports that background mode is at capacity. + +## Beta framing + +- Opt-in only, presented at step 3→4 when the selection exceeds 2,700 tracks. +- Labelled clearly as beta in the UI. +- The opt-in must state plainly: the selected track list is uploaded to + Scrobblify's server; a Last.fm credential is stored until the import finishes; + access can be revoked at any time at last.fm/settings/applications. + +## Analytics + +Follow the conventions in `AGENTS.md` — use the helpers in +`src/services/Analytics.ts`, never `posthog` directly, and never widen an error +message to include raw request parameters. + +New events: `background_offered`, `background_opted_in`, `background_declined`, +`background_handoff_failed`, `background_job_created`, +`background_job_completed`, `background_job_cancelled`, `background_reauth_needed`, +`background_capacity_full`. + +New error contexts: `background.handoff`, `background.upload`, +`background.status`, `worker.scrobbleBatch`, `worker.reconcile`. + +## Open questions for implementation + +- Whether D1 and R2 binding calls count against the 50-subrequest limit. This + determines the achievable batch size and therefore the exact capacity ceiling. + It does not affect correctness, because reconciliation is required regardless. +- Wall-clock limits for scheduled Workers on the free tier, which bound how long + a single tick may pace its batch. From 08d98807d8f401edfa702ebcd683b45da0bdc385 Mon Sep 17 00:00:00 2001 From: Niko Savas Date: Sun, 26 Jul 2026 16:23:53 -0700 Subject: [PATCH 2/6] Fix timestamp assumption in background scrobbling spec; add measured baseline Reconciling against user-supplied timestamps does not work: reTagOldListens assigns one identical Date instance to every listen, so re-tagged imports carry a single timestamp across tens of thousands of tracks. A 37-day job also outlives Last.fm's 14-day window, so client-assigned timestamps expire mid-run regardless. The worker now assigns scrobble timestamps at send time and persists the index-to-timestamp mapping before sending, giving a unique idempotency key we control and narrowing reconciliation to one batch. Also records the PostHog baseline: median user completes 2.1% of their import, reactive rate limits outnumber preventive pauses ~9:1, and the 1-minute cooldown clears the limit in 33 of 2,135 cases. Fixing the client cooldown and thresholds should ship before this feature. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2c093209-9749-4d93-990f-c970f26d570e --- ...2026-07-26-background-scrobbling-design.md | 104 +++++++++++++++--- 1 file changed, 89 insertions(+), 15 deletions(-) diff --git a/docs/superpowers/specs/2026-07-26-background-scrobbling-design.md b/docs/superpowers/specs/2026-07-26-background-scrobbling-design.md index da1b323..0196d92 100644 --- a/docs/superpowers/specs/2026-07-26-background-scrobbling-design.md +++ b/docs/superpowers/specs/2026-07-26-background-scrobbling-design.md @@ -16,6 +16,48 @@ does not make it unnecessary. Most users with large histories never finish. This design adds an opt-in beta backend that continues a user's import while their browser is closed. +## Measured baseline (PostHog, 2026-07-09 to 2026-07-26) + +All figures filtered to `properties.app = 'scrobblify'`. + +| Metric | Value | +| --- | --- | +| People who started scrobbling | 63 | +| People who hit a Last.fm rate limit | 33 | +| Of those, people who completed | 7 | +| Median tracks selected (rate-limited users) | 34,769 | +| Median tracks actually scrobbled | 745 | +| **Median share of import completed** | **2.1%** | + +**The preventive pause is not working.** Reactive rate limits outnumber +preventive pauses roughly 9:1 — 2,135 `scrobble_rate_limited` events against 190 +`burst_limit` and 40 `daily_limit` pauses. + +`BURST_LIMIT` is set too high. The highest `burst_count` ever observed at the +moment of a rate limit is **849**, below the 950 threshold. The median is 187. + +**Half of all first rate limits occur at `burst_count = 0`** — before a single +successful scrobble in the session. Last.fm's limit is a rolling window on the +account, while `burstCount` and `dailyCount` are in-memory and reset on every +page load. Users who return after being throttled are immediately throttled +again. + +**The 1-minute cooldown almost never clears the limit.** Of 2,135 rate limits, +only 33 produced a `scrobble_rate_limit_recovered`. Where recovery did occur, the +median elapsed time was **130 minutes**. Median actual pause duration is exactly +the configured 60 seconds, so this is not timer throttling — the cooldown is +simply two orders of magnitude too short, and recovery happens by accident when a +user walks away. + +Two consequences for this design: + +1. The worker must not reuse the client's cooldown or threshold values. It needs + backoff on the order of hours, and limit counters persisted per user rather + than per session. +2. **Fixing the client's cooldown and thresholds is a cheaper, higher-value + change than this feature, and should ship first.** It benefits all users + immediately and de-risks the backend by establishing real limit values. + ## Non-goals - **Making imports faster.** The 2,700/day per-user limit is imposed by Last.fm @@ -223,30 +265,56 @@ migrating to a dedicated IP would help at all. Jobs run for weeks, so bugs **will** be fixed while jobs are in flight. The design must make that safe. -### Duplicate scrobbles: reconcile, don't prevent +### Timestamps are assigned by the worker, not the client + +Two facts make client-assigned timestamps unusable for a long-running job. + +**Re-tagged listens all share one timestamp.** `UploadStep.vue:173` creates a +single `reTagDate`, and `scrobblify.ts:263-270` assigns that same `Date` instance +to every listen. Users with more than two weeks of history — the overwhelming +majority, and the only users offered background mode — therefore submit tens of +thousands of scrobbles carrying an identical millisecond timestamp. + +**A 37-day job outlives Last.fm's 14-day window.** Any timestamp fixed at job +creation is rejected from roughly day 15 onward. + +Therefore the job blob stores artist, track, and album, plus the original listen +date only as metadata. **The worker assigns the actual scrobble timestamp at send +time**, spreading a batch across the preceding minute so every track in it is +unique. Tracks whose original timestamp still falls inside the 14-day window at +send time keep it; everything else is stamped to the present. + +A visible consequence worth stating in the beta copy: a background import spreads +re-tagged listens across the whole run rather than landing them all on one day. +This is more plausible than the current behaviour, not less. + +### Duplicate scrobbles: persist the timestamp, then reconcile on it **The ambiguity window is irreducible.** Even committing the cursor after every single track, the worker can crash between Last.fm accepting a scrobble and D1 recording it. No cursor granularity eliminates this. -Therefore the worker **reconciles against Last.fm** using the existing -`getAllScrobblesInRange`, fetching the user's actual recent scrobbles and -skipping any already present. The duplicate-detection logic in the upload step -already performs this comparison. +Reconciling against the *user's* timestamps cannot resolve it, for the reasons +above. Reconciling against *ours* can: -An **unclean tick** is precisely: a job whose `locked_until` lease expired -without the tick having committed a final cursor. The next tick to pick up that -job reconciles before scrobbling anything. +1. Before sending a batch, durably write its index→assigned-timestamp mapping to + D1. +2. Send the batch. +3. Commit the cursor. -The **reconciliation range** runs from the timestamp of the earliest track in the -uncommitted cursor range to the present. Tracks found already on the user's -profile are marked complete and skipped; the cursor advances past them. +Because the worker chose those timestamps and they are unique by construction, +the mapping is a stable idempotency key that survives a crash. On recovery, the +next tick fetches the narrow window covering the uncommitted mapping via +`getAllScrobblesInRange` and checks for those exact `(artist, track, timestamp)` +tuples. Present means done; absent means safe to send. Last.fm's own +deduplication is not relied upon. -Once reconciliation exists, batch size is purely a throughput tuning knob rather -than a safety mechanism. Last.fm's own deduplication is not relied upon. +An **unclean tick** is precisely: a job whose `locked_until` lease expired without +the tick having committed a final cursor. The next tick to pick up that job +reconciles before scrobbling anything. -Re-tagged listens carry today's timestamp, so they fall inside the reconciliation -window like any other recent scrobble. +Because the reconciliation window is one batch wide — under a minute — this costs +a single API call, not a paginated crawl of the user's history. ### Safe-deploy mechanics @@ -308,3 +376,9 @@ New error contexts: `background.handoff`, `background.upload`, It does not affect correctness, because reconciliation is required regardless. - Wall-clock limits for scheduled Workers on the free tier, which bound how long a single tick may pace its batch. +- **Whether Last.fm silently deduplicates identical `(artist, track, timestamp)` + submissions.** Because `reTagOldListens` currently assigns one identical + timestamp to every listen, a user who played the same track fifty times may be + credited with a single scrobble today. If confirmed, this is a pre-existing + data-loss bug in the client, independent of this feature, and the fix — unique + timestamps — is the same mechanism this design already requires. From 62f6727192b19aec14d98ab68216653977431ac1 Mon Sep 17 00:00:00 2001 From: Niko Savas Date: Sun, 26 Jul 2026 17:17:59 -0700 Subject: [PATCH 3/6] Add backwards compatibility and beta feedback to background scrobbling spec Background mode is now gated on tracks *remaining* rather than selected, and offered at four entry points: new import, resume from saved state, resume from file, and the mid-scrobble pause screen. The pause screen matters most, since telemetry shows that is where users abandon. Adds rules for handing off partial progress atomically, halting the client loop on handoff to avoid double-scrobbling, tolerating old state files, and falling back to the client-side flow. Beta feedback goes to niko@savas.ca via the prefilled-mailto pattern already in ErrorDialog.vue, including the job ID, and must stay reachable on failed jobs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2c093209-9749-4d93-990f-c970f26d570e --- ...2026-07-26-background-scrobbling-design.md | 97 ++++++++++++++++++- 1 file changed, 95 insertions(+), 2 deletions(-) diff --git a/docs/superpowers/specs/2026-07-26-background-scrobbling-design.md b/docs/superpowers/specs/2026-07-26-background-scrobbling-design.md index 0196d92..9da28c2 100644 --- a/docs/superpowers/specs/2026-07-26-background-scrobbling-design.md +++ b/docs/superpowers/specs/2026-07-26-background-scrobbling-design.md @@ -68,11 +68,15 @@ Two consequences for this design: ## Scope -Background mode is offered **only when the selected track count exceeds +Background mode is offered **only when the number of tracks *remaining* exceeds `DAILY_LIMIT` (2,700)** — that is, only to users who would otherwise have to return at least once. Smaller imports finish in one sitting and stay entirely client-side. +"Remaining" rather than "selected" is deliberate: users already part-way through +an import must be able to hand off what is left. See Backwards compatibility for +the four entry points where the offer appears. + It is an **opt-in beta**. Users must actively choose it, and the UI must label it as beta. @@ -347,14 +351,98 @@ Because this is a beta that may be withdrawn: polling and retries retain headroom. When full, the UI offers the normal client-side flow and reports that background mode is at capacity. +## Backwards compatibility + +The feature must not strand anyone already part-way through an import, and must +not break the existing client-side flow in either direction. + +### Entry points + +Background mode is offered wherever the *remaining* track count exceeds 2,700 — +not only on fresh imports: + +1. **New import**, at step 3→4 after selection. +2. **Resume from saved IndexedDB state**, via `restoreFromState` + (`Scrobblify.vue:154-173`), which already computes the remaining set. +3. **Import of a progress JSON file**, which flows through the same + `restoreFromState` path. +4. **Mid-scrobble, on the step 4 pause screen** — particularly when paused for + `daily_limit` or `rate_limit`. + +Entry point 4 matters most. Telemetry shows users abandon precisely at those +pauses, so that screen is where "let us finish this for you" belongs. The offer +must be available while paused, not only at the start of a run. + +### Handing off partial progress + +The job payload is the **remaining** tracks, exactly as `restoreFromState` +already derives them. Already-scrobbled tracks are never re-sent, so the +timestamps the client previously used are irrelevant to the job. + +Two rules make the transition safe: + +- **The handoff is atomic from the client's perspective.** Local state is cleared + only after the server has confirmed both job creation and blob storage. A + failure at any earlier point leaves local state untouched and the user + continues client-side, none the wiser. +- **The client must halt its scrobble loop on successful handoff.** If both the + browser tab and the worker hold the same remaining track list, both will + scrobble it. This is the single most likely source of duplicates in the whole + design, and it is entirely self-inflicted. + +### Tolerating old and new state files + +`StateManager.importFromFile` requires only `totalTracks`, `completedIndices`, +`failedIndices`, and `tracks`, defaulting everything else. Handoff must depend on +nothing beyond that set — in particular it must not require `userName`, which +older files lack. The Last.fm username comes from the auth handoff itself. + +Any field this feature adds to `ScrobbleState` **must be optional**, so that old +files still import into new clients and new files still import into older cached +clients. + +### Falling back to the client + +The export escape hatch must emit exactly the `ScrobbleState` shape +`importFromFile` accepts, so a user can always return to the client-side flow. +Ordering matters on cancellation: **generate and deliver the export before +deleting the R2 blob**, or the data needed to build it is already gone. + +### Conflict rules + +- A live background job and local saved state must never both be active for one + user. On detecting both, the UI presents the background job as authoritative + and offers to discard the local state. +- The client-side scrobble loop must refuse to start while a live background job + exists for the authenticated user. + ## Beta framing -- Opt-in only, presented at step 3→4 when the selection exceeds 2,700 tracks. +- Opt-in only, offered at any of the four entry points above when the remaining + selection exceeds 2,700 tracks. - Labelled clearly as beta in the UI. - The opt-in must state plainly: the selected track list is uploaded to Scrobblify's server; a Last.fm credential is stored until the import finishes; access can be revoked at any time at last.fm/settings/applications. +### Reporting bugs + +Because this is a beta running unattended for weeks, users need a direct channel +when something looks wrong — there is no support system, and a silently stalled +job is indistinguishable from a slow one. + +Both the opt-in dialog and the status view carry a feedback link to +**niko@savas.ca**, following the prefilled-mailto pattern already established in +`ErrorDialog.vue:52-68`: a fixed subject and a body pre-populated with context. + +The body should include the **job ID**, so a report is traceable to a specific +job without asking the user to describe their state. The job ID is an opaque +identifier and not personal data; analytics already identifies users by +`lastfm_username` regardless. + +The link must remain reachable on a *failed* or *stalled* job, not only a healthy +one. That is the case where it will actually be used. + ## Analytics Follow the conventions in `AGENTS.md` — use the helpers in @@ -366,6 +454,11 @@ New events: `background_offered`, `background_opted_in`, `background_declined`, `background_job_completed`, `background_job_cancelled`, `background_reauth_needed`, `background_capacity_full`. +`background_offered` and `background_opted_in` carry an `entry_point` property +(`new_import`, `resume_saved`, `resume_file`, `paused`) so the four entry points +can be compared. If the paused-screen offer is where users actually convert, that +is worth knowing early. + New error contexts: `background.handoff`, `background.upload`, `background.status`, `worker.scrobbleBatch`, `worker.reconcile`. From 2bfddbd09d9b32b68d3a0c64bba0284a4ea248ec Mon Sep 17 00:00:00 2001 From: Niko Savas Date: Sun, 26 Jul 2026 17:36:54 -0700 Subject: [PATCH 4/6] Revise background scrobbling spec after adversarial review Verified against Last.fm's published API docs and corrected the design: - track.scrobble batches up to 50 scrobbles per request. Rewrote the capacity section and withdrew the ~24-concurrent-user ceiling, which was computed without batching. - Error 29 is an IP-level limit; the per-user daily cap arrives as ignoredMessage code 5 on an HTTP 200. The client conflates the two, which explains why half of all first rate limits occur at burst_count = 0. Added error 26 as a global-halt condition. - savas.ca/lastwave shares the origin, so Domain=savas.ca gives no isolation. Moved to a __Host- cookie on api.savas.ca plus explicit CSRF tokens, and raised a dedicated origin as an open question. - Specified the auth handoff as a transaction: persist before redirect, single-use nonce bound to the expected username, digest-validated upload, finalize before clearing client state, reap abandoned jobs. - Replaced lease-based locking with fencing tokens, since an expired lease does not stop an in-flight tick from writing. - Withdrew the exactly-once claim in favour of an explicit at-least-once choice, and documented why reconciliation is imperfect. - Reframed timestamp reassignment as history rewriting, with an explicit ordering trade-off and a decision to go recent-first. - Fixed internal contradictions around threshold reuse and retention. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2c093209-9749-4d93-990f-c970f26d570e --- ...2026-07-26-background-scrobbling-design.md | 378 +++++++++++++----- 1 file changed, 288 insertions(+), 90 deletions(-) diff --git a/docs/superpowers/specs/2026-07-26-background-scrobbling-design.md b/docs/superpowers/specs/2026-07-26-background-scrobbling-design.md index 9da28c2..f70ad50 100644 --- a/docs/superpowers/specs/2026-07-26-background-scrobbling-design.md +++ b/docs/superpowers/specs/2026-07-26-background-scrobbling-design.md @@ -33,14 +33,15 @@ All figures filtered to `properties.app = 'scrobblify'`. preventive pauses roughly 9:1 — 2,135 `scrobble_rate_limited` events against 190 `burst_limit` and 40 `daily_limit` pauses. -`BURST_LIMIT` is set too high. The highest `burst_count` ever observed at the -moment of a rate limit is **849**, below the 950 threshold. The median is 187. +`BURST_LIMIT` never fires in practice. The highest `burst_count` ever observed at +the moment of a rate limit is **849**, below the 950 threshold. The median is 187. **Half of all first rate limits occur at `burst_count = 0`** — before a single -successful scrobble in the session. Last.fm's limit is a rolling window on the -account, while `burstCount` and `dailyCount` are in-memory and reset on every -page load. Users who return after being throttled are immediately throttled -again. +successful scrobble in the session. Two causes compound here. `burstCount` and +`dailyCount` are in-memory and reset on every page load, so users who return +after being throttled are immediately throttled again. More fundamentally, the +counters measure the wrong quantity: error 29 is an **IP-level** limit (see +Scheduler), which no per-session, per-user counter can predict. **The 1-minute cooldown almost never clears the limit.** Of 2,135 rate limits, only 33 produced a `scrobble_rate_limit_recovered`. Where recovery did occur, the @@ -69,9 +70,13 @@ Two consequences for this design: ## Scope Background mode is offered **only when the number of tracks *remaining* exceeds -`DAILY_LIMIT` (2,700)** — that is, only to users who would otherwise have to -return at least once. Smaller imports finish in one sitting and stay entirely -client-side. +2,700** — that is, only to users who would otherwise have to return at least +once. Smaller imports finish in one sitting and stay entirely client-side. + +2,700 here is an *eligibility* threshold chosen because it matches the client's +current `DAILY_LIMIT`, so the offer appears exactly when the client would have +forced a second visit. It is deliberately **not** reused as a server-side pacing +constant; see Scheduler for why those constants do not describe reality. "Remaining" rather than "selected" is deliberate: users already part-way through an import must be able to hand off what is left. See Backwards compatibility for @@ -105,22 +110,52 @@ track-list blobs. ### Why Cloudflare, and when to leave Cloudflare Workers' free tier costs nothing at idle and requires no ops, which -suits an unproven feature. Its constraints define a hard capacity ceiling: +suits an unproven feature. -| Constraint | Value | Consequence | -| --- | --- | --- | -| Cron ticks/day | 1,440 (1/min) | — | -| Subrequests per invocation (free) | 50 | ~45 scrobbles/tick after D1/R2 overhead | -| **Global throughput** | **~65,000 scrobbles/day** | **~24 concurrent users** | +**Scrobbles are sent in batches of 50.** `track.scrobble` accepts up to 50 +scrobbles per request via array notation (`artist[i]`, `track[i]`, +`timestamp[i]`). The client sends them one at a time; the worker must not. This +changes the capacity picture by up to fiftyfold, and Cloudflare stops being the +binding constraint. -Two things follow. First, ~24 concurrent jobs is the migration trigger: when -capacity is consistently saturated, move the worker to an Oracle Cloud Always -Free ARM VM (4 cores, 24GB RAM, 200GB disk, dedicated IP). +Signing gotcha: Last.fm requires signature parameters sorted by ASCII, so +`artist[10]` precedes `artist[1]`. JavaScript's default `Array.sort()` already +produces this order, so `LastFm.getMethodSignature` is correct as written — but +it is easy to "fix" into being wrong. -Second, 65,000/day averages **0.75 requests/sec**, so the free tier makes it -impossible to breach Last.fm's documented limit of *5 requests/sec per -originating IP, averaged over 5 minutes*. Compliance holds by construction, which -defuses the main concern with Cloudflare's shared egress IPs. +| Constraint | Value | +| --- | --- | +| Cron ticks/day | 1,440 (1/min) | +| Subrequests per invocation (free) | 50, **including D1 and R2 binding calls** | +| Free-tier CPU per invocation | 10ms (wall time awaiting `fetch` does not count) | +| Scrobbles per Last.fm request | 50 | + +With batching, the binding constraints become Last.fm's own limits and the D1 +write budget rather than subrequest count. The real ceiling must therefore be +derived from a proper model — requests, scrobbles, D1 writes, R2 reads, CPU, and +retries costed separately, at five-minute peaks rather than daily averages — +before a concurrency cap is chosen. **The previously stated "~24 concurrent +users" was computed without batching and is withdrawn.** + +Two constraints that do *not* relax: + +- **Last.fm's 5 requests/sec per originating IP**, averaged over 5 minutes. + Batching makes this far easier to satisfy per scrobble, but it must be enforced + as a global token bucket, not assumed from a daily average. A daily mean says + nothing about a five-minute peak, and Cloudflare's egress IPs are shared with + other tenants whose traffic we cannot see. +- **The per-user daily scrobble limit**, which is unchanged by batching. + +**Migration trigger:** move to an Oracle Cloud Always Free ARM VM (4 cores, 24GB +RAM, 200GB disk, dedicated IP) when either the D1 write budget or the shared +egress IP becomes the limiting factor. + +### Blob storage must be chunked + +A single gzipped NDJSON blob cannot be randomly accessed at a cursor. Fetching +and decompressing the whole thing every tick, for every job, will exhaust both +CPU and memory. Store **independently compressed chunks plus a manifest**, sized +so a tick reads exactly the chunk its cursor points into. ### Portability requirements @@ -160,12 +195,40 @@ fresh token for its own independent session key. **The client's session key is never transmitted anywhere.** Because the user has already authorised the application, Last.fm does not re-prompt; the user sees a -redirect, not a second login. After successful handoff the SPA clears its own -session key from `localStorage` — the background job now owns the import. - -The worker then encrypts the session key, creates the job row, sets an httpOnly -cookie, and redirects back to the SPA, which uploads the selected track list as -gzipped NDJSON to R2. +redirect, not a second login. + +#### The handoff is a transaction, and must be written as one + +A full-page redirect destroys all in-memory state. The user's selection lives +only in Vuex (`SelectStep.vue:504-511`) and is **not** persisted, so a naive +"redirect, come back, upload" flow loses the very data the job needs. Worse, the +ordering below is easy to get wrong in ways that either strand a credential or +lose an import. + +Required protocol: + +1. **Persist before leaving.** The SPA writes the selected track list to + `localStorage`/IndexedDB *before* redirecting, together with a + `pending_handoff` record holding a nonce, the expected Last.fm username, and a + SHA-256 digest of the serialised list. +2. **Bind the nonce.** The `cb=` URL carries a worker-signed, single-use, + short-TTL nonce. The worker rejects any callback whose nonce is unknown, + expired, or already consumed. +3. **Verify the account.** `auth.getSession` returns a username. If it does not + match the username bound to the nonce, **abort and discard the session key.** + Without this check a user logged into a second Last.fm account in another tab + — or an attacker who lands their own callback — has one account's history + written into another's. This is the single most damaging failure mode in the + design. +4. **Upload, then finalise.** The job is created in `pending_upload`. The SPA + uploads the blob; the worker verifies the digest matches the one committed in + step 1, then transitions the job to `active`. +5. **Clear last.** The SPA clears its own session key and cached list only after + the worker confirms `active`. Clearing first turns any upload failure into + total data loss. +6. **Reap abandoned handoffs.** Jobs stuck in `pending_upload`, and unconsumed + nonces, expire on a timer, deleting any captured credential. A user who closes + the tab mid-flow must not leave a permanent write credential behind. ### The shared secret stays public @@ -176,23 +239,38 @@ majority of users who never use background mode. This design keeps client-side auth as the default, so **the shared secret remains public**. This is the existing accepted risk and is not made worse. The API key is -public regardless. +public regardless. (See error 26 under Scheduler for the case in favour of a +separate server-only API application.) ### Sessions and identity -- **httpOnly cookie**, scoped `Domain=savas.ca; SameSite=Lax; Secure`. Because - `savas.ca` and `api.savas.ca` share a registrable domain, this is a - first-party cookie and survives Safari's ITP and Firefox's Total Cookie - Protection. This is why the API must not live on `*.workers.dev`. - (`savas.ca` is already served by Cloudflare nameservers, so a Workers custom - domain is the only setup required.) -- **Fallback:** re-authenticate through Last.fm. The username resolves to the - job. Works on any device, from any browser, after any cookie loss. - - This exchange necessarily yields *another* session key. It is used only to - prove identity and **must be discarded immediately, never stored**. The job - continues to use the credential captured at handoff. Storing a second - credential per user would widen the blast radius for no benefit. +**`savas.ca` is a shared origin.** LastWave is served from `savas.ca/lastwave` — +a path, not a subdomain. Any cookie scoped `Domain=savas.ca` is readable by +LastWave's JavaScript, and browser security has no way to distinguish the two +applications. The earlier claim of origin isolation was wrong. + +Accordingly: + +- **Use a `__Host-` cookie set on `api.savas.ca`.** `__Host-` forbids the + `Domain` attribute, so the cookie is locked to that exact host and is invisible + to anything served from `savas.ca`. Attributes: `Secure`, `Path=/`, + `SameSite=Lax`, `HttpOnly`. +- Because the cookie is host-locked, the SPA cannot rely on ambient credentials + for cross-site calls; status and control endpoints take an explicit + **bearer token issued to the SPA**, with CORS restricted to the exact origin. +- **All state-changing endpoints require an anti-CSRF token**, since `SameSite=Lax` + still permits top-level cross-site GET navigations. +- Longer term, moving Scrobblify to its own origin is the only way to get real + isolation from LastWave. Flagged as an open question because it affects the + existing site structure. + +**Fallback:** re-authenticate through Last.fm. The username resolves to the job. +Works on any device, from any browser, after any cookie loss. + +This exchange necessarily yields *another* session key. It is used only to prove +identity and **must be discarded immediately, never stored**. The job continues to +use the credential captured at handoff. Storing a second credential per user would +widen the blast radius for no benefit. No email address, no password, no PII beyond the Last.fm username. @@ -206,10 +284,17 @@ do not control: D1 alongside the ciphertext. - **Delete it the instant** the job completes, fails permanently, is cancelled, or reaches its TTL of 60 days. -- Delete the R2 blob at the same time. Retain only summary statistics. +- Delete the R2 blob at the same time. Retain summary statistics and the failed- + track list the completion page promises — nothing else. (An earlier draft said + "summary statistics only", which contradicted that promise.) - **Expose no generic Last.fm proxy.** The worker may only scrobble tracks - already committed to that job's immutable blob. A stolen cookie therefore - grants nothing beyond reading progress. + already committed to that job's immutable blob. + +**Honest statement of stolen-cookie impact.** A stolen session does not merely +leak progress: it can cancel a job, and — depending on which controls exist — pause +or restart one. It cannot scrobble arbitrary tracks, which is the property the +"no generic proxy" rule buys. The `__Host-` cookie plus CSRF tokens above exist +specifically because the shared `savas.ca` origin makes theft plausible. ## Progress and status @@ -241,35 +326,80 @@ failure list, rather than "no job found". `last_run_at` ascending and each gets a slice of the tick's budget. FIFO would let a single 100k import starve every job behind it for a month. -**Per-user limits reuse the existing constants.** `BURST_LIMIT` (950, 10-minute -cooldown) and `DAILY_LIMIT` (2,700) currently live in `ScrobbleStep.vue:88-89`, -and `LastFm.isRateLimitError` / `isNetworkError` in `src/api/LastFm.ts`. These -move into a `shared/` module imported by both the SPA and the worker. Divergence -between client and server pacing would be a slow-to-surface bug. +### Two different limits, previously conflated + +Last.fm's documentation makes a distinction the client does not: + +- **Error 29** is *"Rate limit exceeded — Your **IP** has made too many requests + in a short period."* It is an IP-level throttle, not a per-user one. +- **The per-user daily scrobble cap** is not an error at all. It arrives as a + *successful* HTTP response carrying `ignoredMessage` **code 5, "Daily scrobble + limit exceeded"**. + +The client treats error 29 as if it were a per-user scrobble limit, which is why +`BURST_LIMIT`/`DAILY_LIMIT` correlate so poorly with observed throttling and why +half of all first rate limits occur at `burst_count = 0`. An IP-level limit is +not something a per-session counter can predict. + +Consequences for the worker: + +- A global circuit breaker on error 29 is **correct**, because the limit really + is shared across every job on that egress IP. Per-job backoff alone would not + help. +- Per-user pacing must be driven by `ignoredMessage` code 5, not by error 29. +- **Do not port `BURST_LIMIT = 950` or `DAILY_LIMIT = 2,700` to the worker.** + Measured data shows they do not describe reality. Share + `LastFm.isRateLimitError`, `isNetworkError`, and the ignore-code taxonomy via a + `shared/` module; derive pacing from observed responses instead of guessed + constants. + +### A 200 response does not mean the scrobble was stored + +`track.scrobble` reports per-track rejections in `ignoredMessage` while returning +HTTP success. Parsing it is a **correctness invariant**, not an optimisation: the +cursor may only advance for entries Last.fm actually accepted. + +The relevant codes are 1 (artist ignored), 2 (track ignored), 3 (timestamp too +old), 4 (timestamp too new), and 5 (daily limit). Code 3 and 4 indicate the +worker's own timestamp assignment is wrong and should be treated as a bug signal, +not a per-track failure. Code 5 means stop scrobbling for that user today. + +Last.fm may also **correct** artist/album/track names, flagged by `corrected`. +Reconciliation must compare against corrected names or it will not find its own +writes. ### Error handling | Condition | Response | | --- | --- | -| Error 29 (rate limit) | Exponential backoff for the job **and** trip a global circuit breaker — on shared egress, a 29 may mean the whole IP is throttled. Log every occurrence with timestamp and active-job count. | -| **Error 9 (invalid session key)** | The user revoked access. Delete the credential immediately, mark the job `needs_reauth`, stop. **Never retry.** | -| Network error | Backoff and retry; the track was not consumed. | -| Per-track failure | Record and continue, matching current client behaviour. | +| Error 29 (IP rate limit) | Trip the **global** circuit breaker; back off all jobs. Log with timestamp and active-job count. | +| `ignoredMessage` code 5 | Per-user daily cap reached. Stop that job until tomorrow; do not treat as failure. | +| `ignoredMessage` code 3/4 | Worker timestamp assignment is wrong. Alert; do not silently drop. | +| `ignoredMessage` code 1/2 | Last.fm rejected the artist/track. Record as permanently failed; never retry. | +| **Error 9 (invalid session key)** | Credential revoked. Delete it, mark `needs_reauth`, stop. **Never retry.** | +| **Error 26 (suspended API key)** | Every job is dead until resolved. Halt globally and alert; retrying makes it worse. | +| Network error | Backoff and retry; nothing was consumed. | | 10 consecutive failures | Pause the job, mark `needs_attention`, surface on the status page. | -Error 9 is new. The client never encounters it because its session key is created -seconds before use; a job running for weeks certainly will. +Error 9 is not exclusive to long-running jobs — the client persists session keys +in `localStorage` indefinitely and can hit revocation too — but the worker must +handle it without a user present to re-authenticate. -The error-29 log is also the dataset that answers whether Last.fm's binding limit -is per-API-key or per-IP — currently unknown, and the deciding factor in whether -migrating to a dedicated IP would help at all. +Error 26 is a single point of failure worth stating plainly: the API key is +shared with the public client bundle, so anyone can abuse it and get it +suspended, killing every background job at once. A **separate, server-only +Last.fm API application** would isolate that risk, at the cost of an honest +second authorisation prompt. ## Correctness under change Jobs run for weeks, so bugs **will** be fixed while jobs are in flight. The design must make that safe. -### Timestamps are assigned by the worker, not the client +### Timestamp reassignment rewrites history — say so plainly + +This is a product decision, not an implementation detail, and it deserves an +honest statement rather than a reassuring one. Two facts make client-assigned timestamps unusable for a long-running job. @@ -277,29 +407,49 @@ Two facts make client-assigned timestamps unusable for a long-running job. single `reTagDate`, and `scrobblify.ts:263-270` assigns that same `Date` instance to every listen. Users with more than two weeks of history — the overwhelming majority, and the only users offered background mode — therefore submit tens of -thousands of scrobbles carrying an identical millisecond timestamp. +thousands of scrobbles carrying an identical millisecond timestamp. (A fix for +this is in flight client-side; the worker must not depend on either version.) **A 37-day job outlives Last.fm's 14-day window.** Any timestamp fixed at job creation is rejected from roughly day 15 onward. Therefore the job blob stores artist, track, and album, plus the original listen date only as metadata. **The worker assigns the actual scrobble timestamp at send -time**, spreading a batch across the preceding minute so every track in it is -unique. Tracks whose original timestamp still falls inside the 14-day window at -send time keep it; everything else is stamped to the present. +time.** Tracks whose original timestamp still falls inside the 14-day window at +send time keep it; everything else is stamped near the present, spread so that +every timestamp in a batch is unique. + +**What this costs the user.** Their Last.fm history will show these plays at the +time the import ran, not when they listened. Play counts and top-artist charts +are correct; the listening *timeline* is fiction. Anyone who values that timeline +should not use background mode — the foreground path has the same problem for +re-tagged listens, but over hours rather than weeks. + +**Ordering is a genuine trade-off, and must be chosen explicitly:** + +| Order | Effect | +| --- | --- | +| Oldest-first | Preserves relative chronology, but tracks near the start of the queue age out of the 14-day window and get restamped anyway. | +| Recent-first | Maximises the number of tracks keeping their true timestamp; reverses chronology for everything else. | -A visible consequence worth stating in the beta copy: a background import spreads -re-tagged listens across the whole run rather than landing them all on one day. -This is more plausible than the current behaviour, not less. +**Decision: recent-first**, because it preserves the most genuine data, and +because the alternative's chronology is itself an artefact of restamping. This +must be stated in the opt-in copy, not buried. -### Duplicate scrobbles: persist the timestamp, then reconcile on it +### Duplicate scrobbles: at-least-once, with a narrow reconciliation **The ambiguity window is irreducible.** Even committing the cursor after every single track, the worker can crash between Last.fm accepting a scrobble and D1 -recording it. No cursor granularity eliminates this. +recording it. No cursor granularity eliminates this. **This design does not +achieve exactly-once**, and should not claim to. -Reconciling against the *user's* timestamps cannot resolve it, for the reasons -above. Reconciling against *ours* can: +The choice is between at-least-once (risk duplicates) and at-most-once (risk +silently dropped tracks). **Choose at-least-once**: a duplicate is visible and +removable by the user; a missing track is invisible and unrecoverable. + +Reconciliation narrows the window but does not close it. Reconciling against the +*user's* timestamps cannot work at all, for the reasons above. Reconciling against +*ours* can: 1. Before sending a batch, durably write its index→assigned-timestamp mapping to D1. @@ -309,13 +459,23 @@ above. Reconciling against *ours* can: Because the worker chose those timestamps and they are unique by construction, the mapping is a stable idempotency key that survives a crash. On recovery, the next tick fetches the narrow window covering the uncommitted mapping via -`getAllScrobblesInRange` and checks for those exact `(artist, track, timestamp)` -tuples. Present means done; absent means safe to send. Last.fm's own -deduplication is not relied upon. +`getAllScrobblesInRange` and looks for those exact tuples. + +Known limits of that check, which is why the guarantee is at-least-once: + +- Last.fm **normalises and corrects** artist/track names, so comparison must be + fuzzy or must use the `corrected` names returned by the scrobble call itself. +- `from`/`to` on `user.getRecentTracks` are strictly exclusive; the window must be + widened by a second on each side. +- There is **no read-after-write guarantee**; a scrobble accepted moments ago may + not yet appear. +- `user.getRecentTracks` caps `limit` at **200**. Note `LastFm.ts:168` currently + sets `PAGE_SIZE = 1000`, which is invalid — a pre-existing bug to fix if this + code is shared. -An **unclean tick** is precisely: a job whose `locked_until` lease expired without -the tick having committed a final cursor. The next tick to pick up that job -reconciles before scrobbling anything. +An **unclean tick** is precisely: a job whose lease expired without the tick +having committed a final cursor. The next tick to pick up that job reconciles +before scrobbling anything. Because the reconciliation window is one batch wide — under a minute — this costs a single API call, not a paginated crawl of the user's history. @@ -324,10 +484,22 @@ a single API call, not a paginated crawl of the user's history. - **Immutable job payload.** The R2 blob is written once at job creation and never mutated. No code change can alter what an existing job will scrobble. -- **Lease-based locking.** Each job carries a `locked_until` timestamp. Two ticks - can never process one job concurrently, and a tick killed mid-batch by a deploy - simply has its lease expire; the next tick resumes from the last committed - cursor and reconciles. +- **Fencing tokens, not just leases.** A `locked_until` timestamp alone does + **not** prevent concurrent execution: a tick that stalls past its lease keeps + running, and its in-flight writes can land after a second tick has claimed the + job. Leases are a liveness hint, not mutual exclusion. + + Each job therefore carries a monotonically increasing `generation`. Acquiring a + job is a single atomic compare-and-set that bumps it + (`UPDATE ... SET generation = generation + 1, locked_until = ? WHERE id = ? AND + locked_until < now`). Every subsequent write by that tick — timestamp mapping, + cursor commit, audit row — carries its generation and is conditioned on it still + being current. A superseded tick's writes are rejected rather than silently + interleaved. + + This is also what makes a deploy safe mid-batch: the killed tick's lease + expires, the next tick bumps the generation, and any zombie writes are fenced + out. - **Additive-only migrations.** Never drop or rename a column a running job depends on. Job rows carry a `schema_version` so the worker can handle rows created by older code. @@ -464,14 +636,40 @@ New error contexts: `background.handoff`, `background.upload`, ## Open questions for implementation -- Whether D1 and R2 binding calls count against the 50-subrequest limit. This - determines the achievable batch size and therefore the exact capacity ceiling. - It does not affect correctness, because reconciliation is required regardless. -- Wall-clock limits for scheduled Workers on the free tier, which bound how long - a single tick may pace its batch. +**Blocking — these change the design:** + +- **Should Scrobblify move to its own origin?** LastWave shares + `https://savas.ca`, so cookie-level isolation between the two is impossible. + `__Host-` cookies on `api.savas.ca` mitigate this; a dedicated origin fixes it. + Affects existing site structure, so it is the user's call. +- **A separate, server-only Last.fm API application?** The public API key can be + abused by anyone into an error-26 suspension that kills every background job. + Isolating it costs an extra, honestly-explained authorisation prompt. +- **D1 write budget on the free tier.** The whole scheduler design assumes cheap + per-batch writes (mapping + cursor + audit row, fenced by generation). Measure + the actual quota before committing to a tick cadence; if writes are the scarce + resource, batch size and audit granularity must be tuned to it, not to + subrequest count. + +**Non-blocking:** + +- Wall-clock and the 10ms CPU limit for scheduled Workers on the free tier, which + bound how much pacing and decompression a single tick may do. - **Whether Last.fm silently deduplicates identical `(artist, track, timestamp)` - submissions.** Because `reTagOldListens` currently assigns one identical - timestamp to every listen, a user who played the same track fifty times may be - credited with a single scrobble today. If confirmed, this is a pre-existing - data-loss bug in the client, independent of this feature, and the fix — unique + submissions.** Because `reTagOldListens` historically assigned one identical + timestamp to every listen, a user who played the same track fifty times may + have been credited with a single scrobble. If confirmed, this is a pre-existing + client data-loss bug independent of this feature, and the fix — unique timestamps — is the same mechanism this design already requires. + +## Review status + +Reviewed adversarially on 2026-07-26. Findings incorporated: batched scrobbling, +the error-29-vs-ignore-code-5 distinction, the shared-origin cookie flaw, the +handoff transaction protocol with username binding, fencing tokens, chunked blob +storage, the withdrawal of the exactly-once and capacity claims, and honest +framing of timestamp reassignment. + +Claims verified against Last.fm's published API documentation rather than +assumed: the 50-scrobble batch limit, the 200-item `getRecentTracks` cap, the +ASCII signature ordering rule, and the wording of error codes 9, 26, and 29. From c7df0190cb565a5bd064f50c0d09ff6113287ac0 Mon Sep 17 00:00:00 2001 From: Niko Savas Date: Sun, 26 Jul 2026 17:45:38 -0700 Subject: [PATCH 5/6] Correct spec after second adversarial review round Round two found that several round-one fixes were themselves wrong: - Recent-first ordering was backwards. A track played today has 14 days of slack before its timestamp expires; one played 13 days ago has one. Sending recent tracks first spends slack on the tracks needing it least. Now earliest-deadline-first over two queues. - The __Host- cookie plus bearer token did not isolate LastWave. Cookies attach by destination, not initiator, so a same-site fetch from savas.ca carries the cookie anyway; and a token the SPA can read LastWave can also read. Replaced with an explicit trust-boundary decision rather than ineffective crypto. - The handoff never committed the digest server-side that step 4 claimed to verify. Added a preflight step and a compare-and-set state machine covering seven previously unenumerated ambiguous states. - A single cursor cannot represent a 50-entry batch containing accepted, permanently failed, rate-limited and unknown entries. Progress is now per-entry, with the cursor advancing only over a terminal prefix. - Fencing tokens protect D1 but cannot revoke an in-flight Last.fm request; documented as part of the at-least-once window. - An immutable payload does not make behaviour immutable. Jobs now pin an algorithm version for parsing, ordering and timestamps. - The concurrency cap still cited the withdrawn capacity ceiling. It is now blocked on a feasibility spike, which is a design gate. Also added chunk upload limits against compression bombs, admission control for jobs too large to finish within the credential TTL, and a correction to the claim that listening history is not personal data. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2c093209-9749-4d93-990f-c970f26d570e --- ...2026-07-26-background-scrobbling-design.md | 312 ++++++++++++++---- 1 file changed, 245 insertions(+), 67 deletions(-) diff --git a/docs/superpowers/specs/2026-07-26-background-scrobbling-design.md b/docs/superpowers/specs/2026-07-26-background-scrobbling-design.md index f70ad50..628aa6b 100644 --- a/docs/superpowers/specs/2026-07-26-background-scrobbling-design.md +++ b/docs/superpowers/specs/2026-07-26-background-scrobbling-design.md @@ -157,6 +157,27 @@ and decompressing the whole thing every tick, for every job, will exhaust both CPU and memory. Store **independently compressed chunks plus a manifest**, sized so a tick reads exactly the chunk its cursor points into. +Chunking solves random access and introduces its own requirements. An +authenticated user uploads arbitrary compressed bytes, so: + +- **Bound both compressed and uncompressed chunk size**, and enforce field length + and per-chunk entry count limits. Otherwise a small compression bomb exhausts + Worker memory at *scheduling* time, taking down every job on that tick rather + than just the attacker's. +- **Hash every chunk** and validate on upload; write chunks with write-once + semantics so a chunk cannot be swapped after validation. +- **Publish the manifest last**, and make the `active` transition conditional on + every chunk being present, hash-valid, and non-overlapping. Missing, duplicated, + or reordered chunks must be detectable from the manifest alone. + +### Listening history is personal data + +An earlier framing treated "no email, no password" as meaning there is no PII +here. That is wrong as a threat model. A user's complete listening history, +bound to their Last.fm username, is personal data — and the retained failed-track +list is a subset of it. It should be minimised, deleted on the same schedule as +the credential, and never logged into analytics. + ### Portability requirements The worker must be movable to a plain VM without a rewrite: @@ -205,27 +226,53 @@ only in Vuex (`SelectStep.vue:504-511`) and is **not** persisted, so a naive ordering below is easy to get wrong in ways that either strand a credential or lose an import. -Required protocol: - -1. **Persist before leaving.** The SPA writes the selected track list to - `localStorage`/IndexedDB *before* redirecting, together with a - `pending_handoff` record holding a nonce, the expected Last.fm username, and a - SHA-256 digest of the serialised list. -2. **Bind the nonce.** The `cb=` URL carries a worker-signed, single-use, - short-TTL nonce. The worker rejects any callback whose nonce is unknown, - expired, or already consumed. +Required protocol. Note step 0 — an earlier draft had the SPA commit the digest +only to `localStorage`, then had the worker "verify against the digest committed +in step 1", which it had never been told. The nonce has the same problem: a +worker-signed value cannot originate on the client. + +0. **Server preflight.** Before redirecting, the SPA calls the worker with the + expected Last.fm username, the payload's SHA-256 digest, the track count, and + the chunk manifest bounds. The worker durably records a `handoff` row in state + `issued` and returns a signed, single-use, short-TTL opaque state value. This + is what makes later verification possible at all. +1. **Persist before leaving.** *Then* the SPA writes the selected track list to + IndexedDB, because a full-page redirect destroys Vuex. +2. **Bind the callback.** The `cb=` URL carries the signed state from step 0. The + worker rejects any callback whose state is unknown, expired, already consumed, + or whose signature fails. 3. **Verify the account.** `auth.getSession` returns a username. If it does not - match the username bound to the nonce, **abort and discard the session key.** + match the username recorded in step 0, **abort and discard the session key.** Without this check a user logged into a second Last.fm account in another tab — or an attacker who lands their own callback — has one account's history written into another's. This is the single most damaging failure mode in the design. -4. **Upload, then finalise.** The job is created in `pending_upload`. The SPA - uploads the blob; the worker verifies the digest matches the one committed in - step 1, then transitions the job to `active`. +4. **Upload, then finalise.** The job sits in `pending_upload`. The SPA uploads + chunks; the worker validates each chunk's hash, publishes the manifest last, + and only then transitions to `active`. 5. **Clear last.** The SPA clears its own session key and cached list only after the worker confirms `active`. Clearing first turns any upload failure into total data loss. + +#### States, because the happy path is the easy part + +The transitions are `issued → exchanging → pending_upload → finalizing → active`, +every one an atomic compare-and-set from the expected prior state. Callback and +finalise must be **idempotent**, because these cases are all reachable: + +| Situation | Required behaviour | +| --- | --- | +| Duplicate callbacks race for one Last.fm token | CAS `issued → exchanging`; the loser returns the winner's outcome, never a second `auth.getSession` | +| State claimed, then `auth.getSession` times out | Bounded retry from `exchanging`; on give-up, fail the handoff and discard any key | +| Session obtained, job row creation fails | The credential must be written in the same transaction as the row that owns it, or it is unreachable garbage holding write access | +| Finalise commits `active`, response lost | **Most dangerous case.** The client must not assume failure; it queries handoff status and resumes locally only if the server says the job is not active | +| Reaper fires during an active upload | Reaping is a CAS from the expected state with a freshness check, never an unconditional delete | +| Two tabs start handoffs for one username | One live handoff per username; the second is refused with a pointer to the first | +| Capacity fills between authorisation and activation | Reserve the slot at step 0, not at activation, and release it if the handoff is reaped | + +The general rule: **on any uncertain outcome the client stays disabled and asks +the server**, rather than resuming a local scrobble run that may now be racing a +live background job. 6. **Reap abandoned handoffs.** Jobs stuck in `pending_upload`, and unconsumed nonces, expire on a timer, deleting any captured credential. A user who closes the tab mid-flow must not leave a permanent write credential behind. @@ -244,25 +291,46 @@ separate server-only API application.) ### Sessions and identity -**`savas.ca` is a shared origin.** LastWave is served from `savas.ca/lastwave` — -a path, not a subdomain. Any cookie scoped `Domain=savas.ca` is readable by -LastWave's JavaScript, and browser security has no way to distinguish the two -applications. The earlier claim of origin isolation was wrong. - -Accordingly: - -- **Use a `__Host-` cookie set on `api.savas.ca`.** `__Host-` forbids the - `Domain` attribute, so the cookie is locked to that exact host and is invisible - to anything served from `savas.ca`. Attributes: `Secure`, `Path=/`, - `SameSite=Lax`, `HttpOnly`. -- Because the cookie is host-locked, the SPA cannot rely on ambient credentials - for cross-site calls; status and control endpoints take an explicit - **bearer token issued to the SPA**, with CORS restricted to the exact origin. -- **All state-changing endpoints require an anti-CSRF token**, since `SameSite=Lax` - still permits top-level cross-site GET navigations. -- Longer term, moving Scrobblify to its own origin is the only way to get real - isolation from LastWave. Flagged as an open question because it affects the - existing site structure. +**`savas.ca` is a shared origin, and no credential scheme can change that.** +LastWave is served from `savas.ca/lastwave` — a path, not a subdomain. Scrobblify +and LastWave are the *same web origin*, so the browser has no mechanism that +distinguishes them. + +A previous draft tried to engineer around this with a `__Host-` cookie plus an +SPA-held bearer token. That does not work, and stating why matters more than the +mitigation did: + +- A `__Host-` cookie on `api.savas.ca` cannot be *read* by `savas.ca` scripts — + but cookies are attached by request **destination**, not by initiator. Since + `savas.ca` and `api.savas.ca` are same-site, a credentialed `fetch` from + LastWave's JavaScript to the API carries the cookie anyway. +- A bearer token the SPA can read is, by definition, one LastWave can read: same + origin, same `localStorage`. +- CORS restricted "to the exact origin" admits both applications, because it *is* + the same origin. +- Anti-CSRF tokens defend against *cross-site* requests. LastWave is not + cross-site. + +So the honest choice is binary: + +| Option | Consequence | +| --- | --- | +| **Dedicated origin** for Scrobblify | Real isolation. `__Host-` cookie on `api.savas.ca` then works as intended. Requires restructuring the existing site. | +| **Accept a shared trust boundary** | Every script served from `savas.ca` — including LastWave and anything it loads — is inside Scrobblify's security perimeter. An XSS in LastWave is an XSS in Scrobblify. | + +**Interim decision: accept the shared trust boundary, and document it.** Both +applications are written and deployed by the same author, so this is a defensible +position rather than a third-party exposure — but it must be a *stated* position, +not an accident, and it means no third-party scripts may be added to `savas.ca` +without reconsidering it. + +Even under that acceptance, keep the mechanisms that defend against everything +*outside* the origin: `__Host-` cookie on `api.savas.ca` (`Secure`, `Path=/`, +`SameSite=Lax`, `HttpOnly`), exact-origin CORS, and CSRF tokens on state-changing +endpoints. They are worth having; they are simply not isolation from LastWave. + +Moving to a dedicated origin remains the recommended long-term fix and is a +blocking open question. **Fallback:** re-authenticate through Last.fm. The username resolves to the job. Works on any device, from any browser, after any cookie loss. @@ -368,12 +436,30 @@ Last.fm may also **correct** artist/album/track names, flagged by `corrected`. Reconciliation must compare against corrected names or it will not find its own writes. +### A batch of 50 has 50 outcomes, and a cursor has one + +Batching breaks the assumption that progress is a single monotonic index. One +response can simultaneously contain accepted entries, permanent code 1/2 +rejections, code 5 entries that must be retried later, and — if the response is +lost — entries whose fate is unknown. Code 5 can begin part-way through a batch, +so the failures are not even a suffix. + +A single `cursor` integer cannot represent that, and advancing it past a mixed +batch silently drops tracks. + +**Therefore:** persist a per-entry outcome (an outcome bitmap or a status column +keyed by index) for every in-flight batch. The cursor advances only across a +**contiguous prefix** of entries that are terminal — accepted or permanently +failed. Unknown and code-5 entries stay behind and are retried or reconciled +individually. Progress counts and the audit log are derived from the per-entry +outcomes, not from the cursor. + ### Error handling | Condition | Response | | --- | --- | | Error 29 (IP rate limit) | Trip the **global** circuit breaker; back off all jobs. Log with timestamp and active-job count. | -| `ignoredMessage` code 5 | Per-user daily cap reached. Stop that job until tomorrow; do not treat as failure. | +| `ignoredMessage` code 5 | Per-user daily cap reached. Stop that job and back off (see below); do not treat as failure. | | `ignoredMessage` code 3/4 | Worker timestamp assignment is wrong. Alert; do not silently drop. | | `ignoredMessage` code 1/2 | Last.fm rejected the artist/track. Record as permanently failed; never retry. | | **Error 9 (invalid session key)** | Credential revoked. Delete it, mark `needs_reauth`, stop. **Never retry.** | @@ -391,6 +477,20 @@ suspended, killing every background job at once. A **separate, server-only Last.fm API application** would isolate that risk, at the cost of an honest second authorisation prompt. +### "Wait until tomorrow" is an assumption, not a fact + +Code 5 proves the daily cap was reached. It says nothing about *when* it resets: +whether at midnight in some timezone, or on a rolling 24-hour window. Encoding +"resume at midnight" into the scheduler risks waking every job at once and +immediately re-hitting the cap — while also making the thundering herd worse for +the shared egress IP. + +**Until measured:** record the timestamps of accepted sends, wait a conservative +24 hours from the *first* accepted send of the capped period, and probe with a +single small batch before resuming full rate. Back off exponentially if the probe +also returns code 5. The observed reset behaviour is a metric worth capturing in +the beta precisely because it is currently unknown. + ## Correctness under change Jobs run for weeks, so bugs **will** be fixed while jobs are in flight. The @@ -420,20 +520,35 @@ send time keep it; everything else is stamped near the present, spread so that every timestamp in a batch is unique. **What this costs the user.** Their Last.fm history will show these plays at the -time the import ran, not when they listened. Play counts and top-artist charts -are correct; the listening *timeline* is fiction. Anyone who values that timeline -should not use background mode — the foreground path has the same problem for -re-tagged listens, but over hours rather than weeks. - -**Ordering is a genuine trade-off, and must be chosen explicitly:** +time the import ran, not when they listened. **Lifetime play counts stay correct** +(modulo the duplicates at-least-once permits), but *time-bounded* charts — +weekly, monthly, yearly — are badly distorted, because the entire import lands in +whatever weeks the job happened to run. The listening timeline is fiction. Anyone +who values that timeline should not use background mode; the foreground path has +the same problem for re-tagged listens, but over hours rather than weeks. + +**Ordering: send by earliest expiry deadline, not by recency.** + +An earlier draft chose recent-first "to maximise preserved timestamps". That is +backwards. A track played today has ~14 days of slack before its timestamp +becomes unscrobbleable; a track played 13 days ago has one day. Sending recent +tracks first spends the slack of the tracks that need it least and lets the +nearly-expired ones expire. This is ordinary earliest-deadline-first scheduling, +and recency is the wrong key. + +Two queues, re-evaluated at send time because deadlines move as the job runs: + +| Queue | Order | Rationale | +| --- | --- | --- | +| Still inside the 14-day window | Earliest expiry first (≈ oldest valid first) | Maximises the number of tracks that keep their true timestamp | +| Already outside the window | Chronological, so relative order survives | Nothing left to preserve; only ordering is a choice | + +**Decision: deadline-first.** For the users this feature targets — large, +re-tagged, multi-year histories — nearly every track is already outside the +window at job creation, so the first queue is usually small. That does not make +the policy unimportant: it is exactly the users with *some* recent listening who +would notice their genuine timestamps being thrown away needlessly. -| Order | Effect | -| --- | --- | -| Oldest-first | Preserves relative chronology, but tracks near the start of the queue age out of the 14-day window and get restamped anyway. | -| Recent-first | Maximises the number of tracks keeping their true timestamp; reverses chronology for everything else. | - -**Decision: recent-first**, because it preserves the most genuine data, and -because the alternative's chronology is itself an artefact of restamping. This must be stated in the opt-in copy, not buried. ### Duplicate scrobbles: at-least-once, with a narrow reconciliation @@ -482,8 +597,16 @@ a single API call, not a paginated crawl of the user's history. ### Safe-deploy mechanics -- **Immutable job payload.** The R2 blob is written once at job creation and - never mutated. No code change can alter what an existing job will scrobble. +- **Immutable job payload — but bytes are not behaviour.** The R2 chunks are + written once at job creation and never mutated. That prevents the *data* from + changing; it does **not** prevent new code from parsing, normalising, ordering, + or timestamping the same bytes differently. Immutability alone is not a + deploy-safety guarantee. + + Each job therefore pins an **algorithm version** alongside its manifest + version, covering parsing, ordering policy, and timestamp assignment. A worker + that encounters a job pinned to semantics it no longer implements must refuse + it and flag it, not reinterpret it. - **Fencing tokens, not just leases.** A `locked_until` timestamp alone does **not** prevent concurrent execution: a tick that stalls past its lease keeps running, and its in-flight writes can land after a second tick has claimed the @@ -500,6 +623,16 @@ a single API call, not a paginated crawl of the user's history. This is also what makes a deploy safe mid-batch: the killed tick's lease expires, the next tick bumps the generation, and any zombie writes are fenced out. + + **Fencing protects D1, not Last.fm.** A superseded tick's *external* request + cannot be revoked: worker A sends a batch, its lease expires in flight, worker + B takes over and reconciles before A's scrobbles are visible, B resends, then + A's request lands too. Fencing rejects A's database writes but not its + scrobbles. Mitigate by having a taking-over tick wait at least the maximum + outbound request timeout plus a read-visibility grace period before + reconciling — and accept that this is exactly the duplicate window the + at-least-once decision already acknowledges. It is a reason that decision has to + be explicit rather than a claim to have eliminated duplicates. - **Additive-only migrations.** Never drop or rename a column a running job depends on. Job rows carry a `schema_version` so the worker can handle rows created by older code. @@ -518,10 +651,24 @@ Because this is a beta that may be withdrawn: both code paths already exist (`StateManager.exportToFile` / `importFromFile`), so this costs almost nothing and guarantees progress is never stranded server-side. -- **Concurrency cap.** Background mode admits a bounded number of concurrent jobs - — initially **20**, deliberately below the ~24 capacity ceiling so that status - polling and retries retain headroom. When full, the UI offers the normal - client-side flow and reports that background mode is at capacity. +- **Concurrency cap.** Background mode admits a bounded number of concurrent jobs. + **The initial number cannot be chosen yet** — the earlier "20, below the ~24 + ceiling" was derived from the withdrawn capacity model and is void. It must come + out of the feasibility spike (see Open questions). When full, the UI offers the + normal client-side flow and reports that background mode is at capacity. + + Define explicitly *which states consume a slot*. A job that is `needs_reauth`, + `needs_attention`, or paused must not squat a scarce slot for 60 days: attach an + inactivity deadline after which it is cancelled, its credential deleted, and the + user told. + +- **Admission control on job size.** At ~2,700 scrobbles/day, the 60-day + credential TTL caps a completable job at roughly **162,000 tracks** — and that + assumes no outages, no global pauses, and no code-5 backoff. Reject jobs whose + conservative completion estimate exceeds the TTL rather than accepting an import + that is guaranteed to be abandoned half-finished. The measured median selection + is 34,769, so this affects few users, but those users are precisely the ones + most motivated to opt in. ## Backwards compatibility @@ -638,38 +785,69 @@ New error contexts: `background.handoff`, `background.upload`, **Blocking — these change the design:** +- **A feasibility spike is a design gate, not a follow-up.** Withdrawing the false + ~24-user number was necessary but is not sufficient; "Cloudflare stops being the + binding constraint" is currently an assertion, and the 10ms CPU limit was filed + as non-blocking despite a tick doing HMAC signing, gzip decompression, JSON + parsing, response processing, and reconciliation. **No concurrency cap may be + chosen until this is measured.** The spike must state, for one 50-track batch: + the exact D1 statements and rows written (including index write amplification), + R2 operations, subrequest count, and measured CPU — comparing a + **one-mapping-row-per-batch** representation against **fifty**. If the free tier + cannot carry even a handful of jobs, that is an argument for starting on Oracle, + and it is much cheaper to learn now than after the auth flow is built. - **Should Scrobblify move to its own origin?** LastWave shares - `https://savas.ca`, so cookie-level isolation between the two is impossible. - `__Host-` cookies on `api.savas.ca` mitigate this; a dedicated origin fixes it. - Affects existing site structure, so it is the user's call. + `https://savas.ca`, and no cookie, token, or CORS configuration can isolate two + applications on one origin. Either accept a shared trust boundary explicitly or + move. Affects existing site structure, so it is the user's call. - **A separate, server-only Last.fm API application?** The public API key can be abused by anyone into an error-26 suspension that kills every background job. Isolating it costs an extra, honestly-explained authorisation prompt. -- **D1 write budget on the free tier.** The whole scheduler design assumes cheap - per-batch writes (mapping + cursor + audit row, fenced by generation). Measure - the actual quota before committing to a tick cadence; if writes are the scarce - resource, batch size and audit granularity must be tuned to it, not to - subrequest count. **Non-blocking:** -- Wall-clock and the 10ms CPU limit for scheduled Workers on the free tier, which - bound how much pacing and decompression a single tick may do. +- **Whether Last.fm's daily cap resets on a clock or a rolling window**, which + determines the resume schedule after code 5. Measure during the beta. - **Whether Last.fm silently deduplicates identical `(artist, track, timestamp)` submissions.** Because `reTagOldListens` historically assigned one identical timestamp to every listen, a user who played the same track fifty times may have been credited with a single scrobble. If confirmed, this is a pre-existing client data-loss bug independent of this feature, and the fix — unique timestamps — is the same mechanism this design already requires. +- Concurrent scrobbling from Spotify or another scrobbler consumes the same + account budget invisibly. The conflict rules cover Scrobblify's own clients + only; nothing can see the rest. ## Review status -Reviewed adversarially on 2026-07-26. Findings incorporated: batched scrobbling, -the error-29-vs-ignore-code-5 distinction, the shared-origin cookie flaw, the -handoff transaction protocol with username binding, fencing tokens, chunked blob -storage, the withdrawal of the exactly-once and capacity claims, and honest -framing of timestamp reassignment. +Reviewed adversarially over two rounds on 2026-07-26. + +**Round one** produced: batched scrobbling, the error-29-vs-ignore-code-5 +distinction, the shared-origin cookie flaw, the handoff transaction protocol with +username binding, fencing tokens, chunked blob storage, withdrawal of the +exactly-once and capacity claims, and honest framing of timestamp reassignment. + +**Round two** corrected the round-one fixes, several of which were wrong: + +- Recent-first ordering was **backwards**; it spends slack on the tracks that + need it least. Now earliest-deadline-first. +- The `__Host-` cookie plus bearer token did **not** isolate LastWave — cookies + attach by destination, and a token the SPA can read LastWave can read. Replaced + with an explicit, documented trust-boundary decision. +- The handoff had no server-side commit of the digest it later claimed to verify. + Added a preflight step and a CAS state machine covering seven ambiguous states. +- A single cursor cannot represent a 50-entry batch's mixed outcomes. +- Fencing protects D1 but cannot revoke an in-flight Last.fm request. +- "Immutable payload" does not make behaviour immutable; jobs now pin an + algorithm version. +- The concurrency cap still referenced the withdrawn ceiling; it is now blocked + on a feasibility spike. Claims verified against Last.fm's published API documentation rather than assumed: the 50-scrobble batch limit, the 200-item `getRecentTracks` cap, the ASCII signature ordering rule, and the wording of error codes 9, 26, and 29. + +**Known unresolved**, carried deliberately rather than papered over: free-tier +feasibility is unproven pending the spike; the shared origin is a product +decision; export-before-deletion cannot guarantee delivery; and duplicates remain +possible by design under at-least-once. From cce7b8212f9b11da44e008efaead8e5e5d27dc88 Mon Sep 17 00:00:00 2001 From: Niko Savas Date: Sun, 26 Jul 2026 18:03:37 -0700 Subject: [PATCH 6/6] Fix repeat listens being silently dropped, and rate-limit handling Last.fm keys a scrobble on (user, artist, track, timestamp) and silently discards any repeat of that tuple, still reporting accepted=1 ignored=0. Verified experimentally against the live API; it is undocumented and undetectable from the response. reTagOldListens assigned the *same* Date object to every play moved into the 14-day window, so every repeat listen of a track collapsed into one scrobble. That path runs on ~78% of imports (300/386 parses, 80/119 people over 60 days). Timestamps for re-tagged plays are now allocated at send time from a cursor rather than baked in at parse time, which also fixes queues saved today and resumed weeks later being rejected wholesale with ignore code 3. The cursor persists as a high-water mark so a resumed run cannot reuse seconds an earlier run already sent, and is allocated once per track so a retry re-sends an identical tuple instead of creating a phantom play. scrobblePlay now parses the response: a 200 did not mean the play was stored, so ignored scrobbles were being counted as successes. Also in this change: - Client-side rate limiting reworked (RateLimitTracker): rolling window of send timestamps with an adaptive limit learned from observed 429s, escalating backoff, and an honest give-up-and-save instead of an infinite 1-minute retry. Across 2,135 observed rate limits the old behaviour produced 33 recoveries, median 130 minutes to recover. - Fixed a 2-second delayed emit in AuthenticateStep that silently undid a resume the user had already started. - Resume telemetry: original_total_tracks, total_succeeded, completion_pct, is_resumed and previously_scrobbled, so completion can be measured against the original import rather than the shrinking remaining chunk. - Legacy saved sessions with collapsed timestamps are migrated on load for users who are mid-import. - filterDuplicates skips re-tagged listens, whose provisional timestamps would otherwise be matched against real history. Known unfixed, pre-existing: failedIndices is always written empty, so failed tracks are never retried on resume. total_succeeded remains an upper bound, since re-scrobbling history a user already has is indistinguishable from a fresh play. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f0df23dc-ed73-4b56-9b7c-4d2846ddf2a2 --- AGENTS.md | 83 +++++- src/api/LastFm.ts | 60 +++- src/components/ScrobbleStep.vue | 465 +++++++++++++++++++++++++------ src/components/Scrobblify.vue | 124 ++++++++- src/components/SelectStep.vue | 6 + src/components/UploadStep.vue | 2 +- src/models/Scrobble.ts | 11 + src/models/SpotifyListen.ts | 3 + src/scrobblify.ts | 45 ++- src/services/RateLimitTracker.ts | 291 +++++++++++++++++++ src/services/StateManager.ts | 88 +++++- src/store.ts | 21 ++ tests/scrobblify.spec.ts | 402 ++++++++++++++++++++++++++ 13 files changed, 1488 insertions(+), 113 deletions(-) create mode 100644 src/services/RateLimitTracker.ts diff --git a/AGENTS.md b/AGENTS.md index 96f4bb1..895086c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -58,13 +58,36 @@ params** — an early version leaked a user's Last.fm session key into analytics `user_logged_out`. Rate limiting has its own events: `scrobble_rate_limited`, -`scrobble_rate_limit_cooldown_complete`, and `scrobble_rate_limit_recovered`. -Network failures emit `scrobble_network_error`. - -`scrobble_paused` carries a `reason` of `burst_limit`, `daily_limit`, or `manual` -— the first two are Last.fm rate limiting, not bugs. These dominate by volume -(`scrobble_paused` and `scrobble_rate_limited` are the highest-count events after -`step_viewed`), so treat them as normal operation, not signal. +`scrobble_rate_limit_cooldown_complete`, `scrobble_rate_limit_recovered`, +`scrobble_rate_limit_gave_up` (the escalating backoff was exhausted and progress +was auto-saved), and `scrobble_burst_limit_lowered` / `scrobble_burst_limit_raised` +from the adaptive limit in `RateLimitTracker`. Network failures emit +`scrobble_network_error`. `scrobble_ignored` fires when Last.fm accepts the +request but discards the play (see below). + +`scrobble_paused` carries a `reason` of `burst_limit`, `daily_limit`, +`rate_limit`, `rate_limit_exhausted`, `network_error`, `lastfm_daily_limit`, or +`manual` — only `manual` is a user action; the rest are Last.fm throttling, not +bugs. These dominate by volume (`scrobble_paused` and `scrobble_rate_limited` +are the highest-count events after `step_viewed`), so treat them as normal +operation, not signal. + +### Measuring completion + +Do **not** build completion percentages from `scrobbled_tracks / total_tracks`. +Those are session-scoped: a resume restores only the *remaining* tracks, so +`total_tracks` shrinks on every resume and the ratio measures progress through +the current chunk, not the import. Real examples: 81,313 → 573, 9,924 → 40. + +Every scrobble event carries resume-stable fields instead — use these: +`original_total_tracks`, `total_succeeded`, `completion_pct`, `is_resumed`, +`previously_scrobbled`. Note these only exist on events emitted after the +telemetry fix, so older events cannot be compared against them. + +`total_succeeded` is still an **upper bound**: Last.fm silently discards a +scrobble duplicating an existing (artist, track, timestamp) while reporting it +as accepted, so re-scrobbling history a user already has inflates the count with +no way to detect it from the API. ### Adding instrumentation @@ -72,6 +95,52 @@ Use the helpers in `src/services/Analytics.ts` (`trackEvent`, `trackError`, `identifyUser`) rather than calling `posthog` directly. Analytics must never break the app: every call is wrapped in try/catch and ignored on failure. +## Scrobble timestamps + +Last.fm keys a scrobble on **(user, artist, track, timestamp)** and silently +drops any repeat of that tuple — it still returns `accepted=1, ignored=0`, so +the loss is **undetectable from the response**. This was verified experimentally +against the live API; Last.fm does not document it and there is no +`ignoredMessage` code for it. Uniqueness therefore has to be guaranteed +client-side. + +An early version gave every play the *same* `Date` when the user ticked +"Scrobble tracks older than 2 weeks" (~78% of imports), which collapsed all +repeat listens of a track into a single scrobble. + +Plays that must be moved into Last.fm's 14-day window are flagged +`reTagged` (`SpotifyListen` → `Scrobble` → `SerializedScrobble`). Their +timestamps are **allocated at send time**, not at parse time, from a cursor in +`ScrobbleStep`: + +``` +reTagCursorSec = min(nowSec, max(reTagCursorSec + 1, nowSec - RETAG_BACKFILL_SECONDS)) +``` + +This matters because absolute timestamps baked in at parse time expire: a queue +saved today and resumed three weeks later would be rejected wholesale with +ignore code 3. The cursor is persisted as a high-water mark +(`lastReTagTimestampSec`) so a resumed run can never reuse seconds an earlier +run already sent. + +`scrobblePlay` returns a `ScrobbleResult`; a 200 does **not** mean the play was +stored. Check `ignored` and `ignoredMessage.code` (1 artist ignored, 2 track +ignored, 3 timestamp too old, 4 timestamp too new, 5 daily limit). Parsing +deliberately defaults to *accepted* on an unrecognised shape so a surprise can +never invent failures. + +Two consequences worth knowing before touching this code: + +- **A retry must re-send the identical timestamp.** It is allocated once per + track and held across attempts. If the original request reached Last.fm and + only the response was lost, an identical resend is deduplicated away; a fresh + second would become a phantom play. +- **`filterDuplicates` skips `reTagged` listens.** Their timestamps are + provisional, so matching them against real history compares fiction against + fact and would silently drop import rows. Their true dates survive on + `originalListenDate`, but checking those would mean fetching the user's whole + Last.fm history back to the start of the export. + ## Linting `npm run lint` auto-fixes; `npm run lint:check` (`--no-fix`) is what CI runs, so diff --git a/src/api/LastFm.ts b/src/api/LastFm.ts index 989ed80..ce3eaf2 100644 --- a/src/api/LastFm.ts +++ b/src/api/LastFm.ts @@ -1,6 +1,15 @@ import Scrobble from '@/models/Scrobble'; import md5 from 'blueimp-md5'; +/** Outcome of a single track.scrobble call, as reported by Last.fm itself. */ +export interface ScrobbleResult { + accepted: number; + ignored: number; + /** 0 when accepted. See LastFm.describeIgnoreCode for the rest. */ + ignoredCode: number; + ignoredMessage: string; +} + export default class LastFm { private API_BASE_URL = 'https://ws.audioscrobbler.com/2.0/'; @@ -244,21 +253,66 @@ export default class LastFm { } // https://www.last.fm/api/show/track.scrobble - public async scrobblePlay(play: Scrobble): Promise { + public async scrobblePlay(play: Scrobble, timestampSecOverride?: number): Promise { if (!this.userAuthKey) { throw new Error('Not authenticated.'); } + const timestampSec = timestampSecOverride !== undefined + ? timestampSecOverride + : Math.floor(play.timestamp.getTime() / 1000); const params: {[key: string]: string} = { method: 'track.scrobble', 'artist[0]': play.artist, 'track[0]': play.track, - 'timestamp[0]': Math.floor(play.timestamp.getTime() / 1000).toString(), + 'timestamp[0]': timestampSec.toString(), }; // Add album if available if (play.album) { params['album[0]'] = play.album; } - await this.makeRequest('POST', params, true); + const response = await this.makeRequest('POST', params, true); + return LastFm.parseScrobbleResponse(response); + } + + /** + * A 200 from track.scrobble does not mean the play was stored. Last.fm + * reports per-scrobble rejections in `ignoredMessage`, which this client used + * to discard entirely — so expired timestamps and daily-limit rejections were + * counted as successes. + * + * Defaults to "accepted" whenever the shape is unrecognised: an unparseable + * response must never turn a working scrobble into a reported failure. + */ + private static parseScrobbleResponse(response: any): ScrobbleResult { + const attr = (response && response.scrobbles && response.scrobbles['@attr']) || {}; + let entry = response && response.scrobbles && response.scrobbles.scrobble; + if (Array.isArray(entry)) { + [entry] = entry; + } + const ignoredMessage = (entry && entry.ignoredMessage) || {}; + + const accepted = Number(attr.accepted); + const ignored = Number(attr.ignored); + const code = Number(ignoredMessage.code); + + return { + accepted: Number.isFinite(accepted) ? accepted : 1, + ignored: Number.isFinite(ignored) ? ignored : 0, + ignoredCode: Number.isFinite(code) ? code : 0, + ignoredMessage: ignoredMessage['#text'] || '', + }; + } + + /** https://www.last.fm/api/show/track.scrobble — ignoredMessage codes. */ + public static describeIgnoreCode(code: number, message: string): string { + const known: {[key: number]: string} = { + 1: 'Last.fm ignored this artist', + 2: 'Last.fm ignored this track', + 3: 'Timestamp was too far in the past (Last.fm only accepts the last 14 days)', + 4: 'Timestamp was in the future', + 5: 'Daily scrobble limit reached', + }; + return known[code] || message || `Last.fm ignored this scrobble (code ${code})`; } private async makeRequest( diff --git a/src/components/ScrobbleStep.vue b/src/components/ScrobbleStep.vue index bf7d859..1e30fc5 100644 --- a/src/components/ScrobbleStep.vue +++ b/src/components/ScrobbleStep.vue @@ -6,6 +6,10 @@ {{ tracksToScrobble.length }} tracks ready to scrobble. Please review them and then click Scrobble to begin.

+

+ Resuming: {{ previouslyScrobbled }} of {{ originalTotalTracks }} already scrobbled in + earlier sessions. +

{{ track.track }} - {{ track.artist }} @ {{ track.timestamp.toString() }}
@@ -20,9 +24,15 @@ -
Scrobbled {{ scrobbledTracks }} of {{ tracksToScrobble.length }} ({{ failedTracks.length }} failed)
-
Burst: {{ burstCount }} / 950
-
Today: {{ dailyCount }} / ~2,700
+
+ Overall: {{ totalSucceeded }} of {{ originalTotalTracks }} scrobbled +
+
+ This session: {{ scrobbledTracks }} of {{ tracksToScrobble.length }} + ({{ failedTracks.length }} failed) +
+
Recent: {{ burstCount }} / {{ burstLimit }} in the last 10 minutes
+
Last 24h: {{ dailyCount }} / {{ dailyLimit }}
 Pause & Save @@ -30,7 +40,7 @@
- + {{ pauseReason }} @@ -41,17 +51,27 @@
You can save progress and leave now, then resume later at any time.
-
{{ scrobbledTracks }} of {{ tracksToScrobble.length }} completed so far
+
+ Your progress has been saved automatically — just come back to this page later + and choose "Resume". +
+
+ {{ totalSucceeded }} of {{ originalTotalTracks }} completed so far +
Save Progress & Leave - Wait Here + Try Again Now + Wait Here
- Scrobbling complete! {{ scrobbledTracks }} of {{ tracksToScrobble.length }} tracks scrobbled. + Scrobbling complete! + + {{ totalSucceeded }} of {{ originalTotalTracks }} tracks scrobbled. +
@@ -77,6 +97,9 @@ text-align: left; margin-top: 20px; } +.overall-progress { + font-weight: 500; +}