mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-31 00:40:46 +02:00
fix(kiro): require positive proof for -32603 completion rescue (MUL-4860, #5509)
Addresses the #5511 review (Elon): Must-fix 1 — a missing result is not proof of success. A mid-command crash / internal session failure produces the same 'tool use, no result, -32603' shape as a genuine completion, so the previous 'saw the use but never saw a result' rescue could mark an unfinished task completed. The guard now rescues only on POSITIVE proof: a terminal ToolResult with status=='completed' for a finishing tool (goal_complete / comment-add). Must-fix 2 — the previous sticky booleans only ever wrote true, so a later failed delivery could not override an earlier success, and call ordering was ignored. Completion state is now the status of the MOST RECENT finishing-tool result (keyed per CallID): 'progress completed → final failed → -32603' stays failed; 'first failed → retry completed → -32603' is a real completion. Comment-add is still recognized by its command payload regardless of the tool's display title (the #5509 core ask), so a completed comment-add carrying a non-terminal title is preserved through the close handshake. Tests: result-less running tool stays failed (both goal_complete and comment-add), completed non-terminal-title comment-add is preserved, failed-final-overrides-earlier-success, and completed-retry-after-failure. Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
Eve
@@ -108,28 +108,28 @@ func (b *kiroBackend) Execute(ctx context.Context, prompt string, opts ExecOptio
|
||||
var output strings.Builder
|
||||
var streamingCurrentTurn atomic.Bool
|
||||
// Completion-preservation state for the -32603 close-handshake guard.
|
||||
// We track each finish-signalling tool (goal_complete, `multica issue
|
||||
// comment add`) along three axes so the guard can tell three cases
|
||||
// apart:
|
||||
// - saw*Use: we observed the tool being invoked this turn.
|
||||
// - saw*Result: we observed a terminal ToolResult for it (any status).
|
||||
// - sawCompleted*: that terminal ToolResult was status=="completed".
|
||||
// The Claude/Kiro path always emits a completed ToolResult, so
|
||||
// sawCompleted* is enough there. The GPT-5.6 Sol adapter, however, can
|
||||
// leave the terminal tool parked at "running" and never emit a
|
||||
// completed/failed ToolCallUpdate — so no ToolResult is ever produced
|
||||
// (hermes.handleToolCallUpdate only dispatches on completed/failed).
|
||||
// For that path we fall back to "saw the use but never saw a result"
|
||||
// (see the guard below), which stays safe because a genuinely failed
|
||||
// tool still emits a failed ToolResult and trips saw*Result.
|
||||
var sawCompletedGoalComplete atomic.Bool
|
||||
var sawCompletedIssueComment atomic.Bool
|
||||
var sawGoalCompleteUse atomic.Bool
|
||||
var sawGoalCompleteResult atomic.Bool
|
||||
var sawIssueCommentUse atomic.Bool
|
||||
var sawIssueCommentResult atomic.Bool
|
||||
//
|
||||
// Kiro raises `session/prompt -32603 "failed to generate a response"`
|
||||
// when the model fails to produce its closing turn AFTER the task has
|
||||
// already done its work. We keep such a run "completed" only when we
|
||||
// have POSITIVE proof the finishing step succeeded — never from the mere
|
||||
// absence of a result. A mid-command crash or internal session failure
|
||||
// produces the very same "tool use, no result, -32603" shape, and
|
||||
// treating that as success would mark a genuinely-unfinished task
|
||||
// completed (see the review on #5511 / MUL-4860).
|
||||
//
|
||||
// The only positive proof available at this layer is a terminal
|
||||
// ToolResult with status=="completed" for a finishing tool
|
||||
// (goal_complete or `multica issue comment add`). We record the status
|
||||
// of the MOST RECENT such result (ordered, keyed per CallID) so a later
|
||||
// failed delivery correctly overrides an earlier success — e.g.
|
||||
// "progress comment completed → final comment failed → -32603" must stay
|
||||
// failed, while "first attempt failed → retry completed → -32603" is a
|
||||
// real completion.
|
||||
var goalCompleteCallIDs sync.Map
|
||||
var issueCommentCallIDs sync.Map
|
||||
var finishingMu sync.Mutex
|
||||
var lastFinishingResultStatus string // "", "completed", or "failed"
|
||||
|
||||
promptDone := make(chan hermesPromptResult, 1)
|
||||
|
||||
@@ -149,33 +149,29 @@ func (b *kiroBackend) Execute(ctx context.Context, prompt string, opts ExecOptio
|
||||
msg.Tool = kiroToolNameFromTitle(msg.Tool)
|
||||
if msg.Tool == "goal_complete" && msg.CallID != "" {
|
||||
goalCompleteCallIDs.Store(msg.CallID, struct{}{})
|
||||
sawGoalCompleteUse.Store(true)
|
||||
}
|
||||
// Recognize `multica issue comment add` by its command
|
||||
// payload regardless of how the adapter titled the tool.
|
||||
// GPT-5.6 Sol may label the shell tool something other than
|
||||
// the aliases kiroToolNameFromTitle folds into "terminal"
|
||||
// (e.g. "execute"/"run"), so gating on msg.Tool=="terminal"
|
||||
// here would miss it entirely.
|
||||
if isKiroIssueCommentAddTool(msg) {
|
||||
sawIssueCommentUse.Store(true)
|
||||
if msg.CallID != "" {
|
||||
issueCommentCallIDs.Store(msg.CallID, struct{}{})
|
||||
}
|
||||
// here would miss it entirely (issue #5509). We still require
|
||||
// a CallID so the tool use can be correlated to its result.
|
||||
if msg.CallID != "" && isKiroIssueCommentAddTool(msg) {
|
||||
issueCommentCallIDs.Store(msg.CallID, struct{}{})
|
||||
}
|
||||
}
|
||||
if msg.Type == MessageToolResult {
|
||||
if _, ok := goalCompleteCallIDs.LoadAndDelete(msg.CallID); ok {
|
||||
sawGoalCompleteResult.Store(true)
|
||||
if msg.Status == "completed" {
|
||||
sawCompletedGoalComplete.Store(true)
|
||||
}
|
||||
}
|
||||
if _, ok := issueCommentCallIDs.LoadAndDelete(msg.CallID); ok {
|
||||
sawIssueCommentResult.Store(true)
|
||||
if msg.Status == "completed" {
|
||||
sawCompletedIssueComment.Store(true)
|
||||
}
|
||||
_, isGoal := goalCompleteCallIDs.LoadAndDelete(msg.CallID)
|
||||
_, isComment := issueCommentCallIDs.LoadAndDelete(msg.CallID)
|
||||
if isGoal || isComment {
|
||||
// Record this finishing tool's terminal outcome. The
|
||||
// most recent result wins, so the guard sees the final
|
||||
// delivery attempt's status and a later failure
|
||||
// overrides an earlier success.
|
||||
finishingMu.Lock()
|
||||
lastFinishingResultStatus = msg.Status
|
||||
finishingMu.Unlock()
|
||||
}
|
||||
}
|
||||
if msg.Type == MessageText {
|
||||
@@ -368,22 +364,21 @@ func (b *kiroBackend) Execute(ctx context.Context, prompt string, opts ExecOptio
|
||||
} else {
|
||||
finalStatus = "failed"
|
||||
finalError = fmt.Sprintf("kiro session/prompt failed: %v", err)
|
||||
// Completion-preservation guard for the -32603 close
|
||||
// handshake Kiro raises after a task has finished its work.
|
||||
//
|
||||
// completedResult — we saw a completed ToolResult for
|
||||
// goal_complete / comment-add (Claude path).
|
||||
// useWithoutResult — we saw the finishing tool invoked but
|
||||
// never saw ANY terminal ToolResult for it
|
||||
// (GPT-5.6 Sol path: the tool stays parked
|
||||
// at "running"). We deliberately exclude
|
||||
// the case where a failed ToolResult DID
|
||||
// arrive — that is a real failure and must
|
||||
// stay failed.
|
||||
completedResult := sawCompletedGoalComplete.Load() || sawCompletedIssueComment.Load()
|
||||
useWithoutResult := (sawGoalCompleteUse.Load() && !sawGoalCompleteResult.Load()) ||
|
||||
(sawIssueCommentUse.Load() && !sawIssueCommentResult.Load())
|
||||
if (completedResult || useWithoutResult) && isKiroGoalCompleteCloseError(err) {
|
||||
// Preserve completion only on POSITIVE proof: the most
|
||||
// recent finishing-tool ToolResult we observed was
|
||||
// status=="completed", paired with the specific -32603 close
|
||||
// handshake. A missing result is NOT proof — a mid-command
|
||||
// crash produces the same result-less shape (Must-fix #1) —
|
||||
// and because we key on the most recent outcome, a later
|
||||
// failed delivery overrides an earlier success (Must-fix #2).
|
||||
finishingMu.Lock()
|
||||
lastFinishing := lastFinishingResultStatus
|
||||
finishingMu.Unlock()
|
||||
if lastFinishing == "completed" && isKiroGoalCompleteCloseError(err) {
|
||||
b.cfg.Logger.Warn("kiro session/prompt failed after a completed finishing-tool result; preserving completed task status", "error", err)
|
||||
finalStatus = "completed"
|
||||
finalError = ""
|
||||
} else if opts.ResumeSessionID != "" && isACPSessionNotFound(err) {
|
||||
b.cfg.Logger.Warn("kiro session/prompt failed after completed task result; preserving completed task status", "error", err)
|
||||
finalStatus = "completed"
|
||||
finalError = ""
|
||||
|
||||
@@ -449,13 +449,52 @@ func TestKiroIssueCommentAddToolIgnoresToolTitle(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// fakeKiroACPRunningToolCloseErrorScript reproduces the GPT-5.6 Sol failure
|
||||
// mode from #5509: the finishing tool is invoked (ToolCall notification) but
|
||||
// the adapter never emits a completed/failed ToolCallUpdate — the tool stays
|
||||
// parked at "running", so no ToolResult is ever produced. Kiro then raises the
|
||||
// -32603 "failed to generate a response" close handshake. `toolName` lets the
|
||||
// caller pick a title that does NOT normalize to "terminal", and `command`
|
||||
// selects goal_complete vs comment-add.
|
||||
// runKiroCloseErrorScript executes a fake-kiro script that ends in the -32603
|
||||
// close handshake and returns the final Result.
|
||||
func runKiroCloseErrorScript(t *testing.T, script string) Result {
|
||||
t.Helper()
|
||||
fakePath := filepath.Join(t.TempDir(), "kiro-cli")
|
||||
writeTestExecutable(t, fakePath, []byte(script))
|
||||
|
||||
backend, err := New("kiro", Config{ExecutablePath: fakePath, Logger: slog.Default()})
|
||||
if err != nil {
|
||||
t.Fatalf("new kiro backend: %v", err)
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
||||
defer cancel()
|
||||
|
||||
session, err := backend.Execute(ctx, "prompt-ignored", ExecOptions{Timeout: 5 * time.Second})
|
||||
if err != nil {
|
||||
t.Fatalf("execute: %v", err)
|
||||
}
|
||||
go func() {
|
||||
for range session.Messages {
|
||||
}
|
||||
}()
|
||||
|
||||
select {
|
||||
case result, ok := <-session.Result:
|
||||
if !ok {
|
||||
t.Fatal("result channel closed without a value")
|
||||
}
|
||||
return result
|
||||
case <-time.After(10 * time.Second):
|
||||
t.Fatal("timeout waiting for result")
|
||||
return Result{}
|
||||
}
|
||||
}
|
||||
|
||||
// closeErrorFrame is the -32603 "failed to generate a response" close
|
||||
// handshake Kiro raises after the task has already finished its work.
|
||||
const closeErrorFrame = `printf '{"jsonrpc":"2.0","id":%s,"error":{"code":-32603,"message":"Internal error","data":"Kiro failed to generate a response"}}\n' "$id"`
|
||||
|
||||
// fakeKiroACPRunningToolCloseErrorScript reproduces the GPT-5.6 Sol shape from
|
||||
// #5509: the finishing tool is invoked (ToolCall notification) but the adapter
|
||||
// never emits a completed/failed ToolCallUpdate — the tool stays parked at
|
||||
// "running", so no ToolResult is ever produced. Kiro then raises the -32603
|
||||
// close handshake. `toolName` picks a title that does NOT normalize to
|
||||
// "terminal"; `command` selects goal_complete (empty) vs comment-add.
|
||||
func fakeKiroACPRunningToolCloseErrorScript(toolName, command string) string {
|
||||
params := `{}`
|
||||
if command != "" {
|
||||
@@ -469,11 +508,11 @@ while IFS= read -r line; do
|
||||
printf '{"jsonrpc":"2.0","id":%s,"result":{"protocolVersion":1,"agentCapabilities":{"loadSession":true}}}\n' "$id"
|
||||
;;
|
||||
*'"method":"session/new"'*)
|
||||
printf '{"jsonrpc":"2.0","id":%s,"result":{"sessionId":"ses_running_done"}}\n' "$id"
|
||||
printf '{"jsonrpc":"2.0","id":%s,"result":{"sessionId":"ses_running"}}\n' "$id"
|
||||
;;
|
||||
*'"method":"session/prompt"'*)
|
||||
printf '{"jsonrpc":"2.0","method":"session/notification","params":{"sessionId":"ses_running_done","update":{"type":"ToolCall","toolCallId":"tc-running","name":"` + toolName + `","status":"pending","parameters":` + params + `}}}\n'
|
||||
printf '{"jsonrpc":"2.0","id":%s,"error":{"code":-32603,"message":"Internal error","data":"Kiro failed to generate a response"}}\n' "$id"
|
||||
printf '{"jsonrpc":"2.0","method":"session/notification","params":{"sessionId":"ses_running","update":{"type":"ToolCall","toolCallId":"tc-running","name":"` + toolName + `","status":"pending","parameters":` + params + `}}}\n'
|
||||
` + closeErrorFrame + `
|
||||
exit 0
|
||||
;;
|
||||
esac
|
||||
@@ -481,97 +520,45 @@ done
|
||||
`
|
||||
}
|
||||
|
||||
// TestKiroBackendTreatsRunningCommentAddCloseErrorAsCompleted is the primary
|
||||
// #5509 regression: a `multica issue comment add` shell tool titled with a
|
||||
// non-terminal name, left at "running" (no completed ToolResult), followed by
|
||||
// the -32603 close handshake, must be preserved as completed.
|
||||
func TestKiroBackendTreatsRunningCommentAddCloseErrorAsCompleted(t *testing.T) {
|
||||
// TestKiroBackendDoesNotCompleteRunningCommentAddWithoutResult is the core
|
||||
// safety property from the #5511 review (Must-fix #1): a finishing tool that
|
||||
// was only ever seen invoked — no terminal ToolResult — is NOT proof the work
|
||||
// succeeded (a mid-command crash produces the same shape). It must stay failed.
|
||||
func TestKiroBackendDoesNotCompleteRunningCommentAddWithoutResult(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
fakePath := filepath.Join(t.TempDir(), "kiro-cli")
|
||||
writeTestExecutable(t, fakePath, []byte(fakeKiroACPRunningToolCloseErrorScript(
|
||||
result := runKiroCloseErrorScript(t, fakeKiroACPRunningToolCloseErrorScript(
|
||||
"execute_bash",
|
||||
"multica issue comment add issue-1 --content-file ./reply.md",
|
||||
)))
|
||||
|
||||
backend, err := New("kiro", Config{ExecutablePath: fakePath, Logger: slog.Default()})
|
||||
if err != nil {
|
||||
t.Fatalf("new kiro backend: %v", err)
|
||||
))
|
||||
if result.Status != "failed" {
|
||||
t.Fatalf("expected status=failed for a result-less running comment-add, got %q (error=%q)", result.Status, result.Error)
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
||||
defer cancel()
|
||||
|
||||
session, err := backend.Execute(ctx, "prompt-ignored", ExecOptions{Timeout: 5 * time.Second})
|
||||
if err != nil {
|
||||
t.Fatalf("execute: %v", err)
|
||||
}
|
||||
go func() {
|
||||
for range session.Messages {
|
||||
}
|
||||
}()
|
||||
|
||||
select {
|
||||
case result, ok := <-session.Result:
|
||||
if !ok {
|
||||
t.Fatal("result channel closed without a value")
|
||||
}
|
||||
if result.Status != "completed" {
|
||||
t.Fatalf("expected status=completed for a running comment-add + close error, got %q (error=%q)", result.Status, result.Error)
|
||||
}
|
||||
if result.Error != "" {
|
||||
t.Fatalf("expected close-handshake error to be suppressed, got %q", result.Error)
|
||||
}
|
||||
case <-time.After(10 * time.Second):
|
||||
t.Fatal("timeout waiting for result")
|
||||
if !strings.Contains(result.Error, "Kiro failed to generate a response") {
|
||||
t.Fatalf("expected original prompt error to be preserved, got %q", result.Error)
|
||||
}
|
||||
}
|
||||
|
||||
// TestKiroBackendTreatsRunningGoalCompleteCloseErrorAsCompleted covers the
|
||||
// goal_complete sibling of the running-tool path.
|
||||
func TestKiroBackendTreatsRunningGoalCompleteCloseErrorAsCompleted(t *testing.T) {
|
||||
// TestKiroBackendDoesNotCompleteRunningGoalCompleteWithoutResult is the
|
||||
// goal_complete sibling of the result-less safety property.
|
||||
func TestKiroBackendDoesNotCompleteRunningGoalCompleteWithoutResult(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
fakePath := filepath.Join(t.TempDir(), "kiro-cli")
|
||||
writeTestExecutable(t, fakePath, []byte(fakeKiroACPRunningToolCloseErrorScript("goal_complete", "")))
|
||||
|
||||
backend, err := New("kiro", Config{ExecutablePath: fakePath, Logger: slog.Default()})
|
||||
if err != nil {
|
||||
t.Fatalf("new kiro backend: %v", err)
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
||||
defer cancel()
|
||||
|
||||
session, err := backend.Execute(ctx, "prompt-ignored", ExecOptions{Timeout: 5 * time.Second})
|
||||
if err != nil {
|
||||
t.Fatalf("execute: %v", err)
|
||||
}
|
||||
go func() {
|
||||
for range session.Messages {
|
||||
}
|
||||
}()
|
||||
|
||||
select {
|
||||
case result, ok := <-session.Result:
|
||||
if !ok {
|
||||
t.Fatal("result channel closed without a value")
|
||||
}
|
||||
if result.Status != "completed" {
|
||||
t.Fatalf("expected status=completed for a running goal_complete + close error, got %q (error=%q)", result.Status, result.Error)
|
||||
}
|
||||
if result.Error != "" {
|
||||
t.Fatalf("expected close-handshake error to be suppressed, got %q", result.Error)
|
||||
}
|
||||
case <-time.After(10 * time.Second):
|
||||
t.Fatal("timeout waiting for result")
|
||||
result := runKiroCloseErrorScript(t, fakeKiroACPRunningToolCloseErrorScript("goal_complete", ""))
|
||||
if result.Status != "failed" {
|
||||
t.Fatalf("expected status=failed for a result-less running goal_complete, got %q (error=%q)", result.Status, result.Error)
|
||||
}
|
||||
}
|
||||
|
||||
// fakeKiroACPFailedRunningToolCloseErrorScript is the safety counterpart: the
|
||||
// finishing tool DID emit a failed ToolResult before the close error. That is
|
||||
// a genuine failure and must stay failed even though we saw the tool use.
|
||||
func fakeKiroACPFailedRunningToolCloseErrorScript() string {
|
||||
// fakeKiroACPCompletedToolCloseErrorScript is the positive-proof path: the
|
||||
// finishing tool emits a completed ToolCallUpdate (the real completion signal)
|
||||
// before the -32603 close handshake. `toolName` deliberately uses a title that
|
||||
// does NOT normalize to "terminal" to prove recognition is title-independent.
|
||||
func fakeKiroACPCompletedToolCloseErrorScript(toolName, command string) string {
|
||||
params := `{}`
|
||||
if command != "" {
|
||||
params = `{"command":"` + command + `"}`
|
||||
}
|
||||
return `#!/bin/sh
|
||||
while IFS= read -r line; do
|
||||
id=$(printf '%s' "$line" | sed -n 's/.*"id":\([0-9]*\).*/\1/p')
|
||||
@@ -580,12 +567,12 @@ while IFS= read -r line; do
|
||||
printf '{"jsonrpc":"2.0","id":%s,"result":{"protocolVersion":1,"agentCapabilities":{"loadSession":true}}}\n' "$id"
|
||||
;;
|
||||
*'"method":"session/new"'*)
|
||||
printf '{"jsonrpc":"2.0","id":%s,"result":{"sessionId":"ses_failed_done"}}\n' "$id"
|
||||
printf '{"jsonrpc":"2.0","id":%s,"result":{"sessionId":"ses_completed"}}\n' "$id"
|
||||
;;
|
||||
*'"method":"session/prompt"'*)
|
||||
printf '{"jsonrpc":"2.0","method":"session/notification","params":{"sessionId":"ses_failed_done","update":{"type":"ToolCall","toolCallId":"tc-fail","name":"execute_bash","status":"pending","parameters":{"command":"multica issue comment add issue-1 --content-file ./reply.md"}}}}\n'
|
||||
printf '{"jsonrpc":"2.0","method":"session/notification","params":{"sessionId":"ses_failed_done","update":{"type":"ToolCallUpdate","toolCallId":"tc-fail","status":"failed","name":"execute_bash","output":"exit status 1"}}}\n'
|
||||
printf '{"jsonrpc":"2.0","id":%s,"error":{"code":-32603,"message":"Internal error","data":"Kiro failed to generate a response"}}\n' "$id"
|
||||
printf '{"jsonrpc":"2.0","method":"session/notification","params":{"sessionId":"ses_completed","update":{"type":"ToolCall","toolCallId":"tc-done","name":"` + toolName + `","status":"pending","parameters":` + params + `}}}\n'
|
||||
printf '{"jsonrpc":"2.0","method":"session/notification","params":{"sessionId":"ses_completed","update":{"type":"ToolCallUpdate","toolCallId":"tc-done","status":"completed","name":"` + toolName + `","output":"ok"}}}\n'
|
||||
` + closeErrorFrame + `
|
||||
exit 0
|
||||
;;
|
||||
esac
|
||||
@@ -593,44 +580,119 @@ done
|
||||
`
|
||||
}
|
||||
|
||||
// TestKiroBackendDoesNotCompleteAfterFailedCommentAddResult guards the
|
||||
// tradeoff: a failed ToolResult must not be rescued by the running-tool path.
|
||||
// TestKiroBackendTreatsCompletedCommentAddWithNonTerminalTitleAsCompleted is
|
||||
// the #5509 fix proper: a comment-add whose shell tool is titled with a
|
||||
// non-terminal name but DID complete (positive proof) must be preserved as
|
||||
// completed through the -32603 close handshake.
|
||||
func TestKiroBackendTreatsCompletedCommentAddWithNonTerminalTitleAsCompleted(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
result := runKiroCloseErrorScript(t, fakeKiroACPCompletedToolCloseErrorScript(
|
||||
"execute_bash",
|
||||
"multica issue comment add issue-1 --content-file ./reply.md",
|
||||
))
|
||||
if result.Status != "completed" {
|
||||
t.Fatalf("expected status=completed for a completed comment-add + close error, got %q (error=%q)", result.Status, result.Error)
|
||||
}
|
||||
if result.Error != "" {
|
||||
t.Fatalf("expected close-handshake error to be suppressed, got %q", result.Error)
|
||||
}
|
||||
}
|
||||
|
||||
// fakeKiroACPTwoCommentCloseErrorScript emits two comment-add tool calls with
|
||||
// distinct CallIDs and configurable terminal statuses (in order), then the
|
||||
// -32603 close handshake. This exercises ordering: only the most recent
|
||||
// finishing-tool result should decide completion.
|
||||
func fakeKiroACPTwoCommentCloseErrorScript(firstStatus, secondStatus string) string {
|
||||
return `#!/bin/sh
|
||||
while IFS= read -r line; do
|
||||
id=$(printf '%s' "$line" | sed -n 's/.*"id":\([0-9]*\).*/\1/p')
|
||||
case "$line" in
|
||||
*'"method":"initialize"'*)
|
||||
printf '{"jsonrpc":"2.0","id":%s,"result":{"protocolVersion":1,"agentCapabilities":{"loadSession":true}}}\n' "$id"
|
||||
;;
|
||||
*'"method":"session/new"'*)
|
||||
printf '{"jsonrpc":"2.0","id":%s,"result":{"sessionId":"ses_two"}}\n' "$id"
|
||||
;;
|
||||
*'"method":"session/prompt"'*)
|
||||
printf '{"jsonrpc":"2.0","method":"session/notification","params":{"sessionId":"ses_two","update":{"type":"ToolCall","toolCallId":"tc-1","name":"terminal","status":"pending","parameters":{"command":"multica issue comment add issue-1 --content-file ./progress.md"}}}}\n'
|
||||
printf '{"jsonrpc":"2.0","method":"session/notification","params":{"sessionId":"ses_two","update":{"type":"ToolCallUpdate","toolCallId":"tc-1","status":"` + firstStatus + `","name":"terminal","output":"first"}}}\n'
|
||||
printf '{"jsonrpc":"2.0","method":"session/notification","params":{"sessionId":"ses_two","update":{"type":"ToolCall","toolCallId":"tc-2","name":"terminal","status":"pending","parameters":{"command":"multica issue comment add issue-1 --content-file ./final.md"}}}}\n'
|
||||
printf '{"jsonrpc":"2.0","method":"session/notification","params":{"sessionId":"ses_two","update":{"type":"ToolCallUpdate","toolCallId":"tc-2","status":"` + secondStatus + `","name":"terminal","output":"second"}}}\n'
|
||||
` + closeErrorFrame + `
|
||||
exit 0
|
||||
;;
|
||||
esac
|
||||
done
|
||||
`
|
||||
}
|
||||
|
||||
// TestKiroBackendFailedFinalCommentOverridesEarlierSuccess is the #5511 review
|
||||
// Must-fix #2: "progress comment completed → final comment failed → -32603"
|
||||
// must stay failed. An earlier success must not mask a later failure.
|
||||
func TestKiroBackendFailedFinalCommentOverridesEarlierSuccess(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
result := runKiroCloseErrorScript(t, fakeKiroACPTwoCommentCloseErrorScript("completed", "failed"))
|
||||
if result.Status != "failed" {
|
||||
t.Fatalf("expected status=failed when the final comment failed after an earlier success, got %q (error=%q)", result.Status, result.Error)
|
||||
}
|
||||
if !strings.Contains(result.Error, "Kiro failed to generate a response") {
|
||||
t.Fatalf("expected original prompt error to be preserved, got %q", result.Error)
|
||||
}
|
||||
}
|
||||
|
||||
// TestKiroBackendCompletedRetryAfterEarlierFailureIsCompleted is the reverse
|
||||
// ordering the review flagged: "first attempt failed → retry completed →
|
||||
// -32603" is a real completion and must be preserved.
|
||||
func TestKiroBackendCompletedRetryAfterEarlierFailureIsCompleted(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
result := runKiroCloseErrorScript(t, fakeKiroACPTwoCommentCloseErrorScript("failed", "completed"))
|
||||
if result.Status != "completed" {
|
||||
t.Fatalf("expected status=completed when a retry completed after an earlier failure, got %q (error=%q)", result.Status, result.Error)
|
||||
}
|
||||
if result.Error != "" {
|
||||
t.Fatalf("expected close-handshake error to be suppressed, got %q", result.Error)
|
||||
}
|
||||
}
|
||||
|
||||
// fakeKiroACPFailedCommentAddCloseErrorScript: the finishing tool emits a
|
||||
// failed ToolResult before the close error — a genuine failure that must stay
|
||||
// failed.
|
||||
func fakeKiroACPFailedCommentAddCloseErrorScript() string {
|
||||
return `#!/bin/sh
|
||||
while IFS= read -r line; do
|
||||
id=$(printf '%s' "$line" | sed -n 's/.*"id":\([0-9]*\).*/\1/p')
|
||||
case "$line" in
|
||||
*'"method":"initialize"'*)
|
||||
printf '{"jsonrpc":"2.0","id":%s,"result":{"protocolVersion":1,"agentCapabilities":{"loadSession":true}}}\n' "$id"
|
||||
;;
|
||||
*'"method":"session/new"'*)
|
||||
printf '{"jsonrpc":"2.0","id":%s,"result":{"sessionId":"ses_failed"}}\n' "$id"
|
||||
;;
|
||||
*'"method":"session/prompt"'*)
|
||||
printf '{"jsonrpc":"2.0","method":"session/notification","params":{"sessionId":"ses_failed","update":{"type":"ToolCall","toolCallId":"tc-fail","name":"execute_bash","status":"pending","parameters":{"command":"multica issue comment add issue-1 --content-file ./reply.md"}}}}\n'
|
||||
printf '{"jsonrpc":"2.0","method":"session/notification","params":{"sessionId":"ses_failed","update":{"type":"ToolCallUpdate","toolCallId":"tc-fail","status":"failed","name":"execute_bash","output":"exit status 1"}}}\n'
|
||||
` + closeErrorFrame + `
|
||||
exit 0
|
||||
;;
|
||||
esac
|
||||
done
|
||||
`
|
||||
}
|
||||
|
||||
// TestKiroBackendDoesNotCompleteAfterFailedCommentAddResult guards the case
|
||||
// where the finishing tool explicitly failed: it must stay failed.
|
||||
func TestKiroBackendDoesNotCompleteAfterFailedCommentAddResult(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
fakePath := filepath.Join(t.TempDir(), "kiro-cli")
|
||||
writeTestExecutable(t, fakePath, []byte(fakeKiroACPFailedRunningToolCloseErrorScript()))
|
||||
|
||||
backend, err := New("kiro", Config{ExecutablePath: fakePath, Logger: slog.Default()})
|
||||
if err != nil {
|
||||
t.Fatalf("new kiro backend: %v", err)
|
||||
result := runKiroCloseErrorScript(t, fakeKiroACPFailedCommentAddCloseErrorScript())
|
||||
if result.Status != "failed" {
|
||||
t.Fatalf("expected status=failed after a failed comment-add result, got %q (error=%q)", result.Status, result.Error)
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
||||
defer cancel()
|
||||
|
||||
session, err := backend.Execute(ctx, "prompt-ignored", ExecOptions{Timeout: 5 * time.Second})
|
||||
if err != nil {
|
||||
t.Fatalf("execute: %v", err)
|
||||
}
|
||||
go func() {
|
||||
for range session.Messages {
|
||||
}
|
||||
}()
|
||||
|
||||
select {
|
||||
case result, ok := <-session.Result:
|
||||
if !ok {
|
||||
t.Fatal("result channel closed without a value")
|
||||
}
|
||||
if result.Status != "failed" {
|
||||
t.Fatalf("expected status=failed after a failed comment-add result, got %q (error=%q)", result.Status, result.Error)
|
||||
}
|
||||
if !strings.Contains(result.Error, "Kiro failed to generate a response") {
|
||||
t.Fatalf("expected original prompt error to be preserved, got %q", result.Error)
|
||||
}
|
||||
case <-time.After(10 * time.Second):
|
||||
t.Fatal("timeout waiting for result")
|
||||
if !strings.Contains(result.Error, "Kiro failed to generate a response") {
|
||||
t.Fatalf("expected original prompt error to be preserved, got %q", result.Error)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user