diff --git a/.surface b/.surface index 1bfc459b8..84f0339bb 100644 --- a/.surface +++ b/.surface @@ -30,6 +30,8 @@ ARG basecamp campfire delete 00 ARG basecamp campfire line 00 ARG basecamp campfire post 00 ARG basecamp campfire show 00 +ARG basecamp campfire update 00 +ARG basecamp campfire update 01 [content] ARG basecamp campfire upload 00 ARG basecamp cards archive 00 ARG basecamp cards column color 00 @@ -62,6 +64,8 @@ ARG basecamp chat delete 00 ARG basecamp chat line 00 ARG basecamp chat post 00 ARG basecamp chat show 00 +ARG basecamp chat update 00 +ARG basecamp chat update 01 [content] ARG basecamp chat upload 00 ARG basecamp checkin answer 00 ARG basecamp checkin answer create 00 @@ -467,6 +471,7 @@ CMD basecamp campfire list CMD basecamp campfire messages CMD basecamp campfire post CMD basecamp campfire show +CMD basecamp campfire update CMD basecamp campfire upload CMD basecamp cards CMD basecamp cards archive @@ -505,6 +510,7 @@ CMD basecamp chat list CMD basecamp chat messages CMD basecamp chat post CMD basecamp chat show +CMD basecamp chat update CMD basecamp chat upload CMD basecamp checkin CMD basecamp checkin answer @@ -2343,6 +2349,30 @@ FLAG basecamp campfire show --stats type=bool FLAG basecamp campfire show --styled type=bool FLAG basecamp campfire show --todolist type=string FLAG basecamp campfire show --verbose type=count +FLAG basecamp campfire update --account type=string +FLAG basecamp campfire update --agent type=bool +FLAG basecamp campfire update --cache-dir type=string +FLAG basecamp campfire update --content type=string +FLAG basecamp campfire update --content-type type=string +FLAG basecamp campfire update --count type=bool +FLAG basecamp campfire update --help type=bool +FLAG basecamp campfire update --hints type=bool +FLAG basecamp campfire update --ids-only type=bool +FLAG basecamp campfire update --in type=string +FLAG basecamp campfire update --jq type=string +FLAG basecamp campfire update --json type=bool +FLAG basecamp campfire update --markdown type=bool +FLAG basecamp campfire update --md type=bool +FLAG basecamp campfire update --no-hints type=bool +FLAG basecamp campfire update --no-stats type=bool +FLAG basecamp campfire update --profile type=string +FLAG basecamp campfire update --project type=string +FLAG basecamp campfire update --quiet type=bool +FLAG basecamp campfire update --room type=string +FLAG basecamp campfire update --stats type=bool +FLAG basecamp campfire update --styled type=bool +FLAG basecamp campfire update --todolist type=string +FLAG basecamp campfire update --verbose type=count FLAG basecamp campfire upload --account type=string FLAG basecamp campfire upload --agent type=bool FLAG basecamp campfire upload --cache-dir type=string @@ -3233,6 +3263,30 @@ FLAG basecamp chat show --stats type=bool FLAG basecamp chat show --styled type=bool FLAG basecamp chat show --todolist type=string FLAG basecamp chat show --verbose type=count +FLAG basecamp chat update --account type=string +FLAG basecamp chat update --agent type=bool +FLAG basecamp chat update --cache-dir type=string +FLAG basecamp chat update --content type=string +FLAG basecamp chat update --content-type type=string +FLAG basecamp chat update --count type=bool +FLAG basecamp chat update --help type=bool +FLAG basecamp chat update --hints type=bool +FLAG basecamp chat update --ids-only type=bool +FLAG basecamp chat update --in type=string +FLAG basecamp chat update --jq type=string +FLAG basecamp chat update --json type=bool +FLAG basecamp chat update --markdown type=bool +FLAG basecamp chat update --md type=bool +FLAG basecamp chat update --no-hints type=bool +FLAG basecamp chat update --no-stats type=bool +FLAG basecamp chat update --profile type=string +FLAG basecamp chat update --project type=string +FLAG basecamp chat update --quiet type=bool +FLAG basecamp chat update --room type=string +FLAG basecamp chat update --stats type=bool +FLAG basecamp chat update --styled type=bool +FLAG basecamp chat update --todolist type=string +FLAG basecamp chat update --verbose type=count FLAG basecamp chat upload --account type=string FLAG basecamp chat upload --agent type=bool FLAG basecamp chat upload --cache-dir type=string @@ -16037,6 +16091,7 @@ SUB basecamp campfire list SUB basecamp campfire messages SUB basecamp campfire post SUB basecamp campfire show +SUB basecamp campfire update SUB basecamp campfire upload SUB basecamp cards SUB basecamp cards archive @@ -16075,6 +16130,7 @@ SUB basecamp chat list SUB basecamp chat messages SUB basecamp chat post SUB basecamp chat show +SUB basecamp chat update SUB basecamp chat upload SUB basecamp checkin SUB basecamp checkin answer diff --git a/API-COVERAGE.md b/API-COVERAGE.md index 72bcf6f76..70acb4411 100644 --- a/API-COVERAGE.md +++ b/API-COVERAGE.md @@ -16,7 +16,7 @@ Out-of-scope sections are excluded from parity totals and scripts: chatbots (dif > 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.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. +**SDK version:** v0.8.0 — adds `Campfires.UpdateLine` (`PUT /chats/{c}/lines/{l}`, basecamp/basecamp-sdk#295), consumed by `chat update`; also carries the BC5 `Tools.Create` create-by-type reshape. API date advanced to 2026-07-22. ## Coverage by Section @@ -38,7 +38,7 @@ The **Since** column tags each row with the Basecamp version that introduced its | messages | 10 | `messages`, `message` | ✅ | BC4 | - | list, show, create, update, publish, pin, unpin. Create supports `--subscribe`/`--no-subscribe` and `--draft`. Publish promotes drafts to active | | message_boards | 3 | `messageboards` | ✅ | BC4 | - | Container, accessed via project dock | | message_types | 5 | `messagetypes` | ✅ | BC4 | - | list, show, create, update, delete | -| campfires | 14 | `chat` | ✅ | BC4 | - | list, messages, post, line show/delete. @mentions in content | +| campfires | 14 | `chat` | ✅ | BC4 | - | list, messages, post, line show/update/delete. @mentions in content | | comments | 8 | `comment`, `comments` | ✅ | BC4 | - | list, show, create, update. @mentions in content | | boosts | 6 | `boost`, `react` | ✅ | BC4 | - | list (recording + event), show, create (recording + event), delete | | notifications | 2 | `notifications` | ✅ | BC4 | - | list, mark as read (BC5: `bubble_ups`/`scheduled_bubble_ups` sections; `memories` is BC4-only) | diff --git a/e2e/chat.bats b/e2e/chat.bats index ace7a0b03..9d435ca28 100644 --- a/e2e/chat.bats +++ b/e2e/chat.bats @@ -95,6 +95,19 @@ load test_helper assert_output_contains "ID required" } +@test "chat update without args shows error" { + create_credentials + create_global_config '{"account_id": 99999, "project_id": 123}' + + run basecamp chat update + assert_failure + # Command-specific arg name (JSON-escapes < > to < >, so match the + # inner "id|url"), and never the generic "Todo ID(s) required" that root.go + # would rewrite a Cobra arg-count error into. + assert_output_contains "id|url" + assert_output_not_contains "Todo ID(s)" +} + # Help flag diff --git a/e2e/smoke/smoke_campfire.bats b/e2e/smoke/smoke_campfire.bats index 50891387d..db0a24a58 100644 --- a/e2e/smoke/smoke_campfire.bats +++ b/e2e/smoke/smoke_campfire.bats @@ -44,6 +44,28 @@ setup_file() { assert_json_not_null '.data.id' } +@test "campfire update edits a message" { + local id_file="$BATS_FILE_TMPDIR/campfire_line_id" + [[ -f "$id_file" ]] || mark_unverifiable "No campfire line created in prior test" + local line_id new_content + line_id=$(<"$id_file") + new_content="Edited smoke test $(date +%s)" + + run_smoke basecamp campfire update "$line_id" "$new_content" \ + --room "$QA_CAMPFIRE" -p "$QA_PROJECT" --json + assert_success + assert_json_value '.ok' 'true' + assert_json_not_null '.data.id' + + # Re-fetch the line and verify its content actually changed (guards against + # a no-op update silently passing). + run_smoke basecamp campfire line "$line_id" \ + --room "$QA_CAMPFIRE" -p "$QA_PROJECT" --json + assert_success + echo "$output" | jq -e --arg expected "$new_content" '.data.content | contains($expected)' >/dev/null \ + || fail "expected updated line content to contain '$new_content', got: $(echo "$output" | jq -r '.data.content')" +} + @test "campfire delete deletes a message" { local id_file="$BATS_FILE_TMPDIR/campfire_line_id" [[ -f "$id_file" ]] || mark_unverifiable "No campfire line created in prior test" diff --git a/go.mod b/go.mod index ae7db6fcf..0e7c5fe29 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.20260722082856-f2086f5e57f6 + github.com/basecamp/basecamp-sdk/go v0.8.0 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 a0231d21d..6bf9c4bc9 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.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/basecamp-sdk/go v0.8.0 h1:ZxrNTyGTMcGocGE582ZMmpjyodvODD2+fZMjISokTYY= +github.com/basecamp/basecamp-sdk/go v0.8.0/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/chat.go b/internal/commands/chat.go index 5a8e1d770..47f765353 100644 --- a/internal/commands/chat.go +++ b/internal/commands/chat.go @@ -12,9 +12,11 @@ import ( "github.com/spf13/cobra" "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/hostutil" "github.com/basecamp/basecamp-cli/internal/output" "github.com/basecamp/basecamp-cli/internal/richtext" "github.com/basecamp/basecamp-cli/internal/tui" + "github.com/basecamp/basecamp-cli/internal/urlarg" ) // NewChatCmd creates the chat command for real-time chat. @@ -44,6 +46,7 @@ Use 'basecamp chat post "message"' to post a message.`, newChatPostCmd(&project, &chatID, &contentType), newChatUploadCmd(&project, &chatID), newChatLineShowCmd(&project, &chatID), + newChatLineUpdateCmd(&project, &chatID, &contentType), newChatLineDeleteCmd(&project, &chatID), ) @@ -733,6 +736,234 @@ You can pass either a line ID or a Basecamp line URL: return cmd } +func newChatLineUpdateCmd(project, chatID, contentType *string) *cobra.Command { + var content string + + cmd := &cobra.Command{ + Use: "update [content]", + Short: "Update an existing message", + Long: `Update the content of an existing chat message. + +You can pass either a line ID or a Basecamp line URL: + basecamp chat update 789 "edited message" --in my-project + basecamp chat update https://3.basecamp.com/123/buckets/456/chats/789/lines/111 --content "edited" + +Chat edits are always stored as rich text. By default, content is treated as +Markdown and converted to HTML, and @mentions resolve like 'chat post'. Use +--content-type text/html to supply HTML directly, or --content-type text/plain +to send the text verbatim (escaped, line breaks preserved, no mention +resolution). --content-type is applied locally — the server always coerces the +edit to rich text.`, + // Accept 0–2 args so a missing yields a purpose-specific usage + // error from RunE, rather than Cobra's generic "requires at least" (which + // root.go rewrites into a misleading "Todo ID(s) required"). Mirrors + // chat post's MaximumNArgs handling. + Args: cobra.MaximumNArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + + if len(args) == 0 { + return missingArg(cmd, "") + } + + messageContent := content + if len(args) > 1 { + messageContent = args[1] + } + + if strings.TrimSpace(messageContent) == "" { + return missingArg(cmd, "") + } + + // Validate the content mode before any request or account setup so an + // unknown --content-type fails fast rather than silently sending raw + // bytes (the SDK no longer validates content type for us). + ct := *contentType + switch ct { + case "", "text/html", "text/plain": + default: + return output.ErrUsage(fmt.Sprintf("unsupported --content-type %q (expected text/html or text/plain)", ct)) + } + + if err := ensureAccount(cmd, app); err != nil { + return err + } + + // Resolve the line reference. A bare numeric ID falls through to + // --room/dock resolution; a URL must be a chat-line URL on a trusted + // host so a pasted card/todo/message URL — or a look-alike on an + // attacker-controlled host — can't be misinterpreted into an edit. + lineID := args[0] + urlChatID := "" + urlProjectID := "" + if urlarg.IsURL(args[0]) { + if !hostutil.IsTrustedBasecampHost(args[0], app.Config.BaseURL) { + return output.ErrUsage("refusing untrusted host in URL — expected a Basecamp URL") + } + parsed := urlarg.Parse(args[0]) + // Require an individual chat-line URL. A room's line-collection URL + // (/chats/{c}/lines) also parses as Type "lines" but with + // IsCollection set and RecordingID holding the campfire ID, not a + // line ID — accepting it would edit PUT /chats/{c}/lines/{c}. + if parsed == nil || parsed.Type != "lines" || parsed.IsCollection { + return output.ErrUsage("expected a chat-line ID or URL of the form /chats/{c}/lines/{l} or /chats/{c}@{l}") + } + // Guard against editing in the wrong account: the URL names an + // account, and if it disagrees with the configured one the safe + // move is to stop rather than silently target a different account. + if parsed.AccountID != "" && app.Config.AccountID != "" && parsed.AccountID != app.Config.AccountID { + return output.ErrUsage(fmt.Sprintf("URL account %s does not match the configured account %s", parsed.AccountID, app.Config.AccountID)) + } + lineID = parsed.RecordingID + urlChatID = parsed.CampfireID + urlProjectID = parsed.ProjectID + } + + // Resolve the chat (campfire) ID, and a project only when needed. The + // PUT needs just the chat + line IDs; a project is required only to + // discover the default room (no URL chat and no --room) and is + // otherwise resolved opportunistically for breadcrumbs — never by + // prompting. URL-derived values win over --room/--in/--project, which + // may be stale from a previous command in the same shell. + effectiveChatID := urlChatID + if effectiveChatID == "" { + effectiveChatID = *chatID + } + + projectHint := urlProjectID + if projectHint == "" { + projectHint = *project + } + if projectHint == "" { + projectHint = app.Flags.Project + } + if projectHint == "" { + projectHint = app.Config.ProjectID + } + + var resolvedProjectID string + var err error + if effectiveChatID == "" { + // No room in hand — resolve a project (prompting as a last resort) + // so we can discover the project's default chat. + if projectHint == "" { + if err = ensureProject(cmd, app); err != nil { + return err + } + projectHint = app.Config.ProjectID + } + resolvedProjectID, _, err = app.Names.ResolveProject(cmd.Context(), projectHint) + if err != nil { + return err + } + effectiveChatID, err = getChatID(cmd, app, resolvedProjectID) + if err != nil { + return err + } + } + // When the room is already known (--room or a URL), the project is + // needed only for breadcrumbs — resolved best-effort *after* the edit + // (below) so a stale/unreachable default project can't fail the PUT. + + chatIDInt, err := strconv.ParseInt(effectiveChatID, 10, 64) + if err != nil { + return output.ErrUsage("Invalid chat room ID") + } + lineIDInt, err := strconv.ParseInt(lineID, 10, 64) + if err != nil { + return output.ErrUsage("Invalid line ID") + } + + // Resolve the content into the exact rich-text body we PUT. Chat edits + // always render as rich text server-side, so: + // - unset (default): Markdown → HTML unconditionally, then resolve + // @mentions (mirrors messages.go, which converts unconditionally); + // - text/html: use the supplied HTML as-is, then resolve @mentions; + // - text/plain: serialize literally (escape + preserve breaks) and + // skip mention resolution. + var mentionNotice string + switch ct { + case "text/plain": + messageContent = richtext.PlainToHTML(messageContent) + default: + if ct == "" { + messageContent = richtext.MarkdownToHTML(messageContent) + } + result, resolveErr := resolveMentions(cmd.Context(), app.Names, messageContent) + if resolveErr != nil { + return resolveErr + } + messageContent = result.HTML + mentionNotice = unresolvedMentionWarning(result.Unresolved) + } + + if err := app.Account().Campfires().UpdateLine(cmd.Context(), chatIDInt, lineIDInt, messageContent); err != nil { + return convertSDKError(err) + } + + // SDK PUT returns 204; re-fetch so the response carries the canonical + // post-update line. A failure here doesn't roll back the update — we + // surface it as a diagnostic rather than the command's exit code. + line, fetchErr := app.Account().Campfires().GetLine(cmd.Context(), chatIDInt, lineIDInt) + + // Resolve the project for breadcrumbs only, best-effort: the edit has + // already been sent using just the chat + line IDs, so a stale or + // unreachable default project must not fail the command — it only + // drops --in from the breadcrumbs. + if resolvedProjectID == "" && projectHint != "" { + if rp, _, perr := app.Names.ResolveProject(cmd.Context(), projectHint); perr == nil { + resolvedProjectID = rp + } + } + + // Include --in only when a project was resolved (mirrors post/upload, + // which omit it when editing via --room without a project). + showCmd := fmt.Sprintf("basecamp chat line %s --room %s", lineID, effectiveChatID) + messagesCmd := fmt.Sprintf("basecamp chat messages --room %s", effectiveChatID) + if resolvedProjectID != "" { + showCmd = fmt.Sprintf("basecamp chat line %s --room %s --in %s", lineID, effectiveChatID, resolvedProjectID) + messagesCmd = fmt.Sprintf("basecamp chat messages --room %s --in %s", effectiveChatID, resolvedProjectID) + } + + respOpts := []output.ResponseOption{ + output.WithSummary(fmt.Sprintf("Updated line #%s", lineID)), + output.WithEntity("chat_line"), + output.WithBreadcrumbs( + output.Breadcrumb{ + Action: "show", + Cmd: showCmd, + Description: "View line", + }, + output.Breadcrumb{ + Action: "messages", + Cmd: messagesCmd, + Description: "Back to messages", + }, + ), + } + if line != nil { + respOpts = append(respOpts, output.WithDisplayData(chatLineDisplayData(line))) + } + if mentionNotice != "" { + respOpts = append(respOpts, output.WithDiagnostic(mentionNotice)) + } + if fetchErr != nil { + respOpts = append(respOpts, output.WithDiagnostic(fmt.Sprintf("update succeeded; refetch failed: %v", fetchErr))) + } + + if line == nil { + return app.OK(map[string]any{"updated": true, "id": lineID}, respOpts...) + } + return app.OK(line, respOpts...) + }, + } + + cmd.Flags().StringVar(&content, "content", "", "New message content") + cmd.Flags().StringVar(contentType, "content-type", "", "Input handling: text/html (supply HTML) or text/plain (verbatim); applied locally, edits always render as rich text") + + return cmd +} + func newChatLineDeleteCmd(project, chatID *string) *cobra.Command { var force bool diff --git a/internal/commands/chat_test.go b/internal/commands/chat_test.go index 27c2e8dea..f58cafd4f 100644 --- a/internal/commands/chat_test.go +++ b/internal/commands/chat_test.go @@ -24,6 +24,7 @@ import ( "github.com/basecamp/basecamp-cli/internal/config" "github.com/basecamp/basecamp-cli/internal/names" "github.com/basecamp/basecamp-cli/internal/output" + "github.com/basecamp/basecamp-cli/internal/richtext" ) // chatTestTokenProvider is a mock token provider for tests. @@ -1093,6 +1094,401 @@ func TestChatPostAgentModeWarningOnStderr(t *testing.T) { } // TestChatDeleteReturnsDeletedPayload verifies that delete returns {"deleted": true, "id": "..."}. +// mockChatUpdateTransport handles resolver GETs, the PUT update, and the +// follow-up GET that UpdateLine performs to re-fetch the line. It also serves +// pingable people for mention-resolution tests. +type mockChatUpdateTransport struct { + capturedMethod string + capturedPath string + capturedBody []byte +} + +func (t *mockChatUpdateTransport) RoundTrip(req *http.Request) (*http.Response, error) { + header := make(http.Header) + header.Set("Content-Type", "application/json") + + if req.Method == "GET" { + var body string + switch { + case strings.Contains(req.URL.Path, "/projects.json"): + body = `[{"id": 123, "name": "Test Project"}]` + case strings.Contains(req.URL.Path, "/projects/"): + body = `{"id": 123, "dock": [{"name": "chat", "id": 789, "enabled": true}]}` + case strings.Contains(req.URL.Path, "/circles/people.json") || strings.Contains(req.URL.Path, "/people/pingable.json"): + body = `[{"id": 42000, "name": "Jane Smith", "email_address": "jane@example.com", "attachable_sgid": "sgid-jane"}]` + case strings.Contains(req.URL.Path, "/lines/"): + body = `{"id": 111, "content": "Edited!", "type": "Chat::Lines::Text", "creator": {"id": 1, "name": "Tester"}}` + default: + body = `{}` + } + return &http.Response{StatusCode: 200, Body: io.NopCloser(strings.NewReader(body)), Header: header}, nil + } + + if req.Method == "PUT" { + t.capturedMethod = req.Method + t.capturedPath = req.URL.Path + if req.Body != nil { + t.capturedBody, _ = io.ReadAll(req.Body) + req.Body.Close() + } + return &http.Response{StatusCode: 204, Body: io.NopCloser(strings.NewReader("")), Header: header}, nil + } + + return nil, errors.New("unexpected request") +} + +func TestChatUpdateSendsPutAndReturnsLine(t *testing.T) { + t.Setenv("BASECAMP_NO_KEYRING", "1") + + transport := &mockChatUpdateTransport{} + app, buf := newChatDeleteTestApp(transport) + + cmd := NewChatCmd() + err := executeChatCommand(cmd, app, "update", "111", "Edited!") + require.NoError(t, err) + + assert.Equal(t, "PUT", transport.capturedMethod) + assert.Contains(t, transport.capturedPath, "/lines/111") + + var requestBody map[string]any + require.NoError(t, json.Unmarshal(transport.capturedBody, &requestBody)) + // Default mode converts Markdown to HTML unconditionally (edits always render + // as rich text), so the wire content is the converted HTML, not the raw input. + assert.Equal(t, richtext.MarkdownToHTML("Edited!"), requestBody["content"]) + _, hasContentType := requestBody["content_type"] + assert.False(t, hasContentType, "content_type is never sent on the wire — the SDK PUT carries only content") + + var envelope map[string]any + require.NoError(t, json.Unmarshal(buf.Bytes(), &envelope)) + data, ok := envelope["data"].(map[string]any) + require.True(t, ok) + assert.Equal(t, float64(111), data["id"]) +} + +// TestChatUpdateConvertsMarkdownWithoutMention is the F1 regression: plain +// Markdown with no @mention must still be converted to HTML. The earlier +// implementation only assigned the converted HTML when mention resolution +// changed the string, so "**bold**" with no mention was sent raw to an +// endpoint that always renders rich text. +func TestChatUpdateConvertsMarkdownWithoutMention(t *testing.T) { + t.Setenv("BASECAMP_NO_KEYRING", "1") + + transport := &mockChatUpdateTransport{} + app, _ := newChatDeleteTestApp(transport) + + cmd := NewChatCmd() + err := executeChatCommand(cmd, app, "update", "111", "**bold**") + require.NoError(t, err) + require.NotEmpty(t, transport.capturedBody) + + var requestBody map[string]any + require.NoError(t, json.Unmarshal(transport.capturedBody, &requestBody)) + content, ok := requestBody["content"].(string) + require.True(t, ok) + assert.Contains(t, content, "bold", + "markdown must be converted to HTML even without a mention") +} + +// TestChatUpdateMentionPromotesToHTML verifies that an @mention in content +// auto-promotes to text/html and resolves to a bc-attachment tag, mirroring +// chat post behavior. +func TestChatUpdateMentionPromotesToHTML(t *testing.T) { + t.Setenv("BASECAMP_NO_KEYRING", "1") + + transport := &mockChatUpdateTransport{} + app, _ := newChatDeleteTestApp(transport) + + cmd := NewChatCmd() + err := executeChatCommand(cmd, app, "update", "111", "Hey @Jane.Smith, see this") + require.NoError(t, err) + require.NotEmpty(t, transport.capturedBody) + + var requestBody map[string]any + require.NoError(t, json.Unmarshal(transport.capturedBody, &requestBody)) + + content, ok := requestBody["content"].(string) + require.True(t, ok) + assert.Contains(t, content, "bc-attachment", + "content should contain bc-attachment mention tag") +} + +// TestChatUpdatePlainTextOptOut verifies that --content-type text/plain +// bypasses mention resolution and sends content as-is. +func TestChatUpdatePlainTextOptOut(t *testing.T) { + t.Setenv("BASECAMP_NO_KEYRING", "1") + + transport := &mockChatUpdateTransport{} + app, _ := newChatDeleteTestApp(transport) + + cmd := NewChatCmd() + err := executeChatCommand(cmd, app, "update", "111", "Hey @Jane.Smith", "--content-type", "text/plain") + require.NoError(t, err) + require.NotEmpty(t, transport.capturedBody) + + var requestBody map[string]any + require.NoError(t, json.Unmarshal(transport.capturedBody, &requestBody)) + + content, ok := requestBody["content"].(string) + require.True(t, ok) + assert.NotContains(t, content, "bc-attachment", + "content should not contain bc-attachment when content-type is text/plain") + assert.Contains(t, content, "@Jane.Smith", + "@mention should be left as literal text") +} + +// TestChatUpdatePlainTextSerializesLiterally is the F2 literal case: text/plain +// serializes via richtext.PlainToHTML, so HTML-special characters are escaped +// (rendered as typed, not interpreted) and line breaks are preserved as
. +func TestChatUpdatePlainTextSerializesLiterally(t *testing.T) { + t.Setenv("BASECAMP_NO_KEYRING", "1") + + transport := &mockChatUpdateTransport{} + app, _ := newChatDeleteTestApp(transport) + + cmd := NewChatCmd() + err := executeChatCommand(cmd, app, "update", "111", "x\nline2", "--content-type", "text/plain") + require.NoError(t, err) + require.NotEmpty(t, transport.capturedBody) + + var requestBody map[string]any + require.NoError(t, json.Unmarshal(transport.capturedBody, &requestBody)) + + content, ok := requestBody["content"].(string) + require.True(t, ok) + assert.Contains(t, content, "<strong>x</strong>", + "HTML-special characters should be escaped, not interpreted as markup") + assert.Contains(t, content, "
", + "line breaks should be preserved as
") + assert.NotContains(t, content, "bc-attachment", + "text/plain skips mention resolution") +} + +// TestChatUpdateRejectsUnknownContentType is the F2 validation case: an +// unsupported --content-type fails fast with a usage error and issues no +// request (replacing the content-type validation lost when UpdateLineOptions +// was dropped). +func TestChatUpdateRejectsUnknownContentType(t *testing.T) { + t.Setenv("BASECAMP_NO_KEYRING", "1") + + transport := &mockChatUpdateTransport{} + app, _ := newChatDeleteTestApp(transport) + + cmd := NewChatCmd() + err := executeChatCommand(cmd, app, "update", "111", "x", "--content-type", "application/json") + require.Error(t, err) + + var e *output.Error + require.ErrorAs(t, err, &e) + assert.Equal(t, output.CodeUsage, e.Code) + assert.Empty(t, transport.capturedMethod, "no request should be issued for an invalid content-type") +} + +// TestChatUpdateExtractsChatIDFromURL verifies that pasting a chat-line URL +// targets the chat referenced by the URL rather than falling back to --room or +// the project's default chat — important for projects with multiple campfires. +func TestChatUpdateExtractsChatIDFromURL(t *testing.T) { + t.Setenv("BASECAMP_NO_KEYRING", "1") + + transport := &mockChatUpdateTransport{} + app, _ := newChatDeleteTestApp(transport) + + cmd := NewChatCmd() + err := executeChatCommand(cmd, app, + "update", + "https://3.basecamp.com/99999/buckets/123/chats/456@111", + "Edited via URL") + require.NoError(t, err) + + // PUT should target /chats/456/lines/111 — the chat ID came from the URL, + // not from --room or the dock default (789). + assert.Equal(t, "PUT", transport.capturedMethod) + assert.Contains(t, transport.capturedPath, "/chats/456/lines/111") +} + +// TestChatUpdateRejectsAccountMismatch verifies a chat-line URL whose account +// differs from the configured account is refused rather than silently edited in +// the configured account. +func TestChatUpdateRejectsAccountMismatch(t *testing.T) { + t.Setenv("BASECAMP_NO_KEYRING", "1") + + transport := &mockChatUpdateTransport{} + app, _ := newChatDeleteTestApp(transport) // configured account is 99999 + + cmd := NewChatCmd() + err := executeChatCommand(cmd, app, + "update", + "https://3.basecamp.com/88888/buckets/123/chats/456@111", + "Edited via URL") + require.Error(t, err) + + var e *output.Error + require.ErrorAs(t, err, &e) + assert.Equal(t, output.CodeUsage, e.Code) + assert.Empty(t, transport.capturedMethod, "no request should be issued on account mismatch") +} + +// TestChatUpdateRejectsCollectionURL verifies a room's line-collection URL +// (/chats/{c}/lines, no line ID) is refused rather than misread as line {c} — +// it parses as Type "lines" with IsCollection set and the campfire ID in +// RecordingID. +func TestChatUpdateRejectsCollectionURL(t *testing.T) { + t.Setenv("BASECAMP_NO_KEYRING", "1") + + transport := &mockChatUpdateTransport{} + app, _ := newChatDeleteTestApp(transport) // configured account is 99999 + + cmd := NewChatCmd() + err := executeChatCommand(cmd, app, + "update", + "https://3.basecamp.com/99999/buckets/123/chats/789/lines", + "Edited via URL") + require.Error(t, err) + + var e *output.Error + require.ErrorAs(t, err, &e) + assert.Equal(t, output.CodeUsage, e.Code) + assert.Empty(t, transport.capturedMethod, "no request should be issued for a collection URL") +} + +// TestChatUpdateRejectsUntrustedHost verifies a chat-line URL on an +// attacker-controlled host is refused even though the host-agnostic router +// would otherwise parse it. +func TestChatUpdateRejectsUntrustedHost(t *testing.T) { + t.Setenv("BASECAMP_NO_KEYRING", "1") + + transport := &mockChatUpdateTransport{} + app, _ := newChatDeleteTestApp(transport) + + cmd := NewChatCmd() + err := executeChatCommand(cmd, app, + "update", + "https://evil.example/99999/buckets/123/chats/456/lines/111", + "Edited via URL") + require.Error(t, err) + + var e *output.Error + require.ErrorAs(t, err, &e) + assert.Equal(t, output.CodeUsage, e.Code) + assert.Empty(t, transport.capturedMethod, "no request should be issued for an untrusted host") +} + +// TestChatUpdateWithRoomNoProject verifies that supplying --room with no +// default/flag project edits directly — project resolution (which would fail +// non-interactively or prompt uselessly) is skipped because the PUT only needs +// the chat and line IDs. +func TestChatUpdateWithRoomNoProject(t *testing.T) { + t.Setenv("BASECAMP_NO_KEYRING", "1") + + transport := &mockChatUpdateTransport{} + buf := &bytes.Buffer{} + cfg := &config.Config{AccountID: "99999"} // deliberately no ProjectID + + sdkClient := basecamp.NewClient(&basecamp.Config{BaseURL: "https://3.basecampapi.com"}, &chatTestTokenProvider{}, + basecamp.WithTransport(transport), + basecamp.WithMaxRetries(1), + ) + authMgr := auth.NewManager(cfg, nil) + nameResolver := names.NewResolver(sdkClient, authMgr, cfg.AccountID) + app := &appctx.App{ + Config: cfg, + Auth: authMgr, + SDK: sdkClient, + Names: nameResolver, + Output: output.New(output.Options{Format: output.FormatJSON, Writer: buf}), + } + + cmd := NewChatCmd() + err := executeChatCommand(cmd, app, "update", "111", "fix", "--room", "789") + require.NoError(t, err) + + assert.Equal(t, "PUT", transport.capturedMethod) + assert.Contains(t, transport.capturedPath, "/chats/789/lines/111") +} + +// mockChatUpdateProjectFailTransport serves the PUT and the line re-fetch but +// fails project resolution (GET /projects...), simulating a stale/inaccessible +// saved default project. +type mockChatUpdateProjectFailTransport struct { + capturedMethod string + capturedPath string +} + +func (t *mockChatUpdateProjectFailTransport) RoundTrip(req *http.Request) (*http.Response, error) { + header := make(http.Header) + header.Set("Content-Type", "application/json") + + if req.Method == "GET" { + if strings.Contains(req.URL.Path, "/projects") { + return &http.Response{StatusCode: 500, Body: io.NopCloser(strings.NewReader(`{"error":"boom"}`)), Header: header}, nil + } + if strings.Contains(req.URL.Path, "/lines/") { + return &http.Response{StatusCode: 200, Body: io.NopCloser(strings.NewReader(`{"id": 111, "content": "Edited!", "type": "Chat::Lines::Text", "creator": {"id": 1, "name": "Tester"}}`)), Header: header}, nil + } + return &http.Response{StatusCode: 200, Body: io.NopCloser(strings.NewReader(`{}`)), Header: header}, nil + } + if req.Method == "PUT" { + t.capturedMethod = req.Method + t.capturedPath = req.URL.Path + if req.Body != nil { + io.ReadAll(req.Body) + req.Body.Close() + } + return &http.Response{StatusCode: 204, Body: io.NopCloser(strings.NewReader("")), Header: header}, nil + } + return nil, errors.New("unexpected request") +} + +// TestChatUpdateRoomStaleDefaultProjectStillEdits verifies that when --room is +// supplied, a stale/unreachable saved default project does not fail the edit: +// project resolution (breadcrumb-only) happens best-effort after the PUT. +func TestChatUpdateRoomStaleDefaultProjectStillEdits(t *testing.T) { + t.Setenv("BASECAMP_NO_KEYRING", "1") + + transport := &mockChatUpdateProjectFailTransport{} + app, _ := newChatDeleteTestApp(transport) // config has ProjectID "123" + + cmd := NewChatCmd() + err := executeChatCommand(cmd, app, "update", "111", "fix", "--room", "789") + require.NoError(t, err, "a stale/unreachable default project must not fail the edit path") + + assert.Equal(t, "PUT", transport.capturedMethod) + assert.Contains(t, transport.capturedPath, "/chats/789/lines/111") +} + +// TestChatUpdateRejectsMissingIDArg verifies that invoking update with no args +// returns a purpose-specific usage error (not Cobra's generic arg error, which +// root.go would rewrite into a misleading "Todo ID(s) required"). +func TestChatUpdateRejectsMissingIDArg(t *testing.T) { + t.Setenv("BASECAMP_NO_KEYRING", "1") + + transport := &mockChatUpdateTransport{} + app, _ := newChatDeleteTestApp(transport) + app.Flags.Agent = true // structured usage error instead of help text + + cmd := NewChatCmd() + err := executeChatCommand(cmd, app, "update") + require.Error(t, err) + + var e *output.Error + require.ErrorAs(t, err, &e) + assert.Equal(t, output.CodeUsage, e.Code) + assert.Contains(t, e.Message, "") + assert.Empty(t, transport.capturedMethod, "no request should be issued when the id/url arg is missing") +} + +func TestChatUpdateRejectsEmptyContent(t *testing.T) { + t.Setenv("BASECAMP_NO_KEYRING", "1") + + transport := &mockChatUpdateTransport{} + app, _ := newChatDeleteTestApp(transport) + app.Flags.Agent = true // forces structured ErrUsageHint instead of help text + + cmd := NewChatCmd() + err := executeChatCommand(cmd, app, "update", "111", "") + require.Error(t, err) + assert.Empty(t, transport.capturedMethod, "no PUT should be issued when content is empty") +} + func TestChatDeleteReturnsDeletedPayload(t *testing.T) { t.Setenv("BASECAMP_NO_KEYRING", "1") diff --git a/internal/commands/commands.go b/internal/commands/commands.go index 492da6800..e8c660dce 100644 --- a/internal/commands/commands.go +++ b/internal/commands/commands.go @@ -42,7 +42,7 @@ func CommandCategories() []CommandCategory { {Name: "gauges", Category: "core", Description: "Manage gauges", Actions: []string{"list", "needles", "needle", "create", "update", "delete", "enable", "disable"}}, {Name: "todolistgroups", Category: "core", Description: "Manage to-do list groups", Actions: []string{"list", "show", "create", "update", "position"}}, {Name: "messages", Category: "core", Description: "Manage messages", Actions: []string{"list", "show", "create", "update", "publish", "pin", "unpin", "trash", "archive", "restore"}}, - {Name: "chat", Category: "core", Description: "Chat in real-time", Actions: []string{"list", "messages", "post", "upload", "line", "delete"}}, + {Name: "chat", Category: "core", Description: "Chat in real-time", Actions: []string{"list", "messages", "post", "upload", "line", "update", "delete"}}, {Name: "cards", Category: "core", Description: "Manage Kanban cards", Actions: []string{"list", "show", "create", "update", "move", "done", "columns", "steps", "trash", "archive", "restore"}}, {Name: "files", Category: "core", Description: "Manage files, documents, and folders", Actions: []string{"list", "show", "download", "update", "trash", "archive", "restore"}}, {Name: "checkins", Category: "core", Description: "View automatic check-ins", Actions: []string{"questions", "question", "answers", "answer"}}, diff --git a/internal/hostutil/hostutil.go b/internal/hostutil/hostutil.go index 77de75f50..d93b85c1a 100644 --- a/internal/hostutil/hostutil.go +++ b/internal/hostutil/hostutil.go @@ -55,6 +55,52 @@ func IsLocalhost(host string) bool { return false } +// trustedBasecampHosts are the production Basecamp 3 hosts the CLI trusts when +// resolving a pasted resource URL: the web host and the API host returned in +// API payloads. +var trustedBasecampHosts = map[string]bool{ + "3.basecamp.com": true, + "3.basecampapi.com": true, +} + +// IsTrustedBasecampHost reports whether rawURL points at a host the CLI trusts +// for resolving Basecamp resource URLs. Trusted hosts are the production +// Basecamp 3 domains, any localhost host (covers *.localhost dev domains), and +// the host of the configured base URL (covers custom/staging deployments and +// http://3.basecamp.localhost:3001-style local dev). Everything else is +// rejected so a look-alike URL on an attacker-controlled host — which the +// host-agnostic URL router would otherwise parse — cannot be trusted into a +// mutating request. cfgBaseURL may be empty. +func IsTrustedBasecampHost(rawURL, cfgBaseURL string) bool { + u, err := url.Parse(rawURL) + if err != nil || u.Host == "" { + return false + } + // Only pasted web/API URLs are trusted. Requiring an http(s) scheme keeps + // non-web schemes (ftp://…) and protocol-relative references (//host/…) — + // which url.Parse still resolves to a trusted host — from being accepted. + if u.Scheme != "http" && u.Scheme != "https" { + return false + } + // Hostnames are case-insensitive, so normalize before comparing against the + // trusted set and the configured host (whose keys/values are lower-case). + host := strings.ToLower(u.Hostname()) + if IsLocalhost(strings.ToLower(u.Host)) { + return true + } + if trustedBasecampHosts[host] { + return true + } + if cfgBaseURL != "" { + if cu, err := url.Parse(cfgBaseURL); err == nil { + if ch := strings.ToLower(cu.Hostname()); ch != "" && ch == host { + return true + } + } + } + return false +} + // RequireSecureURL returns an error if the URL uses http:// for a non-localhost host. // Localhost (127.0.0.1, ::1, *.localhost) is exempt for local development. func RequireSecureURL(rawURL string) error { diff --git a/internal/hostutil/hostutil_test.go b/internal/hostutil/hostutil_test.go index 85c2d54d9..ab65fcf74 100644 --- a/internal/hostutil/hostutil_test.go +++ b/internal/hostutil/hostutil_test.go @@ -157,3 +157,49 @@ func TestIsLocalhost(t *testing.T) { }) } } + +func TestIsTrustedBasecampHost(t *testing.T) { + const prodBaseURL = "https://3.basecampapi.com" + const localBaseURL = "http://3.basecamp.localhost:3001" + + tests := []struct { + name string + rawURL string + cfgBaseURL string + expected bool + }{ + // Production hosts are always trusted. + {"production web host", "https://3.basecamp.com/99/buckets/1/chats/2/lines/3", prodBaseURL, true}, + {"production api host", "https://3.basecampapi.com/99/chats/2/lines/3", prodBaseURL, true}, + + // Host comparison is case-insensitive (hostnames are). + {"uppercased production host", "https://3.BASECAMP.com/99/buckets/1/chats/2/lines/3", prodBaseURL, true}, + {"mixed-case configured host", "https://3.BasecampApi.com/99/chats/2/lines/3", "https://3.basecampapi.com", true}, + + // Localhost dev hosts are trusted regardless of the configured base URL. + {"basecamp.localhost with port", "http://3.basecamp.localhost:3001/99/buckets/1/chats/2/lines/3", "", true}, + {"plain localhost", "http://localhost:3001/99/buckets/1/chats/2/lines/3", "", true}, + + // The configured base URL host is trusted (covers staging/custom deploys). + {"configured base URL host", "https://3.basecampapi.com/99/chats/2/lines/3", "https://3.basecampapi.com", true}, + {"configured local base URL", "http://3.basecamp.localhost:3001/99/buckets/1/chats/2/lines/3", localBaseURL, true}, + + // Everything else is rejected — the router is host-agnostic, so this gate + // is what stops a look-alike URL on an attacker-controlled host. + {"attacker host", "https://evil.example/99/buckets/1/chats/2/lines/3", prodBaseURL, false}, + {"basecamp substring host", "https://3.basecamp.com.evil.example/99/chats/2/lines/3", prodBaseURL, false}, + {"empty URL", "", prodBaseURL, false}, + {"bare id, not a URL", "789", prodBaseURL, false}, + + // Non-web schemes on an otherwise-trusted host are rejected — only pasted + // http(s) URLs are trusted. + {"ftp scheme on trusted host", "ftp://3.basecamp.com/99/buckets/1/chats/2/lines/3", prodBaseURL, false}, + {"protocol-relative on trusted host", "//3.basecamp.com/99/buckets/1/chats/2/lines/3", prodBaseURL, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, IsTrustedBasecampHost(tt.rawURL, tt.cfgBaseURL)) + }) + } +} diff --git a/internal/richtext/richtext.go b/internal/richtext/richtext.go index 22974a52a..7361534c1 100644 --- a/internal/richtext/richtext.go +++ b/internal/richtext/richtext.go @@ -452,6 +452,22 @@ func isEmptyParagraph(block string) bool { return strings.TrimSpace(inner) == "" } +// PlainToHTML serializes literal plain text as Basecamp rich text: HTML-special +// characters are escaped so they render as typed (not interpreted as markup), +// and line breaks are preserved as
so multi-line input keeps its shape. +// Windows CRLF and bare CR are normalized to LF first so a single
is +// emitted per line break. Use this when the caller wants the text delivered +// verbatim to an endpoint that always stores rich text. +func PlainToHTML(s string) string { + if s == "" { + return "" + } + s = strings.ReplaceAll(s, "\r\n", "\n") + s = strings.ReplaceAll(s, "\r", "\n") + s = escapeHTML(s) + return strings.ReplaceAll(s, "\n", "
") +} + // escapeHTML escapes special HTML characters. func escapeHTML(s string) string { s = strings.ReplaceAll(s, "&", "&") diff --git a/internal/richtext/richtext_test.go b/internal/richtext/richtext_test.go index da87d6699..cd7fb1937 100644 --- a/internal/richtext/richtext_test.go +++ b/internal/richtext/richtext_test.go @@ -2520,3 +2520,29 @@ func TestMentionInHTMLBlock_FullPipeline(t *testing.T) { t.Errorf("chip does not carry the supplied SGID: %q", res.HTML) } } + +func TestPlainToHTML(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + {"empty", "", ""}, + {"plain text", "hello world", "hello world"}, + {"escapes HTML special chars", "x & y", "<strong>x</strong> & y"}, + {"preserves line breaks as
", "line1\nline2", "line1
line2"}, + {"collapses CRLF to a single break", "line1\r\nline2", "line1
line2"}, + {"normalizes bare CR", "line1\rline2", "line1
line2"}, + {"leaves @mentions literal", "ping @Jane.Smith now", "ping @Jane.Smith now"}, + {"escapes then breaks multiline markup", "a\nb", "<b>a</b>
<i>b</i>"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := PlainToHTML(tt.input) + if got != tt.want { + t.Errorf("PlainToHTML(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +} diff --git a/internal/urlarg/urlarg.go b/internal/urlarg/urlarg.go index e034b6f04..df10fd133 100644 --- a/internal/urlarg/urlarg.go +++ b/internal/urlarg/urlarg.go @@ -19,6 +19,7 @@ type Parsed struct { ProjectID string // BucketID in Basecamp terminology Type string // e.g., "todos", "messages", "cards" RecordingID string + CampfireID string // parent campfire ID for chat-line URLs (chats/{campfireId}/lines/{lineId}) CommentID string IsCollection bool // URL points to a list of items, not an individual resource OccurrenceDate string // date from schedule_entries/{id}/occurrences/{date} URLs @@ -86,11 +87,21 @@ func Parse(input string) *Parsed { } } + // Chat-line URLs (.../chats/{campfireId}/lines/{lineId}) carry the parent + // campfire ID as a path param. Surface it so callers can target the specific + // campfire that owns the line rather than a project-default room. The + // "campfireId" param name comes from the SDK's url-routes.json route table + // entry for /{accountId}/chats/{campfireId}/lines/{lineId}. Non-chat-line + // URLs simply lack this key — the map read then yields "" (a nil Params on + // structural matches reads the same way), leaving CampfireID empty. + campfireID := m.Params["campfireId"] + return &Parsed{ AccountID: m.AccountID, ProjectID: m.ProjectID, Type: pathType, RecordingID: resourceID, + CampfireID: campfireID, CommentID: m.CommentID, IsCollection: isCollection, OccurrenceDate: occurrenceDate, diff --git a/internal/urlarg/urlarg_test.go b/internal/urlarg/urlarg_test.go index 43e0f2bae..5b8aa2e0e 100644 --- a/internal/urlarg/urlarg_test.go +++ b/internal/urlarg/urlarg_test.go @@ -93,6 +93,7 @@ func TestParse(t *testing.T) { ProjectID: "456", Type: "lines", RecordingID: "111", + CampfireID: "789", }, }, { @@ -103,6 +104,34 @@ func TestParse(t *testing.T) { ProjectID: "456", Type: "lines", RecordingID: "111", + CampfireID: "789", + }, + }, + { + name: "chat line URL with query string keeps clean line ID", + input: "https://3.basecamp.com/123/buckets/456/chats/789/lines/111?foo=bar", + want: &Parsed{ + AccountID: "123", + ProjectID: "456", + Type: "lines", + RecordingID: "111", + CampfireID: "789", + }, + }, + { + // The router captures the whole final segment rather than truncating + // at the first non-digit, so "111junk" surfaces verbatim instead of + // silently resolving to line 111 — the boundary bug the old chat-line + // regex had. The malformed segment also skews Type away from "lines", + // so the command's Type=="lines" guard rejects it before any request. + name: "chat line URL with trailing junk on line ID is not truncated", + input: "https://3.basecamp.com/123/buckets/456/chats/789/lines/111junk", + want: &Parsed{ + AccountID: "123", + ProjectID: "456", + Type: "111junk", + RecordingID: "111junk", + CampfireID: "789", }, }, { @@ -177,6 +206,7 @@ func TestParse(t *testing.T) { ProjectID: "456", Type: "lines", RecordingID: "789", + CampfireID: "789", IsCollection: true, }, }, @@ -250,6 +280,9 @@ func TestParse(t *testing.T) { if got.RecordingID != tt.want.RecordingID { t.Errorf("RecordingID = %q, want %q", got.RecordingID, tt.want.RecordingID) } + if got.CampfireID != tt.want.CampfireID { + t.Errorf("CampfireID = %q, want %q", got.CampfireID, tt.want.CampfireID) + } if got.CommentID != tt.want.CommentID { t.Errorf("CommentID = %q, want %q", got.CommentID, tt.want.CommentID) } diff --git a/internal/version/sdk-provenance.json b/internal/version/sdk-provenance.json index 1bedab532..42fd0755a 100644 --- a/internal/version/sdk-provenance.json +++ b/internal/version/sdk-provenance.json @@ -1,9 +1,9 @@ { "sdk": { "module": "github.com/basecamp/basecamp-sdk/go", - "version": "v0.7.4-0.20260722082856-f2086f5e57f6", - "revision": "f2086f5e57f6", - "updated_at": "2026-07-22T08:28:56Z" + "version": "v0.8.0", + "revision": "f55a6b51a9be", + "updated_at": "2026-07-22T08:57:07Z" }, "api": { "repo": "basecamp/bc3", diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index d6a4db157..36f3e696d 100644 --- a/skills/basecamp/SKILL.md +++ b/skills/basecamp/SKILL.md @@ -843,6 +843,7 @@ basecamp chat messages --in --json # List messages basecamp chat post "Hello!" --in basecamp chat post "@Jane.Smith, check this" --in # With @mention (auto text/html) basecamp chat line --in # Show line +basecamp chat update "edited content" --in # Edit existing message in place basecamp chat delete --in --force # Delete line (permanent, not trashable) ```