diff --git a/hermes_cli/kanban_completion_gates.py b/hermes_cli/kanban_completion_gates.py index 9b4d84c37eca..ab28c9399296 100644 --- a/hermes_cli/kanban_completion_gates.py +++ b/hermes_cli/kanban_completion_gates.py @@ -11,11 +11,13 @@ state is unchanged on rejection and the worker can simply retry with corrected output. -Three gates ship today (Tranche 1 of v6.7, closes #28, #62, #64): +Gates so far (v6.7 + v6.8, closing #28, #62, #64, #73, #74, #77, #78, #79): 1. :func:`verify_runtime_floor` — per-role floor on ``completed_at - started_at``. Catches Tony's 20-second "approve" verdicts - and Friday's 59-second "implemented 7 dispatcher gates" claims. + and Friday's 59-second "implemented 7 dispatcher gates" claims. v6.8 + adds an honest-reject bypass (#74) and progressive message escalation + (#77/#78) so workers stop bare-retrying the same payload. 2. :func:`verify_workspace_diff` — when a non-review worker on a ``dir`` / ``worktree`` workspace claims to have produced code, the workspace @@ -28,6 +30,18 @@ files with no extension and no shebang — the "all prior block evidence files" failure mode). +4. :func:`verify_reviewer_fields` — reviewers must produce a structured + ``verdict:`` line plus the supporting evidence / test_quality bullets + before completion writes. v6.8 accepts bullet-form ``not_applicable`` + with a ≥20-char reason. + +5. :func:`verify_umbrella_review_coverage` (#79) — a goal-mode task + assigned to an orchestration role (jarvis / pepper / banner) cannot + complete until at least one review-role descendant exists somewhere + in its transitive subtree. Forces the umbrella to keep_running until + reviewers spawn — prevents the 2026-06-10 "JARVIS spawns Pepper + + Friday then approves itself before any reviewer queues" case. + See hermes-jarvis#61 for the bootstrap-paradox case study that motivates these gates. """ @@ -1050,6 +1064,127 @@ def verify_doc_drift( ) +# ===================================================================== +# Umbrella review-coverage gate (#79) +# ===================================================================== + + +@dataclass(frozen=True) +class MissingUmbrellaReviewViolation: + umbrella_id: str + has_non_review_descendant: bool + + def message(self) -> str: + if not self.has_non_review_descendant: + # Pathological: a goal-mode umbrella with no descendants + # at all. The orchestrator never decomposed. + return ( + f"umbrella-review-coverage: this goal-mode umbrella " + f"({self.umbrella_id}) has no descendants at all. " + f"A `--goal` umbrella that completes without ever " + f"spawning child tasks is almost always a misfire — " + f"either decompose the work into children first, or, " + f"if this umbrella legitimately has no actionable " + f"sub-tasks, set " + f"metadata={{\"x_umbrella_no_review\": \"<≥20-char " + f"reason — e.g. 'pure status-only ack of upstream " + f"completion'>\"}}." + ) + return ( + f"umbrella-review-coverage: this goal-mode umbrella " + f"({self.umbrella_id}) has build-role descendants but NO " + f"review-role descendants (tony / tchalla / vision / " + f"reviewer) anywhere in its task_links subtree. " + f"Marvel-swarm chains call for at least one per-block " + f"review before the umbrella archives. Either spawn the " + f"review task(s) now via kanban_create (--parent --assignee tony/tchalla/vision), THEN re-call " + f"kanban_complete, or — if this work genuinely doesn't " + f"need review (rare; usually a misfire) — set " + f"metadata={{\"x_umbrella_no_review\": \"<≥20-char " + f"reason>\"}}.\n" + f"This catches the 2026-06-10 validation case where JARVIS " + f"spawned Pepper+Friday and exited done without queuing " + f"any reviewer, leaving Kaipo to hand-spawn the rest of " + f"the chain. See hermes-jarvis#79." + ) + + +def verify_umbrella_review_coverage( + is_goal_mode: bool, + umbrella_assignee: Optional[str], + umbrella_id: str, + descendants: list, # rows from _v6_7_walk_descendants + *, + allow_no_review_needed: bool = False, +) -> Optional[MissingUmbrellaReviewViolation]: + """Reject an orchestration umbrella's kanban_complete if its + descendant subtree contains no review-role tasks. + + Closes hermes-jarvis#79. The 2026-06-10 v6.7 validation chain on + hermes-dashboard had JARVIS spawn Pepper + Friday from the + umbrella, then call kanban_complete on itself before any reviewer + was queued. The umbrella showed as done with build-only + descendants; Kaipo had to manually spawn Tony / Tchalla / Vision + to continue the chain. This gate forces orchestration umbrellas + to keep_running until at least one review task exists in their + subtree. + + Skipped (returns None) when: + - The task is not goal_mode (avoids per-call CTE on the 99% case). + - The task's assignee is NOT in ORCHESTRATION_ROLES (i.e., a + goal-loop Friday/Tony/etc. shouldn't be subject to this + umbrella discipline — only JARVIS/Pepper/Banner). + - At least one descendant has a review-role assignee. **Status + doesn't matter** — a queued / blocked / running review still + proves the orchestrator decomposed correctly. The integrative- + review-at-archive gate (#30) handles terminal-status discipline + at the other end of the lifecycle. + - Opt-out via ``allow_no_review_needed=True``. + + Differs from the v6.7 #30 integrative-review-at-archive gate: + - #30 fires at ARCHIVE time and spawns Tchalla after the chain + settles. The integrative review is intentionally NOT linked + into ``task_links`` (see ``_v6_7_spawn_integrative_review`` — + "Intentionally NO parents" to avoid the parents-not-done + deadlock), so it does NOT satisfy this #79 gate. #79 requires + a PER-BLOCK review (tony/tchalla/vision in the build subtree). + - #79 fires at COMPLETE time and forces JARVIS to spawn + reviewers BEFORE marking itself done — catching the empty- + chain misfire one step earlier in the orchestration lifecycle. + """ + if allow_no_review_needed: + return None + if not is_goal_mode: + return None + # Self-review fix: tighten to orchestration roles only so a + # goal-loop Friday/Tony doesn't get caught by an umbrella gate. + # The 2026-06-10 case was specifically a JARVIS misfire. + if not umbrella_assignee or umbrella_assignee.lower() not in ORCHESTRATION_ROLES: + return None + if not descendants: + return MissingUmbrellaReviewViolation( + umbrella_id=umbrella_id, has_non_review_descendant=False, + ) + has_review = False + for ch in descendants: + assignee = (ch["assignee"] or "").lower() + if assignee in REVIEW_ROLES: + has_review = True + break # one is enough to satisfy the gate + if has_review: + return None + # No review descendants — fail. The umbrella does have other + # children; the message specializes between "no descendants at + # all" and "build-only descendants" based on count, not the + # has_non_review flag (which would lie if descendants were + # all empty-assignee entries). + return MissingUmbrellaReviewViolation( + umbrella_id=umbrella_id, + has_non_review_descendant=bool(descendants), + ) + + # ===================================================================== # Exception class for the integration in `complete_task` # ===================================================================== diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index 6c51263735c1..f53fc2728267 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -98,6 +98,7 @@ verify_pr_urls_exist, verify_reviewer_fields, verify_runtime_floor, + verify_umbrella_review_coverage, verify_workspace_diff, ) @@ -3673,7 +3674,7 @@ def _v6_7_run_completion_gates( - ``x_no_reviewer_fields`` — skip reviewer-fields (#29, #31) """ row = conn.execute( - "SELECT assignee, body, tenant, workspace_kind, workspace_path, started_at " + "SELECT assignee, body, tenant, workspace_kind, workspace_path, started_at, goal_mode " " FROM tasks WHERE id = ?", (task_id,), ).fetchone() @@ -3686,6 +3687,11 @@ def _v6_7_run_completion_gates( no_rf_reason = _validate_opt_out(task_id, "x_no_reviewer_fields", md.get("x_no_reviewer_fields")) phantom_ok_reason = _validate_opt_out(task_id, "x_phantom_pr_ok", md.get("x_phantom_pr_ok")) doc_drift_reason = _validate_opt_out(task_id, "x_doc_drift_ok", md.get("x_doc_drift_ok")) + # hermes-jarvis#79: opt-out for goal-mode umbrellas that + # legitimately don't need any per-block review. + umbrella_no_review_reason = _validate_opt_out( + task_id, "x_umbrella_no_review", md.get("x_umbrella_no_review"), + ) accepted_opt_outs = { k: v for k, v in ( ("x_fast_justified", fast_ok_reason), @@ -3694,6 +3700,7 @@ def _v6_7_run_completion_gates( ("x_no_reviewer_fields", no_rf_reason), ("x_phantom_pr_ok", phantom_ok_reason), ("x_doc_drift_ok", doc_drift_reason), + ("x_umbrella_no_review", umbrella_no_review_reason), ) if v is not None } if accepted_opt_outs: @@ -3767,6 +3774,24 @@ def _v6_7_run_completion_gates( ) if drift is not None: violations.append(drift) + # hermes-jarvis#79: goal-mode umbrellas must have at least one + # review-role descendant before they can complete. Reuses + # _v6_7_walk_descendants from #73 — keeps the transitive-walk + # behavior consistent across the integrative-review-at-archive + # gate (#30) and this complete-time check. + no_review_needed = umbrella_no_review_reason is not None + is_goal_mode = bool(row["goal_mode"]) + if is_goal_mode and not no_review_needed: + descendants = _v6_7_walk_descendants(conn, task_id) + umbrella_review = verify_umbrella_review_coverage( + is_goal_mode=is_goal_mode, + umbrella_assignee=row["assignee"], + umbrella_id=task_id, + descendants=descendants, + allow_no_review_needed=no_review_needed, + ) + if umbrella_review is not None: + violations.append(umbrella_review) return violations diff --git a/tests/cli/test_kanban_completion_gates.py b/tests/cli/test_kanban_completion_gates.py index 4fa35c1a999d..14e5de336e36 100644 --- a/tests/cli/test_kanban_completion_gates.py +++ b/tests/cli/test_kanban_completion_gates.py @@ -2152,3 +2152,323 @@ def test_escalated_message_clamps_negative_seconds_remaining( # Should read "Wait 0s", not "Wait -5s" assert "Wait 0s" in msg or "Wait 0 " in msg assert "-" not in msg.split("Wait")[1].split("s")[0] + + +# ===================================================================== +# hermes-jarvis#79 — umbrella review-coverage gate +# ===================================================================== + + +class TestUmbrellaReviewCoverage: + """Unit tests for verify_umbrella_review_coverage (#79). The + function is pure — pass in descendants + is_goal_mode and it + returns a violation if the goal-mode umbrella has no review-role + descendants.""" + + @staticmethod + def _row(assignee, id_="x", status="done"): + # Simulate a sqlite3.Row-like mapping + return {"id": id_, "assignee": assignee, "status": status, "title": ""} + + def test_non_goal_mode_skips(self) -> None: + from hermes_cli.kanban_completion_gates import verify_umbrella_review_coverage + v = verify_umbrella_review_coverage( + is_goal_mode=False, umbrella_assignee="jarvis", umbrella_id="t_u", + descendants=[self._row("friday")], + ) + assert v is None + + def test_goal_mode_with_review_descendant_passes(self) -> None: + from hermes_cli.kanban_completion_gates import verify_umbrella_review_coverage + v = verify_umbrella_review_coverage( + is_goal_mode=True, umbrella_assignee="jarvis", umbrella_id="t_u", + descendants=[self._row("friday"), self._row("tony")], + ) + assert v is None + + def test_goal_mode_with_only_build_descendants_rejects(self) -> None: + from hermes_cli.kanban_completion_gates import ( + MissingUmbrellaReviewViolation, verify_umbrella_review_coverage, + ) + v = verify_umbrella_review_coverage( + is_goal_mode=True, umbrella_assignee="jarvis", umbrella_id="t_u", + descendants=[self._row("friday"), self._row("pepper")], + ) + assert isinstance(v, MissingUmbrellaReviewViolation) + assert v.has_non_review_descendant is True + msg = v.message() + assert "no review-role descendants" in msg.lower() or "no review" in msg.lower() + # Body suggests the kanban_create command shape + assert "kanban_create" in msg + + def test_goal_mode_with_no_descendants_rejects(self) -> None: + """A goal-mode umbrella that never spawned children is almost + always a misfire; gate forces decomposition or an explicit + opt-out.""" + from hermes_cli.kanban_completion_gates import ( + MissingUmbrellaReviewViolation, verify_umbrella_review_coverage, + ) + v = verify_umbrella_review_coverage( + is_goal_mode=True, umbrella_assignee="jarvis", umbrella_id="t_u", + descendants=[], + ) + assert isinstance(v, MissingUmbrellaReviewViolation) + assert v.has_non_review_descendant is False + assert "no descendants at all" in v.message() + + def test_tchalla_descendant_satisfies_gate(self) -> None: + from hermes_cli.kanban_completion_gates import verify_umbrella_review_coverage + v = verify_umbrella_review_coverage( + is_goal_mode=True, umbrella_assignee="jarvis", umbrella_id="t_u", + descendants=[self._row("friday"), self._row("tchalla")], + ) + assert v is None + + def test_vision_descendant_satisfies_gate(self) -> None: + from hermes_cli.kanban_completion_gates import verify_umbrella_review_coverage + v = verify_umbrella_review_coverage( + is_goal_mode=True, umbrella_assignee="jarvis", umbrella_id="t_u", + descendants=[self._row("friday"), self._row("vision")], + ) + assert v is None + + def test_opt_out_bypasses_gate(self) -> None: + from hermes_cli.kanban_completion_gates import verify_umbrella_review_coverage + v = verify_umbrella_review_coverage( + is_goal_mode=True, umbrella_assignee="jarvis", umbrella_id="t_u", + descendants=[self._row("friday")], + allow_no_review_needed=True, + ) + assert v is None + + # Scope tightening tests (self-review fix): gate applies ONLY to + # orchestration roles (jarvis/pepper/banner). Goal-mode workers + # like Friday are out of scope. + + def test_friday_goal_mode_umbrella_skipped(self) -> None: + """A goal-mode task assigned to Friday should NOT be subject to + the umbrella-review gate — Friday is a worker, not an + orchestrator, and the review umbrella belongs to her parent.""" + from hermes_cli.kanban_completion_gates import verify_umbrella_review_coverage + v = verify_umbrella_review_coverage( + is_goal_mode=True, umbrella_assignee="friday", umbrella_id="t_u", + descendants=[], + ) + assert v is None + + def test_tony_goal_mode_skipped(self) -> None: + """Review-role goal-mode tasks are also out of scope.""" + from hermes_cli.kanban_completion_gates import verify_umbrella_review_coverage + v = verify_umbrella_review_coverage( + is_goal_mode=True, umbrella_assignee="tony", umbrella_id="t_u", + descendants=[], + ) + assert v is None + + def test_empty_assignee_skipped(self) -> None: + """Tasks with no assignee can't be an orchestration umbrella.""" + from hermes_cli.kanban_completion_gates import verify_umbrella_review_coverage + v = verify_umbrella_review_coverage( + is_goal_mode=True, umbrella_assignee=None, umbrella_id="t_u", + descendants=[], + ) + assert v is None + + def test_pepper_umbrella_in_scope(self) -> None: + """Pepper is an orchestration role — also subject to the gate.""" + from hermes_cli.kanban_completion_gates import ( + MissingUmbrellaReviewViolation, verify_umbrella_review_coverage, + ) + v = verify_umbrella_review_coverage( + is_goal_mode=True, umbrella_assignee="pepper", umbrella_id="t_u", + descendants=[self._row("friday")], + ) + assert isinstance(v, MissingUmbrellaReviewViolation) + + def test_banner_umbrella_in_scope(self) -> None: + from hermes_cli.kanban_completion_gates import ( + MissingUmbrellaReviewViolation, verify_umbrella_review_coverage, + ) + v = verify_umbrella_review_coverage( + is_goal_mode=True, umbrella_assignee="banner", umbrella_id="t_u", + descendants=[self._row("friday")], + ) + assert isinstance(v, MissingUmbrellaReviewViolation) + + def test_assignee_case_insensitive(self) -> None: + """Assignee comparison is case-insensitive, matching the rest + of the gate module.""" + from hermes_cli.kanban_completion_gates import ( + MissingUmbrellaReviewViolation, verify_umbrella_review_coverage, + ) + v = verify_umbrella_review_coverage( + is_goal_mode=True, umbrella_assignee="JARVIS", umbrella_id="t_u", + descendants=[self._row("friday")], + ) + assert isinstance(v, MissingUmbrellaReviewViolation) + + def test_review_in_blocked_status_still_counts(self) -> None: + """ANY review descendant satisfies the gate, regardless of + status (queued / running / blocked / done) — the gate is + about the *intent* to review, not the review's progress.""" + from hermes_cli.kanban_completion_gates import verify_umbrella_review_coverage + v = verify_umbrella_review_coverage( + is_goal_mode=True, umbrella_assignee="jarvis", umbrella_id="t_u", + descendants=[ + self._row("friday", status="done"), + self._row("tony", status="blocked"), + ], + ) + assert v is None + + def test_deep_transitive_review_descendant_counts(self) -> None: + """The transitive walk (#73) means a reviewer 4 levels deep + still satisfies the gate. This is a pure-function test — the + descendants list is already flattened by the caller.""" + from hermes_cli.kanban_completion_gates import verify_umbrella_review_coverage + v = verify_umbrella_review_coverage( + is_goal_mode=True, umbrella_assignee="jarvis", umbrella_id="t_u", + descendants=[ + self._row("friday", id_="t_c1"), + self._row("pepper", id_="t_c2"), + self._row("friday", id_="t_gc1"), + self._row("tchalla", id_="t_ggc1"), # deep reviewer + ], + ) + assert v is None + + def test_multi_children_with_multiple_reviews_passes(self) -> None: + from hermes_cli.kanban_completion_gates import verify_umbrella_review_coverage + v = verify_umbrella_review_coverage( + is_goal_mode=True, umbrella_assignee="jarvis", umbrella_id="t_u", + descendants=[ + self._row("friday"), self._row("shuri"), + self._row("tony"), self._row("tchalla"), self._row("vision"), + ], + ) + assert v is None + + +class TestUmbrellaReviewCoverageIntegration: + """End-to-end via complete_task. The 2026-06-10 case: JARVIS + spawned Pepper+Friday from the umbrella, then called + kanban_complete on itself before any reviewer was queued.""" + + @pytest.fixture + def umbrella_with_only_build(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setenv("HERMES_KANBAN_DB", str(tmp_path / "k.db")) + conn = kb.connect(board="default") + import time + now = int(time.time()) + # Umbrella with goal_mode=1, started long ago so floor doesn't fire + conn.execute( + "INSERT INTO tasks (id, title, status, assignee, goal_mode, " + " started_at, created_at, workspace_kind, workspace_path) " + "VALUES ('t_umb', 'umbrella', 'running', 'jarvis', 1, ?, ?, " + " 'scratch', NULL)", + (now - 1000, now), + ) + # Friday child (build) + conn.execute( + "INSERT INTO tasks (id, title, status, assignee, goal_mode, " + " started_at, created_at, workspace_kind, workspace_path) " + "VALUES ('t_fr', 'Friday', 'done', 'friday', 0, ?, ?, " + " 'scratch', NULL)", + (now - 500, now), + ) + conn.execute( + "INSERT INTO task_links (parent_id, child_id) VALUES ('t_umb', 't_fr')", + ) + conn.commit() + yield conn + conn.close() + + def test_jarvis_umbrella_blocks_without_review_descendant( + self, umbrella_with_only_build, + ) -> None: + """The 2026-06-10 case: build-only umbrella tries to complete. + Should be rejected by the new gate.""" + with pytest.raises(kb.CompletionGateError) as excinfo: + kb.complete_task( + umbrella_with_only_build, "t_umb", + summary="decomposed", + result="Decomposed v6.7 widget chain into Pepper + Friday.", + ) + kinds = {type(v).__name__ for v in excinfo.value.violations} + assert "MissingUmbrellaReviewViolation" in kinds + + def test_jarvis_umbrella_passes_when_review_exists( + self, umbrella_with_only_build, + ) -> None: + """Once a tony descendant is added (whether as direct child + or chained off Friday), the gate passes.""" + import time + # Chained tony off friday (the validation chain's actual shape) + umbrella_with_only_build.execute( + "INSERT INTO tasks (id, title, status, assignee, goal_mode, " + " started_at, created_at, workspace_kind, workspace_path) " + "VALUES ('t_tn', 'Tony', 'done', 'tony', 0, ?, ?, 'scratch', NULL)", + (int(time.time()) - 100, int(time.time())), + ) + umbrella_with_only_build.execute( + "INSERT INTO task_links (parent_id, child_id) VALUES ('t_fr', 't_tn')", + ) + umbrella_with_only_build.commit() + ok = kb.complete_task( + umbrella_with_only_build, "t_umb", + summary="decomposed + reviewed", + result="All children terminal.", + ) + assert ok + + def test_x_umbrella_no_review_opt_out_passes( + self, umbrella_with_only_build, + ) -> None: + """An umbrella that legitimately doesn't need review can + opt out with an audit reason.""" + ok = kb.complete_task( + umbrella_with_only_build, "t_umb", + summary="status ack", + result="Acknowledged upstream completion; no review needed.", + metadata={"x_umbrella_no_review": + "pure status ack of upstream complete, no actionable subtasks"}, + ) + assert ok + + def test_opt_out_with_short_reason_rejected( + self, umbrella_with_only_build, + ) -> None: + """Opt-out reason must be ≥20 chars like other v6.7/v6.8 + opt-outs.""" + with pytest.raises(kb.InvalidOptOutError): + kb.complete_task( + umbrella_with_only_build, "t_umb", + summary="x", result="x", + metadata={"x_umbrella_no_review": "ok"}, + ) + + def test_non_goal_mode_task_unaffected( + self, tmp_path, monkeypatch, + ) -> None: + """A plain (non-goal_mode) task with no descendants completes + fine — the gate only fires on goal_mode umbrellas.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setenv("HERMES_KANBAN_DB", str(tmp_path / "k.db")) + conn = kb.connect(board="default") + import time + now = int(time.time()) + conn.execute( + "INSERT INTO tasks (id, title, status, assignee, goal_mode, " + " started_at, created_at, workspace_kind, workspace_path) " + "VALUES ('t_plain', 'plain', 'running', 'jarvis', 0, ?, ?, " + " 'scratch', NULL)", + (now - 1000, now), + ) + conn.commit() + ok = kb.complete_task( + conn, "t_plain", + summary="done", result="all good", + ) + assert ok + conn.close()