feat: OpenCode provider fix + API key validation & model discovery - #24
Conversation
- Added apiKeyFormat for groq (gsk_*), cohere (co_*), perplexity (pplx_*)
- Added apiKeyFormat for openrouter (sk-or-*) to prevent openai override
- Made openai pattern more specific: sk-[a-zA-Z0-9]{20,} instead of sk-[\w-]+
- Added comments explaining why some providers skip auto-detection (google, azure, bedrock)
- Patterns now specific enough to prevent false matches
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ropdown - When user selects a provider from dropdown, set isManuallySelected = true - This prevents auto-detection from overriding the user's explicit choice - Dropdown selection now has precedence over API key pattern matching Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Added tests for OpenRouter, Groq, Cohere, Perplexity detection - Fixed OpenAI test to match tighter pattern (20+ alphanumeric chars) - Added test to verify OpenRouter is NOT matched by OpenAI pattern - All 15 detection tests now pass, verifying no false positives Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fixed syntax error in opencode-settings.tsx line 288 - Removed duplicate }} that was breaking JSX parsing - All 324 tests now pass (7 pre-existing failures unrelated) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…scovery - New file: src/lib/opencode/modelFetcher.ts - validateAndFetchModels() validates API key and fetches available models - Implemented for all 11 providers: - OpenAI: Fetch from /v1/models, filter GPT models - Anthropic: Fetch from /v1/models, verify API key - Google: Fetch from generativelanguage API, filter Gemini - Groq: Fetch from /openai/v1/models endpoint - Mistral: Fetch from /v1/models endpoint - OpenRouter: Fetch public models list - Cohere, Perplexity: Use default model lists (no public API) - Azure, Bedrock, Local: Return static model lists - All requests have 10s timeout protection - Clear error messages for invalid keys or API failures Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- New file: src/lib/opencode/api-key-validator.tsx - ApiKeyValidator component with validation status display - Shows validation status: idle, validating, valid, invalid - Displays color-coded indicator (amber = checking, green = valid, red = invalid) - Auto-fetches available models when key is valid - Dropdown to select model from validated list - Debounces validation by 800ms to avoid excessive API calls - Clear error messages for failed validations - Auto-selects first model when validation succeeds Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Import ApiKeyValidator component in opencode-settings.tsx - Replace manual model input with validation-powered model dropdown - Validator runs automatically when API key is entered - Shows validation status (checking, valid, invalid) with color coding - Auto-selects first available model when key is validated - Displays error messages for invalid keys or API failures - Users no longer need to manually find and enter model names Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add modelFetcher to opencode module exports - Makes validateAndFetchModels available for import from main module - Follows existing pattern with other opencode utilities Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- New file: src/lib/opencode/modelFetcher.test.ts - 12 test cases covering: - Empty/short API key validation - Unknown provider handling - All 11 providers (no-API providers: cohere, perplexity, azure, bedrock, local) - API-backed providers: openai, anthropic, google, groq, mistral, openrouter - Error handling and timeout scenarios - Meaningful error message validation - All 12 tests passing - Tests verify no crashes and proper error handling Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📝 WalkthroughWalkthroughThis PR integrates OpenCode as an alternative AI backend alongside NVIDIA, implementing server lifecycle management, multiple API routes for agent interactions and configuration, OpenCode-aware backend selection logic in existing routes, terminal session management, configuration persistence, and UI components for settings and workbench integration. Changes
Sequence DiagramssequenceDiagram
participant User as User/UI
participant ClientAPI as CodeFlow API<br/>(code-suggestions)
participant BackendSelector as Backend<br/>Selection Logic
participant OpencodeAPI as OpenCode API<br/>(agent/session)
participant OpencodeServer as OpenCode<br/>Server Process
participant NvidiaBackend as NVIDIA<br/>Backend
User->>ClientAPI: POST /code-suggestions<br/>(useOpencode: true)
ClientAPI->>BackendSelector: Check useOpencode flag<br/>& server availability
alt OpenCode Enabled & Running
BackendSelector->>OpencodeAPI: POST /session/{id}/message
OpencodeAPI->>OpencodeServer: Forward prompt
OpencodeServer->>OpencodeServer: Generate code
OpencodeServer-->>OpencodeAPI: Code + tool-calls
OpencodeAPI-->>BackendSelector: Response (success/error)
else OpenCode Disabled or Unavailable
alt NVIDIA Available
BackendSelector->>NvidiaBackend: requestNvidiaChatCompletion
NvidiaBackend-->>BackendSelector: Code completion
else Neither Available
BackendSelector-->>ClientAPI: Error: No AI backend
end
end
BackendSelector-->>ClientAPI: Selected response
ClientAPI-->>User: Code suggestions
sequenceDiagram
participant User as User/UI
participant SettingsPanel as OpencodeSettings<br/>Component
participant ServerAPI as OpenCode API<br/>(start/stop/restart)
participant ServerMgmt as OpenCode Server<br/>Manager
participant ProcessMgr as Process/System
User->>SettingsPanel: Input config<br/>(provider, apiKey, model)
SettingsPanel->>SettingsPanel: Validate config
User->>SettingsPanel: Click Start
SettingsPanel->>ServerAPI: POST /start<br/>(provider, apiKey, model, ...)
ServerAPI->>ServerMgmt: startOpencodeServer(config)
ServerMgmt->>ProcessMgr: spawn 'opencode serve'
ProcessMgr->>ProcessMgr: Start process,<br/>emit stdout/stderr
ServerMgmt->>ServerMgmt: Listen for startup markers<br/>& health checks
ServerMgmt->>ProcessMgr: GET /health (polling)
ProcessMgr-->>ServerMgmt: 200 OK
ServerMgmt-->>ServerAPI: Server info (status, URL)
ServerAPI-->>SettingsPanel: { status: "running", url: "..." }
SettingsPanel->>SettingsPanel: Update UI status
SettingsPanel-->>User: Display running indicator
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
||
| // Store API key in sessionStorage only | ||
| if (config.apiKey) { | ||
| sessionStorage.setItem("codeflow_opencode_api_key", config.apiKey); |
There was a problem hiding this comment.
Code Review
This pull request introduces OpenCode integration as an alternative AI backend for code generation and implementation, including a new configuration panel, several API routes for server management, and enhanced repository retrieval via CodeRAG. It also adds a terminal session management feature. Feedback suggests splitting the unrelated terminal feature into a separate PR for better focus. Additionally, improvements are recommended for the AI backend fallback logic to avoid confusing mutations of request bodies and to refine the agent's response handling when only tool calls are present.
| @@ -0,0 +1,245 @@ | |||
| import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process"; | |||
There was a problem hiding this comment.
This new terminal session management feature seems unrelated to the main goal of this pull request, which is about OpenCode integration. Mixing unrelated features in a single PR makes it harder to review, test, and revert if needed. It's recommended to split this into a separate pull request to maintain a clean and focused commit history.
| if (!useOpencode && !apiKey) { | ||
| // Check if OpenCode is available as fallback | ||
| if (isOpencodeAvailable()) { | ||
| body.useOpencode = true; | ||
| } else { | ||
| return NextResponse.json( | ||
| { error: "No AI backend available. Either start OpenCode server or provide NVIDIA API key." }, | ||
| { status: 400 } | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Mutating the parsed body object to handle the fallback to OpenCode can be confusing. The original useOpencode constant remains false, and the logic later needs to check useOpencode || body.useOpencode. Consider using a mutable let useOpencode variable initialized earlier to make the control flow more straightforward and avoid side effects on the parsed request body.
| if (!useOpencode && !apiKey) { | ||
| // Check if OpenCode is available as fallback | ||
| if (isOpencodeAvailable()) { | ||
| // Redirect to use OpenCode | ||
| rawBody.useOpencode = true; | ||
| } else { | ||
| return NextResponse.json( | ||
| { error: "No AI backend available. Either start OpenCode server or provide NVIDIA API key." }, | ||
| { status: 400 } | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Mutating the parsed rawBody object to handle the fallback to OpenCode can be confusing. The original useOpencode constant remains false, and the logic later needs to check useOpencode || rawBody.useOpencode. Consider using a mutable let useOpencode variable initialized earlier to make the control flow more straightforward and avoid side effects on the parsed request body.
|
|
||
| return { | ||
| success: true, | ||
| response: responseContent || JSON.stringify(parsed), |
There was a problem hiding this comment.
If responseContent is an empty string (e.g., the agent's response only contains tool calls), this will return the stringified version of the entire parsed response. This might be unexpected for clients that anticipate a purely textual response. It might be better to return an empty string or undefined for the response field in such cases, and let the actions field carry the tool call information.
There was a problem hiding this comment.
Pull request overview
This PR expands the OpenCode integration by adding provider-specific API key detection, a server lifecycle wrapper + API routes, and a UI workflow for API key validation/model discovery. It also introduces new server-side terminal session endpoints and a larger refactor of the CodeRAG “repo context” UI into a dock tab with accompanying layout/CSS updates.
Changes:
- Add OpenCode provider configuration, server lifecycle management, proxy API routes, and an OpenCode settings panel with API key validation + model discovery.
- Update Blueprint Workbench to surface CodeRAG as a “repo” dock tab and add OpenCode controls/toggle for agent usage.
- Add a server-side terminal sessions subsystem (session CRUD + input streaming) with Next.js API routes and tests.
Reviewed changes
Copilot reviewed 34 out of 36 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/server/terminal-sessions.ts | Implements in-memory terminal session manager backed by spawned shell processes. |
| src/app/api/terminal/sessions/route.ts | Adds API endpoints to list/create terminal sessions. |
| src/app/api/terminal/sessions/[sessionId]/route.ts | Adds API endpoints to read/write/close a terminal session. |
| src/app/api/terminal/sessions/route.test.ts | Tests terminal session creation/listing API. |
| src/lib/opencode/types.ts | Defines OpenCode types + provider configs and API key formats. |
| src/lib/opencode/config.ts | Adds config helpers (detectProvider/build/save/load/validate/configToEnv). |
| src/lib/opencode/config.test.ts | Adds tests for provider detection and config persistence/validation. |
| src/lib/opencode/server.ts | Adds OpenCode CLI process lifecycle management + health checks. |
| src/lib/opencode/server.test.ts | Adds basic tests for server info reporting. |
| src/lib/opencode/client.ts | Adds browser client wrapper for OpenCode API routes. |
| src/lib/opencode/agent.ts | Adds backend selection + OpenCode request helpers + response extractors. |
| src/lib/opencode/agent.test.ts | Adds unit tests for agent helper utilities and backend selection. |
| src/lib/opencode/modelFetcher.ts | Adds API key validation + model discovery per provider. |
| src/lib/opencode/modelFetcher.test.ts | Adds tests around model fetcher behavior across providers. |
| src/lib/opencode/api-key-validator.tsx | Adds React UI component for debounced validation + model dropdown. |
| src/lib/opencode/index.ts | Exports OpenCode module surface including modelFetcher. |
| src/app/api/opencode/status/route.ts | Adds status endpoint for OpenCode server. |
| src/app/api/opencode/start/route.ts | Adds start endpoint for OpenCode server. |
| src/app/api/opencode/stop/route.ts | Adds stop endpoint for OpenCode server. |
| src/app/api/opencode/restart/route.ts | Adds restart endpoint for OpenCode server. |
| src/app/api/opencode/agent/route.ts | Adds agent messaging endpoint that proxies to OpenCode sessions. |
| src/app/api/opencode/sessions/route.ts | Adds session list/create endpoints that proxy OpenCode sessions. |
| src/app/api/opencode/sessions/[id]/route.ts | Adds session get/message/delete endpoints that proxy OpenCode sessions. |
| src/app/api/opencode/mcp/route.ts | Adds MCP list/config proxy endpoints. |
| src/app/api/opencode/permissions/route.ts | Adds permission list/reply proxy endpoints. |
| src/components/opencode-settings.tsx | Adds settings UI for configuring OpenCode + validation integration. |
| src/components/blueprint-workbench.tsx | Adds “repo” dock tab for CodeRAG and integrates OpenCode controls/toggle. |
| src/components/blueprint-workbench.test.tsx | Updates workbench tests to cover repo dock behavior. |
| src/app/layout.tsx | Moves @xyflow/react CSS import out of layout. |
| src/app/globals.css | Imports @xyflow/react CSS and updates workbench/IDE layout styles + OpenCode styles. |
| src/app/api/implement-node/route.ts | Adds useOpencode support and routes completions to OpenCode when available. |
| src/app/api/code-suggestions/route.ts | Adds useOpencode support and routes suggestions to OpenCode when available. |
| README.md | Documents OpenCode integration and new API routes. |
| package.json | Adds opencode-ai and cross-spawn dependencies (+ types). |
| package-lock.json | Locks newly added dependencies. |
| next-env.d.ts | Changes Next.js generated route types import path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| test("status is one of valid values", () => { | ||
| const info = getOpencodeServerInfo(); | ||
| expect(["stopped", "starting", "running", "stopping", "error"]).toContain(info.status); | ||
| }); |
There was a problem hiding this comment.
OpencodeServerStatus (in types.ts) does not include "stopping", and server.ts never reports it. This assertion will fail once types are enforced consistently; either remove "stopping" from the allowed set here or add a real "stopping" state to the type + implementation.
| openai: { | ||
| provider: "openai", | ||
| defaultModel: "gpt-5.4-mini", | ||
| apiKeyEnvVar: "OPENAI_API_KEY", | ||
| // More specific: OpenAI keys are longer (20+ chars after "sk-") | ||
| apiKeyFormat: /^sk-[a-zA-Z0-9]{20,}$/, | ||
| }, |
There was a problem hiding this comment.
The OpenAI API key regex is too strict for common modern OpenAI keys (e.g. keys with additional hyphens like sk-proj-...). This will cause validateConfig to reject valid keys; consider loosening the pattern (while still avoiding collisions with other providers).
| export async function validateAndFetchModels( | ||
| provider: OpencodeProvider, | ||
| apiKey: string | ||
| ): Promise<ValidationResult> { | ||
| if (!apiKey || apiKey.length < 10) { | ||
| return { valid: false, error: "API key too short" }; | ||
| } | ||
|
|
There was a problem hiding this comment.
validateAndFetchModels rejects any key shorter than 10 chars before checking provider, but some providers (notably local) don't require an API key at all. This makes local impossible to validate/discover unless the user enters a dummy long key; move the length check into provider-specific branches (or skip it for providers that don't require keys).
| try { | ||
| const response = await fetch("https://openrouter.ai/api/v1/models", { | ||
| signal: controller.signal, | ||
| }); | ||
|
|
||
| clearTimeout(timeout); | ||
|
|
||
| if (!response.ok) { | ||
| return { valid: false, error: `OpenRouter API error: ${response.status}` }; | ||
| } |
There was a problem hiding this comment.
fetchOpenRouterModels never sends the provided API key (no Authorization header), so validateAndFetchModels("openrouter", ...) can report valid: true even for an invalid key. If this function is meant to validate keys, call an authenticated endpoint or perform a lightweight authenticated request to confirm the key before returning valid: true.
| // Skip if we're validating the same key | ||
| if (validatingKey === apiKey) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
The debounce skip condition only compares validatingKey to apiKey. If the user changes provider but keeps the same key string, validation is skipped and the component can show stale status/models from the previous provider. Track the last validated tuple (provider+key) or reset validatingKey when provider changes.
| export async function POST(request: Request) { | ||
| try { | ||
| const payload = createSessionSchema.parse( | ||
| await request | ||
| .json() | ||
| .catch(() => ({})) | ||
| ); | ||
| const repoPath = request.headers.get(TERMINAL_REPO_PATH_HEADER) ?? undefined; | ||
| const session = await createTerminalSession({ | ||
| cwd: repoPath, | ||
| title: payload.title | ||
| }); |
There was a problem hiding this comment.
Security: this route allows any caller to create an interactive shell session on the server (and optionally choose its working directory via x-codeflow-repo-path). If this app can be accessed by untrusted users, this is remote code execution. Gate these terminal endpoints behind authentication/authorization and/or disable them in production builds.
| export async function POST( | ||
| request: Request, | ||
| { params }: { params: Promise<{ sessionId: string }> } | ||
| ) { | ||
| try { | ||
| const { sessionId } = await params; | ||
| if (!isValidSessionId(sessionId)) { | ||
| return NextResponse.json({ error: "Invalid terminal session id." }, { status: 400 }); | ||
| } | ||
|
|
||
| const payload = terminalInputSchema.parse(await request.json()); | ||
| const session = await writeTerminalInput(sessionId, payload.input, { | ||
| echoInput: payload.echoInput | ||
| }); | ||
|
|
||
| return NextResponse.json({ session }); |
There was a problem hiding this comment.
Security: this endpoint writes arbitrary input into a spawned shell process without any authz checks. If exposed to untrusted clients it is remote code execution. Require auth/CSRF protection and consider scoping session access to the requesting user (not just a guessable UUID).
| import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process"; | ||
| import fs from "node:fs/promises"; | ||
| import path from "node:path"; | ||
| import { randomUUID } from "node:crypto"; | ||
|
|
There was a problem hiding this comment.
The PR title/description focuses on OpenCode provider selection + API key validation/model discovery, but this PR also introduces a full server-side terminal sessions subsystem and substantial CodeRAG/IDE layout changes. Consider splitting these into separate PRs or updating the PR description to cover the additional scope so reviewers understand the risk/intent.
| async function fetchCohereModels(apiKey: string): Promise<ValidationResult> { | ||
| // Cohere doesn't have a public models list endpoint | ||
| // Return default models and assume key is valid if format is correct | ||
| return { | ||
| valid: true, | ||
| models: [ | ||
| "command-r-plus", | ||
| "command-r", | ||
| "command", | ||
| "command-light", | ||
| "command-nightly", | ||
| ], | ||
| }; | ||
| } |
There was a problem hiding this comment.
fetchCohereModels returns valid: true without performing any request to verify the key (and the comment mentions "assume key is valid if format is correct", but the function never checks format here). This can incorrectly mark invalid keys as valid; consider returning a non-validating status/error, or add at least a lightweight authenticated call to confirm the key.
| async function fetchPerplexityModels( | ||
| apiKey: string | ||
| ): Promise<ValidationResult> { | ||
| // Perplexity doesn't have a public models list endpoint | ||
| // Return default models and assume key is valid if format is correct | ||
| return { | ||
| valid: true, | ||
| models: [ | ||
| "llama-3.1-sonar-large-128k-online", | ||
| "llama-3.1-sonar-small-128k-online", | ||
| "llama-3.1-sonar-large-128k-chat", | ||
| "llama-3.1-sonar-small-128k-chat", | ||
| ], | ||
| }; | ||
| } |
There was a problem hiding this comment.
fetchPerplexityModels returns valid: true without actually verifying the API key. This can mislead users into thinking the key works when it doesn't; consider returning a distinct "unverified" state or performing a lightweight authenticated request if possible.
There was a problem hiding this comment.
Actionable comments posted: 4
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (18)
src/app/api/opencode/sessions/[id]/route.ts-27-29 (1)
27-29:⚠️ Potential issue | 🟠 MajorEncode
sessionIdbefore building the upstream URL.
sessionIdcomes from the route param, so values containing/,?, or#can change the upstream path/query and hit the wrong resource. Buildconst encodedSessionId = encodeURIComponent(sessionId)once and use it in GET/POST/DELETE.Suggested fix
- const sessionId = params.id; + const sessionId = params.id; + const encodedSessionId = encodeURIComponent(sessionId); @@ - const response = await fetch(`${serverInfo.url}/session/${sessionId}`, { + const response = await fetch(`${serverInfo.url}/session/${encodedSessionId}`, { @@ - const sessionId = params.id; + const sessionId = params.id; + const encodedSessionId = encodeURIComponent(sessionId); @@ - const response = await fetch(`${serverInfo.url}/session/${sessionId}/message`, { + const response = await fetch(`${serverInfo.url}/session/${encodedSessionId}/message`, { @@ - const sessionId = params.id; + const sessionId = params.id; + const encodedSessionId = encodeURIComponent(sessionId); @@ - const response = await fetch(`${serverInfo.url}/session/${sessionId}`, { + const response = await fetch(`${serverInfo.url}/session/${encodedSessionId}`, {Also applies to: 75-80, 134-137
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/opencode/sessions/`[id]/route.ts around lines 27 - 29, The route handler uses raw sessionId from the route in building upstream URLs which can break if sessionId contains reserved URL chars; create a single encodedSessionId = encodeURIComponent(sessionId) and replace uses of sessionId in the fetch calls (the GET request that builds `${serverInfo.url}/session/${sessionId}`, the POST/DELETE requests around the other fetches at the other locations) with encodedSessionId so all upstream URLs use the safe, encoded identifier (keep serverInfo.url and other headers unchanged).src/app/api/opencode/agent/route.ts-25-35 (1)
25-35:⚠️ Potential issue | 🟠 MajorSession reuse is leaking context across unrelated requests.
getOrCreateSession()reuses the first session returned bylimit=1without checking the requested agent or flow, so a newbuild,plan, orgeneralprompt can inherit prior conversation state from a different request. Either always create a fresh session here or only reuse a session that explicitly matches the current agent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/opencode/agent/route.ts` around lines 25 - 35, getOrCreateSession currently reuses the first session from `${serverUrl}/session?limit=1` which leaks conversation context across different agent/flow requests; update getOrCreateSession(serverUrl, agentType) to either always create a fresh session or only reuse a session that matches agentType: fetch the session list, filter the returned sessions for one whose agent/flow metadata equals the provided agentType (or any explicit flow identifier), and if none match, POST to `${serverUrl}/session` with a body containing the agentType to create a new session; return the created or matched session id from getOrCreateSession.src/app/api/opencode/mcp/route.ts-45-63 (1)
45-63:⚠️ Potential issue | 🟠 MajorValidate the MCP payload before forwarding it.
request.json()is proxied as-is, so malformed JSON becomes a 500 and invalid MCP configs are only rejected by the downstream service. Parse the body with a schema here and return a 400 for bad input.As per coding guidelines: Validate external I/O - All request bodies, persisted payloads, AI responses, MCP responses, and export manifests must be schema-validated before use.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/opencode/mcp/route.ts` around lines 45 - 63, In POST (src/app/api/opencode/mcp/route.ts) validate the incoming MCP payload before forwarding: define or import a strict mcpSchema (e.g., using Zod) and call mcpSchema.parse or safeParse on the result of await request.json() inside POST; if parsing fails return NextResponse.json({ success: false, error: "Invalid MCP payload", details: <validation errors> }, { status: 400 }); only use the validated/parsed object (not the raw body) when calling fetch to `${serverInfo.url}/mcp/server`; keep existing serverInfo checks (getOpencodeServerInfo) and error handling for downstream failures.src/components/blueprint-workbench.tsx-1516-1517 (1)
1516-1517:⚠️ Potential issue | 🟠 MajorThis overrides the user's backend choice.
As written,
useOpencodeflips totruewhenever OpenCode is merely running, even if the user left “Use OpenCode for code generation” unchecked. That silently reroutes implementation requests to a different backend.Suggested fix
- useOpencode: useOpencodeForAgent || opencodeStatus.status === "running" + useOpencode: useOpencodeForAgent && opencodeStatus.status === "running"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/blueprint-workbench.tsx` around lines 1516 - 1517, The current assignment for useOpencode overrides the user's explicit choice by setting useOpencode: useOpencodeForAgent || opencodeStatus.status === "running"; change this so the user's checkbox (useOpencodeForAgent) is authoritative and only enable automatic fallback when the user has not explicitly chosen otherwise: compute useOpencode to be useOpencodeForAgent ? true : (opencodeStatus.status === "running" && /* only when user left choice unset */ /* e.g., when useOpencodeForAgent is null/undefined */); update the logic around useOpencode, useOpencodeForAgent, and opencodeStatus.status so that opencodeStatus only affects behavior when the user's choice is unset/neutral rather than forcing true when OpenCode is running.src/app/api/opencode/agent/route.ts-99-130 (1)
99-130:⚠️ Potential issue | 🟠 MajorValidate the upstream response before extracting actions.
parsed.partsis external/AI output, but it's traversed as if its shape is guaranteed and rawargs/inputvalues are forwarded to the client. Add a Zod schema for the response/part union so malformed payloads fail as structured errors instead of becoming bogus actions.As per coding guidelines: Treat model output as untrusted input - AI output must be schema-validated, compiled, tested, and reviewed before it is treated as implementation, command input, configuration, or persisted state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/opencode/agent/route.ts` around lines 99 - 130, The code currently trusts parsed.parts and forwards raw part.args/part.input into actions (in route.ts handling parsed.parts), so add a Zod schema for the overall response and a discriminated union for parts (types "text", "tool-call"/"tool_call") that validates required fields (e.g., text for text parts, toolName/name and a validated args/input shape for tool parts and specific tools like write_file/create_file/edit_file/replace/bash/shell). Use zod.parse or safeParse on parsed (and parsed.parts) before extracting responseContent and populating the AgentResponse["actions"] array; only build actions when validation succeeds and otherwise throw or return a structured error/logging response. Ensure the schema names/types are referenced where you currently inspect parsed.parts and where actions is populated so malformed AI output is rejected instead of forwarded.src/lib/opencode/modelFetcher.test.ts-84-94 (1)
84-94:⚠️ Potential issue | 🟠 MajorThese cases won't catch timeout or dispatch regressions.
They only assert that a result-shaped object comes back, so the suite still passes if a provider branch stops validating/fetching models or the timeout path never executes. In particular, the “handles timeout gracefully” case never forces a timeout. Please mock the provider fetches and assert the exact timeout/error/model outcome for each branch instead.
Based on learnings: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : No fake-pass tests - Tests that only assert mocks were called, snapshot-only tests for business logic, tests with no meaningful assertions, widened expectations just to pass CI, or disabled warnings are not acceptable.
Also applies to: 112-126, 130-143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/opencode/modelFetcher.test.ts` around lines 84 - 94, The test "handles providers with API validation" uses a real call shape and can pass falsely; instead explicitly mock the provider network calls used by validateAndFetchModels (e.g., the HTTP client/fetch wrapper or provider-specific methods) to return controlled responses: 1) mock a successful models response and assert the returned object contains the expected models array and valid: true, 2) mock a provider validation error and assert error is set and valid: false, and 3) mock a delayed response (or rejected promise) to trigger the validateAndFetchModels timeout path and assert the timeout-specific error/result; update the test named "handles providers with API validation" and the related "handles timeout gracefully" tests to use these mocks and assert exact outcomes rather than only checking for property presence.src/app/api/opencode/mcp/route.ts-11-29 (1)
11-29:⚠️ Potential issue | 🟠 MajorGET currently reports proxy failures as HTTP 200.
Both error branches omit a status, so callers that rely on
response.okwill treat “OpenCode server is not running” and upstream failures as a successful empty list. Return503when OpenCode is unavailable and propagateresponse.statuswhen the proxy call fails.Suggested fix
- if (serverInfo.status !== "running" || !serverInfo.url) { - return NextResponse.json({ - servers: [], - error: "OpenCode server is not running" - }); - } + if (serverInfo.status !== "running" || !serverInfo.url) { + return NextResponse.json( + { + servers: [], + error: "OpenCode server is not running" + }, + { status: 503 } + ); + } @@ - if (!response.ok) { - const errorText = await response.text(); - return NextResponse.json({ - servers: [], - error: `Failed to fetch MCP servers: ${errorText}` - }); - } + if (!response.ok) { + const errorText = await response.text(); + return NextResponse.json( + { + servers: [], + error: `Failed to fetch MCP servers: ${errorText}` + }, + { status: response.status } + ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/opencode/mcp/route.ts` around lines 11 - 29, The handler currently returns JSON with no HTTP status for error cases, causing callers to see HTTP 200; update the error responses to set proper statuses: when serverInfo.status !== "running" or !serverInfo.url return NextResponse.json({...}, { status: 503 }) to indicate the OpenCode server is unavailable, and when the upstream fetch to `${serverInfo.url}/mcp/servers` returns !response.ok return NextResponse.json({ servers: [], error: `Failed to fetch MCP servers: ${errorText}` }, { status: response.status }) so the proxy propagates the original upstream status; adjust the code paths that call NextResponse.json accordingly (referencing serverInfo.status, serverInfo.url, fetch(...) response handling and NextResponse.json).src/app/api/opencode/sessions/route.ts-12-31 (1)
12-31:⚠️ Potential issue | 🟠 MajorGET hides session-list failures behind HTTP 200.
Both error branches return the default success status, so callers that check
response.okwill read an empty list instead of an unavailable/failed proxy. Return503when OpenCode is down and forward the upstream status on proxy failures.Suggested fix
- if (serverInfo.status !== "running" || !serverInfo.url) { - return NextResponse.json({ - sessions: [], - error: "OpenCode server is not running" - }); - } + if (serverInfo.status !== "running" || !serverInfo.url) { + return NextResponse.json( + { + sessions: [], + error: "OpenCode server is not running" + }, + { status: 503 } + ); + } @@ - if (!response.ok) { - const errorText = await response.text(); - return NextResponse.json({ - sessions: [], - error: `Failed to fetch sessions: ${errorText}` - }); - } + if (!response.ok) { + const errorText = await response.text(); + return NextResponse.json( + { + sessions: [], + error: `Failed to fetch sessions: ${errorText}` + }, + { status: response.status } + ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/opencode/sessions/route.ts` around lines 12 - 31, The handler returns HTTP 200 even when the OpenCode server is down or the upstream fetch fails; update the branches around serverInfo check and the fetch error handling so that when serverInfo.status !== "running" or !serverInfo.url you return a NextResponse.json with status: 503 (service unavailable) and when the upstream fetch returns !response.ok forward that upstream status code (use response.status) in the NextResponse.json error response instead of always returning 200; modify the logic near serverInfo, the fetch to `${serverInfo.url}/session?limit=${limit}`, and the response.ok error branch to set the appropriate status.src/lib/opencode/api-key-validator.tsx-31-67 (1)
31-67:⚠️ Potential issue | 🟠 MajorThis debounce guard skips legitimate revalidation and can leave stale state behind.
The early return only keys on the raw API key, so switching providers with the same key—or clearing and re-entering the same key—short-circuits validation. The async call is also uncaught, so a thrown
validateAndFetchModels()leaves the component stuck at “Validating…” with no visible error.Suggested fix
useEffect(() => { if (!apiKey || apiKey.length < 10) { setStatus("idle"); setModels([]); setError(null); + setValidatingKey(""); return; } - // Skip if we're validating the same key - if (validatingKey === apiKey) { + const validationKey = `${provider}:${apiKey}`; + if (validatingKey === validationKey) { return; } + let cancelled = false; const timer = setTimeout(async () => { setStatus("validating"); - setValidatingKey(apiKey); + setValidatingKey(validationKey); setError(null); - const result = await validateAndFetchModels(provider, apiKey); - - if (result.valid && result.models) { - setStatus("valid"); - setModels(result.models); - setError(null); - // Auto-select first model if available - if (!selectedModel && result.models.length > 0) { - onModelChange?.(result.models[0]); + try { + const result = await validateAndFetchModels(provider, apiKey); + if (cancelled) { + return; } - } else { - setStatus("invalid"); - setModels([]); - setError(result.error || "Validation failed"); + if (result.valid && result.models) { + setStatus("valid"); + setModels(result.models); + setError(null); + if (!selectedModel && result.models.length > 0) { + onModelChange?.(result.models[0]); + } + } else { + setStatus("invalid"); + setModels([]); + setError(result.error || "Validation failed"); + } + } catch (caughtError) { + if (cancelled) { + return; + } + setStatus("invalid"); + setModels([]); + setError(caughtError instanceof Error ? caughtError.message : "Validation failed"); } }, 800); // Debounce for 800ms - return () => clearTimeout(timer); + return () => { + cancelled = true; + clearTimeout(timer); + }; }, [apiKey, provider, selectedModel, validatingKey, onModelChange]);As per coding guidelines: No silent failures - Every catch path must either return a structured error, set a visible UI error or stale state, or log diagnostic context intentionally.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/opencode/api-key-validator.tsx` around lines 31 - 67, The early-return debounce guard only compares validatingKey to apiKey and therefore skips validation when provider changes (or key is cleared/re-entered); also the async call to validateAndFetchModels is uncaught which can leave the component stuck in "validating". Fix by keying the guard and setValidatingKey on a composite identifier (e.g., `${provider}:${apiKey}`) so provider changes retrigger validation, and wrap the validateAndFetchModels call in try/catch/finally inside the debounced callback to always setStatus and setError appropriately (on success setStatus("valid") and setModels, on failure setStatus("invalid"), setModels([]) and setError with the caught error message), and clear or reset validatingKey in finally so state cannot remain stuck; update references to useEffect, validatingKey, validateAndFetchModels, setStatus, setError, setModels, setValidatingKey, and onModelChange accordingly.src/lib/opencode/client.ts-26-35 (1)
26-35:⚠️ Potential issue | 🟠 MajorThe lifecycle request shape is dropping the advanced config fields.
OpencodeSettingsbuildsmcpServers,skills, andhooks, but these wrappers cannot send them. The server rebuilds config from this truncated body, so advanced settings are saved locally and never applied on start/restart.Also applies to: 62-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/opencode/client.ts` around lines 26 - 35, The lifecycle request (in startServer) is sending only the simple config and dropping advanced fields built by OpencodeSettings (mcpServers, skills, hooks), so the server reconstructs a truncated config; update startServer (and the similar lifecycle function around lines 62–72) to include the full advanced fields when serializing the body—accept or build an object that merges config with OpencodeSettings' mcpServers, skills, and hooks (or change the input type to include those fields) and JSON.stringify that combined payload so the server receives and persists the advanced settings.src/components/opencode-settings.tsx-299-311 (1)
299-311:⚠️ Potential issue | 🟠 MajorDon't block the
localprovider on an API key.
validateConfig()explicitly allowsprovider === "local"without a key, but this form still labels the key as required and disables Start whenapiKeyis empty. That makes the Local Model option impossible to use.Suggested fix
<small> {apiKey ? `✓ Key provided (${apiKey.slice(0, 8)}...)` - : `Required for ${provider}`} + : provider === "local" + ? "Optional for local provider" + : `Required for ${provider}`} </small> ... <button onClick={handleStartServer} - disabled={isLoading || !apiKey} + disabled={isLoading || (provider !== "local" && !apiKey)} type="button" className="primary" >Also applies to: 369-372
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/opencode-settings.tsx` around lines 299 - 311, The UI currently treats apiKey as required for all providers even though validateConfig() allows provider === "local" without a key; update the form logic in src/components/opencode-settings.tsx (the component managing provider, apiKey, and the Start button) so that the API key field and any "required"/disable logic only apply when provider !== "local": adjust the input placeholder/small message (where it references PROVIDERS.find(...), apiKey, and provider) to show a non-required hint for "local", and change the Start button disabling/check that currently blocks on apiKey emptiness to instead require apiKey only when provider !== "local" (also apply the same change to the other occurrence around the Start button logic at the later block referenced in the comment).src/lib/opencode/server.ts-228-236 (1)
228-236:⚠️ Potential issue | 🟠 MajorUse one health criterion for startup and steady-state checks.
Line 235 treats
404as healthy, but Line 262 only acceptsresponse.ok. That can mark startup successful and then flip the server to"error"on the first interval, and it can also accept an unrelated process already bound to the port.Also applies to: 257-263
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/opencode/server.ts` around lines 228 - 236, The startup health-check and the periodic/steady-state health-check use inconsistent criteria (one treats response.status === 404 as healthy while the other uses only response.ok); update the health-check logic that inspects the fetch Response (the code using controller.signal, timeoutId and the response variable) so both startup and interval checks use the same single criterion — either always accept only response.ok or consistently accept response.ok || response.status === 404 — and apply that unified check in both places where the health endpoint is polled (the initial startup fetch block and the recurring interval fetch block).src/components/opencode-settings.tsx-67-68 (1)
67-68:⚠️ Potential issue | 🟠 MajorWire
selectedAgentthrough or remove the control.In this file,
selectedAgentis never persisted or included in the save/start/restart payloads, so the “Default Agent” choice is lost on reload and has no runtime effect. As per coding guidelines, "Be truthful about feature maturity - Heuristic, simulated, AI-generated, or scaffold output must be labeled as such in APIs and UI. Do not present them as observed truth or production-ready implementation unless validated".Also applies to: 122-160, 177-196, 237-247, 335-365
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/opencode-settings.tsx` around lines 67 - 68, selectedAgent is only local UI state and never persisted or sent, so the "Default Agent" selection is lost and has no runtime effect; update the settings flow to wire selectedAgent into persistence and runtime payloads by: (1) initializing selectedAgent from the stored settings (where settings are loaded, e.g., the useEffect or loadSettings function) so the UI reflects saved value, (2) include selectedAgent in the object saved by the saveSettings handler (or settings update API call) so it is persisted, and (3) include selectedAgent in the payloads sent by start/restart handlers (e.g., startPipeline/startRun/restartRun or whatever functions construct runPayload) so the agent choice is honored at runtime; ensure the TypeScript union type ("build" | "plan") is preserved when reading/writing and add any necessary defaults when settings are missing.src/lib/opencode/client.ts-165-186 (1)
165-186:⚠️ Potential issue | 🟠 MajorDon't collapse suggestion failures into
[].
[]is a valid “no suggestions” result, so callers can't tell the difference between model output and transport/server failure. Return a structured error here like the other helpers do. As per coding guidelines, "No silent failures - Every catch path must either return a structured error, set a visible UI error or stale state, or log diagnostic context intentionally".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/opencode/client.ts` around lines 165 - 186, getCodeSuggestions currently hides transport/server failures by returning an empty array on error; change it to return a structured error consistent with the other helper functions instead of collapsing to [] so callers can distinguish "no suggestions" from failure. Specifically, update getCodeSuggestions to propagate or return a Result-like object (or throw a descriptive Error) that includes success/failure metadata and diagnostic info from sendAgentMessage (e.g., response.success, response.error or response.response) rather than returning []; adjust the function signature/return type to match the project's existing helper pattern, and include the original response/error details for logging/diagnostics while preserving the normal successful-path return of suggestions extracted from response.response.src/lib/opencode/agent.ts-183-191 (1)
183-191:⚠️ Potential issue | 🟠 MajorDon't append every request to the first existing session.
limit=1+return sessions[0].idmakes unrelated generations share a single conversation. That leaks prior context across tasks/users and makes responses depend on old history.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/opencode/agent.ts` around lines 183 - 191, The code that lists sessions (using listRes, sessions and serverUrl) currently returns sessions[0].id after calling GET `${serverUrl}/session?limit=1`, which causes all requests to reuse the first session and leak prior context; instead, update the logic so you do not unconditionally reuse the first session: either find and return a session that matches the current user/task criteria (if such matching metadata exists) or always create a fresh session by POSTing to `${serverUrl}/session` and returning the new session id. Locate the block that fetches sessions (the listRes/ sessions code) and replace the unconditional return of sessions[0].id with a safe selection/matching check or with a call to create a new session, ensuring the variable that holds the final session id is assigned from the newly-created session response when appropriate.src/lib/opencode/config.ts-92-110 (1)
92-110:⚠️ Potential issue | 🟠 MajorSchema-validate stored config before returning it.
JSON.parse(stored) as OpencodeConfigtrusts arbitrary local/session storage content. A malformedprovider,mcpServers, orhookspayload can flow straight into state andPROVIDER_CONFIGS[provider]lookups with no guard. Parse with a schema and clear corrupt storage on failure. As per coding guidelines, "Validate external I/O - All request bodies, persisted payloads, AI responses, MCP responses, and export manifests must be schema-validated before use".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/opencode/config.ts` around lines 92 - 110, In loadConfig(), avoid trusting JSON.parse(...) as OpencodeConfig; instead parse stored, validate it against the OpencodeConfig schema (ensuring valid provider, mcpServers, hooks, etc. and that provider exists in PROVIDER_CONFIGS) before returning; if validation fails, remove OPENCODE_CONFIG_KEY from localStorage (and any corrupt related keys) and return null; also only overwrite config.apiKey from sessionStorage after the stored config has successfully validated so you never inject a session key into an invalid object.src/lib/opencode/config.ts-76-87 (1)
76-87:⚠️ Potential issue | 🟠 MajorKeep the raw API key out of browser storage.
sessionStorageis still cleartext browser storage, so any XSS or privileged extension can read the key. Keep it only in in-memory tab state, or exchange it for a server-side session handle before persistence. As per coding guidelines, "Do not persist raw secrets in browser storage - Use server environment variables, session-only memory, or explicit secure local handling. Never store raw secrets in exported artifacts, sessions, or long-lived browser storage".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/opencode/config.ts` around lines 76 - 87, The saveConfig function currently persists the raw API key into sessionStorage (via sessionStorage.setItem("codeflow_opencode_api_key", ...)), which violates the "no raw secrets in browser storage" guideline; remove the sessionStorage write and instead keep the API key in ephemeral in-memory tab state or exchange it for a server-side session handle before any persistence. Update saveConfig (and any callers that rely on reading "codeflow_opencode_api_key") to: 1) stop writing the raw API key to sessionStorage, 2) accept or return a server-issued session token or store the API key only in a transient in-memory store (e.g., an in-module variable or context) for the lifetime of the tab, and 3) ensure persisted OPENCODE_CONFIG_KEY contains only sanitized fields (as already done) while any code that needs authentication uses the server-side token or the in-memory secret via functions you add to get/set the in-memory value.src/lib/opencode/agent.ts-272-288 (1)
272-288: 🛠️ Refactor suggestion | 🟠 MajorDon't cast model JSON to
Twithout validation.
extractJsonFromResponse<T>()turns arbitrary model text into a trusted generic with no shape check. Accept a schema here, or require callers to validate before using the parsed object. As per coding guidelines, "Treat model output as untrusted input - AI output must be schema-validated, compiled, tested, and reviewed before it is treated as implementation, command input, configuration, or persisted state".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/opencode/agent.ts` around lines 272 - 288, extractJsonFromResponse<T> currently parses model output and casts it to T without validation; change it to avoid trusting parsed data by either (a) accepting a runtime schema/validator parameter (e.g., a Zod schema, io-ts codec, or a validation function) and validating the parsed object before returning the typed T, or (b) change the return type to unknown (or a safe wrapper like Result/Validation) and require callers to run validation themselves; update the function signature (extractJsonFromResponse) and its call sites to use the validator or handle unknown results, and ensure any errors from validation are surfaced instead of silent null casts.
🟡 Minor comments (6)
src/lib/opencode/server.test.ts-17-20 (1)
17-20:⚠️ Potential issue | 🟡 MinorTest asserts invalid status value
"stopping"not in type definition.The test expects
statusto be one of["stopped", "starting", "running", "stopping", "error"], but persrc/lib/opencode/types.ts:36-40,OpencodeServerStatusonly includes four values:"stopped","starting","running", and"error". The"stopping"literal does not exist in the type.Either remove
"stopping"from the test assertion or add it to the type definition if it's a legitimate state.🔧 Fix to align with current type definition
test("status is one of valid values", () => { const info = getOpencodeServerInfo(); - expect(["stopped", "starting", "running", "stopping", "error"]).toContain(info.status); + expect(["stopped", "starting", "running", "error"]).toContain(info.status); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/opencode/server.test.ts` around lines 17 - 20, The test "status is one of valid values" uses getOpencodeServerInfo() but asserts an invalid literal "stopping" that is not part of the OpencodeServerStatus type; update the test by removing "stopping" from the expected array (i.e., make the expect list ["stopped","starting","running","error"]) so the assertion aligns with the OpencodeServerStatus type defined in src/lib/opencode/types.ts, or alternatively add "stopping" to the OpencodeServerStatus union if that state is legitimately required.src/app/globals.css-5026-5029 (1)
5026-5029:⚠️ Potential issue | 🟡 MinorFix font-family quoting to satisfy Stylelint.
Static analysis flagged
font-family-name-quoteserror. Font family names containing special characters or multiple words should be quoted, but single-word names likeSFMono-Regularmay need consistent handling per project rules.🔧 Suggested fix
.numeric-stepper input { text-align: center; - font-family: "IBM Plex Mono", "SFMono-Regular", monospace; + font-family: "IBM Plex Mono", SFMono-Regular, monospace; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/globals.css` around lines 5026 - 5029, The font-family declaration in .numeric-stepper input uses inconsistent quoting that trips Stylelint; update it so only the multi-word family is quoted and single-word families/generic families are unquoted: keep "IBM Plex Mono" quoted (or switch to single quotes if project prefers), remove the quotes around SFMono-Regular and monospace in the font-family for .numeric-stepper input to satisfy the font-family-name-quotes rule.src/app/api/opencode/start/route.ts-7-13 (1)
7-13:⚠️ Potential issue | 🟡 MinorValidate
provideragainst allowedOpencodeProvidervalues.The schema validates
provideras any string, then casts it toOpencodeProvideron line 21. This bypasses input validation—invalid provider names will pass Zod validation but fail at runtime inbuildOpencodeConfig, violating the "Validate external I/O" guideline.♻️ Suggested improvement using enum validation
const startRequestSchema = z.object({ - provider: z.string(), + provider: z.enum(["anthropic", "openai", "google", "azure", "bedrock", "cohere", "groq", "mistral", "perplexity", "openrouter", "local"]), apiKey: z.string(), model: z.string().optional(), baseUrl: z.string().optional(), logLevel: z.enum(["debug", "info", "warn", "error"]).optional(), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/opencode/start/route.ts` around lines 7 - 13, The startRequestSchema currently accepts any string for provider and then casts to OpencodeProvider later; update startRequestSchema to validate provider against the allowed OpencodeProvider values (e.g., use z.nativeEnum(OpencodeProvider) or z.enum([...]) matching the OpencodeProvider members) so invalid providers are rejected by Zod; then remove the unsafe cast where the code converts provider to OpencodeProvider before calling buildOpencodeConfig and rely on the schema's inferred type to pass a valid provider into buildOpencodeConfig.src/lib/opencode/modelFetcher.ts-237-250 (1)
237-250:⚠️ Potential issue | 🟡 MinorCohere: API key is not validated, returns
valid: trueunconditionally.This function doesn't make any network request, so invalid Cohere keys will pass validation. The user won't discover the key is invalid until they try to use it.
Consider making a lightweight API call to verify the key, or at minimum, document this limitation in the error message.
🔧 Option: validate with a lightweight endpoint
async function fetchCohereModels(apiKey: string): Promise<ValidationResult> { - // Cohere doesn't have a public models list endpoint - // Return default models and assume key is valid if format is correct + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 10000); + + try { + // Use tokenize endpoint for lightweight validation + const response = await fetch("https://api.cohere.ai/v1/tokenize", { + method: "POST", + headers: { + Authorization: `Bearer ${apiKey}`, + "Content-Type": "application/json", + }, + body: JSON.stringify({ text: "test" }), + signal: controller.signal, + }); + + clearTimeout(timeout); + + if (!response.ok) { + if (response.status === 401) { + return { valid: false, error: "Invalid Cohere API key" }; + } + return { valid: false, error: `Cohere API error: ${response.status}` }; + } + } catch (err) { + clearTimeout(timeout); + if (err instanceof Error && err.name === "AbortError") { + return { valid: false, error: "Cohere API request timed out" }; + } + return { valid: false, error: "Failed to validate Cohere API key" }; + } + return { valid: true, models: [ "command-r-plus", "command-r", "command", "command-light", "command-nightly", ], }; }src/lib/opencode/modelFetcher.ts-315-351 (1)
315-351:⚠️ Potential issue | 🟡 MinorOpenRouter: API key is not used in the request, so validation doesn't verify key validity.
The models endpoint is public and doesn't require authentication. This means any string (even invalid keys) will pass validation.
🔧 Add Authorization header to validate the key
const response = await fetch("https://openrouter.ai/api/v1/models", { + headers: { + Authorization: `Bearer ${apiKey}`, + }, signal: controller.signal, });Note: Even if the endpoint doesn't require auth, sending the key may trigger a 401 for invalid keys (verify with OpenRouter docs).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/opencode/modelFetcher.ts` around lines 315 - 351, The fetchOpenRouterModels function currently never uses the apiKey so validation can't fail; update the fetch call in fetchOpenRouterModels to send an Authorization header (e.g., "Authorization: Bearer <apiKey>") and any required Content-Type, then handle 401/403 responses explicitly by returning { valid: false, error: "Invalid OpenRouter API key" } (or similar) when response.status is 401/403 before proceeding to parse JSON; keep the existing timeout/abort handling and existing model filtering logic intact.src/components/opencode-settings.tsx-87-95 (1)
87-95:⚠️ Potential issue | 🟡 MinorDon't mask a
/statusfailure as"stopped".The catch at Line 93 turns a broken status request into the same state as a genuinely stopped server, and
onStatusChangenever receives the failure. Surface an error state or log diagnostic context instead. As per coding guidelines, "No silent failures - Every catch path must either return a structured error, set a visible UI error or stale state, or log diagnostic context intentionally".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/opencode-settings.tsx` around lines 87 - 95, The catch is masking network/HTTP failures by setting server status to "stopped" and never surfacing the error; update the catch on checkServerStatus() to record and surface diagnostics instead — e.g., set a distinct error state via setServerStatus({ status: "error", message: err.message }) or call a new setServerError, and call onStatusChange?.({ status: "error", error: err }) so callers receive the failure; also log the error (processLogger/console.error) for diagnostic context. Ensure you reference checkServerStatus, setServerStatus and onStatusChange when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4f678320-6569-4ca3-8cc0-708cf267e35d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (35)
README.mdnext-env.d.tspackage.jsonsrc/app/api/code-suggestions/route.tssrc/app/api/implement-node/route.tssrc/app/api/opencode/agent/route.tssrc/app/api/opencode/mcp/route.tssrc/app/api/opencode/permissions/route.tssrc/app/api/opencode/restart/route.tssrc/app/api/opencode/sessions/[id]/route.tssrc/app/api/opencode/sessions/route.tssrc/app/api/opencode/start/route.tssrc/app/api/opencode/status/route.tssrc/app/api/opencode/stop/route.tssrc/app/api/terminal/sessions/[sessionId]/route.tssrc/app/api/terminal/sessions/route.test.tssrc/app/api/terminal/sessions/route.tssrc/app/globals.csssrc/app/layout.tsxsrc/components/blueprint-workbench.test.tsxsrc/components/blueprint-workbench.tsxsrc/components/opencode-settings.tsxsrc/lib/opencode/agent.test.tssrc/lib/opencode/agent.tssrc/lib/opencode/api-key-validator.tsxsrc/lib/opencode/client.tssrc/lib/opencode/config.test.tssrc/lib/opencode/config.tssrc/lib/opencode/index.tssrc/lib/opencode/modelFetcher.test.tssrc/lib/opencode/modelFetcher.tssrc/lib/opencode/server.test.tssrc/lib/opencode/server.tssrc/lib/opencode/types.tssrc/lib/server/terminal-sessions.ts
💤 Files with no reviewable changes (1)
- src/app/layout.tsx
| /// <reference types="next" /> | ||
| /// <reference types="next/image-types/global" /> | ||
| import "./.next/types/routes.d.ts"; | ||
| import "./.next/dev/types/routes.d.ts"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if the dev-specific types path exists and verify Next.js type generation
# Check if the dev types path exists
if [ -f ".next/dev/types/routes.d.ts" ]; then
echo "✓ Dev types path exists"
else
echo "✗ Dev types path does NOT exist"
fi
# Check if production types path exists
if [ -f ".next/types/routes.d.ts" ]; then
echo "✓ Production types path exists"
else
echo "✗ Production types path does NOT exist"
fi
# Check Next.js configuration for TypeScript settings
if [ -f "next.config.js" ] || [ -f "next.config.mjs" ] || [ -f "next.config.ts" ]; then
echo "Next.js config found:"
fd -e js -e mjs -e ts '^next\.config\.' --max-depth 1 --exec cat {}
fi
# Check if this file was recently auto-generated or manually edited
echo -e "\nRecent commits touching next-env.d.ts:"
git log --oneline --follow -5 -- next-env.d.tsRepository: nehraa/CodeFlow
Length of output: 499
🏁 Script executed:
# See the git diff for this file in the current commit
git diff HEAD -- next-env.d.tsRepository: nehraa/CodeFlow
Length of output: 41
🏁 Script executed:
# Check the file content
cat -n next-env.d.tsRepository: nehraa/CodeFlow
Length of output: 351
🏁 Script executed:
# Check if there are any TypeScript or build errors related to types
if [ -f "tsconfig.json" ]; then
echo "=== tsconfig.json ==="
cat tsconfig.json
fiRepository: nehraa/CodeFlow
Length of output: 975
🏁 Script executed:
# Search for references to .next/types or .next/dev/types in the codebase
rg '\.next/(dev/)?types' --type ts --type tsx --type js --type jsxRepository: nehraa/CodeFlow
Length of output: 86
🏁 Script executed:
# Check git history to see what changed
git log --oneline -10 -- next-env.d.tsRepository: nehraa/CodeFlow
Length of output: 124
🏁 Script executed:
# See the actual previous version of the file
git show HEAD~1:next-env.d.ts 2>/dev/null || echo "No previous version found"Repository: nehraa/CodeFlow
Length of output: 84
🏁 Script executed:
# Check if there are any build or test failures
if [ -f "package.json" ]; then
echo "=== Build scripts in package.json ==="
rg '"(build|test)"' package.json -A 1
fiRepository: nehraa/CodeFlow
Length of output: 207
🏁 Script executed:
# Search for references to the routes types path
rg 'routes\.d\.ts|\.next.*types' -A 2 -B 2 --max-count=10Repository: nehraa/CodeFlow
Length of output: 497
🏁 Script executed:
# Check if the change is a new file or a modification
git log --format="%H %s" -1 -- next-env.d.ts | head -1
git show --name-status c44e827 | grep next-envRepository: nehraa/CodeFlow
Length of output: 173
🏁 Script executed:
# Verify that Next.js with typedRoutes:true generates types in the .next/dev/types path
# Look at Next.js documentation and check what's the standard behavior
web_search
Next.js typedRoutes typed routes .next dev types path locationRepository: nehraa/CodeFlow
Length of output: 155
🏁 Script executed:
# Check if there's any documented reason for including next-env.d.ts in version control
rg 'next-env' .gitignore README.md --max-count=5 -A 2Repository: nehraa/CodeFlow
Length of output: 41
🌐 Web query:
Next.js typedRoutes routes.d.ts .next/dev/types path generation
💡 Result:
Next.js generates the routes.d.ts file for typedRoutes as part of its type generation process when the typedRoutes option is enabled in next.config.js. This provides type-safe routes, including a Route type and global helpers like PageProps and LayoutProps. To enable: In next.config.js: const nextConfig = { typedRoutes: true, } module.exports = nextConfig The routes.d.ts file is automatically generated: - In development: .next/dev/types/routes.d.ts - In production/build: .next/types/routes.d.ts Generation occurs during: - next dev - next build - Or manually via next typegen (generates types without full build, useful for CI/type-checking) The next-env.d.ts file (in project root) imports the appropriate routes.d.ts path and should be gitignored as it toggles between dev/prod paths. Ensure tsconfig.json includes: "include": [ "next-env.d.ts", ".next/types//.ts", "/.ts", "/*.tsx" ] Usage example for typed routes: const route: Route<'/blog/[slug]'> = '/blog/my-post'; // Provides autocomplete, validation for Link href, navigation methods (push, redirect, etc.), supports dynamic segments. This feature requires TypeScript and works with App Router (and some Pages Router support). For full route tree scanning from app/ directory, parallel routes (@slot), redirects/rewrites.
Citations:
- 1: next-env.d.ts toggles between .next/dev/types/routes.d.ts and .next/types/routes.d.ts even with typedRoutes: false vercel/next.js#86001
- 2: feat: automatically generate route types vercel/next.js#81396
- 3: How to use generated "Route<T>" type (from "typedRoutes") with dynamic routes vercel/next.js#69018
- 4: https://nextjs.org/docs/app/api-reference/config/next-config-js/typedRoutes
- 5: https://nextjs.org/docs/app/api-reference/next-config-js/typedRoutes
- 6: https://nextjs.org/docs/basic-features/typescript
- 7: https://nextjs.org/docs-wip/app/api-reference/config/typescript
- 8: https://nextjs.org/docs/api-reference/cli
🏁 Script executed:
# Check .gitignore to see if .next is ignored
cat .gitignore | grep -E '^\/?\.next'Repository: nehraa/CodeFlow
Length of output: 64
🏁 Script executed:
# Check if next-env.d.ts should be in version control by looking at Next.js best practices
# and checking what the official Next.js template does
rg 'next-env' --type md README.md docs/ -A 2 -B 2 2>/dev/null || echo "No markdown docs found"Repository: nehraa/CodeFlow
Length of output: 81
🏁 Script executed:
# Check if next-env.d.ts is already in .gitignore
cat .gitignore | head -20Repository: nehraa/CodeFlow
Length of output: 313
🏁 Script executed:
# Check the actual commit to see if next-env.d.ts was added or modified
git show --stat c44e827Repository: nehraa/CodeFlow
Length of output: 21096
🏁 Script executed:
# Verify Next.js best practices - check if next-env.d.ts should be gitignored
# by looking at Next.js starter templates
rg 'next-env\.d\.ts' --type md --type txt 2>/dev/null || echo "No references found"Repository: nehraa/CodeFlow
Length of output: 78
🏁 Script executed:
# Verify the original state - check git history before this commit
git show c44e827^:next-env.d.ts 2>/dev/null || echo "File did not exist in previous commit (expected - this is the first)"Repository: nehraa/CodeFlow
Length of output: 127
🏁 Script executed:
# Check if there's any documentation or comments about why next-env.d.ts was committed
rg 'next-env' --max-count=10 -B 3 -A 3Repository: nehraa/CodeFlow
Length of output: 263
🏁 Script executed:
# Verify tsconfig includes both paths
grep -A 5 '"include"' tsconfig.json | head -20Repository: nehraa/CodeFlow
Length of output: 177
Remove next-env.d.ts from version control; it should be auto-generated per environment.
This file should not be committed to git. Per Next.js conventions, next-env.d.ts is auto-generated during next dev and next build with environment-specific paths:
- Development:
.next/dev/types/routes.d.ts - Production/Build:
.next/types/routes.d.ts
By committing it with a hardcoded dev path, production builds will fail when Next.js generates .next/types/routes.d.ts but the committed file tries to import from .next/dev/types/routes.d.ts (which doesn't exist in that build).
Fix: Remove next-env.d.ts from git history and add it to .gitignore. Let Next.js auto-generate it with the correct paths for each environment. The tsconfig.json already correctly includes both possible paths.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@next-env.d.ts` at line 3, Remove the checked-in next-env.d.ts file and stop
committing it: delete the file from the repository (or remove it from git
history if already merged) and add an entry for next-env.d.ts to .gitignore so
Next.js can auto-generate it per environment; verify tsconfig.json remains
configured to include both ".next/dev/types/routes.d.ts" and
".next/types/routes.d.ts" so the correct path is used for dev vs build.
| async function getOrCreateSession(serverUrl: string, agentType: string): Promise<string> { | ||
| // List existing sessions | ||
| const listRes = await fetch(`${serverUrl}/session?limit=1`, { | ||
| headers: { "Content-Type": "application/json" }, | ||
| }); | ||
|
|
||
| if (listRes.ok) { | ||
| const sessions = await listRes.json(); | ||
| if (sessions.length > 0) { | ||
| return sessions[0].id; | ||
| } | ||
| } | ||
|
|
||
| // Create a new session | ||
| const createRes = await fetch(`${serverUrl}/session`, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ | ||
| title: `CodeFlow ${agentType} session`, | ||
| agent: agentType, | ||
| }), | ||
| }); |
There was a problem hiding this comment.
Apply the timeout budget to session discovery and creation too.
sendToOpencode() only aborts the /message request. If either fetch here stalls, the API route can hang indefinitely and the retry loop never gets a chance to run.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/opencode/agent.ts` around lines 182 - 203, getOrCreateSession
currently issues two fetches (the list and the create) without a timeout, so a
stalled request can hang the route; update getOrCreateSession to apply the same
timeout/AbortController budget used by sendToOpencode: create an AbortController
(or accept an incoming AbortSignal/timeoutMs param), pass controller.signal into
both fetch calls to `${serverUrl}/session?limit=1` and `${serverUrl}/session`,
and start a single timer that calls controller.abort() after the configured
timeout; ensure you clear the timer after a response or on error so the
controller isn't left running.
| this.process = spawn("opencode", args, { | ||
| env, | ||
| stdio: ["ignore", "pipe", "pipe"], | ||
| }); | ||
|
|
||
| const serverUrl = await this.waitForServerStart(); | ||
| this.url = serverUrl; | ||
| this.setStatus("running"); | ||
| this.startHealthChecks(); | ||
|
|
||
| this.process.on("exit", (code: number | null) => { | ||
| this.handleExit(code); | ||
| }); | ||
|
|
||
| this.process.on("error", (err: Error) => { | ||
| this.handleError(err); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Node.js child_process/EventEmitter semantics, can a ChildProcess emit "error" or "exit" before listeners are attached, and what happens if an "error" event is emitted with no listener?
💡 Result:
Yes, a ChildProcess can emit "error" or "exit" events before listeners are attached, particularly if the emission occurs synchronously during spawn (e.g., spawn failure sets pid to undefined and emits 'error' asynchronously but quickly). EventEmitter.emit is synchronous, so if it happens after spawn returns but before .on('error'), the event is lost. The 'spawn' event fires only on success; on failure, 'error' is emitted instead. If an "error" event is emitted with no listener on any EventEmitter (including ChildProcess), Node.js throws the error synchronously, prints a stack trace to stderr, and crashes the process with exit code 1. This is the standard unhandled 'error' event behavior to prevent silent failures.
Citations:
- 1: https://nodejs.org/api/child_process.html
- 2: https://nodejs.org/download/release/v18.20.4/docs/api/events.html
- 3: https://www.thenodebook.com/async-patterns/eventemitter-internals
- 4: https://nodejs.org/api/events.html
- 5: child_process.spawn() uncatchable error nodejs/node#3523
Register child-process failure handlers before awaiting startup.
spawn() returns before the "error" and "exit" handlers are registered (lines 71–75). If the child fails to start (e.g., command not found, permission denied), the "error" event is emitted asynchronously but before these handlers are attached. An unhandled "error" event causes Node.js to crash the process immediately instead of allowing the application to handle the failure gracefully. Move the handlers before await this.waitForServerStart() to catch early spawn failures.
Also applies to: 157–212
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/opencode/server.ts` around lines 61 - 77, Register the child-process
handlers immediately after calling spawn and before awaiting startup: after
assigning this.process = spawn(...) attach this.process.on("exit",
this.handleExit.bind(this)) and this.process.on("error",
this.handleError.bind(this)) (or equivalent arrow wrappers) prior to awaiting
this.waitForServerStart(), then continue with setting this.url,
this.setStatus("running"), and this.startHealthChecks(); apply the same fix to
the other spawn usage in this file (the second spawn block around the later
spawn region) so early "error" events are not missed.
| export const PROVIDER_CONFIGS: Record<OpencodeProvider, ProviderModelConfig> = { | ||
| anthropic: { | ||
| provider: "anthropic", | ||
| defaultModel: "claude-sonnet-4.5", | ||
| apiKeyEnvVar: "ANTHROPIC_API_KEY", | ||
| apiKeyFormat: /^sk-ant-api03-[\w-]+$/, | ||
| }, | ||
| openai: { | ||
| provider: "openai", | ||
| defaultModel: "gpt-5.4-mini", | ||
| apiKeyEnvVar: "OPENAI_API_KEY", | ||
| // More specific: OpenAI keys are longer (20+ chars after "sk-") | ||
| apiKeyFormat: /^sk-[a-zA-Z0-9]{20,}$/, | ||
| }, | ||
| google: { | ||
| provider: "google", | ||
| defaultModel: "gemini-2.5-flash", | ||
| apiKeyEnvVar: "GOOGLE_API_KEY", | ||
| // Google API keys don't have a consistent prefix; skip auto-detection | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What are the current default model names for Anthropic Claude, OpenAI GPT, and Google Gemini APIs as of 2025?
💡 Result:
As of early 2026 (today's date: 2026-04-04), the current flagship/default models for the APIs, based on official documentation recommending them for starting or complex tasks, are: - Anthropic Claude API: claude-opus-4-6 (recommended to start with for most complex tasks; latest Opus model) - OpenAI GPT API: gpt-5.4 (flagship model explicitly recommended: "If you're not sure where to start, use gpt-5.4") - Google Gemini API: gemini-2.5-pro (most advanced stable model for complex tasks; featured as high-capability model)
Citations:
- 1: https://docs.claude.com/en/docs/about-claude/models/whats-new-claude-4-5
- 2: https://platform.claude.com/docs/en/about-claude/models/overview
- 3: https://docs.anthropic.com/claude/reference/input-and-output-sizes
- 4: https://platform.openai.com/docs/guides/gpt/gpt-models
- 5: https://developers.openai.com/api/docs/models
- 6: https://ai.google.dev/gemini-api/docs/models
- 7: https://cloud.google.com/vertex-ai/generative-ai/docs/models
Update default model names to match current available models.
The default model names are incorrect and will cause runtime failures if used as fallbacks:
- Line 82: Use
claude-opus-4-6instead ofclaude-sonnet-4.5 - Line 88: Use
gpt-5.4instead ofgpt-5.4-mini(no such variant currently exists) - Line 95: Use
gemini-2.5-proinstead ofgemini-2.5-flash
When model discovery fails, these defaults will be requested from the APIs and fail with "model not found" errors.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/opencode/types.ts` around lines 79 - 98, Update the defaultModel
entries in the PROVIDER_CONFIGS constant to valid current model names: in the
anthropic config (PROVIDER_CONFIGS.anthropic) change "claude-sonnet-4.5" to
"claude-opus-4-6"; in the openai config (PROVIDER_CONFIGS.openai) change
"gpt-5.4-mini" to "gpt-5.4"; and in the google config (PROVIDER_CONFIGS.google)
change "gemini-2.5-flash" to "gemini-2.5-pro"; keep the rest of each
ProviderModelConfig (provider, apiKeyEnvVar, apiKeyFormat) intact and ensure the
updated strings conform to the ProviderModelConfig type.
|
@claude[agent] REsolve every single comment there is on this pr every singel one |
feat: OpenCode provider fix + API key validation & model discovery
Summary
Fixed critical OpenCode provider selection bugs and added automatic API key validation with model discovery.
Changes
Part 1: Provider Selection Override Fix
Commits:
Part 2: API Key Validation & Model Discovery
Commits:
Files Created
Files Modified
Testing
User Impact
Summary by CodeRabbit
New Features
Documentation
Style
Tests