From 9fe9c4fb7225bbf0a243514e88e6772491fde71e Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Tue, 7 Apr 2026 17:30:10 -0500 Subject: [PATCH] fix(sync[hang]) Prevent indefinite blocking on credential prompts and slow fetches why: vcspull sync hangs indefinitely when a repo needs credentials (git waits on stdin) or when fetching very large repos (e.g. 15GB rust-lang/rust). Sequential processing means one stuck repo blocks the entire batch. what: - Set GIT_TERMINAL_PROMPT=0 at sync() entry to make git fail fast on auth - Add 120s timeout to _maybe_fetch subprocess.run() call - Handle subprocess.TimeoutExpired with descriptive error message - Pass no-prompt env to _maybe_fetch subprocess calls - Add tests for timeout handling, env propagation, and prompt suppression Refs: #543 --- src/vcspull/cli/sync.py | 20 +++++ tests/cli/test_sync_plan_helpers.py | 120 +++++++++++++++++++++++++++- 2 files changed, 138 insertions(+), 2 deletions(-) diff --git a/src/vcspull/cli/sync.py b/src/vcspull/cli/sync.py index fadf6e93d..b86154f5f 100644 --- a/src/vcspull/cli/sync.py +++ b/src/vcspull/cli/sync.py @@ -159,6 +159,19 @@ def clamp(n: int, _min: int, _max: int) -> int: NO_REPOS_FOR_TERM_MSG = 'No repo found in config(s) for "{name}"' +_FETCH_TIMEOUT_SECONDS = 120 + +_NO_PROMPT_ENV: dict[str, str] | None = None + + +def _get_no_prompt_env() -> dict[str, str]: + """Return an environment dict that prevents git from prompting on stdin.""" + global _NO_PROMPT_ENV + if _NO_PROMPT_ENV is None: + _NO_PROMPT_ENV = {**os.environ, "GIT_TERMINAL_PROMPT": "0"} + return _NO_PROMPT_ENV + + def _maybe_fetch( repo_path: pathlib.Path, *, @@ -177,7 +190,11 @@ def _maybe_fetch( capture_output=True, text=True, check=False, + timeout=_FETCH_TIMEOUT_SECONDS, + env=_get_no_prompt_env(), ) + except subprocess.TimeoutExpired: + return False, f"git fetch timed out after {_FETCH_TIMEOUT_SECONDS}s" except FileNotFoundError: return False, "git executable not found" except OSError as exc: @@ -642,6 +659,9 @@ def sync( include_worktrees: bool = False, ) -> None: """Entry point for ``vcspull sync``.""" + # Prevent git from blocking on credential prompts during batch sync + os.environ.setdefault("GIT_TERMINAL_PROMPT", "0") + # Show help if no patterns and --all not specified if not repo_patterns and not sync_all: if parser is not None: diff --git a/tests/cli/test_sync_plan_helpers.py b/tests/cli/test_sync_plan_helpers.py index a9a3a9a1c..5d097dee3 100644 --- a/tests/cli/test_sync_plan_helpers.py +++ b/tests/cli/test_sync_plan_helpers.py @@ -2,13 +2,14 @@ from __future__ import annotations +import os import subprocess import typing as t import pytest from vcspull.cli._output import PlanAction -from vcspull.cli.sync import SyncPlanConfig, _determine_plan_action, _maybe_fetch +from vcspull.cli.sync import SyncPlanConfig, _determine_plan_action, _maybe_fetch, sync if t.TYPE_CHECKING: import pathlib @@ -81,6 +82,15 @@ class MaybeFetchFixture(t.NamedTuple): subprocess_behavior="non-zero", expected_result=(True, None), ), + MaybeFetchFixture( + test_id="fetch-timeout", + fetch=True, + offline=False, + create_repo=True, + create_git_dir=True, + subprocess_behavior="timeout", + expected_result=(False, None), # message checked separately via startswith + ), ] @@ -119,6 +129,8 @@ def _patched_run( if subprocess_behavior == "os-error": error_message = "Permission denied" raise OSError(error_message) + if subprocess_behavior == "timeout": + raise subprocess.TimeoutExpired(cmd=args[0], timeout=120) if subprocess_behavior == "non-zero": return subprocess.CompletedProcess( args=args[0], @@ -140,7 +152,13 @@ def _patched_run( config=SyncPlanConfig(fetch=fetch, offline=offline), ) - assert result == expected_result + ok, message = result + assert ok == expected_result[0] + if subprocess_behavior == "timeout": + assert message is not None + assert "timed out" in message + else: + assert result == expected_result class DeterminePlanActionFixture(t.NamedTuple): @@ -248,3 +266,101 @@ def test_determine_plan_action( action, detail = _determine_plan_action(status, config=config) assert action is expected_action assert detail == expected_detail + + +def test_maybe_fetch_passes_no_prompt_env( + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Verify _maybe_fetch sets GIT_TERMINAL_PROMPT=0 to prevent hangs.""" + repo_path = tmp_path / "repo" + repo_path.mkdir() + (repo_path / ".git").mkdir() + + captured_kwargs: dict[str, t.Any] = {} + + def _spy_run( + *args: t.Any, + **kwargs: t.Any, + ) -> subprocess.CompletedProcess[str]: + captured_kwargs.update(kwargs) + return subprocess.CompletedProcess( + args=args[0], + returncode=0, + stdout="", + stderr="", + ) + + monkeypatch.setattr("subprocess.run", _spy_run) + + _maybe_fetch( + repo_path=repo_path, + config=SyncPlanConfig(fetch=True, offline=False), + ) + + assert "env" in captured_kwargs, "subprocess.run must receive env kwarg" + assert captured_kwargs["env"].get("GIT_TERMINAL_PROMPT") == "0" + + +def test_maybe_fetch_passes_timeout( + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Verify _maybe_fetch sets a timeout to prevent indefinite blocking.""" + repo_path = tmp_path / "repo" + repo_path.mkdir() + (repo_path / ".git").mkdir() + + captured_kwargs: dict[str, t.Any] = {} + + def _spy_run( + *args: t.Any, + **kwargs: t.Any, + ) -> subprocess.CompletedProcess[str]: + captured_kwargs.update(kwargs) + return subprocess.CompletedProcess( + args=args[0], + returncode=0, + stdout="", + stderr="", + ) + + monkeypatch.setattr("subprocess.run", _spy_run) + + _maybe_fetch( + repo_path=repo_path, + config=SyncPlanConfig(fetch=True, offline=False), + ) + + assert "timeout" in captured_kwargs, "subprocess.run must receive timeout kwarg" + assert captured_kwargs["timeout"] > 0 + + +def test_sync_sets_git_terminal_prompt( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Verify sync() sets GIT_TERMINAL_PROMPT=0 to prevent credential hangs.""" + # Remove GIT_TERMINAL_PROMPT if already set so setdefault takes effect + monkeypatch.delenv("GIT_TERMINAL_PROMPT", raising=False) + + sync( + repo_patterns=[], + config=None, + workspace_root=None, + dry_run=False, + output_json=False, + output_ndjson=False, + color="never", + exit_on_error=False, + show_unchanged=False, + summary_only=False, + long_view=False, + relative_paths=False, + fetch=False, + offline=False, + verbosity=0, + sync_all=False, + # No parser and no --all means sync() returns early, but env is set first + ) + + assert os.environ.get("GIT_TERMINAL_PROMPT") == "0"