5 Commits

Author SHA1 Message Date
Ben Barclay 5e380d95ba refactor(telemetry): replace consent day-stamp with explicit intervals
Structural fix after five review rounds put four blockers in the same
subsystem. The root cause was representational: consent history is a
sequence of on/off intervals, but it was stored as ONE moving day-stamp
plus a revoked flag. Every fix had to mutate that scalar at exactly the
right moment from exactly the right place, and each round the mutation
was missing from some reachable path (write-once stamp in R3; recorded
inside a loop that never runs when sending is off in R4; dead code
whenever collection was off in R5).

Consent is now recorded as explicit intervals (send_consent_windows) and
eligibility is a pure derivation: a package is sent only when its whole
period falls inside a recorded window. One writer -
reconcile_send_consent - derives window state from an observation of
(config, now). It is idempotent and order-independent, so the wizard,
the relay, and the mid-pass check all call the same function and cannot
disagree; there are no edges to detect and no ordering between writers
to get wrong. The relay reconciles once per process BEFORE the
collection gate, which fixes round-5 D1 (enabled:false made the only
idle-path observer unreachable). The claim reads the table and never
writes it, removing the read-path mutation (D2's rewrite vector).

Timestamp discipline, each rule load-bearing and mutation-tested:
- 'obs' high-water mark: monotonic, advanced only by observations;
  confirms an open window forward (last_confirmed_at).
- 'data' high-water mark: advanced only by stored package period_end;
  clamps window OPENS so a rolled-back clock cannot slide a window
  under refused packages already on disk (round-5 D2).
- A close stamps last_confirmed_at, never "now": consent is asserted
  only for observed time, so a hand-edited config with no process
  running for 90 days fails closed (round-5 D1 strongest form).
- The gate requires period containment, not period_start >=, so an
  intra-day revoke/re-enable holds back the day package (round-5 D3).
- Unlike the day-stamp, a revoke/re-enable cycle no longer destroys the
  undelivered backlog from the earlier consented window (round-5 D4).

The redesign was validated BEFORE implementation against all 13
reproduced defect scenarios on a real store; the first two drafts each
failed scenarios in that harness (v1 leaked the unobserved-gap case by
closing at "now"; v2 leaked refused windows by letting data stamps
confirm consent). The harness ships as
tests/hermes_cli/test_shared_metrics_consent_windows.py.

Deleted: OPT_IN_PERIOD_KEY, SEND_REVOKED_KEY, LAST_SEEN_SEND_KEY,
opt_in_period(), record_revoked(), the relay edge detector body, and the
setup wizard's key bookkeeping (~170 lines of transition machinery).
Schema: two additive tables, version deliberately unchanged; verified
against a copy of the real production DB (13 rows intact, reopen no-op).

Also kills round-5's M8 survivor: the seen-exclusion mutation now fails
the suite. New mutation sweep: 8/8 killed, including one vacuous test of
my own this round (obs-mark monotonicity was covered only by
coincidence of the data mark; now pinned directly).

Documented cost: a fresh package waits at most one process start after
its period completes before release (fail-closed direction).

270 tests pass; ruff and windows-footguns clean. Staging E2E re-run
through the interval gate: both packages 202.
2026-08-27 10:42:31 +10:00
Ben Barclay 613849c190 fix(telemetry): close the consent window on the config transition
Fourth independent review. Two more consent leaks, both reproduced through
the real relay entry point before and after the fix. Both are failures of
my own round-3 fix, which recorded revocation in the wrong place.

BLOCKER 1 - revoking while idle recorded nothing. _record_revocation lived
inside send_pending's loop, but _send_exported_packages returns early when
send is false, before a sender is ever constructed. The dominant case is a
user turning sending off while no pass is running, so the loop that was
meant to observe the revocation could never run. Reproduced: 6 periods
collected during a refused window were transmitted on re-enable.

The window now closes on the observed config EDGE, before the early return.
Last-seen send state is persisted because each hook fires in a fresh
process, so a true->false transition is only visible by comparison. The
rising edge also opens the window explicitly: the sender only runs when
there is something to send, so a user who opts in and out before any
package exists would otherwise have no window for record_revoked to close.

BLOCKER 2 - turning COLLECTION off never recorded revocation. The
not-enabled branch in setup.py force-set send=false and returned without
calling _record_send_consent_change, so `hermes tools` -> disable shared
metrics silently dropped consent while leaving the window open. Same
retroactive release on re-enable. Both consent surfaces now record, and
setup keeps the relay's edge detector in step.

Also, from the same review's mutation sweep:
- the scheme check is now pinned as an allowlist. Replacing the http test
  with `if True` survived the entire suite, because every non-http case
  targeted a REMOTE host where the loopback branch rejects anyway. Only a
  non-http scheme on loopback distinguishes the two. Shipped behaviour was
  already correct; nothing guarded it.
- A.3 no longer claims rotation bounds long-term linkability outright.
  Measured against 11 real packages: resource is a stable low-entropy
  tuple and periods are contiguous across a rotation, so for a RARE
  configuration those can bridge windows. The honest claim is that
  rotation raises the cost, not that it makes correlation impossible.

Two mutants are documented as unkillable rather than papered over with
tests that only appear to cover them: the _defer clamp is unreachable from
any current caller, and widening the falling-edge check to an
unconditional else is behaviourally equivalent because record_revoked is
idempotent and no-ops without an open window.

An earlier version of the anti-spurious-revocation test could not fail
either - it used a never-consented store, where record_revoked no-ops
regardless. Rewritten to opt in, revoke, re-enable, and then assert that a
steady enabled state does not re-close the reopened window.

259 tests pass. Staging E2E re-run: both packages 202.
2026-08-27 09:47:47 +10:00
Alex Fournier 14bed44c8c Reapply "feat(observability): integrate NeMo Relay runtime and shared metrics"
Signed-off-by: Alex Fournier <afournier@nvidia.com>
2026-07-27 21:10:51 -07:00
Jeffrey Quesnelle 841a5a744a Revert "feat(observability): integrate NeMo Relay runtime and shared metrics" 2026-07-27 22:28:08 -04:00
Alex Fournier 36185bf2e2 feat(telemetry): expose shared metrics setup
Signed-off-by: Alex Fournier <afournier@nvidia.com>
2026-07-23 13:36:00 -07:00