Fix Jest runner discovery for deeply nested / dynamic-route TS tests by Serhan-Asad · Pull Request #2003 · promptdriven/pdd

Serhan-Asad · 2026-07-11T21:46:51Z

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:

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.
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 · 2026-07-12T01:54:14Z

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 · 2026-07-12T02:11:03Z

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 · 2026-07-12T05:08:41Z

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 · 2026-07-12T18:23:11Z

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.

Serhan-Asad · 2026-07-13T04:27:19Z

CI / review triage at current head `7d9016bfe`

Posting exact evidence on the three red/blocking signals. Not merging — final merge remains with the maintainers.

1. `@gltanaka`'s `CHANGES_REQUESTED` blocker is resolved at the current head

That review was submitted against an earlier head (082244d6, "65 passed"). Its sole blocking issue — workspace exclusions ignored, and brace expansion treated literally — is fixed. Reproduced at 7d9016bfe:

_package_matches_workspace(("packages","app","test","fixture"), ["packages/**", "!**/test/**"]) -> False   # excluded
_package_matches_workspace(("packages","app"),                  ["packages/**", "!**/test/**"]) -> True    # member
_package_matches_workspace(("packages","app"), ["packages/{app,lib}"]) -> True     # brace expansion
_package_matches_workspace(("packages","web"), ["packages/{app,lib}"]) -> False

The matcher now implements full pnpm/npm/Yarn include-exclude semantics with a hand-written, faithful minimatch subset (per-/-segment */**/? over UTF-16 code units; {a,b} brace expansion masked around opaque ${…}; ranges/classes/extglobs validated per segment and failed closed when unsupported). The exact coverage requested exists: test_pnpm_exclusion_pattern_excludes_matching_package uses the literal ['packages/**', '!**/test/**'] form, alongside 10+ brace-expansion tests. tests/test_get_test_command.py is now 159 tests (up from 65 at 082244d6), all green locally + E2E against a real Jest workspace with a [eventId] dynamic route.

2. `Run Unit Tests` → red on `tests/test_sync_core_pdd_rollout_policy.py`, not this diff

main advanced 4 commits after this PR branched (#1992/#1994/#2009) adding the sync_core protected-PDD rollout-policy governance, which pins each protected prompt's immutable requirement contract. This PR legitimately edits 5 protected prompts, so test_rollout_profiles_cannot_self_authorize reports 937 != 466*2 — the extra 5 are exactly this PR's prompts (get_test_command_python + shell-quoting back-prop to agentic_fix/agentic_langtest/fix_error_loop/get_run_command). That is a signing/approval reconciliation owned by the sync framework (signer_process), not a defect in this diff. This job is not a required check.

3. `heal` / `auto-heal` → private Cloud Build, not a code defect

auto-heal output: "auto-heal failure. Cloud Build: 368d525f-07f4-4104-bd0b-586db02cfc48" — it dispatches a private pdd_cloud LLM-regeneration Cloud Build; the failure is inside that build. It has been red across every review round independent of this PR's contents, and is not a required check.

Required checks / merge gate

repos/promptdriven/pdd/branches/main/protection and .../rules/branches/main are both empty — there are no required status checks. Every authoritative GitHub check is green at 7d9016bfe: CodeQL, Analyze (actions/js-ts/python), Public CLI Regression, Story Regression, Package Preprocess Smoke, Repo Bloat Docker E2E.

Independent adversarial re-review (gpt-5.6-sol, xhigh, fresh detached checkout of the full base...HEAD diff) is ongoing; continuing to iterate to two consecutive clean passes.

get_test_command's TypeScript runner detector had two defects that left colocated Next.js app-page suites unexecutable through PDD: 1. It walked up only 5 parent directories looking for a jest/vitest/ playwright config, so a page test at e.g. frontend/src/app/hackathon/[eventId]/team/__tests__/ never reached frontend/jest.config.js and fell back to `npx tsx` (no describe/it, wrong cwd). The walk now continues to the JS project root (nearest package.json), with a defensive iteration cap, and never escapes above the project root. 2. Jest was invoked with a trailing bare path that Jest treats as a regex. Next.js dynamic-route segments ([eventId]/[slug]) are regex character classes, so the literal bracketed path matched nothing ("No tests found"). Jest is now invoked with `--runTestsByPath` so the resolved absolute path is matched literally. Verified against promptdriven/pdd_cloud#3251's six migrated app-page suites: all six now resolve to the frontend Jest runner and execute (339/339 tests) through PDD's real command path. Prompt and fingerprint updated to match; adds regression tests for both defects. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

PDD-Auto-Heal-Checkpoint: success

Round-1 review fixes for the TS test-runner detector: - Boundary regression (major): stopping the upward walk at the nearest package.json broke workspace monorepos, where a leaf package has its own manifest but inherits the Jest/Vitest/Playwright config from the workspace root. Walk instead to the repository root (nearest ancestor holding .git) — through intermediate package.json files — so the workspace-root config is still found; never escape above the repo root. - Playwright bracket handling (major): Playwright treats its positional argument as a regex, so a literal `.spec` path under a dynamic route ([slug]) never matched. Regex-escape the path for Playwright (Jest --runTestsByPath / Vitest keep it literal). - Shell safety (major): the resolved path was concatenated unquoted into a command string that callers run with shell=True, so spaces / bracket globs / $() could be re-split or reinterpreted. Shell-quote the path for every runner (also makes POSIX shlex.split round-trip cleanly). Prompt and fingerprint updated to match; adds regressions for workspace-root inheritance, the repo-root no-escape boundary, Playwright bracket escaping, and shell-quoted paths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Round-1 fixed a boundary that stopped at the nearest package.json (which missed workspace-root configs), but switching to a plain .git boundary over-corrected: an independent package inside a git repo would wrongly adopt the repository-root config, and a non-git tree could walk to the filesystem root. Boundary is now the JS project root (nearest package.json), crossed only when the package is a member of an ancestor workspace (a `workspaces` field, `pnpm-workspace.yaml`, or `lerna.json`), and never above the repository root (.git). Adds `_belongs_to_ancestor_workspace` plus regressions for an independent leaf package and a non-git stray-ancestor config; prompt + fingerprint updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Clarify that TestCommand.command is a POSIX-shell string (matching how all pdd callers execute verify commands) so shlex.quote is the correct quoting here; making runner execution safe under Windows cmd.exe would require moving all callers to argv + shell=False, a pre-existing cross-cutting change out of scope for runner detection. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…iew) _belongs_to_ancestor_workspace treated any ancestor `workspaces` (or pnpm/lerna) declaration as membership, so an unrelated package (e.g. `vendor/tool`) beneath a workspace root would wrongly adopt the root config. Now read the ancestor's declared package globs and require the leaf package's path (relative to the declaring ancestor) to actually match one, with segment-wise `*`/`**` semantics; unparseable pnpm YAML is treated conservatively as non-member. Adds a negative test for an unrelated package under a workspace root; prompt + fingerprint updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Failing tests for gltanaka's review finding: _belongs_to_ancestor_workspace ignores pnpm `!` exclusions and treats brace expansion literally. A package under packages/app/test/fixture (excluded by `!**/test/**`) is wrongly reported a member, and a packages/{app,lib} member is wrongly missed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

_belongs_to_ancestor_workspace treated any ancestor `workspaces`/pnpm/ lerna declaration as membership and matched globs literally, so a package explicitly excluded by pnpm's supported `!**/test/**` (or missed by a `{a,b}` brace glob) was mis-classified — an excluded/independent package could cross its own package.json boundary and adopt the repo-root runner. Membership now requires a positive glob match AND no `!` exclusion match, with brace alternations expanded and segment-wise `*`/`**` semantics. Adds `_expand_braces`, `_split_top_level_commas`, `_package_matches_workspace`; pnpm YAML parsing narrowed to specific exceptions. Prompt + fingerprint updated. Closes gltanaka's changes-requested review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Independent Codex gpt-5.6-sol xhigh review found six issues; five fixed (one rejected as pre-existing/out-of-scope, see below). - F2 nested workspace root: _workspace_root_for now returns the declaring workspace root, used as a traversal ceiling so an independent intermediate package.json between a member and its workspace root no longer stops the walk (e.g. member vendor/container/packages/app under root glob vendor/container/packages/*). Previously returned None instead of Jest. - F3 source precedence: pnpm-workspace.yaml is authoritative — pnpm ignores the package.json `workspaces` field, so a stale/attacker-controlled list no longer unions in and over-authorizes membership. Missing/unparseable pnpm YAML fails closed. - F4 malformed manifests: a package.json/lerna.json whose parsed top level is not an object ([] or a bare string) contributes no globs instead of raising AttributeError during discovery. - F5 symlink containment: the repo root is anchored lexically (nearest .git without following symlinks); a test dir symlinked outside the repo can no longer smuggle the walk into an out-of-repo config. In-repo symlinks still resolve normally. - F6 brace-bomb budget: brace expansion is bounded by _MAX_BRACE_EXPANSION; an untrusted {a,b}-style brace bomb fails membership closed instead of materializing an exponential list. F1 (Windows shell=True quoting) rejected: pdd's verify boundary is POSIX-only (callers use subprocess start_new_session=True; no Windows classifier; base already returned unquoted shell=True strings). The argv/shell=False migration is the cross-cutting change the module docstring explicitly scopes out. Prompt raised to doctrine altitude for the new behaviors; fingerprint meta re-synced (prompt/code/test hashes). +11 regression/negative-control tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…-5 review) Second independent Codex gpt-5.6-sol xhigh review (fresh, at the new head) found five medium issues, all in the untrusted-manifest trust-boundary class; all five fixed with regression coverage. - R2-1 symlink to foreign checkout: _lexical_repo_root only anchors at a non-symlinked directory, so a symlinked component whose `.git` probe would follow the link out of the tree (repo/link -> outside, both with .git) no longer mis-anchors containment; the out-of-repo config is refused. - R2-2 invalid-UTF-8 pnpm YAML: read now also catches UnicodeError → membership unproven instead of crashing discovery with UnicodeDecodeError. - R2-3 `**` exponential match / RecursionError: glob matching is now an iterative O(n*m) dynamic program (no recursion, no slicing) with a segment budget; a wall of `**` fails closed instead of backtracking/recursing. - R2-4 brace-bomb RecursionError: _expand_braces is iterative (worklist, not recursion) with worklist+output budgets, so deep nesting fails closed via _BraceBudgetError rather than escaping as RecursionError. _package_matches_ workspace now catches _PatternBudgetError/RecursionError defensively. - R2-5 non-string glob entries: `_string_globs` requires every declared entry to be a str; a JSON/YAML `true`/number makes the declaration malformed (no globs) instead of coercing to a glob like "True". Prompt raised to doctrine altitude for bounded/no-recursion pattern evaluation, string-only declarations, invalid-encoding fail-closed, and foreign-checkout symlink containment. Fingerprint meta re-synced. +7 regression tests (85 total). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…ine altitude (round-6 review) Third independent Codex gpt-5.6-sol xhigh review (fresh, at new head) found six findings; all addressed. - R3-1 symlink to nested foreign checkout: repository containment now anchors at the deepest-symlink boundary (component-aware), so a `.git` probe cannot follow a symlinked path component into a foreign checkout below it. Anchors at the true repo; the foreign config is refused. - R3-2 lexical-root depth cap: the lexical repo-root search now walks to the filesystem root (no artificial 200 cap), so a deep path ending in an escaping symlink still anchors containment and refuses the out-of-repo config. - R3-3 aggregate resource budget: brace expansion now shares one budget across the whole membership check; raw-glob count is capped; comma-splitting and segment-splitting are bounded before allocation. A many-glob or comma/slash-wall manifest fails closed instead of exhausting memory. - R3-4 pnpm YAML recursion bomb: YAML parsing also catches RecursionError → fail closed instead of crashing discovery. - R3-5 dotfile matching: glob matching applies minimatch `dot:false` — a wildcard segment no longer matches a leading-dot segment (so `packages/*` excludes `packages/.shadow`; `packages/.*` still includes it). - R3-6 prompt altitude: rewrote the prompt to stable numbered R1–R13 MUST/MUST NOT behavioral contracts with a Vocabulary and pinned Interface, removing transcribed private-helper names and algorithm recipes (DP/fnmatch/iterative), per docs/prompting_guide.md. Fingerprint meta re-synced. +11 regression tests (95 total). Suite green, E2E green, pylint E 10.00. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…& injection gaps (round-7 review) Fourth independent Codex gpt-5.6-sol xhigh review (fresh, at new head) found five findings; all fixed. - R4-1 (high) `..`+symlink mis-anchor: os.path.abspath collapses `..` textually, which is unsound across symlinks and could mis-anchor containment into a foreign checkout. Detection now fails closed when the naive-collapsed path and the true resolution disagree (a `..` traversed a symlink); a `..` with no symlink is unaffected. - R4-2 (high) placeholder re-injection: pdd/fix_error_loop.py re-ran {file}/{test} substitution on the already-complete, shell-quoted command, so a maliciously named path (containing `{test};touch …`) broke the quoting → command injection. get_test_command_for_file already substitutes CSV paths and embeds the quoted runner path, so the redundant re-substitution is removed and documented (prompt R14). - R4-3 config-file symlink escape: a runner config that is itself a symlink (or broken symlink) resolving outside the repository is now refused, anchored on the canonical repo root (reliable even where the lexical anchor is unset by a harmless system symlink); an in-repo config symlink still works. - R4-4 workspace root without package.json: the declaring workspace root now caps the walk even when it has no package.json of its own (pnpm/lerna root), so an unrelated ancestor config above it is not adopted; a config AT the root is still inherited. - R4-5 JSON recursion/oversized manifest: package.json/lerna.json parsing now catches RecursionError, and every declaration file is size-bounded before it is read/parsed. A parse-failing lerna.json no longer falls through to the `packages/*` default (fail closed). Prompt R8/R10 broadened and R14 added (all behavioral). Fingerprint meta re-synced. +11 regression tests (105 total). Suite green, E2E green, pylint E 10.00. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…pt contract (round-8 review) Fifth independent Codex gpt-5.6-sol xhigh review (fresh, at new head) found four findings; three fixed, one (low/optional) rejected with evidence. - R5-1 (high) manifest symlinked to a device: a pnpm-workspace.yaml/package.json symlinked to /dev/zero reports st_size 0 then streams forever. _read_manifest_text now requires the resolved target to be a regular file (S_ISREG) and reads at most _MAX_MANIFEST_BYTES+1 from a byte-capped handle; a symlink to a genuine regular file still works. - R5-2 (high) brace-expansion byte blowup: the count budget allowed a near-5MB prefix glob with a few brace groups to materialize ~5GB of strings. Added a per-raw-glob length cap (_MAX_GLOB_LENGTH) that bounds expansion in bytes, not just result count. Real globs are tiny; an over-long one fails membership closed. - R5-3 (medium) fix-loop prompt provenance: the {file}/{test} no-re-substitution rule lived only in get_test_command's prompt, so regenerating fix_error_loop.py could reopen the injection. Added the MUST NOT rule to fix_error_loop_python.prompt and a direct _run_non_python_initial_verification regression test. - R5-4 (low, optional) REJECTED: nested `{a{b,c}}` bash-brace parity. The impl emits it literally and fails membership CLOSED (never falsely includes); it is not a real workspace-glob pattern (would require a package literally named `{ab}`), and full bash-brace parity is out of proportion/scope. The reviewer independently re-verified fingerprint consistency (prompt/code/ example/test + include_deps all match). Meta re-synced. +5 regression tests (109 total). Suite green, E2E green, pylint E 10.00. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…e-parse (round-9 review) Sixth independent Codex gpt-5.6-sol xhigh review (fresh, at new head) found three medium findings; all fixed. - R6-1 dangling pnpm symlink: `Path.exists()` is False for a dangling symlink, so an authoritative pnpm-workspace.yaml was ignored and membership fell through to a stale package.json `workspaces`. pnpm presence is now detected lexically (exists() or is_symlink()); a present-but-unreadable/dangling pnpm config yields no globs (fail closed) and never falls through. - R6-2 symlink-loop crash: a self-referential/looping symlink path makes Path.resolve() raise RuntimeError on 3.12, which propagated out of get_test_command_for_file and crashed sync/fix orchestration. Resolution is now guarded (OSError, RuntimeError) in _detect_ts_test_runner and the containment helpers → refuse discovery (None) instead of crashing. - R6-3 O(n^2) manifest re-parsing: _workspace_root_for was called at each walk step and re-read every ancestor manifest, so deeply nested packages with padded manifests could re-parse gigabytes. Added a per-discovery cache keyed by canonical ancestor path so each manifest is parsed at most once. Meta re-synced. +5 regression tests (114 total; incl. a read-at-most-once assertion). Suite green, E2E green, pylint E 10.00. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

… package.json boundary (round-10 review) Seventh independent Codex gpt-5.6-sol xhigh review (fresh, at new head) found two findings; both fixed, plus a same-class injection in the delegated smart-detection path. - R7-1 (high) CSV-fallback command injection: `get_test_command_for_file` step 2 substituted the raw test path into the CSV command without shell-quoting, so a path like `/repo/$(touch PWN)/a.py` injected under the callers' `shell=True`. The substitution now uses `shlex.quote`. - Same-class injection in step 3 (smart detection): `default_verify_cmd_for` (pdd/agentic_langtest.py), whose command `get_test_command_for_file` returns as-is, substituted the path unquoted — and its Python fallback used bare double quotes, which do NOT stop `$()` command substitution. Both now use `shlex.quote`. - R7-2 (medium) dangling package.json boundary: the JS-project boundary used `Path.exists()`, which is False for a dangling/looping `package.json` symlink, so the walk slipped past an independent package and adopted an unrelated ancestor config. Boundary detection now uses `os.path.lexists` (present-but-dangling still stops the walk); a proven workspace member still inherits correctly. Prompt R13 updated to require shell-quoting the CSV path. Fingerprint meta re-synced (incl. the agentic_langtest include-dep hash). +6 regression tests (get_test_command 117 + agentic_langtest 15 = shell-injection and dangling/looping symlink coverage). Suites green, E2E green, pylint E 10.00. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…dget, grounding provenance (round-11 review) Eighth independent Codex gpt-5.6-sol xhigh review (fresh, at new head) found three findings; all fixed. - R8-1 (high) agentic_fix.py command injection: `_verify_and_log` and the preflight path re-substituted `{test}`/`{cwd}` into a command that may already be a finalized `default_verify_cmd_for` output, so a resolved path containing a literal `{test}` + shell metacharacters broke its quoting and injected under `bash -lc`. Now the PDD_AGENTIC_VERIFY_CMD *template* is substituted with `shlex.quote`d values, while a *finalized* command (env unset) runs as-is with no re-substitution. No caller passes a template via the param, so the env var is the authoritative template source. - R8-2 (medium) agentic_langtest provenance: the module's own prompt still claimed "all non-Python languages return None" and an unsafe double-quoted Python path, and the grounding example (`context/agentic_langtest_example.py`, an include-dep of the get_test_command prompt) demonstrated the same unsafe double-quoting. Back-propagated the real CSV-then-pytest resolution and the POSIX shell-quoting contract into the prompt, and rewrote the grounding example to `shlex.quote` every shell-substituted path (JS require target JSON-encoded then shell-quoted). - R8-3 (medium) aggregate matching DoS: the per-check DP-cell budget did not bound work across the whole discovery walk, so a heavy manifest re-evaluated at each of many nested package boundaries could stall for tens of seconds. The DP-cell budget is now shared across the entire `_detect_ts_test_runner` call; a legit deep chain still resolves. Fingerprint meta re-synced (incl. the changed grounding-example include-dep hash). +4 regression tests (get_test_command 119 + agentic_langtest 15 + agentic_fix TestVerifyAndLog 9). Suites green, E2E green, pylint E clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…explicit fix-loop provenance, faithful grounding (round-12 review) Ninth independent Codex gpt-5.6-sol xhigh review (fresh, at new head) found five findings; all fixed. - R9-1 (high) get_run_command injection: `get_run_command_for_file` substituted the path into the run template unquoted, and `agentic_fix` preflight/`_verify_and_log` execute it via `bash -lc` — so a Java/other test at `/repo/$(touch PWN)/x` injected. The `{file}` substitution now uses `shlex.quote`. - R9-2 (medium) whitespace-normalized glob: `_package_matches_workspace` stripped surrounding whitespace, turning `" packages/* "` (literal-whitespace, a non-match in workspace tools) into a broader `packages/*` and falsely proving membership. Whitespace is now preserved; only an exactly-empty entry is skipped. - R9-3 (medium) fix-loop provenance heuristic: agentic_fix `_verify_and_log` inferred template-vs-finalized from ambient `PDD_AGENTIC_VERIFY_CMD` state, which mishandles an explicit `verify_cmd=` template arg. Provenance is now tracked explicitly (`verify_cmd_is_template`): templates get shell-quoted `{test}`/`{cwd}` substitution, finalized commands run as-is. - R9-4 (medium) grounding example contradicted the contract: the get_test_command prompt's grounding (`context/agentic_langtest_example.py`) hardcoded JS npm / Java Maven-Gradle logic instead of the real CSV-first→Python-fallback→None resolution. Rewrote `default_verify_cmd_for` to mirror the real contract with `shlex.quote`. - R9-5 (low) interpreter path unquoted: the Python fallback now shell-quotes `sys.executable` too (a Python installed under a path with spaces no longer re-splits), in both the code and the grounding example. Fingerprint meta re-synced (incl. the changed grounding-example + agentic_langtest include-dep hashes). +5 regression tests across get_test_command / get_run_command / agentic_fix. Suites green, E2E green, pylint E clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…s (round-13 review) Tenth independent Codex gpt-5.6-sol xhigh review (fresh, at new head) found one finding; fixed. - R10-1 (medium) pnpm YAML construction crash: a `pnpm-workspace.yaml` value such as `packages: [2020-99-99]` makes PyYAML's timestamp constructor raise a bare `ValueError` ("month must be in 1..12") — which is NOT a `yaml.YAMLError`, so it escaped the handler and crashed runner discovery. The parse now also catches `ValueError`/`TypeError`/`OverflowError` (any construction failure on untrusted YAML) and fails membership closed. Fingerprint meta re-synced. +2 regression tests (malformed-timestamp scalar, end-to-end). Suite 123 passed, E2E green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…und-14 review) Eleventh independent Codex gpt-5.6-sol xhigh review (fresh, at new head) found one finding; fixed. - R11-1 (high) manifest parse OOM: a valid under-5MB workspace manifest containing millions of short entries materialized hundreds of MB of Python objects during json.loads/yaml.safe_load — before the `_MAX_RAW_GLOBS` count guard could run — and could OOM a worker. Real manifests are tiny, so the byte caps are now small: 1 MiB for package.json/lerna.json and 256 KiB for pnpm-workspace.yaml (YAML amplifies more per byte and a real pnpm workspace file is a few KB). Peak parse memory is now bounded to ~100 MB with generous headroom for legit manifests. `_string_globs` also rejects an over-`_MAX_RAW_GLOBS`-cardinality list up front, before validating/copying it, so no second full-list traversal occurs. Fingerprint meta re-synced. +1 regression test (under-byte-cap over-cardinality + over-byte-cap). Suite 124 passed, E2E green, pylint E clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…t (round-15 review) Twelfth independent Codex gpt-5.6-sol xhigh review (fresh, at new head) found one finding; fixed. - R12-1 (medium) lerna explicit-null default: `lerna.json` `{"packages": null}` was treated the same as an omitted key and granted the documented `packages/*` default, falsely proving membership and adopting the root Jest config — which contradicts the prompt contract that only an *omitted* key gets the default. The code now distinguishes absence (`"packages" not in lerna` → default) from an explicit value (`null` → `_string_globs(None)` → no globs, fail closed). Fingerprint meta re-synced. +1 regression test (omitted vs explicit-null, direct + end-to-end). Suite 125 passed, E2E green, pylint E clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…s handling (round-19) Round-19 review (Codex gpt-5.6-sol xhigh) found one false-positive and three over-broad fail-closed rejections: F1 (medium): `${foo,bar}` was brace-expanded, but minimatch's brace-expansion treats a `{` preceded by `$` as literal — so `packages/${foo,bar}` falsely proved `packages/$foo`. `_find_expandable_brace` now skips a `{` immediately preceded by `$` (continuing to scan for later real braces). F2 (medium): the `..` range guard flagged EVERY `..` inside any open brace, rejecting legitimate comma alternations like `{foo..bar,baz}` (literal `..` in one option) and unbalanced `{foo..bar`. `_has_brace_range` now tracks per-level state and flags only a balanced, comma-less brace group containing `..` (a true range); nested ranges inside an alternation are still caught. F3 (low): the astral-`?` gate rejected any `?` glob whenever the path held an astral char anywhere. Now `_astral_question_mark_risk` checks per-aligned-segment: with no `**`, alignment is positional so only a `?` opposite an astral segment fails closed; `**` stays conservative. `packages/*/a??` now matches `packages/😀/app` (the `*` consumes the emoji). F4 (low): empty classes `[]`/`[!]`/`[^]` were treated as complete classes and rejected; both fnmatch and minimatch treat them literally. `_has_complete_bracket_class` now requires a non-empty class (content after the optional `!`/`^`). All prior guards re-verified (escape, non-empty classes, comma-less ranges, extglobs, multi-bang, #-comment, internal-dot, unmatched-[, literal-.., aligned astral-?). Astral check extracted to a helper. Prompt R6 updated; meta re-synced; E2E green; 144 get_test_command + 52 other tests pass; lint 9.75. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…ength-first (round-20) Round-20 review (Codex gpt-5.6-sol xhigh) found three medium defects: F1 (DoS/ordering): the O(len) construct scan ran before the cheap length guard, and `_has_complete_bracket_class` rescanned the suffix for `]` per `[` (quadratic). A ~1 MiB unmatched-`[` glob under the manifest byte cap could stall discovery. The length guard now runs FIRST, and the bracket scan is a single left-to-right pass (a failed `]` search means no `]` exists later → stop). F2 (false-positive + over-rejection): construct checks ran on the RAW glob, but brace expansion can CREATE an unsupported construct from separate alternatives — `{?,x}(foo)` → `?(foo)` (an extglob fnmatch mishandles, so `a(foo)` was falsely proved a member) — or DISSOLVE an apparent one — `{[,x}]` → `[]`, `x]` (supported literals wrongly rejected). Bracket-class, extglob, and range checks now run on each fully-expanded CONCRETE pattern (`_concrete_pattern_unsupported`); only backslash (which expansion would mishandle) stays a raw-level check (`_raw_glob_unsupported`). F3 (false-positive + over-rejection): a balanced `${...}` was skipped by only one character, letting its nested `{bar,baz}` expand (`${foo,{bar,baz}}` falsely proved `${foo,bar}`); and `_has_brace_range` read a `..` inside `${1..3}` as a range (rejecting a legit literal dir). `${...}` is now opaque — the whole balanced group (nested braces included) is skipped in `_find_expandable_brace` (via `_skip_balanced_braces`) and treated as opaque in `_has_brace_range`. All prior guards re-verified (escape, closed/empty classes, comma/comma-less ranges, extglobs, multi-bang, #-comment, internal-dot, unmatched-[, literal-.., aligned astral-?, ${foo,bar}). Regressions added for all three. Prompt R6 updated; meta re-synced; E2E green; 147 get_test_command + 52 other tests pass; lint 9.74. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…eading ./ (round-21) Round-21 review (Codex gpt-5.6-sol xhigh) found two HIGH false-positives and one medium: F1 (high): `_skip_balanced_braces` returned end-of-string for an UNBALANCED `${`, so a later balanced `{a,b}` never expanded — `!packages/${foo/{a,b}` failed to exclude `packages/${foo/a`. It now returns start+1 for an unbalanced `${` (the `{` is literal; scanning resumes after it), matching the round-14 unbalanced-brace rule. F2 (high): the per-segment matcher delegated to Python `fnmatch`, which reinterprets literal bracket forms the guard intentionally permits — `fnmatch` matched `^` against the empty class `[^]`, falsely proving membership. Replace `fnmatch` with a direct `*`/`?`/literal two-pointer matcher (`_wildcard_segment_match`): every other character, brackets included, is literal, so a dir named `[^]` matches the glob `[^]` (parity with minimatch) and no OS case-folding creeps in. `import fnmatch` removed. Also fixed the class detector: a `]` in first-member position makes `[]]`/`[^]]`/`[!]]` real non-empty classes (they now fail closed), while truly empty `[]`/`[!]`/`[^]` stay literal. F3 (medium): the leading-dot strip removed ALL leading `.` segments; npm normalizes only ONE `./`. `././packages/*` no longer collapses to `packages/*` and falsely matches `packages/app` — the second `.` is significant. All prior guards re-verified. Regressions added for all three (incl. direct-matcher literal-bracket controls and `[]]`/`[^]]`/`[!]]`). Prompt R6 updated (matcher is now fnmatch-free); meta re-synced; E2E green; 150 get_test_command + 52 other tests pass; lint 9.75. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…tral/extglob (round-22) Round-22 review (Codex gpt-5.6-sol xhigh) found two medium false-positives and two low over-rejections: F1 (med): comment classification ran before leading-`/` normalization while matching strips `/`, so `/#*` (a minimatch comment after normalization) was matched literally and falsely proved a package named `#evil`. Comment classification now uses `_effective_leading` — the SAME normalization as matching (strip leading `/` and at most one `./`). F2 (med): `${...}` opacity was inferred from `$`+`{` adjacency in the partially-expanded worklist string, so expanding `{$,x}{a,b}` generated a spurious `${a,b}` that was frozen — `!packages/{$,x}{a,b}` failed to exclude `packages/$a`. Genuine balanced `${...}` spans are now masked out of the ORIGINAL pattern before expansion (`_mask_dollar_braces`, restored after via `_restore_dollar_braces`), and the fragile adjacency check is removed, so a generated `$`+`{` expands normally. F3 (low): the astral-`?` gate failed closed on any `**`+`?`+astral combo. `_astral_question_mark_risk` now fails closed only when some `?` segment can actually (code-point) match some astral segment, so `packages/ap?/**` matches `("packages","app","😀")` (`ap?` can't match the emoji). F4 (low): the extglob check was a substring test, rejecting incomplete markers like `foo?(bar` (minimatch reads these as `?` + literal `(`). `_has_complete_extglob` now flags only a marker with a matching `)`. All prior guards re-verified. Regressions added for all four. Prompt R6 updated; meta re-synced; E2E green; 150 get_test_command + 52 other tests pass; lint 9.78. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…mmar (round-23) Round-23 review (Codex gpt-5.6-sol xhigh) found two medium and one low over-rejection: F1 (med): the bracket-class and extglob checks scanned the whole pattern, but a class/extglob cannot cross `/`. `packages/foo[/bar]` (literal `[` in minimatch, class can't span the `/`) and `packages/foo?(/bar)` were wrongly rejected. Both checks now run per `/`-delimited segment; brace ranges still scan the whole pattern (a brace body may contain `/`). F2 (med): the astral-`?` fail-closed took a Cartesian product of `?` segments and astral path segments without checking reachable DP alignments, so a non-matching positive like `packages/?/x` failed the WHOLE check closed even when another positive matched — making include semantics order-dependent. Replaced the entire approximation by matching `?` over UTF-16 code units in `_wildcard_segment_match` (`_utf16_units`): `?` matches exactly one unit, an astral char needs `??` — exact minimatch parity. `_astral_question_mark_risk`/`_segment_has_astral` deleted. F3 (low): every comma-less brace containing `..` was treated as a range, so a literal dir `{foo..bar}` (multi-character endpoints) was rejected. `_has_brace_range` now validates minimatch range grammar via `_is_range_body` (integer or single-character endpoints, optional integer step) and inspects only leaf groups, keeping the scan linear. `{foo..bar}`/`{1.0..3.0}`/`{..}` are literal. All prior guards re-verified; astral tests updated to the exact (non-fail-closed) UTF-16 behavior. Regressions added for all three. Prompt R6 updated; meta re-synced; E2E green; 152 get_test_command + 52 other tests pass; lint 9.78. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…ge grammar (round-24) Round-24 review (Codex gpt-5.6-sol xhigh) found one medium false-positive and one low over-rejection: F1 (med): pnpm parses YAML 1.2 but PyYAML defaults to YAML 1.1. An unquoted `packages: [0o12]` is octal 10 (a non-string entry pnpm rejects) in 1.2 but the string "0o12" in PyYAML — falsely proving membership for a dir literally named `0o12`. Same for exponent/leading-dot floats (`1e3`, `+.5`). And PyYAML silently keeps the last of duplicate mapping keys where pnpm errors. Parse pnpm YAML with a dedicated loader (`_pnpm_yaml_loader`) that adds YAML-1.2 core int/float resolvers (so unquoted number-forms resolve to non-strings → rejected by `_string_globs`, while a quoted "0o12" stays a string glob) and rejects duplicate keys (fail closed). npm/yarn/lerna use JSON, which is unaffected. F2 (low): `_is_range_body` accepted `+`-prefixed and Unicode-digit endpoints, so a literal dir `{+1..+3}` was misclassified as a range and rejected. Range grammar is now ASCII-only: integer endpoints with an optional leading `-` (not `+`), or single ASCII letters, with an optional ASCII-integer step. `{+1..+3}`/`{١..٣}` are literal. Regressions added (unquoted-number + quoted-string + duplicate-key pnpm YAML; plus/Unicode range endpoints). Prompt R6/R8 updated; meta re-synced; E2E green; 154 get_test_command + 52 other tests pass; lint 9.79. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

… (round-25) Round-25 review (Codex gpt-5.6-sol xhigh) found the round-24 pnpm YAML fix was incomplete: it layered octal/float resolvers onto PyYAML's inherited YAML 1.1 table, but the 1.1-vs-1.2 discrepancy runs both directions. Forms YAML 1.1 resolves as non-strings but YAML 1.2 keeps as STRINGS — `yes`/`no`/`on`/`off` (1.1 bool), `0b10` (binary), `1:20` (sexagesimal), `2020-01-01` (timestamp), `1_000` (underscore int) — were still coerced to non-strings, so a valid pnpm declaration like `packages: [packages/*, yes]` was rejected wholesale, denying even `packages/app` its workspace membership and root runner. Replace the inherited YAML 1.1 implicit-resolver table wholesale with the YAML 1.2 core schema (bool = true/false only; null = null/~/empty; int = decimal/0o/0x; float = exponent/.inf/.nan). Now the 1.1-only forms stay strings (valid globs), while true 1.2 numbers/bool/null (`0o12`, `1e3`, `123`, `true`, `null`) are still rejected, and quoting is still respected. A date-like `2020-99-99` is now a plain string glob (YAML 1.2 has no timestamp type), not a construction that crashes — the round-10 fail-closed test is updated accordingly (no crash; literal glob that simply doesn't match `packages/app`). Regressions: `yes`/`on`/`0b10`/`1:20`/`2020-01-01`/`1_000` kept as globs; numbers rejected; quoted string kept; dup keys fail closed. Prompt R8 updated; meta re-synced; E2E green; 156 get_test_command + 52 other tests pass; lint 9.79. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…alar/int-construction (round-26) Round-26 review (Codex gpt-5.6-sol xhigh) found two high and one medium: F1 (high): an empty YAML list item (`- ` with nothing after) resolves to null in YAML 1.2 (a non-string that must fail closed), but the loader's null resolver omitted the empty scalar, so `packages: [packages/*, ]` yielded `["packages/*",""]` and `_string_globs` accepted it → `packages/app` falsely a member. The null resolver now matches the empty scalar (regex allows "", first-char list includes ""). F2 (high): the loader replaced YAML resolution but kept PyYAML's YAML 1.1 integer CONSTRUCTOR, so `012` constructed as octal 10 rather than decimal 12 — making the YAML-1.2-duplicate keys `012:`/`12:` (both integer 12) look distinct and parse successfully instead of failing closed. Added a YAML 1.2 int constructor (`012`→12; `0o`/`0x`→bases 8/16) and a (tag, value) duplicate-key comparison. F3 (medium): membership split all positives/negatives and treated every exclusion as permanent, so npm's re-inclusion `["packages/**", "!packages/legacy/**", "packages/legacy/app"]` wrongly denied `packages/legacy/app`. Membership is now evaluated in declaration order, last matching pattern wins (a later positive re-includes) — matching both `@npmcli/map-workspaces` and `@pnpm/matcher`. A trailing exclusion still excludes; the pnpm `!**/test/**` case is unchanged. Regressions added for all three. Prompt R6 (order semantics) + R8 (empty scalar, int construction) updated; meta re-synced; E2E green; 163 get_test_command + 52 other tests pass; lint 9.80. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…, matcher DoS (round-27) Round-27 review (Codex gpt-5.6-sol xhigh) found one high and three medium: F1 (high, agentic_fix.py): a verify-command TEMPLATE that nests a placeholder in its own quotes (`pytest "{test}"`) defeated shlex.quote — the inserted single quotes become literal inside the double quotes, so a `$(...)` in the repo path was executed (preflight and final gate). Substitution now goes through `_substitute_verify_template`, which requires each `{test}`/`{cwd}` to be a standalone bare word (bounded by whitespace/ends) and REFUSES the template otherwise — the gate fails closed instead of building an injectable command. F2 (medium): `json.loads` accepts `NaN`/`Infinity`/`-Infinity`, which npm's strict JSON parser rejects, so a manifest containing them (even outside `workspaces`) falsely proved membership. Both package.json and lerna.json now pass a `parse_constant` callback that fails the whole-document parse closed. F3 (medium): leading normalization stripped `/` and then `./` independently (two passes), collapsing `/./packages/*` to `packages/*` and falsely matching `packages/app`. It is now a single pass (`_strip_one_leading`: one `./` OR a leading `/`-run), used identically for matching and `#`-comment classification; a residual `/`/`./` is significant and does not match. F4 (medium): the UTF-16 `?` matcher rebuilt unit arrays per DP cell and did unbudgeted per-character work (~11s within the cell cap). Units and dot-flags are now precomputed once per segment (`_segments_dp_match`), character comparisons are charged against the shared work budget, and a `**`-free pattern whose segment count differs from the path is fast-rejected before the DP. Regressions added for all four. Prompts (get_test_command R6/R8/R9, agentic_fix) updated; get_test_command meta re-synced (agentic_fix has no meta); E2E green; 163 get_test_command + 54 other tests pass; lint 9.80. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…safe substitution (round-28) Round-28 adversarial review (gpt-5.6-sol xhigh @ 4cf4e56) — 7 findings, all fixed: - F1/F2/F3 (HIGH): route every {file}/{test}/{cwd} template substitution through shell_safe_substitute (single-pass, shell-lexical-context-aware). A sequential str.replace rescanned inserted values, and a whitespace-bounded bare-word check did not prove a placeholder was UNQUOTED (`echo " {test} "` inside double quotes defeats shlex.quote). Now the helper tracks single/double/backtick state and refuses any quoted or adjacent placeholder (fails closed). Applied in get_test_command CSV path, get_run_command_for_file, agentic_fix template, agentic_langtest, and the grounding example. - F4 (med): resolve a relative template {test} against the passed cwd (the dir run_agentic_fix operates in), not the process CWD. - F5 (med): leading-prefix normalization is one regex ^\.?/+ (optional dot + the entire following slash run, single pass), so `.//`/`.///packages/*` collapse to `packages/*` and match, while `././`/`/./` residuals stay significant. - F6 (HIGH): membership combination rule is source-dependent — npm/yarn/lerna (@npmcli/map-workspaces) treat an exclusion as terminal (any-exclude-wins); only pnpm (@pnpm/matcher) is order-dependent (last-match-wins re-inclusion). Thread an `ordered` flag from _workspace_source_is_pnpm. (Corrects R26-3, which had over-generalized pnpm's re-inclusion to npm.) - F7 (med): recognize every Jest config extension (.js/.mjs/.cjs/.json/.ts/.mts/ .cts) plus .cjs/.mts/.cts for playwright/vitest, so a project whose only config is jest.config.cjs is detected as Jest instead of falling through to tsx. Adds negative regressions for each finding; updates the get_test_command and agentic_fix prompts and re-syncs .pdd/meta/get_test_command_python.json (prompt + code + test + agentic_langtest/example include-dep hashes changed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…l heredoc/adjacency, jest/vitest detection Round-29 adversarial review (gpt-5.6-sol xhigh @ d5dd362) — 6 blocking + 1 low, all fixed: - F1 (high): shell_safe_substitute accepted here-document and comment templates (`cat <<EOF\n{file}\nEOF`, `echo hi # {file}`) → injection under bash -lc despite shlex.quote. Refuse any multiline template (the only way to form a heredoc body), track unquoted `#` comment context, and refuse a placeholder reached there. Adds real `bash -lc` execution regressions. - F2 (med): restore npm's ACTUAL @npmcli/map-workspaces semantics. Round-28's terminal-exclusion was wrong: npm's appendNegatedPatterns removes an earlier negation wholesale when a later positive's pattern-string matches it, re-including the subtree (`["packages/**","!packages/legacy/**","packages/legacy/app"]` makes packages/legacy/* members). pnpm stays per-path last-match-wins (sibling excluded). Verified against upstream lib/index.js. - F3 (med): add npm's built-in `**/node_modules/**` ignore (universal) so a dep under node_modules can never inherit a workspace runner. - F4 (med): my round-28 routing of get_run_command_for_file through shell_safe_substitute made Fortran/Pascal CSV templates (`gfortran -o {file}.out {file}`, `fpc {file} && ./{file}`) return '' — over-strict adjacency rejection. shlex.quote is a self-contained word, safe to concatenate with ordinary literal chars, so allow literal adjacency (still refusing quote/$/backtick- adjacent, quoted, and heredoc/comment placeholders). Real-CSV regressions added. - F5 (med): detect a top-level `"jest"` object in a bounded, strictly-parsed package.json as a Jest project when no dedicated jest.config.* exists. - F6 (med): adopt vite.config.{ts,js,mjs,cjs,mts,cts} as Vitest config ONLY when the manifest proves Vitest (vitest dependency or script) — never for a Vite-only app. - F7 (low): _substitute_verify_template resolves {cwd} to absolute too (not just {test}), so `cd {cwd} && pytest {test}` cannot double-join a relative cwd. Updates get_test_command + agentic_fix prompts (R6 npm re-inclusion + node_modules, R13 adjacency/heredoc, runner-config package.json forms; verify-template doctrine) and re-syncs .pdd/meta/get_test_command_python.json. 231 focused tests pass, E2E green, pylint 10.00. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…n/main Merging latest origin/main changed two include-deps of the get_test_command prompt fingerprint — pdd/data/language_format.csv and pdd/get_language.py — so the composite prompt_hash shifted. Recompute prompt_hash + both include-dep hashes with the merged calculate_prompt_hash; code/test/example unchanged. Fingerprint verified mutually consistent; 231 focused tests, E2E, and pylint (E) all green post-merge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…xts, vitest proof, npm comment parity, prompt contracts Round-30b adversarial review (gpt-5.6-sol xhigh @ 95e7fc0) — 4 blocking findings, all fixed: - F1 (high): shell_safe_substitute still accepted command-evaluation contexts — a placeholder inside arithmetic `$(( ))` / `$( )` / `${ }` / backticks / a `(...)` subshell, and a `#` comment beginning right after a `;`/`&`/`|` control operator (whitespace-only comment detection missed it). Switched to an allowlist of simple command lines: refuse any template containing a newline, `$`, a backtick, or `(`/`)`, and track comment starts after control operators. Real bash -lc regressions for the arithmetic and operator-comment exploits; adjacency + real templates still substitute. - F2 (med): _vitest_proven_by_manifest used substring `"vitest" in val`, so a script `echo no-vitest-installed` false-proved Vitest. Now requires vitest as an invoked command token (basename of a shell token == vitest), covering npx/pnpm exec/yarn/ ./node_modules/.bin wrappers; negative tests for substring-only scripts. - F3 (med): a positive `#` comment pattern was skipped before npm's appendNegatedPatterns preprocessing, so it could not remove an earlier matching negation the way npm does (`["packages/**","!**","#noop"]` must re-include packages/**). Comments now participate in npm negation-removal while never matching a concrete path; pnpm still ignores them. - F4 (med): get_run_command_python.prompt now declares shell_safe_substitute as a required public interface (a regeneration could otherwise drop it, breaking imports in get_test_command/agentic_fix/agentic_langtest); agentic_langtest_python.prompt no longer claims "no internal PDD dependencies". Re-syncs .pdd/meta/get_test_command_python.json (code+test hashes). 233 focused tests pass, E2E green, pylint 10.00. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…ocessing, vitest executable position, substitute guards Round-31b adversarial review (gpt-5.6-sol xhigh @ 12cc809) — 3 findings, all fixed: - F1 (med): npm appendNegatedPatterns preprocessing brace-expanded the positive BEFORE comparing it to earlier negations, so `["packages/**","!packages/a","packages/{a,b}"]` wrongly removed `!packages/a` (via the `packages/a` expansion) and made packages/a a member. npm compares the RAW positive pattern string (braces literal) against each negation glob and expands braces only for the final path test — now `packages/a` stays excluded, `packages/b` is a member. Negations are grouped and removed atomically (a brace negation still expands and is removed as a unit). pnpm is unaffected (per-path last-match re-includes packages/a, correctly). - F2 (med): _vitest_proven_by_manifest counted `vitest` as any token, so `echo vitest` / `node vitest` false-proved. Now the script is split into command clauses and vitest counts only in executable position — as the command basename or the binary invoked by a supported runner (npx/pnpm/yarn/bun, incl. exec/dlx/run). Neighboring negatives/ positives tested. - F3 (low): shell_safe_substitute infinite-looped on an empty placeholder key (`{"": ...}` matched everywhere, cursor never advanced) and returned an unresolved template for an escaped `\{test}`. Now rejects an empty key up front and declines an escaped placeholder (None). Re-syncs .pdd/meta/get_test_command_python.json (code+test hashes). 235 focused tests pass, E2E green, pylint 10.00. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…jection, vitest runner grammar Round-32 adversarial review (gpt-5.6-sol xhigh @ c182284) — 2 findings, both fixed: - F1 (med): shell_safe_substitute did not reject brace-expansion (`{a,b}`) or pathname-expansion (`*`/`?`/`[]`/`~`) metacharacters appearing OUTSIDE a placeholder, so a template like `pre{{file},tail}` brace-expanded the command into extra words (and `shlex.quote` leaves a value's `,` unquoted, re-splitting inside a template brace). Now mask placeholders and refuse any residual `{}[]*?~`. Ordinary adjacency (`{file}.out`, `./{file}`) is unaffected; real bash -lc regression. - F2 (med): the vitest executable-position check mis-parsed two cases — `npx --package vitest echo ok` (vitest is the option's argument, real command is echo) and `echo x\;vitest` (escaped `;` mis-split by the regex clause splitter). Now tokenize with a quote/escape-aware shell lexer, split clauses on unquoted operators, and after a runner skip only known subcommands/boolean flags — an arg-taking or unknown flag fails closed. Adversarial negatives added. Re-syncs .pdd/meta/get_test_command_python.json (code+test hashes). 236 focused tests pass, E2E green, pylint 10.00. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…oval, leading-norm order, runner-script grammar, prompt doctrine Round-33 adversarial review (gpt-5.6-sol xhigh @ 7979a42) — 4 findings, all fixed: - R33-1 (med): implement npm appendNegatedPatterns' SECOND step — each surviving negation prunes any positive PATTERN whose raw pattern string it matches (`minimatch.match(patterns, negated)`). `["packages/*","!packages/?"]` now drops `packages/*` and `packages/app` is not a member (the concrete path doesn't match the single-char negation, but the pattern string does). - R33-2 (med): apply npm leading normalization (`^\.?/+`) to each raw pattern exactly once, BEFORE brace expansion, and never again to generated expansions (new `pre_normalized` flag on the matcher). `["{/packages/*,other/*}"]` now keeps the brace-generated `/packages/*` anchored, so `packages/app` is excluded while `other/app` is a member. - R33-3 (med): runner grammar — `npm run vitest` / bare `pnpm vitest` invoke a package.json SCRIPT (possibly a Vite-only shadow), not the vitest binary. Split runners into direct (npx/bunx) vs exec-subcommand (npm/pnpm/yarn/bun + exec/dlx/x); a bare command or `run <name>` fails closed. Script-shadowing regression added. - R33-4 (med): the get_run_command / get_test_command(R13) / agentic_fix prompts now declare rejection of non-placeholder brace/glob metacharacters (`{}*?[]~`) plus the empty-key and escaped-placeholder rules, at doctrine altitude. Re-syncs .pdd/meta/get_test_command_python.json (prompt+code+test hashes). 238 focused tests pass, E2E green, pylint 10.00. Upstream npm/pnpm parity verified against @npmcli/map-workspaces. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…est DoS/`--`/re-eval guards, prompt doctrine Round-34 adversarial review (gpt-5.6-sol xhigh @ cfe1dd3) — 4 med + 1 low, all fixed: - R34-1 (med): reproduce npm appendNegatedPatterns' EXACT mutation order — its `splice(i)` then `++i` skips the negation adjacent to a removed one. Replaced the simultaneous list-comprehension removal with an index loop (`del` then `i += 1`), so `["packages/**","!packages/**","!packages/*","packages/app"]` leaves `!packages/*` in force and excludes `packages/app` (matching upstream). - R34-2 (med): a ~1MB no-whitespace package.json script made the shell lexer quadratic (~10.8s). Added a per-script length cap (_MAX_SCRIPT_LEN) — an oversized script fails vitest proof closed quickly; the bounded ancestor walk keeps the aggregate bounded. - R34-3 (med): recognize `--` as the options terminator after an exec/dlx/x subcommand, so `npm exec -- vitest` is proof while option-value shadows (`npx --package vitest`) stay negative. - R34-4 (med): shell_safe_substitute now rejects templates that RE-EVALUATE the value as code — `eval {file}`, `bash -c {file}`, `sh -c {file}` — where the second parse undoes shlex.quote (confirmed injecting via bash). A bare `sh {file}`/`bash {file}` (run the file as a script — shipped Shell/Bash/Zsh templates) and a non-shell `-c` option (`pytest -c cfg`) still substitute. Real bash regressions. - R34-5 (low): raised prompt doctrine altitude — R9's matcher/DP performance mechanics and the agentic_fix substitution paragraph now state the OBSERVABLE bounded/fail-closed outcomes and defer HOW (caching, tokenizer, masking) to tests. Re-syncs .pdd/meta/get_test_command_python.json. 240 focused tests pass, E2E green, pylint 10.00. npm/pnpm parity re-verified against @npmcli/map-workspaces. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…recedence, re-eval bypass hardening Round-35 adversarial review (gpt-5.6-sol xhigh @ 0a5b112) — 2 med findings, both fixed: - R35-1 (med): the segment matcher tested literal equality before wildcard handling, so a pattern `*` was consumed as a literal against a name `*` (`packages/**` used as a path during npm's pattern-vs-pattern pruning). `**` therefore failed to match the glob `*`, so `["packages/**","!packages/*"]` did not prune `packages/**` and packages under it stayed members. Handle a pattern `*` as a wildcard FIRST — now that declaration excludes both `packages/app` and a deep `packages/deep/app` (with an end-to-end ancestor-config non-inheritance test), and a literal `*` in a real segment still matches. - R35-2 (med): the re-evaluation check missed three bypasses — a combined short-option group (`bash -lc`, `sh -xc`), a command wrapper (`env bash -c`, `command sh -c`), and a mid-word `#` (`echo a#b && bash -c {file}`) that shlex wrongly treated as a comment, dropping the malicious clause. Now disable shlex comment parsing, detect `c` inside a combined single-dash flag, skip command wrappers to find the effective command, and fail closed. Real bash regressions confirm bash -lc / env bash -c inject in the naive form and are refused; bare `sh {file}`/`bash {file}` and non-shell `-c` stay safe. Re-syncs .pdd/meta/get_test_command_python.json (code+test hashes). 242 focused tests pass, E2E green, pylint 10.00. npm/pnpm parity re-verified against @npmcli/map-workspaces. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…ot:true ignores, #-comment split, empty segments, vitest bounds Round-36 adversarial review (gpt-5.6-sol xhigh @ e38ee45) — 3 med + 3 low, all fixed: - R36-1 (med): re-eval detection missed option-bearing wrappers (`timeout 5 bash -c`, `env -i bash -c`, `nice -n 5 bash -c`) whose operands hid the shell. Replaced the wrapper-skip/command-position logic with a conservative holistic per-clause scan: refuse a clause containing both a shell name and a `-c`-bearing option, or an `eval`. Real bash regressions. - R36-2 (med): npm applies surviving negations as glob's dot:true IGNORE set, so `!packages/*` excludes `packages/.shadow`; the matcher used dot:false everywhere. Added a `dot` flag threaded through the matcher — final negations match dot:true, positives + pattern-vs-pattern stay dot:false. - R36-3 (med): npm's appendNegatedPatterns preprocessing uses default minimatch, where a leading-`#` NEGATION is a comment matching nothing (never removed/pruning), but the final glob is nocomment (literal). `["**","!#foo","#foo"]` now keeps `#foo` excluded; `#` positives match literally in final. Split the semantics; updated prompt + tests. - R36-4 (low): collapse empty/trailing-slash segments in the raw pattern-string used for pattern-vs-pattern pruning (minimatch collapses `//`), so `packages//app` prunes `!packages/*`. - R36-5 (low): vitest script lexer is now word-boundary aware for `#` — a mid-word `#` (`echo a#b && npx vitest`) is literal (proves), a leading-`#` comment does not. - R36-6 (low): a Vitest dependency value must be a STRING spec; `"vitest": false/null/` number no longer proves Vitest. Re-syncs .pdd/meta/get_test_command_python.json (prompt+code+test). 244 focused tests pass, E2E green, pylint 10.00. npm/minimatch parity re-verified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…al, terminal-star budget, dot/dotdot, #-comment provenance, pnpm-#, empty-positive Round-37 adversarial review (gpt-5.6-sol xhigh @ 2447ac2) — 7 findings; 6 fixed, 1 rejected as a false positive: - R37-1 (high): shell_safe_substitute now refuses a value PIPED (`printf %s {file} | bash`) or here-string'd (`bash <<< {file}`) into a re-evaluating shell — the `-c` check missed both. A re-eval shell name + a `|`/`<` in the template → refused. Real bash regressions; a pipe into a non-shell (grep) stays safe. - R37-2 (med): charge the trailing-`*` run in the segment matcher against the shared work budget — a long `*`-wall plus brace expansion and a `**` suffix could otherwise reach ~10^8 unbudgeted iterations under the cell cap. Aggregate adversarial test. - R37-3 (med): a `.`/`..` path segment (arising when a raw pattern string like `packages/./x` is matched as a path during npm pruning) is matched ONLY by an identical literal, never a wildcard/`**` (minimatch parity). - R37-4: REJECTED — the reviewer claimed Jest's JEST_CONFIG_EXT_ORDER excludes `.mts`, but Jest's current source lists `['.js','.ts','.mjs','.mts','.cjs','.cts','.json']` (includes `.mts`/`.cts`). Removing them would regress real jest.config.mts detection, so they are kept. - R37-5 (low): the Vitest script comment stripper is now quote/escape/newline aware — a quoted (`"# x"`), escaped (`\#`), or mid-word `#` is literal; an unquoted comment ends only its own line. - R37-6 (low): pnpm treats `#` LITERALLY (only `!` is special) — `['#app']` matches a `#app` dir and `['!#app','#app']` re-includes it; the npm-preprocessing comment rule no longer applies to pnpm. - R37-7 (low): an EMPTY positive pattern is preserved through npm appendNegatedPatterns and can remove a prior negation (`["packages/**","!**",""]` re-includes packages/**). Updates get_test_command R6 + get_run_command prompt; re-syncs the fingerprint. 248 focused tests pass, E2E green, pylint 10.00. npm/pnpm/minimatch parity verified against upstream (map-workspaces, @pnpm/matcher, jest constants). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

… hidden shell, heredoc-body vitest R38 exited on a DNS/transport error (invalid, not counted); harvested exact-head probes established two legitimate defects, now fixed: - HIGH: shell_safe_substitute accepted `env -S 'bash -c' {file}` — `env -S` (--split-string) re-parses its string argument into a command line, hiding a shell + `-c` beyond the top-level token scan, so bash second-parses the substituted path and a `$(...)` executes. Now refuse `env -S`/`--split-string`, and re-tokenize each quoted token to catch a shell+`-c` (or eval) hidden inside a wrapper's re-parsed argument. `env` without `-S` stays safe. Real bash injection regressions. - MEDIUM: _script_invokes_vitest treated here-document / here-string BODY text as an executed command, so `cat <<EOF\nnpx vitest\nEOF` false-proved Vitest for a Vite-only manifest. Refuse any script containing `<<` (heredoc/here-string) — its body is data, not commands. Updates get_run_command + get_test_command prompts; re-syncs the fingerprint. 248 focused tests pass, E2E green, pylint 10.00. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Serhan-Asad requested a review from gltanaka July 12, 2026 02:42

Serhan-Asad self-assigned this Jul 12, 2026

gltanaka requested changes Jul 12, 2026

View reviewed changes

Serhan-Asad mentioned this pull request Jul 12, 2026

Campaign bundle: PDD end-to-end staging fixes #1998

Open

Serhan-Asad mentioned this pull request Jul 13, 2026

fix: bound TypeScript runner discovery (clean replacement for #2003) #2017

Open

Serhan-Asad and others added 20 commits July 13, 2026 18:16

chore: auto-heal prompt/example drift for get_test_command

cb1e2d9

PDD-Auto-Heal-Checkpoint: success

Serhan-Asad and others added 26 commits July 13, 2026 18:16

fix(prompts): align runner shell contract governance

4ed2291

docs(pdd): make runner contract coverage explicit

022ef1c

chore(pdd): register nested runner story ownership

7bcbb46

chore(pdd): converge runner story fingerprints

6b07adc

chore(pdd): align runner dependency governance

5e2ed4f

Serhan-Asad force-pushed the fix/jest-runner-discovery-dynamic-routes branch from 163326d to 5e2ed4f Compare July 14, 2026 01:52

Serhan-Asad added 3 commits July 14, 2026 12:56

fix: complete nested runner verification contract

5278c33

fix: reject ambiguous env vitest wrappers

b57f221

chore(pdd): align nested runner governance

fc70417

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

Fix Jest runner discovery for deeply nested / dynamic-route TS tests#2003
Serhan-Asad wants to merge 56 commits into
mainfrom
fix/jest-runner-discovery-dynamic-routes

Serhan-Asad commented Jul 11, 2026

Uh oh!

Serhan-Asad commented Jul 12, 2026

Uh oh!

Serhan-Asad commented Jul 12, 2026

Uh oh!

gltanaka left a comment

Uh oh!

Serhan-Asad commented Jul 12, 2026

Uh oh!

Serhan-Asad commented Jul 12, 2026

Uh oh!

Serhan-Asad commented Jul 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Serhan-Asad commented Jul 11, 2026

Problem

Fix

Verification

Downstream

Uh oh!

Serhan-Asad commented Jul 12, 2026

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

Uh oh!

Serhan-Asad commented Jul 12, 2026

Review round 3 — resolved

Uh oh!

gltanaka left a comment

Choose a reason for hiding this comment

Review verdict: changes requested

Uh oh!

Serhan-Asad commented Jul 12, 2026

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

Root cause

Fix

Red → green (the two reproducing tests)

Coverage added

Verification (head 6a8b1fa65)

Uh oh!

Serhan-Asad commented Jul 12, 2026

Uh oh!

Serhan-Asad commented Jul 13, 2026

CI / review triage at current head 7d9016bfe

1. @gltanaka's CHANGES_REQUESTED blocker is resolved at the current head

2. Run Unit Tests → red on tests/test_sync_core_pdd_rollout_policy.py, not this diff

3. heal / auto-heal → private Cloud Build, not a code defect

Required checks / merge gate

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Verification (head `6a8b1fa65`)

CI / review triage at current head `7d9016bfe`

1. `@gltanaka`'s `CHANGES_REQUESTED` blocker is resolved at the current head

2. `Run Unit Tests` → red on `tests/test_sync_core_pdd_rollout_policy.py`, not this diff

3. `heal` / `auto-heal` → private Cloud Build, not a code defect