test: disable commit signing in ephemeral test repos

[ifireball]

Summary

• Disable commit signing in ephemeral git repos created by unit tests and eval fixtures
• Add a Makefile go-test env safety net so future test git subprocesses skip gitsign
• Leaves global/worktree signing unchanged for real developer commits

Related Issue

N/A — local developer ergonomics (gitsign rate limits during make go-test)

Changes

• Add testGitEnv() in internal/gitfetch/gitfetch_test.go with commit.gpgsign=false via GIT_CONFIG_* and local repo config after init
• Export signing-disable GIT_CONFIG vars in Makefile go-test target
• Set commit.gpgsign false on eval fixture repos in eval/scripts/setup-fixture.sh

Testing

• make go-test passes (via mise exec)
• Verified temp test repos have commit.gpgsign=false while worktree still inherits global true

Checklist

• PR title follows Conventional Commits (correct type, ! for breaking changes)
• Commits are signed off (DCO) — human and human-directed agent sessions only
• I wrote this contribution myself and can explain all changes in it

Made with Cursor

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 11:29 AM UTC · Completed 11:36 AM UTC
Commit: 7ab8d39 · View workflow run →

[qodo-code-review]

PR Summary by Qodo

Disable commit signing in ephemeral git repos used by tests/fixtures

🧪 Tests ⚙️ Configuration changes 🕐 20-40 Minutes

AI Description

• Force commit.gpgsign=false for git subprocesses spawned by Go unit tests.
• Add go-test Makefile guardrails so local gitsign/gpgsign config won’t break CI-like runs.
• Disable signing in eval fixture repos without changing developers’ real commit behavior.

Diagram

graph TD
  A["Makefile: go-test"] --> B["go test ./..."] --> C["GIT_CONFIG_* (gpgsign=false)"] --> D["git subprocesses"] --> E[("Temp test repo")]
  F["eval/scripts/setup-fixture.sh"] --> G["git config commit.gpgsign=false"] --> H[("Eval fixture repo")]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use `git -c commit.gpgsign=false ...` on every test/fixture git call

• ➕ Very explicit and localized per invocation
• ➕ Avoids reliance on GIT_CONFIG_COUNT semantics
• ➖ Easy to miss new git call sites over time
• ➖ More verbose/duplicative across helpers/tests/scripts

2. Ignore global config via `GIT_CONFIG_GLOBAL=/dev/null` (and/or system config)

• ➕ Broadly prevents any global signing settings from leaking into tests
• ➕ Can reduce surprises from other global git settings too
• ➖ May unintentionally hide other global git config that tests rely on
• ➖ More invasive than just disabling signing

3. Disable the signer specifically (e.g., gitsign-specific env/config)

• ➕ Targets the root cause when gitsign is the only problem
• ➕ Can preserve other signing-related settings
• ➖ Tool-specific and less portable (not all environments use gitsign)
• ➖ May not cover GPG signing paths if they’re enabled globally

Recommendation: The PR’s approach is the best balance: it disables signing in two layers (env override + repo-local config) for ephemeral repos while leaving real developer/global behavior unchanged. Keeping the Makefile safeguard also helps future tests that spawn git without going through the same helper.

Files changed (3) +25 / -25

Tests (1) +21 / -24

gitfetch_test.goCentralize test git env and disable signing in temp repos +21/-24

Centralize test git env and disable signing in temp repos

• Introduces 'testGitEnv()' to standardize author/committer identity and inject 'GIT_CONFIG_*' overrides to disable commit signing for git commands run by tests. Also sets 'commit.gpgsign=false' in newly initialized repos (and a cloned work repo) to ensure signing is disabled even if environment propagation differs.

internal/gitfetch/gitfetch_test.go

Other (2) +4 / -1

MakefileDisable git commit signing during 'make go-test' +3/-1

Disable git commit signing during 'make go-test'

• Updates the 'go-test' target to export 'GIT_CONFIG_*' env vars that force 'commit.gpgsign=false' for any git subprocesses invoked during tests. This prevents gitsign/GPG signing hooks from being triggered during local test runs.

Makefile

setup-fixture.shTurn off commit signing in eval fixture repos +1/-0

Turn off commit signing in eval fixture repos

• After cloning the ephemeral fixture repository, sets 'commit.gpgsign=false' in the repo’s local git config. This ensures eval fixtures don’t attempt to sign commits when a developer has signing enabled globally.

eval/scripts/setup-fixture.sh

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 54 rules

✅ Skills: writing-user-docs, writing-adrs

1. Redundant signing disable ✗ Dismissed 🐞 Bug ⚙ Maintainability

Description

internal/gitfetch/gitfetch_test.go disables signing both via GIT_CONFIG_* in testGitEnv() and via
repo-local git config commit.gpgsign false after git init, which is redundant and makes it
unclear which mechanism is intended to be authoritative.

Code

internal/gitfetch/gitfetch_test.go[R19-30]

+// testGitEnv returns environment variables for ephemeral test repos,
+// including signing disable so global gitsign/gpgsign config is not invoked.
+func testGitEnv() []string {
+	return []string{
+		"GIT_AUTHOR_NAME=test",
+		"GIT_AUTHOR_EMAIL=test@test.com",
+		"GIT_COMMITTER_NAME=test",
+		"GIT_COMMITTER_EMAIL=test@test.com",
+		"GIT_CONFIG_COUNT=1",
+		"GIT_CONFIG_KEY_0=commit.gpgsign",
+		"GIT_CONFIG_VALUE_0=false",
+	}

Relevance

⭐⭐⭐ High

Team often accepts removing redundant/duplicate config for clarity (e.g., redundant env removed in
PR #586; dedup in #1825).

PR-#586
PR-#1825
PR-#2630

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The file introduces GIT_CONFIG-based signing disable in testGitEnv(), but also sets the same
config in the initialized repos before committing, meaning one of these mechanisms is unnecessary
within the test suite.

internal/gitfetch/gitfetch_test.go[19-31]
internal/gitfetch/gitfetch_test.go[54-56]
internal/gitfetch/gitfetch_test.go[442-444]
internal/gitfetch/gitfetch_test.go[629-640]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`testGitEnv()` injects `GIT_CONFIG_COUNT/KEY/VALUE` to force `commit.gpgsign=false`, but the tests also explicitly set `commit.gpgsign=false` in the repo config after `git init`. Keeping both mechanisms increases maintenance burden and can confuse future contributors about which layer is required.

## Issue Context
Repo-local config persists for the ephemeral repo and covers any git invocations that might not use `testGitEnv()`. The per-invocation env approach is therefore not needed inside these tests if the repo-local config is consistently applied before commits.

## Fix Focus Areas
- internal/gitfetch/gitfetch_test.go[19-31]
- internal/gitfetch/gitfetch_test.go[54-56]
- internal/gitfetch/gitfetch_test.go[442-444]
- internal/gitfetch/gitfetch_test.go[629-640]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[fullsend-ai-review]

Looks good to me

[rh-hemartin]

How this relates to #3858? There is no signing in place yet, right?

[ifireball]

How this relates to #3858? There is no signing in place yet, right?

It happens on my machine because I enabled signing globally

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 7:52 AM UTC · Completed 8:00 AM UTC
Commit: 7ab8d39 · View workflow run →

[fullsend-ai-retro]

PR #4301 was a well-scoped test infrastructure change that disabled commit signing in ephemeral test repos. The review agent correctly approved it in ~7 minutes with zero findings, and the PR merged with two human approvals. Two improvement opportunities were identified: (1) the review agent re-read changed files 8 times unnecessarily across sub-agent boundaries, wasting ~32% of file reads on duplicates; (2) a human reviewer surfaced a valuable contextual question about the relationship to open PR #3858 (gitsign feature) that the review agent did not identify.

Proposals filed

• Review agent sub-agents re-read files already loaded by parent orchestrator (in fullsend-ai/agents)
• Review agent should surface related open PRs when a change anticipates an unreferenced in-flight feature (in fullsend-ai/agents)