install-hooks: prompt for author via the controlling terminal - #2231
Open
svarlamov-git-ai wants to merge 4 commits into
Open
install-hooks: prompt for author via the controlling terminal#2231svarlamov-git-ai wants to merge 4 commits into
svarlamov-git-ai wants to merge 4 commits into
Conversation
svarlamov-git-ai
marked this pull request as ready for review
August 27, 2026 01:20
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
The recommended installs pipe stdio — `curl install.sh | bash` makes stdin the script pipe, and install.ps1 pipes stdout through Out-Host — so the author prompt's stdin/stdout TTY gate never passed there. Follow git's credential-prompt pattern: when stdio is redirected but the process has a controlling terminal, prompt over /dev/tty (Unix) or CONIN$/CONOUT$ (Windows) instead. Because a controlling terminal can be openable in non-interactive flows, the prompt now hard-skips for daemon-triggered silent upgrades (GIT_AI_DAEMON_UPGRADE), the background upgrade worker (GIT_AI_BACKGROUND_UPGRADE_WORKER, which runs in the user's session and does not set the daemon-upgrade env), and background AI-agent sandboxes. Tests: suppression coverage for all three guards; a Linux end-to-end test that drives the prompt through a real pty via script(1) with stdin redirected to /dev/null; and a detach helper (setsid / DETACHED_PROCESS) so skip-assertions hold even when the test runner itself has a terminal. Test children default to GIT_AI_NO_AUTHOR_PROMPT=1 so no unrelated test can ever reach a developer's terminal; prompt tests opt back in with =0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The controlling-terminal fallback made the prompt reachable from flows where a terminal is openable but nobody can answer: - Windows `git-ai upgrade` runs the installer on a hidden CREATE_NO_WINDOW console where CONIN$ opens but is invisible. - `install-hooks --wsl` nests installs in each distro on the same hidden console's pty. - The macOS pkg postinstall keeps the controlling tty when run from a terminal (`sudo installer -pkg`). - install.sh in CI/MDM/containers can have a pty (pty-backed CI steps, `ssh -tt`); reuse its existing CI/MDM detection to suppress. - Backgrounded installs (`git-ai install-hooks &`, nohup) would be stopped by SIGTTIN on the first terminal read, freezing even the timeout; skip when not in the terminal's foreground process group. All now set/honor GIT_AI_NO_AUTHOR_PROMPT (name extracted to a pub(crate) const). Also: gates reordered so the terminal is only opened after the cheap config gates pass (removing the unreachable match arm and the now-tautological should_prompt_for_author helper); save-path warnings route through the prompt channel where the user can see them, including a new warning when the pre-save reload fails; skip/banner wording no longer implies partial saves and is ASCII-only for legacy console codepages. Tests: in-process install_hooks::run tests now isolate HOME and the prompt env (they bypass child-command isolation); positive prompt tests scrub inherited background-agent markers; the pty test passes the binary path via the environment (quote-safe) and self-skips on non-util-linux script(1). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
svarlamov-git-ai
force-pushed
the
feat/author-prompt-dev-tty
branch
from
August 27, 2026 03:29
897df49 to
9e51833
Compare
svarlamov
force-pushed
the
feat/author-prompt-dev-tty
branch
from
August 27, 2026 15:14
9e51833 to
ab13aeb
Compare
Constraint: the stack must not change any installer (install.sh, install.ps1, pkg/MSI packaging, nix) and must stay compatible with older binaries driving the auto-update flow, which fetch and run the new install script with only their own env vars set. Revert the install.sh and macOS postinstall suppression hunks and instead have author_prompt_suppressed() consult the binary's existing is_superuser_expected_environment() detector (CI, containers, K8s, MDM expectations, and GIT_AI_DAEMON_UPGRADE). Combined with the GIT_AI_BACKGROUND_UPGRADE_WORKER check, every suppression signal is one that released binaries (verified against v1.6.24 and v1.7.0) already set on their daemon-silent and background upgrade paths, so no installer or spawner cooperation is required. Tests: new CI-environment suppression test; the ambient scrub helper now also clears is_superuser_expected_environment() markers and is applied to every prompt test, so each test exercises its own gate even when the suite itself runs in CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
svarlamov-git-ai
force-pushed
the
feat/author-prompt-dev-tty
branch
from
August 27, 2026 15:56
ab13aeb to
b568aad
Compare
- Suppress the prompt for root-run installs on Unix (curl | sudo bash, ssh -tt root@ provisioning): HOME resolves to root's, so a typed identity would land in a config.json the real user's git-ai never reads. Deliberately ignores GIT_AI_ALLOW_SUPERUSER, which install.sh exports for every root run. - Add a --no-author-prompt flag for spawners that cannot set env vars (e.g. packaged-installer command lines). Binary-side only; wiring any packaging to it is left to a separate PR since this stack must not touch installers. - Enter-confirming empty defaults now prints the same "skipping author setup" guidance as the timeout/EOF paths instead of silently saving nothing. - Replace the prompt gate's hand-rolled author/api-key resolution with Config::fresh() (canonical env>file precedence), keeping an explicit corrupt-config-file guard so an unparseable config.json still skips before prompting. - Export UNATTENDED_ENV_MARKERS (now also driving is_superuser_expected_environment) and BACKGROUND_AGENT_ENV_MARKERS so the test scrubber stays in lockstep with production detection instead of hand-copying marker lists; use the named env constants across test files. Prompt-firing tests self-skip where an unscrubbable marker (/.dockerenv, root) forces suppression. - De-flake timed_line_reader_times_out (60s reader block vs 30s bound). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The recommended install paths never showed the author prompt from #2227:
curl install.sh | bashgives install-hooks the curl pipe as stdin, and install.ps1 runs& $exe install-hooks --env | Out-Host, piping stdout. Following git's credential-prompt pattern, the prompt now falls back to the controlling terminal —/dev/ttyon Unix,CONIN$/CONOUT$on Windows (mechanism in #2230) — when stdio is redirected but a terminal exists.No installer changes. All unattended-environment detection is in-binary, and it is compatible with older binaries driving the auto-update flow (an old binary fetches and runs the new install script with only its own env vars set):
author_prompt_suppressed()reuses the binary's existingis_superuser_expected_environment()detector — CI markers, containers, K8s, MDM expectations, andGIT_AI_DAEMON_UPGRADE(set by every released binary's daemon-silent upgrade path; verified against v1.6.24 and v1.7.0).GIT_AI_BACKGROUND_UPGRADE_WORKER=1— set by every released binary when spawning the background upgrade worker, which runs in the user's session with an openable /dev/tty and would otherwise pop a prompt mid-session.is_in_background_agent()— AI-agent sandboxes (Claude web, Devin, codex-cloud,GIT_AI_CLOUD_AGENT).GIT_AI_NO_AUTHOR_PROMPTremains an explicit opt-out for anyone who wants one, but nothing depends on it.git-ai install-hooks &, nohup) is skipped via a foreground-process-group check (SIGTTIN would stop the process, freezing even the prompt's timeout).Channel resolution (
resolve_prompt_io):GIT_AI_TEST_FORCE_TTY→ piped stdio (test semantics); stdin+stdout TTYs → stdio; else controlling terminal; else skip. The terminal is only opened after the cheap config gates pass, and the reader thread only spawns after the full gate passes.Tests (TDD, red first)
script(1)with stdin redirected to/dev/null— non-tty stdin, but the pty is the controlling terminal; input piped through script reaches the /dev/tty prompt and the confirmed author lands in config.json. Self-skips where util-linuxscriptis unavailable.detach_from_controlling_terminalhelper (setsid/DETACHED_PROCESS— deliberately notCREATE_NO_WINDOW, which still allocates a hidden console); all test children default toGIT_AI_NO_AUTHOR_PROMPT=1so no test can reach a developer's real terminal, with prompt tests opting back in via=0.Notes / risks
git-ai upgradeon Windows performed by an older binary runs the installer on a hidden console with no suppression env; the prompt falls back to the invisible CONIN$ and times out after 15s, once, then the install continues. New binaries setGIT_AI_NO_AUTHOR_PROMPT=1on that spawn, so this self-heals after one upgrade cycle. The auto (daemon/background) flows are fully suppressed for old binaries via their pre-existing env vars.sudo bash install.shfrom a terminal prompts as root and writes /root/.git-ai — identical to the already-shipped TTY-under-sudo behavior; JAMF/MDM runs have no controlling terminal and skip cleanly.🤖 Generated with Claude Code