mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(telemetry): aggregate requires local telemetry to be on
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.
This commit is contained in:
parent
0d0549d834
commit
24c86e160b
4 changed files with 31 additions and 5 deletions
|
|
@ -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__ = [
|
||||
|
|
|
|||
|
|
@ -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)")
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue