From 60fdc828243c84cee8d2ac3f23000457cf7790bc Mon Sep 17 00:00:00 2001 From: Bohan Jiang <52446949+Bohan-J@users.noreply.github.com> Date: Sat, 25 Apr 2026 01:05:29 +0800 Subject: [PATCH] fix(cli): resolve assignee by exact name or ShortID to avoid substring collisions (#1642) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `multica issue assign --to ` matched agent/member names with a plain `strings.Contains` check, so an exact match on `reviewer` became ambiguous whenever a longer agent like `peer-reviewer` also existed. There was also no way to disambiguate by ID. Rework `resolveAssignee` to bucket candidates by priority: 1. Full UUID or 8-char ShortID (matches `truncateID` output) — case-insensitive. 2. Case-insensitive exact name (with surrounding whitespace trimmed). 3. Substring fallback — preserves the existing partial-name UX. The first non-empty bucket wins. Ambiguity inside a higher-priority bucket still errors and short-circuits lower-priority matching. All six call sites (`issue assign/update/create/list`, `issue subscriber`, `project`) are fixed by this single change. Fixes #1620 --- server/cmd/multica/cmd_issue.go | 75 +++++++++------ server/cmd/multica/cmd_issue_test.go | 134 +++++++++++++++++++++++++++ 2 files changed, 181 insertions(+), 28 deletions(-) diff --git a/server/cmd/multica/cmd_issue.go b/server/cmd/multica/cmd_issue.go index 99b44497c5..6c3d3911bf 100644 --- a/server/cmd/multica/cmd_issue.go +++ b/server/cmd/multica/cmd_issue.go @@ -1104,24 +1104,43 @@ func resolveAssignee(ctx context.Context, client *cli.APIClient, name string) (s return "", "", fmt.Errorf("workspace ID is required to resolve assignees; use --workspace-id or set MULTICA_WORKSPACE_ID") } - nameLower := strings.ToLower(name) - var matches []assigneeMatch + input := strings.TrimSpace(name) + if input == "" { + return "", "", fmt.Errorf("no member or agent found matching %q", name) + } + inputLower := strings.ToLower(input) + + // Matches are collected into three priority buckets. Higher-priority buckets + // short-circuit lower-priority matching so that, e.g., an exact name match + // always wins over a substring collision with another candidate. + // 1. idMatches — full UUID or 8-char ShortID (as shown by `truncateID`). + // 2. exactMatches — case-insensitive full name equality. + // 3. substringMatches — preserves the existing partial-name UX. + var idMatches, exactMatches, substringMatches []assigneeMatch var errs []error + classify := func(entityType, id, displayName string) { + match := assigneeMatch{Type: entityType, ID: id, Name: displayName} + if id != "" && (strings.EqualFold(id, input) || strings.EqualFold(truncateID(id), input)) { + idMatches = append(idMatches, match) + return + } + if strings.EqualFold(displayName, input) { + exactMatches = append(exactMatches, match) + return + } + if strings.Contains(strings.ToLower(displayName), inputLower) { + substringMatches = append(substringMatches, match) + } + } + // Search members. var members []map[string]any if err := client.GetJSON(ctx, "/api/workspaces/"+client.WorkspaceID+"/members", &members); err != nil { errs = append(errs, fmt.Errorf("fetch members: %w", err)) } else { for _, m := range members { - mName := strVal(m, "name") - if strings.Contains(strings.ToLower(mName), nameLower) { - matches = append(matches, assigneeMatch{ - Type: "member", - ID: strVal(m, "user_id"), - Name: mName, - }) - } + classify("member", strVal(m, "user_id"), strVal(m, "name")) } } @@ -1132,14 +1151,7 @@ func resolveAssignee(ctx context.Context, client *cli.APIClient, name string) (s errs = append(errs, fmt.Errorf("fetch agents: %w", err)) } else { for _, a := range agents { - aName := strVal(a, "name") - if strings.Contains(strings.ToLower(aName), nameLower) { - matches = append(matches, assigneeMatch{ - Type: "agent", - ID: strVal(a, "id"), - Name: aName, - }) - } + classify("agent", strVal(a, "id"), strVal(a, "name")) } } @@ -1148,18 +1160,25 @@ func resolveAssignee(ctx context.Context, client *cli.APIClient, name string) (s return "", "", fmt.Errorf("failed to resolve assignee: %v; %v", errs[0], errs[1]) } - switch len(matches) { - case 0: - return "", "", fmt.Errorf("no member or agent found matching %q", name) - case 1: - return matches[0].Type, matches[0].ID, nil - default: - var parts []string - for _, m := range matches { - parts = append(parts, fmt.Sprintf(" %s %q (%s)", m.Type, m.Name, truncateID(m.ID))) + for _, bucket := range [][]assigneeMatch{idMatches, exactMatches, substringMatches} { + switch len(bucket) { + case 0: + continue + case 1: + return bucket[0].Type, bucket[0].ID, nil + default: + return "", "", ambiguousAssigneeError(input, bucket) } - return "", "", fmt.Errorf("ambiguous assignee %q; matches:\n%s", name, strings.Join(parts, "\n")) } + return "", "", fmt.Errorf("no member or agent found matching %q", input) +} + +func ambiguousAssigneeError(input string, matches []assigneeMatch) error { + parts := make([]string, 0, len(matches)) + for _, m := range matches { + parts = append(parts, fmt.Sprintf(" %s %q (%s)", m.Type, m.Name, truncateID(m.ID))) + } + return fmt.Errorf("ambiguous assignee %q; matches:\n%s", input, strings.Join(parts, "\n")) } func formatAssignee(issue map[string]any) string { diff --git a/server/cmd/multica/cmd_issue_test.go b/server/cmd/multica/cmd_issue_test.go index 13b3792f5a..e0535d08c1 100644 --- a/server/cmd/multica/cmd_issue_test.go +++ b/server/cmd/multica/cmd_issue_test.go @@ -137,6 +137,140 @@ func TestResolveAssignee(t *testing.T) { }) } +// TestResolveAssigneeExactMatchWins covers the substring-collision scenario from +// multica-ai/multica#1620: when one name is a substring of another (e.g. +// "reviewer" vs "peer-reviewer"), an exact match on the shorter name must +// short-circuit substring matching instead of erroring out as ambiguous. +func TestResolveAssigneeExactMatchWins(t *testing.T) { + agentsResp := []map[string]any{ + {"id": "f656eab8-1111-1111-1111-111111111111", "name": "reviewer"}, + {"id": "9b0ff9a2-2222-2222-2222-222222222222", "name": "peer-reviewer"}, + } + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/workspaces/ws-1/members": + json.NewEncoder(w).Encode([]map[string]any{}) + case "/api/agents": + json.NewEncoder(w).Encode(agentsResp) + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + + client := cli.NewAPIClient(srv.URL, "ws-1", "test-token") + ctx := context.Background() + + t.Run("exact shorter name resolves to shorter agent", func(t *testing.T) { + aType, aID, err := resolveAssignee(ctx, client, "reviewer") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if aType != "agent" || aID != "f656eab8-1111-1111-1111-111111111111" { + t.Errorf("got (%q, %q), want (agent, f656eab8-...)", aType, aID) + } + }) + + t.Run("exact longer name still resolves unambiguously", func(t *testing.T) { + aType, aID, err := resolveAssignee(ctx, client, "peer-reviewer") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if aType != "agent" || aID != "9b0ff9a2-2222-2222-2222-222222222222" { + t.Errorf("got (%q, %q), want (agent, 9b0ff9a2-...)", aType, aID) + } + }) + + t.Run("exact match is case-insensitive and tolerates whitespace", func(t *testing.T) { + aType, aID, err := resolveAssignee(ctx, client, " Reviewer ") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if aType != "agent" || aID != "f656eab8-1111-1111-1111-111111111111" { + t.Errorf("got (%q, %q), want exact reviewer agent", aType, aID) + } + }) + + t.Run("substring-only input falls back and stays ambiguous", func(t *testing.T) { + // "review" matches both agents via substring and neither via exact name, + // so the existing ambiguity error is preserved. + _, _, err := resolveAssignee(ctx, client, "review") + if err == nil { + t.Fatal("expected error for ambiguous substring match") + } + if got := err.Error(); !strings.Contains(got, "ambiguous") { + t.Errorf("expected ambiguous error, got: %s", got) + } + }) +} + +// TestResolveAssigneeByID covers the ID/ShortID escape hatch from +// multica-ai/multica#1620: passing a full UUID or its 8-char prefix must +// resolve directly without going through name matching. +func TestResolveAssigneeByID(t *testing.T) { + membersResp := []map[string]any{ + {"user_id": "aaaaaaaa-1111-1111-1111-111111111111", "name": "Alice"}, + } + agentsResp := []map[string]any{ + {"id": "f656eab8-1111-1111-1111-111111111111", "name": "reviewer"}, + {"id": "9b0ff9a2-2222-2222-2222-222222222222", "name": "peer-reviewer"}, + } + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/workspaces/ws-1/members": + json.NewEncoder(w).Encode(membersResp) + case "/api/agents": + json.NewEncoder(w).Encode(agentsResp) + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + + client := cli.NewAPIClient(srv.URL, "ws-1", "test-token") + ctx := context.Background() + + t.Run("full UUID resolves agent", func(t *testing.T) { + aType, aID, err := resolveAssignee(ctx, client, "f656eab8-1111-1111-1111-111111111111") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if aType != "agent" || aID != "f656eab8-1111-1111-1111-111111111111" { + t.Errorf("got (%q, %q), want reviewer agent", aType, aID) + } + }) + + t.Run("8-char ShortID resolves agent", func(t *testing.T) { + aType, aID, err := resolveAssignee(ctx, client, "f656eab8") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if aType != "agent" || aID != "f656eab8-1111-1111-1111-111111111111" { + t.Errorf("got (%q, %q), want reviewer agent", aType, aID) + } + }) + + t.Run("uppercase ShortID still resolves", func(t *testing.T) { + aType, aID, err := resolveAssignee(ctx, client, "F656EAB8") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if aType != "agent" || aID != "f656eab8-1111-1111-1111-111111111111" { + t.Errorf("got (%q, %q), want reviewer agent", aType, aID) + } + }) + + t.Run("ShortID resolves a member", func(t *testing.T) { + aType, aID, err := resolveAssignee(ctx, client, "aaaaaaaa") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if aType != "member" || aID != "aaaaaaaa-1111-1111-1111-111111111111" { + t.Errorf("got (%q, %q), want Alice", aType, aID) + } + }) +} + func TestIssueSubscriberList(t *testing.T) { subscribersResp := []map[string]any{ {