diff --git a/docs/observability/relay-shared-metrics.md b/docs/observability/relay-shared-metrics.md index 5b5ce0f8d4..98891103ec 100644 --- a/docs/observability/relay-shared-metrics.md +++ b/docs/observability/relay-shared-metrics.md @@ -33,8 +33,12 @@ than downloading a different implementation. When Relay managed execution is active, the provider request and response pass through that native module in the Hermes process so configured interceptors can operate on the real call. This is separate from the shared-metrics data -contract. Shared-metrics mode installs no network exporter and its subscriber -accepts only the versioned, allowlisted projection described below. Enabling a +contract. Shared-metrics mode installs no rich-observability network exporter, +and its subscriber +accepts only the versioned, allowlisted projection described below. The +opt-in package sender described in Appendix A is the only outbound path, it +transmits nothing unless the user enables both `enabled` and `send`, and it +sends whole packages rather than live spans. Enabling a separately configured rich-observability or dynamic plugin can create a different data path and requires its own policy review. @@ -226,17 +230,17 @@ packages from that profile and can therefore link those local packages. Deleting `$HERMES_HOME/telemetry/shared_metrics` resets the identifier together with all aggregates and package files. -This slice has no remote-delivery path. A future remote exporter must not reuse +Remote delivery is opt-in and off by default. A remote exporter must not reuse the persistent local identifier by default. It requires a separate product and privacy decision covering consent, identity scope, rotation or keyed pseudonymization, reset behavior, retention, and deletion. -> That exporter is now being built as Phase 2 of the Hermes telemetry project. -> The decisions this paragraph asks for are recorded in -> [Appendix A](#appendix-a-remote-exporter-decisions-phase-2). Until Phase 2 -> ships, the statement above still describes shipped behaviour: nothing is -> transmitted, and transmission stays opt-in behind a config key that is off by -> default. +> Those decisions are recorded in +> [Appendix A](#appendix-a-remote-exporter-decisions-phase-2), and the exporter +> implementing them has shipped. Collection alone still transmits nothing: the +> sender runs only when `telemetry.shared_metrics.send` is also true, and it +> transmits a rotating HMAC of the install identity rather than the identifier +> itself. The install identity is scoped to one `HERMES_HOME`. To reset it, stop Hermes processes and remove `$HERMES_HOME/telemetry/shared_metrics`. This deliberately @@ -267,10 +271,13 @@ ID, tool-result, and skill-name canaries are absent from the packages. ## Appendix A: Remote Exporter Decisions (Phase 2) -Status: **decided, not yet built.** This appendix answers the product and +Status: **implemented.** This appendix answers the product and privacy questions that "Current Slices" defers to a future remote exporter. It records what was decided and why, so the reasoning survives the implementation. +Sending is off by default and requires both `telemetry.shared_metrics.enabled` +and `telemetry.shared_metrics.send`. + The exporter sends the package files already written under `$HERMES_HOME/telemetry/shared_metrics/outbox/` to the Hermes telemetry ingest service. That service validates only the envelope (`schema_version` plus a UUID diff --git a/hermes_cli/observability/relay_shared_metrics.py b/hermes_cli/observability/relay_shared_metrics.py index cb4eb44267..c3097114d9 100644 --- a/hermes_cli/observability/relay_shared_metrics.py +++ b/hermes_cli/observability/relay_shared_metrics.py @@ -671,6 +671,12 @@ class _Runtime: self._safe(self.relay.subscribers.deregister, self._subscriber_name) self.host.release_managed_execution(self._subscriber_name) self._registered = False + # The final export above may have started a send. Give it the same + # bounded chance to finish that deactivate() gets — without this a + # short-lived CLI process exits immediately and kills the daemon + # thread mid-request, which is the common case for the one cadence + # this feature has. + self._join_send_thread() try: atexit.unregister(self.shutdown) except Exception: diff --git a/hermes_cli/observability/shared_metrics_send_config.py b/hermes_cli/observability/shared_metrics_send_config.py index 8011c595ab..cb14027593 100644 --- a/hermes_cli/observability/shared_metrics_send_config.py +++ b/hermes_cli/observability/shared_metrics_send_config.py @@ -9,20 +9,21 @@ identity, rotation, retention, and deletion decisions behind this module. from __future__ import annotations import logging -import os from dataclasses import dataclass from urllib.parse import urlparse logger = logging.getLogger(__name__) -#: Production ingest endpoint. Overridable by config or environment so the -#: live E2E can target staging without mutating a user's config. +#: Production ingest endpoint. Overridable through config only. +#: +#: Deliberately NOT overridable by an environment variable: AGENTS.md reserves +#: HERMES_* env vars for secrets, and a behavioural override here would be a +#: consent hazard — a user who agreed to send metrics to Nous could have them +#: silently redirected to any host by an inherited variable, with nothing +#: visible in their config to show it. Tests and the staging E2E write this +#: key into a throwaway profile instead. DEFAULT_ENDPOINT = "https://telemetry.nousresearch.com/v1/telemetry" -#: Environment override, highest precedence. Intended for tests and staging -#: validation, not as the documented user-facing setting (which is config). -ENDPOINT_ENV_VAR = "HERMES_TELEMETRY_ENDPOINT" - _LOCAL_HOSTS = frozenset({"localhost", "127.0.0.1", "::1", "[::1]"}) # Module-level latch: the enabled/send mismatch is a static misconfiguration, @@ -62,8 +63,7 @@ def _endpoint_is_safe(endpoint: str) -> bool: def resolve_send_config(config: dict | None) -> SendConfig: """Resolve transmission settings from config plus the environment. - Endpoint precedence: ``HERMES_TELEMETRY_ENDPOINT`` > config > production - default. + Endpoint precedence: config > production default. ``send`` is returned as False whenever transmission cannot legitimately happen, so callers never have to re-check the combination. @@ -92,7 +92,7 @@ def resolve_send_config(config: dict | None) -> SendConfig: ) return SendConfig(enabled=False, send=False, endpoint=DEFAULT_ENDPOINT) - endpoint = os.environ.get(ENDPOINT_ENV_VAR) or shared.get("endpoint") + endpoint = shared.get("endpoint") if not isinstance(endpoint, str) or not endpoint.strip(): endpoint = DEFAULT_ENDPOINT endpoint = endpoint.strip() diff --git a/hermes_cli/observability/shared_metrics_sender.py b/hermes_cli/observability/shared_metrics_sender.py index 9086c3b359..a55cbf9f7d 100644 --- a/hermes_cli/observability/shared_metrics_sender.py +++ b/hermes_cli/observability/shared_metrics_sender.py @@ -31,7 +31,7 @@ import time import urllib.error import urllib.request from dataclasses import dataclass -from datetime import datetime, timezone +from datetime import datetime, timedelta, timezone from hermes_cli.sqlite_util import write_txn @@ -58,6 +58,13 @@ GZIP_THRESHOLD_BYTES = 4096 #: Packages per pass. Bounds work on an interactive hook even after an outage. MAX_PACKAGES_PER_PASS = 20 +#: How long a claimed row is held by the claiming pass. A claim writes a +#: LEASE INTO THE FUTURE: another process selecting on `next_attempt_at <= now` +#: therefore skips it. Long enough to cover three attempts plus backoff +#: (1+5+25s of jitter plus three 30s timeouts), short enough that a killed +#: process's rows become eligible again quickly. +_CLAIM_LEASE_SECONDS = 180 + #: Floor applied after a pass fails to deliver, so a hard-down service is not #: retried on every task completion. _FAILURE_BACKOFF_SECONDS = 15 * 60 @@ -100,7 +107,12 @@ def _post(endpoint: str, payload: bytes, *, timeout: int) -> _Response: } body = payload if len(payload) > GZIP_THRESHOLD_BYTES: - body = gzip.compress(payload) + # mtime=0: gzip embeds a timestamp by default, which would make two + # sends of one package differ on the wire. The service decompresses + # before storing so it would not change what lands in S3, but a + # deterministic body keeps "a resend is byte-identical" true at the + # transport layer too, and makes the property testable. + body = gzip.compress(payload, mtime=0) headers["Content-Encoding"] = "gzip" request = urllib.request.Request( @@ -185,6 +197,7 @@ class SharedMetricsSender: """ period = opt_in_period(connection, now=now) stamp = _isoformat(now) + lease_until = now + timedelta(seconds=_CLAIM_LEASE_SECONDS) rows = connection.execute( """ SELECT package_id, payload_json, sent_install_id @@ -243,9 +256,14 @@ class SharedMetricsSender: next_attempt_at = ? WHERE package_id = ? """, - # Hold the row for the duration of this pass; success or a - # real backoff overwrite this immediately below. - (_isoformat(now), package_id), + # Lease the row INTO THE FUTURE. Selection above requires + # next_attempt_at <= now, so for the length of the lease no + # other process can claim this package. Writing `now` here (as + # an earlier revision did) claimed nothing: a concurrent pass + # matched the same predicate immediately and sent a duplicate. + # Success or a real backoff overwrites this below; if this + # process dies mid-pass, the lease simply expires. + (_isoformat(lease_until), package_id), ) claimed.append( { @@ -268,12 +286,24 @@ class SharedMetricsSender: payload = substitute_install_id(json.loads(payload_json), derived) return json.dumps(payload, indent=2, sort_keys=True).encode("utf-8") - def _mark(self, package_id: str, **columns) -> None: + def _mark(self, package_id: str, *, only_if_pending: bool = True, **columns) -> None: + """Write send state for one package. + + Guarded on send_state so a pass whose lease lapsed cannot resurrect a + row another process has already finished: without this, a slow sender + could overwrite 'sent' back to 'pending' and cause a re-send. + """ assignments = ", ".join(f"{name} = ?" for name in columns) + predicate = ( + " AND (send_state IS NULL OR send_state = 'pending')" + if only_if_pending + else "" + ) with self._store._connection() as connection: with write_txn(connection): connection.execute( - f"UPDATE package_outbox SET {assignments} WHERE package_id = ?", + f"UPDATE package_outbox SET {assignments} " + f"WHERE package_id = ?{predicate}", (*columns.values(), package_id), ) @@ -315,17 +345,23 @@ class SharedMetricsSender: ) return "sent" - if response.status == 400: - # Permanent per the contract. Keep the file (it is the user's - # history) but never try again. + if response.status == 400 or ( + 400 <= response.status < 500 and response.status != 429 + ): + # The contract only names 400, but every other 4xx is equally + # permanent for an unauthenticated fire-and-forget sender: a + # wrong path (404), an edge rejection (403), or an oversized + # body (413) will not fix itself by being retried every 15 + # minutes until local retention prunes the package. logger.warning( - "Telemetry package %s rejected as malformed; not retrying", + "Telemetry package %s rejected with HTTP %s; not retrying", package_id, + response.status, ) self._mark( package_id, send_state="rejected", - last_error=response.body[:500], + last_error=f"HTTP {response.status}: {response.body[:400]}", ) return "rejected" diff --git a/hermes_cli/setup.py b/hermes_cli/setup.py index d6497fbc05..d7971e14a5 100644 --- a/hermes_cli/setup.py +++ b/hermes_cli/setup.py @@ -2468,11 +2468,34 @@ def setup_telemetry(config: dict): default=shared_metrics.get("send") is True, ) if shared_metrics["send"]: + _record_send_opt_in_day() print_success("Sending shared metrics enabled.") else: print_info("Sending shared metrics disabled (collection stays local).") +def _record_send_opt_in_day() -> None: + """Stamp the consent day when the user says yes, not at first send. + + The gate excludes packages for periods before this day. Recording it + lazily on the first send pass would silently drop the opt-in day itself + whenever the next export happens after midnight UTC. + """ + try: + from hermes_cli.observability.shared_metrics import SharedMetricsStore + from hermes_cli.observability.shared_metrics_sender import opt_in_period + from hermes_cli.sqlite_util import write_txn + + store = SharedMetricsStore() + with store._connection() as connection: + with write_txn(connection): + opt_in_period(connection) + except Exception: + # Never block the wizard on telemetry bookkeeping; the sender still + # records the day on its first pass if this could not run. + logger.debug("Unable to record shared-metrics opt-in day", exc_info=True) + + # ============================================================================= # Post-Migration Section Skip Logic # ============================================================================= diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index 018178d916..3f4b160965 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -5570,6 +5570,43 @@ def _reconfigure_simple_requirements(ts_key: str): # ─── Main Entry Point ───────────────────────────────────────────────────────── +def _shared_metrics_state(config: dict) -> tuple[bool, bool]: + """Return (collection_enabled, send_enabled) from a config dict.""" + telemetry = config.get("telemetry") + telemetry = telemetry if isinstance(telemetry, dict) else {} + shared = telemetry.get("shared_metrics") + shared = shared if isinstance(shared, dict) else {} + return shared.get("enabled") is True, shared.get("send") is True + + +def _shared_metrics_menu_label(config: dict) -> str: + """Menu row for shared metrics, showing both consent states.""" + enabled, send = _shared_metrics_state(config) + if not enabled: + state = "off" + elif send: + state = "collecting + sending to Nous" + else: + state = "collecting locally" + return f"Configure shared metrics ({state})" + + +def _configure_shared_metrics_interactive(config: dict) -> None: + """Toggle shared-metrics collection and sending from `hermes tools`. + + Delegates to the setup wizard's prompt so the consent rules live in one + place: sending requires collection, and turning collection off also turns + sending off. + """ + from hermes_cli.setup import setup_telemetry + + before = _shared_metrics_state(config) + setup_telemetry(config) + after = _shared_metrics_state(config) + if before != after: + save_config(config) + + def tools_command(args=None, first_install: bool = False, config: dict = None): """Entry point for `hermes tools` and `hermes setup tools`. @@ -5694,6 +5731,7 @@ def tools_command(args=None, first_install: bool = False, config: dict = None): if len(platform_keys) > 1: platform_choices.append("Configure all platforms (global)") platform_choices.append("Reconfigure an existing tool's provider or API key") + platform_choices.append(_shared_metrics_menu_label(config)) # Show MCP option if any MCP servers are configured _has_mcp = bool(config.get("mcp_servers")) @@ -5705,8 +5743,9 @@ def tools_command(args=None, first_install: bool = False, config: dict = None): # Index offsets for the extra options after per-platform entries _global_idx = len(platform_keys) if len(platform_keys) > 1 else -1 _reconfig_idx = len(platform_keys) + (1 if len(platform_keys) > 1 else 0) - _mcp_idx = (_reconfig_idx + 1) if _has_mcp else -1 - _done_idx = _reconfig_idx + (2 if _has_mcp else 1) + _metrics_idx = _reconfig_idx + 1 + _mcp_idx = (_metrics_idx + 1) if _has_mcp else -1 + _done_idx = _metrics_idx + (2 if _has_mcp else 1) while True: idx = _prompt_choice("Select an option:", platform_choices, default=0) @@ -5721,6 +5760,13 @@ def tools_command(args=None, first_install: bool = False, config: dict = None): print() continue + # "Shared metrics" selected + if idx == _metrics_idx: + _configure_shared_metrics_interactive(config) + platform_choices[_metrics_idx] = _shared_metrics_menu_label(config) + print() + continue + # "Configure MCP tools" selected if idx == _mcp_idx: _configure_mcp_tools_interactive(config) diff --git a/scripts/e2e_shared_metrics_staging.py b/scripts/e2e_shared_metrics_staging.py index 55e03a7572..6adddcec68 100644 --- a/scripts/e2e_shared_metrics_staging.py +++ b/scripts/e2e_shared_metrics_staging.py @@ -28,9 +28,33 @@ def main() -> int: scratch = Path(tempfile.mkdtemp(prefix="hermes-telemetry-e2e-")) os.environ["HERMES_HOME"] = str(scratch) + # Staging is selected by writing config into the THROWAWAY profile, not by + # an environment override: a runtime env var that can retarget consented + # telemetry would be a consent hazard in production. + (scratch / "config.yaml").write_text( + "telemetry:\n" + " shared_metrics:\n" + " enabled: true\n" + " send: true\n" + f" endpoint: {STAGING}\n" + ) + from hermes_cli.observability.shared_metrics import SharedMetricsStore + from hermes_cli.observability.shared_metrics_send_config import ( + resolve_send_config, + ) from hermes_cli.observability.shared_metrics_sender import SharedMetricsSender + # Resolve through the real config path so this exercises what a user gets. + import yaml + + resolved = resolve_send_config( + yaml.safe_load((scratch / "config.yaml").read_text()) + ) + if not resolved.send or resolved.endpoint != STAGING: + print(f"FAIL: config did not resolve to staging: {resolved}") + return 1 + store = SharedMetricsStore( database_path=scratch / "metrics.sqlite3", outbox_directory=scratch / "outbox", @@ -97,7 +121,7 @@ def main() -> int: print(f" - {package_id} ({count} metrics)") print() - outcome = SharedMetricsSender(store, STAGING).send_pending() + outcome = SharedMetricsSender(store, resolved.endpoint).send_pending() print(f"outcome: sent={outcome.sent} rejected={outcome.rejected} " f"deferred={outcome.deferred}") print() diff --git a/tests/hermes_cli/test_shared_metrics_send_config.py b/tests/hermes_cli/test_shared_metrics_send_config.py index 235777d49a..227c7a1cfa 100644 --- a/tests/hermes_cli/test_shared_metrics_send_config.py +++ b/tests/hermes_cli/test_shared_metrics_send_config.py @@ -9,7 +9,6 @@ import pytest from hermes_cli.config import DEFAULT_CONFIG from hermes_cli.observability.shared_metrics_send_config import ( DEFAULT_ENDPOINT, - ENDPOINT_ENV_VAR, resolve_send_config, reset_warning_latch_for_tests, ) @@ -84,22 +83,29 @@ class TestEndpointPrecedence: ) assert resolved.endpoint == "https://example.test/v1" - def test_env_var_overrides_config(self, monkeypatch): - monkeypatch.setenv(ENDPOINT_ENV_VAR, "https://staging.test/v1") - resolved = resolve_send_config( - _config(enabled=True, send=True, endpoint="https://example.test/v1") - ) - assert resolved.endpoint == "https://staging.test/v1" + def test_no_environment_variable_can_redirect_telemetry(self, monkeypatch): + """A consent hazard: an inherited env var must not silently retarget. + + AGENTS.md also reserves HERMES_* for secrets, not behaviour. + """ + for name in ( + "HERMES_TELEMETRY_ENDPOINT", + "TELEMETRY_ENDPOINT", + "HERMES_SHARED_METRICS_ENDPOINT", + ): + monkeypatch.setenv(name, "https://attacker.test/v1") + resolved = resolve_send_config(_config(enabled=True, send=True)) + assert resolved.endpoint == DEFAULT_ENDPOINT def test_blank_endpoint_falls_back_to_production(self): resolved = resolve_send_config(_config(enabled=True, send=True, endpoint=" ")) assert resolved.endpoint == DEFAULT_ENDPOINT - def test_endpoint_is_stripped(self, monkeypatch): - monkeypatch.setenv(ENDPOINT_ENV_VAR, " https://staging.test/v1 ") - assert resolve_send_config(_config(enabled=True, send=True)).endpoint == ( - "https://staging.test/v1" + def test_endpoint_is_stripped(self): + resolved = resolve_send_config( + _config(enabled=True, send=True, endpoint=" https://staging.test/v1 ") ) + assert resolved.endpoint == "https://staging.test/v1" class TestTransportSafety: diff --git a/tests/hermes_cli/test_shared_metrics_send_wiring.py b/tests/hermes_cli/test_shared_metrics_send_wiring.py index 730376083f..8b90c4f812 100644 --- a/tests/hermes_cli/test_shared_metrics_send_wiring.py +++ b/tests/hermes_cli/test_shared_metrics_send_wiring.py @@ -214,3 +214,41 @@ class TestFailureIsolation: def test_join_is_safe_with_no_thread(self, runtime): runtime._join_send_thread(timeout=0.1) + + def test_join_waits_for_an_in_flight_send(self, runtime, monkeypatch): + """shutdown() must give a started send a chance to finish. + + A short-lived CLI exits straight after its final export; without the + join the daemon thread is killed mid-request, and the hook path is the + only delivery cadence this feature has. + """ + finished = [] + release = threading.Event() + + class SlowSender: + def __init__(self, store, endpoint, **kwargs): + pass + + def send_pending(self): + release.wait(3) + finished.append(True) + + monkeypatch.setattr( + "hermes_cli.observability.shared_metrics_sender.SharedMetricsSender", + SlowSender, + ) + _set_config(monkeypatch, _config(enabled=True, send=True)) + + runtime._export() + release.set() + runtime._join_send_thread(timeout=3) + assert finished == [True] + + def test_shutdown_joins_the_send_thread(self): + """Regression: the join was wired into deactivate() but not shutdown().""" + import inspect + + source = inspect.getsource(mod._Runtime.shutdown) + assert "_join_send_thread" in source, ( + "shutdown() must join the sender, or a CLI exit kills it mid-send" + ) diff --git a/tests/hermes_cli/test_shared_metrics_sender.py b/tests/hermes_cli/test_shared_metrics_sender.py index d0fec15bfa..4da879dfc3 100644 --- a/tests/hermes_cli/test_shared_metrics_sender.py +++ b/tests/hermes_cli/test_shared_metrics_sender.py @@ -148,6 +148,18 @@ class TestContractResponses: _sender(store, transport2).send_pending() assert transport2.calls == [] + @pytest.mark.parametrize("status", [401, 403, 404, 413, 422]) + def test_other_4xx_are_permanent_too(self, store, status): + """Retrying these every 15 minutes for 30 days fixes nothing.""" + _add_package(store, "pkg-1", "2026-08-26") + transport = FakeTransport(FakeResponse(status)) + outcome = _sender(store, transport).send_pending() + assert outcome.rejected == 1 + assert len(transport.calls) == 1 + row = _row(store, "pkg-1") + assert row["send_state"] == "rejected" + assert str(status) in row["last_error"] + def test_429_defers_using_retry_after(self, store): _add_package(store, "pkg-1", "2026-08-26") transport = FakeTransport(FakeResponse(429, retry_after="120")) @@ -342,27 +354,77 @@ class TestClaimingAndBounds: assert outcome.sent == MAX_PACKAGES_PER_PASS def test_two_concurrent_passes_do_not_double_send(self, store): - """Claiming is what stops two Hermes processes duplicating work.""" + """Claiming is what stops two Hermes processes duplicating work. + + The second pass must RECORD what it saw rather than raise: _send_one + catches every exception as a retryable transport failure, so an + assertion thrown inside a transport would be swallowed and this test + would pass no matter what the claim did. + """ _add_package(store, "pkg-1", "2026-08-26") - seen = [] + first_calls = [] + second_calls = [] + + def second_transport(endpoint, payload, *, timeout): + second_calls.append(payload) + return FakeResponse(202) def transport(endpoint, payload, *, timeout): - seen.append(payload) + first_calls.append(payload) # A second sender runs while the first is mid-flight. SharedMetricsSender( store, ENDPOINT, - post=lambda *a, **k: (_ for _ in ()).throw( - AssertionError("second pass must not claim a held package") - ), + post=second_transport, sleep=lambda _s: None, now=lambda: NOW, ).send_pending() return FakeResponse(202) _sender(store, transport).send_pending() - assert len(seen) == 1 + assert len(first_calls) == 1 + assert second_calls == [], ( + "a concurrent pass claimed a package already in flight" + ) + + def test_a_claim_leases_the_row_into_the_future(self, store): + """The lease, not the send result, is what blocks a concurrent pass.""" + _add_package(store, "pkg-1", "2026-08-26") + with store._connection() as connection: + with __import__( + "hermes_cli.sqlite_util", fromlist=["write_txn"] + ).write_txn(connection): + claimed = _sender(store, FakeTransport())._claim(connection, NOW) + assert len(claimed) == 1 + assert _row(store, "pkg-1")["next_attempt_at"] > "2026-08-26T12:00:00Z" + + def test_an_expired_lease_is_reclaimed(self, store): + """A process killed mid-pass must not strand its packages.""" + _add_package(store, "pkg-1", "2026-08-26") + _sender(store, FakeTransport(OSError("killed"), OSError(""), OSError(""))).send_pending() + + later = SharedMetricsSender( + store, + ENDPOINT, + post=(transport := FakeTransport(FakeResponse(202))), + sleep=lambda _s: None, + now=lambda: NOW + timedelta(hours=2), + ) + later.send_pending() + assert len(transport.calls) == 1 + + def test_a_lapsed_sender_cannot_resurrect_a_sent_package(self, store): + """Terminal state must win over a straggler's write.""" + _add_package(store, "pkg-1", "2026-08-26") + _sender(store, FakeTransport(FakeResponse(202))).send_pending() + assert _row(store, "pkg-1")["send_state"] == "sent" + + # A straggler from an earlier pass tries to defer the same row. + _sender(store, FakeTransport())._defer("pkg-1", 600, "stale") + assert _row(store, "pkg-1")["send_state"] == "sent", ( + "a lapsed pass overwrote a completed send" + ) class TestResilience: diff --git a/tests/hermes_cli/test_shared_metrics_sender_e2e.py b/tests/hermes_cli/test_shared_metrics_sender_e2e.py index 8568a9e6fe..552ae93552 100644 --- a/tests/hermes_cli/test_shared_metrics_sender_e2e.py +++ b/tests/hermes_cli/test_shared_metrics_sender_e2e.py @@ -40,6 +40,9 @@ class Ingest(BaseHTTPRequestHandler): { "headers": {k.lower(): v for k, v in self.headers.items()}, "body": json.loads(body.decode("utf-8")), + # Keep the RAW request bytes: comparing only the parsed body + # would not notice a non-deterministic transport encoding. + "raw": raw, "raw_len": len(raw), "decoded_len": len(body), } @@ -212,6 +215,18 @@ class TestRealTransport: _sender(store, server).send_pending() first, second = Ingest.received assert first["body"] == second["body"] + assert first["raw"] == second["raw"], ( + "the raw request bytes must match, not just the parsed body" + ) + + def test_a_gzipped_retry_is_byte_identical_on_the_wire(self, store, server): + """gzip embeds an mtime by default, which would break this.""" + _add(store, "pkg-1", metrics=200) + Ingest.script = [(503, {}, {}), (202, {}, {})] + _sender(store, server).send_pending() + first, second = Ingest.received + assert first["headers"].get("content-encoding") == "gzip" + assert first["raw"] == second["raw"] def test_several_packages_in_one_pass(self, store, server): for i in range(5): diff --git a/tests/hermes_cli/test_shared_metrics_tools_toggle.py b/tests/hermes_cli/test_shared_metrics_tools_toggle.py new file mode 100644 index 0000000000..31718d5d18 --- /dev/null +++ b/tests/hermes_cli/test_shared_metrics_tools_toggle.py @@ -0,0 +1,89 @@ +"""Tests for the `hermes tools` shared-metrics consent toggle. + +AGENTS.md requires outbound telemetry to be reachable from a config gate, the +setup prompt, AND `hermes tools`. These cover the third surface. +""" + +from __future__ import annotations + +import pytest + +from hermes_cli.tools_config import ( + _configure_shared_metrics_interactive, + _shared_metrics_menu_label, + _shared_metrics_state, +) + + +def _config(**shared): + return {"telemetry": {"shared_metrics": shared}} + + +class TestState: + def test_missing_telemetry_section_is_off(self): + assert _shared_metrics_state({}) == (False, False) + + def test_malformed_section_does_not_raise(self): + assert _shared_metrics_state({"telemetry": "nonsense"}) == (False, False) + + def test_reads_both_flags(self): + assert _shared_metrics_state(_config(enabled=True, send=True)) == (True, True) + + +class TestMenuLabel: + def test_off_state(self): + assert "off" in _shared_metrics_menu_label({}) + + def test_local_only_state(self): + label = _shared_metrics_menu_label(_config(enabled=True)) + assert "collecting locally" in label + assert "Nous" not in label + + def test_sending_state_names_the_destination(self): + label = _shared_metrics_menu_label(_config(enabled=True, send=True)) + assert "sending to Nous" in label + + +class TestToggle: + def test_enabling_send_persists(self, monkeypatch): + config = _config(enabled=True) + saved = {} + monkeypatch.setattr( + "hermes_cli.setup.prompt_yes_no", lambda *_a, **_k: True + ) + monkeypatch.setattr( + "hermes_cli.setup._record_send_opt_in_day", lambda: None + ) + monkeypatch.setattr( + "hermes_cli.tools_config.save_config", + lambda cfg: saved.update({"cfg": cfg}), + ) + _configure_shared_metrics_interactive(config) + assert config["telemetry"]["shared_metrics"]["send"] is True + assert saved, "a consent change must be written to disk" + + def test_no_write_when_nothing_changed(self, monkeypatch): + config = _config(enabled=False, send=False) + saved = [] + monkeypatch.setattr( + "hermes_cli.setup.prompt_yes_no", lambda *_a, **_k: False + ) + monkeypatch.setattr( + "hermes_cli.tools_config.save_config", lambda cfg: saved.append(cfg) + ) + _configure_shared_metrics_interactive(config) + assert saved == [] + + def test_disabling_collection_also_disables_sending(self, monkeypatch): + """The toggle must not leave send=true with nothing to send.""" + config = _config(enabled=True, send=True) + monkeypatch.setattr( + "hermes_cli.setup.prompt_yes_no", lambda *_a, **_k: False + ) + monkeypatch.setattr( + "hermes_cli.tools_config.save_config", lambda cfg: None + ) + _configure_shared_metrics_interactive(config) + shared = config["telemetry"]["shared_metrics"] + assert shared["enabled"] is False + assert shared["send"] is False