fix: devforge-tools rename - install 'all' extra + dispatch install-hint bracket escape#11
fix: devforge-tools rename - install 'all' extra + dispatch install-hint bracket escape#11Coding-Dev-Tools wants to merge 4 commits into
Conversation
…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.
…ve cli.py merge conflicts keeping the install-all + dispatch improvements
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f06622c1a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| cmd = mock_run.call_args[0][0] | ||
| assert "api_contract_guardian" in cmd | ||
|
|
||
|
|
There was a problem hiding this comment.
Remove the extra blank line so formatting passes
In this PR, .github/workflows/ci.yml still runs ruff format --check src/ tests/; running that command on this commit reports Would reformat: tests/test_cli.py and the formatter diff removes this extra blank line. As-is, PR CI will fail in the formatting job before tests run, so please run the formatter or remove the blank line.
Useful? React with 👍 / 👎.
Council Gate Verdict — REWORKRisk level: high | Agreement: 0.688 | Council session: Per-model scores (rubric: correctness / safety / style / tests / complexity)
Engraphis referenceCouncil verdict persisted as
|
|
Correction to the above verdict (automated follow-up):
|
Pre-PR Review — devforge-cli #11Verdict: REQUEST_CHANGES (quality gate — formatting). Code logic is correct; tests pass; but What's good
Blocker — lint CI redThe Required fixRun Note
Action: Reformat with ruff and push; then the lint gate goes green and the PR is merge-eligible (subject to the <6h/<3-contributor embargo). |
Council Gate Verdict — REWORK (re-review)Risk level: high | Agreement: 0.167 | Council session: Re-review triggered by new commits pushed after the prior verdict. Per-model scores (rubric: correctness / safety / style / tests / complexity)
Engraphis referenceCouncil verdict persisted as
|
Summary
Fixes two post-rename bugs in the devforge → devforge-tools rename:
install allnow uses the canonicaldevforge-tools[all]extra (was joining the extra list with commas, producing an invalid spec pip rejects).dispatchinstall-hint now escapes[tool]so rich markup stops swallowing the bracketed tool name.Tests updated
devforge→devforge-tools; added a bracket-escape regression test.Verification
ruff check+ruff formatcleancowork/improve-devforge-climerged up to date withorigin/main(main is an ancestor)🤖 Generated by cowork-bot (senior-dev-rotation delivery; code committed 2026-07-16T03:37, pushed this run after GH creds recovered).