refactor(datastore): extract the shared ListRemotes URL-usable gate
🎯 What this MR does
Extracts the URL-usability gate ContainerRemoteRepositoryStore.ListRemotes applies — refuse a row whose stored URL remote.ParseUpstreamBase rejects, sample up to 5 distinct refusal reasons, log one summary line per call — into a shared refsWithUsableURL helper in the new internal/datastore/remote_repository_enumeration.go. Container is the only caller today; this is prep for an npm-format store that needs the identical gate, landing as a separate MR in the stack below. No behavior changes for container: its message text, field set, and sample bound are byte-for-byte unchanged.
Related to below for the tracking item):
| Step | MR | What it delivers |
|---|---|---|
| 0 · this MR | Shared refsWithUsableURL enumeration helper (this MR) |
|
| 1 | !1481 (merged) | npm datastore health seams |
| 2a | !1482 (merged) | npmremote.Target + UpstreamRequestBuilder |
| 2b | !1483 (merged) | npmremote.CacheStore + health-sweep glue |
| 3 | !1484 (merged) | Composition root: wiring, audit sink, breadcrumb |
🧩 The change
ContainerRemoteRepositoryStore.listRemotes used to filter and log inline: loop the rows, call remote.ParseUpstreamBase per row, accumulate a bounded sample keyed by refusal reason, and emit one WARN naming the skip count. That logic is now refsWithUsableURL(ctx, tablePrefix, refs): the store maps its jet rows onto remote.RemoteRepositoryRef first, then hands the slice to the shared gate, which returns only the usable ones and logs the same summary it always did — under a tablePrefix argument (containerRemoteTablePrefix here) so each future caller's line stays greppable on its own.
skippedRemoteRow, skippedRemoteRows, logSkippedRemoteRows, and maxListRemotesSkipSample moved verbatim into the new file; logSkippedRemoteRows gained the tablePrefix parameter.
The prefix is a named type, remoteTablePrefix, whose members are declared in the same file, and skipSummaryMessage maps a member to its text rather than interpolating the argument. Its underlying type is an integer, so neither naming a family with a literal nor converting a row value into one compiles — the greppable prefix is declared in this file, not supplied at a call site. A value that names no declared member takes the unprefixed message and carries no text of its own. Container's line is byte-for-byte unchanged. The AppSec review is answered in its own thread.
A note for !1481 (merged): adding npm's caller means adding its remoteTablePrefix member and its skipSummaryMessage arm, and neither can be skipped quietly. A bare literal does not compile, and a member declared without an arm fails the lint: .golangci.yaml enables exhaustive with default-signifies-exhaustive: false, so the default clause does not make the switch look exhaustive. What is left to add there is the store-side message assertion, the way TestContainerRemoteRepositoryStore_ListRemotes_SkipSummary now does — only a store's own suite can show it passes its own prefix rather than a copied one.
✅ Verification
- Full
ContainerRemoteRepositoryStoresuite passes (11/11, includingTestContainerRemoteRepositoryStore_ListRemotes_SkipSummary,..._SkipSummary_DedupesByReason, and..._PlanShape) — the refactor is behavior-preserving by construction and by test...._SkipSummarynow also asserts the rendered message, which is the one claim neither the compiler norexhaustivecan make: that this store passes its own prefix rather than a copied one or the zero value. Verified by mutation — passing0fails the assertion. refsWithUsableURLalso has direct coverage now, ininternal/datastore/remote_repository_enumeration_test.go(7 tests, no database): the usable refs come back whole and in arrival order, the skip count is exact, the sample stops atmaxListRemotesSkipSampleacross more distinct reasons than the bound, many refs sharing one reason collapse to one entry, a call that skipped nothing or enumerated nothing logs nothing, the container message text is asserted whole, andskipSummaryMessageis pinned for a declared member and for the values that remain reachable once the type is an integer — zero, out of range, negative. The summary is read back through a capturingslogJSON handler, so the sample is asserted against the encoding that ships rather than the Go value.- The two guards the type leans on were checked, not assumed: a bare literal prefix and a
remoteTablePrefix(row.URL)conversion are both compile errors, and adding a second member without askipSummaryMessagearm producesmissing cases in switch of type datastore.remoteTablePrefix (exhaustive). go build ./...,go vet ./internal/datastore/...,golangci-lint run --max-same-issues=0 --max-issues-per-linter=0 ./internal/datastore/...: clean.golangci-lint run --build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 ./internal/datastore/...: clean on both changed files.
🧪 E2E scenario impact
None: pure internal refactor, no request-path or protocol behavior change. No files under docs/testing/ reference this code path.
Related to #346 (closed)