Migrate all commands to typed SDK service methods (#91)

* Add SDK integration layer: client init, error mapping, and test infrastructure

Wire up fizzy-sdk as a dependency with SDK client initialization in root
PersistentPreRunE, typed error conversion (sdk_errors.go), normalizeAny()
for struct→map round-trips, and httptest-backed mock infrastructure that
lets existing MockClient tests exercise real SDK service methods.

* Migrate all commands to typed SDK service methods

Replace raw HTTP calls (getClient().Get/Post/Patch/Delete) with typed
SDK service methods (Cards().Create, Boards().Update, etc.) across all
command files. Legacy getClient() retained only for file upload/download,
multipart PATCH (avatar), and board migration.

* Update AGENTS.md for SDK architecture and remove dead --tag-ids flag

* Address PR review findings: 7 fixes across commands

- step update: bypass omitempty on bool via raw Patch when --not_completed set
- requireAuth: decouple from SDK init so legacy commands aren't blocked
- card create: follow Location header when API returns empty body
- setup: nil-guard accMap["id"] in parseAccounts
- pin_test: use validation error (422) instead of duplicate not-found
- setup_test: use toJSON helper instead of manual json.Marshal
- test_helpers: replace fragile "nauthorized" match with ToLower

* Fix e2e harness isolation and attachment test panics

Use FIZZY_PROFILE env var + clean HOME temp dir instead of --account
flag to avoid profile store resolution. Guard type assertions in
attachment download tests to prevent nil panics.

* Fix auth e2e tests: pass FIZZY_PROFILE and FIZZY_NO_KEYRING to Execute

Auth login/logout tests use harness.Execute() directly (not h.Run()),
so they must explicitly provide the env vars that globalEnv() normally
injects. Without FIZZY_PROFILE the CLI errors with "No profile configured".

* Address PR review feedback

- Fix CodeQL allocation overflow alert in column.go (len+3 flagged as
  potentially large)
- Remove double normalizeAny in step update --not_completed path
- Add configHome temp dir to NewWithConfig for test isolation
- Set Retryable: true on transient sentinel errors (circuit breaker,
  bulkhead, rate limit)

* Address PR review feedback: omitempty bools, network errors, upload SDK

- board update --all_access false: use raw Patch when setting false to
  avoid omitempty dropping the zero value (same pattern as step
  --not_completed)
- comment update: only set Body on request when --body or --body_file
  provided, preventing empty string from clearing comment text
- sdk_errors: catch raw net.Error as CodeNetwork for proper exit codes
- upload: use requireAuth+requireAccount instead of requireAuthAndAccount
  since upload only uses legacy client, not SDK
- AGENTS.md: update stale jsonAny docs to reflect normalizeAny

* Upgrade fizzy-sdk to v0.1.0

No breaking changes for the CLI — just moving from pre-release
commit pin to the tagged release.

* Add printSuccessWithLocation for webhook merge compatibility

The webhook commands (merged to master in #96) call
printSuccessWithLocation which was added to root.go on master.
Our branch has a diverged root.go, so CI's merge produces an
undefined reference. Add the function here so the merge builds.

* Fix board breadcrumb test to use SDK test mode

The breadcrumb test merged from master (#96) used SetTestMode
(legacy client), but board list uses getSDK() on this branch.
Switch to SetTestModeWithSDK to match.

* Fix CodeQL allocation overflow alert in column list

Guard the slice capacity computation to satisfy CodeQL's
size-overflow analysis.

* Address PR review feedback: marshal errors and URL validation

- Handle json.Marshal errors in signup identity data processing
  instead of silently discarding them
- Validate hostname presence for all URL schemes in validateAPIURL,
  not just http://

Author: jeremy

AGENTS.md

@@ -6,16 +6,32 @@
 fizzy-cli/
 ├── cmd/fizzy/           # Main entrypoint
 ├── internal/
-│   ├── client/          # HTTP client wrapper
+│   ├── client/          # Legacy HTTP client (upload, download, multipart, migrate)
 │   ├── commands/        # Command implementations
 │   ├── config/          # Configuration management
 │   ├── errors/          # Error handling and types
-│   └── response/        # Response formatting
+│   └── render/          # Output rendering (styled, markdown, columns)
 ├── e2e/                 # Go integration tests
 ├── skills/              # Agent skills
 └── .claude-plugin/      # Claude Code integration
 ```
 
+## SDK Architecture
+
+Commands use the fizzy-sdk (`github.com/basecamp/fizzy-sdk/go/pkg/fizzy`) for API access:
+
+- **`getSDK()`** returns `*fizzy.AccountClient` — account-scoped SDK client for most commands
+- **`getSDKClient()`** returns `*fizzy.Client` — root client for account-independent operations (e.g. identity)
+- **`getClient()`** (deprecated) returns `client.API` — legacy client, used only for file upload, download, multipart PATCH, and board migration
+
+SDK service methods return typed structs (e.g. `*generated.Board`, `[]generated.Card`). Helper functions convert to the `any` types the output layer expects:
+- **`normalizeAny(data)`** — JSON-round-trips any value (typed structs, `json.RawMessage`, maps) to `map[string]any` / `[]map[string]any`
+- **`jsonAnySlice(pages)`** — converts `[]json.RawMessage` from `GetAll()` pagination to `[]map[string]any`
+- **`convertSDKError(err)`** — converts SDK errors to `*output.Error`
+- **`toSliceAny(v)`** — normalizes `[]map[string]any` or `[]any` to `[]any` for iteration
+
+Account can be a slug or numeric ID.
+
 ## Fizzy API Reference
 
 API documentation: https://github.com/basecamp/fizzy/blob/main/docs/API.md
@@ -39,6 +55,23 @@ make test-run NAME=TestBoardCRUD  # Run a specific test
 
 Requirements: Go 1.26+, API credentials for e2e tests.
 
+### Unit Test Patterns
+
+Tests use `SetTestModeWithSDK(mock)` which creates an httptest server backed by `MockClient`:
+
+```go
+mock := NewMockClient()
+mock.GetResponse = &client.APIResponse{Data: map[string]any{"id": "1"}}
+SetTestModeWithSDK(mock)
+SetTestConfig("token", "account", "https://api.example.com")
+defer resetTest()
+```
+
+- `mock.OnGet(path, resp)` — route-specific GET responses
+- `mock.WithGetData(data)` — set default GET response data
+- `mock.WithListData(data)` — set GetWithPagination response (used as fallback for GET when GetResponse is default empty map)
+- JSON round-trip through httptest converts `int` → `float64` — use `float64(n)` in assertions
+
 E2E environment variables:
 - `FIZZY_TEST_TOKEN` - API token (required)
 - `FIZZY_TEST_ACCOUNT` - Account slug (required)

SURFACE.txt

@@ -450,7 +450,6 @@ FLAG fizzy card create --markdown type=bool
 FLAG fizzy card create --profile type=string
 FLAG fizzy card create --quiet type=bool
 FLAG fizzy card create --styled type=bool
-FLAG fizzy card create --tag-ids type=string
 FLAG fizzy card create --title type=string
 FLAG fizzy card create --token type=string
 FLAG fizzy card create --verbose type=bool

e2e/harness/harness.go

@@ -28,6 +28,9 @@ type Harness struct {
 	// Cleanup tracks created resources for cleanup
 	Cleanup *CleanupTracker
 
+	// configHome is a temporary directory used as HOME to isolate from host config
+	configHome string
+
 	// t is the testing context
 	t *testing.T
 }
@@ -137,12 +140,19 @@ func New(t *testing.T) *Harness {
 		t.Skip("FIZZY_TEST_ACCOUNT not set, skipping integration tests")
 	}
 
+	tmpDir, err := os.MkdirTemp("", "fizzy-e2e-*")
+	if err != nil {
+		t.Fatalf("failed to create temp config dir: %v", err)
+	}
+	t.Cleanup(func() { os.RemoveAll(tmpDir) })
+
 	return &Harness{
 		BinaryPath: cfg.BinaryPath,
 		Token:      cfg.Token,
 		Account:    cfg.Account,
 		APIURL:     cfg.APIURL,
 		Cleanup:    NewCleanupTracker(),
+		configHome: tmpDir,
 		t:          t,
 	}
 }
@@ -151,12 +161,19 @@ func New(t *testing.T) *Harness {
 func NewWithConfig(t *testing.T, cfg *Config) *Harness {
 	t.Helper()
 
+	tmpDir, err := os.MkdirTemp("", "fizzy-e2e-*")
+	if err != nil {
+		t.Fatalf("failed to create temp config dir: %v", err)
+	}
+	t.Cleanup(func() { os.RemoveAll(tmpDir) })
+
 	return &Harness{
 		BinaryPath: cfg.BinaryPath,
 		Token:      cfg.Token,
 		Account:    cfg.Account,
 		APIURL:     cfg.APIURL,
 		Cleanup:    NewCleanupTracker(),
+		configHome: tmpDir,
 		t:          t,
 	}
 }
@@ -174,8 +191,14 @@ func (h *Harness) RunWithEnv(env map[string]string, args ...string) *Result {
 	// Build full argument list with global options
 	fullArgs := h.buildArgs(args...)
 
+	// Merge global env (FIZZY_PROFILE) with caller-provided env
+	mergedEnv := h.globalEnv()
+	for k, v := range env {
+		mergedEnv[k] = v
+	}
+
 	// Execute the command
-	result := Execute(h.BinaryPath, fullArgs, env)
+	result := Execute(h.BinaryPath, fullArgs, mergedEnv)
 
 	// Try to parse JSON response
 	if result.Stdout != "" {
@@ -211,17 +234,25 @@ func (h *Harness) RunWithoutAuth(args ...string) *Result {
 }
 
 // buildArgs builds the full argument list with global options.
-// Thor requires global options to come AFTER the subcommand.
 func (h *Harness) buildArgs(args ...string) []string {
 	globalArgs := []string{
 		"--token", h.Token,
-		"--account", h.Account,
 		"--api-url", h.APIURL,
 	}
 	// Append global args after the command args
 	return append(args, globalArgs...)
 }
 
+// globalEnv returns environment variables for the test harness.
+// Uses a temporary HOME to isolate from host config/keyring.
+func (h *Harness) globalEnv() map[string]string {
+	return map[string]string{
+		"FIZZY_PROFILE":    h.Account,
+		"FIZZY_NO_KEYRING": "1",
+		"HOME":             h.configHome,
+	}
+}
+
 // GetDataString extracts a string value from the response data.
 func (r *Result) GetDataString(key string) string {
 	if r.Response == nil || r.Response.Data == nil {

e2e/tests/attachment_test.go

@@ -346,9 +346,13 @@ func TestCardAttachmentsDownload(t *testing.T) {
 
 		// Check response data
 		data := result.GetDataMap()
-		downloaded := int(data["downloaded"].(float64))
-		if downloaded != 2 {
-			t.Errorf("expected downloaded=2, got %d", downloaded)
+		if data == nil {
+			t.Error("expected data map in response")
+		} else {
+			downloaded := int(data["downloaded"].(float64))
+			if downloaded != 2 {
+				t.Errorf("expected downloaded=2, got %d", downloaded)
+			}
 		}
 	})
 
@@ -388,12 +392,14 @@ func TestCardAttachmentsDownload(t *testing.T) {
 
 		// Check response shows custom saved_to path
 		data := result.GetDataMap()
-		files := data["files"].([]any)
-		if len(files) > 0 {
-			fileInfo := files[0].(map[string]any)
-			savedTo := fileInfo["saved_to"].(string)
-			if savedTo != customFilename {
-				t.Errorf("expected saved_to=%q, got %q", customFilename, savedTo)
+		if data == nil {
+			t.Error("expected data map in response")
+		} else if files, ok := data["files"].([]any); ok && len(files) > 0 {
+			if fileInfo, ok := files[0].(map[string]any); ok {
+				savedTo, _ := fileInfo["saved_to"].(string)
+				if savedTo != customFilename {
+					t.Errorf("expected saved_to=%q, got %q", customFilename, savedTo)
+				}
 			}
 		}
 	})

e2e/tests/auth_test.go

@@ -113,7 +113,9 @@ func TestAuthLogin(t *testing.T) {
 	t.Run("saves token to config file", func(t *testing.T) {
 		// Run login with HOME set to temp directory
 		result := harness.Execute(cfg.BinaryPath, []string{"auth", "login", cfg.Token}, map[string]string{
-			"HOME": tmpDir,
+			"HOME":             tmpDir,
+			"FIZZY_PROFILE":    cfg.Account,
+			"FIZZY_NO_KEYRING": "1",
 		})
 
 		if result.ExitCode != harness.ExitSuccess {
@@ -153,7 +155,9 @@ func TestAuthLogout(t *testing.T) {
 
 	t.Run("removes config file on logout", func(t *testing.T) {
 		result := harness.Execute(cfg.BinaryPath, []string{"auth", "logout"}, map[string]string{
-			"HOME": tmpDir,
+			"HOME":             tmpDir,
+			"FIZZY_PROFILE":    cfg.Account,
+			"FIZZY_NO_KEYRING": "1",
 		})
 
 		if result.ExitCode != harness.ExitSuccess {
@@ -169,7 +173,9 @@ func TestAuthLogout(t *testing.T) {
 	t.Run("succeeds when already logged out", func(t *testing.T) {
 		// Config file already removed, run logout again
 		result := harness.Execute(cfg.BinaryPath, []string{"auth", "logout"}, map[string]string{
-			"HOME": tmpDir,
+			"HOME":             tmpDir,
+			"FIZZY_PROFILE":    cfg.Account,
+			"FIZZY_NO_KEYRING": "1",
 		})
 
 		if result.ExitCode != harness.ExitSuccess {

go.mod

@@ -4,6 +4,7 @@ go 1.26
 
 require (
 	github.com/basecamp/cli v0.1.1
+	github.com/basecamp/fizzy-sdk/go v0.1.0
 	github.com/charmbracelet/huh v1.0.0
 	github.com/charmbracelet/lipgloss v1.1.0
 	github.com/charmbracelet/x/term v0.2.2

go.sum

@@ -10,6 +10,8 @@ github.com/aymanbagabas/go-udiff v0.3.1 h1:LV+qyBQ2pqe0u42ZsUEtPiCaUoqgA9gYRDs3v
 github.com/aymanbagabas/go-udiff v0.3.1/go.mod h1:G0fsKmG+P6ylD0r6N/KgQD/nWzgfnl8ZBcNLgcbrw8E=
 github.com/basecamp/cli v0.1.1 h1:FAF3M09xo1m7gJJXf38glCkT50ZUuvz+31f+c3R3zcc=
 github.com/basecamp/cli v0.1.1/go.mod h1:NTHe+keCTGI2qM5sMXdkUN0QgU3zGbwnBxcmg8vD5QU=
+github.com/basecamp/fizzy-sdk/go v0.1.0 h1:mZaFrOUnbPzaY/5NTWHahjc7p+yVLMvcNFICfa2eARw=
+github.com/basecamp/fizzy-sdk/go v0.1.0/go.mod h1:HpiKRY9LpWobnxISmFM0I6/Tw+Br00mnDm9vcct8IH8=
 github.com/catppuccin/go v0.3.0 h1:d+0/YicIq+hSTo5oPuRi5kOpqkVA5tAsU6dNhvRu+aY=
 github.com/catppuccin/go v0.3.0/go.mod h1:8IHJuMGaUUjQM82qBrGNBv7LFq6JI3NnQCF6MOlZjpc=
 github.com/charmbracelet/bubbles v0.21.1-0.20250623103423-23b8fd6302d7 h1:JFgG/xnwFfbezlUnFMJy0nusZvytYysV4SCS2cYbvws=

internal/commands/attachment.go

@@ -1,6 +1,7 @@
 package commands
 
 import (
+	"encoding/json"
 	"fmt"
 	"path/filepath"
 	"regexp"
@@ -44,30 +45,29 @@ Use --include-comments to also include attachments from comments on the card.`,
 			return err
 		}
 
-		client := getClient()
-		resp, err := client.Get("/cards/" + args[0] + ".json")
+		ac := getSDK()
+		cardData, _, err := ac.Cards().Get(cmd.Context(), args[0])
 		if err != nil {
-			return err
+			return convertSDKError(err)
 		}
 
-		cardData, ok := resp.Data.(map[string]any)
-		if !ok {
+		cardMap := toMap(cardData)
+		if cardMap == nil {
 			return errors.NewError("Invalid card response")
 		}
 
-		descriptionHTML, _ := cardData["description_html"].(string)
+		descriptionHTML, _ := cardMap["description_html"].(string)
 		attachments := parseAttachments(descriptionHTML)
 
 		if attachmentsShowIncludeComments {
-			commentsResp, err := client.GetWithPagination("/cards/"+args[0]+"/comments.json", true)
+			pages, err := ac.GetAll(cmd.Context(), "/cards/"+args[0]+"/comments.json")
 			if err == nil {
-				if comments, ok := commentsResp.Data.([]any); ok {
-					commentAttachments := extractCommentAttachments(comments)
-					// Re-index and append
-					for _, ca := range commentAttachments {
-						ca.Index = len(attachments) + 1
-						attachments = append(attachments, ca.Attachment)
-					}
+				comments := rawPagesToSlice(pages)
+				commentAttachments := extractCommentAttachments(comments)
+				// Re-index and append
+				for _, ca := range commentAttachments {
+					ca.Index = len(attachments) + 1
+					attachments = append(attachments, ca.Attachment)
 				}
 			}
 		}
@@ -100,29 +100,28 @@ Use 'fizzy card attachments show CARD_NUMBER' to see available attachments and t
 
 		cardNumber := args[0]
 
-		client := getClient()
-		resp, err := client.Get("/cards/" + cardNumber + ".json")
+		ac := getSDK()
+		cardData, _, err := ac.Cards().Get(cmd.Context(), cardNumber)
 		if err != nil {
-			return err
+			return convertSDKError(err)
 		}
 
-		cardData, ok := resp.Data.(map[string]any)
-		if !ok {
+		cardMap := toMap(cardData)
+		if cardMap == nil {
 			return errors.NewError("Invalid card response")
 		}
 
-		descriptionHTML, _ := cardData["description_html"].(string)
+		descriptionHTML, _ := cardMap["description_html"].(string)
 		attachments := parseAttachments(descriptionHTML)
 
 		if attachmentsDownloadIncludeComments {
-			commentsResp, err := client.GetWithPagination("/cards/"+cardNumber+"/comments.json", true)
+			pages, err := ac.GetAll(cmd.Context(), "/cards/"+cardNumber+"/comments.json")
 			if err == nil {
-				if comments, ok := commentsResp.Data.([]any); ok {
-					commentAttachments := extractCommentAttachments(comments)
-					for _, ca := range commentAttachments {
-						ca.Index = len(attachments) + 1
-						attachments = append(attachments, ca.Attachment)
-					}
+				comments := rawPagesToSlice(pages)
+				commentAttachments := extractCommentAttachments(comments)
+				for _, ca := range commentAttachments {
+					ca.Index = len(attachments) + 1
+					attachments = append(attachments, ca.Attachment)
 				}
 			}
 		}
@@ -148,7 +147,8 @@ Use 'fizzy card attachments show CARD_NUMBER' to see available attachments and t
 			toDownload = attachments
 		}
 
-		// Download the files
+		// Download the files (uses old client for DownloadFile)
+		client := getClient()
 		results := make([]map[string]any, 0, len(toDownload))
 		for i, attachment := range toDownload {
 			outputPath := buildOutputPath(attachmentDownloadOutput, attachment.Filename, i+1, len(toDownload))
@@ -172,6 +172,18 @@ Use 'fizzy card attachments show CARD_NUMBER' to see available attachments and t
 	},
 }
 
+// rawPagesToSlice converts []json.RawMessage to []any (each element is a map[string]any).
+func rawPagesToSlice(pages []json.RawMessage) []any {
+	result := make([]any, 0, len(pages))
+	for _, raw := range pages {
+		var v any
+		if json.Unmarshal(raw, &v) == nil {
+			result = append(result, v)
+		}
+	}
+	return result
+}
+
 // parseAttachments extracts attachment information from description_html
 func parseAttachments(html string) []Attachment {
 	var attachments []Attachment