Skip to content

Commit 50d996a

Browse files
TakhoffmanTakhoffman
authored
tests: cron coverage and NO_REPLY delivery fixes (#53366)
* tools: extend seam audit inventory * tools: audit cron seam coverage gaps * test: add cron seam coverage tests * fix: avoid marking NO_REPLY cron deliveries as delivered * fix: clean up delete-after-run NO_REPLY cron sessions
1 parent 6142fc1 commit 50d996a

8 files changed

Lines changed: 662 additions & 35 deletions

File tree

‎scripts/audit-seams.mjs‎

Lines changed: 168 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ const testRoot = path.join(repoRoot, "test");
1313
const workspacePackagePaths = ["ui/package.json"];
1414
const MAX_SCAN_BYTES = 2 * 1024 * 1024;
1515
const compareStrings = (left, right) => left.localeCompare(right);
16-
const HELP_TEXT = `Usage: node scripts/audit-seams.mjs [--help]
16+
export const HELP_TEXT = `Usage: node scripts/audit-seams.mjs [--help]
1717
1818
Audit repo seam inventory and emit JSON to stdout.
1919
@@ -22,7 +22,10 @@ Sections:
2222
overlapFiles Production files that touch multiple seam families
2323
optionalClusterStaticLeaks Optional extension/plugin clusters referenced from the static graph
2424
missingPackages Workspace packages whose deps are not mirrored at the root
25-
seamTestInventory High-signal seam candidates with nearby-test gap signals
25+
seamTestInventory High-signal seam candidates with nearby-test gap signals,
26+
including cron orchestration seams for agent handoff,
27+
outbound/media delivery, heartbeat/followup handoff,
28+
and scheduler state crossings
2629
2730
Notes:
2831
- Output is JSON only.
@@ -531,7 +534,135 @@ function stemFromRelativePath(relativePath) {
531534
return relativePath.replace(/\.(m|c)?[jt]sx?$/, "");
532535
}
533536

534-
function describeSeamKinds(relativePath, source) {
537+
function splitNameTokens(name) {
538+
return name
539+
.split(/[^a-zA-Z0-9]+/)
540+
.map((token) => token.trim().toLowerCase())
541+
.filter(Boolean);
542+
}
543+
544+
function escapeForRegExp(value) {
545+
return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
546+
}
547+
548+
function hasImportSource(source, specifier) {
549+
const escaped = escapeForRegExp(specifier);
550+
return new RegExp(`from\\s+["']${escaped}["']|import\\s*\\(\\s*["']${escaped}["']\\s*\\)`).test(
551+
source,
552+
);
553+
}
554+
555+
function hasAnyImportSource(source, specifiers) {
556+
return specifiers.some((specifier) => hasImportSource(source, specifier));
557+
}
558+
559+
function isCronProductionPath(relativePath) {
560+
return relativePath.startsWith("src/cron/") && isProductionLikeFile(relativePath);
561+
}
562+
563+
function describeCronSeamKinds(relativePath, source) {
564+
if (!isCronProductionPath(relativePath)) {
565+
return [];
566+
}
567+
568+
const seamKinds = [];
569+
const importsAgentRunner = hasAnyImportSource(source, [
570+
"../../agents/cli-runner.js",
571+
"../../agents/pi-embedded.js",
572+
"../../agents/model-fallback.js",
573+
"../../agents/subagent-registry.js",
574+
"../../infra/agent-events.js",
575+
]);
576+
const importsOutboundDelivery = hasAnyImportSource(source, [
577+
"../infra/outbound/deliver.js",
578+
"../../infra/outbound/deliver.js",
579+
"../infra/outbound/session-context.js",
580+
"../../infra/outbound/session-context.js",
581+
"../infra/outbound/identity.js",
582+
"../../infra/outbound/identity.js",
583+
"../cli/outbound-send-deps.js",
584+
"../../cli/outbound-send-deps.js",
585+
]);
586+
const importsHeartbeat = hasAnyImportSource(source, [
587+
"../auto-reply/heartbeat.js",
588+
"../../auto-reply/heartbeat.js",
589+
"../infra/heartbeat-wake.js",
590+
"../../infra/heartbeat-wake.js",
591+
]);
592+
const importsFollowup = hasAnyImportSource(source, [
593+
"./subagent-followup.js",
594+
"../../agents/subagent-registry.js",
595+
"../../agents/tools/agent-step.js",
596+
"../../gateway/call.js",
597+
]);
598+
const importsSchedulerModules =
599+
relativePath.startsWith("src/cron/service/") &&
600+
hasAnyImportSource(source, [
601+
"./jobs.js",
602+
"./store.js",
603+
"./timer.js",
604+
"./state.js",
605+
"../schedule.js",
606+
"../store.js",
607+
"../run-log.js",
608+
]);
609+
610+
if (
611+
importsAgentRunner &&
612+
/\brunCliAgent\b|\brunEmbeddedPiAgent\b|\brunWithModelFallback\b|\bregisterAgentRunContext\b/.test(
613+
source,
614+
)
615+
) {
616+
seamKinds.push("cron-agent-handoff");
617+
}
618+
619+
if (
620+
importsOutboundDelivery &&
621+
/\bdeliverOutboundPayloads\b|\bbuildOutboundSessionContext\b|\bresolveAgentOutboundIdentity\b/.test(
622+
source,
623+
)
624+
) {
625+
seamKinds.push("cron-outbound-delivery");
626+
}
627+
628+
if (
629+
importsHeartbeat &&
630+
/\bstripHeartbeatToken\b|\bHeartbeat\b|\bheartbeat\b|\bnext-heartbeat\b/.test(source)
631+
) {
632+
seamKinds.push("cron-heartbeat-handoff");
633+
}
634+
635+
if (
636+
importsSchedulerModules &&
637+
/\bensureLoaded\b|\bpersist\b|\barmTimer\b|\brunMissedJobs\b|\bcomputeJobNextRunAtMs\b|\brecomputeNextRuns\b|\bnextWakeAtMs\b/.test(
638+
source,
639+
)
640+
) {
641+
seamKinds.push("cron-scheduler-state");
642+
}
643+
644+
if (
645+
importsOutboundDelivery &&
646+
/\bmediaUrl\b|\bmediaUrls\b|\bfilename\b|\baudioAsVoice\b|\bdeliveryPayloads\b|\bdeliveryPayloadHasStructuredContent\b/.test(
647+
source,
648+
)
649+
) {
650+
seamKinds.push("cron-media-delivery");
651+
}
652+
653+
if (
654+
importsFollowup &&
655+
/\bwaitForDescendantSubagentSummary\b|\breadDescendantSubagentFallbackReply\b|\bexpectsSubagentFollowup\b|\bcallGateway\b|\blistDescendantRunsForRequester\b/.test(
656+
source,
657+
)
658+
) {
659+
seamKinds.push("cron-followup-handoff");
660+
}
661+
662+
return seamKinds;
663+
}
664+
665+
export function describeSeamKinds(relativePath, source) {
535666
const seamKinds = [];
536667
const isReplyDeliveryPath =
537668
/reply-delivery|reply-dispatcher|deliver-reply|reply\/.*delivery|monitor\/(?:replies|deliver|native-command)|outbound\/deliver|outbound\/message/.test(
@@ -565,11 +696,12 @@ function describeSeamKinds(relativePath, source) {
565696
}
566697
if (
567698
isReplyDeliveryPath &&
568-
/blockStreamingEnabled|directlySentBlockKeys/.test(source) &&
699+
/blockStreamingEnabled|directlySentBlockKeys|resolveSendableOutboundReplyParts/.test(source) &&
569700
/\bmediaUrl\b|\bmediaUrls\b/.test(source)
570701
) {
571702
seamKinds.push("streaming-media-handoff");
572703
}
704+
seamKinds.push(...describeCronSeamKinds(relativePath, source));
573705
return [...new Set(seamKinds)].toSorted(compareStrings);
574706
}
575707

@@ -593,17 +725,6 @@ async function buildTestIndex(testFiles) {
593725
);
594726
}
595727

596-
function splitNameTokens(name) {
597-
return name
598-
.split(/[^a-zA-Z0-9]+/)
599-
.map((token) => token.trim().toLowerCase())
600-
.filter(Boolean);
601-
}
602-
603-
function escapeForRegExp(value) {
604-
return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
605-
}
606-
607728
function hasExecutableImportReference(source, importPath) {
608729
const escapedImportPath = escapeForRegExp(importPath);
609730
const suffix = String.raw`(?:\.[^"'\\\`]+)?`;
@@ -697,7 +818,7 @@ function findRelatedTests(relativePath, testIndex) {
697818
});
698819
}
699820

700-
function determineSeamTestStatus(seamKinds, relatedTestMatches) {
821+
export function determineSeamTestStatus(seamKinds, relatedTestMatches) {
701822
if (relatedTestMatches.length === 0) {
702823
return {
703824
status: "gap",
@@ -709,7 +830,13 @@ function determineSeamTestStatus(seamKinds, relatedTestMatches) {
709830
if (
710831
seamKinds.includes("reply-delivery-media") ||
711832
seamKinds.includes("streaming-media-handoff") ||
712-
seamKinds.includes("tool-result-media")
833+
seamKinds.includes("tool-result-media") ||
834+
seamKinds.includes("cron-agent-handoff") ||
835+
seamKinds.includes("cron-outbound-delivery") ||
836+
seamKinds.includes("cron-heartbeat-handoff") ||
837+
seamKinds.includes("cron-scheduler-state") ||
838+
seamKinds.includes("cron-media-delivery") ||
839+
seamKinds.includes("cron-followup-handoff")
713840
) {
714841
return {
715842
status: "partial",
@@ -765,22 +892,29 @@ async function buildSeamTestInventory() {
765892
});
766893
}
767894

768-
const args = new Set(process.argv.slice(2));
769-
if (args.has("--help") || args.has("-h")) {
770-
process.stdout.write(`${HELP_TEXT}\n`);
771-
process.exit(0);
772-
}
895+
export async function main(argv = process.argv.slice(2)) {
896+
const args = new Set(argv);
897+
if (args.has("--help") || args.has("-h")) {
898+
process.stdout.write(`${HELP_TEXT}\n`);
899+
return;
900+
}
773901

774-
await collectWorkspacePackagePaths();
775-
const inventory = await collectCorePluginSdkImports();
776-
const optionalClusterStaticLeaks = await collectOptionalClusterStaticLeaks();
777-
const staticLeakClusters = new Set(optionalClusterStaticLeaks.map((entry) => entry.cluster));
778-
const result = {
779-
duplicatedSeamFamilies: buildDuplicatedSeamFamilies(inventory),
780-
overlapFiles: buildOverlapFiles(inventory),
781-
optionalClusterStaticLeaks: buildOptionalClusterStaticLeaks(optionalClusterStaticLeaks),
782-
missingPackages: await buildMissingPackages({ staticLeakClusters }),
783-
seamTestInventory: await buildSeamTestInventory(),
784-
};
902+
await collectWorkspacePackagePaths();
903+
const inventory = await collectCorePluginSdkImports();
904+
const optionalClusterStaticLeaks = await collectOptionalClusterStaticLeaks();
905+
const staticLeakClusters = new Set(optionalClusterStaticLeaks.map((entry) => entry.cluster));
906+
const result = {
907+
duplicatedSeamFamilies: buildDuplicatedSeamFamilies(inventory),
908+
overlapFiles: buildOverlapFiles(inventory),
909+
optionalClusterStaticLeaks: buildOptionalClusterStaticLeaks(optionalClusterStaticLeaks),
910+
missingPackages: await buildMissingPackages({ staticLeakClusters }),
911+
seamTestInventory: await buildSeamTestInventory(),
912+
};
785913

786-
process.stdout.write(`${JSON.stringify(result, null, 2)}\n`);
914+
process.stdout.write(`${JSON.stringify(result, null, 2)}\n`);
915+
}
916+
917+
const entryFilePath = process.argv[1] ? path.resolve(process.argv[1]) : null;
918+
if (entryFilePath === fileURLToPath(import.meta.url)) {
919+
await main();
920+
}

‎src/cron/isolated-agent.skips-delivery-without-whatsapp-recipient-besteffortdeliver-true.test.ts‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
44
import * as modelSelection from "../agents/model-selection.js";
55
import { runSubagentAnnounceFlow } from "../agents/subagent-announce.js";
66
import type { CliDeps } from "../cli/deps.js";
7+
import { callGateway } from "../gateway/call.js";
78
import {
89
createCliDeps,
910
expectDirectTelegramDelivery,
@@ -407,6 +408,60 @@ describe("runCronIsolatedAgentTurn", () => {
407408
});
408409
});
409410

411+
it("does not mark NO_REPLY output as delivered when no direct send occurs", async () => {
412+
await withTelegramAnnounceFixture(async ({ home, storePath, deps }) => {
413+
mockAgentPayloads([{ text: "NO_REPLY" }]);
414+
const res = await runTelegramAnnounceTurn({
415+
home,
416+
storePath,
417+
deps,
418+
delivery: { mode: "announce", channel: "telegram", to: "123" },
419+
});
420+
421+
expect(res.status).toBe("ok");
422+
expect(res.delivered).toBe(false);
423+
expect(runSubagentAnnounceFlow).not.toHaveBeenCalled();
424+
expect(deps.sendMessageTelegram).not.toHaveBeenCalled();
425+
});
426+
});
427+
428+
it("deletes the isolated cron session after NO_REPLY when deleteAfterRun is enabled", async () => {
429+
await withTelegramAnnounceFixture(async ({ home, storePath, deps }) => {
430+
mockAgentPayloads([{ text: "NO_REPLY" }]);
431+
vi.mocked(callGateway).mockClear();
432+
433+
const res = await runCronIsolatedAgentTurn({
434+
cfg: makeCfg(home, storePath, {
435+
channels: { telegram: { botToken: "t-1" } },
436+
}),
437+
deps,
438+
job: {
439+
...makeJob({ kind: "agentTurn", message: "do it" }),
440+
deleteAfterRun: true,
441+
delivery: { mode: "announce", channel: "telegram", to: "123" },
442+
},
443+
message: "do it",
444+
sessionKey: "cron:job-1",
445+
lane: "cron",
446+
});
447+
448+
expect(res.status).toBe("ok");
449+
expect(res.delivered).toBe(false);
450+
expect(deps.sendMessageTelegram).not.toHaveBeenCalled();
451+
expect(callGateway).toHaveBeenCalledTimes(1);
452+
expect(callGateway).toHaveBeenCalledWith(
453+
expect.objectContaining({
454+
method: "sessions.delete",
455+
params: expect.objectContaining({
456+
key: "agent:main:cron:job-1",
457+
deleteTranscript: true,
458+
emitLifecycleHooks: false,
459+
}),
460+
}),
461+
);
462+
});
463+
});
464+
410465
it("fails when structured direct delivery fails and best-effort is disabled", async () => {
411466
await expectStructuredTelegramFailure({
412467
payload: { text: "hello from cron", mediaUrl: "https://example.com/img.png" },

‎src/cron/isolated-agent/delivery-dispatch.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -538,11 +538,12 @@ export async function dispatchCronDelivery(
538538
});
539539
}
540540
if (synthesizedText.toUpperCase() === SILENT_REPLY_TOKEN.toUpperCase()) {
541+
await cleanupDirectCronSessionIfNeeded();
541542
return params.withRunSession({
542543
status: "ok",
543544
summary,
544545
outputText,
545-
delivered: true,
546+
delivered: false,
546547
...params.telemetry,
547548
});
548549
}

0 commit comments

Comments
 (0)