feat(agents): resolve agent- and route-type registry package references in agents.<name> and routes.<name> #125
No reviewers
Labels
No labels
auto/blocked-by-deps
auto/ci-timeout
auto/claimed-implementer
auto/claimed-merge
auto/claimed-reviewer
auto/driver-down
auto/invariant-violation
auto/last-attempt-tier-0
auto/last-attempt-tier-1
auto/last-attempt-tier-2
auto/last-attempt-tier-min
Automation Tracking
auto/needs-conflict-resolution
auto/needs-implementer
auto/postmortem
auto/ready-to-merge
auto/restart-throttled
auto/revert
auto/sentinel
auto/stale-inactivity
auto/unstable
Blocked
Bounty
$100
Bounty
$1000
Bounty
$10000
Bounty
$20
Bounty
$2000
Bounty
$250
Bounty
$50
Bounty
$500
Bounty
$5000
Bounty
$750
MoSCoW
Could have
MoSCoW
Must have
MoSCoW
Should have
Needs Feedback
Points
1
Points
13
Points
2
Points
21
Points
3
Points
34
Points
5
Points
55
Points
8
Points
88
Priority
Backlog
Priority
CI Blocker
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Signed-off: Owner
Signed-off: Scrum Master
Signed-off: Tech Lead
Spike
State
Completed
State
Duplicate
State
In Progress
State
In Review
State
Paused
State
Unverified
State
Verified
State
Wont Do
Type
Automation
Type
Bug
Type
Discussion
Type
Documentation
Type
Epic
Type
Feature
Type
Legendary
Type
Refactor
Type
Support
Type
Task
Type
Testing
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Blocks
#121 Add support for resolving agent-type registry package references in agents.<name>
cleveragents/cleveractors-core
Reference
cleveragents/cleveractors-core!125
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feature/agent-package-references"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
An
agents.<name>entry, and likewise aroutes.<name>entry, may now be a bare package-reference string (local:<path>, ahost:namespace/name[@version]registry reference, or anID:pkg_<type>_<sha1>identifier — Package Registry Standard §5.3/§7.3) instead of a mapping, resolved to anagent-type orgraph-type package's content and used exactly as if that content had been written inline.Previously, a bare-string
agents.<name>value crashedcreate_executor/AgentFactory.create_agent/acreate_agentwith a rawAttributeError: 'str' object has no attribute 'get'instead of resolving through the registry. During implementation, the identical defect was confirmed inroutes.main(_execute_graph/_execute_graph_stream) — the Package Registry Standard definesgraph/streampackages the same way it definesagentpackages, so both gaps share one root cause and one fix shape. Per maintainer direction they are addressed together in this PR — see the updated issue #121 description anddocs/adr/ADR-2037-agent-and-route-package-reference-resolution.mdfor the full decision record.AgentReferenceResolver/RouteReferenceResolverclasses mirror the existingSkillReferenceResolverpattern (reusedPackageContentResolver/LocalPackageStore/RegistryClient, same two-tier cache, samelocal:/registry:/ID:parsing).AgentFactorygained an optionalagent_reference_resolverconstructor argument (mirrorsskill_loader); everyruntime_dispatchdispatch function shares the samePackageContentResolver/local_store/registry_api_keyalready used forskills:resolution.AgentCreationErrornaming the reference; an unresolvable route reference fails withConfigurationErrornaming the reference — neither surfaces as anAttributeError.routes.main, fixed topackage_type="graph"(create_executornever dispatches a"stream"-type route).docs/index.mdgains §4.1.1 and §5.1.1 (Version 1.5.0), documented generically since the Actor Configuration Standard is not scoped to this implementation.features/environment.py(a mocked resolver leaking between scenarios) and anagent_modules_steps.pyfixture that assumed any non-dict agent value was invalid — both surfaced by this change.Closes #121
Test plan
nox -s lint/nox -s format -- --check— cleannox -s typecheck— 0 errors (Pyright strict)nox -s security_scan/nox -s dead_code— cleannox -s complexity— cleannox -s unit_tests— all Behave scenarios pass, including newfeatures/agent_package_references.featureandfeatures/route_package_references.featurenox -s coverage_report— 96.6% (≥ 96.5% threshold)nox -s integration_tests— all 351 Robot Framework tests pass, including newrobot/agent_package_references.robotandrobot/route_package_references.robotnox -s build— wheel builds successfullyagents.<name>viacreate_executor) now resolves correctly instead of raisingAttributeError, via the new BDD/Robot scenarios exercising that exact code path end-to-end with a realLocalPackageStore🤖 Generated with Claude Code
PR Review: !125 (Ticket #121)
Verdict: Request Changes
The intended implementation for agent- and route-type package reference resolution is sound and well-tested, but the branch is currently based on an outdated
masterand would revert a recently-merged security fix and regression test if merged. This must be fixed before approval.Known follow-ups documented in the ADR (ReactiveConfigParser support,
stream-type route packages,registry_api_keyfor route references, and package-referencetemplate/agent_templatevalues) are treated as out-of-scope per the issue/ADR and are not repeated below.Critical Issues
Branch not rebased on current
master; merge would revert security fixes.feature/agent-package-referencescurrently branches from00e5482, whilemasterhas advanced tob98bcf7. The PR diff therefore shows the two intervening master commits as deletions:bb3e750— "symlink-safe sandbox file access closing TOCTOU window" (issue #105).b98bcf7— "capture LLM-agent file_write unsafe-mode regression" (issue #115).SymlinkSafeOpenerhardening, the associated BDD/Robot tests, the ADR-2035 D-9 decision, and the issue #115 regression test.origin/masterand force-push; do not merge until the diff againstmastercontains only the intended agent/route reference changes.src/cleveractors/agents/file_access.pyreverts to plainopen(), reintroducing CWE-367.SymlinkSafeOpenerclass) replace the component-wiseO_NOFOLLOWwalk with ordinaryopen(resolved, ...)calls.SymlinkSafeOpenerimplementation and_secure_openusage frommasterafter rebasing.Major Issues
CHANGELOG and ADR-2035 security documentation removed.
CHANGELOG.md: the entire "### Security" entry for issue #105 is deleted.docs/adr/ADR-2035-inline-code-sandboxed-file-access.md: the D-9 decision, Revision History, and consequences bullet about closing the TOCTOU window are deleted.masterafter rebasing.AgentFactory.get_agent_metadatadoes not handle bare-stringagents.<name>references.src/cleveractors/agents/factory.py, lines 650–651.agent_config = agents_config[agent_name]may now be astr. The next line callsagent_config.get("type", "llm"), which will raiseAttributeError: 'str' object has no attribute 'get'— the exact failure mode the acceptance criteria say must not happen.self._agent_reference_resolver(sync) or raisesAgentCreationErrornaming the reference, consistent withcreate_agent/acreate_agent.Tests for unrelated security fixes deleted.
features/symlink_safe_open.feature,features/steps/symlink_safe_open_steps.py,robot/symlink_safe_open.robot,robot/SymlinkSafeOpenTestLib.py,robot/inline_code_file_helpers.robot,robot/InlineFileHelperTestLib.py,features/llm_agent_tool_loop.feature,features/steps/llm_agent_tool_loop_steps.py.masterafter rebasing.Minor Issues
None.
Nits
None.
Summary
The agent- and route-reference resolver design mirrors the existing
SkillReferenceResolverpattern appropriately, the acceptance criteria forcreate_agent/acreate_agentand_execute_graph/_execute_graph_streamare addressed, and the new BDD/Robot coverage for the reference forms is comprehensive. However, the branch is currently unsafe to merge because it would revert the issue #105 symlink-safe hardening and the issue #115 regression test. Rebase onto currentmaster, restore the deleted security content, and fix theget_agent_metadataAttributeError before re-requesting review.7420b011acf01c08f859Response to @hurui200320's review (2026-08-10)
Thanks for the thorough review. I re-verified each point against
docs/index.md,docs/actor-registry-standard.md, issue #121, anddocs/adr/ADR-2037-agent-and-route-package-reference-resolution.mdbefore acting on any of it. Summary: 1 of 5 points was a real, confirmed defect and is now fixed; the other 4 were already resolved by the rebase you correctly called for, and are verified below rather than just asserted.Critical Issues
1. "Branch not rebased on current
master; merge would revert security fixes."✅ Confirmed and already fixed (branch has been rebased).
feature/agent-package-referencesnow sits on top ofb98bcf7(issue #115) withbb3e750(issue #105) in its ancestry —git logon the branch shows both commits below the feature commit.git diff master...HEAD --statfor the new head (f01c08f) touches only the 17 files this feature introduces/modifies (CHANGELOG.md,docs/index.md,docs/adr/ADR-2037-*.md, the new resolver modules, and the new/updated BDD+Robot test files) —src/cleveractors/agents/file_access.pydoes not appear in the diff at all, i.e. it is untouched and identical tomaster.2. "
src/cleveractors/agents/file_access.pyreverts to plainopen(), reintroducing CWE-367."✅ Confirmed and already fixed (side effect of the rebase).
grep -n "SymlinkSafeOpener\|_secure_open\|O_NOFOLLOW" src/cleveractors/agents/file_access.pyon the current head shows the class and all_secure_opencall sites intact and unchanged frommaster. As noted above, this file isn't part of the feature's diff at all now.Major Issues
1. "CHANGELOG and ADR-2035 security documentation removed."
✅ Confirmed and already fixed.
CHANGELOG.md's### Securityentry for issue #105 (theSymlinkSafeOpener/TOCTOU writeup) is present and unmodified.docs/adr/ADR-2035-inline-code-sandboxed-file-access.mdstill contains D-9, its Revision History entry, and the TOCTOU-closing consequence bullet — verified by grep, not just diff-stat absence.2. "
AgentFactory.get_agent_metadatadoes not handle bare-stringagents.<name>references."✅ Confirmed as a real defect — fixed in this push, not by the rebase. You were right:
get_agent_metadata(src/cleveractors/agents/factory.py) still didagent_config = agents_config[agent_name]; agent_config.get("type", "llm")unconditionally, so a bare-string reference would raise the exactAttributeErrorthe rest of this PR exists to eliminate — this one just wasn't caught because none of the original scenarios calledget_agent_metadataon a reference-valued agent.Fix mirrors
create_agent's existing pattern exactly (docs/adr/ADR-2037D-2/D-5):get_agent_metadatanow callsself._resolve_agent_reference(agent_name, agent_config)whenagent_configis astr, before reading.get("type", ...). An unresolvable reference now raisesAgentCreationErrornaming the reference, consistent with every other call site inAgentFactory.Added two new Behave scenarios to
features/agent_package_references.feature(get_agent_metadata resolves a local: agent reference into metadata, and the unresolvable-reference/AgentCreationErrorcounterpart), with step definitions infeatures/steps/agent_package_references_steps.py. Both pass, and I confirmed via a coverage run scoped to this feature's two.featurefiles that the newisinstance(agent_config, str)branch inget_agent_metadatais exercised (lines 651-652 no longer appear in Slipcover'smissing_linesforfactory.py).3. "Tests for unrelated security fixes deleted."
✅ Confirmed and already fixed (side effect of the rebase). All eight files you listed (
features/symlink_safe_open.feature,features/steps/symlink_safe_open_steps.py,robot/symlink_safe_open.robot,robot/SymlinkSafeOpenTestLib.py,robot/inline_code_file_helpers.robot,robot/InlineFileHelperTestLib.py,features/llm_agent_tool_loop.feature,features/steps/llm_agent_tool_loop_steps.py) are present on disk at the current head — checked individually, not just inferred from the diff.Minor Issues / Nits
None reported — nothing to address.
Verification performed before pushing
nox -s lint,nox -s format -- --check,nox -s typecheck,nox -s security_scan,nox -s dead_code— all clean.nox -s unit_tests(full suite): 153 features / 3057 scenarios / 14152 steps, 0 failed — confirms theget_agent_metadatafix doesn't regress any other caller (robot/EmailGraphLib.py,robot/lib/app_lib.py,features/steps/agent_modules_steps.pyall call it with dict-valued configs, which take the unchanged branch).nox -s coverage_reportscoped tofeatures/agent_package_references.featureandfeatures/route_package_references.feature(per this project's convention of measuring the coverage delta for the feature under review rather than re-running the whole-suite 97% gate on every iteration): confirms the newget_agent_metadatabranch is covered.Pushed as an amendment to the same commit (
f01c08f, force-pushed with--force-with-lease) rather than a new commit, since the issue's## Definition of Donecalls for one commit for this issue and the PR was not yet merged.Re-requesting review — please let me know if anything above needs a second look.
PR Review: !125 (Ticket #121)
Verdict: Approve
The branch has been correctly rebased on current
master(HEAD now sits onb98bcf7with the issue #105 and #115 security commits intact), and all items from the previous review cycle have been addressed. The implementation satisfies every acceptance criterion in issue #121, mirrors the establishedSkillReferenceResolverpattern appropriately, and includes comprehensive Behave and Robot coverage. No critical or major issues remain.Out-of-scope/deferred items documented in ADR-2037 (ReactiveConfigParser support,
stream-type route packages,registry_api_keyauthentication for route references, and package-referencetemplate/agent_templatevalues) are respected and not repeated below.Critical Issues
None.
Major Issues
None.
Minor Issues
None.
Nits
features/route_package_references.featureandrobot/route_package_references.robotexercise the newroutes.mainreference branch throughExecutor.execute()(_execute_graph), but there is no corresponding scenario forExecutor.execute_stream()(_execute_graph_stream). The two branches are structurally identical, but a single streaming scenario would close the coverage gap and guard against future divergence. Not a blocker.Summary
Agent reference resolution:
AgentReferenceResolvercorrectly parseslocal:,registry:, andID:references withpackage_type="agent", handles the unresolved/placeholder edge cases, and provides the required sync and async entry points.AgentFactory.create_agent/acreate_agent/validate_configuration/get_agent_metadataall now tolerate a bare-stringagents.<name>value and raiseAgentCreationError(neverAttributeError) on unresolvable references. Theruntime_dispatchfunctions share the samePackageContentResolveralready built forskills:resolution, solocal_store/registry_api_keyflow through as intended.Route reference resolution:
RouteReferenceResolvermirrors the agent resolver withpackage_type="graph", and_execute_graph/_execute_graph_streamnow detect a bare-stringroutes.main, resolve it, and reuse the resolvedmainmapping forparallel_execution— fixing the latent duplicateroutes.mainlookup identified in ADR-2037 D-9. Unresolvable route references raiseConfigurationErrornaming the reference.Tests and docs: The new Behave scenarios cover resolver schemes, malformed/empty/unresolved references, ID placeholders,
AgentFactorysync/async creation,get_agent_metadata, and end-to-end runtime execution. The Robot suites exercise the realLocalPackageStore→PackageContentResolver→ resolver →Executorpipeline for both agent and route references.docs/index.mdis updated to Version 1.5.0 with new §4.1.1 and §5.1.1 sections, and the CHANGELOG entry accurately summarizes the change.Approved for merge.
f01c08f8592bb2f7064f2bb2f7064fb9ee1c2b26b9ee1c2b26a02ecfa088a02ecfa088afef7d5f9b