A single DOIGET_*_BASE override silently rebuilds the HTTP client from six source keys, dropping the rest

[sotashimozono]

Answers the question left at the end of #462: "Worth deciding whether that wants its own issue or belongs on #454." Its own issue — the shape is #454's, but the mechanism is different and it is in both client builders, not one.

What happens

build_http_client_for_fetch (doiget-mcp/src/lib.rs) and build_http_client (doiget-cli/src/commands/fetch.rs) are hand-maintained twins with the same two halves:

if arxiv.is_none() && arxiv_src.is_none() && crossref.is_none()
    && unpaywall.is_none() && oa_publisher.is_none()
    && openalex_base.is_none() && ar5iv_base.is_none()
{
    // ... tier_1 + oa_publisher + tier_2 + fulltext + tier_3 + user-extension
    return HttpClient::new(allowlists);
}
// otherwise: rebuild from scratch
for (source, base) in [
    ("arxiv", …), ("crossref", …), ("unpaywall", …),
    ("oa-publisher", …), ("openalex", …), ("ar5iv", …),
] { … }
Ok(HttpClient::new_for_tests_allow_http_multi(&entries))

Any one of the seven sends you down the second path, and the second path knows about six source keys. Everything else is gone — not overridden, absent from the client's map, so the next request under one of those keys is HttpError::UnknownSource.

Measured against what the production branch registers:

dropped on override	where it comes from
datacite, hal, openaire, core, europe-pmc	tier_2_allowlist()
tdm-aps, tdm-elsevier, tdm-springer, tdm-ieee	tier_3_allowlists()
every user-curated host	ADR-0028 D2 config merge

And the two halves have drifted from each other as well as from the orchestrator, which honours ten base overrides (grep -o 'optional_base("DOIGET_[A-Z_]*' crates/doiget-core/src/orchestrator.rs): APS, CORE, DATACITE, ELSEVIER, EUROPE_PMC, HAL, IEEE, OPENAIRE, OPENALEX, SPRINGER. DOIGET_APS_BASE appears in neither the condition nor the list, so it can be set and honoured by the source while the client has no allowlist for the key it will use.

How it surfaced

The #[ignore]d Tier-3 reproduction added by #577 (doiget-mcp/tests/fetch_paper_e2e.rs). It sets DOIGET_ARXIV_BASE and DOIGET_APS_BASE; the first flips to the override path, the second is not in it:

"detail": "network error: no allowlist registered for source tdm-aps"

That is why the suite cannot cover tdm_fetched — the one route the Tier-3 tier exists for.

Severity

Not a production break. Nothing sets a DOIGET_*_BASE in normal use, so the production branch runs and every key is registered. It bites (a) the e2e suite, which is where #462 wanted coverage, and (b) anyone pointing doiget at a mirror or proxy for one source, who silently loses every other source's allowlist.

The fix worth making

Not "add the missing keys to the list" — that leaves three hand-maintained lists to keep in agreement, which is the defect. The override should be additive: build the production allowlists as usual, then swap in the mock host (and the plain-HTTP-permitting client) for whichever sources have a base set. Nothing is dropped, so nothing has to be kept in sync.

That needs a constructor in doiget-core::http alongside new_for_tests_allow_http_multi, and both twins rewired to it. Expanding the condition instead would be a regression: today DOIGET_APS_BASE alone keeps the production path and works.

[sotashimozono]

Amending my own recommendation before anyone builds it. The additive fix has a cost I did not account for, and it is not obviously worth paying.

Dropping everything is also a test-isolation property. Today, a test that mocks Crossref and then reaches an unmocked source dies immediately on UnknownSource. Make the override additive and that source gets a real production client instead — so the test issues a live request to the real API. A loud, instant, offline failure becomes a slow one that depends on the network, and in the worst case a test quietly starts exercising a third party. That is a bad trade for a suite that runs on every PR.

So the fork is:

1. Additive — nothing is dropped, no lists to keep in sync, but an unmocked source in a test becomes a live call. Would need its own guard to stay safe, and a guard that says "no network for unlisted sources" is close to re-deriving the current behaviour.
2. One table, still exclusive — keep rebuilding from the overrides alone, but generate the selection condition and the registration list from a single table covering all ten DOIGET_*_BASE variables. Test isolation is preserved exactly. The cost is that the table's membership becomes load-bearing: a source whose base is not in it can never be mocked.

(2) now looks right to me, but it needs one thing checked first that I did not check: whether DOIGET_APS_BASE alone works today. The claim in the issue body — that it "keeps the production path and works" — is inference from reading the condition, not a measurement. If it does not actually work (the production tdm-aps allowlist is harvest.aps.org / *.aps.org, which a localhost mock does not match), then adding APS to the table is not a regression and (2) is straightforwardly correct.

Not guessing at that one. It is the same mistake as #583, where I read one function, stopped before the one that mattered, and filed a wrong claim.

[sotashimozono]

Partially addressed in 0.8.13, and the part that shipped was found by walking into it from #462 rather than from here.

What shipped

Both twins now register the four Tier-3 keys in the override branch, each behind its own #[cfg(feature = "tdm-*")]:

• crates/doiget-mcp/src/lib.rs — build_http_client_for_fetch
• crates/doiget-cli/src/commands/fetch.rs — build_http_client

Deliberately not added to the production-branch condition: setting only DOIGET_APS_BASE — which tdm_ieee.rs documents as the way to replay a recorded fixture — must not drop the process into the allow-http test client. That asymmetry is now commented at both sites.

This is what made the #462 reproduction pass, so the route it covers is real coverage rather than a source-text claim.

What did NOT change, and is still the issue

The mechanism you named is untouched. The override branch is still an enumerated table, so it still silently drops every source key nobody remembered to list. The fix moved the Tier-3 keys from "missing" to "listed"; it did not stop the next key from going missing the same way. And the two builders are still hand-maintained twins — I edited both, by hand, from the same clipboard.

Concretely still true:

• tier_2_allowlist() (openalex / semantic-scholar / doaj) contributes only openalex to the override table. semantic-scholar and doaj are absent, so no wiremock test can reach either.
• fulltext_allowlist() contributes ar5iv and nothing else.
• The user-extension hosts from config.toml (ADR-0028 D2) are merged in the production branch and dropped entirely in the override branch.

Also worth knowing

Only the tdm-aps row is proven. The elsevier / springer / ieee rows were added in the same edit and nothing exercises them — a typo in any of those three key strings would be invisible. That gap is recorded on #462 as well.

The structural fix — deriving the override table from the same source the production branch uses, instead of a parallel list — is what would actually close this.