wavecli: Fix refresh consent gate misdetecting non-TTY stdin - #1005
Conversation
In this commit, we fix the refresh consent gate treating /dev/null and other non-terminal character devices as an interactive terminal. The gate's defaultStdinIsTTY reported any character device as a TTY, but /dev/null and a closed descriptor -- the most common non-interactive stdin shapes for agents, CI, and systemd units -- are character devices that are not terminals. On those, the gate printed a y/N prompt that immediately read EOF and failed with EXECUTION_FAILED, the exact hang-then-fail #986 set out to prevent; only a pipe stdin was refused cleanly with the actionable INVALID_ARGS message. We now detect a real terminal with term.IsTerminal instead of the character-device heuristic, so every non-terminal stdin refuses with the message directing the caller to --yes or --dry_run. The custom-reader path (embedded harnesses that drive the prompt themselves) is unchanged. defaultStdinIsTTY had no direct test -- the suite stubs the stdinIsTTY indirection -- which is how the heuristic shipped; a regression test now exercises the real function against /dev/null, a pipe, and a closed descriptor. The same detection backs the leave --all, recovery escalate, and send confirmations, so all four gates are fixed.
Roasbeef
added
the
backport-v0.1.x-branch
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the character-device check for os.Stdin with a proper terminal check using term.IsTerminal to prevent non-interactive inputs (like /dev/null or closed descriptors) from being incorrectly treated as interactive terminals. Additionally, a comprehensive suite of unit tests has been added to verify this behavior under various conditions. I have no feedback to provide as the changes are well-implemented and include thorough test coverage.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Successfully created backport PR for |
…ranch [v0.1.x-branch] Backport #1005: wavecli: Fix refresh consent gate misdetecting non-TTY stdin
1 participant
In this PR, we fix a bug found while manually testing #987 on the arktest
regtest harness: the refresh consent gate treats
/dev/nulland othernon-terminal character devices as an interactive terminal, so a real
refresh on that stdin prints a
y/Nprompt and then dies on EOF insteadof refusing cleanly.
The bug
#987 gates a real
ark vtxos refreshon explicit consent: an interactiveTTY prompts,
--yesskips the prompt for scripted use, andnon-interactive stdin must refuse with an actionable
INVALID_ARGSerrordirecting the caller to
--yesor--dry_run, so an agent is neverblocked on a
y/Nit cannot answer. The refusal is driven bydefaultStdinIsTTY, which reported any character device as a terminal.But
/dev/nulland a closed descriptor -- the most commonnon-interactive stdin shapes for agents, CI, and systemd units -- are
character devices that are not terminals. On those, the gate printed the
prompt and then failed with
read confirmation: EOF(EXECUTION_FAILED,exit 1), the exact hang-then-fail the gate exists to prevent. Only a pipe
(
echo | wavecli) was refused cleanly.Observed on the live daemon:
INVALID_ARGS, exit 2INVALID_ARGS, exit 2/dev/nullINVALID_ARGS, exit 20<&-)INVALID_ARGS, exit 2The fix
We detect a real terminal with
term.IsTerminalinstead of thecharacter-device heuristic. Every non-terminal stdin now refuses with the
actionable message; the custom-reader path (embedded harnesses that drive
the prompt themselves) is unchanged. The same detector backs the
leave --all,recovery escalate, andsendconfirmations, so all fourconsent gates are fixed at once.
defaultStdinIsTTYhad no direct test -- the suite stubs thestdinIsTTYindirection, which is how the heuristic shipped. Aregression test now exercises the real function against
/dev/null, apipe, and a closed descriptor (and fails on the old char-device check).
Validation
go test -race -run TestDefaultStdinIsTTY ./cmd/wavecli/waveclicommands/-racemake fmt-changed-check,make lint-changed-local(0 issues),make commitmsg-lintFound while manually testing #987.