Conversation
`whip up <prompt>` with no terminal to ask on (stdin piped AND /dev/tty unavailable — an editor spawning whip, a daemon) used to abort in checkTrust before the TUI opened, silently dropping the prompt. Now checkTrust returns a tri-state and defers: the TUI opens, parks the up prompt in heldPrompt, and asks the trust question inline via the namePrompt widget. Approve records trust and submits the held prompt as the first turn; decline drops it and quits. When stdin is piped but /dev/tty answers, the gate prompts there directly (git diff | whip up still asks). Adds namePrompt.onCancel (Esc = decline) and routes the answer back through Update as trustAnswerMsg (detached send; the namePrompt commit runs on the UI thread). Plan: .ai-docs/plans/whip-up-in-tui-trust/.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 loupe · go-review
🔴 1 · 5 files
Defers the startup trust gate into the TUI when no terminal exists, holding the up prompt until the inline y/n answer; /dev/tty fallback and the detached Send are sound, but the Init cmd that opens the prompt mutates model state from a cmd goroutine, racing the first WindowSizeMsg/View.
1 inline comment on the diff below.
…close Address loupe review on #103: - Data race (blocker): Init batched a cmd that called openTrustPrompt() directly, but tea.Batch runs cmds on their own goroutines — it mutated m.blocks/m.vp/m.input concurrently with the first WindowSizeMsg/View. Now Init emits a trustOpenMsg and Update (the UI thread) opens the prompt, matching the initialPromptMsg pattern. - CodeQL writable-file-close: the /dev/tty fallback handle's Close error was dropped. Extracted askTrust; the writable tty's close error is now surfaced (the prompt output could otherwise be lost). Test asserts Init does not mutate the model and that the open happens in Update. Race-clean over repeated runs.
There was a problem hiding this comment.
🔍 loupe · go-review
✅ no issues · 3 files
This PR moves the folder-trust gate into the TUI when no terminal exists, holding the up prompt until the inline answer. The hold only covers the initial-prompt kickoff — MCP server subprocesses from the untrusted folder's .mcp.json still spawn eagerly during startup before the user answers (a regression: the old code hard-failed in this case), and a due schedule on a resumed session can start a full agent turn while the gate is open.
Other notes (1)
- 🔴
internal/tui/tui.go:448— When checkTrust defers, startup continues unconditionally: this block reads /.mcp.json (mcp.LoadMergedFiltered) and mcpMgr.Start() eagerly exec-spawns every stdio server it declares (internal/mcp/manager.go:296→871) — before the in-TUI trust question is even rendered, and regardless of the answer. The default mcpImport policy imports .mcp.json (nil = on), and the setup wizard that would default it off is skipped exactly when stdin is non-TTY. Previously this no-terminal case hard-failed in checkTrust before any of this ran, so an untrusted folder gains arbitrary code execution here — the exact thing the gate exists to prevent. Fix: skip LoadMergedFiltered/Start while deferredTrustDir != "" and kick them from the trustAnswerMsg{approved:true} handler instead.
golangci-lint's nilerr flagged returning nil while terr != nil. A missing controlling terminal is the deliberate signal to defer the trust gate into the TUI, not an error to surface — annotate with //nolint:nilerr + reason (also satisfies nolintlint). golangci-lint v2 run: 0 issues.
There was a problem hiding this comment.
🔍 loupe · go-review
🔴 1 · 1 file
Defers the folder-trust gate into the TUI when no terminal exists: tri-state checkTrust with a /dev/tty fallback, held up prompt, and an inline namePrompt ask routed back via a detached Send. Integration is solid (no deadlock/leak, decline path is safe), but the in-TUI prompt's [y/N] label contradicts its Enter-approves behavior — one bare Enter both persists trust and launches the queued agent turn in an untrusted folder.
1 inline comment on the diff below.
Three fixes from the PR review:
- trust.go: the inline prompt's label says [y/N] but Enter approved
(a == "" counted as yes). One bare Enter persisted trust AND launched
the queued agent turn in an untrusted folder. Now Enter declines,
matching the label and failing closed; only y/yes approves.
- tui.go/mcp.go: when checkTrust deferred, startup kept reading the
untrusted folder's .mcp.json (LoadMergedFiltered) and mcpMgr.Start()
exec-spawned its stdio servers before the in-TUI question was even
rendered — arbitrary code execution, the exact thing the gate exists
to prevent (the pre-TUI gate hard-failed here). MCP discovery/start
is now skipped while trustPending and kicked from the
trustAnswerMsg{approved:true} handler via the extracted mcpStart.
- schedule_cmd.go: a due schedule on a resumed session could fire a
full agent turn while the gate was still open. fireDueSchedules now
holds while trustPending (unstamped, so tasks catch up after
approval; a decline quits anyway).
Tests: TestTrustPromptEnterDeclines (bare Enter declines via the real
key path), TestTrustApprovalStartsDeferredMCP (approval runs the
deferred MCP start), TestScheduleHeldWhileTrustPending (due task held
unstamped while the gate is open).
There was a problem hiding this comment.
🔍 loupe · go-review
🔴 1 · 🟡 2 · 6 files
PR defers the whip up folder-trust gate into the TUI when no terminal exists (tri-state checkTrust, /dev/tty fallback, held prompt, inline namePrompt gate, gated MCP/schedule startup). The mechanism is sound, but the gate is only consulted for Enter/Esc — the rest of the UI stays live while pending, and several reachable paths (palette MCP-import exec, palette Goal submit, startupReport skills scan, rename clobbering the prompt) read from or execute in the still-untrusted folder before the user answers, which the old hard-fail gate prevented.
Concerns
- 🔴 The deferred trust gate is bypassable while pending — the UI is not fenced off — While m.trustPending is set, only Enter/Esc are intercepted by the namePrompt; every other key path (Ctrl+P palette, Tab completion, $/@ menus) still runs and several of them read or exec the untrusted cwd before the answer. The fix is a single early guard in key()/submit/mcpSetImport gated on trustPending (see inline findings).
- 🟡 Trust-failure path is fail-open — If config.Trust(dir) fails in the trustAnswerMsg handler the error is only rendered, then MCP is started and the held prompt submitted in a folder not recorded as trusted — the terminal path (askTrust) aborts on the same failure.
1 inline comment on the diff below.
Other notes (4)
- 🔴
internal/tui/tui.go:2940— Ctrl+P (and KeyUp) open the command palette over the trust namePrompt — there is no namePrompt/trustPending guard here, and once m.palette != nil every key routes to paletteKey, so the trust prompt is visually replaced and can never be answered. From the palette: the MCPs panel's import toggle (palette.go:802 → mcpSetImport) reads and exec-spawns the untrusted .mcp.json stdio servers, and the Goal panel (palette.go:820 → commitGoal → m.submit) starts a full agent turn with tools in the untrusted cwd — exactly what the gate exists to prevent. Add an earlyif m.trustPending != ""guard in key() (allowing only the Enter/Esc namePrompt cases) or gate openPalette/submit/mcpSetImport. - 🟡
internal/tui/tui.go:507— startupReport() runs unconditionally while trustPending, and line 601 skills.ScanDetailed(skills.DefaultDirs()...) reads /.agents/skills/*/SKILL.md contents from the untrusted folder before the user answers — the pre-TUI gate aborted before any such read, so this is a regression the deferral introduces. Gate the skills scan on m.trustPending == "" and emit the report (or re-scan) from the approved trustAnswerMsg handler alongside mcpStart. - 🟡
internal/tui/fork.go:253— openNamePrompt overwrites m.namePrompt unconditionally, so while the trust gate is open the palette's "Rename session"/"Fork session" (reachable via Ctrl+P) silently replaces the trust prompt — its onOK/onCancel are dropped, m.trustPending stays set with no prompt to answer it, and the next ordinary Enter falls through to m.submit(text) in the untrusted folder. Refuse to clobber an existing namePrompt (or block the palette while trustPending). - 🟡
internal/tui/tui.go:2667— While the trust namePrompt owns the textarea, refreshMenu/completions still run on every keystroke: typing$triggers skillCands → skills.Scan of /.agents/skills (reads SKILL.md contents) and@triggers refreshFileIndex → filepath.WalkDir of the entire untrusted cwd tree (filename disclosure), all before the gate is answered. Short-circuit the menu/completion path while m.trustPending != "" (covered by the same early key() guard).
There was a problem hiding this comment.
🔍 loupe · go-review
✅ no issues · 2 files
Defers the folder-trust gate into the TUI when no terminal exists, parking the whip up prompt in heldPrompt and answering via the namePrompt widget; MCP startup is correctly held back until approval. The state machine is tight — one-shot guards, detached Send on the UI thread, fail-closed Enter, mcpStart's nil-guard prevents double-start — and I found no correctness or concurrency defects.
Highlights
- ✅ MCP exec/discovery correctly deferred until trust approval — closes the .mcp.json code-exec window the pre-TUI gate used to hard-fail on
2 participants
Problem
whip up <prompt>in a context with no terminal to ask on — stdin piped and/dev/ttyunavailable (an editor spawning whip, a daemon, some IDE integrations) — never showed the folder-trust prompt.checkTrustsaw a non-TTY stdin and errored,tui.Runaborted before the TUI opened, and theupprompt was silently dropped. From the user's view: the command did nothing.Fix
Ask the trust question inside the TUI when there's no terminal to ask on pre-launch, and hold the
upprompt until it's answered — then do the thing.checkTrustis now tri-state (trustGranted/trustDenied/trustDeferred)./dev/ttyanswers, the gate prompts on/dev/ttydirectly (git diff | whip upstill asks). It only defers when there's genuinely no terminal.upprompt inmodel.heldPrompt, and asks inline via the existingnamePromptwidget (the/fork//authpattern) — Enter commits, Esc cancels.config.Trustrecords the folder + the held prompt submits as the first turn. Decline (or Esc) → the prompt is dropped and the session exits.Trusted cwd or a real TTY: behavior unchanged.
Implementation notes
namePromptrather than the heavierpermDialogtool-gate modal (the trust ask is a simple UI-thread y/n).namePrompt.onCancelhook so Esc = decline (not a silent cancel).UpdateastrustAnswerMsgvia a detachedgo m.prog.Send(...)— the namePrompt commit runs on the UI thread, where a synchronous send would self-deadlock (satisfieswhipvet'suilock).Tests
internal/tui/up_test.go—TestDeferredTrustOpensPromptAndHolds,TestTrustApprovedSubmitsHeldPrompt,TestTrustDeclinedQuits.internal/tui/trust_test.go—TestTrustGate,TestTrustGateDefersWhenNoTerminal,TestTrustGateAsksOnDevTTY. Race-clean undergo test -race ./internal/tui.task checkgreen; the only failure is the pre-existinginternal/browserTestE2EDedicatedChrome tempdir flake (passes on retry, zero browser files touched).Plan:
.ai-docs/plans/whip-up-in-tui-trust/.