Maniacs_Mansion/MealQuest
11
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Feature 01 task 3: Embed the PWA build with SPA fallback #4

Merged
approver_agent merged 4 commits from task/embed-pwa-spa-fallback_cdcda39c-8911-4597-8fee-ae1612f7e0a5 into feature/19ad7feb-aba0-4b76-ab70-c47d71ed5e76-repository-scaffold-and-ci 2026-10-02 22:47:02 -04:00
Conversation 4 Commits 4 Files Changed 8
+451 -21
dev_agent commented 2026-10-02 22:32:34 -04:00
Member
Copy Link
Copy Source

Implements task cdcda39c (Feature 01, task 3 of 6).

Done-condition: internal/frontend embeds the built web assets via //go:embed and serves them with Cache-Control immutable for hashed files and no-cache for index.html/sw.js/manifest.webmanifest; any other non-API GET returns index.html (SPA fallback); unknown /api/* returns JSON 404 and never index.html; /auth/, /media/ and /health are not shadowed; tests use a fixture fs.FS; Go quality floor passes.

Local evidence (orchestrator-run at reviewed head): gofmt -l . empty; go vet ./... clean; go test -race -count=1 ./... ok (internal/config, internal/frontend, internal/server, cmd/server); go build ./... ok. Placeholder internal/frontend/dist/index.html is tracked; built assets remain ignored.

Base: feature/19ad7feb-aba0-4b76-ab70-c47d71ed5e76-repository-scaffold-and-ci (f0b2696).

Implements task cdcda39c (Feature 01, task 3 of 6). Done-condition: internal/frontend embeds the built web assets via //go:embed and serves them with Cache-Control immutable for hashed files and no-cache for index.html/sw.js/manifest.webmanifest; any other non-API GET returns index.html (SPA fallback); unknown /api/* returns JSON 404 and never index.html; /auth/*, /media/* and /health are not shadowed; tests use a fixture fs.FS; Go quality floor passes. Local evidence (orchestrator-run at reviewed head): gofmt -l . empty; go vet ./... clean; go test -race -count=1 ./... ok (internal/config, internal/frontend, internal/server, cmd/server); go build ./... ok. Placeholder internal/frontend/dist/index.html is tracked; built assets remain ignored. Base: feature/19ad7feb-aba0-4b76-ab70-c47d71ed5e76-repository-scaffold-and-ci (f0b2696).
dev_agent added 1 commit 2026-10-02 22:32:35 -04:00
Embed the PWA build and serve it with SPA fallback
Independent review Not signed off — 0 high, 1 medium, 1 low (#4)
Details
ee4670754a 
Add internal/frontend, which embeds internal/frontend/dist and serves it
with Cache-Control "no-cache" for index.html, sw.js and
manifest.webmanifest and "public, max-age=31536000, immutable" for every
other asset. Any other GET or HEAD falls back to index.html, except paths
under the reserved /api, /auth, /media and /health prefixes, which get
the JSON not_found error instead.

NewRouter now takes the asset fs.FS so tests use a fstest.MapFS fixture,
mounts an /api subrouter with JSON 404/405 handlers, and fails fast if
the assets lack index.html. A placeholder dist/index.html is tracked so a
fresh clone compiles; built assets remain git-ignored.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
review_agent left a comment
Member
Copy Link
Copy Source

Independent review — Codex gpt-6-sol, reasoning high

Verdict: CHANGES_REQUIRED
Reviewed SHA: ee4670754a18a4db761fb3bcb5aa48f390d02753

{
  "verdict": "CHANGES_REQUIRED",
  "reviewed_sha": "ee4670754a18a4db761fb3bcb5aa48f390d02753",
  "summary": "The embed, placeholder, routing for ordinary paths, and asset caching work in the covered cases. The Go quality checks pass, but path normalization can bypass reserved routes, and the fixture tests omit a normal asset.",
  "findings": [
    {
      "id": "F1",
      "severity": "medium",
      "location": "internal/frontend/frontend.go:68",
      "failure_case": "A GET for /auth/../quests/42 returns index.html with 200. An encoded request for /api%2F..%2Fquests/42 can likewise reach the SPA fallback instead of the required JSON 404.",
      "evidence": "ServeHTTP cleans r.URL.Path before checking reserved prefixes, turning /auth/../quests/42 into /quests/42. Chi routes the encoded API path using RawPath, while this handler checks the decoded, cleaned Path. The tests cover dot segments entering a reserved prefix, but not escaping one.",
      "suggested_correction": "Reject dot-segment paths or check reserved prefixes on both the original decoded path and the cleaned path. Add router tests for both examples."
    },
    {
      "id": "F2",
      "severity": "low",
      "location": "internal/frontend/frontend_test.go:20",
      "failure_case": "A regression that sends a normal, unhashed asset to index.html or gives it the wrong Cache-Control value would pass the current tests.",
      "evidence": "The fixture contains one ordinary asset, assets/index-AbC123.js, which is hashed; the other fixture files are the three no-cache exceptions.",
      "suggested_correction": "Add an unhashed normal asset to the fixture and assert its body, 200 status, and exact immutable Cache-Control value."
    }
  ]
}

Posted unaltered by review_agent. This is transport, not a second review.

## Independent review — Codex `gpt-6-sol`, reasoning high Verdict: **CHANGES_REQUIRED** Reviewed SHA: `ee4670754a18a4db761fb3bcb5aa48f390d02753` ```json { "verdict": "CHANGES_REQUIRED", "reviewed_sha": "ee4670754a18a4db761fb3bcb5aa48f390d02753", "summary": "The embed, placeholder, routing for ordinary paths, and asset caching work in the covered cases. The Go quality checks pass, but path normalization can bypass reserved routes, and the fixture tests omit a normal asset.", "findings": [ { "id": "F1", "severity": "medium", "location": "internal/frontend/frontend.go:68", "failure_case": "A GET for /auth/../quests/42 returns index.html with 200. An encoded request for /api%2F..%2Fquests/42 can likewise reach the SPA fallback instead of the required JSON 404.", "evidence": "ServeHTTP cleans r.URL.Path before checking reserved prefixes, turning /auth/../quests/42 into /quests/42. Chi routes the encoded API path using RawPath, while this handler checks the decoded, cleaned Path. The tests cover dot segments entering a reserved prefix, but not escaping one.", "suggested_correction": "Reject dot-segment paths or check reserved prefixes on both the original decoded path and the cleaned path. Add router tests for both examples." }, { "id": "F2", "severity": "low", "location": "internal/frontend/frontend_test.go:20", "failure_case": "A regression that sends a normal, unhashed asset to index.html or gives it the wrong Cache-Control value would pass the current tests.", "evidence": "The fixture contains one ordinary asset, assets/index-AbC123.js, which is hashed; the other fixture files are the three no-cache exceptions.", "suggested_correction": "Add an unhashed normal asset to the fixture and assert its body, 200 status, and exact immutable Cache-Control value." } ] } ``` --- Posted unaltered by `review_agent`. This is transport, not a second review.
dev_agent added 1 commit 2026-10-02 22:37:17 -04:00
Keep reserved prefixes out of the SPA fallback under dot segments
Independent review Not signed off — 1 high (#4)
Details
efae9bed7e 
Address review findings F1 and F2:
- F1: check reserved prefixes against the request path before and after
  cleaning, so /auth/../quests/42 and /api%2F..%2Fquests/42 get the JSON
  404 instead of index.html. Add frontend and router tests for both.
- F2: add an unhashed asset (robots.txt) to the frontend fixture and
  assert its status, body and immutable Cache-Control.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
review_agent left a comment
Member
Copy Link
Copy Source

Independent review — Codex gpt-6-sol, reasoning high

Verdict: CHANGES_REQUIRED
Reviewed SHA: efae9bed7efa3ba0c3eea0afdab81f365521a5af

{
  "verdict": "CHANGES_REQUIRED",
  "reviewed_sha": "efae9bed7efa3ba0c3eea0afdab81f365521a5af",
  "summary": "The embed, routing, cache headers, fixture tests, and Go quality checks pass. The build does not put the web assets into the embedded directory, so a normal Go build serves the placeholder instead of the app.",
  "findings": [
    {
      "id": "F1",
      "severity": "high",
      "location": "cmd/server/main.go:41",
      "failure_case": "After running the web build and then building the server, GET / serves the committed placeholder page instead of the built PWA.",
      "evidence": "The server embeds internal/frontend/dist. web/package.json:8 runs only `tsc --noEmit && vite build`; web/vite.config.ts sets no output directory, and no tracked build step copies assets into internal/frontend/dist. The only tracked file there is the placeholder index.html.",
      "suggested_correction": "Make the build place the Vite output in internal/frontend/dist before compiling Go, and verify that the resulting server serves the built index and assets."
    }
  ]
}

Posted unaltered by review_agent. This is transport, not a second review.

## Independent review — Codex `gpt-6-sol`, reasoning high Verdict: **CHANGES_REQUIRED** Reviewed SHA: `efae9bed7efa3ba0c3eea0afdab81f365521a5af` ```json { "verdict": "CHANGES_REQUIRED", "reviewed_sha": "efae9bed7efa3ba0c3eea0afdab81f365521a5af", "summary": "The embed, routing, cache headers, fixture tests, and Go quality checks pass. The build does not put the web assets into the embedded directory, so a normal Go build serves the placeholder instead of the app.", "findings": [ { "id": "F1", "severity": "high", "location": "cmd/server/main.go:41", "failure_case": "After running the web build and then building the server, GET / serves the committed placeholder page instead of the built PWA.", "evidence": "The server embeds internal/frontend/dist. web/package.json:8 runs only `tsc --noEmit && vite build`; web/vite.config.ts sets no output directory, and no tracked build step copies assets into internal/frontend/dist. The only tracked file there is the placeholder index.html.", "suggested_correction": "Make the build place the Vite output in internal/frontend/dist before compiling Go, and verify that the resulting server serves the built index and assets." } ] } ``` --- Posted unaltered by `review_agent`. This is transport, not a second review.
dev_agent added 1 commit 2026-10-02 22:41:28 -04:00
Add make build to embed the built PWA in the server binary
Independent review Not signed off — 0 high, 1 medium (#4)
Details
8236c5e349 
Fixes review finding F1: nothing copied web/dist into
internal/frontend/dist, so the server always embedded the placeholder.
make build now builds the web app, replaces internal/frontend/dist with
web/dist, and builds bin/mealquest.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
review_agent left a comment
Member
Copy Link
Copy Source

Independent review — Codex gpt-6-sol, reasoning high

Verdict: CHANGES_REQUIRED
Reviewed SHA: 8236c5e349372307f074d31d05fcfe9844e9c63f

{
  "verdict": "CHANGES_REQUIRED",
  "reviewed_sha": "8236c5e349372307f074d31d05fcfe9844e9c63f",
  "summary": "The embed, router behavior, fixture tests, and all four Go quality checks pass. The build can still embed and serve stale hidden files.",
  "findings": [
    {
      "id": "F1",
      "severity": "medium",
      "location": "Makefile:15",
      "failure_case": "If a hidden file remains in internal/frontend/dist from an earlier build, a later build retains it. A GET for that file can return its contents, including sensitive data if the file contains it.",
      "evidence": "The cleanup glob `dist/*` excludes dotfiles. `frontend.go:20` embeds hidden files with `all:dist`, and `frontend.go:68-81` serves any existing regular file outside reserved paths.",
      "suggested_correction": "Remove and recreate the entire dist directory before copying web/dist, then add a regression check that a stale hidden file is absent from the resulting build."
    }
  ]
}

Posted unaltered by review_agent. This is transport, not a second review.

## Independent review — Codex `gpt-6-sol`, reasoning high Verdict: **CHANGES_REQUIRED** Reviewed SHA: `8236c5e349372307f074d31d05fcfe9844e9c63f` ```json { "verdict": "CHANGES_REQUIRED", "reviewed_sha": "8236c5e349372307f074d31d05fcfe9844e9c63f", "summary": "The embed, router behavior, fixture tests, and all four Go quality checks pass. The build can still embed and serve stale hidden files.", "findings": [ { "id": "F1", "severity": "medium", "location": "Makefile:15", "failure_case": "If a hidden file remains in internal/frontend/dist from an earlier build, a later build retains it. A GET for that file can return its contents, including sensitive data if the file contains it.", "evidence": "The cleanup glob `dist/*` excludes dotfiles. `frontend.go:20` embeds hidden files with `all:dist`, and `frontend.go:68-81` serves any existing regular file outside reserved paths.", "suggested_correction": "Remove and recreate the entire dist directory before copying web/dist, then add a regression check that a stale hidden file is absent from the resulting build." } ] } ``` --- Posted unaltered by `review_agent`. This is transport, not a second review.
dev_agent added 1 commit 2026-10-02 22:44:57 -04:00
Recreate the embed directory so stale dotfiles cannot be served
Independent review Approved — 0 high findings (#4)
Details
7ba2a37da2 
Review finding F1: the cleanup glob internal/frontend/dist/* skipped
hidden files, which all:dist embeds. Remove and recreate the whole
directory before copying web/dist, and add a check-embed-clean target
that plants a stale hidden file and asserts the embed removes it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
review_agent left a comment
Member
Copy Link
Copy Source

Independent review — Codex gpt-6-sol, reasoning high

Verdict: APPROVED
Reviewed SHA: 7ba2a37da2a51c453b6713795f638a8cb3ea82c8

{
  "verdict": "APPROVED",
  "reviewed_sha": "7ba2a37da2a51c453b6713795f638a8cb3ea82c8",
  "summary": "The embed, cache rules, SPA fallback, reserved routes, fixture tests, and tracked assets meet the stated requirements. gofmt, go vet, race tests, and go build passed; the checkout remained unchanged.",
  "findings": []
}

Posted unaltered by review_agent. This is transport, not a second review.

## Independent review — Codex `gpt-6-sol`, reasoning high Verdict: **APPROVED** Reviewed SHA: `7ba2a37da2a51c453b6713795f638a8cb3ea82c8` ```json { "verdict": "APPROVED", "reviewed_sha": "7ba2a37da2a51c453b6713795f638a8cb3ea82c8", "summary": "The embed, cache rules, SPA fallback, reserved routes, fixture tests, and tracked assets meet the stated requirements. gofmt, go vet, race tests, and go build passed; the checkout remained unchanged.", "findings": [] } ``` --- Posted unaltered by `review_agent`. This is transport, not a second review.
approver_agent merged commit 85569b451f into feature/19ad7feb-aba0-4b76-ab70-c47d71ed5e76-repository-scaffold-and-ci 2026-10-02 22:47:02 -04:00
Sign in to join this conversation.
Reviewers
No Reviewers
review_agent
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
approver_agent (The Approver) chatgpt claude-ai Claude-Code codex (Codex) dev_agent grok opencode orchestrator-agent overseer (Joshua) review_agent (The Reviewer)
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Maniacs_Mansion/MealQuest#4