#10. Known Flaws

These were found while reading the code for this pack, on 27 October 2026. Each one is checked against the source, with file and line references. They're ordered by how much damage they could do once the system runs unattended. Items 1–5 are worth raising as issues before or soon after go-live.

Already tracked elsewhere and not repeated here: #14 (date never parsed) and #15 (outage page not retried, and weeklyRunCompleted never reset mid-month, so the daily checker only protects the first week of each month).

#Summary

#1. A failed Spotify add records the songs as added, and they're tombstoned next week

Where: weeklyPlaylistUpdater.ts, lines 164–170, 188–209 and 399–403.

What happens: each matched song is pushed into newTrackUris and newTrackIdentities, and result.addedToPlaylist = true is set before anything is sent to Spotify. If addTracksToPlaylist then fails (Spotify is down, or rate limits run out), the catch emails an error but doesn't stop. Then:

• updatePlaylistMetadata writes those identities, with URIs, into playlists/YYYY-MM.json as if they'd been added
• the song batch records addedToPlaylist: true
• saveState sets weeklyRunCompleted: true, so the daily checker won't retry

Next week, the live read doesn't find those URIs. Lines 399–403 treat "recorded but not live" as manually deleted and tombstone them. Every one of those songs is now permanently blocked from that month's playlist.

No test covers an addTracksToPlaylist failure in the updater.

Suggested fix: if the add fails, don't record that run's additions, don't mark the run complete, and rethrow after notifying, so the daily checker retries.

Scenario: A failed add is retried, not tombstoned
  Given the weekly run matched 3 new songs
  And Spotify rejects the add
  Then the month's metadata does not list those songs
  And weeklyRunCompleted is false
  And an error email is sent
  When the daily check runs and Spotify accepts the add
  Then the 3 songs are in the playlist and none are tombstoned

#2. Match errors are silent: no email, and the run is marked complete

Where: weeklyPlaylistUpdater.ts lines 176–184 and 235–240.

What happens: if matchSong throws (a Spotify search failure, a 5xx, a 429, or an auth failure after the retry), the song goes into matchErrors, which is only ever counted in a log line. No email is sent. If Spotify search is down at Tuesday 00:00, every song errors, the run ends "complete" with zero additions, and you hear nothing. Combined with #15, that week is lost.

This contradicts CLAUDE.md's "All errors trigger email notifications".

Suggested fix: email a summary when matchErrors.length > 0. If more than some share failed (say half), treat the run as failed: don't mark it complete, so the daily checker retries.

#3. The "multiple matches" review path is unreachable

Where: songMatcher.ts, isClearWinner, lines 364–382.

What happens: structural score tiers are 1000, 500, 100 and 10, all multiples of 10. Popularity adds 0–0.99. So the gap between two candidates is always ≥ 10 or < 1, and the return false branch (and the 'multiple matches' outcome in makeDecision) can never run. The matcher always auto-selects among verified candidates.

Since #13 added verification, the damage is limited to choosing the wrong version of the right song. Still, the code and its comments describe a safety net that doesn't exist, and a reader (or an AI) will assume it does.

Suggested fix: decide the policy. Either delete the dead branch and say "verified candidates are always auto-selected", or define ambiguity for real. For example, flag for review when the top two are structurally equal and their popularity is within 5 points. Add a test that reaches the branch.

#4. Search and reads have no rate-limit handling

Where: spotifyService.ts. searchTrack and getPlaylistTrackDetails go through callSpotify, which handles only 401. Only addBatchWithRetry handles 429.

What happens: a 429 during search becomes a match error, which is silent (flaw 2). A 429 during the live playlist read makes the updater fall back to cached metadata, which skips tombstone detection that week. The weekly volume (about 40–80 calls) makes this unlikely, but a backfill or a retry storm could trigger it.

Suggested fix: move the 429 / Retry-After handling into callSpotify, so every call gets it, and remove the duplicate in addBatchWithRetry.

#5. State files are written in place, with no validation on load

Where: stateManager.ts, fs.promises.writeFile(filePath, …) (lines 51, 78, 103) and JSON.parse(content) as ProcessingState (lines 65, 92, 119).

What happens: if the process is killed or the disk fills mid-write, processing-state.json is left truncated. Every later job then throws on JSON.parse until someone fixes the file by hand. as ProcessingState is a type assertion, not a check: a file with missing fields loads silently and fails later in confusing ways. ADR 9 claims "atomic updates", which isn't true.

Suggested fix: write to file.tmp, then rename it over the original (atomic on Linux), the same trick release.sh uses for current. Validate the shape on load with a small hand-written guard or a schema library, and fail with a clear message.

#6. No heartbeat: if the process or server is gone, nothing tells you

What happens: every alert is sent by the app. If the VPS is down, PM2 has given up after max_restarts: 10, or the process is hung, no email ever arrives. Silence looks just like "nothing to report".

Suggested fix: use a dead man's switch. After each daily check, ping an external heartbeat service (healthchecks.io and similar services email you when the ping stops). Or, more simply, send a short weekly "ran OK: N added, M for review" email, so that its absence is the alarm.

#7. Rejecting a review item is impossible, so it's flagged every week

What happens: if the matcher flags a song and you decide not to add any version, nothing records that decision. The next week the song is still on the BBC list, isn't in the playlist, and is flagged again, for every week it stays on the BBC list. Tombstones only exist for tracks that were added and then removed.

Suggested fix: a simple "ignore" list in data/, or a documented workaround: add the track, then delete it, which creates a tombstone.

#8. Three different "normalise a name" implementations

Where:

• songMatcher.normaliseForCompare: lowercase, remove apostrophes, fold punctuation to spaces
• weeklyPlaylistUpdater.makeIdentityKey / normaliseTitle: lowercase, trim, strip (feat. …) with its own regex
• titleCleaner.FEATURED_PATTERN: a third feat. regex

What happens: duplicate detection doesn't fold punctuation. A BBC title with a curly apostrophe ("Don’t") and a hand-added Spotify track with a straight one ("Don't") get different identity keys. The song is re-matched and can be flagged for review again even though it's already in the playlist. Two feat. regexes will drift apart.

Suggested fix: one utils/normalise.ts used by both the matcher and the identity key, with its own tests. Be careful: changing the identity key changes how existing records compare, so plan a backfill or a dual-read.

#9. CLAUDE.md and the ADRs state things that aren't true

This matters more than usual, because CLAUDE.md is instructions to the AI. Wrong statements there get repeated into new code and new docs.

Suggested fix: one docs PR: correct CLAUDE.md, add amendment notes to the ADRs (as was already done for 11.1), and link from CLAUDE.md to this pack.

#10. Smaller items

• retentionDays is ignored. cleanupOldFiles(this.dataDir, 28) in both the weekly updater and the daily checker. Pass the config value through.
• The refresh token isn't persisted if Spotify rotates it (spotifyService.ts line 74). It works until the next restart, then falls back to the old token.
• spotify-web-api-node has seen little maintenance in years. Assess @spotify/web-api-ts-sdk or a thin fetch client before the next Spotify API change forces the decision.
• The monthly creator trusts local state only. If state is wrong, it creates a second "BBC 6 Music Oct 2026". Looking for an existing playlist with that name first would make it idempotent against Spotify, not just against our files. The deploy preflight reduces the risk but doesn't remove it.
• scripts/ isn't linted or type-checked in CI (lint runs on src only, and tsconfig excludes scripts). Several scripts write to the real account, so they shouldn't be allowed to rot.
• No coverage threshold. test:coverage exists, but CI doesn't enforce a floor, so coverage can drift down unnoticed.
• isLive is a regex on the title (/\blive\b/i), so "Live Forever" counts as a live recording. It's harmless while every version of a song shares the title, but it's a trap if the rule is ever used across songs.
• The deploy key gets a full shell as the subscription user. You could harden it with restrict and a forced command in authorized_keys that only runs release.sh.
• Daily check at 01:00 on clock-change days (guide 5): it may be skipped in March or doubled in October. That's harmless, but it's unobserved. Moving it to 03:00 removes the question entirely.
• Housekeeping: e2e-run-output.txt and scripts/dump-playlists.mjs are untracked in the working tree. The plan said to gitignore or delete stray output files.