Skip to content

Commit a6aa4e7

Browse files
committed
fix: TOML parser edge cases, git timeout, dead code removal
- config: handle values containing '=' and proper quote stripping - incremental: use exec.CommandContext with 30s timeout for git diff - filter: remove unused firstErr variable
1 parent 9803ee8 commit a6aa4e7

3 files changed

Lines changed: 35 additions & 15 deletions

File tree

config.go

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func findConfigFile(dir string) string {
9191
}
9292

9393
// parseTOMLConfig parses a simplified TOML format.
94-
// Supports key = "value", key = true/false, key = 123, and [section] headers.
94+
// Supports key = "value", key = true/false, key = 123, arrays, and [section] headers.
9595
func parseTOMLConfig(content string) (*FileConfig, error) {
9696
cfg := &FileConfig{
9797
Prompts: make(map[string]string),
@@ -109,13 +109,10 @@ func parseTOMLConfig(content string) (*FileConfig, error) {
109109
continue
110110
}
111111

112-
parts := strings.SplitN(line, "=", 2)
113-
if len(parts) != 2 {
112+
key, value, ok := parseTOMLKeyValue(line)
113+
if !ok {
114114
continue
115115
}
116-
key := strings.TrimSpace(parts[0])
117-
value := strings.TrimSpace(parts[1])
118-
value = strings.Trim(value, `"'`)
119116

120117
if section == "prompts" {
121118
cfg.Prompts[key] = value
@@ -150,6 +147,25 @@ func parseTOMLConfig(content string) (*FileConfig, error) {
150147
return cfg, nil
151148
}
152149

150+
// parseTOMLKeyValue splits a TOML line into key and value, handling quoted values
151+
// that may contain '=' signs.
152+
func parseTOMLKeyValue(line string) (key, value string, ok bool) {
153+
idx := strings.Index(line, "=")
154+
if idx < 0 {
155+
return "", "", false
156+
}
157+
key = strings.TrimSpace(line[:idx])
158+
value = strings.TrimSpace(line[idx+1:])
159+
// Strip matching outer quotes
160+
if len(value) >= 2 {
161+
if (value[0] == '"' && value[len(value)-1] == '"') ||
162+
(value[0] == '\'' && value[len(value)-1] == '\'') {
163+
value = value[1 : len(value)-1]
164+
}
165+
}
166+
return key, value, key != ""
167+
}
168+
153169
func parseTOMLArray(s string) []string {
154170
s = strings.Trim(s, "[]")
155171
parts := strings.Split(s, ",")

filter.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ func FilterFindings(ctx context.Context, provider Provider, findings []Finding,
5656
sem := make(chan struct{}, config.MaxParallel)
5757
var wg sync.WaitGroup
5858
var mu sync.Mutex
59-
var firstErr error
6059

6160
for i, finding := range toFilter {
6261
wg.Add(1)
@@ -92,9 +91,6 @@ func FilterFindings(ctx context.Context, provider Provider, findings []Finding,
9291
}
9392
}
9493

95-
if firstErr != nil {
96-
return confirmed, results, firstErr
97-
}
9894
return confirmed, results, nil
9995
}
10096

incremental.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"os/exec"
77
"strings"
88
"sync"
9+
"time"
910
)
1011

1112
// IncrementalState tracks the last-reviewed commit SHA for incremental reviews.
@@ -50,7 +51,7 @@ func ReviewIncremental(ctx context.Context, base, head string, state *Incrementa
5051
}
5152
}
5253

53-
diffText, err := gitDiffRange(base, head)
54+
diffText, err := gitDiffRange(ctx, base, head)
5455
if err != nil {
5556
return nil, fmt.Errorf("sight: incremental diff failed: %w", err)
5657
}
@@ -76,16 +77,23 @@ func ReviewIncremental(ctx context.Context, base, head string, state *Incrementa
7677
return result, nil
7778
}
7879

79-
// gitDiffRange runs `git diff base...head` and returns the output.
80-
func gitDiffRange(base, head string) (string, error) {
80+
// gitDiffRange runs `git diff base...head` with a context timeout.
81+
func gitDiffRange(ctx context.Context, base, head string) (string, error) {
82+
// Default 30s timeout if context has no deadline
83+
if _, ok := ctx.Deadline(); !ok {
84+
var cancel context.CancelFunc
85+
ctx, cancel = context.WithTimeout(ctx, 30*time.Second)
86+
defer cancel()
87+
}
88+
8189
// Try three-dot syntax first (merge-base diff)
82-
out, err := exec.Command("git", "diff", base+"..."+head).Output()
90+
out, err := exec.CommandContext(ctx, "git", "diff", base+"..."+head).Output()
8391
if err == nil {
8492
return string(out), nil
8593
}
8694

8795
// Fall back to two-dot syntax
88-
out, err = exec.Command("git", "diff", base, head).Output()
96+
out, err = exec.CommandContext(ctx, "git", "diff", base, head).Output()
8997
if err != nil {
9098
return "", fmt.Errorf("git diff %s %s failed: %w", base, head, err)
9199
}

0 commit comments

Comments
 (0)