highperfocused/multica
1
0
Fork 0
mirror of https://github.com/multica-ai/multica.git synced 2026-07-28 22:17:48 +02:00
Code Issues Packages Projects Releases Wiki Activity
4,311 Commits 1,867 Branches 134 Tags
transcript-all
Commit Graph

14 Commits

Author SHA1 Message Date
Bohan Jiang
a5a42846e6 fix(daemon): retry with a fresh session only when the resume was actually rejected (MUL-4966) (#5715) 
* fix(daemon): gate fresh-session retry on tools executed, not session id (MUL-4966)

Switching provider accounts leaves the stored session id pointing at a
conversation the new account does not own. The daemon still passes it to
--resume, the provider rejects it, and the task dies before doing any work.

The existing fresh-session fallback was supposed to catch this but was
gated on `result.SessionID == ""`, which is not a lifecycle fact:

- Too narrow: a backend that echoes the requested id back when it rejects
  a resume keeps SessionID non-empty, so the fallback never fired — the
  reported bug.
- Too broad: a provider 401 before the first stream message also leaves
  SessionID empty, so an unrecoverable auth failure burned a second full
  run.

Gate on `tools == 0` instead. That states the property that actually makes
a retry safe — the agent executed no tool, so it mutated nothing, so
re-running cannot double-post a comment (comment creation has no
idempotency key and a duplicate re-fires its @mention triggers), reopen a
PR, or re-plan on top of its own half-finished work in the reused workdir.
Auth failures are excluded, mirroring retryableReasons in service/task.go.

The predicate is extracted to shouldRetryWithFreshSession so the tests
exercise production logic; both existing fallback tests re-implemented the
condition inline and would not have caught a regression in it.

Co-authored-by: multica-agent <github@multica.ai>

* fix(daemon): gate fresh-session retry on a positive resume-rejected signal (MUL-4966)

Review of the previous commit was right: `tools == 0` plus "not an auth
error" answers whether re-running is *safe*, not whether a new session can
*fix* the failure. Those are orthogonal, and answering the second by
exclusion inverts the burden of proof — the failures a fresh session cures
are a small enumerable set, while the ones it cannot are open-ended.

Concretely, the previous predicate fresh-retried on provider_network,
429/529, quota, 5xx and unclassified startup failures. provider_network is
the sharpest conflict: internal/service/task.go marks it resume-safe
(MUL-4910) specifically so the platform retry inherits the session and
continues the truncated conversation. Resetting the session first made that
contract unsatisfiable, silently discarding conversation context on a
transient blip — and rate limits got an immediate no-backoff re-run.

Replace the inference with positive evidence: agent.Result gains an explicit
ResumeRejected field, set only when a backend has proof the resume itself
was refused. claude/codebuddy/qwen derive it from resumeWasRejected, which
promotes the predicate resolveSessionID was already computing and encoding
as the side effect of blanking SessionID — using an empty string to carry
that meaning is what made the original bug possible. SessionID keeps being
dropped for a rejected resume (a dead pointer must not be persisted), but it
is no longer the signal the daemon reads to decide *why* a run failed.

The six ACP backends that recover from "session not found" set the flag at
the same points they already clear the id, so their existing recovery is not
caught by the narrower gate. codex needs nothing: thread/resume already falls
back to thread/start in-process, and deliberately does not on transport
errors.

Matching now includes the account-switch guardrail reported in #5704
(Claude Code 2.1.207, zh-CN): "400 此 session 已绑定另外的ai账号，请执行
/new 开启新 session". The en-US wording of the same guardrail has not been
captured yet, so those variants are marked inferred in the source; a miss
degrades to a terminal failure carrying the provider's raw text rather than
a mis-routed run.

Tests: backend-level fixtures drive ResumeRejected from real stream-json for
both the account-binding 400 and a network drop, and the predicate now
covers network/rate-limit/quota/5xx/auth/unclassified as explicit
non-retries.

Co-authored-by: multica-agent <github@multica.ai>

* fix(daemon): restore fresh-session recovery for backends with no rejection signal (MUL-4966)

Final review caught qwen regressing: its verified rejection string
("No saved session found with ID ...", already captured in
testdata/qwen-code-0.20.0-resume-not-found.stderr.txt) was not in the phrase
list, and qwen reports no session id on that path, so the new inclusion gate
turned a working auto-recovery into a terminal failure.

Auditing the other 17 resume-capable backends showed qwen was not alone.
antigravity, copilot, cursor, deveco and opencode all recovered from a
refused resume purely by reporting an empty SessionID, and none of them has
any rejection detection to convert into ResumeRejected — copilot's own
comment documents the hole (session.error before session.start), and
antigravity's helper returns "" when "the CLI exited before dispatching".
Making ResumeRejected the sole gate silently removed recovery from all five.

Fixing that by guessing rejection phrases for five more CLIs is the wrong
trade: no real output has been captured for any of them, and a false
positive discards a recoverable session pointer. So the gate is now two
tiers. Positive evidence (ResumeRejected) decides on its own where a backend
can produce it. Where none is available, an empty SessionID still gates the
retry — it proves no session was established, which is exactly what the
pre-change behaviour relied on — minus the classes a fresh session provably
cannot cure (network, rate limit, quota, provider 5xx, auth). That keeps the
resume-safe contract in internal/service/task.go intact while restoring what
these five backends had.

Also renames claudeResumeRejectedPhrases to resumeRejectedPhrases: it is
matched by claude, codebuddy and qwen, so a qwen-only string living under a
claude-prefixed name would be actively misleading.

Tests: qwen's existing missing-resume fixture now asserts ResumeRejected
(verified failing without the phrase), and the predicate covers the
no-signal tiers — retry when nothing was established, no retry once a
session exists or the failure classifies as uncurable.

Co-authored-by: multica-agent <github@multica.ai>

* fix(daemon): scope the no-signal fallback to backends that cannot detect rejections (MUL-4966)

Final review caught the compatibility path applying to every backend, not
just the five it was justified for. shouldRetryWithFreshSession only saw
(Result, priorSessionID, tools), so a false ResumeRejected could not be told
apart from a backend that has no way to answer — and claude/codebuddy/qwen/ACP
startup failures with no session id still fell through to the exclusion
branch. That contradicted both the stated intent and the function's own doc
comment ("where a backend can produce it, it is the whole answer").

Make the capability explicit. agent.ResumeRejectionUndetectable names the
five backends that scrape SessionID out of stream output and have no
rejection detection at all; the daemon takes provider and consults it, so a
capable backend reporting false is now taken at its word. Membership is
opt-in, so a new backend fails closed instead of silently inheriting a
guess-based retry.

Also completes the exclusion set: missing config, unavailable model, missing
executable, unsupported runtime version and (defensively) agent timeout all
have defined non-session remedies and were reaching `default: true`. What is
left through stays narrow — unknown, process failure, unparseable output,
context overflow — because a real rejection from these five most likely
surfaces as a non-zero exit or unparseable output, none of them reporting one
explicitly.

Tests: one identical result asserted across all five undetectable backends
(retries), twelve capable ones (no retry), and an unregistered provider
(fails closed), plus table cases for each newly excluded reason. Classifier
inputs were verified to map to the intended reasons rather than passing by
accident.

Also updates the ResumeRejected doc comment, which still said the daemon
gates on it alone.

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: multica-agent <github@multica.ai>
 2026-07-21 19:07:46 +08:00
Multica Eve
f0d7a444ac MUL-4860 fix(acp/kiro): GPT-5.6 Sol completion — object rawOutput + positive-proof guard (follow-up to #5511) (#5515) 
* 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>
 2026-07-16 13:36:45 +08:00
Multica Eve
dd692058d7 fix(kiro): preserve completed status on GPT-5.6 Sol -32603 close handshake (#5509) (#5511) 
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>
 2026-07-16 12:33:15 +08:00
ZeroIce
1e85eb0aac Fix Kiro ACP usage accounting (#4867) 
Co-authored-by: multica-agent <github@multica.ai>
 2026-07-03 12:00:08 +08:00
Multica Eve
f0c3b0911c MUL-3828: fix Cursor and Kiro runtime completion transcripts (#4738) 
* fix: preserve cursor and kiro completion transcripts

Co-authored-by: multica-agent <github@multica.ai>

* fix(kiro): tolerate env-prefix and sh -c wrapper in comment-add detection

Harden isKiroIssueCommentAddCommand so the completion-preservation guard
also recognizes 'VAR=x multica issue comment add ...' and
'sh -c "multica issue comment add ..."' invocations. Addresses review nit
on MUL-3828 PR #4738.

---------

Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>
Co-authored-by: J <bhjiang@outlook.com>
 2026-07-01 13:23:12 +08:00
Multica Eve
b71d9d0ab9 MUL-3674: Preserve Kiro goal completion on close error (#4560) 
* fix(daemon): preserve Kiro goal completion on close error

Co-authored-by: multica-agent <github@multica.ai>

* fix(daemon): require completed Kiro goal marker

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>
 2026-06-25 12:43:04 +08:00
Truffle
6acca84c28 fix(agent): clear stale session id when a resumed ACP session is gone [MUL-3216] (#4015) 
* 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.
 2026-06-11 14:54:56 +08:00
Bohan Jiang
3708fb0f07 fix(daemon): inactivity-based agent run timeout, no wall-clock guillotine (MUL-3064) 
Active long-running sessions are no longer killed by a fixed wall-clock deadline. Liveness is delegated to the idle watchdog (MULTICA_AGENT_IDLE_WATCHDOG, default 30m) with a larger in-flight-tool budget (MULTICA_AGENT_TOOL_WATCHDOG, default 2h). MULTICA_AGENT_TIMEOUT is an opt-in absolute cap (default 0 = no cap). The server-side 2.5h sweeper is unchanged as a coarse backstop.

Fixes #3745.
 2026-06-05 15:06:07 +08:00
Bohan Jiang
09f9c7e2ce MUL-2764 feat(agent): wire mcp_config through ACP runtimes (Hermes / Kimi / Kiro) (#3439) 
* 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>
 2026-05-28 16:29:49 +08:00
Bohan Jiang
b73a301bf9 fix(agent): drain stderr before deciding ACP failure promotion (#2333) 
`hermes`, `kimi`, and `kiro` all wired stderr through
`cmd.Stderr = io.MultiWriter(logWriter, providerErrSniffer)`.
The OS-pipe → MultiWriter copy goroutine that exec spawns for
that form is only joined by `cmd.Wait()`, which the lifecycle
goroutine fires in deferred cleanup — *after*
`promoteACPResultOnProviderError` already consulted the sniffer.
When stopReason=end_turn (success) raced ahead of the stderr
drain, the sniffer's `lines` slice was empty, the helper fell
through to the synthetic agent-text fallback ("hermes provider
error: API call failed after 3 retries"), and the actionable
upstream signal (HTTP 429 / usage limit) was lost.

This was visible as a flaky
`TestHermesBackendPromotesProviderErrorWithNonEmptyOutput` in CI
under high parallelism — a real prod bug, not a test issue: live
runs hit the same race when an upstream LLM returns 429 and
hermes' synthetic agent turn beats the stderr drain to the
parent.

Replace the MultiWriter wiring with `cmd.StderrPipe()` + an
explicit copier goroutine that signals on `stderrDone`. The
lifecycle goroutine already awaits `<-readerDone` for stdout;
add `<-stderrDone` next to it before `promoteACPResultOnProviderError`
runs. The deferred `cmd.Wait()` ordering is unchanged — it just
becomes a cheap reap by the time it fires.

Verified: `go test ./pkg/agent/ -run "TestHermes|TestKimi|TestKiro"
-count=10 -race`, then full package `-count=3 -race`, all green.

Co-authored-by: multica-agent <github@multica.ai>
 2026-05-09 17:34:25 +08:00
Bohan Jiang
c57546159d fix(daemon): mark provider 429 / out-of-credit agent runs as failed, not completed (#2323) 
* fix(daemon): mark provider 429 / out-of-credit runs as failed, not completed

Two bugs combined to silently report failed agent runs as
"Completed" in the UI when the upstream LLM returned a 4xx (e.g.
HTTP 429 rate-limit / no credit on the account).

1. ACP backends (hermes, kimi, kiro) only promoted the run status to
   "failed" when their stderr sniffer fired AND the agent output
   buffer was empty. But hermes injects a synthetic agent text turn
   ("API call failed after 3 retries: HTTP 429...") on retry
   exhaustion, so the buffer was never empty in the rate-limit
   case and the promotion never ran. Drop the empty-output
   precondition: the sniffer's regex (HTTP-status markers, named
   error types) is specific enough to trust on its own.

2. The daemon's task-result switch only routed "blocked" through
   FailTask; every other status — including "cancelled", and any
   future status we forget to enumerate — fell through to
   CompleteTask. Invert it so only an explicit "completed" status
   reports success, and extract the switch into reportTaskResult
   for direct testing. Cancelled now defaults to failure_reason
   "cancelled" instead of being silently completed.

Closes GitHub multica#1952.

Co-authored-by: multica-agent <github@multica.ai>

* fix(agent): only promote ACP run to failed on terminal provider error

Address GPT-Boy's review on the multica#1952 fix. The previous
promotion rule ("any sniffer line → fail") was too broad: the
existing sniffer also captures transient per-attempt warnings
("API call failed (attempt 1/3): RateLimitError [HTTP 429]"), and
those lines stay in the buffer for the rest of the run. A retry
sequence whose first attempt blipped but whose third attempt
succeeded would have been wrongly reported as failed.

Tighten the criteria with two additional signals, both defined on
the existing acpProviderErrorSniffer / output buffer:

- acpTerminalErrorRe — sticky `terminal` flag set when stderr shows
  an exhausted/non-retryable marker (❌, [ERROR], "after N retries",
  Non-retryable, BadRequestError, AuthenticationError). Per-attempt
  warnings deliberately don't match.
- acpAgentOutputTerminalRe — matches the synthetic "API call failed
  after N retries..." turn that hermes-style adapters inject into
  the agent text stream when they give up; this catches multica#1952
  even if hermes' stderr only logged transient attempts.

Promotion logic becomes a shared helper, promoteACPResultOnProviderError,
called from hermes / kimi / kiro. Promotes when (a) terminalMessage
is non-empty, (b) output contains the synthetic give-up turn, or
(c) output is empty and the sniffer captured anything at all
(preserves the original empty-output safety net for transient-only
sequences with no real result to fall back on).

Tests:
- TestHermesProviderErrorSnifferTerminalVsTransient — transient
  attempt 1/3 alone returns terminalMessage="" but message!="";
  a follow-on terminal marker flips terminal on.
- TestHermesProviderErrorSnifferTerminalNonRetryable — confirms
  BadRequest / Authentication / Non-retryable / ❌ / [ERROR] are
  classified terminal even on the very first attempt.
- TestHermesBackendDoesNotPromoteOnTransientRetry — fake hermes
  emits attempt 1/3 to stderr then a normal agent text turn and
  end_turn; resulting Status must stay "completed".

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: multica-agent <github@multica.ai>
 2026-05-09 16:13:12 +08:00
prellr
ee10c508fb fix(daemon): trust the agent's session id from session/resume across ACP backends (#2070) 
When the local state.db of an ACP backend (hermes, kimi, kiro) is wiped
— crash, config change, manual kill, container reset — the backend's
session/resume (or session/load, in kiro's case) silently creates a
brand-new session rather than failing, and returns the new id in the
response. Today the daemon ignores the response and stamps
sessionID = opts.ResumeSessionID across all three backends, so every
subsequent session/prompt is addressed to a session id the backend has
no record of. The task fails with JSON-RPC -32603 (Internal error) on
the very first turn, with no operator-visible signal that the problem
is a session-id mismatch one layer down.

The behavior is invisible: agent shows "started", then "failed" with a
generic Internal error. Reproducing in production took repeated runs
because nothing in the logs pointed at the silent reset.

Fix: route all three ACP backends through a small `resolveResumedSessionID`
helper that:

- prefers the id the backend returned in its response (the canonical
  id; the one the backend will accept on the next call)
- falls back to the requested id when the response is malformed,
  empty, or omits sessionId — defensive fallback so older / non-
  conforming backends (notably kiro's current session/load shape)
  behave identically to today
- signals (via a bool) when the id changed, so the caller logs a Warn
  with `backend=<hermes|kimi|kiro>` and operators can grep for silent
  state resets to correlate them with task failures

Why this is at the backend layer rather than the daemon's existing
session-resume fallback: server/internal/daemon/daemon.go:1554-1566
already retries with a fresh session when resume fails, but it gates
on `result.Status == "failed" && result.SessionID == ""`. The backend
WILL hand back a result.SessionID — just the new one it silently
committed to — so the daemon-level fallback never fires for this
failure mode.

The helper is also what session/new already uses (extractACPSessionID,
documented in code as "Shared by all ACP backends"). session/new
extracts the canonical id from the response; session/resume just
didn't, until now.

Coverage:
- hermes.go: confirmed bug, root cause of -32603 in production
- kimi.go: same code shape, same protocol method, same response
  schema as hermes (per extractACPSessionID's comment) — same bug
- kiro.go: same code shape, different method (session/load). Current
  observed response doesn't include sessionId, so the defensive
  fallback means today's behavior is preserved. Routing through the
  same helper means a future kiro release that DOES return a sessionId
  on silent reset works the same way as hermes/kimi without another
  diff.

Tests (server/pkg/agent/hermes_test.go — helper covers all three
backends, no per-backend duplication):
- TestResolveResumedSessionIDMatching — backend confirms requested id
- TestResolveResumedSessionIDDifferent — backend returned a new id;
  caller is told to switch
- TestResolveResumedSessionIDEmptyResponse — older / malformed body;
  defensive fallback to requested id (covers kiro's current shape)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
 2026-05-06 14:15:40 +08:00
LinYushen
03f3180b8f fix(agent): ignore Kiro session/load history replay (#1789) 
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.
 2026-04-28 17:50:13 +08:00
LinYushen
c366cf2ba1 feat(agent): add Kiro CLI ACP runtime (#1780) 
* feat(agent): add kiro cli acp runtime

* fix(agent): align kiro acp prompt and notifications

* chore(agent): clarify kiro acp args compatibility
 2026-04-28 17:03:46 +08:00