fix(cli): enrich template cardinality error with per-candidate presence and profile hints - #4825
Open
wpfleger96 wants to merge 3 commits into
Open
fix(cli): enrich template cardinality error with per-candidate presence and profile hints#4825wpfleger96 wants to merge 3 commits into
wpfleger96 wants to merge 3 commits into
Conversation
…oned-at hints When buzz channels create --template fails with a duplicate-instance error (persona has N > 1 live instances), the error listed bare pubkeys only — indistinguishable at a glance for agents whose kind:0 metadata looks identical (same name, same avatar). Add best-effort hint decoration to the error output: - presence status (online/offline) from kind:40902 - provisioned-at date (YYYY-MM-DD) from kind:0 created_at In the incident that surfaced this defect, the stale instances were precisely the offline ones, making presence the highest-signal field for deciding which to archive. Design constraints preserved: - apply_cardinality_rule stays pure: hints are fetched by the async caller (build_roster_resolution) and passed in, so the rule remains directly unit-testable without relay I/O. - Fail-open: if either lookup fails, the error prints with bare pubkeys rather than failing in a new way. Absent hint entries are silently omitted per candidate. - Zero-instance and single-instance paths are unchanged. - format_candidate is a pure helper, separately testable. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
- Rename provisioned_at -> profile_updated_at: kind:0 is replaceable
state (desktop republishes on rename/reconciliation), so created_at
reflects last profile update, not provisioning time. Output changes
from 'provisioned YYYY-MM-DD' to 'profile updated YYYY-MM-DD'.
- Make hint fetching operationally fail-open: happy path (no duplicate
live instances after archive filtering) performs zero hint queries.
When duplicates exist, the two relay lookups (presence + profile) run
concurrently via tokio::join! and the whole enrichment phase is bounded
by a 3-second timeout; on expiry the error prints promptly with bare
pubkeys. scan_managed_agents and fetch_archived_snapshot also run
concurrently now. Fixes the 'fire-and-forget' comment that was false.
- Extract build_hint_map as a sync production function taking raw
presence/profile event slices, so it is directly unit-testable without
a relay. fetch_candidate_hints becomes a thin async wrapper: query
concurrently, parse, delegate. Five boundary tests added:
- p-tag beats author for relay-signed presence events
- presence failure does not suppress profile hints
- profile failure does not suppress presence hints
- malformed entries are skipped without panic
- both failures yield empty map (bare pubkeys in error)
Mutation check: deleting p-tag selection fails 1 test; gutting
build_hint_map fails 3 tests.
- Reuse presence_subject from users.rs (pub(crate)) instead of
re-implementing the same p-tag/author fallback inline.
- Replace hand-rolled Gregorian arithmetic with chrono::DateTime
(already a direct buzz-cli dep); out-of-range timestamp omits the
hint rather than computing garbage.
- Fix kind-40902 comment: relay-synthesized on demand, not
parameterised-replaceable.
333/333 buzz-cli tests pass. fmt + clippy clean.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Extract assemble_roster_resolution<F, Fut> from build_roster_resolution containing duplicate detection, conditional fetch_hints call, and finalize_roster_resolution delegation. build_roster_resolution reduces to: gather slugs, tokio::join! scan + archive, delegate with the real fetch_candidate_hints closure. Four tokio::test cases against the production function pin both mutations: - duplicate pair -> fetcher invoked with exactly those pubkeys (mut a fails) - single instance -> fetcher never called / panic fires (mut b fails) - trusted archive archives one of a pair -> fetcher suppressed (mut b fails) - untrusted archive with pair -> fetcher called conservatively (mut a fails) Mutation (a) replace conditional fetch with HashMap::new(): 2 tests fail Mutation (b) remove emptiness gate / always fetch: 2 tests fail 337/337 buzz-cli tests pass. fmt + clippy clean. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
buzz channels create --templatefails with a bare-pubkey duplicate-instance error when a persona has more than one live instance. When duplicate instances share the same display name and avatar, the error gives the operator no signal to identify which instance is stale.What changed
crates/buzz-cli/src/commands/channels.rs(boundary) + one visibility line incrates/buzz-cli/src/commands/users.rs(fn presence_subject→pub(crate)).Hint fetch architecture
assemble_roster_resolution<F, Fut>— post-fetch stage extracted frombuild_roster_resolutionas an injectable-fetcher async generic. It contains: archive-filter-based duplicate detection, the conditionalfetch_hintscall, andfinalize_roster_resolutiondelegation. Accepting the fetcher as a closure makes the wiring directly testable without a relay.build_roster_resolutionreduces to: gather slugs →tokio::join!(scan, archive)→ delegate with the realfetch_candidate_hintsclosure.Zero hint queries on the happy path:
assemble_roster_resolutiononly invokes the fetcher when duplicate live instances exist after archive filtering. Untrusted archive snapshot (Err) conservatively treats all found instances as live.When duplicates exist, both queries (presence kind:40902 + profile kind:0) run concurrently via
tokio::join!inside a single 3-secondtokio::time::timeout. On expiry, bare pubkeys are printed promptly.Response-to-map conversion —
build_hint_mapSync production function taking raw presence/profile event slices. Relay-signed presence events carry the agent pubkey in the
ptag (not the event author);build_hint_mapcallspresence_subjectfromusers.rs(nowpub(crate)) to resolve the correct key.Hint display —
format_candidateAppends
[online/offline, profile updated YYYY-MM-DD](or subset) to each candidate pubkey in the error.profile_updated_atnames the field correctly: kind:0 is replaceable state that desktop republishes on rename and profile reconciliation, socreated_atreflects the last profile update, not provisioning time. Date formatted viachrono::DateTime::from_timestamp; out-of-range timestamps omit the date. Missing entries fall back to bare pubkeys.Cardinality rule
apply_cardinality_ruleremains pure and relay-free; zero/one/many semantics unchanged.Tests
16 new tests in
channels.rs:assemble_roster_resolutionwiring tests (4,#[tokio::test]):assemble_roster_resolution_duplicate_pair_invokes_fetcher_with_their_pubkeys— fetcher called, error contains both pubkeysassemble_roster_resolution_single_instance_never_invokes_fetcher— panic closure proves fetcher not calledassemble_roster_resolution_trusted_archive_removes_duplicate_suppresses_fetcher— archived instance resolves pair; fetcher suppressedassemble_roster_resolution_untrusted_archive_invokes_fetcher_conservatively— Err archive → fetcher calledbuild_hint_mapboundary tests (5):build_hint_map_uses_p_tag_over_author_for_presence— relay author is not the map key; agent p-tag winsbuild_hint_map_presence_failure_profile_survives— one lookup failure does not suppress the otherbuild_hint_map_profile_failure_presence_survives— symmetricbuild_hint_map_malformed_entries_are_skipped— no panics on malformed JSONbuild_hint_map_both_failures_yield_empty_map— total failure → empty map → bare pubkeysformat_candidateformatting tests (5):[status][profile updated YYYY-MM-DD][status, profile updated YYYY-MM-DD]apply_cardinality_ruleintegration tests (2):Mutation check results:
HashMap::new(): 2 wiring tests failbuild_hint_map: 3 tests fail337/337
buzz-clitests pass.cargo fmt+cargo clippyclean.