Fix Jest runner discovery for deeply nested / dynamic-route TS tests

[Serhan-Asad]

Problem

get_test_command.py's TypeScript runner detector (_detect_ts_test_runner) left colocated Next.js app-page suites unexecutable through PDD, discovered while addressing the review on promptdriven/pdd_cloud#3251 (issue promptdriven/pdd_cloud#3024). Two independent defects:

1. Shallow discovery. It walked up only 5 parent directories looking for a jest/vitest/playwright config. A colocated page test such as frontend/src/app/hackathon/[eventId]/team/__tests__/test_page.tsx sits 6–8 directories below frontend/jest.config.js, so the config was never found and resolution fell back to npx tsx <file> — which has no describe/it globals and runs from the wrong cwd.

2. Regex path targeting. Jest was invoked as npx jest --no-coverage -- <abs path>. Jest treats the trailing path as a regex; Next.js dynamic-route segments ([eventId], [slug]) are regex character classes, so the literal bracketed path matched nothing (No tests found, exiting with code 1).

Net effect: a regenerating pdd change/sync could not execute these generated suites, preserving a false-green split (generator reports its suite passing while CI's own runner never executed it).

Fix

• Walk up until a runner config is found or the JS project root (nearest package.json) is reached, with a defensive iteration cap. The nearest ancestor config still wins, and the search never escapes above the project root.
• Invoke Jest with --runTestsByPath so the resolved absolute path is matched literally, regardless of bracketed dynamic-route segments.

Verification

Against promptdriven/pdd_cloud#3251's six migrated app-page suites, all six now resolve to the frontend Jest runner (cwd = frontend/) and execute through PDD's real command path: 339/339 tests pass (previously: 2/6 executed, 4/6 returned "No tests found").

New regression tests cover both defects (deep discovery, --runTestsByPath, literal bracketed path, and the package.json project-root boundary). Prompt (get_test_command_python.prompt) and fingerprint (.pdd/meta/...) updated to match.

Downstream

Unblocks the complete fix for promptdriven/pdd_cloud#3251 (coordinated with #1984). On release, pdd_cloud re-pins pdd-cli and its guarded --runTestsByPath regression activates.

🤖 Generated with Claude Code

[Serhan-Asad]

Adversarial review loop (gpt-5.6-codex, xhigh) — resolved

Two rounds of independent review; all correctness findings fixed:

Round 1

• Deep discovery past the 5-parent cap; --runTestsByPath so bracketed dynamic-route paths match literally; regressions + prompt/fingerprint aligned.

Round 2

• Boundary (fixed): the interim .git-only boundary over-corrected — an independent package inside a git repo would adopt the repo-root config. Now: stop at the JS project root (nearest package.json), cross it only when the package is a member of an ancestor workspace (workspaces field / pnpm-workspace.yaml / lerna.json), and never cross the repository root. Added _belongs_to_ancestor_workspace + negative tests (workspace-root inheritance, independent leaf, non-git stray ancestor).
• Shell quoting (scoped): the resolved path is shlex.quoted — correct for pdd's POSIX-shell command convention (all callers run shell=True / shlex.split). Full Windows-cmd.exe safety would require moving every caller to an argv list + shell=False, a pre-existing cross-cutting change out of scope for runner detection; documented in-code.

Verified: the six pdd_cloud app-page suites all resolve to the frontend Jest runner and execute 339/339 through PDD's real command path; existing runner-detection tests green.

[Serhan-Asad]

Review round 3 — resolved

• Workspace membership (fixed): the round-2 workspace-aware boundary treated any ancestor workspaces/pnpm/lerna declaration as membership, so an unrelated package (e.g. vendor/tool) beneath a workspace root could wrongly adopt the root config. Membership is now proven — the ancestor's declared package globs must actually match the leaf's path relative to that ancestor (segment-wise * = one segment, ** = any depth); unparseable pnpm YAML is treated conservatively as non-member. Added a negative test (vendor/tool under workspaces: ["packages/*"] → no match) plus glob unit coverage.

All correctness findings across three review rounds are addressed. The only intentionally-scoped item is Windows cmd.exe quoting, which requires moving pdd's codebase-wide command-as-string convention to argv/shell=False (documented, out of scope for runner detection).

[gltanaka]

Review verdict: changes requested

This PR is needed. The original failure is real: deeply nested TypeScript suites can miss the runner config and fall back to npx tsx, while Jest interprets bracketed dynamic-route paths as regexes without --runTestsByPath. The deep walk, correct cwd, literal Jest targeting, and shell quoting solve that concrete pdd_cloud failure. I also independently confirmed the six downstream suites select cwd=frontend and execute 339/339 through the returned TestCommand at this PR head.

One correctness issue still blocks merge:

• Workspace exclusions are ignored — pdd/get_test_command.py:120-127. _workspace_globs_for() returns positive and negative pnpm patterns as indistinguishable strings, then _belongs_to_ancestor_workspace() uses any(...). With the officially supported pnpm shape packages: ['packages/**', '!**/test/**'], a package under packages/app/test/fixture matches the positive pattern, the exclusion never takes effect, and the code reports member=True. I reproduced the resulting behavior at 082244d6: detection crosses that package's own package.json boundary and selects the repository-root Jest config. An explicitly excluded/independent package can therefore execute the wrong ancestor runner—the same false-positive boundary this PR is intended to prevent. The custom matcher also treats other valid workspace glob syntax such as brace expansion literally, so membership is not yet faithfully proven for supported npm/Yarn/pnpm declarations.

Please implement actual include/exclude workspace-pattern semantics (or use a faithful matcher) and add focused coverage for a broad positive plus matching negative pattern, including pnpm's documented !**/test/** form. Brace-expansion coverage should be added if npm/Yarn workspace declarations remain advertised as supported.

Validation performed at the current head:

• tests/test_get_test_command.py: 65 passed
• compile and git diff --check: passed
• real downstream runner boundary: 6 suites, 339/339 passed
• all GitHub checks: green

The testing is strong for the original deep-path/Jest defect, but it is not sufficient for the expanded workspace-membership implementation: the automated tests cover only simple positive packages/* matching and do not exercise pnpm exclusions or richer workspace glob semantics. The PR also does not include the exact CLI workflow/transcript requested by docs/runbooks/pr-loop-process.md; please include reproducible final evidence with the follow-up.

[Serhan-Asad]

Changes-requested addressed — workspace include/exclude + brace semantics

Thanks @gltanaka. The finding is correct and now fixed with TDD (failing test → fix):

• test(get_test_command) — cac9ddc30
• fix(get_test_command) — 6a8b1fa65

Root cause

_workspace_globs_for() returned positive and !-negative patterns as indistinguishable strings and _belongs_to_ancestor_workspace() used any(...), so pnpm's supported !**/test/** never took effect (a leading ! also didn't even match under the custom matcher), and brace alternations were matched literally. A package under packages/app/test/fixture therefore reported member=True and crossed its own package.json boundary to the repo-root Jest config.

Fix

Membership now requires ≥1 positive glob match AND no ! exclusion match, with brace {a,b} expansion and segment-wise */** matching. New helpers _expand_braces, _split_top_level_commas, _package_matches_workspace; pnpm YAML parsing narrowed to specific exceptions (unparseable → conservatively non-member).

Red → green (the two reproducing tests)

Before (082244d6):

FAILED test_pnpm_exclusion_pattern_excludes_matching_package
  -> npx jest --no-coverage --runTestsByPath .../packages/app/test/fixture/... (adopted repo config)
FAILED test_brace_expansion_in_workspace_glob_matches_member
  -> npx tsx .../packages/app/...                                            (member missed)

After (6a8b1fa65): 2 passed, full detection/monorepo suite 39 passed.

Membership unit checks:

packages/app/test/x  vs [packages/**, !**/test/**] -> False   (excluded)
packages/app         vs [packages/**, !**/test/**] -> True    (member)
packages/app         vs [packages/{app,lib}]        -> True    (brace expanded)
vendor/tool          vs [packages/*]                -> False   (unrelated)

Coverage added

test_pnpm_exclusion_pattern_excludes_matching_package (broad positive packages/** + pnpm !**/test/**), test_brace_expansion_in_workspace_glob_matches_member, on top of the existing workspace-root-inheritance, independent-leaf, and non-git negative tests.

Verification (head 6a8b1fa65)

• tests/test_get_test_command.py detection/workspace suites: 39 passed
• pylint pdd/get_test_command.py: 9.95/10 (only a pre-existing open()-encoding warning in _load_language_format, untouched)
• Downstream real runner boundary: 6/6 suites resolve to cwd=frontend, 339/339 through the returned TestCommand
• git log origin/main..HEAD --oneline: intentional commits only; no regenerated architecture.json/prompts/unrelated pdd/ files

Re: the runbook transcript — this reproducible red→green + downstream evidence is captured above; happy to attach anything further you'd like in the requested format.

[Serhan-Asad]

Coordination note from the #1998 stack orchestrator (stack/e2e-staging-v2).

Checked for overlap: #1998 does not touch pdd/get_test_command.py, its prompt, or tests/test_get_test_command.py, so there is no code conflict between #2003 and #1998 — they can land in either order.

The only coupling is evidence-level: #1998's real-execution/E2E staging runs exercise the test-runner discovery path this PR fixes. If #2003 lands first, #1998's staging evidence benefits from the corrected deeply-nested / dynamic-route TS runner discovery; if #1998 lands first, its evidence was gathered against the pre-fix discovery. No blocker either way — flagging for the manager's awareness when sequencing.