Skip to content

[codex] fix sidebar notifications after host restart #4703

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Kitenite merged 1 commit into from
May 19, 2026
Merged

[codex] fix sidebar notifications after host restart #4703

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/desktop/src/main/lib/agent-setup/agent-wrappers-copilot.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { HOOKS_DIR } from "./paths";
export const COPILOT_HOOK_SCRIPT_NAME = "copilot-hook.sh";

const COPILOT_HOOK_SIGNATURE = "# Superset copilot hook";
const COPILOT_HOOK_VERSION = "v1";
const COPILOT_HOOK_VERSION = "v2";
export const COPILOT_HOOK_MARKER = `${COPILOT_HOOK_SIGNATURE} ${COPILOT_HOOK_VERSION}`;

const COPILOT_HOOK_TEMPLATE_PATH = path.join(
Expand Down
2 changes: 1 addition & 1 deletion apps/desktop/src/main/lib/agent-setup/agent-wrappers-cursor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { HOOKS_DIR } from "./paths";
export const CURSOR_HOOK_SCRIPT_NAME = "cursor-hook.sh";

const CURSOR_HOOK_SIGNATURE = "# Superset cursor hook";
const CURSOR_HOOK_VERSION = "v2";
const CURSOR_HOOK_VERSION = "v3";
export const CURSOR_HOOK_MARKER = `${CURSOR_HOOK_SIGNATURE} ${CURSOR_HOOK_VERSION}`;

const CURSOR_HOOK_TEMPLATE_PATH = path.join(
Expand Down
2 changes: 1 addition & 1 deletion apps/desktop/src/main/lib/agent-setup/agent-wrappers-gemini.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { HOOKS_DIR } from "./paths";
export const GEMINI_HOOK_SCRIPT_NAME = "gemini-hook.sh";

const GEMINI_HOOK_SIGNATURE = "# Superset gemini hook";
const GEMINI_HOOK_VERSION = "v2";
const GEMINI_HOOK_VERSION = "v3";
export const GEMINI_HOOK_MARKER = `${GEMINI_HOOK_SIGNATURE} ${GEMINI_HOOK_VERSION}`;

const GEMINI_HOOK_TEMPLATE_PATH = path.join(
Expand Down
10 changes: 6 additions & 4 deletions apps/desktop/src/main/lib/agent-setup/agent-wrappers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ mock.module("shared/env.shared", () => ({

mock.module("./notify-hook", () => ({
NOTIFY_SCRIPT_NAME: "notify.sh",
NOTIFY_SCRIPT_MARKER: "# Superset agent notification hook",
NOTIFY_SCRIPT_MARKER: "# Superset agent notification hook v3",
getNotifyScriptPath: () => path.join(TEST_HOOKS_DIR, "notify.sh"),
getNotifyScriptContent: () => "#!/bin/bash\nexit 0\n",
createNotifyScript: () => {},
Expand Down Expand Up @@ -66,6 +66,7 @@ const {
createClaudeSettingsJson,
createCodexHooksJson,
createCodexWrapper,
COPILOT_HOOK_MARKER,
CURSOR_HOOK_MARKER,
createDroidSettingsJson,
createDroidWrapper,
Expand Down Expand Up @@ -688,9 +689,10 @@ exit 0
expect(JSON.parse(content2)).toEqual(JSON.parse(content));
});

it("bumps Cursor and Gemini hook script markers when hook semantics change", () => {
expect(CURSOR_HOOK_MARKER).toBe("# Superset cursor hook v2");
expect(GEMINI_HOOK_MARKER).toBe("# Superset gemini hook v2");
it("bumps hook script markers when hook semantics change", () => {
expect(COPILOT_HOOK_MARKER).toBe("# Superset copilot hook v2");
expect(CURSOR_HOOK_MARKER).toBe("# Superset cursor hook v3");
expect(GEMINI_HOOK_MARKER).toBe("# Superset gemini hook v3");
});

it("replaces stale Mastra hook commands from old superset paths", () => {
Expand Down
9 changes: 8 additions & 1 deletion apps/desktop/src/main/lib/agent-setup/notify-hook.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import { describe, expect, it } from "bun:test";
import { readFileSync } from "node:fs";
import path from "node:path";
import { NOTIFY_SCRIPT_MARKER } from "./notify-hook";

describe("getNotifyScriptContent", () => {
it("bumps the notify hook marker when hook semantics change", () => {
expect(NOTIFY_SCRIPT_MARKER).toBe("# Superset agent notification hook v3");
});

it("emits the v2 host-service payload with full agent identity", () => {
const script = readFileSync(
path.join(import.meta.dir, "templates", "notify-hook.template.sh"),
Expand Down Expand Up @@ -41,9 +46,10 @@ describe("getNotifyScriptContent", () => {
'if [ -n "$SUPERSET_HOST_AGENT_HOOK_URL" ] && [ -n "$SUPERSET_TERMINAL_ID" ]; then',
);
expect(script).toContain(
'[ -z "$SUPERSET_TAB_ID" ] && [ -z "$SESSION_ID" ] && exit 0',
'[ -z "$SUPERSET_TAB_ID" ] && [ -z "$SESSION_ID" ] && [ -z "$SUPERSET_TERMINAL_ID" ] && exit 0',
);
expect(script).toContain("/hook/complete");
expect(script).toContain("terminalId=$SUPERSET_TERMINAL_ID");
expect(script).toContain("SUPERSET_TAB_ID");
expect(script).toContain("SUPERSET_PANE_ID");
});
Expand Down Expand Up @@ -71,6 +77,7 @@ describe("per-agent hook scripts dispatch to v2", () => {
expect(script).toContain("/hook/complete");
expect(script).toContain('V1_EVENT_TYPE="$EVENT_TYPE"');
expect(script).toContain("eventType=$V1_EVENT_TYPE");
expect(script).toContain("terminalId=$SUPERSET_TERMINAL_ID");
expect(script).toContain("SUPERSET_TAB_ID");
expect(script).toContain("SUPERSET_PANE_ID");
});
Expand Down
2 changes: 1 addition & 1 deletion apps/desktop/src/main/lib/agent-setup/notify-hook.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { env } from "shared/env.shared";
import { HOOKS_DIR } from "./paths";

export const NOTIFY_SCRIPT_NAME = "notify.sh";
export const NOTIFY_SCRIPT_MARKER = "# Superset agent notification hook";
export const NOTIFY_SCRIPT_MARKER = "# Superset agent notification hook v3";

const NOTIFY_SCRIPT_TEMPLATE_PATH = path.join(
__dirname,
Expand Down
3 changes: 2 additions & 1 deletion apps/desktop/src/main/lib/agent-setup/templates/copilot-hook.template.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,14 @@ if [ -n "$SUPERSET_HOST_AGENT_HOOK_URL" ] && [ -n "$SUPERSET_TERMINAL_ID" ]; the
esac
fi

[ -z "$SUPERSET_TAB_ID" ] && exit 0
[ -z "$SUPERSET_TAB_ID" ] && [ -z "$SUPERSET_TERMINAL_ID" ] && exit 0

curl -sG "http://127.0.0.1:${SUPERSET_PORT:-{{DEFAULT_PORT}}}/hook/complete" \
--connect-timeout 1 --max-time 2 \
--data-urlencode "paneId=$SUPERSET_PANE_ID" \
--data-urlencode "tabId=$SUPERSET_TAB_ID" \
--data-urlencode "workspaceId=$SUPERSET_WORKSPACE_ID" \
--data-urlencode "terminalId=$SUPERSET_TERMINAL_ID" \
--data-urlencode "sessionId=$HOOK_SESSION_ID" \
--data-urlencode "hookSessionId=$HOOK_SESSION_ID" \
--data-urlencode "eventType=$V1_EVENT_TYPE" \
Expand Down
3 changes: 2 additions & 1 deletion apps/desktop/src/main/lib/agent-setup/templates/cursor-hook.template.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,14 @@ if [ -n "$SUPERSET_HOST_AGENT_HOOK_URL" ] && [ -n "$SUPERSET_TERMINAL_ID" ]; the
esac
fi

[ -z "$SUPERSET_TAB_ID" ] && exit 0
[ -z "$SUPERSET_TAB_ID" ] && [ -z "$SUPERSET_TERMINAL_ID" ] && exit 0

curl -sG "http://127.0.0.1:${SUPERSET_PORT:-{{DEFAULT_PORT}}}/hook/complete" \
--connect-timeout 1 --max-time 2 \
--data-urlencode "paneId=$SUPERSET_PANE_ID" \
--data-urlencode "tabId=$SUPERSET_TAB_ID" \
--data-urlencode "workspaceId=$SUPERSET_WORKSPACE_ID" \
--data-urlencode "terminalId=$SUPERSET_TERMINAL_ID" \
--data-urlencode "sessionId=$HOOK_SESSION_ID" \
--data-urlencode "hookSessionId=$HOOK_SESSION_ID" \
--data-urlencode "eventType=$V1_EVENT_TYPE" \
Expand Down
3 changes: 2 additions & 1 deletion apps/desktop/src/main/lib/agent-setup/templates/gemini-hook.template.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,14 @@ if [ -n "$SUPERSET_HOST_AGENT_HOOK_URL" ] && [ -n "$SUPERSET_TERMINAL_ID" ]; the
esac
fi

[ -z "$SUPERSET_TAB_ID" ] && exit 0
[ -z "$SUPERSET_TAB_ID" ] && [ -z "$SUPERSET_TERMINAL_ID" ] && exit 0

curl -sG "http://127.0.0.1:${SUPERSET_PORT:-{{DEFAULT_PORT}}}/hook/complete" \
--connect-timeout 1 --max-time 2 \
--data-urlencode "paneId=$SUPERSET_PANE_ID" \
--data-urlencode "tabId=$SUPERSET_TAB_ID" \
--data-urlencode "workspaceId=$SUPERSET_WORKSPACE_ID" \
--data-urlencode "terminalId=$SUPERSET_TERMINAL_ID" \
--data-urlencode "sessionId=$HOOK_SESSION_ID" \
--data-urlencode "hookSessionId=$HOOK_SESSION_ID" \
--data-urlencode "eventType=$V1_EVENT_TYPE" \
Expand Down
4 changes: 3 additions & 1 deletion apps/desktop/src/main/lib/agent-setup/templates/notify-hook.template.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,15 @@ if [ -n "$SUPERSET_HOST_AGENT_HOOK_URL" ] && [ -n "$SUPERSET_TERMINAL_ID" ]; the
fi

# v1 fallback: Electron localhost hook server. Kept while v1 terminals exist.
[ -z "$SUPERSET_TAB_ID" ] && [ -z "$SESSION_ID" ] && exit 0
[ -z "$SUPERSET_TAB_ID" ] && [ -z "$SESSION_ID" ] && [ -z "$SUPERSET_TERMINAL_ID" ] && exit 0
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Double native notification + sound in healthy v2 scenarios

The relaxed guard now allows the Electron fallback to fire for every v2 terminal event, not just when the v2 hook URL is stale. In a healthy session where the host-service SSE connection is alive, HostNotificationSubscriber already calls handleV2AgentLifecycleEvent (which calls showNativeNotification + playRingtone). The Electron fallback now also fires, causing NotificationManager.handleAgentLifecycle to show a second native notification and play the sound a second time. The same pattern applies to the copilot/cursor/gemini templates.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: apps/desktop/src/main/lib/agent-setup/templates/notify-hook.template.sh
Line: 96

Comment:
**Double native notification + sound in healthy v2 scenarios**

The relaxed guard now allows the Electron fallback to fire for _every_ v2 terminal event, not just when the v2 hook URL is stale. In a healthy session where the host-service SSE connection is alive, `HostNotificationSubscriber` already calls `handleV2AgentLifecycleEvent` (which calls `showNativeNotification` + `playRingtone`). The Electron fallback now also fires, causing `NotificationManager.handleAgentLifecycle` to show a second native notification and play the sound a second time. The same pattern applies to the copilot/cursor/gemini templates.

How can I resolve this? If you propose a fix, please make it concise.


if [ "$DEBUG_HOOKS_ENABLED" = "1" ]; then
STATUS_CODE=$(curl -sG "http://127.0.0.1:${SUPERSET_PORT:-{{DEFAULT_PORT}}}/hook/complete" \
--connect-timeout 1 --max-time 2 \
--data-urlencode "paneId=$SUPERSET_PANE_ID" \
--data-urlencode "tabId=$SUPERSET_TAB_ID" \
--data-urlencode "workspaceId=$SUPERSET_WORKSPACE_ID" \
--data-urlencode "terminalId=$SUPERSET_TERMINAL_ID" \
--data-urlencode "sessionId=$SESSION_ID" \
--data-urlencode "hookSessionId=$HOOK_SESSION_ID" \
--data-urlencode "resourceId=$RESOURCE_ID" \
Expand All @@ -117,6 +118,7 @@ else
--data-urlencode "paneId=$SUPERSET_PANE_ID" \
--data-urlencode "tabId=$SUPERSET_TAB_ID" \
--data-urlencode "workspaceId=$SUPERSET_WORKSPACE_ID" \
--data-urlencode "terminalId=$SUPERSET_TERMINAL_ID" \
--data-urlencode "sessionId=$SESSION_ID" \
--data-urlencode "hookSessionId=$HOOK_SESSION_ID" \
--data-urlencode "resourceId=$RESOURCE_ID" \
Expand Down
55 changes: 55 additions & 0 deletions apps/desktop/src/main/lib/host-service-coordinator.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ const baseManifest = (pid: number, endpoint = "http://127.0.0.1:55555") => ({

const spawnConfig = { authToken: "token", cloudApiUrl: "https://api.example" };

interface HostServiceCoordinatorInternals {
getPreferredPorts(organizationId: string): number[];
rememberPort(organizationId: string, port: number): void;
}

describe("HostServiceCoordinator.tryAdopt — adoption health check", () => {
let coordinator: InstanceType<typeof HostServiceCoordinator>;
let spawnMock: ReturnType<typeof mock>;
Expand Down Expand Up @@ -258,6 +263,56 @@ describe("HostServiceCoordinator.tryAdopt — adoption health check", () => {
});
});

describe("HostServiceCoordinator preferred ports", () => {
let coordinator: InstanceType<typeof HostServiceCoordinator>;

beforeEach(() => {
manifestStore.current = null;
readManifestMock.mockClear();
removeManifestMock.mockClear();
isProcessAliveMock.mockClear();
listManifestsMock.mockClear();
killProcessMock.mockClear();
pollHealthCheckMock.mockClear();

testManifestRoot = fs.mkdtempSync(path.join(os.tmpdir(), "hsc-test-"));
coordinator = new HostServiceCoordinator();
});

afterEach(() => {
coordinator.releaseAll();
if (testManifestRoot) {
fs.rmSync(testManifestRoot, { recursive: true, force: true });
testManifestRoot = "";
}
});

test("prefers the last known port, then the manifest port, then a stable org port", () => {
manifestStore.current = baseManifest(1234, "http://127.0.0.1:45555");
const internals = coordinator as unknown as HostServiceCoordinatorInternals;
internals.rememberPort("org-1", 46666);

const ports = internals.getPreferredPorts("org-1");

expect(ports[0]).toBe(46666);
expect(ports[1]).toBe(45555);
expect(ports[2]).toBeGreaterThanOrEqual(48_000);
expect(ports[2]).toBeLessThan(49_000);
});

test("uses a deterministic stable port when no previous port exists", () => {
const internals = coordinator as unknown as HostServiceCoordinatorInternals;

const ports = internals.getPreferredPorts("org-1");
const secondRead = internals.getPreferredPorts("org-1");

expect(ports).toEqual(secondRead);
expect(ports).toHaveLength(1);
expect(ports[0]).toBeGreaterThanOrEqual(48_000);
expect(ports[0]).toBeLessThan(49_000);
});
});

describe("HostServiceCoordinator.reset", () => {
let coordinator: InstanceType<typeof HostServiceCoordinator>;
let spawnMock: ReturnType<typeof mock>;
Expand Down
Loading
Loading