fix: harden OpenCode ACP bind dispatch

This commit is contained in:
Peter Steinberger
2026-04-25 13:38:00 +01:00
parent 42514156e0
commit dd78b7f773
19 changed files with 348 additions and 31 deletions

View File

@@ -332,6 +332,7 @@ export function createAcpDispatchDeliveryCoordinator(params: {
requesterSenderE164: params.ctx.SenderE164,
threadId: params.ctx.MessageThreadId,
cfg: params.cfg,
mirror: false,
});
if (!result.ok) {
if (tracksVisibleText) {

View File

@@ -0,0 +1,56 @@
import { resolveAcpSessionCwd } from "../../acp/runtime/session-identifiers.js";
import { resolveSessionAgentId } from "../../agents/agent-scope.js";
import { persistAcpTurnTranscript } from "../../agents/command/attempt-execution.js";
import {
loadSessionStore,
resolveSessionStoreEntry,
resolveStorePath,
} from "../../config/sessions.js";
import type { SessionAcpMeta } from "../../config/sessions/types.js";
import type { OpenClawConfig } from "../../config/types.openclaw.js";
export async function persistAcpDispatchTranscript(params: {
cfg: OpenClawConfig;
sessionKey: string;
promptText: string;
finalText: string;
meta?: SessionAcpMeta;
threadId?: string | number;
}): Promise<void> {
const promptText = params.promptText.trim();
const finalText = params.finalText.trim();
if (!promptText && !finalText) {
return;
}
const sessionAgentId = resolveSessionAgentId({
sessionKey: params.sessionKey,
config: params.cfg,
});
const storePath = resolveStorePath(params.cfg.session?.store, {
agentId: sessionAgentId,
});
const sessionStore = loadSessionStore(storePath, { skipCache: true });
const sessionEntry = resolveSessionStoreEntry({
store: sessionStore,
sessionKey: params.sessionKey,
}).existing;
const sessionId = sessionEntry?.sessionId;
if (!sessionId) {
throw new Error(`unknown ACP session key: ${params.sessionKey}`);
}
await persistAcpTurnTranscript({
body: promptText,
transcriptBody: promptText,
finalText,
sessionId,
sessionKey: params.sessionKey,
sessionEntry,
sessionStore,
storePath,
sessionAgentId,
threadId: params.threadId,
sessionCwd: resolveAcpSessionCwd(params.meta) ?? process.cwd(),
});
}

View File

@@ -34,7 +34,9 @@ const policyMocks = vi.hoisted(() => ({
}));
const routeMocks = vi.hoisted(() => ({
routeReply: vi.fn(async (_params: unknown) => ({ ok: true, messageId: "mock" })),
routeReply: vi.fn<
(_params: unknown) => Promise<{ ok: true; messageId: string } | { ok: false; error: string }>
>(async () => ({ ok: true, messageId: "mock" })),
}));
const channelPluginMocks = vi.hoisted(() => ({
@@ -78,6 +80,10 @@ const sessionMetaMocks = vi.hoisted(() => ({
>(() => null),
}));
const transcriptMocks = vi.hoisted(() => ({
persistAcpDispatchTranscript: vi.fn(async (_params: unknown) => undefined),
}));
const bindingServiceMocks = vi.hoisted(() => ({
listBySession: vi.fn<(sessionKey: string) => SessionBindingRecord[]>(() => []),
unbind: vi.fn<(input: unknown) => Promise<SessionBindingRecord[]>>(async () => []),
@@ -162,6 +168,11 @@ vi.mock("./dispatch-acp-session.runtime.js", () => ({
sessionMetaMocks.readAcpSessionEntry(params),
}));
vi.mock("./dispatch-acp-transcript.runtime.js", () => ({
persistAcpDispatchTranscript: (params: unknown) =>
transcriptMocks.persistAcpDispatchTranscript(params),
}));
const sessionKey = "agent:codex-acp:session-1";
const originalFetch = globalThis.fetch;
type MockTtsReply = Awaited<ReturnType<typeof ttsMocks.maybeApplyTtsToPayload>>;
@@ -359,6 +370,7 @@ describe("tryDispatchAcpReply", () => {
mediaUnderstandingMocks.applyMediaUnderstanding.mockResolvedValue(undefined);
sessionMetaMocks.readAcpSessionEntry.mockReset();
sessionMetaMocks.readAcpSessionEntry.mockReturnValue(null);
transcriptMocks.persistAcpDispatchTranscript.mockClear();
bindingServiceMocks.listBySession.mockReset();
bindingServiceMocks.listBySession.mockReturnValue([]);
bindingServiceMocks.unbind.mockReset();
@@ -387,6 +399,26 @@ describe("tryDispatchAcpReply", () => {
expect(dispatcher.sendBlockReply).not.toHaveBeenCalled();
});
it("persists ACP transcript when routed delivery fails", async () => {
setReadyAcpResolution();
mockRoutedTextTurn("hello");
routeMocks.routeReply.mockResolvedValue({ ok: false, error: "missing channel adapter" });
await runDispatch({
bodyForAgent: "reply",
shouldRouteToOriginating: true,
});
expect(transcriptMocks.persistAcpDispatchTranscript).toHaveBeenCalledWith(
expect.objectContaining({
sessionKey,
promptText: "reply",
finalText: "hello",
}),
);
expect(routeMocks.routeReply).toHaveBeenCalledWith(expect.objectContaining({ mirror: false }));
});
it("edits ACP tool lifecycle updates in place when supported", async () => {
setReadyAcpResolution();
mockToolLifecycleTurn("call-1");

View File

@@ -43,6 +43,9 @@ let dispatchAcpSessionRuntimePromise: Promise<
> | null = null;
let dispatchAcpTtsRuntimePromise: Promise<typeof import("./dispatch-acp-tts.runtime.js")> | null =
null;
let dispatchAcpTranscriptRuntimePromise: Promise<
typeof import("./dispatch-acp-transcript.runtime.js")
> | null = null;
function loadDispatchAcpManagerRuntime() {
dispatchAcpManagerRuntimePromise ??= import("./dispatch-acp-manager.runtime.js");
@@ -59,6 +62,11 @@ function loadDispatchAcpTtsRuntime() {
return dispatchAcpTtsRuntimePromise;
}
function loadDispatchAcpTranscriptRuntime() {
dispatchAcpTranscriptRuntimePromise ??= import("./dispatch-acp-transcript.runtime.js");
return dispatchAcpTranscriptRuntimePromise;
}
type DispatchProcessedRecorder = (
outcome: "completed" | "skipped" | "error",
opts?: {
@@ -440,6 +448,30 @@ export async function tryDispatchAcpReply(params: {
});
await projector.flush(true);
if (params.abortSignal?.aborted) {
const counts = params.dispatcher.getQueuedCounts();
delivery.applyRoutedCounts(counts);
params.recordProcessed("completed", { reason: "acp_aborted" });
params.markIdle("message_aborted");
return { queuedFinal, counts };
}
try {
const { persistAcpDispatchTranscript } = await loadDispatchAcpTranscriptRuntime();
await persistAcpDispatchTranscript({
cfg: params.cfg,
sessionKey: canonicalSessionKey,
promptText,
finalText: delivery.getAccumulatedBlockText(),
meta: acpResolution.meta,
threadId: params.ctx.MessageThreadId,
});
} catch (error) {
logVerbose(
`dispatch-acp: transcript persistence failed for ${canonicalSessionKey}: ${formatErrorMessage(
error,
)}`,
);
}
queuedFinal =
(await finalizeAcpTurnOutput({
cfg: params.cfg,

View File

@@ -2153,6 +2153,96 @@ describe("dispatchReplyFromConfig", () => {
expect(replyResolver).toHaveBeenCalled();
});
it("retargets reply_dispatch to a bound generic ACP session before model fallback", async () => {
setNoAbort();
const boundSessionKey = "agent:opencode:acp:bound-session";
const runtime = createAcpRuntime([
{ type: "text_delta", text: "Bound ACP reply" },
{ type: "done" },
]);
acpMocks.readAcpSessionEntry.mockImplementation(
(params: { sessionKey: string; cfg?: OpenClawConfig }) =>
params.sessionKey === boundSessionKey
? {
sessionKey: boundSessionKey,
storeSessionKey: boundSessionKey,
cfg: {},
storePath: "/tmp/mock-sessions.json",
entry: {},
acp: {
backend: "acpx",
agent: "opencode",
runtimeSessionName: "runtime:opencode",
mode: "persistent",
state: "idle",
lastActivityAt: Date.now(),
},
}
: null,
);
acpMocks.requireAcpRuntimeBackend.mockReturnValue({
id: "acpx",
runtime,
});
sessionBindingMocks.resolveByConversation.mockReturnValue({
bindingId: "binding-acp-current",
targetSessionKey: boundSessionKey,
targetKind: "session",
conversation: {
channel: "slack",
accountId: "default",
conversationId: "C123",
},
status: "active",
boundAt: Date.now(),
} satisfies SessionBindingRecord);
const cfg = {
acp: {
enabled: true,
dispatch: { enabled: true },
stream: { coalesceIdleMs: 0, maxChunkChars: 256 },
},
} as OpenClawConfig;
const dispatcher = createDispatcher();
const replyResolver = vi.fn(async () => ({ text: "fallback reply" }) satisfies ReplyPayload);
const ctx = buildTestCtx({
Provider: "slack",
Surface: "slack",
OriginatingChannel: "slack",
OriginatingTo: "slack:C123",
To: "slack:C123",
AccountId: "default",
SessionKey: "agent:main:slack:C123",
BodyForAgent: "continue",
});
const result = await dispatchReplyFromConfig({ ctx, cfg, dispatcher, replyResolver });
expect(result.queuedFinal).toBe(true);
expect(sessionBindingMocks.resolveByConversation).toHaveBeenCalledWith({
channel: "slack",
accountId: "default",
conversationId: "C123",
});
expect(sessionBindingMocks.touch).toHaveBeenCalledWith("binding-acp-current");
expect(runtime.ensureSession).toHaveBeenCalledWith(
expect.objectContaining({
sessionKey: boundSessionKey,
agent: "opencode",
}),
);
expect(runtime.runTurn).toHaveBeenCalledWith(
expect.objectContaining({
text: "continue",
}),
);
expect(replyResolver).not.toHaveBeenCalled();
expect(dispatcher.sendBlockReply).toHaveBeenCalledWith(
expect.objectContaining({ text: "Bound ACP reply" }),
);
});
it("coalesces tiny ACP token deltas into normal Discord text spacing", async () => {
setNoAbort();
const runtime = createAcpRuntime([

View File

@@ -26,6 +26,7 @@ import {
} from "../../hooks/message-hook-mappers.js";
import { isDiagnosticsEnabled } from "../../infra/diagnostic-events.js";
import { formatErrorMessage } from "../../infra/errors.js";
import { getSessionBindingService } from "../../infra/outbound/session-binding-service.js";
import {
logMessageProcessed,
logMessageQueued,
@@ -41,9 +42,10 @@ import {
toPluginConversationBinding,
} from "../../plugins/conversation-binding.js";
import { getGlobalHookRunner, getGlobalPluginRegistry } from "../../plugins/hook-runner-global.js";
import { isAcpSessionKey } from "../../routing/session-key.js";
import { resolveSendPolicy } from "../../sessions/send-policy.js";
import { normalizeLowercaseStringOrEmpty } from "../../shared/string-coerce.js";
import {
normalizeLowercaseStringOrEmpty,
normalizeOptionalLowercaseString,
normalizeOptionalString,
} from "../../shared/string-coerce.js";
@@ -57,6 +59,7 @@ import type { BlockReplyContext } from "../get-reply-options.types.js";
import { getReplyPayloadMetadata, type ReplyPayload } from "../reply-payload.js";
import type { FinalizedMsgContext } from "../templating.js";
import { normalizeVerboseLevel } from "../thinking.js";
import { resolveConversationBindingContextFromMessage } from "./conversation-binding-input.js";
import {
createInternalHookEvent,
loadSessionStore,
@@ -211,6 +214,37 @@ const resolveSessionStoreLookup = (
}
};
const resolveBoundAcpDispatchSessionKey = (params: {
ctx: FinalizedMsgContext;
cfg: OpenClawConfig;
}): string | undefined => {
const bindingContext = resolveConversationBindingContextFromMessage({
cfg: params.cfg,
ctx: params.ctx,
});
if (!bindingContext) {
return undefined;
}
const binding = getSessionBindingService().resolveByConversation({
channel: bindingContext.channel,
accountId: bindingContext.accountId,
conversationId: bindingContext.conversationId,
...(bindingContext.parentConversationId
? { parentConversationId: bindingContext.parentConversationId }
: {}),
});
const targetSessionKey = normalizeOptionalString(binding?.targetSessionKey);
if (!binding || !targetSessionKey || !isAcpSessionKey(targetSessionKey)) {
return undefined;
}
if (isPluginOwnedSessionBindingRecord(binding)) {
return undefined;
}
getSessionBindingService().touch(binding.bindingId);
return targetSessionKey;
};
const createShouldEmitVerboseProgress = (params: {
sessionKey?: string;
storePath?: string;
@@ -300,8 +334,13 @@ export async function dispatchReplyFromConfig(
return { queuedFinal: false, counts: dispatcher.getQueuedCounts() };
}
const sessionStoreEntry = resolveSessionStoreLookup(ctx, cfg);
const acpDispatchSessionKey = sessionStoreEntry.sessionKey ?? sessionKey;
const initialSessionStoreEntry = resolveSessionStoreLookup(ctx, cfg);
const boundAcpDispatchSessionKey = resolveBoundAcpDispatchSessionKey({ ctx, cfg });
const acpDispatchSessionKey =
boundAcpDispatchSessionKey ?? initialSessionStoreEntry.sessionKey ?? sessionKey;
const sessionStoreEntry = boundAcpDispatchSessionKey
? resolveSessionStoreLookup({ ...ctx, SessionKey: boundAcpDispatchSessionKey }, cfg)
: initialSessionStoreEntry;
const sessionAgentId = resolveSessionAgentId({ sessionKey: acpDispatchSessionKey, config: cfg });
const sessionAgentCfg = resolveAgentConfig(cfg, sessionAgentId);
const shouldEmitVerboseProgress = createShouldEmitVerboseProgress({

View File

@@ -38,7 +38,7 @@ const CONNECT_TIMEOUT_MS = 90_000;
const LIVE_TIMEOUT_MS = 240_000;
const DEFAULT_LIVE_CODEX_MODEL = "gpt-5.5";
const DEFAULT_LIVE_PARENT_MODEL = "openai/gpt-5.4";
type LiveAcpAgent = "claude" | "codex" | "gemini";
type LiveAcpAgent = "claude" | "codex" | "gemini" | "opencode";
function createSlackCurrentConversationBindingRegistry() {
return createTestRegistry([
@@ -76,6 +76,9 @@ function normalizeAcpAgent(raw: string | undefined): LiveAcpAgent {
if (normalized === "codex") {
return "codex";
}
if (normalized === "opencode") {
return "opencode";
}
return "claude";
}
@@ -136,6 +139,13 @@ function logLiveStep(message: string): void {
console.info(`[live-acp-bind] ${message}`);
}
function shouldRequireBoundAssistantTranscript(liveAgent: LiveAcpAgent): boolean {
return (
liveAgent === "opencode" ||
isTruthyEnvValue(process.env.OPENCLAW_LIVE_ACP_BIND_REQUIRE_TRANSCRIPT)
);
}
function normalizeOpenAiModelRef(value: string): string {
const trimmed = value.trim();
if (!trimmed) {
@@ -632,6 +642,11 @@ describeLive("gateway live (ACP bind)", () => {
});
} catch {
if (attempt === 2) {
if (shouldRequireBoundAssistantTranscript(liveAgent)) {
throw new Error(
`${liveAgent} ACP bind completed, but the bound session did not emit an assistant transcript`,
);
}
console.error(
`SKIP: ${liveAgent} ACP bind completed, but the bound session did not emit an assistant transcript; skipping post-bind live probes.`,
);
@@ -760,6 +775,11 @@ describeLive("gateway live (ACP bind)", () => {
});
} catch {
if (attempt === 2) {
if (shouldRequireBoundAssistantTranscript(liveAgent)) {
throw new Error(
`${liveAgent} ACP bind completed, but the bound session did not emit the marker transcript`,
);
}
console.error(
`SKIP: ${liveAgent} ACP bind completed, but the bound session did not emit the marker transcript; skipping remaining post-bind live probes.`,
);
@@ -913,7 +933,7 @@ describeLive("gateway live (ACP bind)", () => {
clearRuntimeConfigSnapshot();
await client.stopAndWait({ timeoutMs: 2_000 }).catch(() => {});
await server.close();
await fs.rm(tempRoot, { recursive: true, force: true });
await fs.rm(tempRoot, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
if (previous.configPath === undefined) {
delete process.env.OPENCLAW_CONFIG_PATH;
} else {

View File

@@ -12,6 +12,7 @@ describe("live-agent-probes", () => {
expect(normalizeLiveAgentFamily("claude-cli")).toBe("claude");
expect(normalizeLiveAgentFamily("codex")).toBe("codex");
expect(normalizeLiveAgentFamily("google-gemini-cli")).toBe("gemini");
expect(normalizeLiveAgentFamily("opencode-ai")).toBe("opencode");
});
it("accepts only cat for the shared image probe reply", () => {

View File

@@ -5,7 +5,7 @@ import { normalizeOptionalLowercaseString } from "../shared/string-coerce.js";
const execFileAsync = promisify(execFile);
export type LiveAgentFamily = "claude" | "codex" | "gemini";
export type LiveAgentFamily = "claude" | "codex" | "gemini" | "opencode";
export type CronListCliResult = {
jobs?: Array<{
@@ -39,6 +39,9 @@ export function normalizeLiveAgentFamily(raw: string): LiveAgentFamily {
if (normalized === "gemini" || normalized === "google-gemini-cli") {
return "gemini";
}
if (normalized === "opencode" || normalized === "opencode-ai") {
return "opencode";
}
throw new Error(`unsupported live agent family: ${raw}`);
}