Extract executor abstraction #29

alexander-akhmetov · 2026-02-02T20:57:57Z

This PR mostly refactors the code to make alternative executors possible, there are no user-facing changes except for the breaking config change.

Refactor the hardcoded Claude Code dependency into a configurable executor, selected via executor: in config or --executor CLI flag
Restructure config: replace top-level claude_flags/claude_config_dir/anthropic_api_key with executor: claude selector and nested claude: block

Config changes

Old config:

claude_flags: "--model opus"
claude_config_dir: "/path/to/config"
anthropic_api_key: "sk-ant-..."

New config

executor: "claude"  in the future could be "codex", "pi", etc.

claude:
  flags: "--model opus"
  config_dir: "/path/to/config"
  anthropic_api_key: "sk-ant-..."

Copilot

Pull request overview

This PR refactors the Claude Code integration to support a pluggable executor architecture, allowing different coding agents to be selected via configuration or CLI flag.

Changes:

Introduced an executor abstraction layer with factory pattern in internal/llm
Restructured configuration to use executor selector and nested claude block instead of top-level Claude-specific fields
Updated all references throughout the codebase to use the new nested configuration structure

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
internal/tui/view.go	Updated UI rendering to access Claude config via nested structure
internal/tui/tui.go	Added executor config field and setter method
internal/tui/start.go	Wired executor config through to TUI initialization
internal/tui/run.go	Added executor flag and updated Claude invocation to use factory pattern
internal/tui/plan.go	Updated plan creator to use executor config and factory pattern
internal/tui/helpers_test.go	Updated test assertions for nested Claude config structure
internal/tui/helpers.go	Updated skip permissions helper to use nested config fields
internal/tui/configcmd.go	Updated config display command to show executor and nested Claude settings
internal/safety/safety_test.go	Updated tests for new config structure with executor and nested Claude fields
internal/safety/safety.go	Added ClaudeConfig struct and updated Config to use executor field
internal/loop/loop.go	Extensive updates to use executor config, factory pattern, and conditional Claude-specific logic
internal/llm/factory_test.go	Added comprehensive tests for new executor factory
internal/llm/factory.go	Created executor factory with config-driven invoker creation
internal/config/defaults/config.yaml	Updated default config with executor field and nested Claude block
internal/config/config_test.go	Added tests for executor config loading and merging
internal/config/config.go	Added ClaudeConfig struct and updated Config to support nested structure
internal/config/compat_test.go	Updated compatibility tests for new config structure
internal/config/compat.go	Updated config conversion to use nested Claude structure

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/llm/factory.go

internal/config/config.go

Copilot

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

internal/tui/run.go:124

The executor config's ExtraFlags (which contains the Claude flags from config) are not being merged with the buildCommonFlags() result. This means that any Claude flags configured in the config file or environment will be ignored in print mode.

Consider concatenating both flag sources, for example:

configFlags := execCfg.ExtraFlags
cliFlags := strings.Join(buildCommonFlags(), " ")
combinedFlags := strings.TrimSpace(configFlags + " " + cliFlags)
opts := llm.InvokeOptions{
    WorkingDir: workingDir,
    ExtraFlags: combinedFlags,
    ...
}

	opts := llm.InvokeOptions{
		WorkingDir: workingDir,
		ExtraFlags: strings.Join(buildCommonFlags(), " "),

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/loop/loop.go

internal/tui/plan.go

…s`/`claude_config_dir`/`anthropic_api_key` with `executor` selector and per-executor `claude:` block

…`ExecutorConfig` struct and `NewInvoker()` factory function

…ker() with factory, guard Claude-specific hooks/flags, update log messages

…t.go, run.go (add --executor flag), and plan.go

… tests pass

- Cache invoker after successful factory creation in loop.go - Wire Claude EnvConfig (config_dir, api_key) through run.go - Add PROGRAMMATOR_ANTHROPIC_API_KEY env var handling in safety.go - Fix "invoke Claude" error message to "invoke executor" in plan.go - Add test coverage for PROGRAMMATOR_EXECUTOR and PROGRAMMATOR_ANTHROPIC_API_KEY

When review agents error, the loop would retry without incrementing the iteration counter or tracking stagnation. This caused an infinite loop that consumed all memory. Fix by: 1. Recording agent errors as no-progress iterations to trigger stagnation 2. Checking safety limits before retrying review

Copilot

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-04T15:33:32Z

internal/config/config.go

 		}
 	}

+	if v := os.Getenv("PROGRAMMATOR_EXECUTOR"); v != "" {
+		c.Executor = v
+		c.sources = append(c.sources, "env:PROGRAMMATOR_EXECUTOR")
+	}
+
 	if v := os.Getenv("PROGRAMMATOR_CLAUDE_FLAGS"); v != "" {
-		c.ClaudeFlags = v
+		c.Claude.Flags = v
 		c.sources = append(c.sources, "env:PROGRAMMATOR_CLAUDE_FLAGS")
 	}

 	if v := os.Getenv("CLAUDE_CONFIG_DIR"); v != "" {
-		c.ClaudeConfigDir = v
+		c.Claude.ConfigDir = v
 		c.sources = append(c.sources, "env:CLAUDE_CONFIG_DIR")
 	}

 	if v := os.Getenv("PROGRAMMATOR_ANTHROPIC_API_KEY"); v != "" {
-		c.AnthropicAPIKey = v
+		c.Claude.AnthropicAPIKey = v
 		c.sources = append(c.sources, "env:PROGRAMMATOR_ANTHROPIC_API_KEY")
 	}



This breaking change removes the old config keys (claude_flags, claude_config_dir, anthropic_api_key) and moves them under the nested claude block, but there's no backward compatibility handling or migration support. Consider adding detection of old keys in parseConfigWithTracking (similar to how review.passes is handled at line 278) to either automatically migrate them with a deprecation warning or provide a clear error message explaining the config migration path. This would significantly improve user experience during upgrades.

Copilot · 2026-02-04T15:33:32Z

internal/config/compat.go

+	return llm.ExecutorConfig{
+		Name: c.Executor,
+		Claude: llm.EnvConfig{
+			ClaudeConfigDir: c.Claude.ConfigDir,
+			AnthropicAPIKey: c.Claude.AnthropicAPIKey,
+		},
+		ExtraFlags: c.Claude.Flags,


The ToExecutorConfig method unconditionally copies c.Claude.Flags to ExtraFlags regardless of the executor type (c.Executor). This means if a user configures a non-Claude executor in the future, they will still get Claude-specific flags passed to it. Consider only setting ExtraFlags when c.Executor is "claude" or empty, or add separate flag fields for each executor type.

Suggested change

return llm.ExecutorConfig{

Name: c.Executor,

Claude: llm.EnvConfig{

ClaudeConfigDir: c.Claude.ConfigDir,

AnthropicAPIKey: c.Claude.AnthropicAPIKey,

},

ExtraFlags: c.Claude.Flags,

var extraFlags []string

// Only apply Claude-specific flags when using the Claude executor or when

// no executor is explicitly specified (Claude as default).

if c.Executor == "" || c.Executor == "claude" {

extraFlags = c.Claude.Flags

}

return llm.ExecutorConfig{

Name: c.Executor,

Claude: llm.EnvConfig{

ClaudeConfigDir: c.Claude.ConfigDir,

AnthropicAPIKey: c.Claude.AnthropicAPIKey,

},

ExtraFlags: extraFlags,

Copilot · 2026-02-04T15:33:32Z

internal/loop/loop.go

 	}
+	l.reviewConfig.EnvConfig = l.executorConfig.Claude


The EnvConfig is set unconditionally at line 1301 (l.reviewConfig.EnvConfig = l.executorConfig.Claude) even when the executor is not Claude. This could cause issues if a non-Claude executor is configured. Consider moving this line inside the isClaudeExecutor() conditional block (after line 1300) to ensure Claude-specific configuration is only applied when using the Claude executor.

Suggested change

}

l.reviewConfig.EnvConfig = l.executorConfig.Claude

l.reviewConfig.EnvConfig = l.executorConfig.Claude

}

Copilot · 2026-02-04T15:33:33Z

internal/tui/plan.go

@@ -175,7 +177,7 @@ func (p *planCreator) run() (string, error) {
 		// Invoke Claude


The comment "Invoke Claude" should be executor-agnostic to align with the PR's goal of supporting alternative executors. Consider changing it to "Invoke executor" to match the updated error message at line 180.

The claudeFlags field duplicated executorConfig.ExtraFlags since both are set from cfg.Claude.Flags. Use executorConfig.ExtraFlags directly to prevent config drift.

- Move reviewConfig.EnvConfig inside isClaudeExecutor() guard - Guard ExtraFlags in ToExecutorConfig by executor type - Rename invokeClaudePrint → invokeExecutor - Rename invokeClaude → invokeExecutor in plan creator - Rename processClaudeStatus → processStatus - Rename loopBreakToClaudeInvocation → loopBreakToInvocation - Update comments/logs to use generic "executor" terminology - Add inline comments to Executor and Name fields

alexander-akhmetov requested a review from Copilot February 2, 2026 20:58

Copilot AI reviewed Feb 2, 2026

View reviewed changes

internal/llm/factory.go Show resolved Hide resolved

internal/config/config.go Outdated Show resolved Hide resolved

alexander-akhmetov force-pushed the programmator/pro-ng1w branch 2 times, most recently from 9104dec to 26790df Compare February 2, 2026 23:54

alexander-akhmetov requested a review from Copilot February 2, 2026 23:54

Copilot started reviewing on behalf of alexander-akhmetov February 2, 2026 23:55 View session

Copilot AI reviewed Feb 3, 2026

View reviewed changes

internal/loop/loop.go Outdated Show resolved Hide resolved

internal/tui/plan.go Outdated Show resolved Hide resolved

alexander-akhmetov force-pushed the programmator/pro-ng1w branch from 26790df to a03b841 Compare February 3, 2026 00:15

alexander-akhmetov marked this pull request as ready for review February 3, 2026 00:19

This comment was marked as outdated.

Sign in to view

alexander-akhmetov added 6 commits February 4, 2026 08:25

Phase 1: Restructure executor config - Replace top-level `claude_flag…

238eb60

…s`/`claude_config_dir`/`anthropic_api_key` with `executor` selector and per-executor `claude:` block

Phase 2: Create invoker factory - New internal/llm/factory.go with …

7c901cf

…`ExecutorConfig` struct and `NewInvoker()` factory function

Phase 3: Update loop to use factory - Replace hardcoded NewClaudeInvo…

17ac1a5

…ker() with factory, guard Claude-specific hooks/flags, update log messages

Phase 4: Wire through TUI - Pass executor config through tui.go, star…

b459ffd

…t.go, run.go (add --executor flag), and plan.go

Phase 5: Tests - Factory tests, config env var tests, verify existing…

22c3ce4

… tests pass

alexander-akhmetov force-pushed the programmator/pro-ng1w branch from a03b841 to d1cebc8 Compare February 4, 2026 07:44

alexander-akhmetov requested a review from Copilot February 4, 2026 07:44

Copilot started reviewing on behalf of alexander-akhmetov February 4, 2026 07:45 View session

This comment was marked as outdated.

Sign in to view

alexander-akhmetov added the breaking label Feb 4, 2026

alexander-akhmetov requested a review from Copilot February 4, 2026 15:26

Copilot started reviewing on behalf of alexander-akhmetov February 4, 2026 15:26 View session

Copilot AI reviewed Feb 4, 2026

View reviewed changes

alexander-akhmetov added 2 commits February 4, 2026 18:34

fix: remove duplicate claudeFlags field in planCreator

b8bc7da

The claudeFlags field duplicated executorConfig.ExtraFlags since both are set from cfg.Claude.Flags. Use executorConfig.ExtraFlags directly to prevent config drift.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract executor abstraction #29

Extract executor abstraction #29

Uh oh!

alexander-akhmetov commented Feb 2, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 4, 2026

Uh oh!

Copilot AI Feb 4, 2026

Uh oh!

Copilot AI Feb 4, 2026

Uh oh!

Copilot AI Feb 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		@@ -175,7 +177,7 @@ func (p *planCreator) run() (string, error) {
		// Invoke Claude

Extract executor abstraction #29

Are you sure you want to change the base?

Extract executor abstraction #29

Uh oh!

Conversation

alexander-akhmetov commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Config changes

Old config:

New config

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

alexander-akhmetov commented Feb 2, 2026 •

edited

Loading