5cbded8d

Let a script read what a write command just did

a73x 2026-08-11 10:44

Commit message

Let a script read what a write command just did

Read commands have carried `--json` for a long time; the commands that
create something did not, so a script could learn the id of what it had
just made only by scraping prose — and that prose moved underneath every
such script the moment ids became sized to the repository.

Every command that creates, changes, closes or records something now
takes `--json` and prints exactly one object on stdout. The ids in it are
always full: abbreviation is sized to the repo and overridable with
`collab.abbrev`, which makes it a display policy and nothing a caller
should ever parse. It appears in the prose and never in the JSON.

The two halves of the contract that already existed are matched rather
than re-invented. A failure still prints `{"error": "..."}` on stdout and
exits 1, so a caller that asked for JSON never reads stderr. Auto-sync
still narrates on stderr under its prefix, which is what lets stdout stay
one parseable value whether or not the push happened — and is why
`--json` on a write is safe at all.

`--json` is declared per command rather than as a global flag. A global
one would be *accepted* everywhere, including by commands with no JSON to
give, and a flag that is silently ignored is worse for a script than one
that is refused: it gets prose it will happily mis-parse instead of an
exit code it can check. `sync` and `init` stay out for a reason of their
own — their stdout is a multi-remote report with a partial-failure exit
code, so their `--json` is a shape to design, not an id to emit.

Also adds `patch reopen` (259200d4), the missing counterpart to
`issue reopen`. Merges are recorded rather than derived now, and the
design says a `PatchMerge` recorded in error is corrected "by closing or
by a future reopen event" — only the closing half existed, which left the
patch `closed` when what was wanted was `open`, and left a patch closed
by mistake closed forever.

Reopening clears the recorded merge commit, for the reason `patch close`
already clears it: `merge_commit` is written under the same `(clock, oid)`
guard as `status` precisely so the two cannot disagree, and an open patch
naming the commit that landed it is that disagreement. The command prints
the commit it dropped, so recording the merge again is a copy and paste.

Reopening is an ordinary event, resolved by the same total order as every
other status change, so a close on one clone and a reopen on another
converge. Making that true needed one fix in `sync`: closing moves a ref
between the active and archive namespaces while the id stays the same, and
`reconcile_refs` only ever looked at the active prefix — so one clone's
archived close and another's active reopen were adopted as two divergent
DAGs for one patch, which no later sync could reunite. Both namespaces now
reconcile into whichever ref this clone already holds, and an object the
DAG says is open again comes back out of the archive. Only that direction:
a closed object is filtered out of the default list by its status wherever
it sits, so failing to archive costs nothing, while failing to unarchive
leaves a patch open and unfindable.

Fixes: a6adfe39
Fixes: 259200d4

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

README.md

+and writes nothing. `--json` reports it as a separate `looks_merged` field,
+never inside `status`.
+A merge recorded in error is undone by `git-collab patch reopen`, which is also
+how a patch closed by mistake comes back. It clears the recorded merge commit as
+it goes — an open patch that still named the commit that landed it would be two
+answers to one question — and prints the commit it dropped, so recording the
+merge again is a copy and paste. Reopening is an ordinary event: a close on
+another clone and a reopen on this one are resolved by the same total order as
+every other status change, so the two converge instead of racing.
+## Scripting it
+Every command that creates, changes, closes or records something takes `--json`
+and prints exactly one object on stdout:
+```console
+$ git-collab issue open -t "The bug" --json
+{"action":"issue.open","issue":"7c1e...(full id)"}
+$ git-collab patch merge a1b2c3d4 --json
+{"action":"patch.merge","patch":"...","commit":"...","already_recorded":false,"closed_issue":"..."}
+```
+Ids there are always full. Abbreviation is sized to the repository and can be
+overridden with `collab.abbrev`, which makes it a display policy and nothing a
+script should ever have to parse — so it appears in the prose and never in the
+JSON. A failure prints `{"error": "..."}`, on stdout, and exits 1: a caller that
+asked for JSON never has to read stderr. And stderr is where the push that a
+write triggers narrates itself, under an `auto-sync:` prefix, so stdout stays
+one parseable value whether or not the network was involved.
+## Install
+Requires Rust 1.88 or newer.

src/cli.rs

Old		New
@@ -172,14 +172,24 @@ impl Cli {
172	/// never looks at stderr — so an error delivered only to stderr is a	172	/// never looks at stderr — so an error delivered only to stderr is a
173	/// success as far as they can tell. Knowing the answer here lets the failure	173	/// success as far as they can tell. Knowing the answer here lets the failure
174	/// path speak the same language the caller asked for.	174	/// path speak the same language the caller asked for.
		175	///
		176	/// Every command that creates, changes, closes or records something answers
		177	/// here. The commands that do not are the ones whose stdout is a report
		178	/// rather than a result: `sync` and `init` narrate a fetch, a reconcile and
		179	/// a push across N remotes and exit non-zero on a partial failure, so their
		180	/// `--json` is a sync-report shape to be designed, not an id to be emitted;
		181	/// `patch diff` prints a diff; `status`, `log` and `search` are read
		182	/// commands that never grew the flag. None of them accepts `--json` and is
		183	/// then ignored — clap refuses it, which is the answer a script can act on.
175	pub fn wants_json(&self) -> bool {	184	pub fn wants_json(&self) -> bool {
176	match &self.command {	185	match &self.command {
177	Commands::Issue(IssueCmd::List { json, .. })	186	Commands::Issue(cmd) => cmd.wants_json(),
178	\| Commands::Issue(IssueCmd::Show { json, .. })	187	Commands::Patch(cmd) => cmd.wants_json(),
179	\| Commands::Patch(PatchCmd::List { json, .. })	188	Commands::Release(cmd) => cmd.wants_json(),
180	\| Commands::Patch(PatchCmd::Show { json, .. })	189	Commands::Key(cmd) => cmd.wants_json(),
181	\| Commands::Patch(PatchCmd::Log { json, .. })	190	Commands::Identity(cmd) => cmd.wants_json(),
182	\| Commands::Release(ReleaseCmd::List { json, .. }) => *json,	191	Commands::Hooks(HookCmd::Install { json }) => *json,
		192	Commands::InitKey { json, .. } => *json,
183	_ => false,	193	_ => false,
184	}	194	}
185	}	195	}
@@ -252,6 +262,9 @@ pub enum Commands {
252	/// Overwrite existing key files	262	/// Overwrite existing key files
253	#[arg(long)]	263	#[arg(long)]
254	force: bool,	264	force: bool,
		265	/// Output as JSON
		266	#[arg(long)]
		267	json: bool,
255	},	268	},
256		269
257	/// Manage trusted keys	270	/// Manage trusted keys
@@ -276,7 +289,11 @@ pub enum Commands {
276	#[derive(Subcommand, Debug)]	289	#[derive(Subcommand, Debug)]
277	pub enum HookCmd {	290	pub enum HookCmd {
278	/// Install the commit-msg hook (also done by `git-collab init`)	291	/// Install the commit-msg hook (also done by `git-collab init`)
279	Install,	292	Install {
		293	/// Output as JSON
		294	#[arg(long)]
		295	json: bool,
		296	},
280		297
281	/// Report whether the hook is installed and what it would stamp	298	/// Report whether the hook is installed and what it would stamp
282	Status,	299	Status,
@@ -304,6 +321,99 @@ pub enum HookCmd {
304	/// argument for the flag.	321	/// argument for the flag.
305	const BODY_FILE_HELP: &str = "Read the body from a file, or from stdin with '-'";	322	const BODY_FILE_HELP: &str = "Read the body from a file, or from stdin with '-'";
306		323
		324
		325	// ---------------------------------------------------------------------------
		326	// `--json` is per-command rather than a global flag, and the four `wants_json`
		327	// implementations below are the price of that.
		328	//
		329	// A `global = true` flag would have been one line, but it would also be
		330	// accepted by every command, including the ones that have no JSON rendering
		331	// to offer — and a flag that is quietly ignored is worse for a script than one
		332	// that is refused, because the script gets prose it will happily mis-parse
		333	// instead of an exit code it can check. Declaring it per command keeps clap's
		334	// own "unexpected argument" refusal for everything else.
		335	// ---------------------------------------------------------------------------
		336
		337	impl IssueCmd {
		338	fn wants_json(&self) -> bool {
		339	match self {
		340	IssueCmd::Open { json, .. }
		341	\| IssueCmd::List { json, .. }
		342	\| IssueCmd::Show { json, .. }
		343	\| IssueCmd::Comment { json, .. }
		344	\| IssueCmd::Edit { json, .. }
		345	\| IssueCmd::EditComment { json, .. }
		346	\| IssueCmd::DeleteComment { json, .. }
		347	\| IssueCmd::Label { json, .. }
		348	\| IssueCmd::Unlabel { json, .. }
		349	\| IssueCmd::Relate { json, .. }
		350	\| IssueCmd::Unrelate { json, .. }
		351	\| IssueCmd::Assign { json, .. }
		352	\| IssueCmd::Unassign { json, .. }
		353	\| IssueCmd::Close { json, .. }
		354	\| IssueCmd::Delete { json, .. }
		355	\| IssueCmd::Reopen { json, .. } => *json,
		356	}
		357	}
		358	}
		359
		360	impl PatchCmd {
		361	fn wants_json(&self) -> bool {
		362	match self {
		363	PatchCmd::Create { json, .. }
		364	\| PatchCmd::List { json, .. }
		365	\| PatchCmd::Show { json, .. }
		366	\| PatchCmd::Comment { json, .. }
		367	\| PatchCmd::Review { json, .. }
		368	\| PatchCmd::Revise { json, .. }
		369	\| PatchCmd::EditComment { json, .. }
		370	\| PatchCmd::DeleteComment { json, .. }
		371	\| PatchCmd::EditRevision { json, .. }
		372	\| PatchCmd::Log { json, .. }
		373	\| PatchCmd::Label { json, .. }
		374	\| PatchCmd::Unlabel { json, .. }
		375	\| PatchCmd::Merge { json, .. }
		376	\| PatchCmd::Close { json, .. }
		377	\| PatchCmd::Reopen { json, .. }
		378	\| PatchCmd::Delete { json, .. }
		379	\| PatchCmd::Checkout { json, .. } => *json,
		380	// `patch diff` prints a diff, which is not a result with an id and
		381	// has no JSON rendering to give.
		382	PatchCmd::Diff { .. } => false,
		383	}
		384	}
		385	}
		386
		387	impl KeyCmd {
		388	fn wants_json(&self) -> bool {
		389	match self {
		390	KeyCmd::Generate { json, .. }
		391	\| KeyCmd::Add { json, .. }
		392	\| KeyCmd::Remove { json, .. } => *json,
		393	KeyCmd::List { .. } => false,
		394	}
		395	}
		396	}
		397
		398	impl IdentityCmd {
		399	fn wants_json(&self) -> bool {
		400	match self {
		401	IdentityCmd::Alias { json, .. } \| IdentityCmd::Unalias { json, .. } => *json,
		402	IdentityCmd::List => false,
		403	}
		404	}
		405	}
		406
		407	impl ReleaseCmd {
		408	fn wants_json(&self) -> bool {
		409	match self {
		410	ReleaseCmd::Publish { json, .. }
		411	\| ReleaseCmd::List { json, .. }
		412	\| ReleaseCmd::Delete { json, .. } => *json,
		413	}
		414	}
		415	}
		416
307	#[derive(Subcommand, Debug)]	417	#[derive(Subcommand, Debug)]
308	pub enum IssueCmd {	418	pub enum IssueCmd {
309	/// Open a new issue	419	/// Open a new issue
@@ -322,6 +432,9 @@ pub enum IssueCmd {
322	/// Related issue ID	432	/// Related issue ID
323	#[arg(long)]	433	#[arg(long)]
324	relates_to: Option<String>,	434	relates_to: Option<String>,
		435	/// Output as JSON
		436	#[arg(long)]
		437	json: bool,
325	},	438	},
326	/// List issues	439	/// List issues
327	#[command(alias = "ls")]	440	#[command(alias = "ls")]
@@ -368,6 +481,9 @@ pub enum IssueCmd {
368	body: Option<String>,	481	body: Option<String>,
369	#[arg(short = 'F', long, help = BODY_FILE_HELP)]	482	#[arg(short = 'F', long, help = BODY_FILE_HELP)]
370	body_file: Option<String>,	483	body_file: Option<String>,
		484	/// Output as JSON
		485	#[arg(long)]
		486	json: bool,
371	},	487	},
372	/// Edit an issue's title or body	488	/// Edit an issue's title or body
373	Edit {	489	Edit {
@@ -381,6 +497,9 @@ pub enum IssueCmd {
381	body: Option<String>,	497	body: Option<String>,
382	#[arg(short = 'F', long, help = BODY_FILE_HELP)]	498	#[arg(short = 'F', long, help = BODY_FILE_HELP)]
383	body_file: Option<String>,	499	body_file: Option<String>,
		500	/// Output as JSON
		501	#[arg(long)]
		502	json: bool,
384	},	503	},
385	/// Correct the text of a comment you wrote	504	/// Correct the text of a comment you wrote
386	///	505	///
@@ -397,6 +516,9 @@ pub enum IssueCmd {
397	body: Option<String>,	516	body: Option<String>,
398	#[arg(short = 'F', long, help = BODY_FILE_HELP)]	517	#[arg(short = 'F', long, help = BODY_FILE_HELP)]
399	body_file: Option<String>,	518	body_file: Option<String>,
		519	/// Output as JSON
		520	#[arg(long)]
		521	json: bool,
400	},	522	},
401	/// Delete a comment you wrote, leaving a tombstone in its place	523	/// Delete a comment you wrote, leaving a tombstone in its place
402	///	524	///
@@ -407,6 +529,9 @@ pub enum IssueCmd {
407	id: String,	529	id: String,
408	/// Comment ID (prefix match), as printed by `issue show`	530	/// Comment ID (prefix match), as printed by `issue show`
409	comment: String,	531	comment: String,
		532	/// Output as JSON
		533	#[arg(long)]
		534	json: bool,
410	},	535	},
411	/// Add a label to an issue	536	/// Add a label to an issue
412	Label {	537	Label {
@@ -414,6 +539,9 @@ pub enum IssueCmd {
414	id: String,	539	id: String,
415	/// Label to add	540	/// Label to add
416	label: String,	541	label: String,
		542	/// Output as JSON
		543	#[arg(long)]
		544	json: bool,
417	},	545	},
418	/// Remove a label from an issue	546	/// Remove a label from an issue
419	Unlabel {	547	Unlabel {
@@ -421,6 +549,9 @@ pub enum IssueCmd {
421	id: String,	549	id: String,
422	/// Label to remove	550	/// Label to remove
423	label: String,	551	label: String,
		552	/// Output as JSON
		553	#[arg(long)]
		554	json: bool,
424	},	555	},
425	/// Relate an issue to another issue	556	/// Relate an issue to another issue
426	Relate {	557	Relate {
@@ -428,6 +559,9 @@ pub enum IssueCmd {
428	id: String,	559	id: String,
429	/// Other issue ID to relate to	560	/// Other issue ID to relate to
430	other: String,	561	other: String,
		562	/// Output as JSON
		563	#[arg(long)]
		564	json: bool,
431	},	565	},
432	/// Remove a relation between two issues	566	/// Remove a relation between two issues
433	Unrelate {	567	Unrelate {
@@ -435,6 +569,9 @@ pub enum IssueCmd {
435	id: String,	569	id: String,
436	/// Other issue ID to unrelate from	570	/// Other issue ID to unrelate from
437	other: String,	571	other: String,
		572	/// Output as JSON
		573	#[arg(long)]
		574	json: bool,
438	},	575	},
439	/// Assign an issue to someone	576	/// Assign an issue to someone
440	Assign {	577	Assign {
@@ -442,6 +579,9 @@ pub enum IssueCmd {
442	id: String,	579	id: String,
443	/// Name to assign	580	/// Name to assign
444	name: String,	581	name: String,
		582	/// Output as JSON
		583	#[arg(long)]
		584	json: bool,
445	},	585	},
446	/// Unassign someone from an issue	586	/// Unassign someone from an issue
447	Unassign {	587	Unassign {
@@ -449,6 +589,9 @@ pub enum IssueCmd {
449	id: String,	589	id: String,
450	/// Name to unassign	590	/// Name to unassign
451	name: String,	591	name: String,
		592	/// Output as JSON
		593	#[arg(long)]
		594	json: bool,
452	},	595	},
453	/// Close an issue	596	/// Close an issue
454	Close {	597	Close {
@@ -457,6 +600,9 @@ pub enum IssueCmd {
457	/// Reason for closing	600	/// Reason for closing
458	#[arg(short, long)]	601	#[arg(short, long)]
459	reason: Option<String>,	602	reason: Option<String>,
		603	/// Output as JSON
		604	#[arg(long)]
		605	json: bool,
460	},	606	},
461	/// Delete an issue (removes the local collab ref)	607	/// Delete an issue (removes the local collab ref)
462	///	608	///
@@ -466,11 +612,17 @@ pub enum IssueCmd {
466	Delete {	612	Delete {
467	/// Issue ID (prefix match)	613	/// Issue ID (prefix match)
468	id: String,	614	id: String,
		615	/// Output as JSON
		616	#[arg(long)]
		617	json: bool,
469	},	618	},
470	/// Reopen a closed issue	619	/// Reopen a closed issue
471	Reopen {	620	Reopen {
472	/// Issue ID (prefix match)	621	/// Issue ID (prefix match)
473	id: String,	622	id: String,
		623	/// Output as JSON
		624	#[arg(long)]
		625	json: bool,
474	},	626	},
475	}	627	}
476		628
@@ -486,6 +638,9 @@ pub enum KeyCmd {
486	/// Overwrite existing key files	638	/// Overwrite existing key files
487	#[arg(long)]	639	#[arg(long)]
488	force: bool,	640	force: bool,
		641	/// Output as JSON
		642	#[arg(long)]
		643	json: bool,
489	},	644	},
490		645
491	/// Add a trusted public key	646	/// Add a trusted public key
@@ -501,6 +656,9 @@ pub enum KeyCmd {
501	/// Store in global trust store (~/.config/git-collab/trusted-keys)	656	/// Store in global trust store (~/.config/git-collab/trusted-keys)
502	#[arg(long)]	657	#[arg(long)]
503	global: bool,	658	global: bool,
		659	/// Output as JSON
		660	#[arg(long)]
		661	json: bool,
504	},	662	},
505	/// List trusted public keys	663	/// List trusted public keys
506	#[command(alias = "ls")]	664	#[command(alias = "ls")]
@@ -517,6 +675,9 @@ pub enum KeyCmd {
517	/// Remove from global trust store (~/.config/git-collab/trusted-keys)	675	/// Remove from global trust store (~/.config/git-collab/trusted-keys)
518	#[arg(long)]	676	#[arg(long)]
519	global: bool,	677	global: bool,
		678	/// Output as JSON
		679	#[arg(long)]
		680	json: bool,
520	},	681	},
521	}	682	}
522		683
@@ -544,6 +705,9 @@ pub enum PatchCmd {
544	/// Issue ID this patch fixes (auto-closes on merge; must resolve to one existing issue)	705	/// Issue ID this patch fixes (auto-closes on merge; must resolve to one existing issue)
545	#[arg(long)]	706	#[arg(long)]
546	fixes: Option<String>,	707	fixes: Option<String>,
		708	/// Output as JSON
		709	#[arg(long)]
		710	json: bool,
547	},	711	},
548	/// List patches	712	/// List patches
549	#[command(alias = "ls")]	713	#[command(alias = "ls")]
@@ -628,6 +792,9 @@ pub enum PatchCmd {
628	/// Mark the comment a suggestion rather than something that must change	792	/// Mark the comment a suggestion rather than something that must change
629	#[arg(long)]	793	#[arg(long)]
630	non_blocking: bool,	794	non_blocking: bool,
		795	/// Output as JSON
		796	#[arg(long)]
		797	json: bool,
631	},	798	},
632	/// Review a patch	799	/// Review a patch
633	///	800	///
@@ -646,6 +813,9 @@ pub enum PatchCmd {
646	/// Target revision for review	813	/// Target revision for review
647	#[arg(long)]	814	#[arg(long)]
648	revision: Option<u32>,	815	revision: Option<u32>,
		816	/// Output as JSON
		817	#[arg(long)]
		818	json: bool,
649	},	819	},
650	/// Revise a patch (record a new revision snapshot)	820	/// Revise a patch (record a new revision snapshot)
651	#[command(alias = "update")]	821	#[command(alias = "update")]
@@ -660,6 +830,9 @@ pub enum PatchCmd {
660	/// Source branch to snapshot (defaults to HEAD)	830	/// Source branch to snapshot (defaults to HEAD)
661	#[arg(short = 'B', long)]	831	#[arg(short = 'B', long)]
662	branch: Option<String>,	832	branch: Option<String>,
		833	/// Output as JSON
		834	#[arg(long)]
		835	json: bool,
663	},	836	},
664	/// Correct the text of a comment or review you wrote	837	/// Correct the text of a comment or review you wrote
665	///	838	///
@@ -678,6 +851,9 @@ pub enum PatchCmd {
678	body: Option<String>,	851	body: Option<String>,
679	#[arg(short = 'F', long, help = BODY_FILE_HELP)]	852	#[arg(short = 'F', long, help = BODY_FILE_HELP)]
680	body_file: Option<String>,	853	body_file: Option<String>,
		854	/// Output as JSON
		855	#[arg(long)]
		856	json: bool,
681	},	857	},
682	/// Delete a comment you wrote, leaving a tombstone in its place	858	/// Delete a comment you wrote, leaving a tombstone in its place
683	///	859	///
@@ -689,6 +865,9 @@ pub enum PatchCmd {
689	id: String,	865	id: String,
690	/// Comment ID (prefix match), as printed by `patch show`	866	/// Comment ID (prefix match), as printed by `patch show`
691	comment: String,	867	comment: String,
		868	/// Output as JSON
		869	#[arg(long)]
		870	json: bool,
692	},	871	},
693	/// Correct a revision's description	872	/// Correct a revision's description
694	///	873	///
@@ -706,6 +885,9 @@ pub enum PatchCmd {
706	body: Option<String>,	885	body: Option<String>,
707	#[arg(short = 'F', long, help = BODY_FILE_HELP)]	886	#[arg(short = 'F', long, help = BODY_FILE_HELP)]
708	body_file: Option<String>,	887	body_file: Option<String>,
		888	/// Output as JSON
		889	#[arg(long)]
		890	json: bool,
709	},	891	},
710	/// Show revision log for a patch	892	/// Show revision log for a patch
711	///	893	///
@@ -729,6 +911,9 @@ pub enum PatchCmd {
729	id: String,	911	id: String,
730	/// Label to add	912	/// Label to add
731	label: String,	913	label: String,
		914	/// Output as JSON
		915	#[arg(long)]
		916	json: bool,
732	},	917	},
733	/// Remove a label from a patch	918	/// Remove a label from a patch
734	Unlabel {	919	Unlabel {
@@ -736,6 +921,9 @@ pub enum PatchCmd {
736	id: String,	921	id: String,
737	/// Label to remove	922	/// Label to remove
738	label: String,	923	label: String,
		924	/// Output as JSON
		925	#[arg(long)]
		926	json: bool,
739	},	927	},
740	/// Record that a patch was merged into its base branch	928	/// Record that a patch was merged into its base branch
741	///	929	///
@@ -752,6 +940,9 @@ pub enum PatchCmd {
752	/// Record the merge without closing the issue named by --fixes	940	/// Record the merge without closing the issue named by --fixes
753	#[arg(long)]	941	#[arg(long)]
754	no_close: bool,	942	no_close: bool,
		943	/// Output as JSON
		944	#[arg(long)]
		945	json: bool,
755	},	946	},
756	/// Close a patch	947	/// Close a patch
757	Close {	948	Close {
@@ -760,18 +951,39 @@ pub enum PatchCmd {
760	/// Reason for closing	951	/// Reason for closing
761	#[arg(short, long)]	952	#[arg(short, long)]
762	reason: Option<String>,	953	reason: Option<String>,
		954	/// Output as JSON
		955	#[arg(long)]
		956	json: bool,
		957	},
		958	/// Reopen a closed or merged patch
		959	///
		960	/// Clears the recorded merge commit, if there was one: an open patch that
		961	/// still names the commit that landed it would be two answers to one
		962	/// question. Record the merge again to restore it.
		963	Reopen {
		964	/// Patch ID (prefix match)
		965	id: String,
		966	/// Output as JSON
		967	#[arg(long)]
		968	json: bool,
763	},	969	},
764	/// Delete a patch (removes the local collab ref)	970	/// Delete a patch (removes the local collab ref)
765	#[command(alias = "remove", alias = "rm")]	971	#[command(alias = "remove", alias = "rm")]
766	Delete {	972	Delete {
767	/// Patch ID (prefix match)	973	/// Patch ID (prefix match)
768	id: String,	974	id: String,
		975	/// Output as JSON
		976	#[arg(long)]
		977	json: bool,
769	},	978	},
770	/// Check out a patch's latest revision as a local branch	979	/// Check out a patch's latest revision as a local branch
771	#[command(alias = "co")]	980	#[command(alias = "co")]
772	Checkout {	981	Checkout {
773	/// Patch ID (prefix match)	982	/// Patch ID (prefix match)
774	id: String,	983	id: String,
		984	/// Output as JSON
		985	#[arg(long)]
		986	json: bool,
775	},	987	},
776	}	988	}
777		989
@@ -807,6 +1019,7 @@ impl Commands {
807	\| PatchCmd::Unlabel { .. }	1019	\| PatchCmd::Unlabel { .. }
808	\| PatchCmd::Close { .. }	1020	\| PatchCmd::Close { .. }
809	\| PatchCmd::Merge { .. }	1021	\| PatchCmd::Merge { .. }
		1022	\| PatchCmd::Reopen { .. }
810	),	1023	),
811	_ => false,	1024	_ => false,
812	}	1025	}
@@ -829,6 +1042,9 @@ pub enum ReleaseCmd {
829	/// Remote name	1042	/// Remote name
830	#[arg(long, default_value = "origin")]	1043	#[arg(long, default_value = "origin")]
831	remote: String,	1044	remote: String,
		1045	/// Output as JSON
		1046	#[arg(long)]
		1047	json: bool,
832	},	1048	},
833	/// List releases on the server	1049	/// List releases on the server
834	#[command(alias = "ls")]	1050	#[command(alias = "ls")]
@@ -850,6 +1066,9 @@ pub enum ReleaseCmd {
850	/// Remote name	1066	/// Remote name
851	#[arg(long, default_value = "origin")]	1067	#[arg(long, default_value = "origin")]
852	remote: String,	1068	remote: String,
		1069	/// Output as JSON
		1070	#[arg(long)]
		1071	json: bool,
853	},	1072	},
854	}	1073	}
855		1074
@@ -863,12 +1082,18 @@ pub enum IdentityCmd {
863	Alias {	1082	Alias {
864	/// Email address to add as alias	1083	/// Email address to add as alias
865	email: String,	1084	email: String,
		1085	/// Output as JSON
		1086	#[arg(long)]
		1087	json: bool,
866	},	1088	},
867	/// Remove an email alias	1089	/// Remove an email alias
868	#[command(alias = "remove", alias = "rm", alias = "delete")]	1090	#[command(alias = "remove", alias = "rm", alias = "delete")]
869	Unalias {	1091	Unalias {
870	/// Email address to remove	1092	/// Email address to remove
871	email: String,	1093	email: String,
		1094	/// Output as JSON
		1095	#[arg(long)]
		1096	json: bool,
872	},	1097	},
873	/// Show current identity and aliases	1098	/// Show current identity and aliases
874	#[command(alias = "ls", alias = "show")]	1099	#[command(alias = "ls", alias = "show")]

src/dag.rs

Old		New
@@ -147,6 +147,30 @@ pub fn append_action(repo: &Repository, ref_name: &str, action: Action) -> Resul
147	append_event(repo, ref_name, &event, &sk)	147	append_event(repo, ref_name, &event, &sk)
148	}	148	}
149		149
		150	/// An event appended to a collab object's DAG.
		151	///
		152	/// Both identifiers are full and stay full. Abbreviation is a display policy —
		153	/// it is sized to the repository and can be overridden by `collab.abbrev` — so
		154	/// it belongs where a person reads output, never in what a caller stores or
		155	/// passes back in. `--json` carries these verbatim; the prose abbreviates them
		156	/// on the way out.
		157	pub struct Recorded {
		158	/// The issue or patch the event was appended to.
		159	pub id: String,
		160	/// The event's own commit OID, which is how a caller names it again — to
		161	/// `edit-comment`, or to find it in `git-collab log`.
		162	pub event: Oid,
		163	}
		164
		165	/// A [`Recorded`] that supersedes an earlier event, naming the one it corrects.
		166	pub struct Corrected {
		167	pub id: String,
		168	/// The event whose body this one replaces or tombstones, in full — resolved
		169	/// from whatever prefix the caller gave.
		170	pub target: String,
		171	pub event: Oid,
		172	}
		173
150	/// Convenience wrapper: load signing key, build event, and create a root	174	/// Convenience wrapper: load signing key, build event, and create a root
151	/// (orphan) DAG commit. Returns the new commit OID (entity ID).	175	/// (orphan) DAG commit. Returns the new commit OID (entity ID).
152	pub fn create_root_action(repo: &Repository, action: Action) -> Result<Oid, Error> {	176	pub fn create_root_action(repo: &Repository, action: Action) -> Result<Oid, Error> {
@@ -338,6 +362,7 @@ fn commit_message(action: &Action) -> String {
338	}	362	}
339	Action::PatchClose { .. } => "patch: close".to_string(),	363	Action::PatchClose { .. } => "patch: close".to_string(),
340	Action::PatchMerge { .. } => "patch: merge".to_string(),	364	Action::PatchMerge { .. } => "patch: merge".to_string(),
		365	Action::PatchReopen => "patch: reopen".to_string(),
341	Action::BodyEdit { target, .. } => format!("collab: edit body of {:.8}", target),	366	Action::BodyEdit { target, .. } => format!("collab: edit body of {:.8}", target),
342	Action::CommentDelete { target } => format!("collab: delete comment {:.8}", target),	367	Action::CommentDelete { target } => format!("collab: delete comment {:.8}", target),
343	Action::Merge => "collab: merge".to_string(),	368	Action::Merge => "collab: merge".to_string(),

src/event.rs

Old		New
@@ -169,6 +169,20 @@ pub enum Action {
169	#[serde(default, skip_serializing_if = "String::is_empty")]	169	#[serde(default, skip_serializing_if = "String::is_empty")]
170	commit: String,	170	commit: String,
171	},	171	},
		172	/// A patch returns to `open`, undoing whichever `PatchClose` or `PatchMerge`
		173	/// currently holds the status.
		174	///
		175	/// It carries no payload for the same reason `IssueReopen` does not: there
		176	/// is exactly one thing to say, and the status fold resolves it by
		177	/// `(clock, oid)` like every other status-changing event — so a reopen on
		178	/// one clone and a close on another converge without either being
		179	/// privileged.
		180	///
		181	/// Reopening clears `merge_commit`, exactly as `PatchClose` does. See the
		182	/// fold in `state::PatchState::from_ref_uncached` for why the two fields
		183	/// move together.
		184	#[serde(rename = "patch.reopen")]
		185	PatchReopen,
172	/// Supersede the body of an earlier event in the same DAG.	186	/// Supersede the body of an earlier event in the same DAG.
173	///	187	///
174	/// Prose enters this tool once and used to be stuck there. This is the	188	/// Prose enters this tool once and used to be stuck there. This is the

src/hooks.rs

@@ @@ -224,6 +224,26 @@ fn current_exe() -> PathBuf { @@
+/// Print what [`install`] did, in `init`'s reporting style.
+/// The `--json` shape of an install: what happened, and to which file.
+///
+/// The outcome is named rather than inferred from the exit code, because three
+/// of the four are successes and they mean different things — "I wrote it",
+/// "it was already mine", "somebody else's already calls me", and "somebody
+/// else's is in the way and I touched nothing".
+pub fn install_json(outcome: &InstallOutcome) -> serde_json::Value {
+    let (name, path) = match outcome {
+        InstallOutcome::Installed(path) => ("installed", path),
+        InstallOutcome::AlreadyInstalled(path) => ("already-installed", path),
+        InstallOutcome::ForeignHookCallsUs(path) => ("foreign-hook-calls-us", path),
+        InstallOutcome::Foreign(path) => ("foreign", path),
+    };
+    serde_json::json!({
+        "action": "hooks.install",
+        "outcome": name,
+        "path": path.display().to_string(),
+    })
+}
+pub fn report_install(outcome: &InstallOutcome) {
+    match outcome {
+        InstallOutcome::Installed(path) => {

src/issue.rs

Old		New
@@ -151,84 +151,100 @@ pub fn show(repo: &Repository, id_prefix: &str) -> Result<IssueState, crate::err
151	Ok(issue)	151	Ok(issue)
152	}	152	}
153		153
154	pub fn label(repo: &Repository, id_prefix: &str, label: &str) -> Result<(), crate::error::Error> {	154	pub fn label(
155	let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;	155	repo: &Repository,
156	dag::append_action(	156	id_prefix: &str,
		157	label: &str,
		158	) -> Result<dag::Recorded, crate::error::Error> {
		159	let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?;
		160	let event = dag::append_action(
157	repo,	161	repo,
158	&ref_name,	162	&ref_name,
159	Action::IssueLabel {	163	Action::IssueLabel {
160	label: label.to_string(),	164	label: label.to_string(),
161	},	165	},
162	)?;	166	)?;
163	Ok(())	167	Ok(dag::Recorded { id, event })
164	}	168	}
165		169
166	pub fn unlabel(repo: &Repository, id_prefix: &str, label: &str) -> Result<(), crate::error::Error> {	170	pub fn unlabel(
167	let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;	171	repo: &Repository,
168	dag::append_action(	172	id_prefix: &str,
		173	label: &str,
		174	) -> Result<dag::Recorded, crate::error::Error> {
		175	let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?;
		176	let event = dag::append_action(
169	repo,	177	repo,
170	&ref_name,	178	&ref_name,
171	Action::IssueUnlabel {	179	Action::IssueUnlabel {
172	label: label.to_string(),	180	label: label.to_string(),
173	},	181	},
174	)?;	182	)?;
175	Ok(())	183	Ok(dag::Recorded { id, event })
176	}	184	}
177		185
178	pub fn relate(repo: &Repository, id_prefix: &str, other: &str) -> Result<(), crate::error::Error> {	186	pub fn relate(
179	let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;	187	repo: &Repository,
180	dag::append_action(	188	id_prefix: &str,
		189	other: &str,
		190	) -> Result<dag::Recorded, crate::error::Error> {
		191	let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?;
		192	let event = dag::append_action(
181	repo,	193	repo,
182	&ref_name,	194	&ref_name,
183	Action::IssueRelate {	195	Action::IssueRelate {
184	relates_to: other.to_string(),	196	relates_to: other.to_string(),
185	},	197	},
186	)?;	198	)?;
187	Ok(())	199	Ok(dag::Recorded { id, event })
188	}	200	}
189		201
190	pub fn unrelate(repo: &Repository, id_prefix: &str, other: &str) -> Result<(), crate::error::Error> {	202	pub fn unrelate(
191	let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;	203	repo: &Repository,
192	dag::append_action(	204	id_prefix: &str,
		205	other: &str,
		206	) -> Result<dag::Recorded, crate::error::Error> {
		207	let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?;
		208	let event = dag::append_action(
193	repo,	209	repo,
194	&ref_name,	210	&ref_name,
195	Action::IssueUnrelate {	211	Action::IssueUnrelate {
196	relates_to: other.to_string(),	212	relates_to: other.to_string(),
197	},	213	},
198	)?;	214	)?;
199	Ok(())	215	Ok(dag::Recorded { id, event })
200	}	216	}
201		217
202	pub fn assign(	218	pub fn assign(
203	repo: &Repository,	219	repo: &Repository,
204	id_prefix: &str,	220	id_prefix: &str,
205	assignee: &str,	221	assignee: &str,
206	) -> Result<(), crate::error::Error> {	222	) -> Result<dag::Recorded, crate::error::Error> {
207	let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;	223	let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?;
208	dag::append_action(	224	let event = dag::append_action(
209	repo,	225	repo,
210	&ref_name,	226	&ref_name,
211	Action::IssueAssign {	227	Action::IssueAssign {
212	assignee: assignee.to_string(),	228	assignee: assignee.to_string(),
213	},	229	},
214	)?;	230	)?;
215	Ok(())	231	Ok(dag::Recorded { id, event })
216	}	232	}
217		233
218	pub fn unassign(	234	pub fn unassign(
219	repo: &Repository,	235	repo: &Repository,
220	id_prefix: &str,	236	id_prefix: &str,
221	assignee: &str,	237	assignee: &str,
222	) -> Result<(), crate::error::Error> {	238	) -> Result<dag::Recorded, crate::error::Error> {
223	let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;	239	let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?;
224	dag::append_action(	240	let event = dag::append_action(
225	repo,	241	repo,
226	&ref_name,	242	&ref_name,
227	Action::IssueUnassign {	243	Action::IssueUnassign {
228	assignee: assignee.to_string(),	244	assignee: assignee.to_string(),
229	},	245	},
230	)?;	246	)?;
231	Ok(())	247	Ok(dag::Recorded { id, event })
232	}	248	}
233		249
234	pub fn edit(	250	pub fn edit(
@@ -236,14 +252,14 @@ pub fn edit(
236	id_prefix: &str,	252	id_prefix: &str,
237	title: Option<&str>,	253	title: Option<&str>,
238	body: Option<&str>,	254	body: Option<&str>,
239	) -> Result<(), crate::error::Error> {	255	) -> Result<dag::Recorded, crate::error::Error> {
240	if title.is_none() && body.is_none() {	256	if title.is_none() && body.is_none() {
241	return Err(	257	return Err(
242	git2::Error::from_str("at least one of --title or --body must be provided").into(),	258	git2::Error::from_str("at least one of --title or --body must be provided").into(),
243	);	259	);
244	}	260	}
245	let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;	261	let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?;
246	dag::append_action(	262	let event = dag::append_action(
247	repo,	263	repo,
248	&ref_name,	264	&ref_name,
249	Action::IssueEdit {	265	Action::IssueEdit {
@@ -251,7 +267,7 @@ pub fn edit(
251	body: body.map(\|s\| s.to_string()),	267	body: body.map(\|s\| s.to_string()),
252	},	268	},
253	)?;	269	)?;
254	Ok(())	270	Ok(dag::Recorded { id, event })
255	}	271	}
256		272
257	/// Correct the text of a comment.	273	/// Correct the text of a comment.
@@ -264,22 +280,26 @@ pub fn edit_comment(
264	id_prefix: &str,	280	id_prefix: &str,
265	comment_prefix: &str,	281	comment_prefix: &str,
266	body_args: &crate::body::BodyArgs,	282	body_args: &crate::body::BodyArgs,
267	) -> Result<(), crate::error::Error> {	283	) -> Result<dag::Corrected, crate::error::Error> {
268	let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?;	284	let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?;
269	let issue = IssueState::from_ref(repo, &ref_name, &id)?;	285	let issue = IssueState::from_ref(repo, &ref_name, &id)?;
270	let target = issue.resolve_comment(comment_prefix)?;	286	let target = issue.resolve_comment(comment_prefix)?;
271	crate::patch::require_own_body(repo, &target)?;	287	crate::patch::require_own_body(repo, &target)?;
272		288
273	let body = crate::body::resolve_required(body_args, &target.body, "comment")?;	289	let body = crate::body::resolve_required(body_args, &target.body, "comment")?;
274	dag::append_action(	290	let event = dag::append_action(
275	repo,	291	repo,
276	&ref_name,	292	&ref_name,
277	Action::BodyEdit {	293	Action::BodyEdit {
278	target: target.oid,	294	target: target.oid.clone(),
279	body,	295	body,
280	},	296	},
281	)?;	297	)?;
282	Ok(())	298	Ok(dag::Corrected {
		299	id,
		300	target: target.oid,
		301	event,
		302	})
283	}	303	}
284		304
285	/// Tombstone a comment: drop its text, keep its slot.	305	/// Tombstone a comment: drop its text, keep its slot.
@@ -287,42 +307,50 @@ pub fn delete_comment(
287	repo: &Repository,	307	repo: &Repository,
288	id_prefix: &str,	308	id_prefix: &str,
289	comment_prefix: &str,	309	comment_prefix: &str,
290	) -> Result<(), crate::error::Error> {	310	) -> Result<dag::Corrected, crate::error::Error> {
291	let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?;	311	let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?;
292	let issue = IssueState::from_ref(repo, &ref_name, &id)?;	312	let issue = IssueState::from_ref(repo, &ref_name, &id)?;
293	let target = issue.resolve_comment(comment_prefix)?;	313	let target = issue.resolve_comment(comment_prefix)?;
294	crate::patch::require_own_body(repo, &target)?;	314	crate::patch::require_own_body(repo, &target)?;
295	crate::patch::require_deletable(&target)?;	315	crate::patch::require_deletable(&target)?;
296		316
297	dag::append_action(	317	let event = dag::append_action(
298	repo,	318	repo,
299	&ref_name,	319	&ref_name,
300	Action::CommentDelete {	320	Action::CommentDelete {
301	target: target.oid,	321	target: target.oid.clone(),
302	},	322	},
303	)?;	323	)?;
304	Ok(())	324	Ok(dag::Corrected {
		325	id,
		326	target: target.oid,
		327	event,
		328	})
305	}	329	}
306		330
307	pub fn comment(repo: &Repository, id_prefix: &str, body: &str) -> Result<(), crate::error::Error> {	331	pub fn comment(
308	let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;	332	repo: &Repository,
309	dag::append_action(	333	id_prefix: &str,
		334	body: &str,
		335	) -> Result<dag::Recorded, crate::error::Error> {
		336	let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?;
		337	let event = dag::append_action(
310	repo,	338	repo,
311	&ref_name,	339	&ref_name,
312	Action::IssueComment {	340	Action::IssueComment {
313	body: body.to_string(),	341	body: body.to_string(),
314	},	342	},
315	)?;	343	)?;
316	Ok(())	344	Ok(dag::Recorded { id, event })
317	}	345	}
318		346
319	pub fn close(	347	pub fn close(
320	repo: &Repository,	348	repo: &Repository,
321	id_prefix: &str,	349	id_prefix: &str,
322	reason: Option<&str>,	350	reason: Option<&str>,
323	) -> Result<(), crate::error::Error> {	351	) -> Result<dag::Recorded, crate::error::Error> {
324	let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?;	352	let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?;
325	dag::append_action(	353	let event = dag::append_action(
326	repo,	354	repo,
327	&ref_name,	355	&ref_name,
328	Action::IssueClose {	356	Action::IssueClose {
@@ -333,7 +361,7 @@ pub fn close(
333	if ref_name.starts_with("refs/collab/issues/") {	361	if ref_name.starts_with("refs/collab/issues/") {
334	state::archive_issue_ref(repo, &id)?;	362	state::archive_issue_ref(repo, &id)?;
335	}	363	}
336	Ok(())	364	Ok(dag::Recorded { id, event })
337	}	365	}
338		366
339	pub fn delete(repo: &Repository, id_prefix: &str) -> Result<String, crate::error::Error> {	367	pub fn delete(repo: &Repository, id_prefix: &str) -> Result<String, crate::error::Error> {
@@ -342,13 +370,13 @@ pub fn delete(repo: &Repository, id_prefix: &str) -> Result<String, crate::error
342	Ok(id)	370	Ok(id)
343	}	371	}
344		372
345	pub fn reopen(repo: &Repository, id_prefix: &str) -> Result<(), crate::error::Error> {	373	pub fn reopen(repo: &Repository, id_prefix: &str) -> Result<dag::Recorded, crate::error::Error> {
346	let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?;	374	let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?;
347	dag::append_action(repo, &ref_name, Action::IssueReopen)?;	375	let event = dag::append_action(repo, &ref_name, Action::IssueReopen)?;
348	// Move the ref back out of the archive namespace (undoes what close() did),	376	// Move the ref back out of the archive namespace (undoes what close() did),
349	// so the reopened issue is visible to the default `issue list` again.	377	// so the reopened issue is visible to the default `issue list` again.
350	if ref_name.starts_with("refs/collab/archive/issues/") {	378	if ref_name.starts_with("refs/collab/archive/issues/") {
351	state::unarchive_issue_ref(repo, &id)?;	379	state::unarchive_issue_ref(repo, &id)?;
352	}	380	}
353	Ok(())	381	Ok(dag::Recorded { id, event })
354	}	382	}

src/lib.rs

Old		New
@@ -35,7 +35,7 @@ use git2::Repository;
35	/// Shared by the top-level `init-key` and by `key generate`, which exist as	35	/// Shared by the top-level `init-key` and by `key generate`, which exist as
36	/// two spellings of one command because key generation sits at the top level	36	/// two spellings of one command because key generation sits at the top level
37	/// while key management sits under `key`.	37	/// while key management sits under `key`.
38	fn generate_signing_key(force: bool) -> Result<(), error::Error> {	38	fn generate_signing_key(force: bool, json: bool) -> Result<(), error::Error> {
39	let config_dir = signing::signing_key_dir()?;	39	let config_dir = signing::signing_key_dir()?;
40	let sk_path = config_dir.join("signing-key");	40	let sk_path = config_dir.join("signing-key");
41	if sk_path.exists() && !force {	41	if sk_path.exists() && !force {
@@ -46,9 +46,11 @@ fn generate_signing_key(force: bool) -> Result<(), error::Error> {
46		46
47	let vk = signing::generate_keypair(&config_dir)?;	47	let vk = signing::generate_keypair(&config_dir)?;
48	let pubkey_b64 = base64::engine::general_purpose::STANDARD.encode(vk.to_bytes());	48	let pubkey_b64 = base64::engine::general_purpose::STANDARD.encode(vk.to_bytes());
49	println!("Signing key generated.");	49	report(
50	println!("Public key: {}", pubkey_b64);	50	json,
51	Ok(())	51	\|\| serde_json::json!({ "action": "key.generate", "pubkey": pubkey_b64 }),
		52	\|\| format!("Signing key generated.\nPublic key: {}", pubkey_b64),
		53	)
52	}	54	}
53		55
54	/// Check if the reviewer's base ref has moved ahead of the patch's latest revision.	56	/// Check if the reviewer's base ref has moved ahead of the patch's latest revision.
@@ -160,14 +162,46 @@ fn maybe_auto_sync(repo: &Repository) {
160	});	162	});
161	}	163	}
162		164
		165	/// Report a write command's result on stdout: one JSON object under `--json`,
		166	/// the prose line otherwise, and never both.
		167	///
		168	/// Exactly one value on stdout is the half of the contract a caller depends on
		169	/// — `serde_json` rejects trailing content, so anything else printed alongside
		170	/// would break every reader. The other half is already in place: auto-sync
		171	/// narrates on stderr under its own prefix (see [`output`]), so the push a
		172	/// write triggers cannot get in here.
		173	///
		174	/// Both sides are closures because each is wasted work when the other wins:
		175	/// the prose abbreviates ids, and sizing an abbreviation means reading the ref
		176	/// namespace. The JSON side never abbreviates anything — abbreviation is a
		177	/// display policy, and `--json` is not display.
		178	fn report(
		179	json: bool,
		180	value: impl FnOnce() -> serde_json::Value,
		181	prose: impl FnOnce() -> String,
		182	) -> Result<(), error::Error> {
		183	if json {
		184	println!("{}", value());
		185	} else {
		186	println!("{}", prose());
		187	}
		188	Ok(())
		189	}
		190
163	pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {	191	pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
164	let is_write = cli.command.is_write();	192	let is_write = cli.command.is_write();
165	match cli.command {	193	match cli.command {
166	Commands::Init => sync::init(repo),	194	Commands::Init => sync::init(repo),
167	Commands::Hooks(cmd) => match cmd {	195	Commands::Hooks(cmd) => match cmd {
168	HookCmd::Install => {	196	HookCmd::Install { json } => {
169	let outcome = hooks::install(repo)?;	197	let outcome = hooks::install(repo)?;
170	hooks::report_install(&outcome);	198	if json {
		199	println!("{}", hooks::install_json(&outcome));
		200	} else {
		201	// Multi-line and, for a foreign hook, advisory — not a
		202	// single result line, so it keeps its own printer.
		203	hooks::report_install(&outcome);
		204	}
171	Ok(())	205	Ok(())
172	}	206	}
173	HookCmd::Status => hooks::status(repo),	207	HookCmd::Status => hooks::status(repo),
@@ -184,12 +218,16 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
184	body,	218	body,
185	body_file,	219	body_file,
186	relates_to,	220	relates_to,
		221	json,
187	} => {	222	} => {
188	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());	223	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());
189	let body = body::resolve_optional(&args)?.unwrap_or_default();	224	let body = body::resolve_optional(&args)?.unwrap_or_default();
190	let id = issue::open(repo, &title, &body, relates_to.as_deref())?;	225	let id = issue::open(repo, &title, &body, relates_to.as_deref())?;
191	println!("Opened issue {}", abbrev::for_issues(repo).of(&id));	226	report(
192	Ok(())	227	json,
		228	\|\| serde_json::json!({ "action": "issue.open", "issue": id }),
		229	\|\| format!("Opened issue {}", abbrev::for_issues(repo).of(&id)),
		230	)
193	}	231	}
194	IssueCmd::List {	232	IssueCmd::List {
195	all,	233	all,
@@ -307,92 +345,222 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
307	}	345	}
308	Ok(())	346	Ok(())
309	}	347	}
310	IssueCmd::Label { id, label } => {	348	IssueCmd::Label { id, label, json } => {
311	issue::label(repo, &id, &label)?;	349	let r = issue::label(repo, &id, &label)?;
312	println!("Label '{}' added.", label);	350	report(
313	Ok(())	351	json,
		352	\|\| {
		353	serde_json::json!({
		354	"action": "issue.label",
		355	"issue": r.id,
		356	"label": label,
		357	"event": r.event.to_string(),
		358	})
		359	},
		360	\|\| format!("Label '{}' added.", label),
		361	)
314	}	362	}
315	IssueCmd::Unlabel { id, label } => {	363	IssueCmd::Unlabel { id, label, json } => {
316	issue::unlabel(repo, &id, &label)?;	364	let r = issue::unlabel(repo, &id, &label)?;
317	println!("Label '{}' removed.", label);	365	report(
318	Ok(())	366	json,
		367	\|\| {
		368	serde_json::json!({
		369	"action": "issue.unlabel",
		370	"issue": r.id,
		371	"label": label,
		372	"event": r.event.to_string(),
		373	})
		374	},
		375	\|\| format!("Label '{}' removed.", label),
		376	)
319	}	377	}
320	IssueCmd::Relate { id, other } => {	378	IssueCmd::Relate { id, other, json } => {
321	issue::relate(repo, &id, &other)?;	379	let r = issue::relate(repo, &id, &other)?;
322	println!("Related to '{}'.", other);	380	report(
323	Ok(())	381	json,
		382	\|\| {
		383	serde_json::json!({
		384	"action": "issue.relate",
		385	"issue": r.id,
		386	// As recorded, not as resolved: the event stores
		387	// whatever was typed, and reporting a full id the
		388	// DAG does not contain would be a lie about what
		389	// was written.
		390	"relates_to": other,
		391	"event": r.event.to_string(),
		392	})
		393	},
		394	\|\| format!("Related to '{}'.", other),
		395	)
324	}	396	}
325	IssueCmd::Unrelate { id, other } => {	397	IssueCmd::Unrelate { id, other, json } => {
326	issue::unrelate(repo, &id, &other)?;	398	let r = issue::unrelate(repo, &id, &other)?;
327	println!("Unrelated from '{}'.", other);	399	report(
328	Ok(())	400	json,
		401	\|\| {
		402	serde_json::json!({
		403	"action": "issue.unrelate",
		404	"issue": r.id,
		405	"relates_to": other,
		406	"event": r.event.to_string(),
		407	})
		408	},
		409	\|\| format!("Unrelated from '{}'.", other),
		410	)
329	}	411	}
330	IssueCmd::Assign { id, name } => {	412	IssueCmd::Assign { id, name, json } => {
331	issue::assign(repo, &id, &name)?;	413	let r = issue::assign(repo, &id, &name)?;
332	println!("Assigned to '{}'.", name);	414	report(
333	Ok(())	415	json,
		416	\|\| {
		417	serde_json::json!({
		418	"action": "issue.assign",
		419	"issue": r.id,
		420	"assignee": name,
		421	"event": r.event.to_string(),
		422	})
		423	},
		424	\|\| format!("Assigned to '{}'.", name),
		425	)
334	}	426	}
335	IssueCmd::Unassign { id, name } => {	427	IssueCmd::Unassign { id, name, json } => {
336	issue::unassign(repo, &id, &name)?;	428	let r = issue::unassign(repo, &id, &name)?;
337	println!("Unassigned '{}'.", name);	429	report(
338	Ok(())	430	json,
		431	\|\| {
		432	serde_json::json!({
		433	"action": "issue.unassign",
		434	"issue": r.id,
		435	"assignee": name,
		436	"event": r.event.to_string(),
		437	})
		438	},
		439	\|\| format!("Unassigned '{}'.", name),
		440	)
339	}	441	}
340	IssueCmd::Edit {	442	IssueCmd::Edit {
341	id,	443	id,
342	title,	444	title,
343	body,	445	body,
344	body_file,	446	body_file,
		447	json,
345	} => {	448	} => {
346	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());	449	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());
347	let body = body::resolve_optional(&args)?;	450	let body = body::resolve_optional(&args)?;
348	issue::edit(repo, &id, title.as_deref(), body.as_deref())?;	451	let r = issue::edit(repo, &id, title.as_deref(), body.as_deref())?;
349	println!("Issue updated.");	452	report(
350	Ok(())	453	json,
		454	\|\| {
		455	serde_json::json!({
		456	"action": "issue.edit",
		457	"issue": r.id,
		458	"event": r.event.to_string(),
		459	})
		460	},
		461	\|\| "Issue updated.".to_string(),
		462	)
351	}	463	}
352	IssueCmd::Comment {	464	IssueCmd::Comment {
353	id,	465	id,
354	body,	466	body,
355	body_file,	467	body_file,
		468	json,
356	} => {	469	} => {
357	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());	470	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());
358	let body = body::resolve_required(&args, "", "comment")?;	471	let body = body::resolve_required(&args, "", "comment")?;
359	issue::comment(repo, &id, &body)?;	472	let r = issue::comment(repo, &id, &body)?;
360	println!("Comment added.");	473	report(
361	Ok(())	474	json,
		475	\|\| {
		476	serde_json::json!({
		477	"action": "issue.comment",
		478	"issue": r.id,
		479	// The comment's own id, which is what
		480	// `issue edit-comment` takes.
		481	"comment": r.event.to_string(),
		482	})
		483	},
		484	\|\| "Comment added.".to_string(),
		485	)
362	}	486	}
363	IssueCmd::EditComment {	487	IssueCmd::EditComment {
364	id,	488	id,
365	comment,	489	comment,
366	body,	490	body,
367	body_file,	491	body_file,
		492	json,
368	} => {	493	} => {
369	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());	494	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());
370	issue::edit_comment(repo, &id, &comment, &args)?;	495	let r = issue::edit_comment(repo, &id, &comment, &args)?;
371	println!("Comment updated.");	496	report(
372	Ok(())	497	json,
		498	\|\| {
		499	serde_json::json!({
		500	"action": "issue.edit_comment",
		501	"issue": r.id,
		502	"comment": r.target,
		503	"event": r.event.to_string(),
		504	})
		505	},
		506	\|\| "Comment updated.".to_string(),
		507	)
373	}	508	}
374	IssueCmd::DeleteComment { id, comment } => {	509	IssueCmd::DeleteComment { id, comment, json } => {
375	issue::delete_comment(repo, &id, &comment)?;	510	let r = issue::delete_comment(repo, &id, &comment)?;
376	println!("Comment deleted.");	511	report(
377	Ok(())	512	json,
		513	\|\| {
		514	serde_json::json!({
		515	"action": "issue.delete_comment",
		516	"issue": r.id,
		517	"comment": r.target,
		518	"event": r.event.to_string(),
		519	})
		520	},
		521	\|\| "Comment deleted.".to_string(),
		522	)
378	}	523	}
379	IssueCmd::Close { id, reason } => {	524	IssueCmd::Close { id, reason, json } => {
380	issue::close(repo, &id, reason.as_deref())?;	525	let r = issue::close(repo, &id, reason.as_deref())?;
381	println!("Issue closed.");	526	report(
382	Ok(())	527	json,
		528	\|\| {
		529	serde_json::json!({
		530	"action": "issue.close",
		531	"issue": r.id,
		532	"status": "closed",
		533	"event": r.event.to_string(),
		534	})
		535	},
		536	\|\| "Issue closed.".to_string(),
		537	)
383	}	538	}
384	IssueCmd::Delete { id } => {	539	IssueCmd::Delete { id, json } => {
385	let full_id = issue::delete(repo, &id)?;	540	let full_id = issue::delete(repo, &id)?;
386	// The issue is gone from the set by now, so this is the	541	report(
387	// `of()` case for an id that is not a member: still widened	542	json,
388	// far enough not to collide with what remains.	543	\|\| serde_json::json!({ "action": "issue.delete", "issue": full_id }),
389	println!("Deleted issue {}", abbrev::for_issues(repo).of(&full_id));	544	// The issue is gone from the set by now, so this is the
390	Ok(())	545	// `of()` case for an id that is not a member: still widened
		546	// far enough not to collide with what remains.
		547	\|\| format!("Deleted issue {}", abbrev::for_issues(repo).of(&full_id)),
		548	)
391	}	549	}
392	IssueCmd::Reopen { id } => {	550	IssueCmd::Reopen { id, json } => {
393	issue::reopen(repo, &id)?;	551	let r = issue::reopen(repo, &id)?;
394	println!("Issue reopened.");	552	report(
395	Ok(())	553	json,
		554	\|\| {
		555	serde_json::json!({
		556	"action": "issue.reopen",
		557	"issue": r.id,
		558	"status": "open",
		559	"event": r.event.to_string(),
		560	})
		561	},
		562	\|\| "Issue reopened.".to_string(),
		563	)
396	}	564	}
397	},	565	},
398	Commands::Patch(cmd) => match cmd {	566	Commands::Patch(cmd) => match cmd {
@@ -403,6 +571,7 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
403	base,	571	base,
404	branch,	572	branch,
405	fixes,	573	fixes,
		574	json,
406	} => {	575	} => {
407	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());	576	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());
408	let body = body::resolve_optional(&args)?.unwrap_or_default();	577	let body = body::resolve_optional(&args)?.unwrap_or_default();
@@ -455,8 +624,29 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
455	}	624	}
456	}	625	}
457	let id = created?;	626	let id = created?;
458	println!("Created patch {}", abbrev::for_patches(repo).of(&id));	627	report(
459	Ok(())	628	json,
		629	\|\| {
		630	// Read back rather than echoed: `--fixes` takes a
		631	// prefix and stores the id it resolved to, and the
		632	// resolved one is what the patch actually carries.
		633	//
		634	// Folded directly rather than through `patch::show`,
		635	// which moves the `seen` read-marker — creating a patch
		636	// is not reading it, and `--json` must not do a thing
		637	// the prose does not.
		638	let fixes =
		639	state::PatchState::from_ref(repo, &state::patch_events_ref(&id), &id)
		640	.ok()
		641	.and_then(\|p\| p.fixes);
		642	serde_json::json!({
		643	"action": "patch.create",
		644	"patch": id,
		645	"fixes": fixes,
		646	})
		647	},
		648	\|\| format!("Created patch {}", abbrev::for_patches(repo).of(&id)),
		649	)
460	}	650	}
461	PatchCmd::List {	651	PatchCmd::List {
462	all,	652	all,
@@ -716,6 +906,7 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
716	line,	906	line,
717	revision,	907	revision,
718	non_blocking,	908	non_blocking,
		909	json,
719	} => {	910	} => {
720	// `--at` and `--file`/`--line` say the same thing; saying it	911	// `--at` and `--file`/`--line` say the same thing; saying it
721	// twice is a mistake, not a merge.	912	// twice is a mistake, not a merge.
@@ -733,7 +924,7 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
733	None => (file, line),	924	None => (file, line),
734	};	925	};
735	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());	926	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());
736	let placement = patch::comment(	927	let r = patch::comment(
737	repo,	928	repo,
738	&id,	929	&id,
739	&args,	930	&args,
@@ -742,8 +933,7 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
742	revision,	933	revision,
743	non_blocking,	934	non_blocking,
744	)?;	935	)?;
745	println!("{}", placement);	936	report(json, \|\| r.to_json(), \|\| r.placement.to_string())
746	Ok(())
747	}	937	}
748	PatchCmd::Review {	938	PatchCmd::Review {
749	id,	939	id,
@@ -751,6 +941,7 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
751	body,	941	body,
752	body_file,	942	body_file,
753	revision,	943	revision,
		944	json,
754	} => {	945	} => {
755	let v: ReviewVerdict = verdict.parse().map_err(\|_\| {	946	let v: ReviewVerdict = verdict.parse().map_err(\|_\| {
756	git2::Error::from_str(	947	git2::Error::from_str(
@@ -759,48 +950,107 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
759	})?;	950	})?;
760	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());	951	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());
761	let body = body::resolve_required(&args, "", "review body")?;	952	let body = body::resolve_required(&args, "", "review body")?;
762	patch::review(repo, &id, v, &body, revision)?;	953	let r = patch::review(repo, &id, v, &body, revision)?;
763	println!("Review submitted.");	954	report(
764	Ok(())	955	json,
		956	\|\| {
		957	serde_json::json!({
		958	"action": "patch.review",
		959	"patch": r.id,
		960	"review": r.event.to_string(),
		961	"verdict": r.verdict.as_str(),
		962	"revision": r.revision,
		963	// A reject closes the patch as a side effect;
		964	// reported so a caller does not have to know that.
		965	"status": if r.closed { "closed" } else { "open" },
		966	})
		967	},
		968	\|\| "Review submitted.".to_string(),
		969	)
765	}	970	}
766	PatchCmd::Revise {	971	PatchCmd::Revise {
767	id,	972	id,
768	body,	973	body,
769	body_file,	974	body_file,
770	branch,	975	branch,
		976	json,
771	} => {	977	} => {
772	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());	978	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());
773	let body = body::resolve_optional(&args)?;	979	let body = body::resolve_optional(&args)?;
774	patch::revise(repo, &id, body.as_deref(), branch.as_deref())?;	980	let r = patch::revise(repo, &id, body.as_deref(), branch.as_deref())?;
775	println!("Patch revised.");	981	report(
776	Ok(())	982	json,
		983	\|\| {
		984	serde_json::json!({
		985	"action": "patch.revision",
		986	"patch": r.id,
		987	"revision": r.number,
		988	"commit": r.commit,
		989	"event": r.event.to_string(),
		990	})
		991	},
		992	\|\| "Patch revised.".to_string(),
		993	)
777	}	994	}
778	PatchCmd::EditComment {	995	PatchCmd::EditComment {
779	id,	996	id,
780	comment,	997	comment,
781	body,	998	body,
782	body_file,	999	body_file,
		1000	json,
783	} => {	1001	} => {
784	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());	1002	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());
785	patch::edit_comment(repo, &id, &comment, &args)?;	1003	let r = patch::edit_comment(repo, &id, &comment, &args)?;
786	println!("Comment updated.");	1004	report(
787	Ok(())	1005	json,
		1006	\|\| {
		1007	serde_json::json!({
		1008	"action": "patch.edit_comment",
		1009	"patch": r.id,
		1010	"comment": r.target,
		1011	"event": r.event.to_string(),
		1012	})
		1013	},
		1014	\|\| "Comment updated.".to_string(),
		1015	)
788	}	1016	}
789	PatchCmd::DeleteComment { id, comment } => {	1017	PatchCmd::DeleteComment { id, comment, json } => {
790	patch::delete_comment(repo, &id, &comment)?;	1018	let r = patch::delete_comment(repo, &id, &comment)?;
791	println!("Comment deleted.");	1019	report(
792	Ok(())	1020	json,
		1021	\|\| {
		1022	serde_json::json!({
		1023	"action": "patch.delete_comment",
		1024	"patch": r.id,
		1025	"comment": r.target,
		1026	"event": r.event.to_string(),
		1027	})
		1028	},
		1029	\|\| "Comment deleted.".to_string(),
		1030	)
793	}	1031	}
794	PatchCmd::EditRevision {	1032	PatchCmd::EditRevision {
795	id,	1033	id,
796	revision,	1034	revision,
797	body,	1035	body,
798	body_file,	1036	body_file,
		1037	json,
799	} => {	1038	} => {
800	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());	1039	let args = body::BodyArgs::new(body.as_deref(), body_file.as_deref());
801	patch::edit_revision(repo, &id, revision, &args)?;	1040	let r = patch::edit_revision(repo, &id, revision, &args)?;
802	println!("Revision description updated.");	1041	report(
803	Ok(())	1042	json,
		1043	\|\| {
		1044	serde_json::json!({
		1045	"action": "patch.edit_revision",
		1046	"patch": r.id,
		1047	"revision": revision,
		1048	"target": r.target,
		1049	"event": r.event.to_string(),
		1050	})
		1051	},
		1052	\|\| "Revision description updated.".to_string(),
		1053	)
804	}	1054	}
805	PatchCmd::Log {	1055	PatchCmd::Log {
806	id,	1056	id,
@@ -825,55 +1075,153 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
825	}	1075	}
826	Ok(())	1076	Ok(())
827	}	1077	}
828	PatchCmd::Label { id, label } => {	1078	PatchCmd::Label { id, label, json } => {
829	patch::label(repo, &id, &label)?;	1079	let r = patch::label(repo, &id, &label)?;
830	println!("Label '{}' added.", label);	1080	report(
831	Ok(())	1081	json,
		1082	\|\| {
		1083	serde_json::json!({
		1084	"action": "patch.label",
		1085	"patch": r.id,
		1086	"label": label,
		1087	"event": r.event.to_string(),
		1088	})
		1089	},
		1090	\|\| format!("Label '{}' added.", label),
		1091	)
832	}	1092	}
833	PatchCmd::Unlabel { id, label } => {	1093	PatchCmd::Unlabel { id, label, json } => {
834	patch::unlabel(repo, &id, &label)?;	1094	let r = patch::unlabel(repo, &id, &label)?;
835	println!("Label '{}' removed.", label);	1095	report(
836	Ok(())	1096	json,
		1097	\|\| {
		1098	serde_json::json!({
		1099	"action": "patch.unlabel",
		1100	"patch": r.id,
		1101	"label": label,
		1102	"event": r.event.to_string(),
		1103	})
		1104	},
		1105	\|\| format!("Label '{}' removed.", label),
		1106	)
837	}	1107	}
838	PatchCmd::Merge {	1108	PatchCmd::Merge {
839	id,	1109	id,
840	commit,	1110	commit,
841	no_close,	1111	no_close,
		1112	json,
842	} => {	1113	} => {
843	let report = patch::merge(repo, &id, commit.as_deref(), !no_close)?;	1114	let merged = patch::merge(repo, &id, commit.as_deref(), !no_close)?;
844	let abbrev = abbrev::for_patches(repo);	1115	let closed = merged.close == merge_scan::CloseOutcome::Closed;
845	match report.outcome {	1116	let already =
846	// `report.commit` is a git object name, not a collab id,	1117	matches!(merged.outcome, merge_scan::MergeOutcome::AlreadyMerged);
847	// so it keeps git's own abbreviation rather than this one.	1118	report(
848	merge_scan::MergeOutcome::Recorded => println!(	1119	json,
849	"Recorded patch {} as merged in {:.8}",	1120	\|\| {
850	abbrev.of(&report.id),	1121	serde_json::json!({
851	report.commit	1122	"action": "patch.merge",
852	),	1123	"patch": merged.id,
853	merge_scan::MergeOutcome::AlreadyMerged => {	1124	"commit": merged.commit.to_string(),
854	println!(	1125	"already_recorded": already,
855	"Patch {} is already recorded as merged.",	1126	// Which issue, not merely that one closed: a caller
856	abbrev.of(&report.id)	1127	// that has to go and look it up has learned nothing
857	)	1128	// the exit code did not already tell it.
858	}	1129	"closed_issue": closed.then(\|\| merged.fixes.clone()).flatten(),
859	}	1130	})
860	if report.close == merge_scan::CloseOutcome::Closed {	1131	},
861	println!("Closed the issue it fixes.");	1132	\|\| {
862	}	1133	let abbrev = abbrev::for_patches(repo);
863	Ok(())	1134	// `merged.commit` is a git object name, not a collab
		1135	// id, so it keeps git's own abbreviation, not this one.
		1136	let mut lines = if already {
		1137	format!(
		1138	"Patch {} is already recorded as merged.",
		1139	abbrev.of(&merged.id)
		1140	)
		1141	} else {
		1142	format!(
		1143	"Recorded patch {} as merged in {:.8}",
		1144	abbrev.of(&merged.id),
		1145	merged.commit
		1146	)
		1147	};
		1148	if closed {
		1149	lines.push_str("\nClosed the issue it fixes.");
		1150	}
		1151	lines
		1152	},
		1153	)
864	}	1154	}
865	PatchCmd::Close { id, reason } => {	1155	PatchCmd::Close { id, reason, json } => {
866	patch::close(repo, &id, reason.as_deref())?;	1156	let r = patch::close(repo, &id, reason.as_deref())?;
867	println!("Patch closed.");	1157	report(
868	Ok(())	1158	json,
		1159	\|\| {
		1160	serde_json::json!({
		1161	"action": "patch.close",
		1162	"patch": r.id,
		1163	"status": "closed",
		1164	"event": r.event.to_string(),
		1165	})
		1166	},
		1167	\|\| "Patch closed.".to_string(),
		1168	)
869	}	1169	}
870	PatchCmd::Delete { id } => {	1170	PatchCmd::Reopen { id, json } => {
		1171	let r = patch::reopen(repo, &id)?;
		1172	report(
		1173	json,
		1174	\|\| {
		1175	serde_json::json!({
		1176	"action": "patch.reopen",
		1177	"patch": r.id,
		1178	"status": "open",
		1179	"was": r.was.to_string(),
		1180	"cleared_merge_commit": r.cleared_merge_commit,
		1181	"event": r.event.to_string(),
		1182	})
		1183	},
		1184	\|\| {
		1185	let mut lines = "Patch reopened.".to_string();
		1186	if let Some(ref commit) = r.cleared_merge_commit {
		1187	// Named, not just dropped: it is the only route
		1188	// from a squashed patch back to the code, and
		1189	// someone who reopened by mistake needs it to
		1190	// record the merge again.
		1191	lines.push_str(&format!(
		1192	"\nCleared the recorded merge in {:.8}.",
		1193	commit
		1194	));
		1195	}
		1196	lines
		1197	},
		1198	)
		1199	}
		1200	PatchCmd::Delete { id, json } => {
871	let full_id = patch::delete(repo, &id)?;	1201	let full_id = patch::delete(repo, &id)?;
872	println!("Deleted patch {}", abbrev::for_patches(repo).of(&full_id));	1202	report(
873	Ok(())	1203	json,
		1204	\|\| serde_json::json!({ "action": "patch.delete", "patch": full_id }),
		1205	\|\| format!("Deleted patch {}", abbrev::for_patches(repo).of(&full_id)),
		1206	)
874	}	1207	}
875	PatchCmd::Checkout { id } => {	1208	PatchCmd::Checkout { id, json } => {
876	patch::checkout(repo, &id)?;	1209	let r = patch::checkout(repo, &id)?;
		1210	if json {
		1211	println!(
		1212	"{}",
		1213	serde_json::json!({
		1214	"action": "patch.checkout",
		1215	"patch": r.id,
		1216	"branch": r.branch,
		1217	"revision": r.revision,
		1218	"commit": r.commit,
		1219	"created": r.created,
		1220	})
		1221	);
		1222	} else {
		1223	patch::report_checkout(repo, &r);
		1224	}
877	Ok(())	1225	Ok(())
878	}	1226	}
879	},	1227	},
@@ -883,13 +1231,15 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
883	files,	1231	files,
884	force,	1232	force,
885	remote,	1233	remote,
886	} => release::publish(repo, &remote, &version, &files, force),	1234	json,
		1235	} => release::publish(repo, &remote, &version, &files, force, json),
887	ReleaseCmd::List { json, remote } => release::list(repo, &remote, json),	1236	ReleaseCmd::List { json, remote } => release::list(repo, &remote, json),
888	ReleaseCmd::Delete {	1237	ReleaseCmd::Delete {
889	version,	1238	version,
890	filename,	1239	filename,
891	remote,	1240	remote,
892	} => release::delete(repo, &remote, &version, filename.as_deref()),	1241	json,
		1242	} => release::delete(repo, &remote, &version, filename.as_deref(), json),
893	},	1243	},
894	Commands::Status => {	1244	Commands::Status => {
895	// Status is where someone looks when the tool is behaving	1245	// Status is where someone looks when the tool is behaving
@@ -909,26 +1259,40 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
909	Some(remote) => sync::sync(repo, &remote),	1259	Some(remote) => sync::sync(repo, &remote),
910	None => sync::sync_all(repo),	1260	None => sync::sync_all(repo),
911	},	1261	},
912	Commands::InitKey { force } => generate_signing_key(force),	1262	Commands::InitKey { force, json } => generate_signing_key(force, json),
913	Commands::Whoami => {	1263	Commands::Whoami => {
914	let info = identity::whoami(repo)?;	1264	let info = identity::whoami(repo)?;
915	println!("{}", info);	1265	println!("{}", info);
916	Ok(())	1266	Ok(())
917	}	1267	}
918	Commands::Identity(cmd) => match cmd {	1268	Commands::Identity(cmd) => match cmd {
919	IdentityCmd::Alias { email } => {	1269	IdentityCmd::Alias { email, json } => {
920	let added = identity::add_alias(repo, &email)?;	1270	let added = identity::add_alias(repo, &email)?;
921	if added {	1271	report(
922	println!("Alias '{}' added.", email);	1272	json,
923	} else {	1273	\|\| {
924	println!("Alias '{}' already exists.", email);	1274	serde_json::json!({
925	}	1275	"action": "identity.alias",
926	Ok(())	1276	"email": email,
		1277	"added": added,
		1278	})
		1279	},
		1280	\|\| {
		1281	if added {
		1282	format!("Alias '{}' added.", email)
		1283	} else {
		1284	format!("Alias '{}' already exists.", email)
		1285	}
		1286	},
		1287	)
927	}	1288	}
928	IdentityCmd::Unalias { email } => {	1289	IdentityCmd::Unalias { email, json } => {
929	identity::remove_alias(repo, &email)?;	1290	identity::remove_alias(repo, &email)?;
930	println!("Alias '{}' removed.", email);	1291	report(
931	Ok(())	1292	json,
		1293	\|\| serde_json::json!({ "action": "identity.unalias", "email": email }),
		1294	\|\| format!("Alias '{}' removed.", email),
		1295	)
932	}	1296	}
933	IdentityCmd::List => {	1297	IdentityCmd::List => {
934	let author = identity::get_author(repo)?;	1298	let author = identity::get_author(repo)?;
@@ -949,12 +1313,13 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
949	Commands::Key(cmd) => match cmd {	1313	Commands::Key(cmd) => match cmd {
950	// `key generate` and the top-level `init-key` are the same	1314	// `key generate` and the top-level `init-key` are the same
951	// command reached two ways; see `KeyCmd::Generate`.	1315	// command reached two ways; see `KeyCmd::Generate`.
952	KeyCmd::Generate { force } => generate_signing_key(force),	1316	KeyCmd::Generate { force, json } => generate_signing_key(force, json),
953	KeyCmd::Add {	1317	KeyCmd::Add {
954	pubkey,	1318	pubkey,
955	self_key,	1319	self_key,
956	label,	1320	label,
957	global,	1321	global,
		1322	json,
958	} => {	1323	} => {
959	if self_key && pubkey.is_some() {	1324	if self_key && pubkey.is_some() {
960	return Err(error::Error::Cmd(	1325	return Err(error::Error::Cmd(
@@ -981,17 +1346,30 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
981	} else {	1346	} else {
982	trust::save_trusted_key(repo, &key, label.as_deref())?	1347	trust::save_trusted_key(repo, &key, label.as_deref())?
983	};	1348	};
984	if added {	1349	report(
985	let label_display = label	1350	json,
986	.as_ref()	1351	\|\| {
987	.map(\|l\| format!(" ({})", l))	1352	serde_json::json!({
988	.unwrap_or_default();	1353	"action": "key.add",
989	let scope = if global { " (global)" } else { "" };	1354	"pubkey": key,
990	println!("Trusted key added{}: {}{}", scope, key, label_display);	1355	"label": label,
991	} else {	1356	"global": global,
992	println!("Key {} is already trusted.", key);	1357	"added": added,
993	}	1358	})
994	Ok(())	1359	},
		1360	\|\| {
		1361	if added {
		1362	let label_display = label
		1363	.as_ref()
		1364	.map(\|l\| format!(" ({})", l))
		1365	.unwrap_or_default();
		1366	let scope = if global { " (global)" } else { "" };
		1367	format!("Trusted key added{}: {}{}", scope, key, label_display)
		1368	} else {
		1369	format!("Key {} is already trusted.", key)
		1370	}
		1371	},
		1372	)
995	}	1373	}
996	KeyCmd::List { global } => {	1374	KeyCmd::List { global } => {
997	if global {	1375	if global {
@@ -1029,23 +1407,39 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
1029	}	1407	}
1030	Ok(())	1408	Ok(())
1031	}	1409	}
1032	KeyCmd::Remove { pubkey, global } => {	1410	KeyCmd::Remove {
		1411	pubkey,
		1412	global,
		1413	json,
		1414	} => {
1033	let removed = if global {	1415	let removed = if global {
1034	trust::remove_trusted_key_global(&pubkey)?	1416	trust::remove_trusted_key_global(&pubkey)?
1035	} else {	1417	} else {
1036	trust::remove_trusted_key(repo, &pubkey)?	1418	trust::remove_trusted_key(repo, &pubkey)?
1037	};	1419	};
1038	let label_display = removed	1420	report(
1039	.label	1421	json,
1040	.as_ref()	1422	\|\| {
1041	.map(\|l\| format!(" ({})", l))	1423	serde_json::json!({
1042	.unwrap_or_default();	1424	"action": "key.remove",
1043	let scope = if global { " (global)" } else { "" };	1425	"pubkey": removed.pubkey,
1044	println!(	1426	"label": removed.label,
1045	"Removed trusted key{}: {}{}",	1427	"global": global,
1046	scope, removed.pubkey, label_display	1428	})
1047	);	1429	},
1048	Ok(())	1430	\|\| {
		1431	let label_display = removed
		1432	.label
		1433	.as_ref()
		1434	.map(\|l\| format!(" ({})", l))
		1435	.unwrap_or_default();
		1436	let scope = if global { " (global)" } else { "" };
		1437	format!(
		1438	"Removed trusted key{}: {}{}",
		1439	scope, removed.pubkey, label_display
		1440	)
		1441	},
		1442	)
1049	}	1443	}
1050	},	1444	},
1051	Commands::Search { query } => search(repo, &query),	1445	Commands::Search { query } => search(repo, &query),

src/log.rs

Old		New
@@ -129,6 +129,7 @@ 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::PatchReopen => "PatchReopen".to_string(),
132	Action::BodyEdit { .. } => "BodyEdit".to_string(),	133	Action::BodyEdit { .. } => "BodyEdit".to_string(),
133	Action::CommentDelete { .. } => "CommentDelete".to_string(),	134	Action::CommentDelete { .. } => "CommentDelete".to_string(),
134	Action::Merge => "Merge".to_string(),	135	Action::Merge => "Merge".to_string(),
@@ -184,6 +185,7 @@ fn action_summary(action: &Action) -> String {
184	format!("merge {}", &commit[..commit.len().min(7)])	185	format!("merge {}", &commit[..commit.len().min(7)])
185	}	186	}
186	}	187	}
		188	Action::PatchReopen => "reopen".to_string(),
187	// The log is the audit trail for corrections: it names the event that	189	// 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"	190	// was superseded, so "who changed what, and what did it say before"
189	// is answerable from `git-collab log` plus the original event object.	191	// is answerable from `git-collab log` plus the original event object.

src/patch.rs

Old		New
@@ -385,22 +385,26 @@ pub fn edit_comment(
385	id_prefix: &str,	385	id_prefix: &str,
386	comment_prefix: &str,	386	comment_prefix: &str,
387	body_args: &crate::body::BodyArgs,	387	body_args: &crate::body::BodyArgs,
388	) -> Result<(), crate::error::Error> {	388	) -> Result<dag::Corrected, crate::error::Error> {
389	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;	389	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
390	let patch = PatchState::from_ref(repo, &ref_name, &id)?;	390	let patch = PatchState::from_ref(repo, &ref_name, &id)?;
391	let target = patch.resolve_comment(comment_prefix)?;	391	let target = patch.resolve_comment(comment_prefix)?;
392	require_own_body(repo, &target)?;	392	require_own_body(repo, &target)?;
393		393
394	let body = crate::body::resolve_required(body_args, &target.body, target.kind.label())?;	394	let body = crate::body::resolve_required(body_args, &target.body, target.kind.label())?;
395	dag::append_action(	395	let event = dag::append_action(
396	repo,	396	repo,
397	&ref_name,	397	&ref_name,
398	Action::BodyEdit {	398	Action::BodyEdit {
399	target: target.oid,	399	target: target.oid.clone(),
400	body,	400	body,
401	},	401	},
402	)?;	402	)?;
403	Ok(())	403	Ok(dag::Corrected {
		404	id,
		405	target: target.oid,
		406	event,
		407	})
404	}	408	}
405		409
406	/// Tombstone a comment: drop its text, keep its slot.	410	/// Tombstone a comment: drop its text, keep its slot.
@@ -408,21 +412,25 @@ pub fn delete_comment(
408	repo: &Repository,	412	repo: &Repository,
409	id_prefix: &str,	413	id_prefix: &str,
410	comment_prefix: &str,	414	comment_prefix: &str,
411	) -> Result<(), crate::error::Error> {	415	) -> Result<dag::Corrected, crate::error::Error> {
412	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;	416	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
413	let patch = PatchState::from_ref(repo, &ref_name, &id)?;	417	let patch = PatchState::from_ref(repo, &ref_name, &id)?;
414	let target = patch.resolve_comment(comment_prefix)?;	418	let target = patch.resolve_comment(comment_prefix)?;
415	require_own_body(repo, &target)?;	419	require_own_body(repo, &target)?;
416	require_deletable(&target)?;	420	require_deletable(&target)?;
417		421
418	dag::append_action(	422	let event = dag::append_action(
419	repo,	423	repo,
420	&ref_name,	424	&ref_name,
421	Action::CommentDelete {	425	Action::CommentDelete {
422	target: target.oid,	426	target: target.oid.clone(),
423	},	427	},
424	)?;	428	)?;
425	Ok(())	429	Ok(dag::Corrected {
		430	id,
		431	target: target.oid,
		432	event,
		433	})
426	}	434	}
427		435
428	/// Correct a revision's description.	436	/// Correct a revision's description.
@@ -436,22 +444,84 @@ pub fn edit_revision(
436	id_prefix: &str,	444	id_prefix: &str,
437	revision: u32,	445	revision: u32,
438	body_args: &crate::body::BodyArgs,	446	body_args: &crate::body::BodyArgs,
439	) -> Result<(), crate::error::Error> {	447	) -> Result<dag::Corrected, crate::error::Error> {
440	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;	448	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
441	let patch = PatchState::from_ref(repo, &ref_name, &id)?;	449	let patch = PatchState::from_ref(repo, &ref_name, &id)?;
442	let target = patch.resolve_revision(revision)?;	450	let target = patch.resolve_revision(revision)?;
443	require_own_body(repo, &target)?;	451	require_own_body(repo, &target)?;
444		452
445	let body = crate::body::resolve_required(body_args, &target.body, "revision description")?;	453	let body = crate::body::resolve_required(body_args, &target.body, "revision description")?;
446	dag::append_action(	454	let event = dag::append_action(
447	repo,	455	repo,
448	&ref_name,	456	&ref_name,
449	Action::BodyEdit {	457	Action::BodyEdit {
450	target: target.oid,	458	target: target.oid.clone(),
451	body,	459	body,
452	},	460	},
453	)?;	461	)?;
454	Ok(())	462	Ok(dag::Corrected {
		463	id,
		464	target: target.oid,
		465	event,
		466	})
		467	}
		468
		469	/// What `patch comment` recorded: the patch, the comment's own event OID — the
		470	/// id `patch edit-comment` takes — and where it landed.
		471	pub struct CommentReport {
		472	pub id: String,
		473	pub event: Oid,
		474	pub placement: CommentPlacement,
		475	}
		476
		477	impl CommentReport {
		478	/// The `--json` shape. The action name matches the event the DAG actually
		479	/// holds — `patch.comment` or `patch.inline_comment` — so a caller reading
		480	/// this and a caller reading `patch log` are looking at the same word for
		481	/// the same thing, and the anchor a comment landed on is reported rather
		482	/// than left to be read back.
		483	pub fn to_json(&self) -> serde_json::Value {
		484	match &self.placement {
		485	CommentPlacement::Thread => serde_json::json!({
		486	"action": "patch.comment",
		487	"patch": self.id,
		488	"comment": self.event.to_string(),
		489	}),
		490	CommentPlacement::Inline {
		491	file,
		492	line,
		493	revision,
		494	non_blocking,
		495	} => serde_json::json!({
		496	"action": "patch.inline_comment",
		497	"patch": self.id,
		498	"comment": self.event.to_string(),
		499	"file": file,
		500	"line": line,
		501	"revision": revision,
		502	"non_blocking": non_blocking,
		503	}),
		504	}
		505	}
		506	}
		507
		508	/// What `patch review` recorded. `closed` is the reject verdict's side effect,
		509	/// reported rather than left to be inferred from the verdict.
		510	pub struct ReviewReport {
		511	pub id: String,
		512	pub event: Oid,
		513	pub verdict: ReviewVerdict,
		514	pub revision: u32,
		515	pub closed: bool,
		516	}
		517
		518	/// What `patch revise` recorded: which revision number it became, and the
		519	/// commit it snapshots.
		520	pub struct RevisionReport {
		521	pub id: String,
		522	pub event: Oid,
		523	pub number: u32,
		524	pub commit: String,
455	}	525	}
456		526
457	/// Where a comment landed, so the confirmation can say so. A bare	527	/// Where a comment landed, so the confirmation can say so. A bare
@@ -597,7 +667,7 @@ pub fn comment(
597	line: Option<u32>,	667	line: Option<u32>,
598	target_revision: Option<u32>,	668	target_revision: Option<u32>,
599	non_blocking: bool,	669	non_blocking: bool,
600	) -> Result<CommentPlacement, crate::error::Error> {	670	) -> Result<CommentReport, crate::error::Error> {
601	let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;	671	let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
602	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;	672	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
603	let patch = PatchState::from_ref(repo, &ref_name, &id)?;	673	let patch = PatchState::from_ref(repo, &ref_name, &id)?;
@@ -669,8 +739,12 @@ pub fn comment(
669	action,	739	action,
670	clock: 0,	740	clock: 0,
671	};	741	};
672	dag::append_event(repo, &ref_name, &event, &sk)?;	742	let recorded = dag::append_event(repo, &ref_name, &event, &sk)?;
673	Ok(placement)	743	Ok(CommentReport {
		744	id,
		745	event: recorded,
		746	placement,
		747	})
674	}	748	}
675		749
676	pub fn review(	750	pub fn review(
@@ -679,7 +753,7 @@ pub fn review(
679	verdict: ReviewVerdict,	753	verdict: ReviewVerdict,
680	body: &str,	754	body: &str,
681	target_revision: Option<u32>,	755	target_revision: Option<u32>,
682	) -> Result<(), crate::error::Error> {	756	) -> Result<ReviewReport, crate::error::Error> {
683	let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;	757	let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
684	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;	758	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
685	let patch = PatchState::from_ref(repo, &ref_name, &id)?;	759	let patch = PatchState::from_ref(repo, &ref_name, &id)?;
@@ -720,7 +794,7 @@ pub fn review(
720	},	794	},
721	clock: 0,	795	clock: 0,
722	};	796	};
723	dag::append_event(repo, &ref_name, &event, &sk)?;	797	let recorded = dag::append_event(repo, &ref_name, &event, &sk)?;
724		798
725	// A reject verdict also closes the patch	799	// A reject verdict also closes the patch
726	if is_reject {	800	if is_reject {
@@ -739,7 +813,13 @@ pub fn review(
739	}	813	}
740	}	814	}
741		815
742	Ok(())	816	Ok(ReviewReport {
		817	id,
		818	event: recorded,
		819	verdict,
		820	revision: rev,
		821	closed: is_reject,
		822	})
743	}	823	}
744		824
745	/// Record a new revision from `branch`, or from `HEAD` when none is named.	825	/// Record a new revision from `branch`, or from `HEAD` when none is named.
@@ -753,7 +833,7 @@ pub fn revise(
753	id_prefix: &str,	833	id_prefix: &str,
754	body: Option<&str>,	834	body: Option<&str>,
755	branch: Option<&str>,	835	branch: Option<&str>,
756	) -> Result<(), crate::error::Error> {	836	) -> Result<RevisionReport, crate::error::Error> {
757	let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;	837	let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
758	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;	838	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
759	let patch = PatchState::from_ref(repo, &ref_name, &id)?;	839	let patch = PatchState::from_ref(repo, &ref_name, &id)?;
@@ -798,15 +878,22 @@ pub fn revise(
798	timestamp: chrono::Utc::now().to_rfc3339(),	878	timestamp: chrono::Utc::now().to_rfc3339(),
799	author,	879	author,
800	action: Action::PatchRevision {	880	action: Action::PatchRevision {
801	commit: tip_hex,	881	commit: tip_hex.clone(),
802	tree: tree_oid.to_string(),	882	tree: tree_oid.to_string(),
803	body: body.map(\|s\| s.to_string()),	883	body: body.map(\|s\| s.to_string()),
804	base: Some(base_oid.to_string()),	884	base: Some(base_oid.to_string()),
805	},	885	},
806	clock: 0,	886	clock: 0,
807	};	887	};
808	dag::append_event(repo, &ref_name, &event, &sk)?;	888	let recorded = dag::append_event(repo, &ref_name, &event, &sk)?;
809	Ok(())	889	Ok(RevisionReport {
		890	id,
		891	event: recorded,
		892	// The fold numbers revisions by position, and this one is appended
		893	// after every revision the state we just read already lists.
		894	number: patch.revisions.len() as u32 + 1,
		895	commit: tip_hex,
		896	})
810	}	897	}
811		898
812	/// How to render a diff. All three settings are presentation only — none of	899	/// How to render a diff. All three settings are presentation only — none of
@@ -1336,11 +1423,27 @@ pub fn patch_log_json(patch: &PatchState) -> Result<String, Error> {
1336	/// A detached HEAD gets its commit rather than a branch name, because telling	1423	/// A detached HEAD gets its commit rather than a branch name, because telling
1337	/// someone to `git checkout` a branch they were never on would put them	1424	/// someone to `git checkout` a branch they were never on would put them
1338	/// somewhere they have not been.	1425	/// somewhere they have not been.
1339	struct PreviousHead {	1426	pub struct PreviousHead {
1340	display: String,	1427	display: String,
1341	restore: String,	1428	restore: String,
1342	}	1429	}
1343		1430
		1431	/// What `patch checkout` did: the patch, the branch it left behind, and the
		1432	/// revision that branch stands at.
		1433	pub struct CheckoutReport {
		1434	pub id: String,
		1435	pub branch: String,
		1436	pub revision: u32,
		1437	/// The revision's commit, in full — the branch is a convenience, this is
		1438	/// the thing it points at.
		1439	pub commit: String,
		1440	/// Whether the branch was created, or an existing one already standing at
		1441	/// the same commit was reused.
		1442	pub created: bool,
		1443	/// Where HEAD was, when the checkout moved it somewhere else.
		1444	pub previous: Option<PreviousHead>,
		1445	}
		1446
1344	fn previous_head(repo: &Repository) -> Option<PreviousHead> {	1447	fn previous_head(repo: &Repository) -> Option<PreviousHead> {
1345	let head = repo.head().ok()?;	1448	let head = repo.head().ok()?;
1346	if head.is_branch() {	1449	if head.is_branch() {
@@ -1358,7 +1461,7 @@ fn previous_head(repo: &Repository) -> Option<PreviousHead> {
1358	})	1461	})
1359	}	1462	}
1360		1463
1361	pub fn checkout(repo: &Repository, id_prefix: &str) -> Result<(), crate::error::Error> {	1464	pub fn checkout(repo: &Repository, id_prefix: &str) -> Result<CheckoutReport, crate::error::Error> {
1362	let (_ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;	1465	let (_ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
1363	// Read where we are before anything moves it.	1466	// Read where we are before anything moves it.
1364	let previous = previous_head(repo);	1467	let previous = previous_head(repo);
@@ -1421,30 +1524,44 @@ pub fn checkout(repo: &Repository, id_prefix: &str) -> Result<(), crate::error::
1421	repo.checkout_tree(&obj, None)?;	1524	repo.checkout_tree(&obj, None)?;
1422	repo.set_head(&refname)?;	1525	repo.set_head(&refname)?;
1423		1526
1424	// Checking a patch out is a detour, so say how the detour ends. Without
1425	// this the reviewer has to have remembered where they were, and is left
1426	// with a branch nobody told them about.
1427	// Landing where you already were is not a move, so there is nothing to	1527	// Landing where you already were is not a move, so there is nothing to
1428	// report and nowhere to send you back to.	1528	// report and nowhere to send you back to.
1429	let moved = previous.as_ref().is_some_and(\|p\| p.restore != branch_name);	1529	let moved = previous.as_ref().is_some_and(\|p\| p.restore != branch_name);
1430	match previous.filter(\|_\| moved) {	1530	Ok(CheckoutReport {
		1531	id,
		1532	branch: branch_name,
		1533	revision: latest_rev.number,
		1534	commit: latest_rev.commit.clone(),
		1535	created,
		1536	previous: previous.filter(\|_\| moved),
		1537	})
		1538	}
		1539
		1540	/// Say how the detour ends. Without this the reviewer has to have remembered
		1541	/// where they were, and is left with a branch nobody told them about.
		1542	///
		1543	/// Split from [`checkout`] so `--json` can report the same facts without the
		1544	/// prose: the abbreviated id belongs here, in what a person reads, and the full
		1545	/// one belongs in what a caller parses.
		1546	pub fn report_checkout(repo: &Repository, report: &CheckoutReport) {
		1547	let short_id = crate::abbrev::for_patches(repo).of(&report.id).to_string();
		1548	match &report.previous {
1431	Some(prev) => {	1549	Some(prev) => {
1432	println!(	1550	println!(
1433	"Checked out patch {} (revision {}) on branch {}; you were on {}.",	1551	"Checked out patch {} (revision {}) on branch {}; you were on {}.",
1434	short_id, latest_rev.number, branch_name, prev.display	1552	short_id, report.revision, report.branch, prev.display
1435	);	1553	);
1436	println!("Return with `git checkout {}`.", prev.restore);	1554	println!("Return with `git checkout {}`.", prev.restore);
1437	}	1555	}
1438	None => println!(	1556	None => println!(
1439	"Checked out patch {} (revision {}) on branch {}.",	1557	"Checked out patch {} (revision {}) on branch {}.",
1440	short_id, latest_rev.number, branch_name	1558	short_id, report.revision, report.branch
1441	),	1559	),
1442	}	1560	}
1443	println!(	1561	println!(
1444	"Branch {} is left behind; remove it with `git branch -D {}`.",	1562	"Branch {} is left behind; remove it with `git branch -D {}`.",
1445	branch_name, branch_name	1563	report.branch, report.branch
1446	);	1564	);
1447	Ok(())
1448	}	1565	}
1449		1566
1450	pub fn delete(repo: &Repository, id_prefix: &str) -> Result<String, crate::error::Error> {	1567	pub fn delete(repo: &Repository, id_prefix: &str) -> Result<String, crate::error::Error> {
@@ -1455,28 +1572,36 @@ pub fn delete(repo: &Repository, id_prefix: &str) -> Result<String, crate::error
1455	Ok(id)	1572	Ok(id)
1456	}	1573	}
1457		1574
1458	pub fn label(repo: &Repository, id_prefix: &str, label: &str) -> Result<(), crate::error::Error> {	1575	pub fn label(
1459	let (ref_name, _id) = state::resolve_patch_ref(repo, id_prefix)?;	1576	repo: &Repository,
1460	dag::append_action(	1577	id_prefix: &str,
		1578	label: &str,
		1579	) -> Result<dag::Recorded, crate::error::Error> {
		1580	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
		1581	let event = dag::append_action(
1461	repo,	1582	repo,
1462	&ref_name,	1583	&ref_name,
1463	Action::PatchLabel {	1584	Action::PatchLabel {
1464	label: label.to_string(),	1585	label: label.to_string(),
1465	},	1586	},
1466	)?;	1587	)?;
1467	Ok(())	1588	Ok(dag::Recorded { id, event })
1468	}	1589	}
1469		1590
1470	pub fn unlabel(repo: &Repository, id_prefix: &str, label: &str) -> Result<(), crate::error::Error> {	1591	pub fn unlabel(
1471	let (ref_name, _id) = state::resolve_patch_ref(repo, id_prefix)?;	1592	repo: &Repository,
1472	dag::append_action(	1593	id_prefix: &str,
		1594	label: &str,
		1595	) -> Result<dag::Recorded, crate::error::Error> {
		1596	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
		1597	let event = dag::append_action(
1473	repo,	1598	repo,
1474	&ref_name,	1599	&ref_name,
1475	Action::PatchUnlabel {	1600	Action::PatchUnlabel {
1476	label: label.to_string(),	1601	label: label.to_string(),
1477	},	1602	},
1478	)?;	1603	)?;
1479	Ok(())	1604	Ok(dag::Recorded { id, event })
1480	}	1605	}
1481		1606
1482	/// What `patch merge` did, so the caller can say so.	1607	/// What `patch merge` did, so the caller can say so.
@@ -1486,6 +1611,10 @@ pub struct MergeReport {
1486	pub close: crate::merge_scan::CloseOutcome,	1611	pub close: crate::merge_scan::CloseOutcome,
1487	/// The commit recorded as having landed the patch.	1612	/// The commit recorded as having landed the patch.
1488	pub commit: Oid,	1613	pub commit: Oid,
		1614	/// The issue `--fixes` named, in full, if the patch declared one. Carried
		1615	/// here so a caller can pair it with `close` and know which issue the
		1616	/// merge closed rather than only that it closed one.
		1617	pub fixes: Option<String>,
1489	}	1618	}
1490		1619
1491	/// Record that a patch landed on its base branch — layer 3 of merge recording,	1620	/// Record that a patch landed on its base branch — layer 3 of merge recording,
@@ -1552,6 +1681,7 @@ pub fn merge(
1552	outcome,	1681	outcome,
1553	close,	1682	close,
1554	commit: commit_oid,	1683	commit: commit_oid,
		1684	fixes: patch.fixes.clone(),
1555	})	1685	})
1556	}	1686	}
1557		1687
@@ -1559,9 +1689,9 @@ pub fn close(
1559	repo: &Repository,	1689	repo: &Repository,
1560	id_prefix: &str,	1690	id_prefix: &str,
1561	reason: Option<&str>,	1691	reason: Option<&str>,
1562	) -> Result<(), crate::error::Error> {	1692	) -> Result<dag::Recorded, crate::error::Error> {
1563	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;	1693	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
1564	dag::append_action(	1694	let event = dag::append_action(
1565	repo,	1695	repo,
1566	&ref_name,	1696	&ref_name,
1567	Action::PatchClose {	1697	Action::PatchClose {
@@ -1572,5 +1702,48 @@ pub fn close(
1572	if ref_name.starts_with(state::PATCH_PREFIX) {	1702	if ref_name.starts_with(state::PATCH_PREFIX) {
1573	state::archive_patch_ref(repo, &id)?;	1703	state::archive_patch_ref(repo, &id)?;
1574	}	1704	}
1575	Ok(())	1705	Ok(dag::Recorded { id, event })
		1706	}
		1707
		1708	/// Return a closed or merged patch to `open`.
		1709	///
		1710	/// The counterpart of [`close`], and the other half of the correction the
		1711	/// merge-recording design calls for: a `PatchMerge` recorded in error is undone
		1712	/// "by closing or by a future reopen event", and until now only closing existed
		1713	/// — which left the patch `closed` when what was wanted was `open`. It is also
		1714	/// the only way back for a patch closed by mistake.
		1715	///
		1716	/// Nothing here decides which event wins. The reopen is appended like any other
		1717	/// event and the status fold resolves it against every close and merge on the
		1718	/// same `(clock, oid)` order, so a reopen here and a close on another clone
		1719	/// converge on one answer rather than on whichever synced last.
		1720	pub fn reopen(repo: &Repository, id_prefix: &str) -> Result<ReopenReport, crate::error::Error> {
		1721	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
		1722	let before = PatchState::from_ref(repo, &ref_name, &id)?;
		1723	let event = dag::append_action(repo, &ref_name, Action::PatchReopen)?;
		1724	// Undo what `close` did to the refs, so the reopened patch is visible to a
		1725	// plain `patch list` again.
		1726	if ref_name.starts_with(state::ARCHIVE_PATCH_PREFIX) {
		1727	state::unarchive_patch_ref(repo, &id)?;
		1728	}
		1729	Ok(ReopenReport {
		1730	id,
		1731	event,
		1732	was: before.status,
		1733	cleared_merge_commit: before.merge_commit,
		1734	})
		1735	}
		1736
		1737	/// What `patch reopen` did, so the caller can say so.
		1738	pub struct ReopenReport {
		1739	pub id: String,
		1740	/// The reopen event that was appended.
		1741	pub event: Oid,
		1742	/// The status the patch held before the reopen was recorded.
		1743	pub was: state::PatchStatus,
		1744	/// The merge commit the reopen dropped, if the patch was recorded as
		1745	/// merged. Reported rather than silently discarded: it is the only durable
		1746	/// evidence of where a squashed patch went, and someone undoing a merge by
		1747	/// mistake needs it back.
		1748	pub cleared_merge_commit: Option<String>,
1576	}	1749	}

src/release.rs

Old		New
@@ -233,11 +233,17 @@ pub fn publish(
233	version: &str,	233	version: &str,
234	files: &[PathBuf],	234	files: &[PathBuf],
235	force: bool,	235	force: bool,
		236	json: bool,
236	) -> Result<(), Error> {	237	) -> Result<(), Error> {
237	if !validate_name(version) {	238	if !validate_name(version) {
238	return Err(Error::Cmd(format!("invalid version name: {}", version)));	239	return Err(Error::Cmd(format!("invalid version name: {}", version)));
239	}	240	}
240	let remote = ssh_remote(repo, remote_name)?;	241	let remote = ssh_remote(repo, remote_name)?;
		242	// Under `--json` the per-file lines are held back and emitted as one value
		243	// at the end: stdout must carry exactly one thing a caller can parse, and
		244	// a stream of objects is not that. The prose keeps reporting each file as
		245	// it lands, which is what a long upload needs.
		246	let mut published = Vec::new();
241	for file in files {	247	for file in files {
242	let filename = file	248	let filename = file
243	.file_name()	249	.file_name()
@@ -268,7 +274,21 @@ pub fn publish(
268	let output = run_remote(repo, &remote, &remote_cmd, Stdio::from(handle))?;	274	let output = run_remote(repo, &remote, &remote_cmd, Stdio::from(handle))?;
269	let stdout = String::from_utf8_lossy(&output.stdout);	275	let stdout = String::from_utf8_lossy(&output.stdout);
270	let sha = stdout.trim().strip_prefix("ok ").unwrap_or("").to_string();	276	let sha = stdout.trim().strip_prefix("ok ").unwrap_or("").to_string();
271	println!("Published {}/{} (sha256 {})", version, filename, sha);	277	if json {
		278	published.push(serde_json::json!({ "name": filename, "sha256": sha }));
		279	} else {
		280	println!("Published {}/{} (sha256 {})", version, filename, sha);
		281	}
		282	}
		283	if json {
		284	println!(
		285	"{}",
		286	serde_json::json!({
		287	"action": "release.publish",
		288	"version": version,
		289	"files": published,
		290	})
		291	);
272	}	292	}
273	Ok(())	293	Ok(())
274	}	294	}
@@ -311,6 +331,7 @@ pub fn delete(
311	remote_name: &str,	331	remote_name: &str,
312	version: &str,	332	version: &str,
313	filename: Option<&str>,	333	filename: Option<&str>,
		334	json: bool,
314	) -> Result<(), Error> {	335	) -> Result<(), Error> {
315	if !validate_name(version) {	336	if !validate_name(version) {
316	return Err(Error::Cmd(format!("invalid version name: {}", version)));	337	return Err(Error::Cmd(format!("invalid version name: {}", version)));
@@ -326,9 +347,21 @@ pub fn delete(
326	remote_cmd.push_str(&format!(" '{}'", name));	347	remote_cmd.push_str(&format!(" '{}'", name));
327	}	348	}
328	run_remote(repo, &remote, &remote_cmd, Stdio::null())?;	349	run_remote(repo, &remote, &remote_cmd, Stdio::null())?;
329	match filename {	350	if json {
330	Some(name) => println!("Deleted {}/{}", version, name),	351	println!(
331	None => println!("Deleted {}", version),	352	"{}",
		353	serde_json::json!({
		354	"action": "release.delete",
		355	"version": version,
		356	// Null means the whole version went, not "a file with no name".
		357	"file": filename,
		358	})
		359	);
		360	} else {
		361	match filename {
		362	Some(name) => println!("Deleted {}/{}", version, name),
		363	None => println!("Deleted {}", version),
		364	}
332	}	365	}
333	Ok(())	366	Ok(())
334	}	367	}
@@ -349,7 +382,8 @@ mod tests {
349	let dir_that_looks_like_a_file = tmp.path().join("payload.tar.gz");	382	let dir_that_looks_like_a_file = tmp.path().join("payload.tar.gz");
350	std::fs::create_dir(&dir_that_looks_like_a_file).unwrap();	383	std::fs::create_dir(&dir_that_looks_like_a_file).unwrap();
351		384
352	let err = publish(&repo, "origin", "v1", &[dir_that_looks_like_a_file], false).unwrap_err();	385	let err =
		386	publish(&repo, "origin", "v1", &[dir_that_looks_like_a_file], false, false).unwrap_err();
353	assert!(	387	assert!(
354	err.to_string().contains("not a regular file"),	388	err.to_string().contains("not a regular file"),
355	"got: {}",	389	"got: {}",

src/state.rs

@@ @@ -1306,6 +1306,24 @@ impl PatchState { @@
+                Action::PatchReopen => {
+                    if let Some(ref mut s) = state {
+                        let key = (event.clock, oid.to_string());
+                        if status_key.as_ref().is_none_or(|k| key >= *k) {
+                            s.status = PatchStatus::Open;
+                            // Cleared under the same guard, for the same reason
+                            // `PatchClose` clears it: a patch whose status says
+                            // open while `merge_commit` names the commit that
+                            // landed it is two answers to one question. Reopen
+                            // is the documented correction for a `PatchMerge`
+                            // recorded in error, so it undoes all of what that
+                            // merge recorded, not half of it. Recording the
+                            // merge again restores it.
+                            s.merge_commit = None;
+                            status_key = Some(key);
+                        }
+                    }
+                }
+                _ => {}
+    Ok(())
+/// Move a patch's whole subtree from archive back to the active namespace,
+/// undoing exactly what [`archive_patch_ref`] did.
+///
+/// The subtree, not just the events ref: `archive_patch_ref` moves every
+/// `<id>/r/<n>` along with `<id>/events`, and leaving the revisions behind
+/// would give the reopened patch nothing to review.
+pub fn unarchive_patch_ref(repo: &Repository, id: &str) -> Result<(), crate::error::Error> {
+    for (old_ref, suffix) in patch_subtree(repo, ARCHIVE_PATCH_PREFIX, id)? {
+        let oid = repo.refname_to_id(&old_ref)?;
+        let new_ref = format!("{}{}/{}", PATCH_PREFIX, id, suffix);
+        repo.reference(&new_ref, oid, false, "unarchive patch")?;
+        repo.find_reference(&old_ref)?.delete()?;
+    }
+    Ok(())
+}
+/// The two namespaces a collab object's events ref can live in, active first.
+///
+/// `kind` is `"issues"` or `"patches"`, matching `sync`'s vocabulary.
+fn events_ref_candidates(kind: &str, id: &str) -> [String; 2] {
+    if kind == "patches" {
+        [
+            format!("{}{}/events", PATCH_PREFIX, id),
+            format!("{}{}/events", ARCHIVE_PATCH_PREFIX, id),
+        ]
+    } else {
+        [
+            format!("refs/collab/{}/{}", kind, id),
+            format!("refs/collab/archive/{}/{}", kind, id),
+        ]
+    }
+}
+/// Where `id`'s event DAG currently lives in this clone, or `None` if it has it
+/// in neither namespace.
+///
+/// Sync needs this because closing moves a ref between namespaces while the id
+/// stays the same. An incoming `refs/collab/archive/...` and an incoming
+/// `refs/collab/patches/...` for one id are the same object seen by two clones
+/// that disagree about its status, and both have to reconcile into the one
+/// local DAG — adopting each into its own namespace would leave this clone
+/// holding two divergent histories for one patch, which no later sync could
+/// reunite.
+pub fn existing_events_ref(repo: &Repository, kind: &str, id: &str) -> Option<String> {
+    events_ref_candidates(kind, id)
+        .into_iter()
+        .find(|name| repo.refname_to_id(name).is_ok())
+}
+/// Bring `id` back out of the archive if the events it now carries say it is
+/// open, having arrived from a peer that reopened it.
+///
+/// One direction only, and the asymmetry is the point. Archiving is an
+/// optimisation: a closed object is filtered out of the default list by its
+/// *status* regardless of which namespace it sits in, so a close that arrives
+/// by sync and leaves the ref where it is costs nothing — which is exactly what
+/// has always happened, since only the local `close` command moves a ref.
+/// Failing to *un*archive is a correctness bug in the other direction: nothing
+/// enumerates the archive namespace unless asked, so a reopen that arrived from
+/// a peer would leave the object open and invisible, with no command able to
+/// find it.
+pub fn unarchive_if_reopened(
+    repo: &Repository,
+    kind: &str,
+    id: &str,
+) -> Result<(), crate::error::Error> {
+    let Some(current) = existing_events_ref(repo, kind, id) else {
+        return Ok(());
+    };
+    if !current.starts_with("refs/collab/archive/") {
+        return Ok(());
+    }
+    if kind == "patches" {
+        if PatchState::from_ref(repo, &current, id)?.status == PatchStatus::Open {
+            return unarchive_patch_ref(repo, id);
+        }
+    } else if IssueState::from_ref(repo, &current, id)?.status == IssueStatus::Open {
+        return unarchive_issue_ref(repo, id);
+    }
+    Ok(())
+}
+/// Delete every ref belonging to a patch, in either namespace.
+pub fn delete_patch_refs(repo: &Repository, id: &str) -> Result<(), crate::error::Error> {
+    for prefix in [PATCH_PREFIX, ARCHIVE_PATCH_PREFIX] {

src/sync.rs

Old		New
@@ -913,22 +913,32 @@ fn reconcile_refs(
913	author: &crate::event::Author,	913	author: &crate::event::Author,
914	signing_key: &ed25519_dalek::SigningKey,	914	signing_key: &ed25519_dalek::SigningKey,
915	) -> Result<(), Error> {	915	) -> Result<(), Error> {
916	let sync_prefix = format!("refs/collab/sync/{}/", kind);	916	// Both namespaces, because a close moves a ref between them while the id
917	let sync_refs: Vec<(String, String, SyncRef)> = {	917	// stays the same. A peer that closed an object publishes it under
		918	// `archive/`, a peer that reopened it publishes it under the active prefix,
		919	// and the two are one object: reconciling only the active prefix would take
		920	// one clone's answer and silently drop the other's events. See
		921	// `state::existing_events_ref`.
		922	let sync_prefixes = [
		923	format!("refs/collab/sync/{}/", kind),
		924	format!("refs/collab/sync/archive/{}/", kind),
		925	];
		926	let mut sync_refs: Vec<(String, String, SyncRef)> = Vec::new();
		927	for sync_prefix in &sync_prefixes {
918	let refs = repo.references_glob(&format!("{}*", sync_prefix))?;	928	let refs = repo.references_glob(&format!("{}*", sync_prefix))?;
919	refs.filter_map(\|r\| {	929	sync_refs.extend(refs.filter_map(\|r\| {
920	let r = r.ok()?;	930	let r = r.ok()?;
921	let name = r.name()?.to_string();	931	let name = r.name()?.to_string();
922	let rest = name.strip_prefix(&sync_prefix)?;	932	let rest = name.strip_prefix(sync_prefix)?;
923	let (id, classified) = classify_sync_ref(kind, rest)?;	933	let (id, classified) = classify_sync_ref(kind, rest)?;
924	Some((name, id, classified))	934	Some((name, id, classified))
925	})	935	}));
926	.collect()	936	}
927	};
928		937
929	// Load trust policy once for all refs of this kind	938	// Load trust policy once for all refs of this kind
930	let trust_policy = trust::load_trust_policy(repo)?;	939	let trust_policy = trust::load_trust_policy(repo)?;
931	let mut warned_unconfigured = false;	940	let mut warned_unconfigured = false;
		941	let mut reconciled: Vec<String> = Vec::new();
932		942
933	for (remote_ref, id, classified) in &sync_refs {	943	for (remote_ref, id, classified) in &sync_refs {
934	// Validate the ref ID format before processing	944	// Validate the ref ID format before processing
@@ -981,6 +991,12 @@ fn reconcile_refs(
981	let SyncRef::Events { local_ref } = classified else {	991	let SyncRef::Events { local_ref } = classified else {
982	continue;	992	continue;
983	};	993	};
		994	// Whichever namespace this clone already keeps the object in wins over
		995	// the namespace the remote ref arrived under: the local DAG is the one
		996	// both incoming tips have to merge into. `realign_namespace` below then
		997	// decides where it belongs from the events themselves.
		998	let local_ref = &state::existing_events_ref(repo, kind, id).unwrap_or(local_ref.clone());
		999	reconciled.push(id.clone());
984	if repo.refname_to_id(local_ref).is_ok() {	1000	if repo.refname_to_id(local_ref).is_ok() {
985	match dag::reconcile(repo, local_ref, remote_ref, author, signing_key) {	1001	match dag::reconcile(repo, local_ref, remote_ref, author, signing_key) {
986	Ok((_oid, outcome)) => {	1002	Ok((_oid, outcome)) => {
@@ -1010,5 +1026,16 @@ fn reconcile_refs(
1010	}	1026	}
1011	}	1027	}
1012	}	1028	}
		1029
		1030	// Only now, with every incoming tip merged in: an object this clone had
		1031	// filed away that the reconciled DAG says is open again comes back out, or
		1032	// a peer's reopen would leave it open and unlistable here.
		1033	reconciled.sort();
		1034	reconciled.dedup();
		1035	for id in &reconciled {
		1036	if let Err(e) = state::unarchive_if_reopened(repo, kind, id) {
		1037	errln!(" Failed to unarchive reopened {} {:.8}: {}", kind, id, e);
		1038	}
		1039	}
1013	Ok(())	1040	Ok(())
1014	}	1041	}

src/timeline.rs

Old		New
@@ -121,6 +121,10 @@ pub enum Kind {
121	#[serde(skip_serializing_if = "Option::is_none")]	121	#[serde(skip_serializing_if = "Option::is_none")]
122	commit: Option<String>,	122	commit: Option<String>,
123	},	123	},
		124	/// A `PatchReopen`. Listed for the same reason `Closed` and `Merged` are:
		125	/// a timeline that ended at "merged" while the patch reads `open` would be
		126	/// the one view of a patch that disagrees with every other.
		127	Reopened,
124	}	128	}
125		129
126	impl Kind {	130	impl Kind {
@@ -137,6 +141,7 @@ impl Kind {
137	Kind::Deleted { .. } => "deleted".to_string(),	141	Kind::Deleted { .. } => "deleted".to_string(),
138	Kind::Closed { .. } => "closed".to_string(),	142	Kind::Closed { .. } => "closed".to_string(),
139	Kind::Merged { .. } => "merged".to_string(),	143	Kind::Merged { .. } => "merged".to_string(),
		144	Kind::Reopened => "reopened".to_string(),
140	}	145	}
141	}	146	}
142	}	147	}
@@ -298,6 +303,7 @@ pub fn build(repo: &Repository, id_prefix: &str) -> Result<(PatchState, Vec<Entr
298	},	303	},
299	None,	304	None,
300	),	305	),
		306	Action::PatchReopen => (Kind::Reopened, None),
301	_ => continue,	307	_ => continue,
302	};	308	};
303		309
@@ -429,6 +435,7 @@ pub fn to_writer(entries: &[Entry], writer: &mut dyn std::io::Write) -> Result<(
429	Kind::Merged { commit } => {	435	Kind::Merged { commit } => {
430	parts.extend(commit.as_deref().map(\|c\| format!("{:.8}", c)))	436	parts.extend(commit.as_deref().map(\|c\| format!("{:.8}", c)))
431	}	437	}
		438	Kind::Reopened => {}
432	}	439	}
433		440
434	writeln!(	441	writeln!(

src/tui/widgets.rs

Old		New
@@ -53,6 +53,7 @@ pub(crate) fn action_type_label(action: &Action) -> &str {
53	Action::PatchInlineComment { .. } => "Inline Comment",	53	Action::PatchInlineComment { .. } => "Inline Comment",
54	Action::PatchClose { .. } => "Patch Close",	54	Action::PatchClose { .. } => "Patch Close",
55	Action::PatchMerge { .. } => "Patch Merge",	55	Action::PatchMerge { .. } => "Patch Merge",
		56	Action::PatchReopen => "Patch Reopen",
56	Action::Merge => "Merge",	57	Action::Merge => "Merge",
57	Action::IssueEdit { .. } => "Issue Edit",	58	Action::IssueEdit { .. } => "Issue Edit",
58	Action::IssueLabel { .. } => "Issue Label",	59	Action::IssueLabel { .. } => "Issue Label",
@@ -187,7 +188,7 @@ pub(crate) fn format_event_detail(
187	detail.push_str(&format!("\nMerged as: {}\n", commit));	188	detail.push_str(&format!("\nMerged as: {}\n", commit));
188	}	189	}
189	}	190	}
190	Action::IssueReopen \| Action::Merge => {}	191	Action::IssueReopen \| Action::PatchReopen \| Action::Merge => {}
191	}	192	}
192		193
193	detail	194	detail

tests/alias_test.rs

Old		New
@@ -72,7 +72,7 @@ fn key_generate_reaches_the_same_generator_as_init_key() {
72	let generated = Cli::try_parse_from(["git-collab", "key", "generate", "--force"]).unwrap();	72	let generated = Cli::try_parse_from(["git-collab", "key", "generate", "--force"]).unwrap();
73	assert!(matches!(	73	assert!(matches!(
74	generated.command,	74	generated.command,
75	Commands::Key(KeyCmd::Generate { force: true })	75	Commands::Key(KeyCmd::Generate { force: true, .. })
76	));	76	));
77	assert_same(&["key", "init"], &["key", "generate"]);	77	assert_same(&["key", "init"], &["key", "generate"]);
78	assert_same(&["key", "init-key"], &["key", "generate"]);	78	assert_same(&["key", "init-key"], &["key", "generate"]);

tests/mutating_json_test.rs

@@ @@ -0,0 +1,556 @@ @@
+//! `--json` on the commands that write.
+//!
+//! Read commands have had it for a long time; the commands that *create*
+//! something did not, so a script could only learn the id of what it had just
+//! made by parsing prose — and the prose moved underneath it the moment ids
+//! became sized to the repo. See issue a6adfe39.
+//!
+//! Two rules are asserted everywhere below, because together they are the
+//! contract:
+//!
+//! - stdout is exactly one JSON value and nothing else, even when auto-sync is
+//!   narrating at the same time (`serde_json::from_str` on the whole of stdout
+//!   fails on trailing content, so every `json_ok` call asserts this).
+//! - every id is the **full** id. Abbreviation is a display policy; `--json`
+//!   already carries full ids on the read commands and must not grow a second
+//!   convention here.
+mod common;
+use std::process::Command;
+use serde_json::Value;
+use tempfile::TempDir;
+use common::TestRepo;
+// ---------------------------------------------------------------------------
+// Harness
+// ---------------------------------------------------------------------------
+/// Run a command expected to succeed, and parse the whole of stdout as one
+/// JSON value. Parsing the *whole* string is the assertion that nothing else
+/// was mixed in: `serde_json` rejects trailing content.
+fn json_ok(repo: &TestRepo, args: &[&str]) -> Value {
+    let out = repo.run_ok(args);
+    serde_json::from_str(&out).unwrap_or_else(|e| {
+        panic!(
+            "git-collab {:?} did not print exactly one JSON value: {}\nstdout was:\n{}",
+            args, e, out
+        )
+    })
+}
+/// A full git object name: 40 lowercase hex characters, never an abbreviation.
+fn assert_full_id(value: &Value, what: &str) {
+    let s = value
+        .as_str()
+        .unwrap_or_else(|| panic!("{} is not a string: {}", what, value));
+    assert_eq!(
+        s.len(),
+,
+        "{} must be the full id, got {:?} ({} chars)",
+        what,
+        s,
+        s.len()
+    );
+    assert!(
+        s.chars().all(|c| c.is_ascii_hexdigit() && !c.is_uppercase()),
+        "{} must be a hex object name, got {:?}",
+        what,
+        s
+    );
+}
+fn open_issue_json(repo: &TestRepo, title: &str) -> String {
+    let json = json_ok(repo, &["issue", "open", "-t", title, "--json"]);
+    json["issue"].as_str().unwrap().to_string()
+}
+/// Create a patch on its own branch, with `--json`, and return its full id.
+fn create_patch_json(repo: &TestRepo, branch: &str, file: &str) -> String {
+    repo.git(&["checkout", "-b", branch]);
+    repo.commit_file(file, "content", &format!("work on {}", branch));
+    let json = json_ok(
+        repo,
+        &["patch", "create", "-t", branch, "-B", branch, "--json"],
+    );
+    repo.git(&["checkout", "main"]);
+    json["patch"].as_str().unwrap().to_string()
+}
+fn collab_refs(repo: &TestRepo) -> String {
+    repo.git(&["for-each-ref", "--format=%(refname)", "refs/collab/"])
+}
+// ---------------------------------------------------------------------------
+// The two ids a script most needs: what `open` and `create` just made
+// ---------------------------------------------------------------------------
+#[test]
+fn issue_open_json_emits_the_full_id_and_it_names_a_ref() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let json = json_ok(&repo, &["issue", "open", "-t", "A bug", "--json"]);
+    assert_eq!(json["action"], "issue.open");
+    assert_full_id(&json["issue"], "issue");
+    let id = json["issue"].as_str().unwrap();
+    assert!(
+        collab_refs(&repo).contains(&format!("refs/collab/issues/{}", id)),
+        "the id must be the one the ref is named after"
+    );
+    // The prose the id used to have to be scraped out of is abbreviated, and
+    // the JSON one is not — that difference is the whole point.
+    let prose = repo.run_ok(&["issue", "show", id]);
+    assert!(prose.contains("A bug"));
+}
+#[test]
+fn patch_create_json_emits_the_full_id_and_it_names_a_ref() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = create_patch_json(&repo, "feat", "a.txt");
+    assert_eq!(id.len(), 40, "patch id must be full, got {:?}", id);
+    assert!(
+        collab_refs(&repo).contains(&format!("refs/collab/patches/{}/events", id)),
+        "the id must be the one the ref is named after"
+    );
+}
+#[test]
+fn patch_create_json_carries_the_fixed_issue_in_full() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let issue = open_issue_json(&repo, "A bug");
+    repo.git(&["checkout", "-b", "feat"]);
+    repo.commit_file("a.txt", "x", "work");
+    let json = json_ok(
+        &repo,
+        &[
+            "patch", "create", "-t", "Fix", "-B", "feat", "--fixes", &issue[..8], "--json",
+        ],
+    );
+    assert_full_id(&json["patch"], "patch");
+    assert_eq!(
+        json["fixes"], issue,
+        "an id resolved from a prefix comes back in full"
+    );
+}
+// ---------------------------------------------------------------------------
+// Every other issue mutation
+// ---------------------------------------------------------------------------
+#[test]
+fn every_issue_mutation_reports_the_issue_in_full() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = open_issue_json(&repo, "A bug");
+    let other = open_issue_json(&repo, "Another bug");
+    let short = &id[..8];
+    let comment = json_ok(&repo, &["issue", "comment", short, "-b", "hi", "--json"]);
+    assert_eq!(comment["action"], "issue.comment");
+    assert_full_id(&comment["issue"], "issue");
+    assert_eq!(comment["issue"], id.as_str());
+    assert_full_id(&comment["comment"], "comment");
+    let comment_id = comment["comment"].as_str().unwrap().to_string();
+    let cases: Vec<(Vec<&str>, &str)> = vec![
+        (vec!["issue", "edit", short, "-t", "Retitled"], "issue.edit"),
+        (vec!["issue", "label", short, "bug"], "issue.label"),
+        (vec!["issue", "unlabel", short, "bug"], "issue.unlabel"),
+        (vec!["issue", "assign", short, "alice"], "issue.assign"),
+        (vec!["issue", "unassign", short, "alice"], "issue.unassign"),
+        (vec!["issue", "relate", short, &other[..8]], "issue.relate"),
+        (
+            vec!["issue", "unrelate", short, &other[..8]],
+            "issue.unrelate",
+        ),
+    ];
+    for (args, action) in cases {
+        let mut args = args;
+        args.push("--json");
+        let json = json_ok(&repo, &args);
+        assert_eq!(json["action"], action, "for {:?}", args);
+        assert_full_id(&json["issue"], "issue");
+        assert_eq!(json["issue"], id.as_str(), "for {:?}", args);
+        assert_full_id(&json["event"], "event");
+    }
+    let edited = json_ok(
+        &repo,
+        &[
+            "issue",
+            "edit-comment",
+            short,
+            &comment_id[..8],
+            "-b",
+            "hello",
+            "--json",
+        ],
+    );
+    assert_eq!(edited["action"], "issue.edit_comment");
+    assert_eq!(edited["issue"], id.as_str());
+    assert_eq!(
+        edited["comment"], comment_id,
+        "the corrected comment is named in full, not by the prefix given"
+    );
+    assert_full_id(&edited["event"], "event");
+    let deleted = json_ok(
+        &repo,
+        &[
+            "issue",
+            "delete-comment",
+            short,
+            &comment_id[..8],
+            "--json",
+        ],
+    );
+    assert_eq!(deleted["action"], "issue.delete_comment");
+    assert_eq!(deleted["comment"], comment_id);
+    let closed = json_ok(&repo, &["issue", "close", short, "--json"]);
+    assert_eq!(closed["action"], "issue.close");
+    assert_eq!(closed["issue"], id.as_str());
+    assert_eq!(closed["status"], "closed");
+    let reopened = json_ok(&repo, &["issue", "reopen", short, "--json"]);
+    assert_eq!(reopened["action"], "issue.reopen");
+    assert_eq!(reopened["status"], "open");
+    let deleted = json_ok(&repo, &["issue", "delete", short, "--json"]);
+    assert_eq!(deleted["action"], "issue.delete");
+    assert_eq!(
+        deleted["issue"], id.as_str(),
+        "the id of something deleted is exactly what it was"
+    );
+}
+// ---------------------------------------------------------------------------
+// Every other patch mutation
+// ---------------------------------------------------------------------------
+#[test]
+fn every_patch_mutation_reports_the_patch_in_full() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = create_patch_json(&repo, "feat", "a.txt");
+    let short = &id[..8];
+    let comment = json_ok(&repo, &["patch", "comment", short, "-b", "hi", "--json"]);
+    assert_eq!(comment["action"], "patch.comment");
+    assert_eq!(comment["patch"], id.as_str());
+    assert_full_id(&comment["comment"], "comment");
+    let comment_id = comment["comment"].as_str().unwrap().to_string();
+    let inline = json_ok(
+        &repo,
+        &[
+            "patch",
+            "comment",
+            short,
+            "--at",
+            "a.txt:1",
+            "-b",
+            "nit",
+            "--non-blocking",
+            "--json",
+        ],
+    );
+    assert_eq!(inline["action"], "patch.inline_comment");
+    assert_eq!(inline["patch"], id.as_str());
+    assert_full_id(&inline["comment"], "comment");
+    assert_eq!(inline["file"], "a.txt");
+    assert_eq!(inline["line"], 1);
+    assert_eq!(inline["revision"], 1);
+    assert_eq!(inline["non_blocking"], true);
+    let review = json_ok(
+        &repo,
+        &["patch", "review", short, "-v", "approve", "-b", "lgtm", "--json"],
+    );
+    assert_eq!(review["action"], "patch.review");
+    assert_eq!(review["patch"], id.as_str());
+    assert_full_id(&review["review"], "review");
+    assert_eq!(review["verdict"], "approve");
+    assert_eq!(review["revision"], 1);
+    for (args, action) in [
+        (vec!["patch", "label", short, "wip"], "patch.label"),
+        (vec!["patch", "unlabel", short, "wip"], "patch.unlabel"),
+    ] {
+        let mut args = args;
+        args.push("--json");
+        let json = json_ok(&repo, &args);
+        assert_eq!(json["action"], action);
+        assert_eq!(json["patch"], id.as_str());
+        assert_full_id(&json["event"], "event");
+    }
+    let edited = json_ok(
+        &repo,
+        &[
+            "patch",
+            "edit-comment",
+            short,
+            &comment_id[..8],
+            "-b",
+            "hello",
+            "--json",
+        ],
+    );
+    assert_eq!(edited["action"], "patch.edit_comment");
+    assert_eq!(edited["comment"], comment_id);
+    assert_full_id(&edited["event"], "event");
+    let deleted = json_ok(
+        &repo,
+        &["patch", "delete-comment", short, &comment_id[..8], "--json"],
+    );
+    assert_eq!(deleted["action"], "patch.delete_comment");
+    assert_eq!(deleted["comment"], comment_id);
+    // A revision names the commit it snapshots, in full.
+    repo.git(&["checkout", "feat"]);
+    let commit = repo.commit_file("a.txt", "more", "second");
+    repo.git(&["checkout", "main"]);
+    let revised = json_ok(
+        &repo,
+        &[
+            "patch", "revise", short, "-B", "feat", "-b", "round two", "--json",
+        ],
+    );
+    assert_eq!(revised["action"], "patch.revision");
+    assert_eq!(revised["patch"], id.as_str());
+    assert_eq!(revised["revision"], 2);
+    assert_eq!(revised["commit"], commit.trim());
+    assert_full_id(&revised["commit"], "commit");
+    let edited_rev = json_ok(
+        &repo,
+        &["patch", "edit-revision", short, "2", "-b", "fixed", "--json"],
+    );
+    assert_eq!(edited_rev["action"], "patch.edit_revision");
+    assert_eq!(edited_rev["patch"], id.as_str());
+    assert_eq!(edited_rev["revision"], 2);
+    assert_full_id(&edited_rev["event"], "event");
+    let checked_out = json_ok(&repo, &["patch", "checkout", short, "--json"]);
+    assert_eq!(checked_out["action"], "patch.checkout");
+    assert_eq!(checked_out["patch"], id.as_str());
+    assert!(checked_out["branch"].is_string());
+    assert_full_id(&checked_out["commit"], "commit");
+    repo.git(&["checkout", "main"]);
+    let closed = json_ok(&repo, &["patch", "close", short, "--json"]);
+    assert_eq!(closed["action"], "patch.close");
+    assert_eq!(closed["patch"], id.as_str());
+    assert_eq!(closed["status"], "closed");
+    let reopened = json_ok(&repo, &["patch", "reopen", short, "--json"]);
+    assert_eq!(reopened["action"], "patch.reopen");
+    assert_eq!(reopened["patch"], id.as_str());
+    assert_eq!(reopened["status"], "open");
+    let deleted = json_ok(&repo, &["patch", "delete", short, "--json"]);
+    assert_eq!(deleted["action"], "patch.delete");
+    assert_eq!(deleted["patch"], id.as_str());
+}
+#[test]
+fn patch_merge_json_names_the_patch_the_commit_and_the_issue_it_closed() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let issue = open_issue_json(&repo, "A bug");
+    repo.git(&["checkout", "-b", "feat"]);
+    repo.commit_file("a.txt", "x", "work");
+    let created = json_ok(
+        &repo,
+        &[
+            "patch", "create", "-t", "Fix", "-B", "feat", "--fixes", &issue[..8], "--json",
+        ],
+    );
+    let id = created["patch"].as_str().unwrap().to_string();
+    repo.git(&["checkout", "main"]);
+    repo.git(&["merge", "--ff-only", "feat"]);
+    let landed = repo.git(&["rev-parse", "main"]).trim().to_string();
+    let merged = json_ok(&repo, &["patch", "merge", &id[..8], "--json"]);
+    assert_eq!(merged["action"], "patch.merge");
+    assert_eq!(merged["patch"], id.as_str());
+    assert_eq!(
+        merged["commit"], landed,
+        "the commit that landed the patch, in full — the only route back from a squash"
+    );
+    assert_eq!(merged["already_recorded"], false);
+    assert_eq!(
+        merged["closed_issue"], issue,
+        "the issue `--fixes` named, in full, because the merge closed it"
+    );
+    // Recording it twice says so rather than pretending it was new.
+    let again = json_ok(&repo, &["patch", "merge", &id[..8], "--json"]);
+    assert_eq!(again["already_recorded"], true);
+    assert_eq!(again["patch"], id.as_str());
+}
+#[test]
+fn patch_reopen_json_reports_the_merge_commit_it_cleared() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let id = create_patch_json(&repo, "feat", "a.txt");
+    repo.git(&["merge", "--ff-only", "feat"]);
+    let landed = repo.git(&["rev-parse", "main"]).trim().to_string();
+    repo.run_ok(&["patch", "merge", &id[..8]]);
+    let json = json_ok(&repo, &["patch", "reopen", &id[..8], "--json"]);
+    assert_eq!(json["status"], "open");
+    assert_eq!(
+        json["cleared_merge_commit"], landed,
+        "reopening drops the recorded merge, so it has to hand it back"
+    );
+    // Nothing to clear on a patch that was merely closed.
+    repo.run_ok(&["patch", "close", &id[..8]]);
+    let json = json_ok(&repo, &["patch", "reopen", &id[..8], "--json"]);
+    assert!(json["cleared_merge_commit"].is_null(), "{}", json);
+}
+// ---------------------------------------------------------------------------
+// The rest of the writing surface
+// ---------------------------------------------------------------------------
+#[test]
+fn key_and_identity_mutations_report_what_they_changed() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let added = json_ok(&repo, &["key", "add", "--self", "--label", "mine", "--json"]);
+    assert_eq!(added["action"], "key.add");
+    assert_eq!(added["added"], true);
+    assert_eq!(added["label"], "mine");
+    assert_eq!(added["global"], false);
+    let pubkey = added["pubkey"].as_str().unwrap().to_string();
+    let again = json_ok(&repo, &["key", "add", "--self", "--json"]);
+    assert_eq!(again["added"], false, "already trusted is not a new key");
+    assert_eq!(again["pubkey"], pubkey);
+    let removed = json_ok(&repo, &["key", "remove", &pubkey, "--json"]);
+    assert_eq!(removed["action"], "key.remove");
+    assert_eq!(removed["pubkey"], pubkey);
+    let aliased = json_ok(&repo, &["identity", "alias", "a@b.example", "--json"]);
+    assert_eq!(aliased["action"], "identity.alias");
+    assert_eq!(aliased["email"], "a@b.example");
+    assert_eq!(aliased["added"], true);
+    let unaliased = json_ok(&repo, &["identity", "unalias", "a@b.example", "--json"]);
+    assert_eq!(unaliased["action"], "identity.unalias");
+    assert_eq!(unaliased["email"], "a@b.example");
+}
+#[test]
+fn hooks_install_json_reports_the_outcome_and_the_path() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let json = json_ok(&repo, &["hooks", "install", "--json"]);
+    assert_eq!(json["action"], "hooks.install");
+    assert_eq!(json["outcome"], "installed");
+    assert!(json["path"].as_str().unwrap().ends_with("commit-msg"));
+    let again = json_ok(&repo, &["hooks", "install", "--json"]);
+    assert_eq!(again["outcome"], "already-installed");
+}
+// ---------------------------------------------------------------------------
+// Failure keeps the shape it already had
+// ---------------------------------------------------------------------------
+#[test]
+fn a_failing_write_prints_the_error_object_on_stdout() {
+    // Established by e049a2bb for the read commands; a caller that asked for
+    // JSON never has to read stderr, and that has to hold on the writes too.
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let out = repo.run(&["issue", "close", "deadbeef", "--json"]);
+    assert!(!out.status.success());
+    let stdout = String::from_utf8(out.stdout).unwrap();
+    let json: Value = serde_json::from_str(&stdout)
+        .unwrap_or_else(|e| panic!("stdout was not one JSON value: {}\n{}", e, stdout));
+    assert!(json["error"].is_string(), "{}", json);
+    assert!(json.get("issue").is_none(), "a failure reports no id");
+}
+// ---------------------------------------------------------------------------
+// Auto-sync must not get into stdout
+// ---------------------------------------------------------------------------
+/// A repo with a local bare `origin` and auto-sync left switched **on**, which
+/// `TestRepo::new` otherwise disables. This is the case the separation of
+/// streams exists for: the write's result and the push's narration arrive
+/// together, and only one of them may be on stdout.
+fn repo_with_autosync() -> (TestRepo, TempDir) {
+    let bare = TempDir::new().unwrap();
+    let status = Command::new("git")
+        .args(["init", "--bare", "-b", "main"])
+        .arg(bare.path())
+        .status()
+        .unwrap();
+    assert!(status.success());
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    repo.git(&["remote", "add", "origin", bare.path().to_str().unwrap()]);
+    repo.git(&["push", "-u", "origin", "main"]);
+    repo.run_ok(&["init"]);
+    repo.git(&["config", "collab.autoSync", "true"]);
+    (repo, bare)
+}
+#[test]
+fn json_stdout_stays_pure_while_auto_sync_narrates() {
+    let (repo, _bare) = repo_with_autosync();
+    let out = repo.run(&["issue", "open", "-t", "A bug", "--json"]);
+    let stdout = String::from_utf8(out.stdout).unwrap();
+    let stderr = String::from_utf8(out.stderr).unwrap();
+    assert!(out.status.success(), "stderr:\n{}", stderr);
+    let json: Value = serde_json::from_str(&stdout).unwrap_or_else(|e| {
+        panic!(
+            "auto-sync leaked into stdout: {}\nstdout:\n{}\nstderr:\n{}",
+            e, stdout, stderr
+        )
+    });
+    assert_full_id(&json["issue"], "issue");
+    assert!(
+        stderr.contains("auto-sync: "),
+        "the push narration still has to be reported, on stderr:\n{}",
+        stderr
+    );
+    assert!(
+        !stdout.contains("auto-sync"),
+        "and never on stdout:\n{}",
+        stdout
+    );
+}
+#[test]
+fn json_stdout_stays_pure_when_the_auto_sync_push_fails() {
+    // The failure path prints more, and prints advice — all of it narration
+    // about the network, none of it the command's result.
+    let (repo, bare) = repo_with_autosync();
+    std::fs::remove_dir_all(bare.path()).unwrap();
+    let out = repo.run(&["issue", "open", "-t", "A bug", "--json"]);
+    let stdout = String::from_utf8(out.stdout).unwrap();
+    let stderr = String::from_utf8(out.stderr).unwrap();
+    let json: Value = serde_json::from_str(&stdout).unwrap_or_else(|e| {
+        panic!(
+            "a failed auto-sync leaked into stdout: {}\nstdout:\n{}\nstderr:\n{}",
+            e, stdout, stderr
+        )
+    });
+    assert_full_id(&json["issue"], "issue");
+    assert!(
+        json.get("error").is_none(),
+        "the local write succeeded; a failed push is not the command failing: {}",
+        json
+    );
+}

tests/patch_reopen_test.rs

@@ @@ -0,0 +1,362 @@ @@
+//! `patch reopen`: the other half of the merge-recording escape hatch.
+//!
+//! `issue reopen` has always existed; `patch reopen` did not, so a patch closed
+//! by mistake stayed closed forever, and a `PatchMerge` recorded in error could
+//! only be corrected into `closed` — never back to `open`. See issue 259200d4
+//! and docs/superpowers/specs/2026-08-09-merge-recording-design.md.
+mod common;
+use std::process::Command;
+use serde_json::Value;
+use tempfile::TempDir;
+use common::TestRepo;
+// ---------------------------------------------------------------------------
+// Harness
+// ---------------------------------------------------------------------------
+fn repo_with_origin() -> (TestRepo, TempDir) {
+    let bare = TempDir::new().unwrap();
+    let status = Command::new("git")
+        .args(["init", "--bare", "-b", "main"])
+        .arg(bare.path())
+        .status()
+        .unwrap();
+    assert!(status.success());
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    repo.git(&["remote", "add", "origin", bare.path().to_str().unwrap()]);
+    repo.git(&["push", "-u", "origin", "main"]);
+    repo.run_ok(&["init"]);
+    (repo, bare)
+}
+fn git_in(env_from: &TestRepo, dir: &std::path::Path, args: &[&str]) -> String {
+    let mut command = Command::new("git");
+    env_from.apply_env(&mut command);
+    let output = command.args(args).current_dir(dir).output().unwrap();
+    assert!(
+        output.status.success(),
+        "git {:?} failed: {}",
+        args,
+        String::from_utf8_lossy(&output.stderr)
+    );
+    String::from_utf8(output.stdout).unwrap()
+}
+fn collab_in(env_from: &TestRepo, dir: &std::path::Path, args: &[&str]) -> String {
+    let mut command = Command::new(env!("CARGO_BIN_EXE_git-collab"));
+    env_from.apply_env(&mut command);
+    let output = command.args(args).current_dir(dir).output().unwrap();
+    assert!(
+        output.status.success(),
+        "git-collab {:?} failed:\nstdout: {}\nstderr: {}",
+        args,
+        String::from_utf8_lossy(&output.stdout),
+        String::from_utf8_lossy(&output.stderr)
+    );
+    String::from_utf8(output.stdout).unwrap()
+}
+/// Create a patch on its own branch and return its short id.
+fn patch_on_branch(repo: &TestRepo, branch: &str, file: &str) -> String {
+    repo.git(&["checkout", "-b", branch]);
+    repo.commit_file(file, "content", &format!("work on {}", branch));
+    let out = repo.run_ok(&["patch", "create", "-t", branch, "-B", branch]);
+    let short = out.trim().strip_prefix("Created patch ").unwrap().to_string();
+    repo.git(&["checkout", "main"]);
+    short
+}
+fn show_json(repo: &TestRepo, id: &str) -> Value {
+    serde_json::from_str(&repo.run_ok(&["patch", "show", id, "--json"])).unwrap()
+}
+fn patch_events_ref(repo: &TestRepo, short: &str) -> String {
+    let out = repo.git(&["for-each-ref", "--format=%(refname)", "refs/collab/"]);
+    events_ref_in(&out, short)
+}
+/// The events ref for `short` in a `for-each-ref` listing. Each clone is asked
+/// for its own: a closed object lives in the archive namespace on the clone
+/// that closed it and in the active one on a clone that only learned of the
+/// close by sync, and neither is wrong — the namespace is a filing decision,
+/// the DAG tip and the status are the facts that have to match.
+fn events_ref_in(listing: &str, short: &str) -> String {
+    listing
+        .lines()
+        .find(|r| r.contains(short) && r.ends_with("/events"))
+        .unwrap_or_else(|| panic!("no events ref for {} in {}", short, listing))
+        .to_string()
+}
+// ---------------------------------------------------------------------------
+// The command exists and undoes a close
+// ---------------------------------------------------------------------------
+#[test]
+fn patch_reopen_returns_a_closed_patch_to_open() {
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let short = patch_on_branch(&repo, "feat", "a.txt");
+    repo.run_ok(&["patch", "close", &short]);
+    assert_eq!(show_json(&repo, &short)["status"], "closed");
+    repo.run_ok(&["patch", "reopen", &short]);
+    assert_eq!(
+        show_json(&repo, &short)["status"],
+        "open",
+        "reopen is the missing half of close; a patch closed by mistake has to come back"
+    );
+}
+#[test]
+fn a_reopened_patch_is_listed_again_without_all() {
+    // `close` archives the whole subtree, so a reopen that only appended an
+    // event would leave the patch open and invisible.
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let short = patch_on_branch(&repo, "feat", "a.txt");
+    repo.run_ok(&["patch", "close", &short]);
+    let listed = repo.run_ok(&["patch", "list"]);
+    assert!(
+        !listed.contains(&short),
+        "a closed patch is not in the default list: {}",
+        listed
+    );
+    repo.run_ok(&["patch", "reopen", &short]);
+    let listed = repo.run_ok(&["patch", "list"]);
+    assert!(
+        listed.contains(&short),
+        "a reopened patch belongs back in the default list: {}",
+        listed
+    );
+    let events_ref = patch_events_ref(&repo, &short);
+    assert!(
+        events_ref.starts_with("refs/collab/patches/"),
+        "the subtree moves back out of the archive namespace, got {}",
+        events_ref
+    );
+}
+#[test]
+fn reopening_a_patch_keeps_its_revisions_reachable() {
+    // Archiving moves `<id>/r/<n>` along with `<id>/events`; unarchiving has to
+    // bring them back, or the reopened patch has no revisions to review.
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let short = patch_on_branch(&repo, "feat", "a.txt");
+    repo.run_ok(&["patch", "close", &short]);
+    repo.run_ok(&["patch", "reopen", &short]);
+    let refs = repo.git(&["for-each-ref", "--format=%(refname)", "refs/collab/"]);
+    let revision_refs: Vec<&str> = refs
+        .lines()
+        .filter(|r| r.contains(short.as_str()) && r.contains("/rev/"))
+        .collect();
+    assert!(
+        !revision_refs.is_empty(),
+        "the revision refs must come back with the patch: {}",
+        refs
+    );
+    assert!(
+        revision_refs
+            .iter()
+            .all(|r| r.starts_with("refs/collab/patches/")),
+        "no revision ref may be left in the archive: {:?}",
+        revision_refs
+    );
+    assert_eq!(show_json(&repo, &short)["revisions"][0]["number"], 1);
+}
+// ---------------------------------------------------------------------------
+// Reopening a merged patch
+// ---------------------------------------------------------------------------
+#[test]
+fn reopening_a_merged_patch_clears_the_merge_commit() {
+    // `merge_commit` is written under the same `(clock, oid)` guard as `status`
+    // precisely so the two can never disagree. `PatchClose` already clears it;
+    // an *open* patch still naming the commit that landed it would be the same
+    // contradiction, and reopen is the documented correction for a `PatchMerge`
+    // recorded in error — so it has to undo all of what the merge recorded.
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let short = patch_on_branch(&repo, "feat", "a.txt");
+    repo.git(&["merge", "--ff-only", "feat"]);
+    repo.run_ok(&["patch", "merge", &short]);
+    let merged = show_json(&repo, &short);
+    assert_eq!(merged["status"], "merged");
+    assert!(merged["merge_commit"].is_string(), "{}", merged);
+    repo.run_ok(&["patch", "reopen", &short]);
+    let reopened = show_json(&repo, &short);
+    assert_eq!(reopened["status"], "open");
+    assert!(
+        reopened["merge_commit"].is_null(),
+        "an open patch must not name a merge the status denies: {}",
+        reopened
+    );
+    let shown = repo.run_ok(&["patch", "show", &short]);
+    assert!(
+        !shown.contains("Merged in:"),
+        "the text view must agree with the state: {}",
+        shown
+    );
+}
+#[test]
+fn a_merge_recorded_after_a_reopen_wins_again() {
+    // Reopen is not a terminal state: recording the merge again re-records it,
+    // which is what makes "reopened by mistake" recoverable in turn.
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let short = patch_on_branch(&repo, "feat", "a.txt");
+    repo.git(&["merge", "--ff-only", "feat"]);
+    repo.run_ok(&["patch", "merge", &short]);
+    repo.run_ok(&["patch", "reopen", &short]);
+    repo.run_ok(&["patch", "merge", &short]);
+    let json = show_json(&repo, &short);
+    assert_eq!(json["status"], "merged");
+    assert!(json["merge_commit"].is_string(), "{}", json);
+}
+// ---------------------------------------------------------------------------
+// The event, and how it folds
+// ---------------------------------------------------------------------------
+#[test]
+fn patch_reopen_event_serializes_under_its_own_type() {
+    use git_collab::event::{Action, Author, Event};
+    let event = Event {
+        timestamp: "2026-08-11T12:00:00Z".to_string(),
+        author: Author {
+            name: "Alice".to_string(),
+            email: "alice@example.com".to_string(),
+        },
+        action: Action::PatchReopen,
+        clock: 7,
+    };
+    let json = serde_json::to_string(&event).unwrap();
+    assert!(json.contains("\"type\":\"patch.reopen\""), "{}", json);
+    let round_tripped: Event = serde_json::from_str(&json).unwrap();
+    assert!(matches!(round_tripped.action, Action::PatchReopen));
+}
+#[test]
+fn a_close_recorded_after_a_reopen_still_wins() {
+    // The fold is ordered by `(clock, oid)`, not by variant. A later close has
+    // to beat an earlier reopen exactly as a later reopen beats a close.
+    let repo = TestRepo::new("Alice", "alice@example.com");
+    let short = patch_on_branch(&repo, "feat", "a.txt");
+    repo.run_ok(&["patch", "close", &short]);
+    repo.run_ok(&["patch", "reopen", &short]);
+    repo.run_ok(&["patch", "close", &short]);
+    assert_eq!(show_json(&repo, &short)["status"], "closed");
+}
+// ---------------------------------------------------------------------------
+// Convergence: one clone closes, another reopens, offline
+// ---------------------------------------------------------------------------
+#[test]
+fn a_concurrent_close_and_reopen_converge() {
+    let (alice, bare) = repo_with_origin();
+    let short = patch_on_branch(&alice, "feat", "a.txt");
+    alice.run_ok(&["sync"]);
+    let bob_root = TempDir::new().unwrap();
+    let bob = bob_root.path().join("clone");
+    git_in(
+        &alice,
+        bob_root.path(),
+        &["clone", bare.path().to_str().unwrap(), "clone"],
+    );
+    git_in(&alice, &bob, &["config", "user.name", "Bob"]);
+    git_in(&alice, &bob, &["config", "user.email", "bob@example.com"]);
+    git_in(&alice, &bob, &["config", "collab.autoSync", "false"]);
+    collab_in(&alice, &bob, &["init"]);
+    collab_in(&alice, &bob, &["sync"]);
+    // Neither has seen the other's event when they write theirs.
+    alice.run_ok(&["patch", "close", &short]);
+    collab_in(&alice, &bob, &["patch", "reopen", &short]);
+    // Alice pushes first, so Bob's sync reconciles a genuinely divergent DAG.
+    alice.run_ok(&["sync"]);
+    collab_in(&alice, &bob, &["sync"]);
+    alice.run_ok(&["sync"]);
+    let alice_status = show_json(&alice, &short)["status"].clone();
+    let bob_json: Value =
+        serde_json::from_str(&collab_in(&alice, &bob, &["patch", "show", &short, "--json"]))
+            .unwrap();
+    assert_eq!(
+        alice_status, bob_json["status"],
+        "a close and a reopen that never saw each other must still land on one answer"
+    );
+    let alice_ref = patch_events_ref(&alice, &short);
+    let bob_ref = events_ref_in(
+        &git_in(
+            &alice,
+            &bob,
+            &["for-each-ref", "--format=%(refname)", "refs/collab/"],
+        ),
+        &short,
+    );
+    assert_eq!(
+        alice.git(&["rev-parse", &alice_ref]).trim(),
+        git_in(&alice, &bob, &["rev-parse", &bob_ref]).trim(),
+        "both clones must end at the same DAG tip"
+    );
+}
+#[test]
+fn a_reopen_arriving_by_sync_is_listable_again() {
+    // The clone that closed the patch filed it in the archive namespace, where
+    // nothing enumerates it. A reopen arriving from a peer has to bring it back
+    // out, or the patch is open and unfindable on that clone.
+    let (alice, bare) = repo_with_origin();
+    let short = patch_on_branch(&alice, "feat", "a.txt");
+    alice.run_ok(&["sync"]);
+    let bob_root = TempDir::new().unwrap();
+    let bob = bob_root.path().join("clone");
+    git_in(
+        &alice,
+        bob_root.path(),
+        &["clone", bare.path().to_str().unwrap(), "clone"],
+    );
+    git_in(&alice, &bob, &["config", "user.name", "Bob"]);
+    git_in(&alice, &bob, &["config", "user.email", "bob@example.com"]);
+    git_in(&alice, &bob, &["config", "collab.autoSync", "false"]);
+    collab_in(&alice, &bob, &["init"]);
+    collab_in(&alice, &bob, &["sync"]);
+    // Alice closes and publishes; Bob sees the close, then reopens.
+    alice.run_ok(&["patch", "close", &short]);
+    alice.run_ok(&["sync"]);
+    collab_in(&alice, &bob, &["sync"]);
+    collab_in(&alice, &bob, &["patch", "reopen", &short]);
+    collab_in(&alice, &bob, &["sync"]);
+    alice.run_ok(&["sync"]);
+    assert_eq!(show_json(&alice, &short)["status"], "open");
+    let listed = alice.run_ok(&["patch", "list"]);
+    assert!(
+        listed.contains(&short),
+        "a patch reopened elsewhere has to come back out of this clone's archive: {}",
+        listed
+    );
+}