fix(pi-herdr-rename): harden title synchronization

Author: HenryQW

packages/pi-herdr-rename/README.md

@@ -36,4 +36,4 @@ Display titles are natural task phrases, preferably three or four words and alwa
 
 In a linked worktree, a detached checkout or Herdr `worktree/...` branch is renamed; an existing non-generated branch stays. A generated workspace label such as `worktree-brave-meadow-4aa8` becomes the display title automatically; `/rename` also replaces a custom workspace name. Enclosing Herdr tab updates only when this pane is tab's only pane. Outside Herdr, only Pi session name changes.
 
-Tries assigned profile primary, then fallback, while honoring configured thinking level. Never substitutes current session model. No viable route leaves titles unchanged. Resuming a session created by this version reapplies saved display title and semantic branch without another model request. Older titles receive no migration.
+Tries assigned profile primary, then fallback, while honoring configured thinking level. Never substitutes current session model. No viable route leaves titles unchanged. Resuming a session created by this version reapplies saved display title and semantic branch without another model request. Herdr and Git synchronization failures appear as warnings; cancellation by a newer rename remains silent. Older titles receive no migration.

packages/pi-herdr-rename/extensions/rename.ts

@@ -3,7 +3,7 @@ import {
 	type ExtensionAPI,
 	type ExtensionContext,
 } from "@earendil-works/pi-coding-agent";
-import { createHerdrClient } from "@henryqw/pi-herdr";
+import { createHerdrClient, withWorktreeLock } from "@henryqw/pi-herdr";
 import {
 	readTaskModelsConfig,
 	resolveConfiguredTaskRoutes,
@@ -42,15 +42,22 @@ function configuredRenameRoutes(ctx: ExtensionContext): ResolvedTaskRoute[] {
 	}
 }
 
+function validSubject(subject: string): boolean {
+	return /^[a-z0-9]+(?: [a-z0-9]+)*$/.test(subject)
+		&& subject.length <= DISPLAY_MAX_CHARS
+		&& subject.split(" ").length <= DISPLAY_MAX_WORDS;
+}
+
+function isDisplayTitle(value: unknown): value is string {
+	if (typeof value !== "string" || !value) return false;
+	const subject = value[0].toLowerCase() + value.slice(1);
+	return validSubject(subject) && value === subject[0].toUpperCase() + subject.slice(1);
+}
+
 function parseGeneratedTitle(title: string): GeneratedTitle | undefined {
-	const match = /^([a-z][a-z0-9-]*): ([a-z0-9]+(?: [a-z0-9]+)*)$/.exec(title);
-	if (!match) return undefined;
+	const match = /^([a-z][a-z0-9-]*): (.+)$/.exec(title);
+	if (!match || match[1].length > SEMANTIC_TYPE_MAX_CHARS || !validSubject(match[2])) return undefined;
 	const subject = match[2];
-	if (
-		match[1].length > SEMANTIC_TYPE_MAX_CHARS ||
-		subject.length > DISPLAY_MAX_CHARS ||
-		subject.split(" ").length > DISPLAY_MAX_WORDS
-	) return undefined;
 	return {
 		display: subject[0].toUpperCase() + subject.slice(1),
 		branch: `${match[1]}/${subject.replaceAll(" ", "-")}`,
@@ -63,7 +70,7 @@ function savedTitle(ctx: ExtensionContext): GeneratedTitle | undefined {
 		.find((candidate) => candidate.type === "custom" && candidate.customType === TITLE_STATE_TYPE);
 	if (entry?.type !== "custom" || !entry.data || typeof entry.data !== "object" || Array.isArray(entry.data)) return undefined;
 	const { display, branch } = entry.data as { display?: unknown; branch?: unknown };
-	return typeof display === "string" && typeof branch === "string" && SEMANTIC_BRANCH.test(branch)
+	return isDisplayTitle(display) && typeof branch === "string" && SEMANTIC_BRANCH.test(branch)
 		? { display, branch }
 		: undefined;
 }
@@ -269,15 +276,18 @@ export default function herdrRenameExtension(pi: ExtensionAPI): void {
 			return result.stdout.trim();
 		};
 
-		const branch = await runGit(["branch", "--show-current"]);
-		if (!branch || branch.startsWith("worktree/")) {
+		await withWorktreeLock(checkoutPath, async () => {
 			if (!isCurrent(request, controller)) return;
-			const branches = (await runGit(["for-each-ref", "--format=%(refname:short)", "refs/heads"]))
-				.split("\n")
-				.filter(Boolean);
-			const semanticBranch = availableBranch(branchCandidate, branches);
-			await runGit(branch ? ["branch", "-m", semanticBranch] : ["switch", "-c", semanticBranch]);
-		}
+			const branch = await runGit(["branch", "--show-current"]);
+			if (!branch || branch.startsWith("worktree/")) {
+				if (!isCurrent(request, controller)) return;
+				const branches = (await runGit(["for-each-ref", "--format=%(refname:short)", "refs/heads"]))
+					.split("\n")
+					.filter(Boolean);
+				const semanticBranch = availableBranch(branchCandidate, branches);
+				await runGit(branch ? ["branch", "-m", semanticBranch] : ["switch", "-c", semanticBranch]);
+			}
+		});
 	};
 
 	const begin = () => {
@@ -307,7 +317,7 @@ export default function herdrRenameExtension(pi: ExtensionAPI): void {
 			await applyHerdr(title.display, title.branch, previousDisplayTitle, manual, request, controller);
 			return title.display;
 		} catch (error) {
-			if (isCurrent(request, controller) && (manual || error instanceof RenameModelError)) {
+			if (isCurrent(request, controller)) {
 				ctx.ui.notify(error instanceof Error ? error.message : "Rename failed.", "warning");
 			}
 			return undefined;
@@ -346,7 +356,11 @@ export default function herdrRenameExtension(pi: ExtensionAPI): void {
 
 		const { request, controller } = begin();
 		void applyHerdr(title, saved.branch, saved.display, false, request, controller)
-			.catch(() => undefined)
+			.catch((error) => {
+				if (isCurrent(request, controller)) {
+					ctx.ui.notify(error instanceof Error ? error.message : "Rename failed.", "warning");
+				}
+			})
 			.finally(() => finish(request, controller));
 	});
 

packages/pi-herdr-rename/test/rename.test.ts

@@ -4,6 +4,7 @@ import { tmpdir } from "node:os";
 import { join } from "node:path";
 import test from "node:test";
 import type { ExtensionAPI, ExtensionContext } from "@earendil-works/pi-coding-agent";
+import { withWorktreeLock } from "@henryqw/pi-herdr";
 import herdrRenameExtension from "../extensions/rename.ts";
 
 type Handler = (event: any, ctx: ExtensionContext) => unknown;
@@ -213,8 +214,10 @@ test("manual rename disarms a pending automatic rename", async () => {
 });
 
 test("saved display titles keep semantic branches, replace generated branches, and preserve custom workspace names", async () => {
-	await withAgentDir(async () => {
+	await withAgentDir(async (dir) => {
 		process.env.HERDR_PANE_ID = "pane-1";
+		const checkoutPath = join(dir, "worktree");
+		await mkdir(checkoutPath);
 		for (const [paneCount, workspaceName, isLinkedWorktree, currentBranch, existingBranches, renameWorkspace, gitMutation] of [
 			[1, "worktree-brave-meadow-4aa8", true, "fix/title-length", [], true, undefined],
 			[2, "lucky-field-f694", true, "feat/new-loader", [], true, undefined],
@@ -239,7 +242,7 @@ test("saved display titles keep semantic branches, replace generated branches, a
 						return success(JSON.stringify({ result: { tab: { pane_count: paneCount } } }));
 					}
 					if (args[0] === "workspace" && args[1] === "get") {
-						return success(JSON.stringify({ result: { workspace: { label: workspaceName, worktree: { checkout_path: "/repo/worktree", is_linked_worktree: isLinkedWorktree } } } }));
+						return success(JSON.stringify({ result: { workspace: { label: workspaceName, worktree: { checkout_path: checkoutPath, is_linked_worktree: isLinkedWorktree } } } }));
 					}
 					return success("{}");
 				},
@@ -257,7 +260,7 @@ test("saved display titles keep semantic branches, replace generated branches, a
 				app.execCalls.filter((args) => (args[0] === "branch" && args[1] === "-m") || args[0] === "switch"),
 				gitMutation ? [gitMutation] : [],
 			);
-			assert.deepEqual(gitCwds, Array(isLinkedWorktree ? 1 + Number(Boolean(gitMutation)) * 2 : 0).fill("/repo/worktree"));
+			assert.deepEqual(gitCwds, Array(isLinkedWorktree ? 1 + Number(Boolean(gitMutation)) * 2 : 0).fill(checkoutPath));
 			assert.equal(app.execCalls.filter((args) => args[0] === "for-each-ref").length, Number(Boolean(gitMutation)));
 			if (gitMutation) {
 				assert.ok(app.execCalls.some((args) => args.join("\0") === "for-each-ref\0--format=%(refname:short)\0refs/heads"));
@@ -271,8 +274,10 @@ test("saved display titles keep semantic branches, replace generated branches, a
 });
 
 test("manual rename updates generated and custom workspace titles", async () => {
-	await withAgentDir(async () => {
+	await withAgentDir(async (dir) => {
 		process.env.HERDR_PANE_ID = "pane-1";
+		const checkoutPath = join(dir, "worktree");
+		await mkdir(checkoutPath);
 		for (const workspaceName of ["Saved title", "Custom workspace"]) {
 			const app = harness({
 				sessionName: "Saved title",
@@ -290,7 +295,7 @@ test("manual rename updates generated and custom workspace titles", async () =>
 						return success(JSON.stringify({ result: { tab: { pane_count: 1 } } }));
 					}
 					if (args[0] === "workspace" && args[1] === "get") {
-						return success(JSON.stringify({ result: { workspace: { label: workspaceName, worktree: { checkout_path: "/repo/worktree", is_linked_worktree: true } } } }));
+						return success(JSON.stringify({ result: { workspace: { label: workspaceName, worktree: { checkout_path: checkoutPath, is_linked_worktree: true } } } }));
 					}
 					return success("{}");
 				},
@@ -307,6 +312,47 @@ test("manual rename updates generated and custom workspace titles", async () =>
 	});
 });
 
+test("semantic branch mutation honors the shared worktree lock", async () => {
+	await withAgentDir(async (dir) => {
+		process.env.HERDR_PANE_ID = "pane-1";
+		const checkoutPath = join(dir, "worktree");
+		await mkdir(checkoutPath);
+		let releaseLock!: () => void;
+		const heldLock = withWorktreeLock(checkoutPath, () => new Promise<void>((resolve) => {
+			releaseLock = resolve;
+		}));
+		await eventually(() => Boolean(releaseLock));
+		try {
+			const app = harness({
+				sessionName: "Saved title",
+				branch: [{ type: "message", message: { role: "user", content: "rename this" } }],
+				exec: async (args) => {
+					if (args.join("\0") === "branch\0--show-current") return success("worktree/generated\n");
+					if (args[0] === "for-each-ref") return success("");
+					if (args[0] === "pane" && args[1] === "get") {
+						return success(JSON.stringify({ result: { pane: { tab_id: "tab-1", workspace_id: "workspace-1" } } }));
+					}
+					if (args[0] === "tab" && args[1] === "get") {
+						return success(JSON.stringify({ result: { tab: { pane_count: 1 } } }));
+					}
+					if (args[0] === "workspace" && args[1] === "get") {
+						return success(JSON.stringify({ result: { workspace: { label: "worktree-rapid-meadow-04ae", worktree: { checkout_path: checkoutPath, is_linked_worktree: true } } } }));
+					}
+					return success("{}");
+				},
+			});
+			await app.handlers.get("session_start")?.({}, app.ctx);
+			await app.commands.get("rename")?.("", app.ctx);
+
+			assert.equal(app.execCalls.filter((args) => args[0] === "branch" || args[0] === "for-each-ref" || args[0] === "switch").length, 0);
+			assert.match(app.notifications.at(-1) ?? "", /lock/i);
+		} finally {
+			releaseLock();
+			await heldLock;
+		}
+	});
+});
+
 test("existing and manually changed titles remain untouched", async () => {
 	await withAgentDir(async () => {
 		process.env.HERDR_PANE_ID = "pane-1";
@@ -320,6 +366,40 @@ test("existing and manually changed titles remain untouched", async () => {
 		});
 		await changed.handlers.get("session_start")?.({}, changed.ctx);
 		assert.deepEqual(changed.execCalls, []);
+
+		const malformed = harness({
+			sessionName: "One two three four five",
+			branch: [{ type: "custom", customType: "pi-herdr-rename/title", data: { display: "One two three four five", branch: "fix/saved-title" } }],
+		});
+		await malformed.handlers.get("session_start")?.({}, malformed.ctx);
+		await new Promise((resolve) => setTimeout(resolve, 0));
+		assert.deepEqual(malformed.execCalls, []);
+	});
+});
+
+test("automatic and resumed Herdr failures warn", async () => {
+	await withAgentDir(async () => {
+		process.env.HERDR_PANE_ID = "pane-1";
+		const exec = async (args: string[]) =>
+			args[0] === "pane" && args[1] === "get"
+				? { stdout: "", stderr: "gone", code: 7, killed: false }
+				: success("{}");
+
+		const automatic = harness({ exec });
+		await automatic.handlers.get("session_start")?.({}, automatic.ctx);
+		automatic.handlers.get("input")?.({ source: "interactive", text: "prompt" }, automatic.ctx);
+		await automatic.handlers.get("before_agent_start")?.({ prompt: "prompt" }, automatic.ctx);
+		await eventually(() => automatic.notifications.length > 0);
+		assert.match(automatic.notifications.at(-1) ?? "", /herdr pane get failed/);
+
+		const resumed = harness({
+			sessionName: "Saved title",
+			branch: [{ type: "custom", customType: "pi-herdr-rename/title", data: { display: "Saved title", branch: "fix/saved-title" } }],
+			exec,
+		});
+		await resumed.handlers.get("session_start")?.({}, resumed.ctx);
+		await eventually(() => resumed.notifications.length > 0);
+		assert.match(resumed.notifications.at(-1) ?? "", /herdr pane get failed/);
 	});
 });