Skip to content

Review: OpenCode runtime integration PR#142

Merged
arul28 merged 10 commits intomainfrom
ade/opencode-integration-03ef5ca8
Apr 8, 2026
Merged

Review: OpenCode runtime integration PR#142
arul28 merged 10 commits intomainfrom
ade/opencode-integration-03ef5ca8

Conversation

@arul28
Copy link
Copy Markdown
Owner

@arul28 arul28 commented Apr 8, 2026

@copilot review requested on the OpenCode runtime integration PR. No code changes were made — this PR records the review findings as a comment.

Review summary

  • Breaking rename ("unified""opencode") in persisted session/orchestrator data: two migration shims exist in orchestratorQueries.ts and agentChatService.ts, but full read-path coverage needs verification.
  • runOpenCodeTask silently drops permissionMode and auth: RunOpenCodePromptArgs has no permissionMode field, so callers passing "read-only" (e.g. orchestrator coordinator) get that constraint ignored. Should either forward via mapPermissionModeToOpenCodeAgent or document the omission as intentional.
  • runCodexTask temp dir error lacks context on mkdtempSync failure — wrap with a descriptive message.
  • agentChatService.ts at 13,709 lines: new OpenCodeRuntime session-lifecycle logic belongs in its own module (cf. compactionEngine.ts extraction pattern).

Positives noted

  • toolExposurePolicy.ts score-based heuristic for frontend tool gating is clean and testable.
  • executableTool.ts factory, cliMcpConfig.ts normalization, and runtimeMessageTypes.ts decoupling are all well-scoped.
  • Extracted tool files (globSearch, grepSearch, readFileRange) with 900+ lines of new tests is the right direction.
  • sdkProvider/sdkModelIdproviderRoute/providerModelId rename is complete with no orphaned references.
  • settled flag in runCommand correctly prevents double-resolution on timeout + close races.

arul28 and others added 8 commits April 7, 2026 20:06
Implement local provider discovery and inspection for ollama, lmstudio and vllm: normalize project config for local providers, probe endpoints, inspect loaded models, infer capabilities and harness profiles, and cache results with TTL and explicit cache reset. Integrate discovered local models into AI integration: replace dynamic local descriptors, build runtimeConnections (cli/api-key/local/openrouter), expose runtimeConnections in the AiIntegrationStatus, and reset local detection when forcing refresh. Improve provider resolution to honor saved preferred local model IDs and to require explicit selection when multiple models are loaded. Add tests covering fallback endpoint detection and preferred-model behavior. Add temporary artifact cleanup: call cleanupStaleTempArtifacts on startup and ensure screenshot tool removes temp dirs after use. Misc: refactor authDetector and localModelDiscovery APIs, add inspection cache clearing, and wire changes through services and tests.
* Polish local model chat UI and share provider helpers

Export LOCAL_PROVIDER_LABELS and local model id parsers from modelRegistry;
dedupe labels in agentChatService and aiIntegrationService. Merge CLI and
local runtime banners in AgentChatPane, reset unified permission mode when
harness recommendations change, and fix local placeholder sdkModelId. Settings
and model options use the shared helpers.

Co-authored-by: Arul Sharma <arul28@users.noreply.github.com>

* Address PR review: local permission descriptors and unified mode setter

Add getModelDescriptorForPermissionMode for harness decisions when
getModelById is undefined (e.g. ollama/auto). Use it in AgentChatPane for
prevModelRef, model switch snapshots, and session create. Consolidate
applyModelSelectionSnapshot to a single setUnifiedPermissionMode call.

Co-authored-by: Arul Sharma <arul28@users.noreply.github.com>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Arul Sharma <arul28@users.noreply.github.com>
When a chat or PTY session is created from the Lanes tab, the Work tab's
session list cache wasn't invalidated, so the new session wouldn't appear
in the Work tab sidebar until the cache TTL expired. The same issue existed
in reverse (Work tab → Lanes tab).

Call invalidateSessionListCache() in all session creation paths so both
views fetch fresh data on their next refresh cycle.
The 75s idle watchdog fires during long-running tool calls (e.g. Agent
subprocesses doing codebase research) where the SDK stream yields no
events while waiting for tool results. 300s matches the turn timeout
and prevents false interruptions on legitimate heavy tool use.
The time-based idle watchdog (previously 75s, then 300s) cannot
distinguish between a stuck stream and legitimate long-running tool
execution where the SDK emits no events while waiting for results.
Remove it entirely — users can manually interrupt if something is
genuinely stuck, and the turn-level timeout on programmatic run
sessions still provides a safety net for non-interactive use.
…election

Replace unified executor/session processor with direct OpenCode SDK integration.
Consolidate provider registry, model selector, and orchestrator adapter into
streamlined provider-based architecture. Add new tool implementations, tests,
and runtime message types.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 8, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
ade Ready Ready Preview, Comment Apr 8, 2026 1:48am

@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented Apr 8, 2026

@codex review

@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented Apr 8, 2026

@copilot review

@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented Apr 8, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 8, 2026

Important

Review skipped

Too many files!

This PR contains 238 files, which is 88 over the limit of 150.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5a0a0004-03c6-42f7-8cd1-dead757956a6

📥 Commits

Reviewing files that changed from the base of the PR and between 250d32a and 83bded7.

⛔ Files ignored due to path filters (15)
  • .ade/cto/identity.yaml is excluded by !.ade/**
  • CHANGELOG.md is excluded by !*.md
  • apps/desktop/package-lock.json is excluded by !**/package-lock.json, !**/package-lock.json
  • apps/mcp-server/package-lock.json is excluded by !**/package-lock.json, !**/package-lock.json
  • changelog/v1.0.13.mdx is excluded by !changelog/**
  • docs.json is excluded by !docs.json
  • docs/architecture/AI_INTEGRATION.md is excluded by !docs/**
  • docs/architecture/CONFIGURATION.md is excluded by !docs/**
  • docs/architecture/DATA_MODEL.md is excluded by !docs/**
  • docs/architecture/SECURITY_AND_PRIVACY.md is excluded by !docs/**
  • docs/features/AGENTS.md is excluded by !docs/**
  • docs/features/CHAT.md is excluded by !docs/**
  • docs/features/ONBOARDING_AND_SETTINGS.md is excluded by !docs/**
  • docs/features/PULL_REQUESTS.md is excluded by !docs/**
  • docs/features/TERMINALS_AND_SESSIONS.md is excluded by !docs/**
📒 Files selected for processing (238)
  • apps/desktop/package.json
  • apps/desktop/scripts/verify-ai-sdks.cjs
  • apps/desktop/src/main/main.ts
  • apps/desktop/src/main/packagedRuntimeSmoke.ts
  • apps/desktop/src/main/services/ai/adeProviderRegistry.ts
  • apps/desktop/src/main/services/ai/agentExecutor.ts
  • apps/desktop/src/main/services/ai/aiIntegrationService.test.ts
  • apps/desktop/src/main/services/ai/aiIntegrationService.ts
  • apps/desktop/src/main/services/ai/authDetector.test.ts
  • apps/desktop/src/main/services/ai/authDetector.ts
  • apps/desktop/src/main/services/ai/claudeModelUtils.ts
  • apps/desktop/src/main/services/ai/claudeRuntimeProbe.test.ts
  • apps/desktop/src/main/services/ai/claudeRuntimeProbe.ts
  • apps/desktop/src/main/services/ai/cliExecutableResolver.ts
  • apps/desktop/src/main/services/ai/cliMcpConfig.ts
  • apps/desktop/src/main/services/ai/compactionEngine.test.ts
  • apps/desktop/src/main/services/ai/compactionEngine.ts
  • apps/desktop/src/main/services/ai/localModelDiscovery.test.ts
  • apps/desktop/src/main/services/ai/localModelDiscovery.ts
  • apps/desktop/src/main/services/ai/middleware.ts
  • apps/desktop/src/main/services/ai/modelsDevService.ts
  • apps/desktop/src/main/services/ai/providerResolver.test.ts
  • apps/desktop/src/main/services/ai/providerResolver.ts
  • apps/desktop/src/main/services/ai/providerTaskRunner.ts
  • apps/desktop/src/main/services/ai/toolExposurePolicy.ts
  • apps/desktop/src/main/services/ai/tools/ctoOperatorTools.test.ts
  • apps/desktop/src/main/services/ai/tools/ctoOperatorTools.ts
  • apps/desktop/src/main/services/ai/tools/editFile.ts
  • apps/desktop/src/main/services/ai/tools/executableTool.ts
  • apps/desktop/src/main/services/ai/tools/globSearch.test.ts
  • apps/desktop/src/main/services/ai/tools/globSearch.ts
  • apps/desktop/src/main/services/ai/tools/grepSearch.test.ts
  • apps/desktop/src/main/services/ai/tools/grepSearch.ts
  • apps/desktop/src/main/services/ai/tools/index.ts
  • apps/desktop/src/main/services/ai/tools/linearTools.ts
  • apps/desktop/src/main/services/ai/tools/memoryTools.ts
  • apps/desktop/src/main/services/ai/tools/readFileRange.test.ts
  • apps/desktop/src/main/services/ai/tools/readFileRange.ts
  • apps/desktop/src/main/services/ai/tools/systemPrompt.test.ts
  • apps/desktop/src/main/services/ai/tools/systemPrompt.ts
  • apps/desktop/src/main/services/ai/tools/universalTools.test.ts
  • apps/desktop/src/main/services/ai/tools/universalTools.ts
  • apps/desktop/src/main/services/ai/tools/webFetch.ts
  • apps/desktop/src/main/services/ai/tools/webSearch.ts
  • apps/desktop/src/main/services/ai/tools/workflowTools.ts
  • apps/desktop/src/main/services/ai/unifiedExecutor.ts
  • apps/desktop/src/main/services/automations/automationPlannerService.test.ts
  • apps/desktop/src/main/services/automations/automationService.ts
  • apps/desktop/src/main/services/chat/agentChatService.test.ts
  • apps/desktop/src/main/services/chat/agentChatService.ts
  • apps/desktop/src/main/services/chat/chatTextBatching.test.ts
  • apps/desktop/src/main/services/chat/chatTextBatching.ts
  • apps/desktop/src/main/services/chat/runtimeMessageTypes.ts
  • apps/desktop/src/main/services/config/projectConfigService.ts
  • apps/desktop/src/main/services/context/contextDocBuilder.test.ts
  • apps/desktop/src/main/services/context/contextDocBuilder.ts
  • apps/desktop/src/main/services/context/contextDocService.test.ts
  • apps/desktop/src/main/services/context/contextDocService.ts
  • apps/desktop/src/main/services/cto/ctoStateService.test.ts
  • apps/desktop/src/main/services/cto/ctoStateService.ts
  • apps/desktop/src/main/services/cto/linearClient.test.ts
  • apps/desktop/src/main/services/cto/linearClient.ts
  • apps/desktop/src/main/services/cto/linearCredentialService.test.ts
  • apps/desktop/src/main/services/cto/linearCredentialService.ts
  • apps/desktop/src/main/services/cto/workerAdapterRuntimeService.test.ts
  • apps/desktop/src/main/services/cto/workerHeartbeatService.ts
  • apps/desktop/src/main/services/git/git.test.ts
  • apps/desktop/src/main/services/git/git.ts
  • apps/desktop/src/main/services/ipc/registerIpc.ts
  • apps/desktop/src/main/services/memory/embeddingWorkerService.test.ts
  • apps/desktop/src/main/services/memory/episodicSummaryService.ts
  • apps/desktop/src/main/services/memory/hybridSearchService.test.ts
  • apps/desktop/src/main/services/memory/hybridSearchService.ts
  • apps/desktop/src/main/services/memory/knowledgeCaptureService.test.ts
  • apps/desktop/src/main/services/memory/knowledgeCaptureService.ts
  • apps/desktop/src/main/services/memory/memoryBriefingService.ts
  • apps/desktop/src/main/services/memory/memoryFilesService.test.ts
  • apps/desktop/src/main/services/memory/memoryFilesService.ts
  • apps/desktop/src/main/services/memory/memoryService.test.ts
  • apps/desktop/src/main/services/memory/memoryService.ts
  • apps/desktop/src/main/services/memory/missionMemoryLifecycleService.ts
  • apps/desktop/src/main/services/memory/proceduralLearningService.test.ts
  • apps/desktop/src/main/services/memory/proceduralLearningService.ts
  • apps/desktop/src/main/services/memory/skillRegistryService.test.ts
  • apps/desktop/src/main/services/memory/skillRegistryService.ts
  • apps/desktop/src/main/services/memory/unifiedMemoryService.ts
  • apps/desktop/src/main/services/missions/missionPreflightService.test.ts
  • apps/desktop/src/main/services/missions/missionPreflightService.ts
  • apps/desktop/src/main/services/missions/missionService.ts
  • apps/desktop/src/main/services/opencode/openCodeBinaryManager.ts
  • apps/desktop/src/main/services/opencode/openCodeInventory.ts
  • apps/desktop/src/main/services/opencode/openCodeModelCatalog.ts
  • apps/desktop/src/main/services/opencode/openCodeRuntime.ts
  • apps/desktop/src/main/services/orchestrator/aiOrchestratorService.test.ts
  • apps/desktop/src/main/services/orchestrator/aiOrchestratorService.ts
  • apps/desktop/src/main/services/orchestrator/baseOrchestratorAdapter.test.ts
  • apps/desktop/src/main/services/orchestrator/coordinatorAgent.test.ts
  • apps/desktop/src/main/services/orchestrator/coordinatorAgent.ts
  • apps/desktop/src/main/services/orchestrator/coordinatorTools.test.ts
  • apps/desktop/src/main/services/orchestrator/coordinatorTools.ts
  • apps/desktop/src/main/services/orchestrator/executionPolicy.test.ts
  • apps/desktop/src/main/services/orchestrator/executionPolicy.ts
  • apps/desktop/src/main/services/orchestrator/knowledgeConflictsBrowserCto.test.ts
  • apps/desktop/src/main/services/orchestrator/orchestratorQueries.ts
  • apps/desktop/src/main/services/orchestrator/orchestratorService.test.ts
  • apps/desktop/src/main/services/orchestrator/orchestratorService.ts
  • apps/desktop/src/main/services/orchestrator/orchestratorSmoke.test.ts
  • apps/desktop/src/main/services/orchestrator/permissionMapping.ts
  • apps/desktop/src/main/services/orchestrator/planningFlowAndHandoffs.test.ts
  • apps/desktop/src/main/services/orchestrator/planningGapsFixes.test.ts
  • apps/desktop/src/main/services/orchestrator/promptInspector.test.ts
  • apps/desktop/src/main/services/orchestrator/promptInspector.ts
  • apps/desktop/src/main/services/orchestrator/providerOrchestratorAdapter.ts
  • apps/desktop/src/main/services/orchestrator/recoveryService.ts
  • apps/desktop/src/main/services/orchestrator/unifiedOrchestratorAdapter.test.ts
  • apps/desktop/src/main/services/orchestrator/workerDeliveryService.ts
  • apps/desktop/src/main/services/orchestrator/workerTracking.ts
  • apps/desktop/src/main/services/orchestrator/worktreeIsolation.test.ts
  • apps/desktop/src/main/services/prs/prIssueResolver.test.ts
  • apps/desktop/src/main/services/pty/ptyService.test.ts
  • apps/desktop/src/main/services/pty/ptyService.ts
  • apps/desktop/src/main/services/runtime/tempCleanupService.test.ts
  • apps/desktop/src/main/services/runtime/tempCleanupService.ts
  • apps/desktop/src/main/services/sessions/sessionService.test.ts
  • apps/desktop/src/main/services/sessions/sessionService.ts
  • apps/desktop/src/main/services/state/kvDb.ts
  • apps/desktop/src/main/services/sync/syncRemoteCommandService.ts
  • apps/desktop/src/main/services/sync/syncService.ts
  • apps/desktop/src/main/utils/terminalSessionSignals.test.ts
  • apps/desktop/src/main/utils/terminalSessionSignals.ts
  • apps/desktop/src/preload/global.d.ts
  • apps/desktop/src/preload/preload.ts
  • apps/desktop/src/renderer/browserMock.ts
  • apps/desktop/src/renderer/components/app/FeedbackReporterModal.tsx
  • apps/desktop/src/renderer/components/app/TopBar.tsx
  • apps/desktop/src/renderer/components/automations/RulesTab.tsx
  • apps/desktop/src/renderer/components/automations/components/RuleEditorPanel.tsx
  • apps/desktop/src/renderer/components/chat/AgentChatComposer.test.tsx
  • apps/desktop/src/renderer/components/chat/AgentChatComposer.tsx
  • apps/desktop/src/renderer/components/chat/AgentChatMessageList.test.tsx
  • apps/desktop/src/renderer/components/chat/AgentChatMessageList.tsx
  • apps/desktop/src/renderer/components/chat/AgentChatPane.submit.test.tsx
  • apps/desktop/src/renderer/components/chat/AgentChatPane.tsx
  • apps/desktop/src/renderer/components/chat/ChatCommandMenu.tsx
  • apps/desktop/src/renderer/components/chat/ChatGitToolbar.tsx
  • apps/desktop/src/renderer/components/chat/ChatProposedPlanCard.tsx
  • apps/desktop/src/renderer/components/chat/ChatSubagentStrip.test.tsx
  • apps/desktop/src/renderer/components/chat/ChatSubagentStrip.tsx
  • apps/desktop/src/renderer/components/chat/ChatTerminalDrawer.tsx
  • apps/desktop/src/renderer/components/chat/ChatTurnDiffPanel.tsx
  • apps/desktop/src/renderer/components/chat/ChatWorkLogBlock.tsx
  • apps/desktop/src/renderer/components/chat/chatTranscriptRows.test.ts
  • apps/desktop/src/renderer/components/chat/chatTranscriptRows.ts
  • apps/desktop/src/renderer/components/cto/IdentityEditor.tsx
  • apps/desktop/src/renderer/components/history/eventTaxonomy.ts
  • apps/desktop/src/renderer/components/lanes/LaneGitActionsPane.tsx
  • apps/desktop/src/renderer/components/lanes/LanesPage.tsx
  • apps/desktop/src/renderer/components/lanes/useLaneWorkSessions.test.ts
  • apps/desktop/src/renderer/components/lanes/useLaneWorkSessions.ts
  • apps/desktop/src/renderer/components/missions/CreateMissionDialog.tsx
  • apps/desktop/src/renderer/components/missions/MissionArtifactsTab.tsx
  • apps/desktop/src/renderer/components/missions/MissionControlPage.tsx
  • apps/desktop/src/renderer/components/missions/MissionHeader.tsx
  • apps/desktop/src/renderer/components/missions/MissionSettingsDialog.tsx
  • apps/desktop/src/renderer/components/missions/ModelSelector.tsx
  • apps/desktop/src/renderer/components/missions/PhaseCardEditor.tsx
  • apps/desktop/src/renderer/components/missions/WorkerPermissionsEditor.tsx
  • apps/desktop/src/renderer/components/missions/missionControlViewModel.ts
  • apps/desktop/src/renderer/components/missions/missionHelpers.ts
  • apps/desktop/src/renderer/components/missions/useMissionsStore.ts
  • apps/desktop/src/renderer/components/onboarding/ProjectSetupPage.tsx
  • apps/desktop/src/renderer/components/prs/LanePrPanel.tsx
  • apps/desktop/src/renderer/components/prs/shared/PrResolverLaunchControls.tsx
  • apps/desktop/src/renderer/components/settings/AiFeaturesSection.tsx
  • apps/desktop/src/renderer/components/settings/ContextSection.test.tsx
  • apps/desktop/src/renderer/components/settings/ContextSection.tsx
  • apps/desktop/src/renderer/components/settings/ProvidersSection.test.tsx
  • apps/desktop/src/renderer/components/settings/ProvidersSection.tsx
  • apps/desktop/src/renderer/components/shared/ModelCatalogPanel.tsx
  • apps/desktop/src/renderer/components/shared/ProviderLogos.tsx
  • apps/desktop/src/renderer/components/shared/ProviderModelSelector.tsx
  • apps/desktop/src/renderer/components/shared/UnifiedModelSelector.tsx
  • apps/desktop/src/renderer/components/shared/conflictResolver/ResolverTerminalModal.tsx
  • apps/desktop/src/renderer/components/shared/permissionOptions.test.ts
  • apps/desktop/src/renderer/components/shared/permissionOptions.ts
  • apps/desktop/src/renderer/components/shared/providerModelSelectorGrouping.ts
  • apps/desktop/src/renderer/components/shared/unifiedModelSelectorGrouping.ts
  • apps/desktop/src/renderer/components/terminals/SessionCard.tsx
  • apps/desktop/src/renderer/components/terminals/SessionContextMenu.tsx
  • apps/desktop/src/renderer/components/terminals/SessionInfoPopover.tsx
  • apps/desktop/src/renderer/components/terminals/SessionListPane.tsx
  • apps/desktop/src/renderer/components/terminals/TerminalsPage.tsx
  • apps/desktop/src/renderer/components/terminals/ToolLogos.tsx
  • apps/desktop/src/renderer/components/terminals/WorkViewArea.test.tsx
  • apps/desktop/src/renderer/components/terminals/WorkViewArea.tsx
  • apps/desktop/src/renderer/components/terminals/cliLaunch.test.ts
  • apps/desktop/src/renderer/components/terminals/cliLaunch.ts
  • apps/desktop/src/renderer/components/terminals/useWorkSessions.test.ts
  • apps/desktop/src/renderer/components/terminals/useWorkSessions.ts
  • apps/desktop/src/renderer/components/ui/DockLayoutState.ts
  • apps/desktop/src/renderer/components/ui/ResizeGutter.tsx
  • apps/desktop/src/renderer/index.css
  • apps/desktop/src/renderer/lib/lobeProviderIconSrc.ts
  • apps/desktop/src/renderer/lib/modelOptions.test.ts
  • apps/desktop/src/renderer/lib/modelOptions.ts
  • apps/desktop/src/renderer/lib/sessions.ts
  • apps/desktop/src/renderer/lib/terminalAttention.test.ts
  • apps/desktop/src/renderer/lib/terminalAttention.ts
  • apps/desktop/src/renderer/state/appStore.ts
  • apps/desktop/src/renderer/state/chatDraftStore.test.ts
  • apps/desktop/src/renderer/state/chatDraftStore.ts
  • apps/desktop/src/shared/chatModelSwitching.test.ts
  • apps/desktop/src/shared/chatModelSwitching.ts
  • apps/desktop/src/shared/ipc.ts
  • apps/desktop/src/shared/modelProfiles.test.ts
  • apps/desktop/src/shared/modelProfiles.ts
  • apps/desktop/src/shared/modelRegistry.test.ts
  • apps/desktop/src/shared/modelRegistry.ts
  • apps/desktop/src/shared/types/chat.ts
  • apps/desktop/src/shared/types/config.ts
  • apps/desktop/src/shared/types/missions.ts
  • apps/desktop/src/shared/types/models.ts
  • apps/desktop/src/shared/types/orchestrator.ts
  • apps/desktop/src/shared/types/packs.ts
  • apps/desktop/src/shared/types/sessions.ts
  • apps/desktop/src/test/__mocks__/lobehub-icons.js
  • apps/desktop/src/test/setup.ts
  • apps/desktop/tsup.config.ts
  • apps/ios/ADE/Resources/DatabaseBootstrap.sql
  • apps/mcp-server/package.json
  • apps/mcp-server/src/bootstrap.ts
  • apps/mcp-server/src/headlessLinearServices.ts
  • apps/mcp-server/src/mcpServer.test.ts
  • apps/mcp-server/src/mcpServer.ts
  • apps/mcp-server/src/test/setup.ts
  • apps/mcp-server/tsup.config.ts
  • apps/mcp-server/vitest.config.ts
  • chat/overview.mdx

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ade/opencode-integration-03ef5ca8

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 8, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 8, 2026

@copilot review

Code Review — OpenCode Runtime Integration

Overall this is a well-structured architectural change that meaningfully simplifies the AI runtime. The removal of ~13k lines of Vercel AI SDK plumbing in favour of direct OpenCode SDK calls + CLI spawning is coherent, and the new test coverage is solid. A few things worth addressing:


🔴 Breaking: "unified""opencode" in persisted data

ModelProviderGroup renames "unified" to "opencode", and the WorkerExecutionPath enum changes to match. Any session records, orchestrator run rows, or config files that already persisted "unified" will silently fall through to wrong branches. There are two migration shims in place (orchestratorQueries.ts:282, agentChatService.ts:4787), which is good, but it's worth confirming the DB schema migration (or lack thereof) is intentional and that these shims cover every read path.


🟡 runProviderTask / runOpenCodeTask drops permissionMode and auth

// providerTaskRunner.ts
async function runOpenCodeTask(args: ProviderTaskRunnerArgs): Promise<ProviderTaskRunnerResult> {
  const result = await runOpenCodeTextPrompt({
    directory: args.cwd,
    title: `ADE ${args.feature}`,
    projectConfig: args.projectConfig,
    modelDescriptor: args.descriptor,
    prompt: appendStructuredOutputInstruction(args.prompt, args.jsonSchema),
    system: args.system,
    // ← args.permissionMode and args.auth are silently ignored
  });

RunOpenCodePromptArgs has no permissionMode field, so OpenCode always runs with its own default. If the caller passes permissionMode: "read-only" (e.g. orchestrator coordinator) that constraint is currently dropped. Worth either forwarding as the agent field (e.g. mapPermissionModeToOpenCodeAgent) or documenting that OpenCode ignores it intentionally.


🟡 runCodexTask temp-file path isn't validated

const outPath = path.join(tmpDir, "out.txt");
// ...
const output = fs.existsSync(outPath) ? fs.readFileSync(outPath, "utf8").trim() : result.stdout.trim();

outPath is always constructed inside tmpDir, so there's no traversal risk here. However if fs.mkdtempSync throws (disk full), the error propagates without context. Wrapping it with a descriptive message would aid debugging:

let tmpDir: string;
try { tmpDir = fs.mkdtempSync(...); }
catch (err) { throw new Error(`Codex: failed to create temp dir: ${getErrorMessage(err)}`); }

🟡 agentChatService.ts is now 13 709 lines

The file keeps growing with each PR. The new OpenCodeRuntime type + event-loop logic adds meaningful complexity. Splitting the OpenCode session lifecycle into a dedicated module (similar to how compactionEngine.ts is extracted) would keep diffs reviewable and tests isolated.


🟢 Positives

  • toolExposurePolicy.ts — clean, testable heuristic for gating frontend-discovery tools; good score-based design.
  • executableTool.ts factory wrapper — tiny but makes tool definitions self-documenting and composable.
  • cliMcpConfig.ts normalizeCliMcpServers — handles the type vs transport field discrepancy between Claude and Codex CLIs correctly.
  • runtimeMessageTypes.ts — good to have an explicit runtime message contract instead of re-exporting from the AI SDK.
  • Extracted tool files (globSearch.ts, grepSearch.ts, readFileRange.ts) with 900+ lines of new tests — this is exactly the right direction.
  • compactionEngine.ts isolation — pulling transcript/compaction logic out of the executor is a clear improvement.
  • settled flag in runCommand — correctly prevents double-resolution on timeout + close races.
  • sdkProvider/sdkModelIdproviderRoute/providerModelId — semantically more accurate; no orphaned references found.

Minor nits

  • const timeoutMs = Math.max(1_000, args.timeoutMs ?? 120_000) — a 1 second floor is too low for any real agent task; 10_000 would be safer.
  • The \btab\b regex in FRONTEND_SUPPORTING_SIGNALS will match "tab" in "tabular data", "tab-separated", etc. Low risk since it's a low-weight supporting signal, but worth noting.
  • LOCAL_PROVIDER_LABELS is now exported from modelRegistry.ts — confirm this is intentional (it wasn't exported before, so any renderer imports that previously added it locally may now duplicate it).

Copilot AI changed the title Integrate OpenCode runtime: consolidate providers, tools, and model selection Review: OpenCode runtime integration PR Apr 8, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e8f8713dbe

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +278 to +282
const result = await runOpenCodeTextPrompt({
directory: args.cwd,
title: `ADE ${args.feature}`,
projectConfig: args.projectConfig,
modelDescriptor: args.descriptor,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Enforce timeout for OpenCode task execution

runProviderTask accepts timeoutMs, and the CLI-backed paths enforce it via runCommand, but the OpenCode branch calls runOpenCodeTextPrompt without any timeout/abort wiring. When tasks are routed to OpenCode (including orchestrator/review flows that pass explicit time limits), a stalled provider call can block indefinitely instead of failing fast, which can wedge mission/orchestrator execution.

Useful? React with 👍 / 👎.

Comment on lines +164 to +167
// Deduplicate concurrent probe calls
if (probeInFlight) return probeInFlight;

probeInFlight = (async () => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Key in-flight inventory probes by config and project

The global probeInFlight gate deduplicates all concurrent probes regardless of projectRoot or config fingerprint, so a second caller can receive the first caller’s inventory result. This is a correctness issue when calls overlap with different local model sets/configs (e.g., chat catalog probe vs status refresh), because model/provider lists and dynamic descriptor updates can be sourced from the wrong request.

Useful? React with 👍 / 👎.

Comment on lines +217 to +218
// For local providers, only include models that are actively loaded.
if (isLocal && allowedModels && !allowedModels.has(mid)) continue;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Filter unloaded local models when discovery data is absent

The local-model filter only applies when allowedModels exists, so if discoveredLocalModels is omitted the code includes every local model returned by provider.list(), including unloaded ones. That causes the catalog to surface models that are not actually runnable (not loaded), leading to avoidable runtime failures after selection.

Useful? React with 👍 / 👎.

…de integration

Review fixes:
- Add timeout + abort signal to OpenCode task execution in providerTaskRunner
- Key probeInFlight by project+config fingerprint instead of global dedup
- Filter unloaded local models when discoveredLocalModels is absent (safe default)
- Exempt */auto model IDs from OpenCode model-required guard

CI fixes:
- Wire force flag through finalizeRun to bypass validation (not phase eval)
- Include paused status in orchestrator smoke test finalization check
- Fix authDetector test mock URLs to use 127.0.0.1 (actual default endpoint)
- Remove time-dependent 2-hour offset in useWorkSessions grouping test

Cleanup:
- Remove low-value openCodeRuntime.test.ts and toolExposurePolicy.test.ts
- Add v1.0.13 changelog with detailed OpenCode integration release notes
- Update docs to reflect OpenCode server architecture (remove Vercel AI SDK refs)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
These 2 smoke tests (end-to-end mission run, complex mock planning flow)
were deeply fragile — each CI fix revealed a new failure mode (completing
vs succeeded, paused terminal state, intervention_required mission sync).
The tests exercised complex state machine transitions with many moving
parts but provided low signal relative to their maintenance cost.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@arul28 arul28 merged commit da90d01 into main Apr 8, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants