feat: add demo site hosted on Cloudflare Workers - #69
Merged
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis pull request establishes a comprehensive demo mode infrastructure. It introduces a Cloudflare Worker serving mock API endpoints with seeded demo data, adds TypeScript-based routing for entries, incidents, nodes, and admin resources, integrates demo-mode toggles into the web frontend with conditional session handling, and provides deployment automation via GitHub Actions and shell scripts. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/Browser
participant DemoWorker as Demo Worker
participant AssetBind as ASSETS Binding
participant SPAIndex as index.html
rect rgba(100, 200, 150, 0.5)
note over Client,SPAIndex: Demo Mode: SPA with Mock API
Client->>DemoWorker: GET /incidents
DemoWorker->>DemoWorker: Route to /incidents handler
DemoWorker->>DemoWorker: Filter & paginate seeded incidents
DemoWorker-->>Client: 200 JSON (mock incidents)
Client->>DemoWorker: GET /app.js (SPA asset)
DemoWorker->>AssetBind: Fetch /app.js from ASSETS
AssetBind-->>DemoWorker: Asset found
DemoWorker-->>Client: 200 (asset)
Client->>DemoWorker: GET /nonexistent (404 asset)
DemoWorker->>AssetBind: Fetch /nonexistent from ASSETS
AssetBind-->>DemoWorker: 404 Not Found
DemoWorker->>AssetBind: Fetch /index.html (rewrite)
AssetBind-->>DemoWorker: index.html asset
DemoWorker-->>Client: 200 (index.html, enables client-side routing)
end
rect rgba(150, 150, 200, 0.5)
note over Client,DemoWorker: Session Handling in Demo Mode
Client->>Client: SessionProvider initialized with fixedUser
Client->>Client: Render with DEMO_SESSION_USER (demo-admin)
Client->>Client: WebSocketProvider provides stub context (no real WS)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/deploy-demo.yml:
- Around line 31-33: Replace the "run: npm install" step in the "Install worker
dependencies" GitHub Actions step with "run: npm ci" so the demo/worker install
uses the lockfile-pinned, reproducible install (matching the web frontend step);
ensure demo/worker/package-lock.json is committed to the repo so npm ci
succeeds.
- Around line 12-16: The workflow’s actions/setup-node@v4 step is only caching
web/package-lock.json, so installs for demo/worker are always cold; update the
cache-dependency-path value on the setup-node step to include both lockfiles
(e.g., list web/package-lock.json and demo/worker/package-lock.json using the
multi-line or array syntax supported by actions/setup-node@v4) so the worker’s
npm install can reuse the cache; keep the same setup-node step (uses:
actions/setup-node@v4) and only change the cache-dependency-path key to include
both lockfile paths.
- Around line 35-39: The workflow's "Deploy worker" step runs "npx wrangler
deploy" without specifying account context, causing failures when the API token
can access multiple accounts; fix by adding the Cloudflare account ID either as
an environment variable named CLOUDFLARE_ACCOUNT_ID in the same step (alongside
CLOUDFLARE_API_TOKEN) or by adding account_id to your wrangler.toml so "npx
wrangler deploy" has an unambiguous target; update the step that currently
exports CLOUDFLARE_API_TOKEN and references "npx wrangler deploy" (the Deploy
worker step) to include the account id or ensure wrangler.toml contains the
correct account_id value.
- Around line 6-9: Add a concurrency stanza to the deploy job to prevent
overlapping runs: under the job named "deploy" (the job block containing
"runs-on: ubuntu-latest" and "steps"), add a concurrency key with a stable group
name (e.g., "deploy-demo") and set "cancel-in-progress: true" so any second
workflow run will cancel or queue behind the first, preventing interleaved
rm/cp/wrangler deploy steps.
In `@demo/deploy.sh`:
- Around line 7-14: The script currently runs build and deploy without
installing dependencies; update the deploy.sh flow to run a deterministic
install (e.g., npm ci) in the web directory before running npm run build:demo
and again in the demo/worker directory before running npx wrangler deploy so
both the web build and the worker deploy have their node_modules and pinned
devDependencies (like local wrangler) present; perform the installs by changing
directory to the target folders (use the existing cd "${REPO_ROOT}/web" and cd
"${REPO_ROOT}/demo/worker" steps) and fail early on install/build/deploy errors
to avoid partial deployments.
In `@demo/worker/package.json`:
- Around line 6-19: Add an explicit Node engine baseline and make the test
script cross‑platform: update package.json to include an "engines" entry pinning
Node to a minimum of 22.6.0 (e.g., "node": ">=22.6.0") so scripts like "check"
and the experimental flag used in "test" run predictably, and modify the "test"
script (currently "node --experimental-strip-types --test tests/*.test.ts") to
use Node's glob form (for example --test "tests/**/*.test.ts") to avoid shell
globbing issues on Windows; keep the rest of the scripts and dependencies
unchanged.
In `@demo/worker/src/routes/admin.ts`:
- Around line 53-62: The route handlers for '/admin/audit-logs' and
'/admin/webhook-deliveries' convert query params with Number(...) which yields
NaN for invalid input; update the page and perPage parsing to use the same
safe-clamp pattern as entries.ts (e.g., parse then apply Number.isFinite(page) ?
page : 1 and Number.isFinite(perPage) ? perPage : 50) before calling
paginate(filterAuditLogs(...)) and paginate(filterWebhookDeliveries(...));
alternatively, implement the guard inside paginate() so it normalizes non-finite
page/limit inputs to sensible defaults.
- Around line 44-52: The endpoint handler for app.get('/admin/settings/systemd')
directly calls JSON.parse on source.config which can throw on malformed input;
wrap the parse in a safe try/catch (or use a small parseSafe helper) when
mapping over data.dataSourceInstances so that if JSON.parse fails you default to
an empty { units: [] } (or [] for the units) instead of letting the whole
request crash; update the mapping that creates units (using the existing
source.config and node_id references) to use the safe-parse result and ensure
units is coerced to an array of strings as before.
- Around line 23-26: filterAuditLogs currently uses substring matching via
item.action.includes(action), which differs from the backend's exact equality;
update the function (filterAuditLogs) to perform exact equality (item.action ===
action) when action is provided, preserving the existing behavior of returning
all items sorted by compareByCreatedAtDesc when action is null/undefined, and
keep the sort call using compareByCreatedAtDesc.
In `@demo/worker/src/routes/incidents.ts`:
- Around line 73-102: The /Length value is using stream.length (UTF-16 code
units) not the byte length after UTF-8 encoding, which corrupts PDFs with
non-ASCII characters; inside buildSimplePdf, encode the stream with TextEncoder
first, use the resulting Uint8Array.byteLength for the `/Length` entry (replace
`stream.length`), and then compute xrefOffset and object offsets in bytes (not
string.length) by either building the entire PDF as Uint8Array chunks or
tracking each object's encoded byte length before concatenation; update
references to `stream`, `objects`, and `xrefOffset` accordingly so all offsets
written to the xref table reflect byte positions.
In `@demo/worker/tests/incidents.test.ts`:
- Around line 22-35: The test named "filterIncidents applies status, confidence,
service, and limit filters" mentions a confidence filter but never exercises it;
update the test in which filterIncidents is called (the test block and variables
open/vaultwarden) to either rename the test to remove "confidence" or add a
confidence case: call filterIncidents(demoData.incidents, { confidence: 0.9 })
(or the appropriate confidence value/type used by filterIncidents) into a new
variable (e.g., confidenceFiltered) and assert that every incident in
confidenceFiltered meets the confidence criterion (e.g., incident.confidence >=
0.9 or matches the expected confidence enum/value) so the test name matches its
assertions.
In `@demo/worker/wrangler.toml`:
- Line 12: Change the Worker routing so static assets are served directly by the
Assets binding by removing or setting run_worker_first = false in wrangler.toml;
specifically, update the run_worker_first setting (or delete it) and rely on
not_found_handling = "single-page-application" to perform the SPA fallback
instead of letting the Worker (and its c.env.ASSETS.fetch() path) handle every
request.
In `@web/package.json`:
- Line 9: The "build:demo" npm script currently runs only "vite build --mode
demo" and skips TypeScript checking; update the "build:demo" script to run the
TypeScript build step before Vite (e.g., invoke "tsc -b && vite build --mode
demo") so it mirrors the main "build" script and prevents type regressions from
being deployed; modify the "build:demo" entry in package.json accordingly.
In `@web/src/App.tsx`:
- Around line 21-40: Extract the duplicated "/" shell subtree into a small
helper (e.g., buildShellRoutes or ShellRoutes) that returns the <Route path="/"
element={<Shell />}> with its nested routes (IncidentsPage, TimelinePage,
NodesPage, WebhooksPage, AccountPage, AdminPage, DiagnosticsPage and the
catch-all Navigate to /incidents), then replace the identical block inside
DemoRoutes and the bootstrapped branch of StandardRoutes with a call to that
helper; ensure the helper preserves the index Navigate and replace behavior and
export or place it so both DemoRoutes and StandardRoutes can import/use it.
In `@web/src/components/DemoIntroModal.tsx`:
- Around line 6-110: The component DemoIntroModal uses inline style objects and
a fontFamily constant instead of the project's Tailwind utility classes and
design tokens; refactor the JSX to replace style={{...}} and the fontFamily
symbol with appropriate Tailwind classes (for layout, spacing, colors, borders,
typography, z-index, positioning and hover/focus states) and remove the inline
hex literals, while keeping behavior intact (preserve useState/open, useEffect
reading DEMO_INTRO_DISMISSED_KEY from localStorage and the onClick that sets it
and calls setOpen(false)); ensure icons (Boxes, ShieldAlert, ExternalLink,
ArrowRight) and links remain rendered and that the modal overlay and container
map to existing Tailwind tokens for background opacity, shadow, and max width.
- Around line 18-111: The modal (DemoIntroModal) lacks ARIA semantics and
keyboard handling; add role="dialog" and aria-modal="true" to the modal
container, create unique ids (e.g., titleId/descId) and set
aria-labelledby/aria-describedby to point at the header and description
elements, and ensure the component uses those ids; wire an Escape key handler
that calls the same dismiss path
(window.localStorage.setItem(DEMO_INTRO_DISMISSED_KEY, 'true') and
setOpen(false)) and attach it on mount/unmount; move initial focus into the
dialog by adding a ref to the EXPLORE button and calling focus() on mount (or
set autoFocus on that button) and consider handling backdrop clicks to trigger
the same dismiss function so clicking outside also closes the modal.
In `@web/src/components/WebSocketProvider.tsx`:
- Around line 22-37: The hook useWebSocket(wsUrl) is called conditionally inside
WebSocketProvider when demoMode is false, violating Rules-of-Hooks; refactor so
hooks run unconditionally by either (preferred) splitting into two components —
keep demo-only provider (returns WebSocketContext.Provider with static
disconnected state) and create a separate LiveWebSocketProvider that always
calls useWebSocket(wsUrl) and provides the real state to WebSocketContext, then
have WebSocketProvider choose between rendering the demo component or the live
component; or (alternative) change useWebSocket to accept a nullable/disabled
URL (e.g., wsUrl | null / disabled flag) and handle the disabled case internally
so WebSocketProvider can call useWebSocket(wsUrl) unconditionally while still
returning the demo context when demoMode is true.
In `@web/src/demoMode.ts`:
- Around line 5-11: DEMO_SESSION_USER currently uses user_id 'demo-admin' which
mismatches the backend demo identity 'user-admin'; update the
DEMO_SESSION_USER.user_id to 'user-admin' to align frontend demo identity with
backend demo data and ensure any places referencing DEMO_SESSION_USER (and
related demo constructs like fixedUser/session demo logic) use the same ID so
identities remain consistent across UI and backend.
In `@web/src/session.tsx`:
- Around line 26-29: updateSession currently uses setUser(isFixedSession ?
(fixedUser ?? nextUser) : nextUser) which allows a fixedUser value of null to be
overridden by nextUser; change the fixed-session branch to honor a pinned null
by using fixedUser ?? null (i.e., setUser(isFixedSession ? (fixedUser ?? null) :
nextUser)). This aligns updateSession with refreshSession, logout, and the
effect so a fixed null session remains unauthenticated; keep the same dependency
array ([fixedUser, isFixedSession]).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 060b63b8-5b95-4b7a-b9b0-0af2a21b4d75
📒 Files selected for processing (23)
.github/workflows/deploy-demo.yml.gitignoredemo/deploy.shdemo/worker/package.jsondemo/worker/src/index.tsdemo/worker/src/routes/admin.tsdemo/worker/src/routes/entries.tsdemo/worker/src/routes/incidents.tsdemo/worker/src/routes/nodes.tsdemo/worker/src/seed-data.tsdemo/worker/src/types.tsdemo/worker/tests/entries.test.tsdemo/worker/tests/incidents.test.tsdemo/worker/tsconfig.jsondemo/worker/wrangler.tomlweb/.env.demoweb/package.jsonweb/src/App.tsxweb/src/components/DemoIntroModal.tsxweb/src/components/WebSocketProvider.tsxweb/src/demoMode.tsweb/src/session.tsxweb/tests/demo-mode.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Chores