Review context in the Merge dialog

TODO.md

@@ -8,8 +8,6 @@
 
 # M4 — Merging and history
 
-- Review context in the Merge dialog
-  The Merge confirmation shows a non-blocking review section for the current snapshot: running, failed, skipped, not reviewed, or completed with finding counts by severity and links to each finding (including findings with no path or line), plus how many files changed since the reviewed snapshot. Earlier snapshots' findings sit in an expandable list labelled by snapshot, with resolution unknown rather than "unresolved", since each review covers only its own step. It notes that merging squashes into one commit, pushes, and removes the worktree, and that the Merge turn can still change the branch. Opening a finding and returning keeps the edited title. The section refreshes from events while open and shows a fetch failure as unavailable.
 - Closed tasks list
   The Project view has a searchable "Closed" list with each closed task's title, outcome, merge commit and final summary, read from the events that survive close. Worktree artifacts are not retained.
 

docs/operations.md

@@ -29,7 +29,7 @@ Every task worktree is made by `createTask` (`internal/agents/tasks.go`): new ta
 
 ## Closing a task
 
-A task ends `merged` or `discarded`; either way its sessions are stopped, its snapshot refs, worktree and branch are removed, its `open` inbox items become `dismissed` (resolved by whoever closed it), and its record and events remain. `GET /api/tasks/{id}/unmerged` reports what would be lost or merged: commits on the branch that aren't on the default branch, and files, lines and uncommitted paths that differ from the merge base. The UI's Merge and Discard buttons each ask a confirmation (Merge's holds the editable commit title), opened just below the button like the task picker's switcher (centred when ⌘S opens Merge's from a tab without the button), and are disabled while the task's session is `running`. Merge is also disabled until `unmerged` reports a commit or a changed file; the transcript fetches it when the task is opened and again shortly after each tool result or turn end.
+A task ends `merged` or `discarded`; either way its sessions are stopped, its snapshot refs, worktree and branch are removed, its `open` inbox items become `dismissed` (resolved by whoever closed it), and its record and events remain. `GET /api/tasks/{id}/unmerged` reports what would be lost or merged: commits on the branch that aren't on the default branch, and files, lines and uncommitted paths that differ from the merge base. The UI's Merge and Discard buttons each ask a confirmation, opened just below the button like the task picker's switcher (centred when ⌘S opens Merge's from a tab without the button), and are disabled while the task's session is `running`. Merge is also disabled until `unmerged` reports a commit or a changed file; the transcript fetches it when the task is opened and again shortly after each tool result or turn end. Merge's confirmation (`confirmMerge` in `web/src/task.ts`) is kept brief: the commit title as text with a ✎ button that swaps in an input, a collapsed "N co-authors" line holding the trailers, and one warning line for each thing worth stopping for. Those are the latest snapshot's review still running, failed, missing or with findings (counted by severity); the task's open inbox questions, which merging dismisses; the worktree differing from the latest snapshot (`changes?since=` its commit), or no snapshot at all; and "Review status unavailable" when those fetches fail. Earlier snapshots' reviews aren't mentioned. Warnings never block Merge. The review and question warnings link to the Reviews tab at that snapshot or the Inbox tab; following one closes the dialog and keeps the title in the page, so the next Merge opens with it. While open, the dialog refetches on the task's `snapshot.created` and `review.*` events and redraws on inbox changes.
 
 `POST /api/tasks/{id}/merge-in` (the transcript's **Sync from main** button, labelled with the default branch) brings the default branch into the task branch through the agent: it sends `mergeInPrompt` (`internal/agents/mergein.go`) as a `byre` message to the task's session, or starts a session with it when the task has none. The prompt has the agent run `git merge --no-ff --no-commit`, resolve conflicts, regenerate generated files rather than hand-merging them, check the build and tests, and leave the merge uncommitted; byre's end-of-turn commit concludes it as a merge commit. If resolving the conflicts took substantive changes rather than combining both sides as written, the prompt tells the agent to stop there and end its turn describing them; the turn's snapshot then gets its review like any other (see "Reviews"). byre doesn't inspect the merge itself: whether a change is substantive, and whether to stop, is the agent's call. On a Merge turn the prompt adds that byre squashes and pushes when the turn ends, and tells the agent to call the `stop_merge` MCP tool first if it stops (see below). The prompt ends by showing the task's current merge title and asking the agent to set a better one with `set_merge_title` if it no longer describes all the work on the branch. It is refused (409) while a session is `running`.
 

web/dist/task.js

@@ -4,7 +4,7 @@ import { markdown } from "./markdown.js";
 import { previewURL } from "./preview.js";
 import { findingCount, reviewCounts } from "./reviews.js";
 import { composerText, endSend, flushDrafts, forgetLostImages, lostImageCount, setComposerImages, setComposerText, startSend } from "./drafts.js";
-import { currentSession, findTask, focusTab, focusTask, onEach, pinSnapshot, setFocus, showReview, state } from "./state.js";
+import { currentSession, findTask, focusTab, focusTask, on, onEach, pickReview, pinSnapshot, setFocus, showReview, state } from "./state.js";
 import { taskStyle } from "./taskcolor.js";
 import { fillTodoLinks } from "./todolink.js";
 export const models = [
@@ -989,15 +989,138 @@ export async function confirmMerge(taskId) {
         void showAlert(errorText(err));
         return;
     }
-    const title = h("input", { type: "text", value: task.merge_title, required: true, maxLength: 72, pattern: ".*\\S.*", class: "modal-field", ariaLabel: "Commit title" });
+    const title = mergeDrafts.get(taskId) ?? task.merge_title;
-    const question = [h("p", { class: "modal-text" }, `Merge into ${target} as one commit titled:`), title];
+    mergeDrafts.delete(taskId);
-    const trailers = mergeTrailers(coAuthors);
+    let input;
-    if (trailers) {
+    const edit = h("button", { type: "button", class: "merge-title-edit", title: "Edit the commit title", ariaLabel: "Edit the commit title" }, "✎");
-        question.push(h("p", { class: "modal-text" }, trailers));
+    const row = h("div", { class: "merge-title" }, h("span", { class: "merge-title-text" }, title), edit);
+    edit.onclick = () => {
+        const field = h("input", { type: "text", value: title, required: true, maxLength: 72, pattern: ".*\\S.*", ariaLabel: "Commit title" });
+        field.onkeydown = (e) => {
+            if (e.key === "Enter") {
+                e.preventDefault();
+                row.closest("form")?.querySelector(".modal-actions button:last-child")?.click();
+            }
+        };
+        input = field;
+        replace(row, field);
+        field.focus();
+        field.select();
+    };
+    const warnings = h("ul", { class: "merge-warnings" });
+    const question = [h("p", { class: "merge-into" }, `Merge into ${target} as:`), row, warnings];
+    const co = coAuthorsSummary(coAuthors);
+    if (co) {
+        question.push(co);
     }
-    await confirmThen(question, "Merge", false, document.querySelector(".merge-button"), async () => {
+    const leave = (go) => {
-        await post("/api/tasks/{taskId}/merge", { path: { taskId }, body: { merge_title: title.value } });
+        mergeDrafts.set(taskId, input?.value ?? title);
+        warnings.closest("dialog")?.close("cancel");
+        go();
+    };
+    const watch = watchMergeWarnings(taskId, warnings, leave);
+    await watch.ready;
+    try {
+        await confirmThen(question, "Merge", false, document.querySelector(".merge-button"), async () => {
+            await post("/api/tasks/{taskId}/merge", { path: { taskId }, body: { merge_title: input?.value ?? title } });
+        });
+    }
+    finally {
+        watch.dispose();
+    }
+}
+const mergeDrafts = new Map();
+function watchMergeWarnings(taskId, list, leave) {
+    let reviewItems = [];
+    let target = 0;
+    const render = () => {
+        const questions = state.inbox.filter((i) => i.task_id === taskId && i.kind === "question").length;
+        const items = [...reviewItems];
+        if (questions > 0) {
+            const text = questions === 1 ? "The agent's open question will be dismissed" : `The agent's ${questions} open questions will be dismissed`;
+            items.push(mergeWarning(text, () => leave(() => focusTask(taskId, "inbox"))));
+        }
+        replace(list, ...items);
+        list.hidden = items.length === 0;
+    };
+    const load = async () => {
+        const started = ++target;
+        let items;
+        try {
+            items = await reviewWarnings(taskId, leave);
+        }
+        catch (err) {
+            items = [mergeWarning("Review status unavailable", undefined, errorText(err))];
+        }
+        if (started === target) {
+            reviewItems = items;
+            render();
+        }
+    };
+    const offs = [
+        on("event", (detail) => {
+            const ev = detail;
+            if (ev.task_id === taskId && (ev.kind === "snapshot.created" || ev.kind.startsWith("review."))) {
+                void load();
+            }
+        }),
+        on("inbox", render),
+    ];
+    return {
+        ready: load(),
+        dispose: () => {
+            target++;
+            for (const off of offs) {
+                off();
+            }
+        },
+    };
+}
+async function reviewWarnings(taskId, leave) {
+    const [snaps, reviews] = await Promise.all([get("/api/tasks/{taskId}/snapshots", { path: { taskId } }), get("/api/tasks/{taskId}/reviews", { path: { taskId } })]);
+    const latest = snaps.at(-1);
+    if (!latest) {
+        return [mergeWarning("These changes haven't been reviewed")];
+    }
+    const items = [];
+    const open = () => leave(() => {
+        focusTask(taskId, "reviews");
+        pickReview(latest.n);
     });
+    const r = reviews.find((r) => r.snapshot_n === latest.n);
+    if (!r) {
+        items.push(mergeWarning("The latest snapshot hasn't been reviewed", open));
+    }
+    else if (r.state === "running") {
+        items.push(mergeWarning("Review of the latest snapshot is still running", open));
+    }
+    else if (r.state === "failed") {
+        items.push(mergeWarning("Review of the latest snapshot failed", open));
+    }
+    else if (findingCount(r) > 0) {
+        items.push(mergeWarning(`Review of the latest snapshot found ${reviewCounts(r)}`, open));
+    }
+    const changes = await get("/api/tasks/{taskId}/changes", { path: { taskId }, query: { since: latest.commit_sha } });
+    if (changes.files.length > 0) {
+        items.push(mergeWarning("Changes since the last snapshot haven't been reviewed"));
+    }
+    return items;
+}
+function mergeWarning(text, open, detail) {
+    if (!open) {
+        return h("li", { class: "merge-warning", title: detail ?? "" }, `⚠ ${text}`);
+    }
+    const link = h("button", { type: "button", class: "merge-warning-link" }, text);
+    link.onclick = open;
+    return h("li", { class: "merge-warning" }, "⚠ ", link);
+}
+function coAuthorsSummary(coAuthors) {
+    if (coAuthors.length === 0) {
+        return null;
+    }
+    const missing = coAuthors.filter((p) => !p.git_name || !p.git_email).length;
+    const label = `${coAuthors.length} co-author${coAuthors.length === 1 ? "" : "s"}${missing > 0 ? ` (${missing} without git identity)` : ""}`;
+    return h("details", { class: "merge-coauthors" }, h("summary", {}, label), h("p", { class: "modal-text" }, mergeTrailers(coAuthors)));
 }
 function mergeTrailers(coAuthors) {
     const lines = [];

web/src/task.ts

@@ -4,7 +4,7 @@ import { markdown } from "./markdown.js";
 import { previewURL } from "./preview.js";
 import { findingCount, reviewCounts } from "./reviews.js";
 import { composerText, endSend, flushDrafts, forgetLostImages, lostImageCount, setComposerImages, setComposerText, startSend } from "./drafts.js";
-import { currentSession, findTask, focusTab, focusTask, onEach, pinSnapshot, setFocus, showReview, state, type Topic } from "./state.js";
+import { currentSession, findTask, focusTab, focusTask, on, onEach, pickReview, pinSnapshot, setFocus, showReview, state, type Topic } from "./state.js";
 import { taskStyle } from "./taskcolor.js";
 import { fillTodoLinks } from "./todolink.js";
 
@@ -1171,15 +1171,139 @@ export async function confirmMerge(taskId: string): Promise<void> {
     void showAlert(errorText(err));
     return;
   }
-  const title = h("input", { type: "text", value: task.merge_title, required: true, maxLength: 72, pattern: ".*\\S.*", class: "modal-field", ariaLabel: "Commit title" });
+  const title = mergeDrafts.get(taskId) ?? task.merge_title;
-  const question = [h("p", { class: "modal-text" }, `Merge into ${target} as one commit titled:`), title];
+  mergeDrafts.delete(taskId);
-  const trailers = mergeTrailers(coAuthors);
+  let input: HTMLInputElement | undefined;
-  if (trailers) {
+  const edit = h("button", { type: "button", class: "merge-title-edit", title: "Edit the commit title", ariaLabel: "Edit the commit title" }, "✎");
-    question.push(h("p", { class: "modal-text" }, trailers));
+  const row = h("div", { class: "merge-title" }, h("span", { class: "merge-title-text" }, title), edit);
+  edit.onclick = () => {
+    const field = h("input", { type: "text", value: title, required: true, maxLength: 72, pattern: ".*\\S.*", ariaLabel: "Commit title" });
+    field.onkeydown = (e) => {
+      if (e.key === "Enter") {
+        e.preventDefault();
+        row.closest("form")?.querySelector<HTMLButtonElement>(".modal-actions button:last-child")?.click();
+      }
+    };
+    input = field;
+    replace(row, field);
+    field.focus();
+    field.select();
+  };
+  const warnings = h("ul", { class: "merge-warnings" });
+  const question: Node[] = [h("p", { class: "merge-into" }, `Merge into ${target} as:`), row, warnings];
+  const co = coAuthorsSummary(coAuthors);
+  if (co) {
+    question.push(co);
   }
-  await confirmThen(question, "Merge", false, document.querySelector(".merge-button"), async () => {
+  const leave = (go: () => void) => {
-    await post("/api/tasks/{taskId}/merge", { path: { taskId }, body: { merge_title: title.value } });
+    mergeDrafts.set(taskId, input?.value ?? title);
-  });
+    warnings.closest("dialog")?.close("cancel");
+    go();
+  };
+  const watch = watchMergeWarnings(taskId, warnings, leave);
+  await watch.ready;
+  try {
+    await confirmThen(question, "Merge", false, document.querySelector(".merge-button"), async () => {
+      await post("/api/tasks/{taskId}/merge", { path: { taskId }, body: { merge_title: input?.value ?? title } });
+    });
+  } finally {
+    watch.dispose();
+  }
+}
+
+const mergeDrafts = new Map<string, string>();
+
+function watchMergeWarnings(taskId: string, list: HTMLElement, leave: (go: () => void) => void): { ready: Promise<void>; dispose: () => void } {
+  let reviewItems: HTMLElement[] = [];
+  let target = 0;
+  const render = () => {
+    const questions = state.inbox.filter((i) => i.task_id === taskId && i.kind === "question").length;
+    const items = [...reviewItems];
+    if (questions > 0) {
+      const text = questions === 1 ? "The agent's open question will be dismissed" : `The agent's ${questions} open questions will be dismissed`;
+      items.push(mergeWarning(text, () => leave(() => focusTask(taskId, "inbox"))));
+    }
+    replace(list, ...items);
+    list.hidden = items.length === 0;
+  };
+  const load = async () => {
+    const started = ++target;
+    let items: HTMLElement[];
+    try {
+      items = await reviewWarnings(taskId, leave);
+    } catch (err) {
+      items = [mergeWarning("Review status unavailable", undefined, errorText(err))];
+    }
+    if (started === target) {
+      reviewItems = items;
+      render();
+    }
+  };
+  const offs = [
+    on("event", (detail) => {
+      const ev = detail as ByreEvent;
+      if (ev.task_id === taskId && (ev.kind === "snapshot.created" || ev.kind.startsWith("review."))) {
+        void load();
+      }
+    }),
+    on("inbox", render),
+  ];
+  return {
+    ready: load(),
+    dispose: () => {
+      target++;
+      for (const off of offs) {
+        off();
+      }
+    },
+  };
+}
+
+async function reviewWarnings(taskId: string, leave: (go: () => void) => void): Promise<HTMLElement[]> {
+  const [snaps, reviews] = await Promise.all([get("/api/tasks/{taskId}/snapshots", { path: { taskId } }), get("/api/tasks/{taskId}/reviews", { path: { taskId } })]);
+  const latest = snaps.at(-1);
+  if (!latest) {
+    return [mergeWarning("These changes haven't been reviewed")];
+  }
+  const items: HTMLElement[] = [];
+  const open = () =>
+    leave(() => {
+      focusTask(taskId, "reviews");
+      pickReview(latest.n);
+    });
+  const r = reviews.find((r) => r.snapshot_n === latest.n);
+  if (!r) {
+    items.push(mergeWarning("The latest snapshot hasn't been reviewed", open));
+  } else if (r.state === "running") {
+    items.push(mergeWarning("Review of the latest snapshot is still running", open));
+  } else if (r.state === "failed") {
+    items.push(mergeWarning("Review of the latest snapshot failed", open));
+  } else if (findingCount(r) > 0) {
+    items.push(mergeWarning(`Review of the latest snapshot found ${reviewCounts(r)}`, open));
+  }
+  const changes = await get("/api/tasks/{taskId}/changes", { path: { taskId }, query: { since: latest.commit_sha } });
+  if (changes.files.length > 0) {
+    items.push(mergeWarning("Changes since the last snapshot haven't been reviewed"));
+  }
+  return items;
+}
+
+function mergeWarning(text: string, open?: () => void, detail?: string): HTMLElement {
+  if (!open) {
+    return h("li", { class: "merge-warning", title: detail ?? "" }, `⚠ ${text}`);
+  }
+  const link = h("button", { type: "button", class: "merge-warning-link" }, text);
+  link.onclick = open;
+  return h("li", { class: "merge-warning" }, "⚠ ", link);
+}
+
+function coAuthorsSummary(coAuthors: Participant[]): HTMLElement | null {
+  if (coAuthors.length === 0) {
+    return null;
+  }
+  const missing = coAuthors.filter((p) => !p.git_name || !p.git_email).length;
+  const label = `${coAuthors.length} co-author${coAuthors.length === 1 ? "" : "s"}${missing > 0 ? ` (${missing} without git identity)` : ""}`;
+  return h("details", { class: "merge-coauthors" }, h("summary", {}, label), h("p", { class: "modal-text" }, mergeTrailers(coAuthors)));
 }
 
 function mergeTrailers(coAuthors: Participant[]): string {

web/style.css

@@ -1965,6 +1965,62 @@ pre.diff .mark {
   margin: 0 0 16px;
 }
 
+.merge-into {
+  margin: 0 0 4px;
+  color: var(--muted);
+}
+
+.merge-title {
+  display: flex;
+  align-items: center;
+  gap: 8px;
+  margin: 0 0 16px;
+  font-weight: 700;
+  overflow-wrap: anywhere;
+}
+
+.merge-title input {
+  font-weight: 400;
+}
+
+.merge-title-edit {
+  padding: 0 6px;
+  border-color: transparent;
+  background: none;
+  color: var(--muted);
+  font-weight: 400;
+}
+
+.merge-warnings {
+  list-style: none;
+  margin: 0 0 16px;
+  padding: 0;
+  color: var(--warn);
+}
+
+.merge-warning-link {
+  padding: 0;
+  border: none;
+  background: none;
+  color: inherit;
+  text-align: left;
+  text-decoration: underline;
+}
+
+.merge-coauthors {
+  margin: 0 0 16px;
+  color: var(--muted);
+}
+
+.merge-coauthors summary {
+  cursor: pointer;
+}
+
+.merge-coauthors .modal-text {
+  margin: 8px 0 0;
+  color: var(--text);
+}
+
 .shortcuts-title {
   margin: 0 0 12px;
   font-size: 1.1em;