From 230b212f8e460c06b739decfcffc49dd16b4bc22 Mon Sep 17 00:00:00 2001 From: Roman Romenskyi Date: Mon, 19 Oct 2026 03:44:34 -0600 Subject: [PATCH 1/4] Approvals: the model is told a prompt ran out, not that the user said no A prompt that ran out came back as "denied by user", so the model took the user to be against the call: it dropped it, or worked around a no nobody gave. Now the call reports that nobody answered and the user may be away, and tells the model to go on without it and say at the end that it is still waiting on the user. tool.Ask/AskWith wrap an approval with a slot on ctx that the approver fills via tool.NoAnswer when its prompt expires (the TUI bridge, the plain prompt). Every asker uses it: run, file write/edit/mkdir, git, the risk check's own stop, sub-agent spawns, external agents, MCP launch and call. Co-Authored-By: Claude Opus 5.5 --- README.md | 6 ++- cmd/agent/approval_timeout_test.go | 65 ++++++++++++++++++++++++++++++ cmd/agent/bridge.go | 2 + cmd/agent/external_agent.go | 6 ++- cmd/agent/main.go | 20 ++++++--- cmd/agent/risk.go | 15 ++++--- internal/tool/ask_test.go | 64 +++++++++++++++++++++++++++++ internal/tool/file.go | 15 +++++-- internal/tool/git.go | 9 ++++- internal/tool/run.go | 5 ++- internal/tool/tool.go | 37 +++++++++++++++++ 11 files changed, 225 insertions(+), 19 deletions(-) create mode 100644 internal/tool/ask_test.go diff --git a/README.md b/README.md index 82a2ad3f..3265b8ff 100644 --- a/README.md +++ b/README.md @@ -473,8 +473,10 @@ 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. +anything. The model is told that nobody answered, not that you said no. It goes +on without the call and says at the end that the call is still waiting on you. +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 diff --git a/cmd/agent/approval_timeout_test.go b/cmd/agent/approval_timeout_test.go index 613e0044..10a0b622 100644 --- a/cmd/agent/approval_timeout_test.go +++ b/cmd/agent/approval_timeout_test.go @@ -11,6 +11,9 @@ import ( "github.com/charmbracelet/bubbles/spinner" "github.com/charmbracelet/bubbles/textarea" tea "github.com/charmbracelet/bubbletea" + + "github.com/ipsupport-llc/ipsupport-code/internal/config" + "github.com/ipsupport-llc/ipsupport-code/internal/tool" ) // askThroughTheUI starts an approval on the bridge and shows it on m, the way @@ -211,3 +214,65 @@ func TestAnUnansweredPlainPromptIsANo(t *testing.T) { t.Fatal("the line typed after the expired prompt never reached the REPL — the dead prompt took it") } } + +// silentApprover is a prompt that ran out: a no that nobody gave. +type silentApprover struct{} + +func (silentApprover) Approve(ctx context.Context, _, _ string) bool { + tool.NoAnswer(ctx) + return false +} + +// The model is told a prompt ran out, not that the user said no — through the +// TUI and the plain prompt alike — and a real no is still reported as one. +func TestTheModelIsToldNobodyAnswered(t *testing.T) { + m := &tuiModel{bridge: newBridge(), input: textarea.New(), state: stRunning, spin: spinner.New()} + ask := func() <-chan string { + got := make(chan string, 1) + go func() { _, gone := tool.Ask(context.Background(), m.bridge, "run", "make deploy"); got <- gone }() + m.Update(approvalMsg(<-m.bridge.approvals)) + return got + } + got := ask() + m.approveBy = time.Now().Add(-time.Second) + m.Update(spinner.TickMsg{}) + if gone := <-got; !strings.Contains(gone, "nobody answered") { + t.Errorf("TUI timeout told the model %q, want that nobody answered", gone) + } + got = ask() + m.handleKey(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'n'}}) + if gone := <-got; gone != "" { + t.Errorf("an explicit no came back as %q, want the plain refusal", gone) + } + + 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))} + if _, gone := tool.Ask(context.Background(), s, "run", "make deploy"); !strings.Contains(gone, "nobody answered") { + t.Errorf("plain timeout told the model %q, want that nobody answered", gone) + } +} + +// The calls asked about outside the tools say it too: the risk check's own stop, +// a sub-agent spawn and an external agent. +func TestEveryAskerSaysNobodyAnswered(t *testing.T) { + a, pol, _ := gateApp(t, "run.default=allow") + a.approver = silentApprover{} + ctx := scored(t, a, pol, flaggedCommand) + mustFlag(t, ctx) + if msg := a.riskGate(pol)(ctx, "run", "shell", map[string]any{"command": flaggedCommand}); !strings.Contains(msg, "nobody answered") { + t.Errorf("risk check: %q", msg) + } + + b := &app{cfg: config.Default(), workspace: t.TempDir(), approver: silentApprover{}} + plan := spawnPlan{profile: "p", llmCfg: config.LLM{Model: "m"}, subWorkspace: b.workspace, spawnDefault: "ask"} + if _, err := b.runSpawnPlan(context.Background(), plan, "a task", nil); err == nil || !strings.Contains(err.Error(), "nobody answered") { + t.Errorf("spawn: %v", err) + } + p := config.AgentProfile{Kind: "external", Command: "codex", Args: []string{"exec", "{task}"}} + if _, err := b.spawnExternalAgent(context.Background(), "codex", p, "a task", b.workspace, nil, nil); err == nil || !strings.Contains(err.Error(), "nobody answered") { + t.Errorf("external agent: %v", err) + } +} diff --git a/cmd/agent/bridge.go b/cmd/agent/bridge.go index bc7e43b1..ebcb5418 100644 --- a/cmd/agent/bridge.go +++ b/cmd/agent/bridge.go @@ -7,6 +7,7 @@ import ( "time" "github.com/ipsupport-llc/ipsupport-code/internal/risk" + "github.com/ipsupport-llc/ipsupport-code/internal/tool" ) // uiBridge is the seam between the agent (running in a background goroutine) and @@ -107,6 +108,7 @@ func (b *uiBridge) ApproveAnswered(ctx context.Context, kind, detail string) (ok case ok := <-reply: return ok, true case <-expire: + tool.NoAnswer(ctx) return false, false case <-abort: return false, false diff --git a/cmd/agent/external_agent.go b/cmd/agent/external_agent.go index f9174d6f..9779f38a 100644 --- a/cmd/agent/external_agent.go +++ b/cmd/agent/external_agent.go @@ -13,6 +13,7 @@ import ( "github.com/ipsupport-llc/ipsupport-code/internal/config" "github.com/ipsupport-llc/ipsupport-code/internal/procgroup" "github.com/ipsupport-llc/ipsupport-code/internal/textutil" + "github.com/ipsupport-llc/ipsupport-code/internal/tool" "github.com/ipsupport-llc/ipsupport-code/internal/trace" ) @@ -97,7 +98,10 @@ func (a *app) spawnExternalAgent(ctx context.Context, profile string, p config.A // with its own safety into one without (found by review: the prompt showed // "codex" and the task, never "exec --yolo"). launch := strings.TrimSpace(command + " " + strings.Join(p.Args, " ")) - approved := a.approveGated(a.withLaunchScore(ctx, root, launch), "external agent", head+"\n launch: "+launch+"\n task: "+task) + approved, gone := tool.AskWith(a.withLaunchScore(ctx, root, launch), "external agent", head+"\n launch: "+launch+"\n task: "+task, a.approveGated) + if gone != "" { + return "", errors.New(gone) + } if !approved { return "", fmt.Errorf("external agent denied by user") } diff --git a/cmd/agent/main.go b/cmd/agent/main.go index c6b2f11e..b6e20836 100644 --- a/cmd/agent/main.go +++ b/cmd/agent/main.go @@ -1068,7 +1068,10 @@ func (a *app) runSpawnPlan(ctx context.Context, plan spawnPlan, task string, onL if plan.outsideJail { kind, where = "agent outside workspace", where+" (outside the workspace)" } - approved := a.approveGated(ctx, kind, fmt.Sprintf("%s · %s · %s\n task: %s", plan.profile, plan.llmCfg.Model, where, task)) + approved, gone := tool.AskWith(ctx, kind, fmt.Sprintf("%s · %s · %s\n task: %s", plan.profile, plan.llmCfg.Model, where, task), a.approveGated) + if gone != "" { + return "", errors.New(gone) + } if !approved { return "", fmt.Errorf("spawn denied by user") } @@ -2757,7 +2760,10 @@ func (a *app) mcpClient(ctx context.Context, servers map[string]mcp.Server, name // TestMCPClientLockNotHeldDuringApproval. attemptCtx (rather than ctx) // carries the wait so closeMCP can cut it short on shutdown instead of // leaving the eventual connection an orphan with nothing left to close it. - approved := a.approveGated(a.withLaunchScore(attemptCtx, "", launch), "mcp launch", detail) + approved, gone := tool.AskWith(a.withLaunchScore(attemptCtx, "", launch), "mcp launch", detail, a.approveGated) + if gone != "" { + return nil, errors.New(gone) + } if !approved { return nil, fmt.Errorf("mcp server %q launch denied by user", name) } @@ -2921,18 +2927,21 @@ func mcpCallDetail(server, tool string, args map[string]any) string { // mcpCall runs an MCP tool, asking approval first (it's external code that can do // anything). -func (a *app) mcpCall(ctx context.Context, server, tool string, args map[string]any) (string, error) { +func (a *app) mcpCall(ctx context.Context, server, name string, args map[string]any) (string, error) { c, err := a.mcpClient(ctx, a.cfg.McpServers, server) if err != nil { return "", err } - approved := a.approveGated(ctx, "mcp call", mcpCallDetail(server, tool, args)) + approved, gone := tool.AskWith(ctx, "mcp call", mcpCallDetail(server, name, args), a.approveGated) + if gone != "" { + return "", errors.New(gone) + } if !approved { return "", fmt.Errorf("mcp call denied by user") } cctx, cancel := context.WithTimeout(ctx, 60*time.Second) defer cancel() - return c.Call(cctx, tool, args) + return c.Call(cctx, name, args) } // maybeDetectWindowSync detects the window inline for the REPL/one-shot paths; @@ -6409,6 +6418,7 @@ func (s *stdinApprover) ApproveAnswered(ctx context.Context, kind, detail string 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()) + tool.NoAnswer(ctx) } return false, false } diff --git a/cmd/agent/risk.go b/cmd/agent/risk.go index aa727ff9..0fe9142e 100644 --- a/cmd/agent/risk.go +++ b/cmd/agent/risk.go @@ -11,6 +11,7 @@ import ( "github.com/ipsupport-llc/ipsupport-code/internal/policy" "github.com/ipsupport-llc/ipsupport-code/internal/risk" "github.com/ipsupport-llc/ipsupport-code/internal/shellsplit" + "github.com/ipsupport-llc/ipsupport-code/internal/tool" ) // EnvRiskOff turns risk scoring off for a run, the check included. risk_check @@ -114,9 +115,9 @@ func (a *app) riskStops(ctx context.Context, kind string) bool { // and file (see policyVerdict). A call the policy asks about gets its prompt // from the tool, with the score on it, and one it refuses never runs. func (a *app) riskGate(pol *policy.Engine) func(ctx context.Context, tool, action string, params map[string]any) string { - return func(ctx context.Context, tool, action string, params map[string]any) string { - kind, detail := tool, "" - switch tool { + return func(ctx context.Context, name, action string, params map[string]any) string { + kind, detail := name, "" + switch name { case "run": detail, _ = params["command"].(string) case "file": @@ -125,12 +126,16 @@ func (a *app) riskGate(pol *policy.Engine) func(ctx context.Context, tool, actio default: return "" } - if !a.riskStops(ctx, kind) || policyVerdict(pol, tool, action, params) != risk.VerdictAllow { + if !a.riskStops(ctx, kind) || policyVerdict(pol, name, action, params) != risk.VerdictAllow { return "" } - if a.approveGated(ctx, kind, detail) { + ok, gone := tool.AskWith(ctx, kind, detail, a.approveGated) + if ok { return "" } + if gone != "" { + return gone + } return fmt.Sprintf("%s %s denied by user — the risk check flagged it: %s", kind, detail, riskNote(ctx)) } } diff --git a/internal/tool/ask_test.go b/internal/tool/ask_test.go new file mode 100644 index 00000000..58ee225f --- /dev/null +++ b/internal/tool/ask_test.go @@ -0,0 +1,64 @@ +package tool + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" +) + +// silent is an Approver whose prompt ran out: it says no, and that nobody said it. +type silent struct{} + +func (silent) Approve(ctx context.Context, _, _ string) bool { NoAnswer(ctx); return false } + +// A prompt that ran out used to read "denied by user", so the model took the +// user to be against the call. Every tool that asks now says what happened: +// nobody answered, and the model should go on without it. +func TestAnUnansweredPromptIsNotReportedAsTheUsersNo(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "f.txt"), []byte("old\n"), 0o644); err != nil { + t.Fatal(err) + } + repo := initRepo(t) + if err := os.WriteFile(filepath.Join(repo, "a.txt"), []byte("a\n"), 0o644); err != nil { + t.Fatal(err) + } + for _, tc := range []struct { + name string + tl Tool + action string + params map[string]any + }{ + {"run", runToolFor(t, dir, "ask", silent{}, nil), "shell", map[string]any{"command": "echo hi"}}, + {"write", fileToolFor(t, dir, "ask", silent{}), "write", map[string]any{"path": "x.txt", "content": "x"}}, + {"edit", fileToolFor(t, dir, "ask", silent{}), "edit", map[string]any{"path": "f.txt", "find": "old", "replace": "new"}}, + {"mkdir", fileToolFor(t, dir, "ask", silent{}), "mkdir", map[string]any{"path": "sub"}}, + {"git", gitToolFor(t, repo, silent{}), "add", map[string]any{"paths": "a.txt"}}, + } { + t.Run(tc.name, func(t *testing.T) { + r := tc.tl.Call(ctx, tc.action, tc.params) + if !r.IsError || !strings.Contains(r.Content, "nobody answered") || strings.Contains(r.Content, "denied by user") { + t.Errorf("result = %+v, want it to say nobody answered — not that the user denied it", r) + } + }) + } +} + +// NoAnswer marks only a no, and only under an Ask. +func TestNoAnswerMarksOnlyANo(t *testing.T) { + saidYes := approverFunc(func(_, _ string) bool { return true }) + ok, gone := AskWith(context.Background(), "run", "ls", func(ctx context.Context, kind, detail string) bool { + NoAnswer(ctx) + return saidYes.Approve(ctx, kind, detail) + }) + if !ok || gone != "" { + t.Errorf("Ask = (%v, %q), want a yes to stay a yes", ok, gone) + } + if ok, gone := Ask(context.Background(), no(), "run", "ls"); ok || gone != "" { + t.Errorf("Ask = (%v, %q), want a plain refusal with nothing to add", ok, gone) + } + NoAnswer(context.Background()) // outside an Ask: nothing to mark, and no panic +} diff --git a/internal/tool/file.go b/internal/tool/file.go index 711034cb..7a2f9517 100644 --- a/internal/tool/file.go +++ b/internal/tool/file.go @@ -391,7 +391,10 @@ func (f *fileTool) writeFile(ctx context.Context, action string, a Args, appendM case policy.Deny: return Err(action + " " + path + " denied by workspace policy") case policy.Ask: - if !f.ap.Approve(ctx, action, path) { + if ok, gone := Ask(ctx, f.ap, action, path); !ok { + if gone != "" { + return Err(gone) + } return Err(action + " " + path + " denied by user") } } @@ -529,7 +532,10 @@ func (f *fileTool) edit(ctx context.Context, a Args) Result { case policy.Deny: return Err("edit " + path + " denied by workspace policy") case policy.Ask: - if !f.ap.Approve(ctx, "edit", path) { + if ok, gone := Ask(ctx, f.ap, "edit", path); !ok { + if gone != "" { + return Err(gone) + } return Err("edit " + path + " denied by user") } } @@ -668,7 +674,10 @@ func (f *fileTool) mkdir(ctx context.Context, a Args) Result { case policy.Deny: return Err("mkdir " + path + " denied by workspace policy") case policy.Ask: - if !f.ap.Approve(ctx, "mkdir", path) { + if ok, gone := Ask(ctx, f.ap, "mkdir", path); !ok { + if gone != "" { + return Err(gone) + } return Err("mkdir " + path + " denied by user") } } diff --git a/internal/tool/git.go b/internal/tool/git.go index e201474b..6a12f66b 100644 --- a/internal/tool/git.go +++ b/internal/tool/git.go @@ -622,8 +622,13 @@ func (g *gitTool) run(ctx context.Context, action string, mutating bool, args .. // part of the approval text — what the user is asked to approve is the command // they would have typed, not our hardening. func (g *gitTool) runWith(ctx context.Context, action string, mutating bool, timeout time.Duration, extraEnv, gitFlags []string, note string, args ...string) Result { - if mutating && !g.ap.Approve(ctx, "git", "git "+strings.Join(args, " ")+note) { - return Err("git " + action + " denied by user") + if mutating { + if ok, gone := Ask(ctx, g.ap, "git", "git "+strings.Join(args, " ")+note); !ok { + if gone != "" { + return Err(gone) + } + return Err("git " + action + " denied by user") + } } dir, err := g.pol.Resolve(".") if err != nil { diff --git a/internal/tool/run.go b/internal/tool/run.go index 4ac9ca1b..0d89e294 100644 --- a/internal/tool/run.go +++ b/internal/tool/run.go @@ -98,7 +98,10 @@ func (r *runTool) shell(ctx context.Context, a Args) Result { case policy.Deny: return Err("command denied by workspace policy: " + command) case policy.Ask: - if !r.ap.Approve(ctx, "run", command) { + if ok, gone := Ask(ctx, r.ap, "run", command); !ok { + if gone != "" { + return Err(gone) + } return Err("command denied by user: " + command) } } diff --git a/internal/tool/tool.go b/internal/tool/tool.go index 85456eb5..125de33d 100644 --- a/internal/tool/tool.go +++ b/internal/tool/tool.go @@ -9,6 +9,7 @@ import ( "fmt" "strconv" "strings" + "sync/atomic" ) // Result is a tool's outcome as the model sees it. Content is always model- @@ -58,6 +59,42 @@ type Approver interface { Approve(ctx context.Context, kind, detail string) bool } +type noAnswerKey struct{} + +// NoAnswer is how an Approver marks the no it is about to return as nobody's: +// the prompt ran out unanswered. A no under an Ask is otherwise reported as +// the user's. +func NoAnswer(ctx context.Context) { + if gone, ok := ctx.Value(noAnswerKey{}).(*atomic.Bool); ok { + gone.Store(true) + } +} + +// Ask is ap.Approve, plus — when the no was nobody's answer — what the call +// reports instead of "denied by user", or "" when someone did answer. +func Ask(ctx context.Context, ap Approver, kind, detail string) (ok bool, unanswered string) { + return AskWith(ctx, kind, detail, ap.Approve) +} + +// AskWith is Ask for an approval asked some other way. +// +// A refusal and a prompt that ran out read alike to the model as "denied by +// user", so it took the user to be against the call — dropping it, or working +// around a no nobody gave. What it needs instead is that the user is away: go on +// without it, and leave it for them. +func AskWith(ctx context.Context, kind, detail string, approve func(ctx context.Context, kind, detail string) bool) (ok bool, unanswered string) { + var gone atomic.Bool + if approve(context.WithValue(ctx, noAnswerKey{}, &gone), kind, detail) { + return true, "" + } + if !gone.Load() { + return false, "" + } + what, _, _ := strings.Cut(detail, "\n") + return false, kind + " " + what + " was not run: nobody answered the approval prompt in time — the user may be away. " + + "Don't ask for it again now; carry on with whatever doesn't need it, and when you finish, say it is still waiting on the user." +} + // Tool is one fat domain tool. type Tool interface { Name() string From 940418b28ff9f0580e738b98e86b665587a9ca2a Mon Sep 17 00:00:00 2001 From: Roman Romenskyi Date: Mon, 19 Oct 2026 03:49:43 -0600 Subject: [PATCH 2/4] Approvals: review fixes for the unanswered note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The plain prompt marks a timeout whichever select branch sees it first: the read gives up with the deadline, and when its error won the race the model was still told "denied by user". A closed stdin is marked too — nobody can answer there either. A cancelled task is not. - The risk check's own stop keeps its flag in the note, so the model knows the call left waiting is the one it flagged. - git's note no longer reads "git git …". Co-Authored-By: Claude Opus 5.5 --- cmd/agent/approval_timeout_test.go | 14 +++++++++++++- cmd/agent/main.go | 13 +++++++++---- cmd/agent/risk.go | 2 +- internal/tool/ask_test.go | 3 +++ internal/tool/tool.go | 5 ++++- 5 files changed, 30 insertions(+), 7 deletions(-) diff --git a/cmd/agent/approval_timeout_test.go b/cmd/agent/approval_timeout_test.go index 10a0b622..f4c636d0 100644 --- a/cmd/agent/approval_timeout_test.go +++ b/cmd/agent/approval_timeout_test.go @@ -253,6 +253,18 @@ func TestTheModelIsToldNobodyAnswered(t *testing.T) { if _, gone := tool.Ask(context.Background(), s, "run", "make deploy"); !strings.Contains(gone, "nobody answered") { t.Errorf("plain timeout told the model %q, want that nobody answered", gone) } + + // A closed stdin: nobody can answer at all. + closed := &stdinApprover{stdin: newStdinOwner(bufio.NewReader(strings.NewReader("")))} + if _, gone := tool.Ask(context.Background(), closed, "run", "make deploy"); !strings.Contains(gone, "nobody answered") { + t.Errorf("closed stdin told the model %q, want that nobody answered", gone) + } + // A cancelled task is not told anyone was away. + cctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, gone := tool.Ask(cctx, closed, "run", "make deploy"); gone != "" { + t.Errorf("a cancelled task was told %q, want nothing", gone) + } } // The calls asked about outside the tools say it too: the risk check's own stop, @@ -262,7 +274,7 @@ func TestEveryAskerSaysNobodyAnswered(t *testing.T) { a.approver = silentApprover{} ctx := scored(t, a, pol, flaggedCommand) mustFlag(t, ctx) - if msg := a.riskGate(pol)(ctx, "run", "shell", map[string]any{"command": flaggedCommand}); !strings.Contains(msg, "nobody answered") { + if msg := a.riskGate(pol)(ctx, "run", "shell", map[string]any{"command": flaggedCommand}); !strings.Contains(msg, "nobody answered") || !strings.Contains(msg, "risk check had flagged it") { t.Errorf("risk check: %q", msg) } diff --git a/cmd/agent/main.go b/cmd/agent/main.go index b6e20836..b14ab9d9 100644 --- a/cmd/agent/main.go +++ b/cmd/agent/main.go @@ -6416,15 +6416,20 @@ func (s *stdinApprover) ApproveAnswered(ctx context.Context, kind, detail string select { case res = <-result: case <-wait.Done(): + res.err = wait.Err() + } + // No line: the deadline passed — the read gives up with it, so this is + // reached by either branch — or stdin is closed and nobody can answer at + // all. A cancelled task is neither, and is told nothing. + if res.err != nil { if ctx.Err() == nil { - fmt.Fprintf(os.Stderr, "\n no answer in %s — denied; the task goes on without it\n", approvalWait()) + if wait.Err() != nil { + fmt.Fprintf(os.Stderr, "\n no answer in %s — denied; the task goes on without it\n", approvalWait()) + } tool.NoAnswer(ctx) } return false, false } - if res.err != nil { - return false, false - } switch strings.TrimSpace(strings.ToLower(res.line)) { case "y", "yes": return true, true diff --git a/cmd/agent/risk.go b/cmd/agent/risk.go index 0fe9142e..11b98a54 100644 --- a/cmd/agent/risk.go +++ b/cmd/agent/risk.go @@ -134,7 +134,7 @@ func (a *app) riskGate(pol *policy.Engine) func(ctx context.Context, tool, actio return "" } if gone != "" { - return gone + return gone + " (The risk check had flagged it: " + riskNote(ctx) + ".)" } return fmt.Sprintf("%s %s denied by user — the risk check flagged it: %s", kind, detail, riskNote(ctx)) } diff --git a/internal/tool/ask_test.go b/internal/tool/ask_test.go index 58ee225f..db08d01e 100644 --- a/internal/tool/ask_test.go +++ b/internal/tool/ask_test.go @@ -43,6 +43,9 @@ func TestAnUnansweredPromptIsNotReportedAsTheUsersNo(t *testing.T) { if !r.IsError || !strings.Contains(r.Content, "nobody answered") || strings.Contains(r.Content, "denied by user") { t.Errorf("result = %+v, want it to say nobody answered — not that the user denied it", r) } + if strings.Contains(r.Content, "git git") { + t.Errorf("result = %q names the call twice", r.Content) + } }) } } diff --git a/internal/tool/tool.go b/internal/tool/tool.go index 125de33d..ac2dc1cb 100644 --- a/internal/tool/tool.go +++ b/internal/tool/tool.go @@ -91,7 +91,10 @@ func AskWith(ctx context.Context, kind, detail string, approve func(ctx context. return false, "" } what, _, _ := strings.Cut(detail, "\n") - return false, kind + " " + what + " was not run: nobody answered the approval prompt in time — the user may be away. " + + if !strings.HasPrefix(what, kind+" ") { // git's detail already opens "git …" + what = kind + " " + what + } + return false, what + " was not run: nobody answered the approval prompt in time — the user may be away. " + "Don't ask for it again now; carry on with whatever doesn't need it, and when you finish, say it is still waiting on the user." } From 81dd10691253db121a5ad58bd26df0535e969c11 Mon Sep 17 00:00:00 2001 From: Roman Romenskyi Date: Mon, 19 Oct 2026 03:55:12 -0600 Subject: [PATCH 3/4] Approvals: a prompt esc drops is not "nobody answered" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit esc closed the prompt's expire channel to deny it, which is also the timeout's — so a background job whose prompt esc swept away was told the user may be away, while the user had just pressed a key. esc now closes its own dismiss channel: still a no, without the claim. Co-Authored-By: Claude Opus 5.5 --- cmd/agent/approval_timeout_test.go | 10 ++++++++++ cmd/agent/bridge.go | 9 +++++++-- cmd/agent/tui.go | 4 ++-- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/cmd/agent/approval_timeout_test.go b/cmd/agent/approval_timeout_test.go index f4c636d0..4f739e25 100644 --- a/cmd/agent/approval_timeout_test.go +++ b/cmd/agent/approval_timeout_test.go @@ -244,6 +244,16 @@ func TestTheModelIsToldNobodyAnswered(t *testing.T) { if gone := <-got; gone != "" { t.Errorf("an explicit no came back as %q, want the plain refusal", gone) } + // esc is someone there, not someone away — even for a prompt asked before + // the task, which esc denies by closing its own channel, not the abort. + got = ask() + m.bridge.arm() + m.state, m.cancel = stRunning, func() {} + m.handleKey(tea.KeyMsg{Type: tea.KeyEsc}) + if gone := <-got; gone != "" { + t.Errorf("a prompt esc dropped told the model %q, want no claim the user is away", gone) + } + m.cancel, m.taskCancelled = nil, false defer func(d time.Duration) { approvalTimeout = d }(approvalTimeout) approvalTimeout = 50 * time.Millisecond diff --git a/cmd/agent/bridge.go b/cmd/agent/bridge.go index ebcb5418..09c18098 100644 --- a/cmd/agent/bridge.go +++ b/cmd/agent/bridge.go @@ -40,6 +40,9 @@ 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{} + // dismiss is closed when esc drops the prompt: denied too, but someone was + // there, so the tool is not told the user is away. + dismiss 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 @@ -96,9 +99,9 @@ func (b *uiBridge) ApproveAnswered(ctx context.Context, kind, detail string) (ok abort := b.abort b.mu.Unlock() - reply, expire := make(chan bool, 1), make(chan struct{}) + reply, expire, dismiss := make(chan bool, 1), make(chan struct{}), make(chan struct{}) select { - case b.approvals <- approvalReq{kind: kind, detail: detail, risk: riskNote(ctx), reply: reply, expire: expire, aborted: abort}: + case b.approvals <- approvalReq{kind: kind, detail: detail, risk: riskNote(ctx), reply: reply, expire: expire, dismiss: dismiss, aborted: abort}: case <-abort: return false, false case <-ctx.Done(): @@ -110,6 +113,8 @@ func (b *uiBridge) ApproveAnswered(ctx context.Context, kind, detail string) (ok case <-expire: tool.NoAnswer(ctx) return false, false + case <-dismiss: + return false, false case <-abort: return false, false case <-ctx.Done(): diff --git a/cmd/agent/tui.go b/cmd/agent/tui.go index 5d88fea3..5a6ae5f1 100644 --- a/cmd/agent/tui.go +++ b/cmd/agent/tui.go @@ -1218,8 +1218,8 @@ func (m *tuiModel) handleKey(k tea.KeyMsg) (tea.Model, tea.Cmd) { // 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) + if m.pending.dismiss != nil { + close(m.pending.dismiss) } m.pending = nil return m, m.waitApproval() From c0da69a0def94c884532efdf7b02293e61343d3c Mon Sep 17 00:00:00 2001 From: Roman Romenskyi Date: Mon, 19 Oct 2026 03:58:37 -0600 Subject: [PATCH 4/4] Approvals: an answer typed before stdin closed is not "nobody answered" ReadString hands back the last line with io.EOF when it has no newline; an "n" typed that way was marked as no answer at all. Co-Authored-By: Claude Opus 5.5 --- cmd/agent/approval_timeout_test.go | 5 +++++ cmd/agent/main.go | 5 +++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/cmd/agent/approval_timeout_test.go b/cmd/agent/approval_timeout_test.go index 4f739e25..8dc483ca 100644 --- a/cmd/agent/approval_timeout_test.go +++ b/cmd/agent/approval_timeout_test.go @@ -269,6 +269,11 @@ func TestTheModelIsToldNobodyAnswered(t *testing.T) { if _, gone := tool.Ask(context.Background(), closed, "run", "make deploy"); !strings.Contains(gone, "nobody answered") { t.Errorf("closed stdin told the model %q, want that nobody answered", gone) } + // Nor is a refusal typed without its newline before stdin closed. + unterminated := &stdinApprover{stdin: newStdinOwner(bufio.NewReader(strings.NewReader("n")))} + if _, gone := tool.Ask(context.Background(), unterminated, "run", "make deploy"); gone != "" { + t.Errorf("a refusal before EOF told the model %q, want no claim nobody answered", gone) + } // A cancelled task is not told anyone was away. cctx, cancel := context.WithCancel(context.Background()) cancel() diff --git a/cmd/agent/main.go b/cmd/agent/main.go index b14ab9d9..8923585e 100644 --- a/cmd/agent/main.go +++ b/cmd/agent/main.go @@ -6420,9 +6420,10 @@ func (s *stdinApprover) ApproveAnswered(ctx context.Context, kind, detail string } // No line: the deadline passed — the read gives up with it, so this is // reached by either branch — or stdin is closed and nobody can answer at - // all. A cancelled task is neither, and is told nothing. + // all. A cancelled task is neither, and is told nothing; nor is a last + // line typed without its newline ("n" then EOF), which is someone's answer. if res.err != nil { - if ctx.Err() == nil { + if ctx.Err() == nil && strings.TrimSpace(res.line) == "" { if wait.Err() != nil { fmt.Fprintf(os.Stderr, "\n no answer in %s — denied; the task goes on without it\n", approvalWait()) }