feat(sdk): add control request methods for effort, models, usage, context - #6492
Conversation
…text Add 4 control request methods across CLI, Python SDK, and TypeScript SDK: - set_effort: Set reasoning effort tier (low/medium/high/xhigh/max) at runtime via config.setReasoningEffort(), also accept initial effort in initialize payload - get_available_models: Return models available for current auth type via config.getAvailableModels() - get_usage_info: Return usage dashboard data via loadUsageDashboard() with optional range filter (today/week/month/all) - get_context_usage: Add Python SDK method (TS SDK already has it) CLI: Add request types, dispatcher routing, and SystemController handlers with capability flags (can_set_effort, can_get_available_models, can_get_usage_info). TS SDK: Add ControlRequestType enum values, protocol interfaces, QueryOptions.effort field, Zod schema, and Query methods. Python SDK: Add Effort type, validation, protocol TypedDicts, and Query methods.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Thanks for the PR @juhuan! The technical approach looks solid — clean reuse of existing core APIs (normalizeReasoningEffort, loadUsageDashboard) and consistent with the set_model pattern. However, the PR body doesn't follow the required template, which blocks review.
Template — needs update:
- Missing
## Why it's needed— what motivates bundling these four control methods into one PR? Which SDK consumers need them? - Missing
## Reviewer Test Plan— the current## Test planis just a checkbox list. Reviewers need### How to verify(reproduction steps),### Evidence (Before & After), and### Tested on(OS matrix). - Missing
## Risk & Scope - Missing
## Linked Issues
Preliminary observations (full code review after template is fixed):
handleGetUsageInfohas a redundantsignal.abortedcheck inside the try block — it's already checked at method entry.TransportOptions.effortis typed asstringwhileQueryOptions.effortuses the proper literal union'low' | 'medium' | 'high' | 'xhigh' | 'max'— inconsistency worth tightening.- Python
CLIControlGetUsageInfoRequest.rangeisNotRequired[str]— consider aLiteraltype for type-safety, matching the TS SDK.
Please update the PR body to match the template, then we'll continue with the full review. 🔧
中文说明
感谢 @juhuan 的 PR！技术方案看起来不错——干净地复用了已有的 core API（normalizeReasoningEffort、loadUsageDashboard），和 set_model 的模式一致。但是 PR 描述没有遵循模板，需要更新后才能继续审查。
模板——需要更新：
- 缺少
## Why it's needed——为什么要把这四个控制方法合并在一个 PR 里？哪些 SDK 用户需要它们？ - 缺少
## Reviewer Test Plan——当前的## Test plan只是一个 checkbox 列表。审查者需要### How to verify（复现步骤）、### Evidence (Before & After)和### Tested on（操作系统矩阵）。 - 缺少
## Risk & Scope - 缺少
## Linked Issues
初步观察（模板修复后进行完整代码审查）：
handleGetUsageInfo在 try 块内有一个多余的signal.aborted检查——方法入口已经检查过了。TransportOptions.effort类型是string，而QueryOptions.effort用了正确的字面量联合类型'low' | 'medium' | 'high' | 'xhigh' | 'max'——建议统一。- Python
CLIControlGetUsageInfoRequest.range是NotRequired[str]——建议用Literal类型，和 TS SDK 一致。
请更新 PR 描述以匹配模板，然后继续完整审查。🔧
— Qwen Code · qwen3.7-max
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Test coverage gaps [Critical]
No tests were added for the new control-plane handlers or SDK methods:
- CLI:
handleSetEffort,handleGetAvailableModels,handleGetUsageInfo, and the neweffortfield inhandleInitializehave no tests insystemController.test.ts - CLI: No routing tests for
set_effort,get_available_models,get_usage_infoinControlDispatcher.test.ts - TypeScript SDK:
setEffort(),getAvailableModels(),getUsageInfo()have no tests inQuery.test.ts(the existingsetModel()pattern has dedicated tests) - Python SDK:
set_effort(),get_available_models(),get_context_usage(),get_usage_info()have no tests in the query test suite
Please add tests following the existing patterns for similar handlers (e.g., the set_model test in Query.test.ts).
— qwen3.7-max via Qwen Code /review
| } | ||
| } | ||
|
|
||
| if (payload.effort) { |
There was a problem hiding this comment.
[Critical] Inconsistent effort validation: handleInitialize silently swallows invalid effort values (logs a warning, continues), while handleSetEffort (line ~516) throws an error for the same invalid input. An SDK user passing effort: "banana" at init time gets no feedback — the session starts at default effort. The same value passed via setEffort() post-init throws.
Either throw here (matching handleSetEffort) or echo the applied effort back in the initialize response so the SDK can detect the mismatch:
| if (payload.effort) { | |
| if (payload.effort) { | |
| const normalized = normalizeReasoningEffort(payload.effort); | |
| if (normalized) { | |
| this.context.config.setReasoningEffort(normalized); | |
| debugLogger.info( | |
| `[SystemController] Set reasoning effort to: ${normalized}`, | |
| ); | |
| } else { | |
| throw new Error( | |
| `Invalid effort value: ${payload.effort}. Supported: low, medium, high, xhigh, max`, | |
| ); | |
| } | |
| } |
— qwen3.7-max via Qwen Code /review
|
|
||
| if (payload.effort) { | ||
| const normalized = normalizeReasoningEffort(payload.effort); | ||
| if (normalized) { |
There was a problem hiding this comment.
[Critical] setReasoningEffort() is called without a try/catch here, but every other optional-config call in this method (addMcpServers, setSessionSubagents) is wrapped in try/catch. If config.setReasoningEffort() throws internally, the entire session initialization fails — effort is a non-critical user preference and should not prevent session startup.
| if (normalized) { | |
| if (payload.effort) { | |
| const normalized = normalizeReasoningEffort(payload.effort); | |
| if (normalized) { | |
| try { | |
| this.context.config.setReasoningEffort(normalized); | |
| debugLogger.info( | |
| `[SystemController] Set reasoning effort to: ${normalized}`, | |
| ); | |
| } catch (error) { | |
| debugLogger.error( | |
| `[SystemController] Failed to set effort:`, | |
| error, | |
| ); | |
| } | |
| } else { | |
| debugLogger.warn( | |
| `[SystemController] Invalid effort value: ${payload.effort}`, | |
| ); | |
| } | |
| } |
— qwen3.7-max via Qwen Code /review
| throw new Error('Request aborted'); | ||
| } | ||
|
|
||
| try { |
There was a problem hiding this comment.
[Suggestion] Redundant signal.aborted check — this is inside the try block immediately after the method-entry guard at line ~589, with zero async work in between. This inner check is dead code. The post-await check at line ~601 (after loadUsageDashboard) is legitimate and should stay.
Remove this inner check:
| try { | |
| try { | |
| const range = payload.range; |
— qwen3.7-max via Qwen Code /review
| throw new Error('Request aborted'); | ||
| } | ||
|
|
||
| const range = payload.range; |
There was a problem hiding this comment.
[Suggestion] payload.range is passed directly to loadUsageDashboard() without runtime validation. Although the TypeScript type constrains it to 'today' | 'week' | 'month' | 'all', the payload arrives as JSON over stdio where type safety isn't enforced at runtime. Invalid values fall through getTimeRangeBounds's default case to start = new Date(0), silently returning all-time data.
| const range = payload.range; | |
| const VALID_RANGES = new Set(['today', 'week', 'month', 'all']); | |
| const range = payload.range; | |
| if (range !== undefined && !VALID_RANGES.has(range)) { | |
| throw new Error( | |
| `Invalid range: ${range}. Expected one of: today, week, month, all`, | |
| ); | |
| } | |
| const dashboard = await loadUsageDashboard( | |
| range ? { range } : undefined, | |
| ); |
— qwen3.7-max via Qwen Code /review
| * When resume is provided, this should match the resume ID. | ||
| */ | ||
| sessionId?: string; | ||
| effort?: string; |
There was a problem hiding this comment.
[Suggestion] effort on TransportOptions is dead code — ProcessTransport.buildCliArguments() never reads it, and createQuery.ts never propagates it into transport options. Effort flows exclusively through the control-plane initialize request in Query.ts. Additionally, this is typed as string while QueryOptions.effort (line ~460) uses the narrow union 'low' | 'medium' | 'high' | 'xhigh' | 'max'.
Remove the field from TransportOptions (or align the type if CLI-arg transport is planned):
| effort?: string; | |
| sessionId?: string; | |
| }; |
— qwen3.7-max via Qwen Code /review
| await self._ensure_started() | ||
| return await self._send_control_request("mcp_server_status") | ||
|
|
||
| async def set_effort(self, effort: str) -> dict[str, Any] | None: |
There was a problem hiding this comment.
[Suggestion] Setter return type inconsistency: set_effort returns dict[str, Any] | None while the existing set_model returns None. Both SDKs (Python and TypeScript) have this same inconsistency — setModel/set_model discards the response while setEffort/set_effort surfaces it.
Pick one convention and apply it to both setters. If the confirmation payload is useful, update set_model to also return it. Otherwise, discard the response here to match:
| async def set_effort(self, effort: str) -> dict[str, Any] | None: | |
| async def set_effort(self, effort: str) -> None: | |
| await self._ensure_started() | |
| await self._send_control_request("set_effort", {"effort": effort}) |
— qwen3.7-max via Qwen Code /review
| effort: 'low' | 'medium' | 'high' | 'xhigh' | 'max', | ||
| ): Promise<Record<string, unknown> | null> { | ||
| return this.sendControlRequest(ControlRequestType.SET_EFFORT, { effort }); | ||
| } |
There was a problem hiding this comment.
[Suggestion] Same setter return type inconsistency as the Python SDK: setModel() returns Promise<void> while setEffort() returns Promise<Record<string, unknown> | null>. Apply the same convention decision across both SDKs.
— qwen3.7-max via Qwen Code /review
- Fix TransportOptions.effort type to literal union instead of string - Fix Python CLIControlGetUsageInfoRequest.range to Literal type - Remove redundant signal.aborted checks in handleGetUsageInfo - Add Python SDK tests for set_effort, get_available_models, get_context_usage, get_usage_info, effort in initialize - Add TS SDK tests for setEffort, getAvailableModels, getUsageInfo
| await self._ensure_started() | ||
| return await self._send_control_request("mcp_server_status") | ||
|
|
||
| async def set_effort(self, effort: str) -> dict[str, Any] | None: |
There was a problem hiding this comment.
[Suggestion] set_effort accepts effort: str instead of the Effort type alias defined in types.py. Same for get_usage_info(range: str | None) — the valid range values ('today' | 'week' | 'month' | 'all') aren't reflected in the signature. The TS SDK constrains both to literal unions ('low' | 'medium' | ... and 'today' | 'week' | ...), giving IDE users autocomplete and type-checking. Consider using the existing Effort alias and defining a UsageRange alias:
async def set_effort(self, effort: Effort) -> dict[str, Any] | None:— qwen3.7-max via Qwen Code /review
| ? mcpServersForCli | ||
| : undefined, | ||
| agents: this.options.agents, | ||
| effort: this.options.effort, |
There was a problem hiding this comment.
[Suggestion] Missing test: the Python SDK has test_initialize_sends_effort verifying that effort from QueryOptions reaches the initialize control request payload, but the TS SDK has no equivalent test. If this forwarding regresses, no test would catch it.
Add a test that creates a Query with { effort: 'high' } in options and asserts the initialize request contains effort: 'high'.
— qwen3.7-max via Qwen Code /review
|
|
||
| return { | ||
| subtype: 'get_available_models', | ||
| models, |
There was a problem hiding this comment.
[Critical] get_available_models returns raw AvailableModel[] objects which include optional baseUrl and envKey fields (defined in packages/core/src/models/types.ts). These expose internal API proxy URLs (e.g., https://llm-proxy.corp.internal/v1) and custom environment variable names to any SDK client connected over stdio — including third-party or compromised integrations.
The buildControlCapabilities method correctly gates whether this handler is advertised (can_get_available_models checks typeof config.getAvailableModels === 'function'), but the handler itself returns the full model objects without sanitization.
| models, | |
| const models = this.context.config.getAvailableModels().map( | |
| ({ id, label, capabilities, contextWindowSize }) => ({ | |
| id, | |
| label, | |
| capabilities, | |
| contextWindowSize, | |
| }), | |
| ); |
— qwen3.7-max via Qwen Code /review
|
|
||
| try { | ||
| const range = payload.range; | ||
| const dashboard = await loadUsageDashboard(range ? { range } : undefined); |
There was a problem hiding this comment.
[Suggestion] loadUsageDashboard() is called on every request without caching. The function's JSDoc explicitly warns: "The load can be I/O heavy on large histories, so callers should cache." The daemon route mentioned in the docs does cache the loaded records, but this handler does not.
For long-running SDK sessions or tools that poll usage info, this causes repeated disk I/O and JSON parsing of the entire usage history file (~/.qwen/usage.jsonl).
Consider adding a TTL-based cache at the controller level:
private usageCache?: { timestamp: number; data: UsageDashboard; range?: string };
private readonly USAGE_CACHE_TTL_MS = 60_000;— qwen3.7-max via Qwen Code /review
| const dashboard = await loadUsageDashboard(range ? { range } : undefined); | ||
|
|
||
| return { | ||
| subtype: 'get_usage_info', |
There was a problem hiding this comment.
[Suggestion] handleGetUsageInfo returns { subtype: 'get_usage_info', ...dashboard }, spreading the entire UsageDashboard object (including heatmap, heatmapDays, generatedAt, skills, daily, etc.) directly into the response. This has two issues:
- Fragile API contract: Any future field added to
UsageDashboardautomatically leaks to SDK consumers without review. The response shape is implicitly coupled to the internal dashboard type. - Inconsistent with
handleGetAvailableModels: That handler wraps data under amodelskey ({ subtype: ..., models }), while this one flattens everything. SDK consumers iterating over both response types encounter amodelskey with different semantics (AvailableModel[]vsUsageModelShare[]).
Consider wrapping the dashboard under a typed key (e.g., { subtype: 'get_usage_info', dashboard }) or explicitly selecting fields to return.
— qwen3.7-max via Qwen Code /review
| "get_context_usage", {"show_details": show_details} | ||
| ) | ||
|
|
||
| async def get_usage_info(self, range: str | None = None) -> dict[str, Any] | None: |
There was a problem hiding this comment.
[Suggestion] Two issues with this method signature:
rangeshadows Python's built-inrange()— triggersruffrule A002 andpylint: redefined-builtin. Any future code inside this method that needsrange()(e.g., a loop) would silently get the parameter value instead.- No client-side validation — unlike
set_effortwhich validates against_VALID_EFFORTS,rangeaccepts anystrwithout checking against the allowed values ("today","week","month","all"). Invalid values pass through to the CLI where they silently fall back to all-time data.
| async def get_usage_info(self, range: str | None = None) -> dict[str, Any] | None: | |
| async def get_usage_info(self, time_range: Literal["today", "week", "month", "all"] | None = None) -> dict[str, Any] | None: | |
| await self._ensure_started() | |
| data: dict[str, Any] = {} | |
| if time_range is not None: | |
| data["range"] = time_range | |
| return await self._send_control_request("get_usage_info", data) |
— qwen3.7-max via Qwen Code /review
- Remove dead effort field from TransportOptions (flows via QueryOptions) - Throw on invalid effort in handleInitialize (matching handleSetEffort) - Wrap setReasoningEffort in try/catch (matching addMcpServers pattern) - Sanitize get_available_models response to exclude baseUrl/envKey - Rename Python get_usage_info(range=) to time_range= to avoid shadowing built-in - Use Effort type alias and Literal type in Python set_effort/get_usage_info - Make set_effort/setEffort return None/void to match set_model/setModel - Add TS test for effort in initialize payload
| ); | ||
|
|
||
| return { | ||
| subtype: 'set_effort', |
There was a problem hiding this comment.
[Critical] setReasoningEffort() is a no-op when thinking is disabled (reasoning: false) — it returns silently without applying the effort (see config.ts:3270). This handler returns { effort: normalized } claiming success, but the effort was never actually set. The existing /effort command handles this correctly by reading back config.getReasoningEffort() after the call.
| subtype: 'set_effort', | |
| this.context.config.setReasoningEffort(normalized); | |
| const applied = this.context.config.getReasoningEffort() === normalized; | |
| debugLogger.info( | |
| `[SystemController] Reasoning effort set to: ${normalized} (applied: ${applied})`, | |
| ); | |
| return { | |
| subtype: 'set_effort', | |
| effort: normalized, | |
| applied, | |
| }; |
— qwen3.7-max via Qwen Code /review
| if (payload.effort) { | ||
| const normalized = normalizeReasoningEffort(payload.effort); | ||
| if (normalized) { | ||
| try { |
There was a problem hiding this comment.
[Critical] Same silent no-op issue as handleSetEffort: setReasoningEffort() is a no-op when thinking is disabled, but the initialize response still reports success with full capabilities. An SDK consumer passing effort: "max" in QueryOptions has no feedback that the effort was silently dropped.
Add a read-back check after the call:
this.context.config.setReasoningEffort(normalized);
if (this.context.config.getReasoningEffort() !== normalized) {
debugLogger.warn(
`[SystemController] Effort '${normalized}' was not applied (thinking may be disabled)`,
);
}Consider surfacing this in the initialize response (e.g., effort_applied: boolean) so the SDK can relay the status.
— qwen3.7-max via Qwen Code /review
| `[SystemController] Set reasoning effort to: ${normalized}`, | ||
| ); | ||
| } catch (error) { | ||
| debugLogger.error( |
There was a problem hiding this comment.
[Suggestion] Error handling inconsistency: this try/catch silently swallows setReasoningEffort errors during initialization, but handleSetEffort (line ~530) re-throws the same operation's errors. The same operation has two different error-reporting contracts depending on the call path. Pick one approach consistently — either both swallow or both propagate.
— qwen3.7-max via Qwen Code /review
| /** | ||
| * Handle set_effort request | ||
| * | ||
| * Sets the reasoning effort tier at runtime. |
There was a problem hiding this comment.
[Suggestion] All three new CLI handlers (handleSetEffort, handleGetAvailableModels, handleGetUsageInfo), the payload.effort block in handleInitialize, and the three new ControlDispatcher routing cases have zero CLI-level unit tests. The existing systemController.test.ts and ControlDispatcher.test.ts cover the analogous set_model, get_context_usage, and initialize paths — the new handlers should follow the same pattern.
Key untested paths:
handleSetEffort: valid/invalid/empty effort values,setReasoningEfforterrorhandleGetAvailableModels: happy path and.map()destructuringhandleGetUsageInfo:rangepresent vs absent,loadUsageDashboarderrorhandleInitializeeffort block: all three branches (success, catch, throw)buildControlCapabilities: newcan_set_effort,can_get_available_models,can_get_usage_infofieldsControlDispatcher: routing forset_effort,get_available_models,get_usage_info
— qwen3.7-max via Qwen Code /review
| const normalized = normalizeReasoningEffort(effort); | ||
| if (!normalized) { | ||
| throw new Error( | ||
| `Invalid effort value: ${effort}. Supported: low, medium, high, xhigh, max`, |
There was a problem hiding this comment.
[Suggestion] Raw user input (effort) is reflected verbatim into the error message. The sibling handleSetModel uses a static message ('Invalid model specified for set_model request') that does not echo user input. Consider using the same convention to avoid potential issues if downstream consumers render error messages without escaping:
| `Invalid effort value: ${effort}. Supported: low, medium, high, xhigh, max`, | |
| throw new Error( | |
| `Invalid effort value. Supported: low, medium, high, xhigh, max`, | |
| ); |
Apply the same fix at the initialize handler (line ~291).
— qwen3.7-max via Qwen Code /review
- Add read-back check after setReasoningEffort() in both handleSetEffort and handleInitialize to detect silent no-op when thinking is disabled - Return applied flag in set_effort response so SDK consumers can detect when effort was not applied - Remove user input from error messages to match handleSetModel convention - Add unit tests for set_effort, get_available_models, get_usage_info, and initialize with effort handlers
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
| * | ||
| * @param effort - One of 'low', 'medium', 'high', 'xhigh', 'max' | ||
| * @throws Error if query is closed or effort is invalid | ||
| */ |
There was a problem hiding this comment.
[Critical] setEffort() returns Promise<void>, discarding the CLI's response which includes an applied: boolean field indicating whether the effort change actually took effect. When applied is false (e.g., thinking is disabled for the active model/provider), the caller has no way to detect that their effort change was silently ignored.
Note: getAvailableModels() and getUsageInfo() in the same PR correctly return the control response.
| */ | |
| ): Promise<Record<string, unknown> | null> { | |
| return this.sendControlRequest(ControlRequestType.SET_EFFORT, { effort }); |
— qwen3.7-max via Qwen Code /review
| await self._ensure_started() | ||
| return await self._send_control_request("mcp_server_status") | ||
|
|
||
| async def set_effort(self, effort: Effort) -> None: |
There was a problem hiding this comment.
[Critical] set_effort() discards the control response, which includes an applied: boolean field. When applied is false (e.g., thinking disabled), the caller has no signal that the effort change was a no-op.
Note: get_available_models() and get_usage_info() in the same file correctly return await the response.
| async def set_effort(self, effort: Effort) -> None: | |
| async def set_effort(self, effort: Effort) -> dict[str, Any] | None: | |
| await self._ensure_started() | |
| return await self._send_control_request("set_effort", {"effort": effort}) |
— qwen3.7-max via Qwen Code /review
| } | ||
| } | ||
|
|
||
| if (payload.effort) { |
There was a problem hiding this comment.
[Critical] Effort validation runs here after config-mutating side effects (addMcpServers at ~line 248, setSessionSubagents at ~line 264). If normalizeReasoningEffort returns null and this block throws, MCP servers and subagents have already been added to config. The session is left in a partially-initialized state with no rollback path. A retry would re-add MCP servers on top of the already-added ones.
Move this validation block before addMcpServers so invalid effort fails fast, before any config mutations.
— qwen3.7-max via Qwen Code /review
| throw new Error(errorMessage); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
[Suggestion] Spread order risk: { subtype: 'get_usage_info', ...dashboard } — if UsageDashboard ever gains a subtype property, it would silently overwrite the response discriminator used by the control-plane router. Reverse the order so the explicit subtype always wins: { ...dashboard, subtype: 'get_usage_info' }.
— qwen3.7-max via Qwen Code /review
| } | ||
|
|
||
| /** | ||
| * Set the reasoning effort tier at runtime. |
There was a problem hiding this comment.
[Suggestion] JSDoc claims @throws Error if ... effort is invalid but there is no client-side validation — invalid effort values are sent over the wire and only rejected by the CLI after a round-trip. Either add client-side validation or update the JSDoc to reflect the actual behavior.
— qwen3.7-max via Qwen Code /review
… before side effects - Move effort validation in handleInitialize before MCP servers and subagents processing to prevent config-mutating side effects if effort is invalid - Return applied boolean from setEffort/set_effort so SDK consumers can detect when effort was a no-op (thinking disabled) - Fix spread order in handleGetUsageInfo to prevent subtype overwrite - Update JSDoc to reflect actual behavior
| try { | ||
| const range = payload.range; | ||
| const dashboard = await loadUsageDashboard(range ? { range } : undefined); | ||
|
|
There was a problem hiding this comment.
[Suggestion] Missing post-await signal.aborted check. The sibling handleGetContextUsage checks signal.aborted after its async call (await mod.collectContextData()), but this handler does not check after await loadUsageDashboard(). If the signal is aborted during the dashboard load, stale data is returned instead of throwing 'Request aborted'.
| const dashboard = await loadUsageDashboard(range ? { range } : undefined); | |
| if (signal.aborted) { | |
| throw new Error('Request aborted'); | |
| } | |
| return { |
— qwen3.7-max via Qwen Code /review
| * @returns `true` if the effort was applied, `false` if it was a no-op (e.g. thinking disabled) | ||
| */ | ||
| async setEffort( | ||
| effort: 'low' | 'medium' | 'high' | 'xhigh' | 'max', |
There was a problem hiding this comment.
[Suggestion] The effort literal union 'low' | 'medium' | 'high' | 'xhigh' | 'max' is repeated inline in 3 places (Query.ts:994, types.ts:460, queryOptionsSchema.ts:181) instead of being extracted to a named type alias. The Python SDK correctly defines Effort: TypeAlias = Literal[...] in types.py and reuses it. The TypeScript SDK's own convention uses named aliases (e.g., PermissionMode).
| effort: 'low' | 'medium' | 'high' | 'xhigh' | 'max', | |
| effort: Effort, |
Add export type Effort = 'low' | 'medium' | 'high' | 'xhigh' | 'max'; in types.ts and reference it in all three locations.
— qwen3.7-max via Qwen Code /review
| can_set_permission_mode: | ||
| typeof this.context.config.setApprovalMode === 'function', | ||
| can_set_model: typeof this.context.config.setModel === 'function', | ||
| can_set_effort: |
There was a problem hiding this comment.
[Suggestion] Three new capability flags (can_set_effort, can_get_available_models, can_get_usage_info) are added here but no test asserts their presence in the initialize response. The existing initialize tests only check subtype and session_id. If a flag is accidentally always false (e.g., a typo in the function-name check), SDK consumers would silently lose feature discoverability.
Consider adding an assertion in an initialize test:
expect(result.capabilities.can_set_effort).toBe(true);
expect(result.capabilities.can_get_available_models).toBe(true);
expect(result.capabilities.can_get_usage_info).toBe(true);— qwen3.7-max via Qwen Code /review
| const errorMessage = | ||
| error instanceof Error ? error.message : 'Failed to get usage info'; | ||
|
|
||
| debugLogger.error('[SystemController] Failed to get usage info:', error); |
There was a problem hiding this comment.
[Suggestion] The error message and debug log omit the range parameter. By contrast, handleSetEffort includes the effort value in its error log (Failed to set effort ${effort}). If loadUsageDashboard fails for a specific range (e.g., 'all' triggers a large load that times out), the error has no indication of which range was involved.
| debugLogger.error('[SystemController] Failed to get usage info:', error); | |
| debugLogger.error(`[SystemController] Failed to get usage info (range=${payload.range ?? 'default'}):`, error); |
— qwen3.7-max via Qwen Code /review
| ), | ||
| ) | ||
| .optional(), | ||
| effort: z.enum(['low', 'medium', 'high', 'xhigh', 'max']).optional(), |
There was a problem hiding this comment.
[Suggestion] The valid effort vocabulary is defined in 3 independent locations that accept different input sets: core/reasoning-effort.ts normalizeReasoningEffort accepts aliases ("med", "x-high", "maximum"), while both SDK validators (this zod enum and Python's _VALID_EFFORTS) only accept the 5 canonical names. SDK users cannot pass aliases that the CLI would accept natively, and the three lists could drift independently when adding new tiers.
Consider exporting a shared constant from core (e.g., REASONING_EFFORT_TIERS) and deriving validators from it, or documenting the intentional divergence at each site.
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
packages/sdk-python/tests/unit/test_query_core.py:568, packages/sdk-typescript/test/unit/Query.test.ts:1290 |
Missing test for set_effort/setEffort returning false (applied=false path). Both SDK tests only cover applied: true → true. The CLI test covers both paths, but the SDK-level conversion is untested. |
Add a test where the control response has "applied": false and assert the method returns False/false. |
packages/cli/src/nonInteractive/control/controllers/systemController.test.ts |
CLI handler error paths not tested. None of the three new handlers have tests where their dependencies (getAvailableModels, loadUsageDashboard, setReasoningEffort) throw. |
Mock each dependency to throw and assert the handler rejects with the expected error message. |
— qwen3.7-max via Qwen Code /review
Verification report — real end-to-end run, not just the unit suitesVerdict: the feature works. All four control methods are correct end-to-end, and I could not find a correctness bug. I did find three things worth a follow-up (one behavioural asymmetry, one missing validation, one performance characteristic) — all suggestion-level, none of them merge blockers. I verified HarnessThree layers, each exercising real code:
Isolated What I confirmedThe headline claim — that
The same script against the Python SDK produced Sending the identical control frames to the base build shows every new subtype is genuinely PR-new, and that
Point by point:
Unit suites: CLI 21 passed (10 pre-existing + 11 new), TS SDK To check the new tests actually guard the new code, I reverted the three implementation files to the pre-PR baseline and kept the PR's test file. All 11 new tests go red, the 10 pre-existing stay green, and restoring returns 21/21:
I also re-checked the earlier review points against Finding 1 —
|
| per-call latency | |
|---|---|
get_available_models (control-plane floor) |
1 ms |
get_usage_info, 3-record history |
1–17 ms |
get_usage_info, 50k-record history |
198–322 ms, every call |
Five consecutive calls never got cheaper, confirming there's no memoisation. The control plane is single-threaded, so this blocks other control requests for the duration. Caching the loaded records and re-running buildUsageDashboard per range — the daemon's approach — would fix it.
Smaller notes
get_available_modelsdrops more than secrets. The allowlist keepsid,label,capabilities,contextWindowSizeand discardsdescription,modalities,isVision,authType. A provider entry declaringdescription: "supports images and video"andmodalities: {image: true, video: true}returns neither, so an SDK consumer enumerating models can't tell which ones accept images. Allowlisting is the right call; the list just looks narrower than the use case. All four dropped fields are non-sensitive.- The CLI accepts effort values both SDKs reject.
normalizeReasoningEffortis case- and separator-insensitive with aliases, so"x-high"→xhighand"MAXIMUM"→maxare accepted over the wire, while the Zod enum and_VALID_EFFORTSreject them. Harmless — the SDK is the stricter layer — but the wire contract is wider than the typings suggest. - The three new capability flags have no consumer.
ProcessTransport'sinitialize()discards the response, so neither SDK readscan_set_effortet al.; callingsetEffort()against an older CLI surfaces a rawUnknown control request subtype: set_effort. This matches the existingcan_set_modelconvention, so it's not a regression from this PR. initializewith an invalid effort throws, and the session stays usable afterwards (get_available_modelsstill answers). The validation now runs before the MCP-server and subagent registration, as intended — thoughsetSdkMode(true)and thecanUseTooltimeout are still applied before the throw. Both are idempotent, so this is cosmetic.- The PR currently shows
CHANGES_REQUESTED/BLOCKED, but those bot reviews predate the test-coverage commit (1e7c075) and the fixes above.mergeable: MERGEABLE.
Repro
git worktree add --detach /tmp/pr6492 <pr-head>
git worktree add --detach /tmp/base 3d1122d28
(cd /tmp/pr6492 && npm ci) ; (cd /tmp/base && npm ci)
# mock model endpoint that records request bodies, then:
node e2e-raw.mjs <cli>/packages/cli/dist/index.js $QWEN_HOME $WS # raw control frames, both builds
node e2e-sdk.mjs ... # TS SDK -> real CLI -> wire assertions
python e2e_sdk.py ... # Python SDK, same
node e2e-range.mjs ... # range table aboveQWEN_HOME isolated; settings.json declares modelProviders.openai[] with envKey → a secret env var, one entry with generationConfig.reasoning: false to drive the applied: false path.
Overall
Additive, backward-compatible, well tested, and the behaviour matches the description under real conditions. I'd merge it. Finding 2 (range validation) is the one I'd most like to see before merge since it silently returns wrong data; Findings 1 and 3 are fine as follow-ups.
中文版（合并参考）
验证报告 —— 真实端到端运行，而非仅跑单测
结论：功能是对的。四个控制方法端到端全部正确，我没有找到正确性 bug。 发现三处值得跟进的问题（一处行为不对称、一处缺失校验、一处性能特征），均为建议级，都不构成合并阻塞。
我用 42e791bd5 对比基线 3d1122d28（本 PR 首个 commit 的父提交 —— 本地 main 已严重偏离，git merge-base 在这里不是正确基线）。两个独立 worktree，各自执行真实 npm ci（没有软链 node_modules），驱动真正构建出来的 packages/cli/dist/index.js。
测试手法
三层，每层都跑真实代码：
- 裸控制协议 —— 完全按
ProcessTransport的方式拉起 CLI（--input-format stream-json --output-format stream-json --channel=SDK），手写control_request帧。两个构建上跑同一套帧，这正是 red/green 有意义的前提。 - 真实 SDK —— 构建后的
@qwen-code/sdkquery()与 Pythonqwen_code_sdk.query()，各自拉起真实 CLI 子进程。 - Mock OpenAI 兼容端点，记录每个
/chat/completions请求体，从而检查模型实际收到了什么，而不是只信任 config 对象。
隔离的 QWEN_HOME，配置两个 modelProviders.openai 条目：mock-thinker（开启思考）与 mock-nothink（generationConfig.reasoning: false），两者都带真实 baseUrl 和指向密钥环境变量的 envKey。
已确认的部分
核心主张 —— effort 真的送达模型 —— 在两个 SDK 上都在 wire 层得到验证。第 1 轮没有 reasoning 字段；setEffort('xhigh') 返回 applied: true 之后，第 2 轮发往模型的 HTTP body 携带 reasoning: {"effort":"xhigh"}，会话其他条件完全不变。
Python SDK 同样：set_effort("max") → True，第 2 轮 wire 上出现 reasoning: {"effort":"max"}；get_context_usage(show_details=True) 正确回显 showDetails: true。
同样的控制帧发给基线构建，可见三个新 subtype 确实是本 PR 新增的，且 get_context_usage 本就存在（这里新增的只是 Python 绑定）。
逐条核对：
| 主张 | 结果 |
|---|---|
QueryOptions.effort 在会话启动时生效 |
✅ 第 1 轮请求即携带 reasoning:{effort:"high"}（TS + Python） |
setEffort() / set_effort() 运行时生效 |
✅ applied: true，且下一轮 wire body 携带新档位 |
思考被禁用时返回 applied: false |
✅ 用真实 reasoning: false 模型复现，不只是 mock 读回 |
get_available_models 隐藏 baseUrl / envKey |
✅ 环境变量与 settings 中的真实密钥在任何响应里都未出现 |
get_usage_info 返回实时数据 |
✅ summary.requests 统计到了本会话自己的模型调用 |
dashboard 展开后 subtype 未被覆盖 |
✅ {...dashboard, subtype}，响应判别字段完好 |
get_context_usage（Python 新绑定） |
✅ 含 show_details |
| 非法 effort 在 spawn 之前被拒 | ✅ Zod：Invalid QueryOptions: effort: ...；Python：ValidationError（QueryOptionsDict 形式同样拦截） |
| query 关闭后调用被拒 | ✅ 三个新方法均报 Query is closed |
单测：CLI 21 通过（10 旧 + 11 新），TS SDK Query.test.ts 59 通过，Python 17 + 24 通过。PR 描述里写的 TS SDK 58 个，现在实际是 59 个。
为确认新单测确实守住了新代码，我把三个实现文件回退到 pre-PR 基线、保留 PR 的测试文件：11 个新测试全红，10 个旧测试全绿，恢复后 21/21 通过。
我还逐条复核了此前 review 的意见，以下在 42e791bd5 上已修复：baseUrl/envKey 泄露、setEffort/set_effort 丢弃 applied、{subtype, ...dashboard} 展开顺序、TransportOptions.effort 死字段、Python range= 遮蔽内建函数。
问题 1 —— QueryOptions.effort 静默空操作，而 setEffort() 会上报
本 PR 给 handleSetEffort 和 handleInitialize 都加了读回校验，但只有 set_effort 把结果透出。handleInitialize 里结果只落到 debugLogger.warn，SDK 调用方永远看不到。
在 generationConfig.reasoning: false 的模型上，query({ options: { effort: 'high' } }) 正常跑完 —— 没有报错，initialize 响应里没有 applied 字段 —— 而模型请求里根本没有 reasoning。对同一会话调用 setEffort('high') 却正确返回 applied: false。
该值在 systemController.ts:181 已经算出来了，建议以 effort_applied 之类的字段放进 initialize 响应，让两条路径具备同等可观测性。相关地，handleInitialize 用 try/catch 吞掉了 setReasoningEffort() 的异常，而 handleSetEffort 会重抛 —— 同一个操作，两套失败契约。
问题 2 —— get_usage_info 完全不校验 range
payload.range 直接进 loadUsageDashboard()。getTimeRangeBounds() 的 default: 分支把 start 设为 new Date(0)，所以任何无法识别的 range 都静默等价于 "all time"，并且响应还把这个非法字符串原样回显，仿佛已经生效。
我在使用历史里种入一条 60 天前的记录（7,700 tokens）加上今天的流量（84 tokens）："7d"（调用方想表达"一周"时很自然的笔误）返回的是全时段数据，标签写着 "7d"；"ALL" 同理，因为 switch 区分大小写。同一个 PR 里新增的兄弟 handler set_effort 是会校验并抛错的，这里也应如此。两个 SDK 的类型确实约束了 range，但 TypedDict / TS 字面量联合并非运行时防护 —— 任何非 TS 的协议客户端，或 as any，都能打到这里。
仿照 handleSetEffort 加三行即可：
const VALID_RANGES = ['today', 'week', 'month', 'all'] as const;
if (range && !VALID_RANGES.includes(range)) {
throw new Error('Invalid range value. Supported: today, week, month, all');
}问题 3 —— loadUsageDashboard() 每次请求都无缓存地重新加载
它自己的 JSDoc 就写着 "The load can be I/O heavy on large histories, so callers should cache"，daemon 路由也确实做了缓存。而这个 handler 每次调用都重新读取并解析整个历史文件。在 50,000 条记录 / 26 MB 的 usage_record.jsonl 上实测：
| 单次调用耗时 | |
|---|---|
get_available_models（控制面基准） |
1 ms |
get_usage_info，3 条历史 |
1–17 ms |
get_usage_info，5 万条历史 |
198–322 ms，每一次都是 |
连续 5 次调用没有变快，确认没有任何 memoisation。控制面是单线程的，这段时间会阻塞其他控制请求。按 daemon 的做法缓存已加载的记录、按 range 重新 buildUsageDashboard 即可解决。
其他小点
get_available_models丢掉的不止是密钥。 白名单保留id、label、capabilities、contextWindowSize，丢弃了description、modalities、isVision、authType。一个声明了description: "supports images and video"和modalities: {image: true, video: true}的 provider 条目，两者都拿不到，SDK 调用方枚举模型时无法判断哪些支持图片。用白名单是对的，只是这个名单相对使用场景显得窄了些 —— 被丢弃的这四个字段都不敏感。- CLI 接受两个 SDK 都拒绝的 effort 取值。
normalizeReasoningEffort对大小写和分隔符不敏感且支持别名，所以"x-high"→xhigh、"MAXIMUM"→max在协议层是被接受的，而 Zod enum 与_VALID_EFFORTS会拒绝它们。无害（SDK 是更严格的那层），但 wire 契约比类型声明更宽。 - 三个新 capability flag 没有任何消费方。
ProcessTransport的initialize()丢弃了响应，所以两个 SDK 都不会读can_set_effort等；对着旧版 CLI 调用setEffort()只会抛出裸的Unknown control request subtype: set_effort。这与既有的can_set_model惯例一致，因此不算本 PR 引入的回归。 initialize传入非法 effort 会抛错，且会话之后仍可用（get_available_models仍能应答）。effort 校验现在确实跑在 MCP server 与 subagent 注册之前 —— 不过setSdkMode(true)和canUseTool超时仍在抛错之前应用。二者都是幂等的，所以只是观感问题。- PR 当前显示
CHANGES_REQUESTED/BLOCKED，但那些 bot review 早于测试覆盖那个 commit（1e7c075）以及上述修复。mergeable: MERGEABLE。
总体
改动是纯增量、向后兼容、测试充分，真实条件下的行为与描述一致。我倾向合并。 其中问题 2（range 校验）我最希望在合并前处理，因为它会静默返回错误数据；问题 1 和问题 3 作为后续跟进即可。
|
@qwen-code /triage |
|
Thanks for the PR! Template looks good ✓ (minor: 中文说明 details section and the Tested on table are missing, but all substantive sections are present) Problem: This is a feature addition, not a bug fix. The SDK lacks programmatic access to CLI capabilities (effort control, model listing, usage dashboard, context usage) that already exist as interactive commands. The gap was identified in the SDK requirements analysis and this PR consolidates 4 previously closed PRs into one. Direction: Clearly aligned. SDK consumers need to control reasoning effort at runtime, enumerate available models, and query usage data — these are table-stakes for a programmatic SDK. The CLI already supports all of these internally; the PR exposes them through the control plane. No product-direction concerns. Size: ~480 production lines across 3 packages (CLI control handlers + Python SDK + TypeScript SDK), ~410 lines of tests. No core paths touched. Well within normal scope. Approach: The scope feels right — four related control methods grouped into a single PR is cleaner than four separate ones, and they share the same pattern (control request → handler → SDK method). The implementation is purely additive and backward-compatible. One note: the 中文说明感谢贡献！ 模板完整 ✓（小瑕疵：缺少中文说明 details 和 Tested on 表格，但所有实质性章节都有） 问题：这是一个功能新增而非 bug 修复。SDK 缺少对 CLI 已有能力的编程访问（effort 控制、模型列表、用量面板、上下文用量），这些在 SDK 需求分析中已被识别，本 PR 将 4 个此前关闭的 PR 合并为一个。 方向：明确对齐。SDK 调用方需要运行时控制 reasoning effort、枚举可用模型、查询用量数据——这些是编程 SDK 的基本能力。CLI 内部已支持所有这些功能；本 PR 通过控制面暴露它们。无产品方向问题。 规模：~480 行生产代码，跨 3 个包（CLI 控制处理器 + Python SDK + TypeScript SDK），~410 行测试。未触及核心路径。完全在正常范围内。 方案：范围合理——四个相关控制方法放在一个 PR 比拆成四个更清晰，它们共享同一模式（控制请求 → 处理器 → SDK 方法）。改动纯增量、向后兼容。注意： — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal (before reading diff): I'd add new control request subtypes to the CLI's SystemController, wire them through ControlDispatcher, add protocol types and SDK methods on the Query class for both SDKs, validate effort values at both SDK and CLI layers, and filter secrets from the models response. Tests would cover each handler path and SDK method. Comparison: The PR's approach matches this exactly. The implementation is clean and well-structured — handler methods follow the existing pattern in SystemController, SDK methods are thin wrappers around Findings: No critical blockers. The code is additive, backward-compatible, and follows project conventions. Two notes (both previously flagged by the collaborator's verification):
Reuse check: The PR correctly imports and reuses Test ResultsTmux TestingN/A — this PR adds SDK programmatic APIs (control request methods), not TUI-visible features. The collaborator @wenshao already ran thorough real E2E verification (spawning the actual CLI, driving both SDKs, checking wire-level model request bodies, red/green test coverage validation). Their verification report in the comments above is comprehensive and covers the scenarios tmux testing would target. 中文说明代码审查独立方案（读 diff 之前）： 我会在 CLI 的 SystemController 中添加新的控制请求子类型，通过 ControlDispatcher 路由，为两个 SDK 的 Query 类添加协议类型和 SDK 方法，在 SDK 和 CLI 两层校验 effort 值，并通过白名单过滤模型响应中的敏感信息。测试覆盖每个处理器路径和 SDK 方法。 对比： PR 的方案与此完全一致。实现干净且结构良好——处理器方法遵循 SystemController 的既有模式，SDK 方法是 发现： 无阻塞性问题。代码是增量的、向后兼容的，并遵循项目约定。两个注意事项（均已被协作者的验证报告指出）：
复用检查： PR 正确从 core 导入并复用了 测试结果
Tmux 测试不适用——本 PR 添加的是 SDK 编程 API（控制请求方法），非 TUI 可见功能。协作者 @wenshao 已进行了详尽的真实端到端验证（启动真实 CLI、驱动两个 SDK、检查 wire 级模型请求体、红绿测试覆盖验证），其验证报告已在上方评论中，覆盖了 tmux 测试会针对的场景。 — Qwen Code · qwen3.7-max |
ReflectionThis PR does exactly what it says — exposes four CLI capabilities through the SDK control plane. The implementation is straightforward: thin handler methods in SystemController that delegate to existing core functions, thin SDK methods that delegate to I wrote my independent proposal before reading the diff, and the PR matches it. There's no simpler path — four control methods need four handlers and four SDK methods, and that's what's here. No over-abstraction, no drive-by refactors, no scope creep. The collaborator @wenshao ran one of the most thorough E2E verifications I've seen on a PR — spawning real CLI processes, driving both SDKs, checking wire-level HTTP bodies to the model, testing invalid inputs, measuring performance characteristics, and even reverting implementation files to confirm the new tests actually catch regressions. Their verdict: "I'd merge it." That carries significant weight. The The init effort silent no-op is an observability gap, not a bug. The runtime All 121 unit tests pass. Build and typecheck pass. The code reuses existing core functions ( The existing 中文说明总结本 PR 准确地完成了它所描述的工作——通过 SDK 控制面暴露四个 CLI 能力。实现直接：SystemController 中的薄处理器方法委托给已有的 core 函数，薄 SDK 方法委托给 我在读 diff 之前写了自己的独立方案，PR 与之匹配。没有更简路径——四个控制方法需要四个处理器和四个 SDK 方法，这就是这里的全部内容。没有过度抽象，没有顺手重构，没有范围蔓延。 协作者 @wenshao 进行了我见过的最详尽的端到端验证之一——启动真实 CLI 进程、驱动两个 SDK、检查发往模型的 wire 级 HTTP 请求体、测试非法输入、测量性能特征，甚至回退实现文件确认新测试确实能捕获回归。结论："我会合并。"这具有重要参考价值。
init effort 静默空操作是可观测性缺口而非 bug。 运行时 全部 121 个单元测试通过。Build 和 typecheck 通过。代码复用了已有的 core 函数（ 既有的 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅ The range validation in handleGetUsageInfo is worth a quick follow-up but doesn't block this.
4 participants
What this PR does
Adds 4 control request methods to both Python and TypeScript SDKs, with corresponding CLI control handlers:
set_effort()/setEffort()— Runtime reasoning effort control (low/medium/high/xhigh/max). Also accepted as aneffortoption in QueryOptions for initialization-time configuration.get_available_models()/getAvailableModels()— Query the list of available models from the CLI.get_usage_info()/getUsageInfo(range?)— Query account usage/quota dashboard data, with optional time range filter (today/week/month/all).get_context_usage()(Python only, already existed in TS) — Query context window usage statistics.Changes by package:
CLI (
packages/cli):set_effort,get_available_models,get_usage_infoSystemControllerhandlers usingconfig.setReasoningEffort(),config.getAvailableModels(),loadUsageDashboard()effortfield inCLIControlInitializeRequestfor init-time effort settingcan_set_effort,can_get_available_models,can_get_usage_infoPython SDK (
packages/sdk-python):Efforttype alias (Literal["low", "medium", "high", "xhigh", "max"])effortfield inQueryOptionswith validationQuery:set_effort(),get_available_models(),get_context_usage(),get_usage_info()TypeScript SDK (
packages/sdk-typescript):effortfield inTransportOptionsandQueryOptions(typed as literal union)effortControlRequestTypeenum entries:SET_EFFORT,GET_AVAILABLE_MODELS,GET_USAGE_INFOQuery:setEffort(),getAvailableModels(),getUsageInfo()Why it's needed
These are P1/P2 features identified in the SDK requirements gap analysis (docs/qwen-sdk-requirements.md). The SDK previously had no way to:
/effortcommand)/modelcommand)/contextcommand)Reviewer Test Plan
How to verify
Python SDK:
cd packages/sdk-python python -m pytest tests/unit/test_query_core.py -v python -m pytest tests/unit/test_validation.py -vTypeScript SDK:
cd packages/sdk-typescript npx vitest run test/unit/Query.test.tsEvidence
ruff format --checkpasses on all Python filesTested on
Risk & Scope
effortis optional.Linked Issues
Consolidates closed PRs: #6464 (effort), #6460 (get_available_models), #6473 (get_usage_info), #6471 (get_context_usage)
Related to: SDK requirements gap analysis