mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-09 18:06:45 +08:00
* fix(daemon): enforce runtime ownership on daemon routes (MUL-8154) /api/daemon/* accepts any workspace member's PAT/JWT, and authorization only checked workspace membership. Any member could claim another member's tasks (receiving the agent's custom_env, MCP config and an owner-scoped task token), write task results/messages, and take a runtime over by re-registering its daemon_id. - Runtime-level check on every runtime/task daemon route, HTTP heartbeat, deregister, batch claim (and so the WS claim RPC) and the WS handshake: user credentials must own the runtime (NULL owner fails closed); daemon tokens must match a non-empty runtime daemon_id. - Register: pre-check that every existing row on the daemon_id belongs to the caller, plus a required_owner_id fence in both upserts (incl. the failed-profile path) so a concurrent registration cannot win the row. Daemon-token registration keeps the existing owner. - Legacy merge: only for user-credential registrations, and only when the caller owns both rows, verified on the FOR UPDATE-locked rows. - Tasks without a runtime are not writable through daemon routes; status stays readable. - GetTaskStatus / WS lease queries carry runtime owner/daemon so the hot paths keep one round trip. Co-authored-by: multica-agent <[email protected]> * test(daemon): authenticate fixtures as the runtime owner (MUL-8154) Most handler tests drove daemon routes with a daemon token whose daemon_id did not match the fixture runtime, relying on the workspace-wide token scope this fix removes. They now authenticate the way real daemons do, with the runtime owner's user credential (newOwnerDaemonRequest). Tests about daemon token scoping keep mdt_ and get fixtures with a matching daemon_id; the ownerless-runtime tests also assert that a user credential is refused. Co-authored-by: multica-agent <[email protected]> * fix(daemon): serialize registration per daemon and keep Qoder WS claims working (MUL-8154) Review of #9423: - P1: the owner pre-check and the per-provider/profile upserts hit different conflict keys, so two members registering different providers of the same daemon could both pass the pre-check and both insert, leaving one daemon with two owners (and a daemon token for either runtime reaching the other's). Registration now runs the pre-check and every runtime write in one transaction under a pg_advisory_xact_lock on (workspace_id, daemon_id), on a single connection so queued registrations cannot starve the holder. Legacy merges, analytics and the register event run after commit; failed-profile records use savepoints so they stay best-effort. - P2: the Qoder revocation check read the Authorization header, which the WS RPC bridge's in-process request does not carry, so valid Qoder WS claims silently returned nothing. DaemonAuth now records the mdt_ token hash in the request context, the WS identity keeps it, the RPC bridge re-attaches it, and the check validates that hash on every call (revocation still applies to an open connection). - Regression tests for cross-provider and provider+profile concurrent registration, and for valid/revoked Qoder credentials over WS RPC. Co-authored-by: multica-agent <[email protected]> * fix(daemon): bind daemon tokens to the runtime owner they were issued for (MUL-8154) Second review of #9423: a daemon_id can be re-registered by another member after its runtimes are deleted, and a daemon token was bound only to (workspace, daemon_id), so the previous owner's Remote MCP token reached the new owner's runtime (heartbeat, claim, owner-scoped task token). - daemon_token.owner_id (migration 588). Remote MCP tokens are bound to the locked runtime owner inside FinalizeTaskClaim; live tokens are backfilled only where their daemon has exactly one owner. - DaemonAuth carries the owner binding (DB and cache; cache key bumped to v2 so pre-upgrade entries are never read), the WS identity keeps it and the RPC bridge replays it. - Daemon-token authorization now also requires token owner == runtime owner. Qoder-managed tokens stay unbound (governed by cloud_agent_connection), and user credentials can no longer register the reserved qoder-managed-* ids. - Tests: the reuse lifecycle over HTTP, a new WS handshake and an already open WS connection; the reserved daemon_id; the concurrency barrier now hooks the registration transaction (TxStarter) so the interleaving is real. Co-authored-by: multica-agent <[email protected]> * fix(daemon): owner-bind daemon-token registration; drop unsafe token backfill (MUL-8154) Third review of #9423: - Migration 588 no longer backfills daemon_token.owner_id from the daemon's current owner: that owner may be a later re-registrant, which re-bound the previous owner's live token to them. Existing non-Qoder tokens stay NULL and are refused (24h Remote MCP credentials, re-issued on the next claim). - owner_id is a plain UUID column, no foreign key / cascade (AGENTS.md). - Register via a non-Qoder daemon token now uses the token's owner for the in-transaction pre-check and the SQL owner fence, and refuses unbound tokens. Qoder keeps its connection-based check; daemon tokens still never trigger legacy merges. - Tests: an old owner's bound token and an unbound legacy token can neither register over nor heartbeat the replacement owner's runtime. Co-authored-by: multica-agent <[email protected]> * fix(daemon): grant the Qoder registration exemption only with a live connection (MUL-8154) Fourth review of #9423: register skipped the token owner binding for any qoder-managed-* daemon token, but checked the Qoder connection only when the body declared a qoder_cloud runtime. A pre-upgrade Remote MCP token under that prefix (no owner, no connection) could register type=claude and rewrite another member's runtime. Register now verifies the token against a live cloud_agent_connection before anything else whenever the token is in the Qoder namespace, regardless of the declared runtimes, and restricts that namespace to the bridge's own shape: a built-in qoder_cloud runtime, no custom profiles, no failed_profiles. Test covers built-in, custom-profile, failed-profiles-only and qoder_cloud bodies with an unconnected token, plus the connected token's allowed and refused shapes. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: multica-eve <[email protected]> Co-authored-by: multica-agent <[email protected]>
346 lines
13 KiB
Go
346 lines
13 KiB
Go
package handler
|
|
|
|
import (
|
|
"bytes"
|
|
"context"
|
|
"encoding/json"
|
|
"io"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"net/http/httptrace"
|
|
"strings"
|
|
"sync"
|
|
"sync/atomic"
|
|
"testing"
|
|
"time"
|
|
|
|
chimw "github.com/go-chi/chi/v5/middleware"
|
|
"github.com/gorilla/websocket"
|
|
"github.com/jackc/pgx/v5"
|
|
"github.com/multica-ai/multica/server/internal/daemonws"
|
|
"github.com/multica-ai/multica/server/internal/events"
|
|
"github.com/multica-ai/multica/server/internal/realtime"
|
|
"github.com/multica-ai/multica/server/internal/service"
|
|
db "github.com/multica-ai/multica/server/pkg/db/generated"
|
|
"github.com/multica-ai/multica/server/pkg/protocol"
|
|
"github.com/redis/go-redis/v9"
|
|
)
|
|
|
|
// Exercise the production notifier/relay. XADD honors the actual context the
|
|
// relay supplies, including its independent two-second timeout.
|
|
type stalledWakeupRedis struct {
|
|
redis.UniversalClient
|
|
calls atomic.Int64
|
|
}
|
|
|
|
func (r *stalledWakeupRedis) XAdd(ctx context.Context, _ *redis.XAddArgs) *redis.StringCmd {
|
|
r.calls.Add(1)
|
|
<-ctx.Done()
|
|
cmd := redis.NewStringCmd(ctx)
|
|
cmd.SetErr(ctx.Err())
|
|
return cmd
|
|
}
|
|
|
|
type stalledClaimCache struct {
|
|
redis.UniversalClient
|
|
calls atomic.Int64
|
|
}
|
|
|
|
func (r *stalledClaimCache) ZRem(ctx context.Context, _ string, _ ...any) *redis.IntCmd {
|
|
r.calls.Add(1)
|
|
<-ctx.Done()
|
|
cmd := redis.NewIntCmd(ctx)
|
|
cmd.SetErr(ctx.Err())
|
|
return cmd
|
|
}
|
|
|
|
func (r *stalledClaimCache) Pipeline() redis.Pipeliner { return &stalledClaimPipeline{cache: r} }
|
|
|
|
type stalledClaimPipeline struct {
|
|
redis.Pipeliner
|
|
cache *stalledClaimCache
|
|
}
|
|
|
|
func (p *stalledClaimPipeline) Incr(ctx context.Context, _ string) *redis.IntCmd {
|
|
return redis.NewIntCmd(ctx)
|
|
}
|
|
|
|
func (p *stalledClaimPipeline) Expire(ctx context.Context, _ string, _ time.Duration) *redis.BoolCmd {
|
|
return redis.NewBoolCmd(ctx)
|
|
}
|
|
|
|
func (p *stalledClaimPipeline) Exec(ctx context.Context) ([]redis.Cmder, error) {
|
|
p.cache.calls.Add(1)
|
|
<-ctx.Done()
|
|
return nil, ctx.Err()
|
|
}
|
|
|
|
type claimRecoveryCacheDB struct {
|
|
db.DBTX
|
|
tasks *service.TaskService
|
|
cache *stalledClaimCache
|
|
}
|
|
|
|
func (d *claimRecoveryCacheDB) QueryRow(ctx context.Context, query string, args ...any) pgx.Row {
|
|
// Only stall recovery caches, not the preceding ordinary claim operations.
|
|
if ctx.Err() == nil && strings.Contains(query, "-- name: RequeueAgentTaskAfterClaimFailure :one") {
|
|
d.tasks.EmptyClaim = service.NewEmptyClaimCache(d.cache)
|
|
d.tasks.ReclaimCheck = service.NewReclaimCheckCache(d.cache)
|
|
}
|
|
return d.DBTX.QueryRow(ctx, query, args...)
|
|
}
|
|
|
|
func TestClaimTasksByRuntime_HTTPFollowupDoesNotWaitForRecovery(t *testing.T) {
|
|
if testHandler == nil || testPool == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
ctx := context.Background()
|
|
runtimeID := createClaimReclaimRuntime(t, ctx, "HTTP follow-up runtime")
|
|
agentA, issueA := createClaimReclaimAgentAndIssue(t, ctx, runtimeID, "Delivered before recovery")
|
|
taskA := seedQueuedIssueTask(t, ctx, agentA, runtimeID, issueA)
|
|
agentB, issueB := createClaimReclaimAgentAndIssue(t, ctx, runtimeID, "Retried during recovery")
|
|
taskB := seedQueuedIssueTask(t, ctx, agentB, runtimeID, issueB)
|
|
dbfx.Exec(t, `UPDATE agent_task_queue SET priority = 100 WHERE id = $1`, taskA)
|
|
|
|
// Hold the first handler inside its post-response recovery, after the
|
|
// omitted task is queued. A follow-up claim must run before this is released.
|
|
bus := events.New()
|
|
eventEntered, eventRelease := make(chan struct{}), make(chan struct{})
|
|
releaseEvent := sync.OnceFunc(func() { close(eventRelease) })
|
|
bus.Subscribe(protocol.EventTaskQueued, func(events.Event) {
|
|
close(eventEntered)
|
|
<-eventRelease
|
|
})
|
|
h := *testHandler
|
|
h.TaskService = service.NewTaskService(testHandler.Queries, testPool, testHandler.TaskService.Hub, bus)
|
|
firstHandler := h
|
|
firstHandler.Queries = db.New(&slowBatchClaimDB{DBTX: testPool, failingAgent: parseUUID(agentB)})
|
|
var requests atomic.Int64
|
|
firstCompleted := make(chan struct{})
|
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
wrapped := chimw.NewWrapResponseWriter(chimw.NewWrapResponseWriter(w, r.ProtoMajor), r.ProtoMajor)
|
|
r = asRuntimeOwner(r)
|
|
if requests.Add(1) == 1 {
|
|
defer close(firstCompleted)
|
|
firstHandler.ClaimTasksByRuntime(wrapped, r)
|
|
return
|
|
}
|
|
h.ClaimTasksByRuntime(wrapped, r)
|
|
}))
|
|
defer server.Close()
|
|
defer releaseEvent() // Unblock the handler before server shutdown on failure.
|
|
client := &http.Client{
|
|
Timeout: protocol.DaemonBatchClaimRequestTimeout,
|
|
Transport: http.DefaultTransport.(*http.Transport).Clone(),
|
|
}
|
|
defer client.CloseIdleConnections()
|
|
body, err := json.Marshal(map[string]any{
|
|
"daemon_id": batchClaimTestDaemonID, "runtime_ids": []string{runtimeID}, "max_tasks": 2,
|
|
})
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
claim := func() (batchClaimResponse, bool) {
|
|
t.Helper()
|
|
var reused atomic.Bool
|
|
traceCtx := httptrace.WithClientTrace(ctx, &httptrace.ClientTrace{GotConn: func(info httptrace.GotConnInfo) {
|
|
reused.Store(info.Reused)
|
|
}})
|
|
req, err := http.NewRequestWithContext(traceCtx, http.MethodPost, server.URL, bytes.NewReader(body))
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
req.Header.Set("Content-Type", "application/json")
|
|
resp, err := client.Do(req)
|
|
if err != nil {
|
|
t.Fatalf("claim blocked by the preceding handler's recovery (connection reused=%t): %v", reused.Load(), err)
|
|
}
|
|
defer resp.Body.Close()
|
|
data, err := io.ReadAll(resp.Body) // Consume the full body so keep-alive is eligible.
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if resp.StatusCode != http.StatusOK {
|
|
t.Fatalf("claim status=%d: %s", resp.StatusCode, data)
|
|
}
|
|
var response batchClaimResponse
|
|
if err := json.Unmarshal(data, &response); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
return response, reused.Load()
|
|
}
|
|
first, _ := claim()
|
|
if len(first.Tasks) != 1 || first.Tasks[0].ID != taskA || first.Tasks[0].AuthToken == "" {
|
|
t.Fatalf("first claim lost the successful payload: %+v", first)
|
|
}
|
|
select {
|
|
case <-eventEntered:
|
|
case <-time.After(3 * time.Second):
|
|
t.Fatal("first claim did not reach recovery notification")
|
|
}
|
|
start := time.Now()
|
|
second, reused := claim()
|
|
elapsed := time.Since(start)
|
|
if len(second.Tasks) != 1 || second.Tasks[0].ID != taskB || second.Tasks[0].AuthToken == "" {
|
|
t.Fatalf("follow-up claim lost the recovered payload: %+v", second)
|
|
}
|
|
if reused {
|
|
t.Fatal("follow-up claim reused the connection held by recovery")
|
|
}
|
|
select {
|
|
case <-firstCompleted:
|
|
t.Fatal("recovery finished before the follow-up claim; regression was not exercised")
|
|
default:
|
|
}
|
|
// Only responses with omissions need a new connection. Complete batches
|
|
// must retain ordinary keep-alive and must not deliver either task twice.
|
|
third, reused := claim()
|
|
if len(third.Tasks) != 0 || !reused {
|
|
t.Fatalf("complete batch did not retain keep-alive or duplicated delivery: reused=%t tasks=%+v", reused, third.Tasks)
|
|
}
|
|
releaseEvent()
|
|
select {
|
|
case <-firstCompleted:
|
|
case <-time.After(3 * time.Second):
|
|
t.Fatal("first claim did not finish recovery")
|
|
}
|
|
t.Logf("follow-up delivered the recovered task in %s while the first handler remained blocked", elapsed.Round(time.Millisecond))
|
|
}
|
|
|
|
func TestClaimTasksByRuntime_ResponsePrecedesRecoveryNotifications(t *testing.T) {
|
|
if testHandler == nil || testPool == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
for _, transport := range []string{"http", "ws"} {
|
|
t.Run(transport, func(t *testing.T) {
|
|
ctx := context.Background()
|
|
runtimeID := createClaimReclaimRuntime(t, ctx, "Notification deadline runtime")
|
|
agentA, issueA := createClaimReclaimAgentAndIssue(t, ctx, runtimeID, "Successful payload")
|
|
taskA := seedQueuedIssueTask(t, ctx, agentA, runtimeID, issueA)
|
|
agentB, issueB := createClaimReclaimAgentAndIssue(t, ctx, runtimeID, "Failed payload")
|
|
taskB := seedQueuedIssueTask(t, ctx, agentB, runtimeID, issueB)
|
|
dbfx.Exec(t, `UPDATE agent_task_queue SET priority = 100 WHERE id = $1`, taskA)
|
|
h := *testHandler
|
|
h.Queries = db.New(&slowBatchClaimDB{DBTX: testPool, failingAgent: parseUUID(agentB)})
|
|
hub := daemonws.NewHub()
|
|
rdb := &stalledWakeupRedis{}
|
|
relay := realtime.NewShardedStreamRelay(testHandler.TaskService.Hub, rdb, rdb, realtime.DefaultShardedStreamRelayConfig())
|
|
bus := events.New()
|
|
eventEntered, eventRelease := make(chan struct{}), make(chan struct{})
|
|
releaseEvent := sync.OnceFunc(func() { close(eventRelease) })
|
|
bus.Subscribe(protocol.EventTaskQueued, func(events.Event) {
|
|
close(eventEntered)
|
|
<-eventRelease
|
|
})
|
|
h.TaskService = service.NewTaskService(testHandler.Queries, testPool, testHandler.TaskService.Hub, bus, daemonws.NewRelayNotifier(hub, relay))
|
|
cache := &stalledClaimCache{}
|
|
h.TaskService.Queries = db.New(&claimRecoveryCacheDB{DBTX: testPool, tasks: h.TaskService, cache: cache})
|
|
completed := make(chan struct{})
|
|
hub.SetRPCHandler(func(ctx context.Context, identity daemonws.ClientIdentity, method string, body json.RawMessage) (int, json.RawMessage, error) {
|
|
status, response, err := h.DaemonRPCHandler(ctx, identity, method, body)
|
|
daemonws.AfterRPCResponse(ctx, func() { close(completed) })
|
|
return status, response, err
|
|
})
|
|
identity := daemonws.ClientIdentity{
|
|
UserID: testUserID, // user-credential connection: no daemon id
|
|
WorkspaceID: testWorkspaceID, WorkspaceIDs: []string{testWorkspaceID},
|
|
RuntimeIDs: []string{runtimeID}, Capabilities: protocol.DaemonCapabilityClaimPollHintsV1,
|
|
}
|
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
if transport == "ws" {
|
|
hub.HandleWebSocket(w, r, identity)
|
|
return
|
|
}
|
|
defer close(completed)
|
|
// The production logger and metrics both wrap the HTTP writer.
|
|
wrapped := chimw.NewWrapResponseWriter(chimw.NewWrapResponseWriter(w, r.ProtoMajor), r.ProtoMajor)
|
|
h.ClaimTasksByRuntime(wrapped, asRuntimeOwner(r))
|
|
}))
|
|
defer server.Close()
|
|
defer releaseEvent() // Release a blocked event before server shutdown on failure.
|
|
body, err := json.Marshal(map[string]any{
|
|
"daemon_id": batchClaimTestDaemonID, "runtime_ids": []string{runtimeID}, "max_tasks": 2,
|
|
})
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
start := time.Now()
|
|
var response batchClaimResponse
|
|
var clientErr error
|
|
var conn *websocket.Conn
|
|
if transport == "http" {
|
|
client := &http.Client{Timeout: 5 * time.Second}
|
|
resp, err := client.Post(server.URL, "application/json", bytes.NewReader(body))
|
|
clientErr = err
|
|
if resp != nil {
|
|
clientErr = json.NewDecoder(resp.Body).Decode(&response)
|
|
resp.Body.Close()
|
|
}
|
|
} else {
|
|
conn, _, err = websocket.DefaultDialer.Dial("ws"+strings.TrimPrefix(server.URL, "http"), nil)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
defer conn.Close()
|
|
payload, err := json.Marshal(protocol.RPCRequestPayload{RequestID: "notifications", Method: "tasks.claim", TimeoutMs: 5000, Body: body})
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := conn.WriteJSON(protocol.Message{Type: protocol.EventDaemonRPCRequest, Payload: payload}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := conn.SetReadDeadline(start.Add(7 * time.Second)); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
var msg protocol.Message
|
|
clientErr = conn.ReadJSON(&msg)
|
|
if clientErr == nil {
|
|
if msg.Type != protocol.EventDaemonRPCResponse {
|
|
t.Fatalf("message before recovery notification = %q, want RPC response", msg.Type)
|
|
}
|
|
var rpc protocol.RPCResponsePayload
|
|
if err := json.Unmarshal(msg.Payload, &rpc); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
clientErr = json.Unmarshal(rpc.Body, &response)
|
|
}
|
|
}
|
|
elapsed := time.Since(start)
|
|
select {
|
|
case <-eventEntered:
|
|
case <-time.After(3 * time.Second):
|
|
releaseEvent()
|
|
t.Fatal("recovery did not publish its queued event")
|
|
}
|
|
releaseEvent()
|
|
select {
|
|
case <-completed:
|
|
case <-time.After(10 * time.Second):
|
|
t.Fatal("recovery did not finish in the request worker")
|
|
}
|
|
if clientErr != nil || elapsed >= 5*time.Second {
|
|
t.Fatalf("%s payload waited for recovery notifications: elapsed=%s error=%v", transport, elapsed, clientErr)
|
|
}
|
|
if len(response.Tasks) != 1 || response.Tasks[0].ID != taskA || response.Tasks[0].AuthToken == "" {
|
|
t.Fatalf("successful payload missing: %+v", response)
|
|
}
|
|
if cache.calls.Load() != 2 || rdb.calls.Load() != 1 {
|
|
t.Fatalf("recovery skipped side effects: cache=%d wakeup=%d", cache.calls.Load(), rdb.calls.Load())
|
|
}
|
|
for _, task := range []struct{ id, status string }{{taskA, "dispatched"}, {taskB, "queued"}} {
|
|
row, err := testHandler.Queries.GetAgentTask(ctx, parseUUID(task.id))
|
|
if err != nil || row.Status != task.status {
|
|
t.Fatalf("task %s status=%s error=%v, want %s", task.id, row.Status, err, task.status)
|
|
}
|
|
}
|
|
if conn != nil {
|
|
var wakeup protocol.Message
|
|
if err := conn.ReadJSON(&wakeup); err != nil || wakeup.Type != protocol.EventDaemonTaskAvailable {
|
|
t.Fatalf("local wakeup was lost: type=%s err=%v", wakeup.Type, err)
|
|
}
|
|
}
|
|
t.Logf("%s received complete payload in %s; slow caches, event and production relay completed afterward", transport, elapsed.Round(time.Millisecond))
|
|
})
|
|
}
|
|
}
|