fix: neutralize codex apply_patch tool description to avoid content-policy block

[forrinzhao]

Problem

When using the Feishu/Lark codex chatbot bridged through the WindsurfAPI proxy, every request failed with:

Your request was blocked by our content policy.

Root cause

The codex apply_patch tool description contains the sentence:

"FREEFORM tool, so do not wrap the patch in JSON."

When this tool description is injected into the system prompt via the tool preamble, Devin Connect's content filter flags it and rejects the whole request. This is a false positive triggered purely by the wording of the tool description, not by any user content.

Fix

Added sanitizeToolDescriptions() in src/handlers/identity-neutralize.js, which rewrites the two flagged fragments:

• FREEFORM → free-form
• do not wrap the patch in JSON. → provide the patch as plain text.

It is wired into src/handlers/chat.js at connectTools = sanitizeToolDescriptions(_trim.tools).

Can be disabled with env var WINDSURFAPI_NEUTRALIZE_TOOL_DESC=0.

Also included (codex bot support)

• src/handlers/responses.js: treat bare {role, content} input items (no explicit type:"message", as Codex sends them) as messages so they aren't dropped; route the Responses path through __route: 'messages' (was 'responses').

Changes

• src/handlers/identity-neutralize.js — new exported sanitizeToolDescriptions()
• src/handlers/chat.js — apply sanitization to connect tools
• src/handlers/responses.js — codex input-item + routing handling
• windsurfapi-content-policy.patch — standalone clean patch (applies to upstream master)

Note: This PR is a clean re-base directly on upstream master (supersedes the earlier draft PR #218, which accidentally bundled unrelated local branch changes).

[dwgx]

哦，Bro，我明白了，但我希望你可以进行过多测试，进行一个完美的黄金PR 我会感谢你合并这个PR
非常感谢你的贡献代码 如果是一个完美的黄金S PR 我会很开心 就像你做出的贡献对得起你的 内容
再次感谢各位贡献者

[dwgx]

哎我操 我不在日本国 我无法review这个PR我是糟糕困难的抱歉 等待我20天

[dwgx]

Thanks @forrinzhao 🙏 The diagnosis here is genuinely good. Live-bisecting FREEFORM down to a 7/7 deterministic repro, then A/B-ing that both fragments have to change or it still blocks — that is the only method that works against Devin's content policy, which is non-deterministic (same prompt blocked, then passed hours later). That's exactly why the a6 rule in identity-neutralize.js ships default-OFF: I could never get a reliable A/B on it. Your JSDoc already matches the a1–a6 evidence format, so you clearly read the module before writing into it. That saved me a round.

I'm not merging it as-is though. One hunk has to come out, and the main fix should move somewhere else — details and the exact steps below.

---

1. Drop the __route change — it's an unrelated regression

-    const result = await chatHandler({ ...chatBody, stream: false, __route: 'responses' }, context);
+    const result = await chatHandler({ ...chatBody, stream: false, __route: 'messages' }, context);

__route isn't a label. It reaches pickToolDialect(modelKey, provider, route), and route === 'responses' is the only gate that enables the gpt_native dialect added in v2.0.62 (#115) for the Codex/Responses path:

$ node -e "import('./src/handlers/tool-emulation.js').then(m=>['responses','messages'].forEach(r=>console.log(r,'->',m.pickToolDialect('gpt-5.5-medium','openai',r))))"
responses -> gpt_native
messages  -> openai_json_xml

There are 7 pickToolDialect call sites in tool-emulation.js (175 / 510 / 639 / 686 / 737 / 953 / 1217) covering preamble construction, assistant-history serialization, and the streaming tool-call parser. Flipping the route moves all three back to openai_json_xml — the exact failure mode documented at tool-emulation.js:308-323: GPT-5.x refusing the <tool_call> XML protocol with "please paste the file", and the shell_command probe fabricating head="PROBE_NO_EMUL_1777749121" instead of calling the function.

Here's why your testing didn't catch it, and why this is the important part: DEVIN_CONNECT defaults to ON (config.js:55), and that branch is a short-circuit (chat.js:2274, selectBackend(...).flow === 'devin_connect') that hard-codes route: 'devin_connect' at chat.js:2377 and 2415. On the default deployment body.__route never reaches dialect selection at all — so your change is invisible there. It only bites on the Cascade/LS path (chat.js:3134, DEVIN_CONNECT=0), where it also shifts buildReuseOpts conversation-reuse fingerprints (3385/3430) and the native-bridge WINDSURFAPI_NATIVE_TOOL_BRIDGE_ROUTES allowlist (3032).

So: this hunk is inert on the path you fixed, and breaks a path you didn't test. That's the cleanest possible argument for not bundling it.

CI won't catch it either — I ran npm test on your branch, 2761/2761 green, because test/tool-emulation-gpt-native.test.js asserts against pickToolDialect(..., 'responses') directly and never goes through handleResponses.

__route: 'responses' was introduced deliberately in 38398c4 ("2.0.25 — Cascade conversation reuse hardening, 7 codex audit items"). If you hit something real on the Responses path, open a separate issue with the symptom and a minimal repro and I'll look at it on its own. It just isn't the same bug as the content-policy block.

---

2. Make it rule (a7) instead of a new function — you found a structural gap, use it

This is the part worth your time. There's a root cause underneath your bug that's bigger than FREEFORM.

Look at the actual ordering in the DEVIN_CONNECT branch:

• 2376 — normalizeMessagesForCascade(...) builds connectMessages (the emulation preamble lands in the system prompt here)
• 2388-2394 — neutralizeClientIdentity(m.content) runs over the system prompt body
• 2395-2434 — HYBRID native path (nativeStructured = nativeDefsOn && nativeCallsOn): applyToolPreambleBudget(connectTools, ...) builds the description-only preamble, and 2433 injectPreambleIntoSystemPrompt injects it

The emulation preamble is injected before neutralization and gets covered by a1–a6. The HYBRID native preamble is injected after it and bypasses the entire defense line.

And nativeToolCall defaults to true (runtime-config.js:56), so nativeStructured is true on a default deployment with tools — which means Codex with native tool defs goes down exactly the uncovered branch. That's why this only ever surfaced for you and not for the Cline / Claude Code reports that a1–a5 came from: their triggers sit in the original system prompt or ride the emulation preamble, both of which are already covered.

So the smaller, more architectural fix is two steps:

1. Move neutralization after preamble injection (or run it a second time after 2433 — neutralizeClientIdentity is idempotent, the regexes stop matching once rewritten, so a repeat pass is safe). That puts the HYBRID native preamble behind the same line as everything else.
2. Add your two rewrites as (a7) inside neutralizeClientIdentity, following the existing numbering and evidence-comment format. Yours is live-bisected and confirmed, so it's a1–a5 tier, not a6 tier — it rides the existing WINDSURFAPI_NEUTRALIZE_CLIENT_ID master switch and doesn't need WINDSURFAPI_NEUTRALIZE_TOOL_DESC at all.

What that buys:

• No new export in identity-neutralize.js, no new call site in chat.js — the net diff ends up smaller than what you have now
• You inherit the existing switch, tests, and docs instead of building a parallel set
• It fixes a whole class, not one string. Any client that puts an a1–a6 trigger in a tool description (Cline's capability boast, the Claude Code brand block) is currently just as unprotected as FREEFORM was. Tool descriptions and the system prompt end up in the same pool; they should go through the same filter.

Your current sanitizeToolDescriptions() also only touches connectTools, so the Cascade/LS preamble (chat.js:3134, built from toolRouting.emulationTools) still ships the raw text. The (a7) approach doesn't have that blind spot.

If putting this in identity-neutralize.js feels off given its "client-identity" header — it fits. a2 / a4 / a5 are already content-policy rules; that module is where the content-policy defense line actually lives. Update the top JSDoc to say so.

---

3. Delete windsurfapi-content-policy.patch

130 lines duplicating this PR's own diff, at the repo root. It can't stay in sync with the code and it's stale the day after merge. gh pr diff 219 covers the standalone-patch use case, and your PR description already explains it.

---

What I need to merge this

1. Drop the __route hunk (both call sites in responses.js)
2. Rewrite the fix as (a7) in neutralizeClientIdentity + move/repeat neutralization after injectPreambleIntoSystemPrompt
3. Delete windsurfapi-content-policy.patch
4. Add tests — test/client-identity-neutralize.test.js is the template, every a-rule has coverage there including the =0 off-switch case. For a7: rewrite fires, unrelated descriptions untouched, idempotent, off-switch returns input unchanged. A regression test that the HYBRID native preamble is neutralized would be even better
5. Squash — 1a35871 / 8b7d978 / 84c2af6 have identical messages

Also: the responses.js bare {role, content} fix is correct and I want it. Codex does send input items that way, and item.type === 'message' || (item.role && !item.type) won't swallow function_call / function_call_output since those carry no role. It's just a separate concern — send it as its own PR and I'll merge it ahead of this one. Per CONTRIBUTING, one PR per concern; it also keeps git bisect honest later.

Minor, take or leave:

• Tollerant → Tolerant in the JSDoc
• In fixOne(), a match on t.description returns early and never checks t.function.description. The two shapes don't co-occur in practice, but stacking the two ifs is sturdier
• The rewrite is currently silent. Everything else in this area is observable (recordArgRepair, recordNativeBridgeDecision, recordPolicyBlocked). v3.5.0 shipped a content-policy block ring buffer + dashboard panel (28897bd / b48ab60) — worth a log.debug at minimum so the next case like this doesn't need a human bisect

One more thing I owe you: CONTRIBUTING still says "项目暂无自动测试 / no automated tests yet." That's stale — npm test is 2761 tests now. I'll fix the doc.

Get those five done and I'll merge with you as the original author, and add you to the dashboard credits panel and the README credits (EN + 中文). FREEFORM is a real find — it deserves a rule with your name on it in identity-neutralize.js.

[dwgx]

Correction to my review above — I got one fact wrong, and it flips the severity, so flagging it before you act on it.

I wrote "DEVIN_CONNECT defaults to ON (config.js:55)". That's only true for the packaged .exe — config.js:55 sits inside if (IS_PACKAGED) (IS_PACKAGED = !!process.pkg, config.js:16/54). A source install defaults DEVIN_CONNECT OFF, i.e. the Cascade backend.

What that actually changes:

• If you run DEVIN_CONNECT=1 (likely, since the content-policy block is Devin-side): the request short-circuits into the DEVIN_CONNECT branch at chat.js:2272, where route is hard-coded 'devin_connect' (2377/2415) and body.__route never reaches pickToolDialect. That's why the change looked harmless in your testing — on that path it genuinely is.
• But a source install with DEVIN_CONNECT=0 (the default) runs the Cascade path (chat.js:3129 etc., route: body.__route || 'chat'), where a Codex /v1/responses request carries body.__route='responses' → gpt_native. Flipping it to 'messages' breaks exactly that path.

So I had the blast radius backwards: the regression doesn't hit some edge config, it hits the source-install default. That's more reason to drop the hunk, not less. Everything else in the review stands.

[dwgx]

Where this lands — and it lands with your name on it

Thanks @forrinzhao. I traced this end-to-end (while calibrating a separate billing issue on a paid account) and want to be straight about the outcome.

Your finding is real. FREEFORM tripping the content filter, bisected 7/7 — a genuine discovery, and it's going into the tree credited to you regardless of how it merges.

The root cause is one level deeper than the two string rewrites, though — and it changes the right fix. In the DEVIN_CONNECT branch, neutralizeClientIdentity runs at chat.js:2388, but the HYBRID-native tool-description preamble is injected at chat.js:2433 — after it. Native tool-calls are on by default, so Codex-with-native-tools takes exactly that path. FREEFORM didn't leak for lack of a rule; it leaked because the tool-description preamble is injected past the entire a1–a6 content-policy line. A pre-existing structural gap your PR surfaced — not one it introduced.

So the complete fix is:

• move neutralization to run after preamble injection (it's idempotent, so a second pass is safe) → this covers every client's tool descriptions, not just FREEFORM
• land your two rewrites as rule (a7) inside neutralizeClientIdentity, riding the existing WINDSURFAPI_NEUTRALIZE_CLIENT_ID switch + test harness (no new env var needed)
• take your responses.js bare-{role,content} fix as-is — it's correct, Codex does send input items that way
• minus the __route hunk (see my correction above — it regresses the source-install default) and minus the checked-in .patch

Your call how it ships:

1. You update this PR along those lines (drop __route, delete the .patch, move the rewrites into (a7), add a test mirroring test/client-identity-neutralize.test.js) and we merge your branch; or
2. we land it from our side and credit you as the author of the (a7) rule in the dashboard credits panel + README (EN / 中文) — same as we did for @warelik in fix: neutralize MCP-gate + inject tool-description preamble into system prompt on native path #216 — and close this with thanks.

Either way you're the author of that rule. No rush for a couple of days; if we don't hear back we'll take route 2 so the fix ships, but I'd genuinely rather merge yours.

Small stuff regardless of route:

• Tollerant → Tolerant in the JSDoc
• in fixOne(), a match on t.description returns before it can check t.function.description
• sanitizeToolDescriptions only touches connectTools, so the Cascade preamble (chat.js:3134) still ships the raw text — which the (a7) approach fixes for free