Skip to content

Commit 9b9a124

Browse files
omarshahineomarshahine
authored
Fix Codex message-tool-only source reply completion (#95942)
* fix(codex): recognize message tool source replies * fix(codex): accept numeric source message ids * fix(codex): account for source reply SDK surface * fix(codex): require delivered reply receipts * fix(codex): reject alias-routed source replies * fix(codex): require delivered non-send telemetry * fix(codex): honor normalized source routes --------- Co-authored-by: Omar Shahine <[email protected]>
1 parent 4b36cf4 commit 9b9a124

9 files changed

Lines changed: 950 additions & 13 deletions

‎extensions/codex/src/app-server/dynamic-tools.test.ts‎

Lines changed: 579 additions & 0 deletions
Large diffs are not rendered by default.

‎extensions/codex/src/app-server/dynamic-tools.ts‎

Lines changed: 289 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import {
1818
getChannelAgentToolMeta,
1919
getPluginToolMeta,
2020
type EmbeddedRunAttemptParams,
21+
isDeliveredMessageToolOnlySourceReplyResult,
22+
isDeliveredMessagingToolResult,
2123
isReplaySafeToolCall,
2224
isToolWrappedWithBeforeToolCallHook,
2325
isToolResultError,
@@ -63,9 +65,11 @@ type CodexDynamicToolHookContext = {
6365
currentChannelProvider?: string;
6466
currentChannelId?: string;
6567
currentMessagingTarget?: string;
68+
currentMessageId?: string | number;
6669
currentThreadId?: string;
6770
replyToMode?: "off" | "first" | "all" | "batched";
6871
hasRepliedRef?: { value: boolean };
72+
sourceReplyDeliveryMode?: EmbeddedRunAttemptParams["sourceReplyDeliveryMode"];
6973
onToolOutcome?: EmbeddedRunAttemptParams["onToolOutcome"];
7074
allocateToolOutcomeOrdinal?: EmbeddedRunAttemptParams["allocateToolOutcomeOrdinal"];
7175
};
@@ -100,6 +104,225 @@ function applyCurrentMessageProvider(
100104
return { ...args, provider };
101105
}
102106

107+
function normalizeRouteToken(value: string | number | undefined): string | undefined {
108+
if (typeof value === "number") {
109+
return Number.isFinite(value) ? String(value) : undefined;
110+
}
111+
const normalized = value?.trim().toLowerCase();
112+
return normalized ? normalized : undefined;
113+
}
114+
115+
function sourceRouteTokens(hookContext: CodexDynamicToolHookContext | undefined): Set<string> {
116+
const tokens = new Set<string>();
117+
const currentTarget = normalizeRouteToken(hookContext?.currentMessagingTarget);
118+
const currentChannel = normalizeRouteToken(hookContext?.currentChannelId);
119+
const currentProvider = normalizeRouteToken(hookContext?.currentChannelProvider);
120+
if (currentTarget) {
121+
tokens.add(currentTarget);
122+
}
123+
if (currentChannel) {
124+
tokens.add(currentChannel);
125+
}
126+
const channelPrefixIndex = currentChannel?.indexOf(":") ?? -1;
127+
if (channelPrefixIndex >= 0 && currentChannel) {
128+
const unprefixedChannel = currentChannel.slice(channelPrefixIndex + 1);
129+
if (unprefixedChannel) {
130+
tokens.add(unprefixedChannel);
131+
for (const segment of unprefixedChannel.split(/[;,]/u)) {
132+
const token = normalizeRouteToken(segment);
133+
if (token) {
134+
tokens.add(token);
135+
}
136+
}
137+
}
138+
}
139+
if (currentProvider && currentChannel?.startsWith(`${currentProvider}:`)) {
140+
const unprefixedChannel = currentChannel.slice(currentProvider.length + 1);
141+
if (unprefixedChannel) {
142+
tokens.add(unprefixedChannel);
143+
}
144+
}
145+
return tokens;
146+
}
147+
148+
function routeTokenMatchesSource(
149+
token: string | undefined,
150+
hookContext: CodexDynamicToolHookContext | undefined,
151+
): boolean {
152+
const normalized = normalizeRouteToken(token);
153+
return normalized !== undefined && sourceRouteTokens(hookContext).has(normalized);
154+
}
155+
156+
function routeProviderMatchesSource(
157+
provider: string | undefined,
158+
hookContext: CodexDynamicToolHookContext | undefined,
159+
): boolean {
160+
const normalized = normalizeRouteToken(provider);
161+
if (!normalized) {
162+
return false;
163+
}
164+
const currentProvider = normalizeRouteToken(hookContext?.currentChannelProvider);
165+
const currentChannel = normalizeRouteToken(hookContext?.currentChannelId);
166+
return currentProvider === normalized || currentChannel?.startsWith(`${normalized}:`) === true;
167+
}
168+
169+
function routeTokenMatchesCurrentMessage(
170+
token: string | number | undefined,
171+
hookContext: CodexDynamicToolHookContext | undefined,
172+
): boolean {
173+
const normalized = normalizeRouteToken(token);
174+
return (
175+
normalized !== undefined && normalized === normalizeRouteToken(hookContext?.currentMessageId)
176+
);
177+
}
178+
179+
function readRouteToken(record: Record<string, unknown>, key: string): string | number | undefined {
180+
const value = record[key];
181+
return typeof value === "string" || typeof value === "number" ? value : undefined;
182+
}
183+
184+
function explicitRouteTokensMismatchCurrent(
185+
args: Record<string, unknown>,
186+
keys: readonly string[],
187+
currentToken: string | number | undefined,
188+
): boolean {
189+
const normalizedCurrent = normalizeRouteToken(currentToken);
190+
if (!normalizedCurrent) {
191+
return false;
192+
}
193+
return keys.some((key) => {
194+
const normalized = normalizeRouteToken(readRouteToken(args, key));
195+
return normalized !== undefined && normalized !== normalizedCurrent;
196+
});
197+
}
198+
199+
function explicitThreadRouteTargetsNonSource(
200+
args: Record<string, unknown>,
201+
hookContext: CodexDynamicToolHookContext | undefined,
202+
messagingTarget: MessagingToolSend | undefined,
203+
): boolean {
204+
const normalizedCurrentThread = normalizeRouteToken(hookContext?.currentThreadId);
205+
const explicitThreadTokens = [
206+
...EXPLICIT_MESSAGE_THREAD_KEYS.map((key) => normalizeRouteToken(readRouteToken(args, key))),
207+
normalizeRouteToken(messagingTarget?.threadId),
208+
].filter((value): value is string => value !== undefined);
209+
210+
if (explicitThreadTokens.length === 0) {
211+
return false;
212+
}
213+
return (
214+
normalizedCurrentThread === undefined ||
215+
explicitThreadTokens.some((value) => value !== normalizedCurrentThread)
216+
);
217+
}
218+
219+
function replyReceiptMatchesCurrentMessage(
220+
value: unknown,
221+
hookContext: CodexDynamicToolHookContext | undefined,
222+
depth = 0,
223+
): boolean {
224+
if (depth > 4 || value === null) {
225+
return false;
226+
}
227+
if (typeof value === "string") {
228+
const trimmed = value.trim();
229+
if (!trimmed || !["{", "["].includes(trimmed[0] ?? "")) {
230+
return false;
231+
}
232+
try {
233+
return replyReceiptMatchesCurrentMessage(JSON.parse(trimmed), hookContext, depth + 1);
234+
} catch {
235+
return false;
236+
}
237+
}
238+
if (typeof value !== "object") {
239+
return false;
240+
}
241+
if (Array.isArray(value)) {
242+
return value.some((item) => replyReceiptMatchesCurrentMessage(item, hookContext, depth + 1));
243+
}
244+
const record = value as Record<string, unknown>;
245+
for (const key of ["repliedTo", "replyTo", "replyToId", "replyToIdFull"]) {
246+
if (
247+
routeTokenMatchesCurrentMessage(
248+
typeof record[key] === "string" ? record[key] : undefined,
249+
hookContext,
250+
)
251+
) {
252+
return true;
253+
}
254+
}
255+
for (const key of [
256+
"content",
257+
"details",
258+
"payload",
259+
"receipt",
260+
"result",
261+
"results",
262+
"sendResult",
263+
"text",
264+
]) {
265+
if (replyReceiptMatchesCurrentMessage(record[key], hookContext, depth + 1)) {
266+
return true;
267+
}
268+
}
269+
return false;
270+
}
271+
272+
function hasExplicitNonSourceMessageRoute(
273+
args: Record<string, unknown>,
274+
hookContext: CodexDynamicToolHookContext | undefined,
275+
messagingTarget: MessagingToolSend | undefined,
276+
): boolean {
277+
const currentProvider = normalizeRouteToken(hookContext?.currentChannelProvider);
278+
for (const key of EXPLICIT_MESSAGE_PROVIDER_KEYS) {
279+
const provider = normalizeRouteToken(typeof args[key] === "string" ? args[key] : undefined);
280+
if (
281+
provider &&
282+
currentProvider !== provider &&
283+
!routeProviderMatchesSource(provider, hookContext)
284+
) {
285+
return true;
286+
}
287+
}
288+
const targetValues = [
289+
...EXPLICIT_MESSAGE_TARGET_KEYS.map((key) =>
290+
typeof args[key] === "string" ? args[key] : undefined,
291+
),
292+
...(Array.isArray(args.targets)
293+
? args.targets.map((value) => (typeof value === "string" ? value : undefined))
294+
: []),
295+
].filter((value): value is string => normalizeRouteToken(value) !== undefined);
296+
if (explicitThreadRouteTargetsNonSource(args, hookContext, messagingTarget)) {
297+
return true;
298+
}
299+
if (
300+
explicitRouteTokensMismatchCurrent(
301+
args,
302+
EXPLICIT_MESSAGE_REPLY_KEYS,
303+
hookContext?.currentMessageId,
304+
)
305+
) {
306+
return true;
307+
}
308+
if (
309+
messagingTarget?.to !== undefined &&
310+
!routeTokenMatchesSource(messagingTarget.to, hookContext)
311+
) {
312+
return true;
313+
}
314+
if (messagingTarget?.to !== undefined) {
315+
return false;
316+
}
317+
if (targetValues.length === 0) {
318+
return false;
319+
}
320+
if (targetValues.some((value) => !routeTokenMatchesSource(value, hookContext))) {
321+
return true;
322+
}
323+
return false;
324+
}
325+
103326
/** Runtime bridge returned to Codex app-server attempt code. */
104327
export type CodexDynamicToolBridge = {
105328
availableSpecs: CodexDynamicToolSpec[];
@@ -114,6 +337,7 @@ export type CodexDynamicToolBridge = {
114337
) => Promise<CodexDynamicToolCallResponse>;
115338
telemetry: {
116339
didSendViaMessagingTool: boolean;
340+
didDeliverSourceReplyViaMessageTool: boolean;
117341
messagingToolSentTexts: string[];
118342
messagingToolSentMediaUrls: string[];
119343
messagingToolSentTargets: MessagingToolSend[];
@@ -132,6 +356,10 @@ export const CODEX_OPENCLAW_DYNAMIC_TOOL_NAMESPACE = "openclaw";
132356
// Keep OpenClaw session spawning searchable in Codex mode so Codex's native
133357
// spawn_agent remains the primary Codex subagent surface.
134358
const ALWAYS_DIRECT_DYNAMIC_TOOL_NAMES = new Set(["sessions_yield"]);
359+
const EXPLICIT_MESSAGE_PROVIDER_KEYS = ["channel", "provider"];
360+
const EXPLICIT_MESSAGE_TARGET_KEYS = ["target", "to", "channelId"];
361+
const EXPLICIT_MESSAGE_THREAD_KEYS = ["threadId", "thread_id", "messageThreadId", "topicId"];
362+
const EXPLICIT_MESSAGE_REPLY_KEYS = ["replyTo", "replyToId", "replyToIdFull"];
135363
const DEFAULT_CODEX_DYNAMIC_TOOL_RESULT_MAX_CHARS = 16_000;
136364

137365
/**
@@ -176,6 +404,7 @@ export function createCodexDynamicToolBridge(params: {
176404
emitQuarantinedDynamicToolDiagnostics(quarantinedTools, params.hookContext);
177405
const telemetry: CodexDynamicToolBridge["telemetry"] = {
178406
didSendViaMessagingTool: false,
407+
didDeliverSourceReplyViaMessageTool: false,
179408
messagingToolSentTexts: [],
180409
messagingToolSentMediaUrls: [],
181410
messagingToolSentTargets: [],
@@ -333,10 +562,9 @@ export function createCodexDynamicToolBridge(params: {
333562
executedArgs,
334563
params.hookContext?.currentChannelProvider,
335564
);
336-
const messagingTarget =
337-
isMessagingTool(toolName) && isMessagingToolSendAction(toolName, executedArgs)
338-
? extractMessagingToolSend(toolName, messagingTelemetryArgs, messagingContext)
339-
: undefined;
565+
const messagingTarget = isMessagingTool(toolName)
566+
? extractMessagingToolSend(toolName, messagingTelemetryArgs, messagingContext)
567+
: undefined;
340568
const confirmedMessagingTarget =
341569
!rawIsError && messagingTarget
342570
? extractMessagingToolSendResult(messagingTarget, telemetryRawResult)
@@ -358,12 +586,53 @@ export function createCodexDynamicToolBridge(params: {
358586
},
359587
terminalType,
360588
);
589+
const blocksSourceReplyTermination = hasExplicitNonSourceMessageRoute(
590+
executedArgs,
591+
params.hookContext,
592+
confirmedMessagingTarget,
593+
);
594+
const deliveredSourceReply = isDeliveredMessageToolOnlySourceReplyResult({
595+
sourceReplyDeliveryMode: params.hookContext?.sourceReplyDeliveryMode,
596+
toolName,
597+
args: executedArgs,
598+
result,
599+
hookResult: rawResult,
600+
isError: resultIsError,
601+
allowExplicitSourceRoute: !blocksSourceReplyTermination,
602+
});
603+
const receiptConfirmedSourceReply =
604+
params.hookContext?.sourceReplyDeliveryMode === "message_tool_only" &&
605+
toolName === "message" &&
606+
normalizeRouteToken(
607+
typeof executedArgs.action === "string" ? executedArgs.action : undefined,
608+
) === "reply" &&
609+
!resultIsError &&
610+
!blocksSourceReplyTermination &&
611+
isDeliveredMessagingToolResult({
612+
toolName,
613+
args: executedArgs,
614+
result,
615+
hookResult: rawResult,
616+
isError: resultIsError,
617+
}) &&
618+
(replyReceiptMatchesCurrentMessage(rawResult, params.hookContext) ||
619+
replyReceiptMatchesCurrentMessage(result, params.hookContext));
620+
const toolConfirmedSourceReply =
621+
params.hookContext?.sourceReplyDeliveryMode === "message_tool_only" &&
622+
toolName === "message" &&
623+
!resultIsError &&
624+
(rawResult.terminate === true || result.terminate === true);
625+
if (deliveredSourceReply || receiptConfirmedSourceReply || toolConfirmedSourceReply) {
626+
telemetry.didDeliverSourceReplyViaMessageTool = true;
627+
}
361628
withDynamicToolTermination(
362629
response,
363630
rawResult.terminate === true ||
364631
result.terminate === true ||
365632
isToolResultYield(rawResult) ||
366-
isToolResultYield(result),
633+
isToolResultYield(result) ||
634+
deliveredSourceReply ||
635+
receiptConfirmedSourceReply,
367636
);
368637
const asyncStarted =
369638
isAsyncStartedToolResult(rawResult) || isAsyncStartedToolResult(result);
@@ -801,9 +1070,22 @@ function collectToolTelemetry(params: {
8011070
}
8021071
}
8031072
}
1073+
if (!isMessagingTool(params.toolName)) {
1074+
return;
1075+
}
1076+
const isMessagingSendAction = isMessagingToolSendAction(params.toolName, params.args);
1077+
if (!isMessagingSendAction && !params.messagingTarget) {
1078+
return;
1079+
}
8041080
if (
805-
!isMessagingTool(params.toolName) ||
806-
!isMessagingToolSendAction(params.toolName, params.args)
1081+
!isMessagingSendAction &&
1082+
!isDeliveredMessagingToolResult({
1083+
toolName: params.toolName,
1084+
args: params.args,
1085+
result: params.result,
1086+
hookResult: params.mediaTrustResult,
1087+
isError: params.isError,
1088+
})
8071089
) {
8081090
return;
8091091
}

‎extensions/codex/src/app-server/event-projector.test.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -836,6 +836,19 @@ describe("CodexAppServerEventProjector", () => {
836836
expect(result.toolMediaUrls).toStrictEqual([]);
837837
});
838838

839+
it("propagates message-tool-only source reply delivery telemetry", async () => {
840+
const projector = await createProjector();
841+
842+
const result = projector.buildResult({
843+
...buildEmptyToolTelemetry(),
844+
didSendViaMessagingTool: true,
845+
didDeliverSourceReplyViaMessageTool: true,
846+
});
847+
848+
expect(result.didSendViaMessagingTool).toBe(true);
849+
expect(result.didDeliverSourceReplyViaMessageTool).toBe(true);
850+
});
851+
839852
it("does not promote repeated tool progress text to the final assistant reply", async () => {
840853
const onToolResult = vi.fn();
841854
const projector = await createProjector({

0 commit comments

Comments
 (0)