[codex] hydrate ChatGPT conversations before recovery capture - #345
[codex] hydrate ChatGPT conversations before recovery capture#345pdurlej wants to merge 2 commits into
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed August 1, 2026, 7:22 PM ET / 23:22 UTC. ClawSweeper reviewWhat this changesThe branch replaces fixed browser-recovery delays with a saved-conversation hydration check before Oracle captures a reattached or reloaded ChatGPT response, and rejects shell-only Stop controls as readiness evidence. Merge readiness⛔ Blocked until stronger real behavior proof is added - 10 items remain This PR still needs repair before merge. It correctly identifies the empty-shell hydration race, but it reuses a helper designed for submitting a new prompt; that helper waits for an enabled composer, so reattach and retry capture can time out while ChatGPT is actively generating and the composer is disabled. The same two P1 blockers were already reported at the unchanged head, and the contributor’s neutral smoke does not exercise this recovery path. Priority: P2 Review scores
Verification
How this fits togetherOracle’s browser runner reconnects to a saved ChatGPT conversation after a stall, reload, or session reattach, then captures the assistant reply into its persisted session result. This change inserts a readiness gate between reopening the conversation and reading its response turns. flowchart LR
A[Saved ChatGPT conversation] --> B[Reload or reattach]
B --> C[Recovery hydration check]
C --> D[Read rendered conversation turns]
D --> E[Capture assistant response]
E --> F[Persist Oracle session output]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Split turn hydration from composer readiness: recovery should require stable prior turns and the expected conversation identity, but it must proceed to response capture while the existing answer is still generating; add regression coverage for a hydrated conversation with a disabled composer and real redacted runtime proof of that path. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible: provide stable prior turns while the existing answer is still generating and the prompt input remains disabled, then invoke either changed recovery path. The shared helper waits for that input to become enabled before capture can resume, so a focused regression test can establish the failure without a live ChatGPT run. Is this the best way to solve the issue? No. Reusing the submission-oriented hydration helper is not the best solution because its final composer-readiness check protects typing a new prompt, while recovery must capture an already-running answer. A recovery-only turn-hydration check is the narrower compatible design. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 68b8c51b0ee0. LabelsLabel justifications:
EvidenceAcceptance criteria:
What I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (17 earlier review cycles; latest 8 shown)
|
Co-authored-by: Codex <codex@openai.com>
|
We are consciously stopping further contributor-side work on this PR. The integrated build works locally, the deterministic recovery coverage and CI are green, and the exact-head neutral browser smoke completed successfully. ClawSweeper found no actionable code or security defects. We also agree that the neutral smoke does not directly force the rare empty-shell -> hydration -> recovery-capture sequence, so we are not presenting it as proof of that specific live path. We do not plan to spend additional browser runs trying to manufacture that condition. If a maintainer or contributor wants to take over, exercise the targeted reload/reattach path, or add redacted real-browser evidence, please do. The branch and tests are available for that work, and maintainers can also decide whether the deterministic evidence is sufficient. |
Summary
Wait for a saved ChatGPT conversation to hydrate its prior turns before Oracle restarts response capture after a reload or reattach.
This prevents the browser runner from treating a correctly routed but still-empty ChatGPT SPA shell as the conversation. Recovery also no longer treats a shell-level Stop control as sufficient evidence when no user turn has rendered.
Problem
ChatGPT can return from navigation with the durable
/c/<id>URL and application shell visible before conversation turns hydrate. Oracle currently waits a fixed one second after stall/recheck navigation and begins capture immediately. The same race exists in live reattach and the new-Chrome recovery path.In that state:
promptSubmittedand model-selection evidence can both be valid;Root cause
The resumed-conversation submission path already has
waitForResumedConversationHydration, which waits for stable prior turns, confirms the composer has settled, and verifies the expected conversation id. Stall reload and reattach paths bypassed that gate and used either a fixed delay or no hydration wait.Fix
No new selectors, endpoints, authentication behavior, or browser-profile access are introduced.
Validation
pnpm exec vitest run tests/browser/index.test.ts tests/browser/reattach.test.ts tests/browser/recoverConversation.test.ts tests/browser/recoverConversationFlow.test.ts- 94 passedpnpm run checkpnpm run buildpnpm test- 1,606 passed, 43 skippedExact-head neutral browser smoke on macOS:
Exact head:
38f4acaf18b4d4faf785371ece301c467628f5f1.This complements #339: that change preserves the worker after foreground CLI loss, while this change prevents recovery capture from starting against a still-unhydrated conversation shell.