deps(tko.io): bump astro 5→6, starlight 0.37→0.38, drop patch-package#360
deps(tko.io): bump astro 5→6, starlight 0.37→0.38, drop patch-package#360brianmhunt merged 1 commit intomainfrom
Conversation
Fixes dependabot advisory: Astro XSS in define:vars via incomplete </script> tag sanitization (supersedes #354). Migrations: - content.config.ts: add docsLoader() — Astro 6 requires Content Layer loaders; file-based auto-discovery removed. - Header.astro + tests.astro: resolve sibling/public paths from process.cwd() instead of import.meta.url. Astro 6's prerender bundles into dist/.prerender/chunks/, breaking ../-relative URLs. - Remove patch-package: upstream starlight already includes the head.ts fix the patch carried, and Bun has native patching. Verified: bun run build → 66 pages, clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThis pull request updates Astro dependencies to newer versions, removes the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tko.io/src/pages/tests.astro (1)
12-14: Nit:process.cwd()is invocation-dependent.Works as long as
astro build/astro devis invoked fromtko.io/(which the package scripts ensure), but silently resolves to the wrong path if someone runs the build from another cwd. Sincebundle-tests.mjsalready writesmanifest.jsonto a known absolute location, consider making this resilient later — e.g. probeprocess.cwd()and its parent, or export the absolute path from a shared module. Not blocking given the PR’s successful 66-page build.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tko.io/src/pages/tests.astro` around lines 12 - 14, The current manifestPath uses process.cwd() which is invocation-dependent; update the resolution so manifestPath is robust by either importing an exported absolute path from the bundling script (e.g., export the absolute manifest location from bundle-tests.mjs and consume it where manifestPath is defined) or implement a small probe that checks process.cwd() and its parent for 'public/tests/source/manifest.json' before falling back to process.cwd(); ensure you reference the existing manifestPath, join, and the 'public/tests/source/manifest.json' string when implementing the fix.tko.io/src/components/Header.astro (1)
4-16: LGTM — minor observation on cwd fragility.The relative traversal
'../builds/knockout/package.json'againstprocess.cwd()correctly resolves to<repo>/builds/knockout/package.jsonwhen the build is run fromtko.io/(confirmed by the file’s presence andversionfield). Same caveat astests.astro: this couples path resolution to the invocation directory rather than the module location. If you want a single source of truth for “repo root” across the Astro 6 migration, a small helper (e.g.resolveRepoFile(...)) would make both sites future-proof against invocation-dir changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tko.io/src/components/Header.astro` around lines 4 - 16, The code in Header.astro uses process.cwd() + '../builds/knockout/package.json' (see join, readFileSync, const pkg, const version) which is fragile to invocation directory changes; replace this with a repo-root resolver helper (e.g., resolveRepoFile) that determines the repository root relative to the module (using import.meta.url/fileURLToPath or a consistent project-root constant) and use that helper to resolve and read the package.json so both Header.astro and tests.astro share a single, stable source-of-truth for repo file paths.tko.io/src/content.config.ts (1)
2-2: Drop the semicolon to match the Biome style.This changed import should follow the repo’s no-semicolon TS formatting.
🧹 Proposed style fix
-import { docsLoader } from '@astrojs/starlight/loaders'; +import { docsLoader } from '@astrojs/starlight/loaders'As per coding guidelines,
**/*.{ts,tsx,js,jsx,json}: “Use Biome for linting and formatting: no semicolons, single quotes, no trailing commas, 120 char width, 2-space indent, LF line endings”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tko.io/src/content.config.ts` at line 2, Remove the trailing semicolon from the import statement that imports docsLoader from '@astrojs/starlight/loaders' to conform to the Biome style (no semicolons); update the import line that references docsLoader so it ends without a semicolon and ensure formatting follows the repo's Biome rules (single quotes, 2-space indent, LF endings).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tko.io/src/components/Header.astro`:
- Around line 4-16: The code in Header.astro uses process.cwd() +
'../builds/knockout/package.json' (see join, readFileSync, const pkg, const
version) which is fragile to invocation directory changes; replace this with a
repo-root resolver helper (e.g., resolveRepoFile) that determines the repository
root relative to the module (using import.meta.url/fileURLToPath or a consistent
project-root constant) and use that helper to resolve and read the package.json
so both Header.astro and tests.astro share a single, stable source-of-truth for
repo file paths.
In `@tko.io/src/content.config.ts`:
- Line 2: Remove the trailing semicolon from the import statement that imports
docsLoader from '@astrojs/starlight/loaders' to conform to the Biome style (no
semicolons); update the import line that references docsLoader so it ends
without a semicolon and ensure formatting follows the repo's Biome rules (single
quotes, 2-space indent, LF endings).
In `@tko.io/src/pages/tests.astro`:
- Around line 12-14: The current manifestPath uses process.cwd() which is
invocation-dependent; update the resolution so manifestPath is robust by either
importing an exported absolute path from the bundling script (e.g., export the
absolute manifest location from bundle-tests.mjs and consume it where
manifestPath is defined) or implement a small probe that checks process.cwd()
and its parent for 'public/tests/source/manifest.json' before falling back to
process.cwd(); ensure you reference the existing manifestPath, join, and the
'public/tests/source/manifest.json' string when implementing the fix.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b28047a2-bf3e-42d3-bac7-0d7cd7f72468
⛔ Files ignored due to path filters (1)
tko.io/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
tko.io/package.jsontko.io/patches/@astrojs+starlight+0.37.7.patchtko.io/src/components/Header.astrotko.io/src/content.config.tstko.io/src/pages/tests.astro
💤 Files with no reviewable changes (1)
- tko.io/patches/@astrojs+starlight+0.37.7.patch
There was a problem hiding this comment.
Pull request overview
Updates the tko.io documentation site to Astro 6 / Starlight 0.38, including required Astro 6 migration adjustments and removal of patch-package now that the upstream Starlight fix is available.
Changes:
- Bump
astro5→6,@astrojs/starlight0.37→0.38, and@astrojs/check0.9.6→0.9.8 (plus lockfile refresh). - Migrate
src/content.config.tsto Astro 6’s Content Layer loader (docsLoader()). - Fix SSR-time filesystem path resolution in prerendered bundles (switch to
process.cwd()-based paths) and remove the Starlight patch +patch-packagehook.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tko.io/src/pages/tests.astro | Adjust manifest path resolution for Astro 6 prerender bundling. |
| tko.io/src/content.config.ts | Adds Starlight docsLoader() required by Astro 6 content layer. |
| tko.io/src/components/Header.astro | Updates package.json path resolution to remain correct under prerender chunk bundling. |
| tko.io/patches/@astrojs+starlight+0.37.7.patch | Removes obsolete patch file. |
| tko.io/package.json | Bumps Astro/Starlight deps and removes patch-package postinstall. |
| tko.io/bun.lock | Lockfile update reflecting new dependency tree. |
| // Resolve from cwd (project root) — import.meta.url points into | ||
| // dist/.prerender/chunks/ under Astro 6's prerender bundler. |
There was a problem hiding this comment.
The comment says "Resolve from cwd (project root)", but in this repo process.cwd() will be the tko.io/ package directory (not the monorepo root) because the docs build runs from cd tko.io. Clarifying this (e.g., “site root” / “tko.io root”) would avoid confusion for contributors running scripts from other directories.
| // Resolve from cwd (project root) — import.meta.url points into | |
| // dist/.prerender/chunks/ under Astro 6's prerender bundler. | |
| // Resolve from cwd (the tko.io site/package root during the docs | |
| // build) — import.meta.url points into dist/.prerender/chunks/ | |
| // under Astro 6's prerender bundler. |
| import { defineCollection } from 'astro:content'; | ||
| import { docsLoader } from '@astrojs/starlight/loaders'; | ||
| import { docsSchema } from '@astrojs/starlight/schema'; |
There was a problem hiding this comment.
tko.io code generally follows the repo-wide Biome style (no semicolons). Since this file is being touched for the Astro 6 migration, consider removing the trailing semicolons here as well to keep formatting consistent.
Summary
astro5.18.1 → 6.1.8,@astrojs/starlight0.37.7 → 0.38.3,@astrojs/check0.9.6 → 0.9.8patch-package— obsolete (upstream starlight already has the head.ts fix we carried) and Bun has native patchingMigrations
src/content.config.ts: adddocsLoader(). Astro 6 requires Content Layer API loaders; file-based auto-discovery forsrc/content/docs/is gone.src/components/Header.astroandsrc/pages/tests.astro: resolve sibling/public/paths fromprocess.cwd()instead ofnew URL(..., import.meta.url). Astro 6's prerender bundles page modules intodist/.prerender/chunks/, so../-relative URLs now resolve underdist/and break.Test plan
bun run build→ 66 pages, clean/3to4(Header version readout) and/tests(manifest-driven iframes)🤖 Generated with Claude Code
Summary by CodeRabbit