From 5e72191cac9469c2ade91aaba1e62f69fa1ad94a Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Mon, 1 Jun 2026 09:28:21 +0900 Subject: feat(core): ntfy.sh notification dispatcher module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a transport-agnostic NotificationDispatcher and a fire-and-forget ntfy.sh transport (no SDK; just fetch). Configuration is persisted as a single global JSON blob under the 'ntfy_config' settings key. Event taxonomy (per-event toggles): - turn-completed — assistant turn finished cleanly - turn-error — final turn error (after all fallbacks) - permission-required — new permission prompt was created - agent-spawned — top-level user-agent tab spawned via 'summon' Design: - Single internal notify(event) interface so a future transport (email, webhook) plugs in without changing call sites. - attachToAgentManager + attachToPermissionManager subscribe to the existing event streams via narrow listener interfaces (no @dispatch/api dependency back into core). - 5s in-memory dedupe window on dedupeKey suppresses permission re-emits. - 10s per-request abort timeout so a hung ntfy server can't pin a worker. - All sends are fire-and-forget: void Promise.resolve(...).catch(warn). Tests (39 new): - ntfy transport: URL/headers/body/auth/click, header sanitization, per-event-type defaults, error paths. - config: defaults, normalization tolerance, round-trip, redaction. - dispatcher: master switch, per-event toggle, dedupe, agent/permission hookups, top-level-only filtering for agent-spawned, dispose. --- packages/core/src/index.ts | 2 + packages/core/src/notifications/config.ts | 74 +++++ packages/core/src/notifications/dispatcher.ts | 238 +++++++++++++++ packages/core/src/notifications/index.ts | 29 ++ packages/core/src/notifications/ntfy.ts | 136 +++++++++ packages/core/src/notifications/types.ts | 98 +++++++ packages/core/tests/notifications/config.test.ts | 128 ++++++++ .../core/tests/notifications/dispatcher.test.ts | 323 +++++++++++++++++++++ packages/core/tests/notifications/ntfy.test.ts | 168 +++++++++++ 9 files changed, 1196 insertions(+) create mode 100644 packages/core/src/notifications/config.ts create mode 100644 packages/core/src/notifications/dispatcher.ts create mode 100644 packages/core/src/notifications/index.ts create mode 100644 packages/core/src/notifications/ntfy.ts create mode 100644 packages/core/src/notifications/types.ts create mode 100644 packages/core/tests/notifications/config.test.ts create mode 100644 packages/core/tests/notifications/dispatcher.test.ts create mode 100644 packages/core/tests/notifications/ntfy.test.ts (limited to 'packages/core') diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index b1b17cc..327b0a5 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -68,6 +68,8 @@ export { export { createProvider } from "./llm/provider.js"; // Models export { ModelRegistry } from "./models/index.js"; +// Notifications (ntfy.sh) +export * from "./notifications/index.js"; export * from "./permission/index.js"; // Skills export { diff --git a/packages/core/src/notifications/config.ts b/packages/core/src/notifications/config.ts new file mode 100644 index 0000000..310c606 --- /dev/null +++ b/packages/core/src/notifications/config.ts @@ -0,0 +1,74 @@ +// Persisted ntfy config — single global JSON blob under one settings key. +// +// One global config (no per-user split): the rest of Dispatch's settings +// table is global today (cf. `title_model_*`, `perm_*`), so notification +// config follows the same pattern. + +import { deleteSetting, getSetting, setSetting } from "../db/settings.js"; +import type { NotificationEventType, NtfyConfig } from "./types.js"; +import { NTFY_DEFAULT_EVENTS, NTFY_EVENT_TYPES } from "./types.js"; + +export const NTFY_CONFIG_KEY = "ntfy_config"; + +/** Defaults returned when nothing is persisted yet. */ +export function defaultNtfyConfig(): NtfyConfig { + return { + enabled: false, + topicUrl: "", + authToken: "", + events: { ...NTFY_DEFAULT_EVENTS }, + }; +} + +/** + * Normalize an arbitrary parsed JSON value into a complete `NtfyConfig`. + * Tolerant of missing / unexpected fields so a config from an older build + * never throws — missing event toggles fall back to defaults. + */ +export function normalizeNtfyConfig(raw: unknown): NtfyConfig { + const base = defaultNtfyConfig(); + if (!raw || typeof raw !== "object") return base; + const obj = raw as Record; + const out: NtfyConfig = { + enabled: typeof obj.enabled === "boolean" ? obj.enabled : base.enabled, + topicUrl: typeof obj.topicUrl === "string" ? obj.topicUrl : base.topicUrl, + authToken: typeof obj.authToken === "string" ? obj.authToken : base.authToken, + events: { ...base.events }, + }; + const rawEvents = obj.events; + if (rawEvents && typeof rawEvents === "object") { + const evObj = rawEvents as Record; + for (const key of NTFY_EVENT_TYPES) { + const v = evObj[key]; + if (typeof v === "boolean") out.events[key as NotificationEventType] = v; + } + } + return out; +} + +/** Load the persisted config (or defaults if none/corrupt). */ +export function loadNtfyConfig(): NtfyConfig { + const raw = getSetting(NTFY_CONFIG_KEY); + if (!raw) return defaultNtfyConfig(); + try { + return normalizeNtfyConfig(JSON.parse(raw)); + } catch { + return defaultNtfyConfig(); + } +} + +/** Persist a complete config (after server-side normalization). */ +export function saveNtfyConfig(config: NtfyConfig): void { + const normalized = normalizeNtfyConfig(config); + setSetting(NTFY_CONFIG_KEY, JSON.stringify(normalized)); +} + +/** Wipe the persisted config (revert to defaults on next load). */ +export function clearNtfyConfig(): void { + deleteSetting(NTFY_CONFIG_KEY); +} + +/** Strip the auth token from a config before returning it over the API. */ +export function redactNtfyConfig(config: NtfyConfig): NtfyConfig & { hasAuthToken: boolean } { + return { ...config, authToken: "", hasAuthToken: config.authToken.trim().length > 0 }; +} diff --git a/packages/core/src/notifications/dispatcher.ts b/packages/core/src/notifications/dispatcher.ts new file mode 100644 index 0000000..4f4fc79 --- /dev/null +++ b/packages/core/src/notifications/dispatcher.ts @@ -0,0 +1,238 @@ +// NotificationDispatcher — turns high-level Dispatch events into +// `sendNtfy(...)` calls, gated by the persisted user config. +// +// The dispatcher is transport-agnostic at the `notify(event)` interface +// boundary: only `sendNtfy` is wired today, but adding another transport +// (email, webhook, etc.) means changing this one file, not the call sites. +// +// All sends are non-blocking (fire-and-forget). A 10-second timeout in +// `sendNtfy` bounds the worst case; the dispatcher additionally guards +// every send in a try/catch so a transport bug can never propagate into +// the agent loop. + +import { loadNtfyConfig } from "./config.js"; +import { type FetchLike, sendNtfy } from "./ntfy.js"; +import type { NotificationEvent, NtfyConfig } from "./types.js"; + +/** Minimal shape of an `AgentManager`-style event stream we hook into. */ +export interface AgentEventSource { + onEvent( + listener: (event: { type: string; tabId: string; [key: string]: unknown }) => void, + ): () => void; +} + +/** Minimal shape of a `PermissionManager`-style prompt source. */ +export interface PermissionPromptSource { + onPromptAdded( + listener: (prompt: { id: string; permission: string; description: string }) => void, + ): () => void; +} + +/** Look up a human-readable tab title for nicer notification text. */ +export type TabTitleLookup = (tabId: string) => string | null; + +export interface DispatcherOptions { + /** Override the config loader (tests). Defaults to `loadNtfyConfig`. */ + loadConfig?: () => NtfyConfig; + /** Override the transport (tests). Defaults to the real `sendNtfy`. */ + send?: (config: NtfyConfig, event: NotificationEvent) => Promise; + /** Optional fetch override (forwarded to `sendNtfy` when `send` not set). */ + fetchImpl?: FetchLike; + /** Look up a tab title for richer titles. */ + getTabTitle?: TabTitleLookup; + /** + * How long (ms) a dedupeKey is suppressed for. Permission prompts re-emit + * the whole pending list on every change, so dedupe is essential. + */ + dedupeWindowMs?: number; +} + +export class NotificationDispatcher { + private loadConfig: () => NtfyConfig; + private send: (config: NtfyConfig, event: NotificationEvent) => Promise; + private getTabTitle: TabTitleLookup | undefined; + private dedupeWindowMs: number; + /** Recently-sent dedupeKey → expiresAt epoch ms. */ + private recentlySent = new Map(); + private unsubs: Array<() => void> = []; + + constructor(opts: DispatcherOptions = {}) { + this.loadConfig = opts.loadConfig ?? loadNtfyConfig; + this.send = + opts.send ?? ((config, event) => sendNtfy(config, event, opts.fetchImpl ?? undefined)); + this.getTabTitle = opts.getTabTitle; + this.dedupeWindowMs = opts.dedupeWindowMs ?? 5_000; + } + + /** + * Single internal entry point — every public hook funnels through here. + * Public so a future caller can synthesize an arbitrary notification + * (e.g. a CLI `dispatch notify` command); kept narrow. + */ + notify(event: NotificationEvent): void { + const config = this.loadConfig(); + if (!config.enabled) return; + if (!config.events[event.type]) return; + if (event.dedupeKey && this.isDuplicate(event.dedupeKey)) return; + if (event.dedupeKey) this.markSent(event.dedupeKey); + + // Fire-and-forget: never await, never throw. + try { + void Promise.resolve(this.send(config, event)).catch((err) => { + console.warn( + `[ntfy] send failed for ${event.type}: ${err instanceof Error ? err.message : String(err)}`, + ); + }); + } catch (err) { + // Guard the synchronous portion of `send` too. + console.warn( + `[ntfy] dispatch threw for ${event.type}: ${err instanceof Error ? err.message : String(err)}`, + ); + } + } + + /** + * Hook into an `AgentManager`-style event stream. + * + * Maps: + * - `done` → `turn-completed` + * - `error` → `turn-error` + * - `tab-created` → `agent-spawned` (only top-level user-agent tabs) + * + * `status` events are ignored — they fire on every transition and we'd + * either spam or duplicate the `done`/`error` notifications. + */ + attachToAgentManager(source: AgentEventSource): () => void { + const unsub = source.onEvent((event) => { + if (event.type === "done") { + this.notify(this.buildTurnCompleted(event)); + } else if (event.type === "error") { + this.notify(this.buildTurnError(event)); + } else if (event.type === "tab-created") { + const ev = event as unknown as { + tabId: string; + id: string; + title: string; + parentTabId: string | null; + agentSlug?: string | null; + }; + // Only notify for top-level user-agent tabs spawned via `summon`. + // Filtering on `agentSlug` skips "blank" new tabs the user opened + // manually, which would be noisy. + if (ev.parentTabId === null && ev.agentSlug) { + this.notify(this.buildAgentSpawned(ev)); + } + } + }); + this.unsubs.push(unsub); + return unsub; + } + + /** Hook into a `PermissionManager`-style prompt source. */ + attachToPermissionManager(source: PermissionPromptSource): () => void { + const unsub = source.onPromptAdded((prompt) => { + this.notify(this.buildPermissionRequired(prompt)); + }); + this.unsubs.push(unsub); + return unsub; + } + + /** Release all hooks acquired via `attachTo*`. */ + dispose(): void { + for (const u of this.unsubs) { + try { + u(); + } catch { + // best-effort + } + } + this.unsubs = []; + this.recentlySent.clear(); + } + + // ─── Event builders (internal) ──────────────────────────────── + + private buildTurnCompleted(event: { tabId: string }): NotificationEvent { + const tabLabel = this.tabLabel(event.tabId); + return { + type: "turn-completed", + title: `Turn complete — ${tabLabel}`, + message: `Assistant finished a turn in ${tabLabel}.`, + tabId: event.tabId, + }; + } + + private buildTurnError(event: { + tabId: string; + error?: unknown; + statusCode?: unknown; + }): NotificationEvent { + const tabLabel = this.tabLabel(event.tabId); + const errText = typeof event.error === "string" ? event.error : "Unknown error"; + const statusText = typeof event.statusCode === "number" ? ` (status ${event.statusCode})` : ""; + return { + type: "turn-error", + title: `Turn failed — ${tabLabel}`, + message: `${errText}${statusText}`, + tabId: event.tabId, + }; + } + + private buildPermissionRequired(prompt: { + id: string; + permission: string; + description: string; + }): NotificationEvent { + return { + type: "permission-required", + title: `Permission required: ${prompt.permission}`, + message: prompt.description || `Agent is requesting ${prompt.permission} permission.`, + // Permission prompts can re-emit (e.g. another prompt arrives while + // this one is still pending) — dedupe on the prompt id. + dedupeKey: `permission:${prompt.id}`, + }; + } + + private buildAgentSpawned(ev: { + tabId: string; + id: string; + title: string; + agentSlug?: string | null; + }): NotificationEvent { + return { + type: "agent-spawned", + title: `User agent spawned — ${ev.agentSlug ?? "agent"}`, + message: ev.title, + tabId: ev.tabId ?? ev.id, + }; + } + + private tabLabel(tabId: string): string { + const title = this.getTabTitle?.(tabId); + if (title?.trim()) return title.trim(); + return `tab ${tabId.slice(0, 8)}`; + } + + // ─── Dedupe helpers ─────────────────────────────────────────── + + private isDuplicate(key: string): boolean { + const expires = this.recentlySent.get(key); + if (expires === undefined) return false; + if (expires <= Date.now()) { + this.recentlySent.delete(key); + return false; + } + return true; + } + + private markSent(key: string): void { + // Lazy-evict expired entries when the map gets large. + if (this.recentlySent.size > 256) { + const now = Date.now(); + for (const [k, exp] of this.recentlySent) { + if (exp <= now) this.recentlySent.delete(k); + } + } + this.recentlySent.set(key, Date.now() + this.dedupeWindowMs); + } +} diff --git a/packages/core/src/notifications/index.ts b/packages/core/src/notifications/index.ts new file mode 100644 index 0000000..ea99a58 --- /dev/null +++ b/packages/core/src/notifications/index.ts @@ -0,0 +1,29 @@ +// @dispatch/core — ntfy.sh push notifications + +export { + clearNtfyConfig, + defaultNtfyConfig, + loadNtfyConfig, + NTFY_CONFIG_KEY, + normalizeNtfyConfig, + redactNtfyConfig, + saveNtfyConfig, +} from "./config.js"; +export { + type AgentEventSource, + type DispatcherOptions, + NotificationDispatcher, + type PermissionPromptSource, + type TabTitleLookup, +} from "./dispatcher.js"; +export { type FetchLike, type NtfySendResult, sendNtfy, validateTopicUrl } from "./ntfy.js"; +export { + type NotificationEvent, + type NotificationEventType, + NTFY_DEFAULT_EVENTS, + NTFY_DEFAULT_PRIORITIES, + NTFY_DEFAULT_TAGS, + NTFY_EVENT_TYPES, + type NtfyConfig, + type NtfyPriority, +} from "./types.js"; diff --git a/packages/core/src/notifications/ntfy.ts b/packages/core/src/notifications/ntfy.ts new file mode 100644 index 0000000..07ce33b --- /dev/null +++ b/packages/core/src/notifications/ntfy.ts @@ -0,0 +1,136 @@ +// ntfy.sh HTTP transport. +// +// ntfy's API is a simple POST to `https:///` with the body +// as the message and metadata passed via HTTP headers: +// Title: notification title +// Priority: 1..5 (3 = default) +// Tags: comma-separated emoji shortcodes +// Click: URL opened when the notification is tapped +// +// We intentionally use `fetch` directly — no SDK, no extra deps. + +import type { NotificationEvent, NtfyConfig } from "./types.js"; +import { NTFY_DEFAULT_PRIORITIES, NTFY_DEFAULT_TAGS } from "./types.js"; + +export interface NtfySendResult { + ok: boolean; + status?: number; + error?: string; +} + +/** + * Lightweight fetch shape so callers (and tests) can inject a mock without + * pulling in the DOM `fetch` type from a `Headers` instance. + */ +export type FetchLike = ( + input: string, + init: { method: string; headers: Record; body: string; signal?: AbortSignal }, +) => Promise<{ ok: boolean; status: number; statusText?: string; text(): Promise }>; + +/** + * Validate a ntfy topic URL. Accepts only `http(s)://host/topic` with a + * non-empty topic path. Returns `null` on success, a human-readable error + * string on failure. + */ +export function validateTopicUrl(topicUrl: string): string | null { + const trimmed = topicUrl.trim(); + if (!trimmed) return "Topic URL is required"; + let url: URL; + try { + url = new URL(trimmed); + } catch { + return "Topic URL is not a valid URL"; + } + if (url.protocol !== "http:" && url.protocol !== "https:") { + return "Topic URL must use http:// or https://"; + } + // Path must be a non-empty topic (more than just "/") + const topic = url.pathname.replace(/^\/+|\/+$/g, ""); + if (!topic) return "Topic URL must include a topic name (e.g. https://ntfy.sh/my-topic)"; + return null; +} + +/** + * Send a single notification to the configured ntfy topic. + * + * Fire-and-forget at call sites: the dispatcher uses + * `void sendNtfy(...).catch(...)` so a slow/broken ntfy server never blocks + * a turn. We still return a structured result so the explicit + * `POST /notifications/test` route can surface failures back to the UI. + * + * Pure with respect to `config` / `event` — no DB, no module state. + */ +export async function sendNtfy( + config: NtfyConfig, + event: NotificationEvent, + fetchImpl: FetchLike = globalThis.fetch as unknown as FetchLike, + timeoutMs = 10_000, +): Promise { + if (!config.enabled) return { ok: false, error: "Notifications are disabled" }; + const topicErr = validateTopicUrl(config.topicUrl); + if (topicErr) return { ok: false, error: topicErr }; + + const priority = event.priority ?? NTFY_DEFAULT_PRIORITIES[event.type] ?? 3; + const baseTags = event.tags ?? NTFY_DEFAULT_TAGS[event.type] ?? []; + const tags = [...baseTags]; + if (event.tabId) { + // Short, ASCII-only tag so ntfy's comma-separated header parser is happy. + tags.push(`tab-${event.tabId.slice(0, 8)}`); + } + + const headers: Record = { + // ntfy treats the title/priority/tags/click headers as ASCII-only. Strip + // control chars from the title; the body is sent UTF-8 verbatim. + Title: sanitizeHeader(event.title), + Priority: String(priority), + "Content-Type": "text/plain; charset=utf-8", + }; + if (tags.length > 0) headers.Tags = tags.map(sanitizeHeader).join(","); + if (event.clickUrl) headers.Click = event.clickUrl; + if (config.authToken && config.authToken.trim() !== "") { + headers.Authorization = `Bearer ${config.authToken.trim()}`; + } + + // Per-request abort so a hung server doesn't pin a Bun worker forever. + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), timeoutMs); + + try { + const res = await fetchImpl(config.topicUrl.trim(), { + method: "POST", + headers, + body: event.message, + signal: controller.signal, + }); + if (!res.ok) { + const text = await safeReadText(res); + return { + ok: false, + status: res.status, + error: `ntfy responded ${res.status} ${res.statusText ?? ""}: ${text}`.trim(), + }; + } + return { ok: true, status: res.status }; + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + return { ok: false, error: msg }; + } finally { + clearTimeout(timer); + } +} + +function sanitizeHeader(value: string): string { + // Strip CR/LF (header injection guard) and trim. ntfy is tolerant of + // non-ASCII in titles, but we still drop control chars. + // biome-ignore lint/suspicious/noControlCharactersInRegex: intentional + return value.replace(/[\r\n\u0000-\u001f]+/g, " ").trim(); +} + +async function safeReadText(res: { text(): Promise }): Promise { + try { + const t = await res.text(); + return t.length > 200 ? `${t.slice(0, 200)}…` : t; + } catch { + return ""; + } +} diff --git a/packages/core/src/notifications/types.ts b/packages/core/src/notifications/types.ts new file mode 100644 index 0000000..f6baa27 --- /dev/null +++ b/packages/core/src/notifications/types.ts @@ -0,0 +1,98 @@ +// ntfy.sh push notifications — types + +/** + * Catalog of notification-worthy events. + * + * Kept intentionally small and stable: each entry is something a human + * actually wants pushed to their phone. New event types should be added + * with a sensible default (`NTFY_DEFAULT_EVENTS`) and a mapping in the + * dispatcher. + */ +export type NotificationEventType = + | "turn-completed" + | "turn-error" + | "permission-required" + | "agent-spawned"; + +/** ntfy priority levels (1=min … 5=max). */ +export type NtfyPriority = 1 | 2 | 3 | 4 | 5; + +/** + * A single notification request. Synthesised by the dispatcher from a + * higher-level event source (AgentManager / PermissionManager); fed to + * the ntfy transport. + * + * `dedupeKey` lets the dispatcher suppress duplicate sends (e.g. the + * permission system re-emits the pending list on every change). + */ +export interface NotificationEvent { + type: NotificationEventType; + /** Notification title (short). */ + title: string; + /** Notification body. */ + message: string; + /** Optional ntfy tags (emoji shortcodes — e.g. `["white_check_mark"]`). */ + tags?: string[]; + /** Optional priority override. Defaults are per-event-type. */ + priority?: NtfyPriority; + /** Optional URL the notification deep-links to when tapped. */ + clickUrl?: string; + /** Origin tab id (informational; included in tags as `tab:`). */ + tabId?: string; + /** + * Stable key for suppressing duplicates. Same key + same type within a + * short window ⇒ dropped silently. + */ + dedupeKey?: string; +} + +/** + * Persisted ntfy configuration. Lives in the `settings` table under a + * single key (`ntfy_config`) — one global config, matching the codebase's + * existing single-user assumption (cf. `title_model_*`, `perm_*`). + * + * - `enabled` — master switch. Off ⇒ dispatcher never sends. + * - `topicUrl` — full URL, e.g. `https://ntfy.sh/my-secret-topic`. Missing + * ⇒ dispatcher never sends. + * - `authToken` — optional bearer token for private ntfy servers. + * - `events` — per-event-type enable map. Missing entries default to OFF + * so a newly-added event type doesn't silently start firing. + */ +export interface NtfyConfig { + enabled: boolean; + topicUrl: string; + authToken: string; + events: Record; +} + +/** All event types this build knows about (the source of truth for UI). */ +export const NTFY_EVENT_TYPES: NotificationEventType[] = [ + "turn-completed", + "turn-error", + "permission-required", + "agent-spawned", +]; + +/** Default per-event-type toggles. */ +export const NTFY_DEFAULT_EVENTS: Record = { + "turn-completed": true, + "turn-error": true, + "permission-required": true, + "agent-spawned": false, +}; + +/** Default priority per event type (when the event itself doesn't override). */ +export const NTFY_DEFAULT_PRIORITIES: Record = { + "turn-completed": 3, + "turn-error": 4, + "permission-required": 4, + "agent-spawned": 2, +}; + +/** Default tag (emoji) per event type. */ +export const NTFY_DEFAULT_TAGS: Record = { + "turn-completed": ["white_check_mark"], + "turn-error": ["rotating_light"], + "permission-required": ["lock"], + "agent-spawned": ["sparkles"], +}; diff --git a/packages/core/tests/notifications/config.test.ts b/packages/core/tests/notifications/config.test.ts new file mode 100644 index 0000000..64a9637 --- /dev/null +++ b/packages/core/tests/notifications/config.test.ts @@ -0,0 +1,128 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// In-memory fake for the settings table — mounted before the module under +// test is imported (vi.mock is hoisted). +const fakeSettings = new Map(); + +vi.mock("../../src/db/settings.js", () => ({ + getSetting: vi.fn((key: string) => fakeSettings.get(key) ?? null), + setSetting: vi.fn((key: string, value: string) => { + fakeSettings.set(key, value); + }), + deleteSetting: vi.fn((key: string) => { + fakeSettings.delete(key); + }), +})); + +const { + clearNtfyConfig, + defaultNtfyConfig, + loadNtfyConfig, + normalizeNtfyConfig, + NTFY_CONFIG_KEY, + redactNtfyConfig, + saveNtfyConfig, +} = await import("../../src/notifications/config.js"); + +describe("defaultNtfyConfig", () => { + it("disables notifications and ships sane per-event defaults", () => { + const cfg = defaultNtfyConfig(); + expect(cfg.enabled).toBe(false); + expect(cfg.topicUrl).toBe(""); + expect(cfg.authToken).toBe(""); + expect(cfg.events["turn-completed"]).toBe(true); + expect(cfg.events["turn-error"]).toBe(true); + expect(cfg.events["permission-required"]).toBe(true); + expect(cfg.events["agent-spawned"]).toBe(false); + }); +}); + +describe("normalizeNtfyConfig", () => { + it("returns defaults for non-object input", () => { + expect(normalizeNtfyConfig(null)).toEqual(defaultNtfyConfig()); + expect(normalizeNtfyConfig(undefined)).toEqual(defaultNtfyConfig()); + expect(normalizeNtfyConfig(42)).toEqual(defaultNtfyConfig()); + }); + + it("fills in missing event toggles with defaults (newly-added types default OFF)", () => { + const normalized = normalizeNtfyConfig({ + enabled: true, + topicUrl: "https://ntfy.sh/x", + events: { "turn-completed": false }, + }); + expect(normalized.events["turn-completed"]).toBe(false); + // Defaults preserved for fields the persisted blob doesn't have. + expect(normalized.events["turn-error"]).toBe(true); + expect(normalized.events["agent-spawned"]).toBe(false); + }); + + it("ignores extraneous fields and wrong-typed values", () => { + const normalized = normalizeNtfyConfig({ + enabled: "yes", // wrong type ⇒ default + topicUrl: 42, // wrong type ⇒ default + authToken: null, // wrong type ⇒ default + events: { "turn-completed": "no", bogus: true }, + extra: "ignored", + }); + expect(normalized.enabled).toBe(false); + expect(normalized.topicUrl).toBe(""); + expect(normalized.authToken).toBe(""); + expect(normalized.events["turn-completed"]).toBe(true); // default kept + expect((normalized.events as Record).bogus).toBeUndefined(); + }); +}); + +describe("load/save round-trip", () => { + beforeEach(() => { + fakeSettings.clear(); + }); + + it("returns defaults when nothing is persisted", () => { + expect(loadNtfyConfig()).toEqual(defaultNtfyConfig()); + }); + + it("round-trips a complete config", () => { + const cfg = { + enabled: true, + topicUrl: "https://ntfy.sh/team", + authToken: "tk_abc", + events: { + "turn-completed": false, + "turn-error": true, + "permission-required": true, + "agent-spawned": true, + }, + } as const; + saveNtfyConfig({ ...cfg }); + const loaded = loadNtfyConfig(); + expect(loaded).toEqual(cfg); + // Persisted as a JSON string under the documented key. + expect(fakeSettings.has(NTFY_CONFIG_KEY)).toBe(true); + }); + + it("returns defaults when stored JSON is corrupt", () => { + fakeSettings.set(NTFY_CONFIG_KEY, "{ not json"); + expect(loadNtfyConfig()).toEqual(defaultNtfyConfig()); + }); + + it("clearNtfyConfig removes the persisted entry", () => { + saveNtfyConfig({ ...defaultNtfyConfig(), enabled: true, topicUrl: "https://ntfy.sh/x" }); + expect(fakeSettings.has(NTFY_CONFIG_KEY)).toBe(true); + clearNtfyConfig(); + expect(fakeSettings.has(NTFY_CONFIG_KEY)).toBe(false); + }); +}); + +describe("redactNtfyConfig", () => { + it("strips authToken and surfaces a hasAuthToken flag", () => { + const cfg = { ...defaultNtfyConfig(), authToken: "tk_secret" }; + const redacted = redactNtfyConfig(cfg); + expect(redacted.authToken).toBe(""); + expect(redacted.hasAuthToken).toBe(true); + }); + + it("hasAuthToken is false for blank tokens", () => { + expect(redactNtfyConfig({ ...defaultNtfyConfig(), authToken: "" }).hasAuthToken).toBe(false); + expect(redactNtfyConfig({ ...defaultNtfyConfig(), authToken: " " }).hasAuthToken).toBe(false); + }); +}); diff --git a/packages/core/tests/notifications/dispatcher.test.ts b/packages/core/tests/notifications/dispatcher.test.ts new file mode 100644 index 0000000..db05de4 --- /dev/null +++ b/packages/core/tests/notifications/dispatcher.test.ts @@ -0,0 +1,323 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { NotificationEvent, NtfyConfig } from "../../src/notifications/types.js"; + +// The dispatcher imports `loadNtfyConfig` from config.ts, which transitively +// pulls in `db/index.js` (bun:sqlite). Stub the DB so vitest under Node can +// load this file. All tests inject `loadConfig` explicitly, so the real +// settings table is never read. +vi.mock("../../src/db/index.js", () => ({ + getDatabase: vi.fn(() => ({ + query: () => ({ get: () => null, run: () => {} }), + run: () => {}, + })), +})); + +const { NotificationDispatcher } = await import("../../src/notifications/dispatcher.js"); + +function makeConfig(overrides: Partial = {}): NtfyConfig { + return { + enabled: true, + topicUrl: "https://ntfy.sh/topic", + authToken: "", + events: { + "turn-completed": true, + "turn-error": true, + "permission-required": true, + "agent-spawned": true, + }, + ...overrides, + }; +} + +interface FakeAgentSource { + onEvent( + listener: (event: { type: string; tabId: string; [k: string]: unknown }) => void, + ): () => void; + emit(event: { type: string; tabId: string; [k: string]: unknown }): void; +} + +function makeAgentSource(): FakeAgentSource { + let l: ((event: { type: string; tabId: string; [k: string]: unknown }) => void) | null = null; + return { + onEvent(listener) { + l = listener; + return () => { + l = null; + }; + }, + emit(event) { + l?.(event); + }, + }; +} + +interface FakePermissionSource { + onPromptAdded( + listener: (prompt: { id: string; permission: string; description: string }) => void, + ): () => void; + emit(prompt: { id: string; permission: string; description: string }): void; +} + +function makePermissionSource(): FakePermissionSource { + let l: ((prompt: { id: string; permission: string; description: string }) => void) | null = null; + return { + onPromptAdded(listener) { + l = listener; + return () => { + l = null; + }; + }, + emit(p) { + l?.(p); + }, + }; +} + +// Microtask flush so the dispatcher's `void Promise.resolve(...).catch(...)` +// has a chance to settle before assertions. +async function flush(): Promise { + await Promise.resolve(); + await Promise.resolve(); +} + +describe("NotificationDispatcher.notify", () => { + let warnSpy: ReturnType; + beforeEach(() => { + warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + }); + afterEach(() => { + warnSpy.mockRestore(); + }); + + it("does not send when master switch is disabled", async () => { + const send = vi.fn(async () => ({ ok: true })); + const d = new NotificationDispatcher({ + loadConfig: () => makeConfig({ enabled: false }), + send, + }); + d.notify({ type: "turn-completed", title: "x", message: "y" }); + await flush(); + expect(send).not.toHaveBeenCalled(); + }); + + it("does not send when per-event-type toggle is off", async () => { + const send = vi.fn(async () => ({ ok: true })); + const d = new NotificationDispatcher({ + loadConfig: () => + makeConfig({ + events: { + "turn-completed": false, + "turn-error": true, + "permission-required": true, + "agent-spawned": false, + }, + }), + send, + }); + d.notify({ type: "turn-completed", title: "x", message: "y" }); + await flush(); + expect(send).not.toHaveBeenCalled(); + }); + + it("sends when enabled and toggle is on", async () => { + const send = vi.fn(async () => ({ ok: true })); + const d = new NotificationDispatcher({ loadConfig: () => makeConfig(), send }); + d.notify({ type: "turn-completed", title: "x", message: "y" }); + await flush(); + expect(send).toHaveBeenCalledTimes(1); + }); + + it("does not throw or block when the transport rejects", async () => { + const send = vi.fn(async () => { + throw new Error("boom"); + }); + const d = new NotificationDispatcher({ loadConfig: () => makeConfig(), send }); + expect(() => d.notify({ type: "turn-completed", title: "x", message: "y" })).not.toThrow(); + await flush(); + expect(send).toHaveBeenCalledTimes(1); + expect(warnSpy).toHaveBeenCalled(); + }); + + it("dedupes events with the same dedupeKey within the window", async () => { + const send = vi.fn(async () => ({ ok: true })); + const d = new NotificationDispatcher({ + loadConfig: () => makeConfig(), + send, + dedupeWindowMs: 1000, + }); + const event: NotificationEvent = { + type: "permission-required", + title: "p", + message: "p", + dedupeKey: "permission:42", + }; + d.notify(event); + d.notify(event); + d.notify(event); + await flush(); + expect(send).toHaveBeenCalledTimes(1); + }); + + it("does not dedupe events without a dedupeKey", async () => { + const send = vi.fn(async () => ({ ok: true })); + const d = new NotificationDispatcher({ loadConfig: () => makeConfig(), send }); + d.notify({ type: "turn-completed", title: "x", message: "y" }); + d.notify({ type: "turn-completed", title: "x", message: "y" }); + await flush(); + expect(send).toHaveBeenCalledTimes(2); + }); +}); + +describe("NotificationDispatcher.attachToAgentManager", () => { + let warnSpy: ReturnType; + beforeEach(() => { + warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + }); + afterEach(() => { + warnSpy.mockRestore(); + }); + + it("maps `done` → turn-completed (with tab title in the body)", async () => { + const send = vi.fn(async () => ({ ok: true })); + const source = makeAgentSource(); + const d = new NotificationDispatcher({ + loadConfig: () => makeConfig(), + send, + getTabTitle: (id) => (id === "tab-1" ? "My chat" : null), + }); + d.attachToAgentManager(source); + source.emit({ type: "done", tabId: "tab-1", message: { role: "assistant", chunks: [] } }); + await flush(); + expect(send).toHaveBeenCalledTimes(1); + const event = send.mock.calls[0][1] as NotificationEvent; + expect(event.type).toBe("turn-completed"); + expect(event.title).toContain("My chat"); + expect(event.tabId).toBe("tab-1"); + }); + + it("maps `error` → turn-error and includes the error text", async () => { + const send = vi.fn(async () => ({ ok: true })); + const source = makeAgentSource(); + const d = new NotificationDispatcher({ loadConfig: () => makeConfig(), send }); + d.attachToAgentManager(source); + source.emit({ type: "error", tabId: "tab-1", error: "Rate limit", statusCode: 429 }); + await flush(); + expect(send).toHaveBeenCalledTimes(1); + const event = send.mock.calls[0][1] as NotificationEvent; + expect(event.type).toBe("turn-error"); + expect(event.message).toContain("Rate limit"); + expect(event.message).toContain("429"); + }); + + it("ignores `status` events (would spam every transition)", async () => { + const send = vi.fn(async () => ({ ok: true })); + const source = makeAgentSource(); + const d = new NotificationDispatcher({ loadConfig: () => makeConfig(), send }); + d.attachToAgentManager(source); + source.emit({ type: "status", tabId: "tab-1", status: "running" }); + source.emit({ type: "status", tabId: "tab-1", status: "idle" }); + await flush(); + expect(send).not.toHaveBeenCalled(); + }); + + it("maps `tab-created` to agent-spawned only for top-level user agents (parentTabId=null AND agentSlug set)", async () => { + const send = vi.fn(async () => ({ ok: true })); + const source = makeAgentSource(); + const d = new NotificationDispatcher({ loadConfig: () => makeConfig(), send }); + d.attachToAgentManager(source); + + // Manual "new tab" with no agent slug ⇒ no notification. + source.emit({ + type: "tab-created", + tabId: "tab-1", + id: "tab-1", + title: "New Tab", + parentTabId: null, + agentSlug: null, + }); + // Subagent (has a parent) ⇒ no notification. + source.emit({ + type: "tab-created", + tabId: "tab-2", + id: "tab-2", + title: "Subagent", + parentTabId: "tab-1", + agentSlug: "researcher", + }); + // Top-level user agent ⇒ notify. + source.emit({ + type: "tab-created", + tabId: "tab-3", + id: "tab-3", + title: "Refactor auth code", + parentTabId: null, + agentSlug: "engineer", + }); + await flush(); + expect(send).toHaveBeenCalledTimes(1); + const event = send.mock.calls[0][1] as NotificationEvent; + expect(event.type).toBe("agent-spawned"); + expect(event.message).toBe("Refactor auth code"); + expect(event.title).toContain("engineer"); + }); + + it("respects the per-event-type toggle (turn-completed off ⇒ silent)", async () => { + const send = vi.fn(async () => ({ ok: true })); + const source = makeAgentSource(); + const d = new NotificationDispatcher({ + loadConfig: () => + makeConfig({ + events: { + "turn-completed": false, + "turn-error": true, + "permission-required": true, + "agent-spawned": false, + }, + }), + send, + }); + d.attachToAgentManager(source); + source.emit({ type: "done", tabId: "tab-1", message: { role: "assistant", chunks: [] } }); + await flush(); + expect(send).not.toHaveBeenCalled(); + }); +}); + +describe("NotificationDispatcher.attachToPermissionManager", () => { + let warnSpy: ReturnType; + beforeEach(() => { + warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + }); + afterEach(() => { + warnSpy.mockRestore(); + }); + + it("notifies once per unique prompt id (dedupes re-emits)", async () => { + const send = vi.fn(async () => ({ ok: true })); + const source = makePermissionSource(); + const d = new NotificationDispatcher({ loadConfig: () => makeConfig(), send }); + d.attachToPermissionManager(source); + + source.emit({ id: "1", permission: "bash", description: "Run git status" }); + source.emit({ id: "1", permission: "bash", description: "Run git status" }); + source.emit({ id: "2", permission: "read", description: "Read /etc/hosts" }); + await flush(); + expect(send).toHaveBeenCalledTimes(2); + const events = send.mock.calls.map((c) => c[1] as NotificationEvent); + expect(events.map((e) => e.type)).toEqual(["permission-required", "permission-required"]); + expect(events.every((e) => e.dedupeKey?.startsWith("permission:"))).toBe(true); + }); +}); + +describe("NotificationDispatcher.dispose", () => { + it("releases attached subscriptions", async () => { + const send = vi.fn(async () => ({ ok: true })); + const source = makeAgentSource(); + const d = new NotificationDispatcher({ loadConfig: () => makeConfig(), send }); + d.attachToAgentManager(source); + d.dispose(); + source.emit({ type: "done", tabId: "tab-1", message: { role: "assistant", chunks: [] } }); + await flush(); + expect(send).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/core/tests/notifications/ntfy.test.ts b/packages/core/tests/notifications/ntfy.test.ts new file mode 100644 index 0000000..3fb1d51 --- /dev/null +++ b/packages/core/tests/notifications/ntfy.test.ts @@ -0,0 +1,168 @@ +import { describe, expect, it, vi } from "vitest"; +import { sendNtfy, validateTopicUrl } from "../../src/notifications/ntfy.js"; +import type { NotificationEvent, NtfyConfig } from "../../src/notifications/types.js"; + +function makeConfig(overrides: Partial = {}): NtfyConfig { + return { + enabled: true, + topicUrl: "https://ntfy.sh/my-topic", + authToken: "", + events: { + "turn-completed": true, + "turn-error": true, + "permission-required": true, + "agent-spawned": true, + }, + ...overrides, + }; +} + +function makeEvent(overrides: Partial = {}): NotificationEvent { + return { + type: "turn-completed", + title: "Done", + message: "all good", + ...overrides, + }; +} + +function makeFetch( + response: Partial<{ ok: boolean; status: number; statusText: string; body: string }> = {}, +) { + const fetchImpl = vi.fn(async () => ({ + ok: response.ok ?? true, + status: response.status ?? 200, + statusText: response.statusText ?? "OK", + text: async () => response.body ?? "", + })); + return fetchImpl; +} + +describe("validateTopicUrl", () => { + it("accepts ntfy.sh-style URLs", () => { + expect(validateTopicUrl("https://ntfy.sh/my-topic")).toBeNull(); + expect(validateTopicUrl("http://ntfy.example.com/team-alerts")).toBeNull(); + }); + + it("rejects empty / whitespace", () => { + expect(validateTopicUrl("")).toMatch(/required/); + expect(validateTopicUrl(" ")).toMatch(/required/); + }); + + it("rejects malformed URLs", () => { + expect(validateTopicUrl("not a url")).toMatch(/valid URL/); + }); + + it("rejects non-http(s) schemes", () => { + expect(validateTopicUrl("ftp://ntfy.sh/topic")).toMatch(/http/); + }); + + it("rejects URLs missing a topic path", () => { + expect(validateTopicUrl("https://ntfy.sh")).toMatch(/topic/); + expect(validateTopicUrl("https://ntfy.sh/")).toMatch(/topic/); + }); +}); + +describe("sendNtfy", () => { + it("POSTs to the topic URL with Title/Priority/Tags/Content-Type headers and body", async () => { + const fetchImpl = makeFetch(); + const result = await sendNtfy( + makeConfig(), + makeEvent({ title: "Hello", message: "World", tags: ["bell"], priority: 4 }), + fetchImpl, + ); + expect(result.ok).toBe(true); + expect(fetchImpl).toHaveBeenCalledTimes(1); + const [url, init] = fetchImpl.mock.calls[0]; + expect(url).toBe("https://ntfy.sh/my-topic"); + expect(init.method).toBe("POST"); + expect(init.headers.Title).toBe("Hello"); + expect(init.headers.Priority).toBe("4"); + expect(init.headers.Tags).toBe("bell"); + expect(init.headers["Content-Type"]).toMatch(/text\/plain/); + expect(init.body).toBe("World"); + }); + + it("uses per-event-type defaults for priority and tags", async () => { + const fetchImpl = makeFetch(); + await sendNtfy(makeConfig(), makeEvent({ type: "turn-error" }), fetchImpl); + const init = fetchImpl.mock.calls[0][1]; + expect(init.headers.Priority).toBe("4"); // NTFY_DEFAULT_PRIORITIES["turn-error"] + expect(init.headers.Tags).toBe("rotating_light"); + }); + + it("attaches Authorization header when authToken is set", async () => { + const fetchImpl = makeFetch(); + await sendNtfy(makeConfig({ authToken: "tk_secret " }), makeEvent(), fetchImpl); + const init = fetchImpl.mock.calls[0][1]; + expect(init.headers.Authorization).toBe("Bearer tk_secret"); + }); + + it("omits Authorization when authToken is blank", async () => { + const fetchImpl = makeFetch(); + await sendNtfy(makeConfig({ authToken: " " }), makeEvent(), fetchImpl); + const init = fetchImpl.mock.calls[0][1]; + expect(init.headers.Authorization).toBeUndefined(); + }); + + it("attaches Click header when clickUrl is set", async () => { + const fetchImpl = makeFetch(); + await sendNtfy(makeConfig(), makeEvent({ clickUrl: "https://example.com/tab/abc" }), fetchImpl); + const init = fetchImpl.mock.calls[0][1]; + expect(init.headers.Click).toBe("https://example.com/tab/abc"); + }); + + it("appends short tab tag when tabId is set", async () => { + const fetchImpl = makeFetch(); + await sendNtfy( + makeConfig(), + makeEvent({ tabId: "abcdef0123456789", tags: ["bell"] }), + fetchImpl, + ); + const init = fetchImpl.mock.calls[0][1]; + expect(init.headers.Tags).toBe("bell,tab-abcdef01"); + }); + + it("strips CR/LF/control chars from header values (injection guard)", async () => { + const fetchImpl = makeFetch(); + await sendNtfy(makeConfig(), makeEvent({ title: "line1\r\nInjected: yes" }), fetchImpl); + const init = fetchImpl.mock.calls[0][1]; + expect(init.headers.Title).not.toContain("\n"); + expect(init.headers.Title).not.toContain("\r"); + expect(init.headers.Title).toBe("line1 Injected: yes"); + }); + + it("returns ok:false when notifications are disabled", async () => { + const fetchImpl = makeFetch(); + const result = await sendNtfy(makeConfig({ enabled: false }), makeEvent(), fetchImpl); + expect(result.ok).toBe(false); + expect(result.error).toMatch(/disabled/); + expect(fetchImpl).not.toHaveBeenCalled(); + }); + + it("returns ok:false on invalid topic URL without calling fetch", async () => { + const fetchImpl = makeFetch(); + const result = await sendNtfy(makeConfig({ topicUrl: "not a url" }), makeEvent(), fetchImpl); + expect(result.ok).toBe(false); + expect(result.error).toBeDefined(); + expect(fetchImpl).not.toHaveBeenCalled(); + }); + + it("returns ok:false with status on non-2xx response", async () => { + const fetchImpl = makeFetch({ ok: false, status: 403, statusText: "Forbidden", body: "nope" }); + const result = await sendNtfy(makeConfig(), makeEvent(), fetchImpl); + expect(result.ok).toBe(false); + expect(result.status).toBe(403); + expect(result.error).toMatch(/403/); + expect(result.error).toMatch(/nope/); + }); + + it("returns ok:false with error message on fetch throwing", async () => { + const fetchImpl = vi.fn(async () => { + throw new Error("ECONNREFUSED"); + }); + const result = await sendNtfy(makeConfig(), makeEvent(), fetchImpl); + expect(result.ok).toBe(false); + expect(result.error).toMatch(/ECONNREFUSED/); + }); +}); -- cgit v1.2.3 From 1870d0b86e797077b2a86c39d93fd34004b39e40 Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Mon, 1 Jun 2026 09:50:30 +0900 Subject: fix(notifications): address Gemini review — tighten validation, sanitize Click, support Basic auth, non-optimistic UI clear MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Acted on 4 of 6 findings from the gemini-3-flash-preview second-opinion review (the other 2 were verified-wrong or judged not worth the complexity — see HANDOFF.md). core/src/notifications/ntfy.ts: - validateTopicUrl now enforces ntfy's actual topic-name constraints: exactly one path segment, 1–64 chars, charset [A-Za-z0-9_-]. Prevents users from saving topic URLs that look fine but silently 404 at publish time (cf. binwiederhier/ntfy#1451 for the 64-char limit and binwiederhier/ntfy's topic-name regex for the charset). - Click header now passes through sanitizeHeader, closing the same CRLF-injection vector that Title/Tags already had. - Authorization header construction now factors through a small buildAuthHeaderValue helper: a value that already starts with a scheme token ("Bearer xyz", "Basic dXNlcjpwYXNz") is used verbatim, so users of private ntfy servers that want Basic auth can paste the full header value. Bare tokens still get the "Bearer " prefix automatically. frontend/SettingsPanel.svelte: - clearNtfyAuthToken() was optimistic: it flipped hasAuthToken=false locally before awaiting the network call. If the request failed the UI lied about server state, and worse — a subsequent Save() with authToken:undefined would silently re-arm the original token. Now awaits the response, surfaces failures via the existing ntfySaveError banner, and only mutates local state on success. Adds a ntfyClearingToken loading flag so the button disables + spins during the request. Tests: +6 in ntfy.test.ts (multi-segment rejection, charset rejection, length boundary, 64-char acceptance, Basic auth pass-through, Click sanitization). All 442 tests pass; biome clean; svelte-check clean; manual ntfy.sh end-to-end re-verified. --- packages/core/src/notifications/ntfy.ts | 56 ++++++++++++++++----- packages/core/tests/notifications/ntfy.test.ts | 45 ++++++++++++++++- .../src/lib/components/SettingsPanel.svelte | 57 ++++++++++++++++------ 3 files changed, 130 insertions(+), 28 deletions(-) (limited to 'packages/core') diff --git a/packages/core/src/notifications/ntfy.ts b/packages/core/src/notifications/ntfy.ts index 07ce33b..1f5101c 100644 --- a/packages/core/src/notifications/ntfy.ts +++ b/packages/core/src/notifications/ntfy.ts @@ -28,9 +28,17 @@ export type FetchLike = ( ) => Promise<{ ok: boolean; status: number; statusText?: string; text(): Promise }>; /** - * Validate a ntfy topic URL. Accepts only `http(s)://host/topic` with a - * non-empty topic path. Returns `null` on success, a human-readable error - * string on failure. + * ntfy topic-name rules: 1–64 chars, ASCII alphanumerics + `-` and `_`. Sourced + * from the ntfy server (cf. binwiederhier/ntfy issue #1451 — longer names + * silently 404). Matching this client-side keeps users from saving topic URLs + * that look fine but only fail at publish time. + */ +const NTFY_TOPIC_RE = /^[A-Za-z0-9_-]{1,64}$/; + +/** + * Validate a ntfy topic URL. Accepts only `http(s)://host/topic` where + * `topic` is a single path segment of 1–64 chars matching `[A-Za-z0-9_-]`. + * Returns `null` on success, a human-readable error string on failure. */ export function validateTopicUrl(topicUrl: string): string | null { const trimmed = topicUrl.trim(); @@ -44,9 +52,15 @@ export function validateTopicUrl(topicUrl: string): string | null { if (url.protocol !== "http:" && url.protocol !== "https:") { return "Topic URL must use http:// or https://"; } - // Path must be a non-empty topic (more than just "/") + // Path must be exactly one topic segment. const topic = url.pathname.replace(/^\/+|\/+$/g, ""); if (!topic) return "Topic URL must include a topic name (e.g. https://ntfy.sh/my-topic)"; + if (topic.includes("/")) { + return "Topic URL must point at a single topic (no extra path segments)"; + } + if (!NTFY_TOPIC_RE.test(topic)) { + return "Topic name must be 1–64 characters, letters/numbers/underscore/hyphen only"; + } return null; } @@ -79,17 +93,17 @@ export async function sendNtfy( } const headers: Record = { - // ntfy treats the title/priority/tags/click headers as ASCII-only. Strip - // control chars from the title; the body is sent UTF-8 verbatim. + // ntfy is tolerant of non-ASCII in the Title header but many proxies + // aren't — sanitizeHeader strips CR/LF/control chars (injection guard) + // and leaves UTF-8 in place. Body is sent verbatim as UTF-8. Title: sanitizeHeader(event.title), Priority: String(priority), "Content-Type": "text/plain; charset=utf-8", }; if (tags.length > 0) headers.Tags = tags.map(sanitizeHeader).join(","); - if (event.clickUrl) headers.Click = event.clickUrl; - if (config.authToken && config.authToken.trim() !== "") { - headers.Authorization = `Bearer ${config.authToken.trim()}`; - } + if (event.clickUrl) headers.Click = sanitizeHeader(event.clickUrl); + const authValue = buildAuthHeaderValue(config.authToken); + if (authValue) headers.Authorization = authValue; // Per-request abort so a hung server doesn't pin a Bun worker forever. const controller = new AbortController(); @@ -119,9 +133,27 @@ export async function sendNtfy( } } +/** + * Build the `Authorization` header value from the user-configured token. + * + * - Empty/blank ⇒ no header. + * - Already starts with `Bearer ` / `Basic ` / etc. (RFC 7235 scheme + space) + * ⇒ used verbatim. Lets users paste a complete header for Basic auth or + * any other scheme their private ntfy server supports. + * - Otherwise ⇒ prefixed with `Bearer ` (the common case). + */ +function buildAuthHeaderValue(rawToken: string): string | null { + const trimmed = (rawToken ?? "").trim(); + if (!trimmed) return null; + if (/^[A-Za-z][A-Za-z0-9._~+/-]*\s+\S/.test(trimmed)) { + // Already includes a scheme token (e.g. "Bearer xyz", "Basic dXNlcjpw"). + return sanitizeHeader(trimmed); + } + return `Bearer ${sanitizeHeader(trimmed)}`; +} + function sanitizeHeader(value: string): string { - // Strip CR/LF (header injection guard) and trim. ntfy is tolerant of - // non-ASCII in titles, but we still drop control chars. + // Strip CR/LF (header injection guard) and other control chars, then trim. // biome-ignore lint/suspicious/noControlCharactersInRegex: intentional return value.replace(/[\r\n\u0000-\u001f]+/g, " ").trim(); } diff --git a/packages/core/tests/notifications/ntfy.test.ts b/packages/core/tests/notifications/ntfy.test.ts index 3fb1d51..c183847 100644 --- a/packages/core/tests/notifications/ntfy.test.ts +++ b/packages/core/tests/notifications/ntfy.test.ts @@ -61,6 +61,27 @@ describe("validateTopicUrl", () => { expect(validateTopicUrl("https://ntfy.sh")).toMatch(/topic/); expect(validateTopicUrl("https://ntfy.sh/")).toMatch(/topic/); }); + + it("rejects multi-segment paths (topic must be a single segment)", () => { + expect(validateTopicUrl("https://ntfy.sh/team/sub")).toMatch(/single topic/); + }); + + it("rejects topic names with disallowed characters", () => { + expect(validateTopicUrl("https://ntfy.sh/has.dot")).toMatch(/letters\/numbers/); + expect(validateTopicUrl("https://ntfy.sh/has space")).toMatch(/letters\/numbers/); + expect(validateTopicUrl("https://ntfy.sh/has%20enc")).toMatch(/letters\/numbers/); + }); + + it("rejects topic names longer than 64 chars", () => { + const long = "a".repeat(65); + expect(validateTopicUrl(`https://ntfy.sh/${long}`)).toMatch(/64/); + }); + + it("accepts 64-char topic names and underscore/hyphen mixes", () => { + const maxlen = "a".repeat(64); + expect(validateTopicUrl(`https://ntfy.sh/${maxlen}`)).toBeNull(); + expect(validateTopicUrl("https://ntfy.sh/My_Topic-123")).toBeNull(); + }); }); describe("sendNtfy", () => { @@ -91,13 +112,23 @@ describe("sendNtfy", () => { expect(init.headers.Tags).toBe("rotating_light"); }); - it("attaches Authorization header when authToken is set", async () => { + it("attaches Authorization header with Bearer prefix when authToken is a bare token", async () => { const fetchImpl = makeFetch(); await sendNtfy(makeConfig({ authToken: "tk_secret " }), makeEvent(), fetchImpl); const init = fetchImpl.mock.calls[0][1]; expect(init.headers.Authorization).toBe("Bearer tk_secret"); }); + it("passes a pre-prefixed Authorization value (Basic, custom schemes) through verbatim", async () => { + const fetchImpl = makeFetch(); + await sendNtfy(makeConfig({ authToken: "Basic dXNlcjpwYXNz" }), makeEvent(), fetchImpl); + expect(fetchImpl.mock.calls[0][1].headers.Authorization).toBe("Basic dXNlcjpwYXNz"); + + const fetchImpl2 = makeFetch(); + await sendNtfy(makeConfig({ authToken: "Bearer already_prefixed" }), makeEvent(), fetchImpl2); + expect(fetchImpl2.mock.calls[0][1].headers.Authorization).toBe("Bearer already_prefixed"); + }); + it("omits Authorization when authToken is blank", async () => { const fetchImpl = makeFetch(); await sendNtfy(makeConfig({ authToken: " " }), makeEvent(), fetchImpl); @@ -112,6 +143,18 @@ describe("sendNtfy", () => { expect(init.headers.Click).toBe("https://example.com/tab/abc"); }); + it("sanitizes Click header (CR/LF injection guard)", async () => { + const fetchImpl = makeFetch(); + await sendNtfy( + makeConfig(), + makeEvent({ clickUrl: "https://example.com/\r\nInjected: yes" }), + fetchImpl, + ); + const v = fetchImpl.mock.calls[0][1].headers.Click; + expect(v).not.toContain("\n"); + expect(v).not.toContain("\r"); + }); + it("appends short tab tag when tabId is set", async () => { const fetchImpl = makeFetch(); await sendNtfy( diff --git a/packages/frontend/src/lib/components/SettingsPanel.svelte b/packages/frontend/src/lib/components/SettingsPanel.svelte index 1d9ebf7..8031500 100644 --- a/packages/frontend/src/lib/components/SettingsPanel.svelte +++ b/packages/frontend/src/lib/components/SettingsPanel.svelte @@ -72,6 +72,7 @@ let ntfySaveOk = $state(false); let ntfyTesting = $state(false); let ntfyTestResult = $state(null); let ntfyTestOk = $state(false); +let ntfyClearingToken = $state(false); function onChunkLimitChange(e: Event): void { const input = e.target as HTMLInputElement; @@ -212,22 +213,43 @@ async function sendNtfyTest(): Promise { } } -function clearNtfyAuthToken(): void { +async function clearNtfyAuthToken(): Promise { // `""` ⇒ explicit clear on save (vs. `undefined` which keeps existing). - ntfyAuthTokenInput = ""; - ntfy = { ...ntfy, hasAuthToken: false }; - // Send a save with explicit empty string to clear server-side. - void (async () => { - try { - await fetch(`${apiBase}/notifications`, { - method: "PUT", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ authToken: "" }), - }); - } catch { - // ignore + // Optimistic local state on failure caused a real bug pre-review: UI showed + // the token cleared while the server still held it, then "Save" treated the + // blank input as "keep existing" and silently re-armed the old token. Await + // the response and only flip local state on success. + ntfyClearingToken = true; + ntfySaveError = null; + try { + const res = await fetch(`${apiBase}/notifications`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ authToken: "" }), + }); + const data = (await res.json().catch(() => ({}))) as { + config?: NtfyConfigView; + error?: string; + }; + if (!res.ok) { + ntfySaveError = data.error ?? `Clear failed (HTTP ${res.status})`; + return; } - })(); + ntfyAuthTokenInput = ""; + if (data.config) { + ntfy = { + ...DEFAULT_NTFY, + ...data.config, + events: { ...DEFAULT_NTFY.events, ...(data.config.events ?? {}) }, + }; + } else { + ntfy = { ...ntfy, hasAuthToken: false }; + } + } catch (e) { + ntfySaveError = e instanceof Error ? e.message : "Network error"; + } finally { + ntfyClearingToken = false; + } } async function toggleAutoExpand(): Promise { @@ -421,9 +443,14 @@ $effect(() => { {/if} -- cgit v1.2.3 From 9c93086c0d4acaa1ed753488b12f72c2ca86a22c Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Mon, 1 Jun 2026 10:12:11 +0900 Subject: feat(notifications): add notifySubagents toggle to suppress subagent turn pings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A parent agent that spawns 8 subagents was producing 9 "Turn complete" notifications per round — almost always noise. New `notifySubagents` config flag (defaults to false) gates `turn-completed` and `turn-error` from any tab with a `parentTabId`. The flag is intentionally NOT applied to `permission-required` — a subagent's permission prompt still needs a human tap to proceed, so suppressing it would silently hang the subagent. `agent-spawned` is already top-level-only by construction. Wiring: - core/notifications/types.ts: NtfyConfig.notifySubagents: boolean - core/notifications/config.ts: defaults to false; normalize() tolerates missing / wrong-typed values and falls back to false - core/notifications/dispatcher.ts: new optional TabParentLookup option (getTabParentId). When notifySubagents=false AND the lookup returns a non-empty parent id string, turn-completed/turn-error are dropped. Lookup failures (no lookup configured, throws, returns undefined) fall back to "treat as top-level" so legitimate top-level events are never silently dropped when the DB is briefly unreadable. - api/app.ts: wires getTabParentId via core's getTab(id)?.parentTabId - frontend SettingsPanel.svelte: "Include subagent tabs" checkbox with an explanatory hint that permission prompts still fire Tests (+9): - 3 in config.test.ts: default-false, explicit-true, wrong-typed fallback - 6 in dispatcher.test.ts: suppression of turn-completed/turn-error from subagents, no suppression when flag is true, permission-required not gated, graceful fallback when lookup is missing/throws/returns undefined Live ntfy.sh round-trip re-verified (status: 200). --- packages/api/src/app.ts | 11 ++ packages/api/tests/routes.test.ts | 2 + packages/core/src/notifications/config.ts | 3 + packages/core/src/notifications/dispatcher.ts | 49 ++++++++ packages/core/src/notifications/index.ts | 1 + packages/core/src/notifications/types.ts | 8 ++ packages/core/tests/notifications/config.test.ts | 30 +++++ .../core/tests/notifications/dispatcher.test.ts | 138 +++++++++++++++++++++ .../src/lib/components/SettingsPanel.svelte | 17 +++ 9 files changed, 259 insertions(+) (limited to 'packages/core') diff --git a/packages/api/src/app.ts b/packages/api/src/app.ts index 24cef24..0dabb0d 100644 --- a/packages/api/src/app.ts +++ b/packages/api/src/app.ts @@ -24,6 +24,17 @@ export const notificationDispatcher = new NotificationDispatcher({ return null; } }, + getTabParentId: (tabId) => { + try { + // `undefined` when the lookup fails (tab not found / DB unavailable) + // so the dispatcher falls back to "treat as top-level" rather than + // silently dropping notifications. + const row = getTab(tabId); + return row ? row.parentTabId : undefined; + } catch { + return undefined; + } + }, }); notificationDispatcher.attachToAgentManager(agentManager); notificationDispatcher.attachToPermissionManager(permissionManager); diff --git a/packages/api/tests/routes.test.ts b/packages/api/tests/routes.test.ts index c07f932..1fad690 100644 --- a/packages/api/tests/routes.test.ts +++ b/packages/api/tests/routes.test.ts @@ -290,6 +290,7 @@ vi.mock("@dispatch/core", () => ({ "permission-required": true, "agent-spawned": false, }, + notifySubagents: false, }; }, saveNtfyConfig() {}, @@ -307,6 +308,7 @@ vi.mock("@dispatch/core", () => ({ "permission-required": true, "agent-spawned": false, }, + notifySubagents: false, }; }, redactNtfyConfig(c: { authToken?: string }) { diff --git a/packages/core/src/notifications/config.ts b/packages/core/src/notifications/config.ts index 310c606..faa316e 100644 --- a/packages/core/src/notifications/config.ts +++ b/packages/core/src/notifications/config.ts @@ -17,6 +17,7 @@ export function defaultNtfyConfig(): NtfyConfig { topicUrl: "", authToken: "", events: { ...NTFY_DEFAULT_EVENTS }, + notifySubagents: false, }; } @@ -34,6 +35,8 @@ export function normalizeNtfyConfig(raw: unknown): NtfyConfig { topicUrl: typeof obj.topicUrl === "string" ? obj.topicUrl : base.topicUrl, authToken: typeof obj.authToken === "string" ? obj.authToken : base.authToken, events: { ...base.events }, + notifySubagents: + typeof obj.notifySubagents === "boolean" ? obj.notifySubagents : base.notifySubagents, }; const rawEvents = obj.events; if (rawEvents && typeof rawEvents === "object") { diff --git a/packages/core/src/notifications/dispatcher.ts b/packages/core/src/notifications/dispatcher.ts index 4f4fc79..01ce00c 100644 --- a/packages/core/src/notifications/dispatcher.ts +++ b/packages/core/src/notifications/dispatcher.ts @@ -31,6 +31,15 @@ export interface PermissionPromptSource { /** Look up a human-readable tab title for nicer notification text. */ export type TabTitleLookup = (tabId: string) => string | null; +/** + * Look up a tab's `parentTabId`. Returns `null` for top-level tabs (no + * parent) and `undefined` when the lookup can't be performed (no DB, tab + * not found). Both non-strings cause the dispatcher to fall back to + * "treat as top-level" to avoid silently dropping notifications when the + * lookup is broken. + */ +export type TabParentLookup = (tabId: string) => string | null | undefined; + export interface DispatcherOptions { /** Override the config loader (tests). Defaults to `loadNtfyConfig`. */ loadConfig?: () => NtfyConfig; @@ -40,6 +49,13 @@ export interface DispatcherOptions { fetchImpl?: FetchLike; /** Look up a tab title for richer titles. */ getTabTitle?: TabTitleLookup; + /** + * Look up a tab's `parentTabId`. Used to honour the + * `notifySubagents` config flag — when false, `turn-completed` / + * `turn-error` from subagent tabs (those with a parent) are + * suppressed. + */ + getTabParentId?: TabParentLookup; /** * How long (ms) a dedupeKey is suppressed for. Permission prompts re-emit * the whole pending list on every change, so dedupe is essential. @@ -51,6 +67,7 @@ export class NotificationDispatcher { private loadConfig: () => NtfyConfig; private send: (config: NtfyConfig, event: NotificationEvent) => Promise; private getTabTitle: TabTitleLookup | undefined; + private getTabParentId: TabParentLookup | undefined; private dedupeWindowMs: number; /** Recently-sent dedupeKey → expiresAt epoch ms. */ private recentlySent = new Map(); @@ -61,6 +78,7 @@ export class NotificationDispatcher { this.send = opts.send ?? ((config, event) => sendNtfy(config, event, opts.fetchImpl ?? undefined)); this.getTabTitle = opts.getTabTitle; + this.getTabParentId = opts.getTabParentId; this.dedupeWindowMs = opts.dedupeWindowMs ?? 5_000; } @@ -101,12 +119,22 @@ export class NotificationDispatcher { * * `status` events are ignored — they fire on every transition and we'd * either spam or duplicate the `done`/`error` notifications. + * + * Turn events from subagent tabs are suppressed when + * `config.notifySubagents === false` (the default). A parent agent + * spawning 8 subagents would otherwise produce 9 "Turn complete" + * pushes per round; almost always noise. Permission prompts are NOT + * gated this way — a subagent's permission request still needs human + * input to proceed, so suppressing those would silently hang the + * subagent. */ attachToAgentManager(source: AgentEventSource): () => void { const unsub = source.onEvent((event) => { if (event.type === "done") { + if (this.isSubagentSuppressed(event.tabId)) return; this.notify(this.buildTurnCompleted(event)); } else if (event.type === "error") { + if (this.isSubagentSuppressed(event.tabId)) return; this.notify(this.buildTurnError(event)); } else if (event.type === "tab-created") { const ev = event as unknown as { @@ -213,6 +241,27 @@ export class NotificationDispatcher { return `tab ${tabId.slice(0, 8)}`; } + /** + * Returns true when this `tabId` belongs to a subagent AND the user has + * opted out of subagent turn notifications. On lookup failure + * (`getTabParentId` returns `undefined` or throws) we err on the side + * of "not a subagent" — better to over-notify than to silently drop + * legitimate top-level events when the DB is briefly unreadable. + */ + private isSubagentSuppressed(tabId: string): boolean { + const config = this.loadConfig(); + if (config.notifySubagents) return false; + if (!this.getTabParentId) return false; + let parent: string | null | undefined; + try { + parent = this.getTabParentId(tabId); + } catch { + return false; + } + // Only a non-empty string parent id means "this tab is a subagent". + return typeof parent === "string" && parent.length > 0; + } + // ─── Dedupe helpers ─────────────────────────────────────────── private isDuplicate(key: string): boolean { diff --git a/packages/core/src/notifications/index.ts b/packages/core/src/notifications/index.ts index ea99a58..d9d934d 100644 --- a/packages/core/src/notifications/index.ts +++ b/packages/core/src/notifications/index.ts @@ -14,6 +14,7 @@ export { type DispatcherOptions, NotificationDispatcher, type PermissionPromptSource, + type TabParentLookup, type TabTitleLookup, } from "./dispatcher.js"; export { type FetchLike, type NtfySendResult, sendNtfy, validateTopicUrl } from "./ntfy.js"; diff --git a/packages/core/src/notifications/types.ts b/packages/core/src/notifications/types.ts index f6baa27..238cb68 100644 --- a/packages/core/src/notifications/types.ts +++ b/packages/core/src/notifications/types.ts @@ -57,12 +57,20 @@ export interface NotificationEvent { * - `authToken` — optional bearer token for private ntfy servers. * - `events` — per-event-type enable map. Missing entries default to OFF * so a newly-added event type doesn't silently start firing. + * - `notifySubagents` — when false (default), `turn-completed` and + * `turn-error` notifications from subagent tabs (tabs with a + * `parentTabId`) are suppressed. A parent agent that spawns 8 + * subagents would otherwise push 9 "Turn complete" notifications per + * round — usually noise. `permission-required` is NOT gated: even a + * subagent's permission prompt needs a human tap to proceed. + * `agent-spawned` is already top-level-only by construction. */ export interface NtfyConfig { enabled: boolean; topicUrl: string; authToken: string; events: Record; + notifySubagents: boolean; } /** All event types this build knows about (the source of truth for UI). */ diff --git a/packages/core/tests/notifications/config.test.ts b/packages/core/tests/notifications/config.test.ts index 64a9637..c110788 100644 --- a/packages/core/tests/notifications/config.test.ts +++ b/packages/core/tests/notifications/config.test.ts @@ -34,6 +34,7 @@ describe("defaultNtfyConfig", () => { expect(cfg.events["turn-error"]).toBe(true); expect(cfg.events["permission-required"]).toBe(true); expect(cfg.events["agent-spawned"]).toBe(false); + expect(cfg.notifySubagents).toBe(false); }); }); @@ -72,6 +73,34 @@ describe("normalizeNtfyConfig", () => { }); }); +describe("normalizeNtfyConfig — notifySubagents", () => { + it("defaults notifySubagents to false when absent", () => { + const normalized = normalizeNtfyConfig({ + enabled: true, + topicUrl: "https://ntfy.sh/x", + }); + expect(normalized.notifySubagents).toBe(false); + }); + + it("respects an explicit notifySubagents=true", () => { + const normalized = normalizeNtfyConfig({ + enabled: true, + topicUrl: "https://ntfy.sh/x", + notifySubagents: true, + }); + expect(normalized.notifySubagents).toBe(true); + }); + + it("falls back to default when notifySubagents is wrong-typed", () => { + const normalized = normalizeNtfyConfig({ + enabled: true, + topicUrl: "https://ntfy.sh/x", + notifySubagents: "yes" as unknown, + }); + expect(normalized.notifySubagents).toBe(false); + }); +}); + describe("load/save round-trip", () => { beforeEach(() => { fakeSettings.clear(); @@ -92,6 +121,7 @@ describe("load/save round-trip", () => { "permission-required": true, "agent-spawned": true, }, + notifySubagents: true, } as const; saveNtfyConfig({ ...cfg }); const loaded = loadNtfyConfig(); diff --git a/packages/core/tests/notifications/dispatcher.test.ts b/packages/core/tests/notifications/dispatcher.test.ts index db05de4..750552c 100644 --- a/packages/core/tests/notifications/dispatcher.test.ts +++ b/packages/core/tests/notifications/dispatcher.test.ts @@ -25,6 +25,10 @@ function makeConfig(overrides: Partial = {}): NtfyConfig { "permission-required": true, "agent-spawned": true, }, + // Default to true in the test config so existing tests (which never + // configure a getTabParentId lookup) keep firing for tab-1 / tab-2 / etc. + // Tests of the new subagent gating override this explicitly. + notifySubagents: true, ...overrides, }; } @@ -321,3 +325,137 @@ describe("NotificationDispatcher.dispose", () => { expect(send).not.toHaveBeenCalled(); }); }); + +describe("NotificationDispatcher subagent suppression (notifySubagents flag)", () => { + let warnSpy: ReturnType; + beforeEach(() => { + warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + }); + afterEach(() => { + warnSpy.mockRestore(); + }); + + const parents = new Map([ + ["top-level", null], + ["subagent", "top-level"], + ]); + const getTabParentId = (id: string): string | null | undefined => parents.get(id); + + it("suppresses turn-completed from subagent tabs when notifySubagents=false (default)", async () => { + const send = vi.fn(async () => ({ ok: true })); + const source = makeAgentSource(); + const d = new NotificationDispatcher({ + loadConfig: () => makeConfig({ notifySubagents: false }), + send, + getTabParentId, + }); + d.attachToAgentManager(source); + + source.emit({ type: "done", tabId: "subagent", message: { role: "assistant", chunks: [] } }); + source.emit({ type: "done", tabId: "top-level", message: { role: "assistant", chunks: [] } }); + await flush(); + + expect(send).toHaveBeenCalledTimes(1); + expect((send.mock.calls[0][1] as NotificationEvent).tabId).toBe("top-level"); + }); + + it("suppresses turn-error from subagent tabs when notifySubagents=false", async () => { + const send = vi.fn(async () => ({ ok: true })); + const source = makeAgentSource(); + const d = new NotificationDispatcher({ + loadConfig: () => makeConfig({ notifySubagents: false }), + send, + getTabParentId, + }); + d.attachToAgentManager(source); + + source.emit({ type: "error", tabId: "subagent", error: "boom" }); + source.emit({ type: "error", tabId: "top-level", error: "boom" }); + await flush(); + + expect(send).toHaveBeenCalledTimes(1); + expect((send.mock.calls[0][1] as NotificationEvent).tabId).toBe("top-level"); + }); + + it("still notifies subagents when notifySubagents=true", async () => { + const send = vi.fn(async () => ({ ok: true })); + const source = makeAgentSource(); + const d = new NotificationDispatcher({ + loadConfig: () => makeConfig({ notifySubagents: true }), + send, + getTabParentId, + }); + d.attachToAgentManager(source); + + source.emit({ type: "done", tabId: "subagent", message: { role: "assistant", chunks: [] } }); + source.emit({ type: "done", tabId: "top-level", message: { role: "assistant", chunks: [] } }); + await flush(); + + expect(send).toHaveBeenCalledTimes(2); + }); + + it("does NOT gate permission-required (subagents must still get human input)", async () => { + const send = vi.fn(async () => ({ ok: true })); + const psource = makePermissionSource(); + const d = new NotificationDispatcher({ + loadConfig: () => makeConfig({ notifySubagents: false }), + send, + getTabParentId, + }); + d.attachToPermissionManager(psource); + + psource.emit({ id: "p1", permission: "bash", description: "git status" }); + await flush(); + + expect(send).toHaveBeenCalledTimes(1); + expect((send.mock.calls[0][1] as NotificationEvent).type).toBe("permission-required"); + }); + + it("falls back to notifying when getTabParentId is not provided (treat as top-level)", async () => { + const send = vi.fn(async () => ({ ok: true })); + const source = makeAgentSource(); + const d = new NotificationDispatcher({ + loadConfig: () => makeConfig({ notifySubagents: false }), + send, + // intentionally NO getTabParentId + }); + d.attachToAgentManager(source); + + source.emit({ type: "done", tabId: "anything", message: { role: "assistant", chunks: [] } }); + await flush(); + + // Without a lookup, the dispatcher can't prove this is a subagent; it + // must err on the side of notifying so legitimate top-level events + // aren't silently dropped. + expect(send).toHaveBeenCalledTimes(1); + }); + + it("falls back to notifying when getTabParentId throws or returns undefined", async () => { + const send = vi.fn(async () => ({ ok: true })); + const source = makeAgentSource(); + const d = new NotificationDispatcher({ + loadConfig: () => makeConfig({ notifySubagents: false }), + send, + getTabParentId: () => { + throw new Error("db unavailable"); + }, + }); + d.attachToAgentManager(source); + + source.emit({ type: "done", tabId: "x", message: { role: "assistant", chunks: [] } }); + await flush(); + expect(send).toHaveBeenCalledTimes(1); + + const send2 = vi.fn(async () => ({ ok: true })); + const source2 = makeAgentSource(); + const d2 = new NotificationDispatcher({ + loadConfig: () => makeConfig({ notifySubagents: false }), + send: send2, + getTabParentId: () => undefined, + }); + d2.attachToAgentManager(source2); + source2.emit({ type: "done", tabId: "x", message: { role: "assistant", chunks: [] } }); + await flush(); + expect(send2).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/frontend/src/lib/components/SettingsPanel.svelte b/packages/frontend/src/lib/components/SettingsPanel.svelte index 8031500..08e86ac 100644 --- a/packages/frontend/src/lib/components/SettingsPanel.svelte +++ b/packages/frontend/src/lib/components/SettingsPanel.svelte @@ -36,6 +36,7 @@ interface NtfyConfigView { authToken: string; hasAuthToken?: boolean; events: Record; + notifySubagents: boolean; } const NTFY_EVENT_LABELS: Record = { @@ -56,6 +57,7 @@ const DEFAULT_NTFY: NtfyConfigView = { "permission-required": true, "agent-spawned": false, }, + notifySubagents: false, }; let ntfy = $state({ ...DEFAULT_NTFY, events: { ...DEFAULT_NTFY.events } }); @@ -162,6 +164,7 @@ async function saveNtfy(): Promise { enabled: ntfy.enabled, topicUrl: ntfy.topicUrl, events: ntfy.events, + notifySubagents: ntfy.notifySubagents, }; if (ntfyAuthTokenInput !== "") payload.authToken = ntfyAuthTokenInput; const res = await fetch(`${apiBase}/notifications`, { @@ -469,6 +472,20 @@ $effect(() => { {/each} +
+ + + Off (default): turn-completed/turn-error from subagents are suppressed. Permission prompts still fire so subagents don't silently hang. + +
+