Commit Graph

4 Commits

Author SHA1 Message Date
Multica Eve
af9d90bd83 fix(daemon): wake queued tasks after predecessor exits (#5379)
Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>
2026-07-14 13:16:50 +08:00
Multica Eve
c3dd9ec845 Machine-level batch task claim endpoint (MUL-4257) (#5193)
* feat(daemon-claim): machine-level batch task claim endpoint (MUL-4257)

Collapse the per-runtime /tasks/claim poll fan-out into a single machine-level
batch claim to cut /api/daemon claim request volume.

Server:
- agent.sql: = ANY(runtime_ids) batch variants of the claim queries
  (ListQueuedClaimCandidatesByRuntimes, PromoteDueDeferredTasksForRuntimes,
  ReclaimStaleDispatchedTasksForRuntimes); runtime.sql: GetAgentRuntimes(= ANY)
  so a whole machine's runtimes are resolved/promoted/reclaimed/listed in a
  constant number of queries instead of N.
- service.ClaimTasksForRuntimes: claim up to max_tasks across a runtime set,
  preserving per-(issue,agent) serialization, the concurrency cap, the
  empty-claim cache short-circuit, and every dispatch side effect. Batch
  promote replays the per-row side effects (task:queued + empty-cache Bump).
- handler.ClaimTasksByRuntime (canonical POST /api/daemon/tasks/claim, with a
  transitional /claim alias): validates daemon_id (required; must match the
  mdt_ token) and rejects runtimes bound to a different daemon (group-ownership
  check mirroring the WS path); resolves+authorizes each runtime_id; claims;
  and finalizes each task through the SAME FinalizeTaskClaim as the per-runtime
  endpoint (atomic token + delivered_comment_ids receipt), requeueing the exact
  claim and omitting it on failure. buildClaimedTaskResponse is extracted from
  the per-runtime handler and returns the delivered-comment ids plus a
  structured *claimBuildFailure so both paths share identical payload building
  and failure semantics (workspace-isolation, chat-input load/empty).
- max_tasks: negative -> 400, zero -> empty (never coerce to 1), positive
  capped at 32. runtime_ids parsed with non-panicking util.ParseUUID.

Daemon:
- Client.ClaimTasks posts daemon_id + runtime set + free-slot count to the
  canonical path under a short request-scoped timeout, bounding the
  head-of-line coupling the per-runtime pollers avoid (MUL-1744).

Tests: service batch drain / max_tasks cap / deferred-promote receipt /
finalize-failure rollback+requeue; handler routing + token, cross-workspace
skip, cross-daemon skip, daemon_id required, owner-missing cancel,
max_tasks=0/negative, invalid-uuid skip, comment delivery receipt, stale-reclaim
replacement receipt; client posts/parses (daemon_id + canonical path).

Follow-up: cut the daemon pollLoop over to a single batched poller (flips the
MUL-1744 isolation contract; needs its concurrency tests redesigned).

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

* feat(daemon-ws): generic WS request/response transport for daemon RPC (MUL-4257)

Add a generic daemon->server request/response layer over the existing WS
control connection, the transport for WS-first claim (HTTP fallback):
- protocol: daemon:rpc_request / daemon:rpc_response envelopes with a
  correlation request_id + method + body, and an rpc-v1 capability gate.
- daemonws.Hub: SetRPCHandler + goroutine-dispatched handleRPCFrame (bounded
  by a per-connection in-flight cap) that echoes the request_id; missing
  handler / saturation return non-2xx so the daemon falls back to HTTP.
  Read limit raised to 64KB for rpc requests carrying a runtime set.
- hub tests: round-trip, handler-error->non-2xx, no-handler->503.

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

* feat(daemon-ws): WS-first task claim over the generic RPC transport (MUL-4257)

Bind claim to the WS request/response layer, with HTTP fallback:
- server: handler.DaemonRPCHandler adapts a daemon:rpc_request (method
  tasks.claim) to the existing HTTP ClaimTasksByRuntime via a synthetic
  in-process request carrying the WS connection's identity (daemon_id +
  workspace + capabilities), so all auth / payload-building / finalization is
  reused unchanged. Wired via daemonHub.SetRPCHandler. ClientIdentity now
  captures X-Client-Capabilities so capability gating matches the HTTP path.
- daemon: wsRPCClient correlates responses by request_id over the shared WS
  connection; attached to the live connection's write channel (guarded so a
  Call racing teardown never sends on a closed channel) and detached on
  disconnect. rpc_response frames are routed in the read loop.
  Daemon.ClaimTasksWSFirst issues tasks.claim over WS and falls back to the
  HTTP claim endpoint on any transport failure (no conn / buffer full /
  timeout) — wired into the poller at the poller cutover.
- tests: handler tasks.claim RPC end-to-end (claims + dispatches) + unknown
  method 404; daemon wsRPCClient round-trip / timeout / unavailable /
  server-error / detach-fails-pending (all under -race).

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

* feat(daemon): cut claim poller over to machine-level ClaimTasksWSFirst (MUL-4257)

Replace the per-runtime HTTP poll loop with a single batch poller: each cycle
acquires all free execution slots (slot-before-claim) and issues ONE
ClaimTasksWSFirst across every runtime the daemon hosts (WS-first, HTTP
fallback), dispatching each returned task to its runtime. Wakeups (targeted /
catch-up / runtime-set change) collapse to one nudge. Removes runRuntimePoller
+ runtimePollOffset. The WS handshake now advertises the same capabilities as
HTTP (+ rpc-v1) so WS-built claim payloads keep skill-ref / coalesced-comment
gating.

Trades per-runtime isolation (MUL-1744) for one request, bounded by the short
per-request WS timeout / client timeout. Tests: batch poller claims across
runtimes + skips-at-capacity + pollLoop shutdown drain (replacing the
per-runtime poller tests); heartbeat isolation + runtime-set watcher kept.

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

* fix(daemon-ws): WS RPC disconnect-race panic + batch stale-comment-plan repair (MUL-4257)

Two PR #5193 review blockers:

1) WS RPC send-on-closed-channel race, both ends:
   - server: give each connection a cancelable ctx (cancelled on readPump
     teardown) and run the RPC handler under it, so a slow claim stops on
     disconnect; guard c.send with sendMu/sendClosed (trySend) so a late RPC
     response goroutine never writes to the closed channel. Heartbeat ack routed
     through the same guard.
   - daemon: wsRPCClient.deliver now sends under the mutex, serialized with
     attach(nil)'s close+delete, so a delivered response can't hit a channel
     the detach path just closed.
   - regressions (-race): daemon deliver-vs-detach; server
     disconnect-during-handler-response.

2) batch claim now runs the stale-comment-plan repair: extracted the
   per-runtime handler's repair (trigger deleted, only coalesced survive ->
   cancel + replay survivors) into shared repairStaleCommentPlanIfNeeded, called
   by both claim paths. Prevents the batch path (now the default poller) from
   finalizing+dispatching a task with no comment input and silently dropping the
   surviving user comment. Regression: batch omits the stale task, cancels it,
   and rebuilds the survivor into a new trigger plan.

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

* fix(daemon-ws): server-side RPC deadline + legacy claim fallback (MUL-4257)

Two review blockers:

1) WS RPC timeout/fallback (GPT-Boy): the daemon's WS wait didn't cancel
   server-side claim, so a slow WS claim could commit after the daemon fell
   back to HTTP, leaking dispatched tasks and breaking the free-slot bound.
   Fix: RPC envelope carries TimeoutMs; the server bounds the handler ctx by it
   (so ClaimTasksByRuntime's tx is cancelled/rolled back at the deadline), and
   the daemon waits budget + grace so a claim that committed before the deadline
   still reports back. A committed-then-unreported claim degrades to the same
   stale-reclaim safety net as HTTP, never a double effective claim. Regression:
   server-side TimeoutMs cancels the handler.

2) Backward compat (Terra-Boy): a new daemon against a server without the batch
   route (/api/daemon/tasks/claim 404) couldn't claim. Fix: ClaimTasksWSFirst
   falls back to the legacy per-runtime ClaimTask loop on a batch 404 and caches
   'batch unsupported' (reset on WS reconnect to re-probe after a server
   upgrade). Regression: server exposing only the legacy route.

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

* fix(daemon-ws): no double-claim on WS teardown/detach (MUL-4257)

Sol-Boy review blocker: on reconnect, teardown failed the pending RPC (→ HTTP
fallback) but then flushed the queued tasks.claim frame to the still-alive
socket, so the server committed the WS claim on top of the HTTP one — double
claim, WS batch orphaned to stale reclaim, breaking the free-slot bound.

- Teardown now closes the connection FIRST, so runWSWriter discards the queued
  RPC frame (write error path) instead of delivering it.
- A detach while a claim's frame is already in flight now returns a distinct
  errWSRPCUncertain; ClaimTasksWSFirst does NOT HTTP-fall-back on uncertain (the
  WS claim may have committed) — it skips the cycle and lets reclaim / the next
  poll recover. Genuine 'not sent' / timeout still fall back (safe: the
  server-side deadline guarantees no uncommitted claim by budget+grace).
- Regression: detach during an in-flight WS claim asserts zero HTTP claims
  (at most one path claims); plus the existing detach/deliver-race and
  server-timeout tests.

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

* fix(daemon-ws): cancelable RPC frames close the backpressure double-claim (MUL-4257)

Sol-Boy review blocker: the client's response budget starts at enqueue, but
the socket write is async (10s write deadline). A backpressured writer could
hold a tasks.claim in the local queue past the client timeout — the daemon
HTTP-fell-back, then the writer woke and delivered the stale WS frame, so the
server committed it too: same free slots claimed twice. No detach occurs, so
the prior errWSRPCUncertain fix did not cover it.

- WS frames are now cancelable (wsOutbound{sent,canceled} under a mutex). The
  writer calls beginWrite() before WriteMessage and skips cancelled frames.
- On give-up (timeout / detach / ctx), Call cancels the queued frame: if it was
  still pending the cancel wins and the frame is guaranteed never delivered
  (errWSRPCUnavailable → safe HTTP fallback); if the writer already began
  sending it the cancel loses and the outcome is errWSRPCUncertain (no
  fallback). The decision is atomic, so at most one transport claims.

Tests: wsOutbound cancel-before-write vs write-before-cancel; Call timeout
cancels an unsent frame (writer then drops it) vs uncertain when already sent;
plus the updated detach and existing timeout/race tests.

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

* fix(batch-claim): return partial success instead of dropping committed claims (MUL-4257)

Sol-Boy review blocker: ClaimTasksForRuntimes reclaims (step 2) and claims per
agent (step 6) in independent transactions, but a step-4 candidate-SELECT error
or a mid-loop ClaimTask error did 'return nil, err' — discarding tasks already
committed as dispatched. The handler 500s; the daemon sees a definite (non-
uncertain) 500 and HTTP-falls-back, claiming a SECOND batch into the same free
slots while the first batch waits for stale reclaim — the double-claim this PR
removes.

- Both error paths now prefer partial success: if any task has already
  committed (claimed non-empty), return it (nil error) so the handler finalizes
  and returns 200; the errored candidates stay queued for the next poll. The
  remaining error is logged. Only a genuinely empty result still returns the
  error (safe: no committed claim to lose, HTTP fallback just re-fails).

Regression (internal/service, DB-backed, fault-injected):
- PartialSuccessOnSecondAgentClaimFailure: fail the 2nd ClaimTask's Begin →
  the first agent's committed task is returned, not dropped.
- PartialSuccessOnCandidateQueryFailureAfterReclaim: a stale dispatched task is
  reclaimed, then the candidate SELECT fails → the reclaimed task is returned.

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

---------

Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>
2026-07-14 11:53:42 +08:00
Wes
5e7587ad07 Optimize daemon runtime wakeups (#3859) 2026-06-08 12:51:13 +08:00
Bohan Jiang
b08594f2f6 fix(daemon): isolate runtime poll & heartbeat schedules per runtime (#2116)
* fix(daemon): isolate runtime poll & heartbeat schedules per runtime

A daemon serving multiple workspaces ran a single round-robin poll loop
and a single HTTP heartbeat loop across every registered runtime. A 30s
HTTP timeout for any one runtime serialized that delay across all the
others — observed in production as one workspace's runtimes wedging
every other workspace's runtimes on the same daemon.

This change:

- Replaces the shared runtime-set channel with a multi-subscriber
  watcher so taskWakeupLoop, heartbeatLoop, and pollLoop can each
  react to runtime-set changes independently.
- Splits heartbeatLoop and pollLoop into supervisor + per-runtime
  worker goroutines. Each runtime owns its claim cadence and its
  heartbeat ticker, so a slow request on one runtime no longer blocks
  any other.
- Stagers the per-runtime heartbeat first tick by a jittered delay up
  to one full interval to avoid a thundering herd at startup.
- Sizes the WS writer channel to scale with the runtime count
  (max(16, 2*N)) so a full per-runtime heartbeat batch always fits;
  the previous fixed 8-slot buffer dropped heartbeats whenever a
  daemon watched more than ~8 runtimes.

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

* fix(daemon): acquire execution slot only after ClaimTask, drain pollers before taskWG

Two issues from review on the previous commit:

1. Acquiring the shared task slot before ClaimTask reintroduced the very
   head-of-line blocking the refactor was meant to remove. With
   MaxConcurrentTasks=1, a slow claim on one runtime parked the only slot
   for the duration of the HTTP timeout (up to 30s), starving every other
   runtime's claim attempts. Slots are now acquired after the claim
   returns a task; other runtimes' pollers stay free to claim. The
   already-dispatched task waits for a slot under MaxConcurrentTasks
   bounds, which is the same backpressure shape we had before.

2. pollLoop's shutdown path called taskWG.Wait immediately after
   cancelling pollers, but a poller could still be between ClaimTask
   returning a task and taskWG.Add(1). When taskWG's counter is zero
   that races with Wait — undefined sync.WaitGroup misuse, sometimes
   panic. Added a pollerWG so the supervisor blocks until every poller
   goroutine has actually returned before reaching taskWG.Wait.

Tests:
- TestRunRuntimePollerIsolatesSlowRuntime now uses MaxConcurrentTasks=1
  (was 4) so it would have failed under the old slot-before-claim path.
- New TestPollLoopShutdownWaitsForPollersBeforeTaskWG drives the exact
  race window — claim returns a task at the same moment shutdown fires —
  under -race.

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

* fix(daemon): acquire slot before ClaimTask so capacity-waiters never enter dispatched

The previous commit moved slot acquisition AFTER ClaimTask to address a
review concern about head-of-line blocking with MaxConcurrentTasks=1.
That introduced a strictly worse failure mode: server-side ClaimTask
flips the task to `dispatched` immediately (agent.sql:174-176), and the
runtime sweeper fails any task in `dispatched` for >300s with
`failed/timeout` (runtime_sweeper.go:25-28). When local execution
capacity is full and the next claimed task can't acquire a slot within
5 minutes, the user sees the exact failure this issue is fixing —
`dispatched_at` set, `started_at` NULL, `failure_reason=timeout`.

Reverted to slot-before-claim. The trade-off is the original review
concern: with MaxConcurrentTasks=1 and a slow ClaimTask, other
runtimes' claims are delayed by up to client.Timeout=30s. That's a
30s polling delay, not a failure — server-side those tasks remain
`queued` (no timeout in that state) until a slot frees. 30s ≪ 300s,
so other runtimes' tasks cannot get sweeper-failed because of this.

The pollerWG fix from the previous commit (avoiding sync.WaitGroup
misuse on shutdown) is preserved.

Tests:
- TestRunRuntimePollerIsolatesSlowRuntime: MaxConcurrentTasks back to
  4 (the pre-issue baseline) — the headroom case where slot-before-
  claim still gives full per-runtime isolation.
- New TestRunRuntimePollerSkipsClaimWhenAtCapacity: holds the only
  slot and verifies the poller never calls ClaimTask while sem is
  empty. The previous "claim first" path would have failed this.

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

---------

Co-authored-by: multica-agent <github@multica.ai>
2026-05-06 14:13:27 +08:00