Offer "After review" in the Merge dialog while a review runs
This commit is contained in:
Ian Gulliver
+77
-14
@@ -1,5 +1,5 @@
|
||||
import { get, post, type AgentSession, type ByreEvent, type Participant, type Schemas, type Snapshot } from "./api.js";
|
||||
import { ago, copyable, errorText, h, replace, showAlert, showConfirm } from "./dom.js";
|
||||
import { del, get, post, type AgentSession, type ByreEvent, type Participant, type Schemas, type Snapshot } from "./api.js";
|
||||
import { ago, copyable, errorText, h, replace, showAlert, showChoice, showConfirm } from "./dom.js";
|
||||
import { markdown } from "./markdown.js";
|
||||
import { previewURL } from "./preview.js";
|
||||
import { findingCount, reviewCounts, severityCounts, unignoredFindings } from "./reviews.js";
|
||||
@@ -172,11 +172,13 @@ function taskHead(rerender: () => void): Head {
|
||||
? "Waiting for the parent task's agent to merge this subtask"
|
||||
: held
|
||||
? "Waiting for the merge in progress in this project to finish"
|
||||
: task.parent_task_id !== undefined
|
||||
: task.merge_after_review
|
||||
? "Merges when the running review finishes with no findings; open to merge now instead (⌘S)"
|
||||
: task.parent_task_id !== undefined
|
||||
? `Merge this subtask into its parent task's branch, or into ${target} (⌘S)`
|
||||
: `The agent merges ${target} in, then byre squashes into one commit on ${target} and pushes it to origin; held while another task in the project is merging (⌘S)`,
|
||||
},
|
||||
intoParent ? "Merging into parent" : held ? "Merge held" : "Merge",
|
||||
intoParent ? "Merging into parent" : held ? "Merge held" : task.merge_after_review ? "Merge after review" : "Merge",
|
||||
);
|
||||
merge.onclick = () => confirmMerge(task.id);
|
||||
const mergeIn = h("button", { disabled: busy, title: `Ask the agent to bring ${target} into this branch and resolve any conflicts` }, `Sync from ${target}`);
|
||||
@@ -614,6 +616,7 @@ export function mountChat(root: HTMLElement, src: ChatSource, makeHead: (rerende
|
||||
}
|
||||
}
|
||||
fillTodoLinks(transcript, models[0]!);
|
||||
syncMergeAfterReview(transcript);
|
||||
pin();
|
||||
jump();
|
||||
};
|
||||
@@ -630,6 +633,7 @@ export function mountChat(root: HTMLElement, src: ChatSource, makeHead: (rerende
|
||||
syncRunning();
|
||||
fillTodoLinks(box, models[0]!);
|
||||
earlier.after(box);
|
||||
syncMergeAfterReview(transcript);
|
||||
transcript.scrollTop = transcript.scrollHeight - fromBottom;
|
||||
};
|
||||
|
||||
@@ -707,7 +711,10 @@ export function mountChat(root: HTMLElement, src: ChatSource, makeHead: (rerende
|
||||
loading.remove();
|
||||
watchTop();
|
||||
}),
|
||||
onEach(["tasks", "sessions"], () => fillTodoLinks(transcript, models[0]!)),
|
||||
onEach(["tasks", "sessions"], () => {
|
||||
fillTodoLinks(transcript, models[0]!);
|
||||
syncMergeAfterReview(transcript);
|
||||
}),
|
||||
on("jump", jump),
|
||||
];
|
||||
|
||||
@@ -1273,6 +1280,28 @@ function renderEvent(ev: ByreEvent, ctx: Ctx): HTMLElement | null {
|
||||
case "task.merge_stopped":
|
||||
return h("div", { class: "note" }, `${ev.actor} stopped the merge for review: ${String(p["reason"])} `, when);
|
||||
|
||||
case "task.merge_after_review": {
|
||||
const taskId = String(p["id"]);
|
||||
const cancel = h("button", { type: "button", class: "merge-after-review-cancel", hidden: true, title: "Withdraw this request; nothing is merged", data: { taskId } }, "Cancel");
|
||||
cancel.onclick = async () => {
|
||||
cancel.disabled = true;
|
||||
try {
|
||||
await del("/api/tasks/{taskId}/merge-after-review", { path: { taskId } });
|
||||
} catch (err) {
|
||||
void showAlert(errorText(err));
|
||||
} finally {
|
||||
cancel.disabled = false;
|
||||
}
|
||||
};
|
||||
return h("div", { class: "note" }, `${ev.actor} asked to merge once the review of snapshot #${String(p["snapshot_n"])} finishes with no findings `, when, cancel);
|
||||
}
|
||||
|
||||
case "task.merge_after_review_cancelled":
|
||||
return h("div", { class: "note" }, `${ev.actor} cancelled the merge after review `, when);
|
||||
|
||||
case "task.merge_after_review_dropped":
|
||||
return h("div", { class: "note" }, `didn't merge after the review: ${String(p["reason"])} `, when);
|
||||
|
||||
case "preview.exposed": {
|
||||
const url = previewURL(Number(p["port"]));
|
||||
return h("div", { class: "note" }, `${ev.actor} exposed "${String(p["name"])}" at `, h("a", { href: url, target: "_blank", rel: "noopener" }, url), " ", when);
|
||||
@@ -1302,6 +1331,14 @@ function renderEvent(ev: ByreEvent, ctx: Ctx): HTMLElement | null {
|
||||
return null;
|
||||
}
|
||||
|
||||
function syncMergeAfterReview(root: ParentNode): void {
|
||||
const buttons = [...root.querySelectorAll<HTMLButtonElement>(".merge-after-review-cancel")];
|
||||
const last = buttons.at(-1);
|
||||
for (const b of buttons) {
|
||||
b.hidden = b !== last || findTask(b.dataset["taskId"] ?? "")?.merge_after_review !== true;
|
||||
}
|
||||
}
|
||||
|
||||
function taskLink(id: string): Node {
|
||||
const task = findTask(id);
|
||||
if (task?.status !== "active") {
|
||||
@@ -1427,11 +1464,33 @@ export async function confirmMerge(taskId: string): Promise<void> {
|
||||
};
|
||||
const watch = watchMergeWarnings(taskId, warnings, leave);
|
||||
await watch.ready;
|
||||
const anchor = document.querySelector(".merge-button");
|
||||
const merge = async (afterReview: boolean) => {
|
||||
const body = toParent ? { target: "parent" as const } : { merge_title: input?.value ?? title };
|
||||
try {
|
||||
await post("/api/tasks/{taskId}/merge", { path: { taskId }, body: afterReview ? { ...body, after_review: true } : body });
|
||||
} catch (err) {
|
||||
await showAlert(errorText(err));
|
||||
}
|
||||
};
|
||||
try {
|
||||
await confirmThen(question, "Merge", false, document.querySelector(".merge-button"), async () => {
|
||||
const body = toParent ? { target: "parent" as const } : { merge_title: input?.value ?? title };
|
||||
await post("/api/tasks/{taskId}/merge", { path: { taskId }, body });
|
||||
});
|
||||
if (!watch.reviewRunning()) {
|
||||
if (await showConfirm(question, "Merge", false, anchor)) {
|
||||
await merge(false);
|
||||
}
|
||||
return;
|
||||
}
|
||||
const choice = await showChoice(
|
||||
question,
|
||||
[
|
||||
{ label: "Merge now", value: "now" },
|
||||
{ label: "After review", value: "after", class: "primary", title: "Merge once the running review finishes with no findings" },
|
||||
],
|
||||
anchor,
|
||||
);
|
||||
if (choice === "now" || choice === "after") {
|
||||
await merge(choice === "after");
|
||||
}
|
||||
} finally {
|
||||
watch.dispose();
|
||||
}
|
||||
@@ -1439,8 +1498,9 @@ export async function confirmMerge(taskId: string): Promise<void> {
|
||||
|
||||
const mergeDrafts = new Map<string, string>();
|
||||
|
||||
function watchMergeWarnings(taskId: string, list: HTMLElement, leave: (go: () => void) => void): { ready: Promise<void>; dispose: () => void } {
|
||||
function watchMergeWarnings(taskId: string, list: HTMLElement, leave: (go: () => void) => void): { ready: Promise<void>; reviewRunning: () => boolean; dispose: () => void } {
|
||||
let reviewItems: HTMLElement[] = [];
|
||||
let running = false;
|
||||
let target = 0;
|
||||
const render = () => {
|
||||
const questions = state.inbox.filter((i) => i.task_id === taskId && i.kind === "question").length;
|
||||
@@ -1455,13 +1515,15 @@ function watchMergeWarnings(taskId: string, list: HTMLElement, leave: (go: () =>
|
||||
const load = async () => {
|
||||
const started = ++target;
|
||||
let items: HTMLElement[];
|
||||
let isRunning = false;
|
||||
try {
|
||||
items = await reviewWarnings(taskId, leave);
|
||||
({ items, running: isRunning } = await reviewWarnings(taskId, leave));
|
||||
} catch (err) {
|
||||
items = [mergeWarning("Review status unavailable", undefined, errorText(err))];
|
||||
}
|
||||
if (started === target) {
|
||||
reviewItems = items;
|
||||
running = isRunning;
|
||||
render();
|
||||
}
|
||||
};
|
||||
@@ -1476,6 +1538,7 @@ function watchMergeWarnings(taskId: string, list: HTMLElement, leave: (go: () =>
|
||||
];
|
||||
return {
|
||||
ready: load(),
|
||||
reviewRunning: () => running,
|
||||
dispose: () => {
|
||||
target++;
|
||||
for (const off of offs) {
|
||||
@@ -1485,11 +1548,11 @@ function watchMergeWarnings(taskId: string, list: HTMLElement, leave: (go: () =>
|
||||
};
|
||||
}
|
||||
|
||||
async function reviewWarnings(taskId: string, leave: (go: () => void) => void): Promise<HTMLElement[]> {
|
||||
async function reviewWarnings(taskId: string, leave: (go: () => void) => void): Promise<{ items: HTMLElement[]; running: boolean }> {
|
||||
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")];
|
||||
return { items: [mergeWarning("These changes haven't been reviewed")], running: false };
|
||||
}
|
||||
const items: HTMLElement[] = [];
|
||||
const open = () =>
|
||||
@@ -1511,7 +1574,7 @@ async function reviewWarnings(taskId: string, leave: (go: () => void) => void):
|
||||
if (changes.files.length > 0) {
|
||||
items.push(mergeWarning("Changes since the last snapshot haven't been reviewed"));
|
||||
}
|
||||
return items;
|
||||
return { items, running: r?.state === "running" };
|
||||
}
|
||||
|
||||
function mergeWarning(text: string, open?: () => void, detail?: string): HTMLElement {
|
||||
|
||||
Reference in New Issue
Block a user