fix(config): preserve all RuntimeConfig fields in fillRuntimeDefaults (#971)
* fix(config): preserve all RuntimeConfig fields in fillRuntimeDefaults fillRuntimeDefaults was only copying Command, Args, InitialPrompt, and Env when creating a copy of RuntimeConfig. This caused fields like PromptMode, Provider, Session, Hooks, Tmux, and Instructions to be silently dropped. This broke custom agent configurations, particularly prompt_mode: "none" which is needed for agents like opencode that don't accept a startup beacon. Changes: - Copy all RuntimeConfig fields in fillRuntimeDefaults - Add comprehensive tests for fillRuntimeDefaults - Add integration tests for custom agent configs with prompt_mode: none - Add tests mirroring manual verification: claude-opus, amp, codex, gemini * fix(config): deep copy slices and nested structs in fillRuntimeDefaults The original fix preserved all fields but used shallow copies for slices and nested structs. This could cause mutation of the original config. Changes: - Deep copy Args slice (was sharing backing array) - Deep copy Session, Hooks, Tmux, Instructions structs (were pointer copies) - Deep copy Tmux.ProcessNames slice - Add comprehensive mutation isolation tests for all fields - Fix TestMultipleAgentTypes to test actual built-in presets - Add TestCustomClaudeVariants to clarify that claude-opus/sonnet/haiku are NOT built-in and must be defined as custom agents Built-in presets: claude, gemini, codex, cursor, auggie, amp, opencode Custom variants like claude-opus need explicit definition in Agents map. * docs(test): add manual test settings to TestRoleAgentConfigWithCustomAgent Document the settings/config.json used for manual verification: - default_agent: claude-opus - Custom agents: amp-yolo, opencode-mayor (with prompt_mode: none) - role_agents mapping for all 6 roles - Manual test procedure for all 7 built-in agents * fix(test): address CodeRabbit review feedback - Fix isClaudeCommand to handle Windows paths and .exe extension - Use isClaudeCommand helper instead of brittle equality check - Add skipIfAgentBinaryMissing to tests that depend on external binaries (TestMultipleAgentTypes, TestCustomAgentWithAmp)
This commit is contained in:
@@ -1075,29 +1075,84 @@ func lookupAgentConfig(name string, townSettings *TownSettings, rigSettings *Rig
|
||||
}
|
||||
|
||||
// fillRuntimeDefaults fills in default values for empty RuntimeConfig fields.
|
||||
// It creates a deep copy to prevent mutation of the original config.
|
||||
//
|
||||
// Default behavior:
|
||||
// - Command defaults to "claude" if empty
|
||||
// - Args defaults to ["--dangerously-skip-permissions"] if nil
|
||||
// - Empty Args slice ([]string{}) means "no args" and is preserved as-is
|
||||
//
|
||||
// All fields are deep-copied: modifying the returned config will not affect
|
||||
// the input config, including nested structs and slices.
|
||||
func fillRuntimeDefaults(rc *RuntimeConfig) *RuntimeConfig {
|
||||
if rc == nil {
|
||||
return DefaultRuntimeConfig()
|
||||
}
|
||||
// Create a copy to avoid modifying the original
|
||||
|
||||
// Create result with scalar fields (strings are immutable in Go)
|
||||
result := &RuntimeConfig{
|
||||
Provider: rc.Provider,
|
||||
Command: rc.Command,
|
||||
Args: rc.Args,
|
||||
InitialPrompt: rc.InitialPrompt,
|
||||
PromptMode: rc.PromptMode,
|
||||
}
|
||||
// Copy Env map to avoid mutation and preserve agent-specific env vars
|
||||
|
||||
// Deep copy Args slice to avoid sharing backing array
|
||||
if rc.Args != nil {
|
||||
result.Args = make([]string, len(rc.Args))
|
||||
copy(result.Args, rc.Args)
|
||||
}
|
||||
|
||||
// Deep copy Env map
|
||||
if len(rc.Env) > 0 {
|
||||
result.Env = make(map[string]string, len(rc.Env))
|
||||
for k, v := range rc.Env {
|
||||
result.Env[k] = v
|
||||
}
|
||||
}
|
||||
|
||||
// Deep copy nested structs (nil checks prevent panic on access)
|
||||
if rc.Session != nil {
|
||||
result.Session = &RuntimeSessionConfig{
|
||||
SessionIDEnv: rc.Session.SessionIDEnv,
|
||||
ConfigDirEnv: rc.Session.ConfigDirEnv,
|
||||
}
|
||||
}
|
||||
|
||||
if rc.Hooks != nil {
|
||||
result.Hooks = &RuntimeHooksConfig{
|
||||
Provider: rc.Hooks.Provider,
|
||||
Dir: rc.Hooks.Dir,
|
||||
SettingsFile: rc.Hooks.SettingsFile,
|
||||
}
|
||||
}
|
||||
|
||||
if rc.Tmux != nil {
|
||||
result.Tmux = &RuntimeTmuxConfig{
|
||||
ReadyPromptPrefix: rc.Tmux.ReadyPromptPrefix,
|
||||
ReadyDelayMs: rc.Tmux.ReadyDelayMs,
|
||||
}
|
||||
// Deep copy ProcessNames slice
|
||||
if rc.Tmux.ProcessNames != nil {
|
||||
result.Tmux.ProcessNames = make([]string, len(rc.Tmux.ProcessNames))
|
||||
copy(result.Tmux.ProcessNames, rc.Tmux.ProcessNames)
|
||||
}
|
||||
}
|
||||
|
||||
if rc.Instructions != nil {
|
||||
result.Instructions = &RuntimeInstructionsConfig{
|
||||
File: rc.Instructions.File,
|
||||
}
|
||||
}
|
||||
|
||||
// Apply defaults for required fields
|
||||
if result.Command == "" {
|
||||
result.Command = "claude"
|
||||
}
|
||||
if result.Args == nil {
|
||||
result.Args = []string{"--dangerously-skip-permissions"}
|
||||
}
|
||||
|
||||
return result
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user