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

Critical-only pass (мелочи / style / UX polish ignored). Focus: billing safety, silent GPU left on, security of secrets, broken stop/auto-stop paths.

## Prior Reviews Summary

> Based on `2026-08-21-review-2.md` and `2026-08-21-review-1.md`.

### Still Open (carried forward)

None. All tasks in review-1 and review-2 are `[x]`.

### Resolved Since Last Review

- [x] Prior review-2 closed the fail-closed idle-killer access_rules + revoke on stop, Ollama pull exact-tag, provision_llm not swallowed, etc. Those remain in place; this review finds **new** regressions/gaps around arm false-positive and unreachable Swarm.

---

## Phase 1: Code Quality

### SOLID

No critical issues (god-flow in `session`/`provision` is known debt, not a break).

### Performance

No critical issues.

### Correctness & Bugs

1. **`arm_idle_killer` soft-fails on missing app cred, but provision still marks `armed`.**  
   `create_application_credential` failure is caught, logged (`idle-killer слеп`), and `return`s without raising. Caller always sets `notes["idle_killer"] = "armed"`. Access card only warns on `"failed"`. → User thinks auto-stop works; GPU bills forever.

2. **Remote idle-killer treats any Swarm HTTP error as busy forever.**  
   `swarm_busy`: unreachable → `(True, "swarm unreachable…")`, which resets idle timer every tick. No grace/timeout to delete when Swarm is down for hours. → Crashed Swarm / bad Comfy ExtraArgs → never delete.

3. **`provision_llm` / seed failures can leave ACTIVE compute before killer arm.**  
   Killer is armed last in `provision_vm`. Earlier `CloudError` aborts `up` with server already created and no timer. Known risk amplified by LLM/extensions paths.

4. **Perf tune sets `pip_ok=True` even when `pip install triton/sageattention` fails**, then may still patch `--use-sage-attention` and restart Swarm. Marker prevents retry. → Can leave Swarm broken → compounds (2).

### Code Quality

No critical issues.

---

## Phase 2: Logical Consistency

### Domain & Application Layer

No critical issues.

### Data Flow

Requires-filter for extensions / Ollama install paths / balance notify (notify-only) — no critical billing harm found.

### State Management

`notes.idle_killer = "armed"` does not match actual arm success (see bug 1).

### Consistency

No critical issues.

---

## Phase 3: UI/UX

### Usability

Misleading “armed” status when killer is blind — treated as Bug/Logic above, not UX polish.

### Visual / Interaction / Accessibility

N/A for CLI critical pass (or no critical issues).

---

## Tasks

Critical only:

- [x] 1. [Bug] `arm_idle_killer`: on cred create failure **raise** or return False; never set `notes.idle_killer=armed`; surface same ⚠ as `"failed"` — `src/gpu_rent/idle_killer.py` line 151, `src/gpu_rent/provision.py` line 516
- [x] 2. [Bug] Idle-killer: if Swarm unreachable longer than N minutes (e.g. 2× idle or fixed 60m), treat as idle/allow delete (llm-only already bypasses) — `src/gpu_rent/remote/idle_killer.py` line 75
- [x] 3. [Logic] On mid-`up` failure after server create, arm killer anyway or fail loudly and refuse to leave session without killer / document mandatory `stop` — `src/gpu_rent/provision.py` / `session.py`
- [x] 4. [Bug] Perf tune: set `pip_ok` only if pip succeeded; do **not** write `--use-sage-attention` ExtraArgs unless install OK (or allow retry when `pip_ok` false) — `src/gpu_rent/remote/tune_swarm_perf.py` line 109
- [x] 5. [Security] Write idle-killer creds JSON with `mode=0o600` before `mv` (same as GIT_TOKEN) — `src/gpu_rent/idle_killer.py` line 156

**Verified OK (critical):** `cmd_stop` delete path, tunnel Ctrl+C detach, local-watchdog stop on stale lease, `requires: ollama` filter, balance notify (toast only), Ollama unit bind to data dir, 127 tests green at review time.