fix(output): valid JSON on stdout and numeric toObjectId parsing

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+- `--output json` now writes status/progress messages to stderr instead of stdout, so stdout is valid, parseable JSON (#52)
+- `associations list` no longer fails to parse responses with results: `toObjectId` is now decoded as a number (#51)
+
 ### Changed
 - Improved init/config UX with huh forms, config pre-population, and --force flag on clear (#44)
 - Removed `config set` command - use `init` for configuration changes (#44)

api/associations.go

@@ -5,9 +5,15 @@ import (
 	"fmt"
 )
 
-// Association represents a HubSpot association between objects
+// Association represents a HubSpot association between objects.
+//
+// ToObjectID uses json.Number because the HubSpot CRM v4 associations API
+// returns toObjectId as a JSON number (e.g. 98765), not a string. A plain
+// string field would fail to unmarshal with "cannot unmarshal number into Go
+// struct field". The number is kept as-is and ToObjectID.String() yields it
+// for display without forcing a string-vs-int type choice up front.
 type Association struct {
-	ToObjectID       string            `json:"toObjectId"`
+	ToObjectID       json.Number       `json:"toObjectId"`
 	AssociationTypes []AssociationType `json:"associationTypes"`
 }
 

api/associations_test.go

@@ -0,0 +1,111 @@
+package api
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestClient_ListAssociations(t *testing.T) {
+	t.Run("results with numeric toObjectId parse successfully", func(t *testing.T) {
+		// The HubSpot CRM v4 associations API returns toObjectId as a JSON
+		// number, not a string. This used to fail with:
+		//   json: cannot unmarshal number into Go struct field
+		//   Association.results.toObjectId of type string
+		server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			assert.Equal(t, "/crm/v4/objects/contacts/12345/associations/notes", r.URL.Path)
+			assert.Equal(t, http.MethodGet, r.Method)
+
+			w.WriteHeader(http.StatusOK)
+			_, _ = w.Write([]byte(`{
+				"results": [
+					{
+						"toObjectId": 98765,
+						"associationTypes": [
+							{
+								"category": "HUBSPOT_DEFINED",
+								"typeId": 202,
+								"label": "Contact to Note"
+							}
+						]
+					}
+				]
+			}`))
+		}))
+		defer server.Close()
+
+		client := &Client{
+			BaseURL:     server.URL,
+			AccessToken: "test-token",
+			HTTPClient:  server.Client(),
+		}
+
+		result, err := client.ListAssociations(ObjectTypeContacts, "12345", ObjectTypeNotes, ListOptions{})
+		require.NoError(t, err)
+		require.Len(t, result.Results, 1)
+		assert.Equal(t, "98765", result.Results[0].ToObjectID.String())
+		require.Len(t, result.Results[0].AssociationTypes, 1)
+		assert.Equal(t, "HUBSPOT_DEFINED", result.Results[0].AssociationTypes[0].Category)
+		assert.Equal(t, 202, result.Results[0].AssociationTypes[0].TypeID)
+	})
+
+	t.Run("empty results parse cleanly", func(t *testing.T) {
+		server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusOK)
+			_, _ = w.Write([]byte(`{"results": []}`))
+		}))
+		defer server.Close()
+
+		client := &Client{
+			BaseURL:     server.URL,
+			AccessToken: "test-token",
+			HTTPClient:  server.Client(),
+		}
+
+		result, err := client.ListAssociations(ObjectTypeContacts, "12345", ObjectTypeNotes, ListOptions{})
+		require.NoError(t, err)
+		assert.Empty(t, result.Results)
+	})
+
+	t.Run("result re-serializes to valid JSON with numeric id", func(t *testing.T) {
+		server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusOK)
+			_, _ = w.Write([]byte(`{
+				"results": [
+					{"toObjectId": 98765, "associationTypes": []}
+				]
+			}`))
+		}))
+		defer server.Close()
+
+		client := &Client{
+			BaseURL:     server.URL,
+			AccessToken: "test-token",
+			HTTPClient:  server.Client(),
+		}
+
+		result, err := client.ListAssociations(ObjectTypeContacts, "12345", ObjectTypeNotes, ListOptions{})
+		require.NoError(t, err)
+
+		// Re-marshalling produces valid JSON; json.Number preserves the
+		// number as a number (not a quoted string).
+		out, err := json.Marshal(result)
+		require.NoError(t, err)
+
+		var roundtrip map[string]interface{}
+		require.NoError(t, json.Unmarshal(out, &roundtrip), "marshalled output must be valid JSON")
+		assert.Contains(t, string(out), `"toObjectId":98765`)
+	})
+
+	t.Run("empty from ID returns error", func(t *testing.T) {
+		client := &Client{BaseURL: "https://api.hubapi.com"}
+		result, err := client.ListAssociations(ObjectTypeContacts, "", ObjectTypeNotes, ListOptions{})
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "from object ID is required")
+		assert.Nil(t, result)
+	})
+}

internal/cmd/associations/associations.go

@@ -78,7 +78,7 @@ func newListCmd(opts *root.Options) *cobra.Command {
 						label = fmt.Sprintf("Type %d", at.TypeID)
 					}
 					rows = append(rows, []string{
-						assoc.ToObjectID,
+						assoc.ToObjectID.String(),
 						label,
 						at.Category,
 					})

internal/cmd/configcmd/configcmd.go

@@ -140,14 +140,14 @@ pass/fail status and troubleshooting suggestions on failure.`,
 			token := config.GetAccessToken()
 			if token == "" {
 				v.Error("No HubSpot access token configured")
-				v.Println("")
+				v.PrintlnStatus("")
 				v.Info("Configure with: hspt init")
 				v.Info("Or set environment variable: HUBSPOT_ACCESS_TOKEN")
 				return nil
 			}
 
-			v.Println("Testing connection to HubSpot...")
-			v.Println("")
+			v.PrintlnStatus("Testing connection to HubSpot...")
+			v.PrintlnStatus("")
 
 			client, err := opts.APIClient()
 			if err != nil {
@@ -159,13 +159,13 @@ pass/fail status and troubleshooting suggestions on failure.`,
 			if err != nil {
 				if api.IsUnauthorized(err) {
 					v.Error("Authentication failed: invalid access token")
-					v.Println("")
+					v.PrintlnStatus("")
 					v.Info("Check your token at: HubSpot Settings > Integrations > Private Apps")
 					return nil
 				}
 				if api.IsForbidden(err) {
 					v.Error("Authentication failed: missing required scopes")
-					v.Println("")
+					v.PrintlnStatus("")
 					v.Info("Ensure your private app has the required scopes enabled")
 					return nil
 				}
@@ -174,7 +174,7 @@ pass/fail status and troubleshooting suggestions on failure.`,
 			}
 
 			v.Success("Connection successful!")
-			v.Println("")
+			v.PrintlnStatus("")
 			v.Info("HubSpot account has %d owners", len(owners))
 			if len(owners) > 0 {
 				v.Info("First owner: %s (%s)", owners[0].FullName(), owners[0].Email)

internal/cmd/forms/forms.go

@@ -141,7 +141,7 @@ func newGetCmd(opts *root.Options) *cobra.Command {
 
 			// Show fields in verbose mode
 			if opts.Verbose && len(form.FieldGroups) > 0 {
-				v.Println("")
+				v.PrintlnStatus("")
 				v.Info("Form Fields:")
 				fieldHeaders := []string{"NAME", "LABEL", "TYPE", "REQUIRED"}
 				fieldRows := make([][]string, 0)

internal/view/view.go

@@ -20,7 +20,13 @@ const (
 	FormatPlain Format = "plain"
 )
 
-// View handles output formatting
+// View handles output formatting.
+//
+// Status, progress, and banner text are written to Err (stderr) so that Out
+// (stdout) carries only the primary rendered result. This keeps `--output json`
+// output valid and parseable by tools like jq: in JSON mode stdout
+// is only the JSON payload, and in human/plain mode only the rendered result
+// lands on stdout while status chatter goes to stderr.
 type View struct {
 	Format  Format
 	NoColor bool
@@ -95,36 +101,36 @@ func (v *View) Render(headers []string, rows [][]string, jsonData interface{}) e
 	}
 }
 
-// Success prints a success message
+// Success prints a success status message to stderr.
 func (v *View) Success(format string, args ...interface{}) {
 	msg := fmt.Sprintf(format, args...)
-	fmt.Fprintln(v.Out, color.GreenString("✓ %s", msg))
+	fmt.Fprintln(v.Err, color.GreenString("✓ %s", msg))
 }
 
-// Error prints an error message
+// Error prints an error message to stderr.
 func (v *View) Error(format string, args ...interface{}) {
 	msg := fmt.Sprintf(format, args...)
 	fmt.Fprintln(v.Err, color.RedString("✗ %s", msg))
 }
 
-// Warning prints a warning message
+// Warning prints a warning message to stderr.
 func (v *View) Warning(format string, args ...interface{}) {
 	msg := fmt.Sprintf(format, args...)
 	fmt.Fprintln(v.Err, color.YellowString("⚠ %s", msg))
 }
 
-// Info prints an info message
+// Info prints an informational status message to stderr.
 func (v *View) Info(format string, args ...interface{}) {
 	msg := fmt.Sprintf(format, args...)
-	fmt.Fprintln(v.Out, msg)
+	fmt.Fprintln(v.Err, msg)
 }
 
-// Print prints a message without formatting
-func (v *View) Print(format string, args ...interface{}) {
-	fmt.Fprintf(v.Out, format, args...)
+// PrintStatus prints an unformatted status message to stderr (no trailing newline).
+func (v *View) PrintStatus(format string, args ...interface{}) {
+	fmt.Fprintf(v.Err, format, args...)
 }
 
-// Println prints a message with newline
-func (v *View) Println(format string, args ...interface{}) {
-	fmt.Fprintln(v.Out, fmt.Sprintf(format, args...))
+// PrintlnStatus prints a status message to stderr with a trailing newline.
+func (v *View) PrintlnStatus(format string, args ...interface{}) {
+	fmt.Fprintln(v.Err, fmt.Sprintf(format, args...))
 }

internal/view/view_test.go

@@ -0,0 +1,126 @@
+package view
+
+import (
+	"bytes"
+	"encoding/json"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// newTestView returns a View that writes to the provided buffers, with color
+// disabled so assertions can match raw text without ANSI escape codes.
+func newTestView(format string) (*View, *bytes.Buffer, *bytes.Buffer) {
+	var out, errBuf bytes.Buffer
+	v := New(format, true) // noColor=true
+	v.Out = &out
+	v.Err = &errBuf
+	return v, &out, &errBuf
+}
+
+// TestStatusMessagesGoToStderr ensures human-facing status/progress messages
+// are written to stderr, not stdout. This is what keeps `--output json`
+// output valid (issue #52): only the JSON payload may land on stdout.
+func TestStatusMessagesGoToStderr(t *testing.T) {
+	tests := []struct {
+		name string
+		call func(v *View)
+		want string
+	}{
+		{"Success", func(v *View) { v.Success("created with ID %s", "98765") }, "created with ID 98765"},
+		{"Info", func(v *View) { v.Info("Found %d contact(s)", 3) }, "Found 3 contact(s)"},
+		{"PrintStatus", func(v *View) { v.PrintStatus("progress %d%%", 50) }, "progress 50%"},
+		{"PrintlnStatus", func(v *View) { v.PrintlnStatus("More results available") }, "More results available"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			v, out, errBuf := newTestView("json")
+			tt.call(v)
+
+			assert.Empty(t, out.String(), "status message must NOT be written to stdout")
+			assert.Contains(t, errBuf.String(), tt.want, "status message must be written to stderr")
+		})
+	}
+}
+
+// TestErrorAndWarningGoToStderr documents that Error and Warning remain on
+// stderr (they always have); this is now consistent with the status methods
+// Success/Info/PrintStatus/PrintlnStatus.
+func TestErrorAndWarningGoToStderr(t *testing.T) {
+	v, out, errBuf := newTestView("json")
+
+	v.Error("boom %s", "x")
+	v.Warning("careful %s", "y")
+
+	assert.Empty(t, out.String())
+	assert.Contains(t, errBuf.String(), "boom x")
+	assert.Contains(t, errBuf.String(), "careful y")
+}
+
+// TestStdoutIsValidJSON simulates a command that emits a status message
+// followed by a JSON payload, and verifies stdout alone is parseable JSON.
+func TestStdoutIsValidJSON(t *testing.T) {
+	v, out, _ := newTestView("json")
+
+	// Order mirrors real commands: status first, then structured payload.
+	v.Success("Note created with ID: %s", "98765")
+	v.Info("Found 1 result")
+	require.NoError(t, v.JSON(map[string]interface{}{
+		"id":   "98765",
+		"name": "Demo",
+	}))
+	v.Info("More results available. Use --after abc123 to get the next page.")
+
+	stdout := out.String()
+	assert.NotContains(t, stdout, "Note created", "no status banner may leak to stdout")
+	assert.NotContains(t, stdout, "More results", "no pagination text may leak to stdout")
+
+	var parsed map[string]interface{}
+	require.NoError(t, json.Unmarshal([]byte(stdout), &parsed),
+		"stdout must be valid JSON, got: %q", stdout)
+	assert.Equal(t, "98765", parsed["id"])
+}
+
+// TestPrimaryOutputGoesToStdout guarantees the other half of the invariant:
+// the primary rendered result (Table/Plain/Render/JSON) always lands on stdout,
+// never stderr, even while status chatter is routed to stderr. If a "status"
+// method ever leaked primary output to stderr, this would catch it.
+func TestPrimaryOutputGoesToStdout(t *testing.T) {
+	t.Run("table renders to stdout in human mode", func(t *testing.T) {
+		v, out, errBuf := newTestView("table")
+		// Interleave status messages with the primary rendered result.
+		v.Info("Found 1 result")
+		require.NoError(t, v.Render([]string{"ID", "NAME"}, [][]string{{"98765", "Demo"}}, nil))
+		v.PrintlnStatus("More results available")
+
+		stdout := out.String()
+		assert.Contains(t, stdout, "98765", "primary rendered data must be on stdout")
+		assert.Contains(t, stdout, "Demo", "primary rendered data must be on stdout")
+		// Status chatter must not leak into the primary stdout stream.
+		assert.NotContains(t, stdout, "Found 1 result")
+		assert.NotContains(t, stdout, "More results available")
+		assert.Contains(t, errBuf.String(), "Found 1 result")
+		assert.Contains(t, errBuf.String(), "More results available")
+	})
+
+	t.Run("plain renders to stdout in plain mode", func(t *testing.T) {
+		v, out, errBuf := newTestView("plain")
+		v.Info("Found 1 result")
+		require.NoError(t, v.Render(nil, [][]string{{"98765", "Demo"}}, nil))
+
+		assert.Contains(t, out.String(), "98765", "primary rendered data must be on stdout")
+		assert.NotContains(t, out.String(), "Found 1 result")
+		assert.Contains(t, errBuf.String(), "Found 1 result")
+	})
+}
+
+// TestJSONOutputUnaffectedByColorFlag is a small sanity check that JSON output
+// is plain (no ANSI codes regardless of color setting).
+func TestJSONOutputUnaffectedByColorFlag(t *testing.T) {
+	v, out, _ := newTestView("json")
+	require.NoError(t, v.JSON([]string{"a", "b"}))
+	assert.False(t, strings.Contains(out.String(), "\x1b["), "JSON output must not contain ANSI escapes")
+}