Skip to content

fix(deploy): serve studio static files on both harperdb 4.x and Harper 5.x - #1574

Draft
Devin-Holland wants to merge 1 commit into
stagefrom
fix/deploy-template-fastify5-static
Draft

fix(deploy): serve studio static files on both harperdb 4.x and Harper 5.x#1574
Devin-Holland wants to merge 1 commit into
stagefrom
fix/deploy-template-fastify5-static

Conversation

@Devin-Holland

@Devin-Holland Devin-Holland commented Jul 28, 2026

Copy link
Copy Markdown
Member

Problem

After upgrading the dev CM to harper-pro 5.1.23, browsing the studio root returns:

{"error":"reply.sendFile is not a function"}

and every static asset 404s. Boot log shows why:

[http/2] [error]: fastify-plugin: @fastify/static - expected '4.x' fastify version, '5.10.0' is installed

@fastify/static@7 only supports fastify 4, so on Harper 5.x (fastify 5.10) the plugin registration fails and reply.sendFile never gets decorated — while the / route itself still registers and 500s on every hit.

Fix

The deploy template ships both plugin majors under npm aliases (fastify4-static@fastify/static@^7, fastify5-static@fastify/static@^8) and picks the one matching fastify.version at registration time.

Why not just bump to ^8: this same template deploys to stage/prod CMs still on harperdb 4.7.33 (fastify 4), and the CM fleet upgrades to Harper 5.x one environment at a time. A hard bump would break whichever side doesn't match, and a dev-branch-only pin would be lost on the next branch promotion. Once every CM runs Harper 5.x this collapses back to a single @fastify/static@^8 dependency (comment in static.js marks it).

Verification

Registered the exact template module on both runtimes and validated responses:

runtime GET / GET asset
fastify 5.10.0 200, index.html, CSP headers set 200, cache-control: public, max-age=2592000, immutable
fastify 4.29.1 200, index.html, CSP headers set 200, same

🤖 Generated with Claude Code

Follow-ups from review + dev deploy

  • Fail-closed version pick (David's review): unknown fastify majors now throw at registration instead of silently picking a mismatched build — Harper's autoloader swallows registration errors, so a silent mismatch only ever surfaces as the runtime 500 this PR fixes. Verified: fastify 6.0.0/undefined both throw.
  • Renovate mute hardened (David's review): the @fastify/static disable rule now uses matchDepNames listing both npm aliases, so routine dep hygiene can't re-bump the fastify-4 build out from under 4.x CMs.
  • --no-audit on component install: first dev deploy failed in prepare — npm 11.16 exits 1 (with no error output) when a package.json using npm aliases hits audit advisories, even though the install itself completes. The @fastify/static v7/v8 lines have permanent advisories (fixes only landed in v10+), so the template's config.yaml now sets install.command: npm install --ignore-scripts --no-audit. The install property is supported identically on harperdb 4.7.33 and harper-pro 5.1.23 (same components/Application.ts code). Reproduced + verified in the dev CM container: aliased install with audit → exit 1, with --no-audit → exit 0 with both aliases installed.

@Devin-Holland
Devin-Holland requested a review from a team as a code owner July 28, 2026 19:55
@Devin-Holland
Devin-Holland requested a review from dawsontoth July 28, 2026 19:55

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the Fastify deployment template to support both Fastify 4 and Fastify 5 by dynamically selecting the appropriate version of @fastify/static (aliased as fastify4-static and fastify5-static in package.json) based on the running Fastify version. The feedback suggests adding a safety guard for fastify.version and specifying the radix in Number.parseInt to prevent potential runtime errors if the version property is undefined.

Comment thread .github/deploy-template/fastify/static.js Outdated
@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 54.2% 6181 / 11403
🔵 Statements 54.76% 6626 / 12098
🔵 Functions 45.97% 1484 / 3228
🔵 Branches 47.36% 4190 / 8847
File CoverageNo changed files found.
Generated in workflow #1649 for commit de64aa4 by the Vitest Coverage Report Action

@DavidCockerill DavidCockerill 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.

Approving. Aliasing both majors is the right call for a fleet mid-upgrade, and the both-runtimes verification table in the description is solid.

Checked the things that usually bite on a dual-serve change and none of them apply: only one plugin ever registers, so there's no duplicate or stale asset and no route-precedence question, and CSP, CORS and cache policy are all unchanged.

Two non-blocking notes in threads, both about future decay rather than today — the version pick currently fails open on an unexpected fastify version, and the Renovate mute may not match npm aliases, which would let a routine dep bump reproduce the outage this fixes.

— Reviewed by DAIvid (Claude Opus 5)

Comment thread .github/deploy-template/fastify/static.js Outdated
Comment thread .github/deploy-template/package.json Outdated
@Devin-Holland

Copy link
Copy Markdown
Member Author

Handoff notes (Dawson's taking the branch from here):

1. Verify Commits is failing — two commits lack Conventional Commits prefixes (on both fix/deploy-template-fastify5-static and the -dev variant, and now on dev itself via the force-push):

  • Fail closed on unknown fastify major; mute Renovate on the static aliases → suggest fix(deploy): fail closed on unknown fastify major; mute Renovate on the static aliases
  • Disable npm audit in component install (npm 11 + alias exit-1 bug) → suggest fix(deploy): disable npm audit in component install (npm 11 + alias exit-1 bug)

They're the top two commits on each branch, so a reword + --force-with-lease fixes it. Apologies for the churn — documented the convention in CLAUDE.md via #1575 so it doesn't recur.

2. The install.command fix did NOT take effect on the last dev deployrun 30399339123 deployed the right commit (VITE_STUDIO_VERSION: dev_1be446e) but the prepare phase still logged install: using npm + npm warn using --force and failed with Failed to install dependencies for hdbms using npm default — the default install path, not the custom command. So config.yaml's install.command appears not to be consulted during deployComponent's prepare on harper-pro 5.1.23 (unconfirmed why — I was mid-investigation in core/components/Application.ts; application.install is evidently undefined at that point). Note the default path runs npm install --force --ignore-scripts, and npm 11.16 exits 1 on this package.json because of the alias+audit bug (reproduced in the dev CM container; --no-audit fixes it, details in the PR body).

Fallback if install.command can't be made to work at prepare time: pre-install node_modules in the deploy dir during the workflow and pass skip_node_modules=false to harper deploy_component — the server skips install entirely when node_modules exists.

— devain (Claude Fable 5)

@dawsontoth
dawsontoth force-pushed the fix/deploy-template-fastify5-static branch from 7e8b8cd to fa4610e Compare July 28, 2026 21:35
@dawsontoth
dawsontoth marked this pull request as draft July 29, 2026 16:33
@dawsontoth

Copy link
Copy Markdown
Contributor

I'm flagging this as a draft, do NOT merge this until Central Manager is totally ready with v5 of Harper!

@fastify/static@7 declares fastify 4.x only, so on a Harper 5.x CM
(fastify 5.10) plugin registration fails the fastify-plugin version
check and reply.sendFile is never decorated: GET / returns
{"error":"reply.sendFile is not a function"} and all static assets 404.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

4 participants