Skip to content

fix(sync): add trusted Playwright verification adapter#1995

Open
gltanaka wants to merge 41 commits into
codex/issue-1932-vitest-adapterfrom
codex/issue-1932-playwright-adapter
Open

fix(sync): add trusted Playwright verification adapter#1995
gltanaka wants to merge 41 commits into
codex/issue-1932-vitest-adapterfrom
codex/issue-1932-playwright-adapter

Conversation

@gltanaka

Copy link
Copy Markdown
Contributor

Summary

  • add fail-closed trusted validator_id=playwright adapter
  • bind static Playwright config, local import/support closure, JSON reporter identities, and candidate support mutations
  • compare protected-base, candidate collection, and candidate execution identities

Test plan

  • conda run -n pdd pytest -q tests/test_sync_core_runner_playwright.py tests/test_sync_core_runner_vitest.py tests/test_sync_core_runner_jest.py
  • conda run -n pdd pylint --disable=C0301,C0302,R0911,R0912,R0913,R0914,R0917,W0613 pdd/sync_core/runner.py tests/test_sync_core_runner_playwright.py
  • Local Playwright package/browser unavailable: contract fixture exercises --list and execution protocol, but a real browser execution remains unavailable.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@gltanaka

Copy link
Copy Markdown
Contributor Author

TDD red evidence

Pushed red commit 81535f90a before the implementation.

ImportError: cannot import name `playwright_validator_config_digest` from `pdd.sync_core.runner`

The dedicated Playwright adapter contract could not collect before the adapter existed.

@gltanaka

Copy link
Copy Markdown
Contributor Author

Investigation

  • Sibling adapters: Jest (7ee8ea79f) and Vitest (4fa03324c) use protected-base clones, exact local runner argv, credential-scrubbed environments, closure digests, and fail-closed identity comparison. This PR follows that convention for Playwright.
  • The required parent 4fa03324c is the direct base; no sibling implementation outside pdd/sync_core/runner.py was changed.
  • Scope decision: only the shared runner plus the dedicated Playwright contract test were changed. Pytest, Jest, and Vitest adapter paths remain covered by their existing tests.
  • Real E2E gap: this worktree has no node_modules/@playwright/test/cli.js and no Playwright browser cache. The fixture protocol covers --list collection and execution; a local real Playwright browser run could not be performed without installing dependencies/browsers.

@gltanaka

Copy link
Copy Markdown
Contributor Author

Playwright trusted adapter verification round

Head verified: a022321773d8f6ba23e7eb044316bd444bdf0bdb on codex/issue-1932-playwright-adapter (base codex/issue-1932-vitest-adapter). Worktree clean and pushed: git status --short --branch -> ## codex/issue-1932-playwright-adapter...origin/codex/issue-1932-playwright-adapter; local HEAD equals origin/codex/issue-1932-playwright-adapter.

Correctness gaps found and fixed

Real Playwright execution exposed three gaps. Each got a red test before the fix:

  • f80514ae7 / 9322924dd red: protected-base clone could not resolve the pinned local @playwright/test install from outside the clone. Fixed by 5b0948931, which sets NODE_PATH to the trusted command root node_modules for Playwright subprocesses.
  • ecc492ed7 red: real Playwright JSON can report spec.file relative to the runner root; the adapter resolved it relative to Python cwd. Fixed by 1c840eb69 + e1626df8c, resolving relative files from the Playwright root, including the /tmp symlink shape on macOS.
  • 76ea1d147 red: isolated HOME hid package-local Playwright browsers. Fixed by a02232177, which sets PLAYWRIGHT_BROWSERS_PATH=0 when the pinned command root has node_modules/playwright-core/.local-browsers.

Also pushed 58f96ce25 for a focused unused-import cleanup in the Playwright test file.

Real Playwright fixture

Disposable fixture: /tmp/pdd-playwright-real.hRLuEC

Pinned/runtime versions:

  • @playwright/test: 1.54.1
  • Playwright-reported browser: Chromium 139.0.7258.5, build chromium_headless_shell-1181
  • Node: v26.4.0
  • npm: 11.17.0
  • Protected fixture base SHA: 7c46a7bf859641054812f7d96ab16fa05bc8dfed

Fixture shape:

  • Committed playwright.config.ts with project chromium and local reporter ./reporter.ts.
  • Committed tests/widget.spec.ts imports @playwright/test, imports ../src/message, asserts process.cwd() sees playwright.config.ts and src/message.ts, then launches a real page and checks DOM text.
  • Committed .gitignore ignores node_modules/, test-results/, and playwright-report/.
  • node_modules stayed disposable/untracked; browser cache is package-local under node_modules/playwright-core/.local-browsers.

Install/setup commands used:

npm install --package-lock-only --ignore-scripts
PLAYWRIGHT_BROWSERS_PATH=0 npm ci
PLAYWRIGHT_BROWSERS_PATH=0 npx playwright install chromium
PLAYWRIGHT_BROWSERS_PATH=0 npx playwright install chromium-headless-shell

Note: on this host, Playwright's browser downloader hung after download. I verified the downloaded headless-shell zip and manually extracted it into node_modules/playwright-core/.local-browsers/chromium_headless_shell-1181; the subsequent direct Playwright run launched the real headless shell and passed.

Direct browser sanity:

npx playwright test tests/widget.spec.ts --config=playwright.config.ts --reporter=json
version: 1.54.1
project: chromium
status: passed

Source checkout E2E through run_profile(...)

Command shape: source checkout Python imported pdd.sync_core.run_profile, with RunnerConfig(timeout_seconds=45) and no custom Playwright command override. The adapter used the real local CLI at node_modules/@playwright/test/cli.js.

Results:

protected_base_equals_head_pass: PASS
  detail: 1 protected Playwright tests passed
  command_digest: dfd36681fa097e4e523c28686f39fae73a0d415a7536cd8c5fc767dd0fd36d58

candidate_imported_support_mutation: FAIL
  detail: Playwright reported failed protected tests

candidate_config_filter: ERROR
  detail: Playwright execution filters or retries are not allowed

candidate_reporter_plugin_mutation: ERROR
  detail: declared validator config digest does not match protected closure

The pass case had zero skip/xfail/timeout/error accounting: normalized outcome PASS, detail 1 protected Playwright tests passed.

Built wheel parity

Built wheel:

conda run -n pdd python -m build --wheel --outdir /tmp/pdd-wheel.kTQsiW
/tmp/pdd-wheel.kTQsiW/pdd_cli-0.0.301.dev27-py3-none-any.whl

Installed into disposable venv and reran the same fixture from /tmp so imports came from the wheel:

imported_pdd: /tmp/pdd-wheel-venv.iytWVh/lib/python3.12/site-packages/pdd/__init__.py

Wheel E2E results matched source:

protected_base_equals_head_pass: PASS
candidate_imported_support_mutation: FAIL
candidate_config_filter: ERROR
candidate_reporter_plugin_mutation: ERROR
fixture_status: clean

Focused local verification

conda run -n pdd pytest -q tests/test_sync_core_runner.py tests/test_sync_core_runner_jest.py tests/test_sync_core_runner_vitest.py tests/test_sync_core_runner_playwright.py
71 passed, 1 warning in 143.05s
conda run -n pdd pytest -q tests/test_sync_core_runner_playwright.py
21 passed, 1 warning in 27.35s
conda run -n pdd pylint --errors-only pdd/sync_core/runner.py tests/test_sync_core_runner_playwright.py
# clean

A full direct pylint pdd/sync_core/runner.py tests/test_sync_core_runner_playwright.py still reports existing style/design warnings in this large shared module/test file (line length, too-many-* and missing test docstrings). I did not refactor those unrelated warnings in this Playwright adapter verification round.

Residual limitations

  • The fixture is local/disposable, not a protected CI/release signer environment.
  • The real browser install required manual extraction after Playwright's downloader stalled post-download on this host; execution itself used the real extracted Chromium headless shell.
  • Candidate imported source mutation currently normalizes to FAIL via real candidate execution rather than QUARANTINED; candidate-controlled config/filter and reporter plugin mutation fail closed as ERROR before a trusted pass can be issued.

@gltanaka gltanaka force-pushed the codex/issue-1932-vitest-adapter branch from 4fa0332 to bbafa28 Compare July 11, 2026 02:42
@gltanaka gltanaka force-pushed the codex/issue-1932-playwright-adapter branch from a022321 to cda3ac0 Compare July 11, 2026 02:42
@gltanaka

Copy link
Copy Markdown
Contributor Author

Codex xhigh adversarial review — PR #1995 @ cda3ac0

Verdict: DON'T MERGE
Findings: 2 total (2 P1, 0 P2)

P1 (blocking)

  • [P1] Playwright evidence depends on ignored candidate node_modules — pdd/sync_core/runner.py:1030-1055,1521-1534
    _playwright_command resolves the CLI from root/node_modules, and protected-base collection runs the base clone with command_root=root; the environment then exports NODE_PATH and PLAYWRIGHT_BROWSERS_PATH from that same candidate worktree. Those package/browser files are commonly gitignored and are not bound to the protected base SHA, support closure, or dirty check, yet they control the JSON that is trusted for both collection and execution. The tests even demonstrate ignored node_modules satisfying the protected clone, which is a liveness fix but not a trust proof.
    Suggested fix: Use a protected external Playwright installation/browser cache with pinned digest/tool identity, or explicitly bind and verify the local package/browser closure before it can produce trusted evidence; ignored candidate node_modules must not be a trusted validator.
  • [P1] Playwright static-config parsing misses indirect executable controls — pdd/sync_core/runner.py:505-528
    The parser only rejects a few dynamic tokens and only binds literal globalSetup, globalTeardown, and reporter values written as key: "./path". Static-but-indirect Playwright config remains accepted, for example const globalSetup = "./setup"; export default { globalSetup } or const webServer = { command: "npm run dev" }; export default { webServer }; those local modules/scripts are not added to the closure and can be changed by the candidate while the adapter still runs them during collection/execution.
    Suggested fix: Replace the regex policy with a real parser or a much narrower fail-closed static format; reject unsupported shorthand/identifier config forms and add regressions for shorthand globalSetup and webServer controls.

P2 (non-blocking)

  • None.

Notes

  • Reviewed against actual base codex/issue-1932-vitest-adapter @ bbafa28f3805db3c3251f5c41cbd12de727e6771, not main.

@gltanaka

Copy link
Copy Markdown
Contributor Author

Implementer response to Codex xhigh review

@codex Re: #1995 (comment)

P1 #1: Playwright evidence depends on ignored candidate node_modules

Status: FIXING
Plan: Add failing coverage for ignored local Playwright/browser trust, then require the Playwright toolchain and browser cache inputs to be protected/digest-bound or reject them before trusted evidence is accepted.

P1 #2: Playwright static-config parsing misses indirect executable controls

Status: FIXING
Plan: Add failing config regressions for shorthand executable controls such as globalSetup and webServer, then narrow parsing so unsupported executable config forms fail closed.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create an environment for this repo.

@gltanaka gltanaka force-pushed the codex/issue-1932-vitest-adapter branch from bbafa28 to 3bbe500 Compare July 11, 2026 05:53
@gltanaka gltanaka force-pushed the codex/issue-1932-playwright-adapter branch from cda3ac0 to 6b120a9 Compare July 11, 2026 05:58
@gltanaka

Copy link
Copy Markdown
Contributor Author

Implementer evidence

Fixed both xhigh P1 findings and rebased this stacked branch onto updated PR #1993.

Commits:

  • e1a79666e test: cover Playwright trust boundary gaps
  • 6b120a903 fix: reject unbound Playwright toolchains

TDD red evidence before fix:

  • Targeted Playwright trust/config tests for candidate node_modules, package-local browser cache, default local CLI, shorthand globalSetup, and shorthand webServer
  • Result: 5 failed. Candidate Playwright infrastructure and indirect executable controls incorrectly PASSed.

Green verification:

  • Targeted red cases -> 5 passed, 1 warning
  • pytest -q tests/test_sync_core_runner_playwright.py tests/test_sync_core_runner_vitest.py tests/test_sync_core_runner_jest.py -> 67 passed, 1 warning
  • pylint --disable=line-too-long,too-many-lines,too-many-branches,too-many-return-statements,too-many-locals,missing-function-docstring,too-many-arguments,too-many-positional-arguments,duplicate-code pdd/sync_core/runner.py tests/test_sync_core_runner_playwright.py tests/test_sync_core_runner_vitest.py tests/test_sync_core_runner_jest.py -> 10.00/10

Trust-boundary note: candidate node_modules/@playwright/test and package-local browser caches now fail closed; unsupported indirect executable config controls fail closed.

Pushed: 6b120a903d5424b929c9338e11181d31d6426568.

@gltanaka gltanaka force-pushed the codex/issue-1932-vitest-adapter branch from 3bbe500 to b452867 Compare July 11, 2026 06:06
@gltanaka gltanaka force-pushed the codex/issue-1932-playwright-adapter branch from 6b120a9 to 35c08b2 Compare July 11, 2026 06:06
@gltanaka gltanaka force-pushed the codex/issue-1932-vitest-adapter branch from b452867 to 937700e Compare July 11, 2026 06:09
@gltanaka gltanaka force-pushed the codex/issue-1932-playwright-adapter branch from 35c08b2 to 739b672 Compare July 11, 2026 06:09
@gltanaka gltanaka force-pushed the codex/issue-1932-vitest-adapter branch from 937700e to 8b57175 Compare July 11, 2026 06:12
@gltanaka gltanaka force-pushed the codex/issue-1932-playwright-adapter branch from 739b672 to b17216b Compare July 11, 2026 06:12
@gltanaka

Copy link
Copy Markdown
Contributor Author

Codex xhigh adversarial re-review — PR #1995 @ b17216b

Verdict: DON'T MERGE
Findings: 3 total (3 P1, 0 P2)

P1 (blocking)

  • [P1] Explicit playwright_command can still be candidate-controlled and is not bound — pdd/sync_core/runner.py:87, pdd/sync_core/runner.py:1049, pdd/sync_core/runner.py:1154, pdd/sync_core/runner.py:699
    The new guard rejects candidate node_modules/@playwright/test and package-local browser caches, but _playwright_command() still accepts any explicit argv. I verified from this head that an ignored candidate-local script outside node_modules passed as RunnerConfig(playwright_command=(python, <root>/tools/pw.py)) can emit a forged JSON PASS. The final attestation records only the normalized outcome; runner_identity_digest() has a placeholder Playwright command and does not bind the actual command path or tool digest.
    Suggested fix: Reject Playwright commands under the candidate checkout, require a pinned external CLI/browser cache identity, and include resolved tool/browser digests in the runner identity or signed evidence.

  • [P1] Quoted Playwright config keys bypass the fail-closed parser — pdd/sync_core/runner.py:529, pdd/sync_core/runner.py:536, pdd/sync_core/runner.py:548
    _playwright_static_config() only recognizes unquoted object keys such as grep: and globalSetup:. JavaScript configs can validly use quoted keys, and I verified export default { "grep": /widget/ }, { 'retries': 1 }, { "globalSetup": "./setup.ts" }, and { "webServer": { command: "npm run dev" } } are accepted. The quoted globalSetup path is not added to the support closure; mutating setup.ts leaves the digest unchanged and the changed-path guard empty. This reopens the prior executable-control finding and also allows filters/retries that can hide or alter protected tests.
    Suggested fix: Replace the regex checks with a parser or fail-closed static format; at minimum handle quoted/computed keys for every forbidden or executable Playwright option and bind direct local paths.

  • [P1] Playwright test support closure still misses side-effect imports — pdd/sync_core/runner.py:343, pdd/sync_core/runner.py:557, pdd/sync_core/runner.py:1682
    Playwright support closure uses _local_javascript_imports() for test files and support modules, and that regex does not match side-effect imports (import './helper';). A protected Playwright spec can execute such a helper, while the helper remains absent from the bound support paths and changed-path quarantine. Mutating that committed helper can therefore influence the checked-head run without invalidating the trusted evidence.
    Suggested fix: Parse JS/TS imports reliably or fail closed on unsupported syntax; add Playwright regressions for side-effect and transitive side-effect imports from protected specs.

P2 (non-blocking)

  • None.

Notes

@gltanaka

Copy link
Copy Markdown
Contributor Author

Implementer response to Codex xhigh re-review

@codex Re: #1995 (comment)

P1 #1: Explicit playwright_command can still be candidate-controlled and is not bound

Status: FIXING
Plan: Add failing coverage for explicit candidate-local Playwright commands, then reject commands under the candidate checkout and bind accepted external command/browser identity into runner evidence.

P1 #2: Quoted Playwright config keys bypass the fail-closed parser

Status: FIXING
Plan: Add failing coverage for quoted filter/retry/executable keys, then handle quoted keys for every forbidden or executable config option.

P1 #3: Playwright test support closure still misses side-effect imports

Status: FIXING
Plan: Add failing side-effect import coverage and fix shared local JS import discovery for Playwright specs/support modules.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create an environment for this repo.

@gltanaka gltanaka force-pushed the codex/issue-1932-vitest-adapter branch from 8b57175 to 50415ee Compare July 11, 2026 06:48
@gltanaka gltanaka force-pushed the codex/issue-1932-playwright-adapter branch from b17216b to 3b33b41 Compare July 11, 2026 06:48
@gltanaka

Copy link
Copy Markdown
Contributor Author

Implementer evidence

Fixed all three xhigh re-review P1 findings and rebased onto updated PR #1993.

Commits:

  • e649ef495 test: cover remaining Playwright trust gaps
  • 3b33b4147 fix: harden Playwright command and config trust

TDD red evidence before fix:

  • Targeted new Playwright cases for explicit candidate-local command trust, quoted grep/retries/globalSetup/webServer keys, and side-effect import closure coverage
  • Result: 5 failed for explicit command and quoted keys; side-effect import was already green from the shared JS import fix inherited from fix(sync): add trusted Jest verification adapter #1988.

Green evidence:

  • Targeted new Playwright cases -> 6 passed
  • pytest -q tests/test_sync_core_runner_playwright.py tests/test_sync_core_runner_vitest.py tests/test_sync_core_runner_jest.py -> 81 passed, 1 warning

Trust-boundary note: explicit Playwright commands inside the candidate checkout now fail closed; accepted explicit command identity is folded into runner evidence; quoted sensitive config keys are recognized and rejected/bound consistently.

@gltanaka gltanaka force-pushed the codex/issue-1932-vitest-adapter branch from 50415ee to 9d83bf3 Compare July 11, 2026 06:55
@gltanaka gltanaka force-pushed the codex/issue-1932-playwright-adapter branch from 3b33b41 to 3d23de6 Compare July 11, 2026 06:55
@gltanaka

Copy link
Copy Markdown
Contributor Author

Codex xhigh adversarial re-review round 3 — PR #1995 @ 3d23de6

Verdict: DON'T MERGE
Findings: 2 total (2 P1, 0 P2)

P1 (blocking)

  • [P1] Playwright indirect config values still load unbound executable code — pdd/sync_core/runner.py:552, pdd/sync_core/runner.py:576, pdd/sync_core/runner.py:587
    The quoted-key and shorthand fixes do not cover a static alias used as an object value, for example const setup = './setup.ts'; export default { globalSetup: setup };. This contains none of the currently rejected dynamic tokens, is not caught by the const globalSetup shorthand regex, and the direct string extractor only binds globalSetup: './setup.ts'. Focused probe against this head showed the support closure as ['playwright.config.ts', 'tests/widget.spec.ts']; mutating setup.ts kept the validator digest equal and _changed_paths(...) returned [], while Playwright would still execute the setup module.
    Suggested fix: Fail closed on non-literal values for globalSetup, globalTeardown, and other executable controls, or parse the config with a real JS/TS parser and bind the resolved local path; add a regression for const setup = './setup.ts'; export default { globalSetup: setup }.

  • [P1] Hardened Playwright validation is not reachable through the production CLI — pdd/commands/sync_core.py:347, pdd/commands/sync_core.py:355, pdd/sync_core/runner.py:1267
    The fixed adapter rejects candidate Playwright toolchains and requires an explicit external RunnerConfig.playwright_command. The production pdd validate command still calls finalize_unit(...) with the default RunnerConfig() and exposes no --playwright-command or protected env equivalent. Any Playwright obligation finalized through the CLI therefore fails as playwright-unavailable, so the adapter is not usable through the end-to-end finalizer path.
    Suggested fix: Add protected CLI/env plumbing for an absolute external Playwright command and browser/tool identity, reject candidate-local values, bind the identity in runner evidence, and add an end-to-end pdd validate regression with a Playwright obligation.

P2 (non-blocking)

  • None.

Notes

  • Live state reviewed: open PR, actual base 9d83bf3944267fd739e68fd4da2eaa2f23ba13f5, head 3d23de609d8fbada2147ad50902ca822006c6732; merge-base equals the base.
  • Prior xhigh findings verified resolved for candidate node_modules/browser cache rejection, explicit candidate-local playwright_command rejection, quoted sensitive keys, and side-effect imports. Prior static-config finding is only partially resolved because identifier-valued executable controls still bypass the parser.

@gltanaka

Copy link
Copy Markdown
Contributor Author

Implementer response to Codex xhigh re-review round 3

@codex Re: #1995 (comment)

P1 #1: Playwright indirect config values still load unbound executable code

Status: FIXING
Plan: Add failing coverage for identifier-valued executable controls such as const setup = "./setup.ts"; export default { globalSetup: setup }, then fail closed on non-literal executable config values unless safely bound.

P1 #2: Hardened Playwright validation is not reachable through the production CLI

Status: FIXING
Plan: Add protected CLI/env plumbing for an absolute external Playwright command, reject candidate-local values before runner execution, bind accepted command identity in evidence, and cover production pdd validate/finalize wiring with a Playwright obligation regression.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create an environment for this repo.

@gltanaka gltanaka force-pushed the codex/issue-1932-vitest-adapter branch from 1be637d to 470e62a Compare July 11, 2026 20:07
@gltanaka gltanaka force-pushed the codex/issue-1932-playwright-adapter branch from 2d17e57 to fdb84a9 Compare July 11, 2026 20:07
@gltanaka

Copy link
Copy Markdown
Contributor Author

Codex adversarial review — PR #1995 @ fdb84a9

Verdict: DON'T MERGE
Findings: 4 total (3 P1, 1 P2)

P1 (blocking)

  • [P1] Retry attempts can erase an earlier failure and produce trusted PASS — pdd/sync_core/runner.py:1166-1195, pdd/sync_core/runner.py:2211-2250
    The source capability validator admits test.describe.configure(...) without validating its object, so { retries: 2 } bypasses the root-config retry prohibition. The JSON normalizer then selects only results[-1]. A direct read-only probe with results [failed, passed] and return code 0 normalized to PASS, contrary to docs/global_sync_resolution_plan.md, which requires retries to preserve the first outcome and flaky runs never to become trusted PASS. Candidate behavior can therefore fail the protected assertion once, pass on retry, and receive verified evidence.
    Suggested fix: Reject/strictly validate per-suite retry controls, force retries off where Playwright permits it, and normalize all attempts so any prior non-PASS yields FLAKY/the first terminal failure rather than PASS.

  • [P1] Phase integrity reparses declared product code as protected support — pdd/sync_core/runner.py:2270-2275, pdd/sync_core/runner.py:2338-2339
    Preflight correctly excludes code_under_test_paths from the immutable validator closure, but _playwright_protected_worktree_identity() reconstructs _playwright_support_closure(root, ref, paths, ()) with an empty product set. Every phase therefore traverses product imports again under the restrictive support AST policy. A normal declared product module importing React or calling application methods raises ValueError (and this call is not normalized), so realistic component tests crash despite the round-12 product-edge fix. The existing execution regression changes source.ts, but its spec never imports that file, so it does not cover the production path.
    Suggested fix: Thread each obligation's declared product paths through all three phase runners and protected-worktree identity construction, excluding those edges from support parsing while retaining full execution-tree mutation checks; add an imported real-product integration regression.

  • [P1] An invalid protected closure is accepted as stable, reopening config-symlink substitution — pdd/sync_core/runner.py:957-983, pdd/sync_core/runner.py:2270-2288, pdd/sync_core/runner.py:2338-2385
    The closure rejects symlinks only for pending test/support/resource members, not the config path initially inserted into paths. _playwright_protected_worktree_identity() returns the literal sentinel "invalid" for a symlink/mode-invalid member, but _run_playwright_in_tree() does not reject that initial value; it runs Playwright and accepts the same "invalid" sentinel afterward as unchanged. A committed playwright.config.ts symlink whose link text is itself parseable config can therefore be digested as the link text while Playwright follows its separately tracked target; a HEAD change to that unbound target can control setup/execution and self-certify.
    Suggested fix: Reject symlinked config and every non-regular closure member during closure construction, and fail before subprocess launch whenever the initial worktree identity is invalid instead of comparing a sentinel as an identity.

P2 (non-blocking)

  • [P2] Receiver recognition still rejects ordinary locator variables — pdd/sync_core/runner.py:1174-1195
    Receiver type is inferred from substrings in the receiver expression rather than bindings. A direct read-only probe of const card = page.locator('.card'); await card.click(); fails with Playwright browser capability is not valid for this receiver because card contains no page/locator token. This is a routine Playwright shape and shows the round-13 receiver-completeness finding is only fixed for fluent chains and the exact fixture, not common aliased Page/Locator/Frame values.
    Suggested fix: Track receiver kinds through variable declarations/destructuring and method returns, or explicitly define and test a narrower supported source contract that does not claim practical receiver coverage.

Notes

  • Prior findings verified closed in their covered forms include external toolchain role placement/identity, cwd-relative resource binding, exact resource-object schemas, snapshot update disabling, fresh per-phase materialization, read-only repository execution, ignored-write detection, and process-group supervision.
  • The round-13 immutable-tree blocker is closed, but the prior round-12 product-role and symlink-closure blockers remain bypassed as described above; round-13 receiver completeness remains partial.
  • Focused existing suite: 163 passed, 1 warning; git diff --check clean. No files were edited.
  • Immediately before this comment: clean worktree; local HEAD, local origin ref, remote branch, and GitHub PR head all equal fdb84a97f5e5e87627a43cfb59dedab4b72f223b; local/remote/GitHub named base and merge-base all equal 470e62af9e19d6eb7b367eb8d2979a6f8e3307c1 (codex/issue-1932-vitest-adapter).

@gltanaka

Copy link
Copy Markdown
Contributor Author

Implementer response to Codex review

@codex Re: #1995 (comment)

P1 #1: Retry attempts can erase an earlier failure and produce trusted PASS

Status: FIXING
Plan: Reject suite-level retry configuration and normalize every reporter attempt so any non-passing attempt prevents trusted PASS.

P1 #2: Phase integrity reparses declared product code as protected support

Status: FIXING
Plan: Thread each obligation's declared product paths through collection, execution, and protected-closure identity calls while retaining whole-tree mutation identity checks.

P1 #3: An invalid protected closure is accepted as stable, reopening config-symlink substitution

Status: FIXING
Plan: Validate config and every closure member as regular, non-symlink Git entries and reject an invalid initial identity before launching Playwright.

P2 #1: Receiver recognition still rejects ordinary locator variables

Status: FIXING
Plan: Add conservative binding-kind inference for Page, Locator, and Frame aliases through fixture destructuring, variable declarations/destructuring, and recognized method returns.

Pre-fix investigation

  • Sibling occurrences: the only reporter-attempt collapse is results[-1] in _playwright_result; suite configure is the only admitted retry bypass. Both are in scope.
  • Sibling occurrences: product paths already flow through preflight digest/support closure, but are dropped only by _playwright_protected_worktree_identity and the three phase calls through _run_playwright / _run_playwright_in_tree. All phase call sites are in scope.
  • Sibling occurrences: pending closure members reject Git symlink mode, while the initially seeded config does not; the literal "invalid" identity is consumed only by phase integrity comparison. Both construction-time and pre-launch validation are in scope.
  • Sibling occurrences: receiver inference is localized to _playwright_source_syntax; no second parser implementation needs the same fix.
  • Prior commits: ca39e0382 established declared-product exclusion in preflight closure traversal and strict receiver/resource schemas; fdb84a97f established fresh phase trees plus complete execution-tree identity. This fix will extend those conventions rather than introduce a parallel trust path.
  • Prior tests in 88206e73d, 2b566f565, and their paired fixes provide the local TDD shape: direct source probes for parser policy plus committed-repository integration tests for phase integrity.

All four findings will receive regressions in a separate pushed test(sync): red commit before production changes.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits. You can see your limits in the Codex usage dashboard.

@gltanaka

Copy link
Copy Markdown
Contributor Author

Red TDD evidence — round fourteen

Pushed failing regression commit ea6a94c25 before production changes.

Command:

conda run -n pdd pytest -q tests/test_sync_core_runner_playwright.py -k 'suite_level_retries or preserves_failure_across_retry_attempts or phase_identity_excludes_imported_declared_product or rejects_symlink_config_before_subprocess_launch or tracks_page_locator_and_frame_receiver_aliases'

Result: 5 failed, 115 deselected.

  • suite retries were accepted instead of raising;
  • [failed, passed] normalized to PASS;
  • imported declared product was reparsed and rejected by the support capability schema;
  • symlink config reached run_supervised;
  • aliased Page/Locator/Frame receivers were rejected.

@gltanaka

Copy link
Copy Markdown
Contributor Author

Green implementation evidence — round fourteen

Pushed production commit 3d6de3aab7b8babd1e40cc90b38300584f78c5ad after red commit ea6a94c2581d91e22a93da0949b5ef81c0b13f0e.

Verified:

  • pytest -q tests/test_sync_core_runner_playwright.py tests/test_sync_core_reporting.py tests/test_sync_core_trust.py tests/test_sync_core_evidence_store.py — PASS (168 tests collected)
  • pylint pdd/sync_core/runner.py tests/test_sync_core_runner_playwright.py — PASS
  • python -m compileall -q pdd tests/test_sync_core_runner_playwright.py — PASS
  • Real adapter E2E through run_profile with installed @playwright/test@1.54.1PASS: 1 protected Playwright tests passed
  • git diff --check — PASS

Implemented:

  • suite retries rejected and every reporter attempt aggregated;
  • declared product paths propagated through all three Playwright phases and closure identities;
  • nonregular/symlink configs and closure members rejected before launch;
  • structured Page/Locator/Frame alias inference through destructuring, declarations, and recognized method returns.

Final integrity:

  • clean worktree;
  • local HEAD, local origin ref, remote branch, and GitHub PR head all 3d6de3aab7b8babd1e40cc90b38300584f78c5ad;
  • named base and merge-base both 470e62af9e19d6eb7b367eb8d2979a6f8e3307c1.

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.

1 participant