Files
multica/server/internal/handler/task_lifecycle.go
f1ce9d4542 MUL-6883 perf(server): stop rescanning settled delegated-failure recoveries (#7820)
* feat(server): retire settled delegated-failure recoveries from the outbox scan

ListPendingDelegatedFailureRecoveries had no indexable notion of a finished
recovery. Its partial index (343) selected every recovery comment ever
written, so each sweeper tick re-proved through four joins and two NOT EXISTS
subqueries that history was already handled. LIMIT bounds the rows RETURNED,
never the rows CHECKED, so a tick that found nothing still grew more expensive
as history accumulated.

Add comment.recovery_settled_at and a partial index that holds only unsettled
recovery comments, and write the marker inside the transaction that makes a
task terminal. Every other exclusion the scan applies is reversible — an issue
can leave 'done', a cancelled retry can be superseded — so none of them can be
frozen into an index predicate. A delivery receipt on a terminal task cannot be
taken back, which is what makes it safe to record as durable state.

The marker is deliberately written only once the task is terminal.
SetTaskDeliveredCommentIDs replaces rather than appends a receipt while a task
is still dispatched, so a reclaim by a differently-capable daemon can drop an
id it will not deliver; settling at receipt-write time would freeze that
legitimate transient window into a permanently lost recovery. Exhaustion is the
one terminal outcome whose receipt lands on a row that may still be running, so
it retires the comment directly in the same transaction.

An unmarked row is the safe direction: it is scanned exactly as it was before
this column existed. That is what makes the rolling-deploy window harmless —
an older replica that writes a receipt without the marker only leaves work for
the backfill.

Co-authored-by: multica-agent <[email protected]>

* feat(server): backfill delegated-failure recoveries settled before the marker

The marker only covers tasks that reach a terminal status after the writing
code ships. History written before that keeps matching the unsettled index
predicate and keeps being re-checked on every sweeper tick, which is the cost
this change set exists to remove.

The walk retires a candidate only when a terminal task already recorded it in
delivered_comment_ids — the same monotonic condition the runtime path uses.
Its keyset watermark advances over every candidate EXAMINED rather than every
candidate marked: a recovery that is genuinely still pending must be left
alone, and keying on updated rows would hand back the same watermark forever
and never terminate.

Separate from migrations and startup, in the shape the other backfill commands
already use, so a large comment table cannot block a deploy. Running it is a
performance recovery step, not a correctness step — until it finishes the scan
behaves exactly as it does today.

Co-authored-by: multica-agent <[email protected]>

* fix(server): close the review gaps in the settled-recovery backfill and paths

Three fixes from review.

1. The backfill reproduced the bug it was written to fix. Its keyset ordered by
   id while idx_comment_delegated_failure_unsettled is (created_at, id), so the
   candidate scan read every unsettled row and sorted it to return LIMIT of
   them; and the receipt test ran a per-candidate EXISTS over agent_task_queue,
   the largest table in the database. The keyset now matches the index, and the
   receipt lookup is inverted into one agent_task_queue read per BATCH that
   hash-joins delivered ids back against the candidates.

   Measured on 500k comments / 200k tasks / 50k unsettled recoveries:
   17,393ms and 816,963 temp blocks before, 235ms and no temp spill after. The
   candidate scan goes from reading 50,000 rows to reading the 1,000 it needs.

   Total walk time is flat across batch size (50x228ms, 10x1016ms, 5x2199ms —
   all ~11s), so batch size only trades how long one statement holds row locks.
   1000 stays, now for a measured reason rather than a guessed one.

2. Bulk cancellation paths could strand a delivered receipt permanently
   unsettled, which lets the new index regrow the history it exists to remove.
   Manual rerun's pending-slot clear and agent archive were the two named in
   review; auditing the rest found comment-edit cancellation, both chat-session
   cancellations, workspace revoke and runtime teardown in the same position.
   All now settle — inside the caller's transaction where one exists, and
   through CaptureCancelledTasks for bulk paths that finalize outside one. The
   invariant is stated on the exported helper so the next such path has
   somewhere to read it.

3. --max-batches could not actually resume. The watermark lived only in the
   process, so a bounded rerun restarted at the front of the index — which is
   precisely where the rows this walk never settles live, so repeated bounded
   runs would rescan the same prefix and never reach the settleable history
   behind it. Adds --checkpoint-file (atomic replace, read on start) and
   explicit --after-created-at/--after-id, and logs the resume cursor on every
   exit path including SIGTERM.

Co-authored-by: multica-agent <[email protected]>

* fix(server): make delegated-failure settlement terminal-checked and atomic

Two fixes from review.

1. The settlement SQL trusted its caller. "A delivery receipt is only immutable
   once the task is terminal" is the safety premise of the whole design, but the
   exported helper enforced nothing: passing a dispatched or running task would
   settle a receipt a reclaiming daemon can still replace, and the marker is
   monotonic, so nothing could undo it. The predicate now lives in the SQL, so a
   non-terminal task updates no rows instead of destroying the obligation.

2. Bulk settlement was post-commit best-effort, and the comment claiming the
   sweeper would replay a failed one was simply wrong: the covering task is
   terminal and already holds the receipt, so ListPendingDelegatedFailureRecoveries
   excludes the comment. It would neither replay nor settle — it would sit in
   the partial index forever, restoring the unbounded scan this change set
   removes.

   Every terminal write now commits with its settlement. CancelTasksForIssue /
   ForAgent / ByTriggerComment and the rerun pending-slot clear run in a
   transaction; CancelQueuedChatTasks settles inside the one it already had; the
   sweeper and daemon-recovery bulk fails move behind TaskService wrappers that
   own the transaction; agent archive gets CancelTasksForArchivedAgent, which
   cancels and settles atomically while still skipping the per-task broadcast
   the archive event makes redundant.

   BroadcastCancelledTasks, CaptureCancelledTasks and HandleFailedTasks run
   after their caller committed, so they no longer settle at all — a failure
   there could be neither rolled back nor repaired. The best-effort helper is
   deleted rather than left available; every remaining call site passes a qtx.

Tests: a dispatched/running task with a receipt is not settled (fails without
the SQL predicate), a bulk cancel whose settlement fails rolls the cancel back,
and the archive path settles through its atomic entry point.

Co-authored-by: multica-agent <[email protected]>

* docs(server): correct settlement invariant comment and drop leaked test function

The SettleDeliveredDelegatedFailureRecoveries doc still described the
pre-fix contract — settlement reachable through CaptureCancelledTasks or a
post-commit best-effort call — which is exactly the shape the atomicity fix
removed. Restate the rule as it now stands (same qtx as the terminal write,
never from a post-commit helper) so the stale text cannot guide a future
change back into stranding rows in the partial index.

TestBulkCancelRollsBackWhenSettlementFails dropped its trigger but left
reject_recovery_settlement() behind in the shared test database; drop the
function too.

Co-authored-by: multica-agent <[email protected]>

---------

Co-authored-by: J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
Co-authored-by: Eve <[email protected]>
2026-09-01 12:11:50 +08:00

274 lines
11 KiB
Go

package handler
import (
"encoding/json"
"errors"
"io"
"log/slog"
"net/http"
"github.com/go-chi/chi/v5"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgtype"
"github.com/multica-ai/multica/server/internal/service"
db "github.com/multica-ai/multica/server/pkg/db/generated"
)
// RecoverOrphanedTasks is called by the daemon at startup for each runtime
// it owns. It atomically fails any dispatched/running tasks the server still
// believes belong to that runtime — those are the tasks the previous daemon
// process was running when it died — and triggers MaybeRetryFailedTask for
// each so the user sees a fresh attempt instead of a permanently stuck row.
//
// This is the targeted fix for "issue stuck at in_progress when daemon
// restarts mid-task": the runtime heartbeat sweeper takes up to 75s + the
// in-process task timeout (2.5h) to notice such tasks; the daemon itself
// knows the moment it comes back up, so we let it report orphan recovery.
func (h *Handler) RecoverOrphanedTasks(w http.ResponseWriter, r *http.Request) {
runtimeID := chi.URLParam(r, "runtimeId")
if _, ok := h.requireDaemonRuntimeAccess(w, r, runtimeID); !ok {
return
}
rows, err := h.TaskService.RecoverOrphanedTasksForRuntime(r.Context(), parseUUID(runtimeID))
if err != nil {
slog.Warn("recover-orphans failed", "runtime_id", runtimeID, "error", err)
writeError(w, http.StatusInternalServerError, "recover orphans failed")
return
}
// Funnel through the shared post-failure pipeline so we get the same
// task:failed events, agent reconcile, issue rollback, and auto-retry
// behaviour as the runtime sweeper. This was previously a fast-path
// that bypassed those side effects, leaving the UI stale when no retry
// was created (max_attempts exhausted, autopilot, non-retryable reason).
retried := h.TaskService.HandleFailedTasks(r.Context(), rows)
if len(rows) > 0 {
slog.Info("recover-orphans completed",
"runtime_id", runtimeID,
"orphaned", len(rows),
"retried", retried,
)
}
writeJSON(w, http.StatusOK, map[string]any{
"orphaned": len(rows),
"retried": retried,
})
}
// PinTaskSession lets the daemon persist the agent's session_id and
// work_dir as soon as they're known — typically right after the agent
// emits its first system message — so a crash mid-run doesn't lose the
// resume pointer needed to continue the conversation on the next attempt.
type PinTaskSessionRequest struct {
SessionID string `json:"session_id,omitempty"`
WorkDir string `json:"work_dir,omitempty"`
}
func (h *Handler) PinTaskSession(w http.ResponseWriter, r *http.Request) {
taskID := chi.URLParam(r, "taskId")
if _, ok := h.requireDaemonTaskAccess(w, r, taskID); !ok {
return
}
var req PinTaskSessionRequest
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
writeError(w, http.StatusBadRequest, "invalid request body")
return
}
if req.SessionID == "" && req.WorkDir == "" {
writeError(w, http.StatusBadRequest, "session_id or work_dir required")
return
}
params := db.UpdateAgentTaskSessionParams{ID: parseUUID(taskID)}
if req.SessionID != "" {
params.SessionID = pgtype.Text{String: req.SessionID, Valid: true}
}
if req.WorkDir != "" {
params.WorkDir = pgtype.Text{String: req.WorkDir, Valid: true}
}
// The pin can arrive after the user has already cancelled the run — it is
// asynchronous, and for Codex it waits for the rollout to reach the store.
// The cancel transaction then found no session to publish, so the chat's
// resume pointer is still on the previous turn and would shadow the session
// this pin is about to record (the claim handler reads the pointer before
// the GetLastChatTaskSession fallback). Advancing it here closes that half
// of GH #6340.
//
// Both writes commit together. Landing the session on the task row first and
// the pointer second leaves the same window the cancel path had: the row
// already names the new session while the pointer still names the previous
// turn, and a follow-up claimed in between resumes the older one. The lock
// comes first for the same reason it does in CancelTaskWithResult —
// chat_session -> agent_task_queue is the global order, and ErrNoRows simply
// means there is no session to lock or advance.
tx, err := h.TxStarter.Begin(r.Context())
if err != nil {
slog.Warn("pin-session failed to start tx", "task_id", taskID, "error", err)
writeError(w, http.StatusInternalServerError, "pin session failed")
return
}
defer tx.Rollback(r.Context())
qtx := h.Queries.WithTx(tx)
if _, err := qtx.LockChatSessionForTask(r.Context(), params.ID); err != nil && !errors.Is(err, pgx.ErrNoRows) {
slog.Warn("pin-session failed to lock chat session", "task_id", taskID, "error", err)
writeError(w, http.StatusInternalServerError, "pin session failed")
return
}
if err := qtx.UpdateAgentTaskSession(r.Context(), params); err != nil {
slog.Warn("pin-session failed", "task_id", taskID, "error", err)
writeError(w, http.StatusInternalServerError, "pin session failed")
return
}
// The statement re-reads the row, ignores anything that is not a cancelled
// chat task, and refuses to move the pointer when a newer turn already owns
// a session — so a straggler pin cannot drag the conversation backwards.
if err := qtx.AdvanceCancelledChatSessionPointer(r.Context(), params.ID); err != nil {
slog.Warn("advance cancelled chat session pointer failed", "task_id", taskID, "error", err)
writeError(w, http.StatusInternalServerError, "pin session failed")
return
}
if err := tx.Commit(r.Context()); err != nil {
slog.Warn("pin-session commit failed", "task_id", taskID, "error", err)
writeError(w, http.StatusInternalServerError, "pin session failed")
return
}
w.WriteHeader(http.StatusNoContent)
}
// RerunIssueRequest is the optional body of POST /api/issues/{id}/rerun.
// All fields are optional; an empty body keeps the legacy "rerun the issue's
// current assignee" behaviour used by the CLI.
type RerunIssueRequest struct {
// TaskID identifies the execution-log row the user clicked retry on.
// When set, the rerun targets the agent that ran that specific task
// (and reuses its leader/worker role) rather than the issue's current
// assignee — so clicking retry on row that belonged to a now-displaced
// agent re-fires that same agent, not the new assignee.
TaskID string `json:"task_id,omitempty"`
}
// RerunIssue manually re-enqueues an agent run for the issue. By default it
// targets the issue's current assignee (agent or squad leader); if the
// request body carries task_id, the rerun targets the agent that ran that
// specific past task instead. The new task is flagged force_fresh_session=true:
// the daemon claim handler skips the (agent_id, issue_id) session-resume
// lookup so the agent starts a clean session. A user clicking rerun has just
// judged the prior output bad — replaying the same conversation would replay
// the same poisoned state. (Automatic retry, by contrast, intentionally
// inherits the session — that path handles infrastructure failures, not bad
// output.)
func (h *Handler) RerunIssue(w http.ResponseWriter, r *http.Request) {
id := chi.URLParam(r, "id")
issue, ok := h.loadIssueForUser(w, r, id)
if !ok {
return
}
// Body is optional. A zero-length body or `{}` keeps the legacy
// assignee-driven rerun behaviour the CLI relies on.
var req RerunIssueRequest
if r.ContentLength != 0 {
if err := json.NewDecoder(r.Body).Decode(&req); err != nil && err != io.EOF {
writeError(w, http.StatusBadRequest, "invalid request body")
return
}
}
var sourceTaskID pgtype.UUID
if req.TaskID != "" {
parsed, ok := parseUUIDOrBadRequest(w, req.TaskID, "task_id")
if !ok {
return
}
sourceTaskID = parsed
}
// A manual rerun is a direct human action: attribute the new run to the
// rerunning member (MUL-4302 §5). Resolve the actor the same way assign/promote
// does; an agent A2A actor is not a human and threads an invalid actor.
userID, ok := requireUserID(w, r)
if !ok {
return
}
workspaceID := uuidToString(issue.WorkspaceID)
actorType, actorID := h.resolveActor(r, userID, workspaceID)
actorUserID := memberActorUserID(actorType, actorID)
// Re-validate the operator's invoke permission on the resolved target agent
// before cancelling / creating anything (MUL-4525). Issue visibility does not
// grant the right to trigger a private agent — a task_id rerun must gate the
// historical agent, not the (possibly reassigned) current assignee.
originatorUserID := h.invokeOriginatorFromRequest(r, actorType, actorID)
canInvoke := func(agent db.Agent) bool {
return h.canInvokeAgent(r.Context(), agent, actorType, actorID, originatorUserID, workspaceID)
}
task, err := h.TaskService.RerunIssue(r.Context(), issue.ID, sourceTaskID, pgtype.UUID{}, actorUserID, canInvoke)
if errors.Is(err, service.ErrRerunInvokeNotAllowed) {
h.writeDispatchBlocked(w, http.StatusForbidden, ReasonInvocationNotAllowed)
return
}
if err != nil {
slog.Warn("issue rerun failed", "issue_id", id, "error", err)
writeError(w, http.StatusBadRequest, err.Error())
return
}
resp := taskToResponse(*task, uuidToString(issue.WorkspaceID))
h.hydrateTaskAttributions(r.Context(), []*TaskAttribution{resp.Attribution})
writeJSON(w, http.StatusAccepted, resp)
}
// RetrySourceContextQuickCreate manually re-enqueues a failed issue-less
// quick-create while atomically moving its pending immutable source context to
// the new task. The workspace middleware supplies tenancy; the service also
// requires the original requester and the normal private-agent invoke gate.
func (h *Handler) RetrySourceContextQuickCreate(w http.ResponseWriter, r *http.Request) {
userID, ok := requireUserID(w, r)
if !ok {
return
}
workspaceID, ok := parseUUIDOrBadRequest(w, ctxWorkspaceID(r.Context()), "workspace id")
if !ok {
return
}
requesterID, ok := parseUUIDOrBadRequest(w, userID, "user id")
if !ok {
return
}
taskID, ok := parseUUIDOrBadRequest(w, chi.URLParam(r, "taskId"), "task id")
if !ok {
return
}
canInvoke := func(agent db.Agent) bool {
return h.canInvokeAgent(r.Context(), agent, "member", userID, userID, uuidToString(workspaceID))
}
task, err := h.TaskService.RetrySourceContextQuickCreate(r.Context(), workspaceID, requesterID, taskID, canInvoke)
if writeIssueLimitReached(w, err) {
return
}
if errors.Is(err, service.ErrRerunInvokeNotAllowed) {
h.writeDispatchBlocked(w, http.StatusForbidden, ReasonInvocationNotAllowed)
return
}
if errors.Is(err, service.ErrSourceContextRetryUnavailable) {
writeJSON(w, http.StatusConflict, map[string]any{
"code": "source_context_retry_unavailable",
"error": "This context can no longer be retried. Start again from the branch point.",
})
return
}
if err != nil {
slog.Warn("source context quick-create retry failed", "task_id", uuidToString(taskID), "error", err)
writeError(w, http.StatusInternalServerError, "retry source context quick create")
return
}
resp := taskToResponse(*task, uuidToString(workspaceID))
h.hydrateTaskAttributions(r.Context(), []*TaskAttribution{resp.Attribution})
writeJSON(w, http.StatusAccepted, resp)
}