Skip to content

fix(playwright): shell- and regex-escape test paths#134

Open
pbomb wants to merge 2 commits into
KnapsackPro:mainfrom
pbomb:pbomb/fix-playwright-shell-escape
Open

fix(playwright): shell- and regex-escape test paths#134
pbomb wants to merge 2 commits into
KnapsackPro:mainfrom
pbomb:pbomb/fix-playwright-shell-escape

Conversation

@pbomb
Copy link
Copy Markdown

@pbomb pbomb commented May 13, 2026

Problem

packages/playwright's onSuccess joins test file paths with spaces and passes them to npx playwright test via shell: true. Two related bugs make this break on real-world repos:

1. Shell metacharacters break /bin/sh parsing

Paths are not shell-escaped, so any shell metacharacter in a path breaks the parser before Playwright ever runs. The batch never produces .knapsack-pro/batch.json, and the next readFileSync('.knapsack-pro/batch.json') throws ENOENT, failing the whole CI job.

We hit this at Gusto. Our Playwright tests live under route-group folders like apps/gusto/libs/app-*/src/pages/(layout)/. The ( breaks /bin/sh parsing on the Knapsack-spawned command:

/bin/sh: 1: Syntax error: "(" unexpected
Error: ENOENT: no such file or directory, open '.knapsack-pro/batch.json'

2. Playwright treats positional args as regex filters

Even with shell-escaping, Playwright's CLI interprets each positional argument as a regex matched against test file paths, not a literal path. Paths containing regex metacharacters (., (, ), [, ], *, +, ?, ^, $, {, }, |, \) fail to match the intended file, and Playwright errors with:

Error: No tests found

I confirmed this layer was necessary by running CI with only the shell-escape fix: batches whose paths had no regex metacharacters ran cleanly, but every batch that included a (...)-named folder hit "No tests found".

Fix

Two-layer escape on each path before joining:

  1. Regex-escape so Playwright's positional-arg regex matches the literal path.
  2. Single-quote (with the standard '\'' inner-quote escape) so the shell passes the regex-escaped string through unchanged.

Organized as two commits:

  • fix(playwright): shell-escape test paths in onSuccess
  • fix(playwright): regex-escape test paths for Playwright CLI

Compatibility

No public API change. Paths with no shell or regex metacharacters behave identically.

pbomb added 2 commits May 13, 2026 15:32
Test file paths containing shell metacharacters like (, ), $, [, ], or
spaces broke the shell when Knapsack invoked Playwright via `shell: true`.
This happens in real-world repos that use route-group folder names like
`pages/(layout)/...`. Single-quote each path before joining.
Playwright's CLI treats positional args as regex filters against test
file paths. Paths containing regex metacharacters like `(`, `)`, `.`,
`[`, `]`, `*`, etc. don't match — Playwright errors with "No tests
found". Real-world repos with route-group folders like `pages/(layout)/`
hit this. Regex-escape each path before shell-quoting.
@pbomb pbomb changed the title fix(playwright): shell-escape test paths in onSuccess fix(playwright): shell- and regex-escape test paths May 14, 2026
@3v0k4
Copy link
Copy Markdown
Contributor

3v0k4 commented May 14, 2026

Hey Matt, thanks for the PR and the detailed description.

We'll review as soon as possible.

I see this is a draft, are you planning to do more work on this? Does this fix all the issues you have with PW?

@pbomb pbomb marked this pull request as ready for review May 14, 2026 14:22
@pbomb
Copy link
Copy Markdown
Author

pbomb commented May 14, 2026

Hi @3v0k4. I just marked it ready to review after bringing another change that actually fixed the issue for us. It wasn't quite right after the first commit. Happy to work with you on and test out any different approaches you might have.

Sign up for free to 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.

2 participants