mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-06 22:09:44 +02:00
* 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.
338 lines
11 KiB
Go
338 lines
11 KiB
Go
package agent
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"log/slog"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
"time"
|
|
)
|
|
|
|
func TestNewReturnsKimiBackend(t *testing.T) {
|
|
t.Parallel()
|
|
b, err := New("kimi", Config{ExecutablePath: "/nonexistent/kimi"})
|
|
if err != nil {
|
|
t.Fatalf("New(kimi) error: %v", err)
|
|
}
|
|
if _, ok := b.(*kimiBackend); !ok {
|
|
t.Fatalf("expected *kimiBackend, got %T", b)
|
|
}
|
|
}
|
|
|
|
func TestKimiToolNameFromTitle(t *testing.T) {
|
|
t.Parallel()
|
|
tests := []struct {
|
|
title string
|
|
want string
|
|
}{
|
|
{"Read file: /tmp/foo.go", "read_file"},
|
|
{"read", "read_file"},
|
|
{"Write: /tmp/bar.go", "write_file"},
|
|
{"Edit", "edit_file"},
|
|
{"Patch: /tmp/x", "edit_file"},
|
|
{"Shell: ls -la", "terminal"},
|
|
{"Bash", "terminal"},
|
|
{"Run command: pwd", "terminal"},
|
|
{"Search: foo", "search_files"},
|
|
{"Glob: *.go", "glob"},
|
|
{"Web search: golang acp", "web_search"},
|
|
{"Fetch: https://example.com", "web_fetch"},
|
|
{"Todo Write", "todo_write"},
|
|
// Fallback: snake_case the title.
|
|
{"Custom Thing", "custom_thing"},
|
|
// Empty input returns empty — caller decides how to react.
|
|
{"", ""},
|
|
}
|
|
for _, tt := range tests {
|
|
got := kimiToolNameFromTitle(tt.title)
|
|
if got != tt.want {
|
|
t.Errorf("kimiToolNameFromTitle(%q) = %q, want %q", tt.title, got, tt.want)
|
|
}
|
|
}
|
|
}
|
|
|
|
// fakeKimiACPScript returns a POSIX-sh script that impersonates
|
|
// `kimi acp` for a single short ACP session: it acks initialize /
|
|
// session/new and then replies to session/set_model with a JSON-RPC
|
|
// error — the scenario the kimiBackend must propagate as a failed
|
|
// task rather than silently falling back to the default model.
|
|
func fakeKimiACPScript() string {
|
|
return `#!/bin/sh
|
|
# Fake ` + "`kimi`" + ` binary — used by TestKimiBackendSetModelFailureFailsTask
|
|
# and TestKimiBackendPassesYoloFlag.
|
|
#
|
|
# Writes the full argv (one arg per line) to $KIMI_ARGS_FILE if that env
|
|
# var is set, so tests can assert that the daemon invokes us with the
|
|
# right flags (`+"`--yolo acp`"+`, not bare `+"`acp`"+`).
|
|
#
|
|
# Then reads one JSON-RPC request per line from stdin, matches on the
|
|
# method name, and writes back a canned response. Exits after set_model
|
|
# so the kimiBackend cleanup path can run.
|
|
if [ -n "$KIMI_ARGS_FILE" ]; then
|
|
for arg in "$@"; do
|
|
printf '%s\n' "$arg" >> "$KIMI_ARGS_FILE"
|
|
done
|
|
fi
|
|
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":{}}}\n' "$id"
|
|
;;
|
|
*'"method":"session/new"'*)
|
|
printf '{"jsonrpc":"2.0","id":%s,"result":{"sessionId":"ses_fake"}}\n' "$id"
|
|
;;
|
|
*'"method":"session/set_model"'*)
|
|
printf '{"jsonrpc":"2.0","id":%s,"error":{"code":-32602,"message":"model not available: bogus-model"}}\n' "$id"
|
|
exit 0
|
|
;;
|
|
esac
|
|
done
|
|
`
|
|
}
|
|
|
|
// TestKimiBackendSetModelFailureFailsTask pins the "don't silently
|
|
// fall back" behaviour that landed in this PR: when kimi rejects the
|
|
// caller-selected model via session/set_model, the task result must
|
|
// report status=failed with a message that names the model and the
|
|
// upstream error — not claim success while actually running on the
|
|
// default model.
|
|
func TestKimiBackendSetModelFailureFailsTask(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
fakePath := filepath.Join(t.TempDir(), "kimi")
|
|
writeTestExecutable(t, fakePath, []byte(fakeKimiACPScript()))
|
|
|
|
backend, err := New("kimi", Config{ExecutablePath: fakePath, Logger: slog.Default()})
|
|
if err != nil {
|
|
t.Fatalf("new kimi backend: %v", err)
|
|
}
|
|
|
|
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
|
defer cancel()
|
|
|
|
session, err := backend.Execute(ctx, "prompt-ignored", ExecOptions{
|
|
Model: "bogus-model",
|
|
Timeout: 5 * time.Second,
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("execute: %v", err)
|
|
}
|
|
// Drain message stream so the lifecycle goroutine can progress.
|
|
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, got %q (error=%q)", result.Status, result.Error)
|
|
}
|
|
if !strings.Contains(result.Error, `could not switch to model "bogus-model"`) {
|
|
t.Errorf("expected error to name the requested model, got %q", result.Error)
|
|
}
|
|
if !strings.Contains(result.Error, "model not available") {
|
|
t.Errorf("expected error to surface upstream message, got %q", result.Error)
|
|
}
|
|
if result.SessionID != "ses_fake" {
|
|
t.Errorf("expected session id to be preserved on failure, got %q", result.SessionID)
|
|
}
|
|
case <-time.After(10 * time.Second):
|
|
t.Fatal("timeout waiting for result")
|
|
}
|
|
}
|
|
|
|
// fakeKimiACPStaleResumeSetModelScript impersonates kimi-cli when a
|
|
// resumed session is gone and the caller picked a model:
|
|
// session/resume echoes the requested sessionId back, then
|
|
// session/set_model rejects the unknown session the way kimi-cli
|
|
// actually does — RequestError.invalid_params (-32602) with
|
|
// {"session_id": "Session not found"} in data
|
|
// (src/kimi_cli/acp/server.py, set_session_model).
|
|
func fakeKimiACPStaleResumeSetModelScript() 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":{}}}\n' "$id"
|
|
;;
|
|
*'"method":"session/resume"'*)
|
|
sid=$(printf '%s' "$line" | sed -n 's/.*"sessionId":"\([^"]*\)".*/\1/p')
|
|
printf '{"jsonrpc":"2.0","id":%s,"result":{"sessionId":"%s"}}\n' "$id" "$sid"
|
|
;;
|
|
*'"method":"session/set_model"'*)
|
|
printf '{"jsonrpc":"2.0","id":%s,"error":{"code":-32602,"message":"Invalid params","data":{"session_id":"Session not found"}}}\n' "$id"
|
|
exit 0
|
|
;;
|
|
esac
|
|
done
|
|
`
|
|
}
|
|
|
|
// TestKimiBackendClearsSessionIDWhenSetModelSessionNotFound pins the
|
|
// set_model sibling of the resumed-session fix: with a model override,
|
|
// session/set_model runs before session/prompt, so a dead resumed
|
|
// session surfaces there. The Result must carry an empty SessionID so
|
|
// the daemon's fresh-session retry (gated on SessionID == "") fires.
|
|
func TestKimiBackendClearsSessionIDWhenSetModelSessionNotFound(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
fakePath := filepath.Join(t.TempDir(), "kimi")
|
|
writeTestExecutable(t, fakePath, []byte(fakeKimiACPStaleResumeSetModelScript()))
|
|
|
|
backend, err := New("kimi", Config{ExecutablePath: fakePath, Logger: slog.Default()})
|
|
if err != nil {
|
|
t.Fatalf("new kimi 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,
|
|
ResumeSessionID: "ses_stale",
|
|
Model: "kimi-for-coding",
|
|
})
|
|
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, got %q (error=%q)", result.Status, result.Error)
|
|
}
|
|
if !strings.Contains(result.Error, `could not switch to model "kimi-for-coding"`) {
|
|
t.Errorf("expected error to name the requested model, got %q", result.Error)
|
|
}
|
|
if result.SessionID != "" {
|
|
t.Errorf("expected empty session id so the daemon's fresh-session retry fires, got %q", result.SessionID)
|
|
}
|
|
case <-time.After(10 * time.Second):
|
|
t.Fatal("timeout waiting for result")
|
|
}
|
|
}
|
|
|
|
// TestKimiBackendInvokesACPSubcommand pins the argv for `kimi`. An
|
|
// earlier fix tried passing `--yolo` to bypass per-tool approval
|
|
// prompts, but the `acp` subcommand in kimi-cli takes no options
|
|
// (see cli/__init__.py @cli.command def acp()), so `--yolo` was a
|
|
// no-op and the daemon still hung for 5 min on the first Shell call.
|
|
// The actual bypass is in hermesClient.handleAgentRequest, which
|
|
// auto-approves session/request_permission. This test catches
|
|
// accidental re-introduction of the dead flag.
|
|
func TestKimiBackendInvokesACPSubcommand(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
tempDir := t.TempDir()
|
|
argsFile := filepath.Join(tempDir, "argv.txt")
|
|
fakePath := filepath.Join(tempDir, "kimi")
|
|
writeTestExecutable(t, fakePath, []byte(fakeKimiACPScript()))
|
|
|
|
backend, err := New("kimi", Config{
|
|
ExecutablePath: fakePath,
|
|
Logger: slog.Default(),
|
|
Env: map[string]string{"KIMI_ARGS_FILE": argsFile},
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("new kimi backend: %v", err)
|
|
}
|
|
|
|
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
|
defer cancel()
|
|
|
|
// Set Model so the fake binary exits on set_model and we don't
|
|
// have to wait for the prompt branch. We only care about argv here.
|
|
session, err := backend.Execute(ctx, "prompt-ignored", ExecOptions{
|
|
Model: "bogus-model",
|
|
Timeout: 5 * time.Second,
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("execute: %v", err)
|
|
}
|
|
go func() {
|
|
for range session.Messages {
|
|
}
|
|
}()
|
|
<-session.Result
|
|
|
|
raw, err := os.ReadFile(argsFile)
|
|
if err != nil {
|
|
t.Fatalf("read args file: %v", err)
|
|
}
|
|
lines := strings.Split(strings.TrimSpace(string(raw)), "\n")
|
|
if len(lines) < 1 {
|
|
t.Fatalf("expected at least 1 arg (acp), got %d: %q", len(lines), lines)
|
|
}
|
|
if lines[0] != "acp" {
|
|
t.Errorf("expected first arg to be acp, got %q (full: %q)", lines[0], lines)
|
|
}
|
|
for _, l := range lines {
|
|
switch l {
|
|
case "--yolo", "--auto-approve", "--yes", "-y":
|
|
t.Errorf("kimi acp doesn't accept %q; auto-approval is handled in hermesClient.handleAgentRequest", l)
|
|
}
|
|
}
|
|
}
|
|
|
|
// TestKimiResumeIncludesMcpServers pins the same contract as the matching
|
|
// Hermes test: session/resume must carry the managed MCP set so a resumed
|
|
// Kimi task has the same MCP tools as a fresh one.
|
|
func TestKimiResumeIncludesMcpServers(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
recordPath := filepath.Join(t.TempDir(), "frames.jsonl")
|
|
fakePath := filepath.Join(t.TempDir(), "kimi")
|
|
writeTestExecutable(t, fakePath, []byte(fakeACPRecordingScript(recordPath, "ses_resume", `{}`)))
|
|
|
|
backend, err := New("kimi", Config{ExecutablePath: fakePath, Logger: slog.Default()})
|
|
if err != nil {
|
|
t.Fatalf("new kimi 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,
|
|
ResumeSessionID: "ses_resume",
|
|
McpConfig: json.RawMessage(`{"mcpServers":{"fetch":{"command":"uvx"}}}`),
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("execute: %v", err)
|
|
}
|
|
go func() {
|
|
for range session.Messages {
|
|
}
|
|
}()
|
|
select {
|
|
case <-session.Result:
|
|
case <-time.After(10 * time.Second):
|
|
t.Fatal("timeout waiting for result")
|
|
}
|
|
|
|
frame := findRecordedFrame(t, recordPath, "session/resume")
|
|
params := frame["params"].(map[string]any)
|
|
servers, ok := params["mcpServers"].([]any)
|
|
if !ok {
|
|
t.Fatalf("session/resume.mcpServers: got %T, want []any", params["mcpServers"])
|
|
}
|
|
if len(servers) != 1 || servers[0].(map[string]any)["name"] != "fetch" {
|
|
t.Fatalf("session/resume.mcpServers: got %v, want one entry named fetch", servers)
|
|
}
|
|
}
|