From 62fdd33a647fb18539e5b87d43ec0282c1a044eb Mon Sep 17 00:00:00 2001 From: Baptiste LAFOURCADE Date: Thu, 10 Sep 2026 10:40:58 +0200 Subject: [PATCH] fix(cli): telemetry off keeps the person's own commit hook executable Removing the commit-trailer delegate rewrote prepare-commit-msg through the atomic temp-and-rename write, which does not carry the mode over: a hook that was executable came back without its bit, and git skips such a hook silently, so the rest of the person's own hook stopped running. The bit is read before the write and restored after it, only when it was set. Two tests on a real temporary repository: the removal keeps the hook executable (red without the fix), and a hook that was not executable stays so (red with the restore made unconditional). The second is POSIX-only. Fixes #817 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011x4ms5qcGuZgYhCxfdHMUb AIDD-Session-Id: 4acc9a1c-19bc-4468-b8b6-e86644bcba60 --- cli/src/runtime/git/git-adapter.ts | 2 + .../git/git-adapter.integration.test.ts | 83 +++++++++++++++++++ 2 files changed, 85 insertions(+) create mode 100644 cli/tests/runtime/git/git-adapter.integration.test.ts diff --git a/cli/src/runtime/git/git-adapter.ts b/cli/src/runtime/git/git-adapter.ts index 87ad87360..c54ca7d59 100644 --- a/cli/src/runtime/git/git-adapter.ts +++ b/cli/src/runtime/git/git-adapter.ts @@ -112,7 +112,9 @@ export class GitAdapter implements VersionControl { if (!content.includes(line)) return false; const kept = content.split("\n").filter((entry) => entry.trim() !== line); + const executable = await this.fs.isExecutable(hookPath); await this.fs.writeFile(hookPath, kept.join("\n")); + if (executable) await this.fs.chmodExecutable(hookPath); return true; } diff --git a/cli/tests/runtime/git/git-adapter.integration.test.ts b/cli/tests/runtime/git/git-adapter.integration.test.ts new file mode 100644 index 000000000..b71554bf3 --- /dev/null +++ b/cli/tests/runtime/git/git-adapter.integration.test.ts @@ -0,0 +1,83 @@ +import { spawnSync } from "node:child_process"; +import { chmod, mkdir, mkdtemp, readFile, realpath, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { sessionTrailerHookLine } from "../../../src/contexts/telemetry/domain/formats/commit-session-trailer.js"; +import { FileAdapter } from "../../../src/runtime/filesystem/file-adapter.js"; +import { HasherAdapter } from "../../../src/runtime/filesystem/hasher-adapter.js"; +import { GitAdapter } from "../../../src/runtime/git/git-adapter.js"; +import { environmentWithoutGitVariables } from "../../../src/runtime/git/git-environment.js"; + +const DELEGATE = "aidd-session-trailer.sh"; +const SCRIPT = "#!/bin/sh\necho delegate\n"; + +function git(cwd: string, ...args: string[]): string { + const result = spawnSync( + "git", + ["-c", "user.name=t", "-c", "user.email=t@t", "-c", "commit.gpgsign=false", ...args], + { cwd, encoding: "utf8", env: environmentWithoutGitVariables() } + ); + if (result.status !== 0) throw new Error(`git ${args.join(" ")} failed: ${result.stderr}`); + return result.stdout.trim(); +} + +describe("GitAdapter", () => { + let root: string; + let hooksDir: string; + let adapter: GitAdapter; + + beforeEach(async () => { + root = await realpath(await mkdtemp(join(tmpdir(), "aidd-git-adapter-"))); + git(root, "init", "-q"); + hooksDir = join(root, ".git", "hooks"); + adapter = new GitAdapter(new FileAdapter(new HasherAdapter())); + }); + + afterEach(async () => { + await rm(root, { recursive: true, force: true }); + }); + + describe("removeCommitMessageDelegate", () => { + it("drops the one line and deletes the delegate, leaving the rest of the hook", async () => { + await mkdir(hooksDir, { recursive: true }); + await writeFile(join(hooksDir, "prepare-commit-msg"), "#!/bin/sh\necho mine\n"); + await adapter.installCommitMessageDelegate(root, DELEGATE, SCRIPT); + + const result = await adapter.removeCommitMessageDelegate(root, DELEGATE); + + expect(result).toStrictEqual({ removed: true }); + expect(await readFile(join(hooksDir, "prepare-commit-msg"), "utf8")).toBe( + "#!/bin/sh\necho mine\n" + ); + expect(await adapter.readCommitTrailerSetup(root, DELEGATE, "X", 1)).toStrictEqual({ + delegate: "absent", + hookExecutable: true, + callSite: "missing", + hookHasOtherContent: true, + hooksDir, + }); + }); + + // A mode bit is POSIX: on Windows `access(X_OK)` answers like `F_OK`. + it.skipIf(process.platform === "win32")( + "leaves a hook that was not executable as it found it", + async () => { + await mkdir(hooksDir, { recursive: true }); + await adapter.installCommitMessageDelegate(root, DELEGATE, SCRIPT); + const hookPath = join(hooksDir, "prepare-commit-msg"); + await writeFile( + hookPath, + `#!/bin/sh\n${sessionTrailerHookLine(join(hooksDir, DELEGATE))}\n` + ); + await chmod(hookPath, 0o644); + + await adapter.removeCommitMessageDelegate(root, DELEGATE); + + expect((await adapter.readCommitTrailerSetup(root, DELEGATE, "X", 1)).hookExecutable).toBe( + false + ); + } + ); + }); +});