Repository navigation
fix(tools): keep HY3 tool schemas inline - #1923
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🧰 Additional context used📓 Path-based instructions (5)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**⚙️ CodeRabbit configuration file
Files:
**/*⚙️ CodeRabbit configuration file
Files:
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}⚙️ CodeRabbit configuration file
Files:
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (9)
📝 WalkthroughWalkthroughTencent HY3 capability metadata now disables vision and increases the output limit. Tool-search compatibility excludes HY3 tool references, while OpenAI-compatible conversion recovers HY3 XML tool calls in streaming and non-streaming responses. ChangesTencent HY3 compatibility and tool-call recovery
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
1a5ea89 to
d3170c5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/api/openaiShim.xmlToolCalls.test.ts`:
- Around line 393-398: The test helper run() must restore globalThis.fetch after
each invocation to prevent stale SSE behavior leaking between tests. Wrap its
fetch override and execution in try/finally, restoring the original fetch in the
finally block, matching collectFallbackEvents cleanup; ensure restoration occurs
on both success and failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 16b1dbb2-6ad4-47cd-a8ba-6b747b852bcd
📒 Files selected for processing (8)
src/integrations/brands/tencent.tssrc/integrations/models/tencent.tssrc/integrations/tencent.test.tssrc/services/api/openaiShim.test.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.xmlToolCalls.test.tssrc/utils/toolSearch.test.tssrc/utils/toolSearch.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: typecheck
- GitHub Check: smoke-and-tests (22)
- GitHub Check: smoke-and-tests (24.11.x)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
Files:
src/integrations/models/tencent.tssrc/integrations/tencent.test.tssrc/integrations/brands/tencent.tssrc/utils/toolSearch.test.tssrc/utils/toolSearch.tssrc/services/api/openaiShim.test.tssrc/services/api/openaiShim.xmlToolCalls.test.tssrc/services/api/openaiShim.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.src/integrations/- provider and model integration metadata.src/entrypoints/- CLI, MCP, SDK, and generated public types.src/tasks/- local, remote, workflow, and monitor tas...
Files:
src/integrations/models/tencent.tssrc/integrations/tencent.test.tssrc/integrations/brands/tencent.tssrc/utils/toolSearch.test.tssrc/utils/toolSearch.tssrc/services/api/openaiShim.test.tssrc/services/api/openaiShim.xmlToolCalls.test.tssrc/services/api/openaiShim.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/integrations/models/tencent.tssrc/integrations/tencent.test.tssrc/integrations/brands/tencent.tssrc/utils/toolSearch.test.tssrc/utils/toolSearch.tssrc/services/api/openaiShim.test.tssrc/services/api/openaiShim.xmlToolCalls.test.tssrc/services/api/openaiShim.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/integrations/models/tencent.tssrc/integrations/tencent.test.tssrc/integrations/brands/tencent.tssrc/services/api/openaiShim.test.tssrc/services/api/openaiShim.xmlToolCalls.test.tssrc/services/api/openaiShim.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/integrations/tencent.test.tssrc/utils/toolSearch.test.tssrc/services/api/openaiShim.test.tssrc/services/api/openaiShim.xmlToolCalls.test.ts
🔇 Additional comments (11)
src/integrations/brands/tencent.ts (1)
8-8: LGTM!src/integrations/models/tencent.ts (1)
9-20: LGTM!src/integrations/tencent.test.ts (1)
1-41: LGTM!src/utils/toolSearch.ts (1)
242-265: LGTM!src/utils/toolSearch.test.ts (1)
1-12: LGTM!Also applies to: 57-83
src/services/api/openaiShim.ts (3)
1899-1907: 🔒 Security & PrivacyDo not treat syntax as authorization.
A literal assistant-text example such as
<tool_call:call_1>TaskCreate</tool_call:call_1>passes this check and is converted into atool_use; the current test only rejects examples whose extracted name contains invalid characters. Verify that downstream execution revalidates recovered names against the current authorized tool set, or add stronger gating before emitting executable calls.
1820-1851: LGTM!Also applies to: 1865-1897, 1908-1935, 1954-1971, 1983-2037, 2391-2442, 2920-2920
1947-1951: 🎯 Functional CorrectnessDuplicate identical XML tool calls are intentionally collapsed. The parser already deduplicates repeated
name + argspairs, so this isn’t a bug.> Likely an incorrect or invalid review comment.src/services/api/openaiShim.xmlToolCalls.test.ts (2)
10-15: LGTM!Also applies to: 65-70, 133-201
393-398: 🎯 Functional CorrectnessNo model change needed here. The XML recovery path is content-driven, so these HY3 cases already exercise the Tencent-shaped stream handling; the request model only affects metadata.
> Likely an incorrect or invalid review comment.src/services/api/openaiShim.test.ts (1)
10148-10189: LGTM!
Summary
TaskCreateschema loop reported in Tencent HY3 tool calling issues #1912Root cause
HY3 does not reliably perform the extra
ToolSearchdiscovery turn before invoking a deferred tool, soTaskCreatecould be omitted from the request schema. Live OpenGateway validation also showed that, under OpenClaude's streamed request shape, HY3 emits<tool_call:...>text with named arguments instead of structured OpenAItool_calls; the shim previously surfaced that text instead of executing the tool. The shim now supports both that gateway form and Tencent HY3's documented tagged<tool_call:opensource>/<arg_key:opensource>grammar.Validation
bun test src/utils/toolSearch.test.ts src/integrations/tencent.test.ts src/integrations/index.test.tsbun test src/services/api/openaiShim.xmlToolCalls.test.ts src/services/api/openaiShim.test.tsbun run test:providerbun run typecheckbun run integrations:checkbun run security:pr-scan -- --base upstream/main --head HEADbun run buildnode dist/cli.mjs --versionLive validation used a minimal, isolated TaskCreate prompt against OpenGateway HY3. The shim converted the returned textual wrapper into
TaskCreatewith the expectedsubjectanddescriptionfields.Final reviewed commit:
d3170c5065a717b19a8ae995163e2b9b9e567b4a.Fixes #1912
Summary by CodeRabbit