mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-06 01:55:37 +00:00
fix(agents): preserve literal current session resolution (#93138)
* fix(agents): resolve "current" session alias locally without gateway round-trip The system prompt tells agents to use sessionKey="current" to refer to their own session. Previously, resolveSessionReference sent the literal string "current" to the gateway sessions.resolve action, which rejected it with INVALID_REQUEST and logged a noisy error line on every tool call. The wrapper fell back to requesterInternalKey and succeeded, so the tool worked — but the gateway error was spurious. Add "current" to the well-known client alias check in resolveCurrentSessionClientAlias so it is resolved locally to the requester's session key, matching how TUI/CLI/WebChat client labels are handled. This eliminates the unnecessary gateway round-trip and the error log line. Fixes #78424 * test: update session_status tests for local current-key resolution * test: update session_status tests for local current-key resolution * Revert "test: update session_status tests for local current-key resolution" This reverts commit d9f6c8b5248921c99f43dc222667ffa429b34401. * Revert "test: update session_status tests for local current-key resolution" This reverts commit 40bf77d06711833c1beaeedf562b60a765a559d6. * Revert "fix(agents): resolve "current" session alias locally without gateway round-trip" This reverts commit d92bc9b91e0840ea5823cd44223c139e434c5ec4. * fix(agents): preserve literal current session resolution --------- Co-authored-by: Vincent Koc <25068+vincentkoc@users.noreply.github.com>
This commit is contained in:
@@ -1756,6 +1756,7 @@ public struct SessionsResolveParams: Codable, Sendable {
|
||||
public let spawnedby: String?
|
||||
public let includeglobal: Bool?
|
||||
public let includeunknown: Bool?
|
||||
public let allowmissing: Bool?
|
||||
|
||||
public init(
|
||||
key: String?,
|
||||
@@ -1764,7 +1765,8 @@ public struct SessionsResolveParams: Codable, Sendable {
|
||||
agentid: String? = nil,
|
||||
spawnedby: String?,
|
||||
includeglobal: Bool?,
|
||||
includeunknown: Bool?)
|
||||
includeunknown: Bool?,
|
||||
allowmissing: Bool? = nil)
|
||||
{
|
||||
self.key = key
|
||||
self.sessionid = sessionid
|
||||
@@ -1773,6 +1775,7 @@ public struct SessionsResolveParams: Codable, Sendable {
|
||||
self.spawnedby = spawnedby
|
||||
self.includeglobal = includeglobal
|
||||
self.includeunknown = includeunknown
|
||||
self.allowmissing = allowmissing
|
||||
}
|
||||
|
||||
private enum CodingKeys: String, CodingKey {
|
||||
@@ -1783,6 +1786,7 @@ public struct SessionsResolveParams: Codable, Sendable {
|
||||
case spawnedby = "spawnedBy"
|
||||
case includeglobal = "includeGlobal"
|
||||
case includeunknown = "includeUnknown"
|
||||
case allowmissing = "allowMissing"
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -232,6 +232,8 @@ export const SessionsResolveParamsSchema = Type.Object(
|
||||
spawnedBy: Type.Optional(NonEmptyString),
|
||||
includeGlobal: Type.Optional(Type.Boolean()),
|
||||
includeUnknown: Type.Optional(Type.Boolean()),
|
||||
/** Return a successful `{ ok: false }` response when the selector does not match a session. */
|
||||
allowMissing: Type.Optional(Type.Boolean()),
|
||||
},
|
||||
{ additionalProperties: false },
|
||||
);
|
||||
|
||||
@@ -56,6 +56,7 @@ const DEFAULTED_OPTIONAL_INIT_PARAM_ENTRIES: readonly [string, readonly string[]
|
||||
["SessionsResetParams", ["agentId"]],
|
||||
["SessionsDeleteParams", ["agentId"]],
|
||||
["SessionsCompactParams", ["agentId"]],
|
||||
["SessionsResolveParams", ["allowMissing"]],
|
||||
["SessionsUsageParams", ["agentId", "agentScope"]],
|
||||
["ChatHistoryParams", ["agentId"]],
|
||||
["ChatSendParams", ["agentId"]],
|
||||
|
||||
@@ -118,6 +118,9 @@ async function handleSessionsResolve(params: Record<string, unknown>) {
|
||||
if (!resolved.ok) {
|
||||
throw new Error(resolved.error.message);
|
||||
}
|
||||
if ("missing" in resolved) {
|
||||
return { ok: false };
|
||||
}
|
||||
return { ok: true, key: resolved.key };
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
// verification, and requester-spawned access checks.
|
||||
import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { OpenClawConfig } from "../../config/config.js";
|
||||
import { GatewayClientRequestError } from "../../gateway/client.js";
|
||||
const callGatewayMock = vi.fn();
|
||||
vi.mock("../../gateway/call.js", () => ({
|
||||
callGateway: (opts: unknown) => callGatewayMock(opts),
|
||||
@@ -270,6 +271,7 @@ describe("resolveSessionReference", () => {
|
||||
params: {
|
||||
key: "current",
|
||||
spawnedBy: undefined,
|
||||
allowMissing: true,
|
||||
},
|
||||
});
|
||||
});
|
||||
@@ -295,6 +297,7 @@ describe("resolveSessionReference", () => {
|
||||
params: {
|
||||
key: "current",
|
||||
spawnedBy: undefined,
|
||||
allowMissing: true,
|
||||
},
|
||||
});
|
||||
expect(callGatewayMock).toHaveBeenNthCalledWith(2, {
|
||||
@@ -304,10 +307,94 @@ describe("resolveSessionReference", () => {
|
||||
spawnedBy: undefined,
|
||||
includeGlobal: true,
|
||||
includeUnknown: true,
|
||||
allowMissing: true,
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("retries literal current probes without allowMissing for older gateways", async () => {
|
||||
const unsupportedAllowMissing = () =>
|
||||
new GatewayClientRequestError({
|
||||
code: "INVALID_REQUEST",
|
||||
message: "invalid sessions.resolve params: at root: unexpected property 'allowMissing'",
|
||||
});
|
||||
callGatewayMock
|
||||
.mockRejectedValueOnce(unsupportedAllowMissing())
|
||||
.mockRejectedValueOnce(
|
||||
new GatewayClientRequestError({
|
||||
code: "INVALID_REQUEST",
|
||||
message: "No session found: current",
|
||||
}),
|
||||
)
|
||||
.mockRejectedValueOnce(unsupportedAllowMissing())
|
||||
.mockResolvedValueOnce({ key: "agent:ops:main" });
|
||||
|
||||
const result = await resolveSessionReference({
|
||||
sessionKey: "current",
|
||||
alias: "main",
|
||||
mainKey: "main",
|
||||
requesterInternalKey: "agent:main:subagent:child",
|
||||
restrictToSpawned: false,
|
||||
});
|
||||
expectResolvedSessionReference(result, {
|
||||
key: "agent:ops:main",
|
||||
displayKey: "agent:ops:main",
|
||||
resolvedViaSessionId: true,
|
||||
});
|
||||
expect(callGatewayMock).toHaveBeenNthCalledWith(1, {
|
||||
method: "sessions.resolve",
|
||||
params: {
|
||||
key: "current",
|
||||
spawnedBy: undefined,
|
||||
allowMissing: true,
|
||||
},
|
||||
});
|
||||
expect(callGatewayMock).toHaveBeenNthCalledWith(2, {
|
||||
method: "sessions.resolve",
|
||||
params: {
|
||||
key: "current",
|
||||
spawnedBy: undefined,
|
||||
},
|
||||
});
|
||||
expect(callGatewayMock).toHaveBeenNthCalledWith(3, {
|
||||
method: "sessions.resolve",
|
||||
params: {
|
||||
sessionId: "current",
|
||||
spawnedBy: undefined,
|
||||
includeGlobal: true,
|
||||
includeUnknown: true,
|
||||
allowMissing: true,
|
||||
},
|
||||
});
|
||||
expect(callGatewayMock).toHaveBeenNthCalledWith(4, {
|
||||
method: "sessions.resolve",
|
||||
params: {
|
||||
sessionId: "current",
|
||||
spawnedBy: undefined,
|
||||
includeGlobal: true,
|
||||
includeUnknown: true,
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("does not compatibility-retry unrelated gateway failures", async () => {
|
||||
callGatewayMock.mockRejectedValueOnce(new Error("gateway timeout")).mockResolvedValueOnce({});
|
||||
|
||||
const result = await resolveSessionReference({
|
||||
sessionKey: "current",
|
||||
alias: "main",
|
||||
mainKey: "main",
|
||||
requesterInternalKey: "agent:main:subagent:child",
|
||||
restrictToSpawned: false,
|
||||
});
|
||||
expectResolvedSessionReference(result, {
|
||||
key: "agent:main:subagent:child",
|
||||
displayKey: "agent:main:subagent:child",
|
||||
resolvedViaSessionId: false,
|
||||
});
|
||||
expect(callGatewayMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("skips literal current key lookup when spawned visibility is restricted", async () => {
|
||||
const result = await resolveSessionReference({
|
||||
sessionKey: "current",
|
||||
@@ -328,6 +415,7 @@ describe("resolveSessionReference", () => {
|
||||
spawnedBy: "agent:main:subagent:child",
|
||||
includeGlobal: false,
|
||||
includeUnknown: false,
|
||||
allowMissing: true,
|
||||
},
|
||||
});
|
||||
expect(callGatewayMock).toHaveBeenCalledTimes(1);
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
} from "../../../packages/gateway-protocol/src/client-info.js";
|
||||
import type { OpenClawConfig } from "../../config/types.openclaw.js";
|
||||
import { callGateway } from "../../gateway/call.js";
|
||||
import { GatewayClientRequestError } from "../../gateway/client.js";
|
||||
import { formatErrorMessage } from "../../infra/errors.js";
|
||||
import {
|
||||
listSpawnedSessionKeys,
|
||||
@@ -233,24 +234,53 @@ function buildSessionIdResolveParams(params: {
|
||||
sessionId: string;
|
||||
requesterInternalKey?: string;
|
||||
restrictToSpawned: boolean;
|
||||
allowMissing?: boolean;
|
||||
}) {
|
||||
return {
|
||||
sessionId: params.sessionId,
|
||||
spawnedBy: params.restrictToSpawned ? params.requesterInternalKey : undefined,
|
||||
includeGlobal: !params.restrictToSpawned,
|
||||
includeUnknown: !params.restrictToSpawned,
|
||||
...(params.allowMissing ? { allowMissing: true } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
async function callGatewayResolveSession(
|
||||
params: Record<string, unknown> & { allowMissing?: boolean },
|
||||
) {
|
||||
try {
|
||||
return await sessionsResolutionDeps.callGateway({
|
||||
method: "sessions.resolve",
|
||||
params,
|
||||
});
|
||||
} catch (error) {
|
||||
const olderGatewayRejectedProbe =
|
||||
params.allowMissing === true &&
|
||||
error instanceof GatewayClientRequestError &&
|
||||
error.gatewayCode === "INVALID_REQUEST" &&
|
||||
error.message.includes("invalid sessions.resolve params") &&
|
||||
error.message.includes("unexpected property 'allowMissing'");
|
||||
if (!olderGatewayRejectedProbe) {
|
||||
throw error;
|
||||
}
|
||||
// Protocol v4 gateways predating allowMissing reject the additive field.
|
||||
// Retry without it for mixed-version correctness; remove at the next protocol break.
|
||||
const legacyParams: Record<string, unknown> = { ...params };
|
||||
delete legacyParams.allowMissing;
|
||||
return await sessionsResolutionDeps.callGateway({
|
||||
method: "sessions.resolve",
|
||||
params: legacyParams,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
async function callGatewayResolveSessionId(params: {
|
||||
sessionId: string;
|
||||
requesterInternalKey?: string;
|
||||
restrictToSpawned: boolean;
|
||||
allowMissing?: boolean;
|
||||
}): Promise<string> {
|
||||
const result = await sessionsResolutionDeps.callGateway({
|
||||
method: "sessions.resolve",
|
||||
params: buildSessionIdResolveParams(params),
|
||||
});
|
||||
const result = await callGatewayResolveSession(buildSessionIdResolveParams(params));
|
||||
const key = normalizeOptionalString(result?.key) ?? "";
|
||||
if (!key) {
|
||||
throw new Error(
|
||||
@@ -266,6 +296,7 @@ async function resolveSessionKeyFromSessionId(params: {
|
||||
mainKey: string;
|
||||
requesterInternalKey?: string;
|
||||
restrictToSpawned: boolean;
|
||||
allowMissing?: boolean;
|
||||
}): Promise<SessionReferenceResolution> {
|
||||
try {
|
||||
// Resolve via gateway so we respect store routing and visibility rules.
|
||||
@@ -301,15 +332,14 @@ async function resolveSessionKeyFromKey(params: {
|
||||
mainKey: string;
|
||||
requesterInternalKey?: string;
|
||||
restrictToSpawned: boolean;
|
||||
allowMissing?: boolean;
|
||||
}): Promise<SessionReferenceResolution | null> {
|
||||
try {
|
||||
// Try key-based resolution first so non-standard keys keep working.
|
||||
const result = await sessionsResolutionDeps.callGateway({
|
||||
method: "sessions.resolve",
|
||||
params: {
|
||||
key: params.key,
|
||||
spawnedBy: params.restrictToSpawned ? params.requesterInternalKey : undefined,
|
||||
},
|
||||
const result = await callGatewayResolveSession({
|
||||
key: params.key,
|
||||
spawnedBy: params.restrictToSpawned ? params.requesterInternalKey : undefined,
|
||||
...(params.allowMissing ? { allowMissing: true } : {}),
|
||||
});
|
||||
const key = normalizeOptionalString(result?.key) ?? "";
|
||||
if (!key) {
|
||||
@@ -332,6 +362,7 @@ async function tryResolveSessionKeyFromSessionId(params: {
|
||||
mainKey: string;
|
||||
requesterInternalKey?: string;
|
||||
restrictToSpawned: boolean;
|
||||
allowMissing?: boolean;
|
||||
}): Promise<Extract<SessionReferenceResolution, { ok: true }> | null> {
|
||||
try {
|
||||
const key = await callGatewayResolveSessionId(params);
|
||||
@@ -353,6 +384,7 @@ async function resolveSessionReferenceByKeyOrSessionId(params: {
|
||||
requesterInternalKey?: string;
|
||||
restrictToSpawned: boolean;
|
||||
allowUnresolvedSessionId: boolean;
|
||||
allowMissing?: boolean;
|
||||
skipKeyLookup?: boolean;
|
||||
forceSessionIdLookup?: boolean;
|
||||
}): Promise<SessionReferenceResolution | null> {
|
||||
@@ -364,6 +396,7 @@ async function resolveSessionReferenceByKeyOrSessionId(params: {
|
||||
mainKey: params.mainKey,
|
||||
requesterInternalKey: params.requesterInternalKey,
|
||||
restrictToSpawned: params.restrictToSpawned,
|
||||
allowMissing: params.allowMissing,
|
||||
});
|
||||
if (resolvedByKey) {
|
||||
return resolvedByKey;
|
||||
@@ -379,6 +412,7 @@ async function resolveSessionReferenceByKeyOrSessionId(params: {
|
||||
mainKey: params.mainKey,
|
||||
requesterInternalKey: params.requesterInternalKey,
|
||||
restrictToSpawned: params.restrictToSpawned,
|
||||
allowMissing: params.allowMissing,
|
||||
});
|
||||
}
|
||||
return await resolveSessionKeyFromSessionId({
|
||||
@@ -387,6 +421,7 @@ async function resolveSessionReferenceByKeyOrSessionId(params: {
|
||||
mainKey: params.mainKey,
|
||||
requesterInternalKey: params.requesterInternalKey,
|
||||
restrictToSpawned: params.restrictToSpawned,
|
||||
allowMissing: params.allowMissing,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -410,6 +445,7 @@ export async function resolveSessionReference(params: {
|
||||
requesterInternalKey: params.requesterInternalKey,
|
||||
restrictToSpawned: params.restrictToSpawned,
|
||||
allowUnresolvedSessionId: true,
|
||||
allowMissing: true,
|
||||
skipKeyLookup: params.restrictToSpawned,
|
||||
forceSessionIdLookup: true,
|
||||
});
|
||||
|
||||
@@ -1259,6 +1259,10 @@ export const sessionsHandlers: GatewayRequestHandlers = {
|
||||
respond(false, undefined, resolved.error);
|
||||
return;
|
||||
}
|
||||
if ("missing" in resolved) {
|
||||
respond(true, { ok: false }, undefined);
|
||||
return;
|
||||
}
|
||||
respond(true, { ok: true, key: resolved.key }, undefined);
|
||||
},
|
||||
"sessions.compaction.list": ({ params, respond, context }) => {
|
||||
|
||||
@@ -257,6 +257,10 @@ export const talkSessionHandlers: GatewayRequestHandlers = {
|
||||
respond(false, undefined, resolvedSession.error);
|
||||
return;
|
||||
}
|
||||
if ("missing" in resolvedSession) {
|
||||
respondInvalidRequest(respond, `No session found: ${params.sessionKey}`);
|
||||
return;
|
||||
}
|
||||
const handoff = createTalkHandoff({
|
||||
sessionKey: resolvedSession.key,
|
||||
provider: normalizeOptionalString(params.provider),
|
||||
|
||||
@@ -218,6 +218,19 @@ test("sessions.resolve by sessionId ignores fuzzy-search list limits and returns
|
||||
expect(resolved.payload?.key).toBe("agent:main:subagent:target");
|
||||
});
|
||||
|
||||
test("sessions.resolve can probe a missing selector without returning an RPC error", async () => {
|
||||
await createSessionStoreDir();
|
||||
const { ws } = await openClient();
|
||||
|
||||
const resolved = await rpcReq<{ ok: false }>(ws, "sessions.resolve", {
|
||||
key: "agent:main:missing",
|
||||
allowMissing: true,
|
||||
});
|
||||
|
||||
expect(resolved.ok).toBe(true);
|
||||
expect(resolved.payload).toEqual({ ok: false });
|
||||
});
|
||||
|
||||
test("sessions.resolve by key respects spawnedBy visibility filters", async () => {
|
||||
await createSessionStoreDir();
|
||||
const now = Date.now();
|
||||
|
||||
@@ -146,7 +146,7 @@ describe("resolveSessionKeyFromResolveParams", () => {
|
||||
expect(typeof updateSessionStoreCall?.[1]).toBe("function");
|
||||
});
|
||||
|
||||
it("rejects sessions belonging to a deleted agent (key-based lookup)", async () => {
|
||||
it("does not let allowMissing mask a deleted-agent error", async () => {
|
||||
const deletedAgentKey = "agent:deleted-agent:main";
|
||||
targetStore = {
|
||||
[deletedAgentKey]: { sessionId: "sess-orphan", updatedAt: 1 },
|
||||
@@ -162,7 +162,7 @@ describe("resolveSessionKeyFromResolveParams", () => {
|
||||
|
||||
const result = await resolveSessionKeyFromResolveParams({
|
||||
cfg: {},
|
||||
p: { key: deletedAgentKey },
|
||||
p: { key: deletedAgentKey, allowMissing: true },
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
|
||||
@@ -20,7 +20,10 @@ import {
|
||||
resolveGatewaySessionStoreTargetWithStore,
|
||||
} from "./session-utils.js";
|
||||
|
||||
export type SessionsResolveResult = { ok: true; key: string } | { ok: false; error: ErrorShape };
|
||||
export type SessionsResolveResult =
|
||||
| { ok: true; key: string }
|
||||
| { ok: true; missing: true }
|
||||
| { ok: false; error: ErrorShape };
|
||||
|
||||
function resolveSessionVisibilityFilterOptions(p: SessionsResolveParams) {
|
||||
return {
|
||||
@@ -31,11 +34,14 @@ function resolveSessionVisibilityFilterOptions(p: SessionsResolveParams) {
|
||||
};
|
||||
}
|
||||
|
||||
function noSessionFoundResult(key: string): SessionsResolveResult {
|
||||
function noSessionFoundResult(params: { p: SessionsResolveParams; message: string }) {
|
||||
if (params.p.allowMissing) {
|
||||
return { ok: true, missing: true } as const;
|
||||
}
|
||||
return {
|
||||
ok: false,
|
||||
error: errorShape(ErrorCodes.INVALID_REQUEST, `No session found: ${key}`),
|
||||
};
|
||||
error: errorShape(ErrorCodes.INVALID_REQUEST, params.message),
|
||||
} as const;
|
||||
}
|
||||
|
||||
/** Rejects sessions whose owning agent no longer exists in config (#65524). */
|
||||
@@ -135,7 +141,7 @@ export async function resolveSessionKeyFromResolveParams(params: {
|
||||
key: target.canonicalKey,
|
||||
})
|
||||
) {
|
||||
return noSessionFoundResult(key);
|
||||
return noSessionFoundResult({ p, message: `No session found: ${key}` });
|
||||
}
|
||||
const agentCheck = validateSessionAgentExists(
|
||||
cfg,
|
||||
@@ -150,7 +156,7 @@ export async function resolveSessionKeyFromResolveParams(params: {
|
||||
}
|
||||
const legacyKey = target.storeKeys.find((candidate) => store[candidate]);
|
||||
if (!legacyKey) {
|
||||
return noSessionFoundResult(key);
|
||||
return noSessionFoundResult({ p, message: `No session found: ${key}` });
|
||||
}
|
||||
await updateSessionStore(target.storePath, (s) => {
|
||||
const { primaryKey } = migrateAndPruneGatewaySessionStoreKey({ cfg, key, store: s });
|
||||
@@ -171,7 +177,7 @@ export async function resolveSessionKeyFromResolveParams(params: {
|
||||
key: refreshedTarget.canonicalKey,
|
||||
})
|
||||
) {
|
||||
return noSessionFoundResult(key);
|
||||
return noSessionFoundResult({ p, message: `No session found: ${key}` });
|
||||
}
|
||||
const agentCheckLegacy = validateSessionAgentExists(
|
||||
cfg,
|
||||
@@ -192,10 +198,7 @@ export async function resolveSessionKeyFromResolveParams(params: {
|
||||
const matches = findVisibleSessionIdMatches({ cfg, store, p, sessionId });
|
||||
const selection = resolveSessionIdMatchSelection(matches, sessionId);
|
||||
if (selection.kind === "none") {
|
||||
return {
|
||||
ok: false,
|
||||
error: errorShape(ErrorCodes.INVALID_REQUEST, `No session found: ${sessionId}`),
|
||||
};
|
||||
return noSessionFoundResult({ p, message: `No session found: ${sessionId}` });
|
||||
}
|
||||
if (selection.kind === "ambiguous") {
|
||||
const keys = selection.sessionKeys.join(", ");
|
||||
@@ -242,13 +245,10 @@ export async function resolveSessionKeyFromResolveParams(params: {
|
||||
},
|
||||
});
|
||||
if (list.sessions.length === 0) {
|
||||
return {
|
||||
ok: false,
|
||||
error: errorShape(
|
||||
ErrorCodes.INVALID_REQUEST,
|
||||
`No session found with label: ${parsedLabel.label}`,
|
||||
),
|
||||
};
|
||||
return noSessionFoundResult({
|
||||
p,
|
||||
message: `No session found with label: ${parsedLabel.label}`,
|
||||
});
|
||||
}
|
||||
if (list.sessions.length > 1) {
|
||||
const keys = list.sessions.map((s) => s.key).join(", ");
|
||||
|
||||
Reference in New Issue
Block a user