refactor: extract testable resolve-core + cover album routing; check.sh checks .cjs (closes task/resolver-orchestration-testable)

Source: task/resolver-orchestration-testable (plan/steward-linker-quality) — new album orchestration in resolver.js had zero node:test coverage.

- Extract the pure album/artist orchestration into service/resolve-core.js
  (resolveAll/resolveOne/pickDirect/cacheKey) taking injected search/classify
  fns; resolver.js becomes a thin server wiring the real Spotify search +
  ollama classify. No behavior change.
- Add service/resolver.test.js: self-titled artist+album tie-break, LLM-album
  -> Google fallback, kind-aware cache separation, plus gate/drop/error paths.
- scripts/check.sh: node --check now also matches *.cjs (extract.test.cjs).
- AGENTS.md Testing: note resolve-core coverage + the extract.test.cjs extractor.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

AGENTS.md

@@ -39,9 +39,13 @@ user's PAT). Do **not** use `towl-orchestrate` / worktree+PR here.
 ## Testing (zero-dep)
 
 `node --test` over `service/*.test.js` exercises `lib.js` (norm + the exact-name match
-+ link builders). Keep pure logic in `lib.js` (not `resolver.js`, which has the
++ link builders), `llm.js` (the classifier prompt/parse), and `resolve-core.js` (the
-server side effect) so it stays testable without opening a port. **Add/extend a test
+album/artist orchestration — kindHint tie-break, kind-aware cache key, allowFallback
-with every logic change.** The extension's in-browser behavior is **not** headlessly
+gating — driven with injected fake search/classify fns, no network). Keep pure logic in
+`lib.js`/`resolve-core.js` (not `resolver.js`, which has the server side effect) so it stays
+testable without opening a port. `extension/extract.test.cjs` covers the pure extractor
+(artist/cue/album candidates) and must be extended with each detection change. **Add/extend a
+test with every logic change.** The extension's in-browser behavior is **not** headlessly
 testable in this environment — validate `content.js` with `node --check` and a
 documented manual check on a real reddit thread (`about:debugging` → load temporary
 add-on).

scripts/check.sh

@@ -5,7 +5,7 @@ set -euo pipefail
 cd "$(dirname "$0")/.."
 
 echo "== node --check (syntax) =="
-for f in $(find service extension -name '*.js'); do
+for f in $(find service extension \( -name '*.js' -o -name '*.cjs' \)); do
   node --check "$f"
   echo "  ok $f"
 done

service/resolve-core.js

@@ -0,0 +1,96 @@
+// Pure album/artist resolution orchestration, decoupled from the HTTP server and the live
+// Spotify/ollama clients so `node --test` can drive it with injected fakes. resolver.js wires
+// the real `search` (a type=artist,album Spotify lookup behind the exact-name gate) and `classify`
+// (the ollama LLM gate) into these functions; the cache-key logic, the kindHint tie-break
+// ordering, and the allowFallback gating live here as pure logic.
+
+import { norm, linkResult } from "./lib.js";
+
+// Cap on how many distinct candidates one resolveAll call considers.
+export const MAX_CANDIDATES = 100;
+
+// Decide the single direct-link {url, kind} for a name from a search result, or null. `result`
+// is what an injected `search(name)` returns: { artist, album } where each is the exact-match
+// hit's external url (string) or null/absent. `kindHint` ("artist" | "album", from the LLM gate)
+// breaks the tie when BOTH an artist and an album match the same string (e.g. a self-titled
+// album): album-first when hinted "album", else artist-first (the artist-first MVP default).
+export function pickDirect(result, kindHint) {
+  const artist = result?.artist ?? null;
+  const album = result?.album ?? null;
+  const order = kindHint === "album"
+    ? [["album", album], ["artist", artist]]
+    : [["artist", artist], ["album", album]];
+  for (const [kind, url] of order) {
+    if (url) return { url, kind };
+  }
+  return null;
+}
+
+// The cache key for a name's direct-link lookup. It includes the kind hint because the same
+// string can resolve to an artist OR an album depending on which the gate confirmed, so a
+// hint-blind key would cross-contaminate the two.
+export const cacheKey = (name, kindHint) => `${kindHint || "artist"}:${norm(name)}`;
+
+// Resolve one name to its link object, or null. `search(name) -> { artist, album }` is injected
+// (the live Spotify lookup, or a fake). `cache` is a Map(cacheKey -> {url, kind}|null) memoizing
+// the search half; the link object is rebuilt per call so a name's result never depends on a
+// stale gate decision. allowFallback: when the gate already confirmed this name, a missing direct
+// link still yields a Google->Bandcamp fallback link (instead of null). kindHint biases the
+// lookup tie-break and the fallback link's kind; an actual exact hit's kind wins when there is one.
+export async function resolveOne(name, { search, cache, allowFallback, kindHint }) {
+  if (!norm(name)) return null;
+  try {
+    const key = cacheKey(name, kindHint);
+    let hit;
+    if (cache && cache.has(key)) {
+      hit = cache.get(key);
+    } else {
+      hit = pickDirect(await search(name, kindHint), kindHint);
+      if (cache) cache.set(key, hit);
+    }
+    if (hit) return linkResult(name, hit.url, hit.kind); // direct Spotify page (artist or album)
+    return allowFallback ? linkResult(name, null, kindHint) : null; // google fallback, or unconfirmed
+  } catch (e) {
+    console.error(`resolve "${name}": ${e.message}`); // transient -> caller sees null this time
+    return null;
+  }
+}
+
+// Resolve a batch of candidate names to a { name -> link|null } map. Dependencies are injected:
+//   search(name, kindHint) -> { artist, album }  (live Spotify lookup or a fake; required)
+//   classify(names) -> Map(norm(name) -> "artist"|"album") | null  (the optional LLM gate; when
+//     it returns null — no gate, or LLM down — the Spotify exact-match IS the gate: only names
+//     actually on Spotify link. When it returns a Map, names absent from it are dropped, names in
+//     it may use the Google fallback when not on Spotify, and the mapped kind routes the lookup.)
+//   cache  Map for memoizing the search half (optional)
+export async function resolveAll(candidates, { search, classify, cache } = {}) {
+  const uniq = [...new Set(candidates.map((c) => String(c).trim()).filter(Boolean))].slice(0, MAX_CANDIDATES);
+
+  let confirmed = null;
+  if (classify && uniq.length) {
+    try {
+      confirmed = await classify(uniq);
+    } catch (e) {
+      console.error(`llm classify: ${e.message}`); // LLM down -> fall back to the Spotify gate
+      confirmed = null;
+    }
+  }
+
+  const out = {};
+  await Promise.all(
+    uniq.map(async (name) => {
+      if (confirmed && !confirmed.has(norm(name))) {
+        out[name] = null; // LLM said this is not an artist/album
+        return;
+      }
+      const kindHint = confirmed ? confirmed.get(norm(name)) : undefined;
+      out[name] = await resolveOne(name, {
+        search,
+        cache,
+        allowFallback: confirmed != null,
+        kindHint,
+      });
+    }),
+  );
+  return out;
+}

service/resolver.js

@@ -15,8 +15,9 @@
 
 import { createServer } from "node:http";
 import { readFileSync } from "node:fs";
-import { norm, pickArtist, pickAlbum, linkResult } from "./lib.js";
+import { pickArtist, pickAlbum } from "./lib.js";
 import { buildClassifyMessages, parseClassification, DEFAULT_MODEL } from "./llm.js";
+import { resolveAll as resolveAllCore } from "./resolve-core.js";
 
 // --- minimal .env loader (no dependency) ------------------------------------
 try {
@@ -34,10 +35,10 @@ const USE_SPOTIFY = Boolean(SPOTIFY_ID && SPOTIFY_SECRET);
 const OLLAMA_URL = (process.env.OLLAMA_URL || "").replace(/\/$/, ""); // presence enables the LLM gate
 const OLLAMA_MODEL = process.env.OLLAMA_MODEL || DEFAULT_MODEL;
 const USE_LLM = Boolean(OLLAMA_URL);
-const MAX_CANDIDATES = 100;
 
-// Cache only the network half (Spotify direct-url lookup); the final link object is rebuilt
+// Cache only the network half (the Spotify direct-url lookup); resolve-core rebuilds the final
-// per request so a name's result never depends on a stale gate decision.  norm(name) -> url|null
+// link object per request so a name's result never depends on a stale gate decision. The key
+// includes the kind hint (see resolve-core's cacheKey).
 const spCache = new Map();
 
 // --- Spotify (optional): direct-artist-link lookup --------------------------
@@ -58,36 +59,21 @@ async function spotifyToken() {
   return spToken;
 }
 
-// The direct Spotify url for an exact name match across artist AND album, or null (no creds /
+// The injected `search` for resolve-core: one type=artist,album Spotify search per candidate,
-// no exact hit). One search call covers both types; `kindHint` ("artist" | "album", from the
+// returning the exact-name-match hits as { artist, album } where each is the external Spotify url
-// LLM gate) decides which exact hit wins when BOTH match the same string (e.g. a self-titled
+// (string) or null. No creds -> both null (nothing links). resolve-core's pickDirect applies the
-// album) — default prefers the artist, matching the artist-first MVP behaviour. Returns
+// kindHint tie-break and shapes the link; the cache (spCache) is owned by resolve-core too.
-// { url, kind } so the caller can shape an artist vs album link.
+async function search(name) {
-async function spotifyDirect(name, kindHint) {
+  if (!USE_SPOTIFY) return { artist: null, album: null };
-  if (!USE_SPOTIFY) return null;
   const token = await spotifyToken();
   const url = `https://api.spotify.com/v1/search?type=artist,album&limit=5&q=${encodeURIComponent(name)}`;
   const r = await fetch(url, { headers: { authorization: `Bearer ${token}` } });
   if (!r.ok) throw new Error(`spotify search http ${r.status}`);
   const j = await r.json();
-  const artist = pickArtist(name, j.artists?.items);
+  return {
-  const album = pickAlbum(name, j.albums?.items);
+    artist: pickArtist(name, j.artists?.items)?.external_urls?.spotify ?? null,
-  const order = kindHint === "album" ? [["album", album], ["artist", artist]] : [["artist", artist], ["album", album]];
+    album: pickAlbum(name, j.albums?.items)?.external_urls?.spotify ?? null,
-  for (const [kind, hit] of order) {
+  };
-    const u = hit?.external_urls?.spotify;
-    if (u) return { url: u, kind };
-  }
-  return null;
-}
-
-async function spotifyDirectCached(name, kindHint) {
-  // Cache key includes the kind hint: the same string can resolve to an artist OR an album
-  // depending on which the gate confirmed, so a hint-blind cache would cross-contaminate.
-  const key = `${kindHint || "artist"}:${norm(name)}`;
-  if (spCache.has(key)) return spCache.get(key);
-  const hit = await spotifyDirect(name, kindHint);
-  spCache.set(key, hit);
-  return hit;
 }
 
 // --- LLM gate (optional): which candidates are artists/albums? --------------
@@ -109,52 +95,11 @@ async function ollamaClassify(candidates) {
 }
 
 // --- resolve --------------------------------------------------------------
-// allowFallback: when the gate has already confirmed this name is an artist/album, a missing
+// Run the pure orchestration (resolve-core) with the live clients injected: the Spotify `search`,
-// Spotify direct link still yields a Google->Bandcamp fallback link (instead of null).
+// the ollama `classify` gate (null when no LLM is configured -> the Spotify exact-match is the
-// kindHint ("artist" | "album", from the LLM gate or undefined) biases the Spotify lookup and
+// gate), and the shared spCache.
-// the fallback link's kind; the actual exact Spotify hit's kind wins when there is one.
+const resolveAll = (candidates) =>
-async function resolveOne(name, allowFallback, kindHint) {
+  resolveAllCore(candidates, { search, classify: USE_LLM ? ollamaClassify : null, cache: spCache });
-  if (!norm(name)) return null;
-  try {
-    const sp = await spotifyDirectCached(name, kindHint);
-    if (sp) return linkResult(name, sp.url, sp.kind); // direct Spotify page (artist or album)
-    return allowFallback ? linkResult(name, null, kindHint) : null; // google fallback, or unconfirmed
-  } catch (e) {
-    console.error(`resolve "${name}": ${e.message}`); // transient -> caller sees null this time
-    return null;
-  }
-}
-
-async function resolveAll(candidates) {
-  const uniq = [...new Set(candidates.map((c) => String(c).trim()).filter(Boolean))].slice(0, MAX_CANDIDATES);
-
-  // Gate: which names may link? null => no LLM gate, the Spotify exact-match (artist OR album)
-  // is the gate (only names actually on Spotify link). A Map(name -> "artist"|"album") => the
-  // LLM's verdict; names not in it are dropped, names in it may use the Google fallback when
-  // not on Spotify, and the mapped kind routes the Spotify lookup to the right type.
-  let confirmed = null;
-  if (USE_LLM && uniq.length) {
-    try {
-      confirmed = await ollamaClassify(uniq);
-    } catch (e) {
-      console.error(`llm classify: ${e.message}`); // LLM down -> fall back to the Spotify gate
-      confirmed = null;
-    }
-  }
-
-  const out = {};
-  await Promise.all(
-    uniq.map(async (name) => {
-      if (confirmed && !confirmed.has(norm(name))) {
-        out[name] = null; // LLM said this is not an artist/album
-        return;
-      }
-      const kindHint = confirmed ? confirmed.get(norm(name)) : undefined;
-      out[name] = await resolveOne(name, confirmed != null, kindHint);
-    }),
-  );
-  return out;
-}
 
 // --- HTTP -------------------------------------------------------------------
 function readBody(req) {

service/resolver.test.js

@@ -0,0 +1,141 @@
+// Unit tests for the resolve-core orchestration (run with `node --test`). Zero-dep, hermetic:
+// the live Spotify/ollama clients are replaced by injected fake search/classify functions, so no
+// network and no real timers. Covers the album routing the HTTP server can't unit-test directly.
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { resolveAll, resolveOne, pickDirect, cacheKey } from "./resolve-core.js";
+
+const ARTIST_URL = "https://open.spotify.com/artist/abc";
+const ALBUM_URL = "https://open.spotify.com/album/xyz";
+
+// A fake `search` from a { norm(name) -> { artist, album } } table (urls or null).
+const fakeSearch = (table) => (name) => Promise.resolve(table[name.trim().toLowerCase()] ?? { artist: null, album: null });
+
+// A fake `classify` returning a Map(norm(name) -> kind) from a plain { name: kind } object.
+const fakeClassify = (verdicts) => (names) => {
+  void names;
+  return Promise.resolve(new Map(Object.entries(verdicts).map(([n, k]) => [n.toLowerCase(), k])));
+};
+
+test("pickDirect tie-break: kindHint='album' prefers the album, else artist-first", () => {
+  const both = { artist: ARTIST_URL, album: ALBUM_URL };
+  assert.deepEqual(pickDirect(both, "album"), { url: ALBUM_URL, kind: "album" });
+  assert.deepEqual(pickDirect(both, "artist"), { url: ARTIST_URL, kind: "artist" });
+  assert.deepEqual(pickDirect(both, undefined), { url: ARTIST_URL, kind: "artist" });
+  // single-sided hits ignore the hint
+  assert.deepEqual(pickDirect({ artist: ARTIST_URL, album: null }, "album"), { url: ARTIST_URL, kind: "artist" });
+  assert.equal(pickDirect({ artist: null, album: null }, "album"), null);
+});
+
+test("cacheKey separates the same name by kind hint, defaults to artist", () => {
+  assert.equal(cacheKey("Tool", "album"), "album:tool");
+  assert.equal(cacheKey("Tool", "artist"), "artist:tool");
+  assert.equal(cacheKey("Tool", undefined), "artist:tool");
+});
+
+test("self-titled artist+album tie-break routes to the kindHint'd URL", async () => {
+  // A self-titled name that is BOTH an exact artist hit and an exact album hit on Spotify.
+  const search = fakeSearch({ "bad religion": { artist: ARTIST_URL, album: ALBUM_URL } });
+
+  // kindHint='album' -> the album URL.
+  const asAlbum = await resolveOne("Bad Religion", { search, kindHint: "album", allowFallback: true });
+  assert.equal(asAlbum.kind, "album");
+  assert.equal(asAlbum.spotify, ALBUM_URL);
+  assert.deepEqual(asAlbum.primary, { platform: "spotify", url: ALBUM_URL });
+
+  // undefined and 'artist' both -> the artist URL (artist-first default).
+  for (const hint of [undefined, "artist"]) {
+    const asArtist = await resolveOne("Bad Religion", { search, kindHint: hint, allowFallback: true });
+    assert.equal(asArtist.kind, "artist", `hint=${hint}`);
+    assert.equal(asArtist.spotify, ARTIST_URL);
+    assert.deepEqual(asArtist.primary, { platform: "spotify", url: ARTIST_URL });
+  }
+});
+
+test("album confirmed by the LLM but absent from Spotify falls back to a Google search", async () => {
+  // The LLM gate confirms the name is an album; Spotify has no exact hit -> Google fallback.
+  const search = fakeSearch({}); // nothing on Spotify
+  const classify = fakeClassify({ "Some Local Release": "album" });
+
+  const out = await resolveAll(["Some Local Release"], { search, classify });
+  const r = out["Some Local Release"];
+  assert.ok(r, "confirmed name should resolve, not drop");
+  assert.equal(r.kind, "album");
+  assert.equal(r.primary.platform, "google");
+  assert.match(r.primary.url, /^https:\/\/www\.google\.com\/search\?q=/);
+  assert.ok(r.primary.url.includes("bandcamp"));
+  assert.equal(r.spotify, undefined);
+});
+
+test("LLM confirms artist+album for two names and routes each kind's Spotify lookup", async () => {
+  const search = fakeSearch({
+    devourment: { artist: ARTIST_URL, album: null },
+    "reek of putrefaction": { artist: null, album: ALBUM_URL },
+  });
+  const classify = fakeClassify({ Devourment: "artist", "Reek of Putrefaction": "album" });
+
+  const out = await resolveAll(["Devourment", "Reek of Putrefaction"], { search, classify });
+  assert.equal(out["Devourment"].kind, "artist");
+  assert.equal(out["Devourment"].spotify, ARTIST_URL);
+  assert.equal(out["Reek of Putrefaction"].kind, "album");
+  assert.equal(out["Reek of Putrefaction"].spotify, ALBUM_URL);
+});
+
+test("names the LLM did not confirm are dropped (null)", async () => {
+  const search = fakeSearch({ "real band": { artist: ARTIST_URL, album: null } });
+  const classify = fakeClassify({ "Real Band": "artist" }); // "the best part" not in the map
+
+  const out = await resolveAll(["Real Band", "the best part"], { search, classify });
+  assert.ok(out["Real Band"]);
+  assert.equal(out["the best part"], null);
+});
+
+test("cache-key kind separation: same name as artist vs album does not cross-contaminate", async () => {
+  const cache = new Map();
+  // One name exists on Spotify as BOTH; the kind hint must keep the two lookups distinct.
+  const search = fakeSearch({ "self titled": { artist: ARTIST_URL, album: ALBUM_URL } });
+
+  const asArtist = await resolveOne("Self Titled", { search, cache, kindHint: "artist", allowFallback: true });
+  const asAlbum = await resolveOne("Self Titled", { search, cache, kindHint: "album", allowFallback: true });
+
+  assert.equal(asArtist.spotify, ARTIST_URL);
+  assert.equal(asArtist.kind, "artist");
+  assert.equal(asAlbum.spotify, ALBUM_URL);
+  assert.equal(asAlbum.kind, "album");
+  // Two distinct cache entries, keyed by kind — no cross-contamination.
+  assert.equal(cache.size, 2);
+  assert.deepEqual(cache.get("artist:self titled"), { url: ARTIST_URL, kind: "artist" });
+  assert.deepEqual(cache.get("album:self titled"), { url: ALBUM_URL, kind: "album" });
+});
+
+test("with no LLM gate (classify null) the Spotify exact-match IS the gate; unconfirmed -> null", async () => {
+  // On Spotify -> links; not on Spotify and no confirmation -> null (no Google fallback).
+  const search = fakeSearch({ devourment: { artist: ARTIST_URL, album: null } });
+
+  const out = await resolveAll(["Devourment", "Not A Band"], { search, classify: null });
+  assert.equal(out["Devourment"].kind, "artist");
+  assert.equal(out["Devourment"].spotify, ARTIST_URL);
+  assert.equal(out["Not A Band"], null); // not confirmed, not on Spotify -> dropped
+});
+
+test("LLM-down (classify throws) falls back to the Spotify exact-match gate", async () => {
+  const search = fakeSearch({ devourment: { artist: ARTIST_URL, album: null } });
+  const classify = () => Promise.reject(new Error("ollama down"));
+
+  const out = await resolveAll(["Devourment", "Not A Band"], { search, classify });
+  assert.equal(out["Devourment"].spotify, ARTIST_URL); // Spotify hit still links
+  assert.equal(out["Not A Band"], null); // no fallback once the LLM gate is gone
+});
+
+test("resolveAll dedups, trims, drops blanks, and caps the candidate list", async () => {
+  const search = fakeSearch({ tool: { artist: ARTIST_URL, album: null } });
+  const out = await resolveAll(["Tool", " Tool ", "", "   "], { search, classify: null });
+  assert.deepEqual(Object.keys(out), ["Tool"]); // " Tool " collapses to "Tool"; blanks gone
+  assert.equal(out["Tool"].spotify, ARTIST_URL);
+});
+
+test("resolveOne swallows a search error and yields null for that name", async () => {
+  const search = () => Promise.reject(new Error("spotify 503"));
+  const r = await resolveOne("Tool", { search, allowFallback: true });
+  assert.equal(r, null);
+});