Skip to content

Commit 1ccdb03

Browse files
author
Test User
committed
fix(merge_coordinator): P1 — skip retry on 4xx client errors; P2 — use directory-component matching for contamination patterns
P1: send_with_retry now breaks immediately on client errors (4xx except 429), avoiding 7s wasted retry loops on 404/403/422 responses. P2: check_contamination now matches patterns only as directory components (starts_with or contains '/.pattern/'), preventing false positives on files like src/sessions_parser.rs that coincidentally contain pattern strings. Tests: 35 pass, 0 fail. Clippy clean.
1 parent f9ad75d commit 1ccdb03

2 files changed

Lines changed: 41 additions & 31 deletions

File tree

crates/terraphim_merge_coordinator/src/evaluator.rs

Lines changed: 36 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,12 @@ async fn evaluate_one(
8484
/// Check PR file list for contamination (artefacts, session dumps, etc.).
8585
///
8686
/// Returns `Ok(())` if clean, `Err(reason)` if contaminated.
87+
///
88+
/// Patterns match as directory components: a file is contaminated if it
89+
/// starts with a pattern (e.g. `.sessions/session.md`) or contains the
90+
/// pattern preceded by `/` (e.g. `path/.sessions/session.md`). Plain
91+
/// substring matching is avoided to prevent false positives like
92+
/// `src/sessions_parser.rs` matching `.sessions/`.
8793
async fn check_contamination(
8894
gitea: &GiteaClient,
8995
owner: &str,
@@ -99,7 +105,12 @@ async fn check_contamination(
99105

100106
for file in &files {
101107
for pattern in CONTAMINATED_PATTERNS {
102-
if file.starts_with(pattern) || file.contains(pattern) {
108+
if file.starts_with(pattern) {
109+
return Err(format!("contaminated: {file} (pattern: {pattern})"));
110+
}
111+
// Check for directory-component match: "/.sessions/" within path
112+
let component = ["/", pattern].concat();
113+
if file.contains(&component) {
103114
return Err(format!("contaminated: {file} (pattern: {pattern})"));
104115
}
105116
}
@@ -277,36 +288,30 @@ mod tests {
277288
fn contamination_patterns_match_artefact_files() {
278289
let patterns: &[&str] = &[".sessions/", ".review_tmp/", ".handoff/", ".beads/"];
279290

280-
// Positive matches
281-
assert!(
282-
patterns
283-
.iter()
284-
.any(|p| ".sessions/session-123.md".contains(p))
285-
);
286-
assert!(
287-
patterns
288-
.iter()
289-
.any(|p| ".review_tmp/pr123/file.diff".contains(p))
290-
);
291-
assert!(
292-
patterns
293-
.iter()
294-
.any(|p| ".handoff/pr2664-review.md".contains(p))
295-
);
296-
assert!(patterns.iter().any(|p| ".beads/issues.jsonl".contains(p)));
291+
// Helper: matches as directory component (starts_with or contains "/.sessions/")
292+
let is_contaminated = |file: &str| -> bool {
293+
patterns.iter().any(|p| {
294+
file.starts_with(p)
295+
|| file.contains(&["/", p].concat())
296+
})
297+
};
297298

298-
// Negative matches
299-
assert!(!patterns.iter().any(|p| "src/main.rs".contains(p)));
300-
assert!(
301-
!patterns
302-
.iter()
303-
.any(|p| "crates/terraphim_rlm/src/lib.rs".contains(p))
304-
);
305-
assert!(!patterns.iter().any(|p| "Cargo.toml".contains(p)));
306-
assert!(
307-
!patterns
308-
.iter()
309-
.any(|p| ".github/workflows/ci-pr.yml".contains(p))
310-
);
299+
// Positive matches — files inside contaminated directories
300+
assert!(is_contaminated(".sessions/session-123.md"));
301+
assert!(is_contaminated("subdir/.sessions/session-123.md"));
302+
assert!(is_contaminated(".review_tmp/pr123/file.diff"));
303+
assert!(is_contaminated(".handoff/pr2664-review.md"));
304+
assert!(is_contaminated(".beads/issues.jsonl"));
305+
assert!(is_contaminated("crates/foo/.beads/issues.jsonl"));
306+
307+
// Negative matches — files NOT in contaminated directories
308+
assert!(!is_contaminated("src/main.rs"));
309+
assert!(!is_contaminated("crates/terraphim_rlm/src/lib.rs"));
310+
assert!(!is_contaminated("Cargo.toml"));
311+
assert!(!is_contaminated(".github/workflows/ci-pr.yml"));
312+
// False positive prevention: filename containing pattern string but not as directory
313+
assert!(!is_contaminated("src/sessions_parser.rs"));
314+
assert!(!is_contaminated("docs/review_tmp_guide.md"));
315+
assert!(!is_contaminated("tests/handoff_integration_test.rs"));
311316
}
312317
}

crates/terraphim_merge_coordinator/src/gitea.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,8 +219,13 @@ impl GiteaClient {
219219
}
220220
Ok(resp) => {
221221
let status = resp.status();
222+
let is_client_error = status.is_client_error() && status != reqwest::StatusCode::TOO_MANY_REQUESTS;
222223
let body_text = resp.text().await.unwrap_or_default();
223224
last_err = Some(format!("status {status}: {body_text}"));
225+
if is_client_error {
226+
warn!(method = %method, url = %redact(url), attempt, %status, "gitea client error (non-retryable); failing immediately");
227+
break;
228+
}
224229
warn!(method = %method, url = %redact(url), attempt, %status, "gitea non-success; will retry if attempts remain");
225230
}
226231
Err(e) => {

0 commit comments

Comments
 (0)