From 4f455b1f5140b8ec75ddcea495babebb99dc2e55 Mon Sep 17 00:00:00 2001 From: gb Date: Thu, 3 Sep 2026 17:06:30 -0700 Subject: [PATCH] fix(tests): stop fixture git commits failing when the developer signs commits Tests that need a real repository build a throwaway one in a temp dir and commit into it with an inline fake identity (-c user.email=t@example.com). Every other config key still resolves normally, so a global commit.gpgsign=true makes git try to sign as that fake identity, find no key for it, and abort with "gpg failed to sign the data". CI cannot catch this: runners start with no global git config, so signing is off there and all of these tests pass. It only reproduces on a developer machine, where it looks like a broken test rather than a machine-configuration problem. Add --no-gpg-sign to the eight fixture commit sites. It overrides commit.gpgsign for a single command and is inert where signing was already off. Also switch the router.rs git2 fixture from repo.signature() to a fixed identity, so fixture commits are not authored by whoever ran the suite (a determinism fix; libgit2 does not sign commits), and drop a commit plus two git config calls in bootstrap.rs that nothing depended on - MainRepoRoot resolves via Repository::discover() and never reads HEAD. --- AGENTS.md | 6 ++++++ crates/ai-memory-cli/src/commands/hook_capture.rs | 4 ++++ crates/ai-memory-consolidate/src/bootstrap.rs | 11 +++++++---- crates/ai-memory-hooks/src/router.rs | 11 +++++++---- scripts/managed-workstream-acceptance.sh | 4 ++-- tests/hooks/test_lib.sh | 2 +- 6 files changed, 27 insertions(+), 11 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c37d6051..79dc78e8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -349,6 +349,12 @@ Additional boundary rules: Parsers, ID derivation, and retention/decay math especially. - Filesystem tests use temp dirs or injected roots (`tempfile`); never depend on the real user home directory being writable. +- Tests that build throwaway git repos must not inherit the contributor's + machine config: pass `--no-gpg-sign` on every fixture commit (a global + `commit.gpgsign = true` otherwise signs as the fixture's fake identity and + fails), and build `git2` signatures with a fixed `Signature::now(...)`, + never `repo.signature()`. CI cannot catch either — its runners have no + global gitconfig, so the breakage only ever shows up on a developer's box. - PRs touching scope resolution need table-driven tests for partial scope, missing explicit scope, active-project precedence, and cross-workspace isolation. diff --git a/crates/ai-memory-cli/src/commands/hook_capture.rs b/crates/ai-memory-cli/src/commands/hook_capture.rs index 1b84908e..7e58329a 100644 --- a/crates/ai-memory-cli/src/commands/hook_capture.rs +++ b/crates/ai-memory-cli/src/commands/hook_capture.rs @@ -893,6 +893,9 @@ drop_subagent_captures = "true" .unwrap() .success() ); + // The commit exists only because `git worktree add` refuses a repo with + // no commits. --no-gpg-sign: a global `commit.gpgsign = true` would + // otherwise make git sign as this throwaway identity, and fail. assert!( std::process::Command::new("git") .arg("-C") @@ -904,6 +907,7 @@ drop_subagent_captures = "true" "user.name=t", "commit", "-q", + "--no-gpg-sign", "--allow-empty", "-m", "init", diff --git a/crates/ai-memory-consolidate/src/bootstrap.rs b/crates/ai-memory-consolidate/src/bootstrap.rs index 8580695b..dd4c848f 100644 --- a/crates/ai-memory-consolidate/src/bootstrap.rs +++ b/crates/ai-memory-consolidate/src/bootstrap.rs @@ -1336,15 +1336,19 @@ mod tests { run(&["init", "-q", "-b", "main"])?; run(&["config", "user.email", "test@example.com"])?; run(&["config", "user.name", "Test"])?; + // --no-gpg-sign throughout: a global `commit.gpgsign = true` would make + // git sign as this fixture's throwaway identity, and fail. run(&[ "commit", + "--no-gpg-sign", "--allow-empty", "-m", "feat: initial scaffolding for storage substrate with WAL + supersession chain", ])?; - run(&["commit", "--allow-empty", "-m", "typo"])?; + run(&["commit", "--no-gpg-sign", "--allow-empty", "-m", "typo"])?; run(&[ "commit", + "--no-gpg-sign", "--allow-empty", "-m", "design: choose Karpathy compile-not-retrieve model over RAG for capture", @@ -1683,10 +1687,9 @@ mod tests { assert!(status.success(), "git {args:?} failed"); Ok(()) }; + // `git init` alone is enough: MainRepoRoot resolves through + // `Repository::discover` + `commondir()`, which never reads HEAD. run(&["init", "-q", "-b", "main"]).unwrap(); - run(&["config", "user.email", "t@t"]).unwrap(); - run(&["config", "user.name", "t"]).unwrap(); - run(&["commit", "--allow-empty", "-m", "init"]).unwrap(); let from_sub = repo.join("sub").join("dir"); let (name, root) = diff --git a/crates/ai-memory-hooks/src/router.rs b/crates/ai-memory-hooks/src/router.rs index 386dfb96..8884183c 100644 --- a/crates/ai-memory-hooks/src/router.rs +++ b/crates/ai-memory-hooks/src/router.rs @@ -3730,9 +3730,10 @@ mod tests { fn init_repo_with_commit(path: &std::path::Path) -> git2::Repository { std::fs::create_dir_all(path).unwrap(); let repo = git2::Repository::init(path).unwrap(); - let sig = repo - .signature() - .unwrap_or_else(|_| git2::Signature::now("test", "test@test.com").unwrap()); + // Fixed identity, not `repo.signature()`: that reads the machine's global + // user.name/user.email, so these commits would be authored by whoever ran + // the suite. Unrelated to signing — libgit2 never signs commits. + let sig = git2::Signature::now("test", "test@test.com").unwrap(); let tree_id = repo.index().unwrap().write_tree().unwrap(); { let tree = repo.find_tree(tree_id).unwrap(); @@ -3766,7 +3767,9 @@ mod tests { commit .arg("-C") .arg(path) - .args(["commit", "--allow-empty", "-m", "initial"]); + // --no-gpg-sign: a global `commit.gpgsign = true` would make git + // sign as this fixture's throwaway identity, and fail. + .args(["commit", "--no-gpg-sign", "--allow-empty", "-m", "initial"]); assert_command_success(commit); } diff --git a/scripts/managed-workstream-acceptance.sh b/scripts/managed-workstream-acceptance.sh index c98b6e10..6267e37a 100755 --- a/scripts/managed-workstream-acceptance.sh +++ b/scripts/managed-workstream-acceptance.sh @@ -47,7 +47,7 @@ git -C "$REPO" config user.name "ai-memory acceptance" git -C "$REPO" config user.email "acceptance@localhost" printf '# Managed workstream acceptance\n' >"$REPO/README.md" git -C "$REPO" add README.md -git -C "$REPO" commit -qm "acceptance fixture" +git -C "$REPO" commit -q --no-gpg-sign -m "acceptance fixture" TOKEN="managed-acceptance-$(date +%s)-$$" PORT=${AI_MEMORY_ACCEPTANCE_PORT:-$((52000 + ($$ % 10000)))} @@ -1235,7 +1235,7 @@ git -C "$OTHER_REPO" config user.name "ai-memory acceptance" git -C "$OTHER_REPO" config user.email "acceptance@localhost" printf '# elsewhere\n' >"$OTHER_REPO/README.md" git -C "$OTHER_REPO" add README.md -git -C "$OTHER_REPO" commit -qm "acceptance fixture" +git -C "$OTHER_REPO" commit -q --no-gpg-sign -m "acceptance fixture" other_json=$(cd "$OTHER_REPO" && "$BIN" --data-dir "$DATA" workstreams \ --project "$(basename "$REPO")" --json) jq -e 'length == 0' <<<"$other_json" >/dev/null || { diff --git a/tests/hooks/test_lib.sh b/tests/hooks/test_lib.sh index 9f06260d..b295fcde 100755 --- a/tests/hooks/test_lib.sh +++ b/tests/hooks/test_lib.sh @@ -181,7 +181,7 @@ if command -v git >/dev/null 2>&1; then mkdir -p "$REPO" git init -q "$REPO" git -C "$REPO" -c user.email=t@example.com -c user.name=t \ - commit -q --allow-empty -m init + commit -q --no-gpg-sign --allow-empty -m init # A subdirectory of the main checkout collapses to the repo basename # (not the subdir name) when the marker selects repo-root and pins no