feat(demo): Runbook widget, demo pages, and env example - #7
Merged
Merged
Conversation
Add GitHub and expenses demo surfaces with a floating Runbook assistant widget and task completion that syncs to the dashboard via localStorage with optional POST /api/tasks/update. Include Playwright QA script, screenshot artifacts from the last clean run, and .env.example plus gitignore so .env.example can be committed while .env.local stays private. Set turbopack.root to silence multi-lockfile warnings during local builds. Made-with: Cursor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 32 minutes and 45 seconds. ⌛ 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThis PR introduces a comprehensive task management and QA automation system. It adds environment configuration for API credentials, Playwright-based QA automation with bug logging, updates the dashboard with client-side task tracking, creates demo pages for GitHub and expenses workflows, and implements a RunbookWidget system with localStorage-based task status persistence and API synchronization capabilities. Changes
Sequence DiagramsequenceDiagram
actor User
participant RunbookWidget as RunbookWidget<br/>(Client)
participant localStorage as localStorage
participant API as /api/tasks/update
participant Dashboard as Dashboard<br/>(Subscriber)
User->>RunbookWidget: Click "Mark Complete"
RunbookWidget->>RunbookWidget: Set saving=true
RunbookWidget->>API: POST updateTaskStatus(taskId, "complete")
alt API Success
API-->>RunbookWidget: { success: true }
RunbookWidget->>localStorage: Save status + "api" source
RunbookWidget->>RunbookWidget: Dispatch custom event
else API Failure
API-->>RunbookWidget: Error
RunbookWidget->>localStorage: Save status + "local" source
RunbookWidget->>RunbookWidget: Dispatch custom event
end
RunbookWidget->>RunbookWidget: Set saving=false
RunbookWidget-->>User: Show confirmation
Dashboard->>localStorage: Listen for storage changes
localStorage-->>Dashboard: Trigger update event
Dashboard->>Dashboard: Update completedCount
Dashboard-->>User: Refresh progress indicator
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
Remove useEffect sync that triggered react-hooks/set-state-in-effect. Initialize task status from storage on mount and remount widget via key when demo page context changes. Made-with: Cursor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (5)
package.json (1)
23-23: Optional: expose the QA flow via an npm script.Adding
playwrightis fine, but the newscripts/qa-flow.mjshas no entry in"scripts", so contributors and CI need to know the magic invocation. Consider:♻️ Suggested addition
"scripts": { "dev": "next dev", "build": "next build", "start": "next start", - "lint": "eslint" + "lint": "eslint", + "qa:flow": "node scripts/qa-flow.mjs" },Also note: Playwright browsers aren't auto-installed by
npm install— you'll likely wantnpx playwright install chromiumdocumented in the README or wired into apostinstall/CI step.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 23, Add an npm script entry that exposes the QA flow so contributors/CI can run the new scripts/qa-flow.mjs easily: update package.json's "scripts" to include a descriptive script name (e.g., "qa:flow" or "test:qa") that invokes node with scripts/qa-flow.mjs, and also add guidance to ensure Playwright browsers are installed by wiring a postinstall script (e.g., running npx playwright install chromium) or documenting the command in README/CI; locate changes around the "scripts" object in package.json and the new scripts/qa-flow.mjs file to implement this.src/app/demo/expenses/page.tsx (1)
1-99: LGTM — mirrors the GitHub demo cleanly.Same structure as
demo/github/page.tsxwithpageKey="expenses"correctly threaded intoRunbookWidget. No concerns.Sidebar (not blocking): the github/expenses pages share ~80% of the same JSX scaffolding (header card, two-section grid of read-only labeled inputs, callout). If a third demo page lands, consider extracting a small
<DemoFormCard />to keep them in sync.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/demo/expenses/page.tsx` around lines 1 - 99, The page duplicates scaffolding found in demo/github/page.tsx; extract the shared JSX into a reusable component (e.g., DemoFormCard) and replace the inline card in DemoExpensesPage with <DemoFormCard ...props /> while preserving the existing RunbookWidget(pageKey="expenses") usage; implement DemoFormCard to accept props for title, subtitle, badgeText, and the read-only field values so both DemoExpensesPage (function DemoExpensesPage) and the GitHub demo page can import and render the same component to keep layouts in sync.QA_BUG_LOG.md (1)
1-5: Don't commit a generated artifact — gitignore it instead.
QA_BUG_LOG.mdis overwritten byscripts/qa-flow.mjson every run (line 90), so committing it guarantees noisy diffs and merge conflicts whenever anyone runs QA locally. Same concern applies to thescreenshots/directory written at line 6 of the script.Recommend adding both to
.gitignoreand removing this snapshot from the PR. If you want a record of the last clean run, attach it as a CI artifact or upload to a build report rather than versioning it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@QA_BUG_LOG.md` around lines 1 - 5, Remove the generated QA_BUG_LOG.md from the PR and stop committing generated artifacts: add "QA_BUG_LOG.md" and the screenshots/ directory pattern (e.g., "screenshots/" or the exact path used by scripts/qa-flow.mjs) to .gitignore, and update the QA workflow (scripts/qa-flow.mjs) to publish the log and screenshots as CI artifacts or store them in a build report instead of writing them into the repo; ensure any code that writes the files (the writer at line ~90 in scripts/qa-flow.mjs and the screenshots writer at line ~6) still produces outputs for CI but not tracked in git.scripts/qa-flow.mjs (1)
15-26: Use Playwright's auto-waiting assertions instead ofcount() === 0.
page.getByText(expectedText).count()returns immediately and only reflects the DOM at that instant. Even afterwaitUntil: "networkidle", React may still be hydrating the dashboard, leading to flakes. Preferexpect(...).toBeVisible()from@playwright/test(which retries with a timeout), or at minimumlocator.waitFor({ state: "visible", timeout: ... })followed by a try/catch to log the bug.♻️ Refactor
async function ensureDashboardProgress(page, expectedText) { - const progressPill = page.getByText(expectedText); - if ((await progressPill.count()) === 0) { + try { + await page.getByText(expectedText).waitFor({ state: "visible", timeout: 5000 }); + } catch { logBug( "high", "Dashboard progress mismatch", `Dashboard shows '${expectedText}'`, "Expected progress pill text was not found", "Open /dashboard after completing tasks and inspect progress text", ); - } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/qa-flow.mjs` around lines 15 - 26, The check in ensureDashboardProgress uses page.getByText(expectedText).count() which is instantaneous and flakes; replace it with Playwright's auto-waiting assertion or locator wait: obtain the locator via progressPill = page.getByText(expectedText) (as already named), then await expect(progressPill).toBeVisible() from `@playwright/test` (or await progressPill.waitFor({ state: "visible", timeout: X })) inside a try/catch; on failure call logBug("high", "Dashboard progress mismatch", ...) using the same message fields so the bug is logged when the locator never becomes visible. Ensure you import expect from '@playwright/test' if using toBeVisible.src/lib/taskStatusAdapter.ts (1)
87-94:storagelistener fires for every localStorage key change.
window.addEventListener("storage", handler)is invoked for any key written from another tab/window, not justSTORAGE_KEY. The callback then re-reads and dispatches updates needlessly. Filter onevent.key:🛠️ Suggested fix
- const handler = () => callback(getTaskStatuses()); + const handler = (event: StorageEvent) => { + if (event.key !== null && event.key !== STORAGE_KEY) return; + callback(getTaskStatuses()); + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/taskStatusAdapter.ts` around lines 87 - 94, The storage event handler currently calls callback(getTaskStatuses()) for every storage change; update the handler to accept a StorageEvent, check event.key and only invoke callback when event.key === STORAGE_KEY (or when the change semantics you expect require it), e.g., replace the generic handler with (event: StorageEvent) => { if (event.key === STORAGE_KEY) callback(getTaskStatuses()); } so the code only re-reads and dispatches when the relevant key changes; keep customHandler and STATUS_EVENT_NAME logic unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/qa-flow.mjs`:
- Around line 81-87: The Markdown table rows interpolate raw bug fields
(bug.title, bug.expected, bug.actual, bug.repro) which can contain pipes or
newlines and will break the table; add a small sanitizer function (e.g.,
escapeMarkdownCell) and call it on each field inside the loop that builds lines
(the for (const bug of bugs) block) to replace "|" with "\|" and newline
characters with a space so table cells remain intact.
- Around line 28-93: Wrap the main work in run() with try/finally to always call
await browser.close() (reference: run(), browser.close()), ensure the bug log
write (fs.writeFile to bugLogPath) runs in finally or after a caught error so
the log is always emitted, and propagate failures to the process by adding a
top-level run().catch(...) that writes the bug log if not already written and
calls process.exit(bugs.length ? 1 : 0) (use bugs and bugLogPath identifiers) so
the process exits non‑zero on failures or when bugs exist.
- Around line 78-91: The script currently always exits 0 after writing the
markdown bug report; update the end of the routine (after
fs.writeFile(bugLogPath, lines.join("\n"))) to surface failures to CI by
terminating with a non-zero exit code when bugs.length > 0 (e.g., call
process.exit(1) or throw an Error with a message including the bug count or
bugLogPath). Locate the block that builds the report using the bugs array and
bugLogPath and add the conditional exit so successful runs keep exit 0 and runs
with any bugs exit non-zero.
- Around line 51-67: After clicking the "Mark Complete" button (the
page.getByRole(..., { name: "Mark Complete" }).click() calls), wait for the
widget's post-completion signal before navigating: specifically wait for the
button text to change to "Task Completed" or for the saveSource DOM/text to
appear (the same observable used by RunbookWidget.tsx) so the async
updateTaskStatus/localStorage write has completed; apply this wait in both the
GitHub and expenses flows before calling page.goto(...) and before asserting
ensureDashboardProgress(page, "Progress: 2/2 tasks complete").
In `@src/app/dashboard/page.tsx`:
- Around line 46-51: getTaskStatuses() is being called during render via
useState(getTaskStatuses()) which reads localStorage and causes SSR/CSR
hydration mismatches; change the state initializer to an empty object (e.g.,
useState({})) and move the localStorage hydration into a client-only useEffect:
inside the existing useEffect (where subscribeToTaskStatus is called), call
getTaskStatuses() and call setTaskStatuses(...) with its result (while
preserving the existing subscribeToTaskStatus(setTaskStatuses) subscription and
return unsubscribe) so initial render matches SSR and task statuses are
populated on the client.
In `@src/components/demo/RunbookWidget.tsx`:
- Around line 27-33: handleMarkComplete can update state for a different task if
task.taskId/pageKey changes while updateTaskStatus is in-flight and can leave
isSaving true on thrown errors; fix by capturing the current taskId/pageKey into
a local const (e.g., const currentTaskId = task.taskId or const currentPageKey =
pageKey) before awaiting updateTaskStatus, wrap the await call in
try/catch/finally, set isSaving(false) in the finally block, and only call
setTaskStatus("complete") and setSaveSource(source) if the captured id/key still
matches the latest task.taskId/pageKey to avoid stale writes (also handle/log
errors in catch as needed).
- Around line 20-25: The current RunbookWidget reads localStorage during render
via getTaskStatus(task.taskId) and also calls setTaskStatus/setSaveSource
synchronously inside a useEffect causing hydration mismatch and the
react-hooks/set-state-in-effect CI failure; fix by moving the per-task state
(taskStatus and saveSource) into a new inner component (e.g., TaskBody) and
render it keyed by task.taskId so its useState initializer runs on remount
instead of using setState in effect, ensure the inner component initializes
state without reading localStorage on the server (use a safe empty/default value
for SSR) and perform any localStorage reads inside a client-only effect within
TaskBody to update state.
In `@src/components/demo/RunbookWidgetPanel.tsx`:
- Around line 16-41: Add accessible dialog semantics and an Escape key handler
to RunbookWidgetPanel: give the <aside> a role="dialog" and aria-modal="false"
and add aria-labelledby that points to the heading element for task.title (add
an id to the <h2>, e.g. runbook-title). Also implement a useEffect inside
RunbookWidgetPanel that registers a keydown listener on mount which calls the
existing onClose when Escape is pressed and cleans up the listener on unmount.
Ensure the aria-labelledby value matches the <h2> id so screen readers announce
the dialog heading.
In `@src/lib/taskStatusAdapter.ts`:
- Around line 61-77: The current logic silently treats any non-2xx response from
the POST to "/api/tasks/update" as a successful local save, which masks server
validation/auth failures; update the handling in the async block around the
fetch so that when response.ok is false you (a) log the response status and body
(e.g., using console.error or a logger) including context like the taskId and
attempted status, and (b) do not silently overwrite server rejection by
immediately calling setTaskStatus — instead return a distinct result (e.g.,
"server" or throw) so the caller can surface the server error; keep
setTaskStatus only for true response.ok (the existing branch) and keep the catch
for network failures to fall back to local demo persistence.
- Around line 22-26: The JSON.parse result for storedValue is being blindly cast
to TaskStatusMap; instead parse into a unknown, validate that it's a plain
object, and that each property value is a valid TaskStatus before returning
it—otherwise return {}. In practice: replace `return JSON.parse(storedValue) as
TaskStatusMap` with parsing to an unknown, check `typeof parsed === "object" &&
parsed !== null && !Array.isArray(parsed)`, iterate Object.entries(parsed) and
verify keys are strings and values match the allowed TaskStatus set (the same
values used elsewhere where code checks `taskStatuses[task.id] === "complete"`),
then build and return a typed TaskStatusMap; on any validation failure return an
empty object. Ensure you reference the types/symbols TaskStatusMap, storedValue,
taskStatuses, and TaskStatus while implementing the checks.
---
Nitpick comments:
In `@package.json`:
- Line 23: Add an npm script entry that exposes the QA flow so contributors/CI
can run the new scripts/qa-flow.mjs easily: update package.json's "scripts" to
include a descriptive script name (e.g., "qa:flow" or "test:qa") that invokes
node with scripts/qa-flow.mjs, and also add guidance to ensure Playwright
browsers are installed by wiring a postinstall script (e.g., running npx
playwright install chromium) or documenting the command in README/CI; locate
changes around the "scripts" object in package.json and the new
scripts/qa-flow.mjs file to implement this.
In `@QA_BUG_LOG.md`:
- Around line 1-5: Remove the generated QA_BUG_LOG.md from the PR and stop
committing generated artifacts: add "QA_BUG_LOG.md" and the screenshots/
directory pattern (e.g., "screenshots/" or the exact path used by
scripts/qa-flow.mjs) to .gitignore, and update the QA workflow
(scripts/qa-flow.mjs) to publish the log and screenshots as CI artifacts or
store them in a build report instead of writing them into the repo; ensure any
code that writes the files (the writer at line ~90 in scripts/qa-flow.mjs and
the screenshots writer at line ~6) still produces outputs for CI but not tracked
in git.
In `@scripts/qa-flow.mjs`:
- Around line 15-26: The check in ensureDashboardProgress uses
page.getByText(expectedText).count() which is instantaneous and flakes; replace
it with Playwright's auto-waiting assertion or locator wait: obtain the locator
via progressPill = page.getByText(expectedText) (as already named), then await
expect(progressPill).toBeVisible() from `@playwright/test` (or await
progressPill.waitFor({ state: "visible", timeout: X })) inside a try/catch; on
failure call logBug("high", "Dashboard progress mismatch", ...) using the same
message fields so the bug is logged when the locator never becomes visible.
Ensure you import expect from '@playwright/test' if using toBeVisible.
In `@src/app/demo/expenses/page.tsx`:
- Around line 1-99: The page duplicates scaffolding found in
demo/github/page.tsx; extract the shared JSX into a reusable component (e.g.,
DemoFormCard) and replace the inline card in DemoExpensesPage with <DemoFormCard
...props /> while preserving the existing RunbookWidget(pageKey="expenses")
usage; implement DemoFormCard to accept props for title, subtitle, badgeText,
and the read-only field values so both DemoExpensesPage (function
DemoExpensesPage) and the GitHub demo page can import and render the same
component to keep layouts in sync.
In `@src/lib/taskStatusAdapter.ts`:
- Around line 87-94: The storage event handler currently calls
callback(getTaskStatuses()) for every storage change; update the handler to
accept a StorageEvent, check event.key and only invoke callback when event.key
=== STORAGE_KEY (or when the change semantics you expect require it), e.g.,
replace the generic handler with (event: StorageEvent) => { if (event.key ===
STORAGE_KEY) callback(getTaskStatuses()); } so the code only re-reads and
dispatches when the relevant key changes; keep customHandler and
STATUS_EVENT_NAME logic unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9496fe1c-6d03-423d-bdde-d433b288699d
⛔ Files ignored due to path filters (8)
package-lock.jsonis excluded by!**/package-lock.jsonscreenshots/01-home.pngis excluded by!**/*.pngscreenshots/02-dashboard-before-completion.pngis excluded by!**/*.pngscreenshots/03-github-demo.pngis excluded by!**/*.pngscreenshots/04-github-widget-open.pngis excluded by!**/*.pngscreenshots/05-expenses-demo.pngis excluded by!**/*.pngscreenshots/06-expenses-widget-open.pngis excluded by!**/*.pngscreenshots/07-dashboard-after-completion.pngis excluded by!**/*.png
📒 Files selected for processing (14)
.env.example.gitignoreQA_BUG_LOG.mdnext.config.tspackage.jsonscripts/qa-flow.mjssrc/app/dashboard/page.tsxsrc/app/demo/expenses/page.tsxsrc/app/demo/github/page.tsxsrc/components/demo/RunbookWidget.tsxsrc/components/demo/RunbookWidgetButton.tsxsrc/components/demo/RunbookWidgetPanel.tsxsrc/components/demo/demoTaskMap.tssrc/lib/taskStatusAdapter.ts
- Ignore and untrack QA_BUG_LOG.md and screenshots; document qa:flow in README - Harden qa-flow.mjs: try/finally browser teardown, waits, markdown escape, exit codes - Add npm script qa:flow; CI runs Playwright flow against next start and uploads artifacts - Dashboard: useSyncExternalStore for task map (SSR-safe types) - RunbookWidget: sync store, stale-save guard, rejected API handling - RunbookWidgetPanel: dialog semantics and Escape to close - taskStatusAdapter: validate storage JSON, filter storage events, log API rejections - Extract DemoFormShell for GitHub and expenses demo pages Made-with: Cursor
- Replace networkidle navigation with load (networkidle flakes when connections stay open) - Assert progress via data-testid + waitForFunction until text matches after hydration - Clear runbook localStorage before flow so 0/2 baseline is deterministic in CI Made-with: Cursor
getTaskStatuses() returned a new object every call when storage was empty, which breaks useSyncExternalStore snapshot equality and can prevent the dashboard from settling (CI qa-flow saw persistent progress mismatches). Cache snapshots by serialized storage value, use a frozen empty map, clone before writes, and align dashboard getServerSnapshot with the same empty map. Made-with: Cursor
This was referenced Apr 25, 2026
Merged
Merged
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.
Add GitHub and expenses demo surfaces with a floating Runbook assistant widget and task completion that syncs to the dashboard via localStorage with optional POST /api/tasks/update.
Include Playwright QA script, screenshot artifacts from the last clean run, and .env.example plus gitignore so .env.example can be committed while .env.local stays private. Set turbopack.root to silence multi-lockfile warnings during local builds.
Made-with: Cursor
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores