From 4260129143f064748e8afb5c28646bd35cf9d70c Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 22 Jul 2026 02:25:28 -0700 Subject: [PATCH 1/3] Fix `tools create` to use the BC5 create-by-type API (#471) `basecamp tools create` 404'd: it was modeled on the removed account-scoped clone endpoint (`POST /dock/tools.json` with `source_recording_id`). bc3's BC5 lineage replaced dock-tool creation with a bucket-scoped create-by-type call, `POST /buckets/{bucket_id}/dock/tools.json` with `{tool_type, title?}`. - Bump the SDK to f2086f5e (basecamp-sdk#327), which reshapes ToolsService.Create to (bucketID, toolType, *CreateToolOptions) and posts to the bucket dock. - Remove `--source`/`--clone` (they only ever 404'd); record the removal in `.surface-breaking`. - Add `--type`/`-t` (required), normalized against the CLI's existing dock vocabulary and tolerant of degenerate spellings. A single ordered `toolTypes` slice is the source of truth for normalization, `--type` completion, and the unknown-type error listing. `board` is rejected as ambiguous. - Resolve and send a real destination bucket (the 404 root cause: `--in` was breadcrumb-only). New `resolveToolBucketID` mirrors `resolveColumnBucketID`. - Guard a stray second positional with `cobra.MaximumNArgs(1)`. - Fix stale "clone" prose in the parent and create help and agent_notes. Create-by-type is BC5-only; against BC4 the API rejects it and the error surfaces via convertSDKError. Closes #471 --- .surface | 3 +- .surface-breaking | 2 + e2e/smoke/smoke_tools.bats | 4 +- go.mod | 2 +- go.sum | 4 +- internal/commands/tools.go | 154 ++++++++++++++--- internal/commands/tools_test.go | 243 ++++++++++++++++++++++++--- internal/version/sdk-provenance.json | 10 +- 8 files changed, 367 insertions(+), 55 deletions(-) diff --git a/.surface b/.surface index 7f3d7cd56..1bfc459b8 100644 --- a/.surface +++ b/.surface @@ -13485,7 +13485,6 @@ FLAG basecamp tools --verbose type=count FLAG basecamp tools create --account type=string FLAG basecamp tools create --agent type=bool FLAG basecamp tools create --cache-dir type=string -FLAG basecamp tools create --clone type=string FLAG basecamp tools create --count type=bool FLAG basecamp tools create --help type=bool FLAG basecamp tools create --hints type=bool @@ -13500,10 +13499,10 @@ FLAG basecamp tools create --no-stats type=bool FLAG basecamp tools create --profile type=string FLAG basecamp tools create --project type=string FLAG basecamp tools create --quiet type=bool -FLAG basecamp tools create --source type=string FLAG basecamp tools create --stats type=bool FLAG basecamp tools create --styled type=bool FLAG basecamp tools create --todolist type=string +FLAG basecamp tools create --type type=string FLAG basecamp tools create --verbose type=count FLAG basecamp tools delete --account type=string FLAG basecamp tools delete --agent type=bool diff --git a/.surface-breaking b/.surface-breaking index 342ab672b..efae949ea 100644 --- a/.surface-breaking +++ b/.surface-breaking @@ -739,6 +739,8 @@ FLAG basecamp todos --status type=string FLAG basecamp todos --todoset type=string FLAG basecamp todos create --content type=string FLAG basecamp todosets --todoset type=string +FLAG basecamp tools create --clone type=string +FLAG basecamp tools create --source type=string FLAG basecamp tools create --title type=string FLAG basecamp tools rename --title type=string FLAG basecamp tools update --title type=string diff --git a/e2e/smoke/smoke_tools.bats b/e2e/smoke/smoke_tools.bats index c7742c7bc..70cd51b16 100644 --- a/e2e/smoke/smoke_tools.bats +++ b/e2e/smoke/smoke_tools.bats @@ -17,10 +17,8 @@ setup_file() { } @test "tools create creates a tool" { - ensure_messageboard || return 0 - run_smoke basecamp tools create "Smoke tool $(date +%s)" \ - --source "$QA_MESSAGEBOARD" -p "$QA_PROJECT" --json + --type message_board -p "$QA_PROJECT" --json assert_success assert_json_value '.ok' 'true' assert_json_not_null '.data.id' diff --git a/go.mod b/go.mod index b072f4bfc..ae7db6fcf 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( charm.land/bubbles/v2 v2.1.1 charm.land/bubbletea/v2 v2.0.8 charm.land/lipgloss/v2 v2.0.5 - github.com/basecamp/basecamp-sdk/go v0.7.4-0.20260722045210-81660dc101d5 + github.com/basecamp/basecamp-sdk/go v0.7.4-0.20260722082856-f2086f5e57f6 github.com/basecamp/cli v0.2.1 github.com/charmbracelet/bubbles v1.0.0 github.com/charmbracelet/glamour v1.0.0 diff --git a/go.sum b/go.sum index 9203d9e74..a0231d21d 100644 --- a/go.sum +++ b/go.sum @@ -23,8 +23,8 @@ github.com/aymanbagabas/go-udiff v0.4.1 h1:OEIrQ8maEeDBXQDoGCbbTTXYJMYRCRO1fnodZ github.com/aymanbagabas/go-udiff v0.4.1/go.mod h1:0L9PGwj20lrtmEMeyw4WKJ/TMyDtvAoK9bf2u/mNo3w= github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuPk= github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4= -github.com/basecamp/basecamp-sdk/go v0.7.4-0.20260722045210-81660dc101d5 h1:pOXigTfLk9GNEQ5sZEOIo7/122goh5LabvsdjhbZ04o= -github.com/basecamp/basecamp-sdk/go v0.7.4-0.20260722045210-81660dc101d5/go.mod h1:eX5mEKCdtxSfEL4P/n5AwOl21JVA/K+gRPic/Hd8W/Y= +github.com/basecamp/basecamp-sdk/go v0.7.4-0.20260722082856-f2086f5e57f6 h1:JFKzHPWkDq0I9nSheEYxaxIBzL+WvJKFg0j/A3n2t9s= +github.com/basecamp/basecamp-sdk/go v0.7.4-0.20260722082856-f2086f5e57f6/go.mod h1:eX5mEKCdtxSfEL4P/n5AwOl21JVA/K+gRPic/Hd8W/Y= github.com/basecamp/cli v0.2.1 h1:8GyehPVtsTXla0oOPu4QgXRjwwzJ99prlByvyi+0HRQ= github.com/basecamp/cli v0.2.1/go.mod h1:p8tt/DatJ2LAzWO6N6tNfV8x3gF5T3IxDTo+U8FfWPo= github.com/bmatcuk/doublestar v1.1.1/go.mod h1:UD6OnuiIn0yFxxA2le/rnRU1G4RaI4UvFv1sNto9p6w= diff --git a/internal/commands/tools.go b/internal/commands/tools.go index 351b441d9..8c2c849a7 100644 --- a/internal/commands/tools.go +++ b/internal/commands/tools.go @@ -3,6 +3,7 @@ package commands import ( "fmt" "strconv" + "strings" "unicode/utf8" "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" @@ -25,9 +26,9 @@ Every project has a "dock" with tools like Message Board, To-dos, Docs & Files, Chat, Schedule, etc. Tool IDs can be found in the project's dock array (see 'basecamp projects show '). -Tools can be created by cloning existing ones (e.g., create a second Chat). +Tools are created by type (e.g., add a second chat with --type chat). Disabling a tool hides it from the dock but preserves its content.`, - Annotations: map[string]string{"agent_notes": "Dock tools are the sidebar navigation items in a project\nEnable/disable controls visibility without deleting\nEach tool has a type (e.g., Todoset, Schedule, MessageBoard, Vault, Chat::Campfire)"}, + Annotations: map[string]string{"agent_notes": "Dock tools are the sidebar navigation items in a project\nEnable/disable controls visibility without deleting\nCreate by type with --type: chat, inbox, kanban_board, message_board, questionnaire, schedule, todoset, vault (create-by-type is BC5-only)"}, } cmd.PersistentFlags().StringVarP(&project, "project", "p", "", "Project ID or name (for breadcrumbs)") @@ -150,18 +151,130 @@ func newToolsShowCmd(project *string) *cobra.Command { } } +// toolTypes is the single source of truth for the closed set of dock tool +// types create-by-type accepts. Its order is deterministic and drives three +// things that must never drift: normalization, --type completion, and the +// unknown-type error listing. friendly is the primary dock noun surfaced in +// completion and errors; canonical is the Rails class-name sent to the API. +var toolTypes = []struct { + canonical string // Rails class → the tool_type posted to the API + friendly string // primary dock noun (completion + error list) + aliases []string // extra accepted forms (matched after collapse) +}{ + {"Chat::Transcript", "chat", []string{"campfire"}}, + {"Inbox", "inbox", []string{"forwards", "email"}}, + {"Kanban::Board", "kanban_board", []string{"kanban", "cardtable", "cards", "card"}}, + {"Message::Board", "message_board", []string{"messageboard", "messages", "message"}}, + {"Questionnaire", "questionnaire", []string{"questions", "checkin", "checkins", "automaticcheckins"}}, + {"Schedule", "schedule", []string{"calendar"}}, + {"Todoset", "todoset", []string{"todosets", "todos", "todo", "todolist"}}, + {"Vault", "vault", []string{"docs", "doc", "documents", "files"}}, +} + +// collapseToolType reduces an input to a comparison key: lowercased with all +// separators (::, -, _, spaces) removed. This lets "Message::Board", +// "message_board", and "message-board" all match the same entry. +func collapseToolType(input string) string { + var b strings.Builder + for _, r := range strings.ToLower(input) { + switch r { + case ':', '-', '_', ' ': + continue + default: + b.WriteRune(r) + } + } + return b.String() +} + +// normalizeToolType maps a friendly noun, canonical class-name, or accepted +// degenerate spelling to the canonical tool_type. Strict: unknown input returns +// ok=false. The closed 8-set plus the API's opaque 400 on a bad value justify +// rejecting rather than passing input through (unlike normalizeRecordingType). +func normalizeToolType(input string) (canonical string, ok bool) { + key := collapseToolType(input) + if key == "" { + return "", false + } + for _, t := range toolTypes { + if key == collapseToolType(t.canonical) || key == collapseToolType(t.friendly) { + return t.canonical, true + } + for _, alias := range t.aliases { + if key == collapseToolType(alias) { + return t.canonical, true + } + } + } + return "", false +} + +// toolTypeFriendlyNames returns the friendly nouns in stable slice order, used +// for both --type completion and the unknown-type error listing. +func toolTypeFriendlyNames() []string { + names := make([]string, len(toolTypes)) + for i, t := range toolTypes { + names[i] = t.friendly + } + return names +} + +// resolveToolBucketID resolves the numeric destination bucket for create. +// Unlike the breadcrumb-only resolveToolsProject, this bucket is sent to the +// API, so a project is required. Precedence: --in/--project > --project global +// flag > config default > interactive. Returns the bucket ID and the resolved +// project ID string (for breadcrumbs). +func resolveToolBucketID(cmd *cobra.Command, app *appctx.App, project string) (int64, string, error) { + projectID := project + if projectID == "" { + projectID = app.Flags.Project + } + if projectID == "" { + projectID = app.Config.ProjectID + } + if projectID == "" { + if err := ensureProject(cmd, app); err != nil { + return 0, "", err + } + projectID = app.Config.ProjectID + } + + resolvedProjectID, _, err := app.Names.ResolveProject(cmd.Context(), projectID) + if err != nil { + return 0, "", err + } + + bucketID, err := strconv.ParseInt(resolvedProjectID, 10, 64) + if err != nil { + return 0, "", output.ErrUsage("Project ID must be numeric") + } + return bucketID, resolvedProjectID, nil +} + func newToolsCreateCmd(project *string) *cobra.Command { - var sourceID string + var toolType string cmd := &cobra.Command{ Use: "create [title]", - Short: "Create a new dock tool by cloning", - Long: `Create a new dock tool by cloning an existing one. + Short: "Create a new dock tool by type", + Long: `Create a new dock tool by type in a project's dock. + +For example, add a second chat with --type chat, or a Message Board with +--type message_board. An optional title may be given; without one, Basecamp +assigns the default title for the type. -For example, clone a Chat to create a second chat room in the same project.`, +Accepted types: chat, inbox, kanban_board, message_board, questionnaire, +schedule, todoset, vault. Create-by-type is BC5-only.`, + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if sourceID == "" { - return output.ErrUsage("--source or --clone is required (ID of tool to clone)") + if toolType == "" { + return output.ErrUsage("--type is required") + } + + canonicalType, ok := normalizeToolType(toolType) + if !ok { + return output.ErrUsage(fmt.Sprintf("Unknown --type %q (accepted: %s)", + toolType, strings.Join(toolTypeFriendlyNames(), ", "))) } app := appctx.FromContext(cmd.Context()) @@ -170,11 +283,6 @@ For example, clone a Chat to create a second chat room in the same project.`, return err } - sourceToolID, err := strconv.ParseInt(sourceID, 10, 64) - if err != nil { - return output.ErrUsage("Invalid source tool ID") - } - title := "" if len(args) > 0 { title = args[0] @@ -186,18 +294,18 @@ For example, clone a Chat to create a second chat room in the same project.`, } } - resolvedProjectID, err := resolveToolsProject(cmd, app, *project) + bucketID, resolvedProjectID, err := resolveToolBucketID(cmd, app, *project) if err != nil { return err } inFlag := toolBreadcrumbFlag(resolvedProjectID) - var cloneOpts *basecamp.CloneToolOptions + var opts *basecamp.CreateToolOptions if title != "" { - cloneOpts = &basecamp.CloneToolOptions{Title: title} + opts = &basecamp.CreateToolOptions{Title: title} } - created, err := app.Account().Tools().Create(cmd.Context(), sourceToolID, cloneOpts) + created, err := app.Account().Tools().Create(cmd.Context(), bucketID, canonicalType, opts) if err != nil { return convertSDKError(err) } @@ -208,13 +316,11 @@ For example, clone a Chat to create a second chat room in the same project.`, Cmd: fmt.Sprintf("basecamp tools show %d%s", created.ID, inFlag), Description: "View tool", }, - } - if resolvedProjectID != "" { - crumbs = append(crumbs, output.Breadcrumb{ + { Action: "project", Cmd: fmt.Sprintf("basecamp projects show %s", resolvedProjectID), Description: "View project", - }) + }, } return app.OK(created, @@ -224,8 +330,10 @@ For example, clone a Chat to create a second chat room in the same project.`, }, } - cmd.Flags().StringVarP(&sourceID, "source", "s", "", "Source tool ID to clone (required)") - cmd.Flags().StringVar(&sourceID, "clone", "", "Source tool ID (alias for --source)") + cmd.Flags().StringVarP(&toolType, "type", "t", "", "Tool type to create (required)") + _ = cmd.RegisterFlagCompletionFunc("type", func(_ *cobra.Command, _ []string, _ string) ([]cobra.Completion, cobra.ShellCompDirective) { + return toolTypeFriendlyNames(), cobra.ShellCompDirectiveNoFileComp + }) return cmd } diff --git a/internal/commands/tools_test.go b/internal/commands/tools_test.go index 33c58a839..268814f23 100644 --- a/internal/commands/tools_test.go +++ b/internal/commands/tools_test.go @@ -8,48 +8,253 @@ import ( "strings" "testing" + "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/basecamp/basecamp-cli/internal/output" ) -// TestToolsCreateAcceptsCloneAlias tests that --clone works as an alias for --source. -// Previously, MarkFlagRequired("source") caused Cobra to reject --clone before RunE ran. -func TestToolsCreateAcceptsCloneAlias(t *testing.T) { - app, _ := setupTestApp(t) - app.Config.ProjectID = "123" +// mockToolCreateTransport captures the create request and returns a 201 Tool. +// Project resolution is served so bucket resolution reaches the create call. +type mockToolCreateTransport struct { + createCalled bool + capturedPath string + capturedBody map[string]any +} + +func (t *mockToolCreateTransport) RoundTrip(req *http.Request) (*http.Response, error) { + header := make(http.Header) + header.Set("Content-Type", "application/json") + + if req.Method == http.MethodPost && strings.HasSuffix(req.URL.Path, "/dock/tools.json") { + t.createCalled = true + t.capturedPath = req.URL.Path + if req.Body != nil { + data, _ := io.ReadAll(req.Body) + _ = json.Unmarshal(data, &t.capturedBody) + } + body := `{"id": 777, "title": "Intervention Log", "name": "message_board", "enabled": true,` + + `"position": 3, "status": "active", "url": "https://example.com", "app_url": "https://example.com",` + + `"created_at": "2024-01-01T00:00:00Z", "updated_at": "2024-01-01T00:00:00Z"}` + return &http.Response{ + StatusCode: 201, + Body: io.NopCloser(strings.NewReader(body)), + Header: header, + }, nil + } + + body := `{}` + if strings.Contains(req.URL.Path, "/projects.json") { + body = `[{"id": 123, "name": "Test Project"}, {"id": 456, "name": "Other Project"}]` + } + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(strings.NewReader(body)), + Header: header, + }, nil +} + +// TestToolsCreateRequiresType verifies that omitting --type produces a usage +// error naming the flag and issues no create request. +func TestToolsCreateRequiresType(t *testing.T) { + transport := &mockToolCreateTransport{} + app, _ := newTestAppWithTransport(t, transport) project := "" cmd := newToolsCreateCmd(&project) - // --clone should reach the RunE guard, not fail with "required flag not set" - err := executeCommand(cmd, app, "--clone", "999", "My Tool") - - // Expect an API/network error — NOT "required flag" and NOT the RunE usage guard + err := executeCommand(cmd, app, "My Tool") require.NotNil(t, err) + var e *output.Error - if errors.As(err, &e) { - assert.NotEqual(t, "--source or --clone is required (ID of tool to clone)", e.Message) + require.True(t, errors.As(err, &e)) + assert.Equal(t, "--type is required", e.Message) + assert.False(t, transport.createCalled, "no create request should be issued") +} + +// TestToolsCreateRejectsUnknownType verifies that an unrecognized --type errors +// with the friendly nouns in stable slice order, and issues no create request. +func TestToolsCreateRejectsUnknownType(t *testing.T) { + for _, unknown := range []string{"board", "nope"} { + transport := &mockToolCreateTransport{} + app, _ := newTestAppWithTransport(t, transport) + + project := "" + cmd := newToolsCreateCmd(&project) + + err := executeCommand(cmd, app, "--type", unknown) + require.NotNil(t, err, "type %q should be rejected", unknown) + + var e *output.Error + require.True(t, errors.As(err, &e)) + assert.Contains(t, e.Message, "chat, inbox, kanban_board, message_board, questionnaire, schedule, todoset, vault") + assert.False(t, transport.createCalled, "no create request should be issued for %q", unknown) } - assert.NotContains(t, err.Error(), "required flag") } -// TestToolsCreateRequiresSourceOrClone tests that omitting both --source and --clone -// produces a usage error from the RunE guard. -func TestToolsCreateRequiresSourceOrClone(t *testing.T) { +// TestNormalizeToolType covers the closed 8-set: each friendly noun, a +// degenerate spelling, and the canonical class-name all map to the right +// tool_type; bare "board" is rejected as ambiguous. +func TestNormalizeToolType(t *testing.T) { + cases := []struct { + input string + canonical string + }{ + {"chat", "Chat::Transcript"}, + {"campfire", "Chat::Transcript"}, + {"Chat::Transcript", "Chat::Transcript"}, + {"inbox", "Inbox"}, + {"email", "Inbox"}, + {"Inbox", "Inbox"}, + {"kanban_board", "Kanban::Board"}, + {"cardtable", "Kanban::Board"}, + {"Kanban::Board", "Kanban::Board"}, + {"message_board", "Message::Board"}, + {"message-board", "Message::Board"}, + {"messages", "Message::Board"}, + {"Message::Board", "Message::Board"}, + {"questionnaire", "Questionnaire"}, + {"checkins", "Questionnaire"}, + {"Questionnaire", "Questionnaire"}, + {"schedule", "Schedule"}, + {"calendar", "Schedule"}, + {"Schedule", "Schedule"}, + {"todoset", "Todoset"}, + {"todos", "Todoset"}, + {"Todoset", "Todoset"}, + {"vault", "Vault"}, + {"docs", "Vault"}, + {"Vault", "Vault"}, + } + for _, tc := range cases { + got, ok := normalizeToolType(tc.input) + assert.True(t, ok, "input %q should normalize", tc.input) + assert.Equal(t, tc.canonical, got, "input %q", tc.input) + } + + for _, bad := range []string{"board", "", "nope", "widget"} { + _, ok := normalizeToolType(bad) + assert.False(t, ok, "input %q should be rejected", bad) + } +} + +// TestToolsCreateRejectsExtraPositional verifies MaximumNArgs(1): a stray second +// positional errors instead of being silently dropped. +func TestToolsCreateRejectsExtraPositional(t *testing.T) { app, _ := setupTestApp(t) app.Config.ProjectID = "123" project := "" cmd := newToolsCreateCmd(&project) - err := executeCommand(cmd, app, "My Tool") + err := executeCommand(cmd, app, "--type", "chat", "Title A", "Title B") + require.NotNil(t, err) + assert.Contains(t, err.Error(), "accepts at most 1 arg") +} + +// TestToolsCreateExplicitProjectWinsAndIsSent verifies that an explicit +// --in/--project beats the global flag and config default, and the resolved +// bucket is the one posted to. +func TestToolsCreateExplicitProjectWinsAndIsSent(t *testing.T) { + transport := &mockToolCreateTransport{} + app, _ := newTestAppWithTransport(t, transport) + app.Flags.Project = "123" + app.Config.ProjectID = "123" + + project := "456" // explicit --in wins over flag/config + cmd := newToolsCreateCmd(&project) + + err := executeCommand(cmd, app, "--type", "message_board") + require.NoError(t, err) + + require.True(t, transport.createCalled) + assert.True(t, strings.HasSuffix(transport.capturedPath, "/99999/buckets/456/dock/tools.json"), + "create path = %s", transport.capturedPath) + assert.Equal(t, "Message::Board", transport.capturedBody["tool_type"]) +} + +// TestToolsCreateRequiresResolvableProject verifies that with nothing resolvable +// (no flag, no config, non-interactive) create errors before any request. +func TestToolsCreateRequiresResolvableProject(t *testing.T) { + app, _ := setupTestApp(t) + // No ProjectID configured — nothing to resolve, non-interactive. + + project := "" + cmd := newToolsCreateCmd(&project) + + err := executeCommand(cmd, app, "--type", "chat") require.NotNil(t, err) var e *output.Error require.True(t, errors.As(err, &e)) - assert.Equal(t, "--source or --clone is required (ID of tool to clone)", e.Message) + assert.Equal(t, "Project ID required", e.Message) +} + +// TestToolsCreateSendsTitle verifies a valid title reaches the request body. +func TestToolsCreateSendsTitle(t *testing.T) { + transport := &mockToolCreateTransport{} + app, _ := newTestAppWithTransport(t, transport) + app.Config.ProjectID = "123" + + project := "" + cmd := newToolsCreateCmd(&project) + + err := executeCommand(cmd, app, "--type", "message_board", "Intervention Log") + require.NoError(t, err) + + require.True(t, transport.createCalled) + assert.Equal(t, "Intervention Log", transport.capturedBody["title"]) + assert.Equal(t, "Message::Board", transport.capturedBody["tool_type"]) +} + +// TestToolsCreateHappyPath verifies the full request path and 201 payload +// handling for a title-less create. +func TestToolsCreateHappyPath(t *testing.T) { + transport := &mockToolCreateTransport{} + app, buf := newTestAppWithTransport(t, transport) + app.Config.ProjectID = "123" + + project := "" + cmd := newToolsCreateCmd(&project) + + err := executeCommand(cmd, app, "--type", "todos") + require.NoError(t, err) + + require.True(t, transport.createCalled) + assert.True(t, strings.HasSuffix(transport.capturedPath, "/99999/buckets/123/dock/tools.json"), + "create path = %s", transport.capturedPath) + assert.Equal(t, "Todoset", transport.capturedBody["tool_type"]) + _, hasTitle := transport.capturedBody["title"] + assert.False(t, hasTitle, "title should be omitted when not provided") + + var envelope struct { + OK bool `json:"ok"` + Data struct { + ID int64 `json:"id"` + } `json:"data"` + } + require.NoError(t, json.Unmarshal(buf.Bytes(), &envelope)) + assert.True(t, envelope.OK) + assert.Equal(t, int64(777), envelope.Data.ID) +} + +// TestToolsCreateTypeCompletion verifies the registered --type completion +// returns the eight friendly nouns in stable slice order. +func TestToolsCreateTypeCompletion(t *testing.T) { + project := "" + cmd := newToolsCreateCmd(&project) + + fn, ok := cmd.GetFlagCompletionFunc("type") + require.True(t, ok) + require.NotNil(t, fn) + + comps, directive := fn(cmd, nil, "") + assert.Equal(t, cobra.ShellCompDirectiveNoFileComp, directive) + assert.Equal(t, []string{ + "chat", "inbox", "kanban_board", "message_board", + "questionnaire", "schedule", "todoset", "vault", + }, comps) } // TestToolsRepositionAcceptsPosAlias tests that --pos works as an alias for --position. @@ -132,7 +337,7 @@ func TestToolsCreateRejectsLongTitle(t *testing.T) { cmd := newToolsCreateCmd(&project) longTitle := strings.Repeat("x", 65) - err := executeCommand(cmd, app, "--source", "999", longTitle) + err := executeCommand(cmd, app, "--type", "chat", longTitle) require.NotNil(t, err) var e *output.Error @@ -149,7 +354,7 @@ func TestToolsCreateAcceptsMaxTitle(t *testing.T) { cmd := newToolsCreateCmd(&project) maxTitle := strings.Repeat("x", 64) - err := executeCommand(cmd, app, "--source", "999", maxTitle) + err := executeCommand(cmd, app, "--type", "chat", maxTitle) require.NotNil(t, err) // fails at network, not validation var e *output.Error diff --git a/internal/version/sdk-provenance.json b/internal/version/sdk-provenance.json index ab9b3b2a5..1bedab532 100644 --- a/internal/version/sdk-provenance.json +++ b/internal/version/sdk-provenance.json @@ -1,13 +1,13 @@ { "sdk": { "module": "github.com/basecamp/basecamp-sdk/go", - "version": "v0.7.4-0.20260722045210-81660dc101d5", - "revision": "81660dc101d5", - "updated_at": "2026-07-22T04:52:10Z" + "version": "v0.7.4-0.20260722082856-f2086f5e57f6", + "revision": "f2086f5e57f6", + "updated_at": "2026-07-22T08:28:56Z" }, "api": { "repo": "basecamp/bc3", - "revision": "e52453ff35a6292473eb095a8568649aa132fef9", - "synced_at": "2026-07-17" + "revision": "ba105ba7d7e48bd97afdc98305e9fb8a63a88beb", + "synced_at": "2026-07-22" } } From 1e42b77d665f76b6e9684be580c0f2bca63d92a9 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 22 Jul 2026 02:35:25 -0700 Subject: [PATCH 2/3] Give `--type board` an ambiguity-specific error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bare "board" (and separator variants) is intentionally rejected — it's ambiguous between message_board and kanban_board. Report that specifically instead of the generic "Unknown --type" wording, while still listing the accepted values. Addresses Copilot review feedback on #553. --- internal/commands/tools.go | 5 +++++ internal/commands/tools_test.go | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/internal/commands/tools.go b/internal/commands/tools.go index 8c2c849a7..460df3577 100644 --- a/internal/commands/tools.go +++ b/internal/commands/tools.go @@ -273,6 +273,11 @@ schedule, todoset, vault. Create-by-type is BC5-only.`, canonicalType, ok := normalizeToolType(toolType) if !ok { + if collapseToolType(toolType) == "board" { + return output.ErrUsage(fmt.Sprintf( + "Ambiguous --type %q — use message_board or kanban_board (accepted: %s)", + toolType, strings.Join(toolTypeFriendlyNames(), ", "))) + } return output.ErrUsage(fmt.Sprintf("Unknown --type %q (accepted: %s)", toolType, strings.Join(toolTypeFriendlyNames(), ", "))) } diff --git a/internal/commands/tools_test.go b/internal/commands/tools_test.go index 268814f23..4bfb73f0a 100644 --- a/internal/commands/tools_test.go +++ b/internal/commands/tools_test.go @@ -93,6 +93,27 @@ func TestToolsCreateRejectsUnknownType(t *testing.T) { } } +// TestToolsCreateRejectsAmbiguousBoard verifies the bare "board" (and its +// separator variants) get a message naming both message_board and kanban_board +// rather than the generic "Unknown" wording. +func TestToolsCreateRejectsAmbiguousBoard(t *testing.T) { + for _, ambiguous := range []string{"board", "Board", "BOARD"} { + app, _ := setupTestApp(t) + app.Config.ProjectID = "123" + + project := "" + cmd := newToolsCreateCmd(&project) + + err := executeCommand(cmd, app, "--type", ambiguous) + require.NotNil(t, err, "type %q should be rejected", ambiguous) + + var e *output.Error + require.True(t, errors.As(err, &e)) + assert.Contains(t, e.Message, "Ambiguous") + assert.Contains(t, e.Message, "message_board or kanban_board") + } +} + // TestNormalizeToolType covers the closed 8-set: each friendly noun, a // degenerate spelling, and the canonical class-name all map to the right // tool_type; bare "board" is rejected as ambiguous. From fd3835e9a2c76dcb80cba5e4931225f06996729e Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 22 Jul 2026 02:50:54 -0700 Subject: [PATCH 3/3] Address review: derive type help from the type table; update coverage - Build the create --type help text and the parent agent_notes accepted-type list from toolTypeFriendlyNames() instead of hard-coding them, so help stays in sync with validation and completion (Copilot + cubic review feedback). - API-COVERAGE.md: correct the stale "v0.7.3 / no new methods" line to reflect the f2086f5e bump and the BC5 create-by-type reshape, and add the previously uncounted dock_tools row (Codex P2 feedback). --- API-COVERAGE.md | 11 ++++++----- internal/commands/tools.go | 12 ++++++++---- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/API-COVERAGE.md b/API-COVERAGE.md index 170d0dce2..72bcf6f76 100644 --- a/API-COVERAGE.md +++ b/API-COVERAGE.md @@ -6,17 +6,17 @@ Coverage of Basecamp 3 API endpoints. Source: [bc3-api/sections](https://github. | Status | Sections | Endpoints | |--------|----------|-----------| -| ✅ Implemented | 42 | 155 | +| ✅ Implemented | 43 | 162 | | ⏭️ Out of scope | 4 | 12 | -| **Total (docs)** | **46** | **167** | +| **Total (docs)** | **47** | **174** | -**100% coverage of in-scope API** (155/155 endpoints) +**100% coverage of in-scope API** (162/162 endpoints) Out-of-scope sections are excluded from parity totals and scripts: chatbots (different auth), legacy Clientside (deprecated) -> Note: the per-row `Endpoints` column in the Coverage by Section table sums higher than the Summary totals above. The discrepancy predates the BC5 baseline; the row count (46 sections) is authoritative for the `Since` column. Reconciling endpoint counts is pre-existing maintenance, tracked separately. +> Note: the per-row `Endpoints` column in the Coverage by Section table sums higher than the Summary totals above. The discrepancy predates the BC5 baseline; the row count (47 sections) is authoritative for the `Since` column. Reconciling endpoint counts is pre-existing maintenance, tracked separately. -**SDK version:** v0.7.3 — maintenance bump: API date advanced to 2026-03-23, transitive dependency updates. No new services or methods. +**SDK version:** v0.7.4 (`f2086f5e`) — reshapes `Tools.Create` to the BC5 bucket-scoped create-by-type endpoint (`POST /buckets/{id}/dock/tools.json` with `{tool_type, title?}`), replacing the removed account-scoped clone call. API date advanced to 2026-07-22. ## Coverage by Section @@ -30,6 +30,7 @@ The **Since** column tags each row with the Basecamp version that introduced its | todolists | 8 | `todolists` | ✅ | BC4 | - | list, show, create, update | | todosets | 3 | `todosets` | ✅ | BC4 | - | Container for todolists, accessed via project dock (BC5: `todos_count`, `completed_loose_todos_count`, `todos_url`, `app_todos_url`) | | todolist_groups | 8 | `todolistgroups` | ✅ | BC4 | - | list, show, create, update, position | +| dock_tools | 7 | `tools` | ✅ | BC4 | - | Dock tool management: show, update, trash, enable, disable, reposition. `create` is BC5-only (create-by-type: `POST /buckets/{id}/dock/tools.json`), replacing the removed clone call | | **Hill Charts** | | hill_charts | 2 | `hillcharts` | ✅ | BC4 | - | show, track/untrack todolists | | gauges | 7 | `gauges` | ✅ | BC4 | - | list, needles, needle, create, update, delete, enable/disable | diff --git a/internal/commands/tools.go b/internal/commands/tools.go index 460df3577..78c9c0e09 100644 --- a/internal/commands/tools.go +++ b/internal/commands/tools.go @@ -28,7 +28,11 @@ Chat, Schedule, etc. Tool IDs can be found in the project's dock array Tools are created by type (e.g., add a second chat with --type chat). Disabling a tool hides it from the dock but preserves its content.`, - Annotations: map[string]string{"agent_notes": "Dock tools are the sidebar navigation items in a project\nEnable/disable controls visibility without deleting\nCreate by type with --type: chat, inbox, kanban_board, message_board, questionnaire, schedule, todoset, vault (create-by-type is BC5-only)"}, + Annotations: map[string]string{"agent_notes": fmt.Sprintf( + "Dock tools are the sidebar navigation items in a project\n"+ + "Enable/disable controls visibility without deleting\n"+ + "Create by type with --type: %s (create-by-type is BC5-only)", + strings.Join(toolTypeFriendlyNames(), ", "))}, } cmd.PersistentFlags().StringVarP(&project, "project", "p", "", "Project ID or name (for breadcrumbs)") @@ -257,14 +261,14 @@ func newToolsCreateCmd(project *string) *cobra.Command { cmd := &cobra.Command{ Use: "create [title]", Short: "Create a new dock tool by type", - Long: `Create a new dock tool by type in a project's dock. + Long: fmt.Sprintf(`Create a new dock tool by type in a project's dock. For example, add a second chat with --type chat, or a Message Board with --type message_board. An optional title may be given; without one, Basecamp assigns the default title for the type. -Accepted types: chat, inbox, kanban_board, message_board, questionnaire, -schedule, todoset, vault. Create-by-type is BC5-only.`, +Accepted types: %s. Create-by-type is BC5-only.`, + strings.Join(toolTypeFriendlyNames(), ", ")), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if toolType == "" {