diff --git a/packages/cli/__tests__/commands/doctor.test.ts b/packages/cli/__tests__/commands/doctor.test.ts index 5c2f7ce8ed..541fe4a9e7 100644 --- a/packages/cli/__tests__/commands/doctor.test.ts +++ b/packages/cli/__tests__/commands/doctor.test.ts @@ -41,6 +41,13 @@ vi.mock("../../src/lib/script-runner.js", () => ({ vi.mock("@aoagents/ao-core", () => ({ buildCIFailureNotificationData: () => ({ schemaVersion: 3 }), + buildNotificationPresentation: (event: { priority: string; message: string }) => ({ + version: 1, + category: "generic", + priority: event.priority, + title: "Test notification", + body: event.message, + }), buildPRStateNotificationData: () => ({ schemaVersion: 3 }), buildReactionNotificationData: () => ({ schemaVersion: 3 }), buildSessionTransitionNotificationData: () => ({ schemaVersion: 3 }), @@ -50,6 +57,10 @@ vi.mock("@aoagents/ao-core", () => ({ getObservabilityBaseDir: () => "/tmp/.agent-orchestrator/observability", loadConfig: (...args: unknown[]) => mockLoadConfig(...args), recordNotificationDelivery: (...args: unknown[]) => mockRecordNotificationDelivery(...args), + resolveNotificationRoute: ( + config: { defaults?: { notifiers?: string[] }; notificationRouting?: Record }, + priority: string, + ) => config.notificationRouting?.[priority] ?? config.defaults?.notifiers ?? [], resolveNotifierTarget: ( config: { notifiers?: Record }, reference: string, diff --git a/packages/cli/__tests__/commands/notify.test.ts b/packages/cli/__tests__/commands/notify.test.ts index 2f0d94f616..f8dd2c67eb 100644 --- a/packages/cli/__tests__/commands/notify.test.ts +++ b/packages/cli/__tests__/commands/notify.test.ts @@ -61,6 +61,30 @@ vi.mock("@aoagents/ao-core", () => { reference, pluginName: config.notifiers?.[reference]?.plugin ?? reference, }), + resolveNotificationRoute: ( + config: { + defaults?: { notifiers?: string[] }; + notificationRouting?: Record; + }, + priority: string, + ) => { + if (Object.prototype.hasOwnProperty.call(config.notificationRouting ?? {}, priority)) { + return config.notificationRouting?.[priority] ?? []; + } + const defaults = config.defaults?.notifiers ?? []; + if (priority === "urgent" || priority === "warning") { + const withoutDesktop = defaults.filter((notifier) => notifier !== "desktop"); + return defaults.includes("desktop") ? ["desktop", ...withoutDesktop] : withoutDesktop; + } + return defaults.filter((notifier) => notifier !== "desktop"); + }, + buildNotificationPresentation: (event: { priority: string; message: string }) => ({ + version: 1, + category: "generic", + priority: event.priority, + title: "Test notification", + body: event.message, + }), buildCIFailureNotificationData: (input: { sessionId: string; projectId: string; diff --git a/packages/cli/__tests__/commands/setup.test.ts b/packages/cli/__tests__/commands/setup.test.ts index 3c8155e331..521000d40d 100644 --- a/packages/cli/__tests__/commands/setup.test.ts +++ b/packages/cli/__tests__/commands/setup.test.ts @@ -268,7 +268,7 @@ describe("setup dashboard command", () => { vi.unstubAllGlobals(); }); - it("writes dashboard notifier config with the urgent-action routing default", async () => { + it("adds dashboard to defaults and writes limit overrides without default routing", async () => { const program = createProgram(); await program.parseAsync([ @@ -283,15 +283,31 @@ describe("setup dashboard command", () => { const written = String(mockWriteFileSync.mock.calls[0][1]); const parsed = parseYaml(written) as { + defaults?: { notifiers?: string[] }; notifiers?: Record; notificationRouting?: Record; }; + expect(parsed.defaults?.notifiers).toEqual(["dashboard"]); expect(parsed.notifiers?.["dashboard"]).toEqual({ plugin: "dashboard", limit: 75 }); - expect(parsed.notificationRouting?.urgent).toContain("dashboard"); - expect(parsed.notificationRouting?.action).toContain("dashboard"); - expect(parsed.notificationRouting?.warning ?? []).not.toContain("dashboard"); - expect(parsed.notificationRouting?.info ?? []).not.toContain("dashboard"); + expect(parsed.notificationRouting).toBeUndefined(); + }); + + it("adds dashboard to defaults without writing a config block for default options", async () => { + const program = createProgram(); + + await program.parseAsync(["node", "test", "setup", "dashboard", "--non-interactive"]); + + const written = String(mockWriteFileSync.mock.calls[0][1]); + const parsed = parseYaml(written) as { + defaults?: { notifiers?: string[] }; + notifiers?: Record; + notificationRouting?: Record; + }; + + expect(parsed.defaults?.notifiers).toEqual(["dashboard"]); + expect(parsed.notifiers?.["dashboard"]).toBeUndefined(); + expect(parsed.notificationRouting).toBeUndefined(); }); it("prints status without mutating config", async () => { @@ -2717,7 +2733,7 @@ describe("setup desktop command", () => { expect(setup?.commands.some((command) => command.name() === "desktop")).toBe(true); }); - it("installs the bundled app and wires desktop routing to all priorities", async () => { + it("installs the bundled app and enables desktop without default config or routing blocks", async () => { const program = createProgram(); await program.parseAsync(["node", "test", "setup", "desktop", "--non-interactive"]); @@ -2725,19 +2741,14 @@ describe("setup desktop command", () => { expect(mockCpSync).toHaveBeenCalledWith(sourceApp, targetApp, { recursive: true }); const writtenYaml = mockWriteFileSync.mock.calls[0][1] as string; const parsed = parseYaml(writtenYaml) as { + defaults?: { notifiers?: string[] }; notifiers?: Record; notificationRouting?: Record; }; - expect(parsed.notifiers?.["desktop"]).toMatchObject({ - plugin: "desktop", - backend: "ao-app", - dashboardUrl: "http://localhost:3000", - }); - expect(parsed.notificationRouting?.["urgent"]).toContain("desktop"); - expect(parsed.notificationRouting?.["action"]).toContain("desktop"); - expect(parsed.notificationRouting?.["warning"]).toContain("desktop"); - expect(parsed.notificationRouting?.["info"]).toContain("desktop"); + expect(parsed.defaults?.notifiers).toEqual(["desktop"]); + expect(parsed.notifiers?.["desktop"]).toBeUndefined(); + expect(parsed.notificationRouting).toBeUndefined(); }); it("configures terminal-notifier backend without installing AO Notifier.app", async () => { @@ -2773,8 +2784,10 @@ describe("setup desktop command", () => { const writtenYaml = mockWriteFileSync.mock.calls[0][1] as string; const parsed = parseYaml(writtenYaml) as { + defaults?: { notifiers?: string[] }; notifiers?: Record; }; + expect(parsed.defaults?.notifiers).toEqual(["desktop"]); expect(parsed.notifiers?.["desktop"]).toMatchObject({ plugin: "desktop", backend: "terminal-notifier", @@ -2808,8 +2821,10 @@ describe("setup desktop command", () => { const writtenYaml = mockWriteFileSync.mock.calls[0][1] as string; const parsed = parseYaml(writtenYaml) as { + defaults?: { notifiers?: string[] }; notifiers?: Record; }; + expect(parsed.defaults?.notifiers).toEqual(["desktop"]); expect(parsed.notifiers?.["desktop"]).toMatchObject({ plugin: "desktop", backend: "osascript", @@ -2952,7 +2967,7 @@ projects: expect(mockWriteFileSync).not.toHaveBeenCalled(); }); - it("preserves existing routing entries while adding desktop", async () => { + it("preserves existing routing entries while adding desktop to defaults", async () => { mockReadFileSync.mockReturnValue(` port: 3001 defaults: @@ -2977,9 +2992,10 @@ projects: notificationRouting?: Record; defaults?: { notifiers?: string[] }; }; - expect(parsed.notificationRouting?.["urgent"]).toEqual(["slack", "desktop"]); - expect(parsed.notificationRouting?.["action"]).toEqual(["slack", "desktop"]); - expect(parsed.defaults?.notifiers).toEqual(["slack"]); + expect(parsed.notificationRouting).toEqual({ + urgent: ["slack"], + }); + expect(parsed.defaults?.notifiers).toEqual(["slack", "desktop"]); }); it("fails on conflicting desktop notifier config in non-interactive mode", async () => { @@ -3043,8 +3059,10 @@ projects: const writtenYaml = mockWriteFileSync.mock.calls[0][1] as string; const parsed = parseYaml(writtenYaml) as { + defaults?: { notifiers?: string[] }; notifiers?: Record; }; + expect(parsed.defaults?.notifiers).toEqual(["desktop"]); expect(parsed.notifiers?.["desktop"]).toMatchObject({ plugin: "desktop", backend: "ao-app" }); }); diff --git a/packages/cli/__tests__/commands/start.test.ts b/packages/cli/__tests__/commands/start.test.ts index 1b7a3f5935..4a00cca439 100644 --- a/packages/cli/__tests__/commands/start.test.ts +++ b/packages/cli/__tests__/commands/start.test.ts @@ -19,11 +19,7 @@ import { basename, join } from "node:path"; import { tmpdir } from "node:os"; import { parse as parseYaml } from "yaml"; import { EventEmitter } from "node:events"; -import { - getDefaultRuntime, - recordActivityEvent, - type SessionManager, -} from "@aoagents/ao-core"; +import { getDefaultRuntime, recordActivityEvent, type SessionManager } from "@aoagents/ao-core"; // --------------------------------------------------------------------------- // Hoisted mocks @@ -77,6 +73,11 @@ const { mockProcessCwd } = vi.hoisted(() => ({ mockProcessCwd: vi.fn<() => string | undefined>(), })); +const { mockIsMac, mockInstallAoNotifierAppForStartup } = vi.hoisted(() => ({ + mockIsMac: vi.fn().mockReturnValue(false), + mockInstallAoNotifierAppForStartup: vi.fn().mockResolvedValue(undefined), +})); + const { mockPromptSelect, mockPromptConfirm } = vi.hoisted(() => ({ mockPromptSelect: vi.fn(), mockPromptConfirm: vi.fn().mockResolvedValue(true), @@ -154,6 +155,7 @@ vi.mock("@aoagents/ao-core", async (importOriginal) => { }, findPidByPort: mockFindPidByPort, killProcessTree: mockKillProcessTree, + isMac: () => mockIsMac(), sweepDaemonChildren: mockSweepDaemonChildren, scanAoOrphans: mockScanAoOrphans, reapAoOrphans: mockReapAoOrphans, @@ -246,6 +248,11 @@ vi.mock("../../src/lib/openclaw-probe.js", () => ({ detectOpenClawInstallation: (...args: unknown[]) => mockDetectOpenClawInstallation(...args), })); +vi.mock("../../src/lib/desktop-setup.js", () => ({ + installAoNotifierAppForStartup: (...args: unknown[]) => + mockInstallAoNotifierAppForStartup(...args), +})); + vi.mock("../../src/lib/prompts.js", () => ({ promptSelect: (...args: unknown[]) => mockPromptSelect(...args), promptConfirm: (...args: unknown[]) => mockPromptConfirm(...args), @@ -412,6 +419,10 @@ beforeEach(async () => { }); mockWaitForPortAndOpen.mockReset(); mockWaitForPortAndOpen.mockResolvedValue(undefined); + mockIsMac.mockReset(); + mockIsMac.mockReturnValue(false); + mockInstallAoNotifierAppForStartup.mockReset(); + mockInstallAoNotifierAppForStartup.mockResolvedValue(undefined); mockFindPidByPort.mockReset(); mockFindPidByPort.mockResolvedValue(null); mockKillProcessTree.mockReset(); @@ -550,6 +561,87 @@ describe("start command — project resolution", () => { expect(output).toContain("Startup complete"); }); + it("does not expand default notifier config or routing during startup", async () => { + const configPath = join(tmpDir, "agent-orchestrator.yaml"); + const yaml = [ + "port: 3000", + "defaults:", + " notifiers:", + " - dashboard", + " - desktop", + "projects:", + " my-app:", + " name: My App", + " path: ./main-repo", + "", + ].join("\n"); + writeFileSync(configPath, yaml); + mockConfigRef.current = { + ...makeConfig({ "my-app": makeProject() }), + configPath, + defaults: { + runtime: "process", + agent: "claude-code", + workspace: "worktree", + notifiers: ["dashboard", "desktop"], + }, + notifiers: {}, + notificationRouting: {}, + }; + + await program.parseAsync(["node", "test", "start", "--no-dashboard", "--no-orchestrator"]); + + expect(readFileSync(configPath, "utf-8")).toBe(yaml); + }); + + it("installs AO Notifier.app on macOS without expanding notifier YAML", async () => { + mockIsMac.mockReturnValue(true); + const configPath = join(tmpDir, "agent-orchestrator.yaml"); + const yaml = [ + "port: 3000", + "defaults:", + " notifiers:", + " - dashboard", + " - desktop", + "projects:", + " my-app:", + " name: My App", + " path: ./main-repo", + "", + ].join("\n"); + writeFileSync(configPath, yaml); + mockConfigRef.current = { + ...makeConfig({ "my-app": makeProject() }), + configPath, + defaults: { + runtime: "process", + agent: "claude-code", + workspace: "worktree", + notifiers: ["dashboard", "desktop"], + }, + notifiers: {}, + notificationRouting: {}, + }; + + await program.parseAsync(["node", "test", "start", "--no-dashboard", "--no-orchestrator"]); + + expect(mockInstallAoNotifierAppForStartup).toHaveBeenCalledOnce(); + expect(readFileSync(configPath, "utf-8")).toBe(yaml); + }); + + it("continues startup with a warning when macOS AO Notifier.app setup fails", async () => { + mockIsMac.mockReturnValue(true); + mockInstallAoNotifierAppForStartup.mockRejectedValueOnce(new Error("copy failed")); + mockConfigRef.current = makeConfig({ "my-app": makeProject() }); + + await program.parseAsync(["node", "test", "start", "--no-dashboard", "--no-orchestrator"]); + + const output = vi.mocked(console.log).mock.calls.map((call) => call.join(" ")).join("\n"); + expect(output).toContain("Could not set up AO Notifier.app"); + expect(output).toContain("copy failed"); + expect(output).toContain("Startup complete"); + }); + it("uses explicit project arg when given", async () => { mockConfigRef.current = makeConfig({ frontend: makeProject({ name: "Frontend", sessionPrefix: "fe" }), @@ -2366,7 +2458,7 @@ describe("start command — platform-aware runtime fallback", () => { // --------------------------------------------------------------------------- describe("start command — autoCreateConfig", () => { - it("writes a flat local config and returns the global project identity", async () => { + it("writes a flat local config, returns global project identity, and creates config-light notifier defaults", async () => { const { detectEnvironment } = await import("../../src/lib/detect-env.js"); vi.mocked(detectEnvironment).mockResolvedValue({ isGitRepo: true, @@ -2418,15 +2510,26 @@ describe("start command — autoCreateConfig", () => { const globalContent = readFileSync(globalConfigPath, "utf-8"); const globalParsed = parseYaml(globalContent) as { defaults?: { notifiers?: unknown[] }; + notifiers?: Record< + string, + { plugin?: string; backend?: string; dashboardUrl?: string; limit?: number } + >; + notificationRouting?: Record; projects?: Record; }; - expect(globalParsed.defaults?.notifiers).toEqual(["composio", "desktop"]); + expect(globalParsed.defaults?.notifiers).toEqual(["dashboard", "desktop"]); + expect(globalParsed.notifiers).toEqual({}); + expect(globalParsed.notificationRouting).toEqual({}); + expect(globalContent).not.toContain("composio"); const projectIds = Object.keys(globalParsed.projects ?? {}); expect(projectIds).toHaveLength(1); expect(config.configPath).toBe(configPath); expect(Object.keys(config.projects)).toEqual(projectIds); expect(config.projects[projectIds[0]!]!.path).toBe(realpathSync(tmpDir)); + expect(config.defaults.notifiers).toEqual(["dashboard", "desktop"]); + expect(config.notifiers).toEqual({}); + expect(config.notificationRouting).toEqual({}); }); it("removes the flat local config when global registration fails", async () => { @@ -2459,12 +2562,9 @@ describe("start command — autoCreateConfig", () => { writeFileSync( process.env["AO_GLOBAL_CONFIG"]!, - [ - "projects:", - ` ${basename(tmpDir)}:`, - ` path: ${join(tmpDir, "other-repo")}`, - "", - ].join("\n"), + ["projects:", ` ${basename(tmpDir)}:`, ` path: ${join(tmpDir, "other-repo")}`, ""].join( + "\n", + ), ); await expect(autoCreateConfig(tmpDir)).rejects.toThrow("already registered"); diff --git a/packages/cli/__tests__/lib/notify-test.test.ts b/packages/cli/__tests__/lib/notify-test.test.ts index f93de3d18d..5c576140ff 100644 --- a/packages/cli/__tests__/lib/notify-test.test.ts +++ b/packages/cli/__tests__/lib/notify-test.test.ts @@ -141,6 +141,7 @@ describe("notify test helper", () => { const result = await runNotifyTest(config, registry, { to: ["alerts"] }); expect(result.ok).toBe(true); + expect(result.notification.title).toBe("All sessions complete"); expect(result.targets).toEqual([{ reference: "alerts", pluginName: "slack" }]); expect(registry.get).toHaveBeenCalledWith("notifier", "alerts"); expect(registry.get).toHaveBeenCalledWith("notifier", "slack"); @@ -159,6 +160,32 @@ describe("notify test helper", () => { ]); }); + it("uses implicit dashboard and desktop routing for default routes", () => { + const config = makeConfig({ + defaults: { + runtime: "tmux", + agent: "claude-code", + workspace: "worktree", + notifiers: ["dashboard", "desktop"], + }, + notifiers: {}, + notificationRouting: {}, + }); + + expect( + resolveNotifyTestTargets(config, "urgent", {}).map((target) => target.reference), + ).toEqual(["desktop", "dashboard"]); + expect( + resolveNotifyTestTargets(config, "warning", {}).map((target) => target.reference), + ).toEqual(["desktop", "dashboard"]); + expect( + resolveNotifyTestTargets(config, "action", {}).map((target) => target.reference), + ).toEqual(["dashboard"]); + expect( + resolveNotifyTestTargets(config, "info", {}).map((target) => target.reference), + ).toEqual(["dashboard"]); + }); + it("deduplicates --all targets across configured, default, and routed refs", () => { const config = makeConfig({ defaults: { diff --git a/packages/cli/__tests__/lib/path-equality.test.ts b/packages/cli/__tests__/lib/path-equality.test.ts index dd3c86bcbe..68bb3db4eb 100644 --- a/packages/cli/__tests__/lib/path-equality.test.ts +++ b/packages/cli/__tests__/lib/path-equality.test.ts @@ -82,8 +82,7 @@ describe("canonicalCompareKey", () => { process.env["HOME"] = tmpDir; try { const key = canonicalCompareKey("~"); - // On Windows the result is lowercased; on POSIX it's case-preserved. - expect(key.toLowerCase()).toBe(tmpDir.toLowerCase()); + expect(key).toBe(canonicalCompareKey(tmpDir)); } finally { if (originalHome === undefined) delete process.env["HOME"]; else process.env["HOME"] = originalHome; diff --git a/packages/cli/src/commands/notify.ts b/packages/cli/src/commands/notify.ts index e916d24cdd..b8b21c48f0 100644 --- a/packages/cli/src/commands/notify.ts +++ b/packages/cli/src/commands/notify.ts @@ -62,6 +62,8 @@ function printHumanResult(result: NotifyTestResult, sinkRequests?: unknown[]): v ); console.log(`Event id: ${result.event.id}`); console.log(`Session: ${result.event.projectId}/${result.event.sessionId}`); + console.log(`Title: ${result.notification.title}`); + console.log(`Body: ${result.notification.body}`); if (result.targets.length > 0) { console.log(""); diff --git a/packages/cli/src/commands/start.ts b/packages/cli/src/commands/start.ts index 4f5d86eb01..f1cda43965 100644 --- a/packages/cli/src/commands/start.ts +++ b/packages/cli/src/commands/start.ts @@ -110,6 +110,7 @@ import { installShutdownHandlers, isShutdownInProgress } from "../lib/shutdown.j import { resolveOrCreateProject } from "../lib/resolve-project.js"; import { pathsEqual } from "../lib/path-equality.js"; import { maybePromptForUpdateChannel } from "../lib/update-channel-onboarding.js"; +import { installAoNotifierAppForStartup } from "../lib/desktop-setup.js"; import { DEFAULT_PORT } from "../lib/constants.js"; import { projectSessionUrl } from "../lib/routes.js"; @@ -129,6 +130,26 @@ function isCliFailureEventRecordedError(err: unknown): boolean { return err instanceof CliFailureEventRecordedError; } +async function ensureDefaultStartupNotifiers( + config: OrchestratorConfig, +): Promise { + if (isMac()) { + try { + await installAoNotifierAppForStartup(); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + console.log( + chalk.yellow( + "⚠ Could not set up AO Notifier.app; continuing with dashboard notifications only.", + ), + ); + console.log(chalk.dim(` ${message}`)); + } + } + + return config; +} + function readProjectBehaviorConfig(projectPath: string): LocalProjectConfig { const localConfig = loadLocalProjectConfigDetailed(projectPath); if (localConfig.kind === "loaded") { @@ -874,6 +895,7 @@ async function runStartup( // feature ships. No-op on subsequent runs (idempotent — guarded by the // presence of `updateChannel` in the global config). await maybePromptForUpdateChannel(); + config = await ensureDefaultStartupNotifiers(config); // Install the parent shutdown path before spawning any managed children. // This guarantees a SIGINT/SIGTERM in the middle of startup still performs @@ -1843,303 +1865,306 @@ export function registerStop(program: Command): void { .option("--purge-session", "Delete mapped OpenCode session when stopping") .option("--all", "Stop all running AO instances") .option("-y, --yes", "Confirm stopping active sessions without prompting") - .action(async (projectArg?: string, opts: { purgeSession?: boolean; all?: boolean; yes?: boolean } = {}) => { - recordActivityEvent({ - source: "cli", - kind: "cli.stop_invoked", - level: "info", - summary: "ao stop invoked", - data: { - projectArg: projectArg ?? null, - all: opts.all === true, - purgeSession: opts.purgeSession === true, - }, - }); - try { - // Check running.json first - const running = await getRunning(); + .action( + async ( + projectArg?: string, + opts: { purgeSession?: boolean; all?: boolean; yes?: boolean } = {}, + ) => { + recordActivityEvent({ + source: "cli", + kind: "cli.stop_invoked", + level: "info", + summary: "ao stop invoked", + data: { + projectArg: projectArg ?? null, + all: opts.all === true, + purgeSession: opts.purgeSession === true, + }, + }); + try { + // Check running.json first + const running = await getRunning(); + + if (opts.all) { + // --all: kill via running.json if available, then fallback to config + if (running) { + // Sweep detached Windows pty-hosts BEFORE killing the parent. + // detached:true puts them outside the parent's process tree, so + // taskkill /T cannot reach them. The sweep speaks the named-pipe + // protocol so node-pty disposes ConPTY gracefully (avoids WER + // 0x800700e8). No-op on non-Windows. + await sweepWindowsPtyHostsBeforeParentKill(); + await sweepRegisteredDaemonChildren(running.pid); + // killProcessTree handles process trees on Windows (taskkill /T /F) + // and process groups on Unix; it swallows "already dead" internally. + await killProcessTree(running.pid, "SIGTERM"); + await unregister(); + console.log(chalk.green(`\n✓ Stopped AO on port ${running.port}`)); + console.log(chalk.dim(` Projects: ${running.projects.join(", ")}\n`)); + } else { + console.log(chalk.yellow("No running AO instance found in running.json.")); + } + return; + } - if (opts.all) { - // --all: kill via running.json if available, then fallback to config - if (running) { - // Sweep detached Windows pty-hosts BEFORE killing the parent. - // detached:true puts them outside the parent's process tree, so - // taskkill /T cannot reach them. The sweep speaks the named-pipe - // protocol so node-pty disposes ConPTY gracefully (avoids WER - // 0x800700e8). No-op on non-Windows. - await sweepWindowsPtyHostsBeforeParentKill(); - await sweepRegisteredDaemonChildren(running.pid); - // killProcessTree handles process trees on Windows (taskkill /T /F) - // and process groups on Unix; it swallows "already dead" internally. - await killProcessTree(running.pid, "SIGTERM"); - await unregister(); - console.log(chalk.green(`\n✓ Stopped AO on port ${running.port}`)); - console.log(chalk.dim(` Projects: ${running.projects.join(", ")}\n`)); + let config = loadConfig(); + // ao stop affects all projects (it kills the parent ao start process), + // so load the global config which has all registered projects. + // When a specific project is targeted, only fall back to global if + // the project isn't in the local config. + if (!projectArg || !config.projects[projectArg]) { + const globalPath = getGlobalConfigPath(); + if (existsSync(globalPath)) { + config = loadConfig(globalPath); + } + } + let _projectId: string; + let project: ProjectConfig; + if (projectArg) { + ({ projectId: _projectId, project } = await resolveProject(config, projectArg, "stop")); } else { - console.log(chalk.yellow("No running AO instance found in running.json.")); + const projectIds = Object.keys(config.projects); + if (projectIds.length === 0) { + throw new Error("No projects configured. Add a project to agent-orchestrator.yaml."); + } + const currentDir = resolve(cwd()); + const cwdProjectId = findProjectForDirectory(config.projects, currentDir); + _projectId = + running?.projects.find((id) => config.projects[id]) ?? cwdProjectId ?? projectIds[0]; + project = config.projects[_projectId]; } - return; - } + const port = config.port ?? DEFAULT_PORT; - let config = loadConfig(); - // ao stop affects all projects (it kills the parent ao start process), - // so load the global config which has all registered projects. - // When a specific project is targeted, only fall back to global if - // the project isn't in the local config. - if (!projectArg || !config.projects[projectArg]) { - const globalPath = getGlobalConfigPath(); - if (existsSync(globalPath)) { - config = loadConfig(globalPath); - } - } - let _projectId: string; - let project: ProjectConfig; - if (projectArg) { - ({ projectId: _projectId, project } = await resolveProject(config, projectArg, "stop")); - } else { - const projectIds = Object.keys(config.projects); - if (projectIds.length === 0) { - throw new Error("No projects configured. Add a project to agent-orchestrator.yaml."); + if (projectArg) { + console.log(chalk.bold(`\nStopping orchestrator for ${chalk.cyan(project.name)}\n`)); + } else { + console.log(chalk.bold(`\nStopping AO across all projects\n`)); } - const currentDir = resolve(cwd()); - const cwdProjectId = findProjectForDirectory(config.projects, currentDir); - _projectId = - running?.projects.find((id) => config.projects[id]) ?? - cwdProjectId ?? - projectIds[0]; - project = config.projects[_projectId]; - } - const port = config.port ?? DEFAULT_PORT; - if (projectArg) { - console.log(chalk.bold(`\nStopping orchestrator for ${chalk.cyan(project.name)}\n`)); - } else { - console.log(chalk.bold(`\nStopping AO across all projects\n`)); - } + const sm = await getSessionManager(config); + try { + // When no explicit project is given, list ALL sessions — ao stop + // kills the parent process which affects all projects. When a + // specific project is targeted, scope to that project only. + const stopAll = !projectArg; + const rawSessions = await sm.list(stopAll ? undefined : _projectId); + // Defensive consumer-side filter. `sm.list(projectId)` already scopes + // to the named project, but the kill loop hard-stops processes — a + // contract regression here would silently kill another project's + // work. When a project arg is given, drop anything that isn't ours. + const allSessions = stopAll + ? rawSessions + : rawSessions.filter((s) => s.projectId === _projectId); + const activeSessions = allSessions.filter((s) => !isTerminalSession(s)); + const killedSessionIds: string[] = []; + + // Separate sessions by project for display and recording + const targetActive = activeSessions.filter((s) => s.projectId === _projectId); + const otherActive = activeSessions.filter((s) => s.projectId !== _projectId); + // Group other-project sessions by projectId (used for display + recording) + const otherByProject = new Map(); + + if (activeSessions.length > 0) { + if (!projectArg && opts.yes !== true && isHumanCaller()) { + const confirmed = await promptConfirm( + `Stop AO and ${activeSessions.length} active session(s)?`, + false, + ); + if (!confirmed) { + console.log(chalk.yellow("Stop cancelled.")); + return; + } + } + const spinner = ora(`Stopping ${activeSessions.length} active session(s)`).start(); + const purgeOpenCode = opts?.purgeSession === true; + const warnings: string[] = []; + for (const session of activeSessions) { + try { + const result = await sm.kill(session.id, { purgeOpenCode }); + if (result.cleaned || result.alreadyTerminated) { + killedSessionIds.push(session.id); + } + } catch (err) { + recordActivityEvent({ + projectId: session.projectId ?? _projectId, + sessionId: session.id, + source: "cli", + kind: "cli.stop_session_failed", + level: "warn", + summary: `failed to kill session during ao stop`, + data: { errorMessage: err instanceof Error ? err.message : String(err) }, + }); + warnings.push( + ` Warning: failed to stop ${session.id}: ${err instanceof Error ? err.message : String(err)}`, + ); + } + } + if (killedSessionIds.length === 0) { + spinner.fail("Failed to stop any sessions"); + } else if (killedSessionIds.length < activeSessions.length) { + spinner.warn( + `Stopped ${killedSessionIds.length}/${activeSessions.length} session(s)`, + ); + } else { + spinner.succeed(`Stopped ${killedSessionIds.length} session(s)`); + } + for (const w of warnings) { + console.log(chalk.yellow(w)); + } + // Show stopped sessions grouped by project + const killedTarget = targetActive + .filter((s) => killedSessionIds.includes(s.id)) + .map((s) => s.id); + if (killedTarget.length > 0) { + console.log(chalk.green(` ${project.name}: ${killedTarget.join(", ")}`)); + } + for (const s of otherActive) { + if (!killedSessionIds.includes(s.id)) continue; + const list = otherByProject.get(s.projectId ?? "unknown") ?? []; + list.push(s.id); + otherByProject.set(s.projectId ?? "unknown", list); + } + for (const [pid, ids] of otherByProject) { + console.log(chalk.green(` ${pid}: ${ids.join(", ")}`)); + } + } else { + console.log(chalk.yellow(`No active sessions found`)); + } - const sm = await getSessionManager(config); - try { - // When no explicit project is given, list ALL sessions — ao stop - // kills the parent process which affects all projects. When a - // specific project is targeted, scope to that project only. - const stopAll = !projectArg; - const rawSessions = await sm.list(stopAll ? undefined : _projectId); - // Defensive consumer-side filter. `sm.list(projectId)` already scopes - // to the named project, but the kill loop hard-stops processes — a - // contract regression here would silently kill another project's - // work. When a project arg is given, drop anything that isn't ours. - const allSessions = stopAll - ? rawSessions - : rawSessions.filter((s) => s.projectId === _projectId); - const activeSessions = allSessions.filter((s) => !isTerminalSession(s)); - const killedSessionIds: string[] = []; - - // Separate sessions by project for display and recording - const targetActive = activeSessions.filter((s) => s.projectId === _projectId); - const otherActive = activeSessions.filter((s) => s.projectId !== _projectId); - // Group other-project sessions by projectId (used for display + recording) - const otherByProject = new Map(); - - if (activeSessions.length > 0) { - if (!projectArg && opts.yes !== true && isHumanCaller()) { - const confirmed = await promptConfirm( - `Stop AO and ${activeSessions.length} active session(s)?`, - false, + // Record stopped sessions for restore on next `ao start` + if (killedSessionIds.length > 0) { + const otherProjects: Array<{ projectId: string; sessionIds: string[] }> = []; + for (const [pid, ids] of otherByProject) { + otherProjects.push({ projectId: pid, sessionIds: ids }); + } + + const targetSessionIds = killedSessionIds.filter((id) => + targetActive.some((s) => s.id === id), ); - if (!confirmed) { - console.log(chalk.yellow("Stop cancelled.")); - return; + try { + await writeLastStop({ + stoppedAt: new Date().toISOString(), + projectId: _projectId, + sessionIds: targetSessionIds, + otherProjects: otherProjects.length > 0 ? otherProjects : undefined, + }); + recordActivityEvent({ + projectId: _projectId, + source: "cli", + kind: "cli.last_stop_written", + level: "info", + summary: `last-stop state written with ${killedSessionIds.length} session(s)`, + data: { + targetSessionCount: targetSessionIds.length, + otherProjectCount: otherProjects.length, + totalKilled: killedSessionIds.length, + }, + }); + } catch (err) { + recordActivityEvent({ + projectId: _projectId, + source: "cli", + kind: "cli.last_stop_write_failed", + level: "error", + summary: `failed to write last-stop state during ao stop`, + data: { + targetSessionCount: targetSessionIds.length, + otherProjectCount: otherProjects.length, + totalKilled: killedSessionIds.length, + errorMessage: err instanceof Error ? err.message : String(err), + }, + }); + console.log( + chalk.yellow( + ` Could not write last-stop state: ${err instanceof Error ? err.message : String(err)}`, + ), + ); } } - const spinner = ora(`Stopping ${activeSessions.length} active session(s)`).start(); - const purgeOpenCode = opts?.purgeSession === true; - const warnings: string[] = []; - for (const session of activeSessions) { + } catch (err) { + console.log( + chalk.yellow( + ` Could not list sessions: ${err instanceof Error ? err.message : String(err)}`, + ), + ); + } + + // Only kill the parent `ao start` process and dashboard when stopping + // everything (no project arg). When targeting a specific project, the + // parent process and dashboard serve all projects and must stay alive. + if (!projectArg) { + // Lifecycle polling runs in-process inside the `ao start` process + // (registered via `running.json`). Sending SIGTERM to that PID below + // triggers the shared shutdown handler in `lifecycle-service`, which + // stops every per-project loop. No explicit stop call needed here — + // this CLI invocation is a separate process with an empty active map. + if (running) { + // Sweep detached Windows pty-hosts BEFORE killing the parent. + // detached:true puts them outside the parent's process tree, so + // taskkill /T cannot reach them. The sweep speaks the named-pipe + // protocol so node-pty disposes ConPTY gracefully (avoids WER + // 0x800700e8). No-op on non-Windows. + await sweepWindowsPtyHostsBeforeParentKill(); + await sweepRegisteredDaemonChildren(running.pid); try { - const result = await sm.kill(session.id, { purgeOpenCode }); - if (result.cleaned || result.alreadyTerminated) { - killedSessionIds.push(session.id); - } + await killProcessTree(running.pid, "SIGTERM"); + recordActivityEvent({ + projectId: _projectId, + source: "cli", + kind: "cli.daemon_killed", + level: "info", + summary: `SIGTERM sent to parent ao start`, + data: { pid: running.pid, port: running.port }, + }); } catch (err) { recordActivityEvent({ - projectId: session.projectId ?? _projectId, - sessionId: session.id, + projectId: _projectId, source: "cli", - kind: "cli.stop_session_failed", + kind: "cli.daemon_killed", level: "warn", - summary: `failed to kill session during ao stop`, - data: { errorMessage: err instanceof Error ? err.message : String(err) }, + summary: `parent ao start was already dead`, + data: { + pid: running.pid, + errorMessage: err instanceof Error ? err.message : String(err), + }, }); - warnings.push( - ` Warning: failed to stop ${session.id}: ${err instanceof Error ? err.message : String(err)}`, - ); } - } - if (killedSessionIds.length === 0) { - spinner.fail("Failed to stop any sessions"); - } else if (killedSessionIds.length < activeSessions.length) { - spinner.warn( - `Stopped ${killedSessionIds.length}/${activeSessions.length} session(s)`, - ); + await unregister(); } else { - spinner.succeed(`Stopped ${killedSessionIds.length} session(s)`); - } - for (const w of warnings) { - console.log(chalk.yellow(w)); + await sweepRegisteredDaemonChildren(); } - // Show stopped sessions grouped by project - const killedTarget = targetActive - .filter((s) => killedSessionIds.includes(s.id)) - .map((s) => s.id); - if (killedTarget.length > 0) { - console.log(chalk.green(` ${project.name}: ${killedTarget.join(", ")}`)); - } - for (const s of otherActive) { - if (!killedSessionIds.includes(s.id)) continue; - const list = otherByProject.get(s.projectId ?? "unknown") ?? []; - list.push(s.id); - otherByProject.set(s.projectId ?? "unknown", list); - } - for (const [pid, ids] of otherByProject) { - console.log(chalk.green(` ${pid}: ${ids.join(", ")}`)); - } - } else { - console.log(chalk.yellow(`No active sessions found`)); + await stopDashboard(running?.port ?? port); } + // Targeted stop deliberately does NOT edit `running.json` from this + // child CLI process. The long-lived parent supervises lifecycle + // workers and will remove the project from `running.projects` after + // it observes that the last session became terminal. - // Record stopped sessions for restore on next `ao start` - if (killedSessionIds.length > 0) { - const otherProjects: Array<{ projectId: string; sessionIds: string[] }> = []; - for (const [pid, ids] of otherByProject) { - otherProjects.push({ projectId: pid, sessionIds: ids }); - } - - const targetSessionIds = killedSessionIds.filter((id) => - targetActive.some((s) => s.id === id), - ); - try { - await writeLastStop({ - stoppedAt: new Date().toISOString(), - projectId: _projectId, - sessionIds: targetSessionIds, - otherProjects: otherProjects.length > 0 ? otherProjects : undefined, - }); - recordActivityEvent({ - projectId: _projectId, - source: "cli", - kind: "cli.last_stop_written", - level: "info", - summary: `last-stop state written with ${killedSessionIds.length} session(s)`, - data: { - targetSessionCount: targetSessionIds.length, - otherProjectCount: otherProjects.length, - totalKilled: killedSessionIds.length, - }, - }); - } catch (err) { - recordActivityEvent({ - projectId: _projectId, - source: "cli", - kind: "cli.last_stop_write_failed", - level: "error", - summary: `failed to write last-stop state during ao stop`, - data: { - targetSessionCount: targetSessionIds.length, - otherProjectCount: otherProjects.length, - totalKilled: killedSessionIds.length, - errorMessage: err instanceof Error ? err.message : String(err), - }, - }); - console.log( - chalk.yellow( - ` Could not write last-stop state: ${err instanceof Error ? err.message : String(err)}`, - ), - ); - } + if (projectArg) { + console.log(chalk.bold.green(`\n✓ Stopped sessions for ${project.name}\n`)); + } else { + console.log(chalk.bold.green("\n✓ Orchestrator stopped\n")); + console.log(chalk.dim(` Uptime: since ${running?.startedAt ?? "unknown"}`)); + console.log(chalk.dim(` Projects: ${Object.keys(config.projects).join(", ")}\n`)); } } catch (err) { - console.log( - chalk.yellow( - ` Could not list sessions: ${err instanceof Error ? err.message : String(err)}`, - ), - ); - } - - // Only kill the parent `ao start` process and dashboard when stopping - // everything (no project arg). When targeting a specific project, the - // parent process and dashboard serve all projects and must stay alive. - if (!projectArg) { - // Lifecycle polling runs in-process inside the `ao start` process - // (registered via `running.json`). Sending SIGTERM to that PID below - // triggers the shared shutdown handler in `lifecycle-service`, which - // stops every per-project loop. No explicit stop call needed here — - // this CLI invocation is a separate process with an empty active map. - if (running) { - // Sweep detached Windows pty-hosts BEFORE killing the parent. - // detached:true puts them outside the parent's process tree, so - // taskkill /T cannot reach them. The sweep speaks the named-pipe - // protocol so node-pty disposes ConPTY gracefully (avoids WER - // 0x800700e8). No-op on non-Windows. - await sweepWindowsPtyHostsBeforeParentKill(); - await sweepRegisteredDaemonChildren(running.pid); - try { - await killProcessTree(running.pid, "SIGTERM"); - recordActivityEvent({ - projectId: _projectId, - source: "cli", - kind: "cli.daemon_killed", - level: "info", - summary: `SIGTERM sent to parent ao start`, - data: { pid: running.pid, port: running.port }, - }); - } catch (err) { - recordActivityEvent({ - projectId: _projectId, - source: "cli", - kind: "cli.daemon_killed", - level: "warn", - summary: `parent ao start was already dead`, - data: { - pid: running.pid, - errorMessage: err instanceof Error ? err.message : String(err), - }, - }); - } - await unregister(); + recordActivityEvent({ + source: "cli", + kind: "cli.stop_failed", + level: "error", + summary: `ao stop action failed`, + data: { + projectArg: projectArg ?? null, + errorMessage: err instanceof Error ? err.message : String(err), + }, + }); + if (err instanceof Error) { + console.error(chalk.red("\nError:"), err.message); } else { - await sweepRegisteredDaemonChildren(); + console.error(chalk.red("\nError:"), String(err)); } - await stopDashboard(running?.port ?? port); - } - // Targeted stop deliberately does NOT edit `running.json` from this - // child CLI process. The long-lived parent supervises lifecycle - // workers and will remove the project from `running.projects` after - // it observes that the last session became terminal. - - if (projectArg) { - console.log(chalk.bold.green(`\n✓ Stopped sessions for ${project.name}\n`)); - } else { - console.log(chalk.bold.green("\n✓ Orchestrator stopped\n")); - console.log(chalk.dim(` Uptime: since ${running?.startedAt ?? "unknown"}`)); - console.log(chalk.dim(` Projects: ${Object.keys(config.projects).join(", ")}\n`)); - } - } catch (err) { - recordActivityEvent({ - source: "cli", - kind: "cli.stop_failed", - level: "error", - summary: `ao stop action failed`, - data: { - projectArg: projectArg ?? null, - errorMessage: err instanceof Error ? err.message : String(err), - }, - }); - if (err instanceof Error) { - console.error(chalk.red("\nError:"), err.message); - } else { - console.error(chalk.red("\nError:"), String(err)); + process.exit(1); } - process.exit(1); - } - }); + }, + ); } diff --git a/packages/cli/src/lib/dashboard-setup.ts b/packages/cli/src/lib/dashboard-setup.ts index 49b8de230e..df9fed6fba 100644 --- a/packages/cli/src/lib/dashboard-setup.ts +++ b/packages/cli/src/lib/dashboard-setup.ts @@ -12,10 +12,9 @@ import { } from "@aoagents/ao-core"; import { applyNotifierRoutingPreset, + ensureNotifierDefault, getNotifierRoutingState, - promptNotifierRoutingPreset, resolveRoutingPresetOption, - type ClackPrompts, type NotifierRoutingPreset, } from "./notifier-routing.js"; @@ -40,6 +39,7 @@ interface DashboardConfig { interface ResolvedDashboardSetup { limit: number; + shouldWriteConfig: boolean; routingPreset?: NotifierRoutingPreset; } @@ -114,17 +114,32 @@ function toDashboardConfig(resolved: ResolvedDashboardSetup): DashboardConfig { }; } +function hasDashboardConfig(entry: Record): boolean { + return Object.keys(entry).length > 0; +} + +function shouldWriteDashboardConfig( + existingDashboard: Record, + limit: number, +): boolean { + return hasDashboardConfig(existingDashboard) || limit !== DEFAULT_DASHBOARD_NOTIFICATION_LIMIT; +} + function writeDashboardConfig(configPath: string, resolved: ResolvedDashboardSetup): void { const rawYaml = readFileSync(configPath, "utf-8"); const doc = parseDocument(rawYaml); const rawConfig = (doc.toJS() as Record) ?? {}; const notifiers = isRecord(rawConfig["notifiers"]) ? rawConfig["notifiers"] : {}; - notifiers["dashboard"] = toDashboardConfig(resolved); - rawConfig["notifiers"] = notifiers; + if (resolved.shouldWriteConfig) { + notifiers["dashboard"] = toDashboardConfig(resolved); + rawConfig["notifiers"] = notifiers; + } else if (isRecord(notifiers["dashboard"])) { + delete notifiers["dashboard"]; + rawConfig["notifiers"] = notifiers; + } - const defaults = isRecord(rawConfig["defaults"]) ? rawConfig["defaults"] : {}; - rawConfig["defaults"] = defaults; + ensureNotifierDefault(rawConfig, "dashboard"); applyNotifierRoutingPreset(rawConfig, "dashboard", resolved.routingPreset); @@ -135,19 +150,16 @@ function writeDashboardConfig(configPath: string, resolved: ResolvedDashboardSet } } - doc.setIn(["notifiers"], rawConfig["notifiers"]); doc.setIn(["defaults"], rawConfig["defaults"]); + if (rawConfig["notifiers"] !== undefined) { + doc.setIn(["notifiers"], rawConfig["notifiers"]); + } if (rawConfig["notificationRouting"] !== undefined) { doc.setIn(["notificationRouting"], rawConfig["notificationRouting"]); } writeFileSync(configPath, doc.toString({ indent: 2 })); } -function cancelSetup(clack: ClackPrompts): never { - clack.cancel("Dashboard setup cancelled."); - throw new DashboardSetupError("Dashboard setup cancelled.", 0); -} - async function shouldReplaceConflictingDashboard( plugin: unknown, force: boolean, @@ -168,7 +180,8 @@ async function shouldReplaceConflictingDashboard( }); if (clack.isCancel(answer)) { - cancelSetup(clack); + clack.cancel("Dashboard setup cancelled."); + throw new DashboardSetupError("Dashboard setup cancelled.", 0); } return answer === true; @@ -177,7 +190,6 @@ async function shouldReplaceConflictingDashboard( async function resolveInteractiveSetup( opts: DashboardSetupOptions, existingDashboard: Record, - rawConfig: Record, ): Promise { const clack = await import("@clack/prompts"); const optionRoutingPreset = resolveDashboardRoutingPreset(opts.routingPreset); @@ -200,19 +212,15 @@ async function resolveInteractiveSetup( }); if (clack.isCancel(limitInput)) { - cancelSetup(clack); + clack.cancel("Dashboard setup cancelled."); + throw new DashboardSetupError("Dashboard setup cancelled.", 0); } - const routingSelection = - optionRoutingPreset ?? - (await promptNotifierRoutingPreset(clack, rawConfig, "dashboard", "dashboard", () => - cancelSetup(clack), - )); - if (routingSelection === "back") continue; - + const limit = parseLimit(limitInput); return { - limit: parseLimit(limitInput), - routingPreset: routingSelection === "preserve" ? undefined : routingSelection, + limit, + shouldWriteConfig: shouldWriteDashboardConfig(existingDashboard, limit), + routingPreset: optionRoutingPreset, }; } } @@ -224,14 +232,16 @@ function resolveNonInteractiveSetup( const limit = opts.limit !== undefined ? parseLimit(opts.limit) - : opts.refresh + : opts.refresh || existingDashboard["limit"] !== undefined ? parseLimit(existingDashboard["limit"]) : DEFAULT_DASHBOARD_NOTIFICATION_LIMIT; - const routingPreset = - resolveDashboardRoutingPreset(opts.routingPreset) ?? - (opts.refresh ? undefined : "urgent-action"); + const routingPreset = resolveDashboardRoutingPreset(opts.routingPreset) ?? undefined; - return { limit, routingPreset }; + return { + limit, + shouldWriteConfig: shouldWriteDashboardConfig(existingDashboard, limit), + routingPreset, + }; } function printStatus(): void { @@ -273,7 +283,7 @@ export async function runDashboardSetupAction(opts: DashboardSetupOptions): Prom const resolved = nonInteractive ? resolveNonInteractiveSetup(opts, existingDashboard) - : await resolveInteractiveSetup(opts, existingDashboard, context.rawConfig); + : await resolveInteractiveSetup(opts, existingDashboard); writeDashboardConfig(context.configPath, resolved); console.log(chalk.green(`Config written to ${context.configPath}`)); diff --git a/packages/cli/src/lib/desktop-setup.ts b/packages/cli/src/lib/desktop-setup.ts index 7b2d2e9224..5a3154ff81 100644 --- a/packages/cli/src/lib/desktop-setup.ts +++ b/packages/cli/src/lib/desktop-setup.ts @@ -9,8 +9,8 @@ import { parseDocument } from "yaml"; import { CONFIG_SCHEMA_URL, findConfigFile, isCanonicalGlobalConfigPath } from "@aoagents/ao-core"; import { applyNotifierRoutingPreset, + ensureNotifierDefault, getNotifierRoutingState, - promptNotifierRoutingPreset, resolveRoutingPresetOption, type NotifierRoutingPreset, } from "./notifier-routing.js"; @@ -60,6 +60,7 @@ interface ResolvedDesktopSetup { dashboardUrl?: string; appPath: string; shouldWriteAppPath: boolean; + shouldWriteConfig: boolean; shouldSendTest: boolean; refresh: boolean; routingPreset?: NotifierRoutingPreset; @@ -211,7 +212,9 @@ function printStatus(): void { console.log(` config backend: ${configBackend ?? "not configured"}`); console.log(` dashboardUrl: ${configDashboardUrl ?? "not configured"}`); console.log(` config appPath: ${configAppPath ?? "not configured"}`); - console.log(` terminal-notifier: ${commandExists("terminal-notifier") ? "available" : "missing"}`); + console.log( + ` terminal-notifier: ${commandExists("terminal-notifier") ? "available" : "missing"}`, + ); console.log(` osascript: ${commandExists("osascript") ? "available" : "missing"}`); console.log(` installed: ${installed ? "yes" : "no"}`); console.log(` app: ${appPath}`); @@ -382,7 +385,10 @@ async function chooseDesktopBackend( options: [ { value: "ao-app", - label: existingBackend === "ao-app" ? "AO Notifier.app (current)" : "AO Notifier.app (recommended)", + label: + existingBackend === "ao-app" + ? "AO Notifier.app (current)" + : "AO Notifier.app (recommended)", hint: "Native macOS app with actions and AO-specific behavior", }, { @@ -392,7 +398,10 @@ async function chooseDesktopBackend( }, { value: "terminal-notifier", - label: existingBackend === "terminal-notifier" ? "terminal-notifier (current)" : "terminal-notifier", + label: + existingBackend === "terminal-notifier" + ? "terminal-notifier (current)" + : "terminal-notifier", hint: "Requires Homebrew package; supports click-to-open", }, { @@ -423,6 +432,22 @@ function resolveDashboardUrl( ); } +function hasDesktopConfig(entry: Record): boolean { + return Object.keys(entry).length > 0; +} + +function shouldWriteDesktopConfig( + opts: DesktopSetupOptions, + existingDesktop: Record, +): boolean { + return ( + hasDesktopConfig(existingDesktop) || + opts.backend !== undefined || + opts.dashboardUrl !== undefined || + opts.appPath !== undefined + ); +} + async function maybeInstallTerminalNotifier(nonInteractive: boolean): Promise { if (commandExists("terminal-notifier")) return; @@ -485,23 +510,6 @@ async function resolveDesktopSetup( stringValue(opts.appPath) ?? stringValue(context.existingDesktop["appPath"]) ?? getInstalledNotifierAppPath(); - const routingSelection = - optionRoutingPreset ?? - (nonInteractive || !context.configPath - ? opts.refresh - ? undefined - : "all" - : await promptNotifierRoutingPreset( - await import("@clack/prompts"), - context.rawConfig, - "desktop", - "desktop", - () => { - throw new DesktopSetupError("Setup cancelled.", 0); - }, - )); - - if (routingSelection === "back") continue; return { backend, @@ -510,9 +518,10 @@ async function resolveDesktopSetup( shouldWriteAppPath: Boolean(stringValue(opts.appPath)) || stringValue(context.existingDesktop["appPath"]) !== undefined, + shouldWriteConfig: shouldWriteDesktopConfig(opts, context.existingDesktop), shouldSendTest: opts.test !== false, refresh: Boolean(opts.refresh), - routingPreset: routingSelection === "preserve" ? undefined : routingSelection, + routingPreset: optionRoutingPreset, }; } } @@ -590,6 +599,21 @@ function sendBackendSetupNotification( } } +export async function installAoNotifierAppForStartup(): Promise { + await prepareDesktopBackend( + { + backend: "ao-app", + appPath: getInstalledNotifierAppPath(), + shouldWriteAppPath: false, + shouldWriteConfig: false, + shouldSendTest: false, + refresh: true, + }, + false, + true, + ); +} + async function wireDesktopConfig( configPath: string | undefined, force: boolean, @@ -610,28 +634,28 @@ async function wireDesktopConfig( if ( !conflictAlreadyChecked && - !(await shouldReplaceConflictingDesktop( - existingDesktop["plugin"], - force, - nonInteractive, - )) + !(await shouldReplaceConflictingDesktop(existingDesktop["plugin"], force, nonInteractive)) ) { return false; } - const desktopConfig: Record = { - ...existingDesktop, - plugin: "desktop", - backend: resolved.backend, - }; - if (resolved.dashboardUrl) desktopConfig["dashboardUrl"] = resolved.dashboardUrl; - if (resolved.shouldWriteAppPath) desktopConfig["appPath"] = resolved.appPath; + if (resolved.shouldWriteConfig) { + const desktopConfig: Record = { + ...existingDesktop, + plugin: "desktop", + backend: resolved.backend, + }; + if (resolved.dashboardUrl) desktopConfig["dashboardUrl"] = resolved.dashboardUrl; + if (resolved.shouldWriteAppPath) desktopConfig["appPath"] = resolved.appPath; - notifiers["desktop"] = desktopConfig; - rawConfig["notifiers"] = notifiers; + notifiers["desktop"] = desktopConfig; + rawConfig["notifiers"] = notifiers; + } else if (notifiers["desktop"] !== undefined) { + delete notifiers["desktop"]; + rawConfig["notifiers"] = notifiers; + } - const defaults = (rawConfig["defaults"] as Record | undefined) ?? {}; - rawConfig["defaults"] = defaults; + ensureNotifierDefault(rawConfig, "desktop"); applyNotifierRoutingPreset(rawConfig, "desktop", resolved.routingPreset); @@ -641,8 +665,10 @@ async function wireDesktopConfig( doc.set("$schema", CONFIG_SCHEMA_URL); } } - doc.setIn(["notifiers"], rawConfig["notifiers"]); doc.setIn(["defaults"], rawConfig["defaults"]); + if (rawConfig["notifiers"] !== undefined) { + doc.setIn(["notifiers"], rawConfig["notifiers"]); + } if (rawConfig["notificationRouting"] !== undefined) { doc.setIn(["notificationRouting"], rawConfig["notificationRouting"]); } diff --git a/packages/cli/src/lib/notifier-routing.ts b/packages/cli/src/lib/notifier-routing.ts index b2fc096668..21d143b65d 100644 --- a/packages/cli/src/lib/notifier-routing.ts +++ b/packages/cli/src/lib/notifier-routing.ts @@ -68,7 +68,9 @@ export function isNotifierRoutingPreset(value: string | undefined): value is Not return value === "urgent-only" || value === "urgent-action" || value === "all"; } -export function parseNotifierRoutingPreset(value: string | undefined): NotifierRoutingPreset | undefined { +export function parseNotifierRoutingPreset( + value: string | undefined, +): NotifierRoutingPreset | undefined { return isNotifierRoutingPreset(value) ? value : undefined; } @@ -119,12 +121,15 @@ export function getNotifierRoutingState( }; } -export function ensureNotifierDefault(rawConfig: Record, notifierName: string): void { +export function ensureNotifierDefault( + rawConfig: Record, + notifierName: string, +): void { const defaults = isRecord(rawConfig["defaults"]) ? rawConfig["defaults"] : {}; - defaults["notifiers"] = unique([ - ...asStringArray(defaults["notifiers"]).filter((value) => value !== notifierName), - notifierName, - ]); + const existing = asStringArray(defaults["notifiers"]); + defaults["notifiers"] = existing.includes(notifierName) + ? unique(existing) + : unique([...existing, notifierName]); rawConfig["defaults"] = defaults; } @@ -163,7 +168,9 @@ export function resolveRoutingPresetOption( if (value === undefined) return undefined; const parsed = parseNotifierRoutingPreset(value); if (parsed) return parsed; - throw new Error(`Invalid ${label} routing preset "${value}". Expected ${notifierRoutingPresetValues()}.`); + throw new Error( + `Invalid ${label} routing preset "${value}". Expected ${notifierRoutingPresetValues()}.`, + ); } export async function promptNotifierRoutingPreset( diff --git a/packages/cli/src/lib/notify-test.ts b/packages/cli/src/lib/notify-test.ts index 47b50f05a9..9247bcc052 100644 --- a/packages/cli/src/lib/notify-test.ts +++ b/packages/cli/src/lib/notify-test.ts @@ -2,11 +2,13 @@ import { createServer, type IncomingMessage, type Server, type ServerResponse } import type { AddressInfo } from "node:net"; import { buildCIFailureNotificationData, + buildNotificationPresentation, buildPRStateNotificationData, buildReactionNotificationData, buildSessionTransitionNotificationData, createProjectObserver, recordNotificationDelivery, + resolveNotificationRoute, resolveNotifierTarget, type CICheck, type EventPriority, @@ -14,6 +16,7 @@ import { type NotificationDataV3, type NotificationEventContext, type Notifier, + type NotificationPresentation, type NotifyAction, type OrchestratorConfig, type OrchestratorEvent, @@ -349,6 +352,7 @@ export interface NotifyTestResult { ok: boolean; dryRun: boolean; templateName: NotifyTestTemplateName; + notification: NotificationPresentation; event: OrchestratorEvent; actions: NotifyAction[]; targets: NotifyTestTarget[]; @@ -464,7 +468,7 @@ function refsFromAllKnownSources(config: OrchestratorConfig): string[] { } function refsFromRoute(config: OrchestratorConfig, priority: EventPriority): string[] { - return uniqueRefs(config.notificationRouting?.[priority] ?? config.defaults?.notifiers ?? []); + return uniqueRefs(resolveNotificationRoute(config, priority)); } export function createNotifyTestEvent(request: NotifyTestRequest = {}): { @@ -538,6 +542,7 @@ export async function runNotifyTest( request: NotifyTestRequest = {}, ): Promise { const { templateName, event } = createNotifyTestEvent(request); + const notification = buildNotificationPresentation(event); const targets = resolveNotifyTestTargets(config, event.priority, request); const actions = request.actions ? [...NOTIFY_TEST_ACTIONS] : []; const warnings: string[] = []; @@ -553,6 +558,7 @@ export async function runNotifyTest( ok: false, dryRun: Boolean(request.dryRun), templateName, + notification, event, actions, targets, @@ -669,6 +675,7 @@ export async function runNotifyTest( ok: errors.length === 0, dryRun: Boolean(request.dryRun), templateName, + notification, event, actions, targets, diff --git a/packages/core/src/__tests__/config-generator.test.ts b/packages/core/src/__tests__/config-generator.test.ts index 1e6ce22f74..cf8f9be959 100644 --- a/packages/core/src/__tests__/config-generator.test.ts +++ b/packages/core/src/__tests__/config-generator.test.ts @@ -278,7 +278,7 @@ describe("generateConfigFromUrl", () => { runtime: getDefaultRuntime(), agent: "claude-code", workspace: "worktree", - notifiers: [], + notifiers: ["dashboard", "desktop"], }); // Check project config diff --git a/packages/core/src/__tests__/dashboard-notifications.test.ts b/packages/core/src/__tests__/dashboard-notifications.test.ts index bea9476380..b942225160 100644 --- a/packages/core/src/__tests__/dashboard-notifications.test.ts +++ b/packages/core/src/__tests__/dashboard-notifications.test.ts @@ -67,6 +67,11 @@ describe("dashboard notifications", () => { ); expect(record.id).toBe("evt-1:2026-05-13T11:00:00.000Z"); + expect(record.notification).toMatchObject({ + category: "ci_failing", + title: "CI failing on PR #1", + body: "1 check failed: typecheck.", + }); expect(record.event.timestamp).toBe("2026-05-13T10:00:00.000Z"); expect(record.event.data).toMatchObject({ schemaVersion: 3, @@ -116,7 +121,7 @@ describe("dashboard notifications", () => { expect(readDashboardNotificationsFromFile(filePath)).toEqual([record]); }); - it("keeps legacy JSONL records readable without transforming their data", () => { + it("keeps legacy JSONL records readable while backfilling presentation", () => { const filePath = makeTempPath(); const legacyRecord = { id: "evt-legacy:2026-05-13T11:00:00.000Z", @@ -134,7 +139,19 @@ describe("dashboard notifications", () => { }; writeFileSync(filePath, `${JSON.stringify(legacyRecord)}\n`, "utf-8"); - expect(readDashboardNotificationsFromFile(filePath)).toEqual([legacyRecord]); + expect(readDashboardNotificationsFromFile(filePath)).toEqual([ + { + ...legacyRecord, + notification: { + version: 1, + category: "ci_failing", + priority: "warning", + title: "CI failing", + body: "CI is failing and needs attention.", + details: ["CI is failing"], + }, + }, + ]); }); it("clamps invalid limits", () => { diff --git a/packages/core/src/__tests__/global-config.test.ts b/packages/core/src/__tests__/global-config.test.ts index 2e7c516282..bb38b0e16d 100644 --- a/packages/core/src/__tests__/global-config.test.ts +++ b/packages/core/src/__tests__/global-config.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { parse as parseYaml } from "yaml"; import { + createDefaultGlobalConfig, generateExternalId, loadGlobalConfig, migrateToGlobalConfig, @@ -80,6 +81,15 @@ describe("global-config storage identity", () => { expect(config?.projects[projectId]).not.toHaveProperty("runtime"); }); + it("creates fresh global configs without Composio and with config-light notifier defaults", () => { + const config = createDefaultGlobalConfig(); + + expect(config.defaults.notifiers).toEqual(["dashboard", "desktop"]); + expect(config.notifiers).toEqual({}); + expect(config.notificationRouting).toEqual({}); + expect(JSON.stringify(config)).not.toContain("composio"); + }); + it("rejects registration when another project already owns the generated session prefix", () => { const repoPath = join(tempRoot, "apps", "web"); mkdirSync(join(repoPath, ".git"), { recursive: true }); @@ -379,7 +389,9 @@ describe("global-config storage identity", () => { ].join("\n"), ); - expect(resolveProjectIdentity(projectId, loadGlobalConfig(configPath)!, configPath)).toMatchObject({ + expect( + resolveProjectIdentity(projectId, loadGlobalConfig(configPath)!, configPath), + ).toMatchObject({ resolveError: expect.stringContaining("wrapped projects: format"), }); @@ -393,7 +405,9 @@ describe("global-config storage identity", () => { orchestrator: { agent: "codex" }, worker: { agent: "opencode" }, }); - expect(resolveProjectIdentity(projectId, loadGlobalConfig(configPath)!, configPath)).toMatchObject({ + expect( + resolveProjectIdentity(projectId, loadGlobalConfig(configPath)!, configPath), + ).toMatchObject({ agent: "codex", runtime: "tmux", workspace: "worktree", diff --git a/packages/core/src/__tests__/notification-presentation.test.ts b/packages/core/src/__tests__/notification-presentation.test.ts new file mode 100644 index 0000000000..a84dab09f5 --- /dev/null +++ b/packages/core/src/__tests__/notification-presentation.test.ts @@ -0,0 +1,239 @@ +import { describe, expect, it } from "vitest"; +import type { OrchestratorEvent } from "../types.js"; +import { + buildCIFailureNotificationData, + buildPRStateNotificationData, + buildReactionEscalationNotificationData, + buildReactionNotificationData, + buildSessionTransitionNotificationData, + type NotificationEventContext, +} from "../notification-data.js"; +import { buildNotificationPresentation } from "../notification-presentation.js"; + +const CONTEXT: NotificationEventContext = { + pr: { + number: 42, + url: "https://github.com/acme/app/pull/42", + title: "Add concise notifications", + branch: "feat/notifications", + baseBranch: "main", + }, + issueId: "AO-42", + issueTitle: "Add concise notifications", + summary: "Add concise notifications", + branch: "feat/notifications", +}; + +function event(overrides: Partial): OrchestratorEvent { + return { + id: "evt-1", + type: "session.needs_input", + priority: "action", + sessionId: "worker-1", + projectId: "demo", + timestamp: new Date("2026-05-13T12:00:00.000Z"), + message: "Context: noisy\nStatus: waiting\nBranch: feat/noisy\nTransition: working → waiting", + data: {}, + ...overrides, + }; +} + +function expectNotificationSafe(input: OrchestratorEvent): void { + const presentation = buildNotificationPresentation(input); + expect(`${presentation.title}\n${presentation.body}`).not.toMatch( + /Context:|Status:|Branch:|Transition:/, + ); +} + +describe("buildNotificationPresentation", () => { + it("formats agent needs-input notifications", () => { + const presentation = buildNotificationPresentation( + event({ + type: "session.needs_input", + data: buildSessionTransitionNotificationData({ + eventType: "session.needs_input", + sessionId: "worker-1", + projectId: "demo", + context: CONTEXT, + oldStatus: "working", + newStatus: "needs_input", + }), + }), + ); + + expect(presentation).toMatchObject({ + category: "agent_needs_input", + title: "Agent needs input", + body: "worker-1 is waiting for input.", + }); + }); + + it("formats agent stuck notifications", () => { + const presentation = buildNotificationPresentation( + event({ + type: "session.stuck", + priority: "urgent", + data: buildSessionTransitionNotificationData({ + eventType: "session.stuck", + sessionId: "worker-1", + projectId: "demo", + context: CONTEXT, + oldStatus: "working", + newStatus: "stuck", + }), + }), + ); + + expect(presentation.title).toBe("Agent may be stuck"); + expect(presentation.body).toBe("worker-1 has been inactive and needs attention."); + }); + + it("formats CI failing notifications with failed check names", () => { + const presentation = buildNotificationPresentation( + event({ + type: "ci.failing", + data: buildCIFailureNotificationData({ + sessionId: "worker-1", + projectId: "demo", + context: CONTEXT, + failedChecks: [ + { name: "typecheck", status: "failed" }, + { name: "unit-tests", status: "failed" }, + ], + }), + }), + ); + + expect(presentation.title).toBe("CI failing on PR #42"); + expect(presentation.body).toBe("2 checks failed: typecheck, unit-tests."); + }); + + it("formats review changes requested notifications with thread count", () => { + const data = buildSessionTransitionNotificationData({ + eventType: "review.changes_requested", + sessionId: "worker-1", + projectId: "demo", + context: CONTEXT, + oldStatus: "review_pending", + newStatus: "changes_requested", + }); + data.review = { ...(data.review ?? {}), unresolvedThreads: 3 }; + + const presentation = buildNotificationPresentation(event({ type: "review.changes_requested", data })); + + expect(presentation.title).toBe("Changes requested on PR #42"); + expect(presentation.body).toBe("3 review threads need attention."); + }); + + it("formats merge ready notifications", () => { + const presentation = buildNotificationPresentation( + event({ + type: "merge.ready", + data: buildReactionNotificationData({ + eventType: "reaction.triggered", + sessionId: "worker-1", + projectId: "demo", + context: CONTEXT, + reactionKey: "approved-and-green", + action: "notify", + }), + }), + ); + + expect(presentation.title).toBe("PR #42 is ready to merge"); + expect(presentation.body).toBe("Approved and CI is green."); + }); + + it("formats PR closed notifications", () => { + const presentation = buildNotificationPresentation( + event({ + type: "pr.closed", + priority: "warning", + data: buildPRStateNotificationData({ + eventType: "pr.closed", + sessionId: "worker-1", + projectId: "demo", + context: CONTEXT, + oldPRState: "open", + newPRState: "closed", + }), + }), + ); + + expect(presentation.title).toBe("PR #42 was closed"); + expect(presentation.body).toBe("The pull request was closed before merge."); + }); + + it("formats reaction escalations", () => { + const presentation = buildNotificationPresentation( + event({ + type: "reaction.escalated", + priority: "urgent", + data: buildReactionEscalationNotificationData({ + eventType: "reaction.escalated", + sessionId: "worker-1", + projectId: "demo", + context: CONTEXT, + reactionKey: "ci-failed", + action: "escalated", + attempts: 4, + cause: "max_attempts", + }), + }), + ); + + expect(presentation.title).toBe("Reaction escalated"); + expect(presentation.body).toBe("ci-failed escalated after 4 attempts."); + }); + + it("formats all-complete notifications", () => { + const presentation = buildNotificationPresentation( + event({ + type: "summary.all_complete", + priority: "info", + projectId: "demo", + data: buildReactionNotificationData({ + eventType: "reaction.triggered", + sessionId: "worker-1", + projectId: "demo", + context: { ...CONTEXT, pr: null }, + reactionKey: "all-complete", + action: "notify", + }), + }), + ); + + expect(presentation.title).toBe("All sessions complete"); + expect(presentation.body).toBe("demo has no active work requiring attention."); + }); + + it("uses a generic fallback for unknown event types", () => { + const presentation = buildNotificationPresentation( + event({ + type: "session.spawned", + data: {}, + message: "Session worker-1 spawned", + }), + ); + + expect(presentation.category).toBe("generic"); + expect(presentation.title).toBe("Session Spawned"); + expect(presentation.body).toBe("Session worker-1 spawned"); + }); + + it("keeps desktop-visible title and body free of report labels", () => { + for (const input of [ + event({ type: "session.needs_input" }), + event({ type: "session.stuck" }), + event({ type: "ci.failing" }), + event({ type: "review.changes_requested" }), + event({ type: "merge.ready" }), + event({ type: "pr.closed" }), + event({ type: "reaction.escalated" }), + event({ type: "summary.all_complete" }), + event({ type: "session.spawned" }), + ]) { + expectNotificationSafe(input); + } + }); +}); diff --git a/packages/core/src/__tests__/notifier-resolution.test.ts b/packages/core/src/__tests__/notifier-resolution.test.ts new file mode 100644 index 0000000000..a9045489c0 --- /dev/null +++ b/packages/core/src/__tests__/notifier-resolution.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from "vitest"; +import { resolveNotificationRoute } from "../notifier-resolution.js"; +import type { EventPriority, OrchestratorConfig } from "../types.js"; + +function makeConfig( + defaults: string[], + notificationRouting: Partial> = {}, +): Pick { + return { + defaults: { + runtime: "tmux", + agent: "claude-code", + workspace: "worktree", + notifiers: defaults, + }, + notificationRouting: notificationRouting as OrchestratorConfig["notificationRouting"], + }; +} + +describe("resolveNotificationRoute", () => { + it("applies config-light dashboard + desktop default routing", () => { + const config = makeConfig(["dashboard", "desktop"]); + + expect(resolveNotificationRoute(config, "urgent")).toEqual(["desktop", "dashboard"]); + expect(resolveNotificationRoute(config, "warning")).toEqual(["desktop", "dashboard"]); + expect(resolveNotificationRoute(config, "action")).toEqual(["dashboard"]); + expect(resolveNotificationRoute(config, "info")).toEqual(["dashboard"]); + }); + + it("treats explicit priority routes as authoritative", () => { + const config = makeConfig(["dashboard", "desktop"], { + action: ["desktop"], + warning: [], + }); + + expect(resolveNotificationRoute(config, "action")).toEqual(["desktop"]); + expect(resolveNotificationRoute(config, "warning")).toEqual([]); + }); + + it("routes custom default notifiers to all priorities when no explicit route exists", () => { + const config = makeConfig(["dashboard", "desktop", "alerts"]); + + expect(resolveNotificationRoute(config, "urgent")).toEqual(["desktop", "dashboard", "alerts"]); + expect(resolveNotificationRoute(config, "action")).toEqual(["dashboard", "alerts"]); + }); +}); diff --git a/packages/core/src/__tests__/plugin-registry.test.ts b/packages/core/src/__tests__/plugin-registry.test.ts index 880e96c185..05931e552c 100644 --- a/packages/core/src/__tests__/plugin-registry.test.ts +++ b/packages/core/src/__tests__/plugin-registry.test.ts @@ -352,39 +352,73 @@ describe("loadBuiltins", () => { }); }); - it("passes configPath for named notifier references without explicit config", async () => { + it("registers config-light dashboard and desktop defaults without explicit config blocks", async () => { const registry = createPluginRegistry(); const fakeDashboard = makePlugin("notifier", "dashboard"); + const fakeDesktop = makePlugin("notifier", "desktop"); const cfg = makeOrchestratorConfig({ configPath: "/test/config.yaml", defaults: { runtime: "tmux", agent: "codex", workspace: "worktree", - notifiers: ["dashboard"], - }, - notificationRouting: { - urgent: ["dashboard"], - action: [], - warning: [], - info: [], + notifiers: ["dashboard", "desktop"], }, notifiers: {}, + notificationRouting: {} as OrchestratorConfig["notificationRouting"], }); await registry.loadBuiltins(cfg, async (pkg: string) => { if (pkg === "@aoagents/ao-plugin-notifier-dashboard") return fakeDashboard; + if (pkg === "@aoagents/ao-plugin-notifier-desktop") return fakeDesktop; throw new Error(`Not found: ${pkg}`); }); expect(fakeDashboard.create).toHaveBeenCalledWith({ configPath: "/test/config.yaml", }); + expect(fakeDesktop.create).toHaveBeenCalledWith({ + configPath: "/test/config.yaml", + }); expect( registry.get<{ _config: Record }>("notifier", "dashboard")?._config, ).toEqual({ configPath: "/test/config.yaml", }); + expect( + registry.get<{ _config: Record }>("notifier", "desktop")?._config, + ).toEqual({ + configPath: "/test/config.yaml", + }); + }); + + it("does not implicitly register manual opt-in built-in notifiers from routing alone", async () => { + const registry = createPluginRegistry(); + const fakeComposio = makePlugin("notifier", "composio"); + const cfg = makeOrchestratorConfig({ + configPath: "/test/config.yaml", + defaults: { + runtime: "tmux", + agent: "codex", + workspace: "worktree", + notifiers: ["composio"], + }, + notificationRouting: { + urgent: ["composio"], + action: [], + warning: [], + info: [], + }, + notifiers: {}, + }); + + await registry.loadBuiltins(cfg, async (pkg: string) => { + if (pkg === "@aoagents/ao-plugin-notifier-composio") return fakeComposio; + throw new Error(`Not found: ${pkg}`); + }); + + expect(fakeComposio.create).not.toHaveBeenCalled(); + expect(registry.get("notifier", "composio")).toBeNull(); }); it("does not create an implicit notifier registration over a conflicting explicit entry", async () => { diff --git a/packages/core/src/config-generator.ts b/packages/core/src/config-generator.ts index d4c5def0d6..a5dd927f9b 100644 --- a/packages/core/src/config-generator.ts +++ b/packages/core/src/config-generator.ts @@ -287,7 +287,7 @@ export function generateConfigFromUrl(options: GenerateConfigOptions): Record getDefaultRuntime()), agent: z.string().default("claude-code"), workspace: z.string().default("worktree"), - notifiers: z.array(z.string()).default([]), + notifiers: z.array(z.string()).default(["dashboard", "desktop"]), orchestrator: RoleAgentDefaultsSchema, worker: RoleAgentDefaultsSchema, }); diff --git a/packages/core/src/dashboard-notifications.ts b/packages/core/src/dashboard-notifications.ts index bf6a9f4e4a..f6557b1226 100644 --- a/packages/core/src/dashboard-notifications.ts +++ b/packages/core/src/dashboard-notifications.ts @@ -1,9 +1,14 @@ import { existsSync, mkdirSync, readFileSync } from "node:fs"; import { dirname, join } from "node:path"; -import type { NotifyAction, OrchestratorEvent } from "./types.js"; +import type { EventPriority, EventType, NotifyAction, OrchestratorEvent } from "./types.js"; import type { NotificationDataV3 } from "./notification-data.js"; import { atomicWriteFileSync } from "./atomic-write.js"; import { getObservabilityBaseDir } from "./paths.js"; +import { + buildNotificationPresentation, + isNotificationPresentation, + type NotificationPresentation, +} from "./notification-presentation.js"; export const DEFAULT_DASHBOARD_NOTIFICATION_LIMIT = 50; export const MAX_DASHBOARD_NOTIFICATION_LIMIT = 500; @@ -34,6 +39,7 @@ export interface SerializedDashboardAction { export interface DashboardNotificationRecord { id: string; receivedAt: string; + notification: NotificationPresentation; event: SerializedDashboardEvent; actions?: SerializedDashboardAction[]; } @@ -92,6 +98,7 @@ export function createDashboardNotificationRecord( return { id: `${event.id}:${receivedAtIso}`, receivedAt: receivedAtIso, + notification: buildNotificationPresentation(event), event: { id: event.id, type: event.type, @@ -106,6 +113,35 @@ export function createDashboardNotificationRecord( }; } +function toOrchestratorEvent(event: SerializedDashboardEvent): OrchestratorEvent { + const timestamp = new Date(event.timestamp); + return { + id: event.id, + type: event.type as EventType, + priority: event.priority as EventPriority, + sessionId: event.sessionId, + projectId: event.projectId, + timestamp: Number.isNaN(timestamp.getTime()) ? new Date(0) : timestamp, + message: event.message, + data: event.data, + }; +} + +function withNotificationPresentation( + record: Omit & { + notification?: unknown; + }, +): DashboardNotificationRecord { + if (isNotificationPresentation(record.notification)) { + return record as DashboardNotificationRecord; + } + + return { + ...record, + notification: buildNotificationPresentation(toOrchestratorEvent(record.event)), + }; +} + function isDashboardNotificationRecord(value: unknown): value is DashboardNotificationRecord { if (!value || typeof value !== "object") return false; const candidate = value as Partial; @@ -143,7 +179,7 @@ export function readDashboardNotificationsFromFile( try { const parsed = JSON.parse(trimmed) as unknown; if (isDashboardNotificationRecord(parsed)) { - records.push(parsed); + records.push(withNotificationPresentation(parsed)); } } catch { // Ignore malformed lines. A partial write should not break the dashboard. diff --git a/packages/core/src/global-config.ts b/packages/core/src/global-config.ts index f5840cc39c..5cca7530a7 100644 --- a/packages/core/src/global-config.ts +++ b/packages/core/src/global-config.ts @@ -219,7 +219,7 @@ export const GlobalConfigSchema = z runtime: z.string().default(() => getDefaultRuntime()), agent: z.string().default("claude-code"), workspace: z.string().default("worktree"), - notifiers: z.array(z.string()).default(["composio", "desktop"]), + notifiers: z.array(z.string()).default(["dashboard", "desktop"]), orchestrator: z.object({ agent: z.string().optional() }).optional(), worker: z.object({ agent: z.string().optional() }).optional(), }) @@ -231,12 +231,7 @@ export const GlobalConfigSchema = z /** Notification channel configurations. */ notifiers: z.record(z.object({ plugin: z.string() }).passthrough()).default({}), /** Maps priority levels to notifier channel IDs. */ - notificationRouting: z.record(z.array(z.string())).default({ - urgent: ["desktop", "composio"], - action: ["desktop", "composio"], - warning: ["composio"], - info: ["composio"], - }), + notificationRouting: z.record(z.array(z.string())).default({}), /** Reaction rules (default reactions merged at load time). */ reactions: z.record(z.object({}).passthrough()).default({}), }) @@ -480,7 +475,9 @@ export function writeLocalProjectConfig( } function isRecord(value: unknown): value is Record { - return value !== null && value !== undefined && typeof value === "object" && !Array.isArray(value); + return ( + value !== null && value !== undefined && typeof value === "object" && !Array.isArray(value) + ); } function mergeRoleBehavior( @@ -806,9 +803,10 @@ export function registerProjectInGlobalConfig( } } - const repoIdentity = existing?.repo - ?? normalizeRepoIdentity(originUrl) - ?? (localConfig?.repo ? normalizeLegacyRepoValue(localConfig.repo) : undefined); + const repoIdentity = + existing?.repo ?? + normalizeRepoIdentity(originUrl) ?? + (localConfig?.repo ? normalizeLegacyRepoValue(localConfig.repo) : undefined); const defaultBranch = existing?.defaultBranch ?? localConfig?.defaultBranch ?? "main"; const requestedSessionPrefix = existing?.sessionPrefix ?? @@ -1185,16 +1183,11 @@ export function createDefaultGlobalConfig(): GlobalConfig { runtime: getDefaultRuntime(), agent: "claude-code", workspace: "worktree", - notifiers: ["composio", "desktop"], + notifiers: ["dashboard", "desktop"], }, projects: {}, notifiers: {}, - notificationRouting: { - urgent: ["desktop", "composio"], - action: ["desktop", "composio"], - warning: ["composio"], - info: ["composio"], - }, + notificationRouting: {}, reactions: {}, }; } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 4ca21c112c..dd7610b706 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -265,7 +265,7 @@ export { readObservabilitySummary, } from "./observability.js"; export { execGhObserved, getGhTraceFilePath } from "./gh-trace.js"; -export { resolveNotifierTarget } from "./notifier-resolution.js"; +export { resolveNotificationRoute, resolveNotifierTarget } from "./notifier-resolution.js"; export { recordNotificationDelivery, sanitizeNotificationDeliveryReason, @@ -399,6 +399,12 @@ export { type SerializedDashboardAction, type SerializedDashboardEvent, } from "./dashboard-notifications.js"; +export { + buildNotificationPresentation, + isNotificationPresentation, + type NotificationPresentation, + type NotificationPresentationCategory, +} from "./notification-presentation.js"; // Global config — Option C hybrid architecture (global registry + local behavior) export { diff --git a/packages/core/src/lifecycle-manager.ts b/packages/core/src/lifecycle-manager.ts index 3cae2f77e6..4bfaf50f5e 100644 --- a/packages/core/src/lifecycle-manager.ts +++ b/packages/core/src/lifecycle-manager.ts @@ -68,7 +68,7 @@ import { REPORT_WATCHER_METADATA_KEYS, } from "./report-watcher.js"; import { createCorrelationId, createProjectObserver } from "./observability.js"; -import { resolveNotifierTarget } from "./notifier-resolution.js"; +import { resolveNotificationRoute, resolveNotifierTarget } from "./notifier-resolution.js"; import { recordNotificationDelivery } from "./notification-observability.js"; import { resolveSessionRole } from "./agent-selection.js"; import { @@ -2261,7 +2261,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan /** Send a notification to all configured notifiers. */ async function notifyHuman(event: OrchestratorEvent, priority: EventPriority): Promise { const eventWithPriority = { ...event, priority }; - const notifierNames = config.notificationRouting[priority] ?? config.defaults.notifiers; + const notifierNames = resolveNotificationRoute(config, priority); for (const name of notifierNames) { const target = resolveNotifierTarget(config, name); @@ -2650,8 +2650,8 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan } // For transitions not already notified by a reaction, notify humans. - // All priorities (including "info") are routed through notificationRouting - // so the config controls which notifiers receive each priority level. + // Explicit notificationRouting entries override the implicit defaults + // derived from defaults.notifiers. if (!reactionHandledNotify) { const priority = inferPriority(eventType); const context = buildEventContext(session, prEnrichmentCache); diff --git a/packages/core/src/notification-presentation.ts b/packages/core/src/notification-presentation.ts new file mode 100644 index 0000000000..1bfbf9dcc9 --- /dev/null +++ b/packages/core/src/notification-presentation.ts @@ -0,0 +1,232 @@ +import type { EventPriority, OrchestratorEvent } from "./types.js"; +import { getNotificationDataV3, type NotificationDataV3 } from "./notification-data.js"; + +export type NotificationPresentationCategory = + | "agent_needs_input" + | "agent_stuck" + | "agent_exited" + | "ci_failing" + | "review_changes_requested" + | "merge_ready" + | "merge_conflicts" + | "pr_closed" + | "reaction_escalated" + | "all_complete" + | "generic"; + +export interface NotificationPresentation { + version: 1; + category: NotificationPresentationCategory; + priority: EventPriority; + title: string; + body: string; + details?: string[]; +} + +const REPORT_LABEL_PATTERN = /\b(Context|Status|Branch|Transition):/gi; +const MAX_TITLE_LENGTH = 80; +const MAX_BODY_LENGTH = 180; +const MAX_DETAIL_LENGTH = 160; + +function truncate(value: string, maxLength: number): string { + return value.length > maxLength ? `${value.slice(0, maxLength - 1)}…` : value; +} + +function cleanText(value: string | undefined, maxLength: number): string | undefined { + const cleaned = value + ?.split(/\r?\n/) + .map((line) => line.replace(REPORT_LABEL_PATTERN, "").trim()) + .filter(Boolean) + .join(" ") + .replace(/\s+/g, " ") + .trim(); + return cleaned ? truncate(cleaned, maxLength) : undefined; +} + +function titleCaseEventType(value: string): string { + return value + .split(/[._\s-]+/) + .filter(Boolean) + .map((part) => `${part.slice(0, 1).toUpperCase()}${part.slice(1)}`) + .join(" "); +} + +function prLabel(data: NotificationDataV3 | null): string | undefined { + const number = data?.subject.pr?.number; + return typeof number === "number" ? `PR #${number}` : undefined; +} + +function checkList(checks: Array<{ name: string }> | undefined): string { + if (!checks || checks.length === 0) return ""; + const names = checks.map((check) => check.name).filter(Boolean); + const shown = names.slice(0, 3).join(", "); + const remaining = names.length - 3; + return remaining > 0 ? `${shown}, +${remaining} more` : shown; +} + +function pluralize(count: number, singular: string, plural: string): string { + return count === 1 ? singular : plural; +} + +function semanticType(event: OrchestratorEvent, data: NotificationDataV3 | null): string { + return data?.semanticType ?? event.type; +} + +function categoryFor(event: OrchestratorEvent, data: NotificationDataV3 | null): NotificationPresentationCategory { + if (event.type === "reaction.escalated") return "reaction_escalated"; + + switch (semanticType(event, data)) { + case "session.needs_input": + return "agent_needs_input"; + case "session.stuck": + return "agent_stuck"; + case "session.killed": + case "session.exited": + case "session.errored": + return "agent_exited"; + case "ci.failing": + return "ci_failing"; + case "review.changes_requested": + return "review_changes_requested"; + case "merge.ready": + return "merge_ready"; + case "merge.conflicts": + return "merge_conflicts"; + case "pr.closed": + return "pr_closed"; + case "summary.all_complete": + return "all_complete"; + default: + return "generic"; + } +} + +function presentationCopy( + category: NotificationPresentationCategory, + event: OrchestratorEvent, + data: NotificationDataV3 | null, +): { title: string; body: string } { + const pr = prLabel(data); + + switch (category) { + case "agent_needs_input": + return { + title: "Agent needs input", + body: `${event.sessionId} is waiting for input.`, + }; + case "agent_stuck": + return { + title: "Agent may be stuck", + body: `${event.sessionId} has been inactive and needs attention.`, + }; + case "agent_exited": + return { + title: "Agent exited", + body: `${event.sessionId} exited before completing its task.`, + }; + case "ci_failing": { + const failedChecks = data?.ci?.failedChecks ?? []; + const count = failedChecks.length; + const names = checkList(failedChecks); + return { + title: pr ? `CI failing on ${pr}` : "CI failing", + body: + count > 0 + ? `${count} ${pluralize(count, "check", "checks")} failed${names ? `: ${names}` : ""}.` + : "CI is failing and needs attention.", + }; + } + case "review_changes_requested": { + const threads = data?.review?.unresolvedThreads; + return { + title: pr ? `Changes requested on ${pr}` : "Changes requested", + body: + typeof threads === "number" && threads > 0 + ? `${threads} review ${pluralize(threads, "thread", "threads")} need attention.` + : "Review feedback needs attention.", + }; + } + case "merge_ready": + return { + title: pr ? `${pr} is ready to merge` : "Pull request is ready to merge", + body: "Approved and CI is green.", + }; + case "merge_conflicts": + return { + title: pr ? `Merge conflicts on ${pr}` : "Merge conflicts detected", + body: "Resolve conflicts before merge.", + }; + case "pr_closed": + return { + title: pr ? `${pr} was closed` : "Pull request was closed", + body: "The pull request was closed before merge.", + }; + case "reaction_escalated": { + const reactionKey = data?.reaction?.key; + const attempts = data?.escalation?.attempts; + return { + title: "Reaction escalated", + body: + reactionKey && typeof attempts === "number" + ? `${reactionKey} escalated after ${attempts} ${pluralize(attempts, "attempt", "attempts")}.` + : "A reaction needs human attention.", + }; + } + case "all_complete": + return { + title: "All sessions complete", + body: `${event.projectId} has no active work requiring attention.`, + }; + case "generic": + return { + title: titleCaseEventType(event.type), + body: cleanText(event.message, MAX_BODY_LENGTH) ?? `${event.sessionId} needs attention.`, + }; + } +} + +function presentationDetails(event: OrchestratorEvent, data: NotificationDataV3 | null): string[] | undefined { + const details = [ + data?.subject.pr?.title, + data?.subject.issue?.id, + data?.subject.branch, + event.message, + ] + .map((value) => cleanText(value, MAX_DETAIL_LENGTH)) + .filter((value): value is string => Boolean(value)); + + return details.length > 0 ? [...new Set(details)] : undefined; +} + +export function buildNotificationPresentation(event: OrchestratorEvent): NotificationPresentation { + const data = getNotificationDataV3(event.data); + const category = categoryFor(event, data); + const copy = presentationCopy(category, event, data); + const title = cleanText(copy.title, MAX_TITLE_LENGTH) ?? "AO notification"; + const body = cleanText(copy.body, MAX_BODY_LENGTH) ?? `${event.sessionId} needs attention.`; + const details = presentationDetails(event, data); + + return { + version: 1, + category, + priority: event.priority, + title, + body, + ...(details ? { details } : {}), + }; +} + +export function isNotificationPresentation(value: unknown): value is NotificationPresentation { + if (!value || typeof value !== "object" || Array.isArray(value)) return false; + const candidate = value as Partial; + return ( + candidate.version === 1 && + typeof candidate.category === "string" && + typeof candidate.priority === "string" && + typeof candidate.title === "string" && + typeof candidate.body === "string" && + (candidate.details === undefined || + (Array.isArray(candidate.details) && + candidate.details.every((detail) => typeof detail === "string"))) + ); +} diff --git a/packages/core/src/notifier-resolution.ts b/packages/core/src/notifier-resolution.ts index 5166a6c0ca..61984bcfcc 100644 --- a/packages/core/src/notifier-resolution.ts +++ b/packages/core/src/notifier-resolution.ts @@ -1,10 +1,59 @@ -import type { OrchestratorConfig } from "./types.js"; +import type { EventPriority, OrchestratorConfig } from "./types.js"; export interface ResolvedNotifierTarget { reference: string; pluginName: string; } +const DESKTOP_IMPLICIT_PRIORITIES = new Set(["urgent", "warning"]); + +function unique(values: string[]): string[] { + return [...new Set(values)]; +} + +function hasOwn(record: Record, key: string): boolean { + return Object.prototype.hasOwnProperty.call(record, key); +} + +function asStringArray(value: unknown): string[] { + return Array.isArray(value) + ? value.filter((entry): entry is string => typeof entry === "string") + : []; +} + +/** + * Resolve notifier enablement for a notification priority. + * + * Explicit `notificationRouting.` entries are authoritative. When a + * priority has no explicit route, AO applies its built-in default policy over + * `defaults.notifiers`: dashboard receives all priorities, desktop receives + * urgent + warning, and custom default notifiers keep the historical + * all-priorities fallback behavior. + */ +export function resolveNotificationRoute( + config: Pick, + priority: EventPriority, +): string[] { + const routing = (config.notificationRouting ?? {}) as Record; + if (hasOwn(routing, priority)) { + return unique(asStringArray(routing[priority])); + } + + const defaultNotifiers = unique(asStringArray(config.defaults?.notifiers)); + const resolved: string[] = []; + + if (DESKTOP_IMPLICIT_PRIORITIES.has(priority) && defaultNotifiers.includes("desktop")) { + resolved.push("desktop"); + } + + for (const notifierName of defaultNotifiers) { + if (notifierName === "desktop") continue; + resolved.push(notifierName); + } + + return unique(resolved); +} + /** * Resolve a notifier reference from config. * diff --git a/packages/core/src/plugin-registry.ts b/packages/core/src/plugin-registry.ts index 304d8a893c..c4f727fa2e 100644 --- a/packages/core/src/plugin-registry.ts +++ b/packages/core/src/plugin-registry.ts @@ -94,6 +94,10 @@ function hasExplicitConflictingNotifierEntry( ); } +function canImplicitlyRegisterNotifier(pluginName: string, isExternalLoad: boolean): boolean { + return isExternalLoad || pluginName === "dashboard" || pluginName === "desktop"; +} + function collectNotifierRegistrations( pluginName: string, config: OrchestratorConfig, @@ -129,6 +133,7 @@ function collectNotifierRegistrations( if ( orderedMatches.size === 0 && isReferencedByName && + canImplicitlyRegisterNotifier(pluginName, isExternalLoad) && !hasExplicitConflictingNotifierEntry(pluginName, config) ) { orderedMatches.set(pluginName, {}); @@ -439,7 +444,9 @@ export function createPluginRegistry(): PluginRegistry { if (registrations.length === 0) { if (hasExplicitConflictingNotifierEntry(manifest.name, config)) return; - registerInstance(manifest.slot, manifest.name, manifest, plugin.create(undefined)); + if (isExternalLoad) { + registerInstance(manifest.slot, manifest.name, manifest, plugin.create(undefined)); + } return; } diff --git a/packages/integration-tests/src/notifier-desktop.integration.test.ts b/packages/integration-tests/src/notifier-desktop.integration.test.ts index 52fe1b075a..1ad58d6281 100644 --- a/packages/integration-tests/src/notifier-desktop.integration.test.ts +++ b/packages/integration-tests/src/notifier-desktop.integration.test.ts @@ -11,7 +11,9 @@ import { makeEvent } from "./helpers/event-factory.js"; vi.mock("node:child_process", () => ({ execFile: vi.fn(), execFileSync: vi.fn(() => { - throw new Error("not found"); + const error = new Error("not found") as NodeJS.ErrnoException; + error.code = "ENOENT"; + throw error; }), })); @@ -55,7 +57,7 @@ describe("notifier-desktop integration", () => { const script = mockExecFile.mock.calls[0][1][1] as string; expect(script).not.toContain("sound name"); - expect(script).toContain("URGENT"); + expect(script).toContain("Session Spawned"); }); it("default config + urgent event -> sound clause present", async () => { @@ -102,13 +104,13 @@ describe("notifier-desktop integration", () => { expect(script).toContain("with title"); }); - it("escapes newlines in message text (renders literally in notification)", async () => { + it("normalizes newlines in message text for notification-safe copy", async () => { const notifier = desktopPlugin.create(); await notifier.notify(makeEvent({ message: "line1\nline2" })); const script = mockExecFile.mock.calls[0][1][1] as string; - // newlines in AppleScript strings are ok, they render as literal newlines - expect(script).toContain("line1\nline2"); + expect(script).toContain("line1 line2"); + expect(script).not.toContain("line1\nline2"); }); }); @@ -182,7 +184,7 @@ describe("notifier-desktop integration", () => { }); describe("notifyWithActions full pipeline", () => { - it("formats action labels into notification body text", async () => { + it("keeps action labels out of concise notification body text", async () => { const notifier = desktopPlugin.create(); const actions: NotifyAction[] = [ { label: "Merge PR", url: "https://github.com/pr/1" }, @@ -192,16 +194,17 @@ describe("notifier-desktop integration", () => { await notifier.notifyWithActions!(makeEvent({ priority: "urgent" }), actions); const script = mockExecFile.mock.calls[0][1][1] as string; - expect(script).toContain("Merge PR"); - expect(script).toContain("Kill"); + expect(script).toContain("Session app-1 spawned successfully"); + expect(script).not.toContain("Merge PR"); + expect(script).not.toContain("Kill"); expect(script).toContain('sound name "default"'); }); - it("action labels with special chars are escaped in AppleScript", async () => { + it("message text with special chars is escaped in AppleScript", async () => { const notifier = desktopPlugin.create(); const actions: NotifyAction[] = [{ label: 'Fix "bug"', url: "https://example.com" }]; - await notifier.notifyWithActions!(makeEvent(), actions); + await notifier.notifyWithActions!(makeEvent({ message: 'Fix "bug"' }), actions); const script = mockExecFile.mock.calls[0][1][1] as string; expect(script).toContain('Fix \\"bug\\"'); diff --git a/packages/plugins/notifier-desktop/src/index.test.ts b/packages/plugins/notifier-desktop/src/index.test.ts index 1f7755c413..5d51e15c03 100644 --- a/packages/plugins/notifier-desktop/src/index.test.ts +++ b/packages/plugins/notifier-desktop/src/index.test.ts @@ -140,12 +140,13 @@ describe("notifier-desktop", () => { expect(script).toContain("CI is failing"); }); - it("uses URGENT prefix for urgent priority", async () => { + it("does not encode priority labels into concise title text", async () => { const notifier = create(); await notifier.notify(makeEvent({ priority: "urgent" })); const script = mockExecFile.mock.calls[0][1][1] as string; - expect(script).toContain("URGENT"); + expect(script).toContain("Session Spawned"); + expect(script).not.toContain("URGENT"); }); it("uses event-aware titles for non-urgent priority", async () => { @@ -209,7 +210,7 @@ describe("notifier-desktop", () => { expect(script).toContain("\\\\backslash"); }); - it("formats v3 pull request context into a compact desktop summary", async () => { + it("formats v3 pull request context into concise presentation copy", async () => { const notifier = create(); await notifier.notify( makeEvent({ @@ -240,17 +241,14 @@ describe("notifier-desktop", () => { ); const script = mockExecFile.mock.calls[0][1][1] as string; - expect(script).toContain("PR #1579 ready to merge"); - expect(script).toContain("Normalize AO notifier payloads"); + expect(script).toContain("PR #1579 is ready to merge"); + expect(script).toContain("Approved and CI is green."); expect(script).toContain("demo · demo-agent-29 · PR #1579"); - expect(script).toContain("PR #1579"); - expect(script).toContain("AO-1579"); - expect(script).toContain("Branch: ao/demo-notifier-harness → main"); - expect(script).toContain("CI: Passing"); - expect(script).toContain("Review: Approved"); - expect(script).toContain("Merge: Ready"); - expect(script).toContain("Conflicts: None"); - expect(script).toContain("Transition: approved → mergeable"); + expect(script).not.toContain("Normalize AO notifier payloads"); + expect(script).not.toContain("Context:"); + expect(script).not.toContain("Status:"); + expect(script).not.toContain("Branch:"); + expect(script).not.toContain("Transition:"); }); }); @@ -273,7 +271,7 @@ describe("notifier-desktop", () => { expect(args).toContain("--urgency=critical"); // Options must come before title/message for notify-send const urgencyIdx = args.indexOf("--urgency=critical"); - const titleIdx = args.findIndex((a: string) => a.includes("URGENT")); + const titleIdx = args.findIndex((a: string) => a.includes("Session Spawned")); expect(urgencyIdx).toBeLessThan(titleIdx); }); @@ -358,7 +356,7 @@ describe("notifier-desktop", () => { }); describe("notifyWithActions", () => { - it("includes action labels in the message", async () => { + it("keeps visible action notifications focused on presentation copy", async () => { const notifier = create(); const actions: NotifyAction[] = [ { label: "Merge", url: "https://github.com/pr/1" }, @@ -367,8 +365,10 @@ describe("notifier-desktop", () => { await notifier.notifyWithActions!(makeEvent(), actions); const script = mockExecFile.mock.calls[0][1][1] as string; - expect(script).toContain("Merge"); - expect(script).toContain("Kill"); + expect(script).toContain("Session app-1 spawned"); + expect(script).not.toContain("Actions:"); + expect(script).not.toContain("Merge"); + expect(script).not.toContain("Kill"); }); it("includes sound for urgent with actions", async () => { @@ -415,7 +415,7 @@ describe("notifier-desktop", () => { expect(args).toContain("-title"); expect(args).toContain("-subtitle"); expect(args).toContain("-message"); - expect(args[args.indexOf("-subtitle") + 1]).toBe("my-project · s-1 · Info"); + expect(args[args.indexOf("-subtitle") + 1]).toBe("my-project · s-1"); expect(args[args.indexOf("-message") + 1]).toContain("hello"); }); @@ -483,6 +483,19 @@ describe("notifier-desktop", () => { expect(mockExecFile.mock.calls[0][0]).toBe("notify-send"); }); + it("uses concise body on non-macOS even when backend is ao-app", async () => { + mockPlatform.mockReturnValue("linux"); + setProcessPlatform("linux"); + const notifier = create({ backend: "ao-app", dashboardUrl: "http://localhost:3000" }); + const actions: NotifyAction[] = [{ label: "Open PR", url: "https://example.com/pr/1" }]; + await notifier.notifyWithActions!(makeEvent(), actions); + + expect(mockExecFile.mock.calls[0][0]).toBe("notify-send"); + const args = mockExecFile.mock.calls[0][1] as string[]; + expect(args.join("\n")).toContain("Session app-1 spawned"); + expect(args.join("\n")).not.toContain("Open PR"); + }); + it("uses terminal-notifier for notifyWithActions too", async () => { const notifier = create({ dashboardUrl: "http://localhost:3000" }); const actions: NotifyAction[] = [{ label: "View", url: "https://example.com" }]; @@ -526,7 +539,7 @@ describe("notifier-desktop", () => { }; expect(payload.notificationId).toMatch(/^evt-native\./); expect(payload.threadId).toBe("ao.notifications"); - expect(payload.subtitle).toBe("my-project · s-9 · Info"); + expect(payload.subtitle).toBe("my-project · s-9"); expect(payload.defaultOpenUrl).toBe("http://localhost:3001/projects/my-project/sessions/s-9"); expect(payload.event).toMatchObject({ id: "evt-native", sessionId: "s-9" }); }); @@ -567,7 +580,9 @@ describe("notifier-desktop", () => { expect(payload.actions).toEqual([ { label: "Open PR", url: "https://github.com/example/pr/1" }, ]); - expect(payload.body).toContain("Kill"); + expect(payload.body).toContain("Session app-1 spawned"); + expect(payload.body).not.toContain("Actions:"); + expect(payload.body).not.toContain("Kill"); expect(payload.body).not.toContain("Open PR"); }); diff --git a/packages/plugins/notifier-desktop/src/index.ts b/packages/plugins/notifier-desktop/src/index.ts index cf25ac307a..99b07fad1d 100644 --- a/packages/plugins/notifier-desktop/src/index.ts +++ b/packages/plugins/notifier-desktop/src/index.ts @@ -3,6 +3,7 @@ import { existsSync } from "node:fs"; import { homedir, platform } from "node:os"; import { join } from "node:path"; import { + buildNotificationPresentation, escapeAppleScript, getNotificationDataV3, isMac, @@ -11,7 +12,6 @@ import { type OrchestratorEvent, type NotifyAction, type EventPriority, - type NotificationDataV3, } from "@aoagents/ao-core"; function xmlEscape(s: string): string { @@ -86,157 +86,24 @@ function shouldPlaySound(priority: EventPriority, soundEnabled: boolean): boolea return priority === "urgent"; } -function titleCaseStatus(value: string): string { - return value - .split(/[_\s.-]+/) - .filter(Boolean) - .map((part) => `${part.slice(0, 1).toUpperCase()}${part.slice(1)}`) - .join(" "); -} - function truncate(value: string, maxLength: number): string { return value.length > maxLength ? `${value.slice(0, maxLength - 1)}…` : value; } -function eventTitle(event: OrchestratorEvent, data: NotificationDataV3 | null): string { - const pr = data?.subject.pr; - - switch (event.type) { - case "ci.failing": - return pr ? `CI failing on PR #${pr.number}` : "CI failing"; - case "merge.ready": - return pr ? `PR #${pr.number} ready to merge` : "Pull request ready to merge"; - case "review.changes_requested": - return pr ? `Changes requested on PR #${pr.number}` : "Review changes requested"; - case "session.needs_input": - return "Agent needs input"; - case "session.stuck": - return "Agent may be stuck"; - case "session.killed": - case "session.exited": - return "Agent exited"; - case "pr.closed": - return pr ? `PR #${pr.number} closed` : "Pull request closed"; - case "summary.all_complete": - return "All sessions complete"; - default: - return titleCaseStatus(event.type); - } -} - -function formatTitle(event: OrchestratorEvent): string { +function formatSubtitle(event: OrchestratorEvent): string { const data = getNotificationDataV3(event.data); - const title = eventTitle(event, data); - if (event.priority === "urgent") return `URGENT: ${title}`; - if (event.priority === "warning") return `Warning: ${title}`; - return title; -} - -function formatSubtitle(event: OrchestratorEvent, data: NotificationDataV3 | null): string { const segments = [event.projectId, event.sessionId]; const pr = data?.subject.pr; if (pr) segments.push(`PR #${pr.number}`); - else segments.push(titleCaseStatus(event.priority)); return truncate(segments.join(" · "), 120); } -function formatBranch(data: NotificationDataV3 | null): string | undefined { - const pr = data?.subject.pr; - if (pr?.branch && pr.baseBranch) return `${pr.branch} → ${pr.baseBranch}`; - return pr?.branch ?? pr?.baseBranch ?? data?.subject.branch; -} - -function appendLine(lines: string[], value: string | undefined, maxLength = 180): void { - const trimmed = value?.trim(); - if (!trimmed) return; - lines.push(truncate(trimmed, maxLength)); -} - -function formatStatusLine(data: NotificationDataV3): string | undefined { - const segments: string[] = []; - - if (data.ci?.status) { - const failedChecks = data.ci.failedChecks?.map((check) => check.name) ?? []; - const failedText = failedChecks.length > 0 ? ` (${failedChecks.slice(0, 3).join(", ")})` : ""; - segments.push(`CI: ${titleCaseStatus(data.ci.status)}${failedText}`); - } - - if (data.review?.decision) { - const threads = - typeof data.review.unresolvedThreads === "number" - ? `, ${data.review.unresolvedThreads} threads` - : ""; - segments.push(`Review: ${titleCaseStatus(data.review.decision)}${threads}`); - } - - if (typeof data.merge?.ready === "boolean") { - segments.push(`Merge: ${data.merge.ready ? "Ready" : "Not ready"}`); - } - - if (typeof data.merge?.conflicts === "boolean") { - segments.push(`Conflicts: ${data.merge.conflicts ? "Found" : "None"}`); - } - - return segments.length > 0 ? segments.join(" · ") : undefined; -} - -function formatSubjectLine(data: NotificationDataV3 | null): string | undefined { - if (!data) return undefined; - const pr = data.subject.pr; - const issue = data.subject.issue; - const segments: string[] = []; - - if (pr) segments.push(`PR #${pr.number}`); - if (issue) segments.push(issue.id); - - return segments.length > 0 ? segments.join(" · ") : undefined; -} - -function formatDetailLine(label: string, value: string | undefined): string | undefined { - return value ? `${label}: ${value}` : undefined; -} - -function formatActionLine( - actions: NotifyAction[] | undefined, - hiddenActionIndexes: Set = new Set(), -): string | undefined { - const visibleActions = (actions ?? []).filter((_, index) => !hiddenActionIndexes.has(index)); - if (visibleActions.length === 0) return undefined; - return `Actions: ${visibleActions.map((action) => action.label).join(" · ")}`; -} - -function formatBody( - event: OrchestratorEvent, - actions?: NotifyAction[], - options: { hiddenActionIndexes?: Set } = {}, -): string { - const data = getNotificationDataV3(event.data); - const lines: string[] = []; - const subtitle = data?.subject.pr?.title ?? data?.subject.summary; - const branch = formatBranch(data); - - if (subtitle && subtitle !== event.message) appendLine(lines, subtitle, 150); - appendLine(lines, event.message, 180); - appendLine(lines, formatDetailLine("Context", formatSubjectLine(data)), 160); - if (data) appendLine(lines, formatDetailLine("Status", formatStatusLine(data)), 180); - appendLine(lines, formatDetailLine("Branch", branch), 140); - if (data?.transition) - appendLine(lines, `Transition: ${data.transition.from} → ${data.transition.to}`, 120); - appendLine(lines, formatActionLine(actions, options.hiddenActionIndexes), 160); - - return lines.join("\n"); -} - -function formatContent( - event: OrchestratorEvent, - actions?: NotifyAction[], - options: { hiddenActionIndexes?: Set } = {}, -): DesktopNotificationContent { - const data = getNotificationDataV3(event.data); +function formatContent(event: OrchestratorEvent): DesktopNotificationContent { + const presentation = buildNotificationPresentation(event); return { - title: formatTitle(event), - subtitle: formatSubtitle(event, data), - body: formatBody(event, actions, options), + title: presentation.title, + subtitle: formatSubtitle(event), + body: presentation.body, }; } @@ -333,17 +200,6 @@ function nativeActionPayload( return callbackEndpoint ? { label: action.label, callbackEndpoint } : undefined; } -function nativeActionIndexes( - actions: NotifyAction[] | undefined, - dashboardUrl: string | undefined, -): Set { - const indexes = new Set(); - for (const [index, action] of (actions ?? []).entries()) { - if (nativeActionPayload(action, dashboardUrl)) indexes.add(index); - } - return indexes; -} - function nativeActionPayloads( actions: NotifyAction[] | undefined, dashboardUrl: string | undefined, @@ -517,9 +373,7 @@ function sendNotification( // Don't crash the lifecycle on toast failures — log and resolve. // Common causes: stripped-down Windows SKU without WinRT, locked // group policy, or the user disabled toast notifications. - console.warn( - `[notifier-desktop] Windows toast failed: ${(err as Error).message}`, - ); + console.warn(`[notifier-desktop] Windows toast failed: ${(err as Error).message}`); } resolve(); }, @@ -569,13 +423,7 @@ export function create(config?: Record): Notifier { async notifyWithActions(event: OrchestratorEvent, actions: NotifyAction[]): Promise { const nativeActions = nativeActionPayloads(actions, dashboardUrl); - const content = formatContent(event, actions, { - hiddenActionIndexes: - backend === "ao-app" || (backend === "auto" && detectAoNotifierApp(appPath)) - ? nativeActionIndexes(actions, dashboardUrl) - : undefined, - }); - const fallbackContent = formatContent(event, actions); + const content = formatContent(event); const sound = shouldPlaySound(event.priority, soundEnabled); const isUrgent = event.priority === "urgent"; await sendNotification(content, event, { @@ -585,7 +433,6 @@ export function create(config?: Record): Notifier { notificationId: nextNativeNotificationId(event), openUrl: primaryOpenUrl(event, dashboardUrl, actions), actions: nativeActions, - fallbackContent, }); }, }; diff --git a/packages/web/src/app/globals.css b/packages/web/src/app/globals.css index 1e0d69f91e..1589cecd0a 100644 --- a/packages/web/src/app/globals.css +++ b/packages/web/src/app/globals.css @@ -1807,10 +1807,19 @@ html.light .xterm .xterm-viewport:hover::-webkit-scrollbar-thumb:active { font-family: var(--font-mono); } -.dashboard-notification-item__message { +.dashboard-notification-item__title { margin: 0; color: var(--color-text-primary); font-size: 12px; + font-weight: 650; + line-height: 1.45; + overflow-wrap: anywhere; +} + +.dashboard-notification-item__message { + margin: 2px 0 0; + color: var(--color-text-secondary); + font-size: 12px; font-weight: 500; line-height: 1.45; overflow-wrap: anywhere; diff --git a/packages/web/src/components/DashboardNotificationButton.tsx b/packages/web/src/components/DashboardNotificationButton.tsx index dd14f8f3f4..1b9b7c079c 100644 --- a/packages/web/src/components/DashboardNotificationButton.tsx +++ b/packages/web/src/components/DashboardNotificationButton.tsx @@ -190,6 +190,14 @@ function notificationKey(notification: DashboardNotificationRecord): string { return `${notification.id}:${notification.receivedAt}`; } +function presentationTitle(notification: DashboardNotificationRecord): string { + return notification.notification?.title ?? notification.event.message; +} + +function presentationBody(notification: DashboardNotificationRecord): string { + return notification.notification?.body ?? notification.event.message; +} + function readStoredReadIds(): Set { if (typeof window === "undefined") return new Set(); try { @@ -249,7 +257,8 @@ function NotificationItem({ {formatRelativeTime(notification.receivedAt)} -

{event.message}

+

{presentationTitle(notification)}

+

{presentationBody(notification)}

{event.projectId} {event.sessionId} diff --git a/packages/web/src/components/__tests__/DashboardNotificationButton.test.tsx b/packages/web/src/components/__tests__/DashboardNotificationButton.test.tsx index 7aa35936fa..a8dde7ce2d 100644 --- a/packages/web/src/components/__tests__/DashboardNotificationButton.test.tsx +++ b/packages/web/src/components/__tests__/DashboardNotificationButton.test.tsx @@ -38,6 +38,13 @@ function makeNotification(id: string, message: string): DashboardNotificationRec return { id: `${id}:2026-05-13T12:00:00.000Z`, receivedAt: `2026-05-13T12:00:0${id}.000Z`, + notification: { + version: 1, + category: "agent_needs_input", + priority: "action", + title: `Title ${id}`, + body: message, + }, event: { id, type: "session.needs_input", @@ -153,6 +160,33 @@ describe("DashboardNotificationButton", () => { expect(screen.getByText("First notification")).toBeInTheDocument(); }); + it("renders notification presentation before raw event message", () => { + muxValue.notifications = [ + { + ...makeNotification("1", "Concise body"), + notification: { + version: 1, + category: "ci_failing", + priority: "action", + title: "CI failing on PR #1", + body: "2 checks failed: typecheck, unit-tests.", + }, + event: { + ...makeNotification("1", "Concise body").event, + message: "Context: PR #1\nStatus: failing\nBranch: feat/report\nTransition: ok → bad", + }, + }, + ]; + + render(); + + fireEvent.click(screen.getByRole("button", { name: "Notifications" })); + + expect(screen.getByText("CI failing on PR #1")).toBeInTheDocument(); + expect(screen.getByText("2 checks failed: typecheck, unit-tests.")).toBeInTheDocument(); + expect(screen.queryByText(/Context: PR #1/)).not.toBeInTheDocument(); + }); + it("uses distinct classes for urgent and action notification colors", () => { muxValue.notifications = [ makePriorityNotification("1", "urgent", "Urgent notification"), diff --git a/packages/web/src/providers/__tests__/MuxProvider.test.tsx b/packages/web/src/providers/__tests__/MuxProvider.test.tsx index e2af691fe5..30cb1e81ff 100644 --- a/packages/web/src/providers/__tests__/MuxProvider.test.tsx +++ b/packages/web/src/providers/__tests__/MuxProvider.test.tsx @@ -522,6 +522,13 @@ describe("MuxProvider message handling", () => { { id: "n1", receivedAt: "2026-05-13T10:00:00.000Z", + notification: { + version: 1, + category: "all_complete", + priority: "info", + title: "All sessions complete", + body: "Done", + }, event: { id: "evt-1", type: "summary.all_complete", @@ -549,6 +556,13 @@ describe("MuxProvider message handling", () => { { id: "n2", receivedAt: "2026-05-13T10:01:00.000Z", + notification: { + version: 1, + category: "agent_needs_input", + priority: "action", + title: "Agent needs input", + body: "s2 is waiting for input.", + }, event: { id: "evt-2", type: "session.needs_input", @@ -563,6 +577,13 @@ describe("MuxProvider message handling", () => { { id: "n3", receivedAt: "2026-05-13T10:02:00.000Z", + notification: { + version: 1, + category: "ci_failing", + priority: "action", + title: "CI failing", + body: "CI is failing and needs attention.", + }, event: { id: "evt-3", type: "ci.failing", diff --git a/schema/config.schema.json b/schema/config.schema.json index cf4dd56cc2..db5dc68935 100644 --- a/schema/config.schema.json +++ b/schema/config.schema.json @@ -401,7 +401,7 @@ }, "notifiers": { "type": "array", - "default": [], + "default": ["dashboard", "desktop"], "items": { "type": "string" }