Skip to content

feat(cli): Add multi-interface GCP provisioning tools (CLI, TUI, Desktop, Web)#2831

Draft
jiridanek wants to merge 10 commits intoopendatahub-io:mainfrom
jiridanek:jd/2026/01/gcp-provision-cli-tui
Draft

feat(cli): Add multi-interface GCP provisioning tools (CLI, TUI, Desktop, Web)#2831
jiridanek wants to merge 10 commits intoopendatahub-io:mainfrom
jiridanek:jd/2026/01/gcp-provision-cli-tui

Conversation

@jiridanek
Copy link
Copy Markdown
Member

@jiridanek jiridanek commented Jan 17, 2026

Multi-Interface GCP Provisioning Tools

This PR adds a comprehensive set of tools for provisioning GCP instances with sane defaults, preventing common developer pain points.

Problem Statement

Developers frequently hit these issues when provisioning GCP instances:

Problem Root Cause Impact
🔴 SSH hangs/unresponsive e2-micro has 1GB RAM, 0.25 vCPU Complete lockout, requires serial console
🔴 DNF operations fail No swap configured OOM kills, lost work
🟡 2-5 minute DNF waits 550MB+ metadata from 17+ repos Wasted time
🟡 Forgotten setup steps Manual configuration Inconsistent environments

Solution: Five Interfaces, One Core Logic

We provide the same provisioning logic through 5 different UIs, each optimized for a different use case:

┌─────────────────────────────────────────────────────────────────┐
│                        User Interfaces                          │
├─────────────┬─────────────┬─────────────┬───────────┬───────────┤
│  Typer CLI  │ Textual TUI │   Trogon    │   Flet    │  NiceGUI  │
│  (scripts)  │   (SSH)     │ (auto-TUI)  │ (desktop) │   (web)   │
├─────────────┴─────────────┴─────────────┴───────────┴───────────┤
│                     Core Logic (command.py)                      │
│  • Machine type validation    • Startup script generation       │
│  • Blocked types list         • gcloud command construction      │
└─────────────────────────────────────────────────────────────────┘

📸 Screenshots

1. NiceGUI Web UI (Browser-based)

Initial form with configuration options:

NiceGUI Form

Dark theme with Quasar components, instance configuration form on left, log panel on right

Machine type dropdown with blocked e2-micro warning:

NiceGUI Warning

Orange warning appears when selecting blocked machine type

Provisioning in progress with real-time log:

NiceGUI Provisioning

Shows startup script generation and gcloud command execution


2. Textual TUI (Terminal-based, works over SSH)

Textual TUI

Two-panel layout with form on left, log on right. Machine type dropdown expanded showing all options including blocked e2-micro with ⚠ warning.

Key features:

  • ✅ Works over SSH (no X11 required)
  • ✅ Mouse support in modern terminals (Kitty, Ghostty, iTerm2)
  • ✅ Keyboard navigation
  • ✅ 60fps smooth rendering

3. Flet Desktop App (Flutter/Skia native window)

Flet Desktop

Native macOS window with Material Design 3 dark theme. GPU-accelerated 60fps rendering via Flutter's Skia engine.

Key features:

  • ✅ Native OS window with traffic lights
  • ✅ Smooth animations
  • ✅ Same code runs as web app (flet run --web)

4. Trogon Auto-TUI (CLI explorer)

Trogon TUI

Auto-generated TUI from CLI commands. Navigate command tree on left, fill forms on right, press ^r to run.


Usage

cd cli/commands/provision

# 1. CLI (for scripts/automation)
uv run --with typer python run_cli.py create my-instance --zone us-central1-a

# 2. TUI (for terminal users, works over SSH)
uv run --with textual python run_tui.py

# 3. Desktop App (native window)
uv run --with flet python flet_app.py

# 4. Web UI (opens browser at localhost:8080)
uv run --with nicegui python nicegui_app.py

# 5. Auto-TUI from CLI
uv run --with typer --with trogon python run_trogon.py tui

Safety Features

All interfaces enforce these guardrails:

Feature Description
Machine type blocking e2-micro, f1-micro, g1-small rejected with warning
Swap configuration 1-16GB configurable, default 4GB
DNF optimization Disables debug/source repos by default
Startup script Consistent configuration across all instances

Files Added

Core Implementation

  • cli/commands/provision/__init__.py - Module init
  • cli/commands/provision/command.py - Core logic and validation
  • cli/commands/provision/run_cli.py - CLI entry point
  • cli/commands/provision/provision - Bash wrapper

UI Implementations

  • cli/commands/provision/tui.py - Textual TUI
  • cli/commands/provision/run_tui.py - TUI entry point
  • cli/commands/provision/run_trogon.py - Trogon auto-TUI
  • cli/commands/provision/flet_app.py - Flet desktop app
  • cli/commands/provision/nicegui_app.py - NiceGUI web UI

Documentation

  • cli/commands/provision/README.md - Main usage guide
  • cli/commands/provision/README-typer.md - Typer coding quickstart
  • cli/commands/provision/README-textual.md - Textual coding quickstart
  • cli/commands/provision/README-trogon.md - Trogon coding quickstart
  • cli/commands/provision/README-flet.md - Flet coding quickstart
  • cli/commands/provision/README-nicegui.md - NiceGUI coding quickstart
  • docs/gcpprovisioningfirststeps.md - User guide for manual setup
  • docs/cli-provisioning-design.md - Technical design document
  • docs/architecture/decisions/0005-multi-interface-cli-for-gcp-provisioning.md - ADR

Framework Comparison

Feature CLI Textual TUI Trogon Flet Desktop NiceGUI Web
Works over SSH ✅ (port forward)
Mouse support
Shell completion
Scriptable
Dependencies typer textual typer+trogon flet nicegui

Related

  • Addresses developer friction identified in GCP provisioning workflows
  • See docs/gcpprovisioningfirststeps.md for the manual setup this tool automates
  • ADR-0005 documents the architectural decision

Note: This is a draft PR. Screenshots will be added from actual testing sessions.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added comprehensive GCP provisioning tool with multiple interfaces: Typer CLI, Textual TUI, Trogon auto-TUI, Flet desktop app, and NiceGUI web UI.
    • Instance creation with validation, machine-type safety checks, automatic swap/DNF configuration, and dry-run preview.
    • Real-time logs, startup script generation, and SSH guidance.
  • Documentation

    • Added quickstart guides and tutorials for all provisioning interfaces and architecture decisions.

✏️ Tip: You can customize this high-level summary in your review settings.

This guide helps developers avoid common pitfalls when provisioning
GCP instances for notebook development:

- Instance sizing: Why e2-micro fails (1GB RAM, 0.25 vCPU) and
  recommended alternatives (e2-medium minimum for dev work)
- Swap configuration: Step-by-step setup with persistence and
  vm.swappiness tuning to prevent OOM during DNF operations
- DNF optimization: Disabling debug/source repos (saves ~300MB
  metadata), increasing parallel downloads, enabling fastestmirror
- Quick setup script: All-in-one bash script for new instances
- Troubleshooting: Common issues and recovery steps

Context: DNF metadata refresh downloads 550MB+ from 17 repos,
causing even A100 GPU instances to wait 2-5 minutes. These
optimizations reduce that to under 30 seconds.
Technical design for enhancing notebooks-cli with GCP provisioning
capabilities, addressing developer pain points identified from
real-world usage.

Problem statement:
- SSH hangs on e2-micro (1GB RAM exhausted by DNF)
- No swap = OOM kills during package operations
- 550MB+ metadata downloads from 17+ RHEL repos
- Manual post-provision configuration is error-prone

Solution architecture:
- Layered UI approach: CLI → TUI → Desktop → Web
- Typer for CLI with shell autocompletion
- Textual for SSH-safe terminal UI
- Flet (Flutter/Skia) for native desktop app
- NiceGUI for browser-based interface

Key features:
- Machine type validation (blocks e2-micro, f1-micro, g1-small)
- Automatic swap configuration (1-16GB)
- DNF optimization (disable debug/source repos)
- Startup script generation for gcloud
Implements the core CLI for GCP instance provisioning with built-in
guardrails to prevent common developer mistakes.

Components:
- command.py: Typer CLI with create, bootstrap, check commands
- run_cli.py: Standalone runner with shell completion support
- provision: Bash wrapper for PATH integration

Commands:
- create: Provision new instance with sane defaults
- bootstrap: Configure existing instance with swap/DNF opts
- check: Verify instance configuration
- tui: Launch interactive terminal UI (requires textual)

Safety features:
- Blocks undersized machines: e2-micro, f1-micro, g1-small
- Validates machine type before provisioning
- Generates startup script with swap + DNF optimization

Shell completion:
  ./provision --install-completion zsh
  source ~/.zshrc
  ./provision create <TAB>  # Shows zones, machine types

Dependencies: typer, rich (via uv run --with typer)
Terminal-based UI using Textual framework for interactive GCP
instance provisioning. Works over SSH - no X11/GUI required.

Features:
- Two-panel layout: form (left) + log output (right)
- Mouse and keyboard navigation
- Dropdown selects for zone and machine type
- Visual warning when e2-micro selected (blocked)
- Swap size slider (1-16GB)
- DNF optimization toggle
- Real-time provisioning log with Rich formatting
- Async execution (UI stays responsive during gcloud calls)

UI Components:
- Header with app title and live clock
- Form inputs: instance name, zone, machine type, swap, dnf opt
- Log panel with scrollable output
- Footer with keyboard shortcuts

Usage:
  uv run --with textual python cli/commands/provision/run_tui.py

Why Textual:
- Works in any terminal (SSH, tmux, modern terminals)
- 60fps rendering with smooth animations
- CSS-like styling (.tcss files)
- Built by the Rich team, actively maintained

Dependencies: textual (via uv run --with textual)
Trogon automatically generates a TUI from Typer CLI commands with
zero additional code. Useful for discovering CLI options visually.

Features:
- Auto-introspects CLI commands and generates form UI
- Command tree navigation (arrow keys)
- Form fields for all command options
- Execute commands directly from TUI

Usage:
  uv run --with typer --with trogon python run_trogon.py tui

Keyboard shortcuts:
  ^t  Focus command tree
  ^r  Close & run command
  ^s  Search commands
  ^p  Command palette
  q   Quit

Included CLI commands (also usable without TUI):
  create       - Create instance with sane defaults
  bootstrap    - Configure existing instance
  check        - Verify instance configuration
  list-zones   - Show available GCP zones
  list-machines - Show recommended machine types

Known issue: ^o crashes if no command selected (Trogon bug)

Dependencies: typer, trogon (via uv run --with typer --with trogon)
Native desktop application using Flet framework, which wraps
Flutter and uses the Skia graphics engine for GPU-accelerated
60fps rendering.

Features:
- Native macOS/Linux window with system decorations
- Material Design 3 dark theme
- Two-card layout: form + log panel
- Dropdown selects with icons
- Swap size slider with visual track
- Toggle switch for DNF optimization
- Real-time log output during provisioning
- Async execution with loading states

UI Components:
- AppBar with title and theme toggle
- Instance name TextField with icon
- Zone and machine type Dropdowns
- Swap size Slider (1-16GB)
- DNF optimization Switch
- Create Instance ElevatedButton
- Log TextField (read-only, monospace)

Usage:
  uv run --with flet python cli/commands/provision/flet_app.py

Can also run as web app:
  flet run --web flet_app.py

Why Flet:
- Same codebase for desktop, web, and mobile
- Flutter's Skia renderer = smooth animations
- No Dart required, pure Python
- Modern Material Design out of the box

Dependencies: flet (via uv run --with flet)
Browser-based UI using NiceGUI framework for point-and-click
provisioning. Opens localhost:8080 with a modern web interface.

Features:
- Dark theme with Quasar/Vue.js components
- Two-column card layout: form + log
- Form validation with warning messages
- Real-time log streaming to browser
- Snackbar notifications for success/error
- Async execution (non-blocking UI)

UI Components:
- Header with app branding
- Instance name input
- Zone select dropdown
- Machine type select with warning for e2-micro
- Swap size slider (1-16GB)
- DNF optimization checkbox
- Create Instance button with loading state
- Log area with terminal styling

Usage:
  uv run --with nicegui python cli/commands/provision/nicegui_app.py
  # Opens http://localhost:8080 automatically

Why NiceGUI:
- More flexible than Streamlit for control panels
- Quasar components look professional
- Easy real-time updates via WebSocket
- Good for engineering tools with action buttons

Port forwarding note:
  ssh -L 8080:localhost:8080 user@remote-host
  # Then open http://localhost:8080 locally

Dependencies: nicegui (via uv run --with nicegui)
Detailed documentation covering all 5 UI interfaces:

1. CLI (run_cli.py)
   - Command reference: create, bootstrap, check
   - Shell completion installation
   - Wrapper script usage

2. Textual TUI (run_tui.py)
   - ASCII mockup of interface
   - Keyboard shortcuts table
   - SSH compatibility notes

3. Trogon Auto-TUI (run_trogon.py)
   - Step-by-step usage guide
   - Known issues (^o crash)

4. Flet Desktop (flet_app.py)
   - GUI mockup
   - Flutter runtime notes
   - Web mode option

5. NiceGUI Web (nicegui_app.py)
   - Browser interface mockup
   - SSH port forwarding guide

Also includes:
- Quick start commands
- Feature comparison table
- File structure overview
- Configuration defaults
- Troubleshooting section
- Contributing guidelines
Separate documentation for each UI framework with usage guides and
coding tutorials for developers who want to extend or customize.

Added files:
- README-typer.md: CLI framework with shell completion
- README-textual.md: TUI framework for terminal apps
- README-trogon.md: Auto-TUI generator from CLI
- README-flet.md: Flutter/Skia desktop framework
- README-nicegui.md: Web UI framework

Each README includes:
- Framework overview and features
- Quick start for the provisioning tool
- Hello World example
- Core concepts with code snippets
- Common patterns and components
- Full example showing key features
- Useful resources and links

Coding topics covered:
- Typer: Arguments, options, callbacks, Rich output
- Textual: Widgets, events, CSS styling, async workers
- Trogon: CLI introspection, form generation
- Flet: Controls, layouts, Material Design, dialogs
- NiceGUI: Components, Tailwind CSS, WebSocket updates
Documents the architectural decision to implement GCP instance
provisioning with five different user interfaces sharing core logic.

Key decisions documented:
- Why multiple interfaces (CLI, TUI, Auto-TUI, Desktop, Web)
- Framework choices (Typer, Textual, Trogon, Flet, NiceGUI)
- Why each framework was selected over alternatives
- Why UnoCSS was rejected (Tailwind already in NiceGUI)
- Implementation structure and file organization
- Dependency management strategy (uv run --with)
- Consequences (positive and negative) with mitigations

Related to: ADR-0003 (Prefer Python)

References:
- docs/gcpprovisioningfirststeps.md
- docs/cli-provisioning-design.md
- cli/commands/provision/
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 17, 2026

📝 Walkthrough

Walkthrough

This change adds a comprehensive GCP provisioning system with multiple user interfaces (Typer CLI, Textual TUI, Trogon auto-TUI, Flet desktop application, NiceGUI web UI), core provisioning logic with machine-type validation and startup script generation, shell wrappers, and extensive documentation covering usage guides, architecture decisions, and deployment first steps.

Changes

Cohort / File(s) Summary
Documentation
cli/commands/provision/README.md, cli/commands/provision/README-typer.md, cli/commands/provision/README-textual.md, cli/commands/provision/README-trogon.md, cli/commands/provision/README-flet.md, cli/commands/provision/README-nicegui.md, docs/architecture/decisions/0005-multi-interface-cli-for-gcp-provisioning.md, docs/cli-provisioning-design.md, docs/gcpprovisioningfirststeps.md
Nine comprehensive markdown documentation files covering quick starts, core concepts, usage examples, architecture decisions, and first-steps guidance for GCP provisioning across five UI interfaces. ~2,100 lines total.
Core Provision Package
cli/commands/provision/__init__.py, cli/commands/provision/command.py
Package initializer and main Typer CLI implementation. Introduces BLOCKED_MACHINE_TYPES validation, four commands (create, bootstrap, check, tui), startup script/bootstrap generation, and Rich-based user feedback. New public exports: app, BLOCKED_MACHINE_TYPES, RECOMMENDED_MACHINES.
Textual TUI Implementation
cli/commands/provision/tui.py, cli/commands/provision/run_tui.py
Full Textual-based terminal UI for instance provisioning. Two-panel layout with form and logging. Includes MACHINE_TYPES, ZONES, SWAP_SIZES constants; background provisioning with async gcloud execution; warning display for blocked machine types. New classes and entry point.
Desktop & Web UI Applications
cli/commands/provision/flet_app.py, cli/commands/provision/nicegui_app.py
Flet-based desktop GUI (~325 lines) and NiceGUI-based web UI (~237 lines) for provisioning. Both feature form controls, real-time logging, background gcloud execution, async handlers, and user feedback via snackbars/notifications. Exposes main(page: ft.Page) and main_page() entry points respectively.
CLI Runners & Wrappers
cli/commands/provision/run_cli.py, cli/commands/provision/run_tui.py, cli/commands/provision/run_trogon.py, cli/commands/provision/provision
Standalone entry points and wrappers. Simple runners for CLI and TUI; Trogon runner (~224 lines) adds Typer CLI with machine-type validation callback, conditional Trogon TUI integration, list-zones/list-machines commands, and dry-run support. Bash wrapper orchestrates uv-based Python execution.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is comprehensive and well-structured with problem statement, solution overview, usage examples, safety features, and file listing. However, it lacks required sections from the template: testing instructions and pre-merge checklist items are missing or incomplete. Add a 'How Has This Been Tested?' section with testing details, and complete the self-checklist items (make test confirmation and Konflux sync notes) and merge criteria checkboxes from the template.
Docstring Coverage ⚠️ Warning Docstring coverage is 78.79% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main change: adding multi-interface GCP provisioning tools with five different UIs (CLI, TUI, Desktop, Web).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jan 17, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@github-actions github-actions Bot added the review-requested GitHub Bot creates notification on #pr-review-ai-ide-team slack channel label Jan 17, 2026
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jan 17, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign jiridanek for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added size/xxl and removed size/xxl labels Jan 17, 2026
@jiridanek
Copy link
Copy Markdown
Member Author

Screenshot 2026-01-17 at 4 13 25 PM Screenshot 2026-01-17 at 4 18 23 PM Screenshot 2026-01-17 at 4 18 53 PM Screenshot 2026-01-17 at 4 23 20 PM

@jiridanek
Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 17, 2026

✅ Actions performed

Full review triggered.

@openshift-ci openshift-ci Bot added size/xxl and removed size/xxl labels Jan 17, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 13

🤖 Fix all issues with AI agents
In `@cli/commands/provision/command.py`:
- Around line 234-239: The bootstrap command currently wraps
generate_startup_script() in sudo bash -c '...', which breaks because the
generated script itself contains single quotes and a << 'EOF' heredoc; change
generate_bootstrap_commands to invoke sudo bash -c using a heredoc with a unique
delimiter (e.g., sudo bash -c <<'BOOTSTRAP_EOF' ... BOOTSTRAP_EOF) so the inner
single quotes are preserved, ensure the delimiter string does not appear in
generate_startup_script output, and keep the function signature and return type
the same while returning the full heredoc-wrapped script string from
generate_bootstrap_commands.
- Around line 31-45: The swap_gb CLI Option currently accepts any integer which
can lead to invalid or unsafe swap sizes; update the swap_gb Typer Option in
both create_instance and bootstrap_existing to enforce bounds by adding min=1
and max=16 to the Option(...) declaration so the CLI prevents values outside
1–16GB at parse time (adjust the swap_gb parameter signature in create_instance
and the swap_gb parameter in bootstrap_existing accordingly).

In `@cli/commands/provision/flet_app.py`:
- Around line 83-90: The swap slider component swap_slider (ft.Slider) currently
sets max=8 and divisions=7 which limits selection to 1–8GB; update it to match
the CLI/design by setting max=16 and divisions=15 and ensure the label remains
"{value}GB" so the control supports the full 1–16GB range and displays values
correctly.
- Around line 118-155: The local generate_startup_script implementation in
flet_app.py duplicates the shared function in command.py; delete the local
function definition in flet_app.py (and the duplicate in nicegui_app.py), add an
import for the shared generate_startup_script from the module that currently
defines it (command.py), and ensure any callers in flet_app.py and
nicegui_app.py reference the imported generate_startup_script symbol so behavior
is unchanged and maintenance is centralized.

In `@cli/commands/provision/nicegui_app.py`:
- Around line 125-127: The NiceGUI swap slider currently uses swap_slider with
max=8 which contradicts the 1–16GB requirement; update the slider creation in
nicegui_app.py to use max=16 (and ensure the initial value and step remain
appropriate), and make the identical change in flet_app.py where swap_slider is
defined so both UIs allow 1–16GB swap configuration and the swap_label text
remains driven by int(swap_slider.value).

In `@cli/commands/provision/README-nicegui.md`:
- Around line 27-49: Update the fenced ASCII-art code block so it includes a
language specifier (e.g., change the opening triple backticks to ```text or
```plaintext) to satisfy markdown linting and improve screen-reader
accessibility; locate the ASCII-art fenced block in README-nicegui.md and
replace the opening fence with one that includes the specifier while leaving the
block contents unchanged.

In `@cli/commands/provision/README-trogon.md`:
- Around line 68-81: The Markdown table under the "Keyboard Shortcuts" heading
lacks surrounding blank lines causing markdownlint MD058 failures; edit the
README-trogon.md "Keyboard Shortcuts" section to insert a blank line before the
table and a blank line after the table (i.e., ensure there is an empty line
between the heading "## Keyboard Shortcuts" and the table start, and another
empty line after the table end) so the shortcuts table is properly separated and
passes linting.

In `@cli/commands/provision/run_trogon.py`:
- Line 113: The bare import "from command import generate_startup_script" in
run_trogon.py will fail when executed from the repository root; update the top
of run_trogon.py to use a package-relative import (e.g., adjust to the correct
relative import that resolves the sibling module containing
generate_startup_script) or, if package layout prevents relative imports, add
the small sys.path insertion used in run_tui.py (insert the script's parent
directory into sys.path before importing) so generate_startup_script can be
imported reliably.

In `@cli/commands/provision/run_tui.py`:
- Line 7: The import in run_tui.py (from tui import ProvisionApp) fails when run
from the repository root; update the import to a proper package import (e.g.,
import ProvisionApp from cli.commands.provision.tui) or switch to a relative
import if you will run the script as a module (e.g., use a relative import
inside run_tui.py and run with python -m cli.commands.provision.run_tui); ensure
the symbol ProvisionApp is referenced from the corrected import so the module
resolves regardless of current working directory.

In `@cli/commands/provision/tui.py`:
- Around line 301-302: Remove the unnecessary f-string prefix from the log_write
call that has no interpolation: change the call that passes "[bold]SSH
command:[/bold]" (the log_write(f"[bold]SSH command:[/bold]") invocation) to a
plain string literal without the leading f, leaving the subsequent log_write
that interpolates {name} and {zone} unchanged.
- Line 237: Remove the unused local variable by deleting the assignment "worker
= get_current_worker()" in the function where it appears (reference the
get_current_worker call and the local variable name worker); if cancellation
checks were intended instead, replace the unused assignment with proper
cancellation logic using worker (e.g., check worker.is_cancelled() or similar) —
otherwise simply remove the line to eliminate the unused variable.
- Around line 287-291: Validate the user-provided instance name (the variable
used to build cmd before subprocess.run) against GCP/RFC-1035 using the regex
^[a-z]([a-z0-9-]*[a-z0-9])?$ and reject/show a clear error to the user if it
doesn't match; perform this check immediately before calling subprocess.run(cmd,
...) (i.e., where cmd is built/used) and do not invoke subprocess.run when
validation fails. Use the variable names from the diff (name and cmd) and ensure
the error path returns or raises a user-facing message rather than proceeding to
subprocess.run.

In `@docs/cli-provisioning-design.md`:
- Around line 213-229: The documented CLI directory tree is outdated; update the
snippet in cli-provisioning-design.md to match the actual implementation by
replacing the listed filenames under cli/commands/provision (e.g.,
provision/command.py, provision/tui.py, provision/webui.py,
provision/desktop.py) with the real filenames used in the codebase—open the
file, locate the current tree block and edit the entries so each referenced
symbol (module names and their purpose like command.py, tui.py, webui.py,
desktop.py) reflects the actual filenames and any renamed modules in the
repository.
🧹 Nitpick comments (7)
docs/gcpprovisioningfirststeps.md (1)

14-21: Consider adding blank lines around the table.

The table would render more reliably across Markdown parsers with blank lines before and after.

📝 Proposed formatting fix
 **Why it fails:**
+
 | Resource | e2-micro | Minimum Recommended |
 |----------|----------|---------------------|
 | vCPU | 0.25 (shared) | 0.5+ |
 | RAM | 1 GB | 2 GB+ |
 | Burst | Limited credits | Sustained |
+
 DNF metadata operations alone can consume 500MB+ RAM, leaving the system unresponsive.
cli/commands/provision/README.md (2)

137-145: Consider adding blank lines around the keyboard shortcuts table.

The table would render more reliably with blank lines before and after.

📝 Proposed formatting fix
 **Keyboard Shortcuts:**
+
 | Key | Action |
 |-----|--------|
 | `Tab` / `Shift+Tab` | Navigate between fields |
 | `Enter` | Activate button / Select dropdown item |
 | `↑` / `↓` | Navigate dropdown options |
 | `Esc` | Close dropdown / Quit |
 | `^p` | Command palette |
+
 **Requirements:**

169-179: Consider adding blank lines around the keyboard shortcuts table.

Consistent with the TUI section, this table would benefit from blank lines for better rendering.

📝 Proposed formatting fix
 **Keyboard Shortcuts:**
+
 | Key | Action |
 |-----|--------|
 | `^t` | Focus command tree (left panel) |
 | `↑` / `↓` | Navigate commands |
 | `→` | Expand command to see options |
 | `Enter` | Select command (shows form) |
 | `^r` | Close TUI & run the command |
 | `^s` | Search commands |
 | `q` | Quit |
+
 **How to use:**
cli/commands/provision/README-textual.md (1)

8-14: Consider adding blank lines around the features list.

While this is formatted as a bullet list, adding blank lines would improve readability.

📝 Proposed formatting fix
 Textual is a Python framework for building rich terminal applications. Built by the creators of [Rich](https://rich.readthedocs.io/), it provides:
+
 - **60fps rendering** with smooth animations
 - **CSS-like styling** for layout and theming
 - **Mouse support** in modern terminals
 - **Async-first** architecture for responsive UIs
 - **Works over SSH** - no X11/GUI required
+
 ## Quick Start
cli/commands/provision/README-flet.md (1)

50-57: Consider adding blank lines around the features list.

Consistent with other READMEs, blank lines would improve readability.

📝 Proposed formatting fix
 ## Features
+
 - Native window with system decorations
 - GPU-accelerated rendering
 - Smooth animations (60fps)
 - Dark/light theme support
 - Same code works as web app
+
 ## Notes
cli/commands/provision/tui.py (1)

130-134: Consider annotating mutable class attribute with ClassVar.

BINDINGS is a mutable class attribute that should be annotated with typing.ClassVar to indicate it's not an instance attribute.

+from typing import ClassVar
+from textual.binding import Binding
+
 class ProvisionApp(App):
     """A Textual app for GCP instance provisioning."""
     
     # ... CSS ...
     
-    BINDINGS = [
+    BINDINGS: ClassVar[list[Binding]] = [
         Binding("q", "quit", "Quit"),
         Binding("ctrl+c", "quit", "Quit"),
         Binding("enter", "provision", "Provision", show=False),
     ]
docs/cli-provisioning-design.md (1)

117-152: Update phase descriptions to reflect current implementation status.

Phases 3 (Web UI) and 4 (Desktop App) are marked as "Future" but have been implemented in this PR. Consider updating the documentation to reflect the current state.

Comment on lines +31 to +45
def create_instance(
name: str = typer.Argument(..., help="Instance name"),
zone: str = typer.Option("us-central1-a", "--zone", "-z", help="GCP zone"),
machine_type: str = typer.Option(
"e2-small",
"--machine-type", "-m",
help="Machine type (e2-micro blocked by default)",
),
swap_gb: int = typer.Option(4, "--swap", "-s", help="Swap file size in GB"),
optimize_dnf: bool = typer.Option(True, "--optimize-dnf/--no-optimize-dnf", help="Apply DNF optimizations"),
image_family: str = typer.Option("rhel-9", "--image-family", help="Image family"),
image_project: str = typer.Option("rhel-cloud", "--image-project", help="Image project"),
allow_micro: bool = typer.Option(False, "--allow-micro", help="Override e2-micro block (not recommended)"),
dry_run: bool = typer.Option(False, "--dry-run", "-n", help="Show what would be done"),
):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "command.py" | grep -E "provision|cli" | head -20

Repository: opendatahub-io/notebooks

Length of output: 103


🏁 Script executed:

fd -e py "command" --path "*provision*" 2>/dev/null | head -20

Repository: opendatahub-io/notebooks

Length of output: 50


🏁 Script executed:

git ls-files | grep -E "cli.*provision.*command"

Repository: opendatahub-io/notebooks

Length of output: 101


🏁 Script executed:

cat -n cli/commands/provision/command.py | head -150

Repository: opendatahub-io/notebooks

Length of output: 6991


🏁 Script executed:

cat -n cli/commands/provision/command.py | sed -n '150,300p'

Repository: opendatahub-io/notebooks

Length of output: 3727


🏁 Script executed:

find . -type f \( -name "README*" -o -name "*.md" -o -name "CHANGELOG*" -o -name "*.txt" \) | xargs grep -l "swap\|1.*16" 2>/dev/null | head -10

Repository: opendatahub-io/notebooks

Length of output: 485


🏁 Script executed:

rg -i "1.*16.*gb|swap.*range|provision.*objective" --type md --type txt 2>/dev/null | head -20

Repository: opendatahub-io/notebooks

Length of output: 414


🏁 Script executed:

cat cli/commands/provision/README.md | head -100

Repository: opendatahub-io/notebooks

Length of output: 2765


🏁 Script executed:

cat docs/architecture/decisions/0005-multi-interface-cli-for-gcp-provisioning.md | grep -A 10 -B 10 "1-16GB\|swap"

Repository: opendatahub-io/notebooks

Length of output: 1889


🌐 Web query:

Typer min max int option parameter validation

💡 Result:

Use Typer's built-in min/max on Argument/Option (preferred with typing.Annotated). Examples:

  • Annotated (Python 3.9+)
    from typing import Annotated
    import typer

    app = typer.Typer()

    @app.command()
    def main(
    count: Annotated[int, typer.Option(min=1, max=10)] = 1,
    ):
    print(count)

  • Non-Annotated shortcut
    import typer

    @app.command()
    def main(
    count: int = typer.Option(1, min=1, max=10),
    ):
    print(count)

  • Clamp instead of error (use clamp=True)
    rank: Annotated[int, typer.Option(max=5, clamp=True)] = 0

  • For custom checks use a callback:
    def cb(value: int):
    if value % 2 != 0:
    raise typer.BadParameter("must be even")
    return value

    option: Annotated[int, typer.Option(callback=cb)] = 0

These features (min, max, clamp, callbacks, Annotated usage) are documented in the Typer docs. [1][2]

Sources:
[1] Typer — Number (min/max, clamp).
[2] Typer — Option/Argument callbacks and validation.


Enforce 1–16GB swap bounds in CLI options.

The PR objectives specify 1–16GB swap configuration, but the current options accept any integer (including 0, negative values, or extremely large values), which will cause fallocate to fail or exhaust disk space. Typer supports min/max parameters on Option() to enforce these bounds automatically.

Add min=1, max=16 to both swap_gb parameters:

🛠️ Proposed fix
-    swap_gb: int = typer.Option(4, "--swap", "-s", help="Swap file size in GB"),
+    swap_gb: int = typer.Option(4, "--swap", "-s", min=1, max=16, help="Swap file size in GB (1–16)"),

This applies at line 39 (create_instance) and line 124 (bootstrap_existing).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def create_instance(
name: str = typer.Argument(..., help="Instance name"),
zone: str = typer.Option("us-central1-a", "--zone", "-z", help="GCP zone"),
machine_type: str = typer.Option(
"e2-small",
"--machine-type", "-m",
help="Machine type (e2-micro blocked by default)",
),
swap_gb: int = typer.Option(4, "--swap", "-s", help="Swap file size in GB"),
optimize_dnf: bool = typer.Option(True, "--optimize-dnf/--no-optimize-dnf", help="Apply DNF optimizations"),
image_family: str = typer.Option("rhel-9", "--image-family", help="Image family"),
image_project: str = typer.Option("rhel-cloud", "--image-project", help="Image project"),
allow_micro: bool = typer.Option(False, "--allow-micro", help="Override e2-micro block (not recommended)"),
dry_run: bool = typer.Option(False, "--dry-run", "-n", help="Show what would be done"),
):
def create_instance(
name: str = typer.Argument(..., help="Instance name"),
zone: str = typer.Option("us-central1-a", "--zone", "-z", help="GCP zone"),
machine_type: str = typer.Option(
"e2-small",
"--machine-type", "-m",
help="Machine type (e2-micro blocked by default)",
),
swap_gb: int = typer.Option(4, "--swap", "-s", min=1, max=16, help="Swap file size in GB (1–16)"),
optimize_dnf: bool = typer.Option(True, "--optimize-dnf/--no-optimize-dnf", help="Apply DNF optimizations"),
image_family: str = typer.Option("rhel-9", "--image-family", help="Image family"),
image_project: str = typer.Option("rhel-cloud", "--image-project", help="Image project"),
allow_micro: bool = typer.Option(False, "--allow-micro", help="Override e2-micro block (not recommended)"),
dry_run: bool = typer.Option(False, "--dry-run", "-n", help="Show what would be done"),
):
🤖 Prompt for AI Agents
In `@cli/commands/provision/command.py` around lines 31 - 45, The swap_gb CLI
Option currently accepts any integer which can lead to invalid or unsafe swap
sizes; update the swap_gb Typer Option in both create_instance and
bootstrap_existing to enforce bounds by adding min=1 and max=16 to the
Option(...) declaration so the CLI prevents values outside 1–16GB at parse time
(adjust the swap_gb parameter signature in create_instance and the swap_gb
parameter in bootstrap_existing accordingly).

Comment on lines +234 to +239
def generate_bootstrap_commands(swap_gb: int = 4, optimize_dnf: bool = True) -> str:
"""Generate bootstrap commands for existing instances."""
return f"""
sudo bash -c '
{generate_startup_script(swap_gb, optimize_dnf)}
'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Bootstrap command string is broken by single quotes.

generate_startup_script() contains single quotes ('/swapfile...' and << 'EOF'). Embedding that in sudo bash -c '...' breaks the command. Use a heredoc with a unique delimiter instead.

🛠️ Proposed fix
 def generate_bootstrap_commands(swap_gb: int = 4, optimize_dnf: bool = True) -> str:
     """Generate bootstrap commands for existing instances."""
-    return f"""
-sudo bash -c '
-{generate_startup_script(swap_gb, optimize_dnf)}
-'
-""".strip()
+    script = generate_startup_script(swap_gb, optimize_dnf)
+    return f"""
+sudo bash << 'BOOTSTRAP_EOF'
+{script}
+BOOTSTRAP_EOF
+""".strip()
🤖 Prompt for AI Agents
In `@cli/commands/provision/command.py` around lines 234 - 239, The bootstrap
command currently wraps generate_startup_script() in sudo bash -c '...', which
breaks because the generated script itself contains single quotes and a << 'EOF'
heredoc; change generate_bootstrap_commands to invoke sudo bash -c using a
heredoc with a unique delimiter (e.g., sudo bash -c <<'BOOTSTRAP_EOF' ...
BOOTSTRAP_EOF) so the inner single quotes are preserved, ensure the delimiter
string does not appear in generate_startup_script output, and keep the function
signature and return type the same while returning the full heredoc-wrapped
script string from generate_bootstrap_commands.

Comment on lines +83 to +90
swap_slider = ft.Slider(
min=1,
max=8,
divisions=7,
value=4,
label="{value}GB",
width=300,
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Swap slider range inconsistent with other interfaces.

The swap slider has max=8 GB, but the design document and CLI specify a 1-16GB range. The TUI uses predefined options up to 8GB but the CLI allows 16GB. Consider aligning for consistency.

     swap_slider = ft.Slider(
         min=1,
-        max=8,
-        divisions=7,
+        max=16,
+        divisions=15,
         value=4,
         label="{value}GB",
         width=300,
     )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
swap_slider = ft.Slider(
min=1,
max=8,
divisions=7,
value=4,
label="{value}GB",
width=300,
)
swap_slider = ft.Slider(
min=1,
max=16,
divisions=15,
value=4,
label="{value}GB",
width=300,
)
🤖 Prompt for AI Agents
In `@cli/commands/provision/flet_app.py` around lines 83 - 90, The swap slider
component swap_slider (ft.Slider) currently sets max=8 and divisions=7 which
limits selection to 1–8GB; update it to match the CLI/design by setting max=16
and divisions=15 and ensure the label remains "{value}GB" so the control
supports the full 1–16GB range and displays values correctly.

Comment on lines +118 to +155
def generate_startup_script(swap_gb: int, optimize_dnf: bool) -> str:
"""Generate startup script for the instance."""
script_parts = ["#!/bin/bash", "set -euo pipefail", ""]

script_parts.extend([
"# Setup swap",
"if [[ ! -f /swapfile ]]; then",
f" fallocate -l {swap_gb}G /swapfile",
" chmod 600 /swapfile",
" mkswap /swapfile",
" swapon /swapfile",
" echo '/swapfile none swap sw 0 0' >> /etc/fstab",
" echo 'vm.swappiness=10' > /etc/sysctl.d/99-swappiness.conf",
" sysctl -p /etc/sysctl.d/99-swappiness.conf",
"fi",
"",
])

if optimize_dnf:
script_parts.extend([
"# Optimize DNF",
"dnf config-manager --disable '*-debug-rpms' '*-source-rpms' 2>/dev/null || true",
"",
"if ! grep -q 'max_parallel_downloads' /etc/dnf/dnf.conf; then",
" cat >> /etc/dnf/dnf.conf << 'EOF'",
"",
"# GCP optimizations",
"max_parallel_downloads=10",
"fastestmirror=True",
"metadata_expire=7d",
"EOF",
"fi",
"",
"dnf clean all",
"dnf makecache",
])

return "\n".join(script_parts)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all definitions of generate_startup_script across the codebase
echo "=== Searching for generate_startup_script definitions ==="
rg "def generate_startup_script" --type py -n

# Check the specific files mentioned
echo -e "\n=== Checking command.py ==="
if [ -f "cli/commands/provision/command.py" ]; then
  rg "generate_startup_script" cli/commands/provision/command.py -n -A 2 -B 2
else
  echo "command.py not found at expected path"
fi

echo -e "\n=== Checking tui.py imports ==="
if [ -f "cli/commands/provision/tui.py" ]; then
  rg "generate_startup_script\|from.*command" cli/commands/provision/tui.py -n -B 2 -A 2
else
  echo "tui.py not found at expected path"
fi

Repository: opendatahub-io/notebooks

Length of output: 1035


🏁 Script executed:

#!/bin/bash
# Let me check tui.py more thoroughly for imports and generate_startup_script usage
echo "=== Full tui.py imports section ==="
head -50 cli/commands/provision/tui.py

echo -e "\n=== All occurrences of generate_startup_script in tui.py ==="
rg "generate_startup_script" cli/commands/provision/tui.py -n -B 3 -A 3 --context 5

echo -e "\n=== Compare the three function implementations ==="
echo "--- command.py (lines 192-235) ---"
sed -n '192,235p' cli/commands/provision/command.py

echo -e "\n--- flet_app.py (lines 118-155) ---"
sed -n '118,155p' cli/commands/provision/flet_app.py

echo -e "\n--- nicegui_app.py (lines 37-74) ---"
sed -n '37,74p' cli/commands/provision/nicegui_app.py

Repository: opendatahub-io/notebooks

Length of output: 6440


Remove duplicate generate_startup_script function and import from shared module.

This function is duplicated in flet_app.py (lines 118–155), but an identical version exists in command.py (line 192), which is correctly imported by tui.py. The local copy will cause maintenance issues when startup script logic needs updates.

♻️ Proposed fix
+    from cli.commands.provision.command import generate_startup_script
+
     async def provision_click(e):
         """Handle provision button click."""
         # ... existing validation code ...
         
         # Generate startup script
-        startup_script = generate_startup_script(swap_gb, optimize_dnf)
+        startup_script = generate_startup_script(swap_gb=swap_gb, optimize_dnf=optimize_dnf)

Then remove the local generate_startup_script function definition (lines 118–155).

Note: The same duplication exists in nicegui_app.py (line 37) and should also be resolved.

🤖 Prompt for AI Agents
In `@cli/commands/provision/flet_app.py` around lines 118 - 155, The local
generate_startup_script implementation in flet_app.py duplicates the shared
function in command.py; delete the local function definition in flet_app.py (and
the duplicate in nicegui_app.py), add an import for the shared
generate_startup_script from the module that currently defines it (command.py),
and ensure any callers in flet_app.py and nicegui_app.py reference the imported
generate_startup_script symbol so behavior is unchanged and maintenance is
centralized.

Comment on lines +125 to +127
swap_label = ui.label('Swap Size: 4GB')
swap_slider = ui.slider(min=1, max=8, value=4, step=1).classes('w-full')
swap_slider.on_value_change(lambda: swap_label.set_text(f'Swap Size: {int(swap_slider.value)}GB'))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Read the file to verify the current implementation
cat -n cli/commands/provision/nicegui_app.py | sed -n '120,130p'

Repository: opendatahub-io/notebooks

Length of output: 642


🏁 Script executed:

# Search for any documentation or requirements mentioning swap size limits
rg "swap|1-16|1..16" -i --type md --type txt --type py -A 2 -B 2

Repository: opendatahub-io/notebooks

Length of output: 40752


🏁 Script executed:

# Search for swap_slider in the codebase to see if there are other references
rg "swap.*slider|max.*8|max.*16" -i --type py -A 1 -B 1

Repository: opendatahub-io/notebooks

Length of output: 1959


🏁 Script executed:

# Look for README, requirements, or documentation files
fd -e md -e txt -e rst | head -20

Repository: opendatahub-io/notebooks

Length of output: 905


Align swap slider range with 1–16GB requirement across UI implementations.

The NiceGUI slider incorrectly caps swap at 8GB while the documented requirement is 1–16GB, as specified in the architecture decision, trogon CLI (max=16), and README documentation. This creates inconsistency where users cannot configure swap beyond 8GB through the NiceGUI interface.

Note: The same issue exists in cli/commands/provision/flet_app.py (lines 112–115), which also uses max=8 and should be updated to max=16.

🛠️ Proposed fixes

nicegui_app.py (lines 125-127):

-            swap_slider = ui.slider(min=1, max=8, value=4, step=1).classes('w-full')
+            swap_slider = ui.slider(min=1, max=16, value=4, step=1).classes('w-full')

flet_app.py (lines 112-115):

     swap_slider = ft.Slider(
         min=1,
-        max=8,
+        max=16,
         divisions=7,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
swap_label = ui.label('Swap Size: 4GB')
swap_slider = ui.slider(min=1, max=8, value=4, step=1).classes('w-full')
swap_slider.on_value_change(lambda: swap_label.set_text(f'Swap Size: {int(swap_slider.value)}GB'))
swap_label = ui.label('Swap Size: 4GB')
swap_slider = ui.slider(min=1, max=16, value=4, step=1).classes('w-full')
swap_slider.on_value_change(lambda: swap_label.set_text(f'Swap Size: {int(swap_slider.value)}GB'))
🤖 Prompt for AI Agents
In `@cli/commands/provision/nicegui_app.py` around lines 125 - 127, The NiceGUI
swap slider currently uses swap_slider with max=8 which contradicts the 1–16GB
requirement; update the slider creation in nicegui_app.py to use max=16 (and
ensure the initial value and step remain appropriate), and make the identical
change in flet_app.py where swap_slider is defined so both UIs allow 1–16GB swap
configuration and the swap_label text remains driven by int(swap_slider.value).


# Run with: uv run --with textual python cli/commands/provision/run_tui.py

from tui import ProvisionApp
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Import will fail when run from repository root.

The import from tui import ProvisionApp assumes the script is run from within the cli/commands/provision/ directory. When executed as documented (uv run --with textual python cli/commands/provision/run_tui.py), Python won't find the tui module.

🐛 Proposed fix

Option 1: Use relative import (requires running as module)

-from tui import ProvisionApp
+from .tui import ProvisionApp

Then run with: uv run --with textual python -m cli.commands.provision.run_tui

Option 2: Add path manipulation for script execution

+import sys
+from pathlib import Path
+sys.path.insert(0, str(Path(__file__).parent))
+
 from tui import ProvisionApp

Option 3: Use absolute import

-from tui import ProvisionApp
+from cli.commands.provision.tui import ProvisionApp
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
from tui import ProvisionApp
import sys
from pathlib import Path
sys.path.insert(0, str(Path(__file__).parent))
from tui import ProvisionApp
Suggested change
from tui import ProvisionApp
from cli.commands.provision.tui import ProvisionApp
🤖 Prompt for AI Agents
In `@cli/commands/provision/run_tui.py` at line 7, The import in run_tui.py (from
tui import ProvisionApp) fails when run from the repository root; update the
import to a proper package import (e.g., import ProvisionApp from
cli.commands.provision.tui) or switch to a relative import if you will run the
script as a module (e.g., use a relative import inside run_tui.py and run with
python -m cli.commands.provision.run_tui); ensure the symbol ProvisionApp is
referenced from the corrected import so the module resolves regardless of
current working directory.

optimize_dnf: bool,
) -> None:
"""Run the provisioning in a background thread."""
worker = get_current_worker()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Remove unused variable worker.

The worker variable is assigned but never used. This was likely intended for cancellation checks but isn't utilized.

     def provision_instance(
         self,
         name: str,
         zone: str,
         machine_type: str,
         swap_gb: int,
         optimize_dnf: bool,
     ) -> None:
         """Run the provisioning in a background thread."""
-        worker = get_current_worker()
         log = self.query_one("#log", Log)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
worker = get_current_worker()
def provision_instance(
self,
name: str,
zone: str,
machine_type: str,
swap_gb: int,
optimize_dnf: bool,
) -> None:
"""Run the provisioning in a background thread."""
log = self.query_one("#log", Log)
🧰 Tools
🪛 Ruff (0.14.13)

237-237: Local variable worker is assigned to but never used

Remove assignment to unused variable worker

(F841)

🤖 Prompt for AI Agents
In `@cli/commands/provision/tui.py` at line 237, Remove the unused local variable
by deleting the assignment "worker = get_current_worker()" in the function where
it appears (reference the get_current_worker call and the local variable name
worker); if cancellation checks were intended instead, replace the unused
assignment with proper cancellation logic using worker (e.g., check
worker.is_cancelled() or similar) — otherwise simply remove the line to
eliminate the unused variable.

Comment on lines +287 to +291
result = subprocess.run(
cmd,
capture_output=True,
text=True,
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's read the file around the specified lines to understand the context
cat -n cli/commands/provision/tui.py | sed -n '270,305p'

Repository: opendatahub-io/notebooks

Length of output: 1635


🏁 Script executed:

#!/bin/bash
# Search for where 'name' parameter comes from - look for function definition and parameters
rg -n 'def.*name|name.*=' cli/commands/provision/tui.py | head -20

Repository: opendatahub-io/notebooks

Length of output: 251


🏁 Script executed:

#!/bin/bash
# Check for any validation patterns of instance names in the provision module
rg -n 'validate|strip|pattern|check.*name|name.*valid' cli/commands/provision/ --type=py

Repository: opendatahub-io/notebooks

Length of output: 372


🏁 Script executed:

#!/bin/bash
# Look for where subprocess.run is called and trace the 'name' variable
ast-grep --pattern 'subprocess.run(
  $_,
  $$$
)'

Repository: opendatahub-io/notebooks

Length of output: 2889


🏁 Script executed:

#!/bin/bash
# Look for the Input widget definition with id "#name" to see if validators are configured
rg -B5 -A5 '#name' cli/commands/provision/tui.py | head -40

Repository: opendatahub-io/notebooks

Length of output: 927


🏁 Script executed:

#!/bin/bash
# Search for the Input widget class definition to see if there are any validators
rg -B10 -A10 'class.*Input|Input\(' cli/commands/provision/tui.py | head -60

Repository: opendatahub-io/notebooks

Length of output: 979


🏁 Script executed:

#!/bin/bash
# Check the full function/method containing line 208 to see complete validation context
sed -n '180,230p' cli/commands/provision/tui.py

Repository: opendatahub-io/notebooks

Length of output: 2096


🏁 Script executed:

#!/bin/bash
# Search for gcloud instance naming constraints in documentation or comments
rg -i 'instance.*name.*valid|constraint|requirement|pattern|alphanumeric|regex' cli/commands/provision/ --type=py

Repository: opendatahub-io/notebooks

Length of output: 50


🏁 Script executed:

#!/bin/bash
# Let's examine if there are any other validation patterns in the entire provision module
rg -n 'pattern|regex|match|^[a-z]|validate|constraint' cli/commands/provision/ --type=py

Repository: opendatahub-io/notebooks

Length of output: 4122


🏁 Script executed:

#!/bin/bash
# Check if there's any documentation in README or comments about instance naming
rg -i 'instance.*name|gcloud.*instance' cli/ --type=py | grep -i 'must\|should\|valid\|constraint\|match\|pattern\|alphanumeric' | head -20

Repository: opendatahub-io/notebooks

Length of output: 50


🌐 Web query:

gcloud compute instances create naming requirements constraints

💡 Result:

  • Instance name rules: must be 1–63 characters, comply with RFC‑1035; match regex a-z? — i.e. start with a lowercase letter, may contain lowercase letters, digits and hyphens, and cannot end with a hyphen. [1][2]

  • Hostname (--hostname) must also be RFC‑1035 compliant. [3]

  • To enforce naming constraints organization‑wide, create an Org Policy custom constraint that targets compute.googleapis.com/Instance and use a CEL condition (e.g. check resource.name with regex or string functions) and then set a policy (ALLOW/DENY) — you can create the constraint and apply it with gcloud org-policies set-custom-constraint / set-policy. [2]

Sources:
[1] Compute Engine API / Instances (name requirements).
[2] Create/manage custom constraints (Org Policy) for Compute resources.
[3] gcloud compute instances create reference (hostname RFC1035).


Add instance name validation matching GCP's RFC-1035 requirements.

The name parameter from user input is passed to subprocess.run() without validation. While the list-based command call is safe from injection, GCP instance names must follow RFC-1035: start with a lowercase letter, contain only lowercase letters/digits/hyphens, and cannot end with a hyphen. Validate with pattern ^[a-z]([a-z0-9-]*[a-z0-9])?$ before passing to gcloud to provide clear feedback for invalid names.

🧰 Tools
🪛 Ruff (0.14.13)

287-287: subprocess call: check for execution of untrusted input

(S603)

🤖 Prompt for AI Agents
In `@cli/commands/provision/tui.py` around lines 287 - 291, Validate the
user-provided instance name (the variable used to build cmd before
subprocess.run) against GCP/RFC-1035 using the regex
^[a-z]([a-z0-9-]*[a-z0-9])?$ and reject/show a clear error to the user if it
doesn't match; perform this check immediately before calling subprocess.run(cmd,
...) (i.e., where cmd is built/used) and do not invoke subprocess.run when
validation fails. Use the variable names from the diff (name and cmd) and ensure
the error path returns or raises a user-facing message rather than proceeding to
subprocess.run.

Comment on lines +301 to +302
log_write(f"[bold]SSH command:[/bold]")
log_write(f" [cyan]gcloud compute ssh {name} --zone={zone}[/cyan]")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Remove extraneous f prefix from string without placeholders.

Line 301 uses an f-string but contains no interpolation placeholders.

-                log_write(f"[bold]SSH command:[/bold]")
+                log_write("[bold]SSH command:[/bold]")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
log_write(f"[bold]SSH command:[/bold]")
log_write(f" [cyan]gcloud compute ssh {name} --zone={zone}[/cyan]")
log_write("[bold]SSH command:[/bold]")
log_write(f" [cyan]gcloud compute ssh {name} --zone={zone}[/cyan]")
🧰 Tools
🪛 Ruff (0.14.13)

301-301: f-string without any placeholders

Remove extraneous f prefix

(F541)

🤖 Prompt for AI Agents
In `@cli/commands/provision/tui.py` around lines 301 - 302, Remove the unnecessary
f-string prefix from the log_write call that has no interpolation: change the
call that passes "[bold]SSH command:[/bold]" (the log_write(f"[bold]SSH
command:[/bold]") invocation) to a plain string literal without the leading f,
leaving the subsequent log_write that interpolates {name} and {zone} unchanged.

Comment on lines +213 to +229
```
cli/
├── commands/
│ ├── aipcc.py
│ ├── konflux.py
│ ├── manifest.py
│ ├── provision/ # NEW
│ │ ├── __init__.py
│ │ ├── command.py # CLI commands
│ │ ├── tui.py # Textual TUI
│ │ ├── webui.py # NiceGUI (future)
│ │ └── desktop.py # Flet (future)
│ ├── quay.py
│ └── security.py
├── main.py
└── ...
```
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Update file structure to reflect actual implementation.

The directory structure is outdated. The actual implementation uses different filenames:

 cli/
 ├── commands/
 │   ├── provision/          # NEW
 │   │   ├── __init__.py
 │   │   ├── command.py      # CLI commands
 │   │   ├── tui.py          # Textual TUI
-│   │   ├── webui.py        # NiceGUI (future)
-│   │   └── desktop.py      # Flet (future)
+│   │   ├── nicegui_app.py  # NiceGUI web UI
+│   │   ├── flet_app.py     # Flet desktop app
+│   │   ├── run_tui.py      # TUI runner
+│   │   ├── run_trogon.py   # Trogon CLI/TUI
+│   │   └── run_cli.py      # CLI runner
🤖 Prompt for AI Agents
In `@docs/cli-provisioning-design.md` around lines 213 - 229, The documented CLI
directory tree is outdated; update the snippet in cli-provisioning-design.md to
match the actual implementation by replacing the listed filenames under
cli/commands/provision (e.g., provision/command.py, provision/tui.py,
provision/webui.py, provision/desktop.py) with the real filenames used in the
codebase—open the file, locate the current tree block and edit the entries so
each referenced symbol (module names and their purpose like command.py, tui.py,
webui.py, desktop.py) reflects the actual filenames and any renamed modules in
the repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress review-requested GitHub Bot creates notification on #pr-review-ai-ide-team slack channel size/xxl

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant