Merge pull request '[agent/claude:opus] DX audit and fix. 1) Review CLAUDE.md — update any outdate...' (#1) from agent/dx-audit-and-fix--1--review-claude-md into main

CLAUDE.md

@@ -19,6 +19,8 @@ go vet ./...                # vet check
 - `go test -race ./...` and `go vet ./...` must pass before commit
 - Conventional commits: `type(scope): description`
 - Co-Author: `Co-Authored-By: Virgil <virgil@lethean.io>`
+- Errors must use `log.E()` from `go-log`, never `fmt.Errorf`
+- Config tests must isolate from real `~/.core/config.yaml` via `t.Setenv("HOME", t.TempDir())`
 
 ## Docs
 

docs/architecture.md

@@ -13,6 +13,14 @@ config resolution from file, environment, and CLI flags.
 
 ```
 go-netops/
+├── cmd/unifi/             # CLI commands (cobra)
+│   ├── cmd_unifi.go       # Root 'unifi' command registration
+│   ├── cmd_clients.go     # 'unifi clients' command
+│   ├── cmd_config.go      # 'unifi config' command
+│   ├── cmd_devices.go     # 'unifi devices' command
+│   ├── cmd_networks.go    # 'unifi networks' command
+│   ├── cmd_routes.go      # 'unifi routes' command
+│   └── cmd_sites.go       # 'unifi sites' command
 ├── unifi/
 │   ├── client.go          # Client struct and constructor (New)
 │   ├── clients.go         # Connected client queries with filtering
@@ -22,7 +30,9 @@ go-netops/
 │   ├── routes.go          # Gateway routing table queries
 │   ├── sites.go           # Site listing
 │   ├── client_test.go     # Client creation tests
-│   └── config_test.go     # Config resolution and persistence tests
+│   ├── config_test.go     # Config resolution and persistence tests
+│   ├── networks_test.go   # Network query tests
+│   └── routes_test.go     # Route query and type name tests
 ├── go.mod
 └── go.sum
 ```

docs/development.md

@@ -6,12 +6,9 @@ Module: `forge.lthn.ai/core/go-netops`
 
 | Tool       | Version   | Notes                             |
 |------------|-----------|-----------------------------------|
-| Go         | 1.25+     | Module uses `go 1.25.5`          |
-| core/go    | local     | `replace` directive in go.mod     |
+| Go         | 1.26+     | Module uses `go 1.26.0`          |
 
-The `go.mod` contains a `replace` directive pointing at `../go` for the
-`forge.lthn.ai/core/go` dependency. Ensure the `core/go` repository is
-checked out alongside this one (the standard workspace layout handles this).
+Dependencies are resolved from the Forge registry (`forge.lthn.ai`).
 
 ## Build and Test
 
@@ -60,19 +57,12 @@ Config tests isolate the environment by:
 
 ### What Is Not Tested
 
-The query methods (`GetClients`, `GetDevices`, `GetDeviceList`, `GetNetworks`,
-`GetRoutes`, `GetSites`) require a live or fully-mocked UniFi controller and
-are not covered by unit tests. Current coverage is **39%**.
+The client-discovery methods (`GetClients`, `GetDevices`, `GetDeviceList`,
+`GetSites`) delegate to the unpoller SDK and would require integration tests
+against a real or emulated controller to cover meaningfully.
 
-## Coverage
-
-```
-ok  forge.lthn.ai/core/go-netops/unifi  coverage: 39.0% of statements
-```
-
-Coverage is concentrated on the config resolution and client construction paths.
-The query methods delegate to the unpoller SDK and would require integration
-tests against a real or emulated controller to cover meaningfully.
+`GetNetworks`, `GetRoutes`, and `RouteTypeName` are covered by unit tests
+using httptest mocks and direct assertions.
 
 ## Coding Standards
 

unifi/config.go

@@ -141,5 +141,9 @@ func SaveConfig(url, user, pass, apikey string, insecure *bool) error {
 		}
 	}
 
+	if err := cfg.Commit(); err != nil {
+		return log.E("unifi.SaveConfig", "failed to write config file", err)
+	}
+
 	return nil
 }

unifi/config_test.go

@@ -11,7 +11,8 @@ import (
 )
 
 func TestResolveConfig(t *testing.T) {
-	// Clear environment variables to start clean
+	// Isolate from real config file and environment
+	t.Setenv("HOME", t.TempDir())
 	os.Unsetenv("UNIFI_URL")
 	os.Unsetenv("UNIFI_USER")
 	os.Unsetenv("UNIFI_PASS")
@@ -74,6 +75,14 @@ func TestResolveConfig(t *testing.T) {
 }
 
 func TestNewFromConfig(t *testing.T) {
+	// Isolate from real config file and environment
+	t.Setenv("HOME", t.TempDir())
+	os.Unsetenv("UNIFI_URL")
+	os.Unsetenv("UNIFI_USER")
+	os.Unsetenv("UNIFI_PASS")
+	os.Unsetenv("UNIFI_APIKEY")
+	os.Unsetenv("UNIFI_INSECURE")
+
 	// Mock UniFi controller
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
@@ -88,8 +97,6 @@ func TestNewFromConfig(t *testing.T) {
 	assert.Equal(t, ts.URL, client.URL())
 
 	// 2. Error case: No credentials
-	os.Unsetenv("UNIFI_USER")
-	os.Unsetenv("UNIFI_APIKEY")
 	client, err = NewFromConfig("", "", "", "", nil)
 	assert.Error(t, err)
 	assert.Nil(t, client)

unifi/networks_test.go

@@ -0,0 +1,95 @@
+package unifi
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetNetworks(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		// New-style controller: paths are prefixed with /proxy/network
+		if r.URL.Path == "/proxy/network/api/s/default/rest/networkconf" {
+			fmt.Fprintln(w, `{"data":[{"_id":"abc123","name":"LAN","purpose":"corporate","ip_subnet":"10.69.1.0/24","vlan":0,"vlan_enabled":false,"enabled":true,"networkgroup":"LAN","dhcpd_enabled":true,"dhcpd_start":"10.69.1.100","dhcpd_stop":"10.69.1.254"}]}`)
+			return
+		}
+		fmt.Fprintln(w, `{"meta":{"rc":"ok"}, "data": []}`)
+	}))
+	defer ts.Close()
+
+	client, err := New(ts.URL, "user", "pass", "", true)
+	assert.NoError(t, err)
+
+	networks, err := client.GetNetworks("")
+	assert.NoError(t, err)
+	if assert.Len(t, networks, 1) {
+		assert.Equal(t, "abc123", networks[0].ID)
+		assert.Equal(t, "LAN", networks[0].Name)
+		assert.Equal(t, "corporate", networks[0].Purpose)
+		assert.Equal(t, "10.69.1.0/24", networks[0].IPSubnet)
+		assert.True(t, networks[0].Enabled)
+		assert.True(t, networks[0].DHCPEnabled)
+		assert.Equal(t, "10.69.1.100", networks[0].DHCPStart)
+		assert.Equal(t, "10.69.1.254", networks[0].DHCPStop)
+	}
+}
+
+func TestGetNetworks_CustomSite(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		if r.URL.Path == "/proxy/network/api/s/office/rest/networkconf" {
+			fmt.Fprintln(w, `{"data":[{"_id":"def456","name":"Office LAN","purpose":"corporate","ip_subnet":"172.16.0.0/24"}]}`)
+			return
+		}
+		fmt.Fprintln(w, `{"meta":{"rc":"ok"}, "data": []}`)
+	}))
+	defer ts.Close()
+
+	client, err := New(ts.URL, "user", "pass", "", true)
+	assert.NoError(t, err)
+
+	networks, err := client.GetNetworks("office")
+	assert.NoError(t, err)
+	if assert.Len(t, networks, 1) {
+		assert.Equal(t, "Office LAN", networks[0].Name)
+	}
+}
+
+func TestGetNetworks_Empty(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		fmt.Fprintln(w, `{"meta":{"rc":"ok"}, "data": []}`)
+	}))
+	defer ts.Close()
+
+	client, err := New(ts.URL, "user", "pass", "", true)
+	assert.NoError(t, err)
+
+	networks, err := client.GetNetworks("")
+	assert.NoError(t, err)
+	assert.Empty(t, networks)
+}
+
+func TestGetNetworks_InvalidJSON(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		if r.URL.Path == "/proxy/network/api/s/default/rest/networkconf" {
+			fmt.Fprintln(w, `not valid json`)
+			return
+		}
+		fmt.Fprintln(w, `{"meta":{"rc":"ok"}, "data": []}`)
+	}))
+	defer ts.Close()
+
+	client, err := New(ts.URL, "user", "pass", "", true)
+	assert.NoError(t, err)
+
+	networks, err := client.GetNetworks("")
+	assert.Error(t, err)
+	assert.Nil(t, networks)
+	assert.Contains(t, err.Error(), "failed to parse networks")
+}

unifi/routes_test.go

@@ -0,0 +1,92 @@
+package unifi
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRouteTypeName(t *testing.T) {
+	tests := []struct {
+		code     string
+		expected string
+	}{
+		{"S", "static"},
+		{"C", "connected"},
+		{"K", "kernel"},
+		{"B", "bgp"},
+		{"O", "ospf"},
+		{"X", "X"},
+		{"", ""},
+	}
+
+	for _, tt := range tests {
+		assert.Equal(t, tt.expected, RouteTypeName(tt.code))
+	}
+}
+
+func TestGetRoutes(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		// New-style controller: paths are prefixed with /proxy/network
+		if r.URL.Path == "/proxy/network/api/s/default/stat/routing" {
+			fmt.Fprintln(w, `{"data":[{"pfx":"10.0.0.0/24","nh":"10.0.0.1","intf":"br0","type":"C","distance":0,"metric":0,"uptime":3600,"fib":true}]}`)
+			return
+		}
+		fmt.Fprintln(w, `{"meta":{"rc":"ok"}, "data": []}`)
+	}))
+	defer ts.Close()
+
+	client, err := New(ts.URL, "user", "pass", "", true)
+	assert.NoError(t, err)
+
+	routes, err := client.GetRoutes("")
+	assert.NoError(t, err)
+	if assert.Len(t, routes, 1) {
+		assert.Equal(t, "10.0.0.0/24", routes[0].Network)
+		assert.Equal(t, "10.0.0.1", routes[0].NextHop)
+		assert.Equal(t, "br0", routes[0].Interface)
+		assert.Equal(t, "C", routes[0].Type)
+		assert.True(t, routes[0].Selected)
+	}
+}
+
+func TestGetRoutes_CustomSite(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		if r.URL.Path == "/proxy/network/api/s/mysite/stat/routing" {
+			fmt.Fprintln(w, `{"data":[{"pfx":"192.168.1.0/24","nh":"192.168.1.1","intf":"eth0","type":"S","distance":1,"metric":0,"uptime":0,"fib":true}]}`)
+			return
+		}
+		fmt.Fprintln(w, `{"meta":{"rc":"ok"}, "data": []}`)
+	}))
+	defer ts.Close()
+
+	client, err := New(ts.URL, "user", "pass", "", true)
+	assert.NoError(t, err)
+
+	routes, err := client.GetRoutes("mysite")
+	assert.NoError(t, err)
+	if assert.Len(t, routes, 1) {
+		assert.Equal(t, "192.168.1.0/24", routes[0].Network)
+		assert.Equal(t, "S", routes[0].Type)
+	}
+}
+
+func TestGetRoutes_Empty(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		fmt.Fprintln(w, `{"meta":{"rc":"ok"}, "data": []}`)
+	}))
+	defer ts.Close()
+
+	client, err := New(ts.URL, "user", "pass", "", true)
+	assert.NoError(t, err)
+
+	routes, err := client.GetRoutes("")
+	assert.NoError(t, err)
+	assert.Empty(t, routes)
+}