Give mirrored packages a README, and make os-universal builds downloadable - #158
Conversation
…al builds Every ingested version records `readme: null`, so a mirrored package renders as an empty page. The publish path fills it from the GitHub API at the release tag; the ingest path had no equivalent and never set the column. Two sources, in that order: The archive itself wins. `inspectBundle` already opens the zip and walks every entry for the manifest and the scanability histogram, so reading a root README costs nothing and describes the exact bytes whose digest was verified. Only the root is considered: a bundle vendors dozens of dependency READMEs, and rendering one of those on the package page would be worse than rendering none. Of 33 mirrored bundles on staging, 14 ship one. For the rest, the repository upstream declares is the only description that exists, read from `raw.githubusercontent.com` at the tag recovered from the release-asset URL, falling back to the default branch. Deliberately not the GitHub API: it allows 60 requests/hour unauthenticated, the registry holds no token, and that budget is shared with repo-stat fetches and with claim verification. A nightly pass over the upstream catalog would consume it in the first minute and starve a user-facing flow. Every failure returns null — a mirror with no description is far better than a run that abandons a server because someone's repository moved. Separately, `resolveArtifact` matched a platform exactly, which left an entire artifact shape unreachable. `arch: 'any'` is a stored value, not a query convention: `server-linux.mcpb` declares an OS and no architecture and is recorded as `linux/any`. No caller can name it — the SDK's `detectPlatform` reports the concrete runtime and the query schema admits no `any` — so a package whose every artifact has that shape appears in the catalog and 404s on every download. Two of the 38 mirrored packages are in exactly that state. Resolution now falls back by decreasing specificity: exact, then the os-universal build, then the fully portable one. It stays a 404 when the requested OS has no build at all, so a linux-only package never hands a win32 caller a linux binary. Verified against real mirrored artifacts rather than fixtures: the blackduck bundle yields its own 5,626-char README with the vendored ones ignored, and knowtis — which ships none — resolves through the repository fallback at the tag it was published at.
…lback
upsertVersion's update clause omitted `readme`, so Prisma left the column
untouched whenever the row already existed. Both callers reach that path
holding a README they have already paid for — ingest by opening an archive it
downloaded, the publish route by spending one of 60 hourly GitHub calls behind
an `if (!existingVersion?.readme)` guard that only fires on update — so the
omission discarded the result of every one of those spends. Same class of drift
as the empty `update: {}` this repository already fixed once.
`undefined` still means "nothing to say about this field" rather than "clear
it", so a bundle that stops shipping a README cannot blank a description an
earlier run recorded.
resolveArtifact's comment claimed callers never send `arch=any`. The SDK's
detectPlatform falls through to `any` for any platform outside {darwin, win32,
linux} × {x64, arm64}, and getBundleDownload sends both params unconditionally,
so a 32-bit ARM client sends `arch=any` and is rejected by the query enum
before resolution runs. The comment now says what is true — an exact-match
caller cannot reach an os-universal build — and names the enum as a separate
defect rather than asserting it away.
Also: the repository-slug guard checked shape but not `..`, and `../..`
satisfies the shape (both segments are legal characters). Harmless in practice,
since the host is fixed and URL normalisation makes it a 404, but githubSlug
regex-matches an upstream-controlled URL instead of parsing it, so the value
does arrive. Excluded explicitly, matching the ref guard.
`README_PATHS` gains the lowercase spelling: the in-bundle lookup compares
case-insensitively, but these are URL paths on a case-sensitive host, so
`readme.md` and `README.md` are different requests and a repository using the
former yielded nothing. Root-entry matching now tolerates the `./` prefix some
archivers write. The unused `timeoutMs` option is gone — one bound fits a
single small file, unlike the artifact download it was mirroring.
|
Round 2 adjudicated. Both Criticals were real and are fixed in Critical 1 — README discarded on the update path. Confirmed, fixed. Fixed with the conditional spread, matching the surrounding style. The point about the test not being able to catch it was correct, so I verified the new one is load-bearing rather than assuming: with the fix reverted, Critical 2 — the comment's claim is false. Confirmed, comment corrected. Agreed on charter: the enum widening and the SDK 422 are pre-existing and stay out of this PR. Filed as #159 — including the worse case you didn't name, that a FreeBSD/ppc64 client sends Delete Instead — Suggestions 2, 3, 5 — taken. Case-sensitivity verified live rather than reasoned about: Suggestion 4 — taken. Suggestion 1 — real, deferred with measurement. Filed as #160. You flagged it as non-blocking on a 38-bundle sample; I scanned the full upstream catalog instead — 188 pages, 18,779 servers, 682 MCPB artifacts:
The one instance is So the root fix belongs in 255/255 tests pass, typecheck and biome clean. |
…h in bytes `zip.readAsText(entry) || null` kept a whitespace-only README, because that string is truthy. It then won the source contest in ingest — `downloads.find( d => d.readme)` matched it — and suppressed the repository fallback, so the package rendered an empty Markdown block. That is precisely the symptom this change exists to remove, arriving by the preferred path. The repository source already refused blank content. The guard was on the path that loses; both sources now agree that whitespace is absence. The fetch cap compared `text.length` against a byte constant. That counts UTF-16 code units, so non-ASCII prose could exceed the stated 512 KB several times over while the check claimed to enforce it. Now measured with `Buffer.byteLength`, matching both the Content-Length pre-check and the byte count the in-bundle side reads off the zip header.
|
Round 3 adjudicated. All three Criticals were real. Two are fixed in Critical 1 — a failed README resolution is permanent, and the body claimed otherwise. Confirmed, body corrected, lever filed as #161. Verified the mechanism rather than the summary: The body sentence was overclaiming. "Production has ingested nothing yet, so every row it ever writes gets a README from the start" is only true if resolution never fails; what a row actually gets is whatever was resolvable at the instant of first write, permanently. The 10-sequential-unauthenticated-requests point is the sharp end and is right — I raised that fanout from 8 to 10 in round 2 by adding Agreed on not fixing it inline, and for the reasons given — I reached the same two dead ends. #161 records both rejected remedies so they aren't re-proposed, and suggests the shape that avoids both: a targeted repair over Critical 2 — blank in-bundle README suppresses the fallback. Confirmed, fixed. Reproduced before fixing: Fixed with the one-line form. Mutation-checked: reverting it reds the new test and nothing else. (My first attempt at that check silently failed to apply the mutation and reported a false pass — re-ran it properly rather than trusting the green.) Critical 3 — the sentence is still in the body. Confirmed; my round-2 comment was wrong. I corrected the code comment at Suggestion — byte cap. Taken. 257/257 tests, typecheck and biome clean. Follow-ups now #159, #160, #161. |
1 participant
Mirrored packages render as empty pages.
@blackduck/mcp-serverand@knowtis/knowtison staging both show no description at all, and every one of the 38 ingested versions hasreadme: null.Why it was empty
The publish path fetches a README from the GitHub API at the release tag (
bundles.ts); the ingest path has no equivalent and never set the column. Not a regression — it was never wired.README, from two sources
The archive wins.
inspectBundlealready opens the zip and walks every entry for the manifest and the scanability histogram, so reading a rootREADME.mdcosts no extra network and describes the exact bytes whose digest was verified. Root only: bundles vendor dozens of dependency READMEs, and renderingnode_modules/left-pad/README.mdon a package page is worse than rendering nothing.I measured the corpus by pulling all 38 staging bundles and listing each archive: 14 ship a root README, 19 don't, 5 I couldn't resolve.
The declared repository covers the rest, read from
raw.githubusercontent.comat the tag recovered from the release-asset URL, falling back to the default branch.Deliberately not the GitHub API. It allows 60 requests/hour unauthenticated,
github-verifier.tssends noAuthorizationheader and noGITHUB_TOKENexists in config, and that single budget is shared with repo-stat fetches and with claim verification — a user-facing flow. A nightly pass over ~305 servers would consume it in the first minute and starve them. The raw host is a CDN and is not charged against it, so this needs no token and no new config.Best-effort by construction: every failure returns null. A mirror with no description beats a run that abandons a server because someone's repository moved.
Separately: an artifact shape nobody can download
While measuring the corpus I found 16 of 38 packages 404ing. Most were my own error (the route needs a version), but two are real and permanent:
arch: 'any'is a stored value, not a query convention —inferPlatformmapsserver-linux.mcpb(an OS token, no arch token) tolinux/any. No caller can name it:resolveArtifactlooks forany/any→ 404os=linux&arch=any["x64","arm64"]os=linux&arch=x64So the package appears in the catalog and 404s on every download. A caller on a supported platform asks for its concrete pair, so the exact-match lookup never reaches an
os/anybuild. (Asking forarch=anydirectly is not a way in either — the query schema admits onlyx64/arm64and rejects the request before resolution runs. That rejection also strands the SDK on platforms outside those enums, which is a separate defect: #159.)Fixed by widening
resolveArtifactto fall back by decreasing specificity — exact, then the os-universal build, then the fully portable one. Widening the query enum instead would not have helped, since no real client asks forarch=any. It stays a 404 when the requested OS has no build at all, so a linux-only package never hands a win32 caller a linux binary.Verification
Against real mirrored artifacts, not fixtures:
The two packages from the report resolve through a different source each.
257/257registry tests pass, typecheck clean, biome clean. 34 new tests cover root-only matching, case, preference order, the oversize drop, tag-then-HEAD ordering, the CDN-not-API constraint, path-traversal refusal on both the slug and the ref, and each platform-resolution branch.One fake needed updating:
fakeUpstreamcounted everyfetchas an artifact download, and those counters are how the ingest tests prove idempotence. It now routes by host so a README fetch can't be mistaken for a re-download.Scope note
This fills the column going forward. It does not backfill staging's existing 38 rows — even
--fullskips them, becauseversionIsCurrentshort-circuits on matching digests before any download, and--fullonly widens thesincewindow.That same skip makes a failed README resolution permanent, which is worth stating plainly rather than implying otherwise: a row gets whatever was resolvable at the instant of first write, and no later run revisits it.
fetchOnecollapses 404, 429, 5xx, timeout, and DNS failure into the samenull, so a throttled response is indistinguishable from a genuine absence — and the fallback issues up to 10 sequential unauthenticated requests per README-less server, which is the shape that draws throttling. Staging can be re-ingested from empty; production has ingested nothing yet, so nothing is stuck today. The repair lever is filed as #161 rather than built here: gatingversionIsCurrentonreadmewould re-download half the catalog nightly forever, and a bespoke full-refresh option is net-new surface for a path used twice a year.