Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the chatgpt-web executor, which integrates chatgpt.com's web client endpoints into the system. It implements session token exchange, browser-like warmup, Sentinel requirements (including proof-of-work solving), and support for image generation and editing via WebSockets. The review feedback highlights several critical runtime issues across multiple files where required functions like extractContent, randomUUID, and randomBytes are either not exported or not imported from node:crypto, which would lead to ReferenceErrors.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| return out; | ||
| } | ||
|
|
||
| async function* extractContent( |
| } from "../../services/chatgptImageCache.ts"; | ||
|
|
||
| import { ImagePointerRef, ImageResolver, imageMarkdown, resolveImagePointers } from "./images.ts"; | ||
| import { sseChunk } from "./sse.ts"; |
| import crypto from "node:crypto"; | ||
| import { createHash } from "node:crypto"; |
There was a problem hiding this comment.
The randomUUID function is used on lines 173 and 304 but is not imported from node:crypto. Please import it to avoid a runtime ReferenceError.
| import crypto from "node:crypto"; | |
| import { createHash } from "node:crypto"; | |
| import crypto from "node:crypto"; | |
| import { createHash, randomUUID } from "node:crypto"; |
| import crypto from "node:crypto"; | ||
| import { createHash } from "node:crypto"; |
There was a problem hiding this comment.
The randomUUID function is used on lines 225, 236, and 258 but is not imported from node:crypto. Please import it to avoid a runtime ReferenceError.
| import crypto from "node:crypto"; | |
| import { createHash } from "node:crypto"; | |
| import crypto from "node:crypto"; | |
| import { createHash, randomUUID } from "node:crypto"; |
| import crypto from "node:crypto"; | ||
| import { createHash } from "node:crypto"; |
There was a problem hiding this comment.
The randomBytes (used on line 184) and randomUUID (used on line 254) functions are not imported from node:crypto. Please import them to avoid runtime ReferenceErrors.
| import crypto from "node:crypto"; | |
| import { createHash } from "node:crypto"; | |
| import crypto from "node:crypto"; | |
| import { createHash, randomBytes, randomUUID } from "node:crypto"; |
|
Thanks @oyi77 — the chatgpt-web move looks structurally right ( |
ba5e277 to
21900c2
Compare
…orts Address review feedback on diegosouzapw#3986: - sse.ts: export extractContent (used by responses.ts) - responses.ts: import extractContent from ./sse.ts - executor.ts: add { randomUUID } named import from node:crypto - messages.ts: add { randomUUID } named import from node:crypto - sentinel.ts: add { randomBytes, randomUUID } named imports from node:crypto
…orts Address review feedback on diegosouzapw#3986: - sse.ts: export extractContent (used by responses.ts) - responses.ts: import extractContent from ./sse.ts - executor.ts: add { randomUUID } named import from node:crypto - messages.ts: add { randomUUID } named import from node:crypto - sentinel.ts: add { randomBytes, randomUUID } named imports from node:crypto
…orts Address review feedback on diegosouzapw#3986: - sse.ts: export extractContent (used by responses.ts) - responses.ts: import extractContent from ./sse.ts - executor.ts: add { randomUUID } named import from node:crypto - messages.ts: add { randomUUID } named import from node:crypto - sentinel.ts: add { randomBytes, randomUUID } named imports from node:crypto
Added exports for functions imported by executor.ts and other modules: - images.ts: resolveImagePointers, pollForAsyncImage - responses.ts: buildNonStreamingResponse - session.ts: exchangeSession - sentinel.ts: prepareChatRequirements, fetchDpl, solvePow, buildPrepareToken, solveProofOfWork - thinking.ts: setUserThinkingEffort - warmup.ts: runSessionWarmup Fixes CI known-symbols gate failure.
|
Thanks for all the modularization work here, @oyi77 🙏. We've decided to hold the per-module "non-stacked" refactors and run the decomposition as one coordinated pass after the in-flight quality-gate work lands, instead of merging them piecemeal. Reason: on the two we did merge (#3993, #3988) we caught logic being dropped during the move — and the gates (provider-consistency / typecheck) don't detect internal-logic loss — so each of these needs a full lossless audit, which isn't tractable across many overlapping PRs against a moving release branch right now. The coordinated modularization is tracked in #3501 / #3594; we'd genuinely value your input on that plan once it's up. Closing for now — purely sequencing, not a reflection on the effort. |
…nto focused modules (diegosouzapw#3986) Split the 2864-line monolith into 12 focused modules: - constants.ts, executor.ts, images.ts, messages.ts, responses.ts - sentinel.ts, session.ts, sse.ts, thinking.ts, utils.ts, warmup.ts - index.ts (barrel re-export) The original chatgpt-web.ts is now a thin re-export.
|
Rebased onto upstream/release/v3.8.31. Monolith chatgpt-web.ts (2863→1 line thin re-export) split into 12 focused modules. |
Replaces #3790 as an independent, non-stacked PR branched from release/v3.8.27.
Extracts chatgpt-web handler into a modular structure.
Supersedes: #3790