8485214a

Let prose in through more than one door, and let it be corrected

a73x 2026-08-10 17:02

Commit message

Let prose in through more than one door, and let it be corrected

Bodies could only arrive as a shell argument and could never be changed
afterwards. Three issues, one root cause.

Input (6edf0fa2). Every command taking --body now also takes
--body-file <path>, with '-' for stdin, under git's own -F. With no body
at all and a terminal, $EDITOR opens; without a terminal it is a clear
error rather than a hang, which is the failure mode that matters for
scripts and agents. Nothing is stripped or trimmed on the way in: a body
is markdown, so a leading '#' is a heading and a trailing blank line is
prose. Byte-identity through the DAG is tested directly.

Correction (e804b3e4, 7a299d2c). A new BodyEdit event supersedes an
earlier event's body, the same shape IssueEdit has always had, and
CommentDelete tombstones a comment. One mechanism covers thread
comments, inline comments, review bodies and revision descriptions,
since to it they are all a body hanging off an event OID. The DAG stays
append-only and the superseded event stays in it.

Deletion leaves a tombstone. The event is still in the DAG either way,
so showing nothing would misrepresent a record anyone can read; a
comment may have replies that reference it; and a tombstone makes delete
behave exactly like edit rather than being a second mechanism. The text
is dropped from derived state, so it stops reaching --json, search and
the web UI.

Corrections are applied in a second pass keyed on (clock, oid), so they
are independent of walk order and two clones editing offline converge.
An edit whose author is not the target's author is ignored by the fold,
not merely refused at the CLI: anyone with a copy of the DAG can append
to it, so the rule has to hold where every reader derives state.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

README.md

Old		New
@@ -130,12 +130,49 @@ $ git-collab patch review a1b2c3d4 --verdict request-changes -b "see inline"
130	`git-collab dashboard` opens a TUI over the same data if you would rather browse	130	`git-collab dashboard` opens a TUI over the same data if you would rather browse
131	than type.	131	than type.
132		132
		133	## Writing and correcting prose
		134
		135	Review prose is long, multi-line, and full of things a shell wants to
		136	interpret. Every command that takes `--body` also reads one from a file, or
		137	from stdin with `-`, using git's own `-F` convention — so a body never has to
		138	be written around the shell:
		139
		140	```console
		141	$ git-collab patch review a1b2c3d4 -v approve -F review.md
		142	$ some-tool \| git-collab patch comment a1b2c3d4 -F -
		143	$ git-collab patch comment a1b2c3d4 # no body: opens $EDITOR
		144	```
		145
		146	Whatever you supply is stored byte for byte — trailing newlines, non-ASCII and
		147	leading `#` included. Nothing is stripped, because a body is markdown, not a
		148	commit message.
		149
		150	Typos are fixable. Editing appends a new event that supersedes the old body;
		151	the original stays in the DAG, so the log remains an audit trail:
		152
		153	```console
		154	$ git-collab patch show a1b2c3d4 # comment IDs are printed in [brackets]
		155	$ git-collab patch edit-comment a1b2c3d4 f39bf43b -b "what I meant to say"
		156	$ git-collab patch edit-revision a1b2c3d4 3 -F - # fix a revision's description
		157	$ git-collab patch delete-comment a1b2c3d4 f39bf43b
		158	```
		159
		160	Deleting leaves a tombstone rather than a hole: the comment keeps its position,
		161	author and timestamp, and only its text is dropped. A review is a conversation,
		162	and a comment that silently vanished would leave every reply to it pointing at
		163	nothing.
		164
		165	Only a comment's own author can change it. An edit signed by anyone else is
		166	ignored when state is derived, not merely refused at the command line — anyone
		167	holding a copy of the DAG can append to it, so the rule has to hold where every
		168	reader folds it.
		169
133	## Commands	170	## Commands
134		171
135	\| \| \|	172	\| \| \|
136	\|---\|---\|	173	\|---\|---\|
137	\| `issue` \| open, list, show, comment, edit, label, assign, close \|	174	\| `issue` \| open, list, show, comment, edit, edit-comment, delete-comment, label, assign, close \|
138	\| `patch` \| create, list, show, diff, comment, review, revise, log, checkout, merge, close \|	175	\| `patch` \| create, list, show, diff, comment, review, revise, edit-comment, delete-comment, edit-revision, log, checkout, merge, close \|
139	\| `sync` \| fetch, reconcile and push collab refs \|	176	\| `sync` \| fetch, reconcile and push collab refs \|
140	\| `hooks` \| install and inspect the `commit-msg` trailer hook \|	177	\| `hooks` \| install and inspect the `commit-msg` trailer hook \|
141	\| `status` \| project overview \|	178	\| `status` \| project overview \|

src/body.rs

@@ @@ -0,0 +1,205 @@ @@
+//! Getting prose into git-collab.
+//!
+//! A body used to arrive exactly one way: as a shell argument. That is a bad
+//! door for the kind of text this tool carries. Review prose is long,
+//! multi-line, and full of things a shell wants to interpret — backticks for
+//! code spans, `$` in a variable name being discussed, quotes of both kinds,
+//! and `?` or `*` in ordinary sentences. A reviewer on a hardened shell had a
+//! comment rejected outright for containing a `?` and had to re-author it
+//! around the shell. Prose should never have to be written around the tool
+//! that stores it.
+//!
+//! So every command taking `--body` also takes `--body-file <path>`, with `-`
+//! meaning stdin, under the short flag `-F`. That is git's own convention
+//! (`git commit -F`), which is the point: users already know it, and one
+//! convention covering the whole tool is worth more than a bespoke one
+//! covering the three commands an issue happened to name.
+//!
+//! Nothing here trims, strips, or normalizes. What the caller supplies is
+//! what gets recorded, byte for byte, including trailing newlines, CRLF and
+//! non-ASCII. Bodies are markdown and go into an append-only DAG: a tidy-up
+//! applied on the way in is a corruption nobody can correct afterwards, and
+//! one that would only surface in someone's review three weeks later.
+use std::io::{IsTerminal, Read};
+use crate::editor;
+use crate::error::Error;
+/// How a command was asked to find its body.
+pub struct BodyArgs<'a> {
+    pub body: Option<&'a str>,
+    pub body_file: Option<&'a str>,
+}
+impl<'a> BodyArgs<'a> {
+    pub fn new(body: Option<&'a str>, body_file: Option<&'a str>) -> Self {
+        BodyArgs { body, body_file }
+    }
+    /// Read whichever of the two was supplied, rejecting both at once.
+    fn read_supplied(&self) -> Result<Option<String>, Error> {
+        match (self.body, self.body_file) {
+            (Some(_), Some(_)) => Err(Error::Cmd(
+                "--body and --body-file are mutually exclusive; pass one or the other".to_string(),
+            )),
+            (Some(text), None) => Ok(Some(text.to_string())),
+            (None, Some(path)) => read_file_or_stdin(path).map(Some),
+            (None, None) => Ok(None),
+        }
+    }
+}
+/// Read a body from a path, or from stdin when the path is `-`.
+fn read_file_or_stdin(path: &str) -> Result<String, Error> {
+    let bytes = if path == "-" {
+        let mut buf = Vec::new();
+        std::io::stdin()
+            .read_to_end(&mut buf)
+            .map_err(|e| Error::Cmd(format!("failed to read the body from stdin: {}", e)))?;
+        buf
+    } else {
+        std::fs::read(path)
+            .map_err(|e| Error::Cmd(format!("failed to read the body from {}: {}", path, e)))?
+    };
+    String::from_utf8(bytes).map_err(|_| {
+        Error::Cmd(format!(
+            "the body read from {} is not valid UTF-8",
+            if path == "-" { "stdin" } else { path }
+        ))
+    })
+}
+/// The error a non-interactive caller gets when it supplies no body at all.
+///
+/// Naming the alternatives matters more here than anywhere else in the tool:
+/// the callers that hit this are scripts and agents, which have no terminal
+/// to open an editor on and no human to read a terse refusal.
+fn no_body_error(what: &str) -> Error {
+    Error::Cmd(format!(
+        "no {what} given: pass --body <text>, --body-file <path>, or --body-file - to read stdin.\n\
+         (An editor is opened instead only when stdin is a terminal and $EDITOR or $VISUAL is set.)"
+    ))
+}
+/// Reject a body that is entirely blank.
+///
+/// An empty body is almost always an accident — an editor closed without
+/// saving, an empty file passed by a script — and an accident recorded in an
+/// append-only DAG is permanent. Only whitespace-only bodies are refused;
+/// anything with content in it is passed through untouched, trailing blank
+/// lines and all.
+fn reject_if_blank(body: String, what: &str) -> Result<String, Error> {
+    if body.trim().is_empty() {
+        return Err(Error::Cmd(format!("empty {what}; aborting")));
+    }
+    Ok(body)
+}
+/// Whether an editor can be opened right now: there has to be one configured,
+/// and stdin has to be a terminal.
+///
+/// The terminal check is what keeps this safe for scripts and agents. An
+/// editor launched with no terminal does not fail — it blocks, forever, on a
+/// command the caller expected to return. A clear error beats a hang.
+fn can_open_editor() -> bool {
+    editor::resolve_editor().is_some() && std::io::stdin().is_terminal()
+}
+/// Resolve a body that the command requires, opening an editor seeded with
+/// `initial` when none was supplied and a terminal is available.
+///
+/// `what` names the thing being written ("comment", "review body") so the
+/// error reads as a sentence.
+pub fn resolve_required(args: &BodyArgs, initial: &str, what: &str) -> Result<String, Error> {
+    if let Some(body) = args.read_supplied()? {
+        return reject_if_blank(body, what);
+    }
+    if !can_open_editor() {
+        return Err(no_body_error(what));
+    }
+    reject_if_blank(editor::compose(initial)?, what)
+}
+/// Resolve a body that the command treats as optional.
+///
+/// No body means "leave it as it is" (`issue edit`) or "there isn't one"
+/// (`issue open`), which are both meaningful answers — so unlike
+/// [`resolve_required`], omitting it never opens an editor. Doing otherwise
+/// would turn every existing non-interactive `issue open -t ...` into a
+/// command that blocks on an editor.
+pub fn resolve_optional(args: &BodyArgs) -> Result<Option<String>, Error> {
+    args.read_supplied()
+}
+#[cfg(test)]
+mod tests {
+    use super::*;
+    #[test]
+    fn body_flag_is_passed_through_verbatim() {
+        let args = BodyArgs::new(Some("  spaced\n\n"), None);
+        assert_eq!(
+            resolve_required(&args, "", "comment").unwrap(),
+            "  spaced\n\n"
+        );
+    }
+    #[test]
+    fn both_options_together_are_rejected() {
+        let args = BodyArgs::new(Some("a"), Some("b"));
+        let err = format!("{}", resolve_required(&args, "", "comment").unwrap_err());
+        assert!(err.contains("--body") && err.contains("--body-file"), "{}", err);
+    }
+    #[test]
+    fn a_file_is_read_byte_for_byte() {
+        let mut file = tempfile::NamedTempFile::new().unwrap();
+        let content = "trailing newlines\n\n\nnaïve 🎉\r\n";
+        std::io::Write::write_all(&mut file, content.as_bytes()).unwrap();
+        let path = file.path().to_str().unwrap().to_string();
+        let args = BodyArgs::new(None, Some(&path));
+        assert_eq!(resolve_required(&args, "", "comment").unwrap(), content);
+    }
+    #[test]
+    fn a_missing_file_names_the_path() {
+        let args = BodyArgs::new(None, Some("/nonexistent/git-collab-body.txt"));
+        let err = format!("{}", resolve_required(&args, "", "comment").unwrap_err());
+        assert!(err.contains("/nonexistent/git-collab-body.txt"), "{}", err);
+    }
+    #[test]
+    fn a_blank_body_is_refused() {
+        let args = BodyArgs::new(Some("   \n\t\n"), None);
+        let err = format!("{}", resolve_required(&args, "", "comment").unwrap_err());
+        assert!(err.contains("empty comment"), "{}", err);
+    }
+    #[test]
+    fn invalid_utf8_is_refused_rather_than_mangled() {
+        let mut file = tempfile::NamedTempFile::new().unwrap();
+        std::io::Write::write_all(&mut file, &[0xff, 0xfe, 0x00]).unwrap();
+        let path = file.path().to_str().unwrap().to_string();
+        let args = BodyArgs::new(None, Some(&path));
+        let err = format!("{}", resolve_required(&args, "", "comment").unwrap_err());
+        assert!(err.contains("not valid UTF-8"), "{}", err);
+    }
+    #[test]
+    fn an_optional_body_stays_absent_when_nothing_is_given() {
+        let args = BodyArgs::new(None, None);
+        assert_eq!(resolve_optional(&args).unwrap(), None);
+    }
+    #[test]
+    fn an_optional_body_may_be_blank() {
+        // `issue open -b ""` has always meant "no description", and still does.
+        let args = BodyArgs::new(Some(""), None);
+        assert_eq!(resolve_optional(&args).unwrap(), Some(String::new()));
+    }
+}

src/cache.rs

Old		New
@@ -32,7 +32,15 @@ fn sanitize_ref_name(ref_name: &str) -> String {
32	/// indistinguishably from a patch that really has no recorded merge, so a	32	/// indistinguishably from a patch that really has no recorded merge, so a
33	/// merged patch cached before this field existed would read as merged with no	33	/// merged patch cached before this field existed would read as merged with no
34	/// commit forever. Only the version can force the refold.	34	/// commit forever. Only the version can force the refold.
35	const CACHE_FORMAT_VERSION: u32 = 6;	35	/// v7: bodies became correctable. Comments, reviews and revisions gained the
		36	/// event OID that identifies them, plus `edited`/`deleted`, and the fold grew
		37	/// a second pass that applies `BodyEdit`/`CommentDelete`. Every one of those
		38	/// fields is `#[serde(default)]`, so a v6 entry deserializes perfectly — and
		39	/// wrongly: a comment deleted since would be served with its text intact and
		40	/// `deleted: false`, which is the deleted text leaking back out through the
		41	/// cache. That is the whole point of versioning this separately from the
		42	/// shape, so bump rather than trust the defaults.
		43	const CACHE_FORMAT_VERSION: u32 = 7;
36		44
37	/// Cache entry stored on disk: the tip OID at cache time + serialized state.	45	/// Cache entry stored on disk: the tip OID at cache time + serialized state.
38	#[derive(serde::Serialize, serde::Deserialize)]	46	#[derive(serde::Serialize, serde::Deserialize)]

src/cli.rs

Old		New
@@ -295,6 +295,15 @@ pub enum HookCmd {
295	},	295	},
296	}	296	}
297		297
		298	/// Doc text for `--body-file`, shared by every command that takes one so the
		299	/// convention is stated identically wherever it appears.
		300	///
		301	/// `-F` rather than `-f` throughout: `-f` is already `--file` on
		302	/// `patch comment`, where it names the source file an inline comment is
		303	/// attached to. `-F` is also what `git commit` uses, which is the whole
		304	/// argument for the flag.
		305	const BODY_FILE_HELP: &str = "Read the body from a file, or from stdin with '-'";
		306
298	#[derive(Subcommand, Debug)]	307	#[derive(Subcommand, Debug)]
299	pub enum IssueCmd {	308	pub enum IssueCmd {
300	/// Open a new issue	309	/// Open a new issue
@@ -306,8 +315,10 @@ pub enum IssueCmd {
306	#[arg(short, long)]	315	#[arg(short, long)]
307	title: String,	316	title: String,
308	/// Issue body	317	/// Issue body
309	#[arg(short, long, default_value = "")]	318	#[arg(short, long)]
310	body: String,	319	body: Option<String>,
		320	#[arg(short = 'F', long, help = BODY_FILE_HELP)]
		321	body_file: Option<String>,
311	/// Related issue ID	322	/// Related issue ID
312	#[arg(long)]	323	#[arg(long)]
313	relates_to: Option<String>,	324	relates_to: Option<String>,
@@ -347,12 +358,16 @@ pub enum IssueCmd {
347	json: bool,	358	json: bool,
348	},	359	},
349	/// Comment on an issue	360	/// Comment on an issue
		361	///
		362	/// With no --body and no --body-file, opens $EDITOR.
350	Comment {	363	Comment {
351	/// Issue ID (prefix match)	364	/// Issue ID (prefix match)
352	id: String,	365	id: String,
353	/// Comment body	366	/// Comment body
354	#[arg(short, long)]	367	#[arg(short, long)]
355	body: String,	368	body: Option<String>,
		369	#[arg(short = 'F', long, help = BODY_FILE_HELP)]
		370	body_file: Option<String>,
356	},	371	},
357	/// Edit an issue's title or body	372	/// Edit an issue's title or body
358	Edit {	373	Edit {
@@ -364,6 +379,34 @@ pub enum IssueCmd {
364	/// New body	379	/// New body
365	#[arg(short, long)]	380	#[arg(short, long)]
366	body: Option<String>,	381	body: Option<String>,
		382	#[arg(short = 'F', long, help = BODY_FILE_HELP)]
		383	body_file: Option<String>,
		384	},
		385	/// Correct the text of a comment you wrote
		386	///
		387	/// Records a new event superseding the old body; the original stays in
		388	/// the DAG. Only the comment's own author can edit it. With no --body and
		389	/// no --body-file, opens $EDITOR seeded with the current text.
		390	EditComment {
		391	/// Issue ID (prefix match)
		392	id: String,
		393	/// Comment ID (prefix match), as printed by `issue show`
		394	comment: String,
		395	/// New body
		396	#[arg(short, long)]
		397	body: Option<String>,
		398	#[arg(short = 'F', long, help = BODY_FILE_HELP)]
		399	body_file: Option<String>,
		400	},
		401	/// Delete a comment you wrote, leaving a tombstone in its place
		402	///
		403	/// The comment keeps its position, author and timestamp so replies to it
		404	/// still make sense; only its text is dropped.
		405	DeleteComment {
		406	/// Issue ID (prefix match)
		407	id: String,
		408	/// Comment ID (prefix match), as printed by `issue show`
		409	comment: String,
367	},	410	},
368	/// Add a label to an issue	411	/// Add a label to an issue
369	Label {	412	Label {
@@ -488,8 +531,10 @@ pub enum PatchCmd {
488	#[arg(short, long)]	531	#[arg(short, long)]
489	title: String,	532	title: String,
490	/// Patch description	533	/// Patch description
491	#[arg(short, long, default_value = "")]	534	#[arg(short, long)]
492	body: String,	535	body: Option<String>,
		536	#[arg(short = 'F', long, help = BODY_FILE_HELP)]
		537	body_file: Option<String>,
493	/// Base branch ref	538	/// Base branch ref
494	#[arg(long, default_value = "main")]	539	#[arg(long, default_value = "main")]
495	base: String,	540	base: String,
@@ -549,12 +594,16 @@ pub enum PatchCmd {
549	between: Option<Vec<u32>>,	594	between: Option<Vec<u32>>,
550	},	595	},
551	/// Comment on a patch (use --file and --line for inline comments)	596	/// Comment on a patch (use --file and --line for inline comments)
		597	///
		598	/// With no --body and no --body-file, opens $EDITOR.
552	Comment {	599	Comment {
553	/// Patch ID (prefix match)	600	/// Patch ID (prefix match)
554	id: String,	601	id: String,
555	/// Comment body	602	/// Comment body
556	#[arg(short, long)]	603	#[arg(short, long)]
557	body: String,	604	body: Option<String>,
		605	#[arg(short = 'F', long, help = BODY_FILE_HELP)]
		606	body_file: Option<String>,
558	/// File path for inline comment	607	/// File path for inline comment
559	#[arg(short, long)]	608	#[arg(short, long)]
560	file: Option<String>,	609	file: Option<String>,
@@ -566,6 +615,8 @@ pub enum PatchCmd {
566	revision: Option<u32>,	615	revision: Option<u32>,
567	},	616	},
568	/// Review a patch	617	/// Review a patch
		618	///
		619	/// With no --body and no --body-file, opens $EDITOR.
569	Review {	620	Review {
570	/// Patch ID (prefix match)	621	/// Patch ID (prefix match)
571	id: String,	622	id: String,
@@ -574,7 +625,9 @@ pub enum PatchCmd {
574	verdict: String,	625	verdict: String,
575	/// Review body	626	/// Review body
576	#[arg(short, long)]	627	#[arg(short, long)]
577	body: String,	628	body: Option<String>,
		629	#[arg(short = 'F', long, help = BODY_FILE_HELP)]
		630	body_file: Option<String>,
578	/// Target revision for review	631	/// Target revision for review
579	#[arg(long)]	632	#[arg(long)]
580	revision: Option<u32>,	633	revision: Option<u32>,
@@ -587,10 +640,58 @@ pub enum PatchCmd {
587	/// Revision description	640	/// Revision description
588	#[arg(short, long)]	641	#[arg(short, long)]
589	body: Option<String>,	642	body: Option<String>,
		643	#[arg(short = 'F', long, help = BODY_FILE_HELP)]
		644	body_file: Option<String>,
590	/// Source branch to snapshot (defaults to HEAD)	645	/// Source branch to snapshot (defaults to HEAD)
591	#[arg(short = 'B', long)]	646	#[arg(short = 'B', long)]
592	branch: Option<String>,	647	branch: Option<String>,
593	},	648	},
		649	/// Correct the text of a comment or review you wrote
		650	///
		651	/// Takes the ID of a thread comment, an inline comment or a review, as
		652	/// printed by `patch show`. Records a new event superseding the old body;
		653	/// the original stays in the DAG, and a review's verdict is untouched.
		654	/// Only the author can edit their own. With no --body and no --body-file,
		655	/// opens $EDITOR seeded with the current text.
		656	EditComment {
		657	/// Patch ID (prefix match)
		658	id: String,
		659	/// Comment or review ID (prefix match), as printed by `patch show`
		660	comment: String,
		661	/// New body
		662	#[arg(short, long)]
		663	body: Option<String>,
		664	#[arg(short = 'F', long, help = BODY_FILE_HELP)]
		665	body_file: Option<String>,
		666	},
		667	/// Delete a comment you wrote, leaving a tombstone in its place
		668	///
		669	/// The comment keeps its position, author and timestamp so replies to it
		670	/// still make sense; only its text is dropped. Reviews cannot be deleted
		671	/// — they carry a vote; submit a new review to change it.
		672	DeleteComment {
		673	/// Patch ID (prefix match)
		674	id: String,
		675	/// Comment ID (prefix match), as printed by `patch show`
		676	comment: String,
		677	},
		678	/// Correct a revision's description
		679	///
		680	/// A revision's commit and tree are immutable and stay so; this changes
		681	/// only the descriptive body, for the case where a placeholder or a typo
		682	/// was recorded. With no --body and no --body-file, opens $EDITOR seeded
		683	/// with the current text.
		684	EditRevision {
		685	/// Patch ID (prefix match)
		686	id: String,
		687	/// Revision number, as printed by `patch log`
		688	revision: u32,
		689	/// New description
		690	#[arg(short, long)]
		691	body: Option<String>,
		692	#[arg(short = 'F', long, help = BODY_FILE_HELP)]
		693	body_file: Option<String>,
		694	},
594	/// Show revision log for a patch	695	/// Show revision log for a patch
595	Log {	696	Log {
596	/// Patch ID (prefix match)	697	/// Patch ID (prefix match)
@@ -660,6 +761,8 @@ impl Commands {
660	\| IssueCmd::Comment { .. }	761	\| IssueCmd::Comment { .. }
661	\| IssueCmd::Close { .. }	762	\| IssueCmd::Close { .. }
662	\| IssueCmd::Edit { .. }	763	\| IssueCmd::Edit { .. }
		764	\| IssueCmd::EditComment { .. }
		765	\| IssueCmd::DeleteComment { .. }
663	\| IssueCmd::Label { .. }	766	\| IssueCmd::Label { .. }
664	\| IssueCmd::Unlabel { .. }	767	\| IssueCmd::Unlabel { .. }
665	\| IssueCmd::Relate { .. }	768	\| IssueCmd::Relate { .. }
@@ -674,6 +777,9 @@ impl Commands {
674	\| PatchCmd::Comment { .. }	777	\| PatchCmd::Comment { .. }
675	\| PatchCmd::Review { .. }	778	\| PatchCmd::Review { .. }
676	\| PatchCmd::Revise { .. }	779	\| PatchCmd::Revise { .. }
		780	\| PatchCmd::EditComment { .. }
		781	\| PatchCmd::DeleteComment { .. }
		782	\| PatchCmd::EditRevision { .. }
677	\| PatchCmd::Label { .. }	783	\| PatchCmd::Label { .. }
678	\| PatchCmd::Unlabel { .. }	784	\| PatchCmd::Unlabel { .. }
679	\| PatchCmd::Close { .. }	785	\| PatchCmd::Close { .. }

src/dag.rs

Old		New
@@ -338,6 +338,8 @@ fn commit_message(action: &Action) -> String {
338	}	338	}
339	Action::PatchClose { .. } => "patch: close".to_string(),	339	Action::PatchClose { .. } => "patch: close".to_string(),
340	Action::PatchMerge { .. } => "patch: merge".to_string(),	340	Action::PatchMerge { .. } => "patch: merge".to_string(),
		341	Action::BodyEdit { target, .. } => format!("collab: edit body of {:.8}", target),
		342	Action::CommentDelete { target } => format!("collab: delete comment {:.8}", target),
341	Action::Merge => "collab: merge".to_string(),	343	Action::Merge => "collab: merge".to_string(),
342	}	344	}
343	}	345	}

src/editor.rs

@@ @@ -1,7 +1,63 @@ @@
+use std::io::Write;
+use std::process::Command;
+use crate::error::Error;
+/// Compose text in the user's editor, seeded with `initial`.
+///
+/// Whatever the editor leaves in the file *is* the body: nothing is stripped,
+/// trimmed or normalized on the way back. This is deliberately unlike `git
+/// commit`, which strips `#` lines and trailing whitespace — a review body is
+/// markdown, where a leading `#` is a heading and a trailing blank line is
+/// part of the prose. Anything that "helpfully" tidied it would silently
+/// corrupt a body nobody could then correct.
+pub fn compose(initial: &str) -> Result<String, Error> {
+    let editor_str = resolve_editor()
+        .ok_or_else(|| Error::Cmd("No editor configured. Set $EDITOR or $VISUAL.".to_string()))?;
+    compose_with(initial, &editor_str)
+}
+/// Inner helper: compose with an explicit editor command, so tests can drive
+/// it without mutating environment variables.
+fn compose_with(initial: &str, editor_str: &str) -> Result<String, Error> {
+    let parts: Vec<&str> = editor_str.split_whitespace().collect();
+    let (program, extra_args) = parts
+        .split_first()
+        .ok_or_else(|| Error::Cmd("Editor command is empty".to_string()))?;
+    // `.md` so editors pick markdown highlighting and wrapping for what is,
+    // in practice, always markdown.
+    let mut file = tempfile::Builder::new()
+        .prefix("git-collab-")
+        .suffix(".md")
+        .tempfile()
+        .map_err(|e| Error::Cmd(format!("Failed to create a buffer for the editor: {}", e)))?;
+    file.write_all(initial.as_bytes())
+        .and_then(|()| file.flush())
+        .map_err(|e| Error::Cmd(format!("Failed to seed the editor buffer: {}", e)))?;
+    let path = file.path().to_path_buf();
+    let status = Command::new(program)
+        .args(extra_args)
+        .arg(&path)
+        .status()
+        .map_err(|e| Error::Cmd(format!("Failed to launch editor '{}': {}", program, e)))?;
+    if !status.success() {
+        return Err(Error::Cmd(format!(
+            "Editor exited with status: {}",
+            status.code().unwrap_or(-1)
+        )));
+    }
+    // Read the path rather than the handle: editors routinely save by writing
+    // a new file and renaming it over the old one, which leaves the original
+    // handle pointing at the unedited inode.
+    let bytes = std::fs::read(&path)
+        .map_err(|e| Error::Cmd(format!("Failed to read back the editor buffer: {}", e)))?;
+    String::from_utf8(bytes)
+        .map_err(|_| Error::Cmd("Editor produced text that is not valid UTF-8".to_string()))
+}
+/// Resolve the user's preferred editor by checking $VISUAL, then $EDITOR.
+/// Returns `None` if neither is set or both are empty.
+pub fn resolve_editor() -> Option<String> {
@@ @@ -246,6 +302,76 @@ mod tests { @@
+        assert!(result.is_ok(), "Expected Ok, got {:?}", result);
+    // ---- compose tests ----
+    /// The property the whole feature rests on: what the editor saves is what
+    /// gets recorded. Trailing newlines, CRLF, tabs, non-ASCII, a combining
+    /// mark, and a leading `#` all survive untouched.
+    #[test]
+    fn test_compose_returns_the_buffer_byte_for_byte() {
+        let content = "# Heading\n\n\tTabbed\r\nnaïve café — 🎉 e\u{0301}\n\n";
+        let script = fake_editor(&format!(
+            "#!/bin/sh\ncat > \"$1\" <<'GITCOLLABEOF'\n{}GITCOLLABEOF\n",
+            content
+        ));
+        // A heredoc adds the newline the content already ends with, so drop
+        // one to get back exactly what went in.
+        let out = compose_with("", &script).unwrap();
+        assert_eq!(out.trim_end_matches('\n'), content.trim_end_matches('\n'));
+        assert!(out.starts_with("# Heading"), "no stripping of leading #");
+        assert!(out.contains('\t') && out.contains('\r'), "no whitespace tidying");
+        assert!(out.contains('\u{301}'), "non-ASCII preserved");
+    }
+    #[test]
+    fn test_compose_seeds_the_buffer_with_the_initial_text() {
+        let script = fake_editor("#!/bin/sh\nprintf '%s!' \"$(cat \"$1\")\" > \"$1\"\n");
+        let out = compose_with("seed", &script).unwrap();
+        assert_eq!(out, "seed!");
+    }
+    /// Editors commonly save by writing a temp file and renaming it over the
+    /// original. Reading the path rather than the open handle is what keeps
+    /// that from silently returning the unedited seed.
+    #[test]
+    fn test_compose_survives_an_editor_that_saves_by_rename() {
+        let script = fake_editor(
+            "#!/bin/sh\nprintf 'renamed in\\n' > \"$1.new\"\nmv \"$1.new\" \"$1\"\n",
+        );
+        let out = compose_with("seed", &script).unwrap();
+        assert_eq!(out, "renamed in\n");
+    }
+    #[test]
+    fn test_compose_propagates_a_failing_editor() {
+        let result = compose_with("seed", "false");
+        assert!(result.is_err());
+        assert!(format!("{}", result.unwrap_err()).contains("Editor exited with status"));
+    }
+    #[test]
+    fn test_compose_rejects_non_utf8() {
+        let script = fake_editor("#!/bin/sh\nprintf '\\377\\376' > \"$1\"\n");
+        let result = compose_with("", &script);
+        assert!(format!("{}", result.unwrap_err()).contains("not valid UTF-8"));
+    }
+    /// Write a shell script to a temp dir and return an editor *command* that
+    /// runs it via `sh`, leaking the dir so the script outlives the call.
+    ///
+    /// `sh <path>` rather than making the file executable and running it
+    /// directly: these tests run on many threads, and a `Command::spawn` in
+    /// one thread inherits any write file descriptor another thread happens
+    /// to have open, which makes exec'ing a freshly written script fail with
+    /// ETXTBSY at random. `sh` only ever opens it for reading. Test-only.
+    fn fake_editor(body: &str) -> String {
+        let dir = tempfile::TempDir::new().unwrap();
+        let path = dir.path().join("fake-editor.sh");
+        std::fs::write(&path, body).unwrap();
+        std::mem::forget(dir);
+        format!("sh {}", path.display())
+    }
+    #[test]
+    fn test_open_editor_at_bad_command() {
+        let mut tmp = tempfile::NamedTempFile::new().unwrap();

src/event.rs

@@ @@ -154,6 +154,46 @@ pub enum Action { @@
+        #[serde(default, skip_serializing_if = "String::is_empty")]
+        commit: String,
+    },
+    /// Supersede the body of an earlier event in the same DAG.
+    ///
+    /// Prose enters this tool once and used to be stuck there. This is the
+    /// correction, and it is a new event rather than a mutation because the
+    /// DAG is append-only: the superseded event stays exactly where it is,
+    /// still signed by whoever wrote it, and the log remains an audit trail
+    /// rather than a summary. It is the same shape as `IssueEdit`, which has
+    /// always superseded an issue's title and body this way.
+    ///
+    /// `target` is the hex OID of the event commit that first carried the
+    /// body. One event covers thread comments, inline comments, review bodies
+    /// and revision descriptions, because to this mechanism they are all just
+    /// a body hanging off an event OID — there is no reason for four.
+    ///
+    /// An edit whose author differs from the target's author is **ignored by
+    /// the fold**, not honoured and not an error: anyone holding a copy of the
+    /// DAG can append anything to it, so the rule that nobody rewrites
+    /// somebody else's words has to hold where every reader derives state,
+    /// not merely at the CLI that politely declines. See
+    /// `state::apply_body_overrides`.
+    #[serde(rename = "body.edit")]
+    BodyEdit { target: String, body: String },
+    /// Tombstone a comment: its text is dropped from derived state, but the
+    /// comment keeps its slot, its author and its timestamp.
+    ///
+    /// A tombstone rather than a disappearance, for three reasons. The event
+    /// is still in the DAG either way, so a UI that showed nothing would be
+    /// lying about a record anyone can still read. A review is a conversation
+    /// and the comment above or below may be a reply, which references nothing
+    /// if its antecedent silently evaporates. And it makes delete behave like
+    /// edit — same target, same `(clock, oid)` resolution, same author rule —
+    /// instead of being a second mechanism with its own edge cases.
+    ///
+    /// Comments only. A review carries a vote, so removing one would drop a
+    /// verdict silently; a reviewer changes their mind by submitting a new
+    /// review, which supersedes the old vote through the existing rule.
+    #[serde(rename = "comment.delete")]
+    CommentDelete { target: String },
+    #[serde(rename = "collab.merge")]
+    Merge,

src/issue.rs

@@ @@ -254,6 +254,56 @@ pub fn edit( @@
+    Ok(())
+/// Correct the text of a comment.
+///
+/// `body_args` is resolved *after* the target, so an omitted body can open an
+/// editor seeded with the text as it stands — the one-keystroke typo fix that
+/// is the whole point.
+pub fn edit_comment(
+    repo: &Repository,
+    id_prefix: &str,
+    comment_prefix: &str,
+    body_args: &crate::body::BodyArgs,
+) -> Result<(), crate::error::Error> {
+    let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?;
+    let issue = IssueState::from_ref(repo, &ref_name, &id)?;
+    let target = issue.resolve_comment(comment_prefix)?;
+    crate::patch::require_own_body(repo, &target)?;
+    let body = crate::body::resolve_required(body_args, &target.body, "comment")?;
+    dag::append_action(
+        repo,
+        &ref_name,
+        Action::BodyEdit {
+            target: target.oid,
+            body,
+        },
+    )?;
+    Ok(())
+}
+/// Tombstone a comment: drop its text, keep its slot.
+pub fn delete_comment(
+    repo: &Repository,
+    id_prefix: &str,
+    comment_prefix: &str,
+) -> Result<(), crate::error::Error> {
+    let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?;
+    let issue = IssueState::from_ref(repo, &ref_name, &id)?;
+    let target = issue.resolve_comment(comment_prefix)?;
+    crate::patch::require_own_body(repo, &target)?;
+    crate::patch::require_deletable(&target)?;
+    dag::append_action(
+        repo,
+        &ref_name,
+        Action::CommentDelete {
+            target: target.oid,
+        },
+    )?;
+    Ok(())
+}
+pub fn comment(repo: &Repository, id_prefix: &str, body: &str) -> Result<(), crate::error::Error> {
+    let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;
+    dag::append_action(

src/lib.rs

Old		New
@@ -1,4 +1,5 @@
1	pub mod abbrev;	1	pub mod abbrev;
		2	pub mod body;
2	pub mod cache;	3	pub mod cache;
3	pub mod cli;	4	pub mod cli;
4	pub mod commit_link;	5	pub mod commit_link;
@@ -155,8 +156,11 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
155	IssueCmd::Open {	156	IssueCmd::Open {
156	title,	157	title,
157	body,	158	body,
		159	body_file,
158	relates_to,	160	relates_to,
159	} => {	161	} => {
		162	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());
		163	let body = body::resolve_optional(&args)?.unwrap_or_default();
160	let id = issue::open(repo, &title, &body, relates_to.as_deref())?;	164	let id = issue::open(repo, &title, &body, relates_to.as_deref())?;
161	println!("Opened issue {}", abbrev::for_issues(repo).of(&id));	165	println!("Opened issue {}", abbrev::for_issues(repo).of(&id));
162	Ok(())	166	Ok(())
@@ -220,7 +224,14 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
220	if !i.comments.is_empty() {	224	if !i.comments.is_empty() {
221	println!("\n--- Comments ---");	225	println!("\n--- Comments ---");
222	for c in &i.comments {	226	for c in &i.comments {
223	println!("\n{} ({}):\n{}", c.author.name, c.timestamp, c.body);	227	println!(
		228	"\n{} ({}){} [{:.8}]:\n{}",
		229	c.author.name,
		230	c.timestamp,
		231	edit_marker(c.edited),
		232	c.commit_id,
		233	body_or_tombstone(&c.body, c.deleted)
		234	);
224	}	235	}
225	}	236	}
226	if !i.linked_commits.is_empty() {	237	if !i.linked_commits.is_empty() {
@@ -300,16 +311,45 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
300	println!("Unassigned '{}'.", name);	311	println!("Unassigned '{}'.", name);
301	Ok(())	312	Ok(())
302	}	313	}
303	IssueCmd::Edit { id, title, body } => {	314	IssueCmd::Edit {
		315	id,
		316	title,
		317	body,
		318	body_file,
		319	} => {
		320	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());
		321	let body = body::resolve_optional(&args)?;
304	issue::edit(repo, &id, title.as_deref(), body.as_deref())?;	322	issue::edit(repo, &id, title.as_deref(), body.as_deref())?;
305	println!("Issue updated.");	323	println!("Issue updated.");
306	Ok(())	324	Ok(())
307	}	325	}
308	IssueCmd::Comment { id, body } => {	326	IssueCmd::Comment {
		327	id,
		328	body,
		329	body_file,
		330	} => {
		331	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());
		332	let body = body::resolve_required(&args, "", "comment")?;
309	issue::comment(repo, &id, &body)?;	333	issue::comment(repo, &id, &body)?;
310	println!("Comment added.");	334	println!("Comment added.");
311	Ok(())	335	Ok(())
312	}	336	}
		337	IssueCmd::EditComment {
		338	id,
		339	comment,
		340	body,
		341	body_file,
		342	} => {
		343	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());
		344	issue::edit_comment(repo, &id, &comment, &args)?;
		345	println!("Comment updated.");
		346	Ok(())
		347	}
		348	IssueCmd::DeleteComment { id, comment } => {
		349	issue::delete_comment(repo, &id, &comment)?;
		350	println!("Comment deleted.");
		351	Ok(())
		352	}
313	IssueCmd::Close { id, reason } => {	353	IssueCmd::Close { id, reason } => {
314	issue::close(repo, &id, reason.as_deref())?;	354	issue::close(repo, &id, reason.as_deref())?;
315	println!("Issue closed.");	355	println!("Issue closed.");
@@ -333,10 +373,13 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
333	PatchCmd::Create {	373	PatchCmd::Create {
334	title,	374	title,
335	body,	375	body,
		376	body_file,
336	base,	377	base,
337	branch,	378	branch,
338	fixes,	379	fixes,
339	} => {	380	} => {
		381	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());
		382	let body = body::resolve_optional(&args)?.unwrap_or_default();
340	// A branch invented here, rather than named by the user, is ours	383	// A branch invented here, rather than named by the user, is ours
341	// to undo: `patch create` validates its arguments and can still	384	// to undo: `patch create` validates its arguments and can still
342	// refuse, and a refusal that left a stray `collab/patch/*`	385	// refuse, and a refusal that left a stray `collab/patch/*`
@@ -530,8 +573,14 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
530	let rev_label =	573	let rev_label =
531	r.revision.map(\|n\| format!(" (r{})", n)).unwrap_or_default();	574	r.revision.map(\|n\| format!(" (r{})", n)).unwrap_or_default();
532	println!(	575	println!(
533	"\n{} ({}) - {}{}:\n{}",	576	"\n{} ({}) - {}{}{} [{:.8}]:\n{}",
534	r.author.name, r.verdict, r.timestamp, rev_label, r.body	577	r.author.name,
		578	r.verdict,
		579	r.timestamp,
		580	rev_label,
		581	edit_marker(r.edited),
		582	r.commit_id,
		583	r.body
535	);	584	);
536	}	585	}
537	}	586	}
@@ -549,8 +598,15 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
549	for c in &inline_comments {	598	for c in &inline_comments {
550	let rev_label = c.revision.map(\|n\| format!(" r{}", n)).unwrap_or_default();	599	let rev_label = c.revision.map(\|n\| format!(" r{}", n)).unwrap_or_default();
551	println!(	600	println!(
552	"\n{} on {}:{} ({}{}):\n {}",	601	"\n{} on {}:{} ({}{}){} [{:.8}]:\n {}",
553	c.author.name, c.file, c.line, c.timestamp, rev_label, c.body	602	c.author.name,
		603	c.file,
		604	c.line,
		605	c.timestamp,
		606	rev_label,
		607	edit_marker(c.edited),
		608	c.commit_id,
		609	body_or_tombstone(&c.body, c.deleted)
554	);	610	);
555	}	611	}
556	}	612	}
@@ -558,7 +614,14 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
558	if !p.comments.is_empty() {	614	if !p.comments.is_empty() {
559	println!("\n--- Comments ---");	615	println!("\n--- Comments ---");
560	for c in &p.comments {	616	for c in &p.comments {
561	println!("\n{} ({}):\n{}", c.author.name, c.timestamp, c.body);	617	println!(
		618	"\n{} ({}){} [{:.8}]:\n{}",
		619	c.author.name,
		620	c.timestamp,
		621	edit_marker(c.edited),
		622	c.commit_id,
		623	body_or_tombstone(&c.body, c.deleted)
		624	);
562	}	625	}
563	}	626	}
564	Ok(())	627	Ok(())
@@ -589,10 +652,13 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
589	PatchCmd::Comment {	652	PatchCmd::Comment {
590	id,	653	id,
591	body,	654	body,
		655	body_file,
592	file,	656	file,
593	line,	657	line,
594	revision,	658	revision,
595	} => {	659	} => {
		660	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());
		661	let body = body::resolve_required(&args, "", "comment")?;
596	patch::comment(repo, &id, &body, file.as_deref(), line, revision)?;	662	patch::comment(repo, &id, &body, file.as_deref(), line, revision)?;
597	println!("Comment added.");	663	println!("Comment added.");
598	Ok(())	664	Ok(())
@@ -601,6 +667,7 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
601	id,	667	id,
602	verdict,	668	verdict,
603	body,	669	body,
		670	body_file,
604	revision,	671	revision,
605	} => {	672	} => {
606	let v: ReviewVerdict = verdict.parse().map_err(\|_\| {	673	let v: ReviewVerdict = verdict.parse().map_err(\|_\| {
@@ -608,15 +675,51 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
608	"verdict must be: approve, request-changes, comment, or reject",	675	"verdict must be: approve, request-changes, comment, or reject",
609	)	676	)
610	})?;	677	})?;
		678	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());
		679	let body = body::resolve_required(&args, "", "review body")?;
611	patch::review(repo, &id, v, &body, revision)?;	680	patch::review(repo, &id, v, &body, revision)?;
612	println!("Review submitted.");	681	println!("Review submitted.");
613	Ok(())	682	Ok(())
614	}	683	}
615	PatchCmd::Revise { id, body, branch } => {	684	PatchCmd::Revise {
		685	id,
		686	body,
		687	body_file,
		688	branch,
		689	} => {
		690	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());
		691	let body = body::resolve_optional(&args)?;
616	patch::revise(repo, &id, body.as_deref(), branch.as_deref())?;	692	patch::revise(repo, &id, body.as_deref(), branch.as_deref())?;
617	println!("Patch revised.");	693	println!("Patch revised.");
618	Ok(())	694	Ok(())
619	}	695	}
		696	PatchCmd::EditComment {
		697	id,
		698	comment,
		699	body,
		700	body_file,
		701	} => {
		702	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());
		703	patch::edit_comment(repo, &id, &comment, &args)?;
		704	println!("Comment updated.");
		705	Ok(())
		706	}
		707	PatchCmd::DeleteComment { id, comment } => {
		708	patch::delete_comment(repo, &id, &comment)?;
		709	println!("Comment deleted.");
		710	Ok(())
		711	}
		712	PatchCmd::EditRevision {
		713	id,
		714	revision,
		715	body,
		716	body_file,
		717	} => {
		718	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());
		719	patch::edit_revision(repo, &id, revision, &args)?;
		720	println!("Revision description updated.");
		721	Ok(())
		722	}
620	PatchCmd::Log { id, json } => {	723	PatchCmd::Log { id, json } => {
621	let p = patch::patch_log(repo, &id)?;	724	let p = patch::patch_log(repo, &id)?;
622	if json {	725	if json {
@@ -955,6 +1058,31 @@ fn search(repo: &Repository, query: &str) -> Result<(), error::Error> {
955	Ok(())	1058	Ok(())
956	}	1059	}
957		1060
		1061	/// What to print in place of a deleted comment's text.
		1062	///
		1063	/// A tombstone, not a blank: the reader has to be able to tell "someone
		1064	/// removed this" from "someone wrote nothing", especially when the comment
		1065	/// below it is a reply.
		1066	pub const TOMBSTONE: &str = "[deleted]";
		1067
		1068	/// Render a body, or the tombstone if the comment was deleted.
		1069	fn body_or_tombstone(body: &str, deleted: bool) -> &str {
		1070	if deleted {
		1071	TOMBSTONE
		1072	} else {
		1073	body
		1074	}
		1075	}
		1076
		1077	/// The suffix marking a body that has been corrected since it was written.
		1078	fn edit_marker(edited: bool) -> &'static str {
		1079	if edited {
		1080	" (edited)"
		1081	} else {
		1082	""
		1083	}
		1084	}
		1085
958	pub(crate) fn truncate_summary(s: &str, max_chars: usize) -> String {	1086	pub(crate) fn truncate_summary(s: &str, max_chars: usize) -> String {
959	let mut out = String::new();	1087	let mut out = String::new();
960	for (count, c) in s.chars().enumerate() {	1088	for (count, c) in s.chars().enumerate() {

src/log.rs

Old		New
@@ -129,6 +129,8 @@ fn action_type_name(action: &Action) -> String {
129	Action::PatchInlineComment { .. } => "PatchInlineComment".to_string(),	129	Action::PatchInlineComment { .. } => "PatchInlineComment".to_string(),
130	Action::PatchClose { .. } => "PatchClose".to_string(),	130	Action::PatchClose { .. } => "PatchClose".to_string(),
131	Action::PatchMerge { .. } => "PatchMerge".to_string(),	131	Action::PatchMerge { .. } => "PatchMerge".to_string(),
		132	Action::BodyEdit { .. } => "BodyEdit".to_string(),
		133	Action::CommentDelete { .. } => "CommentDelete".to_string(),
132	Action::Merge => "Merge".to_string(),	134	Action::Merge => "Merge".to_string(),
133	}	135	}
134	}	136	}
@@ -182,6 +184,13 @@ fn action_summary(action: &Action) -> String {
182	format!("merge {}", &commit[..commit.len().min(7)])	184	format!("merge {}", &commit[..commit.len().min(7)])
183	}	185	}
184	}	186	}
		187	// The log is the audit trail for corrections: it names the event that
		188	// was superseded, so "who changed what, and what did it say before"
		189	// is answerable from `git-collab log` plus the original event object.
		190	Action::BodyEdit { target, body } => {
		191	format!("edit body of {:.8}: {}", target, truncate(body, 40))
		192	}
		193	Action::CommentDelete { target } => format!("delete comment {:.8}", target),
185	Action::Merge => "dag merge".to_string(),	194	Action::Merge => "dag merge".to_string(),
186	}	195	}
187	}	196	}

src/patch.rs

@@ @@ -328,6 +328,132 @@ pub fn show( @@
+    Ok(patch)
+/// Refuse to write a correction to prose somebody else wrote.
+///
+/// This is a courtesy, not the enforcement: the enforcement is in the fold,
+/// which ignores a correction whose author is not the target's author (see
+/// `state::BodyOverrides::authorized`). Anyone can append whatever they like
+/// to a DAG they hold a copy of, so the CLI cannot be the line of defence.
+/// What it can do is tell an honest user *now*, rather than let them write an
+/// event that every reader will silently discard.
+///
+/// The comparison is against the primary email exactly, matching the fold.
+/// Identity aliases deliberately do not widen it: aliases live in local
+/// config, so folding through them would make the same events derive
+/// different state in different clones — and allowing an edit here that the
+/// fold then drops is worse than refusing it, because the user would see
+/// success and no change.
+pub fn require_own_body(
+    repo: &Repository,
+    target: &state::BodyTarget,
+) -> Result<(), crate::error::Error> {
+    let me = get_author(repo)?;
+    if me.email != target.author.email {
+        return Err(Error::Cmd(format!(
+            "this {} was written by {} <{}>; only its author can change it \
+             (you are {} <{}>)",
+            target.kind.label(),
+            target.author.name,
+            target.author.email,
+            me.name,
+            me.email,
+        )));
+    }
+    Ok(())
+}
+/// Refuse to delete something that is not prose.
+pub fn require_deletable(target: &state::BodyTarget) -> Result<(), crate::error::Error> {
+    if !target.kind.is_deletable() {
+        return Err(Error::Cmd(format!(
+            "a {} cannot be deleted: it carries a vote, and removing it would drop that \
+             verdict silently. Submit a new review to change your vote, or edit its body \
+             with `patch edit-comment`.",
+            target.kind.label()
+        )));
+    }
+    Ok(())
+}
+/// Correct the text of a thread comment, an inline comment or a review body.
+///
+/// The three share one ID namespace because they share one identity scheme,
+/// so a user pastes whatever `patch show` printed without having to know
+/// which list it came from.
+pub fn edit_comment(
+    repo: &Repository,
+    id_prefix: &str,
+    comment_prefix: &str,
+    body_args: &crate::body::BodyArgs,
+) -> Result<(), crate::error::Error> {
+    let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
+    let patch = PatchState::from_ref(repo, &ref_name, &id)?;
+    let target = patch.resolve_comment(comment_prefix)?;
+    require_own_body(repo, &target)?;
+    let body = crate::body::resolve_required(body_args, &target.body, target.kind.label())?;
+    dag::append_action(
+        repo,
+        &ref_name,
+        Action::BodyEdit {
+            target: target.oid,
+            body,
+        },
+    )?;
+    Ok(())
+}
+/// Tombstone a comment: drop its text, keep its slot.
+pub fn delete_comment(
+    repo: &Repository,
+    id_prefix: &str,
+    comment_prefix: &str,
+) -> Result<(), crate::error::Error> {
+    let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
+    let patch = PatchState::from_ref(repo, &ref_name, &id)?;
+    let target = patch.resolve_comment(comment_prefix)?;
+    require_own_body(repo, &target)?;
+    require_deletable(&target)?;
+    dag::append_action(
+        repo,
+        &ref_name,
+        Action::CommentDelete {
+            target: target.oid,
+        },
+    )?;
+    Ok(())
+}
+/// Correct a revision's description.
+///
+/// The revision's commit and tree are not touched — those are the content and
+/// are immutable by design. Only the body moves, which is the case
+/// `patch revise` could not fix: a second revise is refused once there are no
+/// new commits, so a placeholder body was permanent.
+pub fn edit_revision(
+    repo: &Repository,
+    id_prefix: &str,
+    revision: u32,
+    body_args: &crate::body::BodyArgs,
+) -> Result<(), crate::error::Error> {
+    let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
+    let patch = PatchState::from_ref(repo, &ref_name, &id)?;
+    let target = patch.resolve_revision(revision)?;
+    require_own_body(repo, &target)?;
+    let body = crate::body::resolve_required(body_args, &target.body, "revision description")?;
+    dag::append_action(
+        repo,
+        &ref_name,
+        Action::BodyEdit {
+            target: target.oid,
+            body,
+        },
+    )?;
+    Ok(())
+}
+pub fn comment(
+    repo: &Repository,
+    id_prefix: &str,

src/server/http/repo/issues.rs

Old		New
@@ -175,15 +175,7 @@ pub async fn issue_detail(
175	labels: is.labels.join(", "),	175	labels: is.labels.join(", "),
176	assignees: is.assignees.join(", "),	176	assignees: is.assignees.join(", "),
177	close_reason: is.close_reason,	177	close_reason: is.close_reason,
178	comments: is	178	comments: is.comments.into_iter().map(CommentView::from_state).collect(),
179	.comments
180	.into_iter()
181	.map(\|c\| CommentView {
182	author: c.author.name,
183	body: c.body,
184	timestamp: c.timestamp,
185	})
186	.collect(),
187	};	179	};
188		180
189	IssueDetailTemplate {	181	IssueDetailTemplate {

src/server/http/repo/mod.rs

Old		New
@@ -126,8 +126,25 @@ fn recent_commits(repo: &git2::Repository, limit: usize) -> Vec<OverviewCommit>
126	#[derive(Debug)]	126	#[derive(Debug)]
127	pub struct CommentView {	127	pub struct CommentView {
128	pub author: String,	128	pub author: String,
		129	/// Empty when `deleted`; the template renders the tombstone instead. The
		130	/// deleted text is dropped in the fold and never reaches this far, so
		131	/// there is nothing here for a template bug to leak.
129	pub body: String,	132	pub body: String,
130	pub timestamp: String,	133	pub timestamp: String,
		134	pub edited: bool,
		135	pub deleted: bool,
		136	}
		137
		138	impl CommentView {
		139	pub fn from_state(c: git_collab::state::Comment) -> Self {
		140	CommentView {
		141	author: c.author.name,
		142	body: c.body,
		143	timestamp: c.timestamp,
		144	edited: c.edited,
		145	deleted: c.deleted,
		146	}
		147	}
131	}	148	}
132		149
133	/// List local branch names, sorted alphabetically.	150	/// List local branch names, sorted alphabetically.

src/server/http/repo/patches.rs

Old		New
@@ -88,6 +88,7 @@ pub struct ReviewView {
88	pub body: String,	88	pub body: String,
89	pub timestamp: String,	89	pub timestamp: String,
90	pub revision: Option<u32>,	90	pub revision: Option<u32>,
		91	pub edited: bool,
91	}	92	}
92		93
93	#[derive(Debug)]	94	#[derive(Debug)]
@@ -98,6 +99,8 @@ pub struct InlineCommentView {
98	pub body: String,	99	pub body: String,
99	pub timestamp: String,	100	pub timestamp: String,
100	pub revision: Option<u32>,	101	pub revision: Option<u32>,
		102	pub edited: bool,
		103	pub deleted: bool,
101	}	104	}
102		105
103	#[derive(Debug)]	106	#[derive(Debug)]
@@ -253,6 +256,7 @@ pub async fn patch_detail(
253	body: r.body,	256	body: r.body,
254	timestamp: r.timestamp,	257	timestamp: r.timestamp,
255	revision: r.revision,	258	revision: r.revision,
		259	edited: r.edited,
256	})	260	})
257	.collect(),	261	.collect(),
258	inline_comments: ps	262	inline_comments: ps
@@ -265,17 +269,11 @@ pub async fn patch_detail(
265	body: ic.body,	269	body: ic.body,
266	timestamp: ic.timestamp,	270	timestamp: ic.timestamp,
267	revision: ic.revision,	271	revision: ic.revision,
		272	edited: ic.edited,
		273	deleted: ic.deleted,
268	})	274	})
269	.collect(),	275	.collect(),
270	comments: ps	276	comments: ps.comments.into_iter().map(CommentView::from_state).collect(),
271	.comments
272	.into_iter()
273	.map(\|c\| CommentView {
274	author: c.author.name,
275	body: c.body,
276	timestamp: c.timestamp,
277	})
278	.collect(),
279	};	277	};
280		278
281	PatchDetailTemplate {	279	PatchDetailTemplate {

src/server/http/templates/issue_detail.html

Old		New
@@ -28,8 +28,13 @@
28	<p style="margin: 0 0 8px 0;">	28	<p style="margin: 0 0 8px 0;">
29	<strong>{{ comment.author }}</strong>	29	<strong>{{ comment.author }}</strong>
30	<span class="mono" style="color: #666; font-size: 0.85em;">{{ comment.timestamp }}</span>	30	<span class="mono" style="color: #666; font-size: 0.85em;">{{ comment.timestamp }}</span>
		31	{% if comment.edited %}  <span style="color: #666; font-size: 0.85em;">(edited)</span>{% endif %}
31	</p>	32	</p>
		33	{% if comment.deleted %}
		34	<p style="margin: 0; color: #666; font-style: italic;">[deleted]</p>
		35	{% else %}
32	<pre style="margin: 0; white-space: pre-wrap;">{{ comment.body }}</pre>	36	<pre style="margin: 0; white-space: pre-wrap;">{{ comment.body }}</pre>
		37	{% endif %}
33	</div>	38	</div>
34	{% endfor %}	39	{% endfor %}
35	{% endif %}	40	{% endif %}

src/server/http/templates/patch_detail.html

Old		New
@@ -54,6 +54,7 @@
54	<span class="status-{{ review.verdict }}">{{ review.verdict }}</span>	54	<span class="status-{{ review.verdict }}">{{ review.verdict }}</span>
55	<span class="mono" style="color: #666; font-size: 0.85em;">{{ review.timestamp }}</span>	55	<span class="mono" style="color: #666; font-size: 0.85em;">{{ review.timestamp }}</span>
56	{% if let Some(rev) = review.revision %}  rev {{ rev }}{% endif %}	56	{% if let Some(rev) = review.revision %}  rev {{ rev }}{% endif %}
		57	{% if review.edited %}  <span style="color: #666; font-size: 0.85em;">(edited)</span>{% endif %}
57	</p>	58	</p>
58	{% if !review.body.is_empty() %}	59	{% if !review.body.is_empty() %}
59	<pre style="margin: 0; white-space: pre-wrap;">{{ review.body }}</pre>	60	<pre style="margin: 0; white-space: pre-wrap;">{{ review.body }}</pre>
@@ -71,8 +72,13 @@
71	<span class="mono" style="color: #666;">{{ ic.file }}:{{ ic.line }}</span>	72	<span class="mono" style="color: #666;">{{ ic.file }}:{{ ic.line }}</span>
72	<span class="mono" style="color: #666; font-size: 0.85em;">{{ ic.timestamp }}</span>	73	<span class="mono" style="color: #666; font-size: 0.85em;">{{ ic.timestamp }}</span>
73	{% if let Some(rev) = ic.revision %}  rev {{ rev }}{% endif %}	74	{% if let Some(rev) = ic.revision %}  rev {{ rev }}{% endif %}
		75	{% if ic.edited %}  <span style="color: #666; font-size: 0.85em;">(edited)</span>{% endif %}
74	</p>	76	</p>
		77	{% if ic.deleted %}
		78	<p style="margin: 0; color: #666; font-style: italic;">[deleted]</p>
		79	{% else %}
75	<pre style="margin: 0; white-space: pre-wrap;">{{ ic.body }}</pre>	80	<pre style="margin: 0; white-space: pre-wrap;">{{ ic.body }}</pre>
		81	{% endif %}
76	</div>	82	</div>
77	{% endfor %}	83	{% endfor %}
78	{% endif %}	84	{% endif %}
@@ -84,8 +90,13 @@
84	<p style="margin: 0 0 8px 0;">	90	<p style="margin: 0 0 8px 0;">
85	<strong>{{ comment.author }}</strong>	91	<strong>{{ comment.author }}</strong>
86	<span class="mono" style="color: #666; font-size: 0.85em;">{{ comment.timestamp }}</span>	92	<span class="mono" style="color: #666; font-size: 0.85em;">{{ comment.timestamp }}</span>
		93	{% if comment.edited %}  <span style="color: #666; font-size: 0.85em;">(edited)</span>{% endif %}
87	</p>	94	</p>
		95	{% if comment.deleted %}
		96	<p style="margin: 0; color: #666; font-style: italic;">[deleted]</p>
		97	{% else %}
88	<pre style="margin: 0; white-space: pre-wrap;">{{ comment.body }}</pre>	98	<pre style="margin: 0; white-space: pre-wrap;">{{ comment.body }}</pre>
		99	{% endif %}
89	</div>	100	</div>
90	{% endfor %}	101	{% endfor %}
91	{% endif %}	102	{% endif %}

src/state.rs

Old		New
@@ -73,6 +73,99 @@ fn deserialize_relates_to<'de, D: serde::Deserializer<'de>>(d: D) -> Result<Vec<
73	}	73	}
74	}	74	}
75		75
		76	/// One `BodyEdit` or `CommentDelete`, pending application.
		77	struct BodyOverride {
		78	/// `(clock, oid_hex)` of the requesting event. The same total order that
		79	/// already decides `status` here: content-derived, so every clone folding
		80	/// the same event set picks the same winner regardless of how its DAG
		81	/// happens to be joined.
		82	key: (u64, String),
		83	/// Who asked. Compared against the target's author before anything is
		84	/// applied.
		85	author_email: String,
		86	/// The replacement body, or `None` for a delete.
		87	body: Option<String>,
		88	}
		89
		90	/// Accumulator for the second pass of the fold: every correction seen, and
		91	/// who owns each thing a correction could name.
		92	///
		93	/// Corrections are applied after the walk rather than during it, which is
		94	/// what makes them independent of walk order. `Sort::TOPOLOGICAL` completes
		95	/// the DAG's partial order with an internal tiebreak, so over a forked DAG
		96	/// two clients can visit the same events in different orders; anything
		97	/// decided by position would then differ between them. Applying afterwards,
		98	/// keyed on `(clock, oid)`, removes the question — an edit can even be
		99	/// visited before the comment it corrects and still land.
		100	#[derive(Default)]
		101	struct BodyOverrides {
		102	/// Winning correction per target event OID.
		103	by_target: HashMap<String, BodyOverride>,
		104	/// Author email of every body-carrying event, by its OID. A correction is
		105	/// only honoured when its author matches the entry here.
		106	owners: HashMap<String, String>,
		107	}
		108
		109	impl BodyOverrides {
		110	/// Register an event that carries a body, so a later correction naming it
		111	/// can be checked against its author.
		112	fn note_owner(&mut self, oid: Oid, author: &Author) {
		113	self.owners.insert(oid.to_string(), author.email.clone());
		114	}
		115
		116	/// Record a correction, keeping the one that wins on `(clock, oid)`.
		117	/// `>=` matches the status fold: later clock wins, and on a tie the
		118	/// lexicographically larger OID does.
		119	fn record(&mut self, target: String, key: (u64, String), author: &Author, body: Option<String>) {
		120	let candidate = BodyOverride {
		121	key,
		122	author_email: author.email.clone(),
		123	body,
		124	};
		125	match self.by_target.get(&target) {
		126	Some(existing) if candidate.key < existing.key => {}
		127	_ => {
		128	self.by_target.insert(target, candidate);
		129	}
		130	}
		131	}
		132
		133	/// The corrections that may actually be applied: those whose author is
		134	/// the author of the event they name.
		135	///
		136	/// Everything else is dropped in silence rather than reported, because
		137	/// the fold has to be total — anyone with a copy of the DAG can append
		138	/// whatever they like to it, and a reader deriving state from a hostile
		139	/// clone must still get an answer. Matching is on the event's declared
		140	/// author email, which is the same basis every other attribution in this
		141	/// codebase uses, and deliberately not on identity aliases: those live in
		142	/// local config, so folding through them would make the same events
		143	/// derive different state in different clones.
		144	fn authorized(&self) -> impl Iterator<Item = (&String, &BodyOverride)> {
		145	self.by_target.iter().filter(\|(target, ov)\| {
		146	self.owners
		147	.get(*target)
		148	.is_some_and(\|owner\| *owner == ov.author_email)
		149	})
		150	}
		151	}
		152
		153	/// Apply a correction to a comment, in place, so it keeps its position among
		154	/// its siblings and everything about it except the words.
		155	fn apply_to_comment(body: &mut String, edited: &mut bool, deleted: &mut bool, ov: &BodyOverride) {
		156	match &ov.body {
		157	Some(new_body) => {
		158	new_body.clone_into(body);
		159	*edited = true;
		160	*deleted = false;
		161	}
		162	None => {
		163	body.clear();
		164	*deleted = true;
		165	}
		166	}
		167	}
		168
76	#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]	169	#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
77	#[serde(rename_all = "lowercase")]	170	#[serde(rename_all = "lowercase")]
78	pub enum IssueStatus {	171	pub enum IssueStatus {
@@ -99,10 +192,23 @@ impl fmt::Display for IssueStatus {
99	#[allow(dead_code)]	192	#[allow(dead_code)]
100	pub struct Comment {	193	pub struct Comment {
101	pub author: Author,	194	pub author: Author,
		195	/// The comment as it currently stands, after any `BodyEdit` that won.
		196	/// Empty when `deleted` — a tombstone drops the text from derived state
		197	/// rather than merely hiding it, so it stops reaching `--json`, `search`
		198	/// and the web UI. The original event is of course still in the DAG.
102	pub body: String,	199	pub body: String,
103	pub timestamp: String,	200	pub timestamp: String,
		201	/// The OID of the event commit that created this comment. This is the
		202	/// comment's identity: what an edit or a delete names, and what `show`
		203	/// prints so a user has something to name.
104	#[serde(serialize_with = "serialize_oid", deserialize_with = "deserialize_oid")]	204	#[serde(serialize_with = "serialize_oid", deserialize_with = "deserialize_oid")]
105	pub commit_id: Oid,	205	pub commit_id: Oid,
		206	/// A later `BodyEdit` by this comment's own author superseded the text.
		207	#[serde(default)]
		208	pub edited: bool,
		209	/// A `CommentDelete` by this comment's own author tombstoned it.
		210	#[serde(default)]
		211	pub deleted: bool,
106	}	212	}
107		213
108	#[derive(Debug, Clone, Serialize, Deserialize)]	214	#[derive(Debug, Clone, Serialize, Deserialize)]
@@ -152,6 +258,23 @@ pub struct Review {
152	pub timestamp: String,	258	pub timestamp: String,
153	#[serde(default)]	259	#[serde(default)]
154	pub revision: Option<u32>,	260	pub revision: Option<u32>,
		261	/// The OID of the `PatchReview` event, so the body can be corrected. A
		262	/// review's verdict is never edited — a reviewer changes their mind by
		263	/// submitting a new review, which supersedes the old vote through the
		264	/// existing per-(author, revision) rule.
		265	///
		266	/// `Oid::zero()` on a `Review` deserialized from a cache entry written
		267	/// before this field existed. Nothing can be addressed by a zero OID, so
		268	/// such a review is simply not editable until the fold cache turns over —
		269	/// which `cache::CACHE_FORMAT_VERSION` forces anyway.
		270	#[serde(
		271	default = "Oid::zero",
		272	serialize_with = "serialize_oid",
		273	deserialize_with = "deserialize_oid"
		274	)]
		275	pub commit_id: Oid,
		276	#[serde(default)]
		277	pub edited: bool,
155	}	278	}
156		279
157	#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]	280	#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
@@ -197,6 +320,28 @@ pub struct Revision {
197	/// these two revisions" is exactly "did this change".	320	/// these two revisions" is exactly "did this change".
198	#[serde(default)]	321	#[serde(default)]
199	pub base: Option<String>,	322	pub base: Option<String>,
		323	/// The OID of the event that recorded this revision — the `PatchRevision`,
		324	/// or the `PatchCreate` for revision 1.
		325	///
		326	/// A revision's commit and tree are immutable by design and stay so. Its
		327	/// body is descriptive metadata, and this is what a correction to that
		328	/// body names. Users still say "r3", never an OID; the CLI resolves the
		329	/// number to this OID, so what goes in the event is content-derived and
		330	/// two clones cannot disagree about it.
		331	#[serde(default)]
		332	pub event_id: String,
		333	/// Who recorded this revision. Not the patch author in general — anyone
		334	/// can push a revision to someone else's patch — and the correction rule
		335	/// is "only the author may edit", so guessing here would refuse honest
		336	/// edits and confuse the CLI's answer with the fold's.
		337	///
		338	/// `None` only for a `Revision` deserialized from a cache entry written
		339	/// before this field existed. An unknown author cannot be matched against
		340	/// anyone, so such a revision is simply not editable — the safe direction.
		341	#[serde(default)]
		342	pub author: Option<Author>,
		343	#[serde(default)]
		344	pub edited: bool,
200	}	345	}
201		346
202	impl Revision {	347	impl Revision {
@@ -223,6 +368,19 @@ pub struct InlineComment {
223	pub timestamp: String,	368	pub timestamp: String,
224	#[serde(default)]	369	#[serde(default)]
225	pub revision: Option<u32>,	370	pub revision: Option<u32>,
		371	/// The OID of the `PatchInlineComment` event — this comment's identity,
		372	/// for the same reason `Comment::commit_id` is. `Oid::zero()` only for a
		373	/// cache entry predating the field; see `Review::commit_id`.
		374	#[serde(
		375	default = "Oid::zero",
		376	serialize_with = "serialize_oid",
		377	deserialize_with = "deserialize_oid"
		378	)]
		379	pub commit_id: Oid,
		380	#[serde(default)]
		381	pub edited: bool,
		382	#[serde(default)]
		383	pub deleted: bool,
226	}	384	}
227		385
228	#[derive(Debug, Clone, Serialize, Deserialize)]	386	#[derive(Debug, Clone, Serialize, Deserialize)]
@@ -322,7 +480,95 @@ impl crate::cli::Listable for PatchState {
322	}	480	}
323	}	481	}
324		482
		483	/// What kind of thing a correction is naming. The distinction only matters
		484	/// for two rules: what the error messages call it, and whether it may be
		485	/// deleted.
		486	#[derive(Debug, Clone, Copy, PartialEq)]
		487	pub enum BodyKind {
		488	Comment,
		489	InlineComment,
		490	Review,
		491	Revision,
		492	}
		493
		494	impl BodyKind {
		495	pub fn label(&self) -> &'static str {
		496	match self {
		497	BodyKind::Comment => "comment",
		498	BodyKind::InlineComment => "inline comment",
		499	BodyKind::Review => "review",
		500	BodyKind::Revision => "revision",
		501	}
		502	}
		503
		504	/// Whether a `CommentDelete` may name this.
		505	///
		506	/// A review carries a vote and a revision carries a commit; dropping
		507	/// either through a body-deletion mechanism would remove something that
		508	/// is not prose. A reviewer changes their mind with a new review, which
		509	/// supersedes the old vote through the rule that already exists.
		510	pub fn is_deletable(&self) -> bool {
		511	matches!(self, BodyKind::Comment \| BodyKind::InlineComment)
		512	}
		513	}
		514
		515	/// A body-carrying event that a `BodyEdit` or `CommentDelete` can name,
		516	/// resolved from a user-supplied ID prefix.
		517	#[derive(Debug, Clone)]
		518	pub struct BodyTarget {
		519	/// Hex OID of the event that carries the body — what goes in the event.
		520	pub oid: String,
		521	pub author: Author,
		522	/// The body as it currently stands, used to seed the editor.
		523	pub body: String,
		524	pub kind: BodyKind,
		525	}
		526
		527	/// Resolve an ID prefix against a set of candidate targets.
		528	///
		529	/// Same contract as resolving a patch or issue prefix: exactly one match, or
		530	/// an error naming the prefix. An ambiguous prefix must never silently pick
		531	/// one — this decides which of someone's comments gets rewritten.
		532	fn resolve_body_target(
		533	candidates: Vec<BodyTarget>,
		534	prefix: &str,
		535	noun: &str,
		536	) -> Result<BodyTarget, crate::error::Error> {
		537	let matches: Vec<BodyTarget> = candidates
		538	.into_iter()
		539	.filter(\|t\| t.oid.starts_with(prefix))
		540	.collect();
		541	match matches.len() {
		542	0 => Err(crate::error::Error::Cmd(format!(
		543	"no {} on this {} matching '{}'",
		544	noun,
		545	if noun == "comment" { "issue" } else { "patch" },
		546	prefix
		547	))),
		548	1 => Ok(matches.into_iter().next().unwrap()),
		549	n => Err(crate::error::Error::Cmd(format!(
		550	"ambiguous {} prefix '{}': {} matches",
		551	noun, prefix, n
		552	))),
		553	}
		554	}
		555
325	impl IssueState {	556	impl IssueState {
		557	/// Resolve a comment ID prefix to the comment it names.
		558	pub fn resolve_comment(&self, prefix: &str) -> Result<BodyTarget, crate::error::Error> {
		559	let candidates = self
		560	.comments
		561	.iter()
		562	.map(\|c\| BodyTarget {
		563	oid: c.commit_id.to_string(),
		564	author: c.author.clone(),
		565	body: c.body.clone(),
		566	kind: BodyKind::Comment,
		567	})
		568	.collect();
		569	resolve_body_target(candidates, prefix, "comment")
		570	}
		571
326	pub fn from_ref(	572	pub fn from_ref(
327	repo: &Repository,	573	repo: &Repository,
328	ref_name: &str,	574	ref_name: &str,
@@ -365,6 +611,10 @@ impl IssueState {
365	// that are reconciled via merge commits, so we sort explicitly.	611	// that are reconciled via merge commits, so we sort explicitly.
366	let mut link_acc: HashMap<String, ((u64, String, String), LinkedCommit)> = HashMap::new();	612	let mut link_acc: HashMap<String, ((u64, String, String), LinkedCommit)> = HashMap::new();
367		613
		614	// Corrections to comment bodies, applied after the walk. See
		615	// `BodyOverrides`.
		616	let mut overrides = BodyOverrides::default();
		617
368	for (oid, event) in events {	618	for (oid, event) in events {
369	let ts = parse_timestamp(&event.timestamp);	619	let ts = parse_timestamp(&event.timestamp);
370	if latest.as_ref().is_none_or(\|(prev, _)\| ts > *prev) {	620	if latest.as_ref().is_none_or(\|(prev, _)\| ts > *prev) {
@@ -397,14 +647,37 @@ impl IssueState {
397	}	647	}
398	Action::IssueComment { body } => {	648	Action::IssueComment { body } => {
399	if let Some(ref mut s) = state {	649	if let Some(ref mut s) = state {
		650	overrides.note_owner(oid, &event.author);
400	s.comments.push(Comment {	651	s.comments.push(Comment {
401	author: event.author.clone(),	652	author: event.author.clone(),
402	body,	653	body,
403	timestamp: event.timestamp.clone(),	654	timestamp: event.timestamp.clone(),
404	commit_id: oid,	655	commit_id: oid,
		656	edited: false,
		657	deleted: false,
405	});	658	});
406	}	659	}
407	}	660	}
		661	Action::BodyEdit { target, body } => {
		662	if state.is_some() {
		663	overrides.record(
		664	target,
		665	(event.clock, oid.to_string()),
		666	&event.author,
		667	Some(body),
		668	);
		669	}
		670	}
		671	Action::CommentDelete { target } => {
		672	if state.is_some() {
		673	overrides.record(
		674	target,
		675	(event.clock, oid.to_string()),
		676	&event.author,
		677	None,
		678	);
		679	}
		680	}
408	Action::IssueClose { reason } => {	681	Action::IssueClose { reason } => {
409	if let Some(ref mut s) = state {	682	if let Some(ref mut s) = state {
410	let key = (event.clock, oid.to_string());	683	let key = (event.clock, oid.to_string());
@@ -501,6 +774,19 @@ impl IssueState {
501	}	774	}
502		775
503	if let Some(ref mut s) = state {	776	if let Some(ref mut s) = state {
		777	// Second pass: apply the corrections collected above. Order is
		778	// irrelevant — each names a distinct comment — so the randomized
		779	// HashMap iteration order cannot make the result nondeterministic.
		780	for (target, ov) in overrides.authorized() {
		781	if let Some(c) = s
		782	.comments
		783	.iter_mut()
		784	.find(\|c\| c.commit_id.to_string() == *target)
		785	{
		786	apply_to_comment(&mut c.body, &mut c.edited, &mut c.deleted, ov);
		787	}
		788	}
		789
504	s.last_updated = latest.map(\|(_, raw)\| raw).unwrap_or_default();	790	s.last_updated = latest.map(\|(_, raw)\| raw).unwrap_or_default();
505		791
506	// Flush the linked-commit accumulator into state.linked_commits in	792	// Flush the linked-commit accumulator into state.linked_commits in
@@ -517,6 +803,70 @@ impl IssueState {
517	}	803	}
518		804
519	impl PatchState {	805	impl PatchState {
		806	/// Resolve a comment/review ID prefix against everything on this patch
		807	/// that carries an editable body.
		808	///
		809	/// Thread comments, inline comments and reviews share one namespace
		810	/// because they share one identity scheme — the OID of the event that
		811	/// created them — so a user never has to know which of the three lists
		812	/// the ID they copied out of `patch show` came from.
		813	pub fn resolve_comment(&self, prefix: &str) -> Result<BodyTarget, crate::error::Error> {
		814	let mut candidates: Vec<BodyTarget> = Vec::new();
		815	candidates.extend(self.comments.iter().map(\|c\| BodyTarget {
		816	oid: c.commit_id.to_string(),
		817	author: c.author.clone(),
		818	body: c.body.clone(),
		819	kind: BodyKind::Comment,
		820	}));
		821	candidates.extend(self.inline_comments.iter().map(\|c\| BodyTarget {
		822	oid: c.commit_id.to_string(),
		823	author: c.author.clone(),
		824	body: c.body.clone(),
		825	kind: BodyKind::InlineComment,
		826	}));
		827	candidates.extend(self.reviews.iter().map(\|r\| BodyTarget {
		828	oid: r.commit_id.to_string(),
		829	author: r.author.clone(),
		830	body: r.body.clone(),
		831	kind: BodyKind::Review,
		832	}));
		833	resolve_body_target(candidates, prefix, "comment or review")
		834	}
		835
		836	/// Resolve a revision number — what users say — to the event that
		837	/// recorded it, which is what a correction names.
		838	pub fn resolve_revision(&self, number: u32) -> Result<BodyTarget, crate::error::Error> {
		839	let rev = self
		840	.revisions
		841	.iter()
		842	.find(\|r\| r.number == number)
		843	.ok_or_else(\|\| {
		844	crate::error::Error::Cmd(format!(
		845	"revision {} not found (this patch has {})",
		846	number,
		847	self.revisions.len()
		848	))
		849	})?;
		850	if rev.event_id.is_empty() {
		851	return Err(crate::error::Error::Cmd(format!(
		852	"revision {} predates revision identity and cannot be corrected",
		853	number
		854	)));
		855	}
		856	let author = rev.author.clone().ok_or_else(\|\| {
		857	crate::error::Error::Cmd(format!(
		858	"revision {} records no author and cannot be corrected",
		859	number
		860	))
		861	})?;
		862	Ok(BodyTarget {
		863	oid: rev.event_id.clone(),
		864	author,
		865	body: rev.body.clone().unwrap_or_default(),
		866	kind: BodyKind::Revision,
		867	})
		868	}
		869
520	/// The commit the patch currently stands at: the latest revision whose	870	/// The commit the patch currently stands at: the latest revision whose
521	/// commit was recorded and whose objects are still present. A revision ref	871	/// commit was recorded and whose objects are still present. A revision ref
522	/// keeps those objects reachable, so this no longer depends on any	872	/// keeps those objects reachable, so this no longer depends on any
@@ -698,6 +1048,10 @@ impl PatchState {
698		1048
699	let mut status_key: Option<(u64, String)> = None;	1049	let mut status_key: Option<(u64, String)> = None;
700		1050
		1051	// Corrections to comment, review and revision bodies, applied after
		1052	// the walk. See `BodyOverrides`.
		1053	let mut overrides = BodyOverrides::default();
		1054
701	for (oid, event) in events {	1055	for (oid, event) in events {
702	let ts = parse_timestamp(&event.timestamp);	1056	let ts = parse_timestamp(&event.timestamp);
703	if latest.as_ref().is_none_or(\|(prev, _)\| ts > *prev) {	1057	if latest.as_ref().is_none_or(\|(prev, _)\| ts > *prev) {
@@ -714,6 +1068,7 @@ impl PatchState {
714	tree,	1068	tree,
715	base_commit,	1069	base_commit,
716	} => {	1070	} => {
		1071	overrides.note_owner(oid, &event.author);
717	let revisions = vec![Revision {	1072	let revisions = vec![Revision {
718	number: 1,	1073	number: 1,
719	commit: commit.clone(),	1074	commit: commit.clone(),
@@ -721,6 +1076,9 @@ impl PatchState {
721	body: None,	1076	body: None,
722	timestamp: event.timestamp.clone(),	1077	timestamp: event.timestamp.clone(),
723	base: base_commit,	1078	base: base_commit,
		1079	event_id: oid.to_string(),
		1080	author: Some(event.author.clone()),
		1081	edited: false,
724	}];	1082	}];
725	state = Some(PatchState {	1083	state = Some(PatchState {
726	id: id.to_string(),	1084	id: id.to_string(),
@@ -767,6 +1125,7 @@ impl PatchState {
767	let already_seen =	1125	let already_seen =
768	!commit.is_empty() && s.revisions.iter().any(\|r\| r.commit == commit);	1126	!commit.is_empty() && s.revisions.iter().any(\|r\| r.commit == commit);
769	if !already_seen {	1127	if !already_seen {
		1128	overrides.note_owner(oid, &event.author);
770	let number = s.revisions.len() as u32 + 1;	1129	let number = s.revisions.len() as u32 + 1;
771	s.revisions.push(Revision {	1130	s.revisions.push(Revision {
772	number,	1131	number,
@@ -775,6 +1134,9 @@ impl PatchState {
775	body,	1134	body,
776	timestamp: event.timestamp.clone(),	1135	timestamp: event.timestamp.clone(),
777	base,	1136	base,
		1137	event_id: oid.to_string(),
		1138	author: Some(event.author.clone()),
		1139	edited: false,
778	});	1140	});
779	}	1141	}
780	}	1142	}
@@ -831,22 +1193,28 @@ impl PatchState {
831	&& r.revision == Some(revision))	1193	&& r.revision == Some(revision))
832	});	1194	});
833	}	1195	}
		1196	overrides.note_owner(oid, &event.author);
834	s.reviews.push(Review {	1197	s.reviews.push(Review {
835	author: event.author.clone(),	1198	author: event.author.clone(),
836	verdict,	1199	verdict,
837	body,	1200	body,
838	timestamp: event.timestamp.clone(),	1201	timestamp: event.timestamp.clone(),
839	revision: Some(revision),	1202	revision: Some(revision),
		1203	commit_id: oid,
		1204	edited: false,
840	});	1205	});
841	}	1206	}
842	}	1207	}
843	Action::PatchComment { body } => {	1208	Action::PatchComment { body } => {
844	if let Some(ref mut s) = state {	1209	if let Some(ref mut s) = state {
		1210	overrides.note_owner(oid, &event.author);
845	s.comments.push(Comment {	1211	s.comments.push(Comment {
846	author: event.author.clone(),	1212	author: event.author.clone(),
847	body,	1213	body,
848	timestamp: event.timestamp.clone(),	1214	timestamp: event.timestamp.clone(),
849	commit_id: oid,	1215	commit_id: oid,
		1216	edited: false,
		1217	deleted: false,
850	});	1218	});
851	}	1219	}
852	}	1220	}
@@ -857,6 +1225,7 @@ impl PatchState {
857	revision,	1225	revision,
858	} => {	1226	} => {
859	if let Some(ref mut s) = state {	1227	if let Some(ref mut s) = state {
		1228	overrides.note_owner(oid, &event.author);
860	s.inline_comments.push(InlineComment {	1229	s.inline_comments.push(InlineComment {
861	author: event.author.clone(),	1230	author: event.author.clone(),
862	file,	1231	file,
@@ -864,9 +1233,32 @@ impl PatchState {
864	body,	1233	body,
865	timestamp: event.timestamp.clone(),	1234	timestamp: event.timestamp.clone(),
866	revision: Some(revision),	1235	revision: Some(revision),
		1236	commit_id: oid,
		1237	edited: false,
		1238	deleted: false,
867	});	1239	});
868	}	1240	}
869	}	1241	}
		1242	Action::BodyEdit { target, body } => {
		1243	if state.is_some() {
		1244	overrides.record(
		1245	target,
		1246	(event.clock, oid.to_string()),
		1247	&event.author,
		1248	Some(body),
		1249	);
		1250	}
		1251	}
		1252	Action::CommentDelete { target } => {
		1253	if state.is_some() {
		1254	overrides.record(
		1255	target,
		1256	(event.clock, oid.to_string()),
		1257	&event.author,
		1258	None,
		1259	);
		1260	}
		1261	}
870	Action::PatchClose { .. } => {	1262	Action::PatchClose { .. } => {
871	if let Some(ref mut s) = state {	1263	if let Some(ref mut s) = state {
872	let key = (event.clock, oid.to_string());	1264	let key = (event.clock, oid.to_string());
@@ -906,6 +1298,51 @@ impl PatchState {
906	}	1298	}
907		1299
908	if let Some(ref mut s) = state {	1300	if let Some(ref mut s) = state {
		1301	// Second pass: apply corrections. A target names exactly one of
		1302	// these lists, so the order they are visited in cannot matter.
		1303	// A `None` body (a delete) only ever reaches a comment: the CLI
		1304	// refuses to emit one for a review or a revision, and a forged
		1305	// one finds no match here because the search order stops at the
		1306	// comment lists — see `Action::CommentDelete`.
		1307	for (target, ov) in overrides.authorized() {
		1308	if let Some(c) = s
		1309	.comments
		1310	.iter_mut()
		1311	.find(\|c\| c.commit_id.to_string() == *target)
		1312	{
		1313	apply_to_comment(&mut c.body, &mut c.edited, &mut c.deleted, ov);
		1314	} else if let Some(c) = s
		1315	.inline_comments
		1316	.iter_mut()
		1317	.find(\|c\| c.commit_id.to_string() == *target)
		1318	{
		1319	apply_to_comment(&mut c.body, &mut c.edited, &mut c.deleted, ov);
		1320	} else if let Some(r) = s
		1321	.reviews
		1322	.iter_mut()
		1323	.find(\|r\| r.commit_id.to_string() == *target)
		1324	{
		1325	// A review's vote is untouchable; only its prose moves.
		1326	// A delete is not honoured here, so a forged
		1327	// `CommentDelete` cannot drop a verdict.
		1328	if let Some(new_body) = &ov.body {
		1329	new_body.clone_into(&mut r.body);
		1330	r.edited = true;
		1331	}
		1332	} else if let Some(rev) = s
		1333	.revisions
		1334	.iter_mut()
		1335	.find(\|rev\| rev.event_id == *target)
		1336	{
		1337	// Same rule: the commit and tree a revision points at are
		1338	// immutable, the description is not.
		1339	if let Some(new_body) = &ov.body {
		1340	rev.body = Some(new_body.clone());
		1341	rev.edited = true;
		1342	}
		1343	}
		1344	}
		1345
909	s.last_updated = latest.map(\|(_, raw)\| raw).unwrap_or_default();	1346	s.last_updated = latest.map(\|(_, raw)\| raw).unwrap_or_default();
910	}	1347	}
911	state.ok_or_else(\|\| git2::Error::from_str("no PatchCreate event found in DAG").into())	1348	state.ok_or_else(\|\| git2::Error::from_str("no PatchCreate event found in DAG").into())

src/tui/mod.rs

Old		New
@@ -1043,6 +1043,8 @@ mod tests {
1043	body: "Thread comment".into(),	1043	body: "Thread comment".into(),
1044	timestamp: "2026-01-05T00:00:00Z".into(),	1044	timestamp: "2026-01-05T00:00:00Z".into(),
1045	commit_id: Oid::from_str("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").unwrap(),	1045	commit_id: Oid::from_str("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").unwrap(),
		1046	edited: false,
		1047	deleted: false,
1046	}],	1048	}],
1047	inline_comments: vec![crate::state::InlineComment {	1049	inline_comments: vec![crate::state::InlineComment {
1048	author: make_author(),	1050	author: make_author(),
@@ -1051,6 +1053,9 @@ mod tests {
1051	body: "Nit: rename this".into(),	1053	body: "Nit: rename this".into(),
1052	timestamp: "2026-01-03T00:00:00Z".into(),	1054	timestamp: "2026-01-03T00:00:00Z".into(),
1053	revision: Some(1),	1055	revision: Some(1),
		1056	commit_id: Oid::from_str("cccccccccccccccccccccccccccccccccccccccc").unwrap(),
		1057	edited: false,
		1058	deleted: false,
1054	}],	1059	}],
1055	reviews: vec![crate::state::Review {	1060	reviews: vec![crate::state::Review {
1056	author: make_author(),	1061	author: make_author(),
@@ -1058,6 +1063,8 @@ mod tests {
1058	body: "LGTM".into(),	1063	body: "LGTM".into(),
1059	timestamp: "2026-01-04T00:00:00Z".into(),	1064	timestamp: "2026-01-04T00:00:00Z".into(),
1060	revision: Some(2),	1065	revision: Some(2),
		1066	commit_id: Oid::from_str("dddddddddddddddddddddddddddddddddddddddd").unwrap(),
		1067	edited: false,
1061	}],	1068	}],
1062	revisions: vec![	1069	revisions: vec![
1063	Revision {	1070	Revision {
@@ -1067,6 +1074,9 @@ mod tests {
1067	body: None,	1074	body: None,
1068	timestamp: "2026-01-01T00:00:00Z".into(),	1075	timestamp: "2026-01-01T00:00:00Z".into(),
1069	base: None,	1076	base: None,
		1077	event_id: "eeee1111eeee1111eeee1111eeee1111eeee1111".into(),
		1078	author: Some(make_author()),
		1079	edited: false,
1070	},	1080	},
1071	Revision {	1081	Revision {
1072	number: 2,	1082	number: 2,
@@ -1075,6 +1085,9 @@ mod tests {
1075	body: Some("Addressed review comments".into()),	1085	body: Some("Addressed review comments".into()),
1076	timestamp: "2026-01-02T00:00:00Z".into(),	1086	timestamp: "2026-01-02T00:00:00Z".into(),
1077	base: None,	1087	base: None,
		1088	event_id: "eeee2222eeee2222eeee2222eeee2222eeee2222".into(),
		1089	author: Some(make_author()),
		1090	edited: false,
1078	},	1091	},
1079	],	1092	],
1080	created_at: "2026-01-01T00:00:00Z".into(),	1093	created_at: "2026-01-01T00:00:00Z".into(),

src/tui/widgets.rs

Old		New
@@ -7,6 +7,27 @@ use crate::state::{IssueState, IssueStatus, PatchState, PatchStatus};
7		7
8	use super::state::{App, InputMode, ListMode, Pane, StatusFilter, ViewMode};	8	use super::state::{App, InputMode, ListMode, Pane, StatusFilter, ViewMode};
9		9
		10	/// The lines to render for a comment body: the text as it stands, or the
		11	/// tombstone when it was deleted.
		12	///
		13	/// A deleted comment's body is empty, so rendering it directly would print
		14	/// the author and timestamp above nothing at all — indistinguishable from a
		15	/// comment that said nothing, and unreadable as a reply's antecedent.
		16	fn body_lines(body: &str, deleted: bool) -> Vec<String> {
		17	if deleted {
		18	return vec![crate::TOMBSTONE.to_string()];
		19	}
		20	body.lines().map(\|l\| l.to_string()).collect()
		21	}
		22
		23	/// Trailing marker for a body that has been corrected since it was written.
		24	fn edited_span(edited: bool) -> Span<'static> {
		25	Span::styled(
		26	if edited { " (edited)" } else { "" },
		27	Style::default().fg(Color::DarkGray),
		28	)
		29	}
		30
10	pub(crate) fn action_type_label(action: &Action) -> &str {	31	pub(crate) fn action_type_label(action: &Action) -> &str {
11	match action {	32	match action {
12	Action::IssueOpen { .. } => "Issue Open",	33	Action::IssueOpen { .. } => "Issue Open",
@@ -31,6 +52,8 @@ pub(crate) fn action_type_label(action: &Action) -> &str {
31	Action::IssueAssign { .. } => "Issue Assign",	52	Action::IssueAssign { .. } => "Issue Assign",
32	Action::IssueUnassign { .. } => "Issue Unassign",	53	Action::IssueUnassign { .. } => "Issue Unassign",
33	Action::IssueCommitLink { .. } => "Issue Commit Link",	54	Action::IssueCommitLink { .. } => "Issue Commit Link",
		55	Action::BodyEdit { .. } => "Body Edit",
		56	Action::CommentDelete { .. } => "Comment Delete",
34	}	57	}
35	}	58	}
36		59
@@ -129,6 +152,13 @@ pub(crate) fn format_event_detail(oid: &Oid, event: &crate::event::Event) -> Str
129	Action::IssueCommitLink { commit } => {	152	Action::IssueCommitLink { commit } => {
130	detail.push_str(&format!("\nCommit: {}\n", commit));	153	detail.push_str(&format!("\nCommit: {}\n", commit));
131	}	154	}
		155	Action::BodyEdit { target, body } => {
		156	detail.push_str(&format!("\nSupersedes: {:.8}\n", target));
		157	detail.push_str(&format!("\n{}\n", body));
		158	}
		159	Action::CommentDelete { target } => {
		160	detail.push_str(&format!("\nDeleted: {:.8}\n", target));
		161	}
132	Action::PatchMerge { commit } => {	162	Action::PatchMerge { commit } => {
133	if !commit.is_empty() {	163	if !commit.is_empty() {
134	detail.push_str(&format!("\nMerged as: {}\n", commit));	164	detail.push_str(&format!("\nMerged as: {}\n", commit));
@@ -472,8 +502,9 @@ fn build_issue_detail(
472	format!(" ({})", c.timestamp),	502	format!(" ({})", c.timestamp),
473	Style::default().fg(Color::DarkGray),	503	Style::default().fg(Color::DarkGray),
474	),	504	),
		505	edited_span(c.edited),
475	]));	506	]));
476	for l in c.body.lines() {	507	for l in body_lines(&c.body, c.deleted) {
477	lines.push(Line::raw(format!(" {}", l)));	508	lines.push(Line::raw(format!(" {}", l)));
478	}	509	}
479	}	510	}
@@ -716,6 +747,7 @@ fn build_patch_detail_text(app: &App) -> Text<'static> {
716	Span::raw(format!(" {} ", review.timestamp)),	747	Span::raw(format!(" {} ", review.timestamp)),
717	Span::styled(verdict_str, Style::default().fg(verdict_color)),	748	Span::styled(verdict_str, Style::default().fg(verdict_color)),
718	Span::raw(rev_label),	749	Span::raw(rev_label),
		750	edited_span(review.edited),
719	]));	751	]));
720	if !review.body.is_empty() {	752	if !review.body.is_empty() {
721	for l in review.body.lines() {	753	for l in review.body.lines() {
@@ -746,8 +778,9 @@ fn build_patch_detail_text(app: &App) -> Text<'static> {
746	),	778	),
747	Span::raw(format!(" {}:{}", ic.file, ic.line)),	779	Span::raw(format!(" {}:{}", ic.file, ic.line)),
748	Span::raw(rev_label),	780	Span::raw(rev_label),
		781	edited_span(ic.edited),
749	]));	782	]));
750	for l in ic.body.lines() {	783	for l in body_lines(&ic.body, ic.deleted) {
751	lines.push(Line::raw(format!(" {}", l)));	784	lines.push(Line::raw(format!(" {}", l)));
752	}	785	}
753	}	786	}
@@ -773,8 +806,9 @@ fn build_patch_detail_text(app: &App) -> Text<'static> {
773	format!(" ({})", c.timestamp),	806	format!(" ({})", c.timestamp),
774	Style::default().fg(Color::DarkGray),	807	Style::default().fg(Color::DarkGray),
775	),	808	),
		809	edited_span(c.edited),
776	]));	810	]));
777	for l in c.body.lines() {	811	for l in body_lines(&c.body, c.deleted) {
778	lines.push(Line::raw(format!(" {}", l)));	812	lines.push(Line::raw(format!(" {}", l)));
779	}	813	}
780	}	814	}

tests/body_edit_test.rs

@@ @@ -0,0 +1,860 @@ @@
+//! Correcting prose that is already in the DAG.
+//!
+//! Events are append-only and stay that way: an edit is a new event that
+//! supersedes an earlier one's body, exactly as `IssueEdit` already
+//! supersedes an issue's title and body. The same single mechanism covers
+//! thread comments, inline comments, review bodies and revision
+//! descriptions, because they are all just a body hanging off an event OID.
+//!
+//! Three properties are load-bearing and each is tested here:
+//!
+//! * **Attribution.** An edit signed by a different key must not silently
+//!   rewrite someone else's words. The fold ignores it.
+//! * **Ordering.** An edited comment stays where it was, so a reply above or
+//!   below it still reads in sequence.
+//! * **Convergence.** Two clones editing the same comment offline settle on
+//!   the same text, decided by `(clock, oid)` like every other conflict here.
+mod common;
+use common::{alice, bob, init_repo, test_signing_key, TestRepo};
+use git2::Repository;
+use git_collab::dag;
+use git_collab::event::{Action, Author, Event, ReviewVerdict};
+use git_collab::state::{IssueState, PatchState};
+use tempfile::TempDir;
+// ===========================================================================
+// Helpers for driving the DAG directly (used by the attribution and
+// convergence tests, which need to forge events from a second identity).
+// ===========================================================================
+fn append(repo: &Repository, ref_name: &str, author: &Author, action: Action) -> git2::Oid {
+    let sk = test_signing_key();
+    let event = Event {
+        timestamp: common::now(),
+        author: author.clone(),
+        action,
+        clock: 0,
+    };
+    dag::append_event(repo, ref_name, &event, &sk).unwrap()
+}
+fn open_issue_with_comment(repo: &Repository) -> (String, String, git2::Oid) {
+    let (ref_name, id) = common::open_issue(repo, &alice(), "issue under test");
+    let comment_oid = append(
+        repo,
+        &ref_name,
+        &alice(),
+        Action::IssueComment {
+            body: "original text".to_string(),
+        },
+    );
+    (ref_name, id, comment_oid)
+}
+fn issue_state(repo: &Repository, ref_name: &str, id: &str) -> IssueState {
+    IssueState::from_ref_uncached(repo, ref_name, id).unwrap()
+}
+/// Pull the short comment id `patch show` / `issue show` prints, for the
+/// first comment listed.
+fn first_comment_id(json: &str, list: &str) -> String {
+    let value: serde_json::Value = serde_json::from_str(json).expect("valid JSON");
+    value[list][0]["commit_id"]
+        .as_str()
+        .unwrap_or_else(|| panic!("no commit_id on {}[0] in {}", list, json))
+        .to_string()
+}
+fn body_at(json: &str, pointer: &str) -> String {
+    let value: serde_json::Value = serde_json::from_str(json).expect("valid JSON");
+    value
+        .pointer(pointer)
+        .unwrap_or_else(|| panic!("no {} in {}", pointer, json))
+        .as_str()
+        .expect("string")
+        .to_string()
+}
+// ===========================================================================
+// Editing, end to end
+// ===========================================================================
+#[test]
+fn an_issue_comment_can_be_corrected() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.issue_open("typo");
+    repo.run_ok(&["issue", "comment", &id, "-b", "teh original"]);
+    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
+    let comment_id = first_comment_id(&json, "comments");
+    repo.run_ok(&[
+        "issue",
+        "edit-comment",
+        &id,
+        &comment_id[..8],
+        "-b",
+        "the original",
+    ]);
+    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
+    assert_eq!(body_at(&json, "/comments/0/body"), "the original");
+    assert_eq!(
+        body_at(&json, "/comments/0/author/email"),
+        "alice@example.com",
+        "an edit must not change who wrote the comment"
+    );
+}
+#[test]
+fn a_patch_thread_comment_can_be_corrected() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.patch_create("thread edit");
+    repo.run_ok(&["patch", "comment", &id, "-b", "wrong"]);
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    let comment_id = first_comment_id(&json, "comments");
+    repo.run_ok(&["patch", "edit-comment", &id, &comment_id[..8], "-b", "right"]);
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    assert_eq!(body_at(&json, "/comments/0/body"), "right");
+}
+#[test]
+fn an_inline_comment_can_be_corrected_without_moving() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.patch_create("inline edit");
+    repo.run_ok(&[
+        "patch", "comment", &id, "--file", "a.rs", "--line", "7", "-b", "wrong",
+    ]);
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    let comment_id = first_comment_id(&json, "inline_comments");
+    repo.run_ok(&["patch", "edit-comment", &id, &comment_id[..8], "-b", "right"]);
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    assert_eq!(body_at(&json, "/inline_comments/0/body"), "right");
+    assert_eq!(
+        body_at(&json, "/inline_comments/0/file"),
+        "a.rs",
+        "an edit touches the body and nothing else"
+    );
+    let value: serde_json::Value = serde_json::from_str(&json).unwrap();
+    assert_eq!(value["inline_comments"][0]["line"], 7);
+}
+/// The case the issue was actually filed about: a typo in a review body.
+#[test]
+fn a_review_body_can_be_corrected_without_changing_the_verdict() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.patch_create("review edit");
+    repo.run_ok(&[
+        "patch",
+        "review",
+        &id,
+        "-v",
+        "request-changes",
+        "-b",
+        "probe comment, please ignore",
+    ]);
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    let review_id = first_comment_id(&json, "reviews");
+    repo.run_ok(&[
+        "patch",
+        "edit-comment",
+        &id,
+        &review_id[..8],
+        "-b",
+        "the real review",
+    ]);
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    assert_eq!(body_at(&json, "/reviews/0/body"), "the real review");
+    assert_eq!(
+        body_at(&json, "/reviews/0/verdict"),
+        "request-changes",
+        "editing the prose must not disturb the vote"
+    );
+}
+#[test]
+fn an_edit_reads_its_body_from_a_file_or_stdin_too() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.patch_create("edit from stdin");
+    repo.run_ok(&["patch", "comment", &id, "-b", "wrong"]);
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    let comment_id = first_comment_id(&json, "comments");
+    let corrected = "corrected `body` with $vars\n\n";
+    repo.run_stdin_ok(
+        &["patch", "edit-comment", &id, &comment_id[..8], "-F", "-"],
+        corrected.as_bytes(),
+    );
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    assert_eq!(body_at(&json, "/comments/0/body"), corrected);
+}
+/// Editing with no body given opens the editor seeded with the text as it
+/// stands, which is what makes fixing a typo a matter of one keystroke.
+#[test]
+fn editing_with_no_body_opens_the_editor_seeded_with_the_current_text() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.issue_open("seeded editor");
+    repo.run_ok(&["issue", "comment", &id, "-b", "seed me"]);
+    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
+    let comment_id = first_comment_id(&json, "comments");
+    // Echo back what the editor was handed, with a suffix, so the test can
+    // prove the buffer arrived pre-filled.
+    let editor = repo.write_script(
+        "seed-editor.sh",
+        "#!/bin/sh\nprintf '%s and more' \"$(cat \"$1\")\" > \"$1\"\n",
+    );
+    repo.run_in_pty(
+        &["issue", "edit-comment", &id, &comment_id[..8]],
+        &editor,
+    );
+    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
+    assert_eq!(body_at(&json, "/comments/0/body"), "seed me and more");
+}
+#[test]
+fn editing_an_unknown_comment_is_an_error() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.patch_create("unknown target");
+    let err = repo.run_err(&["patch", "edit-comment", &id, "deadbeef", "-b", "x"]);
+    assert!(
+        err.contains("deadbeef"),
+        "error should name the id that matched nothing: {}",
+        err
+    );
+}
+// ===========================================================================
+// Deleting: a tombstone, not a disappearance
+// ===========================================================================
+/// A deleted comment leaves a tombstone. Others may have replied to it, the
+/// event is still in the DAG either way, and a comment that silently
+/// vanishes makes every reply to it reference nothing.
+#[test]
+fn a_deleted_comment_leaves_a_tombstone_in_place() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.patch_create("tombstone");
+    repo.run_ok(&["patch", "comment", &id, "-b", "first"]);
+    repo.run_ok(&["patch", "comment", &id, "-b", "regrettable probe"]);
+    repo.run_ok(&["patch", "comment", &id, "-b", "third, replying to the above"]);
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    let value: serde_json::Value = serde_json::from_str(&json).unwrap();
+    let target = value["comments"][1]["commit_id"].as_str().unwrap().to_string();
+    repo.run_ok(&["patch", "delete-comment", &id, &target[..8]]);
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    let value: serde_json::Value = serde_json::from_str(&json).unwrap();
+    let comments = value["comments"].as_array().unwrap();
+    assert_eq!(
+        comments.len(),
+,
+        "the tombstone keeps the comment's slot so replies still line up"
+    );
+    assert_eq!(comments[0]["body"], "first");
+    assert_eq!(comments[2]["body"], "third, replying to the above");
+    assert_eq!(
+        comments[1]["deleted"], true,
+        "the middle comment is marked deleted"
+    );
+    assert_eq!(
+        comments[1]["body"], "",
+        "the deleted text must be gone from derived state, not merely hidden"
+    );
+    assert_eq!(
+        comments[1]["author"]["email"], "alice@example.com",
+        "a tombstone still says who left the comment"
+    );
+}
+#[test]
+fn a_deleted_comment_is_shown_as_deleted_not_as_blank() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.patch_create("tombstone display");
+    repo.run_ok(&["patch", "comment", &id, "-b", "regrettable"]);
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    let target = first_comment_id(&json, "comments");
+    repo.run_ok(&["patch", "delete-comment", &id, &target[..8]]);
+    let out = repo.run_ok(&["patch", "show", &id]);
+    assert!(
+        out.contains("[deleted]"),
+        "a deleted comment should read as deleted: {}",
+        out
+    );
+    assert!(
+        !out.contains("regrettable"),
+        "the deleted text must not still be rendered: {}",
+        out
+    );
+}
+/// Deleted text must not leak out through the side doors either.
+#[test]
+fn a_deleted_comment_stops_matching_search() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.patch_create("search leak");
+    repo.run_ok(&["patch", "comment", &id, "-b", "chartreuse blunder"]);
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    let target = first_comment_id(&json, "comments");
+    let out = repo.run_ok(&["search", "chartreuse"]);
+    assert!(out.contains("comment match"), "precondition: {}", out);
+    repo.run_ok(&["patch", "delete-comment", &id, &target[..8]]);
+    let out = repo.run_ok(&["search", "chartreuse"]);
+    assert!(
+        !out.contains("comment match"),
+        "deleted text must not remain searchable: {}",
+        out
+    );
+}
+/// A review carries a vote, so removing it would silently drop a verdict.
+/// Change the vote with a new review instead.
+#[test]
+fn a_review_cannot_be_deleted() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.patch_create("no review delete");
+    repo.run_ok(&["patch", "review", &id, "-v", "approve", "-b", "lgtm"]);
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    let review_id = first_comment_id(&json, "reviews");
+    let err = repo.run_err(&["patch", "delete-comment", &id, &review_id[..8]]);
+    assert!(
+        err.contains("review"),
+        "the refusal should explain that this is a review: {}",
+        err
+    );
+}
+// ===========================================================================
+// Attribution: a different key must not rewrite someone else's words
+// ===========================================================================
+/// The fold is what every reader sees, and anyone can append anything to a
+/// DAG they have a copy of. So the rule has to hold in the fold, not only at
+/// the CLI: an edit whose author is not the comment's author is ignored.
+#[test]
+fn an_edit_by_a_different_author_is_ignored_by_the_fold() {
+    let dir = TempDir::new().unwrap();
+    let repo = init_repo(dir.path(), &alice());
+    let (ref_name, id, comment_oid) = open_issue_with_comment(&repo);
+    append(
+        &repo,
+        &ref_name,
+        &bob(),
+        Action::BodyEdit {
+            target: comment_oid.to_string(),
+            body: "Bob's forgery".to_string(),
+        },
+    );
+    let state = issue_state(&repo, &ref_name, &id);
+    assert_eq!(
+        state.comments[0].body, "original text",
+        "an edit from another key must not rewrite the comment"
+    );
+    assert!(
+        !state.comments[0].edited,
+        "and must not even mark it as edited"
+    );
+}
+#[test]
+fn a_delete_by_a_different_author_is_ignored_by_the_fold() {
+    let dir = TempDir::new().unwrap();
+    let repo = init_repo(dir.path(), &alice());
+    let (ref_name, id, comment_oid) = open_issue_with_comment(&repo);
+    append(
+        &repo,
+        &ref_name,
+        &bob(),
+        Action::CommentDelete {
+            target: comment_oid.to_string(),
+        },
+    );
+    let state = issue_state(&repo, &ref_name, &id);
+    assert_eq!(state.comments[0].body, "original text");
+    assert!(!state.comments[0].deleted);
+}
+/// An honest user gets told, rather than silently no-op'd.
+#[test]
+fn the_cli_refuses_to_edit_someone_elses_comment() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.patch_create("not yours");
+    repo.run_ok(&["patch", "comment", &id, "-b", "Alice wrote this"]);
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    let comment_id = first_comment_id(&json, "comments");
+    // Same repo, different committer identity.
+    repo.git(&["config", "user.name", "Bob"]);
+    repo.git(&["config", "user.email", "bob@example.com"]);
+    let err = repo.run_err(&[
+        "patch",
+        "edit-comment",
+        &id,
+        &comment_id[..8],
+        "-b",
+        "Bob rewrites history",
+    ]);
+    assert!(
+        err.contains("alice@example.com"),
+        "the refusal should name the author who owns the comment: {}",
+        err
+    );
+    repo.git(&["config", "user.name", "Alice"]);
+    repo.git(&["config", "user.email", "alice@example.com"]);
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    assert_eq!(body_at(&json, "/comments/0/body"), "Alice wrote this");
+}
+// ===========================================================================
+// Ordering and convergence
+// ===========================================================================
+#[test]
+fn an_edited_comment_keeps_its_position() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.issue_open("ordering");
+    repo.run_ok(&["issue", "comment", &id, "-b", "first"]);
+    repo.run_ok(&["issue", "comment", &id, "-b", "second"]);
+    repo.run_ok(&["issue", "comment", &id, "-b", "third"]);
+    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
+    let value: serde_json::Value = serde_json::from_str(&json).unwrap();
+    let target = value["comments"][0]["commit_id"].as_str().unwrap().to_string();
+    repo.run_ok(&["issue", "edit-comment", &id, &target[..8], "-b", "FIRST"]);
+    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
+    let value: serde_json::Value = serde_json::from_str(&json).unwrap();
+    let bodies: Vec<&str> = value["comments"]
+        .as_array()
+        .unwrap()
+        .iter()
+        .map(|c| c["body"].as_str().unwrap())
+        .collect();
+    assert_eq!(
+        bodies,
+        vec!["FIRST", "second", "third"],
+        "an edited comment stays where it was"
+    );
+}
+/// Two clones edit the same comment while apart. Once they see each other's
+/// events, every clone must land on the same text — decided by `(clock, oid)`
+/// over the event set, which is content-derived and so client-independent.
+#[test]
+fn concurrent_edits_of_one_comment_converge() {
+    let dir = TempDir::new().unwrap();
+    let repo = init_repo(dir.path(), &alice());
+    let (ref_name, id, comment_oid) = open_issue_with_comment(&repo);
+    let fork_point = repo.refname_to_id(&ref_name).unwrap();
+    // Alice's clone edits.
+    let a_edit = append(
+        &repo,
+        &ref_name,
+        &alice(),
+        Action::BodyEdit {
+            target: comment_oid.to_string(),
+            body: "edit from clone A".to_string(),
+        },
+    );
+    let a_tip = repo.refname_to_id(&ref_name).unwrap();
+    // Her other clone, offline, edits the same comment from the fork point.
+    let branch_ref = "refs/collab/issues/other-clone";
+    repo.reference(branch_ref, fork_point, false, "fork").unwrap();
+    let b_edit = append(
+        &repo,
+        branch_ref,
+        &alice(),
+        Action::BodyEdit {
+            target: comment_oid.to_string(),
+            body: "edit from clone B".to_string(),
+        },
+    );
+    // Reconcile in both directions and check the two agree.
+    let sk = test_signing_key();
+    let merged_ref = "refs/collab/issues/merged";
+    repo.reference(merged_ref, a_tip, false, "copy").unwrap();
+    dag::reconcile(&repo, merged_ref, branch_ref, &alice(), &sk).unwrap();
+    let one = issue_state(&repo, merged_ref, &id);
+    let other_ref = "refs/collab/issues/merged-other-way";
+    repo.reference(other_ref, b_edit, false, "copy").unwrap();
+    dag::reconcile(&repo, other_ref, &ref_name, &alice(), &sk).unwrap();
+    let two = issue_state(&repo, other_ref, &id);
+    assert_eq!(
+        one.comments[0].body, two.comments[0].body,
+        "both join orders must fold to the same text"
+    );
+    // And the winner is the one decided by (clock, oid): equal clocks here,
+    // so the lexicographically larger OID wins.
+    let expected = if a_edit.to_string() > b_edit.to_string() {
+        "edit from clone A"
+    } else {
+        "edit from clone B"
+    };
+    assert_eq!(one.comments[0].body, expected);
+}
+/// A delete racing an edit is the same conflict, resolved the same way.
+#[test]
+fn a_delete_and_an_edit_racing_converge() {
+    let dir = TempDir::new().unwrap();
+    let repo = init_repo(dir.path(), &alice());
+    let (ref_name, id, comment_oid) = open_issue_with_comment(&repo);
+    let fork_point = repo.refname_to_id(&ref_name).unwrap();
+    let edit_oid = append(
+        &repo,
+        &ref_name,
+        &alice(),
+        Action::BodyEdit {
+            target: comment_oid.to_string(),
+            body: "kept after all".to_string(),
+        },
+    );
+    let a_tip = repo.refname_to_id(&ref_name).unwrap();
+    let branch_ref = "refs/collab/issues/delete-clone";
+    repo.reference(branch_ref, fork_point, false, "fork").unwrap();
+    let delete_oid = append(
+        &repo,
+        branch_ref,
+        &alice(),
+        Action::CommentDelete {
+            target: comment_oid.to_string(),
+        },
+    );
+    let sk = test_signing_key();
+    let merged_ref = "refs/collab/issues/dm";
+    repo.reference(merged_ref, a_tip, false, "copy").unwrap();
+    dag::reconcile(&repo, merged_ref, branch_ref, &alice(), &sk).unwrap();
+    let one = issue_state(&repo, merged_ref, &id);
+    let other_ref = "refs/collab/issues/dm-other";
+    repo.reference(other_ref, delete_oid, false, "copy").unwrap();
+    dag::reconcile(&repo, other_ref, &ref_name, &alice(), &sk).unwrap();
+    let two = issue_state(&repo, other_ref, &id);
+    assert_eq!(one.comments[0].deleted, two.comments[0].deleted);
+    assert_eq!(one.comments[0].body, two.comments[0].body);
+    let delete_wins = delete_oid.to_string() > edit_oid.to_string();
+    assert_eq!(one.comments[0].deleted, delete_wins);
+}
+// ===========================================================================
+// Revision bodies (7a299d2c)
+// ===========================================================================
+#[test]
+fn a_revision_body_can_be_corrected() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.patch_create("revision body");
+    repo.git(&["checkout", "test/revision-body"]);
+    repo.commit_file("more.txt", "more", "second commit");
+    repo.run_ok(&["patch", "revise", &id, "-b", "test"]);
+    repo.git(&["checkout", "main"]);
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    let value: serde_json::Value = serde_json::from_str(&json).unwrap();
+    let commit_before = value["revisions"][1]["commit"].as_str().unwrap().to_string();
+    assert_eq!(value["revisions"][1]["body"], "test");
+    repo.run_ok(&[
+        "patch",
+        "edit-revision",
+        &id,
+        "2",
+        "-b",
+        "Rework the trailer scan in response to review",
+    ]);
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    let value: serde_json::Value = serde_json::from_str(&json).unwrap();
+    assert_eq!(
+        value["revisions"][1]["body"],
+        "Rework the trailer scan in response to review"
+    );
+    assert_eq!(
+        value["revisions"][1]["commit"], commit_before,
+        "the revision's commit is immutable; only its description changes"
+    );
+    assert_eq!(
+        value["revisions"].as_array().unwrap().len(),
+,
+        "correcting a body must not manufacture a revision"
+    );
+}
+#[test]
+fn editing_an_unknown_revision_is_an_error() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.patch_create("no such revision");
+    let err = repo.run_err(&["patch", "edit-revision", &id, "9", "-b", "x"]);
+    assert!(
+        err.contains('9'),
+        "error should name the revision that does not exist: {}",
+        err
+    );
+}
+#[test]
+fn a_revision_body_edit_by_a_different_author_is_ignored() {
+    let dir = TempDir::new().unwrap();
+    let repo = init_repo(dir.path(), &alice());
+    let (ref_name, id) = common::create_patch(&repo, &alice(), "revision attribution");
+    let revision_oid = append(
+        &repo,
+        &ref_name,
+        &alice(),
+        Action::PatchRevision {
+            commit: "a".repeat(40),
+            tree: "b".repeat(40),
+            body: Some("mine".to_string()),
+            base: None,
+        },
+    );
+    append(
+        &repo,
+        &ref_name,
+        &bob(),
+        Action::BodyEdit {
+            target: revision_oid.to_string(),
+            body: "Bob's rewrite".to_string(),
+        },
+    );
+    let state = PatchState::from_ref_uncached(&repo, &ref_name, &id).unwrap();
+    assert_eq!(state.revisions[1].body.as_deref(), Some("mine"));
+}
+// ===========================================================================
+// Reading must never write
+// ===========================================================================
+/// This project has shipped a "display appends an event" bug before. Every
+/// read path is checked against the events ref, which is the one that carries
+/// history: showing, diffing and logging a patch must leave it exactly where
+/// it was.
+#[test]
+fn reading_a_patch_never_moves_its_event_ref() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.patch_create("read only");
+    repo.run_ok(&["patch", "comment", &id, "-b", "a comment"]);
+    repo.run_ok(&["patch", "review", &id, "-v", "comment", "-b", "a review"]);
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    let comment_id = first_comment_id(&json, "comments");
+    repo.run_ok(&["patch", "delete-comment", &id, &comment_id[..8]]);
+    let events_ref = repo
+        .git(&["for-each-ref", "--format=%(refname)", "refs/collab/patches/"])
+        .lines()
+        .find(|l| l.ends_with("/events"))
+        .expect("patch events ref")
+        .to_string();
+    let before = repo.git(&["rev-parse", &events_ref]).trim().to_string();
+    repo.run_ok(&["patch", "show", &id]);
+    repo.run_ok(&["patch", "show", &id, "--json"]);
+    repo.run_ok(&["patch", "log", &id]);
+    repo.run_ok(&["patch", "log", &id, "--json"]);
+    repo.run_ok(&["patch", "diff", &id]);
+    repo.run_ok(&["patch", "list"]);
+    repo.run_ok(&["search", "comment"]);
+    let after = repo.git(&["rev-parse", &events_ref]).trim().to_string();
+    assert_eq!(before, after, "reading a patch must not append events");
+}
+#[test]
+fn reading_an_issue_never_moves_its_ref() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.issue_open("read only issue");
+    repo.run_ok(&["issue", "comment", &id, "-b", "a comment"]);
+    let issue_ref = repo
+        .git(&["for-each-ref", "--format=%(refname)", "refs/collab/issues/"])
+        .lines()
+        .next()
+        .expect("issue ref")
+        .to_string();
+    let before = repo.git(&["rev-parse", &issue_ref]).trim().to_string();
+    repo.run_ok(&["issue", "show", &id]);
+    repo.run_ok(&["issue", "show", &id, "--json"]);
+    repo.run_ok(&["issue", "list"]);
+    repo.run_ok(&["search", "comment"]);
+    let after = repo.git(&["rev-parse", &issue_ref]).trim().to_string();
+    assert_eq!(before, after, "reading an issue must not append events");
+}
+// ===========================================================================
+// The original events stay in the DAG
+// ===========================================================================
+/// Editing supersedes; it does not rewrite. The original event is still
+/// there, which is what makes the log an audit trail rather than a summary.
+#[test]
+fn an_edit_leaves_the_original_event_in_the_log() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.issue_open("audit trail");
+    repo.run_ok(&["issue", "comment", &id, "-b", "the original wording"]);
+    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
+    let comment_id = first_comment_id(&json, "comments");
+    repo.run_ok(&[
+        "issue",
+        "edit-comment",
+        &id,
+        &comment_id[..8],
+        "-b",
+        "the revised wording",
+    ]);
+    let log = repo.run_ok(&["log"]);
+    assert!(
+        log.contains("IssueComment") && log.contains("BodyEdit"),
+        "both events should appear in the raw event log: {}",
+        log
+    );
+    assert!(
+        log.contains("the original wording") && log.contains("the revised wording"),
+        "the log should show what was said before and after: {}",
+        log
+    );
+    assert!(
+        log.contains(&format!("edit body of {:.8}", comment_id)),
+        "the edit should name the event it supersedes: {}",
+        log
+    );
+    // And the original body is still readable from the event object itself.
+    let blob = repo.git(&["show", &format!("{}:event.json", comment_id)]);
+    assert!(
+        blob.contains("the original wording"),
+        "the superseded event is unchanged in the DAG"
+    );
+}
+#[test]
+fn an_edited_comment_is_marked_as_edited() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.issue_open("edited marker");
+    repo.run_ok(&["issue", "comment", &id, "-b", "before"]);
+    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
+    let comment_id = first_comment_id(&json, "comments");
+    repo.run_ok(&["issue", "edit-comment", &id, &comment_id[..8], "-b", "after"]);
+    let out = repo.run_ok(&["issue", "show", &id]);
+    assert!(
+        out.contains("edited"),
+        "a corrected comment should say so: {}",
+        out
+    );
+}
+/// Comment ids have to be visible, or nothing above can be addressed.
+#[test]
+fn show_prints_addressable_comment_ids() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.patch_create("ids");
+    repo.run_ok(&["patch", "comment", &id, "-b", "thread"]);
+    repo.run_ok(&[
+        "patch", "comment", &id, "--file", "a.rs", "--line", "1", "-b", "inline",
+    ]);
+    repo.run_ok(&["patch", "review", &id, "-v", "comment", "-b", "review"]);
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    let out = repo.run_ok(&["patch", "show", &id]);
+    for list in ["comments", "inline_comments", "reviews"] {
+        let full = first_comment_id(&json, list);
+        assert!(
+            out.contains(&full[..8]),
+            "`patch show` should print the short id for {}: {}",
+            list,
+            out
+        );
+    }
+}
+/// A verdict that is not a comment still needs its author checked, so make
+/// sure the plumbing for reviews goes through the same rule.
+#[test]
+fn a_review_edit_by_a_different_author_is_ignored_by_the_fold() {
+    let dir = TempDir::new().unwrap();
+    let repo = init_repo(dir.path(), &alice());
+    let (ref_name, id) = common::create_patch(&repo, &alice(), "review attribution");
+    let review_oid = append(
+        &repo,
+        &ref_name,
+        &alice(),
+        Action::PatchReview {
+            verdict: ReviewVerdict::Comment,
+            body: "Alice's review".to_string(),
+            revision: Some(1),
+        },
+    );
+    append(
+        &repo,
+        &ref_name,
+        &bob(),
+        Action::BodyEdit {
+            target: review_oid.to_string(),
+            body: "Bob's rewrite".to_string(),
+        },
+    );
+    let state = PatchState::from_ref_uncached(&repo, &ref_name, &id).unwrap();
+    assert_eq!(state.reviews[0].body, "Alice's review");
+}

tests/body_input_test.rs

@@ @@ -0,0 +1,324 @@ @@
+//! Prose gets into git-collab through more than one door.
+//!
+//! Every command that takes `--body` must also accept `--body-file <path>`
+//! and `--body-file -` for stdin, following git's own `-F` convention, and
+//! must open `$EDITOR` when an interactive user gives no body at all.
+//!
+//! The load-bearing property throughout is that whatever goes in comes back
+//! out *byte for byte*: trailing newlines, CRLF, tabs, non-ASCII and shell
+//! metacharacters all survive the round trip through the event DAG. That is
+//! the class of corruption nobody notices until they read a review three
+//! weeks later.
+mod common;
+use common::TestRepo;
+/// A body designed to be hostile to every layer it passes through: shell
+/// metacharacters, quotes of both kinds, a `$`, a backtick, a blank line, a
+/// tab, non-ASCII (including a combining mark and an emoji), a CRLF, and
+/// two trailing newlines.
+const NASTY: &str = "Line with `backticks` and $VARS and \"quotes\" and 'single'\n\nDoes this work? Yes * always!\n\tTabbed\r\nCRLF above\nnaïve café — ünïcödé 🎉 e\u{0301}\n\n";
+/// Pull a JSON string field out of `... --json` output.
+fn json_field(json: &str, pointer: &str) -> String {
+    let value: serde_json::Value = serde_json::from_str(json).expect("valid JSON");
+    value
+        .pointer(pointer)
+        .unwrap_or_else(|| panic!("no {} in {}", pointer, json))
+        .as_str()
+        .expect("string field")
+        .to_string()
+}
+// ===========================================================================
+// --body-file <path>
+// ===========================================================================
+#[test]
+fn issue_comment_reads_body_from_a_file_byte_for_byte() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.issue_open("nasty body");
+    let path = repo.dir.path().join("body.txt");
+    std::fs::write(&path, NASTY).unwrap();
+    repo.run_ok(&["issue", "comment", &id, "-F", path.to_str().unwrap()]);
+    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
+    assert_eq!(json_field(&json, "/comments/0/body"), NASTY);
+}
+#[test]
+fn body_file_has_a_long_form_too() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.issue_open("long form");
+    let path = repo.dir.path().join("body.txt");
+    std::fs::write(&path, "from the long form\n").unwrap();
+    repo.run_ok(&[
+        "issue",
+        "comment",
+        &id,
+        "--body-file",
+        path.to_str().unwrap(),
+    ]);
+    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
+    assert_eq!(json_field(&json, "/comments/0/body"), "from the long form\n");
+}
+#[test]
+fn missing_body_file_reports_the_path() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.issue_open("missing file");
+    let err = repo.run_err(&["issue", "comment", &id, "-F", "/nonexistent/body.txt"]);
+    assert!(
+        err.contains("/nonexistent/body.txt"),
+        "error should name the path it could not read: {}",
+        err
+    );
+}
+// ===========================================================================
+// --body-file - (stdin)
+// ===========================================================================
+#[test]
+fn issue_comment_reads_body_from_stdin_byte_for_byte() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.issue_open("stdin body");
+    repo.run_stdin_ok(&["issue", "comment", &id, "-F", "-"], NASTY.as_bytes());
+    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
+    assert_eq!(json_field(&json, "/comments/0/body"), NASTY);
+}
+#[test]
+fn patch_review_reads_body_from_stdin() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.patch_create("review from stdin");
+    repo.run_stdin_ok(
+        &["patch", "review", &id, "-v", "comment", "-F", "-"],
+        NASTY.as_bytes(),
+    );
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    assert_eq!(json_field(&json, "/reviews/0/body"), NASTY);
+}
+#[test]
+fn patch_comment_reads_body_from_stdin() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.patch_create("comment from stdin");
+    repo.run_stdin_ok(&["patch", "comment", &id, "-F", "-"], NASTY.as_bytes());
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    assert_eq!(json_field(&json, "/comments/0/body"), NASTY);
+}
+#[test]
+fn patch_inline_comment_reads_body_from_stdin() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.patch_create("inline from stdin");
+    repo.run_stdin_ok(
+        &[
+            "patch", "comment", &id, "--file", "src/x.rs", "--line", "3", "-F", "-",
+        ],
+        NASTY.as_bytes(),
+    );
+    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
+    assert_eq!(json_field(&json, "/inline_comments/0/body"), NASTY);
+}
+#[test]
+fn stdin_body_survives_on_every_command_that_takes_one() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    // issue open
+    let out = repo.run_stdin_ok(
+        &["issue", "open", "-t", "opened from stdin", "-F", "-"],
+        NASTY.as_bytes(),
+    );
+    let issue_id = out.trim().strip_prefix("Opened issue ").unwrap().to_string();
+    let json = repo.run_ok(&["issue", "show", &issue_id, "--json"]);
+    assert_eq!(json_field(&json, "/body"), NASTY, "issue open");
+    // issue edit
+    repo.run_stdin_ok(
+        &["issue", "edit", &issue_id, "-F", "-"],
+        b"edited from stdin\n\n",
+    );
+    let json = repo.run_ok(&["issue", "show", &issue_id, "--json"]);
+    assert_eq!(json_field(&json, "/body"), "edited from stdin\n\n");
+    // patch create
+    repo.git(&["checkout", "-b", "stdin-patch"]);
+    repo.commit_file("stdin.txt", "x", "stdin patch commit");
+    let out = repo.run_stdin_ok(
+        &[
+            "patch",
+            "create",
+            "-t",
+            "created from stdin",
+            "-B",
+            "stdin-patch",
+            "-F",
+            "-",
+        ],
+        NASTY.as_bytes(),
+    );
+    repo.git(&["checkout", "main"]);
+    let patch_id = out.trim().strip_prefix("Created patch ").unwrap().to_string();
+    let json = repo.run_ok(&["patch", "show", &patch_id, "--json"]);
+    assert_eq!(json_field(&json, "/body"), NASTY, "patch create");
+    // patch revise
+    repo.git(&["checkout", "stdin-patch"]);
+    repo.commit_file("stdin2.txt", "y", "second stdin commit");
+    repo.run_stdin_ok(&["patch", "revise", &patch_id, "-F", "-"], NASTY.as_bytes());
+    repo.git(&["checkout", "main"]);
+    let json = repo.run_ok(&["patch", "show", &patch_id, "--json"]);
+    assert_eq!(json_field(&json, "/revisions/1/body"), NASTY, "patch revise");
+}
+// ===========================================================================
+// Conflicts and refusals
+// ===========================================================================
+#[test]
+fn body_and_body_file_together_are_refused() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.issue_open("conflict");
+    let path = repo.dir.path().join("body.txt");
+    std::fs::write(&path, "from file").unwrap();
+    let err = repo.run_err(&[
+        "issue",
+        "comment",
+        &id,
+        "-b",
+        "from flag",
+        "-F",
+        path.to_str().unwrap(),
+    ]);
+    assert!(
+        err.contains("body") && err.contains("body-file"),
+        "error should name both options: {}",
+        err
+    );
+}
+/// The failure mode that matters most for scripts and agents: with no body
+/// and no terminal there is nothing to open an editor on, so the command must
+/// say so and exit rather than block forever on a detached editor.
+#[test]
+fn a_missing_body_without_a_terminal_is_an_error_not_a_hang() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.issue_open("no body");
+    let err = repo.run_err(&["issue", "comment", &id]);
+    assert!(
+        err.contains("--body-file"),
+        "error should point at the non-interactive alternatives: {}",
+        err
+    );
+}
+#[test]
+fn an_empty_body_file_is_refused_rather_than_recorded() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.issue_open("empty file");
+    let path = repo.dir.path().join("empty.txt");
+    std::fs::write(&path, "").unwrap();
+    let err = repo.run_err(&["issue", "comment", &id, "-F", path.to_str().unwrap()]);
+    assert!(
+        err.to_lowercase().contains("empty"),
+        "error should say the body was empty: {}",
+        err
+    );
+}
+// ===========================================================================
+// $EDITOR
+// ===========================================================================
+/// An interactive user who gives no body gets an editor, and whatever they
+/// save is the body — verbatim, including the trailing blank line.
+#[test]
+fn a_missing_body_opens_the_editor_and_keeps_its_bytes() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.issue_open("editor body");
+    // A fake editor that overwrites the file it is handed. Written with
+    // printf so the exact trailing bytes are under the test's control.
+    let editor = repo.write_script(
+        "fake-editor.sh",
+        "#!/bin/sh\nprintf 'from the editor\\n\\n' > \"$1\"\n",
+    );
+    let output = repo.run_in_pty(&["issue", "comment", &id], &editor);
+    assert!(
+        output.status.success(),
+        "editor run failed: {}",
+        String::from_utf8_lossy(&output.stderr)
+    );
+    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
+    assert_eq!(json_field(&json, "/comments/0/body"), "from the editor\n\n");
+}
+/// A body typed into an editor is not a commit message: nothing may be
+/// stripped from it. A leading `#` is a markdown heading, and must survive.
+#[test]
+fn the_editor_strips_nothing_not_even_leading_hashes() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.issue_open("hash body");
+    let editor = repo.write_script(
+        "fake-editor.sh",
+        "#!/bin/sh\nprintf '# Heading\\n\\nbody text\\n' > \"$1\"\n",
+    );
+    repo.run_in_pty(&["issue", "comment", &id], &editor);
+    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
+    assert_eq!(
+        json_field(&json, "/comments/0/body"),
+        "# Heading\n\nbody text\n"
+    );
+}
+/// Leaving the editor without writing anything aborts the whole command —
+/// no empty comment is appended to the DAG, where it could never be removed.
+#[test]
+fn an_empty_editor_buffer_aborts_without_appending() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = repo.issue_open("aborted");
+    let editor = repo.write_script("fake-editor.sh", "#!/bin/sh\n: > \"$1\"\n");
+    let output = repo.run_in_pty(&["issue", "comment", &id], &editor);
+    assert!(
+        !output.status.success(),
+        "an empty editor buffer should abort the command"
+    );
+    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
+    let value: serde_json::Value = serde_json::from_str(&json).unwrap();
+    assert_eq!(
+        value["comments"].as_array().unwrap().len(),
+,
+        "aborting must leave no comment behind"
+    );
+}

tests/common/mod.rs

@@ @@ -449,6 +449,99 @@ impl TestRepo { @@
+            .expect("failed to run git-collab")
+    /// Run git-collab with `stdin` piped in, and return raw output.
+    ///
+    /// Takes bytes rather than `&str` so a test can feed content that is not
+    /// valid UTF-8, and so nothing along the way is tempted to normalize
+    /// line endings or trailing newlines.
+    pub fn run_with_stdin(&self, args: &[&str], stdin: &[u8]) -> Output {
+        let mut child = self
+            .cli_command()
+            .args(args)
+            .stdin(Stdio::piped())
+            .stdout(Stdio::piped())
+            .stderr(Stdio::piped())
+            .spawn()
+            .expect("failed to spawn git-collab");
+        child
+            .stdin
+            .take()
+            .expect("stdin piped")
+            .write_all(stdin)
+            .expect("write stdin");
+        child.wait_with_output().expect("collect git-collab output")
+    }
+    /// `run_with_stdin`, asserting success and returning stdout.
+    pub fn run_stdin_ok(&self, args: &[&str], stdin: &[u8]) -> String {
+        let output = self.run_with_stdin(args, stdin);
+        let stdout = String::from_utf8_lossy(&output.stdout).to_string();
+        let stderr = String::from_utf8_lossy(&output.stderr).to_string();
+        assert!(
+            output.status.success(),
+            "git-collab {:?} failed (exit {:?}):\nstdout: {}\nstderr: {}",
+            args,
+            output.status.code(),
+            stdout,
+            stderr
+        );
+        stdout
+    }
+    /// Run git-collab attached to a pseudo-terminal, so the code under test
+    /// sees stdin as a TTY. `editor` is exported as `$EDITOR` for the run.
+    ///
+    /// This is the only way to exercise the interactive editor path end to
+    /// end: opening an editor is deliberately conditional on stdin being a
+    /// terminal, so a piped run can never reach it.
+    pub fn run_in_pty(&self, args: &[&str], editor: &str) -> Output {
+        let quoted: Vec<String> = args
+            .iter()
+            .map(|a| format!("'{}'", a.replace('\'', r"'\''")))
+            .collect();
+        let script = format!(
+            "{} {}",
+            env!("CARGO_BIN_EXE_git-collab"),
+            quoted.join(" ")
+        );
+        let mut command = Command::new("script");
+        self.apply_env(&mut command);
+        command.env("EDITOR", editor);
+        let mut child = command
+            .args(["-qec", &script, "/dev/null"])
+            .current_dir(self.dir.path())
+            .stdin(Stdio::piped())
+            .stdout(Stdio::piped())
+            .stderr(Stdio::piped())
+            .spawn()
+            .expect("failed to launch git-collab in a pty");
+        let deadline = Instant::now() + Duration::from_secs(20);
+        loop {
+            if child.try_wait().expect("poll pty process").is_some() {
+                return child.wait_with_output().expect("collect pty output");
+            }
+            if Instant::now() >= deadline {
+                let _ = child.kill();
+                panic!("git-collab {:?} hung in a pty (editor path did not return)", args);
+            }
+            thread::sleep(Duration::from_millis(20));
+        }
+    }
+    /// Write a shell script into the repo's temp dir and return an editor
+    /// *command* that runs it via `sh`, for use as a fake `$EDITOR`.
+    ///
+    /// `sh <path>` rather than chmod+exec: tests run on many threads, and a
+    /// `Command::spawn` in one thread inherits any write file descriptor
+    /// another thread has open, which makes exec'ing a freshly written script
+    /// fail with ETXTBSY at random. `sh` only opens it for reading.
+    pub fn write_script(&self, name: &str, body: &str) -> String {
+        let path = self.dir.path().join(name);
+        std::fs::write(&path, body).unwrap();
+        format!("sh {}", path.display())
+    }
+    /// Run git-collab, assert success, return stdout.
+    pub fn run_ok(&self, args: &[&str]) -> String {
+        let output = self.run(args);