feat(kanban): add resolve-fanin command for review verdicts - #40465
feat(kanban): add resolve-fanin command for review verdicts#40465verybigdog wants to merge 4 commits into
Conversation
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment — High surface area
This PR introduces a new resolve-fanin command (775 additions, 1 deletion, affects multiple files). In its current state this is a high-surface-area change that touches CLI argument parsing, kanban command dispatch, and the resolver logic.
Observations
- The new command is well-structured with
--dry-run/--applymutually exclusive modes - Ledger JSON output is emitted regardless of
--jsonflag — good for downstream tooling - The
--fix-assignee,--review-assignee, and--reporter-assigneeparameters provide clear ownership
Suggestions for formal review
- Add integration tests exercising the
--applypath with a real kanban database fixture - Document the classification heuristics in
kanban_resolverfor maintainability - Consider adding a
--max-fan-in-thresholdparameter to bound automatic remediation scope
Please split into smaller, independently reviewable commits (CLI wiring, resolver core, tests) for a faster formal review turnaround.
Reviewed by Hermes Agent
b7cda43 to
f43564d
Compare
|
Addressed the review feedback in the updated head (
Verification run locally: |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for separating the resolver, CLI wiring, and tests. There is one blocking integration issue with the current Kanban review contract.
Problems
hermes_cli/kanban_resolver.py:424creates the fix withparents=[final_task_id]. Current main's documented review handoff iskanban_block(reason="review-required: ...")(website/docs/user-guide/features/kanban-worker-lanes.md:63), and those blocks are intentionally sticky (hermes_cli/kanban_db.py:3324-3329). A child with a non-doneparent becomestodo(hermes_cli/kanban_db.py:2599-2606), while dispatch only considersreadytasks (hermes_cli/kanban_db.py:7326-7330). Thus an applied remediation pair cannot run for the review-required case it is intended to resolve.- The fixture completes the final review at
tests/hermes_cli/test_kanban_resolve_fanin.py:54-59, so it bypasses this production path.
Suggested changes
- Rework the graph/state handling so remediation is runnable while preserving the blocked review handoff.
- Add a real blocked
review-requiredapply-and-dispatch regression test.
Automated hermes-sweeper review.
| body=fix_body, | ||
| assignee=fix_assignee, | ||
| created_by="fanin-resolver", | ||
| parents=[final_task_id], |
There was a problem hiding this comment.
This makes remediation depend on the final review task. The documented review-required flow leaves that task sticky-blocked, so create_task() will place this child in todo; dispatcher selection is limited to ready tasks. Please rework the relationship/state transition so the fix can run without requiring the blocked review handoff to be completed first.
f43564d to
828fdf1
Compare
|
M28.3 remediation pushed: 828fdf1 What changed:
Verification:
Mergeability: PR head is rebased onto current Verdict: GO-for-review. |
|
Verdict: GO Reviewed PR #40465 head 828fdf1 against origin/main 569b912. Verified:
Findings: none blocking. Risk: GitHub still reports mergeable=MERGEABLE but mergeStateStatus=BLOCKED with empty statusCheckRollup, so repository branch-protection/status gates may still need to report green before merge. |
Summary
hermes kanban resolve-faninfor turning review/fan-in task results into a canonical GO/BLOCK/NEED_MORE verdict.Safety / behavior
--execute.Test Plan
git diff --check origin/main..HEADpython3 -m py_compile hermes_cli/commands.py hermes_cli/kanban.py hermes_cli/kanban_resolver.pypython3 -m pytest tests/hermes_cli/test_kanban_resolve_fanin.py -q -o 'addopts='Relationship to active handoff work
This is intentionally separate from #37865. That PR adds the explicit active-wake messaging primitive; this PR is only the generic Kanban fan-in resolver command.