From 82a5368bbcb108f0ff4003e9541b2f8ca8a589c3 Mon Sep 17 00:00:00 2001 From: Shakker Date: Sun, 2 Aug 2026 14:56:32 +0100 Subject: [PATCH] fix: bind confirmed resets to gateway connection --- ui/src/pages/chat/chat-commands.test.ts | 93 +++++++++++++++++++++++-- ui/src/pages/chat/chat-commands.ts | 47 +++++++++++-- ui/src/pages/chat/chat-outbox-drain.ts | 25 +++++++ ui/src/pages/chat/chat-send-delivery.ts | 12 +++- ui/src/pages/chat/chat-send.test.ts | 39 +++++++++++ 5 files changed, 207 insertions(+), 9 deletions(-) diff --git a/ui/src/pages/chat/chat-commands.test.ts b/ui/src/pages/chat/chat-commands.test.ts index 7d717e35711c..435ab23f7c44 100644 --- a/ui/src/pages/chat/chat-commands.test.ts +++ b/ui/src/pages/chat/chat-commands.test.ts @@ -303,6 +303,9 @@ describe("conversation reset confirmation", () => { const sendResetMessage = vi.fn(async () => {}); const result = await dispatchChatSlashCommand( { + ...legacyConnectedSessionAccess(), + connectionEpoch: 1, + sessionKey: "agent:main:current", confirmConversationReset: vi.fn(async () => false), } as never, "reset", @@ -321,6 +324,8 @@ describe("conversation reset confirmation", () => { }); const sendResetMessage = vi.fn(async () => {}); const host = { + ...legacyConnectedSessionAccess(), + connectionEpoch: 1, sessionKey: "agent:main:first", confirmConversationReset: vi.fn(async () => await confirmation), }; @@ -335,6 +340,72 @@ describe("conversation reset confirmation", () => { expect(sendResetMessage).not.toHaveBeenCalled(); }); + it("does not send /reset through a replacement Gateway after confirmation", async () => { + let settleConfirmation: ((confirmed: boolean) => void) | undefined; + const confirmation = new Promise((resolve) => { + settleConfirmation = resolve; + }); + const sendResetMessage = vi.fn(async () => {}); + const host = { + client: { request: vi.fn() } as unknown as GatewayBrowserClient, + connected: true, + connectionEpoch: 1, + hello: { + auth: { role: "operator", scopes: ["operator.write"] }, + features: { methods: ["chat.send"] }, + } as ApplicationGatewaySnapshot["hello"], + sessionKey: "agent:main:current", + chatRunId: null, + confirmConversationReset: vi.fn(async () => await confirmation), + lastError: null as string | null, + chatError: null as string | null, + }; + + const pending = dispatchChatSlashCommand(host as never, "reset", "", { + sendResetMessage, + }); + host.client = { request: vi.fn() } as unknown as GatewayBrowserClient; + host.connectionEpoch += 1; + settleConfirmation?.(true); + + await expect(pending).resolves.toBe("failed"); + expect(sendResetMessage).not.toHaveBeenCalled(); + }); + + it("rechecks /reset write scope after confirmation", async () => { + let settleConfirmation: ((confirmed: boolean) => void) | undefined; + const confirmation = new Promise((resolve) => { + settleConfirmation = resolve; + }); + const sendResetMessage = vi.fn(async () => {}); + const host = { + ...legacyConnectedSessionAccess(), + connectionEpoch: 1, + hello: { + auth: { role: "operator", scopes: ["operator.write"] }, + features: { methods: ["chat.send"] }, + } as ApplicationGatewaySnapshot["hello"], + sessionKey: "agent:main:current", + chatRunId: null, + confirmConversationReset: vi.fn(async () => await confirmation), + lastError: null as string | null, + chatError: null as string | null, + }; + + const pending = dispatchChatSlashCommand(host as never, "reset", "", { + sendResetMessage, + }); + host.hello = { + auth: { role: "operator", scopes: ["operator.read"] }, + features: { methods: ["chat.send"] }, + } as ApplicationGatewaySnapshot["hello"]; + settleConfirmation?.(true); + + await expect(pending).resolves.toBe("failed"); + expect(sendResetMessage).not.toHaveBeenCalled(); + expect(host.lastError).toContain("operator.write"); + }); + it("continues /reset when the session key changes to an equivalent alias", async () => { let settleConfirmation: ((confirmed: boolean) => void) | undefined; const confirmation = new Promise((resolve) => { @@ -342,6 +413,8 @@ describe("conversation reset confirmation", () => { }); const sendResetMessage = vi.fn(async () => {}); const host = { + ...legacyConnectedSessionAccess(), + connectionEpoch: 1, sessionKey: "main", confirmConversationReset: vi.fn(async () => await confirmation), }; @@ -387,14 +460,26 @@ describe("conversation reset confirmation", () => { it("keeps chat-only /reset unchanged", async () => { const sendResetMessage = vi.fn(async () => {}); - const result = await dispatchChatSlashCommand({} as never, "reset", "now", { + const host = { + ...legacyConnectedSessionAccess(), + connectionEpoch: 1, + sessionKey: "agent:main:current", + }; + const result = await dispatchChatSlashCommand(host as never, "reset", "now", { sendResetMessage, }); expect(result).toBe("completed"); - expect(sendResetMessage).toHaveBeenCalledWith("/reset now", { - sendResetMessage, - }); + expect(sendResetMessage).toHaveBeenCalledWith( + "/reset now", + expect.objectContaining({ + target: expect.objectContaining({ + client: host.client, + connectionEpoch: 1, + sessionKey: "agent:main:current", + }), + }), + ); }); it("cancels /clear before resetting a board-bearing session", async () => { diff --git a/ui/src/pages/chat/chat-commands.ts b/ui/src/pages/chat/chat-commands.ts index d7c9ef6be357..cf3209565ab3 100644 --- a/ui/src/pages/chat/chat-commands.ts +++ b/ui/src/pages/chat/chat-commands.ts @@ -12,6 +12,7 @@ import { type SlashCommandDef, } from "../../lib/chat/commands.ts"; import { resolveCurrentUserIdentity } from "../../lib/chat/current-user-identity.ts"; +import { readSessionMethodAccess } from "../../lib/session-method-access.ts"; import { scopedAgentIdForSession, visibleSessionMatches, @@ -46,6 +47,7 @@ const remoteSlashCommandCache = new WeakMap< export type ChatCommandResetOptions = { previousDraft?: string; restoreDraft?: boolean; + target?: ChatCommandTarget; }; type ChatCommandSendOptions = ChatCommandResetOptions & { @@ -54,7 +56,7 @@ type ChatCommandSendOptions = ChatCommandResetOptions & { type ChatCommandDispatchResult = "completed" | "failed" | "uncertain" | "cancelled" | "deferred"; -type ChatCommandTarget = { +export type ChatCommandTarget = { client: GatewayBrowserClient; connectionEpoch: number; sessionKey: string; @@ -108,7 +110,7 @@ function requireChatSessionAction( return false; } -function captureChatCommandTarget(host: ChatCommandHost): ChatCommandTarget | null { +export function captureChatCommandTarget(host: ChatCommandHost): ChatCommandTarget | null { if (!host.client || !host.connected) { return null; } @@ -120,7 +122,10 @@ function captureChatCommandTarget(host: ChatCommandHost): ChatCommandTarget | nu }; } -function isChatCommandTargetCurrent(host: ChatCommandHost, target: ChatCommandTarget): boolean { +export function isChatCommandTargetCurrent( + host: ChatCommandHost, + target: ChatCommandTarget, +): boolean { return ( host.connected && host.client === target.client && @@ -129,6 +134,29 @@ function isChatCommandTargetCurrent(host: ChatCommandHost, target: ChatCommandTa ); } +export function readChatResetTargetAccess( + host: ChatCommandHost, + target: ChatCommandTarget, +): { allowed: true } | { allowed: false; reason: string } { + if (!isChatCommandTargetCurrent(host, target)) { + return { allowed: false, reason: "The Gateway connection changed. Retry the command." }; + } + const access = readSessionMethodAccess(currentSessionAccessSnapshot(host), { + method: "chat.send", + requiredScope: "operator.write", + }); + return access.allowed ? { allowed: true } : access; +} + +function requireChatResetTarget(host: ChatCommandHost, target: ChatCommandTarget): boolean { + const access = readChatResetTargetAccess(host, target); + if (access.allowed) { + return true; + } + setChatCommandError(host, access.reason); + return false; +} + function failStaleChatCommand(host: ChatCommandHost): ChatCommandDispatchResult { setChatCommandError(host, "The Gateway connection changed. Retry the command."); return "failed"; @@ -299,11 +327,22 @@ export async function dispatchChatSlashCommand( } return (await host.createChatSession()) ? "completed" : "cancelled"; case "reset": { + const target = captureChatCommandTarget(host); + if (!target || !requireChatResetTarget(host, target)) { + return "failed"; + } const confirmation = await confirmConversationResetForCurrentSession(host); if (confirmation !== "confirmed") { return confirmation; } - await opts.sendResetMessage(args ? `/reset ${args}` : "/reset", opts); + if (!requireChatResetTarget(host, target)) { + return "failed"; + } + await opts.sendResetMessage(args ? `/reset ${args}` : "/reset", { + previousDraft: opts.previousDraft, + restoreDraft: opts.restoreDraft, + target, + }); return "completed"; } case "clear": { diff --git a/ui/src/pages/chat/chat-outbox-drain.ts b/ui/src/pages/chat/chat-outbox-drain.ts index 121ee3f162db..5f0e510fce48 100644 --- a/ui/src/pages/chat/chat-outbox-drain.ts +++ b/ui/src/pages/chat/chat-outbox-drain.ts @@ -5,8 +5,11 @@ import { visibleSessionMatches } from "../../lib/sessions/index.ts"; import { isUiGlobalSessionKey } from "../../lib/sessions/session-key.ts"; import { releaseChatAttachmentPayloads } from "./attachment-payload-store.ts"; import { + captureChatCommandTarget, confirmConversationResetForCurrentSession, dispatchChatSlashCommand, + readChatResetTargetAccess, + type ChatCommandTarget, type ChatCommandResetOptions, } from "./chat-commands.ts"; import { loadChatHistory, type ChatHistoryResult, type ChatState } from "./chat-history.ts"; @@ -43,6 +46,7 @@ export type QueuedChatSendOptions = { restoreDraft?: boolean; routingSessionKey?: string; storageMode?: QueuedChatStorageMode; + target?: ChatCommandTarget; }; export type ChatOutboxDrainDependencies = { @@ -301,6 +305,17 @@ async function drainStoredChatOutbox( } syncVisibleChatQueueProjection(host); if (item.localCommandName === "reset") { + const resetTarget = captureChatCommandTarget(host); + if (!resetTarget) { + setCommandState("failed", "The Gateway connection changed. Retry the command."); + return "blocked"; + } + const initialAccess = readChatResetTargetAccess(host, resetTarget); + if (!initialAccess.allowed) { + setCommandState("failed", initialAccess.reason); + dependencies.setChatError(host, initialAccess.reason); + return "blocked"; + } const resetText = item.localCommandArgs ? `/reset ${item.localCommandArgs}` : "/reset"; const convertResetToMessage = (sendState?: ChatQueueItem["sendState"]) => updateQueuedMessageForSession(host, outbox.sessionKey, item.id, (entry) => ({ @@ -328,6 +343,16 @@ async function drainStoredChatOutbox( } continue; } + const currentAccess = readChatResetTargetAccess(host, resetTarget); + if (!currentAccess.allowed) { + setCommandState("failed", currentAccess.reason); + dependencies.setChatError(host, currentAccess.reason); + return "blocked"; + } + lane.pendingOptions.set(item.id, { + ...lane.pendingOptions.get(item.id), + target: resetTarget, + }); if (!convertResetToMessage()) { return "blocked"; } diff --git a/ui/src/pages/chat/chat-send-delivery.ts b/ui/src/pages/chat/chat-send-delivery.ts index 1af19d899dda..b0a3eaf9fbc6 100644 --- a/ui/src/pages/chat/chat-send-delivery.ts +++ b/ui/src/pages/chat/chat-send-delivery.ts @@ -6,7 +6,7 @@ import type { ChatQueueItem } from "../../lib/chat/chat-types.ts"; import { scopedAgentIdForSession, visibleSessionMatches } from "../../lib/sessions/index.ts"; import { generateUUID } from "../../lib/uuid.ts"; import { discardChatAttachmentDataUrls } from "./attachment-payload-store.ts"; -import type { ChatCommandResetOptions } from "./chat-commands.ts"; +import { readChatResetTargetAccess, type ChatCommandResetOptions } from "./chat-commands.ts"; import { loadChatBranches, loadChatHistory, type ChatState } from "./chat-history.ts"; import { flushStoredChatOutbox, @@ -256,6 +256,16 @@ async function sendQueuedChatMessage( } const sessionKey = prepared.sessionKey ?? host.sessionKey; const setState = deliveryStateWriter(host, storageMode, sessionKey, id); + if (options?.target) { + const access = readChatResetTargetAccess(host, options.target); + if (!access.allowed) { + setState("failed", access.reason); + if (visibleSessionMatches(host, sessionKey, prepared.agentId)) { + setChatError(host, access.reason); + } + return "failed"; + } + } if (prepared.skillWorkshopRevision && attachments.length) { setState("failed", "Skill Workshop revision requests do not support attachments."); return "failed"; diff --git a/ui/src/pages/chat/chat-send.test.ts b/ui/src/pages/chat/chat-send.test.ts index 77ca07eb3467..99f460ecdce0 100644 --- a/ui/src/pages/chat/chat-send.test.ts +++ b/ui/src/pages/chat/chat-send.test.ts @@ -4363,6 +4363,45 @@ describe("handleSendChat", () => { ]); }); + it("does not convert a queued reset after the Gateway connection changes", async () => { + const confirmation = createDeferred(); + const replacementRequest = makeRequestMock({ + "chat.send": () => ({ status: "ok" }), + }); + const item = createQueuedLocalCommand("queued-reset-reconnect", "/reset"); + const host = makeHost({ + requestHandlers: { + "chat.history": () => idleChatHistory(), + "chat.send": () => ({ status: "ok" }), + }, + chatQueue: [item], + connectionEpoch: 1, + confirmConversationReset: vi.fn(async () => await confirmation.promise), + hello: { + auth: { role: "operator", scopes: ["operator.write"] }, + features: { methods: ["chat.send"] }, + }, + }); + admitHostQueueItems(host); + + const draining = retryReconnectableQueuedChatSends(host); + await waitForFast(() => expect(host.confirmConversationReset).toHaveBeenCalledOnce()); + host.client = clientWithRequest(replacementRequest); + host.connectionEpoch = 2; + confirmation.resolve(true); + await draining; + + expect(host.request).not.toHaveBeenCalledWith("chat.send", expect.anything()); + expect(replacementRequest).not.toHaveBeenCalledWith("chat.send", expect.anything()); + expect(listStoredChatOutboxes(host)[0]?.queue[0]).toEqual( + expect.objectContaining({ + id: item.id, + localCommandName: "reset", + sendState: "failed", + }), + ); + }); + it("retires a queued local command without applying its late result after a route switch", async () => { const command = createDeferred>>(); executeSlashCommandMock.mockImplementationOnce(() => command.promise);