From 632e60768ffe210b14aef4db774f5dbc56324dc1 Mon Sep 17 00:00:00 2001 From: coding-agent-loop Date: Mon, 24 Aug 2026 20:22:13 -0400 Subject: [PATCH] Give agent-authored commits their own git identity Adds a config-driven git.author_name/git.author_email so the loop's commits are visibly distinct from the human repo owner's, closing #5. The identity is written into the shared clone's own git config (so it covers commits Claude makes directly, not just the harness's fallback commit) and also passed to the Claude subprocess as GIT_AUTHOR_*/GIT_COMMITTER_* env vars for defense in depth. Co-Authored-By: Claude Sonnet 5 --- README.md | 12 +++++- cmd/agent.go | 12 +++--- config.example.json | 4 ++ internal/claude/runner.go | 5 +++ internal/claude/runner_test.go | 19 +++++++++ internal/config/config.go | 25 +++++++++++ internal/config/config_test.go | 41 ++++++++++++++++++ internal/git/workspace.go | 36 +++++++++++++++- internal/git/workspace_test.go | 64 ++++++++++++++++++++++++++++ internal/orchestrator/loop.go | 1 + internal/orchestrator/prompt.go | 2 + internal/orchestrator/prompt_test.go | 11 +++++ 12 files changed, 224 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 20d4148..61e7c10 100644 --- a/README.md +++ b/README.md @@ -225,7 +225,11 @@ flight at a time), controlled by `run.max_concurrent_repos`. in the PR body so a human sees it immediately. 7. **Deliver** — the remote is re-checked, then pushed; a draft PR is opened with `Closes #`, verification result, model used, and cost; the issue is commented with the PR link; - `agent-working` and `agent-planned` are swapped for `agent-done` or `agent-failed`. + `agent-working` and `agent-planned` are swapped for `agent-done` or `agent-failed`. Every commit on + the branch — whether made by the harness or by Claude itself during step 5 — carries the + `git.author_name`/`git.author_email` identity, not your own; the PR itself is still opened under + the `gh` token's account, since that is a GitHub-level attribution the identity config does not + touch. 8. **Cleanup** — the worktree is removed (kept on disk if `workspace.keep_failed` and the run failed, for post-mortem). The claim is always released, on every exit path. @@ -302,6 +306,10 @@ This repository's own `config.json` is also **compiled into the binary** at buil "enabled": false, "webhook_url": "" }, + "git": { + "author_name": "coding-agent-loop[bot]", + "author_email": "coding-agent-loop@users.noreply.github.com" + }, "models_path": "models.json" } ``` @@ -336,6 +344,8 @@ This repository's own `config.json` is also **compiled into the binary** at buil | `store.path` | SQLite database path | | `discord.enabled` | turn on Discord status notifications (see [Discord notifications](#discord-notifications)) | | `discord.webhook_url` | the channel's incoming webhook URL; **required** if `discord.enabled` is true | +| `git.author_name` | commit author name for work the loop produces; distinct from your own so agent commits are easy to spot; empty falls back to `coding-agent-loop[bot]` | +| `git.author_email` | commit author email to go with `git.author_name`; empty falls back to `coding-agent-loop@users.noreply.github.com` | | `models_path` | where to look for `models.json`; see [Embedded defaults](#embedded-defaults) for how the default value is resolved | ### models.json diff --git a/cmd/agent.go b/cmd/agent.go index 2dedbf2..37ec655 100644 --- a/cmd/agent.go +++ b/cmd/agent.go @@ -173,11 +173,13 @@ func run(f flags) error { ghBinary = abs } gitMgr := &gitpkg.Manager{ - ReposRoot: cfg.Workspace.ReposRoot, - WorkRoot: cfg.Workspace.Root, - GHBinary: ghBinary, - DryRun: suppressMutations, - Log: func(format string, args ...any) { log.Info(fmt.Sprintf(format, args...)) }, + ReposRoot: cfg.Workspace.ReposRoot, + WorkRoot: cfg.Workspace.Root, + AuthorName: cfg.Git.AuthorName, + AuthorEmail: cfg.Git.AuthorEmail, + GHBinary: ghBinary, + DryRun: suppressMutations, + Log: func(format string, args ...any) { log.Info(fmt.Sprintf(format, args...)) }, } runner := &claude.Runner{Log: func(format string, args ...any) { log.Debug(fmt.Sprintf(format, args...)) }} gateway := gate.New(st, cfg.Claude, func(format string, args ...any) { log.Info(fmt.Sprintf(format, args...)) }) diff --git a/config.example.json b/config.example.json index 160ffd3..1800cfa 100644 --- a/config.example.json +++ b/config.example.json @@ -52,5 +52,9 @@ "enabled": false, "webhook_url": "" }, + "git": { + "author_name": "coding-agent-loop[bot]", + "author_email": "coding-agent-loop@users.noreply.github.com" + }, "models_path": "models.json" } diff --git a/internal/claude/runner.go b/internal/claude/runner.go index 9448af9..4b2922e 100644 --- a/internal/claude/runner.go +++ b/internal/claude/runner.go @@ -139,6 +139,8 @@ type Options struct { PermissionMode string // WorkDir is the process cwd, i.e. the worktree. WorkDir string + // Env is appended to the subprocess's inherited environment. + Env []string // ExtraArgs are appended verbatim. ExtraArgs []string // LogPath receives the raw JSONL transcript. Required. @@ -224,6 +226,9 @@ func (r *Runner) Run(ctx context.Context, opts Options) (*Result, error) { cmd := exec.CommandContext(ctx, opts.Binary, args...) cmd.Dir = opts.WorkDir + if len(opts.Env) > 0 { + cmd.Env = append(os.Environ(), opts.Env...) + } cmd.Stdin = strings.NewReader(opts.Prompt) // Claude Code spawns children (the bash tool, language servers); kill the // whole group on cancellation or a timed-out run leaves the worker stuck. diff --git a/internal/claude/runner_test.go b/internal/claude/runner_test.go index 7d95d51..03f9bc0 100644 --- a/internal/claude/runner_test.go +++ b/internal/claude/runner_test.go @@ -175,6 +175,25 @@ printf '{"type":"result","subtype":"success","is_error":false,"result":"%s"}\n' } } +// Env must reach the child process, since this is how the harness's commit +// identity is delivered to Claude when it commits on its own. +func TestEnvReachesTheChildProcess(t *testing.T) { + bin := stubCLI(t, `cat > /dev/null +printf '{"type":"result","subtype":"success","is_error":false,"result":"%s"}\n' "$GIT_AUTHOR_NAME" +`) + res, err := (&Runner{}).Run(context.Background(), Options{ + Binary: bin, + LogPath: filepath.Join(t.TempDir(), "run.jsonl"), + Env: []string{"GIT_AUTHOR_NAME=coding-agent-loop[bot]"}, + }) + if err != nil { + t.Fatal(err) + } + if res.Result != "coding-agent-loop[bot]" { + t.Fatalf("Env did not reach the child process, got result %q", res.Result) + } +} + func TestLogPathRequired(t *testing.T) { if _, err := (&Runner{}).Run(context.Background(), Options{Binary: "true"}); err == nil { t.Fatal("LogPath must be required so every run leaves a transcript") diff --git a/internal/config/config.go b/internal/config/config.go index 88c5086..0f4bf61 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -47,6 +47,7 @@ type Config struct { Server ServerConfig `json:"server"` Store StoreConfig `json:"store"` Discord DiscordConfig `json:"discord"` + Git GitConfig `json:"git"` // ModelsPath points at models.json, resolved by the caller (cmd/agent.go), // not expanded here: "models.json" (the default) is looked up next to the @@ -152,6 +153,14 @@ type DiscordConfig struct { WebhookURL string `json:"webhook_url"` } +// GitConfig identifies the author of commits the loop produces, so they are +// visibly distinct from the human repo owner's own commits. Empty means "use +// the built-in default" — see git.Manager's author()/email(). +type GitConfig struct { + AuthorName string `json:"author_name"` + AuthorEmail string `json:"author_email"` +} + // Default returns a Config with every field populated to a sane value. func Default() Config { return Config{ @@ -194,6 +203,10 @@ func Default() Config { Server: ServerConfig{Addr: "127.0.0.1:8787"}, Store: StoreConfig{Path: "~/.agent-loop/state.db"}, Discord: DiscordConfig{Enabled: false}, + Git: GitConfig{ + AuthorName: "coding-agent-loop[bot]", + AuthorEmail: "coding-agent-loop@users.noreply.github.com", + }, } } @@ -289,9 +302,21 @@ func (c *Config) Validate() error { if c.Discord.Enabled && c.Discord.WebhookURL == "" { return fmt.Errorf("discord.webhook_url must be set when discord.enabled is true") } + if containsCommitHeaderBreakingChars(c.Git.AuthorName) { + return fmt.Errorf("git.author_name must not contain '<', '>', or a newline") + } + if containsCommitHeaderBreakingChars(c.Git.AuthorEmail) { + return fmt.Errorf("git.author_email must not contain '<', '>', or a newline") + } return nil } +// containsCommitHeaderBreakingChars reports whether s has a character that +// would corrupt the "Name " line git writes into a commit header. +func containsCommitHeaderBreakingChars(s string) bool { + return strings.ContainsAny(s, "<>\n") +} + // validOwners returns the entries of owners that are non-blank after trimming. func validOwners(owners []string) []string { out := make([]string, 0, len(owners)) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index a8101ab..0e124e3 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -174,3 +174,44 @@ func TestRetryBackoffMaxMustNotBeBelowTheBase(t *testing.T) { t.Fatalf("want a backoff validation error, got %v", err) } } + +// Agent-authored commits should carry their own identity by default, distinct +// from whatever the host's global gitconfig would otherwise resolve to. +func TestGitIdentityDefaults(t *testing.T) { + cfg := Default() + if cfg.Git.AuthorName != "coding-agent-loop[bot]" { + t.Fatalf("unexpected default git author name %q", cfg.Git.AuthorName) + } + if cfg.Git.AuthorEmail != "coding-agent-loop@users.noreply.github.com" { + t.Fatalf("unexpected default git author email %q", cfg.Git.AuthorEmail) + } +} + +func TestGitIdentityOverlaysFromConfig(t *testing.T) { + cfg, err := Load(writeConfig(t, `{"github":{"owners":["acme"]},"git":{"author_name":"Acme Bot","author_email":"bot@acme.example"}}`), false) + if err != nil { + t.Fatal(err) + } + if cfg.Git.AuthorName != "Acme Bot" { + t.Fatalf("author_name = %q", cfg.Git.AuthorName) + } + if cfg.Git.AuthorEmail != "bot@acme.example" { + t.Fatalf("author_email = %q", cfg.Git.AuthorEmail) + } +} + +// A name/email containing '<', '>', or a newline would corrupt the commit +// header line git builds ("Name "). +func TestGitIdentityWithAngleBracketIsRejected(t *testing.T) { + _, err := Load(writeConfig(t, `{"github":{"owners":["acme"]},"git":{"author_name":"evil>name"}}`), false) + if err == nil || !strings.Contains(err.Error(), "author_name") { + t.Fatalf("want a git.author_name validation error, got %v", err) + } +} + +func TestGitIdentityEmailWithAngleBracketIsRejected(t *testing.T) { + _, err := Load(writeConfig(t, `{"github":{"owners":["acme"]},"git":{"author_email":""); strings.TrimSpace(got) != "agent " { + t.Fatalf("commit author = %q, want the configured identity", got) + } // Committed work still counts as work, via the commit-ahead check. if work, err := m.HasWork(ctx, wt, "main"); err != nil || !work { t.Fatalf("committed work should still register: work=%v err=%v", work, err) @@ -191,6 +194,67 @@ func TestWorktreeLifecycle(t *testing.T) { } } +// This is the case the issue actually cares about: Claude, or any other +// subprocess, commits directly with a bare `git commit` (no -c flags, unlike +// CommitAll). EnsureRepo must have already written the harness identity into +// the shared clone's own config, so that commit picks it up instead of +// falling through to the host's global gitconfig — which is what caused +// agent commits to be attributed to the human repo owner. +func TestEnsureRepoAppliesIdentityOverridingGlobalConfig(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + ctx := context.Background() + origin := originRepo(t) + root := t.TempDir() + + globalConfig := filepath.Join(root, "global-gitconfig") + if err := os.WriteFile(globalConfig, []byte("[user]\n\tname = Human Author\n\temail = human@example.com\n"), 0o644); err != nil { + t.Fatal(err) + } + t.Setenv("GIT_CONFIG_GLOBAL", globalConfig) + + m := &Manager{ + ReposRoot: filepath.Join(root, "repos"), + WorkRoot: filepath.Join(root, "work"), + AuthorName: "agent", + AuthorEmail: "agent@example.com", + } + + repoPath, err := m.EnsureRepo(ctx, "acme/widgets", origin) + if err != nil { + if strings.Contains(err.Error(), "unknown option") || strings.Contains(err.Error(), "unknown environment variable") { + t.Skip("git too old to support GIT_CONFIG_GLOBAL") + } + t.Fatalf("EnsureRepo: %v", err) + } + + wt := m.WorktreePath("acme/widgets", 7) + if err := m.AddWorktree(ctx, repoPath, wt, "agent/issue-7", "main"); err != nil { + t.Fatalf("AddWorktree: %v", err) + } + if err := os.WriteFile(filepath.Join(wt, "feature.txt"), []byte("implemented\n"), 0o644); err != nil { + t.Fatal(err) + } + + // Simulate Claude: a bare commit, no -c user.name/user.email. + cmd := exec.Command("git", "add", "-A") + cmd.Dir = wt + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git add: %v\n%s", err, out) + } + cmd = exec.Command("git", "commit", "-m", "add feature") + cmd.Dir = wt + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git commit: %v\n%s", err, out) + } + + got := git(t, wt, "log", "-1", "--format=%an <%ae>") + if strings.TrimSpace(got) != "agent " { + t.Fatalf("bare commit author = %q, want the harness identity, not the host's global gitconfig", got) + } +} + // AssertRemote is the last gate before anything leaves the machine. func TestAssertRemote(t *testing.T) { if _, err := exec.LookPath("git"); err != nil { diff --git a/internal/orchestrator/loop.go b/internal/orchestrator/loop.go index bbbe6c0..4eb0603 100644 --- a/internal/orchestrator/loop.go +++ b/internal/orchestrator/loop.go @@ -675,6 +675,7 @@ func (o *Orchestrator) execute(ctx context.Context, log *slog.Logger, cand candi Fallbacks: fallbacks, PermissionMode: permissionMode, WorkDir: worktree, + Env: o.opts.Git.IdentityEnv(), ExtraArgs: cfg.Claude.ExtraArgs, LogPath: logPath, Timeout: cfg.Run.Timeout.D(), diff --git a/internal/orchestrator/prompt.go b/internal/orchestrator/prompt.go index f10739f..1ff5a61 100644 --- a/internal/orchestrator/prompt.go +++ b/internal/orchestrator/prompt.go @@ -38,6 +38,8 @@ The harness, not you, owns version control and GitHub. Specifically: - Do NOT run git push, git rebase, git reset --hard, or any force operation. - Do NOT create branches, tags, pull requests, or issue comments. - Do NOT amend or rewrite any commit that already exists. +- Do NOT change git's user.name/user.email or pass --author/--reset-author; the harness has + already set the commit identity for this worktree. - You MAY commit your work locally. If you do not, the harness commits it for you. Scope rules: diff --git a/internal/orchestrator/prompt_test.go b/internal/orchestrator/prompt_test.go index 00af875..82dce7d 100644 --- a/internal/orchestrator/prompt_test.go +++ b/internal/orchestrator/prompt_test.go @@ -52,6 +52,17 @@ func TestPlanTaskPromptIncludesThePreviousPlanOnReplan(t *testing.T) { } } +// The harness sets the commit identity for the worktree before Claude runs; +// this stops a helpful model from "fixing" an unfamiliar author. +func TestSystemPromptForbidsChangingGitIdentity(t *testing.T) { + p := systemPrompt("acme/widgets", "agent/issue-9", "/work/widgets/issue-9") + for _, want := range []string{"user.name", "user.email", "--author", "already set the commit identity"} { + if !strings.Contains(p, want) { + t.Errorf("system prompt missing %q:\n%s", want, p) + } + } +} + func TestPlanSystemPromptForbidsEditing(t *testing.T) { p := planSystemPrompt("acme/widgets", "/work/widgets/issue-9") for _, want := range []string{"Do NOT edit", "Do NOT", "plan"} {