From 0a62c7d8d639fa03ae507c4fd1fc3093b06871a6 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Thu, 24 Sep 2026 23:47:08 -0400 Subject: [PATCH 1/5] Sign the commits a run's own workers make (placeholder for the PR number) Co-Authored-By: Claude Opus 5.5 (1M context) From b33199b2de7e0dbeff40f4a5a595c4f406401b3b Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 25 Sep 2026 10:30:52 -0400 Subject: [PATCH 2/5] exec: one reading of a CONTRIBUTING ban and a sign-once message SignCommitMessageOnce leaves a message that already carries both codeaf lines byte for byte, gives a message with neither the same two-line block SignCommitMessage writes, and gives a message with one of them only the line it lacks. ContributingRefusesTrailers is the one conservative reading of a repository's ban on AI trailers: a sentence is a ban only when it names attribution, names AI and prohibits, and a requirement to add or keep the lines (including "do not remove") always wins. Co-Authored-By: Claude Opus 5.5 (1M context) --- internal/exec/attribution_contributing.go | 44 +++++++++++++++++++ .../exec/attribution_contributing_test.go | 40 +++++++++++++++++ internal/exec/attribution_test.go | 22 ++++++++++ internal/exec/linear.go | 23 ++++++++++ 4 files changed, 129 insertions(+) create mode 100644 internal/exec/attribution_contributing.go create mode 100644 internal/exec/attribution_contributing_test.go diff --git a/internal/exec/attribution_contributing.go b/internal/exec/attribution_contributing.go new file mode 100644 index 000000000..9fc8b6b8d --- /dev/null +++ b/internal/exec/attribution_contributing.go @@ -0,0 +1,44 @@ +package exec + +import ( + "regexp" + "strings" +) + +var contributingSentence = regexp.MustCompile(`[.!?;]\s+|\n+`) +var contributingDottedAI = regexp.MustCompile(`\ba\.i\.`) +var contributingAttribution = regexp.MustCompile(`\b(?:trailer\w*|attribution\w*|co-authored-by|co-authored|coauthored|co-author\w*|assisted-by|generated-by|generated with|generated by|footer\w*)\b`) +var contributingAI = regexp.MustCompile(`(?:\ba\.i\.|\b(?:ai|ai-generated|ai-assisted|llm\w*|assistant\w*|agent\w*|bots?|copilot|claude|chatgpt|gpt\w*|codeaf|machine-generated|assisted-by|generated-by)\b)`) +var contributingProhibition = regexp.MustCompile(`\b(?:do not|don't|dont|never|must not|mustn't|should not|shouldn't|may not|cannot|can't|not allowed|not permitted|not accepted|forbid\w*|prohibit\w*|bans?|banned|refrain|avoid|remove|strip|omit|delete|drop|edit out|take out|reject\w*)\b`) +var contributingRequirement = regexp.MustCompile(`\b(?:without|with no|missing|lack\w*|must include|must add|must contain|must have|must carry|required|require|requires|mandatory|please add|please include|should include|should add|always add|always include|keep|preserve|retain|leave\b[^.!?;\n]*\bin place)\b`) +var contributingNegatedRemoval = regexp.MustCompile(`\b(?:do not|don't|dont|never|must not|mustn't|should not|shouldn't|may not|cannot|can't)(?:\s+[\w-]+){0,2}\s+(?:remove|strip|omit|delete|drop|edit out|take out)\b`) +var contributingNegatedKeeping = regexp.MustCompile(`\b(?:do not|don't|dont|never|must not|mustn't|should not|shouldn't|may not|cannot|can't)(?:\s+[\w-]+){0,2}\s+(?:keep|preserve|retain|leave\b[^.!?;\n]*\bin place)\b`) +var contributingNo = regexp.MustCompile(`\bno(?:\s+[\w-]+){0,2}\s+(?:trailers?|attributions?|co-authored-by|co-authored|coauthored|co-authors?|assisted-by|generated-by|generated|footers?|ai|a\.i\.|llms?|assistants?|agents?|bots?|copilot|claude|chatgpt|gpt[\w-]*|codeaf|machine-generated)\b`) + +// ContributingRefusesTrailers conservatively reads a repository's own ban on +// AI attribution. A requirement to add or keep a trailer wins over negative +// wording such as "do not submit code without one" or "do not remove it" in +// the same sentence: neither asks the harness to omit attribution. +func ContributingRefusesTrailers(text string) bool { + // Keep A.I. together before periods followed by spaces separate sentences. + text = contributingDottedAI.ReplaceAllString(strings.ToLower(text), "ai") + for _, sentence := range contributingSentence.Split(text, -1) { + sentence = strings.Map(func(r rune) rune { + if strings.ContainsRune("*_`~", r) { + return -1 + } + return r + }, sentence) + // A negated keeping verb is itself a ban, while a negated removal + // verb asks that attribution stay on the work. + if contributingNegatedRemoval.MatchString(sentence) || + (contributingRequirement.MatchString(sentence) && !contributingNegatedKeeping.MatchString(sentence)) { + continue + } + if contributingAttribution.MatchString(sentence) && contributingAI.MatchString(sentence) && + (contributingProhibition.MatchString(sentence) || contributingNo.MatchString(sentence)) { + return true + } + } + return false +} diff --git a/internal/exec/attribution_contributing_test.go b/internal/exec/attribution_contributing_test.go new file mode 100644 index 000000000..64a4e5011 --- /dev/null +++ b/internal/exec/attribution_contributing_test.go @@ -0,0 +1,40 @@ +package exec + +import "testing" + +// Contract 4: a CONTRIBUTING requirement cannot be mistaken for a ban. +func TestContributingRefusesTrailersOnlyForAIBans(t *testing.T) { + for _, row := range []struct { + text string + ban bool + }{ + {"Do not add AI co-author trailers to commits.", true}, + {"Please don't include \"Generated with Claude Code\" or Co-authored-by lines from AI tools.", true}, + {"No AI trailers.", true}, + {"No A.I. trailers.", true}, + {"Commits with AI attribution trailers will be rejected.", true}, + {"Never add Assisted-by lines.", true}, + {"Avoid **AI attribution** footers.", true}, + {"Commits that do not include a Signed-off-by trailer will not be merged.", false}, + {"AI-assisted contributions must include an Assisted-by: trailer.", false}, + {"Commits missing an Assisted-by trailer will be rejected.", false}, + {"Do not submit AI-generated code without an Assisted-by trailer.", false}, + {"Do not remove the Signed-off-by trailer added by the tooling.", false}, + {"Please run the tests before opening a pull request.", false}, + {"Avoid large files. AI attribution trailers are required.", false}, + {"Do not remove the AI co-author trailer.", false}, + {"Never strip Assisted-by lines from commits.", false}, + {"Please keep the Co-authored-by line your AI tool adds.", false}, + {"Preserve AI attribution trailers when squashing.", false}, + {"Remove any AI co-author trailers before submitting.", true}, + {"Strip \"Generated with Claude Code\" footers from pull request descriptions.", true}, + {"Avoid large commits. Use Co-authored-by for pair programming.", false}, + {"Our bot adds a co-author line; do not edit it.", false}, + {"Do not keep AI trailers in commits.", true}, + {"Never preserve Assisted-by lines.", true}, + } { + if got := ContributingRefusesTrailers(row.text); got != row.ban { + t.Errorf("ContributingRefusesTrailers(%q) = %v, want %v", row.text, got, row.ban) + } + } +} diff --git a/internal/exec/attribution_test.go b/internal/exec/attribution_test.go index d8412cd9b..ce8ed2b50 100644 --- a/internal/exec/attribution_test.go +++ b/internal/exec/attribution_test.go @@ -105,6 +105,28 @@ func TestTheTrailerBlockIsTwoExactLinesWithOrWithoutTheModel(t *testing.T) { } } +// Contract 2: a worker's complete message stays byte-exact, and a partial +// message gains just the missing line instead of another trailer block. +func TestSignCommitMessageOncePreservesAndCompletesWorkerTrailers(t *testing.T) { + full := "worker\n\n" + AttributionTrailers("") + "\n" + if got := SignCommitMessageOnce(full, ""); got != full { + t.Fatalf("complete message moved: %q", got) + } + workerFull := "worker\n\n assisted-by: codeaf (worker-model) \n co-authored-by: codeaf <267109073+agentfield-bot@users.noreply.github.com> \n" + if got := SignCommitMessageOnce(workerFull, "different-model"); got != workerFull { + t.Fatalf("case-folded worker lines moved: %q", got) + } + for _, row := range []struct{ message, want string }{ + {"worker\n", SignCommitMessage("worker\n", "")}, + {"worker\n\n" + AttributionAssistedBy + "\n", "worker\n\n" + AttributionTrailers("")}, + {"worker\n\n" + AttributionTrailer + "\n", "worker\n\n" + AttributionTrailer + "\n" + AttributionAssistedBy}, + } { + if got := SignCommitMessageOnce(row.message, ""); got != row.want { + t.Errorf("SignCommitMessageOnce(%q) = %q, want %q", row.message, got, row.want) + } + } +} + // THE SETTINGS ROW'S HINT IS THE TWO LINES THIS PACKAGE WRITES. internal/config // cannot import this package, so it spells them; this holds its spelling to the // one that reaches a commit. diff --git a/internal/exec/linear.go b/internal/exec/linear.go index 5652da6c0..14650d329 100644 --- a/internal/exec/linear.go +++ b/internal/exec/linear.go @@ -277,6 +277,29 @@ func SignCommitMessage(message, model string) string { return strings.TrimRight(message, "\n") + "\n\n" + AttributionTrailers(model) } +// SignCommitMessageOnce keeps a worker's own attribution when it already has +// both lines. A partly signed message gains just the missing line, without +// moving or duplicating the line the worker wrote. +func SignCommitMessageOnce(message, model string) string { + assisted, coauthor := false, false + for _, line := range strings.Split(message, "\n") { + line = strings.TrimSpace(line) + assisted = assisted || strings.HasPrefix(strings.ToLower(line), strings.ToLower(AttributionAssistedBy)) + coauthor = coauthor || strings.EqualFold(line, AttributionTrailer) + } + if assisted && coauthor { + return message + } + if !assisted && !coauthor { + return SignCommitMessage(message, model) + } + missing := AttributionTrailer + if !assisted { + missing = AssistedBy(model) + } + return strings.TrimRight(message, "\n") + "\n" + missing +} + // BareModelName is a model id as the `Assisted-by` line names it: the model and // nothing about who served it or how. // From 86c8c0ed2769a476f87935af58c704a06b8d2203 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 25 Sep 2026 10:30:59 -0400 Subject: [PATCH 3/5] run landing: sign the commits a run's own workers made, and name the work they committed On the worker harness a commit the worker made itself carried no attribution: the landing signed only the work still uncommitted when the run ended. Now, before its work goes home, the landing walks the commits the run made on its own branch since its base and gives each one that lacks them the same bare Assisted-by and co-author lines, as a message-only rewrite (commit-tree with the original tree, author and committer, then one compare-and-swap update-ref), so the tree, the index and the repository's own hooks are untouched. Commits another branch, tag, remote-tracking ref or FETCH_HEAD reaches, and a range holding a gpgsig/mergetag/encoding header, are left exactly as they are. A repository whose CONTRIBUTING forbids AI trailers gets none, from the rewrite or from the landing's own commit. The copy record keeps the run's base across a restart, and a run whose worker committed everything (or committed and reverted) now names its branch and files instead of saying nothing to land. Co-Authored-By: Claude Opus 5.5 (1M context) --- internal/remote/hosted_stop_test.go | 2 +- internal/run/enginewire.go | 4 +- internal/run/land.go | 21 +- internal/run/land_test.go | 120 ++++++++- internal/session/beltfacts.go | 21 +- internal/session/land_run_tree.go | 106 ++++++-- internal/session/land_run_tree_test.go | 36 ++- internal/session/run_commit_sign.go | 259 ++++++++++++++++++ internal/session/run_commit_sign_test.go | 308 ++++++++++++++++++++++ internal/session/run_contributing.go | 53 ++++ internal/session/run_lifecycle_test.go | 2 +- internal/session/run_tree_changes.go | 29 +- internal/session/run_tree_changes_test.go | 23 ++ internal/session/task_batch_run_test.go | 2 +- internal/session/task_run.go | 10 +- internal/session/task_run_belt.go | 4 +- internal/session/task_run_belt_test.go | 57 +++- internal/session/task_run_copy.go | 43 +-- internal/session/task_run_copy_test.go | 15 +- 19 files changed, 1043 insertions(+), 72 deletions(-) create mode 100644 internal/session/run_commit_sign.go create mode 100644 internal/session/run_commit_sign_test.go create mode 100644 internal/session/run_contributing.go diff --git a/internal/remote/hosted_stop_test.go b/internal/remote/hosted_stop_test.go index d408ae62e..8cbdf95cb 100644 --- a/internal/remote/hosted_stop_test.go +++ b/internal/remote/hosted_stop_test.go @@ -91,7 +91,7 @@ func (r hostedSlowRun) Start(ctx context.Context, spec session.RunSpec) session. return session.RunSummary{Outcome: "ran and did not finish", Cut: []string{spec.Store.RootID()}, Nodes: 1, Steps: 1} } -func (hostedSlowRun) Land(context.Context, *plandb.Store, string, string) (session.RunLanding, error) { +func (hostedSlowRun) Land(context.Context, *plandb.Store, string, string, string) (session.RunLanding, error) { return session.RunLanding{}, nil } diff --git a/internal/run/enginewire.go b/internal/run/enginewire.go index e29c7649d..6d8ca1b01 100644 --- a/internal/run/enginewire.go +++ b/internal/run/enginewire.go @@ -100,8 +100,8 @@ func runLimitOf(limit Limit) session.RunLimit { return "" } -func (engine) Land(ctx context.Context, store *plandb.Store, workspace, rootID string) (session.RunLanding, error) { - landing, err := Land(ctx, store, workspace, rootID) +func (engine) Land(ctx context.Context, store *plandb.Store, workspace, base, rootID string) (session.RunLanding, error) { + landing, err := Land(ctx, store, workspace, base, rootID) if err != nil { return session.RunLanding{}, err } diff --git a/internal/run/land.go b/internal/run/land.go index 3ed4920fc..4085fe8bd 100644 --- a/internal/run/land.go +++ b/internal/run/land.go @@ -18,11 +18,23 @@ type Landing struct { Refused string } +// THE LANDING IS SIGNED WITH THE BARE `Assisted-by` LINE. The run's store +// records no model on its root, so neither the landing nor its worker commits +// may name a guessed one. +const landingModel = "" + +// SignWork signs an in-place run's worker commits using the landing's model +// choice. The run door itself makes no commit in the person's folder. +func SignWork(before session.RunTreeSnapshot) (int, error) { + return before.SignRunCommits(landingModel) +} + // Land commits a run's working copy onto its branch and writes the answer as a // note on the root task, so the run's own page carries where its work went. // // THE WORK IS THE COPY'S OWN ([session.LandRunTree]), because a run's workers -// edit through bash and leave no ledger: the tree's own status is the record. +// edit through bash and leave no ledger: the tree's status and commits since +// the run's base are the record. // The commit message is the root task's title — the run's own name for the // thing the person asked for. // @@ -30,7 +42,7 @@ type Landing struct { // of a run that only read, and it is written on the root the same way a landing // is. An error is the run having no working copy to land in at all, and then // there is no note to write, because there is nothing about this run to say. -func Land(ctx context.Context, store *plandb.Store, workspace, rootID string) (Landing, error) { +func Land(ctx context.Context, store *plandb.Store, workspace, base, rootID string) (Landing, error) { if err := ctx.Err(); err != nil { return Landing{}, err } @@ -38,10 +50,7 @@ func Land(ctx context.Context, store *plandb.Store, workspace, rootID string) (L if root == nil { return Landing{}, fmt.Errorf("land a run: no task %s in the store", rootID) } - // THE LANDING IS SIGNED WITH THE BARE `Assisted-by` LINE. A run's store - // records no model on its root, and a line naming a guessed one would be a - // provenance claim nobody made. - branch, changed, refusal, err := session.LandRunTree(workspace, root.Title, "") + branch, changed, refusal, err := session.LandRunTree(workspace, base, root.Title, landingModel) if err != nil { return Landing{}, err } diff --git a/internal/run/land_test.go b/internal/run/land_test.go index fa0962842..434f64f4d 100644 --- a/internal/run/land_test.go +++ b/internal/run/land_test.go @@ -60,7 +60,7 @@ func TestLandCommitsTheRunsWorkOntoItsBranch(t *testing.T) { t.Fatalf("outcome = %q, want done", outcome) } - landing, err := run.Land(ctx, store, work, store.RootID()) + landing, err := run.Land(ctx, store, work, base, store.RootID()) if err != nil { t.Fatalf("Land: %v", err) } @@ -102,6 +102,7 @@ func TestLandCommitsTheRunsWorkOntoItsBranch(t *testing.T) { func TestLandRefusesARunThatWroteNothing(t *testing.T) { t.Setenv("CODEAF_TASK_BELT", "bash") repo, work := runLandWorkspace(t) + base := strings.TrimSpace(runGitOut(t, repo, "rev-parse", "work")) store := runOpenStore(t) ctx := runContext(t) before := runGitOut(t, repo, "branch", "--format=%(refname:short)") @@ -110,7 +111,7 @@ func TestLandRefusesARunThatWroteNothing(t *testing.T) { t.Fatalf("outcome = %q, want done", outcome) } - landing, err := run.Land(ctx, store, work, store.RootID()) + landing, err := run.Land(ctx, store, work, base, store.RootID()) if err != nil { t.Fatalf("Land: %v", err) } @@ -129,6 +130,121 @@ func TestLandRefusesARunThatWroteNothing(t *testing.T) { } } +// Contracts 1, 5 and 7: the real supervisor's worker commits on its branch; +// landing signs that commit, leaves its identity and hook alone, and counts its +// changed path even when the worker left no uncommitted work. +func TestLandSignsWorkerCommitAndNamesCommittedWork(t *testing.T) { + for _, leftover := range []bool{false, true} { + name := "committed only" + if leftover { + name = "committed and leftover" + } + t.Run(name, func(t *testing.T) { + t.Setenv("CODEAF_TASK_BELT", "bash") + repo, work := runLandWorkspace(t) + base := strings.TrimSpace(runGitOut(t, repo, "rev-parse", "work")) + hooks := strings.TrimSpace(runGitOut(t, work, "rev-parse", "--git-path", "hooks")) + if !filepath.IsAbs(hooks) { + hooks = filepath.Join(work, hooks) + } + marker := filepath.Join(t.TempDir(), "hook-ran") + hookConfigBefore := runGitOut(t, work, "config", "--local", "--list") + writeRunFile(t, filepath.Join(hooks, "pre-commit"), "#!/bin/sh\nprintf ran >> '"+marker+"'\n") + if err := os.Chmod(filepath.Join(hooks, "pre-commit"), 0o755); err != nil { + t.Fatal(err) + } + store := runOpenStore(t) + seat := newFakeSeat() + seat.actions["root"] = func(_ context.Context, _ plandb.Task) (run.Report, error) { + writeRunFile(t, filepath.Join(work, "worker.txt"), "worker\n") + runGit(t, work, "add", "worker.txt") + runGit(t, work, "-c", "user.name=Worker", "-c", "user.email=worker@example.test", "commit", "-m", "worker") + if leftover { + writeRunFile(t, filepath.Join(work, "leftover.txt"), "leftover\n") + } + return run.Report{Result: "worker finished"}, nil + } + if got := run.NewSupervisor(store, work, 1, run.Limits{}, seat.workerFor).Run(runContext(t)); got != run.OutcomeDone { + t.Fatalf("run outcome = %q", got) + } + if _, err := os.Stat(marker); err != nil { + t.Fatalf("pre-commit hook did not run: %v", err) + } + landing, err := run.Land(runContext(t), store, work, base, store.RootID()) + if err != nil || landing.Refused != "" || landing.Branch != "run-work" { + t.Fatalf("landing = %+v, %v", landing, err) + } + if data, err := os.ReadFile(marker); err != nil || string(data) != "ran" { + t.Fatalf("landing ran the worker's hook again: %q, %v", data, err) + } + if hookConfigAfter := runGitOut(t, work, "config", "--local", "--list"); hookConfigAfter != hookConfigBefore { + t.Fatalf("landing changed the repository's git config:\n%s\n--- before ---\n%s", hookConfigAfter, hookConfigBefore) + } + want := []string{"worker.txt"} + if leftover { + want = []string{"leftover.txt", "worker.txt"} + } + if !reflect.DeepEqual(landing.Changed, want) { + t.Fatalf("changed = %v, want %v", landing.Changed, want) + } + workerRev := "run-work" + if leftover { + workerRev += "~1" + } + message := runGitOut(t, work, "log", "-1", "--format=%B", workerRev) + if !strings.HasSuffix(message, "\n\nAssisted-by: CodeAF\nCo-Authored-By: CodeAF <267109073+agentfield-bot@users.noreply.github.com>\n") || strings.Count(message, "Assisted-by: CodeAF") != 1 { + t.Fatalf("worker commit was not signed once: %q", message) + } + if who := strings.TrimSpace(runGitOut(t, work, "log", "-1", "--format=%an <%ae>|%cn <%ce>", workerRev)); who != "Worker |Worker " { + t.Fatalf("worker identity changed: %q", who) + } + if leftover { + message := runGitOut(t, work, "log", "-1", "--format=%B", "run-work") + if strings.Count(message, "Assisted-by: CodeAF") != 1 || strings.Count(message, "Co-Authored-By: CodeAF") != 1 { + t.Fatalf("landing commit was not signed once: %q", message) + } + } + if now := strings.TrimSpace(runGitOut(t, repo, "rev-parse", "work")); now != base { + t.Fatalf("person's earlier commit moved: %s -> %s", base, now) + } + note := "landed on run-work: 1 file" + if leftover { + note = "landed on run-work: 2 files" + } + if notes := store.Notes(store.RootID(), 0); len(notes) == 0 || notes[len(notes)-1].Body != note { + t.Fatalf("notes = %v, want %q", notes, note) + } + }) + } +} + +// Contract 7: the run note names a file from committed work even if a later +// worker commit reverted its net tree change. +func TestLandNamesCommittedThenRevertedWork(t *testing.T) { + t.Setenv("CODEAF_TASK_BELT", "bash") + repo, work := runLandWorkspace(t) + base := strings.TrimSpace(runGitOut(t, repo, "rev-parse", "work")) + store := runOpenStore(t) + seat := newFakeSeat() + seat.actions["root"] = func(_ context.Context, _ plandb.Task) (run.Report, error) { + writeRunFile(t, filepath.Join(work, "reverted.txt"), "one\n") + runGit(t, work, "add", "reverted.txt") + runGit(t, work, "-c", "user.name=Worker", "-c", "user.email=worker@example.test", "commit", "-m", "add reverted") + runGit(t, work, "-c", "user.name=Worker", "-c", "user.email=worker@example.test", "revert", "--no-edit", "HEAD") + return run.Report{Result: "reverted"}, nil + } + if got := run.NewSupervisor(store, work, 1, run.Limits{}, seat.workerFor).Run(runContext(t)); got != run.OutcomeDone { + t.Fatalf("run outcome = %q", got) + } + landing, err := run.Land(runContext(t), store, work, base, store.RootID()) + if err != nil || landing.Refused != "" || landing.Branch != "run-work" || !reflect.DeepEqual(landing.Changed, []string{"reverted.txt"}) { + t.Fatalf("landing = %+v, %v", landing, err) + } + if notes := store.Notes(store.RootID(), 0); len(notes) == 0 || notes[len(notes)-1].Body != "landed on run-work: 1 file" { + t.Fatalf("landing notes = %v", notes) + } +} + func runGit(t *testing.T, dir string, args ...string) { t.Helper() command := exec.Command("git", args...) diff --git a/internal/session/beltfacts.go b/internal/session/beltfacts.go index a06329243..3c8b04a08 100644 --- a/internal/session/beltfacts.go +++ b/internal/session/beltfacts.go @@ -183,13 +183,14 @@ func (c Config) assistedByModel() string { } // gitSignature is how the harness signs a commit it writes ITSELF — a node's -// landing, a family's frozen world, a stopped run kept on its branch — and it -// is the same two trailer lines the model is told to write +// landing, a family's frozen world, a stopped run kept on its branch — and +// completes the lines on private worker commits before a run lands. It uses +// the same two trailer lines the model is told to write // (internal/exec's [exec.AttributionTrailers]). // -// ITS ZERO VALUE STILL SIGNS, with the bare `Assisted-by: CodeAF` line. There is -// no value of this type that leaves a commit unsigned, because the signature has -// no off; what it carries is only whether the line names a model and which. +// ITS ZERO VALUE STILL SIGNS, with the bare `Assisted-by: CodeAF` line. The +// repository's CONTRIBUTING rule is checked at the commit door, not represented +// by a value here. This value carries only whether the line names a model. type gitSignature struct { // named is the person's `attribution.model` row: whether the line names // the model at all. @@ -232,6 +233,16 @@ func (s gitSignature) sign(message string) string { return exec.SignCommitMessage(message, model) } +// signOnce preserves lines a worker already wrote and supplies only the +// missing attribution, through the same model choice as the harness's commits. +func (s gitSignature) signOnce(message string) string { + model := "" + if s.named { + model = s.model + } + return exec.SignCommitMessageOnce(message, model) +} + // signsGitWork is the signature a live agent's own commits carry, off the same // two facts the sentence the model is told is rendered from — the model the // conversation is on NOW, and the person's model-name row — so that the diff --git a/internal/session/land_run_tree.go b/internal/session/land_run_tree.go index 76a3b37c4..7996d9b09 100644 --- a/internal/session/land_run_tree.go +++ b/internal/session/land_run_tree.go @@ -1,6 +1,12 @@ package session -import "errors" +import ( + "errors" + "fmt" + "log" + "sort" + "strings" +) // LandRunTree commits a run's own working copy onto the branch it is checked // out on, and it is the landing half of the run engine ([internal/run]) stated @@ -8,19 +14,19 @@ import "errors" // // THE RUN'S WORK IS WHAT ITS TREE SAYS. Every worker of a run edits through // bash ([NewBeltWorker]), and a shell worker fills no write ledger, so the -// working copy's own git status is the record of what the run made — the same +// working copy's status and commits since base record what the run made — the same // road a bash-belt node's landing takes ([commitTaskWork] on its belt arm), // reached by name rather than from inside a task tree. // // IT ANSWERS FOUR THINGS AND ONLY THE FIRST THREE ARE THE LANDING. The branch -// is the one the copy stands on once the commit has landed. The paths are what -// the commit carried, read off the index it built rather than off any list -// handed in. The refusal is the sentence a person reads when the landing had +// is the one the copy stands on once the commit has landed. The paths are the +// net change since base, including a branch the worker merged in; when the net +// tree is unchanged but commits moved HEAD, they are the paths those commits +// touched. The refusal is the +// sentence a person reads when the landing had // nothing to do or the tree would not take the work, and it is empty on every -// other road. The error is non-nil only when there was no working copy a -// landing could be made in at all — a directory that is not a repository, a -// copy standing on no branch — because that is a fault about the place rather -// than a landing outcome, and there is no branch or sentence to answer with. +// other road. The error is non-nil when the working copy or its history cannot +// be read or landed, rather than for an ordinary refusal to bring work home. // // THE SWITCH IS THE BELT'S, read here for the reason [NewBeltWorker] reads it: // the belt road stages the tree's own status minus what the harness itself @@ -28,16 +34,20 @@ import "errors" // with CODEAF_TASK_BELT naming the node belt would change a landing for one nobody // composed. With the flag off this refuses and touches nothing. // -// THE COMMIT IS SIGNED, ALWAYS, with the same two trailer lines every commit -// codeaf writes carries. model is the one the `Assisted-by` line names, and -// empty leaves that line bare — the honest answer for a door that does not know -// which model the run's workers were on, or for a person who turned the name -// off. -func LandRunTree(dir, title, model string) (branch string, changed []string, refusal string, err error) { +// ATTRIBUTION IS ON UNLESS CONTRIBUTING FORBIDS IT. The same two trailer +// lines are added to worker commits and the landing commit. model is the one +// the `Assisted-by` line names; empty leaves it bare because this door does +// not know which model each worker used. +func LandRunTree(dir, base, title, model string) (branch string, changed []string, refusal string, err error) { if !bashBeltAsked() { return "", nil, "", errors.New("the bash belt is off: CODEAF_TASK_BELT names the node belt") } sign := gitSignature{named: model != "", model: model} + // A plumbing failure leaves the original branch intact. The work must still + // come home, so report the signing problem and continue the landing. + if _, err := signRunCommits(dir, base, sign); err != nil { + log.Printf("codeaf: could not sign the run's commits: %v", err) + } saved, problem, why := commitTaskWork(dir, title, nil, sign, true) if problem != "" { if why == refusedByTheTree { @@ -47,13 +57,75 @@ func LandRunTree(dir, title, model string) (branch string, changed []string, ref } return "", nil, problem, nil } - if len(saved) == 0 { + headMoved := false + if base != "" { + out, err := git(dir, "diff", "--name-only", "-z", base, "HEAD") + if err != nil { + return "", nil, "", err + } + for _, name := range strings.Split(out, "\x00") { + if name != "" { + changed = append(changed, name) + } + } + head := runTreeHead(dir) + if head == "" { + return "", nil, "", errors.New("read the run's branch head after landing") + } + headMoved = head != base + if headMoved && len(changed) == 0 { + // The net path list measures this tree since base, not the + // rewrite's private range; include an outside merged branch too. + changed, err = runTouchedPaths(dir, base, head) + if err != nil { + return "", nil, "", err + } + } + } else { + changed = saved + } + if len(changed) == 0 && !headMoved { return "", nil, runNothingToLand, nil } if branch = currentBranch(dir); branch == "" { return "", nil, "", errors.New("the run's working copy stands on no branch, so there is nothing to land") } - return branch, saved, "", nil + return branch, changed, "", nil +} + +// runTouchedPaths names files in non-merge commits when committed work later +// cancels itself out. It is sorted and unique, while an empty-commit run still +// lands on its branch with a count of zero files. +func runTouchedPaths(dir, base, head string) ([]string, error) { + rangeArg := head + if base != "" { + rangeArg = base + ".." + head + } + out, err := git(dir, "rev-list", "--no-merges", rangeArg) + if err != nil { + return nil, fmt.Errorf("list run's touched commits: %w: %s", err, strings.TrimSpace(out)) + } + seen := make(map[string]bool) + for _, sha := range strings.Fields(out) { + if !runObjectID.MatchString(sha) { + return nil, fmt.Errorf("list run's touched commits: invalid object id %q", sha) + } + paths, err := git(dir, "diff-tree", "--root", "--no-commit-id", "--name-only", "-r", "-z", sha) + if err != nil { + return nil, fmt.Errorf("read run commit paths %s: %w", sha, err) + } + for _, path := range strings.Split(paths, "\x00") { + if path != "" { + seen[path] = true + } + } + } + changed := make([]string, 0, len(seen)) + for path := range seen { + changed = append(changed, path) + } + sort.Strings(changed) + return changed, nil } // runNothingToLand is the refusal a run's landing answers when the working copy diff --git a/internal/session/land_run_tree_test.go b/internal/session/land_run_tree_test.go index 9cb8a2264..ad21e1d95 100644 --- a/internal/session/land_run_tree_test.go +++ b/internal/session/land_run_tree_test.go @@ -22,7 +22,7 @@ func TestLandRunTreeCommitsTheTreesOwnWorkOntoItsBranch(t *testing.T) { writeFile(t, filepath.Join(repo, "second.txt"), "two\n") writeFile(t, filepath.Join(repo, "shared.txt"), "the changed line\n") - branch, changed, refusal, err := LandRunTree(repo, "do the thing", "") + branch, changed, refusal, err := LandRunTree(repo, "", "do the thing", "") if err != nil { t.Fatalf("LandRunTree: %v", err) } @@ -50,7 +50,7 @@ func TestLandRunTreeRefusesATreeWithNothingToLand(t *testing.T) { t.Setenv("CODEAF_TASK_BELT", "bash") repo := newTestRepo(t) - branch, changed, refusal, err := LandRunTree(repo, "only read", "") + branch, changed, refusal, err := LandRunTree(repo, "", "only read", "") if err != nil { t.Fatalf("LandRunTree: %v", err) } @@ -62,6 +62,36 @@ func TestLandRunTreeRefusesATreeWithNothingToLand(t *testing.T) { } } +// Contract 7: with a known base, a read-only run still reports no work. +func TestLandRunTreeWithBaseRefusesReadOnlyRun(t *testing.T) { + t.Setenv("CODEAF_TASK_BELT", "bash") + repo := newTestRepo(t) + base := strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD")) + branch, changed, refusal, err := LandRunTree(repo, base, "only read", "") + if err != nil || branch != "" || len(changed) != 0 || refusal != runNothingToLand { + t.Fatalf("read-only landing = %q, %v, %q, %v", branch, changed, refusal, err) + } +} + +// Contract 7: two commits that cancel each other's tree still came home, so +// landing names their touched file instead of claiming there was no work. +func TestLandRunTreeNamesCommittedThenRevertedWork(t *testing.T) { + t.Setenv("CODEAF_TASK_BELT", "bash") + repo := newTestRepo(t) + base := strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD")) + runCommitFile(t, repo, "reverted.txt", "add reverted") + mustGit(t, repo, "-c", "user.name=Worker", "-c", "user.email=worker@example.test", "revert", "--no-edit", "HEAD") + branch, changed, refusal, err := LandRunTree(repo, base, "work then revert", "") + if err != nil || branch != "work" || refusal != "" || !reflect.DeepEqual(changed, []string{"reverted.txt"}) { + t.Fatalf("landing = %q, %v, %q, %v", branch, changed, refusal, err) + } + for _, rev := range []string{"HEAD", "HEAD~1"} { + if message := gitOut(t, repo, "log", "-1", "--format=%B", rev); strings.Count(message, "Assisted-by: CodeAF") != 1 { + t.Fatalf("%s not signed once: %q", rev, message) + } + } +} + // THE SWITCH IS THE BELT'S. With CODEAF_TASK_BELT off the door refuses and not // one byte of the tree moves: no commit, no index, the work still on the floor. func TestLandRunTreeLeavesTheTreeAloneWithTheFlagOff(t *testing.T) { @@ -70,7 +100,7 @@ func TestLandRunTreeLeavesTheTreeAloneWithTheFlagOff(t *testing.T) { before := gitOut(t, repo, "rev-parse", "HEAD") writeFile(t, filepath.Join(repo, "second.txt"), "two\n") - if _, _, _, err := LandRunTree(repo, "do the thing", ""); err == nil { + if _, _, _, err := LandRunTree(repo, "", "do the thing", ""); err == nil { t.Fatal("LandRunTree with the belt off returned no error, want a refusal") } if after := gitOut(t, repo, "rev-parse", "HEAD"); after != before { diff --git a/internal/session/run_commit_sign.go b/internal/session/run_commit_sign.go new file mode 100644 index 000000000..5ae3948e0 --- /dev/null +++ b/internal/session/run_commit_sign.go @@ -0,0 +1,259 @@ +package session + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" +) + +var runObjectID = regexp.MustCompile(`^(?:[0-9a-f]{40}|[0-9a-f]{64})$`) + +type runCommit struct { + old, tree, message string + parents []string + author, committer string + protected bool +} + +// signRunCommits signs only commits private to the run's checked-out branch. +// A missing base on this landing road cannot identify the run's work. +func signRunCommits(dir, base string, sign gitSignature) (int, error) { + return signRunCommitsFrom(dir, base, false, sign) +} + +// signUnbornRunCommits is for an in-place run whose snapshot saw an existing +// repository with no HEAD commit. Only that observation makes an empty base +// mean that the run's first commit is its own work. +func signUnbornRunCommits(dir string, sign gitSignature) (int, error) { + return signRunCommitsFrom(dir, "", true, sign) +} + +// signRunCommitsFrom rewrites messages at the end of a run, when its private +// commits are known. A commit hook on every worker git command would also run +// in the worker's test repositories and replace the project's own hooks. +// Plumbing creates objects without moving the index or worktree, then one +// compare-and-swap moves the branch if it still points at the observed head. +func signRunCommitsFrom(dir, base string, unborn bool, sign gitSignature) (int, error) { + if (base == "" && !unborn) || repositoryRefusesTrailers(dir) { + return 0, nil + } + refOut, err := git(dir, "symbolic-ref", "-q", "HEAD") + if err != nil { + return 0, nil // A detached HEAD has no branch to move. + } + ref := strings.TrimSpace(refOut) + if !strings.HasPrefix(ref, "refs/heads/") { + return 0, nil + } + headOut, err := git(dir, "rev-parse", ref) + if err != nil { + return 0, fmt.Errorf("read run branch: %w", err) + } + head := strings.TrimSpace(headOut) + if !runObjectID.MatchString(head) { + return 0, fmt.Errorf("read run branch: invalid object id %q", headOut) + } + if head == base { + return 0, nil + } + list, err := runCommitRange(dir, ref, head, base) + if err != nil { + return 0, err + } + var commits []runCommit + for _, line := range strings.Split(strings.TrimSpace(list), "\n") { + if line == "" { + continue + } + fields := strings.Fields(line) + // git() combines stderr and stdout, so a warning must never become + // a commit or parent id in the history we rewrite. + for _, field := range fields { + if !runObjectID.MatchString(field) { + return 0, fmt.Errorf("list run commits: invalid object id %q", field) + } + } + commit, err := readRunCommit(dir, fields[0]) + if err != nil { + return 0, err + } + if commit.protected { + // A rewritten ancestor would change the parent of this protected + // commit, invalidating its signature or other special header. + return 0, nil + } + commits = append(commits, commit) + } + if len(commits) == 0 { + return 0, nil + } + remap := make(map[string]string, len(commits)) + rewritten := 0 + for _, commit := range commits { + parents := append([]string(nil), commit.parents...) + parentMoved := false + for i, parent := range parents { + if replacement, ok := remap[parent]; ok { + parents[i] = replacement + parentMoved = parentMoved || replacement != parent + } + } + message := sign.signOnce(commit.message) + if message == commit.message && !parentMoved { + remap[commit.old] = commit.old + continue + } + sha, err := writeRunCommit(dir, commit, parents, message) + if err != nil { + return 0, err + } + remap[commit.old] = sha + rewritten++ + } + newHead := remap[head] + if newHead == "" || newHead == head { + return 0, nil + } + if out, err := git(dir, "update-ref", "-m", "codeaf: sign the run's own commits", ref, newHead, head); err != nil { + return 0, fmt.Errorf("move run branch after signing: %w: %s", err, strings.TrimSpace(out)) + } + return rewritten, nil +} + +// runCommitRange is the one definition of commits this run may sign. Other +// branch, tag, remote and FETCH_HEAD pointers protect published or fetched +// history, even when a worker merged it into its own branch. --exclude applies +// to --branches alone, so the run's branch remains eligible. +func runCommitRange(dir, ref, head, base string) (string, error) { + args := []string{"rev-list", "--reverse", "--topo-order", "--parents", head} + if base != "" { + args = append(args, "^"+base) + } + args = append(args, "--not", "--exclude="+strings.TrimPrefix(ref, "refs/heads/"), "--branches", "--tags", "--remotes") + fetchPath, err := git(dir, "rev-parse", "--git-path", "FETCH_HEAD") + if err != nil { + return "", fmt.Errorf("find FETCH_HEAD: %w: %s", err, strings.TrimSpace(fetchPath)) + } + fetchPath = strings.TrimSpace(fetchPath) + if !filepath.IsAbs(fetchPath) { + fetchPath = filepath.Join(dir, fetchPath) + } + fetched, err := os.ReadFile(fetchPath) + if err != nil && !errors.Is(err, os.ErrNotExist) { + return "", fmt.Errorf("read FETCH_HEAD: %w", err) + } + for _, line := range strings.Split(string(fetched), "\n") { + if line == "" { + continue + } + fields := strings.Fields(line) + if len(fields) == 0 || !runObjectID.MatchString(fields[0]) { + return "", fmt.Errorf("read FETCH_HEAD: invalid object id in %q", line) + } + args = append(args, fields[0]) + } + list, err := git(dir, args...) + if err != nil { + return "", fmt.Errorf("list run commits: %w: %s", err, strings.TrimSpace(list)) + } + return list, nil +} + +// readRunCommit preserves the object's identities and message exactly. Headers +// other than the four ordinary kinds are deliberately not reconstructed. +func readRunCommit(dir, sha string) (runCommit, error) { + raw, err := git(dir, "cat-file", "commit", sha) + if err != nil { + return runCommit{}, fmt.Errorf("read run commit %s: %w", sha, err) + } + headers, message, ok := strings.Cut(raw, "\n\n") + if !ok { + return runCommit{}, fmt.Errorf("run commit %s has no message boundary", sha) + } + commit := runCommit{old: sha, message: message} + for _, line := range strings.Split(headers, "\n") { + key, value, ok := strings.Cut(line, " ") + if !ok { + commit.protected = true + continue + } + switch key { + case "tree": + commit.tree = value + case "parent": + commit.parents = append(commit.parents, value) + case "author": + commit.author = value + case "committer": + commit.committer = value + default: + commit.protected = true + } + } + if commit.tree == "" || commit.author == "" || commit.committer == "" { + return runCommit{}, fmt.Errorf("run commit %s lacks an ordinary identity or tree", sha) + } + return commit, nil +} + +// writeRunCommit keeps the worker's original identities and tree. THE WORKER'S +// AUTHOR AND COMMITTER DO NOT MOVE; only the message and remapped parents do. +func writeRunCommit(dir string, commit runCommit, parents []string, message string) (string, error) { + author, err := runCommitIdentity(commit.author, "AUTHOR") + if err != nil { + return "", err + } + committer, err := runCommitIdentity(commit.committer, "COMMITTER") + if err != nil { + return "", err + } + file, err := os.CreateTemp("", "codeaf-run-commit-*") + if err != nil { + return "", err + } + defer os.Remove(file.Name()) + if _, err := file.WriteString(message); err != nil { + file.Close() + return "", err + } + if err := file.Close(); err != nil { + return "", err + } + args := []string{"commit-tree", "--no-gpg-sign", commit.tree} + for _, parent := range parents { + args = append(args, "-p", parent) + } + args = append(args, "-F", file.Name()) + out, err := gitWith(dir, append(author, committer...), args...) + if err != nil { + return "", fmt.Errorf("sign run commit %s: %w: %s", commit.old, err, strings.TrimSpace(out)) + } + // commit-tree may write a warning before its object id on combined output. + lines := strings.Fields(strings.TrimSpace(out)) + if len(lines) == 0 || !runObjectID.MatchString(lines[len(lines)-1]) { + return "", fmt.Errorf("sign run commit %s: invalid object id in %q", commit.old, out) + } + return lines[len(lines)-1], nil +} + +// runCommitIdentity splits at the final address delimiter because a person's +// name may itself contain the same "> " spelling. +func runCommitIdentity(raw, role string) ([]string, error) { + close := strings.LastIndex(raw, "> ") + if close < 0 { + return nil, fmt.Errorf("run commit has an invalid %s identity", role) + } + identity, date := raw[:close], raw[close+2:] + open := strings.LastIndex(identity, " <") + if open < 0 { + return nil, fmt.Errorf("run commit has an invalid %s address", role) + } + return []string{ + "GIT_" + role + "_NAME=" + identity[:open], + "GIT_" + role + "_EMAIL=" + identity[open+2:], + "GIT_" + role + "_DATE=" + date, + }, nil +} diff --git a/internal/session/run_commit_sign_test.go b/internal/session/run_commit_sign_test.go new file mode 100644 index 000000000..7ab5829c1 --- /dev/null +++ b/internal/session/run_commit_sign_test.go @@ -0,0 +1,308 @@ +package session + +import ( + "os" + "path/filepath" + "reflect" + "strings" + "testing" +) + +func runCommitFile(t *testing.T, repo, name, message string) string { + t.Helper() + writeFile(t, filepath.Join(repo, name), message+"\n") + mustGit(t, repo, "add", name) + mustGit(t, repo, "-c", "user.name=Worker", "-c", "user.email=worker@example.test", "commit", "-m", message) + return strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD")) +} + +// Contract 2: an already-signed worker commit keeps its sha and a second pass +// does not move the branch. A partly signed message gains only its missing line. +func TestSignRunCommitsCompletesOnceAndKeepsWorkerIdentity(t *testing.T) { + repo := newTestRepo(t) + base := strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD")) + first := runCommitFile(t, repo, "one.txt", "one") + mustGit(t, repo, "-c", "user.name=Worker", "-c", "user.email=worker@example.test", "commit", "--allow-empty", "-m", "two", "-m", "Assisted-by: CodeAF") + writeFile(t, filepath.Join(repo, "loose.txt"), "still loose\n") + beforeStatus := gitOut(t, repo, "status", "--porcelain", "-z") + count, err := signRunCommits(repo, base, gitSignature{}) + if err != nil || count != 2 { + t.Fatalf("signRunCommits = %d, %v; want two commits", count, err) + } + if after := gitOut(t, repo, "status", "--porcelain", "-z"); after != beforeStatus { + t.Fatalf("rewrite changed the worktree: %q -> %q", beforeStatus, after) + } + if got := strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD~2")); got != base { + t.Fatalf("base moved: %s -> %s", base, got) + } + if got := strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD~1")); got == first { + t.Fatal("unsigned worker commit kept its old sha") + } + for _, rev := range []string{"HEAD", "HEAD~1"} { + message := gitOut(t, repo, "log", "-1", "--format=%B", rev) + if strings.Count(message, "Assisted-by: CodeAF") != 1 || strings.Count(message, "Co-Authored-By: CodeAF") != 1 { + t.Fatalf("%s has duplicate or missing lines: %q", rev, message) + } + if who := strings.TrimSpace(gitOut(t, repo, "log", "-1", "--format=%an <%ae>|%cn <%ce>", rev)); who != "Worker |Worker " { + t.Fatalf("%s changed worker identity: %s", rev, who) + } + } + head := strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD")) + count, err = signRunCommits(repo, base, gitSignature{}) + if err != nil || count != 0 || strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD")) != head { + t.Fatalf("second signing moved branch: %d, %v", count, err) + } +} + +// Contract 2: a worker that already wrote both lines keeps its original +// object id when no parent needs remapping. +func TestSignRunCommitsKeepsAlreadySignedWorkerSHA(t *testing.T) { + repo := newTestRepo(t) + base := strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD")) + writeFile(t, filepath.Join(repo, "already.txt"), "already\n") + mustGit(t, repo, "add", "already.txt") + mustGit(t, repo, "-c", "user.name=Worker", "-c", "user.email=worker@example.test", + "commit", "-m", "already\n\nAssisted-by: CodeAF (worker-model)\nCo-Authored-By: CodeAF <267109073+agentfield-bot@users.noreply.github.com>") + head := strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD")) + count, err := signRunCommits(repo, base, gitSignature{}) + if err != nil || count != 0 || strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD")) != head { + t.Fatalf("already-signed worker commit moved: %d, %v", count, err) + } +} + +// Contract 3: a merge's outside parent, a tagged commit, and a pushed commit +// keep their original objects while private descendants can still be signed. +func TestSignRunCommitsKeepsOtherRefsAndMergeParents(t *testing.T) { + repo := newTestRepo(t) + base := strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD")) + mustGit(t, repo, "branch", "outside") + mustGit(t, repo, "checkout", "outside") + outside := runCommitFile(t, repo, "outside.txt", "outside") + mustGit(t, repo, "checkout", "work") + runCommitFile(t, repo, "private.txt", "private") + mustGit(t, repo, "-c", "user.name=Worker", "-c", "user.email=worker@example.test", "merge", "--no-ff", "outside", "-m", "merge outside") + count, err := signRunCommits(repo, base, gitSignature{}) + if err != nil || count != 2 { + t.Fatalf("sign merge range = %d, %v", count, err) + } + parents := strings.Fields(gitOut(t, repo, "rev-list", "--parents", "-n", "1", "HEAD")) + if len(parents) != 3 || parents[2] != outside { + t.Fatalf("outside merge parent moved: %v, want %s", parents, outside) + } + if strings.Contains(gitOut(t, repo, "log", "-1", "--format=%B", outside), "Assisted-by") { + t.Fatal("outside branch's commit was signed") + } + tagged := runCommitFile(t, repo, "tagged.txt", "tagged") + mustGit(t, repo, "tag", "published", tagged) + runCommitFile(t, repo, "after-tag.txt", "after tag") + count, err = signRunCommits(repo, tagged, gitSignature{}) + if err != nil || count != 1 { + t.Fatalf("sign after tag = %d, %v", count, err) + } + if strings.Contains(gitOut(t, repo, "log", "-1", "--format=%B", tagged), "Assisted-by") { + t.Fatal("tagged object was signed") + } + remote := strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD")) + mustGit(t, repo, "update-ref", "refs/remotes/origin/published", remote) + count, err = signRunCommits(repo, tagged, gitSignature{}) + if err != nil || count != 0 || strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD")) != remote { + t.Fatalf("remote-tracking object moved: %d, %v", count, err) + } +} + +// Contract 3: a fetched commit belongs to its source even when FETCH_HEAD is +// its only pointer; the worker's own commit and merge still receive the lines. +func TestSignRunCommitsKeepsFetchedMergeParent(t *testing.T) { + repo := newTestRepo(t) + base := strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD")) + outside := filepath.Join(t.TempDir(), "outside") + mustGit(t, t.TempDir(), "clone", repo, outside) + mustGit(t, outside, "checkout", "-b", "outside") + fetched := runCommitFile(t, outside, "fetched.txt", "fetched") + runCommitFile(t, repo, "worker.txt", "worker") + refsBefore := gitOut(t, repo, "for-each-ref", "--format=%(refname) %(objectname)") + mustGit(t, repo, "fetch", outside, "outside") + if refsAfter := gitOut(t, repo, "for-each-ref", "--format=%(refname) %(objectname)"); refsAfter != refsBefore { + t.Fatalf("fetch created a ref: %q -> %q", refsBefore, refsAfter) + } + mustGit(t, repo, "-c", "user.name=Worker", "-c", "user.email=worker@example.test", "merge", "--no-ff", "FETCH_HEAD", "-m", "merge fetched") + count, err := signRunCommits(repo, base, gitSignature{}) + if err != nil || count != 2 { + t.Fatalf("sign fetched merge = %d, %v; want worker and merge only", count, err) + } + parents := strings.Fields(gitOut(t, repo, "rev-list", "--parents", "-n", "1", "HEAD")) + if len(parents) != 3 || parents[2] != fetched { + t.Fatalf("fetched parent moved: %v, want %s", parents, fetched) + } + if message := gitOut(t, repo, "log", "-1", "--format=%B", fetched); strings.Contains(message, "Assisted-by") { + t.Fatalf("fetched commit was signed: %q", message) + } + for _, rev := range []string{"HEAD", "HEAD^1"} { + if message := gitOut(t, repo, "log", "-1", "--format=%B", rev); strings.Count(message, "Assisted-by: CodeAF") != 1 { + t.Fatalf("own commit %s not signed once: %q", rev, message) + } + } +} + +// Contract 3: the name can contain the header's closing-delimiter spelling. +func TestRunCommitIdentityKeepsAngleBracketInName(t *testing.T) { + got, err := runCommitIdentity("Jane > Doe 1700000000 +0000", "AUTHOR") + want := []string{"GIT_AUTHOR_NAME=Jane > Doe", "GIT_AUTHOR_EMAIL=j@example.test", "GIT_AUTHOR_DATE=1700000000 +0000"} + if err != nil || !reflect.DeepEqual(got, want) { + t.Fatalf("identity = %q, %v; want %q", got, err, want) + } +} + +// Contract 8: signed headers, detached HEAD and an unknown base all leave +// their objects and refs exactly as they were. +func TestSignRunCommitsLeavesProtectedAndUnbasedHistoryUntouched(t *testing.T) { + repo := newTestRepo(t) + base := strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD")) + ancestor := runCommitFile(t, repo, "one.txt", "one") + runCommitFile(t, repo, "two.txt", "two") + raw := gitOut(t, repo, "cat-file", "commit", "HEAD") + raw = strings.Replace(raw, "\n\n", "\ngpgsig -----BEGIN PGP SIGNATURE-----\n fake\n -----END PGP SIGNATURE-----\n\n", 1) + file := filepath.Join(t.TempDir(), "commit") + writeFile(t, file, raw) + sha := strings.TrimSpace(gitOut(t, repo, "hash-object", "-t", "commit", "-w", file)) + mustGit(t, repo, "update-ref", "refs/heads/work", sha) + count, err := signRunCommits(repo, base, gitSignature{}) + if err != nil || count != 0 || strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD")) != sha { + t.Fatalf("protected range moved: %d, %v", count, err) + } + if got := gitOut(t, repo, "cat-file", "commit", sha); got != raw { + t.Fatal("protected object changed") + } + if got := strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD~1")); got != ancestor { + t.Fatalf("unsigned ancestor of protected commit moved: %s -> %s", ancestor, got) + } + count, err = signRunCommits(repo, "", gitSignature{}) + if err != nil || count != 0 { + t.Fatalf("empty base = %d, %v", count, err) + } + mustGit(t, repo, "checkout", "--detach") + count, err = signRunCommits(repo, base, gitSignature{}) + if err != nil || count != 0 { + t.Fatalf("detached HEAD = %d, %v", count, err) + } +} + +// Contracts 1, 4, 5 and 7: the real landing names worker-committed paths, +// obeys CONTRIBUTING for its own commit, and leaves the repository's hook in +// control of the worker's git commit. +func TestLandRunTreeSignsWorkerCommitsAndNamesTheirPaths(t *testing.T) { + t.Setenv("CODEAF_TASK_BELT", "bash") + repo := newTestRepo(t) + base := strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD")) + hooks := strings.TrimSpace(gitOut(t, repo, "rev-parse", "--git-path", "hooks")) + if !filepath.IsAbs(hooks) { + hooks = filepath.Join(repo, hooks) + } + marker := filepath.Join(t.TempDir(), "hook-ran") + previousHookPath, _ := git(repo, "config", "--get", "core.hooksPath") + writeFile(t, filepath.Join(hooks, "pre-commit"), "#!/bin/sh\nprintf ran > '"+marker+"'\n") + if err := os.Chmod(filepath.Join(hooks, "pre-commit"), 0o755); err != nil { + t.Fatal(err) + } + first := runCommitFile(t, repo, "worker.txt", "worker") + if _, err := os.Stat(marker); err != nil { + t.Fatalf("worker's pre-commit hook did not run: %v", err) + } + branch, paths, refused, err := LandRunTree(repo, base, "the run", "") + if err != nil || refused != "" || branch != "work" || !reflect.DeepEqual(paths, []string{"worker.txt"}) { + t.Fatalf("landing = %q, %v, %q, %v", branch, paths, refused, err) + } + if got := strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD")); got == first { + t.Fatal("unsigned worker commit stayed on the branch") + } + if message := gitOut(t, repo, "log", "-1", "--format=%B"); !strings.Contains(message, "Assisted-by: CodeAF\n"+"Co-Authored-By: CodeAF") { + t.Fatalf("worker message is unsigned: %q", message) + } + if out, _ := git(repo, "config", "--get", "core.hooksPath"); out != previousHookPath { + t.Fatalf("landing changed core.hooksPath: %q -> %q", previousHookPath, out) + } + writeFile(t, filepath.Join(repo, "CONTRIBUTING.md"), "Do not add AI co-author trailers to commits.\n") + mustGit(t, repo, "add", "CONTRIBUTING.md") + mustGit(t, repo, "-c", "user.name=Person", "-c", "user.email=person@example.test", "commit", "-m", "set policy") + base = strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD")) + runCommitFile(t, repo, "unsigned.txt", "unsigned") + writeFile(t, filepath.Join(repo, "leftover.txt"), "leftover\n") + _, paths, refused, err = LandRunTree(repo, base, "the run", "") + if err != nil || refused != "" || len(paths) != 2 { + t.Fatalf("refusing repository landing = %v, %q, %v", paths, refused, err) + } + for _, rev := range []string{"HEAD", "HEAD~1"} { + if message := gitOut(t, repo, "log", "-1", "--format=%B", rev); strings.Contains(message, "Assisted-by") { + t.Fatalf("CONTRIBUTING was ignored at %s: %q", rev, message) + } + } +} + +// Contract 4: a symlinked CONTRIBUTING file is skipped, including one pointed +// at a device. The reader also accepts case-insensitive names in docs. +func TestRepositoryRefusesTrailersReadsOnlyRegularContributingFiles(t *testing.T) { + repo := newTestRepo(t) + if err := os.Symlink("/dev/tty", filepath.Join(repo, "CONTRIBUTING.md")); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + if repositoryRefusesTrailers(repo) { + t.Fatal("symlinked CONTRIBUTING was read") + } + writeFile(t, filepath.Join(repo, "docs", "contributing.RST"), "No AI trailers.\n") + if !repositoryRefusesTrailers(repo) { + t.Fatal("docs/contributing.RST ban was missed") + } + writeFile(t, filepath.Join(repo, "docs", "contributing.RST"), strings.Repeat("x", contributingReadLimit+1)+"\nNo AI trailers.") + if repositoryRefusesTrailers(repo) { + t.Fatal("CONTRIBUTING ban after read limit was read") + } + writeFile(t, filepath.Join(repo, "docs", "contributing.RST"), "No AI trailers.\n"+strings.Repeat("x", contributingReadLimit)) + if !repositoryRefusesTrailers(repo) { + t.Fatal("CONTRIBUTING ban before read limit was missed") + } + writeFile(t, filepath.Join(repo, "docs", "contributing.RST"), "Please run tests.\n") + writeFile(t, filepath.Join(repo, ".github", "Contributing.TXT"), "No AI trailers.\n") + if !repositoryRefusesTrailers(repo) { + t.Fatal(".github/Contributing.TXT ban was missed") + } +} + +// Contracts 1 and 2: message-only signing retains a worker's raw subject and +// body bytes, including a missing final newline and CRLF paragraphs. +func TestSignRunCommitsKeepsRawWorkerMessageAndCommitIdentity(t *testing.T) { + repo := newTestRepo(t) + base := strings.TrimSpace(gitOut(t, repo, "rev-parse", "HEAD")) + for i, message := range []string{"no final newline", "subject\r\n\r\nbody with CRLF\r\nsecond line\r\n"} { + old := runCommitFile(t, repo, "raw"+string(rune('a'+i))+".txt", "temporary") + raw := gitOut(t, repo, "cat-file", "commit", old) + headers, _, ok := strings.Cut(raw, "\n\n") + if !ok { + t.Fatal("commit has no message boundary") + } + object := headers + "\n\n" + message + file := filepath.Join(t.TempDir(), "raw-commit") + writeFile(t, file, object) + sha := strings.TrimSpace(gitOut(t, repo, "hash-object", "-t", "commit", "-w", file)) + mustGit(t, repo, "update-ref", "refs/heads/work", sha, old) + } + oldObjects := []string{gitOut(t, repo, "cat-file", "commit", "HEAD~1"), gitOut(t, repo, "cat-file", "commit", "HEAD")} + count, err := signRunCommits(repo, base, gitSignature{}) + if err != nil || count != 2 { + t.Fatalf("sign raw messages = %d, %v", count, err) + } + for i, rev := range []string{"HEAD~1", "HEAD"} { + newObject := gitOut(t, repo, "cat-file", "commit", rev) + oldHeaders, oldMessage, _ := strings.Cut(oldObjects[i], "\n\n") + newHeaders, newMessage, _ := strings.Cut(newObject, "\n\n") + // The second commit's parent must move, but its tree and identities do not. + for _, key := range []string{"tree ", "author ", "committer "} { + oldLine := "\n" + key + strings.SplitN(strings.SplitN("\n"+oldHeaders, "\n"+key, 2)[1], "\n", 2)[0] + if !strings.Contains("\n"+newHeaders, oldLine) { + t.Fatalf("%s changed %s: %q", rev, key, newHeaders) + } + } + if !strings.HasPrefix(newMessage, strings.TrimRight(oldMessage, "\n")+"\n\nAssisted-by: CodeAF\n") { + t.Fatalf("%s lost original message bytes: %q", rev, newMessage) + } + } +} diff --git a/internal/session/run_contributing.go b/internal/session/run_contributing.go new file mode 100644 index 000000000..eaa9d991a --- /dev/null +++ b/internal/session/run_contributing.go @@ -0,0 +1,53 @@ +package session + +import ( + "io" + "os" + "path/filepath" + "strings" + + "github.com/Agent-Field/codeaf/internal/exec" +) + +const contributingReadLimit = 256 << 10 + +// repositoryRefusesTrailers is the one repository rule every harness-written +// commit obeys. Only regular CONTRIBUTING files in the top level, .github or +// docs are read; a link or device cannot make a landing follow or block. The +// bounded prefix keeps a very large policy file from holding up the landing, +// while still reading an early ban instead of skipping the whole file. +func repositoryRefusesTrailers(dir string) bool { + root, ok := repositoryRoot(dir) + if !ok { + return false + } + for _, folder := range []string{root, filepath.Join(root, ".github"), filepath.Join(root, "docs")} { + entries, err := os.ReadDir(folder) + if err != nil { + continue + } + for _, entry := range entries { + name := strings.ToLower(entry.Name()) + switch name { + case "contributing", "contributing.md", "contributing.rst", "contributing.txt": + default: + continue + } + path := filepath.Join(folder, entry.Name()) + info, err := os.Lstat(path) + if err != nil || !info.Mode().IsRegular() { + continue + } + file, err := os.Open(path) + if err != nil { + continue + } + content, err := io.ReadAll(io.LimitReader(file, contributingReadLimit)) + file.Close() + if err == nil && exec.ContributingRefusesTrailers(string(content)) { + return true + } + } + } + return false +} diff --git a/internal/session/run_lifecycle_test.go b/internal/session/run_lifecycle_test.go index 0eb004bfb..7bb530a45 100644 --- a/internal/session/run_lifecycle_test.go +++ b/internal/session/run_lifecycle_test.go @@ -66,7 +66,7 @@ func (e *relayEngine) Start(_ context.Context, spec RunSpec) RunSummary { return summary } -func (e *relayEngine) Land(context.Context, *plandb.Store, string, string) (RunLanding, error) { +func (e *relayEngine) Land(context.Context, *plandb.Store, string, string, string) (RunLanding, error) { e.landOnce.Do(func() { close(e.landEntered) }) <-e.landRelease return RunLanding{}, nil diff --git a/internal/session/run_tree_changes.go b/internal/session/run_tree_changes.go index f15b05a6b..42bf85f42 100644 --- a/internal/session/run_tree_changes.go +++ b/internal/session/run_tree_changes.go @@ -12,7 +12,7 @@ import ( // RunTreeSnapshot is what a working copy held before a run touched it: the // commit it stood on and, for every path git already saw as changed, what // that path held. A door that runs IN PLACE — `codeaf do`, which edits the -// directory it was handed and commits nothing — takes one before the run and +// directory it was handed and makes no commit of its own — takes one before the run and // asks it afterwards which paths the RUN changed, so the files it names are // the run's and never the person's own edits that were sitting there first. // @@ -31,11 +31,25 @@ type RunTreeSnapshot struct { dir string root string head string + // unborn means the repository existed but its branch had no commit when + // the run started; an unknown base in a non-repository is different. + unborn bool // held is every path git saw as changed before the run, by absolute path, // and what it held then: a digest of its bytes, or empty when it was gone. held map[string]string } +// SignRunCommits signs commits made since this snapshot in an in-place run. +// A snapshot on an unborn branch can sign the run's first commits; a snapshot +// outside git has no branch whose new history belongs to the run. +func (s RunTreeSnapshot) SignRunCommits(model string) (int, error) { + sign := gitSignature{named: model != "", model: model} + if s.unborn { + return signUnbornRunCommits(s.root, sign) + } + return signRunCommits(s.root, s.head, sign) +} + // SnapshotRunTree reads dir's working copy as it stands now. func SnapshotRunTree(dir string) RunTreeSnapshot { snapshot := RunTreeSnapshot{dir: dir} @@ -45,6 +59,7 @@ func SnapshotRunTree(dir string) RunTreeSnapshot { } snapshot.root = root snapshot.head = runTreeHead(root) + snapshot.unborn = snapshot.head == "" snapshot.held = make(map[string]string) for _, path := range runTreeStatus(root) { snapshot.held[path] = runTreeDigest(path) @@ -87,8 +102,16 @@ func (s RunTreeSnapshot) Changed() []string { } // A RUN THAT COMMITTED ITS OWN WORK moved HEAD, and what it committed is // clean in the status above. Those paths are the run's too. - if head := runTreeHead(s.root); s.head != "" && head != "" && head != s.head { - if out, err := git(s.root, "diff", "--name-only", "-z", s.head, head); err == nil { + if head := runTreeHead(s.root); head != "" && head != s.head && (s.head != "" || s.unborn) { + if s.unborn { + // The unborn branch has no base for git diff; its first commits + // still belong in an in-place run's file envelope. + if paths, err := runTouchedPaths(s.root, "", head); err == nil { + for _, name := range paths { + add(filepath.Join(s.root, filepath.FromSlash(name))) + } + } + } else if out, err := git(s.root, "diff", "--name-only", "-z", s.head, head); err == nil { for _, name := range strings.Split(out, "\x00") { if name = strings.TrimSpace(name); name != "" { add(filepath.Join(s.root, filepath.FromSlash(name))) diff --git a/internal/session/run_tree_changes_test.go b/internal/session/run_tree_changes_test.go index a468b2c72..abd76f675 100644 --- a/internal/session/run_tree_changes_test.go +++ b/internal/session/run_tree_changes_test.go @@ -53,3 +53,26 @@ func TestARunTreeSnapshotOutsideARepositoryNamesNothing(t *testing.T) { t.Fatalf("a folder with no repository named %v", got) } } + +// Contract 1: an in-place run begun on an unborn branch signs its first root +// commit and its child; a folder that was never a repository has no such base. +func TestRunTreeSnapshotSignsFirstCommitsOnUnbornBranch(t *testing.T) { + repo := t.TempDir() + mustGit(t, repo, "init") + mustGit(t, repo, "checkout", "-b", "work") + snapshot := SnapshotRunTree(repo) + runCommitFile(t, repo, "first.txt", "first") + runCommitFile(t, repo, "second.txt", "second") + count, err := snapshot.SignRunCommits("") + if err != nil || count != 2 { + t.Fatalf("sign unborn run = %d, %v; want two commits", count, err) + } + for _, rev := range []string{"HEAD", "HEAD~1"} { + if message := gitOut(t, repo, "log", "-1", "--format=%B", rev); strings.Count(message, "Assisted-by: CodeAF") != 1 { + t.Fatalf("%s not signed once: %q", rev, message) + } + } + if count, err := snapshot.SignRunCommits(""); err != nil || count != 0 { + t.Fatalf("second signing = %d, %v", count, err) + } +} diff --git a/internal/session/task_batch_run_test.go b/internal/session/task_batch_run_test.go index f38ab88d2..de13603c1 100644 --- a/internal/session/task_batch_run_test.go +++ b/internal/session/task_batch_run_test.go @@ -53,7 +53,7 @@ func (d *batchRunDouble) Start(ctx context.Context, spec RunSpec) RunSummary { return RunSummary{Outcome: beltRunOutcomeDone, Result: "done"} } -func (d *batchRunDouble) Land(context.Context, *plandb.Store, string, string) (RunLanding, error) { +func (d *batchRunDouble) Land(context.Context, *plandb.Store, string, string, string) (RunLanding, error) { return RunLanding{}, nil } diff --git a/internal/session/task_run.go b/internal/session/task_run.go index db9926a04..07515bb84 100644 --- a/internal/session/task_run.go +++ b/internal/session/task_run.go @@ -9102,8 +9102,11 @@ func commitTaskWorkAs(dir, message string, wrote []string, sign gitSignature, ba // what it always held. return nil, "", refusedNothing, nil } + if !repositoryRefusesTrailers(dir) { + message = signed(message, sign) + } if out, err := git(dir, append(codeafGitIdentity(), - "commit", "--no-verify", "-m", signed(message, sign))...); err != nil { + "commit", "--no-verify", "-m", message)...); err != nil { // A COMMIT THAT WOULD NOT GO IS USUALLY ABOUT THE COMMIT — a signature it // could not make, a ref it could not lock, a rule the repository holds — // and those are refusals a second answer can get past. Which of the two @@ -9126,9 +9129,8 @@ func commitTaskWorkAs(dir, message string, wrote []string, sign gitSignature, ba // [exec.AttributionLaw]); this is the other half, and it is mechanical because // there is no model in the loop here to tell. // -// IT ALWAYS SIGNS, with the same two lines the model is told to write — one -// blank line, `Assisted-by`, then the co-author, and nothing else -// ([gitSignature.sign]). There is no off: the row that was one is gone. +// THE REPOSITORY'S CONTRIBUTING RULE IS THE EXCEPTION. The caller checks it +// before applying this signature; there is no setting that turns it off. // // AND THE AUTHOR DOES NOT MOVE. These commits stay authored as // codeaf ([codeafGitIdentity]) rather diff --git a/internal/session/task_run_belt.go b/internal/session/task_run_belt.go index f261ee5ff..2c88dd038 100644 --- a/internal/session/task_run_belt.go +++ b/internal/session/task_run_belt.go @@ -173,7 +173,7 @@ type RunLanding struct { // copy onto its branch and answers where the work went. type RunEngine interface { Start(ctx context.Context, spec RunSpec) RunSummary - Land(ctx context.Context, store *plandb.Store, workspace, rootID string) (RunLanding, error) + Land(ctx context.Context, store *plandb.Store, workspace, base, rootID string) (RunLanding, error) } // chatRunEngine is the registered engine, set once by [RegisterRunEngine] and @@ -915,7 +915,7 @@ func (a *Agent) releaseBeltRun(run *beltRun) { // branch the conversation's note names is the ground's, and a merge that would // not go in answers with the sentence that names the kept branch and the files. func (a *Agent) landBeltRun(ctx context.Context, engine RunEngine, run *beltRun) RunLanding { - landing, err := engine.Land(ctx, run.store, run.workspace, run.root) + landing, err := engine.Land(ctx, run.store, run.workspace, run.tree.checkBase, run.root) if err != nil { if g := a.graph(); g != nil { g.planNote("the run's landing failed: " + err.Error()) diff --git a/internal/session/task_run_belt_test.go b/internal/session/task_run_belt_test.go index 53d2343dc..62d6e2bcb 100644 --- a/internal/session/task_run_belt_test.go +++ b/internal/session/task_run_belt_test.go @@ -122,12 +122,12 @@ func (d *beltRunDouble) Start(ctx context.Context, spec RunSpec) RunSummary { return d.summary } -func (d *beltRunDouble) Land(_ context.Context, _ *plandb.Store, workspace, _ string) (RunLanding, error) { +func (d *beltRunDouble) Land(_ context.Context, _ *plandb.Store, workspace, base, _ string) (RunLanding, error) { d.mu.Lock() d.landCalls++ d.mu.Unlock() if d.real { - branch, changed, refusal, err := LandRunTree(workspace, "the run", "") + branch, changed, refusal, err := LandRunTree(workspace, base, "the run", "") return RunLanding{Branch: branch, Changed: changed, Refused: refusal}, err } return d.landing, nil @@ -668,7 +668,7 @@ type landingRunDouble struct { } func (d landingRunDouble) Start(context.Context, RunSpec) RunSummary { return d.summary } -func (d landingRunDouble) Land(context.Context, *plandb.Store, string, string) (RunLanding, error) { +func (d landingRunDouble) Land(context.Context, *plandb.Store, string, string, string) (RunLanding, error) { if d.landCalls != nil { *d.landCalls++ } @@ -980,6 +980,57 @@ func TestABeltRunsWorkComesHomeWhenItEnds(t *testing.T) { } } +// Contracts 1 and 7: the /task door brings two worker commits and a landing +// commit into the person's ground, with each signed once and the base intact. +func TestBeltRunSignsWorkerCommitsInGround(t *testing.T) { + t.Setenv("CODEAF_TASK_BELT", "bash") + ground := beltRunCommittedRepo(t) + base := strings.TrimSpace(gitOut(t, ground, "rev-parse", "HEAD")) + dir := t.TempDir() + double := newBeltRunDouble("three files made") + double.real = true + double.work = func(workspace string) { + for _, name := range []string{"one.txt", "two.txt"} { + writeFile(t, filepath.Join(workspace, name), name+"\n") + mustGit(t, workspace, "add", name) + mustGit(t, workspace, "-c", "user.name=Worker", "-c", "user.email=worker@example.test", "commit", "-m", "add "+name) + } + writeFile(t, filepath.Join(workspace, "leftover.txt"), "leftover\n") + } + registerBeltRunEngine(t, double) + agent, _ := newTestAgent(t, beltRunCompleter{text: "done"}, func(config *Config) { + config.Workspace = ground + config.Place = Place{Dir: dir} + }) + if err := agent.startKnownTaskRun(context.Background(), 171, "make three files", "brief", nil, taskStand{dir: ground, mode: TaskModeWorktree}, ""); err != nil { + t.Fatal(err) + } + <-double.entered + endBeltRun(t, agent, double) + if got := strings.TrimSpace(gitOut(t, ground, "rev-parse", "HEAD~3")); got != base { + t.Fatalf("ground's earlier commit moved: %s -> %s", base, got) + } + for i, name := range []string{"leftover.txt", "two.txt", "one.txt"} { + if data, err := os.ReadFile(filepath.Join(ground, name)); err != nil || len(data) == 0 { + t.Fatalf("%s is not home: %q, %v", name, data, err) + } + rev := "HEAD" + strings.Repeat("~", i) + message := gitOut(t, ground, "log", "-1", "--format=%B", rev) + if strings.Count(message, "Assisted-by: CodeAF") != 1 || strings.Count(message, "Co-Authored-By: CodeAF") != 1 { + t.Fatalf("%s is not signed once: %q", rev, message) + } + } + store := beltRunStoreAt(t, dir) + defer store.Close() + var notes string + for _, note := range store.Notes(store.RootID(), 0) { + notes += note.Body + "\n" + } + if !strings.Contains(notes, "its work is in "+canonicalPath(ground)+" on ") || strings.Contains(notes, "nothing to land") { + t.Fatalf("ground landing note is wrong: %s", notes) + } +} + // A HAND-OFF AFTER A RUN HAS ENDED IS A RUN OF ITS OWN, IN A COPY OF ITS OWN. // It never joins the ended run (whose workers have gone home and whose copy was // given back), and it is cut from the ground AS THE FIRST RUN LEFT IT, so the diff --git a/internal/session/task_run_copy.go b/internal/session/task_run_copy.go index cc1dd35e7..9e2afb7ba 100644 --- a/internal/session/task_run_copy.go +++ b/internal/session/task_run_copy.go @@ -53,6 +53,9 @@ type TaskCopyRecord struct { // was going. Home string `json:"home,omitempty"` HomeSha string `json:"homeSha,omitempty"` + // CheckBase is the commit at the run's start. Old records have none and + // their worker commits are left as written when the run resumes. + CheckBase string `json:"checkBase,omitempty"` // Rung is which rung of the ground ladder made this world and Seal is the // one string that names it, carried for the reason a node carries them: a // landing outlives the run that made the world. @@ -68,15 +71,16 @@ func runCopyOf(tree taskTree) *TaskCopyRecord { return nil } return &TaskCopyRecord{ - Dir: tree.dir, - Branch: tree.branch, - Root: tree.root, - Ground: tree.ground, - Mode: tree.mode, - Home: tree.home, - HomeSha: tree.homeSha, - Rung: tree.rung, - Seal: tree.seal, + Dir: tree.dir, + Branch: tree.branch, + Root: tree.root, + Ground: tree.ground, + Mode: tree.mode, + Home: tree.home, + HomeSha: tree.homeSha, + CheckBase: tree.checkBase, + Rung: tree.rung, + Seal: tree.seal, } } @@ -135,16 +139,17 @@ func runCopyTree(record *TaskCopyRecord, place Place) (taskTree, error) { return taskTree{}, fmt.Errorf("its working copy is gone from %s, so there is nothing left to carry on in", dir) } return taskTree{ - dir: dir, - root: record.Root, - branch: record.Branch, - home: record.Home, - homeSha: record.HomeSha, - ground: record.Ground, - mode: record.Mode, - rung: record.Rung, - seal: record.Seal, - place: place, + dir: dir, + root: record.Root, + branch: record.Branch, + home: record.Home, + homeSha: record.HomeSha, + checkBase: record.CheckBase, + ground: record.Ground, + mode: record.Mode, + rung: record.Rung, + seal: record.Seal, + place: place, // A SHELL WORKER'S COPY, which is what a run's always is: its workers // edit through bash and fill no write ledger (task_run_belt.go states it // where the copy is first made). diff --git a/internal/session/task_run_copy_test.go b/internal/session/task_run_copy_test.go index 158f38687..8bca8ee2e 100644 --- a/internal/session/task_run_copy_test.go +++ b/internal/session/task_run_copy_test.go @@ -65,7 +65,7 @@ func TestAskingForARunsCopyASecondTimeDestroysTheWorkInIt(t *testing.T) { func TestARunsCopyIsWrittenDownFromTheTreeItIsWorkingIn(t *testing.T) { tree := taskTree{ dir: "/w/trees/7", root: "/w/repo", branch: "task/port-the-parser-9c1a2f", - home: "work", homeSha: "abc123", ground: "/w/repo", mode: TaskModeWorktree, + home: "work", homeSha: "abc123", checkBase: "base123", ground: "/w/repo", mode: TaskModeWorktree, } record := runCopyOf(tree) if record == nil { @@ -80,6 +80,9 @@ func TestARunsCopyIsWrittenDownFromTheTreeItIsWorkingIn(t *testing.T) { if record.Home != tree.home || record.HomeSha != tree.homeSha { t.Fatalf("the record forgets the checkout the copy was cut from: %+v", record) } + if record.CheckBase != tree.checkBase { + t.Fatalf("the record forgets the run's commit base: %+v", record) + } } // AND IT SURVIVES THE ROUND TRIP THROUGH THE STORE, which is the only trip that @@ -87,7 +90,7 @@ func TestARunsCopyIsWrittenDownFromTheTreeItIsWorkingIn(t *testing.T) { func TestTheCopySurvivesBeingWrittenDownAndReadBack(t *testing.T) { notice := TaskNotice{ ID: 7, Title: "port the parser", State: TaskRunning, - Copy: &TaskCopyRecord{Dir: "/w/trees/7", Branch: "task/port-the-parser-9c1a2f", Root: "/w/repo"}, + Copy: &TaskCopyRecord{Dir: "/w/trees/7", Branch: "task/port-the-parser-9c1a2f", Root: "/w/repo", CheckBase: "base123"}, } restored := runRowNotice(runRowRecord(notice)) if restored.Copy == nil { @@ -99,6 +102,9 @@ func TestTheCopySurvivesBeingWrittenDownAndReadBack(t *testing.T) { if restored.Copy.Dir != "/w/trees/7" { t.Fatalf("the directory came back as %q", restored.Copy.Dir) } + if restored.Copy.CheckBase != "base123" { + t.Fatalf("the commit base came back as %q", restored.Copy.CheckBase) + } // AND THE ROW IS STILL READ AS INTERRUPTED, which is the state this record // exists to make actionable rather than merely honest. if restored.State != TaskInterrupted { @@ -144,7 +150,7 @@ func TestACopyThatIsThereIsAdoptedAndNothingIsCut(t *testing.T) { dir := t.TempDir() tree, err := runCopyTree(&TaskCopyRecord{ Dir: dir, Branch: "task/port-the-parser-9c1a2f", Root: "/w/repo", - Ground: "/w/repo", Mode: TaskModeWorktree, Home: "work", HomeSha: "abc123", + Ground: "/w/repo", Mode: TaskModeWorktree, Home: "work", HomeSha: "abc123", CheckBase: "base123", }, Place{Dir: "/w/place"}) if err != nil { t.Fatalf("adopting a copy that is there: %v", err) @@ -155,6 +161,9 @@ func TestACopyThatIsThereIsAdoptedAndNothingIsCut(t *testing.T) { if tree.ground != "/w/repo" || tree.mode != TaskModeWorktree { t.Fatalf("the adopted tree forgot what it stands on: %+v", tree) } + if tree.checkBase != "base123" { + t.Fatalf("the adopted tree forgot the run's commit base: %+v", tree) + } if !tree.bashBelt { t.Fatal("the adopted tree is not a shell worker's, so its landing would expect a write ledger no run fills") } From 1708f6969dada147559d09737663d6b6cceb75ca Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 25 Sep 2026 10:31:06 -0400 Subject: [PATCH 4/5] do: sign the commits its workers made in place codeaf do still makes no commit of its own in the folder it was handed. When its run ends it signs the commits its workers made there since the run began (every commit on the branch when the folder had none), with the landing's bare line, before it names the files in the envelope. A signing failure is one codeaf: line on stderr and never changes the exit status; the work is on disk either way. Co-Authored-By: Claude Opus 5.5 (1M context) --- cmd/codeaf/do.go | 12 ++- cmd/codeaf/do_engine_contract_test.go | 111 +++++++++++++++++++++++++- 2 files changed, 118 insertions(+), 5 deletions(-) diff --git a/cmd/codeaf/do.go b/cmd/codeaf/do.go index 50ed45e60..2117733e3 100644 --- a/cmd/codeaf/do.go +++ b/cmd/codeaf/do.go @@ -3425,7 +3425,8 @@ func parseSlots(raw string) (*int, error) { // dispatches is the belt's. // // IT KEEPS THE OLDER ROAD'S CONTRACT WITH THE DIRECTORY: the run edits it in -// place and commits nothing. A landing here once staged the directory's whole +// place and makes no commit of its own, but signs worker commits at the end. +// A landing here once staged the directory's whole // `git status` and committed it on the checked-out branch — the person's own // uncommitted edits and untracked files with it — which no `--dir` help line // ever promised. The files the envelope names are the ones this run changed. @@ -3488,7 +3489,7 @@ func runErrand(request doRequest, seats config.Seats) (outcome headlessOutcome, } else { return headlessOutcome{}, fmt.Errorf("inspect directory %s: %w", workspace, statErr) } - // THE RUN WORKS IN PLACE AND COMMITS NOTHING, which is what `--dir` has + // THE RUN WORKS IN PLACE AND MAKES NO COMMIT OF ITS OWN, which is what `--dir` has // always promised: "the directory to work in, edited in place". The copy is // read before the run starts so that, afterwards, the files this run names // are the ones IT changed — the person's own uncommitted edits and untracked @@ -3599,9 +3600,12 @@ func runErrand(request doRequest, seats config.Seats) (outcome headlessOutcome, errand.BlockedOn, errand.machineHeld = held+" · nothing started before --timeout", true } } + if _, err := runengine.SignWork(before); err != nil { + fmt.Fprintf(request.stderr, "codeaf: could not sign the run's commits: %v\n", err) + } // WHAT THE RUN CHANGED IS WHERE IT STANDS: in the directory it was handed, - // uncommitted, on whatever branch was checked out there. The envelope's - // files are those paths and no others, on every ending — a run stopped short + // committed by its workers or still uncommitted, on the checked-out branch. + // The envelope's files are those paths and no others, on every ending — a run stopped short // still left its edits on disk, and a caller has to be able to find them. errand.Artifacts = landedPaths(workspace, before.Changed()) return errand, nil diff --git a/cmd/codeaf/do_engine_contract_test.go b/cmd/codeaf/do_engine_contract_test.go index 5db8b6677..88b911982 100644 --- a/cmd/codeaf/do_engine_contract_test.go +++ b/cmd/codeaf/do_engine_contract_test.go @@ -5,7 +5,7 @@ package main // so the promises the door's help makes are this road's to keep: // // - `--dir` is "the directory to work in, edited in place". The run edits it -// and commits nothing, and the files it names are the ones it changed — +// and makes no commit of its own; the files it names are the ones it changed — // never the person's own uncommitted edits or untracked files. // - `--yes-spend` is "spend past today's limit and past the plan-price // question, without stopping to ask". Without it an unattended run is @@ -140,6 +140,115 @@ func TestDoOnTheRunEngineNeverCommitsThePersonsOwnWork(t *testing.T) { } } +// Contract 6: the real do door signs a worker's in-place git commit, names its +// file in the envelope, and obeys a repository ban without changing exit 0. +func TestDoOnTheRunEngineSignsWorkerCommitsUnlessContributingForbids(t *testing.T) { + for _, banned := range []bool{false, true} { + name := "signed" + if banned { + name = "CONTRIBUTING ban" + } + t.Run(name, func(t *testing.T) { + beltRunEnv(t) + t.Setenv("CODEAF_PLANDB_BIN", beltPlandbDoor(t)) + workspace := beltRepoWorkspace(t) + if banned { + if err := os.WriteFile(filepath.Join(workspace, "CONTRIBUTING.md"), []byte("Do not add AI co-author trailers to commits.\n"), 0o644); err != nil { + t.Fatal(err) + } + beltGit(t, workspace, "add", "CONTRIBUTING.md") + beltGit(t, workspace, "-c", "user.name=Person", "-c", "user.email=person@example.test", "commit", "-m", "set policy") + } + base := doGitIn(t, workspace, "rev-parse", "HEAD") + seat := &beltSeat{ + script: []func(context.Context, []ai.Message) (*ai.Response, error){ + func(context.Context, []ai.Message) (*ai.Response, error) { + return beltToolReply("printf 'hello from worker\\n' > hello.txt && git add hello.txt && git -c user.name=Worker -c user.email=worker@example.test commit -m 'add hello'"), nil + }, + func(context.Context, []ai.Message) (*ai.Response, error) { + return beltToolReply(beltFinish("added hello.txt")), nil + }, + }, + ever: func(_ context.Context, msgs []ai.Message) (*ai.Response, error) { + if doc := beltDocument(msgs); strings.Contains(doc, "## Who checks this work") { + id := briefTaskID(doc) + return beltToolReply("plandb done " + id + " --agent " + id + " --result 'holds: the acceptance is met'"), nil + } + return beltTextReply("added hello.txt"), nil + }, + } + var stdout, stderr strings.Builder + err := doErrand(doRequest{ + task: "add hello.txt", workspace: workspace, asJSON: true, + timeout: 60 * time.Second, slots: bound(1), stdout: &stdout, stderr: &stderr, + newBeltCompleter: func(string) session.Completer { return seat }, + }) + if err != nil { + t.Fatalf("do returned %v, want exit 0\nstdout:\n%s\nstderr:\n%s", err, stdout.String(), stderr.String()) + } + if got := doGitIn(t, workspace, "rev-parse", "HEAD~1"); got != base { + t.Fatalf("person's earlier commit moved: %s -> %s", base, got) + } + message := doGitIn(t, workspace, "log", "-1", "--format=%B") + if banned { + if message != "add hello" { + t.Fatalf("CONTRIBUTING did not preserve worker message: %q", message) + } + } else if strings.Count(message, "Assisted-by: CodeAF") != 1 || strings.Count(message, "Co-Authored-By: CodeAF") != 1 || strings.Contains(message, "CodeAF (") { + t.Fatalf("worker message was not signed once with bare lines: %q", message) + } + outcome := decodeErrand(t, stdout.String()) + if len(outcome.Artifacts) != 1 || outcome.Artifacts[0] != filepath.Join(workspace, "hello.txt") { + t.Fatalf("envelope files = %v", outcome.Artifacts) + } + }) + } +} + +// Contract 6: an in-place run begun on an unborn branch signs its worker's +// first commit and still reports the file through the do envelope. +func TestDoOnTheRunEngineSignsFirstCommitOnUnbornBranch(t *testing.T) { + beltRunEnv(t) + t.Setenv("CODEAF_PLANDB_BIN", beltPlandbDoor(t)) + workspace := t.TempDir() + beltGit(t, workspace, "init") + beltGit(t, workspace, "checkout", "-b", "work") + seat := &beltSeat{ + script: []func(context.Context, []ai.Message) (*ai.Response, error){ + func(context.Context, []ai.Message) (*ai.Response, error) { + return beltToolReply("printf 'hello from worker\\n' > hello.txt && git add hello.txt && git -c user.name=Worker -c user.email=worker@example.test commit -m 'add hello'"), nil + }, + func(context.Context, []ai.Message) (*ai.Response, error) { + return beltToolReply(beltFinish("added hello.txt")), nil + }, + }, + ever: func(_ context.Context, msgs []ai.Message) (*ai.Response, error) { + if doc := beltDocument(msgs); strings.Contains(doc, "## Who checks this work") { + id := briefTaskID(doc) + return beltToolReply("plandb done " + id + " --agent " + id + " --result 'holds: the acceptance is met'"), nil + } + return beltTextReply("added hello.txt"), nil + }, + } + var stdout, stderr strings.Builder + err := doErrand(doRequest{ + task: "add hello.txt", workspace: workspace, asJSON: true, + timeout: 60 * time.Second, slots: bound(1), stdout: &stdout, stderr: &stderr, + newBeltCompleter: func(string) session.Completer { return seat }, + }) + if err != nil { + t.Fatalf("do returned %v; stdout: %s; stderr: %s", err, stdout.String(), stderr.String()) + } + message := doGitIn(t, workspace, "log", "-1", "--format=%B") + if strings.Count(message, "Assisted-by: CodeAF") != 1 || strings.Count(message, "Co-Authored-By: CodeAF") != 1 { + t.Fatalf("first commit not signed once: %q", message) + } + outcome := decodeErrand(t, stdout.String()) + if len(outcome.Artifacts) != 1 || outcome.Artifacts[0] != filepath.Join(workspace, "hello.txt") { + t.Fatalf("envelope files = %v", outcome.Artifacts) + } +} + // WITHOUT --yes-spend, AN UNATTENDED RUN STOPS AT THE PLAN PRICE. // // The worker never finishes and every call costs a dollar; the plan-price From cd479a5f434e12902109afcb65a98657b5e6306c Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 25 Sep 2026 10:31:06 -0400 Subject: [PATCH 5/5] manual + change entry: a task's workers' own commits are signed permissions.md says the worker harness signs its workers' commits too, once, with the bare line, and when it leaves one alone (a GPG/SSH-signed range, a pushed, tagged, other-branch or fetched commit, a detached HEAD); that codeaf now reads CONTRIBUTING itself, and which files; and no longer promises a sentence the harness does not say. The do and /task pages stop saying a run commits nothing or lands nothing when its worker committed. Two probes in a person's own words reach the page. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../unreleased/1495-worker-commits-signed.md | 12 +++ internal/manual/chat/permissions.md | 87 ++++++++++++------- internal/manual/chat/tasks.md | 2 + internal/manual/chat/worker-harness.md | 31 ++++--- internal/manual/chat_test.go | 2 + 5 files changed, 90 insertions(+), 44 deletions(-) create mode 100644 docs/changes/unreleased/1495-worker-commits-signed.md diff --git a/docs/changes/unreleased/1495-worker-commits-signed.md b/docs/changes/unreleased/1495-worker-commits-signed.md new file mode 100644 index 000000000..7b7279940 --- /dev/null +++ b/docs/changes/unreleased/1495-worker-commits-signed.md @@ -0,0 +1,12 @@ +--- +kind: fixed +title: worker commits carry attribution when a run ends +pr: 1495 +surface: [chat, engine] +invalidates: + - "On the worker harness, a commit a worker made itself carried no attribution unless the worker added it. A run now signs its private worker commits at landing with the bare Assisted-by line, and codeaf do signs its workers' commits in place when the run ends." + - "The harness signed its own landing commit even when CONTRIBUTING forbade AI trailers. It now reads regular CONTRIBUTING files in the repository root, .github and docs, and adds no lines to its landing or worker commits when one forbids them." + - "A run whose worker committed everything answered nothing to land while its work merged home. It now names the branch and files the worker committed; a read-only run still answers nothing to land." +--- + +Worker commits keep their original author and committer. Commits signed with Git's own signature, already pointed at by another branch, tag or remote-tracking ref, or fetched from elsewhere and merged are left as the worker wrote them. In a repository with no prior commits, `codeaf do` signs the worker's first commits too. A run that commits and then reverts its work still reports the touched files. diff --git a/internal/manual/chat/permissions.md b/internal/manual/chat/permissions.md index 2922ae579..d096be45a 100644 --- a/internal/manual/chat/permissions.md +++ b/internal/manual/chat/permissions.md @@ -935,8 +935,9 @@ A row your environment has pinned refuses like it does everywhere else: ## Does codeaf sign my commits — why is there a co-author on my commit, who is agentfield-bot, how do I turn the trailer off -Yes, always, and it cannot be turned off. There are three marks and no others, -and this is exactly what each one looks like. +codeaf signs commits it writes, and its worker harness signs the commits its +workers make. The commit mark cannot be turned off unless the repository's +CONTRIBUTING policy forbids AI trailers. **A commit** ends with one blank line and two trailer lines, `Assisted-by` first and the co-author last, and nothing after them: @@ -950,20 +951,39 @@ The name in brackets is the model that wrote the commit — the model the conversation is talking to at that moment — and only the model: the provider or company in front of it (`z-ai/`) and a routing suffix such as `:free` or `:nitro` come off, and the model's own version or date stays. Switch with -`/model` and the next commit names the model you switched to. - -**A commit a task lands names no model on the default way tasks run.** A task -on the worker harness — what `/task` and `codeaf do` use unless -`CODEAF_TASK_BELT` says `node` — lands with the bare line, -`Assisted-by: CodeAF`, with no brackets, whatever the row below says: a run's -record does not say which model did its work, so the landing names none. Only -on the older node belt (`CODEAF_TASK_BELT=node`) does a landing name the model -that task ran on. - -**The one part you can turn off is the model's name.** Switch the **model in -commits** row (`attribution.model`) off in `/settings`, or set -`CODEAF_ATTRIBUTION_MODEL=0`, and the first line is just `Assisted-by: CodeAF`. -It is on by default. +`/model` and the next commit names the model you switched to. To hide only the +model name, turn off the **model in commits** row (`attribution.model`) in +`/settings`, or set `CODEAF_ATTRIBUTION_MODEL=0`. Then the first line is bare +`Assisted-by: CodeAF`; the co-author line remains. + +**A task on the worker harness (the default) names no model.** Its workers' +own commits and its landing commit carry the bare `Assisted-by: CodeAF` and the +co-author line. The section "Does the task sign the commits it makes itself" +explains when those commits are signed or left alone. + +`agentfield-bot` is codeaf's GitHub account. The numeric ID in +`267109073+agentfield-bot@users.noreply.github.com` makes GitHub link the +co-author to that account and show its avatar. The marks record provenance. + +## Does the task sign the commits it makes itself — when are worker commits left unsigned + +The worker harness signs its workers' commits when the run ends, +before its work comes home, and signs a landing commit for leftover edits. +It uses the bare `Assisted-by: CodeAF` line and the codeaf co-author once. +`codeaf do` signs its workers' commits in your folder when its run ends, even +when the repository had no commits before the run. It makes no landing commit +of its own. A commit you make in that folder during the run may be signed too. +An already signed commit gets no duplicate lines. codeaf keeps the commit's +author, committer, tree and message body, and leaves your repository's hooks +alone. A commit your git signed with GPG or SSH keeps what the worker wrote: +changing its message would invalidate that signature. The same is true of the +whole run's range if one commit has such a signature. A commit already pushed, +tagged, pointed at by another branch, or fetched from elsewhere and merged is +left as it is. A detached HEAD or a folder that was not a git repository when +the run started has no commits to sign. CONTRIBUTING files that forbid AI +trailers stop codeaf from adding them to its own commits too. + +## What marks appear on pull requests, issues and comments **A pull request or an issue** ends its body with a line holding an em dash, and then one sentence: @@ -982,13 +1002,8 @@ with one small muted line, no em dash above it: drafted with [CodeAF](https://agentfield.ai/github/codeaf?utm_source=github&utm_medium=comment&utm_campaign=drafted_with) ``` -`agentfield-bot` is codeaf's own GitHub account. The address is written with -the account's numeric id in front — `267109073+agentfield-bot@users.noreply.github.com` -— because that ID-prefixed form is the one GitHub links to the CodeAF account -and renders the co-author with its avatar. The `Assisted-by` line above it -names the model that wrote the commit when there is one to name, so -`git interpret-trailers` can answer who typed it beyond the account. The marks are provenance — another pair of -hands typed this — and they are the only trace left on your work. +These marks tell a reader codeaf drafted the text. A repository policy or a +policy you state can forbid them. ## Will it sign every comment it leaves — how gentle the comment line is, and where none of the three ever appear @@ -998,14 +1013,19 @@ is also left off entirely on a one-line reply, on anything inside a code block o a suggestion block, and on a comment you dictated word for word — those are your words and codeaf does not sign them. -None of the three ever appear in a commit subject, in a code file, in a README, +The commit, pull-request and comment marks never appear in a commit subject, in a code file, in a README, in anything codeaf writes for you such as a report or a deck, or in what it says back to you in this conversation. If you see one somewhere else, that is a fault worth reporting. -**A repository that says no wins.** If a CONTRIBUTING file or a stated policy -forbids AI trailers or generated-by lines, codeaf leaves all three out and tells -you it did. +**A repository that says no wins.** codeaf reads the first 256 KiB of regular +`CONTRIBUTING`, `CONTRIBUTING.md`, `.rst` and `.txt` files (case-insensitive +names) at the top of the repository, in `.github/` and in `docs/`. It skips +links and other non-file entries. If any forbids AI trailers or +co-author lines, codeaf adds none to its own landing commits or the worker +commits it signs at the end of a run. The model is also told to follow +CONTRIBUTING or a policy you state when it writes pull requests, issues and +comments. The commit a task writes when its work lands carries the same two lines — with the bare `Assisted-by: CodeAF` on the worker harness, the default belt. Those @@ -1013,8 +1033,10 @@ commits are authored as `codeaf `: code reads that identity to tell its own commits from yours when it lands a branch. Older task commits authored as `codeaf ` are still recognised as codeaf's own work, as are those under its earlier local address -`aforge ` . Your own commits are authored -by you and are never touched. +`aforge ` . `/task` works in a private +copy, so your own commits are outside its signing range. `codeaf do` works in +your folder, where a commit you make during its run cannot be distinguished +from a worker commit and may be signed at the end. ## I turned signing off before — why are my commits signed again, can I turn attribution off @@ -1041,9 +1063,10 @@ codeaf cannot change the row for you — ask it to and it says so and points you at `/settings`. A change lands on the next piece of work handed off, and on the next codeaf you start. -**The only thing that takes every mark off is the repository.** A CONTRIBUTING -file or a stated policy that forbids AI trailers or generated-by lines makes -codeaf leave all three out and tell you it did. +**A repository policy can forbid the marks.** When a CONTRIBUTING file in the +repository's top level, `.github/` or `docs/` forbids AI trailers, codeaf's +own commits and its signing of worker commits add none. Tell the model about +any further policy for pull requests, issues or comments. ## A timeout is not a deny — why it said "denied by the person" when nobody said no diff --git a/internal/manual/chat/tasks.md b/internal/manual/chat/tasks.md index ae2f28e48..668627e05 100644 --- a/internal/manual/chat/tasks.md +++ b/internal/manual/chat/tasks.md @@ -1951,6 +1951,8 @@ open. Settled families and families containing only waiting work start folded to root; the root then carries the family's aggregate state glyph and a `▸ +N` badge for the hidden descendants. +## Where did the workers go on the task page — find them as rows under their task + **Workers under a task are the family's own rows and nothing else** — there is no second, smaller list drawn beneath a row. A task that split itself into parts, and an adaptive run and its workers, each announce themselves as tasks with a parent, so every one diff --git a/internal/manual/chat/worker-harness.md b/internal/manual/chat/worker-harness.md index 4cf60e475..98ee5d15b 100644 --- a/internal/manual/chat/worker-harness.md +++ b/internal/manual/chat/worker-harness.md @@ -2,9 +2,9 @@ ## What happens when I type /task -On this road, `/task ` starts a **run** rather than a node of the -conversation's own tree. Everything here hangs on one switch, named under *How to -turn it on* below, and the older road is what a build without it does. +On this shipped default road, `/task ` starts a **run** rather than a +node of the conversation's own tree. *How to turn it off* below names the +switch to the older road. Typing `/task` asks you nothing and waits for nothing in front of it: the work exists as soon as you press enter. What it does instead is: @@ -31,23 +31,28 @@ copy cut from your folder as the first run left it. A task handed off in the few while a run is finishing (its work landing, its summary being written) waits until that run is over and then starts its own: it never joins a run on its way out. -**When the run ends its work comes home by itself.** The copy's work is committed and +## When does a task run's work come home, including commits its workers made + +When a `/task` run on the worker harness ends, its copy's uncommitted work is committed and merged into the folder it was cut from, the copy is given back, and the run's page carries `its work is in on `. The conversation is woken with the same note a landed task sends: the outcome word, the result the root reported, and where the work went (`landed on : N files`, or the sentence saying why it did not). Work that will not go in is never forced: the branch is kept in your repository and the note names it, for example `its branch was kept`, when your checkout moved on after -the copy was cut. A run that only read says `nothing to land: the run's working copy holds no change` and changes no file. The +the copy was cut. A run whose workers committed everything still names its +branch and changed files; codeaf signs those commits before the work comes home. +A run that only read says `nothing to land: the run's working copy holds no change` +and changes no file. The landing card says `merged` when the work is in your folder and `branch kept` only for a branch that is waiting. A hand-off that joined the run ends with it: its row settles `done` or `incomplete` when the run's does. The row the run was published under settles `done` when the run finished whole and `incomplete` on any other ending. -**With the switch unset, none of this is reached.** `/task` raises an ordinary -task on this session's own tree, briefed beside its worker and landed through the -task graph. See *How to turn it on*. +**With the switch unset, this is the road `/task` takes.** Set +`CODEAF_TASK_BELT=node` to use the older session tree road instead. See *How +to turn it off*. ## The tasks pane and a task's page @@ -742,12 +747,14 @@ it was (`done`, `error`, `incomplete`, `unchecked`, `budget`, `turn-cap`, `deadline`, `price`, `question`), and `ok` is true on exactly the runs that leave with 0. -## Does codeaf do commit my changes? It edits the folder in place and commits nothing +## Does codeaf do commit my changes? It edits the folder in place and makes no commit of its own `codeaf do` works in the directory you hand it with `-w` / `--dir` (the current -directory by default), **edited in place, on whatever branch is checked out there, -and nothing is committed**. The run's files are left uncommitted for you to read, -commit or throw away, exactly as the older engine left them. +directory by default), **edited in place, on whatever branch is checked out there**. +codeaf makes no commit of its own. Its workers can commit their changes; when +the run ends, codeaf adds the bare `Assisted-by: CodeAF` and co-author lines to +those commits once. Any work still uncommitted is left for you to read, commit +or throw away. Your own work is never touched by the run's accounting: an edit you had not committed, or an untracked file such as a secrets file, is still yours after the diff --git a/internal/manual/chat_test.go b/internal/manual/chat_test.go index 81abc87aa..cc5cd52e9 100644 --- a/internal/manual/chat_test.go +++ b/internal/manual/chat_test.go @@ -181,6 +181,8 @@ func TestTheChatManualAnswersTheQuestionsPeopleAsk(t *testing.T) { // seen, and wanting it gone. {"why is there a co-author on my commit", "permissions"}, {"does codeaf sign my commits", "permissions"}, + {"does the task sign the commits it makes itself", "permissions"}, + {"why does my commit from codeaf do have a co-author line", "permissions"}, {"who is agentfield-bot", "permissions"}, {"what is the drafted with line at the bottom of my pull request", "permissions"}, {"stop adding a co-author trailer to my commits", "permissions"},