Skip to content

fix(sync): add trusted Jest verification adapter#1988

Merged
gltanaka merged 14 commits into
mainfrom
codex/issue-1932-jest-adapter
Jul 13, 2026
Merged

fix(sync): add trusted Jest verification adapter#1988
gltanaka merged 14 commits into
mainfrom
codex/issue-1932-jest-adapter

Conversation

@gltanaka

Copy link
Copy Markdown
Contributor

Summary

Adds a fail-closed Jest adapter to the trusted sync runner.

Investigation

  • Existing runner.py dispatches only pytest; the Jest adapter follows its base-clone identity, support-closure, dirty-support, exact execution, and secret-scrubbing conventions.
  • Stable identity: relative-test-file::ancestor title > test title, emitted only by a temporary trusted reporter. Candidate code never supplies protected expected identities.
  • Supported configuration is static JSON (jest.config.json or package.json jest object). The closure binds config and local setup, transform, reporter, resolver, project, and static relative-import support. Executable/dynamic Jest config (jest.config.js/cjs/mjs/ts) fails closed.
  • pdd_cloud currently has executable Jest configs, so they are deliberately rejected until an independently reviewable static-config policy exists. Vitest and Playwright are not supported or claimed.
  • Prior runner hardening: f39dd6090 established config-closure binding and credential scrubbing; this PR follows that approach. No sibling trusted-runner dispatch sites require changes.

Test plan

  • conda run -n pdd pytest -q tests/test_sync_core_runner.py tests/test_sync_core_runner_jest.py
  • conda run -n pdd pylint pdd/sync_core/runner.py
  • git diff --check
  • Real local Jest 29.7.0 fixture E2E using /Users/gregtanaka/Developer/pdd_cloud/frontend/node_modules/jest/bin/jest.js
  • python -m build --wheel --outdir /tmp/pdd-jest-wheel-dist

@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

Pre-fix investigation

  • pdd/sync_core/runner.py accepted only pytest; its base-clone node collection, closure digest, dirty support quarantine, exact execution, and secret stripping are the adapter convention.
  • Jest identity is relative-test-file::ancestor title > test title, emitted by a trusted temporary reporter. Expected identities originate only from a protected-base clone.
  • Static JSON Jest configs bind local setupFiles, setupFilesAfterEnv, transforms, reporters, resolvers, projects, and static relative imports. Dynamic/executable config fails closed.
  • pdd_cloud Jest configs are executable .js, therefore correctly unsupported by this adapter. Vitest and Playwright are out of scope.
  • Prior art: f39dd6090 supplied the protected config-closure and credential-isolation pattern. No same-root-cause dispatch siblings were left unchanged.

@gltanaka

Copy link
Copy Markdown
Contributor Author

TDD red/green evidence

Red commit bc5fa4888 was pushed before implementation.

ImportError: cannot import name `jest_validator_config_digest` from `pdd.sync_core.runner`
ERROR tests/test_sync_core_runner_jest.py

Green commit: c7ec9ee61.

16 passed, 1 warning in 16.94s

The dedicated suite covers pass/fail/skip/todo/zero collection, identity mismatch and candidate-only tests, removed protected tests, config/setup/transform mutation, dirty support, timeout, secret isolation, malformed JSON, and dynamic config rejection.

@gltanaka

Copy link
Copy Markdown
Contributor Author

Verification E2E

  • Targeted: conda run -n pdd pytest -q tests/test_sync_core_runner.py tests/test_sync_core_runner_jest.py
  • Lint: conda run -n pdd pylint pdd/sync_core/runner.py
  • Whitespace: git diff --check
  • Wheel build: python -m build --wheel --outdir /tmp/pdd-jest-wheel-dist
  • Live fixture E2E used local Jest 29.7.0 at /Users/gregtanaka/Developer/pdd_cloud/frontend/node_modules/jest/bin/jest.js, with a static JSON config and a committed test. The trusted base/head adapter result was: PASS 1 protected Jest tests passed.

No provider credentials or npm scripts were used.

@gltanaka gltanaka force-pushed the codex/issue-1932-jest-adapter branch from c7ec9ee to dc6d5cc Compare July 11, 2026 01:29
@gltanaka

Copy link
Copy Markdown
Contributor Author

Rebase verification

Rebased the stacked branch onto origin/codex/issue-1932-verifiable-goal at 8a8cf5137, retaining the red test, adapter, and follow-up hardening commits. Pushed with --force-with-lease; current head is dc6d5ccd8 and the worktree is clean.

Post-rebase dedicated adapter verification: conda run --no-capture-output -n pdd pytest -q tests/test_sync_core_runner_jest.py -> 16 passed, 1 warning in 18.33s.

The existing-runner focused command was started after the rebase and emitted six passing tests before the command runner ended its captured output; no failing result was emitted. The prior full targeted pair and pylint run were green before rebase, and the rebase applied cleanly.

@gltanaka gltanaka force-pushed the codex/issue-1932-jest-adapter branch from dc6d5cc to 92dee25 Compare July 11, 2026 01:44
@gltanaka

Copy link
Copy Markdown
Contributor Author

Rebased onto origin/codex/issue-1932-verifiable-goal at f78bad5. New head: 92dee25. Verified: pytest tests/test_sync_core_runner_jest.py (16 passed); git diff --check passed. Worktree and pushed branch are clean.

@gltanaka gltanaka force-pushed the codex/issue-1932-jest-adapter branch from 92dee25 to 39e2690 Compare July 11, 2026 02:41
@gltanaka

Copy link
Copy Markdown
Contributor Author

Codex xhigh adversarial review — PR #1988 @ 39e2690

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

P1 (blocking)

  • [P1] Jest closure omits protected-test imports and some executable hooks — pdd/sync_core/runner.py:246-347,982-996
    _jest_support_closure accepts test_paths, but the digest path deletes/ignores them and seeds the closure only from selected config references. run_obligation then quarantines only the declared test file plus that incomplete support list. A protected test can keep the same tests/widget.test.js identity while importing ./helper, and a candidate commit can change helper.js without appearing in changed; the head Jest run executes the candidate helper and can still produce a trusted PASS. The same allowlist ignores several Jest execution hooks unless they happen to be one of setup/transform/reporter/resolver/projects, so local modules behind other static config keys can also affect the run without being bound.
    Suggested fix: Seed the Jest closure with every protected test path, recursively include literal local imports from tests and support modules, and either bind or fail closed on every Jest config key that can load code or alter collection/execution; add regression tests for a committed helper import and a custom environment/global setup mutation.
  • [P1] The trusted Jest adapter can execute a candidate-controlled runner — pdd/sync_core/runner.py:557-565,615-635
    The default runner is root/node_modules/jest/bin/jest.js, and the process that writes PDD_TRUSTED_JEST_OUTPUT is treated as the trusted reporter source. Ignored or otherwise candidate-controlled node_modules is not part of the Git closure and is not covered by _dirty_jest_support, so a modified local Jest binary can write the expected identities/statuses directly and self-certify a PASS for both protected-base collection and head execution.
    Suggested fix: Do not use the candidate checkout's node_modules as a trusted validator. Require a protected external Jest installation with a pinned digest/tool identity, or bind the full package/tool closure and reject dirty/ignored tool files before accepting reporter output.

P2 (non-blocking)

  • None.

Notes

  • Reviewed against actual base codex/issue-1932-verifiable-goal @ 63d31004f3e9633d11683cf13e2573f9fb8a2d39, not main.

@gltanaka

Copy link
Copy Markdown
Contributor Author

Implementer response to Codex xhigh review

@codex Re: #1988 (comment)

P1 #1: Jest closure omits protected-test imports and executable hooks

Status: FIXING
Plan: Add failing coverage for imported protected-test helpers and executable config hooks, then seed/bind the Jest support closure from protected tests and fail closed on unsupported executable config controls.

P1 #2: Trusted Jest adapter can execute a candidate-controlled runner

Status: FIXING
Plan: Add failing coverage for ignored local node_modules runner trust, then require the Jest toolchain to be digest-bound/protected or reject it before accepting trusted evidence.

@chatgpt-codex-connector

Copy link
Copy Markdown

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

@gltanaka

Copy link
Copy Markdown
Contributor Author

Implementer evidence

Fixed both xhigh P1 findings.

Commits:

  • c37f5571d test: cover Jest trust boundary gaps
  • 70ea71bab fix: bind Jest support closure and reject local runner

TDD red evidence before fix:

  • pytest -q tests/test_sync_core_runner_jest.py::test_jest_imported_test_helper_mutation_cannot_pass tests/test_sync_core_runner_jest.py::test_jest_executable_config_hook_mutation_cannot_pass tests/test_sync_core_runner_jest.py::test_default_candidate_node_modules_jest_is_not_trusted
  • Result: 5 failed. The imported helper and config hook mutations incorrectly PASSed, and the local node_modules/jest runner was trusted.

Green verification:

  • pytest -q tests/test_sync_core_runner_jest.py -> 21 passed, 1 warning
  • pylint --disable=line-too-long,too-many-lines,too-many-branches,too-many-return-statements,too-many-locals,missing-function-docstring pdd/sync_core/runner.py tests/test_sync_core_runner_jest.py -> 10.00/10

Trust-boundary note: default candidate-checkout node_modules/jest now fails closed; trusted Jest execution requires an explicit external command rather than candidate-local infrastructure.

Pushed: 70ea71bab365151292f0366f5a63d298f208fa45.

@gltanaka gltanaka force-pushed the codex/issue-1932-jest-adapter branch 3 times, most recently from 39cb2b8 to 53891da Compare July 11, 2026 06:12
@gltanaka

Copy link
Copy Markdown
Contributor Author

Codex xhigh adversarial re-review — PR #1988 @ 53891da

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

P1 (blocking)

  • [P1] Explicit jest_command can still be candidate-controlled and is not bound into the attestation — pdd/sync_core/runner.py:70, pdd/sync_core/runner.py:580, pdd/sync_core/runner.py:638, pdd/sync_core/runner.py:438
    The default root/node_modules/jest path now fails closed, but the trusted path is simply moved to RunnerConfig.jest_command. _jest_command() returns any caller-supplied argv, _run_jest() executes it, and runner_identity_digest() contains no actual command path, version, package digest, or toolchain digest. A caller can pass an ignored candidate checkout script such as <root>/node_modules/jest/bin/jest.py explicitly and it will write PDD_TRUSTED_JEST_OUTPUT for both protected-base collection and head execution; the final signed attestation records only PASS/FAIL, not the command digest.
    Suggested fix: Reject any explicit Jest command located under the candidate checkout or otherwise unprotected, require a pinned external toolchain identity/digest, and include that resolved command/tool digest in the runner identity or signed evidence.

  • [P1] Jest support closure still misses side-effect imports — pdd/sync_core/runner.py:324, pdd/sync_core/runner.py:347, pdd/sync_core/runner.py:1003
    The previous imported-helper fix only covers require(...), dynamic import(...), and from './x'. It does not match valid side-effect imports such as import './helper';. I verified from this head that a protected tests/widget.test.js with import './helper'; produces a closure containing only jest.config.json and the test file; mutating tests/helper.js leaves jest_validator_config_digest(...) unchanged and the changed-path guard empty. A candidate can therefore alter side-effect code used by the protected test while still receiving trusted PASS evidence.
    Suggested fix: Use a real JS/TS import parser or fail closed on unsupported import syntax, and add regression coverage for side-effect imports and transitive side-effect imports.

  • [P1] Jest executable config hooks are still incomplete — pdd/sync_core/runner.py:50, pdd/sync_core/runner.py:62, pdd/sync_core/runner.py:270
    The config allowlist binds some local code-loading keys, but still ignores valid Jest execution hooks such as snapshotSerializers and testSequencer and resolution controls such as moduleNameMapper. I verified snapshotSerializers: ["<rootDir>/serializer.js"] is accepted while the serializer is absent from the closure; mutating serializer.js leaves the digest unchanged and the changed-path guard empty. Snapshot serializers and sequencers can directly change protected test outcomes/collection.
    Suggested fix: Either fail closed on every unsupported Jest key that can load code or affect collection/execution, or bind those paths/modules transitively; add regressions for snapshotSerializers, testSequencer, and local moduleNameMapper targets.

P2 (non-blocking)

  • None.

Notes

@gltanaka

Copy link
Copy Markdown
Contributor Author

Implementer response to Codex xhigh re-review

@codex Re: #1988 (comment)

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

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

P1 #2: Jest support closure still misses side-effect imports

Status: FIXING
Plan: Add failing coverage for direct and transitive import "./helper" support, then extend local JS import discovery or fail closed on unsupported syntax.

P1 #3: Jest executable config hooks are still incomplete

Status: FIXING
Plan: Add failing coverage for snapshotSerializers, testSequencer, and local moduleNameMapper, then bind those local controls transitively or reject unsupported forms.

@gltanaka

Copy link
Copy Markdown
Contributor Author

Implementer evidence

Fixed all three xhigh re-review P1 findings.

Commits:

  • ff9204597 test: cover remaining Jest trust gaps
  • ca7b8fee5 fix: harden Jest command and closure trust

TDD red evidence before fix:

  • Targeted new Jest cases for explicit candidate-local command trust, side-effect imports, snapshotSerializers, testSequencer, and local moduleNameMapper
  • Result: 5 failed, each previously produced trusted PASS.

Green evidence:

  • Targeted new Jest cases -> 5 passed
  • pytest -q tests/test_sync_core_runner_jest.py -> 26 passed, 1 warning

Trust-boundary note: explicit Jest commands inside the candidate checkout now fail closed; accepted explicit command identity is folded into runner evidence, side-effect imports are included in the support closure, and the additional executable config controls are bound or rejected.

@gltanaka

Copy link
Copy Markdown
Contributor Author

Codex xhigh adversarial re-review round 3 — PR #1988 @ cd292cd

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

P1 (blocking)

  • [P1] Jest moduleDirectories can load unbound local support code — pdd/sync_core/runner.py:300, pdd/sync_core/runner.py:340, pdd/sync_core/runner.py:363
    _jest_config_references() never rejects or binds Jest resolver roots such as moduleDirectories, while _local_javascript_imports() only resolves relative import specifiers. A protected test can use moduleDirectories: ["src", "node_modules"] and require("helper"); Jest executes src/helper.js, but that helper is absent from the support closure and from the changed-path guard. Focused probe against this head: closure was only ['jest.config.json', 'tests/widget.test.js']; mutating src/helper.js kept the validator digest equal and _changed_paths(...) returned [].
    Suggested fix: Fail closed on moduleDirectories and any other unmodeled local resolution controls, or resolve local bare-import roots transitively and include their executable files in the closure; add a regression for a bare import resolved from src.

  • [P1] Hardened Jest 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:706
    The trust fix correctly makes default candidate node_modules/jest fail closed and requires an explicit external RunnerConfig.jest_command. The production pdd validate command still calls finalize_unit(...) without any RunnerConfig and exposes no --jest-command or protected env equivalent, so any Jest obligation finalized through the CLI returns ERROR (jest-unavailable or candidate node_modules untrusted). That means the adapter passes only direct run_profile(...) callers, not the end-to-end finalizer path.
    Suggested fix: Add protected CLI/env plumbing for an absolute external Jest command, resolve/hash that command identity in the runner evidence, reject candidate-local values, and cover pdd validate with a Jest obligation regression.

P2 (non-blocking)

  • None.

Notes

  • Live state reviewed: open PR, actual base b8f05dcd93d3c18067facaedde7bbc81216c4c6e, head cd292cd0c1bad5043e5c92c7a8a9e84b11f1806e; merge-base equals the base.
  • Prior xhigh findings verified resolved for default candidate node_modules, explicit candidate-local jest_command, direct relative imports, side-effect imports, and direct snapshotSerializers/testSequencer/moduleNameMapper cases. The new finding is an unbound Jest resolution mode not covered by those fixes.

@gltanaka

Copy link
Copy Markdown
Contributor Author

Implementer response to Codex xhigh re-review round 3

@codex Re: #1988 (comment)

P1 #1: Jest moduleDirectories can load unbound local support code

Status: FIXING
Plan: Add failing coverage for a bare import resolved through local moduleDirectories, then either bind that resolution mode transitively or fail closed on unmodeled local resolver roots.

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

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

@chatgpt-codex-connector

Copy link
Copy Markdown

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

@gltanaka

Copy link
Copy Markdown
Contributor Author

Implementer evidence

Fixed both round-3 P1 findings for #1988.

Commits:

  • 86a5e094c test: cover Jest round three gaps
  • cb22a8bd7 fix: wire protected Jest validation CLI

TDD red evidence before fix:

  • pytest -q tests/test_sync_core_runner_jest.py::test_jest_module_directories_bare_import_cannot_pass_unbound tests/test_sync_core_reporting.py::test_validate_command_wires_protected_jest_runner_config
  • Result: 2 failed. moduleDirectories bare-import support incorrectly PASSed, and pdd validate had no --jest-command option.

Green evidence:

  • Focused red cases -> 2 passed
  • pytest -q tests/test_sync_core_runner_jest.py tests/test_sync_core_reporting.py -> 46 passed, 1 warning

Trust-boundary note: Jest moduleDirectories now fails closed as an unbound resolver mode, and production pdd validate accepts protected external Jest command configuration while rejecting candidate-local command paths before runner execution.

Pushed head: cb22a8bd728fc91899048e72a92b17bfa6334dad.

@gltanaka gltanaka force-pushed the codex/issue-1932-jest-adapter branch from cb22a8b to 13af150 Compare July 11, 2026 07:37
@gltanaka

Copy link
Copy Markdown
Contributor Author

Codex xhigh adversarial re-review round 4 — PR #1988 @ 13af150

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

P1 (blocking)

  • [P1] Jest closure misses parent-directory imports — pdd/sync_core/runner.py:346-365, pdd/sync_core/runner.py:1107-1110
    _local_javascript_imports() joins a literal import such as ../src/helper as tests/../src/helper and never normalizes the .. before calling read_git_blob(...). The support closure therefore omits a normal committed helper imported by a protected Jest test. Because run_obligation() only quarantines artifact_paths | support_paths, a candidate can mutate src/helper.js while leaving tests/widget.test.js unchanged and the changed-path guard will not see the executable support file. Focused probe against this head: a protected require('../src/helper') left support_paths == ['jest.config.json', 'tests/widget.test.js'], and both jest_validator_config_digest(...) and _jest_support_digest(...) stayed unchanged after mutating src/helper.js.
    Suggested fix: Normalize literal JS import paths against source_path.parent, reject imports that escape the repo, include the normalized file/index candidates transitively, and add regressions for require('../src/helper') / import '../src/helper'.

P2 (non-blocking)

  • None.

Prior xhigh verification

  • Prior findings verified resolved for default candidate node_modules, explicit candidate-local jest_command, command identity binding, same-directory imports, side-effect imports, executable config hooks, moduleDirectories, and production --jest-command CLI wiring.
  • The new finding is a remaining import-resolution hole not covered by the earlier ./helper regressions.

Verification

  • Live base/head: codex/issue-1932-verifiable-goal @ 87aea93087dc246c9224c2df4fd3b6fd8b102dcf; head 13af150edb39d152d02339673f06b6e10f728b09; merge-base equals base.
  • conda run --no-capture-output -n pdd pytest -q tests/test_sync_core_runner_jest.py tests/test_sync_core_reporting.py -> 46 passed, 1 warning.
  • git diff --check 87aea93087dc246c9224c2df4fd3b6fd8b102dcf..13af150edb39d152d02339673f06b6e10f728b09 -> clean.

@gltanaka

Copy link
Copy Markdown
Contributor Author

Round-4 JS import closure fix pushed.

Changed: normalized literal relative JS imports against importer paths including ../, rejected repository escapes, added shared file/index candidate support for JS closure paths, and added Jest direct/side-effect/parent-dir/index/config-digest/escape regressions.

Evidence: focused #1988 suite passed: 51 passed. Top-stack suite from #1997 passed: 174 passed. Real JS smoke passed: Jest 30.4.1, Vitest 4.1.10, Playwright 1.61.1. git diff --check clean.

Lint: full make lint completed but failed on broad existing pylint findings, mainly unrelated cyclic-import reports; targeted pylint also hits existing large adapter style thresholds (line length, missing test docstrings, duplicate-code, too-many-lines/branches).

@gltanaka gltanaka force-pushed the codex/issue-1932-jest-adapter branch from aa0967b to 2a38d3e Compare July 11, 2026 08:47
@gltanaka

Copy link
Copy Markdown
Contributor Author

Stack rebuild evidence — foundation 7fcd7ed

Rebuilt from the PR pre-rewrite merge-base by replaying the exact own-commit range in order. Push used an explicit force-with-lease. Post-push checks: clean worktree, HEAD equals origin head, merge-base equals the expected parent HEAD, scoped name diff inspected, git diff --check passed, and python -m compileall -q pdd passed.

Head: a9c58e5
Parent: 7fcd7ed
Own commits: 12; changed files: 4.
No-exclusion sync-core suite: 386 passed, 1 failed. The sole failure is the inherited candidate replay-ledger parent safety check in test_sync_core_cli.py on the local macOS filesystem.

@gltanaka gltanaka force-pushed the codex/issue-1932-jest-adapter branch 3 times, most recently from eb0d59b to b94d8ba Compare July 11, 2026 20:07
@gltanaka gltanaka force-pushed the codex/issue-1932-verifiable-goal branch from fa4d808 to e54147b Compare July 12, 2026 21:59
@gltanaka gltanaka force-pushed the codex/issue-1932-jest-adapter branch from b94d8ba to 56f7719 Compare July 12, 2026 23:02
@gltanaka gltanaka changed the base branch from codex/issue-1932-verifiable-goal to main July 12, 2026 23:03
@gltanaka

gltanaka commented Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

Sol-high code review — PR #1988 @ 56f771918f5fa4e27d366434644606b8d70e00b1

Verdict: REQUEST_CHANGES

  1. [HIGH] pdd/sync_core/runner.py:670 — Application modules are classified as immutable Jest support. Impact: a normal candidate code change imported by a protected Jest test changes jest_validator_config_digest, so preflight rejects the obligation instead of testing the changed code. Evidence: _jest_support_closure recursively adds every relative import from each test and has no code_under_test_paths exclusion. A read-only probe with tests/widget.test.js -> ../src/widget.js included src/widget.js in the closure and changing only it changed the validator-config digest. The test fixture is self-contained and its fake runner reads source.js out of band, so it misses this case. Smallest correction: separate declared code-under-test from validator support as the pytest path does, and add a real import/code-change regression.

  2. [HIGH] pdd/sync_core/runner.py:1399 — Jest bypasses the merged foundation's untrusted-process supervisor and environment policy. Impact: candidate code runs with host filesystem/network access, descendants are not reaped, and protected capabilities remain available. Evidence: raw subprocess.run is used instead of _managed_subprocess; _jest_environment at line 1210 also retained PDD_ATTESTATION_SIGNER_COMMAND, PDD_CERTIFICATE_ISSUER, and PDD_RELEASED_CHECKER_COMMAND in a read-only probe. This conflicts with docs/global_sync_resolution_plan.md:1127-1129. Smallest correction: use _managed_subprocess with explicit mounts, use untrusted_child_environment, normalize sandbox/descendant failures, and add containment/capability tests.

  3. [HIGH] pdd/sync_core/runner.py:614 — Non-literal local JS loads are silently omitted from the signed support closure. Impact: a candidate can modify validation logic loaded by const path = './helper'; require(path) without changing the digest or triggering quarantine. Evidence: discovery is a literal-only regex with no dynamic-load rejection; a probe returned only config/test paths and an unchanged digest after modifying tests/helper.js. Smallest correction: use syntax-aware import analysis and fail closed on unresolved local loads, with direct/transitive regressions.

  4. [HIGH] pdd/sync_core/runner.py:1268 — Absolute launchers may still resolve a candidate-controlled Jest runner. Impact: /opt/homebrew/bin/npx jest passes validation and runs at cwd=root, so npx may select a candidate-modified tracked node_modules/.bin/jest, while identity binds only the launcher and literal jest. Evidence: pathless non-script operands fall through and a probe returned no command error. Smallest correction: require an absolute external digest-bound Jest entry point or resolve/bind launcher-selected tools outside the checkout; add a bare-launcher regression.

  5. [MEDIUM] pdd/sync_core/runner.py:1399 — Spawn failures are not normalized. Impact: a missing/non-executable accepted command raises out of validation instead of returning ERROR. Evidence: only timeout is caught; /definitely/missing/jest raised FileNotFoundError. Smallest correction: validate executability and catch OSError, with missing/non-executable tests.

Verification: exact head confirmed; git diff --check origin/main...HEAD passed. The focused run produced 59 passed / 4 failed: all 34 Jest tests passed, while four existing finalizer tests failed in protected pytest collection on this macOS host. GitHub has successful auto-heal/heal only, with no unit-test check.

A formal request-changes review was unavailable because the authenticated account owns the PR, so this is recorded as a review comment.

@gltanaka

Copy link
Copy Markdown
Contributor Author

Sol-high focused re-review — PR #1988 @ b56d80c8858a0a449f70dc648e76b68a1b54d7cd

VERIFIED / MERGE

Verified only the five original Jest findings and affected behavior:

  1. pdd/sync_core/runner.py:723 / :1852 — declared code_under_test_paths are excluded from the stable Jest support/config closure and are passed through preflight; the application-import/code-change regression passes.
  2. pdd/sync_core/runner.py:1276 / :1498 — Jest uses the foundation credential-free environment and supervised subprocess with read/write boundaries, timeout normalization, and surviving-descendant rejection; containment/capability regressions pass.
  3. pdd/sync_core/runner.py:643 — unresolved dynamic require/import loads fail closed, including transitive support, while comments and strings do not trigger false positives; regressions pass.
  4. pdd/sync_core/runner.py:1329 / :976 — launcher indirection and pathless entrypoints are rejected; accepted absolute external entrypoint/toolchain bytes are bound into runner identity; regressions pass.
  5. pdd/sync_core/runner.py:1517 — launch OSError is normalized to Jest ERROR, with missing and non-executable commands rejected during preflight; regressions pass.

Verification: exact local and PR head match b56d80c8858a0a449f70dc648e76b68a1b54d7cd; worktree clean; git diff --check origin/main...HEAD passed. The combined local run had all Jest/focused correction tests pass; its only four failures were the same unrelated protected-pytest finalizer collection failures on this macOS host. No remaining original finding.

@gltanaka gltanaka force-pushed the codex/issue-1932-jest-adapter branch from b56d80c to 7b90db4 Compare July 13, 2026 00:26
@gltanaka

Copy link
Copy Markdown
Contributor Author

Sol high rebase review — PR #1988 @ 7b90db4af262f0365ee3049d19a984024fea9bbb

Verdict: MERGE — no findings.

Reviewed read-only per docs/runbooks/pr-loop-process.md against merged denominator main at 5c86dd76f30d8319c075724e720745c48ee1d9a7.

Rebase and conflict-resolution evidence

  • PR/API head and local HEAD both equal 7b90db4af262f0365ee3049d19a984024fea9bbb.
  • PR base, local origin/main, and merge base all equal 5c86dd76f30d8319c075724e720745c48ee1d9a7.
  • Worktree is clean and fully pushed.
  • git range-diff 5ab254a..b56d80c 5c86dd76..7b90db4 reports all 13 commits as =: no patch changed, reordered, or dropped during rebase.
  • git diff --check origin/main...HEAD passed; the diff remains limited to the same four intended files.

Previously approved Jest findings

All remain fixed:

  • immutable support closure excludes declared application code;
  • Jest uses foundation-supervised, credential-free containment and rejects surviving descendants;
  • unresolved dynamic local require/import fails closed;
  • launcher indirection such as npx jest and pathless entrypoints are rejected, with accepted external tool bytes identity-bound;
  • launch OSError, missing, and non-executable commands normalize to ERROR.

Focused verification

  • pytest -q tests/test_sync_core_runner_jest.py tests/test_sync_core_reporting.py::test_validate_command_wires_protected_jest_runner_config46 passed.
  • Shared denominator checks for signer-capability containment, surviving descendants, and unsupported sandbox behavior → 1 passed, 2 host-appropriate skips.
  • Current completed CI checks are green; unit/public CLI/auto-heal/heal were still running at review time.

No remaining approved finding and no rebase regression.

@gltanaka

Copy link
Copy Markdown
Contributor Author

Sol high narrow ownership verification — PR #1988 @ 000b56e5c6de9948c8a2f5f36f7c86cecb4c73e1

MERGE — no findings.

Verified only the requested correction and previously approved Jest behavior:

  • .pdd/sync-ownership.json adds exactly one rule for tests/test_sync_core_runner_jest.py.
  • The rule is the intended narrow protected ownership classification: exact literal path, HUMAN_OWNED, human-maintained, owner pdd-maintainers.
  • The target pattern occurs once; the policy contains zero duplicate patterns, and this commit introduces no wildcard, directory-wide, or otherwise broader matching rule.
  • pdd/commands/sync_core.py, pdd/sync_core/runner.py, tests/test_sync_core_reporting.py, and tests/test_sync_core_runner_jest.py are unchanged from previously approved head 7b90db4af262f0365ee3049d19a984024fea9bbb; all previously approved Jest behavior therefore remains intact.
  • Rollout inventory completeness plus the full focused Jest suite passed: 46 passed.
  • git diff --check origin/main...HEAD passed; worktree is clean and fully pushed.
  • PR API and local HEAD both resolve the requested prefix to exact SHA 000b56e5c6de9948c8a2f5f36f7c86cecb4c73e1.

No remaining narrow ownership or Jest regression finding.

@gltanaka gltanaka force-pushed the codex/issue-1932-jest-adapter branch from 000b56e to 4b63167 Compare July 13, 2026 03:33
@gltanaka

Copy link
Copy Markdown
Contributor Author

Sol high protected-ownership rebase verification — PR #1988 @ 4b63167b0a62c454a6c1ea70c22239b920d1b5ec

MERGE — no findings.

Reviewed read-only against protected ownership base 393246f92f774d6c00af03a8022a271c5c11eefc.

Rebase and ownership integration

  • PR/local head both equal 4b63167b0a62c454a6c1ea70c22239b920d1b5ec; PR base, local origin/main, and merge base all equal 393246f92f774d6c00af03a8022a271c5c11eefc.
  • Range-diff shows the original 13 approved Jest commits unchanged (=).
  • The redundant feature-local ownership commit 000b56e5c was dropped.
  • .pdd/sync-ownership.json has no feature diff and is byte-identical to protected base.
  • Protected base supplies exactly one exact rule for tests/test_sync_core_runner_jest.py: HUMAN_OWNED, human-maintained, owner pdd-maintainers, preauthorize_absent: true.
  • There are no duplicate ownership patterns; inventory and expected-managed denominator remain intact.
  • The only new integration patch makes the protected-base absent-child transition fixture independent of whether that child path is already present after rebase; it removes the path in its temporary clone before exercising the same add transition.

Previously approved Jest behavior

All remains unchanged: application code exclusion from immutable support, supervised credential-free containment, dynamic local load fail-closed behavior, launcher/pathless entrypoint rejection and tool identity binding, and normalized launch errors.

Verification

  • Focused rollout + Jest command: 48 passed.
  • git diff --check origin/main...HEAD passed.
  • Worktree is clean and fully pushed.
  • Exact-head CI completed green for CodeQL, Story Regression, Repo Bloat Docker E2E, Package Preprocess Smoke, and all three language analyses. Unit tests, Public CLI Regression, auto-heal, and heal were still running at review time.

No rebase, ownership, denominator, or previously approved Jest regression finding.

@gltanaka gltanaka merged commit e37b0bf into main Jul 13, 2026
11 checks passed
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