Skip to content

Commit 6e37ee4

Browse files
author
Alex
committed
docs: ADF stability roadmap research and design (Phase 1+2) Refs #1122
1 parent 976980f commit 6e37ee4

2 files changed

Lines changed: 526 additions & 0 deletions

File tree

‎.docs/design-adf-stability-roadmap-2026-05-01.md‎

Lines changed: 276 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,276 @@
1+
# Design & Implementation Plan: ADF Stability Roadmap
2+
3+
## 1. Summary of Target Behaviour
4+
5+
After implementation, the ADF will:
6+
- Build, review, and merge PRs automatically via the orchestrator
7+
- Pass `cargo clippy --workspace --all-targets -- -D warnings` and `cargo test --workspace` on every PR
8+
- Have no confidence-score injection vulnerabilities in any agent script
9+
- Have a functional pr-compliance-watchdog agent
10+
- Have running config that matches git-tracked config
11+
- Have file permissions that prevent secret exposure
12+
13+
## 2. Key Invariants and Acceptance Criteria
14+
15+
| ID | Invariant | Acceptance Criteria |
16+
|----|-----------|---------------------|
17+
| AC-1 | No agent has confidence-score injection | `grep -c 'head -1' scripts/adf-setup/agents/pr-reviewer.toml` returns 0 |
18+
| AC-2 | All agents sanitise ADF_PR_NUMBER | All 5 agents contain `tr -cd '0-9'` after the empty check |
19+
| AC-3 | All agents guard ADF_PR_HEAD_SHA | All 5 agents check all 3 env vars |
20+
| AC-4 | pr-compliance-watchdog is valid TOML | `python3 -c "import tomllib; tomllib.load(open('pr-compliance-watchdog.toml','rb'))"` succeeds |
21+
| AC-5 | CI is green | `cargo clippy --workspace --all-targets -- -D warnings` exits 0 |
22+
| AC-6 | Tests pass | `cargo test --workspace` exits 0 |
23+
| AC-7 | Config reconciled | `diff <(git show main:scripts/.../orchestrator.toml) <(ssh bigbox cat /opt/.../orchestrator.toml)` shows only intentional differences |
24+
| AC-8 | File permissions secure | All .toml files in /opt/ai-dark-factory/ are mode 600 |
25+
| AC-9 | Backup files cleaned | No .bak files in conf.d/ |
26+
| AC-10 | Reconciler observed working | At least one remediation issue opened by tick 40 |
27+
28+
## 3. High-Level Design and Boundaries
29+
30+
### 6 Parallel Streams
31+
32+
```
33+
Stream A: Security Hardening Stream B: CI Restoration Stream C: Config Reconciliation
34+
(agent TOML fixes) (clippy + test fixes) (git <-> bigbox sync)
35+
| | |
36+
v v v
37+
Stream D: Branch Protection Stream E: Watchdog Recovery Stream F: Permissions & Cleanup
38+
(status posting fix) (TOML rewrite) (chmod + cargo clean)
39+
```
40+
41+
**Stream boundaries:**
42+
- A, B, E, F: No dependencies, start immediately
43+
- C: No dependencies but should be done in one coordinated pass
44+
- D: Requires A (agents must post correct statuses)
45+
46+
### Changes Inside Existing Components
47+
48+
| Component | Change Type | Stream |
49+
|-----------|------------|--------|
50+
| `scripts/adf-setup/agents/pr-reviewer.toml` | Modify (security fix) | A |
51+
| `scripts/adf-setup/agents/pr-spec-validator.toml` | Modify (security fix) | A |
52+
| `scripts/adf-setup/agents/pr-security-sentinel.toml` | Modify (security fix) | A |
53+
| `scripts/adf-setup/agents/pr-test-guardian.toml` | Modify (env var guards) | A |
54+
| `scripts/adf-setup/agents/pr-compliance-watchdog.toml` | Rewrite | E |
55+
| `crates/terraphim_orchestrator/tests/*.rs` (4 files) | Modify (add field) | B |
56+
| `crates/terraphim_multi_agent/src/agent.rs` | Modify (prefix unused var) | B |
57+
| `crates/terraphim_agent/tests/offline_mode_tests.rs` | Modify (exit codes) | B |
58+
| `crates/terraphim_agent/tests/exit_codes_integration_test.rs` | Modify (listen validation) | B |
59+
| `crates/terraphim_agent/tests/integration_tests.rs` | Modify (schema fix) | B |
60+
| `crates/terraphim_agent/src/client.rs` or `terraphim_server/src/api.rs` | Modify (ThesaurusResponse) | B |
61+
| `scripts/adf-setup/scripts/adf-setup/orchestrator.toml` | Modify (sync to running) | C |
62+
| `/opt/ai-dark-factory/conf.d/*.toml` | Permissions change | F |
63+
64+
### New Components
65+
66+
None -- all changes are modifications to existing files.
67+
68+
## 4. File/Module-Level Change Plan
69+
70+
### Stream A: Security Hardening (5 files)
71+
72+
| File | Action | Change | Dependencies |
73+
|------|--------|--------|--------------|
74+
| `pr-reviewer.toml` | Modify | Replace `2>&1` with `--output-format text 2>/dev/null`, parse only `REVIEW_FILE`, use `tail -1` instead of `head -1`, add ADF_PR_NUMBER sanitisation, add ADF_PR_HEAD_SHA guard | None |
75+
| `pr-spec-validator.toml` | Modify | Replace `head -1` with `tail -1`, add ADF_PR_NUMBER sanitisation, add ADF_PR_HEAD_SHA guard | None |
76+
| `pr-security-sentinel.toml` | Modify | Replace `head -1` with `tail -1`, add ADF_PR_NUMBER sanitisation, add ADF_PR_HEAD_SHA guard | None |
77+
| `pr-test-guardian.toml` | Modify | Add ADF_PR_NUMBER sanitisation, fix `HEAD_SHORT` env export bug | None |
78+
| `pr-compliance-watchdog.toml` | Rewrite | Extract TOML from double-nested diff OR rewrite from pr-spec-validator template with compliance-specific prompt | None |
79+
80+
### Stream B: CI Restoration (7-8 files)
81+
82+
| File | Action | Change | Dependencies |
83+
|------|--------|--------|--------------|
84+
| `tests/orchestrator_tests.rs` | Modify | Add `gate_reconcile_interval_ticks: 20,` to `test_config()` | None |
85+
| `tests/pause_and_breaker_tests.rs` | Modify | Add `gate_reconcile_interval_ticks: 20,` to `test_config()` | None |
86+
| `tests/auto_merge_tests.rs` | Modify | Add `gate_reconcile_interval_ticks: 20,` to `test_config()` | None |
87+
| `tests/auto_merge_execution_tests.rs` | Modify | Add `gate_reconcile_interval_ticks: 20,` to `test_config()` | None |
88+
| `terraphim_multi_agent/src/agent.rs:1348` | Modify | Rename `agent` to `_agent` in `test_hook_manager_initialized()` | None |
89+
| `terraphim_agent/tests/offline_mode_tests.rs` | Modify | Fix exit code assertions (3 for missing index, 6 for network error) | None |
90+
| `terraphim_agent/tests/exit_codes_integration_test.rs` | Modify | Implement `--server` flag rejection in `listen` subcommand OR update test expectation | None |
91+
| `terraphim_agent/tests/integration_tests.rs` + `client.rs` | Modify | Align ThesaurusResponse schemas between client and server | None |
92+
93+
### Stream C: Config Reconciliation (2 files)
94+
95+
| File | Action | Change | Dependencies |
96+
|------|--------|--------|--------------|
97+
| `scripts/adf-setup/scripts/adf-setup/orchestrator.toml` | Modify | Add `[mentions]`, `[pr_dispatch]`, `gate_reconcile_interval_ticks`, update `tick_interval_secs` to 300, `probe_ttl_secs` to 1800 | None |
98+
| `/etc/systemd/system/adf-orchestrator.service` | Modify | Update `MemoryMax` to 64G, commit to git | None |
99+
100+
### Stream D: Branch Protection Fix (1 file)
101+
102+
| File | Action | Change | Dependencies |
103+
|------|--------|--------|--------------|
104+
| `scripts/adf-setup/agents/build-runner.toml` | Modify | Remove `event_only = true` or add PR-open dispatch logic | Requires A |
105+
106+
### Stream F: Permissions & Cleanup (bigbox only)
107+
108+
| File | Action | Change | Dependencies |
109+
|------|--------|--------|--------------|
110+
| `/opt/ai-dark-factory/*.toml` | Permission change | `chmod 600` on all .toml files | None |
111+
| `/opt/ai-dark-factory/conf.d/*.bak*` | Delete | Remove 17 backup files | None |
112+
113+
## 5. Step-by-Step Implementation Sequence
114+
115+
### Stream A: Security Hardening (parallel agents can work simultaneously)
116+
117+
**Step A1**: Fix pr-reviewer.toml confidence-score injection
118+
- Replace `2>&1` with `--output-format text 2>/dev/null`
119+
- Remove `SCORE_TEXT="$SCORE_TEXT\n$REVIEW_OUTPUT"` concatenation
120+
- Parse only `REVIEW_FILE` content for score
121+
- Replace `head -1` with `tail -1`
122+
- Add `ADF_PR_NUMBER=$(printf '%s' "$ADF_PR_NUMBER" | tr -cd '0-9')` with empty guard
123+
- Add `ADF_PR_HEAD_SHA` to env var guard
124+
- System deployable: yes (TOML is only read on next PR dispatch)
125+
126+
**Step A2**: Fix pr-spec-validator.toml
127+
- Replace `head -1` with `tail -1`
128+
- Add ADF_PR_NUMBER sanitisation
129+
- Add ADF_PR_HEAD_SHA guard
130+
- System deployable: yes
131+
132+
**Step A3**: Fix pr-security-sentinel.toml
133+
- Replace `head -1` with `tail -1`
134+
- Add ADF_PR_NUMBER sanitisation
135+
- Add ADF_PR_HEAD_SHA guard
136+
- System deployable: yes
137+
138+
**Step A4**: Fix pr-test-guardian.toml
139+
- Add ADF_PR_NUMBER sanitisation
140+
- Export `HEAD_SHORT` or use `ADF_PR_HEAD_SHA` directly in Python subprocess
141+
- System deployable: yes
142+
143+
**Step A5**: Recover pr-compliance-watchdog.toml
144+
- Attempt to extract TOML from double-nested diff
145+
- If extraction fails, rewrite from pr-spec-validator template
146+
- Add same sanitisation/guarding pattern
147+
- System deployable: yes
148+
149+
**Step A6**: Deploy all 5 TOMLs to bigbox
150+
- Copy to `/opt/ai-dark-factory/conf.d/` (if that's where agents read from)
151+
- OR run `migrate-to-confd.py` to deploy all 15 agents
152+
- Restart ADF orchestrator
153+
154+
### Stream B: CI Restoration (parallel)
155+
156+
**Step B1**: Fix 4 orchestrator test files (add missing field)
157+
- Add `gate_reconcile_interval_ticks: 20,` to each `test_config()` function
158+
- Verify: `cargo test -p terraphim_orchestrator --no-run`
159+
160+
**Step B2**: Fix unused variable in multi_agent test
161+
- Rename `agent` to `_agent` at `agent.rs:1348`
162+
- Verify: `cargo clippy --workspace --all-targets -- -D warnings`
163+
164+
**Step B3**: Fix offline_mode_tests exit codes
165+
- Update assertions to match `ExitCode` enum: 3 (ErrorIndexMissing), 6 (ErrorNetwork)
166+
- Verify: `cargo test -p terraphim_agent --test offline_mode_tests`
167+
168+
**Step B4**: Fix ThesaurusResponse schema
169+
- Option (a): Update `client.rs` to match server response (`thesaurus` field)
170+
- Option (b): Update server `api.rs` to return `terms` field
171+
- Recommendation: Option (b) -- server should return the same schema the client expects
172+
- Verify: `cargo test -p terraphim_agent --test integration_tests`
173+
174+
**Step B5**: Fix listen_mode_with_server_flag test
175+
- Either: implement `--server` rejection in the `listen` subcommand
176+
- Or: update test to reflect current behaviour
177+
- Recommendation: implement the rejection (3-line change in clap)
178+
- Verify: `cargo test -p terraphim_agent --test exit_codes_integration_test`
179+
180+
**Step B6**: Fix test_full_feature_matrix offline path
181+
- Ensure Default role has a knowledge graph configured in test fixtures, or skip when unavailable
182+
- Verify: `cargo test -p terraphim_agent --test integration_tests test_full_feature_matrix`
183+
184+
**Step B7**: Full validation
185+
- `cargo clippy --workspace --all-targets -- -D warnings`
186+
- `cargo test --workspace`
187+
- `cargo fmt --check`
188+
189+
### Stream C: Config Reconciliation (sequential)
190+
191+
**Step C1**: Update git-tracked orchestrator.toml to match running values
192+
- Set `tick_interval_secs = 300`
193+
- Set `probe_ttl_secs = 1800`
194+
- Add `[mentions]` section
195+
- Add `[pr_dispatch]` section
196+
- Add `gate_reconcile_interval_ticks = 20`
197+
- Commit with message: `chore(adf): reconcile config with running values Refs #1118`
198+
199+
**Step C2**: Update git-tracked service file
200+
- Set `MemoryMax=64G`
201+
- Commit with message: `chore(adf): update MemoryMax to 64G Refs #1118`
202+
203+
**Step C3**: Clean backup files on bigbox
204+
- `rm /opt/ai-dark-factory/conf.d/*.bak*`
205+
- Verify no drift between git and running
206+
207+
### Stream D: Branch Protection Fix (after A completes)
208+
209+
**Step D1**: Fix build-runner to dispatch on PR open events
210+
- Either: remove `event_only = true` from build-runner.toml
211+
- Or: add a webhook handler that dispatches build-runner when a PR is opened/reopened
212+
- Deploy updated build-runner.toml
213+
214+
**Step D2**: Verify pr-reviewer posts commit status
215+
- Ensure pr-reviewer.toml posts `adf/pr-reviewer` status on every PR review (pass or fail)
216+
- The orchestrator's Phase 5 `post_terminal_commit_status()` should handle this for the build context
217+
218+
**Step D3**: Observe status checks appearing on an open PR
219+
- Wait for next tick cycle or trigger manually
220+
- Verify `adf/build` and `adf/pr-reviewer` appear in commit statuses
221+
222+
### Stream F: Permissions & Cleanup (parallel, 5 minutes)
223+
224+
**Step F1**: Fix file permissions on bigbox
225+
```bash
226+
ssh bigbox "sudo chmod 600 /opt/ai-dark-factory/orchestrator.toml /opt/ai-dark-factory/conf.d/*.toml"
227+
```
228+
229+
**Step F2**: Clean backup files
230+
```bash
231+
ssh bigbox "rm /opt/ai-dark-factory/conf.d/*.bak* /opt/ai-dark-factory/conf.d.backup-* -rf"
232+
```
233+
234+
**Step F3**: Verify permissions
235+
```bash
236+
ssh bigbox "ls -la /opt/ai-dark-factory/*.toml /opt/ai-dark-factory/conf.d/*.toml"
237+
```
238+
239+
## 6. Testing & Verification Strategy
240+
241+
| AC | Test Type | Test Command / Location |
242+
|----|-----------|------------------------|
243+
| AC-1 | Grep assertion | `grep -c 'head -1' scripts/adf-setup/agents/pr-reviewer.toml` == 0 |
244+
| AC-1 | Grep assertion | `grep -c 'tail -1' scripts/adf-setup/agents/pr-reviewer.toml` >= 1 |
245+
| AC-1 | Grep assertion | `grep -c 'REVIEW_OUTPUT' scripts/adf-setup/agents/pr-reviewer.toml` after SCORE_TEXT block == 0 |
246+
| AC-2 | Grep assertion | `grep -l 'tr -cd' scripts/adf-setup/agents/*.toml` lists all 5 files |
247+
| AC-3 | Grep assertion | `grep -l 'ADF_PR_HEAD_SHA' scripts/adf-setup/agents/*.toml` lists all 5 files |
248+
| AC-4 | Unit test | `python3 -c "import tomllib; tomllib.load(open('pr-compliance-watchdog.toml','rb'))"` |
249+
| AC-5 | CI gate | `cargo clippy --workspace --all-targets -- -D warnings` |
250+
| AC-6 | CI gate | `cargo test --workspace` |
251+
| AC-7 | Integration | `diff <(git show main:path) <(ssh bigbox cat path)` |
252+
| AC-8 | Integration | `ssh bigbox stat -c '%a' /opt/ai-dark-factory/orchestrator.toml` == 600 |
253+
| AC-9 | Integration | `ssh bigbox ls /opt/ai-dark-factory/conf.d/*.bak* 2>&1` == "No such file" |
254+
| AC-10 | Observation | Gitea issue opened by reconciler within 2 hours |
255+
256+
## 7. Risk & Complexity Review
257+
258+
| Risk | Mitigation | Residual Risk |
259+
|------|------------|---------------|
260+
| Agent TOML change breaks PR review pipeline | Test locally with `bash -n`, deploy one agent at a time | Edge case in bash quoting |
261+
| ThesaurusResponse fix breaks existing clients | Check all callers of server endpoint | Client version mismatch |
262+
| Config reconciliation restarts ADF during active work | Deploy during low activity, keep backup | Tick in progress may be interrupted |
263+
| Reconciler opens flood of remediation issues | Dedup via `remediation_key()`, limit to 1 per PR per tick | Initial burst after first run |
264+
| pr-compliance-watchdog extraction fails | Rewrite from template (Step A5 fallback) | Compliance rules may differ |
265+
266+
## 8. Open Questions / Decisions for Human Review
267+
268+
1. **ThesaurusResponse fix direction**: Fix server to match client (option b), or fix client to match server (option a)? Server change risks breaking other consumers; client change is safer.
269+
270+
2. **listen --server rejection**: Implement the rejection in clap (adds validation), or update the test to match current permissive behaviour?
271+
272+
3. **build-runner dispatch trigger**: Remove `event_only = true` (runs on every tick that has a PR), or add webhook-based dispatch (more precise but more complex)?
273+
274+
4. **Config reconciliation timing**: Should Stream C be committed before or after Stream B? Before means CI tests the reconciled config; after means CI is green first.
275+
276+
5. **pr-compliance-watchdog recovery**: Extract from diff (preserves exact original) or rewrite from template (cleaner but may miss compliance-specific logic)?

0 commit comments

Comments
 (0)