From 155eadb1615c9f80daf1d7cb32f687a327371ca2 Mon Sep 17 00:00:00 2001 From: Delicious233 <101502465+DeliciousBuding@users.noreply.github.com> Date: Mon, 7 Sep 2026 06:28:42 +0800 Subject: [PATCH 1/5] test(dispatch): define cross-transport execution intent fixtures Pin complete Hub payload intent, alias precedence, context continuity and absent/false/zero semantics without inventing runtime or workspace defaults. Refs #2350. Co-authored-by: Codex --- tests/fixtures/dispatch/execution-intent.json | 206 ++++++++++++++++++ 1 file changed, 206 insertions(+) create mode 100644 tests/fixtures/dispatch/execution-intent.json diff --git a/tests/fixtures/dispatch/execution-intent.json b/tests/fixtures/dispatch/execution-intent.json new file mode 100644 index 000000000..a5dc0288b --- /dev/null +++ b/tests/fixtures/dispatch/execution-intent.json @@ -0,0 +1,206 @@ +{ + "version": 1, + "cases": [ + { + "name": "profile-and-request-snake-case", + "payload": { + "task_id": "task-intent-1", + "delivery_id": "delivery-intent-1", + "agent_type": "codex", + "session_id": "conversation-1", + "prompt": "Implement the requested patch.", + "system_prompt": "Use the approved project conventions.", + "tool_whitelist": "[\"Read\",\"Grep\"]", + "model_params": "{\"model\":\"model-selected\",\"reasoning_effort\":\"high\",\"thinking_mode\":\"enabled\",\"max_thinking_tokens\":4096,\"permission_mode\":\"plan\",\"work_dir\":\"/workspace/project\",\"include_partial\":true,\"append_system_prompt\":\"Do not change unrelated files.\",\"allowed_tools\":[\"Write\"],\"config_overrides\":{\"reasoning_summary\":\"auto\",\"discard_number\":3},\"ephemeral\":true,\"session_id\":\"runtime-session-1\",\"continue\":false,\"fork\":true}", + "messages": [ + { + "role": "user", + "content": "Keep the change offline and preserve existing behavior.", + "timestamp": "2026-01-01T00:00:00Z" + }, + { + "role": "assistant", + "content": "The earlier patch uses a bounded retry policy.", + "timestamp": "2026-01-01T00:01:00Z" + } + ], + "pinned_messages": [ + { + "role": "system", + "content": "Run focused tests before reporting success.", + "timestamp": "2026-01-01T00:00:00Z" + } + ], + "structured_output_schema": { + "type": "object", + "properties": { + "result": { + "type": "string" + } + }, + "required": [ + "result" + ] + }, + "trace_id": "trace-fixture-1" + }, + "expectedIntent": { + "prompt": "Implement the requested patch.", + "agentId": "codex", + "model": "model-selected", + "reasoningEffort": "high", + "thinkingMode": "enabled", + "maxThinkingTokens": 4096, + "permissionMode": "plan", + "workDir": "/workspace/project", + "includePartial": true, + "structuredOutputSchema": "{\"type\":\"object\",\"properties\":{\"result\":{\"type\":\"string\"}},\"required\":[\"result\"]}", + "systemPrompt": "Use the approved project conventions.", + "appendSystemPrompt": "Do not change unrelated files.", + "allowedTools": [ + "Read", + "Grep" + ], + "configOverrides": { + "reasoning_summary": "auto" + }, + "ephemeral": true, + "sessionId": "runtime-session-1", + "continue": false, + "fork": true, + "hubTaskId": "task-intent-1", + "deliveryId": "delivery-intent-1", + "messages": [ + { + "role": "user", + "content": "Keep the change offline and preserve existing behavior.", + "timestamp": "2026-01-01T00:00:00Z" + }, + { + "role": "assistant", + "content": "The earlier patch uses a bounded retry policy.", + "timestamp": "2026-01-01T00:01:00Z" + } + ], + "pinnedMessages": [ + { + "role": "system", + "content": "Run focused tests before reporting success.", + "timestamp": "2026-01-01T00:00:00Z" + } + ], + "trace_id": "trace-fixture-1" + } + }, + { + "name": "camel-params-preserve-false-and-zero", + "payload": { + "task_id": "task-intent-2", + "agent_type": "Claude", + "session_id": "conversation-2", + "prompt": "Review without modifying files.", + "model_params": "{\"model\":\"review-model\",\"reasoningEffort\":\"low\",\"thinkingMode\":\"disabled\",\"maxThinkingTokens\":0,\"permissionMode\":\"plan\",\"workDir\":\"/workspace/review\",\"includePartial\":false,\"structuredOutputSchema\":\"{\\\"type\\\":\\\"object\\\"}\",\"systemPrompt\":\"Review only.\",\"appendSystemPrompt\":\"Check existing tests.\",\"allowedTools\":[\"Read\"],\"configOverrides\":{\"summary\":\"concise\"},\"ephemeral\":false,\"sessionId\":\"runtime-session-2\",\"continue\":true,\"fork\":false}" + }, + "expectedIntent": { + "prompt": "Review without modifying files.", + "agentId": "claude-code", + "model": "review-model", + "reasoningEffort": "low", + "thinkingMode": "disabled", + "maxThinkingTokens": 0, + "permissionMode": "plan", + "workDir": "/workspace/review", + "includePartial": false, + "structuredOutputSchema": "{\"type\":\"object\"}", + "systemPrompt": "Review only.", + "appendSystemPrompt": "Check existing tests.", + "allowedTools": [ + "Read" + ], + "configOverrides": { + "summary": "concise" + }, + "ephemeral": false, + "sessionId": "runtime-session-2", + "continue": true, + "fork": false, + "hubTaskId": "task-intent-2" + } + }, + { + "name": "legacy-absent-intent-has-no-invented-model-or-session", + "payload": { + "task_id": "task-intent-3", + "agent_type": "opencode", + "session_id": "hub-conversation-not-runtime-session", + "prompt": "Continue the task.", + "model_params": "{}", + "tool_whitelist": "[]", + "messages": [], + "pinned_messages": [] + }, + "expectedIntent": { + "prompt": "Continue the task.", + "agentId": "opencode", + "hubTaskId": "task-intent-3" + } + }, + { + "name": "top-schema-string-with-model-override-precedence", + "payload": { + "task_id": "task-intent-4", + "delivery_id": "delivery-intent-4", + "agent_type": "codex", + "prompt": "Produce a structured answer.", + "system_prompt": "Top-level system prompt.", + "model_params": "{\"model\":\"model-4\",\"work_dir\":\"/workspace/project\",\"system_prompt\":\"Nested fallback.\",\"structured_output_schema\":\"{\\\"type\\\":\\\"object\\\",\\\"properties\\\":{\\\"count\\\":{\\\"type\\\":\\\"integer\\\"}}}\",\"tool_allowlist\":[\"Read\",\"Bash\"]}", + "structured_output_schema": "{\"type\":\"array\"}", + "pinned_messages": [ + { + "role": "system", + "content": "Run focused tests before reporting success.", + "timestamp": "2026-01-01T00:00:00Z" + } + ] + }, + "expectedIntent": { + "prompt": "Produce a structured answer.", + "agentId": "codex", + "model": "model-4", + "workDir": "/workspace/project", + "systemPrompt": "Top-level system prompt.", + "structuredOutputSchema": "{\"type\":\"object\",\"properties\":{\"count\":{\"type\":\"integer\"}}}", + "allowedTools": [ + "Read", + "Bash" + ], + "hubTaskId": "task-intent-4", + "deliveryId": "delivery-intent-4", + "pinnedMessages": [ + { + "role": "system", + "content": "Run focused tests before reporting success.", + "timestamp": "2026-01-01T00:00:00Z" + } + ] + } + }, + { + "name": "malformed-model-params-do-not-invent-defaults", + "payload": { + "task_id": "task-intent-5", + "agent_type": "codex", + "prompt": "Use only explicit input.", + "model_params": "{bad json", + "tool_whitelist": "not-json", + "structured_output_schema": "{\"type\":\"object\"}" + }, + "expectedIntent": { + "prompt": "Use only explicit input.", + "agentId": "codex", + "hubTaskId": "task-intent-5", + "structuredOutputSchema": "{\"type\":\"object\"}" + } + } + ] +} From 2a369e558f1f2b2d6c22c190959497d529a10292 Mon Sep 17 00:00:00 2001 From: Delicious233 <101502465+DeliciousBuding@users.noreply.github.com> Date: Mon, 7 Sep 2026 06:54:57 +0800 Subject: [PATCH 2/5] fix(edge): persist and enforce run callback ownership Advertise ownership-aware callback capabilities, reject unconfigured direct callbacks before admission, retain the original reporter across replays and prevent Desktop-owned runs from emitting a second Edge callback stream. Refs #2350. Co-authored-by: Codex --- .../internal/api/callback_owner_test.go | 144 ++++++++++++++++++ .../internal/api/handlers_run_callback.go | 56 +++++++ .../internal/api/handlers_run_delivery.go | 5 +- edge-server/internal/api/handlers_runs.go | 27 ++-- edge-server/internal/api/handlers_settings.go | 11 +- .../internal/api/hub_task_replay_test.go | 2 +- edge-server/internal/errcode/codes.go | 2 + edge-server/internal/hub/callback.go | 6 + .../internal/lifecycle/callback_owner_test.go | 47 ++++++ .../internal/lifecycle/mock_executor.go | 3 + .../lifecycle/process_executor_run.go | 2 +- .../lifecycle/thread_transcript_test.go | 2 +- edge-server/internal/runcontrol/admission.go | 7 +- .../internal/runcontrol/admission_test.go | 6 +- edge-server/internal/runcontrol/runcontrol.go | 7 +- .../internal/store/admission_cleanup_test.go | 2 +- edge-server/internal/store/file_store.go | 4 +- .../internal/store/run_callback_owner_test.go | 36 +++++ edge-server/internal/store/sqlite_store.go | 4 +- .../internal/store/store_interfaces.go | 2 +- .../internal/store/store_run_admission.go | 5 +- .../store/store_run_admission_test.go | 36 ++--- edge-server/internal/store/store_types.go | 1 + 23 files changed, 367 insertions(+), 50 deletions(-) create mode 100644 edge-server/internal/api/callback_owner_test.go create mode 100644 edge-server/internal/api/handlers_run_callback.go create mode 100644 edge-server/internal/lifecycle/callback_owner_test.go create mode 100644 edge-server/internal/store/run_callback_owner_test.go diff --git a/edge-server/internal/api/callback_owner_test.go b/edge-server/internal/api/callback_owner_test.go new file mode 100644 index 000000000..acb8f1753 --- /dev/null +++ b/edge-server/internal/api/callback_owner_test.go @@ -0,0 +1,144 @@ +package api + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + "github.com/agenthub/edge-server/internal/deliverydedup" + "github.com/agenthub/edge-server/internal/hub" + "github.com/agenthub/edge-server/internal/lifecycle" + "github.com/agenthub/edge-server/internal/store" +) + +type executionIntentRecorder struct { + contexts chan lifecycle.RunProcessContext +} + +func (e *executionIntentRecorder) Start(_ store.Run, ctx lifecycle.RunProcessContext) error { + e.contexts <- ctx + return nil +} +func (e *executionIntentRecorder) Cancel(string) lifecycle.CancelResult { + return lifecycle.CancelResult{Found: false} +} + +func TestCallbackOwnerUnavailableDoesNotAdmit(t *testing.T) { + executor := &admissionExecutor{} + server, h := newDeliveryTestServer(t, executor, nil) + defer server.Close() + body := admissionRunBody(h.WorkspaceAllowlist[0], "owner-delivery", "owner-task", map[string]any{"callbackOwner": "edge"}) + rejected := postRunsRaw(t, server.URL, body) + if rejected.status != http.StatusServiceUnavailable || errCode(rejected.body) != "callback_unavailable" || executor.StartCount() != 0 { + t.Fatalf("unowned direct admission: %d %#v starts=%d", rejected.status, rejected.body, executor.StartCount()) + } + if len(ensureStore(h).ListRuns("thread_local")) != 0 { + t.Fatal("callback preflight created a run") + } + fallback := postRunsRaw(t, server.URL, admissionRunBody(h.WorkspaceAllowlist[0], "owner-delivery", "owner-task", map[string]any{"callbackOwner": "desktop"})) + if fallback.status != http.StatusAccepted || unwrapSuccess(fallback.body)["callbackOwner"] != "desktop" || executor.StartCount() != 1 { + t.Fatalf("Desktop fallback did not admit exactly once: %#v", fallback.body) + } +} + +func TestCallbackOwnerPersistsAcrossWarmAndColdReplay(t *testing.T) { + executor := &admissionExecutor{} + server, h := newDeliveryTestServer(t, executor, func(h *Handler) { + h.CallbackClient = hub.NewCallbackClient("https://hub.example.invalid", "fixture-token", http.DefaultClient, hub.DefaultCallbackConfig()) + }) + defer server.Close() + body := admissionRunBody(h.WorkspaceAllowlist[0], "owner-replay", "owner-task", map[string]any{"callbackOwner": "edge"}) + accepted := postRunsRaw(t, server.URL, body) + if accepted.status != http.StatusAccepted { + t.Fatalf("edge-owned admission: %d %#v", accepted.status, accepted.body) + } + runID := unwrapSuccess(accepted.body)["runId"] + for _, cold := range []bool{false, true} { + if cold { + h.DeliveryDedup = deliverydedup.New(deliverydedup.DefaultCapacity, deliverydedup.DefaultTTL) + } + replay := postRunsRaw(t, server.URL, admissionRunBody(h.WorkspaceAllowlist[0], "owner-replay", "owner-task", map[string]any{"callbackOwner": "desktop"})) + data := unwrapSuccess(replay.body) + if replay.status != http.StatusAccepted || data["runId"] != runID || data["callbackOwner"] != "edge" || executor.StartCount() != 1 { + t.Fatalf("replay changed owner: cold=%v %#v starts=%d", cold, replay.body, executor.StartCount()) + } + } +} + +func TestCallbackOwnershipHealthIsExplicitAndContainsNoCredential(t *testing.T) { + for _, configured := range []bool{false, true} { + t.Run(map[bool]string{false: "sidecar", true: "direct"}[configured], func(t *testing.T) { + h := newTestHandler() + defer h.Bus.Close() + if configured { + h.CallbackClient = hub.NewCallbackClient("https://hub.example.invalid", "fixture-secret-not-exported", http.DefaultClient, hub.DefaultCallbackConfig()) + } + recorder := httptest.NewRecorder() + h.GetHealth(recorder, httptest.NewRequest(http.MethodGet, "/v1/health", nil)) + var data struct { + Capabilities map[string]bool `json:"capabilities"` + } + if err := json.Unmarshal(recorder.Body.Bytes(), &data); err != nil { + t.Fatal(err) + } + if !data.Capabilities["runCallbackOwnership"] || data.Capabilities["directHubCallbacks"] != configured { + t.Fatalf("wrong ownership capabilities: %s", recorder.Body.String()) + } + }) + } +} + +func TestExecutionIntentFixtureReachesEdgeAdmission(t *testing.T) { + raw, err := os.ReadFile(filepath.Join("..", "..", "..", "tests", "fixtures", "dispatch", "execution-intent.json")) + if err != nil { + t.Fatal(err) + } + var fixtures struct { + Cases []struct { + Name string `json:"name"` + Expected map[string]any `json:"expectedIntent"` + } `json:"cases"` + } + if err := json.Unmarshal(raw, &fixtures); err != nil { + t.Fatal(err) + } + for _, fixture := range fixtures.Cases { + if _, hasWorkDir := fixture.Expected["workDir"]; !hasWorkDir { + continue + } + t.Run(fixture.Name, func(t *testing.T) { + executor := &executionIntentRecorder{contexts: make(chan lifecycle.RunProcessContext, 1)} + server, h := newDeliveryTestServer(t, executor, func(h *Handler) { + h.CallbackClient = hub.NewCallbackClient("https://hub.example.invalid", "fixture-token", http.DefaultClient, hub.DefaultCallbackConfig()) + }) + defer server.Close() + body := fixture.Expected + body["workDir"] = h.WorkspaceAllowlist[0] + body["callbackOwner"] = "edge" + bytes, err := json.Marshal(body) + if err != nil { + t.Fatal(err) + } + result := postRunsRaw(t, server.URL, string(bytes)) + if result.status != http.StatusAccepted { + t.Fatalf("projected direct request rejected: %d %#v", result.status, result.body) + } + ctx := <-executor.contexts + if ctx.WorkDir != h.WorkspaceAllowlist[0] || ctx.Model != body["model"] || ctx.Prompt != body["prompt"] || ctx.Run.CallbackOwner != "edge" { + t.Fatalf("execution intent lost: %#v", ctx) + } + if wanted, ok := body["messages"].([]any); ok && len(ctx.Messages) != len(wanted) { + t.Fatalf("messages lost: %#v", ctx.Messages) + } + if wanted, ok := body["pinnedMessages"].([]any); ok && len(ctx.PinnedMessages) != len(wanted) { + t.Fatalf("pins lost: %#v", ctx.PinnedMessages) + } + if ctx.StructuredOutputSchema != body["structuredOutputSchema"] { + t.Fatalf("schema lost: %q", ctx.StructuredOutputSchema) + } + }) + } +} diff --git a/edge-server/internal/api/handlers_run_callback.go b/edge-server/internal/api/handlers_run_callback.go new file mode 100644 index 000000000..49dcb0ab5 --- /dev/null +++ b/edge-server/internal/api/handlers_run_callback.go @@ -0,0 +1,56 @@ +package api + +import ( + "github.com/agenthub/edge-server/internal/errcode" + "github.com/agenthub/edge-server/internal/store" +) + +func (h *Handler) directHubCallbacksConfigured() bool { + configured, ok := h.CallbackClient.(interface{ Configured() bool }) + return ok && configured.Configured() +} + +func (h *Handler) resolveRunCallbackOwner(req *runRequest) *errcode.Error { + switch req.CallbackOwner { + case "", "edge", "desktop": + default: + return errcode.ErrBadRequest.WithMessage("callbackOwner must be edge or desktop") + } + if req.HubTaskID == "" { + if req.CallbackOwner != "" { + return errcode.ErrBadRequest.WithMessage("callbackOwner requires hubTaskId") + } + return nil + } + // Every new Hub run records one owner. Modern transports send it explicitly. + if req.CallbackOwner == "" { + req.CallbackOwner = "desktop" + if h.directHubCallbacksConfigured() { + req.CallbackOwner = "edge" + } + } + return nil +} + +func (h *Handler) validateCallbackAdmission(req runRequest) *errcode.Error { + if req.CallbackOwner == "edge" && !h.directHubCallbacksConfigured() { + return errcode.ErrCallbackUnavailable + } + return nil +} + +// Never guess legacy ownership during replay: it could leave no output owner +// or produce two independent reporters for the same execution. +func validateReplayCallbackOwner(req runRequest, run store.Run) *errcode.Error { + if req.CallbackOwner != "" && run.CallbackOwner != "edge" && run.CallbackOwner != "desktop" { + return errcode.ErrAdmissionUncertain.WithMessagef("callback ownership for run %s requires reconciliation", run.ID) + } + return nil +} + +func (h *Handler) runCallbackCapabilities() map[string]bool { + return map[string]bool{ + "runCallbackOwnership": true, + "directHubCallbacks": h.directHubCallbacksConfigured(), + } +} diff --git a/edge-server/internal/api/handlers_run_delivery.go b/edge-server/internal/api/handlers_run_delivery.go index 9a0c538ca..5522bd9dc 100644 --- a/edge-server/internal/api/handlers_run_delivery.go +++ b/edge-server/internal/api/handlers_run_delivery.go @@ -60,5 +60,8 @@ func (h *Handler) beginRunDelivery(w http.ResponseWriter, r *http.Request, req r func (h *Handler) validateRunReplay(r *http.Request, req runRequest, run store.Run) *errcode.Error { req.ProjectID = run.ProjectID req.ThreadID = run.ThreadID - return h.validateCapabilityRequest(r, &req) + if err := h.validateCapabilityRequest(r, &req); err != nil { + return err + } + return validateReplayCallbackOwner(req, run) } diff --git a/edge-server/internal/api/handlers_runs.go b/edge-server/internal/api/handlers_runs.go index 266679d8d..bd20fbf6a 100644 --- a/edge-server/internal/api/handlers_runs.go +++ b/edge-server/internal/api/handlers_runs.go @@ -58,6 +58,7 @@ type runRequest struct { AgentDefinitions map[string]runnerctx.AgentDefinition `json:"agentDefinitions"` MCPConfig string `json:"mcpConfig"` Ephemeral bool `json:"ephemeral"` + CallbackOwner string `json:"callbackOwner"` HubTaskID string `json:"hubTaskId"` // Edge-to-Hub direct callback task ID TraceID string `json:"trace_id,omitempty"` // Hub dispatch trace correlation id DeliveryID string `json:"deliveryId"` // Hub delivery_id for dual-channel dedup (#2101 G2) @@ -333,6 +334,10 @@ func (h *Handler) PostRuns(w http.ResponseWriter, r *http.Request) { return } + if err := h.resolveRunCallbackOwner(&req); err != nil { + errcode.Write(w, err) + return + } repository := ensureStore(h) claim, handled := h.beginRunDelivery(w, r, req, repository) if handled { @@ -376,16 +381,18 @@ func (h *Handler) PostRuns(w http.ResponseWriter, r *http.Request) { // contributes the timeline policy and the adapter context builder. var replayRunID string run, err := runcontrol.Create(repository, h.Executor, h.Bus, runcontrol.CreateParams{ - ProjectID: req.ProjectID, - ThreadID: req.ThreadID, - Prompt: req.Prompt, - AgentID: req.AgentID, - Model: req.Model, - PermissionMode: req.PermissionMode, - SessionID: req.SessionID, - ContinueLast: req.Continue, - WorkDir: req.WorkDir, - HubTaskID: req.HubTaskID, + ProjectID: req.ProjectID, + ThreadID: req.ThreadID, + Prompt: req.Prompt, + AgentID: req.AgentID, + Model: req.Model, + PermissionMode: req.PermissionMode, + SessionID: req.SessionID, + ContinueLast: req.Continue, + WorkDir: req.WorkDir, + HubTaskID: req.HubTaskID, + CallbackOwner: req.CallbackOwner, + ValidateAdmission: func() *errcode.Error { return h.validateCallbackAdmission(req) }, AuthorizeReplay: func(existing store.Run) *errcode.Error { replayRunID = existing.ID return h.validateRunReplay(r, req, existing) diff --git a/edge-server/internal/api/handlers_settings.go b/edge-server/internal/api/handlers_settings.go index 23749682a..092e8018a 100644 --- a/edge-server/internal/api/handlers_settings.go +++ b/edge-server/internal/api/handlers_settings.go @@ -188,11 +188,12 @@ func (h *Handler) GetHealth(w http.ResponseWriter, r *http.Request) { } writeJSON(w, httpStatus, map[string]any{ - "status": status, - "http_status": httpStatus, - "version": "v1", - "edgeId": "local", - "checks": checks, + "status": status, + "http_status": httpStatus, + "version": "v1", + "edgeId": "local", + "checks": checks, + "capabilities": h.runCallbackCapabilities(), }) } diff --git a/edge-server/internal/api/hub_task_replay_test.go b/edge-server/internal/api/hub_task_replay_test.go index 4f18fa66b..845c3c306 100644 --- a/edge-server/internal/api/hub_task_replay_test.go +++ b/edge-server/internal/api/hub_task_replay_test.go @@ -150,7 +150,7 @@ func TestHubTaskReplay_UncertainAdmissionExposesReadOnlyEvidence(t *testing.T) { server, h := newDeliveryTestServer(t, executor, nil) defer server.Close() repository := ensureStore(h) - run, err := repository.CreateRunAdmission("run-needs-review", "proj_local", "thread_local", "task-needs-review") + run, err := repository.CreateRunAdmission("run-needs-review", "proj_local", "thread_local", "task-needs-review", "") if err != nil { t.Fatal(err) } diff --git a/edge-server/internal/errcode/codes.go b/edge-server/internal/errcode/codes.go index 452a87b13..d9954ae9d 100644 --- a/edge-server/internal/errcode/codes.go +++ b/edge-server/internal/errcode/codes.go @@ -62,6 +62,8 @@ var ( ErrAdmissionPersistFailed = New("admission_persist_failed", "run admission evidence could not be persisted; retry later", http.StatusServiceUnavailable) ErrAdmissionUncertain = New("admission_uncertain", "run admission outcome requires reconciliation; do not restart automatically", http.StatusConflict) + ErrCallbackUnavailable = New("callback_unavailable", "Edge direct Hub callbacks are not configured; route through Desktop", http.StatusServiceUnavailable) + // Agent discovery ErrInvalidAgentID = New("invalid_agent_id", "unknown agent adapter", http.StatusBadRequest) ErrAgentRegistryNotConfigured = New("agent_registry_not_configured", "agent registry not configured", http.StatusServiceUnavailable) diff --git a/edge-server/internal/hub/callback.go b/edge-server/internal/hub/callback.go index 161841e81..44d3091fe 100644 --- a/edge-server/internal/hub/callback.go +++ b/edge-server/internal/hub/callback.go @@ -124,6 +124,12 @@ func (c *CallbackClient) currentAuthToken() string { return c.authToken } +// Configured reports whether direct task callbacks have a destination and a +// current credential. It exposes no credential and makes no connectivity claim. +func (c *CallbackClient) Configured() bool { + return c != nil && strings.TrimSpace(c.hubURL) != "" && strings.TrimSpace(c.currentAuthToken()) != "" +} + // TaskResult carries the final result of a completed task. type TaskResult struct { RunID string `json:"run_id"` diff --git a/edge-server/internal/lifecycle/callback_owner_test.go b/edge-server/internal/lifecycle/callback_owner_test.go new file mode 100644 index 000000000..c1d182107 --- /dev/null +++ b/edge-server/internal/lifecycle/callback_owner_test.go @@ -0,0 +1,47 @@ +package lifecycle + +import ( + "testing" + "time" + + "github.com/agenthub/edge-server/internal/events" + "github.com/agenthub/edge-server/internal/store" +) + +func TestProcessExecutorCallbackOwnership(t *testing.T) { + for _, owner := range []string{"edge", "desktop"} { + t.Run(owner, func(t *testing.T) { + bus := events.NewBus(100) + defer bus.Close() + repository := store.New() + seed := newExecutorTestRun(t, repository) + run, err := repository.CreateRunAdmission(uniqueHubTestRunID("owner"), seed.ProjectID, seed.ThreadID, "task-owner", owner) + if err != nil { + t.Fatal(err) + } + _, eventsCh, _ := bus.Subscribe(0) + executor := newTestProcessExecutor(t, bus, repository, "success") + callback := newRecordingHubCallback() + executor.WithHubCallback(callback) + if err := executor.Start(run, RunProcessContext{HubTaskID: "task-owner"}); err != nil { + t.Fatal(err) + } + _ = collectEventsUntilRunDone(t, eventsCh) + if owner == "edge" { + select { + case <-callback.doneSeen: + case <-time.After(5 * time.Second): + t.Fatal("Edge owner never reported the result") + } + } + callback.mu.Lock() + defer callback.mu.Unlock() + if owner == "desktop" && (len(callback.acks)+len(callback.streams)+len(callback.dones)+len(callback.fails) != 0) { + t.Fatalf("Desktop-owned run also emitted Edge callbacks: %#v", callback) + } + if owner == "edge" && (len(callback.dones) != 1 || callback.dones[0].RunID != run.ID) { + t.Fatalf("wrong result owner: %#v", callback.dones) + } + }) + } +} diff --git a/edge-server/internal/lifecycle/mock_executor.go b/edge-server/internal/lifecycle/mock_executor.go index 115845c09..64e3989fa 100644 --- a/edge-server/internal/lifecycle/mock_executor.go +++ b/edge-server/internal/lifecycle/mock_executor.go @@ -245,6 +245,9 @@ func RunResponse(run store.Run) map[string]any { if run.WorkDir != "" { payload["workDir"] = run.WorkDir } + if run.CallbackOwner != "" { + payload["callbackOwner"] = run.CallbackOwner + } if run.AdmissionState != "" { payload["admissionState"] = run.AdmissionState } diff --git a/edge-server/internal/lifecycle/process_executor_run.go b/edge-server/internal/lifecycle/process_executor_run.go index c0429b339..567510f65 100644 --- a/edge-server/internal/lifecycle/process_executor_run.go +++ b/edge-server/internal/lifecycle/process_executor_run.go @@ -25,7 +25,7 @@ func (e *ProcessExecutor) run(ctx context.Context, run store.Run, runCtx RunProc // Edge→Hub direct callback reporting. Without the collector, recordHubOutput // no-ops and fireHubDone falls back to literal "Run finished" (#987). // Pure gate only; hubOutputs allocation stays here (#987 residual ownership). - if planHubTaskRecord(runCtx.HubTaskID).Record { + if run.CallbackOwner != "desktop" && planHubTaskRecord(runCtx.HubTaskID).Record { e.mu.Lock() e.hubTasks[run.ID] = runCtx.HubTaskID e.hubOutputs[run.ID] = newHubOutputCollector(hubCallbackFinalMaxBytes) diff --git a/edge-server/internal/lifecycle/thread_transcript_test.go b/edge-server/internal/lifecycle/thread_transcript_test.go index 97d5ac13f..e1ee58ad7 100644 --- a/edge-server/internal/lifecycle/thread_transcript_test.go +++ b/edge-server/internal/lifecycle/thread_transcript_test.go @@ -28,7 +28,7 @@ func (w *stubTranscriptWriter) DeleteThread(id string) bool { return false } func (w *stubTranscriptWriter) CreateRun(id, projectID, threadID string) (store.Run, error) { return store.Run{}, nil } -func (w *stubTranscriptWriter) CreateRunAdmission(id, projectID, threadID, hubTaskID string) (store.Run, error) { +func (w *stubTranscriptWriter) CreateRunAdmission(id, projectID, threadID, hubTaskID, callbackOwner string) (store.Run, error) { return store.Run{}, store.ErrNotFound } func (w *stubTranscriptWriter) RecordRunAdmission(id, errorCode string) (store.Run, error) { diff --git a/edge-server/internal/runcontrol/admission.go b/edge-server/internal/runcontrol/admission.go index 252839b9a..6376a0187 100644 --- a/edge-server/internal/runcontrol/admission.go +++ b/edge-server/internal/runcontrol/admission.go @@ -61,6 +61,11 @@ func prepareRunAdmission(repository store.Repository, executor lifecycle.RunExec if params.HubTaskID != "" && params.BuildContext == nil { return store.Run{}, false, errcode.ErrExecutorUnavailable.WithMessage("Hub task admission requires an executor context") } + if params.ValidateAdmission != nil { + if err := params.ValidateAdmission(); err != nil { + return store.Run{}, false, err + } + } run, err := createRunAdmissionRecord(repository, params) if err != nil { return store.Run{}, false, err @@ -81,7 +86,7 @@ func createRunAdmissionRecord(repository store.Repository, params CreateParams) if params.HubTaskID == "" { run, err = repository.CreateRun(runID, params.ProjectID, params.ThreadID) } else { - run, err = repository.CreateRunAdmission(runID, params.ProjectID, params.ThreadID, params.HubTaskID) + run, err = repository.CreateRunAdmission(runID, params.ProjectID, params.ThreadID, params.HubTaskID, params.CallbackOwner) } if err != nil { if errors.Is(err, store.ErrNotFound) { diff --git a/edge-server/internal/runcontrol/admission_test.go b/edge-server/internal/runcontrol/admission_test.go index e624d195f..837dad7fa 100644 --- a/edge-server/internal/runcontrol/admission_test.go +++ b/edge-server/internal/runcontrol/admission_test.go @@ -15,8 +15,8 @@ type admissionWriteFailure struct { failAccepted bool } -func (r *admissionWriteFailure) CreateRunAdmission(id, project, thread, task string) (store.Run, error) { - run, err := r.Repository.CreateRunAdmission(id, project, thread, task) +func (r *admissionWriteFailure) CreateRunAdmission(id, project, thread, task, callbackOwner string) (store.Run, error) { + run, err := r.Repository.CreateRunAdmission(id, project, thread, task, callbackOwner) if err == nil && r.failPrepare { r.failPrepare = false return run, errors.New("fixture pending write failure") @@ -88,7 +88,7 @@ func TestHubAdmission_ReopenKeepsIdentityWithoutRestarting(t *testing.T) { case "accepted": original, err = Create(repo, executor, nil, params) case "pending": - original, err = repo.CreateRunAdmission("run-pending", "proj_local", "thread_local", params.HubTaskID) + original, err = repo.CreateRunAdmission("run-pending", "proj_local", "thread_local", params.HubTaskID, "") default: original, err = repo.CreateRun("run-legacy", "proj_local", "thread_local") if err == nil { diff --git a/edge-server/internal/runcontrol/runcontrol.go b/edge-server/internal/runcontrol/runcontrol.go index 1edca18d1..859d9b8d8 100644 --- a/edge-server/internal/runcontrol/runcontrol.go +++ b/edge-server/internal/runcontrol/runcontrol.go @@ -76,7 +76,12 @@ type CreateParams struct { // HubTaskID identifies one logical Hub task across delivery transports. // A retained run is replayable only with admission evidence, not merely // because a queued/failed record exists. - HubTaskID string + HubTaskID string + CallbackOwner string + + // ValidateAdmission checks transport prerequisites only for fresh work; + // retained receipts keep their original execution and callback owner. + ValidateAdmission func() *errcode.Error // AuthorizeReplay validates the actual stored scope before replaying a Hub // task. Transports with capability policy must supply it. Without a policy, diff --git a/edge-server/internal/store/admission_cleanup_test.go b/edge-server/internal/store/admission_cleanup_test.go index 9a411f89b..082874600 100644 --- a/edge-server/internal/store/admission_cleanup_test.go +++ b/edge-server/internal/store/admission_cleanup_test.go @@ -14,7 +14,7 @@ func TestCleanupRuns_RetainsPendingAdmissionAfterEarlyExecutionFinish(t *testing t.Fatal(err) } for _, id := range []string{"pending", "accepted"} { - if _, err := repo.CreateRunAdmission(id, "p", "t", id); err != nil { + if _, err := repo.CreateRunAdmission(id, "p", "t", id, ""); err != nil { t.Fatal(err) } repo.SetRunStatus(id, "finished") diff --git a/edge-server/internal/store/file_store.go b/edge-server/internal/store/file_store.go index d52ec61b1..a529a1b91 100644 --- a/edge-server/internal/store/file_store.go +++ b/edge-server/internal/store/file_store.go @@ -250,8 +250,8 @@ func (f *FileStore) CreateRun(id, projectID, threadID string) (Run, error) { return run, nil } -func (f *FileStore) CreateRunAdmission(id, projectID, threadID, hubTaskID string) (Run, error) { - run, err := f.store.CreateRunAdmission(id, projectID, threadID, hubTaskID) +func (f *FileStore) CreateRunAdmission(id, projectID, threadID, hubTaskID, callbackOwner string) (Run, error) { + run, err := f.store.CreateRunAdmission(id, projectID, threadID, hubTaskID, callbackOwner) if err != nil { return Run{}, err } diff --git a/edge-server/internal/store/run_callback_owner_test.go b/edge-server/internal/store/run_callback_owner_test.go new file mode 100644 index 000000000..471190f4b --- /dev/null +++ b/edge-server/internal/store/run_callback_owner_test.go @@ -0,0 +1,36 @@ +package store + +import ( + "path/filepath" + "testing" +) + +func TestRunCallbackOwnerSurvivesReopen(t *testing.T) { + for _, kind := range []string{"file", "sqlite"} { + t.Run(kind, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "owners.db") + repository := openRunAdmissionStore(t, kind, path) + project, thread := seedRunAdmissionStore(t, repository) + for _, owner := range []string{"edge", "desktop"} { + run, err := repository.CreateRunAdmission("run-"+owner, project.ID, thread.ID, "task-"+owner, owner) + if err != nil { + t.Fatal(err) + } + if _, err := repository.CreateRunAdmission(run.ID, project.ID, thread.ID, run.HubTaskID, "different"); err == nil { + t.Fatal("pending owner could be reassigned") + } + if _, err := repository.RecordRunAdmission(run.ID, ""); err != nil { + t.Fatal(err) + } + } + repository.Close() + recovered := openRunAdmissionStore(t, kind, path) + for _, owner := range []string{"edge", "desktop"} { + run, ok := recovered.GetRun("run-" + owner) + if !ok || run.CallbackOwner != owner { + t.Fatalf("owner lost after reopen: %#v", run) + } + } + }) + } +} diff --git a/edge-server/internal/store/sqlite_store.go b/edge-server/internal/store/sqlite_store.go index 8999c9bca..5fd5ab02f 100644 --- a/edge-server/internal/store/sqlite_store.go +++ b/edge-server/internal/store/sqlite_store.go @@ -376,8 +376,8 @@ func (s *SQLiteStore) CreateRun(id, projectID, threadID string) (Run, error) { return persistAfterSQLiteWrite(s, run, err) } -func (s *SQLiteStore) CreateRunAdmission(id, projectID, threadID, hubTaskID string) (Run, error) { - run, err := s.store.CreateRunAdmission(id, projectID, threadID, hubTaskID) +func (s *SQLiteStore) CreateRunAdmission(id, projectID, threadID, hubTaskID, callbackOwner string) (Run, error) { + run, err := s.store.CreateRunAdmission(id, projectID, threadID, hubTaskID, callbackOwner) return persistAfterSQLiteWrite(s, run, err) } diff --git a/edge-server/internal/store/store_interfaces.go b/edge-server/internal/store/store_interfaces.go index b53da49bf..7c9c20d25 100644 --- a/edge-server/internal/store/store_interfaces.go +++ b/edge-server/internal/store/store_interfaces.go @@ -33,7 +33,7 @@ type Writer interface { UpdateThread(id string, title *string, status *string) (Thread, bool) DeleteThread(id string) bool CreateRun(id, projectID, threadID string) (Run, error) - CreateRunAdmission(id, projectID, threadID, hubTaskID string) (Run, error) + CreateRunAdmission(id, projectID, threadID, hubTaskID, callbackOwner string) (Run, error) RecordRunAdmission(id, errorCode string) (Run, error) SetRunStatus(id, status string) (Run, bool) SetRunStatusIf(id, status string, allowedCurrent ...string) (Run, bool) diff --git a/edge-server/internal/store/store_run_admission.go b/edge-server/internal/store/store_run_admission.go index a2dd4ba06..2f42ec11f 100644 --- a/edge-server/internal/store/store_run_admission.go +++ b/edge-server/internal/store/store_run_admission.go @@ -21,7 +21,7 @@ var ( // CreateRunAdmission creates a run and records its Hub task identity and pending // admission atomically under one Store lock. It reuses the existing run create // validation/order helper; only the non-empty HubTaskID and admission marker are new. -func (s *Store) CreateRunAdmission(id, projectID, threadID, hubTaskID string) (Run, error) { +func (s *Store) CreateRunAdmission(id, projectID, threadID, hubTaskID, callbackOwner string) (Run, error) { if hubTaskID == "" { return Run{}, ErrRunAdmissionHubTaskIDRequired } @@ -37,7 +37,7 @@ func (s *Store) CreateRunAdmission(id, projectID, threadID, hubTaskID string) (R if run.AdmissionState == RunAdmissionPending && run.HubTaskID == hubTaskID && run.ProjectID == projectID && - run.ThreadID == threadID { + run.ThreadID == threadID && run.CallbackOwner == callbackOwner { return run, nil } return Run{}, admissionCreateConflictError(id, hubTaskID, projectID, threadID, run) @@ -45,6 +45,7 @@ func (s *Store) CreateRunAdmission(id, projectID, threadID, hubTaskID string) (R s.runOrder = order run.HubTaskID = hubTaskID + run.CallbackOwner = callbackOwner run.AdmissionState = RunAdmissionPending run.AdmissionErrorCode = "" s.runs[id] = run diff --git a/edge-server/internal/store/store_run_admission_test.go b/edge-server/internal/store/store_run_admission_test.go index b63455bd2..27d3413c5 100644 --- a/edge-server/internal/store/store_run_admission_test.go +++ b/edge-server/internal/store/store_run_admission_test.go @@ -13,11 +13,11 @@ func TestRunAdmissionStateMachine(t *testing.T) { s := New() project, thread := seedRunAdmissionStore(t, s) - if _, err := s.CreateRunAdmission("admission_missing_hub", project.ID, thread.ID, ""); !errors.Is(err, ErrRunAdmissionHubTaskIDRequired) { + if _, err := s.CreateRunAdmission("admission_missing_hub", project.ID, thread.ID, "", ""); !errors.Is(err, ErrRunAdmissionHubTaskIDRequired) { t.Fatalf("CreateRunAdmission empty HubTaskID error = %v, want ErrRunAdmissionHubTaskIDRequired", err) } - run, err := s.CreateRunAdmission("admission_run_1", project.ID, thread.ID, "hub-task-1") + run, err := s.CreateRunAdmission("admission_run_1", project.ID, thread.ID, "hub-task-1", "") if err != nil { t.Fatalf("CreateRunAdmission returned error: %v", err) } @@ -66,7 +66,7 @@ func TestRunAdmissionStateMachine(t *testing.T) { t.Fatalf("accepted -> rejected error = %v, want ErrRunAdmissionInvalidTransition", err) } - rejected, err := s.CreateRunAdmission("admission_run_2", project.ID, thread.ID, "hub-task-2") + rejected, err := s.CreateRunAdmission("admission_run_2", project.ID, thread.ID, "hub-task-2", "") if err != nil { t.Fatalf("CreateRunAdmission second returned error: %v", err) } @@ -101,7 +101,7 @@ func TestCreateRunAdmissionDoesNotResetExistingFinalOrLegacy(t *testing.T) { s := New() project, thread := seedRunAdmissionStore(t, s) - accepted, err := s.CreateRunAdmission("same_admission_id", project.ID, thread.ID, "same-task") + accepted, err := s.CreateRunAdmission("same_admission_id", project.ID, thread.ID, "same-task", "") if err != nil { t.Fatalf("CreateRunAdmission accepted returned error: %v", err) } @@ -111,7 +111,7 @@ func TestCreateRunAdmissionDoesNotResetExistingFinalOrLegacy(t *testing.T) { } // Rebuilding an existing run ID must not downgrade a final admission. - if _, err := s.CreateRunAdmission(accepted.ID, project.ID, thread.ID, "same-task"); err == nil || + if _, err := s.CreateRunAdmission(accepted.ID, project.ID, thread.ID, "same-task", ""); err == nil || (!errors.Is(err, ErrRunAdmissionInvalidTransition) && !errors.Is(err, ErrRunAdmissionExists)) { t.Fatalf("CreateRunAdmission same accepted ID error = %v, want explicit admission conflict", err) } @@ -122,7 +122,7 @@ func TestCreateRunAdmissionDoesNotResetExistingFinalOrLegacy(t *testing.T) { } // A different HubTaskID must never overwrite the existing binding. - if _, err := s.CreateRunAdmission(accepted.ID, project.ID, thread.ID, "different-task"); err == nil || + if _, err := s.CreateRunAdmission(accepted.ID, project.ID, thread.ID, "different-task", ""); err == nil || (!errors.Is(err, ErrRunAdmissionInvalidTransition) && !errors.Is(err, ErrRunAdmissionExists)) { t.Fatalf("CreateRunAdmission different HubTaskID error = %v, want explicit conflict", err) } @@ -140,7 +140,7 @@ func TestCreateRunAdmissionDoesNotResetExistingFinalOrLegacy(t *testing.T) { if err != nil { t.Fatalf("CreateThread other returned error: %v", err) } - if _, err := s.CreateRunAdmission(accepted.ID, otherProject.ID, otherThread.ID, "same-task"); err == nil || + if _, err := s.CreateRunAdmission(accepted.ID, otherProject.ID, otherThread.ID, "same-task", ""); err == nil || (!errors.Is(err, ErrRunAdmissionInvalidTransition) && !errors.Is(err, ErrRunAdmissionExists)) { t.Fatalf("CreateRunAdmission different scope error = %v, want explicit conflict", err) } @@ -151,11 +151,11 @@ func TestCreateRunAdmissionDoesNotResetExistingFinalOrLegacy(t *testing.T) { } // A matching pending run ID is idempotent. - pending, err := s.CreateRunAdmission("pending_admission_id", project.ID, thread.ID, "pending-task") + pending, err := s.CreateRunAdmission("pending_admission_id", project.ID, thread.ID, "pending-task", "") if err != nil { t.Fatalf("CreateRunAdmission pending returned error: %v", err) } - again, err := s.CreateRunAdmission(pending.ID, project.ID, thread.ID, "pending-task") + again, err := s.CreateRunAdmission(pending.ID, project.ID, thread.ID, "pending-task", "") if err != nil { t.Fatalf("CreateRunAdmission matching pending retry returned error: %v", err) } @@ -164,7 +164,7 @@ func TestCreateRunAdmissionDoesNotResetExistingFinalOrLegacy(t *testing.T) { } // New attempts keep using a new run ID and remain usable. - attempt, err := s.CreateRunAdmission("new_attempt_id", project.ID, thread.ID, "same-task") + attempt, err := s.CreateRunAdmission("new_attempt_id", project.ID, thread.ID, "same-task", "") if err != nil { t.Fatalf("CreateRunAdmission new attempt returned error: %v", err) } @@ -180,7 +180,7 @@ func TestCreateRunAdmissionDoesNotResetExistingFinalOrLegacy(t *testing.T) { if err != nil { t.Fatalf("CreateRun legacy returned error: %v", err) } - if _, err := s.CreateRunAdmission(legacy.ID, project.ID, thread.ID, "legacy-task"); err == nil || + if _, err := s.CreateRunAdmission(legacy.ID, project.ID, thread.ID, "legacy-task", ""); err == nil || (!errors.Is(err, ErrRunAdmissionInvalidTransition) && !errors.Is(err, ErrRunAdmissionExists)) { t.Fatalf("CreateRunAdmission legacy ID error = %v, want explicit conflict", err) } @@ -197,12 +197,12 @@ func TestRunAdmissionPhasesSurviveReopen(t *testing.T) { first := openRunAdmissionStore(t, kind, path) project, thread := seedRunAdmissionStore(t, first) - pending, err := first.CreateRunAdmission("admission_pending", project.ID, thread.ID, "task-pending") + pending, err := first.CreateRunAdmission("admission_pending", project.ID, thread.ID, "task-pending", "") if err != nil { t.Fatalf("CreateRunAdmission pending returned error: %v", err) } assertDurableRunAdmission(t, first, path, pending.ID, RunAdmissionPending, "task-pending", "") - accepted, err := first.CreateRunAdmission("admission_accepted", project.ID, thread.ID, "task-accepted") + accepted, err := first.CreateRunAdmission("admission_accepted", project.ID, thread.ID, "task-accepted", "") if err != nil { t.Fatalf("CreateRunAdmission accepted returned error: %v", err) } @@ -211,7 +211,7 @@ func TestRunAdmissionPhasesSurviveReopen(t *testing.T) { t.Fatalf("RecordRunAdmission accepted returned error: %v", err) } assertDurableRunAdmission(t, first, path, accepted.ID, RunAdmissionAccepted, "task-accepted", "") - rejected, err := first.CreateRunAdmission("admission_rejected", project.ID, thread.ID, "task-rejected") + rejected, err := first.CreateRunAdmission("admission_rejected", project.ID, thread.ID, "task-rejected", "") if err != nil { t.Fatalf("CreateRunAdmission rejected returned error: %v", err) } @@ -253,18 +253,18 @@ func TestRunAdmissionLatestAttempt(t *testing.T) { repo = openRunAdmissionStore(t, kind, path) } project, thread := seedRunAdmissionStore(t, repo) - first, err := repo.CreateRunAdmission("admission_latest_1", project.ID, thread.ID, "hub-task-latest") + first, err := repo.CreateRunAdmission("admission_latest_1", project.ID, thread.ID, "hub-task-latest", "") if err != nil { t.Fatalf("CreateRunAdmission first returned error: %v", err) } if _, err := repo.RecordRunAdmission(first.ID, "capacity"); err != nil { t.Fatalf("RecordRunAdmission first returned error: %v", err) } - second, err := repo.CreateRunAdmission("admission_latest_2", project.ID, thread.ID, "hub-task-latest") + second, err := repo.CreateRunAdmission("admission_latest_2", project.ID, thread.ID, "hub-task-latest", "") if err != nil { t.Fatalf("CreateRunAdmission second returned error: %v", err) } - third, err := repo.CreateRunAdmission("admission_latest_3", project.ID, thread.ID, "hub-task-latest") + third, err := repo.CreateRunAdmission("admission_latest_3", project.ID, thread.ID, "hub-task-latest", "") if err != nil { t.Fatalf("CreateRunAdmission third returned error: %v", err) } @@ -298,7 +298,7 @@ func TestRunAdmissionPersistenceFailureRetry(t *testing.T) { block, unblock := blockRunAdmissionPersistence(t, repo) block() - run, err := repo.CreateRunAdmission("admission_persist_retry", project.ID, thread.ID, "hub-task-persist") + run, err := repo.CreateRunAdmission("admission_persist_retry", project.ID, thread.ID, "hub-task-persist", "") if err == nil { t.Fatal("CreateRunAdmission with blocked persistence returned nil error") } diff --git a/edge-server/internal/store/store_types.go b/edge-server/internal/store/store_types.go index 4f37ae44a..26c09a7e4 100644 --- a/edge-server/internal/store/store_types.go +++ b/edge-server/internal/store/store_types.go @@ -41,6 +41,7 @@ type Run struct { HubTaskID string `json:"hubTaskId,omitempty"` AdmissionState string `json:"admissionState,omitempty"` AdmissionErrorCode string `json:"admissionErrorCode,omitempty"` + CallbackOwner string `json:"callbackOwner,omitempty"` } type RunDiffFile struct { From 9e8e21878391e466d78292cf63cd69b443b04f00 Mon Sep 17 00:00:00 2001 From: Delicious233 <101502465+DeliciousBuding@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:02:45 +0800 Subject: [PATCH 3/5] fix(dispatch): bind execution intent and callback devices across retries --- .github/workflows/checks.yml | 3 + api/dispatch.md | 34 +++ api/events.md | 18 +- api/openapi.yaml | 78 ++++++- .../github-actions-ci-cd-policy.md | 2 +- .../internal/api/callback_owner_test.go | 18 ++ edge-server/internal/api/handlers_runs.go | 11 +- edge-server/internal/api/handlers_settings.go | 6 +- .../repository/agent_direct_receipt.go | 66 ++++++ .../repository/agent_direct_receipt_test.go | 44 ++++ .../internal/repository/helpers_test.go | 2 +- .../agent/agent_direct_receipt_test.go | 37 ++++ .../service/dispatch/assemble_test.go | 15 +- .../service/dispatch/edge_execution_intent.go | 197 ++++++++++++++++++ .../service/dispatch/edge_http_prep.go | 10 +- .../internal/service/dispatch/edge_request.go | 127 +++++++---- .../service/dispatch/execution_intent_test.go | 117 +++++++++++ .../service/dispatch/helpers_extra_test.go | 40 ++-- .../service/dispatch/residual_test.go | 13 +- .../service/dispatchsvc/agent_dispatch.go | 57 ++++- .../agent_dispatch_callback_route.go | 60 ++++++ .../agent_dispatch_callback_route_test.go | 76 +++++++ .../dispatchsvc/agent_dispatch_edge_http.go | 86 ++++++-- .../agent_dispatch_edge_http_test.go | 54 +++-- .../agent_dispatch_target_bound.go | 9 +- .../direct_dispatch_helpers_test.go | 39 ++++ .../direct_dispatch_recovery_test.go | 161 ++++++++++++++ .../tests/integration/direct_receipt_test.go | 62 ++++++ tests/fixtures/dispatch/execution-intent.json | 18 ++ 29 files changed, 1318 insertions(+), 142 deletions(-) create mode 100644 api/dispatch.md create mode 100644 hub-server/internal/repository/agent_direct_receipt.go create mode 100644 hub-server/internal/repository/agent_direct_receipt_test.go create mode 100644 hub-server/internal/service/agent/agent_direct_receipt_test.go create mode 100644 hub-server/internal/service/dispatch/edge_execution_intent.go create mode 100644 hub-server/internal/service/dispatch/execution_intent_test.go create mode 100644 hub-server/internal/service/dispatchsvc/agent_dispatch_callback_route.go create mode 100644 hub-server/internal/service/dispatchsvc/agent_dispatch_callback_route_test.go create mode 100644 hub-server/internal/service/dispatchsvc/direct_dispatch_helpers_test.go create mode 100644 hub-server/internal/service/dispatchsvc/direct_dispatch_recovery_test.go create mode 100644 hub-server/tests/integration/direct_receipt_test.go diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index faaa17756..8ef01033c 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -1067,6 +1067,7 @@ jobs: with: filters: | desktop: + - 'tests/fixtures/dispatch/**' - 'app/desktop/**' - 'app/shared/**' - 'app/workbench/**' @@ -1102,6 +1103,7 @@ jobs: - 'app/desktop/package.json' - '.github/workflows/checks.yml' go: + - 'tests/fixtures/dispatch/**' - 'hub-server/**' - 'edge-server/**' - 'pkg/**' @@ -1121,6 +1123,7 @@ jobs: # of the unconditional validate lane so a Go-only PR no longer # pays for pnpm install + coverage. frontend: + - 'tests/fixtures/dispatch/**' - 'app/desktop/**' - 'app/web/**' - 'app/shared/**' diff --git a/api/dispatch.md b/api/dispatch.md new file mode 100644 index 000000000..2cd7bcfe1 --- /dev/null +++ b/api/dispatch.md @@ -0,0 +1,34 @@ +# Hub→Edge 任务投递契约 + +> 最后更新：2026-09-07。事件索引见 [events.md](events.md)，REST 字段见 [openapi.yaml](openapi.yaml)。 + +本页定义跨通道执行输入、admission 回执和结果回传方；它不代表真实登录、模型 E2E、部署或进程恢复已验证。 + +### Hub→Edge `delivery_id` admission 契约（#2101 G2 / #2347） + +Hub 的 WS `agent.dispatch` 与 outbox HTTP POST `/v1/runs` 共享同一 `delivery_id`。Desktop 将事件中的 `delivery_id` / `deliveryId` 转交为 Edge 请求的 `deliveryId`；空值保留既有无去重路径。 + +- **原子接收**：先保留 pending claim；仅在 run 接收成功后提交含原 `runId` 的回执，失败或放弃则释放 claim，允许同 ID 重试。 +- **容量与有效期**：进程内缓存默认共容纳 4096 个 pending claim / accepted receipt。成功回执从提交起保留 5 分钟，也可因 LRU 容量压力被淘汰；重放不续期。pending claim 不因 TTL/LRU 被移除，避免首个请求未完成时重复执行。 +- **绑定**：非空 `hubTaskId` 是业务绑定。同一 Hub task 的 HTTP 本地线程与 Desktop 会话线程可以不同，但回执指向同一个原 run；无 `hubTaskId` 的遗留请求按 `projectId` / `threadId` 绑定。缓存内同 delivery ID 的不同绑定返回 409 `delivery_conflict`。 +- **成功重放**：有效回执返回原 run 的正常 202 envelope，包括 `data.runId`、`data.deduplicated: true` 和 `data.deliveryId`；不新建 run、timeline 或 executor。原 run 已删除则返回 404 `not_found`，不静默重建。每次请求先通过 capability 校验；重放还按原 run 实际 project/thread scope 复验。 +- **临时拒绝**：同 ID 正在接收，或容量被 pending claim 占满时，返回 503 `delivery_busy` + `Retry-After`（秒），而不是成功回执。409 `active_run_exists` 表示线程被其他活动 run 占用，也不能当作本次投递成功。Desktop 对这两类拒绝不 ACK、不 FAIL，由现有 Hub outbox 负责重投。 +- **ACK 与业务状态**：每次成功接收或重放都幂等重发 task / relay ACK，以修复丢失的确认；已建立的 run 映射、输出和 running/terminal 状态不因重复投递而回退，业务接收通知只触发一次。 +- **Hub task 接收证据**：非空 `hubTaskId` 在启动执行器前，与 `admissionState: pending` 一起写入 run；File/SQLite 在返回前同步保存。执行器返回后只允许转为 `accepted` 或带 `admissionErrorCode` 的 `rejected`，不改写执行状态。没有 Hub task 的本地/MCP 请求保留原路径。 +- **冷重放**：进程缓存丢失或 delivery ID 改变时，按 Hub task 查最新 attempt，并复验原 run scope。`accepted` 返回原 run，不重新执行；上次最终证据保存失败时只重试保存。已接收 run 后续 `failed` 不等于接收拒绝。 +- **拒绝与未决**：429 `too_many_concurrent_runs` 是执行器持有执行权之前的容量拒绝；503 `admission_persist_failed` 是证据保存失败（执行器可能已经接收），两者都不应 ACK/FAIL，由 Hub 重投向 Edge 核对。只有明确的容量拒绝或调用 Start 之前的保存失败，才允许创建新 attempt；普通 `executor_start_failed` 保持拒绝，不伪装成功。 +- **结果不明**：同一 Hub task 的当前接收者仍在处理时返回 503 `delivery_busy`。恢复后的 `pending`、未知 admission state、无 `startedAt` 的旧 run 返回 409 `admission_uncertain`；Desktop 保持待核对错误并通过现有通知提示用户，同一原因不重复提示，不 ACK/FAIL、不自动启动。旧 run 只有明确 `startedAt` 才能按原身份重放。`GET /v1/runs/{runId}` 暴露已记录的 `admissionState` / `admissionErrorCode` 供核对。 +- **恢复边界**：缓存不是恢复日志；持久化接收证据证明的是是否接收，不保证进程仍在运行，也不提供自动进程恢复。`queued`/`failed` 本身不能证明没有外部副作用。未决 admission 不参与终态自动清理；原 run 因显式删除或正常 retention 消失后，不宣称永久保留 Hub task 的幂等身份。 + + +### 跨投递通道的执行输入与回调归属 + +同一 Hub task 的直接 HTTP 与 Desktop WS/relay 投影共用 `tests/fixtures/dispatch/execution-intent.json`。执行输入不能因通道改变：model、reasoning/thinking、permission、workDir、system/append prompt、tools、config/ephemeral、messages/pinned 和 structured output schema 均保留；显式 `false` / `0` 不当作缺失值。未指定模型或工作目录时不替用户编造默认值，新的执行仍须通过 Edge workspace allowlist。 + +- **输入优先级**：运行参数取 `model_params` 的 snake/camel 别名；顶层 system prompt / tool whitelist 优先于嵌套回退。schema 的 JSON 候选（含对象、布尔值）或字符串统一转为 Edge 字符串，合法性仍由 Edge 校验；消息历史和 pinned 内容保留 role/content/timestamp。Hub `session_id` 是会话身份，不充当 runtime session；只有 model params 的显式 session_id/sessionId、continue/fork 才映射为运行时续接参数；只有 continue 缺省时 Edge 才按本地历史自动续接，显式 false 不被历史覆盖。 +- **允许的通道差异**：Hub HTTP 使用本地 project/thread，Desktop 使用对应会话线程；同 Hub task 的 admission 身份合流不变。直接通道请求 `callbackOwner: edge`，Desktop 请求 `callbackOwner: desktop`。Edge 在 pending admission 中保存首次选择，重放始终返回原 run 的真实 owner，不因新请求换人。 +- **执行前能力检查**：`GET /v1/health` 的 `capabilities.runCallbackOwnership` 证明该版本执行 owner 契约；`directHubCallbacks` 仅表明 Edge 已配置目的地和当前凭据，不证明远端连通或 token 有效。Hub direct 要求两者为 true，且 health 的 `edgeId` 与配置的真实 `device_id` 相同、该注册设备归属于任务 Agent 的邀请用户；Desktop 要求 ownership 支持（sidecar 无直接回调是正常）。缺失/未知能力时不发送 run POST，不以旧端会忽略新字段为兼容策略。Edge 仍在新接收时校验 direct callback 配置，未就绪返回 503 `callback_unavailable`，不创建 run。 +- **单一结果回传方**：edge-owned run 由 Edge 发 task ACK/stream/done/fail；Desktop 只更新本地 run 状态，不发第二套任务回调。desktop-owned run 的 Edge 不建立直接 callback 映射。relay delivery ACK 仍由接收 Desktop 负责，不能与任务结果回调混同。Hub direct 遇到 desktop-owned receipt 时只向该原设备投递，让 Desktop 恢复 bridge；不能改选邀请用户的另一台 Desktop。 +- **Team 控制边界**：typed route/result stream 是事件记录，不等同于调用 Team 的权威 route-decision 接口。带 Team 上下文的任务继续走 Desktop callback owner 与现有控制流程；Hub direct 在 POST 前退出，不把事件透传宣称为 Team 自动调度。 +- **direct 路由保留**：只有执行前能力检查失败、且尚未绑定设备的任务可以走普通 fallback。Hub 在 run POST 前持久化真实设备绑定；POST 超时、连接中断、错误或未知 owner 响应不证明未执行，现有 outbox 只能向原设备核对，不改投另一执行器。成功回执补记原 run ID；迟到回执不得回退已经 running/done/failed 的任务。 +- **旧回执与恢复**：现代请求碰到无法确定 owner 的旧 run，或收到缺失/非法 owner 的接收响应，按未决结果处理，不猜测、不自动重启。该契约不迁移正在执行的旧进程、不转交已接受任务的 owner，也不宣称跨重启恢复已完成。 diff --git a/api/events.md b/api/events.md index ac3f7fcce..26702f027 100644 --- a/api/events.md +++ b/api/events.md @@ -30,21 +30,9 @@ Hub/Edge 实时面均为 **at-least-once**：重连、离线队列、outbox、 > **`seq_id`（Hub WS per-conn）vs `seq`（Edge per-bus）**：Hub `seq_id` 是 `PushToConn` 在单连接上单调递增的投递序号，重连从 1 计、跨连接不可比；Edge `EventEnvelope.seq` 是事件总线上的 stream 单调序号，与持久化 `agent_run_events.event_seq` / `messages.seq_id` 对齐。REST 增量同步接口（`GET .../messages/sync?after_seq=`、`GET .../events?after_seq=`）的 `after_seq` 一律指**持久化表的内部 seq**（`messages.seq_id` 或 `agent_run_events.event_seq`），**不是** WS 帧的 `seq_id`。客户端不得用 WS `seq_id` 作为 REST 游标。 -### Hub→Edge `delivery_id` admission 契约（#2101 G2 / #2347） - -Hub 的 WS `agent.dispatch` 与 outbox HTTP POST `/v1/runs` 共享同一 `delivery_id`。Desktop 将事件中的 `delivery_id` / `deliveryId` 转交为 Edge 请求的 `deliveryId`；空值保留既有无去重路径。 - -- **原子接收**：先保留 pending claim；仅在 run 接收成功后提交含原 `runId` 的回执，失败或放弃则释放 claim，允许同 ID 重试。 -- **容量与有效期**：进程内缓存默认共容纳 4096 个 pending claim / accepted receipt。成功回执从提交起保留 5 分钟，也可因 LRU 容量压力被淘汰；重放不续期。pending claim 不因 TTL/LRU 被移除，避免首个请求未完成时重复执行。 -- **绑定**：非空 `hubTaskId` 是业务绑定。同一 Hub task 的 HTTP 本地线程与 Desktop 会话线程可以不同，但回执指向同一个原 run；无 `hubTaskId` 的遗留请求按 `projectId` / `threadId` 绑定。缓存内同 delivery ID 的不同绑定返回 409 `delivery_conflict`。 -- **成功重放**：有效回执返回原 run 的正常 202 envelope，包括 `data.runId`、`data.deduplicated: true` 和 `data.deliveryId`；不新建 run、timeline 或 executor。原 run 已删除则返回 404 `not_found`，不静默重建。每次请求先通过 capability 校验；重放还按原 run 实际 project/thread scope 复验。 -- **临时拒绝**：同 ID 正在接收，或容量被 pending claim 占满时，返回 503 `delivery_busy` + `Retry-After`（秒），而不是成功回执。409 `active_run_exists` 表示线程被其他活动 run 占用，也不能当作本次投递成功。Desktop 对这两类拒绝不 ACK、不 FAIL，由现有 Hub outbox 负责重投。 -- **ACK 与业务状态**：每次成功接收或重放都幂等重发 task / relay ACK，以修复丢失的确认；已建立的 run 映射、输出和 running/terminal 状态不因重复投递而回退，业务接收通知只触发一次。 -- **Hub task 接收证据**：非空 `hubTaskId` 在启动执行器前，与 `admissionState: pending` 一起写入 run；File/SQLite 在返回前同步保存。执行器返回后只允许转为 `accepted` 或带 `admissionErrorCode` 的 `rejected`，不改写执行状态。没有 Hub task 的本地/MCP 请求保留原路径。 -- **冷重放**：进程缓存丢失或 delivery ID 改变时，按 Hub task 查最新 attempt，并复验原 run scope。`accepted` 返回原 run，不重新执行；上次最终证据保存失败时只重试保存。已接收 run 后续 `failed` 不等于接收拒绝。 -- **拒绝与未决**：429 `too_many_concurrent_runs` 是执行器持有执行权之前的容量拒绝；503 `admission_persist_failed` 是证据保存失败（执行器可能已经接收），两者都不应 ACK/FAIL，由 Hub 重投向 Edge 核对。只有明确的容量拒绝或调用 Start 之前的保存失败，才允许创建新 attempt；普通 `executor_start_failed` 保持拒绝，不伪装成功。 -- **结果不明**：同一 Hub task 的当前接收者仍在处理时返回 503 `delivery_busy`。恢复后的 `pending`、未知 admission state、无 `startedAt` 的旧 run 返回 409 `admission_uncertain`；Desktop 保持待核对错误并通过现有通知提示用户，同一原因不重复提示，不 ACK/FAIL、不自动启动。旧 run 只有明确 `startedAt` 才能按原身份重放。`GET /v1/runs/{runId}` 暴露已记录的 `admissionState` / `admissionErrorCode` 供核对。 -- **恢复边界**：缓存不是恢复日志；持久化接收证据证明的是是否接收，不保证进程仍在运行，也不提供自动进程恢复。`queued`/`failed` 本身不能证明没有外部副作用。未决 admission 不参与终态自动清理；原 run 因显式删除或正常 retention 消失后，不宣称永久保留 Hub task 的幂等身份。 +### Hub→Edge 任务投递 + +同一任务共享 `delivery_id` / `hubTaskId`，消费端区分接收结果与执行状态，重放保留原 run 和 callback owner。执行输入、同步接收证据、能力预检与回调归属的完整合同见 [dispatch.md](dispatch.md)。 标签：**UPSERT by id**（稳定 id 合并，禁止第二行）；**idempotent on apply**（再应用不变）；**水位 / watermark**（只前进 `max`）；**ephemeral**（可丢可重，不写持久态）；**非幂等**（须自备去重或 REST）。 diff --git a/api/openapi.yaml b/api/openapi.yaml index 84d09d82e..ea7502a2a 100644 --- a/api/openapi.yaml +++ b/api/openapi.yaml @@ -107,13 +107,23 @@ paths: get: tags: [Foundation] operationId: getHealth - summary: Health check. + summary: Health and run callback capabilities. x-agenthub-phase: P0 x-agenthub-status: implemented x-agenthub-owner: Edge responses: "200": - $ref: "#/components/responses/Ok" + description: Healthy Edge with explicit callback ownership capabilities. + content: + application/json: + schema: + $ref: "#/components/schemas/EdgeHealth" + "503": + description: Degraded Edge; health body retains the same schema. + content: + application/json: + schema: + $ref: "#/components/schemas/EdgeHealth" /v1/events: get: @@ -755,6 +765,8 @@ paths: admission capacity and includes Retry-After. admission_persist_failed means admission evidence could not be saved; execution may already have been accepted. Retry to reconcile, not to force another start. + callback_unavailable means a direct callback owner is not configured; + no new run is created, and device routing is required. headers: Retry-After: description: Delay in seconds, present for delivery_busy and admission_persist_failed responses. @@ -8626,6 +8638,42 @@ components: type: string format: uuid description: Client-generated idempotency key. + EdgeHealth: + type: object + required: [status, version, edgeId, capabilities] + properties: + status: + type: string + http_status: + type: integer + version: + type: string + edgeId: + type: string + checks: + type: object + additionalProperties: true + capabilities: + type: object + required: [runCallbackOwnership, directHubCallbacks] + properties: + runCallbackOwnership: + type: boolean + description: The Edge enforces and persists per-run callback ownership. + directHubCallbacks: + type: boolean + description: Direct callbacks have a configured destination and current credential; no token or live reachability is exposed. + DispatchContextMessage: + type: object + required: [role, content] + properties: + role: + type: string + content: + type: string + timestamp: + type: string + format: date-time StartRunRequest: type: object properties: @@ -8673,11 +8721,11 @@ components: description: > Runtime permission policy hint. Only the claude adapter honours this field (mapped to Claude Code --permission-mode: default, acceptEdits, - bypassPermissions, plan, dontAsk). All other adapters ignore it; Codex + plan, dontAsk). All other adapters ignore it; Codex sandbox level is controlled separately via its own configuration. workDir: type: string - description: Working directory override for this run. Defaults to the Edge process profile workdir. + description: Explicit working directory required for a new run; must be within the Edge workspace allowlist. includePartial: type: boolean description: Include partial stream_event messages for real-time deltas (Claude Code --include-partial-messages). @@ -8710,6 +8758,24 @@ components: delivery admission. When set, receipts bind to this business task; without it, legacy delivery scope binds projectId/threadId. Callback ownership must still avoid duplicate Edge/Desktop output bridges. + callbackOwner: + type: string + enum: [edge, desktop] + description: Required for modern Hub delivery; selects the task callback reporter on first admission. Requires hubTaskId; replay returns the stored owner without reassignment. + trace_id: + type: string + description: Hub dispatch trace correlation identifier. + messages: + type: array + items: + $ref: "#/components/schemas/DispatchContextMessage" + description: Hub conversation history forwarded unchanged to the runtime context. + pinnedMessages: + type: array + items: + $ref: "#/components/schemas/DispatchContextMessage" + description: Pinned Hub context forwarded unchanged to the runtime. + AgentInfo: type: object required: [id, name, status] @@ -9608,6 +9674,10 @@ components: finishedAt: type: string format: date-time + callbackOwner: + type: string + enum: [edge, desktop] + description: Persisted task callback reporter. Replayed receipts retain the original owner; legacy absence is not an ownership grant. admissionState: type: string enum: [pending, accepted, rejected] diff --git a/docs/architecture/github-actions-ci-cd-policy.md b/docs/architecture/github-actions-ci-cd-policy.md index 7a5b9836f..8c10e6dfb 100644 --- a/docs/architecture/github-actions-ci-cd-policy.md +++ b/docs/architecture/github-actions-ci-cd-policy.md @@ -39,7 +39,7 @@ AgentHub 使用 Ubuntu 和 Windows 原生 runner 验证不同类别的问题： ## 并行与成本策略 1. `concurrency.cancel-in-progress` 取消同一 PR 的旧运行，连续 push 不排队浪费分钟。 -2. `changes` 是统一路径过滤器。Go、前端、移动端、设计 CSS 和视觉 shell 只在相关变更时启动；手动 dispatch 默认运行完整选择面。 +2. `changes` 是统一路径过滤器。Go、前端、移动端、设计 CSS 和视觉 shell 只在相关变更时启动；手动 dispatch 默认运行完整选择面。共享投递样例 `tests/fixtures/dispatch/**` 同时触发 Go、Desktop 与前端聚合，避免只改样例时漏跑跨端合同。 3. Ubuntu Go unit 使用两个不重叠 package shard；Hub 和 Edge 各自的 shard 测试与静态门禁 job 同时起跑，恒报 job 在两者结束后才合并覆盖率证据并断言 lane 结果（关键路径 = max(最慢 shard, 静态 job) + 覆盖率合并，而非两段相加）。 4. 前端 coverage 按 package matrix 并行；Windows 前端按 Desktop/Web matrix 并行。矩阵 `fail-fast: false` 保留所有失败根因，避免一个平台取消另一个平台的诊断。 5. `actions/setup-go`、`actions/setup-node` 的依赖缓存、pnpm store、Rust cache 和 Docker Buildx GHA cache 复用稳定输入；lockfile 或版本变化自然生成新缓存键。 diff --git a/edge-server/internal/api/callback_owner_test.go b/edge-server/internal/api/callback_owner_test.go index acb8f1753..a105856bb 100644 --- a/edge-server/internal/api/callback_owner_test.go +++ b/edge-server/internal/api/callback_owner_test.go @@ -142,3 +142,21 @@ func TestExecutionIntentFixtureReachesEdgeAdmission(t *testing.T) { }) } } + +func TestExecutionIntentExplicitContinueFalseSurvivesLocalHistory(t *testing.T) { + executor := &executionIntentRecorder{contexts: make(chan lifecycle.RunProcessContext, 1)} + server, h := newDeliveryTestServer(t, executor, nil) + defer server.Close() + if _, err := ensureStore(h).CreateItem(store.Item{ID: "prior-assistant", ProjectID: "proj_local", ThreadID: "thread_local", Role: "agent", Type: "agent_message", Content: "Previous local output"}); err != nil { + t.Fatal(err) + } + body := admissionRunBody(h.WorkspaceAllowlist[0], "continue-explicit", "continue-task", map[string]any{"callbackOwner": "desktop", "continue": false, "sessionId": "explicit-runtime-session"}) + result := postRunsRaw(t, server.URL, body) + if result.status != http.StatusAccepted { + t.Fatalf("explicit intent rejected: %d %#v", result.status, result.body) + } + context := <-executor.contexts + if context.ContinueLast || context.SessionID != "explicit-runtime-session" { + t.Fatalf("Edge changed explicit continuation intent: %#v", context) + } +} diff --git a/edge-server/internal/api/handlers_runs.go b/edge-server/internal/api/handlers_runs.go index bd20fbf6a..3e6ba0514 100644 --- a/edge-server/internal/api/handlers_runs.go +++ b/edge-server/internal/api/handlers_runs.go @@ -42,7 +42,7 @@ type runRequest struct { ModelMappingEnabled bool `json:"modelMappingEnabled"` ProviderFallbackEnabled bool `json:"providerFallbackEnabled"` SessionID string `json:"sessionId"` - Continue bool `json:"continue"` + Continue *bool `json:"continue"` Fork bool `json:"fork"` ReasoningEffort string `json:"reasoningEffort"` ThinkingMode string `json:"thinkingMode"` @@ -241,7 +241,7 @@ func (h *Handler) buildRunContext(run store.Run, req *runRequest) lifecycle.RunP AgentID: req.AgentID, Model: req.Model, SessionID: req.SessionID, - ContinueLast: req.Continue, + ContinueLast: req.Continue != nil && *req.Continue, ForkSession: req.Fork, ReasoningEffort: req.ReasoningEffort, ThinkingMode: req.ThinkingMode, @@ -350,8 +350,9 @@ func (h *Handler) PostRuns(w http.ResponseWriter, r *http.Request) { // Auto-detect continue: when the thread has prior assistant messages, // set ContinueLast = true so adapters can resume the conversation. // Each run creates a fresh CC conversation via --session-id. - if !req.Continue && threadHasAssistantHistory(repository, req.ThreadID) { - req.Continue = true + if req.Continue == nil && threadHasAssistantHistory(repository, req.ThreadID) { + resume := true + req.Continue = &resume } // WorkDir normalization previously happened inside validateRunCreateState; // the shared core also trims, this keeps the context builder consistent. @@ -388,7 +389,7 @@ func (h *Handler) PostRuns(w http.ResponseWriter, r *http.Request) { Model: req.Model, PermissionMode: req.PermissionMode, SessionID: req.SessionID, - ContinueLast: req.Continue, + ContinueLast: req.Continue != nil && *req.Continue, WorkDir: req.WorkDir, HubTaskID: req.HubTaskID, CallbackOwner: req.CallbackOwner, diff --git a/edge-server/internal/api/handlers_settings.go b/edge-server/internal/api/handlers_settings.go index 092e8018a..89b10a153 100644 --- a/edge-server/internal/api/handlers_settings.go +++ b/edge-server/internal/api/handlers_settings.go @@ -187,11 +187,15 @@ func (h *Handler) GetHealth(w http.ResponseWriter, r *http.Request) { httpStatus = http.StatusServiceUnavailable } + edgeID := h.EdgeDeviceID + if edgeID == "" { + edgeID = "local" + } writeJSON(w, httpStatus, map[string]any{ "status": status, "http_status": httpStatus, "version": "v1", - "edgeId": "local", + "edgeId": edgeID, "checks": checks, "capabilities": h.runCallbackCapabilities(), }) diff --git a/hub-server/internal/repository/agent_direct_receipt.go b/hub-server/internal/repository/agent_direct_receipt.go new file mode 100644 index 000000000..d35a44fa5 --- /dev/null +++ b/hub-server/internal/repository/agent_direct_receipt.go @@ -0,0 +1,66 @@ +package repository + +import ( + "errors" + "time" + + "gorm.io/gorm" + + "github.com/agenthub/hub-server/internal/model" +) + +// RecordPendingTaskDirectReceipt binds an accepted direct run to its real +// callback device. A fast ACK/done callback may already have advanced status; +// the late HTTP receipt must preserve that state and any existing binding. +func RecordPendingTaskDirectReceipt(db *gorm.DB, id, deviceID, runID string) error { + if deviceID == "" || runID == "" { + return errors.New("direct receipt requires device and run identities") + } + result := db.Model(&model.PendingAgentTask{}). + Where("id = ? AND (edge_device_id IS NULL OR edge_device_id = ?) AND (edge_run_id IS NULL OR edge_run_id = '' OR edge_run_id = ?)", id, deviceID, runID). + Updates(map[string]any{ + "edge_device_id": deviceID, + "edge_run_id": runID, + "status": gorm.Expr("CASE WHEN status = ? THEN ? ELSE status END", model.TaskStatusQueued, model.TaskStatusDispatched), + "dispatched_at": gorm.Expr("COALESCE(dispatched_at, ?)", time.Now()), + }) + if result.Error != nil { + return result.Error + } + if result.RowsAffected == 0 { + return gorm.ErrRecordNotFound + } + return nil +} + +// ReservePendingTaskDirectDevice persists the execution destination before the +// first run POST. If the response is lost, redelivery must stay on this device +// instead of starting the same logical task on an unrelated Desktop. +func ReservePendingTaskDirectDevice(db *gorm.DB, id, deviceID string) error { + if deviceID == "" { + return errors.New("direct dispatch requires a device identity") + } + result := db.Model(&model.PendingAgentTask{}). + Where("id = ? AND (edge_device_id IS NULL OR edge_device_id = ?)", id, deviceID). + Where("status IN ?", []string{model.TaskStatusQueued, model.TaskStatusDispatched, model.TaskStatusRunning}). + Update("edge_device_id", deviceID) + if result.Error != nil { + return result.Error + } + if result.RowsAffected == 0 { + return gorm.ErrRecordNotFound + } + return nil +} + +// DirectCallbackDeviceMatchesTask checks the user boundary of the callback path. +// A configured global Edge cannot report completion for another inviter's task. +func DirectCallbackDeviceMatchesTask(db *gorm.DB, id, deviceID string) (bool, error) { + var count int64 + err := db.Model(&model.PendingAgentTask{}). + Joins("JOIN agent_instances ON agent_instances.id = pending_agent_tasks.agent_instance_id"). + Joins("JOIN devices ON devices.id = ? AND devices.user_id = agent_instances.inviter_user_id", deviceID). + Where("pending_agent_tasks.id = ?", id). + Count(&count).Error + return count > 0, err +} diff --git a/hub-server/internal/repository/agent_direct_receipt_test.go b/hub-server/internal/repository/agent_direct_receipt_test.go new file mode 100644 index 000000000..716e203f9 --- /dev/null +++ b/hub-server/internal/repository/agent_direct_receipt_test.go @@ -0,0 +1,44 @@ +package repository + +import ( + "errors" + "testing" + "time" + + "github.com/agenthub/hub-server/internal/model" + "gorm.io/gorm" +) + +func TestRecordPendingTaskDirectReceiptPreservesProgressAndBinding(t *testing.T) { + for _, status := range []string{model.TaskStatusQueued, model.TaskStatusDispatched, model.TaskStatusRunning, model.TaskStatusDone, model.TaskStatusFailed} { + t.Run(status, func(t *testing.T) { + db := setupSQLite(t) + task := model.PendingAgentTask{ID: "receipt-task", AgentInstanceID: "agent-1", TriggeredByUserID: "user-1", TriggerMessageID: "msg-1", Status: status, ExpireAt: time.Now().Add(time.Hour)} + if err := db.Create(&task).Error; err != nil { + t.Fatal(err) + } + for n := 0; n < 2; n++ { + if err := RecordPendingTaskDirectReceipt(db, task.ID, "actual-edge-device", "actual-run"); err != nil { + t.Fatal(err) + } + } + got, err := GetPendingTaskByID(db, task.ID) + if err != nil { + t.Fatal(err) + } + wantStatus := status + if status == model.TaskStatusQueued { + wantStatus = model.TaskStatusDispatched + } + if got.Status != wantStatus || got.EdgeDeviceID != "actual-edge-device" || got.EdgeRunID != "actual-run" { + t.Fatalf("receipt changed execution state or identity: %#v", got) + } + if err := RecordPendingTaskDirectReceipt(db, task.ID, "different-device", "actual-run"); !errors.Is(err, gorm.ErrRecordNotFound) { + t.Fatalf("device conflict accepted: %v", err) + } + if err := RecordPendingTaskDirectReceipt(db, task.ID, "actual-edge-device", "different-run"); !errors.Is(err, gorm.ErrRecordNotFound) { + t.Fatalf("run conflict accepted: %v", err) + } + }) + } +} diff --git a/hub-server/internal/repository/helpers_test.go b/hub-server/internal/repository/helpers_test.go index eb9edfca6..164e393e5 100644 --- a/hub-server/internal/repository/helpers_test.go +++ b/hub-server/internal/repository/helpers_test.go @@ -176,7 +176,7 @@ func setupSQLite(t *testing.T) *gorm.DB { target_id TEXT, status TEXT NOT NULL, edge_run_id TEXT DEFAULT '', - edge_device_id TEXT DEFAULT '', + edge_device_id TEXT DEFAULT NULL, error_message TEXT DEFAULT '', model_params TEXT DEFAULT '{}', created_at DATETIME, diff --git a/hub-server/internal/service/agent/agent_direct_receipt_test.go b/hub-server/internal/service/agent/agent_direct_receipt_test.go new file mode 100644 index 000000000..3d66f00b1 --- /dev/null +++ b/hub-server/internal/service/agent/agent_direct_receipt_test.go @@ -0,0 +1,37 @@ +package agent + +import ( + "context" + "testing" + + "github.com/agenthub/hub-server/internal/model" + "github.com/agenthub/hub-server/internal/repository" +) + +func TestDirectReceiptRealDeviceAllowsOwnerCallback(t *testing.T) { + db, _ := newStreamPerfTestDB(t) + if err := db.Model(&model.PendingAgentTask{}).Where("id = ?", "task-1").Updates(map[string]any{"status": model.TaskStatusQueued, "edge_device_id": nil, "edge_run_id": ""}).Error; err != nil { + t.Fatal(err) + } + if err := repository.RecordPendingTaskDirectReceipt(db, "task-1", "real-edge-device", "direct-run"); err != nil { + t.Fatal(err) + } + svc := &Service{db: db} + if err := svc.HandleTaskAck(context.Background(), "user-1", "real-edge-device", "task-1", "direct-run"); err != nil { + t.Fatalf("legitimate Edge callback rejected after direct receipt: %v", err) + } + if err := svc.HandleTaskAck(context.Background(), "user-1", "other-device", "task-1", "direct-run"); err == nil { + t.Fatal("unbound callback device was allowed") + } + // A late repeat of the HTTP receipt cannot downgrade the callback's progress. + if err := repository.RecordPendingTaskDirectReceipt(db, "task-1", "real-edge-device", "direct-run"); err != nil { + t.Fatal(err) + } + task, err := repository.GetPendingTaskByID(db, "task-1") + if err != nil { + t.Fatal(err) + } + if task.Status != model.TaskStatusRunning { + t.Fatalf("late HTTP receipt downgraded ACK: %#v", task) + } +} diff --git a/hub-server/internal/service/dispatch/assemble_test.go b/hub-server/internal/service/dispatch/assemble_test.go index f3096e8d3..5380c3d07 100644 --- a/hub-server/internal/service/dispatch/assemble_test.go +++ b/hub-server/internal/service/dispatch/assemble_test.go @@ -332,10 +332,15 @@ func TestTargetBoundAndOutboxHelpers(t *testing.T) { } func TestEdgeHTTPPrepHelpers(t *testing.T) { + payload := Payload{ + TaskID: "task-1", + DeliveryID: "del-1", + AgentType: "claude-code", + Prompt: "hi", + SystemPrompt: "sys", + } parts, insecure, err := PrepareEdgeHTTPRequest( - DefaultEdgeHTTPURL, "secret", - "hi", "claude-code", "sys", "task-1", "del-1", - nil, nil, nil, "cap", + DefaultEdgeHTTPURL, "secret", payload, "cap", ) require.NoError(t, err) assert.False(t, insecure) @@ -346,9 +351,7 @@ func TestEdgeHTTPPrepHelpers(t *testing.T) { assert.Equal(t, time.Duration(EdgeHTTPClientTimeoutSeconds)*time.Second, parts.Timeout) parts, insecure, err = PrepareEdgeHTTPRequest( - "http://example.com:3210", "", - "hi", "claude-code", "", "task-1", "", - nil, nil, nil, "", + "http://example.com:3210", "", Payload{}, "", ) require.NoError(t, err) assert.True(t, insecure) diff --git a/hub-server/internal/service/dispatch/edge_execution_intent.go b/hub-server/internal/service/dispatch/edge_execution_intent.go new file mode 100644 index 000000000..00f417128 --- /dev/null +++ b/hub-server/internal/service/dispatch/edge_execution_intent.go @@ -0,0 +1,197 @@ +package dispatch + +import ( + "encoding/json" + "math" + "strings" +) + +// parseModelParams parses the Hub payload's model_params JSON string. Invalid +// or non-object input is treated as absent, matching the Desktop bridge's +// parseRecord fallback and ensuring no runtime/default fields are invented. +func parseModelParams(raw string) map[string]any { + raw = strings.TrimSpace(raw) + if raw == "" { + return nil + } + var parsed map[string]any + if err := json.Unmarshal([]byte(raw), &parsed); err != nil || parsed == nil { + return nil + } + return parsed +} + +// firstStringValue mirrors the Desktop getFirstString alias precedence: the +// first non-blank string wins and blank values fall through. +func firstStringValue(values ...any) string { + for _, value := range values { + if s, ok := value.(string); ok && strings.TrimSpace(s) != "" { + return s + } + } + return "" +} + +// firstBoolValue preserves explicit false values through a pointer, so false is +// not silently dropped by omitempty JSON encoding. +func firstBoolValue(values ...any) *bool { + for _, value := range values { + if b, ok := value.(bool); ok { + return &b + } + } + return nil +} + +// firstIntValue preserves explicit zero values through a pointer. JSON numbers +// arrive as float64; use the same safe-integer range as the JavaScript mapper. +func firstIntValue(values ...any) *int { + for _, value := range values { + switch n := value.(type) { + case float64: + if n == math.Trunc(n) && math.Abs(n) <= 9007199254740991 { + i := int(n) + return &i + } + case int: + if int64(n) >= -9007199254740991 && int64(n) <= 9007199254740991 { + i := n + return &i + } + } + } + return nil +} + +// firstStringArrayValue returns the first non-empty, all-string filtered array. +// Invalid JSON, empty arrays and arrays containing no valid strings fall through +// to the next alias exactly like the Desktop parseStringArray accessor. +func firstStringArrayValue(values ...any) []string { + for _, value := range values { + if parsed := parseStringArrayValue(value); parsed != nil { + return parsed + } + } + return nil +} + +func parseStringArrayValue(value any) []string { + var source any = value + if s, ok := value.(string); ok { + var parsed []any + if err := json.Unmarshal([]byte(s), &parsed); err != nil { + return nil + } + source = parsed + } + items, ok := source.([]any) + if !ok { + return nil + } + result := make([]string, 0, len(items)) + for _, item := range items { + if s, ok := item.(string); ok && strings.TrimSpace(s) != "" { + result = append(result, s) + } + } + if len(result) == 0 { + return nil + } + return result +} + +// firstStringRecordValue returns the first non-empty map containing string +// values. Non-string and invalid values do not broaden Edge permissions or +// inject runtime configuration. +func firstStringRecordValue(values ...any) map[string]string { + for _, value := range values { + if parsed := parseStringRecordValue(value); parsed != nil { + return parsed + } + } + return nil +} + +func parseStringRecordValue(value any) map[string]string { + var record map[string]any + switch typed := value.(type) { + case map[string]any: + record = typed + case string: + if err := json.Unmarshal([]byte(typed), &record); err != nil || record == nil { + return nil + } + default: + return nil + } + if len(record) == 0 { + return nil + } + result := make(map[string]string, len(record)) + for key, item := range record { + if s, ok := item.(string); ok { + result[key] = s + } + } + if len(result) == 0 { + return nil + } + return result +} + +// firstSchemaString returns the first schema value after converting both JSON +// strings and raw JSON objects/arrays to a string. Empty/null values fall +// through so absent schema never invents a default. +func firstSchemaString(values ...any) string { + for _, value := range values { + if s := schemaStringValue(value); s != "" { + return s + } + } + return "" +} + +func schemaStringValue(value any) string { + switch typed := value.(type) { + case nil: + return "" + case string: + return strings.TrimSpace(typed) + case json.RawMessage: + var raw any + if json.Unmarshal(typed, &raw) == nil && raw == nil { + return "" + } + var encoded string + if json.Unmarshal(typed, &encoded) == nil { + return strings.TrimSpace(encoded) + } + if json.Valid(typed) { + return strings.TrimSpace(string(typed)) + } + default: + encoded, err := json.Marshal(value) + if err == nil && json.Valid(encoded) { + return string(encoded) + } + } + return "" +} + +// RequiresDesktopTeamRouting keeps team orchestration on the existing Desktop +// bridge. Edge typed stream events are observational; they do not invoke the +// authoritative team route-decision endpoint. +func RequiresDesktopTeamRouting(payload Payload) bool { + if payload.TeamID != "" || payload.TeamRunID != "" { + return true + } + params := parseModelParams(payload.ModelParams) + var team map[string]any + switch value := params["agenthub_team_context"].(type) { + case map[string]any: + team = value + case string: + team = parseModelParams(value) + } + return firstStringValue(team["team_id"], team["teamId"], team["team_run_id"], team["teamRunId"]) != "" +} diff --git a/hub-server/internal/service/dispatch/edge_http_prep.go b/hub-server/internal/service/dispatch/edge_http_prep.go index 7147415f1..cd75f9b1a 100644 --- a/hub-server/internal/service/dispatch/edge_http_prep.go +++ b/hub-server/internal/service/dispatch/edge_http_prep.go @@ -1,7 +1,6 @@ package dispatch import ( - "encoding/json" "net/http" "time" ) @@ -21,19 +20,14 @@ type EdgeHTTPRequestParts struct { // err is set only on body marshal failure. func PrepareEdgeHTTPRequest( edgeURL, authToken string, - prompt, agentType, systemPrompt, hubTaskID, deliveryID string, - messages, pinned []Message, - outputSchema *json.RawMessage, + payload Payload, capabilityToken string, ) (parts EdgeHTTPRequestParts, insecure bool, err error) { edgeURL = ResolveEdgeHTTPURL(edgeURL) if IsInsecureNonLoopbackEdge(edgeURL) { return EdgeHTTPRequestParts{EdgeURL: edgeURL}, true, nil } - body, err := MarshalEdgeRunRequest( - prompt, agentType, systemPrompt, hubTaskID, deliveryID, - messages, pinned, outputSchema, - ) + body, err := MarshalEdgeRunRequest(payload) if err != nil { return EdgeHTTPRequestParts{EdgeURL: edgeURL}, false, err } diff --git a/hub-server/internal/service/dispatch/edge_request.go b/hub-server/internal/service/dispatch/edge_request.go index f7586a060..1492730f5 100644 --- a/hub-server/internal/service/dispatch/edge_request.go +++ b/hub-server/internal/service/dispatch/edge_request.go @@ -2,57 +2,102 @@ package dispatch import "encoding/json" +// EdgeCallbackOwner is the fixed callback owner used when Hub dispatches a run +// directly to Edge. Edge is responsible for enforcing/responding to this route. +const EdgeCallbackOwner = "edge" + // EdgeRunRequest is the pure JSON body POSTed to Edge /v1/runs. // Kept free of service orchestration so HTTP dispatch can build it without // embedding field literals in agent_dispatch.go. type EdgeRunRequest struct { - ProjectID string `json:"projectId"` - ThreadID string `json:"threadId"` - Prompt string `json:"prompt"` - AgentID string `json:"agentId,omitempty"` - Model string `json:"model,omitempty"` - SystemPrompt string `json:"systemPrompt,omitempty"` - HubTaskID string `json:"hubTaskId"` - DeliveryID string `json:"deliveryId,omitempty"` - Messages []Message `json:"messages,omitempty"` - PinnedMessages []Message `json:"pinnedMessages,omitempty"` - StructuredOutputSchema string `json:"structuredOutputSchema,omitempty"` + ProjectID string `json:"projectId"` + ThreadID string `json:"threadId"` + CallbackOwner string `json:"callbackOwner"` + Prompt string `json:"prompt"` + AgentID string `json:"agentId,omitempty"` + Model string `json:"model,omitempty"` + SessionID string `json:"sessionId,omitempty"` + Continue *bool `json:"continue,omitempty"` + Fork *bool `json:"fork,omitempty"` + ReasoningEffort string `json:"reasoningEffort,omitempty"` + ThinkingMode string `json:"thinkingMode,omitempty"` + MaxThinkingTokens *int `json:"maxThinkingTokens,omitempty"` + PermissionMode string `json:"permissionMode,omitempty"` + WorkDir string `json:"workDir,omitempty"` + IncludePartial *bool `json:"includePartial,omitempty"` + StructuredOutputSchema string `json:"structuredOutputSchema,omitempty"` + SystemPrompt string `json:"systemPrompt,omitempty"` + AppendSystemPrompt string `json:"appendSystemPrompt,omitempty"` + AllowedTools []string `json:"allowedTools,omitempty"` + ConfigOverrides map[string]string `json:"configOverrides,omitempty"` + Ephemeral *bool `json:"ephemeral,omitempty"` + HubTaskID string `json:"hubTaskId"` + TraceID string `json:"trace_id,omitempty"` + DeliveryID string `json:"deliveryId,omitempty"` + Messages []Message `json:"messages,omitempty"` + PinnedMessages []Message `json:"pinnedMessages,omitempty"` } -// BuildEdgeRunRequest maps dispatch payload fields into an Edge run request. -// agentType is normalized via NormalizeRuntimeAgentType; model defaults to "claude". -func BuildEdgeRunRequest( - prompt, agentType, systemPrompt, hubTaskID, deliveryID string, - messages, pinned []Message, - outputSchema *json.RawMessage, -) EdgeRunRequest { - req := EdgeRunRequest{ - ProjectID: LocalProjectID, - ThreadID: LocalThreadID, - Prompt: prompt, - AgentID: NormalizeRuntimeAgentType(agentType), - Model: "claude", - SystemPrompt: systemPrompt, - HubTaskID: hubTaskID, - DeliveryID: deliveryID, - Messages: messages, - PinnedMessages: pinned, +// BuildEdgeRunRequest maps a dispatch payload into an Edge run request. +// It preserves the Desktop projection aliases/priority for model params, +// workDir/tool allowlist/schema, while keeping Hub Task/Session identity and +// the local transport project/thread scope distinct. +func BuildEdgeRunRequest(payload Payload) EdgeRunRequest { + params := parseModelParams(payload.ModelParams) + + var outputSchema any + if payload.OutputSchema != nil { + outputSchema = *payload.OutputSchema } - if outputSchema != nil && len(*outputSchema) > 0 { - req.StructuredOutputSchema = string(*outputSchema) + + return EdgeRunRequest{ + ProjectID: LocalProjectID, + ThreadID: LocalThreadID, + CallbackOwner: EdgeCallbackOwner, + Prompt: payload.Prompt, + AgentID: NormalizeRuntimeAgentType(payload.AgentType), + Model: firstStringValue(params["model"]), + SessionID: firstStringValue(params["session_id"], params["sessionId"]), + Continue: firstBoolValue(params["continue"]), + Fork: firstBoolValue(params["fork"]), + ReasoningEffort: firstStringValue(params["reasoning_effort"], params["reasoningEffort"]), + ThinkingMode: firstStringValue(params["thinking_mode"], params["thinkingMode"]), + MaxThinkingTokens: firstIntValue(params["max_thinking_tokens"], params["maxThinkingTokens"]), + PermissionMode: firstStringValue(params["permission_mode"], params["permissionMode"]), + WorkDir: firstStringValue(params["work_dir"], params["workDir"]), + IncludePartial: firstBoolValue(params["include_partial"], params["includePartial"]), + StructuredOutputSchema: firstSchemaString( + params["structured_output_schema"], + params["structuredOutputSchema"], + outputSchema, + ), + SystemPrompt: firstStringValue( + payload.SystemPrompt, + params["system_prompt"], + params["systemPrompt"], + ), + AppendSystemPrompt: firstStringValue(params["append_system_prompt"], params["appendSystemPrompt"]), + AllowedTools: firstStringArrayValue( + payload.ToolWhitelist, + params["tool_allowlist"], + params["allowed_tools"], + params["allowedTools"], + ), + ConfigOverrides: firstStringRecordValue( + params["config_overrides"], + params["configOverrides"], + ), + Ephemeral: firstBoolValue(params["ephemeral"]), + HubTaskID: payload.TaskID, + TraceID: payload.TraceID, + DeliveryID: payload.DeliveryID, + Messages: payload.Messages, + PinnedMessages: payload.PinnedMessages, } - return req } // MarshalEdgeRunRequest builds and JSON-marshals an Edge /v1/runs body. // HTTP client construction stays orchestration-side. -func MarshalEdgeRunRequest( - prompt, agentType, systemPrompt, hubTaskID, deliveryID string, - messages, pinned []Message, - outputSchema *json.RawMessage, -) ([]byte, error) { - return json.Marshal(BuildEdgeRunRequest( - prompt, agentType, systemPrompt, hubTaskID, deliveryID, - messages, pinned, outputSchema, - )) +func MarshalEdgeRunRequest(payload Payload) ([]byte, error) { + return json.Marshal(BuildEdgeRunRequest(payload)) } diff --git a/hub-server/internal/service/dispatch/execution_intent_test.go b/hub-server/internal/service/dispatch/execution_intent_test.go new file mode 100644 index 000000000..aeb96a771 --- /dev/null +++ b/hub-server/internal/service/dispatch/execution_intent_test.go @@ -0,0 +1,117 @@ +package dispatch + +import ( + "encoding/json" + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type executionIntentFixture struct { + Version int `json:"version"` + Cases []struct { + Name string `json:"name"` + Payload Payload `json:"payload"` + Expected map[string]any `json:"expectedIntent"` + } `json:"cases"` +} + +func TestExecutionIntentFixtureProjection(t *testing.T) { + raw, err := os.ReadFile("../../../../tests/fixtures/dispatch/execution-intent.json") + require.NoError(t, err) + + var fixture executionIntentFixture + require.NoError(t, json.Unmarshal(raw, &fixture)) + require.NotEmpty(t, fixture.Cases) + + for _, c := range fixture.Cases { + t.Run(c.Name, func(t *testing.T) { + req := BuildEdgeRunRequest(c.Payload) + assert.Equal(t, LocalProjectID, req.ProjectID) + assert.Equal(t, LocalThreadID, req.ThreadID) + assert.Equal(t, "edge", req.CallbackOwner) + + body, err := json.Marshal(req) + require.NoError(t, err) + got := map[string]any{} + require.NoError(t, json.Unmarshal(body, &got)) + delete(got, "projectId") + delete(got, "threadId") + delete(got, "callbackOwner") + + want := cloneMap(c.Expected) + assertJSONFieldEqual(t, got, want, "structuredOutputSchema") + delete(got, "structuredOutputSchema") + delete(want, "structuredOutputSchema") + assert.Equal(t, want, got) + }) + } +} + +func TestPrepareEdgeHTTPRequestUsesPayloadProjection(t *testing.T) { + raw, err := os.ReadFile("../../../../tests/fixtures/dispatch/execution-intent.json") + require.NoError(t, err) + var fixture executionIntentFixture + require.NoError(t, json.Unmarshal(raw, &fixture)) + require.NotEmpty(t, fixture.Cases) + + parts, insecure, err := PrepareEdgeHTTPRequest( + DefaultEdgeHTTPURL, "auth-token", fixture.Cases[0].Payload, "cap-token", + ) + require.NoError(t, err) + assert.False(t, insecure) + assert.Equal(t, DefaultEdgeHTTPURL+"/v1/runs", parts.RunsURL) + assert.Equal(t, "Bearer auth-token", parts.Headers.Get("Authorization")) + assert.Equal(t, "cap-token", parts.Headers.Get(CapabilityTokenHeader)) + + got := map[string]any{} + require.NoError(t, json.Unmarshal(parts.Body, &got)) + assert.Equal(t, "edge", got["callbackOwner"]) + assert.Equal(t, LocalProjectID, got["projectId"]) + assert.Equal(t, LocalThreadID, got["threadId"]) +} + +func cloneMap(in map[string]any) map[string]any { + out := make(map[string]any, len(in)) + for k, v := range in { + out[k] = v + } + return out +} + +func assertJSONFieldEqual(t *testing.T, got, want map[string]any, key string) { + t.Helper() + gotValue, gotOK := got[key] + wantValue, wantOK := want[key] + if !gotOK && !wantOK { + return + } + require.True(t, gotOK, "%s missing from projection", key) + require.True(t, wantOK, "%s missing from expected intent", key) + gotString, ok := gotValue.(string) + require.True(t, ok, "%s must project as a string", key) + wantString, ok := wantValue.(string) + require.True(t, ok, "%s expected as a string", key) + + var gotJSON, wantJSON any + require.NoError(t, json.Unmarshal([]byte(gotString), &gotJSON)) + require.NoError(t, json.Unmarshal([]byte(wantString), &wantJSON)) + assert.Equal(t, wantJSON, gotJSON, "%s must match after JSON semantic comparison", key) +} + +func TestTeamDispatchRequiresAuthoritativeDesktopBridge(t *testing.T) { + for _, payload := range []Payload{ + {TeamRunID: "team-run"}, + {ModelParams: `{"agenthub_team_context":{"team_id":"team","team_run_id":"run"}}`}, + {ModelParams: `{"agenthub_team_context":"{\"teamId\":\"team\",\"teamRunId\":\"run\"}"}`}, + } { + if !RequiresDesktopTeamRouting(payload) { + t.Fatalf("team control route was treated as output-only: %#v", payload) + } + } + if RequiresDesktopTeamRouting(Payload{ModelParams: `{"work_dir":"/workspace/project"}`}) { + t.Fatal("ordinary task unnecessarily requires team routing") + } +} diff --git a/hub-server/internal/service/dispatch/helpers_extra_test.go b/hub-server/internal/service/dispatch/helpers_extra_test.go index 4ece26300..80d587eab 100644 --- a/hub-server/internal/service/dispatch/helpers_extra_test.go +++ b/hub-server/internal/service/dispatch/helpers_extra_test.go @@ -81,29 +81,43 @@ func TestMapPinnedMessagesSkipsEmpty(t *testing.T) { func TestBuildEdgeRunRequest(t *testing.T) { schema := json.RawMessage(`{"type":"object"}`) - req := BuildEdgeRunRequest( - "hello", - "claude-code", - "sys", - "task-1", - "deliv-1", - []Message{{Role: "user", Content: "hi", Timestamp: "2026-01-01T00:00:00Z"}}, - nil, - &schema, - ) + payload := Payload{ + TaskID: "task-1", + DeliveryID: "deliv-1", + AgentType: "claude-code", + SessionID: "conversation-1", + Prompt: "hello", + SystemPrompt: "sys", + ModelParams: `{"model":"selected","work_dir":"/workspace","include_partial":false,"max_thinking_tokens":0}`, + ToolWhitelist: `["Read"]`, + Messages: []Message{{Role: "user", Content: "hi", Timestamp: "2026-01-01T00:00:00Z"}}, + OutputSchema: &schema, + } + req := BuildEdgeRunRequest(payload) assert.Equal(t, LocalProjectID, req.ProjectID) assert.Equal(t, LocalThreadID, req.ThreadID) + assert.Equal(t, EdgeCallbackOwner, req.CallbackOwner) assert.Equal(t, "claude-code", req.AgentID) - assert.Equal(t, "claude", req.Model) + assert.Equal(t, "selected", req.Model) assert.Equal(t, "hello", req.Prompt) assert.Equal(t, "sys", req.SystemPrompt) assert.Equal(t, "task-1", req.HubTaskID) assert.Equal(t, "deliv-1", req.DeliveryID) assert.Equal(t, `{"type":"object"}`, req.StructuredOutputSchema) + assert.Equal(t, []string{"Read"}, req.AllowedTools) + assert.Equal(t, "/workspace", req.WorkDir) + require.NotNil(t, req.IncludePartial) + assert.False(t, *req.IncludePartial) + require.NotNil(t, req.MaxThinkingTokens) + assert.Equal(t, 0, *req.MaxThinkingTokens) + // Hub session_id is conversation identity, not the Edge runtime session. + assert.Equal(t, "", req.SessionID) require.Len(t, req.Messages, 1) - // nil / empty schema → no structured field - req2 := BuildEdgeRunRequest("p", "codex", "", "t", "", nil, nil, nil) + // nil / empty schema → no structured field; no hardcoded model. + req2 := BuildEdgeRunRequest(Payload{TaskID: "t", AgentType: "codex", Prompt: "p"}) assert.Equal(t, "", req2.StructuredOutputSchema) assert.Equal(t, "codex", req2.AgentID) + assert.Equal(t, "", req2.Model) + assert.Equal(t, EdgeCallbackOwner, req2.CallbackOwner) } diff --git a/hub-server/internal/service/dispatch/residual_test.go b/hub-server/internal/service/dispatch/residual_test.go index 6d3a9cd26..c0b1dd769 100644 --- a/hub-server/internal/service/dispatch/residual_test.go +++ b/hub-server/internal/service/dispatch/residual_test.go @@ -292,9 +292,18 @@ func TestFinalizePayloadWithDelivery(t *testing.T) { func TestMarshalEdgeRunRequest(t *testing.T) { schema := json.RawMessage(`{"type":"object"}`) - body, err := MarshalEdgeRunRequest("hi", "claude-code", "sys", "task-1", "del-1", nil, nil, &schema) + payload := Payload{ + TaskID: "task-1", + DeliveryID: "del-1", + AgentType: "claude-code", + Prompt: "hi", + SystemPrompt: "sys", + ModelParams: `{"model":"selected"}`, + OutputSchema: &schema, + } + body, err := MarshalEdgeRunRequest(payload) require.NoError(t, err) - want, err := json.Marshal(BuildEdgeRunRequest("hi", "claude-code", "sys", "task-1", "del-1", nil, nil, &schema)) + want, err := json.Marshal(BuildEdgeRunRequest(payload)) require.NoError(t, err) assert.JSONEq(t, string(want), string(body)) } diff --git a/hub-server/internal/service/dispatchsvc/agent_dispatch.go b/hub-server/internal/service/dispatchsvc/agent_dispatch.go index cdd432c9d..02266a91e 100644 --- a/hub-server/internal/service/dispatchsvc/agent_dispatch.go +++ b/hub-server/internal/service/dispatchsvc/agent_dispatch.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "log/slog" + "strings" "time" "gorm.io/gorm" @@ -229,21 +230,37 @@ func (s *DispatchService) DispatchTask(ctx context.Context, task *model.PendingA // instance; on a miss it falls back to the inviter desktop connection or the // offline target queue (exactly the previous inline route branch). func (s *DispatchService) dispatchRouteHTTP(ctx context.Context, task *model.PendingAgentTask, ai *model.AgentInstance, dp *dispatchPayload, payload []byte, deliveryID string, cacheClient dispatchCache) { - if dispatch.IsHTTPEdgeDispatchSuccess(s.dispatchToEdgeHTTP(ctx, task, dp)) { - if err := repository.UpdatePendingTaskDispatched(s.db, task.ID, dispatch.SyntheticHTTPEdgeDeviceID); !dispatch.RepoUpdateSucceeded(err) { + result := s.dispatchToEdgeHTTP(ctx, task, dp) + if result.RunID != "" { + if err := repository.RecordPendingTaskDirectReceipt(s.db, task.ID, task.EdgeDeviceID, result.RunID); err != nil { slog.Error(dispatch.DispatchLogHTTPMarkFailed, "task_id", task.ID, "error", err) + return } - s.markDeliverySentPlan(ctx, dispatch.PlanLiveDispatchMark(deliveryID), deliveryID, task.ID) + task.EdgeRunID = result.RunID + if result.CallbackOwner == "edge" { + s.markDeliverySentPlan(ctx, dispatch.PlanLiveDispatchMark(deliveryID), deliveryID, task.ID) + return + } + // The original run belongs to Desktop. Restore only that device's + // bridge, never the inviter's other connected executor. + boundPayload, err := dispatch.MarshalPayload(*dp) + if err != nil { + slog.Error(dispatch.DispatchLogPayloadMarshalFailed, "task_id", task.ID, "error", err) + return + } + s.dispatchRouteTargetBound(ctx, task, ai, boundPayload, deliveryID, cacheClient) return } - // HTTP miss: fall through to inviter desktop / offline. + if !result.SafeToFallback { + return // uncertain admission retains its durable device reservation + } + // No execution was attempted and no device was reserved; normal fallback. connID, err := cacheClient.GetRoute(ctx, ai.InviterUserID, dispatch.DesktopDeviceType) if dispatch.IsUnboundInviterDesktopRoute(dispatch.ClassifyUnboundFallbackRoute(connID, dispatch.ManagerPortAvailable(s.mgr != nil), err)) { s.dispatchUnboundInviterDesktop(ctx, task, ai, payload, deliveryID, connID, cacheClient) return } s.pushPendingTaskOffline(ctx, ai.InviterUserID, task.ID, payload, "unbound_only", dispatch.DispatchLogOfflinePushFailed, cacheClient) - // Offline-only path: outbox retains ownership until Edge ack/stream (#1031). s.markDeliverySentPlan(ctx, dispatch.PlanOfflineDispatchMark(deliveryID), deliveryID, task.ID) } @@ -474,13 +491,31 @@ func (s *DispatchService) getPendingTaskForRedelivery(ctx context.Context, taskI // Outbox MarkDeliverySent after success remains on DeliveryOutbox RedispatchDelivery. func (s *DispatchService) retryDispatchToTarget(ctx context.Context, task *pendingTaskSnapshot, dp dispatchPayload, newPayload []byte, rec redispatchTarget) error { minimalTask := dispatch.MinimalPendingTaskForHTTP(*task) - preferDevice := dispatch.RedeliveryPreferDeviceRoute(task.EdgeDeviceID) - if dispatch.ClassifyRedeliveryPrimaryRoute(task.TargetID, task.EdgeDeviceID) == dispatch.RouteHTTP { - if edgeRunID := s.dispatchToEdgeHTTP(ctx, minimalTask, &dp); dispatch.IsHTTPEdgeDispatchSuccess(edgeRunID) { - slog.Info(dispatch.RedispatchLogHTTPSucceeded, - "delivery_id", rec.DeliveryID, "task_id", rec.TaskID, "edge_run_id", edgeRunID) - return nil + reservedDirectDevice := task.TargetID == "" && task.EdgeDeviceID != "" && task.EdgeDeviceID == strings.TrimSpace(s.edgeCfg.DeviceID) + if reservedDirectDevice || dispatch.ClassifyRedeliveryPrimaryRoute(task.TargetID, task.EdgeDeviceID) == dispatch.RouteHTTP { + result := s.dispatchToEdgeHTTP(ctx, minimalTask, &dp) + task.EdgeDeviceID = minimalTask.EdgeDeviceID + if result.RunID != "" { + if err := repository.RecordPendingTaskDirectReceipt(s.db, task.ID, task.EdgeDeviceID, result.RunID); err != nil { + return err + } + if result.CallbackOwner == "edge" { + slog.Info(dispatch.RedispatchLogHTTPSucceeded, + "delivery_id", rec.DeliveryID, "task_id", rec.TaskID, "edge_run_id", result.RunID) + return nil + } + } else if !result.SafeToFallback { + return errors.New("direct admission is unconfirmed; preserve device binding") + } + } + preferDevice := dispatch.RedeliveryPreferDeviceRoute(task.EdgeDeviceID) + if preferDevice { + dp.EdgeDeviceID = task.EdgeDeviceID + var err error + newPayload, err = dispatch.MarshalPayload(dp) + if err != nil { + return err } } diff --git a/hub-server/internal/service/dispatchsvc/agent_dispatch_callback_route.go b/hub-server/internal/service/dispatchsvc/agent_dispatch_callback_route.go new file mode 100644 index 000000000..137014634 --- /dev/null +++ b/hub-server/internal/service/dispatchsvc/agent_dispatch_callback_route.go @@ -0,0 +1,60 @@ +package dispatchsvc + +import ( + "context" + "encoding/json" + "io" + "net/http" + "strings" + + "github.com/agenthub/hub-server/internal/service/dispatch" +) + +// Direct execution requires a runtime that enforces callback ownership and has +// a configured callback path. A normal health response from an old Edge is not +// sufficient: it could accept work without anyone responsible for the result. +func (s *DispatchService) directCallbackRouteReady(ctx context.Context, parts dispatch.EdgeHTTPRequestParts) bool { + deviceID := strings.TrimSpace(s.edgeCfg.DeviceID) + if deviceID == "" { + return false + } + request, err := http.NewRequestWithContext(ctx, http.MethodGet, parts.EdgeURL+"/v1/health", nil) + if err != nil { + return false + } + request.Header = parts.Headers.Clone() + response, err := s.edgeClient.Do(request) + if err != nil { + return false + } + defer response.Body.Close() + if response.StatusCode != http.StatusOK { + return false + } + var health struct { + EdgeID string `json:"edgeId"` + Capabilities struct { + RunCallbackOwnership bool `json:"runCallbackOwnership"` + DirectHubCallbacks bool `json:"directHubCallbacks"` + } `json:"capabilities"` + } + if json.NewDecoder(io.LimitReader(response.Body, dispatch.EdgeHTTPResponseBodyLimit)).Decode(&health) != nil { + return false + } + return health.EdgeID == deviceID && health.Capabilities.RunCallbackOwnership && health.Capabilities.DirectHubCallbacks +} + +func edgeDispatchReceiptOwner(body []byte) string { + var response struct { + Data struct { + CallbackOwner string `json:"callbackOwner"` + } `json:"data"` + } + if json.Unmarshal(body, &response) == nil { + switch response.Data.CallbackOwner { + case "edge", "desktop": + return response.Data.CallbackOwner + } + } + return "" +} diff --git a/hub-server/internal/service/dispatchsvc/agent_dispatch_callback_route_test.go b/hub-server/internal/service/dispatchsvc/agent_dispatch_callback_route_test.go new file mode 100644 index 000000000..fa6128622 --- /dev/null +++ b/hub-server/internal/service/dispatchsvc/agent_dispatch_callback_route_test.go @@ -0,0 +1,76 @@ +package dispatchsvc + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "github.com/agenthub/hub-server/internal/config" + "github.com/agenthub/hub-server/internal/model" + "github.com/agenthub/hub-server/internal/outboundhttp" +) + +func TestDirectCallbackRouteFailsClosedBeforeExecution(t *testing.T) { + cases := []struct { + name, health, receipt, params string + wantPosts int + wantRun string + }{ + {"team-routing-needs-desktop", `{"edgeId":"fixture-edge-device","capabilities":{"runCallbackOwnership":true,"directHubCallbacks":true}}`, "", `{"work_dir":"/workspace/project","agenthub_team_context":{"team_id":"team-1","team_run_id":"team-run-1"}}`, 0, ""}, + {"old-edge", `{"status":"ok"}`, "", `{"work_dir":"/workspace/project"}`, 0, ""}, + {"sidecar-no-callback", `{"edgeId":"fixture-edge-device","capabilities":{"runCallbackOwnership":true,"directHubCallbacks":false}}`, "", `{"work_dir":"/workspace/project"}`, 0, ""}, + {"missing-owner-enforcement", `{"edgeId":"fixture-edge-device","capabilities":{"directHubCallbacks":true}}`, "", `{"work_dir":"/workspace/project"}`, 0, ""}, + {"missing-workspace", `{"edgeId":"fixture-edge-device","capabilities":{"runCallbackOwnership":true,"directHubCallbacks":true}}`, "", "{}", 0, ""}, + {"edge-owned", `{"edgeId":"fixture-edge-device","capabilities":{"runCallbackOwnership":true,"directHubCallbacks":true}}`, `{"code":"ok","data":{"runId":"run-owned","callbackOwner":"edge"}}`, `{"work_dir":"/workspace/project","model":"chosen-model"}`, 1, "run-owned"}, + {"desktop-owned-replay", `{"edgeId":"fixture-edge-device","capabilities":{"runCallbackOwnership":true,"directHubCallbacks":true}}`, `{"code":"ok","data":{"runId":"run-owned","callbackOwner":"desktop"}}`, `{"work_dir":"/workspace/project"}`, 1, "run-owned"}, + {"unknown-receipt-owner", `{"edgeId":"fixture-edge-device","capabilities":{"runCallbackOwnership":true,"directHubCallbacks":true}}`, `{"code":"ok","data":{"runId":"run-owned"}}`, `{"work_dir":"/workspace/project"}`, 1, ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var posts atomic.Int32 + bodies := make(chan map[string]any, 1) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if r.URL.Path == "/v1/health" && r.Method == http.MethodGet { + _, _ = w.Write([]byte(tc.health)) + return + } + if r.URL.Path != "/v1/runs" || r.Method != http.MethodPost { + w.WriteHeader(http.StatusNotFound) + return + } + posts.Add(1) + var body map[string]any + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } + bodies <- body + w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(tc.receipt)) + })) + defer server.Close() + service := NewDispatchService(nil, nil, nil, nil, nil, nil, config.EdgeDispatchConfig{DeviceID: "fixture-edge-device", URL: server.URL, Timeout: time.Second}, outboundhttp.NewClient(time.Second), "") + task := &model.PendingAgentTask{ID: "task-owner"} + service.db = newDirectDispatchDB(t, task) + payload := dispatchPayload{TaskID: task.ID, AgentType: "codex", Prompt: "fixture task", ModelParams: tc.params} + got := service.dispatchToEdgeHTTP(context.Background(), task, &payload) + if got.RunID != tc.wantRun || int(posts.Load()) != tc.wantPosts { + t.Fatalf("run=%q posts=%d; want %q/%d", got.RunID, posts.Load(), tc.wantRun, tc.wantPosts) + } + if got.SafeToFallback != (tc.wantPosts == 0) { + t.Fatalf("fallback=%v after %d POSTs", got.SafeToFallback, posts.Load()) + } + if tc.wantPosts > 0 { + body := <-bodies + if body["workDir"] != "/workspace/project" || body["callbackOwner"] != "edge" { + t.Fatalf("direct request lost intent or ownership: %#v", body) + } + } + }) + } +} diff --git a/hub-server/internal/service/dispatchsvc/agent_dispatch_edge_http.go b/hub-server/internal/service/dispatchsvc/agent_dispatch_edge_http.go index ddb4756a4..9eb0e4848 100644 --- a/hub-server/internal/service/dispatchsvc/agent_dispatch_edge_http.go +++ b/hub-server/internal/service/dispatchsvc/agent_dispatch_edge_http.go @@ -6,6 +6,7 @@ import ( "io" "log/slog" "net/http" + "strings" "time" "github.com/agenthub/pkg/outboundmetrics" @@ -13,18 +14,41 @@ import ( "github.com/agenthub/hub-server/internal/metrics" "github.com/agenthub/hub-server/internal/model" + "github.com/agenthub/hub-server/internal/repository" "github.com/agenthub/hub-server/internal/service/dispatch" ) -func (s *DispatchService) dispatchToEdgeHTTP(ctx context.Context, task *model.PendingAgentTask, dp *dispatchPayload) string { +type edgeHTTPDispatchResult struct { + RunID string + CallbackOwner string + SafeToFallback bool +} + +func (s *DispatchService) dispatchToEdgeHTTP(ctx context.Context, task *model.PendingAgentTask, dp *dispatchPayload) edgeHTTPDispatchResult { + miss := edgeHTTPDispatchResult{SafeToFallback: task == nil || task.EdgeDeviceID == ""} + deviceID := strings.TrimSpace(s.edgeCfg.DeviceID) + if task != nil && task.EdgeDeviceID != "" && task.EdgeDeviceID != deviceID { + return edgeHTTPDispatchResult{} + } // Pure Edge HTTP prep (#946); client/request side-effects stay here. // URL/token come from the injected edgeCfg (composition root), never // os.Getenv (#1549). + if task == nil || dp == nil || strings.TrimSpace(s.edgeCfg.DeviceID) == "" { + return miss + } + payload := *dp + payload.TaskID = task.ID + if dispatch.RequiresDesktopTeamRouting(payload) { + return miss + } + // Workspace selection is part of the task, not a Hub-side fallback. + if strings.TrimSpace(dispatch.BuildEdgeRunRequest(payload).WorkDir) == "" { + return miss + } parts, insecure, err := dispatch.PrepareEdgeHTTPRequest( s.edgeCfg.URL, s.edgeCfg.AuthToken, - dp.Prompt, dp.AgentType, dp.SystemPrompt, task.ID, dp.DeliveryID, - dp.Messages, dp.PinnedMessages, dp.OutputSchema, + payload, s.issueRunStartCapability(dp), ) if insecure { @@ -34,7 +58,7 @@ func (s *DispatchService) dispatchToEdgeHTTP(ctx context.Context, task *model.Pe metrics.AgentDispatchEdgeHTTPFailures.WithLabelValues("insecure_cleartext").Inc() } metrics.OutboundMetrics.Record(outboundmetrics.ProviderEdge, outboundmetrics.PurposeDispatch, outboundmetrics.CategoryFailure, "insecure_cleartext") - return "" + return miss } if err != nil { slog.Error(dispatch.EdgeHTTPLogMarshalFailed, "task_id", task.ID, "error", err) @@ -42,9 +66,11 @@ func (s *DispatchService) dispatchToEdgeHTTP(ctx context.Context, task *model.Pe metrics.AgentDispatchEdgeHTTPFailures.WithLabelValues("marshal_failed").Inc() } metrics.OutboundMetrics.Record(outboundmetrics.ProviderEdge, outboundmetrics.PurposeDispatch, outboundmetrics.CategoryFailure, "marshal_failed") - return "" + return miss } + ctx, cancel := context.WithTimeout(ctx, parts.Timeout) + defer cancel() httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, parts.RunsURL, bytes.NewReader(parts.Body)) if err != nil { slog.Error(dispatch.EdgeHTTPLogCreateReqFailed, "task_id", task.ID, "error", err) @@ -52,7 +78,7 @@ func (s *DispatchService) dispatchToEdgeHTTP(ctx context.Context, task *model.Pe metrics.AgentDispatchEdgeHTTPFailures.WithLabelValues("req_create_failed").Inc() } metrics.OutboundMetrics.Record(outboundmetrics.ProviderEdge, outboundmetrics.PurposeDispatch, outboundmetrics.CategoryFailure, "req_create_failed") - return "" + return miss } httpReq.Header = parts.Headers // Correlation contract (#1595): propagate the caller's request id so the @@ -69,7 +95,7 @@ func (s *DispatchService) dispatchToEdgeHTTP(ctx context.Context, task *model.Pe metrics.AgentDispatchEdgeHTTPFailures.WithLabelValues("unreachable").Inc() } metrics.OutboundMetrics.Record(outboundmetrics.ProviderEdge, outboundmetrics.PurposeDispatch, outboundmetrics.CategoryFailure, "unreachable") - return "" + return miss } // Per-Edge circuit breaker: when Edge is down, consecutive dispatches would // each block for the full HTTP client timeout (~30s), exhausting the @@ -79,14 +105,40 @@ func (s *DispatchService) dispatchToEdgeHTTP(ctx context.Context, task *model.Pe // (insecure/marshal/req_create/edgeClient-nil) are config issues and do // not trip the breaker; only client.Do/non_success/decode_fail indicate // Edge health and are recorded. + if s.db == nil { + return edgeHTTPDispatchResult{} + } + owned, err := repository.DirectCallbackDeviceMatchesTask(s.db.WithContext(ctx), task.ID, deviceID) + if err != nil { + slog.Error("edge direct callback device lookup failed", "task_id", task.ID, "error", err) + return edgeHTTPDispatchResult{} + } + if !owned { + return miss + } if !s.edgeBreaker.Allow() { slog.Warn(dispatch.EdgeHTTPLogUnreachable, "task_id", task.ID, "url", parts.RunsURL, "error", "edge circuit breaker open") if metrics.AgentDispatchEdgeHTTPFailures != nil { metrics.AgentDispatchEdgeHTTPFailures.WithLabelValues("breaker_open").Inc() } metrics.OutboundMetrics.Record(outboundmetrics.ProviderEdge, outboundmetrics.PurposeDispatch, outboundmetrics.CategoryFailure, "breaker_open") - return "" + return miss + } + if !s.directCallbackRouteReady(ctx, parts) { + s.edgeBreaker.RecordFailure() + slog.Info("edge http dispatch: callback ownership route is unavailable", "task_id", task.ID) + return miss + } + + // Reserve the actual executor before POST: a timeout/invalid receipt must + // never let a retry start this task on an unrelated inviter Desktop. + if err := repository.ReservePendingTaskDirectDevice(s.db.WithContext(ctx), task.ID, deviceID); err != nil { + s.edgeBreaker.RecordSuccess() // health succeeded; reservation failure is not an Edge outage + slog.Error("edge direct device reservation failed", "task_id", task.ID, "error", err) + return edgeHTTPDispatchResult{} } + task.EdgeDeviceID = deviceID + dp.EdgeDeviceID = deviceID started := time.Now() resp, err := s.edgeClient.Do(httpReq) if err != nil { @@ -98,11 +150,15 @@ func (s *DispatchService) dispatchToEdgeHTTP(ctx context.Context, task *model.Pe } metrics.OutboundMetrics.Record(outboundmetrics.ProviderEdge, outboundmetrics.PurposeDispatch, outboundmetrics.CategoryFailure, "unreachable") s.edgeBreaker.RecordFailure() - return "" + return edgeHTTPDispatchResult{} } defer resp.Body.Close() - respBody, _ := io.ReadAll(io.LimitReader(resp.Body, dispatch.EdgeHTTPResponseBodyLimit)) + respBody, readErr := io.ReadAll(io.LimitReader(resp.Body, dispatch.EdgeHTTPResponseBodyLimit)) + if readErr != nil { + s.edgeBreaker.RecordFailure() + return edgeHTTPDispatchResult{} + } plan := dispatch.PlanEdgeHTTPClientResponse(resp.StatusCode, respBody) if plan.NonSuccess { slog.Warn(plan.LogMessage, "task_id", task.ID, "status", resp.StatusCode, "body_summary", SummarizeBodyForLog(respBody)) @@ -111,7 +167,7 @@ func (s *DispatchService) dispatchToEdgeHTTP(ctx context.Context, task *model.Pe } metrics.OutboundMetrics.Record(outboundmetrics.ProviderEdge, outboundmetrics.PurposeDispatch, outboundmetrics.CategoryFailure, "non_success") s.edgeBreaker.RecordFailure() - return "" + return edgeHTTPDispatchResult{} } if plan.DecodeFail { slog.Warn(plan.LogMessage, "task_id", task.ID, "error", plan.DecodeErr) @@ -120,11 +176,15 @@ func (s *DispatchService) dispatchToEdgeHTTP(ctx context.Context, task *model.Pe } metrics.OutboundMetrics.Record(outboundmetrics.ProviderEdge, outboundmetrics.PurposeDispatch, outboundmetrics.CategoryFailure, "decode_fail") s.edgeBreaker.RecordFailure() - return "" + return edgeHTTPDispatchResult{} } s.edgeBreaker.RecordSuccess() + owner := edgeDispatchReceiptOwner(respBody) + if plan.RunID == "" || owner == "" { + return edgeHTTPDispatchResult{} + } metrics.OutboundMetrics.Record(outboundmetrics.ProviderEdge, outboundmetrics.PurposeDispatch, outboundmetrics.CategorySuccess, outboundmetrics.StatusOK) metrics.OutboundMetrics.Observe(outboundmetrics.ProviderEdge, outboundmetrics.PurposeDispatch, outboundmetrics.CategorySuccess, outboundmetrics.StatusOK, time.Since(started)) slog.Info(dispatch.EdgeHTTPLogDispatched, "task_id", task.ID, "edge_run_id", plan.RunID, "url", parts.RunsURL) - return plan.RunID + return edgeHTTPDispatchResult{RunID: plan.RunID, CallbackOwner: owner} } diff --git a/hub-server/internal/service/dispatchsvc/agent_dispatch_edge_http_test.go b/hub-server/internal/service/dispatchsvc/agent_dispatch_edge_http_test.go index 1b0c61445..b52fa342e 100644 --- a/hub-server/internal/service/dispatchsvc/agent_dispatch_edge_http_test.go +++ b/hub-server/internal/service/dispatchsvc/agent_dispatch_edge_http_test.go @@ -29,19 +29,19 @@ import ( // no longer reads process env. func TestDispatchToEdgeHTTP_UsesInjectedConfigNotEnv(t *testing.T) { var gotPath, gotAuth string - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + srv := httptest.NewServer(withDirectCallbackHealth(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { gotPath = r.URL.Path gotAuth = r.Header.Get("Authorization") w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(`{"success":true,"data":{"runId":"run-42"}}`)) - })) + _, _ = w.Write([]byte(`{"success":true,"data":{"runId":"run-42","callbackOwner":"edge"}}`)) + }))) defer srv.Close() // Env points at a dead address and a wrong token; the injected config must win. t.Setenv("AGENTHUB_EDGE_URL", "http://127.0.0.1:1") t.Setenv("AGENTHUB_EDGE_AUTH_TOKEN", "env-token-must-not-be-used") - ds := NewDispatchService(nil, nil, nil, nil, nil, nil, config.EdgeDispatchConfig{ + ds := NewDispatchService(nil, nil, nil, nil, nil, nil, config.EdgeDispatchConfig{DeviceID: "fixture-edge-device", URL: srv.URL, AuthToken: "cfg-token", Timeout: 5 * time.Second, @@ -49,6 +49,7 @@ func TestDispatchToEdgeHTTP_UsesInjectedConfigNotEnv(t *testing.T) { task := &model.PendingAgentTask{ID: "task-1", AgentInstanceID: "ai-1"} dp := dispatchPayload{ + ModelParams: `{ "work_dir": "/workspace/fixture" }`, TaskID: "task-1", AgentInstanceID: "ai-1", AgentType: "codex", @@ -58,9 +59,10 @@ func TestDispatchToEdgeHTTP_UsesInjectedConfigNotEnv(t *testing.T) { Prompt: "hello", DisplayName: "agent", } - runID := ds.dispatchToEdgeHTTP(context.Background(), task, &dp) + ds.db = newDirectDispatchDB(t, task) + result := ds.dispatchToEdgeHTTP(context.Background(), task, &dp) - assert.Equal(t, "run-42", runID, "successful dispatch must return the Edge run id") + assert.Equal(t, "run-42", result.RunID, "successful dispatch must return the Edge run id") assert.Equal(t, "/v1/runs", gotPath) assert.Equal(t, "Bearer cfg-token", gotAuth, "token must come from injected config") } @@ -71,16 +73,16 @@ func TestDispatchToEdgeHTTP_UsesInjectedConfigNotEnv(t *testing.T) { // the caller deadline). func TestDispatchToEdgeHTTP_CallerDeadlineCancels(t *testing.T) { release := make(chan struct{}) - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + srv := httptest.NewServer(withDirectCallbackHealth(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // Never respond; block until the client gives up or the test ends. select { case <-r.Context().Done(): case <-release: } - })) + }))) t.Cleanup(func() { close(release); srv.Close() }) - ds := NewDispatchService(nil, nil, nil, nil, nil, nil, config.EdgeDispatchConfig{ + ds := NewDispatchService(nil, nil, nil, nil, nil, nil, config.EdgeDispatchConfig{DeviceID: "fixture-edge-device", URL: srv.URL, Timeout: 10 * time.Second, // client timeout is generous }, outboundhttp.NewClient(10*time.Second), "") @@ -90,16 +92,18 @@ func TestDispatchToEdgeHTTP_CallerDeadlineCancels(t *testing.T) { task := &model.PendingAgentTask{ID: "task-2", AgentInstanceID: "ai-2"} dp := dispatchPayload{ - TaskID: "task-2", AgentInstanceID: "ai-2", AgentType: "codex", + ModelParams: `{ "work_dir": "/workspace/fixture" }`, + TaskID: "task-2", AgentInstanceID: "ai-2", AgentType: "codex", SessionID: "sess-2", TriggerMessageID: "msg-2", TriggerUserID: "user-2", Prompt: "hello", DisplayName: "agent", } start := time.Now() - runID := ds.dispatchToEdgeHTTP(ctx, task, &dp) + ds.db = newDirectDispatchDB(t, task) + result := ds.dispatchToEdgeHTTP(ctx, task, &dp) elapsed := time.Since(start) - assert.Equal(t, "", runID, "cancelled request must not dispatch") + assert.Equal(t, "", result.RunID, "cancelled request must not dispatch") require.Less(t, elapsed, 9*time.Second, "caller deadline must win over the 10s client timeout (took %v)", elapsed) } @@ -111,12 +115,12 @@ func TestDispatchToEdgeHTTP_CallerDeadlineCancels(t *testing.T) { // embedded in the response proves the raw text does not leak through slog. func TestDispatchToEdgeHTTP_NonSuccessLogUsesSummaryNotRawBody(t *testing.T) { const sentinel = "UNIQUE-SENTINEL-DO-NOT-LOG-9f3b8a2e7c1d" - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + srv := httptest.NewServer(withDirectCallbackHealth(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusBadRequest) // Body is well past the summary prefix cap so the sentinel cannot // appear even accidentally via prefix leakage. _, _ = w.Write([]byte(strings.Repeat("x", defaultBodySummaryPrefixBytes+64) + sentinel)) - })) + }))) defer srv.Close() var logBuf bytes.Buffer @@ -124,20 +128,22 @@ func TestDispatchToEdgeHTTP_NonSuccessLogUsesSummaryNotRawBody(t *testing.T) { slog.SetDefault(slog.New(slog.NewJSONHandler(&logBuf, &slog.HandlerOptions{Level: slog.LevelDebug}))) t.Cleanup(func() { slog.SetDefault(prev) }) - ds := NewDispatchService(nil, nil, nil, nil, nil, nil, config.EdgeDispatchConfig{ + ds := NewDispatchService(nil, nil, nil, nil, nil, nil, config.EdgeDispatchConfig{DeviceID: "fixture-edge-device", URL: srv.URL, Timeout: 5 * time.Second, }, outboundhttp.NewClient(5*time.Second), "") task := &model.PendingAgentTask{ID: "task-log", AgentInstanceID: "ai-log"} dp := dispatchPayload{ - TaskID: "task-log", AgentInstanceID: "ai-log", AgentType: "codex", + ModelParams: `{ "work_dir": "/workspace/fixture" }`, + TaskID: "task-log", AgentInstanceID: "ai-log", AgentType: "codex", SessionID: "sess-log", TriggerMessageID: "msg-log", TriggerUserID: "user-log", Prompt: "hello", DisplayName: "agent", } - runID := ds.dispatchToEdgeHTTP(context.Background(), task, &dp) + ds.db = newDirectDispatchDB(t, task) + result := ds.dispatchToEdgeHTTP(context.Background(), task, &dp) - assert.Equal(t, "", runID, "non-success dispatch must return empty run id") + assert.Equal(t, "", result.RunID, "non-success dispatch must return empty run id") logged := logBuf.String() assert.Contains(t, logged, `"body_summary":"len=`) assert.NotContains(t, logged, sentinel, @@ -145,3 +151,15 @@ func TestDispatchToEdgeHTTP_NonSuccessLogUsesSummaryNotRawBody(t *testing.T) { // Also ensure we did not accidentally keep the old "body" key with raw text. assert.NotContains(t, logged, `,"body":"`) } + +// Keep HTTP tests on the enforced ownership-aware route, not an old Edge mock. +func withDirectCallbackHealth(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet && r.URL.Path == "/v1/health" { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"edgeId":"fixture-edge-device","capabilities":{"runCallbackOwnership":true,"directHubCallbacks":true}}`)) + return + } + next.ServeHTTP(w, r) + }) +} diff --git a/hub-server/internal/service/dispatchsvc/agent_dispatch_target_bound.go b/hub-server/internal/service/dispatchsvc/agent_dispatch_target_bound.go index 4200ff9d1..1f2d4b262 100644 --- a/hub-server/internal/service/dispatchsvc/agent_dispatch_target_bound.go +++ b/hub-server/internal/service/dispatchsvc/agent_dispatch_target_bound.go @@ -40,9 +40,12 @@ func (s *DispatchService) dispatchTargetBoundTask(ctx context.Context, cacheClie return false } frame := FramePort{Type: frameTypeAgentDispatch, Payload: json.RawMessage(payload)} - if err := repository.UpdatePendingTaskDispatched(s.db, task.ID, deviceID); !dispatch.RepoUpdateSucceeded(err) { - slog.Error(dispatch.DispatchLogTargetBoundMarkFailed, "task_id", task.ID, "user_id", userID, "target_id", task.TargetID, "device_id", deviceID, "error", err) - return false + // Restoring an accepted Desktop-owned run must not downgrade a fast ACK/done. + if task.EdgeRunID == "" { + if err := repository.UpdatePendingTaskDispatched(s.db, task.ID, deviceID); !dispatch.RepoUpdateSucceeded(err) { + slog.Error(dispatch.DispatchLogTargetBoundMarkFailed, "task_id", task.ID, "user_id", userID, "target_id", task.TargetID, "device_id", deviceID, "error", err) + return false + } } result := s.mgr.PushToConn(connID, frame) if !dispatch.RedeliveryWSPushSucceeded(result.Queued) { diff --git a/hub-server/internal/service/dispatchsvc/direct_dispatch_helpers_test.go b/hub-server/internal/service/dispatchsvc/direct_dispatch_helpers_test.go new file mode 100644 index 000000000..4d7040e53 --- /dev/null +++ b/hub-server/internal/service/dispatchsvc/direct_dispatch_helpers_test.go @@ -0,0 +1,39 @@ +package dispatchsvc + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" + "gorm.io/gorm" + + "github.com/agenthub/hub-server/internal/model" +) + +func newDirectDispatchDB(t *testing.T, task *model.PendingAgentTask) *gorm.DB { + t.Helper() + db := newTestDB(t) + require.NoError(t, db.Exec("CREATE TABLE agent_instances (id TEXT PRIMARY KEY, inviter_user_id TEXT NOT NULL)").Error) + require.NoError(t, db.Exec("CREATE TABLE devices (id TEXT PRIMARY KEY, user_id TEXT NOT NULL)").Error) + if task.AgentInstanceID == "" { + task.AgentInstanceID = "fixture-agent" + } + require.NoError(t, db.Exec("INSERT INTO agent_instances(id, inviter_user_id) VALUES (?, ?)", task.AgentInstanceID, "fixture-user").Error) + require.NoError(t, db.Exec("INSERT INTO devices(id, user_id) VALUES (?, ?)", "fixture-edge-device", "fixture-user").Error) + if task.Status == "" { + task.Status = model.TaskStatusQueued + } + if task.ExpireAt.IsZero() { + task.ExpireAt = time.Now().Add(time.Hour) + } + require.NoError(t, db.Create(task).Error) + return db +} + +// Timestamp decoding is exercised by the real PostgreSQL contract test. This +// SQLite fixture only reads the routing state involved in the transport test. +func readDirectDispatchTask(db *gorm.DB, id string) (*model.PendingAgentTask, error) { + var task model.PendingAgentTask + err := db.Select("id", "status", "edge_device_id", "edge_run_id").Where("id = ?", id).First(&task).Error + return &task, err +} diff --git a/hub-server/internal/service/dispatchsvc/direct_dispatch_recovery_test.go b/hub-server/internal/service/dispatchsvc/direct_dispatch_recovery_test.go new file mode 100644 index 000000000..3f92283b3 --- /dev/null +++ b/hub-server/internal/service/dispatchsvc/direct_dispatch_recovery_test.go @@ -0,0 +1,161 @@ +package dispatchsvc + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/agenthub/hub-server/internal/config" + "github.com/agenthub/hub-server/internal/model" + "github.com/agenthub/hub-server/internal/outboundhttp" + "github.com/agenthub/hub-server/internal/service/dispatch" +) + +func TestDirectHTTPUnknownReceiptKeepsOriginalDeviceAcrossRedelivery(t *testing.T) { + for _, reset := range []bool{false, true} { + name := "missing-owner" + if reset { + name = "response-lost" + } + t.Run(name, func(t *testing.T) { + task := &model.PendingAgentTask{} + db := newDirectDispatchDB(t, task) + var posts atomic.Int32 + var healthy atomic.Bool + healthy.Store(true) + requestTaskIDs := make(chan string, 3) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/v1/health" { + if !healthy.Load() { + w.WriteHeader(http.StatusServiceUnavailable) + return + } + _, _ = w.Write([]byte(`{"edgeId":"fixture-edge-device","capabilities":{"runCallbackOwnership":true,"directHubCallbacks":true}}`)) + return + } + var body dispatch.EdgeRunRequest + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } + requestTaskIDs <- body.HubTaskID + n := posts.Add(1) + if n == 1 && reset { + conn, _, err := w.(http.Hijacker).Hijack() + if err == nil { + _ = conn.Close() + } + return + } + w.WriteHeader(http.StatusAccepted) + if n == 1 { + _, _ = w.Write([]byte(`{"data":{"runId":"original-run"}}`)) + return + } + _, _ = w.Write([]byte(`{"data":{"runId":"original-run","callbackOwner":"edge"}}`)) + })) + defer server.Close() + cache := &recordingDispatchCache{routes: map[string]string{"fixture-user:desktop": "other-conn"}} + manager := &recordingDispatchWS{conn: &ConnPort{ID: "other-conn", UserID: "fixture-user", DeviceType: "desktop", DeviceID: "other-device"}} + outbox := &recordingDispatchOutbox{} + cfg := config.EdgeDispatchConfig{DeviceID: "fixture-edge-device", URL: server.URL, Timeout: time.Second} + service := NewDispatchService(db, nil, manager, cache, nil, outbox, cfg, outboundhttp.NewClient(time.Second), "") + payload := dispatchPayload{TaskID: task.ID, AgentType: "codex", Prompt: "Keep execution on one device.", ModelParams: `{"work_dir":"/workspace/project"}`} + raw, err := dispatch.MarshalPayload(payload) + require.NoError(t, err) + service.dispatchRouteHTTP(context.Background(), task, &model.AgentInstance{InviterUserID: "fixture-user"}, &payload, raw, "delivery-1", cache) + require.EqualValues(t, 1, posts.Load()) + require.Equal(t, 0, manager.pushed) + require.Empty(t, cache.pushed) + require.Zero(t, outbox.marked) + stored, err := readDirectDispatchTask(db, task.ID) + require.NoError(t, err) + require.Equal(t, "fixture-edge-device", stored.EdgeDeviceID) + require.Empty(t, stored.EdgeRunID) + require.Equal(t, model.TaskStatusQueued, stored.Status) + + // A fresh dispatcher has no in-memory admission state. It must use + // the persisted destination even while the Edge health probe fails. + restarted := NewDispatchService(db, nil, manager, cache, nil, outbox, cfg, outboundhttp.NewClient(time.Second), "") + snapshot, err := restarted.getPendingTaskForRedelivery(context.Background(), task.ID) + require.NoError(t, err) + record := redispatchTarget{DeliveryID: "delivery-2", TaskID: task.ID} + healthy.Store(false) + require.Error(t, restarted.retryDispatchToTarget(context.Background(), snapshot, payload, raw, record)) + require.EqualValues(t, 1, posts.Load()) + require.Zero(t, manager.pushed) + require.Empty(t, cache.pushed) + + healthy.Store(true) + require.NoError(t, restarted.retryDispatchToTarget(context.Background(), snapshot, payload, raw, record)) + require.EqualValues(t, 2, posts.Load()) + require.Equal(t, task.ID, <-requestTaskIDs) + require.Equal(t, task.ID, <-requestTaskIDs) + require.Zero(t, manager.pushed) + require.Empty(t, cache.pushed) + stored, err = readDirectDispatchTask(db, task.ID) + require.NoError(t, err) + require.Equal(t, "original-run", stored.EdgeRunID) + require.Equal(t, "fixture-edge-device", stored.EdgeDeviceID) + }) + } +} + +func TestDirectHTTPDesktopReplayRestoresOnlyOriginalDevice(t *testing.T) { + task := &model.PendingAgentTask{} + db := newDirectDispatchDB(t, task) + server := httptest.NewServer(withDirectCallbackHealth(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Simulate an already-running callback arriving before the HTTP receipt. + if err := db.Model(&model.PendingAgentTask{}).Where("id = ?", task.ID).Update("status", model.TaskStatusRunning).Error; err != nil { + w.WriteHeader(http.StatusInternalServerError) + return + } + w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(`{"data":{"runId":"desktop-run","callbackOwner":"desktop"}}`)) + }))) + defer server.Close() + cache := &recordingDispatchCache{routes: map[string]string{"fixture-user:desktop": "other-conn"}} + manager := &recordingDispatchWS{conn: &ConnPort{ID: "other-conn", UserID: "fixture-user", DeviceType: "desktop", DeviceID: "other-device"}} + service := NewDispatchService(db, nil, manager, cache, nil, nil, config.EdgeDispatchConfig{DeviceID: "fixture-edge-device", URL: server.URL, Timeout: time.Second}, outboundhttp.NewClient(time.Second), "") + payload := dispatchPayload{TaskID: task.ID, AgentType: "codex", ModelParams: `{"work_dir":"/workspace/project"}`} + raw, err := dispatch.MarshalPayload(payload) + require.NoError(t, err) + service.dispatchRouteHTTP(context.Background(), task, &model.AgentInstance{InviterUserID: "fixture-user"}, &payload, raw, "delivery", cache) + require.Zero(t, manager.pushed, "another connected Desktop is not the callback owner") + require.Len(t, cache.pushed, 1) + require.Contains(t, cache.pushed[0], "fixture-user::fixture-edge-device:") + require.Contains(t, cache.pushed[0], `"edge_device_id":"fixture-edge-device"`) + // When the original Desktop is connected, bridge recovery may push but not + // re-mark the already running task as merely dispatched. + cache.routes["fixture-user:desktop:fixture-edge-device"] = "owner-conn" + manager.conn = &ConnPort{ID: "owner-conn", UserID: "fixture-user", DeviceType: "desktop", DeviceID: "fixture-edge-device"} + service.dispatchRouteHTTP(context.Background(), task, &model.AgentInstance{InviterUserID: "fixture-user"}, &payload, raw, "delivery", cache) + require.Equal(t, 1, manager.pushed) + stored, err := readDirectDispatchTask(db, task.ID) + require.NoError(t, err) + require.Equal(t, model.TaskStatusRunning, stored.Status) + require.Equal(t, "desktop-run", stored.EdgeRunID) +} + +func TestDirectHTTPRejectsCallbackDeviceOwnedByAnotherUser(t *testing.T) { + task := &model.PendingAgentTask{} + db := newDirectDispatchDB(t, task) + require.NoError(t, db.Exec("UPDATE devices SET user_id = ? WHERE id = ?", "other-user", "fixture-edge-device").Error) + var posts atomic.Int32 + server := httptest.NewServer(withDirectCallbackHealth(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { posts.Add(1) }))) + defer server.Close() + service := NewDispatchService(db, nil, nil, nil, nil, nil, config.EdgeDispatchConfig{DeviceID: "fixture-edge-device", URL: server.URL, Timeout: time.Second}, outboundhttp.NewClient(time.Second), "") + payload := dispatchPayload{TaskID: task.ID, AgentType: "codex", ModelParams: `{"work_dir":"/workspace/project"}`} + result := service.dispatchToEdgeHTTP(context.Background(), task, &payload) + require.True(t, result.SafeToFallback) + require.Zero(t, posts.Load()) + stored, err := readDirectDispatchTask(db, task.ID) + require.NoError(t, err) + require.Empty(t, stored.EdgeDeviceID) +} diff --git a/hub-server/tests/integration/direct_receipt_test.go b/hub-server/tests/integration/direct_receipt_test.go new file mode 100644 index 000000000..ad0101db1 --- /dev/null +++ b/hub-server/tests/integration/direct_receipt_test.go @@ -0,0 +1,62 @@ +//go:build integration + +package integration + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + "gorm.io/gorm" + + "github.com/agenthub/hub-server/internal/model" + "github.com/agenthub/hub-server/internal/repository" +) + +// Use the migrated PostgreSQL UUID column: SQLite text fixtures cannot catch +// comparisons that accidentally coerce an empty string to a UUID. +func TestDirectReceiptPostgresIdentityAndLateCallbacks(t *testing.T) { + t.Cleanup(func() { CleanDB(t, db) }) + owner := register(t, "directreceipt2350", "pass1234", "DirectReceipt") + ownerToken := mintDesktopToken(t, owner.ID, edgeDeviceA) + otherToken := mintDesktopToken(t, owner.ID, edgeDeviceB) + mustOK(t, parse(postAuth("/edge/devices/register", ownerToken, map[string]interface{}{ + "device_id": edgeDeviceA, "app_version": "direct-receipt-fixture", "capabilities": []string{"codex"}, + })), "register direct callback device") + task := seedEdgeCallbackTask(t, owner.ID, model.TaskStatusQueued, "", "") + var seededAgent model.AgentInstance + require.NoError(t, db.Select("id", "session_id").Where("id = ?", task.AgentInstanceID).First(&seededAgent).Error) + require.NoError(t, testCacheClient.InitSeqIfAbsent(context.Background(), seededAgent.SessionID, 1)) + require.NoError(t, db.Model(&model.Session{}).Where("id = ?", seededAgent.SessionID).Update("next_seq", 1).Error) + const runID = "direct-receipt-run" + + owned, err := repository.DirectCallbackDeviceMatchesTask(db, task.ID, edgeDeviceA) + require.NoError(t, err) + require.True(t, owned) + require.NoError(t, repository.ReservePendingTaskDirectDevice(db, task.ID, edgeDeviceA)) + require.ErrorIs(t, repository.ReservePendingTaskDirectDevice(db, task.ID, edgeDeviceB), gorm.ErrRecordNotFound) + require.NoError(t, repository.RecordPendingTaskDirectReceipt(db, task.ID, edgeDeviceA, runID)) + stored, err := repository.GetPendingTaskByID(db, task.ID) + require.NoError(t, err) + require.Equal(t, model.TaskStatusDispatched, stored.Status) + require.Equal(t, edgeDeviceA, stored.EdgeDeviceID) + require.Equal(t, runID, stored.EdgeRunID) + + mustCode(t, parse(postAuth("/edge/agent-tasks/"+task.ID+"/ack", otherToken, + map[string]string{"run_id": runID})), "agent_task_not_found", "wrong callback device") + mustOK(t, parse(postAuth("/edge/agent-tasks/"+task.ID+"/ack", ownerToken, + map[string]string{"run_id": runID})), "direct owner ack") + require.NoError(t, repository.RecordPendingTaskDirectReceipt(db, task.ID, edgeDeviceA, runID)) + stored, err = repository.GetPendingTaskByID(db, task.ID) + require.NoError(t, err) + require.Equal(t, model.TaskStatusRunning, stored.Status, "late HTTP receipt must not undo the ACK") + + mustOK(t, parse(postAuth("/edge/agent-tasks/"+task.ID+"/done", ownerToken, + map[string]string{"run_id": runID, "final_content": "Complete."})), "direct owner done") + require.NoError(t, repository.RecordPendingTaskDirectReceipt(db, task.ID, edgeDeviceA, runID)) + stored, err = repository.GetPendingTaskByID(db, task.ID) + require.NoError(t, err) + require.Equal(t, model.TaskStatusDone, stored.Status, "late HTTP receipt must not undo completion") + require.ErrorIs(t, repository.RecordPendingTaskDirectReceipt(db, task.ID, edgeDeviceB, runID), gorm.ErrRecordNotFound) + require.ErrorIs(t, repository.RecordPendingTaskDirectReceipt(db, task.ID, edgeDeviceA, "other-run"), gorm.ErrRecordNotFound) +} diff --git a/tests/fixtures/dispatch/execution-intent.json b/tests/fixtures/dispatch/execution-intent.json index a5dc0288b..181176ace 100644 --- a/tests/fixtures/dispatch/execution-intent.json +++ b/tests/fixtures/dispatch/execution-intent.json @@ -201,6 +201,24 @@ "hubTaskId": "task-intent-5", "structuredOutputSchema": "{\"type\":\"object\"}" } + }, + { + "name": "safe-integer-fallback-and-literal-schema", + "payload": { + "task_id": "task-intent-6", + "agent_type": "codex", + "prompt": "Preserve the explicit intent.", + "system_prompt": " Keep this spacing.\n", + "model_params": "{\"max_thinking_tokens\":9007199254740992,\"maxThinkingTokens\":0,\"structured_output_schema\":false,\"structuredOutputSchema\":{\"type\":\"object\"},\"system_prompt\":\"Nested fallback.\"}" + }, + "expectedIntent": { + "prompt": "Preserve the explicit intent.", + "agentId": "codex", + "maxThinkingTokens": 0, + "systemPrompt": " Keep this spacing.\n", + "structuredOutputSchema": "false", + "hubTaskId": "task-intent-6" + } } ] } From 0206f73020b3454398b7aec5b5ec1763320e22d0 Mon Sep 17 00:00:00 2001 From: Delicious233 <101502465+DeliciousBuding@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:40:26 +0800 Subject: [PATCH 4/5] fix(dispatch): complete single-owner runtime callback delivery --- api/dispatch.md | 3 +- .../__e2e__/hub-delivery-admission.spec.ts | 246 +++++++- .../src/__tests__/useHubIntegration.test.ts | 582 +++++++++++++++++- app/desktop/src/hooks/executionIntent.test.ts | 105 ++++ .../src/hooks/hubIntegrationEdgeApi.ts | 58 ++ .../src/hooks/hubIntegrationHelpers.test.ts | 46 ++ .../src/hooks/hubIntegrationMappers.ts | 61 +- .../src/hooks/hubIntegrationParseHelpers.ts | 56 ++ app/desktop/src/hooks/useHubIntegration.ts | 248 +++++--- app/desktop/src/stores/taskBridgeStore.ts | 4 + edge-server/internal/hub/callback.go | 64 +- .../internal/hub/callback_event_test.go | 149 +++++ .../internal/lifecycle/callback_event_test.go | 383 ++++++++++++ .../process_executor_hub_callback.go | 30 +- .../process_executor_hub_callback_helpers.go | 98 +++ .../integration/outbox_claim_cas_test.go | 12 +- 16 files changed, 2012 insertions(+), 133 deletions(-) create mode 100644 app/desktop/src/hooks/executionIntent.test.ts create mode 100644 edge-server/internal/hub/callback_event_test.go create mode 100644 edge-server/internal/lifecycle/callback_event_test.go create mode 100644 edge-server/internal/lifecycle/process_executor_hub_callback_helpers.go diff --git a/api/dispatch.md b/api/dispatch.md index 2cd7bcfe1..397c609f8 100644 --- a/api/dispatch.md +++ b/api/dispatch.md @@ -29,6 +29,7 @@ Hub 的 WS `agent.dispatch` 与 outbox HTTP POST `/v1/runs` 共享同一 `delive - **允许的通道差异**：Hub HTTP 使用本地 project/thread，Desktop 使用对应会话线程；同 Hub task 的 admission 身份合流不变。直接通道请求 `callbackOwner: edge`，Desktop 请求 `callbackOwner: desktop`。Edge 在 pending admission 中保存首次选择，重放始终返回原 run 的真实 owner，不因新请求换人。 - **执行前能力检查**：`GET /v1/health` 的 `capabilities.runCallbackOwnership` 证明该版本执行 owner 契约；`directHubCallbacks` 仅表明 Edge 已配置目的地和当前凭据，不证明远端连通或 token 有效。Hub direct 要求两者为 true，且 health 的 `edgeId` 与配置的真实 `device_id` 相同、该注册设备归属于任务 Agent 的邀请用户；Desktop 要求 ownership 支持（sidecar 无直接回调是正常）。缺失/未知能力时不发送 run POST，不以旧端会忽略新字段为兼容策略。Edge 仍在新接收时校验 direct callback 配置，未就绪返回 503 `callback_unavailable`，不创建 run。 - **单一结果回传方**：edge-owned run 由 Edge 发 task ACK/stream/done/fail；Desktop 只更新本地 run 状态，不发第二套任务回调。desktop-owned run 的 Edge 不建立直接 callback 映射。relay delivery ACK 仍由接收 Desktop 负责，不能与任务结果回调混同。Hub direct 遇到 desktop-owned receipt 时只向该原设备投递，让 Desktop 恢复 bridge；不能改选邀请用户的另一台 Desktop。 +- **typed 输出与背压**：Edge 回传 thinking/tool/file/permission/route/result 的结构化事件，`payload` 是 JSON 对象、每条带稳定 `client_msg_id`。typed 边界前先排入待合并文本；typed 与终态共用同一 per-run FIFO，队列满时 typed 等待空位，不通过丢审批或新增无界 goroutine 释放压力。文本仍沿既有丢流策略并独立收集 final fallback。网络重试有预算，永久失败仍可导致未交付；这不是跨进程可靠消息日志或恢复保证。 - **Team 控制边界**：typed route/result stream 是事件记录，不等同于调用 Team 的权威 route-decision 接口。带 Team 上下文的任务继续走 Desktop callback owner 与现有控制流程；Hub direct 在 POST 前退出，不把事件透传宣称为 Team 自动调度。 - **direct 路由保留**：只有执行前能力检查失败、且尚未绑定设备的任务可以走普通 fallback。Hub 在 run POST 前持久化真实设备绑定；POST 超时、连接中断、错误或未知 owner 响应不证明未执行，现有 outbox 只能向原设备核对，不改投另一执行器。成功回执补记原 run ID；迟到回执不得回退已经 running/done/failed 的任务。 -- **旧回执与恢复**：现代请求碰到无法确定 owner 的旧 run，或收到缺失/非法 owner 的接收响应，按未决结果处理，不猜测、不自动重启。该契约不迁移正在执行的旧进程、不转交已接受任务的 owner，也不宣称跨重启恢复已完成。 +- **旧回执与恢复**：现代请求碰到无法确定 owner 的旧 run，或收到缺失/非法 owner 的接收响应，按未决结果处理，不猜测、不自动重启。Desktop 的 run POST 网络失败、5xx 或缺失 run ID 也不能证明执行失败，因此保持待核对而不发 FAIL；明确的执行前客户端拒绝仍可失败收口。该契约不迁移正在执行的旧进程、不转交已接受任务的 owner，也不宣称跨重启恢复已完成。 diff --git a/app/desktop/src/__e2e__/hub-delivery-admission.spec.ts b/app/desktop/src/__e2e__/hub-delivery-admission.spec.ts index 0b615a6b0..038d59e9a 100644 --- a/app/desktop/src/__e2e__/hub-delivery-admission.spec.ts +++ b/app/desktop/src/__e2e__/hub-delivery-admission.spec.ts @@ -13,6 +13,7 @@ const RELAY = 'relay-delivery-fixture'; const TARGET = 'target-delivery-fixture'; const THREAD = 'thread-delivery-fixture'; const EMPTY_LIST = { items: [], page: { hasMore: false } }; +type FixtureCallbackOwner = 'edge' | 'desktop'; async function readTaskState(page: Page) { return page.evaluate(async () => { @@ -29,6 +30,20 @@ async function readTaskState(page: Page) { }); } +async function readTaskStateWithOwner(page: Page) { + return page.evaluate(async () => { + const modulePath = '/src/stores/taskBridgeStore.ts'; + const { useTaskBridgeStore } = await import(/* @vite-ignore */ modulePath); + const state = useTaskBridgeStore.getState(); + return { + tasks: state.tasks.map((task: { taskId: string; status: string; runId?: string; callbackOwner?: string }) => ({ + taskId: task.taskId, status: task.status, runId: task.runId ?? null, callbackOwner: task.callbackOwner ?? null, + })), + runToTask: state.runToTask, + }; + }); +} + async function readTaskError(page: Page) { return page.evaluate(async () => { const modulePath = '/src/stores/taskBridgeStore.ts'; @@ -42,8 +57,10 @@ async function installDispatchFixture(page: Page, baseURL: string, theme: 'light const appOrigin = new URL(baseURL).origin; const hubSockets = new Set(); const edgeSockets = new Set(); + let edgeSeq = 0; const calls = { runs: [] as Record[], + streams: [] as Record[], acks: [] as Record[], relayAcks: [] as Record[], done: [] as Record[], @@ -51,6 +68,11 @@ async function installDispatchFixture(page: Page, baseURL: string, theme: 'light registered: 0, targetsRead: 0, rejection: { status: 503, code: 'delivery_busy' } as { status: number; code: string } | null, + owner: 'desktop' as FixtureCallbackOwner, + deduplicated: false, + healthSupported: true, + abortRun: false, + healthCalls: 0, pageErrors: [] as string[], unhandledWrites: [] as string[], }; @@ -109,6 +131,7 @@ async function installDispatchFixture(page: Page, baseURL: string, theme: 'light calls.fails.push(request.postDataJSON()); await json({ code: 'OK' }); } else if (url.pathname === '/edge/agent-tasks/' + TASK + '/stream') { + calls.streams.push(request.postDataJSON()); await json({ code: 'OK' }); } else if (request.method() === 'GET') { await list(); @@ -120,19 +143,26 @@ async function installDispatchFixture(page: Page, baseURL: string, theme: 'light } if (url.origin === 'http://127.0.0.1:3210') { if (url.pathname === '/v1/health') { - await json({ code: 'OK', data: { status: 'ok', version: 'fixture', edgeId: 'edge-fixture' } }); + calls.healthCalls++; + await json({ code: 'OK', data: { + status: 'ok', version: 'fixture', edgeId: 'edge-fixture', + capabilities: calls.healthSupported + ? { runCallbackOwnership: true, directHubCallbacks: false } + : { directHubCallbacks: true }, + } }); } else if (url.pathname === '/v1/model-catalog') { await json({ code: 'OK', data: { items: [], sources: [] } }); } else if (url.pathname === '/v1/threads' && request.method() === 'POST') { await json({ code: 'OK', data: { threadId: THREAD, projectId: 'proj_local' } }, 201); } else if (url.pathname === '/v1/runs' && request.method() === 'POST') { calls.runs.push(request.postDataJSON()); + if (calls.abortRun) { await route.abort('failed'); return; } if (calls.rejection) { await route.fulfill({ status: calls.rejection.status, headers: calls.rejection.status === 503 ? { 'Retry-After': '1' } : {}, json: { error: { code: calls.rejection.code, message: 'fixture admission rejection: ' + calls.rejection.code, traceId: 'fixture-trace' }, } }); } else { - await json({ code: 'OK', data: { runId: RUN, projectId: 'proj_local', threadId: THREAD, status: 'queued', deduplicated: calls.runs.length > 2, deliveryId: DELIVERY } }, 202); + await json({ code: 'OK', data: { runId: RUN, projectId: 'proj_local', threadId: THREAD, status: 'queued', deduplicated: calls.deduplicated, deliveryId: DELIVERY, callbackOwner: calls.owner } }, 202); } } else if (request.method() === 'GET') { await list(); @@ -173,25 +203,65 @@ async function installDispatchFixture(page: Page, baseURL: string, theme: 'light return { calls, - async dispatch() { - const response = page.waitForResponse((result) => - result.url() === 'http://127.0.0.1:3210/v1/runs' && result.request().method() === 'POST', - ); + async dispatch(waitForRun = true) { + const response = waitForRun + ? page.waitForResponse((result) => + result.url() === 'http://127.0.0.1:3210/v1/runs' && result.request().method() === 'POST', + ) + : null; const frame = { type: 'agent.dispatch', payload: { relay_command_id: RELAY, command_type: 'agent.dispatch', payload: JSON.stringify({ - task_id: TASK, delivery_id: DELIVERY, prompt: 'Fixture task', thread_id: THREAD, - agent_type: 'codex', target_id: TARGET, edge_device_id: DEVICE, + task_id: TASK, delivery_id: DELIVERY, prompt: 'Implement the requested fixture.', + thread_id: THREAD, agent_type: 'codex', target_id: TARGET, edge_device_id: DEVICE, + system_prompt: 'Use the approved project conventions.', + tool_whitelist: '["Read","Grep"]', + model_params: JSON.stringify({ + model: 'gpt-5.5', + reasoning_effort: 'high', + thinking_mode: 'adaptive', + permission_mode: 'plan', + work_dir: '/workspace/project', + include_partial: true, + max_thinking_tokens: 4096, + append_system_prompt: 'Keep output concise.', + config_overrides: { reasoning_summary: 'auto' }, + ephemeral: true, + session_id: 'runtime-session-e2e', + continue: false, + fork: true, + structured_output_schema: { + type: 'object', + properties: { result: { type: 'string' } }, + required: ['result'], + }, + }), + messages: [ + { role: 'user', content: 'Keep the change offline and preserve existing behavior.', timestamp: '2026-01-01T00:00:00Z' }, + { role: 'assistant', content: 'The earlier patch uses a bounded retry policy.', timestamp: '2026-01-01T00:01:00Z' }, + ], + pinned_messages: [ + { role: 'system', content: 'Run focused tests before reporting success.', timestamp: '2026-01-01T00:00:00Z' }, + ], + structured_output_schema: { type: 'array' }, + trace_id: 'trace-e2e', }), } }; for (const socket of hubSockets) socket.send(JSON.stringify(frame)); - await (await response).finished(); + if (response) await (await response).finished(); // Negative assertions must wait until the error body and React effects // have been consumed, not just until the mock records the request. await page.evaluate(() => new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(() => resolve())))); }, + sendEdge(type: string, payload: Record) { + edgeSeq += 1; + for (const socket of edgeSockets) { + socket.send(JSON.stringify({ version: 'v1', id: 'fixture-' + type + '-' + edgeSeq, seq: edgeSeq, type, ts: new Date().toISOString(), scope: { runId: RUN, threadId: THREAD }, payload })); + } + }, finish() { + edgeSeq += 1; for (const socket of edgeSockets) { - socket.send(JSON.stringify({ version: 'v1', id: 'fixture-finished', seq: 1, type: 'run.finished', ts: new Date().toISOString(), scope: { runId: RUN, threadId: THREAD }, payload: { runId: RUN } })); + socket.send(JSON.stringify({ version: 'v1', id: 'fixture-finished-' + edgeSeq, seq: edgeSeq, type: 'run.finished', ts: new Date().toISOString(), scope: { runId: RUN, threadId: THREAD }, payload: { runId: RUN } })); } }, }; @@ -280,3 +350,159 @@ for (const theme of ['light', 'dark'] as const) { await page.screenshot({ path: testInfo.outputPath('delivery-admission-' + theme + '.png') }); }); } + +for (const theme of ['light', 'dark'] as const) { + test('Desktop bridge delivers full execution intent and keeps edge-owned replay local (' + theme + ')', async ({ page, baseURL }) => { + if (!baseURL) throw new Error('Desktop E2E baseURL is required'); + const { calls, dispatch, sendEdge, finish } = await installDispatchFixture(page, baseURL, theme); + calls.rejection = null; + calls.owner = 'edge'; + calls.deduplicated = true; + + await dispatch(); + await expect.poll(() => calls.runs.length).toBe(1); + const run = calls.runs[0]; + if (!run) throw new Error('fixture run request was not recorded'); + expect(run).toMatchObject({ + agentId: 'codex', + model: 'gpt-5.5', + reasoningEffort: 'high', + thinkingMode: 'adaptive', + permissionMode: 'plan', + workDir: '/workspace/project', + includePartial: true, + maxThinkingTokens: 4096, + systemPrompt: 'Use the approved project conventions.', + appendSystemPrompt: 'Keep output concise.', + allowedTools: ['Read', 'Grep'], + configOverrides: { reasoning_summary: 'auto' }, + ephemeral: true, + sessionId: 'runtime-session-e2e', + continue: false, + fork: true, + trace_id: 'trace-e2e', + callbackOwner: 'desktop', + hubTaskId: TASK, + deliveryId: DELIVERY, + targetId: TARGET, + edgeDeviceId: DEVICE, + }); + const schema = run.structuredOutputSchema; + if (typeof schema !== 'string') throw new Error('fixture schema was not serialized to a string'); + expect(JSON.parse(schema)).toEqual({ + type: 'object', + properties: { result: { type: 'string' } }, + required: ['result'], + }); + expect(run.messages).toEqual([ + { role: 'user', content: 'Keep the change offline and preserve existing behavior.', timestamp: '2026-01-01T00:00:00Z' }, + { role: 'assistant', content: 'The earlier patch uses a bounded retry policy.', timestamp: '2026-01-01T00:01:00Z' }, + ]); + expect(run.pinnedMessages).toEqual([ + { role: 'system', content: 'Run focused tests before reporting success.', timestamp: '2026-01-01T00:00:00Z' }, + ]); + + await expect.poll(() => readTaskStateWithOwner(page)).toEqual({ + tasks: [{ taskId: TASK, status: 'running', runId: RUN, callbackOwner: 'edge' }], + runToTask: { [RUN]: TASK }, + }); + expect(calls.acks).toHaveLength(0); + expect(calls.relayAcks).toHaveLength(1); + expect(calls.streams).toHaveLength(0); + expect(calls.done).toHaveLength(0); + expect(calls.fails).toHaveLength(0); + + sendEdge('run.agent.text_delta', { runId: RUN, content: 'visible edge output' }); + finish(); + await expect.poll(() => readTaskStateWithOwner(page)).toEqual({ + tasks: [{ taskId: TASK, status: 'done', runId: RUN, callbackOwner: 'edge' }], + runToTask: { [RUN]: TASK }, + }); + expect(calls.acks).toHaveLength(0); + expect(calls.relayAcks).toHaveLength(1); + expect(calls.streams).toHaveLength(0); + expect(calls.done).toHaveLength(0); + expect(calls.fails).toHaveLength(0); + + calls.deduplicated = true; + await dispatch(); + await expect.poll(() => readTaskStateWithOwner(page)).toEqual({ + tasks: [{ taskId: TASK, status: 'done', runId: RUN, callbackOwner: 'edge' }], + runToTask: { [RUN]: TASK }, + }); + expect(calls.acks).toHaveLength(0); + expect(calls.relayAcks).toHaveLength(2); + // A contradictory owner is not a transport replay: keep the original + // mapping and do not acknowledge the conflicting receipt. + calls.owner = 'desktop'; + await dispatch(); + await expect.poll(() => readTaskError(page)).toContain('conflicts'); + expect(calls.relayAcks).toHaveLength(2); + expect(calls.acks).toHaveLength(0); + expect(calls.streams).toHaveLength(0); + expect(calls.done).toHaveLength(0); + expect(calls.fails).toHaveLength(0); + expect(calls.pageErrors).toEqual([]); + expect(calls.unhandledWrites).toEqual([]); + }); +} + +for (const theme of ['light', 'dark'] as const) { + test('Desktop bridge fails closed before POST on old Edge callback ownership (' + theme + ')', async ({ page, baseURL }) => { + if (!baseURL) throw new Error('Desktop E2E baseURL is required'); + const { calls, dispatch } = await installDispatchFixture(page, baseURL, theme); + calls.rejection = null; + calls.healthSupported = false; + + await dispatch(false); + await expect.poll(() => calls.healthCalls).toBeGreaterThan(0); + await expect.poll(() => readTaskStateWithOwner(page)).toEqual({ + tasks: [{ taskId: TASK, status: 'queued', runId: null, callbackOwner: null }], + runToTask: {}, + }); + await expect.poll(() => readTaskError(page)).toContain('runCallbackOwnership'); + expect(calls.runs).toHaveLength(0); + expect(calls.acks).toHaveLength(0); + expect(calls.relayAcks).toHaveLength(0); + expect(calls.fails).toHaveLength(0); + expect(calls.streams).toHaveLength(0); + expect(calls.pageErrors).toEqual([]); + expect(calls.unhandledWrites).toEqual([]); + }); +} + +for (const theme of ['light', 'dark'] as const) { + test('Desktop keeps unconfirmed run POSTs pending for review (' + theme + ')', async ({ page, baseURL }, testInfo) => { + if (!baseURL) throw new Error('Desktop E2E baseURL is required'); + const { calls, dispatch } = await installDispatchFixture(page, baseURL, theme); + calls.abortRun = true; + await dispatch(false); + await expect.poll(() => calls.runs.length).toBe(1); + await expect.poll(() => readTaskError(page)).toContain('uncertain'); + await expect(page.getByText(/Edge admission result is uncertain/)).toBeVisible(); + await expect.poll(() => readTaskState(page)).toEqual({ tasks: [{ taskId: TASK, status: 'queued', runId: null }], runToTask: {} }); + expect(calls.acks).toHaveLength(0); + expect(calls.fails).toHaveLength(0); + expect(calls.relayAcks).toHaveLength(0); + await page.screenshot({ path: testInfo.outputPath('unconfirmed-post-' + theme + '.png') }); + + calls.abortRun = false; + calls.rejection = { status: 500, code: 'internal_error' }; + await dispatch(); + await expect.poll(() => calls.runs.length).toBe(2); + await expect.poll(() => readTaskState(page)).toEqual({ tasks: [{ taskId: TASK, status: 'queued', runId: null }], runToTask: {} }); + expect(calls.acks).toHaveLength(0); + expect(calls.fails).toHaveLength(0); + expect(calls.relayAcks).toHaveLength(0); + + // Only another Hub delivery reconciles the same admission; no local retry. + calls.rejection = null; + await dispatch(); + await expect.poll(() => calls.runs.length).toBe(3); + await expect.poll(() => calls.acks.length).toBe(1); + await expect.poll(() => readTaskError(page)).toBeNull(); + expect(calls.fails).toHaveLength(0); + expect(calls.pageErrors).toEqual([]); + expect(calls.unhandledWrites).toEqual([]); + }); +} diff --git a/app/desktop/src/__tests__/useHubIntegration.test.ts b/app/desktop/src/__tests__/useHubIntegration.test.ts index 53e25198a..ffb479cd9 100644 --- a/app/desktop/src/__tests__/useHubIntegration.test.ts +++ b/app/desktop/src/__tests__/useHubIntegration.test.ts @@ -120,6 +120,10 @@ import type { HubClient } from '@/api/hubClient'; import { HUB_EVENTS } from '@shared/hubEvents'; import { useToastStore } from '@shared/ui/toast'; import { useHubIntegration } from '@/hooks/useHubIntegration'; +import { + EDGE_HEALTH_CAPABILITY_TIMEOUT_MS, + probeEdgeRunCallbackOwnership, +} from '@/hooks/hubIntegrationEdgeApi'; // ── Helpers ───────────────────────────────────────────── @@ -222,19 +226,29 @@ describe('useHubIntegration', () => { postTeamRouteDecision: vi.fn().mockResolvedValue({ id: 'assignment-1' }), } as unknown as HubClient; - // Mock fetch for Edge REST calls - fetchMock = vi.fn().mockResolvedValue( - new Response( + // Mock fetch for Edge REST calls. Every mocked run response needs an + // explicit callbackOwner because Desktop now fails closed on missing owners. + fetchMock = vi.fn().mockImplementation(async (input: unknown) => { + const url = String(input); + if (url.endsWith('/v1/health')) return healthResponse(); + if (url.endsWith('/v1/threads')) { + return new Response(JSON.stringify({ threadId: 'thread-ok' }), { + status: 201, + headers: { 'Content-Type': 'application/json' }, + }); + } + return new Response( JSON.stringify({ id: 'run-1', runId: 'run-1', projectId: 'proj-1', threadId: 'sess-1', status: 'started', + callbackOwner: 'desktop', }), { status: 200, headers: { 'Content-Type': 'application/json' } }, - ), - ); + ); + }); globalThis.fetch = fetchMock as unknown as typeof globalThis.fetch; }); @@ -270,10 +284,91 @@ describe('useHubIntegration', () => { return fetchMock.mock.calls.filter(([input]) => String(input).endsWith(path)).length; } + function healthResponse(): Response { + return new Response( + JSON.stringify({ + status: 'ok', + version: 'fixture', + edgeId: 'edge-fixture', + capabilities: { runCallbackOwnership: true, directHubCallbacks: false }, + }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ); + } + + it('fails closed when the /v1/health body read exceeds the bounded deadline', async () => { + const originalFetch = globalThis.fetch; + vi.useFakeTimers(); + let healthSignal: AbortSignal | undefined; + const hangingHealthFetch = vi.fn( + async (_input: RequestInfo | URL, init?: RequestInit): Promise => { + healthSignal = init?.signal ?? undefined; + const response = new Response('{}', { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + return Object.assign(response, { + json: () => + new Promise((_resolve, reject) => { + if (healthSignal?.aborted) { + reject(new Error('Health body read aborted')); + return; + } + healthSignal?.addEventListener('abort', () => { + reject(new Error('Health body read aborted')); + }); + }), + }); + }, + ); + globalThis.fetch = hangingHealthFetch; + + try { + const pending = probeEdgeRunCallbackOwnership('http://127.0.0.1:3210/'); + await vi.advanceTimersByTimeAsync(0); + await vi.advanceTimersByTimeAsync(EDGE_HEALTH_CAPABILITY_TIMEOUT_MS + 1); + await expect(pending).resolves.toEqual({ + supported: false, + reason: expect.stringMatching(/abort/i), + }); + expect(hangingHealthFetch).toHaveBeenCalledTimes(1); + expect(String(hangingHealthFetch.mock.calls[0]?.[0])).toBe( + 'http://127.0.0.1:3210/v1/health', + ); + expect(hangingHealthFetch.mock.calls[0]?.[1]).toEqual( + expect.objectContaining({ signal: expect.any(AbortSignal) }), + ); + } finally { + vi.useRealTimers(); + globalThis.fetch = originalFetch; + } + }); + + function mockRunCreateResponseRaw(body: Record) { + fetchMock.mockImplementation(async (input: unknown) => { + const url = String(input); + if (url.endsWith('/v1/health')) return healthResponse(); + if (url.endsWith('/v1/threads')) { + return new Response(JSON.stringify({ threadId: 'thread-ok' }), { + status: 201, + headers: { 'Content-Type': 'application/json' }, + }); + } + if (url.endsWith('/v1/runs')) { + return new Response(JSON.stringify(body), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + } + return new Response('{}', { status: 200, headers: { 'Content-Type': 'application/json' } }); + }); + } + function mockRunSequence(...runIds: string[]) { const queue = [...runIds]; fetchMock.mockImplementation(async (input: unknown) => { const url = String(input); + if (url.endsWith('/v1/health')) return healthResponse(); if (url.endsWith('/v1/threads')) { return new Response(JSON.stringify({ threadId: 'thread-ok' }), { status: 201, @@ -289,6 +384,7 @@ describe('useHubIntegration', () => { projectId: 'proj', threadId: 'sess', status: 'started', + callbackOwner: 'desktop', }), { status: 200, headers: { 'Content-Type': 'application/json' } }, ); @@ -298,27 +394,25 @@ describe('useHubIntegration', () => { } function mockRunCreateResponse(body: Record) { - fetchMock.mockImplementation(async (input: unknown) => { - const url = String(input); - if (url.endsWith('/v1/threads')) { - return new Response(JSON.stringify({ threadId: 'thread-ok' }), { - status: 201, - headers: { 'Content-Type': 'application/json' }, - }); - } - if (url.endsWith('/v1/runs')) { - return new Response(JSON.stringify(body), { - status: 200, - headers: { 'Content-Type': 'application/json' }, - }); - } - return new Response('{}', { status: 200, headers: { 'Content-Type': 'application/json' } }); - }); + const data = body.data; + const hasEnvelopeOwner = + data !== null && + typeof data === 'object' && + !Array.isArray(data) && + Object.prototype.hasOwnProperty.call(data, 'callbackOwner'); + const runBody = + hasEnvelopeOwner || Object.prototype.hasOwnProperty.call(body, 'callbackOwner') + ? body + : data !== null && typeof data === 'object' && !Array.isArray(data) + ? { ...body, data: { ...(data as Record), callbackOwner: 'desktop' } } + : { ...body, callbackOwner: 'desktop' }; + mockRunCreateResponseRaw(runBody); } function mockRunCreateResponseWithStatus(body: Record, status: number) { fetchMock.mockImplementation(async (input: unknown) => { const url = String(input); + if (url.endsWith('/v1/health')) return healthResponse(); if (url.endsWith('/v1/threads')) { return new Response(JSON.stringify({ threadId: 'thread-ok' }), { status: 201, @@ -399,7 +493,7 @@ describe('useHubIntegration', () => { expect(hubClient.failTask).not.toHaveBeenCalled(); }); - it('fails clearly when a unified Edge run envelope has no id or runId in data', async () => { + it('keeps a unified Edge receipt without run identity unresolved', async () => { mockRunCreateResponse({ code: 'OK', data: { @@ -415,10 +509,9 @@ describe('useHubIntegration', () => { }); expect(hubClient.ackTask).not.toHaveBeenCalled(); - expect(hubClient.failTask).toHaveBeenCalledWith( - 'task-1', - 'Edge run created but no id/runId in response data', - ); + expect(hubClient.failTask).not.toHaveBeenCalled(); + expect(hoisted.storeTasks[0]?.status).toBe('queued'); + expect(hoisted.storeTasks[0]?.error).toBeTruthy(); }); it('refuses to hand off dispatches that are not targeted to this Desktop local edge', async () => { @@ -599,7 +692,11 @@ describe('useHubIntegration', () => { }); it('reports failure to Hub when fetch fails', async () => { - fetchMock.mockRejectedValueOnce(new Error('Edge unavailable')); + fetchMock.mockImplementation(async (input: unknown) => { + const url = String(input); + if (url.endsWith('/v1/health')) return healthResponse(); + throw new Error('Edge unavailable'); + }); renderHook(() => useHubIntegration({ hubWS, hubClient })); @@ -963,8 +1060,35 @@ describe('useHubIntegration', () => { expect(hubClient.failTask).not.toHaveBeenCalled(); }); - it('still fails a permanent Edge admission rejection (e.g. 500)', async () => { - mockRunCreateResponseWithStatus({ error: { code: 'internal_error', message: 'boom', traceId: 'trace_001' } }, 500); + + it.each(['network-loss', 'server-error'])( + 'keeps %s after a run POST unresolved instead of reporting execution failure', + async (scenario) => { + if (scenario === 'server-error') { + mockRunCreateResponseWithStatus({ error: { code: 'internal_error', message: 'unconfirmed' } }, 500); + + } else { + const original = fetchMock.getMockImplementation(); + if (!original) throw new Error('fetch fixture is missing'); + fetchMock.mockImplementation(async (...args: unknown[]) => { + if (String(args[0]).endsWith('/v1/runs')) throw new TypeError('run receipt transport lost'); + return original(...args); + }); + } + renderHook(() => useHubIntegration({ hubWS, hubClient })); + await act(async () => { + fireHubEvent(HUB_EVENTS.AGENT_DISPATCH, makeDispatchPayload({ delivery_id: 'uncertain-delivery' })); + }); + expect(hoisted.storeTasks[0]?.status).toBe('queued'); + expect(hoisted.storeTasks[0]?.error).toBeTruthy(); + expect(hubClient.ackTask).not.toHaveBeenCalled(); + expect(hubClient.failTask).not.toHaveBeenCalled(); + expect(hubClient.ackRelayCommand).not.toHaveBeenCalled(); + }, + ); + + it('still fails a definite pre-execution Edge admission rejection', async () => { + mockRunCreateResponseWithStatus({ error: { code: 'workdir_required', message: 'workspace required', traceId: 'trace_001' } }, 400); renderHook(() => useHubIntegration({ hubWS, hubClient })); await act(async () => { @@ -1076,6 +1200,406 @@ describe('useHubIntegration', () => { expect(hubClient.failTask).not.toHaveBeenCalled(); }); + // ── Persistent callback owner ───────────────────────── + + it('persists the explicit desktop owner and keeps forwarding callbacks', async () => { + mockRunCreateResponse({ + id: 'run-1', + runId: 'run-1', + projectId: 'proj', + threadId: 'sess', + status: 'started', + callbackOwner: 'desktop', + }); + + renderHook(() => useHubIntegration({ hubWS, hubClient })); + + await act(async () => { + fireHubEvent(HUB_EVENTS.AGENT_DISPATCH, makeDispatchPayload()); + }); + + expect(hoisted.storeTasks[0]?.callbackOwner).toBe('desktop'); + expect(hubClient.ackTask).toHaveBeenCalledWith('task-1', 'run-1'); + + act(() => { + fireEdgeEvent(makeEvent('run.agent.text_delta', { runId: 'run-1', content: 'Hello' })); + fireEdgeEvent(makeEvent('run.finished', { runId: 'run-1' })); + }); + + expect(hubClient.streamTaskEvent).toHaveBeenCalledTimes(1); + expect(hubClient.doneTask).toHaveBeenCalledWith('task-1', 'Hello', 'run-1'); + expect(hoisted.storeTasks[0]?.status).toBe('done'); + }); + + it('persists edge owner, suppresses Desktop task callbacks, keeps relay ACK and local terminal state', async () => { + mockRunCreateResponse({ + id: 'run-1', + runId: 'run-1', + projectId: 'proj', + threadId: 'sess', + status: 'started', + callbackOwner: 'edge', + }); + const onDispatch = vi.fn(); + + renderHook(() => + useHubIntegration({ + hubWS, + hubClient, + onDispatch, + dispatchTarget: { targetId: 'target-current', deviceId: 'desktop-current' }, + }), + ); + + const relayFrame = { + relay_command_id: 'relay-1', + command_type: 'agent.dispatch', + payload: JSON.stringify( + makeDispatchPayload({ + target_id: 'target-current', + edge_device_id: 'desktop-current', + delivery_id: 'd1', + }), + ), + }; + await act(async () => { + fireHubEvent(HUB_EVENTS.AGENT_DISPATCH, relayFrame); + }); + + expect(hoisted.storeTasks[0]?.callbackOwner).toBe('edge'); + expect(hoisted.storeTasks[0]?.runId).toBe('run-1'); + expect(hoisted.storeTasks[0]?.status).toBe('running'); + expect(hubClient.ackTask).not.toHaveBeenCalled(); + expect(hubClient.ackRelayCommand).toHaveBeenCalledWith('relay-1', 'desktop-current'); + expect(onDispatch).toHaveBeenCalledWith( + expect.objectContaining({ taskId: 'task-1', callbackOwner: 'edge' }), + ); + + act(() => { + fireEdgeEvent(makeEvent('run.agent.text_delta', { runId: 'run-1', content: 'Hello' })); + fireEdgeEvent(makeEvent('run.finished', { runId: 'run-1' })); + }); + + expect(hubClient.streamTaskEvent).not.toHaveBeenCalled(); + expect(hubClient.doneTask).not.toHaveBeenCalled(); + expect(hoisted.storeTasks[0]?.status).toBe('done'); + }); + + it('does not forward Edge callbacks while callbackOwner is unresolved', () => { + hoisted.getStoreState().addTask({ + taskId: 'task-unknown-owner', + agentId: 'codex', + prompt: 'p', + status: 'running', + runId: 'run-unknown-owner', + dispatchPayload: {}, + createdAt: '2026-01-01T00:00:00.000Z', + }); + renderHook(() => useHubIntegration({ hubWS, hubClient })); + + act(() => { + fireEdgeEvent( + makeEvent('run.agent.text_delta', { runId: 'run-unknown-owner', content: 'hidden output' }), + ); + fireEdgeEvent(makeEvent('run.finished', { runId: 'run-unknown-owner' })); + }); + + expect(hubClient.streamTaskEvent).not.toHaveBeenCalled(); + expect(hubClient.doneTask).not.toHaveBeenCalled(); + expect(hubClient.failTask).not.toHaveBeenCalled(); + expect(hoisted.storeTasks[0]?.status).toBe('done'); + }); + + it('backfills a missing owner from a same-run edge receipt without Desktop task ACK', async () => { + hoisted.getStoreState().addTask({ + taskId: 'task-1', + agentId: 'codex', + prompt: 'p', + status: 'running', + runId: 'run-1', + dispatchPayload: {}, + createdAt: '2026-01-01T00:00:00.000Z', + }); + mockRunCreateResponse({ + id: 'run-1', + runId: 'run-1', + projectId: 'proj', + threadId: 'sess', + status: 'started', + callbackOwner: 'edge', + }); + const onDispatch = vi.fn(); + + renderHook(() => + useHubIntegration({ + hubWS, + hubClient, + onDispatch, + dispatchTarget: { targetId: 'target-current', deviceId: 'desktop-current' }, + }), + ); + + const relayFrame = { + relay_command_id: 'relay-1', + command_type: 'agent.dispatch', + payload: JSON.stringify( + makeDispatchPayload({ + target_id: 'target-current', + edge_device_id: 'desktop-current', + delivery_id: 'd1', + }), + ), + }; + await act(async () => { + fireHubEvent(HUB_EVENTS.AGENT_DISPATCH, relayFrame); + }); + + expect(hoisted.storeTasks[0]?.callbackOwner).toBe('edge'); + expect(hoisted.storeTasks[0]?.runId).toBe('run-1'); + expect(hoisted.storeTasks[0]?.status).toBe('running'); + expect(hubClient.ackTask).not.toHaveBeenCalled(); + expect(hubClient.ackRelayCommand).toHaveBeenCalledWith('relay-1', 'desktop-current'); + expect(hubClient.failTask).not.toHaveBeenCalled(); + expect(onDispatch).not.toHaveBeenCalled(); + + act(() => { + fireEdgeEvent(makeEvent('run.agent.text_delta', { runId: 'run-1', content: 'edge output' })); + fireEdgeEvent(makeEvent('run.finished', { runId: 'run-1' })); + }); + + expect(hubClient.streamTaskEvent).not.toHaveBeenCalled(); + expect(hubClient.doneTask).not.toHaveBeenCalled(); + expect(hubClient.failTask).not.toHaveBeenCalled(); + expect(hoisted.storeTasks[0]?.status).toBe('done'); + }); + + it('keeps the first callback owner and does not overwrite it on a later response', async () => { + mockRunCreateResponse({ + id: 'run-1', + runId: 'run-1', + callbackOwner: 'edge', + status: 'started', + }); + const onDispatch = vi.fn(); + + renderHook(() => + useHubIntegration({ + hubWS, + hubClient, + onDispatch, + dispatchTarget: { targetId: 'target-current', deviceId: 'desktop-current' }, + }), + ); + + const relayFrame = { + relay_command_id: 'relay-1', + command_type: 'agent.dispatch', + payload: JSON.stringify( + makeDispatchPayload({ + target_id: 'target-current', + edge_device_id: 'desktop-current', + delivery_id: 'd1', + }), + ), + }; + + await act(async () => { + fireHubEvent(HUB_EVENTS.AGENT_DISPATCH, relayFrame); + }); + expect(hoisted.storeTasks[0]?.callbackOwner).toBe('edge'); + + mockRunCreateResponse({ + id: 'run-1', + runId: 'run-1', + callbackOwner: 'desktop', + status: 'running', + }); + await act(async () => { + fireHubEvent(HUB_EVENTS.AGENT_DISPATCH, relayFrame); + }); + + expect(hoisted.storeTasks[0]?.callbackOwner).toBe('edge'); + expect(hoisted.storeTasks[0]?.runId).toBe('run-1'); + expect(hoisted.storeTasks[0]?.error).toContain('conflict'); + expect(hubClient.ackTask).not.toHaveBeenCalled(); + expect(hubClient.ackRelayCommand).toHaveBeenCalledTimes(1); + expect(hubClient.failTask).not.toHaveBeenCalled(); + expect(onDispatch).toHaveBeenCalledTimes(1); + }); + + it('keeps a persisted run and suppresses ACK/relay-ACK on a different-run receipt conflict', async () => { + hoisted.getStoreState().addTask({ + taskId: 'task-1', + agentId: 'codex', + prompt: 'p', + status: 'running', + runId: 'run-original', + callbackOwner: 'desktop', + dispatchPayload: {}, + createdAt: '2026-01-01T00:00:00.000Z', + }); + mockRunCreateResponse({ + id: 'run-other', + runId: 'run-other', + projectId: 'proj', + threadId: 'sess', + status: 'started', + callbackOwner: 'desktop', + }); + + renderHook(() => + useHubIntegration({ + hubWS, + hubClient, + dispatchTarget: { targetId: 'target-current', deviceId: 'desktop-current' }, + }), + ); + + const relayFrame = { + relay_command_id: 'relay-conflict', + command_type: 'agent.dispatch', + payload: JSON.stringify( + makeDispatchPayload({ + target_id: 'target-current', + edge_device_id: 'desktop-current', + delivery_id: 'd1', + }), + ), + }; + await act(async () => { + fireHubEvent(HUB_EVENTS.AGENT_DISPATCH, relayFrame); + }); + + expect(hoisted.storeTasks[0]?.runId).toBe('run-original'); + expect(hoisted.storeTasks[0]?.callbackOwner).toBe('desktop'); + expect(hoisted.storeTasks[0]?.error).toContain('conflict'); + expect(hubClient.ackTask).not.toHaveBeenCalled(); + expect(hubClient.ackRelayCommand).not.toHaveBeenCalled(); + expect(hubClient.failTask).not.toHaveBeenCalled(); + }); + + it('fails closed before POST when Edge does not publish runCallbackOwnership', async () => { + fetchMock.mockImplementation(async (input: unknown) => { + const url = String(input); + if (url.endsWith('/v1/health')) { + return new Response( + JSON.stringify({ + status: 'ok', + version: 'old', + edgeId: 'edge-fixture', + capabilities: { directHubCallbacks: true }, + }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ); + } + return new Response('{}', { status: 200, headers: { 'Content-Type': 'application/json' } }); + }); + + renderHook(() => + useHubIntegration({ + hubWS, + hubClient, + dispatchTarget: { targetId: 'target-current', deviceId: 'desktop-current' }, + }), + ); + + const relayFrame = { + relay_command_id: 'relay-1', + command_type: 'agent.dispatch', + payload: JSON.stringify( + makeDispatchPayload({ + target_id: 'target-current', + edge_device_id: 'desktop-current', + delivery_id: 'd1', + }), + ), + }; + await act(async () => { + fireHubEvent(HUB_EVENTS.AGENT_DISPATCH, relayFrame); + }); + + expect(fetchCallCountEndingWith('/v1/runs')).toBe(0); + expect(hoisted.storeTasks[0]?.status).toBe('queued'); + expect(hoisted.storeTasks[0]?.runId).toBeUndefined(); + expect(hoisted.storeTasks[0]?.callbackOwner).toBeUndefined(); + expect(hoisted.storeTasks[0]?.error).toContain('runCallbackOwnership'); + expect(hubClient.ackTask).not.toHaveBeenCalled(); + expect(hubClient.ackRelayCommand).not.toHaveBeenCalled(); + expect(hubClient.failTask).not.toHaveBeenCalled(); + }); + + it('fails closed when a capable Edge returns an accepted run without callbackOwner', async () => { + mockRunCreateResponseRaw({ + id: 'run-1', + runId: 'run-1', + projectId: 'proj', + threadId: 'sess', + status: 'started', + }); + + renderHook(() => useHubIntegration({ hubWS, hubClient })); + + await act(async () => { + fireHubEvent(HUB_EVENTS.AGENT_DISPATCH, makeDispatchPayload()); + }); + + expect(hoisted.storeTasks[0]?.status).toBe('queued'); + expect(hoisted.storeTasks[0]?.runId).toBeUndefined(); + expect(hoisted.storeTasks[0]?.callbackOwner).toBeUndefined(); + expect(hoisted.storeTasks[0]?.error).toContain('callbackOwner'); + expect(hubClient.ackTask).not.toHaveBeenCalled(); + expect(hubClient.failTask).not.toHaveBeenCalled(); + }); + + it('does not failTask an edge-owned task on lifecycle failure, but keeps local failure state', async () => { + mockRunCreateResponse({ + id: 'run-1', + runId: 'run-1', + callbackOwner: 'edge', + status: 'started', + }); + + renderHook(() => useHubIntegration({ hubWS, hubClient })); + + await act(async () => { + fireHubEvent(HUB_EVENTS.AGENT_DISPATCH, makeDispatchPayload()); + }); + + act(() => { + fireEdgeEvent(makeEvent('run.failed', { runId: 'run-1', error: 'boom' })); + }); + + expect(hubClient.failTask).not.toHaveBeenCalled(); + expect(hoisted.storeTasks[0]?.status).toBe('failed'); + expect(hoisted.storeTasks[0]?.error).toBe('boom'); + }); + + it('does not failTask an edge-owned task on Hub cancel, but keeps local failed state', async () => { + mockRunCreateResponse({ + id: 'run-1', + runId: 'run-1', + callbackOwner: 'edge', + status: 'started', + }); + + renderHook(() => useHubIntegration({ hubWS, hubClient })); + + await act(async () => { + fireHubEvent(HUB_EVENTS.AGENT_DISPATCH, makeDispatchPayload()); + }); + + await act(async () => { + fireHubEvent(HUB_EVENTS.AGENT_CANCEL, { task_id: 'task-1' }); + }); + + expect(fetchMock).toHaveBeenCalledWith( + 'http://127.0.0.1:3210/v1/runs/run-1:cancel', + expect.objectContaining({ method: 'POST' }), + ); + expect(hubClient.failTask).not.toHaveBeenCalled(); + expect(hoisted.storeTasks[0]?.status).toBe('failed'); + }); + // ── Edge events → Hub callbacks ────────────────────── it('streams text_delta to Hub', async () => { diff --git a/app/desktop/src/hooks/executionIntent.test.ts b/app/desktop/src/hooks/executionIntent.test.ts new file mode 100644 index 000000000..ba863ecba --- /dev/null +++ b/app/desktop/src/hooks/executionIntent.test.ts @@ -0,0 +1,105 @@ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { + buildEdgeRunBody, + normalizeRuntimeAgentId, + type DispatchTargetBindingEvidence, + type EdgeRunRequestBody, +} from './hubIntegrationMappers'; + +interface ExecutionIntentFixtureCase { + name: string; + payload: Record; + expectedIntent: Record; +} + +interface ExecutionIntentFixture { + version: number; + cases: ExecutionIntentFixtureCase[]; +} + +const fixture = JSON.parse( + readFileSync(resolve(process.cwd(), '../../tests/fixtures/dispatch/execution-intent.json'), 'utf8'), +) as ExecutionIntentFixture; + +const binding: DispatchTargetBindingEvidence = { + expectedTargetId: 'target-fixture', + observedTargetId: 'target-fixture', + expectedEdgeDeviceId: 'device-fixture', + observedEdgeDeviceId: 'device-fixture', + status: 'matched', +}; + +type IntentOnlyBody = Omit< + EdgeRunRequestBody, + 'threadId' | 'projectId' | 'targetId' | 'edgeDeviceId' | 'dispatchTargetEvidence' | 'callbackOwner' +>; + +function stripTransportOnly(body: EdgeRunRequestBody): IntentOnlyBody { + const intent = { ...body }; + delete intent.threadId; + delete intent.projectId; + delete intent.targetId; + delete intent.edgeDeviceId; + delete intent.dispatchTargetEvidence; + delete intent.callbackOwner; + return intent; +} + +describe('Desktop Hub execution intent safe integer projection', () => { + it('falls back to the next safe integer alias while preserving zero and negative values', () => { + const decimal = buildEdgeRunBody( + { model_params: JSON.stringify({ max_thinking_tokens: 1.5, maxThinkingTokens: 42 }) }, + '', + 'p', + 'codex', + null, + ); + expect(decimal.maxThinkingTokens).toBe(42); + + const overflow = buildEdgeRunBody( + { model_params: JSON.stringify({ max_thinking_tokens: Number.MAX_SAFE_INTEGER + 1, maxThinkingTokens: -3 }) }, + '', + 'p', + 'codex', + null, + ); + expect(overflow.maxThinkingTokens).toBe(-3); + + const zero = buildEdgeRunBody( + { model_params: JSON.stringify({ max_thinking_tokens: 0 }) }, + '', + 'p', + 'codex', + null, + ); + expect(zero.maxThinkingTokens).toBe(0); + }); +}); + +describe('Desktop Hub execution intent projection', () => { + for (const fixtureCase of fixture.cases) { + it(fixtureCase.name, () => { + const payload = fixtureCase.payload; + const prompt = typeof payload.prompt === 'string' ? payload.prompt : ''; + const rawAgentId = typeof payload.agent_type === 'string' ? payload.agent_type : ''; + const body = buildEdgeRunBody( + payload, + '', + prompt, + normalizeRuntimeAgentId(rawAgentId), + binding, + ); + const intent = stripTransportOnly(body); + + expect(intent).toEqual(fixtureCase.expectedIntent); + + const actualSchema = intent.structuredOutputSchema; + const expectedSchema = fixtureCase.expectedIntent.structuredOutputSchema; + if (typeof actualSchema === 'string' && typeof expectedSchema === 'string') { + expect(JSON.parse(actualSchema)).toEqual(JSON.parse(expectedSchema)); + } + }); + } +}); diff --git a/app/desktop/src/hooks/hubIntegrationEdgeApi.ts b/app/desktop/src/hooks/hubIntegrationEdgeApi.ts index a77e12541..6674b5846 100644 --- a/app/desktop/src/hooks/hubIntegrationEdgeApi.ts +++ b/app/desktop/src/hooks/hubIntegrationEdgeApi.ts @@ -2,7 +2,9 @@ // Isolated from React so permission/thread helpers stay unit-testable. import { edgeAuthHeaders } from '@/api/edgeAuth'; +import { withHubAbortTimeout } from '@shared/hub/hubClientTransportUtils'; import type { EdgePermissionDecisionControl } from './hubIntegrationMappers'; +import { parseRecord } from './hubIntegrationParseHelpers'; export function edgeRequestInit(init: RequestInit = {}, baseHeaders?: HeadersInit): RequestInit { const headers = edgeAuthHeaders(baseHeaders); @@ -53,3 +55,59 @@ export async function ensureEdgeThread( throw new Error(`Edge POST /v1/threads returned ${resp.status}: ${errorText}`); } } + +export interface EdgeRunCallbackCapabilityProbe { + supported: boolean; + reason?: string; +} + +export const EDGE_HEALTH_CAPABILITY_TIMEOUT_MS = 5_000; + +/** + * Fail-closed capability gate for Desktop-managed callback ownership. + * Only a new Edge that explicitly publishes runCallbackOwnership=true is + * allowed to receive a Hub dispatch; unknown, missing, false, or failed + * health probes are treated as unusable for this route. + */ +export async function probeEdgeRunCallbackOwnership( + edgeBaseUrl: string, +): Promise { + try { + return await withHubAbortTimeout(EDGE_HEALTH_CAPABILITY_TIMEOUT_MS, async (signal) => { + const response = await fetch(`${edgeBaseUrl.replace(/\/$/, '')}/v1/health`, { signal }); + if (!response.ok) { + return { + supported: false, + reason: `Edge /v1/health returned ${response.status}`, + }; + } + + let raw: unknown; + try { + raw = await response.json(); + } catch (error) { + return { + supported: false, + reason: error instanceof Error ? error.message : 'Edge /v1/health returned non-JSON', + }; + } + + const root = parseRecord(raw); + const health = parseRecord(root.data); + const source = Object.keys(health).length > 0 ? health : root; + const capabilities = parseRecord(source.capabilities); + if (capabilities.runCallbackOwnership !== true) { + return { + supported: false, + reason: 'Edge /v1/health does not publish runCallbackOwnership=true', + }; + } + return { supported: true }; + }); + } catch (error) { + return { + supported: false, + reason: error instanceof Error ? error.message : String(error), + }; + } +} diff --git a/app/desktop/src/hooks/hubIntegrationHelpers.test.ts b/app/desktop/src/hooks/hubIntegrationHelpers.test.ts index 7bd1e06da..7fdc714c4 100644 --- a/app/desktop/src/hooks/hubIntegrationHelpers.test.ts +++ b/app/desktop/src/hooks/hubIntegrationHelpers.test.ts @@ -7,8 +7,10 @@ import { getFirstString, getString, parseRecord, + parseRunnerMessages, parseStringArray, parseStringRecord, + serializeStructuredOutputSchema, } from './hubIntegrationParseHelpers'; import { bindDispatchPayload, @@ -19,10 +21,12 @@ import { getTeamRouteContext, hasTaskProgressed, isAdmissionUncertain, + isEdgeOwnedTask, isTerminalBridgeTask, isTransientAdmissionRejection, normalizeRouteDecision, normalizeRuntimeAgentId, + parseEdgeCallbackOwner, parsePermissionDecisionControl, permissionDecisionControlKey, routeDecisionFromRuntimePayload, @@ -50,6 +54,7 @@ describe('hubIntegrationParseHelpers', () => { expect(getString({ name: 'x' }, 'name')).toBe('x'); expect(getString({ name: 1 }, 'name')).toBe(''); expect(getFirstString('', ' ', 'ok')).toBe('ok'); + expect(getFirstString(' keep ')).toBe(' keep '); expect(getFirstString(null, undefined)).toBeUndefined(); expect(getFirstBoolean(null, true, false)).toBe(true); expect(getFirstNumber('1', Number.NaN, 2.5)).toBe(2.5); @@ -57,6 +62,34 @@ describe('hubIntegrationParseHelpers', () => { expect(boolValue('true')).toBeUndefined(); }); + it('parseRunnerMessages preserves messages without a timestamp', () => { + expect( + parseRunnerMessages([ + { role: 'user', content: 'no timestamp' }, + { role: 'assistant', content: 'has timestamp', timestamp: '2026-01-01T00:00:00Z' }, + { role: 'assistant', content: 'bad timestamp', timestamp: 7 }, + ]), + ).toEqual([ + { role: 'user', content: 'no timestamp' }, + { role: 'assistant', content: 'has timestamp', timestamp: '2026-01-01T00:00:00Z' }, + ]); + expect(parseRunnerMessages([{ role: 'user', content: 'missing only' }])).toEqual([ + { role: 'user', content: 'missing only' }, + ]); + }); + + it('serializes boolean and finite number schema candidates in alias order', () => { + expect(serializeStructuredOutputSchema(false)).toBe('false'); + expect(serializeStructuredOutputSchema(true)).toBe('true'); + expect(serializeStructuredOutputSchema(0)).toBe('0'); + expect(serializeStructuredOutputSchema(-1.5)).toBe('-1.5'); + expect(serializeStructuredOutputSchema(false, { type: 'object' })).toBe('false'); + expect(serializeStructuredOutputSchema('', 42)).toBe('42'); + expect(serializeStructuredOutputSchema(Number.NaN, true)).toBe('true'); + expect(serializeStructuredOutputSchema(Infinity, false)).toBe('false'); + expect(serializeStructuredOutputSchema(null)).toBeUndefined(); + }); + it('parseStringArray and parseStringRecord filter empty values', () => { expect(parseStringArray(['a', '', 'b'])).toEqual(['a', 'b']); expect(parseStringArray('["x"," "]')).toEqual(['x']); @@ -295,6 +328,19 @@ describe('hubIntegrationMappers', () => { expect(Object.values(body).every((v) => v !== undefined)).toBe(true); }); + it('parses persistent callback owner and detects edge-owned tasks', () => { + expect(parseEdgeCallbackOwner({ id: 'run-a', callbackOwner: 'edge' })).toBe('edge'); + expect( + parseEdgeCallbackOwner({ code: 'ok', data: { runId: 'run-b', callbackOwner: 'desktop' } }), + ).toBe('desktop'); + expect(parseEdgeCallbackOwner({ id: 'run-c' })).toBeUndefined(); + expect(parseEdgeCallbackOwner({ id: 'run-d', callback_owner: 'invalid' })).toBeUndefined(); + + expect(isEdgeOwnedTask(makeTask({ callbackOwner: 'edge' }))).toBe(true); + expect(isEdgeOwnedTask(makeTask({ callbackOwner: 'desktop' }))).toBe(false); + expect(isEdgeOwnedTask(undefined)).toBe(false); + }); + it('extractRunOutputBatch only joins stdout chunk text', () => { expect( extractRunOutputBatch({ diff --git a/app/desktop/src/hooks/hubIntegrationMappers.ts b/app/desktop/src/hooks/hubIntegrationMappers.ts index af2d1d918..855e44f2a 100644 --- a/app/desktop/src/hooks/hubIntegrationMappers.ts +++ b/app/desktop/src/hooks/hubIntegrationMappers.ts @@ -2,16 +2,20 @@ // No React, no Hub client side effects — safe for unit tests. import type { CoordinatorRouteDecision } from '@/api/hubClient'; +import type { StartRunRequest } from '@shared/types'; import type { AgentTask } from '@/stores/taskBridgeStore'; import { boolValue, compactRecord, getFirstBoolean, - getFirstNumber, + getFirstSafeInteger, getFirstString, parseRecord, + parseRunnerMessages, parseStringArray, parseStringRecord, + serializeStructuredOutputSchema, + type RunnerMessageLike, } from './hubIntegrationParseHelpers'; interface TeamRouteContext { @@ -32,7 +36,7 @@ export interface HubDispatchTarget { deviceId: string; } -interface DispatchTargetBindingEvidence { +export interface DispatchTargetBindingEvidence { expectedTargetId: string; observedTargetId?: string; expectedEdgeDeviceId: string; @@ -40,6 +44,29 @@ interface DispatchTargetBindingEvidence { status: 'matched' | 'mismatch'; } +export type EdgeCallbackOwner = 'edge' | 'desktop'; + +export type EdgeRunRequestBody = Omit< + StartRunRequest, + 'messages' | 'pinnedMessages' +> & { + [key: string]: unknown; + deliveryId?: string; + callbackOwner?: EdgeCallbackOwner; + trace_id?: string; + targetId?: string; + edgeDeviceId?: string; + dispatchTargetEvidence?: { + expectedTargetId: string; + observedTargetId?: string; + expectedEdgeDeviceId: string; + observedEdgeDeviceId?: string; + targetStatus: 'matched' | 'mismatch'; + }; + messages?: RunnerMessageLike[]; + pinnedMessages?: RunnerMessageLike[]; +}; + export function isTerminalBridgeTask(task: AgentTask): boolean { return task.status === 'done' || task.status === 'failed'; } @@ -202,7 +229,7 @@ export function buildDispatchTargetBinding( export function bindDispatchPayload( data: Record, binding: DispatchTargetBindingEvidence | null, -): Record { +): EdgeRunRequestBody { if (!binding) return data; return { ...data, @@ -240,7 +267,7 @@ export function buildEdgeRunBody( parseStringArray(modelParams.allowed_tools) ?? parseStringArray(modelParams.allowedTools); - return compactRecord>({ + return compactRecord({ threadId, prompt: prompt || undefined, agentId: agentId || undefined, @@ -257,7 +284,7 @@ export function buildEdgeRunBody( data.thinking_mode, data.thinkingMode, ), - maxThinkingTokens: getFirstNumber( + maxThinkingTokens: getFirstSafeInteger( modelParams.max_thinking_tokens, modelParams.maxThinkingTokens, data.max_thinking_tokens, @@ -276,7 +303,7 @@ export function buildEdgeRunBody( data.include_partial, data.includePartial, ), - structuredOutputSchema: getFirstString( + structuredOutputSchema: serializeStructuredOutputSchema( modelParams.structured_output_schema, modelParams.structuredOutputSchema, data.structured_output_schema, @@ -301,6 +328,15 @@ export function buildEdgeRunBody( ephemeral: getFirstBoolean(modelParams.ephemeral, data.ephemeral), hubTaskId: getFirstString(data.task_id), deliveryId: getFirstString(data.delivery_id, data.deliveryId), + sessionId: getFirstString(modelParams.session_id, modelParams.sessionId), + continue: getFirstBoolean(modelParams.continue, data.continue), + fork: getFirstBoolean(modelParams.fork, data.fork), + messages: parseRunnerMessages(data.messages) ?? parseRunnerMessages(modelParams.messages), + pinnedMessages: + parseRunnerMessages(data.pinned_messages) ?? + parseRunnerMessages(data.pinnedMessages), + trace_id: getFirstString(data.trace_id, data.traceId), + callbackOwner: 'desktop', targetId: targetBinding?.expectedTargetId, edgeDeviceId: targetBinding?.expectedEdgeDeviceId, dispatchTargetEvidence: targetBinding @@ -338,6 +374,19 @@ export function extractCreatedRunId(value: unknown): string { return runId; } +export function parseEdgeCallbackOwner(value: unknown): EdgeCallbackOwner | undefined { + const root = parseRecord(value); + const isEnvelope = + typeof root.code === 'string' && Object.prototype.hasOwnProperty.call(root, 'data'); + const run = isEnvelope ? parseRecord(root.data) : root; + const owner = getFirstString(run.callbackOwner, run.callback_owner)?.toLowerCase(); + return owner === 'edge' || owner === 'desktop' ? owner : undefined; +} + +export function isEdgeOwnedTask(task: AgentTask | undefined): boolean { + return task?.callbackOwner === 'edge'; +} + /** * Read the canonical Edge error code from `{ error: { code, message, traceId } }`. * Accepts an already-parsed object or a raw JSON string. diff --git a/app/desktop/src/hooks/hubIntegrationParseHelpers.ts b/app/desktop/src/hooks/hubIntegrationParseHelpers.ts index a9fb31f10..547161cfb 100644 --- a/app/desktop/src/hooks/hubIntegrationParseHelpers.ts +++ b/app/desktop/src/hooks/hubIntegrationParseHelpers.ts @@ -1,6 +1,11 @@ // Pure parse/record helpers for the Hub↔Edge integration bridge. // Kept free of React / Hub client / Edge fetch so unit tests stay light. +import type { RunnerMessage } from '@shared/types'; + +export type RunnerMessageLike = Pick & + Partial>; + export function parseRecord(value: unknown): Record { if (!value) return {}; if (typeof value === 'object' && !Array.isArray(value)) return value as Record; @@ -46,6 +51,14 @@ export function getFirstNumber(...values: unknown[]): number | undefined { return undefined; } +/** Edge integer fields require a finite value within Number/Go int64-safe range. */ +export function getFirstSafeInteger(...values: unknown[]): number | undefined { + for (const value of values) { + if (typeof value === 'number' && Number.isSafeInteger(value)) return value; + } + return undefined; +} + export function parseStringArray(value: unknown): string[] | undefined { let source = value; if (typeof value === 'string') { @@ -73,3 +86,46 @@ export function parseStringRecord(value: unknown): Record | unde export function boolValue(value: unknown): boolean | undefined { return typeof value === 'boolean' ? value : undefined; } + +/** Parse a Hub message list, preserving role/content and an optional timestamp exactly. */ +export function parseRunnerMessages(value: unknown): RunnerMessageLike[] | undefined { + let source = value; + if (typeof value === 'string') { + try { + source = JSON.parse(value); + } catch { + return undefined; + } + } + if (!Array.isArray(source)) return undefined; + + const messages = source.filter((item): item is RunnerMessageLike => { + if (!item || typeof item !== 'object' || Array.isArray(item)) return false; + const record = item as Record; + return ( + typeof record.role === 'string' && + typeof record.content === 'string' && + (record.timestamp === undefined || typeof record.timestamp === 'string') + ); + }); + + return messages.length > 0 ? messages : undefined; +} + +/** Return the first schema candidate as an Edge wire string. */ +export function serializeStructuredOutputSchema(...values: unknown[]): string | undefined { + for (const value of values) { + if (typeof value === 'string' && value.trim()) return value.trim(); + if ( + typeof value === 'boolean' || + (typeof value === 'number' && Number.isFinite(value)) + ) { + return JSON.stringify(value); + } + if (value !== null && typeof value === 'object') { + const serialized = JSON.stringify(value); + if (serialized) return serialized; + } + } + return undefined; +} diff --git a/app/desktop/src/hooks/useHubIntegration.ts b/app/desktop/src/hooks/useHubIntegration.ts index e229902b4..4c1790cdc 100644 --- a/app/desktop/src/hooks/useHubIntegration.ts +++ b/app/desktop/src/hooks/useHubIntegration.ts @@ -24,7 +24,7 @@ import { useToastStore } from '@shared/ui/toast'; import { hubQueryKeys } from '@shared/stores/queryKeys'; import { useTaskBridgeStore, type AgentTask } from '@/stores/taskBridgeStore'; import { queryClient } from '@/api/queryClient'; -import { edgeRequestInit, ensureEdgeThread, postEdgePermissionDecision } from './hubIntegrationEdgeApi'; +import { edgeRequestInit, ensureEdgeThread, postEdgePermissionDecision, probeEdgeRunCallbackOwnership } from './hubIntegrationEdgeApi'; import { bindDispatchPayload, buildDispatchTargetBinding, @@ -32,6 +32,7 @@ import { parseDispatchFrame, extractCreatedRunId, extractRunOutputBatch, + parseEdgeCallbackOwner, FINAL_OUTPUT_MAX_CHARS, getTeamRouteContext, hasTaskProgressed, @@ -80,6 +81,10 @@ interface HubIntegrationHandle { const HUB_AGENT_CONTROL_EVENT = 'agent.control'; +function showEdgeReviewNotice(error: string): void { + useToastStore.getState().showToast('warning', error, { duration: 10_000 }); +} + // ── Hook ────────────────────────────────────────────── export function useHubIntegration(options: HubIntegrationOptions): HubIntegrationHandle { @@ -145,16 +150,19 @@ export function useHubIntegration(options: HubIntegrationOptions): HubIntegratio if (isTerminalBridgeTask(task)) return; const taskId = task.taskId; + const desktopOwned = task.callbackOwner === 'desktop'; switch (event.type) { case 'run.agent.text_delta': { const content = typeof payload.content === 'string' ? payload.content : ''; if (content) { rememberOutput(runId, content); - void catchHubReport( - `streamTaskEvent:${taskId}:${event.type}`, - hubClient.streamTaskEvent(taskId, event.type, payload, { runId }), - ); + if (desktopOwned) { + void catchHubReport( + `streamTaskEvent:${taskId}:${event.type}`, + hubClient.streamTaskEvent(taskId, event.type, payload, { runId }), + ); + } } break; } @@ -163,10 +171,12 @@ export function useHubIntegration(options: HubIntegrationOptions): HubIntegratio const content = typeof payload.content === 'string' ? payload.content : ''; if (content) { rememberOutput(runId, content); - void catchHubReport( - `streamTaskEvent:${taskId}:${event.type}`, - hubClient.streamTaskEvent(taskId, event.type, payload, { runId }), - ); + if (desktopOwned) { + void catchHubReport( + `streamTaskEvent:${taskId}:${event.type}`, + hubClient.streamTaskEvent(taskId, event.type, payload, { runId }), + ); + } } break; } @@ -175,17 +185,19 @@ export function useHubIntegration(options: HubIntegrationOptions): HubIntegratio const content = extractRunOutputBatch(payload); if (content) { rememberOutput(runId, content); - void catchHubReport( - `streamTaskEvent:${taskId}:${event.type}`, - hubClient.streamTaskEvent(taskId, event.type, payload, { runId }), - ); + if (desktopOwned) { + void catchHubReport( + `streamTaskEvent:${taskId}:${event.type}`, + hubClient.streamTaskEvent(taskId, event.type, payload, { runId }), + ); + } } break; } case 'run.agent.thinking': { const content = typeof payload.content === 'string' ? payload.content : ''; - if (content) { + if (content && desktopOwned) { void catchHubReport( `streamTaskEvent:${taskId}:${event.type}`, hubClient.streamTaskEvent(taskId, event.type, payload, { runId }), @@ -199,44 +211,52 @@ export function useHubIntegration(options: HubIntegrationOptions): HubIntegratio case 'run.agent.file_change': case 'run.agent.permission_requested': case 'run.agent.permission_decided': - // Forward the canonical typed runtime event so Hub can persist and replay it. - void catchHubReport( - `streamTaskEvent:${taskId}:${event.type}`, - hubClient.streamTaskEvent(taskId, event.type, payload, { runId }), - ); + // Edge-owned runs report these callbacks themselves; do not duplicate. + if (desktopOwned) { + void catchHubReport( + `streamTaskEvent:${taskId}:${event.type}`, + hubClient.streamTaskEvent(taskId, event.type, payload, { runId }), + ); + } break; case 'run.agent.route_decision': { - const decision = routeDecisionFromRuntimePayload(payload); - if (decision) { - postRouteDecision(task, decision); + if (desktopOwned) { + const decision = routeDecisionFromRuntimePayload(payload); + if (decision) { + postRouteDecision(task, decision); + } + void catchHubReport( + `streamTaskEvent:${taskId}:${event.type}`, + hubClient.streamTaskEvent(taskId, event.type, payload, { runId }), + ); } - void catchHubReport( - `streamTaskEvent:${taskId}:${event.type}`, - hubClient.streamTaskEvent(taskId, event.type, payload, { runId }), - ); break; } case 'run.agent.result': { - const decision = routeDecisionFromRuntimePayload(payload); - if (decision) { - postRouteDecision(task, decision); + if (desktopOwned) { + const decision = routeDecisionFromRuntimePayload(payload); + if (decision) { + postRouteDecision(task, decision); + } + void catchHubReport( + `streamTaskEvent:${taskId}:${event.type}`, + hubClient.streamTaskEvent(taskId, event.type, payload, { runId }), + ); } - void catchHubReport( - `streamTaskEvent:${taskId}:${event.type}`, - hubClient.streamTaskEvent(taskId, event.type, payload, { runId }), - ); const success = payload.success !== false; if (success) { const output = typeof payload.content === 'string' ? payload.content : outputByRunRef.current.get(runId) || JSON.stringify(payload); - void catchHubReport( - `doneTask:${taskId}`, - hubClient.doneTask(taskId, output, runId), - ); + if (desktopOwned) { + void catchHubReport( + `doneTask:${taskId}`, + hubClient.doneTask(taskId, output, runId), + ); + } store.getState().updateTask(taskId, { status: 'done', }); @@ -244,10 +264,12 @@ export function useHubIntegration(options: HubIntegrationOptions): HubIntegratio } else { const error = typeof payload.error === 'string' ? payload.error : 'Agent reported failure'; - void catchHubReport( - `failTask:${taskId}`, - hubClient.failTask(taskId, error, runId), - ); + if (desktopOwned) { + void catchHubReport( + `failTask:${taskId}`, + hubClient.failTask(taskId, error, runId), + ); + } store.getState().updateTask(taskId, { status: 'failed', error, @@ -259,11 +281,13 @@ export function useHubIntegration(options: HubIntegrationOptions): HubIntegratio } case 'run.finished': { - const output = outputByRunRef.current.get(runId) || 'Run finished'; - void catchHubReport( - `doneTask:${taskId}`, - hubClient.doneTask(taskId, output, runId), - ); + if (desktopOwned) { + const output = outputByRunRef.current.get(runId) || 'Run finished'; + void catchHubReport( + `doneTask:${taskId}`, + hubClient.doneTask(taskId, output, runId), + ); + } store.getState().updateTask(taskId, { status: 'done', }); @@ -273,10 +297,12 @@ export function useHubIntegration(options: HubIntegrationOptions): HubIntegratio case 'run.failed': { const error = typeof payload.error === 'string' ? payload.error : 'Run lifecycle failure'; - void catchHubReport( - `failTask:${taskId}`, - hubClient.failTask(taskId, error, runId), - ); + if (desktopOwned) { + void catchHubReport( + `failTask:${taskId}`, + hubClient.failTask(taskId, error, runId), + ); + } store.getState().updateTask(taskId, { status: 'failed', error, @@ -286,10 +312,12 @@ export function useHubIntegration(options: HubIntegrationOptions): HubIntegratio } case 'run.cancelled': { - void catchHubReport( - `failTask:${taskId}`, - hubClient.failTask(taskId, 'Run cancelled', runId), - ); + if (desktopOwned) { + void catchHubReport( + `failTask:${taskId}`, + hubClient.failTask(taskId, 'Run cancelled', runId), + ); + } store.getState().updateTask(taskId, { status: 'failed', error: 'Run cancelled', @@ -383,14 +411,34 @@ export function useHubIntegration(options: HubIntegrationOptions): HubIntegratio store.getState().addTask(task); - // Create Edge run + // Until a valid run receipt arrives, a POST failure may still have started work. + let runPostUnconfirmed = false; try { + // Fail closed on old/unknown Edge builds before their executor can start + // a Desktop-mediated run without callback ownership semantics. + const capability = await probeEdgeRunCallbackOwnership(edgeBaseUrl); + if (!capability.supported) { + const error = [ + 'Edge callback ownership capability unavailable; upgrade Local Edge before Hub dispatch.', + capability.reason, + ].filter(Boolean).join(' '); + const currentTask = store.getState().tasks.find((t) => t.taskId === taskId); + if (!hasTaskProgressed(currentTask)) { + store.getState().updateTask(taskId, { error }); + if (currentTask?.error !== error) { + showEdgeReviewNotice(error); + } + } + return; + } + await ensureEdgeThread( edgeBaseUrl, threadId, getString(data, 'display_name') || 'Hub dispatch', ); + runPostUnconfirmed = true; const runResp = await fetch( `${edgeBaseUrl}/v1/runs`, edgeRequestInit( @@ -405,8 +453,8 @@ export function useHubIntegration(options: HubIntegrationOptions): HubIntegratio if (!runResp.ok) { // Classify definite transient admission rejections (delivery_busy, // active_run_exists, too_many_concurrent_runs, admission_persist_failed). - // Anything else keeps the existing permanent path unless it is an - // explicit admission_uncertain, which needs manual review. + // Client-side admission rejections are definite; transport/5xx + // failures remain unresolved because execution may already have begun. const errorText = await runResp.text().catch(() => 'Unknown error'); if (isTransientAdmissionRejection(runResp.status, errorText)) { // Keep the task queued/waiting; retry ownership stays with the Hub @@ -426,15 +474,17 @@ export function useHubIntegration(options: HubIntegrationOptions): HubIntegratio // The bridge has no task-error panel. Surface the need for review // through the existing notification UI, once per unchanged reason. if (currentTask?.error !== error) { - useToastStore.getState().showToast('warning', error, { duration: 10_000 }); + showEdgeReviewNotice(error); } } return; } + runPostUnconfirmed = runResp.status < 400 || runResp.status >= 500; throw new Error('Edge POST /v1/runs returned ' + runResp.status + ': ' + errorText); } - const runId = extractCreatedRunId(await runResp.json()); + const runResponse = await runResp.json(); + const responseOwner = parseEdgeCallbackOwner(runResponse); // Re-read before update to resist an async completion race: a duplicate // dispatch must not downgrade a task that already reached running/done/ @@ -442,18 +492,72 @@ export function useHubIntegration(options: HubIntegrationOptions): HubIntegratio const existingTask = store.getState().tasks.find((t) => t.taskId === taskId); const taskProgressed = hasTaskProgressed(existingTask); + // A new Edge must return an explicit persistent owner. Absent/invalid + // owner means the actual callback route is unresolved: keep queued for + // review and do not ACK/FAIL or relay-ACK. + if (!responseOwner) { + const error = 'Edge accepted the run without callbackOwner; callback ownership is unresolved.'; + const currentTask = store.getState().tasks.find((t) => t.taskId === taskId); + if (!hasTaskProgressed(currentTask)) { + store.getState().updateTask(taskId, { error }); + if (currentTask?.error !== error) { + showEdgeReviewNotice(error); + } + } + return; + } + + const runId = extractCreatedRunId(runResponse); + // Business state/mapping is only written once (first accepted instance). // Also clear a stale admission_uncertain manual-review note. Existing - // progressed state keeps its own error untouched. - if (!taskProgressed) { - store.getState().updateTask(taskId, { runId, status: 'running', error: undefined }); + // progressed state may backfill a missing persistent owner from an + // explicit receipt with the same runId, but cannot overwrite a different + // owner or runId. Conflicts stay local for review without any ACK/FAIL. + if (taskProgressed && existingTask) { + const existingRunId = existingTask.runId; + const existingOwner = existingTask.callbackOwner; + const runConflict = existingRunId !== undefined && existingRunId !== runId; + const ownerConflict = existingOwner !== undefined && existingOwner !== responseOwner; + + if (runConflict || ownerConflict) { + const conflictError = [ + 'Callback ownership receipt conflicts with persisted task state.', + `persisted run=${existingRunId ?? 'n/a'}, owner=${existingOwner ?? 'unknown'}`, + `receipt run=${runId}, owner=${responseOwner}`, + ].join(' '); + store.getState().updateTask(taskId, { error: conflictError }); + if (existingTask.error !== conflictError) { + showEdgeReviewNotice(conflictError); + } + return; + } + + const updates: Partial = {}; + if (existingOwner === undefined) updates.callbackOwner = responseOwner; + if (existingRunId === undefined) updates.runId = runId; + if (Object.keys(updates).length > 0) { + store.getState().updateTask(taskId, updates); + } + } else if (!taskProgressed) { + store.getState().updateTask(taskId, { + runId, + status: 'running', + callbackOwner: responseOwner, + error: undefined, + }); } - // Every accepted delivery (first accept AND successful replay) is - // idempotently acknowledged. If the first ACK was lost in transit, Hub - // re-dispatches and we must re-ACK so the outbox/relay can converge; a - // double ACK here is expected recovery, not a race defect. - void catchHubReport(`ackTask:${taskId}`, hubClient.ackTask(taskId, runId)); + const settledTask = store.getState().tasks.find((t) => t.taskId === taskId); + const desktopOwned = settledTask?.callbackOwner === 'desktop'; + + // Desktop callbacks and task ACK are sent only for an explicit + // desktop-owned receipt. Edge-owned and unresolved owners do not get + // duplicate Desktop task work; the relay command ACK still reports that + // this device handled the transport delivery. + if (desktopOwned) { + void catchHubReport(`ackTask:${taskId}`, hubClient.ackTask(taskId, runId)); + } if (relayCommandId && dispatchTarget?.deviceId) { void catchHubReport( `ackRelayCommand:${relayCommandId}`, @@ -472,10 +576,16 @@ export function useHubIntegration(options: HubIntegrationOptions): HubIntegratio } catch (err) { const errorMsg = err instanceof Error ? err.message : String(err); // A duplicate delivery's permanent failure must not corrupt an already - // running/terminal task. Only a not-yet-accepted delivery is failed here - // (permanent errors on new queued tasks still fail). + // running/terminal task. Report failure only for a definite rejection + // before execution, not an unconfirmed POST or malformed receipt. const currentTask = store.getState().tasks.find((t) => t.taskId === taskId); if (!hasTaskProgressed(currentTask)) { + if (runPostUnconfirmed) { + const error = 'Edge admission result is uncertain: ' + errorMsg; + store.getState().updateTask(taskId, { error }); + if (currentTask?.error !== error) showEdgeReviewNotice(error); + return; + } store.getState().updateTask(taskId, { status: 'failed', error: errorMsg, diff --git a/app/desktop/src/stores/taskBridgeStore.ts b/app/desktop/src/stores/taskBridgeStore.ts index 6f7286976..1ea472d4b 100644 --- a/app/desktop/src/stores/taskBridgeStore.ts +++ b/app/desktop/src/stores/taskBridgeStore.ts @@ -4,6 +4,8 @@ import { create } from 'zustand'; import { subscribeWithSelector } from 'zustand/middleware'; +export type TaskCallbackOwner = 'edge' | 'desktop'; + export interface AgentTask { taskId: string; agentId: string; @@ -14,6 +16,8 @@ export interface AgentTask { dispatchPayload: Record; /** Explicit undefined clears a previous error through updateTask. */ error?: string | undefined; + /** Persistent output callback owner returned by Edge; unset until explicitly confirmed. */ + callbackOwner?: TaskCallbackOwner; /** Timestamp when the dispatch was received. */ createdAt: string; } diff --git a/edge-server/internal/hub/callback.go b/edge-server/internal/hub/callback.go index 44d3091fe..4f51716ea 100644 --- a/edge-server/internal/hub/callback.go +++ b/edge-server/internal/hub/callback.go @@ -136,6 +136,16 @@ type TaskResult struct { FinalContent string `json:"final_content"` } +// taskStreamEventBody is the typed stream request body sent by +// CallbackClient.TaskStreamEvent. Payload is embedded as a JSON object, never +// stringified, so the Hub handler can persist and rebroadcast it structurally. +type taskStreamEventBody struct { + RunID string `json:"run_id"` + EventType string `json:"event_type"` + Payload json.RawMessage `json:"payload"` + ClientMsgID string `json:"client_msg_id"` +} + func summarizeHubResponse(status int, body []byte, category string) string { hash := sha256.Sum256(body) return fmt.Sprintf( @@ -281,7 +291,7 @@ func (c *CallbackClient) EnableSQLiteJournal(path string) error { // TaskAck sends an acknowledgement that the Edge server has received the task // and started a run for it. Maps taskID → runID on the Hub side. func (c *CallbackClient) TaskAck(ctx context.Context, taskID string, runID string) error { - return c.callback(ctx, taskID, "ack", map[string]string{ + return c.callback(ctx, taskID, "ack", runID, map[string]string{ "run_id": runID, }) } @@ -293,16 +303,38 @@ func (c *CallbackClient) TaskAck(ctx context.Context, taskID string, runID strin // (older callers that cannot derive one still work, they just lose replay // protection). The body carries client_msg_id alongside run_id and content. func (c *CallbackClient) TaskStream(ctx context.Context, taskID string, runID string, clientMsgID string, content string) error { - return c.callback(ctx, taskID, "stream", map[string]string{ + return c.callback(ctx, taskID, "stream", runID, map[string]string{ "run_id": runID, "client_msg_id": clientMsgID, "content": content, }) } +// TaskStreamEvent sends one typed Edge runtime event through the shared task +// stream endpoint. Payload must be a JSON object; the caller supplies an +// already sanitized RawMessage and a deterministic client_msg_id. Unlike the +// legacy TaskStream path, typed events are retried through the shared callback +// retry loop because their client_msg_id is stable per event and the Hub dedups +// by that key. +func (c *CallbackClient) TaskStreamEvent(ctx context.Context, taskID string, runID string, clientMsgID string, eventType string, payload json.RawMessage) error { + if strings.TrimSpace(clientMsgID) == "" { + return fmt.Errorf("hub callback typed stream requires an idempotency key") + } + trimmed := bytes.TrimSpace(payload) + if len(trimmed) == 0 || trimmed[0] != '{' { + return fmt.Errorf("hub callback typed stream payload must be a JSON object") + } + return c.callbackWithRetry(ctx, taskID, "stream", runID, taskStreamEventBody{ + RunID: runID, + EventType: eventType, + Payload: trimmed, + ClientMsgID: clientMsgID, + }, true) +} + // TaskDone reports that the task has completed successfully. func (c *CallbackClient) TaskDone(ctx context.Context, taskID string, result TaskResult) error { - return c.callback(ctx, taskID, "done", map[string]string{ + return c.callback(ctx, taskID, "done", result.RunID, map[string]string{ "run_id": result.RunID, "final_content": result.FinalContent, }) @@ -310,7 +342,7 @@ func (c *CallbackClient) TaskDone(ctx context.Context, taskID string, result Tas // TaskFail reports that the task has failed with a reason. func (c *CallbackClient) TaskFail(ctx context.Context, taskID string, runID string, reason string) error { - return c.callback(ctx, taskID, "fail", map[string]string{ + return c.callback(ctx, taskID, "fail", runID, map[string]string{ "run_id": runID, "error": reason, }) @@ -333,17 +365,22 @@ func (c *CallbackClient) retryBudget(ctx context.Context) time.Duration { return budget } -// callback sends a POST request to the Hub callback endpoint. +// callback sends a POST request to the Hub callback endpoint with the action's +// default retry policy. +func (c *CallbackClient) callback(ctx context.Context, taskID string, action string, runID string, body any) error { + return c.callbackWithRetry(ctx, taskID, action, runID, body, callbackActionRetryable(action)) +} + +// callbackWithRetry sends a POST request to the Hub callback endpoint. // It retries on transient failures under a total wall-clock budget: 5xx and -// network errors are retried for idempotent actions, Retry-After (429/503) is -// honored and stops the sequence when it overruns the budget, and 4xx (except -// 429 with Retry-After) / 3xx / oversize responses are terminal. -func (c *CallbackClient) callback(ctx context.Context, taskID string, action string, body map[string]string) error { +// network errors are retried when the caller marks the action idempotent, +// Retry-After (429/503) is honored and stops the sequence when it overruns the +// budget, and 4xx (except 429 with Retry-After) / 3xx / oversize responses are +// terminal. The body may be any JSON-marshalable value; existing string-only +// callbacks and the typed stream request share this single serializer and +// retry loop. +func (c *CallbackClient) callbackWithRetry(ctx context.Context, taskID string, action string, runID string, body any, retryableAction bool) error { url := fmt.Sprintf("%s/edge/agent-tasks/%s/%s", c.hubURL, taskID, action) - runID := "" - if body != nil { - runID = body["run_id"] - } payload, err := json.Marshal(body) if err != nil { @@ -353,7 +390,6 @@ func (c *CallbackClient) callback(ctx context.Context, taskID string, action str // The payload (taskID in the URL, runID in the body) is the callback's // idempotency key: every retry re-sends byte-identical content. - retryableAction := callbackActionRetryable(action) budget := c.retryBudget(ctx) startedAt := time.Now() var lastErr error diff --git a/edge-server/internal/hub/callback_event_test.go b/edge-server/internal/hub/callback_event_test.go new file mode 100644 index 000000000..347b775c6 --- /dev/null +++ b/edge-server/internal/hub/callback_event_test.go @@ -0,0 +1,149 @@ +package hub_test + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "sync" + "testing" + "time" + + "github.com/agenthub/edge-server/internal/errcode" + "github.com/agenthub/edge-server/internal/hub" +) + +func TestCallbackClient_TaskStreamEventSendsTypedPayloadObject(t *testing.T) { + var mu sync.Mutex + var ( + method string + path string + raw []byte + ) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + mu.Lock() + method = r.Method + path = r.URL.Path + raw = body + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"code":"` + errcode.OK.Code + `"}`)) + })) + defer srv.Close() + + client := newTestCallbackClient(srv.URL, "test-token") + err := client.TaskStreamEvent( + context.Background(), + "task-001", + "run-001", + "client-msg-001", + "run.agent.permission_requested", + json.RawMessage(`{"requestId":"req-001","toolName":"Bash"}`), + ) + if err != nil { + t.Fatalf("TaskStreamEvent returned error: %v", err) + } + + mu.Lock() + defer mu.Unlock() + if method != http.MethodPost { + t.Fatalf("method = %q, want POST", method) + } + if path != "/edge/agent-tasks/task-001/stream" { + t.Fatalf("path = %q, want /edge/agent-tasks/task-001/stream", path) + } + + var body struct { + RunID string `json:"run_id"` + EventType string `json:"event_type"` + Payload json.RawMessage `json:"payload"` + ClientMsgID string `json:"client_msg_id"` + } + if err := json.Unmarshal(raw, &body); err != nil { + t.Fatalf("request body is not JSON: %v (raw=%s)", err, raw) + } + if body.RunID != "run-001" { + t.Fatalf("run_id = %q, want run-001", body.RunID) + } + if body.EventType != "run.agent.permission_requested" { + t.Fatalf("event_type = %q, want permission_requested", body.EventType) + } + if body.ClientMsgID != "client-msg-001" { + t.Fatalf("client_msg_id = %q, want client-msg-001", body.ClientMsgID) + } + if len(body.Payload) == 0 || body.Payload[0] != '{' { + t.Fatalf("payload = %s, want a JSON object, not a stringified payload", body.Payload) + } + var payload map[string]any + if err := json.Unmarshal(body.Payload, &payload); err != nil { + t.Fatalf("payload is not an object: %v", err) + } + if payload["requestId"] != "req-001" || payload["toolName"] != "Bash" { + t.Fatalf("payload = %#v, want requestId/toolName", payload) + } +} + +func TestCallbackClient_TaskStreamEventRetriesOnServerError(t *testing.T) { + var mu sync.Mutex + attempts := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + attempts++ + current := attempts + mu.Unlock() + if current == 1 { + w.WriteHeader(http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"code":"` + errcode.OK.Code + `"}`)) + })) + defer srv.Close() + + cfg := hub.DefaultCallbackConfig() + cfg.MaxAttempts = 2 + cfg.RetryBaseDelay = time.Millisecond + cfg.RetryBudget = time.Second + client := newPolicyCallbackClient(srv.URL, "test-token", cfg) + err := client.TaskStreamEvent( + context.Background(), + "task-001", + "run-001", + "client-msg-001", + "run.agent.route_decision", + json.RawMessage(`{"action":"finish"}`), + ) + if err != nil { + t.Fatalf("TaskStreamEvent returned error after retry: %v", err) + } + mu.Lock() + defer mu.Unlock() + if attempts != 2 { + t.Fatalf("attempts = %d, want 2 (typed stream is idempotent by client_msg_id)", attempts) + } +} + +func TestCallbackClient_TaskStreamEventWithoutKeyDoesNotSend(t *testing.T) { + var mu sync.Mutex + attempts := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + attempts++ + mu.Unlock() + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + client := newTestCallbackClient(srv.URL, "test-token") + if err := client.TaskStreamEvent(context.Background(), "task", "run", "", "run.agent.permission_requested", json.RawMessage(`{"requestId":"approval"}`)); err == nil { + t.Fatal("typed callback without a replay key must not enter the retry loop") + } + mu.Lock() + defer mu.Unlock() + if attempts != 0 { + t.Fatalf("unkeyed callback was sent %d times", attempts) + } +} diff --git a/edge-server/internal/lifecycle/callback_event_test.go b/edge-server/internal/lifecycle/callback_event_test.go new file mode 100644 index 000000000..7b2989239 --- /dev/null +++ b/edge-server/internal/lifecycle/callback_event_test.go @@ -0,0 +1,383 @@ +package lifecycle + +import ( + "context" + "encoding/json" + "fmt" + "sync" + "testing" + "time" + + "github.com/agenthub/edge-server/internal/adapters" + "github.com/agenthub/edge-server/internal/events" + "github.com/agenthub/edge-server/internal/hub" + "github.com/agenthub/edge-server/internal/store" + "github.com/agenthub/pkg/testkit" +) + +type typedCallbackEvent struct { + taskID string + runID string + clientMsgID string + eventType string + payload json.RawMessage +} + +type typedHubCallback struct { + mu sync.Mutex + order []string + events []typedCallbackEvent + streams []string + acks []string + dones []hub.TaskResult + fails []string + doneSeen chan struct{} +} + +func newTypedHubCallback() *typedHubCallback { + return &typedHubCallback{doneSeen: make(chan struct{})} +} + +func (c *typedHubCallback) TaskAck(_ context.Context, taskID, runID string) error { + c.mu.Lock() + defer c.mu.Unlock() + c.acks = append(c.acks, taskID+":"+runID) + return nil +} + +func (c *typedHubCallback) TaskStream(_ context.Context, _, _, _, content string) error { + c.mu.Lock() + defer c.mu.Unlock() + c.order = append(c.order, "stream") + c.streams = append(c.streams, content) + return nil +} + +func (c *typedHubCallback) TaskStreamEvent(_ context.Context, taskID, runID, clientMsgID, eventType string, payload json.RawMessage) error { + c.mu.Lock() + defer c.mu.Unlock() + c.order = append(c.order, "typed:"+eventType) + c.events = append(c.events, typedCallbackEvent{ + taskID: taskID, + runID: runID, + clientMsgID: clientMsgID, + eventType: eventType, + payload: payload, + }) + return nil +} + +func (c *typedHubCallback) TaskDone(_ context.Context, _ string, result hub.TaskResult) error { + c.mu.Lock() + defer c.mu.Unlock() + c.order = append(c.order, "done") + c.dones = append(c.dones, result) + select { + case <-c.doneSeen: + default: + close(c.doneSeen) + } + return nil +} + +func (c *typedHubCallback) TaskFail(_ context.Context, taskID, runID, reason string) error { + c.mu.Lock() + defer c.mu.Unlock() + c.order = append(c.order, "fail") + c.fails = append(c.fails, taskID+":"+runID+":"+reason) + return nil +} + +func bindTypedHubRun(executor *ProcessExecutor, runID string) { + executor.mu.Lock() + defer executor.mu.Unlock() + executor.hubTasks[runID] = "task-" + runID + executor.hubOutputs[runID] = newHubOutputCollector(hubCallbackFinalMaxBytes) +} + +func newTypedTestEmitter(t *testing.T) (*ProcessExecutor, *typedHubCallback, *hubCallbackEmitter, string) { + t.Helper() + bus := events.NewBus(100) + s := store.New() + executor := newTestProcessExecutor(t, bus, s, "success") + cb := newTypedHubCallback() + executor.WithHubCallback(cb) + runID := uniqueHubTestRunID("typed-test") + bindTypedHubRun(executor, runID) + emitter := newHubCallbackEmitter(executor, runID, adapters.NewBusEventEmitter(bus)) + typed, ok := emitter.(*hubCallbackEmitter) + if !ok { + t.Fatal("newHubCallbackEmitter did not produce a *hubCallbackEmitter") + } + return executor, cb, typed, runID +} + +func waitForTypedCallbackDone(t *testing.T, cb *typedHubCallback, wantEvents, wantDones int) { + t.Helper() + testkit.Eventually(t, 3*time.Second, func() bool { + cb.mu.Lock() + defer cb.mu.Unlock() + return len(cb.events) >= wantEvents && len(cb.dones) >= wantDones + }, fmt.Sprintf("typed events=%d dones=%d want events=%d dones=%d", wantEvents, wantDones, wantEvents, wantDones), func() string { + cb.mu.Lock() + defer cb.mu.Unlock() + return fmt.Sprintf("events=%d dones=%d", len(cb.events), len(cb.dones)) + }) +} + +func TestHubCallbackEmitterForwardsTypedEventsInOrderAndBeforeDone(t *testing.T) { + executor, cb, emitter, runID := newTypedTestEmitter(t) + cases := []struct { + typ string + payload map[string]any + }{ + {adapters.BusEventThinking, map[string]any{"content": "reasoning"}}, + {adapters.BusEventToolCall, map[string]any{"callId": "call-1", "toolName": "Bash"}}, + {adapters.BusEventToolResult, map[string]any{"callId": "call-1", "result": "ok"}}, + {adapters.BusEventFileChange, map[string]any{"path": "src/a.go", "action": "modified"}}, + {adapters.BusEventPermissionRequested, map[string]any{ + "requestId": "req-1", "toolName": "Bash", "input": map[string]any{"apiKey": "placeholder"}, + }}, + {adapters.BusEventPermissionDecided, map[string]any{"requestId": "req-1", "decision": "allow"}}, + {adapters.BusEventRouteDecision, map[string]any{"action": "continue"}}, + {adapters.BusEventResult, map[string]any{"success": true, "content": "final answer"}}, + } + + for _, tc := range cases { + emitter.Emit(tc.typ, map[string]any{}, tc.payload) + } + executor.fireHubDone(runID, nil) + waitForTypedCallbackDone(t, cb, len(cases), 1) + + cb.mu.Lock() + defer cb.mu.Unlock() + if len(cb.events) != len(cases) { + t.Fatalf("typed events = %d, want %d: %#v", len(cb.events), len(cases), cb.events) + } + wantOrder := make([]string, 0, len(cases)+1) + for i, tc := range cases { + if cb.events[i].eventType != tc.typ { + t.Fatalf("event[%d] type = %q, want %q", i, cb.events[i].eventType, tc.typ) + } + if cb.events[i].runID != runID || cb.events[i].taskID != "task-"+runID { + t.Fatalf("event[%d] run/task = %q/%q", i, cb.events[i].runID, cb.events[i].taskID) + } + wantID := hubStreamClientMsgID(runID, int64(i+1)) + if cb.events[i].clientMsgID != wantID { + t.Fatalf("event[%d] clientMsgID = %q, want %q", i, cb.events[i].clientMsgID, wantID) + } + + wantOrder = append(wantOrder, "typed:"+tc.typ) + } + wantOrder = append(wantOrder, "done") + if len(cb.order) != len(wantOrder) { + t.Fatalf("order = %v, want %v", cb.order, wantOrder) + } + for i := range wantOrder { + if cb.order[i] != wantOrder[i] { + t.Fatalf("order[%d] = %q, want %q (full order=%v)", i, cb.order[i], wantOrder[i], cb.order) + } + } +} + +func TestHubCallbackEmitterKeepsTextWithoutTypedDoubleForward(t *testing.T) { + executor, cb, emitter, runID := newTypedTestEmitter(t) + emitter.Emit(adapters.BusEventTextDelta, map[string]any{}, map[string]any{"content": "hello"}) + emitter.Emit(adapters.BusEventThinking, map[string]any{}, map[string]any{"content": "reasoning"}) + executor.fireHubDone(runID, nil) + + waitForTypedCallbackDone(t, cb, 1, 1) + cb.mu.Lock() + defer cb.mu.Unlock() + if len(cb.streams) != 1 || cb.streams[0] != "hello" { + t.Fatalf("streams = %v, want [hello]", cb.streams) + } + if len(cb.order) != 3 || cb.order[0] != "stream" || cb.order[1] != "typed:"+adapters.BusEventThinking || cb.order[2] != "done" { + t.Fatalf("pending text must precede the typed boundary: %v", cb.order) + } + if len(cb.events) != 1 || cb.events[0].eventType != adapters.BusEventThinking { + t.Fatalf("typed events = %#v, want only thinking (text must not be double-forwarded)", cb.events) + } +} + +func TestHubCallbackEmitterLegacyReporterStillStreamsText(t *testing.T) { + emitter, cb, runID := newCoalesceTestEmitter(t, "typed-legacy-reporter") + emitter.Emit(adapters.BusEventThinking, map[string]any{}, map[string]any{"content": "reasoning"}) + emitter.Emit(adapters.BusEventTextDelta, map[string]any{}, map[string]any{"content": "hello"}) + emitter.FlushHubStream() + emitter.executor.fireHubDone(runID, nil) + + waitForStreams(t, cb, 1) + select { + case <-cb.doneSeen: + case <-time.After(3 * time.Second): + t.Fatal("legacy reporter did not receive TaskDone") + } + cb.mu.Lock() + defer cb.mu.Unlock() + if len(cb.streams) != 1 || cb.streams[0] != "hello" { + t.Fatalf("streams = %v, want [hello]", cb.streams) + } +} + +func TestHubCallbackEmitterTypedSkipsWithoutHubBinding(t *testing.T) { + bus := events.NewBus(10) + s := store.New() + executor := newTestProcessExecutor(t, bus, s, "success") + cb := newTypedHubCallback() + executor.WithHubCallback(cb) + runID := uniqueHubTestRunID("typed-no-binding") + emitter := newHubCallbackEmitter(executor, runID, adapters.NewBusEventEmitter(bus)) + emitter.Emit(adapters.BusEventPermissionRequested, map[string]any{}, map[string]any{"requestId": "req-1"}) + emitter.Emit(adapters.BusEventTextDelta, map[string]any{}, map[string]any{"content": "hello"}) + flusher, ok := emitter.(hubStreamFlusher) + if !ok { + t.Fatal("hubCallbackEmitter does not implement hubStreamFlusher") + } + flusher.FlushHubStream() + executor.fireHubDone(runID, nil) + time.Sleep(100 * time.Millisecond) + + cb.mu.Lock() + defer cb.mu.Unlock() + if len(cb.events) != 0 { + t.Fatalf("typed callbacks = %d, want 0 for a run without a Hub task binding", len(cb.events)) + } + if len(cb.streams) != 0 { + t.Fatalf("stream callbacks = %d, want 0 for a run without a Hub task binding", len(cb.streams)) + } +} + +type typedBlockingHubCallback struct { + *blockingHubCallback + typedMu sync.Mutex + order []string + typedEvents []typedCallbackEvent +} + +func newTypedBlockingHubCallback(capacity int) *typedBlockingHubCallback { + return &typedBlockingHubCallback{blockingHubCallback: newBlockingHubCallback(capacity)} +} + +func (c *typedBlockingHubCallback) TaskStreamEvent(_ context.Context, _ string, runID, clientMsgID, eventType string, payload json.RawMessage) error { + c.track() + defer c.untrack() + select { + case c.entered <- struct{}{}: + default: + } + <-c.hold + c.typedMu.Lock() + c.order = append(c.order, "typed:"+eventType) + c.typedEvents = append(c.typedEvents, typedCallbackEvent{ + runID: runID, + clientMsgID: clientMsgID, + eventType: eventType, + payload: payload, + }) + c.typedMu.Unlock() + return nil +} + +func (c *typedBlockingHubCallback) TaskDone(ctx context.Context, taskID string, result hub.TaskResult) error { + if err := c.blockingHubCallback.TaskDone(ctx, taskID, result); err != nil { + return err + } + c.typedMu.Lock() + c.order = append(c.order, "done") + c.typedMu.Unlock() + return nil +} + +func TestHubCallbackQueueTypedApprovalBackpressuresThenDeliversBeforeDone(t *testing.T) { + bus := events.NewBus(10) + s := store.New() + executor := newTestProcessExecutor(t, bus, s, "success") + executor.callbackSem = make(chan struct{}, 1) + cb := newTypedBlockingHubCallback(1) + executor.WithHubCallback(cb) + runID := uniqueHubTestRunID("typed-backpressure") + bindTypedHubRun(executor, runID) + + state := loadOrInitHubCallbackQueue(runID) + for i := 0; i < hubCallbackQueueCapacity; i++ { + state.ch <- hubCallbackJob{ + kind: hubJobStreamEvent, + taskID: "task-" + runID, + runID: runID, + eventType: adapters.BusEventToolCall, + payload: json.RawMessage(`{"callId":"pre-approval"}`), + } + } + executor.startHubCallbackQueue(state) + select { + case <-cb.entered: + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for first blocked typed callback") + } + // One typed delivery is in flight; fill the 128-slot channel completely so + // the approval below must wait for a slot before it can be enqueued. + state.ch <- hubCallbackJob{ + kind: hubJobStreamEvent, + taskID: "task-" + runID, + runID: runID, + eventType: adapters.BusEventToolResult, + payload: json.RawMessage(`{"callId":"pre-approval-2"}`), + } + + entryDone := make(chan bool, 1) + released := false + defer func() { + if !released { + cb.releaseAll() + } + }() + go func() { + entryDone <- executor.enqueueHubTypedEventJob(runID, hubCallbackJob{ + kind: hubJobStreamEvent, + taskID: "task-" + runID, + runID: runID, + eventType: adapters.BusEventPermissionRequested, + payload: json.RawMessage(`{"requestId":"approval-1","toolName":"Bash"}`), + }) + }() + + select { + case ok := <-entryDone: + t.Fatalf("approval typed event enqueue returned early: ok=%v", ok) + case <-time.After(100 * time.Millisecond): + } + cb.releaseAll() + released = true + select { + case ok := <-entryDone: + if !ok { + t.Fatal("approval typed event enqueue failed after queue made progress") + } + case <-time.After(3 * time.Second): + t.Fatal("timed out waiting for approval typed event to enter the queue") + } + + executor.fireHubDone(runID, nil) + testkit.Eventually(t, 5*time.Second, func() bool { + cb.typedMu.Lock() + defer cb.typedMu.Unlock() + if len(cb.order) < hubCallbackQueueCapacity+2 || cb.order[len(cb.order)-1] != "done" { + return false + } + approvalBeforeDone := false + for _, entry := range cb.order { + if entry == "typed:"+adapters.BusEventPermissionRequested { + approvalBeforeDone = true + } + if entry == "done" && !approvalBeforeDone { + return false + } + } + return approvalBeforeDone + }, "approval typed event delivers before TaskDone", func() string { + cb.typedMu.Lock() + defer cb.typedMu.Unlock() + return fmt.Sprintf("order=%v", cb.order) + }) +} diff --git a/edge-server/internal/lifecycle/process_executor_hub_callback.go b/edge-server/internal/lifecycle/process_executor_hub_callback.go index a860035a8..b62ab59cb 100644 --- a/edge-server/internal/lifecycle/process_executor_hub_callback.go +++ b/edge-server/internal/lifecycle/process_executor_hub_callback.go @@ -2,6 +2,7 @@ package lifecycle import ( "context" + "encoding/json" "log/slog" "strconv" "strings" @@ -24,6 +25,16 @@ type CallbackReporter interface { TaskFail(ctx context.Context, taskID string, runID string, reason string) error } +// TaskEventReporter is an optional capability implemented by reporters that +// can carry typed Edge runtime events. *hub.CallbackClient implements it; +// legacy/double-recording stubs intentionally do not, so they keep receiving +// text callbacks without being forced to know the typed shape. +type TaskEventReporter interface { + TaskStreamEvent(ctx context.Context, taskID string, runID string, clientMsgID string, eventType string, payload json.RawMessage) error +} + +var _ TaskEventReporter = (*hub.CallbackClient)(nil) + // hubStreamChunkSeq provides a per-run monotonic chunk index for deterministic // client_msg_id (UUIDv5) generation in fireHubStream. Lazily populated via // sync.Map so concurrent stream events for the same run get strictly @@ -240,6 +251,7 @@ type hubCallbackJobKind int const ( hubJobStream hubCallbackJobKind = iota + hubJobStreamEvent hubJobDone hubJobFail ) @@ -249,8 +261,10 @@ type hubCallbackJob struct { kind hubCallbackJobKind taskID string runID string - clientMsgID string // stream jobs only + clientMsgID string // stream/typed-event jobs only content string // stream chunk content or fail reason + eventType string // typed stream event only + payload json.RawMessage result hub.TaskResult } @@ -375,6 +389,14 @@ func (e *ProcessExecutor) deliverHubCallbackJob(job hubCallbackJob) { if err := e.hubCallback.TaskStream(ctx, job.taskID, job.runID, job.clientMsgID, job.content); shouldLogHubCallbackFailure(err) { slog.Warn("hub callback stream failed", "taskId", job.taskID, "runId", job.runID, "error", err) } + case hubJobStreamEvent: + reporter, ok := e.hubCallback.(TaskEventReporter) + if !ok { + return + } + if err := reporter.TaskStreamEvent(ctx, job.taskID, job.runID, job.clientMsgID, job.eventType, job.payload); shouldLogHubCallbackFailure(err) { + slog.Warn("hub callback typed event failed", "taskId", job.taskID, "runId", job.runID, "eventType", job.eventType, "error", err) + } case hubJobDone: if err := e.hubCallback.TaskDone(ctx, job.taskID, job.result); shouldLogHubCallbackFailure(err) { slog.Warn("hub callback done failed", "taskId", job.taskID, "runId", job.runID, "error", err) @@ -420,6 +442,12 @@ func newHubCallbackEmitter(executor *ProcessExecutor, runID string, inner adapte func (e *hubCallbackEmitter) Emit(eventType string, scope map[string]any, payload any) { e.inner.Emit(eventType, scope, payload) + if _, typed := e.executor.hubCallback.(TaskEventReporter); typed && shouldForwardHubTypedEvent(eventType) { + // A tool/approval/result is a transcript boundary: prior deltas must + // enter the same FIFO before this typed event. + e.flushPendingHubStream() + } + e.executor.fireHubTaskStreamEvent(e.runID, eventType, payload) text, effect := hubCallbackTextForEvent(eventType, payload) if !shouldApplyHubCallbackSideEffect(text, effect) { return diff --git a/edge-server/internal/lifecycle/process_executor_hub_callback_helpers.go b/edge-server/internal/lifecycle/process_executor_hub_callback_helpers.go new file mode 100644 index 000000000..f50f909c6 --- /dev/null +++ b/edge-server/internal/lifecycle/process_executor_hub_callback_helpers.go @@ -0,0 +1,98 @@ +package lifecycle + +import ( + "bytes" + "encoding/json" + "log/slog" + + "github.com/agenthub/edge-server/internal/adapters" +) + +// shouldForwardHubTypedEvent reports whether a local runtime event should be +// carried through the Hub typed stream contract. Text delta/block events keep +// their existing coalesced TaskStream path and are deliberately excluded to +// avoid double-forwarding text. +func shouldForwardHubTypedEvent(eventType string) bool { + switch eventType { + case adapters.BusEventThinking, + adapters.BusEventToolCall, + adapters.BusEventToolResult, + adapters.BusEventFileChange, + adapters.BusEventPermissionRequested, + adapters.BusEventPermissionDecided, + adapters.BusEventRouteDecision, + adapters.BusEventResult: + return true + default: + return false + } +} + +// hubCallbackTypedEventPayload sanitizes the local runtime payload with the +// existing recursive sanitizer, then validates that it serializes as a JSON +// object (the Hub typed stream contract rejects stringified payloads). +func hubCallbackTypedEventPayload(payload any) (json.RawMessage, bool) { + sanitized, _ := SanitizeSubAgentResult(payload) + raw, err := json.Marshal(sanitized) + if err != nil { + return nil, false + } + trimmed := bytes.TrimSpace(raw) + if len(trimmed) == 0 || trimmed[0] != '{' { + return nil, false + } + return append(json.RawMessage(nil), trimmed...), true +} + +// fireHubTaskStreamEvent enqueues one typed Edge runtime event on the same +// per-run FIFO and UUID sequence used by text stream callbacks. It is a +// no-op when the task has no Hub binding (including desktop-owned runs) or +// the configured reporter does not implement TaskEventReporter. +func (e *ProcessExecutor) fireHubTaskStreamEvent(runID, eventType string, payload any) { + if e == nil || !shouldForwardHubTypedEvent(eventType) { + return + } + taskID := e.hubTaskID(runID) + if !shouldFireHubCallback(e.hubCallback != nil, taskID) { + return + } + if _, ok := e.hubCallback.(TaskEventReporter); !ok { + return + } + raw, ok := hubCallbackTypedEventPayload(payload) + if !ok { + slog.Debug("hub callback typed event skipped: payload is not a JSON object", "taskId", taskID, "runId", runID, "eventType", eventType) + return + } + chunkIdx := nextHubStreamChunkIdx(runID) + clientMsgID := hubStreamClientMsgID(runID, chunkIdx) + if !e.enqueueHubTypedEventJob(runID, hubCallbackJob{ + kind: hubJobStreamEvent, + taskID: taskID, + runID: runID, + clientMsgID: clientMsgID, + eventType: eventType, + payload: raw, + }) { + slog.Debug("hub callback typed event skipped after queue close", "taskId", taskID, "runId", runID, "eventType", eventType) + } +} + +// enqueueHubTypedEventJob appends a typed runtime event to the run's FIFO with +// the same bounded backpressure as terminal jobs: it waits for a queue slot +// instead of dropping approvals, and it never closes the queue. The consumer +// is already started by the stream/typed enqueuer that filled the queue, and +// every delivery is bounded by hubCallbackTimeout, so the wait always makes +// progress. No per-event goroutine is created. +func (e *ProcessExecutor) enqueueHubTypedEventJob(runID string, job hubCallbackJob) bool { + state := loadOrInitHubCallbackQueue(runID) + + state.mu.Lock() + defer state.mu.Unlock() + if state.closed { + return false + } + state.ch <- job + e.startHubCallbackQueue(state) + return true +} diff --git a/hub-server/tests/integration/outbox_claim_cas_test.go b/hub-server/tests/integration/outbox_claim_cas_test.go index fe8580e5f..631d7a573 100644 --- a/hub-server/tests/integration/outbox_claim_cas_test.go +++ b/hub-server/tests/integration/outbox_claim_cas_test.go @@ -42,9 +42,15 @@ func openTempMigratedDB(t *testing.T) (*gorm.DB, func()) { t.Fatal("AGENTHUB_DB_PASSWORD not set; required for the PostgreSQL integration path") } - host := "localhost" - port := 5432 - user := "agenthub" + // Use the same config/environment as TestMain; an isolated non-default + // PostgreSQL must not silently fall back to another local database. + cfg, err := config.Load("../../configs/config.yaml") + if err != nil { + t.Fatalf("load PostgreSQL integration config: %v", err) + } + host := cfg.DB.Host + port := cfg.DB.Port + user := cfg.DB.User // Connect to the default "postgres" database to create/drop our temp DB. adminDSN := fmt.Sprintf("host=%s port=%d user=%s password=%s dbname=postgres sslmode=disable", From aab5f0623664942bb54189d021d56d7149ca3033 Mon Sep 17 00:00:00 2001 From: Delicious233 <101502465+DeliciousBuding@users.noreply.github.com> Date: Mon, 7 Sep 2026 09:08:22 +0800 Subject: [PATCH 5/5] test(dispatch): align full callback gates and simplify direct route stages --- .../internal/lifecycle/callback_event_test.go | 4 +- edge-server/tests/hub_e2e_test.go | 220 ++++++++++++++---- .../service/dispatch/edge_execution_intent.go | 2 +- .../agent_dispatch_callback_route.go | 59 +++++ .../dispatchsvc/agent_dispatch_edge_http.go | 60 +---- scripts/verify/test-sleep-baseline.json | 2 +- scripts/verify/test-sleep-budget.json | 13 +- 7 files changed, 249 insertions(+), 111 deletions(-) diff --git a/edge-server/internal/lifecycle/callback_event_test.go b/edge-server/internal/lifecycle/callback_event_test.go index 7b2989239..b785edab7 100644 --- a/edge-server/internal/lifecycle/callback_event_test.go +++ b/edge-server/internal/lifecycle/callback_event_test.go @@ -236,7 +236,9 @@ func TestHubCallbackEmitterTypedSkipsWithoutHubBinding(t *testing.T) { } flusher.FlushHubStream() executor.fireHubDone(runID, nil) - time.Sleep(100 * time.Millisecond) + if _, exists := hubCallbackQueues.Load(runID); exists { + t.Fatal("unbound run created a callback queue") + } cb.mu.Lock() defer cb.mu.Unlock() diff --git a/edge-server/tests/hub_e2e_test.go b/edge-server/tests/hub_e2e_test.go index a55cbbf09..fa9ed481c 100644 --- a/edge-server/tests/hub_e2e_test.go +++ b/edge-server/tests/hub_e2e_test.go @@ -48,6 +48,16 @@ const ( // to reach a terminal status; the headroom covers the full ack→done // callback sequence (#2055). roundTripWaitTimeout = 15 * time.Second + + // fixtureCallbackToken is a mock-only placeholder credential. The + // callback-capability gate requires a non-empty token before it admits an + // edge-owned run; it never reaches a real Hub in this fixture. + fixtureCallbackToken = "fixture-edge-callback-token" + + // runOwnerEdge is the explicit direct-callback owner sent in the /v1/runs + // contract so the Edge records an edge-owned admission instead of guessing + // desktop ownership from an unconfigured callback client. + runOwnerEdge = "edge" ) // isTerminalRunStatus reports whether status is a terminal run lifecycle @@ -67,6 +77,19 @@ func runCallbackDump(runID string, h *api.Handler, mockHub *hubCallbackMock) str runID, status, mockHub.ackCount(), mockHub.doneCount(), mockHub.failCount(), mockHub.streamCount()) } +// requireEdgeAdmissionResponse verifies the HTTP /v1/runs response recorded an +// edge-owned, accepted admission. This is the contract check that prevents a +// fixture from starting a run and then patching the store to fake ownership. +func requireEdgeAdmissionResponse(t *testing.T, data map[string]any) { + t.Helper() + if got := data["callbackOwner"]; got != runOwnerEdge { + t.Fatalf("response callbackOwner = %v, want %q", got, runOwnerEdge) + } + if got := data["admissionState"]; got != store.RunAdmissionAccepted { + t.Fatalf("response admissionState = %v, want %q", got, store.RunAdmissionAccepted) + } +} + // ── Hub mock with full Edge callback endpoint support ────────────────────── // hubCallbackMock is a mock Hub server that records all callback requests @@ -201,9 +224,12 @@ func startEdgeWithHubCallbacks(t *testing.T, hubURL string) (*httptest.Server, * t.Fatalf("failed to create process executor: %v", err) } - // Wire Hub callback client + // Wire Hub callback client. The handler must see the same configured + // client as the executor; otherwise the direct-callback capability gate + // treats every hub-task request as a desktop-owned legacy run. + var hubClient *hub.CallbackClient if hubURL != "" { - hubClient := newE2ECallbackClient(hubURL, "") + hubClient = newE2ECallbackClient(hubURL, fixtureCallbackToken) processExecutor.SetHubCallback(hubClient) } @@ -213,6 +239,7 @@ func startEdgeWithHubCallbacks(t *testing.T, hubURL string) (*httptest.Server, * Registry: runners.NewRegistry(), Store: storeRepo, Executor: processExecutor, + CallbackClient: hubClient, WorkspaceAllowlist: []string{workDir}, } @@ -246,11 +273,12 @@ func TestHubE2E_RunCompletes_FiresDoneCallback(t *testing.T) { // Create a run with hubTaskId to trigger Edge→Hub callbacks runResp := postJSON(t, edgeTS.URL+"/v1/runs", map[string]any{ - "projectId": "proj_local", - "threadId": "thread_local", - "prompt": "E2E test: complete run", - "hubTaskId": taskID, - "workDir": edgeH.WorkspaceAllowlist[0], + "projectId": "proj_local", + "threadId": "thread_local", + "prompt": "E2E test: complete run", + "hubTaskId": taskID, + "callbackOwner": runOwnerEdge, + "workDir": edgeH.WorkspaceAllowlist[0], }) if runResp.StatusCode != http.StatusAccepted { @@ -260,10 +288,12 @@ func TestHubE2E_RunCompletes_FiresDoneCallback(t *testing.T) { } runBody := decodeJSON[map[string]any](t, runResp) - runID, ok := unwrapSuccess(runBody)["runId"].(string) + data := unwrapSuccess(runBody) + runID, ok := data["runId"].(string) if !ok { t.Fatalf("expected runId in response, got %v", runBody) } + requireEdgeAdmissionResponse(t, data) t.Logf("created run %s for task %s", runID, taskID) // Wait for the run to finish (echo exits almost immediately) @@ -277,8 +307,33 @@ func TestHubE2E_RunCompletes_FiresDoneCallback(t *testing.T) { t.Logf("run %s final status: %s", runID, run.Status) } - // Give the async callback goroutine a moment to fire - time.Sleep(500 * time.Millisecond) + // Wait for the complete callback set instead of sleeping a fixed amount; + // missing a callback must fail the fixture rather than being hidden by a + // short race window. + testkit.Eventually(t, runTerminalWaitTimeout, func() bool { + return mockHub.ackCount() >= 1 && mockHub.streamCount() >= 1 && mockHub.doneCount() >= 1 + }, "edge-owned callbacks should reach the mock Hub", func() string { + return runCallbackDump(runID, edgeH, mockHub) + }) + + // Verify Hub received the ack callback and no fail callback. + ackCount := mockHub.ackCount() + if ackCount != 1 { + t.Fatalf("expected exactly 1 ack callback, got %d (done=%d, fail=%d, stream=%d)", + ackCount, mockHub.doneCount(), mockHub.failCount(), mockHub.streamCount()) + } + mockHub.mu.Lock() + ackRecord := mockHub.ackCalls[0] + mockHub.mu.Unlock() + if ackRecord.TaskID != taskID { + t.Errorf("ack callback taskID = %q, want %q", ackRecord.TaskID, taskID) + } + if ackRecord.Body["run_id"] != runID { + t.Errorf("ack callback run_id = %q, want %q", ackRecord.Body["run_id"], runID) + } + if failCount := mockHub.failCount(); failCount != 0 { + t.Errorf("expected 0 fail callbacks for successful run, got %d", failCount) + } // Verify Hub received a stream callback with real process stdout. streamCount := mockHub.streamCount() @@ -340,7 +395,7 @@ func TestHubE2E_RunFails_FiresFailCallback(t *testing.T) { t.Fatalf("failed to create process executor: %v", err) } - hubClient := newE2ECallbackClient(mockHub.URL(), "") + hubClient := newE2ECallbackClient(mockHub.URL(), fixtureCallbackToken) processExecutor.SetHubCallback(hubClient) workDir := t.TempDir() @@ -349,6 +404,7 @@ func TestHubE2E_RunFails_FiresFailCallback(t *testing.T) { Registry: runners.NewRegistry(), Store: storeRepo, Executor: processExecutor, + CallbackClient: hubClient, WorkspaceAllowlist: []string{workDir}, } @@ -360,11 +416,12 @@ func TestHubE2E_RunFails_FiresFailCallback(t *testing.T) { taskID := "task-e2e-fail-001" runResp := postJSON(t, edgeTS.URL+"/v1/runs", map[string]any{ - "projectId": "proj_local", - "threadId": "thread_local", - "prompt": "E2E test: failing run", - "hubTaskId": taskID, - "workDir": workDir, + "projectId": "proj_local", + "threadId": "thread_local", + "prompt": "E2E test: failing run", + "hubTaskId": taskID, + "callbackOwner": runOwnerEdge, + "workDir": workDir, }) if runResp.StatusCode != http.StatusAccepted { @@ -374,7 +431,9 @@ func TestHubE2E_RunFails_FiresFailCallback(t *testing.T) { } runBody := decodeJSON[map[string]any](t, runResp) - runID, _ := unwrapSuccess(runBody)["runId"].(string) + data := unwrapSuccess(runBody) + runID, _ := data["runId"].(string) + requireEdgeAdmissionResponse(t, data) t.Logf("created failing run %s for task %s", runID, taskID) // Wait for run to terminate with failure @@ -388,17 +447,31 @@ func TestHubE2E_RunFails_FiresFailCallback(t *testing.T) { t.Logf("run %s status: %s", runID, run.Status) } - time.Sleep(500 * time.Millisecond) // allow async callback + testkit.Eventually(t, runTerminalWaitTimeout, func() bool { + return mockHub.failCount() == 1 + }, "edge-owned fail callback should reach the mock Hub", func() string { + return runCallbackDump(runID, h, mockHub) + }) - if failCount := mockHub.failCount(); failCount >= 1 { - mockHub.mu.Lock() - failRecord := mockHub.failCalls[0] - mockHub.mu.Unlock() - t.Logf("fail callback received for task %s: error=%s", failRecord.TaskID, failRecord.Body["error"]) - } else { - // It's OK if fail doesn't fire (the run might have "started" before finding the binary) - // The important thing is the wiring doesn't crash - t.Log("no fail callback (run may have failed before started status)") + failCount := mockHub.failCount() + if failCount != 1 { + t.Fatalf("expected exactly 1 fail callback, got %d (ack=%d, done=%d, stream=%d)", + failCount, mockHub.ackCount(), mockHub.doneCount(), mockHub.streamCount()) + } + if doneCount := mockHub.doneCount(); doneCount != 0 { + t.Errorf("expected 0 done callbacks for failed run, got %d", doneCount) + } + mockHub.mu.Lock() + failRecord := mockHub.failCalls[0] + mockHub.mu.Unlock() + if failRecord.TaskID != taskID { + t.Errorf("fail callback taskID = %q, want %q", failRecord.TaskID, taskID) + } + if failRecord.Body["run_id"] != runID { + t.Errorf("fail callback run_id = %q, want %q", failRecord.Body["run_id"], runID) + } + if !strings.Contains(failRecord.Body["error"], "nonexistent_command_xyz_123") { + t.Errorf("fail callback error = %q, want command name", failRecord.Body["error"]) } } @@ -522,6 +595,9 @@ func TestHubE2E_NoCallbackWhenNotConfigured(t *testing.T) { if fails := mockHub.failCount(); fails > 0 { t.Errorf("expected 0 fail callbacks without hubTaskId, got %d", fails) } + if streams := mockHub.streamCount(); streams > 0 { + t.Errorf("expected 0 stream callbacks without hubTaskId, got %d", streams) + } } // TestHubE2E_CompleteRoundTrip verifies the full protocol: @@ -541,11 +617,12 @@ func TestHubE2E_CompleteRoundTrip(t *testing.T) { taskID := fmt.Sprintf("task-roundtrip-%d", time.Now().UnixNano()) runResp := postJSON(t, edgeTS.URL+"/v1/runs", map[string]any{ - "projectId": "proj_local", - "threadId": "thread_local", - "prompt": "Complete round trip test", - "hubTaskId": taskID, - "workDir": edgeH.WorkspaceAllowlist[0], + "projectId": "proj_local", + "threadId": "thread_local", + "prompt": "Complete round trip test", + "hubTaskId": taskID, + "callbackOwner": runOwnerEdge, + "workDir": edgeH.WorkspaceAllowlist[0], }) if runResp.StatusCode != http.StatusAccepted { @@ -555,10 +632,12 @@ func TestHubE2E_CompleteRoundTrip(t *testing.T) { } runBody := decodeJSON[map[string]any](t, runResp) - runID, ok := unwrapSuccess(runBody)["runId"].(string) + data := unwrapSuccess(runBody) + runID, ok := data["runId"].(string) if !ok { t.Fatalf("expected runId, got %v", runBody) } + requireEdgeAdmissionResponse(t, data) t.Logf("roundtrip: run %s, task %s", runID, taskID) // Wait for completion @@ -568,35 +647,74 @@ func TestHubE2E_CompleteRoundTrip(t *testing.T) { }, "round-trip run should reach a terminal status", func() string { return runCallbackDump(runID, edgeH, mockHub) }) - time.Sleep(500 * time.Millisecond) // allow async callbacks to fire + testkit.Eventually(t, roundTripWaitTimeout, func() bool { + return mockHub.ackCount() >= 1 && mockHub.streamCount() >= 1 && mockHub.doneCount() >= 1 + }, "complete edge-owned callback set should reach the mock Hub", func() string { + return runCallbackDump(runID, edgeH, mockHub) + }) - // Verify Hub received both ack and done callbacks + // Verify Hub received the complete edge-owned callback set, not merely + // one of the possible terminal callbacks. ackCount := mockHub.ackCount() + streamCount := mockHub.streamCount() doneCount := mockHub.doneCount() + failCount := mockHub.failCount() t.Logf("final callback counts: ack=%d, done=%d, fail=%d, stream=%d", - ackCount, doneCount, mockHub.failCount(), mockHub.streamCount()) + ackCount, doneCount, failCount, streamCount) + + if ackCount != 1 { + t.Errorf("expected exactly 1 ack callback, got %d", ackCount) + } + if streamCount < 1 { + t.Errorf("expected at least 1 stream callback, got %d", streamCount) + } + if doneCount != 1 { + t.Errorf("expected exactly 1 done callback, got %d", doneCount) + } + if failCount != 0 { + t.Errorf("expected 0 fail callbacks for successful run, got %d", failCount) + } - if ackCount < 1 && doneCount < 1 { - t.Error("expected at least ack or done callback; got none") + // Verify every callback belongs to the same hub task and run. + mockHub.mu.Lock() + var streamContent strings.Builder + for _, streamRecord := range mockHub.streamCalls { + streamContent.WriteString(streamRecord.Body["content"]) + if streamRecord.TaskID != taskID { + t.Errorf("stream taskID = %q, want %q", streamRecord.TaskID, taskID) + } + if streamRecord.Body["run_id"] != runID { + t.Errorf("stream run_id = %q, want %q", streamRecord.Body["run_id"], runID) + } + } + var ackRecord hubCallbackRecord + if ackCount > 0 { + ackRecord = mockHub.ackCalls[0] } + var doneRecord hubCallbackRecord + if doneCount > 0 { + doneRecord = mockHub.doneCalls[0] + } + mockHub.mu.Unlock() - // Verify callback taskID matches - if ackCount >= 1 { - mockHub.mu.Lock() - if mockHub.ackCalls[0].TaskID != taskID { - t.Errorf("ack taskID = %q, want %q", mockHub.ackCalls[0].TaskID, taskID) + if ackCount > 0 { + if ackRecord.TaskID != taskID { + t.Errorf("ack taskID = %q, want %q", ackRecord.TaskID, taskID) + } + if ackRecord.Body["run_id"] != runID { + t.Errorf("ack run_id = %q, want %q", ackRecord.Body["run_id"], runID) } - mockHub.mu.Unlock() } - if doneCount >= 1 { - mockHub.mu.Lock() - if mockHub.doneCalls[0].TaskID != taskID { - t.Errorf("done taskID = %q, want %q", mockHub.doneCalls[0].TaskID, taskID) + if doneCount > 0 { + if doneRecord.TaskID != taskID { + t.Errorf("done taskID = %q, want %q", doneRecord.TaskID, taskID) } - if mockHub.doneCalls[0].Body["run_id"] != runID { - t.Errorf("done run_id = %q, want %q", mockHub.doneCalls[0].Body["run_id"], runID) + if doneRecord.Body["run_id"] != runID { + t.Errorf("done run_id = %q, want %q", doneRecord.Body["run_id"], runID) } - mockHub.mu.Unlock() + } + if !strings.Contains(streamContent.String(), noopCommandOutput) { + t.Errorf("stream callback content = %q, want %q", streamContent.String(), noopCommandOutput) } } diff --git a/hub-server/internal/service/dispatch/edge_execution_intent.go b/hub-server/internal/service/dispatch/edge_execution_intent.go index 00f417128..9cc86026b 100644 --- a/hub-server/internal/service/dispatch/edge_execution_intent.go +++ b/hub-server/internal/service/dispatch/edge_execution_intent.go @@ -76,7 +76,7 @@ func firstStringArrayValue(values ...any) []string { } func parseStringArrayValue(value any) []string { - var source any = value + source := value if s, ok := value.(string); ok { var parsed []any if err := json.Unmarshal([]byte(s), &parsed); err != nil { diff --git a/hub-server/internal/service/dispatchsvc/agent_dispatch_callback_route.go b/hub-server/internal/service/dispatchsvc/agent_dispatch_callback_route.go index 137014634..5b6df328b 100644 --- a/hub-server/internal/service/dispatchsvc/agent_dispatch_callback_route.go +++ b/hub-server/internal/service/dispatchsvc/agent_dispatch_callback_route.go @@ -4,9 +4,15 @@ import ( "context" "encoding/json" "io" + "log/slog" "net/http" "strings" + "github.com/agenthub/pkg/outboundmetrics" + + "github.com/agenthub/hub-server/internal/metrics" + "github.com/agenthub/hub-server/internal/model" + "github.com/agenthub/hub-server/internal/repository" "github.com/agenthub/hub-server/internal/service/dispatch" ) @@ -58,3 +64,56 @@ func edgeDispatchReceiptOwner(body []byte) string { } return "" } + +// prepareDirectCallbackRoute checks callback authority and durably reserves +// the destination before execution. Failure reports whether another route is +// still safe; database uncertainty never grants fallback authority. +func (s *DispatchService) prepareDirectCallbackRoute(ctx context.Context, task *model.PendingAgentTask, dp *dispatchPayload, parts dispatch.EdgeHTTPRequestParts) (ready, safeToFallback bool) { + deviceID := strings.TrimSpace(s.edgeCfg.DeviceID) + if task.EdgeDeviceID != "" && task.EdgeDeviceID != deviceID { + return false, false + } + // Per-Edge circuit breaker: when Edge is down, consecutive dispatches would + // each block for the full HTTP client timeout (~30s), exhausting the + // dispatch semaphore and stalling the TTL/redispatch path. The breaker + // fails fast (no HTTP call) while open and admits a single half-open probe + // after edgeBreakerOpenDuration to test recovery. Pre-HTTP failures + // (insecure/marshal/req_create/edgeClient-nil) are config issues and do + // not trip the breaker; only client.Do/non_success/decode_fail indicate + // Edge health and are recorded. + if s.db == nil { + return false, false + } + owned, err := repository.DirectCallbackDeviceMatchesTask(s.db.WithContext(ctx), task.ID, deviceID) + if err != nil { + slog.Error("edge direct callback device lookup failed", "task_id", task.ID, "error", err) + return false, false + } + if !owned { + return false, task.EdgeDeviceID == "" + } + if !s.edgeBreaker.Allow() { + slog.Warn(dispatch.EdgeHTTPLogUnreachable, "task_id", task.ID, "url", parts.RunsURL, "error", "edge circuit breaker open") + if metrics.AgentDispatchEdgeHTTPFailures != nil { + metrics.AgentDispatchEdgeHTTPFailures.WithLabelValues("breaker_open").Inc() + } + metrics.OutboundMetrics.Record(outboundmetrics.ProviderEdge, outboundmetrics.PurposeDispatch, outboundmetrics.CategoryFailure, "breaker_open") + return false, task.EdgeDeviceID == "" + } + if !s.directCallbackRouteReady(ctx, parts) { + s.edgeBreaker.RecordFailure() + slog.Info("edge http dispatch: callback ownership route is unavailable", "task_id", task.ID) + return false, task.EdgeDeviceID == "" + } + + // Reserve the actual executor before POST: a timeout/invalid receipt must + // never let a retry start this task on an unrelated inviter Desktop. + if err := repository.ReservePendingTaskDirectDevice(s.db.WithContext(ctx), task.ID, deviceID); err != nil { + s.edgeBreaker.RecordSuccess() // health succeeded; reservation failure is not an Edge outage + slog.Error("edge direct device reservation failed", "task_id", task.ID, "error", err) + return false, false + } + task.EdgeDeviceID = deviceID + dp.EdgeDeviceID = deviceID + return true, false +} diff --git a/hub-server/internal/service/dispatchsvc/agent_dispatch_edge_http.go b/hub-server/internal/service/dispatchsvc/agent_dispatch_edge_http.go index 9eb0e4848..1929ab05d 100644 --- a/hub-server/internal/service/dispatchsvc/agent_dispatch_edge_http.go +++ b/hub-server/internal/service/dispatchsvc/agent_dispatch_edge_http.go @@ -14,7 +14,6 @@ import ( "github.com/agenthub/hub-server/internal/metrics" "github.com/agenthub/hub-server/internal/model" - "github.com/agenthub/hub-server/internal/repository" "github.com/agenthub/hub-server/internal/service/dispatch" ) @@ -26,10 +25,6 @@ type edgeHTTPDispatchResult struct { func (s *DispatchService) dispatchToEdgeHTTP(ctx context.Context, task *model.PendingAgentTask, dp *dispatchPayload) edgeHTTPDispatchResult { miss := edgeHTTPDispatchResult{SafeToFallback: task == nil || task.EdgeDeviceID == ""} - deviceID := strings.TrimSpace(s.edgeCfg.DeviceID) - if task != nil && task.EdgeDeviceID != "" && task.EdgeDeviceID != deviceID { - return edgeHTTPDispatchResult{} - } // Pure Edge HTTP prep (#946); client/request side-effects stay here. // URL/token come from the injected edgeCfg (composition root), never // os.Getenv (#1549). @@ -97,54 +92,21 @@ func (s *DispatchService) dispatchToEdgeHTTP(ctx context.Context, task *model.Pe metrics.OutboundMetrics.Record(outboundmetrics.ProviderEdge, outboundmetrics.PurposeDispatch, outboundmetrics.CategoryFailure, "unreachable") return miss } - // Per-Edge circuit breaker: when Edge is down, consecutive dispatches would - // each block for the full HTTP client timeout (~30s), exhausting the - // dispatch semaphore and stalling the TTL/redispatch path. The breaker - // fails fast (no HTTP call) while open and admits a single half-open probe - // after edgeBreakerOpenDuration to test recovery. Pre-HTTP failures - // (insecure/marshal/req_create/edgeClient-nil) are config issues and do - // not trip the breaker; only client.Do/non_success/decode_fail indicate - // Edge health and are recorded. - if s.db == nil { - return edgeHTTPDispatchResult{} - } - owned, err := repository.DirectCallbackDeviceMatchesTask(s.db.WithContext(ctx), task.ID, deviceID) - if err != nil { - slog.Error("edge direct callback device lookup failed", "task_id", task.ID, "error", err) - return edgeHTTPDispatchResult{} - } - if !owned { - return miss - } - if !s.edgeBreaker.Allow() { - slog.Warn(dispatch.EdgeHTTPLogUnreachable, "task_id", task.ID, "url", parts.RunsURL, "error", "edge circuit breaker open") - if metrics.AgentDispatchEdgeHTTPFailures != nil { - metrics.AgentDispatchEdgeHTTPFailures.WithLabelValues("breaker_open").Inc() - } - metrics.OutboundMetrics.Record(outboundmetrics.ProviderEdge, outboundmetrics.PurposeDispatch, outboundmetrics.CategoryFailure, "breaker_open") - return miss - } - if !s.directCallbackRouteReady(ctx, parts) { - s.edgeBreaker.RecordFailure() - slog.Info("edge http dispatch: callback ownership route is unavailable", "task_id", task.ID) - return miss + if ready, safeToFallback := s.prepareDirectCallbackRoute(ctx, task, dp, parts); !ready { + return edgeHTTPDispatchResult{SafeToFallback: safeToFallback} } + return s.executeDirectEdgeRequest(task.ID, httpReq) +} - // Reserve the actual executor before POST: a timeout/invalid receipt must - // never let a retry start this task on an unrelated inviter Desktop. - if err := repository.ReservePendingTaskDirectDevice(s.db.WithContext(ctx), task.ID, deviceID); err != nil { - s.edgeBreaker.RecordSuccess() // health succeeded; reservation failure is not an Edge outage - slog.Error("edge direct device reservation failed", "task_id", task.ID, "error", err) - return edgeHTTPDispatchResult{} - } - task.EdgeDeviceID = deviceID - dp.EdgeDeviceID = deviceID +// executeDirectEdgeRequest runs only after the callback device is reserved. +// Every error here is an unconfirmed admission, never permission to fall back. +func (s *DispatchService) executeDirectEdgeRequest(taskID string, httpReq *http.Request) edgeHTTPDispatchResult { started := time.Now() resp, err := s.edgeClient.Do(httpReq) if err != nil { // G4: Edge unreachable is a classic silent-outage scenario; raised from // Debug to Warn so production can see it (#audit-G4). Counter quantifies rate. - slog.Warn(dispatch.EdgeHTTPLogUnreachable, "task_id", task.ID, "url", parts.RunsURL, "error", err) + slog.Warn(dispatch.EdgeHTTPLogUnreachable, "task_id", taskID, "url", httpReq.URL.String(), "error", err) if metrics.AgentDispatchEdgeHTTPFailures != nil { metrics.AgentDispatchEdgeHTTPFailures.WithLabelValues("unreachable").Inc() } @@ -161,7 +123,7 @@ func (s *DispatchService) dispatchToEdgeHTTP(ctx context.Context, task *model.Pe } plan := dispatch.PlanEdgeHTTPClientResponse(resp.StatusCode, respBody) if plan.NonSuccess { - slog.Warn(plan.LogMessage, "task_id", task.ID, "status", resp.StatusCode, "body_summary", SummarizeBodyForLog(respBody)) + slog.Warn(plan.LogMessage, "task_id", taskID, "status", resp.StatusCode, "body_summary", SummarizeBodyForLog(respBody)) if metrics.AgentDispatchEdgeHTTPFailures != nil { metrics.AgentDispatchEdgeHTTPFailures.WithLabelValues("non_success").Inc() } @@ -170,7 +132,7 @@ func (s *DispatchService) dispatchToEdgeHTTP(ctx context.Context, task *model.Pe return edgeHTTPDispatchResult{} } if plan.DecodeFail { - slog.Warn(plan.LogMessage, "task_id", task.ID, "error", plan.DecodeErr) + slog.Warn(plan.LogMessage, "task_id", taskID, "error", plan.DecodeErr) if metrics.AgentDispatchEdgeHTTPFailures != nil { metrics.AgentDispatchEdgeHTTPFailures.WithLabelValues("decode_fail").Inc() } @@ -185,6 +147,6 @@ func (s *DispatchService) dispatchToEdgeHTTP(ctx context.Context, task *model.Pe } metrics.OutboundMetrics.Record(outboundmetrics.ProviderEdge, outboundmetrics.PurposeDispatch, outboundmetrics.CategorySuccess, outboundmetrics.StatusOK) metrics.OutboundMetrics.Observe(outboundmetrics.ProviderEdge, outboundmetrics.PurposeDispatch, outboundmetrics.CategorySuccess, outboundmetrics.StatusOK, time.Since(started)) - slog.Info(dispatch.EdgeHTTPLogDispatched, "task_id", task.ID, "edge_run_id", plan.RunID, "url", parts.RunsURL) + slog.Info(dispatch.EdgeHTTPLogDispatched, "task_id", taskID, "edge_run_id", plan.RunID, "url", httpReq.URL.String()) return edgeHTTPDispatchResult{RunID: plan.RunID, CallbackOwner: owner} } diff --git a/scripts/verify/test-sleep-baseline.json b/scripts/verify/test-sleep-baseline.json index 366788aea..5851c4ef6 100644 --- a/scripts/verify/test-sleep-baseline.json +++ b/scripts/verify/test-sleep-baseline.json @@ -11,7 +11,7 @@ "edge-server/internal/lifecycle/process_executor_cancel_test.go": 2, "edge-server/internal/lifecycle/process_executor_helper_test.go": 1, "edge-server/internal/lifecycle/process_executor_hub_callback_test.go": 1, - "edge-server/tests/hub_e2e_test.go": 4, + "edge-server/tests/hub_e2e_test.go": 1, "hub-server/internal/app/background_test.go": 2, "hub-server/internal/bus/bus_test.go": 1, "hub-server/internal/bus/publish_close_stress_test.go": 2, diff --git a/scripts/verify/test-sleep-budget.json b/scripts/verify/test-sleep-budget.json index 25b270879..67bb35b31 100644 --- a/scripts/verify/test-sleep-budget.json +++ b/scripts/verify/test-sleep-budget.json @@ -146,17 +146,14 @@ ] }, "edge-server/tests/hub_e2e_test.go": { - "count": 4, - "total_ms": 3500, + "count": 1, + "total_ms": 2000, "max_ms": 2000, "owner": "DeliciousBuding", - "review": "2026-08-29", - "reason": "E2E lane (short-skipped in CI): 500ms x3 async callback grace windows (callbacks fire from a goroutine after the run reaches a terminal status; kept bounded); 2000ms real run completion (subprocess protocol time doubling as the negative window proving no callback fires without hubTaskId). The three former deadline-guarded run-status poll intervals (100/100/200ms) were converted to testkit.Eventually (approved 2026-08-29, #2055).", + "review": "2026-09-07", + "reason": "Fixture subprocess L2 lane: the remaining 2000ms negative observation window checks that a run without hubTaskId emits no callbacks. The three former 500ms callback grace windows were replaced with event-driven waits in #2352; the count/value budget only shrinks.", "sleeps": [ - { "ms": 500, "kind": "grace_window" }, - { "ms": 500, "kind": "grace_window" }, - { "ms": 2000, "kind": "real_protocol" }, - { "ms": 500, "kind": "grace_window" } + { "ms": 2000, "kind": "negative_window" } ] }, "hub-server/internal/app/background_test.go": {