diff --git a/cmd/bd/cli_fast_test.go b/cmd/bd/cli_fast_test.go index a32baa3f..5a64a930 100644 --- a/cmd/bd/cli_fast_test.go +++ b/cmd/bd/cli_fast_test.go @@ -972,4 +972,128 @@ func TestCLI_CreateDryRun(t *testing.T) { }) } +// TestCLI_CommentsAddShortID tests that 'comments add' accepts short IDs (issue #1070) +// Most bd commands accept short IDs (e.g., "5wbm") but comments add previously required +// full IDs (e.g., "mike.vibe-coding-5wbm"). This test ensures short IDs work. +// +// Note: This test runs with --no-daemon (direct mode) where short IDs already work +// because the code calls utils.ResolvePartialID(). The actual bug (GitHub #1070) is +// in daemon mode where the ID isn't resolved before being sent to the RPC server. +// The fix should add daemonClient.ResolveID() before daemonClient.AddComment(), +// following the pattern in update.go and label.go. +func TestCLI_CommentsAddShortID(t *testing.T) { + if testing.Short() { + t.Skip("skipping slow CLI test in short mode") + } + + t.Run("ShortIDWithCommentsAdd", func(t *testing.T) { + tmpDir := setupCLITestDB(t) + + // Create an issue and get its full ID + out := runBDInProcess(t, tmpDir, "create", "Issue for comment test", "-p", "1", "--json") + + jsonStart := strings.Index(out, "{") + if jsonStart < 0 { + t.Fatalf("No JSON found in output: %s", out) + } + jsonOut := out[jsonStart:] + + var issue map[string]interface{} + if err := json.Unmarshal([]byte(jsonOut), &issue); err != nil { + t.Fatalf("Failed to parse JSON: %v\nOutput: %s", err, jsonOut) + } + + fullID := issue["id"].(string) + t.Logf("Created issue with full ID: %s", fullID) + + // Extract short ID (the part after the last hyphen in prefix-hash format) + // For IDs like "test-abc123", the short ID is "abc123" + parts := strings.Split(fullID, "-") + if len(parts) < 2 { + t.Fatalf("Unexpected ID format: %s", fullID) + } + shortID := parts[len(parts)-1] + t.Logf("Using short ID: %s", shortID) + + // Add a comment using the SHORT ID (not full ID) + stdout, stderr, err := runBDInProcessAllowError(t, tmpDir, "comments", "add", shortID, "Test comment with short ID") + if err != nil { + t.Fatalf("comments add failed: %v\nstdout: %s\nstderr: %s", err, stdout, stderr) + } + + if !strings.Contains(stdout, "Comment added") { + t.Errorf("Expected 'Comment added' in output, got: %s", stdout) + } + + // Verify the comment was actually added by listing comments (use full ID for list) + stdout, stderr, err = runBDInProcessAllowError(t, tmpDir, "comments", fullID) + if err != nil { + t.Fatalf("comments list failed: %v\nstdout: %s\nstderr: %s", err, stdout, stderr) + } + + if !strings.Contains(stdout, "Test comment with short ID") { + t.Errorf("Expected comment text in list output, got: %s", stdout) + } + }) + + t.Run("PartialIDWithCommentsAdd", func(t *testing.T) { + tmpDir := setupCLITestDB(t) + + // Create an issue + out := runBDInProcess(t, tmpDir, "create", "Issue for partial ID test", "-p", "1", "--json") + + jsonStart := strings.Index(out, "{") + jsonOut := out[jsonStart:] + + var issue map[string]interface{} + json.Unmarshal([]byte(jsonOut), &issue) + fullID := issue["id"].(string) + + // Extract short ID and use only first 4 characters (partial match) + parts := strings.Split(fullID, "-") + shortID := parts[len(parts)-1] + if len(shortID) > 4 { + shortID = shortID[:4] // Use only first 4 chars for partial match + } + t.Logf("Full ID: %s, Partial ID: %s", fullID, shortID) + + // Add comment using partial ID + stdout, stderr, err := runBDInProcessAllowError(t, tmpDir, "comments", "add", shortID, "Comment via partial ID") + if err != nil { + t.Fatalf("comments add with partial ID failed: %v\nstdout: %s\nstderr: %s", err, stdout, stderr) + } + + if !strings.Contains(stdout, "Comment added") { + t.Errorf("Expected 'Comment added' in output, got: %s", stdout) + } + }) + + t.Run("CommentAliasWithShortID", func(t *testing.T) { + tmpDir := setupCLITestDB(t) + + // Create an issue + out := runBDInProcess(t, tmpDir, "create", "Issue for alias test", "-p", "1", "--json") + + jsonStart := strings.Index(out, "{") + jsonOut := out[jsonStart:] + + var issue map[string]interface{} + json.Unmarshal([]byte(jsonOut), &issue) + fullID := issue["id"].(string) + + // Extract short ID + parts := strings.Split(fullID, "-") + shortID := parts[len(parts)-1] + + // Use the 'comment' alias (deprecated but should still work) + stdout, stderr, err := runBDInProcessAllowError(t, tmpDir, "comment", shortID, "Comment via alias with short ID") + if err != nil { + t.Fatalf("comment alias failed: %v\nstdout: %s\nstderr: %s", err, stdout, stderr) + } + + if !strings.Contains(stdout, "Comment added") { + t.Errorf("Expected 'Comment added' in output, got: %s", stdout) + } + }) +} diff --git a/cmd/bd/comments.go b/cmd/bd/comments.go index 655c7f90..82d80997 100644 --- a/cmd/bd/comments.go +++ b/cmd/bd/comments.go @@ -38,6 +38,18 @@ Examples: comments := make([]*types.Comment, 0) usedDaemon := false if daemonClient != nil { + // Resolve short/partial ID to full ID before sending to daemon (#1070) + resolveArgs := &rpc.ResolveIDArgs{ID: issueID} + resolveResp, err := daemonClient.ResolveID(resolveArgs) + if err != nil { + FatalErrorRespectJSON("resolving ID %s: %v", issueID, err) + } + var resolvedID string + if err := json.Unmarshal(resolveResp.Data, &resolvedID); err != nil { + FatalErrorRespectJSON("unmarshaling resolved ID: %v", err) + } + issueID = resolvedID + resp, err := daemonClient.ListComments(&rpc.CommentListArgs{ID: issueID}) if err != nil { if isUnknownOperationError(err) { @@ -145,6 +157,18 @@ Examples: var comment *types.Comment if daemonClient != nil { + // Resolve short/partial ID to full ID before sending to daemon (#1070) + resolveArgs := &rpc.ResolveIDArgs{ID: issueID} + resolveResp, err := daemonClient.ResolveID(resolveArgs) + if err != nil { + FatalErrorRespectJSON("resolving ID %s: %v", issueID, err) + } + var resolvedID string + if err := json.Unmarshal(resolveResp.Data, &resolvedID); err != nil { + FatalErrorRespectJSON("unmarshaling resolved ID: %v", err) + } + issueID = resolvedID + resp, err := daemonClient.AddComment(&rpc.CommentAddArgs{ ID: issueID, Author: author, diff --git a/internal/rpc/comments_test.go b/internal/rpc/comments_test.go index 6edb008c..2426f3d0 100644 --- a/internal/rpc/comments_test.go +++ b/internal/rpc/comments_test.go @@ -69,3 +69,152 @@ func TestCommentOperationsViaRPC(t *testing.T) { t.Fatalf("expected comment text 'first comment', got %q", comments[0].Text) } } + +// TestCommentAddWithResolvedID verifies that the RPC layer works correctly +// when the CLI resolves short IDs before sending to the daemon (issue #1070). +// +// Note: The RPC server expects full IDs. Short ID resolution happens in the CLI +// (cmd/bd/comments.go) before calling the daemon, following the pattern used by +// update.go, label.go, and other commands. This test simulates that workflow. +func TestCommentAddWithResolvedID(t *testing.T) { + if testing.Short() { + t.Skip("skipping slow RPC test in short mode") + } + _, client, cleanup := setupTestServer(t) + defer cleanup() + + // Create an issue + createResp, err := client.Create(&CreateArgs{ + Title: "Resolved ID comment test", + IssueType: "task", + Priority: 2, + }) + if err != nil { + t.Fatalf("create issue failed: %v", err) + } + + var created types.Issue + if err := json.Unmarshal(createResp.Data, &created); err != nil { + t.Fatalf("failed to decode create response: %v", err) + } + + fullID := created.ID + t.Logf("Created issue with full ID: %s", fullID) + + // Extract the short ID (hash portion after the prefix) + shortID := fullID + for i := len(fullID) - 1; i >= 0; i-- { + if fullID[i] == '-' { + shortID = fullID[i+1:] + break + } + } + t.Logf("Short ID: %s", shortID) + + // Simulate CLI behavior: resolve short ID first, then call RPC with full ID + // In the real CLI, this is done via daemonClient.ResolveID() + resolveResp, err := client.ResolveID(&ResolveIDArgs{ID: shortID}) + if err != nil { + t.Fatalf("resolve ID failed: %v", err) + } + var resolvedID string + if err := json.Unmarshal(resolveResp.Data, &resolvedID); err != nil { + t.Fatalf("failed to decode resolved ID: %v", err) + } + t.Logf("Resolved ID: %s", resolvedID) + + if resolvedID != fullID { + t.Fatalf("expected resolved ID %q, got %q", fullID, resolvedID) + } + + // Now add comment using the RESOLVED (full) ID + addResp, err := client.AddComment(&CommentAddArgs{ + ID: resolvedID, + Author: "tester", + Text: "comment via resolved ID", + }) + if err != nil { + t.Fatalf("add comment failed: %v", err) + } + + var added types.Comment + if err := json.Unmarshal(addResp.Data, &added); err != nil { + t.Fatalf("failed to decode add comment response: %v", err) + } + + if added.Text != "comment via resolved ID" { + t.Errorf("expected comment text 'comment via resolved ID', got %q", added.Text) + } +} + +// TestCommentListWithResolvedID verifies that ListComments works when the CLI +// resolves short IDs before sending to the daemon (issue #1070). +func TestCommentListWithResolvedID(t *testing.T) { + if testing.Short() { + t.Skip("skipping slow RPC test in short mode") + } + _, client, cleanup := setupTestServer(t) + defer cleanup() + + // Create an issue and add a comment + createResp, err := client.Create(&CreateArgs{ + Title: "Resolved ID list test", + IssueType: "task", + Priority: 2, + }) + if err != nil { + t.Fatalf("create issue failed: %v", err) + } + + var created types.Issue + if err := json.Unmarshal(createResp.Data, &created); err != nil { + t.Fatalf("failed to decode create response: %v", err) + } + + fullID := created.ID + + // Add a comment using full ID + _, err = client.AddComment(&CommentAddArgs{ + ID: fullID, + Author: "tester", + Text: "test comment for list", + }) + if err != nil { + t.Fatalf("add comment failed: %v", err) + } + + // Extract short ID + shortID := fullID + for i := len(fullID) - 1; i >= 0; i-- { + if fullID[i] == '-' { + shortID = fullID[i+1:] + break + } + } + t.Logf("Full ID: %s, Short ID: %s", fullID, shortID) + + // Simulate CLI behavior: resolve short ID first + resolveResp, err := client.ResolveID(&ResolveIDArgs{ID: shortID}) + if err != nil { + t.Fatalf("resolve ID failed: %v", err) + } + var resolvedID string + if err := json.Unmarshal(resolveResp.Data, &resolvedID); err != nil { + t.Fatalf("failed to decode resolved ID: %v", err) + } + + // List comments using the RESOLVED (full) ID + listResp, err := client.ListComments(&CommentListArgs{ID: resolvedID}) + if err != nil { + t.Fatalf("list comments failed: %v", err) + } + + var comments []*types.Comment + if err := json.Unmarshal(listResp.Data, &comments); err != nil { + t.Fatalf("failed to decode comment list: %v", err) + } + + if len(comments) != 1 { + t.Fatalf("expected 1 comment, got %d", len(comments)) + } +}