Skip to content

Commit 32ee12e

Browse files
wenshaoclaude
andcommitted
fix: address review feedback and fix critical bugs in context usage feature
- Add missing `get_context_usage` route in ControlDispatcher (SDK calls would throw) - Fix `executionMode` defaulting: use `?? 'interactive'` to match other commands - Validate dynamic import of `collectContextData` before invoking - Preserve original error message in handleGetContextUsage catch block - Add ControlDispatcher test for get_context_usage routing - Add JSDoc comment for context command in non-interactive allowlist Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 7792320 commit 32ee12e

5 files changed

Lines changed: 50 additions & 8 deletions

File tree

packages/cli/src/nonInteractive/control/ControlDispatcher.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import type {
1818
CLIControlInterruptRequest,
1919
CLIControlSetModelRequest,
2020
CLIControlSupportedCommandsRequest,
21+
CLIControlGetContextUsageRequest,
2122
} from '../types.js';
2223

2324
/**
@@ -242,6 +243,41 @@ describe('ControlDispatcher', () => {
242243
});
243244
});
244245

246+
it('should route get_context_usage request to system controller', async () => {
247+
const request: CLIControlRequest = {
248+
type: 'control_request',
249+
request_id: 'req-ctx',
250+
request: {
251+
subtype: 'get_context_usage',
252+
show_details: false,
253+
} as CLIControlGetContextUsageRequest,
254+
};
255+
256+
const mockResponse = {
257+
subtype: 'get_context_usage',
258+
totalTokens: 1000,
259+
};
260+
261+
vi.mocked(mockSystemController.handleRequest).mockResolvedValue(
262+
mockResponse,
263+
);
264+
265+
await dispatcher.dispatch(request);
266+
267+
expect(mockSystemController.handleRequest).toHaveBeenCalledWith(
268+
request.request,
269+
'req-ctx',
270+
);
271+
expect(mockContext.streamJson.send).toHaveBeenCalledWith({
272+
type: 'control_response',
273+
response: {
274+
subtype: 'success',
275+
request_id: 'req-ctx',
276+
response: mockResponse,
277+
},
278+
});
279+
});
280+
245281
it('should send error response when controller throws error', async () => {
246282
const request: CLIControlRequest = {
247283
type: 'control_request',

packages/cli/src/nonInteractive/control/ControlDispatcher.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* which wraps these controllers with a stable programmatic API.
1515
*
1616
* Controllers:
17-
* - SystemController: initialize, interrupt, set_model, supported_commands
17+
* - SystemController: initialize, interrupt, set_model, supported_commands, get_context_usage
1818
* - PermissionController: can_use_tool, set_permission_mode
1919
* - SdkMcpController: mcp_server_status (mcp_message handled via callback)
2020
* - HookController: hook_callback
@@ -380,6 +380,7 @@ export class ControlDispatcher implements IPendingRequestRegistry {
380380
case 'interrupt':
381381
case 'set_model':
382382
case 'supported_commands':
383+
case 'get_context_usage':
383384
return this.systemController;
384385

385386
case 'can_use_tool':

packages/cli/src/nonInteractive/control/controllers/systemController.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,12 @@ export class SystemController extends BaseController {
8282
}
8383

8484
try {
85-
const { collectContextData } = await import(
86-
'../../../ui/commands/contextCommand.js'
87-
);
85+
const mod = await import('../../../ui/commands/contextCommand.js');
86+
if (typeof mod.collectContextData !== 'function') {
87+
throw new Error('collectContextData is not available');
88+
}
8889
const showDetails = payload.show_details ?? false;
89-
const contextUsageItem = await collectContextData(
90+
const contextUsageItem = await mod.collectContextData(
9091
this.context.config,
9192
showDetails,
9293
);
@@ -96,11 +97,13 @@ export class SystemController extends BaseController {
9697
...contextUsageItem,
9798
};
9899
} catch (error) {
100+
const errorMessage =
101+
error instanceof Error ? error.message : 'Failed to get context usage';
99102
debugLogger.error(
100103
'[SystemController] Failed to get context usage:',
101104
error,
102105
);
103-
throw new Error('Failed to get context usage');
106+
throw new Error(errorMessage);
104107
}
105108
}
106109

packages/cli/src/nonInteractiveCliCommands.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ const debugLogger = createDebugLogger('NON_INTERACTIVE_COMMANDS');
3737
* - init: Initialize project configuration
3838
* - summary: Generate session summary
3939
* - compress: Compress conversation history
40+
* - context: Show context window usage (read-only diagnostic)
4041
*/
4142
export const ALLOWED_BUILTIN_COMMANDS_NON_INTERACTIVE = [
4243
'init',

packages/cli/src/ui/commands/contextCommand.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,9 +319,10 @@ export const contextCommand: SlashCommand = {
319319
const showDetails =
320320
args?.trim().toLowerCase() === 'detail' ||
321321
args?.trim().toLowerCase() === '-d';
322+
const executionMode = context.executionMode ?? 'interactive';
322323
const { config } = context.services;
323324
if (!config) {
324-
if (context.executionMode === 'interactive') {
325+
if (executionMode === 'interactive') {
325326
context.ui.addItem(
326327
{
327328
type: MessageType.ERROR,
@@ -340,7 +341,7 @@ export const contextCommand: SlashCommand = {
340341

341342
const contextUsageItem = await collectContextData(config, showDetails);
342343

343-
if (context.executionMode === 'interactive') {
344+
if (executionMode === 'interactive') {
344345
context.ui.addItem(contextUsageItem, Date.now());
345346
return;
346347
} else {

0 commit comments

Comments
 (0)