mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 08:10:44 +00:00
fix(agents): centralize media delivery evidence
This commit is contained in:
@@ -31,6 +31,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Providers/MiniMax: derive Coding Plan usage polling from the configured MiniMax base URL, so global setups no longer query the CN usage host. Fixes #65054. Thanks @sixone74 and @Yanhu007.
|
||||
- Sessions: reject `sessions_send` targets that resolve to thread-scoped chat sessions, so inter-agent coordination cannot be injected into active human-facing Slack or Discord threads. Fixes #52496. Thanks @barry-p5cc.
|
||||
- Subagents: honor `sessions_spawn` with `expectsCompletionMessage: false` by skipping parent completion handoff delivery while still running child cleanup. Fixes #75848. Thanks @alfredjbclaw.
|
||||
- Media/completions: treat media-only message-tool sends as delivered async completion output, avoiding duplicate raw `MEDIA:` fallback posts after video or music generation finishes.
|
||||
- Gateway/logging: keep deferred channel startup logs on the subsystem logger, so Slack, Discord, Telegram, and voice-call startup messages keep timestamped prefixes. Thanks @vincentkoc.
|
||||
- Replies/typing: keep typing alive for queued follow-up messages that are genuinely waiting behind an active run, instead of making chat surfaces look idle while work is queued. Fixes #65685. Thanks @papag00se.
|
||||
- ACP/Discord: suppress completion announce delivery for inline thread-bound ACP session runs, so Discord thread-bound ACP replies are not delivered twice. Fixes #60780. Thanks @solavrc.
|
||||
|
||||
@@ -160,6 +160,7 @@ const makeAttempt = (overrides: Partial<EmbeddedRunAttemptResult>): EmbeddedRunA
|
||||
const didSendViaMessagingTool = overrides.didSendViaMessagingTool ?? false;
|
||||
const messagingToolSentTexts = overrides.messagingToolSentTexts ?? [];
|
||||
const messagingToolSentMediaUrls = overrides.messagingToolSentMediaUrls ?? [];
|
||||
const messagingToolSentTargets = overrides.messagingToolSentTargets ?? [];
|
||||
const successfulCronAdds = overrides.successfulCronAdds;
|
||||
return {
|
||||
aborted: false,
|
||||
@@ -182,12 +183,13 @@ const makeAttempt = (overrides: Partial<EmbeddedRunAttemptResult>): EmbeddedRunA
|
||||
didSendViaMessagingTool,
|
||||
messagingToolSentTexts,
|
||||
messagingToolSentMediaUrls,
|
||||
messagingToolSentTargets,
|
||||
successfulCronAdds,
|
||||
}),
|
||||
didSendViaMessagingTool,
|
||||
messagingToolSentTexts,
|
||||
messagingToolSentMediaUrls,
|
||||
messagingToolSentTargets: [],
|
||||
messagingToolSentTargets,
|
||||
cloudCodeAssistFormatError: false,
|
||||
itemLifecycle: { startedCount: 0, completedCount: 0, activeCount: 0 },
|
||||
...overrides,
|
||||
|
||||
108
src/agents/pi-embedded-runner/delivery-evidence.ts
Normal file
108
src/agents/pi-embedded-runner/delivery-evidence.ts
Normal file
@@ -0,0 +1,108 @@
|
||||
type AgentPayloadLike = {
|
||||
text?: unknown;
|
||||
mediaUrl?: unknown;
|
||||
mediaUrls?: unknown;
|
||||
presentation?: unknown;
|
||||
interactive?: unknown;
|
||||
channelData?: unknown;
|
||||
isError?: unknown;
|
||||
isReasoning?: unknown;
|
||||
};
|
||||
|
||||
export type AgentDeliveryEvidence = {
|
||||
payloads?: unknown;
|
||||
didSendViaMessagingTool?: unknown;
|
||||
messagingToolSentTexts?: unknown;
|
||||
messagingToolSentMediaUrls?: unknown;
|
||||
messagingToolSentTargets?: unknown;
|
||||
successfulCronAdds?: unknown;
|
||||
meta?: {
|
||||
toolSummary?: {
|
||||
calls?: unknown;
|
||||
};
|
||||
};
|
||||
};
|
||||
|
||||
function hasNonEmptyString(value: unknown): value is string {
|
||||
return typeof value === "string" && value.trim().length > 0;
|
||||
}
|
||||
|
||||
function hasNonEmptyArray(value: unknown): boolean {
|
||||
return Array.isArray(value) && value.length > 0;
|
||||
}
|
||||
|
||||
function hasNonEmptyStringArray(value: unknown): boolean {
|
||||
return Array.isArray(value) && value.some(hasNonEmptyString);
|
||||
}
|
||||
|
||||
function hasPositiveNumber(value: unknown): boolean {
|
||||
return typeof value === "number" && Number.isFinite(value) && value > 0;
|
||||
}
|
||||
|
||||
export function getGatewayAgentResult(response: unknown): AgentDeliveryEvidence | null {
|
||||
if (!response || typeof response !== "object" || !("result" in response)) {
|
||||
return null;
|
||||
}
|
||||
const result = (response as { result?: unknown }).result;
|
||||
if (!result || typeof result !== "object") {
|
||||
return null;
|
||||
}
|
||||
return result as AgentDeliveryEvidence;
|
||||
}
|
||||
|
||||
export function hasVisibleAgentPayload(
|
||||
result: Pick<AgentDeliveryEvidence, "payloads">,
|
||||
options: { includeErrorPayloads?: boolean; includeReasoningPayloads?: boolean } = {},
|
||||
): boolean {
|
||||
const payloads = result.payloads;
|
||||
if (!Array.isArray(payloads)) {
|
||||
return false;
|
||||
}
|
||||
return payloads.some((payload) => {
|
||||
if (!payload || typeof payload !== "object") {
|
||||
return false;
|
||||
}
|
||||
const record = payload as AgentPayloadLike;
|
||||
if (options.includeErrorPayloads === false && record.isError === true) {
|
||||
return false;
|
||||
}
|
||||
if (options.includeReasoningPayloads === false && record.isReasoning === true) {
|
||||
return false;
|
||||
}
|
||||
return Boolean(
|
||||
hasNonEmptyString(record.text) ||
|
||||
hasNonEmptyString(record.mediaUrl) ||
|
||||
hasNonEmptyStringArray(record.mediaUrls) ||
|
||||
record.presentation ||
|
||||
record.interactive ||
|
||||
record.channelData,
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
export function hasMessagingToolDeliveryEvidence(result: AgentDeliveryEvidence): boolean {
|
||||
return (
|
||||
result.didSendViaMessagingTool === true || hasCommittedMessagingToolDeliveryEvidence(result)
|
||||
);
|
||||
}
|
||||
|
||||
export function hasCommittedMessagingToolDeliveryEvidence(
|
||||
result: Pick<
|
||||
AgentDeliveryEvidence,
|
||||
"messagingToolSentTexts" | "messagingToolSentMediaUrls" | "messagingToolSentTargets"
|
||||
>,
|
||||
): boolean {
|
||||
return (
|
||||
hasNonEmptyStringArray(result.messagingToolSentTexts) ||
|
||||
hasNonEmptyStringArray(result.messagingToolSentMediaUrls) ||
|
||||
hasNonEmptyArray(result.messagingToolSentTargets)
|
||||
);
|
||||
}
|
||||
|
||||
export function hasOutboundDeliveryEvidence(result: AgentDeliveryEvidence): boolean {
|
||||
return (
|
||||
hasMessagingToolDeliveryEvidence(result) ||
|
||||
hasPositiveNumber(result.successfulCronAdds) ||
|
||||
hasPositiveNumber(result.meta?.toolSummary?.calls)
|
||||
);
|
||||
}
|
||||
@@ -1,6 +1,7 @@
|
||||
import { isSilentReplyPayloadText } from "../../auto-reply/tokens.js";
|
||||
import { isGpt5ModelId } from "../gpt5-prompt-overlay.js";
|
||||
import type { ModelFallbackResultClassification } from "../model-fallback.js";
|
||||
import { hasOutboundDeliveryEvidence, hasVisibleAgentPayload } from "./delivery-evidence.js";
|
||||
import type { EmbeddedPiRunResult } from "./types.js";
|
||||
|
||||
const EMPTY_TERMINAL_REPLY_RE = /Agent couldn't generate a response/i;
|
||||
@@ -16,31 +17,6 @@ function isEmbeddedPiRunResult(value: unknown): value is EmbeddedPiRunResult {
|
||||
);
|
||||
}
|
||||
|
||||
function hasVisibleNonErrorPayload(result: EmbeddedPiRunResult): boolean {
|
||||
return (result.payloads ?? []).some((payload) => {
|
||||
if (!payload || payload.isError === true || payload.isReasoning === true) {
|
||||
return false;
|
||||
}
|
||||
const text = typeof payload.text === "string" ? payload.text.trim() : "";
|
||||
return (
|
||||
text.length > 0 ||
|
||||
Boolean(payload.mediaUrl) ||
|
||||
(Array.isArray(payload.mediaUrls) && payload.mediaUrls.length > 0)
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
function hasOutboundSideEffects(result: EmbeddedPiRunResult): boolean {
|
||||
return (
|
||||
result.didSendViaMessagingTool === true ||
|
||||
(result.messagingToolSentTexts?.length ?? 0) > 0 ||
|
||||
(result.messagingToolSentMediaUrls?.length ?? 0) > 0 ||
|
||||
(result.messagingToolSentTargets?.length ?? 0) > 0 ||
|
||||
(result.successfulCronAdds ?? 0) > 0 ||
|
||||
(result.meta.toolSummary?.calls ?? 0) > 0
|
||||
);
|
||||
}
|
||||
|
||||
function hasDeliberateSilentTerminalReply(result: EmbeddedPiRunResult): boolean {
|
||||
return [result.meta.finalAssistantRawText, result.meta.finalAssistantVisibleText].some(
|
||||
(text) => typeof text === "string" && isSilentReplyPayloadText(text),
|
||||
@@ -90,11 +66,14 @@ export function classifyEmbeddedPiRunResultForModelFallback(params: {
|
||||
params.result.meta.aborted ||
|
||||
params.hasDirectlySentBlockReply === true ||
|
||||
params.hasBlockReplyPipelineOutput === true ||
|
||||
hasVisibleNonErrorPayload(params.result)
|
||||
hasVisibleAgentPayload(params.result, {
|
||||
includeErrorPayloads: false,
|
||||
includeReasoningPayloads: false,
|
||||
})
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
if (hasOutboundSideEffects(params.result)) {
|
||||
if (hasOutboundDeliveryEvidence(params.result)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { OpenClawConfig } from "../../config/config.js";
|
||||
import { hasCommittedMessagingToolDeliveryEvidence } from "./delivery-evidence.js";
|
||||
import { makeAttemptResult } from "./run.overflow-compaction.fixture.js";
|
||||
import {
|
||||
loadRunOverflowCompactionHarness,
|
||||
@@ -18,7 +19,6 @@ import {
|
||||
DEFAULT_REASONING_ONLY_RETRY_LIMIT,
|
||||
EMPTY_RESPONSE_RETRY_INSTRUCTION,
|
||||
extractPlanningOnlyPlanDetails,
|
||||
hasCommittedUserVisibleToolDelivery,
|
||||
isLikelyExecutionAckPrompt,
|
||||
PLANNING_ONLY_RETRY_INSTRUCTION,
|
||||
REASONING_ONLY_RETRY_INSTRUCTION,
|
||||
@@ -1345,24 +1345,34 @@ describe("runEmbeddedPiAgent incomplete-turn safety", () => {
|
||||
expect(incompleteTurnText).toContain("verify before retrying");
|
||||
});
|
||||
|
||||
it("does not treat empty committed messaging arrays as user-visible delivery", () => {
|
||||
it("does not treat empty committed messaging arrays as delivery", () => {
|
||||
expect(
|
||||
hasCommittedUserVisibleToolDelivery({
|
||||
hasCommittedMessagingToolDeliveryEvidence({
|
||||
messagingToolSentTexts: [" "],
|
||||
messagingToolSentMediaUrls: [],
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("treats committed messaging media as user-visible delivery", () => {
|
||||
it("treats committed messaging media as delivery", () => {
|
||||
expect(
|
||||
hasCommittedUserVisibleToolDelivery({
|
||||
hasCommittedMessagingToolDeliveryEvidence({
|
||||
messagingToolSentTexts: [],
|
||||
messagingToolSentMediaUrls: ["file:///tmp/render.png"],
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("treats committed messaging targets as delivery", () => {
|
||||
expect(
|
||||
hasCommittedMessagingToolDeliveryEvidence({
|
||||
messagingToolSentTexts: [],
|
||||
messagingToolSentMediaUrls: [],
|
||||
messagingToolSentTargets: [{ tool: "message", provider: "slack", to: "channel-1" }],
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("treats committed messaging text as replay-invalid side effect metadata", () => {
|
||||
expect(
|
||||
buildAttemptReplayMetadata({
|
||||
@@ -1385,6 +1395,18 @@ describe("runEmbeddedPiAgent incomplete-turn safety", () => {
|
||||
).toEqual({ hadPotentialSideEffects: true, replaySafe: false });
|
||||
});
|
||||
|
||||
it("treats committed messaging targets as replay-invalid side effect metadata", () => {
|
||||
expect(
|
||||
buildAttemptReplayMetadata({
|
||||
toolMetas: [],
|
||||
didSendViaMessagingTool: false,
|
||||
messagingToolSentTexts: [],
|
||||
messagingToolSentMediaUrls: [],
|
||||
messagingToolSentTargets: [{ tool: "message", provider: "slack", to: "channel-1" }],
|
||||
}),
|
||||
).toEqual({ hadPotentialSideEffects: true, replaySafe: false });
|
||||
});
|
||||
|
||||
it("leaves committed delivery plus tool errors to the tool-error payload path", () => {
|
||||
const incompleteTurnText = resolveIncompleteTurnPayloadText({
|
||||
payloadCount: 0,
|
||||
|
||||
@@ -37,6 +37,7 @@ export function makeAttemptResult(
|
||||
const didSendViaMessagingTool = overrides.didSendViaMessagingTool ?? false;
|
||||
const messagingToolSentTexts = overrides.messagingToolSentTexts ?? [];
|
||||
const messagingToolSentMediaUrls = overrides.messagingToolSentMediaUrls ?? [];
|
||||
const messagingToolSentTargets = overrides.messagingToolSentTargets ?? [];
|
||||
const successfulCronAdds = overrides.successfulCronAdds;
|
||||
return {
|
||||
aborted: false,
|
||||
@@ -58,6 +59,7 @@ export function makeAttemptResult(
|
||||
didSendViaMessagingTool,
|
||||
messagingToolSentTexts,
|
||||
messagingToolSentMediaUrls,
|
||||
messagingToolSentTargets,
|
||||
successfulCronAdds,
|
||||
}),
|
||||
itemLifecycle: {
|
||||
@@ -68,7 +70,7 @@ export function makeAttemptResult(
|
||||
didSendViaMessagingTool,
|
||||
messagingToolSentTexts,
|
||||
messagingToolSentMediaUrls,
|
||||
messagingToolSentTargets: [],
|
||||
messagingToolSentTargets,
|
||||
cloudCodeAssistFormatError: false,
|
||||
...overrides,
|
||||
};
|
||||
|
||||
@@ -88,6 +88,7 @@ import { redactRunIdentifier, resolveRunWorkspaceDir } from "../workspace-run.js
|
||||
import { runPostCompactionSideEffects } from "./compaction-hooks.js";
|
||||
import { buildEmbeddedCompactionRuntimeContext } from "./compaction-runtime-context.js";
|
||||
import { runContextEngineMaintenance } from "./context-engine-maintenance.js";
|
||||
import { hasMessagingToolDeliveryEvidence } from "./delivery-evidence.js";
|
||||
import { resolveEmbeddedRunFailureSignal } from "./failure-signal.js";
|
||||
import { resolveGlobalLane, resolveSessionLane } from "./lanes.js";
|
||||
import { log } from "./logger.js";
|
||||
@@ -1204,7 +1205,7 @@ export async function runEmbeddedPiAgent(
|
||||
? sessionLastAssistant.errorMessage?.trim() || formattedAssistantErrorText
|
||||
: undefined;
|
||||
const canRestartForLiveSwitch =
|
||||
!attempt.didSendViaMessagingTool &&
|
||||
!hasMessagingToolDeliveryEvidence(attempt) &&
|
||||
!attempt.didSendDeterministicApprovalPrompt &&
|
||||
!attempt.lastToolError &&
|
||||
(attempt.toolMetas?.length ?? 0) === 0 &&
|
||||
|
||||
@@ -12,6 +12,10 @@ import {
|
||||
stripProviderPrefix,
|
||||
} from "../../execution-contract.js";
|
||||
import { isLikelyMutatingToolName } from "../../tool-mutation.js";
|
||||
import {
|
||||
hasCommittedMessagingToolDeliveryEvidence,
|
||||
hasMessagingToolDeliveryEvidence,
|
||||
} from "../delivery-evidence.js";
|
||||
import { isZeroUsageEmptyStopAssistantTurn } from "../empty-assistant-turn.js";
|
||||
import { assessLastAssistantMessage } from "../thinking.js";
|
||||
import type { EmbeddedRunLivenessState } from "../types.js";
|
||||
@@ -24,7 +28,8 @@ type ReplayMetadataAttempt = Pick<
|
||||
| "messagingToolSentTexts"
|
||||
| "messagingToolSentMediaUrls"
|
||||
| "successfulCronAdds"
|
||||
>;
|
||||
> &
|
||||
Partial<Pick<EmbeddedRunAttemptResult, "messagingToolSentTargets">>;
|
||||
|
||||
type IncompleteTurnAttempt = Pick<
|
||||
EmbeddedRunAttemptResult,
|
||||
@@ -36,6 +41,7 @@ type IncompleteTurnAttempt = Pick<
|
||||
| "didSendViaMessagingTool"
|
||||
| "messagingToolSentTexts"
|
||||
| "messagingToolSentMediaUrls"
|
||||
| "messagingToolSentTargets"
|
||||
| "lastToolError"
|
||||
| "lastAssistant"
|
||||
| "replayMetadata"
|
||||
@@ -54,6 +60,9 @@ type PlanningOnlyAttempt = Pick<
|
||||
| "lastAssistant"
|
||||
| "itemLifecycle"
|
||||
| "replayMetadata"
|
||||
| "messagingToolSentTexts"
|
||||
| "messagingToolSentMediaUrls"
|
||||
| "messagingToolSentTargets"
|
||||
| "toolMetas"
|
||||
>;
|
||||
|
||||
@@ -185,30 +194,13 @@ export type PlanningOnlyPlanDetails = {
|
||||
steps: string[];
|
||||
};
|
||||
|
||||
function hasStringEntry(values: readonly unknown[] | undefined): boolean {
|
||||
return (
|
||||
Array.isArray(values) &&
|
||||
values.some((value) => typeof value === "string" && value.trim().length > 0)
|
||||
);
|
||||
}
|
||||
|
||||
export function hasCommittedUserVisibleToolDelivery(
|
||||
attempt: Pick<EmbeddedRunAttemptResult, "messagingToolSentTexts" | "messagingToolSentMediaUrls">,
|
||||
): boolean {
|
||||
return (
|
||||
hasStringEntry(attempt.messagingToolSentTexts) ||
|
||||
hasStringEntry(attempt.messagingToolSentMediaUrls)
|
||||
);
|
||||
}
|
||||
|
||||
export function buildAttemptReplayMetadata(
|
||||
params: ReplayMetadataAttempt,
|
||||
): EmbeddedRunAttemptResult["replayMetadata"] {
|
||||
const hadMutatingTools = params.toolMetas.some((t) => isLikelyMutatingToolName(t.toolName));
|
||||
const hadPotentialSideEffects =
|
||||
hadMutatingTools ||
|
||||
params.didSendViaMessagingTool ||
|
||||
hasCommittedUserVisibleToolDelivery(params) ||
|
||||
hasMessagingToolDeliveryEvidence(params) ||
|
||||
(params.successfulCronAdds ?? 0) > 0;
|
||||
return {
|
||||
hadPotentialSideEffects,
|
||||
@@ -244,7 +236,7 @@ export function resolveIncompleteTurnPayloadText(params: {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (hasCommittedUserVisibleToolDelivery(params.attempt)) {
|
||||
if (hasCommittedMessagingToolDeliveryEvidence(params.attempt)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -494,7 +486,7 @@ export function shouldTreatEmptyAssistantReplyAsSilent(params: {
|
||||
if (!params.allowEmptyAssistantReplyAsSilent || shouldSkipPlanningOnlyRetry(params)) {
|
||||
return false;
|
||||
}
|
||||
if (hasCommittedUserVisibleToolDelivery(params.attempt)) {
|
||||
if (hasCommittedMessagingToolDeliveryEvidence(params.attempt)) {
|
||||
return false;
|
||||
}
|
||||
return isNonVisibleAssistantTurnEligibleForSilentReply({
|
||||
@@ -830,7 +822,7 @@ export function resolvePlanningOnlyRetryInstruction(params: {
|
||||
params.attempt.clientToolCall ||
|
||||
params.attempt.yieldDetected ||
|
||||
params.attempt.didSendDeterministicApprovalPrompt ||
|
||||
params.attempt.didSendViaMessagingTool ||
|
||||
hasMessagingToolDeliveryEvidence(params.attempt) ||
|
||||
params.attempt.lastToolError ||
|
||||
(hasNonPlanToolActivity(params.attempt.toolMetas) && !allowSingleActionRetryBypass) ||
|
||||
((params.attempt.itemLifecycle?.startedCount ?? 0) > planOnlyToolMetaCount &&
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
sanitizeForConsole,
|
||||
} from "./pi-embedded-error-observation.js";
|
||||
import { classifyFailoverReason, formatAssistantErrorText } from "./pi-embedded-helpers.js";
|
||||
import { hasCommittedMessagingToolDeliveryEvidence } from "./pi-embedded-runner/delivery-evidence.js";
|
||||
import { isIncompleteTerminalAssistantTurn } from "./pi-embedded-runner/run/incomplete-turn.js";
|
||||
import {
|
||||
consumePendingToolMediaReply,
|
||||
@@ -45,8 +46,7 @@ export function handleAgentEnd(ctx: EmbeddedPiSubscribeContext): void | Promise<
|
||||
ctx.state.assistantTexts.some((text) => hasAssistantVisibleReply({ text }));
|
||||
const hadDeterministicSideEffect =
|
||||
ctx.state.hadDeterministicSideEffect === true ||
|
||||
(ctx.state.messagingToolSentTexts?.length ?? 0) > 0 ||
|
||||
(ctx.state.messagingToolSentMediaUrls?.length ?? 0) > 0 ||
|
||||
hasCommittedMessagingToolDeliveryEvidence(ctx.state) ||
|
||||
(ctx.state.successfulCronAdds ?? 0) > 0;
|
||||
const incompleteTerminalAssistant = isIncompleteTerminalAssistantTurn({
|
||||
hasAssistantVisibleText,
|
||||
|
||||
@@ -10,26 +10,27 @@ import { subscribeEmbeddedPiSession } from "./pi-embedded-subscribe.js";
|
||||
function createBlockReplyHarness(blockReplyBreak: "message_end" | "text_end") {
|
||||
const { session, emit } = createStubSessionHarness();
|
||||
const onBlockReply = vi.fn();
|
||||
subscribeEmbeddedPiSession({
|
||||
const subscription = subscribeEmbeddedPiSession({
|
||||
session,
|
||||
runId: "run",
|
||||
onBlockReply,
|
||||
blockReplyBreak,
|
||||
});
|
||||
return { emit, onBlockReply };
|
||||
return { emit, onBlockReply, subscription };
|
||||
}
|
||||
|
||||
async function emitMessageToolLifecycle(params: {
|
||||
emit: (evt: unknown) => void;
|
||||
toolCallId: string;
|
||||
message: string;
|
||||
media?: string;
|
||||
result: unknown;
|
||||
}) {
|
||||
params.emit({
|
||||
type: "tool_execution_start",
|
||||
toolName: "message",
|
||||
toolCallId: params.toolCallId,
|
||||
args: { action: "send", to: "+1555", message: params.message },
|
||||
args: { action: "send", to: "+1555", message: params.message, media: params.media },
|
||||
});
|
||||
// Wait for async handler to complete.
|
||||
await Promise.resolve();
|
||||
@@ -77,6 +78,23 @@ describe("subscribeEmbeddedPiSession", () => {
|
||||
|
||||
expect(onBlockReply).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("tracks media-only message tool sends as messaging delivery", async () => {
|
||||
const { emit, subscription } = createBlockReplyHarness("message_end");
|
||||
|
||||
await emitMessageToolLifecycle({
|
||||
emit,
|
||||
toolCallId: "tool-message-media",
|
||||
message: "",
|
||||
media: "file:///tmp/render.mp4",
|
||||
result: "ok",
|
||||
});
|
||||
await Promise.resolve();
|
||||
|
||||
expect(subscription.didSendViaMessagingTool()).toBe(true);
|
||||
expect(subscription.getMessagingToolSentMediaUrls()).toEqual(["file:///tmp/render.mp4"]);
|
||||
});
|
||||
|
||||
it("does not suppress message_end replies when message tool reports error", async () => {
|
||||
const { emit, onBlockReply } = createBlockReplyHarness("message_end");
|
||||
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
normalizeTextForComparison,
|
||||
} from "./pi-embedded-helpers.js";
|
||||
import type { BlockReplyPayload } from "./pi-embedded-payloads.js";
|
||||
import { hasCommittedMessagingToolDeliveryEvidence } from "./pi-embedded-runner/delivery-evidence.js";
|
||||
import {
|
||||
createEmbeddedRunReplayState,
|
||||
mergeEmbeddedRunReplayState,
|
||||
@@ -855,8 +856,11 @@ export function subscribeEmbeddedPiSession(params: SubscribeEmbeddedPiSessionPar
|
||||
const resetForCompactionRetry = () => {
|
||||
state.hadDeterministicSideEffect =
|
||||
state.hadDeterministicSideEffect === true ||
|
||||
messagingToolSentTexts.length > 0 ||
|
||||
messagingToolSentMediaUrls.length > 0 ||
|
||||
hasCommittedMessagingToolDeliveryEvidence({
|
||||
messagingToolSentTexts,
|
||||
messagingToolSentMediaUrls,
|
||||
messagingToolSentTargets,
|
||||
}) ||
|
||||
state.successfulCronAdds > 0;
|
||||
assistantTexts.length = 0;
|
||||
toolMetas.length = 0;
|
||||
@@ -998,7 +1002,12 @@ export function subscribeEmbeddedPiSession(params: SubscribeEmbeddedPiSessionPar
|
||||
// Returns true if any messaging tool successfully sent a message.
|
||||
// Used to suppress agent's confirmation text (e.g., "Respondi no Telegram!")
|
||||
// which is generated AFTER the tool sends the actual answer.
|
||||
didSendViaMessagingTool: () => messagingToolSentTexts.length > 0,
|
||||
didSendViaMessagingTool: () =>
|
||||
hasCommittedMessagingToolDeliveryEvidence({
|
||||
messagingToolSentTexts,
|
||||
messagingToolSentMediaUrls,
|
||||
messagingToolSentTargets,
|
||||
}),
|
||||
didSendDeterministicApprovalPrompt: () => state.deterministicApprovalPromptSent,
|
||||
getLastToolError: () => (state.lastToolError ? { ...state.lastToolError } : undefined),
|
||||
getUsageTotals,
|
||||
|
||||
@@ -829,6 +829,45 @@ describe("deliverSubagentAnnouncement completion delivery", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("does not fallback when announce-agent delivered media through the message tool", async () => {
|
||||
const callGateway = createGatewayMock({
|
||||
result: {
|
||||
payloads: [],
|
||||
didSendViaMessagingTool: false,
|
||||
messagingToolSentMediaUrls: ["/tmp/generated-night-drive.mp3"],
|
||||
},
|
||||
});
|
||||
const sendMessage = createSendMessageMock();
|
||||
const result = await deliverDiscordDirectMessageCompletion({
|
||||
callGateway,
|
||||
sendMessage,
|
||||
internalEvents: [
|
||||
{
|
||||
type: "task_completion",
|
||||
source: "music_generation",
|
||||
childSessionKey: "music_generate:task-123",
|
||||
childSessionId: "task-123",
|
||||
announceType: "music generation task",
|
||||
taskLabel: "night-drive synthwave",
|
||||
status: "ok",
|
||||
statusLabel: "completed successfully",
|
||||
result: "Generated 1 track.\nMEDIA:/tmp/generated-night-drive.mp3",
|
||||
mediaUrls: ["/tmp/generated-night-drive.mp3"],
|
||||
replyInstruction: "Deliver the generated music through the message tool.",
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
expect(result).toEqual(
|
||||
expect.objectContaining({
|
||||
delivered: true,
|
||||
path: "direct",
|
||||
}),
|
||||
);
|
||||
expect(callGateway).toHaveBeenCalled();
|
||||
expect(sendMessage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("uses a direct channel fallback when announce-agent returns no visible output", async () => {
|
||||
const callGateway = createGatewayMock({
|
||||
result: {
|
||||
|
||||
@@ -19,6 +19,11 @@ import {
|
||||
} from "../utils/message-channel.js";
|
||||
import { buildAnnounceIdempotencyKey, resolveQueueAnnounceId } from "./announce-idempotency.js";
|
||||
import type { AgentInternalEvent } from "./internal-events.js";
|
||||
import {
|
||||
getGatewayAgentResult,
|
||||
hasMessagingToolDeliveryEvidence,
|
||||
hasVisibleAgentPayload,
|
||||
} from "./pi-embedded-runner/delivery-evidence.js";
|
||||
import {
|
||||
callGateway,
|
||||
createBoundDeliveryRouter,
|
||||
@@ -554,43 +559,10 @@ export function extractThreadCompletionFallbackText(internalEvents?: AgentIntern
|
||||
}
|
||||
|
||||
function hasVisibleGatewayAgentPayload(response: unknown): boolean {
|
||||
const result =
|
||||
response && typeof response === "object" && "result" in response
|
||||
? (response as { result?: unknown }).result
|
||||
: undefined;
|
||||
const payloads =
|
||||
result && typeof result === "object" && "payloads" in result
|
||||
? (result as { payloads?: unknown }).payloads
|
||||
: undefined;
|
||||
if (!Array.isArray(payloads)) {
|
||||
return false;
|
||||
}
|
||||
return payloads.some((payload) => {
|
||||
if (!payload || typeof payload !== "object") {
|
||||
return false;
|
||||
}
|
||||
const record = payload as {
|
||||
text?: unknown;
|
||||
mediaUrl?: unknown;
|
||||
mediaUrls?: unknown;
|
||||
presentation?: unknown;
|
||||
interactive?: unknown;
|
||||
channelData?: unknown;
|
||||
};
|
||||
const text = typeof record.text === "string" ? record.text.trim() : "";
|
||||
const mediaUrl = typeof record.mediaUrl === "string" ? record.mediaUrl.trim() : "";
|
||||
const mediaUrls = Array.isArray(record.mediaUrls)
|
||||
? record.mediaUrls.some((item) => typeof item === "string" && item.trim())
|
||||
: false;
|
||||
return Boolean(
|
||||
text ||
|
||||
mediaUrl ||
|
||||
mediaUrls ||
|
||||
record.presentation ||
|
||||
record.interactive ||
|
||||
record.channelData,
|
||||
);
|
||||
});
|
||||
const result = getGatewayAgentResult(response);
|
||||
return Boolean(
|
||||
result && (hasVisibleAgentPayload(result) || hasMessagingToolDeliveryEvidence(result)),
|
||||
);
|
||||
}
|
||||
|
||||
async function sendCompletionFallback(params: {
|
||||
|
||||
@@ -4,6 +4,8 @@ import { SUBAGENT_ENDED_REASON_COMPLETE } from "./subagent-lifecycle-events.js";
|
||||
import { createSubagentRegistryLifecycleController } from "./subagent-registry-lifecycle.js";
|
||||
import type { SubagentRunRecord } from "./subagent-registry.types.js";
|
||||
|
||||
type LifecycleControllerParams = Parameters<typeof createSubagentRegistryLifecycleController>[0];
|
||||
|
||||
const taskExecutorMocks = vi.hoisted(() => ({
|
||||
completeTaskRunByRunId: vi.fn(),
|
||||
failTaskRunByRunId: vi.fn(),
|
||||
@@ -110,7 +112,7 @@ function createLifecycleController({
|
||||
entry: SubagentRunRecord;
|
||||
runs?: Map<string, SubagentRunRecord>;
|
||||
} & Partial<Parameters<typeof createSubagentRegistryLifecycleController>[0]>) {
|
||||
return createSubagentRegistryLifecycleController({
|
||||
const params: LifecycleControllerParams = {
|
||||
runs,
|
||||
resumedRuns: new Set(),
|
||||
subagentAnnounceTimeoutMs: 1_000,
|
||||
@@ -128,8 +130,9 @@ function createLifecycleController({
|
||||
captureSubagentCompletionReply: vi.fn(async () => "final completion reply"),
|
||||
runSubagentAnnounceFlow: vi.fn(async () => true),
|
||||
warn: vi.fn(),
|
||||
...overrides,
|
||||
});
|
||||
};
|
||||
Object.assign(params, overrides);
|
||||
return createSubagentRegistryLifecycleController(params);
|
||||
}
|
||||
|
||||
describe("subagent registry lifecycle hardening", () => {
|
||||
|
||||
@@ -101,6 +101,7 @@ export function makeEmbeddedRunnerAttempt(
|
||||
const didSendViaMessagingTool = overrides.didSendViaMessagingTool ?? false;
|
||||
const messagingToolSentTexts = overrides.messagingToolSentTexts ?? [];
|
||||
const messagingToolSentMediaUrls = overrides.messagingToolSentMediaUrls ?? [];
|
||||
const messagingToolSentTargets = overrides.messagingToolSentTargets ?? [];
|
||||
const successfulCronAdds = overrides.successfulCronAdds;
|
||||
return {
|
||||
aborted: false,
|
||||
@@ -123,12 +124,13 @@ export function makeEmbeddedRunnerAttempt(
|
||||
didSendViaMessagingTool,
|
||||
messagingToolSentTexts,
|
||||
messagingToolSentMediaUrls,
|
||||
messagingToolSentTargets,
|
||||
successfulCronAdds,
|
||||
}),
|
||||
didSendViaMessagingTool,
|
||||
messagingToolSentTexts,
|
||||
messagingToolSentMediaUrls,
|
||||
messagingToolSentTargets: [],
|
||||
messagingToolSentTargets,
|
||||
cloudCodeAssistFormatError: false,
|
||||
itemLifecycle: { startedCount: 0, completedCount: 0, activeCount: 0 },
|
||||
...overrides,
|
||||
|
||||
Reference in New Issue
Block a user