From 993083405ee84ea4d424e428d364a77fde84d8f6 Mon Sep 17 00:00:00 2001 From: Ali jawwad <33836051+jawwad-ali@users.noreply.github.com> Date: Tue, 14 Jul 2026 01:29:15 +0500 Subject: [PATCH] fix(init): don't block on confirmation for 'init --here' without a TTY (#3236) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(init): don't block on confirmation for 'init --here' without a TTY When 'specify init --here' targets a non-empty directory without --force, it called typer.confirm() unconditionally. In a non-interactive session (no TTY -- CI, piped, agent) there is no input, so the prompt reads EOF and aborts unhelpfully (or blocks), with no actionable message. The named-project path already fails fast and points to --force; --here was the inconsistent outlier. Guard the confirmation with the existing _stdin_is_interactive() helper: when non-interactive, print a clear 'directory not empty; re-run with --force' error and exit 1 instead of prompting. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(init): honor piped confirmation for 'init --here'; only fail-fast on empty stdin The first version of this fix short-circuited on '_stdin_is_interactive()' (isatty) before typer.confirm, which broke 'init --here' when confirmation is piped (e.g. 'echo y | specify init --here' / CliRunner input='y\n') -- a non-TTY pipe with valid input was wrongly rejected, regressing test_init_here_without_force_preserves_shared_infra. Instead, call typer.confirm normally (piped 'y'/'n' is honored) and catch the Abort/EOFError it raises only when stdin is empty, converting that to the actionable '--force' guidance. This keeps the UX win for the no-input case without rejecting piped input. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(init): distinguish interactive cancel from no-input; defer merge warning Address Copilot review on the --here non-empty path: (1) treat typer.Abort during an interactive confirm (e.g. Ctrl+C) as a normal cancellation (exit 0), and only emit the '--force' guidance + exit 1 when there is no TTY (empty stdin / EOF) -- no longer conflating the two; (2) move the 'will be merged / may overwrite' warning so it only shows when actually proceeding (force) or folded into the confirmation prompt, not on the fail-fast path where nothing is merged. Piped confirmation (e.g. 'echo y | specify init --here') is still honored, which is why the prompt is attempted rather than refused outright when non-interactive -- the existing test_init_here_without_force_preserves_shared_infra pipes 'y' and must succeed. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(init): fail fast on non-interactive --here instead of prompting Per Copilot review: do not call typer.confirm when stdin is not a TTY -- an open-but-idle non-TTY stdin (CI/agent) could block on the prompt. When the directory is non-empty and --force is not given, fail fast with '--force' guidance unless an interactive terminal is present. Interactive confirm still offers the merge-but-preserve path (distinct from --force, which overwrites); a Ctrl+C there is treated as a normal cancellation (exit 0). The merge/overwrite warning is only printed when actually proceeding, not on the fail-fast path. Updated the preserve-merge E2E test to simulate an interactive terminal so it exercises the confirm path (non-interactive sessions now require --force). Co-Authored-By: Claude Opus 4.8 (1M context) * fix(init): honor piped y/n for 'init --here', error only on no-input Per maintainer review: restore the second-revision shape. Calling typer.confirm normally keeps 'echo y | specify init --here' reaching the non-destructive preserve-merge path (and piped 'n' cancels with exit 0). Only when no confirmation input is available at all (closed/empty stdin -> typer.Abort/EOFError) is it converted into the actionable error that points at --force. This drops the _stdin_is_interactive fail-fast that broke the common piped-confirm idiom and made preserve-merge interactive-only. The preserve test no longer needs to monkeypatch _stdin_is_interactive - it passes on the real contract. Co-Authored-By: Claude Opus 4.8 * fix(init): preserve interactive-cancel semantics; fold merge risk into the prompt Two review-driven refinements to the 'init --here' non-empty confirm, keeping the maintainer-endorsed control flow (piped y/n honored; non-interactive EOF → actionable --force error): 1. typer.confirm raises typer.Abort for BOTH an interactive Ctrl+C and an EOF on closed/empty stdin. Catching it unconditionally reported 'no confirmation input available, use --force' and exited 1 even when the user cancelled at a real TTY. Branch on _stdin_is_interactive(): a TTY cancel is a normal exit 0 ('Operation cancelled'); only non-interactive EOF becomes the --force error. 2. Fold the merge-risk warning into the confirmation question instead of printing it unconditionally beforehand, so the EOF/no-input path (which exits without changing anything) no longer prints a misleading 'will be merged' line first. Adds test_init_here_interactive_cancel_exits_zero (fails before: exit 1 with --force; passes after: exit 0, 'cancelled', pre-existing file untouched). The non-interactive EOF and piped-y preserve-merge tests are unchanged and still pass. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- src/specify_cli/commands/init.py | 39 ++++++++++++++++++--- tests/integrations/test_cli.py | 60 +++++++++++++++++++++++++++++++- 2 files changed, 93 insertions(+), 6 deletions(-) diff --git a/src/specify_cli/commands/init.py b/src/specify_cli/commands/init.py index dd815b8c5..9eb830288 100644 --- a/src/specify_cli/commands/init.py +++ b/src/specify_cli/commands/init.py @@ -220,16 +220,45 @@ def register(app: typer.Typer) -> None: console.print( f"[yellow]Warning:[/yellow] Current directory is not empty ({len(existing_items)} items)" ) - console.print( - "[yellow]Template files will be merged with existing content and may overwrite existing files[/yellow]" - ) if force: + # Proceeding: the merge/overwrite warning is accurate here. + console.print( + "[yellow]Template files will be merged with existing content and may overwrite existing files[/yellow]" + ) console.print( "[cyan]--force supplied: skipping confirmation and proceeding with merge[/cyan]" ) else: - response = typer.confirm("Do you want to continue?") - if not response: + # Fold the merge risk into the confirmation prompt rather than + # printing it unconditionally first: on the EOF/no-input path + # below the command exits without changing anything, so a + # standalone "will be merged" line would mislead. Interactive + # users still see the risk as part of the question. + # + # Call typer.confirm normally so piped y/n is honored — e.g. + # `echo y | specify init --here` keeps reaching the + # non-destructive preserve-merge path. + try: + proceed = typer.confirm( + "Template files will be merged with existing content " + "and may overwrite existing files. Do you want to continue?" + ) + except (typer.Abort, EOFError): + # typer.confirm raises Abort for BOTH an interactive Ctrl+C + # and an EOF on closed/empty stdin. Distinguish them: a real + # TTY cancellation is a normal exit (0, "cancelled"), while a + # missing-input EOF (non-interactive) becomes an actionable + # error pointing at --force. + if _stdin_is_interactive(): + console.print("[yellow]Operation cancelled[/yellow]") + raise typer.Exit(0) from None + console.print( + "[red]Error:[/red] Current directory is not empty and no " + "confirmation input is available. Re-run with " + "[bold]--force[/bold] to merge into it." + ) + raise typer.Exit(1) from None + if not proceed: console.print("[yellow]Operation cancelled[/yellow]") raise typer.Exit(0) else: diff --git a/tests/integrations/test_cli.py b/tests/integrations/test_cli.py index cb2d136e6..7b6461b4d 100644 --- a/tests/integrations/test_cli.py +++ b/tests/integrations/test_cli.py @@ -115,6 +115,63 @@ class TestInitIntegrationFlag: data = json.loads((project / ".specify" / "integration.json").read_text(encoding="utf-8")) assert data["integration"] == specify_cli.DEFAULT_INIT_INTEGRATION + def test_init_here_nonempty_noninteractive_errors_with_force_guidance(self, tmp_path): + """`init --here` on a non-empty directory with no confirmation input (empty + stdin) must fail fast with guidance to use --force, instead of the bare + 'Aborted.' from an EOF on typer.confirm. CliRunner with no `input=` provides + empty stdin, so typer.confirm raises Abort, which the command converts to the + actionable error.""" + from typer.testing import CliRunner + from specify_cli import app + + project = tmp_path / "nonempty-here" + project.mkdir() + (project / "existing.txt").write_text("keep me", encoding="utf-8") + old_cwd = os.getcwd() + try: + os.chdir(project) + result = CliRunner().invoke(app, [ + "init", "--here", "--integration", "copilot", "--script", "sh", "--ignore-agent-tools", + ], catch_exceptions=False) + finally: + os.chdir(old_cwd) + + assert result.exit_code == 1, result.output + assert "--force" in result.output + # Aborted before scaffolding: the pre-existing file is untouched. + assert (project / "existing.txt").read_text(encoding="utf-8") == "keep me" + + def test_init_here_interactive_cancel_exits_zero(self, tmp_path, monkeypatch): + """An interactive Ctrl+C at the merge confirmation (typer.Abort on a TTY) + is a normal cancellation — exit 0, "cancelled" — NOT the missing-input + --force error, which is reserved for non-interactive EOF. Guards the + regression where Abort was caught unconditionally and every cancel became + an exit-1 --force error.""" + from typer.testing import CliRunner + from specify_cli import app + import specify_cli.commands.init as init_mod + + # Simulate an interactive terminal so the Abort is treated as a cancel. + monkeypatch.setattr(init_mod, "_stdin_is_interactive", lambda: True) + + project = tmp_path / "cancel-here" + project.mkdir() + (project / "existing.txt").write_text("keep me", encoding="utf-8") + old_cwd = os.getcwd() + try: + os.chdir(project) + # No input → typer.confirm raises Abort (stands in for Ctrl+C). + result = CliRunner().invoke(app, [ + "init", "--here", "--integration", "copilot", "--script", "sh", "--ignore-agent-tools", + ], catch_exceptions=False) + finally: + os.chdir(old_cwd) + + assert result.exit_code == 0, result.output + assert "cancelled" in result.output.lower() + assert "--force" not in result.output # not the missing-input error + assert (project / "existing.txt").read_text(encoding="utf-8") == "keep me" + def test_integration_copilot_auto_promotes(self, tmp_path): from typer.testing import CliRunner from specify_cli import app @@ -835,7 +892,8 @@ class TestInitIntegrationFlag: assert (scripts_dir / "common.sh").read_text(encoding="utf-8") != custom_content def test_init_here_without_force_preserves_shared_infra(self, tmp_path): - """E2E: specify init --here (no --force) preserves existing shared infra files.""" + """E2E: confirming the merge with piped "y" (no --force) preserves + existing shared infra files (unlike --force, which overwrites them).""" from typer.testing import CliRunner from specify_cli import app