From 5731f016c2200eb870c5b2c73b6baefc44151b29 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Tue, 18 Aug 2026 17:12:24 +0200 Subject: [PATCH] feat(app): default to steering when sending into a running agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Steer is the better default for Enter while an agent is running, but flipping DEFAULT_CLIENT_SETTINGS alone would only reach new installs. Client defaults are materialized into storage — the first load writes the whole default blob and every save re-stamps it — so every existing install already has "interrupt" on disk, and a stored "interrupt" cannot be told apart from one the user actually chose. A one-time migration flips them all; picking "interrupt" afterwards sticks. The applied-migration marker sits in its own storage key rather than in the settings blob: StoredAppSettingsSchema is strict and readValidatedJson clears the value when parsing fails, so an unknown field there would wipe every setting for anyone who downgrades. migrateAppSettings owns both of its writes so the settings blob always lands before the marker. A failed marker write leaves the migration to re-run harmlessly; a failed settings write must leave the marker unwritten, or the migration is lost and the user keeps "interrupt" for good. --- .../browser/agent-message-submission.spec.ts | 58 +++++++++- .../e2e/support/helpers/agent-message-gate.ts | 4 + packages/app/src/composer/input/input.tsx | 9 +- packages/app/src/hooks/use-settings/keys.ts | 13 +++ .../src/hooks/use-settings/migrations.test.ts | 103 ++++++++++++++++++ .../app/src/hooks/use-settings/migrations.ts | 41 +++++++ .../src/hooks/use-settings/storage.test.ts | 31 ++++++ .../app/src/hooks/use-settings/storage.ts | 69 +++++++----- 8 files changed, 292 insertions(+), 36 deletions(-) create mode 100644 packages/app/src/hooks/use-settings/keys.ts create mode 100644 packages/app/src/hooks/use-settings/migrations.test.ts create mode 100644 packages/app/src/hooks/use-settings/migrations.ts 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)