feat(roles): Add configurable AI agent personas (Sage, Harvester, Curator, etc.)#41
feat(roles): Add configurable AI agent personas (Sage, Harvester, Curator, etc.)#41jbdevprimary wants to merge 17 commits into
Conversation
…ator, etc.) - Add roles module with RoleDefinition, RoleConfig, RoleTrigger types - Add default roles: Sage, Harvester, Curator, Reviewer, Fixer, Delegator - Add role executor with executeRole, executeSageRole functions - Add roles CLI commands: list, info, sage, match - Add RolesConfig to AgenticConfig for agentic.config.json support - Export roles module from package index Closes #40
|
/gemini review |
|
/q review |
Summary of ChangesHello @jbdevprimary, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a powerful and well-structured roles module for defining configurable AI agent personas, which is a great addition. The implementation includes default roles, an execution engine, and corresponding CLI commands. The code is generally well-written and the feature is comprehensive.
My review focuses on a couple of key areas. There's a critical issue with the configuration type definitions in src/core/config.ts, which are incomplete and prevent users from accessing the full customization features like custom roles or trigger overrides. This also points to a structural issue with circular dependencies that should be addressed. Additionally, I've noted a type safety issue in the new CLI command for the sage role that could be improved.
| export interface RoleConfig { | ||
| /** Enable/disable the role */ | ||
| enabled?: boolean; | ||
| /** Override the system prompt */ | ||
| systemPrompt?: string; | ||
| /** Model override */ | ||
| model?: string; | ||
| } | ||
|
|
||
| export interface RolesConfig { | ||
| /** Sage role configuration */ | ||
| sage?: RoleConfig; | ||
| /** Harvester role configuration */ | ||
| harvester?: RoleConfig; | ||
| /** Curator role configuration */ | ||
| curator?: RoleConfig; | ||
| /** Reviewer role configuration */ | ||
| reviewer?: RoleConfig; | ||
| /** Fixer role configuration */ | ||
| fixer?: RoleConfig; | ||
| /** Delegator role configuration */ | ||
| delegator?: RoleConfig; | ||
| } |
There was a problem hiding this comment.
The RoleConfig and RolesConfig interfaces are incomplete compared to their definitions in src/roles/types.ts, which prevents users from using the full range of role customization features.
RoleConfigis missingtriggers,additionalCapabilities,revokedCapabilities, andsettings.RolesConfigis missing thecustomproperty for defining custom roles.
Adding these properties by importing types from src/roles/types.ts would create a circular dependency (core/config.ts -> roles/types.ts -> ... -> core/config.ts).
To resolve this structural issue, I recommend moving the core role type definitions (RoleTrigger, AgentCapability, RoleDefinition) from src/roles/types.ts to a more central location that doesn't create import cycles, such as src/core/types.ts. Once the types are moved, you can update RoleConfig and RolesConfig here to be complete.
|
|
||
| console.log('🔮 Sage is thinking...\n'); | ||
|
|
||
| const result = await executeSageRole(query, model as Parameters<typeof executeSageRole>[1]); |
There was a problem hiding this comment.
The type assertion model as Parameters<typeof executeSageRole>[1] is unsafe because the model variable is of type unknown. This bypasses TypeScript's type safety and could lead to runtime errors if the provider loading logic changes.
The root cause appears to be the ModelFactory type definition in src/core/providers.ts, which returns unknown. To improve type safety, consider updating ModelFactory to return LanguageModel from the ai package. This would allow the correct type to flow through the system, making this assertion unnecessary.
Pull Request Test Coverage Report for Build 20566069519Details
💛 - Coveralls |
There was a problem hiding this comment.
Code Review
This pull request introduces a well-structured and powerful roles module for defining configurable AI agent personas. The separation of types, defaults, and execution logic is clean. My review focuses on ensuring this new system is used consistently and correctly throughout the codebase. The main points are:
- A critical issue with redundant and incomplete type definitions in the configuration file that should be consolidated.
- A critical bug in the new CLI commands where they fail to load and respect user-defined role configurations.
- An inconsistency in the
sageCLI command, which bypasses the new roles configuration system. - Some minor improvements for type safety and code clarity.
Addressing these points will make the new feature more robust and consistent.
| .action((opts) => { | ||
| const roles = listRoles(); |
There was a problem hiding this comment.
The roles CLI commands (list, info, match) do not load the user's configuration from agentic.config.json. As a result, they only operate on the default, hardcoded role definitions. This means any user customizations (disabling roles, overriding prompts, defining custom roles) will be completely ignored by the CLI.
You should load the configuration at the beginning of each action and pass it to the role helper functions.
For example:
import { getConfig } from './core/config.js';
// ...
.action((opts) => {
const config = getConfig();
const roles = listRoles(config.roles);
// ...
});This needs to be applied to the list, info, and match commands.
| export interface RoleConfig { | ||
| /** Enable/disable the role */ | ||
| enabled?: boolean; | ||
| /** Override the system prompt */ | ||
| systemPrompt?: string; | ||
| /** Model override */ | ||
| model?: string; | ||
| } | ||
|
|
||
| export interface RolesConfig { | ||
| /** Sage role configuration */ | ||
| sage?: RoleConfig; | ||
| /** Harvester role configuration */ | ||
| harvester?: RoleConfig; | ||
| /** Curator role configuration */ | ||
| curator?: RoleConfig; | ||
| /** Reviewer role configuration */ | ||
| reviewer?: RoleConfig; | ||
| /** Fixer role configuration */ | ||
| fixer?: RoleConfig; | ||
| /** Delegator role configuration */ | ||
| delegator?: RoleConfig; | ||
| } |
There was a problem hiding this comment.
There are redundant and incomplete definitions for RoleConfig and RolesConfig here. The complete and correct types are already defined in packages/agentic-control/src/roles/types.ts.
Using these incomplete local definitions will cause issues. For example, properties like custom roles or additionalCapabilities in the configuration will be ignored or cause type errors.
You should remove these interfaces and instead import them from ../roles/types.js where they are used (e.g., in AgenticConfig).
| .option('--repo <owner/repo>', 'Repository context') | ||
| .option('--issue <number>', 'Issue number for context') | ||
| .option('--json', 'Output as JSON') | ||
| .action(async (query, opts) => { |
There was a problem hiding this comment.
The sage command is tightly coupled to the triage configuration and hardcoded values, which bypasses the new flexible roles configuration system.
- It uses
getTriageConfig()andgetTriageApiKey()instead of sourcing configuration from thesagerole definition. - The error message on line 1038 is hardcoded to
ANTHROPIC_API_KEY, which will be incorrect if the user configures a different provider (e.g., OpenAI). - The fallback model on line 1043 is hardcoded, whereas the
SAGE_ROLEdefinition already specifies adefaultModel.
To align with the new roles architecture, this command should use getEffectiveRole('sage') to get its configuration and then dynamically determine the provider, model, and API key based on that role's effective configuration.
| if (trigger.type === 'comment') { | ||
| console.log(` 💬 Comment: ${(trigger as { pattern: string }).pattern}`); | ||
| } else if (trigger.type === 'schedule') { | ||
| console.log(` ⏰ Schedule: ${(trigger as { cron: string }).cron}`); | ||
| } else if (trigger.type === 'event') { | ||
| console.log(` 🎯 Events: ${(trigger as { events: string[] }).events.join(', ')}`); |
There was a problem hiding this comment.
The type assertions like (trigger as { pattern: string }).pattern are redundant. The RoleTrigger type is a discriminated union based on the type property. When you check trigger.type === 'comment', TypeScript's control flow analysis automatically narrows the type of trigger to { type: 'comment'; pattern: string }, so you can safely access trigger.pattern without a cast. Removing these casts makes the code cleaner and relies on the strength of the type system.
| if (trigger.type === 'comment') { | |
| console.log(` 💬 Comment: ${(trigger as { pattern: string }).pattern}`); | |
| } else if (trigger.type === 'schedule') { | |
| console.log(` ⏰ Schedule: ${(trigger as { cron: string }).cron}`); | |
| } else if (trigger.type === 'event') { | |
| console.log(` 🎯 Events: ${(trigger as { events: string[] }).events.join(', ')}`); | |
| if (trigger.type === 'comment') { | |
| console.log(` 💬 Comment: ${trigger.pattern}`); | |
| } else if (trigger.type === 'schedule') { | |
| console.log(` ⏰ Schedule: ${trigger.cron}`); | |
| } else if (trigger.type === 'event') { | |
| console.log(` 🎯 Events: ${trigger.events.join(', ')}`); |
| } | ||
|
|
||
| // Apply config overrides if present | ||
| const roleConfig = rolesConfig?.[roleId as keyof RolesConfig] as RoleConfig | undefined; |
There was a problem hiding this comment.
The type assertion roleId as keyof RolesConfig is not entirely type-safe, as roleId is a generic string and may not actually be a key of RolesConfig. This can be made safer and clearer by avoiding the direct cast. You can treat rolesConfig as a Record for this lookup to make the intent clearer and slightly safer.
| const roleConfig = rolesConfig?.[roleId as keyof RolesConfig] as RoleConfig | undefined; | |
| const roleConfig = (rolesConfig as Record<string, RoleConfig | undefined>)?.[roleId]; |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| ], | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
Passed model parameter ignored in main execution path
The executeSageRole function accepts a model: LanguageModel parameter but the primary code path ignores it entirely. Instead, new Analyzer() is created without options, causing it to use its own model configuration from environment/config defaults via resolveProviderOptions(). The passed model is only used in the catch block fallback. This means the CLI carefully configures a model with the correct API key and provider, but that configuration is silently discarded in favor of the Analyzer's defaults.
| fixer?: RoleConfig; | ||
| /** Delegator role configuration */ | ||
| delegator?: RoleConfig; | ||
| } |
There was a problem hiding this comment.
Duplicate RolesConfig type missing custom roles field
There are two RolesConfig and RoleConfig type definitions that differ in their properties. The version in config.ts (used by AgenticConfig.roles) is missing the custom field and several RoleConfig properties like triggers, additionalCapabilities, and revokedCapabilities. These features are used by getEffectiveRole and applyRoleConfig in the executor, so users configuring via AgenticConfig won't get proper type hints for these supported features.
Additional Locations (1)
|
|
||
| console.log('🔮 Sage is thinking...\n'); | ||
|
|
||
| const result = await executeSageRole(query, model as Parameters<typeof executeSageRole>[1]); |
There was a problem hiding this comment.
CLI options for repo and issue context never used
The roles sage command defines --repo and --issue options (lines 1026-1027) but never uses them. The call to executeSageRole on line 1047 only passes query and model, omitting the third context parameter where these options would be passed. The executeSageRole function does support context.issueContext for enriching the query, but this feature is inaccessible from the CLI despite being advertised.
- Add tsup.config.ts to all packages (agentic-control, providers, vitest-agentic-control) - Update build scripts to use tsup - Use --no-frozen-lockfile for transition
🤖 Agent Handoff: @cursorEnterprise Dashboard: jbcom/control-center#505 ContextAdds configurable AI agent personas (Sage, Harvester, Curator, etc.) to control package. Your Responsibilities
PriorityPhase 2 - Feature PRs (after ESM foundation). DependencyPlease respond with status. |
- Add missing peer dependencies to vitest-agentic-control - Standardize dependency definitions in providers package - Move @agentic/triage to devDependencies/peerDependencies in providers
|
3 participants
Summary
Adds a roles module to @agentic-dev-library/control for configurable AI agent personas that repositories can use out of the box or customize.
New Files
src/roles/types.ts- RoleDefinition, RoleConfig, RoleTrigger, AgentCapability typessrc/roles/defaults.ts- Default role definitions for Sage, Harvester, Curator, Reviewer, Fixer, Delegatorsrc/roles/executor.ts- Role execution with executeRole, executeSageRolesrc/roles/index.ts- Module exportsChanges
src/cli.ts- Addedagentic rolescommands (list, info, sage, match)src/core/config.ts- Added RolesConfig to AgenticConfigsrc/index.ts- Export roles moduleDefault Roles
CLI Usage
```bash
List all roles
agentic roles list
Get role details
agentic roles info sage
Run Sage directly
agentic roles sage "How do I add a new endpoint?"
Find which role matches a trigger
agentic roles match "@Sage"
```
Configuration
```json
{
"roles": {
"sage": {
"enabled": true,
"model": "claude-sonnet-4-20250514"
},
"harvester": {
"enabled": false
}
}
}
```
Test Plan
Closes #40
Note
Introduces configurable AI roles with default personas and execution utilities, plus CLI and config integration.
roles/module:types.ts(role and config types),defaults.ts(built-in roles),executor.ts(executeRole,executeSageRole,listRoles,findRoleByTrigger,getEffectiveRole), andindex.tsexportsagentic rolessubcommands —list,info,sage,matchinsrc/cli.tsAgenticConfigwithrolesandRoleConfig/RolesConfiginsrc/core/config.tssrc/index.tsWritten by Cursor Bugbot for commit 597bad6. This will update automatically on new commits. Configure here.