Skip to content

Commit 9fe9c4f

Browse files
committed
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
1 parent e498d50 commit 9fe9c4f

2 files changed

Lines changed: 138 additions & 2 deletions

File tree

src/vcspull/cli/sync.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,19 @@ def clamp(n: int, _min: int, _max: int) -> int:
159159
NO_REPOS_FOR_TERM_MSG = 'No repo found in config(s) for "{name}"'
160160

161161

162+
_FETCH_TIMEOUT_SECONDS = 120
163+
164+
_NO_PROMPT_ENV: dict[str, str] | None = None
165+
166+
167+
def _get_no_prompt_env() -> dict[str, str]:
168+
"""Return an environment dict that prevents git from prompting on stdin."""
169+
global _NO_PROMPT_ENV
170+
if _NO_PROMPT_ENV is None:
171+
_NO_PROMPT_ENV = {**os.environ, "GIT_TERMINAL_PROMPT": "0"}
172+
return _NO_PROMPT_ENV
173+
174+
162175
def _maybe_fetch(
163176
repo_path: pathlib.Path,
164177
*,
@@ -177,7 +190,11 @@ def _maybe_fetch(
177190
capture_output=True,
178191
text=True,
179192
check=False,
193+
timeout=_FETCH_TIMEOUT_SECONDS,
194+
env=_get_no_prompt_env(),
180195
)
196+
except subprocess.TimeoutExpired:
197+
return False, f"git fetch timed out after {_FETCH_TIMEOUT_SECONDS}s"
181198
except FileNotFoundError:
182199
return False, "git executable not found"
183200
except OSError as exc:
@@ -642,6 +659,9 @@ def sync(
642659
include_worktrees: bool = False,
643660
) -> None:
644661
"""Entry point for ``vcspull sync``."""
662+
# Prevent git from blocking on credential prompts during batch sync
663+
os.environ.setdefault("GIT_TERMINAL_PROMPT", "0")
664+
645665
# Show help if no patterns and --all not specified
646666
if not repo_patterns and not sync_all:
647667
if parser is not None:

tests/cli/test_sync_plan_helpers.py

Lines changed: 118 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@
22

33
from __future__ import annotations
44

5+
import os
56
import subprocess
67
import typing as t
78

89
import pytest
910

1011
from vcspull.cli._output import PlanAction
11-
from vcspull.cli.sync import SyncPlanConfig, _determine_plan_action, _maybe_fetch
12+
from vcspull.cli.sync import SyncPlanConfig, _determine_plan_action, _maybe_fetch, sync
1213

1314
if t.TYPE_CHECKING:
1415
import pathlib
@@ -81,6 +82,15 @@ class MaybeFetchFixture(t.NamedTuple):
8182
subprocess_behavior="non-zero",
8283
expected_result=(True, None),
8384
),
85+
MaybeFetchFixture(
86+
test_id="fetch-timeout",
87+
fetch=True,
88+
offline=False,
89+
create_repo=True,
90+
create_git_dir=True,
91+
subprocess_behavior="timeout",
92+
expected_result=(False, None), # message checked separately via startswith
93+
),
8494
]
8595

8696

@@ -119,6 +129,8 @@ def _patched_run(
119129
if subprocess_behavior == "os-error":
120130
error_message = "Permission denied"
121131
raise OSError(error_message)
132+
if subprocess_behavior == "timeout":
133+
raise subprocess.TimeoutExpired(cmd=args[0], timeout=120)
122134
if subprocess_behavior == "non-zero":
123135
return subprocess.CompletedProcess(
124136
args=args[0],
@@ -140,7 +152,13 @@ def _patched_run(
140152
config=SyncPlanConfig(fetch=fetch, offline=offline),
141153
)
142154

143-
assert result == expected_result
155+
ok, message = result
156+
assert ok == expected_result[0]
157+
if subprocess_behavior == "timeout":
158+
assert message is not None
159+
assert "timed out" in message
160+
else:
161+
assert result == expected_result
144162

145163

146164
class DeterminePlanActionFixture(t.NamedTuple):
@@ -248,3 +266,101 @@ def test_determine_plan_action(
248266
action, detail = _determine_plan_action(status, config=config)
249267
assert action is expected_action
250268
assert detail == expected_detail
269+
270+
271+
def test_maybe_fetch_passes_no_prompt_env(
272+
tmp_path: pathlib.Path,
273+
monkeypatch: pytest.MonkeyPatch,
274+
) -> None:
275+
"""Verify _maybe_fetch sets GIT_TERMINAL_PROMPT=0 to prevent hangs."""
276+
repo_path = tmp_path / "repo"
277+
repo_path.mkdir()
278+
(repo_path / ".git").mkdir()
279+
280+
captured_kwargs: dict[str, t.Any] = {}
281+
282+
def _spy_run(
283+
*args: t.Any,
284+
**kwargs: t.Any,
285+
) -> subprocess.CompletedProcess[str]:
286+
captured_kwargs.update(kwargs)
287+
return subprocess.CompletedProcess(
288+
args=args[0],
289+
returncode=0,
290+
stdout="",
291+
stderr="",
292+
)
293+
294+
monkeypatch.setattr("subprocess.run", _spy_run)
295+
296+
_maybe_fetch(
297+
repo_path=repo_path,
298+
config=SyncPlanConfig(fetch=True, offline=False),
299+
)
300+
301+
assert "env" in captured_kwargs, "subprocess.run must receive env kwarg"
302+
assert captured_kwargs["env"].get("GIT_TERMINAL_PROMPT") == "0"
303+
304+
305+
def test_maybe_fetch_passes_timeout(
306+
tmp_path: pathlib.Path,
307+
monkeypatch: pytest.MonkeyPatch,
308+
) -> None:
309+
"""Verify _maybe_fetch sets a timeout to prevent indefinite blocking."""
310+
repo_path = tmp_path / "repo"
311+
repo_path.mkdir()
312+
(repo_path / ".git").mkdir()
313+
314+
captured_kwargs: dict[str, t.Any] = {}
315+
316+
def _spy_run(
317+
*args: t.Any,
318+
**kwargs: t.Any,
319+
) -> subprocess.CompletedProcess[str]:
320+
captured_kwargs.update(kwargs)
321+
return subprocess.CompletedProcess(
322+
args=args[0],
323+
returncode=0,
324+
stdout="",
325+
stderr="",
326+
)
327+
328+
monkeypatch.setattr("subprocess.run", _spy_run)
329+
330+
_maybe_fetch(
331+
repo_path=repo_path,
332+
config=SyncPlanConfig(fetch=True, offline=False),
333+
)
334+
335+
assert "timeout" in captured_kwargs, "subprocess.run must receive timeout kwarg"
336+
assert captured_kwargs["timeout"] > 0
337+
338+
339+
def test_sync_sets_git_terminal_prompt(
340+
monkeypatch: pytest.MonkeyPatch,
341+
) -> None:
342+
"""Verify sync() sets GIT_TERMINAL_PROMPT=0 to prevent credential hangs."""
343+
# Remove GIT_TERMINAL_PROMPT if already set so setdefault takes effect
344+
monkeypatch.delenv("GIT_TERMINAL_PROMPT", raising=False)
345+
346+
sync(
347+
repo_patterns=[],
348+
config=None,
349+
workspace_root=None,
350+
dry_run=False,
351+
output_json=False,
352+
output_ndjson=False,
353+
color="never",
354+
exit_on_error=False,
355+
show_unchanged=False,
356+
summary_only=False,
357+
long_view=False,
358+
relative_paths=False,
359+
fetch=False,
360+
offline=False,
361+
verbosity=0,
362+
sync_all=False,
363+
# No parser and no --all means sync() returns early, but env is set first
364+
)
365+
366+
assert os.environ.get("GIT_TERMINAL_PROMPT") == "0"

0 commit comments

Comments
 (0)