mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-20 05:31:30 +00:00
fix(matrix): preserve ACP thread binding targets (#64343)
Merged via squash.
Prepared head SHA: def7dcda96
Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com>
Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com>
Reviewed-by: @gumadeiras
This commit is contained in:
committed by
GitHub
parent
2f84e73c18
commit
5d2225212d
@@ -114,6 +114,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Browser/tabs: route `/tabs/action` close/select through the same browser endpoint reachability and policy checks as list/new (including Playwright-backed remote tab operations), reject CDP HTTP redirects on probe requests, and sanitize blocked-endpoint error responses so tab list/focus/close flows fail closed without echoing raw policy details back to callers. (#63332)
|
||||
- Gateway/OpenAI compat: return real `usage` for non-stream `/v1/chat/completions` responses, emit the final usage chunk when `stream_options.include_usage=true`, and bound usage-gated stream finalization after lifecycle end. (#62986) Thanks @Lellansin.
|
||||
- Matrix/migration: keep packaged warning-only crypto migrations from being misclassified as actionable when only helper chunks are present, so startup and doctor stay on the warning-only path instead of creating unnecessary migration snapshots. (#64373) Thanks @gumadeiras.
|
||||
- Matrix/ACP thread bindings: preserve canonical room casing and parent conversation routing during ACP session spawn so mixed-case room ids bind correctly from top-level rooms and existing Matrix threads. (#64343) Thanks @gumadeiras.
|
||||
|
||||
## 2026.4.9
|
||||
|
||||
|
||||
@@ -322,6 +322,29 @@ function enableLineCurrentConversationBindings(): void {
|
||||
});
|
||||
}
|
||||
|
||||
function enableTelegramCurrentConversationBindings(): void {
|
||||
replaceSpawnConfig({
|
||||
...hoisted.state.cfg,
|
||||
channels: {
|
||||
...hoisted.state.cfg.channels,
|
||||
telegram: {
|
||||
threadBindings: {
|
||||
enabled: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
registerSessionBindingAdapter({
|
||||
channel: "telegram",
|
||||
accountId: "default",
|
||||
capabilities: createSessionBindingCapabilities(),
|
||||
bind: async (input) => await hoisted.sessionBindingBindMock(input),
|
||||
listBySession: (targetSessionKey) => hoisted.sessionBindingListBySessionMock(targetSessionKey),
|
||||
resolveByConversation: (ref) => hoisted.sessionBindingResolveByConversationMock(ref),
|
||||
unbind: async (input) => await hoisted.sessionBindingUnbindMock(input),
|
||||
});
|
||||
}
|
||||
|
||||
describe("spawnAcpDirect", () => {
|
||||
beforeEach(() => {
|
||||
replaceSpawnConfig(createDefaultSpawnConfig());
|
||||
@@ -611,6 +634,128 @@ describe("spawnAcpDirect", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps canonical Matrix room casing for ACP thread bindings", async () => {
|
||||
enableMatrixAcpThreadBindings();
|
||||
hoisted.sessionBindingBindMock.mockImplementationOnce(
|
||||
async (input: {
|
||||
targetSessionKey: string;
|
||||
conversation: { accountId: string; conversationId: string; parentConversationId?: string };
|
||||
metadata?: Record<string, unknown>;
|
||||
}) =>
|
||||
createSessionBinding({
|
||||
targetSessionKey: input.targetSessionKey,
|
||||
conversation: {
|
||||
channel: "matrix",
|
||||
accountId: input.conversation.accountId,
|
||||
conversationId: "child-thread",
|
||||
parentConversationId: input.conversation.parentConversationId ?? "!Room:Example.org",
|
||||
},
|
||||
metadata: {
|
||||
boundBy:
|
||||
typeof input.metadata?.boundBy === "string" ? input.metadata.boundBy : "system",
|
||||
agentId: "codex",
|
||||
webhookId: "wh-1",
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await spawnAcpDirect(
|
||||
{
|
||||
task: "Investigate flaky tests",
|
||||
agentId: "codex",
|
||||
mode: "session",
|
||||
thread: true,
|
||||
},
|
||||
{
|
||||
agentSessionKey: "agent:main:matrix:channel:!room:example.org",
|
||||
agentChannel: "matrix",
|
||||
agentAccountId: "default",
|
||||
agentTo: "room:!Room:Example.org",
|
||||
agentGroupId: "!room:example.org",
|
||||
},
|
||||
);
|
||||
|
||||
expect(result.status, JSON.stringify(result)).toBe("accepted");
|
||||
expect(hoisted.sessionBindingBindMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
placement: "child",
|
||||
conversation: expect.objectContaining({
|
||||
channel: "matrix",
|
||||
accountId: "default",
|
||||
conversationId: "!Room:Example.org",
|
||||
}),
|
||||
}),
|
||||
);
|
||||
expectAgentGatewayCall({
|
||||
deliver: true,
|
||||
channel: "matrix",
|
||||
to: "channel:!Room:Example.org",
|
||||
threadId: "child-thread",
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves Matrix parent room casing when binding from an existing thread", async () => {
|
||||
enableMatrixAcpThreadBindings();
|
||||
hoisted.sessionBindingBindMock.mockImplementationOnce(
|
||||
async (input: {
|
||||
targetSessionKey: string;
|
||||
conversation: { accountId: string; conversationId: string; parentConversationId?: string };
|
||||
metadata?: Record<string, unknown>;
|
||||
}) =>
|
||||
createSessionBinding({
|
||||
targetSessionKey: input.targetSessionKey,
|
||||
conversation: {
|
||||
channel: "matrix",
|
||||
accountId: input.conversation.accountId,
|
||||
conversationId: "child-thread",
|
||||
parentConversationId: input.conversation.parentConversationId ?? "!Room:Example.org",
|
||||
},
|
||||
metadata: {
|
||||
boundBy:
|
||||
typeof input.metadata?.boundBy === "string" ? input.metadata.boundBy : "system",
|
||||
agentId: "codex",
|
||||
webhookId: "wh-1",
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await spawnAcpDirect(
|
||||
{
|
||||
task: "Investigate flaky tests",
|
||||
agentId: "codex",
|
||||
mode: "session",
|
||||
thread: true,
|
||||
},
|
||||
{
|
||||
agentSessionKey: "agent:main:matrix:channel:!room:example.org:thread:$thread-root",
|
||||
agentChannel: "matrix",
|
||||
agentAccountId: "default",
|
||||
agentTo: "room:!Room:Example.org",
|
||||
agentThreadId: "$thread-root",
|
||||
agentGroupId: "!room:example.org",
|
||||
},
|
||||
);
|
||||
|
||||
expect(result.status, JSON.stringify(result)).toBe("accepted");
|
||||
expect(hoisted.sessionBindingBindMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
placement: "child",
|
||||
conversation: expect.objectContaining({
|
||||
channel: "matrix",
|
||||
accountId: "default",
|
||||
conversationId: "$thread-root",
|
||||
parentConversationId: "!Room:Example.org",
|
||||
}),
|
||||
}),
|
||||
);
|
||||
expectAgentGatewayCall({
|
||||
deliver: true,
|
||||
channel: "matrix",
|
||||
to: "channel:!Room:Example.org",
|
||||
threadId: "child-thread",
|
||||
});
|
||||
});
|
||||
|
||||
it("uses the target agent workspace for cross-agent ACP spawns when cwd is omitted", async () => {
|
||||
const fixture = await createCrossAgentWorkspaceFixture();
|
||||
try {
|
||||
@@ -933,6 +1078,58 @@ describe("spawnAcpDirect", () => {
|
||||
},
|
||||
);
|
||||
|
||||
it("preserves LINE fallback conversation precedence when groupId is present", async () => {
|
||||
enableLineCurrentConversationBindings();
|
||||
hoisted.sessionBindingBindMock.mockImplementationOnce(
|
||||
async (input: {
|
||||
targetSessionKey: string;
|
||||
conversation: { accountId: string; conversationId: string };
|
||||
metadata?: Record<string, unknown>;
|
||||
}) =>
|
||||
createSessionBinding({
|
||||
targetSessionKey: input.targetSessionKey,
|
||||
conversation: {
|
||||
channel: "line",
|
||||
accountId: input.conversation.accountId,
|
||||
conversationId: input.conversation.conversationId,
|
||||
},
|
||||
metadata: {
|
||||
boundBy:
|
||||
typeof input.metadata?.boundBy === "string" ? input.metadata.boundBy : "system",
|
||||
agentId: "codex",
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await spawnAcpDirect(
|
||||
{
|
||||
task: "Investigate flaky tests",
|
||||
agentId: "codex",
|
||||
mode: "session",
|
||||
thread: true,
|
||||
},
|
||||
{
|
||||
agentSessionKey: "agent:main:line:direct:R1234567890abcdef1234567890abcdef",
|
||||
agentChannel: "line",
|
||||
agentAccountId: "default",
|
||||
agentTo: "line:user:U1234567890abcdef1234567890abcdef",
|
||||
agentGroupId: "line:room:R1234567890abcdef1234567890abcdef",
|
||||
},
|
||||
);
|
||||
|
||||
expect(result.status).toBe("accepted");
|
||||
expect(hoisted.sessionBindingBindMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
placement: "current",
|
||||
conversation: expect.objectContaining({
|
||||
channel: "line",
|
||||
accountId: "default",
|
||||
conversationId: "R1234567890abcdef1234567890abcdef",
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
name: "does not inline delivery for run-mode spawns from non-subagent requester sessions",
|
||||
@@ -1522,27 +1719,7 @@ describe("spawnAcpDirect", () => {
|
||||
});
|
||||
|
||||
it("binds Telegram forum-topic ACP sessions to the current topic", async () => {
|
||||
replaceSpawnConfig({
|
||||
...hoisted.state.cfg,
|
||||
channels: {
|
||||
...hoisted.state.cfg.channels,
|
||||
telegram: {
|
||||
threadBindings: {
|
||||
enabled: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
registerSessionBindingAdapter({
|
||||
channel: "telegram",
|
||||
accountId: "default",
|
||||
capabilities: createSessionBindingCapabilities(),
|
||||
bind: async (input) => await hoisted.sessionBindingBindMock(input),
|
||||
listBySession: (targetSessionKey) =>
|
||||
hoisted.sessionBindingListBySessionMock(targetSessionKey),
|
||||
resolveByConversation: (ref) => hoisted.sessionBindingResolveByConversationMock(ref),
|
||||
unbind: async (input) => await hoisted.sessionBindingUnbindMock(input),
|
||||
});
|
||||
enableTelegramCurrentConversationBindings();
|
||||
|
||||
const result = await spawnAcpDirect(
|
||||
{
|
||||
@@ -1580,28 +1757,44 @@ describe("spawnAcpDirect", () => {
|
||||
expect(agentCall?.params?.channel).toBe("telegram");
|
||||
});
|
||||
|
||||
it("preserves topic-qualified Telegram targets without a separate threadId", async () => {
|
||||
replaceSpawnConfig({
|
||||
...hoisted.state.cfg,
|
||||
channels: {
|
||||
...hoisted.state.cfg.channels,
|
||||
telegram: {
|
||||
threadBindings: {
|
||||
enabled: true,
|
||||
},
|
||||
},
|
||||
it("drops self-parent Telegram current-conversation refs before binding", async () => {
|
||||
enableTelegramCurrentConversationBindings();
|
||||
|
||||
const result = await spawnAcpDirect(
|
||||
{
|
||||
task: "Investigate flaky tests",
|
||||
agentId: "codex",
|
||||
mode: "session",
|
||||
thread: true,
|
||||
},
|
||||
});
|
||||
registerSessionBindingAdapter({
|
||||
channel: "telegram",
|
||||
accountId: "default",
|
||||
capabilities: createSessionBindingCapabilities(),
|
||||
bind: async (input) => await hoisted.sessionBindingBindMock(input),
|
||||
listBySession: (targetSessionKey) =>
|
||||
hoisted.sessionBindingListBySessionMock(targetSessionKey),
|
||||
resolveByConversation: (ref) => hoisted.sessionBindingResolveByConversationMock(ref),
|
||||
unbind: async (input) => await hoisted.sessionBindingUnbindMock(input),
|
||||
});
|
||||
{
|
||||
agentSessionKey: "agent:main:telegram:direct:6098642967",
|
||||
agentChannel: "telegram",
|
||||
agentAccountId: "default",
|
||||
agentTo: "telegram:6098642967",
|
||||
},
|
||||
);
|
||||
|
||||
const accepted = expectAcceptedSpawn(result);
|
||||
expect(accepted.mode).toBe("session");
|
||||
expect(hoisted.sessionBindingBindMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
placement: "current",
|
||||
conversation: expect.objectContaining({
|
||||
channel: "telegram",
|
||||
accountId: "default",
|
||||
conversationId: "6098642967",
|
||||
}),
|
||||
}),
|
||||
);
|
||||
const bindCall = hoisted.sessionBindingBindMock.mock.calls.at(-1)?.[0] as
|
||||
| { conversation?: { parentConversationId?: string } }
|
||||
| undefined;
|
||||
expect(bindCall?.conversation?.parentConversationId).toBeUndefined();
|
||||
});
|
||||
|
||||
it("preserves topic-qualified Telegram targets without a separate threadId", async () => {
|
||||
enableTelegramCurrentConversationBindings();
|
||||
|
||||
const result = await spawnAcpDirect(
|
||||
{
|
||||
|
||||
@@ -34,6 +34,7 @@ import type { SessionEntry } from "../config/sessions/types.js";
|
||||
import { callGateway } from "../gateway/call.js";
|
||||
import { areHeartbeatsEnabled } from "../infra/heartbeat-wake.js";
|
||||
import { resolveConversationIdFromTargets } from "../infra/outbound/conversation-id.js";
|
||||
import { normalizeConversationTargetRef } from "../infra/outbound/session-binding-normalization.js";
|
||||
import {
|
||||
getSessionBindingService,
|
||||
isSessionBindingError,
|
||||
@@ -171,6 +172,7 @@ type PreparedAcpThreadBinding = {
|
||||
accountId: string;
|
||||
placement: "current" | "child";
|
||||
conversationId: string;
|
||||
parentConversationId?: string;
|
||||
};
|
||||
|
||||
type AcpSpawnInitializedSession = Awaited<
|
||||
@@ -269,6 +271,31 @@ const threadBindingFallbackConversationResolvers = {
|
||||
normalizeTelegramConversationIdFallback(params),
|
||||
} as const;
|
||||
|
||||
function resolvePluginConversationRefForThreadBinding(params: {
|
||||
channelId: string;
|
||||
to?: string;
|
||||
threadId?: string | number;
|
||||
groupId?: string;
|
||||
}): { conversationId: string; parentConversationId?: string } | null {
|
||||
const resolvedConversation = getChannelPlugin(
|
||||
params.channelId,
|
||||
)?.messaging?.resolveInboundConversation?.({
|
||||
// Keep the live delivery target authoritative; conversationId is only a fallback hint.
|
||||
to: params.to,
|
||||
conversationId: params.groupId ?? params.to,
|
||||
threadId: params.threadId,
|
||||
isGroup: true,
|
||||
});
|
||||
const conversationId = normalizeOptionalString(resolvedConversation?.conversationId);
|
||||
if (!conversationId) {
|
||||
return null;
|
||||
}
|
||||
return normalizeConversationTargetRef({
|
||||
conversationId,
|
||||
parentConversationId: resolvedConversation?.parentConversationId,
|
||||
});
|
||||
}
|
||||
|
||||
function resolveSpawnMode(params: {
|
||||
requestedMode?: SpawnAcpMode;
|
||||
threadRequested: boolean;
|
||||
@@ -496,25 +523,25 @@ async function persistAcpSpawnSessionFileBestEffort(params: {
|
||||
}
|
||||
}
|
||||
|
||||
function resolveConversationIdForThreadBinding(params: {
|
||||
function resolveConversationRefForThreadBinding(params: {
|
||||
channel?: string;
|
||||
to?: string;
|
||||
threadId?: string | number;
|
||||
groupId?: string;
|
||||
}): string | undefined {
|
||||
}): { conversationId: string; parentConversationId?: string } | null {
|
||||
const channel = normalizeOptionalLowercaseString(params.channel);
|
||||
const normalizedChannelId = channel ? normalizeChannelId(channel) : null;
|
||||
const channelKey = normalizedChannelId ?? channel ?? null;
|
||||
const pluginResolvedConversationId = normalizedChannelId
|
||||
? getChannelPlugin(normalizedChannelId)?.messaging?.resolveInboundConversation?.({
|
||||
to: params.groupId ?? params.to,
|
||||
conversationId: params.groupId ?? params.to,
|
||||
const pluginResolvedConversation = normalizedChannelId
|
||||
? resolvePluginConversationRefForThreadBinding({
|
||||
channelId: normalizedChannelId,
|
||||
to: params.to,
|
||||
threadId: params.threadId,
|
||||
isGroup: true,
|
||||
})?.conversationId
|
||||
groupId: params.groupId,
|
||||
})
|
||||
: null;
|
||||
if (normalizeOptionalString(pluginResolvedConversationId)) {
|
||||
return normalizeOptionalString(pluginResolvedConversationId);
|
||||
if (pluginResolvedConversation) {
|
||||
return pluginResolvedConversation;
|
||||
}
|
||||
const compatibilityConversationId =
|
||||
channelKey && Object.hasOwn(threadBindingFallbackConversationResolvers, channelKey)
|
||||
@@ -523,16 +550,22 @@ function resolveConversationIdForThreadBinding(params: {
|
||||
](params)
|
||||
: undefined;
|
||||
if (compatibilityConversationId) {
|
||||
return compatibilityConversationId;
|
||||
return normalizeConversationTargetRef({ conversationId: compatibilityConversationId });
|
||||
}
|
||||
const parentConversationId = resolveConversationIdFromTargets({
|
||||
targets: [params.to],
|
||||
});
|
||||
const genericConversationId = resolveConversationIdFromTargets({
|
||||
threadId: params.threadId,
|
||||
targets: [params.to],
|
||||
});
|
||||
if (genericConversationId) {
|
||||
return genericConversationId;
|
||||
return normalizeConversationTargetRef({
|
||||
conversationId: genericConversationId,
|
||||
parentConversationId: params.threadId != null ? parentConversationId : undefined,
|
||||
});
|
||||
}
|
||||
return undefined;
|
||||
return null;
|
||||
}
|
||||
|
||||
function resolveAcpSpawnChannelAccountId(params: {
|
||||
@@ -625,13 +658,13 @@ function prepareAcpThreadBinding(params: {
|
||||
error: `Thread bindings do not support ${placementToUse} placement for ${policy.channel}.`,
|
||||
};
|
||||
}
|
||||
const conversationIdRaw = resolveConversationIdForThreadBinding({
|
||||
const conversationRef = resolveConversationRefForThreadBinding({
|
||||
channel: policy.channel,
|
||||
to: params.to,
|
||||
threadId: params.threadId,
|
||||
groupId: params.groupId,
|
||||
});
|
||||
if (!conversationIdRaw) {
|
||||
if (!conversationRef?.conversationId) {
|
||||
return {
|
||||
ok: false,
|
||||
error: `Could not resolve a ${policy.channel} conversation for ACP thread spawn.`,
|
||||
@@ -644,7 +677,10 @@ function prepareAcpThreadBinding(params: {
|
||||
channel: policy.channel,
|
||||
accountId: policy.accountId,
|
||||
placement: placementToUse,
|
||||
conversationId: conversationIdRaw,
|
||||
conversationId: conversationRef.conversationId,
|
||||
...(conversationRef.parentConversationId
|
||||
? { parentConversationId: conversationRef.parentConversationId }
|
||||
: {}),
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -790,6 +826,9 @@ async function bindPreparedAcpThread(params: {
|
||||
channel: params.preparedBinding.channel,
|
||||
accountId: params.preparedBinding.accountId,
|
||||
conversationId: params.preparedBinding.conversationId,
|
||||
...(params.preparedBinding.parentConversationId
|
||||
? { parentConversationId: params.preparedBinding.parentConversationId }
|
||||
: {}),
|
||||
},
|
||||
placement: params.preparedBinding.placement,
|
||||
metadata: {
|
||||
@@ -867,7 +906,7 @@ function resolveAcpSpawnBootstrapDeliveryPlan(params: {
|
||||
const fallbackThreadId =
|
||||
fallbackThreadIdRaw != null ? normalizeOptionalString(String(fallbackThreadIdRaw)) : undefined;
|
||||
const deliveryThreadId = boundThreadId ?? fallbackThreadId;
|
||||
const requesterConversationId = resolveConversationIdForThreadBinding({
|
||||
const requesterConversationRef = resolveConversationRefForThreadBinding({
|
||||
channel: params.requester.origin?.channel,
|
||||
threadId: fallbackThreadId,
|
||||
to: params.requester.origin?.to,
|
||||
@@ -881,8 +920,10 @@ function resolveAcpSpawnBootstrapDeliveryPlan(params: {
|
||||
params.requester.origin?.channel &&
|
||||
params.binding?.conversation.channel === params.requester.origin.channel &&
|
||||
params.binding?.conversation.accountId === requesterAccountId &&
|
||||
requesterConversationId &&
|
||||
params.binding?.conversation.conversationId === requesterConversationId,
|
||||
requesterConversationRef?.conversationId &&
|
||||
params.binding?.conversation.conversationId === requesterConversationRef.conversationId &&
|
||||
(params.binding?.conversation.parentConversationId ?? undefined) ===
|
||||
(requesterConversationRef.parentConversationId ?? undefined),
|
||||
);
|
||||
const boundDeliveryTarget = resolveConversationDeliveryTarget({
|
||||
channel: params.requester.origin?.channel ?? params.binding?.conversation.channel,
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { normalizeConversationText } from "../../../acp/conversation-id.js";
|
||||
import { normalizeConversationTargetRef } from "../../../infra/outbound/session-binding-normalization.js";
|
||||
import { normalizeLowercaseStringOrEmpty } from "../../../shared/string-coerce.js";
|
||||
import type { HandleCommandsParams } from "../commands-types.js";
|
||||
import {
|
||||
@@ -33,12 +34,10 @@ function resolveAcpCommandConversationRef(params: HandleCommandsParams): {
|
||||
if (!resolved) {
|
||||
return null;
|
||||
}
|
||||
return {
|
||||
return normalizeConversationTargetRef({
|
||||
conversationId: resolved.conversationId,
|
||||
...(resolved.parentConversationId && resolved.parentConversationId !== resolved.conversationId
|
||||
? { parentConversationId: resolved.parentConversationId }
|
||||
: {}),
|
||||
};
|
||||
parentConversationId: resolved.parentConversationId,
|
||||
});
|
||||
}
|
||||
|
||||
export function resolveAcpCommandConversationId(params: HandleCommandsParams): string | undefined {
|
||||
|
||||
@@ -34,6 +34,7 @@ import type { OpenClawConfig } from "../../../config/config.js";
|
||||
import { updateSessionStore } from "../../../config/sessions.js";
|
||||
import type { SessionAcpMeta } from "../../../config/sessions/types.js";
|
||||
import { formatErrorMessage } from "../../../infra/errors.js";
|
||||
import { normalizeConversationRef } from "../../../infra/outbound/session-binding-normalization.js";
|
||||
import {
|
||||
getSessionBindingService,
|
||||
type ConversationRef,
|
||||
@@ -250,15 +251,12 @@ async function bindSpawnedAcpSessionToCurrentConversation(params: {
|
||||
}
|
||||
|
||||
const senderId = normalizeOptionalString(params.commandParams.command.senderId) ?? "";
|
||||
const parentConversationId = normalizeOptionalString(bindingContext.parentConversationId);
|
||||
const conversationRef = {
|
||||
const conversationRef = normalizeConversationRef({
|
||||
channel: bindingPolicy.channel,
|
||||
accountId: bindingPolicy.accountId,
|
||||
conversationId: currentConversationId,
|
||||
...(parentConversationId && parentConversationId !== currentConversationId
|
||||
? { parentConversationId }
|
||||
: {}),
|
||||
};
|
||||
parentConversationId: bindingContext.parentConversationId,
|
||||
});
|
||||
const existingBinding = bindingService.resolveByConversation(conversationRef);
|
||||
const boundBy = normalizeOptionalString(existingBinding?.metadata?.boundBy) ?? "";
|
||||
if (existingBinding && boundBy && boundBy !== "system" && senderId && senderId !== boundBy) {
|
||||
@@ -393,15 +391,12 @@ async function bindSpawnedAcpSessionToThread(params: {
|
||||
}
|
||||
|
||||
const senderId = normalizeOptionalString(commandParams.command.senderId) ?? "";
|
||||
const parentConversationId = normalizeOptionalString(bindingContext.parentConversationId);
|
||||
const conversationRef = {
|
||||
const conversationRef = normalizeConversationRef({
|
||||
channel: spawnPolicy.channel,
|
||||
accountId: spawnPolicy.accountId,
|
||||
conversationId: currentConversationId,
|
||||
...(parentConversationId && parentConversationId !== currentConversationId
|
||||
? { parentConversationId }
|
||||
: {}),
|
||||
};
|
||||
parentConversationId: bindingContext.parentConversationId,
|
||||
});
|
||||
if (placement === "current") {
|
||||
const existingBinding = bindingService.resolveByConversation(conversationRef);
|
||||
const boundBy = normalizeOptionalString(existingBinding?.metadata?.boundBy) ?? "";
|
||||
|
||||
@@ -549,4 +549,37 @@ describe("focus actions", () => {
|
||||
reason: "manual",
|
||||
});
|
||||
});
|
||||
|
||||
it("drops self-parent refs before resolving /unfocus bindings", async () => {
|
||||
hoisted.resolveConversationBindingContextMock.mockReturnValue({
|
||||
channel: THREAD_CHANNEL,
|
||||
accountId: "default",
|
||||
conversationId: "dm-1",
|
||||
parentConversationId: "dm-1",
|
||||
});
|
||||
hoisted.sessionBindingResolveByConversationMock.mockReturnValue(
|
||||
createSessionBindingRecord({
|
||||
bindingId: "default:dm-1",
|
||||
conversation: {
|
||||
channel: THREAD_CHANNEL,
|
||||
accountId: "default",
|
||||
conversationId: "dm-1",
|
||||
},
|
||||
metadata: { boundBy: "user-1" },
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await handleSubagentsUnfocusAction(buildUnfocusContext());
|
||||
|
||||
expect(result.reply?.text).toContain("Conversation unfocused");
|
||||
expect(hoisted.sessionBindingResolveByConversationMock).toHaveBeenCalledWith({
|
||||
channel: THREAD_CHANNEL,
|
||||
accountId: "default",
|
||||
conversationId: "dm-1",
|
||||
});
|
||||
expect(hoisted.sessionBindingUnbindMock).toHaveBeenCalledWith({
|
||||
bindingId: "default:dm-1",
|
||||
reason: "manual",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
resolveThreadBindingPlacementForCurrentContext,
|
||||
resolveThreadBindingSpawnPolicy,
|
||||
} from "../../../channels/thread-bindings-policy.js";
|
||||
import { normalizeConversationRef } from "../../../infra/outbound/session-binding-normalization.js";
|
||||
import { getSessionBindingService } from "../../../infra/outbound/session-binding-service.js";
|
||||
import { normalizeOptionalString } from "../../../shared/string-coerce.js";
|
||||
import type { CommandHandlerResult } from "../commands-types.js";
|
||||
@@ -38,12 +39,18 @@ function resolveFocusBindingContext(
|
||||
return null;
|
||||
}
|
||||
const chatType = normalizeChatType(params.ctx.ChatType);
|
||||
return {
|
||||
const conversation = normalizeConversationRef({
|
||||
channel: bindingContext.channel,
|
||||
accountId: bindingContext.accountId,
|
||||
conversationId: bindingContext.conversationId,
|
||||
...(bindingContext.parentConversationId
|
||||
? { parentConversationId: bindingContext.parentConversationId }
|
||||
parentConversationId: bindingContext.parentConversationId,
|
||||
});
|
||||
return {
|
||||
channel: conversation.channel,
|
||||
accountId: conversation.accountId,
|
||||
conversationId: conversation.conversationId,
|
||||
...(conversation.parentConversationId
|
||||
? { parentConversationId: conversation.parentConversationId }
|
||||
: {}),
|
||||
placement:
|
||||
chatType === "direct"
|
||||
@@ -111,15 +118,13 @@ export async function handleSubagentsFocusAction(
|
||||
}
|
||||
|
||||
const senderId = normalizeOptionalString(params.command.senderId) ?? "";
|
||||
const existingBinding = bindingService.resolveByConversation({
|
||||
const conversationRef = normalizeConversationRef({
|
||||
channel: bindingContext.channel,
|
||||
accountId: bindingContext.accountId,
|
||||
conversationId: bindingContext.conversationId,
|
||||
...(bindingContext.parentConversationId &&
|
||||
bindingContext.parentConversationId !== bindingContext.conversationId
|
||||
? { parentConversationId: bindingContext.parentConversationId }
|
||||
: {}),
|
||||
parentConversationId: bindingContext.parentConversationId,
|
||||
});
|
||||
const existingBinding = bindingService.resolveByConversation(conversationRef);
|
||||
const boundBy =
|
||||
typeof existingBinding?.metadata?.boundBy === "string"
|
||||
? existingBinding.metadata.boundBy.trim()
|
||||
@@ -146,15 +151,12 @@ export async function handleSubagentsFocusAction(
|
||||
binding = await bindingService.bind({
|
||||
targetSessionKey: focusTarget.targetSessionKey,
|
||||
targetKind: focusTarget.targetKind === "acp" ? "session" : "subagent",
|
||||
conversation: {
|
||||
conversation: normalizeConversationRef({
|
||||
channel: bindingContext.channel,
|
||||
accountId: bindingContext.accountId,
|
||||
conversationId: bindingContext.conversationId,
|
||||
...(bindingContext.parentConversationId &&
|
||||
bindingContext.parentConversationId !== bindingContext.conversationId
|
||||
? { parentConversationId: bindingContext.parentConversationId }
|
||||
: {}),
|
||||
},
|
||||
parentConversationId: bindingContext.parentConversationId,
|
||||
}),
|
||||
placement: bindingContext.placement,
|
||||
metadata: {
|
||||
threadName: resolveThreadBindingThreadName({
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { normalizeConversationRef } from "../../../infra/outbound/session-binding-normalization.js";
|
||||
import { getSessionBindingService } from "../../../infra/outbound/session-binding-service.js";
|
||||
import { normalizeOptionalString } from "../../../shared/string-coerce.js";
|
||||
import type { CommandHandlerResult } from "../commands-types.js";
|
||||
@@ -14,15 +15,14 @@ export async function handleSubagentsUnfocusAction(
|
||||
return stopWithText("⚠️ /unfocus must be run inside a focused conversation.");
|
||||
}
|
||||
|
||||
const binding = bindingService.resolveByConversation({
|
||||
channel: bindingContext.channel,
|
||||
accountId: bindingContext.accountId,
|
||||
conversationId: bindingContext.conversationId,
|
||||
...(bindingContext.parentConversationId &&
|
||||
bindingContext.parentConversationId !== bindingContext.conversationId
|
||||
? { parentConversationId: bindingContext.parentConversationId }
|
||||
: {}),
|
||||
});
|
||||
const binding = bindingService.resolveByConversation(
|
||||
normalizeConversationRef({
|
||||
channel: bindingContext.channel,
|
||||
accountId: bindingContext.accountId,
|
||||
conversationId: bindingContext.conversationId,
|
||||
parentConversationId: bindingContext.parentConversationId,
|
||||
}),
|
||||
);
|
||||
if (!binding) {
|
||||
return stopWithText("ℹ️ This conversation is not currently focused.");
|
||||
}
|
||||
|
||||
@@ -123,6 +123,105 @@ describe("generic current-conversation bindings", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("drops self-parent conversation refs when storing generic current bindings", async () => {
|
||||
const bound = await bindGenericCurrentConversation({
|
||||
targetSessionKey: "agent:codex:acp:telegram-dm",
|
||||
targetKind: "session",
|
||||
conversation: {
|
||||
channel: "telegram",
|
||||
accountId: "default",
|
||||
conversationId: "6098642967",
|
||||
parentConversationId: "6098642967",
|
||||
},
|
||||
});
|
||||
|
||||
expect(bound).toMatchObject({
|
||||
bindingId: "generic:telegram\u241fdefault\u241f\u241f6098642967",
|
||||
conversation: {
|
||||
channel: "telegram",
|
||||
accountId: "default",
|
||||
conversationId: "6098642967",
|
||||
},
|
||||
});
|
||||
expect(bound?.conversation.parentConversationId).toBeUndefined();
|
||||
expect(
|
||||
resolveGenericCurrentConversationBinding({
|
||||
channel: "telegram",
|
||||
accountId: "default",
|
||||
conversationId: "6098642967",
|
||||
}),
|
||||
).toMatchObject({
|
||||
bindingId: "generic:telegram\u241fdefault\u241f\u241f6098642967",
|
||||
targetSessionKey: "agent:codex:acp:telegram-dm",
|
||||
});
|
||||
});
|
||||
|
||||
it("migrates persisted legacy self-parent binding ids on load", async () => {
|
||||
const filePath = __testing.resolveBindingsFilePath();
|
||||
await fs.mkdir(path.dirname(filePath), { recursive: true });
|
||||
await fs.writeFile(
|
||||
filePath,
|
||||
JSON.stringify({
|
||||
version: 1,
|
||||
bindings: [
|
||||
{
|
||||
bindingId: "generic:telegram\u241fdefault\u241f6098642967\u241f6098642967",
|
||||
targetSessionKey: "agent:codex:acp:telegram-dm",
|
||||
targetKind: "session",
|
||||
conversation: {
|
||||
channel: "telegram",
|
||||
accountId: "default",
|
||||
conversationId: "6098642967",
|
||||
parentConversationId: "6098642967",
|
||||
},
|
||||
status: "active",
|
||||
boundAt: 1234,
|
||||
metadata: {
|
||||
label: "telegram-dm",
|
||||
},
|
||||
},
|
||||
],
|
||||
}),
|
||||
);
|
||||
|
||||
const resolved = resolveGenericCurrentConversationBinding({
|
||||
channel: "telegram",
|
||||
accountId: "default",
|
||||
conversationId: "6098642967",
|
||||
});
|
||||
|
||||
expect(resolved).toMatchObject({
|
||||
bindingId: "generic:telegram\u241fdefault\u241f\u241f6098642967",
|
||||
targetSessionKey: "agent:codex:acp:telegram-dm",
|
||||
conversation: {
|
||||
channel: "telegram",
|
||||
accountId: "default",
|
||||
conversationId: "6098642967",
|
||||
},
|
||||
});
|
||||
expect(resolved?.conversation.parentConversationId).toBeUndefined();
|
||||
|
||||
await expect(
|
||||
unbindGenericCurrentConversationBindings({
|
||||
bindingId: resolved?.bindingId,
|
||||
reason: "test cleanup",
|
||||
}),
|
||||
).resolves.toEqual([
|
||||
expect.objectContaining({
|
||||
bindingId: "generic:telegram\u241fdefault\u241f\u241f6098642967",
|
||||
}),
|
||||
]);
|
||||
|
||||
__testing.resetCurrentConversationBindingsForTests();
|
||||
expect(
|
||||
resolveGenericCurrentConversationBinding({
|
||||
channel: "telegram",
|
||||
accountId: "default",
|
||||
conversationId: "6098642967",
|
||||
}),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("removes persisted bindings on unbind", async () => {
|
||||
await bindGenericCurrentConversation({
|
||||
targetSessionKey: "agent:codex:acp:googlechat-room",
|
||||
|
||||
@@ -76,9 +76,11 @@ function loadBindingsIntoMemory(): void {
|
||||
if (!record?.bindingId || !record?.conversation?.conversationId || isBindingExpired(record)) {
|
||||
continue;
|
||||
}
|
||||
bindingsByConversationKey.set(buildConversationKey(record.conversation), {
|
||||
const conversation = normalizeConversationRef(record.conversation);
|
||||
bindingsByConversationKey.set(buildConversationKey(conversation), {
|
||||
...record,
|
||||
conversation: normalizeConversationRef(record.conversation),
|
||||
bindingId: buildBindingId(conversation),
|
||||
conversation,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -11,13 +11,30 @@ export type ConversationRefShape = {
|
||||
parentConversationId?: string;
|
||||
};
|
||||
|
||||
export function normalizeConversationRef<T extends ConversationRefShape>(ref: T): T {
|
||||
type ConversationTargetRefShape = {
|
||||
conversationId: string;
|
||||
parentConversationId?: string | null;
|
||||
};
|
||||
|
||||
export function normalizeConversationTargetRef<T extends ConversationTargetRefShape>(ref: T): T {
|
||||
const conversationId = normalizeOptionalString(ref.conversationId) ?? "";
|
||||
const parentConversationId = normalizeOptionalString(ref.parentConversationId);
|
||||
const { parentConversationId: _ignoredParentConversationId, ...rest } = ref;
|
||||
return {
|
||||
...ref,
|
||||
...rest,
|
||||
conversationId,
|
||||
...(parentConversationId && parentConversationId !== conversationId
|
||||
? { parentConversationId }
|
||||
: {}),
|
||||
} as T;
|
||||
}
|
||||
|
||||
export function normalizeConversationRef<T extends ConversationRefShape>(ref: T): T {
|
||||
const normalizedTarget = normalizeConversationTargetRef(ref);
|
||||
return {
|
||||
...normalizedTarget,
|
||||
channel: normalizeLowercaseStringOrEmpty(ref.channel),
|
||||
accountId: normalizeAccountId(ref.accountId),
|
||||
conversationId: normalizeOptionalString(ref.conversationId) ?? "",
|
||||
parentConversationId: normalizeOptionalString(ref.parentConversationId),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user