From bbe70a83a65e9cea3bb4f39b72dfbdcc02a63c30 Mon Sep 17 00:00:00 2001 From: Corey Weathers Date: Sun, 1 Nov 2026 13:16:27 -0500 Subject: [PATCH 01/13] feat(telemetry): honor DO_NOT_TRACK and CI Error reports now also stay off when DO_NOT_TRACK or CI is set, with the same rule as DEEPCTL_TELEMETRY_DISABLED: any non-empty value except 0, false, no or off. They are checked after the dg-specific switch, and the help footer then reads "Telemetry is off.". The README's opt-out list and the help footer name both. Both variables join the root --help cache key, since the footer depends on them. Telemetry's own tests clear both variables, so they behave the same on a CI runner. --- conftest.py | 4 + docs/contracts/help-cache.md | 2 +- .../src/deepctl_telemetry/client.py | 35 +++-- .../src/deepctl_telemetry/notice.py | 7 +- .../tests/golden/notice_on.txt | 2 +- .../tests/unit/test_config_opt_out_caps.py | 1 + .../tests/unit/test_contract.py | 141 ++++++++++++++++++ .../tests/unit/test_envelopes.py | 1 + .../tests/unit/test_telemetry.py | 3 + src/deepctl/_cache.py | 2 + tests/unit/test_cache.py | 5 +- tests/unit/test_telemetry_wiring.py | 4 +- 12 files changed, 189 insertions(+), 18 deletions(-) create mode 100644 packages/deepctl-telemetry/tests/unit/test_contract.py diff --git a/conftest.py b/conftest.py index 5ae8166f..b81ca5da 100644 --- a/conftest.py +++ b/conftest.py @@ -77,6 +77,10 @@ def _disable_telemetry(monkeypatch): """ monkeypatch.setenv("DEEPCTL_TELEMETRY_DISABLED", "1") monkeypatch.setenv("DEEPCTL_TELEMETRY_DSN", "http://k@127.0.0.1:9/1") + # A developer's DO_NOT_TRACK must not change what telemetry's own tests + # see. (CI is cleared only by those tests: + # other suites read it as an automation signal.) + monkeypatch.delenv("DO_NOT_TRACK", raising=False) @pytest.fixture(autouse=True) diff --git a/docs/contracts/help-cache.md b/docs/contracts/help-cache.md index 830a8a8c..13764f35 100644 --- a/docs/contracts/help-cache.md +++ b/docs/contracts/help-cache.md @@ -77,7 +77,7 @@ The launcher serves an entry only when every one of the following holds. Anythin 4. **Each name in `env`** has the same value in the launcher's environment, and `null` means the variable must be unset. On Windows, look the names up case-insensitively, as the OS does. A variable that isn't listed doesn't matter. `DEEPCTL_PROG_NAME` and `DEEPCTL_HELP_CACHE` are never listed: the launcher sets them, and `prog` already records the name. 5. **`text_sha256`** (lowercase hex) equals the SHA-256 of the entry bytes the launcher read. This binds the text to its key: Python writes the text first and the key second, so text from another run under this key is a miss and is never served. -A bundle install (`-I`, no user site, `HOME` is the real home) lists about 10 files and 18 environment variables. Python writes no key over the limits (32 files, 64 variables), so an install with a long `sys.path` (an editable install, say) gets no entry and always takes the Python path. +A bundle install (`-I`, no user site, `HOME` is the real home) lists about 10 files and 20 environment variables. Python writes no key over the limits (32 files, 64 variables), so an install with a long `sys.path` (an editable install, say) gets no entry and always takes the Python path. **Paths and values that aren't valid Unicode.** - **Unix:** a path or environment value with bytes that don't decode under the filesystem encoding reaches Python as a lone surrogate (`\udcXX`). diff --git a/packages/deepctl-telemetry/src/deepctl_telemetry/client.py b/packages/deepctl-telemetry/src/deepctl_telemetry/client.py index af472401..24550578 100644 --- a/packages/deepctl-telemetry/src/deepctl_telemetry/client.py +++ b/packages/deepctl-telemetry/src/deepctl_telemetry/client.py @@ -39,8 +39,12 @@ DSN_ENV_VAR = "DEEPCTL_TELEMETRY_DSN" DISABLE_ENV_VAR = "DEEPCTL_TELEMETRY_DISABLED" +# Cross-tool opt-outs, honored like DEEPCTL_TELEMETRY_DISABLED: the +# consoledonottrack.com convention, and the variable CI services set. +DO_NOT_TRACK_ENV_VAR = "DO_NOT_TRACK" +CI_ENV_VAR = "CI" -# Values of DEEPCTL_TELEMETRY_DISABLED that leave telemetry on. Any other +# Values of the opt-out variables that leave telemetry on. Any other # non-empty value turns it off. _ENV_KEEPS_ON = frozenset({"0", "false", "no", "off"}) @@ -55,10 +59,10 @@ def _fresh_state() -> dict[str, Any]: """Per-run state, stdlib types only. `opt_out` names the switch that turned telemetry off ("build", "dsn" - when there is no destination, "flag", "env" or "config"); `config_path` - is the user config file the decision was read from; `reported` is set - once a report starts, `sent` once an event has been handed to the - transport. + when there is no destination, "flag", "env", "do_not_track", "ci" or + "config"); `config_path` is the user config file the decision was read + from; `reported` is set once a report starts, `sent` once an event has + been handed to the transport. """ return { "enabled": False, @@ -81,11 +85,17 @@ def disable_for_run() -> None: _state["enabled"] = False -def _env_opts_out() -> bool: - value = os.environ.get(DISABLE_ENV_VAR, "").strip().lower() +def _env_set(name: str) -> bool: + """Whether the variable `name` turns telemetry off (any non-empty value + except the ones in _ENV_KEEPS_ON).""" + value = os.environ.get(name, "").strip().lower() return bool(value) and value not in _ENV_KEEPS_ON +def _env_opts_out() -> bool: + return _env_set(DISABLE_ENV_VAR) + + # Keys whose repetition in a config file makes the telemetry setting # ambiguous; a file that repeats one counts as opted out. _TELEMETRY_KEYS = frozenset({"telemetry", "enabled", "telemetry.enabled"}) @@ -189,6 +199,10 @@ def _opt_out_source(config: Config) -> str | None: return "flag" if _env_opts_out(): return "env" + if _env_set(DO_NOT_TRACK_ENV_VAR): + return "do_not_track" + if _env_set(CI_ENV_VAR): + return "ci" if _user_config_opts_out(config): return "config" return None @@ -199,9 +213,10 @@ def is_enabled(config: Config) -> bool: Resolution order: the build switch, then whether there is a destination (a loopback DEEPCTL_TELEMETRY_DSN; without one telemetry is - off), then `--no-telemetry` for this run, then the env override - (DEEPCTL_TELEMETRY_DISABLED), then `telemetry.enabled` in the user's - config file (default on). The project's `deepgram.yaml` is never read. + off), then `--no-telemetry` for this run, then the env overrides + (DEEPCTL_TELEMETRY_DISABLED, DO_NOT_TRACK, CI), then `telemetry.enabled` + in the user's config file (default on). The project's `deepgram.yaml` is + never read. """ return _opt_out_source(config) is None diff --git a/packages/deepctl-telemetry/src/deepctl_telemetry/notice.py b/packages/deepctl-telemetry/src/deepctl_telemetry/notice.py index abba90cc..b4de4428 100644 --- a/packages/deepctl-telemetry/src/deepctl_telemetry/notice.py +++ b/packages/deepctl-telemetry/src/deepctl_telemetry/notice.py @@ -16,12 +16,13 @@ # Golden text, compared byte for byte with tests/golden/notice_*.txt. # NOTICE_ON shows only with a loopback DEEPCTL_TELEMETRY_DSN (tests and local -# sinks). It names the three opt-outs that exist: the flag for one run, the env -# var, and the config-file key. +# sinks). It names every opt-out: the flag for one run, the env var, the +# config-file key, and the cross-tool DO_NOT_TRACK and CI. NOTICE_ON = ( "Telemetry is on: unexpected errors only, with no flag values, paths or " "keys. Turn off: --no-telemetry, DEEPCTL_TELEMETRY_DISABLED=1, or " - "telemetry.enabled: false in config.yaml." + "telemetry.enabled: false in config.yaml. DO_NOT_TRACK=1 and CI turn it " + "off too." ) NOTICE_OFF = "Telemetry is off." # No destination (no loopback DEEPCTL_TELEMETRY_DSN), or the build switch is diff --git a/packages/deepctl-telemetry/tests/golden/notice_on.txt b/packages/deepctl-telemetry/tests/golden/notice_on.txt index b2fca74e..c2ead897 100644 --- a/packages/deepctl-telemetry/tests/golden/notice_on.txt +++ b/packages/deepctl-telemetry/tests/golden/notice_on.txt @@ -1 +1 @@ -Telemetry is on: unexpected errors only, with no flag values, paths or keys. Turn off: --no-telemetry, DEEPCTL_TELEMETRY_DISABLED=1, or telemetry.enabled: false in config.yaml. +Telemetry is on: unexpected errors only, with no flag values, paths or keys. Turn off: --no-telemetry, DEEPCTL_TELEMETRY_DISABLED=1, or telemetry.enabled: false in config.yaml. DO_NOT_TRACK=1 and CI turn it off too. diff --git a/packages/deepctl-telemetry/tests/unit/test_config_opt_out_caps.py b/packages/deepctl-telemetry/tests/unit/test_config_opt_out_caps.py index 333a8acb..368adb7d 100644 --- a/packages/deepctl-telemetry/tests/unit/test_config_opt_out_caps.py +++ b/packages/deepctl-telemetry/tests/unit/test_config_opt_out_caps.py @@ -53,6 +53,7 @@ def _env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> Path: """Telemetry on in this process (env switch cleared), no SDK to send with, no agent session; returns the default config path (absent).""" monkeypatch.delenv(client.DISABLE_ENV_VAR, raising=False) + monkeypatch.delenv(client.CI_ENV_VAR, raising=False) monkeypatch.setattr(client, "_state", client._fresh_state()) monkeypatch.setitem(sys.modules, "sentry_sdk", None) monkeypatch.setattr(output, "is_ai_agent", lambda: False, raising=False) diff --git a/packages/deepctl-telemetry/tests/unit/test_contract.py b/packages/deepctl-telemetry/tests/unit/test_contract.py new file mode 100644 index 00000000..f37148e9 --- /dev/null +++ b/packages/deepctl-telemetry/tests/unit/test_contract.py @@ -0,0 +1,141 @@ +"""The telemetry runtime contract. + +Cross-tool opt-outs: DO_NOT_TRACK and CI. Reports go to a local sink on loopback, never further. +""" + +from __future__ import annotations + +import sys +import tempfile +import threading +from collections.abc import Iterator +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from pathlib import Path +from types import SimpleNamespace +from typing import Any +from unittest.mock import patch + +import pytest +import sentry_sdk +from deepctl_telemetry import client + +@pytest.fixture(autouse=True) +def _env(monkeypatch: pytest.MonkeyPatch) -> Iterator[None]: + """Telemetry on (every opt-out cleared), fresh state, no user config.""" + for name in ( + client.DISABLE_ENV_VAR, + client.DO_NOT_TRACK_ENV_VAR, + client.CI_ENV_VAR, + ): + monkeypatch.delenv(name, raising=False) + monkeypatch.setattr(client, "_state", client._fresh_state()) + default = Path(tempfile.mkdtemp()) / "default-config.yaml" + monkeypatch.setattr(client, "_default_config_path", lambda: default) + saved = (sys.excepthook, threading.excepthook) + yield + sys.excepthook, threading.excepthook = saved + sentry_sdk.get_client().close() + sentry_sdk.get_global_scope().set_client(None) + + +def _config() -> Any: + return SimpleNamespace(config_path=Path(tempfile.mkdtemp()) / "config.yaml") + + +class _Sink: + """An HTTP server on loopback that counts the reports it receives.""" + + def __init__(self) -> None: + self.posts: list[str] = [] + sink = self + + class Handler(BaseHTTPRequestHandler): + def do_POST(self) -> None: + length = int(self.headers.get("Content-Length") or 0) + self.rfile.read(length) + sink.posts.append(self.path) + self.send_response(200) + self.end_headers() + self.wfile.write(b"{}") + + def log_message(self, *args: Any) -> None: + pass + + self.server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + self.port = self.server.server_address[1] + threading.Thread(target=self.server.serve_forever, daemon=True).start() + + @property + def dsn(self) -> str: + return f"http://k@127.0.0.1:{self.port}/1" + + def close(self) -> None: + self.server.shutdown() + self.server.server_close() + + +@pytest.fixture +def sink(monkeypatch: pytest.MonkeyPatch) -> Iterator[_Sink]: + server = _Sink() + monkeypatch.setenv(client.DSN_ENV_VAR, server.dsn) + yield server + server.close() + + +def _fail() -> bool: + try: + raise RuntimeError("forced") + except RuntimeError as exc: + return client.report_exception(exc) + + +class TestCrossToolOptOuts: + @pytest.mark.parametrize( + ("name", "value", "source"), + [ + ("DO_NOT_TRACK", "1", "do_not_track"), + ("DO_NOT_TRACK", "true", "do_not_track"), + ("CI", "true", "ci"), + ("CI", "1", "ci"), + ("CI", "woodpecker", "ci"), + ], + ) + def test_turns_it_off( + self, monkeypatch: pytest.MonkeyPatch, name: str, value: str, source: str + ) -> None: + monkeypatch.setenv(name, value) + assert client.init_telemetry(_config()) is False + assert client._state["opt_out"] == source + + @pytest.mark.parametrize("name", ["DO_NOT_TRACK", "CI"]) + @pytest.mark.parametrize("value", ["", "0", "false", "No", " off "]) + def test_false_values_keep_it_on( + self, monkeypatch: pytest.MonkeyPatch, name: str, value: str + ) -> None: + monkeypatch.setenv(name, value) + assert client.init_telemetry(_config()) is True + + def test_dg_switch_is_named_first(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("DEEPCTL_TELEMETRY_DISABLED", "1") + monkeypatch.setenv("DO_NOT_TRACK", "1") + monkeypatch.setenv("CI", "true") + client.init_telemetry(_config()) + assert client._state["opt_out"] == "env" + + @pytest.mark.parametrize("name", ["DO_NOT_TRACK", "CI"]) + def test_forced_error_sends_nothing( + self, monkeypatch: pytest.MonkeyPatch, sink: _Sink, name: str + ) -> None: + monkeypatch.setenv(name, "1") + client.init_telemetry(_config()) + with patch("sentry_sdk.init") as init: + assert _fail() is False + init.assert_not_called() + assert sink.posts == [] + + def test_sink_receives_a_report_without_them(self, sink: _Sink) -> None: + # Control: the same failure with no opt-out reaches the sink. + client.init_telemetry(_config()) + assert _fail() is True + sentry_sdk.get_client().close(timeout=2) + assert len(sink.posts) == 1 diff --git a/packages/deepctl-telemetry/tests/unit/test_envelopes.py b/packages/deepctl-telemetry/tests/unit/test_envelopes.py index 4a5fbb92..9437aa29 100644 --- a/packages/deepctl-telemetry/tests/unit/test_envelopes.py +++ b/packages/deepctl-telemetry/tests/unit/test_envelopes.py @@ -114,6 +114,7 @@ def init_with_capture(*args: Any, **kwargs: Any) -> Any: saved_thread_hook = threading.excepthook monkeypatch.setattr(sentry_sdk, "init", init_with_capture) monkeypatch.delenv(client.DISABLE_ENV_VAR, raising=False) + monkeypatch.delenv(client.CI_ENV_VAR, raising=False) monkeypatch.setenv(client.DSN_ENV_VAR, "http://public@127.0.0.1:9/1") monkeypatch.setattr(sys, "argv", list(ARGV)) monkeypatch.setattr(client, "_state", client._fresh_state()) diff --git a/packages/deepctl-telemetry/tests/unit/test_telemetry.py b/packages/deepctl-telemetry/tests/unit/test_telemetry.py index f0f6f0d5..0477a1d4 100644 --- a/packages/deepctl-telemetry/tests/unit/test_telemetry.py +++ b/packages/deepctl-telemetry/tests/unit/test_telemetry.py @@ -37,6 +37,7 @@ def _telemetry_env(monkeypatch: pytest.MonkeyPatch, fresh_state: Any) -> None: the rest of the process. """ monkeypatch.delenv(DISABLE_ENV_VAR, raising=False) + monkeypatch.delenv("CI", raising=False) # The default user config file: absent unless a test writes it. default = Path(tempfile.mkdtemp()) / "default-config.yaml" monkeypatch.setattr(fresh_state, "_default_config_path", lambda: default) @@ -281,6 +282,8 @@ def test_on_message_names_existing_opt_outs(self) -> None: assert "--no-telemetry" in notice assert "DEEPCTL_TELEMETRY_DISABLED=1" in notice assert "telemetry.enabled: false" in notice + assert "DO_NOT_TRACK=1" in notice + assert "CI" in notice assert "dg config" not in notice def test_off_message_matches_golden(self) -> None: diff --git a/src/deepctl/_cache.py b/src/deepctl/_cache.py index 8ff9f4cc..2f5191f4 100644 --- a/src/deepctl/_cache.py +++ b/src/deepctl/_cache.py @@ -52,6 +52,8 @@ "FORCE_COLOR", "DEEPGRAM_OUTPUT_COLOR", "DEEPCTL_TELEMETRY_DISABLED", + "DO_NOT_TRACK", + "CI", "DEEPCTL_EAGER_LOAD", "XDG_CONFIG_HOME", "HOME", diff --git a/tests/unit/test_cache.py b/tests/unit/test_cache.py index 04786739..2eefbc4d 100644 --- a/tests/unit/test_cache.py +++ b/tests/unit/test_cache.py @@ -134,7 +134,8 @@ def test_clean_run_writes_a_private_entry_under_the_real_home( assert recorded["prog"] == "dg" and recorded["isatty"] is False assert not any("deepgram.yaml" in f["path"] for f in recorded["files"]) assert not any("cwd_relative" in f for f in recorded["files"]) - assert "DEEPCTL_TELEMETRY_DISABLED" in recorded["env"] + for name in ("DEEPCTL_TELEMETRY_DISABLED", "DO_NOT_TRACK", "CI"): + assert name in recorded["env"], name assert "DEEPCTL_HELP_CACHE" not in recorded["env"] def test_no_gate_writes_nothing( @@ -273,7 +274,7 @@ def test_key_size_in_a_bundle_configuration( key = _cache.help_key("dg", 80, False, b"") assert len(key["files"]) <= _cache.MAX_KEY_FILES == 32 assert _cache.MAX_KEY_ENV == 64 - assert len(key["env"]) == 18 + assert len(key["env"]) == 20 def _poison(stamp: str, commands: list[dict[str, Any]]) -> None: diff --git a/tests/unit/test_telemetry_wiring.py b/tests/unit/test_telemetry_wiring.py index 883efa6f..48581dfe 100644 --- a/tests/unit/test_telemetry_wiring.py +++ b/tests/unit/test_telemetry_wiring.py @@ -30,6 +30,7 @@ def telemetry_on(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): from deepctl_telemetry import client monkeypatch.delenv(client.DISABLE_ENV_VAR, raising=False) + monkeypatch.delenv(client.CI_ENV_VAR, raising=False) monkeypatch.setenv(client.DSN_ENV_VAR, DEAD_DSN) monkeypatch.setenv("HOME", str(tmp_path)) monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / ".config")) @@ -428,7 +429,8 @@ def _probe_env(tmp_path: Path, dsn: str | None = DEAD_DSN) -> dict[str, str]: env = { key: value for key, value in os.environ.items() - if key not in ("DEEPCTL_TELEMETRY_DISABLED", "DEEPCTL_TELEMETRY_DSN") + if key + not in ("DEEPCTL_TELEMETRY_DISABLED", "DEEPCTL_TELEMETRY_DSN", "CI", "DO_NOT_TRACK") and not key.startswith("DEEPGRAM_") } if dsn is not None: From 96dc9dc50b785aa2e3d2b5b980fc1bcaf0c98dc8 Mon Sep 17 00:00:00 2001 From: Corey Weathers Date: Sun, 1 Nov 2026 13:16:27 -0500 Subject: [PATCH 02/13] perf(telemetry): skip the flush when no report was queued When the report is dropped before it is queued (a closed-stream error, or nothing left to send), there is nothing to wait for, so the flush is skipped. Tests pin the two timing cases: a refused connection ends the report at once, and an unanswered one is held to the budget. --- .../src/deepctl_telemetry/client.py | 8 +++- .../tests/unit/test_contract.py | 45 ++++++++++++++++++- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/packages/deepctl-telemetry/src/deepctl_telemetry/client.py b/packages/deepctl-telemetry/src/deepctl_telemetry/client.py index 24550578..60786718 100644 --- a/packages/deepctl-telemetry/src/deepctl_telemetry/client.py +++ b/packages/deepctl-telemetry/src/deepctl_telemetry/client.py @@ -319,7 +319,8 @@ def report_exception(exc: BaseException | None) -> bool: DEEPCTL_TELEMETRY_DSN. Imports and initializes the SDK only here. At most one report per process. The flush waits at most FLUSH_TIMEOUT_SECONDS after init, and the whole call is held to - REPORT_BUDGET_SECONDS (but the flush always gets FLUSH_FLOOR_SECONDS). + REPORT_BUDGET_SECONDS (but the flush always gets FLUSH_FLOOR_SECONDS); + there is no flush when no event was queued. Never raises an Exception; a KeyboardInterrupt during the flush propagates to the caller. Returns whether an event was handed to the SDK. @@ -345,7 +346,10 @@ def report_exception(exc: BaseException | None) -> bool: cli_version = "unknown" sentry_sdk.init(**_sdk_options(dsn, cli_version)) - sentry_sdk.capture_exception(target) + if sentry_sdk.capture_exception(target) is None: + # Dropped before it was queued (a closed-stream error, or no + # field left to send): there is nothing to wait for. + return False left = REPORT_BUDGET_SECONDS - (time.monotonic() - started) timeout = max(FLUSH_FLOOR_SECONDS, min(FLUSH_TIMEOUT_SECONDS, left)) sentry_sdk.flush(timeout=timeout) diff --git a/packages/deepctl-telemetry/tests/unit/test_contract.py b/packages/deepctl-telemetry/tests/unit/test_contract.py index f37148e9..c16247f1 100644 --- a/packages/deepctl-telemetry/tests/unit/test_contract.py +++ b/packages/deepctl-telemetry/tests/unit/test_contract.py @@ -1,13 +1,16 @@ """The telemetry runtime contract. -Cross-tool opt-outs: DO_NOT_TRACK and CI. Reports go to a local sink on loopback, never further. +Cross-tool opt-outs (DO_NOT_TRACK, CI) and the flush that only waits for a +queued event. Reports go to a local sink on loopback, never further. """ from __future__ import annotations +import socket import sys import tempfile import threading +import time from collections.abc import Iterator from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from pathlib import Path @@ -139,3 +142,43 @@ def test_sink_receives_a_report_without_them(self, sink: _Sink) -> None: assert _fail() is True sentry_sdk.get_client().close(timeout=2) assert len(sink.posts) == 1 + + +class TestFlush: + def test_no_flush_when_nothing_was_queued(self) -> None: + client.init_telemetry(_config()) + with patch("sentry_sdk.flush") as flush: + try: + raise BrokenPipeError(32, "Broken pipe") + except BrokenPipeError as exc: + assert client.report_exception(exc) is False + flush.assert_not_called() + + def test_refused_connection_does_not_wait_out_the_budget( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + # A port nothing listens on: the one send fails at once, and the + # flush returns as soon as it has, not at its timeout. + with socket.socket() as probe: + probe.bind(("127.0.0.1", 0)) + port = probe.getsockname()[1] + monkeypatch.setenv(client.DSN_ENV_VAR, f"http://k@127.0.0.1:{port}/1") + client.init_telemetry(_config()) + started = time.monotonic() + assert _fail() is True + assert time.monotonic() - started < client.FLUSH_TIMEOUT_SECONDS + + def test_unanswered_send_is_held_to_the_budget( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + # Accepts the connection and never answers (blocked egress). + with socket.socket() as hole: + hole.bind(("127.0.0.1", 0)) + hole.listen(8) + port = hole.getsockname()[1] + monkeypatch.setenv(client.DSN_ENV_VAR, f"http://k@127.0.0.1:{port}/1") + client.init_telemetry(_config()) + started = time.monotonic() + assert _fail() is True + elapsed = time.monotonic() - started + assert elapsed < client.REPORT_BUDGET_SECONDS + 0.25 From 120bfde93856fe02931b9096550249f506701e96 Mon Sep 17 00:00:00 2001 From: Corey Weathers Date: Sun, 1 Nov 2026 13:16:28 -0500 Subject: [PATCH 03/13] feat(telemetry): add DEEPCTL_TELEMETRY=log and =off With DEEPCTL_TELEMETRY=log, an unexpected error prints the report dg would send to stderr as one JSON line, and nothing is sent: the SDK gets a transport that only writes to stderr and no destination. It changes nothing when telemetry is off. DEEPCTL_TELEMETRY=off (or 0, false, no, disabled) turns telemetry off, like DEEPCTL_TELEMETRY_DISABLED. Any other value keeps the default. The variable joins the root --help cache key. --- conftest.py | 5 +- docs/contracts/help-cache.md | 2 +- .../src/deepctl_telemetry/client.py | 66 ++++++++++++++--- .../tests/unit/test_contract.py | 73 ++++++++++++++++++- src/deepctl/_cache.py | 1 + tests/unit/test_cache.py | 9 ++- 6 files changed, 139 insertions(+), 17 deletions(-) diff --git a/conftest.py b/conftest.py index b81ca5da..3a8e7bf5 100644 --- a/conftest.py +++ b/conftest.py @@ -77,10 +77,11 @@ def _disable_telemetry(monkeypatch): """ monkeypatch.setenv("DEEPCTL_TELEMETRY_DISABLED", "1") monkeypatch.setenv("DEEPCTL_TELEMETRY_DSN", "http://k@127.0.0.1:9/1") - # A developer's DO_NOT_TRACK must not change what telemetry's own tests - # see. (CI is cleared only by those tests: + # A developer's DO_NOT_TRACK or DEEPCTL_TELEMETRY=log must not change + # what telemetry's own tests see. (CI is cleared only by those tests: # other suites read it as an automation signal.) monkeypatch.delenv("DO_NOT_TRACK", raising=False) + monkeypatch.delenv("DEEPCTL_TELEMETRY", raising=False) @pytest.fixture(autouse=True) diff --git a/docs/contracts/help-cache.md b/docs/contracts/help-cache.md index 13764f35..c2f67611 100644 --- a/docs/contracts/help-cache.md +++ b/docs/contracts/help-cache.md @@ -77,7 +77,7 @@ The launcher serves an entry only when every one of the following holds. Anythin 4. **Each name in `env`** has the same value in the launcher's environment, and `null` means the variable must be unset. On Windows, look the names up case-insensitively, as the OS does. A variable that isn't listed doesn't matter. `DEEPCTL_PROG_NAME` and `DEEPCTL_HELP_CACHE` are never listed: the launcher sets them, and `prog` already records the name. 5. **`text_sha256`** (lowercase hex) equals the SHA-256 of the entry bytes the launcher read. This binds the text to its key: Python writes the text first and the key second, so text from another run under this key is a miss and is never served. -A bundle install (`-I`, no user site, `HOME` is the real home) lists about 10 files and 20 environment variables. Python writes no key over the limits (32 files, 64 variables), so an install with a long `sys.path` (an editable install, say) gets no entry and always takes the Python path. +A bundle install (`-I`, no user site, `HOME` is the real home) lists about 10 files and 21 environment variables. Python writes no key over the limits (32 files, 64 variables), so an install with a long `sys.path` (an editable install, say) gets no entry and always takes the Python path. **Paths and values that aren't valid Unicode.** - **Unix:** a path or environment value with bytes that don't decode under the filesystem encoding reaches Python as a lone surrogate (`\udcXX`). diff --git a/packages/deepctl-telemetry/src/deepctl_telemetry/client.py b/packages/deepctl-telemetry/src/deepctl_telemetry/client.py index 60786718..65ecf72c 100644 --- a/packages/deepctl-telemetry/src/deepctl_telemetry/client.py +++ b/packages/deepctl-telemetry/src/deepctl_telemetry/client.py @@ -43,6 +43,12 @@ # consoledonottrack.com convention, and the variable CI services set. DO_NOT_TRACK_ENV_VAR = "DO_NOT_TRACK" CI_ENV_VAR = "CI" +# DEEPCTL_TELEMETRY=log prints the report that would be sent to stderr as +# one JSON line, and sends nothing. The values in _MODE_OFF turn telemetry +# off; any other value keeps the default. +MODE_ENV_VAR = "DEEPCTL_TELEMETRY" +LOG_MODE = "log" +_MODE_OFF = frozenset({"off", "0", "false", "no", "disabled"}) # Values of the opt-out variables that leave telemetry on. Any other # non-empty value turns it off. @@ -93,7 +99,14 @@ def _env_set(name: str) -> bool: def _env_opts_out() -> bool: - return _env_set(DISABLE_ENV_VAR) + if _env_set(DISABLE_ENV_VAR): + return True + return os.environ.get(MODE_ENV_VAR, "").strip().lower() in _MODE_OFF + + +def _log_mode() -> bool: + """DEEPCTL_TELEMETRY=log: print the report instead of sending it.""" + return os.environ.get(MODE_ENV_VAR, "").strip().lower() == LOG_MODE # Keys whose repetition in a config file makes the telemetry setting @@ -214,9 +227,9 @@ def is_enabled(config: Config) -> bool: Resolution order: the build switch, then whether there is a destination (a loopback DEEPCTL_TELEMETRY_DSN; without one telemetry is off), then `--no-telemetry` for this run, then the env overrides - (DEEPCTL_TELEMETRY_DISABLED, DO_NOT_TRACK, CI), then `telemetry.enabled` - in the user's config file (default on). The project's `deepgram.yaml` is - never read. + (DEEPCTL_TELEMETRY_DISABLED or DEEPCTL_TELEMETRY set to off, + DO_NOT_TRACK, CI), then `telemetry.enabled` in the user's config file + (default on). The project's `deepgram.yaml` is never read. """ return _opt_out_source(config) is None @@ -320,10 +333,11 @@ def report_exception(exc: BaseException | None) -> bool: most one report per process. The flush waits at most FLUSH_TIMEOUT_SECONDS after init, and the whole call is held to REPORT_BUDGET_SECONDS (but the flush always gets FLUSH_FLOOR_SECONDS); - there is no flush when no event was queued. - Never raises an Exception; a KeyboardInterrupt during the flush - propagates to the caller. Returns whether an event was handed to the - SDK. + there is no flush when no event was queued. With DEEPCTL_TELEMETRY=log + the event is printed to stderr instead and nothing is sent, not even to + the loopback DSN. Never raises an Exception; a KeyboardInterrupt during + the flush propagates to the caller. Returns whether an event was handed + to the SDK. """ started = time.monotonic() with _report_lock: @@ -345,11 +359,18 @@ def report_exception(exc: BaseException | None) -> bool: except Exception: cli_version = "unknown" - sentry_sdk.init(**_sdk_options(dsn, cli_version)) + if _log_mode(): + options = _sdk_options(None, cli_version) + options["transport"] = _stderr_transport() + else: + options = _sdk_options(dsn, cli_version) + sentry_sdk.init(**options) if sentry_sdk.capture_exception(target) is None: # Dropped before it was queued (a closed-stream error, or no # field left to send): there is nothing to wait for. return False + if _log_mode(): + return True left = REPORT_BUDGET_SECONDS - (time.monotonic() - started) timeout = max(FLUSH_FLOOR_SECONDS, min(FLUSH_TIMEOUT_SECONDS, left)) sentry_sdk.flush(timeout=timeout) @@ -364,7 +385,32 @@ def _read_cli_version() -> str: return importlib.metadata.version("deepctl") -def _sdk_options(dsn: str, cli_version: str) -> dict[str, Any]: +def _stderr_transport() -> Any: + """A transport that writes each event to stderr as one JSON line. + + Used for DEEPCTL_TELEMETRY=log. It never opens a connection, and the + SDK gets no destination at all in that mode. + """ + import json + + from sentry_sdk.transport import Transport + + class StderrTransport(Transport): + def capture_envelope(self, envelope: Any) -> None: + event = envelope.get_event() + if event is None: + return + try: + sys.stderr.write(json.dumps(event, sort_keys=True, default=str)) + sys.stderr.write("\n") + sys.stderr.flush() + except (OSError, ValueError, AttributeError): + pass + + return StderrTransport() + + +def _sdk_options(dsn: str | None, cli_version: str) -> dict[str, Any]: """Explicit SDK settings: one error event per failure and nothing else. No integrations at all (no argv, modules, logging, breadcrumbs, and none diff --git a/packages/deepctl-telemetry/tests/unit/test_contract.py b/packages/deepctl-telemetry/tests/unit/test_contract.py index c16247f1..acadbf8c 100644 --- a/packages/deepctl-telemetry/tests/unit/test_contract.py +++ b/packages/deepctl-telemetry/tests/unit/test_contract.py @@ -1,11 +1,12 @@ """The telemetry runtime contract. -Cross-tool opt-outs (DO_NOT_TRACK, CI) and the flush that only waits for a -queued event. Reports go to a local sink on loopback, never further. +Cross-tool opt-outs (DO_NOT_TRACK, CI), the DEEPCTL_TELEMETRY=log mode, the +flush that only waits for a queued event. Reports go to a local sink on loopback, never further. """ from __future__ import annotations +import json import socket import sys import tempfile @@ -29,6 +30,7 @@ def _env(monkeypatch: pytest.MonkeyPatch) -> Iterator[None]: client.DISABLE_ENV_VAR, client.DO_NOT_TRACK_ENV_VAR, client.CI_ENV_VAR, + client.MODE_ENV_VAR, ): monkeypatch.delenv(name, raising=False) monkeypatch.setattr(client, "_state", client._fresh_state()) @@ -125,6 +127,13 @@ def test_dg_switch_is_named_first(self, monkeypatch: pytest.MonkeyPatch) -> None client.init_telemetry(_config()) assert client._state["opt_out"] == "env" + def test_log_mode_does_not_turn_an_opt_out_back_on( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("DO_NOT_TRACK", "1") + monkeypatch.setenv("DEEPCTL_TELEMETRY", "log") + assert client.init_telemetry(_config()) is False + @pytest.mark.parametrize("name", ["DO_NOT_TRACK", "CI"]) def test_forced_error_sends_nothing( self, monkeypatch: pytest.MonkeyPatch, sink: _Sink, name: str @@ -144,6 +153,66 @@ def test_sink_receives_a_report_without_them(self, sink: _Sink) -> None: assert len(sink.posts) == 1 +class TestLogMode: + def test_prints_the_event_and_sends_nothing( + self, + monkeypatch: pytest.MonkeyPatch, + sink: _Sink, + capsys: pytest.CaptureFixture[str], + ) -> None: + monkeypatch.setenv("DEEPCTL_TELEMETRY", "LOG") + client.init_telemetry(_config()) + with patch("sentry_sdk.flush") as flush: + assert _fail() is True + flush.assert_not_called() + sentry_sdk.get_client().close(timeout=2) + lines = [ln for ln in capsys.readouterr().err.splitlines() if ln.strip()] + assert len(lines) == 1, lines + event = json.loads(lines[0]) + assert event["exception"]["values"][0]["type"] == "RuntimeError" + assert "forced" not in lines[0] + assert sink.posts == [] + + def test_no_destination_is_configured( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("DEEPCTL_TELEMETRY", "log") + client.init_telemetry(_config()) + with patch("sentry_sdk.init", wraps=sentry_sdk.init) as init: + _fail() + assert init.call_args.kwargs["dsn"] is None + transport = sentry_sdk.get_client().transport + assert type(transport).__name__ == "StderrTransport" + + @pytest.mark.parametrize("value", ["off", "0", "false", "No", " disabled "]) + def test_off_values_turn_it_off( + self, monkeypatch: pytest.MonkeyPatch, sink: _Sink, value: str + ) -> None: + monkeypatch.setenv("DEEPCTL_TELEMETRY", value) + assert client.init_telemetry(_config()) is False + assert client._state["opt_out"] == "env" + with patch("sentry_sdk.init") as init: + assert _fail() is False + init.assert_not_called() + assert sink.posts == [] + + @pytest.mark.parametrize("value", ["on", "1", "true", "yes", "print"]) + def test_other_values_keep_the_default( + self, monkeypatch: pytest.MonkeyPatch, value: str + ) -> None: + monkeypatch.setenv("DEEPCTL_TELEMETRY", value) + assert client.init_telemetry(_config()) is True + + def test_other_values_send_as_usual( + self, monkeypatch: pytest.MonkeyPatch, sink: _Sink + ) -> None: + monkeypatch.setenv("DEEPCTL_TELEMETRY", "print") + client.init_telemetry(_config()) + assert _fail() is True + sentry_sdk.get_client().close(timeout=2) + assert len(sink.posts) == 1 + + class TestFlush: def test_no_flush_when_nothing_was_queued(self) -> None: client.init_telemetry(_config()) diff --git a/src/deepctl/_cache.py b/src/deepctl/_cache.py index 2f5191f4..5c287477 100644 --- a/src/deepctl/_cache.py +++ b/src/deepctl/_cache.py @@ -53,6 +53,7 @@ "DEEPGRAM_OUTPUT_COLOR", "DEEPCTL_TELEMETRY_DISABLED", "DO_NOT_TRACK", + "DEEPCTL_TELEMETRY", "CI", "DEEPCTL_EAGER_LOAD", "XDG_CONFIG_HOME", diff --git a/tests/unit/test_cache.py b/tests/unit/test_cache.py index 2eefbc4d..bf5a3daa 100644 --- a/tests/unit/test_cache.py +++ b/tests/unit/test_cache.py @@ -134,7 +134,12 @@ def test_clean_run_writes_a_private_entry_under_the_real_home( assert recorded["prog"] == "dg" and recorded["isatty"] is False assert not any("deepgram.yaml" in f["path"] for f in recorded["files"]) assert not any("cwd_relative" in f for f in recorded["files"]) - for name in ("DEEPCTL_TELEMETRY_DISABLED", "DO_NOT_TRACK", "CI"): + for name in ( + "DEEPCTL_TELEMETRY_DISABLED", + "DEEPCTL_TELEMETRY", + "DO_NOT_TRACK", + "CI", + ): assert name in recorded["env"], name assert "DEEPCTL_HELP_CACHE" not in recorded["env"] @@ -274,7 +279,7 @@ def test_key_size_in_a_bundle_configuration( key = _cache.help_key("dg", 80, False, b"") assert len(key["files"]) <= _cache.MAX_KEY_FILES == 32 assert _cache.MAX_KEY_ENV == 64 - assert len(key["env"]) == 20 + assert len(key["env"]) == 21 def _poison(stamp: str, commands: list[dict[str, Any]]) -> None: From afc76d4385bcfc7c5b88203ed46356d7c9a4d69d Mon Sep 17 00:00:00 2001 From: Corey Weathers Date: Sun, 1 Nov 2026 13:16:28 -0500 Subject: [PATCH 04/13] docs(telemetry): publish the report's field schema docs/telemetry.md lists every field a report can hold, the opt-outs in the order they are checked, the time budget and the log mode. A test sends a report through the real error path (an exception chain, with the command context filled in) and checks that its fields are exactly the documented ones. A missing doc fails the test. --- docs/telemetry.md | 64 ++++++++++++ .../src/deepctl_telemetry/client.py | 3 + .../tests/unit/test_contract.py | 98 ++++++++++++++++++- 3 files changed, 162 insertions(+), 3 deletions(-) create mode 100644 docs/telemetry.md diff --git a/docs/telemetry.md b/docs/telemetry.md new file mode 100644 index 00000000..a9ad2f1f --- /dev/null +++ b/docs/telemetry.md @@ -0,0 +1,64 @@ +# Telemetry + +dg sends one error report when a command fails with an unexpected error. Nothing is sent when a command succeeds, for a usage error or another error dg reports to you, or for `--help` and `--version`. The README's Telemetry section says where reports go. + +## When it runs + +- **At most one report per run**, and only for an unexpected error. +- **No network or reporting code on the success path.** The reporting library is loaded only after an unexpected error, so a command that succeeds opens no connection for telemetry. +- **A short time budget.** dg waits about 0.6 s at most for a report, loading included, and doesn't wait at all when no report was queued. With blocked or slow egress, a failing command takes at most that much longer. + +## Turn it off + +Any one of these turns telemetry off. They are checked in this order: + +| Switch | Scope | +|---|---| +| `--no-telemetry` | One command | +| `DEEPCTL_TELEMETRY_DISABLED=1` | A shell session, a CI job | +| `DEEPCTL_TELEMETRY=off` | A shell session, a CI job | +| `DO_NOT_TRACK=1` | Every tool that follows the [Console Do Not Track](https://consoledonottrack.com/) convention | +| `CI` set (as CI services do) | Any CI job | +| `telemetry.enabled: false` in your `config.yaml` | Always | + +For `DEEPCTL_TELEMETRY_DISABLED`, `DO_NOT_TRACK` and `CI`, any value except an empty one, `0`, `false`, `no` or `off` turns telemetry off. `DEEPCTL_TELEMETRY` turns it off when set to `off`, `0`, `false`, `no` or `disabled`; `log` is described below, and any other value changes nothing. A config file dg won't load (unreadable, too large, not valid YAML, a repeated `telemetry` key, or one dg doesn't trust in an AI agent session) counts as turned off too. A project's `deepgram.yaml` can't change these settings. The last line of `dg --help` shows whether telemetry is on. + +## See what would be sent + +Run a command with `DEEPCTL_TELEMETRY=log`. If it fails with an unexpected error and telemetry is on, dg prints the report it would have sent to stderr, as one JSON line, and sends nothing. + +```sh +DEEPCTL_TELEMETRY=log dg listen recording.wav +``` + +## Field schema + +A report holds these fields and nothing else. A value that doesn't match its expected shape (an identifier, a version string, a command path) is dropped, never shortened or escaped. Names from code outside dg and the libraries it calls, such as a plugin's, are sent as ``. + + +| Field | Contents | +|---|---| +| `event_id` | Random report ID (32 hex characters) | +| `timestamp` | When the error happened | +| `platform` | Always `python` | +| `level` | Always `error` | +| `release` | `deepctl@` and the dg version | +| `environment` | Always `production` | +| `sdk.name` | Name of the reporting library | +| `sdk.version` | Version of the reporting library | +| `tags.cmd.path` | Command path, such as `dg listen` | +| `tags.cmd.flags` | Names of the flags used, never their values, such as `--model,--output` | +| `tags.cli.os` | Operating system, such as `Darwin` | +| `tags.cli.arch` | CPU architecture, such as `arm64` | +| `tags.cli.python` | Python version, such as `3.12.4` | +| `exception.values[].type` | Exception type, and the type of each exception that led to it | +| `exception.values[].mechanism.type` | How the error was caught, such as `generic` | +| `exception.values[].mechanism.handled` | Whether dg caught it | +| `exception.values[].stacktrace.frames[].module` | Module of each stack frame | +| `exception.values[].stacktrace.frames[].function` | Function of each stack frame | +| `exception.values[].stacktrace.frames[].lineno` | Line number of each stack frame | + + +A report never holds flag values or arguments, file names or paths, environment variables, API keys, request or response data, error messages, log output, variable values, source code, your hostname or your username. + +A test (`packages/deepctl-telemetry/tests/unit/test_contract.py`) sends a report through the real error path and checks that its fields are exactly the ones in this table. diff --git a/packages/deepctl-telemetry/src/deepctl_telemetry/client.py b/packages/deepctl-telemetry/src/deepctl_telemetry/client.py index 65ecf72c..5420c70e 100644 --- a/packages/deepctl-telemetry/src/deepctl_telemetry/client.py +++ b/packages/deepctl-telemetry/src/deepctl_telemetry/client.py @@ -9,6 +9,9 @@ records the enabled decision and installs stdlib exception hooks; ``report_exception`` imports and initializes the SDK when an error actually reaches it, sends one event, and flushes within a fixed time budget. + +The field schema is published in docs/telemetry.md; a test keeps the two +in step. """ from __future__ import annotations diff --git a/packages/deepctl-telemetry/tests/unit/test_contract.py b/packages/deepctl-telemetry/tests/unit/test_contract.py index acadbf8c..6a5a3d90 100644 --- a/packages/deepctl-telemetry/tests/unit/test_contract.py +++ b/packages/deepctl-telemetry/tests/unit/test_contract.py @@ -1,12 +1,14 @@ """The telemetry runtime contract. Cross-tool opt-outs (DO_NOT_TRACK, CI), the DEEPCTL_TELEMETRY=log mode, the -flush that only waits for a queued event. Reports go to a local sink on loopback, never further. +flush that only waits for a queued event, and the published field schema in +docs/telemetry.md. Reports go to a local sink on loopback, never further. """ from __future__ import annotations import json +import re import socket import sys import tempfile @@ -22,6 +24,11 @@ import pytest import sentry_sdk from deepctl_telemetry import client +from sentry_sdk.transport import Transport + +REPO = Path(__file__).resolve().parents[4] +DOC = REPO / "docs" / "telemetry.md" + @pytest.fixture(autouse=True) def _env(monkeypatch: pytest.MonkeyPatch) -> Iterator[None]: @@ -231,11 +238,15 @@ def test_refused_connection_does_not_wait_out_the_budget( with socket.socket() as probe: probe.bind(("127.0.0.1", 0)) port = probe.getsockname()[1] + # A 4 s flush timeout and budget: returning well before them shows + # the flush ended with the failed send. + monkeypatch.setattr(client, "FLUSH_TIMEOUT_SECONDS", 4.0) + monkeypatch.setattr(client, "REPORT_BUDGET_SECONDS", 4.0) monkeypatch.setenv(client.DSN_ENV_VAR, f"http://k@127.0.0.1:{port}/1") client.init_telemetry(_config()) started = time.monotonic() assert _fail() is True - assert time.monotonic() - started < client.FLUSH_TIMEOUT_SECONDS + assert time.monotonic() - started < 2.5 def test_unanswered_send_is_held_to_the_budget( self, monkeypatch: pytest.MonkeyPatch @@ -245,9 +256,90 @@ def test_unanswered_send_is_held_to_the_budget( hole.bind(("127.0.0.1", 0)) hole.listen(8) port = hole.getsockname()[1] + # The budget (1.5 s) is under the flush timeout (6 s): the + # call ends at the budget, not the timeout. + monkeypatch.setattr(client, "FLUSH_TIMEOUT_SECONDS", 6.0) + monkeypatch.setattr(client, "REPORT_BUDGET_SECONDS", 1.5) monkeypatch.setenv(client.DSN_ENV_VAR, f"http://k@127.0.0.1:{port}/1") client.init_telemetry(_config()) started = time.monotonic() assert _fail() is True elapsed = time.monotonic() - started - assert elapsed < client.REPORT_BUDGET_SECONDS + 0.25 + assert elapsed < 1.5 + 2.0 + + +# ── Published schema ───────────────────────────────────────────────── + + +def _paths(value: Any, prefix: str = "") -> set[str]: + """Every leaf of `value` as a dotted path; list items as `[]`.""" + if isinstance(value, dict): + found: set[str] = set() + for key, child in value.items(): + found |= _paths(child, f"{prefix}.{key}" if prefix else key) + return found + if isinstance(value, list): + found = set() + for child in value: + found |= _paths(child, f"{prefix}[]") + return found + return {prefix} + + +def _documented() -> set[str]: + text = DOC.read_text(encoding="utf-8") + start = text.index("") + end = text.index("") + rows = [ln for ln in text[start:end].splitlines() if ln.startswith("| `")] + return {re.match(r"\| `([^`]+)`", row).group(1) for row in rows} # type: ignore[union-attr] + + +class _Capturing(Transport): + """Keeps the envelopes the SDK hands over; sends nothing.""" + + def __init__(self) -> None: + super().__init__() + self.events: list[dict[str, Any]] = [] + + def capture_envelope(self, envelope: Any) -> None: + for item in envelope.items: + if item.type == "event": + self.events.append(json.loads(item.get_bytes())) + + +def _raise_from() -> None: + try: + raise KeyError("inner") + except KeyError as inner: + raise ValueError("outer") from inner + + +def test_documented_fields_equal_what_a_report_holds( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The real error path, with every source of fields filled in: what the + SDK is handed has exactly the fields docs/telemetry.md lists.""" + assert DOC.is_file(), f"{DOC} is missing: the field schema must be published" + from deepctl_core import telemetry_context + + monkeypatch.setattr( + telemetry_context, + "command_context", + lambda: {"command": "dg listen", "flags": "--model"}, + ) + capturing = _Capturing() + real_init = sentry_sdk.init + + def init_with_capture(*args: Any, **kwargs: Any) -> Any: + kwargs["transport"] = capturing + return real_init(*args, **kwargs) + + monkeypatch.setattr(sentry_sdk, "init", init_with_capture) + client.init_telemetry(_config()) + try: + _raise_from() + except ValueError as exc: + assert client.report_exception(exc) is True + (event,) = capturing.events + assert len(event["exception"]["values"]) == 2 + assert _paths(event) == _documented() From c5ad9a48afb2558e05114de64006ef5b3bda4f76 Mon Sep 17 00:00:00 2001 From: Corey Weathers Date: Sun, 1 Nov 2026 17:08:39 -0500 Subject: [PATCH 05/13] fix(init): clone only Deepgram templates, with plain names The template service supplies both the repository URL and the template name, and dg init used them as given. It now clones only https://github.com/deepgram/ and https://github.com/deepgram-starters/: no other host, scheme, port, user name, query, fragment, extra path segment, whitespace or control character. Both the repository name and the template name must be plain: ASCII letters, digits, ".", "_" and "-", not starting with "-". Anything else is an error before the name is used in a path, a command or output, and nothing is cloned. --- .../src/deepctl_cmd_init/command.py | 28 ++- .../src/deepctl_cmd_init/lifecycle.py | 50 ++++++ .../src/deepctl_cmd_init/messages.py | 13 ++ .../tests/unit/test_init_repo.py | 167 ++++++++++++++++++ 4 files changed, 253 insertions(+), 5 deletions(-) create mode 100644 packages/deepctl-cmd-init/src/deepctl_cmd_init/messages.py create mode 100644 packages/deepctl-cmd-init/tests/unit/test_init_repo.py diff --git a/packages/deepctl-cmd-init/src/deepctl_cmd_init/command.py b/packages/deepctl-cmd-init/src/deepctl_cmd_init/command.py index 8da8359a..00b87eba 100644 --- a/packages/deepctl-cmd-init/src/deepctl_cmd_init/command.py +++ b/packages/deepctl-cmd-init/src/deepctl_cmd_init/command.py @@ -12,11 +12,14 @@ Config, DeepgramClient, ) -from deepctl_core.output import _agentic +from deepctl_core import messages as core_msg +from deepctl_core.output import _agentic, print_error from rich.console import Console +from rich.markup import escape from rich.table import Table from . import lifecycle, templates_api +from . import messages as msg from .models import InitResult, TemplateDetail, TemplateInfo console = Console() @@ -173,6 +176,25 @@ def handle( if not detail: return InitResult(status="cancelled", message="No template selected") + # ── Check what the template service sent ─────────────────────────────── + # The repository and the template name come from the service; both + # are checked before either is used in a path, a command or output. + github_url = detail.links.get("github") + if not github_url: + return InitResult(status="error", message="Template has no GitHub URL") + if not lifecycle.is_template_repo(github_url): + error = msg.E_INIT_REPO_REFUSED.format( + template=core_msg.display(detail.name) + ) + print_error(escape(error)) + return InitResult(status="error", message=error) + if not lifecycle.valid_name(detail.name): + error = msg.E_INIT_NAME_REFUSED.format( + template=core_msg.display(template_slug or detail.name) + ) + print_error(escape(error)) + return InitResult(status="error", message=error) + # ── Resolve target directory ─────────────────────────────────────────── dest = Path(target_dir) if target_dir else Path(f"./{detail.name}") dest = dest.resolve() @@ -186,10 +208,6 @@ def handle( return InitResult(status="cancelled", message="Clone cancelled") # ── Clone ────────────────────────────────────────────────────────────── - github_url = detail.links.get("github") - if not github_url: - return InitResult(status="error", message="Template has no GitHub URL") - console.print() with console.status("[blue]Cloning repository...[/blue]"): try: diff --git a/packages/deepctl-cmd-init/src/deepctl_cmd_init/lifecycle.py b/packages/deepctl-cmd-init/src/deepctl_cmd_init/lifecycle.py index 47afb667..0adcdce4 100644 --- a/packages/deepctl-cmd-init/src/deepctl_cmd_init/lifecycle.py +++ b/packages/deepctl-cmd-init/src/deepctl_cmd_init/lifecycle.py @@ -3,9 +3,11 @@ from __future__ import annotations import os +import re import shutil import subprocess from typing import TYPE_CHECKING +from urllib.parse import urlsplit from deepctl_core.output import print_hint, print_info, print_warning from rich.console import Console @@ -60,6 +62,54 @@ def check_prereqs() -> list[tuple[str, str, str]]: # ── Git clone ───────────────────────────────────────────────────────────────── +#: The GitHub organizations dg clones templates from. +TEMPLATE_ORGS = ("deepgram", "deepgram-starters") + +# A repository or template name: ASCII letters, digits, ".", "_" and "-", +# not starting with "-" (git or a shell would read it as an option). +_NAME = re.compile(r"[A-Za-z0-9._-]+") + + +def valid_name(name: object) -> bool: + """Whether ``name`` is a usable template or repository name.""" + return ( + isinstance(name, str) + and _NAME.fullmatch(name) is not None + and not name.startswith("-") + and name not in (".", "..") + ) + + +def is_template_repo(url: str) -> bool: + """Whether ``url`` is an https GitHub repository of a Deepgram organization. + + The template service supplies the URL, so only + ``https://github.com/deepgram/`` and + ``https://github.com/deepgram-starters/`` are cloned: no other + host, scheme, port, user name, query or fragment, and the repository + name is plain (``valid_name``): no escapes, separators or option-like + leading ``-``. + """ + if not url.isprintable() or any(c.isspace() for c in url): + return False # urlsplit drops tabs and newlines; git wouldn't + try: + parts = urlsplit(url) + port = parts.port + except ValueError: + return False + if parts.scheme != "https" or parts.netloc != "github.com" or port is not None: + return False + if parts.query or parts.fragment or "\\" in url: + return False + segments = parts.path.split("/") + # "", org, repo (and an optional trailing "") + if len(segments) == 4 and segments[3] == "": + segments = segments[:3] + if len(segments) != 3 or segments[0] != "": + return False + org, repo = segments[1], segments[2] + return org in TEMPLATE_ORGS and valid_name(repo) + def clone_template(github_url: str, target_dir: Path) -> None: """Clone a template repository, keeping the .git directory intact. diff --git a/packages/deepctl-cmd-init/src/deepctl_cmd_init/messages.py b/packages/deepctl-cmd-init/src/deepctl_cmd_init/messages.py new file mode 100644 index 00000000..6450451a --- /dev/null +++ b/packages/deepctl-cmd-init/src/deepctl_cmd_init/messages.py @@ -0,0 +1,13 @@ +"""Strings for ``dg init``'s errors.""" + +# The template service sent a name that isn't plain. +E_INIT_NAME_REFUSED = ( + 'Not using template "{template}": the template service sent a name that ' + 'isn\'t plain (letters, digits, ".", "_" and "-", not starting with "-").' +) + +# The template service named a repository outside Deepgram's organizations. +E_INIT_REPO_REFUSED = ( + 'Not cloning template "{template}": its repository is not on ' + "github.com/deepgram or github.com/deepgram-starters." +) diff --git a/packages/deepctl-cmd-init/tests/unit/test_init_repo.py b/packages/deepctl-cmd-init/tests/unit/test_init_repo.py new file mode 100644 index 00000000..5f0a449a --- /dev/null +++ b/packages/deepctl-cmd-init/tests/unit/test_init_repo.py @@ -0,0 +1,167 @@ +"""dg init clones templates only from Deepgram's GitHub organizations.""" + +from __future__ import annotations + +from unittest.mock import Mock, patch + +import pytest +from deepctl_cmd_init import lifecycle +from deepctl_cmd_init.command import InitCommand +from deepctl_cmd_init.models import InitResult, TemplateDetail +from deepctl_core import AuthManager, Config, DeepgramClient + + +def _detail(name: str, url: str) -> TemplateDetail: + return TemplateDetail( + name=name, + title="t", + description="d", + language="javascript", + links={"github": url}, + ) + + +def _handle(tmp_path, monkeypatch, detail: TemplateDetail, slug: str) -> object: + clone = Mock() + monkeypatch.setattr(lifecycle, "check_prereqs", lambda: []) + monkeypatch.setattr(lifecycle, "clone_template", clone) + auth = Mock(spec=AuthManager) + auth.get_api_key.return_value = None + with patch( + "deepctl_cmd_init.command.templates_api.get_template", return_value=detail + ): + result = InitCommand().handle( + config=Mock(spec=Config), + auth_manager=auth, + client=Mock(spec=DeepgramClient), + template=slug, + dir=None, + search=None, + list=False, + install=False, + no_install=True, + start=False, + no_start=True, + ) + clone.assert_not_called() + return result + + +class TestTemplateName: + """The template name comes from the service too: it must be plain.""" + + @pytest.mark.parametrize( + ("name", "slug", "url"), + [ + ("--yes", "x", "https://github.com/deepgram-starters/x"), + ("--yes", "--yes", "https://github.com/deepgram-starters/x"), + ("../x", "../x", "https://github.com/deepgram-starters/x"), + ("a b", "a b", "https://github.com/deepgram-starters/x"), + ], + ) + def test_refused(self, tmp_path, monkeypatch, name, slug, url) -> None: + monkeypatch.chdir(tmp_path) + result = _handle(tmp_path, monkeypatch, _detail(name, url), slug) + assert isinstance(result, InitResult) + assert result.status == "error" + assert "isn't plain" in (result.message or "") + assert list(tmp_path.iterdir()) == [] + + @pytest.mark.parametrize("name", ["node-transcription", "a.b_c-1", "X"]) + def test_valid_names(self, name: str) -> None: + assert lifecycle.valid_name(name) + + @pytest.mark.parametrize("name", ["", "-x", ".", "..", "a/b", "a b", "\u0435", 1]) + def test_invalid_names(self, name: object) -> None: + assert not lifecycle.valid_name(name) + + +class TestRepositoryCheck: + @pytest.mark.parametrize( + "url", + [ + "https://github.com/deepgram/deepgram-js-sdk", + "https://github.com/deepgram-starters/node-transcription", + "https://github.com/deepgram-starters/node-transcription/", + "https://github.com/deepgram-starters/node-transcription.git", + ], + ) + def test_accepted(self, url: str) -> None: + assert lifecycle.is_template_repo(url) + + @pytest.mark.parametrize( + "url", + [ + "http://github.com/deepgram/x", + "ssh://git@github.com/deepgram/x", + "git@github.com:deepgram/x.git", + "https://github.com/deepgramx/x", + "https://github.com/deepgram-starters-evil/x", + "https://github.com/someone/deepgram", + "https://github.com.evil.example/deepgram/x", + "https://evil.example/github.com/deepgram/x", + "https://user@github.com/deepgram/x", + "https://github.com:8443/deepgram/x", + "https://github.com/deepgram/x?ref=y", + "https://github.com/deepgram/x#y", + "https://github.com/deepgram/x/../../evil/y", + "https://github.com/deepgram/..", + "https://github.com/deepgram/", + "https://github.com/deepgram", + "https://github.com/deepgram/x/tree/main", + "--upload-pack=touch /tmp/pwned", + "file:///tmp/repo", + "https://GITHUB.com/deepgram/x", + "https://github.com\\@evil.example/deepgram/x", + "https://github.com/deepgram/x\n--upload-pack=touch /tmp/pwned", + "https://github.com/deepgram/x\t", + "https://github.com/deepgram/x y", + "https://github.com/deepgram/x\x1b[0m", + "https://github.com/deepgram/%2e%2e", + "https://github.com/deepgram/..%2fevil%2fx", + "https://github.com/deepgram/x%2F..%2F..%2Fevil%2Fy", + "https://github.com/deepgram/x%00", + "https://github.com/deepgram/-u", + "https://github.com/deepgram/--upload-pack=touch", + "https://github.com/deepgram/x;y", + "https://github.com/deepgram/\u0435vil", + "", + ], + ) + def test_refused(self, url: str) -> None: + assert not lifecycle.is_template_repo(url) + + def test_refused_repository_is_not_cloned(self, tmp_path, monkeypatch) -> None: + detail = TemplateDetail( + name="node-transcription", + title="Node Transcription", + description="d", + language="javascript", + links={"github": "https://evil.example/deepgram/x"}, + ) + clone = Mock() + monkeypatch.setattr(lifecycle, "check_prereqs", lambda: []) + monkeypatch.setattr(lifecycle, "clone_template", clone) + auth = Mock(spec=AuthManager) + auth.get_api_key.return_value = None + with patch( + "deepctl_cmd_init.command.templates_api.get_template", return_value=detail + ): + result = InitCommand().handle( + config=Mock(spec=Config), + auth_manager=auth, + client=Mock(spec=DeepgramClient), + template="node-transcription", + dir=str(tmp_path / "app"), + search=None, + list=False, + install=False, + no_install=True, + start=False, + no_start=True, + ) + assert isinstance(result, InitResult) + assert result.status == "error" + assert "github.com/deepgram-starters" in (result.message or "") + clone.assert_not_called() + assert not (tmp_path / "app").exists() From d4eed158a258aa1c5f9f6b76651492a0a1292bcd Mon Sep 17 00:00:00 2001 From: Corey Weathers Date: Sun, 1 Nov 2026 17:09:21 -0500 Subject: [PATCH 06/13] fix(init): keep repository settings from running programs Every git command dg init runs now goes through one helper that adds --no-pager and -c core.fsmonitor=false, core.hooksPath=/dev/null (NUL on Windows), core.pager=cat and diff.external= to the command line, so a repository's own configuration can't name a program for git to run. The commands dg runs inside a repository (rev-parse, ls-files and check-ignore) apply no filters and open no connection. The .env check also skips the global excludes file, which as a FIFO would block it. Inherited GIT_* variables are still dropped for commands in a repository; the clone keeps them, as before. --- .../src/deepctl_cmd_init/lifecycle.py | 79 +++++++---- .../deepctl-cmd-init/tests/unit/test_init.py | 8 +- .../tests/unit/test_init_git.py | 125 ++++++++++++++++++ 3 files changed, 185 insertions(+), 27 deletions(-) create mode 100644 packages/deepctl-cmd-init/tests/unit/test_init_git.py diff --git a/packages/deepctl-cmd-init/src/deepctl_cmd_init/lifecycle.py b/packages/deepctl-cmd-init/src/deepctl_cmd_init/lifecycle.py index 0adcdce4..ef062a4d 100644 --- a/packages/deepctl-cmd-init/src/deepctl_cmd_init/lifecycle.py +++ b/packages/deepctl-cmd-init/src/deepctl_cmd_init/lifecycle.py @@ -60,6 +60,50 @@ def check_prereqs() -> list[tuple[str, str, str]]: return missing +# ── Running git ─────────────────────────────────────────────────────────────── + +# Settings every git command dg runs gets on its command line, so that +# configuration inside a repository can't run a program: no fsmonitor +# daemon command, no hooks, no pager, no external diff. (The commands dg +# runs in a repository, rev-parse, ls-files and check-ignore, apply no +# filters and open no connection.) +_NULL_DEVICE = "NUL" if os.name == "nt" else "/dev/null" +GIT_SAFE_CONFIG = ( + "-c", + "core.fsmonitor=false", + "-c", + f"core.hooksPath={_NULL_DEVICE}", + "-c", + "core.pager=cat", + "-c", + "diff.external=", +) + + +def run_git( + *args: str, repo: Path | None = None, inherit_git_env: bool = False +) -> subprocess.CompletedProcess[str] | None: + """Run ``git [-C repo] args`` with GIT_SAFE_CONFIG; None if git can't run. + + Inherited GIT_* variables are dropped unless ``inherit_git_env`` (for a + clone, which works on no existing repository): GIT_DIR, GIT_WORK_TREE + or GIT_INDEX_FILE would point git at another repository. Call it with + ``repo`` only for a directory dg cloned itself. + """ + if shutil.which("git") is None: + return None + env = None + if not inherit_git_env: + env = {k: v for k, v in os.environ.items() if not k.startswith("GIT_")} + argv = ["git", "--no-pager", *GIT_SAFE_CONFIG] + if repo is not None: + argv += ["-C", str(repo)] + try: + return subprocess.run([*argv, *args], capture_output=True, text=True, env=env) + except OSError: + return None + + # ── Git clone ───────────────────────────────────────────────────────────────── #: The GitHub organizations dg clones templates from. @@ -127,13 +171,11 @@ def clone_template(github_url: str, target_dir: Path) -> None: if target_dir.exists(): raise RuntimeError(f"Directory already exists: {target_dir}") - result = subprocess.run( - # "--" ends git's options: a server-supplied URL that starts with "-" - # is taken as the repository, never as a git option. - ["git", "clone", "--", github_url, str(target_dir)], - capture_output=True, - text=True, - ) + # "--" ends git's options: a server-supplied URL that starts with "-" + # (say --upload-pack=...) is taken as the repository, never an option. + result = run_git("clone", "--", github_url, str(target_dir), inherit_git_env=True) + if result is None: + raise RuntimeError("git clone failed: git could not be run") if result.returncode != 0: raise RuntimeError(f"git clone failed:\n{result.stderr.strip()}") @@ -175,27 +217,16 @@ def _restrict_to_owner(path: Path) -> None: def _git(target_dir: Path, *args: str) -> int | None: - """Exit code of ``git -C target_dir args``, or None if git can't run.""" - if shutil.which("git") is None: - return None - try: - # Only this project's repository: an inherited GIT_DIR, GIT_WORK_TREE - # or GIT_INDEX_FILE would point git at another one. - env = {k: v for k, v in os.environ.items() if not k.startswith("GIT_")} - result = subprocess.run( - ["git", "-C", str(target_dir), *args], - capture_output=True, - text=True, - env=env, - ) - except OSError: - return None - return result.returncode + """Exit code of a git command in ``target_dir``, or None if git can't run.""" + result = run_git(*args, repo=target_dir) + return None if result is None else result.returncode def _env_ignored(target_dir: Path) -> bool | None: """git's answer to "is .env ignored?", or None when git can't say.""" - rc = _git(target_dir, "check-ignore", "-q", ".env") + # No global excludes file: one that is a FIFO would block the check. + # The repository's own .gitignore is what keeps .env out of a commit. + rc = _git(target_dir, "-c", "core.excludesFile=", "check-ignore", "-q", ".env") if rc in (0, 1): return rc == 0 return None diff --git a/packages/deepctl-cmd-init/tests/unit/test_init.py b/packages/deepctl-cmd-init/tests/unit/test_init.py index 95c794a5..45cebff3 100644 --- a/packages/deepctl-cmd-init/tests/unit/test_init.py +++ b/packages/deepctl-cmd-init/tests/unit/test_init.py @@ -412,12 +412,14 @@ def test_without_git_falls_back_to_pattern_check(self, tmp_path, monkeypatch): class TestCloneTemplate: def test_url_is_passed_after_end_of_options(self, tmp_path): - with patch.object(lifecycle.subprocess, "run") as run: + with patch.object(lifecycle.subprocess, "run") as run, patch.object( + lifecycle.shutil, "which", return_value="/usr/bin/git" + ): run.return_value = MagicMock(returncode=0) lifecycle.clone_template("--upload-pack=touch /tmp/pwned", tmp_path / "d") argv = run.call_args.args[0] - assert argv[:3] == ["git", "clone", "--"] - assert argv[3] == "--upload-pack=touch /tmp/pwned" + clone = argv.index("clone") + assert argv[clone : clone + 3] == ["clone", "--", "--upload-pack=touch /tmp/pwned"] def test_option_like_url_is_not_run_as_an_option(self, tmp_path): """With real git: an option-like URL fails as a repository name.""" diff --git a/packages/deepctl-cmd-init/tests/unit/test_init_git.py b/packages/deepctl-cmd-init/tests/unit/test_init_git.py new file mode 100644 index 00000000..7dfca6c9 --- /dev/null +++ b/packages/deepctl-cmd-init/tests/unit/test_init_git.py @@ -0,0 +1,125 @@ +"""Every git command dg init runs gets settings that keep configuration +inside a repository from running a program.""" + +from __future__ import annotations + +import os +import shutil +import subprocess +from typing import TYPE_CHECKING, Any + +import pytest +from deepctl_cmd_init import lifecycle + +if TYPE_CHECKING: + from pathlib import Path + +KEY = "fa1ce0000000000000000000000000000000000d" + + +def _git(repo: Path, *args: str) -> str: + env = {k: v for k, v in os.environ.items() if not k.startswith("GIT_")} + env.update( + GIT_CONFIG_NOSYSTEM="1", + GIT_AUTHOR_NAME="t", + GIT_AUTHOR_EMAIL="t@example.com", + GIT_COMMITTER_NAME="t", + GIT_COMMITTER_EMAIL="t@example.com", + ) + return subprocess.run( + ["git", "-C", str(repo), *args], + capture_output=True, + text=True, + env=env, + check=True, + ).stdout.strip() + + +def test_every_git_command_gets_the_safe_settings( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + calls: list[list[str]] = [] + + def fake(argv: Any, *args: Any, **kwargs: Any) -> Any: + calls.append(list(argv)) + return subprocess.CompletedProcess(argv, 0, "", "") + + monkeypatch.setattr(lifecycle.shutil, "which", lambda name: "/usr/bin/git") + monkeypatch.setattr(lifecycle.subprocess, "run", fake) + lifecycle.run_git("status", repo=tmp_path) + lifecycle.clone_template("https://github.com/deepgram/x", tmp_path / "c") + lifecycle.ensure_env_gitignored(tmp_path) + assert len(calls) >= 3 + for argv in calls: + assert argv[:2] == ["git", "--no-pager"] + assert "core.fsmonitor=false" in argv + assert any(a.startswith("core.hooksPath=") for a in argv) + assert "core.pager=cat" in argv + + +@pytest.mark.skipif(shutil.which("git") is None, reason="git not installed") +def test_repository_config_runs_nothing(tmp_path: Path) -> None: + """A repository whose own config names programs: writing .env (which + asks git whether .env is tracked and ignored) runs none of them.""" + repo = tmp_path / "repo" + repo.mkdir() + _git(repo, "init", "-q") + _git(repo, "commit", "-q", "--allow-empty", "-m", "x") + flag = tmp_path / "ran" + hooks = tmp_path / "hooks" + hooks.mkdir() + for name in ("post-index-change", "reference-transaction", "pre-auto-gc"): + hook = hooks / name + hook.write_text(f"#!/bin/sh\ntouch {flag}-{name}\n") + hook.chmod(0o755) + for key, value in ( + ("core.fsmonitor", f"touch {flag}-fsmonitor; false"), + ("core.hooksPath", str(hooks)), + ("core.pager", f"touch {flag}-pager"), + ("diff.external", f"touch {flag}-diff"), + ): + _git(repo, "config", key, value) + lifecycle.inject_env(repo, KEY) + assert (repo / ".env").exists() + assert lifecycle.run_git("status", "--porcelain", repo=repo) is not None + assert not list(tmp_path.glob("ran-*")) + + +@pytest.mark.skipif(shutil.which("git") is None, reason="git not installed") +def test_without_the_settings_fsmonitor_would_run(tmp_path: Path) -> None: + """Control: the same config does run code when git runs without them.""" + repo = tmp_path / "repo" + repo.mkdir() + _git(repo, "init", "-q") + _git(repo, "commit", "-q", "--allow-empty", "-m", "x") + flag = tmp_path / "ran-fsmonitor" + _git(repo, "config", "core.fsmonitor", f"touch {flag}; false") + subprocess.run(["git", "-C", str(repo), "status"], capture_output=True) + assert flag.exists() + + +@pytest.mark.skipif( + shutil.which("git") is None or not hasattr(os, "mkfifo"), reason="git, FIFOs" +) +def test_excludes_file_fifo_does_not_hang(tmp_path: Path) -> None: + """core.excludesFile naming a FIFO: the .env check doesn't block on it.""" + import threading + + repo = tmp_path / "repo" + repo.mkdir() + _git(repo, "init", "-q") + fifo = tmp_path / "excludes" + os.mkfifo(fifo) + _git(repo, "config", "core.excludesFile", str(fifo)) + done = threading.Event() + + def work() -> None: + lifecycle.ensure_env_gitignored(repo) + done.set() + + threading.Thread(target=work, daemon=True).start() + assert done.wait(15), "check-ignore blocked on the excludes FIFO" + # Unblock a stuck reader, if any, so the test process can exit. + if not done.is_set(): + with open(fifo, "w"): + pass From 454a6e00d8a090879afb079f62ae91b143c8c48d Mon Sep 17 00:00:00 2001 From: Corey Weathers Date: Sun, 1 Nov 2026 17:10:40 -0500 Subject: [PATCH 07/13] feat(init): resume a rerun in a clone dg init recorded dg init failed on any existing directory. It now records each clone it makes in its own state directory (~/.deepctl/init-clones, 0700): one owner-only file per clone, named after the directory's real path, holding that path, the template, its repository, the commit cloned, the phases done and the directory's device and inode. The file is written atomically and never through a link. A rerun into that directory resumes from the record, without cloning again or repeating finished setup, only when all of these hold: - the record names this real path and the template's repository; - the directory, opened without following a link, has the recorded device and inode, so one deleted and made again at the path doesn't count, and it and its .git (a real directory) belong to this user (on Windows, where inodes and owners aren't reliable, only the link and .git checks apply); - git then finds it the top of its own work tree, at the recorded commit. Nothing inside the directory is trusted for this, and no git command runs in it before the checks above. Any other existing directory, however it looks, is invalid_argument: no .env is written and no setup or start step runs. An empty directory can be cloned into. --- .../src/deepctl_cmd_init/clones.py | 192 +++++++++ .../src/deepctl_cmd_init/command.py | 145 +++++-- .../src/deepctl_cmd_init/lifecycle.py | 105 ++++- .../src/deepctl_cmd_init/messages.py | 9 + .../src/deepctl_cmd_init/models.py | 19 + .../tests/unit/_init_harness.py | 157 +++++++ .../deepctl-cmd-init/tests/unit/test_init.py | 6 + .../tests/unit/test_init_resume.py | 406 ++++++++++++++++++ 8 files changed, 1012 insertions(+), 27 deletions(-) create mode 100644 packages/deepctl-cmd-init/src/deepctl_cmd_init/clones.py create mode 100644 packages/deepctl-cmd-init/tests/unit/_init_harness.py create mode 100644 packages/deepctl-cmd-init/tests/unit/test_init_resume.py diff --git a/packages/deepctl-cmd-init/src/deepctl_cmd_init/clones.py b/packages/deepctl-cmd-init/src/deepctl_cmd_init/clones.py new file mode 100644 index 00000000..6beae8b5 --- /dev/null +++ b/packages/deepctl-cmd-init/src/deepctl_cmd_init/clones.py @@ -0,0 +1,192 @@ +"""dg init's record of the clones it made, in dg's own state directory. + +A rerun of ``dg init`` resumes in a directory only when this record names +it: nothing inside the directory (which anyone could have packed into an +archive) is trusted for that. Each record is one owner-only file under +``{state_dir}/init-clones/`` (0700), named after the directory's real path, +holding that path, the template, its repository, the commit dg cloned and +the phases done, and the clone directory's device and inode, so a +directory deleted and made again at the same path is not taken for it. +""" + +from __future__ import annotations + +import errno +import hashlib +import json +import os +import re +import secrets +import stat +import sys +from dataclasses import asdict, dataclass +from typing import Any + +from deepctl_core.login_state import StateDirError, _open_dir, state_dir + +DIR_NAME = "init-clones" +_MAX_BYTES = 16 * 1024 +_COMMIT = re.compile(r"[0-9a-f]{40}|[0-9a-f]{64}") +_NOFOLLOW = getattr(os, "O_NOFOLLOW", 0) +_NONBLOCK = getattr(os, "O_NONBLOCK", 0) +_DIRECTORY = getattr(os, "O_DIRECTORY", 0) +_BINARY = getattr(os, "O_BINARY", 0) + + +@dataclass(frozen=True) +class CloneRecord: + path: str + template: str + url: str + commit: str + phases: list[str] + dev: int + ino: int + + +def _name(real_path: str) -> str: + return hashlib.sha256(real_path.encode("utf-8", "surrogateescape")).hexdigest() + + +def _use_dir_fd() -> bool: + return sys.platform != "win32" and os.open in os.supports_dir_fd + + +def _open_records(*, create: bool) -> int: + """Open the records directory inside the state directory, both checked: + real directories owned by this user, the records directory 0700.""" + parent = _open_dir(state_dir(), create=create) + try: + if create: + try: + if _use_dir_fd(): + os.mkdir(DIR_NAME, 0o700, dir_fd=parent) + else: + os.mkdir(state_dir() / DIR_NAME, 0o700) + except FileExistsError: + pass + flags = os.O_RDONLY | _DIRECTORY | _NOFOLLOW + if _use_dir_fd(): + fd = os.open(DIR_NAME, flags, dir_fd=parent) + else: + fd = os.open(state_dir() / DIR_NAME, flags) + finally: + os.close(parent) + try: + st = os.fstat(fd) + if not stat.S_ISDIR(st.st_mode): + raise StateDirError(errno.ENOTDIR, "not a directory", DIR_NAME) + if sys.platform != "win32": + if st.st_uid != os.geteuid(): + raise StateDirError(errno.EPERM, "owned by another user", DIR_NAME) + if stat.S_IMODE(st.st_mode) & 0o077: + if not create: + raise StateDirError(errno.EPERM, "not private", DIR_NAME) + os.fchmod(fd, 0o700) + except BaseException: + os.close(fd) + raise + return fd + + +def _valid(data: Any, real_path: str) -> CloneRecord | None: + if not isinstance(data, dict) or data.get("path") != real_path: + return None + template, url, commit = data.get("template"), data.get("url"), data.get("commit") + phases = data.get("phases") + if not (isinstance(template, str) and isinstance(url, str)): + return None + if not isinstance(commit, str) or not _COMMIT.fullmatch(commit): + return None + if not isinstance(phases, list) or not all(isinstance(p, str) for p in phases): + return None + dev, ino = data.get("dev"), data.get("ino") + if not isinstance(dev, int) or isinstance(dev, bool): + return None + if not isinstance(ino, int) or isinstance(ino, bool): + return None + return CloneRecord(real_path, template, url, commit, list(phases), dev, ino) + + +def load(real_path: str) -> CloneRecord | None: + """The record for ``real_path``, or None when there is no usable one. + + Only an owner-only regular file with one link, opened without following + a link or blocking on a FIFO, in the checked records directory. + """ + try: + dir_fd = _open_records(create=False) + except OSError: + return None + name = _name(real_path) + ".json" + try: + flags = os.O_RDONLY | _NOFOLLOW | _NONBLOCK | _BINARY + if _use_dir_fd(): + fd = os.open(name, flags, dir_fd=dir_fd) + else: + fd = os.open(state_dir() / DIR_NAME / name, flags) + except OSError: + os.close(dir_fd) + return None + try: + st = os.fstat(fd) + if not stat.S_ISREG(st.st_mode) or st.st_size > _MAX_BYTES: + return None + if sys.platform != "win32" and ( + st.st_uid != os.geteuid() + or stat.S_IMODE(st.st_mode) & 0o077 + or st.st_nlink != 1 + ): + return None + raw = os.read(fd, _MAX_BYTES + 1) + except OSError: + return None + finally: + os.close(fd) + os.close(dir_fd) + try: + return _valid(json.loads(raw.decode("utf-8")), real_path) + except ValueError: + return None + + +def save(record: CloneRecord) -> None: + """Write ``record`` atomically: a new 0600 file (``O_CREAT|O_EXCL| + O_NOFOLLOW``) next to the target, then renamed over it. Raises OSError + (StateDirError for a state directory dg can't use).""" + data = (json.dumps(asdict(record), indent=2) + "\n").encode("utf-8") + name = _name(record.path) + ".json" + tmp = f".{name}.{secrets.token_hex(8)}.tmp" + dir_fd = _open_records(create=True) + base = state_dir() / DIR_NAME + try: + flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL | _NOFOLLOW | _BINARY + if _use_dir_fd(): + fd = os.open(tmp, flags, 0o600, dir_fd=dir_fd) + else: + fd = os.open(base / tmp, flags, 0o600) + try: + try: + if hasattr(os, "fchmod"): + os.fchmod(fd, 0o600) + view = memoryview(data) + while view: + view = view[os.write(fd, view) :] + os.fsync(fd) + finally: + os.close(fd) + if _use_dir_fd(): + os.replace(tmp, name, src_dir_fd=dir_fd, dst_dir_fd=dir_fd) + else: + os.replace(base / tmp, base / name) + except BaseException: + try: + if _use_dir_fd(): + os.unlink(tmp, dir_fd=dir_fd) + else: + os.unlink(base / tmp) + except OSError: + pass + raise + finally: + os.close(dir_fd) diff --git a/packages/deepctl-cmd-init/src/deepctl_cmd_init/command.py b/packages/deepctl-cmd-init/src/deepctl_cmd_init/command.py index 00b87eba..9a3a7ff6 100644 --- a/packages/deepctl-cmd-init/src/deepctl_cmd_init/command.py +++ b/packages/deepctl-cmd-init/src/deepctl_cmd_init/command.py @@ -2,6 +2,7 @@ from __future__ import annotations +import os from pathlib import Path from typing import Any @@ -13,14 +14,30 @@ DeepgramClient, ) from deepctl_core import messages as core_msg -from deepctl_core.output import _agentic, print_error +from deepctl_core.output import _agentic, print_error, print_hint from rich.console import Console from rich.markup import escape from rich.table import Table -from . import lifecycle, templates_api +from . import clones, lifecycle, templates_api from . import messages as msg -from .models import InitResult, TemplateDetail, TemplateInfo +from .models import InitCodedError, InitResult, TemplateDetail, TemplateInfo + + +def _coded( + error: str, + code: str, + hint: str, + next_step: str | None, + details: dict[str, Any] | None = None, +) -> InitCodedError: + """Report a coded error on stderr and return its payload.""" + print_error(escape(error)) + print_hint(escape(hint)) + return InitCodedError( + error=error, code=code, hint=hint, next_step=next_step, details=details + ) + console = Console() status_console = Console(stderr=True) @@ -199,27 +216,56 @@ def handle( dest = Path(target_dir) if target_dir else Path(f"./{detail.name}") dest = dest.resolve() - # ── Confirm ──────────────────────────────────────────────────────────── - self._print_template_summary(detail) - console.print(f"[dim]Destination:[/dim] {dest}") - console.print() + # ── Rerun contract: resume in a clone dg init made ───────────────────── + # Only dg's own record (in its state directory) says a directory is a + # clone dg made; nothing inside the directory is trusted for that, + # and no git command runs in it until the record does. + record: clones.CloneRecord | None = None + if lifecycle.directory_in_use(dest): + record = clones.load(os.path.realpath(dest)) + if record is None: + return self._dir_error(msg.E_INIT_DIR_IN_USE, detail, dest) + if record.template != detail.name: + return self._dir_error( + msg.E_INIT_DIR_OTHER_TEMPLATE, detail, dest, record.template + ) + # The same directory (not one made again at this path), owned by + # this user, before git runs in it; then git checks HEAD. + if ( + record.url != github_url + or not lifecycle.same_directory(dest, record.dev, record.ino) + or not lifecycle.clone_matches(dest, record.commit) + ): + return self._dir_error(msg.E_INIT_DIR_IN_USE, detail, dest) + phases: list[str] = list(record.phases) if record else [] + commit = record.commit if record else None + if record is None: + # ── Confirm ──────────────────────────────────────────────────────── + self._print_template_summary(detail) + console.print(f"[dim]Destination:[/dim] {dest}") + console.print() - if not _agentic and not self.confirm("Clone this template?", default=True): - return InitResult(status="cancelled", message="Clone cancelled") + if not _agentic and not self.confirm("Clone this template?", default=True): + return InitResult(status="cancelled", message="Clone cancelled") - # ── Clone ────────────────────────────────────────────────────────────── - console.print() - with console.status("[blue]Cloning repository...[/blue]"): - try: - lifecycle.clone_template(github_url, dest) - except RuntimeError as e: - return InitResult(status="error", message=str(e)) - - console.print(f"[green]✓[/green] Cloned to [cyan]{dest}[/cyan]") - console.print( - "[dim]The cloned directory is a full git repository — " - "you can add your own remote and push.[/dim]" - ) + # ── Clone ────────────────────────────────────────────────────────── + console.print() + with console.status("[blue]Cloning repository...[/blue]"): + try: + lifecycle.clone_template(github_url, dest) + except RuntimeError as e: + return InitResult(status="error", message=str(e)) + phases = ["clone"] + commit = lifecycle.head_commit(dest) + self._save_progress(dest, detail.name, github_url, commit, phases) + + console.print(f"[green]✓[/green] Cloned to [cyan]{dest}[/cyan]") + console.print( + "[dim]The cloned directory is a full git repository — " + "you can add your own remote and push.[/dim]" + ) + else: + console.print(f"[dim]Resuming in[/dim] [cyan]{dest}[/cyan]") # ── Inject API key ───────────────────────────────────────────────────── try: @@ -232,10 +278,10 @@ def handle( ) # ── Setup (make check-prereqs && make init) ──────────────────────────── - installed = False + installed = "setup" in phases has_makefile = (dest / "Makefile").exists() - if not no_install: + if not installed and not no_install: if _agentic or do_install: run_setup = True elif has_makefile: @@ -255,6 +301,9 @@ def handle( installed = lifecycle.run_lifecycle_step( detail.config.install, dest ) + if installed: + phases.append("setup") + self._save_progress(dest, detail.name, github_url, commit, phases) # ── Start step (optional) ────────────────────────────────────────────── started = False @@ -291,6 +340,54 @@ def handle( started=started, ) + # ── The rerun contract ──────────────────────────────────────────────────── + + def _save_progress( + self, + dest: Path, + template: str, + url: str, + commit: str | None, + phases: list[str], + ) -> None: + """Record the clone and what is done in dg's state, so a rerun + resumes. Without a commit there is nothing a rerun could check.""" + identity = lifecycle.directory_identity(dest) + if commit is None or identity is None: + return + record = clones.CloneRecord( + os.path.realpath(dest), template, url, commit, phases, *identity + ) + try: + clones.save(record) + except OSError as exc: + console.print( + f"[yellow]Could not record progress in {dest}:[/yellow] {exc}" + ) + + def _dir_error( + self, + message: str, + detail: TemplateDetail, + dest: Path, + other: str | None = None, + ) -> InitCodedError: + """A directory dg init can't clone into or resume in.""" + from deepctl_core.transport import prog + + program = prog() + error = message.format( + dest=core_msg.display_path(dest), + template=core_msg.display(detail.name), + other=core_msg.display(other or ""), + ) + return _coded( + error, + msg.CODE_INVALID_ARGUMENT, + msg.H_INIT_DIR, + f"{program} init --help" if program else None, + ) + def output_result(self, result: Any, config: Config) -> None: """Only emit structured output when an explicit --output format is set.""" from deepctl_core.output import get_output_format diff --git a/packages/deepctl-cmd-init/src/deepctl_cmd_init/lifecycle.py b/packages/deepctl-cmd-init/src/deepctl_cmd_init/lifecycle.py index ef062a4d..fb806c53 100644 --- a/packages/deepctl-cmd-init/src/deepctl_cmd_init/lifecycle.py +++ b/packages/deepctl-cmd-init/src/deepctl_cmd_init/lifecycle.py @@ -5,6 +5,7 @@ import os import re import shutil +import stat import subprocess from typing import TYPE_CHECKING from urllib.parse import urlsplit @@ -155,6 +156,15 @@ def is_template_repo(url: str) -> bool: return org in TEMPLATE_ORGS and valid_name(repo) +def directory_in_use(target_dir: Path) -> bool: + """Whether ``target_dir`` exists as anything but an empty directory.""" + if target_dir.is_symlink() or (target_dir.exists() and not target_dir.is_dir()): + return True + if not target_dir.exists(): + return False + return any(target_dir.iterdir()) + + def clone_template(github_url: str, target_dir: Path) -> None: """Clone a template repository, keeping the .git directory intact. @@ -163,12 +173,13 @@ def clone_template(github_url: str, target_dir: Path) -> None: Args: github_url: Full GitHub URL to clone. - target_dir: Local directory to clone into (must not exist). + target_dir: Local directory to clone into (must not exist, or be + an empty directory). Raises: - RuntimeError: If the directory already exists or git clone fails. + RuntimeError: If the directory is in use or git clone fails. """ - if target_dir.exists(): + if directory_in_use(target_dir): raise RuntimeError(f"Directory already exists: {target_dir}") # "--" ends git's options: a server-supplied URL that starts with "-" @@ -180,6 +191,94 @@ def clone_template(github_url: str, target_dir: Path) -> None: raise RuntimeError(f"git clone failed:\n{result.stderr.strip()}") +# ── The clone dg init made ──────────────────────────────────────────────────── + +_COMMIT = re.compile(r"[0-9a-f]{40}|[0-9a-f]{64}") + + +def _git_out(target_dir: Path, *args: str) -> str | None: + """Output of a git command in ``target_dir``, or None if it failed.""" + result = run_git(*args, repo=target_dir) + if result is None or result.returncode != 0: + return None + return result.stdout.strip() or None + + +def head_commit(target_dir: Path) -> str | None: + """``HEAD`` of the repository dg just cloned into ``target_dir``.""" + commit = _git_out(target_dir, "rev-parse", "--verify", "HEAD") + return commit if commit and _COMMIT.fullmatch(commit) else None + + +_O_DIRECTORY = getattr(os, "O_DIRECTORY", 0) + + +def same_directory(target_dir: Path, dev: int, ino: int) -> bool: + """Whether ``target_dir`` is still the directory dg cloned into. + + Checked before any git command runs in it. The directory, opened + without following a symbolic link, must have the recorded device and + inode (a directory deleted and made again at the same path has another + inode), and it and its ``.git`` must be owned by this user, with + ``.git`` a real directory. + + On Windows, where ``st_ino`` can be 0 or unstable and there are no + POSIX owners, only the path checks apply: no link, ``.git`` a real + directory. + """ + if os.name == "nt": + try: + top = os.lstat(target_dir) + git = os.lstat(target_dir / ".git") + except OSError: + return False + return stat.S_ISDIR(top.st_mode) and stat.S_ISDIR(git.st_mode) + try: + fd = os.open(target_dir, os.O_RDONLY | _O_DIRECTORY | _NOFOLLOW) + except OSError: + return False + try: + st = os.fstat(fd) + git = os.stat(".git", dir_fd=fd, follow_symlinks=False) + except OSError: + return False + finally: + os.close(fd) + uid = os.geteuid() + return ( + stat.S_ISDIR(st.st_mode) + and stat.S_ISDIR(git.st_mode) + and (st.st_dev, st.st_ino) == (dev, ino) + and st.st_uid == uid + and git.st_uid == uid + ) + + +def directory_identity(target_dir: Path) -> tuple[int, int] | None: + """Device and inode of ``target_dir`` (not following a link), or None.""" + try: + st = os.lstat(target_dir) + except OSError: + return None + return (st.st_dev, st.st_ino) if stat.S_ISDIR(st.st_mode) else None + + +def clone_matches(target_dir: Path, commit: str) -> bool: + """Whether ``target_dir`` is still at ``commit``, the clone dg recorded. + + Only for a directory dg's own clone record names: git runs in it. Its + own ``.git`` directory (not a link, or a file pointing elsewhere) must + make it the top of a work tree, with ``HEAD`` at ``commit``. + """ + git_dir = target_dir / ".git" + if git_dir.is_symlink() or not git_dir.is_dir(): + return False + top = _git_out(target_dir, "rev-parse", "--show-toplevel") + if top is None or os.path.realpath(top) != os.path.realpath(target_dir): + return False + return head_commit(target_dir) == commit + + # ── .env injection ──────────────────────────────────────────────────────────── diff --git a/packages/deepctl-cmd-init/src/deepctl_cmd_init/messages.py b/packages/deepctl-cmd-init/src/deepctl_cmd_init/messages.py index 6450451a..4928bbf7 100644 --- a/packages/deepctl-cmd-init/src/deepctl_cmd_init/messages.py +++ b/packages/deepctl-cmd-init/src/deepctl_cmd_init/messages.py @@ -1,5 +1,14 @@ """Strings for ``dg init``'s errors.""" +CODE_INVALID_ARGUMENT = "invalid_argument" + +# Rerun contract: a directory dg init can't resume in. +E_INIT_DIR_IN_USE = "Directory already exists: {dest}" +E_INIT_DIR_OTHER_TEMPLATE = ( + 'Directory {dest} holds the template "{other}", not "{template}".' +) +H_INIT_DIR = "Choose another directory with --dir, or remove this one." + # The template service sent a name that isn't plain. E_INIT_NAME_REFUSED = ( 'Not using template "{template}": the template service sent a name that ' diff --git a/packages/deepctl-cmd-init/src/deepctl_cmd_init/models.py b/packages/deepctl-cmd-init/src/deepctl_cmd_init/models.py index d14d2c7b..aef5977d 100644 --- a/packages/deepctl-cmd-init/src/deepctl_cmd_init/models.py +++ b/packages/deepctl-cmd-init/src/deepctl_cmd_init/models.py @@ -5,6 +5,7 @@ from typing import Any from deepctl_core import BaseResult +from deepctl_core.models import CodedErrorResult from pydantic import BaseModel, Field @@ -77,3 +78,21 @@ class InitResult(BaseResult): directory: str | None = None installed: bool = False started: bool = False + + +class InitCodedError(CodedErrorResult): + """A 0.5.0 coded error from ``dg init``. + + ``next_step`` is always present (null when no command fixes it); + ``details`` is left out when there are none. + """ + + next_step: str | None = None + schema_version: int = 1 + details: dict[str, Any] | None = None + + def model_dump(self, **kwargs: Any) -> dict[str, Any]: + data = super().model_dump(**kwargs) + if not data.get("details"): + data.pop("details", None) + return data diff --git a/packages/deepctl-cmd-init/tests/unit/_init_harness.py b/packages/deepctl-cmd-init/tests/unit/_init_harness.py new file mode 100644 index 00000000..2fc1e133 --- /dev/null +++ b/packages/deepctl-cmd-init/tests/unit/_init_harness.py @@ -0,0 +1,157 @@ +"""Shared harness for dg init's command tests: a fake clone (a real git +repository when git is installed), recorded setup and start steps. + +The tests import it and define their own fixtures; a conftest.py in this +directory would clash with other packages' tests/unit/conftest.py.""" + +from __future__ import annotations + +import os +import shutil +import subprocess +from typing import TYPE_CHECKING, Any +from unittest.mock import Mock, patch + +import pytest +from deepctl_cmd_init import clones, lifecycle +from deepctl_cmd_init.command import InitCommand +from deepctl_cmd_init.models import LifecycleStep, TemplateDetail, TomlConfig +from deepctl_core import AuthManager, Config, DeepgramClient + +if TYPE_CHECKING: + from pathlib import Path + +HAVE_GIT = shutil.which("git") is not None +needs_git = pytest.mark.skipif(not HAVE_GIT, reason="git not installed") + + +def git(repo: Path, *args: str) -> str: + env = {k: v for k, v in os.environ.items() if not k.startswith("GIT_")} + env.update( + GIT_CONFIG_NOSYSTEM="1", + GIT_AUTHOR_NAME="t", + GIT_AUTHOR_EMAIL="t@example.com", + GIT_COMMITTER_NAME="t", + GIT_COMMITTER_EMAIL="t@example.com", + ) + return subprocess.run( + ["git", "-C", str(repo), *args], + capture_output=True, + text=True, + env=env, + check=True, + ).stdout.strip() + + +REPO_URL = "https://github.com/deepgram-starters/node-transcription" + + +def template_detail( + *, + install: str | list[str] | None = None, + start: str | None = None, + url: str = REPO_URL, + name: str = "node-transcription", +) -> TemplateDetail: + config = None + if install is not None or start is not None: + config = TomlConfig( + install=LifecycleStep(command=install) if install is not None else None, + start=LifecycleStep(command=start) if start is not None else None, + ) + return TemplateDetail( + name=name, + title="Node Transcription", + description="Get started with Node", + language="javascript", + links={"github": url}, + config=config, + ) + + +class Run: + """Drive InitCommand.handle with a fake clone and recorded steps.""" + + def __init__(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + self.dest = tmp_path / "app" + # dg's state directory (the clone records) lives under HOME. + self.home = tmp_path / "home" + self.home.mkdir() + monkeypatch.setenv("HOME", str(self.home)) + self.makefile = False + self.clones = 0 + self.setup_runs: list[Any] = [] + self.start_runs: list[Any] = [] + self.person = False + self.agent = False + monkeypatch.setattr(lifecycle, "check_prereqs", lambda: []) + monkeypatch.setattr(lifecycle, "clone_template", self._clone) + monkeypatch.setattr(lifecycle, "run_make_init", self._make) + monkeypatch.setattr(lifecycle, "run_lifecycle_step", self._step) + monkeypatch.setattr("deepctl_core.transport.prog", lambda: "dg") + monkeypatch.setattr( + "deepctl_cmd_init.command._person_at_terminal", + lambda: self.person, + raising=False, + ) + monkeypatch.setattr( + "deepctl_cmd_init.command._hard_signal", lambda: self.agent, raising=False + ) + self.detail = template_detail() + + def _clone(self, url: str, dest: Path) -> None: + """A clone: a real git repository with ``origin`` set when git is + installed (a resume checks both), else a plain directory.""" + assert not lifecycle.directory_in_use(dest) + self.clones += 1 + dest.mkdir(exist_ok=True) + (dest / "README.md").write_text("starter\n") + if self.makefile: + (dest / "Makefile").write_text("init:\n") + if HAVE_GIT: + git(dest, "init", "-q") + git(dest, "add", "-A") + git(dest, "commit", "-q", "-m", "init") + git(dest, "remote", "add", "origin", url) + + def _make(self, dest: Path) -> bool: # noqa: ARG002 + self.setup_runs.append("make") + return True + + def _step(self, step: LifecycleStep, dest: Path) -> bool: # noqa: ARG002 + assert self.detail.config is not None + if step is self.detail.config.start: + self.start_runs.append(step.command) + else: + self.setup_runs.append(step.command) + return True + + def __call__(self, api_key: str | None = None, **flags: Any) -> Any: + kwargs = { + "template": self.detail.name, + "dir": str(self.dest), + "search": None, + "list": False, + "install": False, + "no_install": False, + "start": False, + "no_start": False, + "yes": False, + **flags, + } + auth = Mock(spec=AuthManager) + auth.get_api_key.return_value = api_key + with patch( + "deepctl_cmd_init.command.templates_api.get_template", + return_value=self.detail, + ): + return InitCommand().handle( + config=Mock(spec=Config), + auth_manager=auth, + client=Mock(spec=DeepgramClient), + **kwargs, + ) + + def record(self) -> Any: + """dg's record of the clone in ``dest``, or None.""" + return clones.load(os.path.realpath(self.dest)) diff --git a/packages/deepctl-cmd-init/tests/unit/test_init.py b/packages/deepctl-cmd-init/tests/unit/test_init.py index 45cebff3..baa3086b 100644 --- a/packages/deepctl-cmd-init/tests/unit/test_init.py +++ b/packages/deepctl-cmd-init/tests/unit/test_init.py @@ -538,6 +538,8 @@ def test_direct_clone( ): mock_api.get_template.return_value = sample_detail mock_lifecycle.check_prereqs.return_value = [] + mock_lifecycle.directory_in_use.return_value = False + mock_lifecycle.head_commit.return_value = None mock_lifecycle.clone_template.return_value = None mock_lifecycle.run_lifecycle_step.return_value = True @@ -580,6 +582,8 @@ def test_expired_credential_warning_is_visible( ): mock_api.get_template.return_value = sample_detail mock_lifecycle.check_prereqs.return_value = [] + mock_lifecycle.directory_in_use.return_value = False + mock_lifecycle.head_commit.return_value = None mock_lifecycle.clone_template.return_value = None mock_auth_manager.get_api_key.side_effect = AuthenticationError( "Your Deepgram credential has expired. Run `dg login` again." @@ -646,6 +650,8 @@ def test_api_key_injection( ): mock_api.get_template.return_value = sample_detail mock_lifecycle.check_prereqs.return_value = [] + mock_lifecycle.directory_in_use.return_value = False + mock_lifecycle.head_commit.return_value = None mock_lifecycle.clone_template.return_value = None mock_auth_manager.get_api_key.return_value = "sk-my-key" diff --git a/packages/deepctl-cmd-init/tests/unit/test_init_resume.py b/packages/deepctl-cmd-init/tests/unit/test_init_resume.py new file mode 100644 index 00000000..b214a542 --- /dev/null +++ b/packages/deepctl-cmd-init/tests/unit/test_init_resume.py @@ -0,0 +1,406 @@ +"""dg init's rerun contract: a rerun into a clone dg made resumes there, by +dg's own record of it; any other directory is refused untouched, and no git +command runs in it.""" + +from __future__ import annotations + +import importlib.util +import json +import os +import shutil +import stat +import subprocess +import sys +import tarfile +from pathlib import Path +from typing import TYPE_CHECKING, Any + +import pytest +from deepctl_cmd_init import clones, lifecycle +from deepctl_cmd_init.models import InitResult + +# The shared harness, loaded by path under its own name: a relative import +# depends on which package pytest's importlib mode names "tests.unit". +_HARNESS = "deepctl_cmd_init_tests_harness" +if _HARNESS not in sys.modules: + _spec = importlib.util.spec_from_file_location( + _HARNESS, Path(__file__).with_name("_init_harness.py") + ) + assert _spec is not None and _spec.loader is not None + sys.modules[_HARNESS] = importlib.util.module_from_spec(_spec) + _spec.loader.exec_module(sys.modules[_HARNESS]) +Run = sys.modules[_HARNESS].Run +git = sys.modules[_HARNESS].git +template_detail = sys.modules[_HARNESS].template_detail + +needs_git = pytest.mark.skipif(shutil.which("git") is None, reason="git not installed") + + +@pytest.fixture +def run(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Run: + return Run(tmp_path, monkeypatch) + + +@pytest.fixture +def make_detail() -> Any: + """``make_detail(install=..., start=..., url=..., name=...)``.""" + return template_detail + + +@pytest.fixture +def git_cmd() -> Any: + """``git_cmd(repo, *args)``: run git with a clean environment.""" + return git + + +KEY = "fa1ce0000000000000000000000000000000000d" +FORGED_MAKEFILE = "check-prereqs:\n\t@true\ninit:\n\ttouch {flag}\n" + + +def _assert_untouched(run: Any, env_before: str | None = None) -> None: + assert run.clones == 0 + assert run.setup_runs == run.start_runs == [] + env = run.dest / ".env" + if env_before is None: + assert not env.exists() + else: + assert env.read_text() == env_before + + +@needs_git +class TestResume: + def test_rerun_resumes_without_cloning_again(self, run: Any) -> None: + run.makefile = True + assert run(no_install=True).status == "success" + record = run.record() + assert record is not None and record.phases == ["clone"] + assert len(record.commit) == 40 + assert record.url == run.detail.links["github"] + result = run(no_install=True) + assert isinstance(result, InitResult) + assert result.status == "success" + assert run.clones == 1 + + def test_finished_setup_is_not_run_again(self, run: Any) -> None: + run.makefile = True + run(install=True, no_start=True) + run(install=True, no_start=True) + assert run.setup_runs == ["make"] + assert run.record().phases == ["clone", "setup"] + + def test_other_template_is_invalid_argument( + self, run: Any, make_detail: Any + ) -> None: + run(no_install=True) + run.detail = make_detail( + name="flask-transcription", + url="https://github.com/deepgram-starters/flask-transcription", + ) + result = run(no_install=True) + assert result.code == "invalid_argument" + assert "flask-transcription" in result.error + assert result.next_step == "dg init --help" + assert run.clones == 1 + + def test_nothing_is_written_into_the_clone(self, run: Any, git_cmd: Any) -> None: + run(no_install=True) + assert git_cmd(run.dest, "status", "--porcelain") == "" + + def test_clone_moved_on_is_not_resumed(self, run: Any, git_cmd: Any) -> None: + run(no_install=True) + git_cmd(run.dest, "commit", "-q", "--allow-empty", "-m", "later") + assert run(api_key=KEY, install=True).code == "invalid_argument" + assert run.setup_runs == [] + + def test_record_for_another_path_does_not_count( + self, run: Any, tmp_path: Path + ) -> None: + run(no_install=True) + moved = tmp_path / "moved" + run.dest.rename(moved) + run.dest = moved + assert run(api_key=KEY, install=True).code == "invalid_argument" + assert not (moved / ".env").exists() + + +def _record_git(monkeypatch: pytest.MonkeyPatch) -> list[list[str]]: + """Every git command dg runs from here on (and still runs it).""" + calls: list[list[str]] = [] + real_run = subprocess.run + + def recording(argv: Any, *args: Any, **kwargs: Any) -> Any: + calls.append(list(argv)) + return real_run(argv, *args, **kwargs) + + monkeypatch.setattr(lifecycle.subprocess, "run", recording) + return calls + + +@needs_git +class TestRecordedDirectoryReplaced: + """The record names a directory, not just a path: one made again at the + path, one owned by someone else, or a link swapped in is refused before + any git command runs in it.""" + + def _clone(self, run: Any) -> None: + assert run(no_install=True).status == "success" + assert run.record() is not None + + def _refused(self, run: Any, calls: list[list[str]], **flags: Any) -> None: + result = run(api_key=KEY, **flags) + assert result.code == "invalid_argument" + assert run.setup_runs == run.start_runs == [] + assert not [c for c in calls if "-C" in c], calls + + def test_made_again_at_the_same_path( + self, run: Any, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + self._clone(run) + copy = tmp_path / "copy" + shutil.copytree(run.dest, copy, symlinks=True) + shutil.rmtree(run.dest) + copy.rename(run.dest) # same contents and HEAD, another inode + calls = _record_git(monkeypatch) + self._refused(run, calls, install=True) + assert not (run.dest / ".env").exists() + + def test_owned_by_someone_else( + self, run: Any, monkeypatch: pytest.MonkeyPatch + ) -> None: + self._clone(run) + uid = os.geteuid() + monkeypatch.setattr(lifecycle.os, "geteuid", lambda: uid + 1) + calls = _record_git(monkeypatch) + self._refused(run, calls, install=True) + assert not (run.dest / ".env").exists() + + def test_link_swapped_in_after_the_record_check( + self, run: Any, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + self._clone(run) + real = tmp_path / "real" + real_load = clones.load + + def load_then_swap(path: str) -> Any: + record = real_load(path) + run.dest.rename(real) + run.dest.symlink_to(real) + return record + + monkeypatch.setattr(clones, "load", load_then_swap) + calls = _record_git(monkeypatch) + self._refused(run, calls, install=True, yes=True) + assert not (real / ".env").exists() + + +class TestDirectoryNotFromInit: + def test_non_empty_directory_without_record(self, run: Any) -> None: + run.dest.mkdir() + (run.dest / "notes.txt").write_text("mine\n") + result = run(no_install=True) + assert result.code == "invalid_argument" + assert result.error.startswith("Directory already exists: ") + assert run.clones == 0 + + def test_empty_directory_is_cloned_into(self, run: Any) -> None: + run.dest.mkdir() + assert run(no_install=True).status == "success" + assert run.clones == 1 + + def test_a_file_in_the_way(self, run: Any) -> None: + run.dest.write_text("x") + assert run(no_install=True).code == "invalid_argument" + + +def _forge(dest: Path, tmp_path: Path, git_cmd: Any, flag: Path) -> None: + """What an archive or a "fork this starter" zip can deliver: its own + .git with origin set to the Deepgram repository, HEAD committed, and a + 0600 .deepctl-init.json naming HEAD (the marker earlier versions + trusted). Built elsewhere and unpacked from a tarball.""" + work = tmp_path / "forge" / "node-transcription" + work.mkdir(parents=True) + (work / "Makefile").write_text(FORGED_MAKEFILE.format(flag=flag)) + git_cmd(work, "init", "-q") + git_cmd(work, "add", "-A") + git_cmd(work, "commit", "-q", "-m", "x") + git_cmd(work, "remote", "add", "origin", template_detail().links["github"]) + head = git_cmd(work, "rev-parse", "HEAD") + marker = work / ".deepctl-init.json" + marker.write_text( + json.dumps( + {"template": "node-transcription", "commit": head, "phases": ["clone"]} + ) + ) + marker.chmod(0o600) + archive = tmp_path / "forge.tar" + with tarfile.open(archive, "w") as tar: + tar.add(work, arcname=".") + dest.mkdir() + with tarfile.open(archive) as tar: + tar.extractall(dest, filter="tar") # type: ignore[call-arg] + + +@needs_git +class TestForgedClone: + """A self-consistent forged clone gets nothing done under an AI agent: + no .env, no setup, no start, and no git command in it.""" + + @pytest.fixture(autouse=True) + def _agent(self, run: Any) -> None: + run.agent = True + + @pytest.mark.parametrize( + "flags", + [{}, {"yes": True}, {"yes": True, "start": True}, {"install": True}], + ) + def test_refused( + self, + run: Any, + git_cmd: Any, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + make_detail: Any, + flags: dict[str, Any], + ) -> None: + run.detail = make_detail(install="npm install", start="npm start") + flag = tmp_path / "pwned" + _forge(run.dest, tmp_path, git_cmd, flag) + calls: list[list[str]] = [] + real_run = subprocess.run + + def recording(argv: Any, *args: Any, **kwargs: Any) -> Any: + calls.append(list(argv)) + return real_run(argv, *args, **kwargs) + + monkeypatch.setattr(lifecycle.subprocess, "run", recording) + result = run(api_key=KEY, **flags) + assert result.code == "invalid_argument" + assert result.error.startswith("Directory already exists: ") + _assert_untouched(run) + assert not flag.exists() + assert not [c for c in calls if str(run.dest) in " ".join(c)], calls + + +class TestRecord: + @pytest.fixture(autouse=True) + def _home(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("HOME", str(tmp_path / "home")) + + def _record(self, path: str = "/x/app") -> Any: + return clones.CloneRecord( + path, "t", "https://github.com/deepgram/t", "a" * 40, ["clone"], 1, 2 + ) + + def _file(self, path: str = "/x/app") -> Path: + name = clones._name(path) + ".json" + return Path(os.environ["HOME"]) / ".deepctl" / clones.DIR_NAME / name + + def test_round_trip_private(self) -> None: + clones.save(self._record()) + assert clones.load("/x/app") == self._record() + assert clones.load("/x/other") is None + if os.name == "posix": + assert stat.S_IMODE(self._file().stat().st_mode) == 0o600 + assert stat.S_IMODE(self._file().parent.stat().st_mode) == 0o700 + + @pytest.mark.skipif(os.name != "posix", reason="POSIX modes") + def test_loose_mode_is_ignored(self) -> None: + clones.save(self._record()) + self._file().chmod(0o644) + assert clones.load("/x/app") is None + + @pytest.mark.skipif(os.name != "posix", reason="POSIX modes") + def test_loose_directory_is_ignored(self) -> None: + clones.save(self._record()) + self._file().parent.chmod(0o755) + assert clones.load("/x/app") is None + + def test_link_is_not_followed(self, tmp_path: Path) -> None: + clones.save(self._record()) + real = tmp_path / "real.json" + self._file().rename(real) + self._file().symlink_to(real) + assert clones.load("/x/app") is None + + def test_save_replaces_a_link_without_writing_through_it( + self, tmp_path: Path + ) -> None: + clones.save(self._record()) + outside = tmp_path / "outside" + outside.write_text("keep\n") + self._file().unlink() + self._file().symlink_to(outside) + clones.save(self._record()) + assert outside.read_text() == "keep\n" + assert not self._file().is_symlink() + + @pytest.mark.skipif(not hasattr(os, "mkfifo"), reason="needs FIFOs") + def test_fifo_is_not_read(self) -> None: + clones.save(self._record()) + self._file().unlink() + os.mkfifo(self._file()) + assert clones.load("/x/app") is None + + @pytest.mark.parametrize( + "data", + [ + [], + { + "path": "/x/other", + "template": "t", + "url": "u", + "commit": "a" * 40, + "phases": [], + "dev": 1, + "ino": 2, + }, + { + "path": "/x/app", + "template": 1, + "url": "u", + "commit": "a" * 40, + "phases": [], + "dev": 1, + "ino": 2, + }, + { + "path": "/x/app", + "template": "t", + "url": "u", + "commit": "HEAD", + "phases": [], + "dev": 1, + "ino": 2, + }, + { + "path": "/x/app", + "template": "t", + "url": "u", + "commit": "a" * 40, + "phases": [1], + "dev": 1, + "ino": 2, + }, + { + "path": "/x/app", + "template": "t", + "url": "u", + "commit": "a" * 40, + "phases": [], + "ino": 2, + }, + { + "path": "/x/app", + "template": "t", + "url": "u", + "commit": "a" * 40, + "phases": [], + "dev": True, + "ino": 2, + }, + ], + ) + def test_invalid_content_is_ignored(self, data: Any) -> None: + clones.save(self._record()) + self._file().write_text(json.dumps(data)) + assert clones.load("/x/app") is None From b705938dbacdec99c94927259663bf6a9497ddd0 Mon Sep 17 00:00:00 2001 From: Corey Weathers Date: Sun, 1 Nov 2026 17:11:30 -0500 Subject: [PATCH 08/13] feat(init): setup steps need --yes outside a terminal An AI agent session, CI or a run without a terminal no longer runs a template's setup commands unasked, and never starts the app on its own. - New -y/--yes approves the setup commands. Without an AI agent, an explicit --install approves them too; in an agent session only --yes does. Otherwise init returns confirmation_required (exit 1) with details.flag --yes and a next_step that finishes without them: `{prog} init {template} --dir {dest} --no-install --no-start`. - Commands the template service lists are confirmed before anything is cloned, and the refusal names exactly what --yes runs: the listed commands, or make check-prereqs and make init if the repository has a Makefile (whose targets take precedence, as before). A Makefile with no listed commands is confirmed after the clone; next_step then resumes in it. - The start step runs only with --start; otherwise it is confirmation_required with next_step `... --no-start`. A person at a terminal keeps the existing prompts. --- .../src/deepctl_cmd_init/command.py | 224 +++++++++-- .../src/deepctl_cmd_init/messages.py | 67 +++- .../tests/unit/_init_harness.py | 2 + .../deepctl-cmd-init/tests/unit/test_init.py | 8 +- .../tests/unit/test_init_confirm.py | 365 ++++++++++++++++++ .../tests/unit/test_init_resume.py | 75 ++++ src/deepctl/_manifest.d/init.json | 26 +- 7 files changed, 738 insertions(+), 29 deletions(-) create mode 100644 packages/deepctl-cmd-init/tests/unit/test_init_confirm.py diff --git a/packages/deepctl-cmd-init/src/deepctl_cmd_init/command.py b/packages/deepctl-cmd-init/src/deepctl_cmd_init/command.py index 9a3a7ff6..e8343afb 100644 --- a/packages/deepctl-cmd-init/src/deepctl_cmd_init/command.py +++ b/packages/deepctl-cmd-init/src/deepctl_cmd_init/command.py @@ -3,6 +3,8 @@ from __future__ import annotations import os +import shlex +import sys from pathlib import Path from typing import Any @@ -14,14 +16,62 @@ DeepgramClient, ) from deepctl_core import messages as core_msg -from deepctl_core.output import _agentic, print_error, print_hint +from deepctl_core.output import _agentic, is_ai_agent, print_error, print_hint from rich.console import Console from rich.markup import escape from rich.table import Table from . import clones, lifecycle, templates_api from . import messages as msg -from .models import InitCodedError, InitResult, TemplateDetail, TemplateInfo +from .models import ( + InitCodedError, + InitResult, + LifecycleStep, + TemplateDetail, + TemplateInfo, +) + +_MAKE_STEPS = ["make check-prereqs", "make init"] + + +def _hard_signal() -> bool: + """An AI agent marked the environment, or ``DG_AGENT=1`` says one runs dg.""" + return is_ai_agent() or os.environ.get("DG_AGENT") == "1" + + +def _person_at_terminal() -> bool: + """A person can answer here: no agent or automation signal, and stdin + and stderr are terminals. Otherwise setup needs --yes and the start + step --start.""" + if _agentic or _hard_signal(): + return False + try: + return sys.stdin.isatty() and sys.stderr.isatty() + except (AttributeError, ValueError): + return False + + +def _commands(step: LifecycleStep | None) -> list[str]: + if step is None or step.command is None: + return [] + return step.command if isinstance(step.command, list) else [step.command] + + +def _rerun(template: str, dest: Path, *flags: str) -> str | None: + """``{prog} init {template} --dir {dest} {flags}``, or None when it + can't be given as one runnable command.""" + from deepctl_core.transport import prog + + program = prog() + if program is None: + return None + tokens = ["init", template, "--dir", str(dest), *flags] + if any(token.startswith("=") for token in tokens): + return None # zsh's EQUALS option would expand it + if os.name == "nt" and any(shlex.quote(token) != token for token in tokens): + return None + step = f"{program} {shlex.join(tokens)}" + return step if shlex.join(shlex.split(step)) == step else None def _coded( @@ -39,6 +89,21 @@ def _coded( ) +def _prog_text() -> str: + """``{prog}`` in a message: the running command, else ``dg``.""" + from deepctl_core.transport import prog + + return prog() or "dg" + + +def _still_same(dest: Path, identity: tuple[int, int] | None) -> bool: + """Whether ``dest`` is still the directory dg cloned into or resumed + in: checked again right before the API key is written there and before + any setup or start command runs, so a directory swapped in meanwhile + is refused.""" + return identity is not None and lifecycle.same_directory(dest, *identity) + + console = Console() status_console = Console(stderr=True) @@ -66,6 +131,7 @@ class InitCommand(BaseCommand): "dg init node-transcription --dir ./my-app", "dg init node-transcription --install", "dg init node-transcription --no-install", + "dg init node-transcription --yes", "dg init --list", "dg init --list --search flask", ] @@ -73,8 +139,10 @@ class InitCommand(BaseCommand): "Scaffold a Deepgram starter app from the templates gallery. " "Pass the template name/slug directly (e.g. 'node-transcription'). " "Use --list to enumerate available templates. " - "After cloning, runs make check-prereqs then make init automatically. " - "Use --no-install to skip setup. " + "Setup commands (make check-prereqs, make init, or the template's own) " + "run only with --yes: show them to the user and pass --yes only if the " + "user approves, or pass --no-install to skip them. The app is started " + "only with --start. A rerun into the same --dir resumes there. " "Requires git, node, npm, make, and curl to be installed." ) @@ -108,7 +176,10 @@ def get_arguments(self) -> list[dict[str, Any]]: }, { "names": ["--install"], - "help": "Run setup (make check-prereqs && make init) without prompting", + "help": ( + "Run the setup commands without asking (in an AI agent " + "session, use --yes)" + ), "is_flag": True, "is_option": True, }, @@ -130,6 +201,15 @@ def get_arguments(self) -> list[dict[str, Any]]: "is_flag": True, "is_option": True, }, + { + "names": ["--yes", "-y"], + "help": ( + "Approve and run the template's setup commands without " + "asking; the only way in an AI agent session" + ), + "is_flag": True, + "is_option": True, + }, ] def handle( @@ -148,6 +228,7 @@ def handle( no_install: bool = kwargs.get("no_install", False) do_start: bool = kwargs.get("start", False) no_start: bool = kwargs.get("no_start", False) + yes: bool = kwargs.get("yes", False) # ── List mode ────────────────────────────────────────────────────────── if list_mode: @@ -179,15 +260,8 @@ def handle( status="error", message=f"Template not found: {template_slug}" ) elif _agentic: - # In agent mode, require an explicit template name - return InitResult( - status="error", - message=( - "In agent mode, pass the template slug directly, e.g.: " - "dg init node-transcription\n" - "Use dg init --list to see available templates." - ), - ) + # No picker in agent mode: the template is named on the command. + return self._pick_needs_name() else: detail = self._interactive_pick(search) if not detail: @@ -239,13 +313,26 @@ def handle( return self._dir_error(msg.E_INIT_DIR_IN_USE, detail, dest) phases: list[str] = list(record.phases) if record else [] commit = record.commit if record else None + # The directory dg cloned into or resumes in, checked again right + # before anything is written or run in it. + identity = (record.dev, record.ino) if record else None + person = _person_at_terminal() + listed_steps = _commands(detail.config.install if detail.config else None) + + # An explicit --install approves setup, except under an AI agent, + # where only --yes does. Setup commands the template service lists + # are confirmed before anything is written. + approved = yes or (do_install and not _hard_signal()) + if record is None and listed_steps and not (person or approved or no_install): + return self._confirm_steps(detail, dest, listed_steps, cloned=False) + if record is None: # ── Confirm ──────────────────────────────────────────────────────── self._print_template_summary(detail) console.print(f"[dim]Destination:[/dim] {dest}") console.print() - if not _agentic and not self.confirm("Clone this template?", default=True): + if person and not self.confirm("Clone this template?", default=True): return InitResult(status="cancelled", message="Clone cancelled") # ── Clone ────────────────────────────────────────────────────────── @@ -256,8 +343,9 @@ def handle( except RuntimeError as e: return InitResult(status="error", message=str(e)) phases = ["clone"] + identity = lifecycle.directory_identity(dest) commit = lifecycle.head_commit(dest) - self._save_progress(dest, detail.name, github_url, commit, phases) + self._save_progress(dest, detail.name, github_url, commit, phases, identity) console.print(f"[green]✓[/green] Cloned to [cyan]{dest}[/cyan]") console.print( @@ -268,6 +356,8 @@ def handle( console.print(f"[dim]Resuming in[/dim] [cyan]{dest}[/cyan]") # ── Inject API key ───────────────────────────────────────────────────── + if not _still_same(dest, identity): + return self._dir_error(msg.E_INIT_DIR_CHANGED, detail, dest) try: api_key = auth_manager.get_api_key() if api_key: @@ -282,8 +372,13 @@ def handle( has_makefile = (dest / "Makefile").exists() if not installed and not no_install: - if _agentic or do_install: + steps = _MAKE_STEPS if has_makefile else listed_steps + if not steps: + run_setup = False + elif approved or (person and do_install): run_setup = True + elif not person: + return self._confirm_steps(detail, dest, steps, cloned=True) elif has_makefile: console.print() run_setup = self.confirm( @@ -293,6 +388,8 @@ def handle( run_setup = False if run_setup: + if not _still_same(dest, identity): + return self._dir_error(msg.E_INIT_DIR_CHANGED, detail, dest) console.print() if has_makefile: installed = lifecycle.run_make_init(dest) @@ -303,18 +400,25 @@ def handle( ) if installed: phases.append("setup") - self._save_progress(dest, detail.name, github_url, commit, phases) + self._save_progress( + dest, detail.name, github_url, commit, phases, identity + ) # ── Start step (optional) ────────────────────────────────────────────── + # Never started unasked: --start, or a person's answer. started = False if installed and detail.config and detail.config.start and not no_start: - if _agentic or do_start: + if do_start: run_start = True - else: + elif person: console.print() run_start = self.confirm("Run start step?", default=False) + else: + return self._confirm_start(detail, dest) if run_start: + if not _still_same(dest, identity): + return self._dir_error(msg.E_INIT_DIR_CHANGED, detail, dest) console.print() started = lifecycle.run_lifecycle_step(detail.config.start, dest) @@ -340,7 +444,7 @@ def handle( started=started, ) - # ── The rerun contract ──────────────────────────────────────────────────── + # ── Confirmations and the rerun contract ────────────────────────────────── def _save_progress( self, @@ -349,10 +453,10 @@ def _save_progress( url: str, commit: str | None, phases: list[str], + identity: tuple[int, int] | None, ) -> None: """Record the clone and what is done in dg's state, so a rerun resumes. Without a commit there is nothing a rerun could check.""" - identity = lifecycle.directory_identity(dest) if commit is None or identity is None: return record = clones.CloneRecord( @@ -361,9 +465,68 @@ def _save_progress( try: clones.save(record) except OSError as exc: - console.print( - f"[yellow]Could not record progress in {dest}:[/yellow] {exc}" + text = msg.W_INIT_PROGRESS_UNSAVED.format( + exc=exc, prog=_prog_text(), dest=core_msg.display_path(dest) + ) + console.print(f"[yellow]{escape(text)}[/yellow]") + + def _confirm_steps( + self, detail: TemplateDetail, dest: Path, steps: list[str], *, cloned: bool + ) -> InitCodedError: + """Setup commands no one here can approve: confirmation_required.""" + next_step = _rerun(detail.name, dest, "--no-install", "--no-start") + shown = core_msg.display_path(dest) + if not _hard_signal(): + hint = ( + msg.H_CONFIRM_INIT_STEPS + if cloned + else msg.H_CONFIRM_INIT_STEPS_BEFORE_CLONE + ) + elif cloned: + hint = ( + msg.A_CONFIRM_INIT_STEPS if next_step else msg.A_CONFIRM_INIT_STEPS_NULL + ) + else: + hint = ( + msg.A_CONFIRM_INIT_STEPS_BEFORE_CLONE + if next_step + else msg.A_CONFIRM_INIT_STEPS_BEFORE_CLONE_NULL + ) + # Before the clone, the repository's Makefile is unknown, and if it + # has one its targets run instead of the listed commands; say so. + template = core_msg.display(detail.name) + listed = "; ".join(core_msg.display(step) for step in steps) + if cloned: + error = msg.E_CONFIRM_INIT_STEPS.format(template=template, steps=listed) + else: + error = msg.E_CONFIRM_INIT_STEPS_BEFORE_CLONE.format( + template=template, steps=listed + ) + return _coded( + error, + msg.CODE_CONFIRMATION_REQUIRED, + hint.format(dest=shown, prog=_prog_text()), + next_step, + {"flag": msg.YES_FLAG}, + ) + + def _confirm_start(self, detail: TemplateDetail, dest: Path) -> InitCodedError: + """A start step no one here asked for: confirmation_required.""" + next_step = _rerun(detail.name, dest, "--no-start") + if not _hard_signal(): + hint = msg.H_CONFIRM_INIT_START + else: + hint = ( + msg.A_CONFIRM_INIT_START if next_step else msg.A_CONFIRM_INIT_START_NULL ) + error = msg.E_CONFIRM_INIT_START.format(template=core_msg.display(detail.name)) + return _coded( + error, + msg.CODE_CONFIRMATION_REQUIRED, + hint, + next_step, + {"flag": "--start"}, + ) def _dir_error( self, @@ -380,6 +543,7 @@ def _dir_error( dest=core_msg.display_path(dest), template=core_msg.display(detail.name), other=core_msg.display(other or ""), + prog=_prog_text(), ) return _coded( error, @@ -388,6 +552,18 @@ def _dir_error( f"{program} init --help" if program else None, ) + def _pick_needs_name(self) -> InitCodedError: + """No template named where the picker can't be shown.""" + from deepctl_core.transport import prog + + program = prog() + return _coded( + msg.E_CONFIRM_INIT_PICK.format(prog=_prog_text()), + msg.CODE_INVALID_ARGUMENT, + msg.H_CONFIRM_INIT_PICK.format(prog=_prog_text()), + f"{program} init --list" if program else None, + ) + def output_result(self, result: Any, config: Config) -> None: """Only emit structured output when an explicit --output format is set.""" from deepctl_core.output import get_output_format diff --git a/packages/deepctl-cmd-init/src/deepctl_cmd_init/messages.py b/packages/deepctl-cmd-init/src/deepctl_cmd_init/messages.py index 4928bbf7..b69d01a5 100644 --- a/packages/deepctl-cmd-init/src/deepctl_cmd_init/messages.py +++ b/packages/deepctl-cmd-init/src/deepctl_cmd_init/messages.py @@ -1,12 +1,77 @@ -"""Strings for ``dg init``'s errors.""" +"""Strings for ``dg init``'s coded errors. +The confirmation strings are the catalogue's (``E-CONFIRM-INIT-*``). H is +for a person, A for an AI agent; ``*_NULL`` when ``next_step`` is null. +The ``*_BEFORE_CLONE`` hints are their variants for a confirmation raised +before anything was cloned. +""" + +CODE_CONFIRMATION_REQUIRED = "confirmation_required" CODE_INVALID_ARGUMENT = "invalid_argument" +YES_FLAG = "--yes" + +E_CONFIRM_INIT_STEPS = 'Template "{template}" wants to run setup commands: {steps}' +H_CONFIRM_INIT_STEPS = ( + "The template is cloned in {dest}. Review the commands, then run again with " + "--yes to run them, or --no-install to skip them." +) +A_CONFIRM_INIT_STEPS = ( + "The template is cloned in {dest}. next_step finishes without running the " + "setup commands. Show the user the commands and run them (with --yes) only " + "if the user approves." +) +A_CONFIRM_INIT_STEPS_NULL = ( + "The template is cloned in {dest}. Show the user the setup commands and ask " + "them to finish setup in their terminal." +) +# Before the clone, the setup that --yes approves is the listed commands, +# or the Makefile targets if the repository turns out to have a Makefile. +E_CONFIRM_INIT_STEPS_BEFORE_CLONE = ( + 'Template "{template}" has setup commands. --yes runs {steps}, or make ' + "check-prereqs and make init if the repository has a Makefile." +) +H_CONFIRM_INIT_STEPS_BEFORE_CLONE = ( + "Nothing was cloned. Review the commands, then run again with --yes to run " + "them, or --no-install to skip them." +) +A_CONFIRM_INIT_STEPS_BEFORE_CLONE = ( + "Nothing was cloned. next_step clones the template without running the " + "setup commands. Show the user the commands and run them (with --yes) only " + "if the user approves." +) +A_CONFIRM_INIT_STEPS_BEFORE_CLONE_NULL = ( + "Nothing was cloned. Show the user the setup commands and ask them to run " + "{prog} init in their terminal." +) + +E_CONFIRM_INIT_START = 'Template "{template}" can start the app after setup.' +H_CONFIRM_INIT_START = "Run again with --start to start it, or --no-start to skip." +A_CONFIRM_INIT_START = ( + "next_step skips starting the app. Start it only if the user asks." +) +A_CONFIRM_INIT_START_NULL = ( + "Ask the user whether to start the app; rerun with --no-start to skip it." +) + +# No template named, and no picker to choose one (an AI agent session). +E_CONFIRM_INIT_PICK = "{prog} init needs a template name when it can't show the picker." +H_CONFIRM_INIT_PICK = ( + "List templates with {prog} init --list, then run {prog} init