gpu-rent/docs/reviews/2026-08-21-review-1.md

Project Review — 2026-08-21 (1)

Prior Reviews Summary

Based on the last 3 review files analysed in Phase 0.

Still Open (carried forward)

None.

Resolved Since Last Review

None.

---

Phase 1: Code Quality

SOLID

• session.cmd_up / _bind_access orchestrate cloud, SSH, bootstrap, provision, snapshot, notify — god-flow (src/gpu_rent/session.py).
• Docs name separate modules (civitai_seed, models_push, git_seed, watchdog, reconcile) that do not exist as packages; logic is folded into provision.py, sync_files.py, tunnel.py, cli.status.
• Config dataclass is a wide bag of unrelated settings (auth, paths, idle, autocomplete, civitai).

Performance

• push_tree / pull_tree: per-file remote_sha256 + put_file each open a new SSH (sync_files.py, ssh_ops.py).
• Every up re-runs full bootstrap.sh (apt-get) via _bind_access even when already bootstrapped.
• scan_pools on every up: extra Keystone authorize + per-flavor HTTP extra_specs (ThreadPool 16).
• Tunnel watchdog: connect() / Keystone authorize every ~30s.

Correctness & Bugs

• wait_ssh aborts after 3 AuthenticationExceptions (~15s) while cloud-init may still be injecting keys (ssh_ops.py ~114–120).
• cmd_stop leaves bootstrapped=True (session.py ~397–400) → next ACTIVE path mis-labeled / wrong branch when FIP cleared.
• phase set to ready_cloud before bootstrap finishes (session.py ~344–346).
• Application credential created without role/access_rules restriction (idle_killer.py ~52–57) vs architecture “delete/shelve only”.
• GIT_TOKEN embedded in git remote URL and never stripped (clone_ext.py with_token / clone).
• SG rules accumulate old CIDRs; never prune (cloud.py ensure_security_group).
• Session tests out of sync: no bootstrapped+FIP setup for “second GPU”; run_bootstrap mock missing update= (tests/test_session.py).

Code Quality

• State machine phases in docs (bootstrapping, seeding_*, waiting_ui) never written by code (only idle / provisioning / ready_*).
• os_client.connect reports app_version="0.1.0" while package is 0.2.0.
• Duplicated SwarmUI busy/Idle polling logic in ready.py and remote/idle_killer.py.

---

Phase 2: Logical Consistency

Domain & Application Layer

• Architecture table invents module names; actual layout is flatter (cli → session/doctor/tunnel → cloud/provision/ssh_ops).

Data Flow

• Config sources: .env (dotenv override=False) then gpu-rent.vars (override empty only) — OK.
• Volume IDs: state preferred over BOOT_VOLUME_ID / DATA_VOLUME_ID env — OK; AZ not re-validated on reuse.

State Management

• bootstrapped not cleared on stop.
• bootstrapped True only at end of _bind_access; failure mid-bind leaves ready_cloud + bootstrapped=False.
• Marker files on VM (/opt/swarmui/.gpu-rent-bootstrapped) independent of local state.bootstrapped.

Consistency

• Error handling: mostly GpuRentError / CloudError; some soft-log (wait_backend_idle, snapshot, idle-killer arm).
• Civitai bad token is blocking in doctor → blocks up even if user only needs default SwarmUI models.

---

Phase 3: UI/UX (CLI)

Usability

• up always prints full doctor table (noisy).
• Invalid/expired CIVITAI_API_TOKEN blocks entire session start.
• destroy has no --no-pull (always respects PULL_OUTPUT via cmd_stop).

Visual & Consistency

• N/A (CLI). Mixed RU/EN messages by design.

Interaction & Feedback

• Confirm on up without --yes is good; --yes skips.
• Tunnel Ctrl+C messaging is clear.
• logs only tails cloud-init + is-active, not journalctl -u swarmui as docs imply.

Accessibility

• N/A.

---

Full exploration notes (sections 1–10)

See chat report for narrative. Tasks below are actionable.

---

Tasks

• 1. [Bug] Soften wait_ssh early auth abort (retry until timeout / distinguish “sshd up, keys not ready”) — src/gpu_rent/ssh_ops.py line 114
• 2. [Bug] Clear bootstrapped=False (and align phase) in cmd_stop — src/gpu_rent/session.py line 397
• 3. [Security] Restrict idle-killer application credential (access_rules / roles delete-only) — src/gpu_rent/idle_killer.py line 52
• 4. [Security] Strip GIT_TOKEN from git origin after clone/fetch — src/gpu_rent/remote/clone_ext.py line 30
• 5. [Security] Stop accumulating stale SSH SG CIDRs; replace or prune old /32 — src/gpu_rent/cloud.py line 154
• 6. [Security] Revisit default GPU_RENT_SSH_CIDR=0.0.0.0/0 in env.example — env.example line 16
• 7. [Performance] Reuse one SSH/SFTP session in push_tree / pull_tree / hash checks — src/gpu_rent/sync_files.py line 15
• 8. [Performance] Skip full bootstrap (or apt) when VM already marked bootstrapped — src/gpu_rent/session.py line 87
• 9. [Logic] Do not set phase=ready_cloud before _bind_access succeeds — src/gpu_rent/session.py line 344
• 10. [Logic] Make doctor Civitai failure non-blocking when seed is optional — src/gpu_rent/doctor.py line 271
• 11. [CodeQuality] Fix tests/test_session.py mocks (bootstrapped+FIP; run_bootstrap(..., update=)) — tests/test_session.py line 40
• 12. [UX] logs should include journalctl -u swarmui as docs claim — src/gpu_rent/cli.py line 404
• 13. [Logic] Update docs/architecture.md module names, phases, questionary, hold --minutes wording — docs/architecture.md line 37

Closure notes (2026-08-21)

• wait_ssh: auth streak give-up after 180s continuous AuthenticationException (not 3 attempts).
• Idle-killer: access_rules for GET/DELETE server; fallback without rules if Keystone rejects.
• Bootstrap light path: GPU_RENT_BOOTSTRAP_LIGHT=1 skips apt when marker present.
• Also fixed os_client app_version → package __version__ (noted in Phase 1, not a numbered task).