fix(forge-cli): optimizer stop no longer restores the proxy's own dead URL

forge-cli/cmd/optimizer_daemon.go

@@ -202,8 +202,14 @@ func runOptimizerStart(_ *cobra.Command, _ []string) error {
 		}
 	}
 
-	// Wire Claude Code settings so every `claude` inherits the base URL.
-	prevBase, prevTS, err := wireClaudeSettings(listen)
+	// Wire Claude Code settings so every `claude` inherits the base URL. A prior
+	// state file carries the genuine pre-forge values; a re-start must hand them
+	// over rather than recapture settings forge itself wrote.
+	var prior *optDaemonState
+	if p, perr := readOptDaemonState(); perr == nil {
+		prior = &p
+	}
+	prevBase, prevTS, err := wireClaudeSettings(listen, prior)
 	if err != nil {
 		logger.Warn("could not update ~/.claude/settings.json; set ANTHROPIC_BASE_URL yourself", "error", err.Error())
 	} else {
@@ -450,9 +456,42 @@ func captureStr(env map[string]any, key string) *string {
 	return nil
 }
 
+// forgeWiredBaseURL reports whether raw is a base URL an earlier `start` wrote:
+// our current listen addr, or the one the prior daemon state recorded (starting
+// on a different --listen leaves the old address behind in settings.json).
+func forgeWiredBaseURL(raw, listen string, prior *optDaemonState) bool {
+	s := strings.TrimSuffix(strings.TrimSpace(raw), "/")
+	if s == "" {
+		return false
+	}
+	if s == "http://"+listen {
+		return true
+	}
+	return prior != nil && prior.Listen != "" && s == "http://"+prior.Listen
+}
+
+// sanitizePrevWiring keeps `stop` honest. A value read back from settings.json
+// is only a genuine "previous" one when forge did not write it itself; when it
+// did, the real pre-forge value is whatever the prior state file already holds
+// (nil = the key was absent). Without this, a second `start` records the
+// optimizer's own URL as prev_base_url and `stop` dutifully restores it,
+// leaving every `claude` pointed at a dead proxy with ECONNREFUSED.
+func sanitizePrevWiring(prevBase, prevTS *string, listen string, prior *optDaemonState) (*string, *string) {
+	if prevBase == nil || !forgeWiredBaseURL(*prevBase, listen, prior) {
+		return prevBase, prevTS
+	}
+	// Already wired by us, so the ENABLE_TOOL_SEARCH we read back is ours too —
+	// neither value may be trusted. Carry the prior originals forward instead.
+	if prior != nil {
+		return prior.PrevBaseURL, prior.PrevToolSearch
+	}
+	return nil, nil
+}
+
 // wireClaudeSettings sets ANTHROPIC_BASE_URL + ENABLE_TOOL_SEARCH under `env`,
 // returning the prior values (nil = key was absent) so stop can revert exactly.
-func wireClaudeSettings(listen string) (*string, *string, error) {
+// prior is the state a previous `start` left behind, or nil on a clean start.
+func wireClaudeSettings(listen string, prior *optDaemonState) (*string, *string, error) {
 	m, err := readClaudeSettings()
 	if err != nil {
 		return nil, nil, err
@@ -461,8 +500,11 @@ func wireClaudeSettings(listen string) (*string, *string, error) {
 	if env == nil {
 		env = map[string]any{}
 	}
-	prevBase := captureStr(env, "ANTHROPIC_BASE_URL")
-	prevTS := captureStr(env, "ENABLE_TOOL_SEARCH")
+	prevBase, prevTS := sanitizePrevWiring(
+		captureStr(env, "ANTHROPIC_BASE_URL"),
+		captureStr(env, "ENABLE_TOOL_SEARCH"),
+		listen, prior,
+	)
 	env["ANTHROPIC_BASE_URL"] = "http://" + listen
 	env["ENABLE_TOOL_SEARCH"] = "true"
 	m["env"] = env

forge-cli/cmd/optimizer_daemon_test.go

@@ -148,3 +148,109 @@ func TestResolveChildUpstream_EnvChainingCaptured(t *testing.T) {
 		t.Errorf("self-pointing settings should fall back to env, got %q", got)
 	}
 }
+
+func strptr(s string) *string { return &s }
+
+func TestSanitizePrevWiring(t *testing.T) {
+	gateway := "https://gw.corp.example"
+	tests := []struct {
+		name     string
+		prevBase *string
+		prevTS   *string
+		listen   string
+		prior    *optDaemonState
+		wantBase *string
+		wantTS   *string
+	}{
+		{
+			name:     "absent key stays absent",
+			listen:   "127.0.0.1:8787",
+			wantBase: nil, wantTS: nil,
+		},
+		{
+			name:     "real gateway is captured untouched",
+			prevBase: strptr(gateway), prevTS: strptr("false"),
+			listen:   "127.0.0.1:8787",
+			wantBase: strptr(gateway), wantTS: strptr("false"),
+		},
+		{
+			name:     "our own listen is never recorded as previous",
+			prevBase: strptr("http://127.0.0.1:8787"), prevTS: strptr("true"),
+			listen:   "127.0.0.1:8787",
+			wantBase: nil, wantTS: nil,
+		},
+		{
+			name:     "trailing slash on our own listen still detected",
+			prevBase: strptr("http://127.0.0.1:8787/"), prevTS: strptr("true"),
+			listen:   "127.0.0.1:8787",
+			wantBase: nil, wantTS: nil,
+		},
+		{
+			name:     "re-start carries the prior state's real gateway forward",
+			prevBase: strptr("http://127.0.0.1:8787"), prevTS: strptr("true"),
+			listen:   "127.0.0.1:8787",
+			prior:    &optDaemonState{Listen: "127.0.0.1:8787", PrevBaseURL: strptr(gateway), PrevToolSearch: strptr("false")},
+			wantBase: strptr(gateway), wantTS: strptr("false"),
+		},
+		{
+			name:     "start on a new port still spots the old wiring",
+			prevBase: strptr("http://127.0.0.1:8787"), prevTS: strptr("true"),
+			listen:   "127.0.0.1:9000",
+			prior:    &optDaemonState{Listen: "127.0.0.1:8787", PrevBaseURL: nil, PrevToolSearch: nil},
+			wantBase: nil, wantTS: nil,
+		},
+		{
+			name:     "a different localhost service is a real previous value",
+			prevBase: strptr("http://127.0.0.1:3000"), prevTS: strptr("true"),
+			listen:   "127.0.0.1:8787",
+			prior:    &optDaemonState{Listen: "127.0.0.1:8787"},
+			wantBase: strptr("http://127.0.0.1:3000"), wantTS: strptr("true"),
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			gotBase, gotTS := sanitizePrevWiring(tc.prevBase, tc.prevTS, tc.listen, tc.prior)
+			if !eqStrPtr(gotBase, tc.wantBase) {
+				t.Errorf("base = %s, want %s", showStrPtr(gotBase), showStrPtr(tc.wantBase))
+			}
+			if !eqStrPtr(gotTS, tc.wantTS) {
+				t.Errorf("tool search = %s, want %s", showStrPtr(gotTS), showStrPtr(tc.wantTS))
+			}
+		})
+	}
+}
+
+// TestSanitizePrevWiring_StopNeverRestoresDeadProxy is the regression for the
+// reported break: start -> start -> stop must not leave `claude` pointed at the
+// optimizer's own address once the proxy is gone.
+func TestSanitizePrevWiring_StopNeverRestoresDeadProxy(t *testing.T) {
+	const listen = "127.0.0.1:8787"
+
+	// First start: nothing was set before, so nothing is recorded.
+	base, ts := sanitizePrevWiring(nil, nil, listen, nil)
+	first := optDaemonState{Listen: listen, PrevBaseURL: base, PrevToolSearch: ts}
+
+	// Second start reads back what the first one wrote.
+	base, ts = sanitizePrevWiring(strptr("http://"+listen), strptr("true"), listen, &first)
+
+	if base != nil {
+		t.Fatalf("prev_base_url = %q, want nil; stop would restore a dead proxy", *base)
+	}
+	if ts != nil {
+		t.Fatalf("prev_tool_search = %q, want nil; stop would leave ENABLE_TOOL_SEARCH on", *ts)
+	}
+}
+
+func eqStrPtr(a, b *string) bool {
+	if a == nil || b == nil {
+		return a == b
+	}
+	return *a == *b
+}
+
+func showStrPtr(p *string) string {
+	if p == nil {
+		return "<nil>"
+	}
+	return `"` + *p + `"`
+}