fix(agent-claude-code): detect cd-prefixed gh/git commands and use relative hook path

This commit is contained in:
deepak 2026-03-08 10:42:45 +00:00
parent cc2031f0f6
commit 59633e45b4
3 changed files with 537 additions and 24 deletions

View File

@ -0,0 +1,306 @@
import { describe, it, expect, beforeAll, afterAll } from "vitest";
import { execSync } from "node:child_process";
import { mkdtempSync, writeFileSync, readFileSync, rmSync, mkdirSync } from "node:fs";
import { join } from "node:path";
import { tmpdir } from "node:os";
import { METADATA_UPDATER_SCRIPT } from "./index.js";
// ---------------------------------------------------------------------------
// Integration tests for the metadata-updater.sh hook script.
// These execute the actual bash script with various inputs and verify that
// session metadata files are updated correctly.
// ---------------------------------------------------------------------------
let testDir: string;
let hookScriptPath: string;
beforeAll(() => {
testDir = mkdtempSync(join(tmpdir(), "ao-hook-test-"));
hookScriptPath = join(testDir, "metadata-updater.sh");
writeFileSync(hookScriptPath, METADATA_UPDATER_SCRIPT, { mode: 0o755 });
});
afterAll(() => {
rmSync(testDir, { recursive: true, force: true });
});
/**
* Run the hook script with given parameters and return the script's
* stdout plus the updated metadata file contents.
*/
function runHook(opts: {
command: string;
toolName?: string;
output?: string;
exitCode?: number;
metadataContent?: string;
}): { stdout: string; metadata: string } {
const sessionId = `test-session-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`;
const sessionsDir = join(testDir, "sessions");
mkdirSync(sessionsDir, { recursive: true });
const metadataFile = join(sessionsDir, sessionId);
writeFileSync(metadataFile, opts.metadataContent ?? "status=spawning\n");
const input = JSON.stringify({
tool_name: opts.toolName ?? "Bash",
tool_input: { command: opts.command },
tool_response: opts.output ?? "",
exit_code: opts.exitCode ?? 0,
});
let stdout: string;
try {
stdout = execSync(`bash "${hookScriptPath}"`, {
input,
env: {
...process.env,
AO_SESSION: sessionId,
AO_DATA_DIR: sessionsDir,
HOME: testDir,
},
encoding: "utf-8",
timeout: 5000,
});
} catch (err: unknown) {
const e = err as { stdout?: string };
stdout = e.stdout ?? "";
}
let metadata: string;
try {
metadata = readFileSync(metadataFile, "utf-8");
} catch {
metadata = "";
}
return { stdout, metadata };
}
// =========================================================================
// gh pr create detection
// =========================================================================
describe("hook script: gh pr create", () => {
const prUrl = "https://github.com/owner/repo/pull/42";
it("detects plain gh pr create", () => {
const { metadata } = runHook({
command: 'gh pr create --title "fix" --body "test" --base master',
output: `Creating pull request...\n${prUrl}\n`,
});
expect(metadata).toContain(`pr=${prUrl}`);
expect(metadata).toContain("status=pr_open");
});
it("detects gh pr create with cd && prefix", () => {
const { metadata } = runHook({
command: `cd ~/.worktrees/mercury/cleanup && gh pr create --title "fix" --base master`,
output: `${prUrl}`,
});
expect(metadata).toContain(`pr=${prUrl}`);
expect(metadata).toContain("status=pr_open");
});
it("detects gh pr create with cd ; prefix", () => {
const { metadata } = runHook({
command: `cd /some/path ; gh pr create --title "test" --body "body"`,
output: prUrl,
});
expect(metadata).toContain(`pr=${prUrl}`);
expect(metadata).toContain("status=pr_open");
});
it("detects gh pr create with multiple chained cd prefixes", () => {
const { metadata } = runHook({
command: `cd /tmp && cd ~/.worktrees/mercury && gh pr create --title "fix" --base master`,
output: prUrl,
});
expect(metadata).toContain(`pr=${prUrl}`);
expect(metadata).toContain("status=pr_open");
});
it("does NOT update metadata when PR URL is missing from output", () => {
const { metadata } = runHook({
command: 'gh pr create --title "fix"',
output: "Error: something went wrong",
});
expect(metadata).not.toContain("pr=");
expect(metadata).toContain("status=spawning");
});
});
// =========================================================================
// git checkout -b / git switch -c detection
// =========================================================================
describe("hook script: git checkout -b / git switch -c", () => {
it("detects plain git checkout -b", () => {
const { metadata } = runHook({
command: "git checkout -b feat/my-feature",
});
expect(metadata).toContain("branch=feat/my-feature");
});
it("detects plain git switch -c", () => {
const { metadata } = runHook({
command: "git switch -c fix/bug-123",
});
expect(metadata).toContain("branch=fix/bug-123");
});
it("detects git checkout -b with cd && prefix", () => {
const { metadata } = runHook({
command: "cd /some/project && git checkout -b feat/new-feature",
});
expect(metadata).toContain("branch=feat/new-feature");
});
it("detects git switch -c with cd && prefix", () => {
const { metadata } = runHook({
command: "cd ~/.worktrees/project && git switch -c fix/issue-456",
});
expect(metadata).toContain("branch=fix/issue-456");
});
it("detects git checkout -b with cd ; prefix", () => {
const { metadata } = runHook({
command: "cd /project ; git checkout -b feat/semicolon-test",
});
expect(metadata).toContain("branch=feat/semicolon-test");
});
it("detects git checkout -b with multiple cd prefixes", () => {
const { metadata } = runHook({
command: "cd /tmp && cd /project && git checkout -b feat/chained",
});
expect(metadata).toContain("branch=feat/chained");
});
});
// =========================================================================
// gh pr merge detection
// =========================================================================
describe("hook script: gh pr merge", () => {
it("detects plain gh pr merge", () => {
const { metadata } = runHook({
command: "gh pr merge 123 --squash",
});
expect(metadata).toContain("status=merged");
});
it("detects gh pr merge with cd && prefix", () => {
const { metadata } = runHook({
command: "cd ~/.worktrees/project && gh pr merge 42 --squash",
});
expect(metadata).toContain("status=merged");
});
it("detects gh pr merge with cd ; prefix", () => {
const { metadata } = runHook({
command: "cd /project ; gh pr merge --rebase",
});
expect(metadata).toContain("status=merged");
});
});
// =========================================================================
// git checkout <existing-branch> detection (feature branches)
// =========================================================================
describe("hook script: git checkout existing branch", () => {
it("detects plain git checkout of a feature branch", () => {
const { metadata } = runHook({
command: "git checkout feat/existing-branch",
});
expect(metadata).toContain("branch=feat/existing-branch");
});
it("detects git checkout of a feature branch with cd prefix", () => {
const { metadata } = runHook({
command: "cd /project && git checkout feat/existing-branch",
});
expect(metadata).toContain("branch=feat/existing-branch");
});
});
// =========================================================================
// Non-matching commands (should NOT modify metadata)
// =========================================================================
describe("hook script: non-matching commands", () => {
it("ignores plain ls command", () => {
const { metadata } = runHook({
command: "ls -la",
});
expect(metadata).toBe("status=spawning\n");
});
it("ignores non-Bash tool calls", () => {
const { metadata } = runHook({
command: 'gh pr create --title "test"',
toolName: "Read",
output: "https://github.com/owner/repo/pull/1",
});
expect(metadata).toBe("status=spawning\n");
});
it("ignores commands with non-zero exit code", () => {
const { metadata } = runHook({
command: 'gh pr create --title "test"',
exitCode: 1,
output: "https://github.com/owner/repo/pull/1",
});
expect(metadata).toBe("status=spawning\n");
});
it("ignores cd-only commands (no chained git/gh)", () => {
const { metadata } = runHook({
command: "cd /some/directory",
});
expect(metadata).toBe("status=spawning\n");
});
it("ignores git status", () => {
const { metadata } = runHook({
command: "cd /project && git status",
});
expect(metadata).toBe("status=spawning\n");
});
});
// =========================================================================
// Metadata file update mechanics
// =========================================================================
describe("hook script: metadata file updates", () => {
it("updates an existing key in the metadata file", () => {
const { metadata } = runHook({
command: "gh pr merge 10 --squash",
metadataContent: "status=pr_open\nbranch=feat/test\n",
});
expect(metadata).toContain("status=merged");
expect(metadata).toContain("branch=feat/test");
expect(metadata).not.toContain("status=pr_open");
});
it("appends a new key to the metadata file", () => {
const { metadata } = runHook({
command: 'gh pr create --title "test"',
output: "https://github.com/owner/repo/pull/99",
metadataContent: "branch=feat/test\n",
});
expect(metadata).toContain("branch=feat/test");
expect(metadata).toContain("pr=https://github.com/owner/repo/pull/99");
expect(metadata).toContain("status=pr_open");
});
it("returns systemMessage JSON on successful detection", () => {
const { stdout } = runHook({
command: "gh pr merge 1 --squash",
});
expect(stdout).toContain("systemMessage");
expect(stdout).toContain("merged");
});
it("returns empty JSON for non-matching commands", () => {
const { stdout } = runHook({
command: "echo hello",
});
expect(stdout.trim()).toBe("{}");
});
});

View File

@ -1,17 +1,30 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import type { Session, RuntimeHandle, AgentLaunchConfig } from "@composio/ao-core";
import type { Session, RuntimeHandle, AgentLaunchConfig, WorkspaceHooksConfig } from "@composio/ao-core";
// ---------------------------------------------------------------------------
// Hoisted mocks — available inside vi.mock factories
// ---------------------------------------------------------------------------
const { mockExecFileAsync, mockReaddir, mockReadFile, mockStat, mockHomedir } =
vi.hoisted(() => ({
mockExecFileAsync: vi.fn(),
mockReaddir: vi.fn(),
mockReadFile: vi.fn(),
mockStat: vi.fn(),
mockHomedir: vi.fn(() => "/mock/home"),
}));
const {
mockExecFileAsync,
mockReaddir,
mockReadFile,
mockStat,
mockHomedir,
mockWriteFile,
mockMkdir,
mockChmod,
mockExistsSync,
} = vi.hoisted(() => ({
mockExecFileAsync: vi.fn(),
mockReaddir: vi.fn(),
mockReadFile: vi.fn(),
mockStat: vi.fn(),
mockHomedir: vi.fn(() => "/mock/home"),
mockWriteFile: vi.fn().mockResolvedValue(undefined),
mockMkdir: vi.fn().mockResolvedValue(undefined),
mockChmod: vi.fn().mockResolvedValue(undefined),
mockExistsSync: vi.fn().mockReturnValue(false),
}));
vi.mock("node:child_process", () => {
const fn = Object.assign((..._args: unknown[]) => {}, {
@ -24,13 +37,20 @@ vi.mock("node:fs/promises", () => ({
readdir: mockReaddir,
readFile: mockReadFile,
stat: mockStat,
writeFile: mockWriteFile,
mkdir: mockMkdir,
chmod: mockChmod,
}));
vi.mock("node:fs", () => ({
existsSync: mockExistsSync,
}));
vi.mock("node:os", () => ({
homedir: mockHomedir,
}));
import { create, manifest, default as defaultExport, resetPsCache } from "./index.js";
import { create, manifest, default as defaultExport, resetPsCache, METADATA_UPDATER_SCRIPT } from "./index.js";
// ---------------------------------------------------------------------------
// Test helpers
@ -670,3 +690,179 @@ describe("getSessionInfo", () => {
});
});
});
// =========================================================================
// METADATA_UPDATER_SCRIPT — content verification (unit tests)
// =========================================================================
describe("METADATA_UPDATER_SCRIPT content", () => {
it("contains clean_command stripping logic for cd prefixes", () => {
expect(METADATA_UPDATER_SCRIPT).toContain('clean_command="$command"');
expect(METADATA_UPDATER_SCRIPT).toMatch(/while.*clean_command.*cd/);
});
it("uses $clean_command (not $command) for all regex-based command detection", () => {
const lines = METADATA_UPDATER_SCRIPT.split("\n");
for (const line of lines) {
// Skip comment lines, the initial assignment, and the stripping logic itself
if (line.trim().startsWith("#")) continue;
if (line.includes('clean_command="$command"')) continue;
if (line.includes("while") && line.includes("clean_command")) continue;
// Any regex match line (=~) should use $clean_command, NOT $command
if (line.includes("=~") && line.includes("command")) {
expect(line).toContain("clean_command");
expect(line).not.toMatch(/"\$command"/);
}
}
});
it("does NOT use ^-anchored regexes directly on $command for gh/git detection", () => {
// The old buggy patterns matched $command with ^ anchor.
// After the fix, ^ is still used but on $clean_command (which has cd stripped).
expect(METADATA_UPDATER_SCRIPT).not.toMatch(
/"\$command"\s*=~\s*\^gh/,
);
expect(METADATA_UPDATER_SCRIPT).not.toMatch(
/"\$command"\s*=~\s*\^git/,
);
});
it("strips cd prefixes with both && and ; delimiters", () => {
expect(METADATA_UPDATER_SCRIPT).toMatch(/&&\|;/);
});
it("handles multiple chained cd commands via while loop", () => {
expect(METADATA_UPDATER_SCRIPT).toMatch(/while.*clean_command/);
});
it("detects gh pr create on clean_command", () => {
expect(METADATA_UPDATER_SCRIPT).toMatch(
/"\$clean_command"\s*=~\s*\^gh\[/,
);
});
it("detects git checkout -b on clean_command", () => {
expect(METADATA_UPDATER_SCRIPT).toMatch(
/"\$clean_command"\s*=~\s*\^git\[.*checkout/,
);
});
it("detects gh pr merge on clean_command", () => {
expect(METADATA_UPDATER_SCRIPT).toMatch(
/"\$clean_command"\s*=~\s*\^gh\[.*merge/,
);
});
});
// =========================================================================
// setupWorkspaceHooks / postLaunchSetup — hook path (symlink safety)
// =========================================================================
describe("hook setup — relative path (symlink-safe)", () => {
const agent = create();
/** Extract the hook command from the settings.json that was written */
function getWrittenHookCommand(): string {
const settingsWrite = mockWriteFile.mock.calls.find(
([path]: unknown[]) => typeof path === "string" && path.endsWith("settings.json"),
);
expect(settingsWrite).toBeDefined();
const parsed = JSON.parse(settingsWrite![1] as string);
return parsed.hooks.PostToolUse[0].hooks[0].command;
}
it("setupWorkspaceHooks writes a relative hook command (not absolute)", async () => {
await agent.setupWorkspaceHooks!(
"/Users/equinox/.worktrees/integrator/integrator-5",
{} as WorkspaceHooksConfig,
);
const hookCommand = getWrittenHookCommand();
expect(hookCommand).toBe(".claude/metadata-updater.sh");
expect(hookCommand).not.toMatch(/^\//);
});
it("postLaunchSetup writes a relative hook command (not absolute)", async () => {
await agent.postLaunchSetup!(
makeSession({ workspacePath: "/Users/equinox/.worktrees/integrator/integrator-10" }),
);
const hookCommand = getWrittenHookCommand();
expect(hookCommand).toBe(".claude/metadata-updater.sh");
expect(hookCommand).not.toMatch(/^\//);
});
it("different worktree paths produce identical settings.json content", async () => {
await agent.setupWorkspaceHooks!(
"/Users/equinox/.worktrees/integrator/integrator-5",
{} as WorkspaceHooksConfig,
);
const settingsWrite1 = mockWriteFile.mock.calls.find(
([path]: unknown[]) => typeof path === "string" && path.endsWith("settings.json"),
);
const content1 = settingsWrite1![1] as string;
mockWriteFile.mockClear();
await agent.setupWorkspaceHooks!(
"/Users/equinox/.worktrees/integrator/integrator-10",
{} as WorkspaceHooksConfig,
);
const settingsWrite2 = mockWriteFile.mock.calls.find(
([path]: unknown[]) => typeof path === "string" && path.endsWith("settings.json"),
);
const content2 = settingsWrite2![1] as string;
expect(content1).toBe(content2);
});
it("updates an existing absolute hook path to relative", async () => {
mockExistsSync.mockReturnValue(true);
mockReadFile.mockResolvedValue(
JSON.stringify({
hooks: {
PostToolUse: [
{
matcher: "Bash",
hooks: [
{
type: "command",
command:
"/Users/equinox/.worktrees/integrator/integrator-5/.claude/metadata-updater.sh",
timeout: 5000,
},
],
},
],
},
}),
);
await agent.setupWorkspaceHooks!(
"/Users/equinox/.worktrees/integrator/integrator-10",
{} as WorkspaceHooksConfig,
);
const hookCommand = getWrittenHookCommand();
expect(hookCommand).toBe(".claude/metadata-updater.sh");
});
it("still writes the script file to the correct absolute filesystem path", async () => {
await agent.setupWorkspaceHooks!(
"/Users/equinox/.worktrees/integrator/integrator-5",
{} as WorkspaceHooksConfig,
);
const scriptWrite = mockWriteFile.mock.calls.find(
([path]: unknown[]) => typeof path === "string" && path.endsWith("metadata-updater.sh"),
);
expect(scriptWrite).toBeDefined();
expect(scriptWrite![0]).toBe(
"/Users/equinox/.worktrees/integrator/integrator-5/.claude/metadata-updater.sh",
);
});
it("skips postLaunchSetup when workspacePath is null", async () => {
await agent.postLaunchSetup!(makeSession({ workspacePath: null }));
expect(mockWriteFile).not.toHaveBeenCalled();
});
});

View File

@ -36,8 +36,9 @@ function normalizePermissionMode(mode: string | undefined): "permissionless" | "
// Metadata Updater Hook Script
// =============================================================================
/** Hook script content that updates session metadata on git/gh commands */
const METADATA_UPDATER_SCRIPT = `#!/usr/bin/env bash
/** Hook script content that updates session metadata on git/gh commands.
* Exported for integration testing. */
export const METADATA_UPDATER_SCRIPT = `#!/usr/bin/env bash
# Metadata Updater Hook for Agent Orchestrator
#
# This PostToolUse hook automatically updates session metadata when:
@ -124,8 +125,20 @@ update_metadata_key() {
# Command Detection and Parsing
# ============================================================================
# Strip leading directory-change prefixes so that commands like
# cd ~/.worktrees/project && gh pr create ...
# are correctly detected. Agents frequently cd into a worktree first.
clean_command="$command"
while [[ "$clean_command" =~ ^[[:space:]]*cd[[:space:]] ]]; do
if [[ "$clean_command" =~ ^[[:space:]]*cd[[:space:]]+[^'&;']*(&&|;)[[:space:]]*(.*) ]]; then
clean_command="\${BASH_REMATCH[2]}"
else
break
fi
done
# Detect: gh pr create
if [[ "$command" =~ ^gh[[:space:]]+pr[[:space:]]+create ]]; then
if [[ "$clean_command" =~ ^gh[[:space:]]+pr[[:space:]]+create ]]; then
# Extract PR URL from output
pr_url=$(echo "$output" | grep -Eo 'https://github[.]com/[^/]+/[^/]+/pull/[0-9]+' | head -1)
@ -138,8 +151,8 @@ if [[ "$command" =~ ^gh[[:space:]]+pr[[:space:]]+create ]]; then
fi
# Detect: git checkout -b <branch> or git switch -c <branch>
if [[ "$command" =~ ^git[[:space:]]+checkout[[:space:]]+-b[[:space:]]+([^[:space:]]+) ]] || \\
[[ "$command" =~ ^git[[:space:]]+switch[[:space:]]+-c[[:space:]]+([^[:space:]]+) ]]; then
if [[ "$clean_command" =~ ^git[[:space:]]+checkout[[:space:]]+-b[[:space:]]+([^[:space:]]+) ]] || \\
[[ "$clean_command" =~ ^git[[:space:]]+switch[[:space:]]+-c[[:space:]]+([^[:space:]]+) ]]; then
branch="\${BASH_REMATCH[1]}"
if [[ -n "$branch" ]]; then
@ -151,8 +164,8 @@ fi
# Detect: git checkout <branch> (without -b) or git switch <branch> (without -c)
# Only update if the branch name looks like a feature branch (contains / or -)
if [[ "$command" =~ ^git[[:space:]]+checkout[[:space:]]+([^[:space:]-]+[/-][^[:space:]]+) ]] || \\
[[ "$command" =~ ^git[[:space:]]+switch[[:space:]]+([^[:space:]-]+[/-][^[:space:]]+) ]]; then
if [[ "$clean_command" =~ ^git[[:space:]]+checkout[[:space:]]+([^[:space:]-]+[/-][^[:space:]]+) ]] || \\
[[ "$clean_command" =~ ^git[[:space:]]+switch[[:space:]]+([^[:space:]-]+[/-][^[:space:]]+) ]]; then
branch="\${BASH_REMATCH[1]}"
# Avoid updating for checkout of commits/tags
@ -164,7 +177,7 @@ if [[ "$command" =~ ^git[[:space:]]+checkout[[:space:]]+([^[:space:]-]+[/-][^[:s
fi
# Detect: gh pr merge
if [[ "$command" =~ ^gh[[:space:]]+pr[[:space:]]+merge ]]; then
if [[ "$clean_command" =~ ^gh[[:space:]]+pr[[:space:]]+merge ]]; then
update_metadata_key "status" "merged"
echo '{"systemMessage": "Updated metadata: status = merged"}'
exit 0
@ -817,17 +830,15 @@ function createClaudeCodeAgent(): Agent {
},
async setupWorkspaceHooks(workspacePath: string, _config: WorkspaceHooksConfig): Promise<void> {
// Use absolute path for hook command (specific to this workspace)
const hookScriptPath = join(workspacePath, ".claude", "metadata-updater.sh");
await setupHookInWorkspace(workspacePath, hookScriptPath);
// Relative path so that symlinked .claude/ dirs across worktrees
// all produce the same settings.json (last writer doesn't clobber).
await setupHookInWorkspace(workspacePath, ".claude/metadata-updater.sh");
},
async postLaunchSetup(session: Session): Promise<void> {
if (!session.workspacePath) return;
// Use absolute path for hook command (specific to this workspace)
const hookScriptPath = join(session.workspacePath, ".claude", "metadata-updater.sh");
await setupHookInWorkspace(session.workspacePath, hookScriptPath);
await setupHookInWorkspace(session.workspacePath, ".claude/metadata-updater.sh");
},
};
}