fix(server): no-op agent hooks when PASEO_TERMINAL_ID is unset (#1667)
* fix(server): no-op agent hooks when PASEO_TERMINAL_ID is unset
The shell command generated by `buildAgentHookShellCommand` short-circuited
on `[ -n "$PASEO_TERMINAL_ID" ] && ...` and exited 1 when the terminal
ID was absent, so Claude Code reported every Notification/Stop/StopFailure/
SessionEnd/UserPromptSubmit event as `Failed with non-blocking status code`.
The same shape affected the Codex provider.
Switch the gate to `if [ -n ... ]; then ...; fi` so the hook is a true
no-op when the terminal id is unset and preserves the underlying CLI's
exit code when it actually runs. Mirrors `opencode-plugin.ts`, which
already early-returns on the same env var. Adds the matching
`else (exit /b 0)` branch to `buildAgentHookWindowsCommand`.
Updates the literal-string assertions in the existing Claude and Codex
hook tests, and adds an executable regression that runs the generated
shell command through `/bin/sh` and asserts exit 0 across all five
Claude hook events.
* test(server): skip POSIX hook regression on Windows
The /bin/sh exit-code regression spawns /bin/sh -c <posix command>,
which has no equivalent on the Windows runner, so spawnSync returned
status: null and the five Claude events failed in CI.
Gate the it.each block with skipIf(isPlatform("win32")), matching the
pattern already used in worker-terminal-manager.test.ts and
executable-resolution.test.ts. The remaining literal-string and
buildTerminalEnvironment assertions in this file keep running on
every platform.
This commit is contained in:
@@ -138,7 +138,7 @@ export function buildAgentHookShellCommand<TConfig>(
|
|||||||
event: AgentHookEventDefinition,
|
event: AgentHookEventDefinition,
|
||||||
): string {
|
): string {
|
||||||
const hookCommand = `"\${PASEO_HOOK_CLI:-paseo}" hooks ${shellToken(provider.id)} ${shellToken(event.event)}`;
|
const hookCommand = `"\${PASEO_HOOK_CLI:-paseo}" hooks ${shellToken(provider.id)} ${shellToken(event.event)}`;
|
||||||
return `[ -n "$PASEO_TERMINAL_ID" ] && ${hookCommand}`;
|
return `if [ -n "$PASEO_TERMINAL_ID" ]; then ${hookCommand}; fi`;
|
||||||
}
|
}
|
||||||
|
|
||||||
export function buildAgentHookWindowsCommand<TConfig>(
|
export function buildAgentHookWindowsCommand<TConfig>(
|
||||||
@@ -146,7 +146,7 @@ export function buildAgentHookWindowsCommand<TConfig>(
|
|||||||
event: AgentHookEventDefinition,
|
event: AgentHookEventDefinition,
|
||||||
): string {
|
): string {
|
||||||
const hookArgs = `hooks ${windowsToken(provider.id)} ${windowsToken(event.event)}`;
|
const hookArgs = `hooks ${windowsToken(provider.id)} ${windowsToken(event.event)}`;
|
||||||
return `if defined PASEO_TERMINAL_ID (if defined PASEO_HOOK_CLI ("%PASEO_HOOK_CLI%" ${hookArgs}) else (paseo ${hookArgs}))`;
|
return `if defined PASEO_TERMINAL_ID (if defined PASEO_HOOK_CLI ("%PASEO_HOOK_CLI%" ${hookArgs}) else (paseo ${hookArgs})) else (exit /b 0)`;
|
||||||
}
|
}
|
||||||
|
|
||||||
function installAgentHookPluginFile(
|
function installAgentHookPluginFile(
|
||||||
|
|||||||
@@ -1,8 +1,10 @@
|
|||||||
|
import { spawnSync } from "node:child_process";
|
||||||
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
||||||
import { tmpdir } from "node:os";
|
import { tmpdir } from "node:os";
|
||||||
import { delimiter, dirname, join, resolve } from "node:path";
|
import { delimiter, dirname, join, resolve } from "node:path";
|
||||||
import { fileURLToPath } from "node:url";
|
import { fileURLToPath } from "node:url";
|
||||||
import { afterEach, describe, expect, it } from "vitest";
|
import { afterEach, describe, expect, it } from "vitest";
|
||||||
|
import { isPlatform } from "../../../test-utils/platform.js";
|
||||||
import { buildTerminalEnvironment } from "../../terminal.js";
|
import { buildTerminalEnvironment } from "../../terminal.js";
|
||||||
import { buildAgentHookShellCommand } from "../agent-hook-installer.js";
|
import { buildAgentHookShellCommand } from "../agent-hook-installer.js";
|
||||||
import {
|
import {
|
||||||
@@ -81,7 +83,7 @@ describe("Claude terminal agent hooks", () => {
|
|||||||
);
|
);
|
||||||
expect(paseoCommands).toHaveLength(1);
|
expect(paseoCommands).toHaveLength(1);
|
||||||
expect(paseoCommands[0]).toBe(
|
expect(paseoCommands[0]).toBe(
|
||||||
`[ -n "$PASEO_TERMINAL_ID" ] && "\${PASEO_HOOK_CLI:-paseo}" hooks ${provider.id} ${event.event}`,
|
`if [ -n "$PASEO_TERMINAL_ID" ]; then "\${PASEO_HOOK_CLI:-paseo}" hooks ${provider.id} ${event.event}; fi`,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
expect(registeredAgentHooksAreInstalled({ configDir })).toBe(true);
|
expect(registeredAgentHooksAreInstalled({ configDir })).toBe(true);
|
||||||
@@ -146,10 +148,25 @@ describe("Claude terminal agent hooks", () => {
|
|||||||
const command = buildAgentHookShellCommand(provider, provider.events[0]);
|
const command = buildAgentHookShellCommand(provider, provider.events[0]);
|
||||||
|
|
||||||
expect(command).toBe(
|
expect(command).toBe(
|
||||||
'[ -n "$PASEO_TERMINAL_ID" ] && "${PASEO_HOOK_CLI:-paseo}" hooks claude UserPromptSubmit',
|
'if [ -n "$PASEO_TERMINAL_ID" ]; then "${PASEO_HOOK_CLI:-paseo}" hooks claude UserPromptSubmit; fi',
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it.skipIf(isPlatform("win32")).each(AGENT_HOOK_PROVIDERS.claude.events)(
|
||||||
|
"$event hook command exits 0 when PASEO_TERMINAL_ID is unset",
|
||||||
|
(event) => {
|
||||||
|
const provider = AGENT_HOOK_PROVIDERS.claude;
|
||||||
|
const command = buildAgentHookShellCommand(provider, event);
|
||||||
|
|
||||||
|
const result = spawnSync("/bin/sh", ["-c", command], {
|
||||||
|
env: { PATH: process.env.PATH ?? "", PASEO_HOOK_CLI: "paseo" },
|
||||||
|
stdio: "ignore",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result.status).toBe(0);
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
it("keeps provider names out of generic CLI and bootstrap integration points", () => {
|
it("keeps provider names out of generic CLI and bootstrap integration points", () => {
|
||||||
const genericFiles = [
|
const genericFiles = [
|
||||||
join(repositoryRoot, "packages", "cli", "src", "commands", "hooks.ts"),
|
join(repositoryRoot, "packages", "cli", "src", "commands", "hooks.ts"),
|
||||||
|
|||||||
@@ -68,8 +68,8 @@ describe("Codex terminal agent hooks", () => {
|
|||||||
for (const event of codexAgentHookProvider.events) {
|
for (const event of codexAgentHookProvider.events) {
|
||||||
expect(commandHooks(config, event.event)).toEqual([
|
expect(commandHooks(config, event.event)).toEqual([
|
||||||
{
|
{
|
||||||
command: `[ -n "$PASEO_TERMINAL_ID" ] && "\${PASEO_HOOK_CLI:-paseo}" hooks codex ${event.event}`,
|
command: `if [ -n "$PASEO_TERMINAL_ID" ]; then "\${PASEO_HOOK_CLI:-paseo}" hooks codex ${event.event}; fi`,
|
||||||
commandWindows: `if defined PASEO_TERMINAL_ID (if defined PASEO_HOOK_CLI ("%PASEO_HOOK_CLI%" hooks codex ${event.event}) else (paseo hooks codex ${event.event}))`,
|
commandWindows: `if defined PASEO_TERMINAL_ID (if defined PASEO_HOOK_CLI ("%PASEO_HOOK_CLI%" hooks codex ${event.event}) else (paseo hooks codex ${event.event})) else (exit /b 0)`,
|
||||||
},
|
},
|
||||||
]);
|
]);
|
||||||
}
|
}
|
||||||
@@ -102,7 +102,7 @@ describe("Codex terminal agent hooks", () => {
|
|||||||
const stopCommands = commandHooks(readHooksFile(configDir), "Stop").map((hook) => hook.command);
|
const stopCommands = commandHooks(readHooksFile(configDir), "Stop").map((hook) => hook.command);
|
||||||
expect(stopCommands).toEqual([
|
expect(stopCommands).toEqual([
|
||||||
"say codex done",
|
"say codex done",
|
||||||
'[ -n "$PASEO_TERMINAL_ID" ] && "${PASEO_HOOK_CLI:-paseo}" hooks codex Stop',
|
'if [ -n "$PASEO_TERMINAL_ID" ]; then "${PASEO_HOOK_CLI:-paseo}" hooks codex Stop; fi',
|
||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user