fix(sessions): reset stale per-channel origin fields on channel switch (#95328)

Merged via squash.

Prepared head SHA: 3a946cb078
Co-authored-by: ZengWen-DT <290981215+ZengWen-DT@users.noreply.github.com>
Co-authored-by: jalehman <550978+jalehman@users.noreply.github.com>
Reviewed-by: @jalehman
This commit is contained in:
ZengWen-DT
2026-06-20 18:09:37 -07:00
committed by GitHub
parent 8b4eedf1bc
commit 73c988a9c8
4 changed files with 246 additions and 0 deletions
@@ -4025,11 +4025,17 @@ async function runCommandInvocation(invocation, options) {
});
child.on("error", (error) => {
if (forwardedSignalExitCode !== undefined) {
activeChildTree.killChildTree("SIGKILL");
}
activeChildTree.unregister();
finalize(() => rejectPromise(error));
});
child.on("close", (exitCode) => {
if (forwardedSignalExitCode !== undefined) {
activeChildTree.killChildTree("SIGKILL");
}
activeChildTree.unregister();
if (forwardedSignalExitCode !== undefined) {
finalize(exitForwardedSignalWhenChildTreesDone);
+17
View File
@@ -20,6 +20,23 @@ const mergeOrigin = (
return undefined;
}
const merged: SessionOrigin = existing ? { ...existing } : {};
// A provider/surface/account change is a fresh channel identity (e.g. a dmScope:"main" session
// moving Slack -> Telegram, or between Slack accounts). Channel-keyed fields belong to the prior
// channel; drop them so an inbound that omits them does not keep reactions, native threading, and
// status reads pointed at the previous channel.
const channelChanged =
existing != null &&
((existing.provider != null && next?.provider != null && next.provider !== existing.provider) ||
(existing.surface != null && next?.surface != null && next.surface !== existing.surface) ||
(existing.accountId != null &&
next?.accountId != null &&
next.accountId !== existing.accountId));
if (channelChanged) {
delete merged.nativeChannelId;
delete merged.nativeDirectUserId;
delete merged.accountId;
delete merged.threadId;
}
if (next?.label) {
merged.label = next.label;
}
@@ -0,0 +1,222 @@
// Session origin must drop the prior channel's identity when a dmScope:"main" session moves
// across providers, so channel-keyed fields do not reference a now-inactive channel.
import { describe, expect, it } from "vitest";
import type { MsgContext } from "../../auto-reply/templating.js";
import { buildChannelInboundEventContext } from "../../channels/inbound-event/context.js";
import { deriveSessionMetaPatch } from "./metadata.js";
import type { SessionEntry } from "./types.js";
const sessionKey = "agent:user";
function applyOrigin(existing: SessionEntry | undefined, ctx: Partial<MsgContext>): SessionEntry {
const patch = deriveSessionMetaPatch({
ctx: ctx as MsgContext,
sessionKey,
existing,
});
return { ...existing, ...patch } as SessionEntry;
}
const slackTurn = {
Provider: "slack",
Surface: "slack",
ChatType: "direct",
From: "slack:U0001",
To: "slack:D111SLACK",
NativeChannelId: "D111SLACK",
NativeDirectUserId: "U0001",
AccountId: "slack-team-1",
MessageThreadId: "1700000000.000100",
} satisfies Partial<MsgContext>;
const telegramTurn = {
Provider: "telegram",
Surface: "telegram",
ChatType: "direct",
From: "telegram:42",
To: "telegram:42",
AccountId: "telegram-bot-1",
} satisfies Partial<MsgContext>;
describe("session origin across a channel switch", () => {
it("clears stale channel-keyed fields when provider changes and the new turn omits them", () => {
const afterSlack = applyOrigin(undefined, slackTurn);
expect(afterSlack.origin?.nativeChannelId).toBe("D111SLACK");
expect(afterSlack.origin?.threadId).toBe("1700000000.000100");
const afterTelegram = applyOrigin(afterSlack, telegramTurn);
// Provider/surface flip to Telegram, and the Slack-only identity must not survive.
expect(afterTelegram.origin?.provider).toBe("telegram");
expect(afterTelegram.origin?.surface).toBe("telegram");
expect(afterTelegram.origin?.accountId).toBe("telegram-bot-1");
expect(afterTelegram.origin?.nativeChannelId).toBeUndefined();
expect(afterTelegram.origin?.nativeDirectUserId).toBeUndefined();
expect(afterTelegram.origin?.threadId).toBeUndefined();
});
it("does not re-stamp the prior channel id on subsequent same-channel turns", () => {
const afterSlack = applyOrigin(undefined, slackTurn);
const afterTelegram = applyOrigin(afterSlack, telegramTurn);
const afterTelegramAgain = applyOrigin(afterTelegram, telegramTurn);
expect(afterTelegramAgain.origin?.provider).toBe("telegram");
expect(afterTelegramAgain.origin?.nativeChannelId).toBeUndefined();
expect(afterTelegramAgain.origin?.threadId).toBeUndefined();
});
it("clears stale account id when provider changes and the new turn omits it", () => {
const afterSlack = applyOrigin(undefined, slackTurn);
const telegramWithoutAccount = {
Provider: "telegram",
Surface: "telegram",
ChatType: "direct",
From: "telegram:42",
To: "telegram:42",
} satisfies Partial<MsgContext>;
const afterTelegram = applyOrigin(afterSlack, telegramWithoutAccount);
expect(afterTelegram.origin?.provider).toBe("telegram");
expect(afterTelegram.origin?.accountId).toBeUndefined();
});
it("adopts the new channel's identity when the new turn supplies it", () => {
const afterSlack = applyOrigin(undefined, slackTurn);
const telegramWithChannel = {
...telegramTurn,
NativeChannelId: "C222TG",
MessageThreadId: 555,
} satisfies Partial<MsgContext>;
const afterTelegram = applyOrigin(afterSlack, telegramWithChannel);
expect(afterTelegram.origin?.nativeChannelId).toBe("C222TG");
expect(afterTelegram.origin?.threadId).toBe(555);
});
it("preserves sparse channel metadata across turns on the same provider", () => {
const afterSlack = applyOrigin(undefined, slackTurn);
const slackFollowUp = {
Provider: "slack",
Surface: "slack",
ChatType: "direct",
From: "slack:U0001",
To: "slack:D111SLACK",
AccountId: "slack-team-1",
} satisfies Partial<MsgContext>;
const afterFollowUp = applyOrigin(afterSlack, slackFollowUp);
// Same provider: the established channel id and thread are retained when omitted.
expect(afterFollowUp.origin?.nativeChannelId).toBe("D111SLACK");
expect(afterFollowUp.origin?.threadId).toBe("1700000000.000100");
});
it("clears stale channel-keyed fields when the account changes and the new turn omits them", () => {
const afterSlack = applyOrigin(undefined, slackTurn);
const slackOtherAccount = {
Provider: "slack",
Surface: "slack",
ChatType: "direct",
From: "slack:U0002",
To: "slack:D222SLACK",
AccountId: "slack-team-2",
} satisfies Partial<MsgContext>;
const afterAccountSwitch = applyOrigin(afterSlack, slackOtherAccount);
expect(afterAccountSwitch.origin?.provider).toBe("slack");
expect(afterAccountSwitch.origin?.accountId).toBe("slack-team-2");
expect(afterAccountSwitch.origin?.nativeChannelId).toBeUndefined();
expect(afterAccountSwitch.origin?.nativeDirectUserId).toBeUndefined();
expect(afterAccountSwitch.origin?.threadId).toBeUndefined();
});
it("preserves sparse existing channel metadata when optional identity fields are first populated", () => {
const existing = {
sessionId: "session-1",
updatedAt: 1,
origin: {
provider: "slack",
nativeChannelId: "D111SLACK",
threadId: "1700000000.000100",
},
} satisfies SessionEntry;
const slackFollowUp = {
Provider: "slack",
Surface: "slack",
ChatType: "direct",
From: "slack:U0001",
To: "slack:D111SLACK",
AccountId: "slack-team-1",
} satisfies Partial<MsgContext>;
const afterFollowUp = applyOrigin(existing, slackFollowUp);
expect(afterFollowUp.origin?.surface).toBe("slack");
expect(afterFollowUp.origin?.accountId).toBe("slack-team-1");
expect(afterFollowUp.origin?.nativeChannelId).toBe("D111SLACK");
expect(afterFollowUp.origin?.threadId).toBe("1700000000.000100");
});
});
// Drive the production inbound-event context builder so the bug premise itself is proven, not
// assumed: a Slack DM turn populates ctx.NativeChannelId (from conversation.nativeChannelId), a
// Telegram DM turn omits it, and the same derivation path runs that real context.
function buildDirectTurn(opts: {
provider: string;
from: string;
to: string;
accountId: string;
conversationId: string;
nativeChannelId?: string;
}): MsgContext {
return buildChannelInboundEventContext({
channel: opts.provider,
provider: opts.provider,
surface: opts.provider,
accountId: opts.accountId,
messageId: "m-1",
from: opts.from,
sender: { id: opts.from },
conversation: {
kind: "direct",
id: opts.conversationId,
...(opts.nativeChannelId ? { nativeChannelId: opts.nativeChannelId } : {}),
},
route: { agentId: "main", accountId: opts.accountId, routeSessionKey: sessionKey },
reply: { to: opts.to },
message: { rawBody: "hi" },
}) as MsgContext;
}
describe("session origin across a channel switch (real inbound-event context builder)", () => {
const slackCtx = buildDirectTurn({
provider: "slack",
from: "slack:U0001",
to: "slack:D111SLACK",
accountId: "slack-team-1",
conversationId: "D111SLACK",
nativeChannelId: "D111SLACK",
});
const telegramCtx = buildDirectTurn({
provider: "telegram",
from: "telegram:42",
to: "telegram:42",
accountId: "telegram-bot-1",
conversationId: "42",
});
it("confirms the premise: Slack DM context supplies NativeChannelId, Telegram DM context omits it", () => {
expect(slackCtx.NativeChannelId).toBe("D111SLACK");
expect(telegramCtx.NativeChannelId).toBeUndefined();
});
it("resets the stale Slack channel id after a real-context Slack->Telegram switch", () => {
const afterSlack = applyOrigin(undefined, slackCtx);
expect(afterSlack.origin?.nativeChannelId).toBe("D111SLACK");
const afterTelegram = applyOrigin(afterSlack, telegramCtx);
expect(afterTelegram.origin?.provider).toBe("telegram");
expect(afterTelegram.origin?.nativeChannelId).toBeUndefined();
});
});
+1
View File
@@ -880,6 +880,7 @@ describe("test-projects args", () => {
"test/helpers/temp-dir.test.ts",
"test/scripts/android-pin-version.test.ts",
"test/scripts/bench-cli-startup.test.ts",
"test/scripts/check-package-dist-imports.test.ts",
"test/scripts/control-ui-i18n.test.ts",
"test/scripts/ios-configure-signing.test.ts",
"test/scripts/ios-pin-version.test.ts",