From 2aff537f9fdadd28fb1a864baeabddcfbe6f846b Mon Sep 17 00:00:00 2001 From: Mason Huang Date: Sat, 13 Jun 2026 21:04:21 +0800 Subject: [PATCH] fix(cron): require target proof for delivery verifier --- .../run.message-tool-policy.test.ts | 34 +++++++++++++--- src/cron/isolated-agent/run.ts | 1 + .../outbound/source-delivery-plan.test.ts | 40 +++++++++++++++++++ src/infra/outbound/source-delivery-plan.ts | 8 +++- 4 files changed, 75 insertions(+), 8 deletions(-) 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 252793eea368..baa9d39798b9 100644 --- a/src/cron/isolated-agent/run.message-tool-policy.test.ts +++ b/src/cron/isolated-agent/run.message-tool-policy.test.ts @@ -346,6 +346,7 @@ describe("runCronIsolatedAgentTurn message tool policy", () => { }, messageToolEnabled: true, messageToolForced: false, + requireExplicitMessageTargetEvidence: true, directFallback: true, }), skillsSnapshot: emptySkillsSnapshot, @@ -900,14 +901,35 @@ describe("runCronIsolatedAgentTurn message tool policy", () => { }); }); - it("skips cron fallback delivery when the message tool sends to the bound target", async () => { - await expectCronFallbackSkippedForMessageToolDelivery({ - sentTargets: [], - job: { - id: "message-tool-bound-target", - name: "Message Tool Bound Target", + it("uses cron fallback delivery when the message tool returns no target evidence", async () => { + mockRunCronFallbackPassthrough(); + resolveCronDeliveryPlanMock.mockReturnValue(makeAnnounceDeliveryPlan()); + runEmbeddedAgentMock.mockResolvedValue(makeMessageToolRunResult([])); + + const result = await runCronIsolatedAgentTurn({ + ...makeParams(), + job: makeAnnounceMessageToolJob({ + id: "message-tool-no-target-evidence", + name: "Message Tool No Target Evidence", + }), + }); + + expect(dispatchCronDeliveryMock).toHaveBeenCalledTimes(1); + expectDispatchFields({ + deliveryRequested: true, + sourceDeliveryOutcome: { + visibleDeliveries: [], + verifiedMessageToolDelivery: false, + satisfiesSourceDelivery: false, + unverifiedMessageToolDelivery: false, }, }); + expectDeliveryFields(result.delivery, { + intended: { channel: "messagechat", to: "123", source: "explicit" }, + resolved: { ok: true, channel: "messagechat", to: "123", source: "explicit" }, + fallbackUsed: true, + delivered: true, + }); }); it("rewrites generic message provider to resolved channel in delivery trace", async () => { diff --git a/src/cron/isolated-agent/run.ts b/src/cron/isolated-agent/run.ts index 7a2503f0d9a0..a56437640cc9 100644 --- a/src/cron/isolated-agent/run.ts +++ b/src/cron/isolated-agent/run.ts @@ -335,6 +335,7 @@ function resolveCronSourceDeliveryPlan(params: { target, messageToolEnabled: true, messageToolForced: false, + requireExplicitMessageTargetEvidence: true, directFallback: true, skipFallbackWhenMessageToolSentToTarget: params.resolvedDelivery.ok, }); diff --git a/src/infra/outbound/source-delivery-plan.test.ts b/src/infra/outbound/source-delivery-plan.test.ts index a9fb358d1cd9..f37625e4f157 100644 --- a/src/infra/outbound/source-delivery-plan.test.ts +++ b/src/infra/outbound/source-delivery-plan.test.ts @@ -145,6 +145,46 @@ describe("source delivery plan", () => { expect(outcome.unverifiedMessageToolDelivery).toBe(false); }); + it("synthesizes the planned target for legacy message-tool sends by default", () => { + const contract = createSourceDeliveryPlan({ + owner: "message_tool_then_direct_fallback", + reason: "cron_announce", + target: { channel: "slack", to: "channel:C1" }, + }); + + const outcome = resolveSourceDeliveryOutcome(contract, { + didSendViaMessageTool: true, + }); + + expect(outcome.visibleDeliveries).toEqual([ + { + via: "message_tool", + verifiedTarget: true, + target: { tool: "message", provider: "slack", to: "channel:C1" }, + }, + ]); + expect(outcome.verifiedMessageToolDelivery).toBe(true); + expect(outcome.satisfiesSourceDelivery).toBe(true); + }); + + it("does not synthesize the planned target when explicit target evidence is required", () => { + const contract = createSourceDeliveryPlan({ + owner: "message_tool_then_direct_fallback", + reason: "cron_announce", + target: { channel: "slack", to: "channel:C1" }, + requireExplicitMessageTargetEvidence: true, + }); + + const outcome = resolveSourceDeliveryOutcome(contract, { + didSendViaMessageTool: true, + }); + + expect(outcome.visibleDeliveries).toEqual([]); + expect(outcome.verifiedMessageToolDelivery).toBe(false); + expect(outcome.satisfiesSourceDelivery).toBe(false); + expect(outcome.unverifiedMessageToolDelivery).toBe(false); + }); + it("does not synthesize an implicit target without a concrete recipient", () => { const contract = createSourceDeliveryPlan({ owner: "direct_fallback", diff --git a/src/infra/outbound/source-delivery-plan.ts b/src/infra/outbound/source-delivery-plan.ts index 7e9f2a8d28f8..cb0a9a03ec25 100644 --- a/src/infra/outbound/source-delivery-plan.ts +++ b/src/infra/outbound/source-delivery-plan.ts @@ -69,6 +69,7 @@ export type SourceDeliveryPlan = { enabled: boolean; force: boolean; requireExplicitTarget: boolean; + requireExplicitTargetEvidence: boolean; defaultTarget: boolean; }; fallback: { @@ -174,6 +175,7 @@ export function createSourceDeliveryPlan(params: { messageToolEnabled?: boolean; messageToolForced?: boolean; requireExplicitMessageTarget?: boolean; + requireExplicitMessageTargetEvidence?: boolean; directFallback?: boolean; skipFallbackWhenMessageToolSentToTarget?: boolean; fallbackBestEffort?: boolean; @@ -197,6 +199,7 @@ export function createSourceDeliveryPlan(params: { enabled: params.messageToolEnabled ?? messageToolOwnsDelivery, force: params.messageToolForced ?? messageToolOwnsDelivery, requireExplicitTarget: params.requireExplicitMessageTarget ?? false, + requireExplicitTargetEvidence: params.requireExplicitMessageTargetEvidence ?? false, defaultTarget: Boolean(params.target?.channel || params.target?.to), }, fallback: { @@ -239,11 +242,12 @@ export function resolveSourceDeliveryOutcome( ): SourceDeliveryOutcome { const didSendViaMessageTool = params.didSendViaMessageTool === true; const explicitTargets = params.messageToolSentTargets ?? []; - // A send without explicit target metadata still counts when the plan has a default target. + // Cron completion accounting needs concrete target evidence. Legacy + // message-tool-owned flows may still use the plan target as the implicit send. const sentTargets = explicitTargets.length > 0 ? explicitTargets - : didSendViaMessageTool + : didSendViaMessageTool && !plan.messageTool.requireExplicitTargetEvidence ? [resolveImplicitMessageToolDeliveryTarget(plan)].filter( (target): target is SourceDeliveryMessageToolTarget => Boolean(target), )