From 1fea9b325516f2b15e0ebfc6abd672bd0b6c005f Mon Sep 17 00:00:00 2001 From: Lev Velykoivanenko Date: Sat, 17 Oct 2026 21:50:36 +0000 Subject: [PATCH 1/5] feat(commit): let an agent author stand Co-authored-by: Grok 4.7 <261865991+grok-by-xai[bot]@users.noreply.github.com> --- crates/devkit-git/src/backend.rs | 20 ++ crates/devkit-ports/src/templates.rs | 68 +++++- crates/devkit-vcs/src/lib.rs | 13 ++ .../skills/using-devkit/references/commit.md | 2 +- .../using-devkit/references/templates.md | 2 +- tests/commit.rs | 209 +++++++++++++++++- 6 files changed, 304 insertions(+), 10 deletions(-) diff --git a/crates/devkit-git/src/backend.rs b/crates/devkit-git/src/backend.rs index 95114a0cc..b1943026f 100644 --- a/crates/devkit-git/src/backend.rs +++ b/crates/devkit-git/src/backend.rs @@ -360,6 +360,26 @@ impl VersionControl for GitBackend { .to_string()) } + fn author(&self, dir: &Path) -> Result> { + // The same order `git commit` uses: an author variable wins over + // config, and a variable set for only one half does not mix with + // the other half from config. + let from_env = ["GIT_AUTHOR_NAME", "GIT_AUTHOR_EMAIL"] + .map(|key| std::env::var(key).ok().filter(|v| !v.trim().is_empty())); + let [name, email] = match from_env { + [Some(name), Some(email)] => [name, email], + [None, None] => ["user.name", "user.email"].map(|key| { + Git::at(dir) + .args(["config", key]) + .output() + .map(|v| v.trim().to_string()) + .unwrap_or_default() + }), + _ => return Ok(None), + }; + Ok((!name.trim().is_empty() && !email.trim().is_empty()).then_some((name, email))) + } + fn commit(&self, dir: &Path, selection: &Selection<'_>, message: &str) -> Result { super::commit::commit(dir, selection, message) } diff --git a/crates/devkit-ports/src/templates.rs b/crates/devkit-ports/src/templates.rs index 4276e9ec1..cebbb00ae 100644 --- a/crates/devkit-ports/src/templates.rs +++ b/crates/devkit-ports/src/templates.rs @@ -210,12 +210,68 @@ fn render_commit_parts( caller: Caller, ) -> Result { let coauthors = message.coauthors.join("; "); - let parts = [ - message.subject, - message.body, - (!coauthors.is_empty()).then_some(coauthors.as_str()), - ]; - render_commit_message(cfg, start, parts, given, caller, what) + let stand_in = (message.coauthors.is_empty() + && cfg.templates.commit_message.is_none() + && devkit_common::required::is_required(cfg, None, "coauthors", caller)) + .then(|| agent_author(start, caller)) + .transpose()? + .flatten(); + let supplied = stand_in + .as_deref() + .or((!coauthors.is_empty()).then_some(coauthors.as_str())); + let parts = [message.subject, message.body, supplied]; + let mut text = render_commit_message(cfg, start, parts, given, caller, what)?; + if stand_in.is_some() { + text = drop_coauthor_trailers(&text); + } + Ok(text) +} + +/// The git author when it is the agent, as `Name `. A person's author +/// and a human caller get `None`. +fn agent_author(start: &Path, caller: Caller) -> Result> { + if caller != Caller::Agent { + return Ok(None); + } + let Some((name, email)) = Vcs::at(start).author(start)? else { + return Ok(None); + }; + if !agent_email(&email) { + return Ok(None); + } + Ok(Some(format!("{} <{}>", name.trim(), email.trim()))) +} + +/// A GitHub bot address, the form a harness uses when it authors a commit +/// itself (`123+name[bot]@users.noreply.github.com`). A person's address, +/// including `users.noreply.github.com` without `[bot]`, is not one. +fn agent_email(email: &str) -> bool { + let Some((local, host)) = email.trim().rsplit_once('@') else { + return false; + }; + host.eq_ignore_ascii_case("users.noreply.github.com") && local.contains("[bot]") +} + +/// The message without `Co-authored-by` trailers, and without a blank line +/// that existed only to hold them. +fn drop_coauthor_trailers(message: &str) -> String { + let kept: Vec<&str> = message + .lines() + .filter(|line| { + !line + .trim() + .to_ascii_lowercase() + .starts_with("co-authored-by:") + }) + .collect(); + let mut text = kept.join("\n"); + while text.contains("\n\n\n") { + text = text.replace("\n\n\n", "\n\n"); + } + if !text.ends_with('\n') { + text.push('\n'); + } + text } /// Whether `devkit commit` can leave out `part`: every part but the subject, diff --git a/crates/devkit-vcs/src/lib.rs b/crates/devkit-vcs/src/lib.rs index 7ccaf6d35..a2bc158f7 100644 --- a/crates/devkit-vcs/src/lib.rs +++ b/crates/devkit-vcs/src/lib.rs @@ -289,6 +289,15 @@ pub trait VersionControl { /// configured. fn user_email(&self, dir: &::std::path::Path) -> ::anyhow::Result<::std::string::String>; + /// The author a new commit in `dir` would record (`GIT_AUTHOR_*`, else `user.name` and `user.email`), or `Ok(None)` when either half is unset. + fn author( + &self, + dir: &::std::path::Path, + ) -> ::anyhow::Result> { + let _ = dir; + Ok(None) + } + /// Commits `selection` in `dir`'s repository with `message`, leaving the /// working tree, and changes anyone else staged, as they were. Refuses /// rather than committing part of the selection. Returns the backend's @@ -431,6 +440,10 @@ mod tests { fn user_email(&self, _: &Path) -> anyhow::Result { unimplemented!() } + + fn author(&self, _: &Path) -> anyhow::Result> { + Ok(None) + } } #[test] diff --git a/plugin/skills/using-devkit/references/commit.md b/plugin/skills/using-devkit/references/commit.md index 4be647581..e1d2ccf3f 100644 --- a/plugin/skills/using-devkit/references/commit.md +++ b/plugin/skills/using-devkit/references/commit.md @@ -9,6 +9,6 @@ devkit commit --amend --subject '...' # a new message for the last ``` - To commit part of a file, write only the hunks to commit as a patch against HEAD (`git diff -- path > hunks.patch`, then trim it) and pass it to `--patch`. A patch touching a path whose merge driver could not keep the staged hunks exact is refused. -- The message is the subject, then `--body` after a blank line, then a `Co-authored-by` trailer per `--coauthor`. `devkit template show commit_message` prints the project's template, each message part by its `devkit commit` flag with who must pass it, and any `--arg` it reads. Only `--subject` is required unless the project requires another part, as a project can require `--coauthor` of agents. `devkit template render commit_message` previews the message, taking `--subject`, `--body` and `--coauthor` as `devkit commit` does, and needs no more than it does. +- The message is the subject, then `--body` after a blank line, then a `Co-authored-by` trailer per `--coauthor`. `devkit template show commit_message` prints the project's template, each message part by its `devkit commit` flag with who must pass it, and any `--arg` it reads. Only `--subject` is required unless the project requires another part, as a project can require `--coauthor` of agents. An agent author meets that requirement with no trailer; `references/templates.md` says how. `devkit template render commit_message` previews the message, taking `--subject`, `--body` and `--coauthor` as `devkit commit` does, and needs no more than it does. - Commit hooks and signing run as they do for `git commit`. A refused or failed commit leaves HEAD and the index unchanged, and says why. With `--files` or `--patch`, a commit hook that changes the committed tree is refused too. - When the output says recovery files were retained, the new HEAD could not be confirmed: inspect `git log` before retrying, and do not run the commit again blind. diff --git a/plugin/skills/using-devkit/references/templates.md b/plugin/skills/using-devkit/references/templates.md index c3380a26c..ca0b5a2b3 100644 --- a/plugin/skills/using-devkit/references/templates.md +++ b/plugin/skills/using-devkit/references/templates.md @@ -10,7 +10,7 @@ devkit template render --arg-file k=notes.md # a multi-line value from a ``` - `render` prints the text byte for byte, so `> file` or a pipe gets it unchanged. `--json` on all three verbs emits JSON instead; `render --json` is `{"text": ...}`. -- A required arg left out is refused before anything renders, naming each `--arg` and its description the way `devrun task` does. An `--arg` the template never reads is refused too. +- A required arg left out is refused before anything renders, naming each `--arg` and its description the way `devrun task` does. An `--arg` the template never reads is refused too. With the built-in `commit_message`, an agent whose git author is already the agent meets a `coauthors` requirement for agents without `--coauthor`. The author email is a GitHub bot address (`123+name[bot]@users.noreply.github.com`), from `GIT_AUTHOR_NAME` and `GIT_AUTHOR_EMAIL` or from `user.name` and `user.email`. The commit then carries no `Co-authored-by` trailer. A person's author still needs `--coauthor`. A project's own `commit_message` still asks. The other agent-only requirements stay. - Every template sees `prefix` (`defaults.branch_prefix`), `branch`, and `ticket` (also named `issue`), `slug` and `apps` from the worktree's `.devkit/issue.toml`. Any other name it reads is an arg, including those the checkout lacks: outside a workspace, `render branch --arg slug=add-login` names a new branch. In one, `render pr_body --arg input=...` prints the body `devkit pr create --pr-body ...` would send. - The built-ins are every template `workspace`, `ticket` and `devkit pr` render, `branch`, `worktree_dir`, `checkout_worktree_dir`, `issue_summary_path`, `issue_summary`, `pr_title`, `pr_body`, `ticket_title`, `ticket_body` (also rendered by their old names `issue_title` and `issue_body`), `review_request` and `review_finish`, plus `commit_message`, which `devkit commit` renders. `render` applies no length limit, so `branch_max` and the other `*_max` settings do not shorten its output, and `short_slug` is an arg. - A project's own templates live under `[templates.custom.]`, with a `body` and a `description`; `devkit schema` has the details. A custom template wins over a built-in of the same name. diff --git a/tests/commit.rs b/tests/commit.rs index 19a8188c0..ac7467987 100644 --- a/tests/commit.rs +++ b/tests/commit.rs @@ -221,19 +221,205 @@ fn the_project_commit_message_template_renders_the_message() { assert_eq!(message(dir), "fix: a [ENG-1]\n\n"); } +fn require_agent_coauthors(dir: &Path) { + std::fs::write( + dir.join("devkit.toml"), + "[templates.variables]\ncoauthors = { default = \"\", required = \"agents\" }\n", + ) + .unwrap(); +} + #[test] fn an_agent_must_name_its_coauthors_when_the_project_says_so() { + let repo = repo(); + let dir = repo.path(); + require_agent_coauthors(dir); + std::fs::write(dir.join("a.txt"), lines("a.txt", &[1])).unwrap(); + let before = head(dir); + + let out = devkit_commit(dir) + .env("DEVKIT_CALLER", "agent") + .args(["--files", "a.txt", "--subject", "fix: a"]) + .output() + .unwrap(); + + assert!(!out.status.success()); + assert!( + stderr(&out).contains("devkit commit needs --coauthor=... (required for agents)"), + "{}", + stderr(&out) + ); + assert_eq!(head(dir), before); +} + +/// An agent author, as a caller sets it when the harness itself authors the +/// commit: a GitHub bot address, not a person's. +const AGENT_AUTHOR: (&str, &str) = ( + "Grok 4.7", + "261865991+grok-by-xai[bot]@users.noreply.github.com", +); + +fn author_env(cmd: &mut Command, name: &str, email: &str) { + cmd.env("GIT_AUTHOR_NAME", name) + .env("GIT_AUTHOR_EMAIL", email) + .env("GIT_COMMITTER_NAME", name) + .env("GIT_COMMITTER_EMAIL", email); +} + +#[test] +fn an_agent_author_meets_a_coauthor_requirement_with_no_trailer() { + let repo = repo(); + let dir = repo.path(); + require_agent_coauthors(dir); + std::fs::write(dir.join("a.txt"), lines("a.txt", &[1])).unwrap(); + + let mut cmd = devkit_commit(dir); + author_env(&mut cmd, AGENT_AUTHOR.0, AGENT_AUTHOR.1); + let out = cmd + .env("DEVKIT_CALLER", "agent") + .env_remove("GIT_CONFIG_GLOBAL") + .args(["--files", "a.txt", "--subject", "fix: a"]) + .output() + .unwrap(); + + assert!(out.status.success(), "{}", stderr(&out)); + assert_eq!(message(dir), "fix: a\n\n"); + assert!( + !message(dir).contains("Co-authored-by"), + "{}", + message(dir) + ); + assert_eq!( + git(dir, &["log", "-1", "--format=%an <%ae>"]), + format!("{} <{}>\n", AGENT_AUTHOR.0, AGENT_AUTHOR.1) + ); +} + +#[test] +fn an_agent_author_from_user_config_meets_a_coauthor_requirement() { + let repo = repo(); + let dir = repo.path(); + require_agent_coauthors(dir); + std::fs::write(dir.join("a.txt"), lines("a.txt", &[1])).unwrap(); + let home = dir.join(".git/test-home"); + std::fs::create_dir_all(&home).unwrap(); + std::fs::write( + home.join(".gitconfig"), + format!( + "[user]\n\tname = {}\n\temail = {}\n", + AGENT_AUTHOR.0, AGENT_AUTHOR.1 + ), + ) + .unwrap(); + + let out = devkit_commit(dir) + .env("DEVKIT_CALLER", "agent") + .env("GIT_CONFIG_GLOBAL", home.join(".gitconfig")) + .env_remove("GIT_AUTHOR_NAME") + .env_remove("GIT_AUTHOR_EMAIL") + .env_remove("GIT_COMMITTER_NAME") + .env_remove("GIT_COMMITTER_EMAIL") + .args(["--files", "a.txt", "--subject", "fix: a"]) + .output() + .unwrap(); + + assert!(out.status.success(), "{}", stderr(&out)); + assert_eq!(message(dir), "fix: a\n\n"); + assert_eq!( + git(dir, &["log", "-1", "--format=%an <%ae>"]), + format!("{} <{}>\n", AGENT_AUTHOR.0, AGENT_AUTHOR.1) + ); +} + +#[test] +fn a_blank_author_email_still_needs_a_coauthor() { + let repo = repo(); + let dir = repo.path(); + require_agent_coauthors(dir); + std::fs::write(dir.join("a.txt"), lines("a.txt", &[1])).unwrap(); + let before = head(dir); + + let mut cmd = devkit_commit(dir); + author_env(&mut cmd, "Grok 4.7", " "); + let out = cmd + .env("DEVKIT_CALLER", "agent") + .args(["--files", "a.txt", "--subject", "fix: a"]) + .output() + .unwrap(); + + assert!(!out.status.success(), "{}", stderr(&out)); + assert!( + stderr(&out).contains("devkit commit needs --coauthor=... (required for agents)"), + "{}", + stderr(&out) + ); + assert_eq!(head(dir), before); +} + +#[test] +fn a_spaced_bot_author_meets_a_coauthor_requirement() { + let repo = repo(); + let dir = repo.path(); + require_agent_coauthors(dir); + std::fs::write(dir.join("a.txt"), lines("a.txt", &[1])).unwrap(); + + let mut cmd = devkit_commit(dir); + author_env( + &mut cmd, + " Grok 4.7 ", + " 261865991+grok-by-xai[bot]@users.noreply.github.com ", + ); + let out = cmd + .env("DEVKIT_CALLER", "agent") + .args(["--files", "a.txt", "--subject", "fix: a"]) + .output() + .unwrap(); + + assert!(out.status.success(), "{}", stderr(&out)); + assert_eq!(message(dir), "fix: a\n\n"); +} + +#[test] +fn a_noreply_address_without_bot_still_needs_a_coauthor() { + let repo = repo(); + let dir = repo.path(); + require_agent_coauthors(dir); + std::fs::write(dir.join("a.txt"), lines("a.txt", &[1])).unwrap(); + let before = head(dir); + + let mut cmd = devkit_commit(dir); + author_env(&mut cmd, "Ada", "ada@users.noreply.github.com"); + let out = cmd + .env("DEVKIT_CALLER", "agent") + .args(["--files", "a.txt", "--subject", "fix: a"]) + .output() + .unwrap(); + + assert!(!out.status.success()); + assert!( + stderr(&out).contains("required for agents"), + "{}", + stderr(&out) + ); + assert_eq!(head(dir), before); +} + +#[test] +fn a_project_commit_message_still_requires_coauthors_of_an_agent_author() { let repo = repo(); let dir = repo.path(); std::fs::write( dir.join("devkit.toml"), - "[templates.variables]\ncoauthors = { default = \"\", required = \"agents\" }\n", + "[templates]\ncommit_message = \"{{ subject }}\\n\\ntrailers: {{ coauthors }}\"\n\ + [templates.variables]\ncoauthors = { default = \"\", required = \"agents\" }\n", ) .unwrap(); std::fs::write(dir.join("a.txt"), lines("a.txt", &[1])).unwrap(); let before = head(dir); - let out = devkit_commit(dir) + let mut cmd = devkit_commit(dir); + author_env(&mut cmd, AGENT_AUTHOR.0, AGENT_AUTHOR.1); + let out = cmd .env("DEVKIT_CALLER", "agent") .args(["--files", "a.txt", "--subject", "fix: a"]) .output() @@ -248,6 +434,25 @@ fn an_agent_must_name_its_coauthors_when_the_project_says_so() { assert_eq!(head(dir), before); } +#[test] +fn a_human_with_an_agent_author_still_names_coauthors_only_when_required_of_humans() { + let repo = repo(); + let dir = repo.path(); + require_agent_coauthors(dir); + std::fs::write(dir.join("a.txt"), lines("a.txt", &[1])).unwrap(); + + let mut cmd = devkit_commit(dir); + author_env(&mut cmd, AGENT_AUTHOR.0, AGENT_AUTHOR.1); + let out = cmd + .env("DEVKIT_CALLER", "human") + .args(["--files", "a.txt", "--subject", "fix: a"]) + .output() + .unwrap(); + + assert!(out.status.success(), "{}", stderr(&out)); + assert_eq!(message(dir), "fix: a\n\n"); +} + #[test] fn amend_rewords_the_last_commit_and_keeps_staged_changes() { let repo = repo(); From f4c1b42fe5b0342659fa3a58c00fb49b6a2c83dd Mon Sep 17 00:00:00 2001 From: Lev Velykoivanenko Date: Sat, 17 Oct 2026 23:10:16 +0000 Subject: [PATCH 2/5] fix(commit): check the author git records An amend keeps the existing author, and author.email overrides user.email. The co-author exemption follows that author, and only the unchanged built-in commit_message. A custom template still needs --coauthor. The bot form is a numeric id, a name, and a terminal [bot]. Claude and Codex noreply addresses stand too. The supplied body is left as written. Co-authored-by: Grok 4.7 <261865991+grok-by-xai[bot]@users.noreply.github.com> --- crates/devkit-git/src/backend.rs | 35 +- crates/devkit-git/src/commit.rs | 11 +- crates/devkit-ports/src/templates.rs | 104 +++-- crates/devkit-vcs/src/lib.rs | 13 +- .../skills/using-devkit/references/commit.md | 2 +- .../using-devkit/references/templates.md | 2 +- src/bin/devkit/commit.rs | 1 + tests/commit.rs | 401 +++++++++++++++++- 8 files changed, 506 insertions(+), 63 deletions(-) diff --git a/crates/devkit-git/src/backend.rs b/crates/devkit-git/src/backend.rs index b1943026f..f7f474b67 100644 --- a/crates/devkit-git/src/backend.rs +++ b/crates/devkit-git/src/backend.rs @@ -361,25 +361,42 @@ impl VersionControl for GitBackend { } fn author(&self, dir: &Path) -> Result> { - // The same order `git commit` uses: an author variable wins over - // config, and a variable set for only one half does not mix with - // the other half from config. + // GIT_AUTHOR_* wins, and one half set alone does not mix with + // config. author.* overrides user.*. let from_env = ["GIT_AUTHOR_NAME", "GIT_AUTHOR_EMAIL"] .map(|key| std::env::var(key).ok().filter(|v| !v.trim().is_empty())); let [name, email] = match from_env { [Some(name), Some(email)] => [name, email], - [None, None] => ["user.name", "user.email"].map(|key| { - Git::at(dir) - .args(["config", key]) - .output() - .map(|v| v.trim().to_string()) - .unwrap_or_default() + [None, None] => ["name", "email"].map(|half| { + for key in [format!("author.{half}"), format!("user.{half}")] { + if let Ok(value) = Git::at(dir).args(["config", &key]).output() { + let value = value.trim().to_string(); + if !value.is_empty() { + return value; + } + } + } + String::new() }), _ => return Ok(None), }; Ok((!name.trim().is_empty() && !email.trim().is_empty()).then_some((name, email))) } + fn head_author(&self, dir: &Path) -> Result> { + let out = Git::at(dir) + .args(["log", "-1", "--format=%an%x00%ae"]) + .wait()?; + if !out.status.success() { + return Ok(None); + } + let text = String::from_utf8_lossy(&out.stdout); + let mut parts = text.trim_end_matches(['\n', '\r']).splitn(2, '\0'); + let name = parts.next().unwrap_or_default().to_string(); + let email = parts.next().unwrap_or_default().to_string(); + Ok((!name.trim().is_empty() && !email.trim().is_empty()).then_some((name, email))) + } + fn commit(&self, dir: &Path, selection: &Selection<'_>, message: &str) -> Result { super::commit::commit(dir, selection, message) } diff --git a/crates/devkit-git/src/commit.rs b/crates/devkit-git/src/commit.rs index 634ad0468..070eb274c 100644 --- a/crates/devkit-git/src/commit.rs +++ b/crates/devkit-git/src/commit.rs @@ -62,7 +62,14 @@ pub(crate) fn commit(dir: &Path, selection: &Selection<'_>, message: &str) -> Re } Selection::Amend => report( Git::at(dir) - .args(["commit", "--amend", "--only", "-m", message]) + .args([ + "commit", + "--amend", + "--only", + "--cleanup=verbatim", + "-m", + message, + ]) .timeout(SLOW_TIMEOUT) .wait()?, ), @@ -314,7 +321,7 @@ fn commit_locked( let wrappers = write_wrappers(&scratch, &state)?; held.retain = true; let out = private(top, &candidate, &wrappers)? - .args(["commit", "-m", message]) + .args(["commit", "--cleanup=verbatim", "-m", message]) .timeout(SLOW_TIMEOUT) .wait()?; let proposed = fs::read_to_string(&state.proposed).ok(); diff --git a/crates/devkit-ports/src/templates.rs b/crates/devkit-ports/src/templates.rs index cebbb00ae..f67cfae7c 100644 --- a/crates/devkit-ports/src/templates.rs +++ b/crates/devkit-ports/src/templates.rs @@ -189,6 +189,7 @@ pub fn commit_message( message: &CommitMessage<'_>, given: &BTreeMap, caller: Caller, + amend: bool, ) -> Result { render_commit_parts( cfg, @@ -197,6 +198,7 @@ pub fn commit_message( message, given, caller, + amend, ) } @@ -208,32 +210,48 @@ fn render_commit_parts( message: &CommitMessage<'_>, given: &BTreeMap, caller: Caller, + amend: bool, ) -> Result { - let coauthors = message.coauthors.join("; "); - let stand_in = (message.coauthors.is_empty() - && cfg.templates.commit_message.is_none() - && devkit_common::required::is_required(cfg, None, "coauthors", caller)) - .then(|| agent_author(start, caller)) - .transpose()? - .flatten(); - let supplied = stand_in - .as_deref() + let coauthors = message + .coauthors + .iter() + .filter(|c| !c.trim().is_empty()) + .cloned() + .collect::>() + .join("; "); + // An agent author meets the requirement without a trailer. The stand-in + // is empty so the built-in template emits no Co-authored-by line, and + // the supplied body is left as the caller wrote it. + let agent = coauthors.is_empty() + && builtin_commit_message(cfg) + && devkit_common::required::is_required(cfg, None, "coauthors", caller) + && agent_author(start, caller, amend)?.is_some(); + let supplied = agent + .then_some("") .or((!coauthors.is_empty()).then_some(coauthors.as_str())); let parts = [message.subject, message.body, supplied]; - let mut text = render_commit_message(cfg, start, parts, given, caller, what)?; - if stand_in.is_some() { - text = drop_coauthor_trailers(&text); - } - Ok(text) + render_commit_message(cfg, start, parts, given, caller, what) +} + +/// The exemption applies only to the unchanged built-in `commit_message`. +/// A `[templates].commit_message` override and a custom template of that +/// name still require `--coauthor`. +fn builtin_commit_message(cfg: &Config) -> bool { + cfg.templates.commit_message.is_none() && !cfg.templates.custom.contains_key(COMMIT_MESSAGE) } /// The git author when it is the agent, as `Name `. A person's author -/// and a human caller get `None`. -fn agent_author(start: &Path, caller: Caller) -> Result> { +/// and a human caller get `None`. An amend keeps the existing commit's +/// author, so that author is the one checked. +fn agent_author(start: &Path, caller: Caller, amend: bool) -> Result> { if caller != Caller::Agent { return Ok(None); } - let Some((name, email)) = Vcs::at(start).author(start)? else { + let Some((name, email)) = (if amend { + Vcs::at(start).head_author(start)? + } else { + Vcs::at(start).author(start)? + }) else { return Ok(None); }; if !agent_email(&email) { @@ -242,36 +260,34 @@ fn agent_author(start: &Path, caller: Caller) -> Result> { Ok(Some(format!("{} <{}>", name.trim(), email.trim()))) } -/// A GitHub bot address, the form a harness uses when it authors a commit -/// itself (`123+name[bot]@users.noreply.github.com`). A person's address, -/// including `users.noreply.github.com` without `[bot]`, is not one. +/// Addresses an agent harness records when it authors a commit itself, and +/// that have no GitHub bot form: Claude and Codex. +const AGENT_EMAILS: [&str; 2] = ["noreply@anthropic.com", "noreply@openai.com"]; + +/// A GitHub bot address, or one of [`AGENT_EMAILS`]. The bot form is a +/// numeric id, `+`, a nonempty name, and a terminal `[bot]`, at +/// `users.noreply.github.com`. fn agent_email(email: &str) -> bool { - let Some((local, host)) = email.trim().rsplit_once('@') else { + let email = email.trim(); + if AGENT_EMAILS + .iter() + .any(|known| email.eq_ignore_ascii_case(known)) + { + return true; + } + let Some((local, host)) = email.rsplit_once('@') else { return false; }; - host.eq_ignore_ascii_case("users.noreply.github.com") && local.contains("[bot]") -} - -/// The message without `Co-authored-by` trailers, and without a blank line -/// that existed only to hold them. -fn drop_coauthor_trailers(message: &str) -> String { - let kept: Vec<&str> = message - .lines() - .filter(|line| { - !line - .trim() - .to_ascii_lowercase() - .starts_with("co-authored-by:") - }) - .collect(); - let mut text = kept.join("\n"); - while text.contains("\n\n\n") { - text = text.replace("\n\n\n", "\n\n"); - } - if !text.ends_with('\n') { - text.push('\n'); + if !host.eq_ignore_ascii_case("users.noreply.github.com") { + return false; } - text + let Some((id, rest)) = local.split_once('+') else { + return false; + }; + let Some(name) = rest.strip_suffix("[bot]") else { + return false; + }; + !id.is_empty() && id.bytes().all(|b| b.is_ascii_digit()) && !name.is_empty() } /// Whether `devkit commit` can leave out `part`: every part but the subject, @@ -443,7 +459,7 @@ pub fn render( ); } let what = format!("template `{name}`"); - return render_commit_parts(cfg, start, (&what, names), parts, given, caller); + return render_commit_parts(cfg, start, (&what, names), parts, given, caller, false); } let [subject, body, coauthor] = COMMIT_PARTS.map(|(_, flag)| names.part(flag)); ensure!( diff --git a/crates/devkit-vcs/src/lib.rs b/crates/devkit-vcs/src/lib.rs index a2bc158f7..81e546363 100644 --- a/crates/devkit-vcs/src/lib.rs +++ b/crates/devkit-vcs/src/lib.rs @@ -289,7 +289,8 @@ pub trait VersionControl { /// configured. fn user_email(&self, dir: &::std::path::Path) -> ::anyhow::Result<::std::string::String>; - /// The author a new commit in `dir` would record (`GIT_AUTHOR_*`, else `user.name` and `user.email`), or `Ok(None)` when either half is unset. + /// The author a new commit in `dir` would record (`GIT_AUTHOR_*`, else + /// `user.name` and `user.email`), or `Ok(None)` when either half is unset. fn author( &self, dir: &::std::path::Path, @@ -298,6 +299,16 @@ pub trait VersionControl { Ok(None) } + /// The author of the commit `HEAD` names, or `Ok(None)` when there is no + /// commit. An amend keeps this author. + fn head_author( + &self, + dir: &::std::path::Path, + ) -> ::anyhow::Result> { + let _ = dir; + Ok(None) + } + /// Commits `selection` in `dir`'s repository with `message`, leaving the /// working tree, and changes anyone else staged, as they were. Refuses /// rather than committing part of the selection. Returns the backend's diff --git a/plugin/skills/using-devkit/references/commit.md b/plugin/skills/using-devkit/references/commit.md index e1d2ccf3f..f29db0154 100644 --- a/plugin/skills/using-devkit/references/commit.md +++ b/plugin/skills/using-devkit/references/commit.md @@ -9,6 +9,6 @@ devkit commit --amend --subject '...' # a new message for the last ``` - To commit part of a file, write only the hunks to commit as a patch against HEAD (`git diff -- path > hunks.patch`, then trim it) and pass it to `--patch`. A patch touching a path whose merge driver could not keep the staged hunks exact is refused. -- The message is the subject, then `--body` after a blank line, then a `Co-authored-by` trailer per `--coauthor`. `devkit template show commit_message` prints the project's template, each message part by its `devkit commit` flag with who must pass it, and any `--arg` it reads. Only `--subject` is required unless the project requires another part, as a project can require `--coauthor` of agents. An agent author meets that requirement with no trailer; `references/templates.md` says how. `devkit template render commit_message` previews the message, taking `--subject`, `--body` and `--coauthor` as `devkit commit` does, and needs no more than it does. +- The message is the subject, then `--body` after a blank line, then a `Co-authored-by` trailer per `--coauthor`. `devkit template show commit_message` prints the project's template, each message part by its `devkit commit` flag with who must pass it, and any `--arg` it reads. Only `--subject` is required unless the project requires another part, as a project can require `--coauthor` of agents. An agent author meets that requirement with no trailer when the built-in `commit_message` is unchanged; `references/templates.md` says how. A custom `commit_message` still needs `--coauthor`. `devkit template render commit_message` previews the message, taking `--subject`, `--body` and `--coauthor` as `devkit commit` does, and needs no more than it does. - Commit hooks and signing run as they do for `git commit`. A refused or failed commit leaves HEAD and the index unchanged, and says why. With `--files` or `--patch`, a commit hook that changes the committed tree is refused too. - When the output says recovery files were retained, the new HEAD could not be confirmed: inspect `git log` before retrying, and do not run the commit again blind. diff --git a/plugin/skills/using-devkit/references/templates.md b/plugin/skills/using-devkit/references/templates.md index ca0b5a2b3..9b6a9e8f1 100644 --- a/plugin/skills/using-devkit/references/templates.md +++ b/plugin/skills/using-devkit/references/templates.md @@ -10,7 +10,7 @@ devkit template render --arg-file k=notes.md # a multi-line value from a ``` - `render` prints the text byte for byte, so `> file` or a pipe gets it unchanged. `--json` on all three verbs emits JSON instead; `render --json` is `{"text": ...}`. -- A required arg left out is refused before anything renders, naming each `--arg` and its description the way `devrun task` does. An `--arg` the template never reads is refused too. With the built-in `commit_message`, an agent whose git author is already the agent meets a `coauthors` requirement for agents without `--coauthor`. The author email is a GitHub bot address (`123+name[bot]@users.noreply.github.com`), from `GIT_AUTHOR_NAME` and `GIT_AUTHOR_EMAIL` or from `user.name` and `user.email`. The commit then carries no `Co-authored-by` trailer. A person's author still needs `--coauthor`. A project's own `commit_message` still asks. The other agent-only requirements stay. +- A required arg left out is refused before anything renders, naming each `--arg` and its description the way `devrun task` does. An `--arg` the template never reads is refused too. With the unchanged built-in `commit_message`, an agent whose git author is already the agent meets a `coauthors` requirement for agents without `--coauthor`. The author email is the strict GitHub bot form (`123+name[bot]@users.noreply.github.com`: digits, `+`, a nonempty name, a terminal `[bot]`, that domain) or a built-in address, Claude `noreply@anthropic.com` or Codex `noreply@openai.com`. It comes from `GIT_AUTHOR_NAME` and `GIT_AUTHOR_EMAIL`, or from `author.name` and `author.email`, or from `user.name` and `user.email`. An amend uses the author the commit already has. The commit then carries no `Co-authored-by` trailer. A person's author still needs `--coauthor`. A `[templates].commit_message` override and a `[templates.custom.commit_message]` override still need `--coauthor`. The other agent-only requirements stay. - Every template sees `prefix` (`defaults.branch_prefix`), `branch`, and `ticket` (also named `issue`), `slug` and `apps` from the worktree's `.devkit/issue.toml`. Any other name it reads is an arg, including those the checkout lacks: outside a workspace, `render branch --arg slug=add-login` names a new branch. In one, `render pr_body --arg input=...` prints the body `devkit pr create --pr-body ...` would send. - The built-ins are every template `workspace`, `ticket` and `devkit pr` render, `branch`, `worktree_dir`, `checkout_worktree_dir`, `issue_summary_path`, `issue_summary`, `pr_title`, `pr_body`, `ticket_title`, `ticket_body` (also rendered by their old names `issue_title` and `issue_body`), `review_request` and `review_finish`, plus `commit_message`, which `devkit commit` renders. `render` applies no length limit, so `branch_max` and the other `*_max` settings do not shorten its output, and `short_slug` is an arg. - A project's own templates live under `[templates.custom.]`, with a `body` and a `description`; `devkit schema` has the details. A custom template wins over a built-in of the same name. diff --git a/src/bin/devkit/commit.rs b/src/bin/devkit/commit.rs index e0e07ce24..7626611ec 100644 --- a/src/bin/devkit/commit.rs +++ b/src/bin/devkit/commit.rs @@ -83,6 +83,7 @@ pub fn run(cli: CommitCli) -> Result<()> { &cli.message.parts(), &cli.vars.parse()?, devkit_common::caller::caller(), + cli.amend, )?; // git applies the patch from the repository's top, not from here. let patch = cli diff --git a/tests/commit.rs b/tests/commit.rs index ac7467987..659ce675e 100644 --- a/tests/commit.rs +++ b/tests/commit.rs @@ -284,11 +284,7 @@ fn an_agent_author_meets_a_coauthor_requirement_with_no_trailer() { assert!(out.status.success(), "{}", stderr(&out)); assert_eq!(message(dir), "fix: a\n\n"); - assert!( - !message(dir).contains("Co-authored-by"), - "{}", - message(dir) - ); + assert!(!message(dir).contains("Co-authored-by"), "{}", message(dir)); assert_eq!( git(dir, &["log", "-1", "--format=%an <%ae>"]), format!("{} <{}>\n", AGENT_AUTHOR.0, AGENT_AUTHOR.1) @@ -331,6 +327,144 @@ fn an_agent_author_from_user_config_meets_a_coauthor_requirement() { ); } +/// `user.*` in a gitconfig the test points `GIT_CONFIG_GLOBAL` at, so the +/// repository's own config stays unset. +fn user_config(dir: &Path, name: &str, email: &str) -> std::path::PathBuf { + let home = dir.join(".git/test-home"); + std::fs::create_dir_all(&home).unwrap(); + let path = home.join(".gitconfig"); + std::fs::write( + &path, + format!("[user]\n\tname = {name}\n\temail = {email}\n"), + ) + .unwrap(); + path +} + +fn commit_with_config(dir: &Path, config: &Path) -> Output { + devkit_commit(dir) + .env("DEVKIT_CALLER", "agent") + .env("GIT_CONFIG_GLOBAL", config) + .env_remove("GIT_AUTHOR_NAME") + .env_remove("GIT_AUTHOR_EMAIL") + .env_remove("GIT_COMMITTER_NAME") + .env_remove("GIT_COMMITTER_EMAIL") + .args(["--files", "a.txt", "--subject", "fix: a"]) + .output() + .unwrap() +} + +#[test] +fn author_email_set_to_a_person_over_a_bot_user_email_still_needs_a_coauthor() { + let repo = repo(); + let dir = repo.path(); + require_agent_coauthors(dir); + std::fs::write(dir.join("a.txt"), lines("a.txt", &[1])).unwrap(); + let config = user_config(dir, AGENT_AUTHOR.0, AGENT_AUTHOR.1); + let text = std::fs::read_to_string(&config).unwrap(); + std::fs::write( + &config, + format!("{text}[author]\n\tname = Ada Lovelace\n\temail = ada@example.invalid\n"), + ) + .unwrap(); + let before = head(dir); + + let out = commit_with_config(dir, &config); + + assert!(!out.status.success(), "{}", stderr(&out)); + assert!( + stderr(&out).contains("devkit commit needs --coauthor=... (required for agents)"), + "{}", + stderr(&out) + ); + assert_eq!(head(dir), before); +} + +#[test] +fn a_bot_author_email_over_a_person_user_email_needs_no_coauthor() { + let repo = repo(); + let dir = repo.path(); + require_agent_coauthors(dir); + std::fs::write(dir.join("a.txt"), lines("a.txt", &[1])).unwrap(); + let config = user_config(dir, "Ada Lovelace", "ada@example.invalid"); + let text = std::fs::read_to_string(&config).unwrap(); + std::fs::write( + &config, + format!( + "{text}[author]\n\tname = {}\n\temail = {}\n", + AGENT_AUTHOR.0, AGENT_AUTHOR.1 + ), + ) + .unwrap(); + + let out = commit_with_config(dir, &config); + + assert!(out.status.success(), "{}", stderr(&out)); + assert_eq!(message(dir), "fix: a\n\n"); + assert!(!message(dir).contains("Co-authored-by"), "{}", message(dir)); + assert_eq!( + git(dir, &["log", "-1", "--format=%an <%ae>"]), + format!("{} <{}>\n", AGENT_AUTHOR.0, AGENT_AUTHOR.1) + ); +} + +#[test] +fn a_blank_coauthor_still_needs_a_coauthor() { + let repo = repo(); + let dir = repo.path(); + require_agent_coauthors(dir); + std::fs::write(dir.join("a.txt"), lines("a.txt", &[1])).unwrap(); + let before = head(dir); + + let out = devkit_commit(dir) + .env("DEVKIT_CALLER", "agent") + .args([ + "--files", + "a.txt", + "--subject", + "fix: a", + "--coauthor", + " ", + ]) + .output() + .unwrap(); + + assert!(!out.status.success(), "{}", stderr(&out)); + assert!( + stderr(&out).contains("devkit commit needs --coauthor=... (required for agents)"), + "{}", + stderr(&out) + ); + assert_eq!(head(dir), before); +} + +#[test] +fn a_blank_coauthor_does_not_block_an_agent_author() { + let repo = repo(); + let dir = repo.path(); + require_agent_coauthors(dir); + std::fs::write(dir.join("a.txt"), lines("a.txt", &[1])).unwrap(); + + let mut cmd = devkit_commit(dir); + author_env(&mut cmd, AGENT_AUTHOR.0, AGENT_AUTHOR.1); + let out = cmd + .env("DEVKIT_CALLER", "agent") + .args([ + "--files", + "a.txt", + "--subject", + "fix: a", + "--coauthor", + " ", + ]) + .output() + .unwrap(); + + assert!(out.status.success(), "{}", stderr(&out)); + assert_eq!(message(dir), "fix: a\n\n"); + assert!(!message(dir).contains("Co-authored-by")); +} + #[test] fn a_blank_author_email_still_needs_a_coauthor() { let repo = repo(); @@ -379,6 +513,126 @@ fn a_spaced_bot_author_meets_a_coauthor_requirement() { assert_eq!(message(dir), "fix: a\n\n"); } +fn bot_author_commit(dir: &Path, email: &str) -> (Output, String) { + std::fs::write(dir.join("a.txt"), lines("a.txt", &[1])).unwrap(); + let before = head(dir); + let mut cmd = devkit_commit(dir); + author_env(&mut cmd, "Grok 4.7", email); + let out = cmd + .env("DEVKIT_CALLER", "agent") + .args(["--files", "a.txt", "--subject", "fix: a"]) + .output() + .unwrap(); + (out, before) +} + +fn assert_bot_author_refused(email: &str) { + let repo = repo(); + let dir = repo.path(); + require_agent_coauthors(dir); + let (out, before) = bot_author_commit(dir, email); + assert!(!out.status.success(), "{email}: {}", stderr(&out)); + assert!( + stderr(&out).contains("devkit commit needs --coauthor=... (required for agents)"), + "{email}: {}", + stderr(&out) + ); + assert_eq!(head(dir), before, "{email}"); +} + +fn assert_bot_author_stands(email: &str) { + let repo = repo(); + let dir = repo.path(); + require_agent_coauthors(dir); + let (out, _) = bot_author_commit(dir, email); + assert!(out.status.success(), "{email}: {}", stderr(&out)); + assert_eq!(message(dir), "fix: a\n\n"); + assert!(!message(dir).contains("Co-authored-by"), "{email}"); +} + +#[test] +fn a_bot_address_with_no_digits_still_needs_a_coauthor() { + assert_bot_author_refused("grok[bot]@users.noreply.github.com"); +} + +#[test] +fn a_bot_address_with_a_nonnumeric_id_still_needs_a_coauthor() { + assert_bot_author_refused("abc+grok[bot]@users.noreply.github.com"); +} + +#[test] +fn a_bot_address_missing_the_bot_marker_still_needs_a_coauthor() { + assert_bot_author_refused("123+grok@users.noreply.github.com"); +} + +#[test] +fn a_bot_address_on_the_wrong_domain_still_needs_a_coauthor() { + assert_bot_author_refused("123+grok[bot]@example.invalid"); +} + +#[test] +fn an_uppercase_bot_marker_still_needs_a_coauthor() { + assert_bot_author_refused("123+grok[BOT]@users.noreply.github.com"); +} + +#[test] +fn a_bot_marker_in_the_middle_of_the_name_still_needs_a_coauthor() { + assert_bot_author_refused("123+grok[bot]human@users.noreply.github.com"); +} + +#[test] +fn a_bot_address_with_an_empty_name_still_needs_a_coauthor() { + assert_bot_author_refused("123+[bot]@users.noreply.github.com"); +} + +#[test] +fn an_uppercase_bot_domain_meets_a_coauthor_requirement() { + assert_bot_author_stands("123+grok[bot]@USERS.NOREPLY.GITHUB.COM"); +} + +#[test] +fn a_bot_name_with_a_plus_meets_a_coauthor_requirement() { + assert_bot_author_stands("123+grok+extra[bot]@users.noreply.github.com"); +} + +#[test] +fn a_claude_author_meets_a_coauthor_requirement() { + assert_bot_author_stands("noreply@anthropic.com"); +} + +#[test] +fn a_codex_author_meets_a_coauthor_requirement() { + assert_bot_author_stands("noreply@openai.com"); +} + +#[test] +fn a_claude_author_from_user_config_meets_a_coauthor_requirement() { + assert_configured_author_stands("Claude", "noreply@anthropic.com"); +} + +#[test] +fn a_codex_author_from_user_config_meets_a_coauthor_requirement() { + assert_configured_author_stands("Codex", "noreply@openai.com"); +} + +fn assert_configured_author_stands(name: &str, email: &str) { + let repo = repo(); + let dir = repo.path(); + require_agent_coauthors(dir); + std::fs::write(dir.join("a.txt"), lines("a.txt", &[1])).unwrap(); + let config = user_config(dir, name, email); + + let out = commit_with_config(dir, &config); + + assert!(out.status.success(), "{email}: {}", stderr(&out)); + assert_eq!(message(dir), "fix: a\n\n"); + assert!(!message(dir).contains("Co-authored-by"), "{email}"); + assert_eq!( + git(dir, &["log", "-1", "--format=%an <%ae>"]), + format!("{name} <{email}>\n") + ); +} + #[test] fn a_noreply_address_without_bot_still_needs_a_coauthor() { let repo = repo(); @@ -404,6 +658,84 @@ fn a_noreply_address_without_bot_still_needs_a_coauthor() { assert_eq!(head(dir), before); } +#[test] +fn an_agent_author_keeps_a_supplied_body_including_an_interior_trailer() { + let repo = repo(); + let dir = repo.path(); + require_agent_coauthors(dir); + std::fs::write(dir.join("a.txt"), lines("a.txt", &[1])).unwrap(); + let body = "Before\n\nCo-authored-by: Ada \n\nAfter"; + + let mut cmd = devkit_commit(dir); + author_env(&mut cmd, AGENT_AUTHOR.0, AGENT_AUTHOR.1); + let out = cmd + .env("DEVKIT_CALLER", "agent") + .args(["--files", "a.txt", "--subject", "fix: a", "--body", body]) + .output() + .unwrap(); + + assert!(out.status.success(), "{}", stderr(&out)); + assert_eq!(message(dir), format!("fix: a\n\n{body}\n\n")); +} + +#[test] +fn an_agent_author_keeps_body_paragraph_spacing() { + let repo = repo(); + let dir = repo.path(); + require_agent_coauthors(dir); + std::fs::write(dir.join("a.txt"), lines("a.txt", &[1])).unwrap(); + + let mut cmd = devkit_commit(dir); + author_env(&mut cmd, AGENT_AUTHOR.0, AGENT_AUTHOR.1); + let out = cmd + .env("DEVKIT_CALLER", "agent") + .args([ + "--files", + "a.txt", + "--subject", + "fix: a", + "--body", + "Before\n\n\nAfter", + ]) + .output() + .unwrap(); + + assert!(out.status.success(), "{}", stderr(&out)); + assert_eq!(message(dir), "fix: a\n\nBefore\n\n\nAfter\n\n"); +} + +#[test] +fn a_custom_commit_message_still_requires_coauthors_of_an_agent_author() { + let repo = repo(); + let dir = repo.path(); + std::fs::write( + dir.join("devkit.toml"), + "[templates.custom.commit_message]\n\ + description = \"Project commit message\"\n\ + body = \"{{ subject }}\\n\\nCo-authored-by: {{ coauthors }}\"\n\ + [templates.variables]\ncoauthors = { default = \"\", required = \"agents\" }\n", + ) + .unwrap(); + std::fs::write(dir.join("a.txt"), lines("a.txt", &[1])).unwrap(); + let before = head(dir); + + let mut cmd = devkit_commit(dir); + author_env(&mut cmd, AGENT_AUTHOR.0, AGENT_AUTHOR.1); + let out = cmd + .env("DEVKIT_CALLER", "agent") + .args(["--files", "a.txt", "--subject", "fix: a"]) + .output() + .unwrap(); + + assert!(!out.status.success(), "{}", stderr(&out)); + assert!( + stderr(&out).contains("devkit commit needs --coauthor=... (required for agents)"), + "{}", + stderr(&out) + ); + assert_eq!(head(dir), before); +} + #[test] fn a_project_commit_message_still_requires_coauthors_of_an_agent_author() { let repo = repo(); @@ -453,6 +785,65 @@ fn a_human_with_an_agent_author_still_names_coauthors_only_when_required_of_huma assert_eq!(message(dir), "fix: a\n\n"); } +/// Rewrite HEAD's author. `git commit --amend` keeps that author, whatever +/// the environment would record on a new commit. +fn rewrite_author(dir: &Path, name: &str, email: &str) { + git(dir, &[ + "commit", + "--amend", + "--no-edit", + &format!("--author={name} <{email}>"), + ]); +} + +#[test] +fn amending_a_person_commit_with_a_bot_environment_still_needs_a_coauthor() { + let repo = repo(); + let dir = repo.path(); + require_agent_coauthors(dir); + let before = head(dir); + + let mut cmd = devkit_commit(dir); + author_env(&mut cmd, AGENT_AUTHOR.0, AGENT_AUTHOR.1); + let out = cmd + .env("DEVKIT_CALLER", "agent") + .args(["--amend", "--subject", "fix: a"]) + .output() + .unwrap(); + + assert!(!out.status.success(), "{}", stderr(&out)); + assert!( + stderr(&out).contains("devkit commit needs --coauthor=... (required for agents)"), + "{}", + stderr(&out) + ); + assert_eq!(head(dir), before); +} + +#[test] +fn amending_a_bot_commit_with_a_person_environment_needs_no_coauthor() { + let repo = repo(); + let dir = repo.path(); + rewrite_author(dir, AGENT_AUTHOR.0, AGENT_AUTHOR.1); + require_agent_coauthors(dir); + + let mut cmd = devkit_commit(dir); + author_env(&mut cmd, "Ada Lovelace", "ada@example.invalid"); + let out = cmd + .env("DEVKIT_CALLER", "agent") + .args(["--amend", "--subject", "fix: a"]) + .output() + .unwrap(); + + assert!(out.status.success(), "{}", stderr(&out)); + assert_eq!(message(dir), "fix: a\n\n"); + assert!(!message(dir).contains("Co-authored-by"), "{}", message(dir)); + assert_eq!( + git(dir, &["log", "-1", "--format=%an <%ae>"]), + format!("{} <{}>\n", AGENT_AUTHOR.0, AGENT_AUTHOR.1) + ); +} + #[test] fn amend_rewords_the_last_commit_and_keeps_staged_changes() { let repo = repo(); From 336bdafe3dbb6e02bdb5a5de6bdcbddf392a5d83 Mon Sep 17 00:00:00 2001 From: Lev Velykoivanenko Date: Sat, 17 Oct 2026 23:23:00 +0000 Subject: [PATCH 3/5] fix(commit): keep always coauthors required The agent-author exemption treated every required coauthors marking as met. required = "always" still needs --coauthor, including a GitHub bot, Claude or Codex author. The exemption now applies only when the declared requirement is agents. Co-authored-by: Grok 4.7 <261865991+grok-by-xai[bot]@users.noreply.github.com> --- crates/devkit-ports/src/templates.rs | 10 +-- .../skills/using-devkit/references/commit.md | 2 +- .../using-devkit/references/templates.md | 2 +- tests/commit.rs | 72 ++++++++++++++++++- 4 files changed, 78 insertions(+), 8 deletions(-) diff --git a/crates/devkit-ports/src/templates.rs b/crates/devkit-ports/src/templates.rs index f67cfae7c..bf4747dd9 100644 --- a/crates/devkit-ports/src/templates.rs +++ b/crates/devkit-ports/src/templates.rs @@ -11,7 +11,7 @@ use anyhow::{Context, Result, anyhow, ensure}; use devkit_common::{ caller::Caller, record::{self, IssueRecord}, - required::{ensure_supplied, ensure_supplied_as, missing_args}, + required::{declared_required, ensure_supplied, ensure_supplied_as, missing_args}, template, vcs::{self, Vcs, VersionControl}, }; @@ -219,12 +219,12 @@ fn render_commit_parts( .cloned() .collect::>() .join("; "); - // An agent author meets the requirement without a trailer. The stand-in - // is empty so the built-in template emits no Co-authored-by line, and - // the supplied body is left as the caller wrote it. + // An agent author meets required = "agents" without a trailer. The + // stand-in is empty so the built-in template emits no Co-authored-by + // line, and the supplied body is left as the caller wrote it. let agent = coauthors.is_empty() && builtin_commit_message(cfg) - && devkit_common::required::is_required(cfg, None, "coauthors", caller) + && declared_required(cfg, None, "coauthors") == Required::Agents && agent_author(start, caller, amend)?.is_some(); let supplied = agent .then_some("") diff --git a/plugin/skills/using-devkit/references/commit.md b/plugin/skills/using-devkit/references/commit.md index f29db0154..1bd1cad08 100644 --- a/plugin/skills/using-devkit/references/commit.md +++ b/plugin/skills/using-devkit/references/commit.md @@ -9,6 +9,6 @@ devkit commit --amend --subject '...' # a new message for the last ``` - To commit part of a file, write only the hunks to commit as a patch against HEAD (`git diff -- path > hunks.patch`, then trim it) and pass it to `--patch`. A patch touching a path whose merge driver could not keep the staged hunks exact is refused. -- The message is the subject, then `--body` after a blank line, then a `Co-authored-by` trailer per `--coauthor`. `devkit template show commit_message` prints the project's template, each message part by its `devkit commit` flag with who must pass it, and any `--arg` it reads. Only `--subject` is required unless the project requires another part, as a project can require `--coauthor` of agents. An agent author meets that requirement with no trailer when the built-in `commit_message` is unchanged; `references/templates.md` says how. A custom `commit_message` still needs `--coauthor`. `devkit template render commit_message` previews the message, taking `--subject`, `--body` and `--coauthor` as `devkit commit` does, and needs no more than it does. +- The message is the subject, then `--body` after a blank line, then a `Co-authored-by` trailer per `--coauthor`. `devkit template show commit_message` prints the project's template, each message part by its `devkit commit` flag with who must pass it, and any `--arg` it reads. Only `--subject` is required unless the project requires another part, as a project can require `--coauthor` of agents. An agent author meets a `required = "agents"` coauthor requirement with no trailer when the built-in `commit_message` is unchanged; `required = "always"` still needs `--coauthor`. `references/templates.md` says how. A custom `commit_message` still needs `--coauthor`. `devkit template render commit_message` previews the message, taking `--subject`, `--body` and `--coauthor` as `devkit commit` does, and needs no more than it does. - Commit hooks and signing run as they do for `git commit`. A refused or failed commit leaves HEAD and the index unchanged, and says why. With `--files` or `--patch`, a commit hook that changes the committed tree is refused too. - When the output says recovery files were retained, the new HEAD could not be confirmed: inspect `git log` before retrying, and do not run the commit again blind. diff --git a/plugin/skills/using-devkit/references/templates.md b/plugin/skills/using-devkit/references/templates.md index 9b6a9e8f1..f1413f116 100644 --- a/plugin/skills/using-devkit/references/templates.md +++ b/plugin/skills/using-devkit/references/templates.md @@ -10,7 +10,7 @@ devkit template render --arg-file k=notes.md # a multi-line value from a ``` - `render` prints the text byte for byte, so `> file` or a pipe gets it unchanged. `--json` on all three verbs emits JSON instead; `render --json` is `{"text": ...}`. -- A required arg left out is refused before anything renders, naming each `--arg` and its description the way `devrun task` does. An `--arg` the template never reads is refused too. With the unchanged built-in `commit_message`, an agent whose git author is already the agent meets a `coauthors` requirement for agents without `--coauthor`. The author email is the strict GitHub bot form (`123+name[bot]@users.noreply.github.com`: digits, `+`, a nonempty name, a terminal `[bot]`, that domain) or a built-in address, Claude `noreply@anthropic.com` or Codex `noreply@openai.com`. It comes from `GIT_AUTHOR_NAME` and `GIT_AUTHOR_EMAIL`, or from `author.name` and `author.email`, or from `user.name` and `user.email`. An amend uses the author the commit already has. The commit then carries no `Co-authored-by` trailer. A person's author still needs `--coauthor`. A `[templates].commit_message` override and a `[templates.custom.commit_message]` override still need `--coauthor`. The other agent-only requirements stay. +- A required arg left out is refused before anything renders, naming each `--arg` and its description the way `devrun task` does. An `--arg` the template never reads is refused too. With the unchanged built-in `commit_message`, an agent whose git author is already the agent meets a `coauthors` requirement marked `required = "agents"` without `--coauthor`. `required = "always"` still needs `--coauthor`, including when that author is a GitHub bot, Claude or Codex. The author email is the strict GitHub bot form (`123+name[bot]@users.noreply.github.com`: digits, `+`, a nonempty name, a terminal `[bot]`, that domain) or a built-in address, Claude `noreply@anthropic.com` or Codex `noreply@openai.com`. It comes from a complete `GIT_AUTHOR_NAME` and `GIT_AUTHOR_EMAIL` pair. One half set, or either half blank, is not an author. With neither variable set, `author.name` wins before `user.name` and `author.email` wins before `user.email`. An amend uses the author the commit already has. The commit then carries no `Co-authored-by` trailer. A person's author still needs `--coauthor`. A `[templates].commit_message` override and a `[templates.custom.commit_message]` override still need `--coauthor`. The other agent-only requirements stay. - Every template sees `prefix` (`defaults.branch_prefix`), `branch`, and `ticket` (also named `issue`), `slug` and `apps` from the worktree's `.devkit/issue.toml`. Any other name it reads is an arg, including those the checkout lacks: outside a workspace, `render branch --arg slug=add-login` names a new branch. In one, `render pr_body --arg input=...` prints the body `devkit pr create --pr-body ...` would send. - The built-ins are every template `workspace`, `ticket` and `devkit pr` render, `branch`, `worktree_dir`, `checkout_worktree_dir`, `issue_summary_path`, `issue_summary`, `pr_title`, `pr_body`, `ticket_title`, `ticket_body` (also rendered by their old names `issue_title` and `issue_body`), `review_request` and `review_finish`, plus `commit_message`, which `devkit commit` renders. `render` applies no length limit, so `branch_max` and the other `*_max` settings do not shorten its output, and `short_slug` is an arg. - A project's own templates live under `[templates.custom.]`, with a `body` and a `description`; `devkit schema` has the details. A custom template wins over a built-in of the same name. diff --git a/tests/commit.rs b/tests/commit.rs index 659ce675e..37886af6a 100644 --- a/tests/commit.rs +++ b/tests/commit.rs @@ -222,9 +222,15 @@ fn the_project_commit_message_template_renders_the_message() { } fn require_agent_coauthors(dir: &Path) { + require_coauthors(dir, "agents"); +} + +fn require_coauthors(dir: &Path, required: &str) { std::fs::write( dir.join("devkit.toml"), - "[templates.variables]\ncoauthors = { default = \"\", required = \"agents\" }\n", + format!( + "[templates.variables]\ncoauthors = {{ default = \"\", required = \"{required}\" }}\n" + ), ) .unwrap(); } @@ -766,6 +772,70 @@ fn a_project_commit_message_still_requires_coauthors_of_an_agent_author() { assert_eq!(head(dir), before); } +/// `required = "always"` binds every caller, so an agent author still names +/// `--coauthor`. The agent-author exemption covers `required = "agents"` only. +fn an_agent_author_still_needs_a_coauthor_when_always_required(email: &str) { + let repo = repo(); + let dir = repo.path(); + require_coauthors(dir, "always"); + std::fs::write(dir.join("a.txt"), lines("a.txt", &[1])).unwrap(); + let before = head(dir); + + let mut cmd = devkit_commit(dir); + author_env(&mut cmd, "Agent", email); + let out = cmd + .env("DEVKIT_CALLER", "agent") + .args(["--files", "a.txt", "--subject", "fix: a"]) + .output() + .unwrap(); + + assert!(!out.status.success(), "{email}: {}", stderr(&out)); + assert!( + stderr(&out).contains("devkit commit needs --coauthor=..."), + "{email}: {}", + stderr(&out) + ); + assert!( + !stderr(&out).contains("required for agents"), + "{email}: {}", + stderr(&out) + ); + assert_eq!(head(dir), before, "{email}"); +} + +#[test] +fn a_github_bot_author_still_needs_a_coauthor_when_always_required() { + an_agent_author_still_needs_a_coauthor_when_always_required(AGENT_AUTHOR.1); +} + +#[test] +fn a_claude_author_still_needs_a_coauthor_when_always_required() { + an_agent_author_still_needs_a_coauthor_when_always_required("noreply@anthropic.com"); +} + +#[test] +fn a_codex_author_still_needs_a_coauthor_when_always_required() { + an_agent_author_still_needs_a_coauthor_when_always_required("noreply@openai.com"); +} + +#[test] +fn a_human_caller_with_required_agents_still_commits_without_a_coauthor() { + let repo = repo(); + let dir = repo.path(); + require_agent_coauthors(dir); + std::fs::write(dir.join("a.txt"), lines("a.txt", &[1])).unwrap(); + + let out = devkit_commit(dir) + .env("DEVKIT_CALLER", "human") + .args(["--files", "a.txt", "--subject", "fix: a"]) + .output() + .unwrap(); + + assert!(out.status.success(), "{}", stderr(&out)); + assert_eq!(message(dir), "fix: a\n\n"); + assert!(!message(dir).contains("Co-authored-by"), "{}", message(dir)); +} + #[test] fn a_human_with_an_agent_author_still_names_coauthors_only_when_required_of_humans() { let repo = repo(); From e7cc69db56fc571f3ab202938866eb6c0cc1f7b7 Mon Sep 17 00:00:00 2001 From: Lev Velykoivanenko Date: Sat, 17 Oct 2026 23:23:05 +0000 Subject: [PATCH 4/5] docs(vcs): state author lookup order author() omitted that author.name and author.email win before user.name and user.email, and that a partial GIT_AUTHOR_NAME/GIT_AUTHOR_EMAIL pair is not an author. Co-authored-by: Grok 4.7 <261865991+grok-by-xai[bot]@users.noreply.github.com> --- crates/devkit-vcs/src/lib.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/devkit-vcs/src/lib.rs b/crates/devkit-vcs/src/lib.rs index 81e546363..1ad6213d5 100644 --- a/crates/devkit-vcs/src/lib.rs +++ b/crates/devkit-vcs/src/lib.rs @@ -289,8 +289,11 @@ pub trait VersionControl { /// configured. fn user_email(&self, dir: &::std::path::Path) -> ::anyhow::Result<::std::string::String>; - /// The author a new commit in `dir` would record (`GIT_AUTHOR_*`, else - /// `user.name` and `user.email`), or `Ok(None)` when either half is unset. + /// The author a new commit in `dir` would record, or `Ok(None)` when + /// either half is unset. A complete `GIT_AUTHOR_NAME` and + /// `GIT_AUTHOR_EMAIL` pair wins. One half set, or either half blank, is + /// not an author. With neither set, `author.name` wins before + /// `user.name` and `author.email` wins before `user.email`. fn author( &self, dir: &::std::path::Path, From 0cc071dacd488a9fd12eaa21ecc6623b67823017 Mon Sep 17 00:00:00 2001 From: Lev Velykoivanenko Date: Sun, 18 Oct 2026 08:53:12 +0200 Subject: [PATCH 5/5] fix(commit): keep git whitespace cleanup Passing --cleanup=verbatim changed how every devkit commit stores its message: trailing spaces and runs of blank lines were kept instead of tidied as git commit -m does. Only an agent-author spacing test relied on it, and the issue asks for no such guarantee, so devkit keeps git's default cleanup. Co-authored-by: Claude Opus 5.5 --- crates/devkit-git/src/commit.rs | 11 ++--------- tests/commit.rs | 26 -------------------------- 2 files changed, 2 insertions(+), 35 deletions(-) diff --git a/crates/devkit-git/src/commit.rs b/crates/devkit-git/src/commit.rs index 070eb274c..634ad0468 100644 --- a/crates/devkit-git/src/commit.rs +++ b/crates/devkit-git/src/commit.rs @@ -62,14 +62,7 @@ pub(crate) fn commit(dir: &Path, selection: &Selection<'_>, message: &str) -> Re } Selection::Amend => report( Git::at(dir) - .args([ - "commit", - "--amend", - "--only", - "--cleanup=verbatim", - "-m", - message, - ]) + .args(["commit", "--amend", "--only", "-m", message]) .timeout(SLOW_TIMEOUT) .wait()?, ), @@ -321,7 +314,7 @@ fn commit_locked( let wrappers = write_wrappers(&scratch, &state)?; held.retain = true; let out = private(top, &candidate, &wrappers)? - .args(["commit", "--cleanup=verbatim", "-m", message]) + .args(["commit", "-m", message]) .timeout(SLOW_TIMEOUT) .wait()?; let proposed = fs::read_to_string(&state.proposed).ok(); diff --git a/tests/commit.rs b/tests/commit.rs index 37886af6a..f89a9efc3 100644 --- a/tests/commit.rs +++ b/tests/commit.rs @@ -684,32 +684,6 @@ fn an_agent_author_keeps_a_supplied_body_including_an_interior_trailer() { assert_eq!(message(dir), format!("fix: a\n\n{body}\n\n")); } -#[test] -fn an_agent_author_keeps_body_paragraph_spacing() { - let repo = repo(); - let dir = repo.path(); - require_agent_coauthors(dir); - std::fs::write(dir.join("a.txt"), lines("a.txt", &[1])).unwrap(); - - let mut cmd = devkit_commit(dir); - author_env(&mut cmd, AGENT_AUTHOR.0, AGENT_AUTHOR.1); - let out = cmd - .env("DEVKIT_CALLER", "agent") - .args([ - "--files", - "a.txt", - "--subject", - "fix: a", - "--body", - "Before\n\n\nAfter", - ]) - .output() - .unwrap(); - - assert!(out.status.success(), "{}", stderr(&out)); - assert_eq!(message(dir), "fix: a\n\nBefore\n\n\nAfter\n\n"); -} - #[test] fn a_custom_commit_message_still_requires_coauthors_of_an_agent_author() { let repo = repo();