From 94949361600ca8561bddbb77e283516f5cc8694b Mon Sep 17 00:00:00 2001 From: StanPonomarev Date: Thu, 17 Sep 2026 21:32:26 +0200 Subject: [PATCH] fix wrong herdr response parse + add labels and branch names to delegations --- docs/multi-work.md | 17 +++- scripts/test-multi-work-host.ts | 14 +++- src/herdr.ts | 27 +++++-- src/multi-work.ts | 21 +++-- src/routing.ts | 10 ++- tests/composite.test.ts | 20 +++-- tests/fixtures/native-host-plugin.ts | 8 +- tests/herdr.test.ts | 113 +++++++++++++++++++++++++-- tests/multi-work.test.ts | 99 ++++++++++++++++++++++- 9 files changed, 291 insertions(+), 38 deletions(-) diff --git a/docs/multi-work.md b/docs/multi-work.md index 23dd1a0..0b7ca54 100644 --- a/docs/multi-work.md +++ b/docs/multi-work.md @@ -63,13 +63,24 @@ subagent({ agent: "coordinate", description: "Implement API change", prompt: "Goal, scope, constraints, and acceptance checks...", - worktree: { base: "feature/source" }, + worktree: { + base: "main", + branch: "fix/login-timeout", + label: "Fix login timeout", + }, background: true, }); ``` -`base` identifies a locally available branch or remote-tracking branch. The tool -resolves its commit and creates a new unique `multi-work/` branch with Herdr. +`base` identifies a locally available branch or remote-tracking branch. Optional +`branch` chooses the exact new Git branch and optional `label` sets the Herdr workspace +display label through `herdr worktree create --label`. Both must be nonempty after +trimming. Prefer concise, descriptive human-readable names. Existing branch validation +and collision checks apply, and the tool never overwrites a branch. When omitted, the +tool retains the previous behavior: a unique `multi-work/` branch and Herdr's +default workspace label. Naming fields are creation-only; resumes accept only `sessionID`. +The tool +resolves the base commit and creates the branch with Herdr. It does not fetch refs or copy uncommitted root changes. Herdr determines the checkout path; the tool parses and verifies the returned checkout/workspace. Creation explicitly preserves focus with `--no-focus`. diff --git a/scripts/test-multi-work-host.ts b/scripts/test-multi-work-host.ts index 3f355cc..93f9564 100644 --- a/scripts/test-multi-work-host.ts +++ b/scripts/test-multi-work-host.ts @@ -15,6 +15,7 @@ const root = join(temp, "root"), destination = join(temp, "child"), bin = join(temp, "bin"), config = join(temp, "config"); +const herdrCreation = join(temp, "herdr-creation.txt"); for (const path of [root, bin, config, join(temp, "plugin")]) await mkdir(path); const signal = AbortSignal.timeout(120_000); if ((await command([binary, "--version"], temp, signal)) !== "opencode v2.0.4") @@ -41,17 +42,20 @@ const inventory = (path: string, id: string) => source: { repo_key: join(root, ".git"), repo_root: root, - source_checkout_path: path, - source_workspace_id: id, + source_checkout_path: root, + source_workspace_id: "w1", }, worktrees: [ - { path, is_linked_worktree: path !== root, is_bare: false, open_workspace_id: id }, + { path: root, is_linked_worktree: false, is_bare: false, open_workspace_id: "w1" }, + ...(path === root + ? [] + : [{ path, is_linked_worktree: true, is_bare: false, open_workspace_id: id }]), ], }, }); await writeFile( join(bin, "herdr"), - `#!/bin/sh\ncase "$1 $2" in\n"worktree create") git -C '${root}' worktree add -b "$6" '${destination}' "$8" >&2 || exit 1; printf '{"result":{"workspace":{"workspace_id":"w2"},"worktree":{"path":"${destination}","branch":"%s","is_linked_worktree":true}}}' "$6"; exit ;;\nesac\ncase "$1 $2 $4" in\n"workspace list ") echo '{"result":{"workspaces":[{"workspace_id":"w1"}]}}' ;;\n"worktree list w1") echo '${inventory(root, "w1")}' ;;\n"worktree list w2") echo '${inventory(destination, "w2")}' ;;\n*) exit 9 ;;\nesac\n`, + `#!/bin/sh\ncase "$1 $2" in\n"worktree create") [ "$9" = "--label" ] && [ "\${10}" = "Fix native host" ] || exit 8; printf '%s\\n%s\\n' "$6" "\${10}" > '${herdrCreation}'; git -C '${root}' worktree add -b "$6" '${destination}' "$8" >&2 || exit 1; printf '{"result":{"workspace":{"workspace_id":"w2"},"worktree":{"path":"${destination}","branch":"%s","is_linked_worktree":true}}}' "$6"; exit ;;\nesac\ncase "$1 $2 $4" in\n"workspace list ") echo '{"result":{"workspaces":[{"workspace_id":"w1"}]}}' ;;\n"worktree list w1") echo '${inventory(root, "w1")}' ;;\n"worktree list w2") echo '${inventory(destination, "w2")}' ;;\n*) exit 9 ;;\nesac\n`, ); await chmod(join(bin, "herdr"), 0o755); const built = await Bun.build({ @@ -190,6 +194,8 @@ try { .parse(JSON.parse(z.string().parse(result.output))); if (!output.content.includes("FIXTURE_COMPLETE")) throw new Error("Missing native child result"); + if ((await readFile(herdrCreation, "utf8")) !== "fix/native-host\nFix native host\n") + throw new Error("Custom Herdr branch/label mismatch"); const child = await client.session.get({ sessionID: output.metadata.sessionID }); if (child.parentID !== parent.id || child.location.directory !== destination) throw new Error("Child ancestry/placement mismatch"); diff --git a/src/herdr.ts b/src/herdr.ts index 60e2c5e..2ed22b0 100644 --- a/src/herdr.ts +++ b/src/herdr.ts @@ -109,7 +109,13 @@ export class Herdr { return matches.length === 1 ? matches[0] : undefined; } - async create(root: HerdrRoot, base: string, branch: string, signal: AbortSignal) { + async create( + root: HerdrRoot, + base: string, + branch: string, + signal: AbortSignal, + label?: string, + ) { const current = await this.root(root.directory, signal); if (!current || current.workspaceID !== root.workspaceID || current.repo !== root.repo) throw new Error("Herdr root workspace is no longer eligible"); @@ -150,6 +156,7 @@ export class Herdr { branch, "--base", commit, + ...(label === undefined ? [] : ["--label", label]), "--no-focus", ], root.directory, @@ -185,7 +192,7 @@ export class Herdr { throw new Error( "Created checkout does not match the requested repository and base", ); - await this.verify(directory, workspaceID, root.repo, branch, signal); + await this.verify(directory, workspaceID, root, branch, signal); } catch (error) { throw new Error( `Retained Herdr workspace ${workspaceID}, checkout ${directory}, branch ${branch}`, @@ -198,7 +205,7 @@ export class Herdr { async verify( directory: string, workspaceID: string, - repo: string, + root: HerdrRoot, branch: string, signal: AbortSignal, ) { @@ -214,12 +221,20 @@ export class Herdr { const checkout = value.result.worktrees.find( (item) => item.open_workspace_id === workspaceID && item.is_linked_worktree, ); + const sourceCheckout = value.result.worktrees.find( + (item) => + item.open_workspace_id === value.result.source.source_workspace_id && + !item.is_linked_worktree && + !item.is_bare, + ); if ( !checkout || - value.result.source.source_workspace_id !== workspaceID || - (await realpath(value.result.source.source_checkout_path)) !== directory || + !sourceCheckout || + value.result.source.source_workspace_id !== root.workspaceID || + (await realpath(value.result.source.source_checkout_path)) !== root.directory || + (await realpath(sourceCheckout.path)) !== root.directory || (await realpath(checkout.path)) !== directory || - (await realpath(value.result.source.repo_key)) !== repo + (await realpath(value.result.source.repo_key)) !== root.repo ) throw new Error("Bound Herdr checkout is no longer available"); if ( diff --git a/src/multi-work.ts b/src/multi-work.ts index ab7a6a0..8445bf0 100644 --- a/src/multi-work.ts +++ b/src/multi-work.ts @@ -28,7 +28,11 @@ const inputSchema = z sessionID: z.string().optional(), background: z.boolean().optional(), worktree: z - .object({ base: z.string().trim().min(1) }) + .object({ + base: z.string().trim().min(1), + branch: z.string().trim().min(1).optional(), + label: z.string().trim().min(1).optional(), + }) .strict() .optional(), }) @@ -180,7 +184,7 @@ export const installMultiWork = ( agent: { type: "string", enum: ["fast-lo", "coordinate"] }, }; tool.description = - "Delegate to fast-lo locally, or coordinate in a new Herdr worktree using worktree: {base}. Continue an owned coordinate child with sessionID and no worktree argument. Native background and results apply."; + "Delegate to fast-lo locally, or coordinate in a new Herdr worktree using worktree: {base, branch?, label?}. Prefer descriptive human-readable branch and label values, for example {base:'main',branch:'fix/login-timeout',label:'Fix login timeout'}. Continue an owned coordinate child with sessionID and no worktree argument; naming fields are create-only. Native background and results apply."; } }).pipe(Effect.orDie), ); @@ -275,7 +279,7 @@ export const installMultiWork = ( herdr.verify( binding.directory, binding.workspaceID, - binding.root.repo, + binding.root, binding.branch, signal, ), @@ -319,7 +323,8 @@ export const installMultiWork = ( throw new Error( "Native executor returned an unexpected child", ); - const branch = `multi-work/${randomUUID()}`; + const branch = + worktree.branch ?? `multi-work/${randomUUID()}`; yield* context.storage.set(callKey, { child: id, branch, @@ -327,7 +332,13 @@ export const installMultiWork = ( }); yield* call.progress({ ...metadata, branch }); const destination = yield* Effect.tryPromise((signal) => - herdr.create(root, worktree.base, branch, signal), + herdr.create( + root, + worktree.base, + branch, + signal, + worktree.label, + ), ); const binding = { owner: call.sessionID, diff --git a/src/routing.ts b/src/routing.ts index 008ba3f..7ce4c7c 100644 --- a/src/routing.ts +++ b/src/routing.ts @@ -16,12 +16,14 @@ export const descriptions = { export const multiWorkGuide = `You are multi-work, a primary orchestrator in a verified Herdr root checkout. Read, plan, track todos and delegate; never implement or execute shell commands yourself. Use native subagent with agent fast-lo for focused local retrieval. Use agent coordinate -for work in a new Herdr worktree, supplying worktree: {base: "source-branch"}, description, -and a complete prompt with scope, constraints and acceptance checks. The deterministic tool -creates a unique branch from that locally available source branch, moves the coordinate child +for work in a new Herdr worktree, supplying descriptive human-readable names such as +worktree: {base: "main", branch: "fix/login-timeout", label: "Fix login timeout"}, plus +description and a complete prompt with scope, constraints and acceptance checks. The tool +uses the requested branch and Herdr workspace label; when omitted, it creates the prior unique +multi-work/ branch and lets Herdr choose its default label. It moves the coordinate child before its task begins, and preserves your root session. Dirty root changes are not copied. Resume an owned coordinate child using its sessionID and no worktree argument. Never switch -its agent or location. Only fast-lo and coordinate are permitted. Workers are leaves. +its agent or location; naming fields are create-only. Only fast-lo and coordinate are permitted. Workers are leaves. Parallelize independent work with disjoint ownership; serialize dependencies. Native background delegation returns immediately and notifies you when complete; do not poll or duplicate work. Reconcile results and report checkout, branch and child IDs. Retained resources on failure diff --git a/tests/composite.test.ts b/tests/composite.test.ts index cd47a08..1865407 100644 --- a/tests/composite.test.ts +++ b/tests/composite.test.ts @@ -59,16 +59,26 @@ const scenario = async () => { source: { repo_key: join(root, ".git"), repo_root: root, - source_checkout_path: path, - source_workspace_id: id, + source_checkout_path: root, + source_workspace_id: "w1", }, worktrees: [ { - path, - is_linked_worktree: path !== root, + path: root, + is_linked_worktree: false, is_bare: false, - open_workspace_id: id, + open_workspace_id: "w1", }, + ...(path === root + ? [] + : [ + { + path, + is_linked_worktree: true, + is_bare: false, + open_workspace_id: id, + }, + ]), ], }, }); diff --git a/tests/fixtures/native-host-plugin.ts b/tests/fixtures/native-host-plugin.ts index fd1e342..efad3e8 100644 --- a/tests/fixtures/native-host-plugin.ts +++ b/tests/fixtures/native-host-plugin.ts @@ -76,7 +76,13 @@ export default Plugin.define({ background: input.background, ...(input.child ? { sessionID: input.child } - : { worktree: { base: "main" } }), + : { + worktree: { + base: "main", + branch: "fix/native-host", + label: "Fix native host", + }, + }), }, { sessionID: Session.ID.make(input.parent), diff --git a/tests/herdr.test.ts b/tests/herdr.test.ts index e397004..f841fb1 100644 --- a/tests/herdr.test.ts +++ b/tests/herdr.test.ts @@ -38,6 +38,12 @@ async function fixture() { let linked = false; let duplicate = false; let branch = ""; + let label: string | undefined; + let sourceWorkspaceIDOverride: string | undefined; + let sourcePathOverride: string | undefined; + let childWorkspaceIDOverride: string | undefined; + let childPathOverride: string | undefined; + let repoOverride: string | undefined; const run = async (args: string[], cwd: string, signal: AbortSignal) => { calls.push(args); if (args[0] !== "herdr") return command(args, cwd, signal); @@ -52,21 +58,33 @@ async function fixture() { if (args[2] === "list") { const id = args[4]!; const path = id === "w2" ? destination : root; + const sourceWorkspaceID = sourceWorkspaceIDOverride ?? (id === "w2" ? "w1" : id); + const sourcePath = sourcePathOverride ?? root; return JSON.stringify({ result: { source: { - repo_key: join(root, ".git"), + repo_key: repoOverride ?? join(root, ".git"), repo_root: root, - source_checkout_path: path, - source_workspace_id: id, + source_checkout_path: sourcePath, + source_workspace_id: sourceWorkspaceID, }, worktrees: [ { - path, - is_linked_worktree: id === "w2" || linked, + path: sourcePath, + is_linked_worktree: linked, is_bare: false, - open_workspace_id: id, + open_workspace_id: sourceWorkspaceID, }, + ...(id === "w2" + ? [ + { + path: childPathOverride ?? path, + is_linked_worktree: true, + is_bare: false, + open_workspace_id: childWorkspaceIDOverride ?? id, + }, + ] + : []), ], }, }); @@ -75,6 +93,8 @@ async function fixture() { expect(args.slice(3, 5)).toEqual(["--cwd", root]); expect(args.at(-1)).toBe("--no-focus"); branch = args[args.indexOf("--branch") + 1]!; + const labelIndex = args.indexOf("--label"); + label = labelIndex === -1 ? undefined : args[labelIndex + 1]; const base = args[args.indexOf("--base") + 1]!; // Real Git worktree behavior, but only in an isolated temporary repository. await command(["git", "worktree", "add", "-b", branch, destination, base], root, signal); @@ -90,6 +110,9 @@ async function fixture() { destination, signal, calls, + get label() { + return label; + }, herdr: new Herdr(run), close() { open = false; @@ -100,10 +123,23 @@ async function fixture() { duplicate() { duplicate = true; }, + changeInventory(change: { + sourceWorkspaceID?: string | undefined; + sourcePath?: string | undefined; + childWorkspaceID?: string | undefined; + childPath?: string | undefined; + repo?: string | undefined; + }) { + sourceWorkspaceIDOverride = change.sourceWorkspaceID; + sourcePathOverride = change.sourcePath; + childWorkspaceIDOverride = change.childWorkspaceID; + childPathOverride = change.childPath; + repoOverride = change.repo; + }, }; } -test("live inventory recognizes an unannotated root and creates a unique branch from another local branch", async () => { +test("create verifies a linked workspace whose inventory source remains the parent workspace", async () => { const f = await fixture(); const root = await f.herdr.root(f.root, f.signal); expect(root?.workspaceID).toBe("w1"); @@ -114,12 +150,73 @@ test("live inventory recognizes an unannotated root and creates a unique branch await f.herdr.verify( destination.directory, destination.workspaceID, - root!.repo, + root!, destination.branch, f.signal, ); }); +test("create passes an optional workspace label to Herdr", async () => { + const f = await fixture(); + const root = (await f.herdr.root(f.root, f.signal))!; + const destination = await f.herdr.create( + root, + "main", + "fix/login-timeout", + f.signal, + "Fix login timeout", + ); + expect(destination.branch).toBe("fix/login-timeout"); + expect(f.label).toBe("Fix login timeout"); + const creation = f.calls.find((args) => args[0] === "herdr" && args[2] === "create")!; + expect(creation.slice(creation.indexOf("--label"), creation.indexOf("--label") + 2)).toEqual([ + "--label", + "Fix login timeout", + ]); +}); + +test("create rejects an existing destination branch before invoking Herdr", async () => { + const f = await fixture(); + const root = (await f.herdr.root(f.root, f.signal))!; + await expect( + f.herdr.create(root, "main", "invalid branch", f.signal, "Invalid"), + ).rejects.toThrow(); + await expect(f.herdr.create(root, "main", "other", f.signal, "Other")).rejects.toThrow( + "Destination branch already exists: other", + ); + expect(f.calls.filter((args) => args[0] === "herdr" && args[2] === "create")).toHaveLength(0); +}); + +test("verify rejects changed parent, child and repository identities", async () => { + for (const change of [ + { sourceWorkspaceID: "w9" }, + { sourcePath: "child" }, + { childWorkspaceID: "w9" }, + { childPath: "root" }, + { repo: "child" }, + ]) { + const f = await fixture(); + const root = (await f.herdr.root(f.root, f.signal))!; + const destination = await f.herdr.create(root, "main", "multi-work/fixture", f.signal); + f.changeInventory({ + sourceWorkspaceID: change.sourceWorkspaceID, + sourcePath: change.sourcePath === "child" ? f.destination : undefined, + childWorkspaceID: change.childWorkspaceID, + childPath: change.childPath === "root" ? f.root : undefined, + repo: change.repo === "child" ? f.destination : undefined, + }); + await expect( + f.herdr.verify( + destination.directory, + destination.workspaceID, + root, + destination.branch, + f.signal, + ), + ).rejects.toThrow("Bound Herdr checkout is no longer available"); + } +}); + test("closed, linked and ambiguous workspaces fail root gating", async () => { for (const mode of ["close", "link", "duplicate"] as const) { const f = await fixture(); diff --git a/tests/multi-work.test.ts b/tests/multi-work.test.ts index a9941aa..a0043ec 100644 --- a/tests/multi-work.test.ts +++ b/tests/multi-work.test.ts @@ -18,14 +18,26 @@ class FixtureHerdr extends Herdr { verified = 0; delayCreation = false; creationAborted = false; + createdBase: string | undefined; + createdBranch: string | undefined; + createdLabel: string | undefined; creationStarted = Promise.withResolvers(); override async root(directory: string) { return this.eligible && directory === "/root" ? { directory, workspaceID: "w1", repo: "/root/.git" } : undefined; } - override async create(_root: HerdrRoot, _base: string, branch: string, signal: AbortSignal) { + override async create( + _root: HerdrRoot, + base: string, + branch: string, + signal: AbortSignal, + label?: string, + ) { this.creations++; + this.createdBase = base; + this.createdBranch = branch; + this.createdLabel = label; if (!this.eligible) throw new Error("root closed"); if (this.delayCreation) { this.creationStarted.resolve(); @@ -295,7 +307,13 @@ const request = z.object({ agent: z.string(), description: z.string(), prompt: z.string(), - worktree: z.object({ base: z.string() }).optional(), + worktree: z + .object({ + base: z.string(), + branch: z.string().optional(), + label: z.string().optional(), + }) + .optional(), sessionID: z.string().optional(), background: z.boolean().optional(), }); @@ -348,6 +366,83 @@ test("local fast-lo uses native delegation without a worktree", async () => { ); }); +test("custom branch and workspace label pass through delegation", async () => { + const f = fixture(); + await Effect.runPromise( + Effect.scoped( + Effect.gen(function* () { + yield* f.install; + const result = yield* f.execute({ + agent: "coordinate", + description: "Fix login timeout", + prompt: "work", + worktree: { + base: "main", + branch: "fix/login-timeout", + label: "Fix login timeout", + }, + }); + expect(f.herdr.createdBase).toBe("main"); + expect(f.herdr.createdBranch).toBe("fix/login-timeout"); + expect(f.herdr.createdLabel).toBe("Fix login timeout"); + expect(result.metadata?.branch).toBe("fix/login-timeout"); + }), + ), + ); +}); + +test("omitted naming retains generated branch and no explicit label", async () => { + const f = fixture(); + await Effect.runPromise( + Effect.scoped( + Effect.gen(function* () { + yield* f.install; + yield* f.invoke(); + expect(f.herdr.createdBranch).toMatch(/^multi-work\/[0-9a-f-]{36}$/); + expect(f.herdr.createdLabel).toBeUndefined(); + }), + ), + ); +}); + +test("rejects empty naming fields and naming on resume", async () => { + const f = fixture(); + await Effect.runPromise( + Effect.scoped( + Effect.gen(function* () { + yield* f.install; + for (const worktree of [ + { base: "main", branch: " " }, + { base: "main", label: " " }, + ]) + expect( + (yield* Effect.exit( + f.execute({ + agent: "coordinate", + description: "invalid", + prompt: "work", + worktree, + }), + ))._tag, + ).toBe("Failure"); + yield* f.invoke(); + expect( + (yield* Effect.exit( + f.execute({ + agent: "coordinate", + description: "resume", + prompt: "work", + sessionID: "ses_child1", + worktree: { base: "main", branch: "fix/other" }, + }), + ))._tag, + ).toBe("Failure"); + expect(f.herdr.creations).toBe(1); + }), + ), + ); +}); + test("root loss, session permission denial and missing destination fail before prompting", async () => { for (const failure of ["root", "permission", "ask", "destination", "project"] as const) { const f = fixture();