ewilliams28/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-08-25 11:55:47 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(subagents): preserve announce drop vs none delivery reasons (#122855)

Browse Source 
* fix(subagents): preserve announce drop vs none delivery reasons

Keep live-queue refusals as steer_dropped through dispatch mapping and
registry persistence so they stay distinguishable from no viable
requester (sink_unavailable). Do not mark dropped as terminal, so
completion fallback still returns the primary direct result.

* fix(subagents): persist steer_dropped after completion fallback drop

Carry the nonterminal dropped-steer reason on the retained failed
direct completion result so registry cleanup can persist lastDropReason.

* fix(subagents): keep direct announce reason when steer fallback drops

Completion fallback no longer overwrites the direct failure
classification with steer_dropped. Cleanup still persists
steer_dropped from the fallback phase so queue refusal stays
distinct from sink_unavailable.

* test(subagents): keep visible_reply_missing through dropped steer fallback

Use the no-output Discord completion path so deliverSubagentAnnouncement
keeps the direct classification while the fallback phase records
steer_dropped.

* fix(agents): persist subagent drop diagnostics atomically

Co-authored-by: felirami <6752178+felirami@users.noreply.github.com>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Peter Steinberger <steipete@gmail.com>
Co-authored-by: felirami <6752178+felirami@users.noreply.github.com>
This commit is contained in:
felirami
2026-08-25 04:55:41 -04:00
committed by GitHub
parent 72b06b0077
commit ccd8ede95e
8 changed files with 295 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/agents/subagents/announce/subagent-announce-delivery.test.ts
+52 -1
View File
@@ -1340,8 +1340,15 @@ describe("deliverSubagentAnnouncement active requester steering", () => {
expectRecordFields(result, {
delivered: fallsBack,
path: fallsBack ? "direct" : "none",
...(fallsBack ? {} : { reason: "steer_dropped" }),
phases: [
{ phase: "steer-primary", delivered: false, path: "none", error: undefined },
{
phase: "steer-primary",
delivered: false,
path: "none",
error: undefined,
...(fallsBack ? {} : { reason: "steer_dropped" }),
},
...(fallsBack
? [{ phase: "direct-primary", delivered: true, path: "direct", error: undefined }]
: []),
@@ -2581,6 +2588,50 @@ describe("deliverSubagentAnnouncement completion delivery", () => {
expect(sendMessage).not.toHaveBeenCalled();
});
it("preserves visible_reply_missing when completion direct delivery fails and fallback steering drops", async () => {
const callGateway = createPayloadGatewayMock();
const sendMessage = createSendMessageMock();
const queueEmbeddedAgentMessageWithOutcome = createQueueOutcomeMock(false);
const result = await deliverDiscordDirectMessageCompletion({
callGateway,
sendMessage,
isActive: true,
queueEmbeddedAgentMessageWithOutcome,
internalEvents: taskCompletionEvents({
childSessionId: "child-session-id",
status: "error",
statusLabel: "failed: all models failed",
result: "(no output)",
}),
});
expectRecordFields(result, {
delivered: false,
path: "direct",
error: "completion agent did not produce a visible reply",
reason: "visible_reply_missing",
phases: [
{
phase: "direct-primary",
delivered: false,
path: "direct",
reason: "visible_reply_missing",
error: "completion agent did not produce a visible reply",
},
{
phase: "steer-fallback",
delivered: false,
path: "none",
reason: "steer_dropped",
error: undefined,
},
],
});
expect(result.terminal).toBeUndefined();
expect(queueEmbeddedAgentMessageWithOutcome).toHaveBeenCalled();
expect(sendMessage).not.toHaveBeenCalled();
});
it("reports failure for Telegram DMs when announce-agent delivery fails", async () => {
const callGateway = createGatewayMock({
result: {
src/agents/subagents/announce/subagent-announce-dispatch.test.ts
+82 -21
View File
@@ -28,6 +28,7 @@ describe("runSubagentAnnounceDispatch", () => {
expect(direct).toHaveBeenCalledTimes(1);
expect(result.delivered).toBe(true);
expect(result.path).toBe("direct");
expect(result.reason).toBeUndefined();
expect(result.phases).toEqual([
{ phase: "steer-primary", delivered: false, path: "none", error: undefined },
{ phase: "direct-primary", delivered: true, path: "direct", error: undefined },
@@ -170,28 +171,79 @@ describe("runSubagentAnnounceDispatch", () => {
]);
});
it("returns direct failure when completion fallback steering cannot deliver", async () => {
const steer = vi.fn(async () => ({ status: "none" }) as const);
const direct = vi.fn(async () => ({
delivered: false,
path: "direct" as const,
error: "failed",
}));
it.each([
{
name: "cannot deliver",
steerStatus: "none" as const,
directReason: undefined,
expectedReason: undefined,
fallbackPhase: {
phase: "steer-fallback" as const,
delivered: false,
path: "none" as const,
error: undefined,
},
},
{
name: "drops the new item",
steerStatus: "dropped" as const,
directReason: undefined,
expectedReason: undefined,
fallbackPhase: {
phase: "steer-fallback" as const,
delivered: false,
path: "none" as const,
reason: "steer_dropped" as const,
error: undefined,
},
},
{
name: "drops the new item after a visible reply is missing",
steerStatus: "dropped" as const,
directReason: "visible_reply_missing" as const,
expectedReason: "visible_reply_missing" as const,
fallbackPhase: {
phase: "steer-fallback" as const,
delivered: false,
path: "none" as const,
reason: "steer_dropped" as const,
error: undefined,
},
},
])(
"returns direct failure when completion fallback steering $name",
async ({ steerStatus, directReason, expectedReason, fallbackPhase }) => {
const steer = vi.fn(async () => ({ status: steerStatus }));
const direct = vi.fn(async () => ({
delivered: false,
path: "direct" as const,
error: "failed",
...(directReason ? { reason: directReason } : {}),
}));
const result = await runSubagentAnnounceDispatch({
expectsCompletionMessage: true,
steer,
direct,
});
const result = await runSubagentAnnounceDispatch({
expectsCompletionMessage: true,
steer,
direct,
});
expect(result.delivered).toBe(false);
expect(result.path).toBe("direct");
expect(result.error).toBe("failed");
expect(result.phases).toEqual([
{ phase: "direct-primary", delivered: false, path: "direct", error: "failed" },
{ phase: "steer-fallback", delivered: false, path: "none", error: undefined },
]);
});
expect(result.delivered).toBe(false);
expect(result.path).toBe("direct");
expect(result.error).toBe("failed");
expect(result.reason).toBe(expectedReason);
expect(result.terminal).toBeUndefined();
expect(result.phases).toEqual([
{
phase: "direct-primary",
delivered: false,
path: "direct",
error: "failed",
...(directReason ? { reason: directReason } : {}),
},
fallbackPhase,
]);
},
);
it("returns terminal source ownership loss from completion fallback steering", async () => {
const steer = vi.fn(async () => ({ status: "source_owner_changed" }) as const);
@@ -230,7 +282,16 @@ describe("runSubagentAnnounceDispatch", () => {
expect(result).toEqual({
delivered: false,
path: "none",
phases: [{ phase: "steer-primary", delivered: false, path: "none", error: undefined }],
reason: "steer_dropped",
phases: [
{
phase: "steer-primary",
delivered: false,
path: "none",
reason: "steer_dropped",
error: undefined,
},
],
});
});
src/agents/subagents/announce/subagent-announce-dispatch.ts
+3
View File
@@ -20,6 +20,7 @@ type SubagentAnnounceDeliveryFailureReason =
| "message_tool_delivery_missing"
| "requester_abandoned"
| "source_owner_changed"
| "steer_dropped"
| "visible_reply_missing";
type SubagentAnnounceSteerOutcome =
@@ -81,6 +82,7 @@ function mapSteerOutcomeToDeliveryResult(
return {
delivered: false,
path: "none",
...(outcome.status === "dropped" ? { reason: "steer_dropped" } : {}),
};
}
@@ -174,5 +176,6 @@ export async function runSubagentAnnounceDispatch(params: {
return withPhases(fallbackSteer);
}
// Keep the direct failure authoritative; dropped fallback remains in its phase.
return withPhases(primaryDirect);
}
src/agents/subagents/announce/subagent-announce.format.e2e.test.ts
+9 -1
View File
@@ -2007,8 +2007,16 @@ describe("subagent announce formatting", () => {
});
expect(delivery.delivered).toBe(false);
expect(delivery.reason).toBe("steer_dropped");
expect(delivery.terminal).toBeUndefined();
expect(delivery.phases).toEqual([
{ phase: "steer-primary", delivered: false, path: "none", error: undefined },
{
phase: "steer-primary",
delivered: false,
path: "none",
reason: "steer_dropped",
error: undefined,
},
]);
expect(direct).not.toHaveBeenCalled();
});
src/agents/subagents/registry/subagent-registry-lifecycle-announce-cleanup.ts
+7 -5
View File
@@ -576,6 +576,7 @@ export const startSubagentAnnounceCleanupFlow = (
}
: undefined,
onDeliveryResult: (delivery) => {
const previousDropReason = entry.delivery?.lastDropReason;
if (!context.isCleanupAttemptCurrent(runId, entry, cleanupGeneration)) {
retireSupersededCleanupInBackground(context, runId, entry, cleanupGeneration);
return;
@@ -598,12 +599,13 @@ export const startSubagentAnnounceCleanupFlow = (
latestDeliveryError = undefined;
return;
}
if (delivery.path === "none" && delivery.disposition !== "intentional_non_delivery") {
ensureDeliveryState(entry).lastDropReason = "sink_unavailable";
}
const deliveryState = ensureDeliveryState(entry);
latestDeliveryError = formatAnnounceDeliveryError(delivery);
if (ensureDeliveryState(entry).lastError !== latestDeliveryError) {
ensureDeliveryState(entry).lastError = latestDeliveryError;
if (
deliveryState.lastError !== latestDeliveryError ||
deliveryState.lastDropReason !== previousDropReason
) {
deliveryState.lastError = latestDeliveryError;
params.persist(runId);
}
},
src/agents/subagents/registry/subagent-registry-lifecycle-delivery.ts
+10
View File
@@ -85,6 +85,16 @@ export const recordAnnounceDeliveryResult = (
if (typeof delivery.enqueuedAt === "number") {
deliveryState.enqueuedAt ??= delivery.enqueuedAt;
}
if (!delivery.delivered && delivery.disposition !== "intentional_non_delivery") {
if (
delivery.reason === "steer_dropped" ||
delivery.phases?.some((phase) => phase.reason === "steer_dropped")
) {
deliveryState.lastDropReason = "steer_dropped";
} else if (delivery.path === "none") {
deliveryState.lastDropReason = "sink_unavailable";
}
}
if (delivery.delivered) {
const deliveredAt =
typeof delivery.deliveredAt === "number" ? delivery.deliveredAt : Date.now();
src/agents/subagents/registry/subagent-registry-lifecycle.test.ts
+131 -1
View File
@@ -21,7 +21,10 @@ import {
buildAnnounceIdempotencyKey,
} from "../../announce-idempotency.js";
import { createStructuredOutputTool } from "../../tools/structured-output-tool.js";
import type { SubagentAnnounceDeliveryResult } from "../announce/subagent-announce-dispatch.js";
import {
runSubagentAnnounceDispatch,
type SubagentAnnounceDeliveryResult,
} from "../announce/subagent-announce-dispatch.js";
import {
SUBAGENT_ENDED_REASON_COMPLETE,
SUBAGENT_ENDED_REASON_ERROR,
@@ -2851,6 +2854,133 @@ describe("subagent registry lifecycle hardening", () => {
});
});
it.each([
{
name: "persists steer_dropped when announce mapping preserves a live-queue refusal",
delivery: {
delivered: false as const,
path: "none" as const,
reason: "steer_dropped" as const,
},
lastDropReason: "steer_dropped",
lastError: "steer_dropped",
},
{
name: "persists sink_unavailable when announce mapping reports no viable requester",
delivery: {
delivered: false as const,
path: "none" as const,
},
lastDropReason: "sink_unavailable",
lastError: "delivery path none did not complete",
},
])("$name", async ({ delivery, lastDropReason, lastError }) => {
const persist = vi.fn();
const entry = createRunEntry({
endedAt: 4_000,
expectsCompletionMessage: true,
retainAttachmentsOnKeep: true,
});
const runSubagentAnnounceFlow: LifecycleControllerParams["runSubagentAnnounceFlow"] = vi.fn(
async (announceParams) => {
announceParams.onDeliveryResult?.(delivery);
return "retryable" as const;
},
);
const controller = createLifecycleController({ entry, persist, runSubagentAnnounceFlow });
await expect(
completeRun(controller, entry, {
triggerCleanup: true,
terminalReply: { disposition: "visible", text: "final completion reply" },
}),
).resolves.toBeUndefined();
await waitForLifecycleState(() => expect(entry.delivery?.lastDropReason).toBe(lastDropReason));
expect(entry.delivery?.lastError).toBe(lastError);
expect(entry.delivery?.status).toBe("suspended");
expect(persist).toHaveBeenCalledWith(entry.runId);
});
it.each([
{
name: "persists a newly failed completion",
previousDropReason: undefined,
reusePreviousError: false,
persistCalls: 1,
},
{
name: "persists a changed drop reason when the direct error is unchanged",
previousDropReason: "sink_unavailable" as const,
reusePreviousError: true,
persistCalls: 1,
},
{
name: "does not persist unchanged completion diagnostics",
previousDropReason: "steer_dropped" as const,
reusePreviousError: true,
persistCalls: 0,
},
])("$name before stalled announce bookkeeping settles", async (scenario) => {
const lastError = "failed; visible_reply_missing; direct-primary: failed";
const persist = vi.fn();
const entry = createRunEntry({
endedAt: 4_000,
expectsCompletionMessage: true,
retainAttachmentsOnKeep: true,
delivery: {
status: "pending",
...(scenario.reusePreviousError ? { lastError } : {}),
...(scenario.previousDropReason ? { lastDropReason: scenario.previousDropReason } : {}),
},
});
let releaseAnnounce!: () => void;
const announcePending = new Promise<void>((resolve) => {
releaseAnnounce = resolve;
});
const runSubagentAnnounceFlow: LifecycleControllerParams["runSubagentAnnounceFlow"] = vi.fn(
async (announceParams) => {
const delivery = await runSubagentAnnounceDispatch({
expectsCompletionMessage: true,
steer: async () => ({ status: "dropped" }),
direct: async () => ({
delivered: false,
path: "direct",
error: "failed",
reason: "visible_reply_missing",
}),
});
persist.mockClear();
announceParams.onDeliveryResult?.(delivery);
await announcePending;
return "retryable" as const;
},
);
const controller = createLifecycleController({ entry, persist, runSubagentAnnounceFlow });
try {
await expect(
completeRun(controller, entry, {
triggerCleanup: true,
terminalReply: { disposition: "visible", text: "final completion reply" },
}),
).resolves.toBeUndefined();
await waitForLifecycleState(() => expect(entry.delivery?.disposition).toBe("retryable"));
expect(entry.delivery?.lastDropReason).toBe("steer_dropped");
expect(entry.delivery?.lastError).toBe(lastError);
expect(entry.cleanupCompletedAt).toBeUndefined();
expect(persist).toHaveBeenCalledTimes(scenario.persistCalls);
if (scenario.persistCalls > 0) {
expect(persist).toHaveBeenCalledWith(entry.runId);
}
} finally {
releaseAnnounce();
}
await waitForLifecycleState(() => expect(entry.delivery?.status).toBe("suspended"));
});
it("persists identified completion delivery before stalled announce bookkeeping settles", async () => {
const persist = vi.fn();
const entry = createRunEntry({
src/agents/subagents/registry/subagent-registry.types.ts
+1
View File
@@ -172,6 +172,7 @@ export type SubagentCompletionDeliveryState = {
| "queue_cap"
| "parent_run_ended"
| "sink_unavailable"
| "steer_dropped"
| "dedupe"
| "waiting_for_requester_turn";
};