fix(agents): keep merged delivery routes account-bound (#98240)

* fix(agents): keep merged delivery routes account-bound

mergeDeliveryContext gated route-field crossing on channel only, so a
completion origin that knew its account but not a concrete target
inherited a different account's to/threadId on the same channel. A
subagent, cron, or media completion for bot-a could be addressed to
bot-b's chat but sent through bot-a (cross-account misroute) or dropped.

This restores the account-bound guard added in 1ed8592467 and removed as
collateral by 025db6cf9e (PR #89949); same-account and missing-account
merges still backfill so the media route-pin path is preserved. Restores
the deleted regression test.

* fix(agents): centralize account-bound completion routes

---------

Co-authored-by: Peter Steinberger <steipete@golden-gate.local>
This commit is contained in:
Yuval Dinodia
2026-06-30 21:32:35 -04:00
committed by GitHub
parent 35af831fd0
commit 150ca2fedd
4 changed files with 65 additions and 34 deletions

View File

@@ -395,6 +395,27 @@ async function deliverSlackChannelAnnouncement(params: {
}
describe("resolveAnnounceOrigin threaded route targets", () => {
it("does not inherit a target or thread from another account on the same channel", () => {
expect(
resolveAnnounceOrigin(
{
lastChannel: "telegram",
lastTo: "peer-b",
lastAccountId: "bot-b",
lastThreadId: 99,
},
{
channel: "telegram",
accountId: "bot-a",
},
),
).toEqual({
channel: "telegram",
to: undefined,
accountId: "bot-a",
});
});
it("preserves stored thread ids when requester origin omits one for the same chat", () => {
expect(
resolveAnnounceOrigin(

View File

@@ -19,12 +19,7 @@ import {
resolveRequiredCompletionDeliveryFailureTerminalResult,
type RequiredCompletionTerminalResult,
} from "../../tasks/task-completion-contract.js";
import {
deliveryContextFromSession,
normalizeDeliveryContext,
type DeliveryContext,
} from "../../utils/delivery-context.js";
import type { DeliveryContextSessionSource } from "../../utils/delivery-context.types.js";
import { normalizeDeliveryContext, type DeliveryContext } from "../../utils/delivery-context.js";
import {
INTERNAL_MESSAGE_CHANNEL,
isDeliverableMessageChannel,
@@ -65,21 +60,6 @@ export type MediaGenerateBackgroundScheduler = (work: () => Promise<void>) => vo
/** Optional callback invoked when async media generation starts. */
export type MediaGenerateAsyncStartCallback = (message: string) => Promise<void> | void;
function resolvePinnedMediaRequesterOrigin(params: {
requesterOrigin?: DeliveryContext;
sessionEntry?: DeliveryContextSessionSource;
}): DeliveryContext | undefined {
const requesterOrigin = normalizeDeliveryContext(params.requesterOrigin);
const sessionOrigin = deliveryContextFromSession(params.sessionEntry);
const accountsConflict =
requesterOrigin?.accountId &&
sessionOrigin?.accountId &&
requesterOrigin.accountId !== sessionOrigin.accountId;
return accountsConflict
? requesterOrigin
: resolveAnnounceOrigin(params.sessionEntry, requesterOrigin);
}
/** Returns whether a media generation request should detach for a session. */
export function shouldDetachMediaGenerationTask(sessionKey: string | undefined): boolean {
const normalizedSessionKey = sessionKey?.trim();
@@ -168,10 +148,10 @@ function createMediaGenerationTaskRun(params: {
try {
// Pin the complete requester route when detached work starts. Completion-time
// session state can move to another peer while generation is still running.
const requesterOrigin = resolvePinnedMediaRequesterOrigin({
requesterOrigin: params.requesterOrigin,
sessionEntry: loadRequesterSessionEntry(sessionKey).entry,
});
const requesterOrigin = resolveAnnounceOrigin(
loadRequesterSessionEntry(sessionKey).entry,
params.requesterOrigin,
);
const task = createRunningTaskRun({
runtime: "cli",
taskKind: params.taskKind,

View File

@@ -229,7 +229,7 @@ export function deliveryContextFromSession(
return normalizeSessionDeliveryFields(source).deliveryContext;
}
/** Merges delivery contexts without mixing target/account/thread fields across channels. */
/** Merges delivery contexts without mixing target/account/thread fields across route owners. */
export function mergeDeliveryContext(
primary?: DeliveryContext,
fallback?: DeliveryContext,
@@ -243,17 +243,22 @@ export function mergeDeliveryContext(
normalizedPrimary?.channel &&
normalizedFallback?.channel &&
normalizedPrimary.channel !== normalizedFallback.channel;
const accountsConflict =
normalizedPrimary?.accountId &&
normalizedFallback?.accountId &&
normalizedPrimary.accountId !== normalizedFallback.accountId;
const routesConflict = channelsConflict || accountsConflict;
return normalizeDeliveryContext({
channel: normalizedPrimary?.channel ?? normalizedFallback?.channel,
// Keep route fields paired to their channel; avoid crossing fields between
// unrelated channels during session context merges.
to: channelsConflict
? normalizedPrimary?.to
: (normalizedPrimary?.to ?? normalizedFallback?.to),
accountId: channelsConflict
channel: accountsConflict
? normalizedPrimary?.channel
: (normalizedPrimary?.channel ?? normalizedFallback?.channel),
// Keep route fields paired to their channel account; crossing either owner
// can address one account's target through another account's credentials.
to: routesConflict ? normalizedPrimary?.to : (normalizedPrimary?.to ?? normalizedFallback?.to),
accountId: routesConflict
? normalizedPrimary?.accountId
: (normalizedPrimary?.accountId ?? normalizedFallback?.accountId),
threadId: channelsConflict
threadId: routesConflict
? normalizedPrimary?.threadId
: (normalizedPrimary?.threadId ?? normalizedFallback?.threadId),
});

View File

@@ -53,6 +53,31 @@ describe("delivery context helpers", () => {
});
});
it("does not inherit route fields from a different account on the same channel", () => {
const merged = mergeDeliveryContext(
{ channel: "telegram", accountId: "bot-a" },
{ channel: "telegram", to: "123", accountId: "bot-b", threadId: "99" },
);
expect(merged).toEqual({
channel: "telegram",
to: undefined,
accountId: "bot-a",
});
expect(merged?.threadId).toBeUndefined();
expect(
mergeDeliveryContext(
{ accountId: "bot-a" },
{ channel: "telegram", to: "123", accountId: "bot-b", threadId: "99" },
),
).toEqual({
channel: undefined,
to: undefined,
accountId: "bot-a",
});
});
it("uses fallback route fields when fallback has no channel", () => {
const merged = mergeDeliveryContext(
{ channel: "demo-channel" },