From 627cf7b85a26ecb5a5af6924b06b9aabe4ed07f5 Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Sat, 15 Aug 2026 12:37:31 -0700 Subject: [PATCH 1/2] fix(desktop): resolve agent hook endpoint at call time so host-service restarts don't strand running agents SUPERSET_HOST_AGENT_HOOK_URL is baked into each terminal's env at creation with the host-service port of that moment. PTYs survive restarts via the pty-daemon, so when the service comes back on a new port (always, via the CLI's findFreePort), every already-running agent POSTs its status to a dead port forever, silently. The hook scripts now resolve the endpoint when they fire: try the env URL first, then every ~/.superset/host/*/manifest.json endpoint (rewritten on every service start). Only the owning host answers "ignored":false, so probing other orgs' hosts is a harmless no-op. Because the scripts are re-read from disk per event, shipping this heals agents that are already running. Markers bumped: notify v9, cursor v7, copilot v5, gemini v6. Rebased onto #6479's move of agent-setup into packages/agent-setup, which also provisions hooks on headless CLI hosts, extending this fix to pure-headless installs. Root-cause map, repro transcripts, and fix rationale in plans/agent-hook-url-frozen-port.md. --- .../agent-setup/src/agent-wrappers-copilot.ts | 2 +- .../agent-setup/src/agent-wrappers-cursor.ts | 2 +- .../agent-setup/src/agent-wrappers-gemini.ts | 2 +- .../agent-setup/src/agent-wrappers.test.ts | 8 +- packages/agent-setup/src/notify-hook.test.ts | 192 ++++++++++++++++-- packages/agent-setup/src/notify-hook.ts | 2 +- .../templates/copilot-hook.template.sh | 44 +++- .../templates/cursor-hook.template.sh | 44 +++- .../templates/gemini-hook.template.sh | 47 ++++- .../templates/notify-hook.template.sh | 63 ++++-- plans/agent-hook-url-frozen-port.md | 142 +++++++++++++ 11 files changed, 477 insertions(+), 71 deletions(-) create mode 100644 plans/agent-hook-url-frozen-port.md diff --git a/packages/agent-setup/src/agent-wrappers-copilot.ts b/packages/agent-setup/src/agent-wrappers-copilot.ts index 63068c7f970..6a9833117a6 100644 --- a/packages/agent-setup/src/agent-wrappers-copilot.ts +++ b/packages/agent-setup/src/agent-wrappers-copilot.ts @@ -11,7 +11,7 @@ import { getHooksDir } from "./paths"; export const COPILOT_HOOK_SCRIPT_NAME = "copilot-hook.sh"; const COPILOT_HOOK_SIGNATURE = "# Superset copilot hook"; -const COPILOT_HOOK_VERSION = "v4"; +const COPILOT_HOOK_VERSION = "v5"; export const COPILOT_HOOK_MARKER = `${COPILOT_HOOK_SIGNATURE} ${COPILOT_HOOK_VERSION}`; export function getCopilotHookScriptPath(): string { diff --git a/packages/agent-setup/src/agent-wrappers-cursor.ts b/packages/agent-setup/src/agent-wrappers-cursor.ts index d98873332d4..1f5af757e1d 100644 --- a/packages/agent-setup/src/agent-wrappers-cursor.ts +++ b/packages/agent-setup/src/agent-wrappers-cursor.ts @@ -19,7 +19,7 @@ import { getHooksDir } from "./paths"; export const CURSOR_HOOK_SCRIPT_NAME = "cursor-hook.sh"; const CURSOR_HOOK_SIGNATURE = "# Superset cursor hook"; -const CURSOR_HOOK_VERSION = "v6"; +const CURSOR_HOOK_VERSION = "v7"; export const CURSOR_HOOK_MARKER = `${CURSOR_HOOK_SIGNATURE} ${CURSOR_HOOK_VERSION}`; interface CursorHookEntry { diff --git a/packages/agent-setup/src/agent-wrappers-gemini.ts b/packages/agent-setup/src/agent-wrappers-gemini.ts index 830dbccb770..e9041f0f549 100644 --- a/packages/agent-setup/src/agent-wrappers-gemini.ts +++ b/packages/agent-setup/src/agent-wrappers-gemini.ts @@ -19,7 +19,7 @@ import { getHooksDir } from "./paths"; export const GEMINI_HOOK_SCRIPT_NAME = "gemini-hook.sh"; const GEMINI_HOOK_SIGNATURE = "# Superset gemini hook"; -const GEMINI_HOOK_VERSION = "v5"; +const GEMINI_HOOK_VERSION = "v6"; export const GEMINI_HOOK_MARKER = `${GEMINI_HOOK_SIGNATURE} ${GEMINI_HOOK_VERSION}`; interface GeminiHookDefinition { diff --git a/packages/agent-setup/src/agent-wrappers.test.ts b/packages/agent-setup/src/agent-wrappers.test.ts index d4740a0b6f0..dbed78030a0 100644 --- a/packages/agent-setup/src/agent-wrappers.test.ts +++ b/packages/agent-setup/src/agent-wrappers.test.ts @@ -25,7 +25,7 @@ let mockedHomeDir = path.join(TEST_ROOT, "home"); mock.module("./notify-hook", () => ({ NOTIFY_SCRIPT_NAME: "notify.sh", - NOTIFY_SCRIPT_MARKER: "# Superset agent notification hook v8", + NOTIFY_SCRIPT_MARKER: "# Superset agent notification hook v9", getNotifyScriptPath: () => path.join(TEST_HOOKS_DIR, "notify.sh"), getNotifyScriptContent: () => "#!/bin/bash\nexit 0\n", createNotifyScript: () => {}, @@ -762,9 +762,9 @@ exit 0 }); it("bumps hook script markers when hook semantics change", () => { - expect(COPILOT_HOOK_MARKER).toBe("# Superset copilot hook v4"); - expect(CURSOR_HOOK_MARKER).toBe("# Superset cursor hook v6"); - expect(GEMINI_HOOK_MARKER).toBe("# Superset gemini hook v5"); + expect(COPILOT_HOOK_MARKER).toBe("# Superset copilot hook v5"); + expect(CURSOR_HOOK_MARKER).toBe("# Superset cursor hook v7"); + expect(GEMINI_HOOK_MARKER).toBe("# Superset gemini hook v6"); }); it("replaces stale Mastra hook commands from old superset paths", () => { diff --git a/packages/agent-setup/src/notify-hook.test.ts b/packages/agent-setup/src/notify-hook.test.ts index 25a39d6a1c6..f2a14403fb3 100644 --- a/packages/agent-setup/src/notify-hook.test.ts +++ b/packages/agent-setup/src/notify-hook.test.ts @@ -1,5 +1,7 @@ import { describe, expect, it } from "bun:test"; -import { readFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; import { getTemplatePath } from "./config"; import { NOTIFY_SCRIPT_MARKER } from "./notify-hook"; @@ -7,31 +9,102 @@ function readNotifyHookTemplate(): string { return readFileSync(getTemplatePath("notify-hook.template.sh"), "utf-8"); } +// The script scans ${SUPERSET_HOME_DIR:-$HOME/.superset}/host/*/manifest.json +// at call time. Tests must never resolve the developer's real manifests (this +// test process can itself run inside a Superset terminal), so the default env +// points at an empty home. +const emptyHome = mkdtempSync(path.join(tmpdir(), "notify-hook-empty-home-")); + +function renderNotifyHookScript(): string { + return readNotifyHookTemplate() + .replaceAll("{{MARKER}}", NOTIFY_SCRIPT_MARKER) + .replaceAll("{{DEFAULT_PORT}}", "48763"); +} + +function hookEnv(envOverrides: Record) { + return { + ...process.env, + SUPERSET_AGENT_ID: "grok", + SUPERSET_DEBUG_HOOKS: "1", + SUPERSET_TERMINAL_ID: "terminal-test", + SUPERSET_HOME_DIR: emptyHome, + ...envOverrides, + }; +} + function runNotifyHook( input: Record, envOverrides: Record = {}, ) { - const script = readNotifyHookTemplate() - .replaceAll("{{MARKER}}", NOTIFY_SCRIPT_MARKER) - .replaceAll("{{DEFAULT_PORT}}", "48763"); return Bun.spawnSync({ - cmd: ["bash", "-c", script], - env: { - ...process.env, - SUPERSET_AGENT_ID: "grok", - SUPERSET_DEBUG_HOOKS: "1", - SUPERSET_TERMINAL_ID: "terminal-test", - ...envOverrides, - }, + cmd: ["bash", "-c", renderNotifyHookScript()], + env: hookEnv(envOverrides), + stdin: Buffer.from(JSON.stringify(input)), + stdout: "pipe", + stderr: "pipe", + }); +} + +/** + * Async variant for tests that stand up an in-process Bun.serve fake + * host-service: spawnSync would block the event loop and deadlock the + * hook's curl against that server. + */ +async function runNotifyHookAsync( + input: Record, + envOverrides: Record = {}, +) { + const proc = Bun.spawn({ + cmd: ["bash", "-c", renderNotifyHookScript()], + env: hookEnv(envOverrides), stdin: Buffer.from(JSON.stringify(input)), stdout: "pipe", stderr: "pipe", }); + const [exitCode, stderr] = await Promise.all([ + proc.exited, + new Response(proc.stderr).text(), + ]); + return { exitCode, stderr }; +} + +/** Fake host-service answering notifications.hook like the real router. */ +function fakeHostService(ignored: boolean) { + const requests: Array<{ json: { terminalId?: string } }> = []; + const server = Bun.serve({ + port: 0, + fetch: async (req) => { + requests.push((await req.json()) as (typeof requests)[number]); + return Response.json({ + result: { data: { json: { success: true, ignored } } }, + }); + }, + }); + return { + requests, + url: `http://127.0.0.1:${server.port}`, + stop: () => server.stop(true), + }; +} + +function writeHookManifest(home: string, orgId: string, endpoint: string) { + const dir = path.join(home, "host", orgId); + mkdirSync(dir, { recursive: true }); + writeFileSync( + path.join(dir, "manifest.json"), + JSON.stringify({ + pid: 1, + endpoint, + authToken: "test-token", + startedAt: 0, + organizationId: orgId, + }), + ); } describe("getNotifyScriptContent", () => { it("bumps the notify hook marker when hook semantics change", () => { - expect(NOTIFY_SCRIPT_MARKER).toBe("# Superset agent notification hook v8"); + expect(NOTIFY_SCRIPT_MARKER).toBe("# Superset agent notification hook v9"); }); it("ignores hooks fired inside a subagent (agent_id present)", () => { @@ -84,16 +157,25 @@ describe("getNotifyScriptContent", () => { const script = readNotifyHookTemplate(); expect(script).toContain( - 'curl -sX POST "$SUPERSET_HOST_AGENT_HOOK_URL" \\\n --connect-timeout 2 --max-time 5', + 'curl -sX POST "$HOOK_URL" \\\n --connect-timeout 2 --max-time 5', ); }); - it("falls back to the v1 Electron hook when v2 is unavailable", () => { + it("resolves the endpoint at call time from org manifests, not only the frozen env URL", () => { const script = readNotifyHookTemplate(); + expect(script).toContain("SUPERSET_HOME_DIR:-$HOME/.superset"); + expect(script).toContain("/host/*/manifest.json; do"); expect(script).toContain( - 'if [ -n "$SUPERSET_HOST_AGENT_HOOK_URL" ] && [ -n "$SUPERSET_TERMINAL_ID" ]; then', + 'HOOK_CANDIDATE_URLS="$SUPERSET_HOST_AGENT_HOOK_URL"', ); + expect(script).toContain("/trpc/notifications.hook"); + }); + + it("falls back to the v1 Electron hook when v2 is unavailable", () => { + const script = readNotifyHookTemplate(); + + expect(script).toContain('if [ -n "$SUPERSET_TERMINAL_ID" ]; then'); expect(script).toContain( '[ -z "$SUPERSET_TAB_ID" ] && [ -z "$SESSION_ID" ] && [ -z "$SUPERSET_TERMINAL_ID" ] && exit 0', ); @@ -195,10 +277,10 @@ describe("per-agent hook scripts dispatch to v2", () => { '[ -n "$SUPERSET_TERMINAL_ID" ] || [ -n "$SUPERSET_TAB_ID" ] || exit 0', ); expect(script).toContain(buildExpectedV2Payload(agentIdVar)); - expect(script).toContain('curl -sX POST "$SUPERSET_HOST_AGENT_HOOK_URL"'); - expect(script).toContain( - 'if [ -n "$SUPERSET_HOST_AGENT_HOOK_URL" ] && [ -n "$SUPERSET_TERMINAL_ID" ]; then', - ); + expect(script).toContain('curl -sX POST "$HOOK_URL"'); + expect(script).toContain("SUPERSET_HOME_DIR:-$HOME/.superset"); + expect(script).toContain("/host/*/manifest.json; do"); + expect(script).toContain('if [ -n "$SUPERSET_TERMINAL_ID" ]; then'); expect(script).toContain("/hook/complete"); expect(script).toContain('V1_EVENT_TYPE="$EVENT_TYPE"'); expect(script).toContain("eventType=$V1_EVENT_TYPE"); @@ -208,3 +290,73 @@ describe("per-agent hook scripts dispatch to v2", () => { }); } }); + +describe("call-time endpoint resolution (frozen-port healing)", () => { + const stopEvent = { hook_event_name: "Stop", session_id: "s1" }; + // A guaranteed-dead localhost URL, standing in for the port a previous + // host-service instance captured into the agent's env before restarting. + const deadUrl = "http://127.0.0.1:1/trpc/notifications.hook"; + + it("delivers via the org manifest when the env hook URL points at a dead port", async () => { + const live = fakeHostService(false); + const home = mkdtempSync(path.join(tmpdir(), "notify-hook-home-")); + writeHookManifest(home, "org-a", live.url); + try { + const result = await runNotifyHookAsync(stopEvent, { + SUPERSET_HOME_DIR: home, + SUPERSET_HOST_AGENT_HOOK_URL: deadUrl, + }); + + expect(result.exitCode).toBe(0); + expect(live.requests).toHaveLength(1); + expect(live.requests[0]?.json.terminalId).toBe("terminal-test"); + expect(result.stderr).toContain("status=000"); + expect(result.stderr).toContain(`status=200 url=${live.url}`); + } finally { + live.stop(); + } + }); + + it("keeps probing past a host that does not own the terminal (ignored:true)", async () => { + const wrongOrg = fakeHostService(true); + const owningOrg = fakeHostService(false); + const home = mkdtempSync(path.join(tmpdir(), "notify-hook-home-")); + // org-a sorts before org-b in the manifest glob, so the wrong host is + // probed first and must not terminate the dispatch. + writeHookManifest(home, "org-a", wrongOrg.url); + writeHookManifest(home, "org-b", owningOrg.url); + try { + const result = await runNotifyHookAsync(stopEvent, { + SUPERSET_HOME_DIR: home, + SUPERSET_HOST_AGENT_HOOK_URL: deadUrl, + }); + + expect(result.exitCode).toBe(0); + expect(wrongOrg.requests).toHaveLength(1); + expect(owningOrg.requests).toHaveLength(1); + } finally { + wrongOrg.stop(); + owningOrg.stop(); + } + }); + + it("uses the env URL fast path without probing manifests when it answers", async () => { + const envHost = fakeHostService(false); + const manifestHost = fakeHostService(false); + const home = mkdtempSync(path.join(tmpdir(), "notify-hook-home-")); + writeHookManifest(home, "org-a", manifestHost.url); + try { + const result = await runNotifyHookAsync(stopEvent, { + SUPERSET_HOME_DIR: home, + SUPERSET_HOST_AGENT_HOOK_URL: `${envHost.url}/trpc/notifications.hook`, + }); + + expect(result.exitCode).toBe(0); + expect(envHost.requests).toHaveLength(1); + expect(manifestHost.requests).toHaveLength(0); + } finally { + envHost.stop(); + manifestHost.stop(); + } + }); +}); diff --git a/packages/agent-setup/src/notify-hook.ts b/packages/agent-setup/src/notify-hook.ts index 57e3eda3533..715240077e6 100644 --- a/packages/agent-setup/src/notify-hook.ts +++ b/packages/agent-setup/src/notify-hook.ts @@ -5,7 +5,7 @@ import { getHooksDir } from "./paths"; import { writeFileIfChanged } from "./write-file-if-changed"; export const NOTIFY_SCRIPT_NAME = "notify.sh"; -export const NOTIFY_SCRIPT_MARKER = "# Superset agent notification hook v8"; +export const NOTIFY_SCRIPT_MARKER = "# Superset agent notification hook v9"; export function getNotifyScriptPath(): string { return path.join(getHooksDir(), NOTIFY_SCRIPT_NAME); diff --git a/packages/agent-setup/templates/copilot-hook.template.sh b/packages/agent-setup/templates/copilot-hook.template.sh index f0d33574afc..2ca8466d039 100644 --- a/packages/agent-setup/templates/copilot-hook.template.sh +++ b/packages/agent-setup/templates/copilot-hook.template.sh @@ -37,18 +37,44 @@ json_escape() { printf '%s' "$1" | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g' } -if [ -n "$SUPERSET_HOST_AGENT_HOOK_URL" ] && [ -n "$SUPERSET_TERMINAL_ID" ]; then +# Resolve the host-service endpoint at call time: the env URL is frozen at +# terminal creation and goes stale when the host-service restarts on a new +# port, while each org manifest is rewritten with the live endpoint on every +# start. Only the host that owns this terminal answers "ignored":false. +if [ -n "$SUPERSET_TERMINAL_ID" ]; then PAYLOAD="{\"json\":{\"terminalId\":\"$(json_escape "$SUPERSET_TERMINAL_ID")\",\"eventType\":\"$(json_escape "$EVENT_TYPE")\",\"agent\":{\"agentId\":\"$(json_escape "$SUPERSET_AGENT_ID")\",\"sessionId\":\"$(json_escape "$HOOK_SESSION_ID")\"}}}" - STATUS_CODE=$(curl -sX POST "$SUPERSET_HOST_AGENT_HOOK_URL" \ - --connect-timeout 2 --max-time 5 \ - -H "Content-Type: application/json" \ - -d "$PAYLOAD" \ - -o /dev/null -w "%{http_code}" 2>/dev/null) + HOOK_CANDIDATE_URLS="$SUPERSET_HOST_AGENT_HOOK_URL" + for MANIFEST_FILE in "${SUPERSET_HOME_DIR:-$HOME/.superset}"/host/*/manifest.json; do + [ -f "$MANIFEST_FILE" ] || continue + MANIFEST_ENDPOINT=$(grep -oE '"endpoint"[[:space:]]*:[[:space:]]*"[^"]*"' "$MANIFEST_FILE" | head -1 | grep -oE '"[^"]*"$' | tr -d '"') + [ -n "$MANIFEST_ENDPOINT" ] || continue + HOOK_CANDIDATE_URLS="$HOOK_CANDIDATE_URLS $MANIFEST_ENDPOINT/trpc/notifications.hook" + done - case "$STATUS_CODE" in - 2*) exit 0 ;; - esac + HOOK_DELIVERED_2XX="0" + SEEN_HOOK_URLS="" + for HOOK_URL in $HOOK_CANDIDATE_URLS; do + case " $SEEN_HOOK_URLS " in *" $HOOK_URL "*) continue ;; esac + SEEN_HOOK_URLS="$SEEN_HOOK_URLS $HOOK_URL" + + RESPONSE=$(curl -sX POST "$HOOK_URL" \ + --connect-timeout 2 --max-time 5 \ + -H "Content-Type: application/json" \ + -d "$PAYLOAD" \ + -w "|%{http_code}" 2>/dev/null) + STATUS_CODE="${RESPONSE##*|}" + BODY="${RESPONSE%|*}" + + case "$BODY" in + *'"ignored":false'*|*'"ignored": false'*) exit 0 ;; + esac + case "$STATUS_CODE" in + 2*) HOOK_DELIVERED_2XX="1" ;; + esac + done + + [ "$HOOK_DELIVERED_2XX" = "1" ] && exit 0 fi [ -z "$SUPERSET_TAB_ID" ] && [ -z "$SUPERSET_TERMINAL_ID" ] && exit 0 diff --git a/packages/agent-setup/templates/cursor-hook.template.sh b/packages/agent-setup/templates/cursor-hook.template.sh index bf7518d1b17..37c4faeb1a1 100644 --- a/packages/agent-setup/templates/cursor-hook.template.sh +++ b/packages/agent-setup/templates/cursor-hook.template.sh @@ -46,18 +46,44 @@ if [ -z "$AGENT_ID" ]; then fi fi -if [ -n "$SUPERSET_HOST_AGENT_HOOK_URL" ] && [ -n "$SUPERSET_TERMINAL_ID" ]; then +# Resolve the host-service endpoint at call time: the env URL is frozen at +# terminal creation and goes stale when the host-service restarts on a new +# port, while each org manifest is rewritten with the live endpoint on every +# start. Only the host that owns this terminal answers "ignored":false. +if [ -n "$SUPERSET_TERMINAL_ID" ]; then PAYLOAD="{\"json\":{\"terminalId\":\"$(json_escape "$SUPERSET_TERMINAL_ID")\",\"eventType\":\"$(json_escape "$EVENT_TYPE")\",\"agent\":{\"agentId\":\"$(json_escape "$AGENT_ID")\",\"sessionId\":\"$(json_escape "$HOOK_SESSION_ID")\"}}}" - STATUS_CODE=$(curl -sX POST "$SUPERSET_HOST_AGENT_HOOK_URL" \ - --connect-timeout 2 --max-time 5 \ - -H "Content-Type: application/json" \ - -d "$PAYLOAD" \ - -o /dev/null -w "%{http_code}" 2>/dev/null) + HOOK_CANDIDATE_URLS="$SUPERSET_HOST_AGENT_HOOK_URL" + for MANIFEST_FILE in "${SUPERSET_HOME_DIR:-$HOME/.superset}"/host/*/manifest.json; do + [ -f "$MANIFEST_FILE" ] || continue + MANIFEST_ENDPOINT=$(grep -oE '"endpoint"[[:space:]]*:[[:space:]]*"[^"]*"' "$MANIFEST_FILE" | head -1 | grep -oE '"[^"]*"$' | tr -d '"') + [ -n "$MANIFEST_ENDPOINT" ] || continue + HOOK_CANDIDATE_URLS="$HOOK_CANDIDATE_URLS $MANIFEST_ENDPOINT/trpc/notifications.hook" + done - case "$STATUS_CODE" in - 2*) exit 0 ;; - esac + HOOK_DELIVERED_2XX="0" + SEEN_HOOK_URLS="" + for HOOK_URL in $HOOK_CANDIDATE_URLS; do + case " $SEEN_HOOK_URLS " in *" $HOOK_URL "*) continue ;; esac + SEEN_HOOK_URLS="$SEEN_HOOK_URLS $HOOK_URL" + + RESPONSE=$(curl -sX POST "$HOOK_URL" \ + --connect-timeout 2 --max-time 5 \ + -H "Content-Type: application/json" \ + -d "$PAYLOAD" \ + -w "|%{http_code}" 2>/dev/null) + STATUS_CODE="${RESPONSE##*|}" + BODY="${RESPONSE%|*}" + + case "$BODY" in + *'"ignored":false'*|*'"ignored": false'*) exit 0 ;; + esac + case "$STATUS_CODE" in + 2*) HOOK_DELIVERED_2XX="1" ;; + esac + done + + [ "$HOOK_DELIVERED_2XX" = "1" ] && exit 0 fi [ -z "$SUPERSET_TAB_ID" ] && [ -z "$SUPERSET_TERMINAL_ID" ] && exit 0 diff --git a/packages/agent-setup/templates/gemini-hook.template.sh b/packages/agent-setup/templates/gemini-hook.template.sh index f77c2d36f02..e7c1f8cb0d1 100644 --- a/packages/agent-setup/templates/gemini-hook.template.sh +++ b/packages/agent-setup/templates/gemini-hook.template.sh @@ -35,19 +35,46 @@ json_escape() { printf '%s' "$1" | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g' } -if [ -n "$SUPERSET_HOST_AGENT_HOOK_URL" ] && [ -n "$SUPERSET_TERMINAL_ID" ]; then +# Resolve the host-service endpoint at call time: the env URL is frozen at +# terminal creation and goes stale when the host-service restarts on a new +# port, while each org manifest is rewritten with the live endpoint on every +# start. Only the host that owns this terminal answers "ignored":false. +if [ -n "$SUPERSET_TERMINAL_ID" ]; then PAYLOAD="{\"json\":{\"terminalId\":\"$(json_escape "$SUPERSET_TERMINAL_ID")\",\"eventType\":\"$(json_escape "$EVENT_TYPE")\",\"agent\":{\"agentId\":\"$(json_escape "$SUPERSET_AGENT_ID")\",\"sessionId\":\"$(json_escape "$HOOK_SESSION_ID")\"}}}" - STATUS_CODE=$(curl -sX POST "$SUPERSET_HOST_AGENT_HOOK_URL" \ - --connect-timeout 2 --max-time 5 \ - -H "Content-Type: application/json" \ - -d "$PAYLOAD" \ - -o /dev/null -w "%{http_code}" 2>/dev/null) + HOOK_CANDIDATE_URLS="$SUPERSET_HOST_AGENT_HOOK_URL" + for MANIFEST_FILE in "${SUPERSET_HOME_DIR:-$HOME/.superset}"/host/*/manifest.json; do + [ -f "$MANIFEST_FILE" ] || continue + MANIFEST_ENDPOINT=$(grep -oE '"endpoint"[[:space:]]*:[[:space:]]*"[^"]*"' "$MANIFEST_FILE" | head -1 | grep -oE '"[^"]*"$' | tr -d '"') + [ -n "$MANIFEST_ENDPOINT" ] || continue + HOOK_CANDIDATE_URLS="$HOOK_CANDIDATE_URLS $MANIFEST_ENDPOINT/trpc/notifications.hook" + done - case "$STATUS_CODE" in - 2*) exit 0 ;; - *) echo "[gemini-hook] host-service dispatch failed status=$STATUS_CODE; falling back to v1" >&2 ;; - esac + HOOK_DELIVERED_2XX="0" + SEEN_HOOK_URLS="" + for HOOK_URL in $HOOK_CANDIDATE_URLS; do + case " $SEEN_HOOK_URLS " in *" $HOOK_URL "*) continue ;; esac + SEEN_HOOK_URLS="$SEEN_HOOK_URLS $HOOK_URL" + + RESPONSE=$(curl -sX POST "$HOOK_URL" \ + --connect-timeout 2 --max-time 5 \ + -H "Content-Type: application/json" \ + -d "$PAYLOAD" \ + -w "|%{http_code}" 2>/dev/null) + STATUS_CODE="${RESPONSE##*|}" + BODY="${RESPONSE%|*}" + + case "$BODY" in + *'"ignored":false'*|*'"ignored": false'*) exit 0 ;; + esac + case "$STATUS_CODE" in + 2*) HOOK_DELIVERED_2XX="1" ;; + *) echo "[gemini-hook] host-service dispatch failed status=$STATUS_CODE url=$HOOK_URL" >&2 ;; + esac + done + + [ "$HOOK_DELIVERED_2XX" = "1" ] && exit 0 + echo "[gemini-hook] no host-service accepted the event; falling back to v1" >&2 fi [ -z "$SUPERSET_TAB_ID" ] && [ -z "$SUPERSET_TERMINAL_ID" ] && exit 0 diff --git a/packages/agent-setup/templates/notify-hook.template.sh b/packages/agent-setup/templates/notify-hook.template.sh index c0959700041..b8bd5fabf8c 100644 --- a/packages/agent-setup/templates/notify-hook.template.sh +++ b/packages/agent-setup/templates/notify-hook.template.sh @@ -112,23 +112,56 @@ json_escape() { printf '%s' "$1" | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g' } -if [ -n "$SUPERSET_HOST_AGENT_HOOK_URL" ] && [ -n "$SUPERSET_TERMINAL_ID" ]; then +# Resolve the host-service endpoint at call time. SUPERSET_HOST_AGENT_HOOK_URL +# is frozen into the agent's env at terminal creation; after a host-service +# restart on a new port it would point at a dead socket forever (a live +# process's env can't change). Each org's manifest +# (~/.superset/host//manifest.json) is rewritten with the live endpoint +# on every start, so it never goes stale. Try the env URL first (fast path), +# then every org manifest's endpoint. Only the host that owns this terminal +# answers "ignored":false; probing the other orgs' hosts is a harmless no-op. +if [ -n "$SUPERSET_TERMINAL_ID" ]; then PAYLOAD="{\"json\":{\"terminalId\":\"$(json_escape "$SUPERSET_TERMINAL_ID")\",\"eventType\":\"$(json_escape "$EVENT_TYPE")\",\"agent\":{\"agentId\":\"$(json_escape "$SUPERSET_AGENT_ID")\",\"sessionId\":\"$(json_escape "$SESSION_ID")\"}}}" - STATUS_CODE=$(curl -sX POST "$SUPERSET_HOST_AGENT_HOOK_URL" \ - --connect-timeout 2 --max-time 5 \ - -H "Content-Type: application/json" \ - -d "$PAYLOAD" \ - -o /dev/null -w "%{http_code}" 2>/dev/null) - - if [ "$DEBUG_HOOKS_ENABLED" = "1" ]; then - echo "[notify-hook] host-service dispatched status=$STATUS_CODE" >&2 - fi - debug_log "host-service status=$STATUS_CODE url=$SUPERSET_HOST_AGENT_HOOK_URL" - - case "$STATUS_CODE" in - 2*) exit 0 ;; - esac + HOOK_CANDIDATE_URLS="$SUPERSET_HOST_AGENT_HOOK_URL" + for MANIFEST_FILE in "${SUPERSET_HOME_DIR:-$HOME/.superset}"/host/*/manifest.json; do + [ -f "$MANIFEST_FILE" ] || continue + MANIFEST_ENDPOINT=$(grep -oE '"endpoint"[[:space:]]*:[[:space:]]*"[^"]*"' "$MANIFEST_FILE" | head -1 | grep -oE '"[^"]*"$' | tr -d '"') + [ -n "$MANIFEST_ENDPOINT" ] || continue + HOOK_CANDIDATE_URLS="$HOOK_CANDIDATE_URLS $MANIFEST_ENDPOINT/trpc/notifications.hook" + done + + HOOK_DELIVERED_2XX="0" + SEEN_HOOK_URLS="" + for HOOK_URL in $HOOK_CANDIDATE_URLS; do + case " $SEEN_HOOK_URLS " in *" $HOOK_URL "*) continue ;; esac + SEEN_HOOK_URLS="$SEEN_HOOK_URLS $HOOK_URL" + + RESPONSE=$(curl -sX POST "$HOOK_URL" \ + --connect-timeout 2 --max-time 5 \ + -H "Content-Type: application/json" \ + -d "$PAYLOAD" \ + -w "|%{http_code}" 2>/dev/null) + STATUS_CODE="${RESPONSE##*|}" + BODY="${RESPONSE%|*}" + + if [ "$DEBUG_HOOKS_ENABLED" = "1" ]; then + echo "[notify-hook] host-service dispatched status=$STATUS_CODE url=$HOOK_URL" >&2 + fi + debug_log "host-service status=$STATUS_CODE url=$HOOK_URL" + + # "ignored":false means the owning host accepted and fanned out the event. + case "$BODY" in + *'"ignored":false'*|*'"ignored": false'*) exit 0 ;; + esac + case "$STATUS_CODE" in + 2*) HOOK_DELIVERED_2XX="1" ;; + esac + done + + # Delivered somewhere (2xx) but no host owned the terminal: keep the + # pre-existing "any 2xx wins" behavior and skip the v1 fallback. + [ "$HOOK_DELIVERED_2XX" = "1" ] && exit 0 fi # v1 fallback: Electron localhost hook server. Kept while v1 terminals exist. diff --git a/plans/agent-hook-url-frozen-port.md b/plans/agent-hook-url-frozen-port.md new file mode 100644 index 00000000000..06d284735c9 --- /dev/null +++ b/plans/agent-hook-url-frozen-port.md @@ -0,0 +1,142 @@ +# Agent hook URL frozen-port bug (Maxime Auburtin, 2026-08-13) + +Status: root cause confirmed in code; fix implemented (call-time endpoint resolution in the hook scripts). See "Fix" below. + +Note: PR #6479 (merged 2026-08-15) moved agent-setup from apps/desktop into packages/agent-setup and provisions hooks on headless CLI hosts; this fix was rebased onto that layout, and file references below use the new paths. The pre-move analysis references (terminal.ts, spawn.ts, coordinator) are unchanged. + +## Symptom + +`SUPERSET_HOST_AGENT_HOOK_URL` is injected into each terminal's env at creation, carrying the host-service port of that moment. When the host service restarts on a new ephemeral port, every already-running agent POSTs its lifecycle events to a dead port forever. Status silently stops for old agents while new agents work, splitting the fleet. + +## Root-cause map (his observations -> code) + +| Observation | Code | +| --- | --- | +| Hook URL carries a port captured at terminal creation | `packages/host-service/src/terminal/terminal.ts:165-169` `getHostAgentHookUrl()` reads `process.env.HOST_SERVICE_PORT \|\| PORT` (fixed for the life of the host-service process) and formats `http://127.0.0.1:/trpc/notifications.hook` | +| URL frozen into the agent process env | `terminal.ts:2408-2423` passes it to `buildV2TerminalEnv`; `packages/host-service/src/terminal/env.ts:246-248` sets `env.SUPERSET_HOST_AGENT_HOOK_URL`; the env dict goes to `daemon.open(...)` (`terminal.ts:2461-2468`) and becomes the PTY's immutable process env | +| Terminals survive the restart with the old env | PTYs live in the separate pty-daemon process; a restarted host-service *adopts* live sessions (`terminal.ts:2443-2458`) without touching their env | +| Port differs every restart | Only on the CLI path: `packages/cli/src/lib/host/spawn.ts:112` `options.port ?? findFreePort()` picks a random ephemeral port per start. The desktop coordinator already pins: `getPreferredPorts` tries the in-memory last port, then a deterministic per-org stable port `getStablePortForOrganization` (FNV-1a hash into 48000-48999, `host-service-coordinator.ts:124-131,251-267`), so app restarts normally reuse the same port | +| notify.sh POSTs to the frozen URL | `packages/agent-setup/templates/notify-hook.template.sh:115-131` (curl to `$SUPERSET_HOST_AGENT_HOOK_URL`, exits 0 on any 2xx). Same frozen-URL block in `cursor-hook.template.sh:49-61`, `copilot-hook.template.sh:40-52`, `gemini-hook.template.sh:38-51`. pi (`pi-extension.template.ts`) and amp (`amp-plugin.template.ts`) spawn notify.sh with the agent env, so they inherit the same frozen value | +| Failure is silent | notify.sh swallows curl status unless `SUPERSET_DEBUG_HOOKS=1` (template lines 124-127); the endpoint itself is fine (`packages/host-service/src/trpc/router/notifications/notifications.ts:86-134`, unauthenticated by design) | +| Manifest is fresh the whole time | rewritten on every start by both starters: CLI `spawn.ts:168-175`, desktop child `apps/desktop/src/main/host-service/index.ts:126-138` (`~/.superset/host//manifest.json` with `endpoint` + `authToken`) | + +His claim is correct as stated with one nuance: the URL is not "constructed per terminal from a stale source", it is constructed correctly at terminal creation from the *current* host-service port; the freeze happens because process env is immutable and the port is ephemeral per host-service instance. + +His three measured ports decode cleanly against this: 48396 is inside the desktop's stable-port range (48000-48999), so agent 1 (Aug 12 21:11) was created under an app-started service on the org's pinned port. 53080 and 54757 are ordinary ephemeral ports from the CLI's `findFreePort()`: the 10:29:50 `superset start --daemon` bound 53080 (agent 2, 10:46), a later restart bound 54757 (agent 3, 12:38). Two CLI restarts, three port lineages. This also explains why desktop-only users rarely hit the bug: the coordinator's port pinning (his suggestion 3) already exists on the desktop path but not in the CLI. + +## Secondary findings + +### `SUPERSET_AGENT_HOOK_PORT` empty in 2 of 3 agents + +- It is the **v1 Electron notifications port** (desktop default 51741, `apps/desktop/src/shared/env.shared.ts:21`). +- Desktop-spawned host-service sets it (`host-service-coordinator.ts:806`); the **CLI spawn does not set it at all** (`packages/cli/src/lib/host/spawn.ts:133-147`), and `terminal.ts:2420` falls back to `""`. +- Maxime's oldest agent (Aug 12 21:11, port 51741) was created under an app-started host-service; the two newer ones were created under the CLI-daemon-started service (10:29:50 restart) so they got "". +- Impact: none in v2. Nothing consumes `SUPERSET_AGENT_HOOK_PORT` in the hook path (notify.sh's v1 fallback uses `SUPERSET_PORT` / baked-in default). Cosmetic/confusing only; not part of this fix. + +### `superset start --daemon` logs to /dev/null + +- True in 1.20.2: `spawn.ts` used `stdio: "ignore"` for daemon mode. +- Already fixed on main by #6417 (`e266b01ad`, merged 2026-08-13): daemon stdout/stderr now go to the same per-org `~/.superset/host//host-service.log` the desktop writes (rotating, `spawn.ts:117-131`). Not yet in any release; ships with the next CLI/desktop release. + +### Relay tunnel proxying a stale port (his "same class" note) + +Out of scope here, but he is right that it is the same pattern: the relay connection captures `localPort` at `connectRelay(...)` time (`apps/desktop/src/main/host-service/index.ts:140-149`). It dies with its own process on restart though, so it self-heals; his ECONNREFUSED window was the 87s gap between app-service shutdown and CLI-service start. No code change needed for this bug. + +## Repro evidence + +### Layer 1: live agents on this dev machine (read-only, 2026-08-13) + +This machine runs the same topology as Maxime's. Long-running agents from Aug 2-12 all carry `SUPERSET_HOST_AGENT_HOOK_URL=http://127.0.0.1:48703/...` in their env (`ps eww`), and 48703 is still the live port only because the desktop coordinator pins the per-org stable port across restarts (`manifest.json` startedAt = today, agents up to 11 days older). The freeze mechanism (env captured at creation, never re-resolved) is directly observable; the desktop's port pinning is what masks it until a CLI start breaks the lineage. + +### Layer 2: controlled restart (script, run 2026-08-13) + +Temp script (`packages/host-service/repro-frozen-hook.ts`, not committed) boots the real host-service `createApp` on a real TCP port twice against the same host.db, builds the terminal env with the real `buildV2TerminalEnv`, and replays the real rendered `notify-hook.template.sh` (SUPERSET_DEBUG_HOOKS=1, v1 fallback pointed at a dead port): + +``` +[instance 1] host-service listening on port 62970 +[terminal] frozen SUPERSET_HOST_AGENT_HOOK_URL=http://127.0.0.1:62970/trpc/notifications.hook + +--- while instance 1 is alive (healthy baseline) --- +[notify-hook] host-service dispatched status=200 <- works + +[instance 2] host-service RESTARTED on port 62972 (manifest rewritten) + +--- old terminal (frozen env, port 62970) after restart --- +[notify-hook] host-service dispatched status=000 <- dead port +[notify-hook] v1 dispatched status=000 <- silently dropped, exit 0 + +--- new terminal (fresh env, port 62972) after restart --- +[notify-hook] host-service dispatched status=200 <- new agents work +``` + +Exactly Maxime's table: old terminals dead, new terminals fine, zero surfaced errors. + +### After the fix (same script, fixed template) + +``` +--- old terminal (frozen env, port 65310) after restart --- +[notify-hook] host-service dispatched status=000 url=http://127.0.0.1:65310/trpc/notifications.hook +[notify-hook] host-service dispatched status=200 url=http://127.0.0.1:65314/trpc/notifications.hook <- healed via manifest +``` + +The frozen-env terminal falls through the dead env URL and delivers via the manifest endpoint. Because the hook is re-executed from disk on every event, shipping the updated script heals every already-running agent on its next hook fire; no terminal restart needed. + +## Fix decision + +Chosen: **reporter's suggestion 1**, resolve the endpoint at call time from `~/.superset/host//manifest.json`, implemented inside the hook scripts, keeping `SUPERSET_HOST_AGENT_HOOK_URL` as a fallback. + +Why this beats the alternatives: + +- **Stable local socket**: correct long-term but a large change (new listener in host-service, curl `--unix-socket` support assumptions, relay/desktop coordination, migration of all four templates plus adoption). Does not heal existing agents any faster than the manifest read, and is far riskier. +- **Port pinning**: reduces but does not eliminate the failure (pinned port can be taken; first bind after upgrade still moves), keeps the architectural flaw, and still leaves a window where old agents point at a dead port. +- **Manifest read at call time** heals **already-running agents on the next hook fire**: the hook is a script re-read from disk on every event, so updating the script on disk fixes live agents without touching their env. Multi-org is handled by probing every org manifest and using the endpoint's own discriminator: `notifications.hook` returns `"ignored":false` only when the terminal id is known to that host (`notifications.ts:96-104`; wrong org / unknown terminal returns `"ignored":true`). The endpoint is unauthenticated by design so no authToken is needed. + +### Implementation + +All four shell templates get the same dispatch change (repo idiom is to duplicate this block per template): + +1. Candidate URLs = `$SUPERSET_HOST_AGENT_HOOK_URL` (fast path; almost always alive and correct) followed by `/trpc/notifications.hook` for each `${SUPERSET_HOME_DIR:-$HOME/.superset}/host/*/manifest.json`, deduped. +2. POST to each candidate in order. Body `"ignored":false` = delivered to the owning host: exit 0. +3. Any 2xx (delivered-but-ignored) still short-circuits the v1 fallback, preserving today's semantics; no 2xx at all falls through to the v1 Electron fallback as before. +4. Connection-refused candidates fail in milliseconds on localhost (`--connect-timeout 2` is an upper bound), so the healthy-path cost stays one curl. + +Version markers bumped (content change alone re-writes the file via `writeFileIfChanged`, but markers are the semantic version): notify v8 -> v9, cursor v6 -> v7, copilot v4 -> v5, gemini v5 -> v6. + +Files changed: + +- `packages/agent-setup/templates/notify-hook.template.sh` +- `packages/agent-setup/templates/cursor-hook.template.sh` +- `packages/agent-setup/templates/copilot-hook.template.sh` +- `packages/agent-setup/templates/gemini-hook.template.sh` +- `packages/agent-setup/src/notify-hook.ts` (marker bump v8 -> v9) +- `packages/agent-setup/src/agent-wrappers-{cursor,copilot,gemini}.ts` (marker bumps v6->v7, v4->v5, v5->v6) +- `packages/agent-setup/src/notify-hook.test.ts` (behavioral tests against a fake host-service over a real port: manifest failover when the env URL is dead, wrong-org skip via ignored:true, env-URL fast path leaves manifests unprobed; plus updated template-content assertions. Tests default `SUPERSET_HOME_DIR` to an empty temp dir so they never probe the developer's real manifests) +- `packages/agent-setup/src/agent-wrappers.test.ts` (marker assertions) + +Mutation check (tests can fail): broke the manifest glob (`/host/` -> `/host-broken/`) and reran; the 3 resolution tests failed (manifest failover, wrong-org probe, template assertion) while the 17 others passed. Restored; all 20 pass, and the full agent-setup suite (113 tests) passes. + +Deliberately NOT changed: + +- `SUPERSET_HOST_AGENT_HOOK_URL` injection stays (older scripts on disk still need it; it is also the fast path). +- CLI `spawn.ts` `SUPERSET_AGENT_HOOK_PORT` gap: cosmetic, nothing consumes it in v2; noted for a separate cleanup. +- Headless hook provisioning shipped independently in #6479 (host-service provisions `~/.superset/hooks` on CLI hosts), so this fix now reaches pure-headless installs too. +- Suggestion 4 (fail loudly in UI) is a good follow-up but is not needed once the address can no longer go stale; filing separately keeps this change reviewable. + +## Validation + +- `bun test src/main/lib/agent-setup/` (apps/desktop): 113 pass, 0 fail +- Mutation check: broken manifest glob makes 3 tests fail; restored, all pass +- `bun run typecheck`: exit 0 +- `bun run lint`: exit 0 (fixed 2 Biome noTemplateCurlyInString warnings and 1 format error the new tests introduced) +- `bash -n` on all four rendered templates: syntax OK +- End-to-end script repro rerun with the fixed template: frozen-env terminal delivers via manifest (transcript above) + +## Follow-ups (separate tickets) + +- Headless-UI-blind issue (report 3): explicitly out of scope per task. +- Consider removing `SUPERSET_AGENT_HOOK_PORT` injection or setting it from the CLI for consistency. +- Suggestion 4: surface one-shot "agent status unavailable" UI on repeated hook connection failures (needs a reporting channel; moot for the frozen-port case after this fix). + +## Repro transcript + +(filled in below by the repro run) From 330629fc78b1cbc4f8ccabe3e73dbd2bb6c1665f Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Sat, 15 Aug 2026 12:54:07 -0700 Subject: [PATCH 2/2] docs: tag repro transcript code fences as text --- plans/agent-hook-url-frozen-port.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plans/agent-hook-url-frozen-port.md b/plans/agent-hook-url-frozen-port.md index 06d284735c9..b6b3930430e 100644 --- a/plans/agent-hook-url-frozen-port.md +++ b/plans/agent-hook-url-frozen-port.md @@ -52,7 +52,7 @@ This machine runs the same topology as Maxime's. Long-running agents from Aug 2- Temp script (`packages/host-service/repro-frozen-hook.ts`, not committed) boots the real host-service `createApp` on a real TCP port twice against the same host.db, builds the terminal env with the real `buildV2TerminalEnv`, and replays the real rendered `notify-hook.template.sh` (SUPERSET_DEBUG_HOOKS=1, v1 fallback pointed at a dead port): -``` +```text [instance 1] host-service listening on port 62970 [terminal] frozen SUPERSET_HOST_AGENT_HOOK_URL=http://127.0.0.1:62970/trpc/notifications.hook @@ -73,7 +73,7 @@ Exactly Maxime's table: old terminals dead, new terminals fine, zero surfaced er ### After the fix (same script, fixed template) -``` +```text --- old terminal (frozen env, port 65310) after restart --- [notify-hook] host-service dispatched status=000 url=http://127.0.0.1:65310/trpc/notifications.hook [notify-hook] host-service dispatched status=200 url=http://127.0.0.1:65314/trpc/notifications.hook <- healed via manifest