feat: upgrade to Astro 7 with Sätteri markdown pipeline - #47
Conversation
Adopt Astro 7.0.8 (Rust compiler, Vite 8/Rolldown) and make Sätteri the production Markdown/MDX processor, with hast plugins restoring external links and responsive tables. Keep TypeScript 6 for Svelte peer support and lower the pnpm release-age gate to 3 days for installable upgrades. Co-authored-by: Cursor <cursoragent@cursor.com>
AI review for
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict
Solid Astro 7 / Sätteri upgrade — compressHTML: true and the new hast plugins correctly restore Astro 6 whitespace and the rehype transforms that the Sätteri trial had dropped. Two warnings worth addressing before merge (tests + supply-chain cooldown); nothing blocking on trust/security policy.
Findings
| # | Severity | File | Summary |
|---|---|---|---|
| 1 | src/lib/satteri-markdown-plugins.mjs:13 |
New hast plugins have no unit tests covering external-link / table-wrap branches | |
| 2 | pnpm-workspace.yaml:8 |
minimumReleaseAge cut from 7 days to 3 days weakens the documented supply-chain guard |
|
| 3 | ℹ️ info | src/lib/satteri-markdown-plugins.mjs:20 |
Absolute same-origin https://deepworkplan.com/… links get target=_blank (parity with old rehype config) |
Notes (not inline)
- Whitespace fix is correct. Astro 7 defaults
compressHTMLto'jsx'; settingtruerestores Astro 6 HTML-aware compression per the v7 upgrade guide. That matches the PR test-plan concern about glued inline words. - Plugin restore is a real improvement. On
main,satteri()was already the processor and the rehype transforms were intentionally dropped (deadrehype-external-linksdependency). Wiring hast plugins closes that gap for methodology/spec/kit tables and outbound links. - Plugin API usage looks sound.
defineHastPlugin/wrapNode/setPropertymatch the Sätteri docs; emptychildren: []on the wrapper is the documented pattern (wrapNodeinserts the node as first child). - Stack docs:
AGENTS.mdversions are updated. Broaderdocs/**did not mention the old rehype paths in a quick scan — low drift risk. - Trust checklist: no installer pipes, no signing overclaims, no trust-surface regressions, no secrets, no hand-edits under vendored skill packages that change security posture (only a path note in the local lighthouse skill).
Release decision
Recommendation: request-changes
| * Open absolute http(s) links in a new tab with a safe rel. | ||
| * Relative and same-site path links are left alone. | ||
| */ | ||
| export const externalLinksPlugin = defineHastPlugin({ |
There was a problem hiding this comment.
These hast plugins restore production UX across every methodology/spec/kit page (external target/rel, mobile table scroll), but there are no unit tests exercising the branches.
If visit stops firing, or wrapNode/setProperty semantics drift in a Sätteri bump, tables overflow on mobile and outbound links lose noopener with no CI signal — pnpm run test would still pass.
Consider a small Vitest suite that feeds markdown through markdownToHtml(..., { hastPlugins }) and asserts: relative /path links stay untouched, https://example.com gets target="_blank" + rel="noopener noreferrer", and a GFM table is wrapped once in .table-responsive.
| # against compromised packages that get yanked or patched within days. | ||
| # Affects new installs / updates only — the existing lockfile is respected. | ||
| minimumReleaseAge: 10080 | ||
| minimumReleaseAge: 4320 |
There was a problem hiding this comment.
minimumReleaseAge drops from 10080 (7 days) to 4320 (3 days). That shortens the cool-down this file documents as protection against compromised packages that get yanked or patched within days.
If this is only to unblock this Astro 7 install, prefer a one-off pnpm install --config.minimumReleaseAge=… (or a temporary override) and keep the repo default at a week. If 3 days is the new permanent policy, call that out in the PR / comment so future upgrades do not silently assume the old window.
| visit(node, ctx) { | ||
| const href = node.properties?.href; | ||
| if (typeof href !== 'string') return; | ||
| if (!href.startsWith('http://') && !href.startsWith('https://')) return; |
There was a problem hiding this comment.
Any absolute http(s):// href gets target="_blank", including same-origin URLs like https://deepworkplan.com/spec. That matches the previous rehype-external-links options (no site filter), and methodology/spec/kit currently use relative paths — so this is not a regression today.
If you want stricter "external only" behavior, exclude the configured site host (and optionally www.) before setting target/rel. The comment on lines 10–11 is accurate for path-style same-site links; absolute same-site links are still rewritten.
1 participant
Summary
@astrojs/mdx7,@astrojs/svelte9,@astrojs/markdown-satteri0.3.4).target/reland responsive table wrappers previously handled by rehype.@astrojs/sveltepeer compatibility, and setminimumReleaseAgeto 3 days so installable upgrades can land.Changes
astro.config.mjs: dropexperimental.rustCompiler, setcompressHTML: true, wiresatteri({ hastPlugins }).src/lib/satteri-markdown-plugins.mjs; remove unusedrehype-responsive-tables.mjsplusrehype-external-links/@astrojs/markdown-remarkdeps.Test plan
pnpm run astro:check— 0 errorspnpm run biome:check— cleanpnpm run md:check— 1343/1343pnpm run test— 85 passedpnpm run build— 1369 pagespnpm run lighthouse— assertions passedhttps://links open in a new tab, no glued inline wordsMade with Cursor