Conversation
There was a problem hiding this comment.
Code Review
This pull request implements a major refactoring and modularization of the chatCore and imageGeneration handlers, soft-deprecates the @omniroute/opencode-provider package, migrates public OAuth client IDs to secure credential resolution, and resolves several routing, API, and documentation issues. The code review identified critical runtime ReferenceError bugs in the newly added frontend files (connectionManagement.tsx and utils.tsx) due to missing imports of constants and utility functions (such as ERROR_TYPE_LABELS, formatTimeAgo, and CODEX_REASONING_STRENGTH_OPTIONS). Additionally, several high-severity defensive programming issues were highlighted in the modularized image generation handlers where API JSON responses are accessed directly without null or object checks, posing a risk of runtime TypeErrors.
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.
| : t("leaveBlankKeepCurrentApiKey"); | ||
| const codexAccountServiceTierOptions = useMemo( | ||
| () => | ||
| CODEX_ACCOUNT_SERVICE_TIER_VALUES.map((value) => ({ |
| isAnthropicCompatibleProvider(connection.provider); | ||
| const testErrorMeta = | ||
| !testResult?.valid && testResult?.diagnosis?.type | ||
| ? ERROR_TYPE_LABELS[testResult.diagnosis.type] || null |
| label={t("excludedModelsLabel")} | ||
| value={formData.excludedModels} | ||
| onChange={(e) => setFormData({ ...formData, excludedModels: e.target.value })} | ||
| placeholder={t("excludedModelsPlaceholder")} |
There was a problem hiding this comment.
| <Select | ||
| label={t("defaultThinkingStrengthLabel")} | ||
| value={formData.codexReasoningEffort} | ||
| options={CODEX_REASONING_STRENGTH_OPTIONS} |
| title={statusLabel} | ||
| > | ||
| {health.failures}x | ||
| {health.lastFailure ? ` · ${formatTimeAgo(health.lastFailure)}` : ""} |
| if (status.status === "completed" || status.status === "succeeded") { | ||
| const imgUrl = status.creation_url || status.output?.image_url; | ||
| if (imgUrl) { | ||
| const imgRes = await fetch(imgUrl); | ||
| if (!imgRes.ok) { | ||
| return { | ||
| success: false, | ||
| status: imgRes.status, | ||
| error: `Failed to download image: ${imgRes.status}`, | ||
| }; | ||
| } | ||
| const buf = await imgRes.arrayBuffer(); | ||
| saveCallLog({ | ||
| method: "POST", | ||
| path: "/v1/images/generations", | ||
| status: 200, | ||
| model: `${provider}/${model}`, | ||
| provider, | ||
| duration: Date.now() - startTime, | ||
| }).catch(() => {}); | ||
| return { | ||
| success: true, | ||
| data: { | ||
| created: Math.floor(Date.now() / 1000), | ||
| data: [{ b64_json: Buffer.from(buf).toString("base64") }], | ||
| }, | ||
| }; | ||
| } | ||
| } | ||
| if (status.status === "failed") { | ||
| saveCallLog({ | ||
| method: "POST", | ||
| path: "/v1/images/generations", | ||
| status: 502, | ||
| model: `${provider}/${model}`, | ||
| provider, | ||
| duration: Date.now() - startTime, | ||
| error: "Haiper image generation failed", | ||
| }).catch(() => {}); | ||
| return { success: false, status: 502, error: "Haiper image generation failed" }; | ||
| } |
There was a problem hiding this comment.
Defensive programming: status returned from statusRes.json() can be null or not an object if the API response is empty or malformed. Accessing status.status directly without a null/object check can cause a runtime TypeError.
if (status && typeof status === "object") {
if (status.status === "completed" || status.status === "succeeded") {
const imgUrl = status.creation_url || status.output?.image_url;
if (imgUrl) {
const imgRes = await fetch(imgUrl);
if (!imgRes.ok) {
return {
success: false,
status: imgRes.status,
error: `Failed to download image: ${imgRes.status}`,
};
}
const buf = await imgRes.arrayBuffer();
saveCallLog({
method: "POST",
path: "/v1/images/generations",
status: 200,
model: `${provider}/${model}`,
provider,
duration: Date.now() - startTime,
}).catch(() => {});
return {
success: true,
data: {
created: Math.floor(Date.now() / 1000),
data: [{ b64_json: Buffer.from(buf).toString("base64") }],
},
};
}
}
if (status.status === "failed") {
saveCallLog({
method: "POST",
path: "/v1/images/generations",
status: 502,
model: `${provider}/${model}`,
provider,
duration: Date.now() - startTime,
error: "Haiper image generation failed",
}).catch(() => {});
return { success: false, status: 502, error: "Haiper image generation failed" };
}
}| const gen = status.generations_by_pk || status; | ||
| if (gen.status === "COMPLETE") { | ||
| const imgUrl = gen.generated_images?.[0]?.url; | ||
| if (imgUrl) { | ||
| const imgRes = await fetch(imgUrl); | ||
| if (!imgRes.ok) { | ||
| return { | ||
| success: false, | ||
| status: imgRes.status, | ||
| error: `Failed to download image: ${imgRes.status}`, | ||
| }; | ||
| } | ||
| const buf = await imgRes.arrayBuffer(); | ||
| saveCallLog({ | ||
| method: "POST", | ||
| path: "/v1/images/generations", | ||
| status: 200, | ||
| model: `${provider}/${model}`, | ||
| provider, | ||
| duration: Date.now() - startTime, | ||
| }).catch(() => {}); | ||
| return { | ||
| success: true, | ||
| data: { | ||
| created: Math.floor(Date.now() / 1000), | ||
| data: [{ b64_json: Buffer.from(buf).toString("base64") }], | ||
| }, | ||
| }; | ||
| } | ||
| } | ||
| if (gen.status === "FAILED") { | ||
| saveCallLog({ | ||
| method: "POST", | ||
| path: "/v1/images/generations", | ||
| status: 502, | ||
| model: `${provider}/${model}`, | ||
| provider, | ||
| duration: Date.now() - startTime, | ||
| error: "Leonardo image generation failed", | ||
| }).catch(() => {}); | ||
| return { success: false, status: 502, error: "Leonardo image generation failed" }; | ||
| } |
There was a problem hiding this comment.
Defensive programming: status returned from statusRes.json() can be null or not an object, which would make gen null and cause a runtime TypeError when accessing gen.status. Please add a null/object check.
const gen = status && typeof status === "object" ? (status.generations_by_pk || status) : null;
if (gen && typeof gen === "object") {
if (gen.status === "COMPLETE") {
const imgUrl = gen.generated_images?.[0]?.url;
if (imgUrl) {
const imgRes = await fetch(imgUrl);
if (!imgRes.ok) {
return {
success: false,
status: imgRes.status,
error: `Failed to download image: ${imgRes.status}`,
};
}
const buf = await imgRes.arrayBuffer();
saveCallLog({
method: "POST",
path: "/v1/images/generations",
status: 200,
model: `${provider}/${model}`,
provider,
duration: Date.now() - startTime,
}).catch(() => {});
return {
success: true,
data: {
created: Math.floor(Date.now() / 1000),
data: [{ b64_json: Buffer.from(buf).toString("base64") }],
},
};
}
}
if (gen.status === "FAILED") {
saveCallLog({
method: "POST",
path: "/v1/images/generations",
status: 502,
model: `${provider}/${model}`,
provider,
duration: Date.now() - startTime,
error: "Leonardo image generation failed",
}).catch(() => {});
return { success: false, status: 502, error: "Leonardo image generation failed" };
}
}| const data = await res.json(); | ||
| if (data.data && data.data.length > 0) { |
There was a problem hiding this comment.
Defensive programming: data returned from res.json() can be null or not an object. Accessing data.data directly without a null/object check can cause a runtime TypeError.
| const data = await res.json(); | |
| if (data.data && data.data.length > 0) { | |
| const data = await res.json(); | |
| if (data && typeof data === "object" && Array.isArray(data.data) && data.data.length > 0) { |
| if (Array.isArray(data.images)) { | ||
| images.push( | ||
| ...data.images.map((img: Record<string, unknown>) => ({ | ||
| b64_json: img.image ?? img.b64_json ?? img.url ?? img, | ||
| revised_prompt: body.prompt, | ||
| })) | ||
| ); | ||
| } else if (Array.isArray(data.data)) { | ||
| images.push(...data.data); | ||
| } else if (data.url || data.b64_json || data.image) { | ||
| images.push({ | ||
| b64_json: data.image || data.b64_json || data.url, | ||
| url: data.url, | ||
| revised_prompt: body.prompt, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Defensive programming: data returned from response.json() can be null or not an object. Accessing data.images, data.data, data.url, etc. directly without a null/object check can cause a runtime TypeError.
if (data && typeof data === "object") {
if (Array.isArray(data.images)) {
images.push(
...data.images.map((img: Record<string, unknown>) => ({
b64_json: img.image ?? img.b64_json ?? img.url ?? img,
revised_prompt: body.prompt,
}))
);
} else if (Array.isArray(data.data)) {
images.push(...data.data);
} else if (data.url || data.b64_json || data.image) {
images.push({
b64_json: data.image || data.b64_json || data.url,
url: data.url,
revised_prompt: body.prompt,
});
}
}| const initialPayload = await response.json(); | ||
| const finalPayload = initialPayload.polling_url | ||
| ? await pollBlackForestLabsResult({ | ||
| pollingUrl: initialPayload.polling_url, | ||
| token, | ||
| body, | ||
| log, | ||
| }) | ||
| : initialPayload; |
There was a problem hiding this comment.
Defensive programming: initialPayload returned from response.json() can be null or not an object. Accessing initialPayload.polling_url directly without a null/object check can cause a runtime TypeError.
| const initialPayload = await response.json(); | |
| const finalPayload = initialPayload.polling_url | |
| ? await pollBlackForestLabsResult({ | |
| pollingUrl: initialPayload.polling_url, | |
| token, | |
| body, | |
| log, | |
| }) | |
| : initialPayload; | |
| const initialPayload = await response.json(); | |
| const finalPayload = (initialPayload && typeof initialPayload === "object" && initialPayload.polling_url) | |
| ? await pollBlackForestLabsResult({ | |
| pollingUrl: initialPayload.polling_url, | |
| token, | |
| body, | |
| log, | |
| }) | |
| : initialPayload; |
…gosouzapw#3691) Integrated into release/v3.8.23
Integrated into release/v3.8.23
…iegosouzapw#3689) Integrated into release/v3.8.23
…ution (diegosouzapw#3692) Integrated into release/v3.8.23
Integrated into release/v3.8.23
Integrated into release/v3.8.23
…osouzapw#3685) (diegosouzapw#3702) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…osouzapw#3696) (diegosouzapw#3703) Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…souzapw#3660) Integrated into release/v3.8.23
…API (diegosouzapw#3712) Integrated into release/v3.8.23. Vertex dynamic model discovery — surfaces image models (imagen-*, gemini-*-image), embeddings and audio from the live Generative Language catalog, with cached→static fallback and the shared parseGeminiModelsList helper. Validated: parser test 5/5, typecheck:core clean.
Integrated into release/v3.8.23. Makes the diegosouzapw#3588 reasoning token buffer safe and configurable: only inflates max_tokens when the model has a known, non-default output cap and the buffered value fits inside it; otherwise preserves/clamps the client limit. Adds the reasoningTokenBufferEnabled kill switch (default ON). Validated: combo-routing-engine 81/81, combo-config 25/25, combo-quality-validator-reasoning 12/12, phase1f 10/10, typecheck:core clean.
…3408 LOC (-654) (diegosouzapw#3717) Phase 1g-1j of diegosouzapw#3501: client 4062→3408 LOC. Pure extraction (ProviderPlaygroundPanel, useCommandCodeAuth, useExternalLinkFlow+ExternalLinkModal, useAuthFileHandlers) + loadConnProxies ReferenceError fix + phase1f test path fix. Co-authored-by: oyi77 <14921983+oyi77@users.noreply.github.com>
…2553 LOC (-855) (diegosouzapw#3721) Phase 1k-1m of diegosouzapw#3501: client 3408→2553 LOC. Pure extraction (useModelImportHandlers+ImportProgressModal, useModelVisibilityHandlers, ProviderModelsSection). Co-authored-by: oyi77 <14921983+oyi77@users.noreply.github.com>
… release branch The fix itself reached main pre-tag via cherry-pick diegosouzapw#3591, but its changelog bullet (commit e33fdd4) only ever existed on release/v3.8.20 after the squash-merge. Restored under [3.8.20] per the 2026-06-12 release-branch leftover audit (_tasks/release-audit/release-leftovers-audit-2026-06-12.md).
…rofileArn (diegosouzapw#3722) Integrated into release/v3.8.23
…1376 LOC (-1177) (diegosouzapw#3725) Phase 1n-1s of diegosouzapw#3501: client 2553→1376 LOC. Pure extraction (ConnectionsListPanel, ConnectionsHeaderToolbar, ZedImportCard, BatchTestResultsModal, AdaptaTutorialModal, useApiKeySave + helpers). Co-authored-by: oyi77 <14921983+oyi77@users.noreply.github.com>
…cation, and success-decay recovery (diegosouzapw#3629) Integrated into release/v3.8.23
… LOC (≤800 TARGET REACHED ✅) (diegosouzapw#3727) Phase 1t of diegosouzapw#3501: client 1376→781 LOC (≤800 reached). Original god-component 12,882→781 (−94%). Co-authored-by: oyi77 <14921983+oyi77@users.noreply.github.com>
…opencode' CLI command (diegosouzapw#3726) Integrated into release/v3.8.23
…w#3724) Integrated into release/v3.8.23
…egosouzapw#3288) (diegosouzapw#3723) Integrated into release/v3.8.23
… every chatHelpers import diegosouzapw#3692 added a lazy 'await import(proxyEgress)' for egress-IP visibility inside safeLogEvents, which is a sync function — an ES syntax error. It went unnoticed because typecheck:core does not cover src/sse and no test in the merge gates loaded chatHelpers via tsx; any consumer that did (chat-context-relay and chat-route-coverage suites, integration harnesses) failed at module load with 'await can only be used inside an async function'. safeLogEvents is fire-and-forget logging with an outer try/catch, so making it async (and 'void'-ing the single chat.ts call site) preserves behavior exactly. Validation: tests/unit/chat-context-relay.test.ts + chat-route-coverage.test.ts went from failing-at-load to green (+14 tests destravados).
… + combo/proxy audit fixes (diegosouzapw#3699) Integrated into release/v3.8.23
Integrated into release/v3.8.21 — chatCore phase modularization. Adjusted: re-derive idempotencyKey for the save path after the check moved into the module (co-authored). Thanks @oyi77!
…iegosouzapw#3588 (combo reasoning buffer) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…drop shadow/guardrails doc-fiction (diegosouzapw#3496) (diegosouzapw#3602) Integrated into release/v3.8.21 — implements GET /api/guardrails + POST /api/guardrails/test, removes shadow/guardrails doc-fiction. TDD-validated (5/5) + check-docs-symbols/typecheck/eslint green.
Split-out PR C from diegosouzapw#3584. Isolates textual reasoning wrappers (<think>/<thinking>/<thought>/<internal_thought>, including malformed/open tags) into reasoning_content across both the non-streaming sanitizer and the Gemini streaming translator, with split-chunk buffering. Additive to the existing textual tool-call pipeline; does not touch the diegosouzapw#3569 native functionResponse path. Integrated into release/v3.8.21. Thanks @dhaern!
) Split-out PR A from diegosouzapw#3584. Normalizes the Antigravity/agy Gemini 3.5 Flash tier IDs to clean public names (gemini-3.5-flash-low/medium/high), maps them to the live upstream IDs at the executor boundary, and removes Antigravity from the global model resolver so the executor owns wire normalization. Maintainer follow-up: kept gemini-3.5-flash-preview as a hidden backward-compat alias routing to the High tier (so saved combos/configs keep working). Live-validated the tier set via the agy CLI catalog. Integrated into release/v3.8.21. Thanks @dhaern!
…s port 443 (diegosouzapw#3606) (diegosouzapw#3608) Integrated into release/v3.8.21 (diegosouzapw#3606)
…500 (diegosouzapw#3589) (diegosouzapw#3609) Integrated into release/v3.8.21 (diegosouzapw#3589)
…de-plugin (diegosouzapw#3419) (diegosouzapw#3613) Integrated into release/v3.8.21 (diegosouzapw#3419)
…pw#3604) Split-out PR B from diegosouzapw#3584. Normalizes Antigravity/agy provider quotas: prefers retrieveUserQuota for live consumption, falls back to fetchAvailableModels and local usage_history, sanitizes cached Provider Limits so retired upstream IDs are not re-exposed, and schedules a deduplicated post-usage refresh. Maintainer follow-up: decoupled the post-usage refresh via a lightweight usageEvents bus (usageHistory no longer dynamic-imports providerLimits) so it does not pull the executors/translator graph into the typecheck-core surface — typecheck:core stays at 0. Integrated into release/v3.8.21. Thanks @dhaern!
…ode (diegosouzapw#3331) (diegosouzapw#3614) Integrated into release/v3.8.21 (diegosouzapw#3331)
…zapw#3604 (provider quotas) + diegosouzapw#3605 (reasoning wrappers) Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
…apw#3594) Breaks the 12,882-line providers/[id]/page.tsx into 6 domain-focused files: - types.ts — all shared interfaces/types - utils.tsx — helper functions and utility components (ModelSourceBadge, CooldownTimer, ModelCompatPopover, etc.) - modelManagement.tsx — ModelRow, ModelVisibilityToolbar, PassthroughModelsSection, CustomModelsSection, CompatibleModelsSection - connectionManagement.tsx — ConnectionRow, EditConnectionModal, EditCompatibleNodeModal - authModals.tsx — AddApiKeyModal, SiliconFlowEndpointModal, Import/Apply (Codex/Claude/Gemini) auth modals - playground.tsx — ProviderPlaygroundPanel, renderKindPanel page.tsx reduced from 12,882 → 4,687 lines (63% reduction). TypeScript typecheck passes cleanly. ESLint: 0 errors, only 3 pre-existing warnings. Pre-parse and de-duplicate all imports per file to eliminate unused imports. Resolves part of diegosouzapw#3594.
- Replaced Buffer with browser-safe atob in utils.tsx - Fixed memory leaks/infinite intervals in cooldown timers - Typed chatCore handlers with 'any' instead of 'unknown' to fix strict-mode property access - Fixed single-model reasoning payload mutation in combo.ts
) - Splits the 4,600-line combo.ts monolithic file into 12 domain-specific files - Extracts Auto-Combo, Round-Robin, Quota tracking, Shadow routing, and Context Affinity logic into dedicated modules - Uses folder-as-module pattern with index.ts facade for zero backward-compatibility breakages - Auto-prunes and manages all imports for strict typechecking
- Splits the monolithic provider validation file into 7 domain-specific files - Extracts Enterprise, Web, Search, Standard, Specialty, and Utils into dedicated modules - Uses folder-as-module pattern with index.ts facade for zero backward-compatibility breakages - Auto-prunes and manages all imports for strict typechecking
…#3594) - Splits the monolithic image generation handler into 13 domain-specific files - Extracts provider-specific implementations (OpenAI, Gemini, Stability, Fal, BFL, etc.) - Uses folder-as-module pattern with index.ts facade for zero backward-compatibility breakages - Auto-prunes and manages all imports for strict typechecking
…e generation handlers
e51de20 to
851c8e0
Compare
|
Thanks @oyi77 for the modularization work on
What would let this land: re-cut it as a single, independent PR branched from the current |
Summary
This PR addresses another core backend massive file from Issue #3594:
open-sse/handlers/imageGeneration.tsat 3,776 lines.Changes
Splits the monolithic image generation handler into 13 domain-focused files inside
open-sse/handlers/imageGeneration/:utils.tslogging.tsusageDbcore.tshandleImageGenerationswitch dispatcheropenai.tsstability.tsblackForestLabs.tsfal.ts,gemini.ts,imagen3.tschatgptWeb.ts,codex.ts,kie.tsspecialty.tsindex.tsResults
imageGeneration/index.ts)Verification
npm run typecheck:core— passes cleanly.npx eslint open-sse/handlers/imageGeneration/*.ts— 0 errors (preserves pre-existing any-warnings).Resolves part of #3594.