Skip to content

Commit 3e64eb3

Browse files
committed
cowork-bot: fix install/dispatch exception handling — bare except Exception no longer swallows typer.Exit (double error print on pip failure); OSError reported cleanly in install + dispatch; Ctrl-C exits 130; +3 regression tests (27 pass, ruff clean)
1 parent 75fa259 commit 3e64eb3

2 files changed

Lines changed: 56 additions & 13 deletions

File tree

src/devforge/cli.py

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -102,16 +102,21 @@ def install(
102102
repo_url = "https://github.com/Coding-Dev-Tools/devforge-cli.git"
103103
pkg = f"git+{repo_url}[{extras}]"
104104
console.print(f"[yellow]Installing {pkg}...[/yellow]")
105+
# Only catch OS-level failures here. A bare `except Exception` would also
106+
# swallow the typer.Exit raised below (typer.Exit subclasses Exception),
107+
# double-printing an error line ("Error: 1") after the failure message.
105108
try:
106-
result = subprocess.run([sys.executable, "-m", "pip", "install", pkg], capture_output=True, text=True)
107-
if result.returncode == 0:
108-
console.print(f"[green]Successfully installed:[/green] {', '.join(targets)}")
109-
else:
110-
console.print(f"[red]Installation failed:[/red] {result.stderr[:500]}")
111-
raise typer.Exit(code=1)
112-
except Exception as e:
113-
console.print(f"[red]Error: {e}[/red]")
109+
result = subprocess.run(
110+
[sys.executable, "-m", "pip", "install", pkg], capture_output=True, text=True
111+
)
112+
except OSError as e:
113+
console.print(f"[red]Error running pip:[/red] {e}")
114114
raise typer.Exit(code=1) from e
115+
if result.returncode == 0:
116+
console.print(f"[green]Successfully installed:[/green] {', '.join(targets)}")
117+
else:
118+
console.print(f"[red]Installation failed:[/red] {result.stderr[:500]}")
119+
raise typer.Exit(code=1)
115120

116121

117122
@app.command(name="versions")
@@ -188,11 +193,19 @@ def dispatch(ctx: typer.Context):
188193
# `--config file.yaml`) reach the underlying CLI instead of being
189194
# rejected by typer as "No such option".
190195
forwarded = list(ctx.args)
191-
result = subprocess.run(
192-
[sys.executable, "-m", module_name] + forwarded,
193-
capture_output=True,
194-
text=True,
195-
)
196+
try:
197+
result = subprocess.run(
198+
[sys.executable, "-m", module_name] + forwarded,
199+
capture_output=True,
200+
text=True,
201+
)
202+
except OSError as e:
203+
console.print(f"[red]Error launching {tool_name}:[/red] {e}")
204+
raise typer.Exit(code=1) from e
205+
except KeyboardInterrupt:
206+
# Forward Ctrl-C as a conventional 130 exit, not a raw traceback.
207+
console.print("[yellow]Interrupted.[/yellow]")
208+
sys.exit(130)
196209
if result.stdout:
197210
sys.stdout.write(result.stdout)
198211
if result.stderr:

tests/test_cli.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,3 +208,33 @@ def test_versions_reports_error(self, _mock):
208208
result = runner.invoke(app, ["versions", "guard"])
209209
assert result.exit_code == 0
210210
assert "error checking" in result.stdout
211+
212+
213+
class TestInstallErrorHandling:
214+
@mock.patch("devforge.cli.subprocess.run")
215+
def test_install_failure_no_double_error(self, mock_run):
216+
"""A failed pip install must print exactly one failure message.
217+
218+
Regression guard: the old `except Exception` also caught the
219+
typer.Exit raised after 'Installation failed', printing a spurious
220+
second line ('Error: 1').
221+
"""
222+
mock_run.return_value = mock.MagicMock(returncode=1, stdout="", stderr="boom")
223+
result = runner.invoke(app, ["install", "guard"])
224+
assert result.exit_code == 1
225+
assert "Installation failed" in result.stdout
226+
assert "Error: 1" not in result.stdout
227+
228+
@mock.patch("devforge.cli.subprocess.run", side_effect=OSError("pip missing"))
229+
def test_install_oserror_reported(self, mock_run):
230+
result = runner.invoke(app, ["install", "guard"])
231+
assert result.exit_code == 1
232+
assert "Error running pip" in result.stdout
233+
234+
@mock.patch("devforge.cli._is_tool_installed", return_value=True)
235+
@mock.patch("devforge.cli.subprocess.run", side_effect=OSError("python gone"))
236+
def test_dispatch_oserror_reported(self, mock_run, _mock_installed):
237+
"""OSError from the tool subprocess gets a clear message, not a traceback."""
238+
result = runner.invoke(app, ["guard"])
239+
assert result.exit_code == 1
240+
assert "Error launching guard" in result.stdout

0 commit comments

Comments
 (0)