fix(inspect): P3 polish from third PR review (#136)

- Omit empty workflow in /api/runs list JSON (omitempty + test)
- Print BoundAddr after listener is ready in inspect --web
- Validate --port range (1-65535 or 0) with exit 2
- Typed list/state decodes in server_test

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: leo-aa88

internal/cli/inspect_web.go

@@ -7,13 +7,17 @@ import (
 	"os/signal"
 	"strings"
 	"syscall"
+	"time"
 
 	"github.com/LAA-Software-Engineering/agentic-control-plane/internal/inspect"
 	"github.com/LAA-Software-Engineering/agentic-control-plane/internal/state/sqlite"
 	"github.com/spf13/cobra"
 )
 
 func runInspectWeb(cmd *cobra.Command, port int, traceUIBase string) error {
+	if err := inspect.ValidateInspectPort(port); err != nil {
+		return NewExitError(ExitValidationError, err)
+	}
 	g := Globals()
 	graph, root, err := prepareProjectGraph(g.ProjectRoot, g)
 	if err != nil {
@@ -55,15 +59,27 @@ func runInspectWeb(cmd *cobra.Command, port int, traceUIBase string) error {
 		return fmt.Errorf("inspect: %w", err)
 	}
 
-	addr := srv.ListenAddr()
+	runCtx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
+	defer stop()
+
+	errCh := make(chan error, 1)
+	go func() { errCh <- srv.ListenAndServe(runCtx) }()
+
+	deadline := time.Now().Add(3 * time.Second)
+	for !srv.ListenReady() {
+		if time.Now().After(deadline) {
+			return fmt.Errorf("inspect: server did not start listening")
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+
+	addr := srv.BoundAddr()
 	url := "http://" + addr + "/"
 	if _, err := fmt.Fprintf(cmd.OutOrStdout(), "Inspector listening on %s (read-only)\nOpen %s\nPress Ctrl+C to stop.\n", addr, url); err != nil {
 		return err
 	}
 
-	runCtx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
-	defer stop()
-	if err := srv.ListenAndServe(runCtx); err != nil && runCtx.Err() == nil {
+	if err := <-errCh; err != nil && runCtx.Err() == nil {
 		return fmt.Errorf("inspect: server: %w", err)
 	}
 	return nil

internal/cli/inspect_web_test.go

@@ -117,6 +117,24 @@ func TestInspect_web_invalidTraceUI_exit2(t *testing.T) {
 	}
 }
 
+func TestInspect_web_invalidPort_exit2(t *testing.T) {
+	root := runProjRoot(t)
+	db := filepath.Join(t.TempDir(), "port.db")
+
+	ResetGlobalsForTest()
+	cmd := NewRootCmd()
+	cmd.SetOut(io.Discard)
+	cmd.SetErr(io.Discard)
+	cmd.SetArgs([]string{"inspect", "--web", "--project", root, "--state", db, "--port", "99999"})
+	err := cmd.Execute()
+	if err == nil {
+		t.Fatal("expected error")
+	}
+	if ExitCodeOf(err) != ExitValidationError {
+		t.Fatalf("exit=%d err=%v", ExitCodeOf(err), err)
+	}
+}
+
 func TestInspect_web_missingDB_exit2(t *testing.T) {
 	db := filepath.Join(t.TempDir(), "no.db")
 	root := runProjRoot(t)

internal/inspect/port.go

@@ -0,0 +1,14 @@
+package inspect
+
+import "fmt"
+
+// ValidateInspectPort returns an error when port is not 0 (ephemeral) or in 1..65535.
+func ValidateInspectPort(port int) error {
+	if port == 0 {
+		return nil
+	}
+	if port < 1 || port > 65535 {
+		return fmt.Errorf("inspect: port %d out of range (use 1-65535, or 0 for an ephemeral test port)", port)
+	}
+	return nil
+}

internal/inspect/port_test.go

@@ -0,0 +1,26 @@
+package inspect
+
+import "testing"
+
+func TestValidateInspectPort(t *testing.T) {
+	tests := []struct {
+		port  int
+		isErr bool
+	}{
+		{0, false},
+		{8787, false},
+		{1, false},
+		{65535, false},
+		{-1, true},
+		{65536, true},
+	}
+	for _, tc := range tests {
+		err := ValidateInspectPort(tc.port)
+		if tc.isErr && err == nil {
+			t.Fatalf("port %d: want error", tc.port)
+		}
+		if !tc.isErr && err != nil {
+			t.Fatalf("port %d: %v", tc.port, err)
+		}
+	}
+}

internal/inspect/responses.go

@@ -9,7 +9,7 @@ import (
 // ListRunsResponse is GET /api/runs JSON.
 type ListRunsResponse struct {
 	StatePath string                `json:"statePath"`
-	Workflow  string                `json:"workflow"`
+	Workflow  string                `json:"workflow,omitempty"`
 	Runs      []statejson.RunRecord `json:"runs"`
 }
 

internal/inspect/server.go

@@ -83,6 +83,14 @@ func (s *Server) Handler() http.Handler {
 	return securityHeaders(RejectMutation(s.mux))
 }
 
+// ListenReady reports whether [Server.ListenAndServe] has bound a TCP listener.
+func (s *Server) ListenReady() bool {
+	s.mu.RLock()
+	ok := s.boundAddr != ""
+	s.mu.RUnlock()
+	return ok
+}
+
 // BoundAddr returns the address the server is listening on after [Server.ListenAndServe] starts.
 // When Port is 0, this is the kernel-assigned port (127.0.0.1:NNNN).
 func (s *Server) BoundAddr() string {

internal/inspect/server_test.go

@@ -99,13 +99,26 @@ func TestServer_API_readOnly(t *testing.T) {
 		if res.StatusCode != http.StatusOK {
 			t.Fatalf("status=%d", res.StatusCode)
 		}
-		var body map[string]any
+		var body ListRunsResponse
 		if err := json.NewDecoder(res.Body).Decode(&body); err != nil {
 			t.Fatal(err)
 		}
-		runs, ok := body["runs"].([]any)
-		if !ok || len(runs) != 1 {
-			t.Fatalf("runs=%v", body["runs"])
+		if body.Workflow != "" {
+			t.Fatalf("workflow=%q want empty when unfiltered", body.Workflow)
+		}
+		if len(body.Runs) != 1 {
+			t.Fatalf("runs=%v", body.Runs)
+		}
+		raw, err := json.Marshal(body)
+		if err != nil {
+			t.Fatal(err)
+		}
+		var top map[string]json.RawMessage
+		if err := json.Unmarshal(raw, &top); err != nil {
+			t.Fatal(err)
+		}
+		if _, ok := top["workflow"]; ok {
+			t.Fatalf("top-level workflow key in list response: %s", raw)
 		}
 	})
 
@@ -148,13 +161,12 @@ func TestServer_API_readOnly(t *testing.T) {
 			t.Fatal(err)
 		}
 		defer res.Body.Close()
-		var body map[string]any
+		var body StateResponse
 		if err := json.NewDecoder(res.Body).Decode(&body); err != nil {
 			t.Fatal(err)
 		}
-		resources, _ := body["resources"].([]any)
-		if len(resources) != 1 {
-			t.Fatalf("resources=%v", body["resources"])
+		if len(body.Resources) != 1 {
+			t.Fatalf("resources=%v", body.Resources)
 		}
 	})