feat(kanban): on-completion → orchestrator decides the next step - #43
Merged
Conversation
…Signal
Automate constitution rule 2 on the `completed` kanban event: when a
write-lane task completes, the gateway notifier spawns a review card parented
to it (a separate reviewer / second-producer context, never self-review) and
posts a progress line to the configured "Kanban Master" chat. The spawned
reviewer posts the ship/needs-rework verdict and routes any follow-up
(completed PR -> pr-babysitter) via its own kanban tools.
Scope + loop guards: triggers only on write lanes (cpe-dev, bloom-dev,
infra-ops, pr-babysitter, verifier, content-creator, ads-optimizer); read-only
lanes (researcher, reviewer) never fire; auto-spawned cards carry a body marker
that the gate checks FIRST, so a review never spawns a review-of-a-review.
Idempotent on the completed task id (idempotency_key + existing-child check),
so default_notify's per-chat fan-out and event redelivery spawn at most one
review and one progress message.
Config: kanban.on_complete_review {enabled, notify:{platform,chat_id,thread_id}}
in config.yaml (no new HERMES_* env var), resolved fresh each tick (live
reload), fails safe to OFF. Default disabled.
Built on top of feat/kanban-default-notify (PR #39) to avoid a write collision
on gateway/kanban_watchers.py. NOT merged to live-config.
20 new tests + 37 existing notifier/default-notify tests + 101 config tests
green. Live-gateway restart + real-Signal E2E deferred to after merge (running
gateway is on live-config without this code; arming now would be a no-op).
Patch note: ~/.hermes/plans/hermes-patches/kanban-on-complete-review.md
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements an on-completion adversarial review mechanism (Constitution Rule 2) that automatically spawns parented review cards and posts progress notifications when write-lane tasks are completed. The implementation includes loop-prevention guards, configuration parsing, and comprehensive tests. The reviewer feedback is highly actionable and suggests: ensuring spawned review tasks inherit the parent's workspace_path to avoid ValueError crashes for "dir" workspaces; safely invoking the new reaction method using getattr to prevent AttributeError in mocked environments; refactoring _qualifies_for_on_complete_review into a standalone helper; removing unused parameters; and gracefully handling None values for task.assignee in progress messages.
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.
Replace the in-trigger reviewer selection (_select_reviewer_for) and the write-lane allowlist (_WRITE_LANE_ASSIGNEES / _NON_TRIGGER_ASSIGNEES) with a single rule: every completed task spawns ONE decision card assigned to the orchestrator lane (kanban.orchestrator_profile, default 'orchestrator'), parented to the task. The orchestrator reads the completion handoff and ~/.hermes/constitution.md and decides the follow-up (review, babysit, verify, or nothing). Why: routing policy now lives in one place (the constitution) instead of split between Python and a reviewer's prompt. A brand-new lane is covered with zero code change. The marker-in-body loop guard is unchanged and is now the ONLY gate. Net -41 lines. Patch note: ~/.hermes/plans/hermes-patches/kanban-on-complete-orchestrator.md
exiao
changed the title
… params, getattr-safe call Gemini review (3 valid mediums on current code): - _qualifies_for_on_complete_review uses no instance state -> made it a module-level function (unit-tests directly, no AttributeError under a mocked runner). Tests import + call it bare. - _maybe_react_on_complete had unused params adapter / notifier_profile (it uses self.adapters.get(plat)) -> removed from signature and call site. - Call site now invokes via getattr(self, ..., None) guard, per the repo rule for newly-introduced methods on core objects. The HIGH comment (pass workspace_path for 'dir' workspaces) is moot: the orchestrator refactor spawns every decision card with workspace_kind='scratch', so no workspace_path is needed and the ValueError path can't be hit. 25 tests green (on-complete + notifier suites).
Gemini medium: f"(@{task.assignee})" rendered "(@None)" when a task had
no assignee. Use a conditional " (@x)" suffix that collapses to empty.
exiao
added a commit
that referenced
this pull request
Jun 30, 2026
…#72) * fix(kanban): retire on-complete orchestrator reaction (revert PR #43) PR #43 fired ONE "Decide next" orchestrator decision card on EVERY task completion (parented or not, decomposed or not), making ~340 of ~674 cards pure coordination tax. It fired blind (sees only "task X done", reconstructs the next step from a summary) and was redundant with the dependency engine that already auto-promotes children when parents finish. The official Kanban model has no on-completion hook: orchestration clusters around decomposition, the dependency graph advances work mechanically, and workers create/link follow-up cards themselves. The hook inverted "agents declare what's next" into "a cold orchestrator infers it after the fact" — off-grain. Replaced with instructions (constitution "Follow-on work" section + kanban-skill reference rewrite, landed first and approved by Eric), then removed the code: - gateway/kanban_watchers.py: dropped _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 cfg resolve, and the call site in the notifier completed branch. - hermes_cli/config.py: removed the kanban.on_complete_review DEFAULT_CONFIG block. - tests/gateway/test_kanban_on_complete_review.py: deleted (feature gone). Behavior-neutral in prod: the loop was already config-gated OFF (kanban.on_complete_review.enabled: false). The dependency engine and worker-side card creation carry follow-on work; nothing depended on the reaction. Patch note: ~/.hermes/plans/hermes-patches/retire-on-complete-review.md Task: t_710545d4 * fix(ci): stabilize ty lint-diff messages * fix(ci): normalize singular ty union-omission wording The lint_diff stable-message regex hard-coded plural 'union elements', so a 1<->2 omitted-count flip (which crosses ty's singular/plural boundary, '1 union element' vs '2 union elements') still leaked the count into the diff key and flagged a spurious fixed+new pair. Make the trailing 's' optional and cover the boundary with a test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
When any kanban task completes, the gateway spawns ONE decision card assigned to the orchestrator lane, parented to the completed task, and posts a short progress line to the configured "Kanban Master" Signal chat. The orchestrator reads the completion handoff +
~/.hermes/constitution.mdand decides the follow-up: review, babysit a PR, verify a feature, or nothing.Design (per Eric: keep it simple)
Every completed ticket ALWAYS goes to the orchestrator; one agent decides what's next. There is no in-trigger reviewer selection and no write-lane allowlist — routing policy lives in the constitution, one source of truth, so a brand-new lane is covered with zero code change.
_orchestrator_assignee()resolveskanban.orchestrator_profile(defaultorchestrator)._qualifies_for_on_complete_review()(module-level, no instance state) passes every completion EXCEPT a card this automation spawned (the_AUTO_REACTION_MARKERbody stamp). That marker is the only gate and the loop guard._maybe_react_on_complete()spawns one parented, goal-mode decision card in ascratchworkspace (idempotency keyon-complete-decision:<id>), then posts progress. Best-effort send; a notify failure never undoes the card. The call site is invoked via agetattrguard.Config
kanban.on_complete_review.enabled(default false) +notify: {platform, chat_id, thread_id}. Read fresh each tick — flip it with no restart.Tests
19 in the on-complete file (resolver, orchestrator-assignee, the marker loop-guard, integration tests driving the real notifier mixin against a temp board), 25 green across the on-complete + notifier suites.
Review addressed (Gemini)
workspace_kind="scratch"._qualifies_for_on_complete_review→ module-level function (no instance state, unit-tests directly).adapter/notifier_profileparams; call site uses agetattrguard.Noneassignee in the progress message ((@None)→ empty).Stacking
Built on
feat/kanban-default-notify(#39, board-wide auto-subscribe). Merge #39 first.Out of scope (separate PR)
Pre-existing duplicate
"kanban"key in DEFAULT_CONFIG shadowsauto_subscribe_on_create— flagged, left for a focused fix.