thunderbird-patch-review
Simple email patch review tool for Thunderbird
git clone git://mccd.space/thunderbird-patch-reviewcommit c4adc9f80fb2a060196b2ffb9e8015521b010dac
parent a2c3b5f713124ed8149424f17b8727b9f45cff45
Author: Pi Agent <agent@pi.local>
Date: Mon, 20 Jul 2026 10:26:38 +0200
Show Apply in yellow with ⚠ when the working tree is dirty
git am refuses to start on a tree with staged or unstaged tracked
changes (untracked files don't block it), so any Apply fails before
the patches are even read. patchHost.check() now detects this up front
— mirroring git's require_clean_work_tree via diff-index --quiet HEAD
(--ignore-submodules) — and reports a new "dirty" status alongside
applicable/applied/conflict. The review tab renders it as a yellow
"⚠ Apply" (distinct from green ✓ applied and red ✖ conflict), with
the git status -s listing in the tooltip so the user can see what to
commit or stash. Apply/Modify stay disabled until the tree is clean.
Diffstat:
4 files changed, 61 insertions(+), 8 deletions(-)
diff --git a/extension/api/patchHost/implementation.js b/extension/api/patchHost/implementation.js
@@ -273,6 +273,28 @@ async function check(repo, strategy, patches) {
throw new Error("no patches in request");
}
+ // Stage 0: dirty working tree. git am refuses to start on a tree with
+ // staged or unstaged tracked changes (untracked files don't block it),
+ // so any Apply would fail before the patches are even read — regardless
+ // of whether the series would otherwise apply. Surface it up front as a
+ // distinct "dirty" state so the UI can warn in yellow rather than let
+ // the user click Apply into a refusal. Mirror git's
+ // require_clean_work_tree: diff-index --quiet HEAD catches both staged
+ // and unstaged tracked changes in one call (exit 0 clean, 1 dirty, 128
+ // bad/absent HEAD); --ignore-submodules matches am's own check. The 128
+ // case falls through to the normal stages, which will report the real
+ // problem. A short `status -s` listing is folded into `output` for the
+ // tooltip so the user can see what's keeping the tree dirty.
+ const dirty = await run(git, ["-C", repo, "diff-index", "--quiet", "--ignore-submodules", "HEAD"]);
+ if (dirty.exitCode === 1) {
+ const why = await run(git, ["-C", repo, "status", "-s", "--untracked-files=no"]);
+ return {
+ ok: true,
+ status: "dirty",
+ output: why.output.trim() || "the working tree has staged or unstaged changes",
+ };
+ }
+
// Stage 1: cheap --check against the working tree — fast-path the common
// "cleanly applicable" and "exactly already applied" cases without
// paying for a worktree checkout. The probe holds the mboxes
diff --git a/extension/api/patchHost/schema.json b/extension/api/patchHost/schema.json
@@ -43,7 +43,7 @@
"name": "check",
"type": "function",
"async": true,
- "description": "Classify the patches against the repository without applying: resolves to {ok, status, output} where status is \"applicable\", \"applied\", or \"conflict\".",
+ "description": "Classify the patches against the repository without applying: resolves to {ok, status, output} where status is \"applicable\", \"applied\", \"dirty\", or \"conflict\".",
"parameters": [
{ "name": "repo", "type": "string" },
{
diff --git a/extension/review/review.css b/extension/review/review.css
@@ -161,9 +161,11 @@ button.primary:hover { color: #fff; opacity: 0.9; }
/* Apply-button states from the pre-check (patchHost.check): the button
turns green with a checkmark when the series is already in HEAD, red
- when it won't apply, disabled in both. State-checking is the transient
- "querying git" phase. The menu-toggle plants itself in the same split,
- so it shares the deafened look. */
+ when it won't apply, yellow with a warning when the working tree is
+ dirty (git am would refuse before reading the patches) — disabled in
+ all three. State-checking is the transient "querying git" phase. The
+ menu-toggle plants itself in the same split, so it shares the deafened
+ look. */
#btn-apply.state-checking { cursor: progress; opacity: 0.85; }
#btn-apply.state-applied,
@@ -180,6 +182,13 @@ button.primary:hover { color: #fff; opacity: 0.9; }
color: #cf222e;
}
+#btn-apply.state-dirty,
+#btn-apply.state-dirty:hover {
+ background: #fff8c5;
+ border-color: #bf8700;
+ color: #7d4e00;
+}
+
@media (prefers-color-scheme: dark) {
#btn-apply.state-applied,
#btn-apply.state-applied:hover {
@@ -193,6 +202,12 @@ button.primary:hover { color: #fff; opacity: 0.9; }
border-color: #f85149;
color: #ff7b72;
}
+ #btn-apply.state-dirty,
+ #btn-apply.state-dirty:hover {
+ background: #2a2306;
+ border-color: #d29922;
+ color: #e3b341;
+ }
}
.split {
diff --git a/extension/review/review.js b/extension/review/review.js
@@ -14,7 +14,7 @@ const state = {
comments: [], // locator -> comment, per series entry
current: 0,
repo: "",
- applyStatus: null, // "applicable"|"applied"|"conflict"|null (no repo / unknown)
+ applyStatus: null, // "applicable"|"applied"|"dirty"|"conflict"|null (no repo / unknown)
};
async function bg(request) {
@@ -622,7 +622,7 @@ async function refreshApplyState() {
// Leave the menu-toggle and the Download dropdown item alone: a series
// that's already applied or that conflicts still deserves to be exported.
const resetButton = () => {
- apply.classList.remove("state-checking", "state-applied", "state-conflict");
+ apply.classList.remove("state-checking", "state-applied", "state-dirty", "state-conflict");
apply.disabled = false;
apply.textContent = "Apply";
apply.title = "Apply the patches to the repository (git am)";
@@ -671,6 +671,20 @@ async function refreshApplyState() {
"This series does not apply cleanly to the repository.\n\n" +
(response.output || "(no detail)");
if (modifyBtn) modifyBtn.disabled = true;
+ } else if (state.applyStatus === "dirty") {
+ // The working tree has uncommitted tracked changes, so git am will
+ // refuse before even reading the patches. Warn in yellow — distinct
+ // from conflict (the patch content doesn't fit HEAD): this is a
+ // local, fixable condition. Leave Apply and Modify disabled; the
+ // tooltip tells the user to commit or stash and the next probe
+ // (after Browse re-pick or a successful apply elsewhere) re-checks.
+ apply.classList.add("state-dirty");
+ apply.disabled = true;
+ apply.textContent = "⚠ Apply";
+ apply.title =
+ "The repository has uncommitted changes; git am will refuse until you commit or stash them.\n\n" +
+ (response.output || "(no detail)");
+ if (modifyBtn) modifyBtn.disabled = true;
}
}
@@ -693,11 +707,13 @@ async function applySeries(mode) {
// patches, so honor a negative pre-check by refusing up front. (The
// buttons involved are already disabled when this state is set, but a
// stray keyboard shortcut or a future caller could still reach us.)
- if (mode !== "download" && (state.applyStatus === "applied" || state.applyStatus === "conflict")) {
+ if (mode !== "download" && (state.applyStatus === "applied" || state.applyStatus === "dirty" || state.applyStatus === "conflict")) {
showStatus(
state.applyStatus === "applied"
? "This series is already applied to the repository."
- : "This series does not apply cleanly to the repository.",
+ : state.applyStatus === "dirty"
+ ? "The repository has uncommitted changes; commit or stash them before applying."
+ : "This series does not apply cleanly to the repository.",
{ error: true }
);
return;