Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions docs/DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -2867,6 +2867,18 @@ Ctrl-C aborts a model turn mid-stream — the motivating case is a long invisibl

**Out of scope (branch 6b — subprocess killpg).** A cancel requested while a tool *command* is running does NOT kill that subprocess; the command finishes/times out, then the next model call's stream-read sees the set event and aborts the turn. Killing the running subprocess on cancel is deferred.

### 31.16 Approval focus-swap (v2)

Approvals are the exception to the fire-and-forget marshaling of §31.13: `ask_approval` / `ask_plan_approval` RETURN a value, so they cannot be enqueued and forgotten. The full-screen app resolves them with a **focus-swap handshake** (`cli/app_approval.py`, `ApprovalGate`): the worker thread blocks on a `concurrent.futures.Future` while the loop thread renders the prompt into the pane and the dock becomes the approval input.

**Worker blocks, loop thread answers.** `ThreadedUI.ask_approval` delegates to `gate.ask_command(request)` (and `ask_plan_approval` → `gate.ask_plan`). The gate schedules a loop-thread setup thunk (via the same `TurnRunner.schedule` marshaler) and then calls `future.result()`, which blocks the worker. The thunk renders the prompt block — `AppUI.show_approval` (capped diff panel via `render_diff(..., max_rows=DiffReveal.WINDOW_ROWS)`, the risk/reason info line, the CWD line) or `AppUI.show_plan_approval` (the plan panel + the sanitized artifact path) — and arms `gate._pending`. Both `Future` operations are thread-safe stdlib primitives, so no lock is needed; `_pending` is touched only on the loop thread (the setup thunk arms it; the dock keybindings read+clear it).

**Three-way outcome, unchanged contract (§14.6).** The dock's submit keybinding routes the typed line to `gate.submit(line)` when `gate.active`, BEFORE the `/exit` check (a mid-approval `/exit` is an approval answer, not a quit). The pure helpers `parse_command_choice` / `parse_plan_choice` carry the contract: a HIGH-risk **command** executes only on the literal `run` (the typed-`run` gate); every other request takes y/e/n; an unrecognized plan token re-prompts. `[e]dit` enters a steer phase — the next line becomes `ApprovalReply.steer_text` (empty = plain decline), which re-enters the deterministic classifier as a re-proposal exactly as in the REPL; the un-approved action never runs. The accepted line is echoed into the pane (sanitized, via `show_status` — never `show_user_message`, which would reset the live turn indicator).

**Ctrl-C / EOF decline THIS action.** During an approval the worker is blocked on the Future, not in a model-stream read, so the §31.15 model-cancel path would not fire. The `c-c` and `c-d` handlers therefore check `gate.active` FIRST and call `gate.cancel()` (resolve as decline — `DECLINE` for a command, `("n", "")` for a plan), mirroring `TerminalUI.ask_approval`'s `except KeyboardInterrupt: return DECLINE`. The turn continues; turn-level cancel is available again once the prompt returns.

**Shutdown ceiling.** A pending approval at app exit leaves the daemon worker blocked on `future.result()`; harmless for this opt-in dev entry (process exit reaps it). On promotion to the shipping default, resolve pending approvals as DECLINE on exit.

## 32. Model Selection And Preload

### 32.1 Boot Model Picker
Expand Down
28 changes: 28 additions & 0 deletions shellpilot/cli/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from collections.abc import Callable, Sequence
from dataclasses import dataclass
from pathlib import Path
from typing import TYPE_CHECKING

from prompt_toolkit.application import Application, get_app
from prompt_toolkit.buffer import Buffer
Expand All @@ -48,6 +49,9 @@
Glyphs,
)

if TYPE_CHECKING:
from shellpilot.cli.app_approval import ApprovalGate

# The dock grows to fit multi-line input up to this many rows, then scrolls
# internally. NOTE: a fixed cap — a per-terminal-height fraction would be nicer
# on a very short terminal, but a flat cap is enough until the live wiring
Expand Down Expand Up @@ -151,6 +155,7 @@ def build_app(
ui: AppUI | None = None,
on_submit: Callable[[str], None] | None = None,
on_interrupt: Callable[[], bool] | None = None,
approval_gate: ApprovalGate | None = None,
) -> Application[None]:
"""Build the full-screen app shell.

Expand Down Expand Up @@ -296,6 +301,14 @@ def _bar() -> Window:
@kb.add("enter", filter=dock_focused)
@kb.add("c-j", filter=dock_focused)
def _submit(event: KeyPressEvent) -> None:
# During an approval the dock IS the approval input: route the line to the
# gate (which resolves the worker's Future) BEFORE the /exit check, so a
# mid-approval "/exit" is an approval answer, not a quit (§31.16).
if approval_gate is not None and approval_gate.active:
line = dock_buffer.text
dock_buffer.reset()
approval_gate.submit(line)
return
text = dock_buffer.text
if text.strip() == "/exit":
event.app.exit()
Expand Down Expand Up @@ -326,6 +339,14 @@ def _page_down(event: KeyPressEvent) -> None:

@kb.add("c-c")
def _interrupt(event: KeyPressEvent) -> None:
# During an approval the worker is blocked on the gate's Future, not in a
# model-stream read, so on_interrupt (model cancel) would not fire. Ctrl-C
# must resolve the approval as a decline of THIS action; the turn continues
# and turn-level cancel is available again after the prompt returns (mirrors
# TerminalUI.ask_approval's KeyboardInterrupt → DECLINE) (§31.16).
if approval_gate is not None and approval_gate.active:
approval_gate.cancel()
return
# Raw mode disables ISIG, so Ctrl-C is a normal key press, not a SIGINT
# (branch 6, §31.15). When a turn is in flight, on_interrupt cancels it and
# returns True (the worker aborts the stream and renders the marker), so we
Expand All @@ -334,6 +355,13 @@ def _interrupt(event: KeyPressEvent) -> None:
return
_ui.show_status(_IDLE_HINT)

@kb.add("c-d", filter=dock_focused)
def _eof(event: KeyPressEvent) -> None:
# EOF during an approval declines THIS action (same as Ctrl-C). Otherwise a
# no-op: the dock owns c-d so a stray press never tears down the app.
if approval_gate is not None and approval_gate.active:
approval_gate.cancel()

# NOTE: keyboard PageUp/PageDown drive the pane via the cursor-line model
# above (auto-follow + scroll-back). Mouse-wheel scroll-back is deferred to
# branch 9: the wheel nudges the window's own vertical_scroll, which the
Expand Down
240 changes: 240 additions & 0 deletions shellpilot/cli/app_approval.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,240 @@
"""Approval focus-swap rendezvous for the full-screen app (design section 31.16).

The full-screen app runs one conversation turn on a worker thread while the
prompt_toolkit event loop owns the main thread (see ``app_turn.py``). Every
fire-and-forget UI call is marshaled loop-ward, but ``ask_approval`` /
``ask_plan_approval`` RETURN a value, so they cannot be fire-and-forget. This is
the focus-swap handshake: the worker blocks on a ``concurrent.futures.Future``
while the loop thread renders the prompt into the pane, reads the user's
keystrokes in the dock, and resolves the future.

The pure parse helpers (:func:`parse_command_choice` / :func:`parse_plan_choice`)
carry the three-way y/e/n + HIGH typed-"run" contract and are unit-tested
directly, with no threading.
"""

from __future__ import annotations

import functools
from collections.abc import Callable
from concurrent.futures import Future
from dataclasses import dataclass
from typing import TYPE_CHECKING

from shellpilot.cli.render import _sanitize_line
from shellpilot.cli.theme import UNICODE_GLYPHS, Glyphs
from shellpilot.policy.approvals import APPROVE, DECLINE, ApprovalReply
from shellpilot.policy.risk import RiskLevel

if TYPE_CHECKING:
from shellpilot.cli.app_turn import Schedule
from shellpilot.cli.app_ui import AppUI
from shellpilot.policy.approvals import ApprovalRequest
from shellpilot.runtime.planner import TaskPlan


class _NeedSteer:
"""Sentinel: the user chose [e]dit at the choice phase; read steering next."""


NEED_STEER = _NeedSteer()


def parse_command_choice(request: ApprovalRequest, answer: str) -> ApprovalReply | _NeedSteer:
"""Map a command/tool approval keystroke to its three-way outcome.

A HIGH-risk COMMAND requires the literal "run" to execute (the typed-"run"
gate, design section 14.6); [e]dit steers without running and anything else
declines. Every other request takes the y/e/n path.
"""
low = answer.strip().lower()
if request.risk is RiskLevel.HIGH and request.kind == "command":
if low == "run":
return APPROVE
if low in ("e", "edit"):
return NEED_STEER
return DECLINE
if low in ("y", "yes"):
return APPROVE
if low in ("e", "edit"):
return NEED_STEER
return DECLINE


def parse_plan_choice(answer: str) -> str | None:
"""Map a plan-approval keystroke to 'y' / 'e' / 'n', or None to re-prompt.

Mirrors :meth:`TerminalUI.ask_plan_approval`: y/e/n plus empty→n; an
unrecognized non-empty token re-prompts (its loop), modeled here as None.
"""
low = answer.strip().lower()
if low in ("y", "yes"):
return "y"
if low in ("e", "edit"):
return "e"
if low in ("n", "no", ""):
return "n"
return None


@dataclass
class _Pending:
"""The in-flight prompt. Loop-thread-only (see :class:`ApprovalGate`)."""

future: Future[object]
# feed(line) -> True when the future was resolved (prompt done), False when
# another input line is wanted (steer/revision phase, or a re-prompt).
feed: Callable[[str], bool]
# Resolve as decline (Ctrl-C/EOF during approval cancels THIS action only).
on_cancel: Callable[[], None]


class ApprovalGate:
"""Thread-safe approval rendezvous (design section 31.16).

``_pending`` is touched ONLY on the loop thread: :meth:`_enter_command` /
:meth:`_enter_plan` set it (scheduled loop-ward by the worker), and
:meth:`submit` / :meth:`cancel` read+clear it (called from keybindings, which
run on the loop thread). The worker thread only ever touches the ``Future``
(``result()`` blocks it; the loop thread's ``set_result`` unblocks it) — both
are thread-safe stdlib primitives, so no lock is needed.

NOTE: a pending approval at app exit leaves the daemon worker blocked on
result(); harmless (process exit reaps it). On promotion to the shipping
default, resolve pending approvals as DECLINE on exit.
"""

def __init__(self, *, ui: AppUI, schedule: Schedule, glyphs: Glyphs = UNICODE_GLYPHS) -> None:
self._ui = ui
self._schedule = schedule
self._glyphs = glyphs
self._pending: _Pending | None = None

# ------------------------------------------------------------------
# Worker-thread entry points — block on the future.
# ------------------------------------------------------------------

def ask_command(self, request: ApprovalRequest) -> ApprovalReply:
future: Future[object] = Future()
self._schedule(functools.partial(self._enter_command, request, future))
result = future.result() # BLOCKS the worker thread
assert isinstance(result, ApprovalReply)
return result

def ask_plan(self, plan: TaskPlan, path: str) -> tuple[str, str]:
future: Future[object] = Future()
self._schedule(functools.partial(self._enter_plan, plan, path, future))
result = future.result() # BLOCKS the worker thread
assert isinstance(result, tuple)
return result

# ------------------------------------------------------------------
# Loop-thread prompt setup (scheduled by the worker entry points).
# ------------------------------------------------------------------

def _echo(self, line: str) -> None:
# Keep the accepted input visible after the dock clears. show_status
# re-sanitizes, so user-controlled text never reaches the pane raw; it
# also (unlike show_user_message) does NOT reset the live turn indicator.
self._ui.show_status(f" {self._glyphs.chevron} {_sanitize_line(line)}")

def _enter_command(self, request: ApprovalRequest, future: Future[object]) -> None:
# Fail closed: a render sink raising on the loop thread must resolve the
# worker's Future (via set_exception) instead of leaving it blocked on
# result() forever — the turn then ends through TurnRunner._run's except
# ("Turn failed") rather than wedging the app. Never approves by accident.
try:
self._ui.show_approval(request)
if request.risk is RiskLevel.HIGH and request.kind == "command":
hint = ' Type "run" to execute, [e]dit to steer, or Enter to cancel'
else:
hint = " Approve? [y]es / [e]dit / [n]o"
self._ui.show_status(hint)
except Exception as exc: # noqa: BLE001 - never leave the worker hung
self._pending = None
if not future.done():
future.set_exception(exc)
return
phase = {"steer": False}

def feed(line: str) -> bool:
if phase["steer"]:
# Empty steer = plain decline (matches TerminalUI._read_steer).
self._echo(line)
future.set_result(ApprovalReply(approved=False, steer_text=line.strip() or None))
return True
decision = parse_command_choice(request, line)
if isinstance(decision, _NeedSteer):
phase["steer"] = True
self._ui.show_status(" Tell the model what to do instead:")
return False
self._echo(line)
future.set_result(decision)
return True

self._pending = _Pending(future, feed, lambda: future.set_result(DECLINE))

def _enter_plan(self, plan: TaskPlan, path: str, future: Future[object]) -> None:
hint = " Approve plan? [y]es / [e]dit / [n]o"
# Fail closed (see _enter_command): a render sink raising here resolves
# the worker's Future rather than wedging it on result().
try:
self._ui.show_plan_approval(plan, path)
self._ui.show_status(hint)
except Exception as exc: # noqa: BLE001 - never leave the worker hung
self._pending = None
if not future.done():
future.set_exception(exc)
return
phase = {"revision": False}

def feed(line: str) -> bool:
if phase["revision"]:
self._echo(line)
future.set_result(("e", line.strip()))
return True
choice = parse_plan_choice(line)
if choice is None:
self._ui.show_status(hint) # re-prompt (matches the TerminalUI loop)
return False
if choice == "e":
phase["revision"] = True
self._ui.show_status(" Describe the changes you want:")
return False
self._echo(line)
future.set_result((choice, ""))
return True

self._pending = _Pending(future, feed, lambda: future.set_result(("n", "")))

# ------------------------------------------------------------------
# Loop-thread public surface — driven by the dock keybindings.
# ------------------------------------------------------------------

@property
def active(self) -> bool:
return self._pending is not None

def submit(self, line: str) -> None:
pending = self._pending
if pending is None:
return
# Fail closed: feed() echoes via show_status BEFORE resolving the Future,
# so a sink raising there must still resolve it — never leave the worker
# blocked on result().
try:
done = pending.feed(line)
except Exception as exc: # noqa: BLE001 - never leave the worker hung
self._pending = None
if not pending.future.done():
pending.future.set_exception(exc)
return
if done:
self._pending = None

def cancel(self) -> None:
pending = self._pending
if pending is None:
return
self._pending = None
pending.on_cancel()
3 changes: 3 additions & 0 deletions shellpilot/cli/app_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from shellpilot.cli.app_turn import TurnRunner

if TYPE_CHECKING:
from shellpilot.cli.app_approval import ApprovalGate
from shellpilot.cli.app_ui import AppUI
from shellpilot.cli.theme import Glyphs
from shellpilot.runtime.conversation import ConversationRuntime
Expand All @@ -38,6 +39,7 @@ def run_app(
commands: Sequence[str],
is_cloud: bool = False,
ctx_pct: int = 0,
approval_gate: ApprovalGate | None = None,
) -> int:
"""Build the full-screen app around an already-wired conversation and run it.

Expand Down Expand Up @@ -71,6 +73,7 @@ def run_app(
ui=app_ui,
on_submit=runner.start,
on_interrupt=runner.request_cancel,
approval_gate=approval_gate,
)
runner.app = app
runner.conversation = runtime
Expand Down
Loading