feat(persona-registry): load personal agents from the workforce home - #322
feat(persona-registry): load personal agents from the workforce home#322willwashburn wants to merge 7 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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: 3b90217ede
ℹ️ 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".
| return dir === config.userPersonaDir | ||
| ? [entry, userAgentSourceDir(config.userPersonaDir)] |
There was a problem hiding this comment.
For a handler-style personal agent produced by agentworkforce persona compile, persona.json contains intent and onEvent and may legitimately omit harness, model, and systemPrompt. Routing it through this new registry layer does not work: parseOverride discards onEvent, then standaloneSpecFromOverride treats the file as an interactive standalone persona and rejects the omitted runtime fields, so the advertised agent-with-handler is omitted from byId with a warning. Load these nested agent files as full handler-capable persona specs, or preserve onEvent and make standalone validation handler-aware.
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.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
An agent driven by its `onEvent` entry has no interactive launch to configure, so `harness`, `model`, and `systemPrompt` — already optional on `PersonaSpec` — are optional here too. Requiring them kept exactly the agents the `agents/` directory was added for out of the registry, reporting a valid deployable persona as malformed. `onEvent` and `cloud` now survive parse and merge. An overlay that tweaks env no longer strips the handler entry that makes its base deployable, so the merged spec is a complete agent rather than a partial one. `harnessSettings` stays required: `PersonaSpec` types it non-optional, and `reasoning`/`timeoutSeconds` have no defensible default to invent on a persona's behalf. `onEvent` is validated as a relative path that cannot escape the agent directory, matching the sidecar rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Both P1s are the same defect, and correct — a handler-style agent whose compiled It is not specific to this layer: So this PR needs #323 to land, not a change of its own. One caveat #323 documents rather than solves: The P3 on the README is fixed in |
|
Correction: the README fix is |
Hand-rolling the guard drifted from persona-kit twice over. It validated the raw string and stored a trimmed copy, so `" ../x/agent.ts "` cleared the `..` check as the segment `" .."` and escaped the agent directory once trimmed. And it never checked the handler extension, so `onEvent: "README.md"` counted as a handler and skipped the interactive fields the persona never declared. `parseOnEvent` owns both rules and returns the exact string it validated, so the stored value cannot differ from the one that passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
f08bbbb to
662b2da
Compare
Order matters in both directions. Validating the raw string and storing a trimmed copy let `" ../x/agent.ts "` clear the `..` check as the segment `" .."` and escape once trimmed. Validating without trimming stored `" ./agent.ts"`, which passes every check and then resolves against a directory named `" ."` at deploy. Trimming before `parseOnEvent` makes the validated value and the stored value the same string. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An agent directory whose `persona.json` is older than its `persona.ts` loads the compiled spec with none of the edits sitting in the authoring file, and nothing about the result looks wrong. #316 warned about a persona that was never compiled; this covers the quieter case where it was compiled once. The persona is still served. Dropping it would turn a forgotten compile into a missing persona, which is a worse failure than an out-of-date one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… file A directory that still carries an abandoned `persona.ts` after development moved to `persona.js` was measured against the file nobody edits, so a `persona.json` newer than the dead source but older than the live one read as fresh. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`~/.agentworkforce/workforce/agents/<name>/persona.json` is a cascade layer, the personal mirror of the `cwd:agents` layer added in #316. An agent that ships its own handler is available in every repo instead of only the one it was checked into. It rides directly behind the personal personas dir rather than at a fixed depth, so moving that dir in the cascade moves the pair together. A repo agent outranks a personal one of the same id. The directory is derived from wherever the personal personas dir resolves, so AGENT_WORKFORCE_CONFIG_DIR and embedder overrides carry it along instead of silently reading the developer's real home. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The cascade list documented only the internal source string, so a reader who then ran `sources list` saw a label the docs never mention. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
7303cc5 to
3425fa1
Compare
662b2da to
91c03cd
Compare
1 participant
~/.agentworkforce/workforce/agents/<name>/persona.jsonis a cascade layer — the personal mirror of thecwd:agentslayer added in #316. An agent that ships its own handler is available in every repo instead of only the one it was checked into.Rank
It rides directly behind the personal personas dir rather than sitting at a fixed depth. The two are a pair — loose overrides and agents-with-handlers at the same scope — so moving the personas dir in the cascade moves both, and a team source added with
sources add --position 1still outranks both. A repo agent outranks a personal one of the same id, and merges over it, so a project can pin a model or channel on an agent you carry everywhere.Directory derivation
Derived from wherever the personal personas dir resolves (
dirname(userPersonaDir)/agents), not recomputed fromAGENT_WORKFORCE_HOME. Recomputing would make embedders and tests that pass an explicit personas dir silently read the developer's real~/.agentworkforce/workforce/agents— the registry is used by fleet nodes and test harnesses that isolate lookup on purpose.user:agentsdisplays aspersonal:agents, following the existinguser→personalrename; the internal string is unchanged for--jsonconsumers.Verification
Exercised against the real
../salescheckout for the cascade listing. Six new tests cover discovery, relative-skill resolution against the agent's own directory, repo-over-personal precedence with merge, the display label, and a missing directory.local-personassuite passes (56).Semver: minor — new cascade layer and new
userAgentDirForexport.🤖 Generated with Claude Code