[codex] Recover browser image artifact downloads#253
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 12, 2026, 3:57 AM ET / 07:57 UTC. Summary Reproducibility: unclear. The review failed before ClawSweeper could establish a reproduction path. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Review detailsBest possible solution: Retry the Codex review after fixing the execution failure. Do we have a high-confidence way to reproduce the issue? Unclear. The review failed before ClawSweeper could establish a reproduction path. Is this the best way to solve the issue? Unclear. Retry the review first so ClawSweeper can evaluate the actual issue and fix direction. AGENTS.md: unclear because the file could not be read completely. Codex review notes: model internal, reasoning high; reviewed against b1c0755507ed. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
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. How this review workflow works
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8bfe51f7d8
ℹ️ 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".
|
|
||
| function isAllowedChatGptHost(hostname: string): boolean { | ||
| const value = hostname.toLowerCase(); | ||
| return value === "chatgpt.com" || value === "chat.openai.com"; |
There was a problem hiding this comment.
Support chat.openai.com cookies before accepting its images
When a generated image is discovered on the legacy chat.openai.com host, this allow-list now lets it through validation, but saveChatGptGeneratedImages still builds cookies only for https://chatgpt.com/ and returns early on an empty cookie header before the new browser-context Runtime fallback can run. In a signed-in chat.openai.com session with no chatgpt.com cookies, image saving therefore fails with “Missing ChatGPT cookies” even though the authenticated tab could fetch the same-origin artifact.
Useful? React with 👍 / 👎.
Summary
Validation
pnpm exec vitest run tests/browser/chromeLifecycle.test.ts tests/browser/chatgptImages.test.ts tests/browser/pageActionsExpressions.test.tspnpm run buildpnpm testpnpm run checkpnpm run test:packed-cliautoreview --mode localLive proof
distCLI launched the default manual-login profile fresh; the run reached profile auth and failed because that private profile is not signed in.distCLI with the real signed-in Chrome profile forced Node-side ChatGPT image fetch to fail, retried through the authenticated browser context, and saved a valid PNG.2b5fc33b822bdf6acd9b7dad6c60009846bf130fa8080eaef1c5ef7aed1f6abf.Supersedes #252 only because pushing the maintainer fixup to the contributor fork was rejected by GitHub.