fix(pi-herdr-rename): rename workspace on command (#175)

Author: HenryQW

package-lock.json

@@ -13,7 +13,7 @@ Git-safe branch combining task type with display-title words: `Update task logic
 _Avoid_: display title, raw generated branch, arbitrary Git mutation
 
 **Generated worktree label**:
-Herdr linked-worktree label matching its `worktree-<adjective>-<noun>-<hex>` default. It is replaced with current display title; custom workspace labels remain unchanged.
+Herdr linked-worktree label matching its `worktree-<adjective>-<noun>-<hex>` default. It is replaced automatically with current display title; an explicit rename may also replace a custom workspace label.
 _Avoid_: semantic branch, custom workspace name
 
 **Sole-pane tab**:

packages/pi-herdr-rename/CONTEXT.md

@@ -34,6 +34,6 @@ Shared [`pi-task-models` config](../pi-task-models#config) at `~/.pi/agent/confi
 
 Display titles are natural task phrases, preferably three or four words and always at most four words and 20 characters. Model classification stays internal: `refactor: update task logic` displays as `Update task logic` and maps to Git branch `refactor/update-task-logic`.
 
-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 display title; custom workspace names stay. Enclosing Herdr tab updates only when this pane is tab's only pane. Outside Herdr, only Pi session name changes.
+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/README.md

@@ -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;
 }
@@ -216,6 +223,7 @@ export default function herdrRenameExtension(pi: ExtensionAPI): void {
 		displayTitle: string,
 		branchCandidate: string,
 		previousDisplayTitle: string | undefined,
+		forceWorkspaceRename: boolean,
 		request: number,
 		controller: AbortController,
 	): Promise<void> => {
@@ -247,6 +255,16 @@ export default function herdrRenameExtension(pi: ExtensionAPI): void {
 		const workspaceName = workspace?.label;
 		if (typeof workspaceName !== "string") throw new Error("Herdr workspace response omitted label.");
 		const worktree = workspace?.worktree;
+		if (
+			workspaceName !== displayTitle &&
+			(forceWorkspaceRename ||
+				(worktree?.is_linked_worktree === true &&
+					(HERDR_DEFAULT_WORKTREE_NAME.test(workspaceName) || workspaceName === previousDisplayTitle))) &&
+			isCurrent(request, controller)
+		) {
+			await herdr.run(["workspace", "rename", workspaceId, displayTitle], { signal: controller.signal });
+		}
+
 		const checkoutPath = worktree?.checkout_path;
 		if (worktree?.is_linked_worktree !== true || typeof checkoutPath !== "string" || !checkoutPath) return;
 
@@ -258,22 +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]);
-		}
-		if (
-			workspaceName !== displayTitle &&
-			(HERDR_DEFAULT_WORKTREE_NAME.test(workspaceName) || workspaceName === previousDisplayTitle) &&
-			isCurrent(request, controller)
-		) {
-			await herdr.run(["workspace", "rename", workspaceId, displayTitle], { signal: controller.signal });
-		}
+			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 = () => {
@@ -300,10 +314,10 @@ export default function herdrRenameExtension(pi: ExtensionAPI): void {
 			const previousDisplayTitle = saved && pi.getSessionName() === saved.display ? saved.display : undefined;
 			pi.setSessionName(title.display);
 			pi.appendEntry(TITLE_STATE_TYPE, title);
-			await applyHerdr(title.display, title.branch, previousDisplayTitle, request, controller);
+			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;
@@ -341,8 +355,12 @@ export default function herdrRenameExtension(pi: ExtensionAPI): void {
 		if (!title || title !== saved?.display) return;
 
 		const { request, controller } = begin();
-		void applyHerdr(title, saved.branch, saved.display, request, controller)
-			.catch(() => undefined)
+		void applyHerdr(title, saved.branch, saved.display, false, request, controller)
+			.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/extensions/rename.ts

@@ -1,6 +1,6 @@
 {
   "name": "@henryqw/pi-herdr-rename",
-  "version": "2.1.7",
+  "version": "2.1.8",
   "description": "Generate short Pi display titles and rename the current Herdr location.",
   "keywords": [
     "pi-package",

packages/pi-herdr-rename/package.json

@@ -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"));
@@ -270,38 +273,83 @@ test("saved display titles keep semantic branches, replace generated branches, a
 	});
 });
 
-test("manual rename updates its previous generated workspace title", async () => {
-	await withAgentDir(async () => {
+test("manual rename updates generated and custom workspace titles", async () => {
+	await withAgentDir(async (dir) => {
 		process.env.HERDR_PANE_ID = "pane-1";
-		const app = harness({
-			sessionName: "Saved title",
-			branch: [
-				{ type: "custom", customType: "pi-herdr-rename/title", data: { display: "Saved title", branch: "fix/saved-title" } },
-				{ type: "message", message: { role: "user", content: "update task logic" } },
-			],
-			complete: async () => response("refactor: update task logic"),
-			exec: async (args) => {
-				if (args.join("\0") === "branch\0--show-current") return success("fix/saved-title\n");
-				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: "Saved title", worktree: { checkout_path: "/repo/worktree", is_linked_worktree: true } } } }));
-				}
-				return success("{}");
-			},
-		});
-		await app.handlers.get("session_start")?.({}, app.ctx);
-		await eventually(() => app.execCalls.some((args) => args[0] === "workspace" && args[1] === "get"));
-		await app.commands.get("rename")?.("", app.ctx);
+		const checkoutPath = join(dir, "worktree");
+		await mkdir(checkoutPath);
+		for (const workspaceName of ["Saved title", "Custom workspace"]) {
+			const app = harness({
+				sessionName: "Saved title",
+				branch: [
+					{ type: "custom", customType: "pi-herdr-rename/title", data: { display: "Saved title", branch: "fix/saved-title" } },
+					{ type: "message", message: { role: "user", content: "update task logic" } },
+				],
+				complete: async () => response("refactor: update task logic"),
+				exec: async (args) => {
+					if (args.join("\0") === "branch\0--show-current") return success("fix/saved-title\n");
+					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: workspaceName, worktree: { checkout_path: checkoutPath, is_linked_worktree: true } } } }));
+					}
+					return success("{}");
+				},
+			});
+			await app.handlers.get("session_start")?.({}, app.ctx);
+			await eventually(() => app.execCalls.some((args) => args[0] === "workspace" && args[1] === "get"));
+			await app.commands.get("rename")?.("", app.ctx);
 
-		assert.deepEqual(
-			app.execCalls.filter((args) => args[0] === "workspace" && args[1] === "rename"),
-			[["workspace", "rename", "workspace-1", "Update task logic"]],
-		);
+			assert.deepEqual(
+				app.execCalls.filter((args) => args[0] === "workspace" && args[1] === "rename"),
+				[["workspace", "rename", "workspace-1", "Update task logic"]],
+			);
+		}
+	});
+});
+
+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;
+		}
 	});
 });
 
@@ -318,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/);
 	});
 });