diff --git a/Resources/agent-hooks/omp/prowl-hooks.ts b/Resources/agent-hooks/omp/prowl-hooks.ts index c8cdb31d..74dfce8b 100644 --- a/Resources/agent-hooks/omp/prowl-hooks.ts +++ b/Resources/agent-hooks/omp/prowl-hooks.ts @@ -43,13 +43,16 @@ function parentSessionId(ctx: any): string | undefined { return undefined; } -// Deliveries are serialized per extension instance: adjacent lifecycle events (Pi's -// `agent_settled` and `session_shutdown` are milliseconds apart at exit) would otherwise race -// as independent processes and could reach Prowl out of order, where a late session start -// clears the terminal evidence a wait relies on. The runtime callback never waits on the queue, -// and a bridge that hangs is killed after a bound so later events keep flowing. +// Deliveries are serialized process-wide: adjacent lifecycle events (Pi's `agent_settled` and +// `session_shutdown` are milliseconds apart at exit) would otherwise race as independent +// processes and could reach Prowl out of order, where a late session start clears the terminal +// evidence a wait relies on. The queue lives on `globalThis` because the runtime loads a fresh +// module instance on `/reload` and for every sub-agent session (measured), and those instances +// must share one order. The runtime callback never waits on the queue, and a bridge that hangs +// is killed after a bound so later events keep flowing. const DELIVERY_TIMEOUT_MS = 5000; -let deliveries: Promise = Promise.resolve(); +const QUEUE_KEY = "__prowlHookDeliveries"; +const shared = globalThis as unknown as Record | undefined>; function deliver(name: string, payload: Record): Promise { return new Promise((resolve) => { @@ -84,12 +87,18 @@ function deliver(name: string, payload: Record): Promise } function enqueue(name: string, payload: Record): void { - deliveries = deliveries.then(() => deliver(name, payload)).catch(() => {}); + const previous = shared[QUEUE_KEY] ?? Promise.resolve(); + shared[QUEUE_KEY] = previous.then(() => deliver(name, payload)).catch(() => {}); } function relay(name: string, event: any, ctx: any): void { try { if (!process.env[TOKEN_VARIABLE]) return; + // `/reload` swaps the extension instances: the old one reports `session_shutdown` and the + // new one `session_start`, both with reason "reload" and the same session id. The session + // neither ends nor changes, so neither is forwarded — a late shutdown would otherwise read + // as the live session ending. + if (event?.reason === "reload") return; let sessionId = ctx?.sessionManager?.getSessionId?.(); if (typeof sessionId !== "string" || sessionId.length === 0) return; const parent = parentSessionId(ctx); diff --git a/Resources/agent-hooks/opencode/prowl-hooks.ts b/Resources/agent-hooks/opencode/prowl-hooks.ts index 797dbabb..f6903073 100644 --- a/Resources/agent-hooks/opencode/prowl-hooks.ts +++ b/Resources/agent-hooks/opencode/prowl-hooks.ts @@ -15,13 +15,16 @@ const FORWARDED_EVENTS = new Set(["session.idle", "permission.asked", "question. const TOKEN_VARIABLE = "PROWL_AGENT_HOOK_TOKEN"; const CLI = join(dirname(fileURLToPath(import.meta.url)), "..", "..", "prowl-cli", "prowl"); -// Deliveries are serialized per extension instance: adjacent lifecycle events (Pi's -// `agent_settled` and `session_shutdown` are milliseconds apart at exit) would otherwise race -// as independent processes and could reach Prowl out of order, where a late session start -// clears the terminal evidence a wait relies on. The runtime callback never waits on the queue, -// and a bridge that hangs is killed after a bound so later events keep flowing. +// Deliveries are serialized process-wide: adjacent lifecycle events (Pi's `agent_settled` and +// `session_shutdown` are milliseconds apart at exit) would otherwise race as independent +// processes and could reach Prowl out of order, where a late session start clears the terminal +// evidence a wait relies on. The queue lives on `globalThis` because the runtime loads a fresh +// module instance on `/reload` and for every sub-agent session (measured), and those instances +// must share one order. The runtime callback never waits on the queue, and a bridge that hangs +// is killed after a bound so later events keep flowing. const DELIVERY_TIMEOUT_MS = 5000; -let deliveries: Promise = Promise.resolve(); +const QUEUE_KEY = "__prowlHookDeliveries"; +const shared = globalThis as unknown as Record | undefined>; function deliver(name: string, payload: Record): Promise { return new Promise((resolve) => { @@ -56,7 +59,8 @@ function deliver(name: string, payload: Record): Promise } function enqueue(name: string, payload: Record): void { - deliveries = deliveries.then(() => deliver(name, payload)).catch(() => {}); + const previous = shared[QUEUE_KEY] ?? Promise.resolve(); + shared[QUEUE_KEY] = previous.then(() => deliver(name, payload)).catch(() => {}); } export const ProwlHooks = async ({ directory }: any) => { diff --git a/Resources/agent-hooks/pi/prowl-hooks.ts b/Resources/agent-hooks/pi/prowl-hooks.ts index 7e2dfdbd..cca7e2fa 100644 --- a/Resources/agent-hooks/pi/prowl-hooks.ts +++ b/Resources/agent-hooks/pi/prowl-hooks.ts @@ -36,13 +36,16 @@ function parentSessionId(ctx: any): string | undefined { return undefined; } -// Deliveries are serialized per extension instance: adjacent lifecycle events (Pi's -// `agent_settled` and `session_shutdown` are milliseconds apart at exit) would otherwise race -// as independent processes and could reach Prowl out of order, where a late session start -// clears the terminal evidence a wait relies on. The runtime callback never waits on the queue, -// and a bridge that hangs is killed after a bound so later events keep flowing. +// Deliveries are serialized process-wide: adjacent lifecycle events (Pi's `agent_settled` and +// `session_shutdown` are milliseconds apart at exit) would otherwise race as independent +// processes and could reach Prowl out of order, where a late session start clears the terminal +// evidence a wait relies on. The queue lives on `globalThis` because the runtime loads a fresh +// module instance on `/reload` and for every sub-agent session (measured), and those instances +// must share one order. The runtime callback never waits on the queue, and a bridge that hangs +// is killed after a bound so later events keep flowing. const DELIVERY_TIMEOUT_MS = 5000; -let deliveries: Promise = Promise.resolve(); +const QUEUE_KEY = "__prowlHookDeliveries"; +const shared = globalThis as unknown as Record | undefined>; function deliver(name: string, payload: Record): Promise { return new Promise((resolve) => { @@ -77,12 +80,18 @@ function deliver(name: string, payload: Record): Promise } function enqueue(name: string, payload: Record): void { - deliveries = deliveries.then(() => deliver(name, payload)).catch(() => {}); + const previous = shared[QUEUE_KEY] ?? Promise.resolve(); + shared[QUEUE_KEY] = previous.then(() => deliver(name, payload)).catch(() => {}); } function relay(name: string, event: any, ctx: any): void { try { if (!process.env[TOKEN_VARIABLE]) return; + // `/reload` swaps the extension instances: the old one reports `session_shutdown` and the + // new one `session_start`, both with reason "reload" and the same session id. The session + // neither ends nor changes, so neither is forwarded — a late shutdown would otherwise read + // as the live session ending. + if (event?.reason === "reload") return; let sessionId = ctx?.sessionManager?.getSessionId?.(); if (typeof sessionId !== "string" || sessionId.length === 0) return; const parent = parentSessionId(ctx); diff --git a/docs-ai/064-agent-completion-signals/010-s3c-plan.md b/docs-ai/064-agent-completion-signals/010-s3c-plan.md index e2aafcda..25e0ba07 100644 --- a/docs-ai/064-agent-completion-signals/010-s3c-plan.md +++ b/docs-ai/064-agent-completion-signals/010-s3c-plan.md @@ -180,9 +180,12 @@ Sub-agent protection for OpenCode (two layers, both required — measured above) Fail-open rules for the extension code: every handler is wrapped, the spawn uses `stdio: ["pipe","ignore","ignore"]`, nothing is awaited on the runtime's path, no output is written to the runtime's UI, and any failure to spawn is swallowed. A hook that cannot reach -Prowl changes nothing for the agent. Deliveries are serialized per extension instance (the next -bridge process starts only after the previous one closed, with a 5 s kill bound) so adjacent -events keep their order the way a runtime running hook commands sequentially would. +Prowl changes nothing for the agent. Deliveries are serialized process-wide through a queue on +`globalThis` (the next bridge process starts only after the previous one closed, with a 5 s kill +bound) so adjacent events keep their order the way a runtime running hook commands sequentially +would — including across the fresh module instances Pi loads on `/reload` and the Pi family +loads per sub-agent session. Reload-reason `session_shutdown` / `session_start` are not +forwarded: the session continues under the same id. ## Docs and closure diff --git a/docs-ai/064-agent-completion-signals/011-s3c-action.md b/docs-ai/064-agent-completion-signals/011-s3c-action.md index e8d9143c..492726d0 100644 --- a/docs-ai/064-agent-completion-signals/011-s3c-action.md +++ b/docs-ai/064-agent-completion-signals/011-s3c-action.md @@ -131,6 +131,16 @@ approval reported `needs-input` on the pane's session and the wait resolved on t never waits on it, and a bridge that hangs is killed after 5 s so later events keep flowing. The Node harness fires every step back to back and asserts strict order, including a 36-event burst. `make test-scripts` now runs in CI alongside the other test tasks. +- A per-instance queue still raced across instances: Pi's `/reload` has the old instance report + `session_shutdown{reason:"reload"}` and, 4 ms later, a fresh instance report + `session_start{reason:"reload"}` for the same session id (measured), so a late shutdown could + become the live session's exact `session-end` and complete `agents wait --until exit` while + the runtime is alive. Two changes: reload-reason lifecycle events are not forwarded at all (the + session neither ends nor changes; OMP has no `/reload` — the text is treated as input), and the + delivery queue now lives on `globalThis`, which every module instance in the process shares + (measured), so sub-agent instances and reloads keep one order. The harness loads extra + instances through a cache-busting import and pins both the reload sequence and cross-instance + ordering; both tests fail against the previous relay. ### Display sleep is the CREATE_FAILED behind the "intermittent" Profile launches diff --git a/scripts/test_agent_hooks.py b/scripts/test_agent_hooks.py index 488db473..21f40838 100644 --- a/scripts/test_agent_hooks.py +++ b/scripts/test_agent_hooks.py @@ -49,12 +49,24 @@ PI_FAMILY_HARNESS = textwrap.dedent( } } const [extensionPath, scriptPath] = process.argv.slice(2); - const module = await import(pathToFileURL(extensionPath)); - const handlers = new Map(); - module.default({ on(name, handler) { handlers.set(name, handler); } }); const steps = JSON.parse(await import("node:fs").then((fs) => fs.promises.readFile(scriptPath, "utf8"))); + // A step may name an `instance`: the runtime loads a fresh module instance on `/reload` and + // for every sub-agent session, which a cache-busting query reproduces here. + const instances = new Map(); + async function handlersFor(instance) { + const key = instance ?? 0; + if (!instances.has(key)) { + const url = pathToFileURL(extensionPath); + url.searchParams.set("instance", String(key)); + const module = await import(url.href); + const handlers = new Map(); + module.default({ on(name, handler) { handlers.set(name, handler); } }); + instances.set(key, handlers); + } + return instances.get(key); + } for (const step of steps) { - const handler = handlers.get(step.event); + const handler = (await handlersFor(step.instance)).get(step.event); if (!handler) continue; const ctx = { hasUI: step.hasUI, @@ -228,6 +240,41 @@ class AgentHookExtensionTests(unittest.TestCase): [(step["event"], step["session"]) for step in steps], ) + def test_pi_reload_swaps_instances_without_ending_or_reannouncing_the_session(self): + # `/reload` fires `session_shutdown{reload}` on the old instance and, milliseconds later, + # `session_start{reload}` on a fresh instance with the same id; the session continues. + forwarded = self.pi_family( + "pi", + [ + {"event": "session_start", "hasUI": True, "session": "main-1", "file": self.MAIN_1, "instance": 1}, + {"event": "agent_settled", "hasUI": True, "session": "main-1", "file": self.MAIN_1, "instance": 1}, + {"event": "session_shutdown", "hasUI": True, "session": "main-1", "file": self.MAIN_1, "instance": 1, "payload": {"type": "session_shutdown", "reason": "reload"}}, + {"event": "session_start", "hasUI": True, "session": "main-1", "file": self.MAIN_1, "instance": 2, "payload": {"type": "session_start", "reason": "reload"}}, + {"event": "agent_settled", "hasUI": True, "session": "main-1", "file": self.MAIN_1, "instance": 2}, + {"event": "session_shutdown", "hasUI": True, "session": "main-1", "file": self.MAIN_1, "instance": 2, "payload": {"type": "session_shutdown", "reason": "quit"}}, + ], + ) + self.assertEqual( + [(payload["hook_event_name"], payload["session_id"], payload.get("reason")) for _, payload in forwarded], + [("session_start", "main-1", None), ("agent_settled", "main-1", None), ("agent_settled", "main-1", None), ("session_shutdown", "main-1", "quit")], + ) + + def test_deliveries_stay_ordered_across_extension_instances(self): + # Instances loaded for sub-agent sessions (and by `/reload`) share one delivery order. + steps = [] + for index in range(10): + main = f"main-{index}" + main_file = f"/s/2026-08-26T10-00-00-{index:03d}Z_{main}.jsonl" + sub_file = f"/s/2026-08-26T10-00-00-{index:03d}Z_{main}/Worker.jsonl" + steps.append({"event": "session_start", "hasUI": True, "session": main, "file": main_file, "instance": 1}) + steps.append({"event": "tool_approval_requested", "hasUI": False, "session": f"sub-{index}", "file": sub_file, "instance": 2, "payload": {"type": "tool_approval_requested", "toolName": "write"}}) + steps.append({"event": "session_stop", "hasUI": True, "session": main, "file": main_file, "instance": 1}) + forwarded = self.pi_family("omp", steps) + expected = [] + for index in range(10): + expected += [("session_start", f"main-{index}"), ("tool_approval_requested", f"main-{index}"), ("session_stop", f"main-{index}")] + self.assertEqual([(payload["hook_event_name"], payload["session_id"]) for _, payload in forwarded], expected) + def test_pi_without_a_launch_token_spawns_nothing(self): forwarded = self.pi_family("pi", [{"event": "session_start", "hasUI": True, "session": "main-1", "file": self.MAIN_1}], token=None) self.assertEqual(forwarded, [])