mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-03 19:20:07 +02:00
fix(cli): resolve assignee by exact name or ShortID to avoid substring collisions (#1642)
`multica issue assign --to <name>` 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
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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{
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user