Skip to content
Merged
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
22 changes: 22 additions & 0 deletions assistant/src/__tests__/recording-handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,28 @@ describe('handleRecordingStop', () => {
expect(result).toBeUndefined();
});

test('resolves to globally active recording from a different conversation', () => {
const { ctx, sent, fakeSocket } = createCtx();
const convA = 'conv-owner';
const convB = 'conv-stopper';

// Bind socket to conv-A (the owning conversation)
ctx.socketToSession.set(fakeSocket, convA);

// Start a recording on conv-A
const recordingId = handleRecordingStart(convA, undefined, fakeSocket, ctx);
expect(recordingId).not.toBeNull();
sent.length = 0;

// Stop from conv-B — should resolve to the globally active recording on conv-A
const result = handleRecordingStop(convB, ctx);

expect(result).toBe(recordingId!);
expect(sent).toHaveLength(1);
expect(sent[0].type).toBe('recording_stop');
expect(sent[0].recordingId).toBe(recordingId!);
});

test('returns undefined when no socket bound to conversation', () => {
const { ctx, fakeSocket } = createCtx();
const conversationId = 'conv-no-socket';
Expand Down
36 changes: 25 additions & 11 deletions assistant/src/daemon/handlers/recording.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,30 +63,44 @@ export function handleRecordingStart(
// ─── Stop ────────────────────────────────────────────────────────────────────

/**
* Stop the active standalone recording for a conversation.
* Looks up the recording ID from `recordingOwnerByConversation` and sends
* a `recording_stop` message to the client.
* Stop the active standalone recording.
* First checks if the given conversation owns a recording; if not, falls back
* to the globally active recording (since only one can be active at a time).
* This allows users to stop a recording from a different conversation than
* the one that started it.
*
* Returns the recording ID if a stop was sent, or `undefined` if no active
* recording was found for the conversation.
* recording exists.
*/
export function handleRecordingStop(
conversationId: string,
ctx: HandlerContext,
): string | undefined {
const recordingId = recordingOwnerByConversation.get(conversationId);
let recordingId = recordingOwnerByConversation.get(conversationId);
let ownerConversationId = conversationId;

// Global fallback: since only one recording can be active at a time,
// resolve globally if the current conversation doesn't own a recording.
if (!recordingId && recordingOwnerByConversation.size > 0) {
const [activeConv, activeRec] = [...recordingOwnerByConversation.entries()][0];
recordingId = activeRec;
ownerConversationId = activeConv;
log.info({ conversationId, ownerConversationId, resolvedRecordingId: recordingId }, 'Resolved stop to globally active recording');
}

if (!recordingId) {
log.debug({ conversationId }, 'No active standalone recording to stop for conversation');
log.debug({ conversationId }, 'No active standalone recording to stop');
return undefined;
}

// Look up the socket currently bound to the conversation so we can send
// the stop command to the correct client connection.
const socket = findSocketForSession(conversationId, ctx);
// Look up the socket currently bound to the owning conversation so we can
// send the stop command to the correct client connection.
const socket = findSocketForSession(ownerConversationId, ctx)
?? findSocketForSession(conversationId, ctx);
Comment on lines +98 to +99
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 Send stop only to the recording owner socket

The new fallback ?? findSocketForSession(conversationId, ctx) can route recording_stop to a different connected client when the owner conversation is no longer bound (for example, owner socket dropped while another socket is bound to the requesting conversation). In that case the recipient may not own the recording, and RecordingManager.stop(sessionId:) exits early when ownerSessionId does not match (clients/macos/vellum-assistant/ComputerUse/RecordingManager.swift:119-123), so no recording_status cleanup comes back and the daemon can remain stuck with an "active" recording while telling the requester it is stopping.

Useful? React with 👍 / 👎.

if (!socket) {
log.warn({ conversationId, recordingId }, 'Cannot send recording_stop: no socket bound to conversation');
log.warn({ conversationId, ownerConversationId, recordingId }, 'Cannot send recording_stop: no socket bound to conversation');
standaloneRecordingConversationId.delete(recordingId);
recordingOwnerByConversation.delete(conversationId);
recordingOwnerByConversation.delete(ownerConversationId);
return undefined;
}

Expand Down