Leonid Pershin
dc1fde9e3e
- Updated `resolve_llm_runtime` to prioritize live configuration over legacy notes, ensuring accurate runtime resolution. - Enhanced `tunnel_forwards` to prefer current configuration for LLM runtime, improving tunnel setup logic. - Improved idle-killer logic to handle stale markers and provide clearer warnings in the status output. - Updated CLI documentation in `cli.md` to reflect changes in command behavior and runtime handling. - Enhanced tests to validate new runtime resolution logic and ensure proper handling of configuration states.
91 lines
5.4 KiB
Markdown
91 lines
5.4 KiB
Markdown
# 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
|