fix: idempotency key scoping, profile resolution, and unbounded paging

CHANGELOG.md

@@ -9,6 +9,51 @@ Releases before `0.2.0` predate this file. Their notes are on the
 
 ## [Unreleased]
 
+### Fixed
+- `dns import` sent the **same idempotency key on every record**. The key was
+  minted once per invocation, but an invocation can perform many operations —
+  import posts once per record — so a 50-record zone file went out under one
+  key. An API honouring keys as documented ("reusing the same key returns the
+  original result instead of repeating the operation") would create the first
+  record, echo it back for the other 49, and let the CLI report the whole file
+  as imported. Each write now gets its own key. `--idempotency-key` still pins
+  every write in an invocation to one value, which is what makes re-running a
+  failed command collapse onto the original.
+- Credentials that existed were reported as missing. A config file with no
+  top-level `default:` key resolved to no profile at all, so `auth status` said
+  "no credentials configured — run 'namecom auth login'" (which would have
+  overwritten them) while `config list-profiles` printed the profile it was
+  refusing to use. A profile named `default` is now used without the key, as is
+  a lone profile under any name; two or more with no default is an error that
+  names them and suggests `--profile`.
+- Seven list commands could page forever. Only `domain list` bounded its walk
+  with `lastPage`; the rest trusted the server to stop saying "there is more",
+  so one that kept answering `nextPage: 2` made `dns list --all` run
+  indefinitely at the full client rate limit. All paginated walks — including
+  record-ID shell completion and `namecom status` — now stop unless the page
+  number advances and stays within `lastPage`.
+- A 429 carrying a long `Retry-After` was swallowed. The CLI slept on it until
+  the request deadline expired and then reported `context deadline exceeded
+  (Client.Timeout exceeded while awaiting headers)` with exit 1 — a transport
+  error, hiding the rate-limit answer the server had already given. A wait that
+  cannot fit the remaining budget is no longer taken: the 429 is returned as-is,
+  with exit 5 and a hint naming the wait the API asked for. A server-supplied
+  wait is also capped at 30s, matching the cap computed backoff always had.
+- Error bodies that are not the API's JSON envelope are summarized instead of
+  echoed. A 502 HTML page from a proxy became a single 20 KB error message; it
+  is now collapsed to one line and truncated to 400 characters with the dropped
+  byte count disclosed.
+
+### Changed
+- `--timeout` is described as the total budget for one API call including
+  retries, which is what it has always been (`http.Client.Timeout`), rather
+  than "per-request timeout".
+
+### Documentation
+- `CLAUDE.md` claimed `transport.go` retries a POST when `X-Idempotency-Key` is
+  set. It never has: `idempotent()` covers GET/HEAD/PUT/DELETE only, and
+  `transport_test.go` pins that a key does not make a POST retryable on 5xx.
+
 ### Added
 - `--wide` keeps every table column even when the table is wider than the
   terminal.

cmd/cmdutil/args.go

@@ -126,3 +126,30 @@ func suggestionHint(suggestions []string) string {
 	}
 	return b.String()
 }
+
+// NextPage decides whether a paginated walk continues, given the page just
+// fetched and the nextPage/lastPage the API reported alongside it.
+//
+// The only stopping condition used to be `nextPage == nil || *nextPage == 0`,
+// which trusts the server to eventually stop saying "there is more". A server
+// that keeps answering `nextPage: 2` — a caching bug, a filter interaction, a
+// proxy replaying a response — walked forever at the client's full rate limit.
+// `domain list` escaped it by bounding on lastPage; the other seven list
+// commands and record-ID completion did not, and `dns list --all` against such
+// a server never returned.
+//
+// Two guards, both cheap: the page number must advance, and it must not run
+// past lastPage when the API reports one.
+func NextPage(current int32, nextPage, lastPage *int32) (int32, bool) {
+	if nextPage == nil || *nextPage == 0 {
+		return current, false
+	}
+	next := *nextPage
+	if next <= current {
+		return current, false
+	}
+	if lastPage != nil && *lastPage > 0 && next > *lastPage {
+		return current, false
+	}
+	return next, true
+}

cmd/cmdutil/args_test.go

@@ -227,3 +227,61 @@ func TestGroupCmd(t *testing.T) {
 		}
 	})
 }
+
+// TestNextPage guards the paginated-walk stopping conditions.
+//
+// The old condition was `nextPage == nil || *nextPage == 0` alone, which trusts
+// the server to eventually stop saying "there is more". Against one that kept
+// answering nextPage:2, `dns list --all` walked forever at the full client rate
+// limit — confirmed by running it: still going after 20 seconds. domain list
+// escaped only because it bounded on lastPage; the other seven list commands
+// and record-ID completion did not.
+func TestNextPage(t *testing.T) {
+	p := func(v int32) *int32 { return &v }
+
+	tests := []struct {
+		name     string
+		current  int32
+		next     *int32
+		last     *int32
+		wantPage int32
+		wantOK   bool
+	}{
+		{"advances normally", 1, p(2), p(9), 2, true},
+		{"nil next ends the walk", 3, nil, p(9), 3, false},
+		{"zero next ends the walk", 3, p(0), p(9), 3, false},
+		{"a next that repeats the current page ends the walk", 2, p(2), p(99), 2, false},
+		{"a next that goes backwards ends the walk", 5, p(3), p(99), 5, false},
+		{"a next beyond lastPage ends the walk", 9, p(10), p(9), 9, false},
+		{"reaching lastPage exactly is allowed", 8, p(9), p(9), 9, true},
+		{"an unknown lastPage still allows advancing", 1, p(2), nil, 2, true},
+		{"a zero lastPage is treated as unknown", 1, p(2), p(0), 2, true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotPage, gotOK := NextPage(tt.current, tt.next, tt.last)
+			if gotPage != tt.wantPage || gotOK != tt.wantOK {
+				t.Errorf("NextPage(%d, %v, %v) = (%d, %v), want (%d, %v)",
+					tt.current, tt.next, tt.last, gotPage, gotOK, tt.wantPage, tt.wantOK)
+			}
+		})
+	}
+
+	t.Run("a stuck server terminates the walk", func(t *testing.T) {
+		// The exact shape that hung: nextPage pinned at 2 forever.
+		page, steps := int32(1), 0
+		for {
+			next, ok := NextPage(page, p(2), p(99))
+			if !ok {
+				break
+			}
+			page = next
+			if steps++; steps > 100 {
+				t.Fatal("walk did not terminate against a non-advancing nextPage")
+			}
+		}
+		if steps != 1 {
+			t.Errorf("walk took %d steps, want 1 (page 1 -> 2, then stop)", steps)
+		}
+	})
+}

cmd/cmdutil/complete.go

@@ -67,10 +67,11 @@ func CompleteRecordIDs(cmd *cobra.Command, domain string) ([]string, cobra.Shell
 			// "12345\tA @ → 1.2.3.4" — tab separates value from description in zsh/fish
 			completions = append(completions, fmt.Sprintf("%s\t%s %s → %s", id, typ, host, answer))
 		}
-		if result.NextPage == nil || *result.NextPage == 0 {
+		next, ok := NextPage(page, result.NextPage, result.LastPage)
+		if !ok {
 			break
 		}
-		page = *result.NextPage
+		page = next
 	}
 	return completions, cobra.ShellCompDirectiveNoFileComp
 }

cmd/cmdutil/complete_test.go

@@ -0,0 +1,121 @@
+package cmdutil
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/patramsey/namecom-cli/internal/api"
+	"github.com/spf13/cobra"
+)
+
+// cmdWithClient returns a command carrying an API client pointed at srv, the
+// shape completion functions expect to find on the context.
+func cmdWithClient(t *testing.T, srv *httptest.Server) *cobra.Command {
+	t.Helper()
+	client, err := api.New(api.Options{BaseURL: srv.URL})
+	if err != nil {
+		t.Fatalf("api.New: %v", err)
+	}
+	cmd := &cobra.Command{}
+	cmd.SetContext(context.WithValue(context.Background(), KeyClient, client))
+	return cmd
+}
+
+// TestCompleteRecordIDs covers record-ID completion, which had no test at all.
+//
+// It pages, so it carried the same unbounded-walk bug as the list commands —
+// and here the symptom is worse than a hung command: a server whose nextPage
+// never advances hangs the user's shell mid-tab-completion, with no output to
+// explain why and nothing obvious to interrupt.
+func TestCompleteRecordIDs(t *testing.T) {
+	t.Run("returns ids with a descriptive suffix", func(t *testing.T) {
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+			w.Header().Set("Content-Type", "application/json")
+			_, _ = w.Write([]byte(`{"records":[{"id":42,"host":"www","type":"A","answer":"1.2.3.4","ttl":300}]}`))
+		}))
+		t.Cleanup(srv.Close)
+
+		got, directive := CompleteRecordIDs(cmdWithClient(t, srv), "example.com")
+		if directive != cobra.ShellCompDirectiveNoFileComp {
+			t.Errorf("directive = %v, want NoFileComp", directive)
+		}
+		if len(got) != 1 {
+			t.Fatalf("got %d completions, want 1: %v", len(got), got)
+		}
+		// zsh and fish split value from description on the tab, so the ID must
+		// come first and the context after it.
+		id, desc, found := strings.Cut(got[0], "\t")
+		if !found {
+			t.Fatalf("completion %q has no tab separator", got[0])
+		}
+		if id != "42" {
+			t.Errorf("completion value = %q, want the bare record ID %q", id, "42")
+		}
+		for _, want := range []string{"A", "www", "1.2.3.4"} {
+			if !strings.Contains(desc, want) {
+				t.Errorf("description %q does not mention %q", desc, want)
+			}
+		}
+	})
+
+	t.Run("walks every page", func(t *testing.T) {
+		var requests int
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			requests++
+			w.Header().Set("Content-Type", "application/json")
+			if r.URL.Query().Get("page") == "2" {
+				_, _ = w.Write([]byte(`{"records":[{"id":2,"host":"b","type":"A","answer":"2.2.2.2","ttl":300}],"lastPage":2}`))
+				return
+			}
+			_, _ = w.Write([]byte(`{"records":[{"id":1,"host":"a","type":"A","answer":"1.1.1.1","ttl":300}],"nextPage":2,"lastPage":2}`))
+		}))
+		t.Cleanup(srv.Close)
+
+		got, _ := CompleteRecordIDs(cmdWithClient(t, srv), "example.com")
+		if len(got) != 2 {
+			t.Fatalf("got %d completions across 2 pages, want 2: %v", len(got), got)
+		}
+		if requests != 2 {
+			t.Errorf("made %d page requests, want exactly 2", requests)
+		}
+	})
+
+	t.Run("a non-advancing nextPage does not hang the shell", func(t *testing.T) {
+		var requests int
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+			requests++
+			if requests > 10 {
+				t.Errorf("completion did not terminate against a non-advancing nextPage")
+				http.Error(w, "loop", http.StatusInternalServerError)
+				return
+			}
+			w.Header().Set("Content-Type", "application/json")
+			_, _ = w.Write([]byte(`{"records":[{"id":1,"host":"a","type":"A","answer":"1.1.1.1","ttl":300}],"nextPage":2,"lastPage":99}`))
+		}))
+		t.Cleanup(srv.Close)
+
+		CompleteRecordIDs(cmdWithClient(t, srv), "example.com")
+		if requests != 2 {
+			t.Errorf("made %d requests, want 2 (page 1 -> 2, then the page stops advancing)", requests)
+		}
+	})
+
+	t.Run("no client on the context degrades quietly", func(t *testing.T) {
+		// Completion runs before credentials necessarily exist: root.go lets
+		// initContext fail silently for __complete rather than break the shell,
+		// which leaves a context with no client on it. Offering nothing is the
+		// correct answer; erroring would surface in the middle of a tab.
+		cmd := &cobra.Command{}
+		cmd.SetContext(context.Background())
+		got, directive := CompleteRecordIDs(cmd, "example.com")
+		if got != nil {
+			t.Errorf("got %v, want no completions", got)
+		}
+		if directive != cobra.ShellCompDirectiveNoFileComp {
+			t.Errorf("directive = %v, want NoFileComp", directive)
+		}
+	})
+}

cmd/contact/contact.go

@@ -110,7 +110,8 @@ func runUnverified(cmd *cobra.Command, _ []string) error {
 		}
 		contacts = append(contacts, result.UnverifiedContacts...)
 		lastResult = result
-		if result.NextPage == nil || *result.NextPage == 0 {
+		next, ok := cmdutil.NextPage(page, result.NextPage, &result.LastPage)
+		if !ok {
 			break
 		}
 		// --quiet is for scripting, and the "showing first page" hint lives in
@@ -121,7 +122,7 @@ func runUnverified(cmd *cobra.Command, _ []string) error {
 			hasMore = true
 			break
 		}
-		page = *result.NextPage
+		page = next
 	}
 	spin.Stop()
 

cmd/dns/dns.go

@@ -619,14 +619,15 @@ func fetchAllRecords(cmd *cobra.Command, domain string, all bool) (records []gen
 		records = append(records, result.Records...)
 		lastNextPage = result.NextPage
 
-		if result.NextPage == nil || *result.NextPage == 0 {
+		next, ok := cmdutil.NextPage(page, result.NextPage, result.LastPage)
+		if !ok {
 			break
 		}
 		if !all {
 			hasMore = true
 			break
 		}
-		page = *result.NextPage
+		page = next
 	}
 	if hasMore {
 		nextPage = lastNextPage

cmd/email/email.go

@@ -116,14 +116,15 @@ func runList(cmd *cobra.Command, args []string) error {
 		}
 		all = append(all, result.EmailForwarding...)
 		lastResult = result
-		if result.NextPage == nil || *result.NextPage == 0 {
+		next, ok := cmdutil.NextPage(page, result.NextPage, result.LastPage)
+		if !ok {
 			break
 		}
 		if !listAll {
 			hasMore = true
 			break
 		}
-		page = *result.NextPage
+		page = next
 		spin.Update(fmt.Sprintf("Fetching email forwardings… (page %d, %d so far)", page, len(all)))
 	}
 	spin.Stop()