From 3815d153f032f75fb03eef7591dacf3873f94406 Mon Sep 17 00:00:00 2001 From: guyverino Date: Mon, 7 Sep 2026 12:25:46 +0200 Subject: [PATCH 1/2] feat(core): re-read a core's retained diagnostics without an event The protocol has no request for a diagnostics list. A core pushes one on connection and again whenever its own detectors change it; moonproto's docs/problems.md says outright that "there is no periodic diagnostic-list refresh", and the only client command that produces a fresh list is the destructive clear. That would make a refresh button pure theatre, except for one thing the library does silently. moonproto mutates its retained problems through three paths and only two raise an event this feed can follow. The third is the hard-session reset: on a ServerToken or peer-app-token CHANGE - a hard reconnect, or the MoonBot process restarting behind the same endpoint - events/active.rs assigns `settings.problems = Default::default()` and publishes nothing. The terminal never respawns its feed thread for that, so its own store is not rebuilt either and keeps serving the findings of a core instance that no longer exists, with `supported` still true. The restarted core normally pushes a list within seconds and repairs it; a core too old for the extension never does, and the stale rows then stand forever. `CoreCmd::RefreshProblems` re-reads what moonproto holds now and publishes it. Usually that equals what the store already has and the store drops it. After a silent reset it does not, and the honest answer - this core has told this connection nothing - replaces the previous instance's findings, which is the same rule `CoreData::begin_connection_attempt` applies when the terminal itself reconnects a core. The request joins the existing event gate rather than branching beside it: both reasons read the same retained state through the same projection, so one gate is all the difference between them. `convert::snapshot_when` carries the snapshot/flatten/project chain that `settings_event_snapshot` already used, so the tenth publish block in that loop is not a hand-rolled copy of the other nine. `clear_core_problems_many` and `refresh_core_problems_many` follow `set_auto_detect_many`: one command per core, because that is what the protocol offers, and the cores whose channel accepted are returned so a caller can report what it actually reached. --- crates/moon-core/src/feed/live/commands.rs | 15 +++++++- crates/moon-core/src/feed/live/convert.rs | 30 ++++++++++++--- crates/moon-core/src/feed/live/mod.rs | 27 ++++++++----- crates/moon-core/src/feed/mod.rs | 25 ++++++++++++ crates/moon-core/src/session/commands.rs | 45 ++++++++++++++++++++++ 5 files changed, 125 insertions(+), 17 deletions(-) diff --git a/crates/moon-core/src/feed/live/commands.rs b/crates/moon-core/src/feed/live/commands.rs index df7112c9..56d7be23 100644 --- a/crates/moon-core/src/feed/live/commands.rs +++ b/crates/moon-core/src/feed/live/commands.rs @@ -695,7 +695,9 @@ fn rebuild_sync( /// `SetMarket` queue entries are wake/order markers; their payloads can be stale behind an action /// backlog, so the shared authoritative snapshot is adopted before and after the batch. The return /// value tells the live loop whether it disconnected, emptied the queue, or must poll again without -/// blocking. `core_config_events` collects any shared-config edit lifecycle events a queue-drain +/// blocking. `problems_relist` is raised by an operator-requested diagnostics re-read and consumed +/// by the caller's own publish block, so a burst of presses costs one snapshot read rather than +/// one per press. `core_config_events` collects any shared-config edit lifecycle events a queue-drain /// send produced; the caller sends them as `FeedMsg::CoreConfigEdit` and stamps their clock, the /// same as the events an event-batch-driven `SharedConfigSequence::drive` produces. pub(super) fn drain_commands( @@ -706,6 +708,7 @@ pub(super) fn drain_commands( market_role: &mut MarketRoleState, force_market_sample: &mut bool, orders_mutated: &mut bool, + problems_relist: &mut bool, local_strat_edits: &mut LocalStratEdits, strategy_placements: &mut StrategyPlacementGuard, client_settings_sequence: &mut ClientSettingsSequence, @@ -1491,6 +1494,16 @@ pub(super) fn drain_commands( ), } } + Ok(CoreCmd::RefreshProblems) => { + // Deliberately sends NOTHING: see `CoreCmd::RefreshProblems` for why no request + // exists. The flag makes the live loop republish from the retained snapshot once + // this batch is drained. + *problems_relist = true; + log::debug!( + "core {} problems re-read requested", + crate::feed::core_label(server.id) + ); + } Ok(CoreCmd::SetAutoDetect(on)) => { // Passive mode off/on; the new value reaches the store via RuntimeStateUpdated, // the same command that carries `is_started`. diff --git a/crates/moon-core/src/feed/live/convert.rs b/crates/moon-core/src/feed/live/convert.rs index 0c969e55..a9ecffc8 100644 --- a/crates/moon-core/src/feed/live/convert.rs +++ b/crates/moon-core/src/feed/live/convert.rs @@ -269,12 +269,30 @@ pub(super) fn settings_event_snapshot( matched: impl Fn(&Event) -> bool, extract: impl FnOnce(Arc) -> Option, ) -> Option { - events - .iter() - .any(matched) - .then(|| client.snapshot()) - .flatten() - .and_then(extract) + snapshot_when(events.iter().any(matched), client, extract) +} + +/// Read the retained snapshot when `gate` says to, and project it. +/// +/// The half of [`settings_event_snapshot`] that is not about events, split out for the one caller +/// whose gate is not an event at all: an operator asking for a republish of something the wire will +/// never announce again. Written as a helper rather than copied inline so the snapshot/flatten/ +/// project chain has one spelling — nine sibling publish blocks in the live loop reach it through +/// [`settings_event_snapshot`], and a tenth on a hand-rolled copy is how the two drift. +/// +/// Args: +/// gate: Whether there is any reason to read the snapshot at all. +/// client: Connected moonproto client holding the retained state. +/// extract: Projects the retained snapshot into the terminal's own type. +/// +/// Returns: +/// The projection, or `None` when the gate is shut or no snapshot exists yet. +pub(super) fn snapshot_when( + gate: bool, + client: &MoonClient, + extract: impl FnOnce(Arc) -> Option, +) -> Option { + gate.then(|| client.snapshot()).flatten().and_then(extract) } /// Convert protocol-v4 `KernelHealth` into terminal telemetry and stamp its diff --git a/crates/moon-core/src/feed/live/mod.rs b/crates/moon-core/src/feed/live/mod.rs index 0f246e06..85982229 100644 --- a/crates/moon-core/src/feed/live/mod.rs +++ b/crates/moon-core/src/feed/live/mod.rs @@ -668,6 +668,7 @@ pub(super) fn run( // `SetMarket` contains complete desired state; the other coordinator commands are deltas // or actions. A closed channel means the coordinator has exited, so disconnect. let mut orders_mutated = false; + let mut problems_relist = false; let mut core_config_events = Vec::new(); let command_drain = drain_commands( cmd_rx, @@ -677,6 +678,7 @@ pub(super) fn run( market_role, &mut force_market_sample, &mut orders_mutated, + &mut problems_relist, &mut local_strat_edits, &mut strategy_placements, client_settings_sequence, @@ -1542,22 +1544,27 @@ pub(super) fn run( // Nothing is ever requested here: the core sends its list unprompted on connection, and a // core too old for the extension simply never sends one. That is the whole compatibility // story — see `CoreProblems::supported`. - let problems = settings_event_snapshot( - &events, - &client, - |ev| { + // + // An operator-requested re-read is a SECOND reason to read the same retained state, so it + // joins the event gate rather than branching beside it. There is no event to wait for: the + // wire cannot be asked for a list, and moonproto empties this state without one when a + // ServerToken changes — see `CoreCmd::RefreshProblems`. Either reason reads the same + // snapshot through the same projection, so one gate is all the difference between them. + let want_problems = problems_relist + || events.iter().any(|ev| { matches!( ev, &Event::Settings( SettingsEvent::ProblemsUpdated | SettingsEvent::ProblemConfirmed { .. } ) ) - }, - // Wrapped in `Some` because an EMPTY list is a real answer — the core looked and found - // nothing — and the helper would otherwise drop it as "nothing to report", which is the - // one reading this feature must never produce. - |state| Some(convert::problems_from_proto(&state.settings().problems)), - ); + }); + // Wrapped in `Some` because an EMPTY list is a real answer — the core looked and found + // nothing — and the helper would otherwise drop it as "nothing to report", which is the one + // reading this feature must never produce. + let problems = convert::snapshot_when(want_problems, &client, |state| { + Some(convert::problems_from_proto(&state.settings().problems)) + }); if let Some(problems) = problems { if tx.send(FeedMsg::Problems(problems)).is_err() { break; diff --git a/crates/moon-core/src/feed/mod.rs b/crates/moon-core/src/feed/mod.rs index 83b1cb4a..3c14e863 100644 --- a/crates/moon-core/src/feed/mod.rs +++ b/crates/moon-core/src/feed/mod.rs @@ -644,6 +644,31 @@ pub enum CoreCmd { /// The UI must therefore confirm before sending, and must NOT clear its local rows /// optimistically: the answer is the core's next full list. ClearProblems, + /// Re-publish one core's diagnostics from the library's retained snapshot, without an event. + /// + /// NOT a wire request, because there is no such thing. The protocol has no way to ask a core + /// for its diagnostics list: the core pushes one on connection and again whenever its own + /// detectors change it, moonproto's `docs/problems.md` states outright that "there is no + /// periodic diagnostic-list refresh", and the only client command that produces a fresh list is + /// the destructive [`CoreCmd::ClearProblems`]. + /// + /// It is nonetheless a real repair rather than a decorative button, and for one specific reason. + /// moonproto mutates its retained problems through exactly three paths, and only two of them + /// raise an event this feed can follow. The third is the hard-session reset: on a ServerToken or + /// peer-app-token CHANGE — a hard reconnect, or the MoonBot process restarting behind the same + /// endpoint — `events/active.rs` assigns `settings.problems = Default::default()` and publishes + /// NOTHING. Our own store is not rebuilt then either, because the terminal never respawned the + /// feed thread, so it keeps serving the findings of a core instance that no longer exists, with + /// `supported` still true. The restarted core normally pushes a list of its own and repairs + /// that within seconds; a core too old for the extension never does, and the stale rows then + /// stand forever. + /// + /// So this re-reads what moonproto holds NOW and publishes it. Usually that equals what the + /// store already has and the store drops it. After a silent reset it does not, and the honest + /// answer — "this core has not told this connection anything" — replaces the previous + /// instance's findings. That replacement is the point, not a hazard: it is the same rule + /// `CoreData::begin_connection_attempt` applies when the terminal itself reconnects a core. + RefreshProblems, } /// Complete market-role assignment published independently of the bounded command backlog. diff --git a/crates/moon-core/src/session/commands.rs b/crates/moon-core/src/session/commands.rs index 19cfbebc..d3e33b0f 100644 --- a/crates/moon-core/src/session/commands.rs +++ b/crates/moon-core/src/session/commands.rs @@ -890,6 +890,51 @@ impl SessionManager { self.send_core_cmd(core, CoreCmd::ClearProblems, "clear problems") } + /// Clear the diagnostics of a whole scope — every core a panel currently covers. + /// + /// One command per core, like [`Self::set_auto_detect_many`], because that is what the protocol + /// offers. Far more destructive than the single-core form by exactly the number of cores given, + /// so a caller must state that number to the operator before asking. + /// + /// Args: + /// cores: Cores whose diagnostics are dropped. + /// + /// Returns: + /// The cores whose command channel accepted the intent, in the order given. + pub fn clear_core_problems_many(&self, cores: &[CoreId]) -> Vec { + self.send_many(cores, "clear problems", |core| { + self.clear_core_problems(core) + }) + } + + /// Re-read one core's diagnostics from the library's retained snapshot. + /// + /// Nothing is sent to the core and nothing on the core changes: the protocol has no request for + /// a diagnostics list at all, so this only re-publishes what already arrived. See + /// [`CoreCmd::RefreshProblems`] for the whole reasoning. + /// + /// Args: + /// core: Core whose retained list is re-published. + /// + /// Returns: + /// Whether the command reached the core's channel. + pub fn refresh_core_problems(&self, core: CoreId) -> Result<()> { + self.send_core_cmd(core, CoreCmd::RefreshProblems, "refresh problems") + } + + /// Re-read the diagnostics of a whole scope. See [`Self::refresh_core_problems`]. + /// + /// Args: + /// cores: Cores whose retained lists are re-published. + /// + /// Returns: + /// The cores whose command channel accepted the intent, in the order given. + pub fn refresh_core_problems_many(&self, cores: &[CoreId]) -> Vec { + self.send_many(cores, "refresh problems", |core| { + self.refresh_core_problems(core) + }) + } + /// Turn one core's AutoDetect on or off — Moonbot's passive mode, inverted. /// /// The command is an intent: the core answers with a new runtime state, which is what any From 9a0c486500044c0febd838477447007d094e4977 Mon Sep 17 00:00:00 2001 From: guyverino Date: Mon, 7 Sep 2026 12:25:47 +0200 Subject: [PATCH 2/2] feat(core-status): let the Problems tab act on what it is showing MoonBot's own Problems window carries "reset all" and "refresh". This tab had neither: it had a channel test and a clear, both gated on the operator having hand-picked exactly one core in the panel's core selector. That gate was permanently shut on every workspace-owned panel - Auto pins its scope and renders the selector read-only, so the retained Classic pick can never be set or cleared there - which is why both buttons read as always greyed. Three buttons now, and ONE rule behind all of them: a clicked finding narrows them to its core, and no click means the panel's whole scope. That is this panel's own convention for a bulk command, the same "the selection when the operator has made one, the displayed scope otherwise" the footer's fleet update already obeys. The scope itself is set where it always was - the core selector in Classic, the workspace in Auto - so no button carries a private notion of which cores it means any more. The pick is stored as a CORE, never as the clicked row index. The finding list is rebuilt from live core data on every repaint, so an index outlives the row it named: one finding appearing or clearing above it re-points it at a different core, and the reset it narrows cannot be undone. The click resolves its index against the list it was drawn from, while that list is still the one on screen. Clicking any row of the picked core clears the pick, because a narrowing that cannot be undone strands the operator on one core. The table keeps its default selection mode on purpose. Taking the click through `controlled_row_selection` also hands the caller the row highlight and returns early from the table's entire keyboard block, which would cost up/down/home/end navigation in a table that is read far more than it is clicked. Reset is labelled "Reset", not MoonBot's "Reset all": their window belongs to one core, so "all" there means all of that core's findings, while this tab spans a fleet and the same word would read as "all cores" - a lie the moment the operator has clicked one. The tooltip and the confirm both state how many cores the press will reach, the confirm names them through the same non-truncating list the footer's fleet confirm uses, and the outcome reports the shortfall: "reached 9 of 12" rather than a success toast over a partial result. Refresh says in its tooltip that it cannot ask the core for anything, because the label promises a round trip the wire cannot make. Known gaps, deliberate: the synthetic core swallows both new commands, so the tab is inert under --fixture; and a reset still reaches only the connected cores of a scope, which the confirm's count states but its text does not dwell on. --- .../src/panels/core_status/interactions.rs | 346 ++++++++++++++---- .../src/panels/core_status/mod.rs | 168 +++++---- .../src/panels/core_status/problems.rs | 303 +++++++++++---- .../src/panels/core_status/problems/tests.rs | 64 +++- locales/core_status.yml | 132 +++++-- 5 files changed, 768 insertions(+), 245 deletions(-) diff --git a/crates/moon-ui-gpui/src/panels/core_status/interactions.rs b/crates/moon-ui-gpui/src/panels/core_status/interactions.rs index 56effde2..2b735873 100644 --- a/crates/moon-ui-gpui/src/panels/core_status/interactions.rs +++ b/crates/moon-ui-gpui/src/panels/core_status/interactions.rs @@ -18,7 +18,7 @@ use super::model::ServerKey; use super::update_menu; use super::{ChartWindow, CoreStatusMode, CoreStatusView, ordering, server_view}; use crate::design; -use moon_core::feed::{ConnStatus, UpdateTarget}; +use moon_core::feed::UpdateTarget; use moon_core::session::CoreId; use rust_i18n::t; @@ -597,6 +597,10 @@ impl CoreStatusView { let order = self.visible_order(cx); self.select_all_visible_cores(&order, cx); } else if key == "escape" && self.core_selection.len() > 0 { + // Deliberately NOT extended to the Problems row selection. Escape already carries a + // window-level meaning here (it closes an open chart), and adding a second one to the + // same key risks swallowing that. The Problems selection is cleared by clicking its + // row again, which is the gesture that set it. self.clear_core_selection(cx); } } @@ -733,15 +737,9 @@ impl CoreStatusView { /// Returns: /// `true` only for a core whose session reports `Ready`. fn core_is_ready(&self, core: CoreId, cx: &App) -> bool { - matches!( - self.backend - .read(cx) - .session - .store() - .core(core) - .map(|data| data.status.clone()), - Some(ConnStatus::Ready) - ) + // The app's own spelling of "reachable", shared with every other fleet action rather than + // hand-written a third time in this panel. + self.backend.read(cx).session.core_run_state(core).online } /// A core's configured display name, or its id when the config no longer holds it. @@ -791,6 +789,8 @@ impl CoreStatusView { dialog, t!("core_status.problems_test_title").to_string(), question.clone(), + // The test names its one core inside the question, like the one-core reset. + Rc::from([]), "core-status-problem-test", MoonButtonVariant::Blue, cx, @@ -804,50 +804,75 @@ impl CoreStatusView { ); } - /// Open the ONE confirm clearing a core's diagnostics gets. + /// Open the ONE confirm a diagnostics reset gets, for one core or for a whole scope. + /// + /// Every core listed drops every confirmed finding AND every pending hypothesis, for every + /// terminal watching it, with no way back, and it fixes nothing — a cause that persists produces + /// a new fact later. /// - /// The core drops every confirmed finding AND every pending hypothesis, for every terminal - /// watching it, with no way back, and it fixes nothing — a cause that persists produces a new - /// fact later. + /// The question names the BLAST RADIUS, which is the only thing that changed when this action + /// stopped needing a hand-picked core: one core is named, and a scope states how many cores and + /// which. An operator who never opened the panel's selector is the normal case, and their press + /// must not be quieter than a deliberate one. /// - /// Local rows are deliberately NOT cleared on confirmation: the core's next full list is the - /// answer, and clearing optimistically would show a clean bill for a core that may have - /// rejected the command. + /// Local rows are deliberately NOT cleared on confirmation: each core's next full list is the + /// answer, and clearing optimistically would show a clean bill for a core that may have rejected + /// the command. /// /// Args: - /// core: Core whose diagnostics would be dropped. /// window: Window that owns the unique dialog. /// cx: View context used to build the dialog. /// /// Returns: /// Nothing; only Yes sends anything, and it closes the dialog either way. - pub(super) fn confirm_clear_problems( - &mut self, - core: CoreId, - window: &mut Window, - cx: &mut Context, - ) { - let question = t!( - "core_status.problems_clear_q", - core = self.core_display_name(core, cx) - ) - .to_string(); + pub(super) fn confirm_clear_problems(&mut self, window: &mut Window, cx: &mut Context) { + let cores = self.connected_in_scope(cx); + let names: Rc<[String]> = cores + .iter() + .map(|core| self.core_display_name(*core, cx)) + .collect(); + let (title, question) = match cores.as_slice() { + // Both fleet buttons are greyed with no targets, so this is the race where the last + // core dropped between the repaint and the press. It says so rather than swallowing + // the press, which is what the re-read does in the identical state. + [] => return self.no_fleet_targets(window, cx), + [core] => ( + t!("core_status.problems_clear_title").to_string(), + t!( + "core_status.problems_clear_q", + core = self.core_display_name(*core, cx) + ) + .to_string(), + ), + many => ( + t!("core_status.problems_clear_title_many").to_string(), + t!("core_status.problems_clear_q_many", cores = many.len()).to_string(), + ), + }; + let cores: Rc<[CoreId]> = cores.into(); let view = cx.entity().downgrade(); window.open_unique_moon_dialog( "core-status-clear-problems-confirm", cx, move |dialog, _window, cx| { let view = view.clone(); + // Refcount bumps, not copies: this builder is an `Fn` re-run on every frame the + // dialog is drawn, and the confirmed scope can be the whole fleet. + let cores = cores.clone(); problem_confirm_dialog( dialog, - t!("core_status.problems_clear_title").to_string(), + title.clone(), question.clone(), + names.clone(), "core-status-clear-problems", MoonButtonVariant::Danger, cx, move |window, cx| { if let Some(view) = view.upgrade() { - view.update(cx, |this, cx| this.send_clear_problems(core, window, cx)); + let cores = cores.clone(); + view.update(cx, |this, cx| { + this.send_clear_problems(&cores, window, cx) + }); } }, ) @@ -855,6 +880,208 @@ impl CoreStatusView { ); } + /// Re-read the retained diagnostics of every connected core in scope. + /// + /// Unconfirmed on purpose, and it is the only one of the three that can be: nothing leaves this + /// terminal and nothing on any core changes. See `CoreCmd::RefreshProblems` — the protocol has + /// no request for a diagnostics list, so this republishes what the library already holds. + /// + /// Args: + /// window: Window that shows the outcome. + /// cx: View context. + /// + /// Returns: + /// Nothing; the outcome is a notification. + pub(super) fn refresh_problems(&mut self, window: &mut Window, cx: &mut Context) { + let cores = self.connected_in_scope(cx); + self.fleet_send( + &cores, + window, + cx, + |session, live| session.refresh_core_problems_many(live), + |sent| { + MoonNotification::success( + t!("core_status.problems_refreshed", cores = sent).to_string(), + ) + }, + ); + } + + /// Send the reset to every core that is still up, or say why it did not go. + fn send_clear_problems( + &mut self, + cores: &[CoreId], + window: &mut Window, + cx: &mut Context, + ) { + // The one-core success still NAMES its core: that message is what an operator reads back + // when checking which core they just emptied, and a count cannot answer it. + let single = match cores { + [core] => Some(self.core_display_name(*core, cx)), + _ => None, + }; + self.fleet_send( + cores, + window, + cx, + |session, live| session.clear_core_problems_many(live), + move |sent| match single { + Some(name) => MoonNotification::success( + t!("core_status.problems_clear_sent", core = name).to_string(), + ), + None => MoonNotification::success( + t!("core_status.problems_clear_sent_many", cores = sent).to_string(), + ), + }, + ); + } + + /// Run one fleet diagnostics command and report what it actually reached. + /// + /// Both actions are the same four steps — drop the cores that went down, send one command per + /// core, count the acceptances, tell the operator — so they share them rather than each + /// spelling them out and drifting. + /// + /// Readiness is re-read HERE rather than trusted from the button or the confirm that started + /// the action: either can sit open while a core drops, and a command queued for a core that is + /// down waits on its channel and fires on reconnect. + /// + /// The shortfall is the point. A core that went down between the press and the send, and one + /// whose command channel is already gone, mean the same thing to the operator: the action did + /// NOT reach everything it named. Which of the two it was reaches the log. A bare success toast + /// over a partial result is precisely the "failure nobody sees" this surface's rule forbids. + /// + /// Args: + /// cores: Cores the press addressed. + /// window: Window that shows the outcome. + /// cx: View context. + /// send: Issues the command to the still-connected cores, returning those that accepted. + /// whole: Builds this action's own success message when every named core accepted. + /// + /// Returns: + /// Nothing; the outcome is a notification. + fn fleet_send( + &mut self, + cores: &[CoreId], + window: &mut Window, + cx: &mut Context, + send: impl FnOnce(&moon_core::session::SessionManager, &[CoreId]) -> Vec, + whole: impl FnOnce(usize) -> MoonNotification, + ) { + let asked = cores.len(); + if asked == 0 { + return self.no_fleet_targets(window, cx); + } + let live = self.still_connected(cores, cx); + let sent = send(&self.backend.read(cx).session, &live).len(); + // The two failure messages are shared and the success is the caller's: "it did not reach + // everything" means the same for either action, while "the reset went" and "the list was + // re-read" are different facts an operator acts on differently. + let note = if sent == 0 { + log::warn!("core status: a fleet diagnostics action reached none of {asked} cores"); + MoonNotification::warning( + t!("core_status.problems_fleet_none", cores = asked).to_string(), + ) + } else if sent < asked { + log::warn!("core status: a fleet diagnostics action reached {sent} of {asked} cores"); + MoonNotification::warning( + t!( + "core_status.problems_fleet_partial", + sent = sent, + cores = asked + ) + .to_string(), + ) + } else { + whole(sent) + }; + window.push_notification(note, cx); + } + + /// Say that a fleet action found nothing to act on, in the one wording both of them use. + /// + /// The greyed button already carries this reason as its tooltip; this is the same sentence for + /// the race where the last core drops between the repaint that enabled the button and the press. + fn no_fleet_targets(&self, window: &mut Window, cx: &mut Context) { + // Which emptiness it was: a scope full of live cores must not be reported as offline just + // because the ONE core the operator clicked is down. Same split as `fleet_refusal`'s, and + // for the same reason — the two have different remedies. + let key = match self.problems_picked { + Some(_) => "core_status.problems_picked_offline", + None => "core_status.problems_no_online", + }; + window.push_notification(MoonNotification::warning(t!(key).to_string()), cx); + } + + /// Narrow the three Problems actions to one core, or widen them back to the whole scope. + /// + /// A TOGGLE because the narrowing has to be reversible: a click that can reach "just this core" + /// and never get back would strand the operator on one core, and the table's own handling only + /// ever sets a row. Clicking any row of the already-picked core clears the pick — including a + /// different finding of the same core, which is the same answer to the same question. + /// + /// Stored as the CORE, never as the clicked row index. The finding list is rebuilt from live + /// core data on every repaint, so an index outlives the row it named: one finding appearing or + /// clearing above it re-points it at another core, and the action it narrows cannot be undone. + /// + /// Args: + /// core: Core owning the clicked finding. + /// cx: View context used to repaint. + /// + /// Returns: + /// Nothing; the next frame draws that core's rows selected. + pub(super) fn pick_problem_core(&mut self, core: CoreId, cx: &mut Context) { + self.problems_picked = (self.problems_picked != Some(core)).then_some(core); + // The table's own row and cell cursors are dropped on EVERY pick change, set or cleared, so + // the surface carries exactly one highlight. Left behind they draw a second one from a raw + // row INDEX (`data_table.rs:1231` ORs it into the row, and the cell background at :1262 is + // painted whether or not cells are selectable) — and an index is precisely what this pick + // exists to avoid, because the finding list is rebuilt every repaint and the index then + // marks a different core's row. `select_row(None)` alone is not enough: it leaves + // `selected_cell` behind (`data_table.rs:345`), which paints a ghost on the last clicked + // cell after the pick is gone. + self.problems_table_state.update(cx, |state, cx| { + state.selected_row = None; + state.selected_column = None; + state.selected_cell = None; + cx.notify(); + }); + cx.notify(); + } + + /// What a Problems-mode fleet action acts on: the clicked finding's core, else the whole scope. + /// + /// The panel's own rule for a bulk command — the selection when the operator has made one, the + /// displayed scope otherwise (`selected_or_visible`) — applied to the one gesture this mode + /// offers. Problems draws findings rather than cores, so `core_selection` can never be set + /// here; the table's own selected row is its equivalent, and the render arm resolves it to a + /// core in [`CoreStatusView::problems_picked`]. + /// + /// The list is derived at PRESS time rather than carried on the rendered scope: the render arm + /// would otherwise build it on every repaint for something only a click reads, and the click + /// has to re-check reachability at send time regardless. + fn connected_in_scope(&self, cx: &App) -> Vec { + let scope = self.effective_scope(self.backend.read(cx)).ids().to_vec(); + // The pick is intersected with the scope rather than trusted: the panel's selector or its + // workspace can drop a core out from under a pick that was made before either moved. + let ids = match self.problems_picked { + Some(core) if scope.contains(&core) => vec![core], + Some(_) => Vec::new(), + None => scope, + }; + self.still_connected(&ids, cx) + } + + /// Keep only the cores that are connected right now. + fn still_connected(&self, cores: &[CoreId], cx: &App) -> Vec { + let session = &self.backend.read(cx).session; + cores + .iter() + .copied() + .filter(|core| session.core_run_state(*core).online) + .collect() + } + /// Send the test, or say why it did not go. fn send_problem_test(&mut self, core: CoreId, window: &mut Window, cx: &mut Context) { let name = self.core_display_name(core, cx); @@ -887,37 +1114,6 @@ impl CoreStatusView { } } - /// Send the clear, or say why it did not go. - /// - /// The readiness check is repeated HERE rather than trusted from the button that opened the - /// dialog: that dialog can sit open while the core drops, and a queued clear would then fire on - /// reconnect against findings the operator never saw. - fn send_clear_problems(&mut self, core: CoreId, window: &mut Window, cx: &mut Context) { - let name = self.core_display_name(core, cx); - if !self.core_is_ready(core, cx) { - window.push_notification( - MoonNotification::warning(t!("core_status.problems_not_sent_offline").to_string()), - cx, - ); - return; - } - match self.backend.read(cx).session.clear_core_problems(core) { - Ok(()) => window.push_notification( - MoonNotification::success( - t!("core_status.problems_clear_sent", core = name).to_string(), - ), - cx, - ), - Err(error) => { - log::warn!("core status: clear problems for core {core} not sent: {error:#}"); - window.push_notification( - MoonNotification::warning(t!("core_status.problems_not_sent").to_string()), - cx, - ); - } - } - } - /// Open the ONE confirm every footer bulk update gets, naming the core and lane counts /// before the press that fills the per-IP queue. /// @@ -1150,6 +1346,7 @@ fn problem_confirm_dialog( dialog: moon_ui::MoonDialog, title: String, question: String, + names: Rc<[String]>, id_prefix: &'static str, confirm: MoonButtonVariant, cx: &App, @@ -1177,13 +1374,24 @@ fn problem_confirm_dialog( .content(move |content, _window, cx| { let p = MoonPalette::active(cx); content.child( - div() - // MIXED NODE: both questions that reach this dialog weld a CORE NAME into the - // sentence, and a core name is shown verbatim and identically everywhere. - .font_family(design::mono()) - .text_size(design::t_body(cx)) - .text_color(rgb(p.text)) - .child(question.clone()), + v_flex() + .w_full() + .gap_2() + .child( + div() + // MIXED NODE: the one-core questions weld a CORE NAME into the + // sentence, and a core name is shown verbatim and identically + // everywhere. + .font_family(design::mono()) + .text_size(design::t_body(cx)) + .text_color(rgb(p.text)) + .child(question.clone()), + ) + // The same non-truncating list the footer's fleet confirm and the row menu + // draw, from the same helper: dialogs describing one scope must not be able to + // word it differently, and a core name is never shortened. It renders nothing + // for a single core, which the question above has already named. + .children(update_menu::scope_name_list(&names, p, cx)), ) }) .footer( diff --git a/crates/moon-ui-gpui/src/panels/core_status/mod.rs b/crates/moon-ui-gpui/src/panels/core_status/mod.rs index 3ff353aa..212d142d 100644 --- a/crates/moon-ui-gpui/src/panels/core_status/mod.rs +++ b/crates/moon-ui-gpui/src/panels/core_status/mod.rs @@ -310,6 +310,14 @@ pub struct CoreStatusView { /// headings as lines of their own and both presentations re-sort under the user. /// Pruned against the visible rows on every cache rebuild (`cache::rebuild_cache`). core_selection: RowSelection, + /// Core the three Problems actions are narrowed to, or `None` for the panel's whole scope. + /// + /// Written by the row click, which resolves its index against the list it was drawn from while + /// that list is still the one on screen. Held as a CORE for the same reason: the finding list + /// is rebuilt from live core data on every repaint, so a stored index outlives the row it + /// named — one finding appearing or clearing above it re-points it at a different core, and + /// the reset it narrows cannot be undone. + problems_picked: Option, dock: Option>, focus: FocusHandle, } @@ -475,6 +483,7 @@ impl CoreStatusView { group, sel_cores: HashSet::new(), core_selection: RowSelection::default(), + problems_picked: None, last_repaint_ms: 0, last_update_rev: 0, last_history_rev: 0, @@ -926,30 +935,13 @@ impl Render for CoreStatusView { // Everything the arm needs is collected inside this block so the backend borrow // ENDS before the read-mark, which needs `&mut self`. The alternative — marking up // in `render` — is what let the cap consume rows the surface never drew. - let (rows, silent, truncated, core_names, cores, gate, answered, zone) = { + let (rows, core_names, scope, picked, zone) = { let b = self.backend.read(cx); - let scope = self.effective_scope(b); + let effective = self.effective_scope(b); // Scope order, then the core's own listing order inside each core. Neither is // re-sorted: the core chose the order of its findings, and inventing another one // here would present a ranking the core never made. - let scope_ids = scope.ids(); - // Exactly one EXPLICITLY selected core, and only if the resolved scope still holds - // it — a stale selection must not address a core this panel no longer covers. - let chosen = match self.sel_cores.len() { - 1 => self - .sel_cores - .iter() - .copied() - .next() - .filter(|core| scope_ids.contains(core)), - _ => None, - }; - let ready = |core: CoreId| { - matches!( - b.session.store().core(core).map(|data| data.status.clone()), - Some(moon_core::feed::ConnStatus::Ready) - ) - }; + let scope_ids = effective.ids(); let core_names: HashMap = b .config .servers @@ -958,30 +950,48 @@ impl Render for CoreStatusView { .collect(); let mut rows: Vec = Vec::new(); let mut silent: Vec = Vec::new(); + // `matched` counts the cores in scope whatever their connection, `targets` only + // the reachable ones: the command channel outlives a disconnect, so a command + // queued for a core that is down waits there and fires on reconnect — for the + // reset, against findings gathered during the outage that nobody ever saw. The + // difference is what lets a shut gate say "the core you picked is down" rather + // than "pick one". + let mut matched = 0usize; + let mut matched_only = None; + let mut targets = 0usize; + // The same dash the table uses for an unnamed core, so one core cannot appear + // under two different spellings on the one surface. + let name_of = |core: CoreId| { + core_names + .get(&core) + .cloned() + .unwrap_or_else(|| "—".to_string()) + }; for core in scope_ids.iter().copied() { - // A core with no retained state at all has said nothing, which is the same - // "not known" case as one that answered without support: named, never clean. - let supported = b - .session - .store() - .core(core) - .is_some_and(|data| data.problems.supported); - if !supported { - // The same dash the table uses for an unnamed core, so one core cannot - // appear under two different spellings on the one surface. - silent.push( - core_names - .get(&core) - .cloned() - .unwrap_or_else(|| "—".to_string()), - ); - continue; - } - // Not looked up twice: `supported` above proves the entry exists, and this is - // the same borrow rather than a second search. + // Counted for EVERY core in scope, before anything can skip the rest of the + // body. A core with no store entry is still a core this panel covers, and + // counting only the ones that have connected would let a twenty-six-core + // scope report "exactly one core" and open the test on a core nobody chose. + matched += 1; + matched_only = (matched == 1).then_some(core); + // ONE lookup per core: this loop runs on every repaint of the arm, and the + // store search is the expensive half of it. let Some(data) = b.session.store().core(core) else { + // No retained state at all is the same "not known" case as an answer + // without support: named, never read as clean. + silent.push(name_of(core)); continue; }; + // A core that has never delivered a list is still one all three actions + // must reach — see `ProblemsScope::targets` for why `supported` does not + // narrow this. + if moon_core::session::CoreRunState::from_core(data).online { + targets += 1; + } + if !data.problems.supported { + silent.push(name_of(core)); + continue; + } rows.extend(data.problems.items.iter().map(|problem| { problems::ProblemRow { core, @@ -994,30 +1004,63 @@ impl Render for CoreStatusView { // than silently shortened, for the same reason a silent core is. let truncated = rows.len() > problems::PROBLEM_LIST_LIMIT; rows.truncate(problems::PROBLEM_LIST_LIMIT); - ( - rows, + // A CLICKED FINDING narrows all three actions to its core, and that pick is + // the operator's own — set by the click, held as a CORE, never re-derived from + // a row index into a list this arm rebuilds every repaint. No pick keeps the + // whole scope, which is this panel's own rule for a bulk command + // (`selected_or_visible`). + // + // Dropped as soon as the surface can no longer SHOW it. Clicking a row of its + // core is the only gesture that clears a pick, so a pick whose findings have + // all gone — reset on the core, or its `supported` flipped back to unknown — + // would sit there narrowing an irreversible action with nothing on screen + // saying so and no way to undo it. Losing it widens the actions back to the + // scope instead, which the tooltip counts and the confirm names core by core. + // Checked against the DRAWN rows, so the cap cannot keep a pick alive that the + // operator cannot reach either. + let picked = self + .problems_picked + .filter(|core| rows.iter().any(|row| row.core == *core)); + // One store lookup, only on the frames where a pick is live. + let (matched, matched_only, targets) = match picked { + None => (matched, matched_only, targets), + Some(core) => { + let online = b.session.store().core(core).is_some_and(|d| { + moon_core::session::CoreRunState::from_core(d).online + }); + (1, Some(core), usize::from(online)) + } + }; + let scope = problems::ProblemsScope { + cores: scope_ids.len(), silent, truncated, - core_names, - scope_ids.len(), - // The gate reads the operator's OWN selection, not the resolved scope: a - // one-core group under "All", or a pinned Auto workspace, both resolve to a - // single id nobody picked, and an irreversible action must not ride on that - // coincidence. `sel_cores` is the explicit Classic pick and nothing else. - match chosen { - None => problems::ActionGate::NoSingleChoice, - Some(core) if !ready(core) => problems::ActionGate::NotConnected, - Some(core) => problems::ActionGate::Ready(core), + // The test needs ONE core and says which reason it lacks: nothing narrowed + // to one, or the one it has is down. + actions: match (matched, matched_only, targets) { + (1, Some(core), 1) => problems::ActionGate::Ready(core), + (1, Some(_), _) => problems::ActionGate::NotConnected, + _ => problems::ActionGate::NoSingleChoice, }, - chosen.is_some_and(|core| { - b.session - .store() - .core(core) - .is_some_and(|data| data.problems.supported) - }), + // A live pick answers for ITSELF, so a scope that still holds connected + // cores must not be reported as offline just because the picked one is — + // that is the conflation the two refusals exist to keep apart. + picked: picked.is_some(), + targets, + }; + ( + rows, + core_names, + scope, + picked, crate::chrome::clock::resolved_header_clock_zone(b.header_clock_zone()), ) }; + // A pick whose core left the scope is dropped for good, not just for this frame: + // left behind, it would silently re-arm the moment that core came back. + if self.problems_picked != picked { + self.problems_picked = picked; + } // Drawing the findings IS looking at them — the News panel's rule, and for its // reason. The window-active guard is what stops the badge being consumed unseen: an // inactive window still repaints on the shell's clock tick, and "the tab was in @@ -1029,13 +1072,8 @@ impl Render for CoreStatusView { "core-status-problems", Rc::new(rows), Rc::new(core_names), - &problems::ProblemsScope { - cores, - silent, - truncated, - actions: gate, - answered, - }, + &scope, + picked, &self.problems_table_state, zone, cx, diff --git a/crates/moon-ui-gpui/src/panels/core_status/problems.rs b/crates/moon-ui-gpui/src/panels/core_status/problems.rs index bbecd7f2..4a84f031 100644 --- a/crates/moon-ui-gpui/src/panels/core_status/problems.rs +++ b/crates/moon-ui-gpui/src/panels/core_status/problems.rs @@ -43,27 +43,48 @@ pub(super) struct ProblemsScope { pub(super) silent: Vec, /// Whether the row list was cut by [`PROBLEM_LIST_LIMIT`]. pub(super) truncated: bool, - /// Why the per-core actions are or are not available. + /// Why the single-core channel test is or is not available. pub(super) actions: ActionGate, - /// Whether the chosen core has ever delivered a list — the clear has nothing to do otherwise. - pub(super) answered: bool, + /// Whether a clicked finding has narrowed the two fleet actions to its core. + /// + /// It changes what an empty `targets` MEANS, which is the whole reason it is carried: with no + /// pick it says the scope has nothing connected, with a pick it says the ONE core the operator + /// aimed at is down while the rest of the scope may be perfectly reachable. + pub(super) picked: bool, + /// How many cores in scope are connected — what both fleet actions would address. + /// + /// A COUNT, not the list: the tooltips are all this view needs it for, and the press re-derives + /// the cores from the panel's live scope through `CoreStatusView::connected_in_scope`. Carrying + /// the list here would allocate it on every repaint for something only a click reads, and the + /// click would then have to re-check it anyway. + /// + /// Deliberately NOT narrowed to the cores that have delivered a list. `supported == false` means + /// "this core has never demonstrably answered", which is not "this core holds nothing" — it + /// covers a core whose first list is still in flight, and clearing also drops PENDING + /// hypotheses the terminal has never seen. Greying the reset out there would claim knowledge + /// this surface does not have, which is the same conflation its notice exists to prevent. + pub(super) targets: usize, } -/// Whether the two per-core diagnostic actions can be offered, and if not, why. +/// Whether the channel test can be offered, and if not, why. +/// +/// It guards the TEST alone. The test is the one action with no fleet-wide form worth having — +/// publishing a `test` fact on two hundred cores litters two hundred cores — so it needs one core +/// named, while the reset and the re-read address the panel's whole scope and read no gate at all. /// -/// A named reason rather than a bare `Option`, because an irreversible action must not be -/// enabled by an ACCIDENT of scope. "The scope happens to hold one core" is not "the operator chose -/// this core": a one-core group under Classic All, or a pinned Auto workspace, both resolve to a -/// single id nobody picked. Each refusal also has its own remedy, and a single greyed button with -/// one generic tooltip cannot say which one applies. +/// A named reason rather than a bare `Option`, because each refusal has its own remedy and +/// a single greyed button with one generic tooltip cannot say which one applies. #[derive(Clone, Copy, PartialEq, Eq)] pub(super) enum ActionGate { - /// The operator explicitly selected exactly this core, and it is connected. + /// Exactly one core is in play and it is connected — either the operator clicked one of its + /// findings, or the panel's scope holds nothing else. Both are answers to "which core", and + /// both are named in the confirm the test opens before it publishes anything. Ready(CoreId), - /// The selection does not name exactly one core. + /// Nothing narrowed the surface to a single core. NoSingleChoice, - /// One core is chosen, but it is not connected: the command channel would accept the command - /// and hold it until the core returns — destroying findings gathered during the outage. + /// One core is chosen, but it is not connected: the command channel would accept the test and + /// hold it until the core returns, publishing the `test` fact at some unannounced later moment + /// on a core the operator has long stopped looking at. NotConnected, } @@ -81,6 +102,9 @@ impl ActionGate { match self { Self::Ready(_) => None, Self::NoSingleChoice => Some(t!("core_status.problems_pick_core").to_string()), + // Deliberately NOT the fleet actions' offline string. That one reports a scope with + // nothing connected in it; this one reports that the one core the operator is pointing + // at is down, which is a different fact with a different remedy. Self::NotConnected => Some(t!("core_status.problems_core_offline").to_string()), } } @@ -201,6 +225,7 @@ pub(super) const PROBLEM_LIST_LIMIT: usize = 500; /// rows: Findings in display order, already scoped and capped by the caller. /// core_names: Core display name per core id. /// scope: What the scope could and could not report. +/// picked: Core the three actions are narrowed to, whose every row is drawn selected. /// state: Persisted table interaction state. /// zone: User-selected display time zone. /// cx: Panel context. @@ -212,6 +237,7 @@ pub(super) fn problems_view( rows: Rc>, core_names: Rc>, scope: &ProblemsScope, + picked: Option, state: &Entity, zone: chrono_tz::Tz, cx: &Context, @@ -230,33 +256,63 @@ pub(super) fn problems_view( empty_text(scope), p, cx, - MoonDataTable::new(id, row_count, move |ix, _window, _app| { - problem_row(ix, &rows[ix], &core_names, zone) + MoonDataTable::new(id, row_count, { + let rows = Rc::clone(&rows); + move |ix, _window, _app| problem_row(&rows[ix], &core_names, picked, zone) }) .columns(columns()) + // Deliberately NOT `controlled_row_selection`: that mode makes the table's own + // `selected_row` stop driving the highlight AND returns early from its whole keyboard + // block (`data_table.rs:1100`), so taking the click that way costs up/down/home/end + // navigation for a table that is read far more than it is clicked. .state(state) + // Which CORE the click narrowed to, resolved HERE from the list this frame drew — the + // handler must not hand a row INDEX to the panel. The list is rebuilt from live core + // data every repaint, so a finding appearing or clearing on an earlier core re-points + // any stored index at a different core, and the action it narrows is irreversible. + .on_select_row({ + let view = cx.entity().downgrade(); + let rows = Rc::clone(&rows); + move |ix, _window, cx| { + let (Some(view), Some(row)) = (view.upgrade(), rows.get(ix)) else { + return; + }; + let core = row.core; + view.update(cx, |this, cx| this.pick_problem_core(core, cx)); + } + }) .header_height(design::TABLE_HEAD_H) .row_height(design::TABLE_ROW_H) .style(design::table_style(p)), )) } -/// The two per-core diagnostic actions, above the notice. +/// The three diagnostic actions, above the notice — MoonBot's own row, plus the channel test. +/// +/// MoonBot's Problems window carries "reset all" and "refresh"; this row carries both, and the test +/// beside them. The test and the reset ship as a PAIR because the protocol makes them one: a test +/// publishes a `test` fact that stays on the core until something clears it, and the reset is the +/// only thing that does. +/// +/// That is also why BOTH of those are confirmed, not just the destructive one. The test looks +/// harmless and is not: the row it leaves can be removed only by the irreversible reset, so an +/// unconfirmed test press can force an operator to destroy a core's real findings to tidy up after +/// it. The dialog says that in as many words. /// -/// They ship as a PAIR because the protocol makes them one: a test publishes a `test` fact that -/// stays on the core until something clears it, and clearing is the only thing that does. +/// The re-read is deliberately NOT confirmed, and cannot be: nothing leaves this terminal and +/// nothing on any core changes — see `CoreCmd::RefreshProblems` for why the wire has no request to +/// send. Its tooltip states that outright rather than letting the label imply a round trip. /// -/// That is also why BOTH are confirmed, not just the destructive one. The test looks harmless and -/// is not: the row it leaves can be removed only by the irreversible clear, so an unconfirmed test -/// press can force an operator to destroy a core's real findings to tidy up after it. The dialog -/// says that in as many words. +/// SCOPE, not selection, for the two fleet actions: an operator who has picked no core sees the +/// whole scope in the table, and a button above that table acts on what the table shows. Both name +/// their blast radius before they fire — the reset in its dialog, the re-read in its tooltip. /// /// Feedback is a MoonUI notification raised by the action itself, not a line invented here: an /// action whose failure only reaches the log is an action whose failure nobody sees, and the stack /// already has the control for saying so. /// /// Args: -/// scope: What the scope covers, including the gate for these actions. +/// scope: What the scope covers, including each action's targets. /// p: Active palette. /// cx: Panel context. /// @@ -267,43 +323,9 @@ fn actions( p: MoonPalette, cx: &Context, ) -> impl IntoElement { - let gate = scope.actions; - let refusal = gate.refusal(); - // The clear is additionally pointless on a core that has never reported: there is nothing of - // ours to drop, and the press would still be irreversible on the core's own pending state. - let clear_off = gate.core().is_none() || !scope.answered; - let clear_refusal = match (&refusal, scope.answered) { - (Some(reason), _) => Some(reason.clone()), - (None, false) => Some(t!("core_status.problems_clear_nothing").to_string()), - (None, true) => None, - }; - let test_view = cx.entity().downgrade(); - let clear_view = cx.entity().downgrade(); - let test_core = gate.core(); - let clear_core = gate.core(); - - let test = MoonButton::new("core-status-problems-test") - .label(t!("core_status.problems_test").to_string()) - .size(MoonButtonSize::Micro) - .variant(MoonButtonVariant::Panel) - .disabled(test_core.is_none()) - .on_click(move |_, window, cx| { - let (Some(view), Some(core)) = (test_view.upgrade(), test_core) else { - return; - }; - view.update(cx, |this, cx| this.confirm_problem_test(core, window, cx)); - }); - let clear = MoonButton::new("core-status-problems-clear") - .label(t!("core_status.problems_clear").to_string()) - .size(MoonButtonSize::Micro) - .variant(MoonButtonVariant::Panel) - .disabled(clear_off) - .on_click(move |_, window, cx| { - let (Some(view), Some(core)) = (clear_view.upgrade(), clear_core) else { - return; - }; - view.update(cx, |this, cx| this.confirm_clear_problems(core, window, cx)); - }); + let test_core = scope.actions.core(); + let fleet = fleet_refusal(scope.cores, scope.picked, scope.targets); + let view = cx.entity().downgrade(); h_flex() .w_full() @@ -315,20 +337,145 @@ fn actions( .justify_end() .border_b_1() .border_color(rgb(p.border)) - .child(match &refusal { - Some(reason) => test.tooltip(reason.clone()).render(), - None => test.render(), - }) - .child(match &clear_refusal { - Some(reason) => clear.tooltip(reason.clone()).render(), - None => clear.render(), + .child(action_button( + "core-status-problems-test", + t!("core_status.problems_test").to_string(), + scope.actions.refusal(), + t!("core_status.problems_test_tip").to_string(), + view.clone(), + move |this, window, cx| { + if let Some(core) = test_core { + this.confirm_problem_test(core, window, cx); + } + }, + )) + // Tipped even when live, because the label promises a round trip the wire cannot make. + .child(action_button( + "core-status-problems-refresh", + t!("core_status.problems_refresh").to_string(), + fleet.clone(), + fleet_tip( + "core_status.problems_refresh_tip", + "core_status.problems_refresh_tip_one", + scope, + ), + view.clone(), + CoreStatusView::refresh_problems, + )) + // Tipped when live too: this one states its blast radius BEFORE the dialog, because the + // number of cores is the whole difference between a tidy-up and a fleet-wide loss. + .child(action_button( + "core-status-problems-clear", + t!("core_status.problems_clear").to_string(), + fleet, + fleet_tip( + "core_status.problems_clear_tip", + "core_status.problems_clear_tip_one", + scope, + ), + view, + CoreStatusView::confirm_clear_problems, + )) +} + +/// One of the action row's three buttons: same metrics, same refusal-or-tip rule. +/// +/// One builder rather than three chains, following `panels::common::micro_button`'s own note — +/// two builders spelling the same metrics drift apart the moment either is touched. The tooltip is +/// unconditional here because all three buttons say something worth reading when they are live: +/// the refusal when there is one, otherwise what the press would actually do. +/// +/// Args: +/// id: Stable element identity. +/// label: Button caption. +/// refusal: Why the action is unavailable, which also disables the button. +/// tip: What the live action would do, shown when there is no refusal. +/// view: Panel to act on, dropped-safe. +/// press: What the press runs on the panel. +/// +/// Returns: +/// The rendered button. +fn action_button( + id: &'static str, + label: String, + refusal: Option, + tip: String, + view: WeakEntity, + press: impl Fn(&mut CoreStatusView, &mut Window, &mut Context) + 'static, +) -> impl IntoElement { + MoonButton::new(id) + .label(label) + .size(MoonButtonSize::Micro) + .variant(MoonButtonVariant::Panel) + .disabled(refusal.is_some()) + .tooltip(refusal.unwrap_or(tip)) + .on_click(move |_, window, cx| { + let Some(view) = view.upgrade() else { + return; + }; + view.update(cx, |this, cx| press(this, window, cx)); }) + .render() +} + +/// What a live fleet action would do, said in the form that matches what it is aimed at. +/// +/// A pick makes `targets` the PICKED core's own 0-or-1, so the scope-wide wording would state a +/// falsehood about the panel ("connected cores: 1" for a scope of twenty-six) and would go on +/// advising a click that, in that state, cancels the pick rather than making one. +/// +/// Args: +/// scoped_key: Wording for the whole-scope form, taking the connected count. +/// picked_key: Wording for the one-core form. +/// scope: What the surface knows, including whether a pick is live. +/// +/// Returns: +/// The tooltip text for the live action. +fn fleet_tip(scoped_key: &str, picked_key: &str, scope: &ProblemsScope) -> String { + match scope.picked { + true => t!(picked_key).to_string(), + false => t!(scoped_key, cores = scope.targets).to_string(), + } +} + +/// Why the two fleet actions are refused, decided apart from how they are drawn. +/// +/// Split from [`actions`] for the same reason [`notice_text`] is split from [`notice`]: a greyed +/// button whose reason lives only in the render reads as a bug in the terminal. +/// +/// The two refusals it answers are NOT the same fact, and the empty scope is the one the surface +/// would otherwise contradict itself about: its notice already says the scope holds no cores, so a +/// tooltip claiming "none is connected" would give the same emptiness two different explanations. +/// +/// Args: +/// cores: How many cores the panel's scope covers at all. +/// targets: How many of those are connected. +/// +/// Returns: +/// The shared refusal, or `None` where both actions are live. +fn fleet_refusal(cores: usize, picked: bool, targets: usize) -> Option { + if cores == 0 { + return Some(t!("core_status.problems_no_cores").to_string()); + } + if targets > 0 { + return None; + } + // ONE string per fact, shared by both actions: the reason is the connection rather than the + // action, and two spellings of one fact read as two different problems. But "the core you + // clicked is down" and "nothing in this scope is up" are two different facts with two + // different remedies, and a scope full of live cores must never be reported as offline + // because the one picked core is not. + Some(match picked { + true => t!("core_status.problems_picked_offline").to_string(), + false => t!("core_status.problems_no_online").to_string(), + }) } /// What the notice must say, decided apart from how it is drawn. /// -/// Split from [`notice`] so the rule can be asserted without a render context: this is the one -/// decision on the surface that must never silently become "everything is fine". +/// Split from [`notice`] so the rule can be asserted without a render context, like +/// [`fleet_refusal`] beside it: a caveat that quietly becomes "everything is fine" is the one +/// failure this whole surface exists to prevent. /// /// Args: /// scope: What the scope could and could not report. @@ -418,18 +565,23 @@ fn empty_text(scope: &ProblemsScope) -> String { /// established, and `first_seen` is when it began suspecting. Falling back to `first_seen` when the /// core sent no confirmation time keeps a row timed rather than blank. /// +/// A row belonging to the PICKED core is drawn selected, and that highlight is the only thing on +/// the surface saying which core the three actions were narrowed to. It is keyed on the core rather +/// than on the table's own selected index because the finding list is rebuilt from live core data +/// on every repaint: an index survives a list that moved, pointing at a different core. +/// /// Args: -/// ix: Row index, for a stable element id on the tooltip host. /// row: Finding and its reporting core. /// core_names: Display names keyed by core id. +/// picked: Core the three actions are narrowed to, if any. /// zone: Selected IANA display zone. /// /// Returns: /// Complete problems table row. fn problem_row( - ix: usize, row: &ProblemRow, core_names: &HashMap, + picked: Option, zone: chrono_tz::Tz, ) -> MoonDataRow { let core = core_names @@ -455,7 +607,13 @@ fn problem_row( // tooltip repeats nothing and adds what the row could not fit. MoonDataCell::element( div() - .id(SharedString::from(format!("cs-problem-{ix}"))) + // Identity from the FINDING rather than a row index. The enclosing cell's id + // already carries the index, so this never collided; keying it on the finding + // simply means the hover state follows the row's content when the list moves. + .id(SharedString::from(format!( + "cs-problem-{}-{}", + row.core, row.problem.kind_name + ))) // Fills the cell rather than the text: the column was widened so the hover could // carry the body, and a content-sized host leaves most of that width dead. .w_full() @@ -465,6 +623,7 @@ fn problem_row( }), ), ]) + .selected(picked == Some(row.core)) } /// A formatted civil minute, or `None` when the value is absent or outside the printable range. diff --git a/crates/moon-ui-gpui/src/panels/core_status/problems/tests.rs b/crates/moon-ui-gpui/src/panels/core_status/problems/tests.rs index f42056cd..7967eb50 100644 --- a/crates/moon-ui-gpui/src/panels/core_status/problems/tests.rs +++ b/crates/moon-ui-gpui/src/panels/core_status/problems/tests.rs @@ -6,7 +6,7 @@ use rust_i18n::t; use super::{ ActionGate, ProblemRow, ProblemsScope, category_label, details_text, drawn_kinds, empty_text, - mark_signature, notice_text, + fleet_refusal, mark_signature, notice_text, }; /// A scope with nothing truncated, which is every case but the cap's own test. @@ -16,7 +16,8 @@ fn scope(cores: usize, silent: usize) -> ProblemsScope { silent: (0..silent).map(|i| format!("core-{i}")).collect(), truncated: false, actions: ActionGate::NoSingleChoice, - answered: true, + picked: false, + targets: 0, } } @@ -100,7 +101,8 @@ fn a_truncated_list_is_stated_even_when_every_core_answered() { silent: Vec::new(), truncated: true, actions: ActionGate::NoSingleChoice, - answered: true, + picked: false, + targets: 0, }; let text = notice_text(&full).expect("a cut list is stated"); assert!( @@ -165,20 +167,20 @@ fn the_silent_core_hover_names_them_without_outgrowing_the_window() { ); } -/// An irreversible action must never ride on a coincidence of scope. +/// The channel test addresses ONE core, and says which reason keeps it shut. /// -/// Regression target: the gate started as "the scope resolved to one core", which is true for a -/// one-core group under "All cores" and for a pinned Auto workspace — a core nobody picked. The -/// clear destroys a core's confirmed findings for every terminal watching it, so the difference -/// between "the operator chose this core" and "only one was left" is the whole safety margin. +/// The gate guards the test alone. It used to guard the reset as well, on the argument that an +/// irreversible action must not ride on a coincidence of scope — and that argument still holds; it +/// simply moved. The reset no longer reads this gate at all: it carries its own list of targets and +/// names them in its confirm, so nothing about it is decided by which core happened to be left. #[test] -fn the_actions_open_only_for_an_explicitly_chosen_connected_core() { +fn the_channel_test_opens_only_for_one_connected_core() { assert_eq!(ActionGate::Ready(7).core(), Some(7)); assert_eq!(ActionGate::NoSingleChoice.core(), None); assert_eq!( ActionGate::NotConnected.core(), None, - "a queued command would fire on reconnect against findings from the outage" + "a queued test would fire on reconnect and publish its fact unannounced" ); // Each refusal explains itself: one greyed button with one generic tooltip cannot say which of @@ -196,6 +198,48 @@ fn the_actions_open_only_for_an_explicitly_chosen_connected_core() { ); } +/// The fleet actions refuse for their own reasons, and never quietly. +/// +/// Regression target for the shape this replaced: one gate refused BOTH actions whenever the +/// operator had not hand-picked a core, which in a workspace-owned panel is permanent — the +/// selector is pinned there and the pick can never be made. A live button with no targets and a +/// greyed button with no reason are the same defect from opposite sides. +#[test] +fn the_fleet_actions_state_their_own_refusals() { + let no_cores = fleet_refusal(0, false, 0).expect("an empty scope refuses"); + assert_eq!( + no_cores, + t!("core_status.problems_no_cores").to_string(), + "an empty scope is explained the way the notice above the table already explains it" + ); + + let none_up = fleet_refusal(4, false, 0).expect("a scope with nothing connected refuses"); + assert_ne!( + none_up, no_cores, + "covering no cores and covering four that are down are different facts" + ); + + // The regression this argument exists for: a scope of live cores must never be reported as + // offline because the ONE core the operator clicked happens to be down. Same numbers, and the + // two states must still not read alike. + let picked_down = fleet_refusal(4, true, 0).expect("a picked core that is down refuses"); + assert_ne!( + picked_down, none_up, + "the core you clicked being down is not the scope having nothing up" + ); + + assert_eq!( + fleet_refusal(4, false, 1), + None, + "one connected core in scope opens both actions" + ); + assert_eq!( + fleet_refusal(1, true, 1), + None, + "a core that has never delivered a list still holds pending hypotheses to drop" + ); +} + /// What counts as looked at is taken from the ROWS on screen, per core. /// /// Identity, not a timestamp: the stamps come off the wire unbounded and from each core's own diff --git a/locales/core_status.yml b/locales/core_status.yml index 6a64b7f7..39ed3fa3 100644 --- a/locales/core_status.yml +++ b/locales/core_status.yml @@ -918,51 +918,115 @@ core_status.problem_cat.other: en: "Other" es: "Otro" -# --- Problems mode: the two per-core diagnostic actions --- +# --- Problems mode: the three diagnostic actions --- +# The reset and the re-read mirror MoonBot's own Problems window; the test is ours. +# # A test publishes a `test` fact through the core's own detector worker (~2 s) and PROVES the -# channel end to end. It leaves a row behind that only "clear" removes, which is why the pair ships -# together. +# channel end to end. It leaves a row behind that only the reset removes, which is why that pair +# ships together. core_status.problems_test: ru: "Проверить канал" en: "Test the channel" es: "Probar el canal" +# Deliberately NOT MoonBot's "reset all", though it is the same command on the wire. MoonBot's +# window belongs to one core, so "all" there means all of that core's findings; this panel spans a +# fleet, where the same word reads as "all cores" and would be a lie the moment the operator has +# picked one. The button acts on the panel's scope, and the tooltip and the confirm both say how +# many cores that is. core_status.problems_clear: - ru: "Очистить" - en: "Clear" - es: "Borrar" -# Both actions are per-core on the wire and neither has a sane fleet-wide form, so they need one -# core chosen in the panel's own selector. + ru: "Сбросить" + en: "Reset" + es: "Restablecer" +core_status.problems_test_tip: + ru: "Публикует тестовый факт через собственный детектор ядра — примерно две секунды. Доказывает весь путь: детектор, провод, проекция, панель." + en: "Publishes a test fact through the core's own detector — about two seconds. It proves the whole path: detector, wire, projection, panel." + es: "Publica un hecho de prueba mediante el propio detector del núcleo — unos dos segundos. Demuestra todo el recorrido: detector, cable, proyección, panel." +core_status.problems_refresh: + ru: "Обновить" + en: "Refresh" + es: "Actualizar" +# Said out loud because the label promises a round trip the protocol cannot make: a core pushes its +# list, nothing can ask for one. Saying so here is cheaper than an operator concluding the button is +# broken when a stale row does not move. +core_status.problems_refresh_tip: + ru: "Перечитывает то, что успели прислать ядра на связи (%{cores}). Щёлкни строку, чтобы перечитать только её ядро. Запросить свежий список нельзя: в протоколе такой команды нет — ядро присылает его само, при подключении и когда его детекторы список меняют. Помогает после рестарта ядра, когда старые строки уже не про него." + en: "Re-reads whatever the connected cores (%{cores}) have sent so far. Click a row to re-read only that core. A fresh list cannot be requested: the protocol has no such command — a core sends one itself, on connection and whenever its detectors change it. Worth pressing after a core restart, when the old rows are no longer about it." + es: "Vuelve a leer lo que hayan enviado los núcleos conectados (%{cores}). Haz clic en una fila para releer solo ese núcleo. No se puede pedir una lista nueva: el protocolo no tiene ese comando — el núcleo la envía por sí mismo, al conectar y cuando sus detectores la cambian. Útil tras reiniciar un núcleo, cuando las filas antiguas ya no hablan de él." +# The blast radius, stated BEFORE the dialog: an operator who picked no core is acting on the whole +# scope, and that is the difference between a tidy-up and a fleet-wide loss. +# The one-core forms, for when a clicked finding has narrowed the action. The scope-wide wording +# would state a falsehood about the panel ("connected cores: 1" for a scope of twenty-six) and would +# keep advising a click that, in this state, cancels the pick instead of making one. +core_status.problems_clear_tip_one: + ru: "Сбросит диагностики выбранного ядра — необратимо и для всех терминалов. Щёлкни его строку ещё раз, чтобы вернуться ко всей области" + en: "Resets the selected core's diagnostics — irreversibly, and for every terminal. Click its row again to go back to the whole scope" + es: "Restablece los diagnósticos del núcleo seleccionado — de forma irreversible y para todos los terminales. Haz clic en su fila otra vez para volver a todo el ámbito" +core_status.problems_refresh_tip_one: + ru: "Перечитывает то, что успело прислать выбранное ядро. Запросить свежий список нельзя: в протоколе такой команды нет. Щёлкни его строку ещё раз, чтобы вернуться ко всей области" + en: "Re-reads whatever the selected core has sent so far. A fresh list cannot be requested: the protocol has no such command. Click its row again to go back to the whole scope" + es: "Vuelve a leer lo que haya enviado el núcleo seleccionado. No se puede pedir una lista nueva: el protocolo no tiene ese comando. Haz clic en su fila otra vez para volver a todo el ámbito" +core_status.problems_clear_tip: + ru: "Сбросит диагностики. Ядер на связи: %{cores}. Необратимо и для всех терминалов. Щёлкни строку, чтобы сбросить только её ядро" + en: "Resets the diagnostics. Connected cores: %{cores}. Irreversible, and for every terminal. Click a row to reset only that core" + es: "Restablece los diagnósticos. Núcleos conectados: %{cores}. Irreversible y para todos los terminales. Haz clic en una fila para restablecer solo ese núcleo" +# The test is per-core on the wire and has no fleet-wide form worth having: publishing a test fact +# on two hundred cores litters two hundred cores. So it alone needs one core named. core_status.problems_pick_core: - ru: "Выбери ровно одно ядро в селекторе панели (в режиме Авто он закреплён — переключись на Классику)" - en: "Select exactly one core in the panel's selector (it is pinned in Auto mode — switch to Classic)" - es: "Selecciona exactamente un núcleo en el selector del panel (está fijado en modo Auto — cambia a Clásico)" -# The command channel outlives the connection: a command queued for a core that is down waits there -# and fires on the next reconnect, which for the clear means destroying findings from the outage. + ru: "Тест шлётся на одно ядро: щёлкни строку его проблемы в таблице — или сузь область панели до одного ядра" + en: "The test goes to one core: click that core's finding in the table — or narrow the panel's scope to a single core" + es: "La prueba va a un solo núcleo: haz clic en su hallazgo en la tabla — o reduce el ámbito del panel a un único núcleo" +core_status.problems_picked_offline: + ru: "Выбранное ядро не на связи — команда дождалась бы его возвращения" + en: "The selected core is offline — the command would wait for it to come back" + es: "El núcleo seleccionado está desconectado — el comando esperaría su regreso" +core_status.problems_no_online: + ru: "В области панели нет ядер на связи" + en: "No core in the panel's scope is connected" + es: "Ningún núcleo del ámbito del panel está conectado" +# Guards the TEST alone. The command channel outlives the connection, so a queued test would fire on +# the next reconnect and publish its fact at some unannounced later moment, on a core the operator +# stopped looking at hours ago. core_status.problems_core_offline: - ru: "Ядро не на связи — команда дождалась бы переподключения и стёрла бы то, что накопилось за простой" - en: "The core is offline — the command would wait for its reconnect and wipe what accumulated during the outage" - es: "El núcleo está desconectado — el comando esperaría su reconexión y borraría lo acumulado durante la caída" -core_status.problems_clear_nothing: - ru: "Это ядро ещё не присылало список — очищать нечего" - en: "This core has not delivered a list yet — nothing to clear" - es: "Este núcleo aún no ha enviado una lista — nada que borrar" + ru: "Ядро не на связи — тест дождался бы переподключения и опубликовал бы факт неизвестно когда" + en: "The core is offline — the test would wait for its reconnect and publish its fact at some unannounced later moment" + es: "El núcleo está desconectado — la prueba esperaría su reconexión y publicaría su hecho en un momento posterior no anunciado" core_status.problems_test_title: ru: "Отправить тестовую диагностику?" en: "Send a test diagnostic?" es: "¿Enviar un diagnóstico de prueba?" # The consequence is the whole reason this action is confirmed at all. core_status.problems_test_q: - ru: "Ядро %{core} опубликует тестовый факт через свой обычный детектор — примерно две секунды. Строка останется на ядре, и убрать её можно ТОЛЬКО полной очисткой, которая заодно снесёт настоящие находки этого ядра." - en: "Core %{core} will publish a test fact through its own detector — about two seconds. The row stays on the core, and the ONLY way to remove it is the full clear, which also destroys that core's real findings." - es: "El núcleo %{core} publicará un hecho de prueba mediante su propio detector — unos dos segundos. La fila permanece en el núcleo y la ÚNICA forma de quitarla es el borrado completo, que también destruye los hallazgos reales de ese núcleo." + ru: "Ядро %{core} опубликует тестовый факт через свой обычный детектор — примерно две секунды. Строка останется на ядре, и убрать её можно ТОЛЬКО сбросом, который заодно снесёт настоящие находки этого ядра." + en: "Core %{core} will publish a test fact through its own detector — about two seconds. The row stays on the core, and the ONLY way to remove it is the reset, which also destroys that core's real findings." + es: "El núcleo %{core} publicará un hecho de prueba mediante su propio detector — unos dos segundos. La fila permanece en el núcleo y la ÚNICA forma de quitarla es el restablecimiento, que también destruye los hallazgos reales de ese núcleo." core_status.problems_test_sent: ru: "Тест отправлен на %{core} — строка появится через пару секунд" en: "Test sent to %{core} — the row appears in a couple of seconds" es: "Prueba enviada a %{core} — la fila aparece en un par de segundos" core_status.problems_clear_sent: - ru: "Очистка отправлена на %{core} — строки уйдут, когда ядро пришлёт новый список" - en: "Clear sent to %{core} — the rows go when the core sends its next list" - es: "Borrado enviado a %{core} — las filas se irán cuando el núcleo envíe su próxima lista" + ru: "Сброс отправлен на %{core} — строки уйдут, когда ядро пришлёт новый список" + en: "Reset sent to %{core} — the rows go when the core sends its next list" + es: "Restablecimiento enviado a %{core} — las filas se irán cuando el núcleo envíe su próxima lista" +core_status.problems_clear_sent_many: + ru: "Сброс отправлен на %{cores} ядер — строки уйдут, когда ядра пришлют новые списки" + en: "Reset sent to %{cores} cores — the rows go when those cores send their next lists" + es: "Restablecimiento enviado a %{cores} núcleos — las filas se irán cuando esos núcleos envíen sus próximas listas" +# Says what it re-read, not "done": the honest outcome of a re-read that found nothing new is an +# unchanged table, and a message claiming more would make that look like a failure. +core_status.problems_refreshed: + ru: "Перечитано ядер: %{cores}" + en: "Cores re-read: %{cores}" + es: "Núcleos releídos: %{cores}" +# The shortfall, never a bare success: a fleet action that reached nine of twelve cores has failed +# on three, and only the toast can say so. +core_status.problems_fleet_none: + ru: "Не отправлено ни на одно из %{cores} ядер — не на связи или канал команд закрыт" + en: "Reached none of the %{cores} cores — offline, or their command channel is closed" + es: "No llegó a ninguno de los %{cores} núcleos — desconectados o con el canal de comandos cerrado" +core_status.problems_fleet_partial: + ru: "Дошло до %{sent} из %{cores} ядер — остальные не на связи или их канал закрыт" + en: "Reached %{sent} of %{cores} cores — the rest are offline or their channel is closed" + es: "Llegó a %{sent} de %{cores} núcleos — el resto está desconectado o con el canal cerrado" core_status.problems_not_sent: ru: "Не отправлено: канал команд ядра закрыт" en: "Not sent: the core's command channel is closed" @@ -972,11 +1036,21 @@ core_status.problems_not_sent_offline: en: "Not sent: the core is offline" es: "No enviado: el núcleo está desconectado" core_status.problems_clear_title: - ru: "Очистить диагностики ядра?" - en: "Clear the core's diagnostics?" - es: "¿Borrar los diagnósticos del núcleo?" + ru: "Сбросить диагностики ядра?" + en: "Reset the core's diagnostics?" + es: "¿Restablecer los diagnósticos del núcleo?" # Blunt on purpose: the protocol's own warning is that cleared real diagnostics cannot be restored. core_status.problems_clear_q: ru: "Ядро %{core} сбросит все подтверждённые проблемы и неподтверждённые гипотезы — для всех терминалов, без возможности вернуть. Причину это не лечит: если она осталась, ядро подтвердит проблему заново." en: "Core %{core} will drop every confirmed problem and pending hypothesis — for all terminals, with no way back. It fixes no cause: if one remains, the core will confirm the problem again." es: "El núcleo %{core} descartará todos los problemas confirmados y las hipótesis pendientes — para todos los terminales, sin vuelta atrás. No corrige ninguna causa: si persiste, el núcleo confirmará el problema de nuevo." +core_status.problems_clear_title_many: + ru: "Сбросить диагностики на нескольких ядрах?" + en: "Reset the diagnostics of several cores?" + es: "¿Restablecer los diagnósticos de varios núcleos?" +# The core NAMES, not just how many: a count is nothing an operator can check, and this dialog is +# the last place the mistake is still cheap. +core_status.problems_clear_q_many: + ru: "Каждое из этих %{cores} ядер сбросит все подтверждённые проблемы и неподтверждённые гипотезы — для всех терминалов, без возможности вернуть. Причины это не лечит: если они остались, ядра подтвердят проблемы заново." + en: "Each of these %{cores} cores drops every confirmed problem and pending hypothesis — for all terminals, with no way back. It fixes no cause: those that remain will be confirmed again." + es: "Cada uno de estos %{cores} núcleos descartará todos los problemas confirmados y las hipótesis pendientes — para todos los terminales, sin vuelta atrás. No corrige ninguna causa: las que persistan se confirmarán de nuevo."