diff --git a/server/cmd/multica/cmd_id_resolver.go b/server/cmd/multica/cmd_id_resolver.go index 14ed2082e..28a16e636 100644 --- a/server/cmd/multica/cmd_id_resolver.go +++ b/server/cmd/multica/cmd_id_resolver.go @@ -127,22 +127,48 @@ func ambiguousIDPrefixError(kind, input string, matches []idCandidate) error { return fmt.Errorf("ambiguous %s id prefix %q; matches:\n%s\nUse more characters or run the list command with --full-id", kind, input, strings.Join(parts, "\n")) } +// resolveIssueRef accepts only the two canonical issue references: +// +// - the human-facing issue key, e.g. "MUL-1852" (validated by +// looksLikeIssueIdentifier and resolved server-side); +// - the full UUID in dashed canonical form (validated by uuidRegexp). +// +// Short UUID prefixes (e.g. "1881abcd") were briefly supported but are no +// longer — on large workspaces the CLI had to page the entire issue list +// client-side to disambiguate, causing 14–35s timeouts (GH #4701). Since +// `MUL-123` already covers every human use case for an issue reference, the +// short-prefix path is removed instead of being moved server-side. Other +// resources without a human-readable key (autopilots, projects, labels, +// task runs, workspaces, ...) continue to accept short UUID prefixes; see +// resolveIDByPrefix. func resolveIssueRef(ctx context.Context, client *cli.APIClient, input string) (resolvedID, error) { trimmed := strings.TrimSpace(input) if trimmed == "" { return resolvedID{}, fmt.Errorf("issue id is required") } - // Preserve issue-key semantics before considering UUID prefixes. This - // mirrors the server-side loadIssueForUser order and avoids treating - // strings like MUL-1852 as a UUID prefix. if looksLikeIssueIdentifier(trimmed) { return fetchIssueRef(ctx, client, trimmed) } if uuidRegexp.MatchString(trimmed) { return fetchIssueRef(ctx, client, trimmed) } - return resolveIDByPrefix(ctx, client, "issue", trimmed, fetchIssueCandidates) + + // Detect the common "I copied a truncated UUID" case and give a + // tailored hint. normalizeUUIDPrefix succeeds for any input that is + // ≥4 hex chars (after stripping dashes), which matches what the old + // resolver used to accept as a prefix. + if _, err := normalizeUUIDPrefix(trimmed); err == nil { + return resolvedID{}, fmt.Errorf( + "issue ref %q looks like a short UUID prefix; short prefixes are no longer supported for issues. "+ + "Use the issue key (e.g. MUL-123) shown by `multica issue list`, or pass the full UUID (run a list command with --full-id to copy it)", + input, + ) + } + return resolvedID{}, fmt.Errorf( + "issue ref %q is not a recognized issue reference; use the issue key (e.g. MUL-123) shown by `multica issue list`, or pass the full UUID", + input, + ) } func fetchIssueRef(ctx context.Context, client *cli.APIClient, ref string) (resolvedID, error) { @@ -183,41 +209,6 @@ func parsePositiveInt(input string) (int, bool) { return n, true } -func fetchIssueCandidates(ctx context.Context, client *cli.APIClient) ([]idCandidate, error) { - if client.WorkspaceID == "" { - return nil, fmt.Errorf("workspace_id is required to resolve issue id prefixes") - } - const limit = resolverListPageLimit - candidates := []idCandidate{} - for offset := 0; ; { - params := url.Values{} - params.Set("workspace_id", client.WorkspaceID) - params.Set("include_closed", "true") - params.Set("limit", strconv.Itoa(limit)) - if offset > 0 { - params.Set("offset", strconv.Itoa(offset)) - } - var result map[string]any - if err := client.GetJSON(ctx, "/api/issues?"+params.Encode(), &result); err != nil { - return nil, err - } - issuesRaw, _ := result["issues"].([]any) - for _, raw := range issuesRaw { - issue, ok := raw.(map[string]any) - if !ok { - continue - } - candidates = append(candidates, issueCandidate(issue)) - } - offset += len(issuesRaw) - total, _ := result["total"].(float64) - if len(issuesRaw) == 0 || (total > 0 && offset >= int(total)) || (total == 0 && len(issuesRaw) < limit) { - break - } - } - return candidates, nil -} - func resolveAutopilotID(ctx context.Context, client *cli.APIClient, input string) (resolvedID, error) { return resolveIDByPrefix(ctx, client, "autopilot", input, fetchAutopilotCandidates) } diff --git a/server/cmd/multica/cmd_issue_test.go b/server/cmd/multica/cmd_issue_test.go index 8de2af006..467e07b69 100644 --- a/server/cmd/multica/cmd_issue_test.go +++ b/server/cmd/multica/cmd_issue_test.go @@ -708,62 +708,121 @@ func TestResolveIssueRef(t *testing.T) { } }) - t.Run("short UUID prefix resolves from workspace issue list", func(t *testing.T) { + t.Run("short UUID prefix is rejected without any HTTP call", func(t *testing.T) { + // Until commit 9a3a99c this called fetchIssueCandidates and paged + // /api/issues client-side, which timed out on large workspaces + // (GH #4701). The resolver now refuses short prefixes outright + // and the test pins that contract: any HTTP call here means the + // removal regressed. + var hits []string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path != "/api/issues" { - http.NotFound(w, r) - return - } - if got := r.URL.Query().Get("workspace_id"); got != "ws-1" { - t.Errorf("workspace_id = %q, want ws-1", got) - } - if got := r.URL.Query().Get("include_closed"); got != "true" { - t.Errorf("include_closed = %q, want true", got) - } - if got := r.URL.Query().Get("limit"); got != strconv.Itoa(resolverListPageLimit) { - t.Errorf("limit = %q, want %d", got, resolverListPageLimit) - } - json.NewEncoder(w).Encode(map[string]any{ - "issues": []map[string]any{issue}, - "total": 1, - }) + hits = append(hits, r.Method+" "+r.URL.Path) + http.NotFound(w, r) })) defer srv.Close() client := cli.NewAPIClient(srv.URL, "ws-1", "test-token") - got, err := resolveIssueRef(context.Background(), client, "1881") - if err != nil { - t.Fatalf("unexpected error: %v", err) + _, err := resolveIssueRef(context.Background(), client, "1881") + if err == nil { + t.Fatal("expected short UUID prefix to be rejected") } - if got.ID != issue["id"] || got.Display != "MUL-1852" { - t.Fatalf("got %#v", got) + if msg := err.Error(); !strings.Contains(msg, "short UUID prefix") { + t.Fatalf("expected error to flag the short-prefix case, got: %s", msg) + } + if msg := err.Error(); !strings.Contains(msg, "MUL-") { + t.Fatalf("expected error to suggest the issue key form, got: %s", msg) + } + if len(hits) != 0 { + t.Fatalf("short prefix must not perform any HTTP call; got %#v", hits) } }) - t.Run("bare issue number is not resolved as issue number", func(t *testing.T) { + t.Run("short UUID prefix with dashes is rejected", func(t *testing.T) { + // "1881-a167" used to be accepted as a dashed short prefix; make + // sure dashes do not slip past the new rejection. srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path != "/api/issues" { - http.NotFound(w, r) - return - } - json.NewEncoder(w).Encode(map[string]any{ - "issues": []map[string]any{{ - "id": "aaaaaaaa-4bb6-4602-944b-f40ce4192fe6", - "identifier": "MUL-1852", - "title": "Should not resolve by number", - }}, - "total": 1, - }) + t.Errorf("unexpected HTTP call: %s %s", r.Method, r.URL.Path) + http.NotFound(w, r) + })) + defer srv.Close() + + client := cli.NewAPIClient(srv.URL, "ws-1", "test-token") + _, err := resolveIssueRef(context.Background(), client, "1881-a167") + if err == nil { + t.Fatal("expected dashed short prefix to be rejected") + } + if msg := err.Error(); !strings.Contains(msg, "short UUID prefix") { + t.Fatalf("expected short-prefix error, got: %s", msg) + } + }) + + t.Run("bare numeric input is rejected as a short prefix", func(t *testing.T) { + // A 4+ digit string is still pure hex, so the resolver classifies + // it as a short UUID prefix and returns the tailored hint. This + // pins that we never accidentally treat "1852" as the bare issue + // number 1852. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected HTTP call: %s %s", r.Method, r.URL.Path) + http.NotFound(w, r) })) defer srv.Close() client := cli.NewAPIClient(srv.URL, "ws-1", "test-token") _, err := resolveIssueRef(context.Background(), client, "1852") if err == nil { - t.Fatal("expected bare number to be treated only as a UUID prefix") + t.Fatal("expected bare number to be rejected") } - if got := err.Error(); !strings.Contains(got, "id prefix") { - t.Fatalf("expected prefix error, got: %s", got) + if msg := err.Error(); !strings.Contains(msg, "short UUID prefix") { + t.Fatalf("expected short-prefix error, got: %s", msg) + } + }) + + t.Run("non-hex gibberish is rejected with key/UUID guidance", func(t *testing.T) { + // Inputs that are neither MUL-key, full UUID, nor a hex prefix + // fall through to the generic guidance path and must not hit the + // network either. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected HTTP call: %s %s", r.Method, r.URL.Path) + http.NotFound(w, r) + })) + defer srv.Close() + + client := cli.NewAPIClient(srv.URL, "ws-1", "test-token") + _, err := resolveIssueRef(context.Background(), client, "not-an-id") + if err == nil { + t.Fatal("expected gibberish input to be rejected") + } + if msg := err.Error(); strings.Contains(msg, "short UUID prefix") { + t.Fatalf("non-hex input should not be classified as a short prefix; got: %s", msg) + } + if msg := err.Error(); !strings.Contains(msg, "not a recognized issue reference") { + t.Fatalf("expected generic guidance error, got: %s", msg) + } + }) + + t.Run("full UUID still resolves via single GET", func(t *testing.T) { + // Sanity check: the canonical reference forms must still work. + var hits []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hits = append(hits, r.Method+" "+r.URL.Path) + if r.URL.Path == "/api/issues/"+issue["id"].(string) { + json.NewEncoder(w).Encode(issue) + return + } + http.NotFound(w, r) + })) + defer srv.Close() + + client := cli.NewAPIClient(srv.URL, "ws-1", "test-token") + got, err := resolveIssueRef(context.Background(), client, issue["id"].(string)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got.ID != issue["id"] || got.Display != "MUL-1852" { + t.Fatalf("got %#v", got) + } + if len(hits) != 1 { + t.Fatalf("full UUID must resolve via a single request; got %#v", hits) } }) }