Skip to content

fix(kanban): gate children on explicit approval - #67156

Open
peacockesq wants to merge 1 commit into
NousResearch:mainfrom
peacockesq:fix/kanban-approval-gate-67132
Open

peacockesq wants to merge 1 commit into
NousResearch:mainfrom
peacockesq:fix/kanban-approval-gate-67132

Conversation

@peacockesq

Copy link
Copy Markdown

Summary

Fixes #67132.

Kanban now supports an explicit approval contract on selected parent→child dependency edges. Untyped edges preserve the existing done/archived semantics, while approval-gated edges fail closed until the parent has a terminal completed run with unambiguous structured approved: true metadata.

This prevents rejected or ambiguous reviews from releasing candidate/release children without redefining ordinary dependencies or blocking remediation fan-out.

What changed

  • Add additive, idempotent task_links.gate_type SQLite migration (NULL keeps legacy behavior).
  • Add approval_parents to task creation and gate_type="approval" to link creation across DB, CLI, model tool, and dashboard API surfaces.
  • Centralize dependency evaluation in dependency_blockers() and enforce it in:
    • create/link/recompute
    • atomic claim immediately before spawn
    • manual promote (approval gates cannot be force-bypassed)
    • unblock and dashboard/bulk ready transitions
    • stale/manual reclaim paths
  • Validate approval from structured terminal-run metadata only; never parse titles, comments, summaries, or prose verdicts.
  • Fail closed for rejected, missing, malformed, non-boolean, identity-ambiguous, or contradictory verdict metadata.
  • Emit deduplicated, sanitized approval_gate_held events with only parent ID, gate type, and gate state.
  • Surface typed parent-edge state in dashboard task detail responses.

Compatibility

  • Existing databases migrate additively.
  • Existing untyped links remain unchanged.
  • Approved archived review parents remain satisfied.
  • Untyped remediation children still run after a rejected review.
  • No dependencies or runtime configuration changed.

RED proof on pre-fix main

A temporary DB with a proposed typed marker reproduced the bug before implementation:

before_parent_completion=todo
after_rejected_parent_completion=ready
claim_created=True
child_run_count=1

Verification

All commands were run after rebasing onto upstream main at 73e32f37e7bde4f5528ab04b5b6356bcf00d0af0 and with Kanban board environment pins removed so tests used temporary databases.

315 passed
  tests/hermes_cli/test_kanban_approval_gates.py
  tests/hermes_cli/test_kanban_cli.py
  tests/tools/test_kanban_tools.py
  tests/plugins/test_kanban_dashboard_plugin.py
  tests/hermes_cli/test_kanban_promote.py

447 passed
  tests/hermes_cli/test_kanban_db.py
  tests/hermes_cli/test_kanban_core_functionality.py
  tests/hermes_cli/test_kanban_goal_mode.py
  tests/hermes_cli/test_kanban_dispatch_lock.py
  tests/hermes_cli/test_kanban_block_kinds.py
  tests/plugins/test_kanban_worker_runs.py

69 passed
  migration-lock, reclaim-lock, sticky-block, lifecycle-hook,
  decomposition/default-assignee, and gateway dispatcher/watcher suites

python -m ruff check <changed files>
All checks passed!

git diff --check
passed

python -m ty check tests/hermes_cli/test_kanban_approval_gates.py
All checks passed!

The repository-wide type checker still reports existing diagnostics in pre-existing Kanban files outside this change (for example the known judge_goal tuple mismatch); this PR does not expand that unrelated scope.

Safety

  • No installed Hermes upgrade/restart.
  • No live schema rollout.
  • No dependency additions.

@alt-glitch alt-glitch added type/bug Something isn't working comp/cron Cron scheduler and job management comp/plugins Plugin system and bundled plugins comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have labels Jul 18, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused typed-edge implementation. The current-main premise is confirmed: recompute_ready promotes solely from done/archived parent status (hermes_cli/kanban_db.py:3441-3475), and claim_task enforces that same status-only condition (hermes_cli/kanban_db.py:3500-3524).

Problems

  • The new fail-closed gate is not communicated to the parent reviewer. The worker context only documents task/upstream-parent handoff data (hermes_cli/kanban_db.py:8421-8438, hermes_cli/kanban_db.py:8549-8566), while kanban_complete presents metadata as free-form facts (tools/kanban_tools.py:1424-1465). A reviewer can therefore complete normally without approved: true, permanently holding the release child. The new approval tests inject metadata directly into complete_task, so they do not cover this worker-facing path.

Suggested changes

  • Surface outgoing approval-gated children in the parent worker context and state the exact metadata.approved boolean contract in kanban_complete; add an end-to-end handler/context test for approved and missing-approval completions.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 19, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary

One PR addresses issue #67132. #67156 replaces status-only release for explicitly typed approval dependencies with fail-closed structured approval checks across recompute, claim, promotion, reclaim, CLI, tool, and dashboard paths, while intentionally retaining legacy behavior for untyped dependencies.

Related pull requests

  • fix(kanban): gate children on explicit approval #67156 best fix — (+970/-127) — n/a: The diff adds an additive task_links.gate_type migration, centralized dependency-blocker evaluation, enforcement across runnable-state transitions, sanitized diagnostics, and tests for rejected, ambiguous, mixed-fan-in, race, and reclaim cases. Consistent with the MAINTAINER-BOT keep_open review, the salvage gap is that the parent worker context and kanban_complete interface do not communicate the required metadata.approved boolean contract or test that worker-facing path end to end; the non-contributor report also identifies a separate untyped auto-decomposition NO-GO case outside this explicit-edge design.

Suggested consolidation

Keep #67156 open with a salvage path: retain the typed-edge schema, centralized blocker enforcement, diagnostics, and coverage, then expose outgoing approval-gated children and the exact metadata.approved contract in parent worker context and kanban_complete, with end-to-end tests for approved and missing-approval completions. Handle the untyped auto-decomposition NO-GO case separately rather than changing the backward-compatible semantics of ordinary edges; there are no competing PRs to close as duplicates.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I67132(["issue #67132 (open)"])
    P67156["PR #67156 (open)"]
    P67156 -->|best fix| I67132
    class I67132 open
    class P67156 open
    class P67156 best
    class P67156 target
    click I67132 "https://github.com/NousResearch/hermes-agent/issues/67132"
    click P67156 "https://github.com/NousResearch/hermes-agent/pull/67156"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 59 kB of PR diffs, 7 kB of issue/PR text, 2 kB of discussion (2 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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 comp/plugins Plugin system and bundled plugins comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Kanban approval-gated children promote after rejected review parent completes

4 participants