Godopu
f57cd5cdde
- Delete adapters/cline.py and unregister from registry.py - Remove cline branches from lib.sh and all 8 skill scripts (create, resume, stop, status, reconcile, update_yaml_resumed, resolve_session_id, orc_onboard) - Narrow own-key mapping dictionaries across lib_py core modules to 4 supported agents - Delete cline-exclusive tests and retarget shared fixtures to grok/hermes/claude - Update skills documentation and installation guides (439 passed, 0 failures) - Archive cline deprecation consensus and review reports
49 lines
6.0 KiB
Markdown
49 lines
6.0 KiB
Markdown
# 🔍 Cross-Code Review — Complete Cline Removal Implementation (Job 20d45d12)
|
||
|
||
- **Reviewer**: `planner-reviewer-claude-01`
|
||
- **Target diff**: implementation of `plan-264c3b5d.md` Rev.2 — 30 files (adapter deletion, registry, 4 `lib_py` modules, `lib.sh`, 9 skill scripts, 6 test files, 3 docs) plus 1 out-of-scope test-flakiness fix.
|
||
- **Method**: read every changed file's live post-diff state directly (not diff text alone), independently verified the two highest-risk items from my own Rev.2 plan (the `reconcile.sh` drift-C block boundary and the tiered-readiness/modal test retargeting), syntax-checked all 10 modified shell scripts, grepped the entire diff for any surviving `cline` reference, and ran the full test suite myself.
|
||
|
||
---
|
||
|
||
## 1. Fidelity to Rev.2 Plan — Verified, Not Assumed
|
||
|
||
I did not trust the implementation's own claim of compliance — I re-checked the specific corrections `creator-agy-01`'s challenge required in Rev.2 against the live diff:
|
||
|
||
- **Tiered-readiness tests (Rev.2's core correction)**: `test_c3_strong_token_and_hint_token_readiness_succeeds` now mocks `_delegate_py_bin`/`python -m lib_py.agents facts` to return a synthetic `mocktiered` agent with genuinely distinct `MAM_STRONG_READY_TOKENS='MockApp'` / `MAM_WEAK_READY_TOKENS='Use arrow keys'`, keeping the real facts-bridge call path exercised rather than bypassing it with raw env-var pre-injection — exactly what Rev.2 required. The other C4–C7 tests that were already using pre-set env vars (not resolving through the bridge in the original cline-based version either) were correctly left as simple session-name swaps, since they were never testing the bridge to begin with.
|
||
- **`test_c8b` (modal test)**: retargeted fully to `claude`, using `'Try the new fullscreen renderer?'` as the injected screen text and `MAM_MODAL_TOKENS='Try the new fullscreen renderer\?'`, with session name `my-project-worker-claude-01` — this is claude's actual, verified `modal_tokens` value, exactly matching Rev.2's requirement to preserve the full session-name-resolution → `facts claude` → dialog-block path, not a synthetic placeholder.
|
||
- **New facts-bridge round-trip assertion**: `test_a4_adapter_contract.py::test_adapter_required_properties` gained `assert adapter.strong_ready_tokens == adapter.ready_tokens` / `assert adapter.weak_ready_tokens == ''` for all 4 remaining agents, and `test_facts_bridge_eval_contract` now additionally asserts `STRONG=`/`WEAK=` come through the real bash `eval` of the bridge's output — this is actually a **stronger** implementation than what I asked for (I only required the property-level check; this round-trips through the real subprocess + bash eval too).
|
||
- **`docs/NEW_AGENT_INTEGRATION_GUIDE.md`**: all 5 sites I flagged in Rev.2 (diagram, `_ADAPTERS` sample, `lib.sh` kind-mapping sample, binary-tuple sample, test-assertion sample) were updated — the architecture diagram box-drawing was even correctly realigned (`┬` connector fixed) after swapping `ClineAgentAdapter` for `GrokAgentAdapter` in that slot, not just text-deleted.
|
||
|
||
## 2. Independent Verification of the Highest-Risk Edit
|
||
|
||
I flagged the `reconcile.sh` cline drift-C block deletion as the highest-risk single edit in my own plan. Checked the live file directly: the block is cleanly gone, the preceding `hermes` drift-C block and the following `result = {...}` return statement are both intact and correctly adjacent with no orphaned fragments. Extracted and `ast.parse()`'d the actual `RECON_SRC` heredoc (lines 320–794, not the other heredoc earlier in the file, which I made sure to distinguish) — valid Python. `bash -n` on the whole file — valid.
|
||
|
||
## 3. Completeness Check
|
||
|
||
`git diff | grep -n "^+.*[Cc]line"` (every added line, across the entire diff) returns **zero matches** — no newly-written line anywhere in this diff still references cline. Cross-checked a full-repo `cline` grep against `git status`: every remaining match is either inside `.agents/reports/**` (untouched, correct) or inside changelog-style docs (`VERSIONS.md`, `IMPROVEMENTS.md`) describing past releases in the past tense (correctly left alone, consistent with my plan's "spot-check, don't blanket-edit" guidance).
|
||
|
||
## 4. Findings
|
||
|
||
### 4.1 Minor: `MULTI_AGENT_RULES.md`/`.ko.md` line 21 slightly stale (Low, not blocking)
|
||
|
||
`"Newly spawned agents (e.g., antigravity, claude, cline, hermes) act as Team Leaders..."` — an illustrative `e.g.` list, not a hard enumeration, but it does still name cline as a live example post-removal. Low severity since the sentence's substance is about the *role concept*, not a supported-agent contract, and this file wasn't in either of our removal plans' scope. Worth a follow-up touch-up, not blocking.
|
||
|
||
### 4.2 Out-of-scope change present in the diff (informational, not a defect)
|
||
|
||
`tests/test_o2_race_free_lock.py` was modified — replacing a fixed `time.sleep(0.3)` in `acquire_bg()` with an active poll-until-marker-file-written loop (up to 2s, with early exit if the background process dies). This has nothing to do with cline removal; it's a flaky-test timing fix, most likely surfaced while chasing "100% pass, zero regressions" during implementation. I reviewed the change itself: it's strictly safer than what it replaces (removes a fixed-sleep race assumption, fails faster on a dead process) and doesn't touch cline-adjacent code. Flagging for transparency/scope-discipline reasons, not as a defect — I would not block on this alone.
|
||
|
||
## 5. Full Test Suite
|
||
|
||
```
|
||
.venv/bin/python -m pytest tests/ -q
|
||
→ 439 passed in 655.22s (0:10:55), exit code 0
|
||
```
|
||
Ran to completion myself (not the diff's own claim). **Zero failures, zero regressions.**
|
||
|
||
## 6. Verdict
|
||
|
||
Every site from my own Rev.2 plan was implemented faithfully and, in two places (the facts-bridge round-trip assertion, the architecture-diagram realignment), more thoroughly than the plan strictly required. No orphaned `cline` references anywhere in the diff. The highest-risk edit (`reconcile.sh`'s block deletion) is clean and syntactically valid. One low-severity doc staleness and one out-of-scope-but-safe test fix are noted, neither blocking.
|
||
|
||
[VERDICT: PASS]
|