feat(cli): add session runtime reuse and headless runner#12
Open
feat(cli): add session runtime reuse and headless runner#12
Conversation
added 2 commits
March 22, 2026 09:41
添加 SessionRuntime 类来管理会话级资源如聊天服务、工具注册表和审批状态 重构 Agent 创建逻辑以支持显式运行时注入 为 ACP 会话、后台 agent 和服务器路由添加会话运行时支持 添加测试验证会话运行时复用和资源清理
实现 CLI 的无头运行模式，支持完整 agent 循环但不启动交互式 UI 添加 jsonl 格式的结构化事件流输出，适用于 CI 和自动化场景 重构命令输入处理逻辑到共享模块 commandInput.ts 新增 headlessEvents.ts 定义稳定的事件契约和验证逻辑 更新文档和测试用例以覆盖无头模式功能
📝 WalkthroughWalkthroughImplemented a new Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI Entry
participant Headless as Headless Handler
participant Agent as Agent
participant SessionRuntime as SessionRuntime
participant Store as Store/Config
participant Tools as Tool Registry
participant MCP as MCP Registry
CLI->>Headless: main() → handleHeadlessMode()
alt Headless Mode Detected
Headless->>SessionRuntime: SessionRuntime.create({sessionId})
SessionRuntime->>Store: Fetch config & models
SessionRuntime->>Tools: Register builtin tools
SessionRuntime->>MCP: Load MCP tools & servers
SessionRuntime->>SessionRuntime: initialize()
SessionRuntime-->>Headless: Ready with runtime
Headless->>Agent: Agent.createWithRuntime(runtime, {sessionId})
Agent->>SessionRuntime: syncRuntimeState()
Agent-->>Headless: Agent ready
Headless->>Agent: Agent.chat(input, loopOptions)
loop Chat loop
Agent->>Tools: Execute tool
Tools-->>Agent: Result
Agent->>Headless: Emit streaming events
end
Agent-->>Headless: Chat complete
Headless->>Headless: Emit JSONL/text events to stdout/stderr
Headless-->>CLI: Return exit code
else Print Mode
Headless-->>CLI: Return false, continue to handlePrintMode()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 Tip You can validate your CodeRabbit configuration file in your editor.If your editor has YAML language server, you can enable auto-completion and validation by adding |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/ui/hooks/useAgent.ts (1)
85-89:⚠️ Potential issue | 🟠 MajorResource leak:
cleanupAgentdoes not dispose theSessionRuntime.When the consuming component unmounts (e.g.,
useCommandHandlercallscleanupAgent()in its cleanup effect), theruntimeRefholding theSessionRuntimeis never disposed. This can leak MCP connections, chat service resources, and other session-scoped state.🐛 Proposed fix to dispose runtime on cleanup
const cleanupAgent = useMemoizedFn(() => { if (agentRef.current) { agentRef.current = undefined; } + if (runtimeRef.current) { + runtimeRef.current.dispose().catch(() => { + // Ignore disposal errors during cleanup + }); + runtimeRef.current = undefined; + } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/ui/hooks/useAgent.ts` around lines 85 - 89, The cleanupAgent currently only clears agentRef but leaks the SessionRuntime; update cleanupAgent (the useMemoizedFn) to check runtimeRef.current and call its dispose() (or await dispose if it's async) before clearing runtimeRef.current, then clear agentRef.current as before so SessionRuntime resources (MCP connections, chat services) are properly released; reference the runtimeRef and SessionRuntime.dispose() in the patch.
🧹 Nitpick comments (7)
packages/cli/tests/unit/cli/command-input.test.ts (1)
25-63: Add regression coverage forreadCliInput()stdin edge cases.This suite currently validates plugin/slash paths but not the new stdin fallback logic. Please add cases for: non-TTY empty stdin +
defaultMessage, and non-TTY empty stdin without default (throws).🧪 Suggested test additions
+import { PassThrough } from 'node:stream'; + +it('falls back to defaultMessage when piped stdin is empty', async () => { + const { readCliInput } = await import('../../../src/commands/shared/commandInput.js'); + const stdin = new PassThrough(); + Object.defineProperty(stdin, 'isTTY', { value: false }); + stdin.end(''); + + const result = await readCliInput({ stdin: stdin as unknown as NodeJS.ReadStream, defaultMessage: 'hello' }); + expect(result).toBe('hello'); +}); + +it('throws when piped stdin is empty and no default is provided', async () => { + const { readCliInput } = await import('../../../src/commands/shared/commandInput.js'); + const stdin = new PassThrough(); + Object.defineProperty(stdin, 'isTTY', { value: false }); + stdin.end(''); + + await expect( + readCliInput({ stdin: stdin as unknown as NodeJS.ReadStream }) + ).rejects.toThrow('No input provided'); +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/tests/unit/cli/command-input.test.ts` around lines 25 - 63, Add unit tests for readCliInput to cover non-TTY stdin edge cases: mock process.stdin.isTTY to false and simulate empty stdin; verify that calling readCliInput({ defaultMessage: 'foo' }) resolves to 'foo', and that calling readCliInput() (no defaultMessage) rejects/throws. Keep these in the same describe block alongside existing tests, and reference the readCliInput export from '../../../src/commands/shared/commandInput.js' to locate the function under test.packages/cli/src/commands/headless.ts (1)
86-89: Type inconsistency:permissionModeisPermissionMode | stringbut schema usesz.nativeEnum.The interface declares
permissionMode?: PermissionMode | string(line 89), butHeadlessOptionsSchemaat line 66 usesz.nativeEnum(PermissionMode), which only accepts enum values. Consider aligning the type to justPermissionModesince the schema enforces this constraint.♻️ Align type with schema constraint
/** Permission mode override; defaults to YOLO in headless mode. */ - permissionMode?: PermissionMode | string; + permissionMode?: PermissionMode;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/headless.ts` around lines 86 - 89, The permissionMode field in the Headless options type is declared as permissionMode?: PermissionMode | string but the HeadlessOptionsSchema uses z.nativeEnum(PermissionMode) which only accepts PermissionMode enum values; update the type to permissionMode?: PermissionMode to match the schema (locate the interface/type that declares permissionMode and the HeadlessOptionsSchema and change the union type to the enum type PermissionMode so the TypeScript type aligns with zod validation).packages/cli/tests/unit/agent-runtime/agent/background-agent-manager.test.ts (1)
94-118: Consider usingvi.waitFororflushPromisesfor more reliable async test assertions.The
setTimeout(resolve, 0)pattern on line 105 relies on the microtask queue clearing within a single event loop tick, which can be fragile if the production code adds additional async operations or timers.♻️ Suggested improvement using flushPromises
+const flushPromises = () => new Promise((resolve) => setImmediate(resolve)); + it('应为后台 agent 创建独立 runtime', async () => { manager.startBackgroundAgent({ config: { name: 'Explore', description: 'Explore agent', systemPrompt: 'You are an explorer', }, description: 'Test task', prompt: 'Do something', }); - await new Promise((resolve) => setTimeout(resolve, 0)); + await flushPromises(); expect(SessionRuntime.create).toHaveBeenCalledWith({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/tests/unit/agent-runtime/agent/background-agent-manager.test.ts` around lines 94 - 118, The test uses an unreliable setTimeout(resolve, 0) to wait for async work after calling manager.startBackgroundAgent; replace that pattern with a deterministic waiter such as await vi.waitFor(() => { /* expectations */ }) or await flushPromises() before asserting SessionRuntime.create and Agent.createWithRuntime calls, ensuring you await the async completion after invoking manager.startBackgroundAgent and then perform the expect checks for SessionRuntime.create and Agent.createWithRuntime.packages/cli/tests/unit/agent-runtime/server/session-routes.test.ts (1)
36-49: Static analysis flags empty blocks; consider explicit no-op implementations.The empty functions at lines 39 and 47 are flagged by Biome as
noEmptyBlockStatements. While these are valid mock implementations, you could suppress the warnings explicitly or use minimal no-op returns.♻️ Optional: Explicit no-op to silence linter
vi.mock('../../../../src/server/bus.js', () => ({ Bus: { publish: vi.fn(), - subscribe: vi.fn(() => () => {}), + subscribe: vi.fn(() => () => { /* unsubscribe no-op */ }), }, })); vi.mock('../../../../src/services/SessionService.js', () => ({ SessionService: { listSessions: vi.fn(async () => []), loadSession: vi.fn(async () => []), - deleteSession: vi.fn(async () => {}), + deleteSession: vi.fn(async () => { /* no-op */ }), }, }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/tests/unit/agent-runtime/server/session-routes.test.ts` around lines 36 - 49, The linter flags empty block statements in the mocks (Bus.subscribe's inner function and the async mock bodies for SessionService), so replace those empty blocks with explicit no-op returns: have Bus.subscribe return a clear no-op function (e.g., return () => undefined) and make the async SessionService mocks return explicit values (e.g., listSessions/loadSession return Promise.resolve([]) and deleteSession return Promise.resolve(undefined) or async () => undefined) so the functions are no longer empty blocks while preserving mock behavior; update the mocks for Bus.publish, Bus.subscribe, SessionService.listSessions, SessionService.loadSession and SessionService.deleteSession accordingly.packages/cli/src/ui/hooks/useAgent.ts (1)
38-39: Clarify ephemeral runtime condition logic.The
shouldUseEphemeralRuntimecondition triggers whenoverrides?.modelIdexists AND differs fromoptions.modelId. This means:
- If
overrides.modelIdmatchesoptions.modelId, we use session runtime (correct)- If
overrides.modelIdis undefined, we use session runtime (correct)- If
overrides.modelIddiffers, we skip session runtime entirely (creates throwaway agent)However, the naming "ephemeral runtime" is misleading since the
elsebranch at line 66-73 callsAgent.create()without any runtime, not an ephemeral one. Consider renaming toshouldSkipSessionRuntimefor clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/ui/hooks/useAgent.ts` around lines 38 - 39, The variable shouldUseEphemeralRuntime is misnamed—its logic actually detects when overrides.modelId exists and differs from options.modelId (i.e., when we should skip using the session runtime and create a throwaway agent), but the else branch calls Agent.create() without any runtime rather than an "ephemeral runtime." Rename shouldUseEphemeralRuntime to shouldSkipSessionRuntime (and update all references) to reflect intent, and update the surrounding comment/identifier usage so the condition (!!overrides?.modelId && overrides.modelId !== options.modelId) reads clearly as "skip session runtime", leaving the Agent.create() calls unchanged.packages/cli/src/agent/runtime/SessionRuntime.ts (1)
36-41: Unused option:strictMcpConfigis defined but never used.The
strictMcpConfigproperty is declared inSessionRuntimeOptionsbut not referenced anywhere in the class implementation. This appears to be dead code or an incomplete feature.♻️ Either remove unused option or implement strict MCP handling
If the feature is not needed:
export interface SessionRuntimeOptions { sessionId: string; modelId?: string; mcpConfig?: string[]; - strictMcpConfig?: boolean; }Alternatively, implement strict mode in
registerMcpTools()to throw on connection failures whenstrictMcpConfigis true.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/agent/runtime/SessionRuntime.ts` around lines 36 - 41, The SessionRuntimeOptions interface declares strictMcpConfig but it isn't used; either remove this dead option or implement strict behavior in SessionRuntime by reading options.strictMcpConfig inside the SessionRuntime constructor and using it in registerMcpTools (or wherever MCP connections are established) so that when strictMcpConfig === true registerMcpTools throws or propagates connection errors instead of swallowing them; update references to SessionRuntimeOptions, the SessionRuntime constructor, and the registerMcpTools method to wire the flag through and add tests/handling for strict failure behavior if kept.packages/cli/src/agent/Agent.ts (1)
196-201: Minor inefficiency:syncRuntimeState()called even when model unchanged.When
sessionRuntimeis present,syncRuntimeState()is called regardless of whether the model actually changed. WhileSessionRuntime.refresh()internally skips re-applying the same model, the subsequentsyncRuntimeState()still runs unnecessarily.♻️ Proposed optimization
private async switchModelIfNeeded(modelId: string): Promise<void> { if (this.sessionRuntime) { + if (!modelId || modelId === this.currentModelId) return; await this.sessionRuntime.refresh({ modelId }); this.syncRuntimeState(); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/agent/Agent.ts` around lines 196 - 201, The method switchModelIfNeeded calls this.sessionRuntime.refresh(...) and then always calls this.syncRuntimeState(), causing unnecessary work when the model hasn't changed; update switchModelIfNeeded to first check the current model on this.sessionRuntime (e.g., compare this.sessionRuntime.modelId or an equivalent accessor) and if it matches the requested modelId return early, otherwise await this.sessionRuntime.refresh({ modelId }) and then call this.syncRuntimeState(); alternatively, if you prefer to change the session runtime API, have SessionRuntime.refresh return a boolean "changed" and only call this.syncRuntimeState() when that boolean is true (refer to switchModelIfNeeded, sessionRuntime.refresh, and syncRuntimeState).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/cli/middleware.ts`:
- Around line 82-84: The current conditional only checks that --output-format is
used with --print or --headless but doesn't validate mode/format pairs; update
the validation around argv.outputFormat / argv.print / argv.headless to enforce
allowed format mappings (e.g., define allowedFormatsForMode where print
disallows 'jsonl' and headless disallows 'json' based on downstream
capabilities) and throw a clear Error if argv.outputFormat is not in the allowed
set for the selected mode; replace the simple if (argv.outputFormat &&
argv.outputFormat !== 'text' && !argv.print && !argv.headless) check with
mode-aware checks that reference argv.outputFormat, argv.print, and
argv.headless and include the problematic format/mode in the error message.
In `@packages/cli/src/commands/headless.ts`:
- Around line 59-65: The maxTurns Zod schema in headless.ts currently rejects 0;
update the refine predicate on the maxTurns z.number().int().refine(...) to
accept 0 as valid (e.g., change the condition to allow value === -1 || value >=
0) and update the refine error message to reflect that 0 is allowed (e.g., "must
be -1, 0, or a positive integer") so the schema matches the CLI config
semantics.
In `@packages/cli/src/commands/shared/commandInput.ts`:
- Around line 29-39: When stdin is non-TTY the code currently returns the
trimmed string immediately even when it's empty; change the read path to store
the result (e.g., const input = Buffer.concat(chunks).toString('utf-8').trim())
and only return when input is non-empty. If input is empty, fall through so
existing logic can use options.defaultMessage (or reach the final error path)
instead of returning '' from the stdin branch.
In `@packages/cli/src/config/types.ts`:
- Line 177: The print command currently only handles 'json' and 'stream-json' in
packages/cli/src/commands/print.ts (around the branching at lines 112-131),
causing '--output-format jsonl' to fall back to text; update the output handling
in the print command to explicitly support 'jsonl' by serializing each record as
a JSON string and writing one JSON object per line (newline-delimited) when
outputFormat === 'jsonl' (reuse the same input/result iteration used for
'stream-json'), ensuring flags parsed by the middleware
(packages/cli/src/cli/middleware.ts) are honored; alternatively, if you prefer
to disallow jsonl in interactive print mode, update the middleware validation to
reject jsonl when mode === 'print' and surface an appropriate error, but do not
leave the mismatch unhandled.
In `@packages/cli/src/server/routes/session.ts`:
- Around line 442-447: There's a race: multiple requests can call
SessionRuntime.create for the same sessionId because runtimes.get(sessionId) can
be falsy concurrently; fix by introducing a pendingRuntimes Map<string,
Promise<SessionRuntime>> (created in SessionRoutes) and use it to deduplicate
creation: first check runtimes.get(sessionId), then check
pendingRuntimes.get(sessionId) and await that promise if present, otherwise
create a creationPromise = SessionRuntime.create({ sessionId }), store it in
pendingRuntimes, await it, set runtimes.set(sessionId, runtime) and finally
remove pendingRuntimes.delete(sessionId) before calling
Agent.createWithRuntime(runtime, { sessionId }); ensure errors clear the pending
map so subsequent attempts can retry.
---
Outside diff comments:
In `@packages/cli/src/ui/hooks/useAgent.ts`:
- Around line 85-89: The cleanupAgent currently only clears agentRef but leaks
the SessionRuntime; update cleanupAgent (the useMemoizedFn) to check
runtimeRef.current and call its dispose() (or await dispose if it's async)
before clearing runtimeRef.current, then clear agentRef.current as before so
SessionRuntime resources (MCP connections, chat services) are properly released;
reference the runtimeRef and SessionRuntime.dispose() in the patch.
---
Nitpick comments:
In `@packages/cli/src/agent/Agent.ts`:
- Around line 196-201: The method switchModelIfNeeded calls
this.sessionRuntime.refresh(...) and then always calls this.syncRuntimeState(),
causing unnecessary work when the model hasn't changed; update
switchModelIfNeeded to first check the current model on this.sessionRuntime
(e.g., compare this.sessionRuntime.modelId or an equivalent accessor) and if it
matches the requested modelId return early, otherwise await
this.sessionRuntime.refresh({ modelId }) and then call this.syncRuntimeState();
alternatively, if you prefer to change the session runtime API, have
SessionRuntime.refresh return a boolean "changed" and only call
this.syncRuntimeState() when that boolean is true (refer to switchModelIfNeeded,
sessionRuntime.refresh, and syncRuntimeState).
In `@packages/cli/src/agent/runtime/SessionRuntime.ts`:
- Around line 36-41: The SessionRuntimeOptions interface declares
strictMcpConfig but it isn't used; either remove this dead option or implement
strict behavior in SessionRuntime by reading options.strictMcpConfig inside the
SessionRuntime constructor and using it in registerMcpTools (or wherever MCP
connections are established) so that when strictMcpConfig === true
registerMcpTools throws or propagates connection errors instead of swallowing
them; update references to SessionRuntimeOptions, the SessionRuntime
constructor, and the registerMcpTools method to wire the flag through and add
tests/handling for strict failure behavior if kept.
In `@packages/cli/src/commands/headless.ts`:
- Around line 86-89: The permissionMode field in the Headless options type is
declared as permissionMode?: PermissionMode | string but the
HeadlessOptionsSchema uses z.nativeEnum(PermissionMode) which only accepts
PermissionMode enum values; update the type to permissionMode?: PermissionMode
to match the schema (locate the interface/type that declares permissionMode and
the HeadlessOptionsSchema and change the union type to the enum type
PermissionMode so the TypeScript type aligns with zod validation).
In `@packages/cli/src/ui/hooks/useAgent.ts`:
- Around line 38-39: The variable shouldUseEphemeralRuntime is misnamed—its
logic actually detects when overrides.modelId exists and differs from
options.modelId (i.e., when we should skip using the session runtime and create
a throwaway agent), but the else branch calls Agent.create() without any runtime
rather than an "ephemeral runtime." Rename shouldUseEphemeralRuntime to
shouldSkipSessionRuntime (and update all references) to reflect intent, and
update the surrounding comment/identifier usage so the condition
(!!overrides?.modelId && overrides.modelId !== options.modelId) reads clearly as
"skip session runtime", leaving the Agent.create() calls unchanged.
In
`@packages/cli/tests/unit/agent-runtime/agent/background-agent-manager.test.ts`:
- Around line 94-118: The test uses an unreliable setTimeout(resolve, 0) to wait
for async work after calling manager.startBackgroundAgent; replace that pattern
with a deterministic waiter such as await vi.waitFor(() => { /* expectations */
}) or await flushPromises() before asserting SessionRuntime.create and
Agent.createWithRuntime calls, ensuring you await the async completion after
invoking manager.startBackgroundAgent and then perform the expect checks for
SessionRuntime.create and Agent.createWithRuntime.
In `@packages/cli/tests/unit/agent-runtime/server/session-routes.test.ts`:
- Around line 36-49: The linter flags empty block statements in the mocks
(Bus.subscribe's inner function and the async mock bodies for SessionService),
so replace those empty blocks with explicit no-op returns: have Bus.subscribe
return a clear no-op function (e.g., return () => undefined) and make the async
SessionService mocks return explicit values (e.g., listSessions/loadSession
return Promise.resolve([]) and deleteSession return Promise.resolve(undefined)
or async () => undefined) so the functions are no longer empty blocks while
preserving mock behavior; update the mocks for Bus.publish, Bus.subscribe,
SessionService.listSessions, SessionService.loadSession and
SessionService.deleteSession accordingly.
In `@packages/cli/tests/unit/cli/command-input.test.ts`:
- Around line 25-63: Add unit tests for readCliInput to cover non-TTY stdin edge
cases: mock process.stdin.isTTY to false and simulate empty stdin; verify that
calling readCliInput({ defaultMessage: 'foo' }) resolves to 'foo', and that
calling readCliInput() (no defaultMessage) rejects/throws. Keep these in the
same describe block alongside existing tests, and reference the readCliInput
export from '../../../src/commands/shared/commandInput.js' to locate the
function under test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f5035fcd-d9e4-421e-af2d-30bf7747ff89
📒 Files selected for processing (34)
README.en.mdREADME.mdpackages/cli/src/acp/BladeAgent.tspackages/cli/src/acp/Session.tspackages/cli/src/agent/Agent.tspackages/cli/src/agent/runtime/SessionRuntime.tspackages/cli/src/agent/subagents/BackgroundAgentManager.tspackages/cli/src/agent/types.tspackages/cli/src/blade.tsxpackages/cli/src/cli/config.tspackages/cli/src/cli/middleware.tspackages/cli/src/cli/types.tspackages/cli/src/commands/headless.tspackages/cli/src/commands/headlessEvents.tspackages/cli/src/commands/print.tspackages/cli/src/commands/shared/commandInput.tspackages/cli/src/config/types.tspackages/cli/src/server/routes/session.tspackages/cli/src/tools/execution/ExecutionPipeline.tspackages/cli/src/tools/execution/PipelineStages.tspackages/cli/src/tools/execution/SessionApprovalStore.tspackages/cli/src/tools/registry/ToolRegistry.tspackages/cli/src/ui/hooks/useAgent.tspackages/cli/src/ui/hooks/useCommandHandler.tspackages/cli/tests/integration/cli/blade-help.test.tspackages/cli/tests/integration/pipeline.test.tspackages/cli/tests/unit/agent-runtime/acp/session.test.tspackages/cli/tests/unit/agent-runtime/agent/agent-create.test.tspackages/cli/tests/unit/agent-runtime/agent/background-agent-manager.test.tspackages/cli/tests/unit/agent-runtime/agent/session-runtime.test.tspackages/cli/tests/unit/agent-runtime/server/session-routes.test.tspackages/cli/tests/unit/cli/command-input.test.tspackages/cli/tests/unit/cli/headless-events.test.tspackages/cli/tests/unit/cli/headless.test.ts
Comment on lines
+82
to
84
| if (argv.outputFormat && argv.outputFormat !== 'text' && !argv.print && !argv.headless) { | ||
| throw new Error('--output-format can only be used with --print or --headless'); | ||
| } |
There was a problem hiding this comment.
Output-format validation is still allowing unsupported mode/format pairs.
The current check permits combinations that are not actually supported downstream (e.g., --print --output-format jsonl and --headless --output-format json). That can silently degrade to incorrect output behavior.
💡 Proposed fix
export const validateOutput: MiddlewareFunction = (argv) => {
// 验证输出格式组合
if (argv.outputFormat && argv.outputFormat !== 'text' && !argv.print && !argv.headless) {
throw new Error('--output-format can only be used with --print or --headless');
}
+
+ if (argv.outputFormat === 'jsonl' && !argv.headless) {
+ throw new Error('--output-format jsonl requires --headless');
+ }
+
+ if (
+ argv.headless &&
+ (argv.outputFormat === 'json' || argv.outputFormat === 'stream-json')
+ ) {
+ throw new Error('--headless only supports --output-format text or jsonl');
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (argv.outputFormat && argv.outputFormat !== 'text' && !argv.print && !argv.headless) { | |
| throw new Error('--output-format can only be used with --print or --headless'); | |
| } | |
| export const validateOutput: MiddlewareFunction = (argv) => { | |
| // 验证输出格式组合 | |
| if (argv.outputFormat && argv.outputFormat !== 'text' && !argv.print && !argv.headless) { | |
| throw new Error('--output-format can only be used with --print or --headless'); | |
| } | |
| if (argv.outputFormat === 'jsonl' && !argv.headless) { | |
| throw new Error('--output-format jsonl requires --headless'); | |
| } | |
| if ( | |
| argv.headless && | |
| (argv.outputFormat === 'json' || argv.outputFormat === 'stream-json') | |
| ) { | |
| throw new Error('--headless only supports --output-format text or jsonl'); | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/cli/middleware.ts` around lines 82 - 84, The current
conditional only checks that --output-format is used with --print or --headless
but doesn't validate mode/format pairs; update the validation around
argv.outputFormat / argv.print / argv.headless to enforce allowed format
mappings (e.g., define allowedFormatsForMode where print disallows 'jsonl' and
headless disallows 'json' based on downstream capabilities) and throw a clear
Error if argv.outputFormat is not in the allowed set for the selected mode;
replace the simple if (argv.outputFormat && argv.outputFormat !== 'text' &&
!argv.print && !argv.headless) check with mode-aware checks that reference
argv.outputFormat, argv.print, and argv.headless and include the problematic
format/mode in the error message.
Comment on lines
+59
to
+65
| maxTurns: z | ||
| .number() | ||
| .int() | ||
| .refine((value) => value === -1 || value > 0, { | ||
| message: 'must be -1 or a positive integer', | ||
| }) | ||
| .optional(), |
There was a problem hiding this comment.
maxTurns validation rejects 0, inconsistent with CLI config.
The global CLI option at config.ts:97 documents: "Maximum conversation turns (-1: unlimited, 0: disable chat, N>0: limit to N turns)". However, this schema's .refine() rejects 0 since it only allows -1 || value > 0.
🐛 Proposed fix to allow 0
maxTurns: z
.number()
.int()
- .refine((value) => value === -1 || value > 0, {
- message: 'must be -1 or a positive integer',
+ .refine((value) => value === -1 || value >= 0, {
+ message: 'must be -1 (unlimited), 0 (disable chat), or a positive integer',
})
.optional(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| maxTurns: z | |
| .number() | |
| .int() | |
| .refine((value) => value === -1 || value > 0, { | |
| message: 'must be -1 or a positive integer', | |
| }) | |
| .optional(), | |
| maxTurns: z | |
| .number() | |
| .int() | |
| .refine((value) => value === -1 || value >= 0, { | |
| message: 'must be -1 (unlimited), 0 (disable chat), or a positive integer', | |
| }) | |
| .optional(), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/commands/headless.ts` around lines 59 - 65, The maxTurns Zod
schema in headless.ts currently rejects 0; update the refine predicate on the
maxTurns z.number().int().refine(...) to accept 0 as valid (e.g., change the
condition to allow value === -1 || value >= 0) and update the refine error
message to reflect that 0 is allowed (e.g., "must be -1, 0, or a positive
integer") so the schema matches the CLI config semantics.
Comment on lines
+29
to
+39
| if (!stdin.isTTY) { | ||
| const chunks: Buffer[] = []; | ||
| for await (const chunk of stdin) { | ||
| chunks.push(chunk); | ||
| } | ||
| return Buffer.concat(chunks).toString('utf-8').trim(); | ||
| } | ||
|
|
||
| if (options.defaultMessage !== undefined) { | ||
| return options.defaultMessage; | ||
| } |
There was a problem hiding this comment.
Empty piped stdin bypasses fallback/error handling.
When stdin is non-TTY but empty, the function currently returns '' and never reaches defaultMessage or the final error. This breaks the intended "if still missing, fallback/throw" behavior.
💡 Proposed fix
const stdin = options.stdin ?? process.stdin;
if (!stdin.isTTY) {
const chunks: Buffer[] = [];
for await (const chunk of stdin) {
- chunks.push(chunk);
+ chunks.push(typeof chunk === 'string' ? Buffer.from(chunk) : chunk);
}
- return Buffer.concat(chunks).toString('utf-8').trim();
+ const pipedInput = Buffer.concat(chunks).toString('utf-8').trim();
+ if (pipedInput) {
+ return pipedInput;
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/commands/shared/commandInput.ts` around lines 29 - 39, When
stdin is non-TTY the code currently returns the trimmed string immediately even
when it's empty; change the read path to store the result (e.g., const input =
Buffer.concat(chunks).toString('utf-8').trim()) and only return when input is
non-empty. If input is empty, fall through so existing logic can use
options.defaultMessage (or reach the final error path) instead of returning ''
from the stdin branch.
| // CLI 专属字段 - 其他 | ||
| addDirs?: string[]; // 额外允许访问的目录 | ||
| outputFormat?: 'text' | 'json' | 'stream-json'; // 输出格式 | ||
| outputFormat?: 'text' | 'json' | 'stream-json' | 'jsonl'; // 输出格式 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify print.ts output format handling
rg -n -A20 'outputFormat' packages/cli/src/commands/print.ts | head -50Repository: echoVic/blade-code
Length of output: 1487
🏁 Script executed:
# Search for jsonl handling in the codebase
rg -n 'jsonl' packages/cli/src --type tsRepository: echoVic/blade-code
Length of output: 3132
🏁 Script executed:
# Check the full context around line 177 in types.ts
cat -n packages/cli/src/config/types.ts | sed -n '170,190p'Repository: echoVic/blade-code
Length of output: 784
🏁 Script executed:
# Search for any validation logic in print.ts or related command files
rg -n -B5 -A10 'printCommand' packages/cli/src/commands/print.ts | head -80Repository: echoVic/blade-code
Length of output: 942
🏁 Script executed:
# Check if there's validation preventing jsonl with --print mode
rg -n -B5 -A5 'outputFormat.*jsonl|jsonl.*print' packages/cli/src --type tsRepository: echoVic/blade-code
Length of output: 11287
🏁 Script executed:
# Check the CLI entry point and main command setup to see if there's validation
rg -n 'print.*headless|headless.*print' packages/cli/src --type tsRepository: echoVic/blade-code
Length of output: 414
🏁 Script executed:
# Look at how outputFormat is validated/processed in the main flow
rg -n -B3 -A3 'outputFormat' packages/cli/src/cli/config.ts | head -60Repository: echoVic/blade-code
Length of output: 263
Handle jsonl format in --print mode.
The middleware validation in packages/cli/src/cli/middleware.ts:82-83 allows jsonl with --print mode, but packages/cli/src/commands/print.ts:112-131 only explicitly handles 'json' and 'stream-json', falling through to plain text output for jsonl. This creates a silent mismatch where blade --print --output-format jsonl produces text instead of JSONL.
Either add jsonl handling to print.ts or restrict jsonl to headless-only mode by updating the validation to reject it with --print.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/config/types.ts` at line 177, The print command currently
only handles 'json' and 'stream-json' in packages/cli/src/commands/print.ts
(around the branching at lines 112-131), causing '--output-format jsonl' to fall
back to text; update the output handling in the print command to explicitly
support 'jsonl' by serializing each record as a JSON string and writing one JSON
object per line (newline-delimited) when outputFormat === 'jsonl' (reuse the
same input/result iteration used for 'stream-json'), ensuring flags parsed by
the middleware (packages/cli/src/cli/middleware.ts) are honored; alternatively,
if you prefer to disallow jsonl in interactive print mode, update the middleware
validation to reject jsonl when mode === 'print' and surface an appropriate
error, but do not leave the mismatch unhandled.
Comment on lines
+442
to
+447
| let runtime = runtimes.get(sessionId); | ||
| if (!runtime) { | ||
| runtime = await SessionRuntime.create({ sessionId }); | ||
| runtimes.set(sessionId, runtime); | ||
| } | ||
| const agent = await Agent.createWithRuntime(runtime, { sessionId }); |
There was a problem hiding this comment.
Race condition on concurrent runtime creation for the same session.
When two requests for the same sessionId arrive concurrently, both may observe !runtime before either sets the map entry, causing duplicate SessionRuntime.create() calls. This wastes resources (the context snippet shows it's expensive: validates config, registers tools, discovers skills) and the later runtime replaces the earlier one, potentially orphaning resources.
Consider using a pending-creation map or a deduplication pattern:
🛡️ Proposed fix using a creation promise cache
async function executeRunAsync(
run: RunState,
session: SessionInfo,
content: string,
permissionMode: PermissionMode,
- runtimes: Map<string, SessionRuntime>
+ runtimes: Map<string, SessionRuntime>,
+ pendingRuntimes: Map<string, Promise<SessionRuntime>>
): Promise<void> {
// ...
try {
// ...
let runtime = runtimes.get(sessionId);
if (!runtime) {
+ let pending = pendingRuntimes.get(sessionId);
+ if (!pending) {
+ pending = SessionRuntime.create({ sessionId });
+ pendingRuntimes.set(sessionId, pending);
+ }
+ runtime = await pending;
+ pendingRuntimes.delete(sessionId);
- runtime = await SessionRuntime.create({ sessionId });
runtimes.set(sessionId, runtime);
}You'll also need to add const pendingRuntimes = new Map<string, Promise<SessionRuntime>>(); in SessionRoutes() and pass it through.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/server/routes/session.ts` around lines 442 - 447, There's a
race: multiple requests can call SessionRuntime.create for the same sessionId
because runtimes.get(sessionId) can be falsy concurrently; fix by introducing a
pendingRuntimes Map<string, Promise<SessionRuntime>> (created in SessionRoutes)
and use it to deduplicate creation: first check runtimes.get(sessionId), then
check pendingRuntimes.get(sessionId) and await that promise if present,
otherwise create a creationPromise = SessionRuntime.create({ sessionId }), store
it in pendingRuntimes, await it, set runtimes.set(sessionId, runtime) and
finally remove pendingRuntimes.delete(sessionId) before calling
Agent.createWithRuntime(runtime, { sessionId }); ensure errors clear the pending
map so subsequent attempts can retry.
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
1 participant
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
SessionRuntimeso session-scoped resources can persist while turns still use lightweight executorsAgent.create()from accidental session-scoped construction--headlessCLI mode with text/jsonl event output, shared CLI input normalization, docs, and regression coverageTest Plan
env COREPACK_ENABLE_AUTO_PIN=0 pnpm --filter blade-code exec vitest run tests/unit/cli/headless-events.test.ts tests/unit/cli/headless.test.ts tests/unit/cli/command-input.test.ts tests/integration/cli/blade-help.test.ts tests/unit/agent-runtime/agent/agent-create.test.ts tests/unit/agent-runtime/agent/session-runtime.test.ts tests/unit/agent-runtime/acp/session.test.ts tests/unit/agent-runtime/agent/background-agent-manager.test.ts tests/unit/agent-runtime/server/session-routes.test.ts tests/integration/pipeline.test.tsenv COREPACK_ENABLE_AUTO_PIN=0 pnpm --filter blade-code type-checkgit diff --checkSummary by CodeRabbit
New Features
--headlessmode to run the full agent loop without the interactive UI, ideal for CI/automation.--output-format jsonlfor stable, machine-friendly event streaming in CI, sandbox, and testing environments.Documentation