feat(zai): add GLM-5.2 support#1689
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (13)src/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}📄 CodeRabbit inference engine (AGENTS.md)
Files:
{src/services/**/*.ts,src/utils/**/*.ts}📄 CodeRabbit inference engine (AGENTS.md)
Files:
{src/integrations/**/*.ts,src/services/**/*.ts}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx,py,json,md}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{ts,tsx,js,jsx,py}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*⚙️ CodeRabbit configuration file
Files:
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}⚙️ CodeRabbit configuration file
Files:
**⚙️ CodeRabbit configuration file
Files:
**/*.test.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.test.{ts,tsx,js}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (2)
📝 WalkthroughWalkthroughAdds ChangesZ.AI GLM-5.2 Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
chioarub
force-pushed
the
feat/zai-glm-5-2-thinking
branch
from
486ab73 to
eb29647
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.test.ts`:
- Around line 6431-6537: Add a new test that validates the retry mutation
behavior for tool_stream stripping during self-heal. Create a test that mocks
the fetch function to return a 400 error with tool_call_incompatible on the
first request and a successful response on the second request for a Z.AI GLM-5.2
streaming request with tools. Capture both the first and second request bodies
and assert that the first request body includes tool_stream set to true while
the second request body omits tool_stream (or has it undefined). This test
should specifically validate that when the createOpenAIShimClient initiates the
beta.messages.create call with the same parameters as the existing positive test
(model glm-5.2, stream true, with tools), the self-heal retry properly strips
out the tool_stream parameter.
In `@src/services/api/openaiShim.ts`:
- Around line 2513-2516: The zaiThinkingType assignment uses
request.thinking?.type as a fallback without normalization, allowing adaptive
values from model/query overlays to bypass enabled/disabled handling. In the
openaiShim.ts file around the zaiThinkingType variable assignment, apply
normalizeThinkingType() to the request.thinking?.type fallback value to ensure
consistent normalization of all thinking type sources, matching the treatment
already applied to requestedThinkingType. This prevents silent provider behavior
drift when reasoning effort is not explicitly set.
🪄 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 Plus
Run ID: c6c89105-48d0-4c14-993a-f8fdb38c3d17
📒 Files selected for processing (17)
.env.exampleREADME.mdsrc/integrations/brands/glm.tssrc/integrations/descriptors.tssrc/integrations/models/glm.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/runtimeMetadata.tssrc/integrations/vendors/zai.tssrc/services/api/openaiShim.test.tssrc/services/api/openaiShim.tssrc/services/api/providerConfig.test.tssrc/services/api/providerConfig.tssrc/utils/context.test.tssrc/utils/providerFlag.test.tssrc/utils/providerProfiles.test.tssrc/utils/thinking.test.tssrc/utils/thinking.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (16)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/integrations/models/glm.tssrc/utils/thinking.tssrc/utils/providerFlag.test.tssrc/utils/thinking.test.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/descriptors.tssrc/integrations/brands/glm.tssrc/utils/providerProfiles.test.tssrc/services/api/providerConfig.test.tssrc/integrations/runtimeMetadata.tssrc/integrations/vendors/zai.tssrc/services/api/openaiShim.tssrc/services/api/providerConfig.tssrc/utils/context.test.tssrc/services/api/openaiShim.test.ts
{src/integrations/**/*.ts,src/services/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Test the exact provider/model path you changed when possible for provider modifications
Files:
src/integrations/models/glm.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/descriptors.tssrc/integrations/brands/glm.tssrc/services/api/providerConfig.test.tssrc/integrations/runtimeMetadata.tssrc/integrations/vendors/zai.tssrc/services/api/openaiShim.tssrc/services/api/providerConfig.tssrc/services/api/openaiShim.test.ts
src/integrations/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Check existing provider implementations before adding a new pattern
Files:
src/integrations/models/glm.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/descriptors.tssrc/integrations/brands/glm.tssrc/integrations/runtimeMetadata.tssrc/integrations/vendors/zai.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/integrations/models/glm.tssrc/utils/thinking.tssrc/utils/providerFlag.test.tssrc/utils/thinking.test.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/descriptors.tssrc/integrations/brands/glm.tssrc/utils/providerProfiles.test.tsREADME.mdsrc/services/api/providerConfig.test.tssrc/integrations/runtimeMetadata.tssrc/integrations/vendors/zai.tssrc/services/api/openaiShim.tssrc/services/api/providerConfig.tssrc/utils/context.test.tssrc/services/api/openaiShim.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/integrations/models/glm.tssrc/utils/thinking.tssrc/utils/providerFlag.test.tssrc/utils/thinking.test.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/descriptors.tssrc/integrations/brands/glm.tssrc/utils/providerProfiles.test.tssrc/services/api/providerConfig.test.tssrc/integrations/runtimeMetadata.tssrc/integrations/vendors/zai.tssrc/services/api/openaiShim.tssrc/services/api/providerConfig.tssrc/utils/context.test.tssrc/services/api/openaiShim.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/integrations/models/glm.tssrc/utils/thinking.tssrc/utils/providerFlag.test.tssrc/utils/thinking.test.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/descriptors.tssrc/integrations/brands/glm.tssrc/utils/providerProfiles.test.tssrc/services/api/providerConfig.test.tssrc/integrations/runtimeMetadata.tssrc/integrations/vendors/zai.tssrc/services/api/openaiShim.tssrc/services/api/providerConfig.tssrc/utils/context.test.tssrc/services/api/openaiShim.test.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/glm.tssrc/utils/thinking.tssrc/utils/providerFlag.test.tssrc/utils/thinking.test.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/descriptors.tssrc/integrations/brands/glm.tssrc/utils/providerProfiles.test.tsREADME.mdsrc/services/api/providerConfig.test.tssrc/integrations/runtimeMetadata.tssrc/integrations/vendors/zai.tssrc/services/api/openaiShim.tssrc/services/api/providerConfig.tssrc/utils/context.test.tssrc/services/api/openaiShim.test.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/glm.tssrc/utils/providerFlag.test.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/descriptors.tssrc/integrations/brands/glm.tssrc/utils/providerProfiles.test.tssrc/services/api/providerConfig.test.tssrc/integrations/runtimeMetadata.tssrc/integrations/vendors/zai.tssrc/services/api/openaiShim.tssrc/services/api/providerConfig.tssrc/services/api/openaiShim.test.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.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
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/integration...
Files:
src/integrations/models/glm.tssrc/utils/thinking.tssrc/utils/providerFlag.test.tssrc/utils/thinking.test.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/descriptors.tssrc/integrations/brands/glm.tssrc/utils/providerProfiles.test.tsREADME.mdsrc/services/api/providerConfig.test.tssrc/integrations/runtimeMetadata.tssrc/integrations/vendors/zai.tssrc/services/api/openaiShim.tssrc/services/api/providerConfig.tssrc/utils/context.test.tssrc/services/api/openaiShim.test.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/utils/thinking.tssrc/utils/providerFlag.test.tssrc/utils/thinking.test.tssrc/utils/providerProfiles.test.tssrc/services/api/providerConfig.test.tssrc/services/api/openaiShim.tssrc/services/api/providerConfig.tssrc/utils/context.test.tssrc/services/api/openaiShim.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/utils/providerFlag.test.tssrc/utils/thinking.test.tssrc/integrations/runtimeMetadata.test.tssrc/utils/providerProfiles.test.tssrc/services/api/providerConfig.test.tssrc/utils/context.test.tssrc/services/api/openaiShim.test.ts
src/**/*provider*.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Provider implementations must follow documented patterns in docs/integrations/
Files:
src/utils/providerFlag.test.tssrc/utils/providerProfiles.test.tssrc/services/api/providerConfig.test.tssrc/services/api/providerConfig.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/utils/providerFlag.test.tssrc/utils/thinking.test.tssrc/integrations/runtimeMetadata.test.tssrc/utils/providerProfiles.test.tssrc/services/api/providerConfig.test.tssrc/utils/context.test.tssrc/services/api/openaiShim.test.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/utils/providerFlag.test.tssrc/utils/thinking.test.tssrc/integrations/runtimeMetadata.test.tssrc/utils/providerProfiles.test.tssrc/services/api/providerConfig.test.tssrc/utils/context.test.tssrc/services/api/openaiShim.test.ts
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}
⚙️ CodeRabbit configuration file
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}: Review docs for accuracy against current code behavior. Flag security or provider claims that overpromise, stale install commands, missing setup caveats, and instructions that could push users toward unsafe credential handling. Keep purely wording-level suggestions non-blocking.
Files:
README.md
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/services/api/providerConfig.test.tssrc/services/api/openaiShim.tssrc/services/api/providerConfig.tssrc/services/api/openaiShim.test.ts
🔇 Additional comments (16)
src/services/api/providerConfig.test.ts (1)
5-26: LGTM!src/integrations/runtimeMetadata.ts (1)
32-45: LGTM!Also applies to: 284-289, 400-417
src/utils/thinking.ts (1)
93-97: LGTM!src/integrations/runtimeMetadata.test.ts (1)
74-105: LGTM!src/services/api/openaiShim.ts (1)
210-227: LGTM!Also applies to: 2495-2510, 2543-2549, 3280-3283
src/services/api/providerConfig.ts (1)
113-130: LGTM!Also applies to: 146-148, 253-259, 319-326, 787-787
src/integrations/descriptors.ts (1)
40-41: LGTM!src/integrations/brands/glm.ts (1)
16-16: LGTM!src/integrations/models/glm.ts (1)
32-32: LGTM!src/integrations/vendors/zai.ts (1)
8-8: LGTM!Also applies to: 21-21, 47-57
README.md (1)
195-195: LGTM!Also applies to: 229-229, 263-263, 284-284
src/utils/context.test.ts (1)
721-743: Test updates for Z.AI GLM context windows and output caps are accurate.The glm-5.2 assertions correctly reflect the 1M context window and 131K output token limits stated in the PR objectives. Preserving existing GLM uppercase-variant assertions ensures backward compatibility.
src/utils/providerFlag.test.ts (1)
515-515: Default model expectation correctly updated to reflect vendor change.The expected OPENAI_MODEL now matches the Z.AI vendor's defaultModel configuration of
glm-5.2.src/utils/providerProfiles.test.ts (1)
1525-1525: Preset defaults test aligned with upstream vendor configuration.The expected model now matches the Z.AI defaultModel of
glm-5.2.src/utils/thinking.test.ts (1)
82-83: Query-string thinking parameter variants correctly tested.Both space-present and space-absent query parameter variants (
?thinking=disabledand?thinking=disabled) are tested, validating the query-string parsing and stripping behavior described in the PR stack..env.example (1)
174-181: Z.AI GLM-5.2 documentation examples are clear and complete.The expanded examples demonstrate the primary glm-5.2 model, optional thinking control query parameters (
reasoning=high,reasoning=xhigh,thinking=disabled), and alternative GLM model names. Users now have explicit guidance for GLM-5.2 reasoning-effort control.
chioarub
force-pushed
the
feat/zai-glm-5-2-thinking
branch
from
eb29647 to
6940c5e
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.ts`:
- Around line 2512-2531: The code contains a redundant assignment of
body.thinking to { type: 'enabled' } that occurs both in the else if block for
zaiThinkingType === 'enabled' (line 2522) and again in the subsequent if block
when zaiThinkingType !== 'disabled' and request.reasoning?.effort exists (line
2526). Remove the redundant body.thinking assignment from inside the if block
that handles request.reasoning?.effort, keeping only the
normalizeZaiReasoningEffort assignment, since the thinking type is already set
to 'enabled' in the preceding else-if block.
🪄 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 Plus
Run ID: 24f74b76-a77c-479a-b15b-176c1590756a
📒 Files selected for processing (17)
.env.exampleREADME.mdsrc/integrations/brands/glm.tssrc/integrations/descriptors.tssrc/integrations/models/glm.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/runtimeMetadata.tssrc/integrations/vendors/zai.tssrc/services/api/openaiShim.test.tssrc/services/api/openaiShim.tssrc/services/api/providerConfig.test.tssrc/services/api/providerConfig.tssrc/utils/context.test.tssrc/utils/providerFlag.test.tssrc/utils/providerProfiles.test.tssrc/utils/thinking.test.tssrc/utils/thinking.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (16)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/integrations/brands/glm.tssrc/utils/thinking.test.tssrc/integrations/models/glm.tssrc/utils/thinking.tssrc/integrations/descriptors.tssrc/utils/providerFlag.test.tssrc/services/api/providerConfig.test.tssrc/integrations/runtimeMetadata.test.tssrc/utils/context.test.tssrc/integrations/vendors/zai.tssrc/integrations/runtimeMetadata.tssrc/utils/providerProfiles.test.tssrc/services/api/providerConfig.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.ts
{src/integrations/**/*.ts,src/services/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Test the exact provider/model path you changed when possible for provider modifications
Files:
src/integrations/brands/glm.tssrc/integrations/models/glm.tssrc/integrations/descriptors.tssrc/services/api/providerConfig.test.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/vendors/zai.tssrc/integrations/runtimeMetadata.tssrc/services/api/providerConfig.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.ts
src/integrations/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Check existing provider implementations before adding a new pattern
Files:
src/integrations/brands/glm.tssrc/integrations/models/glm.tssrc/integrations/descriptors.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/vendors/zai.tssrc/integrations/runtimeMetadata.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/integrations/brands/glm.tssrc/utils/thinking.test.tssrc/integrations/models/glm.tssrc/utils/thinking.tssrc/integrations/descriptors.tssrc/utils/providerFlag.test.tssrc/services/api/providerConfig.test.tssrc/integrations/runtimeMetadata.test.tsREADME.mdsrc/utils/context.test.tssrc/integrations/vendors/zai.tssrc/integrations/runtimeMetadata.tssrc/utils/providerProfiles.test.tssrc/services/api/providerConfig.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/integrations/brands/glm.tssrc/utils/thinking.test.tssrc/integrations/models/glm.tssrc/utils/thinking.tssrc/integrations/descriptors.tssrc/utils/providerFlag.test.tssrc/services/api/providerConfig.test.tssrc/integrations/runtimeMetadata.test.tssrc/utils/context.test.tssrc/integrations/vendors/zai.tssrc/integrations/runtimeMetadata.tssrc/utils/providerProfiles.test.tssrc/services/api/providerConfig.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/integrations/brands/glm.tssrc/utils/thinking.test.tssrc/integrations/models/glm.tssrc/utils/thinking.tssrc/integrations/descriptors.tssrc/utils/providerFlag.test.tssrc/services/api/providerConfig.test.tssrc/integrations/runtimeMetadata.test.tssrc/utils/context.test.tssrc/integrations/vendors/zai.tssrc/integrations/runtimeMetadata.tssrc/utils/providerProfiles.test.tssrc/services/api/providerConfig.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.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/brands/glm.tssrc/utils/thinking.test.tssrc/integrations/models/glm.tssrc/utils/thinking.tssrc/integrations/descriptors.tssrc/utils/providerFlag.test.tssrc/services/api/providerConfig.test.tssrc/integrations/runtimeMetadata.test.tsREADME.mdsrc/utils/context.test.tssrc/integrations/vendors/zai.tssrc/integrations/runtimeMetadata.tssrc/utils/providerProfiles.test.tssrc/services/api/providerConfig.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.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/brands/glm.tssrc/integrations/models/glm.tssrc/integrations/descriptors.tssrc/utils/providerFlag.test.tssrc/services/api/providerConfig.test.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/vendors/zai.tssrc/integrations/runtimeMetadata.tssrc/utils/providerProfiles.test.tssrc/services/api/providerConfig.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.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.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
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/integration...
Files:
src/integrations/brands/glm.tssrc/utils/thinking.test.tssrc/integrations/models/glm.tssrc/utils/thinking.tssrc/integrations/descriptors.tssrc/utils/providerFlag.test.tssrc/services/api/providerConfig.test.tssrc/integrations/runtimeMetadata.test.tsREADME.mdsrc/utils/context.test.tssrc/integrations/vendors/zai.tssrc/integrations/runtimeMetadata.tssrc/utils/providerProfiles.test.tssrc/services/api/providerConfig.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/utils/thinking.test.tssrc/utils/thinking.tssrc/utils/providerFlag.test.tssrc/services/api/providerConfig.test.tssrc/utils/context.test.tssrc/utils/providerProfiles.test.tssrc/services/api/providerConfig.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/utils/thinking.test.tssrc/utils/providerFlag.test.tssrc/services/api/providerConfig.test.tssrc/integrations/runtimeMetadata.test.tssrc/utils/context.test.tssrc/utils/providerProfiles.test.tssrc/services/api/openaiShim.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/utils/thinking.test.tssrc/utils/providerFlag.test.tssrc/services/api/providerConfig.test.tssrc/integrations/runtimeMetadata.test.tssrc/utils/context.test.tssrc/utils/providerProfiles.test.tssrc/services/api/openaiShim.test.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/utils/thinking.test.tssrc/utils/providerFlag.test.tssrc/services/api/providerConfig.test.tssrc/integrations/runtimeMetadata.test.tssrc/utils/context.test.tssrc/utils/providerProfiles.test.tssrc/services/api/openaiShim.test.ts
src/**/*provider*.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Provider implementations must follow documented patterns in docs/integrations/
Files:
src/utils/providerFlag.test.tssrc/services/api/providerConfig.test.tssrc/utils/providerProfiles.test.tssrc/services/api/providerConfig.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/services/api/providerConfig.test.tssrc/services/api/providerConfig.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.ts
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}
⚙️ CodeRabbit configuration file
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}: Review docs for accuracy against current code behavior. Flag security or provider claims that overpromise, stale install commands, missing setup caveats, and instructions that could push users toward unsafe credential handling. Keep purely wording-level suggestions non-blocking.
Files:
README.md
🔇 Additional comments (29)
src/services/api/providerConfig.test.ts (1)
1-27: LGTM!src/integrations/runtimeMetadata.ts (1)
32-46: LGTM!Also applies to: 284-289, 400-417
src/utils/thinking.ts (1)
93-93: LGTM!src/services/api/openaiShim.ts (4)
210-227: LGTM!
2495-2510: LGTM!
2544-2550: LGTM!
2283-2283: LGTM!src/integrations/runtimeMetadata.test.ts (1)
74-85: LGTM!Also applies to: 87-105
src/services/api/openaiShim.test.ts (1)
5154-5154: LGTM!Also applies to: 6299-6643
src/services/api/providerConfig.ts (5)
113-113: LGTM!Also applies to: 128-130
146-148: LGTM!
253-259: LGTM!
319-319: LGTM!Also applies to: 325-325
787-787: LGTM!src/integrations/descriptors.ts (1)
40-41: LGTM!src/integrations/brands/glm.ts (1)
16-16: LGTM!src/integrations/models/glm.ts (1)
32-32: LGTM!src/integrations/vendors/zai.ts (3)
8-8: LGTM!
21-21: LGTM!
47-57: LGTM!README.md (4)
195-195: LGTM!
229-229: LGTM!
263-263: LGTM!
284-284: LGTM!src/utils/context.test.ts (1)
721-729: LGTM!src/utils/providerFlag.test.ts (1)
515-515: LGTM!src/utils/providerProfiles.test.ts (1)
1525-1525: LGTM!src/utils/thinking.test.ts (1)
82-83: LGTM!.env.example (1)
176-181: LGTM!
chioarub
force-pushed
the
feat/zai-glm-5-2-thinking
branch
from
6940c5e to
e97dec4
Compare
kevincodex1
left a comment
kevincodex1
left a comment
There was a problem hiding this comment.
LGTM . please check too @jatmn
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@README.md`:
- Line 108: The note on line 108 overstates the security guarantee by claiming
the `/provider` command "securely stores credentials" when the actual storage
mechanism is the `.openclaude-profile.json` file, which may not be encrypted or
keychain-backed. Replace the phrase "securely stores credentials" with a more
accurate description that specifies the credentials are persisted to
`.openclaude-profile.json` and avoid implying encryption or advanced security
measures unless that protection is actually implemented.
In `@src/services/api/providerConfig.ts`:
- Around line 822-841: The gheCopilotBaseUrl variable is only derived from
githubEnterpriseEnvUrl (gheUrl), but when isGithubGhe is true due to rawBaseUrl
being set and githubEnterpriseEnvUrl is unset, gheCopilotBaseUrl remains
undefined. This causes the base URL rewrite at lines 893-900 to fail. Fix this
by also checking if rawBaseUrl is a GHE host when gheUrl is undefined, and use
buildGithubEnterpriseCopilotBaseUrl with rawBaseUrl in that case to ensure
gheCopilotBaseUrl is properly set. Additionally, add a regression test in
src/services/api/providerConfig.test.ts covering the scenario where GitHub mode
is enabled with a GHE OPENAI_BASE_URL but no GITHUB_ENTERPRISE_URL, asserting
that the final baseUrl includes the .../api/copilot path.
🪄 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 Plus
Run ID: 72522fba-4818-469c-9263-f3548decbb7c
📒 Files selected for processing (6)
.env.exampleREADME.mdsrc/services/api/openaiShim.test.tssrc/services/api/openaiShim.tssrc/services/api/providerConfig.tssrc/utils/providerProfiles.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (15)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/utils/providerProfiles.test.tssrc/services/api/providerConfig.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/utils/providerProfiles.test.tssrc/services/api/providerConfig.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/utils/providerProfiles.test.tssrc/services/api/providerConfig.tssrc/services/api/openaiShim.tsREADME.mdsrc/services/api/openaiShim.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/utils/providerProfiles.test.tssrc/services/api/openaiShim.test.ts
src/**/*provider*.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Provider implementations must follow documented patterns in docs/integrations/
Files:
src/utils/providerProfiles.test.tssrc/services/api/providerConfig.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/utils/providerProfiles.test.tssrc/services/api/openaiShim.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/utils/providerProfiles.test.tssrc/services/api/providerConfig.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/utils/providerProfiles.test.tssrc/services/api/providerConfig.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.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/utils/providerProfiles.test.tssrc/services/api/providerConfig.tssrc/services/api/openaiShim.tsREADME.mdsrc/services/api/openaiShim.test.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/utils/providerProfiles.test.tssrc/services/api/providerConfig.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.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/utils/providerProfiles.test.tssrc/services/api/openaiShim.test.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.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
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/integration...
Files:
src/utils/providerProfiles.test.tssrc/services/api/providerConfig.tssrc/services/api/openaiShim.tsREADME.mdsrc/services/api/openaiShim.test.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/services/api/providerConfig.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.ts
{src/integrations/**/*.ts,src/services/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Test the exact provider/model path you changed when possible for provider modifications
Files:
src/services/api/providerConfig.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.ts
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}
⚙️ CodeRabbit configuration file
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}: Review docs for accuracy against current code behavior. Flag security or provider claims that overpromise, stale install commands, missing setup caveats, and instructions that could push users toward unsafe credential handling. Keep purely wording-level suggestions non-blocking.
Files:
README.md
🔇 Additional comments (10)
src/services/api/openaiShim.test.ts (1)
21-22: LGTM!Also applies to: 159-160, 199-200, 972-973, 1016-1044
src/services/api/openaiShim.ts (6)
2471-2471: LGTM!
2547-2553: LGTM!
2516-2534: LGTM!
2284-2290: LGTM!
2529-2533:normalizeZaiReasoningEffortis correctly implemented—mapslow,medium, andhighto'high', andxhighto'max'. No issues found.
518-523: GitHub credential precedence change is intentional support for GitHub Enterprise authentication.The credential chain (GITHUB_COPILOT_KEY → OPENAI_API_KEY → GITHUB_TOKEN → GH_TOKEN) aligns with documented behavior in
src/integrations/gateways/github-enterprise.tsand is extensively tested inopenaiShim.test.ts. The change ensures direct Copilot API keys take priority when available, which is the expected pattern for GitHub Enterprise. No split needed—this is part of the broader GitHub integration work.src/utils/providerProfiles.test.ts (1)
346-479: LGTM!Also applies to: 1190-1218, 1261-1270
.env.example (1)
4-15: LGTM!Also applies to: 143-156, 513-524
README.md (1)
127-129: LGTM!Also applies to: 193-205, 223-289
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🤖 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 `@README.md`:
- Line 108: The note on line 108 overstates the security guarantee by claiming
the `/provider` command "securely stores credentials" when the actual storage
mechanism is the `.openclaude-profile.json` file, which may not be encrypted or
keychain-backed. Replace the phrase "securely stores credentials" with a more
accurate description that specifies the credentials are persisted to
`.openclaude-profile.json` and avoid implying encryption or advanced security
measures unless that protection is actually implemented.
In `@src/services/api/providerConfig.ts`:
- Around line 822-841: The gheCopilotBaseUrl variable is only derived from
githubEnterpriseEnvUrl (gheUrl), but when isGithubGhe is true due to rawBaseUrl
being set and githubEnterpriseEnvUrl is unset, gheCopilotBaseUrl remains
undefined. This causes the base URL rewrite at lines 893-900 to fail. Fix this
by also checking if rawBaseUrl is a GHE host when gheUrl is undefined, and use
buildGithubEnterpriseCopilotBaseUrl with rawBaseUrl in that case to ensure
gheCopilotBaseUrl is properly set. Additionally, add a regression test in
src/services/api/providerConfig.test.ts covering the scenario where GitHub mode
is enabled with a GHE OPENAI_BASE_URL but no GITHUB_ENTERPRISE_URL, asserting
that the final baseUrl includes the .../api/copilot path.
🪄 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 Plus
Run ID: 72522fba-4818-469c-9263-f3548decbb7c
📒 Files selected for processing (6)
.env.exampleREADME.mdsrc/services/api/openaiShim.test.tssrc/services/api/openaiShim.tssrc/services/api/providerConfig.tssrc/utils/providerProfiles.test.ts
📜 Review details
🔇 Additional comments (10)
src/services/api/openaiShim.test.ts (1)
21-22: LGTM!Also applies to: 159-160, 199-200, 972-973, 1016-1044
src/services/api/openaiShim.ts (6)
2471-2471: LGTM!
2547-2553: LGTM!
2516-2534: LGTM!
2284-2290: LGTM!
2529-2533:normalizeZaiReasoningEffortis correctly implemented—mapslow,medium, andhighto'high', andxhighto'max'. No issues found.
518-523: GitHub credential precedence change is intentional support for GitHub Enterprise authentication.The credential chain (GITHUB_COPILOT_KEY → OPENAI_API_KEY → GITHUB_TOKEN → GH_TOKEN) aligns with documented behavior in
src/integrations/gateways/github-enterprise.tsand is extensively tested inopenaiShim.test.ts. The change ensures direct Copilot API keys take priority when available, which is the expected pattern for GitHub Enterprise. No split needed—this is part of the broader GitHub integration work.src/utils/providerProfiles.test.ts (1)
346-479: LGTM!Also applies to: 1190-1218, 1261-1270
.env.example (1)
4-15: LGTM!Also applies to: 143-156, 513-524
README.md (1)
127-129: LGTM!Also applies to: 193-205, 223-289
🛑 Comments failed to post (2)
README.md (1)
108-108:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid “securely stores credentials” here.
The profile flow appears to persist provider env values into
.openclaude-profile.json, so this wording overstates the guarantee unless that path is encrypted or keychain-backed. Please rephrase to describe the storage location instead. (gitlawb.com)🤖 Prompt for 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. In `@README.md` at line 108, The note on line 108 overstates the security guarantee by claiming the `/provider` command "securely stores credentials" when the actual storage mechanism is the `.openclaude-profile.json` file, which may not be encrypted or keychain-backed. Replace the phrase "securely stores credentials" with a more accurate description that specifies the credentials are persisted to `.openclaude-profile.json` and avoid implying encryption or advanced security measures unless that protection is actually implemented.src/services/api/providerConfig.ts (1)
822-841:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winGHE base URL rewrite is skipped when only
OPENAI_BASE_URLis set.At Line 839,
gheCopilotBaseUrlis derived only fromgithubEnterpriseEnvUrl. IfrawBaseUrlis already a GHE host andGITHUB_ENTERPRISE_URLis unset,isGithubGhebecomes true but Line 893 cannot rewrite to.../api/copilot, so requests can be sent to the wrong base path.Suggested fix
- const gheUrl = githubEnterpriseEnvUrl + const gheUrl = getGithubEnterpriseUrl(rawBaseUrl) const githubEndpointType = isGithubMode ? (gheUrl && !rawBaseUrl ? 'ghe' : getGithubEndpointType(rawBaseUrl, { githubEnterpriseUrl: gheUrl })) : 'custom' @@ - const gheCopilotBaseUrl = gheUrl + const gheCopilotBaseUrl = gheUrl ? buildGithubEnterpriseCopilotBaseUrl(gheUrl) : undefinedPlease also add a focused regression in
src/services/api/providerConfig.test.tsfor: GitHub mode + GHEOPENAI_BASE_URLset + noGITHUB_ENTERPRISE_URL, assertingbaseUrlresolves to.../api/copilot.As per coding guidelines, “Review provider routing, model selection, env precedence … with high scrutiny” and “Add or update tests when the change affects behavior.”
Also applies to: 893-900
🤖 Prompt for 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. In `@src/services/api/providerConfig.ts` around lines 822 - 841, The gheCopilotBaseUrl variable is only derived from githubEnterpriseEnvUrl (gheUrl), but when isGithubGhe is true due to rawBaseUrl being set and githubEnterpriseEnvUrl is unset, gheCopilotBaseUrl remains undefined. This causes the base URL rewrite at lines 893-900 to fail. Fix this by also checking if rawBaseUrl is a GHE host when gheUrl is undefined, and use buildGithubEnterpriseCopilotBaseUrl with rawBaseUrl in that case to ensure gheCopilotBaseUrl is properly set. Additionally, add a regression test in src/services/api/providerConfig.test.ts covering the scenario where GitHub mode is enabled with a GHE OPENAI_BASE_URL but no GITHUB_ENTERPRISE_URL, asserting that the final baseUrl includes the .../api/copilot path.Source: Coding guidelines
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I do not see any actionable issues from my review.
@kevincodex1 LGTM
3 participants
Summary
glm-5.2model metadata with 1M context and 131,072 max output tokensglm-5.2the default Z.AI GLM Coding Plan modeltool_stream=truefor streaming tool requestsValidation
bun run integrations:checkbun run typecheckbun test src/integrations/runtimeMetadata.test.ts src/services/api/providerConfig.test.ts src/services/api/openaiShim.test.tsbun run smokeSummary by CodeRabbit
reasoning=high|xhigh,thinking=disabled) with improved request shaping./providerOpenAI-compatible setup, and model-query routing guidance (including.envnot auto-loaded).