From 318d8fdf53b97e24b03ddd9c48fc89a776a73efc Mon Sep 17 00:00:00 2001 From: t Date: Sun, 6 Sep 2026 02:56:30 +0900 Subject: [PATCH 1/3] fix(responses): repair native exec patches before tool completion --- .../030_responses_patch.md | 4 +++ .../040_native_tool_parity.md | 12 +++++++ .../content/docs/guides/codex-integration.md | 6 ++++ src/responses/custom-tool-compat.ts | 10 ++++++ src/server/responses-custom-tool-repair.ts | 36 ++++++++++++++----- structure/11_compatibility-contracts.md | 8 +++++ 6 files changed, 67 insertions(+), 9 deletions(-) create mode 100644 devlog/_plan/260906_grok_catalog_and_patch/040_native_tool_parity.md diff --git a/devlog/_plan/260906_grok_catalog_and_patch/030_responses_patch.md b/devlog/_plan/260906_grok_catalog_and_patch/030_responses_patch.md index 84743c0fa9..264cf9c105 100644 --- a/devlog/_plan/260906_grok_catalog_and_patch/030_responses_patch.md +++ b/devlog/_plan/260906_grok_catalog_and_patch/030_responses_patch.md @@ -11,3 +11,7 @@ Verifier: pure standalone synthetic SSE-block imports, compare outputs at each l ## Audit amendment Executable repair is limited to authorized code-mode exec and recognized helper aliases; unrelated same-name native custom tools keep raw input byte-for-byte. Explicit negative: render_diagram input JSON string {"input":"literal"} is not unwrapped. Separate scenarios cover missing input.done, terminal-only completion, failed/incomplete after held deltas, and disposal. Authoritative completion wins over previews. Failure never synthesizes successful completion. All retained buffers release. One simulated execution means choose the client-consumed completed item once, not execute every redundant lifecycle representation. + +## P revalidation + +Consume 020 ff388977a with isolated route/writer proof and remote75+19 tests. 030 remains scoped to the two patch lifecycle gaps. Append040 for independently confirmed ordinary function and dotted-namespace parity gaps; all terminal CI/merge obligations move there unchanged. Main owns production030; a disjoint worker may add tests only in tests/responses/responses-custom-tool-repair.test.ts. Native code-mode exec previews may be held until final when they could be complete raw/wrapped envelopes; unrelated native custom JSON bodies remain raw. diff --git a/devlog/_plan/260906_grok_catalog_and_patch/040_native_tool_parity.md b/devlog/_plan/260906_grok_catalog_and_patch/040_native_tool_parity.md new file mode 100644 index 0000000000..c19a1e2233 --- /dev/null +++ b/devlog/_plan/260906_grok_catalog_and_patch/040_native_tool_parity.md @@ -0,0 +1,12 @@ +# 040 Native function and namespace parity + +Depends on 030 custom-call restoration. Class C3 spec-satisfaction repair. User requested all Chat-era tool repairs be checked. Independent source/probe inventory finds native ordinary calls retain integer-as-float and numeric-as-string mismatches, completed empty arguments, and dotted namespace names that bridge already repairs. These are in scope; assistant output filtering is not. + +NEW src/responses/function-call-compat.ts: collect original current-turn ordinary function declarations using collectResponsesToolGroups, preserving namespace/kind and original parameter schema. Lookup exact declared identity, including reserved functions children as bare and authorized canonical namespace aliases. Do not consume historical-only declarations or provider-normalized schemas. Pure completed-item transform calls coerceIntegerToolArguments(raw||"{}", original.parameters, original.namespace ? undefined : original.name). Only explicit completed empty payload becomes {}; unknown/missing, malformed nonempty, fractions, numeric unions, unsafe integers, custom/hosted/helper calls remain unchanged. +NEW src/server/responses-function-tool-repair.ts: SseBlockRewrite tracks item identity by item_id/output_index and uses same pure completion transform at arguments.done, item.done and terminal snapshots. Preserve in-progress placeholders. Budget any buffered data and release on done/terminal/dispose. Original declaration is authority; attempt wire alias is transport spelling only. Final representations must agree and failed/incomplete never synthesize successful executable input. Revalidate whether delta holding is necessary against existing bridge closeCurrentToolCall (which already repairs authoritative final arguments after streamed numeric previews); use one compatible completion contract rather than introducing arbitrary JSON rewriting. +MODIFY src/server/responses/core.ts: derive ordinary schemas from currentTurnWireToolCatalogBody before lowering; compose native function repair after namespace/custom restores and before undeclared guard. Apply pure repair in JSON, SSE final snapshots, bounded JSON-to-SSE, and rememberPassthroughResponseChecked so client and replay state agree. Canonical forward auth remains byte-pass-through. Rebuild only attempt-specific aliases on retries. +MODIFY src/responses/namespace-tool-compat.ts and, if needed, responses-undeclared-tool-guard.ts: reuse existing collectAmbiguousDottedAliases ownership algorithm rather than duplicate. Add unambiguous dotted aliases after canonical authorization, collisions computed from whole original current-turn catalog including bare spellings before selection. Never reinterpret explicit conflicting namespaces or different kinds; canonical identities retain precedence. +MODIFY existing native Responses repair and namespace tests, or register new domain tests in both layout manifests: integer/string and no-arg scenarios at JSON/each SSE completion/replay, namespace wait exception boundaries, same-inner-name schemas, forbidden selectors/replay-only names, early/interleaved events, terminal/dispose cleanup, dotted collision order independence, unchanged030 code/patch semantics. +UPDATE structure/11_compatibility-contracts.md and guides/codex-integration.md with completion parity boundary and inventory. + +C: standalone synthetic imports with stub tools (no real execution) and remote focused tests/typecheck; all stack PR exact-head hosted CI must pass before merge. Keep original pi-filter/owned-refresh/responses-patch terminal criteria unchanged and satisfy them at final D with PR heads/CI/merge ancestry. Register native GitHub stack, merge approved prefix using async REST and SHA guard, wait for actual merged status, fetch dev and prove all merged SHAs ancestors. No release/deploy/local suites. Stop only verified DONE or actual external blocking evidence. Resource bounds inherited from000. diff --git a/docs-site/src/content/docs/guides/codex-integration.md b/docs-site/src/content/docs/guides/codex-integration.md index b2ae7fcb91..c015b71ebb 100644 --- a/docs-site/src/content/docs/guides/codex-integration.md +++ b/docs-site/src/content/docs/guides/codex-integration.md @@ -324,6 +324,12 @@ encodes that declaration and its history as an upstream function tool, then rest function-call lifecycle to `custom_tool_call` before Codex sees it. Native OpenAI forward routing and the supported `apply_patch` custom tool stay unchanged. +If a routed model sends a complete patch as the entire code-mode `exec` input, opencodex +converts it to the nested `tools.apply_patch` call before the tool-completion events reach +Codex. Native custom calls and converted function calls use the same completion rule; +patch previews are held while their executable form is unresolved. JavaScript that merely +contains patch text and unrelated native custom payloads stay unchanged. + The selected provider must support function/tool calling. A text-only provider without tool-call support cannot use `exec`, Browser, or Computer Use. Native OpenAI rows keep their upstream tool mode unchanged. diff --git a/src/responses/custom-tool-compat.ts b/src/responses/custom-tool-compat.ts index 4e4bcad2ab..ce8f9591d1 100644 --- a/src/responses/custom-tool-compat.ts +++ b/src/responses/custom-tool-compat.ts @@ -287,6 +287,16 @@ export function restoreRoutedCustomCalls( const helper = aliased && sourceInput !== "" ? item.name : resolveCodeModeHelperName(undefined, targetName, sourceInput, itemNamespace, declaredNames); + // Native custom input is already the tool's raw grammar. Only a recognized + // helper/envelope may reinterpret it; a JSON-looking native body is not a wrapper. + if (item.type === "custom_tool_call" && !aliased && !helper) { + const input = repairNames.has(wireName) && typeof sourceInput === "string" + ? normalizeApplyPatchDelimiters(sourceInput) + : sourceInput; + return input !== sourceInput + ? { value: { ...item, input }, changed: true } + : { value: item, changed: false }; + } const restored: Record = { ...item, type: "custom_tool_call", diff --git a/src/server/responses-custom-tool-repair.ts b/src/server/responses-custom-tool-repair.ts index 4177e03426..87bee011c5 100644 --- a/src/server/responses-custom-tool-repair.ts +++ b/src/server/responses-custom-tool-repair.ts @@ -93,7 +93,8 @@ export function createRoutedCustomToolRestoreBlockRewrite( declaredNames?: ReadonlySet, ): SseBlockRewrite { const itemNames = new Map(); - const customAliasItemNames = new Map(); + // Native helper aliases and genuine bare code-mode exec calls share completion repair. + const customExecItemNames = new Map(); const repairItemNames = new Map(); const ordinaryItemIds = new Set(); const openCalls = new Map(); @@ -119,7 +120,7 @@ export function createRoutedCustomToolRestoreBlockRewrite( } pendingArguments = []; itemNames.clear(); - customAliasItemNames.clear(); + customExecItemNames.clear(); repairItemNames.clear(); ordinaryItemIds.clear(); }; @@ -195,15 +196,17 @@ export function createRoutedCustomToolRestoreBlockRewrite( const wireName = routedCustomToolWireName(parsed.item); const targetName = routedCustomToolTargetName(parsed.item, names, declaredNames); const aliased = targetName !== undefined && targetName !== wireName; - if (upstreamItemId && aliased) { - customAliasItemNames.set(upstreamItemId, parsed.item.name); + const codeModeExec = targetName === "exec" && parsed.item.name === "exec" + && parsed.item.namespace === undefined && declaresCodeModeExec(declaredNames); + if (upstreamItemId && (aliased || codeModeExec)) { + customExecItemNames.set(upstreamItemId, parsed.item.name); if (type === "response.output_item.added") { openCalls.set(upstreamItemId, { argumentsText: "", emittedInput: "", retainedBytes: 0 }); } } const repairable = wireName !== undefined && repairNames.has(wireName); if (upstreamItemId && repairable) repairItemNames.set(upstreamItemId, parsed.item.name); - const restored = repairable || aliased + const restored = repairable || aliased || codeModeExec ? restoreRoutedCustomCalls(parsed, names, repairNames, declaredNames) : { value: parsed, changed: false }; if (type === "response.output_item.done" && upstreamItemId) releaseCall(upstreamItemId); @@ -259,7 +262,7 @@ export function createRoutedCustomToolRestoreBlockRewrite( if ( type === "response.custom_tool_call_input.delta" && upstreamItemId - && customAliasItemNames.has(upstreamItemId) + && customExecItemNames.has(upstreamItemId) ) { const open = openCalls.get(upstreamItemId) ?? { argumentsText: "", emittedInput: "", retainedBytes: 0 }; const delta = typeof parsed.delta === "string" ? parsed.delta : ""; @@ -268,19 +271,32 @@ export function createRoutedCustomToolRestoreBlockRewrite( open.argumentsText += delta; open.retainedBytes += deltaBytes; openCalls.set(upstreamItemId, open); - return []; + if (customExecItemNames.get(upstreamItemId) !== "exec" + || mayBecomePatchEnvelope(open.argumentsText) + || FREEFORM_WRAP_PREFIX.startsWith(open.argumentsText.trimStart()) + || FREEFORM_WRAP_PREFIX_RE.test(open.argumentsText)) return []; + // If a held prefix turns out to be ordinary JavaScript, release the entire + // un-emitted suffix. Native custom input remains byte-exact. + const inputDelta = open.argumentsText.slice(open.emittedInput.length); + open.emittedInput = open.argumentsText; + return inputDelta ? [replaceSseDataPayload(block, JSON.stringify({ ...parsed, delta: inputDelta }))] : []; } if ( type === "response.custom_tool_call_input.done" && upstreamItemId - && customAliasItemNames.has(upstreamItemId) + && customExecItemNames.has(upstreamItemId) ) { const source = typeof parsed.input === "string" ? parsed.input : openCalls.get(upstreamItemId)?.argumentsText ?? ""; + const name = customExecItemNames.get(upstreamItemId)!; + const helper = name === "exec" + ? resolveCodeModeHelperName(undefined, name, source, undefined, declaredNames) + : name; + releaseCall(upstreamItemId); return [replaceSseDataPayload(block, JSON.stringify({ ...parsed, - input: compileCodeModeHelperInput(source, customAliasItemNames.get(upstreamItemId)!), + input: helper ? compileCodeModeHelperInput(source, helper) : source, }))]; } if ( @@ -312,6 +328,8 @@ export function createRoutedCustomToolRestoreBlockRewrite( open.argumentsText += delta; open.retainedBytes += deltaBytes; openCalls.set(upstreamItemId, open); + // A helper alias will become JavaScript at completion, never raw patch/JSON. + if (itemNames.get(upstreamItemId)?.aliased) return []; // Still accumulating toward the compact wrapper, or an unrecognized shape: // suppress progressive emission and let the done event carry input. if (FREEFORM_WRAP_PREFIX.startsWith(open.argumentsText)) return []; diff --git a/structure/11_compatibility-contracts.md b/structure/11_compatibility-contracts.md index 27a446eb2a..295590788a 100644 --- a/structure/11_compatibility-contracts.md +++ b/structure/11_compatibility-contracts.md @@ -67,3 +67,11 @@ them independently. - 선택한 방식: Add a passive versioned schema and one exact `openai`/canonical Codex URL/forward/`gpt-5.6-sol` manifest whose claims reference assertion-level fixtures executed against the production adapter. - 다른 대안 대신 이 방식을 선택한 이유: Registry flags do not capture transformations such as local continuation expansion or orphan-output degradation. A broad first matrix would turn unverified assumptions into public promises. - 장점, 단점 및 영향: The first contract is small but trustworthy and can feed future CLI/GUI surfaces. Coverage expands only as fixtures are added; no request behavior changes in this slice. + +## Routed code-mode patch completion + +Native Responses custom exec and function helper aliases apply the same complete-envelope +resolver at input.done, output_item.done and terminal snapshots. Potential raw/wrapped patch +previews are withheld before compilation; ordinary native custom payloads retain their raw +grammar. A string merely containing patch markers remains executable caller input and is +never rewritten. Completion and disposal release retained preview buffers. From b477b731e7c16bf22911a794693274a4809d0947 Mon Sep 17 00:00:00 2001 From: t Date: Sun, 6 Sep 2026 02:57:21 +0900 Subject: [PATCH 2/3] test(responses): cover native patch completion and ambiguous wrappers --- .../030_responses_patch.md | 4 + src/server/responses-custom-tool-repair.ts | 6 +- .../responses-custom-tool-repair.test.ts | 209 ++++++++++++++++++ 3 files changed, 217 insertions(+), 2 deletions(-) diff --git a/devlog/_plan/260906_grok_catalog_and_patch/030_responses_patch.md b/devlog/_plan/260906_grok_catalog_and_patch/030_responses_patch.md index 264cf9c105..a7778f6dc0 100644 --- a/devlog/_plan/260906_grok_catalog_and_patch/030_responses_patch.md +++ b/devlog/_plan/260906_grok_catalog_and_patch/030_responses_patch.md @@ -15,3 +15,7 @@ Executable repair is limited to authorized code-mode exec and recognized helper ## P revalidation Consume 020 ff388977a with isolated route/writer proof and remote75+19 tests. 030 remains scoped to the two patch lifecycle gaps. Append040 for independently confirmed ordinary function and dotted-namespace parity gaps; all terminal CI/merge obligations move there unchanged. Main owns production030; a disjoint worker may add tests only in tests/responses/responses-custom-tool-repair.test.ts. Native code-mode exec previews may be held until final when they could be complete raw/wrapped envelopes; unrelated native custom JSON bodies remain raw. + +## Implementation audit synthesis + +A fragmented pretty JSON wrapper beginning with brace-newline escaped the compact-prefix guard, so preview bytes could contradict compiled completion. The completion parser accepts arbitrary whitespace, escaped property names and property order; native exec now conservatively holds all object-leading inputs to completion. Ordinary JavaScript stays byte-exact, though a block-leading program waits for completion. Added per-character pretty-wrapper and escaped-key regressions. diff --git a/src/server/responses-custom-tool-repair.ts b/src/server/responses-custom-tool-repair.ts index 87bee011c5..c9d8527ebc 100644 --- a/src/server/responses-custom-tool-repair.ts +++ b/src/server/responses-custom-tool-repair.ts @@ -273,8 +273,10 @@ export function createRoutedCustomToolRestoreBlockRewrite( openCalls.set(upstreamItemId, open); if (customExecItemNames.get(upstreamItemId) !== "exec" || mayBecomePatchEnvelope(open.argumentsText) - || FREEFORM_WRAP_PREFIX.startsWith(open.argumentsText.trimStart()) - || FREEFORM_WRAP_PREFIX_RE.test(open.argumentsText)) return []; + // JSON.parse accepts whitespace, escaped keys and arbitrary property order. + // Any object prefix may still wrap a patch; keep it until authoritative completion. + || open.argumentsText.trimStart() === "" + || open.argumentsText.trimStart().startsWith("{")) return []; // If a held prefix turns out to be ordinary JavaScript, release the entire // un-emitted suffix. Native custom input remains byte-exact. const inputDelta = open.argumentsText.slice(open.emittedInput.length); diff --git a/tests/responses/responses-custom-tool-repair.test.ts b/tests/responses/responses-custom-tool-repair.test.ts index ac5995d912..98336d0241 100644 --- a/tests/responses/responses-custom-tool-repair.test.ts +++ b/tests/responses/responses-custom-tool-repair.test.ts @@ -188,6 +188,215 @@ describe("routed Responses custom-tool compatibility", () => { rewrite.dispose?.(); }); + test.each([ + { label: "native raw exec", native: true, name: "exec", input: DECORATED_PATCH }, + { label: "native wrapped exec", native: true, name: "exec", input: WRAPPED_DECORATED_PATCH }, + { label: "native pretty wrapper", native: true, name: "exec", input: JSON.stringify({ input: DECORATED_PATCH }, null, 2) }, + { label: "native escaped-key wrapper", native: true, name: "exec", input: `{ "\\u0069nput": ${JSON.stringify(DECORATED_PATCH)} }` }, + { label: "function apply_patch wrapper alias", native: false, name: "apply_patch", input: WRAPPED_DECORATED_PATCH }, + ])("holds fragmented $label previews and completes with executable patch input", async ({ native, name, input }) => { + const budget = createTestTranslatorBudget(); + const rewrite = createRoutedCustomToolRestoreBlockRewrite( + new Set(["exec"]), budget, new Set(), new Set(["exec"]), + ); + const id = native ? "ctc_patch_lifecycle" : "fc_patch_lifecycle"; + const item = { type: native ? "custom_tool_call" : "function_call", id, call_id: "call_patch_lifecycle", name }; + const payloadKey = native ? "input" : "arguments"; + const eventPrefix = native ? "response.custom_tool_call_input" : "response.function_call_arguments"; + // Independent oracle: do not compute expected source with the production compiler. + const expected = `const result = await tools.apply_patch(${JSON.stringify(CANONICAL_PATCH)});\ntext(result);`; + try { + const added = rewrite(frame("response.output_item.added", { + output_index: 0, item: { ...item, [payloadKey]: "", status: "in_progress" }, + })); + expect(added).toHaveLength(1); + expect(dataPayload(added[0]!).item).toMatchObject({ + type: "custom_tool_call", id: "ctc_patch_lifecycle", call_id: item.call_id, name: "exec", input: "", + }); + // Split both the JSON wrapper and patch markers, including escaped newlines. + for (const delta of input) { + expect(rewrite(frame(`${eventPrefix}.delta`, { output_index: 0, item_id: id, delta }))).toEqual([]); + } + expect(budget.snapshot().currentBytes).toBeGreaterThan(0); + const inputDone = rewrite(frame(`${eventPrefix}.done`, { + output_index: 0, item_id: id, [payloadKey]: input, + })); + expect(inputDone).toHaveLength(1); + expect(dataPayload(inputDone[0]!)).toMatchObject({ + type: "response.custom_tool_call_input.done", item_id: "ctc_patch_lifecycle", input: expected, + }); + if (native) expect(budget.snapshot().currentBytes).toBe(0); + const completedItem = { ...item, [payloadKey]: input, status: "completed" }; + const itemDone = rewrite(frame("response.output_item.done", { output_index: 0, item: completedItem })); + expect(itemDone).toHaveLength(1); + expect(dataPayload(itemDone[0]!).item).toMatchObject({ + type: "custom_tool_call", id: "ctc_patch_lifecycle", call_id: item.call_id, name: "exec", input: expected, + }); + expect(dataPayload(itemDone[0]!).item).not.toHaveProperty("arguments"); + expect(budget.snapshot().currentBytes).toBe(0); + const terminal = rewrite(frame("response.completed", { + response: { id: "resp_patch_lifecycle", status: "completed", output: [completedItem] }, + })); + expect(terminal).toHaveLength(1); + const response = dataPayload(terminal[0]!).response as { output: Array> }; + expect(response.output).toHaveLength(1); + expect(response.output[0]).toMatchObject({ + type: "custom_tool_call", id: "ctc_patch_lifecycle", call_id: item.call_id, name: "exec", input: expected, + }); + expect(response.output[0]).not.toHaveProperty("arguments"); + // Execute the client-consumed terminal item once, not each redundant representation. + const calls: unknown[] = []; + const output: unknown[] = []; + const run = new Function("tools", "text", `return (async () => { ${response.output[0]!.input} })();`); + await run({ apply_patch: async (patch: unknown) => { calls.push(patch); return "patched"; } }, + (value: unknown) => output.push(value)); + expect(calls).toEqual([CANONICAL_PATCH]); + expect(output).toEqual(["patched"]); + expect(budget.snapshot().currentBytes).toBe(0); + } finally { + rewrite.dispose?.(); + } + }); + + test.each([ + { label: "raw item.done without input.done", input: DECORATED_PATCH, started: true, itemDone: true }, + { label: "wrapped item.done without input.done", input: WRAPPED_DECORATED_PATCH, started: true, itemDone: true }, + { label: "terminal after held deltas without either done event", input: WRAPPED_DECORATED_PATCH, started: true, itemDone: false }, + { label: "raw terminal-only", input: DECORATED_PATCH, started: false, itemDone: false }, + { label: "wrapped terminal-only", input: WRAPPED_DECORATED_PATCH, started: false, itemDone: false }, + ])("repairs native exec at $label completion", ({ input, started, itemDone }) => { + const budget = createTestTranslatorBudget(); + const rewrite = createRoutedCustomToolRestoreBlockRewrite( + new Set(["exec"]), budget, new Set(), new Set(["exec"]), + ); + const item = { type: "custom_tool_call", id: "ctc_missing_done", call_id: "call_missing_done", name: "exec" }; + const expected = `const result = await tools.apply_patch(${JSON.stringify(CANONICAL_PATCH)});\ntext(result);`; + try { + if (started) { + rewrite(frame("response.output_item.added", { + output_index: 0, item: { ...item, input: "", status: "in_progress" }, + })); + // The authoritative item must win even when only a prefix was previewed upstream. + expect(rewrite(frame("response.custom_tool_call_input.delta", { + output_index: 0, item_id: item.id, delta: input.slice(0, 12), + }))).toEqual([]); + expect(budget.snapshot().currentBytes).toBeGreaterThan(0); + } + if (itemDone) { + const done = rewrite(frame("response.output_item.done", { + output_index: 0, item: { ...item, input, status: "completed" }, + })); + expect(done).toHaveLength(1); + expect(dataPayload(done[0]!).item).toEqual({ ...item, input: expected, status: "completed" }); + expect(budget.snapshot().currentBytes).toBe(0); + } + const terminal = rewrite(frame("response.completed", { + response: { id: "resp_missing_done", status: "completed", output: [{ ...item, input, status: "completed" }] }, + })); + expect(terminal).toHaveLength(1); + expect(dataPayload(terminal[0]!).response).toMatchObject({ + status: "completed", output: [{ ...item, input: expected, status: "completed" }], + }); + expect(budget.snapshot().currentBytes).toBe(0); + } finally { + rewrite.dispose?.(); + } + }); + + test.each([ + { label: "arbitrary JavaScript mentioning a patch", name: "exec", input: `const patch = ${JSON.stringify(DECORATED_PATCH)};\ntext(patch);` }, + { label: "JavaScript block with an ambiguous brace prefix", name: "exec", input: '{ const value = "literal"; text(value); }' }, + { label: "unrelated custom JSON input", name: "render_diagram", input: '{"input":"literal"}' }, + { label: "incomplete patch envelope", name: "exec", input: "*** Begin Patch ***\n*** Add File: note.txt\n+unfinished" }, + { label: "envelope without an operation", name: "exec", input: "*** Begin Patch ***\nnot an operation\n*** End Patch ***" }, + { label: "flat exec catalog", name: "exec", input: DECORATED_PATCH, flat: true }, + { label: "foreign exec namespace", name: "exec", input: DECORATED_PATCH, namespace: "mcp" }, + { label: "foreign helper namespace", name: "apply_patch", input: DECORATED_PATCH, namespace: "mcp" }, + ])("preserves native $label across completion boundaries", ({ name, input, ...options }) => { + const namespace = "namespace" in options ? options.namespace : undefined; + const flat = "flat" in options && options.flat; + const names = new Set(["exec", "render_diagram", "mcp__exec", "mcp__apply_patch"]); + const rewrite = createRoutedCustomToolRestoreBlockRewrite( + names, undefined, new Set(), new Set([...names, ...(flat ? ["exec_command"] : [])]), + ); + const item = { + type: "custom_tool_call", id: "ctc_preserved", call_id: "call_preserved", name, + ...(namespace ? { namespace } : {}), + }; + try { + rewrite(frame("response.output_item.added", { + output_index: 0, item: { ...item, input: "", status: "in_progress" }, + })); + let preview = ""; + for (const delta of input) { + for (const block of rewrite(frame("response.custom_tool_call_input.delta", { + output_index: 0, item_id: item.id, delta, + }))) { + const payload = dataPayload(block); + expect(payload.type).toBe("response.custom_tool_call_input.delta"); + expect(typeof payload.delta).toBe("string"); + preview += payload.delta; + expect(input.startsWith(preview)).toBe(true); + } + } + // Ordinary JS and unrelated tools retain progressive input; ambiguous exec may be held. + if (input.startsWith("const ") || name === "render_diagram") expect(preview).toBe(input); + const inputDone = rewrite(frame("response.custom_tool_call_input.done", { + output_index: 0, item_id: item.id, input, + })); + expect(inputDone).toHaveLength(1); + expect(dataPayload(inputDone[0]!)).toMatchObject({ type: "response.custom_tool_call_input.done", input }); + const completedItem = { ...item, input, status: "completed" }; + const itemDone = rewrite(frame("response.output_item.done", { output_index: 0, item: completedItem })); + expect(itemDone).toHaveLength(1); + expect(dataPayload(itemDone[0]!).item).toEqual(completedItem); + const terminal = rewrite(frame("response.completed", { + response: { id: "resp_preserved", status: "completed", output: [completedItem] }, + })); + expect(terminal).toHaveLength(1); + expect(dataPayload(terminal[0]!).response).toEqual({ + id: "resp_preserved", status: "completed", output: [completedItem], + }); + } finally { + rewrite.dispose?.(); + } + }); + + test.each(["failed", "incomplete", "dispose"])("releases held native exec input on %s without synthesizing success", outcome => { + const budget = createTestTranslatorBudget(); + const rewrite = createRoutedCustomToolRestoreBlockRewrite( + new Set(["exec"]), budget, new Set(), new Set(["exec"]), + ); + try { + rewrite(frame("response.output_item.added", { + output_index: 0, + item: { type: "custom_tool_call", id: "ctc_cancelled", call_id: "call_cancelled", name: "exec", input: "", status: "in_progress" }, + })); + expect(rewrite(frame("response.custom_tool_call_input.delta", { + output_index: 0, item_id: "ctc_cancelled", delta: WRAPPED_DECORATED_PATCH, + }))).toEqual([]); + expect(budget.snapshot().currentBytes).toBeGreaterThan(0); + if (outcome === "dispose") { + expect(rewrite.dispose?.()).toBeUndefined(); + } else { + const terminal = frame(`response.${outcome}`, { + response: { id: "resp_cancelled", status: outcome, output: [] }, + }); + expect(rewrite(terminal)).toEqual([terminal]); + } + expect(budget.snapshot().currentBytes).toBe(0); + // Late provider bytes cannot reopen a cancelled collector or flush a successful item. + const lateDelta = frame("response.custom_tool_call_input.delta", { + output_index: 0, item_id: "ctc_cancelled", delta: "late", + }); + expect(rewrite(lateDelta)).toEqual([lateDelta]); + rewrite.dispose?.(); + expect(budget.snapshot().currentBytes).toBe(0); + } finally { + rewrite.dispose?.(); + } + }); + test("restores streamed exec_command arguments through unified exec", () => { const rewrite = createRoutedCustomToolRestoreBlockRewrite( new Set(["exec"]), From 4893d0d415955f78a522566a880ab74b224eee55 Mon Sep 17 00:00:00 2001 From: t Date: Sun, 6 Sep 2026 08:01:59 +0900 Subject: [PATCH 3/3] test(clients): bound asynchronous client state probes --- tests/clients/client-connect.test.ts | 94 +++++++++++++++++++++------- 1 file changed, 73 insertions(+), 21 deletions(-) diff --git a/tests/clients/client-connect.test.ts b/tests/clients/client-connect.test.ts index b1f8fc4d19..f6035136e0 100644 --- a/tests/clients/client-connect.test.ts +++ b/tests/clients/client-connect.test.ts @@ -1,6 +1,6 @@ import { describe, expect, spyOn, test } from "bun:test"; import { createHash } from "node:crypto"; -import { spawnSync } from "node:child_process"; +import { spawn, spawnSync } from "node:child_process"; import { existsSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -31,21 +31,73 @@ class ClientStateProbeError extends Error { } } -function readStateProbe(script: string, home: string, timeoutMs = INTERNAL_DEADLINE_MS) { - const child = spawnSync(process.execPath, ["--eval", script], { - cwd: repoRoot, - env: { ...process.env, OPENCODEX_HOME: home }, - encoding: "utf8", - timeout: timeoutMs, - killSignal: "SIGKILL", +async function readStateProbe(script: string, home: string, timeoutMs = INTERNAL_DEADLINE_MS) { + const maxCaptureBytes = 1024 * 1024; + const cleanupMs = 1_000; + const result = await new Promise<{ stdout: string; pid: number; status: number | null; signal: NodeJS.Signals | null }>((resolve, reject) => { + let child: ReturnType; + try { + child = spawn(process.execPath, ["--eval", script], { + cwd: repoRoot, + env: { ...process.env, OPENCODEX_HOME: home }, + stdio: ["ignore", "pipe", "pipe"], + }); + } catch { reject(new ClientStateProbeError(0, null, null, false)); return; } + const chunks: Buffer[] = []; + let bytes = 0; + let failed = false; + let timedOut = false; + let settled = false; + let status: number | null = null; + let signal: NodeJS.Signals | null = null; + let deadline: ReturnType | undefined; + let cleanup: ReturnType | undefined; + const finish = () => { + if (settled) return; + settled = true; + clearTimeout(deadline); + clearTimeout(cleanup); + child.stdout?.destroy(); + child.stderr?.destroy(); + child.unref(); + const pid = child.pid ?? 0; + if (failed || status !== 0 || signal !== null) reject(new ClientStateProbeError(pid, status, signal, timedOut)); + else resolve({ stdout: Buffer.concat(chunks).toString("utf8"), pid, status, signal }); + }; + const boundCleanup = () => { + if (settled) return; + cleanup ??= setTimeout(() => { failed = true; finish(); }, cleanupMs); + }; + const stop = () => { + if (settled || failed) return; + failed = true; + clearTimeout(deadline); + boundCleanup(); + try { child.kill("SIGKILL"); } catch { /* Preserve observed exit metadata, never the OS error text. */ } + }; + const capture = (chunk: Buffer, stdout: boolean) => { + if (settled || failed) return; + bytes += chunk.length; + if (bytes > maxCaptureBytes) { stop(); return; } + if (stdout) chunks.push(chunk); + }; + child.stdout?.on("data", chunk => capture(chunk, true)); + child.stderr?.on("data", chunk => capture(chunk, false)); + child.stdout?.on("error", stop); + child.stderr?.on("error", stop); + child.on("error", stop); + child.once("exit", (code, exitSignal) => { + status = code; signal = exitSignal; + // A descendant retaining a pipe must not turn successful exit into an unbounded wait. + boundCleanup(); + }); + child.once("close", (code, exitSignal) => { status = code; signal = exitSignal; finish(); }); + deadline = setTimeout(() => { timedOut = true; stop(); }, timeoutMs); }); - if (child.error || child.status !== 0 || child.signal !== null) { - throw new ClientStateProbeError( - child.pid, child.status, child.signal, - (child.error as NodeJS.ErrnoException | undefined)?.code === "ETIMEDOUT", - ); + try { return JSON.parse(result.stdout.trim().split("\n").at(-1) ?? "{}"); } + catch { + throw new ClientStateProbeError(result.pid, result.status, result.signal, false); } - return JSON.parse(child.stdout.trim().split("\n").at(-1) ?? "{}"); } function readyBody(protocol = 1, minimumClientProtocol = 1) { @@ -63,7 +115,7 @@ function readyBody(protocol = 1, minimumClientProtocol = 1) { } describe("remote hub client boundary", () => { - test("runtimeRole=hub without client state reads as disconnected so the hub can start", () => { + test("runtimeRole=hub without client state reads as disconnected so the hub can start", async () => { // First clisu-oracle dogfood boot: the hub role refused 'ocx start' because the // client-state reader classified role=hub (no client block) as mismatched. A hub // is a server; without client state it is simply not a connected client. @@ -74,16 +126,16 @@ describe("remote hub client boundary", () => { const home = mkdtempSync(join(tmpdir(), "ocx-hub-role-")); try { writeFileSync(join(home, "config.json"), JSON.stringify({ port: 10190, runtimeRole: "hub" })); - expect(readStateProbe(readScript, home).kind).toBe("disconnected"); + expect((await readStateProbe(readScript, home)).kind).toBe("disconnected"); // Hub role WITH a client block stays mismatched (the honest conflict). writeFileSync(join(home, "config.json"), JSON.stringify({ port: 10190, runtimeRole: "hub", client: { serverUrl: "https://hub.example.test" } })); - expect(readStateProbe(readScript, home).kind).toBe("mismatched"); + expect((await readStateProbe(readScript, home)).kind).toBe("mismatched"); } finally { removeTreeWithRetry(home); } - }, 35_000); // Two 15s child deadlines plus setup and cleanup, below the CI 60s cap. + }, 35_000); // Two 15s child deadlines plus bounded 1s cleanup each, below the CI 60s cap. - test("state probe kills a stalled child before parsing its output", () => { + test("state probe kills a stalled child before parsing its output", async () => { const home = mkdtempSync(join(tmpdir(), "ocx-state-probe-stall-")); const startedPath = join(home, "probe-started"); const script = ` @@ -95,7 +147,7 @@ describe("remote hub client boundary", () => { try { const startedAt = performance.now(); let failure: unknown; - try { readStateProbe(script, home, 2_000); } + try { await readStateProbe(script, home, 2_000); } catch (error) { failure = error; } expect(performance.now() - startedAt).toBeLessThan(10_000); expect(failure).toBeInstanceOf(ClientStateProbeError); @@ -105,7 +157,7 @@ describe("remote hub client boundary", () => { expect(failure.signal).toBe("SIGKILL"); expect(failure.message).not.toContain("not-json"); expect(Number(readFileSync(startedPath, "utf8"))).toBe(failure.pid); - // spawnSync must reap this exact child, not merely return while it remains alive. + // The async probe must reap this exact child, not merely return while it remains alive. let exitCode: string | undefined; try { process.kill(failure.pid, 0); } catch (error) { exitCode = (error as NodeJS.ErrnoException).code; }