* 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>
* fix(acp): don't drop completed tool_call_update when rawOutput is an object (MUL-4860, #5509)
Captured a live kiro-cli 2.12.3 + gpt-5.6-sol ACP trace and found the
true root cause: the completed tool_call_update sends rawOutput as an
OBJECT ({"items":[{"Json":{...}}]}), but handleToolCallUpdate typed
rawOutput/output as Go strings. json.Unmarshal then failed with
'cannot unmarshal object into Go struct field .rawOutput of type string'
and the handler returned early — silently dropping the ENTIRE update,
status:"completed" included. So no completion signal ever reached the
kiro completion-preservation guard, and the durable-but-closed task was
marked failed. This also explains the issue's 'shell tool recorded as
running': the tool_call title is literally 'Running: <cmd>' (kind
execute), which normalizes to 'running', not 'terminal'.
Fix: type rawOutput/output as json.RawMessage and render them via a new
acpRawText helper that accepts both a JSON string and a structured
value. This is a shared ACP-layer fix (hermes/kimi/kiro).
Combined with the earlier payload-based comment-add recognition and the
positive-proof completion guard, the completed result now flows through
and the -32603 close handshake correctly preserves completion.
Adds TestKiroBackendPreservesCompletionOnRealGPT56SolFrames (the exact
captured wire shape end-to-end) and TestACPRawText. Verified the
end-to-end test fails (status=failed) when rawOutput is typed as string
and passes after the fix.
Co-authored-by: multica-agent <github@multica.ai>
* fix(kiro): remove duplicate stale-session branch that faked completion (MUL-4860, #5509)
Addresses Elon's 3rd-round review of #5515. The -32603 guard had two
consecutive, identically-conditioned else-if branches for a stale resumed
session (opts.ResumeSessionID != "" && isACPSessionNotFound(err)). The
first wrongly forced finalStatus="completed" and kept the stale
SessionID; the second — the correct handler that clears SessionID so the
daemon's fresh-session retry fires — was unreachable. So a recovery whose
resumed session was gone at session/prompt time was reported as a fake
success AND skipped the retry.
Remove the bogus first branch, keeping the positive-proof completion
branch and the original stale-session (SessionID="") branch.
Adds TestKiroBackendClearsSessionIDWhenPromptSessionNotFound (the prompt
path; existing coverage only exercised session/set_model). Verified it
returns completed+ses_stale on the pre-fix code and failed+empty
SessionID after. Also gofmt on the touched files.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>
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>
* fix(agent): clear stale session id when a resumed ACP session is gone
When an agent's stored ACP session no longer exists on the runtime side,
session/resume still succeeds — hermes echoes the requested sessionId
back — so the failure only surfaces when session/prompt returns JSON-RPC
-32603 "Session not found". The backend then reported Status=failed with
the stale SessionID still set, which kept the daemon's resume-failure
fallback (gated on SessionID == "") from ever firing. The failed task
never updates the stored session, so every future mention on the same
(agent, issue) dispatched against the same dead id, forever (#4010).
handleResponse now returns a structured acpRPCError instead of a flat
string (rendered text unchanged), and the hermes/kimi/kiro prompt-error
paths clear the session id when the error is session-not-found class on
a resumed session. The daemon's existing retry then re-executes with a
fresh session and stores the replacement id, healing the mapping.
* fix(agent): clear stale session id when set_model hits a dead resumed session
With a model override, session/set_model runs before session/prompt,
so a resumed session that is gone on the agent side surfaces there
instead of at the prompt — and the error branch returned the stale
SessionID, so the daemon's fresh-session retry (gated on
SessionID == "") never fired. Apply the same clear-the-id fix in the
set_model error branch of all three backends.
Also relax isACPSessionNotFound to accept -32602: kimi-cli raises
RequestError.invalid_params({"session_id": "Session not found"}) for
every unknown-session path (src/kimi_cli/acp/server.py), so pinning
-32603 made the fix dead code for kimi. The wording gate keeps
unrelated invalid_params errors (e.g. "model not available") on the
preserve-the-id path.
Regression tests for all three backends: resumed session + model
override + set_model failing with each runtime's observed
session-not-found shape must yield status=failed with an empty
SessionID.
* MUL-2764 feat(agent): wire mcp_config through ACP runtimes (Hermes / Kimi / Kiro)
The MCP config Tab (#3419) already lets admins save mcp_config on an
agent, and the daemon plumbs it through to `agent.ExecOptions.McpConfig`
for every runtime. Claude and Codex consume it; the three ACP runtimes
(Hermes / Kimi / Kiro) ignored the field and hardcoded an empty
`mcpServers: []` in their `session/new` requests.
Add `buildACPMcpServers` to translate the Claude-style `{"mcpServers":
{"<name>": {...}}}` object-of-objects into the array shape ACP requires
(`[{name, command, args, env: [{name,value}, ...]}, ...]` for stdio;
`[{type, name, url, headers: [...]}, ...]` for http/sse), then pass the
translated array on `session/new` (all three) and `session/load` (kiro
resume). Malformed JSON fails the launch closed — same contract Codex's
`renderCodexMcpServersBlock` uses — so users see a real error instead of
silently running with no MCP servers. Individual unclassifiable entries
(no command, no url) are skipped with a warning so one bad row can't
take MCP down for the rest of the agent.
Co-authored-by: multica-agent <github@multica.ai>
* MUL-2764 fix(agent): wire mcp_config through ACP resume + gate http/sse on capability
Addresses the two blockers Elon raised on #3439:
1. session/resume now carries mcpServers for Hermes and Kimi (Kiro's
session/load already did). Per the ACP Session Setup spec the resume
path re-attaches MCP servers, and without this a resumed task lost
access to MCP tools that a fresh task on the same agent would have
had. Pinned with new TestHermesResumeIncludesMcpServers and
TestKimiResumeIncludesMcpServers integration tests that inspect the
recorded wire request.
2. Added extractACPMcpCapabilities + filterACPMcpServersByCapability so
http/sse MCP entries get dropped (with a daemon-log warning naming
the entry) when the runtime's initialize response doesn't advertise
mcpCapabilities.http / .sse. Sending those entries to a stdio-only
runtime is a spec violation and reliably tanks session/new; now they
get filtered and the rest of the session still starts. Stdio entries
pass through unconditionally. Both backends wire the filter in right
after initialize so session/new and session/resume see the same
filtered list.
Also added TestKiroLoadIncludesMcpServersFromConfig — Elon flagged that
no test pinned "non-empty mcp_config actually reaches the wire" for
Kimi/Kiro, so the wire assertions go in for all three runtimes.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
Ignore Kiro ACP session/load history replay before the active prompt starts; keep task messages, usage, and tool state scoped to the current Kiro turn. Verified with go test ./pkg/agent -run TestKiro, go test ./pkg/agent, and git diff --check origin/main...HEAD.
