#52: simplify internal cleanup paths

internal/app/app.go

@@ -61,6 +61,8 @@ func newApplication(
 	client *http.Client,
 	issuer, codexURL string,
 ) (_ *application, err error) {
+	// Keep constructor validation for tests that build config directly.
+	// Production Run passes an already validated loadConfig result.
 	if err := c.validate(); err != nil {
 		return nil, err
 	}
@@ -119,8 +121,6 @@ func newApplication(
 	router := chi.NewRouter()
 	router.Handle("/api/*", admin)
 	router.Handle("/api", admin)
-	router.Handle("/v1/*", clients)
-	router.Handle("/v1", clients)
 	router.NotFound(clients.ServeHTTP)
 	if ctx.Err() != nil {
 		return nil, fmt.Errorf("app: startup canceled: %w", ctx.Err())

internal/app/config.go

@@ -1,6 +1,7 @@
 package app
 
 import (
+	"cmp"
 	"encoding/base64"
 	"errors"
 	"net"
@@ -19,20 +20,15 @@ type config struct {
 }
 
 func loadConfig(getenv func(string) string) (config, error) {
-	value := func(name, fallback string) string {
-		if found := getenv(name); found != "" {
-			return found
-		}
-		return fallback
-	}
 	c := config{
-		listenAddr:   value("CLAN_LISTEN_ADDR", "127.0.0.1:8080"),
-		dbPath:       value("CLAN_DB_PATH", "./clan.db"),
+		listenAddr:   cmp.Or(getenv("CLAN_LISTEN_ADDR"), "127.0.0.1:8080"),
+		dbPath:       cmp.Or(getenv("CLAN_DB_PATH"), "./clan.db"),
 		adminToken:   getenv("CLAN_ADMIN_TOKEN"),
-		callbackAddr: value("CLAN_OAUTH_CALLBACK_ADDR", "127.0.0.1:1455"),
+		callbackAddr: cmp.Or(getenv("CLAN_OAUTH_CALLBACK_ADDR"), "127.0.0.1:1455"),
 	}
 	encoded := getenv("CLAN_ENCRYPTION_KEY")
 	key, err := base64.StdEncoding.Strict().DecodeString(encoded)
+	// Report malformed encoding and invalid lengths with the same safe env error.
 	if err != nil || len(key) != 32 || strings.ContainsAny(encoded, "\r\n") {
 		return config{}, errors.New("app: CLAN_ENCRYPTION_KEY must be standard base64 encoding of 32 bytes")
 	}
@@ -48,6 +44,8 @@ func (c config) validate() error {
 	if management.ValidateConfig(management.Config{AdminToken: c.adminToken}) != nil {
 		return errors.New("app: CLAN_ADMIN_TOKEN must contain a separate valid admin token")
 	}
+	// Keep the key-length invariant for configs constructed directly in tests;
+	// production loadConfig already checks the decoded key length.
 	if len(c.encryptionKey) != 32 {
 		return errors.New("app: CLAN_ENCRYPTION_KEY must decode to 32 bytes")
 	}
@@ -59,8 +57,8 @@ func (c config) validate() error {
 		{name: "CLAN_OAUTH_CALLBACK_ADDR", value: c.callbackAddr},
 	} {
 		_, port, err := net.SplitHostPort(address.value)
-		number, parseErr := strconv.Atoi(port)
-		if err != nil || parseErr != nil || number < 0 || number > 65535 {
+		_, parseErr := strconv.ParseUint(port, 10, 16)
+		if err != nil || parseErr != nil {
 			return errors.New("app: " + address.name + " must be a TCP host:port address")
 		}
 	}

internal/app/config_test.go

@@ -60,3 +60,64 @@ func TestLoadConfigRejectsInvalidValuesWithoutDisclosingThem(t *testing.T) {
 		})
 	}
 }
+
+func TestNewApplicationRejectsInvalidDirectConfig(t *testing.T) {
+	tests := []struct {
+		name   string
+		modify func(*config)
+		want   string
+	}{
+		{
+			name: "admin token",
+			modify: func(c *config) {
+				c.adminToken = "clan_secret-marker"
+			},
+			want: "CLAN_ADMIN_TOKEN",
+		},
+		{
+			name: "encryption key length",
+			modify: func(c *config) {
+				c.encryptionKey = c.encryptionKey[:31]
+			},
+			want: "CLAN_ENCRYPTION_KEY",
+		},
+		{
+			name: "blank database path",
+			modify: func(c *config) {
+				c.dbPath = " "
+			},
+			want: "CLAN_DB_PATH",
+		},
+		{
+			name: "invalid listen port",
+			modify: func(c *config) {
+				c.listenAddr = "localhost:65536"
+			},
+			want: "CLAN_LISTEN_ADDR",
+		},
+		{
+			name: "invalid callback port",
+			modify: func(c *config) {
+				c.callbackAddr = "localhost:-1"
+			},
+			want: "CLAN_OAUTH_CALLBACK_ADDR",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := testConfig(t)
+			tt.modify(&c)
+
+			// A bad config must fail before dependency setup.
+			_, err := newApplication(t.Context(), c, nil, nil, "", "")
+
+			if err == nil || !strings.Contains(err.Error(), tt.want) {
+				t.Fatalf("direct config validation = %v, want %s", err, tt.want)
+			}
+			if strings.Contains(err.Error(), "secret-marker") {
+				t.Fatalf("validation disclosed the supplied secret: %v", err)
+			}
+		})
+	}
+}

internal/codex/catalog.go

@@ -264,9 +264,9 @@ func (c *Catalog) Snapshot(ctx context.Context) (CatalogSnapshot, error) {
 		}
 		result.Accounts = append(result.Accounts, catalog)
 		loaded = loaded || entry.loaded
-		for _, model := range cloneModels(entry.models) {
+		for _, model := range entry.models {
 			if model.Listed && !listed[model.ID] {
-				result.Listed = append(result.Listed, model)
+				result.Listed = append(result.Listed, cloneModel(model))
 				listed[model.ID] = true
 			}
 		}
@@ -320,11 +320,16 @@ func (c *Catalog) refresh(ctx context.Context, entry *catalogEntry, a account.Ac
 	}
 }
 
+func cloneModel(model Model) Model {
+	model.ReasoningEfforts = slices.Clone(model.ReasoningEfforts)
+	model.InputModalities = slices.Clone(model.InputModalities)
+	return model
+}
+
 func cloneModels(models []Model) []Model {
 	cloned := slices.Clone(models)
 	for i := range cloned {
-		cloned[i].ReasoningEfforts = slices.Clone(cloned[i].ReasoningEfforts)
-		cloned[i].InputModalities = slices.Clone(cloned[i].InputModalities)
+		cloned[i] = cloneModel(cloned[i])
 	}
 	return cloned
 }

internal/codex/catalog_test.go

@@ -613,3 +613,66 @@ func TestCatalogCloseWaitsForCanceledRefreshCleanup(t *testing.T) {
 		})
 	}
 }
+
+func TestCatalogSnapshotListedAndAccountModelsAreIndependent(t *testing.T) {
+	c, err := codex.NewCatalog(func(context.Context, account.Account) ([]codex.Model, error) {
+		return []codex.Model{{
+			ID:               "m",
+			Listed:           true,
+			ReasoningEfforts: []string{"low"},
+			InputModalities:  []string{"text"},
+		}}, nil
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(c.Close)
+
+	if err := c.SetAccounts([]account.Account{testAccount(t, "one")}); err != nil {
+		t.Fatal(err)
+	}
+
+	snapshot, err := c.Snapshot(t.Context())
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(snapshot.Accounts) != 1 ||
+		len(snapshot.Accounts[0].Models) != 1 ||
+		len(snapshot.Listed) != 1 {
+		t.Fatalf("unexpected snapshot: %+v", snapshot)
+	}
+
+	accountModel := &snapshot.Accounts[0].Models[0]
+	listedModel := &snapshot.Listed[0]
+
+	accountModel.ReasoningEfforts[0] = "changed-account"
+	accountModel.InputModalities[0] = "changed-account"
+
+	if listedModel.ReasoningEfforts[0] != "low" ||
+		listedModel.InputModalities[0] != "text" {
+		t.Error("mutating account models changed the listed union")
+	}
+
+	listedModel.ReasoningEfforts[0] = "changed-listed"
+	listedModel.InputModalities[0] = "changed-listed"
+
+	if accountModel.ReasoningEfforts[0] != "changed-account" ||
+		accountModel.InputModalities[0] != "changed-account" {
+		t.Error("mutating the listed union changed account models")
+	}
+
+	fresh, err := c.Snapshot(t.Context())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for _, model := range []codex.Model{
+		fresh.Accounts[0].Models[0],
+		fresh.Listed[0],
+	} {
+		if model.ReasoningEfforts[0] != "low" ||
+			model.InputModalities[0] != "text" {
+			t.Errorf("snapshot mutation changed cached models: %+v", model)
+		}
+	}
+}

internal/codex/client.go

@@ -53,6 +53,7 @@ func (e *Failure) Unwrap() error { return e.cause }
 // Usage remains available when the attempt fails; unknown counts are not zero counts.
 type Result struct {
 	Response json.RawMessage
+	Status   string // Empty until a valid terminal response is received.
 	Usage    usage.Snapshot
 }
 
@@ -95,6 +96,9 @@ func NewClient(client *http.Client, baseURL, clientVersion string) (*Client, err
 	}
 	if standard, ok := transport.(*http.Transport); ok {
 		cloned := standard.Clone()
+		// DialContext takes precedence over Dial. Keep the legacy check so
+		// callers with a custom Dial are not silently switched to our dialer.
+		//nolint:staticcheck // Preserve caller-supplied legacy dialers when checking deprecated Dial.
 		if cloned.DialContext == nil && cloned.Dial == nil {
 			cloned.DialContext = (&net.Dialer{Timeout: 30 * time.Second, KeepAlive: 30 * time.Second}).DialContext
 		}

internal/codex/client_test.go

@@ -29,6 +29,40 @@ func httpResponse(status int, contentType, body string) *http.Response {
 	return &http.Response{StatusCode: status, Header: http.Header{"Content-Type": []string{contentType}}, Body: io.NopCloser(strings.NewReader(body))}
 }
 
+func TestClientPreservesLegacyTransportDial(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = io.WriteString(w, `{"models":[]}`)
+	}))
+	defer server.Close()
+
+	var dialCalls atomic.Int32
+	transport := &http.Transport{
+		//nolint:staticcheck // Exercise deprecated Dial to protect existing callers.
+		Dial: func(network, addr string) (net.Conn, error) {
+			dialCalls.Add(1)
+			return net.DialTimeout(network, addr, 3*time.Second)
+		},
+	}
+
+	client := testClient(t, &http.Client{Transport: transport}, server.URL)
+	defer client.CloseIdleConnections()
+
+	models, err := client.FetchModels(t.Context(), testAccount(t, "one"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(models) != 0 {
+		t.Fatalf("models = %+v, want an empty catalog", models)
+	}
+	if got := dialCalls.Load(); got != 1 {
+		t.Fatalf("legacy Dial calls = %d, want 1", got)
+	}
+	if transport.DialContext != nil {
+		t.Error("NewClient mutated the supplied transport")
+	}
+}
+
 func TestClientClosesItsClonedConnectionPool(t *testing.T) {
 	synctest.Test(t, func(t *testing.T) {
 		clientConn, serverConn := net.Pipe()

internal/codex/stream.go

@@ -177,7 +177,7 @@ func (s *Stream) readEvent() (sse.Event, error, bool) {
 func (s *Stream) Result() Result {
 	s.readMu.Lock()
 	defer s.readMu.Unlock()
-	return Result{Response: slices.Clone(s.result.Response), Usage: s.result.Usage}
+	return Result{Response: slices.Clone(s.result.Response), Status: s.result.Status, Usage: s.result.Usage}
 }
 
 // Err returns the known terminal failure, including when Next returned its final event.
@@ -224,11 +224,12 @@ func (s *Stream) closeBody() {
 	})
 }
 
-func (s *Stream) finish(err error) {
+func (s *Stream) finish(failure *Failure) {
 	s.ended = true
-	var failure *Failure
-	if errors.As(err, &failure) {
+	var err error
+	if failure != nil {
 		failure.RetryAfter = s.retryAfter
+		err = failure
 	}
 	s.err = err
 	s.closeBody()
@@ -242,7 +243,7 @@ func (s *Stream) badEvent() (json.RawMessage, error) {
 	return nil, s.err
 }
 
-func (s *Stream) terminal(fields map[string]json.RawMessage, kind string) (json.RawMessage, error, error) {
+func (s *Stream) terminal(fields map[string]json.RawMessage, kind string) (json.RawMessage, *Failure, error) {
 	response, err := object(fields["response"], "response", "")
 	if err != nil {
 		return nil, nil, err
@@ -283,7 +284,7 @@ func (s *Stream) terminal(fields map[string]json.RawMessage, kind string) (json.
 		}
 		response["output"], _ = marshalJSON(output)
 	}
-	var failure error
+	var failure *Failure
 	if status == "failed" {
 		classified := responseFailure(response["error"], s.status)
 		response["error"] = safeErrorJSON(classified)
@@ -293,6 +294,7 @@ func (s *Stream) terminal(fields map[string]json.RawMessage, kind string) (json.
 	if err != nil {
 		return nil, nil, err
 	}
+	s.result.Status = status
 	fields["response"] = s.result.Response
 	raw, err := marshalJSON(fields)
 	return raw, failure, err

internal/codex/stream_test.go

@@ -175,6 +175,9 @@ func TestStreamPreservesOrderAndAssemblesCompletedItems(t *testing.T) {
 	want := `{"id":"r","status":"completed","output":[{"type":"reasoning","id":"rs","summary":[],"encrypted_content":"opaque"},{"type":"function_call","id":"fc_a","name":"a","call_id":"c_a","arguments":"{\"a\":1}"},{"type":"function_call","id":"fc_b","name":"b","call_id":"c_b","arguments":"{\"b\":2}"}],"usage":{"output_tokens":0,"total_tokens":2}}`
 	assertJSON(t, result.Response, want)
 	assertJSON(t, ordinary.Response, want)
+	if result.Status != "completed" || ordinary.Status != "completed" {
+		t.Errorf("terminal statuses = %q, %q; want completed", result.Status, ordinary.Status)
+	}
 	var final struct {
 		Response json.RawMessage `json:"response"`
 	}
@@ -203,6 +206,9 @@ func TestTerminalOutputIsAuthoritativeAndIncompleteIsDistinct(t *testing.T) {
 				t.Fatalf("terminal %s is not an attempt error: %v", status, err)
 			}
 			assertJSON(t, result.Response, final)
+			if result.Status != status {
+				t.Errorf("result status = %q, want %q", result.Status, status)
+			}
 			if result.Usage.Input.Known || result.Usage.Output.Known || result.Usage.Total.Known {
 				t.Errorf("unknown usage became known: %+v", result.Usage)
 			}
@@ -271,6 +277,9 @@ func TestFailedTerminalKeepsUsageAndSanitizesError(t *testing.T) {
 			}
 			_, err = stream.Next()
 			result := stream.Result()
+			if result.Status != "failed" {
+				t.Errorf("result status = %q, want failed", result.Status)
+			}
 
 			var failure *codex.Failure
 			if !errors.As(err, &failure) || failure.Category != tt.category || failure.HTTPStatus != 200 || failure.SafeToRetry {