Skip to content
Open
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
84 changes: 83 additions & 1 deletion packages/persona-registry/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,14 @@ import { join } from 'node:path';
import test from 'node:test';

import type { PersonaSpec } from '@agentworkforce/persona-kit';
import { PersonaResolutionError, resolvePersonaReference, __mergeOverrideForTests } from './index.js';
import {
PersonaResolutionError,
buildPersonaSourceDirectories,
findRepoRoot,
loadLocalPersonas,
resolvePersonaReference,
__mergeOverrideForTests
} from './index.js';

test('a later mount layer can re-enable inherited mount patterns', () => {
const base: PersonaSpec = {
Expand Down Expand Up @@ -105,3 +112,78 @@ test('unknown names fail with a typed resolution error', () => {
rmSync(cwd, { recursive: true, force: true });
}
});

test("a repo's personas load from a subdirectory of it", () => {
const root = mkdtempSync(join(tmpdir(), 'persona-registry-repo-'));
const repo = join(root, 'repo');
const nested = join(repo, 'packages', 'deep');
mkdirSync(join(repo, '.git'), { recursive: true });
mkdirSync(nested, { recursive: true });
const personas = join(repo, '.agentworkforce', 'workforce', 'personas');
mkdirSync(personas, { recursive: true });
writeFileSync(
join(personas, 'scout.json'),
JSON.stringify({
id: 'repo-scout',
extends: 'persona-maker',
description: 'Defined at the repo root'
})
);

try {
// Commands are typically run from a package directory, not the repo root.
const fromNested = loadLocalPersonas({ cwd: nested, personaDirs: [] });
assert.equal(fromNested.byId.get('repo-scout')?.description, 'Defined at the repo root');
assert.equal(fromNested.sources.get('repo-scout'), 'repo');

// The repo root itself still reports the persona as its own cwd layer,
// with no duplicate repo entry.
const fromRoot = loadLocalPersonas({ cwd: repo, personaDirs: [] });
assert.equal(fromRoot.sources.get('repo-scout'), 'cwd');
const rootDirs = buildPersonaSourceDirectories({ cwd: repo, personaDirs: [] }).directories;
assert.equal(rootDirs.some((d) => String(d.source).startsWith('repo')), false);
} finally {
rmSync(root, { recursive: true, force: true });
}
});

test('the cwd layer still outranks the repo root', () => {
const root = mkdtempSync(join(tmpdir(), 'persona-registry-rank-'));
const repo = join(root, 'repo');
const nested = join(repo, 'packages', 'deep');
mkdirSync(join(repo, '.git'), { recursive: true });
const repoPersonas = join(repo, '.agentworkforce', 'workforce', 'personas');
const nestedPersonas = join(nested, '.agentworkforce', 'workforce', 'personas');
mkdirSync(repoPersonas, { recursive: true });
mkdirSync(nestedPersonas, { recursive: true });
writeFileSync(
join(repoPersonas, 'scout.json'),
JSON.stringify({ id: 'scout', extends: 'persona-maker', description: 'repo root' })
);
writeFileSync(
join(nestedPersonas, 'scout.json'),
JSON.stringify({ id: 'scout', extends: 'persona-maker', description: 'package dir' })
);

try {
const loaded = loadLocalPersonas({ cwd: nested, personaDirs: [] });
assert.equal(loaded.byId.get('scout')?.description, 'package dir');
assert.equal(loaded.sources.get('scout'), 'cwd');
} finally {
rmSync(root, { recursive: true, force: true });
}
});

test('the repo walk stops rather than escaping to the home directory', () => {
const root = mkdtempSync(join(tmpdir(), 'persona-registry-norepo-'));
const loose = join(root, 'not', 'a', 'repo');
mkdirSync(loose, { recursive: true });

try {
assert.equal(findRepoRoot(loose), undefined);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The findRepoRoot(loose) === undefined assertion assumes no directory between the temp dir and the walk's stop point is a git repository. findRepoRoot checks .git at every ancestor until it reaches homedir(), so if a runner overrides TMPDIR to point inside a checked-out repo (or beneath a git-initialized home for dotfiles), the walk returns that ancestor instead of undefined and this test fails spuriously. Making the assertion robust to the ambient temp dir would prevent an environment-dependent failure.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/persona-registry/src/index.test.ts, line 183:

<comment>The `findRepoRoot(loose) === undefined` assertion assumes no directory between the temp dir and the walk's stop point is a git repository. `findRepoRoot` checks `.git` at every ancestor until it reaches `homedir()`, so if a runner overrides `TMPDIR` to point inside a checked-out repo (or beneath a git-initialized home for dotfiles), the walk returns that ancestor instead of `undefined` and this test fails spuriously. Making the assertion robust to the ambient temp dir would prevent an environment-dependent failure.</comment>

<file context>
@@ -105,3 +112,78 @@ test('unknown names fail with a typed resolution error', () => {
+  mkdirSync(loose, { recursive: true });
+
+  try {
+    assert.equal(findRepoRoot(loose), undefined);
+    const dirs = buildPersonaSourceDirectories({ cwd: loose, personaDirs: [] }).directories;
+    assert.equal(dirs.some((d) => String(d.source).startsWith('repo')), false);
</file context>

const dirs = buildPersonaSourceDirectories({ cwd: loose, personaDirs: [] }).directories;
assert.equal(dirs.some((d) => String(d.source).startsWith('repo')), false);
} finally {
rmSync(root, { recursive: true, force: true });
}
});
51 changes: 51 additions & 0 deletions packages/persona-registry/src/local-personas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ export type PersonaSource = string;
* - `cwd:agents` → same — `<cwd>/.agentworkforce/workforce/agents/<name>/persona.json`,
* agents that keep their persona next to their handler.
* Also a precise pointer, so also kept as-is.
* - `repo` → `repo` — `<repo root>/.agentworkforce/workforce/personas/`,
* the repository's own personas, visible from any
* subdirectory of it. Present only when cwd is below the
* root and the directory exists. `repo:agents` is its
* nested counterpart.
* - `dir:N` → `dir:N` — extra configurable persona dirs (passed
* through unchanged so position is still legible).
*
Expand Down Expand Up @@ -212,6 +217,27 @@ export function defaultCwdAgentDir(cwd: string): string {
return join(cwd, '.agentworkforce', 'workforce', 'agents');
}

/**
* The repository root at or above `cwd`, or `undefined` outside a repository.
*
* A repo's personas live at its root, but commands are typically run from a
* package or source subdirectory. Without this the cascade looks only at the
* exact cwd, so the same repo yields different personas depending on which
* directory you happen to be standing in. The walk stops at the home directory
* — `~/.agentworkforce/workforce/personas/` is already the `user` layer.
*/
export function findRepoRoot(cwd: string): string | undefined {
const home = resolvePath(homedir());
let dir = resolvePath(cwd);
while (true) {
if (existsSync(join(dir, '.git'))) return dir;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When $HOME itself contains .git and cwd is a child without a nearer repository, findRepoRoot returns $HOME before honoring the home boundary. This duplicates and relabels personal personas as the higher-priority repo layer, so check the home boundary before accepting .git.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/persona-registry/src/local-personas.ts, line 233:

<comment>When `$HOME` itself contains `.git` and `cwd` is a child without a nearer repository, `findRepoRoot` returns `$HOME` before honoring the home boundary. This duplicates and relabels personal personas as the higher-priority `repo` layer, so check the home boundary before accepting `.git`.</comment>

<file context>
@@ -212,6 +217,27 @@ export function defaultCwdAgentDir(cwd: string): string {
+  const home = resolvePath(homedir());
+  let dir = resolvePath(cwd);
+  while (true) {
+    if (existsSync(join(dir, '.git'))) return dir;
+    if (dir === home) return undefined;
+    const parent = dirname(dir);
</file context>
Suggested change
if (existsSync(join(dir, '.git'))) return dir;
if (dir !== home && existsSync(join(dir, '.git'))) return dir;

if (dir === home) return undefined;
Comment on lines +233 to +234

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Stop before treating the home directory as a repository root

When a user's home directory itself contains ~/.git, this check returns the home directory before the following stop condition runs. Consequently every non-repository working directory below home gains an unintended repo layer pointing at ~/.agentworkforce/workforce/personas; it can even re-enable personal personas that an explicit personaDirs configuration omitted and let them override configured sources. Check the home boundary before accepting its .git marker.

Useful? React with 👍 / 👎.

const parent = dirname(dir);
if (parent === dir) return undefined;
dir = parent;
}
}

/** Persona filename read from each subdirectory of a nested source dir. */
export const NESTED_PERSONA_FILENAME = 'persona.json';

Expand Down Expand Up @@ -360,6 +386,30 @@ function sourceForPersonaDir(
return dir === userPersonaDir ? 'user' : `dir:${idx + 1}`;
}

/**
* Persona directories contributed by the repository root when the command runs
* from a subdirectory. Ranked directly below the cwd layers: the directory you
* are standing in stays the most specific, and the repo answers for everywhere
* else inside it.
*/
function repoSourceDirectories(cwd: string): PersonaSourceDirectory[] {
const root = findRepoRoot(cwd);
if (!root || resolvePath(root) === resolvePath(cwd)) return [];
// Unlike the cwd layers — which are always listed because they are where
// `create` writes — a repo layer is only worth naming when it exists. Most
// repositories keep no personas, and listing a path nobody created is noise.
const dirs: PersonaSourceDirectory[] = [];
const personas = defaultCwdPersonaDir(root);
if (existsSync(personas)) {
dirs.push({ source: 'repo', dir: personas, configurable: false });
}
const agents = defaultCwdAgentDir(root);
if (existsSync(agents)) {
dirs.push({ source: 'repo:agents', dir: agents, configurable: false, nested: true });
}
return dirs;
}

export function buildPersonaSourceDirectories(
options: LoadOptions = {}
): { directories: PersonaSourceDirectory[]; config: PersonaSourceConfig } {
Expand All @@ -379,6 +429,7 @@ export function buildPersonaSourceDirectories(
configurable: false,
nested: true
},
...repoSourceDirectories(cwd),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve configured precedence for an already-listed repo directory

When the repo persona directory is already present in config.personaDirs—the existing workaround for making root personas visible from nested directories—this inserts the same directory again ahead of every configured source. For example, with configured sources [companyOverrides, repoPersonas], a conflicting repo persona now wins before companyOverrides is considered, silently changing the selected harness, permissions, and other settings after upgrade. Avoid adding the automatic repo layer when that directory is already explicitly configured so its chosen position remains authoritative.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: When the repository persona directory is already listed in config.personaDirs, this adds it a second time before the configured sources and changes precedence. Filter automatic repo entries whose directories are already configured so the explicit position remains authoritative.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/persona-registry/src/local-personas.ts, line 432:

<comment>When the repository persona directory is already listed in `config.personaDirs`, this adds it a second time before the configured sources and changes precedence. Filter automatic repo entries whose directories are already configured so the explicit position remains authoritative.</comment>

<file context>
@@ -379,6 +429,7 @@ export function buildPersonaSourceDirectories(
       configurable: false,
       nested: true
     },
+    ...repoSourceDirectories(cwd),
     ...config.personaDirs.map((dir, idx) => ({
       source: sourceForPersonaDir(dir, idx, config.userPersonaDir),
</file context>
Suggested change
...repoSourceDirectories(cwd),
...repoSourceDirectories(cwd).filter(({ dir }) => !config.personaDirs.some((configured) => resolvePath(configured) === resolvePath(dir))),

...config.personaDirs.map((dir, idx) => ({
source: sourceForPersonaDir(dir, idx, config.userPersonaDir),
dir,
Expand Down
Loading