You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Add scripts/visual-diff.ts with local capture/diff commands and a full run pipeline that compares the PR base commit against the head commit.
Add playwright/visual-diff.config.ts for the console routes included in screenshots.
Add visual:capture:before, visual:capture:after, visual:diff, and visual:run package scripts.
Add a Visual diff GitHub Action that runs when a PR has the visual-change label or <!-- visual-diff:required --> in its description, uploads the HTML report as an artifact, and updates a sticky PR comment on every push.
Document the workflow in AGENTS.md.
Motivation (AI generated)
UI PRs are hard to review from code alone. This gives authors and reviewers a repeatable before/after screenshot diff locally and in CI, with the summary meant to be pasted into the PR description.
Business Impact (AI generated)
Faster, clearer UI reviews should reduce visual regressions in the Capgo console and make design-related PRs easier to approve with confidence.
Test Plan (AI generated)
bun scripts/visual-diff.ts capture validates CLI argument handling
Run bun run visual:capture:before, make a small UI tweak, then bun run visual:capture:after and bun run visual:diff
Open .context/visual-diff/report/index.html and confirm before/after/diff images render
Open a test PR with the visual-change label and confirm the workflow posts/updates the sticky comment with summary.md
Confirm pushing a new commit refreshes the same PR comment instead of creating duplicates
Introduce a Playwright-based before/after screenshot workflow with CI reporting so PRs that change console UI can attach and refresh visual evidence automatically.
Co-authored-by: Cursor <cursoragent@cursor.com>
Adds a visual diff system: a shared supabase-worktree-status.ts module (extracted from serve-backend-playwright.ts), a Playwright route config, a 700-line scripts/visual-diff.ts CLI that orchestrates local stacks, captures screenshots, computes pixel diffs, and writes an HTML report, plus a GitHub Actions workflow that gates on a PR label or body marker and posts results as a sticky comment.
Changes
Visual diff screenshot pipeline
Layer / File(s)
Summary
Shared Supabase status module scripts/supabase-worktree-status.ts, scripts/serve-backend-playwright.ts
Extracts SupabaseStatus, parseSupabaseStatus, and getSupabaseStatus into a new shared module; serve-backend-playwright.ts drops its duplicate implementations and imports from the new module instead.
Visual diff route config and project wiring playwright/visual-diff.config.ts, package.json, .gitignore
Defines VisualDiffRoute, visualDiffRoutes (six console pages), and the fixed 1280×720 visualDiffViewport. Adds pixelmatch/pngjs devDependencies, four visual:* npm scripts, and a .gitignore entry for .context/visual-diff.
Script: CLI, types, and service readiness scripts/visual-diff.ts (lines 1–304)
Loads config with cache-busting, parses and validates CLI arguments for capture/diff/run commands, implements HTTP/TCP/file-existence polling with timeouts, and adds startProcess/stopProcess lifecycle helpers and git command utilities.
Script: backend and frontend stack orchestration scripts/visual-diff.ts (lines 305–390)
Implements ensureBackendStack (Stripe emulator + Supabase startup, backend.ready wait, URL/key resolution) and ensureFrontendStack (Vite preview build + start with env wiring and HTTP readiness check).
Script: capture, compare, and report generation scripts/visual-diff.ts (lines 391–642)
Captures per-route PNGs after Supabase auth and UI login; computes pixel diffs with pixelmatch (size mismatches treated as 100% diff); writes summary.json, summary.md, and a standalone index.html with before/after/diff image tiles.
Script: git-driven run pipeline and main scripts/visual-diff.ts (lines 643–717)
Implements run: resolves base/head SHAs, checks out base for before capture and head for after capture, generates the report, sets exit code on detected changes, and restores the original ref in a finally block. main() dispatches commands and handles cleanup.
GitHub Actions CI workflow .github/workflows/visual-diff.yml
Adds a workflow with concurrency control; a check job gates on visual-change label or <!-- visual-diff:required --> body marker; the visual-diff job installs runtime/dependencies, runs the script with base/head SHAs, uploads the report artifact, and posts or updates a sticky PR comment with pass/fail status and download link.
AGENTS.md documentation AGENTS.md
Adds a "Visual diff for UI changes" section describing PR signaling, CI behavior, local workflow commands, route config location, and expected output paths.
Sequence Diagram(s)
sequenceDiagram
rect rgba(173, 216, 230, 0.5)
Note over PR,check: Gating
PR->>check: opened/labeled/edited
check-->>visual-diff job: enabled (label or body marker matched)
end
rect rgba(144, 238, 144, 0.5)
Note over visual-diff job,Vite: Stack setup
visual-diff job->>supabase-worktree: start backend (wait backend.ready)
supabase-worktree-->>visual-diff job: API_URL + ANON_KEY
visual-diff job->>Vite: build + start preview server
Vite-->>visual-diff job: HTTP 200 on /login/
end
rect rgba(255, 218, 185, 0.5)
Note over visual-diff job,screenshots: Capture
visual-diff job->>Playwright: checkout base ref → capture before PNGs
visual-diff job->>Playwright: checkout head ref → capture after PNGs
Playwright-->>visual-diff job: per-route PNG files
end
rect rgba(216, 191, 216, 0.5)
Note over visual-diff job,GitHub: Report & comment
visual-diff job->>pixelmatch: compare before/after PNGs
pixelmatch-->>visual-diff job: diff images + summary.json/md/html
visual-diff job->>GitHub Artifacts: upload report directory
GitHub Artifacts-->>PR Comment: artifact download URL
visual-diff job->>PR Comment: post/update sticky comment (pass/fail + summary)
end
Loading
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)
Check name
Status
Explanation
Title check
✅ Passed
The title clearly summarizes the main purpose of the PR: adding visual diff tooling for UI PR reviews, which aligns with the comprehensive feature set introduced.
Description check
✅ Passed
The PR description includes Summary, Motivation, Business Impact, and partial Test Plan sections, but is missing explicit Testing steps per the template and lacks Screenshots section.
Linked Issues check
✅ Passed
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
✅ Passed
Check skipped because no linked issues were found for this pull request.
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches📝 Generate docstrings
Create stacked PR
Commit on current branch
Comment @coderabbitai help to get the list of available commands.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving: Cursor Bugbot reported 3 unresolved findings (including a high-severity stale-routes issue) and its check completed as skipped. Human review is required before merge.
Sent by Cursor Approval Agent: Pull Request Approver
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving: Cursor Bugbot reported 3 unresolved findings (including one high severity), and the Bugbot check finished as skipped. Requested review from WcaleNieWolny and Dalanir.
Sent by Cursor Approval Agent: Pull Request Approver External
Reload route config after git checkout, honor --routes in diff,
restore the original branch on failure, dedupe Supabase status helpers,
and remove an unused import.
Co-authored-by: Cursor <cursoragent@cursor.com>
The reason will be displayed to describe this comment to others. Learn more.
Backend reused after git checkout
Medium Severity
The run pipeline checks out the head commit and rebuilds the frontend, but ensureBackendStack keeps the Supabase/functions stack from the base capture when /functions/v1/ok still responds. After capture, DB reset and edge functions can stay on the base commit while head screenshots run, skewing authenticated routes.
The reason will be displayed to describe this comment to others. Learn more.
HTTP readiness accepts 4xx responses
Medium Severity
isHttpReady treats any HTTP status below 500 as healthy. ensureBackendStack uses it to skip starting backend:playwright, so a 404/401 on /functions/v1/ok can be misread as a running stack and leave captures without a working functions backend.
The reason will be displayed to describe this comment to others. Learn more.
Empty diff reported as unchanged
Medium Severity
When every route is skipped because before/after PNGs are missing, compareScreenshots returns an empty list and generateReport still writes that no visual differences were detected. CI can mark the visual diff as passed and post that summary even though no routes were compared.
The reason will be displayed to describe this comment to others. Learn more.
Restore checkout skips dependency sync
Low Severity
After run finishes, the pipeline force-checkouts the original git ref but does not run bun install, unlike checkoutRef for base/head. node_modules can remain aligned with the head commit while the working tree is restored to the branch the developer started on.
The reason will be displayed to describe this comment to others. Learn more.
Risk: medium. Not approving: Cursor Bugbot found 4 unresolved findings on the latest commit and its check completed as skipped. Human review from WcaleNieWolny and Dalanir is still required before merge.
Sent by Cursor Approval Agent: Pull Request Approver
The reason will be displayed to describe this comment to others. Learn more.
Risk: medium. Not approving: PR risk exceeds the low threshold, Cursor Bugbot reported 4 unresolved findings on the latest commit, and the Bugbot check completed as skipped. WcaleNieWolny and Dalanir are already assigned for human review.
Sent by Cursor Approval Agent: Pull Request Approver External
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/visual-diff.yml:
- Around line 9-15: The pull_request.types section in the workflow file has
invalid YAML syntax where lines after the first item (synchronize, reopened,
labeled, unlabeled, edited) are missing the list item indicator. Add the dash
and space prefix (- ) to the beginning of each bare scalar line in the
pull_request.types list to properly format them as YAML list items, ensuring
each trigger type follows the same format as the opened item.
- Line 34: The grep pattern on line 34 uses a literal string match that only
handles spaced JSON format from toJson(). To support both compact and spaced
JSON formats, modify the pattern to make the whitespace around the colon
optional using a regex pattern that matches either "name": "visual-change" or
"name":"visual-change". Update the grep command to use the -E flag for extended
regex and adjust the pattern accordingly to handle both spacing variations.
In `@scripts/visual-diff.ts`:
- Around line 106-113: Add bounds checking before accessing rest[++index] for
the --threshold and --routes option flags to ensure the index is within bounds
before attempting to access or process the value. For --threshold, validate that
the retrieved value exists and is a valid number before converting it with
Number(), and for --routes, verify the value exists before calling .split(',').
Include error handling that logs a clear error message and exits the process if
required argument values are missing or invalid, preventing silent failures or
unhelpful TypeErrors.
- Around line 476-492: The writeDiffImage function's size-mismatch branch writes
the diff PNG file to diffPath without first ensuring the parent directory
exists, causing an ENOENT error. Before the writeFileSync call that writes the
diff PNG at line 485, add a mkdirSync call to create the directory for diffPath
(using dirname(diffPath) and the same directory creation options used later in
the function), matching the pattern already present in the normal comparison
path.
- Around line 161-169: The fetch call in the isHttpReady function lacks a
timeout mechanism, which can cause indefinite blocking if a server accepts the
connection but then stalls, preventing the outer loop from properly enforcing
its deadline. Add a timeout to the fetch call by passing an AbortSignal using
the AbortSignal.timeout method as the signal option in the fetch request. This
will ensure the fetch request automatically aborts after a reasonable timeout
duration rather than blocking indefinitely.
- Around line 454-463: The login(page) function is being called for every route
in the loop, but after the first successful login the browser session persists,
causing subsequent login attempts to redirect to the dashboard instead of the
login form. To fix this, move the login(page) call outside the routes loop to
execute only once before the loop begins, or add a conditional check inside the
loop to verify the login form exists before attempting to fill it. Either
approach will prevent redundant login attempts and ensure the session is reused
for all authenticated routes.
- Around line 669-675: The exit code logic in the conditional block starting
with `if (summary.changedCount > 0)` is incorrect. When routes have changed
(changedCount is greater than 0), the code currently sets process.exitCode to 0,
which signals success to the CI workflow. This should be reversed: set
process.exitCode to a non-zero value (such as 1) when summary.changedCount > 0
to signal that changes were detected, and allow the else branch to implicitly
result in exit code 0 when no visual differences are detected. This ensures the
CI workflow can properly gate the PR based on whether visual changes occurred.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c3b4cef0-7d7a-4327-b267-0551fcea3f65
📥 Commits
Reviewing files that changed from the base of the PR and between d79d1e2 and bdaa07d.
⛔ Files ignored due to path filters (1)
bun.lock is excluded by !**/*.lock
📒 Files selected for processing (8)
.github/workflows/visual-diff.yml
.gitignore
AGENTS.md
package.json
playwright/visual-diff.config.ts
scripts/serve-backend-playwright.ts
scripts/supabase-worktree-status.ts
scripts/visual-diff.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
types:
- opened
synchronize
reopened
labeled
unlabeled
edited
types:
- opened
- synchronize
- reopened
- labeled
- unlabeled
- edited
🧰 Tools🪛 actionlint (1.7.12)
[error] 11-11: could not parse as YAML: could not find expected ':'
(syntax-check)
🪛 YAMLlint (1.37.1)
[error] 12-12: syntax error: could not find expected ':'
(syntax)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/visual-diff.yml around lines 9 - 15, The
pull_request.types section in the workflow file has invalid YAML syntax where
lines after the first item (synchronize, reopened, labeled, unlabeled, edited)
are missing the list item indicator. Add the dash and space prefix (- ) to the
beginning of each bare scalar line in the pull_request.types list to properly
format them as YAML list items, ensuring each trigger type follows the same
format as the opened item.
#!/bin/bash# Check if the file exists and inspect the relevant sectionif [ -f .github/workflows/visual-diff.yml ];thenecho"File found. Showing lines around line 34:"
cat -n .github/workflows/visual-diff.yml | sed -n '25,45p'elseecho"File not found at expected location"
find . -name "visual-diff.yml"2>/dev/null
fi
Repository: Cap-go/capgo
Length of output: 1068
Fix label detection to handle both compact and spaced JSON formats from toJson().
The current pattern on line 34 only matches spaced JSON ("name": "visual-change"). When GitHub Actions produces compact JSON ("name":"visual-change"), the check fails and the visual-diff job is skipped even though the label exists.
Suggested fix
- if printf '%s' "$PR_LABELS" | grep -q '"name": "visual-change"'; then+ if printf '%s' "$PR_LABELS" | grep -Eq '"name"[[:space:]]*:[[:space:]]*"visual-change"'; then
requested=true
📝 Committable suggestion
‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
if printf '%s' "$PR_LABELS" | grep -q '"name": "visual-change"'; then
if printf '%s' "$PR_LABELS" | grep -Eq '"name"[[:space:]]*:[[:space:]]*"visual-change"'; then
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/visual-diff.yml at line 34, The grep pattern on line 34
uses a literal string match that only handles spaced JSON format from toJson().
To support both compact and spaced JSON formats, modify the pattern to make the
whitespace around the colon optional using a regex pattern that matches either
"name": "visual-change" or "name":"visual-change". Update the grep command to
use the -E flag for extended regex and adjust the pattern accordingly to handle
both spacing variations.
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Guard against missing argument values for option flags.
--threshold/--routes (and --base/--head/--phase) read rest[++index] without a bounds check. A trailing --routes makes rest[++index]undefined, so .split(',') (Line 111) throws an unhelpful TypeError. Likewise --threshold with a non-numeric or missing value yields NaN, which silently makes diffPercent > thresholdPercent always false (no route ever flagged changed).
🛠️ Proposed fix
if (arg === '--threshold') {
- thresholdPercent = Number(rest[++index])+ const value = Number(rest[++index])+ if (!Number.isFinite(value))+ throw new Error('--threshold requires a numeric value')+ thresholdPercent = value
continue
}
if (arg === '--routes') {
- routes = rest[++index].split(',').map(route => route.trim()).filter(Boolean)+ const value = rest[++index]+ if (value === undefined)+ throw new Error('--routes requires a comma-separated value')+ routes = value.split(',').map(route => route.trim()).filter(Boolean)
continue
}
📝 Committable suggestion
‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import type { ChildProcessWithoutNullStreams } from 'node:child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/visual-diff.ts` around lines 106 - 113, Add bounds checking before
accessing rest[++index] for the --threshold and --routes option flags to ensure
the index is within bounds before attempting to access or process the value. For
--threshold, validate that the retrieved value exists and is a valid number
before converting it with Number(), and for --routes, verify the value exists
before calling .split(','). Include error handling that logs a clear error
message and exits the process if required argument values are missing or
invalid, preventing silent failures or unhelpful TypeErrors.
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash# First, let's examine the visual-diff.ts file around lines 161-169
cat -n scripts/visual-diff.ts | sed -n '150,180p'
Repository: Cap-go/capgo
Length of output: 1046
🏁 Script executed:
#!/bin/bash# Let's check the full context of waitForHttp and isHttpReady
ast-grep outline scripts/visual-diff.ts
Repository: Cap-go/capgo
Length of output: 3213
🏁 Script executed:
#!/bin/bash# Check the project's Node.js version requirement in package.json
cat package.json | grep -A 5 '"engines"'
Repository: Cap-go/capgo
Length of output: 150
🏁 Script executed:
#!/bin/bash# Also check .nvmrc or other version hintsif [ -f .nvmrc ];then cat .nvmrc;fiif [ -f .node-version ];then cat .node-version;fi
Repository: Cap-go/capgo
Length of output: 150
🏁 Script executed:
#!/bin/bash# Check the full package.json for engines field
cat package.json
Repository: Cap-go/capgo
Length of output: 25946
🏁 Script executed:
#!/bin/bash# Look at the waitForHttp function context
cat -n scripts/visual-diff.ts | sed -n '190,210p'
Repository: Cap-go/capgo
Length of output: 959
🏁 Script executed:
#!/bin/bash# Check if there are any other uses of AbortSignal in the codebase
rg "AbortSignal" --type ts
Repository: Cap-go/capgo
Length of output: 150
🏁 Script executed:
#!/bin/bash# Check Node version in package.json more carefully
head -50 package.json
Repository: Cap-go/capgo
Length of output: 3393
🏁 Script executed:
#!/bin/bash# Check for bun version specificationif [ -f bun.lockb ];thenecho"Found bun.lockb (Bun lockfile exists)"fi# Look for any .bunrc, bunrc.toml or similar
ls -la | grep -i bun ||echo"No bun config files in root"# Check if there's a version constraint in package.json for bun
cat package.json | grep -A 5 -B 5 '"bun"'
Repository: Cap-go/capgo
Length of output: 279
🏁 Script executed:
#!/bin/bash# Check the tsconfig to understand the runtime target
cat tsconfig.json 2>/dev/null | head -30 ||echo"No tsconfig.json in root"
Repository: Cap-go/capgo
Length of output: 816
🌐 Web query:
AbortSignal.timeout Bun support version
💡 Result:
Bun has supported AbortSignal.timeout since December 2022 [1]. It was added to the Bun codebase in commit fe4f39f, which dates back to early December 2022 [1]. Since its introduction, the implementation has been refined for performance, including significant optimizations such as a 6x speed improvement identified in late 2024 [2] and a 40x speed increase implemented in Bun v1.2.20 (released in August 2025) [3][4]. If you are using TypeScript and encounter issues with AbortSignal.timeout, it is recommended to ensure your project's lib configuration includes DOM types (e.g., lib: ["ESNext", "DOM"]) [5], as there have been historical bugs where the static method was missing from Bun's TypeScript type definitions [5].
Node.js AbortSignal.timeout minimum version requirement
💡 Result:
The AbortSignal.timeout static method was added to Node.js in version 17.3.0 and backported to version 16.14.0 [1][2][3]. Consequently, to use this feature, you must be using Node.js version 17.3.0 or higher, or 16.14.0 or higher within the 16.x release line [1][3].
Add timeout to fetch call to prevent indefinite blocking.
The fetch in isHttpReady (line 163) lacks a timeout. If a server accepts the connection but then stalls, await fetch(url) blocks indefinitely, causing waitForHttp to miss its deadline enforcement—the outer loop never gets a chance to check the elapsed time. Add a per-request timeout using AbortSignal.timeout:
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import type { ChildProcessWithoutNullStreams } from 'node:child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/visual-diff.ts` around lines 161 - 169, The fetch call in the
isHttpReady function lacks a timeout mechanism, which can cause indefinite
blocking if a server accepts the connection but then stalls, preventing the
outer loop from properly enforcing its deadline. Add a timeout to the fetch call
by passing an AbortSignal using the AbortSignal.timeout method as the signal
option in the fetch request. This will ensure the fetch request automatically
aborts after a reasonable timeout duration rather than blocking indefinitely.
#!/bin/bash# Look for login.vue and check for beforeEnter or redirect logic
find . -name "login.vue" -o -name "*auth*router*"
Repository: Cap-go/capgo
Length of output: 172
🏁 Script executed:
#!/bin/bash# Examine login.vue for any redirect guards or auth checks
rg -n "beforeEnter|redirect|isLoggedIn|requiresAuth" src/pages/login.vue src/composables
#!/bin/bash# Check login.vue for auth redirect logic - read the full file
wc -l src/pages/login.vue
Repository: Cap-go/capgo
Length of output: 175
🏁 Script executed:
#!/bin/bash# Look at login.vue content, especially the script section for redirect logic
head -100 src/pages/login.vue
Repository: Cap-go/capgo
Length of output: 5423
🏁 Script executed:
#!/bin/bash# Check for auth redirect logic in login.vue - look at onMounted and other init logic
sed -n '1,200p' src/pages/login.vue | tail -100
Repository: Cap-go/capgo
Length of output: 3328
🏁 Script executed:
#!/bin/bash# Search for autoAuth function which might handle redirects
rg -n "autoAuth|isLoggedIn|getUser" src/pages/login.vue
Repository: Cap-go/capgo
Length of output: 287
🏁 Script executed:
#!/bin/bash# Check main.ts or app.vue for global route guards
cat src/main.ts
Repository: Cap-go/capgo
Length of output: 8560
🏁 Script executed:
#!/bin/bash# Find the autoAuth function to understand if it redirects authenticated users
rg -n "export.*autoAuth|function autoAuth|const autoAuth" src/services/supabase.ts -A 20
Repository: Cap-go/capgo
Length of output: 862
🏁 Script executed:
#!/bin/bash# Check login.vue onMounted to see the full flow and what autoAuth does
sed -n '500,600p' src/pages/login.vue
Repository: Cap-go/capgo
Length of output: 2763
🏁 Script executed:
#!/bin/bash# Look at the entire onMounted hook
rg -n "onMounted" src/pages/login.vue -A 50
Repository: Cap-go/capgo
Length of output: 5911
🏁 Script executed:
#!/bin/bash# Find checkAuthUser to see if it redirects authenticated users
rg -n "checkAuthUser" src/pages/login.vue -A 15
Repository: Cap-go/capgo
Length of output: 1304
🏁 Script executed:
#!/bin/bash# Check if there are any other redirect guards or navigation in login.vue
rg -n "router.push|router.replace|nextLogin" src/pages/login.vue -B 2 -A 2
Repository: Cap-go/capgo
Length of output: 1089
login() is re-invoked for every auth-protected route, causing failures after the first login.
For each route with auth === true, login(page) navigates to /login/ and fills the form. The login page checks for existing sessions via supabase.auth.getClaims() and redirects authenticated users to /dashboard/ (see checkLogin() in src/pages/login.vue lines 558-571). After the first successful login, the browser context retains the session, so subsequent calls to login(page) will be redirected away from the login form, causing page.fill('[data-test="email"]') to time out.
Consider logging in once before the loop or skipping login for subsequent routes (e.g., by checking if a login form is present before attempting to fill it).
🧰 Tools🪛 ast-grep (0.44.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import type { ChildProcessWithoutNullStreams } from 'node:child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/visual-diff.ts` around lines 454 - 463, The login(page) function is
being called for every route in the loop, but after the first successful login
the browser session persists, causing subsequent login attempts to redirect to
the dashboard instead of the login form. To fix this, move the login(page) call
outside the routes loop to execute only once before the loop begins, or add a
conditional check inside the loop to verify the login form exists before
attempting to fill it. Either approach will prevent redundant login attempts and
ensure the session is reused for all authenticated routes.
Size-mismatch branch writes the diff PNG without creating its directory.
In the normal path, mkdirSync(dirname(diffPath), …) runs (Line 501) before writeFileSync. The size-mismatch branch writes to diffPath at Line 485 without that guard. compareScreenshots never creates diffDir() upfront, so the first route with mismatched dimensions throws ENOENT and aborts the whole comparison.
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import type { ChildProcessWithoutNullStreams } from 'node:child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/visual-diff.ts` around lines 476 - 492, The writeDiffImage function's
size-mismatch branch writes the diff PNG file to diffPath without first ensuring
the parent directory exists, causing an ENOENT error. Before the writeFileSync
call that writes the diff PNG at line 485, add a mkdirSync call to create the
directory for diffPath (using dirname(diffPath) and the same directory creation
options used later in the function), matching the pattern already present in the
normal comparison path.
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash# First, let's examine the visual-diff.ts file around lines 669-675
sed -n '665,680p' scripts/visual-diff.ts
Repository: Cap-go/capgo
Length of output: 753
🏁 Script executed:
#!/bin/bash# Check if the workflow file exists and examine how it uses the exit code
find . -name "visual-diff.yml" -o -name "*visual-diff*" -type f | head -20
Repository: Cap-go/capgo
Length of output: 246
🏁 Script executed:
#!/bin/bash# Read the workflow file to understand how it consumes the exit code
cat -n .github/workflows/visual-diff.yml
Repository: Cap-go/capgo
Length of output: 5788
🏁 Script executed:
#!/bin/bash# Look for context around the flagged lines and the start of the main function
sed -n '1,100p' scripts/visual-diff.ts | head -50
Repository: Cap-go/capgo
Length of output: 1655
🏁 Script executed:
#!/bin/bash# Search for any comments or documentation about exit codes or the script's purpose
rg -i "exit|exit.?code|gate|fail|changed" scripts/visual-diff.ts -B 2 -A 2 | head -60
Repository: Cap-go/capgo
Length of output: 1638
🏁 Script executed:
#!/bin/bash# Look at the summary generation and the entire block that handles exit codes
sed -n '650,700p' scripts/visual-diff.ts
Repository: Cap-go/capgo
Length of output: 1756
🏁 Script executed:
#!/bin/bash# Search for any PR or changelog context about this script
find . -name "CHANGELOG*" -o -name "*.md"| xargs rg -l "visual-diff"2>/dev/null | head -5
Repository: Cap-go/capgo
Length of output: 162
🏁 Script executed:
#!/bin/bash# Check git log or recent commits related to visual-diff to understand intent
git log --oneline -20 -- scripts/visual-diff.ts
Repository: Cap-go/capgo
Length of output: 212
🏁 Script executed:
#!/bin/bash# Look for any TODO or FIXME comments in the visual-diff.ts file
rg "TODO|FIXME|BUG|XXX" scripts/visual-diff.ts
Repository: Cap-go/capgo
Length of output: 150
🏁 Script executed:
#!/bin/bash# Check if there's a summary.json file that might be the actual gate mechanism
rg "summary\." scripts/visual-diff.ts -B 2 -A 2 | head -40
Repository: Cap-go/capgo
Length of output: 774
🏁 Script executed:
#!/bin/bash# Check the full commit message for the recent visual-diff fix
git show bdaa07de --format=fuller | head -60
Repository: Cap-go/capgo
Length of output: 3769
🏁 Script executed:
#!/bin/bash# Look at what changed in that commit
git show bdaa07de -- scripts/visual-diff.ts | head -100
Repository: Cap-go/capgo
Length of output: 3807
🏁 Script executed:
#!/bin/bash# See the parent commit to understand what the original behavior was
git show bdaa07de^:scripts/visual-diff.ts 2>/dev/null | sed -n '665,680p'||echo"Parent commit doesn't have this file"
Repository: Cap-go/capgo
Length of output: 150
🏁 Script executed:
#!/bin/bash# Check if there's a PR or issue reference in the recent commits
git log --all --oneline --grep="visual-diff"| head -10
Repository: Cap-go/capgo
Length of output: 212
🏁 Script executed:
#!/bin/bash# Look at the workflow again - specifically line 75-79 to see if the job fails based on exit code
cat -n .github/workflows/visual-diff.yml | sed -n '75,95p'
Repository: Cap-go/capgo
Length of output: 1050
Exit code should be non-zero when routes change, not zero.
When summary.changedCount > 0, the code logs that routes changed but sets process.exitCode = 0 (the default). This makes the job succeed and prevents the CI workflow from gating the PR. The workflow's comment step checks job.status (which depends on the script's exit code) to determine whether to report "passed" or "failed". As written, changed and unchanged runs are indistinguishable by exit status, contradicting the intent to signal failures when visual changes are detected.
‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
console.log('[visual-diff] no visual differences detected')
}
🧰 Tools🪛 ast-grep (0.44.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import type { ChildProcessWithoutNullStreams } from 'node:child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/visual-diff.ts` around lines 669 - 675, The exit code logic in the
conditional block starting with `if (summary.changedCount > 0)` is incorrect.
When routes have changed (changedCount is greater than 0), the code currently
sets process.exitCode to 0, which signals success to the CI workflow. This
should be reversed: set process.exitCode to a non-zero value (such as 1) when
summary.changedCount > 0 to signal that changes were detected, and allow the
else branch to implicitly result in exit code 0 when no visual differences are
detected. This ensures the CI workflow can properly gate the PR based on whether
visual changes occurred.
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
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.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary (AI generated)
scripts/visual-diff.tswith local capture/diff commands and a fullrunpipeline that compares the PR base commit against the head commit.playwright/visual-diff.config.tsfor the console routes included in screenshots.visual:capture:before,visual:capture:after,visual:diff, andvisual:runpackage scripts.Visual diffGitHub Action that runs when a PR has thevisual-changelabel or<!-- visual-diff:required -->in its description, uploads the HTML report as an artifact, and updates a sticky PR comment on every push.AGENTS.md.Motivation (AI generated)
UI PRs are hard to review from code alone. This gives authors and reviewers a repeatable before/after screenshot diff locally and in CI, with the summary meant to be pasted into the PR description.
Business Impact (AI generated)
Faster, clearer UI reviews should reduce visual regressions in the Capgo console and make design-related PRs easier to approve with confidence.
Test Plan (AI generated)
bun scripts/visual-diff.ts capturevalidates CLI argument handlingbun run visual:capture:before, make a small UI tweak, thenbun run visual:capture:afterandbun run visual:diff.context/visual-diff/report/index.htmland confirm before/after/diff images rendervisual-changelabel and confirm the workflow posts/updates the sticky comment withsummary.mdGenerated with AI
Made with Cursor
Summary by CodeRabbit
Release Notes
New Features
visual-changeor marked with<!-- visual-diff:required -->Documentation
Chores