[Bug]: repaired code-mode apply_patch wrappers compile the original malformed body

[Ingwannu]

Client or integration

Codex code mode through translated Responses/Chat providers.

Area

Custom tool compatibility.

Summary

#4983 recognizes additional malformed exec/apply_patch representations, including outer Markdown fences and fallback fields, but helper compilation still receives the original wrapper.

resolveCodeModeHelperName decides that the repaired body is an apply_patch call. Its callers then pass the original argumentsText to compileCodeModeHelperInput. The compiler's apply-patch branch unwraps only input and patch; it does not consume the other accepted fallback fields or strip the outer fence. Generated JavaScript can therefore call tools.apply_patch with the JSON wrapper or fenced text instead of the patch.

Expected behaviour

Recognition and compilation must consume one canonical repaired body. A representation accepted as an apply-patch helper must compile exactly that patch text, while ordinary caller-defined exec input remains byte-exact.

Required regressions

• every accepted fallback field compiles to the same raw patch;
• fenced and unfenced forms compile identically;
• a normal code-mode JavaScript body remains unchanged;
• a caller-defined non-code-mode exec is never reinterpreted; and
• the compiled nested helper succeeds through the actual bridge, not only a recognizer unit test.

Regression origin

Merged in #4983; current dev still calls compileCodeModeHelperInput(args, helper) with the pre-repair args.

[lidge-jun]

리뷰 · 우선순위 74 / 80

이 이슈는 #4983이 exec/apply_patch의 잘못된 wrapper(바깥 Markdown fence, fallback 필드 등)를 인식은 하는데, code-mode 헬퍼를 컴파일할 때는 아직 수리 전 원문을 넘긴다는 틈을 말합니다. 지금 dev(facd2b6ca)의 호출 패턴을 보면 이렇습니다.

resolveCodeModeHelperName(...)은 인자 문자열을 보고 “이건 apply_patch다”라고 판단합니다. 그런데 바로 다음 compileCodeModeHelperInput(args, helper)에는 **같은 원본 args**가 들어갑니다. src/bridge/sse.ts의 freeformInput, src/bridge/response-json.ts, src/responses/custom-tool-compat.ts, src/server/responses-custom-tool-repair.ts가 모두 비슷한 형태입니다.

문제는 컴파일러 쪽에 있습니다. compileCodeModeHelperInput의 apply_patch 분기는 내부 unwrapPatchInput으로 input/patch 키만 풉니다. #4983이 인정한 다른 fallback 필드나 바깥 fence를 전부 소화하지 못합니다. 인식기는 unwrapFreeformToolInput(fallback keys + fence strip)을 쓰는데, 컴파일러는 그보다 좁은 unwrap만 씁니다. 그래서 “이건 apply_patch”라고 판정한 뒤에도 생성된 JS가 tools.apply_patch에 JSON wrapper나 fenced 텍스트를 넣을 수 있습니다. 인식과 컴파일이 같은 정규 본문을 쓰지 않는 전형적인 버그입니다.

기대는 한 줄입니다. 인정된 표현은 하나의 canonical repaired body로 컴파일되어야 하고, 그 결과가 실제 패치 텍스트여야 합니다. 보통 code-mode JS 본문과, code-mode가 아닌 호출자 정의 exec는 바이트 그대로 남겨야 합니다. 회귀도 이슈에 잘 적혀 있습니다. fallback 필드마다 같은 raw patch, fence 유무 동일, 정상 JS 불변, non-code-mode exec 재해석 금지, 그리고 인식 unit만이 아니라 실제 bridge 경로까지.

라인 146-148 (src/bridge/sse.ts) - helper를 정한 뒤 compileCodeModeHelperInput(args, helper)에 수리 전 args를 넘긴다.
라인 12-22 / 37-39 (src/responses/code-mode-helper-compat.ts) - unwrapPatchInput은 input/patch만. fence·다른 fallback key는 여기 없다.
라인 38-58 (src/responses/apply-patch-envelope.ts, unwrapFreeformToolInput) - 인식/판정 쪽은 FREEFORM_FALLBACK_KEYS + stripMarkdownCodeFence를 이미 쓴다. 컴파일 경로가 이 결과를 재사용하지 않는다.
경로 #4983 - 회귀 원점 지적이 checkout과 맞다. tip #5040(combo RF 테스트)과 무관한 tools/proxy 후속이다.

메인테이너의 판단이 필요한 지점

• 고치는 축: (A) 호출부가 unwrapFreeformToolInput 결과를 compile에 넘기기, (B) unwrapPatchInput을 unwrapFreeform과 동일 규칙으로 확장. 중복을 줄이려면 A+공유 헬퍼가 더 안전해 보인다.
• resolve가 본 본문과 compile이 쓰는 본문이 어긋나면 보안/오실행 위험이 있다(잘못된 patch 문자열을 JS data로 넣음). 우선순위를 bug-fix로 둘지.
• 이슈 [Bug]: fallback freeform wrappers stream raw JSON deltas then finish with unwrapped input #5047(스트리밍 partial)과 같은 fix(responses): recover malformed freeform tool wrappers #4983 후속이지만 축이 다르다(완료 컴파일 vs partial delta). 한 PR에 묶지 말고 각각 닫는 편이 리뷰하기 쉽다.

너의 추천
인정. resolveCodeModeHelperName이 true를 반환한 경로에서는 compile 입력을 반드시 unwrapFreeformToolInput(또는 동일 규칙) 결과로 통일하고, 이슈의 bridge-level 회귀를 테스트로 고정한 좁은 PR을 merge. #5047과 스택으로 묶지 말 것.

이 댓글은 grok-bot이 작성했습니다

[lidge-jun]

Fixed on dev by #5051, merged as the commit above.

Recognition and compilation now read one canonical body from one unwrap, under the name the body actually arrived on. compileCodeModeHelperInput takes that wire name as a third argument and all five call sites pass it; unwrapPatchInput is gone, since unwrapFreeformToolInput under apply_patch covers everything it did.

The wire name rather than the helper name matters, and the regression suite pins why. The name-based path arrives as apply_patch, whose fallback fields are patch and content; the body-based path arrives as exec, whose fields are the code-carrying ones and deliberately exclude patch. Recognition already declines {"patch": ...} under exec, so compiling it there would have accepted a body recognition rejected — the drift a second, looser unwrap introduces.

All five regressions you listed are covered in tests/responses/responses-code-mode-patch-compile.test.ts: every accepted fallback field compiles to the same raw patch; fenced and unfenced forms compile identically, including a language-tagged fence and a fence nested in an input wrapper; a normal code-mode JavaScript body is left alone; a caller-defined exec outside a code-mode catalog is never reinterpreted, across three ways of not being code mode; and the last case runs through restoreRoutedCustomCallsInJson rather than the recognizer alone, so the agreement is proven on the item a client receives. Before the fix that path restored tools.apply_patch("{\"content\":\"*** Begin Patch...\"}").

[Ingwannu]

Reopening for the supported default.apply_patch alias that remains broken on current dev.

#5051 made recognition and compilation share a canonical body, but bridge callers pass codeModeHelperName ?? toolName as the wire vocabulary. For a name-based default.apply_patch alias that value is default.apply_patch; unwrapFreeformToolInput recognizes fallback fields and fences only for exact apply_patch. A {"patch":"..."} wrapper or fenced patch can therefore still reach tools.apply_patch wrapped.

Normalize the default. prefix before selecting the wire vocabulary for name-based apply-patch calls, while preserving exec for payload-inferred calls. Add production coverage through both buildResponseJSON and bridgeToResponsesSSE; helper-only tests are insufficient.