fix(registry): invalidate stale documentation builds automatically - #178
TheRealBecks wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 0ea0b0f The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![human0-bot[bot]](https://avatars.githubusercontent.com/in/3310509?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approved — the fingerprint inputs and shared publish flow look sound; no blocker prevents landing.
Data & state — high risk, comment: freshness depends on server metadata being updated on replacements; retries and --force have edge cases worth confirming.
Behavior — high risk, comment: missing upstream refs can now fail an existing bulk publish, and the skip summary mislabels unpublished-tag skips.
Product & UX — low risk, comment: bulk skip logs can overwhelm nightly output; non-409 failures also leave artifacts without a path message.
Maintainability — high risk, comment: peer-suffixed lock entries can break revision generation; add parity coverage and simplify the long traversal.
Simplicity — low risk, comment: source resolution and build summaries are duplicated, while the generator invalidates all task caches and hashes an irrelevant ZIP input.
This codebase is managed by Human0.
| | `name` | string | Package name | | ||
| | `version` | string | Semver version or `"latest"` | | ||
| | `source_commit` | string? | Git SHA for unversioned packages | | ||
| | `source_commit` | string? | Git commit SHA for versioned or unversioned Git packages | |
There was a problem hiding this comment.
Data & state · comment: The freshness check depends on the server copying build_fingerprint and ingestion_revision from the uploaded database into metadata on every upload and replacement. This checkout has no server implementation, and the test is a mock; track or verify server adoption before relying on automatic invalidation.
This codebase is managed by Human0.
| { | ||
| method: "POST", | ||
| headers: { | ||
| Authorization: `Bearer ${getPublishKey()}`, |
There was a problem hiding this comment.
Data & state · question: requestWithRetry resends the POST after a dropped response. If the first upload landed but its response was lost, a server that rejects replacement can return 409 on the retry, making the client report failure although the version is already published. Is that replacement policy idempotent, or should the client verify metadata after this retry case?
This codebase is managed by Human0.
| "Only the N most recent minor versions per package", | ||
| ) | ||
| .option( | ||
| "--force", |
There was a problem hiding this comment.
Data & state · question: --force bypasses fingerprint checks but does not invalidate the HTML-index source cache. A forced rebuild can therefore reuse stale downloaded pages instead of fetching corrected content; clarify that cache refresh is separate or invalidate it here.
This codebase is managed by Human0.
| const source = resolveBuildSource(definition, version); | ||
| // An unversioned archive has no immutable source revision to compare. | ||
| if (!isVersioned(definition) && source.type === "zip") return; | ||
| const commit = |
There was a problem hiding this comment.
Behavior · comment: For an already-published versioned Git package, this now resolves the upstream ref before skipping. If that repo or tag was removed, publish-all records a hard failure and exits instead of skipping the existing version. This is a deliberate new failure mode; consider whether nightly publishing should fail for an unresolvable old source.
This codebase is managed by Human0.
| console.log(`\n--- Summary ---`); | ||
| console.log(`Succeeded: ${succeeded}`); | ||
| console.log(`Skipped (already published): ${skipped}`); | ||
| console.log(`Skipped (up to date or legacy metadata): ${skipped}`); |
There was a problem hiding this comment.
Behavior · comment: This summary also counts the “git tag not published yet” skip branch, which is neither up-to-date nor legacy metadata. Split the counter or use a neutral “Skipped” label so the reported reason matches the count.
This codebase is managed by Human0.
| } | ||
|
|
||
| /** Only the entry selected for this release contributes to its fingerprint. */ | ||
| export function resolveBuildSource( |
There was a problem hiding this comment.
Simplicity · comment: resolveBuildSource re-derives ref, url, and docs_path already resolved separately in buildFromDefinition. Share one resolver so the fingerprint cannot drift from the inputs actually built.
This codebase is managed by Human0.
| const result = isVersioned(definition) | ||
| ? await buildFromDefinition(definition, version, outputDir) | ||
| : await buildUnversioned(definition, outputDir); | ||
| const skipped = |
There was a problem hiding this comment.
Simplicity · comment: This section/token/file summary duplicates formatBuilt in cli.ts; reuse the existing formatter so the two command summaries stay consistent.
This codebase is managed by Human0.
| @@ -1,5 +1,6 @@ | |||
| { | |||
| "$schema": "https://turborepo.com/schema.json", | |||
| "globalDependencies": ["scripts/generate-ingestion-revision.mjs"], | |||
There was a problem hiding this comment.
Simplicity · comment: Making the generator a global Turbo dependency invalidates lint, tests, and builds across every package, although only the registry build consumes this output. Scope the input to registry tasks.
This codebase is managed by Human0.
| ref: source.type === "git" ? (source.ref ?? "HEAD") : undefined, | ||
| docs_path: "docs_path" in source ? source.docs_path : undefined, | ||
| exclude_paths: [...new Set(source.exclude_paths ?? [])].sort(), | ||
| lang: "lang" in source ? (source.lang ?? "en") : undefined, |
There was a problem hiding this comment.
Simplicity · question: ZIP ingestion ignores lang, but the fingerprint includes it, so changing only that field rebuilds an unchanged ZIP package. Is that intentional, or should the ZIP fingerprint omit it?
This codebase is managed by Human0.
| return value; | ||
| } | ||
|
|
||
| export function generateIngestionRevision(root = repository) { |
There was a problem hiding this comment.
Simplicity · question: The generator implements a substantial AST, barrel-export, dynamic-import, and lockfile traversal to discover ingestion dependencies. Is automatic discovery worth this machinery over an explicit maintained list plus lockfile hashing? The automatic benefit is real, so this is a design choice, not a blocker.
This codebase is managed by Human0.
|
Thanks for the work on this. Could you please update the PR for these remaining cases before it can land?
This codebase is managed by Human0. |
1 participant
Documentation could remain stale after definition or parser changes because publication only checked whether a version existed or its source commit matched.
This change adds deterministic build fingerprints covering the resolved source revision, effective definition, and an automatically generated ingestion revision. Both publish commands skip matching fingerprints and rebuild changed inputs.
The ingestion revision follows helper imports and locked dependencies automatically. It is embedded in the build output, so runtime fingerprinting needs no source checkout.
Adds --force for explicit rebuilding, clear HTTP 409 conflict reporting, and preservation of failed upload artifacts. Includes regression coverage and migration documentation.
Servers must expose build_fingerprint from uploaded package metadata to enable automatic checks. Older servers retain their existing skip behavior. Unversioned ZIP sources continue rebuilding because they lack an immutable revision.
Validation: lint, production builds, and 390 tests passed.
Fixes #144.