fix(acp): fall through to thread-bound resolution when token is unresolvable (#66299) (#74641)

* 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 <clawsweeper-repair@users.noreply.github.com>
This commit is contained in:
hcl
2026-04-30 12:24:21 +08:00
committed by GitHub
parent e648f38efc
commit 5716428adc
3 changed files with 93 additions and 6 deletions

View File

@@ -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.

View File

@@ -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([

View File

@@ -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,
});