Skip to content

fix(browser): recover no-op image prompt sends - #432

Closed
oraclexing wants to merge 1 commit into
steipete:mainfrom
oraclexing:fix/chatgpt-image-enter-fallback
Closed

fix(browser): recover no-op image prompt sends#432
oraclexing wants to merge 1 commit into
steipete:mainfrom
oraclexing:fix/chatgpt-image-enter-fallback

Conversation

@oraclexing

@oraclexing oraclexing commented Aug 28, 2026

Copy link
Copy Markdown

Summary

  • recover when ChatGPT leaves an attachment-bearing image prompt staged after a trusted send-button click
  • compare canonical composer whitespace before using a single Enter fallback, and suppress the fallback when a new turn or stop control is already visible
  • persist promptSubmitted only after the user turn is observable
  • document the signed-in image-generation regression smoke for browser changes

Root cause

On the current Windows ChatGPT home composer, an uploaded reference image could leave the visible enabled data-testid="send-button" as a no-op for CDP mouse input. Oracle already had enough evidence to see that the prompt stayed staged, but a raw-string comparison would miss it because ProseMirror renders paragraph boundaries with extra newlines. The run then waited the full prompt-commit timeout with zero conversation turns.

The fallback now canonicalizes whitespace, waits briefly for the click to commit, checks that no new turn or generation stop control appeared, and only then presses Enter once.

Tests

  • pnpm vitest run tests/browser/promptComposer.test.ts tests/browser/promptComposerExpressions.test.ts — 63 passed
  • pnpm run typecheck
  • pnpm test — 153 files passed, 18 skipped; 1871 tests passed, 53 skipped
  • pnpm run build

Signed-in browser smokes

Underlying browser-path reproduction and fallback evidence:

  • Session: chatgpt-image-fallback-smoke-two
  • GPT-5.6 Sol, Instant, one real PNG reference, attachment mode always
  • Observed log: Send click left the prompt staged; submitting once via Enter
  • Runtime: promptSubmitted=true, conversation URL changed from / to /c/...
  • Live DOM: empty composer plus the exact submitted user turn
  • Completed in 1m44s and downloaded two valid 1254x1254 PNG artifacts under ~/.oracle/generated/

Exact MCP wrapper smoke through a fresh stdio oracle-mcp.js process:

  • Tool: chatgpt_image
  • Session: chatgpt-image-mcp-exact-smoke
  • Runtime: promptSubmitted=true, conversation URL /c/6a91962b-9810-83ee-9f62-8d517a5bd878
  • Status: completed in 1m20s
  • structuredContent.images returned the requested output path
  • Saved PNG independently validated as 1254x1254, 817,223 bytes, with a valid PNG signature
  • Oracle browser slot released; retained Chrome/profile lock cleanup verified

@clawsweeper

clawsweeper Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added merge-risk: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Aug 28, 2026
@clawsweeper

clawsweeper Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge. Reviewed August 28, 2026, 10:14 AM ET / 14:14 UTC.

ClawSweeper review

What this changes

The branch retries a ChatGPT image-prompt send with Enter when the composer remains populated after a send-button click, and records submission state only after a user turn appears.

Merge readiness

Blocked by patch quality or review findings - 5 items remain

Keep open: the prior P1 delivery-race finding remains at the same head SHA. The new 750 ms Enter retry can send a second image prompt while a successful click is still committing.

Priority: P2
Reviewed head: c897736679973ebcb537d589a401fe29ff3ec591

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The supplied live proof demonstrates the no-op recovery, but the unresolved P1 duplicate-send race prevents the patch from being merge-ready.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (live_output): The PR body records after-fix signed-in browser and MCP runs with an observed fallback, submitted user turn, completed image output, and independently validated PNG artifacts.
Patch quality 🦪 silver shellfish (2/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Verified Sufficient (live_output): The PR body records after-fix signed-in browser and MCP runs with an observed fallback, submitted user turn, completed image output, and independently validated PNG artifacts.
Evidence reviewed 5 items Introduced retry race: The branch calls the Enter fallback immediately after every reported click; that helper waits only 750 ms before sending Enter whenever the original prompt is still visible and no turn or stop control is visible.
Current commit contract permits delayed success: The current-main verifier explicitly handles a send click that succeeds before its user turn appears and polls until the commit timeout. A slow but successful click can therefore meet the new helper’s 750 ms staged-prompt condition and receive a duplicate Enter.
Regression coverage is incomplete: The added test proves Enter is sent for a permanently staged composer, but it does not model an initially successful click whose turn/composer update occurs after the 750 ms retry delay.
Findings 1 actionable finding [P1] Wait for commit evidence before issuing the Enter retry
Security None None.

How this fits together

Oracle’s browser engine sends prompts through Chrome DevTools, then verifies that ChatGPT created a user turn before continuing the session. This change sits between the initial send click and commit verification, so it directly affects whether an image request is delivered once.

flowchart LR
A[Prompt and reference image] --> B[ChatGPT composer]
B --> C[Send-button click]
C --> D{Commit signal visible?}
D -->|Not yet| E[Enter fallback]
D -->|Yes| F[Commit verification]
E --> F
F --> G[Session submission state]
Loading

Before merge

  • Wait for commit evidence before issuing the Enter retry (P1) - The new helper runs after only 750 ms, but the existing verifier explicitly supports successful clicks whose user turn has not appeared yet. If that successful click still leaves the composer populated at this point, the staged-prompt condition holds and Enter sends the prompt a second time. The added test covers only a no-op click; add a delayed-success case and require evidence that the first click failed. This is the prior blocker still present at the same reviewed SHA.
  • Resolve merge risk (P1) - A slow successful send can remain visibly staged for more than 750 ms before ChatGPT exposes a turn or stop control; merging then causes Enter to submit the same image request twice.
  • Complete next step (P2) - The introduced P1 race has a narrow code-and-regression-test repair path on this branch.
  • Improve patch quality - Add a delayed-success regression case proving Enter is not dispatched after an initial click that commits late.
  • Improve patch quality - Update the fallback guard so it waits for evidence of click failure rather than treating a 750 ms unchanged composer as proof.

Findings

  • [P1] Wait for commit evidence before issuing the Enter retry — src/browser/actions/promptComposer.ts:239-245
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Changed surface 5 files; production +91 net lines, tests +66 net lines The delivery retry is concentrated in one browser action and has focused coverage, documentation, and release-note changes.

Merge-risk options

Maintainer options:

  1. Prove the first click failed before retrying (recommended)
    Replace the fixed-delay retry condition with a failure signal that cannot overlap a delayed successful click, and add coverage for that delayed-success case.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Make the Enter retry conditional on a post-click failure signal that cannot overlap delayed success, and add a regression test proving no second send occurs when the first click commits late.

Technical review

Best possible solution:

Only issue Enter after reliable evidence that the click failed, while preserving the delayed successful-click path so each image request creates one user turn.

Do we have a high-confidence way to reproduce the issue?

Yes, source-reproducible: the current verifier supports clicks whose turn appears later, while the introduced helper sends Enter after 750 ms if that turn has not yet appeared and the prompt remains visible.

Is this the best way to solve the issue?

No: the retry solves the reported no-op, but its fixed short wait does not distinguish failure from delayed successful commit; the guard must establish click failure before sending again.

Full review comments:

  • [P1] Wait for commit evidence before issuing the Enter retry — src/browser/actions/promptComposer.ts:239-245
    The new helper runs after only 750 ms, but the existing verifier explicitly supports successful clicks whose user turn has not appeared yet. If that successful click still leaves the composer populated at this point, the staged-prompt condition holds and Enter sends the prompt a second time. The added test covers only a no-op click; add a delayed-success case and require evidence that the first click failed. This is the prior blocker still present at the same reviewed SHA.
    Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

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

Labels

Label justifications:

  • P2: This is a bounded browser image-prompt delivery repair, but the proposed retry can affect real requests if merged.
  • merge-risk: 🚨 message-delivery: The introduced unconditional post-click retry can duplicate a prompt when the original click commits after the 750 ms observation window.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦞 diamond lobster and patch quality is 🦪 silver shellfish.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body records after-fix signed-in browser and MCP runs with an observed fallback, submitted user turn, completed image output, and independently validated PNG artifacts.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body records after-fix signed-in browser and MCP runs with an observed fallback, submitted user turn, completed image output, and independently validated PNG artifacts.

Evidence

Acceptance criteria:

  • [P1] pnpm vitest run tests/browser/promptComposer.test.ts tests/browser/promptComposerExpressions.test.ts.
  • [P1] pnpm run typecheck.
  • [P1] pnpm test.
  • [P1] pnpm run build.

What I checked:

  • Introduced retry race: The branch calls the Enter fallback immediately after every reported click; that helper waits only 750 ms before sending Enter whenever the original prompt is still visible and no turn or stop control is visible. (src/browser/actions/promptComposer.ts:239, c89773667997)
  • Current commit contract permits delayed success: The current-main verifier explicitly handles a send click that succeeds before its user turn appears and polls until the commit timeout. A slow but successful click can therefore meet the new helper’s 750 ms staged-prompt condition and receive a duplicate Enter. (src/browser/actions/promptComposer.ts:939, fa0e6fd669ad)
  • Regression coverage is incomplete: The added test proves Enter is sent for a permanently staged composer, but it does not model an initially successful click whose turn/composer update occurs after the 750 ms retry delay. (tests/browser/promptComposer.test.ts:299, c89773667997)
  • Prior finding remains unresolved: The supplied previous ClawSweeper review identified this same delayed-commit duplication risk at the identical reviewed head; no later contributor commit is present. (src/browser/actions/promptComposer.ts:239, c89773667997)
  • Feature-history routing: Prompt-commit diagnostics and delayed-turn handling date to the browser commit-verification work, while Peter Steinberger has recent attachment and Enter-fallback history in this path. (src/browser/actions/promptComposer.ts:830, 08c5587f58a7)

Likely related people:

  • LeoLin990405: Authored the current browser prompt-commit diagnostics and delayed-turn handling work. (role: commit-verification contributor; confidence: high; commits: 08c5587f58a7; files: src/browser/actions/promptComposer.ts)
  • Peter Steinberger: Has recent attachment-send and Enter-fallback history on the affected prompt submission path. (role: recent browser submission contributor; confidence: high; commits: fa0e6fd669ad, 0c8fc7ea31b0; files: src/browser/actions/promptComposer.ts, src/browser/constants.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 (1 earlier review cycle)
  • reviewed 2026-08-28T14:06:32.369Z sha c897736 :: needs changes before merge. :: [P1] Do not retry while a successful click may still be committing

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Aug 28, 2026
@steipete

Copy link
Copy Markdown
Owner

Thanks for the detailed reproduction and the signed-in browser/MCP smokes; that evidence is useful for understanding the no-op image send.

I'm closing this version because an unchanged composer after 750 ms cannot establish that the first click failed. A successful click can still be committing with no user turn or stop control visible, so the Enter fallback can submit the same image request twice. Main already improves activation and single-send targeting in #434 while keeping ambiguous commit timeouts out of retries.

A focused follow-up is welcome if the no-op still reproduces on current main. Please include delayed-success coverage and real browser evidence, and preserve one submission attempt unless there is reliable evidence that the first attempt failed. Thanks again for investigating this path.

@steipete steipete closed this Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants