From 9658943f210fb7081068fa4a4235a5b5299bb7ad Mon Sep 17 00:00:00 2001 From: Greg Holmes Date: Wed, 19 Aug 2026 13:50:35 +0100 Subject: [PATCH 01/21] fix(core): send plugin-loader errors to stderr, not stdout The plugin loader printed load failures through a stdout Console, so one broken plugin corrupted `dg ... -o json` payloads for every remaining command -- the amplifier in the core-floor incident, where an ImportError put 24 lines of error text ahead of the JSON. Diagnostics now go through the shared stderr console (the same one main.py and print_error use), and a test pins both the console identity and the stream split. --- .../src/deepctl_core/plugin_manager.py | 7 ++-- .../tests/unit/test_plugin_manager.py | 36 +++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/packages/deepctl-core/src/deepctl_core/plugin_manager.py b/packages/deepctl-core/src/deepctl_core/plugin_manager.py index e25628c..854bb02 100644 --- a/packages/deepctl-core/src/deepctl_core/plugin_manager.py +++ b/packages/deepctl-core/src/deepctl_core/plugin_manager.py @@ -7,12 +7,11 @@ from typing import Any, cast import click -from rich.console import Console from .base_command import BaseCommand from .base_group_command import BaseGroupCommand from .models import ErrorResult, PluginInfo -from .output import print_warning +from .output import print_warning, stderr_console from .plugin_env import ( PLUGIN_VENV, get_plugin_state, @@ -21,7 +20,9 @@ ) from .timing import TimingContext -console = Console() +# Load errors are diagnostics: they go to stderr so a broken plugin can't +# corrupt `dg ... -o json` payloads on stdout. +console = stderr_console class PluginManager: diff --git a/packages/deepctl-core/tests/unit/test_plugin_manager.py b/packages/deepctl-core/tests/unit/test_plugin_manager.py index ad5d16d..e40ec06 100644 --- a/packages/deepctl-core/tests/unit/test_plugin_manager.py +++ b/packages/deepctl-core/tests/unit/test_plugin_manager.py @@ -575,3 +575,39 @@ def test_warn_if_plugin_venv_python_mismatch_warns_on_major_diff( ) as mock_warn: plugin_manager._warn_if_plugin_venv_python_mismatch() mock_warn.assert_called_once() + + +class TestLoadErrorStream: + """Plugin-load diagnostics must go to stderr, never stdout. + + A broken plugin's ImportError used to print through a stdout Console, + corrupting `dg ... -o json` payloads for every remaining command (the + amplifier in the 0.2.x core-floor incident). The module console is the + shared stderr console so that cannot recur. + """ + + def test_module_console_is_the_shared_stderr_console(self): + from deepctl_core import plugin_manager + from deepctl_core.output import stderr_console + + assert plugin_manager.console is stderr_console + assert plugin_manager.console.stderr is True + + def test_load_error_writes_to_stderr_not_stdout(self, capsys): + from deepctl_core import plugin_manager + + mock_entry_point = Mock() + mock_entry_point.name = "broken-command" + mock_entry_point.load.side_effect = ImportError("Module not found") + + with patch( + "deepctl_core.plugin_manager.metadata.entry_points" + ) as mock_eps: + mock_eps.return_value.select.return_value = [mock_entry_point] + plugin_manager.PluginManager()._load_builtin_commands( + click.Group("dg") + ) + + captured = capsys.readouterr() + assert "broken-command" in captured.err + assert captured.out == "" From 5126599b9f3701b724c543679174630ce110a5ae Mon Sep 17 00:00:00 2001 From: Greg Holmes Date: Wed, 19 Aug 2026 13:52:46 +0100 Subject: [PATCH 02/21] fix(deps): pin all root floors to workspace versions so pip upgrades deliver every release The 0.3.0 release raised root floors for the 17 packages publishing new versions, but 13 packages that released in earlier cycles still had floors as low as >=0.0.1 -- the same delivery gap, historical: a user upgrading from an old enough version gets the new deepctl while transcribe, the debug family, shared-utils and friends stay stale, because pip's only-if-needed strategy upgrades nothing the floor does not force. Adds scripts/check_dependency_floors.py, which enforces two rules: root floors must equal workspace versions (the delivery manifest), and sub-package floors must never exceed a sibling's current version (satisfiability). --fix rewrites the root floors; it produced this diff. Every pinned version is already live on PyPI. --- pyproject.toml | 26 +++--- scripts/check_dependency_floors.py | 140 +++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+), 13 deletions(-) create mode 100644 scripts/check_dependency_floors.py diff --git a/pyproject.toml b/pyproject.toml index 0f0a29c..ad16a44 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -38,21 +38,21 @@ dependencies = [ "deepctl-core>=0.2.16", "deepctl-cmd-login>=0.1.17", "deepctl-cmd-projects>=0.2.0", - "deepctl-cmd-transcribe>=0.1.10", + "deepctl-cmd-transcribe>=0.1.12", "deepctl-cmd-usage>=0.2.0", "deepctl-cmd-mcp>=0.1.15", - "deepctl-cmd-api>=0.0.1", - "deepctl-cmd-debug>=0.1.10", - "deepctl-cmd-debug-browser>=0.1.10", - "deepctl-cmd-debug-network>=0.1.10", - "deepctl-cmd-debug-audio>=0.1.10", - "deepctl-cmd-debug-probe>=0.0.1", - "deepctl-cmd-debug-toolkit>=0.0.1", - "deepctl-cmd-ffprobe>=0.0.1", + "deepctl-cmd-api>=0.0.2", + "deepctl-cmd-debug>=0.1.12", + "deepctl-cmd-debug-browser>=0.1.12", + "deepctl-cmd-debug-network>=0.1.12", + "deepctl-cmd-debug-audio>=0.1.13", + "deepctl-cmd-debug-probe>=0.0.2", + "deepctl-cmd-debug-toolkit>=0.1.0", + "deepctl-cmd-ffprobe>=0.0.2", "deepctl-cmd-update>=0.2.6", - "deepctl-cmd-plugin>=0.1.10", + "deepctl-cmd-plugin>=0.1.12", "deepctl-cmd-skills>=0.0.7", - "deepctl-cmd-init>=0.0.1", + "deepctl-cmd-init>=0.0.4", "deepctl-cmd-models>=0.1.0", "deepctl-cmd-speak>=0.0.4", "deepctl-cmd-keys>=0.1.0", @@ -61,8 +61,8 @@ dependencies = [ "deepctl-cmd-requests>=0.1.0", "deepctl-cmd-billing>=0.1.0", "deepctl-cmd-members>=0.1.0", - "deepctl-cmd-completion>=0.0.1", - "deepctl-shared-utils>=0.1.10", + "deepctl-cmd-completion>=0.0.3", + "deepctl-shared-utils>=0.1.12", "deepctl-telemetry>=0.0.6", "pydantic>=2.0.0", "rich>=13.0.0", diff --git a/scripts/check_dependency_floors.py b/scripts/check_dependency_floors.py new file mode 100644 index 0000000..4a91c20 --- /dev/null +++ b/scripts/check_dependency_floors.py @@ -0,0 +1,140 @@ +#!/usr/bin/env python3 +"""Check (or fix) intra-workspace dependency floors. + +Two rules, learned from the 0.3.0 release (PRs #100/#102): + +1. **Root floors equal workspace versions.** `dg update` runs + `pip install --upgrade deepctl`, and pip's default `only-if-needed` + strategy upgrades a sub-package only when the root floor forces it. Any + root floor below the current version means that package's fixes are + published but never delivered on upgrade — `dg --version` reports the new + release while 13 of 17 packages stay stale, which is what happened before + 0.3.0. Root's dependency list is the delivery manifest, so each + `deepctl-*` floor must equal that package's current workspace version. + +2. **Sub-package floors stay satisfiable.** Sub-package floors are API + contracts (e.g. deepctl-cmd-keys needs the deepctl-core that provides + `get_status_console`), hand-raised when a package starts using a newer + sibling API. They must never exceed the sibling's current version. + Keeping them *accurate* is still on the developer: raise the floor in the + same PR that starts importing the new API. + +Run with --fix to rewrite root floors in place (used by the release +workflow's sync job so rule 1 holds automatically on every release PR). +""" + +from __future__ import annotations + +import argparse +import json +import re +import sys +from pathlib import Path + +try: + import tomllib +except ModuleNotFoundError: + try: + import tomli as tomllib # type: ignore[no-redef] + except ModuleNotFoundError: + print("Python 3.11+ required (tomllib), or install tomli: pip install tomli") + sys.exit(1) + +REPO = Path(__file__).resolve().parent.parent +MANIFEST = REPO / ".github" / ".release-please-manifest.json" + + +def workspace_versions() -> dict[str, str]: + """Map package name -> current workspace version, per the manifest.""" + versions: dict[str, str] = {} + for path in json.loads(MANIFEST.read_text()): + pyproject = REPO / ( + "pyproject.toml" if path == "." else f"{path}/pyproject.toml" + ) + project = tomllib.loads(pyproject.read_text())["project"] + versions[project["name"]] = project["version"] + return versions + + +def floors(pyproject: Path) -> list[tuple[str, str, str]]: + """Yield (name, floor, raw-spec) for each intra-workspace dependency.""" + deps = tomllib.loads(pyproject.read_text())["project"].get("dependencies", []) + out = [] + for dep in deps: + m = re.match(r"^(deepctl[\w-]*)>=([0-9][0-9.]*)", dep) + if m: + out.append((m.group(1), m.group(2), dep)) + return out + + +def vkey(version: str) -> list[int]: + return [int(part) for part in version.split(".")] + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--fix", + action="store_true", + help="rewrite root pyproject.toml floors to the workspace versions", + ) + args = parser.parse_args() + + versions = workspace_versions() + problems: list[str] = [] + + # Rule 1: root floors == workspace versions. + root = REPO / "pyproject.toml" + root_text = root.read_text() + fixed = root_text + for name, floor, _ in floors(root): + current = versions.get(name) + if current is None: + problems.append(f"root depends on {name}, which is not in the manifest") + continue + if floor != current: + if args.fix: + fixed = fixed.replace(f'"{name}>={floor}"', f'"{name}>={current}"', 1) + else: + problems.append( + f"root floor {name}>={floor} != workspace version {current}" + " (published fixes will not be delivered by pip upgrades)" + ) + if args.fix and fixed != root_text: + root.write_text(fixed) + print(f"fixed: root floors pinned to workspace versions in {root}") + + # Rule 2: every sub-package floor must be satisfiable at co-release. + for pkg_dir in sorted((REPO / "packages").iterdir()): + pyproject = pkg_dir / "pyproject.toml" + if not pyproject.is_file(): + continue + for name, floor, _ in floors(pyproject): + current = versions.get(name) + if current is None: + problems.append( + f"{pkg_dir.name} depends on {name}, which is not in the manifest" + ) + elif vkey(floor) > vkey(current): + problems.append( + f"{pkg_dir.name}: floor {name}>={floor} exceeds" + f" workspace version {current} (unsatisfiable)" + ) + + if problems: + print("dependency floor check FAILED:", file=sys.stderr) + for problem in problems: + print(f" - {problem}", file=sys.stderr) + print( + "\nRun `python3 scripts/check_dependency_floors.py --fix` to pin" + " root floors; sub-package floors are hand-maintained.", + file=sys.stderr, + ) + return 1 + + print("dependency floors OK") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From 3863fe16d2806951f25c8db82c418496ea48e45e Mon Sep 17 00:00:00 2001 From: Greg Holmes Date: Wed, 19 Aug 2026 13:55:07 +0100 Subject: [PATCH 03/21] ci: guard dependency floors on every PR Third occurrence of hand-maintained floors going stale (#92, then #102's sweep, then the 13 historical ones fixed alongside this). The guard makes the failure mode impossible to merge instead of something someone remembers: `make floors-check` runs in CI, `make floors-fix` repairs root floors locally, and the release workflow will keep them pinned automatically. --- .github/workflows/test.yml | 17 +++++++++++++++++ Makefile | 6 ++++++ 2 files changed, 23 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 78b465b..fb3a7e5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -64,6 +64,23 @@ jobs: - name: Type check run: uv run mypy src/ packages/*/src + floors: + name: Dependency floors + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + + - name: Set up Python + uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 + with: + python-version: "3.12" + + # Root floors must equal workspace versions or `pip install --upgrade + # deepctl` (what `dg update` runs) silently skips the packages whose + # fixes the release advertises. See scripts/check_dependency_floors.py. + - name: Check dependency floors + run: make floors-check + build-test: name: Test Build Process runs-on: ubuntu-latest diff --git a/Makefile b/Makefile index 54e1dba..1f234c0 100644 --- a/Makefile +++ b/Makefile @@ -148,6 +148,12 @@ readmes: ## Generate sub-package READMEs from pyproject.toml metadata readmes-check: ## Check sub-package READMEs are up to date python3 scripts/generate_readmes.py --check +floors-check: ## Check intra-workspace dependency floors (root must pin workspace versions) + python3 scripts/check_dependency_floors.py + +floors-fix: ## Pin root dependency floors to the current workspace versions + python3 scripts/check_dependency_floors.py --fix + # =================================================================== # RUNNING THE CLI # =================================================================== From c7071a94d1240b3e307d0b19b2d921ac5545213d Mon Sep 17 00:00:00 2001 From: Greg Holmes Date: Wed, 19 Aug 2026 13:58:15 +0100 Subject: [PATCH 04/21] chore(release): sync uv.lock and root floors on the release PR automatically release-please bumps every pyproject version but has no updater for uv.lock or for the root dependency floors. The stale lock failed every CI job at `uv sync --locked` two releases running (0.2.28's 32 red checks, 0.3.0's repeat), and stale floors are the delivery gap #102 fixed by hand. Both are mechanical consequences of the version bumps, so a job now regenerates them on the release branch right after release-please pushes it. GITHUB_TOKEN pushes don't retrigger PR checks; the routine hand-edit of the release notes does, and the caveat is documented in the workflow. --- .github/workflows/release.yml | 52 +++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 98475fd..fec0e3b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -17,6 +17,8 @@ jobs: outputs: release_created: ${{ steps.release.outputs.release_created }} tag_name: ${{ steps.release.outputs.tag_name }} + prs_created: ${{ steps.release.outputs.prs_created }} + pr: ${{ steps.release.outputs.pr }} steps: - uses: googleapis/release-please-action@16a9c90856f42705d54a6fda1823352bdc62cf38 # v4 id: release @@ -25,6 +27,56 @@ jobs: config-file: .github/release-please-config.json manifest-file: .github/.release-please-manifest.json + sync-release-pr: + name: Sync lockfile and floors on the release PR + needs: release-please + if: ${{ needs.release-please.outputs.prs_created == 'true' && needs.release-please.outputs.pr }} + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + with: + ref: ${{ fromJSON(needs.release-please.outputs.pr).headBranchName }} + + - name: Set up Python + uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 + with: + python-version: "3.12" + + - name: Install uv + uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0 + + # release-please bumps every pyproject version but updates neither + # uv.lock (CI's `uv sync --locked` then fails all jobs at the install + # step -- the 0.2.28/0.3.0 wall, twice) nor the root dependency floors + # (without which `dg update` on pip delivers the wrapper and skips the + # sub-packages the changelog advertises). Both are mechanical + # consequences of the version bumps, so regenerate them here. + # + # Caveat: this push uses GITHUB_TOKEN, which does not retrigger the + # PR's checks. The routine hand-edit of the release notes retriggers + # them; for an untouched release PR, re-run checks from the UI. If + # releases ever need to go out unattended, switch this push to a + # dedicated PAT or GitHub App token. + - name: Regenerate uv.lock and pin root floors + run: | + set -euo pipefail + python3 scripts/check_dependency_floors.py --fix + uv lock + python3 scripts/check_dependency_floors.py + + - name: Commit and push if changed + run: | + set -euo pipefail + if git diff --quiet; then + echo "uv.lock and floors already in sync" + exit 0 + fi + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git add uv.lock pyproject.toml + git commit -m "chore: sync uv.lock and root dependency floors with release-please bumps" + git push + build: name: Build packages needs: release-please From 06d16a0d79dbb06c1228097d5abf85b51eac5909 Mon Sep 17 00:00:00 2001 From: Greg Holmes Date: Wed, 19 Aug 2026 13:59:08 +0100 Subject: [PATCH 05/21] chore(release): gate mark-latest, web deploy, and brew bump on PyPI resolvability A verify-published job polls `pip install --dry-run deepctl==X` in a clean venv until the full dependency closure resolves from PyPI, then the jobs that advertise the release run. Motivated twice over: 0.2.27 published partially and pip silently backtracked to the previous version with exit 0, and the 0.3.0 rollout showed a fresh install backtracking to 0.2.26 during the CDN propagation window. The brew job's own poll checks only the root JSON endpoint; this exercises the actual resolver across every floor. --- .github/workflows/release.yml | 47 ++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index fec0e3b..28c69d0 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -129,9 +129,50 @@ jobs: pip install --find-links "$DIST_DIR" dist/deepctl-*.whl deepctl --version + verify-published: + name: Verify release is installable from PyPI + needs: [release-please, publish] + # `pip install deepctl==X` must resolve the full dependency closure from + # PyPI before anything advertises the release. Two real failure modes: + # 0.2.27 published partially (root uninstallable, and pip *silently + # backtracks* to the previous version, exit 0), and the 0.3.0 rollout + # showed a fresh install backtracking to 0.2.26 during the CDN + # propagation window. Poll the actual resolver, not just the JSON + # endpoint, so mark-latest / deploy-web / brew never point at a version + # a user cannot install. + if: | + needs.release-please.outputs.release_created == 'true' && + startsWith(needs.release-please.outputs.tag_name, 'v') + runs-on: ubuntu-latest + steps: + - name: Set up Python + uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 + with: + python-version: "3.12" + + - name: Wait until pip can resolve the full release + run: | + set -euo pipefail + VERSION="${TAG_NAME#v}" + python -m venv /tmp/verify + /tmp/verify/bin/pip install --quiet --upgrade pip + for i in $(seq 1 30); do + if /tmp/verify/bin/pip install --dry-run --no-cache-dir \ + "deepctl==${VERSION}" >/dev/null 2>&1; then + echo "deepctl==${VERSION} resolves with its full closure" + exit 0 + fi + echo "Waiting for deepctl==${VERSION} to resolve (${i}/30)..." + sleep 20 + done + echo "::error::deepctl==${VERSION} did not resolve from PyPI after 10 minutes -- a dependency is missing or the index has not propagated" + exit 1 + env: + TAG_NAME: ${{ needs.release-please.outputs.tag_name }} + mark-latest: name: Mark root release as latest - needs: [release-please, publish] + needs: [release-please, publish, verify-published] # Re-assert latest on the root package tag (vX.Y.Z) after PyPI publish so # users clicking "latest" land on a tag whose artifact is actually # installable. Also re-asserts after all sub-package releases since @@ -149,7 +190,7 @@ jobs: deploy-web: name: Deploy web to production - needs: [release-please, publish] + needs: [release-please, publish, verify-published] # Only fire on root-package releases (v0.2.4, v1.0.0, …) and only after # PyPI publish so cli.deepgram.com never advertises a version that isn't # installable yet. Sub-package tags look like deepctl-cmd-listen-v0.0.3 — @@ -180,7 +221,7 @@ jobs: bump-brew-formula: name: Bump Homebrew formula - needs: [release-please, publish] + needs: [release-please, publish, verify-published] # Only fire on root-package releases (v0.2.4, v1.0.0, …). # Sub-package tags look like deepctl-cmd-listen-v0.0.3 — skip those. if: | From 563d162587b5aaea977eb24c17af63f0fbbf1e86 Mon Sep 17 00:00:00 2001 From: Greg Holmes Date: Wed, 19 Aug 2026 14:00:30 +0100 Subject: [PATCH 06/21] docs(readme): document the exit-code contract 0.3.0 started enforcing exit codes, and both the changelog and the BREAKING CHANGES entry point at documentation that only existed in web/public/llms-full.txt -- the LLM-consumption artifact. The README's CI/Automation section now carries the table (0 success, 1 error including crashes and usage errors, 2 user interrupt) plus the stdout/stderr split, where a developer with newly-red CI will actually look. --- README.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/README.md b/README.md index 0be41eb..8a3db32 100644 --- a/README.md +++ b/README.md @@ -319,6 +319,22 @@ dg usage --last-week -o yaml When running in a non-TTY environment (pipes, CI, or AI coding tools), the CLI automatically switches to structured JSON output with plain-text status messages. +### Exit codes + +Since 0.3.0, `dg` exits non-zero when a command fails — branch on the exit +code, not on parsing output: + +| Code | Meaning | +| --- | --- | +| `0` | Success | +| `1` | Error — a failed command, a crash, or a usage error (bad flag, unknown command) | +| `2` | Cancelled by the user (Ctrl-C, or declining a confirmation prompt) | + +Errors and status messages go to stderr; stdout carries only the payload, so +`dg ... -o json | jq` stays parseable even when a command fails. If a CI step +relied on `dg` always exiting `0` (every command did, before 0.3.0), it will +now fail where it previously passed silently. + ### Forcing non-interactive mode Three explicit ways to skip every prompt and run with defaults — useful from a From 33d3b6920efd754f140d70827610b59fe7d0501f Mon Sep 17 00:00:00 2001 From: Greg Holmes Date: Wed, 19 Aug 2026 15:03:30 +0100 Subject: [PATCH 07/21] fix(ci): close three holes in the dependency-floor guard Review of #103 found the guard trustworthy for the case it was written for and quietly wrong outside it. Rule 3 (new): root's dependency list must cover every published package. Rule 1 only validated the floors already listed, so a package that release-please versions and publishes but that nobody added to root's dependencies was invisible -- `pip install --upgrade deepctl` never installs it at all. That is the same delivery gap #100/#102 were about, through the one door the guard left open, and the repo adds command packages regularly. NOT_SHIPPED carries the two deliberate exclusions so the intent is stated in the diff rather than inferred from an omission. --fix no longer reports success after failing. The rewrite matched the literal `"name>=X.Y.Z"` including both quotes, so it silently no-opped on any spec with an upper bound, extra, or environment marker -- and the fix branch never recorded the miss, so the script printed "dependency floors OK" and exited 0 on a file it had not touched. It now rewrites the version inside the matched spec (preserving the rest) and falls through to `problems` when it cannot, which also puts the previously-unused third element of the floors() tuple to work. Rule 3 backstops this: a spec form the regex cannot parse at all now surfaces as a missing root dependency instead of being skipped. vkey() no longer dies on PEP 440 suffixes. A single hand-set 0.4.0rc1 anywhere in the workspace turned `make floors-check` into a bare ValueError traceback naming no package. --- scripts/check_dependency_floors.py | 66 ++++++++++++++++++++++++------ 1 file changed, 54 insertions(+), 12 deletions(-) diff --git a/scripts/check_dependency_floors.py b/scripts/check_dependency_floors.py index 4a91c20..ebe3605 100644 --- a/scripts/check_dependency_floors.py +++ b/scripts/check_dependency_floors.py @@ -1,7 +1,7 @@ #!/usr/bin/env python3 """Check (or fix) intra-workspace dependency floors. -Two rules, learned from the 0.3.0 release (PRs #100/#102): +Three rules, learned from the 0.3.0 release (PRs #100/#102): 1. **Root floors equal workspace versions.** `dg update` runs `pip install --upgrade deepctl`, and pip's default `only-if-needed` @@ -19,6 +19,14 @@ Keeping them *accurate* is still on the developer: raise the floor in the same PR that starts importing the new API. +3. **Root's dependency list covers every published package.** Rule 1 only + validates the floors that are already listed. A package that release-please + versions and publishes but that nobody added to root's `dependencies` is + never installed by `pip install --upgrade deepctl` at all — the same + delivery gap as rule 1, through the door rule 1 leaves open. Anything + deliberately not shipped as part of the CLI goes in NOT_SHIPPED, so that + intent is stated in the diff rather than inferred from an omission. + Run with --fix to rewrite root floors in place (used by the release workflow's sync job so rule 1 holds automatically on every release PR). """ @@ -43,6 +51,13 @@ REPO = Path(__file__).resolve().parent.parent MANIFEST = REPO / ".github" / ".release-please-manifest.json" +# Published packages that are deliberately not part of what `dg` installs. +# Everything else in the manifest must be a root dependency (rule 3). +NOT_SHIPPED = { + "deepctl", # the root package itself + "deepctl-plugin-example", # sample plugin, installed on demand +} + def workspace_versions() -> dict[str, str]: """Map package name -> current workspace version, per the manifest.""" @@ -68,7 +83,15 @@ def floors(pyproject: Path) -> list[tuple[str, str, str]]: def vkey(version: str) -> list[int]: - return [int(part) for part in version.split(".")] + """Sortable key from the numeric release segments only. + + PEP 440 suffixes (0.4.0rc1, 1.0.0.dev1, 0.3.0.post1) and local versions + are ignored rather than crashing the comparison -- a single hand-set + pre-release anywhere in the workspace used to turn `make floors-check` + into a bare ValueError traceback naming no package. + """ + release = re.match(r"\d+(?:\.\d+)*", version) + return [int(part) for part in release.group().split(".")] if release else [0] def main() -> int: @@ -87,19 +110,28 @@ def main() -> int: root = REPO / "pyproject.toml" root_text = root.read_text() fixed = root_text - for name, floor, _ in floors(root): + root_floors = floors(root) + for name, floor, raw in root_floors: current = versions.get(name) if current is None: problems.append(f"root depends on {name}, which is not in the manifest") continue - if floor != current: - if args.fix: - fixed = fixed.replace(f'"{name}>={floor}"', f'"{name}>={current}"', 1) - else: - problems.append( - f"root floor {name}>={floor} != workspace version {current}" - " (published fixes will not be delivered by pip upgrades)" - ) + if floor == current: + continue + if args.fix: + # Rewrite the version inside the spec we matched, so any upper + # bound, extra, or environment marker survives. A whole-spec + # literal replace silently no-ops on those, and an unfixable + # floor has to fall through to `problems` -- reporting "OK" + # after failing to fix is worse than not fixing. + new_raw = raw.replace(f">={floor}", f">={current}", 1) + if new_raw != raw and f'"{raw}"' in fixed: + fixed = fixed.replace(f'"{raw}"', f'"{new_raw}"', 1) + continue + problems.append( + f"root floor {name}>={floor} != workspace version {current}" + " (published fixes will not be delivered by pip upgrades)" + ) if args.fix and fixed != root_text: root.write_text(fixed) print(f"fixed: root floors pinned to workspace versions in {root}") @@ -121,13 +153,23 @@ def main() -> int: f" workspace version {current} (unsatisfiable)" ) + # Rule 3: every published package is a root dependency. + listed = {name for name, _, _ in root_floors} + for name in sorted(set(versions) - listed - NOT_SHIPPED): + problems.append( + f"{name} is published but is not a root dependency" + " (pip upgrades will never install it; add it to root" + " pyproject.toml or to NOT_SHIPPED in this script)" + ) + if problems: print("dependency floor check FAILED:", file=sys.stderr) for problem in problems: print(f" - {problem}", file=sys.stderr) print( "\nRun `python3 scripts/check_dependency_floors.py --fix` to pin" - " root floors; sub-package floors are hand-maintained.", + " stale root floors. Sub-package floors and missing root" + " dependencies are hand-maintained.", file=sys.stderr, ) return 1 From 0dcfd2c0d68a6095f4f6651e51ba6995f2b430d3 Mon Sep 17 00:00:00 2001 From: Greg Holmes Date: Wed, 19 Aug 2026 15:03:45 +0100 Subject: [PATCH 08/21] fix(plugin): exit 2 when the user declines the remove prompt The exit-code table added to the README in this branch says declining a confirmation prompt exits 2, and seven commands honour that by returning status="cancelled", which BaseCommand.EXIT_CODES maps. `dg plugin remove` did not: _handle_remove is a group subcommand returning None, so there is no result to map, and its bare `return` on decline exited 0 -- indistinguishable from a successful removal for any script branching on the exit code. Raise click.Abort() instead, which main.py already turns into the documented 2 for user cancellation. Two tests: the declined path aborts and never calls remove_plugin, and --yes still skips the prompt without aborting. --- .../src/deepctl_cmd_plugin/command.py | 6 ++- .../tests/unit/test_plugin_command.py | 49 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/packages/deepctl-cmd-plugin/src/deepctl_cmd_plugin/command.py b/packages/deepctl-cmd-plugin/src/deepctl_cmd_plugin/command.py index 17fd9f7..8321a48 100644 --- a/packages/deepctl-cmd-plugin/src/deepctl_cmd_plugin/command.py +++ b/packages/deepctl-cmd-plugin/src/deepctl_cmd_plugin/command.py @@ -475,7 +475,11 @@ def _handle_remove( yes = kwargs.get("yes", False) if not yes and not click.confirm(f"Are you sure you want to remove {package}?"): - return + # Abort rather than return: this is a group subcommand, so there is + # no result for BaseCommand.EXIT_CODES to map, and a bare return + # exits 0 -- indistinguishable from a successful removal. main.py + # turns Abort into the documented exit 2 for user cancellation. + raise click.Abort() result = self.remove_plugin(config, auth_manager, client, package) diff --git a/packages/deepctl-cmd-plugin/tests/unit/test_plugin_command.py b/packages/deepctl-cmd-plugin/tests/unit/test_plugin_command.py index 486040e..5c4cfa4 100644 --- a/packages/deepctl-cmd-plugin/tests/unit/test_plugin_command.py +++ b/packages/deepctl-cmd-plugin/tests/unit/test_plugin_command.py @@ -4,6 +4,7 @@ from pathlib import Path from unittest.mock import MagicMock, patch +import click import pytest from click.testing import CliRunner from deepctl_cmd_plugin.command import PluginCommand @@ -234,6 +235,54 @@ def test_remove_plugin_not_installed(self) -> None: assert result.success is False assert "not installed" in result.message + def test_remove_declined_aborts_instead_of_exiting_zero(self) -> None: + """Declining the prompt must exit 2, not 0. + + `_handle_remove` is a group subcommand returning None, so there is no + result for BaseCommand.EXIT_CODES to map to an exit code. A bare + return made a declined removal indistinguishable from a successful + one for any script branching on the exit code, contradicting the + contract published in the README. Abort is what main.py turns into 2. + """ + with ( + patch("click.confirm", return_value=False), + patch.object(self.command, "remove_plugin") as mock_remove, + ): + with pytest.raises(click.Abort): + self.command._handle_remove( + self.config, + self.auth_manager, + self.client, + package="test-plugin", + ) + + mock_remove.assert_not_called() + + def test_remove_with_yes_skips_the_prompt(self) -> None: + """Positive control: --yes must not prompt and must not abort.""" + with ( + patch("click.confirm") as mock_confirm, + patch.object(self.command, "remove_plugin") as mock_remove, + patch.object(self.command, "_maybe_update_skills"), + ): + mock_remove.return_value = PluginOperationResult( + success=True, + action="remove", + package="test-plugin", + message="Successfully removed test-plugin", + ) + + self.command._handle_remove( + self.config, + self.auth_manager, + self.client, + package="test-plugin", + yes=True, + ) + + mock_confirm.assert_not_called() + mock_remove.assert_called_once() + @patch("deepctl_cmd_plugin.command.subprocess.run") def test_discover_from_environment(self, mock_run: MagicMock) -> None: """Test discovering plugins from a specific environment.""" From fd29af150f782f16287e19aaf85ff625f93e4f18 Mon Sep 17 00:00:00 2001 From: Greg Holmes Date: Wed, 19 Aug 2026 15:03:45 +0100 Subject: [PATCH 09/21] chore(release): install the published release, not just resolve it verify-published treated a successful `pip install --dry-run` as proof the release is installable. --dry-run stops after resolution and is satisfied by PyPI metadata -- often via PEP 658, without fetching a single wheel -- so a corrupt artifact or an entry point that cannot import passes it. The build job smoke-tests `deepctl --version`, but against local dist/ artifacts, so nothing checked what PyPI actually serves before three jobs advertise it. Keep the retry loop on --dry-run (cheap, and it is what distinguishes "not propagated yet" from "broken"), then do a real install and run the CLI once it resolves. Also name the recovery path in the timeout message, since the dependents re-evaluate when the job is re-run. Add a concurrency group to sync-release-pr: it pushes to the release branch, so two commits landing on main in quick succession ran two of them against the same branch and the loser failed on a non-fast-forward push. --- .github/workflows/release.yml | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 28c69d0..2f4b8f0 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -32,6 +32,14 @@ jobs: needs: release-please if: ${{ needs.release-please.outputs.prs_created == 'true' && needs.release-please.outputs.pr }} runs-on: ubuntu-latest + # This job pushes to the release branch. Two commits landing on main in + # quick succession would otherwise run two of these against the same + # branch and the loser fails on a non-fast-forward push, reddening the + # release workflow for no real reason. Serialize instead of cancelling: + # the second run still has work to do once the first one's commit lands. + concurrency: + group: sync-release-pr + cancel-in-progress: false steps: - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: @@ -150,6 +158,8 @@ jobs: with: python-version: "3.12" + # Poll with --dry-run: it stops after resolution, which is cheap and is + # exactly what distinguishes "not propagated yet" from "broken". - name: Wait until pip can resolve the full release run: | set -euo pipefail @@ -165,11 +175,26 @@ jobs: echo "Waiting for deepctl==${VERSION} to resolve (${i}/30)..." sleep 20 done - echo "::error::deepctl==${VERSION} did not resolve from PyPI after 10 minutes -- a dependency is missing or the index has not propagated" + echo "::error::deepctl==${VERSION} did not resolve from PyPI after 10 minutes -- a dependency is missing or the index has not propagated. Re-run this job once PyPI has propagated; mark-latest, deploy-web and bump-brew-formula re-evaluate and will then run." exit 1 env: TAG_NAME: ${{ needs.release-please.outputs.tag_name }} + # Resolving is not installing. --dry-run is satisfied by PyPI metadata + # (often via PEP 658, without fetching a single wheel), so a corrupt + # artifact or an entry point that cannot import still passes it. The + # build job smoke-tests `deepctl --version`, but against the local + # dist/ artifacts -- this is the only check against what PyPI serves, + # and it is the last gate before three jobs advertise the release. + - name: Install for real and run the CLI + run: | + set -euo pipefail + VERSION="${TAG_NAME#v}" + /tmp/verify/bin/pip install --quiet --no-cache-dir "deepctl==${VERSION}" + /tmp/verify/bin/deepctl --version + env: + TAG_NAME: ${{ needs.release-please.outputs.tag_name }} + mark-latest: name: Mark root release as latest needs: [release-please, publish, verify-published] From 2e1c6c51248baeab9e0c5f207f4eccbd20fbe0b3 Mon Sep 17 00:00:00 2001 From: Greg Holmes Date: Wed, 19 Aug 2026 15:04:01 +0100 Subject: [PATCH 10/21] docs(readme): sharpen the exit-code section Two precision fixes to the table added earlier in this branch. "Errors and status messages go to stderr" was imprecise: the split is between human-readable diagnostics (stderr) and the structured result, success or failure (stdout). `dg --badflag` puts its message on stderr with empty stdout, but `dg transcribe /nonexistent.wav` puts {"status": "error", ...} on stdout with empty stderr. The promise that matters -- `-o json | jq` stays parseable on failure -- was already true; say where the reason actually is. Note that 2 deviates from the shell's conventional 130 for an interrupt. The reasoning lived only in a comment in base_command.py, so a CI author trapping 130 would write a condition that never fires. --- README.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 8a3db32..8ff3363 100644 --- a/README.md +++ b/README.md @@ -330,8 +330,13 @@ code, not on parsing output: | `1` | Error — a failed command, a crash, or a usage error (bad flag, unknown command) | | `2` | Cancelled by the user (Ctrl-C, or declining a confirmation prompt) | -Errors and status messages go to stderr; stdout carries only the payload, so -`dg ... -o json | jq` stays parseable even when a command fails. If a CI step +Note that `dg` reports `2` for an interrupt rather than the shell's +conventional `130`, so the code is the same whether the cancellation came from +Ctrl-C or from declining a prompt. + +Human-readable status and error messages go to stderr; stdout carries only the +result, so `dg ... -o json | jq` stays parseable even when a command fails — a +failure arrives there as a payload with `"status": "error"`. If a CI step relied on `dg` always exiting `0` (every command did, before 0.3.0), it will now fail where it previously passed silently. From 7ec1ffbc487e6f52bd273122eaa7c9a5bc77740c Mon Sep 17 00:00:00 2001 From: Greg Holmes Date: Wed, 19 Aug 2026 15:04:01 +0100 Subject: [PATCH 11/21] ci: bring scripts/ under the lint and format gates The lint targets scoped to src/ and packages/**/src, so nothing checked scripts/ -- including check_dependency_floors.py, which the release pipeline now depends on for delivery correctness and which will drift. Cleaning scripts/ to pass: build_standalone.py drops an unused `os` import and probes for PyInstaller with importlib.util.find_spec instead of an unused import. The rest is `ruff format` output, mostly in generate_readmes.py; verified semantically inert by comparing token streams before and after -- the only difference is adjacent string literals being joined, which Python does at compile time anyway. --- .github/workflows/test.yml | 4 +- Makefile | 414 ++++++++++++++++++------------------ scripts/build_standalone.py | 10 +- scripts/generate_readmes.py | 74 ++----- 4 files changed, 231 insertions(+), 271 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index fb3a7e5..79e74ea 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -56,10 +56,10 @@ jobs: run: uv sync --group testing --locked - name: Check formatting - run: uv run ruff format --check src/ packages/*/src + run: uv run ruff format --check src/ packages/*/src scripts/ - name: Lint - run: uv run ruff check src/ packages/*/src + run: uv run ruff check src/ packages/*/src scripts/ - name: Type check run: uv run mypy src/ packages/*/src diff --git a/Makefile b/Makefile index 1f234c0..0073dd2 100644 --- a/Makefile +++ b/Makefile @@ -1,207 +1,207 @@ -# =================================================================== -# deepctl Makefile - Development Tools -# =================================================================== -# -# Quick Start: -# make dev-setup # First time setup -# make dev # Daily development (format + lint + test) -# make help # Show organized help -# -# For new contributors: see README.md -# =================================================================== - -.PHONY: help -.DEFAULT_GOAL := help - -# =================================================================== -# HELP & INFO -# =================================================================== - -help: ## Show this help message - @echo "🔧 deepctl - Deepgram CLI Development Tools" - @echo "" - @echo "Usage: make [target]" - @echo "" - @echo "🚀 \033[1mQuick Start:\033[0m" - @echo " \033[36mdev-setup\033[0m Set up development environment" - @echo " \033[36mdev\033[0m Format, lint, and test (full dev cycle)" - @echo " \033[36mtest\033[0m Run tests" - @echo "" - @echo "🧪 \033[1mTesting:\033[0m" - @echo " \033[36mtest\033[0m Run tests (development)" - @echo " \033[36mcheck\033[0m Quick quality check (no tests)" - @echo "" - @echo "🔧 \033[1mCode Quality:\033[0m" - @echo " \033[36mformat\033[0m Auto-format code" - @echo " \033[36mlint\033[0m Run all linters" - @echo " \033[36mtypecheck\033[0m Run mypy type checker" - @echo "" - @echo "🧹 \033[1mUtilities:\033[0m" - @echo " \033[36mclean\033[0m Clean build artifacts" - @echo " \033[36minfo\033[0m Show project information" - @echo " \033[36mhelp-all\033[0m Show all available targets" - @echo "" - @echo "For more targets, run: \033[36mmake help-all\033[0m" - -help-all: ## Show all available targets - @echo "🔧 deepctl - All Available Targets" - @echo "" - @grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | grep -v '^\.' | sort | awk 'BEGIN {FS = ":.*?## "}; {printf " \033[36m%-25s\033[0m %s\n", $$1, $$2}' - -info: ## Show project information - @echo "🔧 deepctl - Deepgram CLI" - @echo "📁 $(shell pwd)" - @echo "🐍 Python: $(shell python --version 2>/dev/null || echo 'Not found')" - @echo "📦 uv: $(shell uv --version 2>/dev/null || echo 'Not found')" - @echo "🎯 Virtual env: $(shell echo $$VIRTUAL_ENV || echo 'Not activated')" - -# =================================================================== -# DEVELOPMENT SETUP -# =================================================================== - -dev-setup: ## Set up complete development environment - uv venv - uv pip install -e ".[dev]" - @echo "✅ Development environment ready!" - @echo "Activate with: source .venv/bin/activate (Linux/macOS) or .venv\\Scripts\\activate (Windows)" - -install: ## Install runtime dependencies only - uv pip install -e . - -install-dev: ## Install all development dependencies (includes testing) - uv pip install -e ".[dev]" - -# =================================================================== -# QUICK DEVELOPMENT WORKFLOWS -# =================================================================== - -dev: format lint-fix test ## Run full development cycle: format, fix lints, test - @echo "✅ Development cycle complete!" - -check: format-check lint-check typecheck ## Quick quality check (no tests) - @echo "✅ Quick check complete!" - -# =================================================================== -# TESTING -# =================================================================== - -test: ## Run tests with pytest - uv run pytest - -test-quick: ## Run tests quickly (no coverage) - uv run pytest -x - -test-verbose: ## Run tests with verbose output - uv run pytest -xvs - -test-watch: ## Run tests in watch mode (requires pytest-watch) - uv run ptw - -# =================================================================== -# CODE QUALITY -# =================================================================== - -## Formatting -format: ## Auto-format code with ruff - uv run ruff format src/ packages/**/src - -format-check: ## Check code formatting (no changes) - uv run ruff format --check src/ packages/**/src - -## Linting -lint: format-check lint-check typecheck ## Run all linters - @echo "✅ All linters passed!" - -lint-fix: ## Run ruff with auto-fix - uv run ruff check --fix src/ packages/**/src - -lint-check: ## Run ruff without fixes - uv run ruff check src/ packages/**/src - -## Type Checking -typecheck: ## Run mypy type checker - uv run mypy src/ packages/**/src - -## All Checks -quality: format-check lint-check typecheck ## Run all quality checks - -# =================================================================== -# RELEASE MANAGEMENT -# =================================================================== - -build: clean ## Build all packages into dist/ - @pip install build - @for pkg in . packages/*; do \ - if [ -f "$$pkg/pyproject.toml" ]; then \ - echo " Building $$pkg..."; \ - python -m build "$$pkg" --outdir dist/; \ - fi; \ - done - -verify-packages: ## Verify built packages with twine - @pip install twine - twine check dist/* - -readmes: ## Generate sub-package READMEs from pyproject.toml metadata - python3 scripts/generate_readmes.py - -readmes-check: ## Check sub-package READMEs are up to date - python3 scripts/generate_readmes.py --check - -floors-check: ## Check intra-workspace dependency floors (root must pin workspace versions) - python3 scripts/check_dependency_floors.py - -floors-fix: ## Pin root dependency floors to the current workspace versions - python3 scripts/check_dependency_floors.py --fix - -# =================================================================== -# RUNNING THE CLI -# =================================================================== - -run: ## Run the CLI (show help) - uv run python -m deepctl --help - -run-version: ## Show CLI version - uv run python -m deepctl --version - -# =================================================================== -# CLEANUP -# =================================================================== - -clean: ## Clean all build artifacts and caches - rm -rf build/ - rm -rf dist/ - rm -rf *.egg-info/ - rm -rf packages/**/*.egg-info/ - find . -type d -name __pycache__ -exec rm -rf {} + - find . -type f -name "*.pyc" -delete - rm -rf .pytest_cache/ - rm -rf .coverage - rm -rf htmlcov/ - rm -rf .mypy_cache/ - rm -rf .ruff_cache/ - -clean-env: ## Remove virtual environment - rm -rf .venv/ - -# =================================================================== -# PRE-COMMIT HOOKS -# =================================================================== - -pre-commit-install: ## Install pre-commit hooks - uv run pre-commit install - -pre-commit-run: ## Run pre-commit on all files - uv run pre-commit run --all-files - -# =================================================================== -# ALIASES (for convenience) -# =================================================================== - -.PHONY: t tl q f l - -t: test ## Alias for test -tl: lint ## Alias for lint -q: check ## Alias for check (quick) -f: format ## Alias for format -l: lint-fix ## Alias for lint-fix +# =================================================================== +# deepctl Makefile - Development Tools +# =================================================================== +# +# Quick Start: +# make dev-setup # First time setup +# make dev # Daily development (format + lint + test) +# make help # Show organized help +# +# For new contributors: see README.md +# =================================================================== + +.PHONY: help +.DEFAULT_GOAL := help + +# =================================================================== +# HELP & INFO +# =================================================================== + +help: ## Show this help message + @echo "🔧 deepctl - Deepgram CLI Development Tools" + @echo "" + @echo "Usage: make [target]" + @echo "" + @echo "🚀 \033[1mQuick Start:\033[0m" + @echo " \033[36mdev-setup\033[0m Set up development environment" + @echo " \033[36mdev\033[0m Format, lint, and test (full dev cycle)" + @echo " \033[36mtest\033[0m Run tests" + @echo "" + @echo "🧪 \033[1mTesting:\033[0m" + @echo " \033[36mtest\033[0m Run tests (development)" + @echo " \033[36mcheck\033[0m Quick quality check (no tests)" + @echo "" + @echo "🔧 \033[1mCode Quality:\033[0m" + @echo " \033[36mformat\033[0m Auto-format code" + @echo " \033[36mlint\033[0m Run all linters" + @echo " \033[36mtypecheck\033[0m Run mypy type checker" + @echo "" + @echo "🧹 \033[1mUtilities:\033[0m" + @echo " \033[36mclean\033[0m Clean build artifacts" + @echo " \033[36minfo\033[0m Show project information" + @echo " \033[36mhelp-all\033[0m Show all available targets" + @echo "" + @echo "For more targets, run: \033[36mmake help-all\033[0m" + +help-all: ## Show all available targets + @echo "🔧 deepctl - All Available Targets" + @echo "" + @grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | grep -v '^\.' | sort | awk 'BEGIN {FS = ":.*?## "}; {printf " \033[36m%-25s\033[0m %s\n", $$1, $$2}' + +info: ## Show project information + @echo "🔧 deepctl - Deepgram CLI" + @echo "📁 $(shell pwd)" + @echo "🐍 Python: $(shell python --version 2>/dev/null || echo 'Not found')" + @echo "📦 uv: $(shell uv --version 2>/dev/null || echo 'Not found')" + @echo "🎯 Virtual env: $(shell echo $$VIRTUAL_ENV || echo 'Not activated')" + +# =================================================================== +# DEVELOPMENT SETUP +# =================================================================== + +dev-setup: ## Set up complete development environment + uv venv + uv pip install -e ".[dev]" + @echo "✅ Development environment ready!" + @echo "Activate with: source .venv/bin/activate (Linux/macOS) or .venv\\Scripts\\activate (Windows)" + +install: ## Install runtime dependencies only + uv pip install -e . + +install-dev: ## Install all development dependencies (includes testing) + uv pip install -e ".[dev]" + +# =================================================================== +# QUICK DEVELOPMENT WORKFLOWS +# =================================================================== + +dev: format lint-fix test ## Run full development cycle: format, fix lints, test + @echo "✅ Development cycle complete!" + +check: format-check lint-check typecheck ## Quick quality check (no tests) + @echo "✅ Quick check complete!" + +# =================================================================== +# TESTING +# =================================================================== + +test: ## Run tests with pytest + uv run pytest + +test-quick: ## Run tests quickly (no coverage) + uv run pytest -x + +test-verbose: ## Run tests with verbose output + uv run pytest -xvs + +test-watch: ## Run tests in watch mode (requires pytest-watch) + uv run ptw + +# =================================================================== +# CODE QUALITY +# =================================================================== + +## Formatting +format: ## Auto-format code with ruff + uv run ruff format src/ packages/**/src scripts/ + +format-check: ## Check code formatting (no changes) + uv run ruff format --check src/ packages/**/src scripts/ + +## Linting +lint: format-check lint-check typecheck ## Run all linters + @echo "✅ All linters passed!" + +lint-fix: ## Run ruff with auto-fix + uv run ruff check --fix src/ packages/**/src scripts/ + +lint-check: ## Run ruff without fixes + uv run ruff check src/ packages/**/src scripts/ + +## Type Checking +typecheck: ## Run mypy type checker + uv run mypy src/ packages/**/src + +## All Checks +quality: format-check lint-check typecheck ## Run all quality checks + +# =================================================================== +# RELEASE MANAGEMENT +# =================================================================== + +build: clean ## Build all packages into dist/ + @pip install build + @for pkg in . packages/*; do \ + if [ -f "$$pkg/pyproject.toml" ]; then \ + echo " Building $$pkg..."; \ + python -m build "$$pkg" --outdir dist/; \ + fi; \ + done + +verify-packages: ## Verify built packages with twine + @pip install twine + twine check dist/* + +readmes: ## Generate sub-package READMEs from pyproject.toml metadata + python3 scripts/generate_readmes.py + +readmes-check: ## Check sub-package READMEs are up to date + python3 scripts/generate_readmes.py --check + +floors-check: ## Check intra-workspace dependency floors (root must pin workspace versions) + python3 scripts/check_dependency_floors.py + +floors-fix: ## Pin root dependency floors to the current workspace versions + python3 scripts/check_dependency_floors.py --fix + +# =================================================================== +# RUNNING THE CLI +# =================================================================== + +run: ## Run the CLI (show help) + uv run python -m deepctl --help + +run-version: ## Show CLI version + uv run python -m deepctl --version + +# =================================================================== +# CLEANUP +# =================================================================== + +clean: ## Clean all build artifacts and caches + rm -rf build/ + rm -rf dist/ + rm -rf *.egg-info/ + rm -rf packages/**/*.egg-info/ + find . -type d -name __pycache__ -exec rm -rf {} + + find . -type f -name "*.pyc" -delete + rm -rf .pytest_cache/ + rm -rf .coverage + rm -rf htmlcov/ + rm -rf .mypy_cache/ + rm -rf .ruff_cache/ + +clean-env: ## Remove virtual environment + rm -rf .venv/ + +# =================================================================== +# PRE-COMMIT HOOKS +# =================================================================== + +pre-commit-install: ## Install pre-commit hooks + uv run pre-commit install + +pre-commit-run: ## Run pre-commit on all files + uv run pre-commit run --all-files + +# =================================================================== +# ALIASES (for convenience) +# =================================================================== + +.PHONY: t tl q f l + +t: test ## Alias for test +tl: lint ## Alias for lint +q: check ## Alias for check (quick) +f: format ## Alias for format +l: lint-fix ## Alias for lint-fix diff --git a/scripts/build_standalone.py b/scripts/build_standalone.py index a2a0449..5a8941d 100644 --- a/scripts/build_standalone.py +++ b/scripts/build_standalone.py @@ -1,7 +1,7 @@ #!/usr/bin/env python3 """Build a standalone deepctl binary for testing system installations.""" -import os +import importlib.util import shutil import subprocess import sys @@ -18,9 +18,7 @@ def build_standalone(): print("🔨 Building standalone deepctl binary...") # Install PyInstaller if needed - try: - import PyInstaller - except ImportError: + if importlib.util.find_spec("PyInstaller") is None: print("📦 Installing PyInstaller...") subprocess.run( [sys.executable, "-m", "pip", "install", "pyinstaller"], check=True @@ -132,9 +130,7 @@ def build_standalone(): print(" ./dist_standalone/deepctl plugin search") print(" ./dist_standalone/deepctl plugin install deepctl-plugin-example") print("\n💡 The standalone binary should detect as 'system' installation") - print( - " and create an isolated plugin environment at ~/.deepctl/plugins/" - ) + print(" and create an isolated plugin environment at ~/.deepctl/plugins/") if __name__ == "__main__": diff --git a/scripts/generate_readmes.py b/scripts/generate_readmes.py index 63262ef..37e58fe 100644 --- a/scripts/generate_readmes.py +++ b/scripts/generate_readmes.py @@ -19,10 +19,7 @@ try: import tomli as tomllib # type: ignore[no-redef] except ModuleNotFoundError: - print( - "Python 3.11+ required (tomllib), " - "or install tomli: pip install tomli" - ) + print("Python 3.11+ required (tomllib), or install tomli: pip install tomli") sys.exit(1) ROOT = Path(__file__).resolve().parent.parent @@ -130,8 +127,7 @@ def render_command_readme( lines = [ f"# {name}", "", - "> Part of [deepctl](https://github.com/deepgram/cli)" - " — Official Deepgram CLI", + "> Part of [deepctl](https://github.com/deepgram/cli) — Official Deepgram CLI", "", description, "", @@ -159,9 +155,7 @@ def render_command_readme( else: lines.append("No external dependencies.") - lines.extend( - ["", "## License", "", "MIT — see [LICENSE](../../LICENSE)", ""] - ) + lines.extend(["", "## License", "", "MIT — see [LICENSE](../../LICENSE)", ""]) return "\n".join(lines) @@ -176,8 +170,7 @@ def render_debug_subcommand_readme( lines = [ f"# {name}", "", - "> Part of [deepctl](https://github.com/deepgram/cli)" - " — Official Deepgram CLI", + "> Part of [deepctl](https://github.com/deepgram/cli) — Official Deepgram CLI", "", description, "", @@ -207,9 +200,7 @@ def render_debug_subcommand_readme( else: lines.append("No external dependencies.") - lines.extend( - ["", "## License", "", "MIT — see [LICENSE](../../LICENSE)", ""] - ) + lines.extend(["", "## License", "", "MIT — see [LICENSE](../../LICENSE)", ""]) return "\n".join(lines) @@ -223,8 +214,7 @@ def render_core_readme( lines = [ f"# {name}", "", - "> Part of [deepctl](https://github.com/deepgram/cli)" - " — Official Deepgram CLI", + "> Part of [deepctl](https://github.com/deepgram/cli) — Official Deepgram CLI", "", description, "", @@ -243,9 +233,7 @@ def render_core_readme( else: lines.append("No external dependencies.") - lines.extend( - ["", "## License", "", "MIT — see [LICENSE](../../LICENSE)", ""] - ) + lines.extend(["", "## License", "", "MIT — see [LICENSE](../../LICENSE)", ""]) return "\n".join(lines) @@ -269,9 +257,7 @@ def generate_readme(package_dir: Path) -> str: name, description, entry_points, external_deps, package_dir ) else: - return render_core_readme( - name, description, external_deps, package_dir - ) + return render_core_readme(name, description, external_deps, package_dir) def get_package_dirs(single: str | None = None) -> list[Path]: @@ -307,13 +293,9 @@ def get_all_packages() -> list[dict]: { "dir_name": pkg_dir.name, "name": project["name"], - "description": project.get( - "description", project["name"] - ), + "description": project.get("description", project["name"]), "commands": eps.get("deepctl.commands", {}), - "debug_subcommands": eps.get( - "deepctl.subcommands.debug", {} - ), + "debug_subcommands": eps.get("deepctl.subcommands.debug", {}), } ) return packages @@ -328,9 +310,7 @@ def generate_commands_section(packages: list[dict]) -> str: for cmd_name in pkg["commands"]: rows.append((f"`deepctl {cmd_name}`", pkg["description"])) for cmd_name in pkg["debug_subcommands"]: - rows.append( - (f"`deepctl debug {cmd_name}`", pkg["description"]) - ) + rows.append((f"`deepctl debug {cmd_name}`", pkg["description"])) rows.sort(key=lambda r: r[0]) lines = [ "| Command | Description |", @@ -349,8 +329,7 @@ def generate_packages_section(packages: list[dict]) -> str: ] for pkg in sorted(packages, key=lambda p: p["name"]): lines.append( - f"| [`{pkg['name']}`](packages/{pkg['dir_name']})" - f" | {pkg['description']} |" + f"| [`{pkg['name']}`](packages/{pkg['dir_name']}) | {pkg['description']} |" ) return "\n".join(lines) @@ -365,9 +344,7 @@ def tree_line(prefix: str, name: str, desc: str) -> str: return f"{line}{' ' * padding}# {desc}" lines = ["```", "cli/"] - lines.append( - tree_line("├── ", "src/deepctl/", "Main CLI entry point") - ) + lines.append(tree_line("├── ", "src/deepctl/", "Main CLI entry point")) lines.append("├── packages/") sorted_pkgs = sorted(packages, key=lambda p: p["dir_name"]) @@ -382,19 +359,13 @@ def tree_line(prefix: str, name: str, desc: str) -> str: ) ) - lines.append( - tree_line("├── ", "tests/", "Integration tests") - ) - lines.append( - tree_line("└── ", "Makefile", "Development tasks") - ) + lines.append(tree_line("├── ", "tests/", "Integration tests")) + lines.append(tree_line("└── ", "Makefile", "Development tasks")) lines.append("```") return "\n".join(lines) -def replace_section( - content: str, section: str, replacement: str -) -> str: +def replace_section(content: str, section: str, replacement: str) -> str: """Replace content between BEGIN:section and END:section markers.""" pattern = re.compile( rf"(\n)" @@ -409,9 +380,7 @@ def repl(m: re.Match) -> str: return pattern.sub(repl, content) -def update_root_readme( - *, dry_run: bool = False, check: bool = False -) -> bool: +def update_root_readme(*, dry_run: bool = False, check: bool = False) -> bool: """Update auto-generated sections in the root README.md. Returns True if the file was (or needs to be) changed. @@ -506,9 +475,7 @@ def main(): # Update root README sections (skip when targeting a single package) if not args.package: - root_changed = update_root_readme( - dry_run=args.dry_run, check=args.check - ) + root_changed = update_root_readme(dry_run=args.dry_run, check=args.check) if root_changed: stale.append("README.md") @@ -522,10 +489,7 @@ def main(): if not args.check and not args.dry_run: total = len(package_dirs) + (0 if args.package else 1) updated = len(stale) - print( - f"\nDone: {updated} updated, " - f"{total - updated} already current" - ) + print(f"\nDone: {updated} updated, {total - updated} already current") if __name__ == "__main__": From 021e0a16aef5974da006da6aa1a7ae4e864bf980 Mon Sep 17 00:00:00 2001 From: Greg Holmes Date: Wed, 19 Aug 2026 15:12:56 +0100 Subject: [PATCH 12/21] ci: add an All checks rollup for branch protection main currently has no required status checks at all -- the ruleset carries only deletion and non_fast_forward -- so the floor guard added in this branch is advisory, and the "unmergeable instead of memorable" claim does not hold as configured. Requiring the matrix contexts directly would mean editing repo settings every time a Python version or OS moves, and a required context that stops reporting blocks every merge until someone notices. This rollup needs test, lint, floors and build-test, so branch protection needs exactly one context. `if: always()` is load-bearing: without it the job is skipped when a prerequisite fails, and a skipped required check never reports failure, it just stalls. Run always and fail on anything that is not success. --- .github/workflows/test.yml | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 79e74ea..746ff55 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -103,3 +103,31 @@ jobs: - name: Verify packages run: make verify-packages + + # Single rollup so branch protection needs exactly one required check. + # Requiring the matrix contexts directly means editing repo settings every + # time a Python version or OS is added or dropped, and a required context + # that stops reporting blocks every merge until someone notices. + # + # `if: always()` is load-bearing: without it this job is *skipped* when a + # prerequisite fails, and a skipped required check never reports failure -- + # it just stalls. Run always, then fail on anything that is not success, so + # cancelled and skipped prerequisites are failures here too. + all-checks: + name: All checks + needs: [test, lint, floors, build-test] + if: always() + runs-on: ubuntu-latest + steps: + - name: Report prerequisite results + run: | + echo "test: ${{ needs.test.result }}" + echo "lint: ${{ needs.lint.result }}" + echo "floors: ${{ needs.floors.result }}" + echo "build-test: ${{ needs.build-test.result }}" + + - name: Fail unless every prerequisite succeeded + if: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') || contains(needs.*.result, 'skipped') }} + run: | + echo "::error::One or more required jobs did not succeed" + exit 1 From 50559357b718da472a59a7d42f0b920ddc20f76a Mon Sep 17 00:00:00 2001 From: Greg Holmes Date: Wed, 19 Aug 2026 15:12:56 +0100 Subject: [PATCH 13/21] chore(release): give verify-published a 20-minute ceiling The poll exits the moment resolution succeeds, so the ceiling only costs anything when it is hit -- which makes a higher number pure insurance with no happy-path cost. Propagation is normally seconds, but a timeout strands a published release unannounced until someone re-runs the job, and there is no reason to run that close to the edge. --- .github/workflows/release.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 2f4b8f0..b8743f6 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -166,16 +166,19 @@ jobs: VERSION="${TAG_NAME#v}" python -m venv /tmp/verify /tmp/verify/bin/pip install --quiet --upgrade pip - for i in $(seq 1 30); do + # The loop exits the moment resolution succeeds, so the ceiling only + # costs anything when it is actually hit. Propagation is normally + # seconds; 20 minutes is insurance, not an expected wait. + for i in $(seq 1 60); do if /tmp/verify/bin/pip install --dry-run --no-cache-dir \ "deepctl==${VERSION}" >/dev/null 2>&1; then echo "deepctl==${VERSION} resolves with its full closure" exit 0 fi - echo "Waiting for deepctl==${VERSION} to resolve (${i}/30)..." + echo "Waiting for deepctl==${VERSION} to resolve (${i}/60)..." sleep 20 done - echo "::error::deepctl==${VERSION} did not resolve from PyPI after 10 minutes -- a dependency is missing or the index has not propagated. Re-run this job once PyPI has propagated; mark-latest, deploy-web and bump-brew-formula re-evaluate and will then run." + echo "::error::deepctl==${VERSION} did not resolve from PyPI after 20 minutes -- a dependency is missing or the index has not propagated. Re-run this job once PyPI has propagated; mark-latest, deploy-web and bump-brew-formula re-evaluate and will then run." exit 1 env: TAG_NAME: ${{ needs.release-please.outputs.tag_name }} From 0bc15bb5cd61b2b1a683417bf9084f904e95e6eb Mon Sep 17 00:00:00 2001 From: Greg Holmes Date: Wed, 19 Aug 2026 15:12:56 +0100 Subject: [PATCH 14/21] docs(contributing): document the release runbook and the delivery manifest sync-release-pr pushes with GITHUB_TOKEN, which by design retriggers nothing, so a release PR shows checks from the bot's first commit and sits red on content that is now correct. The fix is a runbook step, not infrastructure: land release-notes edits as a commit on the branch (retriggers) rather than a PR description edit (fires pull_request: edited, outside the default trigger types). This matters more once required checks land -- a check that is routinely red on the PR type that matters most trains override habits. Also state why the root pyproject dependency in "Adding a New Command" is load-bearing rather than bookkeeping, and what verify-published does when it times out. --- CONTRIBUTING.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b625b65..492c985 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -64,6 +64,34 @@ Or manually create a package under `packages/` following the existing pattern. E Then run `make readmes` to update all READMEs. +The root `pyproject.toml` dependency is not optional bookkeeping — it is the +delivery manifest. `pip install --upgrade deepctl` (what `dg update` runs) +only installs what root depends on, so a published package missing from that +list never reaches anyone. `make floors-check` fails on both that omission and +a floor left below the workspace version; run `make floors-fix` to pin floors. + +### Releasing + +Releases are driven by release-please. Two things are worth knowing before you +run one: + +**Land your release-notes edits as a commit, not a PR description edit.** +`sync-release-pr` regenerates `uv.lock` and the root dependency floors on the +release branch automatically, but it pushes with `GITHUB_TOKEN`, which by +design retriggers nothing — so the PR's checks still reflect the bot's first +commit and stay red on content that is now correct. Editing `CHANGELOG.md` on +the branch is a commit and retriggers them. Editing the PR *description* does +not: that fires `pull_request: edited`, which is outside the default trigger +types. If a release ever needs to go out unattended, switch that push to a +dedicated PAT or GitHub App token. + +**Nothing advertises a release until it is installable.** `verify-published` +polls PyPI until `pip install deepctl==X` resolves its full closure, then +installs it for real and runs the CLI. `mark-latest`, `deploy-web`, and the +Homebrew bump all wait on it. If it times out, PyPI has not propagated or a +sub-package failed to publish — re-run that one job once PyPI has caught up +and the three downstream jobs re-evaluate. + ### Testing - Unit tests: `packages/*/tests/unit/` From 7ab748fc91d319e608819e192322aa6a6f925d00 Mon Sep 17 00:00:00 2001 From: Greg Holmes Date: Thu, 20 Aug 2026 11:50:19 +0100 Subject: [PATCH 15/21] fix(ci): read project files as UTF-8 in the floor guard read_text()/write_text() without an explicit encoding use the locale codec, which is cp1252 on Windows. A single non-ASCII character in any package description or author name would then crash `make floors-check` with a UnicodeDecodeError for a developer on Windows. Latent rather than live -- the floors CI job runs on ubuntu-latest and no pyproject currently holds a non-ASCII byte -- but the same omission in a test added on the stacked branch did fail the Windows matrix, so fix the pattern here too rather than waiting for someone to add an em-dash. --- scripts/check_dependency_floors.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/scripts/check_dependency_floors.py b/scripts/check_dependency_floors.py index ebe3605..5609bcb 100644 --- a/scripts/check_dependency_floors.py +++ b/scripts/check_dependency_floors.py @@ -62,18 +62,20 @@ def workspace_versions() -> dict[str, str]: """Map package name -> current workspace version, per the manifest.""" versions: dict[str, str] = {} - for path in json.loads(MANIFEST.read_text()): + for path in json.loads(MANIFEST.read_text(encoding="utf-8")): pyproject = REPO / ( "pyproject.toml" if path == "." else f"{path}/pyproject.toml" ) - project = tomllib.loads(pyproject.read_text())["project"] + project = tomllib.loads(pyproject.read_text(encoding="utf-8"))["project"] versions[project["name"]] = project["version"] return versions def floors(pyproject: Path) -> list[tuple[str, str, str]]: """Yield (name, floor, raw-spec) for each intra-workspace dependency.""" - deps = tomllib.loads(pyproject.read_text())["project"].get("dependencies", []) + deps = tomllib.loads(pyproject.read_text(encoding="utf-8"))["project"].get( + "dependencies", [] + ) out = [] for dep in deps: m = re.match(r"^(deepctl[\w-]*)>=([0-9][0-9.]*)", dep) @@ -108,7 +110,7 @@ def main() -> int: # Rule 1: root floors == workspace versions. root = REPO / "pyproject.toml" - root_text = root.read_text() + root_text = root.read_text(encoding="utf-8") fixed = root_text root_floors = floors(root) for name, floor, raw in root_floors: @@ -133,7 +135,7 @@ def main() -> int: " (published fixes will not be delivered by pip upgrades)" ) if args.fix and fixed != root_text: - root.write_text(fixed) + root.write_text(fixed, encoding="utf-8") print(f"fixed: root floors pinned to workspace versions in {root}") # Rule 2: every sub-package floor must be satisfiable at co-release. From fe94ced895687966d66b4f34bbcbe0acb2813072 Mon Sep 17 00:00:00 2001 From: Corey Weathers Date: Sun, 20 Sep 2026 10:32:52 -0400 Subject: [PATCH 16/21] docs(readme): drop the stdout/stderr claim until #107 lands The exit-code section promised that status and error messages go to stderr and that `dg ... -o json | jq` survives a failure. On this branch they do not: auth.py, client.py and timing.py still print through module-level stdout consoles, so `dg -o json keys` with no credentials writes four lines of prose to stdout and leaves stderr empty. The code that makes the sentence true is in #107, which is stacked on this branch. Rather than rely on nobody cutting a release in the window between the two merges, the claim moves to the PR that delivers it. The exit-code table and the CI warning stay: both are true here. Co-Authored-By: Claude Opus 5 --- README.md | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 8ff3363..4d93a96 100644 --- a/README.md +++ b/README.md @@ -334,11 +334,8 @@ Note that `dg` reports `2` for an interrupt rather than the shell's conventional `130`, so the code is the same whether the cancellation came from Ctrl-C or from declining a prompt. -Human-readable status and error messages go to stderr; stdout carries only the -result, so `dg ... -o json | jq` stays parseable even when a command fails — a -failure arrives there as a payload with `"status": "error"`. If a CI step -relied on `dg` always exiting `0` (every command did, before 0.3.0), it will -now fail where it previously passed silently. +If a CI step relied on `dg` always exiting `0` (every command did, before +0.3.0), it will now fail where it previously passed silently. ### Forcing non-interactive mode From f58ac3f90cbb3308af434c8864e4c9e34c487c4d Mon Sep 17 00:00:00 2001 From: Corey Weathers Date: Sun, 20 Sep 2026 10:33:09 -0400 Subject: [PATCH 17/21] chore(release): make the publish-gate recovery instruction work The gate told an operator to re-run this one job after a PyPI propagation delay, promising mark-latest, deploy-web and bump-brew-formula would then re-evaluate. They do not: re-running a single job in GitHub Actions does not run its dependents, and "Re-run failed jobs" splices in the earlier run's outputs rather than re-running them (actions/runner#1961, which reproduces in workflows calling reusable workflows -- deploy-web does). Following the instruction left the release published but unannounced, with no signal that anything had been missed. Both the workflow error and the CONTRIBUTING runbook now say "Re-run all jobs", and why that is safe (the publish step uses skip-existing). Four smaller things in the same job: - A timeout produced no diagnosis: every poll discarded its output, so the operator got one generic line and had to reproduce by hand. The final attempt now runs with output visible, so pip names the requirement it could not satisfy. - `permissions: {}`: the job installs from public PyPI and writes nothing, so it has no business inheriting contents+pull-requests write. - `timeout-minutes: 30`, a ceiling for a wedged runner around the poll's own 20-minute bound, so three release-advertising jobs can never queue behind it indefinitely. - bump-brew-formula's own 10-minute PyPI JSON poll is gone. It is behind verify-published now, which does strictly more, so it could only cost time. The sync job runs the floor guard through `uv run python` rather than bare `python3`, so it cannot land on an interpreter without tomllib. Co-Authored-By: Claude Opus 5 --- .github/workflows/release.yml | 44 ++++++++++++++++------------------- CONTRIBUTING.md | 6 +++-- 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b8743f6..01ab6d6 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -68,9 +68,9 @@ jobs: - name: Regenerate uv.lock and pin root floors run: | set -euo pipefail - python3 scripts/check_dependency_floors.py --fix + uv run python scripts/check_dependency_floors.py --fix uv lock - python3 scripts/check_dependency_floors.py + uv run python scripts/check_dependency_floors.py - name: Commit and push if changed run: | @@ -152,6 +152,13 @@ jobs: needs.release-please.outputs.release_created == 'true' && startsWith(needs.release-please.outputs.tag_name, 'v') runs-on: ubuntu-latest + # Reads nothing and writes nothing: it installs from public PyPI. Drop the + # workflow-level contents/pull-requests write it would otherwise inherit. + permissions: {} + # The poll is bounded at 20 minutes on its own; this is the belt-and-braces + # ceiling for a hung runner or a wedged pip, so three release-advertising + # jobs can never sit queued behind this one indefinitely. + timeout-minutes: 30 steps: - name: Set up Python uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 @@ -178,7 +185,13 @@ jobs: echo "Waiting for deepctl==${VERSION} to resolve (${i}/60)..." sleep 20 done - echo "::error::deepctl==${VERSION} did not resolve from PyPI after 20 minutes -- a dependency is missing or the index has not propagated. Re-run this job once PyPI has propagated; mark-latest, deploy-web and bump-brew-formula re-evaluate and will then run." + # Every attempt above discarded its output, so the operator would + # otherwise get a bare error line and have to reproduce by hand. + # Run once more with the output visible: pip's resolver says which + # requirement it could not satisfy. + echo "Resolution still failing. Final attempt, with output:" + /tmp/verify/bin/pip install --dry-run --no-cache-dir "deepctl==${VERSION}" || true + echo "::error::deepctl==${VERSION} did not resolve from PyPI after 20 minutes -- a dependency is missing or the index has not propagated. Once PyPI has propagated, use 'Re-run all jobs' on this workflow run -- re-running only this job leaves mark-latest, deploy-web and bump-brew-formula skipped. Re-running all jobs is safe: the PyPI publish step uses skip-existing." exit 1 env: TAG_NAME: ${{ needs.release-please.outputs.tag_name }} @@ -264,27 +277,10 @@ jobs: with: python-version: "3.13" - - name: Wait for new deepctl version on PyPI - run: | - set -euo pipefail - VERSION="${TAG_NAME#v}" - # Poll the PyPI JSON API rather than `pip index versions`. The - # JSON endpoint flips the moment a release is published, while - # the simple index pip queries can lag 5-15 minutes behind a - # successful publish (CDN caching of project metadata). - URL="https://pypi.org/pypi/deepctl/${VERSION}/json" - for i in $(seq 1 30); do - if [ "$(curl -fsS -o /dev/null -w '%{http_code}' "${URL}")" = "200" ]; then - echo "deepctl==${VERSION} is live on PyPI" - exit 0 - fi - echo "Waiting for deepctl==${VERSION} on PyPI (${i}/30)..." - sleep 20 - done - echo "::error::deepctl==${VERSION} did not appear on PyPI after 10 minutes" - exit 1 - env: - TAG_NAME: ${{ needs.release-please.outputs.tag_name }} + # No PyPI poll here: verify-published is in `needs`, and it does not + # finish until `pip install deepctl==X` resolves its full closure and + # the installed CLI runs. A second, weaker poll behind that gate can + # only cost time. - name: Checkout homebrew-tap uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 492c985..9a6f453 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -89,8 +89,10 @@ dedicated PAT or GitHub App token. polls PyPI until `pip install deepctl==X` resolves its full closure, then installs it for real and runs the CLI. `mark-latest`, `deploy-web`, and the Homebrew bump all wait on it. If it times out, PyPI has not propagated or a -sub-package failed to publish — re-run that one job once PyPI has caught up -and the three downstream jobs re-evaluate. +sub-package failed to publish — re-run all jobs on that workflow run once +PyPI has caught up; re-running only the failed job leaves the three +downstream jobs skipped. Re-running all jobs is safe: the publish step uses +`skip-existing`. ### Testing From 578f677c10015ffc36e888bda589e0776df372ff Mon Sep 17 00:00:00 2001 From: Corey Weathers Date: Sun, 20 Sep 2026 10:33:21 -0400 Subject: [PATCH 18/21] ci: type-check scripts/ and drop the inherited write permissions scripts/ came under ruff format and lint last week but not mypy, so the new release-gating guard was linted and never type-checked. Adding it to the mypy paths (Makefile, test.yml, and the `files` setting so a bare `mypy` agrees) surfaced 25 errors across three scripts; all are fixed here -- generic parameters on dict/list annotations, two missing return types, and the tomllib import restructured as a `sys.version_info` branch so mypy resolves it under python_version = "3.10". tomli, the pre-3.11 fallback, gets an ignore_missing_imports override. test.yml gains `permissions: {contents: read}`: every job here only reads the repository, and the new floors and All-checks jobs were inheriting whatever the repo default happens to be. The floors job gains uv, because `make floors-check` now runs the guard through `uv run python` rather than bare `python3` -- the guard needs tomllib, and an interpreter that happens to be on PATH is not a guarantee. `packaging` (PEP 440 comparisons in the guard) and tomli for 3.10 are declared rather than leaned on as transitives. Co-Authored-By: Claude Opus 5 --- .github/workflows/test.yml | 10 +++++++- Makefile | 6 ++--- pyproject.toml | 16 ++++++++++++- scripts/build_standalone.py | 2 +- scripts/generate_readmes.py | 47 +++++++++++++++++++++---------------- uv.lock | 12 ++++++++-- 6 files changed, 65 insertions(+), 28 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 746ff55..102a946 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -4,6 +4,11 @@ on: push: pull_request: +# Every job here only reads the repository: no job comments, labels, or +# pushes. Least privilege rather than whatever the repo default happens to be. +permissions: + contents: read + jobs: test: name: Test Python ${{ matrix.python-version }} / ${{ matrix.os }} @@ -62,7 +67,7 @@ jobs: run: uv run ruff check src/ packages/*/src scripts/ - name: Type check - run: uv run mypy src/ packages/*/src + run: uv run mypy src/ packages/*/src scripts/ floors: name: Dependency floors @@ -75,6 +80,9 @@ jobs: with: python-version: "3.12" + - name: Install uv + uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0 + # Root floors must equal workspace versions or `pip install --upgrade # deepctl` (what `dg update` runs) silently skips the packages whose # fixes the release advertises. See scripts/check_dependency_floors.py. diff --git a/Makefile b/Makefile index 0073dd2..49b13b8 100644 --- a/Makefile +++ b/Makefile @@ -120,7 +120,7 @@ lint-check: ## Run ruff without fixes ## Type Checking typecheck: ## Run mypy type checker - uv run mypy src/ packages/**/src + uv run mypy src/ packages/**/src scripts/ ## All Checks quality: format-check lint-check typecheck ## Run all quality checks @@ -149,10 +149,10 @@ readmes-check: ## Check sub-package READMEs are up to date python3 scripts/generate_readmes.py --check floors-check: ## Check intra-workspace dependency floors (root must pin workspace versions) - python3 scripts/check_dependency_floors.py + uv run python scripts/check_dependency_floors.py floors-fix: ## Pin root dependency floors to the current workspace versions - python3 scripts/check_dependency_floors.py --fix + uv run python scripts/check_dependency_floors.py --fix # =================================================================== # RUNNING THE CLI diff --git a/pyproject.toml b/pyproject.toml index ad16a44..d629115 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -87,6 +87,7 @@ dev = [ # Code Quality "ruff>=0.8.0", "mypy>=1.0.0", + "packaging>=23.0", # Type Stubs "types-PyYAML>=6.0.0", "types-requests>=2.31.0", @@ -128,7 +129,13 @@ include = ["deepctl*"] [tool.mypy] python_version = "3.10" strict = true -files = "src/,packages/*/src" +files = "src/,packages/*/src,scripts/" + +[[tool.mypy.overrides]] +module = ["tomli"] +# The pre-3.11 fallback for tomllib in scripts/. It is not installed, and +# mypy checks that branch because python_version is pinned to 3.10. +ignore_missing_imports = true [[tool.mypy.overrides]] module = "deepctl_cmd_mcp.*" @@ -232,6 +239,10 @@ members = ["packages/*"] [dependency-groups] testing = [ "pytest>=7.0.0", + # scripts/check_dependency_floors.py reads pyproject files; on 3.10 it + # falls back to tomli, and tests/unit/test_dependency_floors.py drives + # the real script on every version in the CI matrix. + "tomli>=2.0; python_version < '3.11'", "pytest-asyncio>=0.21.0", "pytest-cov>=4.0.0", "pytest-mock>=3.10.0", @@ -244,6 +255,9 @@ dev = [ # Code Quality "ruff>=0.8.0", "mypy>=1.0.0", + # PEP 440 comparisons in scripts/check_dependency_floors.py. Declared + # rather than leaned on as a transitive of build/twine. + "packaging>=23.0", # Type Stubs "types-pyyaml>=6.0.12.20250516", "types-requests>=2.32.4.20250611", diff --git a/scripts/build_standalone.py b/scripts/build_standalone.py index 5a8941d..acb75c6 100644 --- a/scripts/build_standalone.py +++ b/scripts/build_standalone.py @@ -8,7 +8,7 @@ from pathlib import Path -def build_standalone(): +def build_standalone() -> None: """Build a standalone deepctl binary using PyInstaller.""" # Check if we're in the right directory if not Path("pyproject.toml").exists(): diff --git a/scripts/generate_readmes.py b/scripts/generate_readmes.py index 37e58fe..cbc2b55 100644 --- a/scripts/generate_readmes.py +++ b/scripts/generate_readmes.py @@ -12,14 +12,18 @@ import re import sys from pathlib import Path +from typing import Any -try: +if sys.version_info >= (3, 11): import tomllib -except ModuleNotFoundError: +else: # pragma: no cover - only taken on Python 3.10 try: - import tomli as tomllib # type: ignore[no-redef] + import tomli as tomllib except ModuleNotFoundError: - print("Python 3.11+ required (tomllib), or install tomli: pip install tomli") + print( + "Python 3.11+ required (tomllib), or install tomli: pip install tomli", + file=sys.stderr, + ) sys.exit(1) ROOT = Path(__file__).resolve().parent.parent @@ -32,13 +36,14 @@ INTERNAL_PREFIXES = ("deepctl-core", "deepctl-cmd-", "deepctl-shared-") -def load_pyproject(package_dir: Path) -> dict: +def load_pyproject(package_dir: Path) -> dict[str, Any]: """Load and return parsed pyproject.toml.""" with open(package_dir / "pyproject.toml", "rb") as f: - return tomllib.load(f) + data: dict[str, Any] = tomllib.load(f) + return data -def classify_package(name: str, data: dict) -> str: +def classify_package(name: str, data: dict[str, Any]) -> str: """Classify a package as 'command', 'debug-subcommand', or 'core'.""" if re.match(r"^deepctl-cmd-debug-.+$", name): return "debug-subcommand" @@ -61,7 +66,7 @@ def get_external_deps(deps: list[str]) -> list[str]: return external -def get_entry_points(data: dict) -> dict[str, str]: +def get_entry_points(data: dict[str, Any]) -> dict[str, str]: """Extract command entry points from pyproject.toml.""" entry_points = {} eps = data.get("project", {}).get("entry-points", {}) @@ -119,8 +124,8 @@ def render_extra_content(package_dir: Path) -> list[str]: def render_command_readme( name: str, description: str, - entry_points: dict, - external_deps: list, + entry_points: dict[str, str], + external_deps: list[str], package_dir: Path, ) -> str: """Render README for a command package (deepctl-cmd-*).""" @@ -162,8 +167,8 @@ def render_command_readme( def render_debug_subcommand_readme( name: str, description: str, - entry_points: dict, - external_deps: list, + entry_points: dict[str, str], + external_deps: list[str], package_dir: Path, ) -> str: """Render README for a debug subcommand package.""" @@ -207,7 +212,7 @@ def render_debug_subcommand_readme( def render_core_readme( name: str, description: str, - external_deps: list, + external_deps: list[str], package_dir: Path, ) -> str: """Render README for a core/utility package.""" @@ -280,7 +285,7 @@ def get_package_dirs(single: str | None = None) -> list[Path]: # ── Root README section generators ────────────────────────────── -def get_all_packages() -> list[dict]: +def get_all_packages() -> list[dict[str, Any]]: """Load metadata for all packages in the workspace.""" packages = [] for pkg_dir in sorted(PACKAGES_DIR.iterdir()): @@ -301,7 +306,7 @@ def get_all_packages() -> list[dict]: return packages -def generate_commands_section(packages: list[dict]) -> str: +def generate_commands_section(packages: list[dict[str, Any]]) -> str: """Generate the commands markdown table.""" rows = [] for pkg in packages: @@ -321,7 +326,7 @@ def generate_commands_section(packages: list[dict]) -> str: return "\n".join(lines) -def generate_packages_section(packages: list[dict]) -> str: +def generate_packages_section(packages: list[dict[str, Any]]) -> str: """Generate the packages markdown table.""" lines = [ "| Package | Description |", @@ -334,7 +339,7 @@ def generate_packages_section(packages: list[dict]) -> str: return "\n".join(lines) -def generate_architecture_section(packages: list[dict]) -> str: +def generate_architecture_section(packages: list[dict[str, Any]]) -> str: """Generate the ASCII architecture tree.""" comment_col = 38 @@ -374,8 +379,10 @@ def replace_section(content: str, section: str, replacement: str) -> str: re.DOTALL, ) - def repl(m: re.Match) -> str: - return m.group(1) + replacement + "\n" + m.group(2) + def repl(m: re.Match[str]) -> str: + begin: str = m.group(1) + end: str = m.group(2) + return begin + replacement + "\n" + end return pattern.sub(repl, content) @@ -421,7 +428,7 @@ def update_root_readme(*, dry_run: bool = False, check: bool = False) -> bool: return True -def main(): +def main() -> None: parser = argparse.ArgumentParser( description="Generate README files from pyproject.toml metadata" ) diff --git a/uv.lock b/uv.lock index 94ba75a..b702822 100644 --- a/uv.lock +++ b/uv.lock @@ -904,6 +904,7 @@ dependencies = [ dev = [ { name = "build" }, { name = "mypy" }, + { name = "packaging" }, { name = "pre-commit" }, { name = "pytest" }, { name = "pytest-asyncio" }, @@ -922,6 +923,7 @@ dev = [ { name = "build" }, { name = "deepctl-plugin-example" }, { name = "mypy" }, + { name = "packaging" }, { name = "pre-commit" }, { name = "pytest" }, { name = "pytest-asyncio" }, @@ -930,6 +932,7 @@ dev = [ { name = "pytest-timeout" }, { name = "responses" }, { name = "ruff" }, + { name = "tomli", marker = "python_full_version < '3.11'" }, { name = "twine" }, { name = "types-pyyaml" }, { name = "types-requests" }, @@ -942,6 +945,7 @@ testing = [ { name = "pytest-mock" }, { name = "pytest-timeout" }, { name = "responses" }, + { name = "tomli", marker = "python_full_version < '3.11'" }, ] [package.metadata] @@ -981,6 +985,7 @@ requires-dist = [ { name = "httpx", specifier = ">=0.24.0" }, { name = "keyring", specifier = ">=24.0.0" }, { name = "mypy", marker = "extra == 'dev'", specifier = ">=1.0.0" }, + { name = "packaging", marker = "extra == 'dev'", specifier = ">=23.0" }, { name = "platformdirs", specifier = ">=3.0.0" }, { name = "pre-commit", marker = "extra == 'dev'", specifier = ">=3.0.0" }, { name = "pydantic", specifier = ">=2.0.0" }, @@ -1007,6 +1012,7 @@ dev = [ { name = "build", specifier = ">=0.10.0" }, { name = "deepctl-plugin-example", editable = "packages/deepctl-plugin-example" }, { name = "mypy", specifier = ">=1.0.0" }, + { name = "packaging", specifier = ">=23.0" }, { name = "pre-commit", specifier = ">=3.0.0" }, { name = "pytest", specifier = ">=7.0.0" }, { name = "pytest-asyncio", specifier = ">=0.21.0" }, @@ -1015,6 +1021,7 @@ dev = [ { name = "pytest-timeout", specifier = ">=2.3.1,<3" }, { name = "responses", specifier = ">=0.23.0" }, { name = "ruff", specifier = ">=0.8.0" }, + { name = "tomli", marker = "python_full_version < '3.11'", specifier = ">=2.0" }, { name = "twine", specifier = ">=7.0.0" }, { name = "types-pyyaml", specifier = ">=6.0.12.20250516" }, { name = "types-requests", specifier = ">=2.32.4.20250611" }, @@ -1027,6 +1034,7 @@ testing = [ { name = "pytest-mock", specifier = ">=3.10.0" }, { name = "pytest-timeout", specifier = ">=2.3.1,<3" }, { name = "responses", specifier = ">=0.23.0" }, + { name = "tomli", marker = "python_full_version < '3.11'", specifier = ">=2.0" }, ] [[package]] @@ -1785,7 +1793,7 @@ name = "exceptiongroup" version = "1.3.1" source = { registry = "https://pypi.org/simple" } dependencies = [ - { name = "typing-extensions", marker = "python_full_version < '3.11'" }, + { name = "typing-extensions" }, ] sdist = { url = "https://files.pythonhosted.org/packages/50/79/66800aadf48771f6b62f7eb014e352e5d06856655206165d775e675a02c9/exceptiongroup-1.3.1.tar.gz", hash = "sha256:8b412432c6055b0b7d14c310000ae93352ed6754f70fa8f7c34141f91c4e3219", size = 30371, upload-time = "2025-11-21T23:01:54.787Z" } wheels = [ @@ -2024,7 +2032,7 @@ name = "importlib-metadata" version = "9.0.0" source = { registry = "https://pypi.org/simple" } dependencies = [ - { name = "zipp", marker = "python_full_version < '3.12'" }, + { name = "zipp" }, ] sdist = { url = "https://files.pythonhosted.org/packages/a9/01/15bb152d77b21318514a96f43af312635eb2500c96b55398d020c93d86ea/importlib_metadata-9.0.0.tar.gz", hash = "sha256:a4f57ab599e6a2e3016d7595cfd72eb4661a5106e787a95bcc90c7105b831efc", size = 56405, upload-time = "2026-03-20T06:42:56.999Z" } wheels = [ From 95704fe381e382f2191cf21d917d60aef6e9cdaa Mon Sep 17 00:00:00 2001 From: Corey Weathers Date: Sun, 20 Sep 2026 10:33:34 -0400 Subject: [PATCH 19/21] fix(ci): parse whole dependency specifiers in the floor guard Two bugs, both from a regex that matched only bare `name>=digits`. A pre-release anywhere in the workspace made the guard permanently unsatisfiable. With deepctl-core at 0.4.0rc1, the version capture stopped at the first non-digit, so `--fix` wrote `deepctl-core>=0.4.0rc1`, printed "dependency floors OK" and exited 0, while the check immediately after it read that floor back as "0.4.0", compared it by string equality and exited 1. Those two commands run back to back in the release sync job, so the job fails, the release PR never gets its uv.lock regenerated -- the exact wall this guard exists to remove -- and `make floors-check` can never be made to pass. Versions are now compared as packaging.version.Version, which is also stricter than the old numeric-segments-only comparison for rule 2: a floor of >=0.4.0 against a sibling at 0.4.0rc1 is genuinely unsatisfiable, and now says so. A root dependency written with an extra or a non-`>=` specifier was invisible to rule 1, so rule 3 reported it as missing from root entirely -- "add it to root pyproject.toml" about a line already in root pyproject.toml. Extras and arbitrary specifiers now parse; anything that is not a `>=` floor is reported as what it is, with the floor the delivery manifest needs. Tests drive the real script over throwaway workspaces: the fix/check round trip across four PEP 440 suffix shapes, idempotence, upper bounds and extras surviving --fix, and positive controls for all three rules. Co-Authored-By: Claude Opus 5 --- scripts/check_dependency_floors.py | 146 ++++++++++++---- tests/unit/test_dependency_floors.py | 242 +++++++++++++++++++++++++++ 2 files changed, 353 insertions(+), 35 deletions(-) create mode 100644 tests/unit/test_dependency_floors.py diff --git a/scripts/check_dependency_floors.py b/scripts/check_dependency_floors.py index 5609bcb..61da1c0 100644 --- a/scripts/check_dependency_floors.py +++ b/scripts/check_dependency_floors.py @@ -27,6 +27,11 @@ deliberately not shipped as part of the CLI goes in NOT_SHIPPED, so that intent is stated in the diff rather than inferred from an omission. +Versions are compared with `packaging.version.Version`, not as strings, so a +pre-release anywhere in the workspace (0.4.0rc1, 1.0.0.dev1, 0.3.0.post1) +behaves like any other version instead of turning `--fix` into a loop the +following check can never satisfy. + Run with --fix to rewrite root floors in place (used by the release workflow's sync job so rule 1 holds automatically on every release PR). """ @@ -37,17 +42,23 @@ import json import re import sys +from dataclasses import dataclass from pathlib import Path -try: +if sys.version_info >= (3, 11): import tomllib -except ModuleNotFoundError: +else: # pragma: no cover - only taken on Python 3.10 try: - import tomli as tomllib # type: ignore[no-redef] + import tomli as tomllib except ModuleNotFoundError: - print("Python 3.11+ required (tomllib), or install tomli: pip install tomli") + print( + "Python 3.11+ required (tomllib), or install tomli: pip install tomli", + file=sys.stderr, + ) sys.exit(1) +from packaging.version import InvalidVersion, Version + REPO = Path(__file__).resolve().parent.parent MANIFEST = REPO / ".github" / ".release-please-manifest.json" @@ -58,6 +69,34 @@ "deepctl-plugin-example", # sample plugin, installed on demand } +# A PEP 508 requirement, narrowed to intra-workspace packages: the name, an +# optional extras group, then whatever the author wrote after it. Matching +# only bare `name>=version` used to make `deepctl-cmd-keys[extra]>=0.0.1` or +# `deepctl-cmd-keys==0.1.0` invisible to rules 1 and 3, so the guard reported +# a listed package as missing from root entirely. +_DEP_RE = re.compile( + r"^\s*(?Pdeepctl[A-Za-z0-9._-]*)" + r"\s*(?:\[[^\]]*\])?" + r"\s*(?P.*)$" +) +# The floor itself. The version runs to the first delimiter, so an upper +# bound (`,<2`), an environment marker (`; python_version < "3.12"`) and a +# PEP 440 suffix (`0.4.0rc1`) all survive intact. +_FLOOR_RE = re.compile(r"^(?P>=\s*(?P[^\s,;]+))") + + +@dataclass(frozen=True) +class Dep: + """One intra-workspace dependency as root or a package declares it.""" + + name: str + #: The pinned floor, or None when the spec is not a plain `>=`. + floor: str | None + #: Exact `>=…` text matched, so --fix can rewrite it in place. + spec: str + #: The whole dependency string, as written. + raw: str + def workspace_versions() -> dict[str, str]: """Map package name -> current workspace version, per the manifest.""" @@ -71,29 +110,39 @@ def workspace_versions() -> dict[str, str]: return versions -def floors(pyproject: Path) -> list[tuple[str, str, str]]: - """Yield (name, floor, raw-spec) for each intra-workspace dependency.""" +def floors(pyproject: Path) -> list[Dep]: + """Return every intra-workspace dependency declared by `pyproject`.""" deps = tomllib.loads(pyproject.read_text(encoding="utf-8"))["project"].get( "dependencies", [] ) - out = [] + out: list[Dep] = [] for dep in deps: - m = re.match(r"^(deepctl[\w-]*)>=([0-9][0-9.]*)", dep) - if m: - out.append((m.group(1), m.group(2), dep)) + m = _DEP_RE.match(dep) + if not m: + continue + floor = _FLOOR_RE.match(m.group("rest").strip()) + out.append( + Dep( + name=m.group("name"), + floor=floor.group("version") if floor else None, + spec=floor.group("spec") if floor else "", + raw=dep, + ) + ) return out -def vkey(version: str) -> list[int]: - """Sortable key from the numeric release segments only. +def parse(version: str) -> Version | None: + """PEP 440 version, or None when the string is not a valid version. - PEP 440 suffixes (0.4.0rc1, 1.0.0.dev1, 0.3.0.post1) and local versions - are ignored rather than crashing the comparison -- a single hand-set - pre-release anywhere in the workspace used to turn `make floors-check` - into a bare ValueError traceback naming no package. + Returning None rather than raising keeps one hand-typed oddity from + turning `make floors-check` into a bare traceback naming no package; the + caller reports it as an ordinary problem instead. """ - release = re.match(r"\d+(?:\.\d+)*", version) - return [int(part) for part in release.group().split(".")] if release else [0] + try: + return Version(version) + except InvalidVersion: + return None def main() -> int: @@ -113,12 +162,30 @@ def main() -> int: root_text = root.read_text(encoding="utf-8") fixed = root_text root_floors = floors(root) - for name, floor, raw in root_floors: - current = versions.get(name) + for dep in root_floors: + current = versions.get(dep.name) if current is None: - problems.append(f"root depends on {name}, which is not in the manifest") + problems.append(f"root depends on {dep.name}, which is not in the manifest") + continue + if dep.floor is None: + problems.append( + f"root dependency {dep.raw!r} does not pin a `>=` floor" + f" (root is the delivery manifest: write" + f" {dep.name}>={current} so pip upgrades deliver it)" + ) + continue + floor_version, current_version = parse(dep.floor), parse(current) + if floor_version is None: + problems.append( + f"root floor {dep.name}>={dep.floor} is not a valid PEP 440 version" + ) + continue + if current_version is None: + problems.append( + f"{dep.name} workspace version {current} is not a valid PEP 440 version" + ) continue - if floor == current: + if floor_version == current_version: continue if args.fix: # Rewrite the version inside the spec we matched, so any upper @@ -126,12 +193,12 @@ def main() -> int: # literal replace silently no-ops on those, and an unfixable # floor has to fall through to `problems` -- reporting "OK" # after failing to fix is worse than not fixing. - new_raw = raw.replace(f">={floor}", f">={current}", 1) - if new_raw != raw and f'"{raw}"' in fixed: - fixed = fixed.replace(f'"{raw}"', f'"{new_raw}"', 1) + new_raw = dep.raw.replace(dep.spec, f">={current}", 1) + if new_raw != dep.raw and f'"{dep.raw}"' in fixed: + fixed = fixed.replace(f'"{dep.raw}"', f'"{new_raw}"', 1) continue problems.append( - f"root floor {name}>={floor} != workspace version {current}" + f"root floor {dep.name}>={dep.floor} != workspace version {current}" " (published fixes will not be delivered by pip upgrades)" ) if args.fix and fixed != root_text: @@ -143,20 +210,30 @@ def main() -> int: pyproject = pkg_dir / "pyproject.toml" if not pyproject.is_file(): continue - for name, floor, _ in floors(pyproject): - current = versions.get(name) + for dep in floors(pyproject): + current = versions.get(dep.name) if current is None: problems.append( - f"{pkg_dir.name} depends on {name}, which is not in the manifest" + f"{pkg_dir.name} depends on {dep.name}," + " which is not in the manifest" + ) + continue + if dep.floor is None: + continue + floor_version, current_version = parse(dep.floor), parse(current) + if floor_version is None or current_version is None: + problems.append( + f"{pkg_dir.name}: floor {dep.name}>={dep.floor} or workspace" + f" version {current} is not a valid PEP 440 version" ) - elif vkey(floor) > vkey(current): + elif floor_version > current_version: problems.append( - f"{pkg_dir.name}: floor {name}>={floor} exceeds" + f"{pkg_dir.name}: floor {dep.name}>={dep.floor} exceeds" f" workspace version {current} (unsatisfiable)" ) # Rule 3: every published package is a root dependency. - listed = {name for name, _, _ in root_floors} + listed = {dep.name for dep in root_floors} for name in sorted(set(versions) - listed - NOT_SHIPPED): problems.append( f"{name} is published but is not a root dependency" @@ -169,9 +246,8 @@ def main() -> int: for problem in problems: print(f" - {problem}", file=sys.stderr) print( - "\nRun `python3 scripts/check_dependency_floors.py --fix` to pin" - " stale root floors. Sub-package floors and missing root" - " dependencies are hand-maintained.", + "\nRun `make floors-fix` to pin stale root floors. Sub-package" + " floors and missing root dependencies are hand-maintained.", file=sys.stderr, ) return 1 diff --git a/tests/unit/test_dependency_floors.py b/tests/unit/test_dependency_floors.py new file mode 100644 index 0000000..cb1cbae --- /dev/null +++ b/tests/unit/test_dependency_floors.py @@ -0,0 +1,242 @@ +"""Tests for scripts/check_dependency_floors.py. + +The guard runs in CI (`make floors-check`) and, with `--fix`, twice in the +release workflow's sync job: fix, `uv lock`, check. Those two commands run +back to back, so anything that makes `--fix` disagree with the following +check wedges the release PR -- the exact wall the guard exists to remove. +The tests below drive the real script over throwaway workspaces. +""" + +from __future__ import annotations + +import ast +import json +import re +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO = Path(__file__).resolve().parents[2] +SCRIPT = REPO / "scripts" / "check_dependency_floors.py" + + +def make_workspace( + tmp_path: Path, + root_deps: list[str], + packages: dict[str, str], +) -> Path: + """Build a minimal workspace the guard can be pointed at. + + `packages` maps package name -> version; each becomes + packages//pyproject.toml and a manifest entry. + """ + (tmp_path / "scripts").mkdir() + shutil.copy(SCRIPT, tmp_path / "scripts" / SCRIPT.name) + + deps = ", ".join(f'"{dep}"' for dep in root_deps) + (tmp_path / "pyproject.toml").write_text( + "[project]\n" + 'name = "deepctl"\n' + 'version = "1.0.0"\n' + f"dependencies = [{deps}]\n", + encoding="utf-8", + ) + + manifest: dict[str, str] = {".": "1.0.0"} + for name, version in packages.items(): + pkg_dir = tmp_path / "packages" / name + pkg_dir.mkdir(parents=True) + (pkg_dir / "pyproject.toml").write_text( + f'[project]\nname = "{name}"\nversion = "{version}"\n', + encoding="utf-8", + ) + manifest[f"packages/{name}"] = version + + github = tmp_path / ".github" + github.mkdir() + (github / ".release-please-manifest.json").write_text( + json.dumps(manifest), encoding="utf-8" + ) + return tmp_path + + +def run_guard(workspace: Path, *args: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + [sys.executable, str(workspace / "scripts" / SCRIPT.name), *args], + capture_output=True, + text=True, + ) + + +def root_dependencies(workspace: Path) -> list[str]: + """Read back the dependency list this module wrote, after --fix. + + Parsed with ast rather than tomllib so the test itself runs on 3.10, + where the guard uses its tomli fallback. + """ + text = (workspace / "pyproject.toml").read_text(encoding="utf-8") + match = re.search(r"^dependencies = (\[.*\])$", text, re.MULTILINE) + assert match, text + deps: list[str] = ast.literal_eval(match.group(1)) + return deps + + +@pytest.mark.parametrize( + "version", + ["0.4.0rc1", "1.0.0.dev1", "0.3.0.post1", "0.4.0b2"], +) +def test_fix_then_check_round_trips_on_pre_release_versions( + tmp_path: Path, version: str +) -> None: + """`--fix` must produce a tree the very next check accepts. + + The version regex used to stop at the first non-digit, so a workspace + version of 0.4.0rc1 was read as a floor of "0.4.0": --fix wrote + `deepctl-core>=0.4.0rc1` and reported success, and the check that runs + immediately after it in the release sync job exited 1 on the floor it + had just written. `make floors-check` could then never be made to pass. + """ + workspace = make_workspace( + tmp_path, + root_deps=["deepctl-core>=0.1.0"], + packages={"deepctl-core": version}, + ) + + fix = run_guard(workspace, "--fix") + assert fix.returncode == 0, fix.stderr + assert root_dependencies(workspace) == [f"deepctl-core>={version}"] + + check = run_guard(workspace) + assert check.returncode == 0, check.stderr + assert "dependency floors OK" in check.stdout + + +def test_fix_is_idempotent_when_already_pinned(tmp_path: Path) -> None: + """A second --fix on an already-correct tree changes nothing.""" + workspace = make_workspace( + tmp_path, + root_deps=["deepctl-core>=0.4.0rc1"], + packages={"deepctl-core": "0.4.0rc1"}, + ) + before = (workspace / "pyproject.toml").read_text(encoding="utf-8") + + assert run_guard(workspace, "--fix").returncode == 0 + assert (workspace / "pyproject.toml").read_text(encoding="utf-8") == before + + +def test_fix_preserves_upper_bounds_and_extras(tmp_path: Path) -> None: + """Only the floor is rewritten; the rest of the spec survives.""" + workspace = make_workspace( + tmp_path, + root_deps=["deepctl-core[speech]>=0.1.0,<2"], + packages={"deepctl-core": "0.4.0rc1"}, + ) + + assert run_guard(workspace, "--fix").returncode == 0 + assert root_dependencies(workspace) == ["deepctl-core[speech]>=0.4.0rc1,<2"] + assert run_guard(workspace).returncode == 0 + + +def test_extras_do_not_look_like_a_missing_root_dependency(tmp_path: Path) -> None: + """A correctly pinned dep with extras is not reported as missing. + + Rule 3's "published but is not a root dependency" message is derived + from what rule 1's parser saw. When that parser only matched bare + `name>=version`, writing an extras group made the guard tell the + maintainer to add a dependency that was already right there. + """ + workspace = make_workspace( + tmp_path, + root_deps=["deepctl-core[speech]>=0.4.0"], + packages={"deepctl-core": "0.4.0"}, + ) + + result = run_guard(workspace) + assert result.returncode == 0, result.stderr + assert "is not a root dependency" not in result.stderr + + +def test_non_floor_specifier_is_named_for_what_it_is(tmp_path: Path) -> None: + """`==` is reported as an unsupported specifier, not as an omission.""" + workspace = make_workspace( + tmp_path, + root_deps=["deepctl-core==0.4.0"], + packages={"deepctl-core": "0.4.0"}, + ) + + result = run_guard(workspace) + assert result.returncode == 1 + assert "does not pin a `>=` floor" in result.stderr + assert "is not a root dependency" not in result.stderr + + +def test_stale_root_floor_still_fails(tmp_path: Path) -> None: + """Positive control: rule 1 keeps catching what it was written for.""" + workspace = make_workspace( + tmp_path, + root_deps=["deepctl-core>=0.3.0"], + packages={"deepctl-core": "0.4.0"}, + ) + + result = run_guard(workspace) + assert result.returncode == 1 + assert "root floor deepctl-core>=0.3.0 != workspace version 0.4.0" in result.stderr + + +def test_missing_root_dependency_still_fails(tmp_path: Path) -> None: + """Positive control: rule 3 keeps catching an unlisted package.""" + workspace = make_workspace( + tmp_path, + root_deps=["deepctl-core>=0.4.0"], + packages={"deepctl-core": "0.4.0", "deepctl-cmd-keys": "0.1.0"}, + ) + + result = run_guard(workspace) + assert result.returncode == 1 + assert "deepctl-cmd-keys is published but is not a root dependency" in result.stderr + + +def test_unsatisfiable_sub_package_floor_still_fails(tmp_path: Path) -> None: + """Positive control: rule 2 keeps catching a floor above the sibling.""" + workspace = make_workspace( + tmp_path, + root_deps=["deepctl-core>=0.4.0", "deepctl-cmd-keys>=0.1.0"], + packages={"deepctl-core": "0.4.0", "deepctl-cmd-keys": "0.1.0"}, + ) + keys = workspace / "packages" / "deepctl-cmd-keys" / "pyproject.toml" + keys.write_text( + '[project]\nname = "deepctl-cmd-keys"\nversion = "0.1.0"\n' + 'dependencies = ["deepctl-core>=9.9.9"]\n', + encoding="utf-8", + ) + + result = run_guard(workspace) + assert result.returncode == 1 + assert "exceeds workspace version 0.4.0 (unsatisfiable)" in result.stderr + + +def test_final_release_floor_above_a_pre_release_is_unsatisfiable( + tmp_path: Path, +) -> None: + """0.4.0 is not satisfied by 0.4.0rc1, and the guard must say so. + + Comparing only the numeric segments made these two look identical. + """ + workspace = make_workspace( + tmp_path, + root_deps=["deepctl-core>=0.4.0rc1", "deepctl-cmd-keys>=0.1.0"], + packages={"deepctl-core": "0.4.0rc1", "deepctl-cmd-keys": "0.1.0"}, + ) + keys = workspace / "packages" / "deepctl-cmd-keys" / "pyproject.toml" + keys.write_text( + '[project]\nname = "deepctl-cmd-keys"\nversion = "0.1.0"\n' + 'dependencies = ["deepctl-core>=0.4.0"]\n', + encoding="utf-8", + ) + + result = run_guard(workspace) + assert result.returncode == 1 + assert "exceeds workspace version 0.4.0rc1 (unsatisfiable)" in result.stderr From 0497ac1892333c09d2a482f0b10ccd59d1f1dba2 Mon Sep 17 00:00:00 2001 From: Corey Weathers Date: Sun, 20 Sep 2026 10:33:43 -0400 Subject: [PATCH 20/21] fix(update,skills): exit 2 when the user declines, as documented The README says declining a confirmation prompt exits 2. keys, members, login, projects, init and plugin remove do. Two did not. `dg update` returned `UpdateResult(...).model_dump()` on decline -- a plain dict, and BaseCommand.exit_code_for reads `.status` off the result, which a dict has not got. So the process exited 0 while the same JSON payload carried `"status": "success"` next to "Update cancelled by user": a script branching on the exit code read a cancelled update as a completed one. It now returns the model with status="cancelled". `dg skills install --cli X`, answered no at "Install anyway?", returned bare and exited 0. These subcommands are plain click callbacks, so nothing maps a returned result to an exit code -- the same shape as plugin remove, and the same fix: raise click.Abort(), which main.py turns into the documented 2. Also adds err=True to plugin remove's confirm, matching the sibling prompt in deepctl-cmd-keys: a prompt is interaction, not output, and keeping it off stdout is what lets `-o json` stay parseable. Co-Authored-By: Claude Opus 5 --- .../src/deepctl_cmd_plugin/command.py | 7 +- .../src/deepctl_cmd_skills/command.py | 7 +- .../tests/unit/test_skills_command.py | 57 ++++++++ .../src/deepctl_cmd_update/command.py | 18 ++- .../tests/unit/test_update_command.py | 132 ++++++++++++++++++ 5 files changed, 216 insertions(+), 5 deletions(-) diff --git a/packages/deepctl-cmd-plugin/src/deepctl_cmd_plugin/command.py b/packages/deepctl-cmd-plugin/src/deepctl_cmd_plugin/command.py index 8321a48..755112c 100644 --- a/packages/deepctl-cmd-plugin/src/deepctl_cmd_plugin/command.py +++ b/packages/deepctl-cmd-plugin/src/deepctl_cmd_plugin/command.py @@ -474,7 +474,12 @@ def _handle_remove( package = kwargs["package"] yes = kwargs.get("yes", False) - if not yes and not click.confirm(f"Are you sure you want to remove {package}?"): + # err=True: the prompt is interaction, not output. Keeping it off + # stdout is what lets `dg plugin remove ... -o json` stay parseable, + # and matches the sibling prompt in deepctl-cmd-keys. + if not yes and not click.confirm( + f"Are you sure you want to remove {package}?", err=True + ): # Abort rather than return: this is a group subcommand, so there is # no result for BaseCommand.EXIT_CODES to map, and a bare return # exits 0 -- indistinguishable from a successful removal. main.py diff --git a/packages/deepctl-cmd-skills/src/deepctl_cmd_skills/command.py b/packages/deepctl-cmd-skills/src/deepctl_cmd_skills/command.py index 837cd5c..1c65d0e 100644 --- a/packages/deepctl-cmd-skills/src/deepctl_cmd_skills/command.py +++ b/packages/deepctl-cmd-skills/src/deepctl_cmd_skills/command.py @@ -271,7 +271,12 @@ def _handle_install( f"{generators[0].display_name} was not detected on this system." ) if not self.confirm("Install anyway?", default=False): - return + # Abort rather than return: these subcommands are plain + # click callbacks, so nothing maps a returned result to an + # exit code and a bare return exits 0 -- indistinguishable + # from a successful install. main.py turns Abort into the + # documented exit 2 for user cancellation. + raise click.Abort() else: generators = detect_ai_clis() diff --git a/packages/deepctl-cmd-skills/tests/unit/test_skills_command.py b/packages/deepctl-cmd-skills/tests/unit/test_skills_command.py index 4094d87..251cb47 100644 --- a/packages/deepctl-cmd-skills/tests/unit/test_skills_command.py +++ b/packages/deepctl-cmd-skills/tests/unit/test_skills_command.py @@ -2,6 +2,7 @@ from unittest.mock import MagicMock, patch +import click import pytest from deepctl_cmd_skills.command import SkillsCommand @@ -36,6 +37,62 @@ def test_setup_commands_returns_subcommands(self): assert "list" in names assert "status" in names + def test_declining_install_anyway_aborts_instead_of_exiting_zero(self): + """Declining the prompt must exit 2, not 0. + + `_handle_install` is a plain click callback returning None, so there + is no result for BaseCommand.EXIT_CODES to map to an exit code. A + bare return made a declined install indistinguishable from a + successful one, contradicting the exit-code table in the README. + Abort is what main.py turns into 2. + """ + cmd = SkillsCommand() + generator = MagicMock() + generator.cli_name = "claude" + generator.display_name = "Claude Code" + generator.detect.return_value = False + + with ( + patch( + "deepctl_core.skill_generator.get_all_generators", + return_value=[generator], + ), + patch.object(cmd, "confirm", return_value=False), + ): + with pytest.raises(click.Abort): + cmd._handle_install(cli_name="claude") + + generator.install.assert_not_called() + + def test_accepting_install_anyway_does_not_abort(self): + """Positive control: confirming must proceed to the install.""" + cmd = SkillsCommand() + generator = MagicMock() + generator.cli_name = "claude" + generator.display_name = "Claude Code" + generator.detect.return_value = False + generator.install.return_value = [] + + with ( + patch( + "deepctl_core.skill_generator.get_all_generators", + return_value=[generator], + ), + patch( + "deepctl_core.skill_generator.collect_command_metadata", + return_value={}, + ), + patch( + "deepctl_core.skill_generator.get_skills_state", + return_value={"installed_skills": {}}, + ), + patch("deepctl_core.skill_generator.save_skills_state"), + patch.object(cmd, "confirm", return_value=True), + ): + cmd._handle_install(cli_name="claude") + + generator.install.assert_called_once() + class TestSkillsStartupCheck: """Test startup check module.""" diff --git a/packages/deepctl-cmd-update/src/deepctl_cmd_update/command.py b/packages/deepctl-cmd-update/src/deepctl_cmd_update/command.py index f38d478..683c388 100644 --- a/packages/deepctl-cmd-update/src/deepctl_cmd_update/command.py +++ b/packages/deepctl-cmd-update/src/deepctl_cmd_update/command.py @@ -69,8 +69,14 @@ def handle( auth_manager: Any, # Not used for update command client: Any, # Not used for update command **kwargs: Any, - ) -> dict[str, Any]: - """Handle the update command execution.""" + ) -> UpdateResult | dict[str, Any]: + """Handle the update command execution. + + Returns the model rather than a dict on the cancel path: + BaseCommand.exit_code_for reads `.status` off the result, and a dict + has none. output_result unwraps either, so the JSON payload is the + same shape from both. + """ console = get_console() # Extract arguments from kwargs @@ -167,14 +173,20 @@ def handle( # Confirm update if not yes and not Confirm.ask("\nDo you want to proceed with the update?"): print_info("Update cancelled") + # Return the model, not .model_dump(): BaseCommand.exit_code_for + # reads `.status` off the result, and a plain dict has none -- a + # declined update used to exit 0 while its own payload said + # "Update cancelled by user". status="cancelled" is the documented + # exit 2. return UpdateResult( + status="cancelled", success=False, message="Update cancelled by user", current_version=version_info.current_version, latest_version=version_info.latest_version, update_available=version_info.update_available, installation_method=install_info.method.value, - ).model_dump() + ) # Execute update print_info("Updating deepctl...") diff --git a/packages/deepctl-cmd-update/tests/unit/test_update_command.py b/packages/deepctl-cmd-update/tests/unit/test_update_command.py index 04d5214..3f18630 100644 --- a/packages/deepctl-cmd-update/tests/unit/test_update_command.py +++ b/packages/deepctl-cmd-update/tests/unit/test_update_command.py @@ -316,3 +316,135 @@ def test_development_installation( assert result["success"] is False assert result["installation_method"] == "development" mock_print_warning.assert_called_once() + + # ------------------------------------------------------------------ + # declining the confirmation prompt + # ------------------------------------------------------------------ + + @patch("deepctl_cmd_update.command.subprocess.run") + @patch("deepctl_cmd_update.command.InstallationDetector") + @patch("deepctl_cmd_update.command.asyncio.run") + @patch("deepctl_cmd_update.command.VersionChecker") + @patch("deepctl_cmd_update.command.get_console") + @patch("deepctl_cmd_update.command.format_version_message") + @patch("deepctl_cmd_update.command.print_info") + @patch("deepctl_cmd_update.command.Confirm.ask") + def test_declining_the_prompt_reports_cancelled_and_exits_two( + self, + mock_confirm, + mock_print_info, + mock_format_msg, + mock_console, + mock_checker_class, + mock_asyncio_run, + mock_detector_class, + mock_subprocess, + command, + ): + """Declining must exit 2, and say "cancelled" in the payload. + + The README documents exit 2 for declining a confirmation prompt. + This path used to return `.model_dump()` -- a plain dict, which + `BaseCommand.exit_code_for` cannot read a status off -- so the + process exited 0 while the very same JSON payload carried + `"status": "success"` next to "Update cancelled by user". + """ + mock_asyncio_run.return_value = VersionInfo( + current_version="0.1.0", + latest_version="0.2.0", + update_available=True, + ) + mock_format_msg.return_value = "Update available!" + + mock_detector = mock_detector_class.return_value + mock_detector.detect.return_value = InstallationInfo( + method=InstallMethod.PIP, + path="/path/to/deepctl", + virtual_env=True, + editable=False, + python_executable="/usr/bin/python3", + ) + mock_detector.get_update_command.return_value = [ + "pip", + "install", + "--upgrade", + "deepctl", + ] + + mock_confirm.return_value = False + + result = command.handle( + config=MagicMock(), + auth_manager=MagicMock(), + client=MagicMock(), + yes=False, + ) + + assert command.exit_code_for(result) == 2 + assert result.status == "cancelled" + assert result.success is False + assert result.message == "Update cancelled by user" + mock_subprocess.assert_not_called() + + @patch("deepctl_cmd_update.command.subprocess.run") + @patch("deepctl_cmd_update.command.InstallationDetector") + @patch("deepctl_cmd_update.command.asyncio.run") + @patch("deepctl_cmd_update.command.VersionChecker") + @patch("deepctl_cmd_update.command.get_console") + @patch("deepctl_cmd_update.command.format_version_message") + @patch("deepctl_cmd_update.command.print_info") + @patch("deepctl_cmd_update.command.print_success") + @patch("deepctl_cmd_update.command.Confirm.ask") + def test_successful_update_still_exits_zero( + self, + mock_confirm, + mock_print_success, + mock_print_info, + mock_format_msg, + mock_console, + mock_checker_class, + mock_asyncio_run, + mock_detector_class, + mock_subprocess, + command, + ): + """Negative control: accepting the prompt must still exit 0. + + Only the decline path carries a status, so the success payload maps + to 0 -- the point is that adding "cancelled" did not make an accepted + update exit non-zero. + """ + mock_asyncio_run.return_value = VersionInfo( + current_version="0.1.0", + latest_version="0.2.0", + update_available=True, + ) + mock_format_msg.return_value = "Update available!" + + mock_detector = mock_detector_class.return_value + mock_detector.detect.return_value = InstallationInfo( + method=InstallMethod.PIP, + path="/path/to/deepctl", + virtual_env=True, + editable=False, + python_executable="/usr/bin/python3", + ) + mock_detector.get_update_command.return_value = [ + "pip", + "install", + "--upgrade", + "deepctl", + ] + mock_subprocess.return_value = MagicMock(returncode=0, stderr="") + mock_confirm.return_value = True + + result = command.handle( + config=MagicMock(), + auth_manager=MagicMock(), + client=MagicMock(), + yes=False, + ) + + assert command.exit_code_for(result) == 0 + assert result["success"] is True + mock_subprocess.assert_called_once() From 1799c213911f2fbc1d0c2430bb3ce7d4389c7afc Mon Sep 17 00:00:00 2001 From: Corey Weathers Date: Sun, 20 Sep 2026 10:39:46 -0400 Subject: [PATCH 21/21] fix(update,skills): exit 1 when the command fails Round-two review of this branch's own exit-code work. The README table says 1 for a failed command, and four more paths returned 0. `dg update` returned `.model_dump()` from all three of its failure paths -- version check unreachable, pip exiting non-zero, the update raising -- so `dg update` printed "Update failed: No matching distribution" and exited 0. Same root cause as the decline path: exit_code_for reads `.status` off the result and a dict has none. They now carry status="error". `dg skills install --cli nope` printed "Unknown AI CLI" and exited 0, as did `dg skills remove --cli X` for a CLI with nothing installed. Both raise click.ClickException now, which main.py turns into 1. Probed live: both were 0 before, both are 1 now. Also raises verify-published's ceiling from 30 to 45 minutes. The poll's worst case is 20 minutes of sleeps plus 60 resolution attempts against PyPI, which can exceed 30 -- a ceiling that cuts the loop short would kill the job right before it prints the diagnosis that was just added to it. Co-Authored-By: Claude Opus 5 --- .github/workflows/release.yml | 10 ++- .../src/deepctl_cmd_skills/command.py | 14 +-- .../tests/unit/test_skills_command.py | 30 +++++++ .../src/deepctl_cmd_update/command.py | 21 +++-- .../tests/unit/test_update_command.py | 89 +++++++++++++++++++ 5 files changed, 148 insertions(+), 16 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 01ab6d6..b5bfae9 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -155,10 +155,12 @@ jobs: # Reads nothing and writes nothing: it installs from public PyPI. Drop the # workflow-level contents/pull-requests write it would otherwise inherit. permissions: {} - # The poll is bounded at 20 minutes on its own; this is the belt-and-braces - # ceiling for a hung runner or a wedged pip, so three release-advertising - # jobs can never sit queued behind this one indefinitely. - timeout-minutes: 30 + # Belt-and-braces ceiling for a hung runner or a wedged pip, so three + # release-advertising jobs can never sit queued behind this one + # indefinitely. It has to clear the poll's own worst case, which is 20 + # minutes of sleeps *plus* 60 resolution attempts -- a ceiling that cuts + # the loop short would kill the job before it prints the diagnosis below. + timeout-minutes: 45 steps: - name: Set up Python uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 diff --git a/packages/deepctl-cmd-skills/src/deepctl_cmd_skills/command.py b/packages/deepctl-cmd-skills/src/deepctl_cmd_skills/command.py index 1c65d0e..9db92d9 100644 --- a/packages/deepctl-cmd-skills/src/deepctl_cmd_skills/command.py +++ b/packages/deepctl-cmd-skills/src/deepctl_cmd_skills/command.py @@ -11,7 +11,7 @@ from deepctl_core.base_group_command import BaseGroupCommand from deepctl_core.client import DeepgramClient from deepctl_core.config import Config -from deepctl_core.output import print_error, print_info, print_success, print_warning +from deepctl_core.output import print_info, print_success, print_warning from rich.console import Console from rich.table import Table @@ -261,11 +261,13 @@ def _handle_install( if cli_name: generators = [g for g in get_all_generators() if g.cli_name == cli_name] if not generators: - print_error( + # ClickException, not print_error + return: a bare return + # exits 0, and the README documents 1 for a command that + # fails. main.py prints the message and exits 1. + raise click.ClickException( f"Unknown AI CLI: {cli_name}. " "Run 'deepctl skills status' to see supported CLIs." ) - return if not generators[0].detect(): print_warning( f"{generators[0].display_name} was not detected on this system." @@ -402,8 +404,10 @@ def _handle_remove( if cli_name: targets = [cli_name] if cli_name in installed else [] if not targets: - print_error(f"No skills installed for '{cli_name}'.") - return + # Same contract as the unknown-CLI path above: asking to + # remove something that is not there is a failed command, + # which the README documents as exit 1, not 0. + raise click.ClickException(f"No skills installed for '{cli_name}'.") elif remove_all: targets = list(installed.keys()) else: diff --git a/packages/deepctl-cmd-skills/tests/unit/test_skills_command.py b/packages/deepctl-cmd-skills/tests/unit/test_skills_command.py index 251cb47..8564186 100644 --- a/packages/deepctl-cmd-skills/tests/unit/test_skills_command.py +++ b/packages/deepctl-cmd-skills/tests/unit/test_skills_command.py @@ -93,6 +93,36 @@ def test_accepting_install_anyway_does_not_abort(self): generator.install.assert_called_once() + def test_unknown_cli_exits_one(self): + """An unknown --cli must exit 1, not print an error and exit 0.""" + cmd = SkillsCommand() + with patch( + "deepctl_core.skill_generator.get_all_generators", + return_value=[], + ): + with pytest.raises(click.ClickException) as exc: + cmd._handle_install(cli_name="nonexistent-cli") + + assert "Unknown AI CLI: nonexistent-cli" in str(exc.value) + + def test_removing_skills_that_are_not_installed_exits_one(self): + """`skills remove --cli X` with nothing installed for X exits 1.""" + cmd = SkillsCommand() + with ( + patch( + "deepctl_core.skill_generator.get_skills_state", + return_value={"installed_skills": {"claude": {"paths": []}}}, + ), + patch( + "deepctl_core.skill_generator.get_all_generators", + return_value=[], + ), + ): + with pytest.raises(click.ClickException) as exc: + cmd._handle_remove(cli_name="cursor", remove_all=False) + + assert "No skills installed for 'cursor'" in str(exc.value) + class TestSkillsStartupCheck: """Test startup check module.""" diff --git a/packages/deepctl-cmd-update/src/deepctl_cmd_update/command.py b/packages/deepctl-cmd-update/src/deepctl_cmd_update/command.py index 683c388..9bdc277 100644 --- a/packages/deepctl-cmd-update/src/deepctl_cmd_update/command.py +++ b/packages/deepctl-cmd-update/src/deepctl_cmd_update/command.py @@ -72,10 +72,12 @@ def handle( ) -> UpdateResult | dict[str, Any]: """Handle the update command execution. - Returns the model rather than a dict on the cancel path: - BaseCommand.exit_code_for reads `.status` off the result, and a dict - has none. output_result unwraps either, so the JSON payload is the - same shape from both. + Returns the model rather than a dict wherever the exit code has to + be non-zero: BaseCommand.exit_code_for reads `.status` off the + result, and a dict has none, so a dict always exits 0. The three + failure paths carry status="error" (exit 1) and the decline path + status="cancelled" (exit 2). output_result unwraps either, so the + JSON payload is the same shape from both. """ console = get_console() @@ -99,10 +101,13 @@ def handle( version_info = asyncio.run(version_checker.check_version(force=True)) except Exception as e: print_error(f"Failed to check for updates: {e}") + # status="error" on the model, not a dict: see the docstring. + # A command that failed has to exit 1, as the README says. return UpdateResult( + status="error", success=False, message=f"Failed to check for updates: {e}", - ).model_dump() + ) # Display version info message = format_version_message(version_info) @@ -224,13 +229,14 @@ def handle( console.print(f"[yellow]{shlex.join(update_command)}[/yellow]") return UpdateResult( + status="error", success=False, message=f"Update failed: {error_msg}", current_version=version_info.current_version, latest_version=version_info.latest_version, update_available=version_info.update_available, installation_method=install_info.method.value, - ).model_dump() + ) except Exception as e: print_error(f"Failed to execute update: {e}") @@ -240,10 +246,11 @@ def handle( console.print(f"[yellow]{update_command}[/yellow]") return UpdateResult( + status="error", success=False, message=f"Failed to execute update: {e}", current_version=version_info.current_version, latest_version=version_info.latest_version, update_available=version_info.update_available, installation_method=install_info.method.value, - ).model_dump() + ) diff --git a/packages/deepctl-cmd-update/tests/unit/test_update_command.py b/packages/deepctl-cmd-update/tests/unit/test_update_command.py index 3f18630..0a9cb54 100644 --- a/packages/deepctl-cmd-update/tests/unit/test_update_command.py +++ b/packages/deepctl-cmd-update/tests/unit/test_update_command.py @@ -448,3 +448,92 @@ def test_successful_update_still_exits_zero( assert command.exit_code_for(result) == 0 assert result["success"] is True mock_subprocess.assert_called_once() + + # ------------------------------------------------------------------ + # failure paths + # ------------------------------------------------------------------ + + @patch("deepctl_cmd_update.command.subprocess.run") + @patch("deepctl_cmd_update.command.InstallationDetector") + @patch("deepctl_cmd_update.command.asyncio.run") + @patch("deepctl_cmd_update.command.VersionChecker") + @patch("deepctl_cmd_update.command.get_console") + @patch("deepctl_cmd_update.command.format_version_message") + @patch("deepctl_cmd_update.command.print_info") + @patch("deepctl_cmd_update.command.print_error") + @patch("deepctl_cmd_update.command.Confirm.ask") + def test_failed_update_exits_one( + self, + mock_confirm, + mock_print_error, + mock_print_info, + mock_format_msg, + mock_console, + mock_checker_class, + mock_asyncio_run, + mock_detector_class, + mock_subprocess, + command, + ): + """A pip that exits non-zero must make `dg update` exit 1.""" + mock_asyncio_run.return_value = VersionInfo( + current_version="0.1.0", + latest_version="0.2.0", + update_available=True, + ) + mock_format_msg.return_value = "Update available!" + + mock_detector = mock_detector_class.return_value + mock_detector.detect.return_value = InstallationInfo( + method=InstallMethod.PIP, + path="/path/to/deepctl", + virtual_env=True, + editable=False, + python_executable="/usr/bin/python3", + ) + mock_detector.get_update_command.return_value = [ + "pip", + "install", + "--upgrade", + "deepctl", + ] + mock_subprocess.return_value = MagicMock( + returncode=1, stderr="No matching distribution" + ) + mock_confirm.return_value = True + + result = command.handle( + config=MagicMock(), + auth_manager=MagicMock(), + client=MagicMock(), + yes=True, + ) + + assert command.exit_code_for(result) == 1 + assert result.status == "error" + assert "No matching distribution" in result.message + + @patch("deepctl_cmd_update.command.asyncio.run") + @patch("deepctl_cmd_update.command.VersionChecker") + @patch("deepctl_cmd_update.command.get_console") + @patch("deepctl_cmd_update.command.print_error") + def test_failed_version_check_exits_one( + self, + mock_print_error, + mock_console, + mock_checker_class, + mock_asyncio_run, + command, + ): + """An unreachable PyPI must make `dg update` exit 1, not 0.""" + mock_asyncio_run.side_effect = RuntimeError("connection refused") + + result = command.handle( + config=MagicMock(), + auth_manager=MagicMock(), + client=MagicMock(), + ) + + assert command.exit_code_for(result) == 1 + assert result.status == "error" + assert "connection refused" in result.message