feat(persona-registry): load a repo's personas from any subdirectory of it - #319
feat(persona-registry): load a repo's personas from any subdirectory of it#319willwashburn wants to merge 1 commit into
Conversation
…of it Persona discovery looked only at the exact cwd, so a repository's personas in `.agentworkforce/workforce/personas/` were visible from the repo root and nowhere else. Running the same command from a package or source subdirectory — where people actually work — silently dropped them, with no warning that a layer had gone missing. The cascade now walks up to the repository root and contributes its personas as a `repo` layer (plus `repo:agents` for the nested form), ranked directly below the cwd layers: the directory you stand in stays most specific, and the repo answers for everywhere else inside it. The walk stops at the home directory, since `~/.agentworkforce/workforce/ personas/` is already the `user` layer. A repo layer is listed only when it exists. The cwd layers are always listed because that is where `create` writes; naming a repo path nobody created would just be noise, and it would shift the positions that `sources list` consumers read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 49 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6a59529db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| configurable: false, | ||
| nested: true | ||
| }, | ||
| ...repoSourceDirectories(cwd), |
There was a problem hiding this comment.
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 👍 / 👎.
| if (existsSync(join(dir, '.git'))) return dir; | ||
| if (dir === home) return undefined; |
There was a problem hiding this comment.
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 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/persona-registry/src/local-personas.ts">
<violation number="1" location="packages/persona-registry/src/local-personas.ts:233">
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`.</violation>
<violation number="2" location="packages/persona-registry/src/local-personas.ts:432">
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.</violation>
</file>
<file name="packages/persona-registry/src/index.test.ts">
<violation number="1" location="packages/persona-registry/src/index.test.ts:183">
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.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| configurable: false, | ||
| nested: true | ||
| }, | ||
| ...repoSourceDirectories(cwd), |
There was a problem hiding this comment.
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>
| ...repoSourceDirectories(cwd), | |
| ...repoSourceDirectories(cwd).filter(({ dir }) => !config.personaDirs.some((configured) => resolvePath(configured) === resolvePath(dir))), |
| const home = resolvePath(homedir()); | ||
| let dir = resolvePath(cwd); | ||
| while (true) { | ||
| if (existsSync(join(dir, '.git'))) return dir; |
There was a problem hiding this comment.
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>
| if (existsSync(join(dir, '.git'))) return dir; | |
| if (dir !== home && existsSync(join(dir, '.git'))) return dir; |
| mkdirSync(loose, { recursive: true }); | ||
|
|
||
| try { | ||
| assert.equal(findRepoRoot(loose), undefined); |
There was a problem hiding this comment.
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>
1 participant
A repository's personas are visible from anywhere inside it, not only from its root.
Why
.agentworkforce/workforce/personas/is a repo-level directory, but discovery resolved it against the exact cwd. From the repo root the personas load; frompackages/anything/— where people actually work — they silently vanish. Nothing warns that a layer disappeared, so the same command yields different personas depending on which directory you happen to be standing in.What changed
The cascade walks up to the repository root and contributes its personas as a
repolayer, withrepo:agentsfor the nested<name>/persona.jsonform. Ranking is directly below the cwd layers, so precedence reads outside-in:The directory you are standing in stays the most specific; the repo answers for everywhere else inside it. The walk stops at the home directory, since
~/.agentworkforce/workforce/personas/is already theuserlayer.A repo layer is listed only when it exists. The cwd layers are always listed because that is where
createwrites, but naming a repo path nobody created would be noise — and it would shift the positionssources listconsumers read. In a repo with no personas directory the cascade is byte-identical to before.Tests
Three cases in
persona-registry, verified red before green (removing the walk fails the first):reporepolayerSuite: 7/7. The
@agentworkforce/clisuite has 30 failures onorigin/mainbefore this change and the same 30 after — the positionalsources listassertions are unaffected, because this repo has no persona directory of its own.🤖 Generated with Claude Code