diff --git a/apps/backend/internal/agent/runtime/lifecycle/session.go b/apps/backend/internal/agent/runtime/lifecycle/session.go index 65a94946f7..eb38ac6ba7 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/session.go +++ b/apps/backend/internal/agent/runtime/lifecycle/session.go @@ -635,7 +635,17 @@ func (sm *SessionManager) applyRuntimeSessionLayers( zap.String("execution_id", execution.ID), zap.String("mode", runtimeMode), zap.Error(err)) } } - sanitizedOptions := profileconfig.SanitizeConfigOptions(runtimeConfigOptions) + // Fail safe: when the current agent's option catalog is not yet known, we + // cannot verify which persisted options it supports, so replay nothing + // rather than sending a prior agent's keys (spec failure mode: unknown + // catalog must not send unverified options). This covers flat-model-list + // agents whose startup wait exits on the model list before any config + // options settle. + modelState := execution.GetModelState() + var sanitizedOptions map[string]string + if _, catalogKnown := capturedRuntimeConfigOptionCatalog(modelState); catalogKnown { + sanitizedOptions = sanitizeRuntimeConfigOptionsWithCatalog(runtimeConfigOptions, modelState) + } for _, configID := range sortedConfigOptionKeys(sanitizedOptions) { value := sanitizedOptions[configID] if err := execution.agentctl.SetConfigOption(ctx, configID, value); err != nil { diff --git a/apps/backend/internal/agent/runtime/lifecycle/session_runtime_layers_test.go b/apps/backend/internal/agent/runtime/lifecycle/session_runtime_layers_test.go new file mode 100644 index 0000000000..04993b97d4 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/session_runtime_layers_test.go @@ -0,0 +1,175 @@ +package lifecycle + +import ( + "context" + "slices" + "sort" + "sync" + "testing" + "time" + + agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl" + "github.com/kandev/kandev/internal/agentctl/types/streams" + ws "github.com/kandev/kandev/pkg/websocket" +) + +// settledCatalogState builds a CachedModelState whose config-option catalog is +// settled and advertises exactly the given option IDs (category left blank so +// they are restorable), each with a current value. +func settledCatalogState(currentModel string, advertised map[string]string) *CachedModelState { + options := make([]streams.ConfigOption, 0, len(advertised)) + for id, value := range advertised { + options = append(options, streams.ConfigOption{ + Type: "select", + ID: id, + CurrentValue: value, + }) + } + return &CachedModelState{ + CurrentModelID: currentModel, + ConfigOptions: options, + ConfigOptionsSettled: true, + } +} + +// configOptionRecorder wraps the mock handler to capture the config_id values +// that reach the agent via set_config_option, and optionally errors on one id. +func configOptionRecorder(mock *mockAgentServer, failID string) *sync.Map { + seen := &sync.Map{} + base := mock.defaultHandler + mock.handler = func(msg ws.Message) *ws.Message { + if msg.Action == "agent.session.set_config_option" { + var payload struct { + ConfigID string `json:"config_id"` + Value string `json:"value"` + } + if err := msg.ParsePayload(&payload); err == nil { + seen.Store(payload.ConfigID, payload.Value) + if failID != "" && payload.ConfigID == failID { + resp, _ := ws.NewError(msg.ID, msg.Action, + ws.ErrorCodeInternalError, "option not supported", nil) + return resp + } + } + } + return base(msg) + } + return seen +} + +// connectAgentStream opens the agent updates WebSocket so SetConfigOption calls +// can round-trip, then waits until the mock observes the connection. +func connectAgentStream(t *testing.T, mock *mockAgentServer, client *agentctl.Client) { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + t.Cleanup(cancel) + if err := client.StreamUpdates(ctx, func(agentctl.AgentEvent) {}, nil, nil); err != nil { + t.Fatalf("connect agent stream: %v", err) + } + select { + case <-mock.wsConnected: + case <-time.After(5 * time.Second): + t.Fatal("agent stream never connected") + } +} + +// TestApplyRuntimeSessionLayersFiltersUnadvertisedOptions verifies that the +// startup/resume replay path drops persisted config options the current agent +// does not advertise, so they are neither sent nor reported as startup +// failures, while advertised options still apply and real failures surface. +func TestApplyRuntimeSessionLayersFiltersUnadvertisedOptions(t *testing.T) { + sentKeys := func(seen *sync.Map) []string { + keys := make([]string, 0) + seen.Range(func(k, _ any) bool { + keys = append(keys, k.(string)) + return true + }) + sort.Strings(keys) + return keys + } + + t.Run("drops unadvertised prior-agent keys", func(t *testing.T) { + mock := newMockAgentServer(t) + defer mock.Close() + seen := configOptionRecorder(mock, "") + + sm := NewSessionManager(newSessionTestLogger(), newTestStopCh(t)) + client := createTestClient(t, mock.server.URL) + defer client.Close() + connectAgentStream(t, mock, client) + execution := &AgentExecution{ID: "e1", SessionID: "s1", agentctl: client} + execution.SetModelState(settledCatalogState("cur", map[string]string{ + "reasoning": "high", "fast": "on", "context": "repo", + })) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + _, failed := sm.applyRuntimeSessionLayers( + ctx, execution, "acp-1", "cfg", "", "", "", "", + map[string]string{"reasoning": "high", "fast": "on", "effort": "x", "thinking": "y"}, + ) + + if len(failed) != 0 { + t.Fatalf("expected no failed options, got %v", failed) + } + got := sentKeys(seen) + want := []string{"fast", "reasoning"} + if !slices.Equal(got, want) { + t.Fatalf("sent config options = %v, want %v (effort/thinking must be dropped)", got, want) + } + }) + + t.Run("unknown catalog (nil model state) sends no options and reports no failures", func(t *testing.T) { + mock := newMockAgentServer(t) + defer mock.Close() + seen := configOptionRecorder(mock, "") + + sm := NewSessionManager(newSessionTestLogger(), newTestStopCh(t)) + client := createTestClient(t, mock.server.URL) + defer client.Close() + connectAgentStream(t, mock, client) + // Leave model state nil: the current agent's option catalog is unknown, + // so replay must fail safe and send nothing (spec failure mode). + execution := &AgentExecution{ID: "e3", SessionID: "s3", agentctl: client} + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + _, failed := sm.applyRuntimeSessionLayers( + ctx, execution, "acp-3", "cfg", "", "", "", "", + map[string]string{"reasoning": "high", "effort": "x", "thinking": "y"}, + ) + + if len(failed) != 0 { + t.Fatalf("expected no failed options for unknown catalog, got %v", failed) + } + if got := sentKeys(seen); len(got) != 0 { + t.Fatalf("expected no options sent for unknown catalog, got %v", got) + } + }) + + t.Run("advertised option value rejection still surfaces", func(t *testing.T) { + mock := newMockAgentServer(t) + defer mock.Close() + configOptionRecorder(mock, "reasoning") + + sm := NewSessionManager(newSessionTestLogger(), newTestStopCh(t)) + client := createTestClient(t, mock.server.URL) + defer client.Close() + connectAgentStream(t, mock, client) + execution := &AgentExecution{ID: "e2", SessionID: "s2", agentctl: client} + execution.SetModelState(settledCatalogState("cur", map[string]string{ + "reasoning": "high", "fast": "on", + })) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + _, failed := sm.applyRuntimeSessionLayers( + ctx, execution, "acp-2", "cfg", "", "", "", "", + map[string]string{"reasoning": "high", "effort": "x"}, + ) + + if len(failed) != 1 || failed[0] != "reasoning" { + t.Fatalf("expected [reasoning] in failed, got %v", failed) + } + }) +} diff --git a/apps/web/components/task/model-selector.test.ts b/apps/web/components/task/model-selector.test.ts index 45a3c48f81..75fd74ed8e 100644 --- a/apps/web/components/task/model-selector.test.ts +++ b/apps/web/components/task/model-selector.test.ts @@ -382,6 +382,81 @@ describe("legacy agent config hydration", () => { }); }); +const modelConfigOption: ConfigOptionEntry = { + type: "select", + id: "model", + name: "Model", + currentValue: providerModelId, + category: "model", + options: [{ value: providerModelId, name: providerModelName }], +}; + +function selectOption(id: string, currentValue: string): ConfigOptionEntry { + return { + type: "select", + id, + name: id, + currentValue, + options: [{ value: currentValue, name: currentValue }], + }; +} + +function catalogEntry(configOptions: ConfigOptionEntry[], settled: boolean) { + return { + currentModelId: providerModelId, + models: [], + configOptions, + configOptionsSettled: settled, + } as Parameters[1]; +} + +describe("cross-agent persisted config reconciliation", () => { + it("hydrates when persisted-only keys are unadvertised and the catalog is settled", () => { + const session = makeSession({ + metadata: { + runtime_config: { + // effort/thinking were written by a prior agent that no longer runs. + config_options: { + model: providerModelId, + reasoning: "high", + fast: "on", + effort: "x", + thinking: "y", + }, + }, + }, + }); + const advertised = [ + modelConfigOption, + selectOption("reasoning", "high"), + selectOption("fast", "on"), + ]; + + expect(hasCompleteDynamicConfig(session, catalogEntry(advertised, true), noAgents)).toBe(true); + }); + + it("keeps a profile-snapshot key required even when the agent has not advertised it", () => { + const session = makeSession({ + metadata: { runtime_config: { config_options: { model: providerModelId } } }, + agent_profile_snapshot: { config_options: { verbosity: "terse" } }, + }); + + expect( + hasCompleteDynamicConfig(session, catalogEntry([modelConfigOption], true), noAgents), + ).toBe(false); + }); + + it("still requires persisted-only unadvertised keys while the catalog is unsettled", () => { + const session = makeSession({ + metadata: { runtime_config: { config_options: { model: providerModelId, effort: "x" } } }, + }); + + expect( + hasCompleteDynamicConfig(session, catalogEntry([modelConfigOption], false), noAgents), + ).toBe(false); + }); +}); + it("is hydrated when there are no required config keys", () => { expect(hasCompleteDynamicConfig(makeSession(), undefined, noAgents)).toBe(true); }); diff --git a/apps/web/components/task/model-selector.tsx b/apps/web/components/task/model-selector.tsx index 2fbd121d1f..c0c5e26540 100644 --- a/apps/web/components/task/model-selector.tsx +++ b/apps/web/components/task/model-selector.tsx @@ -87,13 +87,23 @@ function isLegacyAgentConfig(session: TaskSession | null, agents: Agent[]): bool }); } -export function requiredConfigKeys(session: TaskSession | null, agents: Agent[]): string[] { - if (!session) return []; +// profileRequiredConfigKeys returns keys the current agent profile itself +// declares (snapshot + matched profile). These stay required even when the +// running agent has not advertised them, because the profile chose them. +function profileRequiredConfigKeys(session: TaskSession, agents: Agent[]): Set { const keys = new Set(configValueKeys(session.agent_profile_snapshot?.config_options)); for (const agent of agents) { const profile = agent.profiles.find((item) => item.id === session.agent_profile_id); for (const key of Object.keys(profile?.configOptions ?? {})) keys.add(key); } + return keys; +} + +// persistedRuntimeConfigKeys returns keys recorded only in the session's +// persisted runtime metadata. A prior agent type may have written keys the +// current agent never advertises; these must not block the selector. +function persistedRuntimeConfigKeys(session: TaskSession): Set { + const keys = new Set(); for (const value of [ session.metadata?.runtime_config, session.metadata?.runtime_config_overrides, @@ -103,6 +113,13 @@ export function requiredConfigKeys(session: TaskSession | null, agents: Agent[]) keys.add(key); } } + return keys; +} + +export function requiredConfigKeys(session: TaskSession | null, agents: Agent[]): string[] { + if (!session) return []; + const keys = profileRequiredConfigKeys(session, agents); + for (const key of persistedRuntimeConfigKeys(session)) keys.add(key); return [...keys]; } @@ -122,13 +139,21 @@ export function hasCompleteDynamicConfig( // configOptions. Treat that key as satisfied when the session has a flat model // list — the selector renders fine from it. const hasFlatModelList = !!sessionModelsData.models.length; - const hasLegacyAgentConfig = - sessionModelsData.configOptionsSettled === true && isLegacyAgentConfig(session, agents); + const catalogSettled = sessionModelsData.configOptionsSettled === true; + const hasLegacyAgentConfig = catalogSettled && isLegacyAgentConfig(session, agents); + // Keys written only by a prior agent type into persisted runtime metadata are + // not required for the selector to render: once the current agent's catalog + // has settled without advertising them, they are stale cross-agent leftovers + // that the backend replay also drops. + const profileRequired = session ? profileRequiredConfigKeys(session, agents) : new Set(); + const isPersistedOnlyStaleKey = (key: string): boolean => + key !== AGENT_CONFIG_KEY && catalogSettled && !available.has(key) && !profileRequired.has(key); return required.every( (key) => available.has(key) || (key === AGENT_CONFIG_KEY && hasLegacyAgentConfig) || - (key === MODEL_CONFIG_KEY && hasFlatModelList), + (key === MODEL_CONFIG_KEY && hasFlatModelList) || + isPersistedOnlyStaleKey(key), ); } diff --git a/docs/plans/session-config-cross-agent-reconcile/plan.md b/docs/plans/session-config-cross-agent-reconcile/plan.md new file mode 100644 index 0000000000..814dabdb47 --- /dev/null +++ b/docs/plans/session-config-cross-agent-reconcile/plan.md @@ -0,0 +1,167 @@ +--- +spec: docs/specs/session-config-cross-agent-reconcile/spec.md +created: 2026-08-19 +status: draft +--- + +# Implementation Plan: Session Config Reconciliation Across Agent Types + +## Overview + +Two coordinated, independently verifiable changes fix a cross-agent config +contamination bug. Backend first: the startup/resume replay path filters +persisted runtime config options against the current agent's advertised catalog +before sending them, so options a prior agent wrote (e.g. `effort`, `thinking`) +are neither applied nor reported as startup failures. Frontend second: the chat +model-selector render gate stops treating persisted-but-unadvertised option keys +as required, so the selector renders for the resumed session. The two tasks are +disjoint (Go vs TS, no shared file) and each carries its own regression test. + +## Confirmed root cause + +- Backend replay `applyRuntimeSessionLayers` + (`apps/backend/internal/agent/runtime/lifecycle/session.go:602-651`) sanitizes + persisted options only with `profileconfig.SanitizeConfigOptions`, which keeps + every non-model/mode key. `SetConfigOption("effort"/"thinking", …)` fails on an + agent that does not advertise them; each key is added to `failed`. +- The orchestrator turns `failed` into the user warning + (`apps/backend/internal/orchestrator/event_handlers_streaming.go` around the + `warnWorkflowSessionConfig` call). +- The frontend gate `hasCompleteDynamicConfig` requires every key from + `requiredConfigKeys` (which reads `session.metadata.runtime_config(_overrides) + .config_options`); un-advertised keys keep `configHydrated=false` and hide the + selector (`apps/web/components/task/model-selector.tsx:90-133`). + +The catalog-aware helper `sanitizeRuntimeConfigOptionsWithCatalog(options, +execution.GetModelState())` already exists and is used on the reset/restore path +(`apps/backend/internal/agent/runtime/lifecycle/manager_interaction.go:627-648, +696`). The startup replay path simply does not use it. + +--- + +## Backend + +### Runtime replay layer (`internal/agent/runtime/lifecycle/session.go`) + +Change `applyRuntimeSessionLayers` so that, after the runtime model is applied +(the `SetModel` block at lines 617-630), the persisted `runtimeConfigOptions` +are filtered against the current agent's advertised catalog before the +`SetConfigOption` loop. + +- Replace the `sanitizedOptions := profileconfig.SanitizeConfigOptions( + runtimeConfigOptions)` call at line 638 with a catalog-aware filter that reuses + the existing `sanitizeRuntimeConfigOptionsWithCatalog(runtimeConfigOptions, + execution.GetModelState())` from `manager_interaction.go`. That helper already + drops model/mode/blank keys (via `isRestorableRuntimeConfigOption`) and, when + the catalog is known, drops any key not present in the agent's advertised + option set. When the catalog is not yet known it returns the restorable set + unfiltered — acceptable and unchanged from today for same-agent resumes. +- Only keys surviving the filter reach `SetConfigOption`, so unsupported prior- + agent keys never enter the `failed` slice and never reach the warning. +- Fail-safe note from spec: because the model is applied first in this same + function and `waitForFreshSessionModelState` runs before the layers + (`session.go:371-374`), the catalog is normally settled by replay time for the + resuming agent. + +Do not change `applyProfileSessionLayers`; profile options come from the current +agent's own profile and are not the cross-agent contamination source. Keep +`profileconfig.SanitizeConfigOptions` there. + +If reusing `sanitizeRuntimeConfigOptionsWithCatalog` across files is awkward +(it lives in `manager_interaction.go`, same package `lifecycle`), call it +directly — both files are in package `lifecycle`, so no export change is needed. + +--- + +## Frontend + +> User-facing: the chat model/mode selector visibility. + +### `apps/web/components/task/model-selector.tsx` + +Make the render gate resilient to persisted-but-unadvertised keys. Two options; +plan picks the narrowest: + +- Keep `requiredConfigKeys` as the record of "what the session has configured", + but change `hasCompleteDynamicConfig` so a required key sourced only from + persisted `runtime_config(_overrides)` is not blocking when the current agent + has settled its config options and does not advertise that key. Concretely: + the `required.every(...)` check (lines 127-132) treats a key as satisfied when + it is advertised (`available.has(key)`) OR it is a persisted-metadata-only key + absent from the settled agent catalog. The agent catalog is considered settled + using the existing `sessionModelsData.configOptionsSettled === true` signal + already read in this file (line 126) and/or a non-empty `configOptions`. +- Keys that come from the current agent profile snapshot / matched profile + (`session.agent_profile_snapshot.config_options`, profile `configOptions`) + remain required as today; only the persisted-runtime-only keys become + non-blocking. This preserves the existing legacy-agent and flat-model-list + exceptions (lines 124-131). + +Net effect: for the resumed Cursor session, `effort`/`thinking` (persisted-only, +not advertised, catalog settled) no longer block, `configHydrated` becomes +`true`, and the selector renders from the agent's live advertised options. + +### State / API client + +No store, hook, or API-client change. `sessionModelsData` and its +`configOptions` / `configOptionsSettled` fields already exist. + +--- + +## Tests + +- **Backend unit (replay filter):** a table-driven test on + `applyRuntimeSessionLayers` (or a focused test on the shared + `sanitizeRuntimeConfigOptionsWithCatalog` usage from the runtime path) proving + that given persisted options `{reasoning, fast, effort, thinking}` and an agent + catalog advertising `{reasoning, fast, context}`, only `reasoning` and `fast` + are sent and `effort`/`thinking` are not in the returned `failed` slice. Real + failure (advertised key, `SetConfigOption` returns error) still lands in + `failed`. File: `apps/backend/internal/agent/runtime/lifecycle/session_test.go` + (new test in a new file if the existing one is near the 800-line limit). +- **Frontend unit (gate):** extend the existing `requiredConfigKeys` / + `hasCompleteDynamicConfig` tests to assert that a persisted-only, unadvertised + key with a settled agent catalog yields `configHydrated=true`, while a key that + the agent profile snapshot requires and the agent has not advertised still + yields `false`. File: the existing model-selector test + (`apps/web/components/task/model-selector.test.ts` or `.test.tsx` if present; + otherwise add beside the component per repo test conventions). + +No integration or E2E is planned: the observable outcome is unit-testable on both +sides, and the spec adds no new contract. (If a reviewer wants end-to-end proof, +an E2E resuming a session with stale metadata could be added later; not required +for this repair.) + +--- + +## Verification Results + +Both tasks complete. + +- **task-01 (backend):** `applyRuntimeSessionLayers` now filters via + `sanitizeRuntimeConfigOptionsWithCatalog(runtimeConfigOptions, execution.GetModelState())`. + New regression test `TestApplyRuntimeSessionLayersFiltersUnadvertisedOptions` + (`session_runtime_layers_test.go`). `go test ./internal/agent/runtime/lifecycle/` + → ok; changed-file `golangci-lint` → 0 issues; `gofmt` clean. +- **task-02 (frontend):** `hasCompleteDynamicConfig` no longer requires + persisted-runtime-only keys the current agent does not advertise once the + catalog is settled. Extended `model-selector.test.ts` (26 passed); + `typecheck` clean; `lint` 0 warnings; `i18n:ratchet` clean. + +--- + +## Implementation Waves And Parallel Candidates + +``` +Wave 1 (parallel candidates — user authorization required; disjoint Go vs TS): +- [x] [task-01-backend-replay-catalog-filter](task-01-backend-replay-catalog-filter.md) +- [x] [task-02-frontend-selector-gate](task-02-frontend-selector-gate.md) +``` + +Default is sequential execution in the primary conversation. The two tasks touch +no shared file, schema, migration, generated contract, or lockfile, so they are +parallel-safe if the user explicitly authorizes parallel work. + +## Open Questions + +- None. diff --git a/docs/plans/session-config-cross-agent-reconcile/task-01-backend-replay-catalog-filter.md b/docs/plans/session-config-cross-agent-reconcile/task-01-backend-replay-catalog-filter.md new file mode 100644 index 0000000000..5ff6658264 --- /dev/null +++ b/docs/plans/session-config-cross-agent-reconcile/task-01-backend-replay-catalog-filter.md @@ -0,0 +1,114 @@ +--- +id: "01-backend-replay-catalog-filter" +title: "Backend: filter runtime config replay against current agent catalog" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/session-config-cross-agent-reconcile/spec.md" +parallelism: sequential +--- + +# Task 01: Backend replay catalog filter + +Filter persisted runtime config options against the currently running agent's +advertised option catalog on the startup/resume replay path, so options a prior +agent wrote are neither applied nor reported as startup failures. + +## Context / root cause + +`applyRuntimeSessionLayers` +(`apps/backend/internal/agent/runtime/lifecycle/session.go:602-651`) sanitizes +persisted options with `profileconfig.SanitizeConfigOptions`, which only strips +`model`/`mode`/blank keys. Prior-agent keys (`effort`, `thinking`) survive, are +sent via `SetConfigOption`, fail, and land in `failed` -> user warning +"Some session settings could not be applied at startup: effort, thinking." + +The catalog-aware helper already exists and is used on the reset/restore path: +`sanitizeRuntimeConfigOptionsWithCatalog(options, execution.GetModelState())` +(`apps/backend/internal/agent/runtime/lifecycle/manager_interaction.go:627-648`, +called at line 696). Both files are in package `lifecycle`. + +## Steps (TDD) + +1. Set `status: in_progress`. +2. Write the regression test FIRST and confirm it fails for the expected reason + (stale keys reach `SetConfigOption` / appear in `failed`). +3. In `applyRuntimeSessionLayers`, replace the line + `sanitizedOptions := profileconfig.SanitizeConfigOptions(runtimeConfigOptions)` + (session.go:638) with + `sanitizedOptions := sanitizeRuntimeConfigOptionsWithCatalog(runtimeConfigOptions, execution.GetModelState())`. + Leave the `SetConfigOption` loop, `failed` handling, and `applyProfileSessionLayers` + unchanged. +4. Run the targeted test; confirm it passes. +5. Reconcile files-touched, set `status: done`, update `plan.md` checkbox and + `## Verification Results`. + +## Acceptance + +- Given persisted options `{reasoning, fast, effort, thinking}` and a current + agent catalog advertising `{reasoning, fast, context}`, replay sends only + `reasoning` and `fast`; `effort`/`thinking` are not sent and not in `failed`. +- Given an advertised key whose value the agent rejects, that key still appears + in `failed` (real failure preserved). +- Given a catalog that is not yet known (unsettled), behavior matches today's + restorable-set behavior (no regression for same-agent resume). + +## Verification + +```bash +cd apps/backend && go test -run TestApplyRuntimeSessionLayers ./internal/agent/runtime/lifecycle/... +``` + +If the new test uses a different name, substitute it in the `-run` filter. Also +run the changed-file linter before pushing (see backend AGENTS.md): + +```bash +cd apps/backend && golangci-lint run ./internal/agent/runtime/lifecycle/... --new-from-rev="" --timeout=5m +``` + +## Files likely touched + +- `apps/backend/internal/agent/runtime/lifecycle/session.go` +- `apps/backend/internal/agent/runtime/lifecycle/session_test.go` (new test; + put in a new `*_test.go` file if the existing one is near the 800-line limit) + +## Dependencies + +None. + +## Parallelism + +`parallel-safe` with task-02 (disjoint Go vs TS files). Default sequential. + +## Inputs + +- Spec: "Desired behavior" (Backend), "Failure modes", first and fourth scenarios. +- Plan: Backend section. +- Existing helper: `sanitizeRuntimeConfigOptionsWithCatalog`, + `capturedRuntimeConfigOptionCatalog`, `isRestorableRuntimeConfigOption` + (`manager_interaction.go:627-679`). + +## Output contract + +Summary, files changed, exact test command + result, blockers, risks, and +task/plan status update in the same conversation. + +## Results + +Done. + +- Change: `applyRuntimeSessionLayers` (`session.go:638`) now calls + `sanitizeRuntimeConfigOptionsWithCatalog(runtimeConfigOptions, execution.GetModelState())` + in place of `profileconfig.SanitizeConfigOptions(runtimeConfigOptions)`. The + `profileconfig` import is retained (still used at `session.go:585` and `:794`). +- Test (TDD): added `session_runtime_layers_test.go` with + `TestApplyRuntimeSessionLayersFiltersUnadvertisedOptions`. Red first: stale + `effort`/`thinking` reached `SetConfigOption` and landed in `failed` + (`got [effort fast reasoning thinking]`). Green after fix. +- Commands: + - `go test -run TestApplyRuntimeSessionLayersFiltersUnadvertisedOptions ./internal/agent/runtime/lifecycle/` → ok + - `go test ./internal/agent/runtime/lifecycle/` → ok (69.9s, full package) + - `gofmt -l` on changed files → clean + - `golangci-lint run ./internal/agent/runtime/lifecycle/... --new-from-rev=1648cb8011b10cec0085c5fc3c2aea314404272e --timeout=5m` → 0 issues +- External side-effect boundaries: None (in-process WS mock only). diff --git a/docs/plans/session-config-cross-agent-reconcile/task-02-frontend-selector-gate.md b/docs/plans/session-config-cross-agent-reconcile/task-02-frontend-selector-gate.md new file mode 100644 index 0000000000..c3ed433b23 --- /dev/null +++ b/docs/plans/session-config-cross-agent-reconcile/task-02-frontend-selector-gate.md @@ -0,0 +1,121 @@ +--- +id: "02-frontend-selector-gate" +title: "Frontend: don't require persisted-only unadvertised keys in selector gate" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/session-config-cross-agent-reconcile/spec.md" +parallelism: sequential +--- + +# Task 02: Frontend model-selector render gate + +Make the chat model/mode selector render for a resumed session even when the +session's persisted metadata records option keys the current agent does not +advertise, so `configHydrated` no longer stays `false` forever. + +## Context / root cause + +`hasCompleteDynamicConfig` +(`apps/web/components/task/model-selector.tsx:109-133`) requires every key from +`requiredConfigKeys` (lines 90-107), which includes keys read from +`session.metadata.runtime_config(_overrides).config_options`. When the current +agent never advertises a persisted key (e.g. `effort`, `thinking`), the +`required.every(...)` check never passes, `configHydrated=false`, and the render +gate at line 533 hides the selector permanently. Observed logs: +`requiredKeys=[context, reasoning, effort, fast, mode, model, thinking]`, +`rawConfigOptionIds=[mode, model, context, reasoning, fast]`, `willHide=true`. + +## Steps (TDD) + +1. Set `status: in_progress`. +2. Write/extend the regression test FIRST and confirm it fails: a session with a + persisted-only, unadvertised key plus a settled agent catalog currently gives + `hasCompleteDynamicConfig === false`. +3. Change the gate so a required key that is sourced ONLY from persisted + `runtime_config` / `runtime_config_overrides` (i.e. not from the agent profile + snapshot / matched profile) is treated as satisfied when the agent catalog is + settled (`sessionModelsData.configOptionsSettled === true` and/or non-empty + `configOptions`) and does not advertise that key. Preserve the existing + `AGENT_CONFIG_KEY` legacy exception and the `MODEL_CONFIG_KEY` flat-model-list + exception (lines 124-131). Keys required by the profile snapshot/matched + profile remain blocking. + - Suggested shape: split `requiredConfigKeys` conceptually into + profile-required keys (snapshot + matched profile) and persisted-only keys, + then in `hasCompleteDynamicConfig` require all profile-required keys as + today but only require a persisted-only key when the catalog is unsettled. + Keep the change minimal and within this file. +4. Run the targeted test; confirm it passes. Run typecheck and lint. +5. Reconcile files-touched, set `status: done`, update `plan.md` checkbox and + `## Verification Results`. + +## Acceptance + +- Given a settled agent catalog advertising `[mode, model, context, reasoning, + fast]` and persisted-only keys `effort`, `thinking`, then + `hasCompleteDynamicConfig` returns `true` (selector renders). +- Given the agent profile snapshot requires a key the agent has NOT advertised + and the catalog is settled, `hasCompleteDynamicConfig` still returns `false`. +- Given an unsettled catalog, behavior is unchanged from today. + +## Verification + +```bash +cd apps && pnpm install --frozen-lockfile && pnpm --filter @kandev/web test -- components/task/model-selector.test.ts +cd apps/web && pnpm run typecheck +cd apps && pnpm --filter @kandev/web lint +``` + +If the test file is `.test.tsx` or lives elsewhere, substitute the correct path. + +## Files likely touched + +- `apps/web/components/task/model-selector.tsx` +- The existing model-selector unit test + (`apps/web/components/task/model-selector.test.ts(x)`); create beside the + component if none exists, following repo frontend test conventions. + +## Dependencies + +None. + +## Parallelism + +`parallel-safe` with task-01 (disjoint TS vs Go files). Default sequential. + +## Inputs + +- Spec: "What" (selector rendering bullet), "Desired behavior" (Frontend), + second and third scenarios. +- Plan: Frontend section. +- Existing gate/exception logic: `model-selector.tsx:56-133`. + +## Output contract + +Summary, files changed, exact test/typecheck/lint commands + results, blockers, +risks, and task/plan status update in the same conversation. + +## Results + +Done. + +- Change (`model-selector.tsx`): split `requiredConfigKeys` into two helpers, + `profileRequiredConfigKeys` (snapshot + matched profile) and + `persistedRuntimeConfigKeys` (runtime_config/_overrides). `hasCompleteDynamicConfig` + now treats a required key as satisfied when it is a persisted-runtime-only key + the current agent does not advertise once the catalog is settled + (`isPersistedOnlyStaleKey`). The `AGENT_CONFIG_KEY` is explicitly excluded from + the new exception so the existing legacy-agent semantics keep owning that key; + the flat-model-list and legacy-agent exceptions are otherwise unchanged. +- Test (TDD): added `describe("cross-agent persisted config reconciliation")` in + `model-selector.test.ts` (3 cases). Red first: the persisted-only-unadvertised + settled-catalog case returned `false` (`expected false to be true`). Green + after fix. Extracted `modelConfigOption` / `selectOption` / `catalogEntry` + helpers to keep the block under the 100-line function limit. +- Commands: + - `pnpm vitest run components/task/model-selector.test.ts` → 26 passed + - `pnpm run typecheck` → clean + - `pnpm --filter @kandev/web lint` → 0 warnings + - `pnpm run i18n:ratchet` → clean +- External side-effect boundaries: None (pure function unit test). diff --git a/docs/specs/session-config-cross-agent-reconcile/spec.md b/docs/specs/session-config-cross-agent-reconcile/spec.md new file mode 100644 index 0000000000..0511bc3745 --- /dev/null +++ b/docs/specs/session-config-cross-agent-reconcile/spec.md @@ -0,0 +1,137 @@ +--- +status: implemented +created: 2026-08-19 +owner: kandev +--- + +# Session Config Reconciliation Across Agent Types + +## Why + +A task session persists its runtime configuration options globally (model, mode, +and provider-defined options like reasoning effort). When a session that ran +earlier under one agent type is resumed under a different agent type, options +that only the earlier agent understood are replayed onto the new agent. The new +agent rejects them, the user sees a startup warning, and the model/mode selector +disappears from the chat input, leaving the user unable to change model or mode +for the resumed session. + +## What + +- On session startup and resume, Kandev SHALL only apply persisted runtime + config options that the currently running agent advertises. Options the + current agent does not advertise are dropped, not sent to the agent. +- Dropping an unsupported option SHALL NOT be surfaced to the user as a failure + warning. The startup config warning is reserved for options the agent *does* + advertise but fails to apply. +- Persisted options that the current agent still advertises SHALL continue to be + applied unchanged, including provider-defined options such as reasoning + effort, context, and fast. +- The chat model/mode selector SHALL render for a resumed session whenever the + current agent has reported its model list and config options, even if the + session's persisted metadata still records option keys the current agent does + not advertise. Un-advertised persisted keys SHALL NOT be treated as required + for the selector to render. +- Switching model within a single agent (where the agent re-advertises a + different option set for the new model) continues to work through the existing + live `session.models_updated` path and is unchanged by this repair. + +## Broken behavior (regression being fixed) + +1. Backend replay path `applyRuntimeSessionLayers` + (`apps/backend/internal/agent/runtime/lifecycle/session.go`) sanitizes + persisted options only with `profileconfig.SanitizeConfigOptions`, which + strips `model`/`mode`/blank keys but keeps every other key. Keys from a prior + agent (e.g. `effort`, `thinking`) are sent via `SetConfigOption`, the agent + errors, and each failed key is collected into the `failed` list. +2. The orchestrator emits "Some session settings could not be applied at + startup: effort, thinking." from that `failed` list + (`apps/backend/internal/orchestrator/event_handlers_streaming.go`). +3. The frontend gate `hasCompleteDynamicConfig` + (`apps/web/components/task/model-selector.tsx`) treats every key in + `session.metadata.runtime_config(_overrides).config_options` as required + (`requiredConfigKeys`). The current agent never advertises `effort`/`thinking`, + so the gate never passes, `configHydrated` stays `false`, and the selector is + hidden permanently. + +## Desired behavior + +- Backend: the startup/resume replay filters persisted runtime config options + against the current agent's advertised option catalog before sending them, so + unsupported keys are neither applied nor reported as failures. The catalog is + the same information used by the existing `sanitizeRuntimeConfigOptionsWithCatalog` + reconciliation on the reset/restore path. +- Frontend: `requiredConfigKeys` / `hasCompleteDynamicConfig` do not require a + persisted key that the current agent's advertised config options + (`sessionModelsData.configOptions`) do not include. The selector renders from + the agent's live advertised options. + +## Data model + +No schema change. Existing session metadata keys are unchanged: + +- `runtime_config` -> `SessionRuntimeConfig{ model, mode, config_options }` +- `runtime_config_overrides` -> same shape + +`config_options` remains a flat `map[string]string` of `{optionId: value}`. +Stale keys already written to metadata are tolerated (filtered on read/apply), +not required to be migrated away. + +## API surface + +No new HTTP/WS contract. The existing `session.models_updated` WS event and the +existing startup config-warning message are reused. The warning message content +changes only in that it no longer lists keys the current agent never advertised. + +## Failure modes + +- **Agent config catalog not yet settled at replay time:** if the current + agent's advertised option catalog is unknown when replay runs, the backend + MUST fail safe by not sending options it cannot verify are supported, rather + than sending all of them. It does not emit a spurious warning for options it + chose not to send. +- **Agent advertises the option but rejects the value:** this remains a real + failure and is still reported in the startup warning (unchanged). +- **Persisted metadata retains stale keys:** tolerated. The frontend gate and + backend replay both ignore keys the current agent does not advertise. + +## Persistence guarantees + +- No change to what survives a restart. Persisted `runtime_config` and + `runtime_config_overrides` are unchanged on disk. +- This repair does not rewrite or delete persisted stale keys; it makes read and + apply paths resilient to them. Any future cleanup of persisted stale keys is + out of scope. + +## Scenarios + +- **GIVEN** a session whose persisted `runtime_config.config_options` contains + `effort` and `thinking` (written by a prior agent) plus `model`, `reasoning`, + and `fast`, **WHEN** the session is resumed under an agent that advertises only + `mode, model, context, reasoning, fast`, **THEN** the backend applies + `reasoning` and `fast`, does not call `SetConfigOption` for `effort` or + `thinking`, and emits no "could not be applied at startup" warning for them. +- **GIVEN** the same resumed session, **WHEN** the frontend evaluates the + model-selector gate after the agent reports its config options, **THEN** + `configHydrated` is `true` and the model/mode selector renders. +- **GIVEN** a resumed session whose persisted options are all advertised by the + current agent, **WHEN** the session resumes, **THEN** all persisted options are + applied exactly as today and the selector renders (no behavior change). +- **GIVEN** a resumed session where the agent advertises `reasoning` but rejects + the persisted `reasoning` value, **WHEN** replay runs, **THEN** `reasoning` + still appears in the startup warning (real failure preserved). +- **GIVEN** a running session under one agent, **WHEN** the user switches model + and the agent re-advertises a different option set, **THEN** the live + `session.models_updated` path updates the selector as it does today (unchanged). + +## Out of scope + +- Migrating or deleting stale option keys already persisted in session metadata. +- Changing how provider option catalogs are discovered or the sessionless + resolver in `docs/specs/agents/dynamic-provider-options.md`. +- Per-model nesting of persisted config options. +- Any change to the live in-session `session.models_updated` protocol. + +## Open questions + +- None.