diff --git a/CLAUDE.md b/CLAUDE.md index 09f17949..5b8961b7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -206,7 +206,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Hook events**: Claude Code hooks trigger via `/api/hook-event`. Key events: `permission_prompt`, `elicitation_dialog`, `elicitation_complete`, `elicitation_response`, `idle_prompt`, `stop`, `teammate_idle`, `task_completed`. See `src/hooks-config.ts`; upstream hook semantics mirrored in `docs/claude-code-hooks-reference.md`. -**Approvals Inbox** (cross-session queue of prompts waiting on a human; `approvalsInboxEnabled`, SYNCED, default OFF: every UI surface is opt-in, though the store/endpoints/push actions run regardless): `web/approval-inbox.ts` is a `sessionWaits`-style singleton fed by `/api/hook-event`, holding at most ONE item per session (a new prompt supersedes), claude-mode only, in-memory. Cards are answered via `POST /api/approvals/:id/answer`, which sends a digit / Esc / idle-prompt text through `writeViaMux` (menu answers never carry `\r`). ⚠️ `option` digits are accepted ONLY when they match options parsed from the captured pane frame, and the answer path RE-CAPTURES the pane first (a dialog that no longer parses on screen means the keystroke would land in the composer, so refuse with 409). ⚠️ Resolution on the heuristic `working` signal is restricted to `idle` items; permission/question items clear only on definitive signals (`stop`, `elicitation_complete`/`elicitation_response`, exit/delete, answer, supersede, 12h TTL). The frontend seeds from `GET /api/approvals` in `handleInit` (which is what makes tab alerts survive reloads), but only with the setting ON; push Approve/Deny actions are answered from `sw.js` directly so they work with no tab open, setting-independent. Surfaces (all gated on the setting): header bell (marker-hidden until count > 0, phones never show it) + drawer (`approvals-ui.js`), phone overview NEEDS YOU answer strips (`mobile-overview.js`). Design: `docs/approvals-inbox-plan.md`. +**Approvals Inbox** (cross-session queue of prompts waiting on a human; `approvalsInboxEnabled`, SYNCED, default OFF: every surface is opt-in; only the store and answer endpoints run regardless, so flipping it ON shows anything already pending): `web/approval-inbox.ts` is a `sessionWaits`-style singleton fed by `/api/hook-event`, holding at most ONE item per session (a new prompt supersedes), claude-mode only, in-memory. Cards are answered via `POST /api/approvals/:id/answer`, which sends a digit / Esc / idle-prompt text through `writeViaMux` (menu answers never carry `\r`). ⚠️ `option` digits are accepted ONLY when they match options parsed from the captured pane frame, and the answer path RE-CAPTURES the pane first (a dialog that no longer parses on screen means the keystroke would land in the composer, so refuse with 409). ⚠️ Resolution on the heuristic `working` signal is restricted to `idle` items; permission/question items clear only on definitive signals (`stop`, `elicitation_complete`/`elicitation_response`, exit/delete, answer, supersede, 12h TTL). The frontend seeds from `GET /api/approvals` in `handleInit` (which is what makes tab alerts survive reloads), but only with the setting ON; push Approve/Deny buttons are also gated on it (`sendPushNotifications` strips `actions`/`approvalId` when OFF) and are answered from `sw.js` directly so they work with no tab open. Surfaces (all gated on the setting): header bell (marker-hidden until count > 0, phones never show it) + drawer (`approvals-ui.js`), phone overview NEEDS YOU answer strips (`mobile-overview.js`). Design: `docs/approvals-inbox-plan.md`. **Agent Teams**: `TeamWatcher` polls `~/.claude/teams/`, matches to sessions via `leadSessionId`. Teammates are in-process threads appearing as subagents. Enable: `CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1`. See `docs/agent-teams/`. diff --git a/docs/approvals-inbox-plan.md b/docs/approvals-inbox-plan.md index 02602f4b..7efa96db 100644 --- a/docs/approvals-inbox-plan.md +++ b/docs/approvals-inbox-plan.md @@ -75,9 +75,9 @@ Normal authed API (NOT the hook-secret bypass), `ApiResponse` envelope, Zod sche ### Push -- `sendPushNotifications` payload gains `approvalId` for the three hook events. +- `sendPushNotifications` payload gains `approvalId` for the three hook events. Both `approvalId` and the Approve/Deny `actions` are **gated on the opt-in setting**: with it off, permission pushes carry no buttons at all (pre-inbox they rendered and did nothing, so stripping them is the honest shape). - `sw.js` `notificationclick`: when `event.action` is `approve`/`deny`, POST `/api/approvals/:id/answer` directly from the worker (same-origin, cookie credentials) so the buttons work **with no tab open**; on failure fall back to focusing/opening a tab. Non-action clicks keep today's behavior. -- Page-side `notification-click` handler: honor `action` instead of dropping it. +- Page-side `notification-click` handler: honor `action` instead of dropping it (also setting-gated, for stale notifications sent before the toggle flipped). - Question/idle pushes keep no action buttons (options vary per dialog); tapping opens the inbox. ## Frontend @@ -88,7 +88,7 @@ New module `approvals-ui.js` (@loadorder 11.2, after panels-ui.js), prettier-for - **Desktop**: header bell `btn-approvals` with count badge. Ships default-hidden via marker class `btn-approvals--hidden` (same policy as the attachments button, so `test/mobile-header-buttons-policy.test.ts` excludes it from the default-visible enumeration); JS shows it only while count > 0. Click toggles a drawer of cards: session name + kind, tool/message summary, mono context block, buttons rendered from parsed options (else Approve/Deny), plus Dismiss and Open session. Esc closes; existing z-index layers respected. - **Phone**: header button stays hidden (`mobile.css`); the phone surface is the overview's NEEDS YOU section, whose rows gain inline ✓/✗ buttons for permission items (tap-through to the session remains the row's main action). Toolbar classes/status language rules from the mobile-overview section of CLAUDE.md apply. - **i18n**: new strings registered in i18n.js (en + zh-CN); status words carry `data-i18n-skip` where they would collide (mirroring the overview pills). -- **Setting**: `approvalsInboxEnabled`, synced (in `SettingsUpdateSchema`), **default OFF** (owner decision: every UI surface is opt-in, meaning no bell, no drawer, no overview strips, no seeding until enabled in App Settings → Panels). The store and answer endpoints keep running regardless, so the push Approve/Deny actions work either way (they are already opt-in per-subscription via push preferences). +- **Setting**: `approvalsInboxEnabled`, synced (in `SettingsUpdateSchema`), **default OFF** (owner decision: the entire feature is opt-in, meaning no bell, no drawer, no overview strips, no seeding, and no push action buttons until enabled in App Settings → Panels). Only the store and answer endpoints keep running regardless, so flipping the toggle ON surfaces anything already pending immediately, with no restart. ## Race honesty diff --git a/src/web/public/approvals-ui.js b/src/web/public/approvals-ui.js index 97c4a5c0..066ad17e 100644 --- a/src/web/public/approvals-ui.js +++ b/src/web/public/approvals-ui.js @@ -126,10 +126,12 @@ Object.assign(CodemanApp.prototype, { /** * Push-notification action relay (sw.js → settings-ui notification-click → - * here). Falls back to opening the session when the item is unknown. + * here). Falls back to opening the session when the item is unknown, or + * when the inbox is disabled (a stale notification from before the toggle + * flipped can still carry an action). */ handleNotificationAction(action, approvalId, sessionId) { - if ((action === 'approve' || action === 'deny') && approvalId) { + if ((action === 'approve' || action === 'deny') && approvalId && this.approvalsInboxEnabled()) { this.answerApproval(approvalId, action); return; } diff --git a/src/web/schemas.ts b/src/web/schemas.ts index 5bbc3239..675f9d0f 100644 --- a/src/web/schemas.ts +++ b/src/web/schemas.ts @@ -791,10 +791,12 @@ export const SettingsUpdateSchema = z */ agentSkillEnabled: z.boolean().optional(), /** - * Approvals Inbox UI (header bell + drawer, phone overview answer buttons). - * SYNCED, default OFF (opt-in): even with items pending, no surface renders - * until this is enabled. The server-side store and answer endpoints run - * regardless, so push Approve/Deny actions keep working either way. + * Approvals Inbox (header bell + drawer, phone overview answer buttons, + * push Approve/Deny action buttons). SYNCED, default OFF (opt-in): even + * with items pending, no surface renders and push payloads carry no + * actions/approvalId until this is enabled. The server-side store and the + * answer endpoints run regardless, so flipping it ON shows anything + * already pending immediately. */ approvalsInboxEnabled: z.boolean().optional(), tunnelEnabled: z.boolean().optional(), diff --git a/src/web/server.ts b/src/web/server.ts index 3edc1b2f..b46d06ea 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -2104,13 +2104,27 @@ export class WebServer extends EventEmitter { * Only events in PUSH_EVENT_MAP trigger push. Per-subscription preferences are checked. * Expired subscriptions (410/404) are auto-removed. */ - private sendPushNotifications(event: string, data: Record): void { + // Async only for the Approvals Inbox settings read below; every call site is + // fire-and-forget (the EventPort signature stays `void`). + private async sendPushNotifications(event: string, data: Record): Promise { const template = WebServer.PUSH_EVENT_MAP[event]; if (!template) return; const subscriptions = this.pushStore.getAll(); if (subscriptions.length === 0) return; + // Approvals Inbox gating: the Approve/Deny action buttons answer through + // the inbox, so both the buttons and the approvalId they act on ship only + // when the OPT-IN `approvalsInboxEnabled` setting is on (default OFF). + // Pre-inbox these buttons rendered and did nothing; stripping them when + // the feature is off is the honest shape. Cheap: the settings read is + // cached (~2s TTL) and only taken for events that carry approval parts. + let approvalsEnabled = false; + if (template.actions || typeof data.approvalId === 'string') { + const settings = await this.readSettings(); + approvalsEnabled = settings.approvalsInboxEnabled === true; + } + const vapidKeys = this.pushStore.getVapidKeys(); webpush.setVapidDetails('mailto:codeman@localhost', vapidKeys.publicKey, vapidKeys.privateKey); @@ -2154,9 +2168,10 @@ export class WebServer extends EventEmitter { sessionId, // Approvals Inbox item id: lets sw.js answer an Approve/Deny action // click directly (POST /api/approvals/:id/answer) with no tab open. - approvalId: typeof data.approvalId === 'string' ? data.approvalId : undefined, + // Gated on the opt-in setting together with the action buttons. + approvalId: approvalsEnabled && typeof data.approvalId === 'string' ? data.approvalId : undefined, urgency: template.urgency, - actions: template.actions, + actions: approvalsEnabled ? template.actions : undefined, }); for (const sub of subscriptions) { diff --git a/test/push-payload-host-title.test.ts b/test/push-payload-host-title.test.ts index b5c39d85..3bf8c176 100644 --- a/test/push-payload-host-title.test.ts +++ b/test/push-payload-host-title.test.ts @@ -76,11 +76,11 @@ describe('push payload hostTitle (Web Push hostname plumbing)', () => { setVapidDetails.mockClear(); }); - it('includes hostTitle = codeman: in the payload', () => { + it('includes hostTitle = codeman: in the payload', async () => { const server = makeServerWithHost('laptop'); - ( + await ( server as unknown as { - sendPushNotifications: (e: string, d: Record) => void; + sendPushNotifications: (e: string, d: Record) => Promise; } ).sendPushNotifications('hook:idle_prompt', { sessionId: 's-1', @@ -93,11 +93,11 @@ describe('push payload hostTitle (Web Push hostname plumbing)', () => { expect(payload.title).toBe('Waiting for Input'); }); - it('falls back to os.hostname() when --title-hostname is not provided', () => { + it('falls back to os.hostname() when --title-hostname is not provided', async () => { const server = makeServerWithHost(''); // empty -> constructor uses getHostname() - ( + await ( server as unknown as { - sendPushNotifications: (e: string, d: Record) => void; + sendPushNotifications: (e: string, d: Record) => Promise; } ).sendPushNotifications('hook:permission_prompt', { sessionId: 's-2', @@ -111,18 +111,18 @@ describe('push payload hostTitle (Web Push hostname plumbing)', () => { expect(payload.title).toBe('Permission Required'); }); - it('different WebServer instances ship distinct hostTitles', () => { + it('different WebServer instances ship distinct hostTitles', async () => { const a = makeServerWithHost('host-a'); const b = makeServerWithHost('host-b'); - ( + await ( a as unknown as { - sendPushNotifications: (e: string, d: Record) => void; + sendPushNotifications: (e: string, d: Record) => Promise; } ).sendPushNotifications('hook:stop', { sessionId: 's-a', sessionName: 'A' }); - ( + await ( b as unknown as { - sendPushNotifications: (e: string, d: Record) => void; + sendPushNotifications: (e: string, d: Record) => Promise; } ).sendPushNotifications('hook:stop', { sessionId: 's-b', sessionName: 'B' }); @@ -164,3 +164,66 @@ describe('service worker displayTitle composition (mirrors sw.js)', () => { expect(computeSwDisplayTitle({})).toBe('Codeman'); }); }); + +// ─── Approvals Inbox gating ────────────────────────────────────────────── +// The Approve/Deny action buttons answer through the Approvals Inbox, so the +// payload ships them (and the approvalId they act on) only when the OPT-IN +// `approvalsInboxEnabled` setting is on. Pre-inbox these buttons rendered and +// did nothing; with the feature off they must not render at all. + +interface ApprovalAwarePayload extends PushPayload { + approvalId?: string; +} + +function setSettings(server: WebServer, settings: Record): void { + (server as unknown as { readSettings: () => Promise> }).readSettings = async () => settings; +} + +async function sendPermissionPush(server: WebServer): Promise { + await ( + server as unknown as { + sendPushNotifications: (e: string, d: Record) => Promise; + } + ).sendPushNotifications('hook:permission_prompt', { + sessionId: 's-gate', + sessionName: 'sess', + tool_name: 'Bash', + approvalId: 's-gate:1', + }); + return lastPayload() as ApprovalAwarePayload; +} + +describe('push payload Approvals Inbox gating', () => { + beforeEach(() => { + sendNotification.mockClear(); + }); + + it('strips actions and approvalId when the setting is off (the default)', async () => { + const server = makeServerWithHost('gate-off'); + setSettings(server, {}); + const payload = await sendPermissionPush(server); + expect(payload.actions).toBeUndefined(); + expect(payload.approvalId).toBeUndefined(); + // The notification itself still goes out; only the inbox parts are gated. + expect(payload.title).toBe('Permission Required'); + }); + + it('ships Approve/Deny actions and the approvalId when the setting is on', async () => { + const server = makeServerWithHost('gate-on'); + setSettings(server, { approvalsInboxEnabled: true }); + const payload = await sendPermissionPush(server); + expect(payload.actions).toEqual([ + { action: 'approve', title: 'Approve' }, + { action: 'deny', title: 'Deny' }, + ]); + expect(payload.approvalId).toBe('s-gate:1'); + }); + + it('an explicit false behaves like the default (only true enables)', async () => { + const server = makeServerWithHost('gate-false'); + setSettings(server, { approvalsInboxEnabled: false }); + const payload = await sendPermissionPush(server); + expect(payload.actions).toBeUndefined(); + expect(payload.approvalId).toBeUndefined(); + }); +});