From 5716428adc51f34e2f8a1580a8e1b41133033c0f Mon Sep 17 00:00:00 2001 From: hcl Date: Thu, 30 Apr 2026 12:24:21 +0800 Subject: [PATCH] fix(acp): fall through to thread-bound resolution when token is unresolvable (#66299) (#74641) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(acp): fall through to thread-bound resolution when token is unresolvable (#66299) resolveAcpTargetSessionKey returned an error immediately when an explicit session token was supplied but could not be resolved as a key/id/label. This blocked the thread-bound and requester-session fallback paths from ever being reached. Discord slash commands auto-fill the current thread ID as a positional ACP target. That value is not a session identifier, so the gateway lookup returns null, and the command returned 'Unable to resolve session target' instead of falling through to the thread-bound session that was already known via the binding context. Fix: when the token lookup returns null, skip the early-exit error and fall through to thread-bound → requester-session → error in the normal way. The 'Missing session key' error still surfaces when neither fallback produces a binding. Adds a focused regression test: unresolvable token + bound thread session → steer command reaches the thread-bound session, not an error. Fixes #66299 * fix(changelog): add Thanks @martingarramon attribution for #66299 Per clawsweeper P2 review — every new CHANGELOG entry must credit at least one author. martingarramon authored the issue analysis and explicitly invited the PR. * fix(acp): preserve bad-token diagnostics after thread fallback --------- Co-authored-by: clawsweeper-repair --- CHANGELOG.md | 1 + src/auto-reply/reply/commands-acp.test.ts | 79 ++++++++++++++++++++ src/auto-reply/reply/commands-acp/targets.ts | 19 +++-- 3 files changed, 93 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ba8db539ae..f0c181abd6d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,7 @@ Docs: https://docs.openclaw.ai - Plugins/runtime-deps: add `openclaw plugins deps` inspection and repair with script-free package-manager defaults shared across plugin installers, so operators can repair missing bundled runtime deps without corrupting JSON output or blocking unrelated conflict-free deps. Thanks @vincentkoc. - Agents/output: strip internal `[tool calls omitted]` replay placeholders from user-facing replies while preserving visible reply whitespace. Fixes #74573. Thanks @blaspat. - Providers/Google Vertex: route authorized_user ADC credentials through OpenClaw's REST transport so Docker installs using gcloud application-default credentials no longer crash in the Google SDK before requests are sent. Fixes #74628. Thanks @frankhal2001-design. +- ACP/resolver: fall through to thread-bound session resolution when an explicit `--session` token cannot be resolved while preserving the bad-token diagnostic when no thread binding exists, so Discord slash commands that auto-fill the current thread ID as the positional ACP target no longer return "Unable to resolve session target" errors. Fixes #66299. Thanks @hclsys, @kindomLee, and @martingarramon. - Agents/sessions: emit a terminal lifecycle backstop when embedded timeout/error turns return without `agent_end`, so Gateway sessions no longer stay stuck in `running` after failover surfaces a timeout. Fixes #74607. Thanks @millerc79. - Gateway/diagnostics: include stuck-session reason hints and recovery skip causes in warnings, so operators can tell whether a lane is waiting on active work, queued work, or stale bookkeeping. Thanks @vincentkoc. - Agents/Codex: bound embedded-run cleanup, trajectory flushing, and command-lane task timeouts after runtime failures, so Discord and other chat sessions return to idle instead of staying stuck in processing. Thanks @vincentkoc. diff --git a/src/auto-reply/reply/commands-acp.test.ts b/src/auto-reply/reply/commands-acp.test.ts index cd08849f580..d342d6a6999 100644 --- a/src/auto-reply/reply/commands-acp.test.ts +++ b/src/auto-reply/reply/commands-acp.test.ts @@ -1618,6 +1618,35 @@ describe("/acp command", () => { expect(hoisted.runTurnMock).not.toHaveBeenCalled(); }); + it("falls through to thread-bound resolution when explicit session token is unresolvable", async () => { + // callGateway returns null for sessions.resolve (unresolvable token) + // but a thread-bound session exists — should use thread-bound, not error out + hoisted.callGatewayMock.mockImplementation(async (request: { method?: string }) => { + if (request.method === "sessions.resolve") { + return null; // token lookup fails + } + return { ok: true }; + }); + mockBoundThreadSession(); + hoisted.readAcpSessionEntryMock.mockReturnValue(createAcpSessionEntry()); + hoisted.runTurnMock.mockImplementation(async function* () { + yield { type: "text_delta", text: "Steered." }; + yield { type: "done" }; + }); + + const result = await runThreadAcpCommand( + `/acp steer --session unresolvable-token-xyz tighten logging`, + ); + + expect(hoisted.runTurnMock).toHaveBeenCalledWith( + expect.objectContaining({ + mode: "steer", + sessionKey: defaultAcpSessionKey, + }), + ); + expect(result?.reply?.text).toContain("Steered."); + }); + it("closes an ACP session, unbinds thread targets, and clears metadata", async () => { mockBoundThreadSession(); hoisted.sessionBindingUnbindMock.mockResolvedValue([ @@ -1637,6 +1666,56 @@ describe("/acp command", () => { expect(result?.reply?.text).toContain("Removed 1 binding"); }); + it("closes the bound thread ACP session when an explicit session token is unresolvable", async () => { + hoisted.callGatewayMock.mockImplementation(async (request: { method?: string }) => { + if (request.method === "sessions.resolve") { + return null; + } + return { ok: true }; + }); + mockBoundThreadSession(); + hoisted.sessionBindingUnbindMock.mockResolvedValue([ + createBoundThreadSession() as SessionBindingRecord, + ]); + + const result = await runThreadAcpCommand("/acp close not-a-session-target"); + + expect(hoisted.closeMock).toHaveBeenCalledWith({ + cfg: baseCfg, + sessionKey: defaultAcpSessionKey, + reason: "manual-close", + allowBackendUnavailable: true, + clearMeta: true, + }); + expect(hoisted.sessionBindingUnbindMock).toHaveBeenCalledWith( + expect.objectContaining({ + targetSessionKey: defaultAcpSessionKey, + reason: "manual", + }), + ); + expect(result?.reply?.text).toContain(`Closed ACP session ${defaultAcpSessionKey}`); + }); + + it("reports an explicit bad ACP session token before requester fallback", async () => { + hoisted.callGatewayMock.mockImplementation(async (request: { method?: string }) => { + if (request.method === "sessions.resolve") { + return null; + } + return { ok: true }; + }); + const params = createConversationParams("/acp close not-a-session-target", { + channel: "discord", + originatingTo: "channel:parent-1", + sessionKey: "requester-session", + }); + + const result = await handleAcpCommand(params, true); + + expect(result?.reply?.text).toContain("Unable to resolve session target: not-a-session-target"); + expect(hoisted.closeMock).not.toHaveBeenCalled(); + expect(hoisted.readAcpSessionEntryMock).not.toHaveBeenCalled(); + }); + it("handles /acp close in a bound thread when text commands are disabled", async () => { mockBoundThreadSession(); hoisted.sessionBindingUnbindMock.mockResolvedValue([ diff --git a/src/auto-reply/reply/commands-acp/targets.ts b/src/auto-reply/reply/commands-acp/targets.ts index 310dd6704fb..5ecc2ed8c75 100644 --- a/src/auto-reply/reply/commands-acp/targets.ts +++ b/src/auto-reply/reply/commands-acp/targets.ts @@ -59,13 +59,13 @@ export async function resolveAcpTargetSessionKey(params: { const token = normalizeOptionalString(params.token) ?? ""; if (token) { const resolved = await resolveSessionKeyByToken(token); - if (!resolved) { - return { - ok: false, - error: `Unable to resolve session target: ${token}`, - }; + if (resolved) { + return { ok: true, sessionKey: resolved }; } - return { ok: true, sessionKey: resolved }; + // Token was supplied but could not be resolved as a session key/id/label. + // Fall through to thread-bound resolution so that callers that auto-fill + // the current thread ID as the token (e.g. Discord slash commands) still + // reach the correct session via the binding context. } const threadBound = resolveBoundAcpThreadSessionKey(params.commandParams); @@ -76,6 +76,13 @@ export async function resolveAcpTargetSessionKey(params: { }; } + if (token) { + return { + ok: false, + error: `Unable to resolve session target: ${token}`, + }; + } + const fallback = resolveRequesterSessionKey(params.commandParams, { preferCommandTarget: true, });