From 3463058475d2d6686c1de342a4c624dafc444792 Mon Sep 17 00:00:00 2001 From: Christopher Tso Date: Mon, 22 Jun 2026 12:23:12 +0200 Subject: [PATCH] fix(results): preserve configured git identity --- packages/core/src/evaluation/results-repo.ts | 121 +++++++--- .../core/test/evaluation/results-repo.test.ts | 209 ++++++++++++++++++ 2 files changed, 305 insertions(+), 25 deletions(-) diff --git a/packages/core/src/evaluation/results-repo.ts b/packages/core/src/evaluation/results-repo.ts index fedce270f..0d98a6c01 100644 --- a/packages/core/src/evaluation/results-repo.ts +++ b/packages/core/src/evaluation/results-repo.ts @@ -35,16 +35,30 @@ const RESULTS_REPO_METADATA_DIR = 'metadata'; // Top-level directories AgentV owns on the results branch. The auto-sync // dirty-commit path stages only these so it never touches unrelated repo files. const RESULTS_REPO_TRACKED_DIRS = [RESULTS_REPO_RUNS_DIR, RESULTS_REPO_METADATA_DIR] as const; -const RESULTS_REPO_COMMIT_EMAIL = 'agentv@results-repo'; -const RESULTS_REPO_COMMIT_NAME = 'AgentV Results'; +const FALLBACK_RESULTS_REPO_COMMIT_EMAIL = 'agentv@results-repo'; +const FALLBACK_RESULTS_REPO_COMMIT_NAME = 'AgentV Results'; +const GIT_COMMIT_IDENTITY_ENV_KEYS = [ + 'GIT_AUTHOR_NAME', + 'GIT_AUTHOR_EMAIL', + 'GIT_COMMITTER_NAME', + 'GIT_COMMITTER_EMAIL', +] as const; +const GIT_ENV_INHERIT_ALLOWLIST = new Set([ + 'GIT_ASKPASS', + 'GIT_PASSWORD', + 'GIT_SSH_COMMAND', + 'GIT_TOKEN', + 'GIT_USERNAME', +]); export const DEFAULT_RESULTS_BRANCH = AGENTV_RESULTS_PRIMARY_REF; const GIT_EMPTY_TREE = '4b825dc642cb6eb9a060e54bf8d69288fbee4904'; // The results branch is a self-rooted orphan whose first commit is a fixed, -// byte-identical empty-tree genesis. Pinning the message, identity (see -// ensureResultsRepoCommitIdentity), and author/committer dates makes the root -// commit SHA deterministic across every machine and clone, so all clients share +// byte-identical empty-tree genesis. Pinning the message, identity, and dates +// through per-command env makes the root commit SHA deterministic across every +// machine and clone, so all clients share // one genesis and fast-forward/append to a single ref instead of each minting a -// divergent root. See createOrphanResultsBranch. +// divergent root. This identity is applied via per-command env only so AgentV +// never overwrites the user's git config. See createOrphanResultsBranch. const RESULTS_REPO_GENESIS_MESSAGE = 'chore(results): initialize AgentV results branch'; const RESULTS_REPO_GENESIS_DATE = '@0 +0000'; const RESULT_INDEX_FILENAME = 'index.jsonl'; @@ -254,7 +268,7 @@ async function runCommand( function getGitEnv(): NodeJS.ProcessEnv { const env: NodeJS.ProcessEnv = {}; for (const [key, value] of Object.entries(process.env)) { - if (value !== undefined && !(key.startsWith('GIT_') && key !== 'GIT_SSH_COMMAND')) { + if (value !== undefined && (!key.startsWith('GIT_') || GIT_ENV_INHERIT_ALLOWLIST.has(key))) { env[key] = value; } } @@ -276,9 +290,72 @@ async function runGh( return runCommand('gh', args, options); } -async function ensureResultsRepoCommitIdentity(repoDir: string): Promise { - await runGit(['config', 'user.email', RESULTS_REPO_COMMIT_EMAIL], { cwd: repoDir }); - await runGit(['config', 'user.name', RESULTS_REPO_COMMIT_NAME], { cwd: repoDir }); +function gitErrorText(error: unknown): string { + const parts: string[] = []; + if (error && typeof error === 'object') { + const record = error as { stdout?: unknown; stderr?: unknown; message?: unknown }; + if (typeof record.stdout === 'string') parts.push(record.stdout); + if (typeof record.stderr === 'string') parts.push(record.stderr); + if (typeof record.message === 'string') parts.push(record.message); + } else if (typeof error === 'string') { + parts.push(error); + } + return parts.join('\n').toLowerCase(); +} + +function isMissingGitIdentityError(error: unknown): boolean { + const text = gitErrorText(error); + return ( + text.includes('author identity unknown') || + text.includes('committer identity unknown') || + text.includes('please tell me who you are') || + text.includes('unable to auto-detect email address') || + text.includes('empty ident name') + ); +} + +function fallbackResultsRepoCommitEnv(): NodeJS.ProcessEnv { + return { + GIT_AUTHOR_NAME: FALLBACK_RESULTS_REPO_COMMIT_NAME, + GIT_AUTHOR_EMAIL: FALLBACK_RESULTS_REPO_COMMIT_EMAIL, + GIT_COMMITTER_NAME: FALLBACK_RESULTS_REPO_COMMIT_NAME, + GIT_COMMITTER_EMAIL: FALLBACK_RESULTS_REPO_COMMIT_EMAIL, + }; +} + +function configuredGitCommitIdentityEnv(): NodeJS.ProcessEnv { + const env: NodeJS.ProcessEnv = {}; + for (const key of GIT_COMMIT_IDENTITY_ENV_KEYS) { + const value = process.env[key]; + if (value !== undefined) { + env[key] = value; + } + } + return env; +} + +async function runGitWithFallbackCommitIdentity( + args: readonly string[], + options: { cwd: string; env?: NodeJS.ProcessEnv }, +): Promise<{ stdout: string; stderr: string }> { + const env = { + ...configuredGitCommitIdentityEnv(), + ...options.env, + }; + try { + return await runGit(args, { ...options, env }); + } catch (error) { + if (!isMissingGitIdentityError(error)) { + throw error; + } + return runGit(args, { + ...options, + env: { + ...env, + ...fallbackResultsRepoCommitEnv(), + }, + }); + } } async function resolveDefaultBranch(repoDir: string): Promise { @@ -432,16 +509,12 @@ async function createOrphanResultsBranch(repoDir: string, branch: string): Promi // identity, and author/committer dates are all fixed, so two inits at different // wall-clock times — on different machines — produce the identical root SHA. async function createResultsGenesisCommit(repoDir: string): Promise { - await ensureResultsRepoCommitIdentity(repoDir); const { stdout } = await runGit( ['commit-tree', GIT_EMPTY_TREE, '-m', RESULTS_REPO_GENESIS_MESSAGE], { cwd: repoDir, env: { - GIT_AUTHOR_NAME: RESULTS_REPO_COMMIT_NAME, - GIT_AUTHOR_EMAIL: RESULTS_REPO_COMMIT_EMAIL, - GIT_COMMITTER_NAME: RESULTS_REPO_COMMIT_NAME, - GIT_COMMITTER_EMAIL: RESULTS_REPO_COMMIT_EMAIL, + ...fallbackResultsRepoCommitEnv(), GIT_AUTHOR_DATE: RESULTS_REPO_GENESIS_DATE, GIT_COMMITTER_DATE: RESULTS_REPO_GENESIS_DATE, }, @@ -1142,8 +1215,7 @@ export async function syncResultsRepoForProject(config: ResultsConfig): Promise< if (inspection.syncStatus === 'dirty') { const trackedDirs = await existingTrackedResultsDirs(repoDir); await runGit(['add', '--all', '--', ...trackedDirs], { cwd: repoDir }); - await ensureResultsRepoCommitIdentity(repoDir); - await runGit( + await runGitWithFallbackCommitIdentity( ['commit', '-m', 'chore(results): sync local result metadata', '--', ...trackedDirs], { cwd: repoDir, @@ -1357,8 +1429,9 @@ export async function commitAndPushResultsBranch(params: { return false; } - await ensureResultsRepoCommitIdentity(params.repoDir); - await runGit(['commit', '-m', params.commitMessage], { cwd: params.repoDir }); + await runGitWithFallbackCommitIdentity(['commit', '-m', params.commitMessage], { + cwd: params.repoDir, + }); await runGit(['push', '-u', 'origin', params.branchName], { cwd: params.repoDir }); return true; } @@ -1873,7 +1946,6 @@ async function commitResultsRunWithTemporaryIndex(params: { }; } - await ensureResultsRepoCommitIdentity(params.repoDir); const commitArgs = [ 'commit-tree', newTree, @@ -1883,7 +1955,9 @@ async function commitResultsRunWithTemporaryIndex(params: { '-m', `AgentV-Run: ${params.targetRunId}`, ]; - const { stdout: commitStdout } = await runGit(commitArgs, { cwd: params.repoDir }); + const { stdout: commitStdout } = await runGitWithFallbackCommitIdentity(commitArgs, { + cwd: params.repoDir, + }); const commitSha = commitStdout.trim(); await runGit( [ @@ -2391,9 +2465,6 @@ export async function setupWipWorktree(params: { await runGit(['worktree', 'add', '-B', params.wipBranch, worktreeDir, baseRef], { cwd: cloneDir, }); - // Ensure commits work even without a global git user config. - await runGit(['config', 'user.email', 'agentv@wip-checkpoint'], { cwd: worktreeDir }); - await runGit(['config', 'user.name', 'AgentV WIP Checkpoint'], { cwd: worktreeDir }); return { wipBranch: params.wipBranch, worktreeDir, @@ -2443,7 +2514,7 @@ export async function pushWipCheckpoint(params: { return false; } const timestamp = new Date().toISOString(); - await runGit( + await runGitWithFallbackCommitIdentity( ['commit', '--amend', '-m', `wip(results): checkpoint ${params.handle.wipBranch} ${timestamp}`], { cwd: params.handle.worktreeDir }, ); diff --git a/packages/core/test/evaluation/results-repo.test.ts b/packages/core/test/evaluation/results-repo.test.ts index 826936ef0..7d09e835f 100644 --- a/packages/core/test/evaluation/results-repo.test.ts +++ b/packages/core/test/evaluation/results-repo.test.ts @@ -194,6 +194,64 @@ function writeRunArtifactsWithPointers( ); } +const GIT_COMMIT_IDENTITY_ENV_KEYS = [ + 'GIT_AUTHOR_NAME', + 'GIT_AUTHOR_EMAIL', + 'GIT_COMMITTER_NAME', + 'GIT_COMMITTER_EMAIL', +] as const; + +async function withGitCommitIdentityEnv( + identity: Partial>, + fn: () => Promise, +): Promise { + const previous: Partial> = {}; + for (const key of GIT_COMMIT_IDENTITY_ENV_KEYS) { + previous[key] = process.env[key]; + const value = identity[key]; + if (value === undefined) { + process.env[key] = undefined; + } else { + process.env[key] = value; + } + } + try { + return await fn(); + } finally { + for (const key of GIT_COMMIT_IDENTITY_ENV_KEYS) { + const value = previous[key]; + if (value === undefined) { + process.env[key] = undefined; + } else { + process.env[key] = value; + } + } + } +} + +async function withIsolatedGitHome(rootDir: string, fn: () => Promise): Promise { + const previousHome = process.env.HOME; + const previousXdgConfigHome = process.env.XDG_CONFIG_HOME; + const homeDir = path.join(rootDir, 'isolated-home'); + mkdirSync(path.join(homeDir, '.config'), { recursive: true }); + process.env.HOME = homeDir; + process.env.XDG_CONFIG_HOME = path.join(homeDir, '.config'); + try { + return await fn(); + } finally { + if (previousHome === undefined) { + process.env.HOME = undefined; + } else { + process.env.HOME = previousHome; + } + if (previousXdgConfigHome === undefined) { + process.env.XDG_CONFIG_HOME = undefined; + } else { + process.env.XDG_CONFIG_HOME = previousXdgConfigHome; + } + } +} + describe('listGitRuns', () => { let repoDir: string; @@ -479,6 +537,157 @@ describe('results repo write path', () => { expect(git('git status --short --branch', projectDir)).toContain('## main'); }, 20000); + it('uses the configured git identity for result commits without overwriting it', async () => { + const projectDir = path.join(rootDir, 'source-project-human-author'); + mkdirSync(projectDir, { recursive: true }); + git('git init --initial-branch=main --quiet', projectDir); + git('git config user.email "human@example.com"', projectDir); + git('git config user.name "Human Author"', projectDir); + writeFileSync(path.join(projectDir, 'README.md'), '# source project\n'); + git('git add README.md && git commit --quiet -m "seed source"', projectDir); + + const runTimestamp = '2026-06-17T10-05-00-000Z'; + const runDir = path.join( + projectDir, + '.agentv', + 'results', + 'runs', + 'human-author', + runTimestamp, + ); + writeRunArtifacts(runDir, 'human-author', '2026-06-17T10:05:00.000Z'); + + const published = await directPushResults({ + config: { + repo_path: projectDir, + branch: DEFAULT_RESULTS_BRANCH, + sync: { auto_push: false }, + }, + sourceDir: runDir, + destinationPath: path.join('human-author', runTimestamp), + commitMessage: 'feat(results): human-author - 1/1 PASS (1.000)', + }); + + expect(published).toBe(true); + expect(git('git config --local user.name', projectDir)).toBe('Human Author'); + expect(git('git config --local user.email', projectDir)).toBe('human@example.com'); + expect(git(`git log -1 --format="%an <%ae>" ${DEFAULT_RESULTS_BRANCH}`, projectDir)).toBe( + 'Human Author ', + ); + }, 20000); + + it('uses git identity from the environment without writing local config', async () => { + await withIsolatedGitHome(rootDir, async () => { + await withGitCommitIdentityEnv( + { + GIT_AUTHOR_NAME: 'Env Author', + GIT_AUTHOR_EMAIL: 'env-author@example.com', + GIT_COMMITTER_NAME: 'Env Committer', + GIT_COMMITTER_EMAIL: 'env-committer@example.com', + }, + async () => { + const projectDir = path.join(rootDir, 'source-project-env-author'); + mkdirSync(projectDir, { recursive: true }); + git('git init --initial-branch=main --quiet', projectDir); + writeFileSync(path.join(projectDir, 'README.md'), '# source project\n'); + git( + 'git -c user.email=seed@example.com -c user.name="Seed Author" add README.md', + projectDir, + ); + git( + 'git -c user.email=seed@example.com -c user.name="Seed Author" commit --quiet -m "seed source"', + projectDir, + ); + + const runTimestamp = '2026-06-17T10-07-00-000Z'; + const runDir = path.join( + projectDir, + '.agentv', + 'results', + 'runs', + 'env-author', + runTimestamp, + ); + writeRunArtifacts(runDir, 'env-author', '2026-06-17T10:07:00.000Z'); + + const published = await directPushResults({ + config: { + repo_path: projectDir, + branch: DEFAULT_RESULTS_BRANCH, + sync: { auto_push: false }, + }, + sourceDir: runDir, + destinationPath: path.join('env-author', runTimestamp), + commitMessage: 'feat(results): env-author - 1/1 PASS (1.000)', + }); + + expect(published).toBe(true); + expect(git('git config --local --get user.name || true', projectDir)).toBe(''); + expect(git('git config --local --get user.email || true', projectDir)).toBe(''); + expect( + git(`git log -1 --format="%an <%ae>|%cn <%ce>" ${DEFAULT_RESULTS_BRANCH}`, projectDir), + ).toBe('Env Author |Env Committer '); + }, + ); + }); + }, 20000); + + it('falls back to AgentV identity only when git has no configured identity', async () => { + await withGitCommitIdentityEnv( + { + GIT_AUTHOR_NAME: undefined, + GIT_AUTHOR_EMAIL: undefined, + GIT_COMMITTER_NAME: undefined, + GIT_COMMITTER_EMAIL: undefined, + }, + async () => { + await withIsolatedGitHome(rootDir, async () => { + const projectDir = path.join(rootDir, 'source-project-fallback-author'); + mkdirSync(projectDir, { recursive: true }); + git('git init --initial-branch=main --quiet', projectDir); + writeFileSync(path.join(projectDir, 'README.md'), '# source project\n'); + git( + 'git -c user.email=seed@example.com -c user.name="Seed Author" add README.md', + projectDir, + ); + git( + 'git -c user.email=seed@example.com -c user.name="Seed Author" commit --quiet -m "seed source"', + projectDir, + ); + + const runTimestamp = '2026-06-17T10-10-00-000Z'; + const runDir = path.join( + projectDir, + '.agentv', + 'results', + 'runs', + 'fallback-author', + runTimestamp, + ); + writeRunArtifacts(runDir, 'fallback-author', '2026-06-17T10:10:00.000Z'); + + const published = await directPushResults({ + config: { + repo_path: projectDir, + branch: DEFAULT_RESULTS_BRANCH, + sync: { auto_push: false }, + }, + sourceDir: runDir, + destinationPath: path.join('fallback-author', runTimestamp), + commitMessage: 'feat(results): fallback-author - 1/1 PASS (1.000)', + }); + + expect(published).toBe(true); + expect(git('git config --local --get user.name || true', projectDir)).toBe(''); + expect(git('git config --local --get user.email || true', projectDir)).toBe(''); + expect(git(`git log -1 --format="%an <%ae>" ${DEFAULT_RESULTS_BRANCH}`, projectDir)).toBe( + 'AgentV Results ', + ); + }); + }, + ); + }, 20000); + it('publishes to an explicit external local repo path', async () => { const projectDir = path.join(rootDir, 'project'); const resultsRepoDir = path.join(rootDir, 'local-results-repo');