fix(server): strip leading slash in serveStatic for absolute webDistPath by Wirasm · Pull Request #1047 · coleam00/Archon

Wirasm · 2026-04-10T13:18:46Z

Summary

Problem: Hono's serveStatic passes c.req.path (e.g. /assets/foo.js) to path.join with the root option. When root is an absolute path (like ~/.archon/web-dist/0.3.4), path.join treats the leading / in the request path as an absolute segment and discards the root entirely — resolving to /assets/foo.js instead of ~/.archon/web-dist/0.3.4/assets/foo.js
Why it matters: archon serve downloads the web UI to an absolute path, so all static files (CSS, JS, favicon) fail to load — the web UI shows a blank page
What changed: Added rewriteRequestPath to strip the leading slash before path.join, and an explicit /favicon.png route
What did not change: SPA fallback (index.html) already worked because it uses path: 'index.html' (relative, no leading slash). API routes unaffected.

UX Journey

Before

  Browser                    Server
  ───────                    ──────
  GET /assets/index.js ────▶ serveStatic({ root: "/Users/.../web-dist/0.3.4" })
                             path.join("/Users/.../0.3.4", "/assets/index.js")
                             → "/assets/index.js" (root discarded!)
                             Bun.file("/assets/index.js") → not found
  blank page ◀──────────────  empty 200

After

  Browser                    Server
  ───────                    ──────
  GET /assets/index.js ────▶ rewriteRequestPath: strip leading /
                             path.join("/Users/.../0.3.4", "assets/index.js")
                             → "/Users/.../0.3.4/assets/index.js" ✓
  web UI loads ◀────────────  200 + file content

Label Snapshot

Risk: risk: low
Size: size: XS
Scope: server
Module: server:static-serving

Change Metadata

Change type: bug
Primary scope: server

Validation Evidence (required)

Tested in production mode with built web dist:

Root:    200  839 bytes (index.html)
CSS:     200  95,637 bytes
Favicon: 200  48,205 bytes
API:     200  7 codebases

Type check: passes
Lint: passes (after adding return type annotation)

Security Impact (required)

New permissions/capabilities? No
New external network calls? No
Secrets/tokens handling changed? No
File system access scope changed? No

Compatibility / Migration

Backward compatible? Yes
Config/env changes? No
Database migration needed? No

Human Verification (required)

Verified: dev server in production mode serves all static assets correctly
Edge cases: SPA fallback for /chat returns index.html
Not verified: compiled binary (needs release build)

Side Effects / Blast Radius (required)

Affected: static file serving in production mode only
No effect on dev mode (uses Vite dev server)
No effect on API routes

Rollback Plan (required)

Fast rollback: git revert <commit>
Observable failure: blank web UI page

Risks and Mitigations

Risk: rewriteRequestPath might affect other Hono middleware
- Mitigation: Only applied to /assets/* route, scoped narrowly

Summary by CodeRabbit

Bug Fixes
- Static asset serving improved: missing production web build now logs a clear warning and favicon requests are handled reliably.
- Server shutdown behavior hardened: termination signals are handled once to ensure predictable, single-run shutdown processing.

Hono's serveStatic passes c.req.path (e.g. '/assets/foo.js') to path.join with the root. When root is absolute, path.join treats the leading slash as an absolute segment and discards root entirely. Add rewriteRequestPath to strip the leading slash so the full path resolves correctly.

coderabbitai · 2026-04-10T13:19:05Z

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Added a production-time check logging web_dist_not_found when the computed web dist path is missing; added an explicit /favicon.png static handler; preserved SPA fallback and /assets/* behavior. Replaced persistent signal listeners with one-time process.once handlers for graceful shutdown.

Changes

Cohort / File(s)	Summary
Server startup & static serving `packages/server/src/index.ts`	Added a runtime check that logs a warning when `webDistPath` does not exist. Added an explicit `/favicon.png` route serving `webDistPath/favicon.png`. Left SPA fallback (`app.get(''...)`) and `/assets/` static handling intact.
CLI signal handling `packages/cli/src/commands/serve.ts`	Replaced persistent `process.on` handlers with `process.once` for `SIGINT` and `SIGTERM`, resolving the shutdown promise on the first received signal to avoid multiple invocations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I sniffed the dist and gave a cheer,
A favicon found, the path made clear.
Signals heard just once, not twice—
The server naps, the startup’s nice.
Hooray for tidy startup lore! 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the primary bug fix: addressing path resolution issues with serveStatic when using absolute webDistPath by stripping the leading slash.
Description check	✅ Passed	The description is comprehensive and well-structured, covering problem statement, UX journey with before/after diagrams, validation evidence, security/compatibility assessment, and rollback plan. All major sections from the template are addressed.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/serve-static-paths

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Wirasm · 2026-04-10T13:24:49Z

PR Review Summary

Reviewed by 5 specialized agents: code-reviewer, docs-impact, silent-failure-hunter, comment-analyzer, code-simplifier. Scope includes PR #1047 diff + latest dev commit 9f727048.

Critical Issues (1 found)

Agent	Issue	Location
comment-analyzer + manual verification	`rewriteRequestPath` is a no-op — the comment claims `path.join` treats leading `/` as root resets, but Bun's `node:path.join('/dist/web', '/assets/foo.js')` correctly returns `/dist/web/assets/foo.js`. Hono's bun adapter imports `join` from `node:path` and passes it to `baseServeStatic`. The `stripLeadingSlash` function changes nothing. The actual fix for the blank page was `9f727048` (keep serve process alive via SIGINT/SIGTERM blocking) — without that, the CLI exited immediately after `startServer()`.	`packages/server/src/index.ts:607-611`

Evidence: Traced through Hono source (dist/adapter/bun/serve-static.js:3,23 → dist/middleware/serve-static/index.js:35-38). The bun adapter always uses node:path.join, which handles leading slashes correctly. Tested: both join('/root', '/assets/foo.js') and join('/root', 'assets/foo.js') produce /root/assets/foo.js.

Important Issues (2 found)

Agent	Issue	Location
code-reviewer, silent-failure-hunter, simplifier	Use `process.once` instead of `process.on` for signal handlers. `process.on` leaves dangling listeners and could accumulate if `serveCommand` were ever called twice. `process.once` auto-removes after first invocation. Also pass `resolve` directly instead of wrapping in arrow functions.	`packages/cli/src/commands/serve.ts:75-82`
silent-failure-hunter	No startup-time existence check on `webDistPath` — if the directory is missing or corrupt, all static file requests silently 404 with no log output. Add an `existsSync` check + `log.warn` at startup so the operator knows why the web UI is blank.	`packages/server/src/index.ts:603-614`

Suggestions (2 found)

Agent	Suggestion	Location
simplifier	Inline `stripLeadingSlash` — it's a single-use one-liner. (Moot if the function is removed per the critical finding.)	`packages/server/src/index.ts:609`
docs-impact	Update the "Static SPA Fallback" code example in `.claude/rules/server-api.md` to reflect the current three-call pattern (`/assets/*`, `/favicon.png`, SPA fallback).	`.claude/rules/server-api.md:67-75`

Strengths

9f727048 (serve keep-alive) is a clean, correct fix — the comment explaining why the blocking promise is needed is accurate and valuable
The favicon.png explicit route is a sensible addition
Error handling in downloadWebDist is thorough (checksums, tar validation, atomic rename)
PR description is excellent — clear UX journey, validation evidence, rollback plan

Recommended Actions

Investigate the critical finding: The rewriteRequestPath + stripLeadingSlash appears to be a no-op with the current Hono bun adapter. Either remove it, or identify the actual root cause that was fixed (likely 9f727048). If there's a scenario where it matters that I missed, document it precisely.
Switch process.on → process.once and pass resolve directly
Add webDistPath existence check at server startup with a log.warn
Update server-api.md static serving example

Verdict

NEEDS INVESTIGATION — The core premise of the PR (that path.join discards the root on absolute path segments) does not hold for Bun's node:path.join. The real fix may already be on dev (9f727048). The rewriteRequestPath change is harmless but misleading.

…ess.once - Remove stripLeadingSlash — Bun's path.join handles leading slashes correctly - Add existsSync check on webDistPath at startup with log.warn - Switch process.on to process.once for signal handlers in serve.ts - Keep favicon.png explicit route (useful addition)

fix(server): strip leading slash in serveStatic for absolute webDistPath

Wirasm merged commit 4b8e4c9 into dev Apr 10, 2026
3 of 4 checks passed

Wirasm deleted the fix/serve-static-paths branch April 10, 2026 13:28

Wirasm mentioned this pull request Apr 10, 2026

Release 0.3.5 #1048

Merged

Tyone88 pushed a commit to Tyone88/Archon that referenced this pull request Apr 16, 2026

Merge pull request coleam00#1047 from coleam00/fix/serve-static-paths

4ddfcf5

fix(server): strip leading slash in serveStatic for absolute webDistPath

joaobmonteiro pushed a commit to joaobmonteiro/Archon that referenced this pull request Apr 26, 2026

Merge pull request coleam00#1047 from coleam00/fix/serve-static-paths

e6f91b3

fix(server): strip leading slash in serveStatic for absolute webDistPath

coderabbitai Bot mentioned this pull request May 30, 2026

fix(web/console): apply BASE_URL prefix to API and favicon paths #1824

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(server): strip leading slash in serveStatic for absolute webDistPath#1047

fix(server): strip leading slash in serveStatic for absolute webDistPath#1047
Wirasm merged 2 commits into
devfrom
fix/serve-static-paths

Wirasm commented Apr 10, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 10, 2026 •

edited

Loading

Review failed

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

Wirasm commented Apr 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Wirasm commented Apr 10, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

UX Journey

Before

After

Label Snapshot

Change Metadata

Validation Evidence (required)

Security Impact (required)

Compatibility / Migration

Human Verification (required)

Side Effects / Blast Radius (required)

Rollback Plan (required)

Risks and Mitigations

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

Wirasm commented Apr 10, 2026

PR Review Summary

Critical Issues (1 found)

Important Issues (2 found)

Suggestions (2 found)

Strengths

Recommended Actions

Verdict

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Wirasm commented Apr 10, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 10, 2026 •

edited

Loading