fix(core): support Workforce persona packages 4.1.39 - #29
Conversation
📝 WalkthroughWalkthroughThe PR updates persona dependencies, strengthens agent constraint typing, permits registered ChangesPersona agent handling
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/core/src/builder.ts`:
- Around line 336-340: Update the AgentDefinition construction in the builder to
use separate CLI and persona branches instead of casting the combined object.
Validate and reject incompatible options before constructing the definition:
prevent both cli and persona from being emitted, allow constraints.model only
for CLI agents, and disallow role, preset, and interactive:false for persona
agents. Ensure the options.model handling around the existing model-copy logic
is gated by the CLI branch so toYaml() produces a valid union.
🪄 Autofix
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: 8147fed1-f017-4bd1-b760-a6d2e733d385
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
packages/core/package.jsonpackages/core/src/__tests__/e2e-owner-review.test.tspackages/core/src/__tests__/workflow-runner.test.tspackages/core/src/builder.tspackages/core/src/persona-runtime.tspackages/core/src/runner.ts
| const def = { | ||
| name, | ||
| ...(options.cli ? { cli: options.cli } : {}), | ||
| ...(options.persona ? { persona: options.persona } : {}), | ||
| }; | ||
| } as AgentDefinition; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Build the correct AgentDefinition variant.
AgentDefinition permits constraints.model only for CLI agents. It also forbids role, preset, and interactive: false for persona agents. The cast at Line 340 bypasses this union, and Lines 366-373 copy options.model without checking options.persona. The builder can emit both cli and persona, or emit a persona with constraints.model. toYaml() can then return invalid configuration that WorkflowRunner.validateConfig rejects later.
Construct CLI and persona branches separately. Reject incompatible options before adding the definition.
Proposed guard
- const def = {
- name,
- ...(options.cli ? { cli: options.cli } : {}),
- ...(options.persona ? { persona: options.persona } : {}),
- } as AgentDefinition;
+ const hasCli = options.cli !== undefined;
+ const hasPersona = options.persona !== undefined;
+ if (hasCli === hasPersona) {
+ throw new Error(`Agent "${name}" must define exactly one of "cli" or "persona"`);
+ }
+ if (
+ hasPersona &&
+ (options.model !== undefined ||
+ options.role !== undefined ||
+ options.preset !== undefined ||
+ options.interactive === false)
+ ) {
+ throw new Error(`Agent "${name}" has options that are not supported for persona agents`);
+ }
+ const def: AgentDefinition = hasCli
+ ? { name, cli: options.cli! }
+ : { name, persona: options.persona! };
- if (options.model !== undefined) constraints.model = options.model;
+ if (hasCli && options.model !== undefined) constraints.model = options.model;Also applies to: 366-373
🤖 Prompt for 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.
In `@packages/core/src/builder.ts` around lines 336 - 340, Update the
AgentDefinition construction in the builder to use separate CLI and persona
branches instead of casting the combined object. Validate and reject
incompatible options before constructing the definition: prevent both cli and
persona from being emitted, allow constraints.model only for CLI agents, and
disallow role, preset, and interactive:false for persona agents. Ensure the
options.model handling around the existing model-copy logic is gated by the CLI
branch so toYaml() produces a valid union.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
Addressed CodeRabbit's builder-union finding in
Revalidated build, typecheck, and 83 affected persona/builder/runner tests. |
3 participants
Summary
Verification
npm ci --ignore-scripts --no-audit --no-fundnpm run buildnpm run typecheckenv -u AGENT_RELAY_STATE_DIR npm test -- --reporter=dot(45 files, 820 tests)The browser primitive has no test files; its required TypeScript build passes.