From 12fdc9e2380b9da4cf5b667da9ec975733b93d3a Mon Sep 17 00:00:00 2001 From: Lyon <88232613+pikasTech@users.noreply.github.com> Date: Sat, 23 May 2026 22:04:35 +0800 Subject: [PATCH] fix: structure codex steer confirmation Fixes #144. --- scripts/code-queue-cli-steer-test.ts | 47 +++++++++++++++- scripts/src/code-queue.ts | 80 ++++++++++++++++++++++++++-- 2 files changed, 120 insertions(+), 7 deletions(-) diff --git a/scripts/code-queue-cli-steer-test.ts b/scripts/code-queue-cli-steer-test.ts index cfa0fe8c..0ef0b4b3 100644 --- a/scripts/code-queue-cli-steer-test.ts +++ b/scripts/code-queue-cli-steer-test.ts @@ -113,6 +113,13 @@ export function runCodeQueueCliSteerContract(): JsonRecord { const usage = stringArray(nestedRecord(help.json?.data, []).usage); assertCondition(usage.some((line) => line.includes("codex steer ")), "codex help should list steer", { usage }); + const advertisedConfirm = runCli(["codex", "steer-confirm", "codex_test_task", "--steer-id", "steer_direct_12345"]); + assertCondition(advertisedConfirm.json !== null, "advertised steer-confirm command should return JSON", { stdout: advertisedConfirm.stdout, stderr: advertisedConfirm.stderr }); + assertCondition(!("error" in (advertisedConfirm.json ?? {})), "advertised steer-confirm command must not fall through to top-level error", advertisedConfirm.json ?? {}); + const advertisedDeliveryStatus = String(nestedRecord(advertisedConfirm.json?.data, ["delivery"]).status || ""); + assertCondition(["confirmed", "pending", "unknown", "not-supported"].includes(advertisedDeliveryStatus), "advertised steer-confirm command should return structured delivery status", { advertisedDeliveryStatus, json: advertisedConfirm.json }); + assertCondition(!String(JSON.stringify(advertisedConfirm.json)).includes("\"message\":\"not found\""), "advertised steer-confirm command must not return top-level not found", advertisedConfirm.json ?? {}); + let dryRunFetchCount = 0; const dryRunDirect = codexSteerTaskForTest("direct_task", ["do not send", "--dry-run"], () => { dryRunFetchCount += 1; @@ -375,10 +382,45 @@ export function runCodeQueueCliSteerContract(): JsonRecord { }; }) as JsonRecord; assertCondition(confirmLookup.ok === true, "trace confirmation lookup should succeed when accepted", confirmLookup); - assertCondition(nestedRecord(confirmLookup, ["delivery"]).status === "accepted", "trace confirmation output should expose accepted status", confirmLookup); + assertCondition(nestedRecord(confirmLookup, ["delivery"]).status === "confirmed", "trace confirmation output should expose confirmed status", confirmLookup); + assertCondition(nestedRecord(confirmLookup, ["traceConfirmation"]).status === "confirmed", "trace confirmation payload should expose confirmed status", confirmLookup); assertCondition(nestedRecord(confirmLookup, ["traceConfirmation", "trace"]).seq === 88, "trace confirmation output should expose bounded trace seq", confirmLookup); assertCondition(!JSON.stringify(confirmLookup).includes("same prompt"), "trace confirmation lookup must not echo prompt", confirmLookup); + const pendingLookup = codexSteerTraceConfirmForTest("direct_task", ["--steer-id", explicitSteerId], () => ({ + ok: true, + status: 200, + body: { + ok: true, + confirmation: { + taskId: "direct_task", + steerId: explicitSteerId, + found: false, + accepted: false, + deliveryState: "unknown", + matchCount: 0, + trace: null, + duplicateSuppressionKey: explicitSteerId, + promptOmitted: true, + }, + }, + })) as JsonRecord; + assertCondition(pendingLookup.ok === false, "pending trace confirmation should not report ok=true", pendingLookup); + assertCondition(nestedRecord(pendingLookup, ["delivery"]).status === "pending", "unmatched supported trace confirmation should be pending", pendingLookup); + + const unsupportedLookup = codexSteerTraceConfirmForTest("direct_task", ["--steer-id", explicitSteerId], (path) => { + assertCondition(path.includes("/api/microservices/code-queue/proxy/api/tasks/direct_task/steer-confirmation"), "unsupported lookup should still call advertised route", { path }); + return { + ok: false, + status: 404, + body: { ok: false, error: "not found", path: "/api/tasks/direct_task/steer-confirmation" }, + }; + }) as JsonRecord; + assertCondition(unsupportedLookup.ok === false, "unsupported trace confirmation should be structured ok=false", unsupportedLookup); + assertCondition(nestedRecord(unsupportedLookup, ["delivery"]).status === "not-supported", "unsupported trace confirmation should expose not-supported status", unsupportedLookup); + assertCondition(nestedRecord(unsupportedLookup, ["traceConfirmation"]).status === "not-supported", "unsupported trace confirmation payload should expose not-supported status", unsupportedLookup); + assertCondition(!JSON.stringify(unsupportedLookup).includes("promptPreview"), "unsupported trace confirmation must not echo prompt previews", unsupportedLookup); + const terminalPrompt = `${"do not leak ".repeat(40)}tail-secret-marker`; const terminalRejection = codexSteerTaskForTest("completed_task", [terminalPrompt], () => ({ ok: false, @@ -465,6 +507,7 @@ export function runCodeQueueCliSteerContract(): JsonRecord { "duplicate prompt source failure", "unsupported option failure", "codex help lists steer", + "advertised steer-confirm CLI command returns structured status", "outer command redacts positional steer prompt", "dry-run does not call stable proxy helper", "dry-run prompt preview is bounded", @@ -473,7 +516,7 @@ export function runCodeQueueCliSteerContract(): JsonRecord { "steer failure classification is JSON-consumable", "retryable tunnel aborts are retried with bounded diagnostics", "retry reuses steerId and trace confirmation distinguishes accepted_response_timeout from unknown", - "duplicate suppression and trace confirmation lookup expose bounded output shape", + "duplicate suppression and trace confirmation lookup expose bounded confirmed/pending/not-supported statuses", "terminal steer rejection is compact and actionable", "terminal steer rejection full/raw disclosure is explicit", ], diff --git a/scripts/src/code-queue.ts b/scripts/src/code-queue.ts index 571543b1..51a2f6b9 100644 --- a/scripts/src/code-queue.ts +++ b/scripts/src/code-queue.ts @@ -261,6 +261,7 @@ type CodexSteerFailureReason = | "invalid-proxy-response"; type CodexSteerAcceptanceStatus = "accepted" | "not_accepted" | "accepted_response_timeout" | "unknown"; +type CodexSteerConfirmStatus = "confirmed" | "pending" | "unknown" | "not-supported"; type CodexResumeAcceptanceStatus = "queued" | "duplicate_suppressed" | "not_accepted" | "accepted_response_timeout" | "unknown"; type CodexResumeDeliveryState = ResumeDeliveryState; @@ -927,6 +928,64 @@ function safeFetchSteerTraceConfirmation(taskId: string, steerId: string, fetche } } +function steerConfirmNotSupported(response: Record | null, targetPath: string): boolean { + const status = responseStatus(response); + if (status !== 404 && status !== 405) return false; + const body = responseBody(response); + const bodyPath = asString(body?.path); + const bodyError = asString(body?.error).toLowerCase(); + return bodyPath === targetPath + || bodyError === "not found" + || bodyError.includes("method not allowed"); +} + +function compactSteerConfirmUnsupported(taskId: string, steerId: string, response: Record | null, targetPath: string, options: CodexSteerConfirmOptions): Record { + const rawStatus = responseStatus(response); + const status: CodexSteerConfirmStatus = steerConfirmNotSupported(response, targetPath) ? "not-supported" : "unknown"; + return { + ok: false, + traceConfirmation: { + taskId, + steerId, + found: false, + accepted: false, + status, + deliveryState: status, + matchCount: 0, + trace: null, + promptOmitted: true, + }, + delivery: { + steerId, + status, + deliveryState: status, + promptOmitted: true, + supported: status === "not-supported" ? false : null, + }, + diagnostics: { + reason: status === "not-supported" ? "steer-confirmation-endpoint-not-supported" : "steer-confirmation-lookup-failed", + status: rawStatus, + message: responseErrorMessage(response), + promptOmitted: true, + }, + commands: { + task: `bun scripts/cli.ts codex task ${taskId}`, + trace: `bun scripts/cli.ts codex task ${taskId} --trace --tail --limit ${defaultTraceLimit}`, + retrySameSteerId: `bun scripts/cli.ts codex steer ${taskId} --prompt-file --steer-id ${steerId}`, + rawLookup: rawSteerConfirmationCommand(taskId, steerId), + }, + ...(options.raw ? { raw: response } : {}), + }; +} + +function steerConfirmStatus(confirmation: Record): CodexSteerConfirmStatus { + if (asBoolean(confirmation.accepted) || asBoolean(confirmation.found)) return "confirmed"; + const deliveryState = asString(confirmation.deliveryState); + if (deliveryState === "not-supported") return "not-supported"; + if (deliveryState === "unknown" || deliveryState.length === 0) return "pending"; + return "unknown"; +} + function unwrapSteerResponse(response: unknown, targetPath: string, stableProxyPath: string, rawProxyEquivalent: string): { ok: true; upstream: { ok: unknown; status: unknown }; body: Record } | { ok: false; diagnostics: ClassifiedCodexSteerError; rawResponse: unknown } { const record = asRecord(response); const body = responseBody(record); @@ -7590,17 +7649,28 @@ function codexSteerTask(taskId: string, args: string[], fetcher: CodexResponseFe function codexSteerTraceConfirm(taskId: string, args: string[], fetcher: CodexResponseFetcher = coreInternalFetch): unknown { const options = parseSteerConfirmOptions(args); const path = steerConfirmationPath(taskId, options.steerId); - const response = unwrapCodexResponse(fetcher(codeQueueProxyPath(path))); + const targetPath = `/api/tasks/${encodeURIComponent(taskId)}/steer-confirmation`; + const rawResponse = fetcher(codeQueueProxyPath(path)); + const record = asRecord(rawResponse); + const body = responseBody(record); + if (record?.ok !== true || body?.ok !== true) return compactSteerConfirmUnsupported(taskId, options.steerId, record, targetPath, options); + const response = { upstream: { ok: record.ok, status: record.status }, body }; const confirmation = compactSteerTraceConfirmation(response.body, taskId, options.steerId); + const status = steerConfirmStatus(confirmation); return { - ok: asBoolean(confirmation.accepted), + ok: status === "confirmed", upstream: response.upstream, - traceConfirmation: confirmation, + traceConfirmation: { + ...confirmation, + status, + deliveryState: status, + }, delivery: { steerId: options.steerId, - status: asBoolean(confirmation.accepted) ? "accepted" : "unknown", - deliveryState: confirmation.deliveryState, + status, + deliveryState: status, promptOmitted: true, + supported: true, }, commands: { task: `bun scripts/cli.ts codex task ${taskId}`,