From 24eee7bab85a657cc9f4e40d5441b6cc3f4f02c0 Mon Sep 17 00:00:00 2001 From: mikeaadd Date: Tue, 3 Mar 2026 11:24:30 -0600 Subject: [PATCH 01/12] feat: add Kiro agent (kiro-cli) Add KiroAgent that wraps kiro-cli for code reviews. Kiro output is captured, ANSI-stripped, and cleaned of UI chrome (logo, tip box, model line, timing footer) before returning the review text. - Add KiroAgent with chat --no-interactive invocation - Strip ANSI escapes and UI chrome via stripKiroOutput - Constrain start-marker search to first 30 lines to avoid false matches on markdown blockquotes in review content - Support --trust-all-tools in agentic mode - Register "kiro" in fallback order, error messages, CLI help - Add comprehensive tests (output stripping, args, agentic mode, chaining, success/failure/empty output) Co-Authored-By: Claude Sonnet 4.6 Co-Authored-By: Claude Opus 4.6 --- cmd/roborev/init_cmd.go | 2 +- cmd/roborev/main.go | 2 +- cmd/roborev/review.go | 2 +- internal/agent/agent.go | 6 +- internal/agent/agent_test_helpers.go | 2 +- internal/agent/kiro.go | 143 ++++++++++++++ internal/agent/kiro_test.go | 278 +++++++++++++++++++++++++++ 7 files changed, 428 insertions(+), 7 deletions(-) create mode 100644 internal/agent/kiro.go create mode 100644 internal/agent/kiro_test.go 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..0158b975 --- /dev/null +++ b/internal/agent/kiro.go @@ -0,0 +1,143 @@ +package agent + +import ( + "bytes" + "context" + "fmt" + "io" + "os" + "os/exec" + "regexp" + "strings" + "time" +) + +// ansiEscape matches ANSI/VT100 terminal escape sequences. +var ansiEscape = regexp.MustCompile(`\x1b(?:\[[0-9;?]*[A-Za-z]|[^\[])`) + +// stripKiroOutput removes Kiro's UI chrome (logo, tip box, model line, timing footer) +// and ANSI escape codes, returning only the review text. +func stripKiroOutput(raw string) string { + s := ansiEscape.ReplaceAllString(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 in review content 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) + } + + // Strip the "> " chat-prompt prefix from the first content line. + lines[start] = strings.TrimPrefix(lines[start], "> ") + + // Drop the timing footer ("▸ Time: Xs") and anything after it. + end := len(lines) + for i := start; i < len(lines); i++ { + if strings.HasPrefix(strings.TrimSpace(lines[i]), "▸ Time:") { + end = i + break + } + } + + return strings.TrimSpace(strings.Join(lines[start:end], "\n")) +} + +// 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) { + agenticMode := a.Agentic || AllowUnsafeAgents() + + // kiro-cli chat --no-interactive [--trust-all-tools] + // The prompt is passed as a positional argument (kiro-cli does not read from 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 + // through roborev's output writer. Capture stdout/stderr 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()) + } + + result := stripKiroOutput(stdout.String()) + 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..45838af4 --- /dev/null +++ b/internal/agent/kiro_test.go @@ -0,0 +1,278 @@ +package agent + +import ( + "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 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 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 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)) + } +} + +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 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) + } +} From 4263e49fe65a6359a64f541cf66373c8361753f7 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Tue, 3 Mar 2026 11:28:57 -0600 Subject: [PATCH 02/12] fix: address review findings for kiro agent - Add kiro to ghaction.go --agents help text - Add test for bare ">" start marker without trailing space - Add test verifying output writer receives review content Co-Authored-By: Claude Opus 4.6 --- cmd/roborev/ghaction.go | 2 +- internal/agent/kiro_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/cmd/roborev/ghaction.go b/cmd/roborev/ghaction.go index 92a3fa3c..f7843228 100644 --- a/cmd/roborev/ghaction.go +++ b/cmd/roborev/ghaction.go @@ -97,7 +97,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/internal/agent/kiro_test.go b/internal/agent/kiro_test.go index 45838af4..cb2231d1 100644 --- a/internal/agent/kiro_test.go +++ b/internal/agent/kiro_test.go @@ -1,6 +1,7 @@ package agent import ( + "bytes" "context" "os" "strings" @@ -56,6 +57,18 @@ func TestStripKiroOutputNoMarker(t *testing.T) { } } +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") + } +} + 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. @@ -151,6 +164,27 @@ func TestKiroReviewSuccess(t *testing.T) { } } +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) From 1e8772b05c65bef108a8b6705cd27eb898be93cc Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Tue, 3 Mar 2026 11:32:47 -0600 Subject: [PATCH 03/12] fix: harden kiro agent output handling and prompt size - Reuse stripTerminalControls (handles OSC sequences and control chars) instead of maintaining a weaker duplicate ANSI regex - Fall back to stderr when stdout is empty on successful exit, preventing silent "No review output generated" results - Reject prompts exceeding 512 KB with a clear error before exec, since kiro-cli requires argv (no stdin support) - Add tests for stderr fallback and oversized prompt rejection Co-Authored-By: Claude Opus 4.6 --- internal/agent/kiro.go | 36 +++++++++++++++++++++++++++--------- internal/agent/kiro_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/internal/agent/kiro.go b/internal/agent/kiro.go index 0158b975..9ee09db5 100644 --- a/internal/agent/kiro.go +++ b/internal/agent/kiro.go @@ -7,18 +7,19 @@ import ( "io" "os" "os/exec" - "regexp" "strings" "time" ) -// ansiEscape matches ANSI/VT100 terminal escape sequences. -var ansiEscape = regexp.MustCompile(`\x1b(?:\[[0-9;?]*[A-Za-z]|[^\[])`) +// 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 ANSI escape codes, returning only the review text. +// and terminal control sequences, returning only the review text. func stripKiroOutput(raw string) string { - s := ansiEscape.ReplaceAllString(raw, "") + 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 @@ -106,10 +107,18 @@ func (a *KiroAgent) CommandLine() string { } 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 read from stdin). + // The prompt is passed as a positional argument + // (kiro-cli does not support stdin). args := a.buildArgs(agenticMode) args = append(args, prompt) @@ -118,17 +127,26 @@ func (a *KiroAgent) Review(ctx context.Context, repoPath, commitSHA, prompt stri cmd.Env = os.Environ() cmd.WaitDelay = 5 * time.Second - // kiro-cli emits ANSI terminal escape codes that are not suitable for streaming - // through roborev's output writer. Capture stdout/stderr and return stripped text. + // 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()) + return "", fmt.Errorf( + "kiro failed: %w\nstderr: %s", + err, stderr.String(), + ) } result := stripKiroOutput(stdout.String()) + if len(result) == 0 { + // Some CLI tools emit review text on stderr. + result = strings.TrimSpace( + stripTerminalControls(stderr.String()), + ) + } if len(result) == 0 { return "No review output generated", nil } diff --git a/internal/agent/kiro_test.go b/internal/agent/kiro_test.go index cb2231d1..32da76ef 100644 --- a/internal/agent/kiro_test.go +++ b/internal/agent/kiro_test.go @@ -295,6 +295,37 @@ func TestKiroReviewAgenticModeFromGlobal(t *testing.T) { } } +func TestKiroReviewStderrFallback(t *testing.T) { + skipIfWindows(t) + + // kiro-cli exits 0 with review text on stderr, nothing on stdout. + script := NewScriptBuilder(). + AddRaw(`echo "review on 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 on stderr") { + t.Fatalf("expected stderr fallback, got: %q", result) + } +} + +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) From b7f7d4345f5ae3181853e377b8e2a4769c72b391 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Tue, 3 Mar 2026 11:39:58 -0600 Subject: [PATCH 04/12] fix: restrict footer scan, add -- separator, improve stderr fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Restrict timing footer detection to last 5 lines to avoid truncating review content that contains "▸ Time:" in a code snippet - Add "--" separator before prompt argument to prevent flag parsing - Use stripKiroOutput (not stripTerminalControls) for stderr fallback so Kiro chrome is fully stripped Co-Authored-By: Claude Opus 4.6 --- internal/agent/kiro.go | 23 +++++++++++++------- internal/agent/kiro_test.go | 42 +++++++++++++++++++++++++++++++++++-- 2 files changed, 56 insertions(+), 9 deletions(-) diff --git a/internal/agent/kiro.go b/internal/agent/kiro.go index 9ee09db5..8ab1b1bb 100644 --- a/internal/agent/kiro.go +++ b/internal/agent/kiro.go @@ -37,12 +37,23 @@ func stripKiroOutput(raw string) string { return strings.TrimSpace(s) } - // Strip the "> " chat-prompt prefix from the first content line. - lines[start] = strings.TrimPrefix(lines[start], "> ") + // 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 "" + } + } else { + lines[start] = strings.TrimPrefix(lines[start], "> ") + } // Drop the timing footer ("▸ Time: Xs") and anything after it. + // Only scan the last 5 lines to avoid truncating review content + // that happens to contain "▸ Time:" in a code snippet. end := len(lines) - for i := start; i < len(lines); i++ { + scanFrom := max(start, end-5) + for i := scanFrom; i < end; i++ { if strings.HasPrefix(strings.TrimSpace(lines[i]), "▸ Time:") { end = i break @@ -120,7 +131,7 @@ func (a *KiroAgent) Review(ctx context.Context, repoPath, commitSHA, prompt stri // The prompt is passed as a positional argument // (kiro-cli does not support stdin). args := a.buildArgs(agenticMode) - args = append(args, prompt) + args = append(args, "--", prompt) cmd := exec.CommandContext(ctx, a.Command, args...) cmd.Dir = repoPath @@ -143,9 +154,7 @@ func (a *KiroAgent) Review(ctx context.Context, repoPath, commitSHA, prompt stri result := stripKiroOutput(stdout.String()) if len(result) == 0 { // Some CLI tools emit review text on stderr. - result = strings.TrimSpace( - stripTerminalControls(stderr.String()), - ) + result = stripKiroOutput(stderr.String()) } if len(result) == 0 { return "No review output generated", nil diff --git a/internal/agent/kiro_test.go b/internal/agent/kiro_test.go index 32da76ef..68c789e1 100644 --- a/internal/agent/kiro_test.go +++ b/internal/agent/kiro_test.go @@ -67,6 +67,30 @@ func TestStripKiroOutputBareMarker(t *testing.T) { 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 TestStripKiroOutputBlockquoteNotStripped(t *testing.T) { @@ -245,6 +269,10 @@ func TestKiroPassesPromptAsArg(t *testing.T) { 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) { @@ -298,9 +326,13 @@ func TestKiroReviewAgenticModeFromGlobal(t *testing.T) { func TestKiroReviewStderrFallback(t *testing.T) { skipIfWindows(t) - // kiro-cli exits 0 with review text on stderr, nothing on stdout. + // 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 "review on stderr" >&2`). + AddRaw(`echo "Model: auto" >&2`). + AddRaw(`echo "> review on stderr" >&2`). + AddRaw(`echo " ▸ Time: 5s" >&2`). Build() cmdPath := writeTempCommand(t, script) a := NewKiroAgent(cmdPath) @@ -312,6 +344,12 @@ func TestKiroReviewStderrFallback(t *testing.T) { 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 TestKiroReviewPromptTooLarge(t *testing.T) { From 6a63fe01c13edc7845755cb392e5019a36425717 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Tue, 3 Mar 2026 11:44:15 -0600 Subject: [PATCH 05/12] fix: trim trailing blanks before footer scan, add -- to CommandLine - Strip trailing blank lines before applying the 5-line footer scan window so blank postamble from kiro-cli doesn't hide the real footer - Update CommandLine() to include -- separator, matching Review() behavior Co-Authored-By: Claude Opus 4.6 --- internal/agent/kiro.go | 11 ++++++++--- internal/agent/kiro_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/internal/agent/kiro.go b/internal/agent/kiro.go index 8ab1b1bb..5f6d8ea7 100644 --- a/internal/agent/kiro.go +++ b/internal/agent/kiro.go @@ -49,9 +49,14 @@ func stripKiroOutput(raw string) string { } // Drop the timing footer ("▸ Time: Xs") and anything after it. - // Only scan the last 5 lines to avoid truncating review content - // that happens to contain "▸ Time:" in a code snippet. + // 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:") { @@ -114,7 +119,7 @@ func (a *KiroAgent) buildArgs(agenticMode bool) []string { func (a *KiroAgent) CommandLine() string { agenticMode := a.Agentic || AllowUnsafeAgents() args := a.buildArgs(agenticMode) - return a.Command + " " + strings.Join(args, " ") + " " + return a.Command + " " + strings.Join(args, " ") + " -- " } func (a *KiroAgent) Review(ctx context.Context, repoPath, commitSHA, prompt string, output io.Writer) (string, error) { diff --git a/internal/agent/kiro_test.go b/internal/agent/kiro_test.go index 68c789e1..68cd1cfa 100644 --- a/internal/agent/kiro_test.go +++ b/internal/agent/kiro_test.go @@ -93,6 +93,18 @@ func TestStripKiroOutputFooterInContent(t *testing.T) { } } +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. @@ -136,6 +148,21 @@ func TestKiroName(t *testing.T) { } } +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 { From b7fd9e506b69decbbc297b02ae0c773426630e1b Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Tue, 3 Mar 2026 12:08:41 -0600 Subject: [PATCH 06/12] fix: add kiro and kilo to ghaction allowlist and install mappings The CLI help text advertised kiro/kilo as valid agents but the ghaction validation allowlist rejected them at runtime. - Add both to allowedAgents - kiro: not CI-compatible yet, install step directs to kiro.dev - kilo: install via @kilocode/cli, uses OPENAI_API_KEY (default) Co-Authored-By: Claude Opus 4.6 --- internal/ghaction/ghaction.go | 9 ++++++++- internal/ghaction/ghaction_test.go | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/internal/ghaction/ghaction.go b/internal/ghaction/ghaction.go index b875291e..9a3a72da 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,8 @@ func AgentEnvVar(agentName string) string { return "GOOGLE_API_KEY" case "copilot": return "GITHUB_TOKEN" + case "kilo": + return "OPENAI_API_KEY" default: return "OPENAI_API_KEY" } @@ -113,6 +116,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" + diff --git a/internal/ghaction/ghaction_test.go b/internal/ghaction/ghaction_test.go index 3947fe01..6af57a68 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 /"}}, @@ -323,6 +331,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 +486,8 @@ func TestAgentEnvVar(t *testing.T) { {"gemini", "GOOGLE_API_KEY"}, {"copilot", "GITHUB_TOKEN"}, {"opencode", "ANTHROPIC_API_KEY"}, + {"kiro", "OPENAI_API_KEY"}, + {"kilo", "OPENAI_API_KEY"}, {"droid", "OPENAI_API_KEY"}, } for _, tt := range tests { From ef9922d9ae36bdec186ea437d8c8b3bc1540b07e Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Tue, 3 Mar 2026 12:10:48 -0600 Subject: [PATCH 07/12] fix: correct kiro/kilo env vars and workflow template - kiro: map to GITHUB_TOKEN so envEntries skips it (not CI-compatible) - kilo: map to ANTHROPIC_API_KEY with multi-provider comment (like opencode) - Extend workflow template and CLI next-steps to handle kilo as multi-provider Co-Authored-By: Claude Opus 4.6 --- cmd/roborev/ghaction.go | 7 ++++--- internal/ghaction/ghaction.go | 10 +++++++--- internal/ghaction/ghaction_test.go | 4 ++-- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/cmd/roborev/ghaction.go b/cmd/roborev/ghaction.go index f7843228..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 "+ diff --git a/internal/ghaction/ghaction.go b/internal/ghaction/ghaction.go index 9a3a72da..0f0d8cc6 100644 --- a/internal/ghaction/ghaction.go +++ b/internal/ghaction/ghaction.go @@ -92,8 +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 "OPENAI_API_KEY" + return "ANTHROPIC_API_KEY" default: return "OPENAI_API_KEY" } @@ -262,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 6af57a68..1b1a2a22 100644 --- a/internal/ghaction/ghaction_test.go +++ b/internal/ghaction/ghaction_test.go @@ -486,8 +486,8 @@ func TestAgentEnvVar(t *testing.T) { {"gemini", "GOOGLE_API_KEY"}, {"copilot", "GITHUB_TOKEN"}, {"opencode", "ANTHROPIC_API_KEY"}, - {"kiro", "OPENAI_API_KEY"}, - {"kilo", "OPENAI_API_KEY"}, + {"kiro", "GITHUB_TOKEN"}, + {"kilo", "ANTHROPIC_API_KEY"}, {"droid", "OPENAI_API_KEY"}, } for _, tt := range tests { From fcb43b92ea6a3c2b0997d200bad48b9249bbaaea Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Tue, 3 Mar 2026 12:15:27 -0600 Subject: [PATCH 08/12] test: add kiro/kilo coverage for workflow template and CLI output - Verify kilo workflow gets multi-provider comment and ANTHROPIC_API_KEY - Verify kiro workflow has no agent-specific secret env vars - Add notContains support to CLI ghaction test table Co-Authored-By: Claude Opus 4.6 --- cmd/roborev/ghaction_test.go | 30 ++++++++++++++++++++++++++++- internal/ghaction/ghaction_test.go | 31 ++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) 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/internal/ghaction/ghaction_test.go b/internal/ghaction/ghaction_test.go index 1b1a2a22..5d331ddb 100644 --- a/internal/ghaction/ghaction_test.go +++ b/internal/ghaction/ghaction_test.go @@ -223,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{ From 6a694d532bd5f651a430ec02afeccb8d803278ea Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Tue, 3 Mar 2026 12:32:25 -0600 Subject: [PATCH 09/12] fix: prefer stderr review over stdout noise in kiro fallback Extract stripKiroReview returning (text, hasMarker) so the Review method can detect when stdout contains only non-review noise (no "> " marker) and prefer stderr's marker-detected content. Previously, stdout noise blocked the stderr fallback because stripKiroOutput returned the noise as non-empty text. Co-Authored-By: Claude Opus 4.6 --- internal/agent/kiro.go | 31 ++++++++++++++++++++++--------- internal/agent/kiro_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/internal/agent/kiro.go b/internal/agent/kiro.go index 5f6d8ea7..ae91cac1 100644 --- a/internal/agent/kiro.go +++ b/internal/agent/kiro.go @@ -19,11 +19,20 @@ 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 in review content for the start marker. + // 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 @@ -34,7 +43,7 @@ func stripKiroOutput(raw string) string { } } if start == -1 { - return strings.TrimSpace(s) + return strings.TrimSpace(s), false } // Strip the prompt marker from the first content line. @@ -42,7 +51,7 @@ func stripKiroOutput(raw string) string { if lines[start] == ">" { start++ if start >= len(lines) { - return "" + return "", true } } else { lines[start] = strings.TrimPrefix(lines[start], "> ") @@ -65,7 +74,7 @@ func stripKiroOutput(raw string) string { } } - return strings.TrimSpace(strings.Join(lines[start:end], "\n")) + return strings.TrimSpace(strings.Join(lines[start:end], "\n")), true } // KiroAgent runs code reviews using the Kiro CLI (kiro-cli) @@ -156,10 +165,14 @@ func (a *KiroAgent) Review(ctx context.Context, repoPath, commitSHA, prompt stri ) } - result := stripKiroOutput(stdout.String()) - if len(result) == 0 { - // Some CLI tools emit review text on stderr. - result = stripKiroOutput(stderr.String()) + // Prefer the stream that contains a "> " review marker. + // Stdout noise without a marker should not block the + // stderr fallback. + result, hasMarker := stripKiroReview(stdout.String()) + if !hasMarker || len(result) == 0 { + if alt, ok := stripKiroReview(stderr.String()); ok && len(alt) > 0 { + result = alt + } } if len(result) == 0 { return "No review output generated", nil diff --git a/internal/agent/kiro_test.go b/internal/agent/kiro_test.go index 68cd1cfa..634a4766 100644 --- a/internal/agent/kiro_test.go +++ b/internal/agent/kiro_test.go @@ -379,6 +379,38 @@ func TestKiroReviewStderrFallback(t *testing.T) { } } +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 TestKiroReviewPromptTooLarge(t *testing.T) { a := NewKiroAgent("kiro-cli") bigPrompt := strings.Repeat("x", maxPromptArgLen+1) From cfbcc64c43f421e842f746d15f5c4e4fc93543f7 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Tue, 3 Mar 2026 12:35:12 -0600 Subject: [PATCH 10/12] fix: accept non-marker stderr in kiro fallback When stdout has no review marker, fall back to stderr unconditionally (not only when stderr also has a marker). This handles the case where stderr contains plain review text without Kiro chrome. Co-Authored-By: Claude Opus 4.6 --- internal/agent/kiro.go | 6 +++--- internal/agent/kiro_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/internal/agent/kiro.go b/internal/agent/kiro.go index ae91cac1..4a6e3749 100644 --- a/internal/agent/kiro.go +++ b/internal/agent/kiro.go @@ -166,11 +166,11 @@ func (a *KiroAgent) Review(ctx context.Context, repoPath, commitSHA, prompt stri } // Prefer the stream that contains a "> " review marker. - // Stdout noise without a marker should not block the - // stderr fallback. + // When stdout has no marker (noise-only) or is empty, + // fall back to stderr unconditionally. result, hasMarker := stripKiroReview(stdout.String()) if !hasMarker || len(result) == 0 { - if alt, ok := stripKiroReview(stderr.String()); ok && len(alt) > 0 { + if alt, _ := stripKiroReview(stderr.String()); len(alt) > 0 { result = alt } } diff --git a/internal/agent/kiro_test.go b/internal/agent/kiro_test.go index 634a4766..f190d77d 100644 --- a/internal/agent/kiro_test.go +++ b/internal/agent/kiro_test.go @@ -411,6 +411,34 @@ func TestKiroReviewStderrPreferredOverStdoutNoise(t *testing.T) { } } +func TestKiroReviewStderrFallbackNoMarker(t *testing.T) { + skipIfWindows(t) + + // stdout has noise without a marker; stderr has plain + // review text also without a marker. stderr should still + // be preferred over stdout noise. + script := NewScriptBuilder(). + AddRaw(`echo "Loading model..."`). + 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 TestKiroReviewPromptTooLarge(t *testing.T) { a := NewKiroAgent("kiro-cli") bigPrompt := strings.Repeat("x", maxPromptArgLen+1) From 7c41d428688de8b430d38df5cd6845f3fed63a0d Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Tue, 3 Mar 2026 12:38:35 -0600 Subject: [PATCH 11/12] fix: keep stdout over stderr noise when neither has a marker Refine the fallback precedence so stderr only overrides stdout when stdout is empty or stderr has a review marker. When both streams have content but neither has a marker, stdout (primary stream) is kept. Co-Authored-By: Claude Opus 4.6 --- internal/agent/kiro.go | 13 ++++++++----- internal/agent/kiro_test.go | 35 +++++++++++++++++++++++++++++++---- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/internal/agent/kiro.go b/internal/agent/kiro.go index 4a6e3749..3cf9c3ee 100644 --- a/internal/agent/kiro.go +++ b/internal/agent/kiro.go @@ -166,11 +166,14 @@ func (a *KiroAgent) Review(ctx context.Context, repoPath, commitSHA, prompt stri } // Prefer the stream that contains a "> " review marker. - // When stdout has no marker (noise-only) or is empty, - // fall back to stderr unconditionally. - result, hasMarker := stripKiroReview(stdout.String()) - if !hasMarker || len(result) == 0 { - if alt, _ := stripKiroReview(stderr.String()); len(alt) > 0 { + // - stdout with marker → use stdout + // - stdout empty → try stderr (marker or not) + // - stdout has content but no marker → use stderr only + // if stderr has a marker (otherwise keep stdout) + result, stdoutMarker := stripKiroReview(stdout.String()) + if !stdoutMarker { + alt, stderrMarker := stripKiroReview(stderr.String()) + if len(alt) > 0 && (len(result) == 0 || stderrMarker) { result = alt } } diff --git a/internal/agent/kiro_test.go b/internal/agent/kiro_test.go index f190d77d..ee18a511 100644 --- a/internal/agent/kiro_test.go +++ b/internal/agent/kiro_test.go @@ -414,11 +414,9 @@ func TestKiroReviewStderrPreferredOverStdoutNoise(t *testing.T) { func TestKiroReviewStderrFallbackNoMarker(t *testing.T) { skipIfWindows(t) - // stdout has noise without a marker; stderr has plain - // review text also without a marker. stderr should still - // be preferred over stdout noise. + // stdout is empty; stderr has plain review text without + // a marker. stderr should be used as fallback. script := NewScriptBuilder(). - AddRaw(`echo "Loading model..."`). AddRaw(`echo "review text without marker" >&2`). Build() cmdPath := writeTempCommand(t, script) @@ -439,6 +437,35 @@ func TestKiroReviewStderrFallbackNoMarker(t *testing.T) { } } +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) From fbf6de17d963b2003c9612ebc70302ffa9619cb1 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Tue, 3 Mar 2026 12:47:48 -0600 Subject: [PATCH 12/12] fix: fall back to stderr when stdout has marker but empty body When stdout contains only a bare marker (e.g., ">") with no review content, check stderr for the actual review instead of returning "No review output generated". Co-Authored-By: Claude Opus 4.6 --- internal/agent/kiro.go | 6 +++--- internal/agent/kiro_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/internal/agent/kiro.go b/internal/agent/kiro.go index 3cf9c3ee..b85c9ec8 100644 --- a/internal/agent/kiro.go +++ b/internal/agent/kiro.go @@ -166,12 +166,12 @@ func (a *KiroAgent) Review(ctx context.Context, repoPath, commitSHA, prompt stri } // Prefer the stream that contains a "> " review marker. - // - stdout with marker → use stdout - // - stdout empty → try stderr (marker or not) + // - 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 { + if !stdoutMarker || len(result) == 0 { alt, stderrMarker := stripKiroReview(stderr.String()) if len(alt) > 0 && (len(result) == 0 || stderrMarker) { result = alt diff --git a/internal/agent/kiro_test.go b/internal/agent/kiro_test.go index ee18a511..06ff8b8c 100644 --- a/internal/agent/kiro_test.go +++ b/internal/agent/kiro_test.go @@ -379,6 +379,33 @@ func TestKiroReviewStderrFallback(t *testing.T) { } } +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)