[codex] Add deployment details review before starting models

internal/mcp/tools_deploy.go

@@ -4,9 +4,103 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"strings"
 )
 
 func registerDeployTools(s *Server, deps *ToolDeps) {
+	// deploy.defaults
+	s.RegisterTool(&Tool{
+		Name:        "deploy.defaults",
+		Description: "Get, set, or clear device-local default deployment settings for one model. This stores operator preference in local system config, not reusable AIMA knowledge.",
+		InputSchema: schema(
+			`"action":{"type":"string","enum":["get","set","clear"],"description":"Operation to perform."},`+
+				`"model":{"type":"string","description":"Model name whose deployment defaults should be managed."},`+
+				`"engine":{"type":"string","description":"Default engine override when action=set."},`+
+				`"slot":{"type":"string","description":"Default slot when action=set."},`+
+				`"no_pull":{"type":"boolean","description":"Default resource policy when action=set."},`+
+				`"port":{"type":"string","description":"Default port value when action=set."},`+
+				`"config":{"type":"object","description":"Default engine config overrides when action=set."}`,
+			"action", "model"),
+		Handler: func(ctx context.Context, params json.RawMessage) (*ToolResult, error) {
+			var p struct {
+				Action string         `json:"action"`
+				Model  string         `json:"model"`
+				Engine string         `json:"engine"`
+				Slot   string         `json:"slot"`
+				NoPull *bool          `json:"no_pull"`
+				Port   string         `json:"port"`
+				Config map[string]any `json:"config"`
+			}
+			if err := json.Unmarshal(params, &p); err != nil {
+				return nil, fmt.Errorf("parse params: %w", err)
+			}
+			model := strings.TrimSpace(p.Model)
+			if model == "" {
+				return ErrorResult("model is required"), nil
+			}
+			key := deployDefaultsConfigKey(model)
+			action := strings.ToLower(strings.TrimSpace(p.Action))
+			switch action {
+			case "get":
+				if deps.GetConfig == nil {
+					return ErrorResult("deploy.defaults get not implemented"), nil
+				}
+				raw, err := deps.GetConfig(ctx, key)
+				if err != nil || strings.TrimSpace(raw) == "" {
+					if err != nil && !isMissingConfigValue(err) {
+						return nil, fmt.Errorf("get deploy defaults for %s: %w", model, err)
+					}
+					data, _ := json.Marshal(map[string]any{"model": model, "exists": false})
+					return TextResult(string(data)), nil
+				}
+				var stored map[string]any
+				if err := json.Unmarshal([]byte(raw), &stored); err != nil {
+					return nil, fmt.Errorf("parse deploy defaults for %s: %w", model, err)
+				}
+				data, _ := json.Marshal(map[string]any{"model": model, "exists": true, "defaults": stored})
+				return TextResult(string(data)), nil
+			case "set":
+				if deps.SetConfig == nil {
+					return ErrorResult("deploy.defaults set not implemented"), nil
+				}
+				noPull := true
+				if p.NoPull != nil {
+					noPull = *p.NoPull
+				}
+				payload := map[string]any{
+					"engine":  strings.TrimSpace(p.Engine),
+					"slot":    strings.TrimSpace(p.Slot),
+					"no_pull": noPull,
+					"port":    strings.TrimSpace(p.Port),
+					"config":  p.Config,
+				}
+				if payload["config"] == nil {
+					payload["config"] = map[string]any{}
+				}
+				raw, err := json.Marshal(payload)
+				if err != nil {
+					return nil, fmt.Errorf("marshal deploy defaults for %s: %w", model, err)
+				}
+				if err := deps.SetConfig(ctx, key, string(raw)); err != nil {
+					return nil, fmt.Errorf("set deploy defaults for %s: %w", model, err)
+				}
+				data, _ := json.Marshal(map[string]any{"model": model, "exists": true, "defaults": payload})
+				return TextResult(string(data)), nil
+			case "clear":
+				if deps.SetConfig == nil {
+					return ErrorResult("deploy.defaults clear not implemented"), nil
+				}
+				if err := deps.SetConfig(ctx, key, ""); err != nil {
+					return nil, fmt.Errorf("clear deploy defaults for %s: %w", model, err)
+				}
+				data, _ := json.Marshal(map[string]any{"model": model, "exists": false})
+				return TextResult(string(data)), nil
+			default:
+				return ErrorResult("action must be one of: get, set, clear"), nil
+			}
+		},
+	})
+
 	// deploy.apply
 	s.RegisterTool(&Tool{
 		Name:        "deploy.apply",
@@ -283,3 +377,18 @@ func registerDeployTools(s *Server, deps *ToolDeps) {
 		},
 	})
 }
+
+func deployDefaultsConfigKey(model string) string {
+	normalized := strings.TrimSpace(strings.ToLower(model))
+	normalized = strings.ReplaceAll(normalized, "\\", "_")
+	normalized = strings.ReplaceAll(normalized, "/", "_")
+	return "deploy.defaults." + normalized
+}
+
+func isMissingConfigValue(err error) bool {
+	if err == nil {
+		return false
+	}
+	text := strings.ToLower(err.Error())
+	return strings.Contains(text, "not found") || strings.Contains(text, "no rows")
+}

internal/mcp/tools_deploy_test.go

@@ -3,6 +3,7 @@ package mcp
 import (
 	"context"
 	"encoding/json"
+	"strings"
 	"testing"
 
 	"github.com/jguan/aima/internal/engine"
@@ -64,6 +65,138 @@ func TestDeployRunPassesConfigOverrides(t *testing.T) {
 	}
 }
 
+func TestDeployDryRunPassesConfigOverrides(t *testing.T) {
+	s := NewServer()
+
+	var (
+		gotModel  string
+		gotEngine string
+		gotSlot   string
+		gotConfig map[string]any
+	)
+	registerDeployTools(s, &ToolDeps{
+		DeployDryRun: func(ctx context.Context, engineType, model, slot string, configOverrides map[string]any) (json.RawMessage, error) {
+			gotModel = model
+			gotEngine = engineType
+			gotSlot = slot
+			gotConfig = configOverrides
+			return json.RawMessage(`{"status":"preview"}`), nil
+		},
+	})
+
+	result, err := s.ExecuteTool(context.Background(), "deploy.dry_run", json.RawMessage(`{
+		"model":"qwen3-8b",
+		"engine":"vllm",
+		"slot":"slot-1",
+		"config":{"gpu_memory_utilization":0.8,"kv_cache_dtype":"fp8"},
+		"max_cold_start_s":12
+	}`))
+	if err != nil {
+		t.Fatalf("ExecuteTool: %v", err)
+	}
+
+	if gotModel != "qwen3-8b" {
+		t.Fatalf("model = %q, want qwen3-8b", gotModel)
+	}
+	if gotEngine != "vllm" {
+		t.Fatalf("engine = %q, want vllm", gotEngine)
+	}
+	if gotSlot != "slot-1" {
+		t.Fatalf("slot = %q, want slot-1", gotSlot)
+	}
+	if gotConfig["gpu_memory_utilization"] != 0.8 {
+		t.Fatalf("gpu_memory_utilization = %#v, want 0.8", gotConfig["gpu_memory_utilization"])
+	}
+	if gotConfig["kv_cache_dtype"] != "fp8" {
+		t.Fatalf("kv_cache_dtype = %#v, want fp8", gotConfig["kv_cache_dtype"])
+	}
+	if gotConfig["max_cold_start_s"] != float64(12) && gotConfig["max_cold_start_s"] != 12 {
+		t.Fatalf("max_cold_start_s = %#v, want 12", gotConfig["max_cold_start_s"])
+	}
+	if len(result.Content) == 0 || result.IsError {
+		t.Fatalf("unexpected result = %+v", result)
+	}
+}
+
+func TestDeployDefaultsStoresDeviceLocalSettings(t *testing.T) {
+	s := NewServer()
+	store := map[string]string{}
+	registerDeployTools(s, &ToolDeps{
+		GetConfig: func(ctx context.Context, key string) (string, error) {
+			value, ok := store[key]
+			if !ok {
+				return "", context.Canceled
+			}
+			return value, nil
+		},
+		SetConfig: func(ctx context.Context, key, value string) error {
+			store[key] = value
+			return nil
+		},
+	})
+
+	setResult, err := s.ExecuteTool(context.Background(), "deploy.defaults", json.RawMessage(`{
+		"action":"set",
+		"model":"Qwen3-8B",
+		"engine":"vllm",
+		"slot":"slot-1",
+		"no_pull":true,
+		"port":"8003",
+		"config":{"gpu_memory_utilization":0.7,"max_model_len":8192}
+	}`))
+	if err != nil {
+		t.Fatalf("set ExecuteTool: %v", err)
+	}
+	if setResult.IsError {
+		t.Fatalf("set returned error: %+v", setResult)
+	}
+
+	getResult, err := s.ExecuteTool(context.Background(), "deploy.defaults", json.RawMessage(`{"action":"get","model":"qwen3-8b"}`))
+	if err != nil {
+		t.Fatalf("get ExecuteTool: %v", err)
+	}
+	if getResult.IsError {
+		t.Fatalf("get returned error: %+v", getResult)
+	}
+	var got struct {
+		Exists   bool `json:"exists"`
+		Defaults struct {
+			Engine string         `json:"engine"`
+			Slot   string         `json:"slot"`
+			NoPull bool           `json:"no_pull"`
+			Port   string         `json:"port"`
+			Config map[string]any `json:"config"`
+		} `json:"defaults"`
+	}
+	if err := json.Unmarshal([]byte(getResult.Content[0].Text), &got); err != nil {
+		t.Fatalf("unmarshal get result: %v", err)
+	}
+	if !got.Exists {
+		t.Fatal("defaults should exist")
+	}
+	if got.Defaults.Engine != "vllm" || got.Defaults.Slot != "slot-1" || !got.Defaults.NoPull || got.Defaults.Port != "8003" {
+		t.Fatalf("defaults = %+v", got.Defaults)
+	}
+	if got.Defaults.Config["gpu_memory_utilization"] != float64(0.7) {
+		t.Fatalf("gpu_memory_utilization = %#v, want 0.7", got.Defaults.Config["gpu_memory_utilization"])
+	}
+
+	clearResult, err := s.ExecuteTool(context.Background(), "deploy.defaults", json.RawMessage(`{"action":"clear","model":"qwen3-8b"}`))
+	if err != nil {
+		t.Fatalf("clear ExecuteTool: %v", err)
+	}
+	if clearResult.IsError {
+		t.Fatalf("clear returned error: %+v", clearResult)
+	}
+	getResult, err = s.ExecuteTool(context.Background(), "deploy.defaults", json.RawMessage(`{"action":"get","model":"qwen3-8b"}`))
+	if err != nil {
+		t.Fatalf("get after clear ExecuteTool: %v", err)
+	}
+	if !strings.Contains(getResult.Content[0].Text, `"exists":false`) {
+		t.Fatalf("get after clear = %s, want exists=false", getResult.Content[0].Text)
+	}
+}
+
 func TestDeployApplyPassesNoPull(t *testing.T) {
 	s := NewServer()
 

internal/ui/handler_test.go

@@ -483,6 +483,52 @@ func TestRegisterRoutes_IndexIncludesDeploymentStageFeedback(t *testing.T) {
 	}
 }
 
+func TestRegisterRoutes_IndexDeployDetailUsesBackendDefaultsAndImmediateClose(t *testing.T) {
+	t.Parallel()
+
+	mux := http.NewServeMux()
+	RegisterRoutes(nil)(mux)
+
+	req := httptest.NewRequest(http.MethodGet, "/ui/", nil)
+	rec := httptest.NewRecorder()
+	mux.ServeHTTP(rec, req)
+
+	body := rec.Body.String()
+	for _, token := range []string{
+		`this.callTool('deploy.defaults', { action: 'get', model: modelName })`,
+		`this.callTool('deploy.defaults', { action: 'set', model: modelName, ...payload })`,
+		`const data = await this.callTool('deploy.run', request);`,
+		`this.deployDetailOpen = false;`,
+		`await this.refreshDeployDryRun();`,
+		`if (!kvApplied) suggestions.push({ key: 'kv_cache_dtype', value: 'fp8'`,
+		`deploy_started_background`,
+		`deploy_restore_recommended: 'Recommended parameters'`,
+	} {
+		if !strings.Contains(body, token) {
+			t.Fatalf("body missing deploy detail token %q", token)
+		}
+	}
+
+	start := strings.Index(body, "async confirmDeployDetail() {")
+	if start == -1 {
+		t.Fatal("confirmDeployDetail not found")
+	}
+	end := strings.Index(body[start:], "\n    componentStatusNote(model)")
+	if end == -1 {
+		t.Fatal("could not isolate confirmDeployDetail body")
+	}
+	fnBody := body[start : start+end]
+	closeIdx := strings.Index(fnBody, `this.deployDetailOpen = false;`)
+	runIdx := strings.Index(fnBody, `const data = await this.callTool('deploy.run', request);`)
+	if closeIdx == -1 || runIdx == -1 || closeIdx > runIdx {
+		t.Fatalf("deploy detail should close before awaiting deploy.run, body=%s", fnBody)
+	}
+
+	if strings.Contains(body, "aima_deploy_defaults:") || strings.Contains(body, "localStorage.setItem(this.deployDefaultsKey()") {
+		t.Fatal("deploy defaults should not be stored only in browser localStorage")
+	}
+}
+
 func TestRegisterRoutes_IndexIncludesDirectModeRoutingAndModelCards(t *testing.T) {
 	t.Parallel()