From 42982075af1fd83088df928463aa1d7666fec585 Mon Sep 17 00:00:00 2001 From: Pouri Date: Fri, 4 Sep 2026 18:00:49 +0330 Subject: [PATCH 1/2] daemon status: answer "no" out loud when the daemon is down MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `DaemonStatus` inherited `omit_defaults=True`, which drops every field equal to its default — and for a stopped daemon that is every field. The result encoded to `{}`: human mode printed nothing at all, `--json` printed an empty object, and `status["running"]` raised a KeyError for the one caller the flag exists for. The operation that exists to tell you the daemon is down was the operation that could not say so. `omit_defaults=False` on the struct, as `WhoAmI` already carries it for the same reason. The offline result also fills in the socket it would have asked and the Telethon layer, so "not running" says where it looked, and `version` is `None` rather than `""` because nobody read a version off a process that is not there. `HealthSummary` behind `tlgr status` had the same shape and the same fix: it is read when nothing works, which is exactly when every answer on it is false. The `json-only` tag was the other suspect and is not implicated — `daemon status` never carried it — but nothing said so. `agent.schema` is the only op that may, and now a test holds that line. --- tests/test_ops_daemon.py | 52 +++++++++++++++++++++++++++++++++ tests/test_registry_contract.py | 10 +++++++ tlgr/models/daemon.py | 26 +++++++++++++---- tlgr/ops/daemon.py | 15 ++++++++-- 4 files changed, 96 insertions(+), 7 deletions(-) diff --git a/tests/test_ops_daemon.py b/tests/test_ops_daemon.py index 9dae032..7f34452 100644 --- a/tests/test_ops_daemon.py +++ b/tests/test_ops_daemon.py @@ -54,6 +54,28 @@ def local(op_id: str, request: dict[str, Any] | None = None, **state: Any) -> An return asyncio.run(spec.impl(context, payload)) +def _human(*args: str) -> dict[str, str]: + """Run a command through the real CLI and read back its key/value table. + + Through click rather than the renderer directly: the bug being pinned was + a model that serialised to `{}`, which no assertion on the *object* can + see and which printed an empty screen. + """ + from click.testing import CliRunner + + from tlgr.cli import cli + + outcome = CliRunner().invoke(cli, list(args)) + assert outcome.exit_code == 0, outcome.output + assert not outcome.output.lstrip().startswith("{"), "human mode printed an envelope" + rows: dict[str, str] = {} + for line in outcome.output.splitlines(): + key, _, value = line.partition(" ") + if key: + rows[key.strip()] = value.strip() + return rows + + async def alocal(in_thread, op_id: str, request: dict[str, Any] | None = None, **state: Any): """`local`, off the event loop. @@ -86,6 +108,36 @@ def test_a_stopped_daemon_is_reported_not_guessed(self, tlgr_home, stub_account) assert status.ready is False assert status.healthy is False + def test_a_stopped_daemon_serialises_the_false_answers(self, tlgr_home, stub_account): + """`omit_defaults` dropped every false and every zero, so the whole + result encoded to `{}` — the one shape this operation must never + return, because "no" is the answer it exists to give.""" + import msgspec + + payload = msgspec.to_builtins(local("daemon.status")) + assert payload["running"] is False + assert payload["ready"] is False + assert payload["healthy"] is False + assert payload["pid"] is None + assert payload["version"] is None + assert payload["socket"].endswith("daemon.sock") + + def test_a_stopped_daemon_prints_a_table_not_an_envelope(self, tlgr_home, stub_account): + """Human mode is the key/value table every other op renders; v1 + printed `RUNNING false` and the empty result printed nothing.""" + rendered = _human("daemon", "status") + assert rendered["running"] == "no" + assert rendered["ready"] == "no" + assert rendered["healthy"] == "no" + assert rendered["pid"] == "-" + + def test_the_status_shortcut_answers_the_same_way(self, tlgr_home, stub_account): + """`tlgr status` is a different operation asking the same question.""" + rendered = _human("status") + assert rendered["daemon_running"] == "no" + assert rendered["connected"] == "no" + assert "the daemon is not running" in rendered["problems"] + def test_check_turns_an_unhealthy_daemon_into_an_exit_code(self, tlgr_home, stub_account): from tlgr.core.errors import DaemonNotRunningError diff --git a/tests/test_registry_contract.py b/tests/test_registry_contract.py index 1cb168f..c354c81 100644 --- a/tests/test_registry_contract.py +++ b/tests/test_registry_contract.py @@ -48,6 +48,16 @@ def test_lint_is_clean(): assert lint() == [] +def test_only_the_schema_document_is_json_only(): + """`json-only` prints an envelope where a person asked for a table. + + Right for exactly one operation — a JSON Schema document has no table + shape — and wrong for every other, most of all for the ones an operator + reads when something is broken. + """ + assert {spec.id for spec in SPECS if "json-only" in spec.tags} == {"agent.schema"} + + class TestOperationContract: @pytest.mark.parametrize("spec", SPECS, ids=IDS) def test_example_validates(self, spec): diff --git a/tlgr/models/daemon.py b/tlgr/models/daemon.py index e2438ec..8eeba03 100644 --- a/tlgr/models/daemon.py +++ b/tlgr/models/daemon.py @@ -79,15 +79,25 @@ class EventBusStatus(Model): dropped: int = 0 -class DaemonStatus(Model): - """`tlgr daemon status`. `running` and `healthy` are different questions.""" +class DaemonStatus(Model, omit_defaults=False): + """`tlgr daemon status`. `running` and `healthy` are different questions. + + `omit_defaults=False` for the whole struct: the answer this operation + exists to give is "no", and `omit_defaults` drops every false and every + zero — so a stopped daemon serialised to `{}`, which printed nothing at + all in human mode and made `status["running"]` a KeyError for the exact + caller the flag is for. v1 printed `RUNNING false`; a field here is + always present, whichever way the answer went. + """ running: bool = False ready: bool = False healthy: bool = False pid: int | None = None uptime_seconds: int = 0 - version: str = "" + #: `None` rather than `""` when the daemon is not answering: nobody read + #: a version off a process that is not there. + version: str | None = None protocol: int = 0 layer: int = 0 socket: str = "" @@ -102,8 +112,14 @@ class DaemonStatus(Model): disconnected: list[str] = [] -class HealthSummary(Model): - """`tlgr status`: one screen, the states where everything else fails.""" +class HealthSummary(Model, omit_defaults=False): + """`tlgr status`: one screen, the states where everything else fails. + + `omit_defaults=False` for the same reason as `DaemonStatus`: this screen + is read when nothing works, which is precisely when every answer on it is + false — and a screen that prints only the fields that happened to be true + is at its least useful exactly then. + """ account: str = "" user_id: int | None = None diff --git a/tlgr/ops/daemon.py b/tlgr/ops/daemon.py index c2cb862..59436a7 100644 --- a/tlgr/ops/daemon.py +++ b/tlgr/ops/daemon.py @@ -730,13 +730,24 @@ async def daemon_status(ctx: OpContext, req: DaemonStatusReq) -> DaemonStatus: are the questions people were actually asking, and v1 could not tell them apart: an account whose connection had died was still counted (COR-37). """ + from tlgr.core.paths import TlgrPaths from tlgr.core.process import read_pid base = _base() pid = read_pid(base) status = _probe() if status is None: - result = DaemonStatus(running=pid is not None, ready=False, healthy=False, pid=pid) + # The socket it *would* have asked, so "not running" says where it + # looked rather than only that it found nothing. + result = DaemonStatus( + running=pid is not None, + ready=False, + healthy=False, + pid=pid, + layer=_telethon_layer(), + socket=str(TlgrPaths(base).socket), + socket_owner=os.getuid(), + ) if req.check: raise DaemonNotRunningError("the daemon is not answering on its socket") return result @@ -752,7 +763,7 @@ async def daemon_status(ctx: OpContext, req: DaemonStatusReq) -> DaemonStatus: healthy=bool(info.get("ready")) and not unhealthy, pid=info.get("pid") or pid, uptime_seconds=int(info.get("uptime_s") or 0), - version=str(info.get("version", "")), + version=str(info["version"]) if info.get("version") else None, protocol=int(info.get("protocol") or 0), layer=_telethon_layer(), socket=str(info.get("socket", "")), From ed05a8b4423593fc45d598342d3575570e1521d4 Mon Sep 17 00:00:00 2001 From: Pouri Date: Fri, 4 Sep 2026 18:00:57 +0330 Subject: [PATCH 2/2] docs(upgrading): the pipx editable install, where git is the upgrade MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit §2 assumed `pipx upgrade tlgr`, which does nothing for an install made with `pipx install -e `: the venv holds a link to that working tree, so the running version is whatever the checkout is on and the upgrade is a `git` command. That is the part worth writing down — moving the branch swaps the code under a running daemon with no install step in between, so §1 and the watchdog it names are load-bearing here rather than housekeeping. The reinstall is conditional and 2.0.0 is the condition: it adds msgspec and pins telethon~=1.44.0, and a `git checkout` puts neither into the pipx venv. `pipx runpip tlgr install -e '[fast]'` does, with a note that it replaces the extra set rather than adding to it. --- docs/UPGRADING.md | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md index 61d2921..cffd168 100644 --- a/docs/UPGRADING.md +++ b/docs/UPGRADING.md @@ -84,6 +84,51 @@ tlgr --version # expect 2.0.0 `pip install -U tlgr` works the same way if that is how it was installed. +### A pipx editable install — the checkout *is* the install + +If tlgr was installed with `pipx install -e `, there is nothing to +download: the pipx venv holds a link to that working tree, so the version +that runs is whatever the checkout is currently on. Upgrading is a `git` +command, and that is exactly why it needs care — **moving the branch swaps +the running code under the daemon**, with no install step in between to warn +you. Section 1 is not optional here, and neither is stopping your own +watchdog: a supervisor that restarts the v1 daemon while the checkout is +mid-upgrade starts it on half-new code. + +With the daemon stopped, move the checkout: + +```bash +git -C fetch +git -C checkout main # or, on the deployed branch: +git -C merge --ff-only origin/main +tlgr --version # expect 2.0.0 +``` + +`--ff-only` on purpose: a merge commit in a deployment checkout is a local +edit nobody will remember making, and it fails loudly when the branch has +drifted instead of quietly resolving it. + +Reinstall **only when the dependencies changed** — the code itself is already +live through the link. 2.0.0 is such a release: it adds `msgspec` and pins +`telethon~=1.44.0`, and neither of those reaches the pipx venv from a `git +checkout`. Install them into it: + +```bash +pipx runpip tlgr install -e '[fast]' +``` + +`pipx runpip` runs that venv's own pip, which is the only pip that can see +it; plain `pip install` from your shell installs into whatever else is on +`PATH`. Keep the extras you originally installed with — `[fast]` above, plus +`proxy`, `media` or `qr` if you use them — because the reinstall replaces the +extra set rather than adding to it. Then check the imports resolve before +starting anything: + +```bash +tlgr --version # expect 2.0.0 +tlgr agent whoami --json # imports msgspec and Telethon; fails loudly if either is missing +``` + --- ## 3. Clear the production marker