`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>
* 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>
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>
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.
