didericis/bot-bottle
1
0
Fork 0
Code Issues 37 Pull Requests 3 Actions Packages Releases Wiki Activity

Remove CLI backend assumptions and add docker fallback prompt #434

Merged
didericis merged 5 commits from remove-cli-backend-assumptions into main 2026-07-20 18:32:49 -04:00
Conversation 6 Commits 5 Files Changed 11
+301 -113
didericis-claude commented 2026-07-20 15:13:35 -04:00
Collaborator
Copy Link
Copy Source

Closes #344.

Summary

  • Remove --backend flag from cli.py start: backend selection is now driven exclusively by BOT_BOTTLE_BACKEND env var or two-tier auto-selection. resume and ./cli.py backend still accept explicit names.
  • Fix cross-platform crash in cli.py cleanup: skip unavailable backends (e.g. macos-container on Linux) rather than calling prepare_cleanup() which called require_container() and died.
  • Remove hardcoded --backend=macos-container from firecracker/util.py error message.
  • Add two-tier auto-selection with docker fallback prompt: when no VM backend is available, warn that docker is less secure and offer an interactive [i]nstall instructions / [d]ocker anyway / [q]uit prompt. When docker is also absent, print VM install instructions and exit.

The VM preference is platform-aware: macos-container on macOS, firecracker on KVM-capable Linux. A KVM-capable host without the firecracker binary still selects firecracker (unchanged) so its preflight prints an install pointer instead of silently falling back to docker.

Closes #344. ## Summary - **Remove `--backend` flag from `cli.py start`**: backend selection is now driven exclusively by `BOT_BOTTLE_BACKEND` env var or two-tier auto-selection. `resume` and `./cli.py backend` still accept explicit names. - **Fix cross-platform crash in `cli.py cleanup`**: skip unavailable backends (e.g. `macos-container` on Linux) rather than calling `prepare_cleanup()` which called `require_container()` and died. - **Remove hardcoded `--backend=macos-container`** from `firecracker/util.py` error message. - **Add two-tier auto-selection with docker fallback prompt**: when no VM backend is available, warn that docker is less secure and offer an interactive `[i]nstall instructions / [d]ocker anyway / [q]uit` prompt. When docker is also absent, print VM install instructions and exit. The VM preference is platform-aware: `macos-container` on macOS, `firecracker` on KVM-capable Linux. A KVM-capable host without the `firecracker` binary still selects `firecracker` (unchanged) so its preflight prints an install pointer instead of silently falling back to docker.
didericis-claude added 1 commit 2026-07-20 15:13:36 -04:00
fix: remove CLI backend assumptions and add docker fallback prompt
test / stage-firecracker-inputs (pull_request) Successful in 3s
Details
tracker-policy-pr / check-pr (pull_request) Successful in 12s
Details
lint / lint (push) Successful in 42s
Details
test / integration-docker (pull_request) Successful in 33s
Details
test / unit (pull_request) Successful in 37s
Details
test / build-infra (pull_request) Successful in 3m47s
Details
test / integration-firecracker (pull_request) Successful in 1m39s
Details
test / coverage (pull_request) Failing after 1m27s
Details
test / publish-infra (pull_request) Has been skipped
Details
8b5b5730ae 
- Remove hardcoded --backend=macos-container flag reference in
  firecracker/util.py require_firecracker() error message
- Remove --backend flag from cli.py start; backend selection now
  driven exclusively by BOT_BOTTLE_BACKEND env var or auto-selection
- Skip unavailable backends in cli.py cleanup (fixes crash on Linux
  when macos-container.prepare_cleanup calls require_container())
- Add two-tier auto-selection: VM backend first (macos-container on
  macOS, firecracker on Linux+KVM); fall back to docker with a
  security warning and interactive i/d/q prompt; exit if docker
  also unavailable and print VM install instructions

Closes #344
didericis added 1 commit 2026-07-20 15:26:07 -04:00
test: cover _read_tty_line, macos install branch, and reply==i path
test / stage-firecracker-inputs (pull_request) Successful in 3s
Details
tracker-policy-pr / check-pr (pull_request) Successful in 11s
Details
test / integration-docker (pull_request) Successful in 14s
Details
lint / lint (push) Successful in 51s
Details
test / unit (pull_request) Successful in 1m29s
Details
test / build-infra (pull_request) Successful in 3m30s
Details
test / integration-firecracker (pull_request) Successful in 1m34s
Details
test / coverage (pull_request) Successful in 1m30s
Details
test / publish-infra (pull_request) Has been skipped
Details
b1ebc6f1b8
didericis reviewed 2026-07-20 15:40:57 -04:00
bot_bottle/backend/__init__.py Outdated
@@ -669,3 +669,3 @@
def _default_backend_name() -> str:
def _read_tty_line() -> str:
didericis commented 2026-07-20 15:40:57 -04:00
Owner
Copy Link
Copy Source

This logic is undoubtably used other places, should be called from a util script/consolidate where this logic is defined

This logic is undoubtably used other places, should be called from a util script/consolidate where this logic is defined
didericis marked this conversation as resolved
didericis-claude reviewed 2026-07-20 15:42:31 -04:00
bot_bottle/backend/__init__.py Outdated
@@ -669,3 +669,3 @@
def _default_backend_name() -> str:
def _read_tty_line() -> str:
didericis-claude commented 2026-07-20 15:42:31 -04:00
Author
Collaborator
Copy Link
Copy Source

Good call — the same logic lives in cli/_common.py::read_tty_line. Will move it to bot_bottle/util.py so both the CLI layer and the backend can import from one place.

Good call — the same logic lives in `cli/_common.py::read_tty_line`. Will move it to `bot_bottle/util.py` so both the CLI layer and the backend can import from one place.
👍 1
didericis added 1 commit 2026-07-20 15:44:40 -04:00
refactor: consolidate read_tty_line into bot_bottle/util.py
tracker-policy-pr / check-pr (pull_request) Successful in 10s
Details
test / integration-docker (pull_request) Successful in 26s
Details
test / unit (pull_request) Successful in 36s
Details
lint / lint (push) Failing after 42s
Details
test / stage-firecracker-inputs (pull_request) Successful in 2s
Details
test / build-infra (pull_request) Successful in 3m57s
Details
test / integration-firecracker (pull_request) Successful in 1m55s
Details
test / coverage (pull_request) Successful in 2m14s
Details
test / publish-infra (pull_request) Has been skipped
Details
b4b73a8acc 
Remove the private _read_tty_line duplicate from backend/__init__.py and
the local definition from cli/_common.py. Both now import from the
shared bot_bottle.util module.
didericis-codex requested changes 2026-07-20 15:47:54 -04:00
didericis-codex left a comment
Collaborator
Copy Link
Copy Source

Found one blocking issue:

  • P1 — start --headless can now block on an interactive backend prompt. The headless launch eventually calls get_bottle_backend() without conveying that it is non-interactive. On a Docker-only host with no BOT_BOTTLE_BACKEND, _auto_select_backend() reads /dev/tty for the [i/d/q] choice. This violates the documented headless contract and can hang CI, webhook dispatch, and orchestrators. Headless selection should either fail immediately with an actionable message or select Docker according to an explicit non-interactive policy; it must not prompt.

The focused suite passed (67 tests via unittest), but the headless tests mock _launch_bottle before backend resolution, so they do not exercise this path.

Found one blocking issue: - **P1 — `start --headless` can now block on an interactive backend prompt.** The headless launch eventually calls `get_bottle_backend()` without conveying that it is non-interactive. On a Docker-only host with no `BOT_BOTTLE_BACKEND`, `_auto_select_backend()` reads `/dev/tty` for the `[i/d/q]` choice. This violates the documented headless contract and can hang CI, webhook dispatch, and orchestrators. Headless selection should either fail immediately with an actionable message or select Docker according to an explicit non-interactive policy; it must not prompt. The focused suite passed (67 tests via unittest), but the headless tests mock `_launch_bottle` before backend resolution, so they do not exercise this path.
didericis added 1 commit 2026-07-20 15:58:23 -04:00
fix(backend): don't prompt in headless/non-interactive backend selection
lint / lint (push) Failing after 45s
Details
test / unit (pull_request) Successful in 1m34s
Details
test / integration-docker (pull_request) Successful in 12s
Details
tracker-policy-pr / check-pr (pull_request) Successful in 20s
Details
test / stage-firecracker-inputs (pull_request) Successful in 3s
Details
test / build-infra (pull_request) Successful in 3m21s
Details
test / integration-firecracker (pull_request) Successful in 1m30s
Details
test / coverage (pull_request) Successful in 1m30s
Details
test / publish-infra (pull_request) Has been skipped
Details
d9e685e860 
_auto_select_backend gains a prompt parameter (default True). When
prompt=False the docker-fallback [i/d/q] menu is skipped and the call
dies immediately with an actionable message ("set
BOT_BOTTLE_BACKEND=docker or install a VM backend"), preventing hangs
in CI, webhook dispatch, and orchestrator launches.

prepare_with_preflight passes prompt=not spec.headless so the headless
start path can never block waiting for TTY input it cannot receive.
didericis-claude reviewed 2026-07-20 15:59:13 -04:00
didericis-claude left a comment
Author
Collaborator
Copy Link
Copy Source

Fixed in d9e685e.

_auto_select_backend now accepts prompt: bool = True. When prompt=False the docker-fallback [i/d/q] menu is replaced by an immediate die() with an actionable message ("set BOT_BOTTLE_BACKEND=docker or install the VM backend"), so no TTY read can block a headless caller.

prepare_with_preflight passes prompt=not spec.headless, wiring the flag from BottleSpec into backend selection without threading a new parameter through _launch_bottle. Added test_docker_fallback_non_interactive_dies to cover the new branch.

Fixed in d9e685e. `_auto_select_backend` now accepts `prompt: bool = True`. When `prompt=False` the docker-fallback `[i/d/q]` menu is replaced by an immediate `die()` with an actionable message (`"set BOT_BOTTLE_BACKEND=docker or install the VM backend"`), so no TTY read can block a headless caller. `prepare_with_preflight` passes `prompt=not spec.headless`, wiring the flag from `BottleSpec` into backend selection without threading a new parameter through `_launch_bottle`. Added `test_docker_fallback_non_interactive_dies` to cover the new branch.
bot_bottle/backend/__init__.py
@@ -682,0 +726,4 @@
"docker is less secure than VM backends — "
"containers share the host kernel."
)
if not prompt:
didericis-claude commented 2026-07-20 15:59:13 -04:00
Author
Collaborator
Copy Link
Copy Source

Added if not prompt: die(...) here. When prompt=False (headless / CI) the docker-fallback prompt is bypassed entirely — die() emits an actionable message and exits instead of blocking on read_tty_line().

Added `if not prompt: die(...)` here. When `prompt=False` (headless / CI) the docker-fallback prompt is bypassed entirely — `die()` emits an actionable message and exits instead of blocking on `read_tty_line()`.
didericis added 1 commit 2026-07-20 16:11:13 -04:00
fix(lint): mark read_tty_line as an explicit re-export in _common.py
test / coverage (push) Successful in 2m6s
Details
test / publish-infra (push) Successful in 2m38s
Details
test / build-infra (push) Successful in 3m49s
Details
test / integration-firecracker (push) Successful in 2m14s
Details
test / stage-firecracker-inputs (pull_request) Successful in 2s
Details
tracker-policy-pr / check-pr (pull_request) Successful in 12s
Details
test / integration-docker (pull_request) Successful in 27s
Details
test / unit (pull_request) Successful in 34s
Details
test / build-infra (pull_request) Successful in 3m39s
Details
test / integration-firecracker (pull_request) Successful in 1m33s
Details
test / coverage (pull_request) Successful in 2m2s
Details
test / publish-infra (pull_request) Has been skipped
Details
test / stage-firecracker-inputs (push) Successful in 3s
Details
test / unit (push) Successful in 34s
Details
test / integration-docker (push) Successful in 32s
Details
lint / lint (push) Successful in 43s
Details
Update Quality Badges / update-badges (push) Failing after 39s
Details
c7375051fd 
Pyright reportUnusedImport fired because the bare import is not used
within _common.py itself. The X as X idiom signals an intentional
public re-export without requiring __all__.
didericis-codex approved these changes 2026-07-20 18:30:13 -04:00
didericis-codex left a comment
Collaborator
Copy Link
Copy Source

Re-reviewed at c7375051fd3b0afb5a9e825c7dc6cd7d6063cacc. The prior blocking issue is resolved: headless launches now pass prompt=False into backend auto-selection, which exits with actionable guidance instead of reading from a TTY, and the selector has direct regression coverage. No new findings. The 68 focused affected tests pass.

Re-reviewed at `c7375051fd3b0afb5a9e825c7dc6cd7d6063cacc`. The prior blocking issue is resolved: headless launches now pass `prompt=False` into backend auto-selection, which exits with actionable guidance instead of reading from a TTY, and the selector has direct regression coverage. No new findings. The 68 focused affected tests pass.
didericis-codex approved these changes 2026-07-20 18:30:21 -04:00
didericis-codex left a comment
Collaborator
Copy Link
Copy Source

Re-reviewed at c7375051fd3b0afb5a9e825c7dc6cd7d6063cacc. The prior blocking issue is resolved and I found no new issues. The 68 focused affected tests pass.

Re-reviewed at `c7375051fd3b0afb5a9e825c7dc6cd7d6063cacc`. The prior blocking issue is resolved and I found no new issues. The 68 focused affected tests pass.
didericis merged commit c7375051fd into main 2026-07-20 18:32:49 -04:00
didericis deleted branch remove-cli-backend-assumptions 2026-07-20 18:32:49 -04:00
Sign in to join this conversation.
Reviewers
No Reviewers
didericis
didericis-claude
didericis-codex
Labels
Clear labels
Compat/Breaking
Breaking change that won't be backward compatible
 Kind/Bug
Something is not working
 Kind/Documentation
Documentation changes
 Kind/Enhancement
Improve existing functionality
 Kind/Feature
New functionality
 Kind/Security
This is security issue
 Kind/Testing
Issue or pull request related to testing
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Low
4
The priority is low
Priority
Medium
3
The priority is medium
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
Status/Needs Triage
Awaiting initial classification
No Label
Milestone
No items
No Milestone
Assignees
Clear assignees
didericis-claude (didericis (claude)) didericis-codex (didericis (codex)) didericis-gemma (gemma) didericis
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: didericis/bot-bottle#434
Delete Branch "remove-cli-backend-assumptions"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?