From 2ffbea20d2f66855c90f0f9791fa78bd038ffc46 Mon Sep 17 00:00:00 2001 From: openperf <16864032@qq.com> Date: Sat, 23 May 2026 16:02:06 +0800 Subject: [PATCH] fix(agents): drop stale exec approval followups after session rebind MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exec approval followups were dispatched by sessionKey only. When /new or /reset rotates the sessionId under that key while an approval is pending, the resolved followup landed in the new session, surfacing stale approval output (or 'Exec denied' / continuation text) in a fresh conversation. Capture the session UUID active when the approval is requested and drop the followup once the key has been rebound to a different sessionId: - agent-run followups: carry the expected id on the agent request and drop it at the gateway as an early preflight, before the handler touches the rebound session (session-store write, chat/agent run + active-run registration, dedupe, accepted ack) — not just before model dispatch. Covers elevated and non-elevated. - denied / direct fallback followups: resolve the key's current sessionId from the session store and drop before the channel send. Fixes #59349. --- .../OpenClawProtocol/GatewayModels.swift | 4 + packages/gateway-protocol/src/schema/agent.ts | 1 + src/agents/agent-tools.ts | 2 + ...bash-tools.exec-approval-followup-state.ts | 15 +++ .../bash-tools.exec-approval-followup.test.ts | 124 ++++++++++++++++++ .../bash-tools.exec-approval-followup.ts | 82 +++++++++++- src/agents/bash-tools.exec-host-gateway.ts | 6 + src/agents/bash-tools.exec-host-node.ts | 2 + src/agents/bash-tools.exec-host-node.types.ts | 4 + .../bash-tools.exec-host-shared.test.ts | 50 +++++++ src/agents/bash-tools.exec-host-shared.ts | 10 ++ src/agents/bash-tools.exec-types.ts | 8 ++ src/agents/bash-tools.exec.ts | 4 + src/gateway/server-methods/agent.test.ts | 57 ++++++++ src/gateway/server-methods/agent.ts | 40 ++++++ 15 files changed, 408 insertions(+), 1 deletion(-) diff --git a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift index 60578baa9dea..5a004c506a04 100644 --- a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift +++ b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift @@ -765,6 +765,7 @@ public struct AgentParams: Codable, Sendable { public let bootstrapcontextrunkind: AnyCodable? public let acpturnsource: String? public let internalruntimehandoffid: String? + public let execapprovalfollowupexpectedsessionid: String? public let internalevents: [[String: AnyCodable]]? public let inputprovenance: [String: AnyCodable]? public let suppresspromptpersistence: Bool? @@ -806,6 +807,7 @@ public struct AgentParams: Codable, Sendable { bootstrapcontextrunkind: AnyCodable?, acpturnsource: String?, internalruntimehandoffid: String?, + execapprovalfollowupexpectedsessionid: String?, internalevents: [[String: AnyCodable]]?, inputprovenance: [String: AnyCodable]?, suppresspromptpersistence: Bool?, @@ -846,6 +848,7 @@ public struct AgentParams: Codable, Sendable { self.bootstrapcontextrunkind = bootstrapcontextrunkind self.acpturnsource = acpturnsource self.internalruntimehandoffid = internalruntimehandoffid + self.execapprovalfollowupexpectedsessionid = execapprovalfollowupexpectedsessionid self.internalevents = internalevents self.inputprovenance = inputprovenance self.suppresspromptpersistence = suppresspromptpersistence @@ -888,6 +891,7 @@ public struct AgentParams: Codable, Sendable { case bootstrapcontextrunkind = "bootstrapContextRunKind" case acpturnsource = "acpTurnSource" case internalruntimehandoffid = "internalRuntimeHandoffId" + case execapprovalfollowupexpectedsessionid = "execApprovalFollowupExpectedSessionId" case internalevents = "internalEvents" case inputprovenance = "inputProvenance" case suppresspromptpersistence = "suppressPromptPersistence" diff --git a/packages/gateway-protocol/src/schema/agent.ts b/packages/gateway-protocol/src/schema/agent.ts index 2fdb3258240e..d4a9dac1d669 100644 --- a/packages/gateway-protocol/src/schema/agent.ts +++ b/packages/gateway-protocol/src/schema/agent.ts @@ -219,6 +219,7 @@ export const AgentParamsSchema = Type.Object( ), acpTurnSource: Type.Optional(Type.Literal("manual_spawn")), internalRuntimeHandoffId: Type.Optional(NonEmptyString), + execApprovalFollowupExpectedSessionId: Type.Optional(NonEmptyString), internalEvents: Type.Optional(Type.Array(AgentInternalEventSchema)), inputProvenance: Type.Optional(InputProvenanceSchema), suppressPromptPersistence: Type.Optional(Type.Boolean()), diff --git a/src/agents/agent-tools.ts b/src/agents/agent-tools.ts index 4b1492d4bdcc..a409d5516dff 100644 --- a/src/agents/agent-tools.ts +++ b/src/agents/agent-tools.ts @@ -824,6 +824,8 @@ export function createOpenClawCodingTools(options?: { allowBackground, scopeKey, sessionKey: options?.sessionKey, + sessionId: options?.sessionId, + sessionStore: options?.config?.session?.store, mainKey: options?.config?.session?.mainKey, sessionScope: options?.config?.session?.scope, eventRouting: resolveEventSessionRoutingPolicy({ diff --git a/src/agents/bash-tools.exec-approval-followup-state.ts b/src/agents/bash-tools.exec-approval-followup-state.ts index d938d3895dfe..4f58a680fed9 100644 --- a/src/agents/bash-tools.exec-approval-followup-state.ts +++ b/src/agents/bash-tools.exec-approval-followup-state.ts @@ -169,6 +169,21 @@ export function consumeExecApprovalFollowupRuntimeHandoff(params: { return cloneExecApprovalFollowupRuntimeHandoff(entry); } +/** + * A persisted exec-approval followup is stale when the session key it targeted + * has since been rebound to a different session id (via `/new` or `/reset`). + * Delivering it would leak the old approval result into the new session, so the + * gateway drops the followup instead of resuming the rebound session. + */ +export function isExecApprovalFollowupSessionRebound(params: { + expectedSessionId?: string; + resolvedSessionId?: string; +}): boolean { + const expected = normalizeOptionalString(params.expectedSessionId); + const resolved = normalizeOptionalString(params.resolvedSessionId); + return Boolean(expected && resolved && expected !== resolved); +} + /** Clear exec approval follow-up handoffs between tests. */ export function resetExecApprovalFollowupRuntimeHandoffsForTests(): void { execApprovalFollowupRuntimeHandoffs.clear(); diff --git a/src/agents/bash-tools.exec-approval-followup.test.ts b/src/agents/bash-tools.exec-approval-followup.test.ts index 3bc2cd96f53d..17be53cd07e6 100644 --- a/src/agents/bash-tools.exec-approval-followup.test.ts +++ b/src/agents/bash-tools.exec-approval-followup.test.ts @@ -13,6 +13,15 @@ vi.mock("../infra/outbound/message.js", () => ({ sendMessage: vi.fn(async () => ({ ok: true })), })); +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { + closeSqliteSessionStoreDatabase, + replaceSqliteSessionStore, +} from "../config/sessions/store-sqlite.js"; +import { clearSessionStoreCacheForTest } from "../config/sessions/store.js"; +import type { SessionEntry } from "../config/sessions/types.js"; import { sendMessage } from "../infra/outbound/message.js"; import { buildExecApprovalFollowupPrompt, @@ -20,8 +29,36 @@ import { } from "./bash-tools.exec-approval-followup.js"; import { callGatewayTool } from "./tools/gateway.js"; +const tempStoreDirs: string[] = []; +const tempStorePaths: string[] = []; + +// Seed the same SQLite-backed session store path the runtime reads; mocking this +// boundary would hide stale-session regressions in shared workers. +function writeTempSessionStore(entries: Record): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "exec-approval-followup-store-")); + tempStoreDirs.push(dir); + const storePath = path.join(dir, "sessions.json"); + tempStorePaths.push(storePath); + replaceSqliteSessionStore(storePath, entries as Record); + clearSessionStoreCacheForTest(); + return storePath; +} + afterEach(() => { vi.resetAllMocks(); + clearSessionStoreCacheForTest(); + while (tempStorePaths.length > 0) { + const storePath = tempStorePaths.pop(); + if (storePath) { + closeSqliteSessionStoreDatabase(storePath); + } + } + while (tempStoreDirs.length > 0) { + const dir = tempStoreDirs.pop(); + if (dir) { + fs.rmSync(dir, { recursive: true, force: true }); + } + } }); function requireRecord(value: unknown, label: string): Record { @@ -118,6 +155,93 @@ describe("exec approval followup", () => { expect(sendMessage).not.toHaveBeenCalled(); }); + it("forwards the approval-time session id so the gateway can drop stale followups", async () => { + await sendExecApprovalFollowup({ + approvalId: "req-pin-59349", + sessionKey: "agent:main:main", + expectedSessionId: "session-original", + resultText: "Exec completed: echo ok", + }); + + expectGatewayAgentFollowup({ + sessionKey: "agent:main:main", + execApprovalFollowupExpectedSessionId: "session-original", + }); + }); + + it("omits the expected session id when none was captured", async () => { + await sendExecApprovalFollowup({ + approvalId: "req-no-pin", + sessionKey: "agent:main:main", + resultText: "Exec completed: echo ok", + }); + + const params = expectGatewayAgentFollowup({ sessionKey: "agent:main:main" }); + expect(params).not.toHaveProperty("execApprovalFollowupExpectedSessionId"); + }); + + it("drops a denied direct followup when the session key was rebound by /new or /reset", async () => { + const sessionStore = writeTempSessionStore({ + "agent:main:main": { sessionId: "session-after-reset" }, + }); + + const result = await sendExecApprovalFollowup({ + approvalId: "req-denied-rebound", + sessionKey: "agent:main:main", + expectedSessionId: "session-original", + sessionStore, + direct: true, + turnSourceChannel: "telegram", + turnSourceTo: "-100123", + resultText: "Exec denied (gateway id=req-denied-rebound, user-denied): uname -a", + }); + + expect(result).toBe(false); + expect(sendMessage).not.toHaveBeenCalled(); + expect(callGatewayTool).not.toHaveBeenCalled(); + }); + + it("delivers a denied direct followup when the key still resolves to the approval-time session", async () => { + const sessionStore = writeTempSessionStore({ + "agent:main:main": { sessionId: "session-original" }, + }); + + await sendExecApprovalFollowup({ + approvalId: "req-denied-same", + sessionKey: "agent:main:main", + expectedSessionId: "session-original", + sessionStore, + direct: true, + turnSourceChannel: "telegram", + turnSourceTo: "-100123", + resultText: "Exec denied (gateway id=req-denied-same, user-denied): uname -a", + }); + + expect(sendMessage).toHaveBeenCalled(); + expect(callGatewayTool).not.toHaveBeenCalled(); + }); + + it("drops a non-denied direct fallback when the session key was rebound", async () => { + const sessionStore = writeTempSessionStore({ + "agent:main:main": { sessionId: "session-after-reset" }, + }); + + const result = await sendExecApprovalFollowup({ + approvalId: "req-finished-rebound", + sessionKey: "agent:main:main", + expectedSessionId: "session-original", + sessionStore, + direct: true, + turnSourceChannel: "telegram", + turnSourceTo: "-100123", + resultText: "Exec finished (gateway id=req-finished-rebound, code 0)\nok", + }); + + expect(result).toBe(false); + expect(sendMessage).not.toHaveBeenCalled(); + expect(callGatewayTool).not.toHaveBeenCalled(); + }); + it("routes denied followups through the originating main session", async () => { await sendExecApprovalFollowup({ approvalId: "req-denied-main", diff --git a/src/agents/bash-tools.exec-approval-followup.ts b/src/agents/bash-tools.exec-approval-followup.ts index c4bcc298c20e..2b33cc386a40 100644 --- a/src/agents/bash-tools.exec-approval-followup.ts +++ b/src/agents/bash-tools.exec-approval-followup.ts @@ -7,14 +7,21 @@ import { normalizeLowercaseStringOrEmpty, normalizeOptionalString, } from "@openclaw/normalization-core/string-coerce"; +import { resolveStorePath } from "../config/sessions/paths.js"; +import { loadSessionStore } from "../config/sessions/store-load.js"; import { resolveExternalBestEffortDeliveryTarget, type ExternalBestEffortDeliveryTarget, } from "../infra/outbound/best-effort-delivery.js"; import { sendMessage } from "../infra/outbound/message.js"; +import { createSubsystemLogger } from "../logging/subsystem.js"; +import { resolveAgentIdFromSessionKey } from "../routing/session-key.js"; import { isCronSessionKey, isSubagentSessionKey } from "../sessions/session-key-utils.js"; import { isGatewayMessageChannel, normalizeMessageChannel } from "../utils/message-channel.js"; -import { buildExecApprovalFollowupIdempotencyKey } from "./bash-tools.exec-approval-followup-state.js"; +import { + buildExecApprovalFollowupIdempotencyKey, + isExecApprovalFollowupSessionRebound, +} from "./bash-tools.exec-approval-followup-state.js"; import { sanitizeUserFacingText } from "./embedded-agent-helpers/sanitize-user-facing-text.js"; import { formatExecDeniedUserMessage, @@ -23,9 +30,17 @@ import { } from "./exec-approval-result.js"; import { callGatewayTool } from "./tools/gateway.js"; +const log = createSubsystemLogger("agents/exec-approval-followup"); + type ExecApprovalFollowupParams = { approvalId: string; sessionKey?: string; + /** Session UUID active when the approval was requested. Carried to the gateway + * so a followup whose session key was rebound by /new or /reset is dropped. */ + expectedSessionId?: string; + /** `session.store` template, used by the direct/denied path to resolve the + * key's current sessionId and drop a rebound followup before sending. */ + sessionStore?: string; turnSourceChannel?: string; turnSourceTo?: string; turnSourceAccountId?: string; @@ -91,6 +106,41 @@ function shouldSuppressExecDeniedFollowup(sessionKey: string | undefined): boole return isSubagentSessionKey(sessionKey) || isCronSessionKey(sessionKey); } +/** + * Direct/denied followups bypass the gateway agent dispatch, so the gateway + * rebind guard never sees them. Resolve the session key's current sessionId + * from the on-disk store and report whether it was rebound away from the + * approval-time session by `/new` or `/reset` (#59349). Failure to resolve is + * treated as "not rebound" so a real result is never suppressed by accident. + */ +function isExecApprovalFollowupDirectDeliveryStale(params: { + sessionKey: string | undefined; + expectedSessionId: string | undefined; + sessionStore: string | undefined; +}): boolean { + const sessionKey = normalizeOptionalString(params.sessionKey); + const expectedSessionId = normalizeOptionalString(params.expectedSessionId); + if (!sessionKey || !expectedSessionId) { + return false; + } + try { + const storePath = resolveStorePath(normalizeOptionalString(params.sessionStore), { + agentId: resolveAgentIdFromSessionKey(sessionKey), + }); + const resolvedSessionId = normalizeOptionalString( + loadSessionStore(storePath)?.[sessionKey]?.sessionId, + ); + return isExecApprovalFollowupSessionRebound({ expectedSessionId, resolvedSessionId }); + } catch (err) { + // Fail open: if the session store can't be resolved we deliver rather than + // risk dropping a real followup, but log it so this rare path is observable. + log.debug( + `exec approval followup session-rebind check skipped for ${sessionKey}; delivering: ${formatUnknownError(err)}`, + ); + return false; + } +} + function formatDirectExecApprovalFollowupText( resultText: string, opts: { allowDenied?: boolean } = {}, @@ -199,6 +249,7 @@ function canDirectSendDeniedFollowup(sessionError: unknown): boolean { function buildAgentFollowupArgs(params: { approvalId: string; sessionKey: string; + expectedSessionId?: string; resultText: string; deliveryTarget: ExternalBestEffortDeliveryTarget; sessionOnlyOriginChannel?: string; @@ -240,6 +291,9 @@ function buildAgentFollowupArgs(params: { buildExecApprovalFollowupIdempotencyKey({ approvalId: params.approvalId, }), + ...(params.expectedSessionId + ? { execApprovalFollowupExpectedSessionId: params.expectedSessionId } + : {}), ...(params.internalRuntimeHandoffId ? { internalRuntimeHandoffId: params.internalRuntimeHandoffId } : {}), @@ -310,6 +364,7 @@ export async function sendExecApprovalFollowup( const agentArgs = buildAgentFollowupArgs({ approvalId: params.approvalId, sessionKey, + expectedSessionId: params.expectedSessionId, resultText, deliveryTarget, sessionOnlyOriginChannel, @@ -341,6 +396,18 @@ export async function sendExecApprovalFollowup( } if (isDenied) { + if ( + isExecApprovalFollowupDirectDeliveryStale({ + sessionKey, + expectedSessionId: params.expectedSessionId, + sessionStore: params.sessionStore, + }) + ) { + log.info( + `Dropping stale denied exec approval followup ${params.approvalId}: session ${sessionKey ?? ""} was rebound before the approval resolved`, + ); + return false; + } if ( await sendDirectFollowupFallback({ approvalId: params.approvalId, @@ -358,6 +425,19 @@ export async function sendExecApprovalFollowup( return false; } + if ( + isExecApprovalFollowupDirectDeliveryStale({ + sessionKey, + expectedSessionId: params.expectedSessionId, + sessionStore: params.sessionStore, + }) + ) { + log.info( + `Dropping stale exec approval followup ${params.approvalId} direct fallback: session ${sessionKey ?? ""} was rebound before the approval resolved`, + ); + return false; + } + if ( await sendDirectFollowupFallback({ approvalId: params.approvalId, diff --git a/src/agents/bash-tools.exec-host-gateway.ts b/src/agents/bash-tools.exec-host-gateway.ts index d21f956aa01c..21ac600744ba 100644 --- a/src/agents/bash-tools.exec-host-gateway.ts +++ b/src/agents/bash-tools.exec-host-gateway.ts @@ -82,6 +82,10 @@ export type ProcessGatewayAllowlistParams = { trigger?: string; agentId?: string; sessionKey?: string; + /** Session UUID active when the approval was requested; pins the followup. */ + sessionId?: string; + /** Session-store template, so the direct/denied followup can detect a rebind. */ + sessionStore?: string; bashElevated?: ExecElevatedDefaults; turnSourceChannel?: string; turnSourceTo?: string; @@ -702,6 +706,8 @@ export async function processGatewayAllowlist( const followupTarget = buildExecApprovalFollowupTarget({ approvalId, sessionKey: params.notifySessionKey ?? params.sessionKey, + expectedSessionId: params.sessionId, + sessionStore: params.sessionStore, bashElevated: params.bashElevated, turnSourceChannel: params.turnSourceChannel, turnSourceTo: params.turnSourceTo, diff --git a/src/agents/bash-tools.exec-host-node.ts b/src/agents/bash-tools.exec-host-node.ts index cd7a0026625e..b4339f459e53 100644 --- a/src/agents/bash-tools.exec-host-node.ts +++ b/src/agents/bash-tools.exec-host-node.ts @@ -306,6 +306,8 @@ export async function executeNodeHostCommand( const followupTarget = execHostShared.buildExecApprovalFollowupTarget({ approvalId, sessionKey: params.notifySessionKey ?? params.sessionKey, + expectedSessionId: params.sessionId, + sessionStore: params.sessionStore, bashElevated: params.bashElevated, turnSourceChannel: params.turnSourceChannel, turnSourceTo: params.turnSourceTo, diff --git a/src/agents/bash-tools.exec-host-node.types.ts b/src/agents/bash-tools.exec-host-node.types.ts index a3bca6db386f..ddeaf55f88e8 100644 --- a/src/agents/bash-tools.exec-host-node.types.ts +++ b/src/agents/bash-tools.exec-host-node.types.ts @@ -16,6 +16,10 @@ export type ExecuteNodeHostCommandParams = { requestedNode?: string; boundNode?: string; sessionKey?: string; + /** Session UUID active when the approval was requested; pins the followup. */ + sessionId?: string; + /** Session-store template, so the direct/denied followup can detect a rebind. */ + sessionStore?: string; bashElevated?: ExecElevatedDefaults; turnSourceChannel?: string; turnSourceTo?: string; diff --git a/src/agents/bash-tools.exec-host-shared.test.ts b/src/agents/bash-tools.exec-host-shared.test.ts index f89f8e4ecc4f..4ff6c23e19ae 100644 --- a/src/agents/bash-tools.exec-host-shared.test.ts +++ b/src/agents/bash-tools.exec-host-shared.test.ts @@ -7,6 +7,7 @@ import { MAX_DATE_TIMESTAMP_MS } from "@openclaw/normalization-core/number-coerc import { beforeEach, describe, expect, it, vi } from "vitest"; import { consumeExecApprovalFollowupRuntimeHandoff, + isExecApprovalFollowupSessionRebound, registerExecApprovalFollowupRuntimeHandoff, resetExecApprovalFollowupRuntimeHandoffsForTests, } from "./bash-tools.exec-approval-followup-state.js"; @@ -133,6 +134,7 @@ describe("sendExecApprovalFollowupResult", () => { internalRuntimeHandoffId?: string; idempotencyKey?: string; execApprovalFollowupToken?: string; + expectedSessionId?: string; bashElevated?: unknown; } | undefined { @@ -141,6 +143,7 @@ describe("sendExecApprovalFollowupResult", () => { internalRuntimeHandoffId?: string; idempotencyKey?: string; execApprovalFollowupToken?: string; + expectedSessionId?: string; bashElevated?: unknown; } | undefined; @@ -304,6 +307,53 @@ describe("sendExecApprovalFollowupResult", () => { expect(call).not.toHaveProperty("idempotencyKey"); expect(call).not.toHaveProperty("bashElevated"); }); + + it("forwards the approval-time session id to the followup dispatch (non-elevated)", async () => { + sendExecApprovalFollowup.mockResolvedValue(true); + + await sendExecApprovalFollowupResult( + { + approvalId: "approval-session-pin-59349", + sessionKey: "agent:main:telegram:direct:123", + expectedSessionId: "session-original", + turnSourceChannel: "telegram", + }, + "Exec finished", + { sendExecApprovalFollowup, logWarn }, + ); + + expect(firstExecApprovalFollowupCall()?.expectedSessionId).toBe("session-original"); + }); +}); + +describe("isExecApprovalFollowupSessionRebound", () => { + it("flags a rebound session when the resolved id differs from the approval-time id", () => { + expect( + isExecApprovalFollowupSessionRebound({ + expectedSessionId: "session-original", + resolvedSessionId: "session-after-reset", + }), + ).toBe(true); + }); + + it("keeps the followup when the session id is unchanged", () => { + expect( + isExecApprovalFollowupSessionRebound({ + expectedSessionId: "session-original", + resolvedSessionId: "session-original", + }), + ).toBe(false); + }); + + it("does not drop when either session id is missing", () => { + expect(isExecApprovalFollowupSessionRebound({ resolvedSessionId: "session-after-reset" })).toBe( + false, + ); + expect(isExecApprovalFollowupSessionRebound({ expectedSessionId: "session-original" })).toBe( + false, + ); + expect(isExecApprovalFollowupSessionRebound({})).toBe(false); + }); }); describe("resolveExecHostApprovalContext", () => { diff --git a/src/agents/bash-tools.exec-host-shared.ts b/src/agents/bash-tools.exec-host-shared.ts index 48f341b96bbe..32ba67d30cb5 100644 --- a/src/agents/bash-tools.exec-host-shared.ts +++ b/src/agents/bash-tools.exec-host-shared.ts @@ -101,6 +101,12 @@ export type RegisteredExecApprovalRequestContext = { export type ExecApprovalFollowupTarget = { approvalId: string; sessionKey?: string; + /** Session UUID active when the approval was requested. Lets the followup be + * dropped if `/new` or `/reset` rebinds the session key to a new session. */ + expectedSessionId?: string; + /** Session-store template, so the direct/denied path can resolve the key's + * current sessionId and drop a rebound followup before sending. */ + sessionStore?: string; turnSourceChannel?: string; turnSourceTo?: string; turnSourceAccountId?: string; @@ -352,6 +358,8 @@ export function buildExecApprovalFollowupTarget( return { approvalId: params.approvalId, sessionKey: params.sessionKey, + expectedSessionId: params.expectedSessionId, + sessionStore: params.sessionStore, turnSourceChannel: params.turnSourceChannel, turnSourceTo: params.turnSourceTo, turnSourceAccountId: params.turnSourceAccountId, @@ -457,6 +465,8 @@ export async function sendExecApprovalFollowupResult( await send({ approvalId: target.approvalId, sessionKey: target.sessionKey, + expectedSessionId: target.expectedSessionId, + sessionStore: target.sessionStore, turnSourceChannel: target.turnSourceChannel, turnSourceTo: target.turnSourceTo, turnSourceAccountId: target.turnSourceAccountId, diff --git a/src/agents/bash-tools.exec-types.ts b/src/agents/bash-tools.exec-types.ts index 25c20ff6cdce..7cb4cc8cfe1e 100644 --- a/src/agents/bash-tools.exec-types.ts +++ b/src/agents/bash-tools.exec-types.ts @@ -50,6 +50,14 @@ export type ExecToolDefaults = { allowBackground?: boolean; scopeKey?: string; sessionKey?: string; + /** Ephemeral session UUID active when this exec tool was built. Regenerated + * on `/new` and `/reset`, so it pins exec-approval followups to the original + * session instance and lets stale followups drop after a session rebind. */ + sessionId?: string; + /** `session.store` template from the runtime config. Lets the direct/denied + * exec approval followup path resolve the session key's current sessionId and + * drop the followup when the key was rebound by `/new` or `/reset`. */ + sessionStore?: string; /** `session.mainKey` from the runtime config; passed through into * runExecProcess so background-exit notifications can remap cron-run * session keys to the agent's main queue without an ambient config load. */ diff --git a/src/agents/bash-tools.exec.ts b/src/agents/bash-tools.exec.ts index 907587b0e25f..80de98af15cb 100644 --- a/src/agents/bash-tools.exec.ts +++ b/src/agents/bash-tools.exec.ts @@ -1758,6 +1758,8 @@ export function createExecTool( requestedNode: params.node?.trim(), boundNode: defaults?.node?.trim(), sessionKey: defaults?.sessionKey, + sessionId: defaults?.sessionId, + sessionStore: defaults?.sessionStore, bashElevated: elevatedDefaults, turnSourceChannel: defaults?.messageProvider, turnSourceTo: defaults?.currentChannelId, @@ -1806,6 +1808,8 @@ export function createExecTool( trigger: defaults?.trigger, agentId, sessionKey: defaults?.sessionKey, + sessionId: defaults?.sessionId, + sessionStore: defaults?.sessionStore, bashElevated: elevatedDefaults, turnSourceChannel: defaults?.messageProvider, turnSourceTo: defaults?.currentChannelId, diff --git a/src/gateway/server-methods/agent.test.ts b/src/gateway/server-methods/agent.test.ts index 729e89b16de0..acc11d3b1dbc 100644 --- a/src/gateway/server-methods/agent.test.ts +++ b/src/gateway/server-methods/agent.test.ts @@ -3005,6 +3005,63 @@ describe("gateway agent handler", () => { }); }); + it("drops a stale exec approval followup at preflight without touching the rebound session (#59349)", async () => { + const bashElevated = { + enabled: true, + allowed: true, + defaultLevel: "on" as const, + }; + const registration = registerExecApprovalFollowupRuntimeHandoff({ + approvalId: "req-rebound-followup", + sessionKey: "agent:main:telegram:direct:123", + bashElevated, + }); + if (!registration) { + throw new Error("expected runtime handoff id"); + } + // Session was rebound by /new or /reset: current sessionId differs from the + // approval-time sessionId carried on the request. + mockMainSessionEntry({ + sessionId: "current-session-after-reset", + lastChannel: "telegram", + lastTo: "123", + }); + const context = makeContext(); + const updateSessionStoreCallsBefore = mocks.updateSessionStore.mock.calls.length; + const agentCommandCallsBefore = mocks.agentCommand.mock.calls.length; + + const respond = await invokeAgent( + { + message: "exec followup", + sessionKey: "agent:main:telegram:direct:123", + channel: "telegram", + idempotencyKey: registration.idempotencyKey, + internalRuntimeHandoffId: registration.handoffId, + execApprovalFollowupExpectedSessionId: "approval-time-session-id", + }, + { + reqId: "exec-followup-rebound-drop", + client: backendGatewayClient(), + context, + flushDispatch: false, + }, + ); + + expect(mockCallArg(respond, 0, 1)).toMatchObject({ + runId: registration.idempotencyKey, + status: "ok", + summary: expect.stringContaining("exec approval followup dropped"), + }); + expect(mocks.updateSessionStore.mock.calls.length).toBe(updateSessionStoreCallsBefore); + expect(mocks.agentCommand.mock.calls.length).toBe(agentCommandCallsBefore); + const dedupeEntry = context.dedupe.get("agent:exec-approval-followup:req-rebound-followup"); + expect(dedupeEntry?.ok).toBe(true); + expect(dedupeEntry?.payload).toMatchObject({ + status: "ok", + summary: expect.stringContaining("exec approval followup dropped"), + }); + }); + it("does not honor caller-supplied exec approval runtime handoff ids without registry state", async () => { mockMainSessionEntry({ sessionId: "existing-session-id", diff --git a/src/gateway/server-methods/agent.ts b/src/gateway/server-methods/agent.ts index 481bb1fc2710..bc80da497403 100644 --- a/src/gateway/server-methods/agent.ts +++ b/src/gateway/server-methods/agent.ts @@ -28,6 +28,7 @@ import { listAgentIds, resolveDefaultAgentId } from "../../agents/agent-scope.js import { resolveTrustedGroupId } from "../../agents/agent-tools.policy.js"; import { consumeExecApprovalFollowupRuntimeHandoff, + isExecApprovalFollowupSessionRebound, parseExecApprovalFollowupApprovalId, } from "../../agents/bash-tools.exec-approval-followup-state.js"; import { clearAllCliSessions } from "../../agents/cli-session.js"; @@ -1061,6 +1062,7 @@ export const agentHandlers: GatewayRequestHandlers = { bootstrapContextRunKind?: "default" | "heartbeat" | "cron"; acpTurnSource?: "manual_spawn"; internalRuntimeHandoffId?: string; + execApprovalFollowupExpectedSessionId?: string; internalEvents?: AgentInternalEvent[]; suppressPromptPersistence?: boolean; sessionEffects?: "visible" | "internal"; @@ -1300,6 +1302,44 @@ export const agentHandlers: GatewayRequestHandlers = { agentId = inferredAgentId; } } + // Drop an exec-approval followup whose session key was rebound by /new or + // /reset while the approval was pending, before the handler touches the + // rebound session (store write, run registration, dedupe, accepted ack). + if (execApprovalFollowupApprovalId && requestedSessionKeyRaw) { + const expectedSessionId = normalizeOptionalString( + request.execApprovalFollowupExpectedSessionId, + ); + let currentSessionId: string | undefined; + try { + currentSessionId = normalizeOptionalString( + loadSessionEntry(requestedSessionKeyRaw).entry?.sessionId, + ); + } catch { + currentSessionId = undefined; + } + if ( + isExecApprovalFollowupSessionRebound({ + expectedSessionId, + resolvedSessionId: currentSessionId, + }) + ) { + context.logGateway.info( + `Dropping stale exec approval followup ${execApprovalFollowupApprovalId}: session ${requestedSessionKeyRaw} rebound (expected ${expectedSessionId}, current ${currentSessionId}) before the approval resolved`, + ); + const droppedPayload = { + runId, + status: "ok" as const, + summary: "exec approval followup dropped: session was reset before the approval resolved", + }; + setGatewayDedupeEntries({ + dedupe: context.dedupe, + keys: agentDedupeKeys, + entry: { ts: Date.now(), ok: true, payload: droppedPayload }, + }); + respond(true, droppedPayload, undefined, { runId }); + return; + } + } const requestedSessionId = normalizeOptionalString(request.sessionId); let requestedSessionKey = requestedSessionKeyRaw ??