From 6eb687455d261d9b62ba06b9c6809f470972f427 Mon Sep 17 00:00:00 2001 From: Roman Romenskyi Date: Sun, 18 Oct 2026 19:51:37 -0600 Subject: [PATCH 1/3] Approvals: no answer in two minutes is a no A prompt nobody answered held the task until someone came back. Now it waits two minutes, counting down in the status line, then denies the call and lets the run go on without it. Only an explicit yes runs anything. The timeout is not an answer: the risk model learns nothing from it, and a panel open over the prompt stays open. The plain prompt runs out the same way, and its stdin read gives up with it so the next line goes to the REPL. Co-Authored-By: Claude Opus 5.5 --- README.md | 8 +- cmd/agent/approval_timeout_test.go | 133 +++++++++++++++++++++++++++++ cmd/agent/bridge.go | 26 +++++- cmd/agent/main.go | 13 ++- cmd/agent/tui.go | 41 ++++++++- 5 files changed, 212 insertions(+), 9 deletions(-) create mode 100644 cmd/agent/approval_timeout_test.go diff --git a/README.md b/README.md index 9dbf102..82a2ad3 100644 --- a/README.md +++ b/README.md @@ -470,6 +470,12 @@ pending either way, and answering it always lands you back where you were. A non-overridable deny floor (`rm -rf`, `sudo`, secrets, `.git`, `.env`, …) is always enforced. +**No answer is a no.** A prompt waits two minutes, with the countdown in the +status line. Then the call is denied and the task goes on without it, instead +of sitting on the prompt until you come back. Only an explicit yes runs +anything. The risk model learns nothing from a prompt that ran out, because +nobody judged that call. + **Grant it for the session.** Press **`a`** on any prompt to stop asking about that whole *kind* of action (file changes, shell, git, sub-agent spawns, MCP) for the rest of the session — in memory only, never written to config, cleared by @@ -528,7 +534,7 @@ it learns from: ``` ⚙ run shell cat ~/.ssh/id_rsa ⚠ possible credential access (0.98) ⚠ approve run: cat ~/.ssh/id_rsa ⚠ possible credential access (0.98) - y approve · n deny · a allow all shell commands this session, flagged ones too + y approve · n deny · a allow all shell commands this session, flagged ones too · ↑ Yes/No · or keep typing · no answer in 2m is a no ``` Nothing is shown for a call it had nothing to say about — a number on every diff --git a/cmd/agent/approval_timeout_test.go b/cmd/agent/approval_timeout_test.go new file mode 100644 index 0000000..227a6b3 --- /dev/null +++ b/cmd/agent/approval_timeout_test.go @@ -0,0 +1,133 @@ +package main + +import ( + "bufio" + "context" + "io" + "strings" + "testing" + "time" + + "github.com/charmbracelet/bubbles/spinner" + "github.com/charmbracelet/bubbles/textarea" + tea "github.com/charmbracelet/bubbletea" +) + +// askThroughTheUI starts an approval on the bridge and shows it on m, the way +// the TUI does, returning where the asking tool's answer will arrive. +func askThroughTheUI(t *testing.T, m *tuiModel) <-chan [2]bool { + t.Helper() + got := make(chan [2]bool, 1) + go func() { + ok, answered := m.bridge.ApproveAnswered(context.Background(), "run", "make deploy") + got <- [2]bool{ok, answered} + }() + m.Update(approvalMsg(<-m.bridge.approvals)) + if m.pending == nil { + t.Fatal("the approval was not shown") + } + return got +} + +// A prompt nobody answers used to hold the task until someone came back. Now, +// past approvalTimeout, it is a no: the tool is told so and the run goes on — +// and since nobody judged the call, the risk model is not told anything. +func TestAnUnansweredApprovalIsANoAndTheTaskGoesOn(t *testing.T) { + m := &tuiModel{bridge: newBridge(), input: textarea.New(), state: stRunning, spin: spinner.New()} + got := askThroughTheUI(t, m) + + m.Update(spinner.TickMsg{}) // still in time + if m.pending == nil { + t.Fatal("the approval was denied before its time ran out") + } + + m.approveBy = time.Now().Add(-time.Second) + _, cmd := m.Update(spinner.TickMsg{}) + select { + case r := <-got: + if r[0] || r[1] { + t.Fatalf("ApproveAnswered = (ok %v, answered %v), want (false, false) — a timeout is neither a yes nor anyone's answer", r[0], r[1]) + } + case <-time.After(3 * time.Second): + t.Fatal("the tool was still waiting after the approval ran out") + } + if m.pending != nil || m.state != stRunning { + t.Errorf("pending = %v, state = %v; want none pending, back to running", m.pending, m.state) + } + // The next approval still reaches the screen: running out re-arms the reader. + go m.bridge.ApproveAnswered(context.Background(), "run", "make test") + msgs := make(chan tea.Msg, 4) + if batch, ok := cmd().(tea.BatchMsg); ok { + for _, c := range batch { + if c != nil { + go func() { msgs <- c() }() + } + } + } + for deadline := time.After(3 * time.Second); ; { + var msg tea.Msg + select { + case msg = <-msgs: + case <-deadline: + t.Fatal("the next approval never reached the UI — the expired one left nobody reading") + } + if _, ok := msg.(approvalMsg); ok { + break + } + } + if log := strings.Join(m.history, "\n"); !strings.Contains(log, "no answer in "+approvalWait()) { + t.Errorf("the log does not say why it was denied:\n%s", log) + } +} + +// Running out is not an answer typed into a panel: a panel open over the prompt +// stays open. +func TestAnExpiredApprovalLeavesAPanelOpen(t *testing.T) { + m := &tuiModel{bridge: newBridge(), input: textarea.New(), state: stRewind, spin: spinner.New()} + got := askThroughTheUI(t, m) + m.approveBy = time.Now().Add(-time.Second) + m.Update(spinner.TickMsg{}) + if r := <-got; r[0] { + t.Fatal("an expired approval ran the call") + } + if m.state != stRewind { + t.Errorf("state = %v, want the rewind panel still open", m.state) + } +} + +// The plain prompt runs out the same way, and its read gives up with it: the +// next line typed belongs to the REPL, not to the prompt that already expired. +func TestAnUnansweredPlainPromptIsANo(t *testing.T) { + defer func(d time.Duration) { approvalTimeout = d }(approvalTimeout) + approvalTimeout = 50 * time.Millisecond + + pr, pw := io.Pipe() + t.Cleanup(func() { pr.Close(); pw.Close() }) + s := &stdinApprover{stdin: newStdinOwner(bufio.NewReader(pr))} + + done := make(chan [2]bool, 1) + go func() { + ok, answered := s.ApproveAnswered(context.Background(), "run", "make deploy") + done <- [2]bool{ok, answered} + }() + select { + case r := <-done: + if r[0] || r[1] { + t.Fatalf("ApproveAnswered = (ok %v, answered %v), want (false, false)", r[0], r[1]) + } + case <-time.After(3 * time.Second): + t.Fatal("the plain prompt did not run out") + } + + go func() { _, _ = io.WriteString(pw, "y\n") }() + line := make(chan string, 1) + go func() { l, _ := s.stdin.readCmdLine(); line <- l }() + select { + case l := <-line: + if strings.TrimSpace(l) != "y" { + t.Errorf("the REPL read %q, want the line typed after the prompt expired", l) + } + case <-time.After(3 * time.Second): + t.Fatal("the line typed after the expired prompt never reached the REPL — the dead prompt took it") + } +} diff --git a/cmd/agent/bridge.go b/cmd/agent/bridge.go index 29eb2ac..dfffff7 100644 --- a/cmd/agent/bridge.go +++ b/cmd/agent/bridge.go @@ -2,7 +2,9 @@ package main import ( "context" + "strings" "sync" + "time" "github.com/ipsupport-llc/ipsupport-code/internal/risk" ) @@ -34,6 +36,24 @@ type approvalReq struct { // sees is a score nobody can judge. risk string reply chan bool + // expire is closed by the UI when nobody answered within approvalTimeout: + // the call is denied, and — like esc — that is nobody's judgement of it. + expire chan struct{} +} + +// approvalTimeout is how long a prompt waits for a person. A run left alone +// used to sit on its first prompt until someone came back; now an unanswered +// prompt is a "no" and the task goes on without that call. Only an explicit +// yes runs anything. A var so tests can shorten it. +var approvalTimeout = 2 * time.Minute + +// approvalWait is approvalTimeout as a prompt says it: "2m", not "2m0s". +func approvalWait() string { + s := approvalTimeout.String() + if strings.HasSuffix(s, "m0s") { + s = s[:len(s)-2] + } + return s } func newBridge() *uiBridge { @@ -70,9 +90,9 @@ func (b *uiBridge) ApproveAnswered(ctx context.Context, kind, detail string) (ok abort := b.abort b.mu.Unlock() - reply := make(chan bool, 1) + reply, expire := make(chan bool, 1), make(chan struct{}) select { - case b.approvals <- approvalReq{kind: kind, detail: detail, risk: riskNote(ctx), reply: reply}: + case b.approvals <- approvalReq{kind: kind, detail: detail, risk: riskNote(ctx), reply: reply, expire: expire}: case <-abort: return false, false case <-ctx.Done(): @@ -81,6 +101,8 @@ func (b *uiBridge) ApproveAnswered(ctx context.Context, kind, detail string) (ok select { case ok := <-reply: return ok, true + case <-expire: + return false, false case <-abort: return false, false case <-ctx.Done(): diff --git a/cmd/agent/main.go b/cmd/agent/main.go index 008a1b6..2c78af3 100644 --- a/cmd/agent/main.go +++ b/cmd/agent/main.go @@ -6391,18 +6391,25 @@ func (s *stdinApprover) Approve(ctx context.Context, kind, detail string) bool { func (s *stdinApprover) ApproveAnswered(ctx context.Context, kind, detail string) (ok, answered bool) { s.mu.Lock() defer s.mu.Unlock() - fmt.Fprintf(os.Stderr, "\n[approve %s] %s%s\n allow? [y/N/a=all %s this session%s] ", kind, detail, riskSuffix(ctx), categoryLabel(approvalCategory(kind)), flaggedToo(riskNote(ctx))) + fmt.Fprintf(os.Stderr, "\n[approve %s] %s%s\n allow? [y/N/a=all %s this session%s] (no answer in %s is a no) ", kind, detail, riskSuffix(ctx), categoryLabel(approvalCategory(kind)), flaggedToo(riskNote(ctx)), approvalWait()) + // The read gives up with the deadline too, so a line typed after it goes to + // whoever asks next, not to this expired prompt. + wait, cancel := context.WithTimeout(ctx, approvalTimeout) + defer cancel() result := make(chan lineResult, 1) // buffered: the read goroutine must never block sending here, even if abandoned go func() { - line, err := s.stdin.readApproveLineCtx(ctx) + line, err := s.stdin.readApproveLineCtx(wait) result <- lineResult{line, err} }() var res lineResult select { case res = <-result: - case <-ctx.Done(): + case <-wait.Done(): + if ctx.Err() == nil { + fmt.Fprintf(os.Stderr, "\n no answer in %s — denied; the task goes on without it\n", approvalWait()) + } return false, false } if res.err != nil { diff --git a/cmd/agent/tui.go b/cmd/agent/tui.go index 401d79b..d2bd51b 100644 --- a/cmd/agent/tui.go +++ b/cmd/agent/tui.go @@ -93,6 +93,7 @@ type tuiModel struct { pending *approvalReq preApprove uiState // state being interrupted when stApprove was entered — stRunning normally, or stIdle when a background job's approval is answered via ↑ while idle; restored when the approval is resolved approveChoice bool // selected Yes(true)/No(false) while answering an approval + approveBy time.Time // when the pending approval is denied for want of an answer (see approvalTimeout) cfgCursor int // selected row in the /config panel (stConfig) cfgPending []stagedEdit // /config changes made while a task ran; saved when it ends (staging.go) cfgPhase int // stConfig sub-flow: cfgPhaseList, or an add-provider form field @@ -607,6 +608,12 @@ func (m *tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case spinner.TickMsg: var cmd tea.Cmd m.spin, cmd = m.spin.Update(msg) + // The spinner ticks for the life of the program, so this is where an + // unanswered approval runs out — and where its countdown redraws. + if m.pending != nil && !m.approveBy.IsZero() && !time.Now().Before(m.approveBy) { + model, next := m.expireApproval() + return model, tea.Batch(cmd, next) + } return m, cmd case eventMsg: @@ -652,6 +659,7 @@ func (m *tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case approvalMsg: req := approvalReq(msg) m.pending = &req + m.approveBy = time.Now().Add(approvalTimeout) // Do NOT fetch the next approval yet — that would overwrite m.pending and // orphan this one's reply channel (a hang when the model batches calls). // Multi-line: the head on the ⚠ line, extra detail lines (e.g. the FULL @@ -664,7 +672,7 @@ func (m *tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } m.push(head) m.pushLines(lines[1:]) - m.push(cDim.Render(" y approve · n deny · a allow all " + categoryLabel(approvalCategory(req.kind)) + " this session" + flaggedToo(req.risk) + " · ↑ Yes/No · or keep typing")) + m.push(cDim.Render(" y approve · n deny · a allow all " + categoryLabel(approvalCategory(req.kind)) + " this session" + flaggedToo(req.risk) + " · ↑ Yes/No · or keep typing · no answer in " + approvalWait() + " is a no")) // Go modal by default: a bare y/n/a/enter answers it, and nothing else // leaks half-typed into the chat input while it's waiting. esc drops back // to typing if you want to steer/queue something instead (the approval @@ -1360,6 +1368,33 @@ func (m *tuiModel) resolveApproval(ok bool) (tea.Model, tea.Cmd) { return m, nil } +// expireApproval denies the pending approval nobody answered in time. Unlike an +// answer it leaves a panel open over the prompt where it is, and teaches the +// risk model nothing: nobody judged the call. +func (m *tuiModel) expireApproval() (tea.Model, tea.Cmd) { + if m.state == stApprove { + m.state = m.preApprove + } + if m.pending.expire != nil { + close(m.pending.expire) + } + m.pending = nil + m.push(cDim.Render(" → ") + cErr.Render("denied") + cDim.Render(" · no answer in "+approvalWait()+" — the task goes on without it")) + if model, cmd, drained := m.finalizeTaskDoneAway(); drained { + return model, tea.Batch(cmd, m.waitApproval()) + } + return m, m.waitApproval() +} + +// approvalLeft is the pending approval's countdown, for the status line. +func (m *tuiModel) approvalLeft() string { + if m.pending == nil || m.approveBy.IsZero() { + return "" + } + left := max(time.Until(m.approveBy), 0).Round(time.Second) + return cDim.Render(fmt.Sprintf(" · no in %d:%02d", int(left.Minutes()), int(left.Seconds())%60)) +} + // approveSession approves the pending request AND stops asking about its whole // category for the rest of the session (in-memory; cleared on /new & /clear). func (m *tuiModel) approveSession() (tea.Model, tea.Cmd) { @@ -2202,7 +2237,7 @@ func (m *tuiModel) View() string { // Waiting for a keypress (though you can keep typing) — same unmissable // badge as RESUME GOAL / PLAN READY, not a dim line indistinguishable from // the routine "⚙ run shell" / "⚙ file write" tool-call chatter above it. - status = m.attention("⚠ APPROVAL NEEDED") + cDim.Render(" — y approve · n deny · a allow-session · ↑ Yes/No · or keep typing") + status = m.attention("⚠ APPROVAL NEEDED") + cDim.Render(" — y approve · n deny · a allow-session · ↑ Yes/No · or keep typing") + m.approvalLeft() case m.state == stRunning: if m.retry != nil { remain := time.Until(m.retry.until).Truncate(100 * time.Millisecond) @@ -2326,7 +2361,7 @@ func (m *tuiModel) approvePrompt() string { } else { no = cErr.Bold(true).Render(" ▸No ") } - return m.attention("⚠ APPROVE") + " " + detail + " " + yes + no + cDim.Render(" (a = allow all this session)") + return m.attention("⚠ APPROVE") + " " + detail + " " + yes + no + cDim.Render(" (a = allow all this session)") + m.approvalLeft() } // modeLine is the bottom indicator: auto (executes) or plan (proposes only), From c972719c5e29ad84a564dc5c73f873cbbc64d3f7 Mon Sep 17 00:00:00 2001 From: Roman Romenskyi Date: Sun, 18 Oct 2026 20:01:14 -0600 Subject: [PATCH 2/3] Approvals: review fixes for the timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - An expired prompt finalizes a task that finished behind it only when it was the modal: one behind a panel waits for the panel to close. - A stdin read whose reader gave up returns instead of waiting for a reply that never comes — one leaked goroutine per expired prompt. - A prompt asked after esc is not shown: the tool already has its no, and an answer or a countdown on it meant nothing. Co-Authored-By: Claude Opus 5.5 --- cmd/agent/approval_timeout_test.go | 55 +++++++++++++++++++++++++++--- cmd/agent/bridge.go | 7 +++- cmd/agent/main.go | 17 +++++++-- cmd/agent/tui.go | 18 ++++++++-- 4 files changed, 86 insertions(+), 11 deletions(-) diff --git a/cmd/agent/approval_timeout_test.go b/cmd/agent/approval_timeout_test.go index 227a6b3..22db16d 100644 --- a/cmd/agent/approval_timeout_test.go +++ b/cmd/agent/approval_timeout_test.go @@ -81,17 +81,64 @@ func TestAnUnansweredApprovalIsANoAndTheTaskGoesOn(t *testing.T) { } // Running out is not an answer typed into a panel: a panel open over the prompt -// stays open. +// stays open — even when the task finished behind it, which is finalized when +// the panel closes, not when the prompt runs out. func TestAnExpiredApprovalLeavesAPanelOpen(t *testing.T) { - m := &tuiModel{bridge: newBridge(), input: textarea.New(), state: stRewind, spin: spinner.New()} + m := &tuiModel{bridge: newBridge(), input: textarea.New(), state: stRewind, spin: spinner.New(), taskDoneAway: true} got := askThroughTheUI(t, m) m.approveBy = time.Now().Add(-time.Second) m.Update(spinner.TickMsg{}) if r := <-got; r[0] { t.Fatal("an expired approval ran the call") } - if m.state != stRewind { - t.Errorf("state = %v, want the rewind panel still open", m.state) + if m.state != stRewind || !m.taskDoneAway { + t.Errorf("state = %v, taskDoneAway = %v; want the rewind panel still open and the finished task still waiting on it", m.state, m.taskDoneAway) + } +} + +// A prompt asked after esc was denied tool-side the moment it was asked: it is +// not shown, and the reader moves on to the next one. +func TestAPromptAskedAfterEscIsNotShown(t *testing.T) { + m := &tuiModel{bridge: newBridge(), input: textarea.New(), state: stRunning, spin: spinner.New()} + aborted := make(chan struct{}) + close(aborted) + _, cmd := m.Update(approvalMsg(approvalReq{kind: "run", detail: "make deploy", reply: make(chan bool, 1), aborted: aborted})) + if m.pending != nil || m.state != stRunning { + t.Errorf("pending = %v, state = %v; want the dead prompt not shown", m.pending, m.state) + } + if cmd == nil { + t.Error("skipping a dead prompt must keep reading the next one") + } + + // The same through the bridge: asked, then esc, then read by the UI. + got := make(chan bool, 1) + go func() { ok, _ := m.bridge.ApproveAnswered(context.Background(), "run", "make deploy"); got <- ok }() + req := <-m.bridge.approvals + m.bridge.Abort() + if <-got { + t.Fatal("an aborted approval ran the call") + } + m.Update(approvalMsg(req)) + if m.pending != nil { + t.Error("the bridge's prompt was shown after esc had already denied it") + } +} + +// A read that gave up returns, rather than waiting for a reply its abandoned +// request will never get. +func TestAReadThatGaveUpReturns(t *testing.T) { + pr, pw := io.Pipe() + t.Cleanup(func() { pr.Close(); pw.Close() }) + o := newStdinOwner(bufio.NewReader(pr)) + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { _, _ = o.readApproveLineCtx(ctx); close(done) }() + time.Sleep(20 * time.Millisecond) // queued, waiting for a line + cancel() + select { + case <-done: + case <-time.After(3 * time.Second): + t.Fatal("the read is still waiting after its reader gave up — a goroutine leaked for good") } } diff --git a/cmd/agent/bridge.go b/cmd/agent/bridge.go index dfffff7..bc7e43b 100644 --- a/cmd/agent/bridge.go +++ b/cmd/agent/bridge.go @@ -39,6 +39,11 @@ type approvalReq struct { // expire is closed by the UI when nobody answered within approvalTimeout: // the call is denied, and — like esc — that is nobody's judgement of it. expire chan struct{} + // aborted is the task's abort signal as of the ask. Once it is closed the + // tool has already been told no, so the UI does not show the prompt: an + // answer to it would change nothing, and its countdown would run for a + // call that is gone. + aborted <-chan struct{} } // approvalTimeout is how long a prompt waits for a person. A run left alone @@ -92,7 +97,7 @@ func (b *uiBridge) ApproveAnswered(ctx context.Context, kind, detail string) (ok reply, expire := make(chan bool, 1), make(chan struct{}) select { - case b.approvals <- approvalReq{kind: kind, detail: detail, risk: riskNote(ctx), reply: reply, expire: expire}: + case b.approvals <- approvalReq{kind: kind, detail: detail, risk: riskNote(ctx), reply: reply, expire: expire, aborted: abort}: case <-abort: return false, false case <-ctx.Done(): diff --git a/cmd/agent/main.go b/cmd/agent/main.go index 2c78af3..c6b2f11 100644 --- a/cmd/agent/main.go +++ b/cmd/agent/main.go @@ -6551,9 +6551,20 @@ func (o *stdinOwner) readApproveLineCtx(ctx context.Context) (string, error) { func (o *stdinOwner) readVia(req chan lineReq, done <-chan struct{}) (string, error) { o.start() reply := make(chan lineResult, 1) - req <- lineReq{reply: reply, done: done} - res := <-reply - return res.line, res.err + // A reader that gives up returns: pickRecipient skips its request, so + // waiting on for a reply would hold this goroutine for the life of the + // process — one more for every approval that ran out. + select { + case req <- lineReq{reply: reply, done: done}: + case <-done: + return "", context.Canceled + } + select { + case res := <-reply: + return res.line, res.err + case <-done: + return "", context.Canceled + } } // logLevel is the log threshold: warnings and up by default (so retries/errors diff --git a/cmd/agent/tui.go b/cmd/agent/tui.go index d2bd51b..ee0ad97 100644 --- a/cmd/agent/tui.go +++ b/cmd/agent/tui.go @@ -658,6 +658,13 @@ func (m *tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case approvalMsg: req := approvalReq(msg) + if req.aborted != nil { + select { + case <-req.aborted: // asked after esc: the tool already has its no + return m, m.waitApproval() + default: + } + } m.pending = &req m.approveBy = time.Now().Add(approvalTimeout) // Do NOT fetch the next approval yet — that would overwrite m.pending and @@ -1372,7 +1379,8 @@ func (m *tuiModel) resolveApproval(ok bool) (tea.Model, tea.Cmd) { // answer it leaves a panel open over the prompt where it is, and teaches the // risk model nothing: nobody judged the call. func (m *tuiModel) expireApproval() (tea.Model, tea.Cmd) { - if m.state == stApprove { + modal := m.state == stApprove + if modal { m.state = m.preApprove } if m.pending.expire != nil { @@ -1380,8 +1388,12 @@ func (m *tuiModel) expireApproval() (tea.Model, tea.Cmd) { } m.pending = nil m.push(cDim.Render(" → ") + cErr.Render("denied") + cDim.Render(" · no answer in "+approvalWait()+" — the task goes on without it")) - if model, cmd, drained := m.finalizeTaskDoneAway(); drained { - return model, tea.Batch(cmd, m.waitApproval()) + // A task that finished behind the prompt is finalized now — but one that + // finished behind a panel waits for the panel to close, as it would have. + if modal { + if model, cmd, drained := m.finalizeTaskDoneAway(); drained { + return model, tea.Batch(cmd, m.waitApproval()) + } } return m, m.waitApproval() } From f53446815dfc45536ab69cd6442bcf35002e41f2 Mon Sep 17 00:00:00 2001 From: Roman Romenskyi Date: Sun, 18 Oct 2026 20:10:15 -0600 Subject: [PATCH 3/3] Approvals: an expired prompt finalizes the task behind it after esc too After esc dropped back to typing, a task that had finished behind the prompt was left unfinalized when the prompt ran out, and the view sat on "running" with nothing behind it. It is now finalized unless a panel is open over it. esc also denies the prompt it drops outright: one asked before the current task began holds an earlier abort signal that Abort never closes, and its tool waited forever. The panel's "approval waiting" line shows the countdown too. Co-Authored-By: Claude Opus 5.5 --- cmd/agent/approval_timeout_test.go | 33 ++++++++++++++++++++++++++++++ cmd/agent/tui.go | 19 ++++++++++------- 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/cmd/agent/approval_timeout_test.go b/cmd/agent/approval_timeout_test.go index 22db16d..613e004 100644 --- a/cmd/agent/approval_timeout_test.go +++ b/cmd/agent/approval_timeout_test.go @@ -96,6 +96,39 @@ func TestAnExpiredApprovalLeavesAPanelOpen(t *testing.T) { } } +// A task that finished behind the prompt is finalized when the prompt runs out, +// even after esc dropped back to typing: nothing else would, and the view sat +// on "running" with no task behind it. +func TestAnExpiredApprovalFinalizesTheTaskBehindIt(t *testing.T) { + m := &tuiModel{bridge: newBridge(), input: textarea.New(), state: stRunning, spin: spinner.New(), taskDoneAway: true} + got := askThroughTheUI(t, m) + m.handleKey(tea.KeyMsg{Type: tea.KeyEsc}) // back to typing; still pending + m.approveBy = time.Now().Add(-time.Second) + m.Update(spinner.TickMsg{}) + <-got + if m.state != stIdle || m.taskDoneAway { + t.Errorf("state = %v, taskDoneAway = %v; want the finished task finalized to idle", m.state, m.taskDoneAway) + } +} + +// esc denies the prompt it drops, whichever task asked it. One asked before the +// current task began holds an abort signal esc never closes. +func TestEscDeniesAPromptFromBeforeTheTask(t *testing.T) { + m := &tuiModel{bridge: newBridge(), input: textarea.New(), state: stIdle, spin: spinner.New()} + got := askThroughTheUI(t, m) // a background job asks while idle + m.bridge.arm() // the next task starts + m.state, m.cancel = stRunning, func() {} + m.handleKey(tea.KeyMsg{Type: tea.KeyEsc}) + select { + case r := <-got: + if r[0] { + t.Fatal("esc approved the call") + } + case <-time.After(3 * time.Second): + t.Fatal("esc dropped the prompt, but its tool is still waiting for an answer") + } +} + // A prompt asked after esc was denied tool-side the moment it was asked: it is // not shown, and the reader moves on to the next one. func TestAPromptAskedAfterEscIsNotShown(t *testing.T) { diff --git a/cmd/agent/tui.go b/cmd/agent/tui.go index ee0ad97..5d88fea 100644 --- a/cmd/agent/tui.go +++ b/cmd/agent/tui.go @@ -1215,7 +1215,12 @@ func (m *tuiModel) handleKey(k tea.KeyMsg) (tea.Model, tea.Cmd) { m.push(cDim.Render(" …cancelling (esc again to force-detach if it's stuck)")) if m.pending != nil { // The shown approval consumed the single reader; re-arm one so the - // next task's approvals are still read. + // next task's approvals are still read. Deny it outright: one asked + // before this task began holds an earlier abort signal, which + // Abort never closes. + if m.pending.expire != nil { + close(m.pending.expire) + } m.pending = nil return m, m.waitApproval() } @@ -1379,8 +1384,7 @@ func (m *tuiModel) resolveApproval(ok bool) (tea.Model, tea.Cmd) { // answer it leaves a panel open over the prompt where it is, and teaches the // risk model nothing: nobody judged the call. func (m *tuiModel) expireApproval() (tea.Model, tea.Cmd) { - modal := m.state == stApprove - if modal { + if m.state == stApprove { m.state = m.preApprove } if m.pending.expire != nil { @@ -1388,9 +1392,10 @@ func (m *tuiModel) expireApproval() (tea.Model, tea.Cmd) { } m.pending = nil m.push(cDim.Render(" → ") + cErr.Render("denied") + cDim.Render(" · no answer in "+approvalWait()+" — the task goes on without it")) - // A task that finished behind the prompt is finalized now — but one that - // finished behind a panel waits for the panel to close, as it would have. - if modal { + // A task that finished behind the prompt is finalized now — also when esc + // had dropped back to typing, or nothing ever would — but one that finished + // behind a panel waits for the panel to close, as it would have. + if m.state == stRunning || m.state == stIdle { if model, cmd, drained := m.finalizeTaskDoneAway(); drained { return model, tea.Batch(cmd, m.waitApproval()) } @@ -2225,7 +2230,7 @@ func (m *tuiModel) View() string { case m.state == stConfig: switch { // /config is the one panel that can sit over a live task case m.pending != nil: - status = m.attention("⚠ APPROVAL WAITING") + cDim.Render(" behind this panel — esc, then answer it") + status = m.attention("⚠ APPROVAL WAITING") + cDim.Render(" behind this panel — esc, then answer it") + m.approvalLeft() case m.cancel != nil: status = cDim.Render("settings — a task is running: tuning applies now, other changes when it ends · esc back to it") default: