fix: validator-time guard on inbound-to-terminal edges (#3)#85
Merged
mattleaverton merged 2 commits intoApr 27, 2026
Merged
Conversation
…x existing graphs The new terminal_condition_edge rule and the existing all_conditional_edges rule were in tension: the natural fix for the new rule (adding condition="outcome=success" to edges flagged by terminal_condition_edge) triggered all_conditional_edges, which fires when a node has only conditional outgoing edges with no fallback. all_conditional_edges now exempts two patterns: 1. Every outgoing edge targets a terminal node — "no condition matched" terminates the run by intent; terminal_condition_edge governs those edges instead. 2. Conditions are exhaustive via an outcome=X / outcome!=X pair — the pair covers the full outcome space, so there is no runtime routing gap. Also adds explicit conditions to the inbound-to-terminal edges in the three existing workflow graphs (build-test, coding-loop, multi-tool-exercise) so all three now validate clean. 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
From
docs/plans/2026-04-24-kilroy-fixes-from-feedback.mditem #3 (option b — validator-time).Bug: A run that follows an unconditional edge to a terminal node after a failed predecessor stage reports
status=successwith no actual work done. Every graph author hits this trap (the canonical example:pr-reviewPR #303 v1 had 4× transient failures insetup, followedsetup -> done, reported success, no review produced).Fix: New validator rule
terminal_condition_edge(ERROR severity). Requires explicitcondition=on every inbound edge to a terminal node (Msquare,doublecircle, or node idexit/end) unless the predecessor is success-only (Mdiamond,circle,diamond, loop.begin/end, concurrent.split/join). Catches the bug at validate time — cheaper than runtime reconciliation. The error message names both offending node IDs and suggests the exact condition to add.Reconciled with
all_conditional_edgesThe natural fix for the new rule (adding
condition="outcome=success"to flagged edges) used to trigger the existingall_conditional_edgesrule, which fires on nodes whose outgoing edges are all conditional with no fallback. That tension is resolved:all_conditional_edgesnow exempts two patterns where there's no actual routing gap:terminal_condition_edgegoverns those edges. The classicreport → done [condition="outcome=success"]pattern is now valid.outcome=X/outcome!=Xpair — the pair covers the full outcome space, so the routing is provably complete. The classicprepare → next [condition="outcome=success"]+prepare → done [condition="outcome!=success"]pattern is now valid.Existing workflow graphs
All three graphs in
workflows/(build-test,coding-loop,multi-tool-exercise) had latent unconditional inbound-to-terminal edges from fallible predecessors. This PR adds explicit conditions to all six flagged edges. All three graphs now validate clean.Test plan
internal/attractor/validate/terminal_condition_test.gocovering all acceptance criteria for the new rule.all_conditional_edgestests to use non-terminal intermediate nodes (the olda → exittopology now correctly falls under the all-targets-terminal exemption).TestValidate_AllConditionalEdges_ExemptOnExhaustivePairandTestValidate_AllConditionalEdges_ExemptWhenAllTargetsTerminal.go test ./internal/attractor/validate/...passes.kilroy attractor validate --graphclean on all three workflow graphs.go build ./...andgo vet ./...clean.Context
Produced by a dogfood quick-launch run against this repo. Initial agent flagged the rule tension but punted to reviewer; resolved in a follow-up commit on this branch.
🤖 Generated with Claude Code