fix(telemetry): the send log records where each share went, and the receiver sentence names it (#2571) by trinity-ability · Pull Request #2708 · Abilityai/trinity

trinity-ability · 2026-09-11T14:41:47Z

Summary

The send log kept what happened to each share (sent_at, backfill, window_days, ok, http_status, error, payload) but not where it went. The panel then judged the newest entry against the currently configured TELEMETRY_SHARING_URL, so an operator who pointed the env var at a local test receiver, got a 200 and restored the default read "The receiving service acknowledged the last send" beside the production address — an acknowledgement the production receiver never gave. The data was truthful; the sentence was not.

Each entry now carries where that send went, and every reading of the log is judged by it.

One new entry key, destination — the RFC 6454 origin scheme://host[:port] of TELEMETRY_SHARING_URL at send time, and nothing else. strip_url_credentials runs first, so no userinfo reaches the parser's output, and the value is reconstructed from scheme / host / port alone — path, query and fragment are dropped by construction rather than filtered (a path can carry a token; an origin cannot). Canonical at write time: scheme lower-cased, host through the existing canonical_origin_host (trailing dot, UTS-46, ASCII passthrough for an underscore host, IP literals folded to one spelling), the port kept only when explicit and not the scheme's default. share_destination never raises — load-bearing, not tidy: share_now's entry dict is built outside its own try (bug(telemetry): the sharing heartbeat's 24h timer restarts with the backend, so an install that restarts daily shares once and never again #2618), so the key is set once, before anything can fail, and every path records it (2xx, transport error, schema refusal, outer catch).
Three additive get_status keys — configured_destination, receiver_destination, destination_changed. The comparison is plain string equality because both sides are canonicalised by the same function; destination_changed is False whenever either side is unknown, since claiming a change without knowing would fabricate exactly the certainty this issue exists to remove. No router change (the router spreads the status dict) and no models.py change.
receiver_hint is re-judged by the recorded destination — a 404 belongs to the send that received it, so the receiver_not_live / receiver_404 split now asks where that send went, falling back to the current URL only for an entry that recorded nothing. Reading a 404 against a URL configured afterwards is the same mistake the issue is filed about.
The panel names the host that answered rather than the address configured now, appends a clause when the address has since moved, and shows the destination per row as one more · segment in the existing gray meta span (<code>, the panel's machine-text idiom) — unknown host for entries written before the key existed. No migration: the log is a JSON list in system_settings, and the reader treats absence as unknown rather than backfilling it from today's URL.
Store defaults for the three new keys, so a pre-change backend during a rolling deploy leaves the panel on known values instead of undefined.

Fixes #2571

Verification

WAVE-4 /verify-local --skip-agent (full suites, before the rebase):

Suite	Result
`tests/unit`	15,100 passed, 31 skipped
Build + import smoke	OK
Boot + `/health`	OK
`tests/integration`	69 passed, 13 skipped, 2 registry-deselected, 1 failed (classified below)
`npm run test:unit` (frontend)	2,659 passed
`npm run check:tokens`	OK

The one integration red is tests/integration/test_circuit_breaker.py::TestDormantState::test_dormant_probes_after_cooldown, proven pre-existing and unrelated: the test file and the services it exercises are byte-identical to the base tree, the same node-id fails from the pristine base against the same stack, and it passed in a sibling lane's run on the same host — a harness race between a 0.05 s monkeypatched cooldown and a /data read-only import traceback.

Post-rebase neighbourhood (rebased onto 682fce300, the #2701 merge train; rebase was clean, no conflicts):

cd tests && python3 -m pytest unit/test_ent437_telemetry_consent.py unit/test_2217_canary_status.py \
  unit/test_2380_install_provenance.py unit/test_ent435_settings_sink_guard.py \
  unit/test_2618_heartbeat_dueness.py unit/test_2338_journey_catalog.py -q
→ 284 passed

cd src/frontend && npx vitest run tests/unit/telemetryConsent.spec.js \
  tests/unit/rawColorRatchet.spec.js tests/unit/loadingGateRatchet.spec.js
→ 3 files, 48 passed

Raw-colour scan of the panel: {raw_nongray: 0, raw_gray: 60, hardcoded_colors: 1} — byte-identical to its raw-color-baseline.json entry. The new segment sits inside the existing text-gray-500 dark:text-gray-400 span, so it adds zero colour classes. Neither ratchet baseline is modified by this branch (five open PRs touch raw-color-baseline.json; regenerating it would turn a no-op into a five-way conflict). The panel has no loading-gate-baseline.json entry and gains no loading gate.

Also run: /review (one fix applied — canonical_origin_host rather than canonical_host, so an IPv6 literal is one address however it is spelled, ent#399) and /cso --diff (clean). A second voice (gemini-critic) reviewed the plan against the code: five objections, two accepted — the IDNA/trailing-dot canonicalisation above, and the product question recorded below.

User-visible changes

Every string below is a delegated routine choice made in receiverCopy's existing voice (short declaratives, the env var named in caps, no "your", mechanism stated). Each names the pattern it copies. No string was reworded at ship time.

What the user sees	Pattern it copies
`The receiver at <destination> acknowledged the last send.` (ok, destination recorded)	[delegated — copies the existing `The receiver at ${shareUrl} answered 404.` phrase]
`The receiving service answered 404 at the default address, <destination>. …`	[delegated — copies FR-5's "at the default address" wording, which the ent#190 spec asserts]
`The receiver at <destination> answered 404. Check TELEMETRY_SHARING_URL.`	[delegated — the existing sentence, with the recorded destination replacing the current URL]
`The last send to <destination> failed; it is recorded below and retried automatically.`	[delegated — copies the existing `failed` sentence's register]
Appended when the address has moved: `TELEMETRY_SHARING_URL now points at <configured>; no send has gone there since.`	[delegated — copies the `receiver_404` sentence: env var in caps, short declaratives, no "your"]
Row segment `· <code>https://intake.abilityai.dev</code>`	[delegated — copies the row's existing `· HTTP {{ n }}` / `· {{ send.error }}` segments; `<code>` is the panel's machine-text idiom (env-var names in the hard-disabled banner, the share id)]
Row segment `· unknown host` for a pre-change entry	[delegated — the AC's own word; copies the `v-else-if="send.error"` fallback arm of that span]

Two open questions for the maintainer

Neither is decided. Both are recorded here as questions, with what the branch does today.

Q1 — what should the receiver sentence say when the newest entry predates this change? A pre-change entry records no host, so the ok sentence either stays as it is or admits the destination is unknown. Both are honest; one changes a shipped sentence for every existing install.

(a) The receiving service acknowledged the last send; its destination was not recorded. — the sentence is never more certain than the data, at the cost of one extra clause every existing install reads.
(b) — what the branch does today. The sentence is left byte-identical. Zero change for existing installs; it stays as ambiguous as the issue complains about, for the one case where the truth is genuinely unknowable — and the row beside it already reads unknown host, which is AC 4's own remedy.

This was withdrawn from the delegated set on purpose: appending a clause to an already-shipped sentence is a new user-visible default on a surface otherwise untouched for those installs, and the written delegation covers the mismatch sentence, the row label and the row's "unknown" rendering. Switching to (a) later costs one string and one spec assertion; nothing else in the branch depends on the answer.

Q2 — the receiver_404 + address-moved composition names TELEMETRY_SHARING_URL twice. When an overridden receiver answered 404 and the address has since moved, the two halves compose to:

The receiver at http://localhost:8787 answered 404. Check TELEMETRY_SHARING_URL. TELEMETRY_SHARING_URL now points at https://intake.abilityai.dev; no send has gone there since.

(a) — what the branch does today. Kept as-is. Both halves are fixed by the plan, each is correct on its own, and this is the only branch where they meet.
(b) Drop Check TELEMETRY_SHARING_URL. in this one branch, since the following sentence already says where the variable points.

Two notes on the acceptance criteria

AC 3 says "table" and "column"; the surface is neither. Recent sends renders as <ul> → <li> → <details> → <summary> with a badge, a timestamp and one gray meta span. There is no header row and no column structure anywhere in it, so "a column following the table's existing headers" has no referent. The pattern that does exist is the meta span's ·-joined segments, and the host is one more of those — the AC is met on the surface's own pattern, not on a structure the surface does not have.
test_share_now_records_a_404_and_names_the_missing_receiver was renamed, to test_404_hint_is_judged_by_the_recorded_destination_then_by_the_current_url, and its tail changed on purpose: a 404 belongs to the send that got it. The old tail asserted that re-pointing the env var after the fact flipped the hint to receiver_404 — which is the bug, stated as an expectation. The new tail asserts the hint stays receiver_not_live while destination_changed goes True, and keeps the old reading for a legacy entry that recorded nothing. This red against existing code was the failing-test-first signal, not a regression.

Follow-ups (not filed — for the maintainer)

get_status()["share_url"] is still the raw, un-stripped URL (pre-existing, not introduced here). The three new keys are all credential-stripped origins, so this change reduces exposure rather than adding to it, but the raw field remains on an admin-gated read.
Optional clause in docs/user-docs/operations/telemetry.md describing the per-row destination and the moved-address sentence for operators. Not required by the AC, which names only the flow doc and the requirements file (both updated).
tests/integration/test_circuit_breaker.py::TestDormantState::test_dormant_probes_after_cooldown is host-dependent and flakes under load — a registry candidate, or an upstream fix to the 0.05 s cooldown race.
.claude/agents/test-runner.md catalog row for the new #2571 test block — that file lives in the private .claude submodule and is out of scope for a public-repo PR.
Two test-strength nits, both in the new backend block: the _ORIGINS row https://tok@LEGACY@host.test/ checks that LEGACY did not survive, but share_destination lower-cases the host, so a leak would read as legacy and slip the assertion; and the fragment (#f) is pinned only on the returned value, not on the persisted JSON the way tok / x=1 / the path are.

Merge-gate note

The diff touches src/frontend/, so frontend-e2e only runs if the PR carries the ui label. This PR is opened as a draft with no labels, reviewers or assignees — labelling is the maintainer's call.

🤖 Generated with Claude Code

Requirements before implementation (Rule #1). §45.2 FR-5 gains the `destination` sentence — the scheme + host + port of TELEMETRY_SHARING_URL at send time, an RFC 6454 origin derived through `strip_url_credentials`, so it never carries userinfo, path or query — plus the three additive status keys (`configured_destination` / `receiver_destination` / `destination_changed`) that let the panel name the host that actually answered. Entries written before the key existed render as unknown; no migration. The feature flow's "Delivery that survives a missing receiver" item 1 replaces "The log does not yet record which host answered" with what it now records and how the 404 wording is judged (by where that send went, not by today's URL), and the `_record_send` line in the flow diagram carries the key. `architecture/backend.md` gains one clause on the ent#437 paragraph. #2571 leaves the Deferred list in both docs. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

The tests before the code. Backend (9 red): the `share_destination` origin table — userinfo, path, query and fragment dropped, case and the scheme's default port folded, an explicit port kept, IPv6 bracketed, total over unparseable input because it runs outside `share_now`'s own try; the same host written two ways as one destination (trailing dot, IDN vs A-label) while `localhost:8787` stays distinct from `localhost`; the key recorded on the 2xx, transport-error and schema-refusal paths with nothing credential-shaped reaching the persisted JSON; the issue's own scenario (a local receiver's 200 read beside the restored default) as `destination_changed`; legacy entries as unknown rather than as agreement with today's URL; and the router spreading the triple. `test_share_now_records_a_404_and_names_the_missing_receiver` is renamed to `test_404_hint_is_judged_by_the_recorded_destination_then_by_the_current_url` and goes red ON PURPOSE: a 404 from the default address stays worded that way after the env var is re-pointed, because the hint belongs to the send, not to the configuration read afterwards. That red is the fix, not a regression. Frontend (5 red): the host named in the sentence, the changed-address clause on every branch, a pre-#2571 entry never borrowing today's URL, no JS-isms for any wire value, and a source-structure guard over the panel's per-row destination and its `receiverCopy` call (no mount harness under environment: 'node'). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…y it (#2571) `share_destination(url)` is one pure helper: `strip_url_credentials` first, `urlsplit` second, `canonical_host` on the name, and the port only when it is explicit and not the scheme's default — an RFC 6454 origin and nothing else, so the stored value cannot carry userinfo, path, query or fragment. It is canonical at write time, which makes the comparison downstream plain string equality with no second normaliser to drift: the two sides are different inputs captured at different times, so one host retyped in another form (trailing dot, IDN vs A-label) must still compare equal or the panel reports a move that never happened. It returns None rather than raising — load-bearing, since `share_now` builds its entry dict outside its own try and `get_status` must never 500 the panel. `share_now` sets one key on that dict, where it is built, so every path records it for free (schema refusal, transport error, non-2xx, 2xx, outer catch). `get_status` answers three additive keys — `configured_destination`, `receiver_destination`, `destination_changed` — and `destination_changed` is False whenever either side is unknown, because claiming a change without knowing fabricates the certainty this exists to remove. `receiver_hint` now judges default-vs-override by the destination the entry recorded, falling back to the current URL only for an entry written before the key existed. A 404 belongs to the send that got it; reading it against the URL configured afterwards is the same mistake as the one reported. No migration, no new setting key (the field lives inside the existing `telemetry_sharing_recent_sends` JSON, so the ent#435 sink guard is untouched), no router change, no model change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…ved (#2571) `receiverCopy` takes an optional third argument — the destination the log recorded, the destination configured now, and whether they differ — so the sentence names the host that ANSWERED instead of the address configured at read time. When the configured address has since moved it appends one clause: "TELEMETRY_SHARING_URL now points at <configured>; no send has gone there since." That is true by construction: the newest send went elsewhere, so nothing later went there. Absence of the argument reproduces the shipped wording exactly — verified byte-identical for all four branches — so a legacy entry with no recorded destination, and an older backend during a rolling deploy, read as they always did. The sentence never borrows today's URL as if it were the one that answered. Each Recent sends row carries its destination as one more segment in the existing meta span, the value in <code> (the panel's machine-text idiom, the same one the env-var names and the share id use), with "unknown host" for a row written before the log recorded it. Zero new colour classes — the segment sits inside the span that already carries the row's gray — and no new loading gate. The store defaults the three keys so the panel never reads undefined. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…#2571) Tail steps. `tests/registry.json`: the ent#437 entry's stale clause — the 404 hint decided by the default URL — now says the hint follows the destination that send recorded, and the entry gains the #2571 coverage (the origin's folding rules and its total-over-garbage contract, the key on every send path, the changed-address signal, legacy entries as unknown, the router spread). `feature-flows/telemetry-sharing.md`: the two Testing bullets name what the pytest file and the vitest spec now pin, including the source-structure guard over the panel's per-row destination. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…2571) Review finding. `share_destination` hand-rolled its own IPv6 arm (`if ":" in raw: host = f"[{raw}]"`) beside `url_validation.canonical_host`, which is a SECOND normalisation of the one field the module's own `effective_port` docstring names as the hazard — and the module already answers this exact question: `canonical_origin_host` is "a host as an ORIGIN key", written for ent#399, where comparing IPv6 literals textually permanently refused a registered peer against its own card. The same defect lands here as the sentence this issue exists to delete: `https://[::1]:8443` and `https://[0:0:0:0:0:0:0:1]:8443` are one address, so a receiver addressed one way and re-typed the other reported `destination_changed` and printed "no send has gone there since" about an address every send was reaching. That is the plan's own argument for adopting `canonical_host` (trailing dot, IDN vs A-label), applied to the case it missed. Folding cannot equate two hosts a resolver would separate: `ipaddress` REFUSES the ambiguous IPv4 spellings rather than folding them, so `http://0177.0.0.1` stays distinct from `http://127.0.0.1` — pinned. Verified over the whole `_ORIGINS` table plus the live default, localhost and host.docker.internal URLs: exactly three outputs change, all of them the IPv6 spellings, and every other row is byte-identical. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

github-actions · 2026-09-12T10:56:44Z

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

github-actions · 2026-09-12T14:03:59Z

⚠️ Live-instance suite skipped — merge conflict against dev.

Resolve by merging dev locally and pushing the result; the next nightly re-tests.

trinity-ability and others added 6 commits September 11, 2026 15:37

Navigation Menu

fix(telemetry): the send log records where each share went, and the receiver sentence names it (#2571) - #2708

fix(telemetry): the send log records where each share went, and the receiver sentence names it (#2571)#2708
trinity-ability wants to merge 6 commits into
devfrom
vybe/issue-2571

trinity-ability commented Sep 11, 2026

Uh oh!

github-actions Bot commented Sep 12, 2026

Uh oh!

github-actions Bot commented Sep 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

trinity-ability commented Sep 11, 2026

Summary

Verification

User-visible changes

Two open questions for the maintainer

Two notes on the acceptance criteria

Follow-ups (not filed — for the maintainer)

Merge-gate note

Uh oh!

github-actions Bot commented Sep 12, 2026

Uh oh!

github-actions Bot commented Sep 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant