From 9ade2b34a591293b52712e1380033a16b685036d Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Tue, 14 Jul 2026 07:24:21 -0700
Subject: [PATCH] docs: fold session-memory lessons into committed docs (closes
 #261)

Extracted from persistent agent memory (ICM + auto-memory) so the
committed docs are self-sufficient:

- README: Node 22 requirement for local dev (better-sqlite3 prebuilds)
- pitfalls: getAlbumList2 inline-SQL exception to #130 hoisting rule
- pitfalls: merge-worker must stay dependency-light
- pitfalls: never proxy the Navidrome native API to peers
- pitfalls (Process): concurrent agent sessions need git worktrees;
  live hub is not a test bed (no DB resets, verify uptime + logs)
- frontend-testing: duplicate img alt-text collision, scope with
  within(dialog)
- CLAUDE.md: Write-tool control-character gotcha (Claude Code only)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SwwPcrohddTUN7kmQVedQC
---
 CLAUDE.md                | 4 ++++
 README.md                | 2 ++
 docs/frontend-testing.md | 1 +
 docs/pitfalls.md         | 7 +++++--
 4 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/CLAUDE.md b/CLAUDE.md
index 1cf03ac..4e5a29f 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -6,6 +6,10 @@
 
 When posting comments, ALWAYS prefix comments with `FROM @claude:` and use blockquote style
 
+### Tool gotchas
+
+* The Write tool can emit literal control characters for `\u0000`–`\u001f` escapes inside regex character classes; a later Edit `old_string` then never matches. Repair the bytes with `perl -i -pe`. (Hit twice.)
+
 ### Workflow
 
 Default flow should be to use Pull Requests.
diff --git a/README.md b/README.md
index f701f39..63aa2fd 100644
--- a/README.md
+++ b/README.md
@@ -22,6 +22,8 @@ Full env var list: [docs/hub-internals.md#environment-variables](docs/hub-intern
 
 ## Local development (without Docker)
 
+Requires **Node 22** (what CI and the Docker image run). Newer majors ship before `better-sqlite3` publishes a prebuild — tests then fail with a `NODE_MODULE_VERSION` mismatch or fall back to a source build.
+
 ```bash
 pnpm install
 pnpm dev                        # Hub on :3000 (tsx watch)
diff --git a/docs/frontend-testing.md b/docs/frontend-testing.md
index 5a65acf..624f105 100644
--- a/docs/frontend-testing.md
+++ b/docs/frontend-testing.md
@@ -47,3 +47,4 @@ Zustand stores expose `setState` / `getState` directly — reset in `beforeEach`
 - `vi.mock` is hoisted — no top-level variables inside the factory.
 - jsdom does not implement `HTMLMediaElement` playback; test the surrounding logic (state, toast dispatch) rather than `<audio>` behavior itself.
 - If a test imports from `@/lib/api` or `@/lib/subsonic`, the `localStorage` polyfill in `setup.ts` must be in place — do not remove it.
+- Two images sharing alt text (e.g. an album thumbnail and its opened lightbox) make `getByRole("img", { name })` throw on multiple matches — scope the query with `within(getByRole("dialog"))`.
diff --git a/docs/pitfalls.md b/docs/pitfalls.md
index a6cdf3f..efeac5b 100644
--- a/docs/pitfalls.md
+++ b/docs/pitfalls.md
@@ -12,7 +12,7 @@ Recurring traps that have caused bugs and follow-up PRs. Read before touching th
 | Letting `unified_release_groups.created_at` default to `datetime('now')` at merge time                                                                 | #186      | Source it from `MAX(instance_albums.created_at)` so the SPA's "Recently Added" sort reflects Navidrome's actual added time         |
 | Changing a dedup key (or just editing metadata) without the id-remap step orphans `user_stars` / `playlist_tracks` / `play_events`                     | #242      | `runMergePipeline()` / `runMergePipelineAsync()` (`merge-pipeline.ts`) snapshot identity, call `mergeLibraries`, then `applyRemap` before auditing — never call `mergeLibraries()` directly outside tests, or user data silently strands on the next merge |
 | Merge running on the worker (#242 Phase 3) doesn't block WAL readers, but a main-connection WRITE started while the merge holds its write lock waits up to the 5s busy timeout, then throws SQLITE_BUSY | #242 | Keep main-thread write transactions short; don't hold one open across an `await runMergePipelineAsync(...)` |
-| Renaming/moving `merge-worker.ts`                                                                                                                       | #242      | `runMergePipelineAsync` resolves both a dev/test `.ts` path and a prod compiled `.js` path from `import.meta.url` — check both branches when the file moves |
+| Renaming/moving `merge-worker.ts`                                                                                                                       | #242      | `runMergePipelineAsync` resolves both a dev/test `.ts` path and a prod compiled `.js` path from `import.meta.url` — check both branches when the file moves. Keep the worker dependency-light (no fastify/routes/services imports): it must load standalone under tsx (dev) and as compiled JS (prod) |
 
 Every `unified_*` insert in `merge.ts` logs the offending row + prior `existingRow` + source IDs on PK conflict. Preserve that — it is the first thing you read when a collision regresses.
 
@@ -63,6 +63,7 @@ Endpoint coverage detail: [opensubsonic.md](opensubsonic.md).
 | Changing a `/federation/*` contract without bumping `FEDERATION_API_VERSION` in `hub/src/version.ts` and updating federation-api.md | recurring | Contract is the doc; the doc is the contract. Bump both together                                                     |
 | Adding a peer-facing gate to `federation/peer-auth.ts` and assuming `/proxy/*` is covered                                          | #244      | `/proxy/*` has its own auth path (`proxy/auth.ts`); every peer-level policy must land in BOTH. Only the federation suite catches the gap |
 | Version-gating peers against `FEDERATION_API_VERSION` instead of the floor                                                        | #244      | Compare against `MIN_FEDERATION_API_VERSION` — the current version silently cuts supported older peers off at every bump |
+| Exposing the Navidrome native API (`/api/*`) to peers                                                                              | #252 design | Never proxy it — it is the full, unversioned admin surface (incl. user mutation) and breaks silently across Navidrome releases. Native-API use stays local-only (`services/navidrome-native.ts`); peer-visible data goes through Subsonic or a Poutine-owned `/federation/*` contract |
 
 Full protocol: [federation-api.md](federation-api.md).
 
@@ -87,7 +88,7 @@ Full flow: [authentication.md](authentication.md).
 | Trap                                                                                          | Caught by | Rule                                                                                                |
 |-----------------------------------------------------------------------------------------------|-----------|-----------------------------------------------------------------------------------------------------|
 | TOCTOU between `artCache.get()` and concurrent eviction                                       | recurring | `get()` returns content + holds a ref; eviction can't delete an in-flight stream                    |
-| Hoisting prepared statements rebuilt per-request                                              | #130      | Hot-path SQL goes through prepared statements built once at module load                             |
+| Hoisting prepared statements rebuilt per-request                                              | #130      | Hot-path SQL goes through prepared statements built once at module load. Deliberate exception: `getAlbumList2`'s dynamically-assembled SQL stays inline in `routes/subsonic/browsing.ts` — don't "fix" it |
 
 ## Navidrome ops
 
@@ -132,3 +133,5 @@ Full flow: [authentication.md](authentication.md).
 | Broken tests landed on green CI                                     | #116      | CI runs `pnpm verify` (typecheck + lint:boundary + test) on every PR                          |
 | Stacked PRs                                                         | CLAUDE.md | Follow-ups go on the same feature branch — never branch off a draft PR                        |
 | Working without a GitHub issue                                      | AGENTS.md | No issue, no work. Reference the issue in the commit and close on merge                       |
+| Two concurrent agent sessions sharing one working tree              | memory    | Checkouts silently land on the other session's branch and stashes mix both sessions' files. Give each concurrent session its own `git worktree add` directory, or serialize |
+| Treating the live hub as a test bed                                 | memory    | Never reset the live instance's DB; run DB-touching tests against a separate instance. After changing the running instance, confirm it is still up and check its logs       |