mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-20 09:09:35 +02:00
Merge pull request #610: fix(approvals): treat DeepSeek permission prompts as terminal-only alerts
This commit is contained in:
commit
0feb5658ca
9 files changed
+119
-24
No files matched your search
@@ -124,10 +124,6 @@ const ALLOWED_BRANCHES: Record<string, Allowance> = {
|
||||
1,
|
||||
'an error message explaining why THIS mode in particular will never deliver a stop signal'
|
||||
),
|
||||
"web/routes/approval-routes.ts::mode === 'deepseek'": allow(
|
||||
1,
|
||||
'the DeepSeek status bridge is the only non-claude source of approval items'
|
||||
),
|
||||
"cron/cron-service.ts::mode === 'claude'": allow(1, 'cron launch handling, not yet generalised'),
|
||||
"cron/cron-service.ts::mode === 'shell'": allow(1, 'cron launch handling, not yet generalised'),
|
||||
"web/routes/session-routes.ts::mode === 'claude'": allow(
|
||||
@@ -169,14 +165,6 @@ const ALLOWED_BRANCHES: Record<string, Allowance> = {
|
||||
),
|
||||
"tui/tui-render.ts::mode !== 'claude'": allow(1, 'cosmetic: suppress the mode badge for the default mode'),
|
||||
|
||||
// Push approve/deny BUTTONS are withheld for dsh because the answer route refuses
|
||||
// keystrokes for its dialogs (third-party TUI, unmeasured contract) — a button whose
|
||||
// answer would be refused is worse than none. Arguably wants an "answerable dialogs"
|
||||
// capability; deliberately not invented here.
|
||||
"web/routes/hook-event-routes.ts::mode !== 'deepseek'": allow(
|
||||
1,
|
||||
'push buttons withheld where the answer route refuses keystrokes'
|
||||
),
|
||||
|
||||
// Legacy <Mode>Config plumbing, same category as the `===` entries above.
|
||||
"web/routes/session-routes.ts::mode !== 'omp'": allow(
|
||||
|
||||
@@ -219,6 +219,27 @@ describe('push payload Approvals Inbox gating', () => {
|
||||
expect(payload.approvalId).toBe('s-gate:1');
|
||||
});
|
||||
|
||||
|
||||
it('ships no Approve/Deny actions when the inbox is on but approvalId is absent', async () => {
|
||||
// Supervised-hook CLIs omit approvalId upstream so buttons cannot answer.
|
||||
const server = makeServerWithHost('gate-no-id');
|
||||
setSettings(server, { approvalsInboxEnabled: true });
|
||||
await (
|
||||
server as unknown as {
|
||||
sendPushNotifications: (e: string, d: Record<string, unknown>) => Promise<void>;
|
||||
}
|
||||
).sendPushNotifications('hook:permission_prompt', {
|
||||
sessionId: 's-gate',
|
||||
sessionName: 'sess',
|
||||
tool_name: 'Bash',
|
||||
// deliberately no approvalId
|
||||
});
|
||||
const payload = lastPayload() as ApprovalAwarePayload;
|
||||
expect(payload.actions).toBeUndefined();
|
||||
expect(payload.approvalId).toBeUndefined();
|
||||
expect(payload.title).toBe('Permission Required');
|
||||
});
|
||||
|
||||
it('an explicit false behaves like the default (only true enables)', async () => {
|
||||
const server = makeServerWithHost('gate-false');
|
||||
setSettings(server, { approvalsInboxEnabled: false });
|
||||
|
||||
@@ -455,6 +455,56 @@ describe('approval routes', () => {
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
describe('approval routes: supervised-hook (DeepSeek) alert-only items', () => {
|
||||
let harness: RouteTestHarness;
|
||||
let session: MockSession;
|
||||
|
||||
beforeEach(async () => {
|
||||
harness = await createEnvelopeHarness();
|
||||
session = harness.ctx.sessions.get(SESSION_ID)!;
|
||||
session.mode = 'deepseek';
|
||||
session.terminalBuffer = 'some dsh dialog frame';
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
for (const item of approvalInbox.listPending()) {
|
||||
approvalInbox.resolveForSession(item.sessionId, 'dismissed');
|
||||
}
|
||||
approvalInbox.onPending = approvalInbox.onUpdated = approvalInbox.onResolved = undefined;
|
||||
await harness.app.close();
|
||||
});
|
||||
|
||||
it('creates an answerInTerminal inbox item and omits approvalId from push', async () => {
|
||||
await postHook(harness, 'permission_prompt', { tool_name: 'Bash' });
|
||||
const approvals = await listApprovals(harness);
|
||||
expect(approvals).toHaveLength(1);
|
||||
expect(approvals[0]).toMatchObject({
|
||||
sessionId: SESSION_ID,
|
||||
kind: 'permission',
|
||||
answerInTerminal: true,
|
||||
});
|
||||
const push = harness.ctx.sendPushNotifications.mock.calls.find((c) => c[0] === 'hook:permission_prompt');
|
||||
expect(push?.[1]).toBeDefined();
|
||||
expect(push?.[1]).not.toHaveProperty('approvalId');
|
||||
});
|
||||
|
||||
it('refuses Approve/Deny with a terminal-only error', async () => {
|
||||
await postHook(harness, 'permission_prompt', {});
|
||||
const [item] = await listApprovals(harness);
|
||||
const res = await harness.app.inject({
|
||||
method: 'POST',
|
||||
url: `/api/approvals/${item.id}/answer`,
|
||||
payload: { action: 'approve' },
|
||||
});
|
||||
expect(res.statusCode).toBe(400);
|
||||
expect(res.json().error).toMatch(/answered in the terminal/i);
|
||||
expect(session.writeBuffer).toEqual([]);
|
||||
// Item stays pending — it is an alert, not a failed answer.
|
||||
expect(await listApprovals(harness)).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('approval routes: multi-user scoping', () => {
|
||||
const saved: Record<string, string | undefined> = {};
|
||||
|
||||
|
||||
Reference in new issue
Block a user