Repository navigation
Implement durable browser runs, scoped testing, visual fallback, and crash recovery - #2
Conversation
…lback, and crash recovery
|
|
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: a128d880db
ℹ️ 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".
| if (!results.every(r => r.passed)) throw new BrowserError({ code: "assertion_failed", reason: "A checkpoint assertion failed. The UI will not be repaired to make it pass." }); | ||
| journal.append("verified_checkpoint", { index }); | ||
| } | ||
| await snap(); if (!rebuilding) await captureFields(page, journal); |
There was a problem hiding this comment.
When a valueFromEnv fill targets a normal text control that is also listed in recovery.fields, the earlier !s.valueFromEnv guard skips the intent write, but this unconditional post-action capture reads the filled value from the DOM and writes it to recovery.json. This silently persists the sensitive value that the environment reference was intended to keep out of run storage; reject that selector/scope overlap during validation or exclude environment-backed fields from recovery capture.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in #4. Task validation rejects recovery overlap with environment-backed controls in both ordinary and reconstruction steps, including equivalent default scopes. Capture-time DOM identity checks reject alternate CSS selectors, frame paths, and open-root aliases before reading recovery values; managed Jev also guards its capture path. Regression tests cover these cases and confirm unrelated recovery fields still work. The fix does not retroactively scrub existing recovery files. Leaving this thread open until the follow-up PR is reviewed/merged.
| const config = yield* io(host.configuration); | ||
| const profile = yield* io(() => fs.promises.realpath(config.profileDir)); | ||
| const { journal, duplicate } = yield* sync(() => createRun(host.home(), task, testing, profile)); | ||
| if (duplicate) return { ...summary(journal.state()), duplicate: true }; |
There was a problem hiding this comment.
If the first request uses requestId and host.connect() fails transiently, the run is saved with only preflight.failed; every retry with that request ID then returns here without reconnecting. Because the run has no retained target, ordinary resume is also unavailable, so that idempotency key is permanently stranded even though no browser action was dispatched. Duplicate handling should allow the existing never-started run to attempt preflight again rather than returning its stale blocked summary.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in #4. The same request ID retries preflight on its existing run only when every recorded event is creation or a preflight failure. An attempt, target, action, model call, or unknown event retains duplicate suppression, as do completed runs and conflicting inputs. The real-browser regression removes/restores the configured endpoint, retries the original ID, and confirms Save is dispatched exactly once. Leaving this thread open until the follow-up PR is reviewed/merged.
Address both follow-up findings on PR #2. Reject environment-backed recovery fields in normal/reconstruction plans and guard live selector aliases in Playwright and Jev capture. Retry an existing request only when its journal contains creation/preflight events and no execution boundary. Add unit and real-browser regressions, including frame/shadow aliases and failed preflight followed by exactly one Save. Harden disposable Chrome fixture readiness against missing/partial metadata and slow cold startup without changing production timeouts or retrying actions. Validated this exact tree with 51 Node and 14 Python tests plus all real-Chrome, managed Jev, scoped/MCP-image, crash-recovery, and controlled-provider Midscene suites in run 35547359928. Only the CI token's final workflow-changing push was denied; publish the tested tree through the authorized connector. Remove temporary validation workflow and source patch. No paid models, production accounts, dependency changes, or added documentation.
1 participant
Actual runtime implementation following merged design PR #1. Package version 0.2.0.
CLI and MCP now share durable runs, request deduplication, action receipts, retained evidence, extraction, cumulative budgets, and profile-scoped leases. The managed Jev worker uses the upstream policy on the exact registered task tab. Playwright handles explicit UI steps and scoped frame/open-shadow assertions; the real Midscene adapter provides configured visual workflows and conservative same-tab fallback. MCP screenshots carry image content directly.
Recovery validates surviving documents, supports explicitly declared UI reconstruction/restart, and requires predeclared UI reconciliation for uncertain submissions. Retained form inputs are allowlisted; missing or forbidden inputs remain blocked. Original crashes and failed attempts stay visible. Explicit reconstruction retires only the abandoned owned tab.
Added reviewed local recipe reuse, working examples, and updates to the existing three short skills and operational guides. No extra design-document hierarchy. Dependencies are locked with patched transitive overrides; normal CI is read-only and all Actions are commit-pinned. Temporary implementation workflows/payloads are absent from the final tree.
Validation includes 43 Node tests, 11 Python tests, legacy Chrome/CLI suites, managed upstream Jev, scoped assertions/native MCP images, real renderer/browser crash recovery, and real Midscene with controlled provider responses. Check the current CI result before merge.
Limits: no paid-model or production-account evaluation, no per-client Codex/Claude/OpenCode/Pi/OMP installation validation, no closed-root DOM access, lossless page restoration, or automatic popup following. Partial autonomous work requires explicit remaining-goal continuation; uncertain actions never silently replay.