Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- CSP: pin sha256 of the app's inline theme/FOUC bootstrap instead of opening script-src to 'unsafe-inline', matching the diffsentry.app nginx precedent. A future edit to that bootstrap is fail-loud (visible CSP console error), caught by the demo smoke. - Add .dockerignore so 'COPY web/ ./web/' can't overlay the host's platform-specific node_modules onto the container npm ci; build context is ~6kB and reproducible from source (applies to the main Dockerfile too). Verified via Playwright on :3027 => 0 console errors, 0 /api calls. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
📝 WalkthroughWalkthroughAdded a standalone nginx configuration to serve the public demo as a static SPA on its own host. Hardened the demo container with SPA routing, caching rules, security headers, and a pinned CSP. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Nginx
Browser->>Nginx: GET /overview
Nginx->>Nginx: try_files $uri $uri/ /index.html
Nginx-->>Browser: index.html with CSP and no-store
Browser->>Nginx: GET /assets/app-hash.js
Nginx-->>Browser: cached static asset with immutable cache headers
sequenceDiagram
participant Monitor
participant Nginx
Monitor->>Nginx: GET /health
Nginx-->>Monitor: 200 ok
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Suggested Labels
Risk AssessmentScore: 0/100 — 🟢 Low No elevated risk signals detected. 🧭 Description Drift
Compares PR description claims to the actual diff. Update the description (or the code) so they tell the same story. ✍️ Commit Message Coach1 of 9 commit messages could be stronger.
Tip: imperative mood ( 🚥 Pre-merge checks | ✅ 5 | ❌ 0✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in your ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Generate docstrings (beta)
🧹 Simplify (beta)
🪄 Autofix unresolved comments (beta)
Comment |
![diffsentry[bot]](https://avatars.githubusercontent.com/in/3360270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
This PR is well-structured and the force-demo split is mostly implemented correctly: the demo routing logic, root-basename behavior, and redirect-loop guard all line up with the stated goal. The main issue is in the demo nginx health endpoint, which currently sets Content-Type with add_header on a return response and can end up serving /health as the default application/octet-stream; that makes the endpoint less predictable for health probes and curl-based checks. Aside from that, the changes look cohesive and low-risk.
🧹 Nitpick comments (1)
deploy/demo-nginx.conf (1)
Line 36: The health endpoint does not reliably return a text/plain Content-Type.
⚠️ Potential issue | 🟡 MinorThe health endpoint does not reliably return a text/plain Content-Type.
add_header Content-Type text/plain;is not the right mechanism for setting the response MIME type on areturn-generated body in nginx. For literalreturn 200 'ok';responses, nginx can still emit its default type, which makes/healthless deterministic for probes and contradicts the intent of the config comment.Set the MIME type with
default_typeinside the exact-match location before returning the body. That guarantees clients and diagnostics consistently seetext/plainfor the health response.🔧 Proposed fix
location = /health { default_type text/plain; return 200 'ok'; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In deploy/demo-nginx.conf around line 36, change the `/health` location to set the response MIME type with `default_type text/plain` instead of `add_header Content-Type`; keep the `return 200 'ok';` response body unchanged. Make the replacement directly in the exact-match `/health` block so health probes get a deterministic `text/plain` response.♻️ Suggested fix
location = /health { default_type text/plain; return 200 'ok'; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In deploy/demo-nginx.conf around line 36, change the `/health` location to set the response MIME type with `default_type text/plain` instead of `add_header Content-Type`; keep the `return 200 'ok';` response body unchanged. Make the replacement directly in the exact-match `/health` block so health probes get a deterministic `text/plain` response.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
In `deploy/demo-nginx.conf`:
- Line 36: In deploy/demo-nginx.conf around line 36, change the `/health` location to set the response MIME type with `default_type text/plain` instead of `add_header Content-Type`; keep the `return 200 'ok';` response body unchanged. Make the replacement directly in the exact-match `/health` block so health probes get a deterministic `text/plain` response.
🪄 Autofix (Beta)
Fix all unresolved DiffSentry comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: .diffsentry.yaml
Review profile: ASSERTIVE
Run ID: d49d0ac3-34e1-404c-a223-72f560b17712
📥 Commits
Reviewing files at 372b027 (base SHA unavailable).
📒 Files selected for processing (10)
.dockerignoreDockerfile.demodeploy/demo-nginx.confdocker-compose.ymldocs/superpowers/plans/2026-07-04-move-demo-to-static-site.mddocs/superpowers/specs/2026-07-04-move-demo-to-static-site-design.mdweb/src/demo/mode.tsweb/src/main.tsxweb/src/vite-env.d.tsweb/vite.config.ts
✅ Files skipped from review due to trivial changes (1)
package.json
✂️ Files with diffs truncated to fit the review size budget (1)
docs/superpowers/plans/2026-07-04-move-demo-to-static-site.md
📌 Status — last updated
|
| Risk score | 0/100 (Low) ███▁ |
| Unresolved threads | 0 |
| Failing checks | 0 |
| Pending checks | 1 |
| Files reviewed | 1 (10 skipped) |
| Updated | 2026-07-04 22:24Z |
Live-updated by DiffSentry on every push. Use @diffsentry ship for a verdict, @diffsentry timeline for full history.
add_header Content-Type appends a header on top of the one nginx already sets from default_type (application/octet-stream in the base image) for the return, yielding a duplicate/nondeterministic Content-Type. default_type sets a single, correct text/plain. Body 'ok' unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
The nginx demo container is a clean extraction of the public demo surface and the config is generally solid: SPA fallback, immutable asset caching, and a narrowly scoped CSP are all in place. The main thing missing is ensuring the shell document itself is never cached, otherwise deploys can strand users on an old index.html that references no-longer-present hashed assets. I also recommend tightening the health endpoint so it is not rewritten through the SPA location precedence in future edits.
🧹 Nitpick comments (1)
deploy/demo-nginx.conf (1)
Line 34: Place the health check before the generic SPA fallback for defensive clarity.🛠️ Refactor suggestion | 🟡 Minor
Place the health check before the generic SPA fallback for defensive clarity.
nginx will currently prefer
location = /healthoverlocation /, so this works as written. The problem is maintainability: the config relies on readers remembering nginx's location precedence, and a later regex or higher-priority location can accidentally change that behavior.Moving the exact-match
/healthblock above the SPA fallback makes the intent obvious and reduces the chance that future edits turn the health endpoint into an HTML response from/index.html. This is a low-risk cleanup, but worth doing in a deployment-facing config file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In deploy/demo-nginx.conf around line 34, move the exact-match `location = /health` block above the generic `location /` SPA fallback. Preserve the existing response body and status code; this is only to make routing intent explicit and safer against future nginx location changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In deploy/demo-nginx.conf around line 34, move the exact-match `location = /health` block above the generic `location /` SPA fallback. Preserve the existing response body and status code; this is only to make routing intent explicit and safer against future nginx location changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
In `deploy/demo-nginx.conf`:
- Line 29: In deploy/demo-nginx.conf around line 29, add an exact-match caching rule for `index.html`; set `Cache-Control` to `no-store` so the SPA shell is always refreshed while `/assets/` remains immutable. Keep the existing `/assets/` caching policy unchanged and place the new block before the generic `location /` fallback.
- Line 34: In deploy/demo-nginx.conf around line 34, move the exact-match `location = /health` block above the generic `location /` SPA fallback. Preserve the existing response body and status code; this is only to make routing intent explicit and safer against future nginx location changes.
🪄 Autofix (Beta)
Fix all unresolved DiffSentry comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: .diffsentry.yaml
Review profile: ASSERTIVE
Run ID: 361e9c9d-d1f5-40bb-9c5e-883cfb9c15ee
📥 Commits
Reviewing files that changed from 372b027 to 9736fbc. Previously-reviewed commits are not re-reviewed.
📒 Files selected for processing (1)
deploy/demo-nginx.conf
✅ Files skipped from review due to trivial changes (1)
package.json
🚧 Files skipped from review as they are similar to previous changes (9)
.dockerignoreDockerfile.demodocker-compose.ymldocs/superpowers/plans/2026-07-04-move-demo-to-static-site.mddocs/superpowers/specs/2026-07-04-move-demo-to-static-site-design.mdweb/src/demo/mode.tsweb/src/main.tsxweb/src/vite-env.d.tsweb/vite.config.ts
…ference gone bundles index.html is the Vite release manifest (points at hashed /assets that are replaced each deploy); a cached shell could request bundles that no longer exist. Default to Cache-Control: no-store at the server level; the /assets location overrides it with immutable for the content-addressed bundles. Server level (not a per-location add_header on the shell) so index.html keeps the security + CSP headers — a location add_header would drop that inheritance. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
The standalone demo nginx config is mostly solid: it serves the SPA with a strict CSP, sensible cache behavior for index.html versus hashed assets, and a simple health endpoint. The main gap is that the /assets/ location overrides inherited headers in nginx, which means hashed JS/CSS responses will likely lose the global security headers and CSP unless they are re-declared there.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
In `deploy/demo-nginx.conf`:
- Line 32: In deploy/demo-nginx.conf around line 32, update the `location /assets/` block so asset responses keep the same hardening headers as the parent `server` block. Re-declare `X-Frame-Options`, `X-Content-Type-Options`, `Referrer-Policy`, and `Content-Security-Policy` inside `/assets/` alongside the asset-specific `Cache-Control` override, because nginx `add_header` in a nested location replaces inherited headers rather than merging them.
🪄 Autofix (Beta)
Fix all unresolved DiffSentry comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: .diffsentry.yaml
Review profile: ASSERTIVE
Run ID: 221b951f-24f2-4dce-9aa5-e19f809e7a05
📥 Commits
Reviewing files that changed from 9736fbc to ba41efb. Previously-reviewed commits are not re-reviewed.
📒 Files selected for processing (1)
deploy/demo-nginx.conf
✅ Files skipped from review due to trivial changes (1)
package.json
🚧 Files skipped from review as they are similar to previous changes (9)
.dockerignoreDockerfile.demodocker-compose.ymldocs/superpowers/plans/2026-07-04-move-demo-to-static-site.mddocs/superpowers/specs/2026-07-04-move-demo-to-static-site-design.mdweb/src/demo/mode.tsweb/src/main.tsxweb/src/vite-env.d.tsweb/vite.config.ts
Defining add_header inside location /assets/ replaces (does not merge) the inherited server-level headers, so the hashed JS/CSS were shipping without X-Frame-Options / X-Content-Type-Options / Referrer-Policy. Re-assert them in the block. CSP is intentionally not repeated: a CSP response header is inert on a static sub-resource (only the document's CSP governs script/style loads), so there's no benefit to duplicating the long policy string. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
The new standalone demo nginx config is clean and well-documented, with sensible SPA fallback, cache policy separation between index.html and hashed assets, and an appropriately strict CSP for a static fixture-backed app. I did not find any correctness, security, or maintainability issues in the added file based on the diff provided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: .diffsentry.yaml
Review profile: ASSERTIVE
Run ID: 2f3d0f59-3914-444c-8233-d179f9f454d5
📥 Commits
Reviewing files that changed from ba41efb to 59b77ad. Previously-reviewed commits are not re-reviewed.
📒 Files selected for processing (1)
deploy/demo-nginx.conf
✅ Files skipped from review due to trivial changes (1)
package.json
🚧 Files skipped from review as they are similar to previous changes (9)
.dockerignoreDockerfile.demodocker-compose.ymldocs/superpowers/plans/2026-07-04-move-demo-to-static-site.mddocs/superpowers/specs/2026-07-04-move-demo-to-static-site-design.mdweb/src/demo/mode.tsweb/src/main.tsxweb/src/vite-env.d.tsweb/vite.config.ts
|
@diffsentry release-notes |
📣 Release Notes✨ What's new
🛠 Improvements
🐛 Fixes
Marketing-speak version of this PR. Re-run with |
1 participant
What & why
The public demo was served from my real instance. The changes in this PR make it a standalone artifact so my instance carries no anonymous public surface.
The demo is the dashboard SPA's existing fixture-backed demo mode (zero network calls). A new build-time flag serves it at the domain root.
Changes
VITE_FORCE_DEMObuild flag (web/src/demo/mode.ts,vite.config.ts,vite-env.d.ts): hard-locks demo mode, router basename/, detection kept on the literal/demo(not the basename). PWA/service-worker disabled in the forced build viaVitePWA({ disable }).web/src/main.tsx): the/demobounce is skipped underFORCE_DEMO.build:web:demoscript;Dockerfile.demo+deploy/demo-nginx.conf→ nginx image serving the fixture SPA on port 3027, SPA fallback,/health, security headers, CSP pinned to the app's one inline theme-bootstrap script (sha256). Ships no server, DB, keys, or.diffsentry.yaml.demoservice indocker-compose.yml(3027:80);.dockerignorefor reproducible builds.Verification (Playwright on :3027)
/→/overview, no/demoprefix, no redirect loop)./apirequests, zero console/CSP errors; deep links render fixtures with the demo-mode banner.Summary
Added a standalone nginx configuration to serve the public demo as a static SPA on its own host. Hardened the demo container with SPA routing, caching rules, security headers, and a pinned CSP.
Changes
deploy/demo-nginx.conf