diff --git a/apps/desktop/docs/EXTERNAL_FILES.md b/apps/desktop/docs/EXTERNAL_FILES.md index 5cc01ad97cb..471bb2970e2 100644 --- a/apps/desktop/docs/EXTERNAL_FILES.md +++ b/apps/desktop/docs/EXTERNAL_FILES.md @@ -45,13 +45,15 @@ its hook entries into these files while preserving user-defined entries: | `~/.codex/hooks.json` | Codex hook registration merge (`SessionStart`, `UserPromptSubmit`, `Stop`) | | `~/.factory/settings.json` | Factory Droid hook registration (`UserPromptSubmit`, `Notification`, `PostToolUse`, `Stop`) | -For Codex specifically, Superset now relies on native `~/.codex/hooks.json` -registration for durable prompt/tool lifecycle events, while the wrapper in -`~/.superset[-{workspace}]/bin/codex` still injects `notify` and keeps the -session-log watcher as a best-effort compatibility bridge for older Codex -releases. On startup, Superset rewrites only its own managed entries in -`~/.codex/hooks.json` to point at the current environment's `notify.sh`, while -preserving any user-defined Codex hooks. +For Codex specifically, Superset relies on native `~/.codex/hooks.json` +registration as the sole source of completion notifications. The wrapper in +`~/.superset[-{workspace}]/bin/codex` only enables `codex_hooks` (by passing +`--enable codex_hooks` to the real binary) and keeps the session-log watcher +as a best-effort bridge for per-prompt Start notifications and permission +requests inside Superset terminals. It no longer injects `--notify=[...]` to +avoid duplicate `/hook/complete` POSTs. On startup, Superset rewrites only its +own managed entries in `~/.codex/hooks.json` to point at the current +environment's `notify.sh`, while preserving any user-defined Codex hooks. ### `zsh/` and `bash/` - Shell Integration diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index f2b28930ddd..4a8a3586b7c 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -57,7 +57,11 @@ import { createWorkspaceMediaProtocolHandler } from "./lib/workspace-media-proto const loadVscodeShim = () => import("./lib/vscode-shim") as Promise; -import { cleanupMainWindowResources, MainWindow } from "./windows/main"; +import { + cleanupMainWindowResources, + initNotifications, + MainWindow, +} from "./windows/main"; console.log("[main] Local database ready:", !!localDb); const IS_DEV = process.env.NODE_ENV === "development"; @@ -773,6 +777,7 @@ if (!gotTheLock) { }); } + initNotifications(); await makeAppSetup(() => MainWindow()); setupAutoUpdater(); setupServiceStatusPolling(); diff --git a/apps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.ts b/apps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.ts index db889900cd5..06ccefe25cb 100644 --- a/apps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.ts +++ b/apps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.ts @@ -275,7 +275,8 @@ export function createClaudeWrapper(): void { } /** - * Creates the Codex wrapper that injects Superset's notify/session-log logic. + * Creates the Codex wrapper that enables native hooks and keeps the + * session-log watcher for prompt/permission events inside Superset terminals. */ export function createCodexWrapper(): void { const notifyPath = getNotifyScriptPath(); @@ -423,14 +424,15 @@ export function getCodexGlobalHooksJsonContent( /** * Writes Superset hook definitions directly into ~/.codex/hooks.json. - * This provides a fallback notification path that works even when the - * binary wrapper is not in PATH (e.g. user runs codex from outside + * This is the primary lifecycle notification path for Codex and also works + * when the binary wrapper is not in PATH (e.g. user runs codex from outside * a Superset terminal). * - * The wrapper still injects Codex's native notify callback and keeps the - * session-log watcher as a best-effort bridge for older releases, but the - * native hooks.json registration is now the primary source for prompt/tool - * lifecycle events. + * The wrapper only enables Codex hooks and keeps the session-log watcher as a + * best-effort bridge for prompt/permission events inside Superset terminals. + * Completion notifications are handled exclusively via hooks.json to avoid + * the duplicate `/hook/complete` POSTs that occurred when the wrapper also + * injected `--notify=[...]`. */ export function createCodexHooksJson(): void { const notifyScriptPath = getNotifyScriptPath(); diff --git a/apps/desktop/src/main/lib/agent-setup/agent-wrappers.test.ts b/apps/desktop/src/main/lib/agent-setup/agent-wrappers.test.ts index 9e96a3e4912..338c6e196f8 100644 --- a/apps/desktop/src/main/lib/agent-setup/agent-wrappers.test.ts +++ b/apps/desktop/src/main/lib/agent-setup/agent-wrappers.test.ts @@ -172,7 +172,7 @@ describe("agent-wrappers copilot", () => { expect(updated).not.toContain("/tmp/old-hook.sh"); }); - it("injects codex start + permission watchers and completion notifications in wrapper", () => { + it("injects codex start + permission watchers and enables native hooks", () => { createCodexWrapper(); const wrapperPath = path.join(TEST_BIN_DIR, "codex"); @@ -197,9 +197,7 @@ describe("agent-wrappers copilot", () => { expect(wrapper).toContain('awk -F\'"approval_id":"\''); expect(wrapper).toContain('_superset_emit_event "Start"'); expect(wrapper).toContain('_superset_emit_event "PermissionRequest"'); - expect(wrapper).toContain( - `"$REAL_BIN" --enable codex_hooks -c 'notify=["bash","${path.join(TEST_HOOKS_DIR, "notify.sh")}"]' "$@"`, - ); + expect(wrapper).toContain(`"$REAL_BIN" --enable codex_hooks "$@"`); expect(wrapper).toContain("SUPERSET_CODEX_START_WATCHER_PID"); expect(wrapper).toContain('kill "$SUPERSET_CODEX_START_WATCHER_PID"'); @@ -239,14 +237,9 @@ exit 0 }); expect(readFileSync(argsFile, "utf-8")).toBe( - `${[ - "--enable", - "codex_hooks", - "-c", - `notify=["bash","${path.join(TEST_HOOKS_DIR, "notify.sh")}"]`, - "exec", - "Reply with exactly OK.", - ].join("\n")}\n`, + `${["--enable", "codex_hooks", "exec", "Reply with exactly OK."].join( + "\n", + )}\n`, ); }); diff --git a/apps/desktop/src/main/lib/agent-setup/templates/codex-wrapper-exec.template.sh b/apps/desktop/src/main/lib/agent-setup/templates/codex-wrapper-exec.template.sh index 8aa12395105..86130c5e18d 100644 --- a/apps/desktop/src/main/lib/agent-setup/templates/codex-wrapper-exec.template.sh +++ b/apps/desktop/src/main/lib/agent-setup/templates/codex-wrapper-exec.template.sh @@ -1,6 +1,6 @@ -# Codex exposes completion notifications via notify. -# For per-prompt Start notifications and permission requests, watch the TUI -# session log for task_started/exec_command_begin and *_approval_request events. +# Native ~/.codex/hooks.json handles SessionStart/UserPromptSubmit/Stop. +# The wrapper keeps the session-log watcher only for per-prompt Start +# notifications and permission requests inside Superset terminals. if [ -n "$SUPERSET_TAB_ID" ] && [ -f "{{NOTIFY_PATH}}" ]; then export CODEX_TUI_RECORD_SESSION=1 if [ -z "$CODEX_TUI_SESSION_LOG_PATH" ]; then @@ -72,7 +72,7 @@ if [ -n "$SUPERSET_TAB_ID" ] && [ -f "{{NOTIFY_PATH}}" ]; then SUPERSET_CODEX_START_WATCHER_PID=$! fi -"$REAL_BIN" --enable codex_hooks -c 'notify=["bash","{{NOTIFY_PATH}}"]' "$@" +"$REAL_BIN" --enable codex_hooks "$@" SUPERSET_CODEX_STATUS=$? if [ -n "$SUPERSET_CODEX_START_WATCHER_PID" ]; then diff --git a/apps/desktop/src/main/windows/main.ts b/apps/desktop/src/main/windows/main.ts index a542061ad0f..ad5d2cda604 100644 --- a/apps/desktop/src/main/windows/main.ts +++ b/apps/desktop/src/main/windows/main.ts @@ -1,3 +1,4 @@ +import type { Server } from "node:http"; import { join } from "node:path"; import * as Sentry from "@sentry/electron/main"; import { projects, workspaces, worktrees } from "@superset/local-db"; @@ -105,6 +106,19 @@ function buildAivisVars(event: AgentLifecycleEvent) { let currentWindow: BrowserWindow | null = null; let mainWindowCleanup: (() => void) | null = null; +let notificationsInitialized = false; +let notificationsServer: Server | null = null; +let notificationManager: NotificationManager | null = null; +let agentLifecycleListener: ((event: AgentLifecycleEvent) => void) | null = + null; +let terminalExitListener: + | ((event: { + paneId: string; + exitCode: number; + signal?: number; + reason?: "killed" | "exited" | "error"; + }) => void) + | null = null; /** Tear down main window resources (notification server, IPC, etc.) * without destroying the BrowserWindow itself. Called from before-quit @@ -112,6 +126,7 @@ let mainWindowCleanup: (() => void) | null = null; export function cleanupMainWindowResources(): void { mainWindowCleanup?.(); mainWindowCleanup = null; + cleanupNotifications(); } function addWindowLifecycleBreadcrumb( @@ -175,6 +190,110 @@ nativeTheme.on("updated", () => { } }); +export function initNotifications(): void { + if (notificationsInitialized) return; + + notificationManager = new NotificationManager({ + isSupported: () => Notification.isSupported(), + createNotification: (opts) => new Notification(opts), + playSound: playNotificationSound, + playAivis: (event) => { + const kind = + event.eventType === "PermissionRequest" ? "permission" : "complete"; + void playAivisNotification(kind, buildAivisVars(event)); + }, + onNotificationClick: (ids) => { + const window = getWindow(); + if (window && !window.isDestroyed()) { + window.show(); + window.focus(); + } else { + app.emit("activate"); + } + notificationsEmitter.emit(NOTIFICATION_EVENTS.FOCUS_TAB, ids); + }, + getVisibilityContext: () => { + const window = getWindow(); + const windowIsReady = window && !window.isDestroyed(); + return { + isFocused: windowIsReady ? window.isFocused() : false, + currentWorkspaceId: windowIsReady + ? extractWorkspaceIdFromUrl(window.webContents.getURL()) + : null, + tabsState: appState.data?.tabsState, + }; + }, + getWorkspaceName: getWorkspaceNameFromDb, + getNotificationTitle: (event) => + getNotificationTitle({ + tabId: event.tabId, + paneId: event.paneId, + tabs: appState.data?.tabsState?.tabs, + panes: appState.data?.tabsState?.panes, + }), + }); + notificationManager.start(); + + agentLifecycleListener = (event: AgentLifecycleEvent) => { + notificationManager?.handleAgentLifecycle(event); + }; + notificationsEmitter.on( + NOTIFICATION_EVENTS.AGENT_LIFECYCLE, + agentLifecycleListener, + ); + + terminalExitListener = (event) => { + notificationsEmitter.emit(NOTIFICATION_EVENTS.TERMINAL_EXIT, { + paneId: event.paneId, + exitCode: event.exitCode, + signal: event.signal, + reason: event.reason, + }); + }; + getWorkspaceRuntimeRegistry() + .getDefault() + .terminal.on("terminalExit", terminalExitListener); + + notificationsServer = notificationsApp.listen( + env.DESKTOP_NOTIFICATIONS_PORT, + "127.0.0.1", + () => { + console.log( + `[notifications] Listening on http://127.0.0.1:${env.DESKTOP_NOTIFICATIONS_PORT}`, + ); + }, + ); + + notificationsInitialized = true; +} + +function cleanupNotifications(): void { + if (!notificationsInitialized) return; + + if (agentLifecycleListener) { + notificationsEmitter.off( + NOTIFICATION_EVENTS.AGENT_LIFECYCLE, + agentLifecycleListener, + ); + agentLifecycleListener = null; + } + + if (terminalExitListener) { + getWorkspaceRuntimeRegistry() + .getDefault() + .terminal.off("terminalExit", terminalExitListener); + terminalExitListener = null; + } + + notificationManager?.dispose(); + notificationManager = null; + + notificationsServer?.close(); + notificationsServer = null; + + notificationsInitialized = false; +} + export async function MainWindow() { const shouldPersistWindowPosition = isWindowPositionPersistenceEnabled(); const savedWindowState = loadWindowState(); @@ -244,77 +363,6 @@ export async function MainWindow() { windowManager.setIpcHandler(ipcHandler); } - const server = notificationsApp.listen( - env.DESKTOP_NOTIFICATIONS_PORT, - "127.0.0.1", - () => { - console.log( - `[notifications] Listening on http://127.0.0.1:${env.DESKTOP_NOTIFICATIONS_PORT}`, - ); - }, - ); - - const notificationManager = new NotificationManager({ - isSupported: () => Notification.isSupported(), - createNotification: (opts) => new Notification(opts), - playSound: playNotificationSound, - playAivis: (event) => { - const kind = - event.eventType === "PermissionRequest" ? "permission" : "complete"; - void playAivisNotification(kind, buildAivisVars(event)); - }, - onNotificationClick: (ids) => { - window.show(); - window.focus(); - notificationsEmitter.emit(NOTIFICATION_EVENTS.FOCUS_TAB, ids); - }, - getVisibilityContext: () => ({ - isFocused: window.isFocused(), - currentWorkspaceId: extractWorkspaceIdFromUrl( - window.webContents.getURL(), - ), - tabsState: appState.data?.tabsState, - }), - getWorkspaceName: getWorkspaceNameFromDb, - getNotificationTitle: (event) => - getNotificationTitle({ - tabId: event.tabId, - paneId: event.paneId, - tabs: appState.data?.tabsState?.tabs, - panes: appState.data?.tabsState?.panes, - }), - }); - notificationManager.start(); - - notificationsEmitter.on( - NOTIFICATION_EVENTS.AGENT_LIFECYCLE, - (event: AgentLifecycleEvent) => { - notificationManager.handleAgentLifecycle(event); - }, - ); - - // Forward low-volume terminal lifecycle events to the renderer via the existing - // notifications subscription. This is used only for correctness (e.g. clearing - // stuck agent lifecycle statuses when terminal panes aren't mounted). - getWorkspaceRuntimeRegistry() - .getDefault() - .terminal.on( - "terminalExit", - (event: { - paneId: string; - exitCode: number; - signal?: number; - reason?: "killed" | "exited" | "error"; - }) => { - notificationsEmitter.emit(NOTIFICATION_EVENTS.TERMINAL_EXIT, { - paneId: event.paneId, - exitCode: event.exitCode, - signal: event.signal, - reason: event.reason, - }); - }, - ); - // macOS Sequoia+: occluded/minimized windows can lose compositor layers, // and NSVisualEffectView's vibrancy/native blur can detach while the // window is in the Dock — restoring without re-applying leaves the @@ -483,10 +531,6 @@ export async function MainWindow() { function doCleanup() { browserManager.unregisterAll(); - server.close(); - notificationManager.dispose(); - notificationsEmitter.removeAllListeners(); - getWorkspaceRuntimeRegistry().getDefault().terminal.detachAllListeners(); ipcHandler?.detachWindow(window); windowManager.unregister("main"); currentWindow = null;