-
Notifications
You must be signed in to change notification settings - Fork 785
fix(codex): harden routed apply_patch contracts #1623
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Wibias
wants to merge
52
commits into
lidge-jun:dev
from
Wibias:test/apply-patch-code-mode-regression
Closed
Changes from 11 commits
Commits
Show all changes
52 commits
Select commit
Hold shift + click to select a range
bba9f4b
test(adapters): verify routed Code Mode keeps apply_patch usable
Wibias e8c1317
test(adapters): cover apply_patch translation round trip
Wibias 38bbb0c
test: extend routed patch regression coverage
Wibias 83a4222
test: correct routed regression assertion
Wibias 89a4c73
test: guard apply_patch across routed adapter nudges
Wibias add4229
test: enforce catalog nudge coverage completeness
Wibias f1946dc
test(adapters): assert nested apply_patch declaration survives
Wibias 56bd9cf
test(cursor): pin apply_patch native mutation policy
Wibias da63a8a
test: pin routed catalog patch contract
Wibias ff85971
test: cover responses native patch contract
Wibias 9a06659
test: classify every adapter patch strategy
Wibias 8468a28
test: fix responses adapter harness
Wibias cf7e8ff
normalize routed catalog metadata
Wibias 84f0d37
test: scope catalog patch normalization
Wibias 6534475
test: verify resolved apply_patch strategies
Wibias 87e92f0
test(adapters): add C+ registry conformance gate
Wibias d88e0e1
refactor(adapters): route construction through registry
Wibias db0cc98
feat(adapters): add authoritative C+ registry
Wibias 3887f57
test(adapters): narrow C+ bypass detector
Wibias 1841484
test(adapters): add protocol-keyed C+ wire drivers
Wibias 635b86d
test(adapters): execute C+ request conformance for registry
Wibias 9311c38
test(adapters): remove reflective adapter completeness parser
Wibias 6c0c905
test(adapters): add apply_patch conformance contract oracle
Wibias fa8960c
test(adapters): mutation-test C+ conformance oracle
Wibias c7c3af7
test(adapters): add provider-native tool response drivers
Wibias 6ecd3c2
test(adapters): release observed request budgets
Wibias f257c43
test(adapters): round-trip apply_patch through every parsed wire
Wibias 31e540e
fix(adapters): satisfy typed registry construction
Wibias 3c602fc
test(adapters): complete C+ apply_patch conformance matrix
Wibias 8096413
testability(adapters): thread Cursor transport deps through registry …
Wibias 0303880
testability(cursor): pass existing runTurn seam through registry
Wibias d253feb
test(cursor): exercise registry runTurn path
Wibias e9b3d3b
test(cursor): align registry runTurn error assertion
Wibias 713cbc5
refactor(adapters): keep generic contracts Cursor-agnostic
Wibias b1c408d
refactor(adapters): keep Cursor test deps inside registry boundary
Wibias a4e81d6
test(cursor): match disabled registry transport error
Wibias e67529e
test(adapters): enforce tool_choice none by wire semantics
Wibias 29fdbc3
test: isolate conformance request observation
Wibias 44bfcf1
fix: isolate MiMo conformance auth
Wibias 57e895d
fix: expose deterministic adapter test deps
Wibias b86b926
test: preserve explicit routed patch tool type
Wibias f361cb6
fix: preserve explicit routed patch tool type
Wibias be4658d
test: cover Cursor apply_patch runTurn roundtrip
Wibias 807b017
test: harden adapter registry conformance
Wibias 00ed5ce
fix: route lab adapter construction through registry
Wibias 472ebd1
test: fix TypeScript AST runtime import
Wibias b547464
test: use Bun parser for registry enforcement
Wibias 571e511
test: detect adapter factory bypasses without blocking shared imports
Wibias 500b5e7
fix: restore Kiro tool-name assertion after guard refactor
Wibias 2b03b92
fix: preserve MiMo buildRequest compatibility
Wibias 52f3379
fix: harden adapter registry invariants
Wibias a955d64
test: enforce registry parent types in source gate
Wibias File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| import { expect, test } from "bun:test"; | ||
| import { readdir, readFile } from "node:fs/promises"; | ||
| import { join, relative, sep } from "node:path"; | ||
| import { fileURLToPath } from "node:url"; | ||
|
|
||
| const adaptersDir = fileURLToPath(new URL("../src/adapters/", import.meta.url)); | ||
| const regressionTestPath = fileURLToPath(new URL("./apply-patch-adapter-nudge-regression.test.ts", import.meta.url)); | ||
| const adapterResolvePath = fileURLToPath(new URL("../src/server/adapter-resolve.ts", import.meta.url)); | ||
| const nudgeImport = /(?:from\s+|import\s*\()\s*["'][^"']*tool-catalog-nudge(?:\.[cm]?[jt]s)?["']/; | ||
|
|
||
| const APPLY_PATCH_ADAPTER_STRATEGIES = { | ||
| "command-code": "nudge-routed", | ||
| "openai-chat": "nudge-routed", | ||
| anthropic: "nudge-routed", | ||
| "openai-responses": "responses-native", | ||
| google: "nudge-routed", | ||
| kiro: "nudge-routed", | ||
| azure: "responses-native-wrapper", | ||
| "azure-openai": "responses-native-wrapper", | ||
| cursor: "cursor-special", | ||
| "mimo-free": "openai-chat-wrapper", | ||
| } as const; | ||
|
|
||
| async function discoverNudgeCallSites(dir = adaptersDir): Promise<string[]> { | ||
| const entries = await readdir(dir, { withFileTypes: true }); | ||
| const found: string[] = []; | ||
|
|
||
| for (const entry of entries) { | ||
| const fullPath = join(dir, entry.name); | ||
| if (entry.isDirectory()) { | ||
| found.push(...await discoverNudgeCallSites(fullPath)); | ||
| continue; | ||
| } | ||
| if (!entry.isFile() || !entry.name.endsWith(".ts")) continue; | ||
|
|
||
| const source = await readFile(fullPath, "utf8"); | ||
| if (!nudgeImport.test(source)) continue; | ||
|
|
||
| const relativePath = relative(adaptersDir, fullPath).split(sep).join("/"); | ||
| found.push(`src/adapters/${relativePath}`); | ||
| } | ||
|
|
||
| return found.sort(); | ||
| } | ||
|
|
||
| async function discoverCoveredAdapterModules(): Promise<string[]> { | ||
| const source = await readFile(regressionTestPath, "utf8"); | ||
| const imports = source.matchAll(/from\s+["']\.\.\/src\/adapters\/([^"']+)["']/g); | ||
| return [...imports] | ||
| .map(match => `src/adapters/${match[1]}.ts`) | ||
| .sort(); | ||
| } | ||
|
|
||
| async function discoverResolvedAdapterNames(): Promise<string[]> { | ||
| const source = await readFile(adapterResolvePath, "utf8"); | ||
| return [...source.matchAll(/case\s+["']([^"']+)["']\s*:/g)] | ||
| .map(match => match[1]!) | ||
| .sort(); | ||
| } | ||
|
|
||
| test("every tool-catalog-nudge adapter has outbound apply_patch coverage", async () => { | ||
| expect(await discoverNudgeCallSites()).toEqual(await discoverCoveredAdapterModules()); | ||
| }); | ||
|
|
||
| test("every resolved adapter has an explicit apply_patch strategy", async () => { | ||
| expect(await discoverResolvedAdapterNames()).toEqual( | ||
| Object.keys(APPLY_PATCH_ADAPTER_STRATEGIES).sort(), | ||
| ); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,121 @@ | ||
| import { expect, test } from "bun:test"; | ||
| import { createAnthropicAdapter } from "../src/adapters/anthropic"; | ||
| import { createCommandCodeAdapter } from "../src/adapters/command-code"; | ||
| import { createGoogleAdapter } from "../src/adapters/google"; | ||
| import { createKiroAdapter } from "../src/adapters/kiro"; | ||
| import { createOpenAIChatAdapter } from "../src/adapters/openai-chat"; | ||
| import type { OcxParsedRequest, OcxProviderConfig } from "../src/types"; | ||
|
|
||
| function codeModeParsed(modelId: string): OcxParsedRequest { | ||
| return { | ||
| modelId, | ||
| stream: true, | ||
| options: {}, | ||
| context: { | ||
| systemPrompt: ["Use apply_patch for local file edits."], | ||
| messages: [{ role: "user", content: "Patch a file.", timestamp: 1 }], | ||
| tools: [ | ||
| { | ||
| name: "exec", | ||
| description: "Run JavaScript. declare const tools: { apply_patch(input: string): Promise<unknown>; };", | ||
| parameters: { type: "object", properties: { input: { type: "string" } } }, | ||
| }, | ||
| { | ||
| name: "wait", | ||
| description: "Wait for work to finish.", | ||
| parameters: { type: "object", properties: {} }, | ||
| }, | ||
| { | ||
| name: "request_user_input", | ||
| description: "Ask the user for input.", | ||
| parameters: { type: "object", properties: {} }, | ||
| }, | ||
| ], | ||
| }, | ||
| } as OcxParsedRequest; | ||
| } | ||
|
|
||
| function assertApplyPatchIsNotForbidden(body: string): void { | ||
| const normalized = body.replace(/\\n/g, " ").replace(/\s+/g, " "); | ||
|
|
||
| // Make sure the adapter actually exercised the shared catalog-nudge call site; | ||
| // otherwise a missing nudge would make the prohibition assertion vacuous. | ||
| expect(normalized).toContain("current tool catalog as ground truth"); | ||
|
|
||
| // The final provider request must still advertise Codex's nested patch helper. | ||
| // This pins the actual Code Mode declaration rather than a nonexistent literal | ||
| // `tools.apply_patch` token in the serialized tool description. | ||
| expect(normalized).toContain("declare const tools: { apply_patch(input: string): Promise<unknown>; };"); | ||
|
|
||
| // Protect against both the original shared warning and adapter-specific wording | ||
| // that would steer routed models away from Codex's own patch tool. | ||
| expect(normalized).not.toMatch(/(?:do not|don't|never|must not|cannot|can't)[^.]{0,260}\bapply_patch\b/i); | ||
| expect(normalized).not.toMatch(/\bapply_patch\b[^.]{0,180}\b(?:forbidden|unavailable|off-limits)\b/i); | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| test("routed adapter call sites never forbid Codex apply_patch", async () => { | ||
| const cases: Array<{ | ||
| name: string; | ||
| modelId: string; | ||
| build: (parsed: OcxParsedRequest) => Promise<{ body: string }>; | ||
| }> = [ | ||
| { | ||
| name: "openai-chat", | ||
| modelId: "grok-4.6", | ||
| build: async parsed => createOpenAIChatAdapter({ | ||
| adapter: "openai-chat", | ||
| baseUrl: "https://api.x.ai/v1", | ||
| apiKey: "test-key", | ||
| } as OcxProviderConfig).buildRequest(parsed), | ||
| }, | ||
| { | ||
| name: "anthropic-oauth", | ||
| modelId: "claude-haiku-4-5", | ||
| build: async parsed => createAnthropicAdapter({ | ||
| adapter: "anthropic", | ||
| baseUrl: "https://api.anthropic.com", | ||
| authMode: "oauth", | ||
| apiKey: "test-oauth-token", | ||
| } as OcxProviderConfig).buildRequest(parsed), | ||
| }, | ||
| { | ||
| name: "google", | ||
| modelId: "gemini-3-pro", | ||
| build: async parsed => createGoogleAdapter({ | ||
| adapter: "google", | ||
| baseUrl: "https://generativelanguage.googleapis.com", | ||
| apiKey: "test-key", | ||
| } as OcxProviderConfig).buildRequest(parsed), | ||
| }, | ||
| { | ||
| name: "command-code", | ||
| modelId: "deepseek/deepseek-v4-flash", | ||
| build: async parsed => createCommandCodeAdapter({ | ||
| adapter: "command-code", | ||
| baseUrl: "https://api.commandcode.ai", | ||
| authMode: "oauth", | ||
| apiKey: "test-command-key", | ||
| defaultMaxOutputTokens: 64_000, | ||
| } as OcxProviderConfig).buildRequest(parsed), | ||
| }, | ||
| { | ||
| name: "kiro", | ||
| modelId: "claude-sonnet-4.5", | ||
| build: async parsed => { | ||
| parsed._kiroAuthContext = { apiRegion: "us-east-1" }; | ||
| return createKiroAdapter({ | ||
| adapter: "kiro", | ||
| baseUrl: "https://runtime.us-east-1.kiro.dev", | ||
| authMode: "key", | ||
| apiKey: "ksk_test", | ||
| } as OcxProviderConfig).buildRequest(parsed); | ||
| }, | ||
| }, | ||
| ]; | ||
|
|
||
| for (const adapterCase of cases) { | ||
| const request = await adapterCase.build(codeModeParsed(adapterCase.modelId)); | ||
| expect(request.body, `${adapterCase.name} should serialize a request body`).toBeTruthy(); | ||
| assertApplyPatchIsNotForbidden(request.body); | ||
| } | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| import { expect, test } from "bun:test"; | ||
| import { normalizeRoutedCatalogEntry } from "../src/codex/catalog/parsing"; | ||
|
|
||
| test("routed catalog rows force Codex apply_patch to the freeform tool contract", () => { | ||
| const row = normalizeRoutedCatalogEntry({ | ||
| slug: "xai/grok-4.6", | ||
| tool_mode: "legacy", | ||
| apply_patch_tool_type: "function", | ||
| context_window: 128_000, | ||
| }); | ||
|
|
||
| expect(row.tool_mode).toBe("code_mode_only"); | ||
| expect(row.apply_patch_tool_type).toBe("freeform"); | ||
|
Check failure on line 13 in tests/apply-patch-catalog-contract.test.ts
|
||
|
Wibias marked this conversation as resolved.
|
||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,166 @@ | ||
| import { expect, test } from "bun:test"; | ||
| import { createOpenAIChatAdapter as createOpenAIChatAdapterProduction } from "../src/adapters/openai-chat"; | ||
| import { bridgeToResponsesSSE } from "../src/bridge"; | ||
| import { parseRequest } from "../src/responses/parser"; | ||
| import { buildToolBridgeMaps } from "../src/server/responses"; | ||
| import type { OcxParsedRequest, OcxProviderConfig } from "../src/types"; | ||
| import { withTestTranslatorBudget } from "./helpers/translator-budget"; | ||
|
|
||
| const provider: OcxProviderConfig = { | ||
| adapter: "openai-chat", | ||
| baseUrl: "https://api.x.ai/v1", | ||
| apiKey: "test-key", | ||
| }; | ||
|
|
||
| const createOpenAIChatAdapter = (...args: Parameters<typeof createOpenAIChatAdapterProduction>) => | ||
| withTestTranslatorBudget(createOpenAIChatAdapterProduction(...args)); | ||
|
|
||
| function parseSse(text: string): Array<{ event?: string; data: Record<string, unknown> }> { | ||
| return text.split("\n\n") | ||
| .map(frame => frame.trim()) | ||
| .filter(frame => frame.length > 0 && frame !== "data: [DONE]") | ||
| .map(frame => { | ||
| const lines = frame.split("\n"); | ||
| const event = lines.find(line => line.startsWith("event: "))?.slice(7); | ||
| const data = lines.find(line => line.startsWith("data: "))?.slice(6) ?? "{}"; | ||
| return { event, data: JSON.parse(data) as Record<string, unknown> }; | ||
| }); | ||
| } | ||
|
|
||
| test("routed Code Mode does not forbid nested apply_patch when it is absent from the flat catalog", () => { | ||
| const parsed: OcxParsedRequest = { | ||
| modelId: "grok-4.6", | ||
| context: { | ||
| systemPrompt: ["Use apply_patch for local file edits."], | ||
| messages: [{ role: "user", content: "Edit the requested file.", timestamp: 0 }], | ||
| tools: [ | ||
| { | ||
| name: "exec", | ||
| description: "Run JavaScript. declare const tools: { apply_patch(input: string): Promise<unknown>; };", | ||
| parameters: {}, | ||
| }, | ||
| { name: "wait", description: "Wait for an exec cell.", parameters: {} }, | ||
| { name: "request_user_input", description: "Ask the user a question.", parameters: {} }, | ||
| ], | ||
| }, | ||
| stream: false, | ||
| options: {}, | ||
| }; | ||
|
|
||
| const request = createOpenAIChatAdapter(provider).buildRequest(parsed); | ||
| const body = JSON.parse(request.body) as { | ||
| messages: Array<{ role: string; content: unknown }>; | ||
| tools?: Array<{ function?: { name?: string; description?: string } }>; | ||
| }; | ||
|
|
||
| const systemText = body.messages | ||
| .filter(message => message.role === "system" && typeof message.content === "string") | ||
| .map(message => message.content as string) | ||
| .join("\n"); | ||
| const execTool = body.tools?.find(tool => tool.function?.name === "exec"); | ||
|
|
||
| // Reproduce the actual Code Mode shape: patching exists only as a nested exec helper, | ||
| // not as a top-level wire tool. Before 0325a5a the injected nudge contradicted this | ||
| // contract by explicitly forbidding apply_patch, which pushed routed models to Python/sed. | ||
| expect(execTool?.function?.description).toContain("apply_patch"); | ||
| expect(body.tools?.some(tool => tool.function?.name === "apply_patch")).toBe(false); | ||
| expect(systemText).toContain("Use apply_patch for local file edits."); | ||
| expect(systemText).toContain("Valid tool names for this turn are exactly `exec`, `wait`, `request_user_input`"); | ||
| expect(systemText).toContain("Do not use neighboring-agent tool names"); | ||
| expect(systemText).not.toMatch(/Do not use neighboring-agent tool names[^.]*apply_patch/); | ||
| }); | ||
|
|
||
| test("routed chat round-trips a declared apply_patch custom tool with valid freeform input", async () => { | ||
| const patch = [ | ||
| "*** Begin Patch", | ||
| "*** Add File: ocx-apply-patch-smoke.txt", | ||
| "+apply patch smoke", | ||
| "*** End Patch", | ||
| ].join("\n"); | ||
| const parsed = parseRequest({ | ||
| model: "xai/grok-4.6", | ||
| input: "Create the smoke-test file.", | ||
| stream: true, | ||
| tools: [{ type: "custom", name: "apply_patch", description: "Apply a patch" }], | ||
| }); | ||
| const maps = buildToolBridgeMaps(parsed); | ||
| const adapter = createOpenAIChatAdapter(provider); | ||
|
|
||
| // Responses custom/freeform tools are lowered to a normal chat function with one string | ||
| // field. If this wrapper changes or disappears, routed chat models cannot call apply_patch. | ||
| const outbound = JSON.parse(adapter.buildRequest(parsed).body) as { | ||
| tools?: Array<{ | ||
| type?: string; | ||
| function?: { | ||
| name?: string; | ||
| parameters?: { properties?: { input?: { type?: string } } }; | ||
| }; | ||
| }>; | ||
| }; | ||
| expect(outbound.tools?.find(tool => tool.function?.name === "apply_patch")).toMatchObject({ | ||
| type: "function", | ||
| function: { | ||
| name: "apply_patch", | ||
| parameters: { properties: { input: { type: "string" } } }, | ||
| }, | ||
| }); | ||
|
|
||
| // Simulate the routed chat model selecting that function. The adapter must parse the call, | ||
| // then the Responses bridge must unwrap {input:string} back into a native custom_tool_call. | ||
| const upstreamPayload = JSON.stringify({ | ||
| choices: [{ | ||
| delta: { | ||
| tool_calls: [{ | ||
| index: 0, | ||
| id: "call_patch", | ||
| type: "function", | ||
| function: { | ||
| name: "apply_patch", | ||
| arguments: JSON.stringify({ input: patch }), | ||
| }, | ||
| }], | ||
| }, | ||
| finish_reason: "tool_calls", | ||
| }], | ||
| }); | ||
| const upstream = new Response(`data: ${upstreamPayload}\n\ndata: [DONE]\n\n`); | ||
| const bridged = bridgeToResponsesSSE( | ||
| adapter.parseStream(upstream), | ||
| parsed.modelId, | ||
| maps.toolNsMap, | ||
| maps.freeformToolNames, | ||
| maps.toolSearchToolNames, | ||
| undefined, | ||
| 2_000, | ||
| { declaredToolNames: maps.declaredToolNames }, | ||
| ); | ||
| const frames = parseSse(await new Response(bridged).text()); | ||
|
|
||
| const inputDone = frames.find(frame => frame.event === "response.custom_tool_call_input.done")?.data; | ||
| expect(inputDone?.input).toBe(patch); | ||
|
|
||
| const itemDone = frames.find(frame => { | ||
| if (frame.event !== "response.output_item.done") return false; | ||
| const item = frame.data.item as Record<string, unknown> | undefined; | ||
| return item?.type === "custom_tool_call" && item.name === "apply_patch"; | ||
| })?.data.item as Record<string, unknown> | undefined; | ||
| expect(itemDone).toMatchObject({ | ||
| type: "custom_tool_call", | ||
| call_id: "call_patch", | ||
| name: "apply_patch", | ||
| input: patch, | ||
| status: "completed", | ||
| }); | ||
|
|
||
| const completed = frames.find(frame => frame.event === "response.completed")?.data.response as | ||
| | { status?: string; output?: Array<Record<string, unknown>> } | ||
| | undefined; | ||
| expect(completed?.status).toBe("completed"); | ||
| expect(completed?.output).toContainEqual(expect.objectContaining({ | ||
| type: "custom_tool_call", | ||
| name: "apply_patch", | ||
| input: patch, | ||
| status: "completed", | ||
| })); | ||
| expect(frames.some(frame => frame.event === "response.failed")).toBe(false); | ||
| }); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.