Files
multica/server/internal/daemon/client_test.go
Jiayuan Zhang f13969b996 refactor(chat): generate quick actions server-side via the LLM layer (MUL-5573) (#6214)
* refactor(chat): generate quick actions server-side via the LLM layer (MUL-5573)

Follow-up suggestions were produced by a second, full provider CLI invocation
per chat turn: the daemon resumed the just-finished session and ran a
suggestion-only pass. That pass inherited the main turn's exec options, so its
20s budget had to cover process spawn, every MCP handshake, session replay, and
model reasoning at the agent's own thinking level — typically 8-15s of visible
skeleton, and every turn paid two provider cold starts.

Generate them here instead, through the same pkg/llm layer that backs chat
auto-titling. Suggestions need no tools, workdir, or agent identity — only the
tail of the conversation — so a bounded 8s call on the deployment's small model
replaces the whole resumed turn.

Quality changes that came with the move:

  - The prompt now states the frame explicitly ("you write FOR THE USER"). The
    old pass ran inside the agent's session and inherited the runtime brief's
    identity, which drifted suggestions toward agent-operations actions.
  - Previously-offered labels are replayed as ALREADY SUGGESTED. The old
    architecture had the opposite effect: on providers that append on resume,
    each pass saw its predecessor's JSON and anchored on it.
  - A failed generation broadcasts failed=true. Before, a timeout delivered an
    empty array — indistinguishable from "nothing worth suggesting", so every
    slow pass read as a quality problem.
  - The in-band footer is still stripped from replies but its actions are now
    discarded, so a pre-upgrade session is not pinned to the retired
    suggestions with the replacing pass suppressed.

The refresh path no longer enqueues an agent task: it validates the target and
calls the same generator, which also drops the not-resumable refusal — a session
whose runtime was rebound can now be refreshed. Client contract is unchanged
(chat:done pending flag, chat:quick_actions supplement); the only frontend
change is the pending window, resized from 30s to 12s to match the new budget.

Also removes the daemon's TMPDIR-after-cleanup hazard by construction: the old
pass started after runTask's defers had already deleted the temp dir it was
still pointed at.

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

* refactor(chat): drop the quick-actions opt-out setting (MUL-5573)

Suggestions are always on. The Settings → Chat toggle is removed along with
the whole per-turn opt-out path it fed: the persisted client preference, the
quick_actions_enabled send field, the quick_actions_disabled task stamp, and
the eligibility gate that read it.

The toggle predates server-side generation, when it could only hide chips a
provider pass had already paid for. Now that generation is a bounded call the
server decides on, an off switch buys nothing a user would miss, and it was
the last piece of UI implying the feature might be unavailable.

agent_task_queue.quick_actions_disabled is no longer written (dropped from
CreateChatTask's INSERT; the column keeps its false default). Left in place
alongside regenerate_quick_actions_for for a later drop migration — removing
columns an already-running binary still inserts would break mid-deploy.

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

* fix(chat): address quick-actions review findings (MUL-5573)

Four defects from review of the server-side generation change.

1. Automatic failures were reported as refresh failures. The generator
   broadcast failed=true on any LLM error, but the client turns every
   failed=true into a "couldn't refresh" toast — so an automatic timeout
   popped a toast for an action the user never took. This also contradicted
   ChatQuickActionsPayload.Failed, which documents false for the automatic
   pass. The caller now passes its origin; only an explicit refresh reports.

2. Generation context was not bound to the target turn. The pass re-read the
   session's newest messages while always writing to the task it was handed,
   so a turn landing between the completion callback and the detached read
   supplied the context for a reply it did not belong to. Worse, a user
   typing a follow-up in the second after a reply left the window ending on
   a user row, which the old code treated as "nothing to build on" — that
   turn silently never got pills. The window is now anchored on the target
   assistant message and queried strictly before it.

3. No concurrency or idempotency bound on generation. Refresh stopped
   creating a task, so the busy check could not see a pass already running:
   two refreshes both returned 202, spent two upstream calls, and raced to
   write one row. Nothing bounded generation process-wide either. Adds a
   per-session in-flight guard (refresh now 409s on a duplicate) and a
   process-wide ceiling; a shed pass still resolves the client placeholder
   so no skeleton hangs on work that never started.

4. A new daemon could not safely talk to an older server. The refresh task
   discriminator was deleted, so a regenerate task from such a server fell
   through to the ordinary chat path: no user message, but the agent would
   answer anyway and the server would persist it as a real reply. The field
   is restored as a refusal marker only — the task completes empty, which is
   the shape the retired pass produced and which that server writes no row
   for. Not a restored execution path.

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

---------

Co-authored-by: Lambda <lambda@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-07-31 15:59:50 +08:00

473 lines
15 KiB
Go

package daemon
import (
"context"
"encoding/json"
"errors"
"net/http"
"net/http/httptest"
"runtime"
"strings"
"sync/atomic"
"testing"
"time"
"github.com/multica-ai/multica/server/pkg/protocol"
)
func TestClient_IdentityHeaders_PostJSON(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if got := r.Header.Get("X-Client-Platform"); got != "daemon" {
t.Errorf("expected X-Client-Platform daemon, got %q", got)
}
if got := r.Header.Get("X-Client-Version"); got != "9.9.9" {
t.Errorf("expected X-Client-Version 9.9.9, got %q", got)
}
if got := r.Header.Get("X-Client-OS"); got != normalizeGOOS(runtime.GOOS) {
t.Errorf("expected X-Client-OS %q, got %q", normalizeGOOS(runtime.GOOS), got)
}
if got := r.Header.Get("Authorization"); got != "Bearer tok" {
t.Errorf("expected Authorization Bearer tok, got %q", got)
}
capabilities := make(map[string]bool)
for _, capability := range strings.Split(r.Header.Get("X-Client-Capabilities"), ",") {
capabilities[strings.TrimSpace(capability)] = true
}
for _, want := range []string{
protocol.DaemonCapabilitySkillBundlesV1,
protocol.DaemonCapabilityCoalescedCommentsV1,
} {
if !capabilities[want] {
t.Errorf("X-Client-Capabilities missing %q: %v", want, capabilities)
}
}
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(map[string]string{"ok": "1"})
}))
defer srv.Close()
c := NewClient(srv.URL)
c.SetToken("tok")
c.SetVersion("9.9.9")
if err := c.postJSON(context.Background(), "/api/daemon/test", map[string]any{}, nil); err != nil {
t.Fatalf("postJSON: %v", err)
}
}
func TestClient_IdentityHeaders_GetJSON(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if got := r.Header.Get("X-Client-Platform"); got != "daemon" {
t.Errorf("expected X-Client-Platform daemon, got %q", got)
}
if got := r.Header.Get("X-Client-Version"); got != "1.2.3" {
t.Errorf("expected X-Client-Version 1.2.3, got %q", got)
}
if got := r.Header.Get("X-Client-OS"); got == "" {
t.Errorf("expected X-Client-OS to be set")
}
w.Header().Set("Content-Type", "application/json")
w.Write([]byte(`{}`))
}))
defer srv.Close()
c := NewClient(srv.URL)
c.SetToken("tok")
c.SetVersion("1.2.3")
var out map[string]any
if err := c.getJSON(context.Background(), "/api/daemon/test", &out); err != nil {
t.Fatalf("getJSON: %v", err)
}
}
func TestClient_VersionOmittedWhenUnset(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if got := r.Header.Get("X-Client-Platform"); got != "daemon" {
t.Errorf("expected X-Client-Platform daemon, got %q", got)
}
// SetVersion not called → header must be omitted (not "").
if vals := r.Header.Values("X-Client-Version"); len(vals) != 0 {
t.Errorf("expected X-Client-Version absent, got %v", vals)
}
w.WriteHeader(http.StatusNoContent)
}))
defer srv.Close()
c := NewClient(srv.URL)
if err := c.postJSON(context.Background(), "/api/daemon/test", nil, nil); err != nil {
t.Fatalf("postJSON: %v", err)
}
}
func TestClient_ListWorkspacesUsesDaemonEndpointAndETag(t *testing.T) {
var calls atomic.Int32
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/api/daemon/workspaces" {
t.Errorf("path = %q, want /api/daemon/workspaces", r.URL.Path)
http.NotFound(w, r)
return
}
call := calls.Add(1)
if call == 1 {
if got := r.Header.Get("If-None-Match"); got != "" {
t.Errorf("first If-None-Match = %q, want empty", got)
}
w.Header().Set("ETag", `W/"workspace-v1"`)
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(`[{"id":"ws-1","name":"One"}]`))
return
}
if got := r.Header.Get("If-None-Match"); got != `W/"workspace-v1"` {
t.Errorf("If-None-Match = %q, want cached ETag", got)
}
w.WriteHeader(http.StatusNotModified)
}))
defer srv.Close()
c := NewClient(srv.URL)
first, err := c.ListWorkspaces(context.Background())
if err != nil {
t.Fatalf("first ListWorkspaces: %v", err)
}
second, err := c.ListWorkspaces(context.Background())
if err != nil {
t.Fatalf("second ListWorkspaces: %v", err)
}
if len(first) != 1 || len(second) != 1 || second[0] != first[0] {
t.Fatalf("cached workspaces mismatch: first=%+v second=%+v", first, second)
}
}
func TestClient_ListWorkspacesFallsBackToLegacyEndpointOnce(t *testing.T) {
var daemonCalls atomic.Int32
var legacyCalls atomic.Int32
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/api/daemon/workspaces":
daemonCalls.Add(1)
http.NotFound(w, r)
case "/api/workspaces":
legacyCalls.Add(1)
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(`[{"id":"ws-legacy","name":"Legacy"}]`))
default:
http.NotFound(w, r)
}
}))
defer srv.Close()
c := NewClient(srv.URL)
for i := 0; i < 2; i++ {
workspaces, err := c.ListWorkspaces(context.Background())
if err != nil {
t.Fatalf("ListWorkspaces call %d: %v", i+1, err)
}
if len(workspaces) != 1 || workspaces[0].ID != "ws-legacy" {
t.Fatalf("workspaces = %+v, want legacy response", workspaces)
}
}
if got := daemonCalls.Load(); got != 1 {
t.Fatalf("daemon endpoint calls = %d, want 1", got)
}
if got := legacyCalls.Load(); got != 2 {
t.Fatalf("legacy endpoint calls = %d, want 2", got)
}
}
// noSleepRetry replaces retrySleep with an immediate no-op so tests don't
// actually wait the 4s/8s/16s/... backoffs. Returns a restore func.
func noSleepRetry(t *testing.T) func() {
t.Helper()
prev := retrySleep
retrySleep = func(ctx context.Context, _ time.Duration) error {
if err := ctx.Err(); err != nil {
return err
}
return nil
}
return func() { retrySleep = prev }
}
func TestIsTransientError(t *testing.T) {
cases := []struct {
name string
err error
want bool
}{
{"nil is not transient", nil, false},
{"5xx is transient", &requestError{StatusCode: http.StatusBadGateway}, true},
{"503 is transient", &requestError{StatusCode: http.StatusServiceUnavailable}, true},
{"408 is transient", &requestError{StatusCode: http.StatusRequestTimeout}, true},
{"429 is transient", &requestError{StatusCode: http.StatusTooManyRequests}, true},
{"400 is permanent", &requestError{StatusCode: http.StatusBadRequest}, false},
{"401 is permanent", &requestError{StatusCode: http.StatusUnauthorized}, false},
{"404 is permanent", &requestError{StatusCode: http.StatusNotFound}, false},
{"transport-level error is transient", errors.New("connection reset by peer"), true},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
if got := isTransientError(tc.err); got != tc.want {
t.Fatalf("isTransientError(%v) = %v, want %v", tc.err, got, tc.want)
}
})
}
}
func TestIsIssueGCBatchUnsupported(t *testing.T) {
tests := []struct {
name string
err error
want bool
}{
{
name: "old server unmatched route",
err: &requestError{StatusCode: http.StatusNotFound, Body: "404 page not found"},
want: true,
},
{
name: "workspace access denied",
err: &requestError{StatusCode: http.StatusNotFound, Body: `{"error":"not found"}`},
want: false,
},
{
name: "transient server error",
err: &requestError{StatusCode: http.StatusInternalServerError, Body: "failure"},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := isIssueGCBatchUnsupported(tt.err); got != tt.want {
t.Fatalf("isIssueGCBatchUnsupported() = %v, want %v", got, tt.want)
}
})
}
}
func TestPostJSONWithRetry_TransientThenSuccess(t *testing.T) {
defer noSleepRetry(t)()
var calls atomic.Int32
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
n := calls.Add(1)
if n < 3 {
w.WriteHeader(http.StatusBadGateway)
return
}
w.WriteHeader(http.StatusOK)
}))
defer srv.Close()
c := NewClient(srv.URL)
schedule := []time.Duration{time.Nanosecond, time.Nanosecond, time.Nanosecond}
if err := c.postJSONWithRetry(context.Background(), "/x", map[string]any{}, nil, schedule); err != nil {
t.Fatalf("postJSONWithRetry: %v", err)
}
if got := calls.Load(); got != 3 {
t.Fatalf("expected 3 attempts (2 transient + 1 success), got %d", got)
}
}
// TestFailTask_RetriesOnTransient5xxThenSucceeds pins the callback half of
// MUL-5305 Must-fix 1: FailTask's terminal transaction is now the sole
// persistence point for the withheld session and continuity-gap flag, so if the
// server returns a transient 5xx (the terminal tx rolled back), the daemon MUST
// retry until it lands — a 400 would make it bail immediately
// (TestPostJSONWithRetry_PermanentBailsImmediately) and drop the gap forever.
func TestFailTask_RetriesOnTransient5xxThenSucceeds(t *testing.T) {
defer noSleepRetry(t)()
var calls atomic.Int32
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
if calls.Add(1) < 3 {
w.WriteHeader(http.StatusInternalServerError)
return
}
w.WriteHeader(http.StatusOK)
}))
defer srv.Close()
c := NewClient(srv.URL)
if err := c.FailTask(context.Background(), "task-1", "boom", "", "", "timeout", true, ""); err != nil {
t.Fatalf("FailTask: %v", err)
}
if got := calls.Load(); got != 3 {
t.Fatalf("expected 3 attempts (2 transient 5xx + 1 success), got %d", got)
}
}
func TestPostJSONWithRetry_TransientExhausts(t *testing.T) {
defer noSleepRetry(t)()
var calls atomic.Int32
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
calls.Add(1)
w.WriteHeader(http.StatusBadGateway)
}))
defer srv.Close()
c := NewClient(srv.URL)
schedule := []time.Duration{time.Nanosecond, time.Nanosecond}
err := c.postJSONWithRetry(context.Background(), "/x", map[string]any{}, nil, schedule)
if err == nil {
t.Fatal("expected error after schedule exhausted, got nil")
}
if !isTransientError(err) {
t.Fatalf("expected transient error, got %v", err)
}
if got := calls.Load(); got != int32(len(schedule)+1) {
t.Fatalf("expected %d attempts (initial + %d retries), got %d", len(schedule)+1, len(schedule), got)
}
}
func TestPostJSONWithRetry_PermanentBailsImmediately(t *testing.T) {
defer noSleepRetry(t)()
var calls atomic.Int32
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
calls.Add(1)
w.WriteHeader(http.StatusBadRequest)
}))
defer srv.Close()
c := NewClient(srv.URL)
schedule := []time.Duration{time.Nanosecond, time.Nanosecond, time.Nanosecond}
err := c.postJSONWithRetry(context.Background(), "/x", map[string]any{}, nil, schedule)
if err == nil {
t.Fatal("expected error, got nil")
}
if got := calls.Load(); got != 1 {
t.Fatalf("expected exactly 1 attempt on permanent error, got %d", got)
}
}
func TestPostJSONWithRetry_CtxCancelStopsRetries(t *testing.T) {
// Use the real sleeper here so we can observe a cancel preempting it.
var calls atomic.Int32
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
calls.Add(1)
w.WriteHeader(http.StatusBadGateway)
}))
defer srv.Close()
ctx, cancel := context.WithCancel(context.Background())
go func() {
// Cancel quickly so the first sleep is aborted long before its 1s.
time.Sleep(50 * time.Millisecond)
cancel()
}()
c := NewClient(srv.URL)
schedule := []time.Duration{time.Second, time.Second, time.Second}
start := time.Now()
err := c.postJSONWithRetry(ctx, "/x", map[string]any{}, nil, schedule)
elapsed := time.Since(start)
if err == nil {
t.Fatal("expected error after ctx cancel, got nil")
}
if elapsed > 750*time.Millisecond {
t.Fatalf("expected ctx cancel to short-circuit retry, took %s", elapsed)
}
if got := calls.Load(); got != 1 {
t.Fatalf("expected exactly 1 attempt before cancel, got %d", got)
}
}
func TestDefaultTerminalRetrySchedule_MatchesAgreedPlan(t *testing.T) {
// MUL-2780 settled on a 5-step exponential backoff (4s, 8s, 16s, 32s, 64s).
// Pin it so a future "tidy this up" refactor can't silently flatten or
// shorten the recovery window without explicit discussion.
want := []time.Duration{4 * time.Second, 8 * time.Second, 16 * time.Second, 32 * time.Second, 64 * time.Second}
if len(defaultTerminalRetrySchedule) != len(want) {
t.Fatalf("schedule length: got %d, want %d", len(defaultTerminalRetrySchedule), len(want))
}
for i, d := range want {
if defaultTerminalRetrySchedule[i] != d {
t.Errorf("schedule[%d]: got %s, want %s", i, defaultTerminalRetrySchedule[i], d)
}
}
}
func TestNormalizeGOOS(t *testing.T) {
cases := map[string]string{
"darwin": "macos",
"windows": "windows",
"linux": "linux",
"freebsd": "freebsd",
}
for in, want := range cases {
if got := normalizeGOOS(in); got != want {
t.Errorf("normalizeGOOS(%q) = %q, want %q", in, got, want)
}
}
}
// TestTerminalReportsCarryRetiredSessionID pins the daemon half of the
// retire-session contract (GH #6066). Before it, a terminal report could only
// say "here is a session" or say nothing — and saying nothing was how a
// recovered turn silently left the poisoned id selectable. The completed path
// matters most: that is exactly the case where a fresh-session retry SUCCEEDED
// and the abandoned transcript would otherwise survive on an older row.
func TestTerminalReportsCarryRetiredSessionID(t *testing.T) {
for _, tc := range []struct {
name string
endpoint string
call func(*Client) error
}{
{
name: "complete",
endpoint: "/api/daemon/tasks/task-1/complete",
call: func(c *Client) error {
return c.CompleteTask(context.Background(), "task-1", "done", "", "", "/tmp/wd", false, "POISONED-S")
},
},
{
name: "fail",
endpoint: "/api/daemon/tasks/task-1/fail",
call: func(c *Client) error {
return c.FailTask(context.Background(), "task-1", "boom", "", "/tmp/wd", "api_invalid_request", false, "POISONED-S")
},
},
} {
t.Run(tc.name, func(t *testing.T) {
var body map[string]any
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != tc.endpoint {
t.Errorf("unexpected path %q", r.URL.Path)
}
_ = json.NewDecoder(r.Body).Decode(&body)
w.WriteHeader(http.StatusOK)
}))
defer srv.Close()
if err := tc.call(NewClient(srv.URL)); err != nil {
t.Fatalf("terminal report: %v", err)
}
if got, _ := body["retired_session_id"].(string); got != "POISONED-S" {
t.Fatalf("retired_session_id = %v, want POISONED-S (body: %v)", body["retired_session_id"], body)
}
})
}
}
// TestTerminalReportsOmitEmptyRetiredSessionID keeps the common case off the
// wire: nearly every run retires nothing, and an empty field would be
// indistinguishable from "retire the empty session".
func TestTerminalReportsOmitEmptyRetiredSessionID(t *testing.T) {
var body map[string]any
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
_ = json.NewDecoder(r.Body).Decode(&body)
w.WriteHeader(http.StatusOK)
}))
defer srv.Close()
if err := NewClient(srv.URL).CompleteTask(context.Background(), "task-1", "done", "", "sess-1", "/tmp/wd", false, ""); err != nil {
t.Fatalf("CompleteTask: %v", err)
}
if _, present := body["retired_session_id"]; present {
t.Fatalf("retired_session_id must be omitted when nothing was retired, got %v", body)
}
}