fix(notifications): main Save keeps webhook edits, glue test, docs and nits (#523 review)

Merge-time fixes for the webhook notification channel (ntfy, Slack, Discord, generic JSON).

Minor 1, App Settings Save silently dropped webhook edits: the modal's main Save now
persists the webhook group beside the settings PUT, the same way it already saves the
model config (saveModelConfigFromSettings), but only when the group differs from what
loadWebhook() put on screen (_webhookPending), so an untouched group never re-PUTs. A
refusal (bad URL, enabled with no URL) shows a warning toast, keeps the modal open and
scrolls to the group with the pasted URL still in the box, instead of a success toast.
Send test now saves pending edits first, so it never tests the old URL while the box
shows a new one. The row says so in one line.

Minor 2, no test for the server.ts glue: new test/webhook-push-glue.test.ts drives the
private sendPushNotifications on a real (never started) WebServer with an EMPTY push
store and webhook.json in the instance data dir, delivering through the real
egress-guarded fetch to a local receiver: a permission prompt arrives with the
host-prefixed ntfy Title and body while Web Push is never called, an immediate repeat is
deduped, "response complete" is skipped under scope attention and sent under all, and a
disabled config or a non-push event sends nothing. Verified it fails when the webhook
call is moved below the "no subscriptions" return.

Minor 3, docs: webhook.json added to CLAUDE.md State Files; a Webhooks section in
docs/wiki/Notifications-And-Approvals.md (setup, what is sent, the secret URL, public
ntfy topics, local targets allowed, dedupe, instance-wide reach in multi-user mode) plus
a table row, and a line in Settings-Reference; new section 10c in
docs/security-architecture.md for the second outbound channel through the web-tab
egress guard.

Nits:
- Orphaned JSDoc: the webhook schema moved below the push schemas, so
  PushSubscribeSchema has its comment back.
- Duplicated enums: WebhookUpdateSchema uses z.enum(WEBHOOK_KINDS/WEBHOOK_SCOPES), so
  the schema cannot accept a kind the store would coerce away.
- describeError classifies egress refusals with isEgressBlockedError (the
  CODEMAN_EGRESS_BLOCKED code anywhere in the cause chain) instead of a message regex;
  tests pin a deep cause chain and that matching words alone are not a refusal.
- Markup: the URL input uses set-input, the whitespace-only line is gone, and the switch
  row hints to pick a long random topic on public ntfy.sh.
- Remove a saved URL: a "Remove URL" button (shown only while a URL is saved, with a
  confirm) sends { url: "", enabled: false }.
- Types placement: WEBHOOK_KINDS/SCOPES and WebhookKind/Scope/Urgency/Config/Result/Status
  moved to src/types/push.ts (the IO-side WebhookMessage/Request/Fetch stay in the module).

Browser test extended: main Save persists a pending edit, a refused URL keeps the modal
open with the URL, Send test saves a newly pasted URL first, Remove URL clears it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

src/types/index.ts

@@ -24,7 +24,7 @@
  * | run-summary  | RunSummary, RunSummaryEvent, RunSummaryStats                          | In-memory → `GET /api/sessions/:id/run-summary` |
  * | tools        | ActiveBashTool, ImageDetectedEvent                                    | In-memory, broadcast via SSE                   |
  * | teams        | TeamConfig, TeamMember, TeamTask, InboxMessage, PaneInfo              | `~/.claude/teams/`, `~/.claude/tasks/` → `GET /api/teams` |
- * | push         | PushSubscriptionRecord, VapidKeys                                     | `~/.codeman/push-keys.json`, `~/.codeman/push-subscriptions.json` |
+ * | push         | PushSubscriptionRecord, VapidKeys, WebhookConfig, WebhookStatus, WebhookResult | `~/.codeman/push-keys.json`, `~/.codeman/push-subscriptions.json`, `~/.codeman/webhook.json` |
  * | plan         | PlanItem, PlanTaskStatus, TddPhase                                    | In-memory → `GET /api/sessions/:id/plan/tasks` |
  * | orchestrator | OrchestratorState, OrchestratorPlan, OrchestratorConfig, OrchestratorPersistState | `~/.codeman/state.json` → `GET /api/orchestrator/status` |
  *

src/types/push.ts

@@ -6,13 +6,17 @@
  * Key exports:
  * - PushSubscriptionRecord — a registered push endpoint with per-event preferences
  * - VapidKeys — VAPID key pair (public + private) for Web Push authentication
+ * - WebhookConfig, WebhookStatus, WebhookResult (+ the kind/scope lists): the webhook channel
+ *   (ntfy, Slack, Discord, generic JSON) that carries the same events as Web Push
  *
  * Persistence:
  * - VAPID keys: `~/.codeman/push-keys.json` (auto-generated on first use)
  * - Subscriptions: `~/.codeman/push-subscriptions.json` (expired auto-cleaned on 410/404)
+ * - Webhook: `~/.codeman/webhook.json` (mode 0600; the URL is a bearer secret)
  *
- * Managed by PushStore (`src/push-store.ts`). Served at `GET /api/push/vapid-key`,
- * `POST /api/push/subscribe`. No dependencies on other domain modules.
+ * Push is managed by PushStore (`src/push-store.ts`), served at `GET /api/push/vapid-key`,
+ * `POST /api/push/subscribe`. The webhook is managed by `src/webhook-notify.ts`, served at
+ * `GET`/`PUT /api/webhook` and `POST /api/webhook/test`. No dependencies on other domain modules.
  */
 
 /** A registered push subscription */
@@ -32,3 +36,40 @@ export interface VapidKeys {
   privateKey: string;
   generatedAt: number;
 }
+
+/** Services the webhook channel can format a message for. */
+export const WEBHOOK_KINDS = ['ntfy', 'slack', 'discord', 'generic'] as const;
+export type WebhookKind = (typeof WEBHOOK_KINDS)[number];
+
+/** `attention`: only events that need a human (critical / warning). `all`: also "response complete". */
+export const WEBHOOK_SCOPES = ['attention', 'all'] as const;
+export type WebhookScope = (typeof WEBHOOK_SCOPES)[number];
+
+export type WebhookUrgency = 'critical' | 'warning' | 'info';
+
+/** The stored webhook config (`~/.codeman/webhook.json`). `url` is a secret and is never returned. */
+export interface WebhookConfig {
+  enabled: boolean;
+  kind: WebhookKind;
+  url: string;
+  scope: WebhookScope;
+}
+
+/** One delivery attempt. `error` never contains the URL. */
+export interface WebhookResult {
+  ok: boolean;
+  status?: number;
+  error?: string;
+  at: number;
+}
+
+/** `GET /api/webhook`: the config without its URL, plus the last delivery result. */
+export interface WebhookStatus {
+  enabled: boolean;
+  kind: WebhookKind;
+  scope: WebhookScope;
+  hasUrl: boolean;
+  /** Scheme + host only; the path and query are the secret. */
+  urlMasked: string;
+  lastResult: WebhookResult | null;
+}

src/web/public/index.html

@@ -2654,14 +2654,14 @@
                   </div>
                 </div>
               </div>
-            
+
               <div class="set-group" id="webhookGroup" style="display:none">
                 <div class="set-group-head"><h4>Webhook (ntfy, Slack, Discord)</h4><span class="set-scope">server</span></div>
                 <div class="set-group-body">
                   <div class="set-row" data-search="webhook ntfy slack discord notification phone headless">
                     <div class="set-row-text">
                       <span class="set-row-label">Send alerts to a webhook</span>
-                      <span class="set-row-desc">Posts the same events as push notifications (permission prompts, questions, errors, idle) to ntfy, Slack, Discord or any URL, so a server with no browser open can still reach your phone. The URL is a secret: it is stored on the server only and is never shown again once saved.</span>
+                      <span class="set-row-desc">Posts the same events as push notifications (permission prompts, questions, errors, idle) to ntfy, Slack, Discord or any URL, so a server with no browser open can still reach your phone. The URL is a secret: it is stored on the server only and is never shown again once saved. On public ntfy.sh anyone who guesses the topic can read it, so pick a long random one.</span>
                     </div>
                     <label class="switch switch-sm"><input type="checkbox" id="webhookEnabled"><span class="slider"></span></label>
                   </div>
@@ -2679,7 +2679,7 @@
                       <span class="set-row-label">Webhook URL</span>
                       <span class="set-row-desc" id="webhookUrlHint">Nothing saved yet.</span>
                     </div>
-                    <input type="password" id="webhookUrl" class="set-select" autocomplete="off" spellcheck="false" placeholder="https://ntfy.sh/your-topic">
+                    <input type="password" id="webhookUrl" class="set-input" autocomplete="off" spellcheck="false" placeholder="https://ntfy.sh/your-topic">
                   </div>
                   <div class="set-row has-field">
                     <div class="set-row-text">
@@ -2692,10 +2692,14 @@
                     </select>
                   </div>
                   <div class="set-row">
-                    <div class="set-row-text"><span class="set-row-label">Save and test</span></div>
+                    <div class="set-row-text">
+                      <span class="set-row-label">Save and test</span>
+                      <span class="set-row-desc">The main Settings Save saves this group too. Send test saves pending edits first.</span>
+                    </div>
                     <span>
                       <button class="btn-toolbar btn-sm btn-primary" id="webhookSaveBtn" onclick="app.saveWebhook()">Save</button>
                       <button class="btn-toolbar btn-sm" id="webhookTestBtn" onclick="app.testWebhook()">Send test</button>
+                      <button class="btn-toolbar btn-sm" id="webhookClearBtn" onclick="app.clearWebhook()" style="display:none">Remove URL</button>
                     </span>
                   </div>
                   <div id="webhookResult" class="set-note" style="display:none" data-i18n-skip></div>

src/web/public/settings-ui.js

@@ -1216,6 +1216,12 @@ Object.assign(CodemanApp.prototype, {
    * Webhook notifications (Settings → Notifications). Server-side config behind /api/webhook, not a
    * settings-payload field: the URL is a secret, so it never round-trips through settings.json or
    * this page. The URL box is write-only; the status line shows scheme + host only.
+   *
+   * Three ways to save, one PUT: the group's own Save, Send test (saves pending edits first, so it
+   * never tests the old URL while the box shows a new one), and the modal's main Save, which calls
+   * saveWebhook() beside the settings PUT the same way it saves the model config
+   * (saveModelConfigFromSettings). `_webhookLoaded` is what loadWebhook() put on screen, so
+   * `_webhookPending()` can tell an edited group from an untouched one.
    */
   _webhookSay(text, bad = false) {
     const out = document.getElementById('webhookResult');
@@ -1230,12 +1236,13 @@ Object.assign(CodemanApp.prototype, {
     if (!group) return;
     const res = await this._api('/api/webhook');
     if (!res || !res.ok) {
+      this._webhookLoaded = null;
       group.style.display = 'none'; // not an admin in multi-user mode, or the server predates the route
       return;
     }
     let body = null;
     try { body = await res.json(); } catch { /* leave hidden */ }
-    if (!body || body.success === false) { group.style.display = 'none'; return; }
+    if (!body || body.success === false) { this._webhookLoaded = null; group.style.display = 'none'; return; }
     const d = body.data;
     group.style.display = '';
     document.getElementById('webhookEnabled').checked = d.enabled === true;
@@ -1245,6 +1252,14 @@ Object.assign(CodemanApp.prototype, {
     url.value = '';
     url.placeholder = d.hasUrl ? 'Saved. Paste a new URL to replace it' : 'https://ntfy.sh/your-topic';
     document.getElementById('webhookUrlHint').textContent = d.hasUrl ? `Saved: ${d.urlMasked}` : 'Nothing saved yet.';
+    const clearBtn = document.getElementById('webhookClearBtn');
+    if (clearBtn) clearBtn.style.display = d.hasUrl ? '' : 'none';
+    // Read back from the controls, so a value the <select> does not offer compares as what is shown.
+    this._webhookLoaded = {
+      enabled: document.getElementById('webhookEnabled').checked,
+      kind: document.getElementById('webhookKind').value,
+      scope: document.getElementById('webhookScope').value,
+    };
     if (d.lastResult) {
       const when = new Date(d.lastResult.at).toLocaleString();
       this._webhookSay(
@@ -1256,6 +1271,20 @@ Object.assign(CodemanApp.prototype, {
     }
   },
 
+  /** True when the visible webhook group differs from what loadWebhook() last showed. */
+  _webhookPending() {
+    const group = document.getElementById('webhookGroup');
+    const loaded = this._webhookLoaded;
+    if (!group || group.style.display === 'none' || !loaded) return false;
+    return (
+      document.getElementById('webhookUrl').value.trim() !== '' ||
+      document.getElementById('webhookEnabled').checked !== loaded.enabled ||
+      document.getElementById('webhookKind').value !== loaded.kind ||
+      document.getElementById('webhookScope').value !== loaded.scope
+    );
+  },
+
+  /** PUT the group's state. Resolves to '' on success, else the error (also shown in the group). */
   async saveWebhook() {
     const payload = {
       enabled: document.getElementById('webhookEnabled').checked,
@@ -1263,23 +1292,40 @@ Object.assign(CodemanApp.prototype, {
       scope: document.getElementById('webhookScope').value,
     };
     const url = document.getElementById('webhookUrl').value.trim();
-    if (url) payload.url = url; // blank = keep the saved one
+    if (url) payload.url = url; // blank = keep the saved one (Remove URL is the way to clear it)
     const res = await this._api('/api/webhook', { method: 'PUT', body: payload });
     let body = null;
     try { body = res ? await res.json() : null; } catch { /* fall through */ }
     if (!res || !res.ok || !body || body.success === false) {
-      this._webhookSay(body?.error || 'Could not save the webhook.', true);
-      return;
+      const error = body?.error || 'Could not save the webhook.';
+      this._webhookSay(error, true);
+      return error;
     }
     await this.loadWebhook();
     this._webhookSay('Saved.');
+    return '';
+  },
+
+  /** Delete the saved URL from the server (the API clears on `url: ""`), which also turns the channel off. */
+  async clearWebhook() {
+    if (!confirm('Remove the saved webhook URL from the server? Webhook alerts stop until you save a new one.')) return;
+    const res = await this._api('/api/webhook', { method: 'PUT', body: { url: '', enabled: false } });
+    let body = null;
+    try { body = res ? await res.json() : null; } catch { /* fall through */ }
+    if (!res || !res.ok || !body || body.success === false) {
+      this._webhookSay(body?.error || 'Could not remove the webhook URL.', true);
+      return;
+    }
+    await this.loadWebhook();
+    this._webhookSay('Webhook URL removed.');
   },
 
   async testWebhook() {
     const btn = document.getElementById('webhookTestBtn');
     if (btn) btn.disabled = true;
-    this._webhookSay('Sending…');
     try {
+      if (this._webhookPending() && (await this.saveWebhook())) return; // the save's error is already shown
+      this._webhookSay('Sending…');
       const res = await this._apiPost('/api/webhook/test', {});
       let body = null;
       try { body = res ? await res.json() : null; } catch { /* fall through */ }
@@ -2613,6 +2659,7 @@ Object.assign(CodemanApp.prototype, {
       sessionLineageLines: _sll,
       ...serverSettings
     } = settings;
+    let webhookError = '';
     try {
       const res = await this._apiPut('/api/settings', {
         ...serverSettings,
@@ -2637,7 +2684,16 @@ Object.assign(CodemanApp.prototype, {
       // Save model configuration separately
       await this.saveModelConfigFromSettings();
 
-      this.showToast('Settings saved', 'success');
+      // The webhook is server state in its own 0600 file (its URL is a secret, kept out of
+      // settings.json), so like the model config above it is saved beside the settings PUT, not in
+      // it. Only when the group was edited: an untouched group must not re-PUT. A refusal (bad URL,
+      // enabled with no URL) keeps the modal open below, with the pasted URL still in the box.
+      webhookError = this._webhookPending() ? await this.saveWebhook() : '';
+      if (webhookError) {
+        this.showToast(`Settings saved, but not the webhook: ${webhookError}`, 'warning');
+      } else {
+        this.showToast('Settings saved', 'success');
+      }
 
       // Show tunnel-specific feedback if toggled on
       if (settings.tunnelEnabled) {
@@ -2648,7 +2704,11 @@ Object.assign(CodemanApp.prototype, {
       this.showToast('Settings saved locally', 'warning');
     }
 
-    this.closeAppSettings();
+    if (webhookError) {
+      document.getElementById('webhookGroup')?.scrollIntoView({ block: 'center' });
+    } else {
+      this.closeAppSettings();
+    }
 
     // Voice availability is a server-side answer, so re-probe after a save:
     // otherwise the mic keeps using the pre-save provider until the next reload.

src/web/routes/webhook-routes.ts

@@ -11,7 +11,14 @@
  */
 
 import type { FastifyInstance, FastifyReply, FastifyRequest } from 'fastify';
-import { ApiErrorCode, createErrorResponse, getErrorMessage, type ApiResponse } from '../../types.js';
+import {
+  ApiErrorCode,
+  createErrorResponse,
+  getErrorMessage,
+  type ApiResponse,
+  type WebhookResult,
+  type WebhookStatus,
+} from '../../types.js';
 import { isAdmin, parseBody } from '../route-helpers.js';
 import { isMultiUserMode } from '../../config/multiuser.js';
 import { WebhookUpdateSchema } from '../schemas.js';
@@ -20,22 +27,9 @@ import {
   readWebhookConfig,
   webhookUrlProblem,
   writeWebhookConfig,
-  type WebhookKind,
   type WebhookNotifier,
-  type WebhookResult,
-  type WebhookScope,
 } from '../../webhook-notify.js';
 
-export interface WebhookStatus {
-  enabled: boolean;
-  kind: WebhookKind;
-  scope: WebhookScope;
-  hasUrl: boolean;
-  /** Scheme + host only; the path and query are the secret. */
-  urlMasked: string;
-  lastResult: WebhookResult | null;
-}
-
 export interface WebhookRouteDeps {
   notifier: WebhookNotifier;
   configDir: string;

src/web/schemas.ts

@@ -19,6 +19,7 @@ import {
 } from '../config/terminal-history.js';
 import { MAX_EDITABLE_BYTES } from '../config/file-editing.js';
 import { CODEX_REASONING_EFFORTS } from '../types/session.js';
+import { WEBHOOK_KINDS, WEBHOOK_SCOPES } from '../types/push.js';
 import { MIN_MATCH_LENGTH, MAX_MATCH_LENGTH } from '../config/agent-wait.js';
 import { MAX_WAKE_MACS } from '../config/remote-wake-limits.js';
 import { MAX_INPUT_LENGTH } from '../config/terminal-limits.js';
@@ -1874,19 +1875,6 @@ export const RespawnEnableSchema = z.object({
 // ========== Web Push ==========
 
 /** POST /api/push/subscribe */
-/**
- * PUT /api/webhook. `.strict()` like every settings-shaped schema; `url` is optional so a change of
- * kind or scope never needs the secret re-sent, and an empty string clears it.
- */
-export const WebhookUpdateSchema = z
-  .object({
-    enabled: z.boolean().optional(),
-    kind: z.enum(['ntfy', 'slack', 'discord', 'generic']).optional(),
-    scope: z.enum(['attention', 'all']).optional(),
-    url: z.string().max(2048).optional(),
-  })
-  .strict();
-
 export const PushSubscribeSchema = z.object({
   endpoint: z
     .string()
@@ -1906,6 +1894,20 @@ export const PushPreferencesUpdateSchema = z.object({
   pushPreferences: z.record(z.string(), z.boolean()),
 });
 
+/**
+ * PUT /api/webhook. `.strict()` like every settings-shaped schema; `url` is optional so a change of
+ * kind or scope never needs the secret re-sent, and an empty string clears it. The kind and scope
+ * lists are the store's own, so the schema can never accept a value the store would coerce away.
+ */
+export const WebhookUpdateSchema = z
+  .object({
+    enabled: z.boolean().optional(),
+    kind: z.enum(WEBHOOK_KINDS).optional(),
+    scope: z.enum(WEBHOOK_SCOPES).optional(),
+    url: z.string().max(2048).optional(),
+  })
+  .strict();
+
 // ========== Ralph Loop ==========
 
 /** POST /api/ralph-loop/start */

src/web/server.ts

@@ -44,7 +44,8 @@ import { hostname as getHostname, uptime as osUptime } from 'node:os';
 import { looksLikeHostReboot, newestPersistedActivity, planRebootRestore } from '../reboot-restore.js';
 import { rebootRestoreRegistry } from './reboot-restore-registry.js';
 import { dataPath, getDataDir, CODEMAN_INSTANCE } from '../config/instance.js';
-import { WebhookNotifier, readWebhookConfig, type WebhookUrgency } from '../webhook-notify.js';
+import { WebhookNotifier, readWebhookConfig } from '../webhook-notify.js';
+import type { WebhookUrgency } from '../types/push.js';
 import { webviewFetch } from './webview-egress.js';
 import { readRemoteHosts, rehydrateRemoteHostFields } from '../remote-hosts.js';
 import type { RemoteWakeRegistry } from '../remote-wake.js';

src/webhook-notify.ts

@@ -26,6 +26,16 @@ import { existsSync, mkdirSync } from 'node:fs';
 import fs from 'node:fs/promises';
 import { join } from 'node:path';
 import { blockedWebviewHostReason } from './web/webview-egress-policy.js';
+import { isEgressBlockedError } from './web/webview-egress.js';
+import {
+  WEBHOOK_KINDS,
+  WEBHOOK_SCOPES,
+  type WebhookConfig,
+  type WebhookKind,
+  type WebhookResult,
+  type WebhookScope,
+  type WebhookUrgency,
+} from './types/push.js';
 
 const WEBHOOK_FILE = 'webhook.json';
 const MAX_URL_LENGTH = 2048;
@@ -35,20 +45,6 @@ const MAX_BODY_CHARS = 500;
 const DEDUPE_WINDOW_MS = 3000;
 const MAX_IN_FLIGHT = 5;
 
-export const WEBHOOK_KINDS = ['ntfy', 'slack', 'discord', 'generic'] as const;
-export type WebhookKind = (typeof WEBHOOK_KINDS)[number];
-/** `attention`: only events that need a human (critical / warning). `all`: also "response complete". */
-export const WEBHOOK_SCOPES = ['attention', 'all'] as const;
-export type WebhookScope = (typeof WEBHOOK_SCOPES)[number];
-export type WebhookUrgency = 'critical' | 'warning' | 'info';
-
-export interface WebhookConfig {
-  enabled: boolean;
-  kind: WebhookKind;
-  url: string;
-  scope: WebhookScope;
-}
-
 export const DEFAULT_WEBHOOK_CONFIG: WebhookConfig = { enabled: false, kind: 'ntfy', url: '', scope: 'attention' };
 
 export interface WebhookMessage {
@@ -62,13 +58,6 @@ export interface WebhookMessage {
   host?: string;
 }
 
-export interface WebhookResult {
-  ok: boolean;
-  status?: number;
-  error?: string;
-  at: number;
-}
-
 // ---------------------------------------------------------------------------
 // Pure
 // ---------------------------------------------------------------------------
@@ -228,13 +217,15 @@ export async function writeWebhookConfig(configDir: string, cfg: WebhookConfig):
 
 export type WebhookFetch = (target: URL, init: RequestInit) => Promise<Response>;
 
-/** What went wrong, without the URL: blocked / timed out / refused / an HTTP status. */
+/**
+ * What went wrong, without the URL: blocked / timed out / refused / an HTTP status. An egress
+ * refusal is recognised by its `CODEMAN_EGRESS_BLOCKED` code anywhere in the cause chain (undici
+ * wraps the lookup's error as `TypeError('fetch failed', { cause })`), never by message text.
+ */
 function describeError(err: unknown): string {
-  const e = err as { name?: string; message?: string; cause?: { code?: string; message?: string } };
+  const e = err as { name?: string; cause?: { code?: string } };
   if (e?.name === 'TimeoutError' || e?.name === 'AbortError') return 'Timed out';
-  const text = `${e?.message ?? ''} ${e?.cause?.message ?? ''}`;
-  if (/link-local|cloud-metadata|EGRESS/i.test(text))
-    return 'Refused: target is a link-local or cloud-metadata address';
+  if (isEgressBlockedError(err)) return 'Refused: target is a link-local or cloud-metadata address';
   if (e?.cause?.code === 'ENOTFOUND') return 'Host not found';
   if (e?.cause?.code === 'ECONNREFUSED') return 'Connection refused';
   return 'Network error';