Add RSS feed XML validation and fix undeclared itunes namespace

[plx]

The RSS feed emitted <itunes:image> without declaring the itunes namespace, producing invalid XML that no existing check caught. This declares the namespace via @astrojs/rss's xmlns option and adds scripts/validate-feed.js, which validates dist/rss.xml with xmllint and—crucially—fails on the undeclared-namespace errors that xmllint prints to stderr while still exiting 0. The check is wired into validate:all, test:ci/test:ci:verbose, and the CI build workflow, with a just validate-feed recipe and an xmllint presence probe added to just setup.

🤖 Generated with Claude Code

[github-actions]

PR Validation ❌

Some checks failed. Please review the errors above.

Checks Performed:

Please check the workflow logs for details on which checks failed.

This is a complete dry-run of the deployment process, ensuring your changes will deploy successfully when merged.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1c838c4920

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Install xmllint before validating the feed

In the ubuntu-latest job used by both deploy and PR validation, this new step invokes scripts/validate-feed.js without first installing xmllint; the validator exits 1 when xmllint is missing, and the current GitHub-hosted Ubuntu 24.04 runner software list does not include libxml2-utils/xmllint (also reproduced in this clean Ubuntu environment). This makes every build fail at RSS validation unless the workflow installs libxml2-utils before running this command.

Useful? React with 👍 / 👎.

[claude]

PR Review

This PR fixes a real bug (undeclared xmlns:itunes on the RSS <rss> root element) and adds a thoughtful validation layer to prevent regressions. The validate-feed.js script's design rationale is well-documented, and the fix in rss.xml.ts is minimal and correct. The wiring into validate:all, test:ci, and CI is consistent.

Two actionable findings survived verification:

---

.github/workflows/build.yml — Implicit xmllint dependency with no install step

The comment says "xmllint ships with ubuntu-latest (libxml2-utils); no install step needed", and this is currently true — libxml2-utils appears in the ubuntu-22.04/24.04 runner image manifests. However, it's an undeclared dependency with no fallback: GitHub has removed pre-installed packages from runner images before, and if it happens here, CI breaks (with a clear error message, but no graceful recovery). An explicit install step costs nothing and eliminates the fragility:

- name: Install xmllint
  run: sudo apt-get install -y libxml2-utils

---

scripts/validate-feed.js line 44 — probe.error doesn't check probe.status

Node's spawnSync only sets .error for OS-level launch failures (e.g. ENOENT). If xmllint exists on PATH but returns a non-zero exit from --version (a broken or stub install), probe.error is null and the guard silently passes. The real validation call at line 52 would then fail with a confusing diagnostic rather than the clear "not found" message.

// current
if (probe.error) {

// more defensive
if (probe.error || probe.status !== 0) {

---

Minor nit

process.exit(0) on line 71 is dead code — Node exits 0 automatically when the script completes without error. Safe to remove.

---

Overall this is a solid, well-motivated PR. The two findings above are both easy one-liners to address.

[claude]

Review

Nice fix — clear root-cause (undeclared itunes namespace) plus a regression check that actually catches it, since xmllint --noout alone would have passed silently (confirmed the exit-0/stderr-only behavior is real and worth guarding against).

Correctness

• src/pages/rss.xml.ts: adding xmlns: { itunes: "..." } to the @astrojs/rss call is the right fix, and the DTD URL matches the standard iTunes podcast namespace.
• scripts/validate-feed.js: treating any stderr output as failure (not just exit code) is the correct call given xmllint's recoverable-error behavior — good catch, and the comment explaining why is helpful for future readers who might otherwise "simplify" this back to just checking the exit code.
• Using spawnSync with an argument array (no shell: true) avoids any command-injection surface — good practice even though inputs here are effectively trusted.
• --nonet on the xmllint call is a nice touch to keep the check hermetic/offline.

Minor suggestions (non-blocking)

• .github/workflows/build.yml: sudo apt-get install -y libxml2-utils runs without apt-get update first. GitHub-hosted ubuntu-latest images usually have a fresh-enough package index for common packages, but it's a small source of flakiness if the index is ever stale — consider sudo apt-get update && sudo apt-get install -y libxml2-utils for robustness, or caching the install.
• No unit test was added for scripts/validate-feed.js (e.g., feeding it a fixture with an undeclared namespace to assert it exits non-zero). That said, this matches the existing convention — scripts/validate-links.js also has no test coverage — so it's consistent with the codebase rather than a regression.
• Since validate-feed.js already accepts an optional path argument for testing, it'd be low-effort to add a small vitest case using a fixture XML string/temp file to lock in the "stderr-means-failure" behavior this PR specifically exists to guarantee.

CI wiring

• Confirmed deploy.yml calls the same reusable build.yml, so the feed check also gates production deploys, not just PR validation — good, this is the workflow that most matters given main auto-deploys.
• Job ordering (link validation → xmllint install → feed validation → artifact upload) is sound: a failure at any prior step will skip the artifact upload since none of the added steps use continue-on-error or if: always().

Overall this is a solid, well-scoped fix with a good explanatory comment in the validator script. No blocking issues found.

[github-actions]

PR Validation ✅

All checks passed! Ready for review.

Checks Performed:

✓ Linting
✓ Spell check (source)
✓ Type checking & Build
✓ Spell check (HTML)
✓ Internal link validation
✓ RSS feed validation
✓ Artifact upload

This is a complete dry-run of the deployment process, ensuring your changes will deploy successfully when merged.

[plx]

Addressed Claude's follow-up feedback in 81ed36f:\n\n- Added apt-get update before installing libxml2-utils in the reusable build workflow.\n- Added a Vitest regression test for validate-feed.js that uses a fake xmllint which exits 0 while writing the namespace error to stderr, locking in the failure behavior without requiring xmllint during unit tests.\n- Extended Vitest's include globs so script-adjacent tests run with npm run test:unit.\n\nVerified locally with npm run test:unit, npm run lint, and npm run validate:feed.

[github-actions]

PR Validation ✅

All checks passed! Ready for review.

Checks Performed:

✓ Linting
✓ Spell check (source)
✓ Type checking & Build
✓ Spell check (HTML)
✓ Internal link validation
✓ RSS feed validation
✓ Artifact upload

This is a complete dry-run of the deployment process, ensuring your changes will deploy successfully when merged.

[claude]

Review: Add RSS feed XML validation and fix undeclared itunes namespace

Solid, well-scoped fix. The root cause (missing xmlns:itunes declaration) is correctly identified and fixed in src/pages/rss.xml.ts, and the new validate-feed.js guards against the exact class of bug that slipped through before — nice attention to the fact that xmllint exits 0 even on a namespace error, which is the whole reason the original bug went undetected.

Strengths:

• Correct fix: the itunes namespace URI (http://www.itunes.com/dtds/podcast-1.0.dtd) matches the standard iTunes podcast RSS namespace, declared via @astrojs/rss's xmlns option rather than hand-rolled XML — good use of the librarys intended extension point.
• No shell-injection surface: spawnSync("xmllint", [...]) is called with an argument array, not a shell string, so the feedPath argument (even though its only ever developer/CI-controlled) cannot be used for command injection.
• Good CI wiring: reusing build.yml from pr-validation.yml means the xmllint install step is automatically covered for PR checks too — no duplicate setup needed.
• Nice DX touches: the xmllint presence probe in just setup and the clear error messages in validate-feed.js (pointing to the apt-get/macOS install path) make failures easy to self-serve.
• CLAUDE.md and the justfile doc-comments were updated alongside the behavior change, per repo convention.

Considerations:

1. "Any stderr = failure" is a broad heuristic in scripts/validate-feed.js. This is well-reasoned and documented in the header comment (necessary because xmllint treats namespace errors as recoverable/exit-0), but it does mean any unrelated future xmllint stderr chatter (e.g. a deprecation notice or an encoding warning) would also fail CI. Worth keeping in mind if this check ever produces a false-positive failure — not a blocker, just a known trade-off.
2. Test coverage is narrow. scripts/validate-feed.test.ts only covers the "exit 0 but namespace error on stderr" case (the specific regression this PR fixes), which is the most important path. It would be worth also covering: the happy path (well-formed feed exits 0 with success message logged), a missing feed file (exit 1 before ever invoking xmllint), and a missing xmllint binary (exit 1 via the --version probe). These are all straightforward given the existing fake-binary-on-PATH test harness, and would lock in the other branches of the script.
3. Minor/non-blocking: the "Install xmllint" step in build.yml runs apt-get update && apt-get install on every CI run with no caching — adds a small amount of time to each build. Not worth over-engineering for a single small package, but flagging in case build times become a concern later.

Nothing here blocks merging — the core fix is correct, tested for the motivating bug, and the validation script is safe and well-documented.