feat(e2e): Chapter 2 explore harness — make explore + /explore skill (#399) - #444

Merged
AndresL230 merged 9 commits into
mainfrom
e2e-chapter2-explore
Jul 28, 2026
Merged

feat(e2e): Chapter 2 explore harness — make explore + /explore skill (#399)#444
AndresL230 merged 9 commits into
mainfrom
e2e-chapter2-explore

Conversation

@AndresL230

@AndresL230AndresL230 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Chapter 2 of the E2E program (epic #403), PR 2 of 3. Closes#399. Builds on the #400 oracle module (PR #443).

make explore boots the deterministic local stack and hands a Claude Code explorer a real browser; the oracles judge what it saw. Local-only by design — never a CI gate.

What's here

  • scripts/explore.sh (+ make explore / make explore-down, .explore/ gitignored) — the full pipeline: machine-singleton stack lock (detached holder process writing self-identifying lock.ok only after flock succeeds — a failed acquisition touches nothing, proven by an A/B two-session simulation) → e2e-up in function mode with a dummy GEMINI_API_KEY (RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439) → storage state minted via POST /api/auth/test-login through the frontend origin (cookie + sapling_user localStorage, secure flag matching the backend's actual local policy) → claude -p with a pinned @playwright/mcp@0.0.78 config and turn budget, transcript captured via stream-json + jq into a grep-able session.log → oracle final pass appended to findings.md → teardown. up/down subcommands support the interactive flow.
  • scripts/explore/explorer-prompt.md — the mission briefing: student persona (Rich Active), break-things mandate, report-never-fix, oracle cadence (cd backend && venv/bin/python -m e2e_oracles), findings format, known-bugs list, and a "stub it before you dig" rule earned in live testing.
  • .claude/skills/explore/SKILL.md — the interactive /explore skill: same pipeline, watchable and steerable, with a same-origin JS sign-in for live-browser sessions.

Hard-won fixes from live acceptance runs

  • @playwright/mcp without --isolated reuses a persistent profile and silently ignores --storage-state — the run-2 "signed-in" state was a stale-cookie artifact; --isolated fixes it (verified down to the bundled playwright-core source and a cookie-DB probe).
  • claude -p's default text output prints only the final message — a turn-budget exit produced an empty transcript despite real browsing; --output-format stream-json --verbose + jq restores the readable transcript.
  • Lock-holder bookkeeping survives concurrent sessions (busy attempt leaves the running session's files byte-identical).

Acceptance (issue #399 bar, definitive run in the SDD report)

One bounded run (EXPLORE_MAX_TURNS=25, 3m52s): real signed-in navigation across dashboard + library ("Rich Active" nav, 4 seeded documents, CS101/MATH210/BIO110/ENG150 visible in snapshots), 2 explorer-written findings — including a novel wrong-data upload bug and a tutor-resume 500 (concept_describe unregistered in the function-handler seam) independently confirmed by the oracle logscan — plus the harness-appended Oracle final pass re-confirming #355. Machine verified clean afterward (ports, containers, lock).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added an end-to-end exploratory testing workflow for the local application.
    • Added commands to start, stop, and run complete exploration sessions.
    • Exploration sessions now support browser-driven testing, automated checks, session logs, and structured findings.
  • Documentation

    • Added guidance covering exploration steps, reporting requirements, known issues, and session summaries.
  • Chores

    • Exploration artifacts are now excluded from version control.

AndresL230and others added 8 commits July 28, 2026 01:13
…itignore (#399)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e, oracle cadence (#399)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…er GEMINI_API_KEY (#399)
A busy lock previously still tripped do_up's trap/wipe path, tearing down
another session's live stack via scripts/e2e-down.sh and deleting its
.explore/ artifacts before the lock check ever ran. start_lock_holder now
runs first, the do_down safety trap arms only after the lock is held, the
.explore/ wipe (still selective, preserving lock.pid/lock.ok) happens after
that, and a failed acquisition cleans up its own holder/pid files before
exiting. GEMINI_API_KEY now defaults only when unset instead of always
overwriting an operator's real key.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ns (#399)
start_lock_holder previously deleted lock.ok and unconditionally wrote
lock.pid before knowing whether the flock would actually succeed. A failed
attempt from session B (lock held by session A) would delete A's lock.ok and
overwrite A's lock.pid with B's own doomed PID, then B's busy-path cleanup
removed the file entirely — leaving A's later `down` unable to find A's
holder, leaking both the detached process and the machine-singleton lock.
Now the holder subprocess itself is the only writer of lock.ok, and only
ever after its own `flock -n 9` succeeds (atomic temp-file + mv, content =
the holder's own $$). The parent only polls for that self-identifying
marker and touches nothing on disk on the failure path, so a busy lock can
never clobber another session's bookkeeping. lock.pid is retired — lock.ok's
content is now the sole source of truth stop_lock_holder reads.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The first bounded acceptance run (task-8-report.md) produced a
session.log with exactly one line — claude -p's own "Error: Reached
max turns" — because the default --output-format=text only prints the
FINAL message, never the intermediate tool_use/tool_result turns. That
failed the #399 acceptance bar ("session.log shows real Playwright MCP
tool calls").
Switch run_explorer to --output-format stream-json --verbose (the CLI
requires both together) and reformat the JSONL with jq into readable
[assistant]/[tool_use]/[tool_result]/[result] lines, truncated to 500
chars each, so session.log is both grep-able and human-skimmable.
fromjson? tolerates stray non-JSON lines instead of aborting the whole
transcript; claude -p's stderr now goes to its own session.stderr.log
so it can never interleave with (and corrupt) the JSON stream jq
parses.
Verified: a second bounded run (EXPLORE_MAX_TURNS=25) produced a
281-line session.log with 18 mcp__playwright__* tool calls across
/dashboard and /library, plus findings.md with a re-confirmed #430
repro and the oracle final pass re-confirming #355.
The follow-up bounded-run diagnosis found a contradiction: run 2's
session cookie was accepted (/api/users 200) but the UI acted fully
signed-out (Sign In button, dashboard skeleton) — the #430 symptom.
Run 1's "Secure cookie dropped over http" diagnosis didn't hold up
either, since Chromium does accept Secure cookies on http://localhost.
Root-caused by reading @playwright/mcp@0.0.78's bundled source
(playwright-core/lib/coreBundle.js): without --isolated, the MCP
server launches ONE PERSISTENT Chrome profile keyed only by
sha256(cwd) — reused across every explore.sh run from this checkout,
never wiped by teardown — and its client factory does
`config.browser.isolated ? await browser.newContext(contextOptions) :
browser.contexts()[0]`. In the default (non-isolated) branch it just
grabs the already-open persistent context and never calls newContext
with our --storage-state at all.
Verified empirically: wiped the profile dir, booted a clean stack, and
drove a 3-turn claude -p probe against the (then-current) mcp.json —
navigating to /dashboard redirected straight to a REAL
accounts.google.com sign-in page, and sqlite3 on the profile's Cookies
db afterward showed zero sapling_session rows (neither cookie nor
localStorage from storageState.json was ever applied). The identical
probe with --isolated added rendered the real, fully-authenticated
dashboard on the first navigation, with localStorage.sapling_user
correctly present — this is the same mechanism (ephemeral
browser.newContext(contextOptions)) @playwright/test itself uses in
Chapter 1's global-setup.ts.
Also corrected mint_storage_state's cookie to secure:false, matching
what the backend's own SECURE_COOKIES policy actually issues for the
http://localhost local stack (config.py derives it from FRONTEND_URL's
scheme) — the file should mirror reality regardless of which flag
turned out to be the load-bearing one.
Verified: EXPLORE_MAX_TURNS=12 make explore reached a signed-in
dashboard (nav shows "Rich Active" / "Account", full authenticated
menu) using only 2 real Playwright tool calls, zero sign-in recovery
turns.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The definitive acceptance run (harness fixes verified: real auth via
--isolated, breadth across 2+ surfaces) hit a genuine new bug —
resuming a tutor session 500'd on POST /api/graph/.../concept-description,
root-caused via the explorer's own backend-log investigation to a
missing SAPLING_FUNCTION_HANDLERS registration for 'concept_describe'
(caught independently by the oracle's logscan pass too, so it's not
lost, just not explorer-authored). But the explorer spent its
remaining turns tailing logs and checking processes to nail the exact
LookupError, and ran out of budget before writing an F<N> entry to
findings.md — a real gap against #399's "readable transcript AND
findings file" bar, distinct from any flag/mechanism defect.
Add a "stub it before you dig" ground rule: write a one-line stub
finding the instant something looks off, before further root-causing.
A written stub survives a turn-budget cutoff; a perfect unwritten
diagnosis does not.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:34 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: dc0a75e3-31f4-42be-8ec2-bb9a848919bb

📥 Commits

Reviewing files that changed from the base of the PR and between 914b0ae and a0360ff.

📒 Files selected for processing (1)
  • scripts/explore.sh
📝 Walkthrough

Walkthrough

Adds a locked local exploratory-testing workflow that boots the E2E stack, authenticates a seeded browser session, runs Claude through Playwright MCP, records findings and oracle output, and reliably tears down the stack.

Changes

Exploration harness

Layer / File(s)Summary
Exploration contract and entrypoints
.claude/skills/explore/SKILL.md, scripts/explore/explorer-prompt.md, Makefile, .gitignore
Defines the exploration workflow, reporting constraints, findings format, operator targets, and ignored .explore/ artifacts.
Locked stack setup
scripts/explore.sh
Adds prerequisite checks, repository configuration, detached flock locking, and local E2E stack startup.
Authenticated browser session
scripts/explore.sh
Creates seeded-user Playwright storage state, writes the pinned MCP configuration, and runs Claude with structured logs and separate stderr output.
Oracle and teardown lifecycle
scripts/explore.sh
Supports full, up, and down modes, appends final oracle results, tears down the stack, and releases the lock.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
participant Operator
participant explore.sh
participant E2EStack
participant AuthTestEndpoint
participant PlaywrightMCP
participant Browser
participant Claude
Operator->>explore.sh: start exploration
explore.sh->>E2EStack: boot local E2E stack
explore.sh->>AuthTestEndpoint: POST seeded student login
explore.sh->>PlaywrightMCP: configure storage state and MCP
Claude->>PlaywrightMCP: execute exploration tools
PlaywrightMCP->>Browser: drive localhost browser
explore.sh->>E2EStack: run oracle and teardown
Loading

Possibly related issues

  • Issue 403 — Covers the broader Chapter 2 AI Agent Explorer epic implemented by this harness.
  • Issue 401 — Proposes nightly automation and regression promotion for findings produced by this exploration workflow.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly names the new explore harness and /explore skill and matches the main change.
Description check✅ PassedThe description covers summary, changes, related issue, testing, screenshots, and reviewer notes with mostly complete content.
Linked Issues check✅ PassedThe PR implements the local Playwright explorer harness, seeded auth, hard budget/localhost limits, structured logs, and readable session output required by #399.
Out of Scope Changes check✅ PassedThe changes stay focused on the explore harness, prompt, Makefile, and ignore rules; no unrelated functionality is introduced.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch e2e-chapter2-explore

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Jul 28, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-staginga0360ffCommit Preview URL

Branch Preview URL
Jul 28 2026, 10:20 AM

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
scripts/explore.sh (1)

149-154: 🩺 Stability & Availability | 🔵 Trivial

stop_lock_holder/do_down release the machine lock without verifying the caller owns it.

do_down (and thus scripts/explore.sh down / make explore-down) unconditionally reads lock.ok's pid and kills it, regardless of whether the invoking shell is the one that acquired it. The elaborate ownership-safety work in start_lock_holder (a failed attempt never touches lock.ok) isn't mirrored here: since the lock is uid-namespaced (/tmp/claude-$(id -u)/...), the realistic blast radius is a single operator accidentally running down from a second terminal while up is still driving an interactive session elsewhere on the same machine — which would yank the shared local stack out from under that live session with no warning. Given the doc explicitly anticipates the "interactive flow" (separate up/down invocations) as a supported mode, this seems worth at least a guard (e.g. warn if a stack that looks actively in-use by claude is being torn down) or an explicit doc callout of the risk.

Also applies to: 404-419

🤖 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/explore.sh` around lines 149 - 154, Update stop_lock_holder and the
do_down flow to guard against tearing down a lock owned by another active
interactive session. Before killing the PID from lock.ok, detect whether it
represents an active claude-managed stack and warn or require explicit
confirmation when ownership cannot be established; preserve cleanup for the
invoking owner and document the supported separate up/down behavior if that is
the chosen safeguard.
🤖 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 `@scripts/explore.sh`:
- Around line 176-202: Update the localStorage payload in mint_storage_state so
the sapling_user name is derived from $EXPLORE_USER instead of hardcoding "Rich
Active". Preserve the existing user ID and other payload fields while ensuring
any configured seeded rich-* user receives a matching cached display name.
- Around line 302-337: Restrict the Write/Edit entries in run_explorer() to the
permitted .explore findings path, using path-scoped rules such as
Edit(.explore/**/*.md) and removing blanket file-edit permissions. Also verify
the current Claude CLI Bash permission syntax for the cd backend &&
venv/bin/python -m e2e_oracles:* rule, tightening it if compound-command parsing
permits commands outside the intended prefix.
- Around line 246-261: Update write_mcp_config to pass Playwright MCP
localhost-only origin restrictions in the generated args, using the required
localhost/127.0.0.1 allowlist or equivalent blocked-origins plus allowlist
configuration. Document alongside the configuration that these flags are
defense-in-depth only and do not prevent redirects from reaching off-localhost
destinations.
---
Nitpick comments:
In `@scripts/explore.sh`:
- Around line 149-154: Update stop_lock_holder and the do_down flow to guard
against tearing down a lock owned by another active interactive session. Before
killing the PID from lock.ok, detect whether it represents an active
claude-managed stack and warn or require explicit confirmation when ownership
cannot be established; preserve cleanup for the invoking owner and document the
supported separate up/down behavior if that is the chosen safeguard.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 762bcd62-0ca2-4e80-bacf-4434944c876d

📥 Commits

Reviewing files that changed from the base of the PR and between db0ecad and 914b0ae.

📒 Files selected for processing (5)
  • .claude/skills/explore/SKILL.md
  • .gitignore
  • Makefile
  • scripts/explore.sh
  • scripts/explore/explorer-prompt.md

Comment threadscripts/explore.sh
Comment threadscripts/explore.sh
Comment on lines +246 to +261
write_mcp_config() {
local headless_args='"--headless", '
[ "$EXPLORE_HEADED" = "1" ] && headless_args=''
cat > "$EXPLORE_DIR/mcp.json" <<EOF
{
"mcpServers": {
"playwright": {
"command": "npx",
"args": ["-y", "@playwright/mcp@$PLAYWRIGHT_MCP_VERSION", "--isolated", ${headless_args}"--browser", "chromium",
"--storage-state", "$EXPLORE_DIR/storageState.json",
"--output-dir", "$EXPLORE_DIR/traces", "--save-session"]
}
}
}
EOF
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== locate files =="
fd -a 'explore.sh|explorer-prompt.md|package.json|pnpm-lock.yaml|package-lock.json|yarn.lock|playwright|mcp.json'.| sed 's#^\./##'| head -200
echoecho"== relevant explore.sh lines =="if [ -f scripts/explore.sh ];then
nl -ba scripts/explore.sh | sed -n '220,280p'fiechoecho"== relevant explorer-prompt lines =="if [ -f scripts/explore/explorer-prompt.md ];then
nl -ba scripts/explore/explorer-prompt.md | sed -n '1,35p'fiif [ -f explorer-prompt.md ];then
nl -ba explorer-prompt.md | sed -n '1,35p'fiechoecho"== search required navigation text and mcp config args =="
rg -n "Restrict navigation|localhost|localhost-only|localhost:3000|allowed-origins|blocked-origins|explorer-prompt|MISSION|write_mcp_config|PLAYWRIGHT_MCP_VERSION" -S .

Repository: SaplingLearn/Sapling

Length of output: 479


🌐 Web query:

Playwright MCP 0.0.78 README --allowed-origins --blocked-origins

💡 Result:

In Playwright MCP, the --allowed-origins and --blocked-origins flags (and their corresponding environment variables) are used to manage network access for the browser instance [1][2][3]. Both settings accept a semicolon-separated list of origins [1][4][3]. Key details: --allowed-origins This flag specifies a list of trusted origins that the browser is permitted to request [1][5]. By default, all origins are allowed [1][2]. Environment variable: PLAYWRIGHT_MCP_ALLOWED_ORIGINS [1][3]. --blocked-origins This flag specifies a list of origins to block the browser from requesting [1][5]. The blocklist is evaluated before the allowlist; if an origin is present in both, it will be blocked [1][2][5]. Environment variable: PLAYWRIGHT_MCP_BLOCKED_ORIGINS [1][3]. Important Security Note: Documentation explicitly states that these flags do not serve as a security boundary and do not affect redirects [1][5][3]. Supported Formats: Both flags support the following formats [1][2]: Full origin: e.g., https://example.com:8080 (matches only that specific origin) [1][2]. Wildcard port: e.g., http://localhost:* (matches any port on localhost with the http protocol) [1][2].

Citations:


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== relevant explore.sh lines =="if [ -f scripts/explore.sh ];then
sed -n '220,280p' scripts/explore.sh | cat -n
fiechoecho"== relevant explorer-prompt lines =="if [ -f scripts/explore/explorer-prompt.md ];then
sed -n '1,40p' scripts/explore/explorer-prompt.md | cat -n
fiechoecho"== search required navigation text and mcp config args =="
rg -n "Restrict navigation|localhost|localhost-only|localhost:3000|allowed-origins|blocked-origins|explorer-prompt|MISSION|write_mcp_config|PLAYWRIGHT_MCP_VERSION" -S .echoecho"== lockfile version snippets =="forfin package-lock.json frontend/package-lock.json;doif [ -f"$f" ];thenecho"--- $f ---"
rg -n '"`@playwright/mcp`"|"`@playwright/test`"|PLAYWRIGHT_MCP_VERSION'"$f" -A 3 -B 2 ||truefidone

Repository: SaplingLearn/Sapling

Length of output: 22370


🌐 Web query:

playwright mcp --allowed-origins blocked navigation error network error

💡 Result:

The "blocked navigation error" or network-related errors when using the --allowed-origins flag in Playwright MCP typically stem from configuration format issues, regex/glob matching errors, or confusing the flag with server-side CORS settings [1][2][3]. Key troubleshooting steps include: 1. Use Correct Origin Format: Ensure you are passing the domain or origin directly (e.g., google.com) rather than including protocol prefixes like https:// or trailing slashes, which have historically caused parsing issues [1]. 2. Avoid Invalid Glob Patterns: Recent versions (as of early 2026) have had bugs where wildcard port patterns like http://localhost:* generate invalid internal glob patterns, causing the browser to block all traffic to those destinations [3]. If you are experiencing this, you may need to use explicit hostnames or check for updates addressing the originOrHostGlob function [3]. 3. Distinguish Between Flags: Ensure you are not confusing --allowed-origins (which controls browser navigation within the sandboxed Playwright instance) with --allowed-hosts (which controls MCP server CORS/connectivity) [2]. Both may need to be set identically if you are navigating to localhost in restricted environments like GitHub Actions or WSL [2][4]. 4. Blocklist Precedence: Keep in mind that Playwright MCP evaluates the blocked-origins list before the allowed-origins list. If a URL matches both, it will be blocked [5]. 5. Debugging Network Issues: If navigation still fails with ERR_CONNECTION_REFUSED, verify that the target service (e.g., a local dev server) is listening on 0.0.0.0 rather than just 127.0.0.1 to ensure it is accessible to the browser process, especially if running within containers (Docker) or WSL [2][4]. For further debugging, you can enable verbose logging by setting the DEBUG environment variable to pw:api or pw:browser* to see exactly which origin is failing to match the navigation request [4].

Citations:


🌐 Web query:

Playwright MCP blocked origins error "Failed to navigate to"

💡 Result:

The error net::ERR_BLOCKED_BY_CLIENT when using Playwright MCP typically indicates that the browser navigation was blocked by a client-side security policy, configuration, or environment restriction [1]. Because Playwright MCP is often used in automated contexts (such as LLM agents), this error is usually triggered by one of the following factors [1]: 1. Allowed Origins Configuration: If you have configured allowed-origins for security, the destination URL may be excluded from the allowlist [2][3]. Verify that the PW_ALLOWED_ORIGINS environment variable or the --allowed-origins CLI flag is correctly set to include the domain you are attempting to visit [2][3]. 2. Browser Environment and Sandbox Restrictions: When running in containerized environments like Docker, Chromium often fails to navigate because it cannot access the necessary system sandbox [1]. To resolve this, you may need to pass explicit launch flags to bypass sandbox restrictions in your PLAYWRIGHT_LAUNCH_OPTIONS [1]: --no-sandbox --disable-setuid-sandbox --disable-gpu 3. Security Settings or Extensions: Although Playwright MCP sessions are typically isolated, ERR_BLOCKED_BY_CLIENT can also occur if the browser launch arguments include restrictive security settings or if there are conflicts with other security-related flags [1]. 4. Troubleshooting Steps: - Check the logs to confirm the specific URL being blocked [1]. - If you are running in Docker, ensure the container has the necessary privileges and sandbox bypass flags [1]. - Review your MCP configuration file (often found in .mcp/config.json or equivalent) to ensure --allowed-origins is formatted correctly (e.g., comma-separated or semicolon-separated depending on your specific implementation) [4][2][3]. - If you suspect the issue is not related to origins, verify that npx and the Playwright runtime are correctly installed and accessible in your environment, as missing dependencies can sometimes lead to unexpected browser behavior [5]. If the error persists despite correct origin configuration, it is often helpful to test a basic navigation command outside of the MCP wrapper to rule out environment-specific sandbox or policy issues [1].

Citations:


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== package manifests mentioning playwright mcp =="
rg -n '"`@playwright/mcp`"|"playwright-mcp"|PLAYWRIGHT_MCP_VERSION' -S --glob '!node_modules/**' --glob '!frontend/node_modules/**' frontend package.json scripts
echoecho"== scripts/explore.sh version assignment and mcp config callers =="
nl -ba scripts/explore.sh | sed -n '60,80p'
nl -ba scripts/explore.sh | sed -n '370,390p'echoecho"== local prompt copy =="
cat -n scripts/explore/explorer-prompt.md | sed -n '1,20p'

Repository: SaplingLearn/Sapling

Length of output: 587


🌐 Web query:

site:github.com/microsoft/playwright-mcp "allowedOrigins" "blockedByClient" "ERR_BLOCKED_BY_CLIENT"

💡 Result:

The error ERR_BLOCKED_BY_CLIENT in the context of Playwright MCP (Model Context Protocol) is a browser-level error that occurs when the browser is instructed to navigate to or request a resource that has been explicitly blocked or is unreachable due to configuration issues [1][2]. In the specific context of the Playwright MCP server, this error frequently arises from the following scenarios: 1. Origin Filtering: If you have configured --allowed-origins or --blocked-origins, the browser will block navigation to any origin not in the allowlist or present in the blocklist [1]. Note that these flags affect navigation but do not serve as a CORS security boundary [1][3]. 2. Extension ID Mismatch: A common cause for ERR_BLOCKED_BY_CLIENT in recent versions (e.g., v0.0.66–0.0.68) is a hardcoded Chrome extension ID in the MCP server's connection logic [2]. If you are using an unpacked extension, a locally built version, or a different browser channel, your extension will have a different ID than the one hardcoded (mmlmfjhmonkocbjadbfplnigmagldckm). This causes the browser to attempt to connect to a non-existent extension, resulting in the error [2]. 3. Configuration Bugs: Bugs in how origin patterns are parsed—such as incorrect handling of wildcard ports (e.g., http://localhost:*)—can lead to requests being unexpectedly blocked [4]. To resolve or debug this error: - Verify your --allowed-origins and --blocked-origins configurations to ensure they match the intended URLs [1][4]. - If using an unpacked browser extension, ensure the extension ID is handled correctly, potentially by setting the PLAYWRIGHT_MCP_EXTENSION_ID environment variable or pinning your extension's ID via the manifest's "key" property [2]. - Check the official issue tracker for your version of Playwright MCP, as specific releases have included fixes for origin pattern parsing and connection logic [4][2][5].

Citations:


🌐 Web query:

github.com/microsoft/playwright-mcp program.ts "allowed-origins" "allowedOrigins" "blockedByClient"

💡 Result:

In the Playwright MCP server, allowedOrigins and blockedOrigins are configuration options used to control which network requests the browser is permitted to make [1][2]. These settings are managed within the server's network configuration and are implemented via Playwright's routing capabilities [3][4]. When these rules are active, the server intercepts network requests and uses the 'blockedbyclient' error code to terminate requests that do not align with the defined allow/block lists [3][4]. Specifically, the implementation follows this logic: 1. If allowedOrigins are defined, the server first blocks all requests by default using route.abort('blockedbyclient') [3][5]. 2. It then explicitly allows requests to the specified origins by calling route.continue for patterns matching the allowed list [3][4]. 3. If blockedOrigins are defined, the server explicitly blocks requests to those origins using route.abort('blockedbyclient') [3][4]. Origins matching both allowedOrigins and blockedOrigins will be blocked [1][2]. The allowed-origins command-line argument accepts a semicolon-separated list of origins, and the configuration supports full origin strings (e.g., https://example.com:8080) and wildcard port patterns (e.g., http://localhost:*) [1][6]. These settings are considered convenience features for managing browser interactions rather than strict security boundaries [1][2]. The file program.ts within the Playwright repository is involved in managing the command-line interface and tool configuration for the MCP server [7].

Citations:


Reinforce the localhost-first prompt with a technical restriction.

This harness requires localhost-only navigation, but mcp.json relies only on scripts/explore/explorer-prompt.md:13 for enforcement. Add a Playwright MCP restriction such as --allowed-origins http://localhost:3000;http://localhost:5000;127.0.0.1:5000 or --blocked-origins * plus the localhost allowlist, while documenting that these flags are not a security boundary against redirects; this gives defense-in-depth against model-driven or page-driven off-localhost clicks.

🤖 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/explore.sh` around lines 246 - 261, Update write_mcp_config to pass
Playwright MCP localhost-only origin restrictions in the generated args, using
the required localhost/127.0.0.1 allowlist or equivalent blocked-origins plus
allowlist configuration. Document alongside the configuration that these flags
are defense-in-depth only and do not prevent redirects from reaching
off-localhost destinations.

Comment threadscripts/explore.sh
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

Code review

Found 3 issues:

  1. do_down() can leak the stack and the machine-singleton lock: it runs under set -euo pipefail and fires as an EXIT trap, but its mkdir -p, the findings-file bootstrap, and the { … } >> findings.md append block have no || true guards (unlike the oracle subshell and e2e-down.sh lines around them). A failed write there (disk full, read-only fs) aborts the trap before e2e-down.sh and stop_lock_holder run — verified bash semantics: a failing command inside an EXIT trap under set -e stops the trap. The header comment's "|| true guards throughout" claim is currently false for these lines.

do_down() {
mkdir -p "$EXPLORE_DIR"
(
cd"$REPO_ROOT/backend"
venv/bin/python -m e2e_oracles --json >"$EXPLORE_DIR/oracle-final.json"||true
venv/bin/python -m e2e_oracles >"$EXPLORE_DIR/oracle-final.txt"2>&1||true
) ||true
[ -f"$EXPLORE_DIR/findings.md" ] ||printf'# Exploration findings\n'>"$EXPLORE_DIR/findings.md"
{
printf'\n## Oracle final pass (%s)\n\n```\n'"$(date -Iseconds)"
cat "$EXPLORE_DIR/oracle-final.txt"2>/dev/null ||true
printf'```\n'
} >>"$EXPLORE_DIR/findings.md"
"$REPO_ROOT/scripts/e2e-down.sh"||true
stop_lock_holder
}

  1. Dummy-key mitigation for RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439 defeats itself in the common developer environment: export GEMINI_API_KEY="${GEMINI_API_KEY:-e2e-dummy-key-no-billing}" defers to an ambient real key, and developers here routinely keep one exported (validate_config fail-closes without it). The below-seam RAG/embedding path (RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439) bills live even in function mode, and the explorer's mission (uploads, quiz, tutor turns, many turns) is worst-case volume for it. CI applies the dummy key unconditionally for exactly this reason; export inside the script can't touch the operator's parent shell, so the conditional protects nothing.

# Default only if unset — never clobber an operator's real key.
export GEMINI_API_KEY="${GEMINI_API_KEY:-e2e-dummy-key-no-billing}"
"$REPO_ROOT/scripts/e2e-up.sh"|| die "scripts/e2e-up.sh failed — see .e2e/*.log"

  1. jq is a hard dependency of the transcript pipeline but is missing from preflight (claude/npx/curl/python3 are checked). Without it the script boots the entire stack — Supabase, migrations, a production Next build, minutes of work — and only then fails in run_explorer, instead of failing fast like the other four checks.

echo"▶ Preflight…"
command -v claude >/dev/null 2>&1|| die "claude CLI not found on PATH — install it first"
command -v npx >/dev/null 2>&1|| die "npx not found on PATH — install Node.js first"
command -v curl >/dev/null 2>&1|| die "curl not found on PATH"
command -v python3 >/dev/null 2>&1|| die "python3 not found on PATH"
[ -f"$REPO_ROOT/scripts/explore/explorer-prompt.md" ] \

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

…eflight, scoped Edit, EXPLORE_USER wiring (#399)
- do_down: guard every fallible write with || true so a failed findings.md
write can never abort the EXIT trap before e2e-down.sh / stop_lock_holder
run (was leaking the stack + machine-singleton lock on write failure).
- GEMINI_API_KEY: export unconditionally (matches CI's dummy, #439) instead
of deferring to an ambient real key that can bill the below-seam RAG path.
- SEED_RICH=1 exported unconditionally so an ambient SEED_RICH=0 can't
silently defeat the rich dataset this harness requires.
- preflight: add missing `jq` check (hard dependency of the transcript
pipeline) so a missing jq fails in seconds, not after a full stack boot.
- allowedTools: replace unscoped Write,Edit with Read,Edit(.explore/**) —
Write(...) patterns aren't matched by the CLI's file permission check, so
only a path-scoped Edit rule actually restricts writes to findings.md.
do_up now pre-creates .explore/findings.md so the explorer always has an
existing file for the scoped Edit grant.
- EXPLORE_USER: derive the sapling_user display name from the user id
(case map for the five seeded rich-* users, verified against
db/seed_local_rich.py) instead of hardcoding "Rich Active"; pass
--user "$EXPLORE_USER" to both oracle invocations in do_down.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

All three review findings fixed in a0360ff (do_down teardown/lock-release guards verified with a real write-failure injection; unconditional dummy GEMINI_API_KEY per #439; jq preflight), plus three panel-verified hardening items in the same commit: allowedTools Write dropped / Edit scoped to .explore/**, EXPLORE_USER wired through the name map and both oracle --user calls, unconditional SEED_RICH=1. Scoped re-review confirms all addressed, no new breakage.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(agent): LLM explorer harness driving Playwright against the local stack

1 participant

@AndresL230
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

feat(e2e): Chapter 2 explore harness — make explore + /explore skill (#399) - #444

Merged
AndresL230 merged 9 commits into
mainfrom
e2e-chapter2-explore
Jul 28, 2026
Merged

feat(e2e): Chapter 2 explore harness — make explore + /explore skill (#399)#444
AndresL230 merged 9 commits into
mainfrom
e2e-chapter2-explore

Conversation

@AndresL230

@AndresL230AndresL230 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Chapter 2 of the E2E program (epic #403), PR 2 of 3. Closes#399. Builds on the #400 oracle module (PR #443).

make explore boots the deterministic local stack and hands a Claude Code explorer a real browser; the oracles judge what it saw. Local-only by design — never a CI gate.

What's here

  • scripts/explore.sh (+ make explore / make explore-down, .explore/ gitignored) — the full pipeline: machine-singleton stack lock (detached holder process writing self-identifying lock.ok only after flock succeeds — a failed acquisition touches nothing, proven by an A/B two-session simulation) → e2e-up in function mode with a dummy GEMINI_API_KEY (RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439) → storage state minted via POST /api/auth/test-login through the frontend origin (cookie + sapling_user localStorage, secure flag matching the backend's actual local policy) → claude -p with a pinned @playwright/mcp@0.0.78 config and turn budget, transcript captured via stream-json + jq into a grep-able session.log → oracle final pass appended to findings.md → teardown. up/down subcommands support the interactive flow.
  • scripts/explore/explorer-prompt.md — the mission briefing: student persona (Rich Active), break-things mandate, report-never-fix, oracle cadence (cd backend && venv/bin/python -m e2e_oracles), findings format, known-bugs list, and a "stub it before you dig" rule earned in live testing.
  • .claude/skills/explore/SKILL.md — the interactive /explore skill: same pipeline, watchable and steerable, with a same-origin JS sign-in for live-browser sessions.

Hard-won fixes from live acceptance runs

  • @playwright/mcp without --isolated reuses a persistent profile and silently ignores --storage-state — the run-2 "signed-in" state was a stale-cookie artifact; --isolated fixes it (verified down to the bundled playwright-core source and a cookie-DB probe).
  • claude -p's default text output prints only the final message — a turn-budget exit produced an empty transcript despite real browsing; --output-format stream-json --verbose + jq restores the readable transcript.
  • Lock-holder bookkeeping survives concurrent sessions (busy attempt leaves the running session's files byte-identical).

Acceptance (issue #399 bar, definitive run in the SDD report)

One bounded run (EXPLORE_MAX_TURNS=25, 3m52s): real signed-in navigation across dashboard + library ("Rich Active" nav, 4 seeded documents, CS101/MATH210/BIO110/ENG150 visible in snapshots), 2 explorer-written findings — including a novel wrong-data upload bug and a tutor-resume 500 (concept_describe unregistered in the function-handler seam) independently confirmed by the oracle logscan — plus the harness-appended Oracle final pass re-confirming #355. Machine verified clean afterward (ports, containers, lock).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added an end-to-end exploratory testing workflow for the local application.
    • Added commands to start, stop, and run complete exploration sessions.
    • Exploration sessions now support browser-driven testing, automated checks, session logs, and structured findings.
  • Documentation

    • Added guidance covering exploration steps, reporting requirements, known issues, and session summaries.
  • Chores

    • Exploration artifacts are now excluded from version control.

AndresL230and others added 8 commits July 28, 2026 01:13
…itignore (#399)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e, oracle cadence (#399)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…er GEMINI_API_KEY (#399)
A busy lock previously still tripped do_up's trap/wipe path, tearing down
another session's live stack via scripts/e2e-down.sh and deleting its
.explore/ artifacts before the lock check ever ran. start_lock_holder now
runs first, the do_down safety trap arms only after the lock is held, the
.explore/ wipe (still selective, preserving lock.pid/lock.ok) happens after
that, and a failed acquisition cleans up its own holder/pid files before
exiting. GEMINI_API_KEY now defaults only when unset instead of always
overwriting an operator's real key.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ns (#399)
start_lock_holder previously deleted lock.ok and unconditionally wrote
lock.pid before knowing whether the flock would actually succeed. A failed
attempt from session B (lock held by session A) would delete A's lock.ok and
overwrite A's lock.pid with B's own doomed PID, then B's busy-path cleanup
removed the file entirely — leaving A's later `down` unable to find A's
holder, leaking both the detached process and the machine-singleton lock.
Now the holder subprocess itself is the only writer of lock.ok, and only
ever after its own `flock -n 9` succeeds (atomic temp-file + mv, content =
the holder's own $$). The parent only polls for that self-identifying
marker and touches nothing on disk on the failure path, so a busy lock can
never clobber another session's bookkeeping. lock.pid is retired — lock.ok's
content is now the sole source of truth stop_lock_holder reads.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The first bounded acceptance run (task-8-report.md) produced a
session.log with exactly one line — claude -p's own "Error: Reached
max turns" — because the default --output-format=text only prints the
FINAL message, never the intermediate tool_use/tool_result turns. That
failed the #399 acceptance bar ("session.log shows real Playwright MCP
tool calls").
Switch run_explorer to --output-format stream-json --verbose (the CLI
requires both together) and reformat the JSONL with jq into readable
[assistant]/[tool_use]/[tool_result]/[result] lines, truncated to 500
chars each, so session.log is both grep-able and human-skimmable.
fromjson? tolerates stray non-JSON lines instead of aborting the whole
transcript; claude -p's stderr now goes to its own session.stderr.log
so it can never interleave with (and corrupt) the JSON stream jq
parses.
Verified: a second bounded run (EXPLORE_MAX_TURNS=25) produced a
281-line session.log with 18 mcp__playwright__* tool calls across
/dashboard and /library, plus findings.md with a re-confirmed #430
repro and the oracle final pass re-confirming #355.
The follow-up bounded-run diagnosis found a contradiction: run 2's
session cookie was accepted (/api/users 200) but the UI acted fully
signed-out (Sign In button, dashboard skeleton) — the #430 symptom.
Run 1's "Secure cookie dropped over http" diagnosis didn't hold up
either, since Chromium does accept Secure cookies on http://localhost.
Root-caused by reading @playwright/mcp@0.0.78's bundled source
(playwright-core/lib/coreBundle.js): without --isolated, the MCP
server launches ONE PERSISTENT Chrome profile keyed only by
sha256(cwd) — reused across every explore.sh run from this checkout,
never wiped by teardown — and its client factory does
`config.browser.isolated ? await browser.newContext(contextOptions) :
browser.contexts()[0]`. In the default (non-isolated) branch it just
grabs the already-open persistent context and never calls newContext
with our --storage-state at all.
Verified empirically: wiped the profile dir, booted a clean stack, and
drove a 3-turn claude -p probe against the (then-current) mcp.json —
navigating to /dashboard redirected straight to a REAL
accounts.google.com sign-in page, and sqlite3 on the profile's Cookies
db afterward showed zero sapling_session rows (neither cookie nor
localStorage from storageState.json was ever applied). The identical
probe with --isolated added rendered the real, fully-authenticated
dashboard on the first navigation, with localStorage.sapling_user
correctly present — this is the same mechanism (ephemeral
browser.newContext(contextOptions)) @playwright/test itself uses in
Chapter 1's global-setup.ts.
Also corrected mint_storage_state's cookie to secure:false, matching
what the backend's own SECURE_COOKIES policy actually issues for the
http://localhost local stack (config.py derives it from FRONTEND_URL's
scheme) — the file should mirror reality regardless of which flag
turned out to be the load-bearing one.
Verified: EXPLORE_MAX_TURNS=12 make explore reached a signed-in
dashboard (nav shows "Rich Active" / "Account", full authenticated
menu) using only 2 real Playwright tool calls, zero sign-in recovery
turns.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The definitive acceptance run (harness fixes verified: real auth via
--isolated, breadth across 2+ surfaces) hit a genuine new bug —
resuming a tutor session 500'd on POST /api/graph/.../concept-description,
root-caused via the explorer's own backend-log investigation to a
missing SAPLING_FUNCTION_HANDLERS registration for 'concept_describe'
(caught independently by the oracle's logscan pass too, so it's not
lost, just not explorer-authored). But the explorer spent its
remaining turns tailing logs and checking processes to nail the exact
LookupError, and ran out of budget before writing an F<N> entry to
findings.md — a real gap against #399's "readable transcript AND
findings file" bar, distinct from any flag/mechanism defect.
Add a "stub it before you dig" ground rule: write a one-line stub
finding the instant something looks off, before further root-causing.
A written stub survives a turn-budget cutoff; a perfect unwritten
diagnosis does not.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:34 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: dc0a75e3-31f4-42be-8ec2-bb9a848919bb

📥 Commits

Reviewing files that changed from the base of the PR and between 914b0ae and a0360ff.

📒 Files selected for processing (1)
  • scripts/explore.sh
📝 Walkthrough

Walkthrough

Adds a locked local exploratory-testing workflow that boots the E2E stack, authenticates a seeded browser session, runs Claude through Playwright MCP, records findings and oracle output, and reliably tears down the stack.

Changes

Exploration harness

Layer / File(s)Summary
Exploration contract and entrypoints
.claude/skills/explore/SKILL.md, scripts/explore/explorer-prompt.md, Makefile, .gitignore
Defines the exploration workflow, reporting constraints, findings format, operator targets, and ignored .explore/ artifacts.
Locked stack setup
scripts/explore.sh
Adds prerequisite checks, repository configuration, detached flock locking, and local E2E stack startup.
Authenticated browser session
scripts/explore.sh
Creates seeded-user Playwright storage state, writes the pinned MCP configuration, and runs Claude with structured logs and separate stderr output.
Oracle and teardown lifecycle
scripts/explore.sh
Supports full, up, and down modes, appends final oracle results, tears down the stack, and releases the lock.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
participant Operator
participant explore.sh
participant E2EStack
participant AuthTestEndpoint
participant PlaywrightMCP
participant Browser
participant Claude
Operator->>explore.sh: start exploration
explore.sh->>E2EStack: boot local E2E stack
explore.sh->>AuthTestEndpoint: POST seeded student login
explore.sh->>PlaywrightMCP: configure storage state and MCP
Claude->>PlaywrightMCP: execute exploration tools
PlaywrightMCP->>Browser: drive localhost browser
explore.sh->>E2EStack: run oracle and teardown
Loading

Possibly related issues

  • Issue 403 — Covers the broader Chapter 2 AI Agent Explorer epic implemented by this harness.
  • Issue 401 — Proposes nightly automation and regression promotion for findings produced by this exploration workflow.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly names the new explore harness and /explore skill and matches the main change.
Description check✅ PassedThe description covers summary, changes, related issue, testing, screenshots, and reviewer notes with mostly complete content.
Linked Issues check✅ PassedThe PR implements the local Playwright explorer harness, seeded auth, hard budget/localhost limits, structured logs, and readable session output required by #399.
Out of Scope Changes check✅ PassedThe changes stay focused on the explore harness, prompt, Makefile, and ignore rules; no unrelated functionality is introduced.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch e2e-chapter2-explore

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Jul 28, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-staginga0360ffCommit Preview URL

Branch Preview URL
Jul 28 2026, 10:20 AM

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
scripts/explore.sh (1)

149-154: 🩺 Stability & Availability | 🔵 Trivial

stop_lock_holder/do_down release the machine lock without verifying the caller owns it.

do_down (and thus scripts/explore.sh down / make explore-down) unconditionally reads lock.ok's pid and kills it, regardless of whether the invoking shell is the one that acquired it. The elaborate ownership-safety work in start_lock_holder (a failed attempt never touches lock.ok) isn't mirrored here: since the lock is uid-namespaced (/tmp/claude-$(id -u)/...), the realistic blast radius is a single operator accidentally running down from a second terminal while up is still driving an interactive session elsewhere on the same machine — which would yank the shared local stack out from under that live session with no warning. Given the doc explicitly anticipates the "interactive flow" (separate up/down invocations) as a supported mode, this seems worth at least a guard (e.g. warn if a stack that looks actively in-use by claude is being torn down) or an explicit doc callout of the risk.

Also applies to: 404-419

🤖 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/explore.sh` around lines 149 - 154, Update stop_lock_holder and the
do_down flow to guard against tearing down a lock owned by another active
interactive session. Before killing the PID from lock.ok, detect whether it
represents an active claude-managed stack and warn or require explicit
confirmation when ownership cannot be established; preserve cleanup for the
invoking owner and document the supported separate up/down behavior if that is
the chosen safeguard.
🤖 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 `@scripts/explore.sh`:
- Around line 176-202: Update the localStorage payload in mint_storage_state so
the sapling_user name is derived from $EXPLORE_USER instead of hardcoding "Rich
Active". Preserve the existing user ID and other payload fields while ensuring
any configured seeded rich-* user receives a matching cached display name.
- Around line 302-337: Restrict the Write/Edit entries in run_explorer() to the
permitted .explore findings path, using path-scoped rules such as
Edit(.explore/**/*.md) and removing blanket file-edit permissions. Also verify
the current Claude CLI Bash permission syntax for the cd backend &&
venv/bin/python -m e2e_oracles:* rule, tightening it if compound-command parsing
permits commands outside the intended prefix.
- Around line 246-261: Update write_mcp_config to pass Playwright MCP
localhost-only origin restrictions in the generated args, using the required
localhost/127.0.0.1 allowlist or equivalent blocked-origins plus allowlist
configuration. Document alongside the configuration that these flags are
defense-in-depth only and do not prevent redirects from reaching off-localhost
destinations.
---
Nitpick comments:
In `@scripts/explore.sh`:
- Around line 149-154: Update stop_lock_holder and the do_down flow to guard
against tearing down a lock owned by another active interactive session. Before
killing the PID from lock.ok, detect whether it represents an active
claude-managed stack and warn or require explicit confirmation when ownership
cannot be established; preserve cleanup for the invoking owner and document the
supported separate up/down behavior if that is the chosen safeguard.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 762bcd62-0ca2-4e80-bacf-4434944c876d

📥 Commits

Reviewing files that changed from the base of the PR and between db0ecad and 914b0ae.

📒 Files selected for processing (5)
  • .claude/skills/explore/SKILL.md
  • .gitignore
  • Makefile
  • scripts/explore.sh
  • scripts/explore/explorer-prompt.md

Comment threadscripts/explore.sh
Comment threadscripts/explore.sh
Comment on lines +246 to +261
write_mcp_config() {
local headless_args='"--headless", '
[ "$EXPLORE_HEADED" = "1" ] && headless_args=''
cat > "$EXPLORE_DIR/mcp.json" <<EOF
{
"mcpServers": {
"playwright": {
"command": "npx",
"args": ["-y", "@playwright/mcp@$PLAYWRIGHT_MCP_VERSION", "--isolated", ${headless_args}"--browser", "chromium",
"--storage-state", "$EXPLORE_DIR/storageState.json",
"--output-dir", "$EXPLORE_DIR/traces", "--save-session"]
}
}
}
EOF
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== locate files =="
fd -a 'explore.sh|explorer-prompt.md|package.json|pnpm-lock.yaml|package-lock.json|yarn.lock|playwright|mcp.json'.| sed 's#^\./##'| head -200
echoecho"== relevant explore.sh lines =="if [ -f scripts/explore.sh ];then
nl -ba scripts/explore.sh | sed -n '220,280p'fiechoecho"== relevant explorer-prompt lines =="if [ -f scripts/explore/explorer-prompt.md ];then
nl -ba scripts/explore/explorer-prompt.md | sed -n '1,35p'fiif [ -f explorer-prompt.md ];then
nl -ba explorer-prompt.md | sed -n '1,35p'fiechoecho"== search required navigation text and mcp config args =="
rg -n "Restrict navigation|localhost|localhost-only|localhost:3000|allowed-origins|blocked-origins|explorer-prompt|MISSION|write_mcp_config|PLAYWRIGHT_MCP_VERSION" -S .

Repository: SaplingLearn/Sapling

Length of output: 479


🌐 Web query:

Playwright MCP 0.0.78 README --allowed-origins --blocked-origins

💡 Result:

In Playwright MCP, the --allowed-origins and --blocked-origins flags (and their corresponding environment variables) are used to manage network access for the browser instance [1][2][3]. Both settings accept a semicolon-separated list of origins [1][4][3]. Key details: --allowed-origins This flag specifies a list of trusted origins that the browser is permitted to request [1][5]. By default, all origins are allowed [1][2]. Environment variable: PLAYWRIGHT_MCP_ALLOWED_ORIGINS [1][3]. --blocked-origins This flag specifies a list of origins to block the browser from requesting [1][5]. The blocklist is evaluated before the allowlist; if an origin is present in both, it will be blocked [1][2][5]. Environment variable: PLAYWRIGHT_MCP_BLOCKED_ORIGINS [1][3]. Important Security Note: Documentation explicitly states that these flags do not serve as a security boundary and do not affect redirects [1][5][3]. Supported Formats: Both flags support the following formats [1][2]: Full origin: e.g., https://example.com:8080 (matches only that specific origin) [1][2]. Wildcard port: e.g., http://localhost:* (matches any port on localhost with the http protocol) [1][2].

Citations:


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== relevant explore.sh lines =="if [ -f scripts/explore.sh ];then
sed -n '220,280p' scripts/explore.sh | cat -n
fiechoecho"== relevant explorer-prompt lines =="if [ -f scripts/explore/explorer-prompt.md ];then
sed -n '1,40p' scripts/explore/explorer-prompt.md | cat -n
fiechoecho"== search required navigation text and mcp config args =="
rg -n "Restrict navigation|localhost|localhost-only|localhost:3000|allowed-origins|blocked-origins|explorer-prompt|MISSION|write_mcp_config|PLAYWRIGHT_MCP_VERSION" -S .echoecho"== lockfile version snippets =="forfin package-lock.json frontend/package-lock.json;doif [ -f"$f" ];thenecho"--- $f ---"
rg -n '"`@playwright/mcp`"|"`@playwright/test`"|PLAYWRIGHT_MCP_VERSION'"$f" -A 3 -B 2 ||truefidone

Repository: SaplingLearn/Sapling

Length of output: 22370


🌐 Web query:

playwright mcp --allowed-origins blocked navigation error network error

💡 Result:

The "blocked navigation error" or network-related errors when using the --allowed-origins flag in Playwright MCP typically stem from configuration format issues, regex/glob matching errors, or confusing the flag with server-side CORS settings [1][2][3]. Key troubleshooting steps include: 1. Use Correct Origin Format: Ensure you are passing the domain or origin directly (e.g., google.com) rather than including protocol prefixes like https:// or trailing slashes, which have historically caused parsing issues [1]. 2. Avoid Invalid Glob Patterns: Recent versions (as of early 2026) have had bugs where wildcard port patterns like http://localhost:* generate invalid internal glob patterns, causing the browser to block all traffic to those destinations [3]. If you are experiencing this, you may need to use explicit hostnames or check for updates addressing the originOrHostGlob function [3]. 3. Distinguish Between Flags: Ensure you are not confusing --allowed-origins (which controls browser navigation within the sandboxed Playwright instance) with --allowed-hosts (which controls MCP server CORS/connectivity) [2]. Both may need to be set identically if you are navigating to localhost in restricted environments like GitHub Actions or WSL [2][4]. 4. Blocklist Precedence: Keep in mind that Playwright MCP evaluates the blocked-origins list before the allowed-origins list. If a URL matches both, it will be blocked [5]. 5. Debugging Network Issues: If navigation still fails with ERR_CONNECTION_REFUSED, verify that the target service (e.g., a local dev server) is listening on 0.0.0.0 rather than just 127.0.0.1 to ensure it is accessible to the browser process, especially if running within containers (Docker) or WSL [2][4]. For further debugging, you can enable verbose logging by setting the DEBUG environment variable to pw:api or pw:browser* to see exactly which origin is failing to match the navigation request [4].

Citations:


🌐 Web query:

Playwright MCP blocked origins error "Failed to navigate to"

💡 Result:

The error net::ERR_BLOCKED_BY_CLIENT when using Playwright MCP typically indicates that the browser navigation was blocked by a client-side security policy, configuration, or environment restriction [1]. Because Playwright MCP is often used in automated contexts (such as LLM agents), this error is usually triggered by one of the following factors [1]: 1. Allowed Origins Configuration: If you have configured allowed-origins for security, the destination URL may be excluded from the allowlist [2][3]. Verify that the PW_ALLOWED_ORIGINS environment variable or the --allowed-origins CLI flag is correctly set to include the domain you are attempting to visit [2][3]. 2. Browser Environment and Sandbox Restrictions: When running in containerized environments like Docker, Chromium often fails to navigate because it cannot access the necessary system sandbox [1]. To resolve this, you may need to pass explicit launch flags to bypass sandbox restrictions in your PLAYWRIGHT_LAUNCH_OPTIONS [1]: --no-sandbox --disable-setuid-sandbox --disable-gpu 3. Security Settings or Extensions: Although Playwright MCP sessions are typically isolated, ERR_BLOCKED_BY_CLIENT can also occur if the browser launch arguments include restrictive security settings or if there are conflicts with other security-related flags [1]. 4. Troubleshooting Steps: - Check the logs to confirm the specific URL being blocked [1]. - If you are running in Docker, ensure the container has the necessary privileges and sandbox bypass flags [1]. - Review your MCP configuration file (often found in .mcp/config.json or equivalent) to ensure --allowed-origins is formatted correctly (e.g., comma-separated or semicolon-separated depending on your specific implementation) [4][2][3]. - If you suspect the issue is not related to origins, verify that npx and the Playwright runtime are correctly installed and accessible in your environment, as missing dependencies can sometimes lead to unexpected browser behavior [5]. If the error persists despite correct origin configuration, it is often helpful to test a basic navigation command outside of the MCP wrapper to rule out environment-specific sandbox or policy issues [1].

Citations:


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== package manifests mentioning playwright mcp =="
rg -n '"`@playwright/mcp`"|"playwright-mcp"|PLAYWRIGHT_MCP_VERSION' -S --glob '!node_modules/**' --glob '!frontend/node_modules/**' frontend package.json scripts
echoecho"== scripts/explore.sh version assignment and mcp config callers =="
nl -ba scripts/explore.sh | sed -n '60,80p'
nl -ba scripts/explore.sh | sed -n '370,390p'echoecho"== local prompt copy =="
cat -n scripts/explore/explorer-prompt.md | sed -n '1,20p'

Repository: SaplingLearn/Sapling

Length of output: 587


🌐 Web query:

site:github.com/microsoft/playwright-mcp "allowedOrigins" "blockedByClient" "ERR_BLOCKED_BY_CLIENT"

💡 Result:

The error ERR_BLOCKED_BY_CLIENT in the context of Playwright MCP (Model Context Protocol) is a browser-level error that occurs when the browser is instructed to navigate to or request a resource that has been explicitly blocked or is unreachable due to configuration issues [1][2]. In the specific context of the Playwright MCP server, this error frequently arises from the following scenarios: 1. Origin Filtering: If you have configured --allowed-origins or --blocked-origins, the browser will block navigation to any origin not in the allowlist or present in the blocklist [1]. Note that these flags affect navigation but do not serve as a CORS security boundary [1][3]. 2. Extension ID Mismatch: A common cause for ERR_BLOCKED_BY_CLIENT in recent versions (e.g., v0.0.66–0.0.68) is a hardcoded Chrome extension ID in the MCP server's connection logic [2]. If you are using an unpacked extension, a locally built version, or a different browser channel, your extension will have a different ID than the one hardcoded (mmlmfjhmonkocbjadbfplnigmagldckm). This causes the browser to attempt to connect to a non-existent extension, resulting in the error [2]. 3. Configuration Bugs: Bugs in how origin patterns are parsed—such as incorrect handling of wildcard ports (e.g., http://localhost:*)—can lead to requests being unexpectedly blocked [4]. To resolve or debug this error: - Verify your --allowed-origins and --blocked-origins configurations to ensure they match the intended URLs [1][4]. - If using an unpacked browser extension, ensure the extension ID is handled correctly, potentially by setting the PLAYWRIGHT_MCP_EXTENSION_ID environment variable or pinning your extension's ID via the manifest's "key" property [2]. - Check the official issue tracker for your version of Playwright MCP, as specific releases have included fixes for origin pattern parsing and connection logic [4][2][5].

Citations:


🌐 Web query:

github.com/microsoft/playwright-mcp program.ts "allowed-origins" "allowedOrigins" "blockedByClient"

💡 Result:

In the Playwright MCP server, allowedOrigins and blockedOrigins are configuration options used to control which network requests the browser is permitted to make [1][2]. These settings are managed within the server's network configuration and are implemented via Playwright's routing capabilities [3][4]. When these rules are active, the server intercepts network requests and uses the 'blockedbyclient' error code to terminate requests that do not align with the defined allow/block lists [3][4]. Specifically, the implementation follows this logic: 1. If allowedOrigins are defined, the server first blocks all requests by default using route.abort('blockedbyclient') [3][5]. 2. It then explicitly allows requests to the specified origins by calling route.continue for patterns matching the allowed list [3][4]. 3. If blockedOrigins are defined, the server explicitly blocks requests to those origins using route.abort('blockedbyclient') [3][4]. Origins matching both allowedOrigins and blockedOrigins will be blocked [1][2]. The allowed-origins command-line argument accepts a semicolon-separated list of origins, and the configuration supports full origin strings (e.g., https://example.com:8080) and wildcard port patterns (e.g., http://localhost:*) [1][6]. These settings are considered convenience features for managing browser interactions rather than strict security boundaries [1][2]. The file program.ts within the Playwright repository is involved in managing the command-line interface and tool configuration for the MCP server [7].

Citations:


Reinforce the localhost-first prompt with a technical restriction.

This harness requires localhost-only navigation, but mcp.json relies only on scripts/explore/explorer-prompt.md:13 for enforcement. Add a Playwright MCP restriction such as --allowed-origins http://localhost:3000;http://localhost:5000;127.0.0.1:5000 or --blocked-origins * plus the localhost allowlist, while documenting that these flags are not a security boundary against redirects; this gives defense-in-depth against model-driven or page-driven off-localhost clicks.

🤖 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/explore.sh` around lines 246 - 261, Update write_mcp_config to pass
Playwright MCP localhost-only origin restrictions in the generated args, using
the required localhost/127.0.0.1 allowlist or equivalent blocked-origins plus
allowlist configuration. Document alongside the configuration that these flags
are defense-in-depth only and do not prevent redirects from reaching
off-localhost destinations.

Comment threadscripts/explore.sh
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

Code review

Found 3 issues:

  1. do_down() can leak the stack and the machine-singleton lock: it runs under set -euo pipefail and fires as an EXIT trap, but its mkdir -p, the findings-file bootstrap, and the { … } >> findings.md append block have no || true guards (unlike the oracle subshell and e2e-down.sh lines around them). A failed write there (disk full, read-only fs) aborts the trap before e2e-down.sh and stop_lock_holder run — verified bash semantics: a failing command inside an EXIT trap under set -e stops the trap. The header comment's "|| true guards throughout" claim is currently false for these lines.

do_down() {
mkdir -p "$EXPLORE_DIR"
(
cd"$REPO_ROOT/backend"
venv/bin/python -m e2e_oracles --json >"$EXPLORE_DIR/oracle-final.json"||true
venv/bin/python -m e2e_oracles >"$EXPLORE_DIR/oracle-final.txt"2>&1||true
) ||true
[ -f"$EXPLORE_DIR/findings.md" ] ||printf'# Exploration findings\n'>"$EXPLORE_DIR/findings.md"
{
printf'\n## Oracle final pass (%s)\n\n```\n'"$(date -Iseconds)"
cat "$EXPLORE_DIR/oracle-final.txt"2>/dev/null ||true
printf'```\n'
} >>"$EXPLORE_DIR/findings.md"
"$REPO_ROOT/scripts/e2e-down.sh"||true
stop_lock_holder
}

  1. Dummy-key mitigation for RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439 defeats itself in the common developer environment: export GEMINI_API_KEY="${GEMINI_API_KEY:-e2e-dummy-key-no-billing}" defers to an ambient real key, and developers here routinely keep one exported (validate_config fail-closes without it). The below-seam RAG/embedding path (RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439) bills live even in function mode, and the explorer's mission (uploads, quiz, tutor turns, many turns) is worst-case volume for it. CI applies the dummy key unconditionally for exactly this reason; export inside the script can't touch the operator's parent shell, so the conditional protects nothing.

# Default only if unset — never clobber an operator's real key.
export GEMINI_API_KEY="${GEMINI_API_KEY:-e2e-dummy-key-no-billing}"
"$REPO_ROOT/scripts/e2e-up.sh"|| die "scripts/e2e-up.sh failed — see .e2e/*.log"

  1. jq is a hard dependency of the transcript pipeline but is missing from preflight (claude/npx/curl/python3 are checked). Without it the script boots the entire stack — Supabase, migrations, a production Next build, minutes of work — and only then fails in run_explorer, instead of failing fast like the other four checks.

echo"▶ Preflight…"
command -v claude >/dev/null 2>&1|| die "claude CLI not found on PATH — install it first"
command -v npx >/dev/null 2>&1|| die "npx not found on PATH — install Node.js first"
command -v curl >/dev/null 2>&1|| die "curl not found on PATH"
command -v python3 >/dev/null 2>&1|| die "python3 not found on PATH"
[ -f"$REPO_ROOT/scripts/explore/explorer-prompt.md" ] \

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

…eflight, scoped Edit, EXPLORE_USER wiring (#399)
- do_down: guard every fallible write with || true so a failed findings.md
write can never abort the EXIT trap before e2e-down.sh / stop_lock_holder
run (was leaking the stack + machine-singleton lock on write failure).
- GEMINI_API_KEY: export unconditionally (matches CI's dummy, #439) instead
of deferring to an ambient real key that can bill the below-seam RAG path.
- SEED_RICH=1 exported unconditionally so an ambient SEED_RICH=0 can't
silently defeat the rich dataset this harness requires.
- preflight: add missing `jq` check (hard dependency of the transcript
pipeline) so a missing jq fails in seconds, not after a full stack boot.
- allowedTools: replace unscoped Write,Edit with Read,Edit(.explore/**) —
Write(...) patterns aren't matched by the CLI's file permission check, so
only a path-scoped Edit rule actually restricts writes to findings.md.
do_up now pre-creates .explore/findings.md so the explorer always has an
existing file for the scoped Edit grant.
- EXPLORE_USER: derive the sapling_user display name from the user id
(case map for the five seeded rich-* users, verified against
db/seed_local_rich.py) instead of hardcoding "Rich Active"; pass
--user "$EXPLORE_USER" to both oracle invocations in do_down.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

All three review findings fixed in a0360ff (do_down teardown/lock-release guards verified with a real write-failure injection; unconditional dummy GEMINI_API_KEY per #439; jq preflight), plus three panel-verified hardening items in the same commit: allowedTools Write dropped / Edit scoped to .explore/**, EXPLORE_USER wired through the name map and both oracle --user calls, unconditional SEED_RICH=1. Scoped re-review confirms all addressed, no new breakage.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(agent): LLM explorer harness driving Playwright against the local stack

1 participant

@AndresL230
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(e2e): Chapter 2 explore harness — make explore + /explore skill (#399) - #444

Merged
AndresL230 merged 9 commits into
mainfrom
e2e-chapter2-explore
Jul 28, 2026
Merged

feat(e2e): Chapter 2 explore harness — make explore + /explore skill (#399)#444
AndresL230 merged 9 commits into
mainfrom
e2e-chapter2-explore

Conversation

@AndresL230

@AndresL230AndresL230 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Chapter 2 of the E2E program (epic #403), PR 2 of 3. Closes#399. Builds on the #400 oracle module (PR #443).

make explore boots the deterministic local stack and hands a Claude Code explorer a real browser; the oracles judge what it saw. Local-only by design — never a CI gate.

What's here

  • scripts/explore.sh (+ make explore / make explore-down, .explore/ gitignored) — the full pipeline: machine-singleton stack lock (detached holder process writing self-identifying lock.ok only after flock succeeds — a failed acquisition touches nothing, proven by an A/B two-session simulation) → e2e-up in function mode with a dummy GEMINI_API_KEY (RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439) → storage state minted via POST /api/auth/test-login through the frontend origin (cookie + sapling_user localStorage, secure flag matching the backend's actual local policy) → claude -p with a pinned @playwright/mcp@0.0.78 config and turn budget, transcript captured via stream-json + jq into a grep-able session.log → oracle final pass appended to findings.md → teardown. up/down subcommands support the interactive flow.
  • scripts/explore/explorer-prompt.md — the mission briefing: student persona (Rich Active), break-things mandate, report-never-fix, oracle cadence (cd backend && venv/bin/python -m e2e_oracles), findings format, known-bugs list, and a "stub it before you dig" rule earned in live testing.
  • .claude/skills/explore/SKILL.md — the interactive /explore skill: same pipeline, watchable and steerable, with a same-origin JS sign-in for live-browser sessions.

Hard-won fixes from live acceptance runs

  • @playwright/mcp without --isolated reuses a persistent profile and silently ignores --storage-state — the run-2 "signed-in" state was a stale-cookie artifact; --isolated fixes it (verified down to the bundled playwright-core source and a cookie-DB probe).
  • claude -p's default text output prints only the final message — a turn-budget exit produced an empty transcript despite real browsing; --output-format stream-json --verbose + jq restores the readable transcript.
  • Lock-holder bookkeeping survives concurrent sessions (busy attempt leaves the running session's files byte-identical).

Acceptance (issue #399 bar, definitive run in the SDD report)

One bounded run (EXPLORE_MAX_TURNS=25, 3m52s): real signed-in navigation across dashboard + library ("Rich Active" nav, 4 seeded documents, CS101/MATH210/BIO110/ENG150 visible in snapshots), 2 explorer-written findings — including a novel wrong-data upload bug and a tutor-resume 500 (concept_describe unregistered in the function-handler seam) independently confirmed by the oracle logscan — plus the harness-appended Oracle final pass re-confirming #355. Machine verified clean afterward (ports, containers, lock).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added an end-to-end exploratory testing workflow for the local application.
    • Added commands to start, stop, and run complete exploration sessions.
    • Exploration sessions now support browser-driven testing, automated checks, session logs, and structured findings.
  • Documentation

    • Added guidance covering exploration steps, reporting requirements, known issues, and session summaries.
  • Chores

    • Exploration artifacts are now excluded from version control.

AndresL230and others added 8 commits July 28, 2026 01:13
…itignore (#399)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e, oracle cadence (#399)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…er GEMINI_API_KEY (#399)
A busy lock previously still tripped do_up's trap/wipe path, tearing down
another session's live stack via scripts/e2e-down.sh and deleting its
.explore/ artifacts before the lock check ever ran. start_lock_holder now
runs first, the do_down safety trap arms only after the lock is held, the
.explore/ wipe (still selective, preserving lock.pid/lock.ok) happens after
that, and a failed acquisition cleans up its own holder/pid files before
exiting. GEMINI_API_KEY now defaults only when unset instead of always
overwriting an operator's real key.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ns (#399)
start_lock_holder previously deleted lock.ok and unconditionally wrote
lock.pid before knowing whether the flock would actually succeed. A failed
attempt from session B (lock held by session A) would delete A's lock.ok and
overwrite A's lock.pid with B's own doomed PID, then B's busy-path cleanup
removed the file entirely — leaving A's later `down` unable to find A's
holder, leaking both the detached process and the machine-singleton lock.
Now the holder subprocess itself is the only writer of lock.ok, and only
ever after its own `flock -n 9` succeeds (atomic temp-file + mv, content =
the holder's own $$). The parent only polls for that self-identifying
marker and touches nothing on disk on the failure path, so a busy lock can
never clobber another session's bookkeeping. lock.pid is retired — lock.ok's
content is now the sole source of truth stop_lock_holder reads.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The first bounded acceptance run (task-8-report.md) produced a
session.log with exactly one line — claude -p's own "Error: Reached
max turns" — because the default --output-format=text only prints the
FINAL message, never the intermediate tool_use/tool_result turns. That
failed the #399 acceptance bar ("session.log shows real Playwright MCP
tool calls").
Switch run_explorer to --output-format stream-json --verbose (the CLI
requires both together) and reformat the JSONL with jq into readable
[assistant]/[tool_use]/[tool_result]/[result] lines, truncated to 500
chars each, so session.log is both grep-able and human-skimmable.
fromjson? tolerates stray non-JSON lines instead of aborting the whole
transcript; claude -p's stderr now goes to its own session.stderr.log
so it can never interleave with (and corrupt) the JSON stream jq
parses.
Verified: a second bounded run (EXPLORE_MAX_TURNS=25) produced a
281-line session.log with 18 mcp__playwright__* tool calls across
/dashboard and /library, plus findings.md with a re-confirmed #430
repro and the oracle final pass re-confirming #355.
The follow-up bounded-run diagnosis found a contradiction: run 2's
session cookie was accepted (/api/users 200) but the UI acted fully
signed-out (Sign In button, dashboard skeleton) — the #430 symptom.
Run 1's "Secure cookie dropped over http" diagnosis didn't hold up
either, since Chromium does accept Secure cookies on http://localhost.
Root-caused by reading @playwright/mcp@0.0.78's bundled source
(playwright-core/lib/coreBundle.js): without --isolated, the MCP
server launches ONE PERSISTENT Chrome profile keyed only by
sha256(cwd) — reused across every explore.sh run from this checkout,
never wiped by teardown — and its client factory does
`config.browser.isolated ? await browser.newContext(contextOptions) :
browser.contexts()[0]`. In the default (non-isolated) branch it just
grabs the already-open persistent context and never calls newContext
with our --storage-state at all.
Verified empirically: wiped the profile dir, booted a clean stack, and
drove a 3-turn claude -p probe against the (then-current) mcp.json —
navigating to /dashboard redirected straight to a REAL
accounts.google.com sign-in page, and sqlite3 on the profile's Cookies
db afterward showed zero sapling_session rows (neither cookie nor
localStorage from storageState.json was ever applied). The identical
probe with --isolated added rendered the real, fully-authenticated
dashboard on the first navigation, with localStorage.sapling_user
correctly present — this is the same mechanism (ephemeral
browser.newContext(contextOptions)) @playwright/test itself uses in
Chapter 1's global-setup.ts.
Also corrected mint_storage_state's cookie to secure:false, matching
what the backend's own SECURE_COOKIES policy actually issues for the
http://localhost local stack (config.py derives it from FRONTEND_URL's
scheme) — the file should mirror reality regardless of which flag
turned out to be the load-bearing one.
Verified: EXPLORE_MAX_TURNS=12 make explore reached a signed-in
dashboard (nav shows "Rich Active" / "Account", full authenticated
menu) using only 2 real Playwright tool calls, zero sign-in recovery
turns.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The definitive acceptance run (harness fixes verified: real auth via
--isolated, breadth across 2+ surfaces) hit a genuine new bug —
resuming a tutor session 500'd on POST /api/graph/.../concept-description,
root-caused via the explorer's own backend-log investigation to a
missing SAPLING_FUNCTION_HANDLERS registration for 'concept_describe'
(caught independently by the oracle's logscan pass too, so it's not
lost, just not explorer-authored). But the explorer spent its
remaining turns tailing logs and checking processes to nail the exact
LookupError, and ran out of budget before writing an F<N> entry to
findings.md — a real gap against #399's "readable transcript AND
findings file" bar, distinct from any flag/mechanism defect.
Add a "stub it before you dig" ground rule: write a one-line stub
finding the instant something looks off, before further root-causing.
A written stub survives a turn-budget cutoff; a perfect unwritten
diagnosis does not.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:34 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: dc0a75e3-31f4-42be-8ec2-bb9a848919bb

📥 Commits

Reviewing files that changed from the base of the PR and between 914b0ae and a0360ff.

📒 Files selected for processing (1)
  • scripts/explore.sh
📝 Walkthrough

Walkthrough

Adds a locked local exploratory-testing workflow that boots the E2E stack, authenticates a seeded browser session, runs Claude through Playwright MCP, records findings and oracle output, and reliably tears down the stack.

Changes

Exploration harness

Layer / File(s)Summary
Exploration contract and entrypoints
.claude/skills/explore/SKILL.md, scripts/explore/explorer-prompt.md, Makefile, .gitignore
Defines the exploration workflow, reporting constraints, findings format, operator targets, and ignored .explore/ artifacts.
Locked stack setup
scripts/explore.sh
Adds prerequisite checks, repository configuration, detached flock locking, and local E2E stack startup.
Authenticated browser session
scripts/explore.sh
Creates seeded-user Playwright storage state, writes the pinned MCP configuration, and runs Claude with structured logs and separate stderr output.
Oracle and teardown lifecycle
scripts/explore.sh
Supports full, up, and down modes, appends final oracle results, tears down the stack, and releases the lock.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
participant Operator
participant explore.sh
participant E2EStack
participant AuthTestEndpoint
participant PlaywrightMCP
participant Browser
participant Claude
Operator->>explore.sh: start exploration
explore.sh->>E2EStack: boot local E2E stack
explore.sh->>AuthTestEndpoint: POST seeded student login
explore.sh->>PlaywrightMCP: configure storage state and MCP
Claude->>PlaywrightMCP: execute exploration tools
PlaywrightMCP->>Browser: drive localhost browser
explore.sh->>E2EStack: run oracle and teardown
Loading

Possibly related issues

  • Issue 403 — Covers the broader Chapter 2 AI Agent Explorer epic implemented by this harness.
  • Issue 401 — Proposes nightly automation and regression promotion for findings produced by this exploration workflow.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly names the new explore harness and /explore skill and matches the main change.
Description check✅ PassedThe description covers summary, changes, related issue, testing, screenshots, and reviewer notes with mostly complete content.
Linked Issues check✅ PassedThe PR implements the local Playwright explorer harness, seeded auth, hard budget/localhost limits, structured logs, and readable session output required by #399.
Out of Scope Changes check✅ PassedThe changes stay focused on the explore harness, prompt, Makefile, and ignore rules; no unrelated functionality is introduced.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch e2e-chapter2-explore

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Jul 28, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-staginga0360ffCommit Preview URL

Branch Preview URL
Jul 28 2026, 10:20 AM

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
scripts/explore.sh (1)

149-154: 🩺 Stability & Availability | 🔵 Trivial

stop_lock_holder/do_down release the machine lock without verifying the caller owns it.

do_down (and thus scripts/explore.sh down / make explore-down) unconditionally reads lock.ok's pid and kills it, regardless of whether the invoking shell is the one that acquired it. The elaborate ownership-safety work in start_lock_holder (a failed attempt never touches lock.ok) isn't mirrored here: since the lock is uid-namespaced (/tmp/claude-$(id -u)/...), the realistic blast radius is a single operator accidentally running down from a second terminal while up is still driving an interactive session elsewhere on the same machine — which would yank the shared local stack out from under that live session with no warning. Given the doc explicitly anticipates the "interactive flow" (separate up/down invocations) as a supported mode, this seems worth at least a guard (e.g. warn if a stack that looks actively in-use by claude is being torn down) or an explicit doc callout of the risk.

Also applies to: 404-419

🤖 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/explore.sh` around lines 149 - 154, Update stop_lock_holder and the
do_down flow to guard against tearing down a lock owned by another active
interactive session. Before killing the PID from lock.ok, detect whether it
represents an active claude-managed stack and warn or require explicit
confirmation when ownership cannot be established; preserve cleanup for the
invoking owner and document the supported separate up/down behavior if that is
the chosen safeguard.
🤖 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 `@scripts/explore.sh`:
- Around line 176-202: Update the localStorage payload in mint_storage_state so
the sapling_user name is derived from $EXPLORE_USER instead of hardcoding "Rich
Active". Preserve the existing user ID and other payload fields while ensuring
any configured seeded rich-* user receives a matching cached display name.
- Around line 302-337: Restrict the Write/Edit entries in run_explorer() to the
permitted .explore findings path, using path-scoped rules such as
Edit(.explore/**/*.md) and removing blanket file-edit permissions. Also verify
the current Claude CLI Bash permission syntax for the cd backend &&
venv/bin/python -m e2e_oracles:* rule, tightening it if compound-command parsing
permits commands outside the intended prefix.
- Around line 246-261: Update write_mcp_config to pass Playwright MCP
localhost-only origin restrictions in the generated args, using the required
localhost/127.0.0.1 allowlist or equivalent blocked-origins plus allowlist
configuration. Document alongside the configuration that these flags are
defense-in-depth only and do not prevent redirects from reaching off-localhost
destinations.
---
Nitpick comments:
In `@scripts/explore.sh`:
- Around line 149-154: Update stop_lock_holder and the do_down flow to guard
against tearing down a lock owned by another active interactive session. Before
killing the PID from lock.ok, detect whether it represents an active
claude-managed stack and warn or require explicit confirmation when ownership
cannot be established; preserve cleanup for the invoking owner and document the
supported separate up/down behavior if that is the chosen safeguard.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 762bcd62-0ca2-4e80-bacf-4434944c876d

📥 Commits

Reviewing files that changed from the base of the PR and between db0ecad and 914b0ae.

📒 Files selected for processing (5)
  • .claude/skills/explore/SKILL.md
  • .gitignore
  • Makefile
  • scripts/explore.sh
  • scripts/explore/explorer-prompt.md

Comment threadscripts/explore.sh
Comment threadscripts/explore.sh
Comment on lines +246 to +261
write_mcp_config() {
local headless_args='"--headless", '
[ "$EXPLORE_HEADED" = "1" ] && headless_args=''
cat > "$EXPLORE_DIR/mcp.json" <<EOF
{
"mcpServers": {
"playwright": {
"command": "npx",
"args": ["-y", "@playwright/mcp@$PLAYWRIGHT_MCP_VERSION", "--isolated", ${headless_args}"--browser", "chromium",
"--storage-state", "$EXPLORE_DIR/storageState.json",
"--output-dir", "$EXPLORE_DIR/traces", "--save-session"]
}
}
}
EOF
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== locate files =="
fd -a 'explore.sh|explorer-prompt.md|package.json|pnpm-lock.yaml|package-lock.json|yarn.lock|playwright|mcp.json'.| sed 's#^\./##'| head -200
echoecho"== relevant explore.sh lines =="if [ -f scripts/explore.sh ];then
nl -ba scripts/explore.sh | sed -n '220,280p'fiechoecho"== relevant explorer-prompt lines =="if [ -f scripts/explore/explorer-prompt.md ];then
nl -ba scripts/explore/explorer-prompt.md | sed -n '1,35p'fiif [ -f explorer-prompt.md ];then
nl -ba explorer-prompt.md | sed -n '1,35p'fiechoecho"== search required navigation text and mcp config args =="
rg -n "Restrict navigation|localhost|localhost-only|localhost:3000|allowed-origins|blocked-origins|explorer-prompt|MISSION|write_mcp_config|PLAYWRIGHT_MCP_VERSION" -S .

Repository: SaplingLearn/Sapling

Length of output: 479


🌐 Web query:

Playwright MCP 0.0.78 README --allowed-origins --blocked-origins

💡 Result:

In Playwright MCP, the --allowed-origins and --blocked-origins flags (and their corresponding environment variables) are used to manage network access for the browser instance [1][2][3]. Both settings accept a semicolon-separated list of origins [1][4][3]. Key details: --allowed-origins This flag specifies a list of trusted origins that the browser is permitted to request [1][5]. By default, all origins are allowed [1][2]. Environment variable: PLAYWRIGHT_MCP_ALLOWED_ORIGINS [1][3]. --blocked-origins This flag specifies a list of origins to block the browser from requesting [1][5]. The blocklist is evaluated before the allowlist; if an origin is present in both, it will be blocked [1][2][5]. Environment variable: PLAYWRIGHT_MCP_BLOCKED_ORIGINS [1][3]. Important Security Note: Documentation explicitly states that these flags do not serve as a security boundary and do not affect redirects [1][5][3]. Supported Formats: Both flags support the following formats [1][2]: Full origin: e.g., https://example.com:8080 (matches only that specific origin) [1][2]. Wildcard port: e.g., http://localhost:* (matches any port on localhost with the http protocol) [1][2].

Citations:


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== relevant explore.sh lines =="if [ -f scripts/explore.sh ];then
sed -n '220,280p' scripts/explore.sh | cat -n
fiechoecho"== relevant explorer-prompt lines =="if [ -f scripts/explore/explorer-prompt.md ];then
sed -n '1,40p' scripts/explore/explorer-prompt.md | cat -n
fiechoecho"== search required navigation text and mcp config args =="
rg -n "Restrict navigation|localhost|localhost-only|localhost:3000|allowed-origins|blocked-origins|explorer-prompt|MISSION|write_mcp_config|PLAYWRIGHT_MCP_VERSION" -S .echoecho"== lockfile version snippets =="forfin package-lock.json frontend/package-lock.json;doif [ -f"$f" ];thenecho"--- $f ---"
rg -n '"`@playwright/mcp`"|"`@playwright/test`"|PLAYWRIGHT_MCP_VERSION'"$f" -A 3 -B 2 ||truefidone

Repository: SaplingLearn/Sapling

Length of output: 22370


🌐 Web query:

playwright mcp --allowed-origins blocked navigation error network error

💡 Result:

The "blocked navigation error" or network-related errors when using the --allowed-origins flag in Playwright MCP typically stem from configuration format issues, regex/glob matching errors, or confusing the flag with server-side CORS settings [1][2][3]. Key troubleshooting steps include: 1. Use Correct Origin Format: Ensure you are passing the domain or origin directly (e.g., google.com) rather than including protocol prefixes like https:// or trailing slashes, which have historically caused parsing issues [1]. 2. Avoid Invalid Glob Patterns: Recent versions (as of early 2026) have had bugs where wildcard port patterns like http://localhost:* generate invalid internal glob patterns, causing the browser to block all traffic to those destinations [3]. If you are experiencing this, you may need to use explicit hostnames or check for updates addressing the originOrHostGlob function [3]. 3. Distinguish Between Flags: Ensure you are not confusing --allowed-origins (which controls browser navigation within the sandboxed Playwright instance) with --allowed-hosts (which controls MCP server CORS/connectivity) [2]. Both may need to be set identically if you are navigating to localhost in restricted environments like GitHub Actions or WSL [2][4]. 4. Blocklist Precedence: Keep in mind that Playwright MCP evaluates the blocked-origins list before the allowed-origins list. If a URL matches both, it will be blocked [5]. 5. Debugging Network Issues: If navigation still fails with ERR_CONNECTION_REFUSED, verify that the target service (e.g., a local dev server) is listening on 0.0.0.0 rather than just 127.0.0.1 to ensure it is accessible to the browser process, especially if running within containers (Docker) or WSL [2][4]. For further debugging, you can enable verbose logging by setting the DEBUG environment variable to pw:api or pw:browser* to see exactly which origin is failing to match the navigation request [4].

Citations:


🌐 Web query:

Playwright MCP blocked origins error "Failed to navigate to"

💡 Result:

The error net::ERR_BLOCKED_BY_CLIENT when using Playwright MCP typically indicates that the browser navigation was blocked by a client-side security policy, configuration, or environment restriction [1]. Because Playwright MCP is often used in automated contexts (such as LLM agents), this error is usually triggered by one of the following factors [1]: 1. Allowed Origins Configuration: If you have configured allowed-origins for security, the destination URL may be excluded from the allowlist [2][3]. Verify that the PW_ALLOWED_ORIGINS environment variable or the --allowed-origins CLI flag is correctly set to include the domain you are attempting to visit [2][3]. 2. Browser Environment and Sandbox Restrictions: When running in containerized environments like Docker, Chromium often fails to navigate because it cannot access the necessary system sandbox [1]. To resolve this, you may need to pass explicit launch flags to bypass sandbox restrictions in your PLAYWRIGHT_LAUNCH_OPTIONS [1]: --no-sandbox --disable-setuid-sandbox --disable-gpu 3. Security Settings or Extensions: Although Playwright MCP sessions are typically isolated, ERR_BLOCKED_BY_CLIENT can also occur if the browser launch arguments include restrictive security settings or if there are conflicts with other security-related flags [1]. 4. Troubleshooting Steps: - Check the logs to confirm the specific URL being blocked [1]. - If you are running in Docker, ensure the container has the necessary privileges and sandbox bypass flags [1]. - Review your MCP configuration file (often found in .mcp/config.json or equivalent) to ensure --allowed-origins is formatted correctly (e.g., comma-separated or semicolon-separated depending on your specific implementation) [4][2][3]. - If you suspect the issue is not related to origins, verify that npx and the Playwright runtime are correctly installed and accessible in your environment, as missing dependencies can sometimes lead to unexpected browser behavior [5]. If the error persists despite correct origin configuration, it is often helpful to test a basic navigation command outside of the MCP wrapper to rule out environment-specific sandbox or policy issues [1].

Citations:


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== package manifests mentioning playwright mcp =="
rg -n '"`@playwright/mcp`"|"playwright-mcp"|PLAYWRIGHT_MCP_VERSION' -S --glob '!node_modules/**' --glob '!frontend/node_modules/**' frontend package.json scripts
echoecho"== scripts/explore.sh version assignment and mcp config callers =="
nl -ba scripts/explore.sh | sed -n '60,80p'
nl -ba scripts/explore.sh | sed -n '370,390p'echoecho"== local prompt copy =="
cat -n scripts/explore/explorer-prompt.md | sed -n '1,20p'

Repository: SaplingLearn/Sapling

Length of output: 587


🌐 Web query:

site:github.com/microsoft/playwright-mcp "allowedOrigins" "blockedByClient" "ERR_BLOCKED_BY_CLIENT"

💡 Result:

The error ERR_BLOCKED_BY_CLIENT in the context of Playwright MCP (Model Context Protocol) is a browser-level error that occurs when the browser is instructed to navigate to or request a resource that has been explicitly blocked or is unreachable due to configuration issues [1][2]. In the specific context of the Playwright MCP server, this error frequently arises from the following scenarios: 1. Origin Filtering: If you have configured --allowed-origins or --blocked-origins, the browser will block navigation to any origin not in the allowlist or present in the blocklist [1]. Note that these flags affect navigation but do not serve as a CORS security boundary [1][3]. 2. Extension ID Mismatch: A common cause for ERR_BLOCKED_BY_CLIENT in recent versions (e.g., v0.0.66–0.0.68) is a hardcoded Chrome extension ID in the MCP server's connection logic [2]. If you are using an unpacked extension, a locally built version, or a different browser channel, your extension will have a different ID than the one hardcoded (mmlmfjhmonkocbjadbfplnigmagldckm). This causes the browser to attempt to connect to a non-existent extension, resulting in the error [2]. 3. Configuration Bugs: Bugs in how origin patterns are parsed—such as incorrect handling of wildcard ports (e.g., http://localhost:*)—can lead to requests being unexpectedly blocked [4]. To resolve or debug this error: - Verify your --allowed-origins and --blocked-origins configurations to ensure they match the intended URLs [1][4]. - If using an unpacked browser extension, ensure the extension ID is handled correctly, potentially by setting the PLAYWRIGHT_MCP_EXTENSION_ID environment variable or pinning your extension's ID via the manifest's "key" property [2]. - Check the official issue tracker for your version of Playwright MCP, as specific releases have included fixes for origin pattern parsing and connection logic [4][2][5].

Citations:


🌐 Web query:

github.com/microsoft/playwright-mcp program.ts "allowed-origins" "allowedOrigins" "blockedByClient"

💡 Result:

In the Playwright MCP server, allowedOrigins and blockedOrigins are configuration options used to control which network requests the browser is permitted to make [1][2]. These settings are managed within the server's network configuration and are implemented via Playwright's routing capabilities [3][4]. When these rules are active, the server intercepts network requests and uses the 'blockedbyclient' error code to terminate requests that do not align with the defined allow/block lists [3][4]. Specifically, the implementation follows this logic: 1. If allowedOrigins are defined, the server first blocks all requests by default using route.abort('blockedbyclient') [3][5]. 2. It then explicitly allows requests to the specified origins by calling route.continue for patterns matching the allowed list [3][4]. 3. If blockedOrigins are defined, the server explicitly blocks requests to those origins using route.abort('blockedbyclient') [3][4]. Origins matching both allowedOrigins and blockedOrigins will be blocked [1][2]. The allowed-origins command-line argument accepts a semicolon-separated list of origins, and the configuration supports full origin strings (e.g., https://example.com:8080) and wildcard port patterns (e.g., http://localhost:*) [1][6]. These settings are considered convenience features for managing browser interactions rather than strict security boundaries [1][2]. The file program.ts within the Playwright repository is involved in managing the command-line interface and tool configuration for the MCP server [7].

Citations:


Reinforce the localhost-first prompt with a technical restriction.

This harness requires localhost-only navigation, but mcp.json relies only on scripts/explore/explorer-prompt.md:13 for enforcement. Add a Playwright MCP restriction such as --allowed-origins http://localhost:3000;http://localhost:5000;127.0.0.1:5000 or --blocked-origins * plus the localhost allowlist, while documenting that these flags are not a security boundary against redirects; this gives defense-in-depth against model-driven or page-driven off-localhost clicks.

🤖 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/explore.sh` around lines 246 - 261, Update write_mcp_config to pass
Playwright MCP localhost-only origin restrictions in the generated args, using
the required localhost/127.0.0.1 allowlist or equivalent blocked-origins plus
allowlist configuration. Document alongside the configuration that these flags
are defense-in-depth only and do not prevent redirects from reaching
off-localhost destinations.

Comment threadscripts/explore.sh
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

Code review

Found 3 issues:

  1. do_down() can leak the stack and the machine-singleton lock: it runs under set -euo pipefail and fires as an EXIT trap, but its mkdir -p, the findings-file bootstrap, and the { … } >> findings.md append block have no || true guards (unlike the oracle subshell and e2e-down.sh lines around them). A failed write there (disk full, read-only fs) aborts the trap before e2e-down.sh and stop_lock_holder run — verified bash semantics: a failing command inside an EXIT trap under set -e stops the trap. The header comment's "|| true guards throughout" claim is currently false for these lines.

do_down() {
mkdir -p "$EXPLORE_DIR"
(
cd"$REPO_ROOT/backend"
venv/bin/python -m e2e_oracles --json >"$EXPLORE_DIR/oracle-final.json"||true
venv/bin/python -m e2e_oracles >"$EXPLORE_DIR/oracle-final.txt"2>&1||true
) ||true
[ -f"$EXPLORE_DIR/findings.md" ] ||printf'# Exploration findings\n'>"$EXPLORE_DIR/findings.md"
{
printf'\n## Oracle final pass (%s)\n\n```\n'"$(date -Iseconds)"
cat "$EXPLORE_DIR/oracle-final.txt"2>/dev/null ||true
printf'```\n'
} >>"$EXPLORE_DIR/findings.md"
"$REPO_ROOT/scripts/e2e-down.sh"||true
stop_lock_holder
}

  1. Dummy-key mitigation for RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439 defeats itself in the common developer environment: export GEMINI_API_KEY="${GEMINI_API_KEY:-e2e-dummy-key-no-billing}" defers to an ambient real key, and developers here routinely keep one exported (validate_config fail-closes without it). The below-seam RAG/embedding path (RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439) bills live even in function mode, and the explorer's mission (uploads, quiz, tutor turns, many turns) is worst-case volume for it. CI applies the dummy key unconditionally for exactly this reason; export inside the script can't touch the operator's parent shell, so the conditional protects nothing.

# Default only if unset — never clobber an operator's real key.
export GEMINI_API_KEY="${GEMINI_API_KEY:-e2e-dummy-key-no-billing}"
"$REPO_ROOT/scripts/e2e-up.sh"|| die "scripts/e2e-up.sh failed — see .e2e/*.log"

  1. jq is a hard dependency of the transcript pipeline but is missing from preflight (claude/npx/curl/python3 are checked). Without it the script boots the entire stack — Supabase, migrations, a production Next build, minutes of work — and only then fails in run_explorer, instead of failing fast like the other four checks.

echo"▶ Preflight…"
command -v claude >/dev/null 2>&1|| die "claude CLI not found on PATH — install it first"
command -v npx >/dev/null 2>&1|| die "npx not found on PATH — install Node.js first"
command -v curl >/dev/null 2>&1|| die "curl not found on PATH"
command -v python3 >/dev/null 2>&1|| die "python3 not found on PATH"
[ -f"$REPO_ROOT/scripts/explore/explorer-prompt.md" ] \

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

…eflight, scoped Edit, EXPLORE_USER wiring (#399)
- do_down: guard every fallible write with || true so a failed findings.md
write can never abort the EXIT trap before e2e-down.sh / stop_lock_holder
run (was leaking the stack + machine-singleton lock on write failure).
- GEMINI_API_KEY: export unconditionally (matches CI's dummy, #439) instead
of deferring to an ambient real key that can bill the below-seam RAG path.
- SEED_RICH=1 exported unconditionally so an ambient SEED_RICH=0 can't
silently defeat the rich dataset this harness requires.
- preflight: add missing `jq` check (hard dependency of the transcript
pipeline) so a missing jq fails in seconds, not after a full stack boot.
- allowedTools: replace unscoped Write,Edit with Read,Edit(.explore/**) —
Write(...) patterns aren't matched by the CLI's file permission check, so
only a path-scoped Edit rule actually restricts writes to findings.md.
do_up now pre-creates .explore/findings.md so the explorer always has an
existing file for the scoped Edit grant.
- EXPLORE_USER: derive the sapling_user display name from the user id
(case map for the five seeded rich-* users, verified against
db/seed_local_rich.py) instead of hardcoding "Rich Active"; pass
--user "$EXPLORE_USER" to both oracle invocations in do_down.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

All three review findings fixed in a0360ff (do_down teardown/lock-release guards verified with a real write-failure injection; unconditional dummy GEMINI_API_KEY per #439; jq preflight), plus three panel-verified hardening items in the same commit: allowedTools Write dropped / Edit scoped to .explore/**, EXPLORE_USER wired through the name map and both oracle --user calls, unconditional SEED_RICH=1. Scoped re-review confirms all addressed, no new breakage.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(agent): LLM explorer harness driving Playwright against the local stack

1 participant

@AndresL230
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(e2e): Chapter 2 explore harness — make explore + /explore skill (#399) - #444

Merged
AndresL230 merged 9 commits into
mainfrom
e2e-chapter2-explore
Jul 28, 2026
Merged

feat(e2e): Chapter 2 explore harness — make explore + /explore skill (#399)#444
AndresL230 merged 9 commits into
mainfrom
e2e-chapter2-explore

Conversation

@AndresL230

@AndresL230AndresL230 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Chapter 2 of the E2E program (epic #403), PR 2 of 3. Closes#399. Builds on the #400 oracle module (PR #443).

make explore boots the deterministic local stack and hands a Claude Code explorer a real browser; the oracles judge what it saw. Local-only by design — never a CI gate.

What's here

  • scripts/explore.sh (+ make explore / make explore-down, .explore/ gitignored) — the full pipeline: machine-singleton stack lock (detached holder process writing self-identifying lock.ok only after flock succeeds — a failed acquisition touches nothing, proven by an A/B two-session simulation) → e2e-up in function mode with a dummy GEMINI_API_KEY (RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439) → storage state minted via POST /api/auth/test-login through the frontend origin (cookie + sapling_user localStorage, secure flag matching the backend's actual local policy) → claude -p with a pinned @playwright/mcp@0.0.78 config and turn budget, transcript captured via stream-json + jq into a grep-able session.log → oracle final pass appended to findings.md → teardown. up/down subcommands support the interactive flow.
  • scripts/explore/explorer-prompt.md — the mission briefing: student persona (Rich Active), break-things mandate, report-never-fix, oracle cadence (cd backend && venv/bin/python -m e2e_oracles), findings format, known-bugs list, and a "stub it before you dig" rule earned in live testing.
  • .claude/skills/explore/SKILL.md — the interactive /explore skill: same pipeline, watchable and steerable, with a same-origin JS sign-in for live-browser sessions.

Hard-won fixes from live acceptance runs

  • @playwright/mcp without --isolated reuses a persistent profile and silently ignores --storage-state — the run-2 "signed-in" state was a stale-cookie artifact; --isolated fixes it (verified down to the bundled playwright-core source and a cookie-DB probe).
  • claude -p's default text output prints only the final message — a turn-budget exit produced an empty transcript despite real browsing; --output-format stream-json --verbose + jq restores the readable transcript.
  • Lock-holder bookkeeping survives concurrent sessions (busy attempt leaves the running session's files byte-identical).

Acceptance (issue #399 bar, definitive run in the SDD report)

One bounded run (EXPLORE_MAX_TURNS=25, 3m52s): real signed-in navigation across dashboard + library ("Rich Active" nav, 4 seeded documents, CS101/MATH210/BIO110/ENG150 visible in snapshots), 2 explorer-written findings — including a novel wrong-data upload bug and a tutor-resume 500 (concept_describe unregistered in the function-handler seam) independently confirmed by the oracle logscan — plus the harness-appended Oracle final pass re-confirming #355. Machine verified clean afterward (ports, containers, lock).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added an end-to-end exploratory testing workflow for the local application.
    • Added commands to start, stop, and run complete exploration sessions.
    • Exploration sessions now support browser-driven testing, automated checks, session logs, and structured findings.
  • Documentation

    • Added guidance covering exploration steps, reporting requirements, known issues, and session summaries.
  • Chores

    • Exploration artifacts are now excluded from version control.

AndresL230and others added 8 commits July 28, 2026 01:13
…itignore (#399)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e, oracle cadence (#399)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…er GEMINI_API_KEY (#399)
A busy lock previously still tripped do_up's trap/wipe path, tearing down
another session's live stack via scripts/e2e-down.sh and deleting its
.explore/ artifacts before the lock check ever ran. start_lock_holder now
runs first, the do_down safety trap arms only after the lock is held, the
.explore/ wipe (still selective, preserving lock.pid/lock.ok) happens after
that, and a failed acquisition cleans up its own holder/pid files before
exiting. GEMINI_API_KEY now defaults only when unset instead of always
overwriting an operator's real key.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ns (#399)
start_lock_holder previously deleted lock.ok and unconditionally wrote
lock.pid before knowing whether the flock would actually succeed. A failed
attempt from session B (lock held by session A) would delete A's lock.ok and
overwrite A's lock.pid with B's own doomed PID, then B's busy-path cleanup
removed the file entirely — leaving A's later `down` unable to find A's
holder, leaking both the detached process and the machine-singleton lock.
Now the holder subprocess itself is the only writer of lock.ok, and only
ever after its own `flock -n 9` succeeds (atomic temp-file + mv, content =
the holder's own $$). The parent only polls for that self-identifying
marker and touches nothing on disk on the failure path, so a busy lock can
never clobber another session's bookkeeping. lock.pid is retired — lock.ok's
content is now the sole source of truth stop_lock_holder reads.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The first bounded acceptance run (task-8-report.md) produced a
session.log with exactly one line — claude -p's own "Error: Reached
max turns" — because the default --output-format=text only prints the
FINAL message, never the intermediate tool_use/tool_result turns. That
failed the #399 acceptance bar ("session.log shows real Playwright MCP
tool calls").
Switch run_explorer to --output-format stream-json --verbose (the CLI
requires both together) and reformat the JSONL with jq into readable
[assistant]/[tool_use]/[tool_result]/[result] lines, truncated to 500
chars each, so session.log is both grep-able and human-skimmable.
fromjson? tolerates stray non-JSON lines instead of aborting the whole
transcript; claude -p's stderr now goes to its own session.stderr.log
so it can never interleave with (and corrupt) the JSON stream jq
parses.
Verified: a second bounded run (EXPLORE_MAX_TURNS=25) produced a
281-line session.log with 18 mcp__playwright__* tool calls across
/dashboard and /library, plus findings.md with a re-confirmed #430
repro and the oracle final pass re-confirming #355.
The follow-up bounded-run diagnosis found a contradiction: run 2's
session cookie was accepted (/api/users 200) but the UI acted fully
signed-out (Sign In button, dashboard skeleton) — the #430 symptom.
Run 1's "Secure cookie dropped over http" diagnosis didn't hold up
either, since Chromium does accept Secure cookies on http://localhost.
Root-caused by reading @playwright/mcp@0.0.78's bundled source
(playwright-core/lib/coreBundle.js): without --isolated, the MCP
server launches ONE PERSISTENT Chrome profile keyed only by
sha256(cwd) — reused across every explore.sh run from this checkout,
never wiped by teardown — and its client factory does
`config.browser.isolated ? await browser.newContext(contextOptions) :
browser.contexts()[0]`. In the default (non-isolated) branch it just
grabs the already-open persistent context and never calls newContext
with our --storage-state at all.
Verified empirically: wiped the profile dir, booted a clean stack, and
drove a 3-turn claude -p probe against the (then-current) mcp.json —
navigating to /dashboard redirected straight to a REAL
accounts.google.com sign-in page, and sqlite3 on the profile's Cookies
db afterward showed zero sapling_session rows (neither cookie nor
localStorage from storageState.json was ever applied). The identical
probe with --isolated added rendered the real, fully-authenticated
dashboard on the first navigation, with localStorage.sapling_user
correctly present — this is the same mechanism (ephemeral
browser.newContext(contextOptions)) @playwright/test itself uses in
Chapter 1's global-setup.ts.
Also corrected mint_storage_state's cookie to secure:false, matching
what the backend's own SECURE_COOKIES policy actually issues for the
http://localhost local stack (config.py derives it from FRONTEND_URL's
scheme) — the file should mirror reality regardless of which flag
turned out to be the load-bearing one.
Verified: EXPLORE_MAX_TURNS=12 make explore reached a signed-in
dashboard (nav shows "Rich Active" / "Account", full authenticated
menu) using only 2 real Playwright tool calls, zero sign-in recovery
turns.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The definitive acceptance run (harness fixes verified: real auth via
--isolated, breadth across 2+ surfaces) hit a genuine new bug —
resuming a tutor session 500'd on POST /api/graph/.../concept-description,
root-caused via the explorer's own backend-log investigation to a
missing SAPLING_FUNCTION_HANDLERS registration for 'concept_describe'
(caught independently by the oracle's logscan pass too, so it's not
lost, just not explorer-authored). But the explorer spent its
remaining turns tailing logs and checking processes to nail the exact
LookupError, and ran out of budget before writing an F<N> entry to
findings.md — a real gap against #399's "readable transcript AND
findings file" bar, distinct from any flag/mechanism defect.
Add a "stub it before you dig" ground rule: write a one-line stub
finding the instant something looks off, before further root-causing.
A written stub survives a turn-budget cutoff; a perfect unwritten
diagnosis does not.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:34 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: dc0a75e3-31f4-42be-8ec2-bb9a848919bb

📥 Commits

Reviewing files that changed from the base of the PR and between 914b0ae and a0360ff.

📒 Files selected for processing (1)
  • scripts/explore.sh
📝 Walkthrough

Walkthrough

Adds a locked local exploratory-testing workflow that boots the E2E stack, authenticates a seeded browser session, runs Claude through Playwright MCP, records findings and oracle output, and reliably tears down the stack.

Changes

Exploration harness

Layer / File(s)Summary
Exploration contract and entrypoints
.claude/skills/explore/SKILL.md, scripts/explore/explorer-prompt.md, Makefile, .gitignore
Defines the exploration workflow, reporting constraints, findings format, operator targets, and ignored .explore/ artifacts.
Locked stack setup
scripts/explore.sh
Adds prerequisite checks, repository configuration, detached flock locking, and local E2E stack startup.
Authenticated browser session
scripts/explore.sh
Creates seeded-user Playwright storage state, writes the pinned MCP configuration, and runs Claude with structured logs and separate stderr output.
Oracle and teardown lifecycle
scripts/explore.sh
Supports full, up, and down modes, appends final oracle results, tears down the stack, and releases the lock.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
participant Operator
participant explore.sh
participant E2EStack
participant AuthTestEndpoint
participant PlaywrightMCP
participant Browser
participant Claude
Operator->>explore.sh: start exploration
explore.sh->>E2EStack: boot local E2E stack
explore.sh->>AuthTestEndpoint: POST seeded student login
explore.sh->>PlaywrightMCP: configure storage state and MCP
Claude->>PlaywrightMCP: execute exploration tools
PlaywrightMCP->>Browser: drive localhost browser
explore.sh->>E2EStack: run oracle and teardown
Loading

Possibly related issues

  • Issue 403 — Covers the broader Chapter 2 AI Agent Explorer epic implemented by this harness.
  • Issue 401 — Proposes nightly automation and regression promotion for findings produced by this exploration workflow.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly names the new explore harness and /explore skill and matches the main change.
Description check✅ PassedThe description covers summary, changes, related issue, testing, screenshots, and reviewer notes with mostly complete content.
Linked Issues check✅ PassedThe PR implements the local Playwright explorer harness, seeded auth, hard budget/localhost limits, structured logs, and readable session output required by #399.
Out of Scope Changes check✅ PassedThe changes stay focused on the explore harness, prompt, Makefile, and ignore rules; no unrelated functionality is introduced.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch e2e-chapter2-explore

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Jul 28, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-staginga0360ffCommit Preview URL

Branch Preview URL
Jul 28 2026, 10:20 AM

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
scripts/explore.sh (1)

149-154: 🩺 Stability & Availability | 🔵 Trivial

stop_lock_holder/do_down release the machine lock without verifying the caller owns it.

do_down (and thus scripts/explore.sh down / make explore-down) unconditionally reads lock.ok's pid and kills it, regardless of whether the invoking shell is the one that acquired it. The elaborate ownership-safety work in start_lock_holder (a failed attempt never touches lock.ok) isn't mirrored here: since the lock is uid-namespaced (/tmp/claude-$(id -u)/...), the realistic blast radius is a single operator accidentally running down from a second terminal while up is still driving an interactive session elsewhere on the same machine — which would yank the shared local stack out from under that live session with no warning. Given the doc explicitly anticipates the "interactive flow" (separate up/down invocations) as a supported mode, this seems worth at least a guard (e.g. warn if a stack that looks actively in-use by claude is being torn down) or an explicit doc callout of the risk.

Also applies to: 404-419

🤖 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/explore.sh` around lines 149 - 154, Update stop_lock_holder and the
do_down flow to guard against tearing down a lock owned by another active
interactive session. Before killing the PID from lock.ok, detect whether it
represents an active claude-managed stack and warn or require explicit
confirmation when ownership cannot be established; preserve cleanup for the
invoking owner and document the supported separate up/down behavior if that is
the chosen safeguard.
🤖 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 `@scripts/explore.sh`:
- Around line 176-202: Update the localStorage payload in mint_storage_state so
the sapling_user name is derived from $EXPLORE_USER instead of hardcoding "Rich
Active". Preserve the existing user ID and other payload fields while ensuring
any configured seeded rich-* user receives a matching cached display name.
- Around line 302-337: Restrict the Write/Edit entries in run_explorer() to the
permitted .explore findings path, using path-scoped rules such as
Edit(.explore/**/*.md) and removing blanket file-edit permissions. Also verify
the current Claude CLI Bash permission syntax for the cd backend &&
venv/bin/python -m e2e_oracles:* rule, tightening it if compound-command parsing
permits commands outside the intended prefix.
- Around line 246-261: Update write_mcp_config to pass Playwright MCP
localhost-only origin restrictions in the generated args, using the required
localhost/127.0.0.1 allowlist or equivalent blocked-origins plus allowlist
configuration. Document alongside the configuration that these flags are
defense-in-depth only and do not prevent redirects from reaching off-localhost
destinations.
---
Nitpick comments:
In `@scripts/explore.sh`:
- Around line 149-154: Update stop_lock_holder and the do_down flow to guard
against tearing down a lock owned by another active interactive session. Before
killing the PID from lock.ok, detect whether it represents an active
claude-managed stack and warn or require explicit confirmation when ownership
cannot be established; preserve cleanup for the invoking owner and document the
supported separate up/down behavior if that is the chosen safeguard.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 762bcd62-0ca2-4e80-bacf-4434944c876d

📥 Commits

Reviewing files that changed from the base of the PR and between db0ecad and 914b0ae.

📒 Files selected for processing (5)
  • .claude/skills/explore/SKILL.md
  • .gitignore
  • Makefile
  • scripts/explore.sh
  • scripts/explore/explorer-prompt.md

Comment threadscripts/explore.sh
Comment threadscripts/explore.sh
Comment on lines +246 to +261
write_mcp_config() {
local headless_args='"--headless", '
[ "$EXPLORE_HEADED" = "1" ] && headless_args=''
cat > "$EXPLORE_DIR/mcp.json" <<EOF
{
"mcpServers": {
"playwright": {
"command": "npx",
"args": ["-y", "@playwright/mcp@$PLAYWRIGHT_MCP_VERSION", "--isolated", ${headless_args}"--browser", "chromium",
"--storage-state", "$EXPLORE_DIR/storageState.json",
"--output-dir", "$EXPLORE_DIR/traces", "--save-session"]
}
}
}
EOF
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== locate files =="
fd -a 'explore.sh|explorer-prompt.md|package.json|pnpm-lock.yaml|package-lock.json|yarn.lock|playwright|mcp.json'.| sed 's#^\./##'| head -200
echoecho"== relevant explore.sh lines =="if [ -f scripts/explore.sh ];then
nl -ba scripts/explore.sh | sed -n '220,280p'fiechoecho"== relevant explorer-prompt lines =="if [ -f scripts/explore/explorer-prompt.md ];then
nl -ba scripts/explore/explorer-prompt.md | sed -n '1,35p'fiif [ -f explorer-prompt.md ];then
nl -ba explorer-prompt.md | sed -n '1,35p'fiechoecho"== search required navigation text and mcp config args =="
rg -n "Restrict navigation|localhost|localhost-only|localhost:3000|allowed-origins|blocked-origins|explorer-prompt|MISSION|write_mcp_config|PLAYWRIGHT_MCP_VERSION" -S .

Repository: SaplingLearn/Sapling

Length of output: 479


🌐 Web query:

Playwright MCP 0.0.78 README --allowed-origins --blocked-origins

💡 Result:

In Playwright MCP, the --allowed-origins and --blocked-origins flags (and their corresponding environment variables) are used to manage network access for the browser instance [1][2][3]. Both settings accept a semicolon-separated list of origins [1][4][3]. Key details: --allowed-origins This flag specifies a list of trusted origins that the browser is permitted to request [1][5]. By default, all origins are allowed [1][2]. Environment variable: PLAYWRIGHT_MCP_ALLOWED_ORIGINS [1][3]. --blocked-origins This flag specifies a list of origins to block the browser from requesting [1][5]. The blocklist is evaluated before the allowlist; if an origin is present in both, it will be blocked [1][2][5]. Environment variable: PLAYWRIGHT_MCP_BLOCKED_ORIGINS [1][3]. Important Security Note: Documentation explicitly states that these flags do not serve as a security boundary and do not affect redirects [1][5][3]. Supported Formats: Both flags support the following formats [1][2]: Full origin: e.g., https://example.com:8080 (matches only that specific origin) [1][2]. Wildcard port: e.g., http://localhost:* (matches any port on localhost with the http protocol) [1][2].

Citations:


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== relevant explore.sh lines =="if [ -f scripts/explore.sh ];then
sed -n '220,280p' scripts/explore.sh | cat -n
fiechoecho"== relevant explorer-prompt lines =="if [ -f scripts/explore/explorer-prompt.md ];then
sed -n '1,40p' scripts/explore/explorer-prompt.md | cat -n
fiechoecho"== search required navigation text and mcp config args =="
rg -n "Restrict navigation|localhost|localhost-only|localhost:3000|allowed-origins|blocked-origins|explorer-prompt|MISSION|write_mcp_config|PLAYWRIGHT_MCP_VERSION" -S .echoecho"== lockfile version snippets =="forfin package-lock.json frontend/package-lock.json;doif [ -f"$f" ];thenecho"--- $f ---"
rg -n '"`@playwright/mcp`"|"`@playwright/test`"|PLAYWRIGHT_MCP_VERSION'"$f" -A 3 -B 2 ||truefidone

Repository: SaplingLearn/Sapling

Length of output: 22370


🌐 Web query:

playwright mcp --allowed-origins blocked navigation error network error

💡 Result:

The "blocked navigation error" or network-related errors when using the --allowed-origins flag in Playwright MCP typically stem from configuration format issues, regex/glob matching errors, or confusing the flag with server-side CORS settings [1][2][3]. Key troubleshooting steps include: 1. Use Correct Origin Format: Ensure you are passing the domain or origin directly (e.g., google.com) rather than including protocol prefixes like https:// or trailing slashes, which have historically caused parsing issues [1]. 2. Avoid Invalid Glob Patterns: Recent versions (as of early 2026) have had bugs where wildcard port patterns like http://localhost:* generate invalid internal glob patterns, causing the browser to block all traffic to those destinations [3]. If you are experiencing this, you may need to use explicit hostnames or check for updates addressing the originOrHostGlob function [3]. 3. Distinguish Between Flags: Ensure you are not confusing --allowed-origins (which controls browser navigation within the sandboxed Playwright instance) with --allowed-hosts (which controls MCP server CORS/connectivity) [2]. Both may need to be set identically if you are navigating to localhost in restricted environments like GitHub Actions or WSL [2][4]. 4. Blocklist Precedence: Keep in mind that Playwright MCP evaluates the blocked-origins list before the allowed-origins list. If a URL matches both, it will be blocked [5]. 5. Debugging Network Issues: If navigation still fails with ERR_CONNECTION_REFUSED, verify that the target service (e.g., a local dev server) is listening on 0.0.0.0 rather than just 127.0.0.1 to ensure it is accessible to the browser process, especially if running within containers (Docker) or WSL [2][4]. For further debugging, you can enable verbose logging by setting the DEBUG environment variable to pw:api or pw:browser* to see exactly which origin is failing to match the navigation request [4].

Citations:


🌐 Web query:

Playwright MCP blocked origins error "Failed to navigate to"

💡 Result:

The error net::ERR_BLOCKED_BY_CLIENT when using Playwright MCP typically indicates that the browser navigation was blocked by a client-side security policy, configuration, or environment restriction [1]. Because Playwright MCP is often used in automated contexts (such as LLM agents), this error is usually triggered by one of the following factors [1]: 1. Allowed Origins Configuration: If you have configured allowed-origins for security, the destination URL may be excluded from the allowlist [2][3]. Verify that the PW_ALLOWED_ORIGINS environment variable or the --allowed-origins CLI flag is correctly set to include the domain you are attempting to visit [2][3]. 2. Browser Environment and Sandbox Restrictions: When running in containerized environments like Docker, Chromium often fails to navigate because it cannot access the necessary system sandbox [1]. To resolve this, you may need to pass explicit launch flags to bypass sandbox restrictions in your PLAYWRIGHT_LAUNCH_OPTIONS [1]: --no-sandbox --disable-setuid-sandbox --disable-gpu 3. Security Settings or Extensions: Although Playwright MCP sessions are typically isolated, ERR_BLOCKED_BY_CLIENT can also occur if the browser launch arguments include restrictive security settings or if there are conflicts with other security-related flags [1]. 4. Troubleshooting Steps: - Check the logs to confirm the specific URL being blocked [1]. - If you are running in Docker, ensure the container has the necessary privileges and sandbox bypass flags [1]. - Review your MCP configuration file (often found in .mcp/config.json or equivalent) to ensure --allowed-origins is formatted correctly (e.g., comma-separated or semicolon-separated depending on your specific implementation) [4][2][3]. - If you suspect the issue is not related to origins, verify that npx and the Playwright runtime are correctly installed and accessible in your environment, as missing dependencies can sometimes lead to unexpected browser behavior [5]. If the error persists despite correct origin configuration, it is often helpful to test a basic navigation command outside of the MCP wrapper to rule out environment-specific sandbox or policy issues [1].

Citations:


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== package manifests mentioning playwright mcp =="
rg -n '"`@playwright/mcp`"|"playwright-mcp"|PLAYWRIGHT_MCP_VERSION' -S --glob '!node_modules/**' --glob '!frontend/node_modules/**' frontend package.json scripts
echoecho"== scripts/explore.sh version assignment and mcp config callers =="
nl -ba scripts/explore.sh | sed -n '60,80p'
nl -ba scripts/explore.sh | sed -n '370,390p'echoecho"== local prompt copy =="
cat -n scripts/explore/explorer-prompt.md | sed -n '1,20p'

Repository: SaplingLearn/Sapling

Length of output: 587


🌐 Web query:

site:github.com/microsoft/playwright-mcp "allowedOrigins" "blockedByClient" "ERR_BLOCKED_BY_CLIENT"

💡 Result:

The error ERR_BLOCKED_BY_CLIENT in the context of Playwright MCP (Model Context Protocol) is a browser-level error that occurs when the browser is instructed to navigate to or request a resource that has been explicitly blocked or is unreachable due to configuration issues [1][2]. In the specific context of the Playwright MCP server, this error frequently arises from the following scenarios: 1. Origin Filtering: If you have configured --allowed-origins or --blocked-origins, the browser will block navigation to any origin not in the allowlist or present in the blocklist [1]. Note that these flags affect navigation but do not serve as a CORS security boundary [1][3]. 2. Extension ID Mismatch: A common cause for ERR_BLOCKED_BY_CLIENT in recent versions (e.g., v0.0.66–0.0.68) is a hardcoded Chrome extension ID in the MCP server's connection logic [2]. If you are using an unpacked extension, a locally built version, or a different browser channel, your extension will have a different ID than the one hardcoded (mmlmfjhmonkocbjadbfplnigmagldckm). This causes the browser to attempt to connect to a non-existent extension, resulting in the error [2]. 3. Configuration Bugs: Bugs in how origin patterns are parsed—such as incorrect handling of wildcard ports (e.g., http://localhost:*)—can lead to requests being unexpectedly blocked [4]. To resolve or debug this error: - Verify your --allowed-origins and --blocked-origins configurations to ensure they match the intended URLs [1][4]. - If using an unpacked browser extension, ensure the extension ID is handled correctly, potentially by setting the PLAYWRIGHT_MCP_EXTENSION_ID environment variable or pinning your extension's ID via the manifest's "key" property [2]. - Check the official issue tracker for your version of Playwright MCP, as specific releases have included fixes for origin pattern parsing and connection logic [4][2][5].

Citations:


🌐 Web query:

github.com/microsoft/playwright-mcp program.ts "allowed-origins" "allowedOrigins" "blockedByClient"

💡 Result:

In the Playwright MCP server, allowedOrigins and blockedOrigins are configuration options used to control which network requests the browser is permitted to make [1][2]. These settings are managed within the server's network configuration and are implemented via Playwright's routing capabilities [3][4]. When these rules are active, the server intercepts network requests and uses the 'blockedbyclient' error code to terminate requests that do not align with the defined allow/block lists [3][4]. Specifically, the implementation follows this logic: 1. If allowedOrigins are defined, the server first blocks all requests by default using route.abort('blockedbyclient') [3][5]. 2. It then explicitly allows requests to the specified origins by calling route.continue for patterns matching the allowed list [3][4]. 3. If blockedOrigins are defined, the server explicitly blocks requests to those origins using route.abort('blockedbyclient') [3][4]. Origins matching both allowedOrigins and blockedOrigins will be blocked [1][2]. The allowed-origins command-line argument accepts a semicolon-separated list of origins, and the configuration supports full origin strings (e.g., https://example.com:8080) and wildcard port patterns (e.g., http://localhost:*) [1][6]. These settings are considered convenience features for managing browser interactions rather than strict security boundaries [1][2]. The file program.ts within the Playwright repository is involved in managing the command-line interface and tool configuration for the MCP server [7].

Citations:


Reinforce the localhost-first prompt with a technical restriction.

This harness requires localhost-only navigation, but mcp.json relies only on scripts/explore/explorer-prompt.md:13 for enforcement. Add a Playwright MCP restriction such as --allowed-origins http://localhost:3000;http://localhost:5000;127.0.0.1:5000 or --blocked-origins * plus the localhost allowlist, while documenting that these flags are not a security boundary against redirects; this gives defense-in-depth against model-driven or page-driven off-localhost clicks.

🤖 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/explore.sh` around lines 246 - 261, Update write_mcp_config to pass
Playwright MCP localhost-only origin restrictions in the generated args, using
the required localhost/127.0.0.1 allowlist or equivalent blocked-origins plus
allowlist configuration. Document alongside the configuration that these flags
are defense-in-depth only and do not prevent redirects from reaching
off-localhost destinations.

Comment threadscripts/explore.sh
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

Code review

Found 3 issues:

  1. do_down() can leak the stack and the machine-singleton lock: it runs under set -euo pipefail and fires as an EXIT trap, but its mkdir -p, the findings-file bootstrap, and the { … } >> findings.md append block have no || true guards (unlike the oracle subshell and e2e-down.sh lines around them). A failed write there (disk full, read-only fs) aborts the trap before e2e-down.sh and stop_lock_holder run — verified bash semantics: a failing command inside an EXIT trap under set -e stops the trap. The header comment's "|| true guards throughout" claim is currently false for these lines.

do_down() {
mkdir -p "$EXPLORE_DIR"
(
cd"$REPO_ROOT/backend"
venv/bin/python -m e2e_oracles --json >"$EXPLORE_DIR/oracle-final.json"||true
venv/bin/python -m e2e_oracles >"$EXPLORE_DIR/oracle-final.txt"2>&1||true
) ||true
[ -f"$EXPLORE_DIR/findings.md" ] ||printf'# Exploration findings\n'>"$EXPLORE_DIR/findings.md"
{
printf'\n## Oracle final pass (%s)\n\n```\n'"$(date -Iseconds)"
cat "$EXPLORE_DIR/oracle-final.txt"2>/dev/null ||true
printf'```\n'
} >>"$EXPLORE_DIR/findings.md"
"$REPO_ROOT/scripts/e2e-down.sh"||true
stop_lock_holder
}

  1. Dummy-key mitigation for RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439 defeats itself in the common developer environment: export GEMINI_API_KEY="${GEMINI_API_KEY:-e2e-dummy-key-no-billing}" defers to an ambient real key, and developers here routinely keep one exported (validate_config fail-closes without it). The below-seam RAG/embedding path (RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439) bills live even in function mode, and the explorer's mission (uploads, quiz, tutor turns, many turns) is worst-case volume for it. CI applies the dummy key unconditionally for exactly this reason; export inside the script can't touch the operator's parent shell, so the conditional protects nothing.

# Default only if unset — never clobber an operator's real key.
export GEMINI_API_KEY="${GEMINI_API_KEY:-e2e-dummy-key-no-billing}"
"$REPO_ROOT/scripts/e2e-up.sh"|| die "scripts/e2e-up.sh failed — see .e2e/*.log"

  1. jq is a hard dependency of the transcript pipeline but is missing from preflight (claude/npx/curl/python3 are checked). Without it the script boots the entire stack — Supabase, migrations, a production Next build, minutes of work — and only then fails in run_explorer, instead of failing fast like the other four checks.

echo"▶ Preflight…"
command -v claude >/dev/null 2>&1|| die "claude CLI not found on PATH — install it first"
command -v npx >/dev/null 2>&1|| die "npx not found on PATH — install Node.js first"
command -v curl >/dev/null 2>&1|| die "curl not found on PATH"
command -v python3 >/dev/null 2>&1|| die "python3 not found on PATH"
[ -f"$REPO_ROOT/scripts/explore/explorer-prompt.md" ] \

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

…eflight, scoped Edit, EXPLORE_USER wiring (#399)
- do_down: guard every fallible write with || true so a failed findings.md
write can never abort the EXIT trap before e2e-down.sh / stop_lock_holder
run (was leaking the stack + machine-singleton lock on write failure).
- GEMINI_API_KEY: export unconditionally (matches CI's dummy, #439) instead
of deferring to an ambient real key that can bill the below-seam RAG path.
- SEED_RICH=1 exported unconditionally so an ambient SEED_RICH=0 can't
silently defeat the rich dataset this harness requires.
- preflight: add missing `jq` check (hard dependency of the transcript
pipeline) so a missing jq fails in seconds, not after a full stack boot.
- allowedTools: replace unscoped Write,Edit with Read,Edit(.explore/**) —
Write(...) patterns aren't matched by the CLI's file permission check, so
only a path-scoped Edit rule actually restricts writes to findings.md.
do_up now pre-creates .explore/findings.md so the explorer always has an
existing file for the scoped Edit grant.
- EXPLORE_USER: derive the sapling_user display name from the user id
(case map for the five seeded rich-* users, verified against
db/seed_local_rich.py) instead of hardcoding "Rich Active"; pass
--user "$EXPLORE_USER" to both oracle invocations in do_down.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

All three review findings fixed in a0360ff (do_down teardown/lock-release guards verified with a real write-failure injection; unconditional dummy GEMINI_API_KEY per #439; jq preflight), plus three panel-verified hardening items in the same commit: allowedTools Write dropped / Edit scoped to .explore/**, EXPLORE_USER wired through the name map and both oracle --user calls, unconditional SEED_RICH=1. Scoped re-review confirms all addressed, no new breakage.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(agent): LLM explorer harness driving Playwright against the local stack

1 participant

@AndresL230
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

feat(e2e): Chapter 2 explore harness — make explore + /explore skill (#399) - #444

Merged
AndresL230 merged 9 commits into
mainfrom
e2e-chapter2-explore
Jul 28, 2026
Merged

feat(e2e): Chapter 2 explore harness — make explore + /explore skill (#399)#444
AndresL230 merged 9 commits into
mainfrom
e2e-chapter2-explore

Conversation

@AndresL230

@AndresL230AndresL230 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Chapter 2 of the E2E program (epic #403), PR 2 of 3. Closes#399. Builds on the #400 oracle module (PR #443).

make explore boots the deterministic local stack and hands a Claude Code explorer a real browser; the oracles judge what it saw. Local-only by design — never a CI gate.

What's here

  • scripts/explore.sh (+ make explore / make explore-down, .explore/ gitignored) — the full pipeline: machine-singleton stack lock (detached holder process writing self-identifying lock.ok only after flock succeeds — a failed acquisition touches nothing, proven by an A/B two-session simulation) → e2e-up in function mode with a dummy GEMINI_API_KEY (RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439) → storage state minted via POST /api/auth/test-login through the frontend origin (cookie + sapling_user localStorage, secure flag matching the backend's actual local policy) → claude -p with a pinned @playwright/mcp@0.0.78 config and turn budget, transcript captured via stream-json + jq into a grep-able session.log → oracle final pass appended to findings.md → teardown. up/down subcommands support the interactive flow.
  • scripts/explore/explorer-prompt.md — the mission briefing: student persona (Rich Active), break-things mandate, report-never-fix, oracle cadence (cd backend && venv/bin/python -m e2e_oracles), findings format, known-bugs list, and a "stub it before you dig" rule earned in live testing.
  • .claude/skills/explore/SKILL.md — the interactive /explore skill: same pipeline, watchable and steerable, with a same-origin JS sign-in for live-browser sessions.

Hard-won fixes from live acceptance runs

  • @playwright/mcp without --isolated reuses a persistent profile and silently ignores --storage-state — the run-2 "signed-in" state was a stale-cookie artifact; --isolated fixes it (verified down to the bundled playwright-core source and a cookie-DB probe).
  • claude -p's default text output prints only the final message — a turn-budget exit produced an empty transcript despite real browsing; --output-format stream-json --verbose + jq restores the readable transcript.
  • Lock-holder bookkeeping survives concurrent sessions (busy attempt leaves the running session's files byte-identical).

Acceptance (issue #399 bar, definitive run in the SDD report)

One bounded run (EXPLORE_MAX_TURNS=25, 3m52s): real signed-in navigation across dashboard + library ("Rich Active" nav, 4 seeded documents, CS101/MATH210/BIO110/ENG150 visible in snapshots), 2 explorer-written findings — including a novel wrong-data upload bug and a tutor-resume 500 (concept_describe unregistered in the function-handler seam) independently confirmed by the oracle logscan — plus the harness-appended Oracle final pass re-confirming #355. Machine verified clean afterward (ports, containers, lock).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added an end-to-end exploratory testing workflow for the local application.
    • Added commands to start, stop, and run complete exploration sessions.
    • Exploration sessions now support browser-driven testing, automated checks, session logs, and structured findings.
  • Documentation

    • Added guidance covering exploration steps, reporting requirements, known issues, and session summaries.
  • Chores

    • Exploration artifacts are now excluded from version control.

AndresL230and others added 8 commits July 28, 2026 01:13
…itignore (#399)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e, oracle cadence (#399)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…er GEMINI_API_KEY (#399)
A busy lock previously still tripped do_up's trap/wipe path, tearing down
another session's live stack via scripts/e2e-down.sh and deleting its
.explore/ artifacts before the lock check ever ran. start_lock_holder now
runs first, the do_down safety trap arms only after the lock is held, the
.explore/ wipe (still selective, preserving lock.pid/lock.ok) happens after
that, and a failed acquisition cleans up its own holder/pid files before
exiting. GEMINI_API_KEY now defaults only when unset instead of always
overwriting an operator's real key.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ns (#399)
start_lock_holder previously deleted lock.ok and unconditionally wrote
lock.pid before knowing whether the flock would actually succeed. A failed
attempt from session B (lock held by session A) would delete A's lock.ok and
overwrite A's lock.pid with B's own doomed PID, then B's busy-path cleanup
removed the file entirely — leaving A's later `down` unable to find A's
holder, leaking both the detached process and the machine-singleton lock.
Now the holder subprocess itself is the only writer of lock.ok, and only
ever after its own `flock -n 9` succeeds (atomic temp-file + mv, content =
the holder's own $$). The parent only polls for that self-identifying
marker and touches nothing on disk on the failure path, so a busy lock can
never clobber another session's bookkeeping. lock.pid is retired — lock.ok's
content is now the sole source of truth stop_lock_holder reads.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The first bounded acceptance run (task-8-report.md) produced a
session.log with exactly one line — claude -p's own "Error: Reached
max turns" — because the default --output-format=text only prints the
FINAL message, never the intermediate tool_use/tool_result turns. That
failed the #399 acceptance bar ("session.log shows real Playwright MCP
tool calls").
Switch run_explorer to --output-format stream-json --verbose (the CLI
requires both together) and reformat the JSONL with jq into readable
[assistant]/[tool_use]/[tool_result]/[result] lines, truncated to 500
chars each, so session.log is both grep-able and human-skimmable.
fromjson? tolerates stray non-JSON lines instead of aborting the whole
transcript; claude -p's stderr now goes to its own session.stderr.log
so it can never interleave with (and corrupt) the JSON stream jq
parses.
Verified: a second bounded run (EXPLORE_MAX_TURNS=25) produced a
281-line session.log with 18 mcp__playwright__* tool calls across
/dashboard and /library, plus findings.md with a re-confirmed #430
repro and the oracle final pass re-confirming #355.
The follow-up bounded-run diagnosis found a contradiction: run 2's
session cookie was accepted (/api/users 200) but the UI acted fully
signed-out (Sign In button, dashboard skeleton) — the #430 symptom.
Run 1's "Secure cookie dropped over http" diagnosis didn't hold up
either, since Chromium does accept Secure cookies on http://localhost.
Root-caused by reading @playwright/mcp@0.0.78's bundled source
(playwright-core/lib/coreBundle.js): without --isolated, the MCP
server launches ONE PERSISTENT Chrome profile keyed only by
sha256(cwd) — reused across every explore.sh run from this checkout,
never wiped by teardown — and its client factory does
`config.browser.isolated ? await browser.newContext(contextOptions) :
browser.contexts()[0]`. In the default (non-isolated) branch it just
grabs the already-open persistent context and never calls newContext
with our --storage-state at all.
Verified empirically: wiped the profile dir, booted a clean stack, and
drove a 3-turn claude -p probe against the (then-current) mcp.json —
navigating to /dashboard redirected straight to a REAL
accounts.google.com sign-in page, and sqlite3 on the profile's Cookies
db afterward showed zero sapling_session rows (neither cookie nor
localStorage from storageState.json was ever applied). The identical
probe with --isolated added rendered the real, fully-authenticated
dashboard on the first navigation, with localStorage.sapling_user
correctly present — this is the same mechanism (ephemeral
browser.newContext(contextOptions)) @playwright/test itself uses in
Chapter 1's global-setup.ts.
Also corrected mint_storage_state's cookie to secure:false, matching
what the backend's own SECURE_COOKIES policy actually issues for the
http://localhost local stack (config.py derives it from FRONTEND_URL's
scheme) — the file should mirror reality regardless of which flag
turned out to be the load-bearing one.
Verified: EXPLORE_MAX_TURNS=12 make explore reached a signed-in
dashboard (nav shows "Rich Active" / "Account", full authenticated
menu) using only 2 real Playwright tool calls, zero sign-in recovery
turns.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The definitive acceptance run (harness fixes verified: real auth via
--isolated, breadth across 2+ surfaces) hit a genuine new bug —
resuming a tutor session 500'd on POST /api/graph/.../concept-description,
root-caused via the explorer's own backend-log investigation to a
missing SAPLING_FUNCTION_HANDLERS registration for 'concept_describe'
(caught independently by the oracle's logscan pass too, so it's not
lost, just not explorer-authored). But the explorer spent its
remaining turns tailing logs and checking processes to nail the exact
LookupError, and ran out of budget before writing an F<N> entry to
findings.md — a real gap against #399's "readable transcript AND
findings file" bar, distinct from any flag/mechanism defect.
Add a "stub it before you dig" ground rule: write a one-line stub
finding the instant something looks off, before further root-causing.
A written stub survives a turn-budget cutoff; a perfect unwritten
diagnosis does not.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:34 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: dc0a75e3-31f4-42be-8ec2-bb9a848919bb

📥 Commits

Reviewing files that changed from the base of the PR and between 914b0ae and a0360ff.

📒 Files selected for processing (1)
  • scripts/explore.sh
📝 Walkthrough

Walkthrough

Adds a locked local exploratory-testing workflow that boots the E2E stack, authenticates a seeded browser session, runs Claude through Playwright MCP, records findings and oracle output, and reliably tears down the stack.

Changes

Exploration harness

Layer / File(s)Summary
Exploration contract and entrypoints
.claude/skills/explore/SKILL.md, scripts/explore/explorer-prompt.md, Makefile, .gitignore
Defines the exploration workflow, reporting constraints, findings format, operator targets, and ignored .explore/ artifacts.
Locked stack setup
scripts/explore.sh
Adds prerequisite checks, repository configuration, detached flock locking, and local E2E stack startup.
Authenticated browser session
scripts/explore.sh
Creates seeded-user Playwright storage state, writes the pinned MCP configuration, and runs Claude with structured logs and separate stderr output.
Oracle and teardown lifecycle
scripts/explore.sh
Supports full, up, and down modes, appends final oracle results, tears down the stack, and releases the lock.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
participant Operator
participant explore.sh
participant E2EStack
participant AuthTestEndpoint
participant PlaywrightMCP
participant Browser
participant Claude
Operator->>explore.sh: start exploration
explore.sh->>E2EStack: boot local E2E stack
explore.sh->>AuthTestEndpoint: POST seeded student login
explore.sh->>PlaywrightMCP: configure storage state and MCP
Claude->>PlaywrightMCP: execute exploration tools
PlaywrightMCP->>Browser: drive localhost browser
explore.sh->>E2EStack: run oracle and teardown
Loading

Possibly related issues

  • Issue 403 — Covers the broader Chapter 2 AI Agent Explorer epic implemented by this harness.
  • Issue 401 — Proposes nightly automation and regression promotion for findings produced by this exploration workflow.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly names the new explore harness and /explore skill and matches the main change.
Description check✅ PassedThe description covers summary, changes, related issue, testing, screenshots, and reviewer notes with mostly complete content.
Linked Issues check✅ PassedThe PR implements the local Playwright explorer harness, seeded auth, hard budget/localhost limits, structured logs, and readable session output required by #399.
Out of Scope Changes check✅ PassedThe changes stay focused on the explore harness, prompt, Makefile, and ignore rules; no unrelated functionality is introduced.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch e2e-chapter2-explore

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Jul 28, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-staginga0360ffCommit Preview URL

Branch Preview URL
Jul 28 2026, 10:20 AM

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
scripts/explore.sh (1)

149-154: 🩺 Stability & Availability | 🔵 Trivial

stop_lock_holder/do_down release the machine lock without verifying the caller owns it.

do_down (and thus scripts/explore.sh down / make explore-down) unconditionally reads lock.ok's pid and kills it, regardless of whether the invoking shell is the one that acquired it. The elaborate ownership-safety work in start_lock_holder (a failed attempt never touches lock.ok) isn't mirrored here: since the lock is uid-namespaced (/tmp/claude-$(id -u)/...), the realistic blast radius is a single operator accidentally running down from a second terminal while up is still driving an interactive session elsewhere on the same machine — which would yank the shared local stack out from under that live session with no warning. Given the doc explicitly anticipates the "interactive flow" (separate up/down invocations) as a supported mode, this seems worth at least a guard (e.g. warn if a stack that looks actively in-use by claude is being torn down) or an explicit doc callout of the risk.

Also applies to: 404-419

🤖 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/explore.sh` around lines 149 - 154, Update stop_lock_holder and the
do_down flow to guard against tearing down a lock owned by another active
interactive session. Before killing the PID from lock.ok, detect whether it
represents an active claude-managed stack and warn or require explicit
confirmation when ownership cannot be established; preserve cleanup for the
invoking owner and document the supported separate up/down behavior if that is
the chosen safeguard.
🤖 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 `@scripts/explore.sh`:
- Around line 176-202: Update the localStorage payload in mint_storage_state so
the sapling_user name is derived from $EXPLORE_USER instead of hardcoding "Rich
Active". Preserve the existing user ID and other payload fields while ensuring
any configured seeded rich-* user receives a matching cached display name.
- Around line 302-337: Restrict the Write/Edit entries in run_explorer() to the
permitted .explore findings path, using path-scoped rules such as
Edit(.explore/**/*.md) and removing blanket file-edit permissions. Also verify
the current Claude CLI Bash permission syntax for the cd backend &&
venv/bin/python -m e2e_oracles:* rule, tightening it if compound-command parsing
permits commands outside the intended prefix.
- Around line 246-261: Update write_mcp_config to pass Playwright MCP
localhost-only origin restrictions in the generated args, using the required
localhost/127.0.0.1 allowlist or equivalent blocked-origins plus allowlist
configuration. Document alongside the configuration that these flags are
defense-in-depth only and do not prevent redirects from reaching off-localhost
destinations.
---
Nitpick comments:
In `@scripts/explore.sh`:
- Around line 149-154: Update stop_lock_holder and the do_down flow to guard
against tearing down a lock owned by another active interactive session. Before
killing the PID from lock.ok, detect whether it represents an active
claude-managed stack and warn or require explicit confirmation when ownership
cannot be established; preserve cleanup for the invoking owner and document the
supported separate up/down behavior if that is the chosen safeguard.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 762bcd62-0ca2-4e80-bacf-4434944c876d

📥 Commits

Reviewing files that changed from the base of the PR and between db0ecad and 914b0ae.

📒 Files selected for processing (5)
  • .claude/skills/explore/SKILL.md
  • .gitignore
  • Makefile
  • scripts/explore.sh
  • scripts/explore/explorer-prompt.md

Comment threadscripts/explore.sh
Comment threadscripts/explore.sh
Comment on lines +246 to +261
write_mcp_config() {
local headless_args='"--headless", '
[ "$EXPLORE_HEADED" = "1" ] && headless_args=''
cat > "$EXPLORE_DIR/mcp.json" <<EOF
{
"mcpServers": {
"playwright": {
"command": "npx",
"args": ["-y", "@playwright/mcp@$PLAYWRIGHT_MCP_VERSION", "--isolated", ${headless_args}"--browser", "chromium",
"--storage-state", "$EXPLORE_DIR/storageState.json",
"--output-dir", "$EXPLORE_DIR/traces", "--save-session"]
}
}
}
EOF
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== locate files =="
fd -a 'explore.sh|explorer-prompt.md|package.json|pnpm-lock.yaml|package-lock.json|yarn.lock|playwright|mcp.json'.| sed 's#^\./##'| head -200
echoecho"== relevant explore.sh lines =="if [ -f scripts/explore.sh ];then
nl -ba scripts/explore.sh | sed -n '220,280p'fiechoecho"== relevant explorer-prompt lines =="if [ -f scripts/explore/explorer-prompt.md ];then
nl -ba scripts/explore/explorer-prompt.md | sed -n '1,35p'fiif [ -f explorer-prompt.md ];then
nl -ba explorer-prompt.md | sed -n '1,35p'fiechoecho"== search required navigation text and mcp config args =="
rg -n "Restrict navigation|localhost|localhost-only|localhost:3000|allowed-origins|blocked-origins|explorer-prompt|MISSION|write_mcp_config|PLAYWRIGHT_MCP_VERSION" -S .

Repository: SaplingLearn/Sapling

Length of output: 479


🌐 Web query:

Playwright MCP 0.0.78 README --allowed-origins --blocked-origins

💡 Result:

In Playwright MCP, the --allowed-origins and --blocked-origins flags (and their corresponding environment variables) are used to manage network access for the browser instance [1][2][3]. Both settings accept a semicolon-separated list of origins [1][4][3]. Key details: --allowed-origins This flag specifies a list of trusted origins that the browser is permitted to request [1][5]. By default, all origins are allowed [1][2]. Environment variable: PLAYWRIGHT_MCP_ALLOWED_ORIGINS [1][3]. --blocked-origins This flag specifies a list of origins to block the browser from requesting [1][5]. The blocklist is evaluated before the allowlist; if an origin is present in both, it will be blocked [1][2][5]. Environment variable: PLAYWRIGHT_MCP_BLOCKED_ORIGINS [1][3]. Important Security Note: Documentation explicitly states that these flags do not serve as a security boundary and do not affect redirects [1][5][3]. Supported Formats: Both flags support the following formats [1][2]: Full origin: e.g., https://example.com:8080 (matches only that specific origin) [1][2]. Wildcard port: e.g., http://localhost:* (matches any port on localhost with the http protocol) [1][2].

Citations:


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== relevant explore.sh lines =="if [ -f scripts/explore.sh ];then
sed -n '220,280p' scripts/explore.sh | cat -n
fiechoecho"== relevant explorer-prompt lines =="if [ -f scripts/explore/explorer-prompt.md ];then
sed -n '1,40p' scripts/explore/explorer-prompt.md | cat -n
fiechoecho"== search required navigation text and mcp config args =="
rg -n "Restrict navigation|localhost|localhost-only|localhost:3000|allowed-origins|blocked-origins|explorer-prompt|MISSION|write_mcp_config|PLAYWRIGHT_MCP_VERSION" -S .echoecho"== lockfile version snippets =="forfin package-lock.json frontend/package-lock.json;doif [ -f"$f" ];thenecho"--- $f ---"
rg -n '"`@playwright/mcp`"|"`@playwright/test`"|PLAYWRIGHT_MCP_VERSION'"$f" -A 3 -B 2 ||truefidone

Repository: SaplingLearn/Sapling

Length of output: 22370


🌐 Web query:

playwright mcp --allowed-origins blocked navigation error network error

💡 Result:

The "blocked navigation error" or network-related errors when using the --allowed-origins flag in Playwright MCP typically stem from configuration format issues, regex/glob matching errors, or confusing the flag with server-side CORS settings [1][2][3]. Key troubleshooting steps include: 1. Use Correct Origin Format: Ensure you are passing the domain or origin directly (e.g., google.com) rather than including protocol prefixes like https:// or trailing slashes, which have historically caused parsing issues [1]. 2. Avoid Invalid Glob Patterns: Recent versions (as of early 2026) have had bugs where wildcard port patterns like http://localhost:* generate invalid internal glob patterns, causing the browser to block all traffic to those destinations [3]. If you are experiencing this, you may need to use explicit hostnames or check for updates addressing the originOrHostGlob function [3]. 3. Distinguish Between Flags: Ensure you are not confusing --allowed-origins (which controls browser navigation within the sandboxed Playwright instance) with --allowed-hosts (which controls MCP server CORS/connectivity) [2]. Both may need to be set identically if you are navigating to localhost in restricted environments like GitHub Actions or WSL [2][4]. 4. Blocklist Precedence: Keep in mind that Playwright MCP evaluates the blocked-origins list before the allowed-origins list. If a URL matches both, it will be blocked [5]. 5. Debugging Network Issues: If navigation still fails with ERR_CONNECTION_REFUSED, verify that the target service (e.g., a local dev server) is listening on 0.0.0.0 rather than just 127.0.0.1 to ensure it is accessible to the browser process, especially if running within containers (Docker) or WSL [2][4]. For further debugging, you can enable verbose logging by setting the DEBUG environment variable to pw:api or pw:browser* to see exactly which origin is failing to match the navigation request [4].

Citations:


🌐 Web query:

Playwright MCP blocked origins error "Failed to navigate to"

💡 Result:

The error net::ERR_BLOCKED_BY_CLIENT when using Playwright MCP typically indicates that the browser navigation was blocked by a client-side security policy, configuration, or environment restriction [1]. Because Playwright MCP is often used in automated contexts (such as LLM agents), this error is usually triggered by one of the following factors [1]: 1. Allowed Origins Configuration: If you have configured allowed-origins for security, the destination URL may be excluded from the allowlist [2][3]. Verify that the PW_ALLOWED_ORIGINS environment variable or the --allowed-origins CLI flag is correctly set to include the domain you are attempting to visit [2][3]. 2. Browser Environment and Sandbox Restrictions: When running in containerized environments like Docker, Chromium often fails to navigate because it cannot access the necessary system sandbox [1]. To resolve this, you may need to pass explicit launch flags to bypass sandbox restrictions in your PLAYWRIGHT_LAUNCH_OPTIONS [1]: --no-sandbox --disable-setuid-sandbox --disable-gpu 3. Security Settings or Extensions: Although Playwright MCP sessions are typically isolated, ERR_BLOCKED_BY_CLIENT can also occur if the browser launch arguments include restrictive security settings or if there are conflicts with other security-related flags [1]. 4. Troubleshooting Steps: - Check the logs to confirm the specific URL being blocked [1]. - If you are running in Docker, ensure the container has the necessary privileges and sandbox bypass flags [1]. - Review your MCP configuration file (often found in .mcp/config.json or equivalent) to ensure --allowed-origins is formatted correctly (e.g., comma-separated or semicolon-separated depending on your specific implementation) [4][2][3]. - If you suspect the issue is not related to origins, verify that npx and the Playwright runtime are correctly installed and accessible in your environment, as missing dependencies can sometimes lead to unexpected browser behavior [5]. If the error persists despite correct origin configuration, it is often helpful to test a basic navigation command outside of the MCP wrapper to rule out environment-specific sandbox or policy issues [1].

Citations:


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== package manifests mentioning playwright mcp =="
rg -n '"`@playwright/mcp`"|"playwright-mcp"|PLAYWRIGHT_MCP_VERSION' -S --glob '!node_modules/**' --glob '!frontend/node_modules/**' frontend package.json scripts
echoecho"== scripts/explore.sh version assignment and mcp config callers =="
nl -ba scripts/explore.sh | sed -n '60,80p'
nl -ba scripts/explore.sh | sed -n '370,390p'echoecho"== local prompt copy =="
cat -n scripts/explore/explorer-prompt.md | sed -n '1,20p'

Repository: SaplingLearn/Sapling

Length of output: 587


🌐 Web query:

site:github.com/microsoft/playwright-mcp "allowedOrigins" "blockedByClient" "ERR_BLOCKED_BY_CLIENT"

💡 Result:

The error ERR_BLOCKED_BY_CLIENT in the context of Playwright MCP (Model Context Protocol) is a browser-level error that occurs when the browser is instructed to navigate to or request a resource that has been explicitly blocked or is unreachable due to configuration issues [1][2]. In the specific context of the Playwright MCP server, this error frequently arises from the following scenarios: 1. Origin Filtering: If you have configured --allowed-origins or --blocked-origins, the browser will block navigation to any origin not in the allowlist or present in the blocklist [1]. Note that these flags affect navigation but do not serve as a CORS security boundary [1][3]. 2. Extension ID Mismatch: A common cause for ERR_BLOCKED_BY_CLIENT in recent versions (e.g., v0.0.66–0.0.68) is a hardcoded Chrome extension ID in the MCP server's connection logic [2]. If you are using an unpacked extension, a locally built version, or a different browser channel, your extension will have a different ID than the one hardcoded (mmlmfjhmonkocbjadbfplnigmagldckm). This causes the browser to attempt to connect to a non-existent extension, resulting in the error [2]. 3. Configuration Bugs: Bugs in how origin patterns are parsed—such as incorrect handling of wildcard ports (e.g., http://localhost:*)—can lead to requests being unexpectedly blocked [4]. To resolve or debug this error: - Verify your --allowed-origins and --blocked-origins configurations to ensure they match the intended URLs [1][4]. - If using an unpacked browser extension, ensure the extension ID is handled correctly, potentially by setting the PLAYWRIGHT_MCP_EXTENSION_ID environment variable or pinning your extension's ID via the manifest's "key" property [2]. - Check the official issue tracker for your version of Playwright MCP, as specific releases have included fixes for origin pattern parsing and connection logic [4][2][5].

Citations:


🌐 Web query:

github.com/microsoft/playwright-mcp program.ts "allowed-origins" "allowedOrigins" "blockedByClient"

💡 Result:

In the Playwright MCP server, allowedOrigins and blockedOrigins are configuration options used to control which network requests the browser is permitted to make [1][2]. These settings are managed within the server's network configuration and are implemented via Playwright's routing capabilities [3][4]. When these rules are active, the server intercepts network requests and uses the 'blockedbyclient' error code to terminate requests that do not align with the defined allow/block lists [3][4]. Specifically, the implementation follows this logic: 1. If allowedOrigins are defined, the server first blocks all requests by default using route.abort('blockedbyclient') [3][5]. 2. It then explicitly allows requests to the specified origins by calling route.continue for patterns matching the allowed list [3][4]. 3. If blockedOrigins are defined, the server explicitly blocks requests to those origins using route.abort('blockedbyclient') [3][4]. Origins matching both allowedOrigins and blockedOrigins will be blocked [1][2]. The allowed-origins command-line argument accepts a semicolon-separated list of origins, and the configuration supports full origin strings (e.g., https://example.com:8080) and wildcard port patterns (e.g., http://localhost:*) [1][6]. These settings are considered convenience features for managing browser interactions rather than strict security boundaries [1][2]. The file program.ts within the Playwright repository is involved in managing the command-line interface and tool configuration for the MCP server [7].

Citations:


Reinforce the localhost-first prompt with a technical restriction.

This harness requires localhost-only navigation, but mcp.json relies only on scripts/explore/explorer-prompt.md:13 for enforcement. Add a Playwright MCP restriction such as --allowed-origins http://localhost:3000;http://localhost:5000;127.0.0.1:5000 or --blocked-origins * plus the localhost allowlist, while documenting that these flags are not a security boundary against redirects; this gives defense-in-depth against model-driven or page-driven off-localhost clicks.

🤖 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/explore.sh` around lines 246 - 261, Update write_mcp_config to pass
Playwright MCP localhost-only origin restrictions in the generated args, using
the required localhost/127.0.0.1 allowlist or equivalent blocked-origins plus
allowlist configuration. Document alongside the configuration that these flags
are defense-in-depth only and do not prevent redirects from reaching
off-localhost destinations.

Comment threadscripts/explore.sh
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

Code review

Found 3 issues:

  1. do_down() can leak the stack and the machine-singleton lock: it runs under set -euo pipefail and fires as an EXIT trap, but its mkdir -p, the findings-file bootstrap, and the { … } >> findings.md append block have no || true guards (unlike the oracle subshell and e2e-down.sh lines around them). A failed write there (disk full, read-only fs) aborts the trap before e2e-down.sh and stop_lock_holder run — verified bash semantics: a failing command inside an EXIT trap under set -e stops the trap. The header comment's "|| true guards throughout" claim is currently false for these lines.

do_down() {
mkdir -p "$EXPLORE_DIR"
(
cd"$REPO_ROOT/backend"
venv/bin/python -m e2e_oracles --json >"$EXPLORE_DIR/oracle-final.json"||true
venv/bin/python -m e2e_oracles >"$EXPLORE_DIR/oracle-final.txt"2>&1||true
) ||true
[ -f"$EXPLORE_DIR/findings.md" ] ||printf'# Exploration findings\n'>"$EXPLORE_DIR/findings.md"
{
printf'\n## Oracle final pass (%s)\n\n```\n'"$(date -Iseconds)"
cat "$EXPLORE_DIR/oracle-final.txt"2>/dev/null ||true
printf'```\n'
} >>"$EXPLORE_DIR/findings.md"
"$REPO_ROOT/scripts/e2e-down.sh"||true
stop_lock_holder
}

  1. Dummy-key mitigation for RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439 defeats itself in the common developer environment: export GEMINI_API_KEY="${GEMINI_API_KEY:-e2e-dummy-key-no-billing}" defers to an ambient real key, and developers here routinely keep one exported (validate_config fail-closes without it). The below-seam RAG/embedding path (RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439) bills live even in function mode, and the explorer's mission (uploads, quiz, tutor turns, many turns) is worst-case volume for it. CI applies the dummy key unconditionally for exactly this reason; export inside the script can't touch the operator's parent shell, so the conditional protects nothing.

# Default only if unset — never clobber an operator's real key.
export GEMINI_API_KEY="${GEMINI_API_KEY:-e2e-dummy-key-no-billing}"
"$REPO_ROOT/scripts/e2e-up.sh"|| die "scripts/e2e-up.sh failed — see .e2e/*.log"

  1. jq is a hard dependency of the transcript pipeline but is missing from preflight (claude/npx/curl/python3 are checked). Without it the script boots the entire stack — Supabase, migrations, a production Next build, minutes of work — and only then fails in run_explorer, instead of failing fast like the other four checks.

echo"▶ Preflight…"
command -v claude >/dev/null 2>&1|| die "claude CLI not found on PATH — install it first"
command -v npx >/dev/null 2>&1|| die "npx not found on PATH — install Node.js first"
command -v curl >/dev/null 2>&1|| die "curl not found on PATH"
command -v python3 >/dev/null 2>&1|| die "python3 not found on PATH"
[ -f"$REPO_ROOT/scripts/explore/explorer-prompt.md" ] \

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

…eflight, scoped Edit, EXPLORE_USER wiring (#399)
- do_down: guard every fallible write with || true so a failed findings.md
write can never abort the EXIT trap before e2e-down.sh / stop_lock_holder
run (was leaking the stack + machine-singleton lock on write failure).
- GEMINI_API_KEY: export unconditionally (matches CI's dummy, #439) instead
of deferring to an ambient real key that can bill the below-seam RAG path.
- SEED_RICH=1 exported unconditionally so an ambient SEED_RICH=0 can't
silently defeat the rich dataset this harness requires.
- preflight: add missing `jq` check (hard dependency of the transcript
pipeline) so a missing jq fails in seconds, not after a full stack boot.
- allowedTools: replace unscoped Write,Edit with Read,Edit(.explore/**) —
Write(...) patterns aren't matched by the CLI's file permission check, so
only a path-scoped Edit rule actually restricts writes to findings.md.
do_up now pre-creates .explore/findings.md so the explorer always has an
existing file for the scoped Edit grant.
- EXPLORE_USER: derive the sapling_user display name from the user id
(case map for the five seeded rich-* users, verified against
db/seed_local_rich.py) instead of hardcoding "Rich Active"; pass
--user "$EXPLORE_USER" to both oracle invocations in do_down.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

All three review findings fixed in a0360ff (do_down teardown/lock-release guards verified with a real write-failure injection; unconditional dummy GEMINI_API_KEY per #439; jq preflight), plus three panel-verified hardening items in the same commit: allowedTools Write dropped / Edit scoped to .explore/**, EXPLORE_USER wired through the name map and both oracle --user calls, unconditional SEED_RICH=1. Scoped re-review confirms all addressed, no new breakage.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(agent): LLM explorer harness driving Playwright against the local stack

1 participant

@AndresL230
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(e2e): Chapter 2 explore harness — make explore + /explore skill (#399) - #444

Merged
AndresL230 merged 9 commits into
mainfrom
e2e-chapter2-explore
Jul 28, 2026
Merged

feat(e2e): Chapter 2 explore harness — make explore + /explore skill (#399)#444
AndresL230 merged 9 commits into
mainfrom
e2e-chapter2-explore

Conversation

@AndresL230

@AndresL230AndresL230 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Chapter 2 of the E2E program (epic #403), PR 2 of 3. Closes#399. Builds on the #400 oracle module (PR #443).

make explore boots the deterministic local stack and hands a Claude Code explorer a real browser; the oracles judge what it saw. Local-only by design — never a CI gate.

What's here

  • scripts/explore.sh (+ make explore / make explore-down, .explore/ gitignored) — the full pipeline: machine-singleton stack lock (detached holder process writing self-identifying lock.ok only after flock succeeds — a failed acquisition touches nothing, proven by an A/B two-session simulation) → e2e-up in function mode with a dummy GEMINI_API_KEY (RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439) → storage state minted via POST /api/auth/test-login through the frontend origin (cookie + sapling_user localStorage, secure flag matching the backend's actual local policy) → claude -p with a pinned @playwright/mcp@0.0.78 config and turn budget, transcript captured via stream-json + jq into a grep-able session.log → oracle final pass appended to findings.md → teardown. up/down subcommands support the interactive flow.
  • scripts/explore/explorer-prompt.md — the mission briefing: student persona (Rich Active), break-things mandate, report-never-fix, oracle cadence (cd backend && venv/bin/python -m e2e_oracles), findings format, known-bugs list, and a "stub it before you dig" rule earned in live testing.
  • .claude/skills/explore/SKILL.md — the interactive /explore skill: same pipeline, watchable and steerable, with a same-origin JS sign-in for live-browser sessions.

Hard-won fixes from live acceptance runs

  • @playwright/mcp without --isolated reuses a persistent profile and silently ignores --storage-state — the run-2 "signed-in" state was a stale-cookie artifact; --isolated fixes it (verified down to the bundled playwright-core source and a cookie-DB probe).
  • claude -p's default text output prints only the final message — a turn-budget exit produced an empty transcript despite real browsing; --output-format stream-json --verbose + jq restores the readable transcript.
  • Lock-holder bookkeeping survives concurrent sessions (busy attempt leaves the running session's files byte-identical).

Acceptance (issue #399 bar, definitive run in the SDD report)

One bounded run (EXPLORE_MAX_TURNS=25, 3m52s): real signed-in navigation across dashboard + library ("Rich Active" nav, 4 seeded documents, CS101/MATH210/BIO110/ENG150 visible in snapshots), 2 explorer-written findings — including a novel wrong-data upload bug and a tutor-resume 500 (concept_describe unregistered in the function-handler seam) independently confirmed by the oracle logscan — plus the harness-appended Oracle final pass re-confirming #355. Machine verified clean afterward (ports, containers, lock).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added an end-to-end exploratory testing workflow for the local application.
    • Added commands to start, stop, and run complete exploration sessions.
    • Exploration sessions now support browser-driven testing, automated checks, session logs, and structured findings.
  • Documentation

    • Added guidance covering exploration steps, reporting requirements, known issues, and session summaries.
  • Chores

    • Exploration artifacts are now excluded from version control.

AndresL230and others added 8 commits July 28, 2026 01:13
…itignore (#399)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e, oracle cadence (#399)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…er GEMINI_API_KEY (#399)
A busy lock previously still tripped do_up's trap/wipe path, tearing down
another session's live stack via scripts/e2e-down.sh and deleting its
.explore/ artifacts before the lock check ever ran. start_lock_holder now
runs first, the do_down safety trap arms only after the lock is held, the
.explore/ wipe (still selective, preserving lock.pid/lock.ok) happens after
that, and a failed acquisition cleans up its own holder/pid files before
exiting. GEMINI_API_KEY now defaults only when unset instead of always
overwriting an operator's real key.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ns (#399)
start_lock_holder previously deleted lock.ok and unconditionally wrote
lock.pid before knowing whether the flock would actually succeed. A failed
attempt from session B (lock held by session A) would delete A's lock.ok and
overwrite A's lock.pid with B's own doomed PID, then B's busy-path cleanup
removed the file entirely — leaving A's later `down` unable to find A's
holder, leaking both the detached process and the machine-singleton lock.
Now the holder subprocess itself is the only writer of lock.ok, and only
ever after its own `flock -n 9` succeeds (atomic temp-file + mv, content =
the holder's own $$). The parent only polls for that self-identifying
marker and touches nothing on disk on the failure path, so a busy lock can
never clobber another session's bookkeeping. lock.pid is retired — lock.ok's
content is now the sole source of truth stop_lock_holder reads.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The first bounded acceptance run (task-8-report.md) produced a
session.log with exactly one line — claude -p's own "Error: Reached
max turns" — because the default --output-format=text only prints the
FINAL message, never the intermediate tool_use/tool_result turns. That
failed the #399 acceptance bar ("session.log shows real Playwright MCP
tool calls").
Switch run_explorer to --output-format stream-json --verbose (the CLI
requires both together) and reformat the JSONL with jq into readable
[assistant]/[tool_use]/[tool_result]/[result] lines, truncated to 500
chars each, so session.log is both grep-able and human-skimmable.
fromjson? tolerates stray non-JSON lines instead of aborting the whole
transcript; claude -p's stderr now goes to its own session.stderr.log
so it can never interleave with (and corrupt) the JSON stream jq
parses.
Verified: a second bounded run (EXPLORE_MAX_TURNS=25) produced a
281-line session.log with 18 mcp__playwright__* tool calls across
/dashboard and /library, plus findings.md with a re-confirmed #430
repro and the oracle final pass re-confirming #355.
The follow-up bounded-run diagnosis found a contradiction: run 2's
session cookie was accepted (/api/users 200) but the UI acted fully
signed-out (Sign In button, dashboard skeleton) — the #430 symptom.
Run 1's "Secure cookie dropped over http" diagnosis didn't hold up
either, since Chromium does accept Secure cookies on http://localhost.
Root-caused by reading @playwright/mcp@0.0.78's bundled source
(playwright-core/lib/coreBundle.js): without --isolated, the MCP
server launches ONE PERSISTENT Chrome profile keyed only by
sha256(cwd) — reused across every explore.sh run from this checkout,
never wiped by teardown — and its client factory does
`config.browser.isolated ? await browser.newContext(contextOptions) :
browser.contexts()[0]`. In the default (non-isolated) branch it just
grabs the already-open persistent context and never calls newContext
with our --storage-state at all.
Verified empirically: wiped the profile dir, booted a clean stack, and
drove a 3-turn claude -p probe against the (then-current) mcp.json —
navigating to /dashboard redirected straight to a REAL
accounts.google.com sign-in page, and sqlite3 on the profile's Cookies
db afterward showed zero sapling_session rows (neither cookie nor
localStorage from storageState.json was ever applied). The identical
probe with --isolated added rendered the real, fully-authenticated
dashboard on the first navigation, with localStorage.sapling_user
correctly present — this is the same mechanism (ephemeral
browser.newContext(contextOptions)) @playwright/test itself uses in
Chapter 1's global-setup.ts.
Also corrected mint_storage_state's cookie to secure:false, matching
what the backend's own SECURE_COOKIES policy actually issues for the
http://localhost local stack (config.py derives it from FRONTEND_URL's
scheme) — the file should mirror reality regardless of which flag
turned out to be the load-bearing one.
Verified: EXPLORE_MAX_TURNS=12 make explore reached a signed-in
dashboard (nav shows "Rich Active" / "Account", full authenticated
menu) using only 2 real Playwright tool calls, zero sign-in recovery
turns.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The definitive acceptance run (harness fixes verified: real auth via
--isolated, breadth across 2+ surfaces) hit a genuine new bug —
resuming a tutor session 500'd on POST /api/graph/.../concept-description,
root-caused via the explorer's own backend-log investigation to a
missing SAPLING_FUNCTION_HANDLERS registration for 'concept_describe'
(caught independently by the oracle's logscan pass too, so it's not
lost, just not explorer-authored). But the explorer spent its
remaining turns tailing logs and checking processes to nail the exact
LookupError, and ran out of budget before writing an F<N> entry to
findings.md — a real gap against #399's "readable transcript AND
findings file" bar, distinct from any flag/mechanism defect.
Add a "stub it before you dig" ground rule: write a one-line stub
finding the instant something looks off, before further root-causing.
A written stub survives a turn-budget cutoff; a perfect unwritten
diagnosis does not.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:34 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: dc0a75e3-31f4-42be-8ec2-bb9a848919bb

📥 Commits

Reviewing files that changed from the base of the PR and between 914b0ae and a0360ff.

📒 Files selected for processing (1)
  • scripts/explore.sh
📝 Walkthrough

Walkthrough

Adds a locked local exploratory-testing workflow that boots the E2E stack, authenticates a seeded browser session, runs Claude through Playwright MCP, records findings and oracle output, and reliably tears down the stack.

Changes

Exploration harness

Layer / File(s)Summary
Exploration contract and entrypoints
.claude/skills/explore/SKILL.md, scripts/explore/explorer-prompt.md, Makefile, .gitignore
Defines the exploration workflow, reporting constraints, findings format, operator targets, and ignored .explore/ artifacts.
Locked stack setup
scripts/explore.sh
Adds prerequisite checks, repository configuration, detached flock locking, and local E2E stack startup.
Authenticated browser session
scripts/explore.sh
Creates seeded-user Playwright storage state, writes the pinned MCP configuration, and runs Claude with structured logs and separate stderr output.
Oracle and teardown lifecycle
scripts/explore.sh
Supports full, up, and down modes, appends final oracle results, tears down the stack, and releases the lock.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
participant Operator
participant explore.sh
participant E2EStack
participant AuthTestEndpoint
participant PlaywrightMCP
participant Browser
participant Claude
Operator->>explore.sh: start exploration
explore.sh->>E2EStack: boot local E2E stack
explore.sh->>AuthTestEndpoint: POST seeded student login
explore.sh->>PlaywrightMCP: configure storage state and MCP
Claude->>PlaywrightMCP: execute exploration tools
PlaywrightMCP->>Browser: drive localhost browser
explore.sh->>E2EStack: run oracle and teardown
Loading

Possibly related issues

  • Issue 403 — Covers the broader Chapter 2 AI Agent Explorer epic implemented by this harness.
  • Issue 401 — Proposes nightly automation and regression promotion for findings produced by this exploration workflow.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly names the new explore harness and /explore skill and matches the main change.
Description check✅ PassedThe description covers summary, changes, related issue, testing, screenshots, and reviewer notes with mostly complete content.
Linked Issues check✅ PassedThe PR implements the local Playwright explorer harness, seeded auth, hard budget/localhost limits, structured logs, and readable session output required by #399.
Out of Scope Changes check✅ PassedThe changes stay focused on the explore harness, prompt, Makefile, and ignore rules; no unrelated functionality is introduced.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch e2e-chapter2-explore

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Jul 28, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-staginga0360ffCommit Preview URL

Branch Preview URL
Jul 28 2026, 10:20 AM

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
scripts/explore.sh (1)

149-154: 🩺 Stability & Availability | 🔵 Trivial

stop_lock_holder/do_down release the machine lock without verifying the caller owns it.

do_down (and thus scripts/explore.sh down / make explore-down) unconditionally reads lock.ok's pid and kills it, regardless of whether the invoking shell is the one that acquired it. The elaborate ownership-safety work in start_lock_holder (a failed attempt never touches lock.ok) isn't mirrored here: since the lock is uid-namespaced (/tmp/claude-$(id -u)/...), the realistic blast radius is a single operator accidentally running down from a second terminal while up is still driving an interactive session elsewhere on the same machine — which would yank the shared local stack out from under that live session with no warning. Given the doc explicitly anticipates the "interactive flow" (separate up/down invocations) as a supported mode, this seems worth at least a guard (e.g. warn if a stack that looks actively in-use by claude is being torn down) or an explicit doc callout of the risk.

Also applies to: 404-419

🤖 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/explore.sh` around lines 149 - 154, Update stop_lock_holder and the
do_down flow to guard against tearing down a lock owned by another active
interactive session. Before killing the PID from lock.ok, detect whether it
represents an active claude-managed stack and warn or require explicit
confirmation when ownership cannot be established; preserve cleanup for the
invoking owner and document the supported separate up/down behavior if that is
the chosen safeguard.
🤖 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 `@scripts/explore.sh`:
- Around line 176-202: Update the localStorage payload in mint_storage_state so
the sapling_user name is derived from $EXPLORE_USER instead of hardcoding "Rich
Active". Preserve the existing user ID and other payload fields while ensuring
any configured seeded rich-* user receives a matching cached display name.
- Around line 302-337: Restrict the Write/Edit entries in run_explorer() to the
permitted .explore findings path, using path-scoped rules such as
Edit(.explore/**/*.md) and removing blanket file-edit permissions. Also verify
the current Claude CLI Bash permission syntax for the cd backend &&
venv/bin/python -m e2e_oracles:* rule, tightening it if compound-command parsing
permits commands outside the intended prefix.
- Around line 246-261: Update write_mcp_config to pass Playwright MCP
localhost-only origin restrictions in the generated args, using the required
localhost/127.0.0.1 allowlist or equivalent blocked-origins plus allowlist
configuration. Document alongside the configuration that these flags are
defense-in-depth only and do not prevent redirects from reaching off-localhost
destinations.
---
Nitpick comments:
In `@scripts/explore.sh`:
- Around line 149-154: Update stop_lock_holder and the do_down flow to guard
against tearing down a lock owned by another active interactive session. Before
killing the PID from lock.ok, detect whether it represents an active
claude-managed stack and warn or require explicit confirmation when ownership
cannot be established; preserve cleanup for the invoking owner and document the
supported separate up/down behavior if that is the chosen safeguard.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 762bcd62-0ca2-4e80-bacf-4434944c876d

📥 Commits

Reviewing files that changed from the base of the PR and between db0ecad and 914b0ae.

📒 Files selected for processing (5)
  • .claude/skills/explore/SKILL.md
  • .gitignore
  • Makefile
  • scripts/explore.sh
  • scripts/explore/explorer-prompt.md

Comment threadscripts/explore.sh
Comment threadscripts/explore.sh
Comment on lines +246 to +261
write_mcp_config() {
local headless_args='"--headless", '
[ "$EXPLORE_HEADED" = "1" ] && headless_args=''
cat > "$EXPLORE_DIR/mcp.json" <<EOF
{
"mcpServers": {
"playwright": {
"command": "npx",
"args": ["-y", "@playwright/mcp@$PLAYWRIGHT_MCP_VERSION", "--isolated", ${headless_args}"--browser", "chromium",
"--storage-state", "$EXPLORE_DIR/storageState.json",
"--output-dir", "$EXPLORE_DIR/traces", "--save-session"]
}
}
}
EOF
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== locate files =="
fd -a 'explore.sh|explorer-prompt.md|package.json|pnpm-lock.yaml|package-lock.json|yarn.lock|playwright|mcp.json'.| sed 's#^\./##'| head -200
echoecho"== relevant explore.sh lines =="if [ -f scripts/explore.sh ];then
nl -ba scripts/explore.sh | sed -n '220,280p'fiechoecho"== relevant explorer-prompt lines =="if [ -f scripts/explore/explorer-prompt.md ];then
nl -ba scripts/explore/explorer-prompt.md | sed -n '1,35p'fiif [ -f explorer-prompt.md ];then
nl -ba explorer-prompt.md | sed -n '1,35p'fiechoecho"== search required navigation text and mcp config args =="
rg -n "Restrict navigation|localhost|localhost-only|localhost:3000|allowed-origins|blocked-origins|explorer-prompt|MISSION|write_mcp_config|PLAYWRIGHT_MCP_VERSION" -S .

Repository: SaplingLearn/Sapling

Length of output: 479


🌐 Web query:

Playwright MCP 0.0.78 README --allowed-origins --blocked-origins

💡 Result:

In Playwright MCP, the --allowed-origins and --blocked-origins flags (and their corresponding environment variables) are used to manage network access for the browser instance [1][2][3]. Both settings accept a semicolon-separated list of origins [1][4][3]. Key details: --allowed-origins This flag specifies a list of trusted origins that the browser is permitted to request [1][5]. By default, all origins are allowed [1][2]. Environment variable: PLAYWRIGHT_MCP_ALLOWED_ORIGINS [1][3]. --blocked-origins This flag specifies a list of origins to block the browser from requesting [1][5]. The blocklist is evaluated before the allowlist; if an origin is present in both, it will be blocked [1][2][5]. Environment variable: PLAYWRIGHT_MCP_BLOCKED_ORIGINS [1][3]. Important Security Note: Documentation explicitly states that these flags do not serve as a security boundary and do not affect redirects [1][5][3]. Supported Formats: Both flags support the following formats [1][2]: Full origin: e.g., https://example.com:8080 (matches only that specific origin) [1][2]. Wildcard port: e.g., http://localhost:* (matches any port on localhost with the http protocol) [1][2].

Citations:


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== relevant explore.sh lines =="if [ -f scripts/explore.sh ];then
sed -n '220,280p' scripts/explore.sh | cat -n
fiechoecho"== relevant explorer-prompt lines =="if [ -f scripts/explore/explorer-prompt.md ];then
sed -n '1,40p' scripts/explore/explorer-prompt.md | cat -n
fiechoecho"== search required navigation text and mcp config args =="
rg -n "Restrict navigation|localhost|localhost-only|localhost:3000|allowed-origins|blocked-origins|explorer-prompt|MISSION|write_mcp_config|PLAYWRIGHT_MCP_VERSION" -S .echoecho"== lockfile version snippets =="forfin package-lock.json frontend/package-lock.json;doif [ -f"$f" ];thenecho"--- $f ---"
rg -n '"`@playwright/mcp`"|"`@playwright/test`"|PLAYWRIGHT_MCP_VERSION'"$f" -A 3 -B 2 ||truefidone

Repository: SaplingLearn/Sapling

Length of output: 22370


🌐 Web query:

playwright mcp --allowed-origins blocked navigation error network error

💡 Result:

The "blocked navigation error" or network-related errors when using the --allowed-origins flag in Playwright MCP typically stem from configuration format issues, regex/glob matching errors, or confusing the flag with server-side CORS settings [1][2][3]. Key troubleshooting steps include: 1. Use Correct Origin Format: Ensure you are passing the domain or origin directly (e.g., google.com) rather than including protocol prefixes like https:// or trailing slashes, which have historically caused parsing issues [1]. 2. Avoid Invalid Glob Patterns: Recent versions (as of early 2026) have had bugs where wildcard port patterns like http://localhost:* generate invalid internal glob patterns, causing the browser to block all traffic to those destinations [3]. If you are experiencing this, you may need to use explicit hostnames or check for updates addressing the originOrHostGlob function [3]. 3. Distinguish Between Flags: Ensure you are not confusing --allowed-origins (which controls browser navigation within the sandboxed Playwright instance) with --allowed-hosts (which controls MCP server CORS/connectivity) [2]. Both may need to be set identically if you are navigating to localhost in restricted environments like GitHub Actions or WSL [2][4]. 4. Blocklist Precedence: Keep in mind that Playwright MCP evaluates the blocked-origins list before the allowed-origins list. If a URL matches both, it will be blocked [5]. 5. Debugging Network Issues: If navigation still fails with ERR_CONNECTION_REFUSED, verify that the target service (e.g., a local dev server) is listening on 0.0.0.0 rather than just 127.0.0.1 to ensure it is accessible to the browser process, especially if running within containers (Docker) or WSL [2][4]. For further debugging, you can enable verbose logging by setting the DEBUG environment variable to pw:api or pw:browser* to see exactly which origin is failing to match the navigation request [4].

Citations:


🌐 Web query:

Playwright MCP blocked origins error "Failed to navigate to"

💡 Result:

The error net::ERR_BLOCKED_BY_CLIENT when using Playwright MCP typically indicates that the browser navigation was blocked by a client-side security policy, configuration, or environment restriction [1]. Because Playwright MCP is often used in automated contexts (such as LLM agents), this error is usually triggered by one of the following factors [1]: 1. Allowed Origins Configuration: If you have configured allowed-origins for security, the destination URL may be excluded from the allowlist [2][3]. Verify that the PW_ALLOWED_ORIGINS environment variable or the --allowed-origins CLI flag is correctly set to include the domain you are attempting to visit [2][3]. 2. Browser Environment and Sandbox Restrictions: When running in containerized environments like Docker, Chromium often fails to navigate because it cannot access the necessary system sandbox [1]. To resolve this, you may need to pass explicit launch flags to bypass sandbox restrictions in your PLAYWRIGHT_LAUNCH_OPTIONS [1]: --no-sandbox --disable-setuid-sandbox --disable-gpu 3. Security Settings or Extensions: Although Playwright MCP sessions are typically isolated, ERR_BLOCKED_BY_CLIENT can also occur if the browser launch arguments include restrictive security settings or if there are conflicts with other security-related flags [1]. 4. Troubleshooting Steps: - Check the logs to confirm the specific URL being blocked [1]. - If you are running in Docker, ensure the container has the necessary privileges and sandbox bypass flags [1]. - Review your MCP configuration file (often found in .mcp/config.json or equivalent) to ensure --allowed-origins is formatted correctly (e.g., comma-separated or semicolon-separated depending on your specific implementation) [4][2][3]. - If you suspect the issue is not related to origins, verify that npx and the Playwright runtime are correctly installed and accessible in your environment, as missing dependencies can sometimes lead to unexpected browser behavior [5]. If the error persists despite correct origin configuration, it is often helpful to test a basic navigation command outside of the MCP wrapper to rule out environment-specific sandbox or policy issues [1].

Citations:


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== package manifests mentioning playwright mcp =="
rg -n '"`@playwright/mcp`"|"playwright-mcp"|PLAYWRIGHT_MCP_VERSION' -S --glob '!node_modules/**' --glob '!frontend/node_modules/**' frontend package.json scripts
echoecho"== scripts/explore.sh version assignment and mcp config callers =="
nl -ba scripts/explore.sh | sed -n '60,80p'
nl -ba scripts/explore.sh | sed -n '370,390p'echoecho"== local prompt copy =="
cat -n scripts/explore/explorer-prompt.md | sed -n '1,20p'

Repository: SaplingLearn/Sapling

Length of output: 587


🌐 Web query:

site:github.com/microsoft/playwright-mcp "allowedOrigins" "blockedByClient" "ERR_BLOCKED_BY_CLIENT"

💡 Result:

The error ERR_BLOCKED_BY_CLIENT in the context of Playwright MCP (Model Context Protocol) is a browser-level error that occurs when the browser is instructed to navigate to or request a resource that has been explicitly blocked or is unreachable due to configuration issues [1][2]. In the specific context of the Playwright MCP server, this error frequently arises from the following scenarios: 1. Origin Filtering: If you have configured --allowed-origins or --blocked-origins, the browser will block navigation to any origin not in the allowlist or present in the blocklist [1]. Note that these flags affect navigation but do not serve as a CORS security boundary [1][3]. 2. Extension ID Mismatch: A common cause for ERR_BLOCKED_BY_CLIENT in recent versions (e.g., v0.0.66–0.0.68) is a hardcoded Chrome extension ID in the MCP server's connection logic [2]. If you are using an unpacked extension, a locally built version, or a different browser channel, your extension will have a different ID than the one hardcoded (mmlmfjhmonkocbjadbfplnigmagldckm). This causes the browser to attempt to connect to a non-existent extension, resulting in the error [2]. 3. Configuration Bugs: Bugs in how origin patterns are parsed—such as incorrect handling of wildcard ports (e.g., http://localhost:*)—can lead to requests being unexpectedly blocked [4]. To resolve or debug this error: - Verify your --allowed-origins and --blocked-origins configurations to ensure they match the intended URLs [1][4]. - If using an unpacked browser extension, ensure the extension ID is handled correctly, potentially by setting the PLAYWRIGHT_MCP_EXTENSION_ID environment variable or pinning your extension's ID via the manifest's "key" property [2]. - Check the official issue tracker for your version of Playwright MCP, as specific releases have included fixes for origin pattern parsing and connection logic [4][2][5].

Citations:


🌐 Web query:

github.com/microsoft/playwright-mcp program.ts "allowed-origins" "allowedOrigins" "blockedByClient"

💡 Result:

In the Playwright MCP server, allowedOrigins and blockedOrigins are configuration options used to control which network requests the browser is permitted to make [1][2]. These settings are managed within the server's network configuration and are implemented via Playwright's routing capabilities [3][4]. When these rules are active, the server intercepts network requests and uses the 'blockedbyclient' error code to terminate requests that do not align with the defined allow/block lists [3][4]. Specifically, the implementation follows this logic: 1. If allowedOrigins are defined, the server first blocks all requests by default using route.abort('blockedbyclient') [3][5]. 2. It then explicitly allows requests to the specified origins by calling route.continue for patterns matching the allowed list [3][4]. 3. If blockedOrigins are defined, the server explicitly blocks requests to those origins using route.abort('blockedbyclient') [3][4]. Origins matching both allowedOrigins and blockedOrigins will be blocked [1][2]. The allowed-origins command-line argument accepts a semicolon-separated list of origins, and the configuration supports full origin strings (e.g., https://example.com:8080) and wildcard port patterns (e.g., http://localhost:*) [1][6]. These settings are considered convenience features for managing browser interactions rather than strict security boundaries [1][2]. The file program.ts within the Playwright repository is involved in managing the command-line interface and tool configuration for the MCP server [7].

Citations:


Reinforce the localhost-first prompt with a technical restriction.

This harness requires localhost-only navigation, but mcp.json relies only on scripts/explore/explorer-prompt.md:13 for enforcement. Add a Playwright MCP restriction such as --allowed-origins http://localhost:3000;http://localhost:5000;127.0.0.1:5000 or --blocked-origins * plus the localhost allowlist, while documenting that these flags are not a security boundary against redirects; this gives defense-in-depth against model-driven or page-driven off-localhost clicks.

🤖 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/explore.sh` around lines 246 - 261, Update write_mcp_config to pass
Playwright MCP localhost-only origin restrictions in the generated args, using
the required localhost/127.0.0.1 allowlist or equivalent blocked-origins plus
allowlist configuration. Document alongside the configuration that these flags
are defense-in-depth only and do not prevent redirects from reaching
off-localhost destinations.

Comment threadscripts/explore.sh
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

Code review

Found 3 issues:

  1. do_down() can leak the stack and the machine-singleton lock: it runs under set -euo pipefail and fires as an EXIT trap, but its mkdir -p, the findings-file bootstrap, and the { … } >> findings.md append block have no || true guards (unlike the oracle subshell and e2e-down.sh lines around them). A failed write there (disk full, read-only fs) aborts the trap before e2e-down.sh and stop_lock_holder run — verified bash semantics: a failing command inside an EXIT trap under set -e stops the trap. The header comment's "|| true guards throughout" claim is currently false for these lines.

do_down() {
mkdir -p "$EXPLORE_DIR"
(
cd"$REPO_ROOT/backend"
venv/bin/python -m e2e_oracles --json >"$EXPLORE_DIR/oracle-final.json"||true
venv/bin/python -m e2e_oracles >"$EXPLORE_DIR/oracle-final.txt"2>&1||true
) ||true
[ -f"$EXPLORE_DIR/findings.md" ] ||printf'# Exploration findings\n'>"$EXPLORE_DIR/findings.md"
{
printf'\n## Oracle final pass (%s)\n\n```\n'"$(date -Iseconds)"
cat "$EXPLORE_DIR/oracle-final.txt"2>/dev/null ||true
printf'```\n'
} >>"$EXPLORE_DIR/findings.md"
"$REPO_ROOT/scripts/e2e-down.sh"||true
stop_lock_holder
}

  1. Dummy-key mitigation for RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439 defeats itself in the common developer environment: export GEMINI_API_KEY="${GEMINI_API_KEY:-e2e-dummy-key-no-billing}" defers to an ambient real key, and developers here routinely keep one exported (validate_config fail-closes without it). The below-seam RAG/embedding path (RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439) bills live even in function mode, and the explorer's mission (uploads, quiz, tutor turns, many turns) is worst-case volume for it. CI applies the dummy key unconditionally for exactly this reason; export inside the script can't touch the operator's parent shell, so the conditional protects nothing.

# Default only if unset — never clobber an operator's real key.
export GEMINI_API_KEY="${GEMINI_API_KEY:-e2e-dummy-key-no-billing}"
"$REPO_ROOT/scripts/e2e-up.sh"|| die "scripts/e2e-up.sh failed — see .e2e/*.log"

  1. jq is a hard dependency of the transcript pipeline but is missing from preflight (claude/npx/curl/python3 are checked). Without it the script boots the entire stack — Supabase, migrations, a production Next build, minutes of work — and only then fails in run_explorer, instead of failing fast like the other four checks.

echo"▶ Preflight…"
command -v claude >/dev/null 2>&1|| die "claude CLI not found on PATH — install it first"
command -v npx >/dev/null 2>&1|| die "npx not found on PATH — install Node.js first"
command -v curl >/dev/null 2>&1|| die "curl not found on PATH"
command -v python3 >/dev/null 2>&1|| die "python3 not found on PATH"
[ -f"$REPO_ROOT/scripts/explore/explorer-prompt.md" ] \

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

…eflight, scoped Edit, EXPLORE_USER wiring (#399)
- do_down: guard every fallible write with || true so a failed findings.md
write can never abort the EXIT trap before e2e-down.sh / stop_lock_holder
run (was leaking the stack + machine-singleton lock on write failure).
- GEMINI_API_KEY: export unconditionally (matches CI's dummy, #439) instead
of deferring to an ambient real key that can bill the below-seam RAG path.
- SEED_RICH=1 exported unconditionally so an ambient SEED_RICH=0 can't
silently defeat the rich dataset this harness requires.
- preflight: add missing `jq` check (hard dependency of the transcript
pipeline) so a missing jq fails in seconds, not after a full stack boot.
- allowedTools: replace unscoped Write,Edit with Read,Edit(.explore/**) —
Write(...) patterns aren't matched by the CLI's file permission check, so
only a path-scoped Edit rule actually restricts writes to findings.md.
do_up now pre-creates .explore/findings.md so the explorer always has an
existing file for the scoped Edit grant.
- EXPLORE_USER: derive the sapling_user display name from the user id
(case map for the five seeded rich-* users, verified against
db/seed_local_rich.py) instead of hardcoding "Rich Active"; pass
--user "$EXPLORE_USER" to both oracle invocations in do_down.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

All three review findings fixed in a0360ff (do_down teardown/lock-release guards verified with a real write-failure injection; unconditional dummy GEMINI_API_KEY per #439; jq preflight), plus three panel-verified hardening items in the same commit: allowedTools Write dropped / Edit scoped to .explore/**, EXPLORE_USER wired through the name map and both oracle --user calls, unconditional SEED_RICH=1. Scoped re-review confirms all addressed, no new breakage.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(agent): LLM explorer harness driving Playwright against the local stack

1 participant

@AndresL230
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(e2e): Chapter 2 explore harness — make explore + /explore skill (#399) - #444

Merged
AndresL230 merged 9 commits into
mainfrom
e2e-chapter2-explore
Jul 28, 2026
Merged

feat(e2e): Chapter 2 explore harness — make explore + /explore skill (#399)#444
AndresL230 merged 9 commits into
mainfrom
e2e-chapter2-explore

Conversation

@AndresL230

@AndresL230AndresL230 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Chapter 2 of the E2E program (epic #403), PR 2 of 3. Closes#399. Builds on the #400 oracle module (PR #443).

make explore boots the deterministic local stack and hands a Claude Code explorer a real browser; the oracles judge what it saw. Local-only by design — never a CI gate.

What's here

  • scripts/explore.sh (+ make explore / make explore-down, .explore/ gitignored) — the full pipeline: machine-singleton stack lock (detached holder process writing self-identifying lock.ok only after flock succeeds — a failed acquisition touches nothing, proven by an A/B two-session simulation) → e2e-up in function mode with a dummy GEMINI_API_KEY (RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439) → storage state minted via POST /api/auth/test-login through the frontend origin (cookie + sapling_user localStorage, secure flag matching the backend's actual local policy) → claude -p with a pinned @playwright/mcp@0.0.78 config and turn budget, transcript captured via stream-json + jq into a grep-able session.log → oracle final pass appended to findings.md → teardown. up/down subcommands support the interactive flow.
  • scripts/explore/explorer-prompt.md — the mission briefing: student persona (Rich Active), break-things mandate, report-never-fix, oracle cadence (cd backend && venv/bin/python -m e2e_oracles), findings format, known-bugs list, and a "stub it before you dig" rule earned in live testing.
  • .claude/skills/explore/SKILL.md — the interactive /explore skill: same pipeline, watchable and steerable, with a same-origin JS sign-in for live-browser sessions.

Hard-won fixes from live acceptance runs

  • @playwright/mcp without --isolated reuses a persistent profile and silently ignores --storage-state — the run-2 "signed-in" state was a stale-cookie artifact; --isolated fixes it (verified down to the bundled playwright-core source and a cookie-DB probe).
  • claude -p's default text output prints only the final message — a turn-budget exit produced an empty transcript despite real browsing; --output-format stream-json --verbose + jq restores the readable transcript.
  • Lock-holder bookkeeping survives concurrent sessions (busy attempt leaves the running session's files byte-identical).

Acceptance (issue #399 bar, definitive run in the SDD report)

One bounded run (EXPLORE_MAX_TURNS=25, 3m52s): real signed-in navigation across dashboard + library ("Rich Active" nav, 4 seeded documents, CS101/MATH210/BIO110/ENG150 visible in snapshots), 2 explorer-written findings — including a novel wrong-data upload bug and a tutor-resume 500 (concept_describe unregistered in the function-handler seam) independently confirmed by the oracle logscan — plus the harness-appended Oracle final pass re-confirming #355. Machine verified clean afterward (ports, containers, lock).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added an end-to-end exploratory testing workflow for the local application.
    • Added commands to start, stop, and run complete exploration sessions.
    • Exploration sessions now support browser-driven testing, automated checks, session logs, and structured findings.
  • Documentation

    • Added guidance covering exploration steps, reporting requirements, known issues, and session summaries.
  • Chores

    • Exploration artifacts are now excluded from version control.

AndresL230and others added 8 commits July 28, 2026 01:13
…itignore (#399)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e, oracle cadence (#399)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…er GEMINI_API_KEY (#399)
A busy lock previously still tripped do_up's trap/wipe path, tearing down
another session's live stack via scripts/e2e-down.sh and deleting its
.explore/ artifacts before the lock check ever ran. start_lock_holder now
runs first, the do_down safety trap arms only after the lock is held, the
.explore/ wipe (still selective, preserving lock.pid/lock.ok) happens after
that, and a failed acquisition cleans up its own holder/pid files before
exiting. GEMINI_API_KEY now defaults only when unset instead of always
overwriting an operator's real key.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ns (#399)
start_lock_holder previously deleted lock.ok and unconditionally wrote
lock.pid before knowing whether the flock would actually succeed. A failed
attempt from session B (lock held by session A) would delete A's lock.ok and
overwrite A's lock.pid with B's own doomed PID, then B's busy-path cleanup
removed the file entirely — leaving A's later `down` unable to find A's
holder, leaking both the detached process and the machine-singleton lock.
Now the holder subprocess itself is the only writer of lock.ok, and only
ever after its own `flock -n 9` succeeds (atomic temp-file + mv, content =
the holder's own $$). The parent only polls for that self-identifying
marker and touches nothing on disk on the failure path, so a busy lock can
never clobber another session's bookkeeping. lock.pid is retired — lock.ok's
content is now the sole source of truth stop_lock_holder reads.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The first bounded acceptance run (task-8-report.md) produced a
session.log with exactly one line — claude -p's own "Error: Reached
max turns" — because the default --output-format=text only prints the
FINAL message, never the intermediate tool_use/tool_result turns. That
failed the #399 acceptance bar ("session.log shows real Playwright MCP
tool calls").
Switch run_explorer to --output-format stream-json --verbose (the CLI
requires both together) and reformat the JSONL with jq into readable
[assistant]/[tool_use]/[tool_result]/[result] lines, truncated to 500
chars each, so session.log is both grep-able and human-skimmable.
fromjson? tolerates stray non-JSON lines instead of aborting the whole
transcript; claude -p's stderr now goes to its own session.stderr.log
so it can never interleave with (and corrupt) the JSON stream jq
parses.
Verified: a second bounded run (EXPLORE_MAX_TURNS=25) produced a
281-line session.log with 18 mcp__playwright__* tool calls across
/dashboard and /library, plus findings.md with a re-confirmed #430
repro and the oracle final pass re-confirming #355.
The follow-up bounded-run diagnosis found a contradiction: run 2's
session cookie was accepted (/api/users 200) but the UI acted fully
signed-out (Sign In button, dashboard skeleton) — the #430 symptom.
Run 1's "Secure cookie dropped over http" diagnosis didn't hold up
either, since Chromium does accept Secure cookies on http://localhost.
Root-caused by reading @playwright/mcp@0.0.78's bundled source
(playwright-core/lib/coreBundle.js): without --isolated, the MCP
server launches ONE PERSISTENT Chrome profile keyed only by
sha256(cwd) — reused across every explore.sh run from this checkout,
never wiped by teardown — and its client factory does
`config.browser.isolated ? await browser.newContext(contextOptions) :
browser.contexts()[0]`. In the default (non-isolated) branch it just
grabs the already-open persistent context and never calls newContext
with our --storage-state at all.
Verified empirically: wiped the profile dir, booted a clean stack, and
drove a 3-turn claude -p probe against the (then-current) mcp.json —
navigating to /dashboard redirected straight to a REAL
accounts.google.com sign-in page, and sqlite3 on the profile's Cookies
db afterward showed zero sapling_session rows (neither cookie nor
localStorage from storageState.json was ever applied). The identical
probe with --isolated added rendered the real, fully-authenticated
dashboard on the first navigation, with localStorage.sapling_user
correctly present — this is the same mechanism (ephemeral
browser.newContext(contextOptions)) @playwright/test itself uses in
Chapter 1's global-setup.ts.
Also corrected mint_storage_state's cookie to secure:false, matching
what the backend's own SECURE_COOKIES policy actually issues for the
http://localhost local stack (config.py derives it from FRONTEND_URL's
scheme) — the file should mirror reality regardless of which flag
turned out to be the load-bearing one.
Verified: EXPLORE_MAX_TURNS=12 make explore reached a signed-in
dashboard (nav shows "Rich Active" / "Account", full authenticated
menu) using only 2 real Playwright tool calls, zero sign-in recovery
turns.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The definitive acceptance run (harness fixes verified: real auth via
--isolated, breadth across 2+ surfaces) hit a genuine new bug —
resuming a tutor session 500'd on POST /api/graph/.../concept-description,
root-caused via the explorer's own backend-log investigation to a
missing SAPLING_FUNCTION_HANDLERS registration for 'concept_describe'
(caught independently by the oracle's logscan pass too, so it's not
lost, just not explorer-authored). But the explorer spent its
remaining turns tailing logs and checking processes to nail the exact
LookupError, and ran out of budget before writing an F<N> entry to
findings.md — a real gap against #399's "readable transcript AND
findings file" bar, distinct from any flag/mechanism defect.
Add a "stub it before you dig" ground rule: write a one-line stub
finding the instant something looks off, before further root-causing.
A written stub survives a turn-budget cutoff; a perfect unwritten
diagnosis does not.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:34 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: dc0a75e3-31f4-42be-8ec2-bb9a848919bb

📥 Commits

Reviewing files that changed from the base of the PR and between 914b0ae and a0360ff.

📒 Files selected for processing (1)
  • scripts/explore.sh
📝 Walkthrough

Walkthrough

Adds a locked local exploratory-testing workflow that boots the E2E stack, authenticates a seeded browser session, runs Claude through Playwright MCP, records findings and oracle output, and reliably tears down the stack.

Changes

Exploration harness

Layer / File(s)Summary
Exploration contract and entrypoints
.claude/skills/explore/SKILL.md, scripts/explore/explorer-prompt.md, Makefile, .gitignore
Defines the exploration workflow, reporting constraints, findings format, operator targets, and ignored .explore/ artifacts.
Locked stack setup
scripts/explore.sh
Adds prerequisite checks, repository configuration, detached flock locking, and local E2E stack startup.
Authenticated browser session
scripts/explore.sh
Creates seeded-user Playwright storage state, writes the pinned MCP configuration, and runs Claude with structured logs and separate stderr output.
Oracle and teardown lifecycle
scripts/explore.sh
Supports full, up, and down modes, appends final oracle results, tears down the stack, and releases the lock.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
participant Operator
participant explore.sh
participant E2EStack
participant AuthTestEndpoint
participant PlaywrightMCP
participant Browser
participant Claude
Operator->>explore.sh: start exploration
explore.sh->>E2EStack: boot local E2E stack
explore.sh->>AuthTestEndpoint: POST seeded student login
explore.sh->>PlaywrightMCP: configure storage state and MCP
Claude->>PlaywrightMCP: execute exploration tools
PlaywrightMCP->>Browser: drive localhost browser
explore.sh->>E2EStack: run oracle and teardown
Loading

Possibly related issues

  • Issue 403 — Covers the broader Chapter 2 AI Agent Explorer epic implemented by this harness.
  • Issue 401 — Proposes nightly automation and regression promotion for findings produced by this exploration workflow.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly names the new explore harness and /explore skill and matches the main change.
Description check✅ PassedThe description covers summary, changes, related issue, testing, screenshots, and reviewer notes with mostly complete content.
Linked Issues check✅ PassedThe PR implements the local Playwright explorer harness, seeded auth, hard budget/localhost limits, structured logs, and readable session output required by #399.
Out of Scope Changes check✅ PassedThe changes stay focused on the explore harness, prompt, Makefile, and ignore rules; no unrelated functionality is introduced.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch e2e-chapter2-explore

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Jul 28, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-staginga0360ffCommit Preview URL

Branch Preview URL
Jul 28 2026, 10:20 AM

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
scripts/explore.sh (1)

149-154: 🩺 Stability & Availability | 🔵 Trivial

stop_lock_holder/do_down release the machine lock without verifying the caller owns it.

do_down (and thus scripts/explore.sh down / make explore-down) unconditionally reads lock.ok's pid and kills it, regardless of whether the invoking shell is the one that acquired it. The elaborate ownership-safety work in start_lock_holder (a failed attempt never touches lock.ok) isn't mirrored here: since the lock is uid-namespaced (/tmp/claude-$(id -u)/...), the realistic blast radius is a single operator accidentally running down from a second terminal while up is still driving an interactive session elsewhere on the same machine — which would yank the shared local stack out from under that live session with no warning. Given the doc explicitly anticipates the "interactive flow" (separate up/down invocations) as a supported mode, this seems worth at least a guard (e.g. warn if a stack that looks actively in-use by claude is being torn down) or an explicit doc callout of the risk.

Also applies to: 404-419

🤖 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/explore.sh` around lines 149 - 154, Update stop_lock_holder and the
do_down flow to guard against tearing down a lock owned by another active
interactive session. Before killing the PID from lock.ok, detect whether it
represents an active claude-managed stack and warn or require explicit
confirmation when ownership cannot be established; preserve cleanup for the
invoking owner and document the supported separate up/down behavior if that is
the chosen safeguard.
🤖 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 `@scripts/explore.sh`:
- Around line 176-202: Update the localStorage payload in mint_storage_state so
the sapling_user name is derived from $EXPLORE_USER instead of hardcoding "Rich
Active". Preserve the existing user ID and other payload fields while ensuring
any configured seeded rich-* user receives a matching cached display name.
- Around line 302-337: Restrict the Write/Edit entries in run_explorer() to the
permitted .explore findings path, using path-scoped rules such as
Edit(.explore/**/*.md) and removing blanket file-edit permissions. Also verify
the current Claude CLI Bash permission syntax for the cd backend &&
venv/bin/python -m e2e_oracles:* rule, tightening it if compound-command parsing
permits commands outside the intended prefix.
- Around line 246-261: Update write_mcp_config to pass Playwright MCP
localhost-only origin restrictions in the generated args, using the required
localhost/127.0.0.1 allowlist or equivalent blocked-origins plus allowlist
configuration. Document alongside the configuration that these flags are
defense-in-depth only and do not prevent redirects from reaching off-localhost
destinations.
---
Nitpick comments:
In `@scripts/explore.sh`:
- Around line 149-154: Update stop_lock_holder and the do_down flow to guard
against tearing down a lock owned by another active interactive session. Before
killing the PID from lock.ok, detect whether it represents an active
claude-managed stack and warn or require explicit confirmation when ownership
cannot be established; preserve cleanup for the invoking owner and document the
supported separate up/down behavior if that is the chosen safeguard.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 762bcd62-0ca2-4e80-bacf-4434944c876d

📥 Commits

Reviewing files that changed from the base of the PR and between db0ecad and 914b0ae.

📒 Files selected for processing (5)
  • .claude/skills/explore/SKILL.md
  • .gitignore
  • Makefile
  • scripts/explore.sh
  • scripts/explore/explorer-prompt.md

Comment threadscripts/explore.sh
Comment threadscripts/explore.sh
Comment on lines +246 to +261
write_mcp_config() {
local headless_args='"--headless", '
[ "$EXPLORE_HEADED" = "1" ] && headless_args=''
cat > "$EXPLORE_DIR/mcp.json" <<EOF
{
"mcpServers": {
"playwright": {
"command": "npx",
"args": ["-y", "@playwright/mcp@$PLAYWRIGHT_MCP_VERSION", "--isolated", ${headless_args}"--browser", "chromium",
"--storage-state", "$EXPLORE_DIR/storageState.json",
"--output-dir", "$EXPLORE_DIR/traces", "--save-session"]
}
}
}
EOF
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== locate files =="
fd -a 'explore.sh|explorer-prompt.md|package.json|pnpm-lock.yaml|package-lock.json|yarn.lock|playwright|mcp.json'.| sed 's#^\./##'| head -200
echoecho"== relevant explore.sh lines =="if [ -f scripts/explore.sh ];then
nl -ba scripts/explore.sh | sed -n '220,280p'fiechoecho"== relevant explorer-prompt lines =="if [ -f scripts/explore/explorer-prompt.md ];then
nl -ba scripts/explore/explorer-prompt.md | sed -n '1,35p'fiif [ -f explorer-prompt.md ];then
nl -ba explorer-prompt.md | sed -n '1,35p'fiechoecho"== search required navigation text and mcp config args =="
rg -n "Restrict navigation|localhost|localhost-only|localhost:3000|allowed-origins|blocked-origins|explorer-prompt|MISSION|write_mcp_config|PLAYWRIGHT_MCP_VERSION" -S .

Repository: SaplingLearn/Sapling

Length of output: 479


🌐 Web query:

Playwright MCP 0.0.78 README --allowed-origins --blocked-origins

💡 Result:

In Playwright MCP, the --allowed-origins and --blocked-origins flags (and their corresponding environment variables) are used to manage network access for the browser instance [1][2][3]. Both settings accept a semicolon-separated list of origins [1][4][3]. Key details: --allowed-origins This flag specifies a list of trusted origins that the browser is permitted to request [1][5]. By default, all origins are allowed [1][2]. Environment variable: PLAYWRIGHT_MCP_ALLOWED_ORIGINS [1][3]. --blocked-origins This flag specifies a list of origins to block the browser from requesting [1][5]. The blocklist is evaluated before the allowlist; if an origin is present in both, it will be blocked [1][2][5]. Environment variable: PLAYWRIGHT_MCP_BLOCKED_ORIGINS [1][3]. Important Security Note: Documentation explicitly states that these flags do not serve as a security boundary and do not affect redirects [1][5][3]. Supported Formats: Both flags support the following formats [1][2]: Full origin: e.g., https://example.com:8080 (matches only that specific origin) [1][2]. Wildcard port: e.g., http://localhost:* (matches any port on localhost with the http protocol) [1][2].

Citations:


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== relevant explore.sh lines =="if [ -f scripts/explore.sh ];then
sed -n '220,280p' scripts/explore.sh | cat -n
fiechoecho"== relevant explorer-prompt lines =="if [ -f scripts/explore/explorer-prompt.md ];then
sed -n '1,40p' scripts/explore/explorer-prompt.md | cat -n
fiechoecho"== search required navigation text and mcp config args =="
rg -n "Restrict navigation|localhost|localhost-only|localhost:3000|allowed-origins|blocked-origins|explorer-prompt|MISSION|write_mcp_config|PLAYWRIGHT_MCP_VERSION" -S .echoecho"== lockfile version snippets =="forfin package-lock.json frontend/package-lock.json;doif [ -f"$f" ];thenecho"--- $f ---"
rg -n '"`@playwright/mcp`"|"`@playwright/test`"|PLAYWRIGHT_MCP_VERSION'"$f" -A 3 -B 2 ||truefidone

Repository: SaplingLearn/Sapling

Length of output: 22370


🌐 Web query:

playwright mcp --allowed-origins blocked navigation error network error

💡 Result:

The "blocked navigation error" or network-related errors when using the --allowed-origins flag in Playwright MCP typically stem from configuration format issues, regex/glob matching errors, or confusing the flag with server-side CORS settings [1][2][3]. Key troubleshooting steps include: 1. Use Correct Origin Format: Ensure you are passing the domain or origin directly (e.g., google.com) rather than including protocol prefixes like https:// or trailing slashes, which have historically caused parsing issues [1]. 2. Avoid Invalid Glob Patterns: Recent versions (as of early 2026) have had bugs where wildcard port patterns like http://localhost:* generate invalid internal glob patterns, causing the browser to block all traffic to those destinations [3]. If you are experiencing this, you may need to use explicit hostnames or check for updates addressing the originOrHostGlob function [3]. 3. Distinguish Between Flags: Ensure you are not confusing --allowed-origins (which controls browser navigation within the sandboxed Playwright instance) with --allowed-hosts (which controls MCP server CORS/connectivity) [2]. Both may need to be set identically if you are navigating to localhost in restricted environments like GitHub Actions or WSL [2][4]. 4. Blocklist Precedence: Keep in mind that Playwright MCP evaluates the blocked-origins list before the allowed-origins list. If a URL matches both, it will be blocked [5]. 5. Debugging Network Issues: If navigation still fails with ERR_CONNECTION_REFUSED, verify that the target service (e.g., a local dev server) is listening on 0.0.0.0 rather than just 127.0.0.1 to ensure it is accessible to the browser process, especially if running within containers (Docker) or WSL [2][4]. For further debugging, you can enable verbose logging by setting the DEBUG environment variable to pw:api or pw:browser* to see exactly which origin is failing to match the navigation request [4].

Citations:


🌐 Web query:

Playwright MCP blocked origins error "Failed to navigate to"

💡 Result:

The error net::ERR_BLOCKED_BY_CLIENT when using Playwright MCP typically indicates that the browser navigation was blocked by a client-side security policy, configuration, or environment restriction [1]. Because Playwright MCP is often used in automated contexts (such as LLM agents), this error is usually triggered by one of the following factors [1]: 1. Allowed Origins Configuration: If you have configured allowed-origins for security, the destination URL may be excluded from the allowlist [2][3]. Verify that the PW_ALLOWED_ORIGINS environment variable or the --allowed-origins CLI flag is correctly set to include the domain you are attempting to visit [2][3]. 2. Browser Environment and Sandbox Restrictions: When running in containerized environments like Docker, Chromium often fails to navigate because it cannot access the necessary system sandbox [1]. To resolve this, you may need to pass explicit launch flags to bypass sandbox restrictions in your PLAYWRIGHT_LAUNCH_OPTIONS [1]: --no-sandbox --disable-setuid-sandbox --disable-gpu 3. Security Settings or Extensions: Although Playwright MCP sessions are typically isolated, ERR_BLOCKED_BY_CLIENT can also occur if the browser launch arguments include restrictive security settings or if there are conflicts with other security-related flags [1]. 4. Troubleshooting Steps: - Check the logs to confirm the specific URL being blocked [1]. - If you are running in Docker, ensure the container has the necessary privileges and sandbox bypass flags [1]. - Review your MCP configuration file (often found in .mcp/config.json or equivalent) to ensure --allowed-origins is formatted correctly (e.g., comma-separated or semicolon-separated depending on your specific implementation) [4][2][3]. - If you suspect the issue is not related to origins, verify that npx and the Playwright runtime are correctly installed and accessible in your environment, as missing dependencies can sometimes lead to unexpected browser behavior [5]. If the error persists despite correct origin configuration, it is often helpful to test a basic navigation command outside of the MCP wrapper to rule out environment-specific sandbox or policy issues [1].

Citations:


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== package manifests mentioning playwright mcp =="
rg -n '"`@playwright/mcp`"|"playwright-mcp"|PLAYWRIGHT_MCP_VERSION' -S --glob '!node_modules/**' --glob '!frontend/node_modules/**' frontend package.json scripts
echoecho"== scripts/explore.sh version assignment and mcp config callers =="
nl -ba scripts/explore.sh | sed -n '60,80p'
nl -ba scripts/explore.sh | sed -n '370,390p'echoecho"== local prompt copy =="
cat -n scripts/explore/explorer-prompt.md | sed -n '1,20p'

Repository: SaplingLearn/Sapling

Length of output: 587


🌐 Web query:

site:github.com/microsoft/playwright-mcp "allowedOrigins" "blockedByClient" "ERR_BLOCKED_BY_CLIENT"

💡 Result:

The error ERR_BLOCKED_BY_CLIENT in the context of Playwright MCP (Model Context Protocol) is a browser-level error that occurs when the browser is instructed to navigate to or request a resource that has been explicitly blocked or is unreachable due to configuration issues [1][2]. In the specific context of the Playwright MCP server, this error frequently arises from the following scenarios: 1. Origin Filtering: If you have configured --allowed-origins or --blocked-origins, the browser will block navigation to any origin not in the allowlist or present in the blocklist [1]. Note that these flags affect navigation but do not serve as a CORS security boundary [1][3]. 2. Extension ID Mismatch: A common cause for ERR_BLOCKED_BY_CLIENT in recent versions (e.g., v0.0.66–0.0.68) is a hardcoded Chrome extension ID in the MCP server's connection logic [2]. If you are using an unpacked extension, a locally built version, or a different browser channel, your extension will have a different ID than the one hardcoded (mmlmfjhmonkocbjadbfplnigmagldckm). This causes the browser to attempt to connect to a non-existent extension, resulting in the error [2]. 3. Configuration Bugs: Bugs in how origin patterns are parsed—such as incorrect handling of wildcard ports (e.g., http://localhost:*)—can lead to requests being unexpectedly blocked [4]. To resolve or debug this error: - Verify your --allowed-origins and --blocked-origins configurations to ensure they match the intended URLs [1][4]. - If using an unpacked browser extension, ensure the extension ID is handled correctly, potentially by setting the PLAYWRIGHT_MCP_EXTENSION_ID environment variable or pinning your extension's ID via the manifest's "key" property [2]. - Check the official issue tracker for your version of Playwright MCP, as specific releases have included fixes for origin pattern parsing and connection logic [4][2][5].

Citations:


🌐 Web query:

github.com/microsoft/playwright-mcp program.ts "allowed-origins" "allowedOrigins" "blockedByClient"

💡 Result:

In the Playwright MCP server, allowedOrigins and blockedOrigins are configuration options used to control which network requests the browser is permitted to make [1][2]. These settings are managed within the server's network configuration and are implemented via Playwright's routing capabilities [3][4]. When these rules are active, the server intercepts network requests and uses the 'blockedbyclient' error code to terminate requests that do not align with the defined allow/block lists [3][4]. Specifically, the implementation follows this logic: 1. If allowedOrigins are defined, the server first blocks all requests by default using route.abort('blockedbyclient') [3][5]. 2. It then explicitly allows requests to the specified origins by calling route.continue for patterns matching the allowed list [3][4]. 3. If blockedOrigins are defined, the server explicitly blocks requests to those origins using route.abort('blockedbyclient') [3][4]. Origins matching both allowedOrigins and blockedOrigins will be blocked [1][2]. The allowed-origins command-line argument accepts a semicolon-separated list of origins, and the configuration supports full origin strings (e.g., https://example.com:8080) and wildcard port patterns (e.g., http://localhost:*) [1][6]. These settings are considered convenience features for managing browser interactions rather than strict security boundaries [1][2]. The file program.ts within the Playwright repository is involved in managing the command-line interface and tool configuration for the MCP server [7].

Citations:


Reinforce the localhost-first prompt with a technical restriction.

This harness requires localhost-only navigation, but mcp.json relies only on scripts/explore/explorer-prompt.md:13 for enforcement. Add a Playwright MCP restriction such as --allowed-origins http://localhost:3000;http://localhost:5000;127.0.0.1:5000 or --blocked-origins * plus the localhost allowlist, while documenting that these flags are not a security boundary against redirects; this gives defense-in-depth against model-driven or page-driven off-localhost clicks.

🤖 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/explore.sh` around lines 246 - 261, Update write_mcp_config to pass
Playwright MCP localhost-only origin restrictions in the generated args, using
the required localhost/127.0.0.1 allowlist or equivalent blocked-origins plus
allowlist configuration. Document alongside the configuration that these flags
are defense-in-depth only and do not prevent redirects from reaching
off-localhost destinations.

Comment threadscripts/explore.sh
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

Code review

Found 3 issues:

  1. do_down() can leak the stack and the machine-singleton lock: it runs under set -euo pipefail and fires as an EXIT trap, but its mkdir -p, the findings-file bootstrap, and the { … } >> findings.md append block have no || true guards (unlike the oracle subshell and e2e-down.sh lines around them). A failed write there (disk full, read-only fs) aborts the trap before e2e-down.sh and stop_lock_holder run — verified bash semantics: a failing command inside an EXIT trap under set -e stops the trap. The header comment's "|| true guards throughout" claim is currently false for these lines.

do_down() {
mkdir -p "$EXPLORE_DIR"
(
cd"$REPO_ROOT/backend"
venv/bin/python -m e2e_oracles --json >"$EXPLORE_DIR/oracle-final.json"||true
venv/bin/python -m e2e_oracles >"$EXPLORE_DIR/oracle-final.txt"2>&1||true
) ||true
[ -f"$EXPLORE_DIR/findings.md" ] ||printf'# Exploration findings\n'>"$EXPLORE_DIR/findings.md"
{
printf'\n## Oracle final pass (%s)\n\n```\n'"$(date -Iseconds)"
cat "$EXPLORE_DIR/oracle-final.txt"2>/dev/null ||true
printf'```\n'
} >>"$EXPLORE_DIR/findings.md"
"$REPO_ROOT/scripts/e2e-down.sh"||true
stop_lock_holder
}

  1. Dummy-key mitigation for RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439 defeats itself in the common developer environment: export GEMINI_API_KEY="${GEMINI_API_KEY:-e2e-dummy-key-no-billing}" defers to an ambient real key, and developers here routinely keep one exported (validate_config fail-closes without it). The below-seam RAG/embedding path (RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439) bills live even in function mode, and the explorer's mission (uploads, quiz, tutor turns, many turns) is worst-case volume for it. CI applies the dummy key unconditionally for exactly this reason; export inside the script can't touch the operator's parent shell, so the conditional protects nothing.

# Default only if unset — never clobber an operator's real key.
export GEMINI_API_KEY="${GEMINI_API_KEY:-e2e-dummy-key-no-billing}"
"$REPO_ROOT/scripts/e2e-up.sh"|| die "scripts/e2e-up.sh failed — see .e2e/*.log"

  1. jq is a hard dependency of the transcript pipeline but is missing from preflight (claude/npx/curl/python3 are checked). Without it the script boots the entire stack — Supabase, migrations, a production Next build, minutes of work — and only then fails in run_explorer, instead of failing fast like the other four checks.

echo"▶ Preflight…"
command -v claude >/dev/null 2>&1|| die "claude CLI not found on PATH — install it first"
command -v npx >/dev/null 2>&1|| die "npx not found on PATH — install Node.js first"
command -v curl >/dev/null 2>&1|| die "curl not found on PATH"
command -v python3 >/dev/null 2>&1|| die "python3 not found on PATH"
[ -f"$REPO_ROOT/scripts/explore/explorer-prompt.md" ] \

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

…eflight, scoped Edit, EXPLORE_USER wiring (#399)
- do_down: guard every fallible write with || true so a failed findings.md
write can never abort the EXIT trap before e2e-down.sh / stop_lock_holder
run (was leaking the stack + machine-singleton lock on write failure).
- GEMINI_API_KEY: export unconditionally (matches CI's dummy, #439) instead
of deferring to an ambient real key that can bill the below-seam RAG path.
- SEED_RICH=1 exported unconditionally so an ambient SEED_RICH=0 can't
silently defeat the rich dataset this harness requires.
- preflight: add missing `jq` check (hard dependency of the transcript
pipeline) so a missing jq fails in seconds, not after a full stack boot.
- allowedTools: replace unscoped Write,Edit with Read,Edit(.explore/**) —
Write(...) patterns aren't matched by the CLI's file permission check, so
only a path-scoped Edit rule actually restricts writes to findings.md.
do_up now pre-creates .explore/findings.md so the explorer always has an
existing file for the scoped Edit grant.
- EXPLORE_USER: derive the sapling_user display name from the user id
(case map for the five seeded rich-* users, verified against
db/seed_local_rich.py) instead of hardcoding "Rich Active"; pass
--user "$EXPLORE_USER" to both oracle invocations in do_down.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

All three review findings fixed in a0360ff (do_down teardown/lock-release guards verified with a real write-failure injection; unconditional dummy GEMINI_API_KEY per #439; jq preflight), plus three panel-verified hardening items in the same commit: allowedTools Write dropped / Edit scoped to .explore/**, EXPLORE_USER wired through the name map and both oracle --user calls, unconditional SEED_RICH=1. Scoped re-review confirms all addressed, no new breakage.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(agent): LLM explorer harness driving Playwright against the local stack

1 participant

@AndresL230
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

feat(e2e): Chapter 2 explore harness — make explore + /explore skill (#399) - #444

Merged
AndresL230 merged 9 commits into
mainfrom
e2e-chapter2-explore
Jul 28, 2026
Merged

feat(e2e): Chapter 2 explore harness — make explore + /explore skill (#399)#444
AndresL230 merged 9 commits into
mainfrom
e2e-chapter2-explore

Conversation

@AndresL230

@AndresL230AndresL230 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Chapter 2 of the E2E program (epic #403), PR 2 of 3. Closes#399. Builds on the #400 oracle module (PR #443).

make explore boots the deterministic local stack and hands a Claude Code explorer a real browser; the oracles judge what it saw. Local-only by design — never a CI gate.

What's here

  • scripts/explore.sh (+ make explore / make explore-down, .explore/ gitignored) — the full pipeline: machine-singleton stack lock (detached holder process writing self-identifying lock.ok only after flock succeeds — a failed acquisition touches nothing, proven by an A/B two-session simulation) → e2e-up in function mode with a dummy GEMINI_API_KEY (RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439) → storage state minted via POST /api/auth/test-login through the frontend origin (cookie + sapling_user localStorage, secure flag matching the backend's actual local policy) → claude -p with a pinned @playwright/mcp@0.0.78 config and turn budget, transcript captured via stream-json + jq into a grep-able session.log → oracle final pass appended to findings.md → teardown. up/down subcommands support the interactive flow.
  • scripts/explore/explorer-prompt.md — the mission briefing: student persona (Rich Active), break-things mandate, report-never-fix, oracle cadence (cd backend && venv/bin/python -m e2e_oracles), findings format, known-bugs list, and a "stub it before you dig" rule earned in live testing.
  • .claude/skills/explore/SKILL.md — the interactive /explore skill: same pipeline, watchable and steerable, with a same-origin JS sign-in for live-browser sessions.

Hard-won fixes from live acceptance runs

  • @playwright/mcp without --isolated reuses a persistent profile and silently ignores --storage-state — the run-2 "signed-in" state was a stale-cookie artifact; --isolated fixes it (verified down to the bundled playwright-core source and a cookie-DB probe).
  • claude -p's default text output prints only the final message — a turn-budget exit produced an empty transcript despite real browsing; --output-format stream-json --verbose + jq restores the readable transcript.
  • Lock-holder bookkeeping survives concurrent sessions (busy attempt leaves the running session's files byte-identical).

Acceptance (issue #399 bar, definitive run in the SDD report)

One bounded run (EXPLORE_MAX_TURNS=25, 3m52s): real signed-in navigation across dashboard + library ("Rich Active" nav, 4 seeded documents, CS101/MATH210/BIO110/ENG150 visible in snapshots), 2 explorer-written findings — including a novel wrong-data upload bug and a tutor-resume 500 (concept_describe unregistered in the function-handler seam) independently confirmed by the oracle logscan — plus the harness-appended Oracle final pass re-confirming #355. Machine verified clean afterward (ports, containers, lock).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added an end-to-end exploratory testing workflow for the local application.
    • Added commands to start, stop, and run complete exploration sessions.
    • Exploration sessions now support browser-driven testing, automated checks, session logs, and structured findings.
  • Documentation

    • Added guidance covering exploration steps, reporting requirements, known issues, and session summaries.
  • Chores

    • Exploration artifacts are now excluded from version control.

AndresL230and others added 8 commits July 28, 2026 01:13
…itignore (#399)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e, oracle cadence (#399)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…er GEMINI_API_KEY (#399)
A busy lock previously still tripped do_up's trap/wipe path, tearing down
another session's live stack via scripts/e2e-down.sh and deleting its
.explore/ artifacts before the lock check ever ran. start_lock_holder now
runs first, the do_down safety trap arms only after the lock is held, the
.explore/ wipe (still selective, preserving lock.pid/lock.ok) happens after
that, and a failed acquisition cleans up its own holder/pid files before
exiting. GEMINI_API_KEY now defaults only when unset instead of always
overwriting an operator's real key.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ns (#399)
start_lock_holder previously deleted lock.ok and unconditionally wrote
lock.pid before knowing whether the flock would actually succeed. A failed
attempt from session B (lock held by session A) would delete A's lock.ok and
overwrite A's lock.pid with B's own doomed PID, then B's busy-path cleanup
removed the file entirely — leaving A's later `down` unable to find A's
holder, leaking both the detached process and the machine-singleton lock.
Now the holder subprocess itself is the only writer of lock.ok, and only
ever after its own `flock -n 9` succeeds (atomic temp-file + mv, content =
the holder's own $$). The parent only polls for that self-identifying
marker and touches nothing on disk on the failure path, so a busy lock can
never clobber another session's bookkeeping. lock.pid is retired — lock.ok's
content is now the sole source of truth stop_lock_holder reads.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The first bounded acceptance run (task-8-report.md) produced a
session.log with exactly one line — claude -p's own "Error: Reached
max turns" — because the default --output-format=text only prints the
FINAL message, never the intermediate tool_use/tool_result turns. That
failed the #399 acceptance bar ("session.log shows real Playwright MCP
tool calls").
Switch run_explorer to --output-format stream-json --verbose (the CLI
requires both together) and reformat the JSONL with jq into readable
[assistant]/[tool_use]/[tool_result]/[result] lines, truncated to 500
chars each, so session.log is both grep-able and human-skimmable.
fromjson? tolerates stray non-JSON lines instead of aborting the whole
transcript; claude -p's stderr now goes to its own session.stderr.log
so it can never interleave with (and corrupt) the JSON stream jq
parses.
Verified: a second bounded run (EXPLORE_MAX_TURNS=25) produced a
281-line session.log with 18 mcp__playwright__* tool calls across
/dashboard and /library, plus findings.md with a re-confirmed #430
repro and the oracle final pass re-confirming #355.
The follow-up bounded-run diagnosis found a contradiction: run 2's
session cookie was accepted (/api/users 200) but the UI acted fully
signed-out (Sign In button, dashboard skeleton) — the #430 symptom.
Run 1's "Secure cookie dropped over http" diagnosis didn't hold up
either, since Chromium does accept Secure cookies on http://localhost.
Root-caused by reading @playwright/mcp@0.0.78's bundled source
(playwright-core/lib/coreBundle.js): without --isolated, the MCP
server launches ONE PERSISTENT Chrome profile keyed only by
sha256(cwd) — reused across every explore.sh run from this checkout,
never wiped by teardown — and its client factory does
`config.browser.isolated ? await browser.newContext(contextOptions) :
browser.contexts()[0]`. In the default (non-isolated) branch it just
grabs the already-open persistent context and never calls newContext
with our --storage-state at all.
Verified empirically: wiped the profile dir, booted a clean stack, and
drove a 3-turn claude -p probe against the (then-current) mcp.json —
navigating to /dashboard redirected straight to a REAL
accounts.google.com sign-in page, and sqlite3 on the profile's Cookies
db afterward showed zero sapling_session rows (neither cookie nor
localStorage from storageState.json was ever applied). The identical
probe with --isolated added rendered the real, fully-authenticated
dashboard on the first navigation, with localStorage.sapling_user
correctly present — this is the same mechanism (ephemeral
browser.newContext(contextOptions)) @playwright/test itself uses in
Chapter 1's global-setup.ts.
Also corrected mint_storage_state's cookie to secure:false, matching
what the backend's own SECURE_COOKIES policy actually issues for the
http://localhost local stack (config.py derives it from FRONTEND_URL's
scheme) — the file should mirror reality regardless of which flag
turned out to be the load-bearing one.
Verified: EXPLORE_MAX_TURNS=12 make explore reached a signed-in
dashboard (nav shows "Rich Active" / "Account", full authenticated
menu) using only 2 real Playwright tool calls, zero sign-in recovery
turns.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The definitive acceptance run (harness fixes verified: real auth via
--isolated, breadth across 2+ surfaces) hit a genuine new bug —
resuming a tutor session 500'd on POST /api/graph/.../concept-description,
root-caused via the explorer's own backend-log investigation to a
missing SAPLING_FUNCTION_HANDLERS registration for 'concept_describe'
(caught independently by the oracle's logscan pass too, so it's not
lost, just not explorer-authored). But the explorer spent its
remaining turns tailing logs and checking processes to nail the exact
LookupError, and ran out of budget before writing an F<N> entry to
findings.md — a real gap against #399's "readable transcript AND
findings file" bar, distinct from any flag/mechanism defect.
Add a "stub it before you dig" ground rule: write a one-line stub
finding the instant something looks off, before further root-causing.
A written stub survives a turn-budget cutoff; a perfect unwritten
diagnosis does not.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:34 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: dc0a75e3-31f4-42be-8ec2-bb9a848919bb

📥 Commits

Reviewing files that changed from the base of the PR and between 914b0ae and a0360ff.

📒 Files selected for processing (1)
  • scripts/explore.sh
📝 Walkthrough

Walkthrough

Adds a locked local exploratory-testing workflow that boots the E2E stack, authenticates a seeded browser session, runs Claude through Playwright MCP, records findings and oracle output, and reliably tears down the stack.

Changes

Exploration harness

Layer / File(s)Summary
Exploration contract and entrypoints
.claude/skills/explore/SKILL.md, scripts/explore/explorer-prompt.md, Makefile, .gitignore
Defines the exploration workflow, reporting constraints, findings format, operator targets, and ignored .explore/ artifacts.
Locked stack setup
scripts/explore.sh
Adds prerequisite checks, repository configuration, detached flock locking, and local E2E stack startup.
Authenticated browser session
scripts/explore.sh
Creates seeded-user Playwright storage state, writes the pinned MCP configuration, and runs Claude with structured logs and separate stderr output.
Oracle and teardown lifecycle
scripts/explore.sh
Supports full, up, and down modes, appends final oracle results, tears down the stack, and releases the lock.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
participant Operator
participant explore.sh
participant E2EStack
participant AuthTestEndpoint
participant PlaywrightMCP
participant Browser
participant Claude
Operator->>explore.sh: start exploration
explore.sh->>E2EStack: boot local E2E stack
explore.sh->>AuthTestEndpoint: POST seeded student login
explore.sh->>PlaywrightMCP: configure storage state and MCP
Claude->>PlaywrightMCP: execute exploration tools
PlaywrightMCP->>Browser: drive localhost browser
explore.sh->>E2EStack: run oracle and teardown
Loading

Possibly related issues

  • Issue 403 — Covers the broader Chapter 2 AI Agent Explorer epic implemented by this harness.
  • Issue 401 — Proposes nightly automation and regression promotion for findings produced by this exploration workflow.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly names the new explore harness and /explore skill and matches the main change.
Description check✅ PassedThe description covers summary, changes, related issue, testing, screenshots, and reviewer notes with mostly complete content.
Linked Issues check✅ PassedThe PR implements the local Playwright explorer harness, seeded auth, hard budget/localhost limits, structured logs, and readable session output required by #399.
Out of Scope Changes check✅ PassedThe changes stay focused on the explore harness, prompt, Makefile, and ignore rules; no unrelated functionality is introduced.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch e2e-chapter2-explore

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Jul 28, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-staginga0360ffCommit Preview URL

Branch Preview URL
Jul 28 2026, 10:20 AM

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
scripts/explore.sh (1)

149-154: 🩺 Stability & Availability | 🔵 Trivial

stop_lock_holder/do_down release the machine lock without verifying the caller owns it.

do_down (and thus scripts/explore.sh down / make explore-down) unconditionally reads lock.ok's pid and kills it, regardless of whether the invoking shell is the one that acquired it. The elaborate ownership-safety work in start_lock_holder (a failed attempt never touches lock.ok) isn't mirrored here: since the lock is uid-namespaced (/tmp/claude-$(id -u)/...), the realistic blast radius is a single operator accidentally running down from a second terminal while up is still driving an interactive session elsewhere on the same machine — which would yank the shared local stack out from under that live session with no warning. Given the doc explicitly anticipates the "interactive flow" (separate up/down invocations) as a supported mode, this seems worth at least a guard (e.g. warn if a stack that looks actively in-use by claude is being torn down) or an explicit doc callout of the risk.

Also applies to: 404-419

🤖 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/explore.sh` around lines 149 - 154, Update stop_lock_holder and the
do_down flow to guard against tearing down a lock owned by another active
interactive session. Before killing the PID from lock.ok, detect whether it
represents an active claude-managed stack and warn or require explicit
confirmation when ownership cannot be established; preserve cleanup for the
invoking owner and document the supported separate up/down behavior if that is
the chosen safeguard.
🤖 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 `@scripts/explore.sh`:
- Around line 176-202: Update the localStorage payload in mint_storage_state so
the sapling_user name is derived from $EXPLORE_USER instead of hardcoding "Rich
Active". Preserve the existing user ID and other payload fields while ensuring
any configured seeded rich-* user receives a matching cached display name.
- Around line 302-337: Restrict the Write/Edit entries in run_explorer() to the
permitted .explore findings path, using path-scoped rules such as
Edit(.explore/**/*.md) and removing blanket file-edit permissions. Also verify
the current Claude CLI Bash permission syntax for the cd backend &&
venv/bin/python -m e2e_oracles:* rule, tightening it if compound-command parsing
permits commands outside the intended prefix.
- Around line 246-261: Update write_mcp_config to pass Playwright MCP
localhost-only origin restrictions in the generated args, using the required
localhost/127.0.0.1 allowlist or equivalent blocked-origins plus allowlist
configuration. Document alongside the configuration that these flags are
defense-in-depth only and do not prevent redirects from reaching off-localhost
destinations.
---
Nitpick comments:
In `@scripts/explore.sh`:
- Around line 149-154: Update stop_lock_holder and the do_down flow to guard
against tearing down a lock owned by another active interactive session. Before
killing the PID from lock.ok, detect whether it represents an active
claude-managed stack and warn or require explicit confirmation when ownership
cannot be established; preserve cleanup for the invoking owner and document the
supported separate up/down behavior if that is the chosen safeguard.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 762bcd62-0ca2-4e80-bacf-4434944c876d

📥 Commits

Reviewing files that changed from the base of the PR and between db0ecad and 914b0ae.

📒 Files selected for processing (5)
  • .claude/skills/explore/SKILL.md
  • .gitignore
  • Makefile
  • scripts/explore.sh
  • scripts/explore/explorer-prompt.md

Comment threadscripts/explore.sh
Comment threadscripts/explore.sh
Comment on lines +246 to +261
write_mcp_config() {
local headless_args='"--headless", '
[ "$EXPLORE_HEADED" = "1" ] && headless_args=''
cat > "$EXPLORE_DIR/mcp.json" <<EOF
{
"mcpServers": {
"playwright": {
"command": "npx",
"args": ["-y", "@playwright/mcp@$PLAYWRIGHT_MCP_VERSION", "--isolated", ${headless_args}"--browser", "chromium",
"--storage-state", "$EXPLORE_DIR/storageState.json",
"--output-dir", "$EXPLORE_DIR/traces", "--save-session"]
}
}
}
EOF
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== locate files =="
fd -a 'explore.sh|explorer-prompt.md|package.json|pnpm-lock.yaml|package-lock.json|yarn.lock|playwright|mcp.json'.| sed 's#^\./##'| head -200
echoecho"== relevant explore.sh lines =="if [ -f scripts/explore.sh ];then
nl -ba scripts/explore.sh | sed -n '220,280p'fiechoecho"== relevant explorer-prompt lines =="if [ -f scripts/explore/explorer-prompt.md ];then
nl -ba scripts/explore/explorer-prompt.md | sed -n '1,35p'fiif [ -f explorer-prompt.md ];then
nl -ba explorer-prompt.md | sed -n '1,35p'fiechoecho"== search required navigation text and mcp config args =="
rg -n "Restrict navigation|localhost|localhost-only|localhost:3000|allowed-origins|blocked-origins|explorer-prompt|MISSION|write_mcp_config|PLAYWRIGHT_MCP_VERSION" -S .

Repository: SaplingLearn/Sapling

Length of output: 479


🌐 Web query:

Playwright MCP 0.0.78 README --allowed-origins --blocked-origins

💡 Result:

In Playwright MCP, the --allowed-origins and --blocked-origins flags (and their corresponding environment variables) are used to manage network access for the browser instance [1][2][3]. Both settings accept a semicolon-separated list of origins [1][4][3]. Key details: --allowed-origins This flag specifies a list of trusted origins that the browser is permitted to request [1][5]. By default, all origins are allowed [1][2]. Environment variable: PLAYWRIGHT_MCP_ALLOWED_ORIGINS [1][3]. --blocked-origins This flag specifies a list of origins to block the browser from requesting [1][5]. The blocklist is evaluated before the allowlist; if an origin is present in both, it will be blocked [1][2][5]. Environment variable: PLAYWRIGHT_MCP_BLOCKED_ORIGINS [1][3]. Important Security Note: Documentation explicitly states that these flags do not serve as a security boundary and do not affect redirects [1][5][3]. Supported Formats: Both flags support the following formats [1][2]: Full origin: e.g., https://example.com:8080 (matches only that specific origin) [1][2]. Wildcard port: e.g., http://localhost:* (matches any port on localhost with the http protocol) [1][2].

Citations:


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== relevant explore.sh lines =="if [ -f scripts/explore.sh ];then
sed -n '220,280p' scripts/explore.sh | cat -n
fiechoecho"== relevant explorer-prompt lines =="if [ -f scripts/explore/explorer-prompt.md ];then
sed -n '1,40p' scripts/explore/explorer-prompt.md | cat -n
fiechoecho"== search required navigation text and mcp config args =="
rg -n "Restrict navigation|localhost|localhost-only|localhost:3000|allowed-origins|blocked-origins|explorer-prompt|MISSION|write_mcp_config|PLAYWRIGHT_MCP_VERSION" -S .echoecho"== lockfile version snippets =="forfin package-lock.json frontend/package-lock.json;doif [ -f"$f" ];thenecho"--- $f ---"
rg -n '"`@playwright/mcp`"|"`@playwright/test`"|PLAYWRIGHT_MCP_VERSION'"$f" -A 3 -B 2 ||truefidone

Repository: SaplingLearn/Sapling

Length of output: 22370


🌐 Web query:

playwright mcp --allowed-origins blocked navigation error network error

💡 Result:

The "blocked navigation error" or network-related errors when using the --allowed-origins flag in Playwright MCP typically stem from configuration format issues, regex/glob matching errors, or confusing the flag with server-side CORS settings [1][2][3]. Key troubleshooting steps include: 1. Use Correct Origin Format: Ensure you are passing the domain or origin directly (e.g., google.com) rather than including protocol prefixes like https:// or trailing slashes, which have historically caused parsing issues [1]. 2. Avoid Invalid Glob Patterns: Recent versions (as of early 2026) have had bugs where wildcard port patterns like http://localhost:* generate invalid internal glob patterns, causing the browser to block all traffic to those destinations [3]. If you are experiencing this, you may need to use explicit hostnames or check for updates addressing the originOrHostGlob function [3]. 3. Distinguish Between Flags: Ensure you are not confusing --allowed-origins (which controls browser navigation within the sandboxed Playwright instance) with --allowed-hosts (which controls MCP server CORS/connectivity) [2]. Both may need to be set identically if you are navigating to localhost in restricted environments like GitHub Actions or WSL [2][4]. 4. Blocklist Precedence: Keep in mind that Playwright MCP evaluates the blocked-origins list before the allowed-origins list. If a URL matches both, it will be blocked [5]. 5. Debugging Network Issues: If navigation still fails with ERR_CONNECTION_REFUSED, verify that the target service (e.g., a local dev server) is listening on 0.0.0.0 rather than just 127.0.0.1 to ensure it is accessible to the browser process, especially if running within containers (Docker) or WSL [2][4]. For further debugging, you can enable verbose logging by setting the DEBUG environment variable to pw:api or pw:browser* to see exactly which origin is failing to match the navigation request [4].

Citations:


🌐 Web query:

Playwright MCP blocked origins error "Failed to navigate to"

💡 Result:

The error net::ERR_BLOCKED_BY_CLIENT when using Playwright MCP typically indicates that the browser navigation was blocked by a client-side security policy, configuration, or environment restriction [1]. Because Playwright MCP is often used in automated contexts (such as LLM agents), this error is usually triggered by one of the following factors [1]: 1. Allowed Origins Configuration: If you have configured allowed-origins for security, the destination URL may be excluded from the allowlist [2][3]. Verify that the PW_ALLOWED_ORIGINS environment variable or the --allowed-origins CLI flag is correctly set to include the domain you are attempting to visit [2][3]. 2. Browser Environment and Sandbox Restrictions: When running in containerized environments like Docker, Chromium often fails to navigate because it cannot access the necessary system sandbox [1]. To resolve this, you may need to pass explicit launch flags to bypass sandbox restrictions in your PLAYWRIGHT_LAUNCH_OPTIONS [1]: --no-sandbox --disable-setuid-sandbox --disable-gpu 3. Security Settings or Extensions: Although Playwright MCP sessions are typically isolated, ERR_BLOCKED_BY_CLIENT can also occur if the browser launch arguments include restrictive security settings or if there are conflicts with other security-related flags [1]. 4. Troubleshooting Steps: - Check the logs to confirm the specific URL being blocked [1]. - If you are running in Docker, ensure the container has the necessary privileges and sandbox bypass flags [1]. - Review your MCP configuration file (often found in .mcp/config.json or equivalent) to ensure --allowed-origins is formatted correctly (e.g., comma-separated or semicolon-separated depending on your specific implementation) [4][2][3]. - If you suspect the issue is not related to origins, verify that npx and the Playwright runtime are correctly installed and accessible in your environment, as missing dependencies can sometimes lead to unexpected browser behavior [5]. If the error persists despite correct origin configuration, it is often helpful to test a basic navigation command outside of the MCP wrapper to rule out environment-specific sandbox or policy issues [1].

Citations:


🏁 Script executed:

#!/bin/bashset -euo pipefail
echo"== package manifests mentioning playwright mcp =="
rg -n '"`@playwright/mcp`"|"playwright-mcp"|PLAYWRIGHT_MCP_VERSION' -S --glob '!node_modules/**' --glob '!frontend/node_modules/**' frontend package.json scripts
echoecho"== scripts/explore.sh version assignment and mcp config callers =="
nl -ba scripts/explore.sh | sed -n '60,80p'
nl -ba scripts/explore.sh | sed -n '370,390p'echoecho"== local prompt copy =="
cat -n scripts/explore/explorer-prompt.md | sed -n '1,20p'

Repository: SaplingLearn/Sapling

Length of output: 587


🌐 Web query:

site:github.com/microsoft/playwright-mcp "allowedOrigins" "blockedByClient" "ERR_BLOCKED_BY_CLIENT"

💡 Result:

The error ERR_BLOCKED_BY_CLIENT in the context of Playwright MCP (Model Context Protocol) is a browser-level error that occurs when the browser is instructed to navigate to or request a resource that has been explicitly blocked or is unreachable due to configuration issues [1][2]. In the specific context of the Playwright MCP server, this error frequently arises from the following scenarios: 1. Origin Filtering: If you have configured --allowed-origins or --blocked-origins, the browser will block navigation to any origin not in the allowlist or present in the blocklist [1]. Note that these flags affect navigation but do not serve as a CORS security boundary [1][3]. 2. Extension ID Mismatch: A common cause for ERR_BLOCKED_BY_CLIENT in recent versions (e.g., v0.0.66–0.0.68) is a hardcoded Chrome extension ID in the MCP server's connection logic [2]. If you are using an unpacked extension, a locally built version, or a different browser channel, your extension will have a different ID than the one hardcoded (mmlmfjhmonkocbjadbfplnigmagldckm). This causes the browser to attempt to connect to a non-existent extension, resulting in the error [2]. 3. Configuration Bugs: Bugs in how origin patterns are parsed—such as incorrect handling of wildcard ports (e.g., http://localhost:*)—can lead to requests being unexpectedly blocked [4]. To resolve or debug this error: - Verify your --allowed-origins and --blocked-origins configurations to ensure they match the intended URLs [1][4]. - If using an unpacked browser extension, ensure the extension ID is handled correctly, potentially by setting the PLAYWRIGHT_MCP_EXTENSION_ID environment variable or pinning your extension's ID via the manifest's "key" property [2]. - Check the official issue tracker for your version of Playwright MCP, as specific releases have included fixes for origin pattern parsing and connection logic [4][2][5].

Citations:


🌐 Web query:

github.com/microsoft/playwright-mcp program.ts "allowed-origins" "allowedOrigins" "blockedByClient"

💡 Result:

In the Playwright MCP server, allowedOrigins and blockedOrigins are configuration options used to control which network requests the browser is permitted to make [1][2]. These settings are managed within the server's network configuration and are implemented via Playwright's routing capabilities [3][4]. When these rules are active, the server intercepts network requests and uses the 'blockedbyclient' error code to terminate requests that do not align with the defined allow/block lists [3][4]. Specifically, the implementation follows this logic: 1. If allowedOrigins are defined, the server first blocks all requests by default using route.abort('blockedbyclient') [3][5]. 2. It then explicitly allows requests to the specified origins by calling route.continue for patterns matching the allowed list [3][4]. 3. If blockedOrigins are defined, the server explicitly blocks requests to those origins using route.abort('blockedbyclient') [3][4]. Origins matching both allowedOrigins and blockedOrigins will be blocked [1][2]. The allowed-origins command-line argument accepts a semicolon-separated list of origins, and the configuration supports full origin strings (e.g., https://example.com:8080) and wildcard port patterns (e.g., http://localhost:*) [1][6]. These settings are considered convenience features for managing browser interactions rather than strict security boundaries [1][2]. The file program.ts within the Playwright repository is involved in managing the command-line interface and tool configuration for the MCP server [7].

Citations:


Reinforce the localhost-first prompt with a technical restriction.

This harness requires localhost-only navigation, but mcp.json relies only on scripts/explore/explorer-prompt.md:13 for enforcement. Add a Playwright MCP restriction such as --allowed-origins http://localhost:3000;http://localhost:5000;127.0.0.1:5000 or --blocked-origins * plus the localhost allowlist, while documenting that these flags are not a security boundary against redirects; this gives defense-in-depth against model-driven or page-driven off-localhost clicks.

🤖 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/explore.sh` around lines 246 - 261, Update write_mcp_config to pass
Playwright MCP localhost-only origin restrictions in the generated args, using
the required localhost/127.0.0.1 allowlist or equivalent blocked-origins plus
allowlist configuration. Document alongside the configuration that these flags
are defense-in-depth only and do not prevent redirects from reaching
off-localhost destinations.

Comment threadscripts/explore.sh
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

Code review

Found 3 issues:

  1. do_down() can leak the stack and the machine-singleton lock: it runs under set -euo pipefail and fires as an EXIT trap, but its mkdir -p, the findings-file bootstrap, and the { … } >> findings.md append block have no || true guards (unlike the oracle subshell and e2e-down.sh lines around them). A failed write there (disk full, read-only fs) aborts the trap before e2e-down.sh and stop_lock_holder run — verified bash semantics: a failing command inside an EXIT trap under set -e stops the trap. The header comment's "|| true guards throughout" claim is currently false for these lines.

do_down() {
mkdir -p "$EXPLORE_DIR"
(
cd"$REPO_ROOT/backend"
venv/bin/python -m e2e_oracles --json >"$EXPLORE_DIR/oracle-final.json"||true
venv/bin/python -m e2e_oracles >"$EXPLORE_DIR/oracle-final.txt"2>&1||true
) ||true
[ -f"$EXPLORE_DIR/findings.md" ] ||printf'# Exploration findings\n'>"$EXPLORE_DIR/findings.md"
{
printf'\n## Oracle final pass (%s)\n\n```\n'"$(date -Iseconds)"
cat "$EXPLORE_DIR/oracle-final.txt"2>/dev/null ||true
printf'```\n'
} >>"$EXPLORE_DIR/findings.md"
"$REPO_ROOT/scripts/e2e-down.sh"||true
stop_lock_holder
}

  1. Dummy-key mitigation for RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439 defeats itself in the common developer environment: export GEMINI_API_KEY="${GEMINI_API_KEY:-e2e-dummy-key-no-billing}" defers to an ambient real key, and developers here routinely keep one exported (validate_config fail-closes without it). The below-seam RAG/embedding path (RAG embedding path sits below the SAPLING_MODEL_MODE seam — live embed calls fire even in function mode #439) bills live even in function mode, and the explorer's mission (uploads, quiz, tutor turns, many turns) is worst-case volume for it. CI applies the dummy key unconditionally for exactly this reason; export inside the script can't touch the operator's parent shell, so the conditional protects nothing.

# Default only if unset — never clobber an operator's real key.
export GEMINI_API_KEY="${GEMINI_API_KEY:-e2e-dummy-key-no-billing}"
"$REPO_ROOT/scripts/e2e-up.sh"|| die "scripts/e2e-up.sh failed — see .e2e/*.log"

  1. jq is a hard dependency of the transcript pipeline but is missing from preflight (claude/npx/curl/python3 are checked). Without it the script boots the entire stack — Supabase, migrations, a production Next build, minutes of work — and only then fails in run_explorer, instead of failing fast like the other four checks.

echo"▶ Preflight…"
command -v claude >/dev/null 2>&1|| die "claude CLI not found on PATH — install it first"
command -v npx >/dev/null 2>&1|| die "npx not found on PATH — install Node.js first"
command -v curl >/dev/null 2>&1|| die "curl not found on PATH"
command -v python3 >/dev/null 2>&1|| die "python3 not found on PATH"
[ -f"$REPO_ROOT/scripts/explore/explorer-prompt.md" ] \

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

…eflight, scoped Edit, EXPLORE_USER wiring (#399)
- do_down: guard every fallible write with || true so a failed findings.md
write can never abort the EXIT trap before e2e-down.sh / stop_lock_holder
run (was leaking the stack + machine-singleton lock on write failure).
- GEMINI_API_KEY: export unconditionally (matches CI's dummy, #439) instead
of deferring to an ambient real key that can bill the below-seam RAG path.
- SEED_RICH=1 exported unconditionally so an ambient SEED_RICH=0 can't
silently defeat the rich dataset this harness requires.
- preflight: add missing `jq` check (hard dependency of the transcript
pipeline) so a missing jq fails in seconds, not after a full stack boot.
- allowedTools: replace unscoped Write,Edit with Read,Edit(.explore/**) —
Write(...) patterns aren't matched by the CLI's file permission check, so
only a path-scoped Edit rule actually restricts writes to findings.md.
do_up now pre-creates .explore/findings.md so the explorer always has an
existing file for the scoped Edit grant.
- EXPLORE_USER: derive the sapling_user display name from the user id
(case map for the five seeded rich-* users, verified against
db/seed_local_rich.py) instead of hardcoding "Rich Active"; pass
--user "$EXPLORE_USER" to both oracle invocations in do_down.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

All three review findings fixed in a0360ff (do_down teardown/lock-release guards verified with a real write-failure injection; unconditional dummy GEMINI_API_KEY per #439; jq preflight), plus three panel-verified hardening items in the same commit: allowedTools Write dropped / Edit scoped to .explore/**, EXPLORE_USER wired through the name map and both oracle --user calls, unconditional SEED_RICH=1. Scoped re-review confirms all addressed, no new breakage.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(agent): LLM explorer harness driving Playwright against the local stack

1 participant

@AndresL230