Fix campfire list on multi-campfire projects

internal/commands/campfire.go

@@ -1,6 +1,7 @@
 package commands
 
 import (
+	"encoding/json"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -30,14 +31,14 @@ func NewCampfireCmd() *cobra.Command {
 Use 'basecamp campfire list' to see campfires in a project.
 Use 'basecamp campfire messages' to view recent messages.
 Use 'basecamp campfire post "message"' to post a message.`,
-		Annotations: map[string]string{"agent_notes": "Each project has one campfire (the chat room)\nContent supports Markdown — converted to HTML automatically\nCampfire is project-scoped, no cross-project campfire queries"},
+		Annotations: map[string]string{"agent_notes": "Projects may have multiple campfires; use `campfire list` to see them\nContent supports Markdown — converted to HTML automatically\nCampfire is project-scoped, no cross-project campfire queries"},
 	}
 
 	cmd.PersistentFlags().StringVarP(&project, "project", "p", "", "Project ID or name")
 	cmd.PersistentFlags().StringVar(&project, "in", "", "Project ID (alias for --project)")
 	cmd.PersistentFlags().StringVarP(&campfireID, "campfire", "c", "", "Campfire ID")
 	cmd.AddCommand(
-		newCampfireListCmd(&project),
+		newCampfireListCmd(&project, &campfireID),
 		newCampfireMessagesCmd(&project, &campfireID),
 		newCampfirePostCmd(&project, &campfireID, &contentType),
 		newCampfireUploadCmd(&project, &campfireID),
@@ -48,7 +49,7 @@ Use 'basecamp campfire post "message"' to post a message.`,
 	return cmd
 }
 
-func newCampfireListCmd(project *string) *cobra.Command {
+func newCampfireListCmd(project, campfireID *string) *cobra.Command {
 	var all bool
 
 	cmd := &cobra.Command{
@@ -60,7 +61,7 @@ func newCampfireListCmd(project *string) *cobra.Command {
 			if err := ensureAccount(cmd, app); err != nil {
 				return err
 			}
-			return runCampfireList(cmd, app, *project, all)
+			return runCampfireList(cmd, app, *project, *campfireID, all)
 		},
 	}
 
@@ -69,7 +70,7 @@ func newCampfireListCmd(project *string) *cobra.Command {
 	return cmd
 }
 
-func runCampfireList(cmd *cobra.Command, app *appctx.App, project string, all bool) error {
+func runCampfireList(cmd *cobra.Command, app *appctx.App, project, campfireID string, all bool) error {
 	// Account-wide campfire listing
 	if all {
 		result, err := app.Account().Campfires().List(cmd.Context(), nil)
@@ -117,49 +118,105 @@ func runCampfireList(cmd *cobra.Command, app *appctx.App, project string, all bo
 		return err
 	}
 
-	// Get campfire from project dock
-	campfireIDStr, err := getCampfireID(cmd, app, resolvedProjectID)
-	if err != nil {
-		return err
+	// If a specific campfire ID was given via -c, fetch just that one
+	if campfireID != "" {
+		campfireIDInt, parseErr := strconv.ParseInt(campfireID, 10, 64)
+		if parseErr != nil {
+			return output.ErrUsage("Invalid campfire ID")
+		}
+
+		campfire, getErr := app.Account().Campfires().Get(cmd.Context(), campfireIDInt)
+		if getErr != nil {
+			return getErr
+		}
+
+		return app.OK([]*basecamp.Campfire{campfire},
+			output.WithSummary(fmt.Sprintf("Campfire: %s", campfireTitle(campfire))),
+			output.WithBreadcrumbs(campfireListBreadcrumbs(campfireID, resolvedProjectID)...),
+		)
 	}
 
-	campfireIDInt, err := strconv.ParseInt(campfireIDStr, 10, 64)
+	// Fetch project dock and find all chat entries (supports multi-campfire projects)
+	path := fmt.Sprintf("/projects/%s.json", resolvedProjectID)
+	resp, err := app.Account().Get(cmd.Context(), path)
 	if err != nil {
-		return output.ErrUsage("Invalid campfire ID")
+		return convertSDKError(err)
 	}
 
-	// Get campfire details using SDK
-	campfire, err := app.Account().Campfires().Get(cmd.Context(), campfireIDInt)
-	if err != nil {
-		return err
+	var projectData struct {
+		Dock []DockTool `json:"dock"`
+	}
+	if err := json.Unmarshal(resp.Data, &projectData); err != nil {
+		return fmt.Errorf("failed to parse project: %w", err)
+	}
+
+	// Collect enabled chat dock entries and fetch full campfire details
+	var campfires []*basecamp.Campfire
+	var hasDisabled bool
+	for _, tool := range projectData.Dock {
+		if tool.Name != "chat" {
+			continue
+		}
+		if !tool.Enabled {
+			hasDisabled = true
+			continue
+		}
+		campfire, getErr := app.Account().Campfires().Get(cmd.Context(), tool.ID)
+		if getErr != nil {
+			return getErr
+		}
+		campfires = append(campfires, campfire)
 	}
 
-	title := "Campfire"
-	if campfire.Title != "" {
-		title = campfire.Title
+	if len(campfires) == 0 {
+		hint := "Project has no campfire"
+		if hasDisabled {
+			hint = "Campfire is disabled for this project"
+		}
+		return output.ErrNotFoundHint("campfire", resolvedProjectID, hint)
 	}
 
-	// Return as array for consistency
-	result := []*basecamp.Campfire{campfire}
-	summary := fmt.Sprintf("Campfire: %s", title)
+	summary := fmt.Sprintf("%d campfire(s)", len(campfires))
 
-	return app.OK(result,
+	return app.OK(campfires,
 		output.WithSummary(summary),
 		output.WithBreadcrumbs(
 			output.Breadcrumb{
 				Action:      "messages",
-				Cmd:         fmt.Sprintf("basecamp campfire %s messages --in %s", campfireIDStr, resolvedProjectID),
+				Cmd:         fmt.Sprintf("basecamp campfire messages -c <id> --in %s", resolvedProjectID),
 				Description: "View messages",
 			},
 			output.Breadcrumb{
 				Action:      "post",
-				Cmd:         fmt.Sprintf("basecamp campfire %s post \"message\" --in %s", campfireIDStr, resolvedProjectID),
+				Cmd:         fmt.Sprintf("basecamp campfire post \"message\" -c <id> --in %s", resolvedProjectID),
 				Description: "Post message",
 			},
 		),
 	)
 }
 
+func campfireTitle(c *basecamp.Campfire) string {
+	if c.Title != "" {
+		return c.Title
+	}
+	return "Campfire"
+}
+
+func campfireListBreadcrumbs(campfireID, projectID string) []output.Breadcrumb {
+	return []output.Breadcrumb{
+		{
+			Action:      "messages",
+			Cmd:         fmt.Sprintf("basecamp campfire messages -c %s --in %s", campfireID, projectID),
+			Description: "View messages",
+		},
+		{
+			Action:      "post",
+			Cmd:         fmt.Sprintf("basecamp campfire post \"message\" -c %s --in %s", campfireID, projectID),
+			Description: "Post message",
+		},
+	}
+}
+
 func newCampfireMessagesCmd(project, campfireID *string) *cobra.Command {
 	var limit int
 

internal/commands/campfire_test.go

@@ -239,6 +239,182 @@ func TestCampfirePostDefaultOmitsContentType(t *testing.T) {
 		"content_type should not be sent when --content-type is not specified")
 }
 
+// mockMultiCampfireTransport returns a project with multiple chat dock entries
+// and serves individual campfire GET requests.
+type mockMultiCampfireTransport struct{}
+
+func (t *mockMultiCampfireTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	header := make(http.Header)
+	header.Set("Content-Type", "application/json")
+
+	if req.Method != "GET" {
+		return &http.Response{
+			StatusCode: 405,
+			Body:       io.NopCloser(strings.NewReader(`{}`)),
+			Header:     header,
+		}, nil
+	}
+
+	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/123"):
+		body = `{"id": 123, "dock": [` +
+			`{"name": "chat", "id": 1001, "title": "General", "enabled": true},` +
+			`{"name": "chat", "id": 1002, "title": "Engineering", "enabled": true}` +
+			`]}`
+	case strings.HasSuffix(req.URL.Path, "/chats/1001"):
+		body = `{"id": 1001, "title": "General", "type": "Chat::Transcript", "status": "active",` +
+			`"visible_to_clients": false, "inherits_status": true,` +
+			`"url": "https://example.com", "app_url": "https://example.com",` +
+			`"created_at": "2024-01-01T00:00:00Z", "updated_at": "2024-01-01T00:00:00Z",` +
+			`"bucket": {"id": 123, "name": "Test"}, "creator": {"id": 1, "name": "Test"}}`
+	case strings.HasSuffix(req.URL.Path, "/chats/1002"):
+		body = `{"id": 1002, "title": "Engineering", "type": "Chat::Transcript", "status": "active",` +
+			`"visible_to_clients": false, "inherits_status": true,` +
+			`"url": "https://example.com", "app_url": "https://example.com",` +
+			`"created_at": "2024-01-01T00:00:00Z", "updated_at": "2024-01-01T00:00:00Z",` +
+			`"bucket": {"id": 123, "name": "Test"}, "creator": {"id": 1, "name": "Test"}}`
+	default:
+		body = `{}`
+	}
+
+	return &http.Response{
+		StatusCode: 200,
+		Body:       io.NopCloser(strings.NewReader(body)),
+		Header:     header,
+	}, nil
+}
+
+func newTestAppWithTransport(t *testing.T, transport http.RoundTripper) (*appctx.App, *bytes.Buffer) {
+	t.Helper()
+	t.Setenv("BASECAMP_NO_KEYRING", "1")
+
+	buf := &bytes.Buffer{}
+	cfg := &config.Config{
+		AccountID: "99999",
+		ProjectID: "123",
+	}
+
+	sdkClient := basecamp.NewClient(&basecamp.Config{}, &campfireTestTokenProvider{},
+		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,
+		}),
+	}
+	return app, buf
+}
+
+// TestCampfireListMultipleCampfires verifies that `campfire list` succeeds on
+// projects with multiple campfires (no ambiguous error).
+func TestCampfireListMultipleCampfires(t *testing.T) {
+	app, buf := newTestAppWithTransport(t, &mockMultiCampfireTransport{})
+
+	cmd := NewCampfireCmd()
+	err := executeCampfireCommand(cmd, app, "list")
+	require.NoError(t, err)
+
+	var envelope struct {
+		Data []map[string]any `json:"data"`
+	}
+	require.NoError(t, json.Unmarshal(buf.Bytes(), &envelope))
+	require.Len(t, envelope.Data, 2)
+
+	titles := []string{envelope.Data[0]["title"].(string), envelope.Data[1]["title"].(string)}
+	assert.Contains(t, titles, "General")
+	assert.Contains(t, titles, "Engineering")
+}
+
+// TestCampfireListWithCampfireFlag verifies that `campfire list -c <id>` returns
+// only the specified campfire.
+func TestCampfireListWithCampfireFlag(t *testing.T) {
+	app, buf := newTestAppWithTransport(t, &mockMultiCampfireTransport{})
+
+	cmd := NewCampfireCmd()
+	err := executeCampfireCommand(cmd, app, "list", "--campfire", "1002")
+	require.NoError(t, err)
+
+	var envelope struct {
+		Data []map[string]any `json:"data"`
+	}
+	require.NoError(t, json.Unmarshal(buf.Bytes(), &envelope))
+	require.Len(t, envelope.Data, 1)
+	assert.Equal(t, "Engineering", envelope.Data[0]["title"])
+}
+
+// mockCampfireDockTransport returns a project whose dock payload is configurable.
+type mockCampfireDockTransport struct {
+	dockJSON string // JSON array for the dock field
+}
+
+func (t *mockCampfireDockTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	header := make(http.Header)
+	header.Set("Content-Type", "application/json")
+
+	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/123"):
+		body = `{"id": 123, "dock": ` + t.dockJSON + `}`
+	default:
+		body = `{}`
+	}
+	return &http.Response{
+		StatusCode: 200,
+		Body:       io.NopCloser(strings.NewReader(body)),
+		Header:     header,
+	}, nil
+}
+
+// TestCampfireListNoCampfires verifies the not-found error when a project has
+// no chat dock entries at all.
+func TestCampfireListNoCampfires(t *testing.T) {
+	transport := &mockCampfireDockTransport{
+		dockJSON: `[{"name": "todoset", "id": 500, "enabled": true}]`,
+	}
+	app, _ := newTestAppWithTransport(t, transport)
+
+	cmd := NewCampfireCmd()
+	err := executeCampfireCommand(cmd, app, "list")
+	require.Error(t, err)
+
+	var e *output.Error
+	require.ErrorAs(t, err, &e)
+	assert.Equal(t, output.CodeNotFound, e.Code)
+	assert.Contains(t, e.Hint, "no campfire")
+}
+
+// TestCampfireListDisabledCampfire verifies the not-found error hints that
+// campfire is disabled when only disabled chat entries exist.
+func TestCampfireListDisabledCampfire(t *testing.T) {
+	transport := &mockCampfireDockTransport{
+		dockJSON: `[{"name": "chat", "id": 900, "title": "Campfire", "enabled": false}]`,
+	}
+	app, _ := newTestAppWithTransport(t, transport)
+
+	cmd := NewCampfireCmd()
+	err := executeCampfireCommand(cmd, app, "list")
+	require.Error(t, err)
+
+	var e *output.Error
+	require.ErrorAs(t, err, &e)
+	assert.Equal(t, output.CodeNotFound, e.Code)
+	assert.Contains(t, e.Hint, "disabled")
+}
+
 // TestCampfirePostViaSubcommandWithCampfireFlag verifies the proper way to post
 // to a specific campfire: `basecamp campfire post <msg> --campfire <id>`.
 func TestCampfirePostViaSubcommandWithCampfireFlag(t *testing.T) {