fix(kanban): retire on-complete orchestrator reaction (revert PR #43)

[exiao]

Retire the on-complete orchestrator reaction (revert PR #43)

Date: 2026-06-29
Branch: fix/retire-on-complete-review → live-config
Task: t_710545d4

Symptom

PR #43 (feat: on-completion → orchestrator decides the next step, merged to
live-config) fired ONE "Decide next" orchestrator decision card on EVERY kanban
task completion — parented or not, decomposed or not. Result: ~340 of ~674
cards on the board were auto-spawned "Decide next" coordination cards: pure
coordination tax. The hook fired blind (it sees only "task X done" and must
reconstruct from a summary whether a real next step exists), so it over-fired,
and it was redundant with the dependency engine that already auto-promotes
children when their parents finish.

Root cause / why it's the wrong shape

The official Kanban docs have NO on-completion hook. The documented model:
orchestration clusters around DECOMPOSITION (the orchestrator judges completion
only of parents it decomposed); the dependency graph advances work mechanically
(a parented child auto-promotes when its parents are done); and workers
create/link follow-up cards themselves via kanban_create + kanban_link as
they discover work. PR #43 inverted "agents declare what's next while they hold
the context" into "a cold orchestrator infers it after the fact" — off-grain.

Decision (Eric, 2026-06-29): replace the hook with INSTRUCTIONS in the
constitution, then remove the feature. The loop was already turned OFF in
~/.hermes/config.yaml (kanban.on_complete_review.enabled: false, read fresh
each tick) ahead of this revert.

Fix (instructions first, then remove the code)

Instructions (landed first, approved by Eric):

• ~/.hermes/constitution.md — new "Follow-on work: workers open the cards, the
  graph carries the rest" section: bake mandatory review/verify into the
  decomposition graph up front (auto-promoting dependency children, no watcher),
  and have working agents open + assign + link follow-on cards as they uncover
  work. Silence = nothing's next; no manufactured "decide next" coordination
  cards.
• ~/.hermes/skills/devops/kanban/references/on-complete-orchestrator-routing.md
  — rewritten from "how to wire the on-complete reaction" to "why there is no
  on-completion hook and how follow-on work actually flows," documenting the
  retired feature and why not to re-add it. SKILL.md reference-index line
  updated to match.

Code removed (this PR):

• gateway/kanban_watchers.py — deleted _AUTO_REACTION_MARKER,
  _resolve_on_complete_review_config, _orchestrator_assignee,
  _qualifies_for_on_complete_review, the _maybe_react_on_complete mixin
  method, the per-tick on_complete_review_cfg resolution, and the call site in
  the notifier completed branch (with its getattr guard).
• hermes_cli/config.py — removed the kanban.on_complete_review DEFAULT_CONFIG
  block.
• tests/gateway/test_kanban_on_complete_review.py — deleted (the feature it
  tested is gone).

Files Changed

• gateway/kanban_watchers.py
• hermes_cli/config.py
• tests/gateway/test_kanban_on_complete_review.py (deleted)
• (runtime, not in this repo) ~/.hermes/constitution.md,
  ~/.hermes/skills/devops/kanban/references/on-complete-orchestrator-routing.md,
  ~/.hermes/skills/devops/kanban/SKILL.md

Verification

• grep -rn 'on_complete_review\|_AUTO_REACTION_MARKER\|_maybe_react_on_complete\|_qualifies_for_on_complete_review\|_orchestrator_assignee' across the repo
  returns zero hits after the change.
• scripts/run_tests.sh tests/gateway/ -- -k kanban stays green (the deleted
  test no longer collected; remaining kanban watcher tests pass).

Residual risk

Low. The feature was already config-gated OFF in production (enabled: false),
so removing the code changes no live behavior. The dependency engine and
worker-side card creation (both pre-existing) carry follow-on work; nothing
depended on the reaction. kanban.orchestrator_profile is unaffected — it's
still read by Triage auto-decompose.

[gemini-code-assist]

Code Review

This pull request removes the on-completion review reaction feature from the Kanban watcher system. This includes deleting the loop-guard marker, configuration resolvers, orchestrator assignee resolution, qualification gating, the reaction handler method _maybe_react_on_complete, its integration within the notifier watcher loop, default configuration values, and all associated unit and integration tests. There are no review comments, so I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[github-actions]

🔎 Lint report: fix/retire-on-complete-review vs origin/live-config

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 11580 on HEAD, 11581 on base (✅ -1)

🆕 New issues: none

✅ Fixed issues (1):

Rule	Count
unresolved-attribute	1

First entries

gateway/kanban_watchers.py:1367: [unresolved-attribute] unresolved-attribute: Object of type `Self@_maybe_react_on_complete` has no attribute `adapters`

Unchanged: 6089 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

[exiao]

Independent Code Review — PR #72

Ran an independent, read-only review (skeptical pass, find-the-bug framing) against the worktree at HEAD 70dbe1ad, base live-config. Method: read AGENTS.md, full diff, tree-wide grep for every deleted symbol, full-context read of changed files, py_compile on all three modified modules.

Verdict: clean revert, safe to merge. The removal is total and correct.

Confirmed clean

• Symbol removal complete. Tree-wide grep for on_complete_review, _AUTO_REACTION_MARKER, _maybe_react_on_complete, _qualifies_for_on_complete_review, _orchestrator_assignee, _resolve_on_complete_review_config, and the kanban-on-complete-reaction marker returns zero surviving references in any .py / .md / .yaml. No dangling caller, doc, or test import.
• Notifier completed branch intact. Post-removal the artifact block closes cleanly into the failure-counter reset (gateway/kanban_watchers.py ~848–863); no orphaned variable, no dangling if. The per-tick on_complete_review_cfg / react_fn / review_cfg readers are all gone (0 matches).
• No stranded imports. asyncio, Callable, Optional, Platform, kanban_db are all still used elsewhere in the file.
• kanban.orchestrator_profile untouched. Still defined in hermes_cli/config.py and read by kanban_decompose.py and the dashboard plugin api. Only the dead _orchestrator_assignee consumer was removed.
• Live-config migration safe. Config merge is a permissive _deep_merge with no unknown-key rejection in validate_config_structure, so a running gateway whose live config.yaml still carries kanban.on_complete_review will not KeyError or fail validation — the orphaned block is inert dead config.
• Prompt/cache/alternation path: untouched (lint_diff is CI-only).

One nit found and fixed in this PR

• scripts/lint_diff.py regex was plural-only. _TY_UNION_OMISSION_RE hard-coded ... omitted \d+ union elements, so a singular omitted 1 union element diagnostic would not normalize — the exact spurious fixed+new flip the rider claims to kill, on the 1↔2 boundary. Impact is cosmetic (lint-diff legibility, never fails the build). Fixed by making the trailing s optional (elements?) and adding a singular/plural boundary test. Verified fail-before (old regex misses singular) / pass-after.

Reviewed by EX Bot (independent read-only subagent) — no prior human/bot review existed on this PR.