refactor(marketplace): move source parsing, slugging and the fetch/re-slot body out of the CLI; clean up orphan caches

[spxrogers]

Follow-up to #233 (the "related consolidation" that issue set aside).

1. Domain logic living in the CLI

internal/cli/marketplace.go holds three pieces of internal/marketplace domain logic — parseMarketplaceSource, deriveMarketplaceSlug, sanitizeSlug — plus the fetch + declared-name + re-slot body of addMarketplaceSource. The poll engine (plugin_poll.go, same package) calls parseMarketplaceSource too, while nothing under internal/marketplace can use any of it.

Proposal: move the three helpers there with their tests, and consider a marketplace.FetchInto(home, src, rawURL) returning (name, headSHA) that owns the fetch and the cache re-slot (reslotMarketplaceCache, added by #233's fix), leaving the CLI the TOML/state writes and the printing.

2. A second reader/writer for marketplaces/*.toml

marketplaceTOMLSpec (marketplace.go) gives marketplaces/*.toml a second shape alongside source.MarketplaceSpec + marketplaceStrictFile (internal/source/loader.go). The split is deliberate and documented (head_sha/name are fetch-cache metadata, not canonical schema — #171), but three structs describe one file. Worth deciding whether the CLI shape should sit beside the canonical one so a fourth key cannot be added to one and missed by the others. Docs to update with it: docs/components.md, plus docs/architecture.md §9 if the network-boundary description moves.

3. Orphan caches left by pre-fix re-adds

Before #233's fix, every re-add of a marketplace whose declared name differs from its URL slug left a duplicate tree under .state/cache/marketplaces/<slug>. Those pre-fix orphans are consumed only when that marketplace is re-added (the next add pulls/copies into the orphan and swaps it in); nothing else touches them, marketplace remove cannot reach them, and searchAllMarketplaces offers them to a bare-id plugin add as unregistered marketplaces. A doctor check (a cache dir with no marketplaces/<name>.toml) and/or a cleanup for orphans under .state/cache/marketplaces is a reasonable item here.

Deliberately out of scope

marketplace add is last-writer-wins when two sources declare the same name (TOML, state entry and, since #233's fix, the cache). Whether that should be refused is its own question.

[spxrogers]

One more item for this issue, measured by the adversarial lens during #260's review and confirmed pre-existing at 0b29542:

A source whose URL slug coincides with an already-registered declared name fetches into that marketplace's cache. deriveMarketplaceSlug maps ./foo, /foo and ../foo all to foo, so marketplace add /foo while a marketplace declared foo is registered (from any other source) fetches straight into .state/cache/marketplaces/foo, then moves that tree away to the new source's declared name. The registered foo keeps its marketplaces/foo.toml and state record with no cache behind them. The base did exactly this (fetch into the slug dir, then rename it away); #260's fix does not change it, and on the failure path its discard of the fetched tree removes that same directory.

A fetch into a temporary directory that is only slotted under a name once the declared name is known (part of the marketplace.FetchInto shape proposed above) would close it: the slug would never be a cache directory name at all.

---

Generated by Claude Code

[spxrogers]

One more orphan shape for the cleanup item, from #260 round 3: swapDir (internal/cli/plugin_poll.go, shared by the marketplace re-slot and the plugin upgrade cache swap) is now rename-aside — the old tree moves to <cache>..old while the new one moves in, and the aside is discarded afterwards. A process killed between the two renames leaves <name>..old in the cache root until the next swap of that name clears it, and searchAllMarketplaces would offer such a leftover to a bare-id plugin add exactly like a slug orphan (the same is already true of extractSubdir's .subdir_old in internal/marketplace/fetch_git.go). A sweep of the cache roots should treat any entry whose name holds .. (which sanitizeCacheKey never produces) as scratch, and the marketplace lookup could skip such entries outright. The swapDir doc-wording item from this issue's first comment is done in that round.

---

Generated by Claude Code

[spxrogers]

Two updates from #260 round 4:

• The aside is now skipped by every cache-root scan. marketplace.CacheAsideSuffix (<dir>..old) and marketplace.IsCacheAside (internal/marketplace/cache_aside.go) define it once; searchAllMarketplaces, buildMarketplaceIndex and the direct scan in resolveInstalledEntry skip any entry whose name holds ... So the ..old orphan shape from my previous comment is closed for lookups; the cleanup item here still applies to the aside as a leftover directory (a sweep should delete it), and to extractSubdir's .subdir_old/.subdir_tmp, which the scans do not skip (their names are legal cache-key shapes).
• .golangci.yml's comment on the plugin_poll.go whole-file exclusion is stale. It says "its six os.* sites"; the count was already ten at the base of fix(cli): marketplace add fails instead of registering an unfindable cache (#233) #260 and is fifteen after swapDir's rename-aside shape. Three review lenses flagged it; the PR does not touch .golangci.yml (nor should a fix PR), so the number wants dropping or refreshing here, along with the "temp fetch dirs + the swap-in rename" description, which should now say the swap parks the old tree aside and restores it on failure.

---

Generated by Claude Code