From 39485b90c171ca9981cb77a67c5c7814633a0fdb Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 20 Apr 2026 10:23:51 -0400 Subject: [PATCH 1/6] Preserve untouched document fields in files update --- internal/commands/files.go | 55 ++++++++++++++++----- internal/commands/files_test.go | 87 +++++++++++++++++++++++++++++++++ skills/basecamp/SKILL.md | 4 ++ 3 files changed, 135 insertions(+), 11 deletions(-) diff --git a/internal/commands/files.go b/internal/commands/files.go index 03a94e6a..82e6f9be 100644 --- a/internal/commands/files.go +++ b/internal/commands/files.go @@ -1208,10 +1208,13 @@ func newFilesUpdateCmd(project *string) *cobra.Command { Short: "Update a document, vault, or upload", Long: `Update a document, vault, or upload. +For documents, updating only --title or only --content preserves the untouched field. + You can pass either an item ID or a Basecamp URL: basecamp files update 789 --title "new title" --in my-project basecamp files update 789 --content "new content" --in my-project`, - Args: cobra.ExactArgs(1), + Annotations: map[string]string{"agent_notes": "Document updates preserve untouched title/content by fetching current state first because Basecamp API clears omitted fields on PUT."}, + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if strings.TrimSpace(title) == "" && strings.TrimSpace(content) == "" && itemType == "" { return noChanges(cmd) @@ -1269,12 +1272,10 @@ You can pass either an item ID or a Basecamp URL: result = vault detectedType = "vault" case "document", "doc": - docHTML := richtext.MarkdownToHTML(content) - docHTML, err = resolveLocalImages(cmd, app, docHTML) + req, err := buildDocumentUpdateRequest(cmd, app, itemID, nil, title, content) if err != nil { - return err + return convertSDKError(err) } - req := &basecamp.UpdateDocumentRequest{Title: title, Content: docHTML} doc, err := app.Account().Documents().Update(cmd.Context(), itemID, req) if err != nil { return convertSDKError(err) @@ -1304,14 +1305,12 @@ You can pass either an item ID or a Basecamp URL: var firstErr error // Try document first (most common update case) - _, err := app.Account().Documents().Get(cmd.Context(), itemID) + existingDoc, err := app.Account().Documents().Get(cmd.Context(), itemID) if err == nil { - docHTML := richtext.MarkdownToHTML(content) - docHTML, resolveErr := resolveLocalImages(cmd, app, docHTML) - if resolveErr != nil { - return resolveErr + req, buildErr := buildDocumentUpdateRequest(cmd, app, itemID, existingDoc, title, content) + if buildErr != nil { + return convertSDKError(buildErr) } - req := &basecamp.UpdateDocumentRequest{Title: title, Content: docHTML} doc, err := app.Account().Documents().Update(cmd.Context(), itemID, req) if err != nil { return convertSDKError(err) @@ -1380,6 +1379,40 @@ You can pass either an item ID or a Basecamp URL: return cmd } +func buildDocumentUpdateRequest(cmd *cobra.Command, app *appctx.App, itemID int64, existingDoc *basecamp.Document, title, content string) (*basecamp.UpdateDocumentRequest, error) { + // BC3 document updates are destructive PUTs: omitted title/content fields are + // replaced with empty values. Fetch and merge when the caller only updates + // one field so untouched content is preserved. + if existingDoc == nil && (title == "" || content == "") { + var err error + existingDoc, err = app.Account().Documents().Get(cmd.Context(), itemID) + if err != nil { + return nil, err + } + } + + req := &basecamp.UpdateDocumentRequest{} + if existingDoc != nil { + req.Title = existingDoc.Title + req.Content = existingDoc.Content + } + + if title != "" { + req.Title = title + } + if content != "" { + docHTML := richtext.MarkdownToHTML(content) + var err error + docHTML, err = resolveLocalImages(cmd, app, docHTML) + if err != nil { + return nil, err + } + req.Content = docHTML + } + + return req, nil +} + func newFilesDownloadCmd(project *string) *cobra.Command { var outDir string diff --git a/internal/commands/files_test.go b/internal/commands/files_test.go index da3acf38..8741bee4 100644 --- a/internal/commands/files_test.go +++ b/internal/commands/files_test.go @@ -3,7 +3,11 @@ package commands import ( "bytes" "context" + "encoding/json" "errors" + "fmt" + "io" + "net/http" "strings" "testing" @@ -157,3 +161,86 @@ func TestFilesDownloadStdoutStreamsUploadID(t *testing.T) { assert.Equal(t, fileContent, stdout.String(), "upload body should be streamed directly to stdout") } + +type mockFilesUpdateTransport struct { + capturedBody []byte + requests []string +} + +func (t *mockFilesUpdateTransport) RoundTrip(req *http.Request) (*http.Response, error) { + t.requests = append(t.requests, req.Method+" "+req.URL.Path) + + header := make(http.Header) + header.Set("Content-Type", "application/json") + + switch { + case req.Method == http.MethodGet && strings.Contains(req.URL.Path, "/projects.json"): + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(strings.NewReader(`[{"id":456,"name":"Test Project"}]`)), + Header: header, + }, nil + case req.Method == http.MethodGet && strings.Contains(req.URL.Path, "/documents/999"): + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(strings.NewReader( + `{"id":999,"title":"Existing title","content":"
Existing body
","status":"active","bucket":{"id":456,"name":"Test Project","type":"Project"}}`, + )), + Header: header, + }, nil + case req.Method == http.MethodPut && strings.Contains(req.URL.Path, "/documents/999"): + if req.Body != nil { + body, err := io.ReadAll(req.Body) + if err != nil { + return nil, fmt.Errorf("reading request body: %w", err) + } + t.capturedBody = body + _ = req.Body.Close() + } + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(strings.NewReader( + `{"id":999,"title":"Updated title","content":"
Existing body
","status":"active","bucket":{"id":456,"name":"Test Project","type":"Project"}}`, + )), + Header: header, + }, nil + default: + return nil, fmt.Errorf("unexpected request: %s %s", req.Method, req.URL.Path) + } +} + +func TestFilesUpdateDocumentTitlePreservesExistingContent(t *testing.T) { + transport := &mockFilesUpdateTransport{} + app := showTestApp(t, transport) + app.Config.ProjectID = "456" + + cmd := NewFilesCmd() + err := executeMessagesCommand(cmd, app, "update", "999", "--type", "document", "--title", "Updated title") + require.NoError(t, err) + require.NotEmpty(t, transport.capturedBody) + + var body map[string]any + err = json.Unmarshal(transport.capturedBody, &body) + require.NoError(t, err) + + assert.Equal(t, "Updated title", body["title"]) + assert.Equal(t, "
Existing body
", body["content"]) +} + +func TestFilesUpdateDocumentContentPreservesExistingTitle(t *testing.T) { + transport := &mockFilesUpdateTransport{} + app := showTestApp(t, transport) + app.Config.ProjectID = "456" + + cmd := NewFilesCmd() + err := executeMessagesCommand(cmd, app, "update", "999", "--content", "Updated **body**") + require.NoError(t, err) + require.NotEmpty(t, transport.capturedBody) + + var body map[string]any + err = json.Unmarshal(transport.capturedBody, &body) + require.NoError(t, err) + + assert.Equal(t, "Existing title", body["title"]) + assert.Equal(t, "

Updated body

", body["content"]) +} diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index b29ab263..d192f5f2 100644 --- a/skills/basecamp/SKILL.md +++ b/skills/basecamp/SKILL.md @@ -533,8 +533,12 @@ basecamp files doc create "Doc" "Body" --in basecamp files doc create "Draft" --draft --in basecamp files doc create "Notes" "..." --no-subscribe --in basecamp files update --title "New" --content "Updated" +basecamp files update --title "New" --in # Preserves existing document content +basecamp files update --content "Updated" --in # Preserves existing document title ``` +**Document update semantics:** `basecamp files update ` is safe for partial updates in the CLI: when you pass only `--title` or only `--content`, the CLI first fetches the current document and preserves the untouched field. + **Subcommands:** `folders`, `uploads`, `documents` (each with pagination flags) ### Schedule From 3abd94fe1f7c63cf9ec1325a5d46e339044032ca Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 20 Apr 2026 10:47:11 -0400 Subject: [PATCH 2/6] Respect explicit empty document fields in files update --- internal/commands/files.go | 27 ++++++++++++------- internal/commands/files_test.go | 47 +++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 10 deletions(-) diff --git a/internal/commands/files.go b/internal/commands/files.go index 82e6f9be..95e31785 100644 --- a/internal/commands/files.go +++ b/internal/commands/files.go @@ -1216,7 +1216,9 @@ You can pass either an item ID or a Basecamp URL: Annotations: map[string]string{"agent_notes": "Document updates preserve untouched title/content by fetching current state first because Basecamp API clears omitted fields on PUT."}, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if strings.TrimSpace(title) == "" && strings.TrimSpace(content) == "" && itemType == "" { + titleChanged := cmd.Flags().Changed("title") + contentChanged := cmd.Flags().Changed("content") + if !titleChanged && !contentChanged { return noChanges(cmd) } @@ -1272,7 +1274,7 @@ You can pass either an item ID or a Basecamp URL: result = vault detectedType = "vault" case "document", "doc": - req, err := buildDocumentUpdateRequest(cmd, app, itemID, nil, title, content) + req, err := buildDocumentUpdateRequest(cmd, app, itemID, nil, titleChanged, contentChanged, title, content) if err != nil { return convertSDKError(err) } @@ -1307,7 +1309,7 @@ You can pass either an item ID or a Basecamp URL: // Try document first (most common update case) existingDoc, err := app.Account().Documents().Get(cmd.Context(), itemID) if err == nil { - req, buildErr := buildDocumentUpdateRequest(cmd, app, itemID, existingDoc, title, content) + req, buildErr := buildDocumentUpdateRequest(cmd, app, itemID, existingDoc, titleChanged, contentChanged, title, content) if buildErr != nil { return convertSDKError(buildErr) } @@ -1379,11 +1381,12 @@ You can pass either an item ID or a Basecamp URL: return cmd } -func buildDocumentUpdateRequest(cmd *cobra.Command, app *appctx.App, itemID int64, existingDoc *basecamp.Document, title, content string) (*basecamp.UpdateDocumentRequest, error) { - // BC3 document updates are destructive PUTs: omitted title/content fields are - // replaced with empty values. Fetch and merge when the caller only updates - // one field so untouched content is preserved. - if existingDoc == nil && (title == "" || content == "") { +func buildDocumentUpdateRequest(cmd *cobra.Command, app *appctx.App, itemID int64, existingDoc *basecamp.Document, titleChanged, contentChanged bool, title, content string) (*basecamp.UpdateDocumentRequest, error) { + // Basecamp document updates are destructive PUTs: omitted title/content + // fields are replaced with empty values. Fetch and merge when the caller + // updates only one field so the untouched field is preserved, while still + // allowing explicit clears via --title "" or --content "". + if existingDoc == nil && (!titleChanged || !contentChanged) { var err error existingDoc, err = app.Account().Documents().Get(cmd.Context(), itemID) if err != nil { @@ -1397,10 +1400,14 @@ func buildDocumentUpdateRequest(cmd *cobra.Command, app *appctx.App, itemID int6 req.Content = existingDoc.Content } - if title != "" { + if titleChanged { req.Title = title } - if content != "" { + if contentChanged { + if content == "" { + req.Content = "" + return req, nil + } docHTML := richtext.MarkdownToHTML(content) var err error docHTML, err = resolveLocalImages(cmd, app, docHTML) diff --git a/internal/commands/files_test.go b/internal/commands/files_test.go index 8741bee4..a1fc46f3 100644 --- a/internal/commands/files_test.go +++ b/internal/commands/files_test.go @@ -244,3 +244,50 @@ func TestFilesUpdateDocumentContentPreservesExistingTitle(t *testing.T) { assert.Equal(t, "Existing title", body["title"]) assert.Equal(t, "

Updated body

", body["content"]) } + +func TestFilesUpdateDocumentEmptyTitleClearsWhilePreservingContent(t *testing.T) { + transport := &mockFilesUpdateTransport{} + app := showTestApp(t, transport) + app.Config.ProjectID = "456" + + cmd := NewFilesCmd() + err := executeMessagesCommand(cmd, app, "update", "999", "--type", "document", "--title", "") + require.NoError(t, err) + require.NotEmpty(t, transport.capturedBody) + + var body map[string]any + err = json.Unmarshal(transport.capturedBody, &body) + require.NoError(t, err) + + _, hasTitle := body["title"] + assert.False(t, hasTitle) + assert.Equal(t, "
Existing body
", body["content"]) +} + +func TestFilesUpdateDocumentEmptyContentClearsWhilePreservingTitle(t *testing.T) { + transport := &mockFilesUpdateTransport{} + app := showTestApp(t, transport) + app.Config.ProjectID = "456" + + cmd := NewFilesCmd() + err := executeMessagesCommand(cmd, app, "update", "999", "--content", "") + require.NoError(t, err) + require.NotEmpty(t, transport.capturedBody) + + var body map[string]any + err = json.Unmarshal(transport.capturedBody, &body) + require.NoError(t, err) + + assert.Equal(t, "Existing title", body["title"]) + _, hasContent := body["content"] + assert.False(t, hasContent) +} + +func TestFilesUpdateTypeWithoutChangesShowsHelp(t *testing.T) { + app, _ := setupMessagesTestApp(t) + app.Config.ProjectID = "456" + + cmd := NewFilesCmd() + err := executeMessagesCommand(cmd, app, "update", "999", "--type", "document") + assert.NoError(t, err) +} From c5ca84a0179cc46f5dfdf980dc1a9c822b5e4d30 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 20 Apr 2026 11:24:23 -0400 Subject: [PATCH 3/6] Validate files update fields by item type --- internal/commands/files.go | 24 ++++++++++++++++++++++-- internal/commands/files_test.go | 22 ++++++++++++++++++++++ skills/basecamp/SKILL.md | 6 +++--- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/internal/commands/files.go b/internal/commands/files.go index 95e31785..fb06328d 100644 --- a/internal/commands/files.go +++ b/internal/commands/files.go @@ -1218,8 +1218,28 @@ You can pass either an item ID or a Basecamp URL: RunE: func(cmd *cobra.Command, args []string) error { titleChanged := cmd.Flags().Changed("title") contentChanged := cmd.Flags().Changed("content") - if !titleChanged && !contentChanged { - return noChanges(cmd) + itemType = strings.ToLower(strings.TrimSpace(itemType)) + switch itemType { + case "", "document", "doc": + if !titleChanged && !contentChanged { + return noChanges(cmd) + } + case "vault", "folder": + if contentChanged { + return output.ErrUsage("--content can only be used with --type document or upload") + } + if !titleChanged { + return noChanges(cmd) + } + case "upload", "file": + if !titleChanged && !contentChanged { + return noChanges(cmd) + } + default: + return output.ErrUsageHint( + fmt.Sprintf("Invalid type: %s", itemType), + "Use: vault, document, or upload", + ) } app := appctx.FromContext(cmd.Context()) diff --git a/internal/commands/files_test.go b/internal/commands/files_test.go index a1fc46f3..b5b0e778 100644 --- a/internal/commands/files_test.go +++ b/internal/commands/files_test.go @@ -291,3 +291,25 @@ func TestFilesUpdateTypeWithoutChangesShowsHelp(t *testing.T) { err := executeMessagesCommand(cmd, app, "update", "999", "--type", "document") assert.NoError(t, err) } + +func TestFilesUpdateVaultRejectsContentFlag(t *testing.T) { + app, _ := setupMessagesTestApp(t) + app.Config.ProjectID = "456" + + cmd := NewFilesCmd() + err := executeMessagesCommand(cmd, app, "update", "999", "--type", "vault", "--content", "desc") + require.Error(t, err) + + var e *output.Error + require.True(t, errors.As(err, &e), "expected *output.Error, got %T: %v", err, err) + assert.Contains(t, e.Message, "--content can only be used with --type document or upload") +} + +func TestFilesUpdateVaultWithoutTitleShowsHelp(t *testing.T) { + app, _ := setupMessagesTestApp(t) + app.Config.ProjectID = "456" + + cmd := NewFilesCmd() + err := executeMessagesCommand(cmd, app, "update", "999", "--type", "vault") + assert.NoError(t, err) +} diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index d192f5f2..0aaa8599 100644 --- a/skills/basecamp/SKILL.md +++ b/skills/basecamp/SKILL.md @@ -532,9 +532,9 @@ basecamp files folder create "Folder" --in basecamp files doc create "Doc" "Body" --in basecamp files doc create "Draft" --draft --in basecamp files doc create "Notes" "..." --no-subscribe --in -basecamp files update --title "New" --content "Updated" -basecamp files update --title "New" --in # Preserves existing document content -basecamp files update --content "Updated" --in # Preserves existing document title +basecamp files update --title "New" --content "Updated" +basecamp files update --title "New" --in # Preserves existing document content +basecamp files update --content "Updated" --in # Preserves existing document title ``` **Document update semantics:** `basecamp files update ` is safe for partial updates in the CLI: when you pass only `--title` or only `--content`, the CLI first fetches the current document and preserves the untouched field. From 657e20d01bca41f62287999df6197b3f14670c8a Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 20 Apr 2026 11:38:04 -0400 Subject: [PATCH 4/6] Reject content-only vault updates in autodetect mode --- internal/commands/files.go | 3 +++ internal/commands/files_test.go | 45 +++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/internal/commands/files.go b/internal/commands/files.go index fb06328d..af7858b6 100644 --- a/internal/commands/files.go +++ b/internal/commands/files.go @@ -1344,6 +1344,9 @@ You can pass either an item ID or a Basecamp URL: // Try vault _, err = app.Account().Vaults().Get(cmd.Context(), itemID) if err == nil { + if contentChanged { + return output.ErrUsage("detected a folder/vault; use --title to rename it") + } req := &basecamp.UpdateVaultRequest{Title: title} vault, err := app.Account().Vaults().Update(cmd.Context(), itemID, req) if err != nil { diff --git a/internal/commands/files_test.go b/internal/commands/files_test.go index b5b0e778..67dee2f6 100644 --- a/internal/commands/files_test.go +++ b/internal/commands/files_test.go @@ -313,3 +313,48 @@ func TestFilesUpdateVaultWithoutTitleShowsHelp(t *testing.T) { err := executeMessagesCommand(cmd, app, "update", "999", "--type", "vault") assert.NoError(t, err) } + +type mockFilesAutodetectVaultTransport struct{} + +func (t *mockFilesAutodetectVaultTransport) RoundTrip(req *http.Request) (*http.Response, error) { + header := make(http.Header) + header.Set("Content-Type", "application/json") + + switch { + case req.Method == http.MethodGet && strings.Contains(req.URL.Path, "/projects.json"): + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(strings.NewReader(`[{"id":456,"name":"Test Project"}]`)), + Header: header, + }, nil + case req.Method == http.MethodGet && strings.Contains(req.URL.Path, "/documents/999"): + return &http.Response{ + StatusCode: 404, + Body: io.NopCloser(strings.NewReader(`{"error":"not found"}`)), + Header: header, + }, nil + case req.Method == http.MethodGet && strings.Contains(req.URL.Path, "/vaults/999"): + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(strings.NewReader(`{"id":999,"title":"Existing folder"}`)), + Header: header, + }, nil + case req.Method == http.MethodPut: + return nil, fmt.Errorf("unexpected update request: %s", req.URL.Path) + default: + return nil, fmt.Errorf("unexpected request: %s %s", req.Method, req.URL.Path) + } +} + +func TestFilesUpdateAutodetectVaultRejectsContentOnly(t *testing.T) { + app := showTestApp(t, &mockFilesAutodetectVaultTransport{}) + app.Config.ProjectID = "456" + + cmd := NewFilesCmd() + err := executeMessagesCommand(cmd, app, "update", "999", "--content", "desc") + require.Error(t, err) + + var e *output.Error + require.True(t, errors.As(err, &e), "expected *output.Error, got %T: %v", err, err) + assert.Contains(t, e.Message, "detected a folder/vault; use --title to rename it") +} From e3c708c93edced8e3ad205824e2f95397c497b6f Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 23 Apr 2026 16:11:13 -0700 Subject: [PATCH 5/6] Reject empty upload/vault fields in files update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Explicit-empty flags (e.g. `--title ""`) are a document-only "clear" semantic because BC3 rebuilds documents from permitted params. Upload and vault updates don't clear by omission — the SDK strips empty strings, so an empty-valued flag produced a no-op PUT reported as success. Reject empty values CLI-side in both the typed validation switch and the autodetect upload/vault branches. Document the layered dependency on SDK empty-string stripping in the document helper's docstring and agent annotation. --- internal/commands/files.go | 28 ++++++++--- internal/commands/files_test.go | 83 +++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 7 deletions(-) diff --git a/internal/commands/files.go b/internal/commands/files.go index af7858b6..4fa08f9a 100644 --- a/internal/commands/files.go +++ b/internal/commands/files.go @@ -1213,7 +1213,7 @@ For documents, updating only --title or only --content preserves the untouched f You can pass either an item ID or a Basecamp URL: basecamp files update 789 --title "new title" --in my-project basecamp files update 789 --content "new content" --in my-project`, - Annotations: map[string]string{"agent_notes": "Document updates preserve untouched title/content by fetching current state first because Basecamp API clears omitted fields on PUT."}, + Annotations: map[string]string{"agent_notes": "Document updates preserve untouched title/content by fetching current state first because BC3 rebuilds documents from permitted params on PUT; explicit clears via --title \"\"/--content \"\" work because the SDK strips empty strings to absent fields, which the controller then nulls. Upload/vault updates do not clear by omission, so empty-valued flags are rejected CLI-side."}, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { titleChanged := cmd.Flags().Changed("title") @@ -1228,11 +1228,13 @@ You can pass either an item ID or a Basecamp URL: if contentChanged { return output.ErrUsage("--content can only be used with --type document or upload") } - if !titleChanged { + if !titleChanged || title == "" { return noChanges(cmd) } case "upload", "file": - if !titleChanged && !contentChanged { + hasTitle := titleChanged && title != "" + hasContent := contentChanged && content != "" + if !hasTitle && !hasContent { return noChanges(cmd) } default: @@ -1347,6 +1349,9 @@ You can pass either an item ID or a Basecamp URL: if contentChanged { return output.ErrUsage("detected a folder/vault; use --title to rename it") } + if !titleChanged || title == "" { + return noChanges(cmd) + } req := &basecamp.UpdateVaultRequest{Title: title} vault, err := app.Account().Vaults().Update(cmd.Context(), itemID, req) if err != nil { @@ -1358,6 +1363,11 @@ You can pass either an item ID or a Basecamp URL: // Try upload _, err = app.Account().Uploads().Get(cmd.Context(), itemID) if err == nil { + hasTitle := titleChanged && title != "" + hasContent := contentChanged && content != "" + if !hasTitle && !hasContent { + return noChanges(cmd) + } req := &basecamp.UpdateUploadRequest{Description: content} if title != "" { req.BaseName = title @@ -1405,10 +1415,14 @@ You can pass either an item ID or a Basecamp URL: } func buildDocumentUpdateRequest(cmd *cobra.Command, app *appctx.App, itemID int64, existingDoc *basecamp.Document, titleChanged, contentChanged bool, title, content string) (*basecamp.UpdateDocumentRequest, error) { - // Basecamp document updates are destructive PUTs: omitted title/content - // fields are replaced with empty values. Fetch and merge when the caller - // updates only one field so the untouched field is preserved, while still - // allowing explicit clears via --title "" or --content "". + // BC3 rebuilds documents from permitted params on PUT, so omitted + // title/content fields are replaced with empty values. Fetch and merge when + // the caller updates only one field so the untouched field is preserved. + // + // Explicit clears via --title "" or --content "" work by composition: the + // SDK strips empty strings to absent JSON fields, and the controller then + // nulls those absent fields during rebuild. The wire-shape assertion in + // TestFilesUpdateDocumentEmptyTitleClearsWhilePreservingContent pins this. if existingDoc == nil && (!titleChanged || !contentChanged) { var err error existingDoc, err = app.Account().Documents().Get(cmd.Context(), itemID) diff --git a/internal/commands/files_test.go b/internal/commands/files_test.go index 67dee2f6..594ec4df 100644 --- a/internal/commands/files_test.go +++ b/internal/commands/files_test.go @@ -358,3 +358,86 @@ func TestFilesUpdateAutodetectVaultRejectsContentOnly(t *testing.T) { require.True(t, errors.As(err, &e), "expected *output.Error, got %T: %v", err, err) assert.Contains(t, e.Message, "detected a folder/vault; use --title to rename it") } + +func TestFilesUpdateTypedVaultEmptyTitleNoChanges(t *testing.T) { + app, _ := setupMessagesTestApp(t) + app.Config.ProjectID = "456" + + cmd := NewFilesCmd() + err := executeMessagesCommand(cmd, app, "update", "999", "--type", "vault", "--title", "") + assert.NoError(t, err) +} + +func TestFilesUpdateTypedUploadEmptyTitleNoChanges(t *testing.T) { + app, _ := setupMessagesTestApp(t) + app.Config.ProjectID = "456" + + cmd := NewFilesCmd() + err := executeMessagesCommand(cmd, app, "update", "999", "--type", "upload", "--title", "") + assert.NoError(t, err) +} + +func TestFilesUpdateTypedUploadEmptyContentNoChanges(t *testing.T) { + app, _ := setupMessagesTestApp(t) + app.Config.ProjectID = "456" + + cmd := NewFilesCmd() + err := executeMessagesCommand(cmd, app, "update", "999", "--type", "upload", "--content", "") + assert.NoError(t, err) +} + +func TestFilesUpdateAutodetectVaultEmptyTitleNoChanges(t *testing.T) { + app := showTestApp(t, &mockFilesAutodetectVaultTransport{}) + app.Config.ProjectID = "456" + + cmd := NewFilesCmd() + err := executeMessagesCommand(cmd, app, "update", "999", "--title", "") + assert.NoError(t, err) +} + +type mockFilesAutodetectUploadTransport struct{} + +func (t *mockFilesAutodetectUploadTransport) RoundTrip(req *http.Request) (*http.Response, error) { + header := make(http.Header) + header.Set("Content-Type", "application/json") + + switch { + case req.Method == http.MethodGet && strings.Contains(req.URL.Path, "/projects.json"): + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(strings.NewReader(`[{"id":456,"name":"Test Project"}]`)), + Header: header, + }, nil + case req.Method == http.MethodGet && strings.Contains(req.URL.Path, "/documents/999"): + return &http.Response{ + StatusCode: 404, + Body: io.NopCloser(strings.NewReader(`{"error":"not found"}`)), + Header: header, + }, nil + case req.Method == http.MethodGet && strings.Contains(req.URL.Path, "/vaults/999"): + return &http.Response{ + StatusCode: 404, + Body: io.NopCloser(strings.NewReader(`{"error":"not found"}`)), + Header: header, + }, nil + case req.Method == http.MethodGet && strings.Contains(req.URL.Path, "/uploads/999"): + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(strings.NewReader(`{"id":999,"filename":"report.pdf"}`)), + Header: header, + }, nil + case req.Method == http.MethodPut: + return nil, fmt.Errorf("unexpected update request: %s", req.URL.Path) + default: + return nil, fmt.Errorf("unexpected request: %s %s", req.Method, req.URL.Path) + } +} + +func TestFilesUpdateAutodetectUploadEmptyContentNoChanges(t *testing.T) { + app := showTestApp(t, &mockFilesAutodetectUploadTransport{}) + app.Config.ProjectID = "456" + + cmd := NewFilesCmd() + err := executeMessagesCommand(cmd, app, "update", "999", "--content", "") + assert.NoError(t, err) +} From 169f34bfaf9f57294c83011a6661da8887c8f537 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 23 Apr 2026 17:14:32 -0700 Subject: [PATCH 6/6] Treat whitespace-only files update values as no-op Match the repo convention (messages.go:438) of treating trimmed-empty --title/--content as no-changes. Previously, `--title " "` on a vault or upload would pass validation and send a whitespace title/description to the API. For documents, preserve the explicit-clear behavior when the value is exactly "" while still rejecting whitespace-only as a no-op. Apply the same trimmed semantics in the autodetect vault and upload branches. --- internal/commands/files.go | 19 ++++++++++++------- internal/commands/files_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/internal/commands/files.go b/internal/commands/files.go index 4fa08f9a..04f61fcd 100644 --- a/internal/commands/files.go +++ b/internal/commands/files.go @@ -1218,22 +1218,27 @@ You can pass either an item ID or a Basecamp URL: RunE: func(cmd *cobra.Command, args []string) error { titleChanged := cmd.Flags().Changed("title") contentChanged := cmd.Flags().Changed("content") + titleTrimmed := strings.TrimSpace(title) + contentTrimmed := strings.TrimSpace(content) itemType = strings.ToLower(strings.TrimSpace(itemType)) switch itemType { case "", "document", "doc": - if !titleChanged && !contentChanged { + // Explicit "" clears are valid for documents; whitespace-only is not. + hasTitle := titleChanged && (title == "" || titleTrimmed != "") + hasContent := contentChanged && (content == "" || contentTrimmed != "") + if !hasTitle && !hasContent { return noChanges(cmd) } case "vault", "folder": if contentChanged { return output.ErrUsage("--content can only be used with --type document or upload") } - if !titleChanged || title == "" { + if !titleChanged || titleTrimmed == "" { return noChanges(cmd) } case "upload", "file": - hasTitle := titleChanged && title != "" - hasContent := contentChanged && content != "" + hasTitle := titleChanged && titleTrimmed != "" + hasContent := contentChanged && contentTrimmed != "" if !hasTitle && !hasContent { return noChanges(cmd) } @@ -1349,7 +1354,7 @@ You can pass either an item ID or a Basecamp URL: if contentChanged { return output.ErrUsage("detected a folder/vault; use --title to rename it") } - if !titleChanged || title == "" { + if !titleChanged || titleTrimmed == "" { return noChanges(cmd) } req := &basecamp.UpdateVaultRequest{Title: title} @@ -1363,8 +1368,8 @@ You can pass either an item ID or a Basecamp URL: // Try upload _, err = app.Account().Uploads().Get(cmd.Context(), itemID) if err == nil { - hasTitle := titleChanged && title != "" - hasContent := contentChanged && content != "" + hasTitle := titleChanged && titleTrimmed != "" + hasContent := contentChanged && contentTrimmed != "" if !hasTitle && !hasContent { return noChanges(cmd) } diff --git a/internal/commands/files_test.go b/internal/commands/files_test.go index 594ec4df..e6fec86c 100644 --- a/internal/commands/files_test.go +++ b/internal/commands/files_test.go @@ -441,3 +441,30 @@ func TestFilesUpdateAutodetectUploadEmptyContentNoChanges(t *testing.T) { err := executeMessagesCommand(cmd, app, "update", "999", "--content", "") assert.NoError(t, err) } + +func TestFilesUpdateTypedVaultWhitespaceTitleNoChanges(t *testing.T) { + app, _ := setupMessagesTestApp(t) + app.Config.ProjectID = "456" + + cmd := NewFilesCmd() + err := executeMessagesCommand(cmd, app, "update", "999", "--type", "vault", "--title", " ") + assert.NoError(t, err) +} + +func TestFilesUpdateTypedUploadWhitespaceContentNoChanges(t *testing.T) { + app, _ := setupMessagesTestApp(t) + app.Config.ProjectID = "456" + + cmd := NewFilesCmd() + err := executeMessagesCommand(cmd, app, "update", "999", "--type", "upload", "--content", " ") + assert.NoError(t, err) +} + +func TestFilesUpdateTypedDocumentWhitespaceTitleNoChanges(t *testing.T) { + app, _ := setupMessagesTestApp(t) + app.Config.ProjectID = "456" + + cmd := NewFilesCmd() + err := executeMessagesCommand(cmd, app, "update", "999", "--type", "document", "--title", " ") + assert.NoError(t, err) +}