diff --git a/packages/coding-agent/src/modes/interactive/components/session-loadout-selector.ts b/packages/coding-agent/src/modes/interactive/components/session-loadout-selector.ts index e950f1bc9..341551223 100644 --- a/packages/coding-agent/src/modes/interactive/components/session-loadout-selector.ts +++ b/packages/coding-agent/src/modes/interactive/components/session-loadout-selector.ts @@ -28,6 +28,11 @@ import { type ResourceCollection = keyof ResolvedPaths; +export interface SessionLoadoutSelection { + overrides: LoadoutOverride[]; + explicitReset: boolean; +} + type FlatEntry = | { type: "reset" } | { type: "group"; group: LoadoutResourceGroup } @@ -107,7 +112,7 @@ class SessionLoadoutList implements Component, Focusable { private resetSelected = false; private _focused = false; - onApply?: (overrides: LoadoutOverride[]) => void; + onApply?: (selection: SessionLoadoutSelection) => void; onCancel?: () => void; onChange?: () => void; @@ -253,9 +258,14 @@ class SessionLoadoutList implements Component, Focusable { return; } if (keybindings.matches(data, "tui.select.confirm")) { - this.onApply?.( - buildSessionLoadoutOverrides(this.snapshot, this.enabledByReference, this.clearExistingOverrides), - ); + this.onApply?.({ + overrides: buildSessionLoadoutOverrides( + this.snapshot, + this.enabledByReference, + this.clearExistingOverrides, + ), + explicitReset: this.clearExistingOverrides, + }); return; } if (data === " ") { @@ -330,7 +340,7 @@ export class SessionLoadoutSelectorComponent extends Container implements Focusa snapshot: LoadoutSnapshot; agentDir: string; terminalHeight?: number; - onApply: (overrides: LoadoutOverride[]) => void; + onApply: (selection: SessionLoadoutSelection) => void; onCancel: () => void; requestRender: () => void; }) { diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index a32928b83..c42f1fcee 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -75,12 +75,7 @@ import type { import { FooterDataProvider, type ReadonlyFooterDataProvider } from "../../core/footer-data-provider.ts"; import { configureHttpDispatcher, formatHttpIdleTimeoutMs } from "../../core/http-dispatcher.ts"; import { type AppKeybinding, KeybindingsManager } from "../../core/keybindings.ts"; -import { - appendSessionLoadout, - getSessionLoadout, - type LoadoutOverride, - loadoutOverridesEqual, -} from "../../core/loadout.ts"; +import { appendSessionLoadout, getSessionLoadout, loadoutOverridesEqual } from "../../core/loadout.ts"; import { createCompactionSummaryMessage } from "../../core/messages.ts"; import { defaultModelPerProvider, @@ -132,7 +127,10 @@ import { OAuthSelectorComponent, } from "./components/oauth-selector.ts"; import { ScopedModelsSelectorComponent } from "./components/scoped-models-selector.ts"; -import { SessionLoadoutSelectorComponent } from "./components/session-loadout-selector.ts"; +import { + type SessionLoadoutSelection, + SessionLoadoutSelectorComponent, +} from "./components/session-loadout-selector.ts"; import { SessionSelectorComponent } from "./components/session-selector.ts"; import { SettingsSelectorComponent } from "./components/settings-selector.ts"; import { SkillInvocationMessageComponent } from "./components/skill-invocation-message.ts"; @@ -4179,7 +4177,7 @@ export class InteractiveMode { } } - private async applySessionLoadout(overrides: LoadoutOverride[], persist: boolean): Promise { + private async applySessionLoadout(selection: SessionLoadoutSelection, persist: boolean): Promise { if (this.loadoutActionBlocked("applying")) return; const loader = this.getLoadoutLoader(); if (!loader) { @@ -4187,13 +4185,13 @@ export class InteractiveMode { return; } const previousOverrides = loader.getLoadoutSnapshot().overrides; - loader.setLoadoutOverrides(overrides); + loader.setLoadoutOverrides(selection.overrides); if (!(await this.handleReloadCommand())) { loader.setLoadoutOverrides(previousOverrides); return; } - if (persist && !loadoutOverridesEqual(previousOverrides, overrides)) { - appendSessionLoadout(this.sessionManager, overrides); + if (persist && (selection.explicitReset || !loadoutOverridesEqual(previousOverrides, selection.overrides))) { + appendSessionLoadout(this.sessionManager, selection.overrides); } this.showLoadoutDiagnostics(); } @@ -4210,10 +4208,10 @@ export class InteractiveMode { snapshot: loader.getLoadoutSnapshot(), agentDir: this.runtimeHost.services.agentDir, terminalHeight: this.ui.terminal.rows, - onApply: (overrides) => { + onApply: (selection) => { if (this.loadoutActionBlocked("applying")) return; done(); - void this.applySessionLoadout(overrides, true); + void this.applySessionLoadout(selection, true); }, onCancel: () => { done(); @@ -4241,7 +4239,7 @@ export class InteractiveMode { "Continue with the last session's extensions, skills, prompts, and themes? Missing resources will be skipped.", ); if (!accepted) return; - await this.applySessionLoadout(saved.overrides, false); + await this.applySessionLoadout({ overrides: saved.overrides, explicitReset: false }, false); } private showSettingsSelector(): void { diff --git a/packages/coding-agent/test/session-loadout-selector.test.ts b/packages/coding-agent/test/session-loadout-selector.test.ts index 1554c985a..c2db6345f 100644 --- a/packages/coding-agent/test/session-loadout-selector.test.ts +++ b/packages/coding-agent/test/session-loadout-selector.test.ts @@ -76,10 +76,13 @@ describe("SessionLoadoutSelectorComponent", () => { list.handleInput!("\n"); expect(onApply).toHaveBeenCalledOnce(); - expect(onApply.mock.calls[0]?.[0]).toEqual([ - { reference: missingPromptReference, enabled: true }, - { reference: extensionReference, enabled: false }, - ]); + expect(onApply.mock.calls[0]?.[0]).toEqual({ + overrides: [ + { reference: missingPromptReference, enabled: true }, + { reference: extensionReference, enabled: false }, + ], + explicitReset: false, + }); }); it("discards with Escape and emits no staged changes", () => { @@ -119,7 +122,7 @@ describe("SessionLoadoutSelectorComponent", () => { list.handleInput!(" "); list.handleInput!("\n"); - expect(onApply).toHaveBeenCalledWith([]); + expect(onApply).toHaveBeenCalledWith({ overrides: [], explicitReset: true }); expect(stripAnsi(selector.render(100).join("\n"))).toContain("[x] Use persistent settings"); }); @@ -142,7 +145,27 @@ describe("SessionLoadoutSelectorComponent", () => { list.handleInput!(" "); list.handleInput!("\n"); - expect(onApply).toHaveBeenCalledWith([{ reference: extensionReference, enabled: false }]); + expect(onApply).toHaveBeenCalledWith({ + overrides: [{ reference: extensionReference, enabled: false }], + explicitReset: true, + }); + }); + + it("distinguishes applying an unchanged empty loadout from an explicit reset", () => { + const snapshot = createSnapshot(); + snapshot.overrides = []; + const onApply = vi.fn(); + const selector = new SessionLoadoutSelectorComponent({ + snapshot, + agentDir: "/agent", + onApply, + onCancel: () => {}, + requestRender: () => {}, + }); + + selector.getResourceList().handleInput!("\n"); + + expect(onApply).toHaveBeenCalledWith({ overrides: [], explicitReset: false }); }); }); diff --git a/packages/coding-agent/test/suite/session-loadout-interactive.test.ts b/packages/coding-agent/test/suite/session-loadout-interactive.test.ts index 4db830dd4..4713bd8a5 100644 --- a/packages/coding-agent/test/suite/session-loadout-interactive.test.ts +++ b/packages/coding-agent/test/suite/session-loadout-interactive.test.ts @@ -5,6 +5,7 @@ import { type LoadoutOverride, type LoadoutSnapshot, } from "../../src/core/loadout.ts"; +import type { SessionLoadoutSelection } from "../../src/modes/interactive/components/session-loadout-selector.ts"; import { InteractiveMode } from "../../src/modes/interactive/interactive-mode.ts"; import { createHarness, type Harness } from "./harness.ts"; @@ -21,7 +22,7 @@ type RestoreContext = { sessionManager: Harness["sessionManager"]; getLoadoutRestoreDecisionKey: () => string; showExtensionConfirm: (title: string, message: string) => Promise; - applySessionLoadout: (overrides: LoadoutOverride[], persist: boolean) => Promise; + applySessionLoadout: (selection: SessionLoadoutSelection, persist: boolean) => Promise; }; type ApplyContext = { @@ -44,7 +45,7 @@ type ReloadBlockedContext = { type InteractiveModePrivate = { maybeRestoreSavedLoadout(this: RestoreContext): Promise; - applySessionLoadout(this: ApplyContext, overrides: LoadoutOverride[], persist: boolean): Promise; + applySessionLoadout(this: ApplyContext, selection: SessionLoadoutSelection, persist: boolean): Promise; loadoutActionBlocked(this: Pick, action: "opening" | "applying"): boolean; handleReloadCommand(this: ReloadBlockedContext): Promise; }; @@ -81,7 +82,7 @@ describe("interactive session loadout restore", () => { await interactiveModePrototype.maybeRestoreSavedLoadout.call(context); expect(showExtensionConfirm).toHaveBeenCalledOnce(); - expect(applySessionLoadout).toHaveBeenCalledWith(overrides, false); + expect(applySessionLoadout).toHaveBeenCalledWith({ overrides, explicitReset: false }, false); expect(harness.sessionManager.getEntries()).toHaveLength(entryCount); }); @@ -165,7 +166,7 @@ describe("interactive /loadout application", () => { showWarning, }; - await interactiveModePrototype.applySessionLoadout.call(context, overrides, true); + await interactiveModePrototype.applySessionLoadout.call(context, { overrides, explicitReset: false }, true); expect(setLoadoutOverrides).toHaveBeenCalledWith(overrides); expect(handleReloadCommand).toHaveBeenCalledOnce(); @@ -173,9 +174,10 @@ describe("interactive /loadout application", () => { expect(showWarning).toHaveBeenCalledWith("Saved loadout skill is unavailable"); }); - it("does not write a reset marker when the declined saved loadout is already inactive", async () => { + it("writes a reset tombstone when reset is explicit after a declined restore", async () => { const harness = await createTestHarness(); appendSessionLoadout(harness.sessionManager, overrides); + const entryCount = harness.sessionManager.getEntries().length; const setLoadoutOverrides = vi.fn(); const context: ApplyContext = { session: { isStreaming: false, isCompacting: false, isBashRunning: false }, @@ -190,9 +192,35 @@ describe("interactive /loadout application", () => { showWarning: vi.fn(), }; - await interactiveModePrototype.applySessionLoadout.call(context, [], true); + await interactiveModePrototype.applySessionLoadout.call(context, { overrides: [], explicitReset: true }, true); - expect(setLoadoutOverrides).toHaveBeenCalledOnce(); + expect(setLoadoutOverrides).toHaveBeenCalledWith([]); + expect(harness.sessionManager.getEntries()).toHaveLength(entryCount + 1); + expect(getSessionLoadout(harness.sessionManager)?.overrides).toEqual([]); + }); + + it("does not write a reset tombstone for an unchanged apply after a declined restore", async () => { + const harness = await createTestHarness(); + appendSessionLoadout(harness.sessionManager, overrides); + const entryCount = harness.sessionManager.getEntries().length; + const setLoadoutOverrides = vi.fn(); + const context: ApplyContext = { + session: { isStreaming: false, isCompacting: false, isBashRunning: false }, + sessionManager: harness.sessionManager, + getLoadoutLoader: () => ({ + getLoadoutSnapshot: () => ({ resources: [], overrides: [], diagnostics: [] }), + setLoadoutOverrides, + }), + loadoutActionBlocked: () => false, + handleReloadCommand: async () => true, + showLoadoutDiagnostics: () => {}, + showWarning: vi.fn(), + }; + + await interactiveModePrototype.applySessionLoadout.call(context, { overrides: [], explicitReset: false }, true); + + expect(setLoadoutOverrides).toHaveBeenCalledWith([]); + expect(harness.sessionManager.getEntries()).toHaveLength(entryCount); expect(getSessionLoadout(harness.sessionManager)?.overrides).toEqual(overrides); }); @@ -212,7 +240,7 @@ describe("interactive /loadout application", () => { showWarning: vi.fn(), }; - await interactiveModePrototype.applySessionLoadout.call(context, overrides, true); + await interactiveModePrototype.applySessionLoadout.call(context, { overrides, explicitReset: false }, true); expect(setLoadoutOverrides).toHaveBeenNthCalledWith(1, overrides); expect(setLoadoutOverrides).toHaveBeenNthCalledWith(2, []); @@ -244,7 +272,7 @@ describe("interactive /loadout application", () => { showWarning, }; - await interactiveModePrototype.applySessionLoadout.call(context, overrides, true); + await interactiveModePrototype.applySessionLoadout.call(context, { overrides, explicitReset: false }, true); expect(setLoadoutOverrides).not.toHaveBeenCalled(); expect(handleReloadCommand).not.toHaveBeenCalled();