feat(profiles): env-gated discovery + event-driven dropdown health dots#48
Merged
Merged
Conversation
Three changes addressing the post-#46 audit findings on the profile system (tracked as bug #159 and design ticket #160). A) Event-driven health dots in the dropdown - NodePicker.handleSwitchNode fires NodesService.healthCheck() in the post-switch background refresh. - NodePicker.handleDiscover (Rescan for Nodes button) fires it after the scan completes. - nodesSlice.setProfileHealth now MERGES results by profileId instead of replacing the whole array. A partial probe -- one capped by WALLET_HEALTH_PROBE_MAX, or a switch that probed a subset -- no longer clears previously-known dot colors. Sticky-red: a node that went red stays red until the next successful probe answers from that profileId. - No continuous polling added; dots refresh ONLY on profile switch or on Rescan button click. Entries are never auto-removed. B) Env-gated auto-discovery + standardized profile-file path - node-profiles.service.ts: PROFILES_FILE now defaults to ./node-profiles.json (unchanged behavior) but honors WALLET_PROFILES_FILE for an explicit absolute-path override. Isolated demos and orchestrator-driven test runs set this to a deterministic fixture path instead of relying on cwd. - node-manager.service.ts: discoverNodes() bails immediately when WALLET_AUTODISCOVER=false. Production keeps the friendly auth-time scan; isolated/test runs get a frozen profile list. D) Sequential capped probe in checkAllHealth - Was Promise.allSettled(profiles.map(probe)) -- all profiles probed at once. Now: sequential loop with a configurable delay between probes (WALLET_HEALTH_PROBE_DELAY_MS, default 200ms) and a hard cap on attempts per scan (WALLET_HEALTH_PROBE_MAX, default 64). - One commando connection at a time keeps the burst small. Profiles beyond the cap are skipped with a logger.warn line. Together: predictable behavior, gentle probing, deterministic dot state for the orchestrator + the isolated demo, no impact on production users who don't set the new env vars.
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.
Background
The post-#46 orchestrator audit surfaced three related issues with the wallet's profile system:
nodeInfo.erroris truthy (i.e., when you're already broken).node-profiles.jsonby adding every local CLN node it can find, breaking isolation for the demo (8 profiles instead of the intended 2 after a single restart).checkAllHealthprobes all profiles in parallel — fine at small counts, but a connection burst with many profiles.This PR addresses all three, opt-in by env var where appropriate.
Changes
A — Event-driven dropdown health dots
No continuous polling. Per the design discussion, dots refresh only on:
NodePicker.handleSwitchNodefiresNodesService.healthCheck()in the post-switch background refresh.NodePicker.handleDiscoverfires it after the rescan completes.The slice's
setProfileHealthnow merges byprofileIdinstead of replacing the whole array. Partial probes (e.g. one capped byWALLET_HEALTH_PROBE_MAX) don't clear previously-known colors, and a profile that went red stays red until the next good probe answers from thatprofileId— the sticky-red behavior you asked for. Entries are never auto-removed.B — Env-gated discovery + standardized profile-file path
Two new env vars:
WALLET_AUTODISCOVERtrue)falseto skip the auth-time local-socket scan entirely — the wallet uses only the profiles in the file you handed itWALLET_PROFILES_FILE./node-profiles.json(cwd-relative, backward compat)Production: both unset → current friendly auto-discovery, current file location.
Isolated demo / orchestrator: both set → frozen, deterministic profile list every restart.
D — Sequential, capped probe in
checkAllHealthPromise.allSettled(profiles.map(probe))→ sequentialfor…awaitloop with two new env vars:WALLET_HEALTH_PROBE_DELAY_MS200WALLET_HEALTH_PROBE_MAX64logger.warnOne commando connection at a time. Worst case at 64 profiles, 200ms delay, 5s probe timeout ≈ a sequential scan that takes a few seconds — acceptable since it only fires on switch / explicit refresh.
Validation
WALLET_AUTODISCOVER=false+WALLET_PROFILES_FILE=/root/ss-walletdemo/node-profiles.jsoninenv.sh, restart, confirm dropdown returns to just the 2 demo profiles. Switch between them and watch the dots flip.Files changed
apps/backend/source/service/node-profiles.service.tsWALLET_PROFILES_FILEenv overrideapps/backend/source/service/node-manager.service.tsWALLET_AUTODISCOVER=falseearly-out + sequential cappedcheckAllHealthapps/frontend/src/components/ui/NodePicker/NodePicker.tsxhealthCheckcalls in both handlers +setProfileHealthimportapps/frontend/src/store/nodesSlice.tsxsetProfileHealthreducer