test(cron): trim message trace comments

This commit is contained in:
Ayaan Zaidi
2026-04-22 07:59:14 +05:30
parent 9db67e79a5
commit 2da1406b29
4 changed files with 1 additions and 26 deletions

View File

@@ -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" },

View File

@@ -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;
}

View File

@@ -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" }],
}),
);

View File

@@ -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 &&