Skip to content

Commit 30b39c6

Browse files
Jerry-Xin忻役
Jerry-Xin
and
忻役
authored
fix(cron): guard against undefined sourceDelivery in isolated executor (#85249)
* fix(cron): guard against undefined sourceDelivery in isolated executor * fix: narrow legacy sourceReplyDeliveryMode to closed union Validate legacy sourceReplyDeliveryMode values against the SourceReplyDeliveryMode union before forwarding them to typed runner APIs. Unrecognized legacy values now fall back to the sourceDelivery plan with a warning log. * fix: align test imports with upstream renames - SkillSnapshot moved from agents/skills.js to skills/types.js - runEmbeddedPiAgentMock renamed to runEmbeddedAgentMock * fix(cron): align sourceDelivery fallback defaults with current cron semantics * refactor(cron): extract shared source-delivery fallback helper aligned with current main semantics Replace inline legacy field sniffing in both createCronPromptExecutor and executeCronRun with a shared resolveFallbackCronSourceDeliveryPlan helper that derives webhook/none/announce plans from resolveCronDeliveryPlan(job) and resolvedDelivery, matching current main's resolveCronSourceDeliveryPlan exactly. Changes: - Created source-delivery-fallback.ts with shared helper - Removed all legacy toolPolicy/sourceReplyDeliveryMode/messageChannel reads - Updated guard tests to drive fallback via job delivery config - Added tests verifying stale legacy fields are ignored - All 12 guard tests pass, 99 cron files / 1002 tests pass * refactor(cron): unify source-delivery planner into single shared helper Extract resolveCronSourceDeliveryPlan as the single canonical cron source-delivery planner in source-delivery-fallback.ts. Both the normal cron execution path (run.ts) and the version-skew fallback path (run-executor.ts) now use this shared helper, eliminating the duplicate implementation that ClawSweeper flagged as P1. - Remove private resolveCronSourceDeliveryPlan from run.ts - Export shared resolveCronSourceDeliveryPlan from source-delivery-fallback.ts - Keep resolveFallbackCronSourceDeliveryPlan as thin wrapper for executor - Add CronSourceDeliveryResolvedTarget type for shared contract - Add parity tests verifying normal path matches fallback wrapper - All 1000 cron tests pass, autoreview clean * fix: preserve announce fallback skip state when resolvedDelivery.ok is absent When a stale executor artifact passes resolvedDelivery without the ok field, the announce fallback path set skipFallbackWhenMessageToolSentToTarget to undefined (falsy), causing the direct fallback to fire even when the message tool already delivered — resulting in double-posted messages. Default ok to true (skip fallback when message tool sent) so stale callers preserve duplicate suppression. Callers that explicitly set ok=false still get fallback delivery as intended. Add regression test for the missing-ok announce fallback path. --------- Co-authored-by: 忻役 <[email protected]>
1 parent 0647260 commit 30b39c6

4 files changed

Lines changed: 589 additions & 55 deletions

File tree

‎src/cron/isolated-agent/run-executor.ts‎

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import type {
3939
PersistCronSessionEntry,
4040
} from "./run-session-state.js";
4141
import { syncCronSessionLiveSelection } from "./run-session-state.js";
42+
import { resolveFallbackCronSourceDeliveryPlan } from "./source-delivery-fallback.js";
4243
import { isLikelyInterimCronMessage } from "./subagent-followup-hints.js";
4344

4445
type AgentTurnPayload = Extract<CronJob["payload"], { kind: "agentTurn" }> | null;
@@ -210,11 +211,12 @@ export function createCronPromptExecutor(params: {
210211
accountId?: string;
211212
to?: string;
212213
threadId?: string | number;
214+
ok?: boolean;
213215
};
214216
resolvedDeliveryOk: boolean;
215217
messageToolPromptEnabled: boolean;
216218
deliveryRequested?: boolean;
217-
sourceDelivery: SourceDeliveryPlan;
219+
sourceDelivery?: SourceDeliveryPlan;
218220
skillsSnapshot: SkillSnapshot;
219221
agentPayload: AgentTurnPayload;
220222
useSubagentFallbacks: boolean;
@@ -260,13 +262,21 @@ export function createCronPromptExecutor(params: {
260262
params.cronSession.sessionEntry.systemPromptReport,
261263
);
262264
const bootstrapContextMode = resolveCronBootstrapContextMode(params.agentPayload);
263-
const sourceReplyDeliveryMode = params.sourceDelivery.sourceReplyDeliveryMode;
264-
const messageChannel = params.sourceDelivery.target.channel ?? params.resolvedDelivery.channel;
265+
if (!params.sourceDelivery) {
266+
logWarn(
267+
`[cron:${params.job.id}] sourceDelivery is undefined; using fallback — possible build artifact mismatch`,
268+
);
269+
}
270+
const sourceDelivery =
271+
params.sourceDelivery ??
272+
resolveFallbackCronSourceDeliveryPlan(params.job, params.resolvedDelivery);
273+
const sourceReplyDeliveryMode = sourceDelivery.sourceReplyDeliveryMode;
274+
const messageChannel = sourceDelivery.target.channel ?? params.resolvedDelivery.channel;
265275
const deliveryTargetRuntimeContext = buildCronDeliveryTargetRuntimeContext({
266276
resolvedDeliveryOk: params.resolvedDeliveryOk,
267277
messageToolPromptEnabled: params.messageToolPromptEnabled,
268278
resolvedDelivery: params.resolvedDelivery,
269-
sourceDelivery: params.sourceDelivery,
279+
sourceDelivery,
270280
});
271281

272282
const runPrompt = async (promptText: string) => {
@@ -338,7 +348,7 @@ export function createCronPromptExecutor(params: {
338348
skillsSnapshot: params.skillsSnapshot,
339349
messageChannel,
340350
sourceReplyDeliveryMode,
341-
requireExplicitMessageTarget: params.sourceDelivery.messageTool.requireExplicitTarget,
351+
requireExplicitMessageTarget: sourceDelivery.messageTool.requireExplicitTarget,
342352
toolsAllow: resolveCliRuntimeToolsAllow(
343353
params.agentPayload?.toolsAllow,
344354
params.agentPayload?.toolsAllowIsDefault,
@@ -437,9 +447,9 @@ export function createCronPromptExecutor(params: {
437447
: undefined,
438448
sourceReplyDeliveryMode,
439449
runId: params.cronSession.sessionEntry.sessionId,
440-
requireExplicitMessageTarget: params.sourceDelivery.messageTool.requireExplicitTarget,
441-
disableMessageTool: !params.sourceDelivery.messageTool.enabled,
442-
forceMessageTool: params.sourceDelivery.messageTool.force,
450+
requireExplicitMessageTarget: sourceDelivery.messageTool.requireExplicitTarget,
451+
disableMessageTool: !sourceDelivery.messageTool.enabled,
452+
forceMessageTool: sourceDelivery.messageTool.force,
443453
allowTransientCooldownProbe: runOptions?.allowTransientCooldownProbe,
444454
abortSignal: params.abortSignal,
445455
onExecutionStarted: params.onExecutionStarted,
@@ -490,11 +500,12 @@ export async function executeCronRun(params: {
490500
accountId?: string;
491501
to?: string;
492502
threadId?: string | number;
503+
ok?: boolean;
493504
};
494505
resolvedDeliveryOk: boolean;
495506
messageToolPromptEnabled: boolean;
496507
deliveryRequested?: boolean;
497-
sourceDelivery: SourceDeliveryPlan;
508+
sourceDelivery?: SourceDeliveryPlan;
498509
skillsSnapshot: SkillSnapshot;
499510
agentPayload: AgentTurnPayload;
500511
useSubagentFallbacks: boolean;
@@ -530,6 +541,14 @@ export async function executeCronRun(params: {
530541
sessionId: params.cronSession.sessionEntry.sessionId,
531542
verboseLevel: resolvedVerboseLevel,
532543
});
544+
if (!params.sourceDelivery) {
545+
logWarn(
546+
`[cron:${params.job.id}] sourceDelivery is undefined; using fallback — possible build artifact mismatch`,
547+
);
548+
}
549+
const sourceDelivery =
550+
params.sourceDelivery ??
551+
resolveFallbackCronSourceDeliveryPlan(params.job, params.resolvedDelivery);
533552
const executor = createCronPromptExecutor({
534553
cfg: params.cfg,
535554
cfgWithAgentDefaults: params.cfgWithAgentDefaults,
@@ -549,7 +568,7 @@ export async function executeCronRun(params: {
549568
resolvedDeliveryOk: params.resolvedDeliveryOk,
550569
messageToolPromptEnabled: params.messageToolPromptEnabled,
551570
deliveryRequested: params.deliveryRequested,
552-
sourceDelivery: params.sourceDelivery,
571+
sourceDelivery,
553572
skillsSnapshot: params.skillsSnapshot,
554573
agentPayload: params.agentPayload,
555574
useSubagentFallbacks: params.useSubagentFallbacks,

0 commit comments

Comments
 (0)