Merge #782 fix(approvals): the list leaves a dialog too new to judge

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer committed 2026-11-08 04:04:59 +01:00
commit 49e1dda23b
3 files changed
+36 -3

No files matched your search

+15
View File
@@ -373,6 +373,21 @@ export class ApprovalInbox {
return true;
}
/**
* The list route's sweep: `verifyStillAnswerable`, but never before the delay
* `notePrompt` waits out. A frame that shows no dialog proves nothing about a
* dialog Ink has not painted yet, and the working line still on screen then
* would resolve a live prompt, clearing its red alert on every device. The
* list used to run once per page load, which made that rare; a client that
* polls it (the macOS app's logo) would hit it on every new dialog. Each
* call on an older dialog costs one synchronous pane capture.
*/
sweepForList(id: string): boolean {
const item = this.getById(id);
if (item && Date.now() - item.createdAt < STALE_CHECK_DELAY_MS) return true;
return this.verifyStillAnswerable(id);
}
/**
* "This session's pane started moving again": re-verify its pending DIALOG
* item against the screen and resolve it if the dialog is gone.
+3 -2
View File
@@ -84,8 +84,9 @@ export function registerApprovalRoutes(app: FastifyInstance, ctx: SessionPort):
// conservative rule the answer path uses (`verifyStillAnswerable`): only
// an item whose original frame parsed options can be resolved this way, so
// an unreadable capture keeps the alert rather than dropping it. Resolving
// here broadcasts `approval:resolved`, so the other devices clear too.
return approvalInbox.verifyStillAnswerable(item.id);
// here broadcasts `approval:resolved`, so the other devices clear too. A
// dialog too new to judge is left alone (`sweepForList`).
return approvalInbox.sweepForList(item.id);
});
return { success: true, data: { approvals } };
});
+18 -1
View File
@@ -7,7 +7,7 @@
* The routes read the process-wide `approvalInbox` singleton, so every test
* drains it in afterEach; a leaked pending item would bleed into the next test.
*/
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import Fastify, { type FastifyInstance } from 'fastify';
import fastifyCookie from '@fastify/cookie';
import { registerApprovalRoutes } from '../../src/web/routes/approval-routes.js';
@@ -285,6 +285,10 @@ describe('approval routes', () => {
});
describe('staleness sweep on GET /api/approvals', () => {
/** Past the delay `notePrompt` waits before it judges a new dialog. */
const pastTheDelay = () => vi.spyOn(Date, 'now').mockReturnValue(Date.now() + 3_100);
afterEach(() => vi.restoreAllMocks());
it('resolves an item whose dialog left the pane, and tells the other clients', async () => {
const resolved: Array<Record<string, unknown>> = [];
await postHook(harness, 'permission_prompt', { tool_name: 'Bash' });
@@ -294,11 +298,24 @@ describe('approval routes', () => {
// the pane knows. The dialog is gone from the frame the next capture sees.
approvalInbox.onResolved = (info) => resolved.push({ ...info });
session.terminalBuffer = 'claude> back at the composer';
pastTheDelay();
expect(await listApprovals(harness)).toHaveLength(0);
expect(resolved).toEqual([expect.objectContaining({ resolution: 'resolved_in_terminal' })]);
});
it('leaves a dialog too new to judge, however often it is listed', async () => {
// The hook beat Ink's paint: the frame still shows the turn, which alone
// would count as proof that no dialog is up.
session.terminalBuffer = '✻ Actualizing… (13s · esc to interrupt)';
await postHook(harness, 'permission_prompt', { tool_name: 'Bash' });
expect(await listApprovals(harness)).toHaveLength(1);
expect(await listApprovals(harness)).toHaveLength(1);
pastTheDelay();
expect(await listApprovals(harness)).toHaveLength(0);
});
it('keeps an item whose dialog is still on screen', async () => {
await postHook(harness, 'permission_prompt', { tool_name: 'Bash' });
// Pane unchanged (PERMISSION_DIALOG): the human has not answered yet.