fix(browser): clear stale ChatGPT cookies before navigation#283
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 6, 2026, 12:41 AM ET / 04:41 UTC. Summary Reproducibility: no. high-confidence live reproduction is available in this review. Source inspection confirms current and merged-result ChatGPT navigation paths that sync or reuse cookies without clearing Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Centralize the narrow ChatGPT conversation-cookie cleanup for every ChatGPT path that syncs or reuses browser cookies before navigation, then add redacted live browser proof against real Do we have a high-confidence way to reproduce the issue? No high-confidence live reproduction is available in this review. Source inspection confirms current and merged-result ChatGPT navigation paths that sync or reuse cookies without clearing Is this the best way to solve the issue? No. The helper is narrow and plausibly useful, but this branch should cover the remaining ChatGPT cookie-navigation paths and show real browser proof before merge. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against c159bf863fdd. Label changesLabel 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
Review history (1 earlier review cycle)
|
clawsweeper
Bot
added
rating: 🧂 unranked krab
Co-authored-by: Dongwoo Jeong <dongwoo@duck.com>
steipete
force-pushed
the
codex/clear-chatgpt-conv-cookies
branch
from
a02eb53 to
753257d
Compare
steipete
changed the title
![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: 753257dd03
ℹ️ 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".
| .map((id) => `conv_key_${id}`), | ||
| ); | ||
| try { | ||
| const { targetInfos = [] } = await Target.getTargets(); |
There was a problem hiding this comment.
In the normal local and non-browser-WebSocket remote paths, the client passed here is attached to a page target (connectToNewTarget opens CDP({ ..., target: target.id })), and page-level CDP sessions do not support browser-level target enumeration reliably. When Target.getTargets() fails, the catch returns before Network.getAllCookies() runs, so copy-profile/manual-login/remote profiles keep the stale conv_key_* cookies and still hit the oversized-header failure this change is meant to prevent. Use the browser target or the existing CDP.List style discovery for open conversations, or fall back to deleting with only the explicit preserve list.
Useful? React with 👍 / 👎.
2 participants
Summary
about:blank, so stale cookies can be removed before the first ChatGPT request.conv_key_*cookies on ChatGPT domains.Why
Per-conversation keys can accumulate until ChatGPT requests exceed header limits. A public reproduction recorded 63
conv_key_*cookies causing HTTP 431, with service restored after deleting those volatile keys: https://github.com/ratacat/pro-cli/blob/f4dbaafe572e662cfd6d40e5c296566dd0a80e03/docs/chatgpt-431-cookie-bloat.mdThe original branch cleaned after the new target had already navigated and omitted reattach/Project Sources. It also deleted every conversation key, including keys for concurrent or resumed work. This revision fixes those ordering and ownership boundaries.
Verification
pnpm test: 1,415 passed, 43 skippedpnpm run checkpnpm run buildAuthenticated remote-Chrome proof seeded twelve stale 2.5 KB keys plus one small key for a synthetic open conversation (30,016 value bytes total). Oracle:
about:blank;No cookie values are logged. Thanks @jung0han for the original patch.