fix(cli): a batch a CDN refuses is split before it falls back - #43
Merged
Merged
Conversation
A CDN answers a batch of packages as a whole, so one spec its generator can't build was a "no" for every package beside it: `bin/importmap update` with three outdated packages reported `Couldn't find any packages in ["cheap-ruler", "mapbox-gl", "mermaid"] on jspm`, updated nothing and exited 0. Commenting out the one package let the other two through. The provider chain made half of that worse rather than better. It handed the whole spec list to the next CDN, so `md5` — vendored from jspm and perfectly healthy — was silently re-pinned from esm.run and had its provenance comment rewritten because `mermaid` failed beside it. A refused batch holding more than one spec is now asked for one spec at a time, along the path that spec would have taken alone: through the chain when the group named no CDN, of the same provider once when it did. Only the package the CDN actually refused travels the rest of the chain. The batch stays the fast path — one round trip, and jspm still resolves the specs' shared dependencies as a single graph — because it is split only on refusal. `pin`, `update` and `pristine` exit 1 when a package was left unresolved, so `bin/importmap update && git commit` can't commit an import map that quietly missed one. The packages that did resolve are still written. The work lives in Importmap::BatchResolver, a fork-only collaborator, because commands.rb is upstream-owned and packager.rb is at its size limit. commands.rb is a require, a delegation, a memoised reader and three `||` clauses shorter than it was; upstream's for_each_import and unpin are untouched. ## Test coverage - test/batch_resolver_test.rb: which requests each shape makes, that the refused spec alone changes CDN, that an unreachable CDN for one spec doesn't stop the next, and both halves of the shared-dependency rule - test/commands_test.rb: the live jspm contract for the split, both with the fallback chain and with --from jspm, including the exit code and the provenance comment each package ends up with Refs #34 Fixes #32 Claude-Session: https://claude.ai/code/session_01B8NAp4J4RS7BBUDDQtRA7W
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A CDN answers a batch of packages as a whole, so one spec its generator can't build was a "no" for every package beside it.
bin/importmap updatewith three outdated packages printedCouldn't find any packages in ["cheap-ruler", "mapbox-gl", "mermaid"] on jspm, updated nothing and exited 0.The provider chain made the other half worse: it handed the whole spec list to the next CDN, so a healthy
md5was silently re-pinned from esm.run and had its provenance comment rewritten becausemermaidfailed beside it.Batch first, then split. A refused batch holding more than one spec is now asked for one spec at a time, along the path that spec would have taken alone — through the chain when the group named no CDN, of the same provider once when it did. Only the package the CDN actually refused travels the rest of the chain.
pin,updateandpristinenow exit 1 when a package was left unresolved; the packages that did resolve are still written.The batch stays the fast path — one round trip, and jspm still resolves the specs' shared dependencies as a single graph — because it is split only on refusal, and only when it holds more than one spec.
Files
lib/importmap/batch_resolver.rb— new, fork-only. All the new behaviour, becausecommands.rbis upstream-owned andpackager.rbis at its size limit.lib/importmap/commands.rb— net −12 lines: a require, a one-line delegation infor_each_import_grouped_by_provider, a memoisedresolverreader, and three|| resolver.unresolved.any?clauses.for_each_import_with_fallback(fork-added) is deleted; upstream'sfor_each_import,handle_package_not_foundandunpinare untouched.lib/importmap/packager.rb— unchanged.pinning.rb,updating.rb,cli.rb,upgrading.rbCHANGELOG.mdunder the existing## 1.2.0→### Fixed.VERSIONis not bumped — 1.2.0 is already inversion.rband unreleased.Closes #32
Closes #34
Test plan
bundle exec rake test— 459 runs, 0 failures, with bun installed so the--minifycases ran (2 runs, 0 skips)bundle exec ruby -Itest test/batch_resolver_test.rb— 9 new cases, each seen failing first; every case asserts both what was yielded and what was recorded unresolvedbundle exec ruby -Itest test/provider_chain_test.rb— unchanged, green (ProviderChain's contract is not touched)cd docs && bundle exec rake lint && bundle exec rspec— 53 files clean, 54 specs passtest/dummy:bin/importmap pin md5@2.2.0 mermaid@10.6.0→ exit 0,pin "md5" # @2.2.0(no provider — jspm),pin "mermaid" # @10.6.0 (esm.run), both vendoredbin/importmap pin md5@2.2.0 mermaid@10.6.0 --from jspm→ exit 1,md5still pinned,mermaidabsent, notrying esm.runin the outputThree existing live cases now exit 1 and were switched to
run_importmap_command_expecting_failure; their assertions are unchanged.No live
updatecase: a live update targets the registry's moving latest, so whether jspm can build it is not a stable contract.updateshares the code path and is covered by the unit tests.Deviations & judgment calls
Discoveries
lode/no longer exists in this repo (reverted in Revert "chore: seed the lode and enable the pre-PR gate" #33), so step 7 of issue fix(cli): one package a CDN can't resolve no longer blocks or moves the rest of the batch #34 (/lode:sync,/lode:gate, readinglode/review/*) is moot and was skipped. The same rules live in.claude/rules/.commands.rbis 812 lines, not the 641 the issue names, andskipped+exit 1already exist inpin,pristineandupdate. Step 3's exit codes were folded into the existing lines rather than added.Judgment calls
on_miss:callable, not a class method. The issue offered either. A callable leaveshandle_package_not_found— upstream-owned — completely untouched, so the "couldn't find" sentence still lives in exactly one place.Packager::Erroron the batch splits; it isn't only a 401/404 that does. In fallback mode the batch is asked of jspm directly rather than through the chain, so a jspm transport failure has to reach esm.run somehow — splitting sends each spec down the chain, which handles errors properly. A named-provider batch splits for the same reason: a transport failure is evidence about the run, not about any one spec.resolve_onerescuesPackager::Errorfor a single-spec group too. The issue scoped the rescue to "the specs after it". Extending it means a raised error becomesCouldn't resolve "x" from jspm: …plus exit 1 instead of a Thor backtrace, and one provider group can no longer abort the next.@named/@seen) are scoped per group, not per run. The issue's rule is about split responses, which happen inside one group. Per-run scoping would silently change how two provider groups that share a dependency behave today —updateofphotoswipe+photoswipe/lightboxis exactly that shape.version()falls back to the whole URL when a URL carries no@x.y.z, so theKeeping …sentence can't printatwith nothing after it.FakePackagerwas not extracted into a shared file.provider_chain_test.rb's is keyed by provider; rekeying its thirteen cases to[ from, specs ]is noise for no gain.batch_resolver_test.rbhas its own, which delegatespackage_key_forandextract_package_version_fromto a realPackagerso the fake can't drift from it.https://claude.ai/code/session_01B8NAp4J4RS7BBUDDQtRA7W
Summary by cubic
Fixes batch package resolution so a CDN refusing one spec no longer blocks (or silently moves) the rest. Previously the whole batch was answered as one request, so one unresolvable spec failed every package with exit 0; now the batch is split on refusal and only the refused spec falls back.
Behavior
pin,update, andpristineexit 1 when any package is left unresolved, while still writing the ones that succeeded.Written for commit 6fbdf82. Summary will update on new commits.