v6.8 Part 4: umbrella keep_running until reviewers spawn (#79)#19
Merged
Conversation
…wn (NousResearch#79) 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 NousResearch#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 <build-id>. 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 NousResearch#30 - NousResearch#30 (integrative review at archive): fires at ARCHIVE time, spawns Tchalla after a chain settles. - NousResearch#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 NousResearch#73 + NousResearch#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) <noreply@anthropic.com>
…earch#79) 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) <noreply@anthropic.com>
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.
Summary
Final v6.8 issue. 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` fires when a goal_mode umbrella calls `kanban_complete`. Walks transitive descendants (reusing `_v6_7_walk_descendants` from NousResearch#73 so chained shapes count). If no review-role descendant exists anywhere in the subtree, rejects.
Two violation messages:
Opt-out: `metadata={"x_no_review_needed": "<≥20-char reason>"}` for legitimate cases. Standard opt-out conventions (audit event, bool/short rejected).
Relationship to v6.7 NousResearch#30
Together they catch the empty-chain failure mode at both ends of the umbrella lifecycle.
Test plan
🤖 Generated with Claude Code