diff --git a/crates/lumbridge-settings/Cargo.toml b/crates/lumbridge-settings/Cargo.toml index 72fd630..5b69a66 100644 --- a/crates/lumbridge-settings/Cargo.toml +++ b/crates/lumbridge-settings/Cargo.toml @@ -10,6 +10,14 @@ repository.workspace = true [dependencies] serde = { version = "1.0.228", features = ["derive"] } toml = "0.9" +# `toml` reads; `toml_edit` writes. Serialising the parsed model back over the +# file would strip every comment and every key this build does not recognise, +# and in this design the comments are the documentation. +toml_edit = "0.25" [lints] workspace = true + +[dev-dependencies] +# Already in the workspace's dependency closure via lumbridge-storage's tests. +tempfile = "3.27.0" diff --git a/crates/lumbridge-settings/assets/default.toml b/crates/lumbridge-settings/assets/default.toml new file mode 100644 index 0000000..c4ff95d --- /dev/null +++ b/crates/lumbridge-settings/assets/default.toml @@ -0,0 +1,145 @@ +# Lumbridge settings. +# +# This file is the documentation. Every key Lumbridge understands appears below, +# commented out, showing the value Lumbridge uses when you say nothing — so the +# whole configuration surface can be read without leaving the file. +# +# Two kinds of comment appear here and the difference is load-bearing: +# +# #key = value a setting, commented out, showing the compiled default. +# Remove the '#' to take it over. +# # a sentence explanation. Never a setting. +# +# A test uncomments the first kind, parses the result, and checks that the set +# of keys is exactly the set Lumbridge knows about. Neither half can drift. +# +# --------------------------------------------------------------------------- +# Layers +# +# compiled default -> this file -> the environment +# +# The environment wins, which is the reverse of the usual arrangement and is +# deliberate. LUMBRIDGE_CLAUDE_OAUTH=0 is documented as one switch off, and a +# switch a configuration file can silently turn back on is not a switch. Where a +# variable has pinned a value the settings pane shows that row disabled and +# names the variable, rather than accepting an edit that would do nothing. +# +# --------------------------------------------------------------------------- +# What Lumbridge does to this file +# +# Changing something in the settings pane rewrites the one key you changed and +# leaves everything else exactly as you left it: your comments, your ordering, +# your spacing, and any key this build does not recognise. +# +# An unrecognised key is kept, not deleted, and listed on the Advanced page — +# a file written by a newer Lumbridge still opens in an older one. A syntax +# error is a different matter: Lumbridge keeps the last configuration it read +# and shows a banner naming the line, because a typo must never quietly reset +# your settings to defaults. +# +# --------------------------------------------------------------------------- +# Who may write each key +# +# Every key below says either "a person only" or "a person, or an agent holding +# the Configure capability". Anything that names a program, a filesystem path, +# or a network destination is a person only, and a request from anywhere else is +# refused rather than quietly downgraded. An agent that can arrange panes must +# not be able to choose what runs inside them. +# +# --------------------------------------------------------------------------- +# What is deliberately not here +# +# Four things could plausibly be settings and never will be, because each would +# turn this file into an attack: +# +# - The address an access token is sent to. A configuration file that can +# redirect it is a credential exfiltration path with a friendly name. +# - The file that token is read from. The same attack from the other end. +# - The name Lumbridge gives when it calls a provider. Lumbridge identifies +# itself instead of borrowing the harness's identity, so that its requests +# stay distinguishable from the harness's in the provider's own logs. +# - The program a new pane launches. A layout-only agent cannot choose the +# shell you get, and a settings key would hand it exactly that at one +# remove. +# +# Nothing here turns telemetry on or off, because there is none to turn off. +# The Advanced page lists every file, endpoint, and child process Lumbridge +# reads, by name. + + +[appearance] + +# The syntax theme every interface colour is derived from. Lumbridge does not +# store a palette; it takes five anchors from the theme and computes the rest, +# so a theme name changes the whole window and not just the editor. +# Written by a person, or by an agent holding the Configure capability. +# Takes effect immediately. +#theme = "lumbridge-slate" + +# The action colour: focus rings, selection, the active tab. Kept separate from +# the theme so that the colour meaning "you are here" can be chosen once and +# survive a change of theme. +# Written by a person, or by an agent holding the Configure capability. +# Takes effect immediately. +#accent = "blue" + + +[usage] + +# Read the access token Claude Code stored on this machine, and use it to ask +# Anthropic one documented question about your own subscription: the per-model +# weekly limits a Max plan meters separately, and an answer on a cold start +# before any session has taken a turn. +# +# The token is never persisted, never logged, never copied into application +# state, never written to a crash report, and never passed as a command-line +# argument. It is released as soon as the request is made. No refresh token is +# ever used — renewing a credential is the harness's job. The request identifies +# Lumbridge as the caller. +# +# LUMBRIDGE_CLAUDE_OAUTH=0 in the environment turns this off and outranks +# whatever this file says. +# Written by a person, or by an agent holding the Configure capability. +# Restarts the probe. +#claude_account_endpoint = true + +# Follow the session transcripts Claude Code writes, for token spend. This is +# the only local surface that reports what that harness actually spent. The +# reader models four token counters and nothing else, so the conversations in +# those files are not representable in a Lumbridge value. It reports nothing at +# all until it has read the existing backlog to the end, because a partly-read +# backlog is indistinguishable from a burst of spend. +# Written by a person, or by an agent holding the Configure capability. +# Restarts the probe. +#claude_transcripts = true + +# Launch `codex app-server` and ask it for the quota windows it already knows. +# The client cannot express a request outside a two-variant enum and answers +# every server-to-client request with "method not found", so a harness asking +# Lumbridge for a credential is refused by construction. +# Written by a person, or by an agent holding the Configure capability. +# Restarts the probe. +#codex_app_server = true + +# Where the installed status-line bridge writes Claude Code's rate-limit +# windows. Left unset, Lumbridge looks where the bridge installer puts it; set +# this only if you installed the bridge somewhere else. It has to match what the +# bridge was installed with exactly, or the windows read as unavailable forever. +# +# The example below is an example, not a default — there is no compiled default +# for this key. LUMBRIDGE_CLAUDE_FEED in the environment overrides it. +# Written by a person only: it names a filesystem path. +# Restarts the probe. +#claude_rate_limit_feed = "/example/path/to/claude-rate-limits.jsonl" + + +[terminal] + +# Let a program running in a pane write your system clipboard through the OSC 52 +# escape sequence. Off unless you ask for it: OSC 52 is available to anything +# that can write to your terminal, which includes the contents of a log file you +# happen to cat, so leaving it on means a file can replace what you are about to +# paste. +# Written by a person only: it hands a program a capability outside its pane. +# Applies to new panes; panes already open keep what they were launched with. +#allow_osc52_clipboard = false diff --git a/crates/lumbridge-settings/src/file.rs b/crates/lumbridge-settings/src/file.rs new file mode 100644 index 0000000..2de3397 --- /dev/null +++ b/crates/lumbridge-settings/src/file.rs @@ -0,0 +1,1564 @@ +//! The settings file on disk: reading it, and rewriting it without damage. +//! +//! Three things here are less obvious than they look. +//! +//! **An absent file is not a failure.** A fresh install has no `settings.toml` +//! and is correctly configured. [`LoadOutcome`] therefore separates "not there, +//! use the compiled defaults" from "there and unreadable" from "there and not +//! valid TOML". A parse error in particular never falls back to defaults: the +//! caller keeps the last content it successfully read and shows a banner +//! naming the line, because a typo must never silently reset a configuration. +//! +//! **Writes go through [`toml_edit`], not through `serde`.** Serialising a +//! `SettingsContent` back over the file would produce a semantically identical +//! document with every comment and every unrecognised key stripped out — and in +//! this design the comments *are* the documentation, so that is data loss. A +//! `DocumentMut` edit changes the one key that changed and leaves the rest of +//! the bytes alone. +//! +//! **A write has to be able to recognise its own echo.** The file is watched so +//! that a hand edit shows up without a restart. A watcher cannot tell our write +//! apart from a person's, so a debounced write bounces off its own event and +//! reloads, which schedules another write, forever. [`SettingsFile`] records a +//! fingerprint of the bytes it last wrote and [`SettingsFile::is_own_echo`] +//! answers the watcher's question before any reload work happens. + +use std::fmt; +use std::fs; +use std::io; +use std::io::Write as _; +use std::path::{Path, PathBuf}; + +use toml_edit::{DocumentMut, Item, Table}; + +use crate::paths::SettingsPaths; +use crate::{FORBIDDEN_PATHS, SETTINGS, SettingItem, SettingsContent, UnknownKey, WriteAuthority}; + +/// The commented reference file, embedded in the binary. +/// +/// This is the file's own documentation, and it is what Lumbridge writes when +/// it has to create a `settings.toml` that was not there. Shipping a file full +/// of commented-out defaults rather than active keys matters: an active key +/// would report its origin as `settings.toml` when nobody had chosen it, and +/// the pane would show every row as user-set on a machine nobody had +/// configured. +pub const DEFAULT_TOML: &str = include_str!("../assets/default.toml"); + +/// What reading the settings file produced. +/// +/// Not a `Result`, because the three outcomes call for three different +/// behaviours and collapsing them into "error" loses the distinction that +/// matters: a missing file is normal, an unparseable one must not be +/// overwritten, and an unreadable one is a problem with the machine rather than +/// with the configuration. +#[derive(Clone, Debug)] +pub enum LoadOutcome { + /// No file. Use the compiled defaults; this is not an error. + Absent, + /// A file this build could read. + Loaded { + /// The keys this build understands. + content: SettingsContent, + /// The keys it does not, kept so the Advanced page can report them. + unknown: Vec, + }, + /// A file that is not valid TOML. + /// + /// The caller keeps whatever it last loaded and shows the failure. It must + /// not substitute defaults, and [`SettingsFile`] will not write over the + /// file while it is in this state. + Malformed(ParseFailure), + /// A file that exists and could not be read at all: permissions, a + /// directory where a file should be, bytes that are not UTF-8. + Unreadable { + /// The operating system's description of what went wrong. + message: String, + }, +} + +/// A settings file that is not valid TOML. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct ParseFailure { + /// The parser's description of what it found. + pub message: String, + /// The 1-based line, when the parser reported a position. + /// + /// `Option` rather than a `0` placeholder. A parser that reported no span + /// has told us it does not know where the problem is, and a banner pointing + /// at line 0 — or at line 1 — would send someone to look at a line that is + /// not the problem. + pub line: Option, + /// The 1-based column, on the same terms as [`Self::line`]. + pub column: Option, +} + +impl ParseFailure { + fn at(message: String, text: &str, span: Option>) -> Self { + let position = span.and_then(|span| position_of(text, span.start)); + Self { + message, + line: position.map(|(line, _)| line), + column: position.map(|(_, column)| column), + } + } + + fn from_toml(error: &toml::de::Error, text: &str) -> Self { + Self::at(error.message().to_owned(), text, error.span()) + } + + fn from_toml_edit(error: &toml_edit::TomlError, text: &str) -> Self { + Self::at(error.message().to_owned(), text, error.span()) + } +} + +impl fmt::Display for ParseFailure { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match (self.line, self.column) { + (Some(line), Some(column)) => { + write!(formatter, "line {line}, column {column}: {}", self.message) + } + (Some(line), None) => write!(formatter, "line {line}: {}", self.message), + _ => write!( + formatter, + "at a position the parser did not report: {}", + self.message + ), + } + } +} + +impl std::error::Error for ParseFailure {} + +/// Translates a byte offset into a 1-based line and column. +/// +/// Returns `None` when the offset does not land on a character boundary, which +/// would make any answer a guess. Columns are counted in characters, because +/// that is what a person counts when they look at the line. +fn position_of(text: &str, offset: usize) -> Option<(usize, usize)> { + let before = text.get(..offset)?; + let line = before.matches('\n').count() + 1; + let column = before + .rsplit('\n') + .next() + .map_or(0, |partial| partial.chars().count()) + + 1; + Some((line, column)) +} + +/// Reads the settings file named by `paths`. +/// +/// A file that is not there yields [`LoadOutcome::Absent`] rather than an +/// error, because a machine nobody has configured is configured correctly. +#[must_use] +pub fn load(paths: &impl SettingsPaths) -> LoadOutcome { + load_from(&paths.settings_file()) +} + +/// Reads a settings file at an exact path. +/// +/// [`load`] is the one to call; this exists for a caller that already resolved +/// the path, and for tests that put a file in a temporary directory. +#[must_use] +pub fn load_from(path: &Path) -> LoadOutcome { + let text = match fs::read_to_string(path) { + Ok(text) => text, + Err(error) if error.kind() == io::ErrorKind::NotFound => return LoadOutcome::Absent, + Err(error) => { + return LoadOutcome::Unreadable { + message: error.to_string(), + }; + } + }; + match crate::parse(&text) { + Ok((content, unknown)) => LoadOutcome::Loaded { content, unknown }, + Err(error) => LoadOutcome::Malformed(ParseFailure::from_toml(&error, &text)), + } +} + +/// Who is asking for a write. +/// +/// The counterpart to [`WriteAuthority`], which says who is *allowed*. Keeping +/// them as two types means a caller cannot pass "this field is agent-writable" +/// where "an agent is asking" was wanted. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum WriteOrigin { + /// A person at the keyboard. + Human, + /// An agent or automation holding the Configure capability. + Configure, +} + +impl WriteOrigin { + /// Whether this origin may write a field with the given authority. + #[must_use] + pub const fn satisfies(self, required: WriteAuthority) -> bool { + matches!( + (self, required), + (Self::Human, _) | (Self::Configure, WriteAuthority::Configure) + ) + } +} + +/// A value a setting can be given. +/// +/// Deliberately two variants rather than a re-exported `toml_edit::Value`. A +/// caller cannot ask for an array, a table, or a datetime in a field whose +/// schema is a boolean, so the shapes the file schema cannot hold are not +/// expressible at the call site. +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum SettingValue { + /// A boolean. + Flag(bool), + /// A string. + Text(String), +} + +impl SettingValue { + fn into_item(self) -> Item { + match self { + Self::Flag(flag) => toml_edit::value(flag), + Self::Text(text) => toml_edit::value(text), + } + } +} + +/// Why a write did not happen. +/// +/// Every variant means nothing was written. There is no partial-failure case: +/// the document is edited in memory, checked, and only then does a single +/// rename publish it. +#[derive(Debug)] +pub enum WriteError { + /// No setting has that path in this build. + UnknownPath(String), + /// A path `FORBIDDEN_PATHS` names, which is never a setting. + Forbidden { + /// The path that was asked for. + path: String, + /// Why it is not, and will not become, a setting. + reason: &'static str, + }, + /// The field is human-only and something else asked to write it. + /// + /// Refused, not downgraded and not silently dropped. A caller that learns + /// its write was ignored can say so; a caller told "done" cannot. + AuthorityRefused { + /// The path that was asked for. + path: String, + /// The authority that path requires. + required: WriteAuthority, + /// The origin that asked. + origin: WriteOrigin, + }, + /// The file already on disk is not valid TOML. + /// + /// Refusing here is the point: a person with a typo in their file has a + /// file full of their own comments and choices, and rewriting it from a + /// parsed model would throw all of that away to fix a missing bracket. + Malformed(ParseFailure), + /// A section in the file is not a table — `usage = 3`, say. + NotASection { + /// The section name that is occupied by something else. + section: String, + }, + /// The edit produced a document this build cannot read back. + /// + /// A last check before the rename. No edit this API can express reaches it + /// today — [`SettingValue`] cannot name a shape the schema will not hold, + /// and a file that was already unreadable is caught as + /// [`WriteError::Malformed`] before the edit. It stays because writing a + /// file we would then refuse to load is the one failure that locks a person + /// out of their own settings with our bug rather than their typo, and that + /// is worth checking rather than reasoning about. + Rejected(ParseFailure), + /// The filesystem refused. + Io { + /// What was being written. + path: PathBuf, + /// The underlying failure. + source: io::Error, + }, +} + +impl fmt::Display for WriteError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::UnknownPath(path) => write!(formatter, "no setting named {path}"), + Self::Forbidden { path, reason } => { + write!(formatter, "{path} is never a setting: {reason}") + } + Self::AuthorityRefused { + path, + required, + origin, + } => write!( + formatter, + "{path} may be written by {required:?} only, and {origin:?} asked" + ), + Self::Malformed(failure) => write!( + formatter, + "the settings file on disk is not valid TOML ({failure}); \ + nothing was written, so nothing was lost" + ), + Self::NotASection { section } => { + write!(formatter, "[{section}] is not a table in the settings file") + } + Self::Rejected(failure) => write!( + formatter, + "the edit would have produced a file this build cannot read ({failure})" + ), + Self::Io { path, source } => write!(formatter, "writing {}: {source}", path.display()), + } + } +} + +impl std::error::Error for WriteError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::Io { source, .. } => Some(source), + Self::Malformed(failure) | Self::Rejected(failure) => Some(failure), + _ => None, + } + } +} + +/// A fingerprint of file content, for telling our own write apart from a +/// person's edit. +/// +/// FNV-1a over the bytes, plus their length, rather than SHA-256. The property +/// needed is "are these the bytes I just wrote", against an accidental +/// collision and not against an adversary: anyone who could craft a colliding +/// settings file already has permission to write the settings file, so a +/// cryptographic hash buys nothing here and costs a dependency. `std`'s +/// `DefaultHasher` is excluded for the opposite reason — it is documented as +/// unstable across releases, and a fingerprint is only useful if it means the +/// same thing every time. Comparing the length as well makes an accidental +/// collision require agreement on both. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +struct Fingerprint { + length: usize, + hash: u64, +} + +impl Fingerprint { + const OFFSET_BASIS: u64 = 0xcbf2_9ce4_8422_2325; + const PRIME: u64 = 0x0000_0100_0000_01b3; + + fn of(bytes: &[u8]) -> Self { + let mut hash = Self::OFFSET_BASIS; + for byte in bytes { + hash ^= u64::from(*byte); + hash = hash.wrapping_mul(Self::PRIME); + } + Self { + length: bytes.len(), + hash, + } + } +} + +/// The settings file, and the memory of what we last wrote to it. +/// +/// Holds no configuration of its own: [`Self::load`] hands back content and the +/// caller resolves it against the environment. What it does own is the +/// fingerprint of the last write, which is the only state that cannot be +/// recomputed from the filesystem. +#[derive(Clone, Debug)] +pub struct SettingsFile { + path: PathBuf, + last_written: Option, +} + +impl SettingsFile { + /// Opens the settings file named by `paths`. + /// + /// Touches nothing on disk; the file may not exist yet. + #[must_use] + pub fn at(paths: &impl SettingsPaths) -> Self { + Self::at_path(paths.settings_file()) + } + + /// Opens a settings file at an exact path. + #[must_use] + pub fn at_path(path: PathBuf) -> Self { + Self { + path, + last_written: None, + } + } + + /// The file being read and written. + #[must_use] + pub fn path(&self) -> &Path { + &self.path + } + + /// The directory a watcher should watch. + /// + /// The parent, not the file: an atomic write replaces the file by rename, + /// so a watch on the inode stops seeing anything after the first write. + #[must_use] + pub fn watch_directory(&self) -> Option<&Path> { + self.path.parent() + } + + /// Reads the file. + #[must_use] + pub fn load(&self) -> LoadOutcome { + load_from(&self.path) + } + + /// Whether these bytes are the ones this instance last wrote. + /// + /// The watcher's first question, asked before any parse or reload. Answers + /// `false` until something has been written, and keeps answering `true` for + /// as long as the content matches — one write can produce several watcher + /// events, so this must not be a one-shot token. Bytes that match a write + /// we made are bytes identical to what we already believe, so suppressing a + /// person who happened to type the file back to exactly that costs nothing. + #[must_use] + pub fn is_own_echo(&self, bytes: &[u8]) -> bool { + self.last_written == Some(Fingerprint::of(bytes)) + } + + /// Writes the commented reference file if there is nothing there yet. + /// + /// Returns whether it created the file. Creating it records the echo + /// fingerprint, because the watcher will report the creation and that + /// report is ours. + /// + /// The check is a plain `exists` and not an atomic create-if-absent. The + /// only writer that can lose that race is a second Lumbridge starting at + /// the same moment, and it would write byte-identical content. + /// + /// # Errors + /// + /// [`WriteError::Io`] if the directory cannot be created or the file cannot + /// be written. An existing file — valid or not — is left exactly alone. + pub fn create_from_default_if_absent(&mut self) -> Result { + if self.path.exists() { + return Ok(false); + } + self.publish(DEFAULT_TOML)?; + Ok(true) + } + + /// Sets one setting. + /// + /// # Errors + /// + /// See [`WriteError`]. Every variant leaves the file untouched. + pub fn set( + &mut self, + path: &str, + value: SettingValue, + origin: WriteOrigin, + ) -> Result<(), WriteError> { + self.apply(vec![(path.to_owned(), value)], origin) + } + + /// Sets several settings as one write. + /// + /// One document, one check, one rename, one watcher echo — rather than a + /// burst of writes that a debounced watcher would coalesce into a reload + /// whose fingerprint matches only the last of them. + /// + /// # Errors + /// + /// See [`WriteError`]. If any edit is refused, none of them are applied. + pub fn apply( + &mut self, + edits: Vec<(String, SettingValue)>, + origin: WriteOrigin, + ) -> Result<(), WriteError> { + let mut text = self.read_or_seed()?; + // Checked before the edit rather than only after it, so that a file + // that was already broken is reported as the person's typo instead of + // as our edit having broken it. + if let Err(error) = crate::parse(&text) { + return Err(WriteError::Malformed(ParseFailure::from_toml( + &error, &text, + ))); + } + // Authorise the whole batch before touching anything, so that a refused + // edit halfway down cannot leave the earlier ones applied. + let authorised = edits + .into_iter() + .map(|(path, value)| authorise(&path, origin).map(|item| (item, value))) + .collect::, _>>()?; + for (item, _) in &authorised { + if let Some(revived) = revive_placeholder(&text, item.path) { + text = revived; + } + } + let mut document = text + .parse::() + .map_err(|error| WriteError::Malformed(ParseFailure::from_toml_edit(&error, &text)))?; + for (item, value) in authorised { + set_in_document(&mut document, item.path, value)?; + } + let rendered = document.to_string(); + if let Err(error) = crate::parse(&rendered) { + return Err(WriteError::Rejected(ParseFailure::from_toml( + &error, &rendered, + ))); + } + self.publish(&rendered) + } + + /// The text to edit: what is on disk, or the reference file if nothing is. + /// + /// Seeding from the reference file rather than from an empty string means a + /// person who first changes a setting in the pane and then opens the file + /// finds it documented, instead of finding one bare line. + fn read_or_seed(&self) -> Result { + match fs::read_to_string(&self.path) { + Ok(text) => Ok(text), + Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(DEFAULT_TOML.to_owned()), + Err(source) => Err(WriteError::Io { + path: self.path.clone(), + source, + }), + } + } + + fn publish(&mut self, text: &str) -> Result<(), WriteError> { + write_atomically(&self.path, text.as_bytes()).map_err(|source| WriteError::Io { + path: self.path.clone(), + source, + })?; + self.last_written = Some(Fingerprint::of(text.as_bytes())); + Ok(()) + } +} + +/// Checks that this path is writable at all, and by this origin. +fn authorise(path: &str, origin: WriteOrigin) -> Result<&'static SettingItem, WriteError> { + if let Some((_, reason)) = FORBIDDEN_PATHS.iter().find(|(known, _)| *known == path) { + return Err(WriteError::Forbidden { + path: path.to_owned(), + reason, + }); + } + let item = SETTINGS + .iter() + .find(|item| item.path == path) + .ok_or_else(|| WriteError::UnknownPath(path.to_owned()))?; + if origin.satisfies(item.authority) { + Ok(item) + } else { + Err(WriteError::AuthorityRefused { + path: path.to_owned(), + required: item.authority, + origin, + }) + } +} + +/// Turns `#theme = "lumbridge-slate"` back into a live `theme` key, in place. +/// +/// The reference file documents each setting as a commented-out line directly +/// below the prose that explains it. Letting `toml_edit` append a live +/// `theme = "nord"` elsewhere in the section would leave the file arguing with +/// itself: one active value, one commented value for the same key, and the +/// explanation attached to the wrong one. Reviving the placeholder puts the +/// chosen value exactly where the file says that key belongs. +/// +/// Returns `None` — leaving the append to `toml_edit` — when the key is already +/// live in that section or has no placeholder. The convention is the one stated +/// at the top of the reference file: `#key = value` is a commented-out setting, +/// `# a sentence` is prose, and only the first is ever touched. +fn revive_placeholder(text: &str, path: &str) -> Option { + let (section, key) = path.split_once('.')?; + let mut current = ""; + let mut offset = 0; + let mut marker = None; + for line in text.split_inclusive('\n') { + let start = offset; + offset += line.len(); + let trimmed = line.trim(); + if let Some(name) = trimmed + .strip_prefix('[') + .and_then(|rest| rest.strip_suffix(']')) + { + current = name; + } else if current == section { + if assignment_to(trimmed, key) { + // Already live; editing its value in place is `toml_edit`'s job. + return None; + } + if marker.is_none() + && trimmed + .strip_prefix('#') + .is_some_and(|rest| assignment_to(rest, key)) + { + marker = Some(start + line.find('#')?); + } + } + } + // Deleting the single `#` rather than rebuilding the line keeps the + // indentation and the line ending exactly as the file had them; rebuilding + // from `lines()` would quietly rewrite a CRLF file as LF. + let marker = marker?; + Some(format!("{}{}", &text[..marker], &text[marker + 1..])) +} + +/// Whether this line assigns to exactly `key`, and not to a key it prefixes. +fn assignment_to(line: &str, key: &str) -> bool { + line.strip_prefix(key) + .is_some_and(|rest| rest.trim_start().starts_with('=')) +} + +/// Writes one dotted path into the document, keeping everything around it. +fn set_in_document( + document: &mut DocumentMut, + path: &str, + value: SettingValue, +) -> Result<(), WriteError> { + let (section, key) = path + .split_once('.') + .ok_or_else(|| WriteError::UnknownPath(path.to_owned()))?; + let table = section_table(document, section)?; + // The value's decor is its surrounding whitespace and any comment on the + // same line. Replacing the item would drop both, so a file written as + // theme = "gruvbox" # the one that matches my terminal + // would lose the note explaining the choice the moment the theme changed. + let existing = table + .get(key) + .and_then(Item::as_value) + .map(|value| value.decor().clone()); + let mut item = value.into_item(); + if let (Some(decor), Some(new)) = (existing, item.as_value_mut()) { + *new.decor_mut() = decor; + } + // `Table::insert` reformats the key, which erases the comment block sitting + // above it; the entry API keeps the key exactly as the file spelled it. + match table.entry(key) { + toml_edit::Entry::Occupied(mut occupied) => { + occupied.insert(item); + } + toml_edit::Entry::Vacant(vacant) => { + vacant.insert(item); + } + } + Ok(()) +} + +/// Finds or creates the `[section]` table. +fn section_table<'document>( + document: &'document mut DocumentMut, + section: &str, +) -> Result<&'document mut Table, WriteError> { + let item = document + .entry(section) + .or_insert_with(|| Item::Table(Table::new())); + let table = item.as_table_mut().ok_or_else(|| WriteError::NotASection { + section: section.to_owned(), + })?; + // A table created on the way to a key is implicit and renders as nothing at + // all, so the key would be emitted at the top level under a bare name and + // land in a different place on the next read. + table.set_implicit(false); + Ok(table) +} + +/// Writes `bytes` to `path` so that no reader ever sees a partial file. +/// +/// A temporary file in the *same* directory — a rename across filesystems is +/// not atomic and `/tmp` is very often a different filesystem — then fsync, then +/// rename over the destination. A reader either sees the whole old file or the +/// whole new one. Truncating the real file and writing into it would expose an +/// empty settings file to anyone who read during the window, and to everyone +/// forever if the machine lost power inside it. +fn write_atomically(path: &Path, bytes: &[u8]) -> io::Result<()> { + let directory = path.parent().ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "a settings file must have a parent directory", + ) + })?; + fs::create_dir_all(directory)?; + let (temporary, mut file) = create_private_file(directory)?; + let written = file + .write_all(bytes) + .and_then(|()| file.sync_all()) + .and_then(|()| { + drop(file); + fs::rename(&temporary, path) + }); + if let Err(error) = written { + // The temporary file is ours and half-written; leaving it behind would + // accumulate one per failure in the user's config directory. + drop(fs::remove_file(&temporary)); + return Err(error); + } + sync_directory(directory) +} + +/// Creates a uniquely named, owner-only file in `directory`. +/// +/// Named for the settings file it will become, and hidden, so that a person who +/// finds one after a crash can tell what it was. +fn create_private_file(directory: &Path) -> io::Result<(PathBuf, fs::File)> { + let mut last = None; + for attempt in 0..32_u32 { + let candidate = directory.join(format!( + ".settings.toml.{}.{attempt}.tmp", + std::process::id() + )); + match private_options().open(&candidate) { + Ok(file) => return Ok((candidate, file)), + // `create_new` failing this way means a previous run died here or a + // second Lumbridge is writing; try the next name rather than + // truncating a file we did not create. + Err(error) if error.kind() == io::ErrorKind::AlreadyExists => last = Some(error), + Err(error) => return Err(error), + } + } + Err(last.unwrap_or_else(|| { + io::Error::new( + io::ErrorKind::AlreadyExists, + "no free temporary name in the settings directory", + ) + })) +} + +/// Open options that create a file only the owner can read. +/// +/// The settings file is not a secret, but it names paths on this machine and it +/// sits in a directory that is ours alone; mode 0600 is what the rest of a +/// user's config directory looks like and there is no reason to be looser. +#[cfg(unix)] +fn private_options() -> fs::OpenOptions { + use std::os::unix::fs::OpenOptionsExt as _; + + let mut options = fs::OpenOptions::new(); + options.write(true).create_new(true).mode(0o600); + options +} + +/// The same, on a platform with no Unix mode bits. +/// +/// The file inherits the directory's access control, which on Windows means the +/// user profile's. Reported honestly rather than pretending 0600 was applied. +#[cfg(not(unix))] +fn private_options() -> fs::OpenOptions { + let mut options = fs::OpenOptions::new(); + options.write(true).create_new(true); + options +} + +/// Flushes the rename itself. +/// +/// The rename is already atomic with respect to readers; this is about the +/// power cut afterwards, where an unsynced directory entry can leave the old +/// file back in place. Nothing is ever half-written either way. +#[cfg(unix)] +fn sync_directory(directory: &Path) -> io::Result<()> { + fs::File::open(directory)?.sync_all() +} + +/// The same, where opening a directory as a file is not allowed. +#[cfg(not(unix))] +fn sync_directory(_directory: &Path) -> io::Result<()> { + Ok(()) +} + +#[cfg(test)] +mod tests { + use std::fs; + use std::path::{Path, PathBuf}; + + use tempfile::TempDir; + + use super::{ + DEFAULT_TOML, Fingerprint, LoadOutcome, ParseFailure, SettingValue, SettingsFile, + WriteError, WriteOrigin, create_private_file, load_from, + }; + use crate::{FORBIDDEN_PATHS, MapEnv, Origin, SETTINGS, Settings, WriteAuthority}; + + /// A settings file in a directory that disappears with the test. + struct Scratch { + directory: TempDir, + } + + impl Scratch { + fn new() -> Self { + Self { + directory: TempDir::new().expect("a temporary directory"), + } + } + + fn path(&self) -> PathBuf { + self.directory.path().join("settings.toml") + } + + fn file(&self) -> SettingsFile { + SettingsFile::at_path(self.path()) + } + + fn seed(&self, text: &str) -> SettingsFile { + fs::write(self.path(), text).expect("writing the fixture"); + self.file() + } + + fn text(&self) -> String { + fs::read_to_string(self.path()).expect("reading the file back") + } + } + + fn configure(file: &mut SettingsFile, path: &str, value: SettingValue) { + file.set(path, value, WriteOrigin::Configure) + .unwrap_or_else(|error| panic!("{path} should have been writable: {error}")); + } + + // ---- loading ----------------------------------------------------------- + + #[test] + fn a_file_that_is_not_there_is_not_an_error() { + let scratch = Scratch::new(); + assert!( + matches!(load_from(&scratch.path()), LoadOutcome::Absent), + "a machine nobody has configured is configured correctly, and must \ + not be reported as a failure" + ); + } + + #[test] + fn a_loaded_file_carries_both_what_this_build_knows_and_what_it_does_not() { + let scratch = Scratch::new(); + scratch.seed("[appearance]\ntheme = \"gruvbox\"\nsparkles = true\n"); + let LoadOutcome::Loaded { content, unknown } = load_from(&scratch.path()) else { + panic!("a valid file must load"); + }; + assert_eq!( + content.appearance.theme.as_deref(), + Some("gruvbox"), + "the known key must reach the caller" + ); + assert_eq!( + unknown + .iter() + .map(|key| key.path.as_str()) + .collect::>(), + vec!["appearance.sparkles"], + "a key from a newer build is reported, not dropped in silence" + ); + } + + #[test] + fn a_typo_is_reported_with_its_line_and_never_as_defaults() { + let scratch = Scratch::new(); + scratch.seed("[appearance]\ntheme = \"ok\"\n\n[usage\ncodex_app_server = true\n"); + let LoadOutcome::Malformed(failure) = load_from(&scratch.path()) else { + panic!( + "a typo must be an error: falling back to defaults would silently \ + discard the whole configuration" + ); + }; + assert_eq!( + failure.line, + Some(4), + "the banner has to name the line the person must go and fix" + ); + assert!( + !failure.message.is_empty(), + "a banner with no message tells nobody anything" + ); + } + + #[test] + fn an_unreported_position_renders_as_unknown_rather_than_as_line_one() { + let failure = ParseFailure { + message: "something went wrong".to_owned(), + line: None, + column: None, + }; + assert_eq!( + failure.to_string(), + "at a position the parser did not report: something went wrong", + "pointing at line 1 would send someone to a line that is not the problem" + ); + } + + #[test] + fn a_file_that_cannot_be_read_is_not_confused_with_one_that_is_not_there() { + let scratch = Scratch::new(); + fs::create_dir(scratch.path()).expect("a directory where the file should be"); + assert!( + matches!(load_from(&scratch.path()), LoadOutcome::Unreadable { .. }), + "treating an unreadable file as absent would quietly replace a \ + configuration the user still has" + ); + } + + // ---- comments and order survive --------------------------------------- + + const HAND_WRITTEN: &str = "\ +# what I actually run + +[appearance] +# this one matches my terminal +theme = \"gruvbox-dark\" +accent = \"amber\" + +[usage] +codex_app_server = false +sparkles = \"from a build newer than this one\" +"; + + #[test] + fn a_commented_file_keeps_its_comments_after_a_programmatic_edit() { + let scratch = Scratch::new(); + let mut file = scratch.seed(HAND_WRITTEN); + configure( + &mut file, + "appearance.theme", + SettingValue::Text("catppuccin-mocha".to_owned()), + ); + let text = scratch.text(); + assert!( + text.contains("# what I actually run"), + "a comment above the first section is the file's title and must \ + survive an edit; got:\n{text}" + ); + assert!( + text.contains("# this one matches my terminal"), + "the comment explaining a choice must outlive the choice; got:\n{text}" + ); + assert!( + text.contains("theme = \"catppuccin-mocha\""), + "the edit must actually have happened; got:\n{text}" + ); + } + + #[test] + fn key_order_is_unchanged_by_an_edit() { + let scratch = Scratch::new(); + let mut file = scratch.seed(HAND_WRITTEN); + configure( + &mut file, + "appearance.theme", + SettingValue::Text("catppuccin-mocha".to_owned()), + ); + let text = scratch.text(); + let theme = text.find("theme =").expect("theme is still there"); + let accent = text.find("accent =").expect("accent is still there"); + assert!( + theme < accent, + "a file re-serialised from a parsed model comes back in the \ + schema's order, not the person's; got:\n{text}" + ); + } + + #[test] + fn a_key_this_build_does_not_understand_survives_a_write_by_this_build() { + let scratch = Scratch::new(); + let mut file = scratch.seed(HAND_WRITTEN); + configure( + &mut file, + "usage.codex_app_server", + SettingValue::Flag(true), + ); + assert!( + scratch + .text() + .contains("sparkles = \"from a build newer than this one\""), + "an older build must not delete a newer build's settings, or \ + downgrading once loses them permanently" + ); + } + + #[test] + fn a_note_written_beside_a_value_outlives_the_value_changing() { + let scratch = Scratch::new(); + let mut file = scratch.seed("[appearance]\ntheme = \"gruvbox\" # chosen deliberately\n"); + configure( + &mut file, + "appearance.theme", + SettingValue::Text("nord".to_owned()), + ); + let text = scratch.text(); + assert!( + text.contains("# chosen deliberately"), + "an inline note is the reason for the setting, not decoration; got:\n{text}" + ); + assert!(text.contains("theme = \"nord\""), "got:\n{text}"); + } + + #[test] + fn a_setting_the_file_did_not_mention_is_added_to_its_section() { + let scratch = Scratch::new(); + let mut file = scratch.seed("[appearance]\ntheme = \"nord\"\n"); + configure( + &mut file, + "usage.codex_app_server", + SettingValue::Flag(false), + ); + let LoadOutcome::Loaded { content, .. } = file.load() else { + panic!("the file must still load after an edit"); + }; + assert_eq!( + content.usage.codex_app_server, + Some(false), + "a section the file never had must be created rather than dropped" + ); + assert_eq!( + content.appearance.theme.as_deref(), + Some("nord"), + "and the sections it did have must survive" + ); + } + + #[test] + fn a_commented_out_default_becomes_the_live_key_where_the_file_documents_it() { + let scratch = Scratch::new(); + let mut file = scratch.file(); + configure( + &mut file, + "appearance.theme", + SettingValue::Text("nord".to_owned()), + ); + let text = scratch.text(); + assert!( + text.contains("theme = \"nord\""), + "the chosen value must be live; got:\n{text}" + ); + assert!( + !text.contains("#theme = "), + "leaving the commented default behind makes the file argue with \ + itself: one active value, one commented value, and the \ + explanation attached to the wrong one; got:\n{text}" + ); + let explanation = text + .find("# The syntax theme every interface colour") + .expect("the prose is still there"); + let live = text.find("theme = \"nord\"").expect("the value is there"); + assert!( + explanation < live, + "the value must stay directly under the paragraph explaining it" + ); + } + + #[test] + fn reviving_a_placeholder_changes_nothing_but_the_comment_marker() { + let before = "[appearance]\r\n#theme = \"lumbridge-slate\"\r\n"; + let revived = super::revive_placeholder(before, "appearance.theme") + .expect("the placeholder is there to revive"); + assert_eq!( + revived, "[appearance]\r\ntheme = \"lumbridge-slate\"\r\n", + "rebuilding the file from its lines would silently rewrite a CRLF \ + file as LF, which is a change nobody asked for" + ); + } + + #[test] + fn a_placeholder_for_a_longer_key_is_not_mistaken_for_this_one() { + let text = "[appearance]\n#theme_variant = \"dark\"\n"; + assert!( + super::revive_placeholder(text, "appearance.theme").is_none(), + "a key that another key merely starts with is a different key" + ); + } + + #[test] + fn a_placeholder_in_another_section_is_left_alone() { + let text = "[usage]\n#theme = \"nord\"\n\n[appearance]\n"; + assert!( + super::revive_placeholder(text, "appearance.theme").is_none(), + "keys are dotted paths, and the section half of the path counts" + ); + } + + #[test] + fn prose_that_happens_to_look_like_an_assignment_is_left_as_prose() { + let text = "[appearance]\n# theme = whatever you like\n"; + assert!( + super::revive_placeholder(text, "appearance.theme").is_none(), + "'# words' is a sentence and '#key = value' is a setting; \ + uncommenting a sentence would corrupt the file" + ); + } + + // ---- write authority --------------------------------------------------- + + #[test] + fn an_agent_cannot_write_a_field_that_names_a_path() { + let scratch = Scratch::new(); + let mut file = scratch.seed(HAND_WRITTEN); + let refused = file.set( + "usage.claude_rate_limit_feed", + SettingValue::Text("/somewhere/an/agent/chose".to_owned()), + WriteOrigin::Configure, + ); + assert!( + matches!( + refused, + Err(WriteError::AuthorityRefused { + required: WriteAuthority::Human, + .. + }) + ), + "decision 0006 guarantees a layout-only agent cannot choose what \ + Lumbridge reads or runs; got {refused:?}" + ); + assert_eq!( + scratch.text(), + HAND_WRITTEN, + "a refused write must be refused, not downgraded and not partial" + ); + } + + #[test] + fn a_person_may_write_a_field_an_agent_may_not() { + let scratch = Scratch::new(); + let mut file = scratch.seed(HAND_WRITTEN); + file.set( + "usage.claude_rate_limit_feed", + SettingValue::Text("/home/example/feed.jsonl".to_owned()), + WriteOrigin::Human, + ) + .expect("a person at the keyboard may set a path"); + assert!(scratch.text().contains("/home/example/feed.jsonl")); + } + + #[test] + fn a_path_that_must_never_be_a_setting_is_refused_at_the_write() { + let scratch = Scratch::new(); + let mut file = scratch.seed(HAND_WRITTEN); + for (path, reason) in FORBIDDEN_PATHS { + let refused = file.set( + path, + SettingValue::Text("anything".to_owned()), + WriteOrigin::Human, + ); + assert!( + matches!(refused, Err(WriteError::Forbidden { .. })), + "{path} must be refused by name and not merely absent: {reason}" + ); + } + assert_eq!(scratch.text(), HAND_WRITTEN, "and nothing may be written"); + } + + #[test] + fn a_path_this_build_has_no_setting_for_is_refused() { + let scratch = Scratch::new(); + let mut file = scratch.seed(HAND_WRITTEN); + let refused = file.set( + "appearance.sparkles", + SettingValue::Flag(true), + WriteOrigin::Human, + ); + assert!( + matches!(refused, Err(WriteError::UnknownPath(_))), + "writing a key with no schema would produce a file that reports \ + itself unknown on the next load; got {refused:?}" + ); + } + + #[test] + fn one_refused_edit_in_a_batch_applies_none_of_them() { + let scratch = Scratch::new(); + let mut file = scratch.seed(HAND_WRITTEN); + let refused = file.apply( + vec![ + ( + "appearance.theme".to_owned(), + SettingValue::Text("nord".to_owned()), + ), + ( + "usage.claude_rate_limit_feed".to_owned(), + SettingValue::Text("/agent/chose/this".to_owned()), + ), + ], + WriteOrigin::Configure, + ); + assert!(refused.is_err(), "the batch names a human-only field"); + assert_eq!( + scratch.text(), + HAND_WRITTEN, + "half an applied batch is a configuration nobody asked for" + ); + } + + // ---- refusing to damage a file ---------------------------------------- + + #[test] + fn a_file_with_a_typo_in_it_is_never_rewritten() { + let broken = "[appearance\ntheme = \"gruvbox\"\n# a comment worth keeping\n"; + let scratch = Scratch::new(); + let mut file = scratch.seed(broken); + let refused = file.set( + "appearance.theme", + SettingValue::Text("nord".to_owned()), + WriteOrigin::Human, + ); + assert!( + matches!(refused, Err(WriteError::Malformed(_))), + "rewriting from a parsed model would fix the bracket by throwing \ + away everything else in the file; got {refused:?}" + ); + assert_eq!(scratch.text(), broken, "the person's file is left alone"); + } + + #[test] + fn a_file_whose_value_has_the_wrong_type_is_reported_rather_than_repaired() { + // Valid TOML, invalid against the schema: `parse` rejects it, so an + // edit must not quietly drop the offending key on its way past. + let wrong = "[usage]\ncodex_app_server = \"yes please\"\n"; + let scratch = Scratch::new(); + let mut file = scratch.seed(wrong); + let refused = file.set( + "appearance.theme", + SettingValue::Text("nord".to_owned()), + WriteOrigin::Human, + ); + assert!( + matches!(refused, Err(WriteError::Malformed(_))), + "got {refused:?}" + ); + assert_eq!(scratch.text(), wrong); + } + + // ---- atomic write ------------------------------------------------------ + + #[test] + fn the_temporary_file_shares_a_directory_with_its_destination() { + let scratch = Scratch::new(); + let (temporary, _handle) = + create_private_file(scratch.directory.path()).expect("a temporary file"); + assert_eq!( + temporary.parent(), + Some(scratch.directory.path()), + "a rename across filesystems is not atomic, so the temporary file \ + cannot live in the system temporary directory" + ); + } + + #[test] + fn a_write_leaves_no_temporary_file_behind() { + let scratch = Scratch::new(); + let mut file = scratch.file(); + configure( + &mut file, + "appearance.theme", + SettingValue::Text("nord".to_owned()), + ); + let names: Vec = fs::read_dir(scratch.directory.path()) + .expect("listing the settings directory") + .map(|entry| { + entry + .expect("an entry") + .file_name() + .to_string_lossy() + .into_owned() + }) + .collect(); + assert_eq!( + names, + vec!["settings.toml".to_owned()], + "a temporary file per write would accumulate in the user's config \ + directory forever" + ); + } + + #[cfg(unix)] + #[test] + fn a_written_file_is_readable_only_by_its_owner() { + use std::os::unix::fs::PermissionsExt as _; + + let scratch = Scratch::new(); + let mut file = scratch.file(); + configure( + &mut file, + "appearance.theme", + SettingValue::Text("nord".to_owned()), + ); + let mode = fs::metadata(scratch.path()) + .expect("the written file") + .permissions() + .mode() + & 0o777; + assert_eq!( + mode, 0o600, + "the settings file names paths on this machine and sits in a \ + directory that is ours alone" + ); + } + + // ---- seeding from the reference file ----------------------------------- + + #[test] + fn a_missing_file_is_seeded_with_the_documented_reference_file() { + let scratch = Scratch::new(); + let mut file = scratch.file(); + assert!( + file.create_from_default_if_absent() + .expect("seeding a fresh directory"), + "it must report that it created the file" + ); + assert_eq!( + scratch.text(), + DEFAULT_TOML, + "the file a person opens is the reference file, comments and all" + ); + } + + #[test] + fn an_existing_file_is_never_replaced_by_the_reference_file() { + let scratch = Scratch::new(); + let mut file = scratch.seed(HAND_WRITTEN); + assert!( + !file + .create_from_default_if_absent() + .expect("an existing file"), + "it must report that it created nothing" + ); + assert_eq!(scratch.text(), HAND_WRITTEN); + } + + #[test] + fn a_first_edit_writes_a_documented_file_rather_than_one_bare_line() { + let scratch = Scratch::new(); + let mut file = scratch.file(); + configure( + &mut file, + "appearance.theme", + SettingValue::Text("nord".to_owned()), + ); + let text = scratch.text(); + assert!( + text.contains("Lumbridge settings."), + "changing a setting in the pane must not produce a file that \ + documents nothing; got:\n{text}" + ); + assert!(text.contains("theme = \"nord\""), "got:\n{text}"); + } + + // ---- echo suppression -------------------------------------------------- + + #[test] + fn nothing_is_an_echo_before_anything_has_been_written() { + let scratch = Scratch::new(); + let file = scratch.seed(HAND_WRITTEN); + assert!( + !file.is_own_echo(HAND_WRITTEN.as_bytes()), + "a file we never wrote is somebody else's, however familiar it looks" + ); + } + + #[test] + fn the_bytes_we_just_wrote_are_recognised_as_our_own_echo() { + let scratch = Scratch::new(); + let mut file = scratch.seed(HAND_WRITTEN); + configure( + &mut file, + "appearance.theme", + SettingValue::Text("nord".to_owned()), + ); + assert!( + file.is_own_echo(scratch.text().as_bytes()), + "without this the watcher reloads our own write, which schedules \ + another write, forever" + ); + } + + #[test] + fn an_echo_is_recognised_every_time_because_one_write_raises_several_events() { + let scratch = Scratch::new(); + let mut file = scratch.file(); + configure( + &mut file, + "appearance.theme", + SettingValue::Text("nord".to_owned()), + ); + let written = scratch.text(); + for event in 0..3 { + assert!( + file.is_own_echo(written.as_bytes()), + "event {event} of one rename must still be recognised; a \ + one-shot token would let create-then-modify through" + ); + } + } + + #[test] + fn a_hand_edit_is_not_mistaken_for_our_own_echo() { + let scratch = Scratch::new(); + let mut file = scratch.file(); + configure( + &mut file, + "appearance.theme", + SettingValue::Text("nord".to_owned()), + ); + let edited = format!("{}\n# and then I changed my mind\n", scratch.text()); + assert!( + !file.is_own_echo(edited.as_bytes()), + "suppressing a person's edit is worse than the loop it prevents" + ); + } + + #[test] + fn only_the_most_recent_write_echoes() { + let scratch = Scratch::new(); + let mut file = scratch.file(); + configure( + &mut file, + "appearance.theme", + SettingValue::Text("nord".to_owned()), + ); + let first = scratch.text(); + configure( + &mut file, + "appearance.theme", + SettingValue::Text("gruvbox".to_owned()), + ); + assert!(file.is_own_echo(scratch.text().as_bytes())); + assert!( + !file.is_own_echo(first.as_bytes()), + "content we overwrote reappearing means somebody restored it, and \ + that is news" + ); + } + + #[test] + fn a_fingerprint_covers_length_as_well_as_content() { + assert_ne!( + Fingerprint::of(b"theme"), + Fingerprint::of(b"theme\n"), + "two files that differ only by a trailing newline are two files" + ); + assert_eq!( + Fingerprint::of(b"theme = \"nord\""), + Fingerprint::of(b"theme = \"nord\""), + "the same bytes must fingerprint the same on every run, which is \ + why this is not std's DefaultHasher" + ); + } + + // ---- the reference file is the documentation -------------------------- + + /// The settings the reference file spells out, as dotted paths. + /// + /// `#key = value` is a commented-out setting; `# a sentence` is prose. The + /// space after the `#` is the whole difference, and it is stated at the top + /// of the file itself. + fn documented_paths() -> Vec { + let mut section = String::new(); + let mut paths = Vec::new(); + for line in DEFAULT_TOML.lines() { + let line = line.trim(); + if let Some(name) = line + .strip_prefix('[') + .and_then(|rest| rest.strip_suffix(']')) + { + section = name.to_owned(); + } else if let Some(setting) = commented_setting(line) { + let key = setting.split('=').next().unwrap_or_default().trim(); + paths.push(format!("{section}.{key}")); + } + } + paths + } + + fn commented_setting(line: &str) -> Option<&str> { + let rest = line.strip_prefix('#')?; + // Prose is `# words`; a setting is `#key = value`. + if rest.starts_with(' ') || !rest.contains('=') { + None + } else { + Some(rest) + } + } + + #[test] + fn the_reference_file_documents_every_setting_this_build_has() { + let documented = documented_paths(); + for item in SETTINGS { + assert!( + documented.iter().any(|path| path == item.path), + "{} exists but the reference file never mentions it, so the \ + file that is supposed to be the documentation is not", + item.path + ); + } + } + + #[test] + fn the_reference_file_documents_nothing_this_build_does_not_have() { + for path in documented_paths() { + assert!( + SETTINGS.iter().any(|item| item.path == path), + "the reference file documents {path}, which Lumbridge would \ + ignore — documentation of a key that does nothing is a lie" + ); + } + } + + #[test] + fn uncommenting_the_reference_file_produces_a_document_this_build_understands() { + let mut text = String::new(); + for line in DEFAULT_TOML.lines() { + if let Some(setting) = commented_setting(line.trim()) { + text.push_str(setting); + } else if !line.trim_start().starts_with('#') { + text.push_str(line); + } + text.push('\n'); + } + let (_, unknown) = crate::parse(&text).unwrap_or_else(|error| { + panic!("the documented example values must be valid: {error}\n{text}") + }); + assert!( + unknown.is_empty(), + "every documented key must land somewhere this build reads: {unknown:?}" + ); + } + + #[test] + fn the_reference_file_as_shipped_chooses_nothing() { + let (content, unknown) = crate::parse(DEFAULT_TOML).expect("the reference file is valid"); + assert!(unknown.is_empty(), "unrecognised: {unknown:?}"); + let settings = Settings::resolve(&content, unknown, &MapEnv::default()); + assert_eq!( + settings.theme.origin, + Origin::Default, + "the reference file ships with every key commented out; an active \ + key would make the pane report a value as user-chosen on a \ + machine nobody had configured" + ); + assert_eq!(settings.accent.origin, Origin::Default); + assert_eq!(settings.claude_transcripts.origin, Origin::Default); + assert_eq!(settings.allow_osc52_clipboard.origin, Origin::Default); + } + + #[test] + fn the_reference_file_never_names_a_path_that_must_not_be_a_setting() { + for (path, reason) in FORBIDDEN_PATHS { + assert!( + !DEFAULT_TOML.contains(path), + "the reference file names {path}; someone reading it would \ + believe they could set it, and uncommenting it would be an \ + invitation: {reason}" + ); + } + } + + #[test] + fn the_watcher_is_pointed_at_the_directory_and_not_at_the_file() { + let scratch = Scratch::new(); + let file = scratch.file(); + assert_eq!( + file.watch_directory(), + Some(scratch.directory.path()), + "an atomic write replaces the file by rename, so a watch on the \ + old inode stops seeing anything after the first write" + ); + } + + #[test] + fn a_settings_file_reports_the_path_it_was_opened_on() { + let scratch = Scratch::new(); + assert_eq!(scratch.file().path(), Path::new(&scratch.path())); + } +} diff --git a/crates/lumbridge-settings/src/lib.rs b/crates/lumbridge-settings/src/lib.rs index 133d1b8..586990f 100644 --- a/crates/lumbridge-settings/src/lib.rs +++ b/crates/lumbridge-settings/src/lib.rs @@ -27,14 +27,27 @@ //! file that can redirect where an access token is sent is a credential //! exfiltration path with a friendly name. +//! +//! **The file is edited, never regenerated.** [`file`] rewrites the one key +//! that changed through `toml_edit`, so a hand-written file keeps its comments, +//! its ordering, and any key a newer build wrote that this one does not +//! understand. `assets/default.toml` is the reference file, and it is the +//! documentation: a test asserts it names every setting in [`SETTINGS`]. + #![forbid(unsafe_code)] +pub mod file; pub mod paths; use std::collections::BTreeMap; use serde::{Deserialize, Serialize}; +pub use file::{ + DEFAULT_TOML, LoadOutcome, ParseFailure, SettingValue, SettingsFile, WriteError, WriteOrigin, + load, +}; + /// Who is allowed to write a field. #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum WriteAuthority { diff --git a/crates/lumbridge-storage/src/lib.rs b/crates/lumbridge-storage/src/lib.rs index 8fc7626..371c388 100644 --- a/crates/lumbridge-storage/src/lib.rs +++ b/crates/lumbridge-storage/src/lib.rs @@ -1,8 +1,23 @@ //! Local, device-owned persistence for Lumbridge. //! -//! This database stores workspace metadata and remote routing profiles. API -//! keys, subscription credentials, SSH private keys, and Tailscale credentials -//! are intentionally outside its contract. +//! This database stores workspace metadata, remote routing profiles, and the +//! durable interface state described below. API keys, subscription credentials, +//! SSH private keys, and Tailscale credentials are intentionally outside its +//! contract. +//! +//! # Interface state is not configuration +//! +//! Two kinds of value survive a restart and they do not live in the same place. +//! If a person would want it in a dotfile and copied to their other machine, it +//! is configuration and belongs in `settings.toml`, which `lumbridge-settings` +//! owns. If losing it is *annoying rather than wrong* — the sidebar comes back +//! at its default width instead of the width you dragged it to — it is +//! interface state and belongs here, in a file that is device-local by design +//! and never synchronised. +//! +//! That is what [`UiStateKey`] and [`Store::set_ui_state`] are for. Decision +//! 0021 listed sidebar width, collapsed sections and visibility as the first +//! three, under "Not in this pass". use std::path::Path; @@ -10,21 +25,36 @@ use lumbridge_core::{RemoteHost, RemoteTransport}; use rusqlite::{Connection, OptionalExtension, params}; /// The schema version this build writes and understands. -pub const SUPPORTED_SCHEMA_VERSION: u32 = 2; +pub const SUPPORTED_SCHEMA_VERSION: u32 = 3; -/// The baseline schema. +/// The version-stamp table, created before anything else can be decided. +/// +/// [`Store::migrate`] has to read the stamp before it applies a single +/// statement, so this one table cannot be part of [`SCHEMA`] — it must already +/// exist to answer the question that chooses between creating and migrating. +/// It is a named constant so that the table a v2 file already has and the table +/// a fresh file gets are the same text rather than two copies that can drift. +const APP_META_TABLE: &str = r" +CREATE TABLE IF NOT EXISTS app_meta ( + key TEXT PRIMARY KEY NOT NULL, + value TEXT NOT NULL +) STRICT; +"; + +/// The baseline schema, as of version 2. /// /// Creation only. It no longer stamps the version, because it used to do so /// unconditionally inside the same batch as the `CREATE TABLE IF NOT EXISTS` /// statements — so opening an older file added no columns but flipped the stamp /// forward anyway, and opening a *newer* file silently stamped it back down. /// Both produced a database whose recorded version was a lie. +/// +/// It is deliberately frozen at version 2. Everything a later version adds is +/// its own constant, applied by the fresh-create path and by the migration step +/// that earns it — the *same* constant in both, which is what makes "created +/// fresh" and "upgraded from v2" produce an identical schema instead of two +/// that merely look alike. const SCHEMA: &str = r" -CREATE TABLE IF NOT EXISTS app_meta ( - key TEXT PRIMARY KEY NOT NULL, - value TEXT NOT NULL -) STRICT; - CREATE TABLE IF NOT EXISTS remote_hosts ( id TEXT PRIMARY KEY NOT NULL, label TEXT NOT NULL, @@ -90,6 +120,116 @@ CREATE TABLE IF NOT EXISTS workspace_snapshots ( ) STRICT; "; +/// The `ui_state` table, added at schema version 3. +/// +/// Run exactly once per database: either by the fresh-create path or by the +/// v2 → v3 step, never both. Unlike [`SCHEMA`] it is therefore *not* +/// `IF NOT EXISTS`. A file that already has something called `ui_state` at the +/// moment we would create it is not the file we think it is, and adopting an +/// object of unknown shape because its name happened to match is exactly the +/// class of quiet lie the version stamp was rebuilt to stop. Failing here rolls +/// the whole migration back and leaves the file at the version it really is. +/// +/// `PRIMARY KEY NOT NULL` because `SQLite` still admits a NULL into a `TEXT` +/// primary key otherwise, and a row nobody can name is a row nobody can delete. +/// +/// The `CHECK` is a backstop, not the cap. [`Store::set_ui_state`] rejects in +/// bytes, and `length()` counts characters, so the Rust check is always the +/// stricter of the two and always fires first; this one exists so a value +/// cannot grow without bound through some future path that writes the table +/// directly. +const UI_STATE_TABLE: &str = r" +CREATE TABLE ui_state ( + key TEXT PRIMARY KEY NOT NULL, + value TEXT NOT NULL CHECK (length(value) <= 16384), + updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP +) STRICT; +"; + +/// The largest value [`Store::set_ui_state`] will store, in bytes. +/// +/// Interface state is a width, a flag, a short list of collapsed section names. +/// Sixteen kibibytes is far more than any of those and far less than anything +/// that would make this table a place to park a transcript, a scrollback buffer +/// or a serialised workspace — all of which have their own homes, with their +/// own retention. +pub const MAX_UI_STATE_VALUE_BYTES: usize = 16 * 1024; + +// [`UI_STATE_TABLE`] has shipped, so the 16384 in its CHECK can never be +// edited. Raising the cap above it would turn a clean refusal naming the key +// into a bare constraint violation from SQLite, so the drift is caught here, at +// compile time, rather than by whoever writes the first oversized value. +const _: () = { + const { assert!(MAX_UI_STATE_VALUE_BYTES <= 16384) }; +}; + +/// The complete set of durable interface-state keys. +/// +/// Closed on purpose. The obvious design is a free-form `&str` key, and it +/// means a typo is a silent miss at runtime — you read `sidebar.wdith`, get +/// `None`, and fall back to the default forever without anything going wrong +/// loudly enough to notice. Here a typo does not compile, and the stored names +/// live in one place where a rename is visibly a schema change. +/// +/// Values are opaque strings at this boundary. Storage does not know that a +/// width is a number or that a collapsed-section set is a list; the caller +/// serialises and parses, and must treat an unparseable value as absent rather +/// than as a zero. +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +pub enum UiStateKey { + /// How wide the sidebar was, in pixels, when the drag was released. + SidebarWidth, + /// Whether the sidebar was showing at all. + SidebarVisible, + /// Which sidebar sections the user had folded away. + SidebarCollapsedSections, +} + +impl UiStateKey { + /// Every key, for exhaustive tests and for dumping the table. + /// + /// A `match` in [`Self::storage_key`] makes a *new* key impossible to + /// forget; this array makes it impossible to forget here too, because + /// adding a variant without extending it fails the length assertion in the + /// round-trip test. + pub const ALL: [Self; 3] = [ + Self::SidebarWidth, + Self::SidebarVisible, + Self::SidebarCollapsedSections, + ]; + + /// The name this key is stored under. + /// + /// Part of the on-disk format: changing one of these strings orphans the + /// value a user already has, so it is a migration, not a rename. + #[must_use] + pub const fn storage_key(self) -> &'static str { + match self { + Self::SidebarWidth => "sidebar.width", + Self::SidebarVisible => "sidebar.visible", + Self::SidebarCollapsedSections => "sidebar.collapsed_sections", + } + } + + /// Recover a key from its stored name, or `None` if this build has no such + /// key. + /// + /// `None` is the honest answer for a row written by a newer build: an + /// unknown key is left alone rather than guessed at or deleted. + #[must_use] + pub fn from_storage_key(value: &str) -> Option { + Self::ALL + .into_iter() + .find(|candidate| candidate.storage_key() == value) + } +} + +impl std::fmt::Display for UiStateKey { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter.write_str(self.storage_key()) + } +} + #[derive(Debug)] pub enum StorageError { Database(rusqlite::Error), @@ -111,6 +251,17 @@ pub enum StorageError { MissingMigration { from: u32, }, + /// A durable interface-state value exceeded [`MAX_UI_STATE_VALUE_BYTES`]. + /// + /// Refused whole rather than truncated. A width or a list of collapsed + /// sections cut off in the middle would come back as unparseable at best + /// and as a plausible wrong value at worst, and this table is the one place + /// where losing state is supposed to be merely annoying. + UiStateValueTooLarge { + key: UiStateKey, + bytes: usize, + limit: usize, + }, } impl std::fmt::Display for StorageError { @@ -122,6 +273,10 @@ impl std::fmt::Display for StorageError { formatter, "no migration is defined from schema {from}; this build cannot upgrade the file" ), + Self::UiStateValueTooLarge { key, bytes, limit } => write!( + formatter, + "interface state for {key} is {bytes} bytes, over the {limit}-byte limit" + ), Self::SchemaTooNew { found, supported } => write!( formatter, "this workspace was written by a newer Lumbridge (schema {found}; this build understands {supported})" @@ -140,6 +295,12 @@ impl From for StorageError { pub type Result = std::result::Result; +/// The columns every `remote_hosts` query selects, in order. +/// +/// Named so that the two queries share one reader: a `SELECT` list and a +/// positional `row.get(n)` that disagree is a bug no type checks. +type RemoteHostColumns = (String, String, String, Option, Option, String); + /// The one `SQLite` connection owned by a runtime instance. pub struct Store { connection: Connection, @@ -181,12 +342,7 @@ impl Store { /// The version is read *before* anything is applied, which is the whole /// point: a migration has to know where it is starting from. fn migrate(connection: &Connection) -> Result<()> { - connection.execute_batch( - "CREATE TABLE IF NOT EXISTS app_meta ( - key TEXT PRIMARY KEY NOT NULL, - value TEXT NOT NULL - ) STRICT;", - )?; + connection.execute_batch(APP_META_TABLE)?; let found: Option = connection .query_row( "SELECT value FROM app_meta WHERE key = 'schema_version'", @@ -211,7 +367,15 @@ impl Store { connection.execute_batch("BEGIN IMMEDIATE;")?; let applied = (|| -> Result<()> { if found.is_none() { + // A fresh file is created at the current version directly + // rather than by replaying every step: the steps exist to move + // files that already hold data, and replaying them on an empty + // database is a slower way to reach the same place with more + // chances to diverge. What keeps the two paths honest is that + // each version's tables are one constant, used here and by the + // step below. connection.execute_batch(SCHEMA)?; + connection.execute_batch(UI_STATE_TABLE)?; } // Ordered, forward-only steps. Each takes the database from the // version named to the next one; none may be edited once shipped. @@ -235,14 +399,17 @@ impl Store { /// One forward step, from the named version to the next. /// - /// There are none yet: version 2 is the baseline. An unhandled step is an - /// error rather than a silent success, so raising + /// Version 2 is the baseline; `from = 2` adds the `ui_state` table. An + /// unhandled step is an error rather than a silent success, so raising /// [`SUPPORTED_SCHEMA_VERSION`] without writing the step that earns it /// fails at the first open instead of stamping a version the file has not /// actually reached. A step must never be edited once it has shipped. - fn migration_step(_connection: &Connection, from: u32) -> Result<()> { - // 1 => connection.execute_batch("ALTER TABLE …")?, - Err(StorageError::MissingMigration { from }) + fn migration_step(connection: &Connection, from: u32) -> Result<()> { + match from { + 2 => connection.execute_batch(UI_STATE_TABLE)?, + _ => return Err(StorageError::MissingMigration { from }), + } + Ok(()) } /// Return the currently installed schema version. @@ -301,32 +468,122 @@ impl Store { .query_row( "SELECT id, label, hostname, username, port, transport FROM remote_hosts WHERE id = ?1", [id], - |row| { - Ok(( - row.get::<_, String>(0)?, - row.get::<_, String>(1)?, - row.get::<_, String>(2)?, - row.get::<_, Option>(3)?, - row.get::<_, Option>(4)?, - row.get::<_, String>(5)?, - )) - }, + Self::remote_host_columns, ) .optional()?; - row.map(|(id, label, hostname, username, port, transport)| { - let transport = RemoteTransport::from_storage_name(&transport) - .ok_or_else(|| StorageError::UnknownTransport(transport.clone()))?; - Ok(RemoteHost { - id, - label, - hostname, - username, - port, - transport, - }) + row.map(Self::remote_host_from_columns).transpose() + } + + /// Load every saved remote host profile, ordered by label. + /// + /// Ordered here rather than by the caller so that two lists of the same + /// hosts are never in two different orders. `NOCASE` sorts `alpha` beside + /// `Alpha` instead of putting every capital letter first, which is what a + /// person reading a list of their own machines expects; it folds ASCII + /// only, so a non-ASCII label sorts by code point. The identifier breaks + /// ties, because two hosts may share a label and a list that reshuffles + /// between reads is a list you cannot click. + /// + /// A row naming a transport this build does not know is an error for the + /// whole call rather than a silently dropped host: a devices list that is + /// quietly missing an entry is worse than one that refuses to render. + /// + /// # Errors + /// + /// Returns an error when a row cannot be read or names a transport this + /// build does not understand. + pub fn remote_hosts(&self) -> Result> { + let mut statement = self.connection.prepare( + "SELECT id, label, hostname, username, port, transport + FROM remote_hosts + ORDER BY label COLLATE NOCASE, id", + )?; + let rows = statement.query_map([], Self::remote_host_columns)?; + + let mut hosts = Vec::new(); + for row in rows { + hosts.push(Self::remote_host_from_columns(row?)?); + } + Ok(hosts) + } + + /// The raw column tuple, shared so one query cannot drift from the other. + fn remote_host_columns(row: &rusqlite::Row<'_>) -> rusqlite::Result { + Ok(( + row.get(0)?, + row.get(1)?, + row.get(2)?, + row.get(3)?, + row.get(4)?, + row.get(5)?, + )) + } + + fn remote_host_from_columns(columns: RemoteHostColumns) -> Result { + let (id, label, hostname, username, port, transport) = columns; + let transport = RemoteTransport::from_storage_name(&transport) + .ok_or(StorageError::UnknownTransport(transport))?; + Ok(RemoteHost { + id, + label, + hostname, + username, + port, + transport, }) - .transpose() + } + + /// Read one piece of durable interface state, if it has ever been written. + /// + /// `None` means "never written", and the caller must answer it with its own + /// default rather than with a zero. The value is an opaque string; this + /// boundary does not know what a sidebar width looks like. + /// + /// # Errors + /// + /// Returns an error when `SQLite` cannot read the row. + pub fn ui_state(&self, key: UiStateKey) -> Result> { + self.connection + .query_row( + "SELECT value FROM ui_state WHERE key = ?1", + [key.storage_key()], + |row| row.get(0), + ) + .optional() + .map_err(StorageError::from) + } + + /// Write one piece of durable interface state, replacing any previous + /// value. + /// + /// The caller owns the encoding. Storage stores strings, so that adding a + /// field to the sidebar's own types is not a schema change here. + /// + /// # Errors + /// + /// Returns [`StorageError::UiStateValueTooLarge`] when the value exceeds + /// [`MAX_UI_STATE_VALUE_BYTES`], in which case nothing is written, and a + /// database error when `SQLite` cannot store the row. + pub fn set_ui_state(&self, key: UiStateKey, value: &str) -> Result<()> { + if value.len() > MAX_UI_STATE_VALUE_BYTES { + return Err(StorageError::UiStateValueTooLarge { + key, + bytes: value.len(), + limit: MAX_UI_STATE_VALUE_BYTES, + }); + } + self.connection.execute( + r" + INSERT INTO ui_state (key, value) + VALUES (?1, ?2) + ON CONFLICT(key) DO UPDATE SET + value = excluded.value, + updated_at = CURRENT_TIMESTAMP + ", + params![key.storage_key(), value], + )?; + Ok(()) } /// Save a credential-free UI snapshot for a workspace. @@ -380,19 +637,72 @@ impl Store { } Ok(false) } + + /// Every object the file declares, as `(type, name, sql)`, name-ordered. + /// + /// Ordered by name rather than by creation order precisely because the two + /// paths under test create the same objects in different orders. + #[cfg(test)] + fn schema_objects(&self) -> Result)>> { + let mut statement = self + .connection + .prepare("SELECT type, name, sql FROM sqlite_master ORDER BY name")?; + let rows = statement.query_map([], |row| Ok((row.get(0)?, row.get(1)?, row.get(2)?)))?; + rows.collect::>>() + .map_err(StorageError::from) + } } #[cfg(test)] mod tests { + use std::path::Path; + use lumbridge_core::{RemoteHost, RemoteTransport}; - use super::{SUPPORTED_SCHEMA_VERSION, StorageError, Store}; + use super::{ + APP_META_TABLE, MAX_UI_STATE_VALUE_BYTES, SCHEMA, SUPPORTED_SCHEMA_VERSION, StorageError, + Store, UiStateKey, + }; use rusqlite::Connection; + /// Build the file a version-2 build would have left behind. + /// + /// Deliberately assembled from the same constants that build shipped — + /// [`APP_META_TABLE`] then [`SCHEMA`], stamped by hand — so that "upgraded + /// from v2" in these tests means the real thing and not a v3 file with a + /// table removed. + fn version_two_database(path: &Path) { + let connection = Connection::open(path).expect("a v2 file should open"); + connection + .execute_batch(APP_META_TABLE) + .expect("the stamp table"); + connection.execute_batch(SCHEMA).expect("the v2 baseline"); + connection + .execute( + "INSERT INTO app_meta (key, value) VALUES ('schema_version', '2')", + [], + ) + .expect("stamp version 2"); + } + + fn a_host(id: &str, label: &str) -> RemoteHost { + RemoteHost { + id: id.to_owned(), + label: label.to_owned(), + hostname: format!("{id}.example"), + username: None, + port: None, + transport: RemoteTransport::OpenSsh, + } + } + #[test] fn initializes_schema_and_round_trips_remote_host() { let store = Store::open_in_memory().expect("store should initialize"); - assert_eq!(store.schema_version().expect("schema version"), 2); + assert_eq!( + store.schema_version().expect("schema version"), + SUPPORTED_SCHEMA_VERSION + ); let host = RemoteHost { id: "metal".into(), @@ -535,4 +845,383 @@ mod tests { SUPPORTED_SCHEMA_VERSION ); } + + #[test] + fn a_version_two_database_is_upgraded_in_place_and_keeps_its_rows() { + let directory = tempfile::tempdir().expect("a temporary directory"); + let path = directory.path().join("workspace.db"); + version_two_database(&path); + + let host = a_host("build", "Build box"); + { + let connection = Connection::open(&path).expect("the v2 file"); + connection + .execute( + "INSERT INTO remote_hosts (id, label, hostname, transport) + VALUES (?1, ?2, ?3, 'openssh')", + rusqlite::params![host.id, host.label, host.hostname], + ) + .expect("a row written by the older build"); + } + + let store = Store::open(&path).expect("a v2 file should upgrade, not be refused"); + assert_eq!( + store.schema_version().expect("version"), + 3, + "the upgraded file must claim the version it actually reached" + ); + assert_eq!( + store.remote_host("build").expect("host should load"), + Some(host), + "a migration must carry the user's existing rows forward, not reset them" + ); + + store + .set_ui_state(UiStateKey::SidebarWidth, "312") + .expect("the migrated file should have ui_state"); + assert_eq!( + store.ui_state(UiStateKey::SidebarWidth).expect("read back"), + Some("312".to_owned()) + ); + } + + /// The two ways to reach version 3 must arrive at the same place. + /// + /// A fresh database is created directly rather than by replaying steps, so + /// nothing but this test stops the creation path and the migration path + /// from drifting into two schemas that merely look alike — the failure + /// mode being a column that exists only on machines old enough to have + /// been upgraded. + #[test] + fn an_upgraded_database_has_the_same_schema_as_a_freshly_created_one() { + let directory = tempfile::tempdir().expect("a temporary directory"); + + let fresh_path = directory.path().join("fresh.db"); + let fresh = Store::open(&fresh_path).expect("a fresh store"); + + let upgraded_path = directory.path().join("upgraded.db"); + version_two_database(&upgraded_path); + let upgraded = Store::open(&upgraded_path).expect("an upgraded store"); + + assert_eq!( + fresh.schema_objects().expect("fresh schema"), + upgraded.schema_objects().expect("upgraded schema"), + "creating a v3 file and upgrading a v2 file must produce one schema" + ); + } + + /// The half-way case: the step applied, and then the run stopped. + /// + /// This is the one the transaction exists for. The step's `CREATE TABLE` + /// succeeds and the stamp that would record it does not, which without a + /// transaction leaves a file carrying a v3 table under a v2 stamp — a lie + /// in the other direction, and one that would make the next open try the + /// step again and fail forever. The abort is forced by a temporary trigger + /// so that the failure lands between the two writes, where no natural error + /// could be aimed. + #[test] + fn a_migration_interrupted_after_its_step_rolls_the_step_back_too() { + let directory = tempfile::tempdir().expect("a temporary directory"); + let path = directory.path().join("workspace.db"); + version_two_database(&path); + + { + let connection = Connection::open(&path).expect("the v2 file"); + connection + .execute_batch( + "CREATE TEMP TRIGGER stop_the_stamp BEFORE UPDATE ON main.app_meta + BEGIN SELECT RAISE(ABORT, 'interrupted'); END;", + ) + .expect("a trigger that stops the run between its two writes"); + assert!( + Store::migrate(&connection).is_err(), + "a migration whose stamp cannot be written must not report success" + ); + } + + let connection = Connection::open(&path).expect("reopen"); + let stamp: String = connection + .query_row( + "SELECT value FROM app_meta WHERE key = 'schema_version'", + [], + |row| row.get(0), + ) + .expect("a stamp"); + assert_eq!( + stamp, "2", + "the file is still a version 2 file and must say so" + ); + let tables: i64 = connection + .query_row( + "SELECT count(*) FROM sqlite_master WHERE type = 'table' AND name = 'ui_state'", + [], + |row| row.get(0), + ) + .expect("count"); + assert_eq!( + tables, 0, + "a version 2 stamp over a version 3 table is exactly the half-way state the transaction prevents" + ); + } + + /// An interrupted migration must leave the file at the version it was. + /// + /// The interruption is a real one rather than a simulated panic: a file + /// that already holds an object called `ui_state` makes the v2 → v3 step + /// fail before it can apply. What matters is what is left behind — a file + /// still stamped 2, with its own `ui_state` untouched, that a fixed build + /// can try again on. + #[test] + fn a_migration_that_cannot_apply_leaves_the_file_at_version_two() { + let directory = tempfile::tempdir().expect("a temporary directory"); + let path = directory.path().join("workspace.db"); + version_two_database(&path); + { + let connection = Connection::open(&path).expect("the v2 file"); + connection + .execute_batch("CREATE TABLE ui_state (something_else TEXT NOT NULL) STRICT;") + .expect("an object in the way"); + } + + let Err(error) = Store::open(&path) else { + panic!("a migration that cannot complete must not report success"); + }; + assert!( + matches!(error, StorageError::Database(_)), + "expected the failing statement to surface, got {error}" + ); + + let connection = Connection::open(&path).expect("reopen"); + let stamp: String = connection + .query_row( + "SELECT value FROM app_meta WHERE key = 'schema_version'", + [], + |row| row.get(0), + ) + .expect("a stamp"); + assert_eq!( + stamp, "2", + "a rolled-back migration must leave the file at the version it really is" + ); + + let column: String = connection + .query_row( + "SELECT name FROM pragma_table_info('ui_state')", + [], + |row| row.get(0), + ) + .expect("the pre-existing table"); + assert_eq!( + column, "something_else", + "the rollback must undo the migration's own work, not the file's" + ); + } + + #[test] + fn ui_state_is_absent_until_written_and_then_round_trips() { + let store = Store::open_in_memory().expect("store should initialize"); + assert_eq!( + store + .ui_state(UiStateKey::SidebarCollapsedSections) + .expect("read"), + None, + "a key nobody has written is unknown, and the caller supplies the default" + ); + + store + .set_ui_state(UiStateKey::SidebarCollapsedSections, "detached,quota") + .expect("write"); + assert_eq!( + store + .ui_state(UiStateKey::SidebarCollapsedSections) + .expect("read back") + .as_deref(), + Some("detached,quota") + ); + } + + #[test] + fn writing_a_key_twice_replaces_it_rather_than_failing() { + let store = Store::open_in_memory().expect("store should initialize"); + store + .set_ui_state(UiStateKey::SidebarVisible, "false") + .expect("first write"); + store + .set_ui_state(UiStateKey::SidebarVisible, "true") + .expect("a second write must update the row, not collide with it"); + assert_eq!( + store.ui_state(UiStateKey::SidebarVisible).expect("read"), + Some("true".to_owned()), + "the last write wins; interface state has no history" + ); + } + + #[test] + fn keys_are_independent_of_one_another() { + let store = Store::open_in_memory().expect("store should initialize"); + store + .set_ui_state(UiStateKey::SidebarWidth, "240") + .expect("write"); + assert_eq!( + store.ui_state(UiStateKey::SidebarVisible).expect("read"), + None, + "writing one key must not appear to have written another" + ); + } + + #[test] + fn a_value_at_the_cap_is_stored_and_a_larger_one_is_refused_whole() { + let store = Store::open_in_memory().expect("store should initialize"); + let at_cap = "x".repeat(MAX_UI_STATE_VALUE_BYTES); + store + .set_ui_state(UiStateKey::SidebarCollapsedSections, &at_cap) + .expect("the cap itself is allowed; the limit is inclusive"); + + let over_cap = "y".repeat(MAX_UI_STATE_VALUE_BYTES + 1); + let Err(error) = store.set_ui_state(UiStateKey::SidebarCollapsedSections, &over_cap) else { + panic!("a value over the cap must be refused"); + }; + assert!( + matches!( + error, + StorageError::UiStateValueTooLarge { + key: UiStateKey::SidebarCollapsedSections, + bytes, + limit: MAX_UI_STATE_VALUE_BYTES, + } if bytes == MAX_UI_STATE_VALUE_BYTES + 1 + ), + "expected UiStateValueTooLarge naming the key and the size, got {error}" + ); + assert_eq!( + store + .ui_state(UiStateKey::SidebarCollapsedSections) + .expect("read back"), + Some(at_cap), + "a refused write must leave the previous value intact, not truncate over it" + ); + } + + /// The cap counts bytes, so a multi-byte value cannot slip past it. + #[test] + fn the_cap_counts_bytes_rather_than_characters() { + let store = Store::open_in_memory().expect("store should initialize"); + // Four-byte characters: a quarter of the cap in characters, but one + // byte over it once encoded. + let value = "𝄞".repeat(MAX_UI_STATE_VALUE_BYTES / 4 + 1); + assert!( + store + .set_ui_state(UiStateKey::SidebarWidth, &value) + .is_err(), + "a value whose character count is under the cap but whose byte count is over it must still be refused" + ); + } + + #[test] + fn ui_state_keys_round_trip_through_their_stored_names() { + assert_eq!( + UiStateKey::ALL.len(), + 3, + "a new key must be added to ALL, or nothing here covers it" + ); + for key in UiStateKey::ALL { + assert_eq!( + UiStateKey::from_storage_key(key.storage_key()), + Some(key), + "the stored name of {key} is part of the on-disk format" + ); + } + assert_eq!( + UiStateKey::from_storage_key("sidebar.wdith"), + None, + "a name this build does not know is unknown, never the nearest match" + ); + } + + #[test] + fn remote_hosts_is_empty_rather_than_an_error_when_none_are_saved() { + let store = Store::open_in_memory().expect("store should initialize"); + assert_eq!( + store.remote_hosts().expect("listing"), + Vec::new(), + "no saved hosts is a fact, not a failure" + ); + } + + #[test] + fn remote_hosts_are_listed_ordered_by_label() { + let store = Store::open_in_memory().expect("store should initialize"); + for (id, label) in [ + ("c", "zenith"), + ("a", "Build box"), + ("b", "apex"), + ("d", "apex"), + ] { + store.save_remote_host(&a_host(id, label)).expect("save"); + } + + let labels: Vec<(String, String)> = store + .remote_hosts() + .expect("listing") + .into_iter() + .map(|host| (host.label, host.id)) + .collect(); + assert_eq!( + labels, + vec![ + ("apex".to_owned(), "b".to_owned()), + ("apex".to_owned(), "d".to_owned()), + ("Build box".to_owned(), "a".to_owned()), + ("zenith".to_owned(), "c".to_owned()), + ], + "labels sort case-insensitively, and a shared label is broken by identifier so the order never reshuffles" + ); + } + + /// A host we cannot route to must not be silently omitted from the list. + #[test] + fn a_host_with_an_unreadable_transport_fails_the_listing_rather_than_vanishing() { + let directory = tempfile::tempdir().expect("a temporary directory"); + let path = directory.path().join("workspace.db"); + { + let store = Store::open(&path).expect("a fresh store"); + store + .save_remote_host(&a_host("a", "Build box")) + .expect("save"); + } + { + // This build's CHECK constraint keeps an unknown transport out, so + // the fixture has to be the schema a *newer* build would have left: + // the constraint widened, and a row written against it. A separate + // connection then reads the amended schema. + let connection = Connection::open(&path).expect("the file"); + connection + .execute_batch( + "PRAGMA writable_schema = ON; + UPDATE sqlite_master + SET sql = replace(sql, \"transport IN ('openssh', 'tailscale-ssh')\", \"1\") + WHERE name = 'remote_hosts'; + PRAGMA writable_schema = OFF;", + ) + .expect("widen the constraint the way a newer schema would"); + } + { + let connection = Connection::open(&path).expect("the amended file"); + connection + .execute( + "INSERT INTO remote_hosts (id, label, hostname, transport) + VALUES ('z', 'Zebra', 'z.example', 'quantum-ssh')", + [], + ) + .expect("a row from a build that knows more transports than this one"); + } + + let store = Store::open(&path).expect("reopen"); + let Err(error) = store.remote_hosts() else { + panic!("an unknown transport must be reported, not skipped"); + }; + assert!( + matches!(error, StorageError::UnknownTransport(ref name) if name == "quantum-ssh"), + "expected UnknownTransport naming the value, got {error}" + ); + } }