From aad4f6bce1cace88e862e2c32d9a308eaa499e88 Mon Sep 17 00:00:00 2001 From: Trung Nguyen Date: Tue, 16 Jun 2026 11:18:46 +0200 Subject: [PATCH 1/2] feat: add session-scoped plan/build mode Plan mode lets the runtime filter tools to read-only ones and inject a per-turn system reminder, so the agent drafts a plan instead of taking actions. Build mode is the default and preserves today's behaviour. The mode is a per-session property, persisted alongside the rest of the session, and exposed via: - POST /api/sessions { ..., mode } - GET /api/sessions/:id -> { ..., mode } - PATCH /api/sessions/:id/mode { mode } Tool filtering is driven by the MCP-spec ReadOnlyHint annotation, so it extends to user-added MCP tools without any per-tool config. --- pkg/api/types.go | 13 ++++++ pkg/runtime/harness.go | 3 +- pkg/runtime/loop.go | 41 +++++++++++++++++-- pkg/runtime/plan_mode.go | 45 +++++++++++++++++++++ pkg/runtime/runtime_test.go | 54 +++++++++++++++++++++++++ pkg/server/server.go | 22 ++++++++++ pkg/server/server_test.go | 58 +++++++++++++++++++++++++++ pkg/server/session_manager.go | 28 +++++++++++++ pkg/session/branch.go | 1 + pkg/session/migrations.go | 7 ++++ pkg/session/migrations_pinned_test.go | 2 +- pkg/session/session.go | 50 +++++++++++++++++++++++ pkg/session/store.go | 27 ++++++++----- pkg/session/store_test.go | 45 +++++++++++++++++++++ 14 files changed, 379 insertions(+), 17 deletions(-) create mode 100644 pkg/runtime/plan_mode.go diff --git a/pkg/api/types.go b/pkg/api/types.go index 5994ea5fb..ec6aac21b 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -135,6 +135,7 @@ type SessionResponse struct { OutputTokens int64 `json:"output_tokens"` WorkingDir string `json:"working_dir,omitempty"` Permissions *session.PermissionsConfig `json:"permissions,omitempty"` + Mode session.Mode `json:"mode,omitempty"` } // UpdateSessionPermissionsRequest represents a request to update session permissions. @@ -142,6 +143,17 @@ type UpdateSessionPermissionsRequest struct { Permissions *session.PermissionsConfig `json:"permissions"` } +// UpdateSessionModeRequest represents a request to update a session's mode. +type UpdateSessionModeRequest struct { + Mode session.Mode `json:"mode"` +} + +// UpdateSessionModeResponse represents the response from updating a session's mode. +type UpdateSessionModeResponse struct { + ID string `json:"id"` + Mode session.Mode `json:"mode"` +} + // ResumeSessionRequest represents a request to resume a session type ResumeSessionRequest struct { Confirmation string `json:"confirmation"` @@ -304,6 +316,7 @@ type SessionSnapshotResponse struct { Messages []session.Message `json:"messages"` ToolsApproved bool `json:"tools_approved"` Permissions *session.PermissionsConfig `json:"permissions,omitempty"` + Mode session.Mode `json:"mode,omitempty"` InputTokens int64 `json:"input_tokens"` OutputTokens int64 `json:"output_tokens"` diff --git a/pkg/runtime/harness.go b/pkg/runtime/harness.go index c48b380f8..ffc0b9c34 100644 --- a/pkg/runtime/harness.go +++ b/pkg/runtime/harness.go @@ -46,7 +46,8 @@ func (r *LocalRuntime) runHarnessAgent(ctx context.Context, sess *session.Sessio }() turnStartMsgs := r.executeTurnStartHooks(ctx, sess, a, events) - messages := sess.GetMessages(a, append(baseExtra, turnStartMsgs...)...) + planReminder := planModeReminderMessages(sess) + messages := sess.GetMessages(a, append(append(baseExtra, turnStartMsgs...), planReminder...)...) stop, msg, rewritten := r.executeBeforeLLMCallHooks(ctx, sess, a, modelID, 1, messages) if stop { slog.WarnContext(ctx, "before_llm_call hook signalled run termination", diff --git a/pkg/runtime/loop.go b/pkg/runtime/loop.go index 59f23b2d1..11696d4b9 100644 --- a/pkg/runtime/loop.go +++ b/pkg/runtime/loop.go @@ -266,7 +266,7 @@ func (r *LocalRuntime) runStreamLoop(ctx context.Context, sess *session.Session, sink.Emit(ErrorWithCode(ErrorCodeToolFailed, fmt.Sprintf("failed to get tools: %v", err))) return } - agentTools = filterExcludedTools(agentTools, sess.ExcludedTools) + agentTools = filterToolsForSession(agentTools, sess) sink.Emit(ToolsetInfo(len(agentTools), false, a.Name())) @@ -348,7 +348,7 @@ func (r *LocalRuntime) runStreamLoop(ctx context.Context, sess *session.Session, sink.Emit(ErrorWithCode(ErrorCodeToolFailed, fmt.Sprintf("failed to get tools: %v", err))) return } - agentTools = filterExcludedTools(agentTools, sess.ExcludedTools) + agentTools = filterToolsForSession(agentTools, sess) // Emit updated tool count. After a ToolListChanged MCP notification // the cache is invalidated, so getTools above re-fetches from the @@ -554,7 +554,13 @@ func (r *LocalRuntime) runTurn( // files) refresh every turn while session-level context (cwd, OS, // arch) stays stable — all without bloating the stored history. turnStartMsgs := r.executeTurnStartHooks(ctx, sess, a, events) - messages := sess.GetMessages(a, slices.Concat(ls.sessionStartMsgs, ls.userPromptMsgs, turnStartMsgs)...) + // Plan-mode reminder rides alongside the turn_start hook output so it + // participates in the same per-turn splice (and the cache_control marker + // that GetMessages applies to the last extra). It is appended last so its + // instruction is the most recent system context the model sees before the + // user prompt — minimising the chance the model ignores it. + planReminder := planModeReminderMessages(sess) + messages := sess.GetMessages(a, slices.Concat(ls.sessionStartMsgs, ls.userPromptMsgs, turnStartMsgs, planReminder)...) slog.DebugContext(ctx, "Retrieved messages for processing", "agent", a.Name(), "message_count", len(messages)) // before_llm_call hooks fire just before the model is invoked. @@ -990,6 +996,33 @@ func filterExcludedTools(agentTools []tools.Tool, excluded []string) []tools.Too return filtered } +// filterToolsForSession applies all session-level tool filters: the explicit +// ExcludedTools name list (used by skill sub-sessions) and, when the session +// is in plan mode, anything whose tool definition doesn't advertise +// ReadOnlyHint. The MCP spec's ReadOnlyHint is the canonical "this tool has +// no side effects" signal, so it's the right knob for plan mode and it +// extends naturally to user-added MCP tools without any per-tool config. +func filterToolsForSession(agentTools []tools.Tool, sess *session.Session) []tools.Tool { + out := filterExcludedTools(agentTools, sess.ExcludedTools) + if sess.Mode == session.ModePlan { + out = filterToReadOnlyTools(out) + } + return out +} + +// filterToReadOnlyTools keeps only tools whose definition advertises +// ReadOnlyHint. Used by plan mode to hide every write/execute tool from the +// model so it can't reach for them even if the system reminder is ignored. +func filterToReadOnlyTools(agentTools []tools.Tool) []tools.Tool { + filtered := make([]tools.Tool, 0, len(agentTools)) + for _, t := range agentTools { + if t.Annotations.ReadOnlyHint { + filtered = append(filtered, t) + } + } + return filtered +} + // reprobe re-runs ensureToolSetsAreStarted after a batch of tool calls. // If new tools became available (by name-set diff), it emits a ToolsetInfo // event to update the TUI immediately. The new tools will be picked up by @@ -1010,7 +1043,7 @@ func (r *LocalRuntime) reprobe( slog.WarnContext(ctx, "reprobe: getTools failed", "agent", a.Name(), "error", err) return } - updated = filterExcludedTools(updated, sess.ExcludedTools) + updated = filterToolsForSession(updated, sess) // Emit any pending warnings that getTools just generated. r.emitAgentWarnings(a, events) diff --git a/pkg/runtime/plan_mode.go b/pkg/runtime/plan_mode.go new file mode 100644 index 000000000..c0a916b77 --- /dev/null +++ b/pkg/runtime/plan_mode.go @@ -0,0 +1,45 @@ +package runtime + +import ( + "github.com/docker/docker-agent/pkg/chat" + "github.com/docker/docker-agent/pkg/session" +) + +// planModeReminder is the per-turn system instruction injected when a session +// is in plan mode. Two layers enforce plan mode: the runtime hides every +// non-read-only tool from the model (see filterToolsForSession in loop.go), +// and this reminder tells the model how it should behave. Hiding the tools +// is the hard guarantee; the reminder is the explanation, so the model +// produces a useful plan instead of just bouncing off missing tools. +const planModeReminder = ` +You are currently in PLAN MODE. + +In this mode you research the codebase, ask clarifying questions, and write a +clear, actionable plan for the user. You MUST NOT make any changes to the +system: + +- No edits to files (no write, edit, create, or delete). +- No shell commands or background jobs. +- No state-changing tool calls of any kind. + +Only read-only tools have been made available to you for this turn. If you try +to call a tool that isn't in your list, the user has explicitly disabled it +for planning. + +End the turn by presenting the plan in your final message and asking the user +to review it. The user will switch you to BUILD MODE when they want execution +to begin. +` + +// planModeReminderMessages returns the system-reminder messages to splice +// before the conversation history when sess is in plan mode. Returns nil for +// other modes so callers can use it unconditionally. +func planModeReminderMessages(sess *session.Session) []chat.Message { + if sess == nil || sess.Mode != session.ModePlan { + return nil + } + return []chat.Message{{ + Role: chat.MessageRoleSystem, + Content: planModeReminder, + }} +} diff --git a/pkg/runtime/runtime_test.go b/pkg/runtime/runtime_test.go index 86cc3fc62..1203ccf83 100644 --- a/pkg/runtime/runtime_test.go +++ b/pkg/runtime/runtime_test.go @@ -2395,6 +2395,60 @@ func TestFilterExcludedTools(t *testing.T) { }) } +func TestFilterToolsForSession_PlanMode(t *testing.T) { + readOnly := tools.Tool{Name: "read_file", Annotations: tools.ToolAnnotations{ReadOnlyHint: true}} + mutating := tools.Tool{Name: "write_file"} + all := []tools.Tool{readOnly, mutating, {Name: "shell"}} + + t.Run("build mode keeps all tools", func(t *testing.T) { + sess := &session.Session{Mode: session.ModeBuild} + result := filterToolsForSession(all, sess) + assert.Len(t, result, 3) + }) + + t.Run("empty mode is treated as build", func(t *testing.T) { + // Sessions loaded before the mode column existed have Mode == "". + sess := &session.Session{} + result := filterToolsForSession(all, sess) + assert.Len(t, result, 3) + }) + + t.Run("plan mode keeps only read-only tools", func(t *testing.T) { + sess := &session.Session{Mode: session.ModePlan} + result := filterToolsForSession(all, sess) + assert.Len(t, result, 1) + assert.Equal(t, "read_file", result[0].Name) + }) + + t.Run("plan mode still respects ExcludedTools", func(t *testing.T) { + readOnly2 := tools.Tool{Name: "list_directory", Annotations: tools.ToolAnnotations{ReadOnlyHint: true}} + sess := &session.Session{ + Mode: session.ModePlan, + ExcludedTools: []string{"read_file"}, + } + result := filterToolsForSession([]tools.Tool{readOnly, readOnly2, mutating}, sess) + assert.Len(t, result, 1) + assert.Equal(t, "list_directory", result[0].Name) + }) +} + +func TestPlanModeReminderMessages(t *testing.T) { + t.Run("build mode returns nil", func(t *testing.T) { + assert.Nil(t, planModeReminderMessages(&session.Session{Mode: session.ModeBuild})) + }) + + t.Run("nil session returns nil", func(t *testing.T) { + assert.Nil(t, planModeReminderMessages(nil)) + }) + + t.Run("plan mode returns a single system reminder", func(t *testing.T) { + msgs := planModeReminderMessages(&session.Session{Mode: session.ModePlan}) + assert.Len(t, msgs, 1) + assert.Equal(t, chat.MessageRoleSystem, msgs[0].Role) + assert.Contains(t, msgs[0].Content, "PLAN MODE") + }) +} + func TestMergeExcludedTools(t *testing.T) { t.Run("both empty", func(t *testing.T) { assert.Nil(t, mergeExcludedTools(nil, nil)) diff --git a/pkg/server/server.go b/pkg/server/server.go index 257c25faf..84475ae14 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -71,6 +71,7 @@ func (s *Server) registerRoutes() { group.POST("/sessions/:id/resume", s.resumeSession) group.POST("/sessions/:id/tools/toggle", s.toggleSessionYolo) group.PATCH("/sessions/:id/permissions", s.updateSessionPermissions) + group.PATCH("/sessions/:id/mode", s.updateSessionMode) group.PATCH("/sessions/:id/title", s.updateSessionTitle) group.PATCH("/sessions/:id/tokens", s.updateSessionTokens) group.PATCH("/sessions/:id/starred", s.setSessionStarred) @@ -249,6 +250,7 @@ func (s *Server) getSession(c echo.Context) error { OutputTokens: sess.OutputTokens, WorkingDir: sess.WorkingDir, Permissions: sess.Permissions, + Mode: sess.Mode, }) } @@ -329,6 +331,26 @@ func (s *Server) updateSessionPermissions(c echo.Context) error { return c.JSON(http.StatusOK, map[string]string{"message": "session permissions updated"}) } +func (s *Server) updateSessionMode(c echo.Context) error { + sessionID := c.Param("id") + var req api.UpdateSessionModeRequest + if err := c.Bind(&req); err != nil { + return echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("invalid request body: %v", err)) + } + if !req.Mode.IsValid() { + return echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("invalid mode %q; must be one of: %s, %s", req.Mode, session.ModeBuild, session.ModePlan)) + } + + if err := s.sm.UpdateSessionMode(c.Request().Context(), sessionID, req.Mode); err != nil { + return echo.NewHTTPError(http.StatusInternalServerError, fmt.Sprintf("failed to update session mode: %v", err)) + } + + return c.JSON(http.StatusOK, api.UpdateSessionModeResponse{ + ID: sessionID, + Mode: req.Mode, + }) +} + func (s *Server) updateSessionTitle(c echo.Context) error { sessionID := c.Param("id") var req api.UpdateSessionTitleRequest diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index 4bf503544..21146761c 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -199,6 +199,64 @@ func TestServer_UpdateSessionTitle(t *testing.T) { assert.Equal(t, newTitle, sessionResp.Title) } +func TestServer_UpdateSessionMode(t *testing.T) { + t.Parallel() + + ctx := t.Context() + store := session.NewInMemorySessionStore() + lnPath := startServerWithStore(t, ctx, prepareAgentsDir(t), store) + + // Create a session in default (build) mode. + createResp := httpDo(t, ctx, http.MethodPost, lnPath, "/api/sessions", map[string]any{}) + var createdSession session.Session + unmarshal(t, createResp, &createdSession) + require.NotEmpty(t, createdSession.ID) + + // Switch the session into plan mode. + patchResp := httpDo(t, ctx, http.MethodPatch, lnPath, + "/api/sessions/"+createdSession.ID+"/mode", + api.UpdateSessionModeRequest{Mode: session.ModePlan}) + var modeResp api.UpdateSessionModeResponse + unmarshal(t, patchResp, &modeResp) + + assert.Equal(t, createdSession.ID, modeResp.ID) + assert.Equal(t, session.ModePlan, modeResp.Mode) + + // GET should reflect the new mode. + getResp := httpGET(t, ctx, lnPath, "/api/sessions/"+createdSession.ID) + var sessionResp api.SessionResponse + unmarshal(t, getResp, &sessionResp) + assert.Equal(t, session.ModePlan, sessionResp.Mode) + + // Switch back to build mode. + patchResp = httpDo(t, ctx, http.MethodPatch, lnPath, + "/api/sessions/"+createdSession.ID+"/mode", + api.UpdateSessionModeRequest{Mode: session.ModeBuild}) + unmarshal(t, patchResp, &modeResp) + assert.Equal(t, session.ModeBuild, modeResp.Mode) +} + +func TestServer_CreateSession_AcceptsMode(t *testing.T) { + t.Parallel() + + ctx := t.Context() + store := session.NewInMemorySessionStore() + lnPath := startServerWithStore(t, ctx, prepareAgentsDir(t), store) + + // Creating a session with mode=plan should persist that mode. + createResp := httpDo(t, ctx, http.MethodPost, lnPath, "/api/sessions", + map[string]any{"mode": string(session.ModePlan)}) + var createdSession session.Session + unmarshal(t, createResp, &createdSession) + require.NotEmpty(t, createdSession.ID) + assert.Equal(t, session.ModePlan, createdSession.Mode) + + getResp := httpGET(t, ctx, lnPath, "/api/sessions/"+createdSession.ID) + var sessionResp api.SessionResponse + unmarshal(t, getResp, &sessionResp) + assert.Equal(t, session.ModePlan, sessionResp.Mode) +} + func startServerWithStore(t *testing.T, ctx context.Context, agentsDir string, store session.Store) string { t.Helper() diff --git a/pkg/server/session_manager.go b/pkg/server/session_manager.go index ba13671f3..781603a33 100644 --- a/pkg/server/session_manager.go +++ b/pkg/server/session_manager.go @@ -316,6 +316,7 @@ func (sm *SessionManager) GetSessionSnapshot(ctx context.Context, id string) (*a Messages: sess.GetAllMessages(), ToolsApproved: sess.ToolsApproved, Permissions: sess.Permissions, + Mode: sess.Mode, InputTokens: sess.InputTokens, OutputTokens: sess.OutputTokens, Streaming: streaming, @@ -353,6 +354,10 @@ func (sm *SessionManager) CreateSession(ctx context.Context, sessionTemplate *se opts = append(opts, session.WithPermissions(sessionTemplate.Permissions)) } + if sessionTemplate.Mode != "" { + opts = append(opts, session.WithMode(sessionTemplate.Mode)) + } + sess := session.New(opts...) // Copy model-related fields from the template so callers can pin a @@ -741,6 +746,29 @@ func (sm *SessionManager) UpdateSessionPermissions(ctx context.Context, sessionI return sm.sessionStore.UpdateSession(ctx, sess) } +// UpdateSessionMode updates the interaction mode (build/plan) for a session. +// If the session is actively running, it also updates the in-memory session +// object so the next turn's tool filter and plan-mode reminder see the new +// mode without having to round-trip through the store. +func (sm *SessionManager) UpdateSessionMode(ctx context.Context, sessionID string, mode session.Mode) error { + mode = session.NormalizeMode(mode) + sm.mux.Lock() + defer sm.mux.Unlock() + + if rt, ok := sm.runtimeSessions.Load(sessionID); ok && rt.session != nil { + rt.session.Mode = mode + slog.DebugContext(ctx, "Updated mode for active session", "session_id", sessionID, "mode", mode) + return sm.sessionStore.UpdateSession(ctx, rt.session) + } + + sess, err := sm.sessionStore.GetSession(ctx, sessionID) + if err != nil { + return err + } + sess.Mode = mode + return sm.sessionStore.UpdateSession(ctx, sess) +} + // UpdateSessionTitle updates the title for a session. // If the session is actively running, it also updates the in-memory session // object to prevent subsequent runtime saves from overwriting the title. diff --git a/pkg/session/branch.go b/pkg/session/branch.go index 6de7b5ed6..32dc22fa5 100644 --- a/pkg/session/branch.go +++ b/pkg/session/branch.go @@ -76,6 +76,7 @@ func (s *Session) Clone() *Session { CustomModelsUsed: cloneStringSlice(s.CustomModelsUsed), AttachedFiles: cloneStringSlice(s.AttachedFiles), ExcludedTools: cloneStringSlice(s.ExcludedTools), + Mode: s.Mode, AgentName: s.AgentName, ParentID: s.ParentID, MessageUsageHistory: slices.Clone(s.MessageUsageHistory), diff --git a/pkg/session/migrations.go b/pkg/session/migrations.go index 400d41bd1..3cd176964 100644 --- a/pkg/session/migrations.go +++ b/pkg/session/migrations.go @@ -400,6 +400,13 @@ func getAllMigrations() []Migration { Description: "Add first_kept_entry column to session_items for compaction-preserved messages", UpSQL: `ALTER TABLE session_items ADD COLUMN first_kept_entry INTEGER DEFAULT 0`, }, + { + ID: 22, + Name: "022_add_mode_column", + Description: "Add mode column to sessions table for build/plan mode", + UpSQL: `ALTER TABLE sessions ADD COLUMN mode TEXT DEFAULT ''`, + DownSQL: `ALTER TABLE sessions DROP COLUMN mode`, + }, } } diff --git a/pkg/session/migrations_pinned_test.go b/pkg/session/migrations_pinned_test.go index 1ffdff8d0..96ae7c5aa 100644 --- a/pkg/session/migrations_pinned_test.go +++ b/pkg/session/migrations_pinned_test.go @@ -39,7 +39,7 @@ func TestMigrationCatalogIsContentPinned(t *testing.T) { got := digestMigrationCatalog(getAllMigrations()) - const wantDigest = "0c6d5df46b970104cf49988ee3931e33643d5db85c68dd41b74b639d0094cec9" + const wantDigest = "399611d010efb60b9349257e05e0e68702d432e5019cdd56b4ae4e69654ac691" if got != wantDigest { t.Fatalf(`migration catalogue content has changed. diff --git a/pkg/session/session.go b/pkg/session/session.go index fffdb82e5..90b31a93f 100644 --- a/pkg/session/session.go +++ b/pkg/session/session.go @@ -159,6 +159,13 @@ type Session struct { // recursive run_skill calls. ExcludedTools []string `json:"-"` + // Mode is the session's interaction mode. ModeBuild (default) gives the + // agent its full toolset. ModePlan filters the toolset to read-only tools + // and injects a system reminder so the agent drafts a plan instead of + // making changes. The mode can be flipped at any time via + // PATCH /api/sessions/:id/mode; the next turn picks it up. + Mode Mode `json:"mode,omitempty"` + // AgentName, when set, tells RunStream which agent to use for this session // instead of reading from the shared runtime currentAgent field. This is // required for background agent tasks where multiple sessions may run @@ -185,6 +192,41 @@ type MessageUsageRecord struct { Usage chat.Usage `json:"usage"` } +// Mode is the session's interaction mode (build vs plan). +// +// ModeBuild is the default and gives the agent its full toolset. +// ModePlan filters the toolset to read-only tools (anything whose tool +// definition lacks Annotations.ReadOnlyHint) and injects a per-turn system +// reminder telling the agent to plan rather than act. The runtime applies +// both effects automatically based on this field, so callers only need to +// flip the mode — they don't have to compute tool lists themselves. +type Mode string + +const ( + ModeBuild Mode = "build" + ModePlan Mode = "plan" +) + +// IsValid reports whether m is a known mode. +func (m Mode) IsValid() bool { + switch m { + case ModeBuild, ModePlan: + return true + default: + return false + } +} + +// NormalizeMode returns m if it is a known mode, or ModeBuild otherwise. +// Use this when reading mode from external input (persistence, HTTP body) +// to make sure downstream code always sees a valid mode. +func NormalizeMode(m Mode) Mode { + if m.IsValid() { + return m + } + return ModeBuild +} + // PermissionsConfig defines session-level tool permission overrides // using pattern-based rules (Allow/Ask/Deny arrays). type PermissionsConfig struct { @@ -767,6 +809,14 @@ func WithExcludedTools(names []string) Opt { } } +// WithMode sets the session's interaction mode. An empty or unknown mode is +// normalised to ModeBuild so callers can pass through user input directly. +func WithMode(mode Mode) Opt { + return func(s *Session) { + s.Mode = NormalizeMode(mode) + } +} + // WithAttachedFiles seeds the session with absolute paths of files the user // attached. Used when creating sub-sessions so that delegated agents inherit // the parent's file context. Empty and duplicate paths are dropped. diff --git a/pkg/session/store.go b/pkg/session/store.go index 40970c579..a54ac95db 100644 --- a/pkg/session/store.go +++ b/pkg/session/store.go @@ -230,6 +230,7 @@ func (s *InMemorySessionStore) UpdateSession(_ context.Context, session *Session AgentModelOverrides: cloneStringMap(session.AgentModelOverrides), CustomModelsUsed: cloneStringSlice(session.CustomModelsUsed), AttachedFiles: slices.Clone(session.AttachedFiles), + Mode: session.Mode, ParentID: session.ParentID, } session.mu.RUnlock() @@ -354,7 +355,7 @@ type SQLiteSessionStore struct { // sessionSelectColumns is the canonical SELECT list for the sessions table. // The column order matches what scanSession expects; all read paths use this // constant so that adding a column requires updating exactly one place. -const sessionSelectColumns = `id, tools_approved, input_tokens, output_tokens, title, cost, send_user_message, max_iterations, working_dir, created_at, starred, permissions, agent_model_overrides, custom_models_used, thinking, parent_id` +const sessionSelectColumns = `id, tools_approved, input_tokens, output_tokens, title, cost, send_user_message, max_iterations, working_dir, created_at, starred, permissions, agent_model_overrides, custom_models_used, thinking, parent_id, mode` // sessionPersistedFields holds the encoded form of a Session's JSON-bearing // columns plus the SQL representation of parent_id (nil for the empty @@ -595,12 +596,12 @@ func (s *SQLiteSessionStore) AddSession(ctx context.Context, session *Session) e `INSERT INTO sessions ( id, tools_approved, input_tokens, output_tokens, title, cost, send_user_message, max_iterations, working_dir, created_at, permissions, agent_model_overrides, - custom_models_used, thinking, parent_id - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + custom_models_used, thinking, parent_id, mode + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, session.ID, session.ToolsApproved, session.InputTokens, session.OutputTokens, session.Title, session.Cost, session.SendUserMessage, session.MaxIterations, session.WorkingDir, session.CreatedAt.Format(time.RFC3339), fields.PermissionsJSON, fields.AgentModelOverridesJSON, - fields.CustomModelsUsedJSON, false, fields.ParentID) + fields.CustomModelsUsedJSON, false, fields.ParentID, string(session.Mode)) if err != nil { return err } @@ -628,6 +629,7 @@ func scanSession(scanner interface { workingDir sql.NullString permissionsJSON sql.NullString parentID sql.NullString + modeStr sql.NullString agentModelOverridesJSON string customModelsUsedJSON string createdAtStr string @@ -639,6 +641,7 @@ func scanSession(scanner interface { &sess.Title, &sess.Cost, &sess.SendUserMessage, &sess.MaxIterations, &workingDir, &createdAtStr, &sess.Starred, &permissionsJSON, &agentModelOverridesJSON, &customModelsUsedJSON, &thinking, &parentID, + &modeStr, ) if err != nil { return nil, err @@ -651,6 +654,7 @@ func scanSession(scanner interface { sess.WorkingDir = workingDir.String sess.ParentID = parentID.String + sess.Mode = NormalizeMode(Mode(modeStr.String)) if permissionsJSON.Valid && permissionsJSON.String != "" { sess.Permissions = &PermissionsConfig{} @@ -908,9 +912,9 @@ func (s *SQLiteSessionStore) UpdateSession(ctx context.Context, session *Session `INSERT INTO sessions ( id, tools_approved, input_tokens, output_tokens, title, cost, send_user_message, max_iterations, working_dir, created_at, starred, permissions, agent_model_overrides, - custom_models_used, thinking, parent_id + custom_models_used, thinking, parent_id, mode ) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(id) DO UPDATE SET title = excluded.title, tools_approved = excluded.tools_approved, @@ -925,11 +929,12 @@ func (s *SQLiteSessionStore) UpdateSession(ctx context.Context, session *Session agent_model_overrides = excluded.agent_model_overrides, custom_models_used = excluded.custom_models_used, thinking = excluded.thinking, - parent_id = excluded.parent_id`, + parent_id = excluded.parent_id, + mode = excluded.mode`, session.ID, session.ToolsApproved, session.InputTokens, session.OutputTokens, session.Title, session.Cost, session.SendUserMessage, session.MaxIterations, session.WorkingDir, session.CreatedAt.Format(time.RFC3339), session.Starred, fields.PermissionsJSON, fields.AgentModelOverridesJSON, - fields.CustomModelsUsedJSON, false, fields.ParentID) + fields.CustomModelsUsedJSON, false, fields.ParentID, string(session.Mode)) if err != nil { return err } @@ -1076,14 +1081,14 @@ func (s *SQLiteSessionStore) addSessionTx(ctx context.Context, tx *sql.Tx, sessi `INSERT INTO sessions ( id, tools_approved, input_tokens, output_tokens, title, cost, send_user_message, max_iterations, working_dir, created_at, starred, permissions, agent_model_overrides, - custom_models_used, thinking, parent_id + custom_models_used, thinking, parent_id, mode ) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, session.ID, session.ToolsApproved, session.InputTokens, session.OutputTokens, session.Title, session.Cost, session.SendUserMessage, session.MaxIterations, session.WorkingDir, session.CreatedAt.Format(time.RFC3339), session.Starred, fields.PermissionsJSON, fields.AgentModelOverridesJSON, fields.CustomModelsUsedJSON, false, - fields.ParentID) + fields.ParentID, string(session.Mode)) return err } diff --git a/pkg/session/store_test.go b/pkg/session/store_test.go index cef3d0479..2b66673e2 100644 --- a/pkg/session/store_test.go +++ b/pkg/session/store_test.go @@ -542,6 +542,51 @@ func TestUpdateSession_Permissions(t *testing.T) { assert.Equal(t, []string{"dangerous_*"}, retrieved.Permissions.Deny) } +func TestSessionMode_SQLite(t *testing.T) { + tempDB := filepath.Join(t.TempDir(), "test_session_mode.db") + + store, err := NewSQLiteSessionStore(tempDB) + require.NoError(t, err) + defer store.(*SQLiteSessionStore).Close() + + // Default Mode (empty string) round-trips as ModeBuild after scan. + defaultSess := &Session{ + ID: "default-mode-session", + Title: "Default mode", + CreatedAt: time.Now(), + } + require.NoError(t, store.AddSession(t.Context(), defaultSess)) + retrieved, err := store.GetSession(t.Context(), defaultSess.ID) + require.NoError(t, err) + assert.Equal(t, ModeBuild, retrieved.Mode) + + // ModePlan persists and reloads. + planSess := &Session{ + ID: "plan-mode-session", + Title: "Plan mode", + CreatedAt: time.Now(), + Mode: ModePlan, + } + require.NoError(t, store.AddSession(t.Context(), planSess)) + retrieved, err = store.GetSession(t.Context(), planSess.ID) + require.NoError(t, err) + assert.Equal(t, ModePlan, retrieved.Mode) + + // Mode can be flipped via UpdateSession. + planSess.Mode = ModeBuild + require.NoError(t, store.UpdateSession(t.Context(), planSess)) + retrieved, err = store.GetSession(t.Context(), planSess.ID) + require.NoError(t, err) + assert.Equal(t, ModeBuild, retrieved.Mode) +} + +func TestNormalizeMode(t *testing.T) { + assert.Equal(t, ModeBuild, NormalizeMode("")) + assert.Equal(t, ModeBuild, NormalizeMode("garbage")) + assert.Equal(t, ModeBuild, NormalizeMode(ModeBuild)) + assert.Equal(t, ModePlan, NormalizeMode(ModePlan)) +} + func TestAgentModelOverrides_SQLite(t *testing.T) { tempDB := filepath.Join(t.TempDir(), "test_model_overrides.db") From 9896c19f0220bacb9e8cefe232bf0c234641470d Mon Sep 17 00:00:00 2001 From: Trung Nguyen Date: Fri, 19 Jun 2026 10:22:48 +0200 Subject: [PATCH 2/2] feat: let agents declare a plan_persona instruction In plan mode (added in PR #3140), the runtime filters the agent's toolset to read-only tools and injects a per-turn system reminder. For agents whose canonical instruction is heavily tuned for execution ("act now", "never ask clarifying questions", "fix files immediately"), layering a 'plan, don't act' reminder on top leaves two contradictory specs in the same context: the model can still push to an action-oriented conclusion even with the mutating tools hidden. Add an optional plan_persona block on AgentConfig that lets the agent author replace the per-turn plan-mode system reminder with a persona instruction tailored for planning. The runtime wraps the persona in the existing envelope and prefixes a short guardrail line stating that only read-only tools are available, so persona authors own the workflow framing and tone while the read-only contract stays runtime-owned. Agents that don't declare plan_persona fall back to the canned reminder, preserving today's behaviour. agents: root: instruction: | You are an executor. Act now. plan_persona: instruction: | You plan. You do not execute. Iterate with the user. --- agent-schema.json | 15 +++++++ pkg/agent/agent.go | 11 +++++ pkg/agent/agent_test.go | 25 +++++++++++ pkg/agent/opts.go | 9 ++++ pkg/config/latest/types.go | 31 +++++++++++++ pkg/runtime/harness.go | 2 +- pkg/runtime/loop.go | 2 +- pkg/runtime/plan_mode.go | 52 +++++++++++++++++++--- pkg/runtime/runtime_test.go | 53 +++++++++++++++++++++-- pkg/teamloader/teamloader.go | 4 ++ pkg/teamloader/teamloader_test.go | 25 +++++++++++ pkg/teamloader/testdata/plan-persona.yaml | 6 +++ 12 files changed, 222 insertions(+), 13 deletions(-) create mode 100644 pkg/teamloader/testdata/plan-persona.yaml diff --git a/agent-schema.json b/agent-schema.json index ef22f4332..07a30821d 100644 --- a/agent-schema.json +++ b/agent-schema.json @@ -725,6 +725,10 @@ "$ref": "#/definitions/CacheConfig", "description": "Optional response cache: when the same user question is asked again, replay the previous answer instead of calling the model." }, + "plan_persona": { + "$ref": "#/definitions/PlanPersonaConfig", + "description": "Optional per-mode overrides applied while the session is in plan mode. Plan mode already filters the agent's toolset to read-only tools and injects a per-turn system reminder; plan_persona additionally replaces the agent's instruction for the duration of plan mode so the persona's framing matches its restricted toolset. Useful when the agent's normal instruction is heavily tuned for execution and would conflict with the plan-mode reminder." + }, "skills": { "description": "Enable skills discovery for this agent. Set to true to load all discovered skills from local filesystem sources; false disables skills. A list can mix sources (\"local\" or an HTTP/HTTPS URL), skill names to include, and inline skill definitions (objects). If only names are given, local sources are loaded and filtered to just those skills.", "oneOf": [ @@ -1016,6 +1020,17 @@ }, "additionalProperties": false }, + "PlanPersonaConfig": { + "type": "object", + "description": "Per-mode overrides the runtime applies to an agent when the session is in plan mode. Fields are optional; leaving one unset means 'fall back to the agent's normal value'. Plan mode also filters the agent's toolset to read-only tools and prepends a short runtime guardrail so persona authors don't need to repeat the no-edits / no-shell contract.", + "properties": { + "instruction": { + "type": "string", + "description": "Replaces the per-turn plan-mode system reminder while plan mode is active. The runtime wraps the value in a envelope and prefixes a short guardrail line stating that only read-only tools are available, so persona authors own the workflow framing and tone. Empty (or the whole plan_persona block omitted) means 'use the runtime's canned plan-mode reminder unchanged', preserving today's behaviour." + } + }, + "additionalProperties": false + }, "HooksConfig": { "type": "object", "description": "Lifecycle hooks configuration for an agent. Hooks allow running shell commands at various points in the agent's execution lifecycle.", diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index 802024355..44f89d204 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -48,6 +48,10 @@ type Agent struct { harness *latest.HarnessConfig hooks *latest.HooksConfig cache *cache.Cache + // planInstruction is the persona instruction used by the runtime + // when the session is in plan mode. Empty means "use the runtime's + // canned plan-mode reminder". See [latest.PlanPersonaConfig]. + planInstruction string // warningsMu guards pendingWarnings. AddToolWarning and DrainWarnings // may be called concurrently from the runtime loop, the MCP server, @@ -79,6 +83,13 @@ func (a *Agent) Instruction() string { return a.instruction } +// PlanInstruction returns the persona instruction the runtime uses while +// the session is in plan mode. Empty means the agent did not declare a +// plan persona — the runtime falls back to its canned plan-mode reminder. +func (a *Agent) PlanInstruction() string { + return a.planInstruction +} + func (a *Agent) AddDate() bool { return a.addDate } diff --git a/pkg/agent/agent_test.go b/pkg/agent/agent_test.go index 13bd072be..e2d4e3db7 100644 --- a/pkg/agent/agent_test.go +++ b/pkg/agent/agent_test.go @@ -563,3 +563,28 @@ func TestAgentToolsRecoversWhenUnderlyingToolsetDies(t *testing.T) { assert.Equal(t, 1, stub.startCalls) assert.Equal(t, 1, stub.restartCalls) } + +func TestPlanInstruction(t *testing.T) { + t.Run("defaults to empty when no persona is configured", func(t *testing.T) { + a := New("root", "you are an executor") + assert.Empty(t, a.PlanInstruction()) + }) + + t.Run("WithPlanInstruction stores the persona body verbatim", func(t *testing.T) { + persona := "You plan. You do not execute." + a := New("root", "you are an executor", WithPlanInstruction(persona)) + assert.Equal(t, persona, a.PlanInstruction()) + // The agent's normal instruction must remain untouched — the + // persona is a per-mode override applied by the runtime, not a + // replacement of the canonical instruction. + assert.Equal(t, "you are an executor", a.Instruction()) + }) + + t.Run("empty WithPlanInstruction clears any previously stored persona", func(t *testing.T) { + a := New("root", "you are an executor", + WithPlanInstruction("first"), + WithPlanInstruction(""), + ) + assert.Empty(t, a.PlanInstruction()) + }) +} diff --git a/pkg/agent/opts.go b/pkg/agent/opts.go index eb719d9ea..720c232fd 100644 --- a/pkg/agent/opts.go +++ b/pkg/agent/opts.go @@ -18,6 +18,15 @@ func WithInstruction(instruction string) Opt { } } +// WithPlanInstruction sets the persona instruction used by the runtime +// when the session is in plan mode. An empty string clears the persona +// so the runtime falls back to its canned plan-mode reminder. +func WithPlanInstruction(instruction string) Opt { + return func(a *Agent) { + a.planInstruction = instruction + } +} + func WithToolSets(toolSet ...tools.ToolSet) Opt { var startableToolSet []*tools.StartableToolSet for _, ts := range toolSet { diff --git a/pkg/config/latest/types.go b/pkg/config/latest/types.go index b00fc4f13..15662095d 100644 --- a/pkg/config/latest/types.go +++ b/pkg/config/latest/types.go @@ -473,6 +473,37 @@ type AgentConfig struct { UseSkills []string `json:"use_skills,omitempty"` Hooks *HooksConfig `json:"hooks,omitempty"` Cache *CacheConfig `json:"cache,omitempty"` + + // PlanPersona overrides parts of the agent's configuration when the + // session is in plan mode. Plan mode (see session.Mode) already filters + // the agent's toolset to read-only tools and injects a per-turn system + // reminder; PlanPersona lets the agent author additionally replace the + // agent's instruction for the duration of plan mode, so the persona's + // framing matches its restricted toolset. + // + // This is useful when the agent's normal instruction is heavily tuned + // for execution (e.g. "fix files immediately", "never ask clarifying + // questions") and would conflict with the plan-mode reminder otherwise. + // Without a PlanPersona the runtime applies the canned reminder on top + // of the agent's normal instruction, which leaves the two specs in + // tension. + PlanPersona *PlanPersonaConfig `json:"plan_persona,omitempty" yaml:"plan_persona,omitempty"` +} + +// PlanPersonaConfig holds the per-mode overrides the runtime applies to an +// agent when the session is in plan mode. Fields are optional; leaving one +// empty means "fall back to the agent's normal value". +type PlanPersonaConfig struct { + // Instruction replaces the per-turn plan-mode system reminder while + // plan mode is active. The runtime still wraps the value in a + // envelope and prefixes a short guardrail line + // stating that only read-only tools are available, so persona authors + // don't need to repeat the constraint — they own the workflow framing + // and tone. + // + // Empty (or PlanPersona nil) means "use the runtime's canned plan-mode + // reminder unchanged", preserving today's behaviour. + Instruction string `json:"instruction,omitempty" yaml:"instruction,omitempty"` } // CacheConfig configures the agent's response cache. When set and Enabled diff --git a/pkg/runtime/harness.go b/pkg/runtime/harness.go index ffc0b9c34..3db28a19c 100644 --- a/pkg/runtime/harness.go +++ b/pkg/runtime/harness.go @@ -46,7 +46,7 @@ func (r *LocalRuntime) runHarnessAgent(ctx context.Context, sess *session.Sessio }() turnStartMsgs := r.executeTurnStartHooks(ctx, sess, a, events) - planReminder := planModeReminderMessages(sess) + planReminder := planModeReminderMessages(sess, a) messages := sess.GetMessages(a, append(append(baseExtra, turnStartMsgs...), planReminder...)...) stop, msg, rewritten := r.executeBeforeLLMCallHooks(ctx, sess, a, modelID, 1, messages) if stop { diff --git a/pkg/runtime/loop.go b/pkg/runtime/loop.go index 11696d4b9..9ee068b22 100644 --- a/pkg/runtime/loop.go +++ b/pkg/runtime/loop.go @@ -559,7 +559,7 @@ func (r *LocalRuntime) runTurn( // that GetMessages applies to the last extra). It is appended last so its // instruction is the most recent system context the model sees before the // user prompt — minimising the chance the model ignores it. - planReminder := planModeReminderMessages(sess) + planReminder := planModeReminderMessages(sess, a) messages := sess.GetMessages(a, slices.Concat(ls.sessionStartMsgs, ls.userPromptMsgs, turnStartMsgs, planReminder)...) slog.DebugContext(ctx, "Retrieved messages for processing", "agent", a.Name(), "message_count", len(messages)) diff --git a/pkg/runtime/plan_mode.go b/pkg/runtime/plan_mode.go index c0a916b77..b4b956949 100644 --- a/pkg/runtime/plan_mode.go +++ b/pkg/runtime/plan_mode.go @@ -1,16 +1,20 @@ package runtime import ( + "strings" + + "github.com/docker/docker-agent/pkg/agent" "github.com/docker/docker-agent/pkg/chat" "github.com/docker/docker-agent/pkg/session" ) // planModeReminder is the per-turn system instruction injected when a session -// is in plan mode. Two layers enforce plan mode: the runtime hides every -// non-read-only tool from the model (see filterToolsForSession in loop.go), -// and this reminder tells the model how it should behave. Hiding the tools -// is the hard guarantee; the reminder is the explanation, so the model -// produces a useful plan instead of just bouncing off missing tools. +// is in plan mode and the active agent has not declared a plan persona. Two +// layers enforce plan mode: the runtime hides every non-read-only tool from +// the model (see filterToolsForSession in loop.go), and this reminder tells +// the model how it should behave. Hiding the tools is the hard guarantee; the +// reminder is the explanation, so the model produces a useful plan instead +// of just bouncing off missing tools. const planModeReminder = ` You are currently in PLAN MODE. @@ -31,15 +35,49 @@ to review it. The user will switch you to BUILD MODE when they want execution to begin. ` +// planPersonaGuardrail is the short preamble the runtime prepends to a +// declared plan persona. The persona owns the workflow framing and tone; the +// guardrail owns the read-only contract so persona authors don't have to +// repeat it (and can't accidentally drop it). +const planPersonaGuardrail = `You are currently in PLAN MODE. Only read-only tools have been made available to you for this turn; every state-changing tool has been filtered out by the runtime.` + // planModeReminderMessages returns the system-reminder messages to splice // before the conversation history when sess is in plan mode. Returns nil for // other modes so callers can use it unconditionally. -func planModeReminderMessages(sess *session.Session) []chat.Message { +// +// When the active agent has declared a plan persona (see +// [latest.PlanPersonaConfig]), the persona's instruction is wrapped in a +// envelope and prefixed with [planPersonaGuardrail] so +// persona authors own the workflow framing while the runtime keeps the +// read-only contract intact. When no persona is declared, the canned +// [planModeReminder] is used unchanged — preserving today's behaviour for +// agents that haven't opted in. +func planModeReminderMessages(sess *session.Session, a *agent.Agent) []chat.Message { if sess == nil || sess.Mode != session.ModePlan { return nil } return []chat.Message{{ Role: chat.MessageRoleSystem, - Content: planModeReminder, + Content: planModeReminderContent(a), }} } + +// planModeReminderContent picks the right reminder body for the active +// agent: the agent's declared plan persona (wrapped in the runtime's +// guardrail envelope) when set, otherwise the canned reminder. +func planModeReminderContent(a *agent.Agent) string { + if a == nil { + return planModeReminder + } + persona := strings.TrimSpace(a.PlanInstruction()) + if persona == "" { + return planModeReminder + } + var b strings.Builder + b.WriteString("\n") + b.WriteString(planPersonaGuardrail) + b.WriteString("\n\n") + b.WriteString(persona) + b.WriteString("\n") + return b.String() +} diff --git a/pkg/runtime/runtime_test.go b/pkg/runtime/runtime_test.go index 1203ccf83..bb96befca 100644 --- a/pkg/runtime/runtime_test.go +++ b/pkg/runtime/runtime_test.go @@ -2433,19 +2433,64 @@ func TestFilterToolsForSession_PlanMode(t *testing.T) { } func TestPlanModeReminderMessages(t *testing.T) { + canned := agent.New("canned", "you are an executor") + t.Run("build mode returns nil", func(t *testing.T) { - assert.Nil(t, planModeReminderMessages(&session.Session{Mode: session.ModeBuild})) + assert.Nil(t, planModeReminderMessages(&session.Session{Mode: session.ModeBuild}, canned)) }) t.Run("nil session returns nil", func(t *testing.T) { - assert.Nil(t, planModeReminderMessages(nil)) + assert.Nil(t, planModeReminderMessages(nil, canned)) }) - t.Run("plan mode returns a single system reminder", func(t *testing.T) { - msgs := planModeReminderMessages(&session.Session{Mode: session.ModePlan}) + t.Run("plan mode returns the canned reminder when the agent has no persona", func(t *testing.T) { + msgs := planModeReminderMessages(&session.Session{Mode: session.ModePlan}, canned) assert.Len(t, msgs, 1) assert.Equal(t, chat.MessageRoleSystem, msgs[0].Role) assert.Contains(t, msgs[0].Content, "PLAN MODE") + // The canned reminder spells out the no-edits / no-shell rules + // itself; the persona path doesn't. + assert.Contains(t, msgs[0].Content, "No edits to files") + }) + + t.Run("plan mode uses the agent's persona when set", func(t *testing.T) { + personaText := "You are a planning specialist. Ask questions. Write a numbered plan." + withPersona := agent.New( + "with-persona", + "you are an executor", + agent.WithPlanInstruction(personaText), + ) + msgs := planModeReminderMessages(&session.Session{Mode: session.ModePlan}, withPersona) + require.Len(t, msgs, 1) + // Persona body must be present. + assert.Contains(t, msgs[0].Content, personaText) + // Runtime guardrail must be prepended so persona authors don't + // have to (and can't accidentally drop) the read-only contract. + assert.Contains(t, msgs[0].Content, "Only read-only tools have been made available") + // And the body must live inside a envelope so + // it visually matches the canned path. + assert.True(t, strings.HasPrefix(msgs[0].Content, "")) + assert.True(t, strings.HasSuffix(msgs[0].Content, "")) + // The canned no-edits enumeration must NOT leak in — the persona + // owns the framing once set. + assert.NotContains(t, msgs[0].Content, "No shell commands or background jobs") + }) + + t.Run("plan mode falls back to canned reminder when persona is whitespace only", func(t *testing.T) { + blank := agent.New( + "blank-persona", + "you are an executor", + agent.WithPlanInstruction(" \n\t "), + ) + msgs := planModeReminderMessages(&session.Session{Mode: session.ModePlan}, blank) + require.Len(t, msgs, 1) + assert.Contains(t, msgs[0].Content, "No edits to files") + }) + + t.Run("plan mode tolerates a nil agent", func(t *testing.T) { + msgs := planModeReminderMessages(&session.Session{Mode: session.ModePlan}, nil) + require.Len(t, msgs, 1) + assert.Contains(t, msgs[0].Content, "No edits to files") }) } diff --git a/pkg/teamloader/teamloader.go b/pkg/teamloader/teamloader.go index 3465c7e57..6e01260c2 100644 --- a/pkg/teamloader/teamloader.go +++ b/pkg/teamloader/teamloader.go @@ -258,6 +258,10 @@ func LoadWithConfig(ctx context.Context, agentSource config.Source, runConfig *c opts = append(opts, agent.WithToolSets(agentTools...)) + if persona := agentConfig.PlanPersona; persona != nil && persona.Instruction != "" { + opts = append(opts, agent.WithPlanInstruction(expander.Expand(ctx, persona.Instruction, nil))) + } + ag := agent.New(agentConfig.Name, expander.Expand(ctx, agentConfig.Instruction, nil), opts...) agents = append(agents, ag) agentsByName[agentConfig.Name] = ag diff --git a/pkg/teamloader/teamloader_test.go b/pkg/teamloader/teamloader_test.go index 840dfc3c7..e66f527f6 100644 --- a/pkg/teamloader/teamloader_test.go +++ b/pkg/teamloader/teamloader_test.go @@ -327,6 +327,31 @@ func TestToolsetInstructions(t *testing.T) { require.Equal(t, expected, instructions) } +// TestPlanPersonaInstruction verifies that agents..plan_persona.instruction +// is loaded onto the resulting *agent.Agent so the runtime can pick it up when +// the session enters plan mode. ${env.X} placeholders are expanded the same way +// as the canonical instruction. +func TestPlanPersonaInstruction(t *testing.T) { + t.Setenv("OPENAI_API_KEY", "dummy") + t.Setenv("USER", "alice") + + agentSource, err := config.Resolve("testdata/plan-persona.yaml", nil) + require.NoError(t, err) + + team, err := Load(t.Context(), agentSource, &config.RuntimeConfig{}) + require.NoError(t, err) + + rootAgent, err := team.Agent("root") + require.NoError(t, err) + + // The canonical (build-mode) instruction must remain unchanged: the + // persona is a per-mode override, not a replacement. + assert.Equal(t, "You are an executor. Act now.", rootAgent.Instruction()) + + // The plan persona must be loaded with ${env.X} placeholders expanded. + assert.Equal(t, "Hello alice, you plan. You do not execute.", rootAgent.PlanInstruction()) +} + // TestInstructionExpansion verifies that ${env.X} placeholders are expanded // at load time both in agent.instruction and in toolsets[*].instruction. // See https://github.com/docker/docker-agent/issues/2614. diff --git a/pkg/teamloader/testdata/plan-persona.yaml b/pkg/teamloader/testdata/plan-persona.yaml new file mode 100644 index 000000000..1acbced2b --- /dev/null +++ b/pkg/teamloader/testdata/plan-persona.yaml @@ -0,0 +1,6 @@ +agents: + root: + model: openai/gpt-4o + instruction: "You are an executor. Act now." + plan_persona: + instruction: "Hello ${env.USER}, you plan. You do not execute."