mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-08 01:16:45 +08:00
7ebe0bf58d
* 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]>