mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-26 12:35:35 +02:00
The -32603 completion-preservation guard coupled to the Claude/Kiro ACP event shape. GPT-5.6 Sol leaves the finishing tool (goal_complete / 'multica issue comment add') parked at 'running' and titles the shell tool with a name that does not normalize to 'terminal', so: - isKiroIssueCommentAddTool's msg.Tool=='terminal' gate dropped the tool use entirely, and - with no completed/failed ToolCallUpdate, no ToolResult is emitted, so the saw*Completed flags never flipped. Completed tasks were therefore reversed to failed with agent_error.provider_server_error ~17s after finishing their work. Fix: - Recognize comment-add by its command payload, independent of the normalized tool title. - Track each finishing tool along use / result / completed axes and preserve completed when the -32603 close handshake follows a tool use that never produced a terminal result. A genuinely failed ToolResult still trips saw*Result and keeps the task failed. Adds regression fixtures for the running-tool path (comment-add and goal_complete) plus a failed-result safety case. Co-authored-by: Eve <eve@multica-ai.local> Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
@@ -107,8 +107,27 @@ func (b *kiroBackend) Execute(ctx context.Context, prompt string, opts ExecOptio
|
||||
var outputMu sync.Mutex
|
||||
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
|
||||
var goalCompleteCallIDs sync.Map
|
||||
var issueCommentCallIDs sync.Map
|
||||
|
||||
@@ -130,17 +149,33 @@ 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)
|
||||
}
|
||||
if msg.CallID != "" && isKiroIssueCommentAddTool(msg) {
|
||||
issueCommentCallIDs.Store(msg.CallID, struct{}{})
|
||||
// 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{}{})
|
||||
}
|
||||
}
|
||||
}
|
||||
if msg.Type == MessageToolResult {
|
||||
if _, ok := goalCompleteCallIDs.LoadAndDelete(msg.CallID); ok {
|
||||
sawCompletedGoalComplete.Store(msg.Status == "completed")
|
||||
sawGoalCompleteResult.Store(true)
|
||||
if msg.Status == "completed" {
|
||||
sawCompletedGoalComplete.Store(true)
|
||||
}
|
||||
}
|
||||
if _, ok := issueCommentCallIDs.LoadAndDelete(msg.CallID); ok {
|
||||
sawCompletedIssueComment.Store(msg.Status == "completed")
|
||||
sawIssueCommentResult.Store(true)
|
||||
if msg.Status == "completed" {
|
||||
sawCompletedIssueComment.Store(true)
|
||||
}
|
||||
}
|
||||
}
|
||||
if msg.Type == MessageText {
|
||||
@@ -333,7 +368,22 @@ func (b *kiroBackend) Execute(ctx context.Context, prompt string, opts ExecOptio
|
||||
} else {
|
||||
finalStatus = "failed"
|
||||
finalError = fmt.Sprintf("kiro session/prompt failed: %v", err)
|
||||
if (sawCompletedGoalComplete.Load() || sawCompletedIssueComment.Load()) && isKiroGoalCompleteCloseError(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) {
|
||||
b.cfg.Logger.Warn("kiro session/prompt failed after completed task result; preserving completed task status", "error", err)
|
||||
finalStatus = "completed"
|
||||
finalError = ""
|
||||
@@ -431,10 +481,14 @@ func isKiroGoalCompleteCloseError(err error) bool {
|
||||
return strings.Contains(strings.ToLower(rpcErr.Data), "failed to generate a response")
|
||||
}
|
||||
|
||||
// isKiroIssueCommentAddTool reports whether a tool-use message is a
|
||||
// `multica issue comment add` invocation. It keys purely off the command
|
||||
// payload, not the normalized tool name: GPT-5.6 Sol adapters may title the
|
||||
// shell tool with a name that doesn't fold into "terminal" (the earlier
|
||||
// msg.Tool=="terminal" gate silently dropped those and left completed tasks
|
||||
// marked failed — see #5509). isKiroIssueCommentAddCommand is strict enough
|
||||
// that no non-shell tool's input will accidentally match.
|
||||
func isKiroIssueCommentAddTool(msg Message) bool {
|
||||
if msg.Tool != "terminal" {
|
||||
return false
|
||||
}
|
||||
command, _ := msg.Input["command"].(string)
|
||||
return isKiroIssueCommentAddCommand(command)
|
||||
}
|
||||
|
||||
@@ -412,6 +412,228 @@ func TestKiroIssueCommentAddCommand(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestKiroIssueCommentAddToolIgnoresToolTitle pins the #5509 decoupling:
|
||||
// `multica issue comment add` must be recognized from its command payload
|
||||
// regardless of the tool's normalized name. The GPT-5.6 Sol adapter can title
|
||||
// the shell tool something that doesn't fold into "terminal", and the old
|
||||
// msg.Tool=="terminal" gate silently dropped those.
|
||||
func TestKiroIssueCommentAddToolIgnoresToolTitle(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
tool string
|
||||
want bool
|
||||
}{
|
||||
{"terminal", "terminal", true},
|
||||
{"gpt-style execute", "execute_bash", true},
|
||||
{"gpt-style run", "run", true},
|
||||
{"empty tool name", "", true},
|
||||
{"tool name is irrelevant when command matches", "read_file", true},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
msg := Message{
|
||||
Type: MessageToolUse,
|
||||
Tool: tt.tool,
|
||||
Input: map[string]any{"command": "multica issue comment add issue-1 --content-file ./reply.md"},
|
||||
}
|
||||
if got := isKiroIssueCommentAddTool(msg); got != tt.want {
|
||||
t.Errorf("%s: isKiroIssueCommentAddTool(tool=%q) = %v, want %v", tt.name, tt.tool, got, tt.want)
|
||||
}
|
||||
}
|
||||
|
||||
// A non-comment-add command must still be rejected regardless of title.
|
||||
notComment := Message{Type: MessageToolUse, Tool: "terminal", Input: map[string]any{"command": "ls -la"}}
|
||||
if isKiroIssueCommentAddTool(notComment) {
|
||||
t.Errorf("isKiroIssueCommentAddTool should reject a non-comment-add command")
|
||||
}
|
||||
}
|
||||
|
||||
// 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.
|
||||
func fakeKiroACPRunningToolCloseErrorScript(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')
|
||||
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_running_done"}}\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"
|
||||
exit 0
|
||||
;;
|
||||
esac
|
||||
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) {
|
||||
t.Parallel()
|
||||
|
||||
fakePath := filepath.Join(t.TempDir(), "kiro-cli")
|
||||
writeTestExecutable(t, fakePath, []byte(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)
|
||||
}
|
||||
|
||||
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")
|
||||
}
|
||||
}
|
||||
|
||||
// TestKiroBackendTreatsRunningGoalCompleteCloseErrorAsCompleted covers the
|
||||
// goal_complete sibling of the running-tool path.
|
||||
func TestKiroBackendTreatsRunningGoalCompleteCloseErrorAsCompleted(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")
|
||||
}
|
||||
}
|
||||
|
||||
// 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 {
|
||||
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_done"}}\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"
|
||||
exit 0
|
||||
;;
|
||||
esac
|
||||
done
|
||||
`
|
||||
}
|
||||
|
||||
// TestKiroBackendDoesNotCompleteAfterFailedCommentAddResult guards the
|
||||
// tradeoff: a failed ToolResult must not be rescued by the running-tool path.
|
||||
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)
|
||||
}
|
||||
|
||||
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")
|
||||
}
|
||||
}
|
||||
|
||||
// fakeKiroACPStaleLoadSetModelScript impersonates kiro when a resumed
|
||||
// session is gone and the caller picked a model: session/load returns
|
||||
// an empty result (so the requested id is kept), then
|
||||
|
||||
Reference in New Issue
Block a user