From 81800413874644d985d2f86edb1e2c4b1a6d756e Mon Sep 17 00:00:00 2001 From: Nikita Nemirovsky Date: Wed, 10 Jun 2026 14:56:19 +0800 Subject: [PATCH 1/5] fix(cards): carry over step title when updating assignees or due date The step update endpoint rejects requests without a title even though the API docs mark it optional. Updating only assignees or the due date sent a body with no title and the server returned 400 Bad Request. Carry over the current title instead. The step update command fetches the step first when no new title is given. Assign and unassign reuse the step they already fetched. --- internal/commands/assign.go | 6 +++ internal/commands/cards.go | 8 ++++ internal/commands/cards_test.go | 69 +++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+) diff --git a/internal/commands/assign.go b/internal/commands/assign.go index 072b4db1..dc069b47 100644 --- a/internal/commands/assign.go +++ b/internal/commands/assign.go @@ -644,7 +644,10 @@ func doAssignStep(cmd *cobra.Command, app *appctx.App, stepIDStr, assigneeID str } assigneeIDs = append(assigneeIDs, assigneeIDInt) + // The API rejects step updates without a title, so carry over the + // current one. updated, err := app.Account().CardSteps().Update(cmd.Context(), stepID, &basecamp.UpdateStepRequest{ + Title: step.Title, AssigneeIDs: assigneeIDs, }) if err != nil { @@ -746,7 +749,10 @@ func doUnassignStep(cmd *cobra.Command, app *appctx.App, stepIDStr string, assig assigneeIDs := removeID(existingAssigneeIDs(step.Assignees), assigneeIDInt) + // The API rejects step updates without a title, so carry over the + // current one. updated, err := app.Account().CardSteps().Update(cmd.Context(), stepID, &basecamp.UpdateStepRequest{ + Title: step.Title, AssigneeIDs: assigneeIDs, }) if err != nil { diff --git a/internal/commands/cards.go b/internal/commands/cards.go index 49f52e4a..f64361ef 100644 --- a/internal/commands/cards.go +++ b/internal/commands/cards.go @@ -2009,6 +2009,14 @@ You can pass either a step ID or a Basecamp URL: req := &basecamp.UpdateStepRequest{} if title != "" { req.Title = title + } else { + // The API rejects step updates without a title, so carry + // over the current one when only other fields change. + current, err := app.Account().CardSteps().Get(cmd.Context(), stepID) + if err != nil { + return convertSDKError(err) + } + req.Title = current.Title } if dueOn != "" { req.DueOn = dateparse.Parse(dueOn) diff --git a/internal/commands/cards_test.go b/internal/commands/cards_test.go index e6ba6884..809c6fca 100644 --- a/internal/commands/cards_test.go +++ b/internal/commands/cards_test.go @@ -189,6 +189,75 @@ func TestCardsStepUpdateRequiresFields(t *testing.T) { assert.NoError(t, err, "expected help output, not error") } +// mockStepUpdateTransport serves the current step on GET and captures the +// update body on PUT. +type mockStepUpdateTransport struct { + getCount int + capturedPut []byte +} + +func (t *mockStepUpdateTransport) RoundTrip(req *http.Request) (*http.Response, error) { + header := make(http.Header) + header.Set("Content-Type", "application/json") + + stepJSON := `{"id": 456, "title": "Current title", "completed": false, "assignees": []}` + + switch req.Method { + case "GET": + t.getCount++ + case "PUT": + if req.Body != nil { + body, _ := io.ReadAll(req.Body) + t.capturedPut = body + req.Body.Close() + } + default: + return nil, errors.New("unexpected request") + } + + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(strings.NewReader(stepJSON)), + Header: header, + }, nil +} + +// TestCardsStepUpdateAssigneesOnlyCarriesTitle verifies that updating only +// assignees fetches the current step and includes its title in the request — +// the API rejects step updates without a title. +func TestCardsStepUpdateAssigneesOnlyCarriesTitle(t *testing.T) { + transport := &mockStepUpdateTransport{} + app := setupCardsMockApp(t, transport) + + cmd := newCardsStepUpdateCmd() + err := executeCommand(cmd, app, "456", "--assignees", "789") + require.NoError(t, err) + + assert.Equal(t, 1, transport.getCount) + + var body map[string]any + require.NoError(t, json.Unmarshal(transport.capturedPut, &body)) + assert.Equal(t, "Current title", body["title"]) + assert.Equal(t, []any{float64(789)}, body["assignee_ids"]) +} + +// TestCardsStepUpdateWithTitleSkipsFetch verifies that an explicit title is +// sent as-is without fetching the current step. +func TestCardsStepUpdateWithTitleSkipsFetch(t *testing.T) { + transport := &mockStepUpdateTransport{} + app := setupCardsMockApp(t, transport) + + cmd := newCardsStepUpdateCmd() + err := executeCommand(cmd, app, "456", "New title") + require.NoError(t, err) + + assert.Equal(t, 0, transport.getCount) + + var body map[string]any + require.NoError(t, json.Unmarshal(transport.capturedPut, &body)) + assert.Equal(t, "New title", body["title"]) +} + // TestCardsStepMoveRequiresCard tests that --card is required for step move. func TestCardsStepMoveShowsHelp(t *testing.T) { app, _ := setupTestApp(t) From 9f04fdc3a7b8adc1ead6b5bd6a432a204561fbc5 Mon Sep 17 00:00:00 2001 From: Nikita Nemirovsky Date: Wed, 10 Jun 2026 17:17:55 +0800 Subject: [PATCH 2/5] test(cards): assert step endpoint path and handle mock IO errors Validate that step updates hit the single-step endpoint so a stray call to the wrong path fails the test instead of passing on stale data. Echo the title back in the PUT response and surface IO errors from the mock transport. --- internal/commands/cards_test.go | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/internal/commands/cards_test.go b/internal/commands/cards_test.go index 809c6fca..eae7d9bf 100644 --- a/internal/commands/cards_test.go +++ b/internal/commands/cards_test.go @@ -190,7 +190,8 @@ func TestCardsStepUpdateRequiresFields(t *testing.T) { } // mockStepUpdateTransport serves the current step on GET and captures the -// update body on PUT. +// update body on PUT. It only answers the single-step endpoint so a stray +// call to the wrong path fails the test instead of passing on stale data. type mockStepUpdateTransport struct { getCount int capturedPut []byte @@ -200,20 +201,37 @@ func (t *mockStepUpdateTransport) RoundTrip(req *http.Request) (*http.Response, header := make(http.Header) header.Set("Content-Type", "application/json") - stepJSON := `{"id": 456, "title": "Current title", "completed": false, "assignees": []}` + if req.URL.Path != "/99999/card_tables/steps/456" { + return nil, fmt.Errorf("unexpected request path: %s", req.URL.Path) + } switch req.Method { case "GET": t.getCount++ case "PUT": - if req.Body != nil { - body, _ := io.ReadAll(req.Body) - t.capturedPut = body - req.Body.Close() + body, err := io.ReadAll(req.Body) + if err != nil { + return nil, err + } + t.capturedPut = body + if err := req.Body.Close(); err != nil { + return nil, err } default: - return nil, errors.New("unexpected request") + return nil, fmt.Errorf("unexpected request method: %s", req.Method) + } + + // Echo the title back so callers see the effective value, not a constant. + title := "Current title" + if t.capturedPut != nil { + var put struct { + Title string `json:"title"` + } + if err := json.Unmarshal(t.capturedPut, &put); err == nil && put.Title != "" { + title = put.Title + } } + stepJSON := fmt.Sprintf(`{"id": 456, "title": %q, "completed": false, "assignees": []}`, title) return &http.Response{ StatusCode: 200, From 65d1a2a4e5069df5c5ba345472184d959d148e69 Mon Sep 17 00:00:00 2001 From: Nikita Nemirovsky Date: Wed, 10 Jun 2026 17:27:57 +0800 Subject: [PATCH 3/5] test(cards): match step endpoint by suffix instead of exact path Suffix matching keeps the routing assertion while tolerating account ID and prefix changes in the SDK path shape. --- internal/commands/cards_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/commands/cards_test.go b/internal/commands/cards_test.go index eae7d9bf..473de00d 100644 --- a/internal/commands/cards_test.go +++ b/internal/commands/cards_test.go @@ -201,7 +201,7 @@ func (t *mockStepUpdateTransport) RoundTrip(req *http.Request) (*http.Response, header := make(http.Header) header.Set("Content-Type", "application/json") - if req.URL.Path != "/99999/card_tables/steps/456" { + if !strings.HasSuffix(req.URL.Path, "/card_tables/steps/456") { return nil, fmt.Errorf("unexpected request path: %s", req.URL.Path) } From a1e7958524ba8697fc93b24842bc8ad9aa45eb85 Mon Sep 17 00:00:00 2001 From: Nikita Nemirovsky Date: Wed, 10 Jun 2026 17:40:09 +0800 Subject: [PATCH 4/5] test(assign): cover title carry-over on step assign and unassign The assign and unassign step paths must send the current title or the API rejects the update with 400. Add tests asserting the captured PUT body includes the carried-over title for both paths. --- internal/commands/assign_test.go | 74 ++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/internal/commands/assign_test.go b/internal/commands/assign_test.go index 181c7b71..d922fd73 100644 --- a/internal/commands/assign_test.go +++ b/internal/commands/assign_test.go @@ -3,6 +3,7 @@ package commands import ( "bytes" "context" + "encoding/json" "errors" "fmt" "io" @@ -706,3 +707,76 @@ func TestAssignBatchAllFailNeverResolvesAssignee(t *testing.T) { "person resolution should not be attempted when all items fail validation") } } + +// mockStepAssignTransport serves the current step on GET and captures the +// PUT body so the assign/unassign step paths can be checked for the +// carried-over title the API requires. +type mockStepAssignTransport struct { + capturedPut []byte +} + +func (m *mockStepAssignTransport) RoundTrip(req *http.Request) (*http.Response, error) { + header := make(http.Header) + header.Set("Content-Type", "application/json") + + stepJSON := `{"id": 456, "title": "Existing step", "completed": false, "assignees": [{"id": 11, "name": "Existing Person"}]}` + + switch req.Method { + case "GET": + body := `{}` + if strings.Contains(req.URL.Path, "/card_tables/steps/") { + body = stepJSON + } else if strings.Contains(req.URL.Path, "/projects.json") { + body = `[{"id": 123, "name": "Test Project"}]` + } else if strings.Contains(req.URL.Path, "/projects/") { + body = `{"id": 123, "dock": [{"name": "kanban_board", "id": 789, "title": "Card Table"}]}` + } else if strings.Contains(req.URL.Path, "/people.json") { + body = `[{"id": 11, "name": "Existing Person"}, {"id": 99, "name": "New Person"}]` + } + return &http.Response{StatusCode: 200, Body: io.NopCloser(strings.NewReader(body)), Header: header}, nil + case "PUT": + body, err := io.ReadAll(req.Body) + if err != nil { + return nil, err + } + m.capturedPut = body + if err := req.Body.Close(); err != nil { + return nil, err + } + return &http.Response{StatusCode: 200, Body: io.NopCloser(strings.NewReader(stepJSON)), Header: header}, nil + default: + return nil, fmt.Errorf("unexpected request method: %s", req.Method) + } +} + +// TestAssignStepCarriesTitle verifies that assigning a person to a step sends +// the current title in the update, which the API requires. +func TestAssignStepCarriesTitle(t *testing.T) { + transport := &mockStepAssignTransport{} + app := setupCardsMockApp(t, transport) + + cmd := NewAssignCmd() + err := executeAssignCommand(cmd, app, "456", "--step", "--to", "99") + require.NoError(t, err) + + var body map[string]any + require.NoError(t, json.Unmarshal(transport.capturedPut, &body)) + assert.Equal(t, "Existing step", body["title"]) + assert.Equal(t, []any{float64(11), float64(99)}, body["assignee_ids"]) +} + +// TestUnassignStepCarriesTitle verifies that removing a person from a step +// also sends the current title in the update. +func TestUnassignStepCarriesTitle(t *testing.T) { + transport := &mockStepAssignTransport{} + app := setupCardsMockApp(t, transport) + + cmd := NewUnassignCmd() + err := executeAssignCommand(cmd, app, "456", "--step", "--from", "11") + require.NoError(t, err) + + var body map[string]any + require.NoError(t, json.Unmarshal(transport.capturedPut, &body)) + assert.Equal(t, "Existing step", body["title"]) + assert.Empty(t, body["assignee_ids"]) +} From 84c2c5f4cc746725d1a3c29c7543136e48722021 Mon Sep 17 00:00:00 2001 From: Nikita Nemirovsky Date: Wed, 10 Jun 2026 17:46:00 +0800 Subject: [PATCH 5/5] test(assign): tighten step mock to assert endpoint and empty array Fail on unexpected GET routes, assert the PUT targets the step endpoint, and check assignee_ids is sent as an explicit empty array on unassign. --- internal/commands/assign_test.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/internal/commands/assign_test.go b/internal/commands/assign_test.go index d922fd73..1c455d88 100644 --- a/internal/commands/assign_test.go +++ b/internal/commands/assign_test.go @@ -723,18 +723,24 @@ func (m *mockStepAssignTransport) RoundTrip(req *http.Request) (*http.Response, switch req.Method { case "GET": - body := `{}` - if strings.Contains(req.URL.Path, "/card_tables/steps/") { + var body string + switch { + case strings.Contains(req.URL.Path, "/card_tables/steps/"): body = stepJSON - } else if strings.Contains(req.URL.Path, "/projects.json") { + case strings.Contains(req.URL.Path, "/projects.json"): body = `[{"id": 123, "name": "Test Project"}]` - } else if strings.Contains(req.URL.Path, "/projects/") { + case strings.Contains(req.URL.Path, "/projects/"): body = `{"id": 123, "dock": [{"name": "kanban_board", "id": 789, "title": "Card Table"}]}` - } else if strings.Contains(req.URL.Path, "/people.json") { + case strings.Contains(req.URL.Path, "/people.json"): body = `[{"id": 11, "name": "Existing Person"}, {"id": 99, "name": "New Person"}]` + default: + return nil, fmt.Errorf("unexpected GET path: %s", req.URL.Path) } return &http.Response{StatusCode: 200, Body: io.NopCloser(strings.NewReader(body)), Header: header}, nil case "PUT": + if !strings.HasSuffix(req.URL.Path, "/card_tables/steps/456") { + return nil, fmt.Errorf("unexpected PUT path: %s", req.URL.Path) + } body, err := io.ReadAll(req.Body) if err != nil { return nil, err @@ -778,5 +784,5 @@ func TestUnassignStepCarriesTitle(t *testing.T) { var body map[string]any require.NoError(t, json.Unmarshal(transport.capturedPut, &body)) assert.Equal(t, "Existing step", body["title"]) - assert.Empty(t, body["assignee_ids"]) + assert.Equal(t, []any{}, body["assignee_ids"]) }