mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-08 01:16:45 +08:00
* feat(server): start no run for an issue in triage (MUL-7213) An issue waiting in Triage produces no execution run, and the triage run itself leaves no trace an execution run can inherit. The rule cannot be enforced at the call sites. Five paths enqueue without looking at status at all (comment and mention dispatch, quick action, manual rerun, auto-retry), and dispatchIssueRun discards the enqueue error entirely, so a missed call site would fail silently. So the check sits at the queue door — immediately before every INSERT of an issue-linked agent_task_queue row — and the upstream short-circuits exist only so that previews and dispatch reasons stay honest about what the door will do. guardIssueNotInTriage covers all five insert sites: both enqueue funnels, FailTask's in-transaction retry, the orphan sweeper's retry, and the delegated-failure recovery dispatch. Two future paths are meant to be exempt and both are Triage's own — the triage run, and accept, which leaves Triage in the same transaction that enqueues the execution run. Upstream, WillEnqueueRun, maybeEnqueueOnAssign, the two shouldEnqueue*OnAssign gates and the comment trigger router all refuse Triage; rerun and quick action refuse with 403 issue_in_triage, a new dispatch reason code. WillEnqueueRun also treats leaving Triage as a creation rather than a status change: accept is when the issue becomes work, and the backlog-to-elsewhere rule would refuse it outright. Two narrower rules follow. retryEligible excludes triage runs for the reason it excludes autopilot runs: re-triaging reads the entry as it stands now, so replaying a failed attempt is never the right recovery. And GetLastTaskSession excludes them from all three CTEs, so the first execution run after accept does not resume the conversation in which the agent was still deciding whether the entry was worth keeping. Refusals are deliberately quiet where a caller cannot act on them. The FailTask guard never returns — that transaction also carries the parent's failed status, and losing it would leave the task stuck in 'running'. Verified: internal/handler, internal/service and cmd/server pass; every new test was confirmed to fail with the change neutralized. Co-authored-by: multica-agent <[email protected]> * fix(server): close two triage session bypasses around rerun (MUL-7213) Review found that session isolation had two holes the query-level exclusion could not reach, and that the two quietest queue-door paths had no direct coverage. A manual rerun resolves its session from the task the user named, via rerun_of_task_id, not from GetLastTaskSession — so after accept, naming the historical triage task reran the triager and resumed the triage conversation, walking straight around the exclusion. RerunIssue now refuses a triage source: redoing triage is re-triage, a different action on a different status. rerunSourceMatchesTaskScope refuses it too, which is what still holds for a rerun row an older server wrote mid-deploy. GetLatestTaskRolloutMissing reports only whether GetLastTaskSession fell back, so the two have to exclude the same rows. It did not, so a terminal triage run with session_rollout_missing told the first execution run after accept that it had lost the previous turn's context — about a turn it was never entitled to. Tests now cover the two doors that refuse silently: FailTask's in-transaction retry, which must leave the parent failed and add no child, and dispatchDelegatedFailureRecovery, which builds its coordinator task with a direct CreateAgentTask call. Both assert against a positive control so a zero cannot come from an inert fixture; the recovery test also shows the obligation is deferred rather than dropped — the same pending comment wakes the coordinator once the issue is accepted. Verified: internal/handler, internal/service and cmd/server pass; all five new tests were confirmed to fail with the fixes neutralized. Co-authored-by: multica-agent <[email protected]> * refactor(server): let a named agent talk on a Triage issue (MUL-7213) The rule shipped as "an issue in Triage starts no run", which is a mechanical restatement of the product red line rather than the line itself, and it was too wide: @-mentioning an agent to discuss a Triage entry was refused along with everything else. What Triage actually lacks is an EXECUTOR. Nothing may derive one from the issue — the assignee, a prefilled owner, a squad leader, an automatic retry — because that is exactly what the triager is still deciding, and because prefilling an owner is how a run starts today, the triager would otherwise dispatch the very owner it was only proposing. A person naming an agent by hand is a different act: no triager action produces it and the cost is one the person chose. That is the same shape as backlog, which parks assignment but lets comments through, one notch tighter because in Triage the assignee is only a proposal. So the queue door now judges the run's origin rather than the funnel it came through. OriginDerived is the zero value, so a caller that says nothing still gets the strict answer and a new enqueue path cannot leak in by omission; the two mixed wrappers take it as a parameter, which made the compiler enumerate every call site. Comment routing loses its blanket short-circuit and instead refuses the two routes that derive an executor — the assignee fallback and the squad leader fallback. Explicit mentions, replies to an agent, and thread continuation route as they do anywhere else. Rerun now follows its source: a discussion run may be repeated, naming no source means "run the assignee again" and is refused, and the triage run itself keeps its own reason. Assignment, quick action, auto-retry and all three session isolations are unchanged. The delegated-failure door is gone because main's #8406 gate subsumes it: Triage is its own lifecycle category, so it is neither unstarted nor started and both that gate and the pending-recovery query already exclude it. A test pins that. Verified: internal/handler, internal/service and cmd/server pass; both halves of the new rule were confirmed to fail when neutralized in place. Co-authored-by: multica-agent <[email protected]> * refactor(server): hold Triage in its own column, not in status (MUL-7213) `status` answers "how far along is this work". Triage answers "is this work at all". Holding the second question inside the first field makes every reader of `status` re-answer it, which is where the scattered `<> 'triage'` checks came from — main carried 9 of them and this branch had grown to 23. A column of its own asks it once. issue.triage_state is NULL for an ordinary issue and `pending` for one waiting in Triage. Adding a nullable column with no default is a catalog-only change, so no table rewrite and no backfill; the queue reads a partial index sized to itself rather than to the issue table. What follows from the move: - issuestatus goes back to 7 keys and 4 categories. Triage is no longer a category, a behavior, or something ExpandCategories can name, so CategoryForBehavior, BehaviorsForCategory, ParseCategory and ExpandCategories lose their special cases, and the dead board-order arm and status label go with them. - WillEnqueueRun loses `acceptedFromTriage`. Accept clears a column and then takes the ordinary create / assign path, so there is no "was in triage" transition left for the predicate to recognise. - The write lock disappears rather than narrowing. Status, project and parent on a Triage entry are the triager's proposal and accept is what confirms them, so ordinary writes reach them; the marker itself is not a field of the issue API, so there is nothing left to lock. - The duplicate guards carry `triage_state IS NULL` instead of a status key, and the delegated-failure recovery predicate gains the same, in both its SQL and its Go mirror. What does NOT follow, deliberately: the reserved NAME stays. Migration 475's CHECK is deployed and 476's renames are irreversible, so `triage` remains unavailable as a custom status key and Resolve keeps refusing it — one reserved word rather than five special cases. Read-path exclusion is a separate sub-issue and is not attempted here. Also from review: the quick-action refusal is now documented as a first-phase product choice rather than a consequence of the rule (its target is configured, not derived), and ErrIssueInTriage no longer claims that nothing runs. Verified: handler, service, issuestatus, cmd/migrate and cmd/server pass on a database migrated from 001; every column read was confirmed to turn its own tests red when neutralized. cmd/migrate's concurrent-index convention test caught a missing cleanup hook for the new index. Co-authored-by: multica-agent <[email protected]> * fix(server): hold the parent of a Triage entry (MUL-7213) Removing the write lock went one field too far. Status, project, assignee and the rest on a Triage entry are the triager's proposal and accept confirms them, so those writes belong. The parent is not a proposal: - the routing panel has no parent field. "Merge into an existing issue" links rather than re-parents, so a Triage entry hanging off a parent is a half-state the product never asks for; - a child in Triage carries a proposed status, so it is never terminal. stageBarrierClosed would wait on it forever — holding the whole sibling set, or one stage's frontier — and ChildIssueProgress would count it in the denominator. Both reach it through parent_issue_id, so refusing the write is what keeps them out of reach. That is behavior, not display, and it is why this one is worth a lock rather than a read-path filter. UpdateIssue and batch refuse a request that carries parent_issue_id for an issue in Triage, with or without a value: null clears, which is equally a parent decision the triager does not get to make. The batch asks one count rather than listing statuses, since "any" is all it needs. Verified: handler, service, issuestatus, issueguard, cmd/migrate and cmd/server pass; both new tests were confirmed to fail with the guard neutralized. Co-authored-by: multica-agent <[email protected]> * fix(server): drop the triage case from the handoff predicate (MUL-7213) MUL-7379 (#8435) landed on main while this branch was open and added issueStatusIsHandoff with a `triage` case, resting on Triage resolving to a category of its own without a catalog read. Once Triage stops being a status that resolution is gone, so the call errors on a value no issue can hold any more. The case is removed rather than repaired: a Triage entry carries whatever status the triager has proposed and is answered on that. Suppressing notifications for Triage entries is a different rule and belongs to MUL-7219 — the status model was giving it incidentally, and that was never the mechanism the design asks for. Verified on a database migrated from 001: cmd/server, internal/handler, internal/service, internal/issuestatus and cmd/migrate pass. Co-authored-by: multica-agent <[email protected]> * chore(db): renumber the triage migrations past the telemetry batch (MUL-7213) MUL-7236 (#8252) took 479-482 on main while this branch was open, so both triage migrations collided. 483 adds issue.triage_state, 484 builds its partial index; the concurrent-index cleanup hook moves with it. Verified on a database migrated from 001 with the new numbering. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]>
281 lines
11 KiB
Go
281 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 errors.Is(err, service.ErrIssueInTriage) {
|
|
h.writeDispatchBlocked(w, http.StatusForbidden, ReasonIssueInTriage)
|
|
return
|
|
}
|
|
// Not a dispatch refusal: the issue may well be runnable, and only the named
|
|
// source is ineligible. It falls through to the 400 below with the
|
|
// sentinel's own sentence, like the sibling "does not belong to this issue".
|
|
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)
|
|
}
|