Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions apps/cli/src/mcp/lody-mcp-server-chat-sync.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const mocks = vi.hoisted(() => ({
activeInvocation: vi.fn(),
findMatchingRetry: vi.fn(),
getDocMeta: vi.fn(),
snapshot: vi.fn(),
validateSessionChatTarget: vi.fn(),
}));

Expand Down Expand Up @@ -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()),
};
Expand All @@ -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';
Expand Down Expand Up @@ -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 },
},
]);
});
});
11 changes: 10 additions & 1 deletion apps/cli/src/mcp/lody-mcp-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2869,7 +2869,13 @@ const startSessionChatOperation = async (args: SessionChatToolInput): Promise<un
delegatedRequester: toDelegatedSessionRequester(invoking.identity),
});
} catch (error) {
if (error instanceof WorkspaceSyncUnavailableError) {
// ensureTargetMachineOnline reaches classifyLocalDaemonIpcError for a target on
// this machine, so chat prevalidation raises the daemon error as well as the sync
// one; session_create already passes the daemon error through.
if (
error instanceof WorkspaceSyncUnavailableError ||
error instanceof LocalDaemonAvailabilityError
) {
throw error;
}
throw new LodyOperationStoreError('COMMAND_REJECTED', formatMcpErrorMessage(error), false);
Expand Down Expand Up @@ -3446,6 +3452,9 @@ const startSessionChatManyOperation = async (args: SessionChatManyToolInput): Pr
if (error instanceof WorkspaceSyncUnavailableError) {
throw error;
}
if (error instanceof LocalDaemonAvailabilityError) {
return batchFailure(error.code, error.message, error.retryable, item.label);
Comment on lines +3455 to +3456

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep retryable batch failures outside the finished operation

When local prevalidation returns DAEMON_BUSY, this stores the item as terminally failed; finishOperationWhenEveryItemIsTerminal then finishes the Operation, and findMatchingRetry returns that finished snapshot on every retry. This contradicts DAEMON_BUSY_MESSAGE, which instructs the caller to reuse the same operationId, so the prompt can never be retried after the daemon recovers. A retryable dependency failure must remain recoverable under that ID or abort before accepting the batch rather than becoming a terminal item.

Useful? React with 👍 / 👎.

}
return batchFailure('INVALID_ITEM', formatMcpErrorMessage(error), false, item.label);
}
return activeOperationItem(target.id, randomUUID(), item.label);
Expand Down
Loading