From 2da1406b2969229b5fd835f58d2822a733a3fc03 Mon Sep 17 00:00:00 2001 From: Ayaan Zaidi Date: Wed, 22 Apr 2026 07:59:14 +0530 Subject: [PATCH] test(cron): trim message trace comments --- .../delivery-dispatch.named-agent.test.ts | 7 +------ src/cron/isolated-agent/delivery-dispatch.ts | 5 ----- .../isolated-agent/run.message-tool-policy.test.ts | 10 ---------- src/cron/isolated-agent/run.ts | 5 ----- 4 files changed, 1 insertion(+), 26 deletions(-) diff --git a/src/cron/isolated-agent/delivery-dispatch.named-agent.test.ts b/src/cron/isolated-agent/delivery-dispatch.named-agent.test.ts index 4400fcb988e..f1e903b7f51 100644 --- a/src/cron/isolated-agent/delivery-dispatch.named-agent.test.ts +++ b/src/cron/isolated-agent/delivery-dispatch.named-agent.test.ts @@ -72,13 +72,8 @@ describe("matchesMessagingToolDeliveryTarget", () => { ), ).toBe(false); }); + it("matches when delivery has accountId and target omits it (tool fills accountId at exec)", () => { - // message-tool resolves accountId from the agent's bound account at - // execution time (message-tool.ts: `accountId ?? agentAccountId`), so - // an absent target.accountId is equivalent to the delivery's bound - // account. Rejecting this case caused duplicate sends for account-bound - // cron jobs (codex review on PR #69940). CWE-284 spoofing is still - // prevented by the "accountIds differ" case above. expect( matchesMessagingToolDeliveryTarget( { provider: "message", to: "123456" }, diff --git a/src/cron/isolated-agent/delivery-dispatch.ts b/src/cron/isolated-agent/delivery-dispatch.ts index a1bcb8ef567..98b13310097 100644 --- a/src/cron/isolated-agent/delivery-dispatch.ts +++ b/src/cron/isolated-agent/delivery-dispatch.ts @@ -80,11 +80,6 @@ export function matchesMessagingToolDeliveryTarget( if (provider && provider !== "message" && provider !== channel) { return false; } - // CWE-284: when the tool-reported target explicitly names a different - // accountId than the account-bound delivery, reject so attribution cannot - // be spoofed to another bot identity. An omitted target.accountId is - // legitimate — message-tool fills accountId from the agent's bound account - // at exec time, which equals delivery.accountId for account-bound jobs. if (delivery.accountId && target.accountId && target.accountId !== delivery.accountId) { return false; } diff --git a/src/cron/isolated-agent/run.message-tool-policy.test.ts b/src/cron/isolated-agent/run.message-tool-policy.test.ts index 877d746f5e0..b2fb4a3b35d 100644 --- a/src/cron/isolated-agent/run.message-tool-policy.test.ts +++ b/src/cron/isolated-agent/run.message-tool-policy.test.ts @@ -536,12 +536,6 @@ describe("runCronIsolatedAgentTurn message tool policy", () => { }); it("rewrites generic message provider when tool send omits accountId (tool fills at exec)", async () => { - // message-tool resolves accountId from the agent's bound account at exec - // time (message-tool.ts: `accountId ?? agentAccountId`), so a tool call - // that omits accountId is the common path for account-bound cron jobs. - // The trace rewrite must still happen here, otherwise cron's - // delivery-suppression flag is lost and dispatchCronDelivery would - // double-send for account-bound jobs (codex review on PR #69940). mockRunCronFallbackPassthrough(); resolveCronDeliveryPlanMock.mockReturnValue({ requested: true, @@ -585,8 +579,6 @@ describe("runCronIsolatedAgentTurn message tool policy", () => { }); it("does not rewrite generic message provider when tool names a different accountId (spoof guard)", async () => { - // CWE-284: a tool that explicitly sets a foreign accountId must not be - // attributed to this account-bound delivery in the trace. mockRunCronFallbackPassthrough(); resolveCronDeliveryPlanMock.mockReturnValue({ requested: true, @@ -626,8 +618,6 @@ describe("runCronIsolatedAgentTurn message tool policy", () => { expect(result.delivery).toEqual( expect.objectContaining({ - // Channel stays as "message" because the tool named bot-b, which does - // not match the resolved delivery's bot-a binding. messageToolSentTo: [{ channel: "message", to: "123", accountId: "bot-b" }], }), ); diff --git a/src/cron/isolated-agent/run.ts b/src/cron/isolated-agent/run.ts index 71d6f325b88..3abad6a4ace 100644 --- a/src/cron/isolated-agent/run.ts +++ b/src/cron/isolated-agent/run.ts @@ -156,11 +156,6 @@ function normalizeMessagingToolTarget( if (!channel) { return undefined; } - // Rewrite the generic "message" provider to the resolved channel in the - // trace when the tool send actually matches the resolved cron delivery - // target. This makes `intended.channel === messageToolSentTo[i].channel` - // diffable for the happy path, while genuine unmatched generic sends keep - // the literal "message" provider so audits can still flag them. const traceChannel = channel === "message" && resolvedDelivery.ok &&