fix: resolve short IDs in comments add/list daemon mode (#1070)
Add daemonClient.ResolveID() calls before AddComment and ListComments operations in daemon mode, following the pattern from update.go. Previously, short IDs (e.g., "5wbm") worked with most bd commands but failed with `comments add` and `comments list` when using the daemon. The short ID was passed directly to the RPC server which expected full IDs (e.g., "prefix-5wbm"). Changes: - cmd/bd/comments.go: Add ID resolution before daemon RPC calls - internal/rpc/comments_test.go: Update tests to reflect client-side resolution pattern (RPC server expects full IDs, CLI resolves first) Fixes: https://github.com/steveyegge/beads/issues/1070
This commit is contained in:
@@ -38,6 +38,18 @@ Examples:
|
|||||||
comments := make([]*types.Comment, 0)
|
comments := make([]*types.Comment, 0)
|
||||||
usedDaemon := false
|
usedDaemon := false
|
||||||
if daemonClient != nil {
|
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})
|
resp, err := daemonClient.ListComments(&rpc.CommentListArgs{ID: issueID})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if isUnknownOperationError(err) {
|
if isUnknownOperationError(err) {
|
||||||
@@ -145,6 +157,18 @@ Examples:
|
|||||||
|
|
||||||
var comment *types.Comment
|
var comment *types.Comment
|
||||||
if daemonClient != nil {
|
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{
|
resp, err := daemonClient.AddComment(&rpc.CommentAddArgs{
|
||||||
ID: issueID,
|
ID: issueID,
|
||||||
Author: author,
|
Author: author,
|
||||||
|
|||||||
@@ -70,12 +70,13 @@ func TestCommentOperationsViaRPC(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// TestCommentAddWithShortID tests that AddComment accepts short IDs (issue #1070).
|
// TestCommentAddWithResolvedID verifies that the RPC layer works correctly
|
||||||
// Currently, the RPC layer requires full IDs. The fix should resolve short IDs
|
// when the CLI resolves short IDs before sending to the daemon (issue #1070).
|
||||||
// to full IDs before performing operations.
|
|
||||||
//
|
//
|
||||||
// This test is expected to FAIL until the bug is fixed.
|
// Note: The RPC server expects full IDs. Short ID resolution happens in the CLI
|
||||||
func TestCommentAddWithShortID(t *testing.T) {
|
// (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() {
|
if testing.Short() {
|
||||||
t.Skip("skipping slow RPC test in short mode")
|
t.Skip("skipping slow RPC test in short mode")
|
||||||
}
|
}
|
||||||
@@ -84,7 +85,7 @@ func TestCommentAddWithShortID(t *testing.T) {
|
|||||||
|
|
||||||
// Create an issue
|
// Create an issue
|
||||||
createResp, err := client.Create(&CreateArgs{
|
createResp, err := client.Create(&CreateArgs{
|
||||||
Title: "Short ID comment test",
|
Title: "Resolved ID comment test",
|
||||||
IssueType: "task",
|
IssueType: "task",
|
||||||
Priority: 2,
|
Priority: 2,
|
||||||
})
|
})
|
||||||
@@ -101,7 +102,6 @@ func TestCommentAddWithShortID(t *testing.T) {
|
|||||||
t.Logf("Created issue with full ID: %s", fullID)
|
t.Logf("Created issue with full ID: %s", fullID)
|
||||||
|
|
||||||
// Extract the short ID (hash portion after the prefix)
|
// Extract the short ID (hash portion after the prefix)
|
||||||
// For IDs like "test-abc123", the short ID is "abc123"
|
|
||||||
shortID := fullID
|
shortID := fullID
|
||||||
for i := len(fullID) - 1; i >= 0; i-- {
|
for i := len(fullID) - 1; i >= 0; i-- {
|
||||||
if fullID[i] == '-' {
|
if fullID[i] == '-' {
|
||||||
@@ -109,17 +109,32 @@ func TestCommentAddWithShortID(t *testing.T) {
|
|||||||
break
|
break
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
t.Logf("Using short ID: %s", shortID)
|
t.Logf("Short ID: %s", shortID)
|
||||||
|
|
||||||
// Try to add a comment using the SHORT ID (not full ID)
|
// Simulate CLI behavior: resolve short ID first, then call RPC with full ID
|
||||||
// This should work once the bug is fixed
|
// 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{
|
addResp, err := client.AddComment(&CommentAddArgs{
|
||||||
ID: shortID, // Using short ID instead of full ID
|
ID: resolvedID,
|
||||||
Author: "tester",
|
Author: "tester",
|
||||||
Text: "comment via short ID",
|
Text: "comment via resolved ID",
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("add comment with short ID failed: %v (this is the bug - short IDs should work)", err)
|
t.Fatalf("add comment failed: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
var added types.Comment
|
var added types.Comment
|
||||||
@@ -127,29 +142,14 @@ func TestCommentAddWithShortID(t *testing.T) {
|
|||||||
t.Fatalf("failed to decode add comment response: %v", err)
|
t.Fatalf("failed to decode add comment response: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
if added.Text != "comment via short ID" {
|
if added.Text != "comment via resolved ID" {
|
||||||
t.Errorf("expected comment text 'comment via short ID', got %q", added.Text)
|
t.Errorf("expected comment text 'comment via resolved ID', got %q", added.Text)
|
||||||
}
|
|
||||||
|
|
||||||
// Verify by listing comments using the full ID
|
|
||||||
listResp, err := client.ListComments(&CommentListArgs{ID: fullID})
|
|
||||||
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))
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// TestCommentListWithShortID tests that ListComments accepts short IDs (issue #1070).
|
// TestCommentListWithResolvedID verifies that ListComments works when the CLI
|
||||||
// This test is expected to FAIL until the bug is fixed.
|
// resolves short IDs before sending to the daemon (issue #1070).
|
||||||
func TestCommentListWithShortID(t *testing.T) {
|
func TestCommentListWithResolvedID(t *testing.T) {
|
||||||
if testing.Short() {
|
if testing.Short() {
|
||||||
t.Skip("skipping slow RPC test in short mode")
|
t.Skip("skipping slow RPC test in short mode")
|
||||||
}
|
}
|
||||||
@@ -158,7 +158,7 @@ func TestCommentListWithShortID(t *testing.T) {
|
|||||||
|
|
||||||
// Create an issue and add a comment
|
// Create an issue and add a comment
|
||||||
createResp, err := client.Create(&CreateArgs{
|
createResp, err := client.Create(&CreateArgs{
|
||||||
Title: "Short ID list test",
|
Title: "Resolved ID list test",
|
||||||
IssueType: "task",
|
IssueType: "task",
|
||||||
Priority: 2,
|
Priority: 2,
|
||||||
})
|
})
|
||||||
@@ -193,11 +193,20 @@ func TestCommentListWithShortID(t *testing.T) {
|
|||||||
}
|
}
|
||||||
t.Logf("Full ID: %s, Short ID: %s", fullID, shortID)
|
t.Logf("Full ID: %s, Short ID: %s", fullID, shortID)
|
||||||
|
|
||||||
// Try to list comments using the SHORT ID
|
// Simulate CLI behavior: resolve short ID first
|
||||||
// This should work once the bug is fixed
|
resolveResp, err := client.ResolveID(&ResolveIDArgs{ID: shortID})
|
||||||
listResp, err := client.ListComments(&CommentListArgs{ID: shortID})
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("list comments with short ID failed: %v (this is the bug - short IDs should work)", err)
|
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
|
var comments []*types.Comment
|
||||||
@@ -206,6 +215,6 @@ func TestCommentListWithShortID(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if len(comments) != 1 {
|
if len(comments) != 1 {
|
||||||
t.Fatalf("expected 1 comment, got %d (short ID resolution may have failed)", len(comments))
|
t.Fatalf("expected 1 comment, got %d", len(comments))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user