Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions apps/server/src/personas/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,33 @@ export async function loadPersonas(
return personas;
}

export function mergePersonasWithWorktreePrecedence<
T extends { slug: string },
>(input: { worktreePersonas: T[]; repoPersonas: T[] }): T[] {
const worktreeSlugs = new Set(input.worktreePersonas.map((p) => p.slug));
return [
...input.worktreePersonas,
...input.repoPersonas.filter((p) => !worktreeSlugs.has(p.slug)),
];
}

export async function loadPersonasFromRoots(input: {
worktreeRoot?: string | null;
repoRoot?: string | null;
}): Promise<PersonaDefinition[]> {
const worktreeRoot = input.worktreeRoot ?? null;
const repoRoot = input.repoRoot ?? null;

const worktreePersonas = worktreeRoot ? await loadPersonas(worktreeRoot) : [];
const repoPersonas =
repoRoot && repoRoot !== worktreeRoot ? await loadPersonas(repoRoot) : [];

return mergePersonasWithWorktreePrecedence({
worktreePersonas,
repoPersonas,
});
}

export async function loadPersonaBySlug(
repoRoot: string,
slug: string
Expand Down
27 changes: 22 additions & 5 deletions apps/server/src/routes/persona-reviews.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
CLI_AGENT_TYPES,
getEnabledAgentTypes,
} from "../agent-type-settings.js";
import { loadPersonas } from "../personas/loader.js";
import { loadPersonasFromRoots } from "../personas/loader.js";
import {
resolveRepoRoot,
resolveWorktreeRoot,
Expand Down Expand Up @@ -39,6 +39,24 @@ type PersonaReviewRouteDeps = {

const PERSONA_SLUG_PATTERN = /^[a-zA-Z0-9_-]+$/;

async function resolveOptionalWorktreeRoot(
cwd: string
): Promise<string | null> {
try {
return await resolveWorktreeRoot(cwd);
} catch {
return null;
}
}

async function resolveOptionalRepoRoot(cwd: string): Promise<string | null> {
try {
return await resolveRepoRoot(cwd);
} catch {
return null;
}
}

export async function registerPersonaReviewRoutes(
app: FastifyInstance,
deps: PersonaReviewRouteDeps
Expand All @@ -51,10 +69,9 @@ export async function registerPersonaReviewRoutes(
.send({ error: "cwd query parameter is required." });
}
try {
let personas = await loadPersonas(await resolveWorktreeRoot(query.cwd));
if (personas.length === 0) {
personas = await loadPersonas(await resolveRepoRoot(query.cwd));
}
const worktreeRoot = await resolveOptionalWorktreeRoot(query.cwd);
const repoRoot = await resolveOptionalRepoRoot(query.cwd);
const personas = await loadPersonasFromRoots({ worktreeRoot, repoRoot });
return { personas };
} catch {
return { personas: [] };
Expand Down
38 changes: 28 additions & 10 deletions apps/server/src/shared/mcp/persona-interaction-tools.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
import * as z from "zod/v4";

import { mergePersonasWithWorktreePrecedence } from "../../personas/loader.js";
import type { McpRequestContext } from "./server.js";
import { toToolError } from "./tool-error.js";

Expand All @@ -24,6 +25,27 @@ export type PersonaInteractionCallbacks = {
submitResolution?: McpRequestContext["submitResolution"];
};

type PersonaSummary = { slug: string; name: string; description: string };

export async function resolvePersonaList(
listPersonas: (root: string) => Promise<PersonaSummary[]>,
worktreeRoot?: string | null,
repoRoot?: string | null
): Promise<PersonaSummary[]> {
const worktreePersonas = worktreeRoot
? await listPersonas(worktreeRoot).catch(() => [])
: [];
const repoPersonas =
repoRoot && repoRoot !== worktreeRoot
? await listPersonas(repoRoot).catch(() => [])
: [];

return mergePersonasWithWorktreePrecedence({
worktreePersonas,
repoPersonas,
});
}

export function registerPersonaInteractionTools(
server: McpServer,
allowed: Set<string>,
Expand All @@ -33,8 +55,8 @@ export function registerPersonaInteractionTools(

// ── list_personas ────────────────────────────────────────────────
if (allowed.has("list_personas") && callbacks.listPersonas) {
const personaRoot = callbacks.worktreeRoot ?? callbacks.repoRoot;
const listPersonas = callbacks.listPersonas;
const { worktreeRoot, repoRoot } = callbacks;

server.registerTool(
"list_personas",
Expand All @@ -44,16 +66,12 @@ export function registerPersonaInteractionTools(
inputSchema: {},
},
async () => {
if (!personaRoot) {
return {
content: [
{ type: "text", text: JSON.stringify({ personas: [] }, null, 2) },
],
structuredContent: { personas: [] },
};
}
try {
const personas = await listPersonas(personaRoot);
const personas = await resolvePersonaList(
listPersonas,
worktreeRoot,
repoRoot
);
return {
content: [
{ type: "text", text: JSON.stringify({ personas }, null, 2) },
Expand Down
92 changes: 92 additions & 0 deletions apps/server/test/persona-list-merge.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import { describe, expect, it, vi } from "vitest";

import { resolvePersonaList } from "../src/shared/mcp/persona-interaction-tools.js";

const persona = (slug: string, name = slug) => ({
slug,
name,
description: "",
});

describe("resolvePersonaList", () => {
it("includes personas from both the worktree and repo root", async () => {
const listPersonas = vi.fn(async (root: string) =>
root === "/wt" ? [persona("local")] : [persona("repo")]
);

const result = await resolvePersonaList(listPersonas, "/wt", "/repo");

expect(result.map((p) => p.slug)).toEqual(["local", "repo"]);
expect(listPersonas).toHaveBeenCalledWith("/wt");
expect(listPersonas).toHaveBeenCalledWith("/repo");
});

it("uses worktree personas as the override for duplicate slugs", async () => {
const listPersonas = vi.fn(async (root: string) =>
root === "/wt"
? [persona("review", "Worktree Review")]
: [persona("review", "Repo Review"), persona("release")]
);

const result = await resolvePersonaList(listPersonas, "/wt", "/repo");

expect(result).toEqual([
persona("review", "Worktree Review"),
persona("release"),
]);
});

it("uses the repo root directly when there is no worktree root", async () => {
const listPersonas = vi.fn(async (root: string) =>
root === "/repo" ? [persona("repo")] : []
);

const result = await resolvePersonaList(listPersonas, null, "/repo");

expect(result).toEqual([persona("repo")]);
expect(listPersonas).toHaveBeenCalledTimes(1);
expect(listPersonas).toHaveBeenCalledWith("/repo");
});

it("does not re-query when the worktree root and repo root are identical", async () => {
const listPersonas = vi.fn(async () => [persona("repo")]);

const result = await resolvePersonaList(listPersonas, "/repo", "/repo");

expect(result).toEqual([persona("repo")]);
expect(listPersonas).toHaveBeenCalledTimes(1);
});

it("keeps worktree personas when the repo root listing fails", async () => {
const listPersonas = vi.fn(async (root: string) => {
if (root === "/repo") throw new Error("repo unavailable");
return [persona("local")];
});

const result = await resolvePersonaList(listPersonas, "/wt", "/repo");

expect(result).toEqual([persona("local")]);
expect(listPersonas).toHaveBeenCalledTimes(2);
});

it("keeps repo personas when the worktree root listing fails", async () => {
const listPersonas = vi.fn(async (root: string) => {
if (root === "/wt") throw new Error("worktree unavailable");
return [persona("repo")];
});

const result = await resolvePersonaList(listPersonas, "/wt", "/repo");

expect(result).toEqual([persona("repo")]);
expect(listPersonas).toHaveBeenCalledTimes(2);
});

it("returns an empty list when neither root is available", async () => {
const listPersonas = vi.fn(async () => []);

const result = await resolvePersonaList(listPersonas, null, null);

expect(result).toEqual([]);
expect(listPersonas).not.toHaveBeenCalled();
});
});
94 changes: 94 additions & 0 deletions apps/server/test/persona-loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {
INLINE_DIFF_THRESHOLD_BYTES,
loadPersonaBySlug,
loadPersonas,
loadPersonasFromRoots,
mergePersonasWithWorktreePrecedence,
parseFrontmatter,
} from "../src/personas/loader.js";
import type { PersonaDefinition } from "../src/personas/loader.js";
Expand Down Expand Up @@ -444,6 +446,98 @@ feedbackFormat: checklist
});
});

describe("mergePersonasWithWorktreePrecedence", () => {
const persona = (slug: string, name = slug): PersonaDefinition => ({
slug,
name,
description: "",
feedbackFormat: "findings",
body: "",
});

it("includes repo personas that are absent from the worktree", () => {
const merged = mergePersonasWithWorktreePrecedence({
worktreePersonas: [persona("worktree-only")],
repoPersonas: [persona("repo-only")],
});

expect(merged.map((p) => p.slug)).toEqual(["worktree-only", "repo-only"]);
});

it("uses the worktree persona when both roots define the same slug", () => {
const merged = mergePersonasWithWorktreePrecedence({
worktreePersonas: [persona("review", "Worktree Review")],
repoPersonas: [persona("review", "Repo Review"), persona("release")],
});

expect(merged).toEqual([
persona("review", "Worktree Review"),
persona("release"),
]);
});
});

describe("loadPersonasFromRoots", () => {
const tmpBase = `/tmp/dispatch-persona-roots-test-${process.pid}`;
const worktreeRoot = path.join(tmpBase, "worktree");
const repoRoot = path.join(tmpBase, "repo");

beforeAll(() => {
mkdirSync(path.join(worktreeRoot, ".dispatch", "personas"), {
recursive: true,
});
mkdirSync(path.join(repoRoot, ".dispatch", "personas"), {
recursive: true,
});
writeFileSync(
path.join(worktreeRoot, ".dispatch", "personas", "security.md"),
`---
name: Worktree Security
---

# Worktree security`
);
writeFileSync(
path.join(repoRoot, ".dispatch", "personas", "security.md"),
`---
name: Repo Security
---

# Repo security`
);
writeFileSync(
path.join(repoRoot, ".dispatch", "personas", "release.md"),
`---
name: Release
---

# Release`
);
});

afterAll(() => {
rmSync(tmpBase, { recursive: true, force: true });
});

it("loads both roots and lets the worktree override duplicate slugs", async () => {
const personas = await loadPersonasFromRoots({ worktreeRoot, repoRoot });

expect(personas.map((p) => [p.slug, p.name])).toEqual([
["security", "Worktree Security"],
["release", "Release"],
]);
});

it("does not read the repo twice when both roots are the same", async () => {
const personas = await loadPersonasFromRoots({
worktreeRoot: repoRoot,
repoRoot,
});

expect(personas.map((p) => p.slug)).toEqual(["release", "security"]);
});
});

describe("loadPersonaBySlug", () => {
const tmpRoot = `/tmp/dispatch-persona-slug-test-${process.pid}`;
const personasDir = path.join(tmpRoot, ".dispatch", "personas");
Expand Down
Loading