From 6c744f8677eb33a0a2936f605ad8f7a2892bef61 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sun, 9 Aug 2026 15:55:51 +0200 Subject: [PATCH] feat(approvals): make the inbox opt-in (default OFF) and drop em-dashes Owner decision: every Approvals Inbox UI surface (header bell, drawer, phone overview answer strips, reload seeding) now requires enabling approvalsInboxEnabled in App Settings -> Panels; only an explicit true turns it on. The store, endpoints, and push Approve/Deny actions keep running regardless (the push buttons are already opt-in per subscription). Also replaces em-dashes with plain punctuation across the newly authored comments, docs, and strings. Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 2 +- docs/approvals-inbox-plan.md | 2 +- src/web/approval-inbox.ts | 22 +++++++++++----------- src/web/public/approvals-ui.js | 26 ++++++++++++++------------ src/web/public/mobile.css | 2 +- src/web/public/settings-ui.js | 8 ++++---- src/web/public/styles.css | 4 ++-- src/web/public/sw.js | 2 +- src/web/routes/approval-routes.ts | 12 ++++++------ src/web/schemas.ts | 9 +++++---- src/web/server.ts | 2 +- test/approval-inbox.test.ts | 4 ++-- test/routes/approval-routes.test.ts | 10 +++++----- 13 files changed, 54 insertions(+), 51 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 3cb69c82..09f17949 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 ON): `web/approval-inbox.ts` is a `sessionWaits`-style singleton fed by `/api/hook-event` — 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 — 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; push Approve/Deny actions are answered from `sw.js` directly so they work with no tab open. Surfaces: 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 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`. **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 1503ba02..02602f4b 100644 --- a/docs/approvals-inbox-plan.md +++ b/docs/approvals-inbox-plan.md @@ -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 ON, resolved from `merged` per the partial-PUT rule. OFF hides the UI surfaces and stops seeding; the store itself keeps running (harmless, and push actions keep working). +- **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). ## Race honesty diff --git a/src/web/approval-inbox.ts b/src/web/approval-inbox.ts index 9a3a67a6..97e34411 100644 --- a/src/web/approval-inbox.ts +++ b/src/web/approval-inbox.ts @@ -1,5 +1,5 @@ /** - * @fileoverview Approvals Inbox — server-side registry of prompts waiting on a human. + * @fileoverview Approvals Inbox: server-side registry of prompts waiting on a human. * * One cross-session queue of pending Claude prompts (permission dialogs, * AskUserQuestion/elicitation questions, idle prompts), fed by `/api/hook-event` @@ -48,7 +48,7 @@ export interface ApprovalOption { } export interface ApprovalItem { - /** `${sessionId}:${seq}` — stable across re-captures, unique per prompt. */ + /** `${sessionId}:${seq}`, stable across re-captures, unique per prompt. */ id: string; sessionId: string; sessionName: string; @@ -96,7 +96,7 @@ const ITEM_TTL_MS = 12 * 60 * 60 * 1000; * single delayed re-capture picks up the frame the immediate capture missed. */ const RECAPTURE_DELAY_MS = 600; -/** Context kept per item — enough for a dialog plus a few lines above it. */ +/** Context kept per item: enough for a dialog plus a few lines above it. */ const MAX_CONTEXT_CHARS = 4000; const MAX_CONTEXT_LINES = 30; const MAX_OPTION_LABEL_CHARS = 120; @@ -144,9 +144,9 @@ export function normalizeCapturedFrame(raw: string | null | undefined): string | * Options must be consecutively numbered from 1 (2..6 of them); description / * wrap / separator lines between options are tolerated up to a small gap * (AskUserQuestion puts a description under every option and a ─ separator - * before its "Chat about this" entry — measured against the live dialog). The + * before its "Chat about this" entry, measured against the live dialog). The * LAST complete block in the frame wins (dialogs render at the bottom). - * Returns undefined when nothing parses — callers then fall back to + * Returns undefined when nothing parses; callers then fall back to * approve/deny only, so a mis-parse can never route a digit at a dialog that * does not have it. */ @@ -171,7 +171,7 @@ export function parseDialogOptions(context: string | undefined): ApprovalOption[ commit(); run = [{ n: 1, label: m[2].trim().slice(0, MAX_OPTION_LABEL_CHARS) }]; } else if (run.length > 0 && ++gap > 3) { - // Too far past the last option for this to still be its description — + // Too far past the last option for this to still be its description: // the block is over. commit(); } @@ -183,7 +183,7 @@ export function parseDialogOptions(context: string | undefined): ApprovalOption[ // ─── Registry ──────────────────────────────────────────────────────────────── export class ApprovalInbox { - /** Keyed by sessionId — the one-active-item-per-session invariant lives here. */ + /** Keyed by sessionId; the one-active-item-per-session invariant lives here. */ private items = new Map(); private recaptureTimers = new Map>(); /** Capture callbacks kept for answer-time re-verification; dropped on remove. */ @@ -235,7 +235,7 @@ export class ApprovalInbox { * Answer-time guard: re-capture the pane and check the dialog is still on * screen before keystrokes are sent at it. Only conclusive when the ORIGINAL * frame parsed options: if a fresh capture then parses none, the dialog is - * gone (answered in the terminal moments ago) — the item resolves and the + * gone (answered in the terminal moments ago), so the item resolves and the * answer must be refused, because the digit would land in whatever now has * focus. Unparseable-from-the-start items stay answerable (approve/deny * only), same risk the terminal user already carries. @@ -250,7 +250,7 @@ export class ApprovalInbox { try { raw = capture(); } catch { - return true; // capture hiccup — inconclusive, keep the item answerable + return true; // capture hiccup: inconclusive, keep the item answerable } const context = normalizeCapturedFrame(raw); if (!context) return true; @@ -316,7 +316,7 @@ export class ApprovalInbox { /** * Resolve a session's pending item, if any (stop hook, exit, ...). `kinds` - * restricts which item kinds the signal may clear — the heuristic `working` + * restricts which item kinds the signal may clear: the heuristic `working` * transition passes `['idle']` so a mid-turn flap cannot false-clear a * pending permission/question dialog. */ @@ -347,7 +347,7 @@ export class ApprovalInbox { const context = normalizeCapturedFrame(raw); if (!context) return; item.context = context; - // Idle prompts are not dialogs — never offer digit answers for them. + // Idle prompts are not dialogs; never offer digit answers for them. if (item.kind !== 'idle') item.options = parseDialogOptions(context); } diff --git a/src/web/public/approvals-ui.js b/src/web/public/approvals-ui.js index a760100d..97c4a5c0 100644 --- a/src/web/public/approvals-ui.js +++ b/src/web/public/approvals-ui.js @@ -1,10 +1,12 @@ /** - * @fileoverview Approvals Inbox UI — cross-session queue of prompts waiting on a human. + * @fileoverview Approvals Inbox UI: cross-session queue of prompts waiting on a human. * - * Renders the header bell (count badge, shown only while items are pending) and - * the right-side drawer of approval cards, seeds pending items from - * `GET /api/approvals` on init/reconnect (so tab alerts survive a reload), and - * answers items in place via `POST /api/approvals/:id/answer`. Cards render + * Everything here is gated on the OPT-IN `approvalsInboxEnabled` setting + * (synced, default OFF): with it off, no bell, no drawer, no overview strips, + * no seeding. When on, the header bell renders only while items are pending + * (count badge), opening a right-side drawer of approval cards; pending items + * are seeded from `GET /api/approvals` on init/reconnect (so tab alerts + * survive a reload) and answered in place via `POST /api/approvals/:id/answer`. Cards render * buttons from the server-parsed dialog options; without parsed options they * fall back to Approve/Deny (permission/question) or a text prompt (idle). * Backend: src/web/approval-inbox.ts, design: docs/approvals-inbox-plan.md. @@ -13,7 +15,7 @@ * @dependency app.js (CodemanApp class, this.approvals, setPendingHook/clearPendingHooks, selectSession) * @dependency constants.js (escapeHtml) * @dependency api-client.js at runtime (this._apiJson; loads later but is only called after init) - * @loadorder 11.6 of 17 — after ultracode-panel.js, before admin-ui.js + * @loadorder 11.6 of 17, after ultracode-panel.js, before admin-ui.js */ /** Map an approval kind to the pendingHooks entry that drives tab alerts. */ @@ -22,14 +24,14 @@ function approvalKindToHook(kind) { } Object.assign(CodemanApp.prototype, { - /** Synced setting, default ON (only an explicit false disables). */ + /** Synced setting, default OFF, opt-in via App Settings → Panels. */ approvalsInboxEnabled() { - return this.loadAppSettingsFromStorage().approvalsInboxEnabled !== false; + return this.loadAppSettingsFromStorage().approvalsInboxEnabled === true; }, /** * Seed pending approvals from the server. Called from handleInit, i.e. on - * every page load AND SSE reconnect — this is what makes pending alerts + * every page load AND SSE reconnect; this is what makes pending alerts * survive a reload (pre-inbox they lived only in SSE-transient memory). */ async seedApprovals() { @@ -51,7 +53,7 @@ Object.assign(CodemanApp.prototype, { _onApprovalPending(item) { if (!item || !item.id) return; if (!this.approvals) this.approvals = new Map(); - // One active item per session (server invariant) — drop any stale sibling. + // One active item per session (server invariant): drop any stale sibling. for (const [id, existing] of this.approvals) { if (existing.sessionId === item.sessionId) this.approvals.delete(id); } @@ -88,7 +90,7 @@ Object.assign(CodemanApp.prototype, { this.showToast(action === 'deny' ? 'Denied' : 'Answer sent', 'success'); } else { // 404/409 = resolved elsewhere or the dialog left the screen; refresh truth. - this.showToast('Could not answer — prompt may already be resolved', 'warning'); + this.showToast('Could not answer, the prompt may already be resolved', 'warning'); this.seedApprovals(); } }, @@ -104,7 +106,7 @@ Object.assign(CodemanApp.prototype, { }); if (data) this.showToast('Prompt sent', 'success'); else { - this.showToast('Could not send — session may be busy', 'warning'); + this.showToast('Could not send, the session may be busy', 'warning'); this.seedApprovals(); } }, diff --git a/src/web/public/mobile.css b/src/web/public/mobile.css index 024d6d0c..6d977bc5 100644 --- a/src/web/public/mobile.css +++ b/src/web/public/mobile.css @@ -2509,7 +2509,7 @@ html.mobile-init .file-browser-panel { /* Approvals Inbox answer strip: sits under a NEEDS YOU row (sibling of the row