fix(changelogs): SBOM null-fallback, GHCR tag enumeration, cache key …#743
Conversation
There was a problem hiding this comment.
Code Review
This pull request migrates the SBOM data source from GitHub Releases to the GHCR OCI distribution API and fixes a bug where version chips disappeared when SBOM data was incomplete. The fix involves implementing a fallback to release notes versions within the enrichment logic across multiple files. Additionally, the PR introduces comprehensive unit and E2E tests to ensure data consistency. Feedback was provided to improve the robustness of the GHCR tag pagination logic by handling relative URLs and more flexible Link header formats to comply with the OCI distribution spec.
| const nextMatch = linkHeader.match(/<([^>]+)>;\s*rel="next"/); | ||
| url = nextMatch ? nextMatch[1] : null; |
There was a problem hiding this comment.
The pagination logic for GHCR tags is fragile and may fail in two scenarios:
- Relative URLs: The OCI distribution spec allows the URI in the
Linkheader to be relative. Passing a relative URL directly tofetch()in Node.js will throw aTypeError. - Parameter Order and Flexibility: RFC 5988 specifies that link parameters can appear in any order and values can be tokens or quoted-strings. The current regex
/<([^>]+)>;\s*rel="next"/only matches ifrel="next"immediately follows the URI and is exactly lowercased and quoted.
Using new URL(relative, base) and a more robust regex ensures compatibility with various registry implementations.
| const nextMatch = linkHeader.match(/<([^>]+)>;\s*rel="next"/); | |
| url = nextMatch ? nextMatch[1] : null; | |
| const nextMatch = linkHeader.match(/<([^>]+)>;[^<]*\brel=("?)next\2\b/i); | |
| url = nextMatch ? new URL(nextMatch[1], res.url).href : null; |
There was a problem hiding this comment.
Pull request overview
This PR fixes SBOM enrichment behavior so version chips don’t disappear when the SBOM has null package versions, and updates the SBOM ingestion pipeline to enumerate dated tags directly from GHCR (so daily builds without GitHub Releases are included). It also aligns the Pages workflow cache key with the nightly SBOM cache workflow and adds new tests around chip consistency and tag filtering.
Changes:
- Fix SBOM null-version fallback in both the UI feed (
FirehoseFeed.tsx) and card image generator (generate-card-images.mjs). - Replace GitHub Releases tag enumeration with GHCR tag listing (OCI distribution API) in
fetch-github-sbom.js, plus new tag-filtering tests. - Align SBOM cache restore key in
pages.yml, and add Playwright e2e coverage for changelog chip consistency.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/changelogs.spec.ts | Adds Playwright checks to ensure pinned stable cards render expected version chips/values. |
| src/components/FirehoseFeed.tsx | Fixes SBOM enrichment to fall back to release notes when SBOM package version is null. |
| scripts/generate-card-images.mjs | Mirrors the SBOM fallback fix for generated card images. |
| scripts/fetch-github-sbom.test.js | Adds unit tests for GHCR tag filtering via findRecentTagsForStream. |
| scripts/fetch-github-sbom.js | Implements GHCR tag listing + updates stream processing and tag filtering for SBOM pipeline. |
| scripts/enrichment.test.js | Adds unit tests documenting SBOM null-fallback behavior (currently includes a failing-by-design test). |
| .github/workflows/pages.yml | Updates SBOM cache restore key to match the key saved by update-sbom-cache.yml. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| test("buggy: flatpak chip disappears when SBOM has null (demonstrates the bug)", () => { | ||
| const result = enrichFromSbomBuggy(mockRelease, partialSbomPackages); | ||
| const flatpakChip = result.majorPackages.find((p) => p.name.toLowerCase() === "flatpak"); | ||
| // This assertion SHOULD fail with the buggy implementation | ||
| // (flatpak is filtered from nonSbomPackages AND skipped from sbomPackages) | ||
| assert.ok(flatpakChip, "flatpak chip must survive when SBOM has null — but it disappears (BUG)"); |
There was a problem hiding this comment.
This test is guaranteed to fail: enrichFromSbomBuggy() drops the flatpak chip when SBOM has null, but the assertion requires the chip to exist. If this file is executed by any test runner it will break CI/local tests; consider converting this to a test.skip/test.todo, or invert the assertion to explicitly assert the buggy behavior, and keep the passing assertions focused on the fixed implementation.
| test("buggy: flatpak chip disappears when SBOM has null (demonstrates the bug)", () => { | |
| const result = enrichFromSbomBuggy(mockRelease, partialSbomPackages); | |
| const flatpakChip = result.majorPackages.find((p) => p.name.toLowerCase() === "flatpak"); | |
| // This assertion SHOULD fail with the buggy implementation | |
| // (flatpak is filtered from nonSbomPackages AND skipped from sbomPackages) | |
| assert.ok(flatpakChip, "flatpak chip must survive when SBOM has null — but it disappears (BUG)"); | |
| test("buggy: flatpak chip disappears when SBOM has null (documents the bug)", () => { | |
| const result = enrichFromSbomBuggy(mockRelease, partialSbomPackages); | |
| const flatpakChip = result.majorPackages.find((p) => p.name.toLowerCase() === "flatpak"); | |
| // The buggy implementation drops flatpak entirely when SBOM has null | |
| // (flatpak is filtered from nonSbomPackages AND skipped from sbomPackages). | |
| assert.equal(flatpakChip, undefined, "buggy implementation drops the flatpak chip when SBOM has null"); |
| const month = dateStr.slice(4, 6); | ||
| const day = dateStr.slice(6, 8); | ||
| const publishedAt = `${year}-${month}-${day}T00:00:00Z`; | ||
| const publishedMs = Date.parse(publishedAt); |
There was a problem hiding this comment.
Date.parse(publishedAt) can return NaN for an invalid dateStr (e.g. tags containing an impossible date like 20260230). Because NaN < cutoff is always false, those tags would incorrectly be treated as "recent" and publishedAt would be invalid downstream. Add an isNaN(publishedMs) guard (and skip/ignore invalid dates) before applying the cutoff comparison.
| const publishedMs = Date.parse(publishedAt); | |
| const publishedMs = Date.parse(publishedAt); | |
| if (isNaN(publishedMs)) continue; |
| test("findRecentTagsForStream: picks stable-YYYYMMDD tags from GHCR list", () => { | ||
| const now = new Date(); | ||
| const recent = `${now.getFullYear()}${String(now.getMonth() + 1).padStart(2, "0")}${String(now.getDate()).padStart(2, "0")}`; | ||
| const ghcrTags = [ |
There was a problem hiding this comment.
This test is time-dependent (new Date()), which makes it non-deterministic and potentially flaky (e.g. around local/UTC day boundaries). Prefer using a fixed Date.now() stub (restore after the test) and a hard-coded tag date within the LOOKBACK_DAYS window so the assertion remains stable over time.
| test("findRecentTagsForStream: deduplicates same date", () => { | ||
| const now = new Date(); | ||
| const recent = `${now.getFullYear()}${String(now.getMonth() + 1).padStart(2, "0")}${String(now.getDate()).padStart(2, "0")}`; | ||
| const ghcrTags = [`stable-${recent}`, `stable-${recent}`]; // duplicate |
There was a problem hiding this comment.
This test is also time-dependent (new Date()), so it can become flaky/non-reproducible. Consider sharing a helper that stubs Date.now() and uses a fixed tag date, rather than deriving recent from the current clock.
| * @returns {Array<{tag: string, cacheKey: string, dateStr: string, imageRef: string, publishedAt: string|null}>} | ||
| * Most-recent first, limited to MAX_RELEASES within LOOKBACK_DAYS. | ||
| */ | ||
| function findRecentTagsForStream(releaseTags, spec) { | ||
| /** | ||
| * Filter a list of GHCR tag strings to find recent dated tags for a given stream. | ||
| * |
There was a problem hiding this comment.
There are two consecutive JSDoc blocks above findRecentTagsForStream(). The first one still documents the old Releases API shape (releaseTags with {tagName, publishedAt}) and is now stale/misleading since the function takes raw GHCR tag strings; remove or update the old block so the docs match the current signature.
…alignment - Fix enrichFromSbom null-fallback bug in FirehoseFeed.tsx and generate-card-images.mjs: SBOM null now falls back to release notes version rather than silently dropping the chip (fixes pipewire and other stable packages disappearing from cards when SBOM has partial data) - Replace GitHub Releases API with GHCR OCI distribution API in fetch-github-sbom.js so daily builds without a GitHub Release are included in the SBOM pipeline; robust RFC 5988 Link header parsing with relative URL resolution and isNaN guard for impossible dates - Remove dead -fresh- prefix from pages.yml SBOM cache restore key, aligning it with the key saved by update-sbom-cache.yml - Add Playwright e2e tests for changelogs chip consistency - Add unit tests (enrichment.test.js) documenting the null-fallback bug and validating the fix; add tests for findRecentTagsForStream using a fixed reference date to avoid day-boundary flakiness Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
20fff36 to
a6922c7
Compare
…alignment