refactor(cli,ai,container,sandbox): extract AIRuntimeOptions and improve error handling

Extract shared AI runtime configuration into AIRuntimeOptions struct to enable reuse by non-prompt commands (e.g., gavel's ai-fix loop) without inheriting prompt-specific fields. Update ToRequest() to accept prompt parameters explicitly rather than reading from struct fields.

Add proper error handling for artifact writes and log syncs in fixture runner, capturing and returning errors instead of silently ignoring them.

Remove unused functions: printLeft() in history_render, stageCategoryFiles/copyFile/copyDir in container/generate, hasOAuth/extractOAuthToken in container/sandbox, and quoteShellArg/quoteShellArgs in sandbox/sandbox_utils.

Add isolateSavedAI() test helper to prevent ~/.captain.yaml from leaking into test expectations. Add TestAIRuntimeOptions_ToRequest_OverlaysSaved to verify saved config overlays work correctly for non-prompt callers.

Fix struct field alignment in SandboxConfig.

Author: moshloop

pkg/ai/fixture/runner.go

@@ -382,12 +382,16 @@ func executeRun(parent context.Context, fixtureDir string, run Run, opts Options
 	defer stopHeartbeat()
 
 	inflight := map[string]*pendingCall{}
+	var artifactErr error
 
 	for scanner.Scan() {
 		line := scanner.Bytes()
-		if artifact != nil {
-			artifact.Write(line)
-			artifact.Write([]byte{'\n'})
+		if artifact != nil && artifactErr == nil {
+			if _, err := artifact.Write(line); err != nil {
+				artifactErr = fmt.Errorf("writing artifact for %q: %w", run.Name, err)
+			} else if _, err := artifact.Write([]byte{'\n'}); err != nil {
+				artifactErr = fmt.Errorf("writing artifact for %q: %w", run.Name, err)
+			}
 		}
 		ev, ok := ParseLine(line)
 		if !ok {
@@ -402,15 +406,26 @@ func executeRun(parent context.Context, fixtureDir string, run Run, opts Options
 
 	runErr := cmd.Wait()
 	flushPendingCalls(inflight, &summary)
+	var syncErr error
 	if kubectlLog != nil {
-		kubectlLog.Sync()
+		if err := kubectlLog.Sync(); err != nil {
+			syncErr = fmt.Errorf("syncing kubectl api log: %w", err)
+		}
 	}
 	if mcpLog != nil {
-		mcpLog.Sync()
+		if err := mcpLog.Sync(); err != nil && syncErr == nil {
+			syncErr = fmt.Errorf("syncing mcp api log: %w", err)
+		}
 	}
 	if runErr != nil {
 		return summary, claudeRunError(run.Name, stderrBuf, summary, runErr)
 	}
+	if artifactErr != nil {
+		return summary, artifactErr
+	}
+	if syncErr != nil {
+		return summary, syncErr
+	}
 	if kubectlLog != nil {
 		summary.KubectlAPILog = readKubectlAPILog(env.kubectlLogPath)
 		summary.KubectlAPICalls = len(summary.KubectlAPILog)

pkg/cli/ai.go

@@ -68,16 +68,21 @@ func (o AIProviderOptions) ToConfig() ai.Config {
 	return cfg
 }
 
-type AIPromptOptions struct {
+// AIRuntimeOptions binds the per-invocation knobs every AI command shares —
+// model selection (via embedded AIProviderOptions), generation parameters
+// (max tokens, temperature, timeout, reasoning), permission/sandbox toggles
+// (edit, allowed/disallowed tools, permission mode), and ambient-context
+// toggles (mcp/hooks/skills/user/project/memory/bare). It deliberately
+// omits the user-prompt fields so non-prompt commands (e.g. gavel's lint
+// --ai-fix loop) can embed it without inheriting a required --prompt flag.
+//
+// AIPromptOptions embeds this struct and adds Prompt/System/AppendSystem/
+// NoStream on top.
+type AIRuntimeOptions struct {
 	AIProviderOptions
-	Prompt       string `flag:"prompt" help:"Prompt text" short:"p" required:"true" stdin:"true"`
-	System       string `flag:"system" help:"System prompt" short:"s"`
-	AppendSystem string `flag:"append-system" help:"Append text to the default system prompt"`
-	MaxTokens    int    `flag:"max-tokens" help:"Maximum output tokens" default:"4096"`
-	Temperature  string `flag:"temperature" help:"Sampling temperature" default:"0"`
-	Timeout      string `flag:"timeout" help:"Request timeout" default:"120s"`
 
-	NoStream bool `flag:"no-stream" help:"Disable streaming; print only the final text to stdout"`
+	MaxTokens   int    `flag:"max-tokens" help:"Maximum output tokens" default:"4096"`
+	Temperature string `flag:"temperature" help:"Sampling temperature" default:"0"`
 
 	Edit            bool     `flag:"edit" help:"Safe defaults: acceptEdits + Read/Edit/Write/Glob/Grep allowlist"`
 	AllowedTools    []string `flag:"allowed-tools" help:"Override --edit's built-in allowlist (claude only)"`
@@ -94,6 +99,16 @@ type AIPromptOptions struct {
 	Bare      bool     `flag:"bare" help:"Skip hooks, skills, memory, and ambient settings"`
 }
 
+type AIPromptOptions struct {
+	AIRuntimeOptions
+
+	Prompt       string `flag:"prompt" help:"Prompt text" short:"p" required:"true" stdin:"true"`
+	System       string `flag:"system" help:"System prompt" short:"s"`
+	AppendSystem string `flag:"append-system" help:"Append text to the default system prompt"`
+	Timeout      string `flag:"timeout" help:"Request timeout" default:"120s"`
+	NoStream     bool   `flag:"no-stream" help:"Disable streaming; print only the final text to stdout"`
+}
+
 type AIPromptResult struct {
 	Text     string  `json:"text" pretty:"label=Response"`
 	Model    string  `json:"model" pretty:"label=Model"`
@@ -104,25 +119,29 @@ type AIPromptResult struct {
 	Duration string  `json:"duration" pretty:"label=Duration"`
 }
 
-// ToRequest translates the user-facing AIPromptOptions into the typed
-// ai.Request. Truthy flags like --mcp/--hooks/--memory invert into the
-// No*-style fields the providers consume. When --bare is set, it implicitly
-// strips memory/hooks/skills/user/project regardless of those flags' values
-// (claude --bare composes them) so we let the provider decide the final argv.
+// ToRequest translates the runtime knobs into the typed ai.Request. Truthy
+// flags like --mcp/--hooks/--memory invert into the No*-style fields the
+// providers consume. When --bare is set, it implicitly strips memory/hooks/
+// skills/user/project regardless of those flags' values (claude --bare
+// composes them) so we let the provider decide the final argv.
+//
+// systemPrompt / appendSystemPrompt / userPrompt are passed explicitly so
+// non-prompt callers (gavel's ai-fix loop) can build them per-iteration
+// without leaking those fields into the shared runtime struct.
 //
 // Saved defaults from ~/.captain.yaml overlay onto unset fields. For the
-// boolean toggles, "saved off" wins over "flag default on" because clicky does
-// not yet expose a Changed() bit.
+// boolean toggles, "saved off" wins over "flag default on" because clicky
+// does not yet expose a Changed() bit.
 //
-// WORKAROUND(no-flag-changed-bit): The boolean toggles default to true, so we
-// cannot tell whether --mcp=true was passed explicitly or inherited from the
-// default. As a result, when the saved config has NoMCP=true the user cannot
-// force MCP back on from the command line by passing --mcp=true alone — they
-// must edit ~/.captain.yaml or rerun `captain configure`.
+// WORKAROUND(no-flag-changed-bit): The boolean toggles default to true, so
+// we cannot tell whether --mcp=true was passed explicitly or inherited from
+// the default. As a result, when the saved config has NoMCP=true the user
+// cannot force MCP back on from the command line by passing --mcp=true alone
+// — they must edit ~/.captain.yaml or rerun `captain configure`.
 // Correct fix: thread clicky's per-flag Changed() bit (or a tri-state flag
-// type) through AIPromptOptions so we can distinguish "explicitly true" from
-// "default true". Ref: discussed with user 2026-05-07.
-func (o AIPromptOptions) ToRequest() ai.Request {
+// type) through AIRuntimeOptions so we can distinguish "explicitly true"
+// from "default true". Ref: discussed with user 2026-05-07.
+func (o AIRuntimeOptions) ToRequest(systemPrompt, appendSystemPrompt, userPrompt string) ai.Request {
 	saved := loadSavedAI()
 	temperature, _ := strconv.ParseFloat(o.Temperature, 64)
 
@@ -134,9 +153,9 @@ func (o AIPromptOptions) ToRequest() ai.Request {
 	effort := saved.ReasoningEffort
 
 	return ai.Request{
-		SystemPrompt:       o.System,
-		AppendSystemPrompt: o.AppendSystem,
-		Prompt:             o.Prompt,
+		SystemPrompt:       systemPrompt,
+		AppendSystemPrompt: appendSystemPrompt,
+		Prompt:             userPrompt,
 		MaxTokens:          maxTokens,
 		Temperature:        temperature,
 		ReasoningEffort:    effort,
@@ -155,6 +174,12 @@ func (o AIPromptOptions) ToRequest() ai.Request {
 	}
 }
 
+// ToRequest delegates to AIRuntimeOptions.ToRequest, lifting the prompt
+// fields the prompt-shaped command owns onto the typed request.
+func (o AIPromptOptions) ToRequest() ai.Request {
+	return o.AIRuntimeOptions.ToRequest(o.System, o.AppendSystem, o.Prompt)
+}
+
 func RunAIPrompt(opts AIPromptOptions) (any, error) {
 	if opts.Prompt == "" {
 		return nil, fmt.Errorf("prompt text required (use --prompt or pipe via stdin)")

pkg/cli/ai_test.go

@@ -1,11 +1,29 @@
 package cli
 
 import (
+	"os"
+	"path/filepath"
 	"reflect"
 	"testing"
+
+	"github.com/flanksource/captain/pkg/captainconfig"
 )
 
+// isolateSavedAI redirects captainconfig.Path() to an empty file inside
+// t.TempDir() so loadSavedAI() returns zero defaults rather than leaking
+// the developer's real ~/.captain.yaml into table-test expectations.
+func isolateSavedAI(t *testing.T) {
+	t.Helper()
+	p := filepath.Join(t.TempDir(), ".captain.yaml")
+	if err := os.WriteFile(p, []byte(""), 0o644); err != nil {
+		t.Fatalf("seed empty captain config: %v", err)
+	}
+	captainconfig.SetPathForTesting(p)
+	t.Cleanup(func() { captainconfig.SetPathForTesting("") })
+}
+
 func TestAIPromptOptions_ToRequest_Defaults(t *testing.T) {
+	isolateSavedAI(t)
 	opts := defaultPromptOptions(t)
 	req := opts.ToRequest()
 
@@ -29,6 +47,7 @@ func TestAIPromptOptions_ToRequest_Defaults(t *testing.T) {
 }
 
 func TestAIPromptOptions_ToRequest_TruthyInversion(t *testing.T) {
+	isolateSavedAI(t)
 	cases := []struct {
 		name   string
 		mutate func(*AIPromptOptions)
@@ -101,6 +120,7 @@ func TestAIPromptOptions_ToRequest_TruthyInversion(t *testing.T) {
 }
 
 func TestAIPromptOptions_ToRequest_PassesScalars(t *testing.T) {
+	isolateSavedAI(t)
 	opts := defaultPromptOptions(t)
 	opts.System = "be careful"
 	opts.AppendSystem = "also be brief"
@@ -146,19 +166,66 @@ func TestAIPromptOptions_ToRequest_PassesScalars(t *testing.T) {
 	}
 }
 
+// TestAIRuntimeOptions_ToRequest_OverlaysSaved verifies the path gavel (and any
+// other embedder) takes: AIRuntimeOptions with zero/default flag values should
+// still pick up NoMCP/NoHooks/MaxTokens/ReasoningEffort from ~/.captain.yaml.
+func TestAIRuntimeOptions_ToRequest_OverlaysSaved(t *testing.T) {
+	tmp := filepath.Join(t.TempDir(), ".captain.yaml")
+	saved := []byte("ai:\n  noMCP: true\n  noHooks: true\n  noSkills: true\n  noUser: true\n  noProject: true\n  noMemory: true\n  maxTokens: 16000\n  reasoningEffort: high\n")
+	if err := os.WriteFile(tmp, saved, 0o644); err != nil {
+		t.Fatalf("seed captain config: %v", err)
+	}
+	captainconfig.SetPathForTesting(tmp)
+	t.Cleanup(func() { captainconfig.SetPathForTesting("") })
+
+	opts := AIRuntimeOptions{
+		MaxTokens: 4096, // sentinel default — should be overridden by saved 16000
+		MCP:       true, // flag-default true; saved NoMCP=true must still win
+		Hooks:     true,
+		Skills:    true,
+		User:      true,
+		Project:   true,
+		Memory:    true,
+	}
+	req := opts.ToRequest("sys", "", "user")
+
+	if req.SystemPrompt != "sys" {
+		t.Errorf("SystemPrompt = %q, want sys", req.SystemPrompt)
+	}
+	if req.Prompt != "user" {
+		t.Errorf("Prompt = %q, want user", req.Prompt)
+	}
+	if req.MaxTokens != 16000 {
+		t.Errorf("MaxTokens = %d, want 16000 (saved overlay)", req.MaxTokens)
+	}
+	if req.ReasoningEffort != "high" {
+		t.Errorf("ReasoningEffort = %q, want high", req.ReasoningEffort)
+	}
+	for name, got := range map[string]bool{
+		"NoMCP": req.NoMCP, "NoHooks": req.NoHooks, "NoSkills": req.NoSkills,
+		"NoUser": req.NoUser, "NoProject": req.NoProject, "NoMemory": req.NoMemory,
+	} {
+		if !got {
+			t.Errorf("%s = false, want true (from saved config)", name)
+		}
+	}
+}
+
 func defaultPromptOptions(t *testing.T) AIPromptOptions {
 	t.Helper()
 	return AIPromptOptions{
-		Prompt:      "hello",
-		MaxTokens:   4096,
-		Temperature: "0",
-		Timeout:     "120s",
-		MCP:         true,
-		Hooks:       true,
-		Skills:      true,
-		User:        true,
-		Project:     true,
-		Memory:      true,
+		AIRuntimeOptions: AIRuntimeOptions{
+			MaxTokens:   4096,
+			Temperature: "0",
+			MCP:         true,
+			Hooks:       true,
+			Skills:      true,
+			User:        true,
+			Project:     true,
+			Memory:      true,
+		},
+		Timeout: "120s",
+		Prompt:  "hello",
 	}
 }
 

pkg/cli/history_render.go

@@ -189,10 +189,6 @@ func toLineEntry(t tools.Tool, compact bool, width, toolWidth int) lineEntry {
 	return e
 }
 
-func printLeft(e lineEntry, toolWidth int) {
-	printLeftTo(os.Stdout, e, toolWidth)
-}
-
 func printLeftTo(w io.Writer, e lineEntry, toolWidth int) {
 	timeStr := e.Time
 	if timeStr != "" {

pkg/container/generate.go

@@ -2,10 +2,8 @@ package container
 
 import (
 	"fmt"
-	"io"
 	"os"
 	"path/filepath"
-	"strings"
 	"text/template"
 )
 
@@ -224,72 +222,3 @@ func renderDockerfile(contextDir, tmplStr string, data templateData) (string, er
 	}
 	return dockerfilePath, nil
 }
-
-func stageCategoryFiles(contextDir string, cat Category, items []Component) ([]copyBlock, []string, bool, error) {
-	var blocks []copyBlock
-	var hookFiles []string
-	hasDotfiles := false
-
-	for _, item := range items {
-		relPath := filepath.Join(string(cat), item.Name)
-		destInContext := filepath.Join(contextDir, relPath)
-
-		if err := os.MkdirAll(filepath.Dir(destInContext), 0o755); err != nil {
-			return nil, nil, false, err
-		}
-
-		if item.IsDir {
-			if err := copyDir(item.SourcePath, destInContext); err != nil {
-				return nil, nil, false, fmt.Errorf("copying dir %s: %w", item.Name, err)
-			}
-		} else {
-			if err := copyFile(item.SourcePath, destInContext); err != nil {
-				return nil, nil, false, fmt.Errorf("copying file %s: %w", item.Name, err)
-			}
-		}
-
-		blocks = append(blocks, copyBlock{
-			Comment:     fmt.Sprintf("%s: %s", cat, item.Name),
-			Instruction: fmt.Sprintf("COPY %s %s", relPath, item.TargetPath),
-		})
-
-		if cat == CategoryHooks {
-			hookFiles = append(hookFiles, item.Name)
-		}
-		if strings.Contains(item.TargetPath, ".dotfiles") {
-			hasDotfiles = true
-		}
-	}
-	return blocks, hookFiles, hasDotfiles, nil
-}
-
-func copyFile(src, dst string) error {
-	in, err := os.Open(src)
-	if err != nil {
-		return err
-	}
-	defer in.Close() //nolint:errcheck
-
-	out, err := os.Create(dst)
-	if err != nil {
-		return err
-	}
-	defer out.Close() //nolint:errcheck
-
-	_, err = io.Copy(out, in)
-	return err
-}
-
-func copyDir(src, dst string) error {
-	return filepath.Walk(src, func(path string, info os.FileInfo, err error) error {
-		if err != nil {
-			return err
-		}
-		rel, _ := filepath.Rel(src, path)
-		target := filepath.Join(dst, rel)
-		if info.IsDir() {
-			return os.MkdirAll(target, 0o755)
-		}
-		return copyFile(path, target)
-	})
-}