diff --git a/cmd/roborev/ghaction.go b/cmd/roborev/ghaction.go index 92a3fa3c..b1434fb4 100644 --- a/cmd/roborev/ghaction.go +++ b/cmd/roborev/ghaction.go @@ -65,13 +65,14 @@ After generating the workflow, add repository secrets ` + // List required secrets per agent infos := ghaction.AgentSecrets(cfg.Agents) for i, info := range infos { - if info.Name == "opencode" { + if info.Name == "opencode" || + info.Name == "kilo" { fmt.Printf( " %d. Add a repository secret "+ "named %q (default for "+ - "opencode; change if using "+ + "%s; change if using "+ "a different provider)\n", - i+1, info.SecretName) + i+1, info.SecretName, info.Name) } else { fmt.Printf( " %d. Add a repository secret "+ @@ -97,7 +98,7 @@ After generating the workflow, add repository secrets ` + cmd.Flags().StringVar(&agentFlag, "agent", "", "agents to use, comma-separated "+ "(codex, claude-code, gemini, copilot, "+ - "opencode, cursor, droid)") + "opencode, cursor, kiro, kilo, droid)") cmd.Flags().StringVar(&outputPath, "output", "", "output path for workflow file "+ "(default: .github/workflows/roborev.yml)") diff --git a/cmd/roborev/ghaction_test.go b/cmd/roborev/ghaction_test.go index 8d9eada1..7b91b1ba 100644 --- a/cmd/roborev/ghaction_test.go +++ b/cmd/roborev/ghaction_test.go @@ -36,6 +36,7 @@ func TestGhActionCmd(t *testing.T) { expectError bool errorContains string expectedContains []string + notContains []string }{ { name: "default flags", @@ -70,6 +71,27 @@ func TestGhActionCmd(t *testing.T) { flags: []string{"--agent", "codex"}, expectedContains: []string{"OPENAI_API_KEY"}, }, + { + name: "kilo gets multi-provider guidance", + flags: []string{"--agent", "kilo"}, + expectedContains: []string{ + "ANTHROPIC_API_KEY", + "@kilocode/cli@latest", + "different model provider", + "default for kilo", + }, + }, + { + name: "kiro has no secret in env block", + flags: []string{"--agent", "kiro"}, + expectedContains: []string{ + "kiro.dev", + }, + notContains: []string{ + "OPENAI_API_KEY:", + "AWS_ACCESS_KEY_ID:", + }, + }, { name: "infers agents from repo CI config", repoConfig: "[ci]\nagents = " + @@ -120,7 +142,8 @@ func TestGhActionCmd(t *testing.T) { t.Fatalf("unexpected error: %v", err) } - if len(tt.expectedContains) > 0 { + if len(tt.expectedContains) > 0 || + len(tt.notContains) > 0 { contentBytes, err := os.ReadFile(outPath) if err != nil { t.Fatalf("failed to read generated file: %v", err) @@ -131,6 +154,11 @@ func TestGhActionCmd(t *testing.T) { t.Errorf("generated file missing expected content: %q", expected) } } + for _, bad := range tt.notContains { + if strings.Contains(content, bad) { + t.Errorf("generated file should not contain: %q", bad) + } + } } }) } diff --git a/cmd/roborev/init_cmd.go b/cmd/roborev/init_cmd.go index fa600c84..c78d2834 100644 --- a/cmd/roborev/init_cmd.go +++ b/cmd/roborev/init_cmd.go @@ -125,7 +125,7 @@ func initCmd() *cobra.Command { }, } - cmd.Flags().StringVar(&agent, "agent", "", "default agent (codex, claude-code, gemini, copilot, opencode, cursor, kilo)") + cmd.Flags().StringVar(&agent, "agent", "", "default agent (codex, claude-code, gemini, copilot, opencode, cursor, kiro, kilo)") cmd.Flags().BoolVar(&noDaemon, "no-daemon", false, "skip auto-starting daemon (useful with systemd/launchd)") registerAgentCompletion(cmd) diff --git a/cmd/roborev/main.go b/cmd/roborev/main.go index 20f7b8c8..67ce95d4 100644 --- a/cmd/roborev/main.go +++ b/cmd/roborev/main.go @@ -15,7 +15,7 @@ func main() { rootCmd := &cobra.Command{ Use: "roborev", Short: "Automatic code review for git commits", - Long: "roborev automatically reviews git commits using AI agents (Codex, Claude Code, Gemini, Copilot, OpenCode, Cursor)", + Long: "roborev automatically reviews git commits using AI agents (Codex, Claude Code, Gemini, Copilot, OpenCode, Cursor, Kiro)", } rootCmd.PersistentFlags().StringVar(&serverAddr, "server", "http://127.0.0.1:7373", "daemon server address") diff --git a/cmd/roborev/review.go b/cmd/roborev/review.go index 43b5f657..cb52766b 100644 --- a/cmd/roborev/review.go +++ b/cmd/roborev/review.go @@ -322,7 +322,7 @@ Examples: cmd.Flags().StringVar(&repoPath, "repo", "", "path to git repository (default: current directory)") cmd.Flags().StringVar(&sha, "sha", "HEAD", "commit SHA to review (used when no positional args)") - cmd.Flags().StringVar(&agent, "agent", "", "agent to use (codex, claude-code, gemini, copilot, opencode, cursor, kilo)") + cmd.Flags().StringVar(&agent, "agent", "", "agent to use (codex, claude-code, gemini, copilot, opencode, cursor, kiro, kilo)") cmd.Flags().StringVar(&model, "model", "", "model for agent (format varies: opencode uses provider/model, others use model name)") cmd.Flags().StringVar(&reasoning, "reasoning", "", "reasoning level: thorough (default), standard, or fast") cmd.Flags().BoolVar(&fast, "fast", false, "shorthand for --reasoning fast") diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 662787b6..41d2b5ea 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -177,8 +177,8 @@ func GetAvailable(preferred string) (Agent, error) { return Get(preferred) } - // Fallback order: codex, claude-code, gemini, copilot, opencode, cursor, kilo, droid - fallbacks := []string{"codex", "claude-code", "gemini", "copilot", "opencode", "cursor", "kilo", "droid"} + // Fallback order: codex, claude-code, gemini, copilot, opencode, cursor, kiro, kilo, droid + fallbacks := []string{"codex", "claude-code", "gemini", "copilot", "opencode", "cursor", "kiro", "kilo", "droid"} for _, name := range fallbacks { if name != preferred && IsAvailable(name) { return Get(name) @@ -194,7 +194,7 @@ func GetAvailable(preferred string) (Agent, error) { } if len(available) == 0 { - return nil, fmt.Errorf("no agents available (install one of: codex, claude-code, gemini, copilot, opencode, cursor, kilo, droid)\nYou may need to run 'roborev daemon restart' from a shell that has access to your agents") + return nil, fmt.Errorf("no agents available (install one of: codex, claude-code, gemini, copilot, opencode, cursor, kiro, kilo, droid)\nYou may need to run 'roborev daemon restart' from a shell that has access to your agents") } return Get(available[0]) diff --git a/internal/agent/agent_test_helpers.go b/internal/agent/agent_test_helpers.go index cdd10269..c43e41fd 100644 --- a/internal/agent/agent_test_helpers.go +++ b/internal/agent/agent_test_helpers.go @@ -16,7 +16,7 @@ import ( ) // expectedAgents is the single source of truth for registered agent names. -var expectedAgents = []string{"codex", "claude-code", "gemini", "copilot", "opencode", "cursor", "kilo", "droid", "test"} +var expectedAgents = []string{"codex", "claude-code", "gemini", "copilot", "opencode", "cursor", "kiro", "kilo", "droid", "test"} // verifyAgentPassesFlag creates a mock command that echoes args, runs the agent's Review method, // and validates that the output contains the expected flag and value. diff --git a/internal/agent/kiro.go b/internal/agent/kiro.go new file mode 100644 index 00000000..b85c9ec8 --- /dev/null +++ b/internal/agent/kiro.go @@ -0,0 +1,191 @@ +package agent + +import ( + "bytes" + "context" + "fmt" + "io" + "os" + "os/exec" + "strings" + "time" +) + +// maxPromptArgLen is a conservative limit for passing prompts as +// CLI arguments. macOS ARG_MAX is ~1 MB; we leave headroom for +// the command name, flags, and environment. +const maxPromptArgLen = 512 * 1024 + +// stripKiroOutput removes Kiro's UI chrome (logo, tip box, model line, timing footer) +// and terminal control sequences, returning only the review text. +func stripKiroOutput(raw string) string { + text, _ := stripKiroReview(raw) + return text +} + +// stripKiroReview strips Kiro chrome and returns the cleaned text +// plus a bool indicating whether a "> " review marker was found. +// When no marker is found the full ANSI-stripped text is returned +// (hasMarker == false), which may be non-review noise. +func stripKiroReview(raw string) (string, bool) { + s := stripTerminalControls(raw) + + // Kiro prepends a splash screen and tip box before the response. + // The "> " prompt marker appears near the top; limit the search + // to avoid mistaking markdown blockquotes for the start marker. + lines := strings.Split(s, "\n") + limit := min(30, len(lines)) + start := -1 + for i, line := range lines[:limit] { + if strings.HasPrefix(line, "> ") || line == ">" { + start = i + break + } + } + if start == -1 { + return strings.TrimSpace(s), false + } + + // Strip the prompt marker from the first content line. + // A bare ">" (no trailing content) is skipped entirely. + if lines[start] == ">" { + start++ + if start >= len(lines) { + return "", true + } + } else { + lines[start] = strings.TrimPrefix(lines[start], "> ") + } + + // Drop the timing footer ("▸ Time: Xs") and anything after it. + // Trim trailing blank lines first so they don't push the real + // footer outside the scan window, then scan the last 5 non-blank + // lines to avoid truncating review content that happens to + // contain "▸ Time:" in a code snippet. + end := len(lines) + for end > start && strings.TrimSpace(lines[end-1]) == "" { + end-- + } + scanFrom := max(start, end-5) + for i := scanFrom; i < end; i++ { + if strings.HasPrefix(strings.TrimSpace(lines[i]), "▸ Time:") { + end = i + break + } + } + + return strings.TrimSpace(strings.Join(lines[start:end], "\n")), true +} + +// KiroAgent runs code reviews using the Kiro CLI (kiro-cli) +type KiroAgent struct { + Command string // The kiro-cli command to run (default: "kiro-cli") + Reasoning ReasoningLevel // Reasoning level (stored; kiro-cli has no reasoning flag) + Agentic bool // Whether agentic mode is enabled (uses --trust-all-tools) +} + +// NewKiroAgent creates a new Kiro agent with standard reasoning +func NewKiroAgent(command string) *KiroAgent { + if command == "" { + command = "kiro-cli" + } + return &KiroAgent{Command: command, Reasoning: ReasoningStandard} +} + +// WithReasoning returns a copy with the reasoning level stored. +// kiro-cli has no reasoning flag; callers can map reasoning to agent selection instead. +func (a *KiroAgent) WithReasoning(level ReasoningLevel) Agent { + return &KiroAgent{Command: a.Command, Reasoning: level, Agentic: a.Agentic} +} + +// WithAgentic returns a copy of the agent configured for agentic mode. +// In agentic mode, --trust-all-tools is passed so kiro can use tools without confirmation. +func (a *KiroAgent) WithAgentic(agentic bool) Agent { + return &KiroAgent{Command: a.Command, Reasoning: a.Reasoning, Agentic: agentic} +} + +// WithModel returns the agent unchanged; kiro-cli does not expose a --model CLI flag. +func (a *KiroAgent) WithModel(model string) Agent { + return a +} + +func (a *KiroAgent) Name() string { + return "kiro" +} + +func (a *KiroAgent) CommandName() string { + return a.Command +} + +func (a *KiroAgent) buildArgs(agenticMode bool) []string { + args := []string{"chat", "--no-interactive"} + if agenticMode { + args = append(args, "--trust-all-tools") + } + return args +} + +func (a *KiroAgent) CommandLine() string { + agenticMode := a.Agentic || AllowUnsafeAgents() + args := a.buildArgs(agenticMode) + return a.Command + " " + strings.Join(args, " ") + " -- " +} + +func (a *KiroAgent) Review(ctx context.Context, repoPath, commitSHA, prompt string, output io.Writer) (string, error) { + if len(prompt) > maxPromptArgLen { + return "", fmt.Errorf( + "prompt too large for kiro-cli argv (%d bytes, max %d)", + len(prompt), maxPromptArgLen, + ) + } + + agenticMode := a.Agentic || AllowUnsafeAgents() + + // kiro-cli chat --no-interactive [--trust-all-tools] + // The prompt is passed as a positional argument + // (kiro-cli does not support stdin). + args := a.buildArgs(agenticMode) + args = append(args, "--", prompt) + + cmd := exec.CommandContext(ctx, a.Command, args...) + cmd.Dir = repoPath + cmd.Env = os.Environ() + cmd.WaitDelay = 5 * time.Second + + // kiro-cli emits ANSI terminal escape codes that are not + // suitable for streaming. Capture and return stripped text. + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + if err := cmd.Run(); err != nil { + return "", fmt.Errorf( + "kiro failed: %w\nstderr: %s", + err, stderr.String(), + ) + } + + // Prefer the stream that contains a "> " review marker. + // - stdout with marker and content → use stdout + // - stdout empty or marker-only → try stderr + // - stdout has content but no marker → use stderr only + // if stderr has a marker (otherwise keep stdout) + result, stdoutMarker := stripKiroReview(stdout.String()) + if !stdoutMarker || len(result) == 0 { + alt, stderrMarker := stripKiroReview(stderr.String()) + if len(alt) > 0 && (len(result) == 0 || stderrMarker) { + result = alt + } + } + if len(result) == 0 { + return "No review output generated", nil + } + if sw := newSyncWriter(output); sw != nil { + _, _ = sw.Write([]byte(result + "\n")) + } + return result, nil +} + +func init() { + Register(NewKiroAgent("")) +} diff --git a/internal/agent/kiro_test.go b/internal/agent/kiro_test.go new file mode 100644 index 00000000..06ff8b8c --- /dev/null +++ b/internal/agent/kiro_test.go @@ -0,0 +1,522 @@ +package agent + +import ( + "bytes" + "context" + "os" + "strings" + "testing" +) + +func TestStripKiroOutput(t *testing.T) { + // Simulated kiro-cli stdout with ANSI codes, logo, tip box, model line, and timing footer. + raw := "\x1b[38;5;141m⠀⠀logo⠀⠀\x1b[0m\n" + + "\x1b[38;5;244m╭─── Did you know? ───╮\x1b[0m\n" + + "\x1b[38;5;244m│ tip text │\x1b[0m\n" + + "\x1b[38;5;244m╰─────────────────────╯\x1b[0m\n" + + "\x1b[38;5;244mModel: auto\x1b[0m\n" + + "\n" + + "\x1b[38;5;141m> \x1b[0m\x1b[1m## Summary\x1b[0m\n" + + "This commit does something.\n" + + "\n" + + "## Issues Found\n" + + "\n" + + " \u25b8 Time: 21s\n" + + got := stripKiroOutput(raw) + + if strings.Contains(got, "\x1b[") { + t.Error("result still contains ANSI escape codes") + } + if !strings.Contains(got, "## Summary") { + t.Errorf("expected '## Summary' in result, got: %q", got) + } + if !strings.Contains(got, "## Issues Found") { + t.Errorf("expected '## Issues Found' in result, got: %q", got) + } + if strings.Contains(got, "Did you know") { + t.Error("result should not contain tip box text") + } + if strings.Contains(got, "Model:") { + t.Error("result should not contain model line") + } + if strings.Contains(got, "Time:") { + t.Error("result should not contain timing footer") + } + if strings.HasPrefix(got, "> ") { + t.Error("result should not have leading '> ' prefix") + } +} + +func TestStripKiroOutputNoMarker(t *testing.T) { + // If there's no "> " marker in the first 30 lines, return ANSI-stripped text as-is. + raw := "\x1b[1msome output without marker\x1b[0m\n" + got := stripKiroOutput(raw) + if got != "some output without marker" { + t.Errorf("unexpected result: %q", got) + } +} + +func TestStripKiroOutputBareMarker(t *testing.T) { + // A bare ">" (no trailing space) followed by content on the next line. + raw := "chrome\n>\nreview content here\n" + got := stripKiroOutput(raw) + if !strings.Contains(got, "review content here") { + t.Errorf("expected review content, got: %q", got) + } + if strings.Contains(got, "chrome") { + t.Error("chrome should be stripped before the bare > marker") + } + if strings.HasPrefix(got, ">") { + t.Error("bare > marker should not appear in output") + } +} + +func TestStripKiroOutputFooterInContent(t *testing.T) { + // "▸ Time:" appearing in review content (not in the last 5 lines) + // should not be treated as a footer. + var lines []string + lines = append(lines, "> ## Review") + lines = append(lines, "some content") + lines = append(lines, "▸ Time: 10s") // looks like footer but is in content + for range 10 { + lines = append(lines, "more content") + } + raw := strings.Join(lines, "\n") + + got := stripKiroOutput(raw) + if !strings.Contains(got, "▸ Time: 10s") { + t.Errorf( + "'▸ Time:' in content body should be preserved, got: %q", + got, + ) + } +} + +func TestStripKiroOutputFooterWithTrailingBlanks(t *testing.T) { + // Footer followed by trailing blank lines should still be stripped. + raw := "> ## Review\ncontent\n ▸ Time: 12s\n\n\n\n\n\n\n\n" + got := stripKiroOutput(raw) + if strings.Contains(got, "Time:") { + t.Errorf("footer should be stripped despite trailing blanks, got: %q", got) + } + if !strings.Contains(got, "content") { + t.Errorf("review content should be preserved, got: %q", got) + } +} + +func TestStripKiroOutputBlockquoteNotStripped(t *testing.T) { + // A "> " blockquote deep in review content should not be treated as the start marker. + // Build output where "> " only appears after line 30. + var lines []string + for range 31 { + lines = append(lines, "chrome line") + } + lines = append(lines, "> this is a blockquote in review content") + lines = append(lines, "more content") + raw := strings.Join(lines, "\n") + + got := stripKiroOutput(raw) + if !strings.Contains(got, "> this is a blockquote") { + t.Errorf("blockquote should be preserved in output, got: %q", got) + } +} + +func TestKiroBuildArgs(t *testing.T) { + a := NewKiroAgent("kiro-cli") + + // Non-agentic mode: no --trust-all-tools + args := a.buildArgs(false) + assertContainsArg(t, args, "chat") + assertContainsArg(t, args, "--no-interactive") + assertNotContainsArg(t, args, "--trust-all-tools") + + // Agentic mode: adds --trust-all-tools + args = a.buildArgs(true) + assertContainsArg(t, args, "chat") + assertContainsArg(t, args, "--no-interactive") + assertContainsArg(t, args, "--trust-all-tools") +} + +func TestKiroName(t *testing.T) { + a := NewKiroAgent("") + if a.Name() != "kiro" { + t.Fatalf("expected name 'kiro', got %s", a.Name()) + } + if a.CommandName() != "kiro-cli" { + t.Fatalf("expected command name 'kiro-cli', got %s", a.CommandName()) + } +} + +func TestKiroCommandLine(t *testing.T) { + a := NewKiroAgent("kiro-cli") + cl := a.CommandLine() + if !strings.Contains(cl, "-- ") { + t.Errorf( + "CommandLine should include -- separator, got: %q", cl, + ) + } + if !strings.Contains(cl, "--no-interactive") { + t.Errorf( + "CommandLine should include --no-interactive, got: %q", cl, + ) + } +} + +func TestKiroWithAgentic(t *testing.T) { + a := NewKiroAgent("kiro-cli") + if a.Agentic { + t.Fatal("expected non-agentic by default") + } + + a2 := a.WithAgentic(true).(*KiroAgent) + if !a2.Agentic { + t.Fatal("expected agentic after WithAgentic(true)") + } + if a.Agentic { + t.Fatal("original should be unchanged") + } +} + +func TestKiroWithReasoning(t *testing.T) { + a := NewKiroAgent("kiro-cli") + b := a.WithReasoning(ReasoningThorough).(*KiroAgent) + if b.Reasoning != ReasoningThorough { + t.Errorf("expected thorough reasoning, got %q", b.Reasoning) + } + if a.Reasoning != ReasoningStandard { + t.Error("original reasoning should be unchanged") + } +} + +func TestKiroWithModelIsNoop(t *testing.T) { + a := NewKiroAgent("kiro-cli") + b := a.WithModel("some-model") + // kiro-cli has no --model flag; WithModel returns self unchanged + if b != a { + t.Error("WithModel should return the same agent (kiro does not support model selection)") + } +} + +func TestKiroReviewSuccess(t *testing.T) { + skipIfWindows(t) + + output := "LGTM: looks good to me" + script := NewScriptBuilder().AddOutput(output).Build() + cmdPath := writeTempCommand(t, script) + a := NewKiroAgent(cmdPath) + + result, err := a.Review(context.Background(), t.TempDir(), "deadbeef", "review this commit", nil) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if !strings.Contains(result, output) { + t.Fatalf("expected result to contain %q, got %q", output, result) + } +} + +func TestKiroReviewWritesOutputWriter(t *testing.T) { + skipIfWindows(t) + + output := "review findings here" + script := NewScriptBuilder().AddOutput(output).Build() + cmdPath := writeTempCommand(t, script) + a := NewKiroAgent(cmdPath) + + var buf bytes.Buffer + result, err := a.Review(context.Background(), t.TempDir(), "deadbeef", "review", &buf) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(result, output) { + t.Fatalf("result missing output: %q", result) + } + if !strings.Contains(buf.String(), output) { + t.Fatalf("output writer missing content: %q", buf.String()) + } +} + +func TestKiroReviewFailure(t *testing.T) { + skipIfWindows(t) + + script := NewScriptBuilder(). + AddRaw(`echo "error: auth failed" >&2`). + AddRaw("exit 1"). + Build() + cmdPath := writeTempCommand(t, script) + a := NewKiroAgent(cmdPath) + + _, err := a.Review(context.Background(), t.TempDir(), "deadbeef", "review this commit", nil) + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "kiro failed") { + t.Fatalf("expected 'kiro failed' in error, got %v", err) + } +} + +func TestKiroReviewEmptyOutput(t *testing.T) { + skipIfWindows(t) + + script := NewScriptBuilder().AddRaw("exit 0").Build() + cmdPath := writeTempCommand(t, script) + a := NewKiroAgent(cmdPath) + + result, err := a.Review(context.Background(), t.TempDir(), "deadbeef", "review this commit", nil) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if result != "No review output generated" { + t.Fatalf("expected 'No review output generated', got %q", result) + } +} + +func TestKiroPassesPromptAsArg(t *testing.T) { + skipIfWindows(t) + + mock := mockAgentCLI(t, MockCLIOpts{ + CaptureArgs: true, + StdoutLines: []string{"review complete"}, + }) + + a := NewKiroAgent(mock.CmdPath) + prompt := "Review this commit for issues" + _, err := a.Review(context.Background(), t.TempDir(), "HEAD", prompt, nil) + if err != nil { + t.Fatalf("Review failed: %v", err) + } + + args, err := os.ReadFile(mock.ArgsFile) + if err != nil { + t.Fatalf("read args capture: %v", err) + } + if !strings.Contains(string(args), prompt) { + t.Errorf("expected prompt in args, got: %s", string(args)) + } + if !strings.Contains(string(args), "--no-interactive") { + t.Errorf("expected --no-interactive in args, got: %s", string(args)) + } + if !strings.Contains(string(args), " -- ") { + t.Errorf("expected -- separator before prompt, got: %s", + string(args)) + } +} + +func TestKiroReviewAgenticMode(t *testing.T) { + skipIfWindows(t) + + mock := mockAgentCLI(t, MockCLIOpts{ + CaptureArgs: true, + StdoutLines: []string{"review complete"}, + }) + + a := NewKiroAgent(mock.CmdPath) + a2 := a.WithAgentic(true).(*KiroAgent) + + _, err := a2.Review(context.Background(), t.TempDir(), "HEAD", "prompt", nil) + if err != nil { + t.Fatalf("Review failed: %v", err) + } + + args, err := os.ReadFile(mock.ArgsFile) + if err != nil { + t.Fatalf("read args capture: %v", err) + } + if !strings.Contains(string(args), "--trust-all-tools") { + t.Errorf("expected --trust-all-tools in args, got: %s", string(args)) + } +} + +func TestKiroReviewAgenticModeFromGlobal(t *testing.T) { + withUnsafeAgents(t, true) + + mock := mockAgentCLI(t, MockCLIOpts{ + CaptureArgs: true, + StdoutLines: []string{"review complete"}, + }) + + a := NewKiroAgent(mock.CmdPath) + _, err := a.Review(context.Background(), t.TempDir(), "HEAD", "prompt", nil) + if err != nil { + t.Fatalf("Review failed: %v", err) + } + + args, err := os.ReadFile(mock.ArgsFile) + if err != nil { + t.Fatalf("read args capture: %v", err) + } + if !strings.Contains(string(args), "--trust-all-tools") { + t.Fatalf("expected --trust-all-tools when global unsafe enabled, got: %s", strings.TrimSpace(string(args))) + } +} + +func TestKiroReviewStderrFallback(t *testing.T) { + skipIfWindows(t) + + // kiro-cli exits 0 with Kiro chrome + review text on stderr, + // nothing on stdout. Validates that stripKiroOutput (not just + // stripTerminalControls) is applied to stderr. + script := NewScriptBuilder(). + AddRaw(`echo "Model: auto" >&2`). + AddRaw(`echo "> review on stderr" >&2`). + AddRaw(`echo " ▸ Time: 5s" >&2`). + Build() + cmdPath := writeTempCommand(t, script) + a := NewKiroAgent(cmdPath) + + result, err := a.Review(context.Background(), t.TempDir(), "deadbeef", "review", nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(result, "review on stderr") { + t.Fatalf("expected stderr fallback, got: %q", result) + } + if strings.Contains(result, "Model:") { + t.Error("Kiro chrome should be stripped from stderr") + } + if strings.Contains(result, "Time:") { + t.Error("timing footer should be stripped from stderr") + } +} + +func TestKiroReviewStderrFallbackMarkerOnlyStdout(t *testing.T) { + skipIfWindows(t) + + // stdout has only a bare ">" marker with no review body; + // stderr has the actual review. stderr should be used. + script := NewScriptBuilder(). + AddRaw(`echo ">"`). + AddRaw(`echo "> review from stderr" >&2`). + Build() + cmdPath := writeTempCommand(t, script) + a := NewKiroAgent(cmdPath) + + result, err := a.Review( + context.Background(), t.TempDir(), + "deadbeef", "review", nil, + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(result, "review from stderr") { + t.Fatalf( + "expected stderr when stdout is marker-only, got: %q", + result, + ) + } +} + +func TestKiroReviewStderrPreferredOverStdoutNoise(t *testing.T) { + skipIfWindows(t) + + // stdout has status noise without a "> " marker; + // stderr has the actual review with a marker. + // The fallback should prefer stderr. + script := NewScriptBuilder(). + AddRaw(`echo "Model: auto"`). + AddRaw(`echo "Loading..."`). + AddRaw(`echo "> actual review content" >&2`). + Build() + cmdPath := writeTempCommand(t, script) + a := NewKiroAgent(cmdPath) + + result, err := a.Review( + context.Background(), t.TempDir(), + "deadbeef", "review", nil, + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(result, "actual review content") { + t.Fatalf( + "expected stderr review over stdout noise, got: %q", + result, + ) + } + if strings.Contains(result, "Loading") { + t.Error("stdout noise should not appear in result") + } +} + +func TestKiroReviewStderrFallbackNoMarker(t *testing.T) { + skipIfWindows(t) + + // stdout is empty; stderr has plain review text without + // a marker. stderr should be used as fallback. + script := NewScriptBuilder(). + AddRaw(`echo "review text without marker" >&2`). + Build() + cmdPath := writeTempCommand(t, script) + a := NewKiroAgent(cmdPath) + + result, err := a.Review( + context.Background(), t.TempDir(), + "deadbeef", "review", nil, + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(result, "review text without marker") { + t.Fatalf( + "expected stderr fallback even without marker, got: %q", + result, + ) + } +} + +func TestKiroReviewStdoutPreservedOverStderrNoise(t *testing.T) { + skipIfWindows(t) + + // Both streams have content, neither has a marker. + // Stdout (primary stream) should be kept. + script := NewScriptBuilder(). + AddRaw(`echo "plain review on stdout"`). + AddRaw(`echo "warning: something" >&2`). + Build() + cmdPath := writeTempCommand(t, script) + a := NewKiroAgent(cmdPath) + + result, err := a.Review( + context.Background(), t.TempDir(), + "deadbeef", "review", nil, + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(result, "plain review on stdout") { + t.Fatalf( + "expected stdout to be kept, got: %q", result, + ) + } + if strings.Contains(result, "warning") { + t.Error("stderr noise should not replace stdout content") + } +} + +func TestKiroReviewPromptTooLarge(t *testing.T) { + a := NewKiroAgent("kiro-cli") + bigPrompt := strings.Repeat("x", maxPromptArgLen+1) + _, err := a.Review(context.Background(), t.TempDir(), "HEAD", bigPrompt, nil) + if err == nil { + t.Fatal("expected error for oversized prompt") + } + if !strings.Contains(err.Error(), "too large") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestKiroWithChaining(t *testing.T) { + a := NewKiroAgent("kiro-cli") + b := a.WithReasoning(ReasoningThorough).WithAgentic(true) + kiro := b.(*KiroAgent) + + if kiro.Reasoning != ReasoningThorough { + t.Errorf("expected thorough reasoning, got %q", kiro.Reasoning) + } + if !kiro.Agentic { + t.Error("expected agentic true") + } + if kiro.Command != "kiro-cli" { + t.Errorf("expected command 'kiro-cli', got %q", kiro.Command) + } +} diff --git a/internal/ghaction/ghaction.go b/internal/ghaction/ghaction.go index b875291e..0f0d8cc6 100644 --- a/internal/ghaction/ghaction.go +++ b/internal/ghaction/ghaction.go @@ -17,7 +17,8 @@ import ( var ( allowedAgents = []string{ "codex", "claude-code", "gemini", - "copilot", "opencode", "cursor", "droid", + "copilot", "opencode", "cursor", + "kiro", "kilo", "droid", } safeVersionRE = regexp.MustCompile( `^[0-9]+\.[0-9]+\.[0-9]+(-[A-Za-z0-9.]+)?$`) @@ -91,6 +92,12 @@ func AgentEnvVar(agentName string) string { return "GOOGLE_API_KEY" case "copilot": return "GITHUB_TOKEN" + case "kiro": + // kiro-cli is not CI-compatible yet; use GITHUB_TOKEN + // so envEntries skips it (same as copilot). + return "GITHUB_TOKEN" + case "kilo": + return "ANTHROPIC_API_KEY" default: return "OPENAI_API_KEY" } @@ -113,6 +120,10 @@ func AgentInstallCmd(agentName string) string { case "cursor": return "echo 'Cursor agent is not available in CI" + " environments; choose a different agent'" + case "kiro": + return "echo 'Install kiro-cli: see https://kiro.dev/'" + case "kilo": + return "npm install -g @kilocode/cli@latest" case "droid": return "pip install droid-cli || echo 'Note: droid" + " agent may require additional setup; see" + @@ -255,8 +266,8 @@ var workflowTemplate = `# roborev CI Review # # Required setup: {{- range .EnvEntries }} -{{- if eq .Name "opencode" }} -# - Add a repository secret named "{{ .SecretName }}" (default for opencode). +{{- if or (eq .Name "opencode") (eq .Name "kilo") }} +# - Add a repository secret named "{{ .SecretName }}" (default for {{ .Name }}). # If you use a different model provider, replace with the appropriate key # (e.g., OPENAI_API_KEY, GOOGLE_API_KEY) and update the env block below. {{- else }} diff --git a/internal/ghaction/ghaction_test.go b/internal/ghaction/ghaction_test.go index 3947fe01..5d331ddb 100644 --- a/internal/ghaction/ghaction_test.go +++ b/internal/ghaction/ghaction_test.go @@ -34,6 +34,14 @@ func TestValidate(t *testing.T) { Agents: []string{"codex", "claude-code"}, }, }, + { + name: "valid kiro agent", + cfg: WorkflowConfig{Agents: []string{"kiro"}}, + }, + { + name: "valid kilo agent", + cfg: WorkflowConfig{Agents: []string{"kilo"}}, + }, { name: "invalid agent", cfg: WorkflowConfig{Agents: []string{"evil; rm -rf /"}}, @@ -215,6 +223,37 @@ func TestGenerate(t *testing.T) { } }, }, + { + name: "kilo gets multi-provider comment", + cfg: WorkflowConfig{ + Agents: []string{"kilo"}, + }, + wantStrs: []string{ + "@kilocode/cli@latest", + "ANTHROPIC_API_KEY", + "different model provider", + "default for kilo", + }, + envChecks: func(t *testing.T, env map[string]string) { + if _, ok := env["ANTHROPIC_API_KEY"]; !ok { + t.Error("expected ANTHROPIC_API_KEY in env") + } + }, + }, + { + name: "kiro skipped from env entries", + cfg: WorkflowConfig{ + Agents: []string{"kiro"}, + }, + wantStrs: []string{ + "kiro.dev", + }, + notWantStrs: []string{ + "OPENAI_API_KEY", + "ANTHROPIC_API_KEY", + "AWS_ACCESS_KEY_ID", + }, + }, { name: "dedupes env vars", cfg: WorkflowConfig{ @@ -323,6 +362,14 @@ func TestAgentInstallCmd(t *testing.T) { agent: "cursor", wantPkg: "not available in CI", }, + { + agent: "kiro", + wantPkg: "kiro.dev", + }, + { + agent: "kilo", + wantPkg: "@kilocode/cli@latest", + }, { agent: "droid", wantPkg: "droid-cli", @@ -470,6 +517,8 @@ func TestAgentEnvVar(t *testing.T) { {"gemini", "GOOGLE_API_KEY"}, {"copilot", "GITHUB_TOKEN"}, {"opencode", "ANTHROPIC_API_KEY"}, + {"kiro", "GITHUB_TOKEN"}, + {"kilo", "ANTHROPIC_API_KEY"}, {"droid", "OPENAI_API_KEY"}, } for _, tt := range tests {