fix: handle parked task sessions (#1711)
This commit is contained in:
@@ -479,7 +479,25 @@ func resolveScriptCommand(
|
||||
}
|
||||
}
|
||||
|
||||
// httpSetSessionMode sets the session mode for a running agent.
|
||||
func (h *ProcessHandlers) applyIfSessionReady(sessionID string, allowPassthrough bool, apply func() error) error {
|
||||
execution, running := h.lifecycleMgr.GetExecutionBySessionID(sessionID)
|
||||
if !running {
|
||||
return nil
|
||||
}
|
||||
if !h.lifecycleMgr.WasSessionInitialized(execution.ID) &&
|
||||
(!allowPassthrough || execution.PassthroughProcessID == "") {
|
||||
return nil
|
||||
}
|
||||
if err := apply(); err != nil {
|
||||
if _, stillRunning := h.lifecycleMgr.GetExecutionBySessionID(sessionID); !stillRunning {
|
||||
return nil
|
||||
}
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// httpSetSessionMode applies the session mode now or records it for the next launch.
|
||||
func (h *ProcessHandlers) httpSetSessionMode(c *gin.Context) {
|
||||
sessionID := c.Param("id")
|
||||
var req struct {
|
||||
@@ -489,7 +507,9 @@ func (h *ProcessHandlers) httpSetSessionMode(c *gin.Context) {
|
||||
c.JSON(400, gin.H{"error": "invalid request: " + err.Error()})
|
||||
return
|
||||
}
|
||||
if err := h.lifecycleMgr.SetSessionModeBySessionID(c.Request.Context(), sessionID, req.ModeID); err != nil {
|
||||
if err := h.applyIfSessionReady(sessionID, false, func() error {
|
||||
return h.lifecycleMgr.SetSessionModeBySessionID(c.Request.Context(), sessionID, req.ModeID)
|
||||
}); err != nil {
|
||||
h.logger.Error("failed to set session mode",
|
||||
zap.String("session_id", sessionID),
|
||||
zap.String("mode_id", req.ModeID),
|
||||
@@ -509,7 +529,7 @@ func (h *ProcessHandlers) httpSetSessionMode(c *gin.Context) {
|
||||
c.JSON(200, gin.H{"ok": true})
|
||||
}
|
||||
|
||||
// httpSetSessionModel sets the session model for a running agent.
|
||||
// httpSetSessionModel applies the session model now or records it for the next launch.
|
||||
func (h *ProcessHandlers) httpSetSessionModel(c *gin.Context) {
|
||||
sessionID := c.Param("id")
|
||||
var req struct {
|
||||
@@ -519,7 +539,9 @@ func (h *ProcessHandlers) httpSetSessionModel(c *gin.Context) {
|
||||
c.JSON(400, gin.H{"error": "invalid request: " + err.Error()})
|
||||
return
|
||||
}
|
||||
if err := h.lifecycleMgr.SetSessionModelBySessionID(c.Request.Context(), sessionID, req.ModelID); err != nil {
|
||||
if err := h.applyIfSessionReady(sessionID, true, func() error {
|
||||
return h.lifecycleMgr.SetSessionModelBySessionID(c.Request.Context(), sessionID, req.ModelID)
|
||||
}); err != nil {
|
||||
h.logger.Error("failed to set session model",
|
||||
zap.String("session_id", sessionID),
|
||||
zap.String("model_id", req.ModelID),
|
||||
@@ -539,7 +561,7 @@ func (h *ProcessHandlers) httpSetSessionModel(c *gin.Context) {
|
||||
c.JSON(200, gin.H{"ok": true})
|
||||
}
|
||||
|
||||
// httpSetSessionConfigOption sets a session config option for a running agent.
|
||||
// httpSetSessionConfigOption applies an option now or records it for the next launch.
|
||||
func (h *ProcessHandlers) httpSetSessionConfigOption(c *gin.Context) {
|
||||
sessionID := c.Param("id")
|
||||
var req struct {
|
||||
@@ -550,7 +572,9 @@ func (h *ProcessHandlers) httpSetSessionConfigOption(c *gin.Context) {
|
||||
c.JSON(400, gin.H{"error": "invalid request: " + err.Error()})
|
||||
return
|
||||
}
|
||||
if err := h.lifecycleMgr.SetSessionConfigOptionBySessionID(c.Request.Context(), sessionID, req.ConfigID, req.Value); err != nil {
|
||||
if err := h.applyIfSessionReady(sessionID, false, func() error {
|
||||
return h.lifecycleMgr.SetSessionConfigOptionBySessionID(c.Request.Context(), sessionID, req.ConfigID, req.Value)
|
||||
}); err != nil {
|
||||
h.logger.Error("failed to set session config option",
|
||||
zap.String("session_id", sessionID),
|
||||
zap.String("config_id", req.ConfigID),
|
||||
|
||||
@@ -523,6 +523,16 @@ func (m *mockRepository) UpdateSessionMetadata(ctx context.Context, sessionID st
|
||||
return nil
|
||||
}
|
||||
func (m *mockRepository) SetSessionMetadataKey(ctx context.Context, sessionID, key string, value interface{}) error {
|
||||
if session := m.sessions[sessionID]; session != nil {
|
||||
if session.Metadata == nil {
|
||||
session.Metadata = make(map[string]interface{})
|
||||
}
|
||||
if value == nil {
|
||||
delete(session.Metadata, key)
|
||||
} else {
|
||||
session.Metadata[key] = value
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
func (m *mockRepository) DismissLastAgentError(_ context.Context, _ string, _ models.LastAgentError, _ time.Time) (bool, error) {
|
||||
@@ -705,6 +715,101 @@ func TestResolveScriptCommandErrors(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetSessionRuntimeConfigPersistsWithoutRunningAgent(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
path string
|
||||
body string
|
||||
assertions func(*testing.T, models.SessionRuntimeConfig, map[string]interface{})
|
||||
}{
|
||||
{
|
||||
name: "model",
|
||||
path: "/api/v1/task-sessions/session-idle/set-model",
|
||||
body: `{"model_id":"claude-opus-4-8"}`,
|
||||
assertions: func(t *testing.T, cfg models.SessionRuntimeConfig, _ map[string]interface{}) {
|
||||
t.Helper()
|
||||
if cfg.Model != "claude-opus-4-8" {
|
||||
t.Fatalf("expected persisted model, got %q", cfg.Model)
|
||||
}
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "dynamic model option",
|
||||
path: "/api/v1/task-sessions/session-idle/set-config-option",
|
||||
body: `{"config_id":"model","value":"claude-opus-4-8"}`,
|
||||
assertions: func(t *testing.T, cfg models.SessionRuntimeConfig, _ map[string]interface{}) {
|
||||
t.Helper()
|
||||
if cfg.Model != "claude-opus-4-8" || cfg.ConfigOptions["model"] != "claude-opus-4-8" {
|
||||
t.Fatalf("expected persisted dynamic model option, got %+v", cfg)
|
||||
}
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "mode",
|
||||
path: "/api/v1/task-sessions/session-idle/set-mode",
|
||||
body: `{"mode_id":"acceptEdits"}`,
|
||||
assertions: func(t *testing.T, cfg models.SessionRuntimeConfig, metadata map[string]interface{}) {
|
||||
t.Helper()
|
||||
if cfg.Mode != "acceptEdits" || metadata[models.SessionMetaKeySessionMode] != "acceptEdits" {
|
||||
t.Fatalf("expected persisted mode, got config=%+v metadata=%+v", cfg, metadata)
|
||||
}
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
log, err := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json"})
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create logger: %v", err)
|
||||
}
|
||||
session := &models.TaskSession{
|
||||
ID: "session-idle",
|
||||
TaskID: "task-1",
|
||||
State: models.TaskSessionStateIdle,
|
||||
Metadata: make(map[string]interface{}),
|
||||
}
|
||||
repo := &mockRepository{sessions: map[string]*models.TaskSession{session.ID: session}}
|
||||
svc := service.NewService(service.Repos{
|
||||
Workspaces: repo, Tasks: repo, TaskRepos: repo,
|
||||
Workflows: repo, Messages: repo, Turns: repo,
|
||||
Sessions: repo, GitSnapshots: repo, RepoEntities: repo,
|
||||
Executors: repo, Environments: repo, TaskEnvironments: repo,
|
||||
Reviews: repo,
|
||||
}, nil, log, service.RepositoryDiscoveryConfig{})
|
||||
|
||||
lifecycleMgr := newLifecycleManager(t, log)
|
||||
workspaceOnlyExecution := (&lifecycle.ExecutorInstance{
|
||||
InstanceID: "execution-workspace-only",
|
||||
TaskID: session.TaskID,
|
||||
SessionID: session.ID,
|
||||
}).ToAgentExecution(&lifecycle.ExecutorCreateRequest{
|
||||
InstanceID: "execution-workspace-only",
|
||||
TaskID: session.TaskID,
|
||||
SessionID: session.ID,
|
||||
AgentProfileID: "profile-1",
|
||||
WorkspacePath: "/tmp",
|
||||
})
|
||||
addExecution(t, lifecycleMgr, workspaceOnlyExecution)
|
||||
|
||||
router := newProcessStopRouter(t, svc, lifecycleMgr, log)
|
||||
req := httptest.NewRequest(http.MethodPost, tt.path, strings.NewReader(tt.body))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
resp := httptest.NewRecorder()
|
||||
router.ServeHTTP(resp, req)
|
||||
|
||||
if resp.Code != http.StatusOK {
|
||||
t.Fatalf("expected idle session update to succeed, got %d: %s", resp.Code, resp.Body.String())
|
||||
}
|
||||
cfg, ok := models.LoadSessionRuntimeConfig(session.Metadata)
|
||||
if !ok {
|
||||
t.Fatalf("expected persisted runtime config, got %+v", session.Metadata)
|
||||
}
|
||||
tt.assertions(t, cfg, session.Metadata)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestStopProcessRejectsDifferentSession(t *testing.T) {
|
||||
log, err := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json"})
|
||||
if err != nil {
|
||||
|
||||
@@ -1,10 +1,11 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { Task } from "@/lib/types/http";
|
||||
import {
|
||||
buildDebugEntries,
|
||||
hasResolvedTaskDetails,
|
||||
resolveTaskContentState,
|
||||
resolveTaskProps,
|
||||
syncActiveTaskSession,
|
||||
} from "./task-page-content-helpers";
|
||||
|
||||
function baseParams(overrides: Partial<Parameters<typeof buildDebugEntries>[0]> = {}) {
|
||||
@@ -150,3 +151,37 @@ describe("hasResolvedTaskDetails", () => {
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("syncActiveTaskSession", () => {
|
||||
it("restores the initial session without creating a user pin", () => {
|
||||
const setActiveSessionAuto = vi.fn();
|
||||
const setActiveTask = vi.fn();
|
||||
|
||||
syncActiveTaskSession({
|
||||
initialTaskId: "task-1",
|
||||
fallbackTaskId: null,
|
||||
initialSessionId: "session-1",
|
||||
setActiveSessionAuto,
|
||||
setActiveTask,
|
||||
});
|
||||
|
||||
expect(setActiveSessionAuto).toHaveBeenCalledWith("task-1", "session-1");
|
||||
expect(setActiveTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("falls back to selecting the task when there is no initial session", () => {
|
||||
const setActiveSessionAuto = vi.fn();
|
||||
const setActiveTask = vi.fn();
|
||||
|
||||
syncActiveTaskSession({
|
||||
initialTaskId: "task-1",
|
||||
fallbackTaskId: null,
|
||||
initialSessionId: null,
|
||||
setActiveSessionAuto,
|
||||
setActiveTask,
|
||||
});
|
||||
|
||||
expect(setActiveTask).toHaveBeenCalledWith("task-1");
|
||||
expect(setActiveSessionAuto).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -127,6 +127,19 @@ export function hasResolvedTaskDetails(params: {
|
||||
);
|
||||
}
|
||||
|
||||
export function syncActiveTaskSession(params: {
|
||||
initialTaskId: string | undefined;
|
||||
fallbackTaskId: string | null | undefined;
|
||||
initialSessionId: string | null;
|
||||
setActiveSessionAuto: (taskId: string, sessionId: string) => void;
|
||||
setActiveTask: (taskId: string) => void;
|
||||
}) {
|
||||
const taskId = params.initialTaskId ?? params.fallbackTaskId;
|
||||
if (!taskId) return;
|
||||
if (params.initialSessionId) params.setActiveSessionAuto(taskId, params.initialSessionId);
|
||||
else params.setActiveTask(taskId);
|
||||
}
|
||||
|
||||
export function resolveTaskIds(task: Task | null) {
|
||||
return {
|
||||
taskId: task?.id ?? null,
|
||||
|
||||
@@ -28,6 +28,7 @@ import {
|
||||
buildArchivedValue,
|
||||
hasResolvedTaskDetails,
|
||||
resolveTaskContentState,
|
||||
syncActiveTaskSession,
|
||||
} from "@/components/task/task-page-content-helpers";
|
||||
import { TaskPageInner } from "@/components/task/task-page-inner";
|
||||
import { GridSpinner } from "@/components/grid-spinner";
|
||||
@@ -193,19 +194,6 @@ export function useMergedAgentState(
|
||||
return { isResuming, isResumed, taskSessionState, worktreePath, worktreeBranch, isAgentWorking };
|
||||
}
|
||||
|
||||
function syncActiveTaskSession(params: {
|
||||
initialTaskId: string | undefined;
|
||||
fallbackTaskId: string | null | undefined;
|
||||
initialSessionId: string | null;
|
||||
setActiveSession: (taskId: string, sessionId: string) => void;
|
||||
setActiveTask: (taskId: string) => void;
|
||||
}) {
|
||||
const taskId = params.initialTaskId ?? params.fallbackTaskId;
|
||||
if (!taskId) return;
|
||||
if (params.initialSessionId) params.setActiveSession(taskId, params.initialSessionId);
|
||||
else params.setActiveTask(taskId);
|
||||
}
|
||||
|
||||
function TaskLoadingState() {
|
||||
return (
|
||||
<div
|
||||
@@ -301,7 +289,7 @@ function useTaskPageData(
|
||||
initialRepositories: Repository[],
|
||||
) {
|
||||
const activeTaskId = useAppStore((state) => state.tasks.activeTaskId);
|
||||
const setActiveSession = useAppStore((state) => state.setActiveSession);
|
||||
const setActiveSessionAuto = useAppStore((state) => state.setActiveSessionAuto);
|
||||
const setActiveTask = useAppStore((state) => state.setActiveTask);
|
||||
|
||||
// Validate that activeSessionId belongs to activeTaskId to prevent showing
|
||||
@@ -325,10 +313,10 @@ function useTaskPageData(
|
||||
initialTaskId: initialTask?.id,
|
||||
fallbackTaskId,
|
||||
initialSessionId,
|
||||
setActiveSession,
|
||||
setActiveSessionAuto,
|
||||
setActiveTask,
|
||||
});
|
||||
}, [initialTask?.id, fallbackTaskId, initialSessionId, setActiveSession, setActiveTask]);
|
||||
}, [initialTask?.id, fallbackTaskId, initialSessionId, setActiveSessionAuto, setActiveTask]);
|
||||
|
||||
const { repositories } = useRepositories(task?.workspace_id ?? null, Boolean(task?.workspace_id));
|
||||
const effectiveRepositories = repositories.length ? repositories : initialRepositories;
|
||||
|
||||
@@ -53,4 +53,18 @@ describe("kanban slice active session selection", () => {
|
||||
lastSessionByTaskId: { [TASK_ID]: AUTO_SESSION_ID },
|
||||
});
|
||||
});
|
||||
|
||||
it("clears a pin when auto-selecting a session for a different task", () => {
|
||||
const store = makeStore();
|
||||
|
||||
store.getState().setActiveSession(TASK_ID, PINNED_SESSION_ID);
|
||||
store.getState().setActiveSessionAuto("task-2", AUTO_SESSION_ID);
|
||||
|
||||
expect(store.getState().tasks).toMatchObject({
|
||||
activeTaskId: "task-2",
|
||||
activeSessionId: AUTO_SESSION_ID,
|
||||
pinnedSessionId: null,
|
||||
lastSessionByTaskId: { [TASK_ID]: PINNED_SESSION_ID, "task-2": AUTO_SESSION_ID },
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -58,6 +58,9 @@ export const createKanbanSlice: StateCreator<
|
||||
}),
|
||||
setActiveSessionAuto: (taskId, sessionId) =>
|
||||
set((draft) => {
|
||||
if (draft.tasks.activeTaskId !== taskId) {
|
||||
draft.tasks.pinnedSessionId = null;
|
||||
}
|
||||
draft.tasks.activeTaskId = taskId;
|
||||
draft.tasks.activeSessionId = sessionId;
|
||||
draft.tasks.lastSessionByTaskId[taskId] = sessionId;
|
||||
|
||||
@@ -111,6 +111,21 @@ describe("shouldAdoptNewSession", () => {
|
||||
expect(shouldAdoptNewSession(state, "t-1", "STARTING")).toBe(true);
|
||||
});
|
||||
|
||||
it("adopts when the active session is parked idle", () => {
|
||||
const state = makeAppState({
|
||||
tasks: {
|
||||
activeTaskId: "t-1",
|
||||
activeSessionId: "s-old",
|
||||
pinnedSessionId: null,
|
||||
lastSessionByTaskId: {},
|
||||
},
|
||||
taskSessions: {
|
||||
items: { "s-old": { id: "s-old", task_id: "t-1", state: "IDLE" } },
|
||||
} as unknown as AppState["taskSessions"],
|
||||
});
|
||||
expect(shouldAdoptNewSession(state, "t-1", "STARTING")).toBe(true);
|
||||
});
|
||||
|
||||
it("does NOT adopt while the current active session is still running", () => {
|
||||
const state = makeAppState({
|
||||
tasks: {
|
||||
@@ -187,6 +202,22 @@ describe("pickReplacementSessionId", () => {
|
||||
});
|
||||
|
||||
describe("shouldPreservePinnedSessionForTask", () => {
|
||||
it("preserves a pinned idle session for the active task", () => {
|
||||
const state = makeAppState({
|
||||
tasks: {
|
||||
activeTaskId: "t-1",
|
||||
activeSessionId: "s-pinned",
|
||||
pinnedSessionId: "s-pinned",
|
||||
lastSessionByTaskId: {},
|
||||
},
|
||||
taskSessions: {
|
||||
items: { "s-pinned": { id: "s-pinned", task_id: "t-1", state: "IDLE" } },
|
||||
} as unknown as AppState["taskSessions"],
|
||||
});
|
||||
|
||||
expect(shouldPreservePinnedSessionForTask(state, "t-1")).toBe(true);
|
||||
});
|
||||
|
||||
it("preserves a missing pinned session while the per-task list is hydrating", () => {
|
||||
const state = makeAppState({
|
||||
tasks: {
|
||||
|
||||
@@ -112,10 +112,10 @@ function maybePromoteAgentctlReady(
|
||||
* follow the switch. Returns true when the caller should adopt the new session
|
||||
* as the task's active session.
|
||||
*
|
||||
* Adopts only when the current active session is missing, cross-task, or
|
||||
* already terminal — not while a live session for the same task is still
|
||||
* running (the backend only creates sessions during workflow step transitions
|
||||
* after stopping the previous one, but WS events may arrive out of order).
|
||||
* Adopts only when the current active session is missing, cross-task, parked
|
||||
* IDLE, or already terminal — not while a live session for the same task is
|
||||
* still running. A parked IDLE session has no live process, so a newly started
|
||||
* workflow session should replace it just like a terminal session.
|
||||
*/
|
||||
export function shouldAdoptNewSession(
|
||||
state: AppState,
|
||||
@@ -127,7 +127,11 @@ export function shouldAdoptNewSession(
|
||||
const activeSessionId = state.tasks.activeSessionId;
|
||||
if (activeSessionId) {
|
||||
const activeSession = state.taskSessions.items[activeSessionId];
|
||||
if (activeSession?.task_id === taskId && !isTerminalSessionState(activeSession.state)) {
|
||||
if (
|
||||
activeSession?.task_id === taskId &&
|
||||
activeSession.state !== "IDLE" &&
|
||||
!isTerminalSessionState(activeSession.state)
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -365,6 +365,17 @@ preserved.
|
||||
binds to `task_sessions.current_execution_id` and shows live state.
|
||||
When the execution ends, the dockview renders dormant.
|
||||
|
||||
- **GIVEN** advanced mode is showing an unpinned `IDLE` session,
|
||||
**WHEN** a workflow transition starts a different session for the
|
||||
same task, **THEN** the dockview selects the newly started session.
|
||||
An explicitly selected live session remains selected.
|
||||
|
||||
- **GIVEN** advanced mode is showing a parked `IDLE` session, **WHEN**
|
||||
the user changes its model, mode, or dynamic runtime option before
|
||||
sending the next message, **THEN** the selection is persisted and
|
||||
applied when the agent process resumes. A running session applies the
|
||||
same selection immediately and persists it for future recovery.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Per-workspace strategy lock UI. The unified model serves mixed
|
||||
|
||||
Reference in New Issue
Block a user