a73x

29768e2c

Stamp Patch: trailers from a commit-msg hook

a73x   2026-08-10 11:14

Commit message
Stamp Patch: trailers from a commit-msg hook

Layer 1 of merge recording. Layers 2 and 3 landed already: sync scans a
patch's base branch for `Patch: <id>` trailers, and `patch merge` records
one by hand. Nothing wrote a trailer, so the automatic path never fired.

`git-collab init` now installs a four-line `commit-msg` shim that calls
back into `git-collab hooks run-commit-msg`, so the logic that can be
wrong is Rust and is tested. A commit made on a branch that exactly one
open patch was created from gets the trailer; zero matches, several
matches, a detached HEAD, unreadable state or no binary on PATH all
leave the message byte-identical.

Placement is the subtle part. The file a commit-msg hook receives is not
the message git stores: it still carries git's `#` commentary and, below
a scissors line, a whole diff. Appending at the end of the file puts the
trailer below the commentary, and once that is stripped the trailer
joins whatever paragraph came before it — prose, usually, which our
parser refuses. So the trailer goes at the end of the message *as git
will store it*, joining an existing trailer block rather than starting a
new paragraph that would strand the trailers already there.

Writer and reader share `trailer::final_paragraph`, so "what is a
trailer block" has one implementation rather than two that can drift.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
README.md
Old New
@@ -61,6 +61,23 @@ message along. Or record it by hand:
61 $ git-collab patch merge a1b2c3d4 61 $ git-collab patch merge a1b2c3d4
62 ``` 62 ```
63 63
64 `git-collab init` installs a `commit-msg` hook that writes the trailer for you,
65 on commits made while you are on a branch that exactly one open patch was
66 created from. It is a convenience only — `.git/hooks` is not cloned, so it helps
67 the machine it was installed on and only for commits made afterwards, which is
68 why scanning at sync time stays the mechanism that actually records merges.
69
70 The hook never fails a commit and never touches a message it is unsure about:
71 no matching patch, more than one, a detached HEAD, or no `git-collab` on `PATH`
72 all leave the message exactly as you wrote it. If a `commit-msg` hook of your
73 own is already there, `init` will not touch it and tells you the one line to add
74 if you want the trailer:
75
76 ```console
77 $ git-collab hooks status # is it installed, what would it stamp
78 $ git-collab hooks install # install it on its own
79 ```
80
64 Either way, a patch created with `--fixes` closes the issue it fixes at the same 81 Either way, a patch created with `--fixes` closes the issue it fixes at the same
65 moment. `git-collab issue reopen` then stays reopened: the fix landing and 82 moment. `git-collab issue reopen` then stays reopened: the fix landing and
66 turning out to be wrong is ordinary, and nothing re-closes an issue over a 83 turning out to be wrong is ordinary, and nothing re-closes an issue over a
@@ -84,7 +101,7 @@ $ make install # installs git-collab and git-collab-server, plus m
84 ```console 101 ```console
85 $ cd your-repo 102 $ cd your-repo
86 $ git-collab init-key # generate an Ed25519 signing key 103 $ git-collab init-key # generate an Ed25519 signing key
87 $ git-collab init # add collab refspecs to your remotes 104 $ git-collab init # collab refspecs on your remotes, plus the commit-msg hook
88 105
89 $ git-collab issue open -t "Parser drops trailing newline" 106 $ git-collab issue open -t "Parser drops trailing newline"
90 $ git-collab issue list 107 $ git-collab issue list
@@ -120,6 +137,7 @@ than type.
120 | `issue` | open, list, show, comment, edit, label, assign, close | 137 | `issue` | open, list, show, comment, edit, label, assign, close |
121 | `patch` | create, list, show, diff, comment, review, revise, log, checkout, merge, close | 138 | `patch` | create, list, show, diff, comment, review, revise, log, checkout, merge, close |
122 | `sync` | fetch, reconcile and push collab refs | 139 | `sync` | fetch, reconcile and push collab refs |
140 | `hooks` | install and inspect the `commit-msg` trailer hook |
123 | `status` | project overview | 141 | `status` | project overview |
124 | `dashboard` | interactive TUI | 142 | `dashboard` | interactive TUI |
125 | `search` | full-text across issues and patches | 143 | `search` | full-text across issues and patches |
@@ -132,7 +150,9 @@ Every command takes `--help`, and `man git-collab` covers the same ground.
132 150
133 ## Sync 151 ## Sync
134 152
135 `git-collab init` adds collab refspecs to every remote of the repo. `git-collab 153 `git-collab init` adds collab refspecs to every remote of the repo (and installs
154 the `commit-msg` hook). It is idempotent: running it again configures nothing
155 twice and reports what was already in place. `git-collab
136 sync` (no arguments) syncs all of them — fetch, reconcile, push — and only 156 sync` (no arguments) syncs all of them — fetch, reconcile, push — and only
137 reports success once every one of them has actually succeeded. A remote that 157 reports success once every one of them has actually succeeded. A remote that
138 fails does not stop the others: failures are reported together, alongside 158 fails does not stop the others: failures are reported together, alongside
src/cli.rs
Old New
@@ -103,6 +103,10 @@ pub enum Commands {
103 /// Initialize collab refspecs on all remotes 103 /// Initialize collab refspecs on all remotes
104 Init, 104 Init,
105 105
106 /// Manage the commit-msg hook that stamps `Patch:` trailers
107 #[command(subcommand)]
108 Hooks(HookCmd),
109
106 /// Manage issues 110 /// Manage issues
107 #[command(subcommand)] 111 #[command(subcommand)]
108 Issue(IssueCmd), 112 Issue(IssueCmd),
@@ -179,6 +183,28 @@ pub enum Commands {
179 } 183 }
180 184
181 #[derive(Subcommand)] 185 #[derive(Subcommand)]
186 pub enum HookCmd {
187 /// Install the commit-msg hook (also done by `git-collab init`)
188 Install,
189
190 /// Report whether the hook is installed and what it would stamp
191 Status,
192
193 /// Stamp a commit message file. Called by the hook; not for typing.
194 ///
195 /// Hidden because it is an internal entry point with a shell contract:
196 /// it takes the path git passes a `commit-msg` hook, edits it in place,
197 /// prints nothing, and never fails.
198 #[command(name = "run-commit-msg", hide = true)]
199 RunCommitMsg {
200 // Fully qualified: `src/cli.rs` is `include!`d by `build.rs`, which has
201 // its own `use std::path::PathBuf`, and a second import there collides.
202 /// Path to the commit message file
203 file: std::path::PathBuf,
204 },
205 }
206
207 #[derive(Subcommand)]
182 pub enum IssueCmd { 208 pub enum IssueCmd {
183 /// Open a new issue 209 /// Open a new issue
184 Open { 210 Open {
src/hooks.rs
Old New
@@ -0,0 +1,655 @@
1 //! The `commit-msg` hook that stamps `Patch: <id>` trailers — layer 1 of merge
2 //! recording.
3 //!
4 //! See: docs/superpowers/specs/2026-08-09-merge-recording-design.md
5 //!
6 //! [`merge_scan`](crate::merge_scan) records a merge when it finds `Patch: <id>`
7 //! on the patch's base branch. Something has to put it there. This does, for
8 //! commits made after `git-collab init` ran on this machine — which is all a
9 //! hook can ever cover, since `.git/hooks` is not cloned. Sync-time scanning
10 //! stays the mechanism that works for everyone, retroactively; this only
11 //! removes the need to type the trailer.
12 //!
13 //! The installed hook is a four-line shell shim that calls back into
14 //! `git-collab hooks run-commit-msg`, so the part that can be wrong is Rust and
15 //! is tested (`tests/commit_msg_hook_test.rs`).
16 //!
17 //! Three properties, in the order they matter:
18 //!
19 //! **It fails open.** Every path here returns without touching the message
20 //! rather than reporting an error, and the shim discards our exit status on top
21 //! of that. A `commit-msg` hook that can block a commit is a hook that gets
22 //! deleted, and it would be blocking commits to add a convenience.
23 //!
24 //! **It never clobbers an existing hook.** If a `commit-msg` hook is already
25 //! there and we did not write it, installation declines and says so. See
26 //! [`install`] for why declining rather than chaining.
27 //!
28 //! **It is idempotent.** A message already mentioning a `Patch:` trailer is
29 //! left exactly as it is, which is what makes `git commit --amend` — the hook
30 //! re-running over its own output — a no-op.
31
32 use std::path::{Path, PathBuf};
33
34 use git2::Repository;
35
36 use crate::error::Error;
37 use crate::state::{self, PatchState, PatchStatus};
38 use crate::trailer;
39
40 /// Marks a hook file as ours. Installation only ever overwrites a file
41 /// containing this, so a hook we did not write cannot be lost.
42 const MARKER: &str = "git-collab-managed-hook";
43
44 /// The subcommand the shim calls. Also what [`install`] looks for inside a
45 /// foreign hook to tell "someone wired us in by hand" apart from "someone
46 /// else's hook entirely".
47 const SHIM_SUBCOMMAND: &str = "hooks run-commit-msg";
48
49 /// The trailer key written into messages. Lowercase [`trailer::PATCH_TOKEN`] is
50 /// what reads it back; the match is case-insensitive.
51 const TRAILER_KEY: &str = "Patch";
52
53 /// Git's default comment character, used when `core.commentChar` is unset or
54 /// is something we cannot resolve to a single character (`auto`).
55 const DEFAULT_COMMENT_CHAR: char = '#';
56
57 // ---------------------------------------------------------------------------
58 // The script
59 // ---------------------------------------------------------------------------
60
61 /// The shim installed as `commit-msg`.
62 ///
63 /// `exe` is baked in as an absolute path because a hook runs in whatever
64 /// environment invoked git — a GUI client, an editor, a cron job — and those
65 /// routinely have a `PATH` that never included `~/.local/bin`. The bare name is
66 /// kept as a fallback for the case the binary moved, and if neither resolves
67 /// the `|| true` makes that a silent no-op rather than a failed commit.
68 pub fn hook_script(exe: &Path) -> String {
69 format!(
70 "#!/bin/sh\n\
71 # {marker}: stamps `{key}: <id>` trailers onto commit messages.\n\
72 #\n\
73 # Written by `git-collab init`, which rewrites this file in place —\n\
74 # edits here are not preserved. Safe to delete: it is a convenience,\n\
75 # and merges are recorded by scanning at sync time regardless.\n\
76 #\n\
77 # This must never fail a commit, so the exit status below is discarded\n\
78 # deliberately — do not \"fix\" it.\n\
79 GIT_COLLAB='{exe}'\n\
80 [ -x \"$GIT_COLLAB\" ] || GIT_COLLAB=git-collab\n\
81 \"$GIT_COLLAB\" {shim} \"$1\" >/dev/null 2>&1 || true\n\
82 exit 0\n",
83 marker = MARKER,
84 key = TRAILER_KEY,
85 exe = exe.display().to_string().replace('\'', "'\\''"),
86 shim = SHIM_SUBCOMMAND,
87 )
88 }
89
90 /// The line someone with their own `commit-msg` hook adds to it by hand.
91 pub fn shim_line() -> String {
92 format!("git-collab {} \"$1\" >/dev/null 2>&1 || true", SHIM_SUBCOMMAND)
93 }
94
95 // ---------------------------------------------------------------------------
96 // Installation
97 // ---------------------------------------------------------------------------
98
99 /// Where git looks for hooks in this repo.
100 ///
101 /// `core.hooksPath` has to be honoured: when it is set, git runs hooks from
102 /// there and nowhere else, so installing into `.git/hooks` anyway would leave a
103 /// hook that never runs and produces no symptom but silence — the one failure
104 /// mode this feature cannot afford, since silence is also what it looks like
105 /// when everything is fine.
106 ///
107 /// The common dir, not the gitdir: in a linked worktree those differ, and
108 /// hooks live with the main repository, so installing into the worktree's own
109 /// gitdir would put the hook somewhere git never looks.
110 pub fn hooks_dir(repo: &Repository) -> PathBuf {
111 if let Ok(config) = repo.config() {
112 if let Ok(configured) = config.get_path("core.hooksPath") {
113 if configured.is_absolute() {
114 return configured;
115 }
116 if let Some(workdir) = repo.workdir() {
117 return workdir.join(configured);
118 }
119 }
120 }
121 common_dir(repo).join("hooks")
122 }
123
124 /// The repository's common directory.
125 ///
126 /// git2 0.19 exposes no `commondir`, so this reads the `commondir` file git
127 /// writes into a linked worktree's gitdir — the same file git itself reads.
128 /// Its content is usually relative to the gitdir. A plain repository has no
129 /// such file, and there the gitdir *is* the common dir.
130 fn common_dir(repo: &Repository) -> PathBuf {
131 let git_dir = repo.path();
132 let Ok(contents) = std::fs::read_to_string(git_dir.join("commondir")) else {
133 return git_dir.to_path_buf();
134 };
135 let relative = Path::new(contents.trim());
136 if relative.as_os_str().is_empty() {
137 return git_dir.to_path_buf();
138 }
139 if relative.is_absolute() {
140 relative.to_path_buf()
141 } else {
142 git_dir.join(relative)
143 }
144 }
145
146 /// The path of the `commit-msg` hook for this repo.
147 pub fn hook_path(repo: &Repository) -> PathBuf {
148 hooks_dir(repo).join("commit-msg")
149 }
150
151 /// What [`install`] did, or declined to do.
152 #[derive(Debug, Clone, PartialEq, Eq)]
153 pub enum InstallOutcome {
154 Installed(PathBuf),
155 /// Ours already, byte-for-byte. Rewriting it would be equivalent; not
156 /// rewriting it is what makes repeated `init` provably inert.
157 AlreadyInstalled(PathBuf),
158 /// Someone else's hook that already calls us. Reported rather than
159 /// "corrected", so that following the advice in [`InstallOutcome::Foreign`]
160 /// does not produce a nag on every subsequent `init`.
161 ForeignHookCallsUs(PathBuf),
162 /// Someone else's hook. Nothing was written.
163 Foreign(PathBuf),
164 }
165
166 /// Install the `commit-msg` hook, unless a hook we did not write is there.
167 ///
168 /// **Declines rather than chains,** and the choice is not close. Chaining means
169 /// moving the user's file to a name of our invention and calling it from ours:
170 /// a destructive filesystem change made by a command whose whole job is to edit
171 /// config. It breaks hooks that inspect `$0`, it is invisible in `git config`,
172 /// and it loses outright against `husky`/`pre-commit`/`lefthook`, all of which
173 /// own `commit-msg` and regenerate it — silently reverting our chain, or
174 /// double-invoking the user's hook after we renamed something they still
175 /// reference. Against that, the cost of declining is one line the user pastes
176 /// if they want it, and *nothing else stops working*: sync-time scanning still
177 /// records every merge, on every machine, retroactively. The hook is a
178 /// convenience. Convenience is not worth a chance of eating someone's hook.
179 pub fn install(repo: &Repository) -> Result<InstallOutcome, Error> {
180 let path = hook_path(repo);
181 let script = hook_script(&current_exe());
182
183 if path.exists() {
184 let existing = std::fs::read_to_string(&path).unwrap_or_default();
185 if existing.contains(MARKER) {
186 if existing == script {
187 return Ok(InstallOutcome::AlreadyInstalled(path));
188 }
189 // Ours, but from another install (a different binary path, or an
190 // older version of this script). Refreshing it is safe precisely
191 // because the marker says nobody else's work is in there.
192 write_executable(&path, &script)?;
193 return Ok(InstallOutcome::Installed(path));
194 }
195 if existing.contains(SHIM_SUBCOMMAND) {
196 return Ok(InstallOutcome::ForeignHookCallsUs(path));
197 }
198 return Ok(InstallOutcome::Foreign(path));
199 }
200
201 if let Some(parent) = path.parent() {
202 std::fs::create_dir_all(parent)?;
203 }
204 write_executable(&path, &script)?;
205 Ok(InstallOutcome::Installed(path))
206 }
207
208 fn write_executable(path: &Path, contents: &str) -> Result<(), Error> {
209 std::fs::write(path, contents)?;
210 #[cfg(unix)]
211 {
212 use std::os::unix::fs::PermissionsExt;
213 let mut perms = std::fs::metadata(path)?.permissions();
214 perms.set_mode(0o755);
215 std::fs::set_permissions(path, perms)?;
216 }
217 Ok(())
218 }
219
220 /// The absolute path of the running binary, falling back to the bare name if
221 /// the OS will not say (which the shim then resolves through `PATH`).
222 fn current_exe() -> PathBuf {
223 std::env::current_exe().unwrap_or_else(|_| PathBuf::from("git-collab"))
224 }
225
226 /// Print what [`install`] did, in `init`'s reporting style.
227 pub fn report_install(outcome: &InstallOutcome) {
228 match outcome {
229 InstallOutcome::Installed(path) => {
230 println!("Installed commit-msg hook ({})", path.display());
231 }
232 InstallOutcome::AlreadyInstalled(path) => {
233 println!("commit-msg hook already installed ({})", path.display());
234 }
235 InstallOutcome::ForeignHookCallsUs(path) => {
236 println!(
237 "commit-msg hook already invokes git-collab ({})",
238 path.display()
239 );
240 }
241 InstallOutcome::Foreign(path) => {
242 println!(
243 "commit-msg hook not installed: {} already exists and was not written by git-collab.",
244 path.display()
245 );
246 println!(" Nothing was changed. To stamp Patch: trailers from your own hook, add:");
247 println!(" {}", shim_line());
248 }
249 }
250 }
251
252 // ---------------------------------------------------------------------------
253 // Which patch a commit on HEAD belongs to
254 // ---------------------------------------------------------------------------
255
256 /// What the hook would do for HEAD as it stands.
257 #[derive(Debug, Clone)]
258 pub enum Target {
259 /// Exactly one open patch records this branch: the only case that stamps.
260 One { branch: String, patch: Box<PatchState> },
261 /// No open patch records this branch.
262 NoPatch { branch: String },
263 /// Several do. Ambiguous, so nothing is stamped — see [`target`].
264 Ambiguous { branch: String, count: usize },
265 /// HEAD is not on a branch (detached, mid-rebase, mid-bisect).
266 NoBranch,
267 /// The patches could not be read at all.
268 Unreadable(String),
269 }
270
271 /// Which patch, if any, a commit made right now belongs to.
272 ///
273 /// Matching is by branch name, against `PatchState::branch`. That field is
274 /// documented as provenance only — nothing else resolves through it any more,
275 /// because patches are addressed by their own revision refs — and this is the
276 /// one place a branch name is the only thing available: at `commit-msg` time
277 /// the commit does not exist yet, so there is no commit to look up.
278 ///
279 /// The consequence is that the hook inherits the known weakness of
280 /// branch-addressing (see the revision-refs spec, issue `659f0350`): an
281 /// ephemeral or renamed worktree branch does not match, and two worktrees on
282 /// the same generated name look like one branch. Both land in a *silent* case
283 /// below, which is why silence is the right answer for anything but an exact
284 /// single match: guessing would record a merge of a patch that never landed,
285 /// and a `commit-msg` hook has nobody to ask.
286 pub fn target(repo: &Repository) -> Target {
287 let Some(branch) = head_branch(repo) else {
288 return Target::NoBranch;
289 };
290 let patches = match state::list_patches(repo) {
291 Ok(p) => p,
292 Err(e) => return Target::Unreadable(e.to_string()),
293 };
294 let mut matched: Vec<PatchState> = patches
295 .into_iter()
296 .filter(|p| p.status == PatchStatus::Open && p.branch == branch)
297 .collect();
298 match matched.len() {
299 0 => Target::NoPatch { branch },
300 1 => Target::One {
301 branch,
302 patch: Box::new(matched.remove(0)),
303 },
304 count => Target::Ambiguous { branch, count },
305 }
306 }
307
308 /// The branch HEAD points at, or `None` when HEAD is detached.
309 ///
310 /// Reads the symbolic ref rather than `Repository::head`, which errors on an
311 /// unborn branch — the very first commit in a repo, where erroring would be
312 /// wrong and where a hook must be as quiet as anywhere else.
313 fn head_branch(repo: &Repository) -> Option<String> {
314 let head = repo.find_reference("HEAD").ok()?;
315 let target = head.symbolic_target()?;
316 target
317 .strip_prefix("refs/heads/")
318 .map(|name| name.to_string())
319 }
320
321 // ---------------------------------------------------------------------------
322 // Stamping
323 // ---------------------------------------------------------------------------
324
325 /// The hook body: stamp the message file in place, or leave it exactly alone.
326 ///
327 /// Returns nothing, on purpose. There is no error here worth a caller's
328 /// attention: every failure means "the message is unchanged", which is a state
329 /// the caller already handles because it is also the common case.
330 pub fn run_commit_msg(repo: &Repository, file: &Path) {
331 let Ok(contents) = std::fs::read_to_string(file) else {
332 return;
333 };
334 let Target::One { patch, .. } = target(repo) else {
335 return;
336 };
337 let Some(stamped) = stamp(&contents, comment_char(repo), &patch.id) else {
338 return;
339 };
340 let _ = replace_atomically(file, &stamped);
341 }
342
343 /// Replace `file` with `contents` by rename, never by truncate-and-write.
344 ///
345 /// git reads this file the moment we exit. A crash or a full disk halfway
346 /// through a truncating write would hand it a message that is neither the
347 /// author's nor ours; a rename either happened or did not.
348 fn replace_atomically(file: &Path, contents: &str) -> std::io::Result<()> {
349 let dir = file.parent().unwrap_or_else(|| Path::new("."));
350 let tmp = dir.join(format!(
351 ".git-collab-commit-msg.{}",
352 std::process::id()
353 ));
354 std::fs::write(&tmp, contents)?;
355 if let Err(e) = std::fs::rename(&tmp, file) {
356 let _ = std::fs::remove_file(&tmp);
357 return Err(e);
358 }
359 Ok(())
360 }
361
362 /// `core.commentChar`, or `#`.
363 ///
364 /// `auto` (and any multi-character `core.commentString`) resolves to `#`, which
365 /// is what git itself starts from. Getting this wrong costs a missed stamp at
366 /// worst — the trailer lands in a paragraph that does not parse — never a
367 /// mangled message.
368 fn comment_char(repo: &Repository) -> char {
369 repo.config()
370 .ok()
371 .and_then(|c| c.get_string("core.commentChar").ok())
372 .and_then(|s| {
373 let mut chars = s.chars();
374 let first = chars.next()?;
375 chars.next().is_none().then_some(first)
376 })
377 .unwrap_or(DEFAULT_COMMENT_CHAR)
378 }
379
380 /// Insert `Patch: <id>` into a raw commit-message file, or return `None` to
381 /// leave it untouched.
382 ///
383 /// The input is the file git hands a `commit-msg` hook, which is **not** the
384 /// message git will store: it still carries git's `#` commentary, and below a
385 /// scissors line it may carry a whole diff. Both are stripped afterwards, and
386 /// that stripping is what makes placement subtle — appending at the end of the
387 /// file puts the trailer *after* the commentary, and once the commentary
388 /// vanishes the trailer joins whatever paragraph preceded it. If that paragraph
389 /// is prose, the trailer is unreadable to [`trailer::parse_trailers`], and the
390 /// merge it was supposed to record silently never happens.
391 ///
392 /// So the insertion point is the end of the message *as git will store it*: the
393 /// last line that is neither commentary nor below the scissors. Two shapes:
394 ///
395 /// - the final paragraph is already a trailer block (and is not the subject) —
396 /// join it, because starting a new paragraph would push the trailers that
397 /// were there out of the final paragraph and unlink them;
398 /// - anything else — a blank line, then the trailer.
399 ///
400 /// Returns `None` — writing nothing at all — when:
401 ///
402 /// - the message has no content yet. Stamping an empty message would make it
403 /// non-empty and commit work the author was in the middle of abandoning;
404 /// git aborts on an empty message and that abort has to keep working.
405 /// - a `Patch:` line is already there, anywhere. This is what makes `git commit
406 /// --amend` idempotent, and it also leaves a hand-written short-id trailer
407 /// alone rather than stapling a 40-char one underneath it.
408 /// - the file uses CRLF. Rebuilding it would rewrite every line ending in the
409 /// file, which is a much larger edit than the one asked for.
410 pub fn stamp(contents: &str, comment_char: char, id: &str) -> Option<String> {
411 if contents.contains("\r\n") {
412 return None;
413 }
414
415 let lines: Vec<&str> = contents.lines().collect();
416 let cut = lines
417 .iter()
418 .position(|line| is_scissors(line, comment_char))
419 .unwrap_or(lines.len());
420 let body = &lines[..cut];
421
422 // The message as git will store it, so that every decision below is made
423 // against what the reader will eventually see.
424 let stored: String = body
425 .iter()
426 .filter(|line| !is_comment(line, comment_char))
427 .copied()
428 .collect::<Vec<&str>>()
429 .join("\n");
430
431 if stored.trim().is_empty() {
432 return None;
433 }
434 if trailer::contains_trailer_line(&stored, trailer::PATCH_TOKEN) {
435 return None;
436 }
437
438 let last_content = body
439 .iter()
440 .rposition(|line| !is_comment(line, comment_char) && !line.trim().is_empty())?;
441 let paragraph = trailer::final_paragraph(&stored)?;
442 let join = paragraph.is_trailer_block && !paragraph.starts_the_message;
443
444 let mut out: Vec<String> = lines.iter().map(|line| line.to_string()).collect();
445 let at = last_content + 1;
446 let new_line = format!("{}: {}", TRAILER_KEY, id);
447 if join {
448 out.insert(at, new_line);
449 } else {
450 out.splice(at..at, [String::new(), new_line]);
451 }
452
453 let mut result = out.join("\n");
454 // `lines()` drops the final newline; commit message files have one, and a
455 // message file that did not should not grow one.
456 if contents.ends_with('\n') {
457 result.push('\n');
458 }
459 Some(result)
460 }
461
462 /// A git commentary line: comment character in column one, exactly as git
463 /// tests it.
464 fn is_comment(line: &str, comment_char: char) -> bool {
465 line.starts_with(comment_char)
466 }
467
468 /// The `# ------------------------ >8 ------------------------` line that
469 /// `commit -v` writes, below which git truncates everything.
470 fn is_scissors(line: &str, comment_char: char) -> bool {
471 let Some(rest) = line.strip_prefix(comment_char) else {
472 return false;
473 };
474 let rest = rest.trim();
475 rest.contains(">8")
476 && !rest.is_empty()
477 && rest
478 .chars()
479 .all(|c| c == '-' || c == '>' || c == '8' || c == ' ')
480 }
481
482 // ---------------------------------------------------------------------------
483 // Status
484 // ---------------------------------------------------------------------------
485
486 /// Explain what the hook is and what it would do right now.
487 ///
488 /// The hook is silent by design, so when it is not working the only symptom is
489 /// that nothing happens — indistinguishable from nothing needing to happen.
490 /// This is the command that tells those apart, and it is the reason a
491 /// standalone `hooks` command group exists at all.
492 pub fn status(repo: &Repository) -> Result<(), Error> {
493 let path = hook_path(repo);
494 let contents = std::fs::read_to_string(&path).unwrap_or_default();
495 if !path.exists() {
496 println!(
497 "commit-msg hook: not installed ({} does not exist)",
498 path.display()
499 );
500 println!("Install it with: git-collab hooks install");
501 } else if contents.contains(MARKER) {
502 println!("commit-msg hook: installed ({})", path.display());
503 } else if contents.contains(SHIM_SUBCOMMAND) {
504 println!(
505 "commit-msg hook: another hook that invokes git-collab ({})",
506 path.display()
507 );
508 } else {
509 println!(
510 "commit-msg hook: not installed — {} is another hook",
511 path.display()
512 );
513 println!(" To stamp Patch: trailers from it, add:");
514 println!(" {}", shim_line());
515 }
516
517 match target(repo) {
518 Target::One { branch, patch } => println!(
519 "HEAD is on '{}': commits would be stamped `Patch: {}` (patch {:.8} {})",
520 branch, patch.id, patch.id, patch.title
521 ),
522 Target::NoPatch { branch } => println!(
523 "HEAD is on '{}': no open patch records this branch, so nothing would be stamped",
524 branch
525 ),
526 Target::Ambiguous { branch, count } => println!(
527 "HEAD is on '{}': {} open patches record this branch, so nothing would be stamped",
528 branch, count
529 ),
530 Target::NoBranch => {
531 println!("HEAD is not on a branch, so nothing would be stamped")
532 }
533 Target::Unreadable(e) => {
534 println!("Patches could not be read ({}), so nothing would be stamped", e)
535 }
536 }
537 Ok(())
538 }
539
540 #[cfg(test)]
541 mod tests {
542 use super::*;
543
544 /// The round trip in miniature: whatever `stamp` writes, `parse_trailers`
545 /// reads back. The integration tests drive this through real `git commit`,
546 /// which is what actually proves it; these pin the shapes that are awkward
547 /// to reach from there.
548 fn assert_round_trip(input: &str) {
549 let out = stamp(input, '#', "abc123").expect("expected a stamp");
550 // Approximate git's cleanup: drop commentary and anything below the
551 // scissors line. The integration tests use git's real implementation.
552 let cut = out
553 .lines()
554 .position(|l| is_scissors(l, '#'))
555 .unwrap_or(usize::MAX);
556 let stored: String = out
557 .lines()
558 .take(cut)
559 .filter(|l| !is_comment(l, '#'))
560 .collect::<Vec<_>>()
561 .join("\n");
562 assert_eq!(
563 trailer::parse_trailers(&stored, trailer::PATCH_TOKEN),
564 vec!["abc123".to_string()],
565 "did not read back from:\n{}",
566 stored
567 );
568 }
569
570 #[test]
571 fn stamps_a_subject_only_message_in_its_own_paragraph() {
572 let out = stamp("subject\n", '#', "abc123").unwrap();
573 assert_eq!(out, "subject\n\nPatch: abc123\n");
574 assert_round_trip("subject\n");
575 }
576
577 #[test]
578 fn a_subject_that_looks_like_a_trailer_still_gets_a_blank_line() {
579 // Joining here would fold the trailer into the subject: git takes the
580 // whole first paragraph as the subject line.
581 let out = stamp("Fix: thing\n", '#', "abc123").unwrap();
582 assert_eq!(out, "Fix: thing\n\nPatch: abc123\n");
583 }
584
585 #[test]
586 fn joins_an_existing_trailer_block() {
587 let out = stamp("subject\n\nIssue: xyz\n", '#', "abc123").unwrap();
588 assert_eq!(out, "subject\n\nIssue: xyz\nPatch: abc123\n");
589 assert_round_trip("subject\n\nIssue: xyz\n");
590 }
591
592 #[test]
593 fn writes_above_the_comment_block_not_below_it() {
594 let input = "subject\n\nbody prose\n\n# commentary\n# more\n";
595 let out = stamp(input, '#', "abc123").unwrap();
596 assert_eq!(
597 out,
598 "subject\n\nbody prose\n\nPatch: abc123\n\n# commentary\n# more\n"
599 );
600 assert_round_trip(input);
601 }
602
603 #[test]
604 fn writes_above_the_scissors_line() {
605 let input = "subject\n\n# ------------------------ >8 ------------------------\ndiff --git a b\n+Patch: notmine\n";
606 let out = stamp(input, '#', "abc123").unwrap();
607 assert!(
608 out.find("Patch: abc123").unwrap() < out.find(">8").unwrap(),
609 "trailer landed below the scissors line:\n{}",
610 out
611 );
612 }
613
614 #[test]
615 fn a_patch_trailer_below_the_scissors_does_not_count_as_present() {
616 // It is part of a diff, not of the message, and git throws it away.
617 let input = "subject\n# ------------------------ >8 ------------------------\n+Patch: notmine\n";
618 assert!(stamp(input, '#', "abc123").is_some());
619 }
620
621 #[test]
622 fn leaves_a_message_that_already_has_a_patch_trailer() {
623 assert_eq!(stamp("subject\n\nPatch: abc123\n", '#', "abc123"), None);
624 assert_eq!(stamp("subject\n\nPatch: abc\n", '#', "abc123"), None);
625 // Unparseable, but written by a human all the same.
626 assert_eq!(stamp("subject\n\nPatch: abc oops\n", '#', "abc123"), None);
627 assert_eq!(stamp("subject\n\nPatch: abc\nprose\n", '#', "abc123"), None);
628 }
629
630 #[test]
631 fn leaves_an_empty_message() {
632 assert_eq!(stamp("", '#', "abc123"), None);
633 assert_eq!(stamp("\n\n", '#', "abc123"), None);
634 assert_eq!(stamp("# just commentary\n", '#', "abc123"), None);
635 }
636
637 #[test]
638 fn leaves_a_crlf_message() {
639 assert_eq!(stamp("subject\r\n", '#', "abc123"), None);
640 }
641
642 #[test]
643 fn honours_a_non_default_comment_char() {
644 let out = stamp("subject\n; commentary\n", ';', "abc123").unwrap();
645 assert_eq!(out, "subject\n\nPatch: abc123\n; commentary\n");
646 }
647
648 #[test]
649 fn the_script_carries_the_marker_and_exactly_one_shim_line() {
650 let script = hook_script(Path::new("/usr/bin/git-collab"));
651 assert!(script.contains(MARKER));
652 assert_eq!(script.matches(SHIM_SUBCOMMAND).count(), 1);
653 assert!(script.contains("|| true"));
654 }
655 }
src/lib.rs
Old New
@@ -5,6 +5,7 @@ pub mod dag;
5 pub mod editor; 5 pub mod editor;
6 pub mod error; 6 pub mod error;
7 pub mod event; 7 pub mod event;
8 pub mod hooks;
8 pub mod identity; 9 pub mod identity;
9 pub mod issue; 10 pub mod issue;
10 pub mod log; 11 pub mod log;
@@ -21,7 +22,7 @@ pub mod trust;
21 pub mod tui; 22 pub mod tui;
22 23
23 use base64::Engine; 24 use base64::Engine;
24 use cli::{Commands, IdentityCmd, IssueCmd, KeyCmd, PatchCmd, ReleaseCmd}; 25 use cli::{Commands, HookCmd, IdentityCmd, IssueCmd, KeyCmd, PatchCmd, ReleaseCmd};
25 use event::ReviewVerdict; 26 use event::ReviewVerdict;
26 use git2::Repository; 27 use git2::Repository;
27 28
@@ -114,6 +115,20 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
114 let is_write = cli.command.is_write(); 115 let is_write = cli.command.is_write();
115 match cli.command { 116 match cli.command {
116 Commands::Init => sync::init(repo), 117 Commands::Init => sync::init(repo),
118 Commands::Hooks(cmd) => match cmd {
119 HookCmd::Install => {
120 let outcome = hooks::install(repo)?;
121 hooks::report_install(&outcome);
122 Ok(())
123 }
124 HookCmd::Status => hooks::status(repo),
125 // Never returns an error: this runs inside `git commit`, and the
126 // one thing it must not do is give git a reason to abort.
127 HookCmd::RunCommitMsg { file } => {
128 hooks::run_commit_msg(repo, &file);
129 Ok(())
130 }
131 },
117 Commands::Issue(cmd) => match cmd { 132 Commands::Issue(cmd) => match cmd {
118 IssueCmd::Open { 133 IssueCmd::Open {
119 title, 134 title,
src/merge_scan.rs
Old New
@@ -8,7 +8,7 @@
8 //! squash merge at all. So a merge is recorded, three ways that degrade into 8 //! squash merge at all. So a merge is recorded, three ways that degrade into
9 //! each other: 9 //! each other:
10 //! 10 //!
11 //! 1. a `commit-msg` hook stamps `Patch: <id>` onto commits (not implemented); 11 //! 1. a `commit-msg` hook stamps `Patch: <id>` onto commits — [`crate::hooks`];
12 //! 2. `sync` scans the base branch for those trailers — [`scan_and_record_merges`]; 12 //! 2. `sync` scans the base branch for those trailers — [`scan_and_record_merges`];
13 //! 3. `git-collab patch merge <id>` records it by hand — [`crate::patch::merge`]. 13 //! 3. `git-collab patch merge <id>` records it by hand — [`crate::patch::merge`].
14 //! 14 //!
src/sync.rs
Old New
@@ -444,7 +444,18 @@ fn has_collab_refspec(repo: &Repository, remote_name: &str) -> Result<bool, Erro
444 /// check a second `init` silently gives the remote a duplicate refspec — and 444 /// check a second `init` silently gives the remote a duplicate refspec — and
445 /// says "Configured remote" as though it had done something new, which is how 445 /// says "Configured remote" as though it had done something new, which is how
446 /// a repo ends up fetching every collab ref twice with nothing to indicate it. 446 /// a repo ends up fetching every collab ref twice with nothing to indicate it.
447 /// Configure collab refspecs on every remote, and install the `commit-msg`
448 /// hook.
449 ///
450 /// Idempotent in both halves: a second run reconfigures nothing and rewrites
451 /// nothing, and says so.
447 pub fn init(repo: &Repository) -> Result<(), Error> { 452 pub fn init(repo: &Repository) -> Result<(), Error> {
453 init_refspecs(repo)?;
454 init_hook(repo);
455 Ok(())
456 }
457
458 fn init_refspecs(repo: &Repository) -> Result<(), Error> {
448 let remotes = repo.remotes()?; 459 let remotes = repo.remotes()?;
449 if remotes.is_empty() { 460 if remotes.is_empty() {
450 println!("No remotes configured."); 461 println!("No remotes configured.");
@@ -463,6 +474,16 @@ pub fn init(repo: &Repository) -> Result<(), Error> {
463 Ok(()) 474 Ok(())
464 } 475 }
465 476
477 /// Install the hook, and never let that failure fail `init`. The hook is a
478 /// convenience; the refspecs above are the part that has to have happened when
479 /// `init` returns.
480 fn init_hook(repo: &Repository) {
481 match crate::hooks::install(repo) {
482 Ok(outcome) => crate::hooks::report_install(&outcome),
483 Err(e) => eprintln!("warning: could not install the commit-msg hook: {}", e),
484 }
485 }
486
466 // --------------------------------------------------------------------------- 487 // ---------------------------------------------------------------------------
467 // Multi-remote orchestration 488 // Multi-remote orchestration
468 // --------------------------------------------------------------------------- 489 // ---------------------------------------------------------------------------
src/trailer.rs
Old New
@@ -36,27 +36,14 @@ pub fn parse_trailers(message: &str, token: &str) -> Vec<String> {
36 // 36 //
37 // Walking from the end: skip trailing blank/whitespace-only lines, 37 // Walking from the end: skip trailing blank/whitespace-only lines,
38 // then collect lines until we hit a blank line. 38 // then collect lines until we hit a blank line.
39 let mut end = lines.len(); 39 let Some((start, end)) = final_paragraph_bounds(&lines) else {
40 while end > 0 && lines[end - 1].trim().is_empty() {
41 end -= 1;
42 }
43 if end == 0 {
44 return Vec::new(); 40 return Vec::new();
45 } 41 };
46 let mut start = end;
47 while start > 0 && !lines[start - 1].trim().is_empty() {
48 start -= 1;
49 }
50 let paragraph = &lines[start..end]; 42 let paragraph = &lines[start..end];
51 43
52 // 3. Validate every non-empty line in the paragraph is trailer-shaped. 44 // 3. Validate every non-empty line in the paragraph is trailer-shaped.
53 for line in paragraph { 45 if !is_trailer_block(paragraph) {
54 if line.trim().is_empty() { 46 return Vec::new();
55 continue;
56 }
57 if !is_trailer_shaped(line) {
58 return Vec::new();
59 }
60 } 47 }
61 48
62 // 4. Extract the values whose key is `token`. 49 // 4. Extract the values whose key is `token`.
@@ -69,32 +56,109 @@ pub fn parse_trailers(message: &str, token: &str) -> Vec<String> {
69 out 56 out
70 } 57 }
71 58
72 /// Returns true if a line looks like a git trailer: `<token>: <value>`, where 59 /// The span of the final paragraph of `lines` — the tail run of non-blank
73 /// token starts with a letter and consists of `[A-Za-z0-9-]`, and value is at 60 /// lines, ignoring trailing blanks. `None` when there is no content at all.
74 /// least one non-whitespace character. 61 fn final_paragraph_bounds(lines: &[&str]) -> Option<(usize, usize)> {
75 fn is_trailer_shaped(line: &str) -> bool { 62 let mut end = lines.len();
63 while end > 0 && lines[end - 1].trim().is_empty() {
64 end -= 1;
65 }
66 if end == 0 {
67 return None;
68 }
69 let mut start = end;
70 while start > 0 && !lines[start - 1].trim().is_empty() {
71 start -= 1;
72 }
73 Some((start, end))
74 }
75
76 /// Whether every non-empty line of `paragraph` is trailer-shaped — git's rule
77 /// for a paragraph being a trailer block, and the one [`parse_trailers`]
78 /// enforces before reading anything out of it.
79 fn is_trailer_block(paragraph: &[&str]) -> bool {
80 paragraph
81 .iter()
82 .all(|line| line.trim().is_empty() || is_trailer_shaped(line))
83 }
84
85 /// What the writer of a trailer needs to know about the message it is about to
86 /// append to. See [`final_paragraph`].
87 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
88 pub struct FinalParagraph {
89 /// The final paragraph is also the first — a message with no blank line in
90 /// it, i.e. a bare subject. Appending to it would fold the trailer into the
91 /// subject line, since git takes the whole first paragraph as the subject.
92 pub starts_the_message: bool,
93 /// Every non-empty line in it is trailer-shaped, so a trailer appended
94 /// directly to it stays readable — *and* so do the trailers already there,
95 /// which starting a new paragraph instead would strand outside the final
96 /// paragraph and silently unlink.
97 pub is_trailer_block: bool,
98 }
99
100 /// Describe the final paragraph of `message`, or `None` if it has no content.
101 ///
102 /// This exists so that whatever *writes* a trailer decides where to put it
103 /// using the same notion of "trailer block" that [`parse_trailers`] uses to
104 /// read it back. Those are two implementations of the same rule when they are
105 /// written separately, and two implementations of one rule is how a writer
106 /// starts emitting trailers the reader cannot see.
107 pub fn final_paragraph(message: &str) -> Option<FinalParagraph> {
108 let lines: Vec<&str> = message.lines().collect();
109 let (start, end) = final_paragraph_bounds(&lines)?;
110 Some(FinalParagraph {
111 starts_the_message: start == 0,
112 is_trailer_block: is_trailer_block(&lines[start..end]),
113 })
114 }
115
116 /// Whether *any* line of `message`, in any paragraph, is a `<token>:` trailer
117 /// line.
118 ///
119 /// Deliberately laxer than [`parse_trailers`], and only ever used to decide not
120 /// to write: a `Patch:` line that the parser refuses — because it sits in a
121 /// paragraph with prose, or carries a value with interior whitespace — is still
122 /// a line the author wrote, and stapling a second one underneath it helps
123 /// nobody. "Already mentions it" is the right test for leaving a message alone;
124 /// "already parses" is not.
125 pub fn contains_trailer_line(message: &str, token: &str) -> bool {
126 message
127 .lines()
128 .any(|line| trailer_key(line).is_some_and(|key| key.eq_ignore_ascii_case(token)))
129 }
130
131 /// The key of a trailer-shaped line: `<token>: <value>`, where token starts
132 /// with a letter and consists of `[A-Za-z0-9-]`, and value is at least one
133 /// non-whitespace character. `None` if the line is not trailer-shaped.
134 fn trailer_key(line: &str) -> Option<&str> {
76 let trimmed = line.trim_start(); 135 let trimmed = line.trim_start();
77 let Some(colon_pos) = trimmed.find(':') else { 136 let colon_pos = trimmed.find(':')?;
78 return false;
79 };
80 // Use trim_end() so that `ISSUE : abc` is recognized as the token `ISSUE` 137 // Use trim_end() so that `ISSUE : abc` is recognized as the token `ISSUE`
81 // — matching what `match_trailer_line` does. Without this, the space before 138 // — matching what `match_trailer_line` does. Without this, the space before
82 // the colon would disqualify the line and make the whole paragraph fail 139 // the colon would disqualify the line and make the whole paragraph fail
83 // the trailer-shape check. 140 // the trailer-shape check.
84 let token = trimmed[..colon_pos].trim_end(); 141 let token = trimmed[..colon_pos].trim_end();
85 if token.is_empty() { 142 if token.is_empty() {
86 return false; 143 return None;
87 } 144 }
88 let mut chars = token.chars(); 145 let mut chars = token.chars();
89 let first = chars.next().unwrap(); 146 let first = chars.next().unwrap();
90 if !first.is_ascii_alphabetic() { 147 if !first.is_ascii_alphabetic() {
91 return false; 148 return None;
92 } 149 }
93 if !chars.all(|c| c.is_ascii_alphanumeric() || c == '-') { 150 if !chars.all(|c| c.is_ascii_alphanumeric() || c == '-') {
94 return false; 151 return None;
152 }
153 if trimmed[colon_pos + 1..].trim().is_empty() {
154 return None;
95 } 155 }
96 let value = trimmed[colon_pos + 1..].trim(); 156 Some(token)
97 !value.is_empty() 157 }
158
159 /// Returns true if a line looks like a git trailer.
160 fn is_trailer_shaped(line: &str) -> bool {
161 trailer_key(line).is_some()
98 } 162 }
99 163
100 /// If `line` is a `<token>: <value>` trailer with exactly one non-whitespace 164 /// If `line` is a `<token>: <value>` trailer with exactly one non-whitespace
tests/commit_msg_hook_test.rs
Old New
@@ -0,0 +1,692 @@
1 //! Layer 1 of merge recording: the `commit-msg` hook that stamps `Patch: <id>`.
2 //!
3 //! See: docs/superpowers/specs/2026-08-09-merge-recording-design.md
4 //!
5 //! Two things are being tested here and they are not the same thing:
6 //!
7 //! 1. **The round trip.** Whatever writes the trailer has to produce something
8 //! [`git_collab::trailer::parse_trailers`] reads back. Writer and reader are
9 //! separate implementations of "what is a trailer", and between them sits
10 //! git's own message cleanup — which strips comments, truncates at the
11 //! scissors line and collapses blank runs, and so can move our trailer into
12 //! a paragraph that no longer parses. So every round-trip test below drives
13 //! a *real* `git commit` through the *real* installed hook and reads the
14 //! stored message back out of the object database. A test that called the
15 //! stamping function with a hand-built string would agree with itself and
16 //! prove nothing about what git stores.
17 //!
18 //! 2. **Failing open.** A `commit-msg` hook that can block a commit gets
19 //! deleted by its user the same day. Every failure mode has to leave the
20 //! message byte-identical and let the commit through.
21
22 mod common;
23
24 use std::path::Path;
25 use std::process::{Command, Output};
26
27 use serde_json::Value;
28
29 use common::TestRepo;
30 use git_collab::trailer::{parse_trailers, ISSUE_TOKEN, PATCH_TOKEN};
31
32 // ---------------------------------------------------------------------------
33 // Harness
34 // ---------------------------------------------------------------------------
35
36 /// A repo with the hook installed, on branch `feature`, with exactly one open
37 /// patch recorded against that branch. Returns the repo and the patch's full
38 /// 40-char id.
39 ///
40 /// The first commit on the branch is made *before* the patch exists — that is
41 /// forced by `patch create`, which needs a commit to point at, and it is why
42 /// the hook can only ever stamp the commits that come after it.
43 fn repo_with_hook_and_patch() -> (TestRepo, String) {
44 let repo = TestRepo::new("Alice", "alice@example.com");
45 repo.run_ok(&["init"]);
46 repo.git(&["checkout", "-b", "feature"]);
47 repo.commit_file("f.txt", "one", "first commit");
48 let out = repo.run_ok(&["patch", "create", "-t", "feature work", "-B", "feature"]);
49 let short = out
50 .trim()
51 .strip_prefix("Created patch ")
52 .unwrap_or_else(|| panic!("unexpected patch create output: {}", out))
53 .to_string();
54 let id = full_id(&repo, &short);
55 (repo, id)
56 }
57
58 fn full_id(repo: &TestRepo, short: &str) -> String {
59 let json: Value = serde_json::from_str(&repo.run_ok(&["patch", "show", short, "--json"])).unwrap();
60 json["id"].as_str().unwrap().to_string()
61 }
62
63 /// The path of the installed `commit-msg` hook.
64 fn hook_path(repo: &TestRepo) -> std::path::PathBuf {
65 repo.dir.path().join(".git/hooks/commit-msg")
66 }
67
68 /// Commit `file` with `message` and return the message git actually stored.
69 fn commit_and_read_back(repo: &TestRepo, file: &str, message: &str) -> String {
70 repo.commit_file(file, "content", message);
71 repo.git(&["log", "-1", "--format=%B"])
72 }
73
74 /// Assert the stored message carries exactly one `Patch:` trailer, that our
75 /// own parser reads it back, and that the value it reads names `id`.
76 ///
77 /// This is the round trip: git wrote the final message, our parser reads it.
78 fn assert_round_trips(stored: &str, id: &str) {
79 let found = parse_trailers(stored, PATCH_TOKEN);
80 assert_eq!(
81 found,
82 vec![id.to_string()],
83 "parse_trailers did not read back the trailer the hook wrote.\n\
84 stored message was:\n---\n{}\n---",
85 stored
86 );
87 }
88
89 fn assert_no_patch_trailer(stored: &str) {
90 assert!(
91 parse_trailers(stored, PATCH_TOKEN).is_empty(),
92 "expected no Patch: trailer, message was:\n---\n{}\n---",
93 stored
94 );
95 }
96
97 /// Write an editor script that prepends `message` to whatever git put in
98 /// COMMIT_EDITMSG — i.e. exactly what a human typing into `$EDITOR` produces,
99 /// comment block and scissors line and all. Returns its path.
100 fn editor_writing(repo: &TestRepo, message: &str) -> std::path::PathBuf {
101 let path = repo.dir.path().join(".git").join("test-editor.sh");
102 std::fs::write(
103 &path,
104 format!(
105 "#!/bin/sh\nprintf '%s' '{}' > \"$1.new\"\ncat \"$1\" >> \"$1.new\"\nmv \"$1.new\" \"$1\"\n",
106 message.replace('\'', "'\\''")
107 ),
108 )
109 .unwrap();
110 let mut perms = std::fs::metadata(&path).unwrap().permissions();
111 std::os::unix::fs::PermissionsExt::set_mode(&mut perms, 0o755);
112 std::fs::set_permissions(&path, perms).unwrap();
113 path
114 }
115
116 fn git_with_editor(repo: &TestRepo, editor: &Path, args: &[&str]) -> Output {
117 let mut command = Command::new("git");
118 repo.apply_env(&mut command);
119 command
120 .env("GIT_EDITOR", editor)
121 .args(args)
122 .current_dir(repo.dir.path())
123 .output()
124 .expect("failed to run git")
125 }
126
127 // ---------------------------------------------------------------------------
128 // Installation
129 // ---------------------------------------------------------------------------
130
131 #[test]
132 fn init_installs_the_commit_msg_hook() {
133 let repo = TestRepo::new("Alice", "alice@example.com");
134 let out = repo.run_ok(&["init"]);
135
136 let path = hook_path(&repo);
137 assert!(path.exists(), "init did not install a hook:\n{}", out);
138 let mode = std::os::unix::fs::PermissionsExt::mode(&std::fs::metadata(&path).unwrap().permissions());
139 assert!(mode & 0o111 != 0, "hook is not executable (mode {:o})", mode);
140 assert!(
141 out.contains("commit-msg hook"),
142 "init did not report the hook:\n{}",
143 out
144 );
145 }
146
147 #[test]
148 fn init_installs_the_hook_even_with_no_remotes() {
149 // `init` returns early when there are no remotes. The hook has nothing to
150 // do with remotes, and a fresh local repo is exactly where someone runs
151 // `init` first.
152 let repo = TestRepo::new("Alice", "alice@example.com");
153 assert!(repo.git(&["remote"]).trim().is_empty());
154 repo.run_ok(&["init"]);
155 assert!(hook_path(&repo).exists(), "no hook in a repo with no remotes");
156 }
157
158 #[test]
159 fn init_twice_leaves_exactly_one_hook_and_one_shim_line() {
160 let repo = TestRepo::new("Alice", "alice@example.com");
161 repo.run_ok(&["init"]);
162 let first = std::fs::read_to_string(hook_path(&repo)).unwrap();
163 let out = repo.run_ok(&["init"]);
164 let second = std::fs::read_to_string(hook_path(&repo)).unwrap();
165
166 assert_eq!(first, second, "second init rewrote the hook");
167 assert_eq!(
168 second.matches("run-commit-msg").count(),
169 1,
170 "second init appended another shim line:\n{}",
171 second
172 );
173 assert!(
174 out.contains("already installed"),
175 "second init did not report the hook as already installed:\n{}",
176 out
177 );
178 }
179
180 #[test]
181 fn init_refuses_to_touch_an_existing_foreign_hook() {
182 let repo = TestRepo::new("Alice", "alice@example.com");
183 let path = hook_path(&repo);
184 std::fs::create_dir_all(path.parent().unwrap()).unwrap();
185 let theirs = "#!/bin/sh\n# my own hook\nexit 0\n";
186 std::fs::write(&path, theirs).unwrap();
187
188 let out = repo.run_ok(&["init"]);
189
190 assert_eq!(
191 std::fs::read_to_string(&path).unwrap(),
192 theirs,
193 "init modified a hook it did not write"
194 );
195 assert!(
196 out.contains("not installed") && out.contains("already exists"),
197 "init did not say clearly that it declined to install:\n{}",
198 out
199 );
200 assert!(
201 out.contains("run-commit-msg"),
202 "init did not offer the line to add by hand:\n{}",
203 out
204 );
205 }
206
207 #[test]
208 fn init_recognizes_a_foreign_hook_that_already_calls_git_collab() {
209 // Someone who took the refusal message's advice has a hook of their own
210 // that invokes us. Reporting that as "not installed" and re-offering the
211 // line would be wrong, and appending it would be worse.
212 let repo = TestRepo::new("Alice", "alice@example.com");
213 let path = hook_path(&repo);
214 std::fs::create_dir_all(path.parent().unwrap()).unwrap();
215 let theirs = "#!/bin/sh\ngit-collab hooks run-commit-msg \"$1\" >/dev/null 2>&1 || true\nexit 0\n";
216 std::fs::write(&path, theirs).unwrap();
217
218 let out = repo.run_ok(&["init"]);
219
220 assert_eq!(std::fs::read_to_string(&path).unwrap(), theirs);
221 assert!(
222 out.contains("already invokes git-collab"),
223 "init did not recognize its own shim line in a foreign hook:\n{}",
224 out
225 );
226 }
227
228 #[test]
229 fn hooks_install_honours_core_hooks_path() {
230 // git only runs hooks from core.hooksPath when it is set. Installing into
231 // .git/hooks anyway would leave a hook that never runs, and no symptom.
232 let repo = TestRepo::new("Alice", "alice@example.com");
233 repo.git(&["config", "core.hooksPath", "my-hooks"]);
234 repo.run_ok(&["hooks", "install"]);
235 assert!(
236 repo.dir.path().join("my-hooks/commit-msg").exists(),
237 "hook was not installed into core.hooksPath"
238 );
239 assert!(
240 !hook_path(&repo).exists(),
241 "hook was installed into .git/hooks, which git is not reading"
242 );
243 }
244
245 #[test]
246 fn installing_from_a_linked_worktree_writes_to_the_main_hooks_dir() {
247 // A linked worktree's gitdir is `.git/worktrees/<name>`, and git runs hooks
248 // from the common dir regardless. Installing into the worktree's own gitdir
249 // would leave a hook that never runs — and worktrees are where this project
250 // does most of its work.
251 let repo = TestRepo::new("Alice", "alice@example.com");
252 let elsewhere = tempfile::TempDir::new().unwrap();
253 let tree = elsewhere.path().join("wt");
254 repo.git(&["worktree", "add", tree.to_str().unwrap(), "-b", "side"]);
255
256 let mut command = Command::new(env!("CARGO_BIN_EXE_git-collab"));
257 repo.apply_env(&mut command);
258 let out = command
259 .args(["hooks", "install"])
260 .current_dir(&tree)
261 .output()
262 .unwrap();
263 assert!(
264 out.status.success(),
265 "hooks install failed in a worktree: {}",
266 String::from_utf8_lossy(&out.stderr)
267 );
268
269 assert!(
270 hook_path(&repo).exists(),
271 "hook did not land in the main repo's hooks dir; install said: {}",
272 String::from_utf8_lossy(&out.stdout)
273 );
274 assert!(
275 !repo
276 .dir
277 .path()
278 .join(".git/worktrees/wt/hooks/commit-msg")
279 .exists(),
280 "hook landed in the worktree gitdir, where git never looks"
281 );
282 }
283
284 #[test]
285 fn hooks_status_reports_what_the_hook_would_stamp() {
286 // The hook is silent by design, so "nothing happened" is its only symptom
287 // when something is wrong. This is the command that explains it.
288 let (repo, id) = repo_with_hook_and_patch();
289 let out = repo.run_ok(&["hooks", "status"]);
290 assert!(out.contains("installed"), "status did not report installation:\n{}", out);
291 assert!(
292 out.contains(&id[..8]),
293 "status did not name the patch it would stamp:\n{}",
294 out
295 );
296
297 repo.git(&["checkout", "main"]);
298 let out = repo.run_ok(&["hooks", "status"]);
299 assert!(
300 out.contains("no open patch"),
301 "status did not explain why nothing would be stamped:\n{}",
302 out
303 );
304 }
305
306 // ---------------------------------------------------------------------------
307 // The round trip: writer vs reader, over real commit-message shapes
308 // ---------------------------------------------------------------------------
309
310 #[test]
311 fn round_trip_single_line_message() {
312 let (repo, id) = repo_with_hook_and_patch();
313 let stored = commit_and_read_back(&repo, "a.txt", "just a subject");
314 assert_round_trips(&stored, &id);
315 assert!(
316 stored.starts_with("just a subject\n\n"),
317 "trailer was glued onto the subject paragraph:\n---\n{}\n---",
318 stored
319 );
320 }
321
322 #[test]
323 fn round_trip_body_with_blank_lines() {
324 let (repo, id) = repo_with_hook_and_patch();
325 let stored = commit_and_read_back(
326 &repo,
327 "a.txt",
328 "subject\n\nfirst paragraph\n\nsecond paragraph",
329 );
330 assert_round_trips(&stored, &id);
331 }
332
333 #[test]
334 fn round_trip_last_paragraph_is_prose() {
335 // The parser refuses a final paragraph containing any prose line, so the
336 // trailer must start a paragraph of its own here.
337 let (repo, id) = repo_with_hook_and_patch();
338 let stored = commit_and_read_back(
339 &repo,
340 "a.txt",
341 "subject\n\nThanks Bob. This explains the change.",
342 );
343 assert_round_trips(&stored, &id);
344 }
345
346 #[test]
347 fn round_trip_message_already_ending_in_an_issue_trailer() {
348 // The trailer has to join the existing block, not start a new paragraph:
349 // a new paragraph would make `Issue:` stop being in the final paragraph,
350 // which silently breaks the commit-issue link the author wrote by hand.
351 let (repo, id) = repo_with_hook_and_patch();
352 let issue = repo.issue_open("something to fix");
353 let stored = commit_and_read_back(
354 &repo,
355 "a.txt",
356 &format!("subject\n\nIssue: {}", issue),
357 );
358 assert_round_trips(&stored, &id);
359 assert_eq!(
360 parse_trailers(&stored, ISSUE_TOKEN),
361 vec![issue],
362 "stamping the Patch: trailer broke the Issue: trailer:\n---\n{}\n---",
363 stored
364 );
365 }
366
367 #[test]
368 fn round_trip_message_with_a_comment_block() {
369 // What `git commit` without -m actually hands the hook: the message, then
370 // a blank line, then git's own `#` commentary.
371 //
372 // The message ends in a trailer here on purpose. Without that, this test
373 // passes even if the trailer is appended *below* the commentary: the
374 // commentary vanishes, the blank line we wrote survives, and the trailer
375 // ends up in a paragraph of its own either way. It is the existing `Issue:`
376 // trailer that makes position observable — appended below the comments, our
377 // trailer is separated from it by the blank line that preceded them, which
378 // pushes `Issue:` out of the final paragraph and unlinks it.
379 let (repo, id) = repo_with_hook_and_patch();
380 let issue = repo.issue_open("something to fix");
381 let editor = editor_writing(
382 &repo,
383 &format!("subject\n\nbody prose here\n\nIssue: {}\n", issue),
384 );
385 std::fs::write(repo.dir.path().join("a.txt"), "content").unwrap();
386 repo.git(&["add", "a.txt"]);
387 let out = git_with_editor(&repo, &editor, &["commit"]);
388 assert!(
389 out.status.success(),
390 "commit failed: {}",
391 String::from_utf8_lossy(&out.stderr)
392 );
393 let stored = repo.git(&["log", "-1", "--format=%B"]);
394 assert_round_trips(&stored, &id);
395 assert_eq!(
396 parse_trailers(&stored, ISSUE_TOKEN),
397 vec![issue],
398 "the Issue: trailer did not survive stamping:\n---\n{}\n---",
399 stored
400 );
401 assert!(
402 !stored.contains('#'),
403 "commentary leaked into the stored message:\n---\n{}\n---",
404 stored
405 );
406 }
407
408 #[test]
409 fn round_trip_message_with_a_scissors_line_and_a_diff() {
410 // `commit -v` puts a scissors line and a raw diff below the message. The
411 // trailer must land above the scissors, or git truncates it away.
412 let (repo, id) = repo_with_hook_and_patch();
413 let editor = editor_writing(&repo, "subject\n\nbody prose here\n");
414 std::fs::write(repo.dir.path().join("a.txt"), "content").unwrap();
415 repo.git(&["add", "a.txt"]);
416 let out = git_with_editor(&repo, &editor, &["commit", "-v"]);
417 assert!(
418 out.status.success(),
419 "commit failed: {}",
420 String::from_utf8_lossy(&out.stderr)
421 );
422 let stored = repo.git(&["log", "-1", "--format=%B"]);
423 assert_round_trips(&stored, &id);
424 assert!(
425 !stored.contains("diff --git"),
426 "the diff leaked into the stored message:\n---\n{}\n---",
427 stored
428 );
429 }
430
431 #[test]
432 fn an_empty_message_still_aborts_the_commit() {
433 // Stamping an otherwise-empty message would make it non-empty, and git
434 // would commit something the author meant to abandon. Nothing else in
435 // this suite can catch that: the abort *is* the assertion.
436 let (repo, _id) = repo_with_hook_and_patch();
437 let before = repo.git(&["rev-parse", "HEAD"]);
438 let editor = editor_writing(&repo, "");
439 std::fs::write(repo.dir.path().join("a.txt"), "content").unwrap();
440 repo.git(&["add", "a.txt"]);
441 let out = git_with_editor(&repo, &editor, &["commit"]);
442
443 assert!(
444 !out.status.success(),
445 "an empty commit message was committed anyway:\n{}",
446 String::from_utf8_lossy(&out.stdout)
447 );
448 assert_eq!(
449 before,
450 repo.git(&["rev-parse", "HEAD"]),
451 "HEAD moved despite the empty message"
452 );
453 }
454
455 // ---------------------------------------------------------------------------
456 // Idempotence
457 // ---------------------------------------------------------------------------
458
459 #[test]
460 fn amending_a_stamped_commit_does_not_add_a_second_trailer() {
461 let (repo, id) = repo_with_hook_and_patch();
462 commit_and_read_back(&repo, "a.txt", "subject");
463 repo.git(&["commit", "--amend", "--no-edit"]);
464 let stored = repo.git(&["log", "-1", "--format=%B"]);
465
466 assert_round_trips(&stored, &id);
467 assert_eq!(
468 stored.matches("Patch:").count(),
469 1,
470 "amend added a second trailer:\n---\n{}\n---",
471 stored
472 );
473 }
474
475 #[test]
476 fn a_hand_written_short_trailer_is_left_alone() {
477 // Someone who wrote `Patch: 2575fe16` by hand must not get a second,
478 // 40-char trailer stapled underneath it.
479 let (repo, id) = repo_with_hook_and_patch();
480 let stored = commit_and_read_back(&repo, "a.txt", &format!("subject\n\nPatch: {}", &id[..8]));
481 assert_eq!(
482 parse_trailers(&stored, PATCH_TOKEN),
483 vec![id[..8].to_string()],
484 "the hand-written trailer was not left alone:\n---\n{}\n---",
485 stored
486 );
487 }
488
489 // ---------------------------------------------------------------------------
490 // Which patch gets stamped
491 // ---------------------------------------------------------------------------
492
493 #[test]
494 fn a_branch_with_no_open_patch_is_not_stamped() {
495 let repo = TestRepo::new("Alice", "alice@example.com");
496 repo.run_ok(&["init"]);
497 repo.git(&["checkout", "-b", "unrelated"]);
498 let stored = commit_and_read_back(&repo, "a.txt", "subject");
499 assert_no_patch_trailer(&stored);
500 }
501
502 #[test]
503 fn two_open_patches_on_one_branch_stamp_nothing() {
504 // Ambiguous, so silence: guessing would record a merge of a patch that
505 // never landed, and there is no way to ask at commit time.
506 let (repo, _id) = repo_with_hook_and_patch();
507 repo.commit_file("b.txt", "two", "second commit");
508 repo.run_ok(&["patch", "create", "-t", "another patch", "-B", "feature"]);
509 let stored = commit_and_read_back(&repo, "c.txt", "subject");
510 assert_no_patch_trailer(&stored);
511 }
512
513 #[test]
514 fn a_closed_patch_on_the_branch_is_not_stamped() {
515 let (repo, id) = repo_with_hook_and_patch();
516 repo.patch_close(&id[..8]);
517 let stored = commit_and_read_back(&repo, "a.txt", "subject");
518 assert_no_patch_trailer(&stored);
519 }
520
521 #[test]
522 fn a_detached_head_is_not_stamped() {
523 let (repo, _id) = repo_with_hook_and_patch();
524 let head = repo.git(&["rev-parse", "HEAD"]).trim().to_string();
525 repo.git(&["checkout", "--detach", &head]);
526 let stored = commit_and_read_back(&repo, "a.txt", "subject");
527 assert_no_patch_trailer(&stored);
528 }
529
530 // ---------------------------------------------------------------------------
531 // Failing open
532 // ---------------------------------------------------------------------------
533
534 #[test]
535 fn a_commit_succeeds_when_the_git_collab_binary_is_gone() {
536 // Uninstalled, moved, or a PATH that a GUI git client never had. The hook
537 // outlives the binary and must not take the commit down with it.
538 let (repo, _id) = repo_with_hook_and_patch();
539 let path = hook_path(&repo);
540 let script = std::fs::read_to_string(&path).unwrap();
541 let broken = script.replace(
542 env!("CARGO_BIN_EXE_git-collab"),
543 "/nonexistent/bin/git-collab",
544 );
545 assert_ne!(script, broken, "hook does not embed the binary path");
546 std::fs::write(&path, broken).unwrap();
547
548 // A PATH with git on it and nothing else — so `git commit` still runs, and
549 // the hook's fallback to a bare `git-collab` finds nothing.
550 let bin = tempfile::TempDir::new().unwrap();
551 let git = String::from_utf8(
552 Command::new("sh")
553 .args(["-c", "command -v git"])
554 .output()
555 .unwrap()
556 .stdout,
557 )
558 .unwrap();
559 std::os::unix::fs::symlink(git.trim(), bin.path().join("git")).unwrap();
560
561 std::fs::write(repo.dir.path().join("a.txt"), "content").unwrap();
562 repo.git(&["add", "a.txt"]);
563 let mut command = Command::new("git");
564 repo.apply_env(&mut command);
565 let out = command
566 .env("PATH", bin.path())
567 .args(["commit", "-m", "subject"])
568 .current_dir(repo.dir.path())
569 .output()
570 .unwrap();
571
572 assert!(
573 out.status.success(),
574 "a missing binary blocked the commit:\n{}",
575 String::from_utf8_lossy(&out.stderr)
576 );
577 assert_eq!(
578 repo.git(&["log", "-1", "--format=%B"]).trim(),
579 "subject",
580 "the message was not left byte-identical"
581 );
582 }
583
584 #[test]
585 fn a_commit_succeeds_when_the_collab_refs_are_corrupt() {
586 let (repo, id) = repo_with_hook_and_patch();
587 // Point the patch's event ref at an object that is not there at all.
588 let ref_file = repo
589 .dir
590 .path()
591 .join(".git/refs/collab/patches")
592 .join(&id)
593 .join("events");
594 std::fs::create_dir_all(ref_file.parent().unwrap()).unwrap();
595 std::fs::write(&ref_file, "0000000000000000000000000000000000000001\n").unwrap();
596
597 let stored = commit_and_read_back(&repo, "a.txt", "subject");
598 assert_eq!(stored.trim(), "subject", "corrupt refs changed the message");
599 }
600
601 #[test]
602 fn the_hook_exits_zero_outside_a_git_repository() {
603 // git-collab itself exits 1 when it cannot open a repo. The shim has to
604 // swallow that, or a hook copied into a non-repo (or a repo whose gitdir
605 // has gone) blocks every commit.
606 let repo = TestRepo::new("Alice", "alice@example.com");
607 repo.run_ok(&["init"]);
608 let script = std::fs::read_to_string(hook_path(&repo)).unwrap();
609
610 let elsewhere = tempfile::TempDir::new().unwrap();
611 let hook = elsewhere.path().join("commit-msg");
612 std::fs::write(&hook, &script).unwrap();
613 let msg = elsewhere.path().join("MSG");
614 std::fs::write(&msg, "subject\n").unwrap();
615
616 let out = Command::new("sh")
617 .arg(&hook)
618 .arg(&msg)
619 .current_dir(elsewhere.path())
620 .env("HOME", elsewhere.path())
621 .output()
622 .unwrap();
623
624 assert!(
625 out.status.success(),
626 "hook exited {:?} outside a repo:\n{}",
627 out.status.code(),
628 String::from_utf8_lossy(&out.stderr)
629 );
630 assert_eq!(std::fs::read_to_string(&msg).unwrap(), "subject\n");
631 }
632
633 #[test]
634 fn the_hook_exits_zero_when_the_message_file_is_missing() {
635 let repo = TestRepo::new("Alice", "alice@example.com");
636 repo.run_ok(&["init"]);
637 let out = Command::new("sh")
638 .arg(hook_path(&repo))
639 .arg(repo.dir.path().join("no-such-file"))
640 .current_dir(repo.dir.path())
641 .output()
642 .unwrap();
643 assert!(out.status.success(), "a missing message file failed the hook");
644 }
645
646 // ---------------------------------------------------------------------------
647 // End to end: the three layers connected
648 // ---------------------------------------------------------------------------
649
650 #[test]
651 fn a_hook_stamped_commit_is_recorded_as_merged_by_sync() {
652 // The whole point of layer 1. No hand-written trailer, no `patch merge`.
653 let bare = tempfile::TempDir::new().unwrap();
654 let status = Command::new("git")
655 .args(["init", "--bare", "-b", "main"])
656 .arg(bare.path())
657 .status()
658 .unwrap();
659 assert!(status.success());
660
661 let repo = TestRepo::new("Alice", "alice@example.com");
662 repo.git(&["remote", "add", "origin", bare.path().to_str().unwrap()]);
663 repo.git(&["push", "-u", "origin", "main"]);
664 repo.run_ok(&["init"]);
665
666 repo.git(&["checkout", "-b", "feature"]);
667 repo.commit_file("f.txt", "one", "first commit");
668 let out = repo.run_ok(&["patch", "create", "-t", "feature work", "-B", "feature"]);
669 let short = out.trim().strip_prefix("Created patch ").unwrap().to_string();
670
671 // A commit made after the patch exists — the one the hook can stamp.
672 repo.commit_file("f.txt", "two", "more work");
673 repo.run_ok(&["patch", "revise", &short, "-b", "second pass"]);
674
675 let stored = repo.git(&["log", "-1", "--format=%B"]);
676 assert!(
677 !parse_trailers(&stored, PATCH_TOKEN).is_empty(),
678 "the hook did not stamp the commit:\n---\n{}\n---",
679 stored
680 );
681
682 repo.git(&["checkout", "main"]);
683 repo.git(&["merge", "--no-ff", "-m", "merge feature", "feature"]);
684 repo.run_ok(&["sync"]);
685
686 let json: Value = serde_json::from_str(&repo.run_ok(&["patch", "show", &short, "--json"])).unwrap();
687 assert_eq!(
688 json["status"].as_str().unwrap().to_lowercase(),
689 "merged",
690 "sync did not record the merge the hook made possible"
691 );
692 }
tests/merge_recording_test.rs
Old New
@@ -4,10 +4,10 @@
4 //! 4 //!
5 //! Layer 3 (`patch merge`) and layer 2 (sync scanning `Patch:` trailers) are 5 //! Layer 3 (`patch merge`) and layer 2 (sync scanning `Patch:` trailers) are
6 //! covered here, along with the demotion of reachability detection to a hint 6 //! covered here, along with the demotion of reachability detection to a hint
7 //! that never writes. Layer 1 (stamping the trailer — the `commit-msg` hook 7 //! that never writes. Every test below writes the trailer by hand, which is
8 //! and `patch create --stamp`) is deliberately not implemented, so every test 8 //! exactly what a user without the hook does — and what everyone does for
9 //! below writes the trailer by hand, which is exactly what a user without the 9 //! commits that predate it, since `patch create --stamp` is still out.
10 //! hook does. 10 //! Layer 1, the `commit-msg` hook, is in `tests/commit_msg_hook_test.rs`.
11 11
12 mod common; 12 mod common;
13 13