Skip to content

Commit dccaac1

Browse files
author
Test User
committed
docs: gap remediation plan — 3 active gaps with research, design, and timelines
1 parent c4299a0 commit dccaac1

1 file changed

Lines changed: 146 additions & 0 deletions

File tree

‎.docs/gap-remediation-plan-2026-06-29.md‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
# Gap Remediation Plan — Disciplined Research & Design
2+
3+
**Date**: 2026-06-29
4+
**Gaps identified**: 3 active, 1 already fixed
5+
6+
---
7+
8+
## Gap 1: BlockerKind Classification (PR #2588, Issue #2465)
9+
10+
### Research
11+
12+
**What**: Add `BlockerKind` enum to distinguish CI failures (`ci_failed`, `ci_pending`, `ci_no_status`) from policy holds (`not_mergeable`) in merge coordinator logs.
13+
14+
**Problem**: Operators cannot tell whether a blocked PR failed CI (needs code fix), is pending CI (needs patience), or hit a policy gate (needs human review). Example: PR can be held by CI failure alone, but logs just say "not mergeable".
15+
16+
**Current state on main**: No `BlockerKind` enum exists. `evaluate_one` is synchronous, no CI status querying. `PrSummary` has no `sha` field for commit lookup.
17+
18+
**PR #2588 branch analysis**: 44 files, 27K lines — 90%+ noise (3 session dumps at 9K lines, 2 reconcile snapshots, handoff files). Core code is ~200 lines in `types.rs` (BlockerKind enum), `gitea.rs` (CommitCombinedStatus, get_commit_status, sha field), `evaluator.rs` (async evaluate_one with classification), and `lib.rs` (poll_pending_reviews).
19+
20+
**Essential Questions**:
21+
| Question | Answer |
22+
|----------|--------|
23+
| Energising? | Yes — operators need this daily |
24+
| Leverages strengths? | Yes — core merge coordinator code |
25+
| Meets real need? | Yes — Issue #2465 has real-world examples |
26+
27+
### Design
28+
29+
**Approach**: Cherry-pick only the BlockerKind core from the branch. Create a clean branch from main with 4 files touched.
30+
31+
**Scope**:
32+
| File | Change |
33+
|------|--------|
34+
| `types.rs` | Add `BlockerKind` enum + `Display` impl |
35+
| `gitea.rs` | Add `PrSummary.sha`, `CommitCombinedStatus` struct, `get_commit_status()` |
36+
| `evaluator.rs` | Make `evaluate_one` async, classify blocker via CI status |
37+
| `lib.rs` | Add `poll_pending_reviews()` or enrich existing poll with blocker_kind |
38+
39+
**API Design**:
40+
```rust
41+
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)]
42+
pub enum BlockerKind {
43+
CiFailed,
44+
CiPending,
45+
CiNoStatus,
46+
NotMergeable,
47+
}
48+
```
49+
50+
**Test Strategy**:
51+
| Test | Purpose |
52+
|------|---------|
53+
| `test_blocker_kind_ci_failed` | CI status "failure" → CiFailed |
54+
| `test_blocker_kind_ci_pending` | CI status "pending" → CiPending |
55+
| `test_blocker_kind_no_status` | No CI status → CiNoStatus |
56+
| `test_blocker_kind_not_mergeable` | Mergeable=False, CI green → NotMergeable |
57+
58+
**Estimated effort**: 1 hour
59+
60+
**Verdict**: IMPLEMENT on clean branch. Close PR #2588 with comment linking to new PR.
61+
62+
---
63+
64+
## Gap 2: FirecrackerExecutor::ensure_pool() (PR #2590, #2521, Issue #2521)
65+
66+
### Research
67+
68+
**What**: Implement Firecracker VM pool warming so `ensure_pool()` doesn't always return Err.
69+
70+
**Current state on main**: `ensure_pool()` IS implemented. The function:
71+
1. Checks if pool already exists (returns clone)
72+
2. Creates PoolConfig from RLM config
73+
3. Initialises pool with min/max/target sizes
74+
75+
The function signature exists and compiles. Issue #2521 reported "always returns Err" but the current implementation shows it CAN return Ok.
76+
77+
**PR #2590**: Closed. 42 files, 27K lines (similar noise pattern to #2588).
78+
79+
**Verdict**: **ALREADY FIXED**. The `ensure_pool()` stub was replaced with a real implementation during the merge sprint (likely via PR #2902 or executor changes). Issue #2521 should be closed.
80+
81+
**Close comment**: "`ensure_pool()` is now implemented in main (firecracker.rs) with real PoolConfig from RlmConfig. Verify: `grep -A20 'async fn ensure_pool' crates/terraphim_rlm/src/executor/firecracker.rs`"
82+
83+
---
84+
85+
## Gap 3: Path::starts_with in validate_path (PR #2898, Issue #2884)
86+
87+
### Research
88+
89+
**What**: Use `Path::starts_with` to prevent sibling-directory bypass in `validate_path()`. String-based `starts_with` allows `/safe/a` to match `/safe-attack/x` — Path-based comparison doesn't.
90+
91+
**Current state on main**: `validate_path()` does NOT exist in `gitea.rs`. There's no path validation function in the merge coordinator. This gap is larger than expected — the entire path validation is missing, not just the Path::starts_with fix.
92+
93+
**PR #2898**: Closed (mergeable=True when closed). 1 file, +48/-9 lines. The PR adds `validate_path()` to `gitea.rs` with `Path::starts_with` and tests.
94+
95+
**Verdict**: **NEEDS REOPENING + REBASE**. The PR was closed as "already merged" but the code is not in main. The branch `task/2884-path-starts-with-fix` likely has the changes. Rebase and re-merge.
96+
97+
**Essential Questions**:
98+
| Question | Answer |
99+
|----------|--------|
100+
| Energising? | Medium — security fix for path traversal |
101+
| Leverages strengths? | Yes — simple Rust standard library usage |
102+
| Meets real need? | Yes — Issue #2884 describes a real sibling-dir bypass |
103+
104+
**Estimated effort**: 15 minutes (rebase + merge)
105+
106+
---
107+
108+
## Gap 4: Contamination Gate Pagination (Issue #2409)
109+
110+
### Research
111+
112+
**What**: `list_pr_files` doesn't paginate — returns only the first page of files. PRs with >50 changed files have their contamination gate silently skipped because files from page 2+ are missed.
113+
114+
**Current state on main**:
115+
- `list_pr_files()` exists in `gitea.rs` — calls Gitea API but without `?page=2` pagination
116+
- Contamination gate in `evaluator.rs` does NOT exist (empty grep)
117+
- The entire contamination check is missing from main
118+
- Issue #2409 is open
119+
120+
**More complex than initially thought**: The gap has TWO parts:
121+
1. Add pagination to `list_pr_files()` (loop through pages)
122+
2. Wire the contamination gate into `evaluate_all()` (check file lists for banned patterns)
123+
124+
**Verdict**: **MEDIUM EFFORT**. Split into two sub-tasks:
125+
- Sub-task A: Paginate `list_pr_files()` (simple API loop)
126+
- Sub-task B: Wire contamination gate into evaluation (new logic)
127+
128+
**Estimated effort**: 2-3 hours total
129+
130+
---
131+
132+
## Summary
133+
134+
| # | Gap | Status | Effort | Next Step |
135+
|---|-----|--------|--------|-----------|
136+
| 1 | BlockerKind classification | NOT in main | 1h | Extract from PR #2588, create clean PR |
137+
| 2 | ensure_pool() | ALREADY FIXED | 0 | Close issue #2521 |
138+
| 3 | Path::starts_with validate_path | PR closed, code missing | 15min | Rebase PR #2898, re-merge |
139+
| 4a | list_pr_files pagination | NOT in main | 1h | Add page loop to API call |
140+
| 4b | Contamination gate wiring | NOT in main | 2h | New feature in evaluator.rs |
141+
142+
### Priority Order
143+
1. **Gap 3** (15 min) — Quickest win, PR already written
144+
2. **Gap 2** (0 min) — Already fixed, just close issue
145+
3. **Gap 1** (1h) — Most operator-facing value
146+
4. **Gap 4** (3h) — Largest scope, split into A/B

0 commit comments

Comments
 (0)