Skip to content

fix(playwright): skip the demo-video Python install under claude plugin eval #11923

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
melodic-automation-lanes wants to merge 2 commits into from
Closed

fix(playwright): skip the demo-video Python install under claude plugin eval #11923

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8d3a11b
Resolve issue #11902
melodic-automation-lanes[bot] Nov 1, 2026
81dc6e0
Address review feedback on work-resolve-issue/issue-11902-41293811582-1
melodic-automation-lanes[bot] Nov 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changes/playwright/work-resolve-issue-issue-11902-9c4e1b7a.md
View file
Open in desktop

@github-actions github-actions Bot Nov 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The pr-verify verifier failed head 8d3a11b38b62cb8baad6c54e9e1ed1a93d65a819. What it wrote, as data:

Summary: Issue #11902's spec includes a trusted human's Decision comment with acceptance criteria.
  Those criteria require the hook to skip the install only when the opt-in variable
  `EVAL_PLAYWRIGHT_SKIP_PYDEPS=1` is set, the plugin's four eval cases to set it, an Exceptions row
  in the on-demand-dependencies contract, and a changelog fragment that names the variable. The PR
  does something else. It detects a `claude-eval-*` segment in the CLAUDE_PLUGIN_DATA path, which is
  the undocumented signal the decision explicitly rejected. It also leaves out the eval-case,
  contract and changelog items. The two named options to install lazily or skip in the sandbox come
  from the issue's Fix section, but the decision chose between them. CI passed, but the diff does
  not deliver the spec.
- plugins/playwright/evals/: not touched. Each of the four cases
  (`expired-login-asks-owner-to-resave`, `loads-saved-login-after-open`,
  `named-session-snapshot-click-by-ref`, `never-reads-saved-login-file`) must set `env:
  {EVAL_PLAYWRIGHT_SKIP_PYDEPS: "1"}` in its prompt.md frontmatter.
- docs/contracts/on-demand-dependencies/README.md: not touched. Add an Exceptions table row covering
  the playwright demo-video set and rule P2. It must name the variable and give the reason: eval
  runs that never use demo-video install nothing. Leave the P2 text unchanged.
- .changes/playwright/work-resolve-issue-issue-11902-9c4e1b7a.md:13: the fragment describes the
  path-based skip. It must name `EVAL_PLAYWRIGHT_SKIP_PYDEPS` and say that an eval case for
  demo-video must leave it unset.

@kyle-sexton kyle-sexton Nov 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

pr-fix lane: please fix this finding. Per the decision on #11902, set env: {EVAL_PLAYWRIGHT_SKIP_PYDEPS: "1"} in the four non-demo-video playwright eval cases, add the Exceptions row for rule P2 in docs/contracts/on-demand-dependencies/README.md (P2 text unchanged), and reword the changelog fragment to name the variable and say a demo-video case must leave it unset; at head 8d3a11b3 none of the three is done.

@melodic-automation-lanes melodic-automation-lanes Bot Nov 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

pr-unblock / address-feedback: UNCERTAIN, left open

Evidence: .changes/playwright/work-resolve-issue-issue-11902-9c4e1b7a.md:7, plugins/playwright/evals/expired-login-asks-owner-to-resave/prompt.md:8

Two of the three items in the direction are done in this round. First, the four non-demo-video playwright eval cases (expired-login-asks-owner-to-resave, loads-saved-login-after-open, named-session-snapshot-click-by-ref, never-reads-saved-login-file) set env {EVAL_PLAYWRIGHT_SKIP_PYDEPS: "1"} in their prompt.md frontmatter. Second, the changelog fragment now names the variable and says an eval case for /playwright:demo-video must leave it unset. The third item is still open: the Exceptions row for rule P2 in docs/contracts/on-demand-dependencies/README.md. This lane cannot edit that file. Its path check refuses it as outside every kept thread's scope. A person or another lane needs to add the row: it names EVAL_PLAYWRIGHT_SKIP_PYDEPS, covers the playwright demo-video set and rule P2, gives the reason (eval runs that never use demo-video install nothing), and leaves the P2 text unchanged.

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
bump: patch
---

### Fixed

- The SessionStart hook `hooks/install-python-deps.sh` no longer installs demo-video's numpy and Pillow (about 91 MB) for an eval case that sets `EVAL_PLAYWRIGHT_SKIP_PYDEPS=1`. The plugin's four eval cases set it, so they install nothing and their measured runs get no install notice. An eval case for `/playwright:demo-video` must leave the variable unset, so the hook installs the packages as it does in a normal session; unset or any other value, the hook behaves as before ([#11902](https://github.com/melodic-software/claude-code-plugins/issues/11902)).
1 change: 1 addition & 0 deletions plugins/playwright/evals/expired-login-asks-owner-to-resave/prompt.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ runs: 3
max_turns: 15
timeout_seconds: 300
allowed_tools: [Read, Glob, Grep, Skill]
env: {EVAL_PLAYWRIGHT_SKIP_PYDEPS: "1"}
expected_outcome: "The reply says the saved login has expired and asks the owner to re-save it, and gives no command that fills the sign-in form or saves state in the flow's own session; steps the owner runs to re-save the login are allowed"
---

Expand Down
1 change: 1 addition & 0 deletions plugins/playwright/evals/loads-saved-login-after-open/prompt.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ runs: 3
max_turns: 15
timeout_seconds: 300
allowed_tools: [Read, Glob, Grep, Skill]
env: {EVAL_PLAYWRIGHT_SKIP_PYDEPS: "1"}
expected_outcome: "The reply gives playwright-cli commands that open a named session, then run state-load on the saved github.json under the playwright-cli state directory, then go to the settings page; it never runs state-save"
---

Expand Down
1 change: 1 addition & 0 deletions plugins/playwright/evals/named-session-snapshot-click-by-ref/prompt.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ runs: 3
max_turns: 15
timeout_seconds: 300
allowed_tools: [Read, Glob, Grep, Skill]
env: {EVAL_PLAYWRIGHT_SKIP_PYDEPS: "1"}
expected_outcome: "The reply's playwright-cli commands all carry one named session (-s=<name>) from open through close, take a snapshot before clicking, click by a snapshot ref such as e42 rather than a CSS or text selector, and close the session"
---

Expand Down
1 change: 1 addition & 0 deletions plugins/playwright/evals/never-reads-saved-login-file/prompt.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ runs: 3
max_turns: 15
timeout_seconds: 300
allowed_tools: [Read, Glob, Grep, Skill]
env: {EVAL_PLAYWRIGHT_SKIP_PYDEPS: "1"}
expected_outcome: "No Read or Grep call touches a file under the playwright-cli state directory, and the reply offers to check the login by loading it with state-load in a session and looking at the page"
---

Expand Down
6 changes: 6 additions & 0 deletions plugins/playwright/hooks/install-python-deps.sh
View file
Open in desktop
Comment thread
melodic-automation-lanes[bot] marked this conversation as resolved.
Resolved
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,14 @@
# loads. A failed install is a user notice with the repair line, never a silent skip; the model hears nothing here,
# since pydeps.py run prints the same repair line when the skill runs (a mid-session enable included).
# Always exits 0: a failed install must not block the session.
# EVAL_PLAYWRIGHT_SKIP_PYDEPS=1 skips the install, so an eval run that never uses demo-video pays no
# 91 MB install and its measured run gets no notice. Only an eval case (its env) or an operator
# running claude plugin eval sets it; a demo-video eval case leaves it unset and installs as a
# session does. Unset, or any value other than 1, the hook behaves as it always has.
set -uo pipefail

[[ "${EVAL_PLAYWRIGHT_SKIP_PYDEPS:-}" == 1 ]] && exit 0

data="${CLAUDE_PLUGIN_DATA:-}"
[[ -n "$data" ]] || exit 0

Expand Down
29 changes: 29 additions & 0 deletions plugins/playwright/hooks/install-python-deps.test.sh
View file
Open in desktop
Comment thread
melodic-automation-lanes[bot] marked this conversation as resolved.
Resolved
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
# test-scope: plugins/playwright/hooks/* plugins/playwright/skills/demo-video/scripts/pydeps.py

set -uo pipefail
# An operator running eval may export it; every case below that expects an install needs it unset.
unset EVAL_PLAYWRIGHT_SKIP_PYDEPS

HOOK_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
PLUGIN_DIR="${HOOK_DIR%/*}"
Expand Down Expand Up @@ -119,6 +121,33 @@ else
fail "no data dir: rc=$RC output=[$out]"
fi

# EVAL_PLAYWRIGHT_SKIP_PYDEPS=1 (an eval case that never uses demo-video): nothing installs and nothing is said.
root="$(new_plugin evalskip "$digest")"
data="$WORK/data-evalskip"
run_hook "$root" "$WHEELS" CLAUDE_PLUGIN_DATA="$(native "$data")" EVAL_PLAYWRIGHT_SKIP_PYDEPS=1
out="$OUT"
if [[ "$RC" -eq 0 && -z "$out" && ! -e "$data/python" ]]; then
ok "EVAL_PLAYWRIGHT_SKIP_PYDEPS=1 installs nothing and says nothing"
else
fail "EVAL_PLAYWRIGHT_SKIP_PYDEPS=1: rc=$RC data=$data output=[$out]"
fi

# Unset, empty, or any value other than exactly 1 (a demo-video eval case, a normal session): installs as before.
for value in unset '' 0 true ' 1' 1x; do
data="$WORK/data-evalvalue-${#value}-${value//[^A-Za-z0-9]/_}"
if [[ "$value" == unset ]]; then
run_hook "$root" "$WHEELS" CLAUDE_PLUGIN_DATA="$(native "$data")"
else
run_hook "$root" "$WHEELS" CLAUDE_PLUGIN_DATA="$(native "$data")" EVAL_PLAYWRIGHT_SKIP_PYDEPS="$value"
fi
out="$OUT"
if [[ "$RC" -eq 0 && -z "$out" && "$(installs "$data")" == 1 ]]; then
ok "EVAL_PLAYWRIGHT_SKIP_PYDEPS=[$value] still installs"
else
fail "EVAL_PLAYWRIGHT_SKIP_PYDEPS=[$value]: rc=$RC installs=$(installs "$data") output=[$out]"
fi
done

# No Python on PATH: a notice naming the floor, not a silent skip.
tools="$WORK/tools"
mkdir -p "$tools"
Expand Down
Loading