diff --git a/agent-schema.json b/agent-schema.json index 066ff369a..d73d6591e 100644 --- a/agent-schema.json +++ b/agent-schema.json @@ -659,6 +659,15 @@ "type": "boolean", "description": "When true, makes every one of this agent's toolsets read-only: only tools whose annotations carry a read-only hint are listed and callable, and all other (mutating) tools are filtered out. Equivalent to setting 'readonly: true' on each toolset. A sub-agent or handoff agent is made read-only by setting this flag on that agent's own definition." }, + "safety": { + "type": "string", + "enum": [ + "strict", + "balanced", + "autonomous" + ], + "description": "Safety mode new sessions started on this agent default to when the user has not chosen one (no --safety/--yolo flag, no alias option, no user-config setting). Takes precedence over the config-wide runtime.safety. A default only: it never overrides a user choice and never replaces the mode stored on a resumed session. Trust warning: 'autonomous' auto-approves every tool call — review configs from URLs or OCI registries before running them." + }, "redact_secrets": { "type": "boolean", "default": true, @@ -1840,6 +1849,15 @@ "items": { "type": "string" } + }, + "safety": { + "type": "string", + "enum": [ + "strict", + "balanced", + "autonomous" + ], + "description": "Safety mode new sessions default to when the user has not chosen one (no --safety/--yolo flag, no alias option, no user-config setting). A per-agent 'safety' takes precedence over this config-wide default. A default only: it never overrides a user choice and never replaces the mode stored on a resumed session. Trust warning: 'autonomous' auto-approves every tool call — review configs from URLs or OCI registries before running them." } }, "additionalProperties": false diff --git a/cmd/root/alias.go b/cmd/root/alias.go index 16495d778..5246a1850 100644 --- a/cmd/root/alias.go +++ b/cmd/root/alias.go @@ -13,6 +13,7 @@ import ( "github.com/docker/docker-agent/pkg/cli" "github.com/docker/docker-agent/pkg/config" + latestcfg "github.com/docker/docker-agent/pkg/config/latest" pathx "github.com/docker/docker-agent/pkg/path" "github.com/docker/docker-agent/pkg/telemetry" "github.com/docker/docker-agent/pkg/userconfig" @@ -49,6 +50,7 @@ func newAliasCmd() *cobra.Command { type aliasAddFlags struct { yolo bool + safety string model string hideToolResults bool sandbox bool @@ -66,6 +68,7 @@ You can optionally specify runtime options that will be applied whenever the alias is used: --yolo Automatically approve all tool calls without prompting + --safety Default safety mode: strict, balanced, or autonomous --model Override the agent's model (format: [agent=]provider/model) --hide-tool-results Hide tool call results in the TUI --sandbox Always run the agent inside a Docker sandbox`, @@ -75,6 +78,9 @@ the alias is used: # Create an alias that always runs in yolo mode docker-agent alias add yolo-coder myorg/coder --yolo + # Create an alias that defaults to the balanced safety mode + docker-agent alias add careful-coder myorg/coder --safety balanced + # Create an alias with a specific model docker-agent alias add fast-coder myorg/coder --model openai/gpt-4o-mini @@ -93,6 +99,7 @@ the alias is used: } cmd.Flags().BoolVar(&flags.yolo, "yolo", false, "Automatically approve all tool calls without prompting") + cmd.Flags().StringVar(&flags.safety, "safety", "", "Default safety mode when running the alias: strict, balanced, or autonomous (wins over --yolo)") cmd.Flags().StringVar(&flags.model, "model", "", "Override agent model (format: [agent=]provider/model)") cmd.Flags().BoolVar(&flags.hideToolResults, "hide-tool-results", false, "Hide tool call results in the TUI") cmd.Flags().BoolVar(&flags.sandbox, "sandbox", false, "Always run the agent inside a Docker sandbox") @@ -138,6 +145,12 @@ func runAliasAddCommand(cmd *cobra.Command, args []string, flags *aliasAddFlags) name := args[0] agentPath := args[1] + // Fail fast on a typo: only the three canonical modes may be stored. + safety := latestcfg.SafetyMode(flags.safety) + if err := safety.Validate(); err != nil { + return fmt.Errorf("invalid --safety value: %w", err) + } + absAgentPath, err := pathx.ExpandHomeDir(agentPath) if err != nil { return err @@ -155,6 +168,7 @@ func runAliasAddCommand(cmd *cobra.Command, args []string, flags *aliasAddFlags) alias := &userconfig.Alias{ Path: absAgentPath, Yolo: flags.yolo, + Safety: safety, Model: flags.model, HideToolResults: flags.hideToolResults, Sandbox: flags.sandbox, @@ -173,6 +187,9 @@ func runAliasAddCommand(cmd *cobra.Command, args []string, flags *aliasAddFlags) if flags.yolo { out.Printf(" Yolo: enabled\n") } + if safety != "" { + out.Printf(" Safety: %s\n", string(safety)) + } if flags.model != "" { out.Printf(" Model: %s\n", flags.model) } @@ -254,6 +271,9 @@ func runAliasListCommand(cmd *cobra.Command, args []string, asJSON bool) (comman if alias.Yolo { options = append(options, "yolo") } + if alias.Safety != "" { + options = append(options, "safety="+string(alias.Safety)) + } if alias.Model != "" { options = append(options, "model="+alias.Model) } diff --git a/cmd/root/alias_test.go b/cmd/root/alias_test.go index 6c123b786..1a49c3df7 100644 --- a/cmd/root/alias_test.go +++ b/cmd/root/alias_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + latestcfg "github.com/docker/docker-agent/pkg/config/latest" "github.com/docker/docker-agent/pkg/paths" "github.com/docker/docker-agent/pkg/userconfig" ) @@ -65,3 +66,42 @@ func TestRunAliasListCommand_JSONFormatEmpty(t *testing.T) { assert.Empty(t, entries) assert.Equal(t, "[]", string(bytes.TrimSpace(buf.Bytes()))) } + +func TestRunAliasAddCommand_Safety(t *testing.T) { + // Not parallel: SetConfigDir mutates process-global state. + dir := t.TempDir() + paths.SetConfigDir(dir) + t.Cleanup(func() { paths.SetConfigDir("") }) + + var buf bytes.Buffer + cmd := &cobra.Command{} + cmd.SetOut(&buf) + err := runAliasAddCommand(cmd, []string{"careful", "myorg/coder"}, &aliasAddFlags{safety: "balanced"}) + require.NoError(t, err) + assert.Contains(t, buf.String(), "Safety: balanced") + + cfg, err := userconfig.Load() + require.NoError(t, err) + alias, ok := cfg.GetAlias("careful") + require.True(t, ok) + assert.Equal(t, latestcfg.SafetyModeBalanced, alias.Safety) +} + +func TestRunAliasAddCommand_InvalidSafety(t *testing.T) { + // Not parallel: SetConfigDir mutates process-global state. + dir := t.TempDir() + paths.SetConfigDir(dir) + t.Cleanup(func() { paths.SetConfigDir("") }) + + var buf bytes.Buffer + cmd := &cobra.Command{} + cmd.SetOut(&buf) + err := runAliasAddCommand(cmd, []string{"careful", "myorg/coder"}, &aliasAddFlags{safety: "yolo"}) + require.ErrorContains(t, err, "invalid --safety value") + require.ErrorContains(t, err, "strict, balanced, autonomous") + + cfg, err := userconfig.Load() + require.NoError(t, err) + _, ok := cfg.GetAlias("careful") + assert.False(t, ok, "an invalid alias must not be stored") +} diff --git a/cmd/root/payload.go b/cmd/root/payload.go index 4f5fc6dad..0d760af58 100644 --- a/cmd/root/payload.go +++ b/cmd/root/payload.go @@ -3,7 +3,6 @@ package root import ( "github.com/docker/docker-agent/pkg/config" "github.com/docker/docker-agent/pkg/runtime" - "github.com/docker/docker-agent/pkg/session" ) // loadTeamRequest builds a runtime.LoadTeamRequest from the current flags. @@ -17,12 +16,17 @@ func (f *runExecFlags) loadTeamRequest(agentSource config.Source) runtime.LoadTe } // createSessionRequest builds a runtime.CreateSessionRequest from the -// current flags and the supplied working directory. +// current flags and the supplied working directory. SafetyPolicy carries +// the user-owned mode only (explicit CLI flags, then alias/settings +// defaults); author-declared YAML defaults are resolved later, when a +// fresh session is actually built, so they can never masquerade as a +// user choice. func (f *runExecFlags) createSessionRequest(workingDir string) runtime.CreateSessionRequest { return runtime.CreateSessionRequest{ AgentName: f.agentName, ToolsApproved: f.autoApprove, - SafetyPolicy: session.SafetyPolicy(f.safety), + SafetyPolicy: f.userSafetyPolicy(), + SafetyExplicit: f.explicitCLISafety() != "", HideToolResults: f.hideToolResults, SessionDB: sessionDBPath(f.sessionDB), ResumeSessionID: f.sessionID, diff --git a/cmd/root/run.go b/cmd/root/run.go index 87ccb5b6b..06bdda7ce 100644 --- a/cmd/root/run.go +++ b/cmd/root/run.go @@ -1,6 +1,7 @@ package root import ( + "cmp" "context" "errors" "fmt" @@ -53,9 +54,20 @@ const worktreeAutoName = "auto" var projectDefaultAgentFiles = []string{"docker-agent.yaml", "docker-agent.yml", "docker-agent.hcl"} type runExecFlags struct { - agentName string - autoApprove bool - safety string + agentName string + autoApprove bool + safety string + // safetyChanged / yoloChanged record whether --safety / --yolo were + // explicitly passed on the command line. Explicit flags are the only + // safety sources allowed to override a resumed session's stored mode; + // alias options and user settings are defaults that never do. + safetyChanged bool + yoloChanged bool + // defaultSafety is the user-owned safety default resolved from alias + // options and user settings (alias wins; within each scope safety wins + // over the legacy yolo/YOLO flag). Never populated from CLI flags or + // author YAML. + defaultSafety session.SafetyPolicy attachmentPath string remoteAddress string modelOverrides []string @@ -254,6 +266,8 @@ func (f *runExecFlags) runRunCommand(cmd *cobra.Command, args []string) (command default: return fmt.Errorf("invalid --safety value %q (valid: strict, balanced, autonomous)", f.safety) } + f.safetyChanged = cmd.Flags().Changed("safety") + f.yoloChanged = cmd.Flags().Changed("yolo") useTUI := !f.exec && (f.forceTUI || isatty.IsTerminal(os.Stdout.Fd())) f.leanChanged = cmd.Flags().Changed("lean") @@ -357,29 +371,25 @@ func (f *runExecFlags) runOrExec(ctx context.Context, out *cli.Printer, args []s agentFileName := f.resolveRunAgentFileName(args) + // Load the user config once and fail loudly: falling back to defaults + // here would silently drop the user's settings (safety, permissions, + // hooks) and alias options, and an invalid settings/alias safety value + // must surface as a clear validation error, not a quiet reset. + userCfg, err := userconfig.Load() + if err != nil { + return fmt.Errorf("loading user config: %w", err) + } + // Apply global user settings first (lowest priority) // User settings only apply if the flag wasn't explicitly set by the user - userSettings := userconfig.Get() + userSettings := userCfg.GetSettings() f.applyUserSettings(ctx, userSettings) f.runConfig.GlobalHooks = config.MergeHooks(userSettings.GlobalHooks(), config.LoadHookDropIns()) // Apply alias options if this is an alias reference // Alias options only apply if the flag wasn't explicitly set by the user - if alias := config.ResolveAlias(agentFileName); alias != nil { - slog.DebugContext(ctx, "Applying alias options", "yolo", alias.Yolo, "model", alias.Model, "hide_tool_results", alias.HideToolResults, "sandbox", alias.Sandbox) - if alias.Yolo && !f.autoApprove { - f.autoApprove = true - } - if alias.Model != "" && len(f.modelOverrides) == 0 { - f.modelOverrides = append(f.modelOverrides, alias.Model) - } - if alias.HideToolResults && !f.hideToolResults { - f.hideToolResults = true - } - // alias.Sandbox is consumed earlier in runRunCommand before - // dispatch; reaching runOrExec means the sandbox decision - // resolved to false (or the user opted out via --sandbox=false), - // so flipping it here would be a no-op. + if alias := aliasOptions(userCfg, agentFileName); alias != nil { + f.applyAliasOptions(ctx, alias) } // Build global permissions checker from user config settings. @@ -601,10 +611,14 @@ func (f *runExecFlags) applyUserSettings(ctx context.Context, userSettings *user f.hideToolResults = true slog.DebugContext(ctx, "Applying user settings", "hide_tool_results", true) } - if userSettings.YOLO && !f.autoApprove { + if userSettings.YOLO && !f.yoloChanged && !f.autoApprove { f.autoApprove = true slog.DebugContext(ctx, "Applying user settings", "YOLO", true) } + if s := f.scopedSafetyDefault(userSettings.Safety, userSettings.YOLO); s != "" { + f.defaultSafety = s + slog.DebugContext(ctx, "Applying user settings", "safety", string(s)) + } // The tour needs the full TUI's overlay support, so a lean default from // user config is not applied when the tour was requested. if userSettings.Lean && !f.leanChanged && !f.lean && !f.tour { @@ -617,6 +631,44 @@ func (f *runExecFlags) applyUserSettings(ctx context.Context, userSettings *user } } +// applyAliasOptions applies an alias's bundled runtime options. Like user +// settings they are defaults: an explicitly-passed flag wins. They are +// applied after applyUserSettings so an alias's own choices (including its +// safety default) take priority over the global settings. +func (f *runExecFlags) applyAliasOptions(ctx context.Context, alias *userconfig.Alias) { + slog.DebugContext(ctx, "Applying alias options", "yolo", alias.Yolo, "safety", string(alias.Safety), "model", alias.Model, "hide_tool_results", alias.HideToolResults, "sandbox", alias.Sandbox) + if alias.Yolo && !f.yoloChanged && !f.autoApprove { + f.autoApprove = true + } + // The alias's safety default (safety option, or legacy yolo → + // autonomous) outranks the user-settings default applied before it. + if s := f.scopedSafetyDefault(alias.Safety, alias.Yolo); s != "" { + f.defaultSafety = s + } + if alias.Model != "" && len(f.modelOverrides) == 0 { + f.modelOverrides = append(f.modelOverrides, alias.Model) + } + if alias.HideToolResults && !f.hideToolResults { + f.hideToolResults = true + } + // alias.Sandbox is consumed earlier in runRunCommand before + // dispatch; reaching runOrExec means the sandbox decision + // resolved to false (or the user opted out via --sandbox=false), + // so flipping it here would be a no-op. +} + +// aliasOptions resolves the alias options for an agent reference from an +// already-loaded user config, mirroring config.ResolveAlias: the empty +// reference maps to the "default" alias, and an alias without options is +// not returned. +func aliasOptions(cfg *userconfig.Config, agentFileName string) *userconfig.Alias { + alias, ok := cfg.GetAlias(cmp.Or(agentFileName, "default")) + if !ok || !alias.HasOptions() { + return nil + } + return alias +} + func (f *runExecFlags) loadTeamInWorktree(ctx context.Context, b backend, wd string) (*teamloader.LoadResult, *worktree.Worktree, string, error) { createdWorktree, err := f.setupWorktree(ctx, wd) if err != nil { @@ -901,20 +953,18 @@ func (f *runExecFlags) createLocalRuntimeAndSession(ctx context.Context, loadRes sess, err = sessStore.GetSession(ctx, resolvedID) switch { case err == nil: - // Flags override whatever the stored session carried, via - // the options (not raw field writes) so the legacy - // ToolsApproved flag and the mode stay in sync; --safety - // wins over --yolo when both are given, matching the - // option order buildSessionOpts applies to new sessions. - // The explicit autonomous escalation matters: --yolo must - // override even a session stored as balanced/strict. - // Without safety flags the stored state is left untouched — - // a plain resume must not reset ToolsApproved out from - // under a stored autonomous policy. - if req.SafetyPolicy != "" { + // Only an explicit CLI flag (--safety / --yolo, both resolved + // into req.SafetyPolicy with --safety winning) may override the + // mode a resumed session carries: alias options, user settings + // and author-declared YAML defaults are defaults for NEW + // sessions and must never replace persisted state. The override + // goes through the option (not a raw field write) so the legacy + // ToolsApproved flag and the mode stay in sync. Without an + // explicit flag the stored state is left untouched — a plain + // resume must not reset ToolsApproved out from under a stored + // autonomous policy. + if req.SafetyExplicit && req.SafetyPolicy != "" { session.WithSafetyPolicy(req.SafetyPolicy)(sess) - } else if req.ToolsApproved { - session.WithSafetyPolicy(session.SafetyPolicyAutonomous)(sess) } sess.HideToolResults = req.HideToolResults @@ -935,13 +985,13 @@ func (f *runExecFlags) createLocalRuntimeAndSession(ctx context.Context, loadRes // reuse it across runs — the first run creates, later runs resume. // A relative ref (-1, -2, ...) never lands here: it must resolve // against existing sessions. - sess = session.New(append(f.buildSessionOpts(agt, req), session.WithID(resolvedID))...) + sess = session.New(append(f.buildSessionOpts(agt, t, req), session.WithID(resolvedID))...) slog.DebugContext(ctx, "Creating session with caller-supplied ID", "session_id", resolvedID, "agent", agentName) default: return nil, nil, fmt.Errorf("loading session %q: %w", resolvedID, err) } } else { - sess = session.New(f.buildSessionOpts(agt, req)...) + sess = session.New(f.buildSessionOpts(agt, t, req)...) // Session is stored lazily on first UpdateSession call (when content is added) // This avoids creating empty sessions in the database slog.DebugContext(ctx, "Using local runtime", "agent", agentName) @@ -961,17 +1011,26 @@ func (f *runExecFlags) handleExecMode(ctx context.Context, out *cli.Printer, rt userMessages = args[1:] } - err := cli.Run(ctx, out, cli.Config{ + err := cli.Run(ctx, out, f.execCLIConfig(sess), rt, sess, userMessages) + if cliErr, ok := errors.AsType[cli.RuntimeError](err); ok { + return RuntimeError{Err: cliErr.Err} + } + return err +} + +// execCLIConfig builds the cli.Config for a non-TUI (--exec) run. +// AutoApprove is derived from the session's resolved safety mode rather +// than the raw --yolo flag: when a legacy yolo boolean and a typed safety +// default coexist (settings/alias), the typed mode wins for the session, +// and max-iteration auto-extension must match that resolved mode. +func (f *runExecFlags) execCLIConfig(sess *session.Session) cli.Config { + return cli.Config{ AppName: AppName, AttachmentPath: f.attachmentPath, HideToolCalls: f.hideToolCalls, OutputJSON: f.outputJSON, - AutoApprove: f.autoApprove, - }, rt, sess, userMessages) - if cliErr, ok := errors.AsType[cli.RuntimeError](err); ok { - return RuntimeError{Err: cliErr.Err} + AutoApprove: sess.GetSafetyPolicy() == session.SafetyPolicyAutonomous, } - return err } func readInitialMessage(args []string) (*string, error) { @@ -1113,19 +1172,93 @@ func (f *runExecFlags) buildAppOpts(args []string) ([]app.Opt, error) { // buildSessionOpts returns the canonical set of session options derived from // CLI flags and agent configuration. Both the initial session and spawned // sessions use this method so their options never drift apart. -func (f *runExecFlags) buildSessionOpts(agt *agent.Agent, req runtime.CreateSessionRequest) []session.Opt { +func (f *runExecFlags) buildSessionOpts(agt *agent.Agent, t *team.Team, req runtime.CreateSessionRequest) []session.Opt { return []session.Opt{ session.WithMaxIterations(agt.MaxIterations()), session.WithMaxConsecutiveToolCalls(agt.MaxConsecutiveToolCalls()), session.WithMaxOldToolCallTokens(agt.MaxOldToolCallTokens()), session.WithMaxToolResultTokens(agt.MaxToolResultTokens()), + // WithToolsApproved before WithSafetyPolicy so a resolved safety + // policy wins over the legacy yolo boolean; an empty policy is a + // no-op and leaves the yolo-derived state alone. session.WithToolsApproved(req.ToolsApproved), - session.WithSafetyPolicy(req.SafetyPolicy), + session.WithSafetyPolicy(effectiveNewSessionSafety(req, agt, t)), session.WithHideToolResults(req.HideToolResults), session.WithWorkingDir(req.WorkingDir), } } +// effectiveNewSessionSafety resolves the safety mode a FRESH session starts +// with. The user-owned request value (explicit CLI flags, alias options, +// user settings — already resolved in that order by createSessionRequest) +// always wins; only when the user expressed no preference at all do the +// author-declared YAML defaults apply: the selected agent's safety first, +// then the config-wide runtime.safety. Author configs — local, URL, or OCI +// — can therefore never override a user choice. Resumed sessions never go +// through this resolution; their stored mode is handled separately. +func effectiveNewSessionSafety(req runtime.CreateSessionRequest, agt *agent.Agent, t *team.Team) session.SafetyPolicy { + if req.SafetyPolicy != "" { + return req.SafetyPolicy + } + if req.ToolsApproved { + // Legacy user-owned yolo without a resolved policy: WithToolsApproved + // already pins autonomous; author defaults must not downgrade it. + return "" + } + if agt != nil { + if s := agt.Safety(); s != "" { + return session.SafetyPolicy(s) + } + } + if t != nil { + if s := t.RuntimeSafety(); s != "" { + return session.SafetyPolicy(s) + } + } + return "" +} + +// explicitCLISafety returns the safety mode explicitly requested on the +// command line, or empty when neither --safety nor --yolo was passed. +// --safety wins over --yolo; an explicit --yolo=false expresses no mode +// (it only suppresses yolo defaults from alias options and user settings). +func (f *runExecFlags) explicitCLISafety() session.SafetyPolicy { + if f.safetyChanged && f.safety != "" { + return session.SafetyPolicy(f.safety) + } + if f.yoloChanged && f.autoApprove { + return session.SafetyPolicyAutonomous + } + return "" +} + +// userSafetyPolicy resolves the user-owned safety mode for this run: +// explicit CLI flags first, then the alias/settings default. Empty means +// the user expressed no preference and author-declared YAML defaults may +// apply to fresh sessions. +func (f *runExecFlags) userSafetyPolicy() session.SafetyPolicy { + if s := f.explicitCLISafety(); s != "" { + return s + } + return f.defaultSafety +} + +// scopedSafetyDefault resolves one scope's (user settings or alias) +// safety default: the typed safety field wins over the legacy yolo +// boolean at the same scope, and the legacy boolean is ignored entirely +// once --yolo was explicitly passed — --yolo=true is already an explicit +// CLI mode, and --yolo=false must not be resurrected by a lower-scope +// yolo default. +func (f *runExecFlags) scopedSafetyDefault(safety latestcfg.SafetyMode, legacyYolo bool) session.SafetyPolicy { + if safety != "" { + return session.SafetyPolicy(safety) + } + if legacyYolo && !f.yoloChanged { + return session.SafetyPolicyAutonomous + } + return "" +} + // createSessionSpawner creates a function that can spawn new sessions with different working directories. func (f *runExecFlags) createSessionSpawner(agentSource config.Source, sessStore session.Store) tui.SessionSpawner { return func(spawnCtx context.Context, workingDir string) (*app.App, *session.Session, func(), error) { @@ -1170,7 +1303,7 @@ func (f *runExecFlags) createSessionSpawner(agentSource config.Source, sessStore // Create a new session spawnReq := f.createSessionRequest(workingDir) spawnReq.AgentName = agt.Name() - newSess := session.New(f.buildSessionOpts(agt, spawnReq)...) + newSess := session.New(f.buildSessionOpts(agt, t, spawnReq)...) // Create cleanup function cleanup := func() { diff --git a/cmd/root/run_session_test.go b/cmd/root/run_session_test.go index 2f76cbca0..40773bf61 100644 --- a/cmd/root/run_session_test.go +++ b/cmd/root/run_session_test.go @@ -7,10 +7,12 @@ import ( "github.com/stretchr/testify/require" "github.com/docker/docker-agent/pkg/agent" + "github.com/docker/docker-agent/pkg/config/latest" "github.com/docker/docker-agent/pkg/runtime" "github.com/docker/docker-agent/pkg/session" "github.com/docker/docker-agent/pkg/team" "github.com/docker/docker-agent/pkg/teamloader" + "github.com/docker/docker-agent/pkg/userconfig" ) func newSessionTestLoadResult() *teamloader.LoadResult { @@ -18,6 +20,18 @@ func newSessionTestLoadResult() *teamloader.LoadResult { return &teamloader.LoadResult{Team: team.New(team.WithAgents(agt))} } +// newSafetyTestLoadResult builds a two-agent team carrying author-declared +// safety defaults: "root" gets rootSafety, "other" declares none, and the +// team carries the config-wide runtime.safety default. +func newSafetyTestLoadResult(rootSafety, runtimeSafety latest.SafetyMode) *teamloader.LoadResult { + root := agent.New("root", "instructions", agent.WithModel(rootTestProvider{}), agent.WithSafety(rootSafety)) + other := agent.New("other", "instructions", agent.WithModel(rootTestProvider{})) + return &teamloader.LoadResult{Team: team.New( + team.WithAgents(root, other), + team.WithRuntimeSafety(runtimeSafety), + )} +} + // An explicit --session ID that doesn't exist yet creates a session with that // exact ID instead of failing, so a caller can own the ID across runs. func TestCreateLocalRuntimeAndSession_ExplicitUnknownIDCreatesWithThatID(t *testing.T) { @@ -51,6 +65,86 @@ func TestCreateLocalRuntimeAndSession_ExplicitExistingIDResumes(t *testing.T) { assert.Equal(t, "existing", sess.ID) } +// A fresh session resolves its safety mode from the user-owned request +// value first, then the selected agent's author default, then the +// config-wide runtime.safety default. +func TestCreateLocalRuntimeAndSession_FreshSessionSafetyResolution(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + rootSafety latest.SafetyMode + runtimeSafety latest.SafetyMode + req runtime.CreateSessionRequest + wantStored session.SafetyPolicy + wantEffective session.SafetyPolicy + }{ + { + name: "unset everywhere keeps the historical empty default", + }, + { + name: "runtime safety applies when nothing else is set", + runtimeSafety: latest.SafetyModeBalanced, + wantStored: session.SafetyPolicyBalanced, + wantEffective: session.SafetyPolicyBalanced, + }, + { + name: "selected agent safety wins over runtime safety", + rootSafety: latest.SafetyModeStrict, + runtimeSafety: latest.SafetyModeAutonomous, + wantStored: session.SafetyPolicyStrict, + wantEffective: session.SafetyPolicyStrict, + }, + { + name: "agent without safety falls back to runtime safety", + rootSafety: latest.SafetyModeStrict, + runtimeSafety: latest.SafetyModeBalanced, + req: runtime.CreateSessionRequest{AgentName: "other"}, + wantStored: session.SafetyPolicyBalanced, + wantEffective: session.SafetyPolicyBalanced, + }, + { + name: "author autonomous applies when the user is silent", + rootSafety: latest.SafetyModeAutonomous, + wantStored: session.SafetyPolicyAutonomous, + wantEffective: session.SafetyPolicyAutonomous, + }, + { + name: "user-owned safety wins over author defaults", + rootSafety: latest.SafetyModeAutonomous, + runtimeSafety: latest.SafetyModeAutonomous, + req: runtime.CreateSessionRequest{SafetyPolicy: session.SafetyPolicyStrict}, + wantStored: session.SafetyPolicyStrict, + wantEffective: session.SafetyPolicyStrict, + }, + { + name: "legacy user yolo wins over author defaults", + rootSafety: latest.SafetyModeStrict, + req: runtime.CreateSessionRequest{ToolsApproved: true}, + wantStored: session.SafetyPolicyAutonomous, + wantEffective: session.SafetyPolicyAutonomous, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + store := session.NewInMemorySessionStore() + f := &runExecFlags{} + req := tt.req + if req.AgentName == "" { + req.AgentName = "root" + } + + _, sess, err := f.createLocalRuntimeAndSession(t.Context(), newSafetyTestLoadResult(tt.rootSafety, tt.runtimeSafety), req, store) + require.NoError(t, err) + assert.Equal(t, tt.wantStored, sess.SafetyPolicy) + assert.Equal(t, tt.wantEffective, sess.GetSafetyPolicy()) + }) + } +} + // Resuming with --yolo a session created without it backfills // SafetyPolicy=autonomous (issue #3479), matching fresh --yolo sessions. func TestCreateLocalRuntimeAndSession_ResumeWithYoloBackfillsSafetyPolicy(t *testing.T) { @@ -59,8 +153,9 @@ func TestCreateLocalRuntimeAndSession_ResumeWithYoloBackfillsSafetyPolicy(t *tes store := session.NewInMemorySessionStore() require.NoError(t, store.AddSession(t.Context(), session.New(session.WithID("existing")))) - f := &runExecFlags{} - req := runtime.CreateSessionRequest{AgentName: "root", ResumeSessionID: "existing", ToolsApproved: true} + f := &runExecFlags{autoApprove: true, yoloChanged: true, sessionID: "existing"} + req := f.createSessionRequest("") + req.AgentName = "root" _, sess, err := f.createLocalRuntimeAndSession(t.Context(), newSessionTestLoadResult(), req, store) require.NoError(t, err) @@ -147,8 +242,9 @@ func TestCreateLocalRuntimeAndSession_ResumeWithYoloEscalatesStoredPolicy(t *tes session.WithSafetyPolicy(session.SafetyPolicyBalanced), ))) - f := &runExecFlags{} - req := runtime.CreateSessionRequest{AgentName: "root", ResumeSessionID: "existing", ToolsApproved: true} + f := &runExecFlags{autoApprove: true, yoloChanged: true, sessionID: "existing"} + req := f.createSessionRequest("") + req.AgentName = "root" _, sess, err := f.createLocalRuntimeAndSession(t.Context(), newSessionTestLoadResult(), req, store) require.NoError(t, err) @@ -164,13 +260,15 @@ func TestCreateLocalRuntimeAndSession_ResumeSafetyFlagWinsOverYolo(t *testing.T) store := session.NewInMemorySessionStore() require.NoError(t, store.AddSession(t.Context(), session.New(session.WithID("existing")))) - f := &runExecFlags{} - req := runtime.CreateSessionRequest{ - AgentName: "root", - ResumeSessionID: "existing", - ToolsApproved: true, - SafetyPolicy: session.SafetyPolicyStrict, + f := &runExecFlags{ + autoApprove: true, + yoloChanged: true, + safety: string(session.SafetyPolicyStrict), + safetyChanged: true, + sessionID: "existing", } + req := f.createSessionRequest("") + req.AgentName = "root" _, sess, err := f.createLocalRuntimeAndSession(t.Context(), newSessionTestLoadResult(), req, store) require.NoError(t, err) @@ -178,6 +276,57 @@ func TestCreateLocalRuntimeAndSession_ResumeSafetyFlagWinsOverYolo(t *testing.T) assert.False(t, sess.ToolsApproved, "explicit strict must revoke the blanket approval") } +// A safety default from user settings or an alias (settings.safety / +// settings.YOLO / alias options) is not an explicit CLI flag: it seeds +// NEW sessions only and must never replace the mode a resumed session +// carries. +func TestCreateLocalRuntimeAndSession_ResumeUserDefaultDoesNotOverride(t *testing.T) { + t.Parallel() + + store := session.NewInMemorySessionStore() + require.NoError(t, store.AddSession(t.Context(), session.New( + session.WithID("existing"), + session.WithSafetyPolicy(session.SafetyPolicyBalanced), + ))) + + // The settings.YOLO shape after applyUserSettings: autoApprove and the + // resolved default are set, but no CLI flag was passed. + f := &runExecFlags{autoApprove: true, defaultSafety: session.SafetyPolicyAutonomous, sessionID: "existing"} + req := f.createSessionRequest("") + req.AgentName = "root" + + _, sess, err := f.createLocalRuntimeAndSession(t.Context(), newSessionTestLoadResult(), req, store) + require.NoError(t, err) + assert.Equal(t, session.SafetyPolicyBalanced, sess.SafetyPolicy, "settings/alias defaults must not escalate a resumed session") + assert.False(t, sess.ToolsApproved) +} + +// Author-declared YAML defaults (agents..safety / runtime.safety) +// seed NEW sessions only: a resumed session keeps its stored mode — even +// the empty pre-modes default — no matter what the config declares. +func TestCreateLocalRuntimeAndSession_ResumeAuthorDefaultDoesNotOverride(t *testing.T) { + t.Parallel() + + stored := session.New(session.WithID("existing"), session.WithSafetyPolicy(session.SafetyPolicyStrict)) + empty := session.New(session.WithID("empty")) + + store := session.NewInMemorySessionStore() + require.NoError(t, store.AddSession(t.Context(), stored)) + require.NoError(t, store.AddSession(t.Context(), empty)) + + loadResult := newSafetyTestLoadResult(latest.SafetyModeAutonomous, latest.SafetyModeAutonomous) + + f := &runExecFlags{} + _, sess, err := f.createLocalRuntimeAndSession(t.Context(), loadResult, runtime.CreateSessionRequest{AgentName: "root", ResumeSessionID: "existing"}, store) + require.NoError(t, err) + assert.Equal(t, session.SafetyPolicyStrict, sess.SafetyPolicy) + assert.False(t, sess.ToolsApproved) + + _, sess, err = f.createLocalRuntimeAndSession(t.Context(), loadResult, runtime.CreateSessionRequest{AgentName: "root", ResumeSessionID: "empty"}, store) + require.NoError(t, err) + assert.Equal(t, session.SafetyPolicy(""), sess.SafetyPolicy, "author defaults must not be written onto a resumed session") +} + // A relative ref (e.g. -1) is resume-only: it must resolve against existing // sessions and never creates one. func TestCreateLocalRuntimeAndSession_RelativeRefDoesNotCreate(t *testing.T) { @@ -190,3 +339,37 @@ func TestCreateLocalRuntimeAndSession_RelativeRefDoesNotCreate(t *testing.T) { _, _, err := f.createLocalRuntimeAndSession(t.Context(), newSessionTestLoadResult(), req, store) require.Error(t, err) } + +// --exec derives its auto-approve behaviour (max-iteration auto-extension) +// from the session's resolved safety mode, not the raw legacy yolo boolean: +// when settings carry both YOLO=true and a typed non-autonomous safety, the +// typed mode wins and exec must not auto-approve. +func TestExecCLIConfig_TypedSafetyWinsOverLegacyYolo(t *testing.T) { + t.Parallel() + + f := &runExecFlags{} + f.applyUserSettings(t.Context(), &userconfig.Settings{YOLO: true, Safety: latest.SafetyModeBalanced}) + require.True(t, f.autoApprove, "the legacy boolean still applies for compatibility") + + req := f.createSessionRequest("") + req.AgentName = "root" + _, sess, err := f.createLocalRuntimeAndSession(t.Context(), newSessionTestLoadResult(), req, session.NewInMemorySessionStore()) + require.NoError(t, err) + require.Equal(t, session.SafetyPolicyBalanced, sess.GetSafetyPolicy()) + + assert.False(t, f.execCLIConfig(sess).AutoApprove, + "exec auto-approve must follow the resolved session mode, not the raw --yolo flag") +} + +// An autonomous session (e.g. explicit --yolo) keeps auto-approving in exec. +func TestExecCLIConfig_AutonomousSessionAutoApproves(t *testing.T) { + t.Parallel() + + f := &runExecFlags{autoApprove: true, yoloChanged: true} + req := f.createSessionRequest("") + req.AgentName = "root" + _, sess, err := f.createLocalRuntimeAndSession(t.Context(), newSessionTestLoadResult(), req, session.NewInMemorySessionStore()) + require.NoError(t, err) + + assert.True(t, f.execCLIConfig(sess).AutoApprove) +} diff --git a/cmd/root/run_user_config_test.go b/cmd/root/run_user_config_test.go new file mode 100644 index 000000000..e663cbf35 --- /dev/null +++ b/cmd/root/run_user_config_test.go @@ -0,0 +1,72 @@ +package root + +import ( + "io" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/cli" + "github.com/docker/docker-agent/pkg/config/latest" + "github.com/docker/docker-agent/pkg/paths" + "github.com/docker/docker-agent/pkg/userconfig" +) + +// `docker agent run` must fail with a clear validation error when the user +// config carries an invalid safety value, instead of silently discarding +// the whole config and running with defaults (issue #3837). +func TestRunOrExec_InvalidUserConfigFailsClearly(t *testing.T) { + // Not parallel: SetConfigDir mutates process-global state. + dir := t.TempDir() + paths.SetConfigDir(dir) + t.Cleanup(func() { paths.SetConfigDir("") }) + + require.NoError(t, os.WriteFile(filepath.Join(dir, "config.yaml"), + []byte("settings:\n safety: yolo\n"), 0o600)) + + f := &runExecFlags{} + err := f.runOrExec(t.Context(), cli.NewPrinter(io.Discard), nil, false) + require.ErrorContains(t, err, "loading user config") + require.ErrorContains(t, err, "settings.safety") + require.ErrorContains(t, err, "strict, balanced, autonomous") +} + +// Same for an invalid alias safety value: the error names the alias. +func TestRunOrExec_InvalidAliasSafetyFailsClearly(t *testing.T) { + // Not parallel: SetConfigDir mutates process-global state. + dir := t.TempDir() + paths.SetConfigDir(dir) + t.Cleanup(func() { paths.SetConfigDir("") }) + + require.NoError(t, os.WriteFile(filepath.Join(dir, "config.yaml"), + []byte("aliases:\n turbo:\n path: ./turbo.yaml\n safety: full-speed\n"), 0o600)) + + f := &runExecFlags{} + err := f.runOrExec(t.Context(), cli.NewPrinter(io.Discard), nil, false) + require.ErrorContains(t, err, "loading user config") + require.ErrorContains(t, err, "aliases.turbo.safety") +} + +// aliasOptions mirrors config.ResolveAlias against an already-loaded +// config: the empty reference maps to the "default" alias, and aliases +// without options are skipped. +func TestAliasOptions(t *testing.T) { + t.Parallel() + + cfg := &userconfig.Config{Aliases: map[string]*userconfig.Alias{ + "default": {Path: "./default.yaml", Safety: latest.SafetyModeBalanced}, + "plain": {Path: "./plain.yaml"}, + "turbo": {Path: "./turbo.yaml", Yolo: true}, + }} + + def := aliasOptions(cfg, "") + require.NotNil(t, def, "the empty reference resolves to the default alias") + assert.Equal(t, latest.SafetyModeBalanced, def.Safety) + + assert.Nil(t, aliasOptions(cfg, "plain"), "an alias without options is not applied") + assert.NotNil(t, aliasOptions(cfg, "turbo")) + assert.Nil(t, aliasOptions(cfg, "./file.yaml"), "a non-alias reference has no alias options") +} diff --git a/cmd/root/run_user_settings_test.go b/cmd/root/run_user_settings_test.go index 66072bcce..b840b19c5 100644 --- a/cmd/root/run_user_settings_test.go +++ b/cmd/root/run_user_settings_test.go @@ -5,6 +5,8 @@ import ( "github.com/stretchr/testify/assert" + "github.com/docker/docker-agent/pkg/config/latest" + "github.com/docker/docker-agent/pkg/session" "github.com/docker/docker-agent/pkg/userconfig" ) @@ -48,3 +50,168 @@ func TestRunExecFlagsApplyUserSettingsLean(t *testing.T) { }) } } + +func TestRunExecFlagsApplyUserSettingsSafety(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + settings userconfig.Settings + flags *runExecFlags + wantDefault session.SafetyPolicy + wantAutoApprove bool + }{ + { + name: "settings safety becomes the default", + settings: userconfig.Settings{Safety: latest.SafetyModeBalanced}, + wantDefault: session.SafetyPolicyBalanced, + }, + { + name: "legacy YOLO maps to autonomous", + settings: userconfig.Settings{YOLO: true}, + wantDefault: session.SafetyPolicyAutonomous, + wantAutoApprove: true, + }, + { + name: "safety wins over legacy YOLO at the same scope", + settings: userconfig.Settings{YOLO: true, Safety: latest.SafetyModeStrict}, + wantDefault: session.SafetyPolicyStrict, + wantAutoApprove: true, + }, + { + name: "explicit --yolo=false suppresses the legacy YOLO flag", + settings: userconfig.Settings{YOLO: true}, + flags: &runExecFlags{yoloChanged: true}, + // Both the auto-approve mutation and the yolo-derived safety + // default are suppressed by the explicit flag; a typed + // settings.safety would still apply. + wantDefault: "", + wantAutoApprove: false, + }, + { + name: "explicit --yolo=false keeps a typed safety default", + settings: userconfig.Settings{YOLO: true, Safety: latest.SafetyModeBalanced}, + flags: &runExecFlags{yoloChanged: true}, + wantDefault: session.SafetyPolicyBalanced, + wantAutoApprove: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + flags := tt.flags + if flags == nil { + flags = &runExecFlags{} + } + flags.applyUserSettings(t.Context(), &tt.settings) + + assert.Equal(t, tt.wantDefault, flags.defaultSafety) + assert.Equal(t, tt.wantAutoApprove, flags.autoApprove) + }) + } +} + +// Alias options are applied after user settings and outrank them; within +// the alias, safety wins over the legacy yolo flag. +func TestRunExecFlagsApplyAliasOptionsSafety(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + alias userconfig.Alias + flags *runExecFlags + wantDefault session.SafetyPolicy + }{ + { + name: "alias safety overrides the settings default", + alias: userconfig.Alias{Path: "x", Safety: latest.SafetyModeStrict}, + flags: &runExecFlags{defaultSafety: session.SafetyPolicyAutonomous}, + wantDefault: session.SafetyPolicyStrict, + }, + { + name: "alias legacy yolo maps to autonomous", + alias: userconfig.Alias{Path: "x", Yolo: true}, + flags: &runExecFlags{defaultSafety: session.SafetyPolicyBalanced}, + wantDefault: session.SafetyPolicyAutonomous, + }, + { + name: "alias safety wins over alias yolo", + alias: userconfig.Alias{Path: "x", Yolo: true, Safety: latest.SafetyModeBalanced}, + wantDefault: session.SafetyPolicyBalanced, + }, + { + name: "alias without safety keeps the settings default", + alias: userconfig.Alias{Path: "x", HideToolResults: true}, + flags: &runExecFlags{defaultSafety: session.SafetyPolicyBalanced}, + wantDefault: session.SafetyPolicyBalanced, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + flags := tt.flags + if flags == nil { + flags = &runExecFlags{} + } + flags.applyAliasOptions(t.Context(), &tt.alias) + + assert.Equal(t, tt.wantDefault, flags.defaultSafety) + }) + } +} + +// userSafetyPolicy resolves the user-owned mode: explicit CLI flags first +// (--safety over --yolo), then the alias/settings default. +func TestRunExecFlagsUserSafetyPolicy(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + flags *runExecFlags + want session.SafetyPolicy + wantExplicit bool + }{ + { + name: "nothing set resolves to empty", + }, + { + name: "explicit --safety wins over everything", + flags: &runExecFlags{safety: "strict", safetyChanged: true, autoApprove: true, yoloChanged: true, defaultSafety: session.SafetyPolicyAutonomous}, + want: session.SafetyPolicyStrict, + wantExplicit: true, + }, + { + name: "explicit --yolo wins over defaults", + flags: &runExecFlags{autoApprove: true, yoloChanged: true, defaultSafety: session.SafetyPolicyBalanced}, + want: session.SafetyPolicyAutonomous, + wantExplicit: true, + }, + { + name: "defaults apply without explicit flags", + flags: &runExecFlags{autoApprove: true, defaultSafety: session.SafetyPolicyBalanced}, + want: session.SafetyPolicyBalanced, + }, + { + name: "explicit --yolo=false is not an explicit mode", + flags: &runExecFlags{yoloChanged: true, defaultSafety: session.SafetyPolicyBalanced}, + want: session.SafetyPolicyBalanced, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + flags := tt.flags + if flags == nil { + flags = &runExecFlags{} + } + assert.Equal(t, tt.want, flags.userSafetyPolicy()) + assert.Equal(t, tt.wantExplicit, flags.explicitCLISafety() != "") + }) + } +} diff --git a/cmd/root/sandbox.go b/cmd/root/sandbox.go index 6751a8c07..a2a908709 100644 --- a/cmd/root/sandbox.go +++ b/cmd/root/sandbox.go @@ -283,6 +283,7 @@ func dockerAgentArgs(cmd *cobra.Command, args []string, configDir string) []stri var dockerAgentArgs []string hasYolo := false + hasSafety := false cmd.Flags().Visit(func(f *pflag.Flag) { if skip[f.Name] { return @@ -291,6 +292,9 @@ func dockerAgentArgs(cmd *cobra.Command, args []string, configDir string) []stri if f.Name == "yolo" { hasYolo = true } + if f.Name == "safety" { + hasSafety = true + } if f.Value.Type() == "bool" { dockerAgentArgs = append(dockerAgentArgs, "--"+f.Name+"="+f.Value.String()) @@ -298,7 +302,7 @@ func dockerAgentArgs(cmd *cobra.Command, args []string, configDir string) []stri dockerAgentArgs = append(dockerAgentArgs, "--"+f.Name, f.Value.String()) } }) - if !hasYolo { + if !hasYolo && !hasSafety { dockerAgentArgs = append(dockerAgentArgs, "--yolo") } diff --git a/cmd/root/sandbox_test.go b/cmd/root/sandbox_test.go index 74cd6de94..97e1b63b0 100644 --- a/cmd/root/sandbox_test.go +++ b/cmd/root/sandbox_test.go @@ -91,6 +91,27 @@ func TestDockerAgentArgs_PreservesUserYolo(t *testing.T) { assert.Contains(t, got, "--yolo=true") } +func TestDockerAgentArgs_PreservesUserSafetyWithoutInjectingYolo(t *testing.T) { + t.Parallel() + + cmd := &cobra.Command{ + RunE: func(*cobra.Command, []string) error { return nil }, + } + var sandboxFlag bool + var safety string + cmd.PersistentFlags().BoolVar(&sandboxFlag, "sandbox", false, "") + cmd.PersistentFlags().StringVar(&safety, "safety", "", "") + + require.NoError(t, cmd.ParseFlags([]string{"--sandbox", "--safety", "strict"})) + + got := dockerAgentArgs(cmd, []string{"./agent.yaml"}, "/cfg") + + assert.Contains(t, got, "--safety") + assert.Contains(t, got, "strict") + assert.NotContains(t, got, "--yolo", + "the sandbox default must not add --yolo when the user explicitly selected a safety mode") +} + func TestDockerAgentArgs_PreservesExplicitFalseBool(t *testing.T) { t.Parallel() diff --git a/docs/configuration/permissions/index.md b/docs/configuration/permissions/index.md index f944a175d..58bad45b0 100644 --- a/docs/configuration/permissions/index.md +++ b/docs/configuration/permissions/index.md @@ -33,6 +33,46 @@ Every session runs in a **safety mode** that decides what happens when no permis Pick a mode with the `--safety` flag (`docker-agent run --safety balanced ...`), the `safety_policy` field on session create (`POST /api/sessions`) or mid-session (`PATCH /api/sessions/:id/safety-policy`), or escalate directly from a confirmation prompt (`B` switches to balanced, `A` to autonomous). Sessions that never choose a mode keep the historical default: read-only tools auto-approve, everything else asks. +### Declarative Safety Defaults + +Safety modes can also be declared as **defaults** in YAML, at four scopes: + +| Scope | Location | Owner | +| ----- | -------- | ----- | +| Alias | `aliases..safety` in `~/.config/cagent/config.yaml` (or `docker agent alias add ... --safety `) | User | +| Global settings | `settings.safety` in `~/.config/cagent/config.yaml` | User | +| Per-agent | `agents..safety` in the agent YAML | Agent author | +| Config-wide | `runtime.safety` in the agent YAML | Agent author | + +```yaml +# Agent YAML (author-declared defaults) +runtime: + safety: balanced # config-wide default for new sessions + +agents: + root: + safety: strict # overrides runtime.safety for this agent +``` + +All four fields accept only the three canonical modes — `strict`, `balanced`, `autonomous` (yes, an author may declare `autonomous`) — and any other value fails loading with an error naming the field. The legacy spellings remain as aliases for `autonomous`: `settings.YOLO`, the alias `yolo` option, and the `--yolo` flag. When both are set at the same scope, `safety` wins over the legacy `YOLO`/`yolo`. + +For a **new** root session the first source in this order wins: + +1. explicit `--safety` flag +2. explicit `--yolo` flag +3. alias `safety`/`yolo` option +4. `settings.safety`/`settings.YOLO` (user config) +5. selected agent's `agents..safety` +6. `runtime.safety` +7. the historical default (read-only tools auto-approve, everything else asks) + +**Resuming a session never re-applies defaults**: the stored mode is kept unless you pass an explicit `--safety` or `--yolo` flag for that run. Agent switches, handoffs, and delegated sub-agent sessions inherit the active session's mode rather than resetting it. + +Sessions created through the API (`POST /api/sessions`) without a `safety_policy` receive the author-declared defaults (5–6) when their first run starts — the earliest point the agent configuration is loaded. If the server restarts before that first run, the session keeps the historical unset default (7). + +> [!WARNING] +> **Trust: author defaults never outrank you.** `runtime.safety` and `agents..safety` are written by the agent's author — which may be a config you pulled from a URL or an OCI registry. They only fill the gap when you expressed no preference: any user-owned source (CLI flag, alias option, user settings) always takes precedence, and a resumed session keeps its stored mode. Still, an author default of `autonomous` means a fresh session runs every tool call unprompted — review third-party configs before running them, or pin your own floor with `settings.safety` / `--safety`. + **Custom rules always win over the mode**, with one asymmetry: `ask:` rules written in an agent's YAML (or global config) are agent-author advisories and yield to a user-chosen `balanced`/`autonomous` mode, while `ask:` rules granted at the session level (interactive "always ask" decisions, the session permissions API) always prompt. ## Permission Levels diff --git a/docs/configuration/user-settings/index.md b/docs/configuration/user-settings/index.md index 5c338dec7..77a1b1329 100644 --- a/docs/configuration/user-settings/index.md +++ b/docs/configuration/user-settings/index.md @@ -43,7 +43,8 @@ You rarely need to hand-edit this file. Most fields are managed from the TUI's ` | `theme` | string | `default` | Theme name, loaded from a built-in theme or `~/.cagent/themes/.yaml`. The special value `auto` follows the terminal's light/dark background. See [Theming](../../features/tui/index.md#theming). | | `theme_dark` | string | `default` | Theme applied when `theme: auto` and the terminal background is dark. | | `theme_light` | string | `default-light` | Theme applied when `theme: auto` and the terminal background is light. | -| `YOLO` | boolean | `false` | Auto-approve all tool calls globally, across every agent you run. Mirrors the `--yolo` flag and the `/yolo` command. | +| `YOLO` | boolean | `false` | Auto-approve all tool calls globally, across every agent you run. Mirrors the `--yolo` flag and the `/yolo` command. Legacy alias for `safety: autonomous`; when both are set, `safety` wins. | +| `safety` | string | _unset_ | Default [safety mode](../permissions/index.md#safety-modes) for new sessions: `strict`, `balanced`, or `autonomous` (any other value fails config loading). Wins over the legacy `YOLO` flag. Applied when no explicit `--safety`/`--yolo` flag and no alias safety option was given; wins over the agent YAML's `agents..safety` / `runtime.safety` defaults. Never changes the mode of a resumed session. | | `lean` | boolean | `false` | Make the [lean TUI](../../features/tui/index.md#lean-tui) (simplified, minimal-chrome interface) the default for interactive runs instead of the full TUI. | | `tab_title_max_length` | int | `20` | Maximum display length for tab titles; longer titles are truncated with an ellipsis. | | `restore_tabs` | boolean | `false` | Restore previously open tabs when launching the TUI. | @@ -120,10 +121,11 @@ settings: ## Precedence Rules -User settings are the **lowest-priority** source: they establish defaults, and anything more specific wins. +User settings are a **low-priority** source: they establish defaults, and anything more specific wins. -- **CLI flags over user settings — except plain boolean flags going from `true` to `false`.** Where a `docker agent run` flag mirrors a setting, passing the flag for a specific run takes precedence over the setting for that run only, and the flag never modifies the saved user config file. This holds cleanly for `--lean` / `lean` and `--theme` / `theme`, which track whether the flag was explicitly passed on the command line. `--yolo` / `YOLO` and `--hide-tool-results` / `hide_tool_results` don't: they're plain booleans with no "was this explicitly set" tracking, so passing `--yolo=false` or `--hide-tool-results=false` cannot turn a saved `YOLO: true` / `hide_tool_results: true` setting off for that run — the saved `true` wins and is reapplied on top of the flag. Passing the flag to turn either *on* (`--yolo`, `--hide-tool-results`) works as expected regardless of the saved setting. -- **Aliases sit between CLI flags and user settings.** An [alias](../../features/cli/index.md#docker-agent-alias) (`docker agent alias add ...`) can bundle its own `yolo`, `model`, `hide_tool_results`, and `sandbox` defaults; those apply when the corresponding flag was not explicitly passed, the same way user settings do, but are resolved after user settings so an alias's own choices take priority over your global defaults. +- **CLI flags over user settings — except plain boolean flags going from `true` to `false`.** Where a `docker agent run` flag mirrors a setting, passing the flag for a specific run takes precedence over the setting for that run only, and the flag never modifies the saved user config file. This holds cleanly for `--lean` / `lean`, `--theme` / `theme`, and the safety flags `--safety` / `--yolo` (see below), which track whether the flag was explicitly passed on the command line. `--hide-tool-results` / `hide_tool_results` doesn't: it's a plain boolean with no "was this explicitly set" tracking, so passing `--hide-tool-results=false` cannot turn a saved `hide_tool_results: true` setting off for that run — the saved `true` wins and is reapplied on top of the flag. Passing the flag to turn it *on* works as expected regardless of the saved setting. +- **Safety has its own, fully-specified chain.** For a **new** session the first source in this order wins: explicit `--safety` flag > explicit `--yolo` flag > alias `safety`/`yolo` option > `settings.safety`/`settings.YOLO` > the agent YAML's `agents..safety` > the agent YAML's `runtime.safety` > the built-in default (read-only tools auto-approve, everything else asks). At each scope the `safety` field wins over the legacy `YOLO`/`yolo` boolean. Your settings therefore beat anything an agent author declared in YAML — an agent config loaded from a file, URL, or OCI registry can never override your `settings.safety`, alias option, or CLI flag. An explicit `--yolo=false` suppresses a saved `YOLO: true` / alias `yolo` for that run (other settings and YAML defaults still apply). **Resumed sessions keep their stored mode**: settings and alias defaults never touch them; only an explicit `--safety` or `--yolo` flag overrides a resume. See [Safety Modes](../permissions/index.md#safety-modes). +- **Aliases sit between CLI flags and user settings.** An [alias](../../features/cli/index.md#docker-agent-alias) (`docker agent alias add ...`) can bundle its own `yolo`, `safety`, `model`, `hide_tool_results`, and `sandbox` defaults; those apply when the corresponding flag was not explicitly passed, the same way user settings do, but are resolved after user settings so an alias's own choices take priority over your global defaults. - **Permissions are merged, not overridden.** Global `settings.permissions` and an agent's own `permissions:` are combined into a single set of `deny` → `allow` → `ask` patterns before evaluation — a global deny always blocks, regardless of what the agent config allows. See [Merging Behavior](../permissions/index.md#merging-behavior). - **Hooks are additive, not overridden.** For a given lifecycle event, hooks from the agent config, `settings.hooks`, `hooks.d/` drop-ins, and `--hook-*` CLI flags **all** run, in that order. Global hooks cannot be suppressed by an individual agent. - **Everything else is a plain default.** Fields with no CLI or agent-config equivalent (`sound`, `sound_threshold`, `restore_tabs`, `tab_title_max_length`, `split_diff_view`, `render_images`, `cache_stable_prompts`, `warn_on_cache_miss`, `busy_send_mode`, `keybindings`, `layout`) only ever come from `settings:` (or the `/settings` dialog that writes it) — there is nothing to override them per run. diff --git a/docs/features/cli/index.md b/docs/features/cli/index.md index 2bdebfeb1..149393a18 100644 --- a/docs/features/cli/index.md +++ b/docs/features/cli/index.md @@ -28,7 +28,8 @@ $ docker agent run [config] [message...] [flags] | Flag | Description | | --------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------- | | `-a, --agent ` | Run a specific agent from the config | -| `--yolo` | Auto-approve tool calls (unless explicitly denied) | +| `--yolo` | Auto-approve tool calls (unless explicitly denied). Legacy alias for `--safety autonomous`. | +| `--safety ` | Safety mode for tool approval: `strict` (ask for everything), `balanced` (auto-approve safe calls), or `autonomous` (approve everything). Wins over `--yolo` when both are given. Without the flag, the mode falls back to alias/user-config defaults, then the agent YAML's `agents..safety` / `runtime.safety`; a resumed session keeps its stored mode unless `--safety`/`--yolo` is passed explicitly. See [Safety Modes](../../configuration/permissions/index.md#safety-modes). | | `--model ` | Override model(s). Use `provider/model` for all agents, or `agent=provider/model` for specific agents. Comma-separate multiple overrides. | | `--session ` | Resume a previous session. Supports relative refs (`-1` = newest by creation time, `-2` = second-newest, … — creation order, not last-used). An explicit ID that does not exist yet is created with that ID, so a supervisor can own the session ID upfront and reuse it across runs. | | `-s, --session-db ` | Path to the SQLite session database (default: `/session.db`, so `~/.cagent/session.db` unless `--data-dir` is set) | @@ -506,6 +507,7 @@ $ docker agent alias add other ociReference # Add an alias with runtime options $ docker agent alias add yolo-coder myorg/coder --yolo +$ docker agent alias add careful-coder myorg/coder --safety balanced $ docker agent alias add fast-coder myorg/coder --model openai/gpt-4o-mini $ docker agent alias add safe-coder myorg/coder --sandbox $ docker agent alias add turbo myorg/coder --yolo --model anthropic/claude-sonnet-4-5 @@ -517,11 +519,14 @@ $ docker agent run yolo-coder **Alias Options:** Aliases can include runtime options that apply automatically when used: -- `--yolo` — Auto-approve tool calls (unless explicitly denied) when running the alias +- `--yolo` — Auto-approve tool calls (unless explicitly denied) when running the alias. Legacy alias for `--safety autonomous`. +- `--safety ` — Default [safety mode](../../configuration/permissions/index.md#safety-modes) (`strict`, `balanced`, or `autonomous`) when running the alias. Wins over the alias's `yolo` option; both are stored declaratively in the user config (`aliases..safety` / `aliases..yolo`), so you can also edit them there by hand. - `--model ` — Override the model for the alias - `--hide-tool-results` — Hide tool call results in the TUI when running the alias - `--sandbox` — Always run the alias inside a [Docker sandbox](../../configuration/sandbox/index.md) +Alias safety options are defaults for new sessions: an explicit `--safety`/`--yolo` on the command line wins over them, they win over `settings.safety`/`settings.YOLO` and over anything declared in the agent YAML, and they never change the mode of a resumed session. + When listing aliases, options are shown in brackets: ```bash diff --git a/docs/guides/headless/index.md b/docs/guides/headless/index.md index 38b90fde2..5d1c94013 100644 --- a/docs/guides/headless/index.md +++ b/docs/guides/headless/index.md @@ -77,7 +77,7 @@ For an untrusted or autonomous agent — anything acting without a human watchin $ docker agent run --sandbox --exec agent.yaml --json "Fix the failing test" ``` -Because the blast radius is contained by the VM boundary, `--sandbox` also makes unattended operation reasonable in CI — and it defaults to exactly that: unless you already passed a `--yolo` flag of your own, `--sandbox` injects `--yolo` for the agent process it runs inside the VM, so the command above already runs unattended with no confirmation prompts. Passing `--yolo` explicitly (`--sandbox --yolo --exec ...`) is equivalent and can make the intent clearer in a script, but it's optional. To keep confirmation prompts even inside the sandbox, opt out with `--yolo=false` — `--sandbox` only fills in the flag when you haven't set one yourself. +Because the blast radius is contained by the VM boundary, `--sandbox` also makes unattended operation reasonable in CI — and it defaults to exactly that: unless you already passed a `--yolo` or `--safety` flag of your own, `--sandbox` injects `--yolo` for the agent process it runs inside the VM, so the command above already runs unattended with no confirmation prompts. Passing `--yolo` explicitly (`--sandbox --yolo --exec ...`) is equivalent and can make the intent clearer in a script, but it's optional. To keep confirmation prompts even inside the sandbox, select a stricter mode (`--sandbox --safety strict`) or opt out of the legacy default with `--yolo=false` — `--sandbox` only fills in `--yolo` when neither safety flag was set. If your CI provider already runs each job in its own disposable VM or container — many hosted runners do — and nothing on the runner matters once the job ends, that may already give you an isolation boundary on its own. `--sandbox` still gives you the same guarantee independent of the CI provider, and starts to matter as soon as the agent runs on a persistent self-hosted runner, a long-lived container, or your own workstation. diff --git a/examples/README.md b/examples/README.md index 95b3b9134..838f288a0 100644 --- a/examples/README.md +++ b/examples/README.md @@ -68,7 +68,7 @@ Examples that wire up one of the toolsets shipped with docker-agent | [`background_jobs.yaml`](background_jobs.yaml) | `background_jobs` toolset for servers, watchers, and other long-running commands. | | [`shell_recall.yaml`](shell_recall.yaml) | `background_jobs` with recall enabled for finite long-running commands. | | [`docker-wiki.yaml`](docker-wiki.yaml) | OpenWiki-inspired documentation agent that initializes and updates a `docker-wiki/` directory with `/init`, `/update`, and `/status` commands. | -| [`safety_modes.yaml`](safety_modes.yaml) | Shell agent demonstrating the three safety modes (strict / balanced / autonomous): every shell command is classified safe / destructive / unknown and the session's mode decides what auto-runs and what asks. | +| [`safety_modes.yaml`](safety_modes.yaml) | Shell agents demonstrating the three safety modes (strict / balanced / autonomous) and the declarative YAML defaults: a config-wide `runtime.safety` plus a per-agent `safety` override. User choices (CLI flags, alias options, user settings) always win over the YAML defaults. | | [`filesystem.yaml`](filesystem.yaml) | Plain `filesystem` toolset. | | [`filesystem_allow_deny.yaml`](filesystem_allow_deny.yaml) | Restricting the filesystem tool with allow/deny path lists. | | [`script_shell.yaml`](script_shell.yaml) | Defining custom shell commands as named tools via `type: script`. | diff --git a/examples/safety_modes.yaml b/examples/safety_modes.yaml index b8210bd8c..368a5dba3 100644 --- a/examples/safety_modes.yaml +++ b/examples/safety_modes.yaml @@ -1,3 +1,23 @@ +# Safety modes can be declared right in the agent YAML: +# runtime.safety — config-wide default for new sessions +# agents..safety — per-agent override of the config default +# +# Only strict, balanced, and autonomous are accepted. These are DEFAULTS +# only; precedence (highest first) when a new root session starts: +# +# --safety flag > --yolo flag > alias safety/yolo option > +# settings.safety/YOLO (user config) > agents..safety > +# runtime.safety > built-in default (read-only tools auto-approve, +# everything else asks) +# +# A resumed session keeps its stored mode unless an explicit --safety or +# --yolo flag is passed. Trust warning: YAML defaults come from the agent +# author — a config pulled from a URL or OCI registry can declare +# `autonomous`, so review third-party configs before running them; your +# own settings, alias options, and CLI flags always win over them. +runtime: + safety: balanced + agents: root: model: anthropic/claude-haiku-4-5 @@ -12,10 +32,25 @@ agents: autonomous — everything runs (legacy --yolo); only custom deny/ask rules still gate - Pick a mode with `docker-agent run --safety balanced ...`, the - `safety_policy` field on session create (API), or escalate from a - confirmation prompt ([B] balanced, [A] autonomous). Custom - permissions rules (see permissions.yaml) always win over the mode. + This config declares `runtime.safety: balanced`, so fresh sessions + start balanced unless you choose otherwise (`--safety`, `--yolo`, + an alias option, or `settings.safety` in your user config). The + `auditor` agent overrides the config default with `safety: strict`. + You can still escalate from a confirmation prompt ([B] balanced, + [A] autonomous). Custom permissions rules (see permissions.yaml) + always win over the mode. instruction: Use the shell tool to run the command the user asks for. toolsets: - type: shell + + # Per-agent override: `docker-agent run safety_modes.yaml -a auditor` + # starts strict even though the config-wide default is balanced. + auditor: + model: anthropic/claude-haiku-4-5 + description: Read-oriented agent that asks before every tool call + safety: strict + instruction: > + Inspect the system with the shell tool. Prefer read-only commands + and explain what each command does before running it. + toolsets: + - type: shell diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index 7fd11413e..e787bddb1 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -42,6 +42,7 @@ type Agent struct { addEnvironmentInfo bool addDescriptionParameter bool redactSecrets bool + safety latest.SafetyMode // Author-declared safety-mode default for new sessions; empty when unset maxIterations int maxConsecutiveToolCalls int maxOldToolCallTokens int @@ -114,6 +115,15 @@ func (a *Agent) MaxIterations() int { return a.maxIterations } +// Safety returns the safety-mode default the agent's author declared in +// its config (agents..safety), or empty when unset. It is a +// default only: any user-owned choice (CLI flags, alias options, user +// settings) takes precedence when a session is created, and it never +// replaces the mode stored on a resumed session. +func (a *Agent) Safety() latest.SafetyMode { + return a.safety +} + func (a *Agent) MaxConsecutiveToolCalls() int { return a.maxConsecutiveToolCalls } diff --git a/pkg/agent/agent_test.go b/pkg/agent/agent_test.go index 77e7aedbd..2d2a88a8b 100644 --- a/pkg/agent/agent_test.go +++ b/pkg/agent/agent_test.go @@ -15,11 +15,20 @@ import ( "github.com/docker/docker-agent/pkg/chat" "github.com/docker/docker-agent/pkg/concurrent" + "github.com/docker/docker-agent/pkg/config/latest" "github.com/docker/docker-agent/pkg/model/provider/base" "github.com/docker/docker-agent/pkg/modelsdev" "github.com/docker/docker-agent/pkg/tools" ) +// Safety returns the author-declared default set via WithSafety, empty +// when unset. +func TestSafety(t *testing.T) { + t.Parallel() + assert.Equal(t, latest.SafetyMode(""), New("a", "").Safety()) + assert.Equal(t, latest.SafetyModeStrict, New("a", "", WithSafety(latest.SafetyModeStrict)).Safety()) +} + type stubToolSet struct { startErr error tools []tools.Tool diff --git a/pkg/agent/opts.go b/pkg/agent/opts.go index 4d58c5410..bd7464620 100644 --- a/pkg/agent/opts.go +++ b/pkg/agent/opts.go @@ -168,6 +168,14 @@ func WithRedactSecrets(redactSecrets bool) Opt { } } +// WithSafety sets the author-declared safety-mode default applied to new +// sessions started on this agent when the user has not chosen a mode. +func WithSafety(mode latest.SafetyMode) Opt { + return func(a *Agent) { + a.safety = mode + } +} + func WithAddDescriptionParameter(addDescriptionParameter bool) Opt { return func(a *Agent) { a.addDescriptionParameter = addDescriptionParameter diff --git a/pkg/config/latest/types.go b/pkg/config/latest/types.go index 52f71d33f..9efc5cb4b 100644 --- a/pkg/config/latest/types.go +++ b/pkg/config/latest/types.go @@ -128,6 +128,34 @@ func (b *BudgetConfig) validate() error { return nil } +// SafetyMode is a declarative safety-mode default that agent authors +// (runtime.safety, agents..safety) and users (settings.safety, +// alias safety) can put in YAML. Only the three canonical session modes +// are accepted; the legacy aliases the session layer still normalizes +// (unsafe, safer, safe-auto) are not valid in configuration files. +// +// The values mirror pkg/session.SafetyPolicy but are defined here so +// the config layer does not depend on the session layer. +type SafetyMode string + +const ( + // SafetyModeStrict prompts on every tool call. + SafetyModeStrict SafetyMode = "strict" + // SafetyModeBalanced auto-approves classifier-safe calls only. + SafetyModeBalanced SafetyMode = "balanced" + // SafetyModeAutonomous auto-approves every call (legacy yolo). + SafetyModeAutonomous SafetyMode = "autonomous" +) + +// Validate accepts the three canonical modes and empty (unset). +func (m SafetyMode) Validate() error { + switch m { + case "", SafetyModeStrict, SafetyModeBalanced, SafetyModeAutonomous: + return nil + } + return fmt.Errorf("invalid safety mode %q (valid: strict, balanced, autonomous)", string(m)) +} + // RuntimeDefaults captures execution-time defaults the agent author // wants applied when this config is run. The values act as defaults // only: an explicit CLI flag or user-config setting always wins. @@ -137,6 +165,13 @@ type RuntimeDefaults struct { // Useful for agents that always need filesystem/network isolation. Sandbox bool `json:"sandbox,omitempty" yaml:"sandbox,omitempty"` + // Safety is the safety mode new sessions default to when the user + // has not chosen one (no --safety/--yolo flag, no alias option, no + // user-config setting). It never overrides a user choice and never + // replaces the mode stored on a resumed session. A per-agent + // AgentConfig.Safety takes precedence over this config-wide default. + Safety SafetyMode `json:"safety,omitempty" yaml:"safety,omitempty"` + // NetworkAllowlist is the list of hosts that should be added to // the sandbox's default-deny network proxy when this agent runs in // a sandbox. Each entry is a hostname with an optional ":port" @@ -622,6 +657,12 @@ type AgentConfig struct { // tools whose annotations carry a read-only hint are listed and // callable. Equivalent to setting `readonly: true` on each toolset. ReadOnly bool `json:"readonly,omitempty" yaml:"readonly,omitempty"` + // Safety is the safety mode new sessions started on this agent + // default to when the user has not chosen one (no --safety/--yolo + // flag, no alias option, no user-config setting). It never overrides + // a user choice and never replaces the mode stored on a resumed + // session. Takes precedence over the config-wide RuntimeDefaults.Safety. + Safety SafetyMode `json:"safety,omitempty" yaml:"safety,omitempty"` // RedactSecrets enables every leg of the redact_secrets feature: // the pre_tool_use builtin (scrubs tool arguments), the // before_llm_call hook (scrubs outgoing chat content), and the diff --git a/pkg/config/latest/validate.go b/pkg/config/latest/validate.go index 6dc428e45..89647c767 100644 --- a/pkg/config/latest/validate.go +++ b/pkg/config/latest/validate.go @@ -22,6 +22,11 @@ func (t *Config) Validate() error { if err := t.Budget.validate(); err != nil { return fmt.Errorf("budget: %w", err) } + if t.Runtime != nil { + if err := t.Runtime.Safety.Validate(); err != nil { + return fmt.Errorf("runtime.safety: %w", err) + } + } for name := range t.Budgets { b := t.Budgets[name] if err := b.validate(); err != nil { @@ -70,6 +75,9 @@ func (t *Config) Validate() error { if err := agent.validateHarness(); err != nil { return err } + if err := agent.Safety.Validate(); err != nil { + return fmt.Errorf("agents.%s.safety: %w", agent.Name, err) + } if err := validateCompactionThreshold(agent.CompactionThreshold); err != nil { return fmt.Errorf("agents.%s: %w", agent.Name, err) } diff --git a/pkg/config/latest/validate_test.go b/pkg/config/latest/validate_test.go index e811e0cdd..584bf53da 100644 --- a/pkg/config/latest/validate_test.go +++ b/pkg/config/latest/validate_test.go @@ -563,6 +563,21 @@ func TestConfigValidateErrorWrapping(t *testing.T) { config: Config{Agents: Agents{{Name: "root", Hooks: &HooksConfig{Stop: HookDefinitions{{}}}}}}, wantErr: "hooks.stop[0]: type is required", }, + { + name: "runtime safety error", + config: Config{Runtime: &RuntimeDefaults{Safety: "yolo"}}, + wantErr: "runtime.safety: invalid safety mode \"yolo\" (valid: strict, balanced, autonomous)", + }, + { + name: "runtime safety legacy alias rejected", + config: Config{Runtime: &RuntimeDefaults{Safety: "unsafe"}}, + wantErr: "runtime.safety: invalid safety mode \"unsafe\"", + }, + { + name: "agent safety error", + config: Config{Agents: Agents{{Name: "root", Safety: "Strict"}}}, + wantErr: "agents.root.safety: invalid safety mode \"Strict\" (valid: strict, balanced, autonomous)", + }, } for _, tt := range tests { @@ -602,10 +617,14 @@ func TestConfigValidateValidConfig(t *testing.T) { Toolsets: map[string]Toolset{ "web": {Type: "fetch", AllowedDomains: []string{"example.com", "*.example.org"}}, }, + // All three canonical modes are explicitly permitted at runtime and + // agent scope, autonomous included. + Runtime: &RuntimeDefaults{Safety: SafetyModeAutonomous}, Agents: Agents{ { Name: "root", Model: "main", + Safety: SafetyModeBalanced, Fallback: &FallbackConfig{Models: []string{"pick"}, Retries: -1, Cooldown: Duration{Duration: time.Minute}}, Harness: &HarnessConfig{Type: "claude-code", Effort: "high"}, CompactionThreshold: new(1.0), @@ -615,8 +634,38 @@ func TestConfigValidateValidConfig(t *testing.T) { }, Hooks: &HooksConfig{Stop: HookDefinitions{{Type: "command", Command: "echo done"}}}, }, + {Name: "careful", Model: "main", Safety: SafetyModeStrict}, }, } require.NoError(t, cfg.Validate()) } + +func TestSafetyModeValidate(t *testing.T) { + t.Parallel() + + cases := map[SafetyMode]bool{ + "": true, + SafetyModeStrict: true, + SafetyModeBalanced: true, + SafetyModeAutonomous: true, + + // Legacy session aliases and near-misses are rejected: only the + // three canonical modes may appear in YAML safety fields. + "unsafe": false, + "safer": false, + "safe-auto": false, + "yolo": false, + "Balanced": false, // case-sensitive on purpose + } + + for in, wantOK := range cases { + err := in.Validate() + if wantOK { + require.NoErrorf(t, err, "SafetyMode(%q).Validate()", string(in)) + } else { + require.ErrorContainsf(t, err, "invalid safety mode", "SafetyMode(%q).Validate()", string(in)) + require.ErrorContainsf(t, err, "strict, balanced, autonomous", "SafetyMode(%q).Validate()", string(in)) + } + } +} diff --git a/pkg/config/schema_test.go b/pkg/config/schema_test.go index e7827e665..2d81318c4 100644 --- a/pkg/config/schema_test.go +++ b/pkg/config/schema_test.go @@ -143,6 +143,7 @@ func TestSchemaMatchesGoTypes(t *testing.T) { "ScriptShellToolConfig": reflect.TypeFor[latest.ScriptShellToolConfig](), "PostEditConfig": reflect.TypeFor[latest.PostEditConfig](), "PermissionsConfig": reflect.TypeFor[latest.PermissionsConfig](), + "RuntimeDefaults": reflect.TypeFor[latest.RuntimeDefaults](), "BudgetConfig": reflect.TypeFor[latest.BudgetConfig](), "HooksConfig": reflect.TypeFor[latest.HooksConfig](), "HookMatcherConfig": reflect.TypeFor[latest.HookMatcherConfig](), diff --git a/pkg/runtime/payload.go b/pkg/runtime/payload.go index f775ce3dd..f53e92494 100644 --- a/pkg/runtime/payload.go +++ b/pkg/runtime/payload.go @@ -46,9 +46,19 @@ type CreateSessionRequest struct { // ToolsApproved is the legacy --yolo signal. New callers should // prefer SafetyPolicy; option setters keep both in sync. ToolsApproved bool `json:"tools_approved"` - // SafetyPolicy is the per-session safety preference; empty falls - // back to the ToolsApproved-derived default. See [session.SafetyPolicy]. - SafetyPolicy session.SafetyPolicy `json:"safety_policy,omitempty"` + // SafetyPolicy is the user-owned per-session safety preference (CLI + // flags, alias options, user settings — never author YAML); empty + // falls back to the ToolsApproved-derived default or, for fresh local + // sessions, to the author-declared config defaults. See + // [session.SafetyPolicy]. + SafetyPolicy session.SafetyPolicy `json:"safety_policy,omitempty"` + // SafetyExplicit marks a SafetyPolicy chosen explicitly on the command + // line (--safety / --yolo) rather than resolved from alias or + // user-settings defaults. Resuming a stored session only honours + // explicit values: defaults must never replace the persisted mode. + // Not serialized: the remote protocol has no resume path yet (--remote + // is mutually exclusive with --session). + SafetyExplicit bool `json:"-"` HideToolResults bool `json:"hide_tool_results"` SessionDB string `json:"session_db,omitempty"` ResumeSessionID string `json:"resume_session_id,omitempty"` diff --git a/pkg/server/author_safety_test.go b/pkg/server/author_safety_test.go new file mode 100644 index 000000000..866e732fc --- /dev/null +++ b/pkg/server/author_safety_test.go @@ -0,0 +1,293 @@ +package server + +import ( + "context" + "errors" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/config" + "github.com/docker/docker-agent/pkg/runtime" + "github.com/docker/docker-agent/pkg/session" + "github.com/docker/docker-agent/pkg/session/sqlitestore" + "github.com/docker/docker-agent/pkg/team" +) + +// authorSafetyConfig declares author safety defaults on both levels: the +// root agent carries its own mode while the sibling inherits the +// config-wide runtime.safety. Harness-backed agents need no model provider +// or API key, so runtimeForSession can build a real runtime offline (same +// trick as TestRuntimeForSession_RegistersSessionScopedElicitationSink). +const authorSafetyConfig = `agents: + root: + description: Test agent + instruction: Be helpful. + safety: balanced + harness: + type: claude-code + other: + description: Second agent + instruction: Be helpful. + harness: + type: claude-code +runtime: + safety: strict +` + +// plainConfig declares no safety default anywhere. +const plainConfig = `agents: + root: + description: Test agent + instruction: Be helpful. + harness: + type: claude-code +` + +// strictRootConfig declares a default different from authorSafetyConfig's +// root agent, so a retry after a failed build can prove the failed +// attempt's selection was never committed. +const strictRootConfig = `agents: + root: + description: Test agent + instruction: Be helpful. + safety: strict + harness: + type: claude-code +` + +func newAuthorSafetySessionManager(t *testing.T) (*SessionManager, session.Store) { + t.Helper() + + ctx := t.Context() + store, err := sqlitestore.New(ctx, filepath.Join(t.TempDir(), "sessions.db")) + require.NoError(t, err) + t.Cleanup(func() { _ = store.Close() }) + + sources := config.Sources{ + "agent.yaml": config.NewBytesSource("agent.yaml", []byte(authorSafetyConfig)), + "plain.yaml": config.NewBytesSource("plain.yaml", []byte(plainConfig)), + "strict.yaml": config.NewBytesSource("strict.yaml", []byte(strictRootConfig)), + } + return NewSessionManager(ctx, sources, store, 0, &config.RuntimeConfig{}), store +} + +// buildRuntime builds a runtime for the session the way RunSession's first +// call does, closing it on test cleanup. +func buildRuntime(t *testing.T, sm *SessionManager, sess *session.Session, agentFilename, currentAgent string) { + t.Helper() + + run, _, err := sm.runtimeForSession(t.Context(), sess, agentFilename, currentAgent, &config.RuntimeConfig{}) + require.NoError(t, err) + t.Cleanup(func() { _ = run.Close() }) +} + +// A new API session with no safety choice takes the selected agent's +// author-declared default on its first run, and the choice is persisted so +// it survives later resumes. +func TestAuthorSafetyDefault_SelectedAgentWinsAndPersists(t *testing.T) { + t.Parallel() + + ctx := t.Context() + sm, store := newAuthorSafetySessionManager(t) + + sess, err := sm.CreateSession(ctx, session.New()) + require.NoError(t, err) + require.Equal(t, session.SafetyPolicy(""), sess.GetSafetyPolicy()) + + buildRuntime(t, sm, sess, "agent.yaml", "root") + + assert.Equal(t, session.SafetyPolicyBalanced, sess.GetSafetyPolicy(), + "selected agent safety must win over the config-wide runtime.safety") + + stored, err := store.GetSession(ctx, sess.ID) + require.NoError(t, err) + assert.Equal(t, session.SafetyPolicyBalanced, stored.GetSafetyPolicy(), + "the applied default must be persisted so resumes keep it") +} + +// When the selected agent declares no safety, the config-wide +// runtime.safety default applies. +func TestAuthorSafetyDefault_RuntimeSafetyFallback(t *testing.T) { + t.Parallel() + + ctx := t.Context() + sm, store := newAuthorSafetySessionManager(t) + + sess, err := sm.CreateSession(ctx, session.New()) + require.NoError(t, err) + + buildRuntime(t, sm, sess, "agent.yaml", "other") + + assert.Equal(t, session.SafetyPolicyStrict, sess.GetSafetyPolicy()) + + stored, err := store.GetSession(ctx, sess.ID) + require.NoError(t, err) + assert.Equal(t, session.SafetyPolicyStrict, stored.GetSafetyPolicy()) +} + +// An explicit safety policy supplied with the create request is never +// overwritten by author defaults. +func TestAuthorSafetyDefault_ExplicitPolicyPreserved(t *testing.T) { + t.Parallel() + + ctx := t.Context() + sm, _ := newAuthorSafetySessionManager(t) + + sess, err := sm.CreateSession(ctx, session.New(session.WithSafetyPolicy(session.SafetyPolicyAutonomous))) + require.NoError(t, err) + + buildRuntime(t, sm, sess, "agent.yaml", "root") + + assert.Equal(t, session.SafetyPolicyAutonomous, sess.GetSafetyPolicy(), + "an explicit API safety policy must not be replaced by the agent's default") +} + +// The legacy tools_approved=true template signal is a user choice +// (autonomous); author defaults must not downgrade it. +func TestAuthorSafetyDefault_LegacyToolsApprovedPreserved(t *testing.T) { + t.Parallel() + + ctx := t.Context() + sm, _ := newAuthorSafetySessionManager(t) + + tpl := session.New() + tpl.ToolsApproved = true // raw shape an old API client sends + sess, err := sm.CreateSession(ctx, tpl) + require.NoError(t, err) + + buildRuntime(t, sm, sess, "agent.yaml", "root") + + assert.Equal(t, session.SafetyPolicyAutonomous, sess.GetSafetyPolicy(), + "legacy tools_approved must stay effective as autonomous") + assert.True(t, sess.IsToolsApproved()) +} + +// A session that already exists in the store (created by an earlier +// process) is indistinguishable from a deliberate empty mode: resuming it +// must not apply author defaults. +func TestAuthorSafetyDefault_ExistingSessionNotRedefaulted(t *testing.T) { + t.Parallel() + + ctx := t.Context() + sm, store := newAuthorSafetySessionManager(t) + + sess := session.New() + require.NoError(t, store.AddSession(ctx, sess)) + + buildRuntime(t, sm, sess, "agent.yaml", "root") + + assert.Equal(t, session.SafetyPolicy(""), sess.GetSafetyPolicy(), + "a persisted session not created by this process must keep its stored mode") +} + +// A mode the client sets between CreateSession and the first run is a user +// choice: the pending author default must not clobber it. +func TestAuthorSafetyDefault_ClientChoiceBeforeFirstRunWins(t *testing.T) { + t.Parallel() + + ctx := t.Context() + sm, store := newAuthorSafetySessionManager(t) + + created, err := sm.CreateSession(ctx, session.New()) + require.NoError(t, err) + require.NoError(t, sm.SetSessionSafetyPolicy(ctx, created.ID, session.SafetyPolicyStrict)) + + // RunSession re-reads the session from the store before building the + // first runtime; mirror that so the update above is visible. + sess, err := store.GetSession(ctx, created.ID) + require.NoError(t, err) + buildRuntime(t, sm, sess, "agent.yaml", "root") + + assert.Equal(t, session.SafetyPolicyStrict, sess.GetSafetyPolicy()) +} + +// The pending marker is consumed by the first runtime build even when the +// loaded config declares no default, so a later rebuild (restart, agent or +// config switch) can never re-default the session. +func TestAuthorSafetyDefault_ConsumedOnFirstBuild(t *testing.T) { + t.Parallel() + + ctx := t.Context() + sm, _ := newAuthorSafetySessionManager(t) + + sess, err := sm.CreateSession(ctx, session.New()) + require.NoError(t, err) + + buildRuntime(t, sm, sess, "plain.yaml", "root") + require.Equal(t, session.SafetyPolicy(""), sess.GetSafetyPolicy(), + "plain.yaml declares no default, so the mode stays empty") + + buildRuntime(t, sm, sess, "agent.yaml", "root") + assert.Equal(t, session.SafetyPolicy(""), sess.GetSafetyPolicy(), + "a later build must not apply defaults the first build did not") +} + +// Switching agents after the first run must not change an established +// safety mode, even when the new agent declares its own default. +func TestAuthorSafetyDefault_AgentSwitchKeepsMode(t *testing.T) { + t.Parallel() + + ctx := t.Context() + sm, _ := newAuthorSafetySessionManager(t) + + sess, err := sm.CreateSession(ctx, session.New()) + require.NoError(t, err) + + buildRuntime(t, sm, sess, "agent.yaml", "other") + require.Equal(t, session.SafetyPolicyStrict, sess.GetSafetyPolicy()) + + // A rebuild selecting the root agent (safety: balanced) — e.g. after a + // server restart with a dynamic agent switch — keeps the seeded mode. + buildRuntime(t, sm, sess, "agent.yaml", "root") + assert.Equal(t, session.SafetyPolicyStrict, sess.GetSafetyPolicy()) +} + +// A runtime build that fails after the author default could be selected +// must not commit anything: the session (in memory and in the store) +// stays unchosen and the pending marker survives, so a retry with a +// different config applies THAT config's default and consumes the marker +// exactly once. +func TestAuthorSafetyDefault_FailedBuildKeepsMarkerForRetry(t *testing.T) { + t.Parallel() + + ctx := t.Context() + sm, store := newAuthorSafetySessionManager(t) + + sess, err := sm.CreateSession(ctx, session.New()) + require.NoError(t, err) + + // First build: team load and agent selection succeed (root declares + // safety: balanced, so a default is selectable), then runtime + // construction fails. + buildErr := errors.New("synthetic runtime construction failure") + sm.newRuntime = func(context.Context, *team.Team, ...runtime.Opt) (runtime.Runtime, error) { + return nil, buildErr + } + _, _, err = sm.runtimeForSession(ctx, sess, "agent.yaml", "root", &config.RuntimeConfig{}) + require.ErrorIs(t, err, buildErr) + + assert.Equal(t, session.SafetyPolicy(""), sess.GetSafetyPolicy(), + "a failed build must not seed the in-memory session") + stored, err := store.GetSession(ctx, sess.ID) + require.NoError(t, err) + assert.Equal(t, session.SafetyPolicy(""), stored.GetSafetyPolicy(), + "a failed build must not persist any default") + _, pending := sm.pendingSafetyDefaults.Load(sess.ID) + assert.True(t, pending, "the pending marker must survive a failed build") + + // Retry with a valid config declaring a different default: the retry + // must apply and persist that default, not the failed attempt's. + sm.newRuntime = nil + buildRuntime(t, sm, sess, "strict.yaml", "root") + + assert.Equal(t, session.SafetyPolicyStrict, sess.GetSafetyPolicy()) + stored, err = store.GetSession(ctx, sess.ID) + require.NoError(t, err) + assert.Equal(t, session.SafetyPolicyStrict, stored.GetSafetyPolicy(), + "the retry's default must be persisted") + _, pending = sm.pendingSafetyDefaults.Load(sess.ID) + assert.False(t, pending, "the marker must be consumed by the successful build") +} diff --git a/pkg/server/session_manager.go b/pkg/server/session_manager.go index c4f8053da..cfde25130 100644 --- a/pkg/server/session_manager.go +++ b/pkg/server/session_manager.go @@ -52,6 +52,11 @@ type SessionManager struct { runConfig *config.RuntimeConfig + // newRuntime, when non-nil, replaces runtime.New as the runtime + // constructor in runtimeForSession. Test seam: lets a build fail + // deterministically after the team has been loaded. + newRuntime func(context.Context, *team.Team, ...runtime.Opt) (runtime.Runtime, error) + refreshInterval time.Duration mux sync.Mutex @@ -79,6 +84,15 @@ type SessionManager struct { sessionReady chan struct{} sessionReadyOnce sync.Once + // pendingSafetyDefaults tracks sessions created by CreateSession in this + // process without any user/API safety choice. The author-declared YAML + // defaults (selected agent safety, then runtime.safety) are only known + // once the team is loaded, so they are applied when the first runtime is + // built for such a session (see applyAuthorSafetyDefault) and the ID is + // dropped. Older persisted sessions resumed with an empty mode never + // appear here and are never re-defaulted. + pendingSafetyDefaults *concurrent.Map[string, struct{}] + // followUpInjectors routes follow-ups and idle recalls for an attached // session to its owner instead of queues that are only drained mid-stream. // The injector starts a real turn whose events reach the owner and every @@ -110,17 +124,18 @@ func NewSessionManager(ctx context.Context, sources config.Sources, sessionStore } sm := &SessionManager{ - runtimeSessions: concurrent.NewMap[string, *activeRuntimes](), - deletedSessions: concurrent.NewMap[string, *activeRuntimes](), - eventLogs: concurrent.NewMap[string, *pumpedEventLog](), - deletedEventLogs: make(map[string]struct{}), - followUpInjectors: concurrent.NewMap[string, FollowUpInjector](), - followUpKeys: concurrent.NewMap[string, *idempotencyCache](), - sessionStore: sessionStore, - Sources: loaders, - refreshInterval: refreshInterval, - runConfig: runConfig, - sessionReady: make(chan struct{}), + runtimeSessions: concurrent.NewMap[string, *activeRuntimes](), + deletedSessions: concurrent.NewMap[string, *activeRuntimes](), + eventLogs: concurrent.NewMap[string, *pumpedEventLog](), + deletedEventLogs: make(map[string]struct{}), + followUpInjectors: concurrent.NewMap[string, FollowUpInjector](), + followUpKeys: concurrent.NewMap[string, *idempotencyCache](), + pendingSafetyDefaults: concurrent.NewMap[string, struct{}](), + sessionStore: sessionStore, + Sources: loaders, + refreshInterval: refreshInterval, + runConfig: runConfig, + sessionReady: make(chan struct{}), } return sm @@ -471,10 +486,10 @@ func (sm *SessionManager) GetSessionSnapshot(ctx context.Context, id string) (*a // title) and fall back to the store when the session is not attached. var sess *session.Session streaming := false - agent := "" + agentName := "" if rs, ok := sm.runtimeSessions.Load(id); ok { sess = rs.session - agent = rs.runtime.CurrentAgentName(ctx) + agentName = rs.runtime.CurrentAgentName(ctx) // Probe streaming state without interfering: TryLock succeeds only // when no RunStream is in progress. if rs.streaming.TryLock() { @@ -507,7 +522,7 @@ func (sm *SessionManager) GetSessionSnapshot(ctx context.Context, id string) (*a InputTokens: inputTokens, OutputTokens: outputTokens, Streaming: streaming, - Agent: agent, + Agent: agentName, LastEventSeq: lastSeq, Cost: sess.TotalCost(), }, nil @@ -567,7 +582,20 @@ func (sm *SessionManager) CreateSession(ctx context.Context, sessionTemplate *se sess.CustomModelsUsed = append([]string(nil), sessionTemplate.CustomModelsUsed...) } - return sess, sm.sessionStore.AddSession(ctx, sess) + if err := sm.sessionStore.AddSession(ctx, sess); err != nil { + return nil, err + } + + // The caller expressed no safety choice (no explicit policy, no legacy + // tools_approved): the author-declared YAML defaults may seed the mode, + // but they are only known once the team is loaded for the first run. + // Mark the session so the first runtime build applies them exactly once + // (see applyAuthorSafetyDefault); a template-supplied choice stands as-is. + if sess.GetSafetyPolicy() == "" { + sm.pendingSafetyDefaults.Store(sess.ID, struct{}{}) + } + + return sess, nil } // Sentinel errors returned by ForkSession. Matched via errors.Is by @@ -741,6 +769,7 @@ func (sm *SessionManager) DeleteSession(ctx context.Context, sessionID string) e sm.dropEventLog(sess.ID) sm.followUpInjectors.Delete(sess.ID) sm.followUpKeys.Delete(sess.ID) + sm.pendingSafetyDefaults.Delete(sess.ID) return nil } @@ -1315,6 +1344,18 @@ func (sm *SessionManager) runtimeForSession(ctx context.Context, sess *session.S sess.MaxOldToolCallTokens = agt.MaxOldToolCallTokens() sess.MaxToolResultTokens = agt.MaxToolResultTokens() + // Select (but do not yet commit) the author-declared safety default: + // the selected agent's safety first, then the config-wide + // runtime.safety. Committing — mutating the session, persisting, + // consuming the pending marker — waits until the whole construction + // below has succeeded, so a failed build leaves the session eligible + // for a retry with a different config/agent whose default may differ + // (see applyAuthorSafetyDefault). + authorSafetyDefault := session.SafetyPolicy(agt.Safety()) + if authorSafetyDefault == "" { + authorSafetyDefault = session.SafetyPolicy(t.RuntimeSafety()) + } + modelSwitcherCfg := &runtime.ModelSwitcherConfig{ Models: loadResult.Models, Providers: loadResult.Providers, @@ -1342,10 +1383,24 @@ func (sm *SessionManager) runtimeForSession(ctx context.Context, sess *session.S runtime.WithTracer(otel.Tracer(version.AppName)), runtime.WithModelSwitcherConfig(modelSwitcherCfg), } - run, err := runtime.New(ctx, t, opts...) + newRuntime := runtime.New + if sm.newRuntime != nil { + newRuntime = sm.newRuntime + } + run, err := newRuntime(ctx, t, opts...) if err != nil { return nil, nil, err } + // If any later construction step fails, close the runtime before + // returning: the caller only ever sees the error, so an unclosed + // runtime would leak its tool sets. + defer func() { + if err != nil { + if closeErr := run.Close(); closeErr != nil { + slog.WarnContext(ctx, "Failed to close runtime after failed construction", "session_id", sess.ID, "error", closeErr) + } + } + }() // Give this session an out-of-band, session-scoped route for // elicitations raised while nobody is synchronously reading this @@ -1372,11 +1427,54 @@ func (sm *SessionManager) runtimeForSession(ctx context.Context, sess *session.S titleGen = sessiontitle.New(titleModels[0], titleModels[1:]...) } + // Construction succeeded: the selected author default may now be + // committed and the pending marker consumed, exactly once. + sm.applyAuthorSafetyDefault(ctx, sess, authorSafetyDefault) + slog.DebugContext(ctx, "Runtime created for session", "session_id", sess.ID) return run, titleGen, nil } +// applyAuthorSafetyDefault seeds an API-created session that carries no +// user safety choice with the author-declared YAML default selected by +// runtimeForSession (the selected agent's safety first, then the +// config-wide runtime.safety). It must only run once the session's first +// runtime has been fully constructed: a failed build keeps the pending +// marker and leaves the session untouched, so a retry — possibly with a +// different config or agent — applies that configuration's default +// instead. Only sessions this process created via CreateSession +// (pendingSafetyDefaults) are seeded, so an older persisted session +// resumed with an empty mode is never re-defaulted behind the user's +// back. The marker is consumed even when no default applies: later +// rebuilds and agent switches must not change an established mode. +func (sm *SessionManager) applyAuthorSafetyDefault(ctx context.Context, sess *session.Session, policy session.SafetyPolicy) { + if _, pending := sm.pendingSafetyDefaults.Load(sess.ID); !pending { + return + } + sm.pendingSafetyDefaults.Delete(sess.ID) + + // Re-check: the client may have chosen a mode between CreateSession and + // this first run (safety_policy update or the legacy tools_approved + // toggle); a user choice always wins over author defaults. + if sess.GetSafetyPolicy() != "" { + return + } + if policy == "" { + return + } + + // SetSafetyPolicy keeps the legacy ToolsApproved flag in sync. + sess.SetSafetyPolicy(policy) + // Persist so the default survives resumes after this process exits. + // RunSession persists the session again right after building the + // runtime, so a failure here is logged rather than failing the run. + if err := sm.sessionStore.UpdateSession(ctx, sess); err != nil { + slog.WarnContext(ctx, "failed to persist author-declared safety default", + "session_id", sess.ID, "safety_policy", string(policy), "err", err) + } +} + func (sm *SessionManager) loadTeam(ctx context.Context, agentFilename string, runConfig *config.RuntimeConfig) (*team.Team, error) { agentSource, err := sm.resolveSource(agentFilename) if err != nil { @@ -1797,10 +1895,13 @@ func (sm *SessionManager) SetSessionAgentModel(ctx context.Context, sessionID, m title := sess.TitleSnapshot() inputTokens, outputTokens, cost := sess.TokensAndCost() updatedSess := &session.Session{ - ID: sess.ID, - Title: title, - CreatedAt: sess.CreatedAt, - WorkingDir: sess.WorkingDir, + ID: sess.ID, + Title: title, + CreatedAt: sess.CreatedAt, + WorkingDir: sess.WorkingDir, + // SafetyPolicy must travel with ToolsApproved: omitting it would + // reset a strict/balanced session to the legacy default on reload. + SafetyPolicy: sess.SafetyPolicy, ToolsApproved: sess.ToolsApproved, Permissions: sess.Permissions, MaxIterations: sess.MaxIterations, @@ -1903,6 +2004,7 @@ func (sm *SessionManager) BatchDeleteSessions(ctx context.Context, sessionIDs [] sm.dropEventLog(sessionID) sm.followUpInjectors.Delete(sessionID) sm.followUpKeys.Delete(sessionID) + sm.pendingSafetyDefaults.Delete(sessionID) } } diff --git a/pkg/server/session_manager_test.go b/pkg/server/session_manager_test.go index 6604a4eb5..555e541d8 100644 --- a/pkg/server/session_manager_test.go +++ b/pkg/server/session_manager_test.go @@ -94,16 +94,17 @@ func newTestSessionManager(t *testing.T, sess *session.Session, fake runtime.Run require.NoError(t, store.AddSession(ctx, sess)) sm := &SessionManager{ - runtimeSessions: concurrent.NewMap[string, *activeRuntimes](), - deletedSessions: concurrent.NewMap[string, *activeRuntimes](), - eventLogs: concurrent.NewMap[string, *pumpedEventLog](), - deletedEventLogs: make(map[string]struct{}), - followUpInjectors: concurrent.NewMap[string, FollowUpInjector](), - followUpKeys: concurrent.NewMap[string, *idempotencyCache](), - sessionStore: store, - Sources: config.Sources{}, - runConfig: &config.RuntimeConfig{}, - sessionReady: make(chan struct{}), + runtimeSessions: concurrent.NewMap[string, *activeRuntimes](), + deletedSessions: concurrent.NewMap[string, *activeRuntimes](), + eventLogs: concurrent.NewMap[string, *pumpedEventLog](), + deletedEventLogs: make(map[string]struct{}), + followUpInjectors: concurrent.NewMap[string, FollowUpInjector](), + followUpKeys: concurrent.NewMap[string, *idempotencyCache](), + pendingSafetyDefaults: concurrent.NewMap[string, struct{}](), + sessionStore: store, + Sources: config.Sources{}, + runConfig: &config.RuntimeConfig{}, + sessionReady: make(chan struct{}), } // Pre-register a runtime for this session so RunSession skips agent loading. diff --git a/pkg/server/session_models_test.go b/pkg/server/session_models_test.go index 3c5214bce..0f691c8d4 100644 --- a/pkg/server/session_models_test.go +++ b/pkg/server/session_models_test.go @@ -432,6 +432,39 @@ func TestSessionManager_SetSessionAgentModel_RuntimeFailureLeavesStateUntouched( assert.Equal(t, []string{"openai/gpt-4o"}, sess.CustomModelsUsed) } +// SetSessionAgentModel persists a partial clone of the live session; a +// clone that drops SafetyPolicy would silently reset a strict/balanced +// session to the legacy default on the next reload, breaking the +// guarantee that resumes preserve the chosen safety mode. +func TestSessionManager_SetSessionAgentModel_PreservesSafetyPolicy(t *testing.T) { + t.Parallel() + + for _, policy := range []session.SafetyPolicy{session.SafetyPolicyStrict, session.SafetyPolicyBalanced} { + t.Run(string(policy), func(t *testing.T) { + t.Parallel() + + ctx := t.Context() + store := session.NewInMemorySessionStore() + sess := session.New(session.WithSafetyPolicy(policy)) + require.NoError(t, store.AddSession(ctx, sess)) + + fake := newModelSwitchingRuntime(nil) + + sm := NewSessionManager(ctx, config.Sources{}, store, 0, &config.RuntimeConfig{}) + sm.AttachRuntime(t.Context(), sess.ID, fake, sess) + + _, _, err := sm.SetSessionAgentModel(ctx, sess.ID, "openai/gpt-4o") + require.NoError(t, err) + + stored, err := store.GetSession(ctx, sess.ID) + require.NoError(t, err) + assert.Equal(t, policy, stored.GetSafetyPolicy(), + "a model switch must not reset the persisted safety mode") + assert.Equal(t, "openai/gpt-4o", stored.AgentModelOverrides["root"]) + }) + } +} + // Server-side errors (store-write failures, runtime errors that aren't // the well-known sentinels) must be reported as 500, not 400. 400 is // reserved for client-side mistakes like an invalid request body. diff --git a/pkg/team/team.go b/pkg/team/team.go index 092738a93..e533596c3 100644 --- a/pkg/team/team.go +++ b/pkg/team/team.go @@ -15,6 +15,11 @@ import ( type Team struct { agents []*agent.Agent permissions *permissions.Checker + // runtimeSafety is the config-wide safety-mode default declared under + // runtime.safety, retained so session constructors can apply it when + // neither the user nor the selected agent chose a mode. Empty when the + // config declares none (or the team was built without a config). + runtimeSafety latest.SafetyMode // agentConfigs holds the raw resolved config for each agent, keyed by // name. It is retained only when the team is built from a config file // (WithAgentConfigs) so surfaces like the agent inspector can show @@ -46,6 +51,14 @@ func WithAgentConfigs(configs map[string]latest.AgentConfig) Opt { } } +// WithRuntimeSafety records the config-wide runtime.safety default the +// team was loaded with. +func WithRuntimeSafety(mode latest.SafetyMode) Opt { + return func(t *Team) { + t.runtimeSafety = mode + } +} + func New(opts ...Opt) *Team { t := &Team{} for _, opt := range opts { @@ -167,6 +180,13 @@ func (t *Team) Permissions() *permissions.Checker { return t.permissions } +// RuntimeSafety returns the config-wide safety-mode default declared under +// runtime.safety, or empty when the config declares none. It is a default +// only: user-owned choices and per-agent safety take precedence. +func (t *Team) RuntimeSafety() latest.SafetyMode { + return t.runtimeSafety +} + // SetPermissions replaces the team's permission checker. // This is used to merge additional permission sources (e.g. user-level global // permissions) into the team's checker after construction. diff --git a/pkg/team/team_test.go b/pkg/team/team_test.go index fa3b60238..fe8a38781 100644 --- a/pkg/team/team_test.go +++ b/pkg/team/team_test.go @@ -14,6 +14,14 @@ func newAgent(name string) *agent.Agent { return agent.New(name, "") } +// RuntimeSafety returns the config-wide default set via WithRuntimeSafety, +// empty when unset. +func TestRuntimeSafety(t *testing.T) { + t.Parallel() + assert.Equal(t, latest.SafetyMode(""), New().RuntimeSafety()) + assert.Equal(t, latest.SafetyModeBalanced, New(WithRuntimeSafety(latest.SafetyModeBalanced)).RuntimeSafety()) +} + func TestDefaultAgent(t *testing.T) { t.Parallel() t.Run("empty team returns error", func(t *testing.T) { diff --git a/pkg/teamloader/teamloader.go b/pkg/teamloader/teamloader.go index 90c846822..5aadbd1aa 100644 --- a/pkg/teamloader/teamloader.go +++ b/pkg/teamloader/teamloader.go @@ -310,6 +310,7 @@ func LoadWithConfig(ctx context.Context, agentSource config.Source, runConfig *c agent.WithAddEnvironmentInfo(agentConfig.AddEnvironmentInfo), agent.WithAddDescriptionParameter(agentConfig.AddDescriptionParameter), agent.WithRedactSecrets(agentConfig.RedactSecretsEnabled()), + agent.WithSafety(agentConfig.Safety), agent.WithAddPromptFiles(promptFiles), agent.WithMaxIterations(agentConfig.MaxIterations), agent.WithMaxConsecutiveToolCalls(agentConfig.MaxConsecutiveToolCalls), @@ -488,11 +489,19 @@ func LoadWithConfig(ctx context.Context, agentSource config.Source, runConfig *c } } + // runtime.safety is a config-wide session default; it travels on the + // team so session constructors can consult it without the raw config. + var runtimeSafety latest.SafetyMode + if cfg.Runtime != nil { + runtimeSafety = cfg.Runtime.Safety + } + return &LoadResult{ Team: team.New( team.WithAgents(agents...), team.WithPermissions(permChecker), team.WithAgentConfigs(agentConfigs), + team.WithRuntimeSafety(runtimeSafety), ), Models: cfg.Models, Providers: cfg.Providers, diff --git a/pkg/teamloader/teamloader_test.go b/pkg/teamloader/teamloader_test.go index ed3cf2214..a5d59d577 100644 --- a/pkg/teamloader/teamloader_test.go +++ b/pkg/teamloader/teamloader_test.go @@ -1131,6 +1131,82 @@ func TestLoadPropagatesMaxToolResultTokens(t *testing.T) { assert.Equal(t, 512, agt.MaxToolResultTokens()) } +// TestLoadPropagatesSafetyDefaults verifies the author-declared safety +// defaults travel from the YAML config to the built team: runtime.safety +// lands on the team (team.RuntimeSafety) and agents..safety on the +// agent (agent.Safety), where session constructors resolve them. +func TestLoadPropagatesSafetyDefaults(t *testing.T) { + t.Setenv("OPENAI_API_KEY", "dummy") + + data := []byte(`runtime: + safety: autonomous +agents: + root: + model: openai/gpt-4o + instruction: test + safety: balanced + careful: + model: openai/gpt-4o + instruction: test +`) + + team, err := Load(t.Context(), config.NewBytesSource("safety.yaml", data), &config.RuntimeConfig{}, withTestProviderRegistry()...) + require.NoError(t, err) + + assert.Equal(t, latest.SafetyModeAutonomous, team.RuntimeSafety()) + + root, err := team.Agent("root") + require.NoError(t, err) + assert.Equal(t, latest.SafetyModeBalanced, root.Safety()) + + careful, err := team.Agent("careful") + require.NoError(t, err) + assert.Empty(t, careful.Safety(), "agent without a safety default carries none of its own") +} + +// TestLoadRejectsInvalidSafety pins the load-time failure: a non-canonical +// safety value anywhere in the config must fail loading with an error that +// names the offending field. +func TestLoadRejectsInvalidSafety(t *testing.T) { + t.Setenv("OPENAI_API_KEY", "dummy") + + tests := []struct { + name string + data string + wantErr string + }{ + { + name: "runtime scope", + data: `runtime: + safety: yolo +agents: + root: + model: openai/gpt-4o + instruction: test +`, + wantErr: "runtime.safety: invalid safety mode \"yolo\"", + }, + { + name: "agent scope", + data: `agents: + root: + model: openai/gpt-4o + instruction: test + safety: safe-auto +`, + wantErr: "agents.root.safety: invalid safety mode \"safe-auto\"", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := Load(t.Context(), config.NewBytesSource("bad.yaml", []byte(tt.data)), &config.RuntimeConfig{}, withTestProviderRegistry()...) + require.ErrorContains(t, err, tt.wantErr) + require.ErrorContains(t, err, "strict, balanced, autonomous") + }) + } +} + // TestLoadWithConfig_GlobalProviders covers user-level custom providers // (seeded into the runtime config from the user config file): they must // resolve inline `provider/model` references in any agent config, while diff --git a/pkg/userconfig/userconfig.go b/pkg/userconfig/userconfig.go index 827f3a064..d8a776f31 100644 --- a/pkg/userconfig/userconfig.go +++ b/pkg/userconfig/userconfig.go @@ -26,8 +26,13 @@ import ( type Alias struct { // Path is the agent file path or OCI reference Path string `yaml:"path" json:"path"` - // Yolo enables auto-approve mode for all tool calls + // Yolo enables auto-approve mode for all tool calls. Legacy alias for + // Safety: autonomous; when both are set, Safety wins. Yolo bool `yaml:"yolo,omitempty" json:"yolo,omitempty"` + // Safety is the default safety mode applied when the alias is run and + // no explicit --safety/--yolo flag was passed: strict, balanced, or + // autonomous. Wins over the legacy Yolo flag. + Safety latest.SafetyMode `yaml:"safety,omitempty" json:"safety,omitempty"` // Model overrides the agent's model (format: [agent=]provider/model) Model string `yaml:"model,omitempty" json:"model,omitempty"` // HideToolResults hides tool call results in the TUI @@ -38,7 +43,22 @@ type Alias struct { // HasOptions returns true if the alias has any runtime options set func (a *Alias) HasOptions() bool { - return a != nil && (a.Yolo || a.Model != "" || a.HideToolResults || a.Sandbox) + return a != nil && (a.Yolo || a.Safety != "" || a.Model != "" || a.HideToolResults || a.Sandbox) +} + +// GetSafety returns the alias's safety-mode default: Safety when set, +// otherwise autonomous when the legacy Yolo flag is set, otherwise empty. +func (a *Alias) GetSafety() latest.SafetyMode { + if a == nil { + return "" + } + if a.Safety != "" { + return a.Safety + } + if a.Yolo { + return latest.SafetyModeAutonomous + } + return "" } // Settings represents global user settings @@ -65,8 +85,13 @@ type Settings struct { // ThemeLight is the theme applied when Theme is "auto" and the terminal // background is light. Defaults to "default-light". ThemeLight string `yaml:"theme_light,omitempty"` - // YOLO enables auto-approve mode for all tool calls globally + // YOLO enables auto-approve mode for all tool calls globally. Legacy + // alias for Safety: autonomous; when both are set, Safety wins. YOLO bool `yaml:"YOLO,omitempty"` + // Safety is the global default safety mode applied when no explicit + // --safety/--yolo flag and no alias safety option was given: strict, + // balanced, or autonomous. Wins over the legacy YOLO flag. + Safety latest.SafetyMode `yaml:"safety,omitempty"` // Lean makes the simplified TUI with minimal chrome the default UI. Lean bool `yaml:"lean,omitempty"` // TabTitleMaxLength is the maximum display length for tab titles in the TUI. @@ -234,6 +259,21 @@ func (s *Settings) SnapshotsEnabled() bool { return s != nil && s.Snapshot != nil && *s.Snapshot } +// GetSafety returns the global safety-mode default: Safety when set, +// otherwise autonomous when the legacy YOLO flag is set, otherwise empty. +func (s *Settings) GetSafety() latest.SafetyMode { + if s == nil { + return "" + } + if s.Safety != "" { + return s.Safety + } + if s.YOLO { + return latest.SafetyModeAutonomous + } + return "" +} + // CacheStablePromptsEnabled reports whether chronological instruction updates are enabled. func (s *Settings) CacheStablePromptsEnabled() bool { return s != nil && s.CacheStablePrompts != nil && *s.CacheStablePrompts @@ -402,9 +442,34 @@ func readConfig(configPath string) (*Config, error) { "path", configPath, "version", config.Version) } + if err := config.validateSafety(); err != nil { + return nil, fmt.Errorf("invalid config file %s: %w", configPath, err) + } + return config, nil } +// validateSafety rejects non-canonical safety modes in settings and +// aliases. YAML parsing is lenient about unknown values, so this is the +// only place a typo like `safety: yolo` gets a clear error instead of +// silently behaving as "unset". +func (c *Config) validateSafety() error { + if c.Settings != nil { + if err := c.Settings.Safety.Validate(); err != nil { + return fmt.Errorf("settings.safety: %w", err) + } + } + for name, alias := range c.Aliases { + if alias == nil { + continue + } + if err := alias.Safety.Validate(); err != nil { + return fmt.Errorf("aliases.%s.safety: %w", name, err) + } + } + return nil +} + // migrateFromLegacy migrates aliases from the legacy aliases.yaml file. // Returns true if any aliases were migrated. // After successful migration, the legacy file is deleted. @@ -565,6 +630,9 @@ func (c *Config) SetAlias(name string, alias *Alias) error { if alias == nil || alias.Path == "" { return errors.New("agent path cannot be empty") } + if err := alias.Safety.Validate(); err != nil { + return fmt.Errorf("safety: %w", err) + } c.mu.Lock() defer c.mu.Unlock() diff --git a/pkg/userconfig/userconfig_test.go b/pkg/userconfig/userconfig_test.go index 61027dfc6..3d5ee3584 100644 --- a/pkg/userconfig/userconfig_test.go +++ b/pkg/userconfig/userconfig_test.go @@ -743,6 +743,7 @@ func TestAlias_HasOptions(t *testing.T) { {"nil alias", nil, false}, {"empty alias", &Alias{Path: "test"}, false}, {"yolo only", &Alias{Path: "test", Yolo: true}, true}, + {"safety only", &Alias{Path: "test", Safety: latest.SafetyModeBalanced}, true}, {"model only", &Alias{Path: "test", Model: "openai/gpt-4o"}, true}, {"hide_tool_results only", &Alias{Path: "test", HideToolResults: true}, true}, {"sandbox only", &Alias{Path: "test", Sandbox: true}, true}, @@ -1415,3 +1416,128 @@ func TestLoad_UnknownVersionStillLoads(t *testing.T) { require.NoError(t, err) assert.Equal(t, "https://gw.example.com", cfg.ModelsGateway) } + +func TestConfig_Settings_Safety(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + configFile := filepath.Join(tmpDir, "config.yaml") + require.NoError(t, os.WriteFile(configFile, []byte(`settings: + safety: balanced +`), 0o644)) + + cfg, err := loadFrom(configFile, "") + require.NoError(t, err) + assert.Equal(t, latest.SafetyModeBalanced, cfg.GetSettings().Safety) + assert.Equal(t, latest.SafetyModeBalanced, cfg.GetSettings().GetSafety()) +} + +func TestConfig_Settings_InvalidSafetyFailsLoad(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + configFile := filepath.Join(tmpDir, "config.yaml") + require.NoError(t, os.WriteFile(configFile, []byte(`settings: + safety: yolo +`), 0o644)) + + _, err := loadFrom(configFile, "") + require.ErrorContains(t, err, "settings.safety") + require.ErrorContains(t, err, `invalid safety mode "yolo" (valid: strict, balanced, autonomous)`) +} + +func TestConfig_Alias_InvalidSafetyFailsLoad(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + configFile := filepath.Join(tmpDir, "config.yaml") + require.NoError(t, os.WriteFile(configFile, []byte(`aliases: + coder: + path: myorg/coder + safety: unsafe +`), 0o644)) + + _, err := loadFrom(configFile, "") + require.ErrorContains(t, err, "aliases.coder.safety") + require.ErrorContains(t, err, `invalid safety mode "unsafe"`) +} + +func TestSettings_GetSafety(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + settings *Settings + want latest.SafetyMode + }{ + {"nil settings", nil, ""}, + {"unset", &Settings{}, ""}, + {"safety set", &Settings{Safety: latest.SafetyModeStrict}, latest.SafetyModeStrict}, + {"legacy YOLO maps to autonomous", &Settings{YOLO: true}, latest.SafetyModeAutonomous}, + {"safety wins over legacy YOLO", &Settings{YOLO: true, Safety: latest.SafetyModeBalanced}, latest.SafetyModeBalanced}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, tt.settings.GetSafety()) + }) + } +} + +func TestAlias_GetSafety(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + alias *Alias + want latest.SafetyMode + }{ + {"nil alias", nil, ""}, + {"unset", &Alias{Path: "x"}, ""}, + {"safety set", &Alias{Path: "x", Safety: latest.SafetyModeBalanced}, latest.SafetyModeBalanced}, + {"legacy yolo maps to autonomous", &Alias{Path: "x", Yolo: true}, latest.SafetyModeAutonomous}, + {"safety wins over legacy yolo", &Alias{Path: "x", Yolo: true, Safety: latest.SafetyModeStrict}, latest.SafetyModeStrict}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, tt.alias.GetSafety()) + }) + } +} + +func TestConfig_SetAlias_InvalidSafety(t *testing.T) { + t.Parallel() + + config := &Config{Aliases: make(map[string]*Alias)} + err := config.SetAlias("bad", &Alias{Path: "myorg/coder", Safety: "yolo"}) + require.ErrorContains(t, err, "safety") + require.ErrorContains(t, err, "invalid safety mode") +} + +// Alias safety must survive a save/load round trip alongside the other +// options, and unknown keys must still round-trip via Extra. +func TestConfig_AliasSafetyRoundTrip(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + configFile := filepath.Join(tmpDir, "config.yaml") + + config := &Config{ + Aliases: map[string]*Alias{ + "careful": {Path: "myorg/coder", Safety: latest.SafetyModeStrict}, + }, + Settings: &Settings{Safety: latest.SafetyModeBalanced}, + } + require.NoError(t, config.saveTo(configFile)) + + loaded, err := loadFrom(configFile, "") + require.NoError(t, err) + + alias, ok := loaded.GetAlias("careful") + require.True(t, ok) + assert.Equal(t, latest.SafetyModeStrict, alias.Safety) + assert.Equal(t, latest.SafetyModeBalanced, loaded.GetSettings().Safety) +}