Skip to content

[codex] hydrate ChatGPT conversations before recovery capture - #345

Open
pdurlej wants to merge 2 commits into
steipete:mainfrom
pdurlej:codex/hydrate-reloaded-conversations
Open

[codex] hydrate ChatGPT conversations before recovery capture#345
pdurlej wants to merge 2 commits into
steipete:mainfrom
pdurlej:codex/hydrate-reloaded-conversations

Conversation

@pdurlej

@pdurlej pdurlej commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

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:

  • promptSubmitted and model-selection evidence can both be valid;
  • the URL can already identify the intended conversation;
  • the turn list is still empty;
  • a global Stop control may be visible;
  • capture eventually times out or harvest returns UI placeholder text instead of an answer.

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

  • Replace fixed post-navigation delays in local and remote delayed rechecks with the existing hydration gate.
  • Gate the one-time stall reload before restarting assistant capture.
  • Gate both live-target reattach and new-Chrome recovery before reading turn indexes.
  • Require a hydrated user turn before a visible Stop control can mark recovery as ready.
  • Add deterministic coverage for an empty shell that later hydrates, reattach ordering and fallback, and shell-only Stop controls.

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 passed
  • pnpm run check
  • pnpm run build
  • pnpm test - 1,606 passed, 43 skipped

Exact-head neutral browser smoke on macOS:

  • no files or private context;
  • current Pro picker requested and independently verified;
  • exact marker captured;
  • transcript artifact persisted;
  • browser slot released;
  • 19.0 seconds end to end.

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.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. labels Jul 30, 2026
@clawsweeper

clawsweeper Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed August 1, 2026, 7:22 PM ET / 23:22 UTC.

ClawSweeper review

What this changes

The 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
Reviewed head: 4492b33e0039e985bb877b9efc4b3b982bfea1a8

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The branch has useful deterministic coverage, but unchanged P1 recovery defects and path-mismatched live proof keep it below merge-ready quality.
Proof confidence 🦪 silver shellfish (2/6) Needs stronger real behavior proof before merge: The PR reports a successful neutral browser smoke, but the contributor explicitly says it does not exercise the empty-shell hydration followed by recovery capture path; provide redacted terminal output, runtime logs, or a recording of that path and redact private URLs, IPs, keys, phone numbers, and non-public endpoints. Updating the PR body should trigger a fresh review; if it does not, ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🦪 silver shellfish (2/6) 2 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: The PR reports a successful neutral browser smoke, but the contributor explicitly says it does not exercise the empty-shell hydration followed by recovery capture path; provide redacted terminal output, runtime logs, or a recording of that path and redact private URLs, IPs, keys, phone numbers, and non-public endpoints. Updating the PR body should trigger a fresh review; if it does not, ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 5 items Shared helper requires an enabled composer: Current main's hydration helper polls for stable prior turns, then always calls ensurePromptReady before returning. ensurePromptReady waits until a prompt input exists without a disabled attribute, which is not a safe precondition while an existing assistant answer is still generating.
PR routes live recovery through that submission-oriented helper: The branch replaces fixed delays in local and remote rechecks, stall reload, live reattach, and new-Chrome recovery with waitForResumedConversationHydration using requirePriorTurns, so each affected capture path inherits the enabled-composer wait.
Existing recovery behavior belongs to the current browser path: Blame attributes the existing retry-after-reload flow and the shared hydration helper to the current main lineage; the merged browser-worker recovery change is also present on main, making Peter Steinberger and Rokurolize relevant routing contacts.
Findings 2 actionable findings [P1] Keep live reattach capture independent of composer readiness
[P1] Do not gate retry capture on an idle composer
Security None None.

How this fits together

Oracle’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]
Loading

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: The PR reports a successful neutral browser smoke, but the contributor explicitly says it does not exercise the empty-shell hydration followed by recovery capture path; provide redacted terminal output, runtime logs, or a recording of that path and redact private URLs, IPs, keys, phone numbers, and non-public endpoints. Updating the PR body should trigger a fresh review; if it does not, ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Keep live reattach capture independent of composer readiness (P1) - This new call uses waitForResumedConversationHydration, which always ends with ensurePromptReady. That helper is correct before typing a follow-up, but ensurePromptReady waits for an enabled composer; ChatGPT can keep it disabled while the recovered assistant answer is still generating. As a result, live reattach waits until timeoutMs and falls into recovery instead of capturing the in-progress answer. This is the unchanged P1 blocker from the previous review at the same head; use a recovery-only stable-turn gate here.
  • Do not gate retry capture on an idle composer (P1) - The delayed recheck now calls a helper that requires the prompt input to be enabled after turns hydrate. These paths retry capture precisely when an existing assistant response may still be running, so the disabled composer makes the new gate time out and prevents the retry from reading that answer. Keep conversation-ID and stable-turn validation, but remove submission readiness from retry and reload capture. This remains the prior P1 blocker at the unchanged head.
  • Resolve merge risk (P2) - Merging as written can turn a recoverable in-progress ChatGPT answer into a timeout: the recovery gate waits for an enabled composer even though active generation commonly leaves that composer disabled.
  • Resolve merge risk (P1) - The branch changes five recovery entry points, so the same readiness mismatch affects local and remote delayed rechecks, reload retries, live reattach, and new-Chrome recovery.
  • Resolve merge risk (P1) - The supplied browser smoke establishes a normal exact-head run but does not show the empty-shell-to-hydration-to-recovery-capture sequence; the contributor explicitly confirms that limit.
  • Complete next step (P2) - Two concrete, unchanged P1 defects have a narrow mechanical repair: retain hydration and conversation-ID checks while removing the submission-only enabled-composer requirement from recovery capture.
  • Improve patch quality - Replace the shared submission hydration call with a recovery-safe stable-turn check that does not await an enabled composer.
  • Improve patch quality - Add focused tests where prior turns hydrate while the composer stays disabled because an assistant answer is in progress.
  • Improve patch quality - Post redacted real-browser diagnostics or a recording showing the repaired recovery sequence after the fix.

Findings

  • [P1] Keep live reattach capture independent of composer readiness — src/browser/reattach.ts:185-188
  • [P1] Do not gate retry capture on an idle composer — src/browser/index.ts:1827-1832
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Recovery entry points 5 paths changed Local and remote rechecks, reload retry, live reattach, and new-Chrome recovery all receive the same shared readiness behavior.
Patch scope 134 added, 7 removed across 7 files Most of the patch is focused browser recovery coverage, but the shared readiness assumption has broad runtime reach.

Merge-risk options

Maintainer options:

  1. Use a recovery-only hydration gate (recommended)
    Keep stable-turn and conversation-ID validation, but omit prompt-composer readiness from paths that are resuming capture of an existing answer.
  2. Pause the recovery change
    Do not merge this branch if the browser contract intentionally requires an idle composer before any reattach capture, because that would leave active-answer recovery unsupported.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Separate recovery hydration from prompt submission readiness, add disabled-composer regression coverage for reattach and retry capture, and retain the existing expected-conversation validation.

Technical review

Best 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:

  • [P1] Keep live reattach capture independent of composer readiness — src/browser/reattach.ts:185-188
    This new call uses waitForResumedConversationHydration, which always ends with ensurePromptReady. That helper is correct before typing a follow-up, but ensurePromptReady waits for an enabled composer; ChatGPT can keep it disabled while the recovered assistant answer is still generating. As a result, live reattach waits until timeoutMs and falls into recovery instead of capturing the in-progress answer. This is the unchanged P1 blocker from the previous review at the same head; use a recovery-only stable-turn gate here.
    Confidence: 0.98
  • [P1] Do not gate retry capture on an idle composer — src/browser/index.ts:1827-1832
    The delayed recheck now calls a helper that requires the prompt input to be enabled after turns hydrate. These paths retry capture precisely when an existing assistant response may still be running, so the disabled composer makes the new gate time out and prevents the retry from reading that answer. Keep conversation-ID and stable-turn validation, but remove submission readiness from retry and reload capture. This remains the prior P1 blocker at the unchanged head.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 68b8c51b0ee0.

Labels

Label justifications:

  • P2: This is a bounded browser response-capture reliability regression with a clear repair path, not an emergency-wide outage.
  • merge-risk: 🚨 availability: The new gate can make an already-running ChatGPT response unrecoverable by waiting for the disabled composer to become ready.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR reports a successful neutral browser smoke, but the contributor explicitly says it does not exercise the empty-shell hydration followed by recovery capture path; provide redacted terminal output, runtime logs, or a recording of that path and redact private URLs, IPs, keys, phone numbers, and non-public endpoints. Updating the PR body should trigger a fresh review; if it does not, ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

Acceptance criteria:

  • [P1] pnpm exec vitest run tests/browser/pageActions.test.ts tests/browser/index.test.ts tests/browser/reattach.test.ts tests/browser/recoverConversation.test.ts.
  • [P1] pnpm run check.
  • [P1] pnpm run build.
  • [P1] Run a redacted real browser recovery trace or recording that shows a saved conversation hydrate while the assistant is still generating, then confirms Oracle captures the answer without waiting for an idle composer.

What I checked:

  • Shared helper requires an enabled composer: Current main's hydration helper polls for stable prior turns, then always calls ensurePromptReady before returning. ensurePromptReady waits until a prompt input exists without a disabled attribute, which is not a safe precondition while an existing assistant answer is still generating. (src/browser/actions/navigation.ts:659, 68b8c51b0ee0)
  • PR routes live recovery through that submission-oriented helper: The branch replaces fixed delays in local and remote rechecks, stall reload, live reattach, and new-Chrome recovery with waitForResumedConversationHydration using requirePriorTurns, so each affected capture path inherits the enabled-composer wait. (src/browser/index.ts:1827, 4492b33e0039)
  • Existing recovery behavior belongs to the current browser path: Blame attributes the existing retry-after-reload flow and the shared hydration helper to the current main lineage; the merged browser-worker recovery change is also present on main, making Peter Steinberger and Rokurolize relevant routing contacts. (src/browser/index.ts:3948, 3f9db037ef6d)
  • Prior P1 findings remain at the identical PR head: The previous completed review named the same composer-readiness and retry-capture blockers at 4492b33, which is still the current PR head; no author commit has addressed them. (src/browser/reattach.ts:185, 4492b33e0039)
  • No release or current-main inclusion: The PR head is not contained by current main or a local release tag; this remains an unmerged change rather than an implemented-on-main fix. (38f4acaf18b4)

Likely related people:

  • steipete: Current-main blame attributes the shared hydration helper and the existing reload-and-retry capture flow to Peter Steinberger’s current lineage. (role: feature owner; confidence: high; commits: 5daa6ce8c352; files: src/browser/actions/navigation.ts, src/browser/index.ts, src/browser/reattach.ts)
  • Rokurolize: The merged browser-recovery worker work is the adjacent current-main change that made reattach and recovery capture a central browser-runtime path. (role: recent area contributor; confidence: medium; commits: 3f9db037ef6d; files: src/browser/index.ts, src/browser/reattach.ts)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (17 earlier review cycles; latest 8 shown)
  • reviewed 2026-07-31T20:14:18.803Z sha 4492b33 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-31T21:40:36.895Z sha 4492b33 :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-01T13:53:15.380Z sha 4492b33 :: needs real behavior proof before merge. :: [P1] Avoid requiring an idle composer before capturing a reattached answer
  • reviewed 2026-08-01T15:48:59.978Z sha 4492b33 :: needs real behavior proof before merge. :: [P1] Avoid waiting for an idle composer before reattach capture
  • reviewed 2026-08-01T17:03:16.933Z sha 4492b33 :: needs real behavior proof before merge. :: [P1] Avoid requiring an idle composer before reattach capture
  • reviewed 2026-08-01T18:18:52.566Z sha 4492b33 :: needs real behavior proof before merge. :: [P1] Avoid requiring an idle composer before reattach capture
  • reviewed 2026-08-01T19:46:40.805Z sha 4492b33 :: needs real behavior proof before merge. :: [P1] Avoid requiring an idle composer before recovery capture | [P1] Keep reload retries independent of composer readiness
  • reviewed 2026-08-01T22:12:31.967Z sha 4492b33 :: needs real behavior proof before merge. :: [P1] Do not require an idle composer during reattach | [P1] Keep retry capture independent of composer readiness

Co-authored-by: Codex <codex@openai.com>
@pdurlej

pdurlej commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

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.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant