[chrarnoldus]

Summary

Follow-up to #7646. removeTopLevelToolSchemaCombinators rewrote Claude tool schemas without leaving a trace, so we could not tell how often it fires or which tools trigger it.

It now emits one warnExceptInTest (console.warn outside automated tests) per request when it changes at least one tool, in the same structured style as the existing removeUnsupportedRequestServiceTier and removeNonSensicalMaxTokens logs:

[removeTopLevelToolSchemaCombinators] removed top-level tool schema combinators
{ model, requestKind, tools: [{ name, combinators: ['oneOf'], strict }] }

• Only tool names, the removed keywords and the strict flag are logged, not schemas or descriptions, to keep log lines small.
• Nothing is logged when no tool has a top-level combinator.

Verification

• No manual test. The log has no unit test: warnExceptInTest is a no-op in tests, and asserting a diagnostic log line is not worth mocking it. The existing combinator tests pass unchanged.

Visual Changes

N/A

Reviewer Notes

A client that always sends an affected tool (e.g. the nteract MCP server) produces one log line per Claude request. That matches the other gateway workaround logs; we can sample or lower it once we know the volume.

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

The incremental change (commit 16cb2e5) only reverts the test-side log verification added earlier: it drops the jest.mock('@kilocode/web-shared/lib/utils.server', ...), the warn capture and the logs the changed tools once per request assertion. The test file is now identical to the base branch, so the PR's net diff is limited to remove-top-level-tool-schema-combinators.ts. The source still calls warnExceptInTest, which is a no-op under automated tests, so the remaining suite passes unchanged and no runtime, security, or memory-leak concern is introduced. The author's justification for omitting a log assertion (a diagnostic no-op in tests, not worth mocking) is reasonable and does not reduce effective coverage of the schema-rewrite behavior, which the surviving tests still assert.

Files Reviewed (2 files)

• packages/web-shared/src/lib/ai-gateway/providers/remove-top-level-tool-schema-combinators.ts
• packages/web-shared/src/lib/ai-gateway/providers/remove-top-level-tool-schema-combinators.test.ts

Previous Review Summaries (2 snapshots, latest commit ed2a518)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit ed2a518)

Status: No Issues Found | Recommendation: Merge

The incremental change replaced the direct console.warn in removeTopLevelToolSchemaCombinators with the repo-standard warnExceptInTest helper, and updated the tests to mock @kilocode/web-shared/lib/utils.server (the same mock pattern used in handler.test.ts and auto-routing-decision.test.ts). The mock exposes only the used named export, so it cannot break unrelated imports; warn is captured via jest.mocked and reset per test, and the existing payload assertions still validate the changed-tool shape. No bug, security, or memory-leak concern introduced.

Files Reviewed (2 files)

• packages/web-shared/src/lib/ai-gateway/providers/remove-top-level-tool-schema-combinators.ts
• packages/web-shared/src/lib/ai-gateway/providers/remove-top-level-tool-schema-combinators.test.ts

Previous review (commit 5450a1f)

Status: No Issues Found | Recommendation: Merge

The logging added to removeTopLevelToolSchemaCombinators is correct and matches the existing gateway workaround logs (removeUnsupportedRequestServiceTier, removeNonSensicalMaxTokens): the per-tool combinator detection mirrors the condition used inside removeTopLevelSchemaCombinators, so the reported combinators/strict values reflect the same mutations the function already performs; strict is normalized to a boolean and only tool names/keywords are emitted, which is consistent with the repo's logging baseline (no tokens, credentials, or schemas). No memory-leak or unbounded-retention concern: changedTools is function-local and nothing is retained after the call. The test additions correctly verify the payload for mixed changed/unchanged tools and that nothing is logged when no tool changes.

Files Reviewed (2 files)

• packages/web-shared/src/lib/ai-gateway/providers/remove-top-level-tool-schema-combinators.ts
• packages/web-shared/src/lib/ai-gateway/providers/remove-top-level-tool-schema-combinators.test.ts

---

_{Reviewed by deepseek-v4.1-flash · Input: 33.2K · Output: 4.7K · Cached: 321.8K}

_{Review guidance: REVIEW.md from base branch main}