test(attachments): pin the CLI capability header and the rotation premise

Two review nits from #6119.

The CLI's setHeaders call is the only place Phase 1 is switched on, and
nothing observed it: the server tests prove a request carrying
`X-Client-Capabilities: stable_attachment_urls` gets stable paths, but a
typo in the CLI token — or dropping the header — would have left every
test green while the CLI silently went back to ~800-char signed URLs.
Assert the header verbatim across GET, GET-with-headers, POST, and the
no-token client. Verified the assertion is load-bearing by mutating the
constant to `stable_attachment_url`: the test fails.

TestAttachmentToResponse_SignedModeRotatesButStableDoesNot only checked
the "stable does not" half, so its name overstated coverage. Add the
rotation half. It drives the signer with two explicit expiries rather
than calling attachmentToResponse twice: that function mints its expiry
from time.Now() at second granularity, so consecutive calls usually land
in the same second and asserting on them directly would be a flaky test
of a real property. Also pins that only the query rotates, and that a
fixed expiry re-signs deterministically — without which the rotation
assertion would prove nothing about the clock.

MUL-5372

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

server/internal/cli/client_test.go

@@ -427,3 +427,101 @@ func TestNormalizeGOOS(t *testing.T) {
 		}
 	}
 }
+
+// TestSetHeaders_AdvertisesStableAttachmentURLs is the only test that proves
+// Phase 1 of MUL-5372 is actually switched on.
+//
+// The server-side tests verify that a request carrying
+// `X-Client-Capabilities: stable_attachment_urls` gets stable attachment paths,
+// but nothing there observes what the CLI sends. Without this assertion a typo
+// in the token, or dropping the header from setHeaders entirely, would leave
+// every other test green while the CLI silently went back to receiving ~800-char
+// signed URLs on every attachment of every list read.
+//
+// The exact string is load-bearing: the server matches the token literally
+// (handler.requestHasClientCapability), so it is asserted verbatim rather than
+// through the constant.
+func TestSetHeaders_AdvertisesStableAttachmentURLs(t *testing.T) {
+	const wantCapability = "stable_attachment_urls"
+
+	// Every verb goes through setHeaders, and an agent's read path is not only
+	// GET (comment add posts, attachment upload multiparts). Cover the shapes
+	// that actually carry attachment payloads back.
+	t.Run("GET", func(t *testing.T) {
+		var got string
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			got = r.Header.Get("X-Client-Capabilities")
+			w.WriteHeader(http.StatusOK)
+			_, _ = w.Write([]byte(`{}`))
+		}))
+		defer srv.Close()
+
+		client := NewAPIClient(srv.URL, "ws-1", "test-token")
+		var out map[string]any
+		if err := client.GetJSON(context.Background(), "/api/issues/x/comments", &out); err != nil {
+			t.Fatalf("GetJSON: %v", err)
+		}
+		if got != wantCapability {
+			t.Errorf("X-Client-Capabilities = %q, want %q — Phase 1 is off unless the CLI advertises this", got, wantCapability)
+		}
+	})
+
+	t.Run("GET with headers", func(t *testing.T) {
+		var got string
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			got = r.Header.Get("X-Client-Capabilities")
+			w.WriteHeader(http.StatusOK)
+			_, _ = w.Write([]byte(`[]`))
+		}))
+		defer srv.Close()
+
+		client := NewAPIClient(srv.URL, "ws-1", "test-token")
+		var out []map[string]any
+		if _, err := client.GetJSONWithHeaders(context.Background(), "/api/issues/x/comments", &out); err != nil {
+			t.Fatalf("GetJSONWithHeaders: %v", err)
+		}
+		if got != wantCapability {
+			t.Errorf("X-Client-Capabilities = %q, want %q", got, wantCapability)
+		}
+	})
+
+	t.Run("POST", func(t *testing.T) {
+		var got string
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			got = r.Header.Get("X-Client-Capabilities")
+			w.WriteHeader(http.StatusOK)
+			_, _ = w.Write([]byte(`{}`))
+		}))
+		defer srv.Close()
+
+		client := NewAPIClient(srv.URL, "ws-1", "test-token")
+		var out map[string]any
+		if err := client.PostJSON(context.Background(), "/api/issues/x/comments", map[string]string{"content": "hi"}, &out); err != nil {
+			t.Fatalf("PostJSON: %v", err)
+		}
+		if got != wantCapability {
+			t.Errorf("X-Client-Capabilities = %q, want %q", got, wantCapability)
+		}
+	})
+
+	// An unauthenticated client (no token configured yet) must still advertise:
+	// the capability describes what the binary can parse, not who it is.
+	t.Run("without a token", func(t *testing.T) {
+		var got string
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			got = r.Header.Get("X-Client-Capabilities")
+			w.WriteHeader(http.StatusOK)
+			_, _ = w.Write([]byte(`{}`))
+		}))
+		defer srv.Close()
+
+		client := NewAPIClient(srv.URL, "", "")
+		var out map[string]any
+		if err := client.GetJSON(context.Background(), "/api/health", &out); err != nil {
+			t.Fatalf("GetJSON: %v", err)
+		}
+		if got != wantCapability {
+			t.Errorf("X-Client-Capabilities = %q, want %q", got, wantCapability)
+		}
+	})
+}

server/internal/handler/attachment_url_mode_test.go

@@ -7,6 +7,7 @@ import (
 	"net/http/httptest"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/go-chi/chi/v5"
 	db "github.com/multica-ai/multica/server/pkg/db/generated"
@@ -124,9 +125,10 @@ func TestAttachmentToResponse_StableModeDropsSignature(t *testing.T) {
 	}
 }
 
-// TestAttachmentToResponse_SignedModeStillRotates documents why this change
-// exists at all: the signed value is re-minted per request, so identical
-// content yields different bytes and defeats content-keyed caching.
+// TestAttachmentToResponse_SignedModeRotatesButStableDoesNot documents why this
+// change exists at all: the signed value is a function of an expiry the server
+// re-derives per request, so identical content yields different bytes on every
+// read and defeats content-keyed caching. Stable mode has no such term.
 func TestAttachmentToResponse_SignedModeRotatesButStableDoesNot(t *testing.T) {
 	withCloudFrontSigner(t)
 
@@ -141,6 +143,31 @@ func TestAttachmentToResponse_SignedModeRotatesButStableDoesNot(t *testing.T) {
 	if strings.Contains(stableA.DownloadURL, "Policy=") || strings.Contains(stableA.DownloadURL, "Signature=") {
 		t.Errorf("stable download_url leaked signature params: %q", stableA.DownloadURL)
 	}
+
+	// The rotation half. attachmentToResponse mints its expiry from time.Now()
+	// at second granularity, so calling it twice in a row would usually land in
+	// the same second and produce identical bytes — asserting on that directly
+	// would be a flaky test of a real property. Drive the signer with two
+	// explicit expiries instead: it proves the URL varies with the expiry term,
+	// which is exactly what makes the response unstable as the clock advances.
+	signed := testHandler.attachmentToResponse(att, attachmentURLModeSigned)
+	if !strings.Contains(signed.DownloadURL, "Policy=") {
+		t.Fatalf("signed mode should embed a policy, got %q", signed.DownloadURL)
+	}
+	base := time.Now()
+	first := testHandler.CFSigner.SignedURL(att.Url, base.Add(30*time.Minute))
+	second := testHandler.CFSigner.SignedURL(att.Url, base.Add(30*time.Minute+time.Second))
+	if first == second {
+		t.Errorf("signed URL did not change with the expiry; rotation is the premise of this whole change")
+	}
+	if strings.Split(first, "?")[0] != strings.Split(second, "?")[0] {
+		t.Errorf("only the query should rotate, the resource path must be stable: %q vs %q", first, second)
+	}
+	// And re-signing with the SAME expiry must reproduce the same bytes —
+	// otherwise the rotation above would prove nothing about the clock.
+	if again := testHandler.CFSigner.SignedURL(att.Url, base.Add(30*time.Minute)); again != first {
+		t.Errorf("signing is not deterministic for a fixed expiry: %q vs %q", again, first)
+	}
 }
 
 // TestAttachmentToResponse_StableModeIsNoOpWithoutSigner pins the presign /