Skip to content

Commit c95b4cc

Browse files
author
Test User
committed
feat(merge_coordinator): add paginated list_pr_files + contamination gate Refs #2409
Step 1: Paginated list_pr_files() using X-Total-Count header and page loop Step 2: Added check_contamination() to detect .sessions/, .review_tmp/, .handoff/, .beads/ patterns Step 3: Wired contamination check into evaluate_one() before mergeability evaluation Tests: 35 pass, 0 fail. Clippy clean.
1 parent b168375 commit c95b4cc

2 files changed

Lines changed: 102 additions & 11 deletions

File tree

crates/terraphim_merge_coordinator/src/evaluator.rs

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ pub async fn evaluate_all(
4242
Ok(out)
4343
}
4444

45+
#[allow(clippy::collapsible_match)]
4546
async fn evaluate_one(
4647
gitea: Option<&GiteaClient>,
4748
owner: &str,
@@ -50,6 +51,19 @@ async fn evaluate_one(
5051
) -> PrEvaluation {
5152
let mergeable = pr.mergeable.unwrap_or(false);
5253
let fixes_issues = extract_fixes(pr.body.as_deref().unwrap_or(""));
54+
55+
// Check contamination before mergeability to prevent artefact PRs from merging.
56+
if let Some(c) = gitea
57+
&& let Err(reason) = check_contamination(c, owner, repo, pr.number).await {
58+
return PrEvaluation {
59+
pr_index: pr.number,
60+
mergeable,
61+
fixes_issues,
62+
verdict: EvalVerdict::Hold(reason),
63+
blocker_kind: None,
64+
};
65+
}
66+
5367
let (verdict, blocker_kind) = if !mergeable {
5468
let kind = classify_blocker(gitea, owner, repo, pr).await;
5569
let reason = format!("not mergeable ({kind})");
@@ -66,6 +80,40 @@ async fn evaluate_one(
6680
}
6781
}
6882

83+
/// Check PR file list for contamination (artefacts, session dumps, etc.).
84+
///
85+
/// Returns `Ok(())` if clean, `Err(reason)` if contaminated.
86+
async fn check_contamination(
87+
gitea: &GiteaClient,
88+
owner: &str,
89+
repo: &str,
90+
pr_index: u64,
91+
) -> Result<(), String> {
92+
const CONTAMINATED_PATTERNS: &[&str] = &[
93+
".sessions/",
94+
".review_tmp/",
95+
".handoff/",
96+
".beads/",
97+
];
98+
99+
let files = gitea
100+
.list_pr_files(owner, repo, pr_index)
101+
.await
102+
.map_err(|e| format!("contamination check failed: {e}"))?;
103+
104+
for file in &files {
105+
for pattern in CONTAMINATED_PATTERNS {
106+
if file.starts_with(pattern) || file.contains(pattern) {
107+
return Err(format!(
108+
"contaminated: {file} (pattern: {pattern})"
109+
));
110+
}
111+
}
112+
}
113+
114+
Ok(())
115+
}
116+
69117
/// Query CI status and classify why a PR is blocked.
70118
async fn classify_blocker(
71119
gitea: Option<&GiteaClient>,
@@ -230,4 +278,21 @@ mod tests {
230278
assert_eq!(last.verdict, EvalVerdict::Merge);
231279
assert_eq!(last.blocker_kind, None);
232280
}
281+
282+
#[test]
283+
fn contamination_patterns_match_artefact_files() {
284+
let patterns: &[&str] = &[".sessions/", ".review_tmp/", ".handoff/", ".beads/"];
285+
286+
// Positive matches
287+
assert!(patterns.iter().any(|p| ".sessions/session-123.md".contains(p)));
288+
assert!(patterns.iter().any(|p| ".review_tmp/pr123/file.diff".contains(p)));
289+
assert!(patterns.iter().any(|p| ".handoff/pr2664-review.md".contains(p)));
290+
assert!(patterns.iter().any(|p| ".beads/issues.jsonl".contains(p)));
291+
292+
// Negative matches
293+
assert!(!patterns.iter().any(|p| "src/main.rs".contains(p)));
294+
assert!(!patterns.iter().any(|p| "crates/terraphim_rlm/src/lib.rs".contains(p)));
295+
assert!(!patterns.iter().any(|p| "Cargo.toml".contains(p)));
296+
assert!(!patterns.iter().any(|p| ".github/workflows/ci-pr.yml".contains(p)));
297+
}
233298
}

crates/terraphim_merge_coordinator/src/gitea.rs

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -123,23 +123,49 @@ impl GiteaClient {
123123
}
124124

125125
/// List files changed in a PR by index. Returns the `filename` of each
126-
/// changed file.
126+
/// changed file, paginating through all pages. Gitea defaults to 50
127+
/// files per page; PRs with more changes would silently miss
128+
/// contamination checks without pagination (issue #2409).
127129
pub async fn list_pr_files(
128130
&self,
129131
owner: &str,
130132
repo: &str,
131133
index: u64,
132134
) -> Result<Vec<String>, MergeCoordinatorError> {
133-
let url = format!(
134-
"{}/api/v1/repos/{}/{}/pulls/{}/files",
135-
self.base_url, owner, repo, index
136-
);
137-
let resp = self.get_with_retry(&url).await?;
138-
let files = resp
139-
.json::<Vec<PrFile>>()
140-
.await
141-
.map_err(|e| MergeCoordinatorError::Api(format!("decode pr files: {e}")))?;
142-
Ok(files.into_iter().map(|f| f.filename).collect())
135+
const PER_PAGE: u32 = 50;
136+
let mut all_files = Vec::new();
137+
let mut page: u32 = 1;
138+
139+
loop {
140+
let url = format!(
141+
"{}/api/v1/repos/{}/{}/pulls/{}/files?page={page}&limit={PER_PAGE}",
142+
self.base_url, owner, repo, index,
143+
);
144+
let resp = self.get_with_retry(&url).await?;
145+
146+
// Use X-Total-Count header to detect last page.
147+
let total_count: Option<u32> = resp
148+
.headers()
149+
.get("x-total-count")
150+
.and_then(|v| v.to_str().ok())
151+
.and_then(|v| v.parse().ok());
152+
153+
let page_files: Vec<PrFile> = resp
154+
.json()
155+
.await
156+
.map_err(|e| MergeCoordinatorError::Api(format!("decode pr files: {e}")))?;
157+
158+
let page_len = page_files.len();
159+
all_files.extend(page_files.into_iter().map(|f| f.filename));
160+
161+
// Stop when we've fetched all items or this page was empty.
162+
if total_count.is_some_and(|t| all_files.len() as u32 >= t) || page_len == 0 {
163+
break;
164+
}
165+
page += 1;
166+
}
167+
168+
Ok(all_files)
143169
}
144170

145171
async fn get_with_retry(&self, url: &str) -> Result<reqwest::Response, MergeCoordinatorError> {

0 commit comments

Comments
 (0)