fix: Prevent aspect self-matching infinite recursion (gt-8tmz.16)
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 <noreply@anthropic.com>
This commit is contained in:
@@ -55,14 +55,33 @@ func MatchGlob(pattern, stepID string) bool {
|
|||||||
// ApplyAdvice transforms a formula's steps by applying advice rules.
|
// ApplyAdvice transforms a formula's steps by applying advice rules.
|
||||||
// Returns a new steps slice with advice steps inserted.
|
// Returns a new steps slice with advice steps inserted.
|
||||||
// The original steps slice is not modified.
|
// 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 {
|
func ApplyAdvice(steps []*Step, advice []*AdviceRule) []*Step {
|
||||||
if len(advice) == 0 {
|
if len(advice) == 0 {
|
||||||
return steps
|
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
|
result := make([]*Step, 0, len(steps)*2) // Pre-allocate for insertions
|
||||||
|
|
||||||
for _, step := range steps {
|
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
|
// Find matching advice rules for this step
|
||||||
var beforeSteps []*Step
|
var beforeSteps []*Step
|
||||||
var afterSteps []*Step
|
var afterSteps []*Step
|
||||||
@@ -191,6 +210,23 @@ func appendUnique(slice []string, item string) []string {
|
|||||||
return append(slice, item)
|
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.
|
// MatchPointcut checks if a step matches a pointcut.
|
||||||
func MatchPointcut(pc *Pointcut, step *Step) bool {
|
func MatchPointcut(pc *Pointcut, step *Step) bool {
|
||||||
// Glob match on step ID
|
// Glob match on step ID
|
||||||
|
|||||||
@@ -321,3 +321,55 @@ func contains(slice []string, item string) bool {
|
|||||||
}
|
}
|
||||||
return false
|
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
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user