From 4069657183b8c77100689c90982582decc418a73 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Thu, 11 Jun 2026 13:04:53 -0700 Subject: [PATCH 1/2] =?UTF-8?q?feat(kanban):=20v6.8=20Part=204=20=E2=80=94?= =?UTF-8?q?=20umbrella=20keep=5Frunning=20until=20reviewers=20spawn=20(#79?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 2026-06-10 v6.7 validation chain caught this orchestration bug: JARVIS umbrella spawned Pepper + Friday from the umbrella, then called 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. Closes hermes-jarvis#79. ## What this PR adds New gate ``verify_umbrella_review_coverage`` that fires when a goal_mode umbrella calls kanban_complete. The gate walks the umbrella's transitive descendants (reusing _v6_7_walk_descendants from #73 so chained shapes count). If no review-role descendant exists anywhere in the subtree, the gate rejects. Two violation messages: 1. ``no descendants at all`` — pathological goal-mode umbrella that never decomposed. The orchestrator misfired. 2. ``build-role descendants but NO review-role descendants`` — the exact 2026-06-10 case. Message points the operator to spawn tony/tchalla/vision now via kanban_create --parent . Opt-out: ``metadata={"x_no_review_needed": "<≥20-char reason>"}`` for legitimate cases (e.g. pure status-ack umbrellas). Standard v6.7/v6.8 opt-out conventions — emits completion_opt_out_used audit event with verbatim reason; bool/short strings rejected. ## Relationship to v6.7 #30 - #30 (integrative review at archive): fires at ARCHIVE time, spawns Tchalla after a chain settles. - #79 (this PR): fires at COMPLETE time, forces JARVIS to spawn reviewers BEFORE marking itself done. Together they catch the empty-chain failure mode at both ends of the lifecycle. ## Tests 12 new tests: TestUmbrellaReviewCoverage (7) — non_goal skips, review descendant passes, only-build rejects with helpful message, no-descendants rejects with different message, tchalla/vision also satisfy, opt-out bypasses. TestUmbrellaReviewCoverageIntegration (5) — end-to-end via complete_task: build-only blocks, adding chained tony unblocks (closes #73 + #79 together), x_no_review_needed passes, short opt-out rejected, non-goal_mode tasks unaffected. 135 in test_kanban_completion_gates.py pass. 231/231 across full v6.7+v6.8 + adjacent regression set, zero failures. Co-Authored-By: Claude Opus 4.7 (1M context) --- hermes_cli/kanban_completion_gates.py | 103 +++++++++++ hermes_cli/kanban_db.py | 26 ++- tests/cli/test_kanban_completion_gates.py | 213 ++++++++++++++++++++++ 3 files changed, 341 insertions(+), 1 deletion(-) diff --git a/hermes_cli/kanban_completion_gates.py b/hermes_cli/kanban_completion_gates.py index 9b4d84c37eca..f869e5d2493f 100644 --- a/hermes_cli/kanban_completion_gates.py +++ b/hermes_cli/kanban_completion_gates.py @@ -1050,6 +1050,109 @@ 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_no_review_needed\": \"<≥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_no_review_needed\": \"<≥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_id: str, + descendants: "list", # rows from _v6_7_walk_descendants + *, + allow_no_review_needed: bool = False, +) -> Optional[MissingUmbrellaReviewViolation]: + """Reject a goal-mode 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 the JARVIS umbrella to + keep_running until at least one review task exists in its + subtree. + + Skipped (returns None) when: + - The task is not goal_mode (orchestration umbrellas always run + with --goal; non-goal tasks don't need this discipline). + - At least one descendant has a review-role assignee. + - Opt-out via ``allow_no_review_needed=True``. + + Differs from the v6.7 #30 integrative-review-at-archive gate: + that gate fires at ARCHIVE time and spawns Tchalla after a chain + settles. This gate 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 + if not descendants: + return MissingUmbrellaReviewViolation( + umbrella_id=umbrella_id, has_non_review_descendant=False, + ) + has_review = False + has_non_review = False + for ch in descendants: + assignee = (ch["assignee"] or "").lower() + if assignee in REVIEW_ROLES: + has_review = True + elif assignee: # any non-empty non-review assignee + has_non_review = True + if has_review: + return None + # No review descendants — fail. Surface whether there's at least + # one build descendant so the message can specialize. + return MissingUmbrellaReviewViolation( + umbrella_id=umbrella_id, + has_non_review_descendant=has_non_review, + ) + + # ===================================================================== # Exception class for the integration in `complete_task` # ===================================================================== diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index 6c51263735c1..ef0fc5e4449b 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. + no_review_needed_reason = _validate_opt_out( + task_id, "x_no_review_needed", md.get("x_no_review_needed"), + ) 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_no_review_needed", no_review_needed_reason), ) if v is not None } if accepted_opt_outs: @@ -3767,6 +3774,23 @@ 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 = no_review_needed_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_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..40c580b2eb84 100644 --- a/tests/cli/test_kanban_completion_gates.py +++ b/tests/cli/test_kanban_completion_gates.py @@ -2152,3 +2152,216 @@ 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_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_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_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_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_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_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_id="t_u", + descendants=[self._row("friday")], + allow_no_review_needed=True, + ) + 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_no_review_needed_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_no_review_needed": + "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_no_review_needed": "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() From cc502e3dd6c6f7f77fc44ffdce173256a5869e17 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Thu, 11 Jun 2026 13:17:02 -0700 Subject: [PATCH 2/2] fix(kanban): self-review fixes for v6.8 Part 4 umbrella gate (#79) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from the independent code review on PR #19: 1. Scope tightening: verify_umbrella_review_coverage now requires umbrella_assignee in ORCHESTRATION_ROLES (jarvis/pepper/banner). A goal-mode worker like Friday is out of scope — only orchestrators own umbrella-decomposition discipline. Was over-applying. 2. Empty-assignee message correctness: has_non_review_descendant is now bool(descendants), not the unset has_non_review flag. Prevents the "no descendants at all" message lying when descendants exist but all have empty assignees. Also added break after first review match for early termination. 3. Opt-out rename: x_no_review_needed → x_umbrella_no_review across gate messages, kanban_db opt-out validation, accepted_opt_outs audit entry, and tests. Avoids future collision with x_no_reviewer_fields (reviewer-field opt-out). Plus 8 new unit tests covering the scope tightening (friday/tony skipped, empty-assignee skipped, pepper/banner in scope, case- insensitive matching) and edge cases the reviewer flagged (blocked-status review counts, deep transitive review, multi-children multi-reviews). Module docstring updated: was "Three gates ship today (Tranche 1 of v6.7)" — now reflects the v6.7+v6.8 gate roster (5 gates, closing 7 issues). All 144 gate tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- hermes_cli/kanban_completion_gates.py | 74 +++++++++---- hermes_cli/kanban_db.py | 9 +- tests/cli/test_kanban_completion_gates.py | 127 ++++++++++++++++++++-- 3 files changed, 175 insertions(+), 35 deletions(-) diff --git a/hermes_cli/kanban_completion_gates.py b/hermes_cli/kanban_completion_gates.py index f869e5d2493f..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. """ @@ -1072,7 +1086,7 @@ def message(self) -> str: f"either decompose the work into children first, or, " f"if this umbrella legitimately has no actionable " f"sub-tasks, set " - f"metadata={{\"x_no_review_needed\": \"<≥20-char " + f"metadata={{\"x_umbrella_no_review\": \"<≥20-char " f"reason — e.g. 'pure status-only ack of upstream " f"completion'>\"}}." ) @@ -1087,7 +1101,7 @@ def message(self) -> str: f"task-id> --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_no_review_needed\": \"<≥20-char " + 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 " @@ -1098,12 +1112,13 @@ def message(self) -> str: def verify_umbrella_review_coverage( is_goal_mode: bool, + umbrella_assignee: Optional[str], umbrella_id: str, - descendants: "list", # rows from _v6_7_walk_descendants + descendants: list, # rows from _v6_7_walk_descendants *, allow_no_review_needed: bool = False, ) -> Optional[MissingUmbrellaReviewViolation]: - """Reject a goal-mode umbrella's kanban_complete if its + """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 @@ -1111,45 +1126,62 @@ def verify_umbrella_review_coverage( 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 the JARVIS umbrella to - keep_running until at least one review task exists in its + 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 (orchestration umbrellas always run - with --goal; non-goal tasks don't need this discipline). - - At least one descendant has a review-role assignee. + - 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: - that gate fires at ARCHIVE time and spawns Tchalla after a chain - settles. This gate 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. + - #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 - has_non_review = False for ch in descendants: assignee = (ch["assignee"] or "").lower() if assignee in REVIEW_ROLES: has_review = True - elif assignee: # any non-empty non-review assignee - has_non_review = True + break # one is enough to satisfy the gate if has_review: return None - # No review descendants — fail. Surface whether there's at least - # one build descendant so the message can specialize. + # 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=has_non_review, + has_non_review_descendant=bool(descendants), ) diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index ef0fc5e4449b..f53fc2728267 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -3689,8 +3689,8 @@ def _v6_7_run_completion_gates( 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. - no_review_needed_reason = _validate_opt_out( - task_id, "x_no_review_needed", md.get("x_no_review_needed"), + 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 ( @@ -3700,7 +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_no_review_needed", no_review_needed_reason), + ("x_umbrella_no_review", umbrella_no_review_reason), ) if v is not None } if accepted_opt_outs: @@ -3779,12 +3779,13 @@ def _v6_7_run_completion_gates( # _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 = no_review_needed_reason is not None + 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, diff --git a/tests/cli/test_kanban_completion_gates.py b/tests/cli/test_kanban_completion_gates.py index 40c580b2eb84..14e5de336e36 100644 --- a/tests/cli/test_kanban_completion_gates.py +++ b/tests/cli/test_kanban_completion_gates.py @@ -2173,7 +2173,7 @@ def _row(assignee, id_="x", status="done"): 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_id="t_u", + is_goal_mode=False, umbrella_assignee="jarvis", umbrella_id="t_u", descendants=[self._row("friday")], ) assert v is None @@ -2181,7 +2181,7 @@ def test_non_goal_mode_skips(self) -> 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_id="t_u", + is_goal_mode=True, umbrella_assignee="jarvis", umbrella_id="t_u", descendants=[self._row("friday"), self._row("tony")], ) assert v is None @@ -2191,7 +2191,7 @@ def test_goal_mode_with_only_build_descendants_rejects(self) -> None: MissingUmbrellaReviewViolation, verify_umbrella_review_coverage, ) v = verify_umbrella_review_coverage( - is_goal_mode=True, umbrella_id="t_u", + is_goal_mode=True, umbrella_assignee="jarvis", umbrella_id="t_u", descendants=[self._row("friday"), self._row("pepper")], ) assert isinstance(v, MissingUmbrellaReviewViolation) @@ -2209,7 +2209,7 @@ def test_goal_mode_with_no_descendants_rejects(self) -> None: MissingUmbrellaReviewViolation, verify_umbrella_review_coverage, ) v = verify_umbrella_review_coverage( - is_goal_mode=True, umbrella_id="t_u", + is_goal_mode=True, umbrella_assignee="jarvis", umbrella_id="t_u", descendants=[], ) assert isinstance(v, MissingUmbrellaReviewViolation) @@ -2219,7 +2219,7 @@ def test_goal_mode_with_no_descendants_rejects(self) -> None: 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_id="t_u", + is_goal_mode=True, umbrella_assignee="jarvis", umbrella_id="t_u", descendants=[self._row("friday"), self._row("tchalla")], ) assert v is None @@ -2227,7 +2227,7 @@ def test_tchalla_descendant_satisfies_gate(self) -> 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_id="t_u", + is_goal_mode=True, umbrella_assignee="jarvis", umbrella_id="t_u", descendants=[self._row("friday"), self._row("vision")], ) assert v is None @@ -2235,12 +2235,119 @@ def test_vision_descendant_satisfies_gate(self) -> 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_id="t_u", + 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 @@ -2315,7 +2422,7 @@ def test_jarvis_umbrella_passes_when_review_exists( ) assert ok - def test_x_no_review_needed_opt_out_passes( + def test_x_umbrella_no_review_opt_out_passes( self, umbrella_with_only_build, ) -> None: """An umbrella that legitimately doesn't need review can @@ -2324,7 +2431,7 @@ def test_x_no_review_needed_opt_out_passes( umbrella_with_only_build, "t_umb", summary="status ack", result="Acknowledged upstream completion; no review needed.", - metadata={"x_no_review_needed": + metadata={"x_umbrella_no_review": "pure status ack of upstream complete, no actionable subtasks"}, ) assert ok @@ -2338,7 +2445,7 @@ def test_opt_out_with_short_reason_rejected( kb.complete_task( umbrella_with_only_build, "t_umb", summary="x", result="x", - metadata={"x_no_review_needed": "ok"}, + metadata={"x_umbrella_no_review": "ok"}, ) def test_non_goal_mode_task_unaffected(