mirror of
https://github.com/usestrix/strix.git
synced 2026-08-16 09:26:39 +02:00
fix(tui): make the mount prompt clickable, and skip the mount instead of abandoning the scan (#1015)
* make the working-directory prompt answer the mouse Its Confirm and Cancel were drawn as buttons and did nothing when clicked: the modal mouse handler had a case for every dialog except this one, so a click fell through and the scan sat waiting on an answer the user believed they had given. Only the keyboard could answer it. The prompt is docked in a corner rather than centered, so it also needs its own bounds; the centered ones every other dialog uses would have put the buttons in the wrong place. Those bounds now come from the same placement cornerOverlay draws with. Two returns that hand back the model alongside a call that mutates it are now sequenced explicitly. They work, but only because the compiler happens to evaluate the call first, and one of them is what puts the prompt back in the composer when the mount is declined. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * skip the mount instead of abandoning the scan Declining the working-directory prompt threw the whole launch away and dropped back to the start screen, which is a lot to lose for answering one question about one directory. The two answers are now about the directory alone: mount it, or run without it. The prompt is the whole of the input either way. The buttons say which is which - Mount and Skip rather than Confirm and Cancel - and the prompt says what skipping costs. A run with neither target nor directory is a real run, so two things follow it. It can be resumed: its instruction is what drives it, and that is in the run record. And it tells the agent plainly that it has neither, because an agent given no scope goes looking for the one it assumes it was meant to have. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
f8a8801d56
commit
b260a4ee38
@@ -138,6 +138,15 @@ def build_root_task(scan_config: dict[str, Any]) -> str:
|
||||
"target to assess: the instructions below are the only source of "
|
||||
"truth for what to do."
|
||||
)
|
||||
elif not parts and user_instructions:
|
||||
# Neither a target nor a directory, but there is an instruction: the user
|
||||
# declined the mount, so the instruction is all there is. Say so, or the
|
||||
# agent goes looking for a scope that was never given.
|
||||
parts.append(
|
||||
"\n\nNo scan target and no working directory were provided. The "
|
||||
"instructions below are the only source of truth for what to do; "
|
||||
"work from them and from what you can reach yourself."
|
||||
)
|
||||
|
||||
parts.extend(_render_diff_scope(diff_scope))
|
||||
|
||||
|
||||
@@ -328,10 +328,11 @@ def _load_resume_state(args: argparse.Namespace, parser: argparse.ArgumentParser
|
||||
parser.error(f"--resume {args.resume}: run.json unreadable: {exc}")
|
||||
|
||||
args.targets_info = state.get("targets_info") or []
|
||||
# A target-less run has no targets_info at all: it works in a mounted
|
||||
# directory, driven by its instruction.
|
||||
# A target-less run has no targets_info at all. It is driven by its
|
||||
# instruction, over a mounted working directory or over nothing when the
|
||||
# mount was declined, so either of those is enough to resume it.
|
||||
workspace_mount = state.get("workspace_mount") or None
|
||||
if not args.targets_info and not workspace_mount:
|
||||
if not args.targets_info and not workspace_mount and not state.get("user_instruction"):
|
||||
parser.error(f"--resume {args.resume}: run.json has no targets_info")
|
||||
|
||||
for target in args.targets_info:
|
||||
|
||||
@@ -138,13 +138,6 @@ class TuiController:
|
||||
self.error = detail
|
||||
self.notify_changed()
|
||||
|
||||
def enter_setup(self) -> None:
|
||||
"""Return a session to the start screen, e.g. on a declined mount."""
|
||||
self.setup_mode = True
|
||||
self.scan_started = False
|
||||
self.scan_state = "setup"
|
||||
self.notify_changed()
|
||||
|
||||
def add_message(self, text: str, level: str = "info") -> None:
|
||||
self._append_message(text, level)
|
||||
self.notify_changed()
|
||||
@@ -356,14 +349,12 @@ class TuiController:
|
||||
if not isinstance(approved, bool):
|
||||
raise TypeError("approved must be a boolean")
|
||||
self.pending_workspace_mount = None
|
||||
if not approved:
|
||||
# Nothing was prepared, so return to the start screen untouched.
|
||||
self.workspace_mount = None
|
||||
self.enter_setup()
|
||||
return {"approved": False}
|
||||
self.workspace_mount = mount
|
||||
# Declining skips the mount, it does not abandon the scan. The prompt is
|
||||
# the whole of the input either way; the working directory is only an
|
||||
# extra the agent may look at, so the run goes ahead without one.
|
||||
self.workspace_mount = mount if approved else None
|
||||
await self._begin_scan(self._pending_verify)
|
||||
return {"approved": True}
|
||||
return {"approved": approved}
|
||||
|
||||
async def _send_message(self, payload: dict[str, Any]) -> dict[str, Any]:
|
||||
agent_id = self._required_string(payload, "agent_id")
|
||||
|
||||
@@ -66,13 +66,10 @@ func (m *Model) submitSetupPrompt(value string) (tea.Model, tea.Cmd) {
|
||||
}
|
||||
|
||||
// answerMountConfirmation replies to the working-directory mount the backend is
|
||||
// waiting on. Declining returns to the start screen, so the prompt goes back in
|
||||
// the composer to be edited or given a target instead.
|
||||
// waiting on. Either answer starts the scan - declining only means it runs
|
||||
// without the directory - so the prompt stays with the run rather than coming
|
||||
// back to the composer.
|
||||
func (m *Model) answerMountConfirmation(approved bool) tea.Cmd {
|
||||
if !approved && m.pendingPrompt != "" {
|
||||
m.input.SetValue(m.pendingPrompt)
|
||||
m.resizeViewport()
|
||||
}
|
||||
m.pendingPrompt = ""
|
||||
return send(m.client, "setup.confirm_mount", map[string]any{"approved": approved})
|
||||
}
|
||||
|
||||
@@ -247,13 +247,10 @@ func TestMountConfirmationAnswers(t *testing.T) {
|
||||
if payload.Approved != tc.approved {
|
||||
t.Fatalf("%s: approved=%v, want %v", tc.name, payload.Approved, tc.approved)
|
||||
}
|
||||
// Declining returns to the start screen, so the prompt comes back.
|
||||
want := ""
|
||||
if !tc.approved {
|
||||
want = "find auth bugs in the login flow"
|
||||
}
|
||||
if got := model.input.Value(); got != want {
|
||||
t.Fatalf("%s: composer = %q, want %q", tc.name, got, want)
|
||||
// Either answer launches, so the prompt stays with the run rather than
|
||||
// coming back to the composer.
|
||||
if got := model.input.Value(); got != "" {
|
||||
t.Fatalf("%s: composer = %q, want it cleared", tc.name, got)
|
||||
}
|
||||
if model.pendingPrompt != "" {
|
||||
t.Fatalf("%s: held prompt was not cleared: %q", tc.name, model.pendingPrompt)
|
||||
@@ -290,3 +287,101 @@ func TestSetupPromptWithTargetLaunches(t *testing.T) {
|
||||
t.Fatalf("setup.start (%d) must come after setup.set_instruction (%d): %v", start, instr, types)
|
||||
}
|
||||
}
|
||||
|
||||
// The prompt's buttons are buttons: clicking Cancel has to answer the backend,
|
||||
// which it could not do while the mouse handler had no case for this modal.
|
||||
func TestMountPromptButtonsAreClickable(t *testing.T) {
|
||||
for _, testCase := range []struct {
|
||||
label string
|
||||
approved bool
|
||||
}{
|
||||
{mountConfirmLabel, true},
|
||||
{mountCancelLabel, false},
|
||||
} {
|
||||
connection := &recordingConn{}
|
||||
model := New(&Client{conn: connection})
|
||||
model.width, model.height = 130, 40
|
||||
model.snapshot = protocol.Snapshot{SetupMode: true, WorkingDir: "/Users/me/code/api"}
|
||||
updated, _ := model.submit("find auth bugs in the login flow")
|
||||
model = updated.(Model)
|
||||
connection.Reset()
|
||||
model.snapshot = protocol.Snapshot{
|
||||
ScanStarted: true, ScanState: "preparing", PendingMount: "/Users/me/code/api",
|
||||
}
|
||||
model.syncMountPrompt()
|
||||
|
||||
left, top, panel := model.mountPromptBounds()
|
||||
clicked := false
|
||||
for row, line := range strings.Split(panel, "\n") {
|
||||
plain := ansi.Strip(line)
|
||||
index := strings.Index(plain, testCase.label)
|
||||
if index < 0 {
|
||||
continue
|
||||
}
|
||||
updated, cmd := model.updateModalMouse(tea.MouseMsg{
|
||||
X: left + ansi.StringWidth(plain[:index]) + 1, Y: top + row,
|
||||
Button: tea.MouseButtonLeft, Action: tea.MouseActionPress,
|
||||
})
|
||||
model = updated.(Model)
|
||||
envelopes := drainCommands(t, cmd, connection)
|
||||
if len(envelopes) != 1 || envelopes[0].Type != "setup.confirm_mount" {
|
||||
t.Fatalf("clicking %s sent %v", testCase.label, commandTypes(envelopes))
|
||||
}
|
||||
var payload struct {
|
||||
Approved bool `json:"approved"`
|
||||
}
|
||||
if err := json.Unmarshal(envelopes[0].Payload, &payload); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if payload.Approved != testCase.approved {
|
||||
t.Fatalf("clicking %s answered approved=%v", testCase.label, payload.Approved)
|
||||
}
|
||||
clicked = true
|
||||
break
|
||||
}
|
||||
if !clicked {
|
||||
t.Fatalf("%s was not found in the prompt", testCase.label)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Skipping the mount runs the scan without a directory. It must not throw the
|
||||
// session back to the start screen, and it must not hand the prompt back: the
|
||||
// run has it.
|
||||
func TestSkippingTheMountKeepsTheScanRunning(t *testing.T) {
|
||||
connection := &recordingConn{}
|
||||
model := New(&Client{conn: connection})
|
||||
model.width, model.height = 130, 40
|
||||
model.snapshot = protocol.Snapshot{SetupMode: true, WorkingDir: "/Users/me/code/api"}
|
||||
updated, _ := model.submit("find auth bugs in the login flow")
|
||||
model = updated.(Model)
|
||||
model.snapshot = protocol.Snapshot{
|
||||
ScanStarted: true, ScanState: "preparing", PendingMount: "/Users/me/code/api",
|
||||
}
|
||||
model.syncMountPrompt()
|
||||
if model.modal != modalConfirmMount {
|
||||
t.Fatal("the prompt did not open")
|
||||
}
|
||||
|
||||
model.modalChoice = 1
|
||||
updated, _ = model.updateModal(tea.KeyMsg{Type: tea.KeyEnter})
|
||||
model = updated.(Model)
|
||||
|
||||
// The backend answers by starting the scan with no mount.
|
||||
model.handleEnvelope(stateEnvelope(t, 2, protocol.Snapshot{
|
||||
ScanStarted: true, ScanState: "running",
|
||||
}))
|
||||
|
||||
if model.modal != modalNone {
|
||||
t.Fatalf("the prompt is still open: %v", model.modal)
|
||||
}
|
||||
if model.snapshot.SetupMode {
|
||||
t.Fatal("skipping the mount fell back to the start screen")
|
||||
}
|
||||
if got := model.input.Value(); got != "" {
|
||||
t.Fatalf("the prompt came back to the composer: %q", got)
|
||||
}
|
||||
if model.pendingPrompt != "" {
|
||||
t.Fatalf("the held prompt was not released: %q", model.pendingPrompt)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -474,6 +474,18 @@ func (m Model) updateModalMouse(msg tea.MouseMsg) (tea.Model, tea.Cmd) {
|
||||
m.modalChoice = 1
|
||||
return m.updateModal(tea.KeyMsg{Type: tea.KeyEnter})
|
||||
}
|
||||
case modalConfirmMount:
|
||||
left, top, panel := m.mountPromptBounds()
|
||||
if labelHitAt(panel, mountConfirmLabel, left, top, msg.X, msg.Y) {
|
||||
m.modalChoice = 0
|
||||
cmd := m.answerMountConfirmation(true)
|
||||
return m, cmd
|
||||
}
|
||||
if labelHitAt(panel, mountCancelLabel, left, top, msg.X, msg.Y) {
|
||||
m.modalChoice = 1
|
||||
cmd := m.answerMountConfirmation(false)
|
||||
return m, cmd
|
||||
}
|
||||
case modalVulnerability:
|
||||
for _, button := range m.reportButtons() {
|
||||
if button == reportCopy || button == reportDone {
|
||||
@@ -507,7 +519,14 @@ func (m Model) centeredViewBounds(view string) (left, top, width, height int) {
|
||||
|
||||
func (m Model) centeredLabelHit(view, label string, x, y int) bool {
|
||||
left, top, _, _ := m.centeredViewBounds(view)
|
||||
for row, line := range strings.Split(view, "\n") {
|
||||
return labelHitAt(view, label, left, top, x, y)
|
||||
}
|
||||
|
||||
// labelHitAt reports whether a click landed on a label drawn in a panel whose
|
||||
// top-left corner is at (left, top). The mount prompt is docked in a corner
|
||||
// rather than centered, so it cannot use the centered bounds.
|
||||
func labelHitAt(panel, label string, left, top, x, y int) bool {
|
||||
for row, line := range strings.Split(panel, "\n") {
|
||||
plain := ansi.Strip(line)
|
||||
index := strings.Index(plain, label)
|
||||
if index < 0 || y != top+row {
|
||||
@@ -593,7 +612,8 @@ func (m Model) updateModal(key tea.KeyMsg) (tea.Model, tea.Cmd) {
|
||||
case "esc":
|
||||
if m.modal == modalConfirmMount {
|
||||
// The backend is waiting on an answer; escape declines it.
|
||||
return m, m.answerMountConfirmation(false)
|
||||
cmd := m.answerMountConfirmation(false)
|
||||
return m, cmd
|
||||
}
|
||||
m.closeModal()
|
||||
return m, nil
|
||||
@@ -604,7 +624,10 @@ func (m Model) updateModal(key tea.KeyMsg) (tea.Model, tea.Cmd) {
|
||||
modal, choice := m.modal, m.modalChoice
|
||||
if modal == modalConfirmMount {
|
||||
// The snapshot closes this prompt once the backend has the answer.
|
||||
return m, m.answerMountConfirmation(choice == 0)
|
||||
// Bound to a variable first: the call restores the held prompt into
|
||||
// the composer, and that has to be in the model being returned.
|
||||
cmd := m.answerMountConfirmation(choice == 0)
|
||||
return m, cmd
|
||||
}
|
||||
m.closeModal()
|
||||
if choice == 1 {
|
||||
|
||||
@@ -271,6 +271,23 @@ func (m Model) viewInner() string {
|
||||
return m.toastOverlay(main)
|
||||
}
|
||||
|
||||
// mountPromptBounds is where the working-directory prompt is drawn. It is placed
|
||||
// by cornerOverlay rather than centered, so a click has to be tested against
|
||||
// these bounds and not the ones the other modals use.
|
||||
func (m Model) mountPromptBounds() (left, top int, panel string) {
|
||||
panel = m.modalView()
|
||||
if panel == "" {
|
||||
return 0, 0, ""
|
||||
}
|
||||
_, _, chatWidth, _ := m.layout()
|
||||
left = max(0, min(chatWidth, m.width)-lipgloss.Width(panel))
|
||||
statusH := 0
|
||||
if m.statusVisible() {
|
||||
statusH = 1
|
||||
}
|
||||
return left, max(0, m.inputTop()-statusH-lipgloss.Height(panel)), panel
|
||||
}
|
||||
|
||||
// cornerOverlay splices a panel in directly above the composer, right-aligned
|
||||
// with it, leaving the rest of the view visible behind it.
|
||||
func (m Model) cornerOverlay(view, panel string) string {
|
||||
|
||||
@@ -220,6 +220,13 @@ func (m Model) confirmView(title string, width int, border, titleColor lipgloss.
|
||||
return m.confirmDialog(title, "", width, border, titleColor, red, "Yes", "No")
|
||||
}
|
||||
|
||||
// The mount prompt's buttons, named so the renderer and the click test cannot
|
||||
// drift apart.
|
||||
const (
|
||||
mountConfirmLabel = "Mount"
|
||||
mountCancelLabel = "Skip"
|
||||
)
|
||||
|
||||
// mountConfirmView asks before a target-less scan mounts the working directory.
|
||||
// It is a compact prompt docked in the corner of the live view: nothing is
|
||||
// prepared until it is answered, and the directory is a workspace rather than a
|
||||
@@ -232,8 +239,8 @@ func (m Model) mountConfirmView() string {
|
||||
}
|
||||
title := render.Bold(amber).Render("△ Mount working directory?")
|
||||
body := render.Col(white).Render(truncatePath(dir, width-4)) + "\n" +
|
||||
render.Dim().Render("writable in the sandbox")
|
||||
return m.cornerPrompt(title, body, width, "Confirm", "Cancel")
|
||||
render.Dim().Render("writable in the sandbox · skip to run without it")
|
||||
return m.cornerPrompt(title, body, width, mountConfirmLabel, mountCancelLabel)
|
||||
}
|
||||
|
||||
// truncatePath keeps the tail of a path visible, which is the part that
|
||||
|
||||
@@ -234,12 +234,11 @@ async def test_confirming_the_mount_starts_the_scan_without_a_target() -> None:
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_declining_the_mount_returns_to_the_start_screen() -> None:
|
||||
started = False
|
||||
async def test_declining_the_mount_runs_without_one() -> None:
|
||||
started: list[bool] = []
|
||||
|
||||
async def start(_verify: bool = True) -> None:
|
||||
nonlocal started
|
||||
started = True
|
||||
async def start(verify: bool = True) -> None:
|
||||
started.append(verify)
|
||||
|
||||
os.environ["STRIX_LLM"] = "anthropic/claude-sonnet-4"
|
||||
os.environ["ANTHROPIC_API_KEY"] = "test-key"
|
||||
@@ -250,14 +249,34 @@ async def test_declining_the_mount_returns_to_the_start_screen() -> None:
|
||||
result = await controller.handle("setup.confirm_mount", {"approved": False})
|
||||
|
||||
assert result == {"approved": False}
|
||||
# Nothing was prepared, so the session goes back to the start screen and can
|
||||
# be launched again.
|
||||
assert started is False
|
||||
# Declining skips the directory; it does not abandon the scan.
|
||||
assert started == [False]
|
||||
assert controller.workspace_mount is None
|
||||
assert controller.pending_workspace_mount is None
|
||||
assert controller.setup_mode is True
|
||||
assert controller.scan_started is False
|
||||
assert controller.scan_state == "setup"
|
||||
assert controller.setup_mode is False
|
||||
assert controller.scan_started is True
|
||||
assert controller.scan_state == "running"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_approving_the_mount_runs_with_it() -> None:
|
||||
started: list[bool] = []
|
||||
|
||||
async def start(verify: bool = True) -> None:
|
||||
started.append(verify)
|
||||
|
||||
os.environ["STRIX_LLM"] = "anthropic/claude-sonnet-4"
|
||||
os.environ["ANTHROPIC_API_KEY"] = "test-key"
|
||||
loader._cached = None
|
||||
controller = TuiController(args(), on_start=start)
|
||||
await controller.handle("setup.start", {"verify": False, "mount_working_dir": True})
|
||||
|
||||
result = await controller.handle("setup.confirm_mount", {"approved": True})
|
||||
|
||||
assert result == {"approved": True}
|
||||
assert started == [False]
|
||||
assert controller.workspace_mount == str(Path.cwd())
|
||||
assert controller.scan_state == "running"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
Reference in New Issue
Block a user