mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-05 21:39:54 +02:00
feat(cli)!: drop short UUID prefix resolution for multica issue (MUL-3838) (#4732)
BREAKING CHANGE: `multica issue <command> <ref>` no longer accepts short UUID prefixes (e.g. `1881abcd`). Pass the issue key shown by `multica issue list` (`MUL-123`) or the full UUID instead. Other resources without a human-readable key (autopilots, projects, labels, task runs, workspaces) continue to accept short UUID prefixes. The previous resolver paged the entire workspace issue list client-side to disambiguate a short prefix, which timed out on workspaces with ~1000 issues (14–35s; reported in GH #4701). Since the issue key (`MUL-123`) already covers every human use case for an issue reference and the full UUID covers every machine case, supporting a third identifier form has no real product value and forces every issue command to carry ambiguous / min-length / hex-validation semantics through the CLI. Rather than pushing the prefix resolver down into the server (with a new DB query and an expression / generated index), this change removes the path entirely. The user-facing migration is trivial: the `identifier` column shown by `multica issue list` is already routable. Changes: - `resolveIssueRef` now accepts only the issue key (`MUL-123`) or the full UUID. A short hex prefix returns a tailored error pointing to the supported forms; non-hex gibberish returns a generic guidance error. Neither path makes an HTTP call. - The unused `fetchIssueCandidates` paginator is removed. - Tests cover: full UUID succeeds via a single GET, identifier-first resolution does not list, and short prefix / dashed short prefix / bare numeric / non-hex inputs all fail fast with no HTTP traffic. Product rationale and the first-principles discussion are recorded on Multica issue MUL-3838. Co-authored-by: Eve <eve@multica-ai.local> Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user