From 458525d4a8db7d0c4b145d7df50728d59ce15783 Mon Sep 17 00:00:00 2001 From: audichuang Date: Sun, 6 Sep 2026 10:52:01 +0800 Subject: [PATCH] fix(mcp): keep typed dependency failures retryable through chat prevalidation session_create already rethrows LocalDaemonAvailabilityError from its prevalidation catch, and session_create_many already returns it per item with the producer's own code and retryability. The two chat entry points did not. Chat prevalidation raises both typed dependency errors, not just the sync one. validateSessionChatTarget calls syncWorkspaceMetaForRead, which raises WorkspaceSyncUnavailableError, and then ensureTargetMachineOnline, which for a target on the machine answering the MCP call routes through ensureLocalRuntimeAvailable -> dispatchLocalControl -> classifyLocalDaemonIpcError and raises LocalDaemonAvailabilityError with code DAEMON_BUSY and retryable: true. Only the first was passed through. The second was flattened into COMMAND_REJECTED (single) or INVALID_ITEM (batch), both with retryable: false, so the retryability the CLI had just computed was destroyed one frame later and a caller honouring `retryable` gave up on a condition that clears on its own. COMMAND_REJECTED also asserts the command was evaluated and refused on its merits, which is not what an unreachable local daemon is. Batch items carry the failure per item rather than aborting the batch, matching session_create_many for the same error type: daemon availability is per target machine, while a workspace sync failure is workspace-wide and still aborts. Untyped errors are deliberately left alone. Both the local-project Flock sync on the create path and the remote-target branch of ensureTargetMachineOnline throw plain Errors, so they still reach the COMMAND_REJECTED fallback; classifying those is the separate transport-classification question #400 raises first. Refs #400 Co-Authored-By: Claude Opus 5 (1M context) Model: claude-opus-5[1m] --- .../src/mcp/lody-mcp-server-chat-sync.test.ts | 77 +++++++++++++++++++ apps/cli/src/mcp/lody-mcp-server.ts | 11 ++- 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/apps/cli/src/mcp/lody-mcp-server-chat-sync.test.ts b/apps/cli/src/mcp/lody-mcp-server-chat-sync.test.ts index 5989ae7f..e713a751 100644 --- a/apps/cli/src/mcp/lody-mcp-server-chat-sync.test.ts +++ b/apps/cli/src/mcp/lody-mcp-server-chat-sync.test.ts @@ -5,6 +5,7 @@ const mocks = vi.hoisted(() => ({ activeInvocation: vi.fn(), findMatchingRetry: vi.fn(), getDocMeta: vi.fn(), + snapshot: vi.fn(), validateSessionChatTarget: vi.fn(), })); @@ -45,6 +46,7 @@ vi.mock('@/orchestration/operation-store', async (importOriginal) => { LodyOperationStore: class { findMatchingRetry = mocks.findMatchingRetry; accept = mocks.accept; + snapshot = mocks.snapshot; }, runWithOperationStoreBusyRetry: vi.fn(async (fn: () => unknown) => await fn()), }; @@ -67,6 +69,8 @@ vi.mock('@lody/shared/node/local-ipc', async (importOriginal) => { }); import { + DAEMON_BUSY_MESSAGE, + LocalDaemonAvailabilityError, WORKSPACE_SYNC_UNAVAILABLE_MESSAGE, WorkspaceSyncUnavailableError, } from '@/lib/command-runtime'; @@ -197,3 +201,76 @@ describe('session chat prevalidation sync failures', () => { expect(mocks.accept).not.toHaveBeenCalled(); }); }); + +describe('session chat prevalidation daemon availability failures', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.stubEnv('LODY_MCP_MACHINE_ID', 'machine-id'); + vi.stubEnv('LODY_MCP_WORKSPACE_ID', 'workspace-id'); + vi.stubEnv('LODY_MCP_SESSION_ID', requesterSession.id); + mocks.activeInvocation.mockReturnValue({ + type: 'session/active-invocation-context' as const, + sessionId: requesterSession.id, + active: true as const, + requesterUserId: requesterSession.userId, + sourceTurnId: 'requester-turn-id', + inputConfig: {}, + }); + mocks.findMatchingRetry.mockReturnValue(undefined); + mocks.getDocMeta + .mockResolvedValueOnce({ meta: requesterSession }) + .mockResolvedValueOnce({ meta: targetSession }); + // validateSessionChatTarget -> ensureTargetMachineOnline -> ensureLocalRuntimeAvailable -> + // classifyLocalDaemonIpcError(IpcTimeoutError) produces exactly this when the chat target + // lives on the machine answering the MCP call. + mocks.validateSessionChatTarget.mockRejectedValue( + new LocalDaemonAvailabilityError({ + code: 'DAEMON_BUSY', + message: DAEMON_BUSY_MESSAGE, + retryable: true, + }) + ); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + }); + + it('reports a busy local daemon as retryable instead of a command refusal', async () => { + const result = await callAndMapMcpError(() => + startSessionChatOperation({ + operationId: 'single-chat-operation', + sessionId: targetSession.id, + prompt: 'continue', + }) + ); + + const content = result.content[0]; + if (!content || content.type !== 'text') throw new Error('expected text result'); + expect(JSON.parse(content.text)).toEqual({ + ok: false, + error: { code: 'DAEMON_BUSY', message: DAEMON_BUSY_MESSAGE, retryable: true }, + }); + expect(mocks.accept).not.toHaveBeenCalled(); + }); + + it('keeps a busy local daemon retryable on the batch item it failed', async () => { + mocks.accept.mockImplementation(async (input: { items: unknown[] }) => ({ + operation: { ...input, state: 'finished' as const }, + claimedItemIndexes: [] as number[], + })); + mocks.snapshot.mockImplementation(async (operation: unknown) => operation); + + const operation = (await startSessionChatManyOperation({ + operationId: 'batch-chat-operation', + items: [{ sessionId: targetSession.id, prompt: 'continue' }], + })) as { items: { status: string; error: unknown }[] }; + + expect(operation.items).toEqual([ + { + status: 'failed', + error: { code: 'DAEMON_BUSY', message: DAEMON_BUSY_MESSAGE, retryable: true }, + }, + ]); + }); +}); diff --git a/apps/cli/src/mcp/lody-mcp-server.ts b/apps/cli/src/mcp/lody-mcp-server.ts index 3b8433b3..9abaeceb 100644 --- a/apps/cli/src/mcp/lody-mcp-server.ts +++ b/apps/cli/src/mcp/lody-mcp-server.ts @@ -2869,7 +2869,13 @@ const startSessionChatOperation = async (args: SessionChatToolInput): Promise