# Project Review — 2026-08-21 (2)

## Prior Reviews Summary

> Based on the last 3 review files analysed in Phase 0 (only `2026-08-21-review-1.md` exists).

### Still Open (carried forward)
None.

### Resolved Since Last Review
- [x] [source: 2026-08-21-review-1, tasks 1–13] All prior tasks closed (wait_ssh, bootstrapped clear, access_rules attempt, GIT scrub, SG prune, CIDR example, SSH reuse in sync, light bootstrap flag, phase bootstrapping, Civitai non-blocking, session tests, journalctl logs, architecture docs).

---

## Phase 1: Code Quality

### SOLID
- `session.cmd_up` / `_bind_access` still orchestrate cloud + SSH + bootstrap + provision + LLM + snapshot + notify (god-flow). Acceptable for a small CLI, but growing with LLM.
- New modules (`llm_runtime`, `setup_wizard`, `access_card`) are reasonably scoped.

### Performance
- `arm_idle_killer` still opens many short SSH sessions (`put_text` / `run_ssh` per step) — sync_files was fixed in review-1; idle-arm was not.
- Every `up` still runs full `doctor` (Keystone + Glance + …) even when user already ran doctor.

### Correctness & Bugs
- Ollama pull skip can treat a different tag of the same model as “already present” (`listed()` stores bare name; match uses `startswith`).
- Pull marker `.gpu-rent-ollama-pulling` can stick after SSH timeout → idle-killer forever busy.
- `provision_llm` exceptions swallowed → tunnel/access card can show LLM URLs while install failed; `notes.llm_runtime` may stay stale.
- Light bootstrap requires `state.bootstrapped` **and** VM marker; `stop` clears local `bootstrapped` → almost always full apt after stop→up (conflicts with review-1 perf goal).
- `clone_ext` `SystemExit(2)` on origin mismatch skips token file unlink in `main()`.
- App cred `access_rules` include `/v2.1/servers/*` (any server) plus unrestricted fallback; not revoked on `stop`.

### Code Quality
- Dead branch in `push_tree` (`_is_weight_name` + empty `pass`).
- `OllamaModelEntry.default` written/parsed but unused in pull.
- Invalid `LLM_RUNTIME` in env silently coerced to `none`.

---

## Phase 2: Logical Consistency

### Domain & Application Layer
- Docs/architecture updated for LLM; product still SwarmUI-first with opt-in LLM — consistent with decisions.

### Data Flow
- Tunnel / `open --llm` / access card prefer `state.notes["llm_runtime"]` over live config — desync if provision failed or vars changed without re-provision.

### State Management
- `LLM_RUNTIME=none` does not stop/disable remote LLM systemd units → GPU may stay occupied.
- Soft-fail `wait_backend_idle` / soft-fail `arm_idle_killer` still yield `ready_cloud` / “ready” UX while killer may be blind.

### Consistency
- Error handling: mix of hard `CloudError` (bootstrap) vs soft log (LLM, killer, ready). Uneven.

---

## Phase 3: UI/UX (CLI)

### Usability
- Bare `gpu-rent` invokes full interactive `up` (expensive surprise).
- Full doctor table on every `up` is noisy.
- Interactive Ollama preset typo raises raw `ValueError`.
- Access card at tunnel end is a clear UX win.

### Visual & Consistency
- Rich access panel is coherent; toast text still SwarmUI-only (minor).

### Interaction & Feedback
- `--no-tunnel` still shows access card with tunnel hint — good.
- Soft LLM failure gives little signal beyond a log line.

### Accessibility
- N/A (CLI).

---

## Tasks

- [x] 1. [Security] Narrow idle-killer access_rules to this `server_id` only (drop `/servers/*`); fail closed or warn loudly instead of unrestricted fallback; revoke app cred on `stop`/`destroy` — `src/gpu_rent/idle_killer.py` line 57
- [x] 2. [Security] Ensure `GIT_TOKEN` file is always removed (`try/finally` around `main`, convert `SystemExit` paths to raised errors) — `src/gpu_rent/remote/clone_ext.py` line 116
- [x] 3. [Bug] Fix Ollama pull skip: match exact tag only (do not skip `foo:7b` because `foo:3b` exists) — `src/gpu_rent/remote/ollama_pull.py` line 14
- [x] 4. [Bug] Age out or clear stale `.gpu-rent-ollama-pulling` in idle-killer (e.g. ignore marker older than N minutes) — `src/gpu_rent/remote/idle_killer.py` line 93
- [x] 5. [Logic] Do not swallow `provision_llm` failures; update `notes.llm_runtime` only after success; prefer cfg over stale notes when they disagree — `src/gpu_rent/provision.py` line 340
- [x] 6. [Logic] When `LLM_RUNTIME=none` (or switching runtime), stop/disable previous `gpu-rent-ollama` / `gpu-rent-llamacpp` units — `src/gpu_rent/provision.py` line 262
- [x] 7. [Performance] Light bootstrap when VM marker exists even if local `bootstrapped=False` after stop (or restore bootstrapped from marker after SSH) — `src/gpu_rent/session.py` line 101
- [x] 8. [UX] Bare `gpu-rent` should show help or a short menu, not auto-`up`; keep launcher `GPU_RENT_DEFAULT_ARGS` for double-click — `src/gpu_rent/cli.py` line 53
- [x] 9. [UX] Quiet doctor on `up` (summary / only failures) unless `--verbose` — `src/gpu_rent/cli.py` line 403
- [x] 10. [Logic] Surface idle-killer arm failure as warning in status/access card, not silent soft-log only — `src/gpu_rent/provision.py` line 344
- [x] 11. [CodeQuality] Remove dead `_is_weight_name` pass in `push_tree`; use `OllamaModelEntry.default` or drop the field — `src/gpu_rent/sync_files.py` line 44
- [x] 12. [Security] Pin/checksum Ollama install script and llama.cpp release asset (or document supply-chain risk) — `src/gpu_rent/remote/install_ollama.sh` line 20