Files
multica/server/internal/handler/trigger_test.go
Bohan Jiang 69744241e8 fix(comment): restore autopilot @mention delegation authority (MUL-4857) (#5512)
* fix(comment): restore autopilot @mention delegation authority (MUL-4857)

A schedule/webhook autopilot run is unattributed by design (no top-of-chain
human originator, MUL-4302). Since MUL-3963 the A2A invoke gate (canInvokeAgent)
keys on that originator, so a mid-run @agent/@squad delegation on an
autopilot-created issue fails closed for the DEFAULT private agent (and
member-scoped public_to agents): the mention renders but no run is enqueued.
The SAME autopilot's first dispatch is admitted via the autopilot creator
(autopilotAdmitInvoke -> canCreatorInvokeAgent), so first-dispatch and mid-run
delegation disagreed.

Align them: when an unattributed agent/system-authored comment on an
autopilot-origin issue reaches computeCommentAgentTriggers with no originator,
fall back to the autopilot creator as the effective invoking user for the gate.
The gate still runs (no unrestricted agent-to-agent bypass); it is authorization
only -- the enqueued task's originator/attribution stays unattributed. Scoped to
autopilot-origin issues so other unattributed chains stay fail-closed.

Adds a DB-backed regression test covering: creator-owns-target admits, a
non-autopilot unattributed run stays denied, and a creator without invoke rights
stays denied.

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

* fix(comment): bind autopilot @mention authority to verified task lineage (MUL-4857)

Address the review's confused-deputy finding on the P0 fix. The first cut keyed
the invoke-gate fallback on issue provenance + an empty originator alone
(invokeAuthorityForAutopilotIssue took only the issue), so any unattributed run
could borrow a stranger autopilot creator's rights merely by commenting on that
autopilot's issue — and the fallback also leaked past explicit @mention into the
plain-comment squad-leader path and system actors.

Rework it so the autopilot-creator authority is granted ONLY when the SPEAKING
task's lineage is verified against this issue:

  - resolve the authority separately (new AutopilotDelegationAuthorityUserID on
    commentTriggerComputeOptions), never by overwriting OriginatorUserID; the
    gate reads it through opts.effectiveInvoker() only when no human originator
    resolved, so attribution stays untouched;
  - resolve from a server-trusted speaking task — X-Task-ID on create/preview,
    comment.source_task_id on edit/reconcile — via autopilotDelegationAuthority,
    which admits only when author == task agent AND task.issue_id == this issue
    AND the issue is autopilot-origin, then keys on the member autopilot creator;
  - do NOT key on autopilot_run_id: in create_issue mode (the reported case) the
    leader task is enqueued through the ordinary issue-assignment path and has no
    autopilot_run_id — the task.issue_id == issue binding is what proves the run
    is part of this autopilot's work while rejecting foreign-issue runs.

Tests: replace the provenance-only regression with lineage-bound coverage —
verified-lineage-admits, creator-without-rights-denied, non-autopilot-denied,
missing-source-task-denied, cross-issue-source-task-denied, author!=task-agent-
denied — plus an end-to-end CreateComment path asserting the private worker is
enqueued and the delegated run stays unattributed. Verified the fallback is
load-bearing (positive + e2e fail with it disabled) and the full internal/handler
package passes. Skill docs (multica-mentioning) updated to the lineage-bound
contract and new helper names.

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

* fix(comment): make autopilot @mention authority consistent across defer/edit (MUL-4857)

Second review round (Elon) surfaced two must-fixes on top of the lineage binding.

1. Busy-target completion reconcile lost the authority. A delegation to a target
   that is already running is deferred to that target's completion reconcile
   (reconcileCommentsOnCompletion). That path recomputed triggers with only the
   (empty) originator, so an unattributed autopilot delegation's follow-up was
   gate-denied again and silently dropped. It now restores the delegation
   authority from comment.source_task_id, so the follow-up fires once the target
   frees up — still unattributed.

2. Edit could borrow the old authoring run's authority, and preview != save. The
   edit preview keyed authority on the current request task while save keyed it on
   the comment's original source_task_id, so an agent editing its old autopilot
   comment from a task on an UNRELATED issue would fail-closed in preview but reuse
   the old autopilot creator's authority on save (cross-issue confused-deputy, and
   a preview/side-effect divergence). Fix: treat source_task_id as the persisted
   per-action authority lineage and re-stamp it on edit to the CURRENT editing
   task, issue-scoped exactly like CreateComment. A cross-issue edit re-stamps it
   to NULL, so preview, save, AND the deferred reconcile all fail closed
   identically. UpdateComment query gains a source_task_id param (sqlc regen).

Also locks the review-accepted behavior that effectiveInvoker() carries the
autopilot-creator authority into the plain assigned-squad-leader wake (a worker's
result comment on the autopilot issue can still wake the private leader).

Tests: reconcile-restores-authority (owns -> one unattributed follow-up; no rights
-> none); edit re-stamp (same-issue keeps authority and triggers; cross-issue
clears source_task_id and fails closed); worker-result wakes private squad leader.
Verified both fixes are load-bearing (each negative control reproduces the exact
regression Elon described), full internal/handler + internal/service packages pass,
gofmt/vet clean. Skill docs (multica-mentioning) updated.

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

* fix(comment): clear stale task lineage on non-author comment edits (MUL-4857)

An admin editing an autopilot Agent's comment previously preserved the
comment's original source_task_id. The immediate save is judged on the
admin's member identity and correctly fails closed, but the deferred
completion-reconcile routes the comment under its original agent author
and resolved the delegation authority from the stale source_task_id,
resurrecting the autopilot creator's invoke authority once the busy
target freed up — an admin (manage rights) could thereby trigger another
owner's private agent (invoke rights).

Now a content edit re-derives lineage from the edit action: only the
agent author editing its own comment re-stamps source_task_id to the
current editing task; every other editor (member/admin, or any
non-author) clears it, so preview, save, and reconcile all fail closed.

Adds a regression covering the admin-edit + busy-target path and syncs
the multica-mentioning skill docs.

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

---------

Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-07-16 15:23:56 +08:00

118 lines
3.9 KiB
Go

package handler
import (
"context"
"fmt"
"testing"
"github.com/jackc/pgx/v5/pgtype"
"github.com/multica-ai/multica/server/internal/util"
db "github.com/multica-ai/multica/server/pkg/db/generated"
)
// Helper to build a pgtype.UUID from a string.
func testUUID(s string) pgtype.UUID {
return parseUUID(s)
}
// Helper to build a pgtype.Text.
func testText(s string) pgtype.Text {
return pgtype.Text{String: s, Valid: true}
}
const (
agentAssigneeID = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
otherAgentID = "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"
memberID = "cccccccc-cccc-cccc-cccc-cccccccccccc"
otherMemberID = "dddddddd-dddd-dddd-dddd-dddddddddddd"
)
func issueWithAgentAssignee() db.Issue {
return db.Issue{
AssigneeType: testText("agent"),
AssigneeID: testUUID(agentAssigneeID),
}
}
func issueNoAssignee() db.Issue {
return db.Issue{}
}
func TestHasAgentOrSquadMention(t *testing.T) {
tests := []struct {
name string
content string
want bool
}{
{"plain", "just a plain comment", false},
{"agent", fmt.Sprintf("[@Agent](mention://agent/%s) please fix", agentAssigneeID), true},
{"squad", "[@Squad](mention://squad/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa) please coordinate", true},
{"member only", fmt.Sprintf("[@Bob](mention://member/%s) take a look", memberID), false},
{"issue only", "[PAN-1](mention://issue/44c266e7-f6dd-4be3-9140-5ac40233f79c) is related", false},
{"all only", "[@all](mention://all/all) heads up", false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := hasAgentOrSquadMention(parseMentionsForTest(tt.content))
if got != tt.want {
t.Errorf("hasAgentOrSquadMention() = %v, want %v", got, tt.want)
}
})
}
}
func parseMentionsForTest(content string) []util.Mention {
return util.ParseMentions(content)
}
// -------------------------------------------------------------------
// isNoteComment — the /note opt-out prefix
// -------------------------------------------------------------------
func TestIsNoteComment(t *testing.T) {
tests := []struct {
name string
content string
want bool
}{
{"plain comment triggers", "just a plain comment", false},
{"note prefix skips", "/note check the API expiry", true},
{"bare note skips", "/note", true},
{"uppercase note skips (case-insensitive)", "/NOTE shout", true},
{"mixed case note skips", "/Note mixed", true},
{"leading whitespace tolerated", " /note leading space", true},
{"note followed by newline skips", "/note\nmultiline body", true},
{"plural notes does not match (word boundary)", "/notes are plural", false},
{"noteworthy does not match", "/noteworthy idea", false},
{"slash space note does not match", "/ note has a space", false},
{"mid-sentence note does not match", "see foo/note here", false},
{"note as second token does not match", "fyi /note", false},
{"empty content does not match", "", false},
{"whitespace-only content does not match", " ", false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := isNoteComment(tt.content); got != tt.want {
t.Errorf("isNoteComment(%q) = %v, want %v", tt.content, got, tt.want)
}
})
}
}
// TestTriggerTasksForComment_NoteShortCircuits proves a /note comment returns
// before the cascade tries to resolve mentions, parent ownership, or assignee
// fallback. A nil-Queries Handler would panic if the /note guard were missing or
// moved below those branches.
func TestTriggerTasksForComment_NoteShortCircuits(t *testing.T) {
h := &Handler{} // nil Queries / TaskService on purpose
issue := issueWithAgentAssignee()
comment := db.Comment{
Content: fmt.Sprintf("/note cc [@Other](mention://agent/%s) just an fyi", otherAgentID),
}
// Must not panic — the guard short-circuits before any DB access.
h.triggerTasksForComment(context.Background(), issue, comment, nil, "member", memberID, memberID, "", nil)
}