From 5cd34966d0ed4c9a519f61a8a8208f51c3b88f5c Mon Sep 17 00:00:00 2001 From: David Mora Date: Thu, 19 Mar 2026 19:50:57 -0600 Subject: [PATCH 1/2] feat: remove EffortMax, add OptionSessionName and OptionRemoteControl (#46) Remove EffortMax globally (no longer supported in Claude CLI). Add root OptionSessionName mapped to --name (Claude) and --title (OpenCode) with precedence over backend-specific options. Add Claude-specific OptionRemoteControl for --remote-control flag with strict validation. Add session_name to MCP allowlist. Keep rate_limit_event unparsed (accessible via msg.Raw) per shared-vocabulary design rule. Fixes dmora/dors-orchestrator#46 Co-Authored-By: Claude Opus 4.6 (1M context) --- CLAUDE.md | 2 +- README.md | 2 +- agentrun_test.go | 4 +- cmd/agentrun-mcp/security.go | 1 + cmd/agentrun-mcp/security_test.go | 1 + engine/acp/engine.go | 2 +- engine/acp/engine_test.go | 16 +++ engine/cli/claude/args_test.go | 129 ++++++++++++++++++++++++- engine/cli/claude/claude.go | 23 ++++- engine/cli/claude/doc.go | 3 + engine/cli/codex/codex.go | 4 +- engine/cli/codex/codex_test.go | 1 - engine/cli/engine_test.go | 20 ++++ engine/cli/internal/optutil/optutil.go | 2 +- engine/cli/opencode/doc.go | 5 +- engine/cli/opencode/opencode.go | 20 +++- engine/cli/opencode/opencode_test.go | 74 +++++++++++++- session.go | 18 ++-- 18 files changed, 300 insertions(+), 27 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 4d4798a..a687905 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -117,7 +117,7 @@ agentrun (interfaces) - **Separate examples module**: `examples/go.mod` avoids pulling example deps into library consumers - **Platform build constraints**: Engine implementations using OS-specific features (signals, process groups) use `//go:build !windows` on implementation files. Interface and option files remain platform-agnostic. - **Signal safety**: All process Signal/Kill calls use `signalProcess()` helper which handles `os.ErrProcessDone` — prevents errors on already-exited processes -- **Cross-cutting session controls**: `Mode` (plan/act), `HITL` (on/off), and `Effort` (low/medium/high/max) types live in root with `Valid()` methods. Root options and backend-specific options (e.g., `claude.OptionPermissionMode`) are independent control surfaces — root wins when set, backend used when absent. Effort validation runs at engine level (`Start()`) for symmetric spawn/resume coverage. See `resolvePermissionFlag()` in Claude backend and `resolveVariant()` in OpenCode backend. +- **Cross-cutting session controls**: `Mode` (plan/act), `HITL` (on/off), and `Effort` (low/medium/high) types live in root with `Valid()` methods. `OptionSessionName` is a cross-cutting naming option (Claude `--name`, OpenCode `--title`; takes precedence over backend-specific `opencode.OptionTitle`). Root options and backend-specific options (e.g., `claude.OptionPermissionMode`) are independent control surfaces — root wins when set, backend used when absent. Effort validation runs at engine level (`Start()`) for symmetric spawn/resume coverage. See `resolvePermissionFlag()` in Claude backend and `resolveVariant()` in OpenCode backend. - **Session.Clone()**: Deep-copies Options and Env maps. Used by both CLI and ACP engines in `cloneSession()` — single implementation in root, no "keep in sync" duplication. - **ValidateModeHITL**: Shared in `engine/cli/internal/optutil` with prefix parameter for error messages. Claude and Codex backends delegate to it. - **Option parse helpers**: `ParsePositiveIntOption`, `ParseBoolOption`, `StringOption`, `ParseListOption` in `session_options.go` — backends use these instead of scattered `strconv` parsing. Both typed parsers validate null bytes and return `(value, ok, error)`. diff --git a/README.md b/README.md index a158b74..9d68dac 100644 --- a/README.md +++ b/README.md @@ -194,7 +194,7 @@ session := agentrun.Session{ Options: map[string]string{ agentrun.OptionSystemPrompt: "You are a Go expert.", agentrun.OptionMode: "act", // plan or act - agentrun.OptionEffort: "high", // low/medium/high/max + agentrun.OptionEffort: "high", // low/medium/high agentrun.OptionThinkingBudget: "10000", // enable extended thinking agentrun.OptionHITL: "off", // human-in-the-loop agentrun.OptionAddDirs: "/shared/lib\n/shared/proto", // newline-separated diff --git a/agentrun_test.go b/agentrun_test.go index 8ca292f..6aa9dc2 100644 --- a/agentrun_test.go +++ b/agentrun_test.go @@ -335,13 +335,13 @@ func TestSessionOptions_ResumeID_RoundTrip(t *testing.T) { } func TestEffortValid(t *testing.T) { - valid := []Effort{EffortLow, EffortMedium, EffortHigh, EffortMax} + valid := []Effort{EffortLow, EffortMedium, EffortHigh} for _, e := range valid { if !e.Valid() { t.Errorf("Effort(%q).Valid() = false, want true", e) } } - invalid := []Effort{"", "invalid", "LOW", "Medium", "xhigh"} + invalid := []Effort{"", "invalid", "LOW", "Medium", "xhigh", "max"} for _, e := range invalid { if e.Valid() { t.Errorf("Effort(%q).Valid() = true, want false", e) diff --git a/cmd/agentrun-mcp/security.go b/cmd/agentrun-mcp/security.go index 16bd206..bcf09cf 100644 --- a/cmd/agentrun-mcp/security.go +++ b/cmd/agentrun-mcp/security.go @@ -101,6 +101,7 @@ var allowedOptions = map[string]bool{ agentrun.OptionEffort: true, agentrun.OptionAgentID: true, agentrun.OptionAddDirs: true, + agentrun.OptionSessionName: true, } // validateOptions rejects any option key not in the allowlist. diff --git a/cmd/agentrun-mcp/security_test.go b/cmd/agentrun-mcp/security_test.go index 3268d29..ddb926b 100644 --- a/cmd/agentrun-mcp/security_test.go +++ b/cmd/agentrun-mcp/security_test.go @@ -185,6 +185,7 @@ func TestOptionsAllowlist_Allows(t *testing.T) { "max_turns": "5", "thinking_budget": "10000", "effort": "high", + "session_name": "my-session", } if err := validateOptions(opts); err != nil { t.Fatalf("expected nil, got %v", err) diff --git a/engine/acp/engine.go b/engine/acp/engine.go index b96b971..a8e4b23 100644 --- a/engine/acp/engine.go +++ b/engine/acp/engine.go @@ -73,7 +73,7 @@ func (e *Engine) Start(ctx context.Context, session agentrun.Session, opts ...ag // Validate cross-cutting options. if e := agentrun.Effort(session.Options[agentrun.OptionEffort]); e != "" && !e.Valid() { - return nil, fmt.Errorf("acp: unknown effort %q: valid: low, medium, high, max", e) + return nil, fmt.Errorf("acp: unknown effort %q: valid: low, medium, high", e) } // Validate CWD. diff --git a/engine/acp/engine_test.go b/engine/acp/engine_test.go index 9a55771..b3f3959 100644 --- a/engine/acp/engine_test.go +++ b/engine/acp/engine_test.go @@ -1037,6 +1037,22 @@ func TestEngine_Start_InvalidEffort(t *testing.T) { } } +func TestEngine_Start_EffortMaxRejected(t *testing.T) { + engine := newEngine(t) + ctx, cancel := context.WithTimeout(context.Background(), integrationTimeout) + defer cancel() + _, err := engine.Start(ctx, agentrun.Session{ + CWD: t.TempDir(), + Options: map[string]string{agentrun.OptionEffort: "max"}, + }) + if err == nil { + t.Fatal("expected error for effort 'max'") + } + if !strings.Contains(err.Error(), "unknown effort") { + t.Errorf("error = %v, want to contain 'unknown effort'", err) + } +} + func TestEngine_Validate(t *testing.T) { t.Run("valid binary", func(t *testing.T) { engine := newEngine(t) diff --git a/engine/cli/claude/args_test.go b/engine/cli/claude/args_test.go index 32a53a0..a3d2d25 100644 --- a/engine/cli/claude/args_test.go +++ b/engine/cli/claude/args_test.go @@ -1184,7 +1184,7 @@ func TestSpawnArgs_Effort(t *testing.T) { {"low", "low", []string{"--effort", "low"}, nil}, {"medium", "medium", []string{"--effort", "medium"}, nil}, {"high", "high", []string{"--effort", "high"}, nil}, - {"max_skipped", "max", nil, []string{"--effort"}}, + {"max_invalid", "max", nil, []string{"--effort"}}, {"empty", "", nil, []string{"--effort"}}, {"invalid", "xhigh", nil, []string{"--effort"}}, } @@ -1214,7 +1214,7 @@ func TestResumeArgs_Effort(t *testing.T) { {"low", "low", []string{"--effort", "low"}, nil, false}, {"medium", "medium", []string{"--effort", "medium"}, nil, false}, {"high", "high", []string{"--effort", "high"}, nil, false}, - {"max_skipped", "max", nil, []string{"--effort"}, false}, + {"max_invalid", "max", nil, nil, true}, {"invalid", "xhigh", nil, nil, true}, } @@ -1486,6 +1486,131 @@ func TestSpawnArgs_AllowedToolsPrecedence(t *testing.T) { } } +// --- SessionName option tests --- + +func TestSpawnArgs_SessionName(t *testing.T) { + tests := []struct { + name string + sessName string + contains []string + excludes []string + }{ + {"valid", "my-session", []string{"--name", "my-session"}, nil}, + {"null_byte_skipped", "bad\x00name", nil, []string{"--name"}}, + {"leading_dash_skipped", "-evil", nil, []string{"--name"}}, + {"empty_skipped", "", nil, []string{"--name"}}, + } + + b := New() + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + session := agentrun.Session{ + Prompt: testPrompt, + Options: map[string]string{agentrun.OptionSessionName: tt.sessName}, + } + _, args := b.SpawnArgs(session) + assertArgs(t, args, tt.contains, tt.excludes, testPrompt, false) + }) + } +} + +func TestStreamArgs_SessionName(t *testing.T) { + b := New() + session := agentrun.Session{ + Options: map[string]string{agentrun.OptionSessionName: "stream-session"}, + } + _, args := b.StreamArgs(session) + assertArgs(t, args, []string{"--name", "stream-session"}, nil, "", false) +} + +func TestResumeArgs_SessionName(t *testing.T) { + b := New() + session := agentrun.Session{ + Options: map[string]string{ + agentrun.OptionResumeID: testResumeID, + agentrun.OptionSessionName: "resume-session", + }, + } + _, args, err := b.ResumeArgs(session, testPrompt) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + assertArgs(t, args, []string{"--name", "resume-session"}, nil, testPrompt, false) +} + +// --- RemoteControl option tests --- + +func TestSpawnArgs_RemoteControl(t *testing.T) { + tests := []struct { + name string + value string + contains []string + excludes []string + }{ + {"true", "true", []string{"--remote-control"}, nil}, + {"one", "1", []string{"--remote-control"}, nil}, + {"on", "on", []string{"--remote-control"}, nil}, + {"yes", "yes", []string{"--remote-control"}, nil}, + {"false", "false", nil, []string{"--remote-control"}}, + {"zero", "0", nil, []string{"--remote-control"}}, + {"empty", "", nil, []string{"--remote-control"}}, + {"invalid", "maybe", nil, []string{"--remote-control"}}, + } + + b := New() + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + session := agentrun.Session{ + Prompt: testPrompt, + Options: map[string]string{OptionRemoteControl: tt.value}, + } + _, args := b.SpawnArgs(session) + assertArgs(t, args, tt.contains, tt.excludes, testPrompt, false) + }) + } +} + +func TestStreamArgs_RemoteControl(t *testing.T) { + b := New() + session := agentrun.Session{ + Options: map[string]string{OptionRemoteControl: "true"}, + } + _, args := b.StreamArgs(session) + assertArgs(t, args, []string{"--remote-control"}, nil, "", false) +} + +func TestResumeArgs_RemoteControl(t *testing.T) { + b := New() + session := agentrun.Session{ + Options: map[string]string{ + agentrun.OptionResumeID: testResumeID, + OptionRemoteControl: "true", + }, + } + _, args, err := b.ResumeArgs(session, testPrompt) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + assertArgs(t, args, []string{"--remote-control"}, nil, testPrompt, false) +} + +func TestResumeArgs_RemoteControl_Invalid(t *testing.T) { + b := New() + session := agentrun.Session{ + Options: map[string]string{ + agentrun.OptionResumeID: testResumeID, + OptionRemoteControl: "maybe", + }, + } + _, _, err := b.ResumeArgs(session, testPrompt) + if err == nil { + t.Fatal("expected error for invalid remote_control value") + } + if !strings.Contains(err.Error(), "invalid remote_control") { + t.Errorf("error = %v, want to contain 'invalid remote_control'", err) + } +} + // --- Helpers --- func assertArgs(t *testing.T, args, contains, excludes []string, last string, noNullByte bool) { diff --git a/engine/cli/claude/claude.go b/engine/cli/claude/claude.go index abc5e3d..c75e8fe 100644 --- a/engine/cli/claude/claude.go +++ b/engine/cli/claude/claude.go @@ -34,6 +34,12 @@ const ( // of which mode is active. The CLI decides enforcement semantics. // Backend-specific: only Claude CLI supports --allowedTools. OptionAllowedTools = "claude.allowed_tools" + + // OptionRemoteControl enables Claude Code's MCP remote control mode. + // When set to a truthy value ("true", "1", "on", "yes"), adds + // --remote-control to the CLI arguments. + // Backend-specific: only Claude CLI supports --remote-control. + OptionRemoteControl = "claude.remote_control" ) // validResumeID matches safe Claude session identifiers. @@ -250,14 +256,24 @@ func appendSessionArgs(args []string, session agentrun.Session) []string { args = appendPositiveInt(args, session.Options, agentrun.OptionMaxTurns, "--max-turns") args = appendPositiveInt(args, session.Options, agentrun.OptionThinkingBudget, "--max-thinking-tokens") - // Effort: Claude CLI supports low, medium, high (max has no equivalent). - if e := agentrun.Effort(session.Options[agentrun.OptionEffort]); e.Valid() && e != agentrun.EffortMax { + // Effort: Claude CLI supports low, medium, high. + if e := agentrun.Effort(session.Options[agentrun.OptionEffort]); e.Valid() { args = append(args, "--effort", string(e)) } // Additional directories. args = optutil.AppendAddDirs(args, session.Options, "--add-dir") + // Session name. + if name := session.Options[agentrun.OptionSessionName]; name != "" && !jsonutil.ContainsNull(name) && !strings.HasPrefix(name, "-") { + args = append(args, "--name", name) + } + + // Remote control. + if rc, ok, _ := agentrun.ParseBoolOption(session.Options, OptionRemoteControl); ok && rc { + args = append(args, "--remote-control") + } + // Allowed tools — orthogonal to permission mode (always appended when set). for _, tool := range agentrun.ParseListOption(session.Options, OptionAllowedTools) { if !strings.HasPrefix(tool, "-") { @@ -330,6 +346,9 @@ func validateSessionOptions(opts map[string]string) error { if err := validatePositiveIntOption(opts, agentrun.OptionThinkingBudget, "thinking budget"); err != nil { return err } + if _, _, err := agentrun.ParseBoolOption(opts, OptionRemoteControl); err != nil { + return fmt.Errorf("claude: invalid remote_control: %w", err) + } return optutil.ValidateEffort("claude", opts) } diff --git a/engine/cli/claude/doc.go b/engine/cli/claude/doc.go index 6cdc8a9..2173e0e 100644 --- a/engine/cli/claude/doc.go +++ b/engine/cli/claude/doc.go @@ -47,6 +47,7 @@ // // - [agentrun.OptionMode] — sets session intent ("plan" or "act") // - [agentrun.OptionHITL] — controls human-in-the-loop ("on" or "off") +// - [agentrun.OptionSessionName] — sets --name (human-readable session name) // // When OptionMode or OptionHITL are set, they map to --permission-mode: // @@ -67,4 +68,6 @@ // Claude-specific options: // // - [OptionPermissionMode] — sets --permission-mode (use [PermissionMode] values) +// - [OptionAllowedTools] — sets --allowedTools (newline-separated tool names) +// - [OptionRemoteControl] — sets --remote-control (truthy boolean enables MCP remote control) package claude diff --git a/engine/cli/codex/codex.go b/engine/cli/codex/codex.go index 8ffc690..6fcf212 100644 --- a/engine/cli/codex/codex.go +++ b/engine/cli/codex/codex.go @@ -211,12 +211,10 @@ func buildResumeCommand(threadID string, session agentrun.Session) []string { } // codexEffort maps root Effort values to Codex model_reasoning_effort values. -// max → "xhigh" is a Codex-specific mapping. var codexEffort = map[agentrun.Effort]string{ agentrun.EffortLow: "low", agentrun.EffortMedium: "medium", agentrun.EffortHigh: "high", - agentrun.EffortMax: "xhigh", } // appendCommonArgs appends flags available on both exec and exec resume. @@ -233,7 +231,7 @@ func appendCommonArgs(args []string, session agentrun.Session) []string { args = append(args, "--skip-git-repo-check") } - // Effort: Codex supports low, medium, high, max (max → "xhigh"). + // Effort: Codex supports low, medium, high. if e := agentrun.Effort(session.Options[agentrun.OptionEffort]); e != "" { if v, ok := codexEffort[e]; ok { args = append(args, "-c", "model_reasoning_effort="+v) diff --git a/engine/cli/codex/codex_test.go b/engine/cli/codex/codex_test.go index 78925da..5779719 100644 --- a/engine/cli/codex/codex_test.go +++ b/engine/cli/codex/codex_test.go @@ -899,7 +899,6 @@ func TestSpawnArgs_Effort(t *testing.T) { {"low", "low", "model_reasoning_effort=low"}, {"medium", "medium", "model_reasoning_effort=medium"}, {"high", "high", "model_reasoning_effort=high"}, - {"max_to_xhigh", "max", "model_reasoning_effort=xhigh"}, } b := New() diff --git a/engine/cli/engine_test.go b/engine/cli/engine_test.go index 43f7850..a23e36b 100644 --- a/engine/cli/engine_test.go +++ b/engine/cli/engine_test.go @@ -1707,6 +1707,26 @@ func TestStart_InvalidEffort(t *testing.T) { } } +func TestStart_EffortMaxRejected(t *testing.T) { + b := withResumer(testBackend{ + spawnFn: func(_ agentrun.Session) (string, []string) { + return binEcho, []string{"x"} + }, + parseFn: textParser, + }) + eng := cli.NewEngine(b) + _, err := eng.Start(testCtx(t), agentrun.Session{ + CWD: tempDir(t), + Options: map[string]string{agentrun.OptionEffort: "max"}, + }) + if err == nil { + t.Fatal("expected error for effort 'max'") + } + if !strings.Contains(err.Error(), "unknown effort") { + t.Errorf("error = %v, want to contain 'unknown effort'", err) + } +} + // --------------------------------------------------------------------------- // Concurrency tests // --------------------------------------------------------------------------- diff --git a/engine/cli/internal/optutil/optutil.go b/engine/cli/internal/optutil/optutil.go index 22b20c3..0ccb2e2 100644 --- a/engine/cli/internal/optutil/optutil.go +++ b/engine/cli/internal/optutil/optutil.go @@ -32,7 +32,7 @@ func ValidateModeHITL(prefix string, opts map[string]string) error { // The prefix is used in error messages (e.g., "claude", "codex"). func ValidateEffort(prefix string, opts map[string]string) error { if e := agentrun.Effort(opts[agentrun.OptionEffort]); e != "" && !e.Valid() { - return fmt.Errorf("%s: unknown effort %q: valid: low, medium, high, max", prefix, e) + return fmt.Errorf("%s: unknown effort %q: valid: low, medium, high", prefix, e) } return nil } diff --git a/engine/cli/opencode/doc.go b/engine/cli/opencode/doc.go index 269acf6..a192708 100644 --- a/engine/cli/opencode/doc.go +++ b/engine/cli/opencode/doc.go @@ -24,6 +24,9 @@ // - OptionAgentID → --agent // - OptionThinkingBudget → --thinking (boolean: any non-empty value) // +// Cross-cutting (root package — session controls): +// - OptionSessionName → --title (takes precedence over OptionTitle) +// // Cross-cutting (root package — session resume): // - OptionResumeID → --session (auto-captured or explicit cold resume) // Consumers capture the session ID from MessageInit.ResumeID. @@ -31,7 +34,7 @@ // Backend-specific (namespaced with "opencode." prefix): // - OptionVariant → --variant (VariantHigh, VariantMax, VariantMinimal, VariantLow) // - OptionFork → --fork (fork session on resume) -// - OptionTitle → --title (session title, max 512 bytes) +// - OptionTitle → --title (session title, max 512 bytes; overridden by root OptionSessionName) // // # Event types // diff --git a/engine/cli/opencode/opencode.go b/engine/cli/opencode/opencode.go index b89197d..4437d4e 100644 --- a/engine/cli/opencode/opencode.go +++ b/engine/cli/opencode/opencode.go @@ -112,8 +112,8 @@ func (b *Backend) SpawnArgs(session agentrun.Session) (string, []string) { args = append(args, "--agent", id) } - if t := session.Options[OptionTitle]; t != "" && !jsonutil.ContainsNull(t) && len(t) <= maxTitleLen { - args = append(args, "--title", t) + if title := resolveTitle(session.Options); title != "" { + args = append(args, "--title", title) } // Prompt is always the last positional argument. @@ -181,7 +181,6 @@ func baseArgs() []string { var effortToVariant = map[agentrun.Effort]Variant{ agentrun.EffortLow: VariantLow, agentrun.EffortHigh: VariantHigh, - agentrun.EffortMax: VariantMax, } // resolveVariant returns the --variant value from root OptionEffort @@ -202,6 +201,21 @@ func resolveVariant(opts map[string]string) string { return "" } +// resolveTitle returns the --title value from root OptionSessionName +// (precedence) or backend-specific OptionTitle. +// Returns empty string when no title should be emitted. +func resolveTitle(opts map[string]string) string { + // Root OptionSessionName takes precedence. + if v := opts[agentrun.OptionSessionName]; v != "" && !jsonutil.ContainsNull(v) && len(v) <= maxTitleLen { + return v + } + // Fall back to backend-specific OptionTitle. + if v := opts[OptionTitle]; v != "" && !jsonutil.ContainsNull(v) && len(v) <= maxTitleLen { + return v + } + return "" +} + // appendCommonArgs appends model, thinking, and variant flags. // Mode, HITL, SystemPrompt, and MaxTurns are silently ignored // (OpenCode has no CLI flags for these). diff --git a/engine/cli/opencode/opencode_test.go b/engine/cli/opencode/opencode_test.go index 7122f2d..6671ffd 100644 --- a/engine/cli/opencode/opencode_test.go +++ b/engine/cli/opencode/opencode_test.go @@ -500,7 +500,6 @@ func TestSpawnArgs_Effort(t *testing.T) { }{ {"low", "low", "low"}, {"high", "high", "high"}, - {"max", "max", "max"}, {"medium_no_equivalent", "medium", ""}, {"invalid", "xhigh", ""}, {"empty", "", ""}, @@ -563,7 +562,6 @@ func TestResumeArgs_Effort(t *testing.T) { }{ {"low", "low", "low"}, {"high", "high", "high"}, - {"max", "max", "max"}, {"medium_no_variant", "medium", ""}, {"empty", "", ""}, } @@ -622,6 +620,78 @@ func TestSpawnArgs_Variant_FallbackWhenNoEffort(t *testing.T) { assertContains(t, args, "high") } +// --- SessionName option tests --- + +func TestSpawnArgs_SessionName(t *testing.T) { + b := New() + session := agentrun.Session{ + Prompt: "hi", + Options: map[string]string{agentrun.OptionSessionName: "my-session"}, + } + _, args := b.SpawnArgs(session) + assertContains(t, args, "--title") + assertContains(t, args, "my-session") +} + +func TestSpawnArgs_SessionName_PrecedenceOverTitle(t *testing.T) { + b := New() + session := agentrun.Session{ + Prompt: "hi", + Options: map[string]string{ + agentrun.OptionSessionName: "root-name", + OptionTitle: "backend-title", + }, + } + _, args := b.SpawnArgs(session) + assertContains(t, args, "--title") + assertContains(t, args, "root-name") + assertNotContains(t, args, "backend-title") +} + +func TestSpawnArgs_SessionName_FallsThrough(t *testing.T) { + b := New() + session := agentrun.Session{ + Prompt: "hi", + Options: map[string]string{ + agentrun.OptionSessionName: "", + OptionTitle: "fallback-title", + }, + } + _, args := b.SpawnArgs(session) + assertContains(t, args, "--title") + assertContains(t, args, "fallback-title") +} + +func TestSpawnArgs_SessionName_NullByte(t *testing.T) { + b := New() + session := agentrun.Session{ + Prompt: "hi", + Options: map[string]string{ + agentrun.OptionSessionName: "bad\x00name", + OptionTitle: "fallback-title", + }, + } + _, args := b.SpawnArgs(session) + assertContains(t, args, "--title") + assertContains(t, args, "fallback-title") +} + +func TestSpawnArgs_SessionName_OverLimit(t *testing.T) { + b := New() + longName := strings.Repeat("x", maxTitleLen+1) + session := agentrun.Session{ + Prompt: "hi", + Options: map[string]string{ + agentrun.OptionSessionName: longName, + OptionTitle: "fallback-title", + }, + } + _, args := b.SpawnArgs(session) + assertContains(t, args, "--title") + assertContains(t, args, "fallback-title") + assertNotContains(t, args, longName) +} + func assertContains(t *testing.T, args []string, want string) { t.Helper() for _, a := range args { diff --git a/session.go b/session.go index 25bc803..5fda960 100644 --- a/session.go +++ b/session.go @@ -60,11 +60,11 @@ const ( OptionAgentID = "agent_id" // OptionEffort controls reasoning depth/quality tradeoff. - // Value should be an Effort constant (low, medium, high, max). + // Value should be an Effort constant (low, medium, high). // Backend support varies — unsupported values are silently skipped: // - Claude CLI: low, medium, high (maps to --effort) - // - Codex CLI: low, medium, high, max (maps to -c model_reasoning_effort; max → "xhigh") - // - OpenCode: low, high, max (maps to --variant; medium has no equivalent) + // - Codex CLI: low, medium, high (maps to -c model_reasoning_effort) + // - OpenCode: low, high (maps to --variant; medium has no equivalent) // // When set and mappable to the backend's native values, OptionEffort // takes precedence over backend-specific effort options (e.g., @@ -79,6 +79,13 @@ const ( // Backend support: Claude (--add-dir), Codex (--add-dir). // Backends without directory scoping silently ignore this option. OptionAddDirs = "add_dirs" + + // OptionSessionName sets a human-readable name for the session. + // Backend support: Claude CLI (--name), OpenCode (--title). + // Backends without session naming silently ignore this option. + // When set, takes precedence over backend-specific naming options + // (e.g., opencode.OptionTitle). + OptionSessionName = "session_name" ) // Mode represents the operating mode for a session. @@ -125,14 +132,11 @@ const ( // EffortHigh requests thorough reasoning for complex tasks. EffortHigh Effort = "high" - - // EffortMax requests maximum reasoning depth. - EffortMax Effort = "max" ) // Valid reports whether e is a recognized Effort value. func (e Effort) Valid() bool { - return e == EffortLow || e == EffortMedium || e == EffortHigh || e == EffortMax + return e == EffortLow || e == EffortMedium || e == EffortHigh } // Session is the minimal session state passed to engines. From 46f2a385ad6f2b38706e1dba4a1a30f433d8aeec Mon Sep 17 00:00:00 2001 From: David Mora Date: Thu, 19 Mar 2026 20:27:29 -0600 Subject: [PATCH 2/2] fix: add leading-dash guard to resolveTitle in opencode backend Addresses review feedback from gemini-code-assist on PR #47: resolveTitle() was missing !strings.HasPrefix(v, "-") checks on both OptionSessionName and OptionTitle, allowing potential CLI argument injection. The Claude backend already had this guard for --name. Adds two tests: leading-dash SessionName (falls through to OptionTitle) and leading-dash OptionTitle (skipped entirely). Co-Authored-By: Claude Opus 4.6 (1M context) --- engine/cli/opencode/opencode.go | 4 ++-- engine/cli/opencode/opencode_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/engine/cli/opencode/opencode.go b/engine/cli/opencode/opencode.go index 4437d4e..ef61418 100644 --- a/engine/cli/opencode/opencode.go +++ b/engine/cli/opencode/opencode.go @@ -206,11 +206,11 @@ func resolveVariant(opts map[string]string) string { // Returns empty string when no title should be emitted. func resolveTitle(opts map[string]string) string { // Root OptionSessionName takes precedence. - if v := opts[agentrun.OptionSessionName]; v != "" && !jsonutil.ContainsNull(v) && len(v) <= maxTitleLen { + if v := opts[agentrun.OptionSessionName]; v != "" && !jsonutil.ContainsNull(v) && len(v) <= maxTitleLen && !strings.HasPrefix(v, "-") { return v } // Fall back to backend-specific OptionTitle. - if v := opts[OptionTitle]; v != "" && !jsonutil.ContainsNull(v) && len(v) <= maxTitleLen { + if v := opts[OptionTitle]; v != "" && !jsonutil.ContainsNull(v) && len(v) <= maxTitleLen && !strings.HasPrefix(v, "-") { return v } return "" diff --git a/engine/cli/opencode/opencode_test.go b/engine/cli/opencode/opencode_test.go index 6671ffd..d92a77b 100644 --- a/engine/cli/opencode/opencode_test.go +++ b/engine/cli/opencode/opencode_test.go @@ -692,6 +692,32 @@ func TestSpawnArgs_SessionName_OverLimit(t *testing.T) { assertNotContains(t, args, longName) } +func TestSpawnArgs_SessionName_LeadingDash(t *testing.T) { + b := New() + session := agentrun.Session{ + Prompt: "hi", + Options: map[string]string{ + agentrun.OptionSessionName: "-evil", + OptionTitle: "fallback-title", + }, + } + _, args := b.SpawnArgs(session) + assertContains(t, args, "--title") + assertContains(t, args, "fallback-title") + assertNotContains(t, args, "-evil") +} + +func TestSpawnArgs_Title_LeadingDash(t *testing.T) { + b := New() + session := agentrun.Session{ + Prompt: "hi", + Options: map[string]string{OptionTitle: "-malicious"}, + } + _, args := b.SpawnArgs(session) + assertNotContains(t, args, "--title") + assertNotContains(t, args, "-malicious") +} + func assertContains(t *testing.T, args []string, want string) { t.Helper() for _, a := range args {