fix(hoverclient): read NS from /api/domains (not PUT-only control_panel GET) — v0.5.4

cmd/workflow-plugin-hover/plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "workflow-plugin-hover",
-  "version": "0.5.3",
+  "version": "0.5.4",
   "description": "Hover DNS provider (browser-style login + TOTP, no official SDK)",
   "author": "GoCodeAlone",
   "license": "MIT",

pkg/hoverclient/client.go

@@ -50,6 +50,11 @@ type Client struct {
 	loggedAt  time.Time
 	UserAgent string
 	backend   executionBackend
+	// domainNS caches nameservers keyed by domain name. Populated by
+	// listDomainsHTTP so GetDomainDelegation can short-circuit the
+	// per-domain GET when a prior ListDomains call has already fetched them.
+	// Guarded by mu.
+	domainNS map[string][]string
 }
 
 // NewClient returns a fresh Client. Pass httpClient=nil for the browser
@@ -393,9 +398,10 @@ type DNSRecord struct {
 
 // Domain is the API shape returned by GET /api/domains.
 type Domain struct {
-	ID      string      `json:"id"`
-	Name    string      `json:"domain_name"`
-	Records []DNSRecord `json:"entries"`
+	ID          string      `json:"id"`
+	Name        string      `json:"domain_name"`
+	Records     []DNSRecord `json:"entries"`
+	Nameservers []string    `json:"nameservers"`
 }
 
 // GetDomainDelegation fetches the registrar-level nameserver delegation for
@@ -412,12 +418,38 @@ func (c *Client) GetDomainDelegation(ctx context.Context, domainName string) (*D
 }
 
 // getDomainDelegationHTTP is the HTTP-backend implementation of GetDomainDelegation.
+//
+// Read path (in priority order):
+//  1. NS cache (populated by listDomainsHTTP from GET /api/domains).
+//     A non-empty cache hit short-circuits the network call entirely.
+//  2. Per-domain fallback: GET /api/domains/<name>
+//     Used when no prior ListDomains call has populated the cache.
+//
+// The old GET /api/control_panel/domains/domain-<name> is PUT-only on live
+// Hover (returns 404 on GET). It is never used for reads.
 func (c *Client) getDomainDelegationHTTP(ctx context.Context, domainName string) (*DomainDelegation, error) {
 	if err := c.ensureLogin(ctx); err != nil {
 		return nil, err
 	}
-	endpoint := fmt.Sprintf("%s/api/control_panel/domains/domain-%s", hoverHost, url.PathEscape(domainName))
+
+	// 1. Check NS cache (populated by ListDomains).
+	c.mu.Lock()
+	cached, ok := c.domainNS[domainName]
+	if ok {
+		ns := make([]string, len(cached))
+		copy(ns, cached)
+		c.mu.Unlock()
+		if len(ns) == 0 {
+			return nil, fmt.Errorf("hover: GetDomainDelegation %q: %w", domainName, ErrEmptyNameservers)
+		}
+		return &DomainDelegation{Name: domainName, Nameservers: ns}, nil
+	}
+	c.mu.Unlock()
+
+	// 2. Cache miss — fall back to per-domain GET /api/domains/<name>.
+	endpoint := fmt.Sprintf("%s/api/domains/%s", hoverHost, url.PathEscape(domainName))
 	req, _ := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil)
+	req.Header.Set("Accept", "application/json")
 	req.Header.Set("User-Agent", c.UserAgent)
 	resp, err := c.do(req)
 	if err != nil {
@@ -428,14 +460,22 @@ func (c *Client) getDomainDelegationHTTP(ctx context.Context, domainName string)
 		body, _ := io.ReadAll(io.LimitReader(resp.Body, 512))
 		return nil, fmt.Errorf("hover: GetDomainDelegation %q: HTTP %d: %s", domainName, resp.StatusCode, strings.TrimSpace(string(body)))
 	}
-	var d DomainDelegation
-	if err := json.NewDecoder(resp.Body).Decode(&d); err != nil {
+	// The per-domain endpoint wraps the domain in {"succeeded":...,"domain":{...}}.
+	var wrap struct {
+		Succeeded bool   `json:"succeeded"`
+		Domain    Domain `json:"domain"`
+	}
+	if err := json.NewDecoder(resp.Body).Decode(&wrap); err != nil {
 		return nil, fmt.Errorf("hover: GetDomainDelegation %q: decode: %w", domainName, err)
 	}
-	if len(d.Nameservers) == 0 {
+	if len(wrap.Domain.Nameservers) == 0 {
 		return nil, fmt.Errorf("hover: GetDomainDelegation %q: %w", domainName, ErrEmptyNameservers)
 	}
-	return &d, nil
+	return &DomainDelegation{
+		ID:          wrap.Domain.ID,
+		Name:        wrap.Domain.Name,
+		Nameservers: wrap.Domain.Nameservers,
+	}, nil
 }
 
 // SetNameservers updates the registrar-level nameservers for a domain via
@@ -551,6 +591,22 @@ func (c *Client) listDomainsHTTP(ctx context.Context) ([]Domain, error) {
 	if !body.Succeeded {
 		return nil, fmt.Errorf("hover: ListDomains: API returned succeeded=false")
 	}
+	// Populate the NS cache so subsequent GetDomainDelegation calls can
+	// short-circuit the per-domain GET (the list endpoint already includes
+	// nameservers for every domain in one call).
+	c.mu.Lock()
+	if c.domainNS == nil {
+		c.domainNS = make(map[string][]string, len(body.Domains))
+	}
+	for _, d := range body.Domains {
+		if d.Name == "" {
+			continue
+		}
+		ns := make([]string, len(d.Nameservers))
+		copy(ns, d.Nameservers)
+		c.domainNS[d.Name] = ns
+	}
+	c.mu.Unlock()
 	return body.Domains, nil
 }
 

pkg/hoverclient/client_test.go

@@ -464,11 +464,13 @@ func TestFetchControlPanelCSRFLocked_Non2xx(t *testing.T) {
 }
 
 func TestGetDomainDelegation_HappyPath(t *testing.T) {
+	// No prior ListDomains → cache is empty → falls back to GET /api/domains/<name>.
+	// The per-domain endpoint wraps the result in {"succeeded":true,"domain":{...}}.
 	c, srv := newStubClient(t, func(w http.ResponseWriter, r *http.Request) {
-		if r.URL.Path != "/api/control_panel/domains/domain-example.com" {
+		if r.URL.Path != "/api/domains/example.com" {
 			t.Errorf("unexpected path: %s", r.URL.Path)
 		}
-		_, _ = w.Write([]byte(`{"id":"domain-example.com","domain_name":"example.com","nameservers":["ns1.do.com","ns2.do.com"]}`))
+		_, _ = w.Write([]byte(`{"succeeded":true,"domain":{"id":"domain-example.com","domain_name":"example.com","nameservers":["ns1.do.com","ns2.do.com"]}}`))
 	})
 	defer srv.Close()
 	c.loggedAt = time.Now()
@@ -486,8 +488,9 @@ func TestGetDomainDelegation_HappyPath(t *testing.T) {
 }
 
 func TestGetDomainDelegation_EmptyNameserversReturnsSentinel(t *testing.T) {
+	// No cache → per-domain GET returns empty nameservers → ErrEmptyNameservers.
 	c, srv := newStubClient(t, func(w http.ResponseWriter, r *http.Request) {
-		_, _ = w.Write([]byte(`{"id":"domain-example.com","domain_name":"example.com","nameservers":[]}`))
+		_, _ = w.Write([]byte(`{"succeeded":true,"domain":{"id":"domain-example.com","domain_name":"example.com","nameservers":[]}}`))
 	})
 	defer srv.Close()
 	c.loggedAt = time.Now()
@@ -828,6 +831,138 @@ func TestClient_429GivesUpAfterMax(t *testing.T) {
 	}
 }
 
+// ── delegation read endpoint fix tests ───────────────────────────────────────
+
+// TestListDomains_ParsesNameservers verifies that /api/domains includes
+// nameservers for each domain and they are surfaced in the returned []Domain.
+func TestListDomains_ParsesNameservers(t *testing.T) {
+	respBody := `{
+		"succeeded": true,
+		"domains": [
+			{"id":"dom1","domain_name":"a.com","nameservers":["ns1.x","ns2.x"]},
+			{"id":"dom2","domain_name":"b.com","nameservers":[]}
+		]
+	}`
+	c, srv := newRecordStub(t, func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodGet || r.URL.Path != "/api/domains" {
+			t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path)
+			http.Error(w, "unexpected", http.StatusBadRequest)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = io.WriteString(w, respBody)
+	})
+	defer srv.Close()
+
+	domains, err := c.ListDomains(context.Background())
+	if err != nil {
+		t.Fatalf("ListDomains: %v", err)
+	}
+	if len(domains) != 2 {
+		t.Fatalf("want 2 domains; got %d", len(domains))
+	}
+	if got := domains[0].Nameservers; len(got) != 2 || got[0] != "ns1.x" || got[1] != "ns2.x" {
+		t.Errorf("domains[0].Nameservers = %v; want [ns1.x ns2.x]", got)
+	}
+	if got := domains[1].Nameservers; len(got) != 0 {
+		t.Errorf("domains[1].Nameservers = %v; want empty", got)
+	}
+}
+
+// TestGetDomainDelegation_UsesCacheFromListDomains verifies that after
+// ListDomains populates the NS cache, GetDomainDelegation returns cached
+// values without hitting /api/domains/<name>.
+func TestGetDomainDelegation_UsesCacheFromListDomains(t *testing.T) {
+	var perDomainHits int
+
+	mux := http.NewServeMux()
+	mux.HandleFunc("/signin/auth.json", func(w http.ResponseWriter, _ *http.Request) {
+		_ = json.NewEncoder(w).Encode(map[string]any{"status": "completed"})
+	})
+	mux.HandleFunc("/api/domains", func(w http.ResponseWriter, r *http.Request) {
+		// Return a.com with ns1.x — only exact /api/domains (not /api/domains/*)
+		if r.URL.Path != "/api/domains" {
+			// This is a per-domain fallback hit — count it.
+			perDomainHits++
+			http.Error(w, "should not be called", http.StatusInternalServerError)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = io.WriteString(w, `{"succeeded":true,"domains":[{"id":"dom1","domain_name":"a.com","nameservers":["ns1.x"]}]}`)
+	})
+
+	srv := httptest.NewServer(mux)
+	defer srv.Close()
+
+	jar, _ := cookiejar.New(nil)
+	httpc := &http.Client{Jar: jar, Transport: rewriteTransport{base: srv.URL}}
+	c, err := NewClient(Credentials{Username: "alice", Password: "pw"}, httpc)
+	if err != nil {
+		t.Fatalf("NewClient: %v", err)
+	}
+
+	// Populate the cache.
+	if _, err := c.ListDomains(context.Background()); err != nil {
+		t.Fatalf("ListDomains: %v", err)
+	}
+
+	// Now call GetDomainDelegation — should use the cache, NOT the per-domain endpoint.
+	d, err := c.GetDomainDelegation(context.Background(), "a.com")
+	if err != nil {
+		t.Fatalf("GetDomainDelegation: %v", err)
+	}
+	if len(d.Nameservers) != 1 || d.Nameservers[0] != "ns1.x" {
+		t.Errorf("Nameservers = %v; want [ns1.x]", d.Nameservers)
+	}
+	if perDomainHits > 0 {
+		t.Errorf("per-domain endpoint was hit %d times; want 0 (cache should serve)", perDomainHits)
+	}
+}
+
+// TestGetDomainDelegation_FallsBackToPerDomainGET verifies that when the cache
+// is empty (no prior ListDomains), GetDomainDelegation falls back to
+// GET /api/domains/<name> and does NOT hit /api/control_panel/domains/domain-<name>.
+func TestGetDomainDelegation_FallsBackToPerDomainGET(t *testing.T) {
+	var controlPanelHits int
+
+	mux := http.NewServeMux()
+	mux.HandleFunc("/signin/auth.json", func(w http.ResponseWriter, _ *http.Request) {
+		_ = json.NewEncoder(w).Encode(map[string]any{"status": "completed"})
+	})
+	mux.HandleFunc("/api/domains/", func(w http.ResponseWriter, r *http.Request) {
+		// Per-domain GET: /api/domains/a.com
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = io.WriteString(w, `{"succeeded":true,"domain":{"id":"dom1","domain_name":"a.com","nameservers":["ns9.x"]}}`)
+	})
+	mux.HandleFunc("/api/control_panel/", func(w http.ResponseWriter, r *http.Request) {
+		controlPanelHits++
+		http.Error(w, "should not be called", http.StatusNotFound)
+	})
+
+	srv := httptest.NewServer(mux)
+	defer srv.Close()
+
+	jar, _ := cookiejar.New(nil)
+	httpc := &http.Client{Jar: jar, Transport: rewriteTransport{base: srv.URL}}
+	c, err := NewClient(Credentials{Username: "alice", Password: "pw"}, httpc)
+	if err != nil {
+		t.Fatalf("NewClient: %v", err)
+	}
+	c.loggedAt = time.Now() // skip login
+
+	// No ListDomains call — cache is empty.
+	d, err := c.GetDomainDelegation(context.Background(), "a.com")
+	if err != nil {
+		t.Fatalf("GetDomainDelegation: %v", err)
+	}
+	if len(d.Nameservers) != 1 || d.Nameservers[0] != "ns9.x" {
+		t.Errorf("Nameservers = %v; want [ns9.x]", d.Nameservers)
+	}
+	if controlPanelHits > 0 {
+		t.Errorf("control_panel endpoint was hit %d times; want 0", controlPanelHits)
+	}
+}
+
 // TestNewClientWithOptions_PreservesExplicitBrowserConfig verifies that
 // explicit ClientOptions.Browser values survive NewClientWithOptions.
 func TestNewClientWithOptions_PreservesExplicitBrowserConfig(t *testing.T) {

plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "workflow-plugin-hover",
-  "version": "0.5.3",
+  "version": "0.5.4",
   "description": "Hover DNS provider (browser-style login + TOTP, no official SDK)",
   "author": "GoCodeAlone",
   "license": "MIT",