feat(kanban): enforce native review handoff evidence

[solovision24]

Summary

• validate reviewer profiles and immutable PR evidence before review mutation
• deduplicate active review submissions by PR head and preserve implementer provenance
• expose native review tools and update guidance/docs

Verification

• HERMES_PYTHON=/home/solo/.hermes/hermes-agent/venv/bin/python scripts/run_tests.sh tests/hermes_cli/test_kanban_review_lifecycle.py tests/hermes_cli/test_kanban_db.py tests/tools/test_kanban_tools.py -q
• ruff check (targeted files)
• git diff --check

[solovision24]

Orion Review — changes required

Reviewed immutable head 47d8fbbfb3732cb59cf4fdffdfa50d559512f915. This artifact is not mergeable or reviewable yet.

Blocking findings

1. PR baseline is invalid / overlapping. GitHub reports PR feat(kanban): enforce native review handoff evidence #10 as CONFLICTING / DIRTY, with 8,097 changed files, +2,125,712/-176,746, and no CI checks. The fork main base is 202f84cd13f35d005b596265d7ce8d1bf22c5a7c; this branch has no usable three-dot merge base with current upstream main. PR feat(kanban): route implementation handoffs through review #5 is also still open and carries the same 8,097-file overlap, violating the task's exactly-one-PR requirement. Reconcile the fork/base and update one canonical PR so only the intentional Review/Scheduled compatibility delta remains; close the superseded PR.

2. The new tool implementation is dead because it duplicates existing definitions and registrations. tools/kanban_tools.py defines _handle_submit_review at lines 718 and 889, _handle_review_changes at 769 and 912, schemas at 1766/1793 and again at 1855/1875, and registers both tools twice at 2296/2305 and 2323/2332. Runtime inspection proves the effective submit handler is line 889 and the effective schema requires only reviewer, summary, so the new metadata-required schema and ownership checks never become the exposed contract. Consolidate each handler/schema/registration to one definition and add a runtime registry/dispatch regression test.

3. The prompt reintroduces the forbidden Blocked-as-Review lifecycle. agent/prompt_builder.py:251-255 tells workers to use kanban_block(reason="review-required: ...") when PR evidence is absent. The canonical lifecycle explicitly says Review is not Blocked and review-required is not a Review trigger. Keep kanban_block only for genuine external blockers; do not present it as a review fallback.

4. Abbreviated SHAs are accepted and were used in the handoff. hermes_cli/kanban_db.py:4284 accepts 7–64 hex characters, and this task was submitted with 47d8fbbfb, not the exact 40-character GitHub head. An abbreviated SHA is not the immutable PR artifact required by the contract. Require the exact GitHub head format and resubmit with the full live head SHA.

5. Required conformance evidence and documentation are incomplete. The changed lifecycle test file contains only four tests. The required webhook/submit orderings, merged/closed reconciliation, re-review, rollback, forced sdlc-review, and full Scheduled-vs-scheduled_at contract are not evidenced by this PR. The docs also contain no Upstream released / SoLo compatibility extension / pending upstream matrix, no upgrade checklist, and no references to upstream issue Kanban review status exists but has no first-class request-review transition NousResearch/hermes-agent#42896 / open PR fix(kanban): complete first-class review lifecycle NousResearch/hermes-agent#75451. Add behavior-level coverage and the requested boundary documentation; do not claim pending upstream behavior as released.

Verification performed

• Focused suite: 68 passed (test_kanban_review_lifecycle.py, test_kanban_db.py, test_kanban_tools.py)
• Targeted Ruff: passed
• git diff --check for the submitted commit: passed
• Runtime registry inspection: reproduced the duplicate-definition/schema shadowing above
• GitHub: no checks reported; PR currently conflicting

Acceptance criteria for re-review

• One open canonical fork PR, narrow diff, clean mergeability, exact new 40-character head SHA.
• One effective definition/schema/registration per Review tool; runtime test proves required metadata and ownership enforcement.
• No Blocked/review-required fallback in worker guidance.
• Full required conformance scenarios and the upstream/compatibility/pending matrix + upgrade checklist.
• Focused suites, relevant broader suites, lint/docs checks, and dry-run evidence attached to the same task before native re-review submission.

[chatgpt-codex-connector]

💡 Codex Review

https://github.com/solovision24/hermes-agent/blob/47d8fbbfb3732cb59cf4fdffdfa50d559512f915/tools/kanban_tools.py#L723-L725
Remove the shadowing review handler definitions

These guarded handlers are overwritten by the later definitions at lines 889 and 912 before either registration executes; the hardened schemas are similarly overwritten at lines 1855 and 1875. Once this commit exposes the tools through toolsets.py, a dispatcher worker therefore receives the old handlers without _enforce_worker_task_ownership, so passing a foreign running task ID makes _worker_run_id return None and allows the worker to submit or close a sibling task's run. Remove the duplicate legacy definitions and test the actually imported registry entries.

AGENTS.md reference: AGENTS.md:L1259-L1262

---

https://github.com/solovision24/hermes-agent/blob/47d8fbbfb3732cb59cf4fdffdfa50d559512f915/hermes_cli/kanban_db.py#L4367-L4369
Preserve the task's creation idempotency key

When an implementation task originated from a retriable webhook or automation with its own idempotency_key, submitting it for review replaces that key with the PR identity. A retry of the original create_task(..., idempotency_key=...) then cannot find the non-archived task and creates a duplicate implementation card. Store the review identity separately, or otherwise preserve the creation key while performing review deduplication.

---

https://github.com/solovision24/hermes-agent/blob/47d8fbbfb3732cb59cf4fdffdfa50d559512f915/hermes_cli/kanban_db.py#L4344-L4345
Reject the implementer as the reviewer

For an implementation assigned to an existing profile, callers can pass that same profile as reviewer; this check verifies only that the profile exists, and the subsequent update reassigns the card back to the implementer. The same profile can therefore claim and approve its own work despite the new schema and documentation promising independent review. Compare the canonical reviewer with the task's original assignee and reject self-review.

---

https://github.com/solovision24/hermes-agent/blob/47d8fbbfb3732cb59cf4fdffdfa50d559512f915/hermes_cli/kanban_db.py#L4289-L4290
Require metadata in the submit-review CLI

The submit-review parser still defines --metadata as optional in hermes_cli/kanban.py:604-610, so argparse's generated help permits hermes kanban submit-review <task> <reviewer> <summary> without it. This newly unconditional validation then raises instead of performing the previously valid handoff. Mark the CLI option required and describe the five required fields, or have the CLI obtain and construct the evidence itself.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[solovision24]

Orion Review — changes still required

Reviewed immutable head e0c5b8200554797fe94282761bf7f4625400decb directly. The duplicate tool shadowing, blocked-as-review guidance, abbreviated-SHA acceptance, and giant diff were fixed, and the focused suite passes 69/69. The PR is not acceptable yet.

Blocking findings

1. Critical — the PR does not land on the fork integration branch and the superseded PR remains open. PR feat(kanban): enforce native review handoff evidence #10 now targets synthetic branch review-base-origin-main, not fork main; merging it would not ship this lifecycle to the fork default branch. PR feat(kanban): route implementation handoffs through review #5 is still OPEN, CONFLICTING, and carries the same 8,097-file lineage. Reconcile the fork integration boundary, make exactly one canonical PR that lands on the intended fork branch, and close feat(kanban): route implementation handoffs through review #5.

2. High — changes-requested still breaks the required same-card review loop. hermes_cli/kanban_db.py:4561-4593 creates a new physical remediation task and marks the reviewed card done. The required lifecycle and pending upstream intent are same-card: reviewer requests changes -> original card returns to the original implementer -> fixes -> re-review on the new immutable head. Implement that idempotent loop and prove multi-round re-review; do not multiply implementation/reviewer/fix cards.

3. High — submission corrupts producer idempotency and permits self-review. submit_for_review() overwrites tasks.idempotency_key with the PR identity at hermes_cli/kanban_db.py:4513-4517. Runtime reproduction showed a retry using the original producer key creates a duplicate task. The same runtime probe also showed reviewer == original assignee is accepted, contradicting the independent-review contract. Preserve creation idempotency separately from review deduplication and reject self-review before mutation.

4. High — review rollback is only fixed for one crash path. release_stale_claims() and reclaim_task() still force reviewer-owned runs to ready at hermes_cli/kanban_db.py:4732-4747 and 4804-4822. Runtime reproduction returned a claimed Review card to ready; that bypasses the Review dispatcher and loses forced sdlc-review. Preserve source phase across stale/manual reclaim, timeout, spawn failure, rate-limit, and crash paths, with rollback tests.

5. High — the required producer/verdict and conformance contract remains incomplete. The new lifecycle file has only five tests and there are no changed webhook/dashboard ingestion paths. Missing evidence includes duplicate webhook-before-submit and submit-before-webhook orderings, merged/closed PR reconciliation, same-card re-review, invalid alias/non-mutation dry run, full approval/changes/escalation outcomes, Scheduled status vs scheduled_at, and rollback. The tool suite also has no runtime registry/dispatch regression despite that being explicit acceptance criteria.

6. High — CLI/default-review and documentation boundaries remain incomplete. hermes_cli/kanban.py:610 leaves --metadata optional even though the domain rejects omission; the producer has no SoLoVision orion default; and the schema/kernel do not enforce the complete handoff contract described by the task. The docs do not contain the required literal Upstream released / SoLo compatibility extension / pending upstream matrix, do not reference open issue Kanban review status exists but has no first-class request-review transition NousResearch/hermes-agent#42896 and open PR fix(kanban): complete first-class review lifecycle NousResearch/hermes-agent#75451, and do not document Scheduled versus scheduled_at semantics.

Verification performed

• PR live state: OPEN, MERGEABLE/CLEAN, 8 files, +858/-33, no CI checks reported.
• Focused suite: 69 passed, 0 failed via scripts/run_tests.sh.
• Runtime registry: effective submit handler line 797; schema requires reviewer, summary, and metadata.
• Runtime probes reproduced producer-key duplication, accepted self-review, and stale reviewer reclaim flattening to ready.
• Upstream state checked live: issue Kanban review status exists but has no first-class request-review transition NousResearch/hermes-agent#42896 OPEN; PR fix(kanban): complete first-class review lifecycle NousResearch/hermes-agent#75451 OPEN/unmerged.

Re-review acceptance

• Exactly one open canonical fork PR, targeting the actual integration branch; feat(kanban): route implementation handoffs through review #5 closed.
• Same logical card cycles implementation -> Review -> changes -> original implementer -> Review, idempotently.
• Creation idempotency preserved; PR-head dedup stored separately; self-review rejected.
• Every reviewer recovery path returns to Review and force-loads sdlc-review.
• All required webhook/order/reconciliation/Scheduled/rollback/runtime-tool tests and a live dry run included.
• Exact upstream/compatibility/pending matrix and upgrade checklist, with Kanban review status exists but has no first-class request-review transition NousResearch/hermes-agent#42896/fix(kanban): complete first-class review lifecycle NousResearch/hermes-agent#75451 references.
• Re-submit the same task through native Review with the exact new 40-character head SHA and complete verification evidence.