diff --git a/CHANGELOG.md b/CHANGELOG.md index 0677963..13f56a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,17 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Fixed + +- **The wipe-profile screen names the context it tells you to delete.** It sent + the operator to `pnm contexts delete` with no id — on the one screen that is + about to take the config file and the keyring entry holding that id with it. + It now prints `pnm contexts delete ` on its own row, + with the id positional, as `pnm-cli`'s `ContextCommands` defines it. A nested + context keeps its whole `/` path; an unloaded account still falls + back to a placeholder rather than naming the wrong context in a destructive + command. + ### Added - **`openvtc health` prints the DID this install authenticates to the VTA as.** diff --git a/openvtc/src/state_handler/main_page/content.rs b/openvtc/src/state_handler/main_page/content.rs index 899532e..d81760c 100644 --- a/openvtc/src/state_handler/main_page/content.rs +++ b/openvtc/src/state_handler/main_page/content.rs @@ -1488,6 +1488,13 @@ pub struct SettingsState { pub persona_agent_name: Option, /// How the config is protected (Token/Encrypted/Plaintext) pub protection_type: String, + /// This account's top trust context at the agent (`top_context_id`). + /// + /// Display-only, and here for the wipe screen: that screen sends the + /// operator to `pnm contexts delete`, which takes the id positionally, and + /// once the profile is gone this pane was the last place the id appeared. + /// Empty when no account is loaded. + pub context_id: String, /// Warning shown when this profile's secret is in a store that will not /// keep it — the Linux kernel keyring, which is RAM-only. `None` when the diff --git a/openvtc/src/state_handler/main_page/mod.rs b/openvtc/src/state_handler/main_page/mod.rs index 294f078..06bbee0 100644 --- a/openvtc/src/state_handler/main_page/mod.rs +++ b/openvtc/src/state_handler/main_page/mod.rs @@ -392,6 +392,7 @@ impl MainPageState { .agent_name_for(config.persona_did()) .map(str::to_owned); self.content_panel.settings.did_git_sign = detect_did_git_sign_info(config.persona_did()); + self.content_panel.settings.context_id = config.account.top_context_id.clone(); // Sync VTA info self.content_panel.vta.persona_did = config.persona_did().to_string(); self.content_panel.vta.persona_agent_name = config diff --git a/openvtc/src/ui/pages/main/components/settings_panel.rs b/openvtc/src/ui/pages/main/components/settings_panel.rs index c7e77cb..45b1489 100644 --- a/openvtc/src/ui/pages/main/components/settings_panel.rs +++ b/openvtc/src/ui/pages/main/components/settings_panel.rs @@ -56,7 +56,7 @@ pub fn render(state: &SettingsState) -> Vec> { SettingsMode::TokenManagement { selected_index } => { render_token_management(state, *selected_index) } - SettingsMode::WipeConfirm { confirm_input } => render_wipe_confirm(confirm_input), + SettingsMode::WipeConfirm { confirm_input } => render_wipe_confirm(state, confirm_input), SettingsMode::View => render_view(state), } } @@ -70,7 +70,7 @@ const VALUE_WIDTH: usize = 50; /// `settings_actions`, which must not open an edit mode for it. pub(crate) const MEDIATOR_ROW: usize = 1; -fn render_wipe_confirm(confirm_input: &str) -> Vec> { +fn render_wipe_confirm(state: &SettingsState, confirm_input: &str) -> Vec> { let mut lines = vec![Line::from("")]; lines.push( Line::from(" Wipe profile") @@ -105,10 +105,18 @@ fn render_wipe_confirm(confirm_input: &str) -> Vec> { Style::new().fg(COLOR_DARK_GRAY), )); lines.push(Line::styled( - " If you want to clean those up too, run `pnm contexts delete` first.", + " If you want to clean those up too, run this first:", Style::new().fg(COLOR_DARK_GRAY), )); lines.push(Line::from("")); + // Its own row, like every other command this TUI hands over: it is meant to + // be retyped in another terminal, and a command sharing a line with the + // prose around it is one an eye has to pick apart first. + lines.push(Line::styled( + format!(" {}", wipe_context_command(&state.context_id)), + Style::new().fg(COLOR_ORANGE), + )); + lines.push(Line::from("")); lines.push(Line::from(vec![ Span::styled(" Type ", Style::new().fg(COLOR_TEXT_DEFAULT)), Span::styled( @@ -542,6 +550,25 @@ fn render_change_protection( lines } +/// The `pnm` command that removes what this wipe deliberately leaves behind. +/// +/// Named with the account's own context id when we have it. This screen is the +/// last place that id appears — the wipe takes the config file and the keyring +/// entry holding it with it — so sending the operator away to look it up is +/// sending them somewhere that will not exist in a moment. Hence "first". +/// +/// `id` is positional on `pnm contexts delete` (verified against `pnm-cli`'s +/// `ContextCommands`); the placeholder is only for an account that has not been +/// loaded, where naming the wrong context would be worse than naming none. +fn wipe_context_command(context_id: &str) -> String { + let id = if context_id.trim().is_empty() { + "" + } else { + context_id + }; + format!("pnm contexts delete {id}") +} + /// Wrap the volatile-storage warning to the settings panel's width, prefixed so /// it reads as an aside rather than another selectable row. /// @@ -565,6 +592,79 @@ fn textwrap_warning(warning: &str) -> Vec { out } +#[cfg(test)] +mod wipe_confirm_tests { + use super::*; + + fn text(lines: &[Line<'static>]) -> String { + lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n") + } + + /// The wipe takes the config file and the keyring entry with it, so this + /// screen is the last place the context id appears. A command that made the + /// operator go and look it up would be sending them somewhere that is about + /// to stop existing. + #[test] + fn the_cleanup_command_names_this_accounts_context() { + let state = SettingsState { + context_id: "openvtc".to_string(), + ..SettingsState::default() + }; + let out = text(&render_wipe_confirm(&state, "")); + assert!(out.contains("pnm contexts delete openvtc"), "{out}"); + assert!(!out.contains(""), "{out}"); + } + + /// `id` is positional on `pnm contexts delete`. A grown `--id` flag would be + /// a command that fails on paste — the shape that had to be fixed on the + /// identity pane's `pnm acl update` hint. + #[test] + fn the_cleanup_command_passes_the_id_positionally() { + assert_eq!( + wipe_context_command("openvtc"), + "pnm contexts delete openvtc" + ); + assert!(!wipe_context_command("openvtc").contains("--id")); + } + + /// A nested context keeps its full path: `pnm` addresses a sub-context by + /// `/`, and the leaf alone names a different context or none. + #[test] + fn a_nested_context_keeps_its_whole_path() { + assert_eq!( + wipe_context_command("acme/eng"), + "pnm contexts delete acme/eng" + ); + } + + /// With no account loaded there is no id to name, and inventing one would + /// point a destructive command at the wrong context. + #[test] + fn an_unloaded_account_falls_back_to_a_placeholder() { + assert_eq!( + wipe_context_command(" "), + "pnm contexts delete " + ); + } + + /// The screen must keep saying what the wipe does *not* touch. The command + /// is the remedy; the sentence above it is the fact that makes it needed. + #[test] + fn the_screen_still_says_what_survives_the_wipe() { + let out = text(&render_wipe_confirm(&SettingsState::default(), "")); + assert!(out.contains("NOT affected"), "{out}"); + } +} + #[cfg(test)] mod storage_warning_tests { use super::textwrap_warning;