mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-18 13:14:46 +00:00
fix(auto-reply): clean up no-reply migration paths
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
11a321fb232fb821b47721055fb2efdc79921453bd045f64c1186316ab59515a config-baseline.json
|
||||
dbb937da47062678c8305a3111b986ee331440f1d39c2cf95bd284550f0bbf61 config-baseline.core.json
|
||||
a8d34475e1b9f6ef9407172ba2e96d7f5783784073dd0dd5d5c21190613aa002 config-baseline.json
|
||||
60316e075939edea25391c4d5bd2bbc8205b16239d605ecf922033118ae1bfe5 config-baseline.core.json
|
||||
fe4f1cb00d7d1dee9746779ec3cf14236e5f672c91502268a12ad6e467a2c4ad config-baseline.channel.json
|
||||
e9049ce0154f484f44bb0ac174a44198269256044da5ba62a6e107e78bfd7a70 config-baseline.plugin.json
|
||||
|
||||
@@ -202,9 +202,8 @@ raw runner details are shown only when `/verbose` is `on` or `full`.
|
||||
Defaults live under `agents.defaults.silentReply`; `surfaces.<id>.silentReply`
|
||||
can override group/internal policy per surface.
|
||||
|
||||
When the parent session has one or more pending spawned subagent runs, bare
|
||||
silent replies are dropped on all surfaces, so the parent stays quiet until the
|
||||
child completion event delivers the real reply.
|
||||
Bare silent replies are dropped on all surfaces, so parent sessions stay quiet
|
||||
instead of rewriting sentinel text into fallback chatter.
|
||||
|
||||
## Related
|
||||
|
||||
|
||||
@@ -11,7 +11,6 @@ import type { ResolveContextEngineOptions } from "../context-engine/registry.js"
|
||||
import type { ContextEngine, SubagentEndReason } from "../context-engine/types.js";
|
||||
import { callGateway } from "../gateway/call.js";
|
||||
import { getAgentRunContext, onAgentEvent } from "../infra/agent-events.js";
|
||||
import { registerPendingSpawnedChildrenQuery } from "../infra/outbound/pending-spawn-query.js";
|
||||
import { createSubsystemLogger } from "../logging/subsystem.js";
|
||||
import { createLazyImportLoader, createLazyPromiseLoader } from "../shared/lazy-promise.js";
|
||||
import { importRuntimeModule } from "../shared/runtime-import.js";
|
||||
@@ -1244,20 +1243,3 @@ export function initSubagentRegistry() {
|
||||
// Importing this module also registers the subagent maintenance preserve-key
|
||||
// provider as a side effect (see subagent-registry-maintenance.ts).
|
||||
export { listSessionMaintenanceProtectedSubagentSessionKeys } from "./subagent-registry-maintenance.js";
|
||||
|
||||
// Let the shared outbound plan treat bare silent replies as dropped (instead
|
||||
// of rewriting them to visible fallback text) when the parent session has at
|
||||
// least one pending spawned child whose completion will deliver the real
|
||||
// reply. Uses the pending-descendant count so runs that have ended but whose
|
||||
// announce/cleanup is still in flight continue to suppress rewriting; without
|
||||
// this the window between `completeSubagentRun` setting `endedAt` and
|
||||
// `startSubagentAnnounceCleanupFlow` finishing could briefly re-enable
|
||||
// fallback chatter. Runtime-enforced, so it does not rely on agent prompt
|
||||
// compliance.
|
||||
registerPendingSpawnedChildrenQuery((sessionKey) => {
|
||||
const key = sessionKey?.trim();
|
||||
if (!key) {
|
||||
return false;
|
||||
}
|
||||
return countPendingDescendantRuns(key) > 0;
|
||||
});
|
||||
|
||||
@@ -63,7 +63,7 @@ describe("group runtime loading", () => {
|
||||
vi.doUnmock("./groups.runtime.js");
|
||||
});
|
||||
|
||||
it("builds direct chat context from the resolved silent reply policy", () => {
|
||||
it("builds direct chat context without silent-token guidance", () => {
|
||||
expect(
|
||||
groups.buildDirectChatContext({
|
||||
sessionCtx: { ChatType: "direct", Provider: "telegram" },
|
||||
@@ -71,13 +71,6 @@ describe("group runtime loading", () => {
|
||||
).toBe(
|
||||
"You are in a Telegram direct conversation. Your replies are automatically sent to this conversation.",
|
||||
);
|
||||
|
||||
expect(
|
||||
groups.buildDirectChatContext({
|
||||
sessionCtx: { ChatType: "direct", Provider: "telegram" },
|
||||
}),
|
||||
).not.toContain("NO_REPLY");
|
||||
|
||||
expect(
|
||||
groups.buildDirectChatContext({
|
||||
sessionCtx: { ChatType: "direct", Provider: "telegram" },
|
||||
|
||||
@@ -69,6 +69,28 @@ describe("legacy silent reply config migrate", () => {
|
||||
"Removed surfaces.telegram.silentReplyRewrite",
|
||||
]);
|
||||
});
|
||||
|
||||
it("removes malformed silent reply rewrite keys by presence", () => {
|
||||
const res = migrateLegacyConfigForTest({
|
||||
agents: {
|
||||
defaults: {
|
||||
silentReplyRewrite: true,
|
||||
},
|
||||
},
|
||||
surfaces: {
|
||||
telegram: {
|
||||
silentReplyRewrite: false,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
expect(res.config?.agents?.defaults).not.toHaveProperty("silentReplyRewrite");
|
||||
expect(res.config?.surfaces?.telegram).not.toHaveProperty("silentReplyRewrite");
|
||||
expectMigrationChangesToIncludeFragments(res.changes, [
|
||||
"Removed agents.defaults.silentReplyRewrite",
|
||||
"Removed surfaces.telegram.silentReplyRewrite",
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("legacy session maintenance migrate", () => {
|
||||
|
||||
@@ -241,13 +241,19 @@ function removeLegacyAgentRuntimePolicy(
|
||||
}
|
||||
}
|
||||
|
||||
function hasOwnRecordProperty(value: unknown, key: string): boolean {
|
||||
const record = getRecord(value);
|
||||
return Boolean(record && Object.prototype.hasOwnProperty.call(record, key));
|
||||
}
|
||||
|
||||
function hasSurfaceSilentReplyRewrite(value: unknown): boolean {
|
||||
const surfaces = getRecord(value);
|
||||
if (!surfaces) {
|
||||
return false;
|
||||
}
|
||||
return Object.values(surfaces).some(
|
||||
(surface) => getRecord(getRecord(surface)?.silentReplyRewrite) !== null,
|
||||
return Object.entries(surfaces).some(
|
||||
([surfaceId, surface]) =>
|
||||
!isBlockedObjectKey(surfaceId) && hasOwnRecordProperty(surface, "silentReplyRewrite"),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -271,7 +277,7 @@ function removeLegacySilentReplyConfig(raw: Record<string, unknown>, changes: st
|
||||
delete defaultSilentReply.direct;
|
||||
changes.push("Removed agents.defaults.silentReply.direct; direct chats never use NO_REPLY.");
|
||||
}
|
||||
if (defaults && getRecord(defaults.silentReplyRewrite) !== null) {
|
||||
if (defaults && hasOwnRecordProperty(defaults, "silentReplyRewrite")) {
|
||||
delete defaults.silentReplyRewrite;
|
||||
changes.push("Removed agents.defaults.silentReplyRewrite.");
|
||||
}
|
||||
@@ -295,7 +301,7 @@ function removeLegacySilentReplyConfig(raw: Record<string, unknown>, changes: st
|
||||
`Removed surfaces.${surfaceId}.silentReply.direct; direct chats never use NO_REPLY.`,
|
||||
);
|
||||
}
|
||||
if (getRecord(surface.silentReplyRewrite) !== null) {
|
||||
if (hasOwnRecordProperty(surface, "silentReplyRewrite")) {
|
||||
delete surface.silentReplyRewrite;
|
||||
changes.push(`Removed surfaces.${surfaceId}.silentReplyRewrite.`);
|
||||
}
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
import { resolveSendableOutboundReplyParts } from "openclaw/plugin-sdk/reply-payload";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { ReplyPayload } from "../../auto-reply/types.js";
|
||||
import type { OpenClawConfig } from "../../config/types.openclaw.js";
|
||||
import { typedCases } from "../../test-utils/typed-cases.js";
|
||||
import {
|
||||
createOutboundPayloadPlan,
|
||||
@@ -15,7 +14,6 @@ import {
|
||||
projectOutboundPayloadPlanForOutbound,
|
||||
summarizeOutboundPayloadForTransport,
|
||||
} from "./payloads.js";
|
||||
import { registerPendingSpawnedChildrenQuery } from "./pending-spawn-query.js";
|
||||
|
||||
function resolveMirrorProjection(payloads: readonly ReplyPayload[]) {
|
||||
const normalized = normalizeReplyPayloadsForDelivery(payloads);
|
||||
@@ -191,45 +189,20 @@ describe("normalizeReplyPayloadsForDelivery", () => {
|
||||
});
|
||||
|
||||
it("drops bare silent replies for direct conversations", () => {
|
||||
const cfg: OpenClawConfig = {
|
||||
agents: {
|
||||
defaults: {
|
||||
silentReply: {
|
||||
group: "allow",
|
||||
internal: "allow",
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const sessionKey = "agent:main:telegram:direct:123";
|
||||
expect(
|
||||
projectOutboundPayloadPlanForDelivery(
|
||||
createOutboundPayloadPlan([{ text: "NO_REPLY" }], {
|
||||
cfg,
|
||||
sessionKey,
|
||||
sessionKey: "agent:main:telegram:direct:123",
|
||||
surface: "telegram",
|
||||
}),
|
||||
),
|
||||
).toStrictEqual([]);
|
||||
});
|
||||
|
||||
it("drops bare silent replies for groups when policy allows silence", () => {
|
||||
const cfg: OpenClawConfig = {
|
||||
agents: {
|
||||
defaults: {
|
||||
silentReply: {
|
||||
group: "allow",
|
||||
internal: "allow",
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
it("drops bare silent replies for groups", () => {
|
||||
expect(
|
||||
projectOutboundPayloadPlanForDelivery(
|
||||
createOutboundPayloadPlan([{ text: "NO_REPLY" }], {
|
||||
cfg,
|
||||
sessionKey: "agent:main:telegram:group:123",
|
||||
surface: "telegram",
|
||||
}),
|
||||
@@ -238,20 +211,8 @@ describe("normalizeReplyPayloadsForDelivery", () => {
|
||||
});
|
||||
|
||||
it("does not add silent-reply chatter when visible content is already being delivered", () => {
|
||||
const cfg: OpenClawConfig = {
|
||||
agents: {
|
||||
defaults: {
|
||||
silentReply: {
|
||||
group: "allow",
|
||||
internal: "allow",
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const delivery = projectOutboundPayloadPlanForDelivery(
|
||||
createOutboundPayloadPlan([{ text: "NO_REPLY" }, { text: "visible reply" }], {
|
||||
cfg,
|
||||
sessionKey: "agent:main:telegram:direct:123",
|
||||
surface: "telegram",
|
||||
}),
|
||||
@@ -260,50 +221,6 @@ describe("normalizeReplyPayloadsForDelivery", () => {
|
||||
expect(delivery[0]?.text).toBe("visible reply");
|
||||
});
|
||||
|
||||
describe("pending spawned subagent children", () => {
|
||||
const cfg: OpenClawConfig = {
|
||||
agents: {
|
||||
defaults: {
|
||||
silentReply: { group: "allow", internal: "allow" },
|
||||
},
|
||||
},
|
||||
};
|
||||
const planSilent = (sessionKey: string, hasPendingSpawnedChildren?: boolean) =>
|
||||
projectOutboundPayloadPlanForDelivery(
|
||||
createOutboundPayloadPlan([{ text: "NO_REPLY" }], {
|
||||
cfg,
|
||||
sessionKey,
|
||||
surface: "telegram",
|
||||
hasPendingSpawnedChildren,
|
||||
}),
|
||||
);
|
||||
|
||||
it("drops bare silent replies when the context flag is set", () => {
|
||||
expect(planSilent("agent:main:telegram:direct:123", true)).toStrictEqual([]);
|
||||
});
|
||||
|
||||
it("drops bare silent replies via the registered runtime query", () => {
|
||||
const sessionKey = "agent:main:telegram:direct:456";
|
||||
const previousQuery = registerPendingSpawnedChildrenQuery((key) => key === sessionKey);
|
||||
try {
|
||||
expect(planSilent(sessionKey)).toStrictEqual([]);
|
||||
} finally {
|
||||
registerPendingSpawnedChildrenQuery(previousQuery);
|
||||
}
|
||||
});
|
||||
|
||||
it("still drops bare silent replies when the query throws", () => {
|
||||
const previousQuery = registerPendingSpawnedChildrenQuery(() => {
|
||||
throw new Error("registry unavailable");
|
||||
});
|
||||
try {
|
||||
expect(planSilent("agent:main:telegram:direct:789")).toStrictEqual([]);
|
||||
} finally {
|
||||
registerPendingSpawnedChildrenQuery(previousQuery);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it("is idempotent for already-normalized delivery payloads", () => {
|
||||
const once = normalizeReplyPayloadsForDelivery([
|
||||
{
|
||||
|
||||
@@ -6,7 +6,6 @@ import {
|
||||
shouldSuppressReasoningPayload,
|
||||
} from "../../auto-reply/reply/reply-payloads.js";
|
||||
import type { ReplyPayload } from "../../auto-reply/types.js";
|
||||
import { resolveSilentReplySettings } from "../../config/silent-reply.js";
|
||||
import type { OpenClawConfig } from "../../config/types.openclaw.js";
|
||||
import {
|
||||
hasInteractiveReplyBlocks,
|
||||
@@ -18,7 +17,6 @@ import {
|
||||
type ReplyPayloadDelivery,
|
||||
} from "../../interactive/payload.js";
|
||||
import { type SilentReplyConversationType } from "../../shared/silent-reply-policy.js";
|
||||
import { resolvePendingSpawnedChildren } from "./pending-spawn-query.js";
|
||||
|
||||
export type NormalizedOutboundPayload = {
|
||||
text: string;
|
||||
@@ -57,13 +55,6 @@ type OutboundPayloadPlanContext = {
|
||||
sessionKey?: string;
|
||||
surface?: string;
|
||||
conversationType?: SilentReplyConversationType;
|
||||
/**
|
||||
* Set by callers that know the parent session has at least one pending
|
||||
* spawned child whose completion will deliver the real reply. If omitted, the
|
||||
* outbound plan consults the registered runtime query (see
|
||||
* `pending-spawn-query.ts`).
|
||||
*/
|
||||
hasPendingSpawnedChildren?: boolean;
|
||||
extractMarkdownImages?: boolean;
|
||||
};
|
||||
|
||||
@@ -188,14 +179,6 @@ export function createOutboundPayloadPlan(
|
||||
// Intentionally scoped to channel-agnostic normalization and projection inputs.
|
||||
// Transport concerns (queueing, hooks, retries), channel transforms, and
|
||||
// heartbeat-specific token semantics remain outside this plan boundary.
|
||||
const resolvedSilentReplySettings = resolveSilentReplySettings({
|
||||
cfg: context.cfg,
|
||||
sessionKey: context.sessionKey,
|
||||
surface: context.surface,
|
||||
conversationType: context.conversationType,
|
||||
});
|
||||
const hasPendingSpawnedChildren =
|
||||
context.hasPendingSpawnedChildren ?? resolvePendingSpawnedChildren(context.sessionKey);
|
||||
const prepared: IndexedPreparedOutboundPayloadPlanEntry[] = [];
|
||||
for (const [sourceIndex, payload] of payloads.entries()) {
|
||||
const entry = createOutboundPayloadPlanEntry(payload, {
|
||||
@@ -206,16 +189,6 @@ export function createOutboundPayloadPlan(
|
||||
}
|
||||
prepared.push({ ...entry, sourceIndex });
|
||||
}
|
||||
const hasVisibleNonSilentContent = prepared.some((entry) => {
|
||||
if (entry.isSilent) {
|
||||
return false;
|
||||
}
|
||||
const parts = resolveSendableOutboundReplyParts(entry.payload);
|
||||
return hasReplyPayloadContent(
|
||||
{ ...entry.payload, text: parts.text, mediaUrls: parts.mediaUrls },
|
||||
{ hasChannelData: entry.hasChannelData },
|
||||
);
|
||||
});
|
||||
const plan: OutboundPayloadPlan[] = [];
|
||||
for (const entry of prepared) {
|
||||
if (!entry.isSilent) {
|
||||
@@ -229,12 +202,6 @@ export function createOutboundPayloadPlan(
|
||||
});
|
||||
continue;
|
||||
}
|
||||
if (hasVisibleNonSilentContent || resolvedSilentReplySettings.policy === "allow") {
|
||||
continue;
|
||||
}
|
||||
if (hasPendingSpawnedChildren) {
|
||||
continue;
|
||||
}
|
||||
}
|
||||
return plan;
|
||||
}
|
||||
|
||||
@@ -1,57 +0,0 @@
|
||||
import { createHash } from "node:crypto";
|
||||
import { createSubsystemLogger } from "../../logging/subsystem.js";
|
||||
|
||||
/**
|
||||
* Synchronous predicate: does `sessionKey` have pending spawned subagent runs?
|
||||
* Runs on the outbound plan hot path, so implementations must be cheap/bounded
|
||||
* (default in `subagent-registry.ts` is an in-memory map lookup). Internal to
|
||||
* core; not re-exported through `openclaw/plugin-sdk`.
|
||||
*/
|
||||
export type PendingSpawnedChildrenQuery = (sessionKey?: string) => boolean;
|
||||
|
||||
const log = createSubsystemLogger("outbound/pending-spawn");
|
||||
const THROW_LOG_INTERVAL_MS = 60_000;
|
||||
let lastThrowLogAt = 0;
|
||||
let pendingSpawnedChildrenQuery: PendingSpawnedChildrenQuery | undefined;
|
||||
|
||||
export function registerPendingSpawnedChildrenQuery(
|
||||
query: PendingSpawnedChildrenQuery | undefined,
|
||||
): PendingSpawnedChildrenQuery | undefined {
|
||||
const previous = pendingSpawnedChildrenQuery;
|
||||
pendingSpawnedChildrenQuery = query;
|
||||
return previous;
|
||||
}
|
||||
|
||||
function summarizeError(err: unknown): { name: string; message: string } {
|
||||
if (err instanceof Error) {
|
||||
return { name: err.name, message: err.message };
|
||||
}
|
||||
return { name: "Unknown", message: typeof err === "string" ? err : "non-error throw" };
|
||||
}
|
||||
|
||||
function hashSessionKey(key: string | undefined): string | undefined {
|
||||
const trimmed = key?.trim();
|
||||
if (!trimmed) {
|
||||
return undefined;
|
||||
}
|
||||
return createHash("sha256").update(trimmed).digest("hex").slice(0, 12);
|
||||
}
|
||||
|
||||
export function resolvePendingSpawnedChildren(sessionKey: string | undefined): boolean {
|
||||
if (!pendingSpawnedChildrenQuery) {
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
return pendingSpawnedChildrenQuery(sessionKey);
|
||||
} catch (err) {
|
||||
const now = Date.now();
|
||||
if (now - lastThrowLogAt >= THROW_LOG_INTERVAL_MS) {
|
||||
lastThrowLogAt = now;
|
||||
log.warn("pending-spawn query threw; defaulting to false", {
|
||||
err: summarizeError(err),
|
||||
sessionKeyHash: hashSessionKey(sessionKey),
|
||||
});
|
||||
}
|
||||
return false;
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user