Skip to content

feat: add kanban PR review polling loop - #29413

Open
tw3akercc wants to merge 5 commits into
NousResearch:mainfrom
tw3akercc:feat/kanban-pr-review-poller-v2
Open

feat: add kanban PR review polling loop#29413
tw3akercc wants to merge 5 commits into
NousResearch:mainfrom
tw3akercc:feat/kanban-pr-review-poller-v2

Conversation

@tw3akercc

Copy link
Copy Markdown

Summary

  • Re-scopes the Kanban PR follow-up loop onto upstream's existing single review status instead of adding in_review / code_review / merge_ready.
  • Moves PR-bearing task completions into review while preserving run metadata, avoiding completed_at, and preventing dependent tasks from unblocking prematurely.
  • Adds hermes kanban pr-review-poll <task_id> plus parser/poller logic for gh pr view, gh pr checks, requested changes, unresolved review threads, closed-unmerged PRs, and seen-item dedupe.
  • Updates CLI handling, docs, bundled skills, and tests for the simplified review-loop metadata shape.

Test plan

  • python -m pytest tests/hermes_cli/test_kanban_pr_review.py tests/hermes_cli/test_kanban_cli.py tests/hermes_cli/test_kanban_db.py tests/hermes_cli/test_kanban_core_functionality.py tests/tools/test_kanban_tools.py -o 'addopts=' -q -> 462 passed
  • git diff --check -> passed
  • python -m compileall -q hermes_cli/kanban_pr_review.py hermes_cli/kanban.py hermes_cli/kanban_db.py -> passed

Notes

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard tool/skills Skills system (list, view, manage) labels May 20, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for re-scoping this onto the existing review lifecycle. Two blocking issues need correction before this can provide the documented polling behavior.

Problems

  • hermes_cli/kanban_pr_review.py:297 reads only latest_run() for PR metadata. Current main creates a newer, metadata-free review run in claim_review_task() (hermes_cli/kanban_db.py:3538-3559), and latest_run() selects that newest run (hermes_cli/kanban_db.py:8916-8923). After dispatch starts the review worker, pr-review-poll therefore cannot locate the original PR reference.
  • hermes_cli/kanban_pr_review.py:332 requests conclusion from gh pr checks --json. The current CLI rejects that field; _gh_json(..., default=[]) suppresses the error and makes all checks appear absent.

Suggested changes

  • Persist or search historical PR metadata across the implementation handoff and review run; add an end-to-end DB test for poll-after-review-claim.
  • Use supported gh pr checks fields (bucket/state) and test the actual constructed command shape.

Automated hermes-sweeper review.

if not task:
raise ValueError(f"unknown task {task_id}")
run = kb.latest_run(conn, task_id)
ref = pr_reference_from_task(task, run)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

latest_run() becomes the newly-created review-agent run as soon as the dispatcher claims the card, but that run has no copied PR metadata. The original implementation handoff is then older, so this raises before polling. Resolve the latest run that contains PR metadata, or persist the normalized PR reference across the review handoff.

cmd = [
"gh", "pr", "checks", ref.selector, "--json",
"name,state,conclusion,link,bucket,workflow",
]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gh pr checks --json does not support conclusion (the CLI reports only bucket, completedAt, description, event, link, name, startedAt, state, and workflow). This command fails and the default [] path hides it, so check failures and pending checks are never detected. Classify the supported bucket/state fields instead.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state tool/skills Skills system (list, view, manage) type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants