Migrate an old config instead of refusing to start
The AI Sessions backend was crash-looping on the host, and I caused it. A config written before setups existed makes `Config::load` bail, the process exits 1 immediately, and under OpenRC's `command_background=true` that presents as a service that will not stay up. The refusal was deliberate and it was the wrong trade. I chose it to avoid silently emptying a config and re-seeding over it -- a real hazard -- but weighed it against the wrong cost. This process is how a phone reaches that machine at all, so refusing to run strands the person who would have to fix it, at a terminal, on the machine they were trying to avoid needing. And what it was protecting is the cheap half: providers and hosts are rediscoverable now, while the half that genuinely cannot be recovered -- the enrolled token hashes -- survives a migration untouched. So it migrates. Each old host becomes a setup keeping its name, since that is what sessions referenced; the top-level providers belong to the machine this server runs on; and every session's host becomes its setup, so conversations keep working. The original is copied to `config.ron.pre-setups` first, because this is a one-way conversion of the only record of what was configured and one file makes it reversible by hand. **Migrated hosts arrive with no providers, deliberately.** The old file never recorded which machine had which program -- that was the flaw the setups model exists to fix -- so inventing an answer would recreate exactly the impossible pairings it was meant to end. Rediscover asks the machine. Both driver-kind spellings are read. The kebab rename and the RON move landed on the same day, so a file written that morning says `r#claude-cli` and one from the afternoon says `claude_cli`; reading only one would have turned this fix into a different crash. Verified against a host-shaped config: the server starts, the token and both sessions survive, the remote session points at the migrated setup and the local one at `local`, the original is kept, and a second start is an ordinary load that neither migrates again nor overwrites the backup. Found by Iris, who had to check `rc-service` by hand because the card reported it as merely stopped -- dev-updater's session is adding a `failed` state for that separately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017xn8nHw1tw1R6PtiY1eEtw
This commit is contained in:
1 parent
4d162331e3
commit
5ccadffeaa
1 file changed
+257
-21
+257
-21
@@ -18,7 +18,7 @@
|
||||
use std::collections::BTreeMap;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
use anyhow::{Context, Result, bail};
|
||||
use anyhow::{Context, Result};
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use crate::private;
|
||||
@@ -242,6 +242,103 @@ pub struct SessionConfig {
|
||||
pub created: f64,
|
||||
}
|
||||
|
||||
/// The shape `config.ron` had before setups (up to 2026-08-28).
|
||||
///
|
||||
/// Here only so an existing install upgrades rather than refusing to
|
||||
/// start. Nothing else may use these types, and when no install can
|
||||
/// plausibly still be on that shape they go, along with the branch in
|
||||
/// [`Config::load`] that reaches them.
|
||||
mod legacy {
|
||||
use serde::Deserialize;
|
||||
|
||||
use super::{DriverKind, ProviderConfig, TokenEntry};
|
||||
|
||||
#[derive(Deserialize, Default)]
|
||||
#[serde(rename_all = "camelCase", default)]
|
||||
pub struct Config {
|
||||
pub tokens: Vec<TokenEntry>,
|
||||
pub providers: Vec<Provider>,
|
||||
pub hosts: Vec<Host>,
|
||||
pub sessions: Vec<Session>,
|
||||
}
|
||||
|
||||
/// A provider, when the driver kind was spelled in kebab case.
|
||||
///
|
||||
/// Both spellings are accepted because the rename happened on the same
|
||||
/// day as the RON move, so a config written that morning says
|
||||
/// `r#claude-cli` and one written that afternoon says `claude_cli`.
|
||||
/// Reading only one of them would turn this migration into a different
|
||||
/// crash.
|
||||
#[derive(Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct Provider {
|
||||
pub name: String,
|
||||
pub kind: LegacyKind,
|
||||
#[serde(default)]
|
||||
pub command: Option<String>,
|
||||
#[serde(default)]
|
||||
pub models: Vec<String>,
|
||||
}
|
||||
|
||||
#[derive(Deserialize, Clone, Copy)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
pub enum LegacyKind {
|
||||
Echo,
|
||||
#[serde(alias = "claude-cli")]
|
||||
ClaudeCli,
|
||||
#[serde(alias = "llama-cpp")]
|
||||
LlamaCpp,
|
||||
}
|
||||
|
||||
impl From<Provider> for ProviderConfig {
|
||||
fn from(old: Provider) -> Self {
|
||||
Self {
|
||||
name: old.name,
|
||||
kind: match old.kind {
|
||||
LegacyKind::Echo => DriverKind::Echo,
|
||||
LegacyKind::ClaudeCli => DriverKind::ClaudeCli,
|
||||
LegacyKind::LlamaCpp => DriverKind::LlamaCpp,
|
||||
},
|
||||
command: old.command,
|
||||
models: old.models,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// A machine, when the name lived on the connection rather than on a
|
||||
/// setup.
|
||||
#[derive(Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct Host {
|
||||
pub name: String,
|
||||
pub address: String,
|
||||
#[serde(default)]
|
||||
pub port: Option<u16>,
|
||||
#[serde(default)]
|
||||
pub identity_file: Option<std::path::PathBuf>,
|
||||
#[serde(default)]
|
||||
pub options: Vec<String>,
|
||||
}
|
||||
|
||||
/// A session, when it named a provider and a host independently.
|
||||
#[derive(Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct Session {
|
||||
pub id: String,
|
||||
pub provider: String,
|
||||
#[serde(default)]
|
||||
pub host: Option<String>,
|
||||
pub title: String,
|
||||
#[serde(default)]
|
||||
pub model: Option<String>,
|
||||
#[serde(default)]
|
||||
pub cwd: Option<std::path::PathBuf>,
|
||||
#[serde(default)]
|
||||
pub permission_mode: Option<String>,
|
||||
pub created: f64,
|
||||
}
|
||||
}
|
||||
|
||||
/// The name of the echo provider, and of the setup this machine gets on
|
||||
/// first run.
|
||||
///
|
||||
@@ -303,17 +400,20 @@ impl Config {
|
||||
match std::fs::read_to_string(path) {
|
||||
Ok(text) => {
|
||||
// `Config` defaults unknown fields away, so a file from
|
||||
// before setups existed would load as "no setups at all"
|
||||
// and be re-seeded over -- losing every configured
|
||||
// provider and host without a word. Say so instead.
|
||||
// before setups existed would otherwise load as "no setups
|
||||
// at all" and be re-seeded over, losing what was
|
||||
// configured without a word.
|
||||
//
|
||||
// This used to refuse to start, which was the wrong trade
|
||||
// and took the server down on the machine it was meant to
|
||||
// protect: this process is how a phone reaches that
|
||||
// machine at all, so refusing to run strands the person
|
||||
// who would have to fix it. Migrating keeps the one thing
|
||||
// that genuinely cannot be recovered -- the enrolled token
|
||||
// hashes -- and rebuilds the rest, which is now
|
||||
// discoverable anyway.
|
||||
if text.contains("\nproviders:") || text.contains("\nhosts:") {
|
||||
bail!(
|
||||
"{} is in the old shape: `providers` and `hosts` were separate lists, \
|
||||
and are now `setups`, each carrying the providers that machine has. \
|
||||
Rewrite it as `setups: [(name: \"...\", providers: [...])]` -- a \
|
||||
setup with no `ssh` is this machine.",
|
||||
path.display(),
|
||||
);
|
||||
return Self::migrate_from_pre_setups(path, &text);
|
||||
}
|
||||
format::parse(&text)
|
||||
.with_context(|| format!("{} is not valid config RON", path.display()))
|
||||
@@ -328,6 +428,96 @@ impl Config {
|
||||
}
|
||||
}
|
||||
|
||||
/// Reads a config written before setups existed and rewrites it in
|
||||
/// the current shape, keeping a copy of the original.
|
||||
///
|
||||
/// The old model had `providers` and `hosts` as independent lists and
|
||||
/// a session naming one of each. The mapping is mechanical: each host
|
||||
/// becomes a setup, the top-level providers belong to the machine this
|
||||
/// server runs on, and a session's host becomes its setup.
|
||||
///
|
||||
/// Migrated hosts arrive with **no providers**, deliberately. The old
|
||||
/// file never recorded which machine had which program -- that was the
|
||||
/// flaw the setups model exists to fix -- so inventing an answer here
|
||||
/// would recreate exactly the impossible pairings this was meant to
|
||||
/// end. `Rediscover` on the card asks the machine instead.
|
||||
///
|
||||
/// Writes as a side effect, which a loader would not normally do: this
|
||||
/// runs once, and leaving the old file in place would mean migrating
|
||||
/// again on every start and never recording the result.
|
||||
fn migrate_from_pre_setups(path: &Path, text: &str) -> Result<Self> {
|
||||
let old: legacy::Config = format::parse(text)
|
||||
.with_context(|| format!("{} is not valid config RON", path.display()))?;
|
||||
|
||||
let mut setups = Vec::new();
|
||||
setups.push(SetupConfig {
|
||||
id: LOCAL_SETUP_ID.to_string(),
|
||||
name: LOCAL_SETUP.to_string(),
|
||||
ssh: None,
|
||||
providers: old.providers.into_iter().map(Into::into).collect(),
|
||||
});
|
||||
// Name and id both taken from the old host's name: it is what
|
||||
// sessions referenced, so reusing it is what lets them keep
|
||||
// working without a second lookup table.
|
||||
for host in &old.hosts {
|
||||
setups.push(SetupConfig {
|
||||
id: crate::setups::id_from(&host.name),
|
||||
name: host.name.clone(),
|
||||
ssh: Some(SshConfig {
|
||||
address: host.address.clone(),
|
||||
port: host.port,
|
||||
identity_file: host.identity_file.clone(),
|
||||
options: host.options.clone(),
|
||||
}),
|
||||
providers: Vec::new(),
|
||||
});
|
||||
}
|
||||
|
||||
let sessions = old
|
||||
.sessions
|
||||
.into_iter()
|
||||
.map(|session| SessionConfig {
|
||||
id: session.id,
|
||||
setup: match &session.host {
|
||||
Some(name) => crate::setups::id_from(name),
|
||||
None => LOCAL_SETUP_ID.to_string(),
|
||||
},
|
||||
provider: session.provider,
|
||||
title: session.title,
|
||||
model: session.model,
|
||||
cwd: session.cwd,
|
||||
permission_mode: session.permission_mode,
|
||||
params: BTreeMap::new(),
|
||||
created: session.created,
|
||||
})
|
||||
.collect();
|
||||
|
||||
let migrated = Self {
|
||||
tokens: old.tokens,
|
||||
setups,
|
||||
sessions,
|
||||
};
|
||||
|
||||
// The original is kept rather than overwritten in place: this is a
|
||||
// one-way conversion of the only record of what was configured,
|
||||
// and it costs one file to make it reversible by hand.
|
||||
let backup = path.with_extension("ron.pre-setups");
|
||||
std::fs::copy(path, &backup)
|
||||
.with_context(|| format!("back up {} to {}", path.display(), backup.display()))?;
|
||||
migrated.save(path)?;
|
||||
tracing::warn!(
|
||||
"{} was in the pre-setups shape and has been migrated: {} token(s) and {} session(s) \
|
||||
kept, {} host(s) became setups with no providers yet -- press Rediscover on each to \
|
||||
ask it what it has. The original is at {}.",
|
||||
path.display(),
|
||||
migrated.tokens.len(),
|
||||
migrated.sessions.len(),
|
||||
old.hosts.len(),
|
||||
backup.display(),
|
||||
);
|
||||
Ok(migrated)
|
||||
}
|
||||
|
||||
/// Writes the config, owner-readable only.
|
||||
///
|
||||
/// The token hashes here are verifiers, not secrets -- a 256-bit
|
||||
@@ -474,22 +664,68 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
/// A config from before setups existed must not load as an empty one.
|
||||
/// `Config` defaults unknown fields away, so without this check the
|
||||
/// old `providers` and `hosts` would vanish and be silently re-seeded
|
||||
/// over -- the worst kind of migration, the sort nobody notices.
|
||||
fn a_config_in_the_old_shape_is_refused_rather_than_emptied() {
|
||||
/// The upgrade path, and the reason it exists: refusing to start took
|
||||
/// the whole server down on an install that had done nothing wrong.
|
||||
/// What must survive is the token hashes -- everything else can be
|
||||
/// rebuilt, and losing those means re-enrolling every device by hand
|
||||
/// at the terminal, which is the position this server exists to keep
|
||||
/// people out of.
|
||||
fn a_config_from_before_setups_is_migrated_rather_than_refused() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let path = dir.path().join("config.ron");
|
||||
std::fs::write(
|
||||
&path,
|
||||
"tokens: [],\nproviders: [\n (name: \"claude-cli\", kind: claude_cli),\n],\nhosts: [],\nsessions: [],\n",
|
||||
// `r#claude-cli` deliberately: the kebab spelling and the RON
|
||||
// move landed the same day, so a real file may have either.
|
||||
"tokens: [(name: \"phone\", sha256: \"ab\")],\n\
|
||||
providers: [(name: \"claude-cli\", kind: r#claude-cli, models: [\"opus\"])],\n\
|
||||
hosts: [(name: \"dev vm\", address: \"bob@10.0.0.2\", port: 2222)],\n\
|
||||
sessions: [\n\
|
||||
(id: \"a\", provider: \"claude-cli\", host: \"dev vm\", title: \"remote\", created: 1.0),\n\
|
||||
(id: \"b\", provider: \"claude-cli\", title: \"local\", created: 2.0),\n\
|
||||
],\n",
|
||||
)
|
||||
.expect("write");
|
||||
let err = Config::load(&path).expect_err("should refuse");
|
||||
let message = format!("{err:#}");
|
||||
assert!(message.contains("old shape"), "{message}");
|
||||
assert!(message.contains("setups"), "{message}");
|
||||
|
||||
let migrated = Config::load(&path).expect("migrates rather than refusing");
|
||||
|
||||
// The irreplaceable part.
|
||||
assert_eq!(migrated.tokens.len(), 1);
|
||||
assert_eq!(migrated.tokens[0].sha256, "ab");
|
||||
|
||||
// The old host is a setup, keeping the name sessions referenced.
|
||||
let vm = migrated.setup("dev-vm").expect("host became a setup");
|
||||
assert_eq!(vm.name, "dev vm");
|
||||
assert_eq!(vm.ssh.as_ref().expect("ssh").port, Some(2222));
|
||||
// Empty on purpose: the old file never recorded what was on which
|
||||
// machine, so this asks rather than invents.
|
||||
assert!(vm.providers.is_empty());
|
||||
|
||||
// Top-level providers belonged to the machine running the server.
|
||||
let local = migrated.setup(LOCAL_SETUP_ID).expect("local setup");
|
||||
assert_eq!(
|
||||
local.provider("claude-cli").expect("kept").kind,
|
||||
DriverKind::ClaudeCli
|
||||
);
|
||||
|
||||
// Sessions keep working, each pointing at the right machine.
|
||||
let remote = migrated
|
||||
.sessions
|
||||
.iter()
|
||||
.find(|s| s.id == "a")
|
||||
.expect("remote");
|
||||
assert_eq!(remote.setup, "dev-vm");
|
||||
let local_session = migrated
|
||||
.sessions
|
||||
.iter()
|
||||
.find(|s| s.id == "b")
|
||||
.expect("local");
|
||||
assert_eq!(local_session.setup, LOCAL_SETUP_ID);
|
||||
|
||||
// The original is kept, and a second load is an ordinary one.
|
||||
assert!(path.with_extension("ron.pre-setups").is_file());
|
||||
let again = Config::load(&path).expect("reload");
|
||||
assert_eq!(again.setups.len(), 2);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in new issue
Block a user