Files
multica/server/pkg/db
Multica Eve 75695a2e40 fix(comments): guarantee at-least-once processing of user comments (MUL-4195) (#5068)
* fix(comments): guarantee at-least-once processing of user comments (MUL-4195)

Consecutive comments on an issue were silently dropped: a new comment that
arrived while the agent already had a queued/dispatched task was discarded by
the HasPendingTaskForIssueAndAgent dedup, losing the user's follow-up
instruction with no visible trace. Comments — unlike chat — are deliberate,
addressed, persisted input and must never vanish.

This makes comment handling at-least-once while keeping concurrency bounded to
one run per (issue, agent):

- Merge, don't drop (PR1): a comment landing while a not-yet-started task
  exists is folded into that task — the prior trigger becomes a coalesced
  comment and the new one becomes the trigger, so a single run still covers
  every deliberate comment. Falls back to a fresh enqueue if the pending task
  was claimed mid-flight, so nothing is lost in the race.
- Completion reconciliation (PR2): on task completion, a member comment newer
  than the run's started_at schedules exactly one follow-up via the normal
  trigger pipeline. Loop-safe: member-authored only, capped by the existing
  per-(issue,agent) dedup, and terminating.
- Visibility (PR3): coalesced_comment_ids is surfaced on the task API and in
  the run prompt so the covered comments are explicit.

Migration 145 adds agent_task_queue.coalesced_comment_ids UUID[].

Tests: merge-not-drop preserves all three of a rapid burst and repoints the
trigger to the newest; reconciliation query gates on member/since; e2e
CompleteTask enqueues a follow-up for a mid-run member comment and does not for
none.

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

* fix(comments): address review — originator gate, agent-scoped reconcile, cross-thread coalesced prompt (MUL-4195)

Resolves GPT-Boy's Request-changes review on PR #5068.

Must-fix #1 — merge no longer inherits a stale originator/runtime context.
MergeCommentIntoPendingTask now only folds a comment into a pending task
whose originator_user_id IS NOT DISTINCT FROM the new comment's originator.
runtime_mcp_overlay / runtime_connected_apps are a pure function of
(originator, agent) and the agent is fixed, so a matching originator keeps
the stored overlay/attribution valid; a differing originator (e.g. user B
commenting on a task originated by user A) matches no row and the caller
enqueues a fresh follow-up with B's own context instead of reusing A's.
trigger_summary is refreshed to the new trigger comment.

Must-fix #2 — completion reconcile no longer re-wakes unrelated agents.
reconcileCommentsOnCompletion computes the latest member comment's triggers
and keeps ONLY the agent that just completed, instead of fanning the comment
out through the full pipeline. An @-mention of agent B during agent A's run
is triggered once at creation time and is no longer replayed (double-run)
when A completes.

Should-fix #3 — coalesced-comment prompt no longer assumes a single thread.
The claim response now carries each folded comment's thread id / author /
created_at / content (CoalescedCommentData); the prompt embeds them directly
so the agent addresses cross-thread folded comments without the wrong
"they are in the triggering thread" hint. Old servers that ship only ids
fall back to an issue-wide fetch, still without the same-thread assumption.

Tests: TestMergeCommentIntoPendingTask_OriginatorGate (query gate),
TestCompleteTask_DoesNotReTriggerOtherAgentMentionedDuringRun (reconcile
scoping), TestBuildCommentPromptCoalescedCrossThread / IDsOnlyFallback
(prompt). Existing MUL-4195 suites still pass.

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

* fix(comments): close unique-index drop + dispatched-window race in comment coalescing (MUL-4195)

Second-round review follow-up on PR #5068.

Must-fix #1 — originator-mismatch no longer drops the comment.
The previous originator gate returned ErrNoRows on a different originator and
the caller fell through to a fresh enqueue, which collided with the
idx_one_pending_task_per_issue_agent unique index (one queued/dispatched task
per (issue, agent)) — silently dropping the second user's comment. Replaced
the gate with recompute-on-merge: MergeCommentIntoPendingTask now re-stamps
originator_user_id, runtime_mcp_overlay, runtime_connected_apps and
trigger_summary to the new comment's originator. A different member's comment
folds into the single coalescing run carrying the latest instruction's own
identity/overlay (no cross-user capability bleed, no drop, no collision).

Must-fix #2 — comment arriving in the claim→StartTask window is no longer lost.
Merge now targets only PRE-CLAIM states ('queued','deferred'); a
dispatched/running task is never a merge target, so a post-claim comment is
never falsely stamped into coalesced_comment_ids as "delivered". Completion
reconcile is re-anchored on dispatched_at (the moment the claim response is
built) instead of started_at, and sweeps ALL undelivered member comments since
that anchor — replaying each through the normal enqueue path so they coalesce
into one bounded, agent-scoped follow-up run. This covers the dispatch→start
window a started_at anchor missed.

Enqueue path: on a merge miss the caller no longer blindly fresh-enqueues
(which could collide with a dispatched sibling); it defers to the active
task's completion reconcile via HasActiveTaskForIssueAndAgent, and only
fresh-enqueues when no active task exists.

Tests: rewrote the query test to
TestMergeCommentIntoPendingTask_RecomputesOriginatorAndSkipsDispatched;
added TestConsecutiveCommentsDifferentOriginatorsFullEnqueuePath (full handler
enqueue path, two distinct originators) and
TestCompleteTask_ReconcilesDispatchedWindowComment (claim→start window). All
existing MUL-4195 handler/cmd-server/daemon/service suites still pass.

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

* fix(comments): catch pre-dispatch merge-race comment in completion reconcile (MUL-4195)

Third-round review follow-up on PR #5068.

Race: a member comment is created while the task is still queued, but its
merge loses the race to the daemon claiming the task (queued→dispatched). The
merge then finds no pre-claim row (ErrNoRows), the enqueue path defers to
reconcile — but the comment's created_at is BEFORE dispatched_at, so the
dispatched_at-anchored reconcile skipped it and the comment vanished with no
task coverage.

Fix: anchor completion reconcile on the task's created_at (which always
precedes dispatch) instead of a dispatch/start timestamp, and exclude the
run's DELIVERED SET — trigger_comment_id ∪ coalesced_comment_ids. Because
merges only ever touch pre-claim rows, that set is exactly what the claim
response carried, so any member comment created since the task was made that
is NOT in it was genuinely undelivered and earns a bounded follow-up. This
catches the pre-dispatch merge-race comment and the dispatch→start comment,
while never re-firing a comment that was delivered as a pre-claim coalesced
entry.

Test: TestCompleteTask_ReconcilesPreDispatchMergeRaceComment reproduces the
race (comment created pre-dispatch, task dispatched before merge, plus a
delivered coalesced comment) and asserts exactly one follow-up, triggered by
the race comment, with the delivered coalesced comment excluded. Existing
reconcile fixtures updated to set a realistic created_at (the production
invariant that created_at is the earliest task timestamp).

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

* fix(comments): merge only into the queued task, never a deferred fallback (MUL-4195)

Fourth-round review follow-up on PR #5068.

MergeCommentIntoPendingTask targeted status IN ('queued','deferred') ordered
by created_at DESC. When a (issue, agent) pair had both an older queued task
(the run about to be claimed) and a newer deferred assignee-fallback task, a
new comment merged into the deferred row instead of the queued one — so the
comment missed the imminent run and the deferred fallback could later promote
into a duplicate/conflicting run.

This merge is only ever reached when HasPendingTaskForIssueAndAgent matched a
queued/dispatched task (it never inspects deferred), so the coalescing target
must be the queued row. Restricted the merge target to status = 'queued'
(the unique index guarantees at most one). Deferred fallbacks keep their own
fire_at/promotion escalation lifecycle and are never a merge target.

Test: TestMergeCommentIntoPendingTask_TargetsQueuedNotDeferred seeds an older
queued task + a newer deferred fallback for the same (issue, agent), merges a
new comment, and asserts it lands on the queued task (trigger repointed, old
trigger coalesced) while the deferred fallback is left untouched.

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-09 12:48:57 +08:00
..