Repository navigation
enhance-commit-release-artifact to follow best practice passing CI - #64
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
me | 4c361d8 | Commit Preview URL Branch Preview URL |
Apr 06 2026, 05:42 PM |
📝 WalkthroughWalkthroughCI no longer writes release artifacts back to protected main; release metadata (version/changelog) is emitted to GITHUB_OUTPUT and GitHub Releases/tags are the canonical source. Frontend fetches GitHub release data at runtime for the footer and /whats-new page. Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant CI as GitHub Actions CI
participant ReleaseScript as scripts/release.js
participant GitHubAPI as GitHub Releases API
participant Browser as Browser
Dev->>CI: Push to main
CI->>CI: Run quality & build jobs
CI->>ReleaseScript: Run release job
ReleaseScript->>ReleaseScript: Compute version & changelog from git
ReleaseScript->>CI: Write version/changelog to GITHUB_OUTPUT
CI->>GitHubAPI: Create GitHub Release using GITHUB_OUTPUT
GitHubAPI-->>CI: Release created
Browser->>GitHubAPI: Fetch releases at runtime
GitHubAPI-->>Browser: Return release data
Browser->>Browser: Render footer latest version and /whats-new list
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Bundle ReportChanges will increase total bundle size by 4.08kB (870.73%) ⬆️
Affected Assets, Files, and Routes:view changes for bundle: molecular-mars-esmAssets Changed:
Files in
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 90: Replace the phrase "Cloudflare Pages project" with wording that
reflects the Workers + Assets Git integration model: update the sentence that
references pushing to the main branch so it states that Cloudflare Git
integration (Workers + Assets) will automatically deploy pushes to main, and
keep the build step (npm run build) and output directory (./dist/) guidance
intact; specifically modify the sentence containing "Cloudflare Pages project"
to instead mention "Cloudflare Git integration (Workers + Assets)" and ensure
surrounding text about pushing to `main`, `npm run build`, and `./dist/` remains
accurate.
In `@scripts/release.js`:
- Around line 34-58: The release script currently only writes to GITHUB_OUTPUT
and keeps changelog in memory, dropping updates to tracked artifacts; update
scripts/release.js (around the changelog/ githubOutput logic) to also write the
release metadata into src/data/version.json (e.g., JSON containing the new
version and changelog) and to update/append the human CHANGELOG.md file before
or alongside the fs.appendFileSync calls to GITHUB_OUTPUT, using the same
computed changelog and version variables (referencing changelog, commits,
version, githubOutput, and fs.appendFileSync) so the source-of-truth files
remain generated and committable.
In `@src/utils/github-releases.ts`:
- Around line 36-57: Update fetchGitHubReleases to include an Authorization
header using a GITHUB_TOKEN from env and add observability: when calling
RELEASES_API_URL in fetchGitHubReleases, read process.env.GITHUB_TOKEN and, if
present, add an "Authorization: token <token>" header (preserving existing
Accept header); when response.ok is false, log the status, statusText and any
rate-limit headers (e.g., "x-ratelimit-limit", "x-ratelimit-remaining",
"x-ratelimit-reset") before returning [], and in the catch block log the caught
error; keep existing behavior of parsing GitHubReleaseApiItem, calling
normalizeRelease and returning SiteRelease[].
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3c6f924a-8ff7-4b4f-96c2-2e01cfc68a24
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (21)
.github/copilot-instructions.md.github/workflows/ci.ymlAGENTS.mdREADME.md__tests__/release.spec.ts__tests__/sentry.server.config.spec.tsastro.config.mjseslint.config.mjspackage.jsonscripts/release.jssentinel.mdsentry.server.config.tssrc/__tests__/github-releases.test.tssrc/__tests__/sentry.client.config.test.tssrc/components/Footer.astrosrc/pages/whats-new.astrosrc/sentry.client.config.tssrc/utils/github-releases.tstsconfig.jsonvantage.mdvitest.config.ts
💤 Files with no reviewable changes (1)
- vitest.config.ts
| let changelog = ''; | ||
|
|
||
| if (commits) { | ||
| const list = commits | ||
| const entries = commits | ||
| .split('\n') | ||
| .filter((c) => !c.includes('[skip ci]')) | ||
| .map((c) => `- ${c}`) | ||
| .join('\n'); | ||
| if (list) { | ||
| changelogEntry += `${list}\n\n`; | ||
| .map((commit) => commit.trim()) | ||
| .filter(Boolean) | ||
| .filter((commit) => !commit.includes('[skip ci]')) | ||
| .map((commit) => `- ${commit}`); | ||
|
|
||
| if (entries.length > 0) { | ||
| changelog = `${entries.join('\n')}\n`; | ||
| } else { | ||
| changelogEntry += `- Internal CI updates.\n\n`; | ||
| changelog = '- Internal CI updates.\n'; | ||
| } | ||
| } else { | ||
| changelogEntry += `- No documented changes.\n\n`; | ||
| } | ||
|
|
||
| // Prepend to CHANGELOG.md | ||
| let existingChangelog = CHANGELOG_HEADER; | ||
| try { | ||
| existingChangelog = fs.readFileSync(CHANGELOG_PATH, 'utf-8'); | ||
| } catch (e) { | ||
| console.log('CHANGELOG.md not found. A new one will be created.'); | ||
| console.debug('File read error:', e.message); | ||
| } | ||
|
|
||
| let changelogBody = existingChangelog; | ||
| if (existingChangelog.startsWith(CHANGELOG_HEADER)) { | ||
| changelogBody = existingChangelog.slice(CHANGELOG_HEADER.length); | ||
| changelog = '- No documented changes.\n'; | ||
| } | ||
|
|
||
| changelogBody = changelogBody.replaceAll(CHANGELOG_HEADER, '').replace(/^\s+/, ''); | ||
|
|
||
| const nextChangelog = CHANGELOG_HEADER + changelogEntry + changelogBody; | ||
|
|
||
| fs.writeFileSync(CHANGELOG_PATH, nextChangelog); | ||
|
|
||
| // Write outputs to GitHub Actions for workflow consumption | ||
| const githubOutput = process.env.GITHUB_OUTPUT; | ||
| if (githubOutput) { | ||
| try { | ||
| fs.appendFileSync(githubOutput, `version=${version}\n`); | ||
| fs.appendFileSync(githubOutput, `changelog<<EOF\n${changelogEntry}EOF\n`); | ||
| fs.appendFileSync(githubOutput, `changelog<<EOF\n${changelog}EOF\n`); | ||
| console.log('Outputs written to GITHUB_OUTPUT'); |
There was a problem hiding this comment.
Restore tracked release artifact writes before publishing outputs.
Line 34 onward now keeps changelog only in memory, and Line 57 writes only to GITHUB_OUTPUT. That drops updates to src/data/version.json and CHANGELOG.md from the release source-of-truth script.
💡 Proposed fix
let changelog = '';
@@
} else {
changelog = '- No documented changes.\n';
}
+
+// Keep tracked release artifacts current
+const versionFilePath = 'src/data/version.json';
+const changelogFilePath = 'CHANGELOG.md';
+fs.mkdirSync('src/data', { recursive: true });
+fs.writeFileSync(versionFilePath, `${JSON.stringify({ version }, null, 2)}\n`);
+
+const changelogHeader = '# Changelog\n\n';
+const changelogEntry = `## ${version}\n${changelog}\n`;
+const existingChangelog = fs.existsSync(changelogFilePath)
+ ? fs.readFileSync(changelogFilePath, 'utf8')
+ : changelogHeader;
+const normalizedExisting = existingChangelog.startsWith(changelogHeader)
+ ? existingChangelog.slice(changelogHeader.length)
+ : existingChangelog;
+fs.writeFileSync(changelogFilePath, `${changelogHeader}${changelogEntry}${normalizedExisting}`);
const githubOutput = process.env.GITHUB_OUTPUT;
if (githubOutput) {Based on learnings: Applies to {src/data/version.json,CHANGELOG.md} : Maintain tracked release artifacts: src/data/version.json and CHANGELOG.md must be generated and kept current by scripts/release.js; scripts/release.js is the source of truth for src/data/version.json and CHANGELOG.md; these files must remain commitable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/release.js` around lines 34 - 58, The release script currently only
writes to GITHUB_OUTPUT and keeps changelog in memory, dropping updates to
tracked artifacts; update scripts/release.js (around the changelog/ githubOutput
logic) to also write the release metadata into src/data/version.json (e.g., JSON
containing the new version and changelog) and to update/append the human
CHANGELOG.md file before or alongside the fs.appendFileSync calls to
GITHUB_OUTPUT, using the same computed changelog and version variables
(referencing changelog, commits, version, githubOutput, and fs.appendFileSync)
so the source-of-truth files remain generated and committable.
4967fc7 to
d403d9b
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/components/Footer.astro (1)
33-47: Minor: Line 45 is redundant since HTML already shows "Release notes".When
releases[0]is falsy, settingtextContent = 'Release notes'duplicates what's already rendered. This is harmless but could be simplified.♻️ Proposed simplification
if (versionLink instanceof HTMLAnchorElement) { fetchGitHubReleases().then((releases) => { if (releases[0]) { versionLink.textContent = `v${releases[0].version}`; - return; } - - versionLink.textContent = 'Release notes'; }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Footer.astro` around lines 33 - 47, The code in Footer.astro redundantly sets versionLink.textContent = 'Release notes' when no release exists even though the static HTML already contains that text; update the logic in the block that uses fetchGitHubReleases (referencing fetchGitHubReleases, versionLink, and the releases[0] check) to only update textContent when a release is present (i.e., remove the fallback assignment or guard the update so nothing happens on falsy releases), leaving the initial HTML content intact.src/utils/github-releases.ts (1)
24-34: Consider consolidating draft/prerelease filtering in one location.
normalizeReleasefilters outdraftreleases (line 25), whilefetchGitHubReleasesfilters outprereleasereleases (line 61). This split logic works but could be confusing for maintainers.Consider moving both filters to the same location for clarity:
♻️ Option: consolidate filtering in normalizeRelease
export function normalizeRelease(release: GitHubReleaseApiItem): SiteRelease | null { - if (!release.tag_name || release.draft) return null; + if (!release.tag_name || release.draft || release.prerelease) return null; return {Then simplify the fetch call:
return releases - .filter((release) => !release.prerelease) .map(normalizeRelease) .filter((release): release is SiteRelease => release !== null);Also applies to: 60-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/github-releases.ts` around lines 24 - 34, Consolidate draft/prerelease filtering into a single function by moving prerelease filtering into normalizeRelease (which already checks release.draft) so normalizeRelease(release: GitHubReleaseApiItem) returns null for draft or prerelease releases; then simplify fetchGitHubReleases to stop filtering out prerelease there (remove the prerelease check around where normalizeRelease is called) and rely solely on normalizeRelease to filter invalid releases. Ensure references to release.draft and release.prerelease are handled inside normalizeRelease and update any callers to expect null-filtering from that function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/Footer.astro`:
- Around line 33-47: The code in Footer.astro redundantly sets
versionLink.textContent = 'Release notes' when no release exists even though the
static HTML already contains that text; update the logic in the block that uses
fetchGitHubReleases (referencing fetchGitHubReleases, versionLink, and the
releases[0] check) to only update textContent when a release is present (i.e.,
remove the fallback assignment or guard the update so nothing happens on falsy
releases), leaving the initial HTML content intact.
In `@src/utils/github-releases.ts`:
- Around line 24-34: Consolidate draft/prerelease filtering into a single
function by moving prerelease filtering into normalizeRelease (which already
checks release.draft) so normalizeRelease(release: GitHubReleaseApiItem) returns
null for draft or prerelease releases; then simplify fetchGitHubReleases to stop
filtering out prerelease there (remove the prerelease check around where
normalizeRelease is called) and rely solely on normalizeRelease to filter
invalid releases. Ensure references to release.draft and release.prerelease are
handled inside normalizeRelease and update any callers to expect null-filtering
from that function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fe760469-7be5-4949-a38c-08884de8cafc
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
.github/copilot-instructions.md.github/workflows/ci.ymlAGENTS.mdREADME.md__tests__/release.spec.ts__tests__/sentry.server.config.spec.tsastro.config.mjseslint.config.mjspackage.jsonscripts/release.jssentry.server.config.tssrc/__tests__/github-releases.test.tssrc/__tests__/sentry.client.config.test.tssrc/components/Footer.astrosrc/pages/whats-new.astrosrc/sentry.client.config.tssrc/utils/github-releases.tstsconfig.jsonvitest.config.ts
💤 Files with no reviewable changes (1)
- vitest.config.ts
✅ Files skipped from review due to trivial changes (5)
- tsconfig.json
- eslint.config.mjs
- sentry.server.config.ts
- package.json
- src/tests/github-releases.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- src/sentry.client.config.ts
- tests/sentry.server.config.spec.ts
- .github/copilot-instructions.md
- scripts/release.js
- README.md
- astro.config.mjs
- src/tests/sentry.client.config.test.ts
d403d9b to
4c361d8
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/utils/github-releases.ts (1)
36-44:⚠️ Potential issue | 🟠 Major
GITHUB_TOKENauth path is ineffective in the current client-side execution path.Line 37/Line 43 assumes Node-style env access, but this helper is called from client
<script>blocks (src/pages/whats-new.astroandsrc/components/Footer.astro). In browser runtime, the Authorization header won’t be set, so requests stay unauthenticated and are still rate-limit-prone.Please move GitHub API calls behind a server-side endpoint (or SSR-only fetch with caching) and have the client call that internal endpoint instead.
#!/bin/bash # Verify where fetchGitHubReleases() executes and whether server-side proxy exists. set -euo pipefail echo "== Client-side call sites ==" rg -n --type=astro -C3 '\bfetchGitHubReleases\s*\(' src/pages/whats-new.astro src/components/Footer.astro echo echo "== Env-token usage in utility ==" rg -n --type=ts -C3 'process\.env\.GITHUB_TOKEN|Authorization:\s*`token' src/utils/github-releases.ts echo echo "== Check for an internal server endpoint/proxy for GitHub releases ==" rg -n -C2 'api\.github\.com/repos/.*/releases|fetchGitHubReleases\s*\(' srcExpected verification result: call sites appear inside client scripts, and no server-side proxy/cached endpoint is found for release fetching.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/github-releases.ts` around lines 36 - 44, fetchGitHubReleases currently tries to read process.env.GITHUB_TOKEN (and uses RELEASES_API_URL) but is invoked from client-side scripts (whats-new.astro and Footer.astro) so the Authorization path is ineffective; move the GitHub call server-side by implementing an SSR/api endpoint (e.g., GET /api/releases) that performs the fetch to RELEASES_API_URL using process.env.GITHUB_TOKEN, adds appropriate caching and error handling, and then change the client code to call that internal endpoint instead of calling fetchGitHubReleases directly; alternatively make fetchGitHubReleases SSR-only (exported/used only in server code) and remove client-side usage so token-based auth and rate-limit caching work reliably.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/utils/github-releases.ts`:
- Around line 36-44: fetchGitHubReleases currently tries to read
process.env.GITHUB_TOKEN (and uses RELEASES_API_URL) but is invoked from
client-side scripts (whats-new.astro and Footer.astro) so the Authorization path
is ineffective; move the GitHub call server-side by implementing an SSR/api
endpoint (e.g., GET /api/releases) that performs the fetch to RELEASES_API_URL
using process.env.GITHUB_TOKEN, adds appropriate caching and error handling, and
then change the client code to call that internal endpoint instead of calling
fetchGitHubReleases directly; alternatively make fetchGitHubReleases SSR-only
(exported/used only in server code) and remove client-side usage so token-based
auth and rate-limit caching work reliably.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a0837efa-6137-4935-8c88-50dd50289928
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
.github/copilot-instructions.md.github/workflows/ci.ymlAGENTS.mdREADME.md__tests__/release.spec.ts__tests__/sentry.server.config.spec.tsastro.config.mjseslint.config.mjspackage.jsonscripts/release.jssentry.server.config.tssrc/__tests__/github-releases.test.tssrc/__tests__/sentry.client.config.test.tssrc/components/Footer.astrosrc/pages/whats-new.astrosrc/sentry.client.config.tssrc/utils/github-releases.tstsconfig.jsonvitest.config.ts
💤 Files with no reviewable changes (1)
- vitest.config.ts
✅ Files skipped from review due to trivial changes (7)
- tsconfig.json
- eslint.config.mjs
- package.json
- .github/copilot-instructions.md
- scripts/release.js
- src/components/Footer.astro
- src/tests/github-releases.test.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- src/sentry.client.config.ts
- sentry.server.config.ts
- README.md
- tests/sentry.server.config.spec.ts
- src/tests/sentry.client.config.test.ts
- src/pages/whats-new.astro
- astro.config.mjs
- tests/release.spec.ts
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Tests