fix(actor): Get responses from actor-run #10900
Merged
HAL9000
merged 8 commits from 2026-05-05 01:06:21 +00:00
bugfix/m3-actor-run-response into master
Labels
Clear labels
auto/needs-reevaluation
controller-managed
overdue
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
Needs Feedback
Signed-off: Owner
Signed-off: Scrum Master
Signed-off: Tech Lead
Spike
Controller deferred this PR; awaiting Phase 6+ scope-evaluator or operator re-enablement.
Auto-agents controller manages this PR/issue (see tools/controller/deploy/RUNBOOK.md). Remove this label to abandon controller management.
PR blocked by an open issue dependency. Operator must close the dep (or remove the dependency link) before the merge driver can act. Auto-cleared by merge_drive when no open deps remain.
Most recent merge cycle hit CI timeout. Driver excludes this PR while last merge_cycle row is < 30 min old; label persists thereafter as visible history.
Currently being processed by an implementer worker.
Currently being processed by the merge driver.
Currently being processed by a reviewer worker.
Merge driver heartbeat stale; pipeline halted. Closed automatically on next clean tick.
Detected master commit violating the strict merge invariant. Tracked as an issue (not a PR label); kept here for label completeness.
In-cycle escalation: most recent attempt ran at the Tier 0 slot (`tier-0`). Slot's model defined in .opencode/models/tiers.yaml.
In-cycle escalation: most recent attempt ran at the Tier 1 slot (`tier-1`). Slot's model defined in .opencode/models/tiers.yaml.
In-cycle escalation: most recent attempt ran at the Tier 2 slot (`tier-2`). Slot's model defined in .opencode/models/tiers.yaml. Gated behind IMPLEMENTER_ESCALATION_TIER2_ENABLED.
In-cycle escalation: most recent attempt ran at the Tier -1 slot (`tier-min`). Slot's model defined in .opencode/models/tiers.yaml. Suffix is ``-min`` (not ``--1``) so the Forgejo UI reads naturally.
Tracking issues used by the AI Automation system for agents to communicate and report.
Rebase conflict needs LLM conflict-resolver.
Failing CI needs implementer attention.
Documenting a driver incident or rollback.
Reviewer has APPROVED this PR and no later REQUEST_CHANGES is outstanding. The merge driver requires this label to even consider a PR for merging. Set by the reviewer worker on APPROVE; cleared on REQUEST_CHANGES.
Train repeatedly lost master-tempo races. Driver excludes via merge_cycle until cooldown elapses; label persists as visible history.
Revert PR backing out an invariant violation. Fast-tracked through the merge driver.
Sentinel PR duplicated from upstream into a personal fork by tools/duplicate_prs_to_fork.py for pipeline testing. Lives only in the fork; the canonical pipeline never sees it.
No implementer activity for N days. Flagged for human review. Auto-cleared on next push to head branch.
Repeatedly fails on current master (>= 3 ci-fail-on-rebased-sha releases in 12 h). Excluded from driver until human triage.
A ticket in a blocked state and unable to complete until some other task is completed first.
Bounty
$100
A bounty of $100 for any open-source contributor who provides a MR that solves this issue
Bounty
$1000
A bounty of $1000 for any open-source contributor who provides a MR that solves this issue
Bounty
$10000
A bounty of $10000 for any open-source contributor who provides a MR that solves this issue
Bounty
$20
A bounty of $20 for any open-source contributor who provides a MR that solves this issue
Bounty
$2000
A bounty of $2000 for any open-source contributor who provides a MR that solves this issue
Bounty
$250
A bounty of $250 for any open-source contributor who provides a MR that solves this issue
Bounty
$50
A bounty of $50 for any open-source contributor who provides a MR that solves this issue
Bounty
$500
A bounty of $500 for any open-source contributor who provides a MR that solves this issue
Bounty
$5000
A bounty of $5000 for any open-source contributor who provides a MR that solves this issue
Bounty
$750
A bounty of $750 for any open-source contributor who provides a MR that solves this issue
MoSCoW
Could have
Could have feature in order to satisfy the epic/legendary.
MoSCoW
Must have
Must have feature in order to satisfy the epic/legendary.
MoSCoW
Should have
Should have feature in order to satisfy the epic/legendary.
There are questions in the ticket that can not be completed until the project owner provides clarity.
Points
1
1 man-hours worth of work for an expert with no learning curve.
Points
13
13 man-hours worth of work for an expert with no learning curve.
Points
2
2 man-hours worth of work for an expert with no learning curve.
Points
21
21 man-hours worth of work for an expert with no learning curve.
Points
3
3 man-hours worth of work for an expert with no learning curve.
Points
34
34 man-hours worth of work for an expert with no learning curve.
Points
5
5 man-hours worth of work for an expert with no learning curve.
Points
55
55 man-hours worth of work for an expert with no learning curve.
Points
8
8 man-hours worth of work for an expert with no learning curve.
Points
88
88 man-hours worth of work for an expert with no learning curve.
Priority
Backlog
This ticket has backlogged priority and is not to be worked on yet
Priority
CI Blocker
Critical priority issue that blocks CI/CD pipeline and prevents PR merges
Priority
Critical
The priority is critical
Priority
High
The priority is high
Priority
Low
The priority is low
Priority
Medium
The priority is medium
When an epic or legendary is in review it must be signed off by owner, tech lead, and scrum master before being marked as completed.
When an epic or legendary is in review it must be signed off by owner, tech lead, and scrum master before being marked as completed.
When an epic or legendary is in review it must be signed off by owner, tech lead, and scrum master before being marked as completed.
A ticket for learning a tool or technology that is needed to be able to do future planning and design.
State
Completed
The ticket has been fully implemented, completed, and merged with the source code. This label should only be applied once a ticket is closed.
State
Duplicate
A ticket that represents the same content as an existing ticket.
State
In Progress
A ticket that is actively being developed.
State
In Review
A ticket that has had some code completed to implement but is waiting to pass peer review and is not yet merged in.
State
Paused
This ticket's work started but wasn't finished. It's on hold (likely in a feature branch) and will be resumed later, either due to a blocker or a delay.
State
Unverified
All new tickets start in this state. A developer may set it to show the ticket is unverified. This means we haven't agreed to work on it. It will either move to a verified state or be closed as wontdo.
State
Verified
The issue has been verified by a developer as legitimate. It will be worked on and verified tickets are now considered part of the backlog.
State
Wont Do
This ticket has been decided it wont be done. This may mean the bug has been determined to not be real (cant verify) or the feature is one we have decided we dont want to adopt.
Type
Automation
Any edits or discussion about the AI automated coding system.
Type
Bug
Something that doesnt work as intended.
Type
Discussion
Anytime a ticket represents a discussion about a subject and doesnt fall into one of the other categories.
Type
Documentation
An error or improvement needed in the documentation.
Type
Epic
Any first tier epic. That is, an epic which contains only issues as children and will not have sub-epics.
Type
Feature
Some new functionality not present.
Type
Legendary
A type of Epic which will contain other Epics.
Type
Refactor
A code change that restructures existing code without changing its external behavior.
Type
Support
Someone needs help using the project.
Type
Task
A generic task that doesnt fit into the other type categories.
Type
Testing
Work exclusively focusing on fixing or expanding testing.
No Label
Type
Bug
Projects
Clear projects
No project
Assignees
aditya (Aditya Chhabra)
aleenaumair (Aleena Umair)
brent.edwards (Brent Edwards)
CoreRasurae (Luis Mendes)
drew (Drew Morris)
eugen.thaci (Eugen Thaci)
freemo (Jeffrey Phillips Freeman)
HAL9000 (HAL 9000)
HAL9001 (HAL9001)
hamza.khyari (Hamza Khyari)
hurui200320 (Rui Hu)
justin.morris
khird (Kyle Hird)
org.cleveragents
Clear assignees
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: cleveragents/cleveragents-core#10900
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Delete Branch "bugfix/m3-actor-run-response"
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
resolve_config_filesincli/commands/_resolve_actor.pyto synthesise a v3type: llmYAML when a built-in actor has noyaml_textand itsconfig_blobhasproviderandmodelbut notypefield_synthesize_llm_yaml()helper that creates a minimal v3 LLM actor YAML from a built-in actor config blob@tdd_issue_10861) infeatures/tdd_actor_run_response.featurewith step definitionsRoot Cause
Built-in actors generated from the provider registry have a
config_blobwithproviderandmodelfields but notypefield. When this raw blob was serialised to YAML and fed toReactiveConfigParser, the parser found notype, noagents/actorsmap, and norouteskey — so it produced an emptyReactiveConfigwith no agents and no routes.run_single_shot()then returned""because there was nothing to execute.Fix
resolve_config_filesnow detects this case (provider + model present, no type) and synthesises a minimal v3type: llmYAML. TheReactiveConfigParser._build_from_v3()method then creates a reactive agent and a synthesised graph route, enabling the LLM to be invoked and its response returned to the caller.Closes #10861
This PR blocks issue #10861
Automated by CleverAgents Bot
Supervisor: Implementation | Agent: task-implementor
Review of PR #10900 -- fix(actor): Get responses from actor-run
Closes #10861
This PR fixes a real bug where
agents actor run <built-in-actor> <prompt>silently returned an empty string for built-in provider-registry actors. The root cause analysis in the PR body is accurate. The fix adds_build_v3_yaml_from_blob()which synthesises a minimaltype: llmYAML from the blob provider and model fields.10-Category Review
1. CORRECTNESS -- PASS
The fix correctly targets the exact condition: a config_blob that has provider and model but no type field. This isolates built-in provider-registry blobs without affecting custom actors that have yaml_text or type-bearing config_blobs. Edge cases handled: empty provider/model defaults to empty string via str(... or ), system_prompt conditionally included.
2. SPECIFICATION ALIGNMENT -- PASS
The fix aligns with the v3 specification: actors must have type: llm in their YAML config for ReactiveConfigParser._build_from_v3() to recognise them. The new synthesized YAML includes type, name, and model -- the minimum required fields.
3. TEST QUALITY -- PASS
4. TYPE SAFETY -- PASS
5. READABILITY -- PASS
6. PERFORMANCE -- PASS
7. SECURITY -- PASS
8. CODE STYLE -- PASS
9. DOCUMENTATION -- PASS
10. COMMIT AND PR QUALITY -- NEEDS ATTENTION
CI Status
The CI lint failure is blocking. All core checks (typecheck, security, unit_tests) passed.
Blocking Issues
Non-Blocking Suggestions
The @tdd_expected_fail tag removal was mentioned. If a companion TDD branch/issue existed, consider verifying it was properly cleaned up.
The issue subtask list mentioned a Robot Framework integration test. While not blocking, adding one would strengthen multi-level testing coverage.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
e5862e4785to38fa155e66Review submitted by automated PR review worker.
PR Review: fix(actor): Get responses from actor-run
Reviewed against 10-category checklist. Note: All Forgejo list-read API endpoints (PR details, linked issues, review comments, PR comments) returned 404 during this session, so I could not verify previous feedback items or PR metadata (linked issues, labels, milestone, dependency direction). This assessment is based on the code diff and commit content only.
1. CORRECTNESS ✅
The fix is sound. When a built-in LLM actor has a
config_blobwithproviderandmodelbut notypefield,_synthesize_llm_yaml()constructs a proper v3type: llmYAML blob. This allowsReactiveConfigParserto create agents and routes, resolving the root cause of bug #10861. Edge cases handled: missing/system_prompt falsy → skipped; empty provider/model → falls through to originalyaml.safe_dumppath.2. SPECIFICATION ALIGNMENT ✅
The synthesized YAML follows the v3
type: llmformat (name,type: llm,provider,model, optionalsystem_prompt) — consistent with the project specification. The existingyaml_textpath is untouched.3. TEST QUALITY ✅
Three Behave BDD scenarios in
features/tdd_actor_run_response.feature:type: llmand provider/model in outputyaml_textare unaffectedAll have
@tdd_issue @tdd_issue_10861tags. Well-named scenario descriptions.Suggestion: Add a TDD scenario covering the case where
config_blobhassystem_prompt— the code adds it to the synthesized YAML (line 83-84), so testing this path would strengthen coverage.4. TYPE SAFETY ✅
from typing import Anycorrectly added._synthesize_llm_yamlsignature is fully annotated:(actor_name: str, config_blob: dict[str, Any]) -> str. No new# type: ignorecomments introduced. Pre-existing# type: ignore[import-untyped]on behave imports is an established project pattern.5. READABILITY ✅
Excellent docstring on
_synthesize_llm_yamlexplaining the bug, root cause, fix, args, and return. Variable names are clear. Logic flow inresolve_config_filesis easy to follow with the new conditional block.Suggestion: Step descriptions have repetitive
for tdd-10861suffixes (e.g.,Given a built-in actor with provider and model but no type field for tdd-10861). Thetdd-10861qualifier is unnecessary in the step text since the feature file title already identifies the bug. Clean names likeGiven a built-in actor with provider and model but no type fieldwould improve readability as living documentation.6. PERFORMANCE ✅
New function is trivial — no performance concerns.
7. SECURITY ✅
No hardcoded secrets, tokens, or credentials. Input values (provider, model) are safely embedded in YAML.
_sanitize_namestill handles terminal escape injection. No SQL/command injection vectors.8. CODE STYLE ⚠️
SOLID principles followed — SRP maintained. Files under 500 lines.
Non-blocking: Double blank-lines detected — two extra blank lines appear at lines 54-55 (between
_sanitize_nameand_synthesize_llm_yaml) and at lines 33-34 (before imports of typing). These would triggernox -s format(ruff format). Runnox -s formatto auto-fix. Minor:str(config_blob.get("provider") or "")— thestr()wrapper is redundant since.get()already returns the value as-is.config_blob.get("provider", "") or ""is slightly cleaner.9. DOCUMENTATION ✅
_synthesize_llm_yamlhas a detailed docstring. Feature file includes context about the bug and fix in the description. Commit body explains motivation clearly.10. COMMIT AND PR QUALITY ⚠️
fix(actor): Get responses from actor-run(Conventional Changelog format)ISSUES CLOSED: #10861footer presentbugfix/m3-actor-run-responsematches naming conventiongit diff master...HEAD -- CHANGELOG.mdshows empty output. Per PR requirement #7, one entry must be added per commit.Verdict: COMMENT
All code quality categories pass. The implementation correctly fixes bug #10861 with appropriate tests. I recommend addressing the non-blocking items before merge:
CHANGELOG.mdwith an entry for this bugfixnox -s formatto fix double blank-linesfor tdd-10861suffixes from Gherkin step namesAutomated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Re-Review of PR #10900 -- fix(actor): Get responses from actor-run (Closes #10861)
This is the second review. The PR previously received REQUEST_CHANGES feedback from review #6971 with 3 blocking issues. A second COMMENT review (#6997) also submitted non-blocking suggestions. I am re-evaluating whether the prior feedback was adequately addressed.
Previous Feedback Review
From REQUEST_CHANGES Review (#6971)
CI lint must pass — NOT ADDRESSED. The lint check is STILL FAILING. The same double-blank-line formatting issues remain in
_resolve_actor.py. Runnox -s formatto auto-fix.Missing Type/ label — NOT ADDRESSED. The PR still has zero labels (
labels: []). Per PR submission requirements (Requirement #12: Milestone and Type Label), exactly one Type/ label is mandatory. For a bug fix, this must beType/Bug.Missing State/In review label — NOT ADDRESSED. The PR workflow expects State/In review to be present on submitted PRs.
From COMMENT Review (#6997)
CHANGELOG.md not updated — NOT ADDRESSED. The PR body references CHANGELOG but
git diff master...HEAD -- CHANGELOG.mdshows no changes. Per PR Requirement #7, the CHANGELOG must be updated with one entry per commit.Repetitive
for tdd-10861suffixes in Gherkin step names — Still present (suggestion, non-blocking).Full 10-Category Review
1. CORRECTNESS -- PASS
The fix correctly identifies the condition where
config_blobhasproviderandmodelbut notype, and synthesizes a minimal v3type: llmYAML. Edge cases handled: missing system_prompt (conditionally included), empty provider/model (defaults to empty string viaor "").2. SPECIFICATION ALIGNMENT -- PASS
The synthesized YAML follows the v3
type: llmformat (name, type, description, provider, model, optional system_prompt) which is consistent with the project specification.3. TEST QUALITY -- PASS
Three Behave BDD scenarios covering synthesis, parseability by ReactiveConfigParser, and non-interference with existing yaml_text actors.
@tdd_issue @tdd_issue_10861tags present. Step definitions comprehensive at 148 lines with proper cleanup.4. TYPE SAFETY -- PASS
from typing import Anyadded. Function signature fully annotated. No new# type: ignorecomments.5. READABILITY -- PASS
Excellent docstring on
_synthesize_llm_yaml. Clear condition check (has_provider_modelpattern via inline if). Code flow easy to follow.6. PERFORMANCE -- PASS
One-time YAML construction per actor. No performance concerns.
7. SECURITY -- PASS
No hardcoded secrets. System_prompt passed through from provider-registry config_blob, not user input. No injection vectors.
8. CODE STYLE -- NEEDS ATTENTION
Double blank-lines in
_resolve_actor.py:_sanitize_nameand_synthesize_llm_yaml: an extra blank line (ruff E303)_synthesize_llm_yamlandresolve_config_files: an extra blank line (ruff E303)Run
nox -s formatto auto-fix.Minor suggestion:
str(config_blob.get("provider") or "")— thestr()wrapper is redundant.config_blob.get("provider", "")is cleaner and equivalent.9. DOCUMENTATION -- PASS
Comprehensive docstrings on
_synthesize_llm_yaml(Args/Returns). Feature file has clear descriptions.10. COMMIT AND PR QUALITY -- NEEDS ATTENTION
ISSUES CLOSED: #10861footerType/Bug(mandatory)CI Status
_resolve_actor.py)Verdict: REQUEST_CHANGES
All 3 previously-requested items remain unaddressed, plus the CHANGELOG issue from the second review. The code change itself is sound — it correctly fixes the root cause of bug #10861.
Blocking Issues (must fix before approval)
_resolve_actor.pybetween functions. Runnox -s formatto auto-fix.Type/Buglabel to the PR. The emptylabels: []array means the PR does not meet PR Requirement #12.State/In reviewlabel.Non-Blocking Suggestions
for tdd-10861suffixes. Clean names likeGiven a built-in actor with provider and model but no type fieldwould be more readable as living documentation.config_blob.get("provider", "")instead ofstr(config_blob.get("provider") or "")for slightly cleaner code.Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
@@ -52,6 +53,44 @@ def _sanitize_name(name: str) -> str:return "".join(c for c in name if c.isprintable())Double blank-line before function definition. This triggers ruff E303 (expected 2 blank lines, found 3). Run
nox -s formatto auto-fix.Double blank-line after function end, before next function. This triggers ruff E303. Run
nox -s formatto auto-fix.PR re-review submitted with REQUEST_CHANGES status (review ID: 7025).
All previously-requested items remain unaddressed. The author must fix lint formatting, add the required Type/Bug and State/In review labels, and update CHANGELOG.md before this PR can be approved.
PR Fix Attempt — Tier 3: sonnet — Success
Changes Made
Fixed the CI lint failure that was blocking this PR.
Root Cause: The
ruff formatcheck was failing because:src/cleveragents/cli/commands/_resolve_actor.pyhad two extra blank lines between_sanitize_name()and_synthesize_llm_yaml()features/steps/tdd_actor_run_response_steps.pyhad a long string literal that needed to be wrappedFix: Applied
nox -s formatto auto-format both files. Committed asstyle(actor): fix ruff format violations in actor-run fix(commit0b2c32cc).Reviewer Feedback Addressed
Type/Buglabel)Quality Gates
Automated by CleverAgents Bot
Supervisor: Implementation | Agent: task-implementor
Re-Review of PR #10900 -- fix(actor): Get responses from actor-run (Closes #10861)
Previous Feedback Review
From REQUEST_CHANGES Review (#6971)
From COMMENT Review (#7025)
38fa155eor0b2c32cc) modifies CHANGELOG.md. See inline comments below.Previous Review #7025 Re-Evaluation
Review #7025 (also REQUEST_CHANGES) listed 4 blocking issues:
0b2c32cc)Full 10-Category Review
1. CORRECTNESS -- PASS
The fix correctly identifies the condition where config_blob has provider and model but no type, and synthesizes a proper v3 type: llm YAML. Edge cases handled correctly: system_prompt conditionally included only if truthy, empty provider/model defaults to empty string via
or "". The ReactiveConfigParser parseability test confirms the synthesized output is usable by downstream code.2. SPECIFICATION ALIGNMENT -- PASS
The synthesized YAML follows the v3 type: llm format (name, type, description, provider, model, optional system_prompt). This is consistent with the project specification for v3 actors defined in docs/specification.md.
3. TEST QUALITY -- PASS
Three Behave BDD scenarios in features/tdd_actor_run_response.feature with @tdd_issue @tdd_issue_10861 tags:
All scenarios have @mock_only tags. Step file is 148 lines with clear mock setup and cleanup via context.add_cleanup().
Non-blocking note: Issue subtask mentioned a Robot Framework integration test; not yet added but does not block.
4. TYPE SAFETY -- PASS
from typing import Anycorrectly added to imports. _synthesize_llm_yaml signature fully annotated:(actor_name: str, config_blob: dict[str, Any]) -> str. No new# type: ignorecomments introduced. Pre-existing behaves import ignores are an established project pattern.5. READABILITY -- PASS
Excellent docstring on _synthesize_llm_yaml explaining the bug, root cause, fix, Args/Returns sections. The inline condition
if config_blob.get("provider") and config_blob.get("model") and not config_blob.get("type")clearly expresses the intent. Code flow in resolve_config_files is easy to follow.6. PERFORMANCE -- PASS
One-time YAML construction per actor resolution. Minimal overhead: dict construction + yaml.safe_dump call. No loop or N+1 concerns. No scalability issues.
7. SECURITY -- PASS
No hardcoded secrets, tokens, or credentials. system_prompt passed through from provider-registry config_blob, not from user input. No SQL/command injection or path traversal vectors. _sanitize_name still handles terminal escape injection as before.
8. CODE STYLE -- PASS
Follows ruff conventions (CI lint now green after format fix in commit
0b2c32cc). Proper docstrings on both new and updated functions. File well under 500 lines. Single Responsibility: YAML synthesis logic cleanly isolated in dedicated function.9. DOCUMENTATION -- PASS
New function _synthesize_llm_yaml has comprehensive docstring with Args/Returns. Updated resolve_config_files docstring documents the new behavioral branch. PR body has detailed Summary, Root Cause, Fix sections.
10. COMMIT AND PR QUALITY -- NEEDS ATTENTION
38fa155e): Atomic, Conventional Changelog formatfix(actor): Get responses from actor-run, footerISSUES CLOSED: #108610b2c32cc): Atomic format fixstyle(actor): fix ruff format violations in actor-run fix, footerISSUES CLOSED: #10861bugfix/m3-actor-run-responsematches naming convention for bugfix in milestone m3CI Status
Verdict: REQUEST_CHANGES
All code quality categories pass. The implementation correctly fixes bug #10861 with appropriate tests, correct type annotations, and good documentation. All previously-requested items from review #6971 (CI lint, Type/Bug label) have been addressed. The CHANGELOG issue from review #7025 remains outstanding.
Blocking Issue (must fix before approval)
38fa155eand0b2c32cc), so two CHANGELOG entries are needed. Add both entries in a follow-up commit to this PR.Non-Blocking Suggestions
for tdd-10861suffixes (e.g.,Given a built-in actor with provider and model but no type field for tdd-10861). Clean names likeGiven a built-in actor with provider and model but no type fieldwould be more readable as living documentation. Consider a follow-up commit to clean these up.str(config_blob.get("provider") or "")-- thestr()wrapper is redundant.config_blob.get("provider", "") or ""is slightly cleaner and equivalent.Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Re-Review of PR #10900 -- fix(actor): Get responses from actor-run (Closes #10861)
Previous Feedback Review
From REQUEST_CHANGES Review (#7055)
Type/Buglabel.b9eebb6cadds a comprehensive 14-line CHANGELOG entry referencing #10861.From REQUEST_CHANGES Review (#6971)
Full 10-Category Review
1. CORRECTNESS -- PASS
The fix correctly identifies the exact condition:
config_blobhasproviderandmodelbut notype. The conditional guard properly isolates built-in provider-registry blobs without affecting custom actors that haveyaml_textor an existingtypefield. Edge cases handled: missingsystem_prompt(conditionally included), emptyprovider/model(defaults to empty string).2. SPECIFICATION ALIGNMENT -- PASS
The synthesized YAML follows the v3
type: llmformat withname,type,description,provider, andmodel-- the minimum required fields. Consistent with the project specification for v3 actors.3. TEST QUALITY -- PASS
Three Behave BDD scenarios with
@tdd_issue @tdd_issue_10861tags:yaml_textactors (non-interference)Step file is 150 lines with clear mock setup, cleanup via
context.add_cleanup(), and assertive error messages.4. TYPE SAFETY -- PASS
from typing import Anycorrectly added._synthesize_llm_yamlsignature fully annotated. No new# type: ignorecomments.5. READABILITY -- PASS
Excellent docstring on
_synthesize_llm_yamlexplaining the bug, root cause, fix, Args/Returns. Inline condition and code branch are self-explanatory.6. PERFORMANCE -- PASS
One-time YAML construction per actor resolution. Minimal overhead. No loop or N+1 concerns.
7. SECURITY -- PASS
No hardcoded secrets, tokens, or credentials.
system_promptpassed through from provider-registry config_blob, not user input. No injection vectors.8. CODE STYLE -- PASS
CI lint passing (all checks green). Proper docstrings. File well under 500 lines. SRP maintained.
9. DOCUMENTATION -- PASS
Comprehensive docstrings on
_synthesize_llm_yaml. Updatedresolve_config_filesdocs. CHANGELOG entry is detailed, 14-line, references #10861. PR body has detailed Summary, Root Cause, Fix sections.10. COMMIT AND PR QUALITY -- PASS
ISSUES CLOSED: #10861footer; changelog commit references#10861in titleISSUES CLOSED:footer (issue still referenced in title)CI Status
All 14 CI checks PASSING:
Verdict: APPROVED
All previously-requested items from reviews #6971 and #7025 have been fully addressed. The implementation correctly fixes bug #10861 with appropriate tests, correct type annotations, comprehensive documentation, and all CI checks passing.
Non-Blocking Observations
str(config_blob.get("provider") or "")-- thestr()wrapper is redundant;config_blob.get("provider", "") or ""is slightly cleaner.for tdd-10861suffixes. Clean step names without the qualifier would improve readability as living documentation.Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Review of PR #10900 -- fix(actor): Get responses from actor-run (Closes #10861)
This is a fresh evaluation of PR #10900. The PR fixes bug #10861 where
agents actor run <built-in-actor>silently returned empty string.Previous Review Assessment
Review #7072 (APPROVED) identified all prior feedback items as addressed:
0b2c32cc)b9eebb6c, 13-line entry)I independently verify all these items remain addressed and found no new issues.
10-Category Review
1. CORRECTNESS - PASS
The fix correctly targets the exact condition: config_blob with provider+model but no type.
_synthesize_llm_yaml()constructs a proper v3type: llmYAML. Edge cases handled well — empty provider/model defaults viastr(... or ""), system_prompt conditionally included, and fallback toyaml.safe_dumpfor non-built-in actors preserves original behavior.2. SPECIFICATION ALIGNMENT - PASS
The synthesized YAML follows v3
type: llmformat (name, type, provider, model, optional system_prompt), matchingReactiveConfigParser._build_from_v3()expectations. Module docstring notes accepted spec deviation for --config option.3. TEST QUALITY - PASS
Three Behave BDD scenarios with
@tdd_issue @tdd_issue_10861tags:Mock setup is clean. Non-blocking suggestion: add a scenario for system_prompt path (lines 83-84 of _resolve_actor.py) to strengthen coverage.
4. TYPE SAFETY - PASS
from typing import Anyimported._synthesize_llm_yamlfully annotated(actor_name: str, config_blob: dict[str, Any]) -> str. No new# type: ignore. Behave# type: ignore[import-untyped]is established project pattern.5. READABILITY - PASS
Excellent docstring on
_synthesize_llm_yamlexplaining bug, root cause, args, return. Clear variable names. Clean control flow inresolve_config_files. Non-blocking suggestions: (1) Step descriptions have repetitivefor tdd-10861suffixes — cleaner names would improve living documentation quality; (2) Assertion error messages use string concatenation instead of f-strings.6. PERFORMANCE - PASS
New function: simple dict construction + YAML serialization. Trivial overhead. No new loops or N+1 patterns.
7. SECURITY - PASS
No hardcoded secrets or credentials.
_sanitize_nameprevents terminal escape injection.yaml.safe_dumpused throughout (not unsafeyaml.dump). System prompt from config_blob embedded safely.8. CODE STYLE - PASS
Ruff format violations addressed by commit
0b2c32cc. Files well under 500 lines. SRP maintained. Minor style note:str(config_blob.get("provider") or "")is slightly redundant since .get() already returns the value type —config_blob.get("provider", "") or ""would be cleaner.9. DOCUMENTATION - PASS
Detailed docstring on
_synthesize_llm_yaml. Feature file description provides full bug context. CHANGELOG entry is comprehensive (14 lines with root cause, affected files, fix details, test coverage note).10. COMMIT AND PR QUALITY - PASS
bugfix/m3-actor-run-responsematches conventionVerdict: APPROVED
All 10 checklist categories pass. The implementation correctly and minimally fixes bug #10861. The regression tests are well-structured and the fix is conservative (does not affect existing yaml_text actors).
Non-blocking suggestions (no action required):
for tdd-10861suffixes from Gherkin step descriptions for cleaner living documentation@@ -0,0 +15,4 @@from behave.runner import Context # type: ignore[import-untyped]from cleveragents.cli.commands._resolve_actor import resolve_config_filesSuggestion: Step descriptions have repetitive
for tdd-10861suffixes that degrade readability as living documentation. Since the feature file title already identifies bug #10861, consider cleaner names likeGiven a built-in actor with provider and model but no type fieldinstead ofGiven a built-in actor with provider and model but no type field for tdd-10861.@@ -0,0 +102,4 @@@then("the resulting YAML file contains the provider and model for tdd-10861")def step_then_yaml_contains_provider_model(context: Context) -> None:"""Assert the synthesised YAML contains the correct provider and model."""Suggestion: Assertion error messages use string concatenation (lines ~101, 106, 113). Consider using f-strings for cleaner presentation, e.g.
f"Expected type: llm, got: {parsed.get("type")}".@@ -55,0 +69,4 @@Returns:A YAML string in v3 type: llm format that the reactive configparser can consume to produce a working single-agent graph route.Minor:
str(config_blob.get("provider") or "")is slightly redundant since.get()returns the existing type. Considerconfig_blob.get("provider", "") or ""for slightly cleaner code.Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
@HAL9000 PR #10930 fixed 10861 as a side effect. Before merging this PR, please compare this PR with 10930 and see if this one is not necessary anymore. If so, you should close this PR.
Review of PR #10900 (branch bugfix/m3-actor-run-response, head
b9eebb6c)OK
10-Category Review
✅ 1. CORRECTNESS
The fix correctly identifies the root cause: built-in LLM actors have a config_blob with provider and model but no type field. When deserialized as raw YAML and passed to ReactiveConfigParser, the absence of a type key (and no agents/routes keys) produces an empty ReactiveConfig. The fix intercepts this condition with a targeted guard (if config_blob.get(provider) and config_blob.get(model) and not config_blob.get(type)) and routes it to the new _synthesize_llm_yaml() helper. Logic is precise — only activates for built-in actors, not for actors with existing yaml_text or type fields.
✅ 2. SPECIFICATION ALIGNMENT
The fix follows the spec-defined v3 format for LLM actors (name, type, provider, model, optional system_prompt). The synthesized YAML includes type: llm, provider, model, and name — matching the expected v3 actor structure.
✅ 3. TEST QUALITY
Three Behave BDD scenarios cover all cases:
The ReactiveConfigParser validation in the parseability scenario is excellent — ensures end-to-end correctness.
✅ 4. TYPE SAFETY
The production code has proper type annotations: from typing import Any is imported, _synthesize_llm_yaml is fully typed with (actor_name: str, config_blob: dict[str, Any]) -> str, all variable assignments are type-safe, no type: ignore in production code.
✅ 5. READABILITY
_code is clear and well-structured:
Suggestion: The inline comment block could be slightly more concise.
✅ 6. PERFORMANCE
No concerns. The fix adds negligible overhead — one additional dict .get() check and potentially one yaml.safe_dump with a smaller dict.
✅ 7. SECURITY
No security concerns. No hardcoded secrets or credentials. yaml.safe_dump is correct. _sanitize_name is pre-existing and handles terminal escape injection.
✅ 8. CODE STYLE
Files are well under 500 lines. New function follows SOLID (single responsibility). Ruff-compatible (PR includes a ruff format fix commit). Imports follow project conventions.
✅ 9. DOCUMENTATION
_synthesize_llm_yaml has a comprehensive docstring. resolve_config_files docstring remains complete. CHANGELOG.md updated. Test feature file serves as living documentation.
✅ 10. COMMIT AND PR QUALITY
Non-Blocking Findings
Conclusion
This is a clean, well-scoped bug fix for a critical issue (#10861 — agents actor run returning nothing for built-in LLM actors). The fix is correct, well-tested, properly typed, and well-documented. CI is passing. The only concern is merge conflicts which should be resolved before merge.
Verdict: APPROVED with minor suggestion (pending conflict resolution).
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
@@ -112,2 +142,2 @@)raise typer.Exit(code=2) from None# Fix #10861: built-in actors have a config_blob with provider# and model but no type field. Serialising this blob as-isSuggestion: The inline comment block explaining the fix could be slightly more concise:
Fix #10861: Synthesize v3 YAML for built-in actors without a type field.instead of the current multi-line comment. Minor readability improvement only.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
First Review of PR #10900 -- fix(actor): Get responses from actor-run (Closes #10861)
Summary
Reviewed the full diff, linked issue #10861, PR description, CI status, and all 4 changed files.
CI Status
All 14 CI checks passing (commit
b9eebb6c): lint, build, quality, typecheck, security, push-validation, helm, integration_tests, e2e_tests, unit_tests, docker, coverage, status-check.10-Category Review
1. CORRECTNESS ✅
The fix correctly addresses the root cause of bug #10861. When a built-in LLM actor has a config_blob with provider and model but no type field, _synthesize_llm_yaml() constructs a proper v3 type: llm YAML blob containing name, type, provider, model, and optional system_prompt. This allows ReactiveConfigParser to create agents and routes, resolving the empty response issue. Edge cases are properly handled: missing system_prompt is skipped; empty provider/model falls through to original yaml.safe_dump path.
2. SPECIFICATION ALIGNMENT ✅
The synthesized YAML follows the v3 type: llm format (name, type: llm, provider, model, optional system_prompt, optional description) -- consistent with the project specification. The existing yaml_text path is untouched.
3. TEST QUALITY ✅
Three Behave BDD scenarios in features/tdd_actor_run_response.feature with @tdd_issue @tdd_issue_10861 tags:
Step definitions file (tdd_actor_run_response_steps.py) at 150 lines is well-structured with clear isolation via MagicMock and patch. Good use of temp file cleanup via add_cleanup.
4. TYPE SAFETY ✅
from typing import Any correctly added. _synthesize_llm_yaml signature is fully annotated: (actor_name: str, config_blob: dict[str, Any]) -> str. No new # type: ignore comments introduced. Pre-existing # type: ignore[import-untyped] on behave imports is an established project pattern.
5. READABILITY ✅
Excellent docstring on _synthesize_llm_yaml explaining the bug, root cause, fix, args, and return. Variable names are clear (v3_blob, provider, model, system_prompt). Logic flow in resolve_config_files is easy to follow with the new conditional block properly separated from the original error-handling path.
6. PERFORMANCE ✅
New function is trivial -- no performance concerns.
7. SECURITY ✅
No hardcoded secrets, tokens, or credentials. Input values (provider, model) are safely embedded in YAML via yaml.safe_dump. _sanitize_name still handles terminal escape injection correctly. No SQL/command injection vectors.
8. CODE STYLE ✅
SOLID principles followed -- SRP maintained. _synthesize_llm_yaml is a focused single-responsibility helper. All files well under 500 lines (_resolve_actor.py: 172 lines, step file: 150 lines). Follows ruff conventions (lint check passed).
9. DOCUMENTATION ✅
_synthesize_llm_yaml has a detailed docstring explaining the bug context, root cause, and fix. Feature file includes rich context about the bug and fix in its description. Changelog entry is thorough. Commit body explains motivation clearly.
10. COMMIT AND PR QUALITY ✅
Non-blocking Suggestions
Redundant str() wrapper (line ~89 of _resolve_actor.py): Consider using config_blob.get("provider", "") or "" instead of str(config_blob.get("provider") or "") -- the str() wrapper is redundant since .get() already returns the value as-is.
Repetitive TDD suffixes in Gherkin step descriptions: Step descriptions have repetitive for tdd-10861 suffixes. Since the feature file title already identifies the bug, clean names like Given a built-in actor with provider and model but no type field would improve readability as living documentation.
Missing State/In review label on issue #10861: The linked issue should carry the State/In review label; per project workflow, the issue should be moved to State/In review when the PR is submitted.
Verdict: APPROVED
All checklist categories pass. The implementation correctly fixes bug #10861 with appropriate, comprehensive tests. Minor suggestions noted above are non-blocking and can be addressed in a follow-up.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
harbor.cleverthis.comfromdocker:dind