mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-09 01:46:46 +08:00
* 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]>
274 lines
11 KiB
Go
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)
|
|
}
|