Commit 2fe2855ca93b

Vincent Demeester <vincent@sbr.pm>
2026-03-17 20:18:43
fix(pi): serialize approval dialogs in extensions
Added mutex to approval gates in both github and jira extensions to prevent UI deadlocks when the LLM issues multiple write tool calls in parallel. Also added explicit instruction in tool descriptions to call write operations sequentially.
1 parent d08c849
dots/pi/agent/extensions/github/index.ts
@@ -149,6 +149,7 @@ export default function (pi: ExtensionAPI) {
 		description:
 			"Manage GitHub PRs, issues, checks, and runs via gh CLI. " +
 			"Write operations require user approval. " +
+			"IMPORTANT: Call write operations (pr-create, pr-merge, pr-review, pr-comment, pr-close, pr-ready, pr-line-comment, pr-review-comments, issue-create, issue-close, issue-comment, issue-edit, checks-restart) ONE AT A TIME, never in parallel — parallel approval dialogs deadlock the UI. " +
 			"checks-log accepts runId or number (PR) — PR auto-selects first failed run. " +
 			"pr-review-comments submits a review with inline comments. " +
 			"issue-create with parent auto-links as sub-issue.",
dots/pi/agent/extensions/github/utils.ts
@@ -511,7 +511,7 @@ export function getReviewDecisionText(decision: string): string {
 }
 
 // ============================================================================
-// Approval gate helper
+// Approval gate helper (with mutex to prevent parallel dialog deadlocks)
 // ============================================================================
 
 export type ApprovalResult =
@@ -519,6 +519,12 @@ export type ApprovalResult =
 	| { outcome: "modify" }
 	| { outcome: "rejected" };
 
+// Mutex to serialize concurrent approval dialogs.
+// When the LLM issues multiple write tool calls in parallel, each hits
+// approvalGate(). Without serialization, multiple ctx.ui.select() dialogs
+// overlap and cause a UI deadlock (the same bug as the Jira extension).
+let approvalMutex: Promise<void> = Promise.resolve();
+
 /**
  * Three-way approval gate: Accept / Modify / Reject.
  *
@@ -527,6 +533,9 @@ export type ApprovalResult =
  *   submitting. The LLM should ask the user what to change.
  * - "Reject" means the user does not want this action at all.
  *
+ * Uses a mutex so parallel tool calls present dialogs one at a time
+ * instead of all at once (which deadlocks the UI).
+ *
  * Returns the user's choice so the caller can build the appropriate response.
  */
 export async function approvalGate(
@@ -534,17 +543,23 @@ export async function approvalGate(
 	title: string,
 	description: string,
 ): Promise<ApprovalResult> {
-	const prompt = description ? `${title}\n\n${description}` : title;
-	const choice = await ctx.ui.select(prompt, [
-		"✓ Accept",
-		"✎ Modify",
-		"✗ Reject",
-	]);
+	// Chain onto the mutex so dialogs appear sequentially
+	const result = await new Promise<ApprovalResult>((resolve) => {
+		approvalMutex = approvalMutex.then(async () => {
+			const prompt = description ? `${title}\n\n${description}` : title;
+			const choice = await ctx.ui.select(prompt, [
+				"✓ Accept",
+				"✎ Modify",
+				"✗ Reject",
+			]);
 
-	if (choice === "✓ Accept") return { outcome: "accepted" };
-	if (choice === "✎ Modify") return { outcome: "modify" };
-	// undefined (Escape) or "✗ Reject"
-	return { outcome: "rejected" };
+			if (choice === "✓ Accept") resolve({ outcome: "accepted" });
+			else if (choice === "✎ Modify") resolve({ outcome: "modify" });
+			else resolve({ outcome: "rejected" });
+		});
+	});
+
+	return result;
 }
 
 /**
dots/pi/agent/extensions/jira/actions.ts
@@ -13,6 +13,7 @@ import {
 	extractIssueKeys,
 	getErrorMessage,
 	parseIssueListJSON,
+	serializedConfirm,
 } from "./utils";
 
 /**
@@ -269,7 +270,7 @@ export async function handleCreate(
 	if (ctx.hasUI) {
 		const confirmMessage = buildCreateConfirmation(params);
 
-		const confirmed = await ctx.ui.confirm("Create Jira Issue?", confirmMessage);
+		const confirmed = await serializedConfirm(ctx, "Create Jira Issue?", confirmMessage);
 
 		if (!confirmed) {
 			ctx.ui.notify("Create cancelled", "info");
@@ -370,7 +371,7 @@ export async function handleUpdate(
 	if (ctx.hasUI) {
 		const confirmMessage = buildUpdateConfirmation(params);
 
-		const confirmed = await ctx.ui.confirm(`Update ${params.key}?`, confirmMessage);
+		const confirmed = await serializedConfirm(ctx, `Update ${params.key}?`, confirmMessage);
 
 		if (!confirmed) {
 			ctx.ui.notify("Update cancelled", "info");
@@ -472,7 +473,7 @@ export async function handleComment(
 	if (ctx.hasUI) {
 		const confirmMessage = buildCommentConfirmation(params);
 
-		const confirmed = await ctx.ui.confirm(`Add comment to ${params.key}?`, confirmMessage);
+		const confirmed = await serializedConfirm(ctx, `Add comment to ${params.key}?`, confirmMessage);
 
 		if (!confirmed) {
 			ctx.ui.notify("Comment cancelled", "info");
@@ -535,7 +536,7 @@ export async function handleTransition(
 	if (ctx.hasUI) {
 		const confirmMessage = buildTransitionConfirmation(params);
 
-		const confirmed = await ctx.ui.confirm(`Transition ${params.key}?`, confirmMessage);
+		const confirmed = await serializedConfirm(ctx, `Transition ${params.key}?`, confirmMessage);
 
 		if (!confirmed) {
 			ctx.ui.notify("Transition cancelled", "info");
dots/pi/agent/extensions/jira/attachment-actions.ts
@@ -4,7 +4,7 @@
 
 import type { ExtensionAPI, ExtensionContext } from "@mariozechner/pi-coding-agent";
 import type { JiraDetails } from "./types";
-import { getErrorMessage } from "./utils";
+import { getErrorMessage, serializedConfirm } from "./utils";
 
 /**
  * Attach file to issue
@@ -46,7 +46,7 @@ export async function handleAttach(
 	if (ctx.hasUI) {
 		const confirmMessage = `Issue: ${params.key}\nFile: ${params.file}\n\nThis will upload the file as an attachment.`;
 
-		const confirmed = await ctx.ui.confirm("Attach file?", confirmMessage);
+		const confirmed = await serializedConfirm(ctx, "Attach file?", confirmMessage);
 
 		if (!confirmed) {
 			ctx.ui.notify("Attach cancelled", "info");
dots/pi/agent/extensions/jira/epic-actions.ts
@@ -4,7 +4,7 @@
 
 import type { ExtensionAPI, ExtensionContext } from "@mariozechner/pi-coding-agent";
 import type { JiraDetails } from "./types";
-import { getErrorMessage } from "./utils";
+import { getErrorMessage, serializedConfirm } from "./utils";
 
 /**
  * View epic with all child issues
@@ -93,7 +93,7 @@ export async function handleLinkToEpic(
 	if (ctx.hasUI) {
 		const confirmMessage = `Issue: ${params.issue}\nEpic: ${params.epic}\n\nThis will link the issue to the epic.`;
 
-		const confirmed = await ctx.ui.confirm(`Link ${params.issue} to epic?`, confirmMessage);
+		const confirmed = await serializedConfirm(ctx, `Link ${params.issue} to epic?`, confirmMessage);
 
 		if (!confirmed) {
 			ctx.ui.notify("Link to epic cancelled", "info");
dots/pi/agent/extensions/jira/index.ts
@@ -105,7 +105,8 @@ export default function (pi: ExtensionAPI) {
 			"search (JQL search), create (create issue), update (update field), " +
 			"comment (add comment), transition (change state). " +
 			"Use 'me' as assignee value to refer to current user. " +
-			"Write operations (create, update, comment, transition) require user approval.",
+			"Write operations (create, update, comment, transition, link, unlink, attach) require user approval. " +
+			"IMPORTANT: Call write operations ONE AT A TIME, never in parallel — parallel approval dialogs deadlock the UI.",
 
 		parameters: Type.Object({
 			action: StringEnum([
dots/pi/agent/extensions/jira/utils.ts
@@ -2,9 +2,38 @@
  * Utility functions for Jira extension
  */
 
-import type { Theme } from "@mariozechner/pi-coding-agent";
+import type { Theme, ExtensionContext } from "@mariozechner/pi-coding-agent";
 import type { JiraIssue } from "./types";
 
+// ============================================================================
+// Serialized confirm helper (prevents parallel approval dialog deadlocks)
+// ============================================================================
+
+// Mutex to serialize concurrent confirmation dialogs.
+// When the LLM issues multiple write tool calls in parallel, each hits
+// ctx.ui.confirm(). Without serialization, multiple concurrent dialogs
+// deadlock the UI.
+let confirmMutex: Promise<void> = Promise.resolve();
+
+/**
+ * Serialized wrapper around ctx.ui.confirm().
+ * Queues concurrent calls so they present one at a time instead of
+ * all at once (which deadlocks the UI).
+ */
+export async function serializedConfirm(
+	ctx: ExtensionContext,
+	title: string,
+	description: string,
+): Promise<boolean> {
+	const result = await new Promise<boolean>((resolve) => {
+		confirmMutex = confirmMutex.then(async () => {
+			const confirmed = await ctx.ui.confirm(title, description);
+			resolve(confirmed);
+		});
+	});
+	return result;
+}
+
 /**
  * Parse jira issue list output from JSON (--raw flag)
  */