feat: sanctioned AgentRole extension for plugin agent roles - #1492
feat: sanctioned AgentRole extension for plugin agent roles#1492lambdabaa wants to merge 2 commits into
Conversation
…kashgit#1484) register_agent_role() in factory/workflow/primitives.py gives plugins a tested, collision-checked way to extend the AgentRole enum so their roles work inside workflow graphs (AgentNode.role, GateNode.evaluator_role), matching what docs/plugins.md already promised. PluginRegistry.add_agent_roles() now calls it, so a role registered at the plugin level is usable in both the CLI and graphs without enum-mutation workarounds. Beyond centralizing the insertion, this fixes a latent bug in the import-time mutation workaround: Pydantic freezes an enum's valid values into model core schemas at class-definition time, so workflows containing dynamically added roles failed JSON validation (and thus to_dict/from_dict roundtrips via json paths). register_agent_role() rebuilds the schemas of models defined in primitives.py after insertion, so serialization is roundtrip-clean. Builtin collisions raise ValueError at the register_agent_role level; add_agent_roles keeps its existing skip-with-warning behavior, consistent with the other registry methods. Co-Authored-By: Claude Code <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1492 +/- ##
==========================================
+ Coverage 83.59% 83.66% +0.06%
==========================================
Files 225 225
Lines 25398 25467 +69
Branches 4128 4144 +16
==========================================
+ Hits 21232 21307 +75
+ Misses 3202 3194 -8
- Partials 964 966 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@ceo-review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Factory Review: KEEP
Verdict: KEEP
Reason: QA: CLEAN — 846 tests pass (1 pre-existing failure unrelated to PR), 24 new PR tests pass, lint clean, mypy clean (5 pre-existing errors in untouched files), composite score 0.9254 with no regression. Code review: 6/7 PASS (1 minor file-length nit at 502 lines). Adversarial QA: 10/10 tests verified with evidence — core JSON roundtrip fix confirmed, all collision/edge cases correct, PluginRegistry integration working.
QA Analysis
Adversarial QA — PR #1492: Sanctioned AgentRole Extension for Plugin Agent Roles
Detected project type: Library
Date: 2026-09-10
Tester: Adversarial QA Agent
Smoke Test
Command:
uv run pytest tests/test_models.py tests/test_guards.py tests/test_runners.py -x -q --tb=short -k 'not (BobAuth or preflight_error_unchanged)'Result: 165 passed in 4.57s
Status: ✅ PASS
Feature Tests
Test 1: Basic Registration
Criterion: register_agent_role('paper-reader') returns a member with name='PAPER_READER' and value='paper-reader'; AgentRole.PAPER_READER exists and AgentRole('paper-reader') works.
Command:
from factory.workflow.primitives import register_agent_role, AgentRole
m = register_agent_role('paper-reader')
print(f'name={m.name!r}, value={m.value!r}')
print(f'AgentRole.PAPER_READER is m: {AgentRole.PAPER_READER is m}')
print(f'AgentRole("paper-reader") is m: {AgentRole("paper-reader") is m}')Output:
name='PAPER_READER', value='paper-reader'
AgentRole.PAPER_READER is m: True
AgentRole("paper-reader") is m: True
Status: ✅ VERIFIED
Test 2: JSON Roundtrip (Core Bug Fix)
Criterion: A Workflow with an AgentNode using a registered plugin role can serialize via model_dump_json() and deserialize via model_validate_json() without ValidationError.
Command:
m = register_agent_role('paper-reader')
node = AgentNode(id='pr1', role=AgentRole.PAPER_READER, model='sonnet', prompt_template='read papers')
wf = Workflow(name='test-wf', nodes={'pr1': node}, edges=[], start_node='pr1')
json_str = wf.model_dump_json()
wf2 = Workflow.model_validate_json(json_str)
print(f'Role matches: {wf2.nodes["pr1"].role == AgentRole.PAPER_READER}')Output:
Serialized OK, length=360
Contains paper-reader: True
Deserialized OK, role=AgentRole.PAPER_READER
Role matches: True
Status: ✅ VERIFIED
Test 3: Idempotent Re-Registration
Criterion: Calling register_agent_role('paper-reader') twice returns the same member (identity check with is).
Command:
m1 = register_agent_role('paper-reader')
m2 = register_agent_role('paper-reader')
print(f'm1 is m2: {m1 is m2}')Output:
m1 is m2: True
Same name: True
Same value: True
Status: ✅ VERIFIED
Test 4: Collision with Builtin
Criterion: register_agent_role('builder') raises ValueError; register_agent_role('new-role', name='BUILDER') also raises ValueError.
Command:
try:
register_agent_role('builder')
except ValueError as e:
print(f'4a OK: {e}')
try:
register_agent_role('new-role', name='BUILDER')
except ValueError as e:
print(f'4b OK: {e}')Output:
4a OK: ValueError raised: agent role name 'BUILDER' already exists with value 'builder'
4b OK: ValueError raised: agent role name 'BUILDER' already exists with value 'builder'
Status: ✅ VERIFIED
Test 5: Collision with Plugin Role
Criterion: Register 'dup-role', then register_agent_role('dup-role', name='DIFFERENT_NAME') raises ValueError.
Command:
register_agent_role('dup-role')
try:
register_agent_role('dup-role', name='DIFFERENT_NAME')
except ValueError as e:
print(f'OK: {e}')Output:
OK: ValueError raised: agent role 'dup-role' is already registered with member name 'DUP_ROLE', not 'DIFFERENT_NAME'
Status: ✅ VERIFIED
Test 6: Invalid Inputs
Criterion: register_agent_role('') and register_agent_role('bad role!') both raise ValueError.
Command:
register_agent_role('') # → ValueError
register_agent_role('bad role!') # → ValueErrorOutput:
6a OK: ValueError for empty: agent role value must be a non-empty string
6b OK: ValueError for invalid: agent role 'bad role!' does not map to a valid enum member name ('BAD ROLE!')
Status: ✅ VERIFIED
Test 7: Plugin Registry Integration
Criterion: PluginRegistry.add_agent_roles(['test-agent']) adds 'test-agent' to registry.agent_roles, AgentRole('test-agent') works, and a Workflow with that role roundtrips through JSON.
Command:
from factory.plugins import PluginRegistry
reg = PluginRegistry()
reg.add_agent_roles(['test-agent'])
print(f'"test-agent" in agent_roles: {"test-agent" in reg.agent_roles}')
r = AgentRole('test-agent')
# ... build Workflow, serialize, deserialize ...Output:
agent_roles in registry: ['test-agent']
"test-agent" in agent_roles: True
AgentRole("test-agent"): AgentRole.TEST_AGENT
name=TEST_AGENT, value=test-agent
JSON roundtrip OK, role=test-agent
Status: ✅ VERIFIED
Test 8: Builtin Survival
Criterion: After registering a plugin role, builtin roles (BUILDER, RESEARCHER, CEO) still survive JSON roundtrips.
Command:
register_agent_role('custom-agent')
for builtin_role in [AgentRole.BUILDER, AgentRole.RESEARCHER, AgentRole.CEO]:
# build workflow, serialize, deserialize, assert matchOutput:
BUILDER: JSON roundtrip OK
RESEARCHER: JSON roundtrip OK
CEO: JSON roundtrip OK
Status: ✅ VERIFIED
Test 9: to_dict/from_dict Roundtrip
Criterion: A Workflow with a plugin role survives to_dict() → from_dict().
Command:
m = register_agent_role('dict-role')
node = AgentNode(id='dr1', role=m, model='sonnet', prompt_template='test')
wf = Workflow(name='dict-test', nodes={'dr1': node}, edges=[], start_node='dr1')
d = wf.to_dict()
wf2 = Workflow.from_dict(d)
print(f'Role matches: {wf2.nodes["dr1"].role == m}')Output:
to_dict OK, role value in dict: dict-role
from_dict OK, role=AgentRole.DICT_ROLE
Role value: dict-role
Role matches: True
Status: ✅ VERIFIED
Test 10: Cleanup Isolation
Criterion: A registered role can be manually cleaned up from the enum internals and no longer appears in list(AgentRole).
Command:
m = register_agent_role('ephemeral-role')
# manual cleanup of _member_map_, _value2member_map_, _member_names_, delattr
# verify gone from list(AgentRole)Output:
Before cleanup: EPHEMERAL_ROLE in members: True
After cleanup: EPHEMERAL_ROLE in members: False
After cleanup: hasattr EPHEMERAL_ROLE: False
EPHEMERAL_ROLE in list(AgentRole): False
Builtin count preserved: 3
Status: ✅ VERIFIED
Dedicated Test Suite
Command:
uv run pytest tests/test_plugin_agent_roles.py -v --tb=shortResult: 24 passed in 0.56s
Status: ✅ PASS
Summary
| # | Test | Status |
|---|---|---|
| 1 | Basic Registration | ✅ VERIFIED |
| 2 | JSON Roundtrip (core fix) | ✅ VERIFIED |
| 3 | Idempotent Re-Registration | ✅ VERIFIED |
| 4 | Collision with Builtin | ✅ VERIFIED |
| 5 | Collision with Plugin Role | ✅ VERIFIED |
| 6 | Invalid Inputs | ✅ VERIFIED |
| 7 | Plugin Registry Integration | ✅ VERIFIED |
| 8 | Builtin Survival | ✅ VERIFIED |
| 9 | to_dict/from_dict Roundtrip | ✅ VERIFIED |
| 10 | Cleanup Isolation | ✅ VERIFIED |
Adversarial Verdict: PASS
All 10 acceptance criteria are verified with evidence. The core claim — that plugin-registered roles survive Pydantic JSON serialization roundtrips — is confirmed. Edge cases (collisions, invalid input, idempotency, builtin survival, cleanup) all behave correctly. The feature works as advertised.
Posted by Factory CEO
|
The schema-rebuild fix is the important part here — the frozen-enum JSON-roundtrip failure is a nasty silent one, glad it's tackled head-on. One adjacent gap worth noting (possibly out of scope): Role registration makes a role valid, but not behaved. |
Review feedback on akashgit#1492: registering a role makes it valid, not behaved — a plugin role still needs its prompt resolvable somewhere, and a broken install (prompt data files missing from the built wheel) only surfaced as a bare FileNotFoundError deep into a CEO cycle. Two safety nets plus docs: - Load time: _warn_missing_role_prompts() runs after plugin registration and warns for any registered role with no user-global or factory-default prompt (a project override still satisfies the role; the warning names that escape hatch). - Invocation time: resolve_prompt's FileNotFoundError now adds a hint that the role is plugin-registered when it is, pointing at wheel data files as the likely cause. - docs/plugins.md documents the three-tier resolution contract for plugin roles and both safety nets. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
Thanks Shiv — took the "at least" version in fb36b95: the documented resolution contract plus warnings on both ends. The full "register a prompt alongside the role" API I've left as a possible follow-up, reasons below. What landed:
Why not the full prompt-registration API yet: the interesting design question is where a registered prompt would live in the resolution order. Writing to |
|
@ceo-review |
There was a problem hiding this comment.
✅ Factory Review: KEEP
Verdict: KEEP
Reason: QA: CLEAN — 5534 tests pass (29 PR-specific), composite 0.9255 (delta 0.000), lint clean, 0 new type errors, all 9 adversarial tests verified with evidence, JSON roundtrip bug fix confirmed
QA Analysis
Adversarial QA Report — PR #1492
Feature: Sanctioned AgentRole extension for plugin agent roles (register_agent_role())
Project type: Library
Date: 2026-09-10
Verdict: ✅ PASS
Smoke Test
Status: PASS
$ python -c "from factory.workflow.primitives import register_agent_role, AgentRole; print('import OK')"
import OK
Test Plan
Derived from PR scope: register_agent_role() must allow plugins to extend AgentRole at runtime such that (a) registration works with correct naming, (b) JSON/dict serialization roundtrips succeed (the core bug fix), (c) collisions are caught, (d) builtins are not broken, and (e) the plugin registry integration works end-to-end.
Feature Tests
1. REGISTRATION BASICS
Criterion: Import register_agent_role, register a custom role. Verify it appears in list(AgentRole), can be accessed as AgentRole.CUSTOM_NAME, and AgentRole('custom-value') works.
Status: ✅ VERIFIED
$ python -c "
from factory.workflow.primitives import register_agent_role, AgentRole
member = register_agent_role('paper-reader')
print(f'member.name = {member.name!r}')
print(f'member.value = {member.value!r}')
print(f'AgentRole.PAPER_READER is member: {AgentRole.PAPER_READER is member}')
print(f'AgentRole(\"paper-reader\") is member: {AgentRole(\"paper-reader\") is member}')
print(f'member in list(AgentRole): {member in list(AgentRole)}')
print(f'total members: {len(list(AgentRole))}')
"
member.name = 'PAPER_READER'
member.value = 'paper-reader'
AgentRole.PAPER_READER is member: True
AgentRole("paper-reader") is member: True
member in list(AgentRole): True
total members: 11
2. JSON ROUNDTRIP (core bug fix)
Criterion: Create a Workflow containing the registered role, serialize to JSON, deserialize back. Verify the roundtrip is clean.
Status: ✅ VERIFIED
$ python -c "
import json
from factory.workflow.primitives import register_agent_role, AgentRole, AgentNode, Workflow
member = register_agent_role('paper-reader')
wf = Workflow(name='test-wf', nodes={'a': AgentNode(id='a', role=member)}, edges=[], start_node='a')
json_str = wf.model_dump_json()
restored = Workflow.model_validate_json(json_str)
print(f'Deserialized role: {restored.nodes[\"a\"].role!r}')
print(f'Role is same object: {restored.nodes[\"a\"].role is member}')
d = wf.to_dict()
print(f'to_dict role value: {d[\"nodes\"][\"a\"][\"role\"]}')
restored2 = Workflow.from_dict(d)
print(f'from_dict role is member: {restored2.nodes[\"a\"].role is member}')
"
Deserialized role: <AgentRole.PAPER_READER: 'paper-reader'>
Role is same object: True
Role value matches: True
to_dict role value: paper-reader
from_dict role: <AgentRole.PAPER_READER: 'paper-reader'>
from_dict role is member: True
Both model_dump_json()→model_validate_json() and to_dict()→from_dict() roundtrips succeed. This is the core bug that the PR fixes (Pydantic's frozen schema rejecting dynamically-added enum members).
3. COLLISION DETECTION
Criterion: Registering 'builder' (builtin) raises ValueError with clear message. Registering same role twice with different names raises ValueError.
Status: ✅ VERIFIED
$ python -c "
from factory.workflow.primitives import register_agent_role
try:
register_agent_role('builder')
except ValueError as e:
print(f'builtin collision caught: {e}')
register_agent_role('dup-role', name='DUP_A')
try:
register_agent_role('dup-role', name='DUP_B')
except ValueError as e:
print(f'name mismatch caught: {e}')
try:
register_agent_role('totally-new', name='BUILDER')
except ValueError as e:
print(f'builtin name collision caught: {e}')
"
builtin collision caught: agent role name 'BUILDER' already exists with value 'builder'
name mismatch caught: agent role 'dup-role' is already registered with member name 'DUP_A', not 'DUP_B'
builtin name collision caught: agent role name 'BUILDER' already exists with value 'builder'
All three collision vectors properly detected with clear error messages.
4. IDEMPOTENCY
Criterion: Register same role twice with same args, verify same object returned.
Status: ✅ VERIFIED
$ python -c "
from factory.workflow.primitives import register_agent_role
first = register_agent_role('paper-reader')
second = register_agent_role('paper-reader')
print(f'first is second: {first is second}')
third = register_agent_role('x-role', name='X_ROLE')
fourth = register_agent_role('x-role', name='X_ROLE')
print(f'custom name idempotent: {third is fourth}')
"
first is second: True
custom name idempotent: True
5. PLUGIN REGISTRY INTEGRATION
Criterion: Create a PluginRegistry, call add_agent_roles(['test-role']), verify the role is graph-usable (create AgentNode with it).
Status: ✅ VERIFIED
$ python -c "
from factory.plugins import PluginRegistry
from factory.workflow.primitives import AgentRole, AgentNode, Workflow
registry = PluginRegistry()
registry.add_agent_roles(['test-role'])
print(f'test-role in registry.agent_roles: {\"test-role\" in registry.agent_roles}')
print(f'AgentRole.TEST_ROLE: {AgentRole.TEST_ROLE!r}')
node = AgentNode(id='t', role=AgentRole.TEST_ROLE)
print(f'AgentNode created with role: {node.role!r}')
wf = Workflow(name='t', nodes={'t': node}, edges=[], start_node='t')
restored = Workflow.model_validate_json(wf.model_dump_json())
print(f'Workflow roundtrip role: {restored.nodes[\"t\"].role!r}')
"
test-role in registry.agent_roles: True
AgentRole.TEST_ROLE: <AgentRole.TEST_ROLE: 'test-role'>
AgentNode created with role: <AgentRole.TEST_ROLE: 'test-role'>
Workflow roundtrip role: <AgentRole.TEST_ROLE: 'test-role'>
Full end-to-end: PluginRegistry → register_agent_role → AgentNode → Workflow → JSON roundtrip.
6. PROMPT WARNING
Criterion: Call _warn_missing_role_prompts with a registry that has a role with no prompt file. Verify warning is emitted.
Status: ✅ VERIFIED
$ python -c "
import structlog
from factory.plugins import PluginRegistry, _warn_missing_role_prompts
registry = PluginRegistry()
registry.add_agent_roles(['ghost-role'])
with structlog.testing.capture_logs() as logs:
_warn_missing_role_prompts(registry)
events = [e for e in logs if e['event'] == 'plugin_agent_role_prompt_missing']
print(f'Warning emitted: {len(events) > 0}')
print(f'Warning role: {events[0][\"role\"]}')
print(f'Has expected_user key: {\"expected_user\" in events[0]}')
print(f'Has hint key: {\"hint\" in events[0]}')
"
Warning emitted: True
Warning role: ghost-role
Has expected_user key: True
Has hint key: True
7. BUILTIN SURVIVAL
Criterion: After registering a plugin role, verify all 10 builtin roles still work in Workflow validation (schema rebuild must not break builtins).
Status: ✅ VERIFIED
$ python -c "
from factory.workflow.primitives import register_agent_role, AgentRole, AgentNode, Workflow
register_agent_role('plugin-role')
builtins = ['researcher', 'strategist', 'builder', 'health_checker', 'code_reviewer',
'adversarial_tester', 'failure_analyst', 'ceo', 'archivist', 'refiner']
all_ok = True
for role_val in builtins:
member = AgentRole(role_val)
node = AgentNode(id=f'n_{role_val}', role=member)
wf = Workflow(name='t', nodes={'n': node}, edges=[], start_node='n')
restored = Workflow.model_validate_json(wf.model_dump_json())
assert restored.nodes['n'].role.value == role_val
print(f'All builtins survived: {all_ok}')
"
All builtins survived: True
All 10 builtin roles roundtrip through JSON validation after a plugin role has been registered.
8. EDGE CASES
Criterion: Empty string, whitespace-only, invalid identifier all raise ValueError.
Status: ✅ VERIFIED
$ python -c "
from factory.workflow.primitives import register_agent_role
try: register_agent_role('')
except ValueError as e: print(f'empty string: {e}')
try: register_agent_role(' ')
except ValueError as e: print(f'whitespace only: {e}')
try: register_agent_role('bad role!')
except ValueError as e: print(f'invalid identifier: {e}')
r = register_agent_role('my-custom-role')
print(f'valid hyphenated: {r.name}={r.value}')
"
empty string: agent role value must be a non-empty string
whitespace only: agent role value must be a non-empty string
invalid identifier: agent role 'bad role!' does not map to a valid enum member name ('BAD ROLE!')
valid hyphenated: MY_CUSTOM_ROLE=my-custom-role
9. FIXTURE ISOLATION & FULL TEST SUITE
Criterion: Run the full test_plugin_agent_roles.py suite and verify all 29 tests pass without leaking state.
Status: ✅ VERIFIED
$ python -m pytest tests/test_plugin_agent_roles.py -v --tb=short
tests/test_plugin_agent_roles.py::TestRegisterAgentRole::test_returns_member_with_derived_name PASSED [ 3%]
tests/test_plugin_agent_roles.py::TestRegisterAgentRole::test_custom_member_name PASSED [ 6%]
tests/test_plugin_agent_roles.py::TestRegisterAgentRole::test_idempotent PASSED [ 10%]
tests/test_plugin_agent_roles.py::TestRegisterAgentRole::test_idempotent_with_same_custom_name PASSED [ 13%]
tests/test_plugin_agent_roles.py::TestRegisterAgentRole::test_builtin_value_collision_raises PASSED [ 17%]
tests/test_plugin_agent_roles.py::TestRegisterAgentRole::test_builtin_name_collision_via_custom_name_raises PASSED [ 20%]
tests/test_plugin_agent_roles.py::TestRegisterAgentRole::test_value_collision_with_plugin_role_raises PASSED [ 24%]
tests/test_plugin_agent_roles.py::TestRegisterAgentRole::test_name_mismatch_on_reregistration_raises PASSED [ 27%]
tests/test_plugin_agent_roles.py::TestRegisterAgentRole::test_invalid_identifier_raises PASSED [ 31%]
tests/test_plugin_agent_roles.py::TestRegisterAgentRole::test_empty_role_raises PASSED [ 34%]
tests/test_plugin_agent_roles.py::TestGraphUsage::test_agent_node_accepts_plugin_role PASSED [ 37%]
tests/test_plugin_agent_roles.py::TestGraphUsage::test_lax_validation_accepts_role_string PASSED [ 41%]
tests/test_plugin_agent_roles.py::TestGraphUsage::test_gate_node_accepts_plugin_role PASSED [ 44%]
tests/test_plugin_agent_roles.py::TestGraphUsage::test_agent_config_accepts_plugin_role PASSED [ 48%]
tests/test_plugin_agent_roles.py::TestGraphUsage::test_workflow_json_roundtrip PASSED [ 51%]
tests/test_plugin_agent_roles.py::TestGraphUsage::test_workflow_to_from_dict_roundtrip PASSED [ 55%]
tests/test_plugin_agent_roles.py::TestGraphUsage::test_gate_node_json_roundtrip PASSED [ 58%]
tests/test_plugin_agent_roles.py::TestGraphUsage::test_factory_container_roundtrip PASSED [ 62%]
tests/test_plugin_agent_roles.py::TestGraphUsage::test_builtin_roles_still_validate_after_registration PASSED [ 65%]
tests/test_plugin_agent_roles.py::TestSkillExportRendering::test_plugin_role_renders_agent_command PASSED [ 68%]
tests/test_plugin_agent_roles.py::TestPluginRegistryIntegration::test_add_agent_roles_registers_graph_usable_role PASSED [ 72%]
tests/test_plugin_agent_roles.py::TestPluginRegistryIntegration::test_add_agent_roles_builtin_collision_skipped PASSED [ 75%]
tests/test_plugin_agent_roles.py::TestPluginRegistryIntegration::test_add_agent_roles_duplicate_skipped PASSED [ 79%]
tests/test_plugin_agent_roles.py::TestPluginRegistryIntegration::test_add_agent_roles_invalid_role_skipped PASSED [ 82%]
tests/test_plugin_agent_roles.py::TestPromptResolutionContract::test_load_plugins_warns_for_role_without_prompt PASSED [ 86%]
tests/test_plugin_agent_roles.py::TestPromptResolutionContract::test_load_plugins_no_warning_when_user_prompt_exists PASSED [ 89%]
tests/test_plugin_agent_roles.py::TestPromptResolutionContract::test_no_roles_means_no_check_output PASSED [ 93%]
tests/test_plugin_agent_roles.py::TestPromptResolutionContract::test_resolve_prompt_error_mentions_plugin_registration PASSED [ 96%]
tests/test_plugin_agent_roles.py::TestPromptResolutionContract::test_resolve_prompt_error_plain_for_builtin_role PASSED [100%]
======================== 29 passed, 1 warning in 0.70s =========================
Post-suite state verification — no leaked plugin roles:
$ python -c "
from factory.workflow.primitives import AgentRole
members = [m.name for m in AgentRole]
print(f'Total members: {len(members)}')
print(f'Members: {members}')
"
Total members: 10
Members: ['RESEARCHER', 'STRATEGIST', 'BUILDER', 'HEALTH_CHECKER', 'CODE_REVIEWER', 'ADVERSARIAL_TESTER', 'FAILURE_ANALYST', 'CEO', 'ARCHIVIST', 'REFINER']
Bonus: Re-export from factory.workflow
Status: ✅ VERIFIED
$ python -c "
from factory.workflow import register_agent_role, AgentRole
m = register_agent_role('via-init')
print(f'Re-export works: {m.value == \"via-init\" and AgentRole.VIA_INIT is m}')
"
Re-export works: True
Acceptance Criteria Summary
| # | Criterion | Status |
|---|---|---|
| 1 | Registration basics (name derivation, membership, value lookup) | ✅ VERIFIED |
| 2 | JSON roundtrip (model_dump_json + model_validate_json, to_dict + from_dict) | ✅ VERIFIED |
| 3 | Collision detection (builtin value, builtin name, plugin name-mismatch) | ✅ VERIFIED |
| 4 | Idempotent re-registration (same args → same object) | ✅ VERIFIED |
| 5 | PluginRegistry.add_agent_roles() end-to-end integration | ✅ VERIFIED |
| 6 | Prompt warning for roles with no prompt file | ✅ VERIFIED |
| 7 | Builtin survival (all 10 builtins still roundtrip after plugin role) | ✅ VERIFIED |
| 8 | Edge cases (empty, whitespace, invalid identifier) | ✅ VERIFIED |
| 9 | Full test suite (29/29 pass, fixture isolation, no state leakage) | ✅ VERIFIED |
Adversarial Verdict: ✅ PASS
All 9 acceptance criteria verified with evidence. The core feature — runtime AgentRole extension with Pydantic schema rebuild for JSON roundtrip correctness — works exactly as designed. Collision detection, idempotency, edge-case validation, and test isolation are all solid.
Posted by Factory CEO
2 participants
Closes #1484.
What this does
Gives plugins a sanctioned way to extend the
AgentRoleenum, so plugin-defined roles finally work inside workflow graphs — the thingdocs/plugins.mdhas been promising all along ("plugin workflows can mix plugin-defined agents with built-in agents").The public API.
factory.workflow.primitives.register_agent_role("paper-reader")addsAgentRole.PAPER_READER(member name derived from the value, or given explicitly). The role is then valid inAgentNode.role,GateNode.evaluator_role, andAgentConfig.role. Registration is idempotent, and collisions raiseValueErrorwith a clear message: builtin value, builtin member name, value already taken, name already taken.Registry wiring.
PluginRegistry.add_agent_roles(["paper-reader"])now callsregister_agent_role(), so a plugin that registers roles for the CLI gets graph-usable roles for free — no enum mutation in the plugin'sregister(). The registry keeps its existing skip-with-warning behavior on builtin collisions (builtins always win), consistent withadd_commandsandadd_modes.The latent bug this fixes
Downstream plugins (we run one in production) work around the closed enum by inserting members into
AgentRole._member_map_/_value2member_map_at import time. That makes in-process construction work, but it hides a real bug: Pydantic freezes an enum's valid values into model core schemas at class-definition time. A dynamically added member serializes to JSON fine, butmodel_validate_jsonof a workflow containing it fails withInput should be 'researcher', 'strategist', ...— the frozen schema never heard of the new role. So ephemeral mode JSONs, outer-loop candidate serialization, anything that roundtrips workflow IR through JSON, silently breaks for plugin roles.I verified this empirically on
main: mutate the enum the way the workaround does,Workflow.model_validate_json(wf.model_dump_json())raises 59 validation errors. Afterregister_agent_role()rebuilds the schemas (it walks everyBaseModeldefined inprimitives.py, children before the containers that embed them), the same roundtrip is clean.Notes for reviewers
vars()ofprimitives.pyin definition order, which visits node classes beforeWorkflow/Factory— the order the nested schema graph needs._registered_plugin_roles(keyed by value) is what makes re-registration idempotent while a builtin collision still raises; without it,register_agent_role("builder")would happily returnAgentRole.BUILDERand look like success.AgentNode(role="paper-reader")) fails exactly as it does for builtins (AgentNode(role="builder")also raises in strict mode) — plugin roles now have parity with builtins everywhere builtins work: enum members in strict mode, strings in lax mode (from_dict), and strings in JSON.Testing
24 tests in
tests/test_plugin_agent_roles.py: registration semantics (derived/custom names, idempotency, all four collision cases, identifier validation), graph usage (AgentNode/GateNode/AgentConfig, JSON and to_dict/from_dict roundtrips, builtin roles still validating after a registration), skill export rendering (factory agent paper-readerappears in the generated SKILL.md), and registry integration (graph-usable roles viaadd_agent_roles, builtin-skip warning, duplicates, invalid roles). An autouse fixture snapshots and restores the enum between tests, since registration is process-global — without it roles would leak into the outer loop'srandom.choice(list(AgentRole))and friends. Full related suites (819 tests across workflow, skill, plugin, and outer_loop) pass, plus ruff and mypy clean.For midstream specifically:
ensure_cve_roles()inlightwell/workflows/_helpers.pycan be replaced by sevenregister_agent_role()calls (or just the existingadd_agent_roles()list, if it's already registered as a plugin), and the JSON roundtrip bug it was carrying goes away with it.🤖 Generated with Claude Code