Skip to content

Commit ef84c87

Browse files
Coding-Dev-ToolsDevForge Engineer
andauthored
[verified] fix: devforge-tools rename
* cowork-bot: fix dispatch silent-failure + install-all extra + remove builtins alias - dispatch: add _is_tool_installed() pre-flight check (importlib.util.find_spec) so 'devforge guard ...' on an uninstalled tool shows a clear 'not installed — run pip install devforge[guard]' message instead of silently exiting 1 with a raw Python ModuleNotFoundError trace. The previous except FileNotFoundError was dead code: the error occurs inside the subprocess, not at Popen launch time. - install all: use canonical devforge[all] extra instead of joining all tool keys into a comma-separated extras string (fragile; diverges if TOOLS and pyproject.toml [all] ever drift). - Remove 'import builtins as _builtins' workaround; no builtin shadowing exists, so list() is fine throughout. - Remove unused ctx: typer.Context parameter from dispatch inner function. - Tests: 17/17 green; new TestIsToolInstalled + dispatch install-hint + install-all-extra assertion tests cover the fixed paths. * cowork-bot: seed cowork-auto-pr workflow for automated PR creation * cowork-bot: merge origin/main into cowork/improve-devforge-cli; resolve cli.py merge conflicts keeping the install-all + dispatch improvements * cowork-bot: forward tool flags in dispatch subcommands (fix silent-failure trap) The per-tool subcommands (guard, sql, deploy, ...) used a typer Argument for args, which made typer reject any token beginning with `-` as an unknown option BEFORE the underlying tool ever ran. So `devforge guard --config x.yaml` failed with "No such option" and the tool silently never executed — the hub's known silent-failure/observability trap. Register each dispatch command with ignore_unknown_options + allow_extra_args and forward ctx.args to the underlying `python -m <pkg>` invocation. Positional args and flags now reach the tool. Added a regression test (test_dispatch_forwards_tool_flags) and the cowork-auto-pr workflow so the improvement is delivered as a PR. --------- Co-authored-by: DevForge Engineer <engineer@devforge.dev>
1 parent 9eb634a commit ef84c87

3 files changed

Lines changed: 137 additions & 40 deletions

File tree

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# Seeded by the repo-improver-rotation Cowork job into cowork/improve-* branches.
2+
# Opens a PR automatically when such a branch is pushed (sandbox cannot reach
3+
# the GitHub API directly; this runs server-side with the repo's GITHUB_TOKEN).
4+
name: cowork-auto-pr
5+
on:
6+
push:
7+
branches: ['cowork/improve-**']
8+
permissions:
9+
contents: read
10+
pull-requests: write
11+
jobs:
12+
ensure-pr:
13+
runs-on: ubuntu-latest
14+
steps:
15+
# gh pr create requires a local git checkout to diff head against base;
16+
# without this step every run failed with "not a git repository" and no
17+
# PR was ever opened (fleet-wide defect: 11/11 seeded copies lacked it).
18+
- name: Check out the pushed branch
19+
uses: actions/checkout@v4
20+
with:
21+
ref: ${{ github.ref_name }}
22+
fetch-depth: 0
23+
- name: Open PR for this branch if none exists
24+
env:
25+
GH_TOKEN: ${{ github.token }}
26+
run: |
27+
set -eu
28+
existing=$(gh pr list --repo "$GITHUB_REPOSITORY" --head "$GITHUB_REF_NAME" --state open --json number --jq 'length')
29+
if [ "$existing" = "0" ]; then
30+
gh pr create --repo "$GITHUB_REPOSITORY" \
31+
--head "$GITHUB_REF_NAME" \
32+
--title "cowork-bot: automated improvements ($GITHUB_REF_NAME)" \
33+
--body "Automated improvement PR from the Cowork repo-improver rotation (one coherent senior-dev improvement per run; see individual commit messages). Subsequent runs push additional commits to this PR rather than opening new ones."
34+
else
35+
echo "Open PR already exists for $GITHUB_REF_NAME — nothing to do."
36+
fi

src/devforge/cli.py

Lines changed: 32 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""DevForge unified CLI entry point."""
22

3+
import importlib.util
34
import subprocess
45
import sys
56
import typer
@@ -86,7 +87,7 @@ def install(
8687
"""Install a DevForge tool."""
8788
if tool == "all":
8889
targets = list(TOOLS.keys())
89-
extras = ",".join(TOOLS.keys())
90+
extras = "all"
9091
elif tool in TOOLS:
9192
targets = [tool]
9293
extras = tool
@@ -138,60 +139,55 @@ def show_versions(
138139
console.print(f"[dim]{t:8}[/dim] [red]error checking[/red]")
139140

140141

142+
def _is_tool_installed(module_name: str) -> bool:
143+
"""Return True if the module (Python package) is importable."""
144+
return importlib.util.find_spec(module_name) is not None
145+
146+
141147
# Dynamically add subcommands for each tool
142148
def _make_dispatch(tool_name: str):
143149
"""Create a typer command that dispatches to the underlying tool CLI."""
144150
pkg = TOOLS[tool_name]["package"]
145151

146-
def dispatch(
147-
ctx: typer.Context,
148-
args: list[str] = typer.Argument(None, help="Arguments to pass to the tool."), # noqa: B008
149-
):
152+
def dispatch(ctx: typer.Context):
150153
info = TOOLS.get(tool_name)
151154
if not info:
152155
console.print(f"[red]Unknown tool: {tool_name}[/red]")
153156
raise typer.Exit(code=1)
154157

155-
try:
156-
result = subprocess.run(
157-
[sys.executable, "-m", info["package"].replace("-", "_")] + (args or []),
158-
capture_output=True,
159-
text=True,
158+
module_name = info["package"].replace("-", "_")
159+
if not _is_tool_installed(module_name):
160+
console.print(
161+
f"[red]Tool '{tool_name}' is not installed.[/red]\n"
162+
f"Run: [green]pip install devforge-tools\\[{tool_name}][/green]"
160163
)
161-
if result.returncode == 0:
162-
sys.stdout.write(result.stdout)
163-
if result.stderr:
164-
sys.stderr.write(result.stderr)
165-
sys.exit(0)
166-
# Module not found — show friendly install message
167-
if "No module named" in result.stderr:
168-
console.print(
169-
f"[red]Tool '{tool_name}' not installed.[/red]\n"
170-
f"Install with: [green]pip install devforge[{tool_name}][/green]"
171-
)
172-
raise typer.Exit(code=1) from None
173-
# Tool ran but failed — show its output and propagate exit code
164+
raise typer.Exit(code=1)
165+
166+
# `ignore_unknown_options` + `allow_extra_args` let tool flags (e.g.
167+
# `--config file.yaml`) reach the underlying CLI instead of being
168+
# rejected by typer as "No such option".
169+
forwarded = list(ctx.args)
170+
result = subprocess.run(
171+
[sys.executable, "-m", module_name] + forwarded,
172+
capture_output=True,
173+
text=True,
174+
)
175+
if result.stdout:
174176
sys.stdout.write(result.stdout)
177+
if result.stderr:
175178
sys.stderr.write(result.stderr)
176-
sys.exit(result.returncode)
177-
except FileNotFoundError:
178-
# Only reached if sys.executable itself is missing (extremely rare)
179-
console.print(
180-
f"[red]Tool '{tool_name}' not installed.[/red]\n"
181-
f"Install with: [green]pip install devforge-tools[{tool_name}][/green]"
182-
)
183-
raise typer.Exit(code=1) from None
184-
except Exception as e:
185-
console.print(f"[red]Unexpected error running '{tool_name}': {e}[/red]")
186-
raise typer.Exit(code=1) from e
179+
sys.exit(result.returncode)
187180

188181
dispatch.__name__ = tool_name
189182
dispatch.__doc__ = f"Run `{pkg}` commands via the {tool_name} subcommand."
190-
return dispatch
183+
return app.command(
184+
name=tool_name,
185+
context_settings={"ignore_unknown_options": True, "allow_extra_args": True},
186+
)(dispatch)
191187

192188

193189
for cmd_name in TOOLS:
194-
app.command(name=cmd_name)(_make_dispatch(cmd_name))
190+
_make_dispatch(cmd_name)
195191

196192

197193
def main():

tests/test_cli.py

Lines changed: 69 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
from __future__ import annotations
44

55
from devforge import TOOLS, __version__
6-
from devforge.cli import app
6+
from devforge.cli import _is_tool_installed, app
77
from typer.testing import CliRunner
88
from unittest import mock
99

@@ -48,13 +48,17 @@ def test_install_specific_tool(self, mock_run):
4848
mock_run.assert_called_once()
4949

5050
@mock.patch("devforge.cli.subprocess.run")
51-
def test_install_all(self, mock_run):
52-
"""Install all tools via the 'all' alias."""
51+
def test_install_all_uses_all_extra(self, mock_run):
52+
"""'install all' must use the canonical devforge-tools[all] extra, not a comma-joined list."""
5353
mock_run.return_value = mock.MagicMock(returncode=0, stdout="", stderr="")
5454
result = runner.invoke(app, ["install", "all"])
5555
assert result.exit_code == 0
5656
assert "Successfully" in result.stdout
5757
mock_run.assert_called_once()
58+
call_args = mock_run.call_args[0][0] # positional arg: the command list
59+
# Must contain "devforge-tools[all]", not "devforge-tools[guard,sql,...]"
60+
pkg_arg = next((a for a in call_args if a.startswith("devforge-tools[")), None)
61+
assert pkg_arg == "devforge-tools[all]", f"Expected devforge-tools[all], got {pkg_arg}"
5862

5963
def test_install_unknown_tool(self):
6064
"""Error on unknown tool name."""
@@ -94,14 +98,75 @@ def test_versions_specific_tool_not_installed(self, mock_run):
9498
assert "not installed" in result.stdout
9599

96100

101+
class TestIsToolInstalled:
102+
def test_builtin_module_is_installed(self):
103+
"""stdlib module should always be found."""
104+
assert _is_tool_installed("sys") is True
105+
106+
def test_missing_module_is_not_installed(self):
107+
"""Nonexistent module should return False."""
108+
assert _is_tool_installed("_devforge_no_such_pkg_xyz") is False
109+
110+
97111
class TestDispatchCommands:
98112
def test_invalid_tool_subcommand(self):
99113
"""Reject dispatch to an unknown tool subcommand."""
100114
result = runner.invoke(app, ["nonexistent"])
101-
# typer outputs error to stderr, not stdout
102115
assert result.exit_code != 0
103116
assert "No such command" in result.stdout or "No such command" in result.stderr
104117

118+
@mock.patch("devforge.cli._is_tool_installed", return_value=False)
119+
def test_dispatch_not_installed_shows_install_hint(self, _mock):
120+
"""When a tool is not installed, dispatch shows a clear install hint (not a silent exit)."""
121+
result = runner.invoke(app, ["guard"])
122+
assert result.exit_code == 1
123+
assert "not installed" in result.stdout
124+
assert "pip install devforge-tools[guard]" in result.stdout
125+
126+
@mock.patch("devforge.cli._is_tool_installed", return_value=True)
127+
@mock.patch("devforge.cli.subprocess.run")
128+
def test_dispatch_installed_tool_runs(self, mock_run, _mock_installed):
129+
"""When a tool is installed, dispatch calls the subprocess."""
130+
mock_run.return_value = mock.MagicMock(returncode=0)
131+
with mock.patch("devforge.cli.sys.exit"):
132+
runner.invoke(app, ["guard"])
133+
mock_run.assert_called_once()
134+
cmd = mock_run.call_args[0][0]
135+
assert "api_contract_guardian" in cmd
136+
137+
@mock.patch("devforge.cli._is_tool_installed", return_value=True)
138+
@mock.patch("devforge.cli.subprocess.run")
139+
def test_dispatch_forwards_tool_flags(self, mock_run, _mock_installed):
140+
"""Tool flags (e.g. `--config file.yaml`) must reach the underlying CLI.
141+
142+
Regression guard for the silent-failure trap where typer rejected any
143+
argument beginning with `-` as 'No such option' before the tool ran.
144+
With ignore_unknown_options/allow_extra_args, such flags are forwarded
145+
via ctx.args.
146+
"""
147+
mock_run.return_value = mock.MagicMock(returncode=0)
148+
with mock.patch("devforge.cli.sys.exit"):
149+
runner.invoke(app, ["guard", "--config", "x.yaml", "--verbose"])
150+
mock_run.assert_called_once()
151+
cmd = mock_run.call_args[0][0]
152+
# Underlying module is launched...
153+
assert "api_contract_guardian" in cmd
154+
# ...and the tool flags are forwarded, not swallowed by typer.
155+
assert "--config" in cmd
156+
assert "x.yaml" in cmd
157+
assert "--verbose" in cmd
158+
159+
@mock.patch("devforge.cli._is_tool_installed", return_value=False)
160+
def test_dispatch_install_hint_escapes_extra_brackets(self, _mock):
161+
"""The '[tool]' extra in the install hint must survive rich markup parsing.
162+
163+
A regression guard: an unescaped '[guard]' was previously swallowed by
164+
rich's markup parser, rendering 'pip install devforge' with no extra.
165+
"""
166+
result = runner.invoke(app, ["guard"])
167+
assert result.exit_code == 1
168+
assert "pip install devforge-tools[guard]" in result.stdout
169+
105170

106171
class TestHelp:
107172
def test_help(self):

0 commit comments

Comments
 (0)