From 24c86e160be2e0b6b0b73e2f020f31b4a89b40a6 Mon Sep 17 00:00:00 2001 From: emozilla Date: Fri, 26 Jun 2026 15:10:38 -0400 Subject: [PATCH] fix(telemetry): aggregate requires local telemetry to be on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Aggregate metrics are derived from the local tel_* tables — they're a coarsened view of local data, not an independent capture path. With telemetry.local=false nothing is written, so an aggregate opt-in had nothing to aggregate, yet may_upload_aggregate() returned True and `status` showed "Aggregate metrics: on". The config could claim a state it couldn't fulfill. Gate aggregate on local being enabled: - may_upload_aggregate() now requires local_enabled AND allow_aggregate AND consent_state == aggregate. - `telemetry status` computes aggregate_enabled the same way and, when consent is aggregate but local is off, prints "inert: local telemetry is off — nothing to aggregate" instead of the opt-in hint. Happy path is unchanged (local on + consent aggregate -> on). Adds policy and CLI tests for the inert combo. --- agent/telemetry/policy.py | 8 +++++--- hermes_cli/main.py | 7 +++++-- tests/telemetry/test_cli_telemetry.py | 14 ++++++++++++++ tests/telemetry/test_policy_consent.py | 7 +++++++ 4 files changed, 31 insertions(+), 5 deletions(-) diff --git a/agent/telemetry/policy.py b/agent/telemetry/policy.py index f98797c2b4f..77e794cf8a2 100644 --- a/agent/telemetry/policy.py +++ b/agent/telemetry/policy.py @@ -51,13 +51,15 @@ def ensure_install_id(config: Dict[str, Any]) -> str: def may_upload_aggregate(config: Dict[str, Any]) -> bool: """Whether aggregate metrics may upload — the gate a future uploader consults. - True only when the admin hard gate allows it AND the user has opted in via - ``telemetry.consent_state``. + Aggregate metrics are derived from the local telemetry tables, so they require + local telemetry to be on. True only when local telemetry is enabled, the admin + hard gate allows it, and the user has opted in via ``telemetry.consent_state``. """ tel = _telemetry_cfg(config) + local_enabled = bool(tel.get("local", True)) allow_aggregate = bool(tel.get("allow_aggregate", True)) state = tel.get("consent_state", CONSENT_UNKNOWN) - return allow_aggregate and state == CONSENT_AGGREGATE + return local_enabled and allow_aggregate and state == CONSENT_AGGREGATE __all__ = [ diff --git a/hermes_cli/main.py b/hermes_cli/main.py index f8bcea1a57d..173eb2a1ede 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -14328,7 +14328,8 @@ def cmd_telemetry(args): consent_state = tel.get("consent_state", policy.CONSENT_UNKNOWN) if consent_state not in policy.VALID_CONSENT_STATES: consent_state = policy.CONSENT_UNKNOWN - aggregate_enabled = allow_aggregate and consent_state == policy.CONSENT_AGGREGATE + aggregate_enabled = (local_enabled and allow_aggregate + and consent_state == policy.CONSENT_AGGREGATE) install_id = policy.ensure_install_id(config) def _persist_install_id(): @@ -14343,7 +14344,9 @@ def cmd_telemetry(args): f"(telemetry.local)") print(f" Aggregate metrics: {'on' if aggregate_enabled else 'off'} " f"(opt-in; consent_state={consent_state})") - if consent_state != policy.CONSENT_AGGREGATE and allow_aggregate: + if consent_state == policy.CONSENT_AGGREGATE and not local_enabled: + print(" ⚠ inert: local telemetry is off — nothing to aggregate") + elif consent_state != policy.CONSENT_AGGREGATE and allow_aggregate: print(" opt in: hermes config set telemetry.consent_state aggregate") if not allow_aggregate: print(" ⚠ allow_aggregate is false (egress hard-disabled)") diff --git a/tests/telemetry/test_cli_telemetry.py b/tests/telemetry/test_cli_telemetry.py index d38424df47c..4aaf4cab744 100644 --- a/tests/telemetry/test_cli_telemetry.py +++ b/tests/telemetry/test_cli_telemetry.py @@ -92,3 +92,17 @@ def test_allow_aggregate_false_keeps_metrics_off_in_status(home, capsys): out = capsys.readouterr().out assert "Aggregate metrics: off" in out assert "allow_aggregate is false" in out + + +def test_local_off_with_consent_shows_inert_in_status(home, capsys): + # local off + opted in: aggregate is off and the status explains why. + from hermes_cli.config import load_config, save_config + cfg = load_config() + tel = cfg.setdefault("telemetry", {}) + tel["local"] = False + tel["consent_state"] = "aggregate" + save_config(cfg) + _run("status") + out = capsys.readouterr().out + assert "Aggregate metrics: off" in out + assert "inert: local telemetry is off" in out diff --git a/tests/telemetry/test_policy_consent.py b/tests/telemetry/test_policy_consent.py index 7bc0d68e2fe..7205c33b275 100644 --- a/tests/telemetry/test_policy_consent.py +++ b/tests/telemetry/test_policy_consent.py @@ -38,6 +38,13 @@ def test_allow_aggregate_false_overrides_opt_in(): assert policy.may_upload_aggregate(cfg) is False # the hard gate wins +def test_local_off_makes_aggregate_inert(): + # Aggregate metrics derive from the local tables; with local off there is + # nothing to aggregate, so opting in cannot upload. + cfg = _cfg(local=False, consent_state="aggregate", allow_aggregate=True) + assert policy.may_upload_aggregate(cfg) is False + + def test_install_id_minted_when_empty_and_stable_when_set(): cfg = _cfg(install_id="") minted = policy.ensure_install_id(cfg)