From acf45df22e5caa97f8e67919db126ef83628da0d Mon Sep 17 00:00:00 2001 From: Ame <123734885+luokerenx4@users.noreply.github.com> Date: Sun, 6 Sep 2026 21:36:42 +0800 Subject: [PATCH] fix(workspaces): launch Grok ACP with agent-scoped flags --- docs/web-conversation-surface.md | 10 +++++++++- src/workspaces/adapters/grok.ts | 6 +++--- src/workspaces/adapters/web-command.spec.ts | 2 +- src/workspaces/service.ts | 4 +++- src/workspaces/web-session-host.spec.ts | 15 +++++++++++++++ src/workspaces/web-session-host.ts | 18 ++++++++++++++++-- src/workspaces/web-session/acp-transport.ts | 3 +++ 7 files changed, 50 insertions(+), 8 deletions(-) diff --git a/docs/web-conversation-surface.md b/docs/web-conversation-surface.md index ce3bb21b8..2405bdfdc 100644 --- a/docs/web-conversation-surface.md +++ b/docs/web-conversation-surface.md @@ -33,7 +33,7 @@ approval or cannot reopen an exact recorded conversation. | Wire | Runtimes | Process | Permission prompts | Fresh session | |---|---|---|---|---| | `pi-rpc` | `pi`, `omp` | `--mode rpc` JSONL; Pi additionally `--approve`, omp `--auto-approve` | none in RPC mode; launch-time approval | yes (RPC allocates the id) | -| `acp` | `cursor`, `grok`, `opencode` | Agent Client Protocol JSON-RPC over stdio (`cursor-agent acp`, `grok agent stdio`, `opencode acp`) | `session/request_permission` with the agent's own options | `session/new`; resume via `session/load` when advertised | +| `acp` | `cursor`, `grok`, `opencode` | Agent Client Protocol JSON-RPC over stdio (`cursor-agent acp`, `grok agent --no-leader stdio`, `opencode acp`) | `session/request_permission` with the agent's own options | `session/new`; resume via `session/load` when advertised | | `claude-stream-json` | `claude` | `-p --input-format stream-json --output-format stream-json --include-partial-messages --permission-prompt-tool stdio` | `control_request` `can_use_tool`; answered with allow/deny | `--session-id ` chosen by the adapter | | `codex-app-server` | `codex` | `codex app-server --listen stdio://` with MCP registration, `approvalPolicy: on-request`, `sandbox: workspace-write` | `item/commandExecution/requestApproval`, `item/fileChange/requestApproval` (answered with a `decision` enum), `item/permissions/requestApproval` (answered with the granted `permissions` profile + `scope`), `item/tool/requestUserInput` | `thread/start`; resume via `thread/resume` | @@ -122,3 +122,11 @@ change. each runtime, open one Session in Web, send a prompt that needs a tool, answer the card, stop mid-turn, then reopen the same Session in the TUI and confirm the native transcript is shared. State the gap when this was not run. + +Grok agent flags belong after the `agent` subcommand; the pager TUI rejects +top-level `--no-leader` when launching ACP. Verify argv with the installed CLI, +not just a fake process. Startup errors retain the child exit diagnostic rather +than replacing it with transport-disposal errors. Failed opens are rolled back +by their route, so exit callbacks must not race that registry write. During a +live ACP turn, user-message echoes are ignored because prompt() already +appended the message; history replay still consumes user-message chunks. diff --git a/src/workspaces/adapters/grok.ts b/src/workspaces/adapters/grok.ts index f5b07f2bf..ef956efdf 100644 --- a/src/workspaces/adapters/grok.ts +++ b/src/workspaces/adapters/grok.ts @@ -353,16 +353,16 @@ export const grokAdapter: CliAdapter = { return [...cmd, ...grokResumeArgs(ctx.resume)]; }, - // Web surface: `grok --no-leader [model/effort] [--rules …] agent stdio`. + // Web surface: `grok [--rules …] agent --no-leader [model/effort] stdio`. // Session identity is negotiated over ACP, so no `--resume`/`--continue`. composeWebCommand(_base: readonly string[], ctx: SpawnContext): readonly string[] { if (ctx.resume === 'last') throw new Error('the Web surface requires a concrete Grok session id or a fresh Session'); return [ 'grok', - '--no-leader', - ...(ctx.sessionRuntime?.webArgs ?? ctx.sessionRuntime?.interactiveArgs ?? []), ...grokRulesArgs(ctx), 'agent', + '--no-leader', + ...(ctx.sessionRuntime?.webArgs ?? ctx.sessionRuntime?.interactiveArgs ?? []), 'stdio', ]; }, diff --git a/src/workspaces/adapters/web-command.spec.ts b/src/workspaces/adapters/web-command.spec.ts index 94ee44e97..926aeabe8 100644 --- a/src/workspaces/adapters/web-command.spec.ts +++ b/src/workspaces/adapters/web-command.spec.ts @@ -65,7 +65,7 @@ describe('Web surface command composition', () => { it('composes the three native ACP agents', () => { expect(cursorAdapter.composeWebCommand!([], ctx({ approveProject: true }))).toEqual(['cursor-agent', '--trust', 'acp']) - expect(grokAdapter.composeWebCommand!([], ctx())).toEqual(['grok', '--no-leader', 'agent', 'stdio']) + expect(grokAdapter.composeWebCommand!([], ctx())).toEqual(['grok', 'agent', '--no-leader', 'stdio']) expect(opencodeAdapter.composeWebCommand!([], ctx())).toEqual(['opencode', 'acp']) }) }) diff --git a/src/workspaces/service.ts b/src/workspaces/service.ts index 9be9c35a1..07890cad8 100644 --- a/src/workspaces/service.ts +++ b/src/workspaces/service.ts @@ -2859,7 +2859,9 @@ export async function createWorkspaceService(opts: CreateWorkspaceServiceOptions // Intentional handoffs are followed by an explicit caller-owned state // update (paused, terminal-running, or deleted). Letting this async // callback also write `paused` would race a Web -> TUI switch. - if (reason.intentional) return; + // The opening route also owns rollback when the handshake fails. + // A second registry write here races its atomic-file replacement. + if (reason.intentional || reason.startupFailed) return; const record = sessionRegistry.findById(recordId); if (!record) return; void sessionRegistry.update(record.wsId, record.id, { diff --git a/src/workspaces/web-session-host.spec.ts b/src/workspaces/web-session-host.spec.ts index 3e62a2d38..3d7849f5a 100644 --- a/src/workspaces/web-session-host.spec.ts +++ b/src/workspaces/web-session-host.spec.ts @@ -217,6 +217,7 @@ function acpProcess(options: { loadSession?: boolean; failAuth?: boolean } = {}) } if (method === 'session/prompt') { const sessionId = params['sessionId'] + self.line({ jsonrpc: '2.0', method: 'session/update', params: { sessionId, update: { sessionUpdate: 'user_message_chunk', content: { type: 'text', text: 'summarize the readme' } } } }) self.line({ jsonrpc: '2.0', method: 'session/update', params: { sessionId, update: { sessionUpdate: 'agent_thought_chunk', content: { type: 'text', text: 'let me look' } } } }) self.line({ jsonrpc: '2.0', method: 'session/update', params: { sessionId, update: { sessionUpdate: 'agent_message_chunk', content: { type: 'text', text: 'Reading ' } } } }) self.line({ jsonrpc: '2.0', method: 'session/update', params: { sessionId, update: { sessionUpdate: 'tool_call', toolCallId: 'call_1', title: 'Read README.md', kind: 'read', status: 'pending', rawInput: { path: 'README.md' } } } }) @@ -623,3 +624,17 @@ describe('Codex question answers', () => { await host.stopAll() }) }) + + +it('preserves the child diagnostic when ACP exits during its handshake', async () => { + const exit = vi.fn() + const process = new FakeProcess((_command, self) => { + self.stderr.write('Error: --no-leader belongs after agent\n') + self.emit('exit', 1, null) + }) + const host = new WebSessionHost(logger, { onExit: exit }, () => process as never) + await expect(host.start(input({ agent: 'grok', wire: 'acp', command: ['grok', 'agent', 'stdio'] }))) + .rejects.toThrow('--no-leader belongs after agent') + expect(exit).toHaveBeenCalledWith('record-1', { code: 1, signal: null, intentional: false, startupFailed: true }) + expect(host.has('record-1')).toBe(false) +}) diff --git a/src/workspaces/web-session-host.ts b/src/workspaces/web-session-host.ts index 12005b299..81df642a1 100644 --- a/src/workspaces/web-session-host.ts +++ b/src/workspaces/web-session-host.ts @@ -52,6 +52,7 @@ export interface WebSessionExitReason { readonly code: number | null readonly signal: NodeJS.Signals | null readonly intentional: boolean + readonly startupFailed?: boolean } interface HostCallbacks { @@ -162,6 +163,7 @@ class LiveWebSession { private stderrTail = '' private intentionalStop = false private exited = false + private startupComplete = false private readonly startedAt = Date.now() constructor( @@ -196,9 +198,17 @@ class LiveWebSession { this.child.once('error', reject) }) this.logger.info('web_session.started', { pid: this.child.pid ?? null, command: this.input.command }) - await this.transport.start() + try { + await this.transport.start() + } catch (error) { + // Transport disposal rejects its handshake waiters with a generic + // "session stopped" error. Preserve the child process diagnostic. + if (this.exited) throw new Error(this.state.error ?? 'Web session process exited during startup') + throw error + } if (this.exited) throw new Error(this.state.error ?? 'Web session process exited during startup') if (this.state.phase === 'starting') this.state.setPhase('idle') + this.startupComplete = true } snapshot(): WebSessionSnapshot { @@ -276,6 +286,10 @@ class LiveWebSession { if (this.exited) return this.exited = true this.channel.close() + if (!this.intentionalStop && !this.state.error) { + const detail = this.stderrTail.trim().slice(-2000) + this.state.error = `${this.input.agent} exited (code=${String(code)}, signal=${String(signal)})${detail ? `: ${detail}` : ''}` + } try { this.transport.dispose?.() } catch { @@ -288,7 +302,7 @@ class LiveWebSession { this.state.clearRequests() this.state.bump() this.logger.info('web_session.exited', { code, signal, intentional: this.intentionalStop }) - this.callbacks.onExit({ code, signal, intentional: this.intentionalStop }) + this.callbacks.onExit({ code, signal, intentional: this.intentionalStop, startupFailed: !this.startupComplete }) } } diff --git a/src/workspaces/web-session/acp-transport.ts b/src/workspaces/web-session/acp-transport.ts index 5c2b73ea4..af7664c87 100644 --- a/src/workspaces/web-session/acp-transport.ts +++ b/src/workspaces/web-session/acp-transport.ts @@ -141,6 +141,9 @@ export class AcpTransport implements WebSessionTransport { if (kind !== 'user_message_chunk') this.flushUser() switch (kind) { case 'user_message_chunk': + // prompt() already appended this turn's user message. ACP runtimes + // such as Grok echo it; only history replay should append these chunks. + if (this.turnActive) break this.pendingUserText = `${this.pendingUserText ?? ''}${contentBlockText(update['content'])}` break case 'agent_message_chunk':