Skip to content

refactor(backends): extend NativeToolEngine from ClaudeCodeEngine#1020

Merged
aaight merged 1 commit intodevfrom
feature/claude-code-extends-native-tool-engine
Mar 23, 2026
Merged

refactor(backends): extend NativeToolEngine from ClaudeCodeEngine#1020
aaight merged 1 commit intodevfrom
feature/claude-code-extends-native-tool-engine

Conversation

@aaight
Copy link
Copy Markdown
Collaborator

@aaight aaight commented Mar 23, 2026

Summary

  • Migrates ClaudeCodeEngine to extend NativeToolEngine instead of directly implementing AgentEngine
  • Implements the three abstract methods: getAllowedEnvExact() returns Claude-specific env vars, getExtraEnvVars() returns { CLAUDE_AGENT_SDK_CLIENT_APP: 'cascade/1.0.0' }, and resolveEngineModel() delegates to resolveClaudeModel()
  • Updates afterExecute() to call super.afterExecute() (context file cleanup) then clean up the Claude-specific persisted session directory
  • Deprecates the standalone filterProcessEnv wrapper in claude-code/env.ts (kept for backward compat with tests); env filtering is now handled by the base class buildEnv()

Test plan

  • All existing claude-code.test.ts tests pass without any changes to assertions (82 tests)
  • All existing claude-code-env.test.ts tests pass without changes (15 tests)
  • NativeToolEngine.test.ts passes (19 tests)
  • Full unit test suite passes (6717 tests across 350 files)
  • TypeScript type checking passes with no errors
  • Linting passes with no errors

Trello: https://trello.com/c/69c1aa44cc343ba5291ec4dd

🤖 Generated with Claude Code

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

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 23, 2026

Codecov Report

❌ Patch coverage is 72.72727% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/backends/claude-code/index.ts 72.72% 3 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

LGTM — Clean refactoring that correctly migrates ClaudeCodeEngine to extend the NativeToolEngine base class. The three abstract method implementations are consistent with the existing values used by buildClaudeEnv(), afterExecute() correctly chains to super, and the removed supportsAgentType/resolveModel are properly inherited. All CI checks pass.

Should Fix (non-blocking)

  • Residual duplication: execute() still calls the standalone buildClaudeEnv() (which hardcodes ALLOWED_ENV_EXACT and the extra env vars) rather than this.buildEnv() from the base class. Both produce identical output today, so this is not a bug — but it means the env-building logic is now duplicated in two places (buildClaudeEnv and the base class buildEnv). A follow-up could replace buildClaudeEnv() usage in execute() with this.buildEnv(...), which would make buildClaudeEnv truly dead code (and the @deprecated annotation fully actionable).

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

@aaight aaight merged commit 19b1460 into dev Mar 23, 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