From f8b8e5b02f1a420b69e8a31cab7e6bcd44b0e83a Mon Sep 17 00:00:00 2001 From: Alexandre Balmes Date: Tue, 5 May 2026 14:00:53 +0200 Subject: [PATCH] feat(agents): add GitHub Copilot agent provider - `internal/infrastructure/agents/copilot_provider.go`: Implement `github_copilot` provider with single-turn and multi-turn conversation support via `copilot` CLI - `internal/infrastructure/agents/copilot_provider_unit_test.go`: Add unit tests for provider construction, execution, and session handling - `internal/infrastructure/agents/copilot_provider_delegation_test.go`: Add delegation tests validating shared behavior contracts - `internal/infrastructure/agents/copilot_provider_extract_test.go`: Add extraction tests for session ID and output parsing - `internal/infrastructure/agents/copilot_provider_parse_display_events_test.go`: Add JSONL stream display event parsing tests - `internal/infrastructure/agents/copilot_provider_validation_unit_test.go`: Add option validation tests for mode and effort enums - `internal/infrastructure/agents/options.go`: Add Copilot-specific option keys (model, mode, effort, tools, allow_all) - `internal/infrastructure/agents/provider_options_test.go`: Extend provider options tests to cover Copilot options - `internal/infrastructure/agents/registry.go`: Register `github_copilot` as 6th provider in `RegisterDefaults()` - `internal/infrastructure/agents/registry_test.go`: Update registry tests to include Copilot provider assertions - `internal/infrastructure/agents/stream_filter.go`: Extend JSONL stream filtering to handle Copilot output format - `pkg/display/event.go`: Update display event parsing to support Copilot JSONL event structure - `CHANGELOG.md`: Document F089 GitHub Copilot provider addition - `CLAUDE.md`: Add provider name prefix rule and update project overview - `README.md`: Update feature descriptions to include GitHub Copilot - `docs/README.md`: Update agent steps documentation link description - `docs/user-guide/agent-steps.md`: Add GitHub Copilot provider documentation with options and auth - `docs/user-guide/conversation-steps.md`: Add `github_copilot` to conversation mode provider list Closes #328 --- CHANGELOG.md | 3 +- CLAUDE.md | 5 +- README.md | 8 +- docs/README.md | 2 +- docs/user-guide/agent-steps.md | 40 +- docs/user-guide/conversation-steps.md | 4 +- .../infrastructure/agents/copilot_provider.go | 251 +++++++++ .../copilot_provider_delegation_test.go | 388 +++++++++++++ .../agents/copilot_provider_extract_test.go | 287 ++++++++++ ...ilot_provider_parse_display_events_test.go | 261 +++++++++ .../agents/copilot_provider_unit_test.go | 517 ++++++++++++++++++ .../copilot_provider_validation_unit_test.go | 216 ++++++++ internal/infrastructure/agents/options.go | 14 + .../agents/provider_options_test.go | 98 ++++ internal/infrastructure/agents/registry.go | 1 + .../infrastructure/agents/registry_test.go | 54 +- .../infrastructure/agents/stream_filter.go | 22 +- pkg/display/event.go | 14 +- 18 files changed, 2116 insertions(+), 69 deletions(-) create mode 100644 internal/infrastructure/agents/copilot_provider.go create mode 100644 internal/infrastructure/agents/copilot_provider_delegation_test.go create mode 100644 internal/infrastructure/agents/copilot_provider_extract_test.go create mode 100644 internal/infrastructure/agents/copilot_provider_parse_display_events_test.go create mode 100644 internal/infrastructure/agents/copilot_provider_unit_test.go create mode 100644 internal/infrastructure/agents/copilot_provider_validation_unit_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ff70dae..e9598ce1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,8 +13,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **F089**: GitHub Copilot agent provider — new `github_copilot` provider implementing `AgentProvider` interface via the `copilot` CLI binary; single-turn execution (`-p --output-format=json --silent`), multi-turn conversation support (`--resume=`), CLI flag mapping for model (`--model`), mode (`--mode`: interactive/plan/autopilot), effort (`--effort`: low/medium/high), tool permissions (`--allow-tool`, `--deny-tool`, `--allow-all`); JSONL streaming display event parsing; option validation for `mode` and `effort` enums at `awf validate` time; graceful fallback to stateless mode when session ID extraction fails; registered in `RegisterDefaults()` as 6th provider; authenticate via `copilot login` or environment variables (`COPILOT_GITHUB_TOKEN`, `GH_TOKEN`) - **F088**: Terminal User Interface (TUI) — `awf tui` launches a full-screen Bubble Tea dashboard with five tabs: Workflows (filterable list with launch/validate actions), Monitoring (real-time execution tree with status icons and live log viewport via 200ms tick polling of `ExecutionContext`), History (browse past executions from SQLite with filtering by name/status/date), Agent Conversations (Glamour-rendered Markdown chat view), and External Logs (fsnotify-based live tailing of Claude Code JSONL session files); TUI lives in `internal/interfaces/tui/` as a new interface adapter bridging to existing `WorkflowService`, `ExecutionService`, and `HistoryService` via async `tea.Cmd` factories; secret masking applied to all views; terminal state restored on exit/panic/signal; requires 256-color terminal support (graceful fallback to basic ANSI) -- **F085**: Unified display-event abstraction across all agent providers — replaces per-provider `LineExtractor` function-field with a `DisplayEventParser` returning structured `DisplayEvent` values (discriminated by `EventText` and `EventToolUse` kinds); all 5 providers (Claude, Codex, Gemini, OpenCode, OpenAI-Compatible) now emit events through the same parser contract; single interfaces-layer `RenderEvents` renderer with two display modes: default (text only, byte-equivalent to F082 behaviour) and verbose (text + tool-use markers in `[tool: Name(Arg)]` format); well-known tools (`Read`, `Write`, `Edit`, `Bash`, `Grep`, `Glob`, `Task`) display concise markers with argument truncation (≤ 40 chars); unknown tool names degrade gracefully; parser implementations return plain strings with no ANSI escapes (rendering concerns confined to interfaces layer); `output_format: json` bypasses event parsing entirely for raw passthrough; `DisplayOutput` aggregation on `AgentResult`/`ConversationResult` preserved via text-event concatenation +- **F085**: Unified display-event abstraction across all agent providers — replaces per-provider `LineExtractor` function-field with a `DisplayEventParser` returning structured `DisplayEvent` values (discriminated by `EventText` and `EventToolUse` kinds); all 6 providers (Claude, Codex, Gemini, GitHub Copilot, OpenCode, OpenAI-Compatible) now emit events through the same parser contract; single interfaces-layer `RenderEvents` renderer with two display modes: default (text only, byte-equivalent to F082 behaviour) and verbose (text + tool-use markers in `[tool: Name(Arg)]` format); well-known tools (`Read`, `Write`, `Edit`, `Bash`, `Grep`, `Glob`, `Task`) display concise markers with argument truncation (≤ 40 chars); unknown tool names degrade gracefully; parser implementations return plain strings with no ANSI escapes (rendering concerns confined to interfaces layer); `output_format: json` bypasses event parsing entirely for raw passthrough; `DisplayOutput` aggregation on `AgentResult`/`ConversationResult` preserved via text-event concatenation ### Changed diff --git a/CLAUDE.md b/CLAUDE.md index 236f3548..30fc2c05 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -72,7 +72,7 @@ Use `mcp__plugin_common_serena__write_memory` or `mcp__plugin_common_serena__edi ## Project Overview -**ai-workflow-cli** (`awf`) - A Go CLI tool for orchestrating AI agents (Claude, Gemini, Codex) through YAML-configured workflows with state machine execution. +**ai-workflow-cli** (`awf`) - A Go CLI tool for orchestrating AI agents (Claude, Gemini, Codex, GitHub Copilot) through YAML-configured workflows with state machine execution. ## Architecture @@ -217,7 +217,6 @@ func TestWorkflowValidation(t *testing.T) { ## Architecture Rules -- Pass optional turn-specific configuration (e.g., system_prompt) through options map in application layer; keeps infrastructure providers independent of turn logic - Validate agent provider options only against what each CLI actually accepts; do not validate against API documentation if the underlying CLI rejects the option - Plugin binaries must be discoverable at //awf-plugin-; host validates binary existence and version compatibility via gRPC handshake after process start - Commit generated protobuf files (.pb.go, _grpc.pb.go) to git; treat as source artifacts for build reproducibility, not ephemeral build outputs @@ -240,6 +239,8 @@ func TestWorkflowValidation(t *testing.T) { - Wire optional render callbacks alongside event parsers in stream processors; decouples rendering from parsing and enables multiple render modes (DefaultMode, VerboseMode) without modifying parser implementations - When integrating external UI frameworks, create Bridge adapters in the interface layer that wrap application services; maintain zero infrastructure imports in bridge implementation +- Use provider name prefixes for all infrastructure provider helper methods (buildCopilot, extractCopilot, parseCopilot, validateCopilot) to prevent naming collisions across implementations + ## Common Pitfalls - Use 0o755 for executable scripts, 0o644 for data files, 0o700 for private temp files; match permissions to file purpose and access expectations diff --git a/README.md b/README.md index c3570e3e..bd0d0420 100644 --- a/README.md +++ b/README.md @@ -3,17 +3,17 @@ [![Go Version](https://img.shields.io/badge/Go-1.24+-00ADD8?style=flat&logo=go)](https://go.dev/) [![License: EUPL-1.2](https://img.shields.io/badge/License-EUPL--1.2-blue.svg)](LICENSE) -A Go CLI tool for orchestrating AI agents (Claude, Gemini, Codex, OpenAI-Compatible APIs) through YAML-configured workflows with state machine execution. +A Go CLI tool for orchestrating AI agents (Claude, Gemini, Codex, GitHub Copilot, OpenAI-Compatible APIs) through YAML-configured workflows with state machine execution. ## Features - **State Machine Execution** - Define workflows as state machines with conditional transitions based on exit codes, command output, or custom expressions - **Inline Error Handling** - Specify error messages and exit codes directly on steps without creating separate terminal states -- **Agent Steps** - Invoke AI agents via CLI tools (Claude, Codex, Gemini) or direct HTTP (OpenAI, Ollama, vLLM, Groq) with prompt templates, response parsing, and accurate token tracking -- **Output Formatting for Agent Steps** - Automatically strip markdown code fences and validate JSON output; human-readable streaming display controlled by `output_format` field (text vs raw NDJSON); unified display-event abstraction across all 5 providers with optional verbose mode showing tool-use markers (`[tool: Name(Arg)]`) +- **Agent Steps** - Invoke AI agents via CLI tools (Claude, Codex, Gemini, GitHub Copilot) or direct HTTP (OpenAI, Ollama, vLLM, Groq) with prompt templates, response parsing, and accurate token tracking +- **Output Formatting for Agent Steps** - Automatically strip markdown code fences and validate JSON output; human-readable streaming display controlled by `output_format` field (text vs raw NDJSON); unified display-event abstraction across all 6 providers with optional verbose mode showing tool-use markers (`[tool: Name(Arg)]`) - **External Prompt Files** - Load agent prompts from `.md` files with full template interpolation, helper functions, and local override support - **External Script Files** - Load commands from external script files with shebang-based interpreter dispatch, template interpolation, path resolution, and local override support -- **Conversation Mode** - Multi-turn conversations with native session resume for CLI providers (`claude`, `codex`, `gemini`, `opencode`), automatic context window management for HTTP providers, mid-conversation context injection via `inject_context` field, and token tracking across all turns +- **Conversation Mode** - Multi-turn conversations with native session resume for CLI providers (`claude`, `codex`, `gemini`, `opencode`, `github_copilot`), automatic context window management for HTTP providers, mid-conversation context injection via `inject_context` field, and token tracking across all turns - **OpenAI-Compatible Provider** - Use any Chat Completions API (OpenAI, Ollama, vLLM, Groq) with native HTTP integration, accurate token reporting, and no CLI tool required - **Parallel Execution** - Run multiple steps concurrently with configurable strategies - **Loop Constructs** - For-each and while loops with full context access diff --git a/docs/README.md b/docs/README.md index fa70ac08..c899ff2d 100644 --- a/docs/README.md +++ b/docs/README.md @@ -28,7 +28,7 @@ Learn how to use AWF effectively: - [Commands](user-guide/commands.md) - All CLI commands and flags - [Interactive Input Collection](user-guide/interactive-inputs.md) - Automatic prompting for missing workflow inputs -- [Agent Steps](user-guide/agent-steps.md) - Invoke AI agents via CLI (Claude, Codex, Gemini) or HTTP APIs (OpenAI, Ollama, vLLM, Groq) +- [Agent Steps](user-guide/agent-steps.md) - Invoke AI agents via CLI (Claude, Codex, Gemini, GitHub Copilot) or HTTP APIs (OpenAI, Ollama, vLLM, Groq) - [Output Formatting](user-guide/agent-steps.md#output-formatting) - Automatic code fence stripping and JSON validation (`output_format: json|text`) - [Streaming Output Display & Tool Markers](user-guide/agent-steps.md#streaming-output-display--tool-markers) - Human-readable filtered output and tool-use markers for `--output streaming` and `--output buffered` modes - [External Prompt Files](user-guide/agent-steps.md#external-prompt-files) - Load prompts from Markdown files with template interpolation diff --git a/docs/user-guide/agent-steps.md b/docs/user-guide/agent-steps.md index a0ab4513..126ea042 100644 --- a/docs/user-guide/agent-steps.md +++ b/docs/user-guide/agent-steps.md @@ -2,7 +2,7 @@ title: "Agent Steps Guide" --- -Invoke AI agents (Claude, Codex, Gemini, OpenCode, OpenAI-Compatible) in your workflows with structured prompts and response parsing. +Invoke AI agents (Claude, Codex, Gemini, GitHub Copilot, OpenCode, OpenAI-Compatible) in your workflows with structured prompts and response parsing. ## Overview @@ -126,6 +126,38 @@ summarize: - `model`: Gemini model identifier — validated to start with `gemini-` prefix (see [Model Validation](#model-validation) below) - `dangerously_skip_permissions`: Skip permission prompts (boolean, maps to `--approval-mode=yolo`). **Security warning**: bypasses all safety prompts — use only in trusted, automated environments. +### GitHub Copilot + +Requires the `copilot` CLI tool installed and authentication via `copilot login` or environment variables. + +```yaml +code_generate: + type: agent + provider: github_copilot + prompt: "Generate a function to: {{.inputs.requirement}}" + options: + model: gpt-4o + mode: interactive + timeout: 60 + on_success: next +``` + +**Provider-Specific Options:** +- `model`: GitHub Copilot model identifier (e.g., `gpt-4o`, `gpt-4`, `gpt-3.5-turbo`) +- `mode`: Agent mode — one of `interactive` (default), `plan`, or `autopilot` +- `effort`: Reasoning effort level — one of `low`, `medium`, or `high` +- `allowed_tools`: Comma-separated list of tools to allow (e.g., `"bash,github_api"` → `--allow-tool bash --allow-tool github_api`) +- `denied_tools`: Comma-separated list of tools to deny (maps to `--deny-tool`) +- `allow_all`: Allow all available tools (boolean, maps to `--allow-all`) +- `system_prompt`: Custom system message (passed via prompt prepending) + +**Authentication:** +GitHub Copilot CLI supports authentication via: +- `copilot login` (interactive authentication) +- `COPILOT_GITHUB_TOKEN` environment variable +- `GH_TOKEN` environment variable +- `GITHUB_TOKEN` environment variable (classic PATs not supported) + ### OpenCode Requires the `opencode` CLI tool installed. @@ -242,9 +274,9 @@ options: step validation error: model must start with "gpt-", "codex-", or match o-series pattern (e.g., o1, o3-mini) ``` -### OpenCode & OpenAI-Compatible +### GitHub Copilot, OpenCode & OpenAI-Compatible -No model validation for `opencode` or `openai_compatible` providers — these use arbitrary backend models. +No model validation for `github_copilot`, `opencode`, or `openai_compatible` providers — these support arbitrary backend models. ### When Validation Occurs @@ -699,7 +731,7 @@ Tool markers show: - **Interleaved order** — markers appear in the same source order as agent output - **Graceful degradation** — unknown tool names display as-is with no crash or error -This works consistently across all 5 supported providers (Claude, Codex, Gemini, OpenCode, OpenAI-Compatible). Verbose mode has no effect on `output_format: json` — raw NDJSON is always passed through unchanged. +This works consistently across all 6 supported providers (Claude, Codex, Gemini, GitHub Copilot, OpenCode, OpenAI-Compatible). Verbose mode has no effect on `output_format: json` — raw NDJSON is always passed through unchanged. #### Buffered Mode (`--output buffered`) diff --git a/docs/user-guide/conversation-steps.md b/docs/user-guide/conversation-steps.md index 79a6ef5d..dba6046a 100644 --- a/docs/user-guide/conversation-steps.md +++ b/docs/user-guide/conversation-steps.md @@ -66,7 +66,7 @@ You'll see the agent's first reply, then a `> ` prompt where you can type. When | Field | Required | Description | |---|---|---| -| `provider` | Yes | Agent provider (`claude`, `gemini`, `codex`, `opencode`, `openai_compatible`) | +| `provider` | Yes | Agent provider (`claude`, `gemini`, `codex`, `opencode`, `github_copilot`, `openai_compatible`) | | `mode` | Yes | Must be `conversation` | | `prompt` | Yes | First user message — sent automatically as turn 1 | | `system_prompt` | No | System message preserved for the whole session | @@ -137,7 +137,7 @@ states: Both steps run as `mode: single` (the default — no `mode:` line needed). There is no interactive loop. Each step runs exactly one agent turn. - `seed` has `conversation: {}`. This marks the step as session-tracked: AWF calls `provider.ExecuteConversation` (instead of `provider.Execute`), the provider runs one turn, and the session ID returned by the CLI is captured into `state.conversation.session_id`. -- `recall` has `conversation: {continue_from: seed}`. AWF clones the conversation state from `seed` (session ID + turn history) and passes it to the provider, which resumes the session via its native flag (`claude -r `, `gemini --resume `, `codex resume `, `opencode -s `). +- `recall` has `conversation: {continue_from: seed}`. AWF clones the conversation state from `seed` (session ID + turn history) and passes it to the provider, which resumes the session via its native flag (`claude -r `, `gemini --resume `, `codex resume `, `opencode -s `, `copilot --resume=`). ### Why the Empty `conversation: {}`? diff --git a/internal/infrastructure/agents/copilot_provider.go b/internal/infrastructure/agents/copilot_provider.go new file mode 100644 index 00000000..009a4cd6 --- /dev/null +++ b/internal/infrastructure/agents/copilot_provider.go @@ -0,0 +1,251 @@ +package agents + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "os/exec" + "strings" + + "github.com/awf-project/cli/internal/domain/ports" + "github.com/awf-project/cli/internal/domain/workflow" + "github.com/awf-project/cli/internal/infrastructure/logger" +) + +// CopilotProvider implements AgentProvider for GitHub Copilot CLI. +// Invokes: copilot -p "prompt" --output-format=json --silent +type CopilotProvider struct { + base *baseCLIProvider + logger ports.Logger + executor ports.CLIExecutor +} + +func NewCopilotProvider() *CopilotProvider { + p := &CopilotProvider{ + logger: logger.NopLogger{}, + executor: NewExecCLIExecutor(), + } + p.base = p.newBase() + return p +} + +func NewCopilotProviderWithOptions(opts ...CopilotProviderOption) *CopilotProvider { + p := &CopilotProvider{ + logger: logger.NopLogger{}, + executor: NewExecCLIExecutor(), + } + for _, opt := range opts { + opt(p) + } + p.base = p.newBase() + return p +} + +func (p *CopilotProvider) newBase() *baseCLIProvider { + return newBaseCLIProvider("github_copilot", "copilot", p.executor, p.logger, cliProviderHooks{ + buildExecuteArgs: p.buildCopilotExecuteArgs, + buildConversationArgs: p.buildCopilotConversationArgs, + extractSessionID: p.extractCopilotSessionID, + extractTextContent: p.extractCopilotTextContent, + validateOptions: validateCopilotOptions, + parseDisplayEvents: p.parseCopilotDisplayEvents, + }) +} + +func (p *CopilotProvider) Execute(ctx context.Context, prompt string, options map[string]any, stdout, stderr io.Writer) (*workflow.AgentResult, error) { + result, rawOutput, err := p.base.execute(ctx, prompt, options, stdout, stderr) + if err != nil { + return nil, err + } + if extracted := p.extractCopilotTextContent(rawOutput); extracted != "" { + result.Output = extracted + result.Tokens = estimateTokens(extracted) + } + return result, nil +} + +func (p *CopilotProvider) ExecuteConversation(ctx context.Context, state *workflow.ConversationState, prompt string, options map[string]any, stdout, stderr io.Writer) (*workflow.ConversationResult, error) { + result, _, err := p.base.executeConversation(ctx, state, prompt, options, stdout, stderr) + if err != nil { + return nil, err + } + return result, nil +} + +func (p *CopilotProvider) Name() string { + return "github_copilot" +} + +func (p *CopilotProvider) Validate() error { + _, err := exec.LookPath("copilot") + if err != nil { + return fmt.Errorf("copilot CLI not found in PATH: %w", err) + } + return nil +} + +func (p *CopilotProvider) buildCopilotExecuteArgs(prompt string, options map[string]any) ([]string, error) { + args := []string{"-p", prompt, "--output-format=json", "--silent"} + args = appendCopilotOptions(args, options) + return args, nil +} + +func (p *CopilotProvider) buildCopilotConversationArgs(state *workflow.ConversationState, prompt string, options map[string]any) ([]string, error) { + var args []string + if state.SessionID != "" { + args = []string{"--resume=" + state.SessionID, "-p", prompt, "--output-format=json", "--silent"} + } else { + effectivePrompt := buildCopilotFirstTurnPrompt(prompt, options) + args = []string{"-p", effectivePrompt, "--output-format=json", "--silent"} + } + args = appendCopilotOptions(args, options) + return args, nil +} + +// buildCopilotFirstTurnPrompt prepends an optional system prompt. +// Copilot CLI has no --system-prompt flag; the system context must be embedded in the message. +func buildCopilotFirstTurnPrompt(userPrompt string, options map[string]any) string { + if sysPrompt, ok := getStringOption(options, "system_prompt"); ok && sysPrompt != "" { + return sysPrompt + "\n\n" + userPrompt + } + return userPrompt +} + +// appendCopilotOptions appends Copilot CLI flags from options; unknown keys are silently ignored. +func appendCopilotOptions(args []string, options map[string]any) []string { + if model, ok := getStringOption(options, "model"); ok && model != "" { + args = append(args, "--model="+model) + } + if mode, ok := getStringOption(options, "mode"); ok && mode != "" { + args = append(args, "--mode="+mode) + } + if effort, ok := getStringOption(options, "effort"); ok && effort != "" { + args = append(args, "--effort="+effort) + } + if tools, ok := options["allowed_tools"]; ok { + for _, t := range toStringSlice(tools) { + args = append(args, "--allow-tool="+t) + } + } + if tools, ok := options["denied_tools"]; ok { + for _, t := range toStringSlice(tools) { + args = append(args, "--deny-tool="+t) + } + } + if allow, ok := getBoolOption(options, "allow_all"); ok && allow { + args = append(args, "--allow-all") + } + return args +} + +func toStringSlice(v any) []string { + switch typed := v.(type) { + case []string: + return typed + case []any: + result := make([]string, 0, len(typed)) + for _, item := range typed { + if s, ok := item.(string); ok { + result = append(result, s) + } + } + return result + } + return nil +} + +func validateCopilotOptions(options map[string]any) error { + var errs []string + + if mode, ok := getStringOption(options, "mode"); ok && mode != "" { + switch mode { + case "interactive", "plan", "autopilot": + default: + errs = append(errs, fmt.Sprintf("invalid mode %q: must be one of interactive, plan, autopilot", mode)) + } + } + + if effort, ok := getStringOption(options, "effort"); ok && effort != "" { + switch effort { + case "low", "medium", "high": + default: + errs = append(errs, fmt.Sprintf("invalid effort %q: must be one of low, medium, high", effort)) + } + } + + if len(errs) > 0 { + return fmt.Errorf("%s", strings.Join(errs, "; ")) + } + return nil +} + +func (p *CopilotProvider) extractCopilotSessionID(output string) (string, error) { + if output == "" { + return "", errors.New("empty output") + } + evt := findFirstNDJSONEvent(output, "result") + if evt == nil { + return "", errors.New("result event not found") + } + // Copilot CLI uses camelCase "sessionId" (not snake_case) + sessionIDVal, ok := evt["sessionId"] + if !ok || sessionIDVal == nil { + return "", errors.New("sessionId missing") + } + if str, ok := sessionIDVal.(string); ok && str != "" { + return str, nil + } + return "", errors.New("sessionId is not a non-empty string") +} + +func (p *CopilotProvider) parseCopilotDisplayEvents(line []byte) []DisplayEvent { + var evt struct { + Type string `json:"type"` + Data map[string]any `json:"data"` + } + if err := json.Unmarshal(line, &evt); err != nil { + return nil + } + switch evt.Type { + case "assistant.message_delta": + if delta, ok := evt.Data["deltaContent"].(string); ok && delta != "" { + return []DisplayEvent{{Type: evt.Type, Kind: EventText, Text: delta, Delta: true}} + } + case "tool.execution_start": + name, ok := evt.Data["toolName"].(string) + if !ok { + name = "" + } + return []DisplayEvent{{Type: evt.Type, Kind: EventToolUse, Name: name}} + } + return nil +} + +// extractCopilotTextContent scans JSONL output for the last assistant.message event +// and returns its data.content field. Falls back to raw output when not found. +func (p *CopilotProvider) extractCopilotTextContent(output string) string { + var lastContent string + for _, line := range strings.Split(output, "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + var evt map[string]any + if err := json.Unmarshal([]byte(line), &evt); err != nil { + continue + } + if t, ok := evt["type"].(string); ok && t == "assistant.message" { + if data, ok := evt["data"].(map[string]any); ok { + if content, ok := data["content"].(string); ok && content != "" { + lastContent = content + } + } + } + } + if lastContent != "" { + return lastContent + } + return output +} diff --git a/internal/infrastructure/agents/copilot_provider_delegation_test.go b/internal/infrastructure/agents/copilot_provider_delegation_test.go new file mode 100644 index 00000000..b18ba706 --- /dev/null +++ b/internal/infrastructure/agents/copilot_provider_delegation_test.go @@ -0,0 +1,388 @@ +package agents + +import ( + "context" + "testing" + + "github.com/awf-project/cli/internal/domain/workflow" + "github.com/awf-project/cli/internal/testutil/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// T004: Copilot provider delegation tests verify that CopilotProvider correctly +// delegates Execute and ExecuteConversation to baseCLIProvider through hooks. +// Tests fail against stub (buildCopilotExecuteArgs/buildCopilotConversationArgs return nil) +// and pass after implementation provides proper CLI arguments. + +func TestCopilotProvider_Execute_DelegationToBase(t *testing.T) { + tests := []struct { + name string + prompt string + options map[string]any + mockStdout string + wantOutput string + }{ + { + name: "simple prompt delegation", + prompt: "Hello world", + options: nil, + mockStdout: `{"type":"assistant.message","data":{"content":"response text","messageId":"m1"}}` + "\n" + `{"type":"result","sessionId":"s1","exitCode":0}`, + wantOutput: "response text", + }, + { + name: "prompt with model option delegation", + prompt: "test prompt", + options: map[string]any{"model": "gpt-4o"}, + mockStdout: `{"type":"assistant.message","data":{"content":"model response","messageId":"m1"}}` + "\n" + `{"type":"result","sessionId":"s1","exitCode":0}`, + wantOutput: "model response", + }, + { + name: "prompt with multiple options", + prompt: "create function", + options: map[string]any{"model": "gpt-4", "mode": "plan"}, + mockStdout: `{"type":"assistant.message","data":{"content":"function code","messageId":"m1"}}` + "\n" + `{"type":"result","sessionId":"s1","exitCode":0}`, + wantOutput: "function code", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte(tt.mockStdout), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + result, err := provider.Execute(context.Background(), tt.prompt, tt.options, nil, nil) + + require.NoError(t, err) + require.NotNil(t, result) + assert.Equal(t, "github_copilot", result.Provider) + assert.Equal(t, tt.wantOutput, result.Output) + assert.True(t, result.TokensEstimated, "Execute must set TokensEstimated=true") + }) + } +} + +func TestCopilotProvider_Execute_EmptyPrompt_ValidationByBase(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + result, err := provider.Execute(context.Background(), "", nil, nil, nil) + + assert.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "prompt") + // Base validates before calling hook + calls := mockExec.GetCalls() + assert.Empty(t, calls) +} + +func TestCopilotProvider_Execute_ContextCancellation_DelegationByBase(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + result, err := provider.Execute(ctx, "test", nil, nil, nil) + + assert.Error(t, err) + assert.Nil(t, result) + // Base checks context before hook execution + calls := mockExec.GetCalls() + assert.Empty(t, calls) +} + +func TestCopilotProvider_ExecuteConversation_DelegationToBase(t *testing.T) { + tests := []struct { + name string + systemPrompt string + userPrompt string + options map[string]any + mockStdout string + expectSessionID bool + }{ + { + name: "simple conversation turn", + systemPrompt: "You are helpful", + userPrompt: "What is recursion?", + options: nil, + mockStdout: `{"type":"assistant.message","data":{"content":"Recursion is...","messageId":"m1"}}` + "\n" + `{"type":"result","sessionId":"s1","exitCode":0}`, + expectSessionID: false, + }, + { + name: "conversation with session event", + systemPrompt: "Code assistant", + userPrompt: "Write a function", + options: map[string]any{"model": "gpt-4o"}, + mockStdout: `{"type":"assistant.message","data":{"content":"response","messageId":"m1"}}` + "\n" + `{"type":"result","sessionId":"sess-abc","exitCode":0}`, + expectSessionID: true, + }, + { + name: "conversation with options", + systemPrompt: "System", + userPrompt: "Query", + options: map[string]any{"mode": "autopilot", "effort": "high"}, + mockStdout: `{"type":"assistant.message","data":{"content":"answer","messageId":"m1"}}` + "\n" + `{"type":"result","sessionId":"s1","exitCode":0}`, + expectSessionID: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte(tt.mockStdout), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + state := workflow.NewConversationState(tt.systemPrompt) + result, err := provider.ExecuteConversation(context.Background(), state, tt.userPrompt, tt.options, nil, nil) + + require.NoError(t, err) + require.NotNil(t, result) + assert.Equal(t, "github_copilot", result.Provider) + assert.NotNil(t, result.State) + }) + } +} + +func TestCopilotProvider_ExecuteConversation_FirstTurn_ArgsStructure(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"code\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + state := workflow.NewConversationState("") + _, err := provider.ExecuteConversation(context.Background(), state, "first prompt", nil, nil, nil) + + require.NoError(t, err) + calls := mockExec.GetCalls() + require.Len(t, calls, 1) + args := calls[0].Args + + // First turn should have: -p --output-format=json --silent + assert.Contains(t, args, "-p") + assert.Contains(t, args, "first prompt") + assert.Contains(t, args, "--output-format=json") + assert.Contains(t, args, "--silent") + assert.NotContains(t, args, "--resume") +} + +func TestCopilotProvider_ExecuteConversation_ResumeTurn_ArgsStructure(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"code\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + state := workflow.NewConversationState("") + state.SessionID = "sess-123" + _, err := provider.ExecuteConversation(context.Background(), state, "follow-up", nil, nil, nil) + + require.NoError(t, err) + calls := mockExec.GetCalls() + require.Len(t, calls, 1) + args := calls[0].Args + + // Resume turn should have: --resume= -p --output-format=json --silent + assert.Contains(t, args, "--resume=sess-123") + assert.Contains(t, args, "-p") + assert.Contains(t, args, "follow-up") + assert.Contains(t, args, "--output-format=json") + assert.Contains(t, args, "--silent") +} + +func TestCopilotProvider_ExecuteConversation_FirstTurn_SystemPromptInlined(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"response\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + state := workflow.NewConversationState("") + _, err := provider.ExecuteConversation( + context.Background(), + state, + "user query", + map[string]any{"system_prompt": "You are an AI"}, + nil, + nil, + ) + + require.NoError(t, err) + calls := mockExec.GetCalls() + require.Len(t, calls, 1) + args := calls[0].Args + + // System prompt should be inlined in the first prompt argument + for i, arg := range args { + if arg == "-p" && i+1 < len(args) { + prompt := args[i+1] + assert.Contains(t, prompt, "You are an AI") + assert.Contains(t, prompt, "user query") + break + } + } +} + +func TestCopilotProvider_ExecuteConversation_ResumeTurn_SystemPromptNotInlined(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"response\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + state := workflow.NewConversationState("") + state.SessionID = "sess-abc" + _, err := provider.ExecuteConversation( + context.Background(), + state, + "follow-up", + map[string]any{"system_prompt": "You are an AI"}, + nil, + nil, + ) + + require.NoError(t, err) + calls := mockExec.GetCalls() + require.Len(t, calls, 1) + args := calls[0].Args + + // On resume turn, system_prompt is silently ignored (preserved for future use) + // The prompt should just be the user's follow-up + for i, arg := range args { + if arg == "-p" && i+1 < len(args) { + prompt := args[i+1] + // Should be just the follow-up, not the system prompt + assert.Equal(t, "follow-up", prompt) + break + } + } +} + +func TestCopilotProvider_ExecuteConversation_EmptyPrompt_ValidationByBase(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + state := workflow.NewConversationState("") + result, err := provider.ExecuteConversation(context.Background(), state, "", nil, nil, nil) + + assert.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "prompt") +} + +func TestCopilotProvider_ExecuteConversation_NilState_ValidationByBase(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + result, err := provider.ExecuteConversation(context.Background(), nil, "test", nil, nil, nil) + + assert.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "conversation state") +} + +func TestCopilotProvider_ExecuteConversation_ContextCancellation_DelegationByBase(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + state := workflow.NewConversationState("") + result, err := provider.ExecuteConversation(ctx, state, "test", nil, nil, nil) + + assert.Error(t, err) + assert.Nil(t, result) + // Base checks context before hook execution + calls := mockExec.GetCalls() + assert.Empty(t, calls) +} + +func TestCopilotProvider_ExecuteConversation_StatePreservation(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"response\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + originalState := workflow.NewConversationState("") + originalState.AddTurn(workflow.NewTurn(workflow.TurnRoleUser, "initial")) + originalLen := len(originalState.Turns) + + result, err := provider.ExecuteConversation(context.Background(), originalState, "test", nil, nil, nil) + + require.NoError(t, err) + require.NotNil(t, result) + + // Original state should be unchanged + assert.Len(t, originalState.Turns, originalLen) + assert.Equal(t, "initial", originalState.Turns[0].Content) + + // Result state should be a clone with new turns + assert.NotEqual(t, originalState, result.State) + assert.Len(t, result.State.Turns, originalLen+2) // +2 for user and assistant +} + +func TestCopilotProvider_ExecuteConversation_TurnsAppendedCorrectly(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"assistant output\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + state := workflow.NewConversationState("") + state.AddTurn(workflow.NewTurn(workflow.TurnRoleUser, "first")) + state.AddTurn(workflow.NewTurn(workflow.TurnRoleAssistant, "first response")) + + result, err := provider.ExecuteConversation(context.Background(), state, "second", nil, nil, nil) + + require.NoError(t, err) + require.NotNil(t, result) + + // Should have 4 turns: first 2 + new user + new assistant + assert.Len(t, result.State.Turns, 4) + + // Verify new turns + assert.Equal(t, workflow.TurnRoleUser, result.State.Turns[2].Role) + assert.Equal(t, "second", result.State.Turns[2].Content) + assert.Equal(t, workflow.TurnRoleAssistant, result.State.Turns[3].Role) + assert.Equal(t, "assistant output", result.State.Turns[3].Content) +} + +func TestCopilotProvider_ExecuteConversation_WithOptions_ArgsConstruction(t *testing.T) { + tests := []struct { + name string + options map[string]any + wantArgList []string + }{ + { + name: "model option", + options: map[string]any{"model": "gpt-4o"}, + wantArgList: []string{"--model=gpt-4o"}, + }, + { + name: "mode option", + options: map[string]any{"mode": "interactive"}, + wantArgList: []string{"--mode=interactive"}, + }, + { + name: "effort option", + options: map[string]any{"effort": "high"}, + wantArgList: []string{"--effort=high"}, + }, + { + name: "multiple options", + options: map[string]any{"model": "gpt-4", "mode": "plan"}, + wantArgList: []string{"--model=gpt-4", "--mode=plan"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"response\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + state := workflow.NewConversationState("") + _, err := provider.ExecuteConversation(context.Background(), state, "test", tt.options, nil, nil) + + require.NoError(t, err) + calls := mockExec.GetCalls() + require.Len(t, calls, 1) + + for _, wantArg := range tt.wantArgList { + assert.Contains(t, calls[0].Args, wantArg) + } + }) + } +} diff --git a/internal/infrastructure/agents/copilot_provider_extract_test.go b/internal/infrastructure/agents/copilot_provider_extract_test.go new file mode 100644 index 00000000..a15c7696 --- /dev/null +++ b/internal/infrastructure/agents/copilot_provider_extract_test.go @@ -0,0 +1,287 @@ +package agents + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// T001: Extract tests for CopilotProvider validate session ID and text content extraction from JSONL + +func TestCopilotProvider_extractSessionID(t *testing.T) { + provider := NewCopilotProvider() + + tests := []struct { + name string + output string + wantID string + wantErr bool + }{ + { + name: "result event carries session_id", + output: `{"type":"result","sessionId":"abc-123"}`, + wantID: "abc-123", + wantErr: false, + }, + { + name: "result event with result and session_id", + output: `{"type":"result","sessionId":"sess-xyz","result":"output"}`, + wantID: "sess-xyz", + wantErr: false, + }, + { + name: "result event with uuid-style session_id", + output: `{"type":"result","sessionId":"019bd456-d3d4-70c3-90de-51d31a6c8571"}`, + wantID: "019bd456-d3d4-70c3-90de-51d31a6c8571", + wantErr: false, + }, + { + name: "result with subsequent events", + output: `{"type":"result","sessionId":"sess-abc","result":"Generated"}` + "\n" + `{"type":"done"}`, + wantID: "sess-abc", + wantErr: false, + }, + { + name: "numeric-looking session_id string", + output: `{"type":"result","sessionId":"98765"}`, + wantID: "98765", + wantErr: false, + }, + { + name: "no result event returns error", + output: `{"type":"message","content":"text"}`, + wantID: "", + wantErr: true, + }, + { + name: "empty output returns error", + output: "", + wantID: "", + wantErr: true, + }, + { + name: "plain text returns error", + output: "plain text with no JSON", + wantID: "", + wantErr: true, + }, + { + name: "result event without session_id returns error", + output: `{"type":"result","result":"output"}`, + wantID: "", + wantErr: true, + }, + { + name: "result with empty session_id returns error", + output: `{"type":"result","sessionId":""}`, + wantID: "", + wantErr: true, + }, + { + name: "result with null session_id returns error", + output: `{"type":"result","sessionId":null}`, + wantID: "", + wantErr: true, + }, + { + name: "session_id in non-result event is ignored", + output: `{"type":"message","sessionId":"should-not-match"}`, + wantID: "", + wantErr: true, + }, + { + name: "malformed JSON line is skipped", + output: `{"type":"result","sessionId":"incomplete`, + wantID: "", + wantErr: true, + }, + { + name: "multiple JSONL lines with result first", + output: `{"type":"result","sessionId":"found-id"}` + "\n" + `{"type":"message"}` + "\n" + `{"type":"done"}`, + wantID: "found-id", + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := provider.extractCopilotSessionID(tt.output) + + if tt.wantErr { + assert.Error(t, err) + assert.Empty(t, got) + } else { + // Graceful fallback: no error, just empty string if not found + assert.NoError(t, err) + assert.Equal(t, tt.wantID, got) + } + }) + } +} + +func TestCopilotProvider_extractTextContent(t *testing.T) { + provider := NewCopilotProvider() + + tests := []struct { + name string + output string + wantOut string + }{ + { + name: "assistant.message event carries content", + output: `{"type":"assistant.message","data":{"content":"hello world","messageId":"msg-1"}}`, + wantOut: "hello world", + }, + { + name: "assistant.message with multiline content", + output: `{"type":"assistant.message","data":{"content":"line1\nline2\nline3","messageId":"msg-2"}}`, + wantOut: "line1\nline2\nline3", + }, + { + name: "assistant.message with empty content falls back to raw", + output: `{"type":"assistant.message","data":{"content":"","messageId":"msg-3"}}`, + wantOut: `{"type":"assistant.message","data":{"content":"","messageId":"msg-3"}}`, + }, + { + name: "assistant.message with special characters", + output: `{"type":"assistant.message","data":{"content":"Hello \"World\"!","messageId":"msg-4"}}`, + wantOut: `Hello "World"!`, + }, + { + name: "full JSONL stream extracts last assistant.message", + output: `{"type":"user.message","data":{"content":"test"}}` + "\n" + `{"type":"assistant.message","data":{"content":"response","messageId":"msg-5"}}` + "\n" + `{"type":"result","sessionId":"sess-1","exitCode":0}`, + wantOut: "response", + }, + { + name: "multiple assistant.message events returns last", + output: `{"type":"assistant.message","data":{"content":"first","messageId":"msg-6"}}` + "\n" + `{"type":"assistant.message","data":{"content":"second","messageId":"msg-7"}}`, + wantOut: "second", + }, + { + name: "no assistant.message returns raw output", + output: `{"type":"result","sessionId":"sess-1","exitCode":0}`, + wantOut: `{"type":"result","sessionId":"sess-1","exitCode":0}`, + }, + { + name: "empty output returns empty unchanged", + output: "", + wantOut: "", + }, + { + name: "plain text returns raw text unchanged", + output: "plain text with no JSON", + wantOut: "plain text with no JSON", + }, + { + name: "malformed JSON line skipped", + output: `{"type":"assistant.message","data":{"content":"incomplete`, + wantOut: `{"type":"assistant.message","data":{"content":"incomplete`, + }, + { + name: "assistant.message with json content", + output: `{"type":"assistant.message","data":{"content":"{\"key\":\"value\"}","messageId":"msg-8"}}`, + wantOut: `{"key":"value"}`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := provider.extractCopilotTextContent(tt.output) + assert.Equal(t, tt.wantOut, got) + }) + } +} + +func TestCopilotProvider_extractSessionID_MultilineJSONL(t *testing.T) { + provider := NewCopilotProvider() + + // Test that we find the first result event with session_id in JSONL stream + output := `{"type":"progress","pct":25} +{"type":"result","sessionId":"sess-important","result":"code"} +{"type":"progress","pct":100} +{"type":"done"}` + + id, err := provider.extractCopilotSessionID(output) + + assert.NoError(t, err) + assert.Equal(t, "sess-important", id) +} + +func TestCopilotProvider_extractTextContent_LastMessageWins(t *testing.T) { + provider := NewCopilotProvider() + + output := `{"type":"assistant.message","data":{"content":"first turn","messageId":"msg-1"}} +{"type":"tool.execution_start","data":{"toolName":"bash"}} +{"type":"assistant.message","data":{"content":"second turn","messageId":"msg-2"}} +{"type":"result","sessionId":"sess-1","exitCode":0}` + + content := provider.extractCopilotTextContent(output) + + assert.Equal(t, "second turn", content) +} + +func TestCopilotProvider_extractTextContent_NoAssistantMessage(t *testing.T) { + provider := NewCopilotProvider() + + tests := []struct { + name string + output string + wantOut string + }{ + { + name: "only progress events", + output: `{"type":"session.mcp_servers_loaded","data":{}}` + "\n" + `{"type":"result","sessionId":"s","exitCode":0}`, + wantOut: `{"type":"session.mcp_servers_loaded","data":{}}` + "\n" + `{"type":"result","sessionId":"s","exitCode":0}`, + }, + { + name: "user message only", + output: `{"type":"user.message","data":{"content":"hello"}}`, + wantOut: `{"type":"user.message","data":{"content":"hello"}}`, + }, + { + name: "plain error text", + output: "command failed: permission denied", + wantOut: "command failed: permission denied", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + content := provider.extractCopilotTextContent(tt.output) + assert.Equal(t, tt.wantOut, content) + }) + } +} + +func TestCopilotProvider_extractTextContent_ContentVariations(t *testing.T) { + provider := NewCopilotProvider() + + tests := []struct { + name string + output string + wantOut string + }{ + { + name: "unicode characters", + output: `{"type":"assistant.message","data":{"content":"Hello 世界 🌍","messageId":"msg-1"}}`, + wantOut: "Hello 世界 🌍", + }, + { + name: "content with newlines", + output: `{"type":"assistant.message","data":{"content":"line1\nline2","messageId":"msg-2"}}`, + wantOut: "line1\nline2", + }, + { + name: "content with tool requests is still extracted", + output: `{"type":"assistant.message","data":{"content":"I will run ls","toolRequests":[{"name":"bash"}],"messageId":"msg-3"}}`, + wantOut: "I will run ls", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := provider.extractCopilotTextContent(tt.output) + assert.Equal(t, tt.wantOut, got) + }) + } +} diff --git a/internal/infrastructure/agents/copilot_provider_parse_display_events_test.go b/internal/infrastructure/agents/copilot_provider_parse_display_events_test.go new file mode 100644 index 00000000..68b84b6f --- /dev/null +++ b/internal/infrastructure/agents/copilot_provider_parse_display_events_test.go @@ -0,0 +1,261 @@ +package agents + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCopilotProvider_parseCopilotDisplayEvents_MessageDelta(t *testing.T) { + provider := NewCopilotProvider() + + tests := []struct { + name string + line []byte + wantLen int + wantKind EventKind + wantText string + }{ + { + name: "message delta with text", + line: []byte(`{"type":"assistant.message_delta","data":{"messageId":"msg-1","deltaContent":"hello world"}}`), + wantLen: 1, + wantKind: EventText, + wantText: "hello world", + }, + { + name: "message delta with multiline text", + line: []byte(`{"type":"assistant.message_delta","data":{"messageId":"msg-2","deltaContent":"line1\nline2"}}`), + wantLen: 1, + wantKind: EventText, + wantText: "line1\nline2", + }, + { + name: "message delta with special characters", + line: []byte(`{"type":"assistant.message_delta","data":{"messageId":"msg-3","deltaContent":"Response {braces} 你好"}}`), + wantLen: 1, + wantKind: EventText, + wantText: "Response {braces} 你好", + }, + { + name: "message delta with escaped quotes", + line: []byte(`{"type":"assistant.message_delta","data":{"messageId":"msg-4","deltaContent":"She said \"hello\""}}`), + wantLen: 1, + wantKind: EventText, + wantText: `She said "hello"`, + }, + { + name: "message delta with empty deltaContent is skipped", + line: []byte(`{"type":"assistant.message_delta","data":{"messageId":"msg-5","deltaContent":""}}`), + wantLen: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := provider.parseCopilotDisplayEvents(tt.line) + if tt.wantLen == 0 { + assert.Empty(t, got) + return + } + require.Len(t, got, tt.wantLen) + assert.Equal(t, tt.wantKind, got[0].Kind) + assert.Equal(t, tt.wantText, got[0].Text) + }) + } +} + +func TestCopilotProvider_parseCopilotDisplayEvents_ToolExecution(t *testing.T) { + provider := NewCopilotProvider() + + tests := []struct { + name string + line []byte + wantLen int + wantKind EventKind + wantName string + }{ + { + name: "tool execution start with bash", + line: []byte(`{"type":"tool.execution_start","data":{"toolCallId":"call-1","toolName":"bash","arguments":{"command":"ls"}}}`), + wantLen: 1, + wantKind: EventToolUse, + wantName: "bash", + }, + { + name: "tool execution start with read", + line: []byte(`{"type":"tool.execution_start","data":{"toolCallId":"call-2","toolName":"read","arguments":{"file_path":"main.go"}}}`), + wantLen: 1, + wantKind: EventToolUse, + wantName: "read", + }, + { + name: "tool execution start with report_intent", + line: []byte(`{"type":"tool.execution_start","data":{"toolCallId":"call-3","toolName":"report_intent","arguments":{"intent":"fixing bug"}}}`), + wantLen: 1, + wantKind: EventToolUse, + wantName: "report_intent", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := provider.parseCopilotDisplayEvents(tt.line) + require.Len(t, got, tt.wantLen) + assert.Equal(t, tt.wantKind, got[0].Kind) + assert.Equal(t, tt.wantName, got[0].Name) + }) + } +} + +func TestCopilotProvider_parseCopilotDisplayEvents_IgnoredTypes(t *testing.T) { + provider := NewCopilotProvider() + + tests := []struct { + name string + line []byte + }{ + { + name: "result event is not a display event", + line: []byte(`{"type":"result","sessionId":"sess-1","exitCode":0}`), + }, + { + name: "session.mcp_servers_loaded is not a display event", + line: []byte(`{"type":"session.mcp_servers_loaded","data":{"servers":[]}}`), + }, + { + name: "user.message is not a display event", + line: []byte(`{"type":"user.message","data":{"content":"hello"}}`), + }, + { + name: "assistant.turn_start is not a display event", + line: []byte(`{"type":"assistant.turn_start","data":{"turnId":"0"}}`), + }, + { + name: "assistant.turn_end is not a display event", + line: []byte(`{"type":"assistant.turn_end","data":{"turnId":"0"}}`), + }, + { + name: "assistant.message (complete) is not a display event", + line: []byte(`{"type":"assistant.message","data":{"content":"hello","messageId":"msg-1"}}`), + }, + { + name: "tool.execution_complete is not a display event", + line: []byte(`{"type":"tool.execution_complete","data":{"toolCallId":"call-1","success":true}}`), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := provider.parseCopilotDisplayEvents(tt.line) + assert.Empty(t, got, "non-display event types should return empty slice") + }) + } +} + +func TestCopilotProvider_parseCopilotDisplayEvents_ErrorPaths(t *testing.T) { + provider := NewCopilotProvider() + + tests := []struct { + name string + line []byte + }{ + { + name: "empty line", + line: []byte(""), + }, + { + name: "malformed JSON", + line: []byte("{broken"), + }, + { + name: "incomplete JSON object", + line: []byte(`{"type":"assistant.message_delta"`), + }, + { + name: "not JSON at all", + line: []byte("this is not json at all"), + }, + { + name: "JSON with null type", + line: []byte(`{"type":null,"data":{"deltaContent":"hello"}}`), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := provider.parseCopilotDisplayEvents(tt.line) + assert.Empty(t, got, "malformed input should return empty slice") + }) + } +} + +func TestCopilotProvider_parseCopilotDisplayEvents_EventKindMetadata(t *testing.T) { + provider := NewCopilotProvider() + + tests := []struct { + name string + line []byte + wantKind EventKind + }{ + { + name: "message delta has EventText kind", + line: []byte(`{"type":"assistant.message_delta","data":{"deltaContent":"test"}}`), + wantKind: EventText, + }, + { + name: "tool execution has EventToolUse kind", + line: []byte(`{"type":"tool.execution_start","data":{"toolName":"bash"}}`), + wantKind: EventToolUse, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := provider.parseCopilotDisplayEvents(tt.line) + require.Len(t, got, 1) + assert.Equal(t, tt.wantKind, got[0].Kind) + assert.NotEqual(t, EventKind(""), got[0].Kind, "EventKind should never be zero-value") + }) + } +} + +func TestCopilotProvider_parseCopilotDisplayEvents_LargeInput(t *testing.T) { + provider := NewCopilotProvider() + + largeContent := strings.Repeat("x", 1024*1024) + line := []byte(`{"type":"assistant.message_delta","data":{"deltaContent":"` + largeContent + `"}}`) + + got := provider.parseCopilotDisplayEvents(line) + + require.Len(t, got, 1) + assert.Equal(t, EventText, got[0].Kind) + assert.Len(t, got[0].Text, len(largeContent)) +} + +func BenchmarkParseCopilotDisplayEvents(b *testing.B) { + provider := NewCopilotProvider() + line := []byte(`{"type":"assistant.message_delta","data":{"deltaContent":"Hello from Copilot!"}}`) + + b.ReportAllocs() + b.ResetTimer() + + for i := 0; i < b.N; i++ { + _ = provider.parseCopilotDisplayEvents(line) + } +} + +func BenchmarkParseCopilotDisplayEvents_LargePayload(b *testing.B) { + provider := NewCopilotProvider() + largeContent := strings.Repeat("x", 10000) + line := []byte(`{"type":"assistant.message_delta","data":{"deltaContent":"` + largeContent + `"}}`) + + b.ReportAllocs() + b.ResetTimer() + + for i := 0; i < b.N; i++ { + _ = provider.parseCopilotDisplayEvents(line) + } +} diff --git a/internal/infrastructure/agents/copilot_provider_unit_test.go b/internal/infrastructure/agents/copilot_provider_unit_test.go new file mode 100644 index 00000000..be06e090 --- /dev/null +++ b/internal/infrastructure/agents/copilot_provider_unit_test.go @@ -0,0 +1,517 @@ +package agents + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/awf-project/cli/internal/domain/workflow" + "github.com/awf-project/cli/internal/testutil/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// CopilotProvider unit tests covering Execute, ExecuteConversation, Name, Validate, and args building + +func TestCopilotProvider_Name(t *testing.T) { + provider := NewCopilotProvider() + assert.Equal(t, "github_copilot", provider.Name()) +} + +func TestCopilotProvider_Execute_RawOutputFallback(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("plain text output"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + result, err := provider.Execute(context.Background(), "test", nil, nil, nil) + + require.NoError(t, err) + require.NotNil(t, result) + assert.Equal(t, "github_copilot", result.Provider) + assert.Equal(t, "plain text output", result.Output) + assert.True(t, result.TokensEstimated) +} + +func TestCopilotProvider_Execute_WithOptions(t *testing.T) { + tests := []struct { + name string + prompt string + options map[string]any + mockStdout string + wantCLIArgs []string + }{ + { + name: "model option", + prompt: "test", + options: map[string]any{"model": "gpt-4o"}, + mockStdout: `{"type":"assistant.message","data":{"content":"code","messageId":"m1"}}` + "\n" + `{"type":"result","sessionId":"s1","exitCode":0}`, + wantCLIArgs: []string{"-p", "test", "--output-format=json", "--silent", "--model=gpt-4o"}, + }, + { + name: "mode option", + prompt: "test", + options: map[string]any{"mode": "interactive"}, + mockStdout: `{"type":"assistant.message","data":{"content":"code","messageId":"m1"}}` + "\n" + `{"type":"result","sessionId":"s1","exitCode":0}`, + wantCLIArgs: []string{"-p", "test", "--output-format=json", "--silent", "--mode=interactive"}, + }, + { + name: "effort option", + prompt: "test", + options: map[string]any{"effort": "high"}, + mockStdout: `{"type":"assistant.message","data":{"content":"code","messageId":"m1"}}` + "\n" + `{"type":"result","sessionId":"s1","exitCode":0}`, + wantCLIArgs: []string{"-p", "test", "--output-format=json", "--silent", "--effort=high"}, + }, + { + name: "allowed_tools single", + prompt: "test", + options: map[string]any{"allowed_tools": []string{"browser"}}, + mockStdout: `{"type":"assistant.message","data":{"content":"code","messageId":"m1"}}` + "\n" + `{"type":"result","sessionId":"s1","exitCode":0}`, + wantCLIArgs: []string{"-p", "test", "--output-format=json", "--silent", "--allow-tool=browser"}, + }, + { + name: "allowed_tools multiple", + prompt: "test", + options: map[string]any{"allowed_tools": []string{"browser", "python"}}, + mockStdout: `{"type":"assistant.message","data":{"content":"code","messageId":"m1"}}` + "\n" + `{"type":"result","sessionId":"s1","exitCode":0}`, + wantCLIArgs: []string{"-p", "test", "--output-format=json", "--silent", "--allow-tool=browser", "--allow-tool=python"}, + }, + { + name: "denied_tools", + prompt: "test", + options: map[string]any{"denied_tools": []string{"bash"}}, + mockStdout: `{"type":"assistant.message","data":{"content":"code","messageId":"m1"}}` + "\n" + `{"type":"result","sessionId":"s1","exitCode":0}`, + wantCLIArgs: []string{"-p", "test", "--output-format=json", "--silent", "--deny-tool=bash"}, + }, + { + name: "allow_all", + prompt: "test", + options: map[string]any{"allow_all": true}, + mockStdout: `{"type":"assistant.message","data":{"content":"code","messageId":"m1"}}` + "\n" + `{"type":"result","sessionId":"s1","exitCode":0}`, + wantCLIArgs: []string{"-p", "test", "--output-format=json", "--silent", "--allow-all"}, + }, + { + name: "unknown options silently ignored", + prompt: "test", + options: map[string]any{"language": "python", "quiet": true}, + mockStdout: `{"type":"assistant.message","data":{"content":"code","messageId":"m1"}}` + "\n" + `{"type":"result","sessionId":"s1","exitCode":0}`, + wantCLIArgs: []string{"-p", "test", "--output-format=json", "--silent"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte(tt.mockStdout), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + _, err := provider.Execute(context.Background(), tt.prompt, tt.options, nil, nil) + + require.NoError(t, err) + calls := mockExec.GetCalls() + require.Len(t, calls, 1) + assert.Equal(t, "copilot", calls[0].Name) + // Check all expected args are present + for _, arg := range tt.wantCLIArgs { + assert.Contains(t, calls[0].Args, arg, "arg %s should be in CLI args", arg) + } + }) + } +} + +func TestCopilotProvider_Execute_EmptyPrompt(t *testing.T) { + tests := []struct { + name string + prompt string + wantErr string + }{ + { + name: "empty string", + prompt: "", + wantErr: "prompt cannot be empty", + }, + { + name: "whitespace only", + prompt: " \t\n ", + wantErr: "prompt cannot be empty", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"code\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + result, err := provider.Execute(context.Background(), tt.prompt, nil, nil, nil) + + assert.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + assert.Nil(t, result) + // Executor should not be called for empty prompt validation + calls := mockExec.GetCalls() + assert.Empty(t, calls) + }) + } +} + +func TestCopilotProvider_Execute_ContextCancellation(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"code\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + result, err := provider.Execute(ctx, "test", nil, nil, nil) + + assert.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "context canceled") +} + +func TestCopilotProvider_Execute_ContextDeadline(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"code\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Nanosecond) + defer cancel() + time.Sleep(10 * time.Millisecond) + + result, err := provider.Execute(ctx, "test", nil, nil, nil) + + assert.Error(t, err) + assert.Nil(t, result) +} + +func TestCopilotProvider_Execute_CLIError(t *testing.T) { + tests := []struct { + name string + mockErr error + wantErr string + }{ + { + name: "command not found", + mockErr: errors.New("command not found: copilot"), + wantErr: "copilot execution failed", + }, + { + name: "generic error", + mockErr: errors.New("unknown error"), + wantErr: "copilot execution failed", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetError(tt.mockErr) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + result, err := provider.Execute(context.Background(), "test", nil, nil, nil) + + assert.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), tt.wantErr) + }) + } +} + +func TestCopilotProvider_ExecuteConversation_FirstTurn(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"response\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + state := workflow.NewConversationState("") + result, err := provider.ExecuteConversation(context.Background(), state, "first prompt", nil, nil, nil) + + require.NoError(t, err) + require.NotNil(t, result) + assert.Equal(t, "github_copilot", result.Provider) + assert.Equal(t, "response", result.Output) + assert.True(t, result.TokensEstimated) +} + +func TestCopilotProvider_ExecuteConversation_ResumeTurn(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"follow-up response\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + state := workflow.NewConversationState("") + state.SessionID = "sess-abc123" + + result, err := provider.ExecuteConversation(context.Background(), state, "follow-up", nil, nil, nil) + + require.NoError(t, err) + require.NotNil(t, result) + assert.Equal(t, "follow-up response", result.Output) + // Verify --resume flag was used + calls := mockExec.GetCalls() + require.Len(t, calls, 1) + args := calls[0].Args + require.Contains(t, args, "--resume=sess-abc123") +} + +func TestCopilotProvider_ExecuteConversation_FirstTurnWithSystemPrompt(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"response\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + state := workflow.NewConversationState("") + result, err := provider.ExecuteConversation( + context.Background(), + state, + "user prompt", + map[string]any{"system_prompt": "You are a helper"}, + nil, + nil, + ) + + require.NoError(t, err) + require.NotNil(t, result) + // Verify system prompt was embedded in the prompt + calls := mockExec.GetCalls() + require.Len(t, calls, 1) + args := calls[0].Args + // Find the -p argument (should contain both system and user prompt) + for i, arg := range args { + if arg == "-p" && i+1 < len(args) { + assert.Contains(t, args[i+1], "You are a helper") + assert.Contains(t, args[i+1], "user prompt") + break + } + } +} + +func TestCopilotProvider_ExecuteConversation_StateCloning(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"response\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + originalState := workflow.NewConversationState("") + originalState.AddTurn(workflow.NewTurn(workflow.TurnRoleUser, "initial")) + + result, err := provider.ExecuteConversation(context.Background(), originalState, "test", nil, nil, nil) + + require.NoError(t, err) + require.NotNil(t, result) + // Original state should be unchanged + assert.Len(t, originalState.Turns, 1) + assert.Equal(t, "initial", originalState.Turns[0].Content) + // Result state should have new turns + assert.Len(t, result.State.Turns, 3) +} + +func TestCopilotProvider_ExecuteConversation_StateTurnsAppended(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"assistant response\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + state := workflow.NewConversationState("") + state.AddTurn(workflow.NewTurn(workflow.TurnRoleUser, "first")) + + result, err := provider.ExecuteConversation(context.Background(), state, "second", nil, nil, nil) + + require.NoError(t, err) + require.NotNil(t, result) + // Should have original + 2 new turns (user + assistant) + assert.Len(t, result.State.Turns, 3) + assert.Equal(t, workflow.TurnRoleUser, result.State.Turns[1].Role) + assert.Equal(t, "second", result.State.Turns[1].Content) + assert.Equal(t, workflow.TurnRoleAssistant, result.State.Turns[2].Role) + assert.Equal(t, "assistant response", result.State.Turns[2].Content) +} + +func TestCopilotProvider_ExecuteConversation_NilState(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + result, err := provider.ExecuteConversation(context.Background(), nil, "test", nil, nil, nil) + + assert.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "conversation state cannot be nil") +} + +func TestCopilotProvider_ExecuteConversation_EmptyPrompt(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + state := workflow.NewConversationState("") + result, err := provider.ExecuteConversation(context.Background(), state, "", nil, nil, nil) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "prompt cannot be empty") + assert.Nil(t, result) +} + +func TestCopilotProvider_ExecuteConversation_ContextCancellation(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + state := workflow.NewConversationState("") + result, err := provider.ExecuteConversation(ctx, state, "test", nil, nil, nil) + + assert.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "context canceled") +} + +func TestCopilotProvider_ExecuteConversation_CLIError(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetError(errors.New("command failed")) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + state := workflow.NewConversationState("") + result, err := provider.ExecuteConversation(context.Background(), state, "test", nil, nil, nil) + + assert.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "copilot execution failed") +} + +func TestCopilotProvider_Validate_BinaryMissing(t *testing.T) { + t.Setenv("PATH", "") + provider := NewCopilotProvider() + + err := provider.Validate() + + require.Error(t, err) + assert.Contains(t, err.Error(), "copilot CLI not found") +} + +func TestCopilotProvider_NewCopilotProvider_DefaultExecutor(t *testing.T) { + provider := NewCopilotProvider() + assert.NotNil(t, provider) + assert.NotNil(t, provider.executor) + _, ok := provider.executor.(*ExecCLIExecutor) + assert.True(t, ok, "default executor should be ExecCLIExecutor") +} + +func TestCopilotProvider_BuildExecuteArgs_MinimalInvocation(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"code\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + _, err := provider.Execute(context.Background(), "test", nil, nil, nil) + + require.NoError(t, err) + calls := mockExec.GetCalls() + require.Len(t, calls, 1) + // Should have: -p test --output-format=json --silent + args := calls[0].Args + assert.Contains(t, args, "-p") + assert.Contains(t, args, "test") + assert.Contains(t, args, "--output-format=json") + assert.Contains(t, args, "--silent") +} + +func TestCopilotProvider_BuildConversationArgs_FirstTurnNoSessionID(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"code\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + state := workflow.NewConversationState("") + _, err := provider.ExecuteConversation(context.Background(), state, "test", nil, nil, nil) + + require.NoError(t, err) + calls := mockExec.GetCalls() + require.Len(t, calls, 1) + args := calls[0].Args + // First turn should use -p format, not --resume + assert.NotContains(t, args, "--resume") + assert.Contains(t, args, "-p") + assert.Contains(t, args, "test") +} + +func TestCopilotProvider_BuildConversationArgs_ResumeTurn(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"code\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + state := workflow.NewConversationState("") + state.SessionID = "sess-xyz" + _, err := provider.ExecuteConversation(context.Background(), state, "follow-up", nil, nil, nil) + + require.NoError(t, err) + calls := mockExec.GetCalls() + require.Len(t, calls, 1) + args := calls[0].Args + // Resume turn should use --resume= format + assert.Contains(t, args, "--resume=sess-xyz") +} + +func TestCopilotProvider_AllowedToolsAsSliceAny(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"code\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + options := map[string]any{"allowed_tools": []any{"tool1", "tool2"}} + _, err := provider.Execute(context.Background(), "test", options, nil, nil) + + require.NoError(t, err) + calls := mockExec.GetCalls() + require.Len(t, calls, 1) + args := calls[0].Args + assert.Contains(t, args, "--allow-tool=tool1") + assert.Contains(t, args, "--allow-tool=tool2") +} + +func TestCopilotProvider_DeniedToolsMultiple(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"code\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + options := map[string]any{"denied_tools": []string{"bash", "python"}} + _, err := provider.Execute(context.Background(), "test", options, nil, nil) + + require.NoError(t, err) + calls := mockExec.GetCalls() + require.Len(t, calls, 1) + args := calls[0].Args + assert.Contains(t, args, "--deny-tool=bash") + assert.Contains(t, args, "--deny-tool=python") +} + +func TestCopilotProvider_AllowAllFalseOmitsFlag(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"code\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + options := map[string]any{"allow_all": false} + _, err := provider.Execute(context.Background(), "test", options, nil, nil) + + require.NoError(t, err) + calls := mockExec.GetCalls() + require.Len(t, calls, 1) + args := calls[0].Args + assert.NotContains(t, args, "--allow-all") +} + +func TestCopilotProvider_TokenEstimationOnExecute(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"hello world test\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + result, err := provider.Execute(context.Background(), "test", nil, nil, nil) + + require.NoError(t, err) + assert.True(t, result.TokensEstimated) + assert.Greater(t, result.Tokens, 0) +} + +func TestCopilotProvider_TokenEstimationOnExecuteConversation(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"response text\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + state := workflow.NewConversationState("") + result, err := provider.ExecuteConversation(context.Background(), state, "test", nil, nil, nil) + + require.NoError(t, err) + assert.True(t, result.TokensEstimated) +} diff --git a/internal/infrastructure/agents/copilot_provider_validation_unit_test.go b/internal/infrastructure/agents/copilot_provider_validation_unit_test.go new file mode 100644 index 00000000..09a3f009 --- /dev/null +++ b/internal/infrastructure/agents/copilot_provider_validation_unit_test.go @@ -0,0 +1,216 @@ +package agents + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Tests for validateCopilotOptions enum validation +// Implements F089: GitHub Copilot Agent Provider Integration +// Scope: mode and effort option validation + +func TestValidateCopilotOptions_ValidMode(t *testing.T) { + tests := []struct { + name string + options map[string]any + wantErr bool + }{ + { + name: "mode interactive", + options: map[string]any{"mode": "interactive"}, + wantErr: false, + }, + { + name: "mode plan", + options: map[string]any{"mode": "plan"}, + wantErr: false, + }, + { + name: "mode autopilot", + options: map[string]any{"mode": "autopilot"}, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateCopilotOptions(tt.options) + if tt.wantErr { + assert.Error(t, err, "validateCopilotOptions should return error") + } else { + assert.NoError(t, err, "validateCopilotOptions should not return error") + } + }) + } +} + +func TestValidateCopilotOptions_InvalidMode(t *testing.T) { + tests := []struct { + name string + options map[string]any + wantErr bool + errMsg string + }{ + { + name: "mode invalid_mode", + options: map[string]any{"mode": "invalid_mode"}, + wantErr: true, + errMsg: "invalid mode", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateCopilotOptions(tt.options) + + require.Error(t, err, "validateCopilotOptions should return error for %q", tt.options["mode"]) + assert.Contains(t, err.Error(), tt.errMsg, "error message should contain expected text") + assert.Contains(t, err.Error(), "interactive", "error should mention valid mode values") + assert.Contains(t, err.Error(), "plan", "error should mention valid mode values") + assert.Contains(t, err.Error(), "autopilot", "error should mention valid mode values") + assert.Contains(t, err.Error(), "invalid_mode", "error should include the invalid value") + }) + } +} + +func TestValidateCopilotOptions_ValidEffort(t *testing.T) { + tests := []struct { + name string + options map[string]any + wantErr bool + }{ + { + name: "effort low", + options: map[string]any{"effort": "low"}, + wantErr: false, + }, + { + name: "effort medium", + options: map[string]any{"effort": "medium"}, + wantErr: false, + }, + { + name: "effort high", + options: map[string]any{"effort": "high"}, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateCopilotOptions(tt.options) + if tt.wantErr { + assert.Error(t, err, "validateCopilotOptions should return error") + } else { + assert.NoError(t, err, "validateCopilotOptions should not return error") + } + }) + } +} + +func TestValidateCopilotOptions_InvalidEffort(t *testing.T) { + tests := []struct { + name string + options map[string]any + wantErr bool + errMsg string + }{ + { + name: "effort ultra", + options: map[string]any{"effort": "ultra"}, + wantErr: true, + errMsg: "invalid effort", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateCopilotOptions(tt.options) + + require.Error(t, err, "validateCopilotOptions should return error for %q", tt.options["effort"]) + assert.Contains(t, err.Error(), tt.errMsg, "error message should contain expected text") + assert.Contains(t, err.Error(), "low", "error should mention valid effort values") + assert.Contains(t, err.Error(), "medium", "error should mention valid effort values") + assert.Contains(t, err.Error(), "high", "error should mention valid effort values") + assert.Contains(t, err.Error(), "ultra", "error should include the invalid value") + }) + } +} + +func TestValidateCopilotOptions_EmptyOptions(t *testing.T) { + tests := []struct { + name string + options map[string]any + wantErr bool + }{ + { + name: "empty options map", + options: map[string]any{}, + wantErr: false, + }, + { + name: "nil options", + options: nil, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateCopilotOptions(tt.options) + if tt.wantErr { + assert.Error(t, err, "validateCopilotOptions should return error") + } else { + assert.NoError(t, err, "validateCopilotOptions should not return error") + } + }) + } +} + +func TestValidateCopilotOptions_UnknownKeysIgnored(t *testing.T) { + options := map[string]any{ + "foo": "bar", + "baz": 42, + } + + err := validateCopilotOptions(options) + assert.NoError(t, err, "validateCopilotOptions should silently ignore unknown keys") +} + +func TestValidateCopilotOptions_UnknownKeysWithValidOptions(t *testing.T) { + options := map[string]any{ + "mode": "interactive", + "foo": "bar", + } + + err := validateCopilotOptions(options) + assert.NoError(t, err, "validateCopilotOptions should silently ignore unknown keys with valid options") +} + +func TestValidateCopilotOptions_BothInvalid(t *testing.T) { + options := map[string]any{ + "mode": "invalid_mode", + "effort": "ultra", + } + + err := validateCopilotOptions(options) + + require.Error(t, err, "validateCopilotOptions should return error for both invalid mode and effort") + assert.Contains(t, err.Error(), "invalid_mode", "error should include the invalid mode value") + assert.Contains(t, err.Error(), "ultra", "error should include the invalid effort value") +} + +func TestValidateCopilotOptions_WithOtherOptions(t *testing.T) { + options := map[string]any{ + "mode": "interactive", + "effort": "high", + "model": "claude-opus", + "system_prompt": "Be helpful", + "allowed_tools": []string{"tool1", "tool2"}, + } + + err := validateCopilotOptions(options) + assert.NoError(t, err, "validateCopilotOptions should only validate mode and effort, ignore others") +} diff --git a/internal/infrastructure/agents/options.go b/internal/infrastructure/agents/options.go index abe989b7..eda912d1 100644 --- a/internal/infrastructure/agents/options.go +++ b/internal/infrastructure/agents/options.go @@ -49,6 +49,20 @@ func WithOpenCodeLogger(l ports.Logger) OpenCodeProviderOption { } } +type CopilotProviderOption func(*CopilotProvider) + +func WithCopilotExecutor(executor ports.CLIExecutor) CopilotProviderOption { + return func(p *CopilotProvider) { + p.executor = executor + } +} + +func WithCopilotLogger(l ports.Logger) CopilotProviderOption { + return func(p *CopilotProvider) { + p.logger = l + } +} + type OpenAICompatibleProviderOption func(*OpenAICompatibleProvider) func WithHTTPClient(client *httpx.Client) OpenAICompatibleProviderOption { diff --git a/internal/infrastructure/agents/provider_options_test.go b/internal/infrastructure/agents/provider_options_test.go index 48b583f5..f0fa2c40 100644 --- a/internal/infrastructure/agents/provider_options_test.go +++ b/internal/infrastructure/agents/provider_options_test.go @@ -245,6 +245,70 @@ func TestOpenCodeProvider_NewWithOptions_HappyPath(t *testing.T) { } } +func TestCopilotProvider_NewWithOptions_HappyPath(t *testing.T) { + tests := []struct { + name string + setupMock func(*mocks.MockCLIExecutor) + options []CopilotProviderOption + }{ + { + name: "no options uses default executor", + setupMock: func(m *mocks.MockCLIExecutor) { + m.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"copilot output\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), []byte("")) + }, + options: nil, + }, + { + name: "with custom executor option", + setupMock: func(m *mocks.MockCLIExecutor) { + m.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"custom copilot output\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), []byte("")) + }, + options: []CopilotProviderOption{ + WithCopilotExecutor(mocks.NewMockCLIExecutor()), + }, + }, + { + name: "with custom logger option", + setupMock: func(m *mocks.MockCLIExecutor) { + m.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"copilot with logger\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), []byte("")) + }, + options: []CopilotProviderOption{ + WithCopilotExecutor(mocks.NewMockCLIExecutor()), + WithCopilotLogger(&mocks.MockLogger{}), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var mockExec *mocks.MockCLIExecutor + var opts []CopilotProviderOption + if tt.setupMock != nil { + mockExec = mocks.NewMockCLIExecutor() + tt.setupMock(mockExec) + // Always use mock executor for testing + opts = []CopilotProviderOption{WithCopilotExecutor(mockExec)} + } else if tt.options != nil { + opts = tt.options + } + + provider := NewCopilotProviderWithOptions(opts...) + + require.NotNil(t, provider) + assert.NotNil(t, provider.executor) + assert.NotNil(t, provider.logger) + + // Verify executor is functional + if mockExec != nil { + ctx := context.Background() + result, err := provider.Execute(ctx, "test prompt", nil, nil, nil) + assert.NoError(t, err) + assert.NotNil(t, result) + } + }) + } +} + func TestProviderOptions_EdgeCases(t *testing.T) { t.Run("nil executor option panics are prevented", func(t *testing.T) { // Note: Passing nil executor should work but will cause runtime issues later @@ -375,6 +439,20 @@ func TestProviderOptions_ErrorHandling(t *testing.T) { assert.Nil(t, result) assert.Contains(t, err.Error(), "opencode execution failed") }) + + t.Run("copilot provider executor error propagates", func(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetError(errors.New("copilot CLI failed")) + + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + ctx := context.Background() + + result, err := provider.Execute(ctx, "test prompt", nil, nil, nil) + + assert.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "copilot execution failed") + }) } func TestProviderOptions_Integration(t *testing.T) { @@ -494,4 +572,24 @@ func TestProviderOptions_Integration(t *testing.T) { geminiCalls := geminiMock.GetCalls() require.Len(t, geminiCalls, 1) }) + + t.Run("copilot provider with mock executor executes successfully", func(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"Copilot response\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), []byte("")) + + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + ctx := context.Background() + + result, err := provider.Execute(ctx, "Generate code", nil, nil, nil) + + require.NoError(t, err) + require.NotNil(t, result) + assert.Equal(t, "github_copilot", result.Provider) + assert.Contains(t, result.Output, "Copilot response") + + // Verify executor was called correctly + calls := mockExec.GetCalls() + require.Len(t, calls, 1) + assert.Equal(t, "copilot", calls[0].Name) + }) } diff --git a/internal/infrastructure/agents/registry.go b/internal/infrastructure/agents/registry.go index 09978c41..c1fb96a0 100644 --- a/internal/infrastructure/agents/registry.go +++ b/internal/infrastructure/agents/registry.go @@ -77,6 +77,7 @@ func (r *AgentRegistry) RegisterDefaults() error { NewGeminiProvider(), NewOpenAICompatibleProvider(), NewOpenCodeProvider(), + NewCopilotProvider(), } var errs []error diff --git a/internal/infrastructure/agents/registry_test.go b/internal/infrastructure/agents/registry_test.go index 9d0b5810..194ee424 100644 --- a/internal/infrastructure/agents/registry_test.go +++ b/internal/infrastructure/agents/registry_test.go @@ -269,10 +269,11 @@ func TestAgentRegistry_RegisterDefaults(t *testing.T) { // Verify default providers are registered list := registry.List() - assert.Len(t, list, 5) + assert.Len(t, list, 6) assert.Contains(t, list, "claude") assert.Contains(t, list, "codex") assert.Contains(t, list, "gemini") + assert.Contains(t, list, "github_copilot") assert.Contains(t, list, "openai_compatible") assert.Contains(t, list, "opencode") } @@ -281,7 +282,7 @@ func TestAgentRegistry_RegisterDefaults_EachProviderRetrievable(t *testing.T) { registry := NewAgentRegistry() _ = registry.RegisterDefaults() - tests := []string{"claude", "codex", "gemini", "openai_compatible", "opencode"} + tests := []string{"claude", "codex", "gemini", "github_copilot", "openai_compatible", "opencode"} for _, name := range tests { t.Run(name, func(t *testing.T) { @@ -446,15 +447,16 @@ func TestAgentRegistry_RegisterDefaults_PartialFailure(t *testing.T) { // Verify RegisterDefaults continues on error - other providers should be registered list := registry.List() - assert.Len(t, list, 5, "All 5 default providers should be registered (1 pre-existing + 4 new)") + assert.Len(t, list, 6, "All 6 default providers should be registered (1 pre-existing + 5 new)") assert.Contains(t, list, "claude") assert.Contains(t, list, "codex") assert.Contains(t, list, "gemini") + assert.Contains(t, list, "github_copilot") assert.Contains(t, list, "openai_compatible") assert.Contains(t, list, "opencode") // Verify each provider is retrievable - for _, name := range []string{"claude", "codex", "gemini", "openai_compatible", "opencode"} { + for _, name := range []string{"claude", "codex", "gemini", "github_copilot", "openai_compatible", "opencode"} { provider, getErr := registry.Get(name) assert.NoError(t, getErr, "Provider %s should be retrievable", name) assert.NotNil(t, provider) @@ -462,38 +464,6 @@ func TestAgentRegistry_RegisterDefaults_PartialFailure(t *testing.T) { } } -func TestAgentRegistry_RegisterDefaults_EmptyRegistry(t *testing.T) { - // Create fresh registry, call RegisterDefaults - // Verify: no error returned - // Verify: all 4 default providers registered (claude, gemini, codex, opencode) - registry := NewAgentRegistry() - - err := registry.RegisterDefaults() - - // Should succeed without errors - require.NoError(t, err) - - // Verify all 5 default providers are registered - list := registry.List() - assert.Len(t, list, 5) - assert.Contains(t, list, "claude") - assert.Contains(t, list, "codex") - assert.Contains(t, list, "gemini") - assert.Contains(t, list, "openai_compatible") - assert.Contains(t, list, "opencode") - - // Verify each provider is retrievable and functional - for _, name := range []string{"claude", "codex", "gemini", "openai_compatible", "opencode"} { - provider, getErr := registry.Get(name) - require.NoError(t, getErr, "Provider %s should be retrievable", name) - require.NotNil(t, provider) - assert.Equal(t, name, provider.Name()) - - // Verify provider has Has() check - assert.True(t, registry.Has(name)) - } -} - func TestAgentRegistry_RegisterDefaults_MultiplePreRegistered(t *testing.T) { // Edge case: pre-register multiple default providers registry := NewAgentRegistry() @@ -509,12 +479,13 @@ func TestAgentRegistry_RegisterDefaults_MultiplePreRegistered(t *testing.T) { assert.Contains(t, err.Error(), "claude") assert.Contains(t, err.Error(), "gemini") - // All 5 providers should still be registered + // All 6 providers should still be registered list := registry.List() - assert.Len(t, list, 5) + assert.Len(t, list, 6) assert.Contains(t, list, "claude") assert.Contains(t, list, "codex") assert.Contains(t, list, "gemini") + assert.Contains(t, list, "github_copilot") assert.Contains(t, list, "openai_compatible") assert.Contains(t, list, "opencode") } @@ -530,15 +501,16 @@ func TestAgentRegistry_RegisterDefaults_AllPreRegistered(t *testing.T) { // Try to register defaults again err2 := registry.RegisterDefaults() - // Should fail with aggregated error for all 5 providers + // Should fail with aggregated error for all 6 providers assert.Error(t, err2) assert.Contains(t, err2.Error(), "claude") assert.Contains(t, err2.Error(), "codex") assert.Contains(t, err2.Error(), "gemini") + assert.Contains(t, err2.Error(), "github_copilot") assert.Contains(t, err2.Error(), "openai_compatible") assert.Contains(t, err2.Error(), "opencode") - // Should still have exactly 5 providers (no duplicates) + // Should still have exactly 6 providers (no duplicates) list := registry.List() - assert.Len(t, list, 5) + assert.Len(t, list, 6) } diff --git a/internal/infrastructure/agents/stream_filter.go b/internal/infrastructure/agents/stream_filter.go index 344e0f23..6d8fa04b 100644 --- a/internal/infrastructure/agents/stream_filter.go +++ b/internal/infrastructure/agents/stream_filter.go @@ -118,10 +118,20 @@ func (w *StreamFilterWriter) parseAndRenderLine(line []byte) error { if w.renderer != nil { w.renderer(events) } + return w.writeTextEvents(events) +} + +func (w *StreamFilterWriter) writeTextEvents(events []DisplayEvent) error { for _, event := range events { if event.Kind == EventText && event.Text != "" { - if err := w.writeExtracted(event.Text); err != nil { - return err + if event.Delta { + if _, err := io.WriteString(w.inner, event.Text); err != nil { + return fmt.Errorf("write delta text: %w", err) + } + } else { + if err := w.writeExtracted(event.Text); err != nil { + return err + } } } } @@ -140,12 +150,8 @@ func (w *StreamFilterWriter) Flush() error { if w.renderer != nil { w.renderer(events) } - for _, event := range events { - if event.Kind == EventText && event.Text != "" { - if err := w.writeExtracted(event.Text); err != nil { - return err - } - } + if err := w.writeTextEvents(events); err != nil { + return err } } w.buf = w.buf[:0] diff --git a/pkg/display/event.go b/pkg/display/event.go index b58df868..24b9369b 100644 --- a/pkg/display/event.go +++ b/pkg/display/event.go @@ -13,13 +13,15 @@ const ( // DisplayEvent represents a parsed event from a provider's streaming output. // Kind discriminates the event type for rendering; Type holds the raw provider JSON event type. // Text holds displayable output for EventText events; Name, Arg, ID are populated for EventToolUse events. +// Delta indicates the event is a streaming token fragment that should be written without a trailing newline. type DisplayEvent struct { - Type string - Kind EventKind - Text string - Name string - Arg string - ID string + Type string + Kind EventKind + Text string + Name string + Arg string + ID string + Delta bool } // DisplayMode controls which event categories are rendered to the writer.