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]

已合入 master(7d3bf3d),保留你为共同作者 🙏 因为最终落地的形态和这个 PR 的分支不同,说明一下具体怎么处理的。

采纳的

① FREEFORM 触发词 → 落地为规则 (a7)

你的发现是对的,而且 7/7 确定性复现 + 双片段 A/B(只改一处仍拦、两处都改才过)这个方法尤其对 —— Devin 的 content policy 是非确定性的(同 prompt 先拦后过),重复验证是唯一靠得住的定位手段,项目里 a6 规则当初就是因为拿不到可靠 A/B 才 ship 成默认关的。

但排查你这个 case 的时候,发现根因比两个字符串更深一层:chat.js 里工具描述 preamble 是注入在 neutralizeClientIdentity 之后的(injectPreambleIntoSystemPrompt 在后、中和在前),所以 native 路径上任何经由工具描述进来的触发词都完整绕过 a1-a6 整条防线 —— 漏的不只是 FREEFORM。

所以最终版做了两件事:把中和移到 preamble 注入之后,并把你那两条改写做成 (a7) 进既有规则序列。这样复用主开关 WINDSURFAPI_NEUTRALIZE_CLIENT_ID 和现有测试体系(不用新加 env),而且覆盖的是所有客户端、所有触发词 —— 你原来的 sanitizeToolDescriptions() 只作用于 connectTools,Cascade 路径的 preamble 还是原文。

② responses.js 裸 {role, content} —— 原样采纳

Codex 确实这么发,item.type === 'message' || (item.role && !item.type) 的判定也不会误吃 function_call / function_call_output(那些没有 role)。

没采纳的

__route: 'responses' → 'messages' —— 见上面那两条评论,它会让 Codex 的 GPT 请求退回 openai_json_xml dialect。补充一句我之前说错又更正过的:这个回归影响的是源码部署的默认路径(DEVIN_CONNECT 默认只在打包 exe 里开),不是边缘配置。

windsurfapi-content-policy.patch —— 130 行自我复制,没法跟代码同步。

现在的状态

identity-neutralize.js 覆盖四个客户端:Claude Code(a1-a4)/ Cline(a5)/ Grok(a6-grok,来自 #227)/ codex(a7,你)。合并后做了四客户端共存的交叉验证,混合 prompt 同时命中、互不干扰。

已加入致谢名单(dashboard credits 面板 + README 中英),评级 B+ —— FREEFORM 是真发现,而且顺带牵出了那个顺序缺陷,这个价值比两行字符串大。

commit 里带了 Co-authored-by: forrinzhao <fulin.zhao@gmail.com>,GitHub 贡献统计能识别到。因为落地形态与分支差异较大(且分支含未采纳的 hunk),走的是 cherry-pick 合入而不是 Merge 按钮,所以这个 PR 我关闭了 —— 代码 100% 已在主干。

下次有这类发现,直接开 issue 带最小复现也行,通常比 PR 更快落地。谢谢 🙏