diff --git a/packages/app/e2e/browser/agent-message-submission.spec.ts b/packages/app/e2e/browser/agent-message-submission.spec.ts index 16a112dc4..4c17a0383 100644 --- a/packages/app/e2e/browser/agent-message-submission.spec.ts +++ b/packages/app/e2e/browser/agent-message-submission.spec.ts @@ -302,13 +302,29 @@ async function configureSteerInSettings(page: Page): Promise { } async function selectSteerInSettings(page: Page): Promise { - await page.getByRole("button", { name: "Steer", exact: true }).click(); + await selectSendBehaviorInSettings(page, "Steer", "steer"); +} + +/** Steer is the default, so the interrupt path only gets exercised by opting back into it. */ +async function configureInterruptInSettings(page: Page): Promise { + const modifier = process.platform === "darwin" ? "Meta" : "Control"; + await page.keyboard.press(`${modifier}+Comma`); + await expect(page).toHaveURL(/\/settings\/general$/); + await selectSendBehaviorInSettings(page, "Interrupt", "interrupt"); +} + +async function selectSendBehaviorInSettings( + page: Page, + buttonName: string, + stored: string, +): Promise { + await page.getByRole("button", { name: buttonName, exact: true }).click(); await expect .poll(async () => { const raw = await page.evaluate(() => localStorage.getItem("@paseo:app-settings")); return raw ? (JSON.parse(raw) as { sendBehavior?: unknown }).sendBehavior : null; }) - .toBe("steer"); + .toBe(stored); } async function replaySteeredSleepTurnInBrowser( @@ -932,9 +948,9 @@ test.describe("Agent message submission", () => { gate.holdNextClientRequest("send_agent_message_request"); await fillComposerDraft(page, "Replace the running turn without duplicating its action."); - await expect( - page.getByRole("button", { name: "Send and interrupt", exact: true }), - ).toHaveCount(1); + await expect(page.getByRole("button", { name: "Send and steer", exact: true })).toHaveCount( + 1, + ); await expect(page.getByRole("button", { name: "Stop agent", exact: true })).toHaveCount(0); await expect(page.getByRole("button", { name: "Interrupt agent", exact: true })).toHaveCount( 0, @@ -1161,6 +1177,38 @@ test.describe("Agent message submission", () => { } }); + test("sends interrupt behavior on the wire when the user opts out of steering", async ({ + page, + }, testInfo) => { + test.setTimeout(120_000); + const gate = await gateNextAgentMessage(page); + const agent = await startRunningMockAgent(page, { + prefix: `interrupt-submission-${testInfo.workerIndex}-`, + model: "one-minute-stream", + prompt: "Keep this turn active until the user interrupts it.", + }); + try { + await configureInterruptInSettings(page); + await page.goBack(); + await expectComposerVisible(page); + await expectAgentReadyToInterrupt(page); + + const prompt = "Interrupt the running turn."; + await fillComposerDraft(page, prompt); + await expect( + page.getByRole("button", { name: "Send and interrupt", exact: true }), + ).toHaveCount(1); + await composerLocator(page).press("Enter"); + + const request = await gate.waitForRequest(); + expect(request.activeTurnBehavior).toBe("interrupt"); + gate.accept(); + await expect(page.getByTestId("user-message").filter({ hasText: prompt })).toHaveCount(1); + } finally { + await agent.cleanup(); + } + }); + test("replays Claude-shaped steering inside one active turn", async ({ page }, testInfo) => { test.setTimeout(120_000); await replaySteeredSleepTurnInBrowser(page, testInfo, "claude"); diff --git a/packages/app/e2e/support/helpers/agent-message-gate.ts b/packages/app/e2e/support/helpers/agent-message-gate.ts index 866d25770..7444fee79 100644 --- a/packages/app/e2e/support/helpers/agent-message-gate.ts +++ b/packages/app/e2e/support/helpers/agent-message-gate.ts @@ -7,6 +7,8 @@ interface SendAgentMessageRequest { type: "send_agent_message_request"; requestId: string; agentId: string; + /** Absent when the client sends into an idle agent. */ + activeTurnBehavior?: string; } function readSendRequest(message: WebSocketMessage): SendAgentMessageRequest | null { @@ -28,6 +30,8 @@ function readSendRequest(message: WebSocketMessage): SendAgentMessageRequest | n type: "send_agent_message_request", requestId: request.requestId, agentId: request.agentId, + activeTurnBehavior: + typeof request.activeTurnBehavior === "string" ? request.activeTurnBehavior : undefined, }; } catch { return null; diff --git a/packages/app/src/composer/input/input.tsx b/packages/app/src/composer/input/input.tsx index c0c087e38..3aa877d18 100644 --- a/packages/app/src/composer/input/input.tsx +++ b/packages/app/src/composer/input/input.tsx @@ -147,9 +147,10 @@ export interface MessageInputProps { voiceAgentId?: string; /** When true and there's sendable content, calls onQueue instead of onSubmit */ isAgentRunning?: boolean; - /** Controls what the default send action (Enter, send button, dictation) does - * when the agent is running. "interrupt" sends immediately, "queue" queues. */ - defaultSendBehavior?: "interrupt" | "steer" | "queue"; + /** Controls what the default send action (Enter, send button, dictation) does when the agent is + * running. "interrupt" and "steer" send immediately, "queue" queues. Required so the default + * lives only in DEFAULT_CLIENT_SETTINGS. */ + defaultSendBehavior: "interrupt" | "steer" | "queue"; /** Callback for queue button when agent is running */ onQueue?: (payload: MessagePayload) => void; /** Optional handler used when submit button is in loading state. */ @@ -1132,7 +1133,7 @@ function resolveMessageInputProps(props: MessageInputProps): ResolvedMessageInpu voiceServerId: props.voiceServerId, voiceAgentId: props.voiceAgentId, isAgentRunning: props.isAgentRunning ?? false, - defaultSendBehavior: props.defaultSendBehavior ?? "interrupt", + defaultSendBehavior: props.defaultSendBehavior, onQueue: props.onQueue, onSubmitLoadingPress: props.onSubmitLoadingPress, onKeyPressCallback: props.onKeyPress, diff --git a/packages/app/src/hooks/use-settings/keys.ts b/packages/app/src/hooks/use-settings/keys.ts new file mode 100644 index 000000000..5483ef10b --- /dev/null +++ b/packages/app/src/hooks/use-settings/keys.ts @@ -0,0 +1,13 @@ +/** Storage keys live here so `storage` and `migrations` can share them without importing each other. */ + +export const APP_SETTINGS_KEY = "@paseo:app-settings"; +export const LEGACY_SETTINGS_KEY = "@paseo:settings"; + +/** + * The applied-migration marker deliberately sits outside the settings blob: + * `StoredAppSettingsSchema` is a strict object and `readValidatedJson` clears the value when + * parsing fails, so a new field there would wipe every setting for anyone who downgrades to a + * client that predates it. Keeping the marker as a list of ids also means the next migration is + * a new entry rather than a schema change. + */ +export const SETTINGS_MIGRATIONS_KEY = "@paseo:settings-migrations"; diff --git a/packages/app/src/hooks/use-settings/migrations.test.ts b/packages/app/src/hooks/use-settings/migrations.test.ts new file mode 100644 index 000000000..5c4e5b1f8 --- /dev/null +++ b/packages/app/src/hooks/use-settings/migrations.test.ts @@ -0,0 +1,103 @@ +import { describe, expect, it } from "vitest"; +import { createInMemoryKeyValueStorage } from "./fakes"; +import { APP_SETTINGS_KEY, SETTINGS_MIGRATIONS_KEY } from "./keys"; +import { migrateAppSettings } from "./migrations"; +import { DEFAULT_CLIENT_SETTINGS, type AppSettings, type SendBehavior } from "./storage"; + +function settingsWith(sendBehavior: SendBehavior): AppSettings { + return { ...DEFAULT_CLIENT_SETTINGS, sendBehavior }; +} + +type Storage = ReturnType; + +function appliedIds(storage: Storage): string[] { + const raw = storage.entries.get(SETTINGS_MIGRATIONS_KEY); + return raw === undefined ? [] : JSON.parse(raw).applied; +} + +function storedSendBehavior(storage: Storage): SendBehavior | undefined { + const raw = storage.entries.get(APP_SETTINGS_KEY); + return raw === undefined ? undefined : JSON.parse(raw).sendBehavior; +} + +/** An in-memory storage whose write to `failingKey` always throws, as a full disk would. */ +function createFailingWriteStorage(failingKey: string): Storage { + const storage = createInMemoryKeyValueStorage(); + const setItem = storage.setItem.bind(storage); + return Object.assign(storage, { + async setItem(key: string, value: string) { + if (key === failingKey) throw new Error(`write to ${key} failed`); + await setItem(key, value); + }, + }); +} + +describe("migrateAppSettings", () => { + it("flips a stored interrupt to steer and marks itself applied", async () => { + const storage = createInMemoryKeyValueStorage(); + + const result = await migrateAppSettings(settingsWith("interrupt"), storage); + + expect(result.sendBehavior).toBe("steer"); + expect(storedSendBehavior(storage)).toBe("steer"); + expect(appliedIds(storage)).toEqual(["steer-default"]); + }); + + it("leaves interrupt alone once the migration has run", async () => { + const storage = createInMemoryKeyValueStorage(); + await migrateAppSettings(settingsWith("interrupt"), storage); + + const result = await migrateAppSettings(settingsWith("interrupt"), storage); + + expect(result.sendBehavior).toBe("interrupt"); + }); + + it("leaves queue alone", async () => { + const storage = createInMemoryKeyValueStorage(); + + const result = await migrateAppSettings(settingsWith("queue"), storage); + + expect(result.sendBehavior).toBe("queue"); + expect(storage.entries.has(APP_SETTINGS_KEY)).toBe(false); + expect(appliedIds(storage)).toEqual(["steer-default"]); + }); + + it("marks itself applied on a fresh install without rewriting settings", async () => { + const storage = createInMemoryKeyValueStorage(); + + await migrateAppSettings(settingsWith("steer"), storage); + + expect(storage.entries.has(APP_SETTINGS_KEY)).toBe(false); + expect(appliedIds(storage)).toEqual(["steer-default"]); + }); + + it("keeps unknown migration ids written by a newer client", async () => { + const storage = createInMemoryKeyValueStorage({ + [SETTINGS_MIGRATIONS_KEY]: JSON.stringify({ applied: ["some-later-migration"] }), + }); + + await migrateAppSettings(settingsWith("interrupt"), storage); + + expect(appliedIds(storage)).toEqual(["some-later-migration", "steer-default"]); + }); + + it("stays unmarked when the settings write fails, so a later launch retries", async () => { + const storage = createFailingWriteStorage(APP_SETTINGS_KEY); + + await expect(migrateAppSettings(settingsWith("interrupt"), storage)).rejects.toThrow(); + + expect(appliedIds(storage)).toEqual([]); + }); + + it("re-runs harmlessly when the marker write fails after settings landed", async () => { + const failing = createFailingWriteStorage(SETTINGS_MIGRATIONS_KEY); + await expect(migrateAppSettings(settingsWith("interrupt"), failing)).rejects.toThrow(); + expect(storedSendBehavior(failing)).toBe("steer"); + + const recovered = createInMemoryKeyValueStorage(Object.fromEntries(failing.entries)); + const result = await migrateAppSettings(settingsWith("steer"), recovered); + + expect(result.sendBehavior).toBe("steer"); + expect(appliedIds(recovered)).toEqual(["steer-default"]); + }); +}); diff --git a/packages/app/src/hooks/use-settings/migrations.ts b/packages/app/src/hooks/use-settings/migrations.ts new file mode 100644 index 000000000..4ec632f4d --- /dev/null +++ b/packages/app/src/hooks/use-settings/migrations.ts @@ -0,0 +1,41 @@ +import { z } from "zod"; +import { readValidatedJson } from "@/storage/validated-storage"; +import { APP_SETTINGS_KEY, SETTINGS_MIGRATIONS_KEY } from "./keys"; +import type { AppSettings, KeyValueStorage } from "./storage"; + +const AppliedMigrationsSchema = z.strictObject({ applied: z.array(z.string()) }); + +/** + * `sendBehavior` defaulted to "interrupt" for the months before steering existed, and defaults + * are materialized into storage on first load, so a stored "interrupt" cannot be told apart from + * a deliberate one. This flips every stored "interrupt" to "steer" exactly once; picking + * "interrupt" afterwards sticks. + */ +const STEER_DEFAULT_MIGRATION = "steer-default"; + +/** + * Brings stored settings up to date, returning what the caller should use. Owns both writes so + * the marker can only ever be written after the settings it describes: a failed marker write + * leaves the migration to re-run harmlessly, while a failed settings write must leave the marker + * unwritten or the migration is lost for good. + */ +export async function migrateAppSettings( + settings: AppSettings, + storage: KeyValueStorage, +): Promise { + const stored = await readValidatedJson(storage, SETTINGS_MIGRATIONS_KEY, AppliedMigrationsSchema); + const applied = new Set(stored?.applied ?? []); + if (applied.has(STEER_DEFAULT_MIGRATION)) { + return settings; + } + + const migrated: AppSettings = + settings.sendBehavior === "interrupt" ? { ...settings, sendBehavior: "steer" } : settings; + if (migrated !== settings) { + await storage.setItem(APP_SETTINGS_KEY, JSON.stringify(migrated)); + } + + applied.add(STEER_DEFAULT_MIGRATION); + await storage.setItem(SETTINGS_MIGRATIONS_KEY, JSON.stringify({ applied: [...applied] })); + return migrated; +} diff --git a/packages/app/src/hooks/use-settings/storage.test.ts b/packages/app/src/hooks/use-settings/storage.test.ts index d0eb29bb9..c3be7aa3a 100644 --- a/packages/app/src/hooks/use-settings/storage.test.ts +++ b/packages/app/src/hooks/use-settings/storage.test.ts @@ -48,6 +48,37 @@ describe("loadAppSettingsFromStorage", () => { }); expect((await loadAppSettingsFromStorage(deps)).sendBehavior).toBe("steer"); }); + it("migrates a stored interrupt to steer and persists it", async () => { + const deps = makeDeps({ + storage: createInMemoryKeyValueStorage({ + [APP_SETTINGS_KEY]: JSON.stringify({ sendBehavior: "interrupt" }), + }), + }); + + const result = await loadAppSettingsFromStorage(deps); + + expect(result.sendBehavior).toBe("steer"); + expect(JSON.parse(deps.storage.entries.get(APP_SETTINGS_KEY) ?? "{}").sendBehavior).toBe( + "steer", + ); + }); + + it("keeps an interrupt the user picked after the migration ran", async () => { + const deps = makeDeps({ + storage: createInMemoryKeyValueStorage({ + [APP_SETTINGS_KEY]: JSON.stringify({ sendBehavior: "interrupt" }), + }), + }); + await loadAppSettingsFromStorage(deps); + await saveAppSettings({ + queryClient: new QueryClient(), + updates: { sendBehavior: "interrupt" }, + deps, + }); + + expect((await loadAppSettingsFromStorage(deps)).sendBehavior).toBe("interrupt"); + }); + it("defaults theme to auto when storage is empty", async () => { const deps = makeDeps(); diff --git a/packages/app/src/hooks/use-settings/storage.ts b/packages/app/src/hooks/use-settings/storage.ts index b60ac1938..ebca947a1 100644 --- a/packages/app/src/hooks/use-settings/storage.ts +++ b/packages/app/src/hooks/use-settings/storage.ts @@ -18,10 +18,11 @@ import { isNative } from "@/constants/platform"; import { FONT_SIZE, THEME_OPTIONS, type ThemePreference } from "@/styles/theme"; import { z } from "zod"; import { readValidatedJson } from "@/storage/validated-storage"; +import { APP_SETTINGS_KEY, LEGACY_SETTINGS_KEY } from "./keys"; +import { migrateAppSettings } from "./migrations"; -export const APP_SETTINGS_KEY = "@paseo:app-settings"; +export { APP_SETTINGS_KEY } from "./keys"; export const APP_SETTINGS_QUERY_KEY = ["app-settings"]; -const LEGACY_SETTINGS_KEY = "@paseo:settings"; export type SendBehavior = ActiveTurnBehavior | "queue"; export type ReleaseChannel = "stable" | "beta"; @@ -135,7 +136,7 @@ type StoredAppSettings = z.infer; export const DEFAULT_CLIENT_SETTINGS: AppSettings = { theme: "auto", language: "system", - sendBehavior: "interrupt", + sendBehavior: "steer", serviceUrlBehavior: "ask", terminalScrollbackLines: DEFAULT_TERMINAL_SCROLLBACK_LINES, useLegacyTerminalRenderer: false, @@ -196,37 +197,51 @@ export async function saveAppSettings(input: { export async function loadAppSettingsFromStorage(deps: SettingsDeps): Promise { try { - const stored = await readValidatedJson(deps.storage, APP_SETTINGS_KEY, StoredAppSettingsSchema); - if (stored) { - const normalized = normalizeAppSettings(stored); - if (stored.uiBaseFontSize === undefined && stored.uiFontSize !== undefined) { - await deps.storage.setItem(APP_SETTINGS_KEY, JSON.stringify(normalized)); - } - return normalized; + const read = await readAppSettings(deps); + if (read.needsWrite) { + await deps.storage.setItem(APP_SETTINGS_KEY, JSON.stringify(read.settings)); } - - const legacyStored = await readValidatedJson( - deps.storage, - LEGACY_SETTINGS_KEY, - LegacyRendererSettingsSchema, - ); - if (legacyStored) { - const next = { - ...DEFAULT_CLIENT_SETTINGS, - ...pickAppSettingsFromLegacy(legacyStored), - } satisfies AppSettings; - await deps.storage.setItem(APP_SETTINGS_KEY, JSON.stringify(next)); - return next; - } - - await deps.storage.setItem(APP_SETTINGS_KEY, JSON.stringify(DEFAULT_CLIENT_SETTINGS)); - return DEFAULT_CLIENT_SETTINGS; + return await migrateAppSettings(read.settings, deps.storage); } catch (error) { console.error("[AppSettings] Failed to load settings:", error); throw error; } } +/** + * Reads whichever of the settings blobs exists, without migrating. `needsWrite` covers the reads + * that produce settings the stored blob does not already spell out. + */ +async function readAppSettings( + deps: SettingsDeps, +): Promise<{ settings: AppSettings; needsWrite: boolean }> { + const stored = await readValidatedJson(deps.storage, APP_SETTINGS_KEY, StoredAppSettingsSchema); + if (stored) { + return { + settings: normalizeAppSettings(stored), + // COMPAT(uiFontSizeScale): persist the converted base size, remove after 2027-08-17. + needsWrite: stored.uiBaseFontSize === undefined && stored.uiFontSize !== undefined, + }; + } + + const legacyStored = await readValidatedJson( + deps.storage, + LEGACY_SETTINGS_KEY, + LegacyRendererSettingsSchema, + ); + if (legacyStored) { + return { + settings: { + ...DEFAULT_CLIENT_SETTINGS, + ...pickAppSettingsFromLegacy(legacyStored), + } satisfies AppSettings, + needsWrite: true, + }; + } + + return { settings: DEFAULT_CLIENT_SETTINGS, needsWrite: true }; +} + export async function loadSettingsFromStorage(deps: SettingsDeps): Promise { const legacyDesktopSettings = deps.desktop.isElectron() ? await loadLegacyDesktopSettingsFromStorage(deps.storage)