fix(openai-compatible): add max reasoning effort option - #1051
Merged
edelauna merged 1 commit intoJul 31, 2026
Merged
Conversation
ivanarifin
requested review from
JamesRobert20,
edelauna,
hannesrudolph,
navedmerchant and
taltas
as code owners
July 29, 2026 06:36
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe OpenAI-compatible settings now supports the extended ChangesOpenAI-compatible reasoning effort
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
github-actions
Bot
added
the
awaiting-review
xavier-arosemena
added a commit
to xavier-arosemena/roo-plus
that referenced
this pull request
Aug 3, 2026
…tocol, slim ClineProvider, upstream sync (#112) * refactor(webview): use canonical provider identifiers (Zoo-Code-Org#1023) * refactor(webview): use canonical provider identifiers * test(webview): remove brittle identifier mutation tests * fix(webview): handle remaining model providers explicitly * test(webview): avoid unreachable coverage branch * refactor(webview): use canonical anthropic identifier * test(webview): cover Anthropic Opus 1M tier * test(webview): use canonical Anthropic identifier * fix(webview): address canonical provider review * test(webview): use canonical provider identifiers * test(webview): cover Kimi Code model selection * refactor: finish canonical provider identifier audit (Zoo-Code-Org#1030) * refactor: finish canonical provider identifier audit * fix: address provider identifier review feedback * refactor: centralize Anthropic protocol value * refactor: reuse canonical provider protocol constants * refactor: centralize OpenAI protocol value * test: use canonical keyless provider identifiers * test: address protocol routing review feedback * refactor(api): centralize service-tier primitives (Zoo-Code-Org#1040) * refactor(api): centralize service-tier primitives * test(openai-native): cover omitted completion tiers * test(openai-native): cover resolved streaming tiers * test(api): cover service tier fallbacks * test(api): cover remaining service tier branches * refactor(api): remove duplicate tier capture * test(openai-native): cover flex service tier * test(openai-native): expand service tier coverage * refactor: address service tier review feedback * refactor(api): address service tier review feedback * test(webview): add model info visual snapshot * fix(webview): isolate visual test dependencies * test(webview): update service tier visual baseline * fix(webview): add @source directive so Tailwind utilities generate in CT builds --------- Co-authored-by: Elliott de Launay <edelauna@gmail.com> * fix(openai-compatible): add max reasoning effort option (Zoo-Code-Org#882) (Zoo-Code-Org#1051) * feat(openai-codex): persist and send Fast priority mode (Zoo-Code-Org#1063) * feat(openai-codex): persist and send Fast priority mode * refactor(openai-codex): align service tier types * fix(readiness): remove stale cloud test, fix floating promises, rebrand webview title * chore: bump custom-modes submodule (user-friendly descriptions) + add roomodes sync guard - Point custom-modes at 029a0d4 (description rewrite + YAML normalization) - Make sync-custom-modes.mjs importable and add scripts/verify-roomodes-sync.mjs which fails when .roomodes is not reproducible from the submodule * feat(protocol): add zod message registry + boundary validation (S1-M1/M2) * feat(protocol): type + validate allowedCommands/deniedCommands (S1-M3 domain 1) - Add commands.ts schemas registered in webviewMessageSchemas (allowedCommands, deniedCommands require a string[]). - Migrate the handler cases to schema-validated typed payloads, dropping the runtime Array.isArray/typeof sanitization now guaranteed by zod. - Boundary now rejects crafted non-array commands payloads (unit + handler + ClineProvider boundary tests). * feat(protocol): type + validate updateSettings (S1-M3 domain 2) - rooCodeSettingsSchema now uses .passthrough() so unknown future settings fields are retained, not stripped. - Add settings.ts updateSettings schema (updatedSettings?: RooCodeSettings) registered in webviewMessageSchemas. - Migrate the updateSettings handler case to the schema-validated payload; malformed known-field types (e.g. non-string terminalProfile) are rejected before side effects. - Boundary + handler tests cover valid dispatch and malformed rejection. * feat(protocol): type + validate provider config messages (S1-M3 domain 3) - Add providerConfig.ts schemas for saveApiConfiguration, upsertApiConfiguration (text + ProviderSettings-passthrough) and setApiConfigPassword, registered in webviewMessageSchemas. - apiConfiguration reuses providerSettingsSchema.passthrough(): key fields (incl. apiProvider enum) validated, provider-specific passthrough fields retained. - Migrate save/upsert handler cases to schema-validated typed payloads, dropping the runtime text/apiConfiguration guards. - setApiConfigPassword remains a no-op but malformed shapes are now rejected at the boundary. * feat(protocol): type + validate marketplace install messages (S1-M3 domain 4) - Add marketplace.ts schemas for installMarketplaceItem, installMarketplaceItems (min 1 item) and installMarketplaceItemWithParameters, registered in webviewMessageSchemas. - mpItem/mpItems reuse marketplaceItemSchema; mpInstallOptions reuses installMarketplaceItemOptionsSchema. - Migrate the three handler cases to schema-validated typed payloads; crafted non-item payloads are rejected at the boundary. * feat(protocol): type + validate chat message queue messages (S1-M3 domain 5) - Add messageQueue.ts schemas for queueMessage (text + images), removeQueuedMessage (text) and editQueuedMessage (payload reused from queuedMessageSchema.pick), registered in webviewMessageSchemas. - Migrate the three handler cases to schema-validated typed payloads, removing the payload-as-EditQueuedMessagePayload cast and the now-unused import. - Boundary + handler tests cover valid dispatch and malformed rejection (non-string text). * feat(protocol): type + validate todo list and custom mode messages (S1-M3 domain 6) - Add customModes.ts schemas for updateTodoList (payload.todos from todoItemSchema), updateCustomMode (slug + modeConfig) and deleteCustomMode (slug + optional checkOnly), registered in webviewMessageSchemas. - Migrate the three handler cases to schema-validated typed payloads; updateTodoList drops the payload-as-any cast (handler no-explicit-any suppression 5 -> 4). - Fix the ClineProvider updateCustomMode boundary test to include the top-level slug, matching the real webview sender contract (do not loosen the schema). - Boundary + handler tests cover valid dispatch and malformed rejection. * test(protocol): guard updateSettings sender compatibility Add parseWebviewMessage assertions for the CLI extension-host initialSettings shape and the webview SettingsView handleSubmit payload (incl. nullable/edge fields) so the updateSettings schema never regresses the real senders. * ci: add message-schema ratchet guard (S1-M4) * refactor(webview): extract shared handler helpers and narrow provider types (S2 scaffold) - Add handlers/shared.ts with getGlobalState/updateGlobalState/getCurrentCwd/resolveIncomingImages extracted verbatim from webviewMessageHandler's pre-switch setup. - Narrow provider params in checkpointRestoreHandler, generateSystemPrompt, worktree/handlers, skillsMessageHandler, rulesMessageHandler to minimal Pick<ClineProvider, ...> types so domain handlers depend only on the members they use (dependency inversion; ClineProvider satisfies each Pick structurally, so callers need no cast). - Exempt core/webview/handlers/*.ts from no-case-declarations (same rule the original dispatcher used for its switch case bodies). - Re-home the 4 no-explicit-any suppressions from webviewMessageHandler.ts to handlers/chat.ts (the moved casts keep their existing suppressions). No behavior change: moved helper bodies are identical and the old dispatcher still compiles against the narrowed signatures. * refactor(webview): add per-domain webview message handler modules (S2) Move every case from webviewMessageHandler's giant switch into domain modules under core/webview/handlers/, each exporting a ReadonlySet<WebviewMessageType> of the types it handles plus a handle<Domain>Messages(provider, marketplaceManager, message) function that switch-dispatches those cases VERBATIM (same bodies, ordering, and error handling). chat.ts 17 types (message edit/delete/confirm, queue, tts, checkpoints, enhancePrompt) task.ts 17 types (new/clear/cancel/condense/export/delete tasks, system prompt, commits, todos) settings.ts 27 types (updateSettings, allowed/deniedCommands, custom modes, prompts, models) providerProfiles.ts 15 types (api config CRUD, pins, provider OAuth sign-in/out, rate limits) mcp.ts 9 types (server lifecycle, tool toggles, timeout, settings) marketplace.ts 7 types (install/remove/filter/fetch, mdm notification) worktree.ts 11 types (list/create/delete/switch worktrees, branches, includes, picker) codeIndex.ts 8 types (settings, indexing lifecycle, secrets, auto-enable) skills.ts 6 types rules.ts 5 types commands.ts 4 types terminal.ts 3 types images.ts 3 types debug.ts 3 types misc.ts 19 types (webviewDidLaunch, import/export, files, search, upsells, preview) Each module declares a minimal Pick<ClineProvider, ...> limited to the members it actually uses. The old dispatcher is untouched in this commit; the router that consumes these modules lands next. No behavior change; cases are byte-for-byte the original statements relocated. * refactor(webview): replace dispatcher switch with thin domain router (S2) webviewMessageHandler.ts shrinks from a 4k-line switch to a ~170-line router: it builds a Map<WebviewMessageType, handler> from each domain module's exported MessageTypes set + handle<Domain>Messages function and delegates by message.type. The exported signature (provider, message, marketplaceManager?) is unchanged, so the boundary (ClineProvider.setWebviewMessageListener) and all spec files pass untouched. Unknown types fall through to the same commented default as before. The 4 no-explicit-any suppressions that moved to handlers/chat.ts in the scaffold commit are now the only re-homed entry; webviewMessageHandler.ts is fully typed (0 any). * refactor(webview): extract TaskHistoryService from ClineProvider (S3a) Move task-history mutation, webview broadcast, debounced globalState write-through, and recent-tasks caching into a focused TaskHistoryService with narrow DI ports. ClineProvider keeps identical public method signatures and delegates to the service. recentTasksCache stays on the provider (owned through a port) so delegation flows and existing tests that read/write the field keep working unchanged. * refactor(webview): extract ProviderProfileService from ClineProvider (S3a) Move provider-profile CRUD, activation, and sticky-profile persistence into a focused ProviderProfileService with narrow DI ports. ClineProvider keeps identical public method signatures and delegates. providerSettingsManager is injected as a getter port (read at call time) so tests that replace the field after construction keep working; updateTaskHistory is forwarded with exact argument arity to preserve spy call signatures. * refactor(webview): extract MarketplaceService from ClineProvider (S3a) Move on-demand marketplace data fetching into a focused MarketplaceService with narrow DI ports. The timeout warning is injected as a port (wired to vscode.window.showWarningMessage in ClineProvider) so the service has no direct vscode dependency and stays unit-testable. ClineProvider keeps the same public fetchMarketplaceData signature and delegates. * feat(core): extract TaskOrchestrator service for task lifecycle and delegation state machine Adds src/core/services/TaskOrchestrator.ts owning the task lifecycle and delegation/subtask state machine previously embedded in ClineProvider (S3b). - TaskOrchestratorDeps: narrow DI ports (S3a pattern) bound to the provider at call time so spies (getState/getGlobalState/updateTaskHistory/getTaskWithId) and post-construction taskRegistry/taskScheduler swaps keep working. - Moves createTask, createTaskWithHistoryItem, cancelTask/cancelTaskInternal, clearTask, resumeTask, addClineToStack, removeClineFromStack, evictCurrentTask, markDelegatedChildInterrupted, delegateParentAndOpenChild, reopenParentFromDelegation, abandonSubtask, and runDelegationTransition. - Behavior preserved byte-for-byte (error messages, assertValidTransition order, instanceId guards, cancelledDelegationChildIds semantics, tool_result injection, flush/retry, log output). - Adds focused unit tests at the narrowest layer (single-open invariant on createTask, cancelTask instanceId guard + rehydrate, delegation parent-metadata persistence + child scheduling, reopenParentFromDelegation tool_result injection, abandonSubtask orphan/transition logic). * refactor(core): slim ClineProvider by delegating task orchestration to TaskOrchestrator ClineProvider now keeps only webview lifecycle, state assembly, settings/misc, and thin delegates. Every moved method remains a public method on ClineProvider with an identical name/signature/return type that delegates to the orchestrator via a lazily-cached static helper (ClineProvider.getTaskOrchestrator(this)), so: - Delegation specs that invoke ClineProvider.prototype.<method>.call(fakeProvider) against plain `this` objects keep working (no reliance on the prototype chain). - vi.spyOn(provider, getState|getGlobalState|updateTaskHistory|getTaskWithId) and post-construction provider.taskRegistry/taskScheduler swaps keep intercepting (all deps are closures read at call time). - handleModeSwitch/showTaskWithId/performPreparationTasks/getTaskWithId stay on the provider and are consumed via narrow ports. Moved methods: addClineToStack, removeClineFromStack, evictCurrentTask, markDelegatedChildInterrupted, createTaskWithHistoryItem, createTask, cancelTask, cancelTaskInternal, clearTask, resumeTask, delegateParentAndOpenChild, reopenParentFromDelegation, abandonSubtask, runDelegationTransition. Also removes now-unused imports and drops the ClineProvider eslint-suppression count from 12 to 6 (no increase; no new suppressions). * X1: add DOMPurify sanitizeHtml primitive for webview dangerouslySetInnerHTML sites - Add dompurify dependency to webview-ui and a strict allowlist sanitizer (sanitizeHtml.ts) using an isolated DOMPurify instance (no global hooks). - TerminalOutput: keep escapeXML pinned to true (exported const + regression test) and sanitize converter output as belt-and-suspenders. - MermaidBlock: set mermaid securityLevel to strict (was loose) so labels are HTML-escaped at the source. - MermaidButton: sanitize the copied SVG HTML before injecting into the zoom modal (child-node rendering would detach the live diagram). - TaskItem: sanitize search-highlight HTML before injecting. - Add sanitizeHtml.spec.ts asserting script/event-handler/javascript: removal, span+color preservation, and SVG path allow-listing with nested script stripping. * X2: tighten HMR CSP and verify the local dev server is Vite before serving HMR HTML - Remove the https://* wildcard from the dev-only HMR CSP (script-src, style-src, connect-src). script-src now allows only https://*.posthog.com (telemetry), the local Vite origins, and the nonce. 'unsafe-eval' is kept (required by Vite HMR/react-refresh) with a comment that it is dev-only and must never appear in getHtmlContent's production CSP. - Harden the localhost probe: after the root reachability check, GET /@vite/client and require a 2xx whose body identifies Vite, otherwise fall back to getHtmlContent (production HTML). A rogue process on :5173 can no longer serve scripts to the webview. Keeps the .vite-port gate and the existing hmr_not_running error message. - Extend ClineProvider.spec.ts with a dev-mode getHMRHtmlContent suite (scoped beforeEach/afterEach reset/restore the axios mock since the outer beforeEach's vi.clearAllMocks does not clear mockImplementationOnce queues): CSP has no bare https://* wildcard, and the vite-identity probe falls back to production HTML when /@vite/client is unreachable or not-Vite. * docs: update changelog, debt log, readme and add typed-message-protocol ADR * [Fix] Subtask e2e suite can inherit a cancelled delayed mock stream from the previous test (Zoo-Code-Org#1074) * test(e2e): drain delayed mock stream deterministically in subtask suite The API-hang subtask fixture used aimock's flat latency, which applies per SSE chunk and is never interrupted by client disconnects, so a cancelled delayed stream stayed pending server-side for chunks x latency and could flush into the next test's traffic. - delay only the first chunk via streamingProfile.ttft so the pending window is exactly the shared SUBTASK_API_HANG_RESPONSE_LATENCY_MS - anchor the post-test drain to the request's aimock journal timestamp and wait out only the remainder of that bounded window * docs(e2e): clarify subtask drain invariants from code review --------- Co-authored-by: Roomote <roomote@roomote.dev> Co-authored-by: Elliott de Launay <edelauna@gmail.com> * fix(router-provider): fetch model metadata before context management decisions (Zoo-Code-Org#1053) * fix(router-provider): fetch model metadata before context management decisions Router providers (zoo-gateway, kimi-code) that are auth-scoped skip the model cache entirely. On a fresh handler instance getModel() falls back to hardcoded defaults (e.g. 200k context window) because the real model list has not been fetched yet. Context management runs before createMessage() which is where fetchModel() normally happens, so condensing/truncation decisions use the wrong context window. Add ensureModelFetched() to RouterProvider that fetches once when the instance model map is empty. Call it in Task before context management so getModel() returns accurate metadata from the API. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(router-provider): single-flight ensureModelFetched and earlier call site Make ensureModelFetched single-flight so concurrent callers share a single in-flight fetch instead of firing duplicates. Move the call site before the cachedStreamingModel snapshot so the model info is accurate from the start of the streaming session, not just for context management. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(router-provider): address review feedback on fetch failures and double-fetch Make fetchModel single-flight and short-circuit once models are loaded so auth-scoped providers do not hit the models endpoint twice per request. Catch ensureModelFetched failures in Task via safeEnsureModelFetched so a metadata fetch error falls back to defaults instead of ending the task. Add reject-then-recover coverage and Task tests for the new call sites. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(test): spy private addToApiConversationHistory via TaskTestAccess vi.spyOn on the private method fails check-types; route it through the existing test access cast like the other private helpers. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> * v3.76.0: Release Readiness & Architecture Program — typed message protocol, slim ClineProvider, upstream sync - Merge upstream/main (7 commits: canonical provider identifiers, router-provider metadata, openai-codex Fast mode, service-tier refactor, e2e subtask fix) - Preserve Zoo Gateway removal from the fork; drop upstream's zoo-gateway references/tests that conflict with removed provider - Typed + runtime-validated webview message protocol (16 security-sensitive types) - Domain-split webview dispatcher and slim ClineProvider (TaskOrchestrator, TaskHistoryService, ProviderProfileService, MarketplaceService) - Semble one-dir EACCES fix, download-only binary, pre-installed mode description merge-fill - CLI event-listener leak fix, legacy credential write path retired, vscode-shim logger wired - Webview HTML sanitization (DOMPurify) + HMR CSP hardening - Fix path-mentions double-escaping regression; remove blank Zoo Gateway test stubs - Update CHANGELOG.md + README.md; bump version to 3.76.0 Closes: #98 Co-authored-by: hanneke-de-vries <dhanneke204@gmail.com> * fix: remediate new CodeQL alerts (port validation, temp files, command race, path escaping) Co-authored-by: hanneke-de-vries <dhanneke204@gmail.com> * fix: remediate 100 pre-existing CodeQL code-scanning alerts - js/file-system-race: drop fs.access pre-checks in favor of direct read-with-error-handling (McpHub, extract-text, ReadFileTool), use readdirSync withFileTypes (find-missing-i18n-key), and exclusive 'wx' writes (bootstrap.mjs) - js/insecure-temporary-file: use fs.mkdtemp private dirs + 0600 modes (diagnosticsHandler, ShadowCheckpointService spec/service) - js/remote-property-injection: validate property keys in ModesView and FileChangesPanel - js/disabling-certificate-validation: expand justification comment for the debug-only TLS override (networkProxy) - js/file-access-to-http / http-to-file-access: sanitize image references (image-generation), validate base64 image payloads (openai-native, openai-codex), validate image bytes before writing (GenerateImageTool), validate OAuth token response schema (qwen-code) - js/indirect-command-line-injection: use spawnSync with args array + editor allowlist (install-vsix) - js/log-injection: sanitize control chars in mock-server URL logs - js/missing-origin-check: add isTrustedMessage origin/source validator and apply it across ~30 webview message handlers Co-authored-by: hanneke-de-vries <dhanneke204@gmail.com> * fix: remediate remaining CodeQL alerts flagged on the branch - ReadFileTool.ts: open a single file handle and stat/read through it to eliminate the stat-then-read (TOCTOU) race in both the native and legacy read paths - FileChangesPanel.tsx + ModesView.tsx: store webview-sourced content by Map key instead of object property to prevent prototype pollution (remote-property-injection) via untrusted paths/slugs - mcp-oauth.test.ts: sanitize the HTTP method as well as the URL in the mock-server log (log-injection) - qwen-code.ts: validate refresh_token as a string primitive before persisting credentials (network-data-to-file) - apps/vscode-e2e/tsconfig.json: use non-deprecated moduleResolution (Node10 + ignoreDeprecations 5.0) and set explicit rootDir - readFileTool.spec.ts: mock fs.open handle so tests exercise the new single-handle read path (no new eslint suppressions) Co-authored-by: hanneke-de-vries <dhanneke204@gmail.com> --------- Co-authored-by: Alexei Gubin <36731953+WebMad@users.noreply.github.com> Co-authored-by: Elliott de Launay <edelauna@gmail.com> Co-authored-by: Ivan Ramadhan Arifin <111653938+ivanarifin@users.noreply.github.com> Co-authored-by: zoomote[bot] <305051434+zoomote[bot]@users.noreply.github.com> Co-authored-by: Roomote <roomote@roomote.dev> Co-authored-by: James Mtendamema <59908268+JamesRobert20@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: hanneke-de-vries <dhanneke204@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Focused tests
cd src && npx vitest run api/providers/__tests__/openai.spec.ts— 1 file / 64 tests passed.cd webview-ui && npx vitest run src/components/settings/providers/__tests__/OpenAICompatible.spec.tsx— 1 file / 11 tests passed.Closes: #882
Summary by CodeRabbit
New Features
Tests