Skip to content

fix(kanban): stop contradictory review approvals from completing - #102477

Open
fangliquanflq wants to merge 2 commits into
NousResearch:mainfrom
fangliquanflq:fix/kanban-structured-review-contract
Open

fangliquanflq wants to merge 2 commits into
NousResearch:mainfrom
fangliquanflq:fix/kanban-structured-review-contract

Conversation

@fangliquanflq

Copy link
Copy Markdown

What does this PR do?

Protected Kanban reviews can no longer transition to done while their structured approval evidence contradicts the declared acceptance contract. The kernel now validates opt-in task or board review contracts before every completion path, records rejected attempts without ending the review, and provides a separately audited manual risk override.

Symptom

A same-card reviewer could report review_outcome: "approved" while also recording failed checks, unresolved findings, untested requirements, non-passing acceptance rows, or a conflicting approved/verdict field. The task still transitioned from review to done.

Impact

Downstream work could be released from a task whose own structured review evidence documented that mandatory acceptance criteria were not met. Operators then had to audit and reverse the false acceptance manually.

Bug Cause

Trigger: hermes_cli/kanban_db.py / complete_task() when completing a task parked in review or claimed from the review lane.

Causal chain:

  1. A task entered the first-class same-card review lifecycle.
  2. The reviewer supplied contradictory structured metadata to a completion surface.
  3. complete_task() treated metadata as entirely free-form and executed the done transition without validation.

Why it is wrong: Review metadata could claim approval and failure simultaneously, so the durable lifecycle state did not match the machine-readable evidence attached to it.

Working sibling / contrast: The existing created_cards completion gate already validates structured claims in the kernel and writes a rejection event while preserving task state. Review approval evidence had no equivalent invariant.

Ruled out: Parsing caveat or summary prose is intentionally excluded. Free text is explanatory and cannot provide a dependable machine contract.

Fix

  • Resolve an optional schema-v1 review contract from the task's review handoff metadata or board metadata, preserving task contracts across re-review cycles.
  • Validate exact approval outcome, required acceptance rows, empty failure lists, optional full commit SHA, and conflicting boolean/verdict fields in complete_task() so CLI, worker tool, dashboard/API, retries, and goal-mode all share one gate.
  • Emit review_completion_rejected without mutating review state; expose a manual-only --review-override-reason that emits the distinct review_completion_overridden event.
  • Document the contract and add kernel, CLI, worker-tool, and dashboard regressions.

Related Issue

Fixes #102457

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Changes Made

  • hermes_cli/kanban_db.py - enforce and audit opt-in structured review contracts.
  • hermes_cli/kanban.py - expose the explicit manual risk-acceptance override.
  • tools/kanban_tools.py - route board identity into the shared kernel and document protected-review metadata.
  • plugins/kanban/dashboard/plugin_api.py - return an actionable conflict response and support explicit operator override.
  • tests/ - cover valid approval, contradictory fields, review recovery, re-review persistence, CLI, worker tool, and dashboard paths.
  • website/docs/user-guide/features/kanban.md - document protected structured reviews.

How to Test

  1. Request review with metadata.review_contract, then attempt completion with a non-PASS required row or non-empty failure list; completion is rejected and the task remains in review.
  2. Complete with all required rows at PASS and empty failure lists; the task transitions to done.
  3. Run:
scripts/run_tests.sh tests/hermes_cli/test_kanban_review_lifecycle_complete.py tests/hermes_cli/test_kanban_goal_mode.py tests/hermes_cli/test_kanban_cli.py tests/tools/test_kanban_tools.py tests/plugins/test_kanban_dashboard_plugin.py -q

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've run the repository test entry on the relevant suites and all tests pass
  • I've added tests for my changes
  • I've tested on my platform: Windows 11

Documentation & Housekeeping

  • I've updated relevant documentation
  • cli-config.yaml.example is N/A because no config key changed
  • CONTRIBUTING.md and AGENTS.md are N/A because no project workflow changed
  • I've considered cross-platform impact; the implementation uses Python, JSON, SQLite, and pathlib-neutral metadata paths
  • I've updated tool descriptions/schemas for the protected-review metadata contract

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/cron Cron scheduler and job management labels Sep 3, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management P3 Low — cosmetic, nice to have type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Kanban review completion should reject contradictory structured approval metadata

2 participants