fix(kanban): enforce exact PASS review verdicts

[djagya]

What does this PR do?

A first-class Kanban review task can currently transition to done through the DB, CLI, model tool, or dashboard even when its completion metadata is missing a verdict or explicitly records a rejected/conditional verdict. Because done satisfies ordinary dependency edges, that false terminal state can immediately promote downstream work.

This PR makes review approval a DB-level invariant:

• review work may complete only when metadata.verdict is a string that normalizes via strip().upper() to exact PASS;
• missing, malformed, empty, FAIL, and conditional verdicts fail closed without changing task state or promoting dependents;
• claimed reviewer runs and reviewer escalations parked in blocked retain review identity, so leaving the literal review status cannot bypass the gate;
• verdict eligibility and expected_run_id are re-read inside the final SQLite write transaction, closing the completion time-of-check/time-of-use race;
• rejected attempts emit a durable completion_blocked_nonpassing_verdict event while leaving the task in flight;
• CLI, model-tool, and dashboard callers surface the same actionable verdict error for both ordinary preflight rejection and the racy in-transaction rejection; the latter commits its audit event before raising.

Ordinary completion keeps the existing free-form metadata contract, including a caller-defined metadata.verdict; exact PASS is enforced only when durable task context identifies review work.

Compatibility: complete_task now raises NonPassingVerdictError for a semantic review rejection after committing its audit event. All four first-party production callers handle that domain error; direct Python/plugin callers that complete review work should handle it as well. Ineligible, stale, and non-review completion paths retain their existing boolean behavior.

Related Issue

Related to #67132, but intentionally does not claim to close it. That issue asks for typed approval dependency edges, mixed fan-in, and remediation fan-out semantics. This PR instead enforces the narrower first-class review-task invariant before the parent can become terminal.

Overlap review:

• fix(kanban): gate children on explicit approval #67156 and fix(kanban): gate dependencies on semantic verdicts #68850 gate selected dependency edges after parent terminality; this PR is complementary because it prevents a review task from reaching terminal done without approval.
• feat(kanban): review-gate guard + ADR-19 object-fidelity + gate-stalled recovery #94861 prevents an assignee from dispatching before a review-gate card is linked; that is an earlier, orthogonal lifecycle boundary.
• fix(kanban): enforce swarm review gates #63124 was a withdrawn swarm-specific metadata.gate proposal with additional unrelated scope; this PR does not supersede it.
• The change is follow-up hardening for the review lifecycle merged through fix(kanban): first-class review lifecycle — salvage of #75451 with review fixes #83412.

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

• hermes_cli/kanban_db.py — normalize review verdicts, identify durable review context, revalidate inside write_txn, commit structured rejection events before raising, preserve ordinary free-form metadata, and keep task/dependency state unchanged on rejection.
• hermes_cli/kanban.py, tools/kanban_tools.py, plugins/kanban/dashboard/plugin_api.py — return actionable completion errors consistently across CLI, model-tool, and dashboard surfaces.
• agent/prompt_builder.py, skills/devops/sdlc-review/SKILL.md — teach reviewers to approve only with metadata={"verdict": "PASS"} and route correctable findings through kanban_request_changes.
• website/docs/user-guide/features/kanban-tutorial.md, website/docs/user-guide/features/kanban-worker-lanes.md, and the corresponding zh-Hans tutorial — document the enforced verdict contract.
• tests/hermes_cli/test_kanban_verdict_gate.py plus existing lifecycle/tool/dashboard/skill suites — cover missing, malformed, conditional, stale-run, claimed-review, blocked-review, ordinary free-form metadata, transactional review races, durable rejection events, and dashboard race UX.

How to Test

1. Run the focused regression and surface suites:

   python -m pytest -q \
     tests/hermes_cli/test_kanban_verdict_gate.py \
     tests/hermes_cli/test_kanban_review_lifecycle.py \
     tests/hermes_cli/test_kanban_review_lifecycle_complete.py \
     tests/tools/test_kanban_tools.py \
     tests/plugins/test_kanban_dashboard_plugin.py \
     tests/skills/test_sdlc_review_skill.py

   Result: 138 passed.

2. Run Ruff on all touched Python files. Result: All checks passed!.

3. Run git diff --check upstream/main...HEAD. Result: clean.

The regression suite verifies that rejected completion leaves the review task in flight and its dependent child held, while normalized exact PASS completes the task and promotes the child.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits
• 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 pytest tests/ -q and all tests pass — the focused 138-test affected suite passes; the full repository suite was not run
• I've added tests for my changes
• I've tested on my platform: Linux

Documentation & Housekeeping

• I've updated relevant documentation
• I've updated cli-config.yaml.example — N/A; no configuration keys changed
• I've updated CONTRIBUTING.md or AGENTS.md — N/A; no repository architecture/workflow convention changed
• I've considered cross-platform impact — the invariant is implemented in platform-neutral SQLite/Python paths
• I've updated tool descriptions/schemas to document review-only verdict semantics and ordinary free-form metadata

Screenshots / Logs

N/A — this is a lifecycle invariant and structured error-path change with automated coverage.

[Enough1122]

AI code review — automated review for reference; please use your judgment.

Kernel-enforced verdicts are the right place for this invariant, and the TOCTOU recheck inside the write transaction (with its own regression test) is exactly how a race like this should be closed. Test coverage is excellent — CLI, tool surface, dashboard PATCH/bulk, blocked-escalation, stale-run, and concurrency paths. Notes:

• hermes_cli/kanban_db.py:5557 — asymmetric rejection surfaces: the preflight path raises NonPassingVerdictError (friendly CLI/409/tool messages), but the racy in-transaction path return Falses — so the CLI prints "cannot complete (unknown id or terminal state)" and the dashboard PATCH presumably a generic failure. A reviewer who loses the race gets a misleading "unknown id"-style error instead of the verdict guidance. Consider raising from the in-txn path too (the event is already committed; the exception only changes caller UX).
• hermes_cli/kanban_db.py:5501 — contract change beyond the diff: complete_task now raises where it previously returned False-style outcomes. The three updated callers (CLI, kanban_tools, dashboard) are covered, but any other in-repo or community-plugin caller of kb.complete_task (MCP surfaces, cron/automation scripts, other plugins) now receives a ValueError instead of a bool — on an unhandled path a reviewer mistake crashes the worker instead of leaving the task in-flight. A quick repo-wide call-site audit is warranted before merge.
• tools/kanban_tools.py:1813 — verdict is now reserved even for non-review completions (explicit FAIL on an ordinary running task fails closed — intentional per the tests). That's a behavioral break for any existing automation that already writes a free-form metadata.verdict key at completion; worth an explicit changelog/migration note, not just the schema text.
• hermes_cli/kanban_db.py:5352 — nit: _normalise_completion_verdict uses British spelling while the error message and schema text say "normalize"; one codebase convention would be kinder to future greps.

No blocking issues found.

[djagya]

Thanks — I audited each point and updated the PR.

• The in-transaction race path now commits completion_blocked_nonpassing_verdict and then raises the same NonPassingVerdictError as preflight. A dashboard regression test forces review to land between preflight and the final write and verifies a 409 with verdict guidance, a durable audit event, and no task/dependency promotion.
• A current-tree AST audit found four first-party production callers of complete_task (CLI, model tool, dashboard PATCH, and dashboard bulk); all four handle the domain error. The PR now documents the compatibility requirement for direct Python/plugin callers as well.
• metadata.verdict is no longer reserved globally. Exact PASS is enforced only when durable task context identifies review work; ordinary completion retains free-form metadata, with regression coverage and updated tool descriptions.
• _normalise_completion_verdict is now _normalize_completion_verdict.

The branch was rebased onto current main. Focused verification: 138 passed; Ruff clean; git diff --check clean.

[djagya]

Closing due to age and conflicts (15 files, thousands of commits behind current main).

The review-verdict invariant is still needed (#67132), but this patch no longer rebases cleanly and I am not picking that fight in this form. If I return to it, it will be a new PR against current main.