fix: address Codex harness review issues

This commit is contained in:
Peter Steinberger
2026-04-10 15:48:28 +01:00
parent bfc0889776
commit b79f9f965e
6 changed files with 79 additions and 3 deletions

View File

@@ -138,4 +138,41 @@ describe("createCodexDynamicToolBridge", () => {
],
});
});
it("does not record messaging side effects when the send fails", async () => {
const tool = createTool({
name: "message",
execute: vi.fn(async () => {
throw new Error("send failed");
}),
});
const bridge = createCodexDynamicToolBridge({
tools: [tool],
signal: new AbortController().signal,
});
const result = await bridge.handleToolCall({
threadId: "thread-1",
turnId: "turn-1",
callId: "call-1",
tool: "message",
arguments: {
action: "send",
text: "not delivered",
provider: "slack",
to: "C123",
},
});
expect(result).toEqual({
success: false,
contentItems: [{ type: "inputText", text: "send failed" }],
});
expect(bridge.telemetry).toMatchObject({
didSendViaMessagingTool: false,
messagingToolSentTexts: [],
messagingToolSentMediaUrls: [],
messagingToolSentTargets: [],
});
});
});

View File

@@ -103,6 +103,9 @@ function collectToolTelemetry(params: {
telemetry: CodexDynamicToolBridge["telemetry"];
isError: boolean;
}): void {
if (params.isError) {
return;
}
if (!params.isError && params.toolName === "cron" && isCronAddAction(params.args)) {
params.telemetry.successfulCronAdds = (params.telemetry.successfulCronAdds ?? 0) + 1;
}

View File

@@ -104,6 +104,23 @@ describe("CodexAppServerEventProjector", () => {
expect(result.assistantTexts).toEqual([]);
});
it("preserves sessions_yield detection in attempt results", () => {
const params = createParams();
const projector = new CodexAppServerEventProjector(params, "thread-1", "turn-1");
const result = projector.buildResult(
{
didSendViaMessagingTool: false,
messagingToolSentTexts: [],
messagingToolSentMediaUrls: [],
messagingToolSentTargets: [],
},
{ yieldDetected: true },
);
expect(result.yieldDetected).toBe(true);
});
it("projects reasoning end, plan updates, compaction state, and tool metadata", async () => {
const onReasoningStream = vi.fn();
const onReasoningEnd = vi.fn();

View File

@@ -116,7 +116,10 @@ export class CodexAppServerEventProjector {
}
}
buildResult(toolTelemetry: CodexAppServerToolTelemetry): EmbeddedRunAttemptResult {
buildResult(
toolTelemetry: CodexAppServerToolTelemetry,
options?: { yieldDetected?: boolean },
): EmbeddedRunAttemptResult {
const assistantTexts = this.collectAssistantTexts();
const lastAssistant =
assistantTexts.length > 0
@@ -172,7 +175,7 @@ export class CodexAppServerEventProjector {
? { compactionCount: this.completedCompactionCount }
: {}),
},
yieldDetected: false,
yieldDetected: options?.yieldDetected || false,
didSendDeterministicApprovalPrompt: this.guardianReviewCount > 0 ? false : undefined,
};
}

View File

@@ -81,6 +81,7 @@ export async function runCodexAppServerAttempt(
config: params.config,
agentId: params.agentId,
});
let yieldDetected = false;
const tools = await buildDynamicTools({
params,
resolvedWorkspace,
@@ -89,6 +90,9 @@ export async function runCodexAppServerAttempt(
sandbox,
runAbortController,
sessionAgentId,
onYieldDetected: () => {
yieldDetected = true;
},
});
const toolBridge = createCodexDynamicToolBridge({
tools,
@@ -216,7 +220,7 @@ export async function runCodexAppServerAttempt(
try {
await completion;
const result = activeProjector.buildResult(toolBridge.telemetry);
const result = activeProjector.buildResult(toolBridge.telemetry, { yieldDetected });
await mirrorTranscriptBestEffort({
params,
result,
@@ -248,6 +252,7 @@ type DynamicToolBuildParams = {
sandbox: Awaited<ReturnType<typeof resolveSandboxContext>>;
runAbortController: AbortController;
sessionAgentId: string | undefined;
onYieldDetected: () => void;
};
async function buildDynamicTools(input: DynamicToolBuildParams) {
@@ -306,6 +311,7 @@ async function buildDynamicTools(input: DynamicToolBuildParams) {
params.requireExplicitMessageTarget ?? isSubagentSessionKey(params.sessionKey),
disableMessageTool: params.disableMessageTool,
onYield: (message) => {
input.onYieldDetected();
params.onAgentEvent?.({
stream: "codex_app_server.tool",
data: { name: "sessions_yield", message },

10
pnpm-lock.yaml generated
View File

@@ -418,6 +418,16 @@ importers:
specifier: workspace:*
version: link:../../packages/plugin-sdk
extensions/codex:
dependencies:
'@mariozechner/pi-coding-agent':
specifier: 0.65.2
version: 0.65.2(@modelcontextprotocol/sdk@1.29.0(zod@4.3.6))(ws@8.20.0)(zod@4.3.6)
devDependencies:
'@openclaw/plugin-sdk':
specifier: workspace:*
version: link:../../packages/plugin-sdk
extensions/comfy:
devDependencies:
'@openclaw/plugin-sdk':