fix(plan): wrap plan tree JSON/YAML output in spec-required command envelope #11071
No reviewers
Labels
No 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
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
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
cleveragents/cleveragents-core!11071
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/11041-plan-tree-envelope"
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
Fix the
agents plan tree --format json|yamlCLI command to wrap its output in the spec-required command envelope as defined in the Output Rendering Framework.Previously, JSON/YAML output was the raw decision-tree list directly, missing the required envelope structure. This made it impossible for consuming systems to identify the originating CLI command and parse structured responses consistently.
Changes
command="agents plan tree"parameter toformat_output()call intree_decisions_cmdfunction_capture_format_outputhelper and BDD step functions to pass command parameter; replaced old assertion steps with 7 new envelope-aware validators (JSON envelope key check, envelope command check, envelope data check for decision IDs, YAML envelope checks)@tdd_expected_failtag from issue #4254 test casePR Compliance Checklist
ISSUES CLOSED: #11041includedRelated Issues
Review Summary
This PR addresses the correct spec gap (wrapping
plan treeJSON/YAML output in the command envelope) and the production code change inplan.pyis correct and minimal. However, there are multiple blocking issues in the test code and PR metadata that must be fixed before this can be approved.CI is currently failing on
lint,unit_tests,benchmark-regression, and the aggregatestatus-check. Thecoveragegate was skipped (it only runs whenunit_testspasses) — so we cannot verify the >= 97% coverage requirement. The PR cannot be merged until all required gates are green.CI Status
Blocking Issues Found
Step phrase mismatch — Feature file scenario "Tree with json format contains tree decision IDs" uses
the json tree envelope has key "data"but the only registered step definition is decorated with@then('the json tree envelope should have key "{key}"'). The missing step causes Behave to error, explaining the unit_tests CI failure.Wrong function signature on YAML step —
step_tree_yaml_command_valueis decorated with@then('the yaml tree output should contain "plan tree"')(no capture group), yet the function accepts(context: Context, text: str)— Behave will error because it won't provide atextargument. Fix: remove thetext: strparameter.Mid-function imports violate project rules —
from collections import dequeappears inside two function bodies inplan_explain_steps.py. Project rules mandate all imports at the top of the file. This also likely contributes to thelintCI failure.Coverage is skipped — The
coverageCI job was skipped becauseunit_testsfailed. Coverage >= 97% is a hard merge gate. Once unit tests are fixed, the author must confirm coverage still passes.PR has no milestone — The PR is missing its milestone assignment. Per CONTRIBUTING.md, the PR must be assigned to the same milestone as the linked issue. Issue #11041 is in milestone
v3.2.0— the PR must match.Linked entity #11041 is a PR, not an issue —
Closes: #11041closes another PR, not a tracking issue. PRs should close Issues. Ensure the PR body usesCloses #Nreferencing the correct issue (not another PR), and verify the correct issue number.Forgejo dependency direction not set — Per CONTRIBUTING.md, the PR must block the linked issue (PR blocks issue). Currently PR #11071 has no blocking relationships set in Forgejo. Fix: on PR #11071, add the issue under "blocks".
Benchmark regression CI failing — The
benchmark-regressionjob is failing. Investigate whether this regression was introduced by this PR or is pre-existing. If introduced by this PR, it must be fixed.Missing trailing newline in CONTRIBUTORS.md — The diff shows no newline at end of file for the last line of
CONTRIBUTORS.md. This will fail lint EOF checks. Fix: ensure the file ends with a newline.Non-Blocking Observations
The
step_tree_yaml_containsimplementation is overly complex: the condition checking"decision_id" in context.pe_tree_yamlin the if-branch is always true if the YAML contains any decision_id. Suggestion: simplify to a straight string-in-string check.Changelog entry has a redundant parenthetical: "Output Rendering Framework (Output Rendering Framework)" — the inline parenthetical repeats the section title. Suggestion: remove the redundant parenthetical.
The
deque[dict]annotation uses an unsubscripteddict. For Pyright strict compliance, preferdeque[dict[str, Any]].What is correct
src/cleveragents/cli/commands/plan.pyis correct: addingcommand="agents plan tree"to theformat_output()call properly populates the envelope per spec._capture_format_outputhelper update in steps (adding thecommandparameter) is correct.step_format_tree_jsonandstep_format_tree_yamlwhen-steps correctly passcommand="plan tree"to the helper.### Fixedsection under[Unreleased].Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Suggestion (non-blocking): The changelog entry contains a redundant parenthetical — "Output Rendering Framework (Output Rendering Framework)" repeats the section name. Consider removing the parenthetical:
@ -39,3 +39,4 @@* HAL 9000 has contributed the Strategize phase full context snapshot fix (issue #9056): added `_build_strategize_context_snapshot()` helper to `PlanLifecycleService`, updated `_try_record_decision()` to accept and forward a `ContextSnapshot` parameter, and added BDD test coverage verifying all four `ContextSnapshot` fields (`hot_context_hash`, `hot_context_ref`, `actor_state_ref`, `relevant_resources`) are populated during the Strategize phase.* HAL 9000 has contributed the ACMS context path matching fix (PR #10975 / issue #10972): corrects `_path_matches()` and `_matches_pattern()` to properly match absolute fragment paths against relative glob patterns by auto-prefixing with `**/` before calling `PurePath.full_match()`, preventing silent inefficacy of include/exclude filters for absolute paths in fragment metadata.* HAL 9000 has contributed the plan tree JSON/YAML spec-compliant envelope fix (issue #11041): wrapped `agents plan tree` JSON and YAML output in the spec-required command envelope (`command`, `status`, `exit_code`, `data`, `timing`, `messages`), updated BDD step definitions to validate envelope structure, and removed the `@tdd_expected_fail` tag from the previously-failing JSON tree format test (issue #4254).BLOCKER: Missing trailing newline at end of file
The diff shows no newline at end of file for this last line. This will fail ruff's EOF lint check.
Fix: Ensure the file ends with a newline character after the last bullet line.
BLOCKER: Step phrase mismatch causing unit_tests CI failure
This line uses:
But the only registered step definition in
plan_explain_steps.pyis:Note the difference:
has keyvsshould have key. Behave will raiseNotImplementedError: Step not foundfor this line, causing the scenario to fail.Fix: Change the feature file to match the existing step:
BLOCKER: Mid-function import violates project rules and likely causes lint failure
This import appears inside the function body. Project rules are explicit: all imports must be at the top of the file (
if TYPE_CHECKING:is the only exception). Ruff will flag this as a lint violation.Fix: Move
from collections import dequeto the top of the file, alongside the other standard-library imports. The same issue exists in a second function further down in this file.BLOCKER: Wrong function signature — extra
text: strparameter Behave will not provideThis step:
The decorator has no capture group (no
{text}in the pattern), so Behave will call this function with onlycontext. The extratext: strparameter has no corresponding capture group — Behave will raise aTypeErrorat runtime.Fix: Remove the unused
text: strparameter:Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
First Review —
fix(plan): wrap plan tree JSON/YAML output in spec-required command envelopeSummary
The intent of this PR is correct and the core production-code change is minimal and well-targeted. However, there are four blocking issues that must be fixed before this PR can be approved:
commandenvelope value does not match what the specification requires.TypeErrorat runtime.CONTRIBUTORS.md— causes a lint failure (ruff W292), directly confirmed by the failingCI / lintcheck.CI is failing on
lint,unit_tests, andbenchmark-regression, all consistent with defects 1–4 above.CI Status
Review Checklist Results
commandvalue"agents plan tree"contradicts spec which requires"plan tree"# type: ignore; signatures are finedequeimport inside function body should move to module top-levelPlease address all four blocking issues. Inline comments below provide exact fix instructions for each.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Suggestion (non-blocking) — Duplicate phrase in prose
Line 100 reads:
The phrase
Output Rendering Frameworkappears twice — likely a copy-paste artifact from a Markdown link that lost its anchor text. Consider simplifying to:Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
BLOCKING — Missing trailing newline (ruff W292 / lint failure)
The new line appended to this file does not end with a newline character (
\ No newline at end of filein the diff). This violates the ruff W292 rule (no-newline-at-end-of-file) and is a direct cause of the failingCI / lintcheck.How to fix: Ensure the file ends with a newline character after the final line. Most editors insert this automatically; verify with:
If missing, append one:
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
BLOCKING — Undefined Behave step: text mismatch
Line 123 of the feature file reads:
But the only relevant step definition is decorated with:
has keydoes not matchshould have key. Behave will report this as an Undefined step, failing theTree with json format contains tree decision IDsscenario at runtime.How to fix: Update line 123 of the feature file to use the already-defined step text:
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Suggestion (non-blocking) — Move
from collections import dequeto module top-levelThe
from collections import dequeimport appears inline insidestep_tree_envelope_data_contains(new code, around line 444) and also inside_collect_ids(line 514). Moving it to the module-level import block (alongside the existing imports at lines 7–20) is consistent with project style, avoids repeated lazy imports, and makes the dependency immediately visible to readers and static analysers.Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
BLOCKING — Broken step function signature: spurious
textparameterThe step decorator is:
The pattern
'the yaml tree output should contain "plan tree"'contains no capture groups, so Behave calls this function with onlycontext. The declared second parametertext: strcauses Python to raiseTypeError: step_tree_yaml_command_value() missing 1 required positional argument: 'text'. The function body also never usestext.How to fix: Remove the
text: strparameter:Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
BLOCKING — Spec Violation: wrong
commandstring in envelopeThe spec (
docs/specification.mdline 14538) explicitly defines the envelopecommandfield for this command as:and for YAML (line 14598):
But this code passes
command="agents plan tree"(with theagentsprefix), making production output non-conformant with the specification.How to fix:
Note: the BDD step helpers at
features/steps/plan_explain_steps.pyalready passcommand="plan tree"(without the prefix), so fixing the production code also makes it consistent with the test helpers.Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
🌱 Grooming: proceed — PR cleared for processing.
(check
no_duplicates, categoryno_duplicates)PR #11071 is not a duplicate. While PR #11044 exists with an identical title ("fix(plan): wrap plan tree JSON/YAML output in spec-required command envelope") and targets the same issue (#11041), #11071 is substantially more complete: 109 additions across 5 files vs #11044's 1 addition across 1 file. #11071 includes full BDD test updates, CHANGELOG, and CONTRIBUTORS entries, making it the canonical/superior implementation. Other plan-envelope PRs (#9817, #9827, #11107, #11224) wrap different commands (apply, status), not plan tree.
📋 Estimate: tier 1.
Core fix is a single-line change in plan.py (adding command parameter to format_output call), but CI has two distinct failures requiring fixes: (1) trivial lint — two F541 f-string-without-placeholder errors in plan_explain_steps.py; (2) AmbiguousStep collision — a new specific Behave step '@then the yaml tree output should contain "command:"' conflicts with the existing generic '@then the yaml tree output should contain "{text}"' step at line 456. Resolving the step conflict requires understanding the BDD step registry and restructuring the new assertions to avoid the ambiguity. Multi-file scope (plan.py, plan_explain_steps.py, plan_explain.feature), touches test fixtures with real logic issues — standard Tier 1 engineering work.
- Change command from "agents plan tree" to "plan tree" per spec - Fix AmbiguousStep: remove three literal step functions that shadowed the generic '{text}' step, causing all 32 feature files to error - Fix step_tree_envelope_data_contains to check for key presence in nodes rather than checking for the literal key name as a value - Fix plan_explain.feature step text mismatch (has key -> should have key) - Move deque import to module level; remove two mid-function imports - Remove f-prefix from two f-strings without placeholders (F541)(attempt #5, tier 1)
🔧 Implementer attempt —
resolved.Pushed 2 commits:
daa3e63,e3e5aed.Files touched:
features/plan_explain.feature,features/steps/plan_explain_steps.py,src/cleveragents/cli/commands/plan.py.🌱 Grooming: proceed — PR cleared for processing.
(check
no_duplicates, categoryno_duplicates)PR #11071 shares an identical title with #11044 ("fix(plan): wrap plan tree JSON/YAML output in spec-required command envelope"), but is substantially more complete: 88 additions across 6 files (including comprehensive test updates in features/steps/ and features/plan_explain.feature) versus #11044's minimal 1-addition stub. The anchor PR is the better, more-complete implementation and should proceed rather than be marked as a duplicate.
📋 Estimate: tier 1.
Multi-file change (6 files) touching CLI command code, BDD feature file, and BDD step definitions. Core fix is straightforward (adding a command parameter to format_output()), but test burden is moderate: 7 new envelope-aware BDD validators added and @tdd_expected_fail tag removed. All 13 CI gates are failing, indicating the branch is likely stale or has implementation gaps requiring diagnosis and repair. Cross-file scope with non-trivial test logic changes puts this firmly at tier 1.
(attempt #8, tier 1)
🔧 Implementer attempt —
blocked.Blockers:
d5b35ecbadbut dispatch base was6d8f0ebe1c. The implementer pushed from inside the worktree (forbidden by the git contract) OR a third party pushed during the attempt. Re-dispatch will re-prefetch and pick up the new head.(attempt #9, tier 2)
🔧 Implementer attempt —
rebase-failed.Blockers:
d5b35ecbad14d6a9ed0d(attempt #11, tier 2)
🔧 Implementer attempt —
ci-not-ready.✅ Approved
Reviewed at commit
14d6a9e.Confidence: high.
Claimed by
merge_drive.py(pid 2202036) until2026-06-17T11:18:50.144554+00:00.This claim is advisory and will be released when the cycle ends, or after the TTL by a sibling driver's expired-claim sweep.
Approved by the controller reviewer stage (workflow 454).