From 5bd998ccf597b78302f142b3b374034efec556a5 Mon Sep 17 00:00:00 2001 From: Glenn Gore Date: Tue, 8 Sep 2026 11:06:25 +0200 Subject: [PATCH] fix(settings): name the context the wipe screen tells you to delete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The wipe-profile screen ends by saying the VTA-side context, persona DID and keys survive, and to run `pnm contexts delete` if you want those gone too. It named no context — on the one screen that is about to remove the config file and the keyring entry that hold the id, which is why the advice says "first". Sending the operator away to look it up was sending them somewhere that stops existing a keypress later. The id is now carried on `SettingsState` from `account.top_context_id`, set in the same `sync_from_config` pass as the rest of the pane, and the command is printed complete on its own row — the treatment every other command this TUI hands over already gets, because it is meant to be retyped in another terminal. Positional, as `pnm-cli`'s `ContextCommands` defines it; the `--did` flag that had to come off the identity pane's `pnm acl update` hint in #283 is the same mistake one repo boundary away, so the shape is asserted rather than assumed. A nested context keeps its whole `/` path, since `pnm` addresses a sub-context that way and the leaf alone names a different context or none. An unloaded account keeps a placeholder: no id is better than a wrong one in a command that deletes a context and everything under it. Signed-off-by: Glenn Gore --- CHANGELOG.md | 11 ++ .../src/state_handler/main_page/content.rs | 7 ++ openvtc/src/state_handler/main_page/mod.rs | 1 + .../pages/main/components/settings_panel.rs | 106 +++++++++++++++++- 4 files changed, 122 insertions(+), 3 deletions(-) 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;