Skip to content

feat(opencode): extend NativeToolEngine in OpenCodeEngine#1026

Merged
aaight merged 1 commit intodevfrom
feature/opencode-extends-nativetoolengine
Mar 24, 2026
Merged

feat(opencode): extend NativeToolEngine in OpenCodeEngine#1026
aaight merged 1 commit intodevfrom
feature/opencode-extends-nativetoolengine

Conversation

@aaight
Copy link
Copy Markdown
Collaborator

@aaight aaight commented Mar 24, 2026

Summary

  • Migrates OpenCodeEngine to extend NativeToolEngine instead of directly implementing AgentEngine
  • Implements abstract methods: getAllowedEnvExact(), getExtraEnvVars(), resolveEngineModel()
  • Overrides afterExecute() to call super.afterExecute() for context file cleanup
  • Removes standalone filterProcessEnv wrapper from opencode/env.ts

Changes

src/backends/opencode/index.ts

  • Changed implements AgentEngineextends NativeToolEngine
  • Added getAllowedEnvExact() returning OPENAI_API_KEY, ANTHROPIC_API_KEY, OPENROUTER_API_KEY, SQUINT_DB_PATH (plus shared set)
  • Added getExtraEnvVars() returning { CI: 'true' }
  • Added resolveEngineModel() delegating to resolveOpenCodeModel()
  • Replaced direct cleanupContextFiles() call in afterExecute() with super.afterExecute() call
  • Removed redundant supportsAgentType() and resolveModel() overrides (now handled by base class)
  • Removed AgentEngine import (no longer needed)

src/backends/opencode/env.ts

  • Removed standalone filterProcessEnv export (deprecated)
  • Kept buildEnv() for use by server.ts which calls it directly

Test plan

  • All 6764 unit tests pass (npm test)
  • TypeScript type check passes (npm run typecheck)
  • Linter passes (npm run lint)
  • 19 OpenCode-specific tests pass without changes to assertions
  • NativeToolEngine contract tests pass for OpenCodeEngine

Trello card

https://trello.com/c/69c1aa83cf1734b4aa1bab3c

🤖 Generated with Claude Code

🕵️ claude-code · claude-sonnet-4-6 · run details

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 24, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 9 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/backends/opencode/index.ts 47.05% 9 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Collaborator

@nhopeatall nhopeatall left a comment

Choose a reason for hiding this comment

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

Summary

Clean migration of OpenCodeEngine from implements AgentEngine to extends NativeToolEngine. The abstract method implementations are correct, the removed filterProcessEnv has no remaining consumers, CI is green, and the change follows the same pattern established by the Codex engine migration.

Should Fix (non-blocking)

  • Duplicate env allowlist (src/backends/opencode/index.ts:420-436 / src/backends/opencode/env.ts:15-25): getAllowedEnvExact() in the engine class and ALLOWED_ENV_EXACT in env.ts define the same set of env vars (SHARED_ALLOWED_ENV_EXACT + OPENAI_API_KEY, ANTHROPIC_API_KEY, OPENROUTER_API_KEY, SQUINT_DB_PATH). If one is updated without the other, NativeToolEngine.buildEnv() and the standalone buildEnv() from env.ts (used by server.ts) will diverge silently. This is a pre-existing pattern shared with Codex, so not blocking — but worth a follow-up to make server.ts consume the engine instance's buildEnv() instead, establishing a single source of truth.

  • Unnecessary afterExecute override (src/backends/opencode/index.ts:458-460): The override is a pure passthrough — it only calls super.afterExecute(plan, result) with no additional logic. The base class method would be inherited automatically if removed. The Claude Code engine has a similar override but adds cleanupPersistedSession() after super, justifying its existence. Here it adds no behavior, just documentation. Consider removing it to reduce noise — the JSDoc on the base class already documents the contract.

🕵️ claude-code · claude-opus-4-6 · run details

@aaight aaight merged commit c7a57fe into dev Mar 24, 2026
8 of 9 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