fix(acp): preserve model across context reset (#2079)

This commit is contained in:
Carlos Florêncio
2026-07-30 22:40:45 +01:00
committed by GitHub
parent 3760f12939
commit 5e486fccf9
7 changed files with 435 additions and 7 deletions
@@ -405,6 +405,44 @@ func (m *Manager) reapplySessionModeAfterReset(ctx context.Context, execution *A
zap.String("mode", mode))
}
// reapplySessionModelAfterReset re-applies the effective model to a freshly
// initialized ACP session so a context reset does not replace the task's model
// with the provider default advertised by the new session.
func (m *Manager) reapplySessionModelAfterReset(
ctx context.Context,
execution *AgentExecution,
newSessionID, modelID string,
) {
if execution.agentctl == nil || modelID == "" {
return
}
if err := execution.agentctl.SetModel(ctx, modelID); err != nil {
m.logger.Warn("failed to re-apply session model after context reset",
zap.String("execution_id", execution.ID),
zap.String("model", modelID),
zap.Error(err))
return
}
m.logger.Info("re-applied session model after context reset",
zap.String("execution_id", execution.ID),
zap.String("session_id", execution.SessionID),
zap.String("new_acp_session_id", newSessionID),
zap.String("model", modelID))
}
func (m *Manager) effectiveSessionModelForReset(ctx context.Context, execution *AgentExecution) string {
modelFallback := ""
if previous := execution.GetModelState(); previous != nil {
modelFallback = previous.CurrentModelID
}
if modelFallback == "" {
profileModel, _, _ := m.resolveProfileSessionConfig(ctx, execution.AgentProfileID)
modelFallback = profileModel
}
model, _, _ := m.effectiveSessionRuntimeConfig(ctx, execution, modelFallback, "", nil)
return model
}
// ResetAgentContext resets the agent's conversation context. For ACP agents that support
// session reset, this creates a new session on the existing connection (fast, no process restart).
// For all other agents, this falls back to RestartAgentProcess (full subprocess restart).
@@ -423,10 +461,11 @@ func (m *Manager) ResetAgentContext(ctx context.Context, executionID string) err
return fmt.Errorf("execution %q has no agentctl client", executionID)
}
// Capture the active session mode before the reset so it can be re-applied to
// the fresh ACP session (issue #1183). The agent's new session starts at its
// default mode, so without this the user's chosen mode is lost.
// Capture active session state before the reset. The fresh ACP session starts
// at provider defaults, so asynchronous model/mode events from it must not
// replace the task's effective pre-reset configuration.
prevMode := execution.GetModeState()
effectiveModel := m.effectiveSessionModelForReset(ctx, execution)
// Resolve agent config and MCP servers for session reset
agentConfig, err := m.getAgentConfigForExecution(execution)
@@ -467,7 +506,9 @@ func (m *Manager) ResetAgentContext(ctx context.Context, executionID string) err
}
})
// Restore the user's session permission mode onto the fresh ACP session.
// Restore the task's effective model and the user's session permission mode
// onto the fresh ACP session.
m.reapplySessionModelAfterReset(ctx, execution, newSessionID, effectiveModel)
m.reapplySessionModeAfterReset(ctx, execution, newSessionID, prevMode)
m.logger.Info("agent context reset via session (no process restart)",
@@ -42,6 +42,7 @@ type restartMockAgentctlServer struct {
mu sync.Mutex
httpActions []string
wsActions []string
setModelIDs []string
failStop bool
failSessionNew bool
@@ -109,7 +110,7 @@ func newRestartMockAgentctlServer(t *testing.T, failStop, failSessionNew bool) *
continue
}
m.recordWS(msg.Action)
m.recordWS(msg)
var resp *ws.Message
switch msg.Action {
@@ -142,6 +143,10 @@ func newRestartMockAgentctlServer(t *testing.T, failStop, failSessionNew bool) *
resp, _ = ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{
"success": true,
})
case "agent.session.set_model":
resp, _ = ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{
"success": true,
})
case "agent.session.set_config_option":
resp, _ = ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{
"success": true,
@@ -185,10 +190,19 @@ func (m *restartMockAgentctlServer) recordHTTP(action string) {
m.httpActions = append(m.httpActions, action)
}
func (m *restartMockAgentctlServer) recordWS(action string) {
func (m *restartMockAgentctlServer) recordWS(message ws.Message) {
m.mu.Lock()
defer m.mu.Unlock()
m.wsActions = append(m.wsActions, action)
m.wsActions = append(m.wsActions, message.Action)
if message.Action != "agent.session.set_model" {
return
}
var payload struct {
ModelID string `json:"model_id"`
}
if err := json.Unmarshal(message.Payload, &payload); err == nil {
m.setModelIDs = append(m.setModelIDs, payload.ModelID)
}
}
func (m *restartMockAgentctlServer) getHTTPActions() []string {
@@ -207,6 +221,14 @@ func (m *restartMockAgentctlServer) getWSActions() []string {
return out
}
func (m *restartMockAgentctlServer) getSetModelIDs() []string {
m.mu.Lock()
defer m.mu.Unlock()
out := make([]string, len(m.setModelIDs))
copy(out, m.setModelIDs)
return out
}
func TestManager_RestartAgentProcess_Success(t *testing.T) {
mgr := newTestManager(t)
mock := newRestartMockAgentctlServer(t, false, false)
@@ -686,6 +708,55 @@ func TestManager_ResetAgentContext_ReappliesSessionMode(t *testing.T) {
require.Zero(t, exec.assistantHistoryBuffer.Len())
}
// TestManager_ResetAgentContext_ReappliesSessionModel is the regression test
// for an ACP fast-path reset replacing the task's selected model with the
// provider default from the freshly-created session.
func TestManager_ResetAgentContext_ReappliesSessionModel(t *testing.T) {
mgr := newTestManager(t)
mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{
infos: map[string]*WorkspaceInfo{
"session-1": {SessionID: "session-1", RuntimeModel: "mock-smart"},
},
}
mock := newRestartMockAgentctlServer(t, false, false)
client := createTestClient(t, mock.server.URL)
t.Cleanup(client.Close)
ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
defer cancel()
require.NoError(t, client.StreamUpdates(ctx, func(agentctl.AgentEvent) {}, nil, nil))
exec := &AgentExecution{
ID: "exec-model-reset",
TaskID: "task-1",
SessionID: "session-1",
AgentProfileID: "profile-1",
ACPSessionID: "old-session",
AgentCommand: "auggie --model test",
Status: v1.AgentStatusRunning,
WorkspacePath: "/workspace",
sessionInitialized: true,
agentctl: client,
promptDoneCh: make(chan PromptCompletionSignal, 1),
}
// The cache may lag a just-persisted workflow/user model change. The
// persisted runtime model is the authoritative pre-reset selection.
exec.SetModelState(&CachedModelState{CurrentModelID: "mock-fast"})
require.NoError(t, mgr.executionStore.Add(exec))
require.NoError(t, mgr.ResetAgentContext(ctx, exec.ID))
actions := mock.getWSActions()
resetIndex := slices.Index(actions, "agent.session.reset")
modelIndex := slices.Index(actions, "agent.session.set_model")
require.GreaterOrEqual(t, resetIndex, 0)
require.Greater(t, modelIndex, resetIndex,
"the model must be reapplied after the fresh ACP session is created")
require.Equal(t, []string{"mock-smart"}, mock.getSetModelIDs(),
"reset must restore the persisted effective model, not the fresh-session default")
}
// TestManager_RestartAgentProcess_PrefersPersistedModeOverStaleCache is the
// regression for the ordering hazard flagged on #1188: when a set_session_mode
// action persists a newer mode to the DB in the same on_enter batch just before
@@ -111,6 +111,63 @@ test.describe("Manual proceed to next workflow step", () => {
await expect(session.idleInput()).toBeVisible({ timeout: 15_000 });
});
test("preserves selected model across context reset", async ({
testPage,
apiClient,
seedData,
}) => {
const { agents } = await apiClient.listAgents();
const agent = agents.find((item) => item.name === "mock-agent");
if (!agent) {
throw new Error("E2E mock agent is required for model-reset coverage");
}
const smartProfile = await apiClient.createAgentProfile(agent.id, "Reset Model Profile", {
model: "mock-smart",
});
const workflow = await apiClient.createWorkflow(seedData.workspaceId, "Reset Model Workflow");
const startStep = await apiClient.createWorkflowStep(workflow.id, "Start", 0);
const resetStep = await apiClient.createWorkflowStep(workflow.id, "Reset", 1);
await apiClient.updateWorkflowStep(startStep.id, {
prompt: 'e2e:message("start complete")\n{{task_prompt}}',
events: { on_enter: [{ type: "auto_start_agent" }] },
});
await apiClient.updateWorkflowStep(resetStep.id, {
prompt: 'e2e:message("reset complete")\n{{task_prompt}}',
events: {
on_enter: [{ type: "reset_agent_context" }, { type: "auto_start_agent" }],
},
});
const task = await apiClient.createTask(seedData.workspaceId, "Reset Model Task", {
workflow_id: workflow.id,
workflow_step_id: startStep.id,
agent_profile_id: smartProfile.id,
repository_ids: [seedData.repositoryId],
});
await testPage.goto(`/t/${task.id}`);
const session = new SessionPage(testPage);
await session.waitForLoad();
await session.waitForChatIdle({ timeout: 30_000 });
const modelTrigger = testPage.getByRole("button", { name: "Session model settings" });
await expect(modelTrigger).toContainText("Mock Smart", { timeout: 15_000 });
await session.proceedNextStepButton().click();
await expect(session.stepperStep("Reset")).toHaveAttribute("aria-current", "step", {
timeout: 15_000,
});
await session.waitForChatIdle({ timeout: 30_000 });
await expect(modelTrigger).toContainText("Mock Smart", { timeout: 15_000 });
await testPage.reload();
await session.waitForLoad();
await session.waitForChatIdle({ timeout: 30_000 });
await expect(modelTrigger).toContainText("Mock Smart", { timeout: 15_000 });
});
test("shows next step auto-start prompt when its message-added notification is missed", async ({
testPage,
apiClient,
@@ -0,0 +1,138 @@
---
spec: docs/specs/ui/acp-model-configuration-summary.md
created: 2026-07-30
status: implemented
---
# Implementation Plan: Preserve ACP Model Across Context Reset
## Overview
Capture the task session's effective model before the ACP fast-path creates a
fresh provider-native session, then reapply that model after reset. Add a
lifecycle regression test first, implement the minimal backend repair, and
cover the user-visible workflow/reset/refresh path with the existing mock ACP
agent.
## Confirmed root cause
- `ResetAgentContext` in
`apps/backend/internal/agent/runtime/lifecycle/manager_interaction.go`
captures and reapplies only `CachedModeState`.
- The ACP adapter implements reset by calling `NewSession`, so OpenCode and the
E2E mock agent advertise their provider-default model for the fresh session.
- That `session_models` event replaces the execution cache and is persisted as
task-session runtime state. The frontend correctly renders the authoritative
event/boot state, so after refresh it shows the fresh session's default model.
- Full process restart already uses `effectiveSessionRuntimeConfig` during ACP
initialization and is not the missing path.
---
## Backend
### Capture and reapply the pre-reset model
- Update
`apps/backend/internal/agent/runtime/lifecycle/manager_interaction.go` so
`ResetAgentContext` captures `execution.GetModelState()` and resolves the
effective model through the existing persisted-runtime-over-profile/cache
precedence before calling `ResetSession`.
- Add a focused model-reapply helper beside
`reapplySessionModeAfterReset`. After the fresh session is installed on the
execution, call `agentctl.SetModel` with the captured effective model.
- Treat an empty model as a no-op. Match the existing mode-reset failure
behavior: log a warning and leave provider-reported state authoritative if
the provider rejects the model.
- Keep config-option restoration out of this repair. Session mode continues to
use its existing independent reapply path.
### Regression fixture
- Extend `restartMockAgentctlServer` in
`apps/backend/internal/agent/runtime/lifecycle/manager_interaction_test.go`
to accept `agent.session.set_model` and record its `model_id` payload.
- Add `TestManager_ResetAgentContext_ReappliesSessionModel`. Seed a non-default
persisted/current model, invoke the ACP fast-path reset, and assert that
`mock-smart` is sent after `agent.session.reset`, rather than accepting the
fresh session's `mock-fast` default.
---
## Frontend
No frontend production code changes are planned. The task model selector in
`apps/web/components/task/model-selector.tsx` already consumes live
`session_models` state and boot hydration correctly; the backend repair restores
the authoritative event stream it needs.
The rendered desktop and mobile surfaces share the same session-model state.
There is no composition, navigation, touch-target, scrolling, or breakpoint
change. The existing
`apps/web/e2e/tests/chat/mobile-model-selector.spec.ts` remains the mobile
reachability and rendering coverage for that shared selector.
---
## Tests
- **What:** an ACP context reset reapplies the active non-default model to the
fresh session using the exact model ID captured before reset.
**File:**
`apps/backend/internal/agent/runtime/lifecycle/manager_interaction_test.go`.
**How:** extend the WebSocket mock to record request payloads; the test must
first fail because no `agent.session.set_model` request is sent, then pass
after the lifecycle change.
- **What:** persisted runtime model wins over the fresh provider default and
remains the session's hydrated model.
**File:**
`apps/backend/internal/agent/runtime/lifecycle/manager_interaction_test.go`.
**How:** use `mockWorkspaceInfoProvider` plus a non-default cached model and
assert the exact post-reset request ordering and value under `go test -race`.
## E2E Tests
- **Scenario:** **GIVEN** a task starts with the mock agent's non-default
`Mock Smart` model, **WHEN** proceeding into a workflow step whose `on_enter`
actions reset context and auto-start the agent, **THEN** the selector remains
`Mock Smart` after the step completes and after a page refresh.
- **File:** `apps/web/e2e/tests/workflow/workflow-step-proceed.spec.ts`.
- **What to verify:** seed the workflow/profile through `ApiClient`, proceed
through the task UI, assert the selector's accessible trigger before reset,
after reset, and after reload. The mock agent deliberately advertises
`Mock Fast` on every new session, so the assertion fails on the current bug
without relying on OpenCode or external credentials.
The existing mobile selector spec is sufficient for mobile parity because the
repair changes only backend state convergence and does not alter the shared
selector's phone composition or interaction.
## Implementation Waves And Parallel Candidates
Wave 1:
- [x] [Task 01: Reapply the effective model after ACP context reset](task-01-reapply-model-after-reset.md)
Wave 2:
- [x] [Task 02: Cover workflow reset and refresh in Playwright](task-02-workflow-reset-e2e.md)
Execution is sequential in the primary conversation. Task 02 depends on the
backend behavior from Task 01, and no task is marked parallel-safe.
## Validation commands
- `cd apps/backend && go test -race -run 'TestManager_ResetAgentContext_ReappliesSession(Model|Mode)$' ./internal/agent/runtime/lifecycle`
- `cd apps/web && pnpm e2e:run tests/workflow/workflow-step-proceed.spec.ts -- --grep "preserves selected model across context reset"`
## Risks and non-goals
- A fresh ACP session can publish its default model while reset is in flight.
Capturing the effective model before `ResetSession` avoids treating that
transient event as reset intent; the post-reset `SetModel` convergence event
restores durable state.
- The provider may reject a previously valid model. The repair logs that
failure and reports the provider's actual state instead of fabricating
success.
- This repair does not restore arbitrary non-model ACP config options, change
frontend model-selection precedence, or alter full-process restart behavior.
@@ -0,0 +1,59 @@
---
id: "01-reapply-model-after-reset"
title: "Reapply the effective model after ACP context reset"
status: done
wave: 1
depends_on: []
plan: "plan.md"
spec: "../../specs/ui/acp-model-configuration-summary.md"
---
# Task 01: Reapply the effective model after ACP context reset
## Acceptance
- `ResetAgentContext` captures the effective task-session model before the
fresh ACP session can advertise its provider default.
- A successful ACP fast-path reset sends that exact non-empty model through
`agent.session.set_model` after `agent.session.reset`.
- A rejected or empty model does not fail the completed context reset or
fabricate cached state; existing session-mode preservation remains intact.
## Verification
- `cd apps/backend && go test -race -run 'TestManager_ResetAgentContext_ReappliesSession(Model|Mode)$' ./internal/agent/runtime/lifecycle`
The new model regression must fail first because the current lifecycle sends no
`agent.session.set_model` request, then pass after the production change.
## Files likely touched
- `apps/backend/internal/agent/runtime/lifecycle/manager_interaction.go`
- `apps/backend/internal/agent/runtime/lifecycle/manager_interaction_test.go`
## Dependencies
None.
## Parallelism
Sequential. This task defines the repaired backend behavior required by Task
02.
## Inputs
- Repair additions in `What`, `Scenarios`, `Failure Modes`, and `Persistence
Guarantees` in the linked ACP model-configuration spec
- Plan sections `Capture and reapply the pre-reset model` and `Regression
fixture`
- Existing `reapplySessionModeAfterReset`,
`effectiveSessionRuntimeConfig`, and
`TestManager_ResetAgentContext_ReappliesSessionMode` patterns
- ACP adapter `SetModel` convergence events as the authoritative cache and
persistence update
## Output contract
Report the RED failure, the captured-model precedence and request ordering,
files changed, exact targeted test result, blockers, and risk notes. Mark this
task `done` and update its plan checkbox in the same conversation.
@@ -0,0 +1,57 @@
---
id: "02-workflow-reset-e2e"
title: "Cover workflow reset and refresh in Playwright"
status: done
wave: 2
depends_on:
- "01-reapply-model-after-reset"
plan: "plan.md"
spec: "../../specs/ui/acp-model-configuration-summary.md"
---
# Task 02: Cover workflow reset and refresh in Playwright
## Acceptance
- A task launched with the mock agent's non-default `Mock Smart` model retains
that model when proceeding into a reset-context workflow step.
- The task selector shows `Mock Smart` after the reset turn settles and after a
full page reload.
- The E2E uses the isolated mock agent and existing task/workflow UI; it does
not require OpenCode credentials or production-agent discovery.
## Verification
- `cd apps/web && pnpm e2e:run tests/workflow/workflow-step-proceed.spec.ts -- --grep "preserves selected model across context reset"`
The Playwright regression must fail against the pre-fix backend because every
new mock ACP session advertises `Mock Fast`, then pass with Task 01 applied.
## Files likely touched
- `apps/web/e2e/tests/workflow/workflow-step-proceed.spec.ts`
## Dependencies
Task 01.
## Parallelism
Sequential. The test exercises and depends on the lifecycle repair.
## Inputs
- Repair scenario in the linked ACP model-configuration spec
- Plan section `E2E Tests`
- Existing workflow seeding and proceed patterns in
`apps/web/e2e/tests/workflow/workflow-step-proceed.spec.ts`
- Existing selector assertions in
`apps/web/e2e/tests/chat/model-selector-error.spec.ts`
- Existing phone reachability/rendering coverage in
`apps/web/e2e/tests/chat/mobile-model-selector.spec.ts`
## Output contract
Report the seeded workflow/profile, observable selector assertions, exact
Playwright result, failure artifacts if any, blockers, and risk notes. Mark
this task `done` and update its plan checkbox in the same conversation.
@@ -20,6 +20,7 @@ ACP agents can advertise an arbitrary ordered set of model-adjacent session conf
- Hovering or focusing the closed task selector shows a compact tooltip containing every currently rendered selector option as provider-supplied `Name: Value` rows, including baseline-matching values. The tooltip contains no descriptions or inferred provider knowledge. Opening the selector shows compact option names and selected values; entering an option submenu shows that option's provider description and the provider descriptions of its selectable values when supplied.
- Kandev preserves optional ACP descriptions for both top-level config options and selectable values throughout the adapter, backend event, WebSocket, store, and selector pipeline. Missing descriptions produce no invented or hard-coded explanatory text.
- Task-detail boot data includes the last persisted model list, live config options, and provider-default baseline so the compact label is complete on the first render instead of repainting after WebSocket reconnection.
- Resetting agent context creates a fresh provider-native conversation without changing the task session's effective model. Kandev captures the authoritative active model before creating the fresh ACP session, reapplies it after the reset, and lets the resulting provider convergence event remain the source of truth for persistence and UI hydration.
- An unnamed agent tab derives its title from the task session's authoritative current model. Live config values may identify the current model, but non-model config values and the session mode are never appended to the tab title. The title updates when the model changes and remains correct after a task-detail refresh; a user-supplied session name continues to override the derived title.
- Each turn stores an immutable configuration snapshot when the turn is created. The snapshot contains the effective model, mode, ordered selected config values with their provider-supplied display names, and the task-session provider-default baseline used for comparison. Agent-message metadata renders from this turn snapshot instead of the session's latest mutable runtime configuration.
- Agent-message metadata always shows the attributed model and shows only non-model config values that differ from the turn's captured baseline. It keeps option names in the compact row because message attribution is read outside the selector context. Baseline-matching values and the session mode are omitted from the compact row; the mode remains available in turn metadata.
@@ -40,6 +41,7 @@ ACP agents can advertise an arbitrary ordered set of model-adjacent session conf
- **GIVEN** an ACP option or value supplies a description, **WHEN** the user enters that option's submenu, **THEN** Kandev shows the provider text. The closed trigger and top-level option list remain compact, and missing descriptions leave the description region absent.
- **GIVEN** a task selector has current model and config values, **WHEN** the user hovers or focuses its closed trigger, **THEN** a compact tooltip lists every selector option as `Name: Value` without provider descriptions, including values omitted from the changed-only trigger label.
- **GIVEN** a task session has persisted dynamic model configuration, **WHEN** the task-detail page is refreshed, **THEN** the first rendered selector label includes all changed values without waiting for a WebSocket event.
- **GIVEN** a task session is running a non-default model, **WHEN** a workflow step resets agent context, **THEN** the fresh ACP session continues with that model and the task selector still shows it after a page refresh.
- **GIVEN** an unnamed agent tab whose selected model changes during the session, **WHEN** the model update converges or the task-detail page is refreshed, **THEN** the existing tab title shows only the current selected model instead of the agent profile's original model label or any session mode and non-model config values.
- **GIVEN** turn A runs with reasoning `High`, **WHEN** reasoning changes to `Low` before turn B, **THEN** turn A continues to show `Reasoning effort: High` and turn B shows `Reasoning effort: Low`.
- **GIVEN** a turn uses only provider-default options, **WHEN** its agent-message metadata renders, **THEN** the compact row shows only the attributed model.
@@ -58,18 +60,21 @@ ACP agents can advertise an arbitrary ordered set of model-adjacent session conf
## Failure Modes
- Failure to persist the first baseline must not prevent the session from running or configuration from being changed. Kandev reports the persistence failure and retries on a later settled configuration event without overwriting a baseline that was successfully stored.
- If the provider rejects reapplying the captured model after context reset, the reset remains complete, Kandev logs the failure, and provider-reported state remains authoritative rather than presenting a model that was not applied.
- Unknown option types remain ignored according to existing ACP graceful-degradation behavior.
- Missing option names, value names, or descriptions fall back only to existing raw identifiers/values; Kandev does not infer provider semantics.
## Persistence Guarantees
- Once stored, the baseline is not replaced by later ACP updates, user selections, agent-initiated selections, backend restarts, or session resume.
- The effective model is captured before a context reset can emit the fresh ACP session's provider default. A later default event from that fresh session cannot become the durable task-session model merely because reset created a new provider-native conversation.
- Baseline comparison is scoped to the task session, not the agent profile or provider globally.
- Once a turn is created, its configuration snapshot is not changed by later session configuration events. Only provider-reported attribution fields such as the actual model and usage may be added to the turn.
## Out of Scope
- Defining or inferring provider defaults beyond the task session's initial provider-advertised configuration.
- Restoring non-model ACP configuration options as part of context reset.
- Hard-coded descriptions, aliases, importance rankings, or default values for individual ACP providers.
- Reconstructing configuration snapshots for turns created before this behavior shipped.
- Changing the closed-label behavior in agent-profile settings or other non-task selector surfaces.