Uh oh!
There was an error while loading. Please reload this page.
feat(sdk): spawn agents from named AgentWorkforce personas - #818
Conversation
Adds `relay.spawnPersona(id, options?)` so callers can pick a persona JSON by id from `./agentworkforce/personas` (or any configured dir) and spawn it without hand-rolling cli/model/system-prompt/MCP wiring. - `src/personas.ts` loads, resolves a tier (best/best-value/minimum), applies one level of `extends`, and delegates harness translation to `@agentworkforce/harness-kit#buildInteractiveSpec` so launch args match the AgentWorkforce CLI exactly. - `AgentRelay` gains a `personaDirs` constructor option for the default search-dir cascade; per-call `searchDirs` / `extraDirs` / `tier` still override. - For opencode personas, `opencode.json` is materialized in the spawn cwd and restored on agent exit. - Subpath export `@agent-relay/sdk/personas`, plus a runnable example at `src/examples/persona-spawn.ts` and 14 unit tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds a personas subsystem: JSON persona discovery and lookup across cascaded dirs, persona loading with tier selection and one-level extends merging and validation, translation to harness-specific spawn specs, sandboxed config-file materialization/restoration, AgentRelay.spawnPersona integration, package exports, tests, and a CLI example. ChangesPersona Subsystem
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
barryollama
left a comment
There was a problem hiding this comment.
📝 Code Review: feat(sdk): spawn agents from named AgentWorkforce personas
Summary
This is a well-designed feature that cleanly bridges Agent Relay with the AgentWorkforce persona system. The implementation is thorough with good abstractions and comprehensive test coverage.
🔍 Key Findings
Good
- Clean separation of concerns between persona loading, resolution, and translation
- Delegation to
@agentworkforce/harness-kitensures consistency with CLI - Proper path traversal protection in
materializePersonaConfigFiles - Config file cleanup is handled via
waitForExit().finally()- nicely done - Comprehensive test coverage (14 tests covering cascade, tiers, extends, harness translation, file materialization)
Suggestions
Error message clarity in
loadPersona: When a persona is missing required fields, the error messages are helpful. Consider also validating at parse time thattiers.*.harnessvalues are inHARNESS_VALUESto fail earlier.Codex system prompt handling: The composition approach works but consider documenting this explicitly in the persona schema docs since it's a platform limitation.
Config file race condition: If
restorePersonaConfigFilesthrows (despite best-effort swallowing), the.finally()inspawnPersonacould fail silently. Consider logging restore failures.Type safety: The type assertion
value as unknown as PersonaFileinparsePersonaFilecould be tightened with runtime validation, though likely acceptable for internal use.
🧪 Test Coverage
- ✅ Cascade lookup and resolution
- ✅ Tier resolution (best/best-value/minimum)
- ✅
extendsmerging (including permissions) - ✅ Claude/Codex/OpenCode arg shapes
- ✅ Config file materialize/restore
- ✅ Path traversal rejection
- ✅ Constructor vs per-call overrides
📝 Documentation
The example file and PR description are excellent. Consider adding a small note in the SDK README about the persona cascade order for discoverability.
✅ Approval
LGTM - nice work on the abstraction layers and test coverage.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:9756048939
ℹ️ 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".
| throw new Error(`persona config file path must be relative: ${file.path}`); | ||
| } | ||
| const target = resolvePath(cwd, file.path); | ||
| if (target !== cwdAbs && !target.startsWith(cwdAbs + '/')) { |
There was a problem hiding this comment.
The escape check in materializePersonaConfigFiles hardcodes '/' in target.startsWith(cwdAbs + '/'), which breaks on Windows where resolvePath returns backslash-separated paths (for example, C:\proj\opencode.json). In that environment, even valid relative config files are treated as escaping cwd, so spawnPersona fails for personas that require materialized config files (notably opencode). Use a separator-agnostic containment check (e.g., path.relative or path.sep) to avoid rejecting legitimate paths.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9b7a4c3. The check now uses path.relative(cwdAbs, target) and rejects only when the relative path starts with .. or contains a .. segment — separator-agnostic, so backslash-separated Windows paths work.
Also added a test (materializePersonaConfigFiles allows nested paths inside cwd) that exercises a multi-segment relative path.
| throw new Error(`persona config file path must be relative: ${file.path}`); | ||
| } | ||
| const target = resolvePath(cwd, file.path); | ||
| if (target !== cwdAbs && !target.startsWith(cwdAbs + '/')) { |
There was a problem hiding this comment.
🔴 Hardcoded '/' separator in path-escape check breaks materializePersonaConfigFiles on Windows
On Windows, path.resolve returns backslash-separated paths (e.g. C:\Users\project\opencode.json), but the path-traversal guard at line 463 compares against cwdAbs + '/'. Since a backslash path never starts with a forward-slash prefix, target.startsWith(cwdAbs + '/') is always false on Windows, causing every config file materialization to throw "persona config file path escapes cwd" — even perfectly valid relative paths like opencode.json.
This breaks opencode persona spawning entirely on Windows. The existing codebase already uses path.sep for the identical check pattern (see packages/sdk/src/provisioner/seeder.ts:153 and packages/sdk/src/workflows/verification.ts:276).
Prompt for agents
In packages/sdk/src/personas.ts, the path-escape check on line 463 uses a hardcoded forward slash:
if (target !== cwdAbs && !target.startsWith(cwdAbs + '/')) {
This breaks on Windows because path.resolve returns backslash paths. Import sep from node:path (already importing dirname, isAbsolute, join, resolve) and change the check to use path.sep instead of '/'. The existing codebase uses this exact pattern — see packages/sdk/src/provisioner/seeder.ts:153 which does !resolved.startsWith(rootDir + path.sep) && resolved !== rootDir, and packages/sdk/src/workflows/verification.ts:276 which does !resolved.startsWith(normalizedCwd + path.sep).
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in 9b7a4c3. Switched to path.relative + path.sep for separator-agnostic containment, matching the pattern used in provisioner/seeder.ts and workflows/verification.ts. New test verifies nested paths inside cwd work, and the existing escape-cwd test still passes.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/sdk/src/personas.ts`:
- Around line 457-465: The path-escape check incorrectly concatenates a
hardcoded '/' to cwdAbs, which fails on Windows where resolvePath returns
backslashes; update the check in the loop that validates persona file paths (the
block using isAbsolute(file.path), resolvePath(cwd, file.path), and cwdAbs) to
use the platform path separator: import sep from 'path' (or use path.sep) and
replace the startsWith comparison cwdAbs + '/' with cwdAbs + sep so it correctly
recognizes child paths on all platforms.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 35502659-93d1-45ed-9e3c-f0d4342e0eae
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
packages/sdk/package.jsonpackages/sdk/src/__tests__/personas.test.tspackages/sdk/src/examples/persona-spawn.tspackages/sdk/src/index.tspackages/sdk/src/personas.tspackages/sdk/src/relay.ts
Uh oh!
There was an error while loading. Please reload this page.
…validation) - materializePersonaConfigFiles: replace hardcoded `cwdAbs + '/'` startsWith check with path.relative + path.sep so backslash-separated Windows paths aren't falsely rejected (P1 from codex / devin / coderabbit). - materializePersonaConfigFiles: collapse existsSync + readFileSync into a single readFileSync that interprets ENOENT as "did not exist", closing the CodeQL-flagged TOCTOU on line 469. - listPersonas / findPersona: drop statSync + existsSync prechecks for the same reason (CodeQL line 215). For convention-named persona files the parse error now propagates so a typo isn't silently reported as "not found". - parsePersonaFile: validate top-level and per-tier `harness` against HARNESS_VALUES so authoring errors surface at load time rather than spawn (per @barryollama review). - restorePersonaConfigFiles: log restore failures via console.warn rather than swallowing them (per @barryollama review). 3 new tests cover nested paths, invalid top-level harness, and invalid per-tier harness. 17 tests total, typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
willwashburn
commented
May 8, 2026
Pushed 9b7a4c3 to address review feedback. Fixed
Notes on @barryollama's other suggestions
Tests: 17 total (3 new), typecheck clean. |
To use Codex here, create an environment for this repo. |
The On your open notes:
Happy to trigger a fresh review pass on the updated commit if you'd like. ✅ Actions performedReview triggered.
|
Uh oh!
There was an error while loading. Please reload this page.
Summary
relay.spawnPersona(personaId, options?)so callers can pick a persona JSON by id (from./agentworkforce/personas,~/.agentworkforce/..., or any configured dir) and spawn it without hand-wiring cli/model/system-prompt/MCP/permission flags.src/personas.tsloads + resolves the persona, applies one level ofextends, and delegates harness translation to@agentworkforce/harness-kit#buildInteractiveSpec— so launch args match the AgentWorkforce CLI exactly.AgentRelayconstructor gainspersonaDirs?: string[]to set the default persona search-dir cascade once; per-callsearchDirs/extraDirs/tierstill override.Behavior
--model,--append-system-prompt,--mcp-config,--strict-mcp-config,--allowedTools/--disallowedTools/--permission-modefrom the persona.-m <model>and folds the persona'ssystemPromptinto the task (codex has no system-prompt flag).opencode.jsonin the spawn cwd, selects with--agent <id>, and restores the prior file when the agent exits.best; supportsbest-valueandminimum.console.warn.API
Subpath export:
@agent-relay/sdk/personasexposesloadPersona,listPersonas,findPersona,buildPersonaSpawnSpec, plus the persona schema types.Dependencies
Adds
@agentworkforce/harness-kit@^0.11.0and@agentworkforce/workload-router@^0.11.0(small, pure-data packages).Test plan
tsc --noEmitcleansrc/__tests__/personas.test.ts(cascade lookup, tier resolution, extends merging, claude/codex/opencode arg shapes, config-file materialize/restore, escape-cwd rejection, constructorpersonaDirsdefault, per-call override)node dist/examples/persona-spawn.js <persona-id> "<task>"once a workspace key is available🤖 Generated with Claude Code