From fd57f61f341ae0adcd1803e77925516205b011c6 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Thu, 25 Dec 2025 12:21:23 -0800 Subject: [PATCH] fix: Prevent aspect self-matching infinite recursion (gt-8tmz.16) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ApplyAdvice now collects original step IDs before applying rules and only matches steps in that set. This prevents advice patterns (like "*") from matching their own inserted before/after steps. Added test case demonstrating that a "*" pattern only adds advice to original steps, not to inserted steps. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- internal/formula/advice.go | 36 +++++++++++++++++++++++ internal/formula/advice_test.go | 52 +++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/internal/formula/advice.go b/internal/formula/advice.go index dc378f58..e707432f 100644 --- a/internal/formula/advice.go +++ b/internal/formula/advice.go @@ -55,14 +55,33 @@ func MatchGlob(pattern, stepID string) bool { // ApplyAdvice transforms a formula's steps by applying advice rules. // Returns a new steps slice with advice steps inserted. // The original steps slice is not modified. +// +// Self-matching prevention (gt-8tmz.16): Advice only matches steps that +// existed BEFORE this call. Steps inserted by advice (before/after/around) +// are not matched, preventing infinite recursion. func ApplyAdvice(steps []*Step, advice []*AdviceRule) []*Step { if len(advice) == 0 { return steps } + // Collect original step IDs to prevent self-matching (gt-8tmz.16) + originalIDs := collectStepIDs(steps) + + return applyAdviceWithGuard(steps, advice, originalIDs) +} + +// ApplyAdviceToOriginalOnly applies advice rules but only matches steps +// whose IDs are in the originalIDs set. This prevents aspects from matching +// steps they themselves inserted. +func applyAdviceWithGuard(steps []*Step, advice []*AdviceRule, originalIDs map[string]bool) []*Step { result := make([]*Step, 0, len(steps)*2) // Pre-allocate for insertions for _, step := range steps { + // Skip steps not in original set (gt-8tmz.16) + if !originalIDs[step.ID] { + result = append(result, step) + continue + } // Find matching advice rules for this step var beforeSteps []*Step var afterSteps []*Step @@ -191,6 +210,23 @@ func appendUnique(slice []string, item string) []string { return append(slice, item) } +// collectStepIDs returns a set of all step IDs (including nested children). +// Used by ApplyAdvice to prevent self-matching (gt-8tmz.16). +func collectStepIDs(steps []*Step) map[string]bool { + ids := make(map[string]bool) + var collect func([]*Step) + collect = func(steps []*Step) { + for _, step := range steps { + ids[step.ID] = true + if len(step.Children) > 0 { + collect(step.Children) + } + } + } + collect(steps) + return ids +} + // MatchPointcut checks if a step matches a pointcut. func MatchPointcut(pc *Pointcut, step *Step) bool { // Glob match on step ID diff --git a/internal/formula/advice_test.go b/internal/formula/advice_test.go index 28f57509..dedf23bc 100644 --- a/internal/formula/advice_test.go +++ b/internal/formula/advice_test.go @@ -321,3 +321,55 @@ func contains(slice []string, item string) bool { } return false } + +// TestApplyAdvice_SelfMatchingPrevention verifies that advice doesn't match +// steps it inserted (gt-8tmz.16). +func TestApplyAdvice_SelfMatchingPrevention(t *testing.T) { + // An advice rule with a broad pattern that would match its own insertions + // if self-matching weren't prevented. + advice := []*AdviceRule{ + { + Target: "*", // Matches everything + Around: &AroundAdvice{ + Before: []*AdviceStep{ + {ID: "{step.id}-before", Title: "Before {step.id}"}, + }, + After: []*AdviceStep{ + {ID: "{step.id}-after", Title: "After {step.id}"}, + }, + }, + }, + } + + steps := []*Step{ + {ID: "implement", Title: "Implement"}, + } + + result := ApplyAdvice(steps, advice) + + // Without self-matching prevention, pattern "*" would match the inserted + // "implement-before" and "implement-after" steps, causing them to also + // get before/after steps, leading to potential infinite expansion. + // With prevention, we should only get the original step + its advice. + + // Expected: implement-before, implement, implement-after (3 steps) + if len(result) != 3 { + t.Errorf("ApplyAdvice() produced %d steps, want 3. Got IDs: %v", + len(result), getStepIDs(result)) + } + + expectedIDs := []string{"implement-before", "implement", "implement-after"} + for i, want := range expectedIDs { + if result[i].ID != want { + t.Errorf("result[%d].ID = %q, want %q", i, result[i].ID, want) + } + } +} + +func getStepIDs(steps []*Step) []string { + ids := make([]string, len(steps)) + for i, s := range steps { + ids[i] = s.ID + } + return ids +}