Skip to content

Commit 16d4e80

Browse files
matt-FFFFFFmatt-FFFFFF
committed
feat: circular dependency checks and better context in config build
1 parent 7c1e77a commit 16d4e80

13 files changed

Lines changed: 1514 additions & 5 deletions

File tree

‎cmd/porch/run/run.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"context"
88
"fmt"
99
"os"
10+
"time"
1011

1112
"github.com/matt-FFFFFF/porch/internal/commands"
1213
"github.com/matt-FFFFFF/porch/internal/config"
@@ -93,7 +94,11 @@ func actionFunc(ctx context.Context, cmd *cli.Command) error {
9394

9495
factory := ctx.Value(commands.FactoryContextKey{}).(commands.CommanderFactory)
9596

96-
rb, err := config.BuildFromYAML(ctx, factory, bytes)
97+
// Create a timeout context for configuration building (30 seconds)
98+
configCtx, configCancel := context.WithTimeout(ctx, 30*time.Second)
99+
defer configCancel()
100+
101+
rb, err := config.BuildFromYAML(configCtx, factory, bytes)
97102
if err != nil {
98103
return cli.Exit(fmt.Sprintf("failed to build config from file %s: %s", yamlFileName, err.Error()), 1)
99104
}

‎docs/circular-dependency-detection.md‎

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
# Circular Dependency Detection
2+
3+
## Overview
4+
5+
The porch configuration system now includes robust circular dependency detection to prevent infinite loops in command group references. This feature helps catch configuration errors early and provides clear error messages to help debug issues.
6+
7+
## Features
8+
9+
### 1. Circular Dependency Detection
10+
11+
The system detects several types of circular dependencies:
12+
13+
- **Simple circular dependencies**: Group A references Group B, which references Group A
14+
- **Multi-way circular dependencies**: Group A → Group B → Group C → Group A
15+
- **Self-referencing groups**: A group that references itself
16+
- **Deep nested cycles**: Circular dependencies buried deep in nested command structures
17+
18+
### 2. Maximum Recursion Depth Protection
19+
20+
To prevent stack overflow and excessive processing, the system enforces a maximum recursion depth of 100 levels when resolving command groups.
21+
22+
### 3. Enhanced Error Messages
23+
24+
When a circular dependency is detected, the system provides clear error messages showing:
25+
26+
- The exact circular path (e.g., "group_a → group_b → group_a")
27+
- Which command within a group caused the issue
28+
- The command index for easier debugging
29+
30+
### 4. Context Cancellation Support
31+
32+
Configuration building now respects context cancellation, allowing for:
33+
34+
- Graceful interruption with Ctrl+C during config parsing
35+
- Timeout protection (30-second default for config building)
36+
- Responsive signal handling during long configuration processes
37+
38+
## Examples
39+
40+
### Detecting Circular Dependencies
41+
42+
**Example 1: Simple Circular Dependency**
43+
44+
```yaml
45+
name: infinite loop example
46+
command_groups:
47+
- name: group_one
48+
commands:
49+
- type: serial
50+
name: reference to two
51+
command_group: group_two
52+
- name: group_two
53+
commands:
54+
- type: serial
55+
name: reference to one
56+
command_group: group_one
57+
commands:
58+
- type: parallel
59+
name: start
60+
command_group: group_one
61+
```
62+
63+
**Error Output:**
64+
65+
```
66+
failed to build config: invalid command group 'group_one':
67+
in command 0 of group group_one: circular dependency detected: group_one → group_two → group_one
68+
```
69+
70+
**Example 2: Self-Referencing Group**
71+
72+
```yaml
73+
name: self reference example
74+
command_groups:
75+
- name: recursive_group
76+
commands:
77+
- type: shell
78+
name: first command
79+
command_line: echo "first"
80+
- type: serial
81+
name: self reference
82+
command_group: recursive_group
83+
commands:
84+
- type: serial
85+
name: start
86+
command_group: recursive_group
87+
```
88+
89+
**Error Output:**
90+
91+
```
92+
failed to build config: invalid command group 'recursive_group':
93+
in command 1 of group recursive_group: circular dependency detected: recursive_group → recursive_group
94+
```
95+
96+
### Valid Nested Groups
97+
98+
This configuration is valid and will work correctly:
99+
100+
```yaml
101+
name: valid nested example
102+
command_groups:
103+
- name: setup
104+
commands:
105+
- type: shell
106+
name: prepare
107+
command_line: echo "preparing"
108+
- type: serial
109+
name: run tests
110+
command_group: test_suite
111+
- name: test_suite
112+
commands:
113+
- type: shell
114+
name: unit tests
115+
command_line: echo "running unit tests"
116+
- type: serial
117+
name: cleanup
118+
command_group: cleanup
119+
- name: cleanup
120+
commands:
121+
- type: shell
122+
name: clean up
123+
command_line: echo "cleaning up"
124+
commands:
125+
- type: serial
126+
name: main workflow
127+
command_group: setup
128+
```
129+
130+
## Signal Handling Improvements
131+
132+
### Configuration Building Timeout
133+
134+
Configuration building now has a 30-second timeout to prevent hanging on malformed configurations:
135+
136+
```go
137+
// In cmd/porch/run/run.go
138+
configCtx, configCancel := context.WithTimeout(ctx, 30*time.Second)
139+
defer configCancel()
140+
141+
rb, err := config.BuildFromYAML(configCtx, factory, bytes)
142+
```
143+
144+
### Context Cancellation Checks
145+
146+
The system now checks for context cancellation at key points:
147+
148+
- Before starting configuration parsing
149+
- After command group validation
150+
- During individual command processing
151+
152+
### Graceful Interruption
153+
154+
When you press Ctrl+C during configuration building, you'll see:
155+
156+
```
157+
failed to build config: configuration building timed out: context canceled
158+
```
159+
160+
## Implementation Details
161+
162+
### Registry-Level Detection
163+
164+
The `commandregistry.Registry` now includes:
165+
166+
- `resolveCommandGroupWithDepth()`: Tracks recursion depth and visiting state
167+
- `validateCommandForCircularDeps()`: Validates individual commands for group references
168+
- `formatCircularDependencyPath()`: Creates human-readable circular dependency paths
169+
170+
### Configuration-Level Validation
171+
172+
The `config.BuildFromYAML()` function:
173+
174+
- Validates all command groups before proceeding with command creation
175+
- Includes context cancellation checks throughout the process
176+
- Provides detailed error messages with command indices
177+
178+
### Error Types
179+
180+
New error types have been added:
181+
182+
- `ErrCircularDependency`: For circular dependency detection
183+
- `ErrConfigurationTimeout`: For configuration timeout handling
184+
- `ErrMaxRecursionDepth`: For recursion depth protection
185+
186+
## Testing
187+
188+
Comprehensive tests have been added covering:
189+
190+
- Simple circular dependencies
191+
- Multi-way circular dependencies
192+
- Self-referencing groups
193+
- Maximum recursion depth
194+
- Context cancellation
195+
- Configuration timeouts
196+
197+
Run tests with:
198+
199+
```bash
200+
go test ./internal/config -v -run TestCircularDependencyDetection
201+
go test ./internal/config -v -run TestConfigurationTimeout
202+
go test ./internal/commandregistry -v
203+
```

‎examples/complex-example.yaml‎

Lines changed: 0 additions & 1 deletion
This file was deleted.

‎examples/complex_circular.yml‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# More complex circular dependency example
2+
name: complex circular dependency test
3+
description: Test more complex circular dependency scenarios
4+
command_groups:
5+
- name: setup
6+
commands:
7+
- type: shell
8+
name: prepare environment
9+
command_line: echo "preparing environment"
10+
- type: parallel
11+
name: run setup tasks
12+
command_group: setup_tasks
13+
14+
- name: setup_tasks
15+
commands:
16+
- type: shell
17+
name: task 1
18+
command_line: echo "task 1"
19+
- type: serial
20+
name: run validation
21+
command_group: validation
22+
23+
- name: validation
24+
commands:
25+
- type: shell
26+
name: validate setup
27+
command_line: echo "validating"
28+
- type: serial
29+
name: re-run setup if needed
30+
command_group: setup # This creates the circular dependency
31+
32+
commands:
33+
- type: serial
34+
name: main workflow
35+
command_group: setup

‎examples/infinite_loop.yml‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
# yaml-language-server: $schema=/Users/matt/code/porch/schema.json
2+
name: infinite loop
3+
command_groups:
4+
- name: one
5+
commands:
6+
- type: shell
7+
name: one-0
8+
command_line: echo "This is a shell command in group one"
9+
- type: serial
10+
name: one-1
11+
command_group: two
12+
- name: two
13+
commands:
14+
- type: shell
15+
name: two-0
16+
command_line: echo "This is a shell command in group two"
17+
- type: serial
18+
name: two-1
19+
command_group: one
20+
commands:
21+
- type: shell
22+
name: infinite loop
23+
command_line: echo "This is an infinite loop"
24+
- type: parallel
25+
name: crash
26+
command_group: one

‎examples/signal_test.yml‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
# Test file for signal handling during configuration
2+
name: test signal handling during config build
3+
description: Test that signal handling works during configuration building
4+
command_groups:
5+
- name: deep_group_1
6+
commands:
7+
- type: serial
8+
name: reference to deep_group_2
9+
command_group: deep_group_2
10+
- name: deep_group_2
11+
commands:
12+
- type: serial
13+
name: reference to deep_group_3
14+
command_group: deep_group_3
15+
- name: deep_group_3
16+
commands:
17+
- type: shell
18+
name: simple echo
19+
command_line: echo "test"
20+
commands:
21+
- type: shell
22+
name: start
23+
command_line: echo "starting"
24+
- type: serial
25+
name: use deep group
26+
command_group: deep_group_1

0 commit comments

Comments
 (0)