Skip to content

Commit 6ab3787

Browse files
committed
Fix empty fields causing permission switching to fail
1 parent 7482a2d commit 6ab3787

3 files changed

Lines changed: 77 additions & 3 deletions

File tree

packages/codingcode/src/agent/agent.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ export const sendMessage = (
3131
llm: LLMClient,
3232
options?: {
3333
signal?: AbortSignal;
34+
approvalOverride?: any;
3435
}
3536
) =>
3637
Effect.gen(function* () {
@@ -60,6 +61,7 @@ export const sendMessage = (
6061
llm,
6162
skillInstruction: matchedSkill?.instruction,
6263
abortSignal: options?.signal,
64+
approvalOverride: options?.approvalOverride,
6365
});
6466

6567
return { stream, sessionId: sid };

packages/codingcode/src/server/routes/messages.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,9 @@ messagesRouter.post('/sessions/:id/messages', async (c) => {
4646
);
4747
await Effect.runPromise(forked.setPermissionMode(mode));
4848
approvalOverride = forked;
49-
registerSessionApproval(sessionId, forked);
49+
registerSessionApproval(sessionId, {
50+
setPermissionMode: (m) => Effect.runPromise(forked.setPermissionMode(m)),
51+
});
5052
}
5153
}
5254

@@ -55,7 +57,7 @@ messagesRouter.post('/sessions/:id/messages', async (c) => {
5557
input,
5658
normalizedCwd,
5759
llm,
58-
{ signal: c.req.raw.signal }
60+
{ signal: c.req.raw.signal, approvalOverride }
5961
);
6062

6163
const result = await runWithLayer(program);
@@ -75,7 +77,9 @@ messagesRouter.post('/sessions/:id/messages', async (c) => {
7577
}).pipe(Effect.provide(AppLayer) as any)
7678
);
7779
approvalOverride = forked;
78-
registerSessionApproval(sessionId, forked);
80+
registerSessionApproval(sessionId, {
81+
setPermissionMode: (m) => Effect.runPromise(forked.setPermissionMode(m)),
82+
});
7983
}
8084

8185
return sseHandler(
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import { describe, it, expect, beforeEach } from 'vitest';
2+
import { Effect } from 'effect';
3+
import {
4+
registerSessionApproval,
5+
unregisterSessionApproval,
6+
updateSessionPermissionMode,
7+
} from '../../src/approval/session-registry.js';
8+
import type { PermissionMode } from '../../src/approval/types.js';
9+
10+
describe('session-registry', () => {
11+
beforeEach(() => {
12+
// Clean up any leftover registrations
13+
for (const key of Array.from((globalThis as any).__testForks?.keys() ?? [])) {
14+
unregisterSessionApproval(key);
15+
}
16+
});
17+
18+
it('updateSessionPermissionMode returns false for unregistered session', () => {
19+
expect(updateSessionPermissionMode('nonexistent', 'bypass')).toBe(false);
20+
});
21+
22+
it('updateSessionPermissionMode returns true for registered session', () => {
23+
let receivedMode: PermissionMode | null = null;
24+
registerSessionApproval('test-session', {
25+
setPermissionMode: (mode) => {
26+
receivedMode = mode;
27+
},
28+
});
29+
expect(updateSessionPermissionMode('test-session', 'bypass')).toBe(true);
30+
expect(receivedMode).toBe('bypass');
31+
unregisterSessionApproval('test-session');
32+
});
33+
34+
it('wraps Effect-returning setPermissionMode correctly', async () => {
35+
let currentMode: PermissionMode = 'default';
36+
// Simulate the real forked ApprovalService: setPermissionMode returns an Effect
37+
const fakeFork = {
38+
setPermissionMode: (mode: PermissionMode): Effect.Effect<void> =>
39+
Effect.sync(() => {
40+
currentMode = mode;
41+
}),
42+
};
43+
// Register with wrapper that runs the Effect (same pattern as messages.ts)
44+
registerSessionApproval('effect-session', {
45+
setPermissionMode: (m) => Effect.runPromise(fakeFork.setPermissionMode(m)),
46+
});
47+
48+
expect(currentMode).toBe('default');
49+
const result = updateSessionPermissionMode('effect-session', 'bypass');
50+
// The wrapper returns a Promise, but updateSessionPermissionMode doesn't await it
51+
// We need to wait for the microtask
52+
expect(result).toBe(true);
53+
// Allow the microtask queue to flush
54+
await new Promise((r) => setTimeout(r, 0));
55+
expect(currentMode).toBe('bypass');
56+
57+
unregisterSessionApproval('effect-session');
58+
});
59+
60+
it('unregisterSessionApproval removes the session', () => {
61+
registerSessionApproval('temp-session', {
62+
setPermissionMode: () => {},
63+
});
64+
expect(updateSessionPermissionMode('temp-session', 'plan')).toBe(true);
65+
unregisterSessionApproval('temp-session');
66+
expect(updateSessionPermissionMode('temp-session', 'plan')).toBe(false);
67+
});
68+
});

0 commit comments

Comments
 (0)