Fix recovery secret retention and retry never-started requests - #4
Conversation
Reject environment-backed controls in recovery allowlists during normal/reconstruction task validation. Guard live selector aliases before capture in both Playwright and managed Jev. Preserve unrelated allowlisted fields without storing the protected values. Retry the same request ID after preflight failure only when the journal proves execution never started. Keep completed, partial, uncertain, and unknown-event requests deduplicated. Add unit/browser regressions including failed preflight followed by exactly one Save. Build on the merged Chrome readiness fix without replacing it. Allow 30 seconds for cold startup in the shared Node fixture. No production timeout, dependency, skill, documentation, or Action pin changes.
|
Review changes with Changed Files
|
|
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67b3a4949e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Address PR #4 review comment 4058589871. Resolve protected selectors with document.querySelectorAll and compare actual DOM node identity instead of Element.matches, whose :scope root differs. Extend the managed-Jev browser regression across normal/reconstruction plans, :scope descendants and child selectors, selector lists, and multiple matches with a decoy first. Assert protected values are never read or retained and unrelated recovery fields still work. Locally reproduced disk retention with the original capture function in Chromium, then passed all 10 protected cases and the unrelated-field control with the fix. Python syntax checks pass. Full upstream and repository validation remains in CI. No new docs, dependencies, or workflow changes.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd8461ae21
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30e5bf02f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4aa5d52c6f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
@codex review |
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5cb9115353
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a7a48907f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Exercise environment-array exposure in page callbacks, future and nested iframe scopes, sensitive value getters, and existing fail-closed expectations through the real page worker. Use only local fixtures and synthetic environment values.
Keep environment-value comparisons in Node instead of passing secrets into page-realm evaluations. Reject protected identities and sensitive controls before reading value getters, retaining Playwright's atomic shadow-aware protection. Resolve only currently present recovery/protected frame scopes without waits; preserve strict malformed/ambiguous/action/check handling. Skip capture entirely without recovery fields, and re-evaluate protection when later frames appear.
Apply the recovery capture fixes and regression coverage from stacked PR #5 (source commit 50889f2) directly to PR #4. - Keep environment-secret comparisons in Node, outside page evaluations. - Skip absent future frame scopes during passive capture and bypass empty recovery allowlists without relaxing required action or assertion scopes. - Reject protected identities and sensitive controls before reading values, preserving pinned-node and shadow-aware checks. - Cache protected frame traversal within each recovery snapshot. - Include 21 real-Chrome regressions in the browser command and CI. Addresses Codex review comments 4061550418, 4061550427, and 4061578096.
Assert that eight recovery fields and twenty protected steps over two frame paths require only three path lookups per snapshot, and that the next snapshot resolves afresh. Keep the legacy visible=false result for an absent top-level control and cover it alongside strict missing-frame checks.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b410efc03b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Persist a recovery-value capture suspension before an environment-backed UI dispatch. Current selectors and exact value equality cannot prove a transformed or rerendered credential became public. Preserve prior snapshots and explicit non-secret input intents, while preventing later automatic observations from replacing them. Honor the same durable boundary in managed Jev, including across reattachment, reconstruction, missing environment values and legacy Playwright receipts without provenance. Keep protected-alias preflight checks active in the scoped adapter. Add eight real-Chrome formatter/mirror/reconstruction tests, cross-language handoff assertions, and four Python policy tests. No new dependencies or documentation. Addresses PR #4 comment 4072551621.
Restore the zero-match visible=false case without changing ambiguity or unsupported-scope handling. Extend the existing browser suite with scoped absence, legacy parity, hidden/ambiguous controls and disappearance polling. Defer transformed-secret tracking across replacement controls as a known prototype limitation. Document the risk and require recovery allowlists to remain non-sensitive; do not add taint tracking or blanket capture shutdown. Addresses review comment 4072551608; documents the scope decision for 4072551621. No new dependencies or runtime abstractions.
Adopt the existing, CI-validated conservative safeguard from PR #5 head 1438b9c, preserving this branch's added visibility regressions and documenting the recovery tradeoff. Persist a capture-suspended boundary before environment-backed dispatch; honor it across resume, reconstruction, Jev and Midscene. Prior snapshots and explicit non-secret Playwright fill intents remain usable. Do not try to track transformed secrets through node replacements or page formatters. This supersedes the initial deferral in cb060b2: the concurrent PR supplies a small fail-closed policy rather than a taint-tracking subsystem. Include its browser/Python regressions and keep dependencies unchanged. Addresses Codex review comment 4072551621.
There was a problem hiding this comment.
💡 Codex Review
fastest-e2e/worker/jev_runner.py
Line 108 in afa9239
Reject environment values before Jev persists recovery fields
When an autonomous goal precedes a later valueFromEnv step and an unrelated allowlisted control currently contains the same value as that environment variable, managed-Jev treats it as an ordinary observed field and writes the secret to recovery.json; unlike the Playwright capture path, this code checks only protected-node identity and never compares the returned value against environment-backed inputs. Compare result["value"] with the referenced environment values in Python before calling save_field, so the secrets do not need to enter page evaluation arguments.
AGENTS.md reference: AGENTS.md:L9-L9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Fix all open Codex recovery-capture findings on PR #4
1 participant
Addresses both follow-up review findings on merged PR #2 with runtime changes and regression tests.
Built on the separately merged Chrome-startup fix; its fixtures and tests remain intact. The shared Node fixture now allows a bounded 30-second cold start without relaunching Chrome or retrying browser actions.
Validation: PR CI run 35547743903 passed on
67b3a4949e26fdf70b5d3c6f83ef32a526901830: dependency audit, typecheck/unit/protocol tests, legacy browser, public CLI lifecycle, managed Jev, scoped assertions/native MCP images, real crash recovery, Midscene with a controlled provider, and the new retention/preflight browser regressions. The new unit tests reproduced both original failures before the fixes.Both original review threads have replies linking to this PR. No paid-model calls or production accounts. No new docs, skills, dependencies, or changed Action pins. Existing recovery files are not automatically scrubbed; this prevents the reviewed capture path rather than promising general secret redaction.