[kyle-sexton]

Closes #11902

Summary

The playwright SessionStart hook no longer installs demo-video's numpy and Pillow (about 91 MB) when the plugin-defined opt-in EVAL_PLAYWRIGHT_SKIP_PYDEPS=1 is set. The plugin's four eval cases set it in their env, so their measured runs install nothing and get no install notice. This is the approach the #11902 decision chose: an EVAL_* variable, the channel the plugin-eval docs give a case for passing the plugin anything, rather than inferring an eval run from the sandbox directory name. Unset, or any value other than 1, the hook behaves as before, so a demo-video eval case leaves it unset and installs as a session does. pydeps.py run is unchanged and still never installs.

This PR carries the lane PR #11923's commits (merged in, history kept) plus the one change that lane's path check refused: the contract's Exceptions row.

Fix

• plugins/playwright/hooks/install-python-deps.sh: exit 0 before looking for Python when EVAL_PLAYWRIGHT_SKIP_PYDEPS is exactly 1; the header comment says what it does and who sets it
• plugins/playwright/hooks/install-python-deps.test.sh: a case with the variable set to 1 (exit 0, no output, no python/ directory) and cases for unset, empty, 0, true, 1 and 1x (each still installs); the suite unsets the variable at the top so an operator's export cannot skew it
• plugins/playwright/evals/*/prompt.md (four cases, none using demo-video): env: {EVAL_PLAYWRIGHT_SKIP_PYDEPS: "1"}
• docs/contracts/on-demand-dependencies/README.md: a row in the Exceptions table for the playwright demo-video set under rule P2, naming the variable and the reason (eval runs that never use demo-video install nothing); the P2 text is unchanged
• .changes/playwright/work-resolve-issue-issue-11902-9c4e1b7a.md: patch fragment naming the variable and saying a demo-video eval case must leave it unset

Verification

Run locally on the merged branch head:

• bash plugins/playwright/hooks/install-python-deps.test.sh: passed 16, failed 0
• python3 -I plugins/evals/skills/validate/scripts/validate-cases.py plugins/playwright/evals: exit 0, no findings
• bash scripts/check-changelog-fragments.sh --check: all 113 fragments valid
• markdownlint-cli2 docs/contracts/on-demand-dependencies/README.md: 0 issues
• pre-push check-affected: passed

Not verified: whether a live claude plugin eval run passes a case's env to the plugin's SessionStart hook process. The docs say env reaches the child session and do not mention hooks; the decision tracks that check on host-run issue #6587.

Related

Related: replaces #11923

• Decision: fix(playwright): stop installing demo-video Python deps on every SessionStart #11902 (comment)
• Live-run check: Concise skills rollout: playwright (3 skills, 2 model-invoked) #6587

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-01T13:06:53.595103Z	0e5840e	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

claude-security-review reviewed 0e5840e74fb45b2e35af48904c6c2a6855c982d5: read 8 of 8 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #11929, no security issues found

I read every hunk of all 8 changed files in .claude-lane/pr.diff. None of them matches a TRUSTED PATHS: glob. The contract doc is under docs/contracts/, not docs/conventions/ or docs/standards/.

What changed: plugins/playwright/hooks/install-python-deps.sh now exits 0 before installing anything when EVAL_PLAYWRIGHT_SKIP_PYDEPS is exactly 1. The four playwright eval cases set that variable in their env. The test suite covers set, unset and near-miss values. The on-demand-dependencies contract gets an Exceptions row, and a changelog fragment is added.

What I checked:

• The skip itself. The only effect of the new branch is to skip the hook's pip install of the hashed numpy and Pillow wheels. Nothing new is installed, run or fetched. To set the variable, a party must already control the hook's process environment, and that is already full control of the session. So the variable gives no one more access than they already have. When the packages are missing, pydeps.py run still prints the repair line and exits without installing, so the skip fails closed rather than falling back to an unpinned install.
• Instruction-surface lens. The contract change adds an exception row under P2. It does not delete, narrow or soften any rule about where or how installs happen. P2 still forbids installing while a skill runs, and hash pinning is untouched. Nothing new can now reach an install, a credential or a write.
• Plugin manifests. No .claude-plugin/plugin.json or marketplace.json changed, and no userConfig key changed. The hook's registration is unchanged.
• Embedded text in the PR context. I found no directive aimed at the review.
• Trust controls, tokens, secrets and Actions workflows. The diff touches none of them.

Spend: none. The change only cuts install time out of eval runs.

critical-findings: 0

[github-actions]

claude-review reviewed 0e5840e74fb45b2e35af48904c6c2a6855c982d5: read 8 of 8 changed file(s); 0 finding(s), 0 as inline comments.

Code review: no findings

I read every hunk of all 8 changed files in .claude-lane/pr.diff. I also read the full hook (plugins/playwright/hooks/install-python-deps.sh), its test suite, and the contract's Rule P2 text, and I checked how the eval-case validator handles env. None of the changed files match a trusted-path glob. The changed path is docs/contracts/**, not docs/conventions/** or docs/standards/**.

The change. The playwright SessionStart hook now stops before it looks for Python when EVAL_PLAYWRIGHT_SKIP_PYDEPS is exactly 1. The four playwright eval cases set it through env, and none of them use demo-video. A changelog fragment and a new row in the on-demand-dependencies Exceptions table record the exception.

What I checked:

• Hook guard (install-python-deps.sh:14): [[ "${EVAL_PLAYWRIGHT_SKIP_PYDEPS:-}" == 1 ]] && exit 0 is safe under set -uo pipefail. The script has no -e, so the guard's false result does not end the hook. The :- default keeps -u from failing when the variable is unset. The == comparison is exact, so 1 and 1x do not match it.
• Tests:
  • The suite unsets the variable at the top, so a value the operator exported cannot change the results.
  • The skip case checks exit 0, no output, and that $data/python does not exist.
  • The loop over unset, '', 0, true, 1 and 1x gives each value its own data directory. The ${#value}-${value//[^A-Za-z0-9]/_} key keeps the names apart, and each value is checked for exactly one install.
  • run_hook passes the variable through env NAME=VALUE, so the empty-string case really sets it to empty rather than leaving it unset.
• Eval cases: env: {EVAL_PLAYWRIGHT_SKIP_PYDEPS: "1"} is a flow mapping. Its key matches the EVAL_[A-Z0-9_]* pattern that plugins/evals/skills/validate/scripts/validate-cases.py enforces (lines 680–693).
• Contract row: it says P2's own text is unchanged and that pydeps.py run still never installs, which matches lines 116–130.
• Changelog fragment: bump: patch fits a fix that is off unless a run sets the variable. The diff touches no plugin.json or marketplace.json, so the manifest checks do not apply.

Closing issue #11902:

• "Either way a trigger-only run installs nothing": met in the hook. One assumption is still open. The PR description says outright that nobody has confirmed whether a live claude plugin eval run passes a case's env to the plugin's SessionStart hook process. The issue's decision comment hands that check to Concise skills rollout: playwright (3 skills, 2 model-invoked) #6587. I could not verify it either. It is not a code defect, but the fix only works in a live eval run if that holds.
• "Add a test showing that a SessionStart under the eval sandbox installs nothing": met, as the decision comment redefined it (the sandbox is signalled by the opt-in variable). The EVAL_PLAYWRIGHT_SKIP_PYDEPS=1 installs nothing and says nothing case at install-python-deps.test.sh:124-133 would fail if the guard broke.

What I did not verify. The PR's Verification section (test suite 16/0, validate-cases, fragment check, markdownlint, pre-push) is the author's claim. I had no shell and ran none of it.

No paid resources or billing changes are involved. The change slightly reduces eval-run install time and disk use.