feat(container): re-wire the companion reaper behind the durable registry
The 2026-08-08 unwire set the bar: a DURABLE record of 'this app is installed' must drive reaping, never runtime inference. installed-apps.json is that record (written on install, cleared on uninstall, backfilled from live containers, held through container absence). The reconciler's companion loop now reaps against it — and only when the registry file actually exists and parses: 'I could not look' and 'nothing is installed' both surface as an empty set from the lossy loader, so a new load_installed_apps_if_recorded keeps the distinction alive. The runtime-derived set is deliberately not a fallback; it is the input class that caused the incident. ORPHAN_GRACE still applies on top. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
dbe37f7ffe
commit
a0ffc5ca3d
@@ -107,6 +107,7 @@ impl BootReconciler {
|
||||
let companion_handle = if self.companion_stage {
|
||||
let orchestrator = self.orchestrator.clone();
|
||||
let interval = self.interval;
|
||||
let data_dir = orchestrator.data_dir().to_path_buf();
|
||||
Some(tokio::spawn(async move {
|
||||
let mut failure_rounds: u32 = 0;
|
||||
loop {
|
||||
@@ -128,34 +129,48 @@ impl BootReconciler {
|
||||
continue;
|
||||
};
|
||||
let failures = crate::container::companion::reconcile(&installed).await;
|
||||
// `reap_orphans` is deliberately NOT called here. It is
|
||||
// implemented and tested, and it must stay unwired until a
|
||||
// DURABLE record of "this app is installed" exists.
|
||||
// Reaper, RE-WIRED 2026-08-10 — driven by the DURABLE
|
||||
// installed-apps registry, never by runtime inference.
|
||||
//
|
||||
// Proven harmful on archi-dev-box 2026-08-08: it removed
|
||||
// archy-bitcoin-ui (36 minutes of no Bitcoin UI, until the
|
||||
// operator reinstalled the backend) and archy-lnd-ui, both
|
||||
// for apps that ARE installed. It was not a logic error —
|
||||
// it did exactly what it was told. The inputs lied: the
|
||||
// backends' containers were missing because of the
|
||||
// clean-exit vanishing bug, and both had already aged out
|
||||
// of running-containers.json, which only ever records what
|
||||
// is CURRENTLY RUNNING. So container-presence and
|
||||
// installation-evidence, the two independent signals the
|
||||
// reaper trusts, were false at the same time and for the
|
||||
// same underlying reason.
|
||||
// History: this call was unwired on 2026-08-08 after it
|
||||
// removed archy-bitcoin-ui and archy-lnd-ui for apps that
|
||||
// WERE installed. Not a logic error — the inputs lied:
|
||||
// `installed_app_ids` infers installation from runtime
|
||||
// state (containers present + running-containers.json),
|
||||
// and the clean-exit vanishing bug falsified both signals
|
||||
// at once. The unwire commit set the re-wire bar: a
|
||||
// durable record of "this app is installed".
|
||||
//
|
||||
// Reaping turns one lost app into two, which is strictly
|
||||
// worse than the orphan it cleans up. Leaving an orphan
|
||||
// costs a stale UI tile; reaping a live app's companion
|
||||
// costs the operator a working screen. Until "installed"
|
||||
// can be answered without inferring it from runtime state,
|
||||
// absence is not evidence of uninstallation.
|
||||
// That record now exists — installed-apps.json, written on
|
||||
// install, cleared on deliberate uninstall, backfilled at
|
||||
// boot from demonstrably-present containers, and immune to
|
||||
// container absence by construction (89b03c47 holds
|
||||
// entries while a container is gone). A vanished backend
|
||||
// no longer looks uninstalled, so the failure mode that
|
||||
// burned archi-dev-box cannot recur through this path.
|
||||
//
|
||||
// The provisioning half above is the actual fix for
|
||||
// "fedimint installs but does not work" and stands on its
|
||||
// own: a companion is never stood up for an app nobody
|
||||
// installed, so no NEW orphans are created.
|
||||
// `None` = the registry could not be read (missing or
|
||||
// corrupt) — which is "I could not look", NOT "nothing is
|
||||
// installed". The reaper stays idle in that case; the
|
||||
// runtime-derived `installed` set above is deliberately
|
||||
// NOT used as a fallback (it is exactly the input class
|
||||
// that caused the 2026-08-08 incident). ORPHAN_GRACE still
|
||||
// applies on top: a companion must be orphaned for the
|
||||
// full grace period before it is touched.
|
||||
if let Some(durable) =
|
||||
crate::crash_recovery::load_installed_apps_if_recorded(&data_dir).await
|
||||
{
|
||||
let durable: Vec<String> = durable.into_iter().collect();
|
||||
for (companion, err) in
|
||||
crate::container::companion::reap_orphans(&durable).await
|
||||
{
|
||||
tracing::warn!(
|
||||
companion = %companion,
|
||||
error = %err,
|
||||
"companion reap failed"
|
||||
);
|
||||
}
|
||||
}
|
||||
for (companion, err) in &failures {
|
||||
tracing::warn!(
|
||||
companion = %companion,
|
||||
|
||||
@@ -682,8 +682,12 @@ fn due_after_grace(
|
||||
|
||||
/// Stop and remove any companion whose backend app is not installed.
|
||||
///
|
||||
/// ⚠️ NOT WIRED, ON PURPOSE. Do not call this from the reconciler until a
|
||||
/// DURABLE record of "this app is installed" exists to drive it.
|
||||
/// ⚠️ WIRED (2026-08-10) to exactly one caller — the boot reconciler's
|
||||
/// companion loop — and ONLY behind the durable installed-apps registry
|
||||
/// (`crash_recovery::load_installed_apps_if_recorded`). That satisfies the
|
||||
/// bar the 2026-08-08 unwire set: a DURABLE record of "this app is
|
||||
/// installed" drives it, never runtime inference. Do not add callers fed
|
||||
/// from runtime state; the history below is why.
|
||||
///
|
||||
/// It ran on archi-dev-box on 2026-08-08 and removed two companions whose
|
||||
/// backends were installed — archy-bitcoin-ui (36 minutes of no Bitcoin UI)
|
||||
|
||||
@@ -1503,6 +1503,10 @@ impl ProdContainerOrchestrator {
|
||||
self.data_dir = data_dir;
|
||||
}
|
||||
|
||||
pub fn data_dir(&self) -> &std::path::Path {
|
||||
&self.data_dir
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub fn set_lnd_paths(&mut self, paths: lnd::EnsurePaths) {
|
||||
self.lnd_paths = paths;
|
||||
|
||||
@@ -204,6 +204,19 @@ pub async fn load_installed_apps(data_dir: &Path) -> std::collections::HashSet<S
|
||||
}
|
||||
}
|
||||
|
||||
/// Like `load_installed_apps`, but keeps "no record" distinguishable from
|
||||
/// "empty record". The companion reaper must only ever run on `Some`:
|
||||
/// "I could not look" and "nothing is installed" both come back as an empty
|
||||
/// set from the lossy loader, yet they demand opposite behaviour — the
|
||||
/// distinction has to survive to the caller (see `reap_orphans`' contract).
|
||||
pub async fn load_installed_apps_if_recorded(
|
||||
data_dir: &Path,
|
||||
) -> Option<std::collections::HashSet<String>> {
|
||||
let path = data_dir.join(INSTALLED_APPS_FILE);
|
||||
let content = fs::read_to_string(&path).await.ok()?;
|
||||
serde_json::from_str(&content).ok()
|
||||
}
|
||||
|
||||
async fn save_installed_apps(data_dir: &Path, installed: &std::collections::HashSet<String>) {
|
||||
let path = data_dir.join(INSTALLED_APPS_FILE);
|
||||
if let Ok(json) = serde_json::to_string_pretty(installed) {
|
||||
@@ -1193,6 +1206,33 @@ mod tests {
|
||||
use super::*;
|
||||
use tempfile::TempDir;
|
||||
|
||||
#[tokio::test]
|
||||
async fn if_recorded_distinguishes_no_record_from_empty_record() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
// No file: the reaper must see "could not look", never "empty".
|
||||
assert!(load_installed_apps_if_recorded(tmp.path()).await.is_none());
|
||||
// Corrupt file: same — refuse to answer rather than guess.
|
||||
tokio::fs::write(tmp.path().join(INSTALLED_APPS_FILE), "{ not json")
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(load_installed_apps_if_recorded(tmp.path()).await.is_none());
|
||||
// A real (even empty) record answers.
|
||||
tokio::fs::write(tmp.path().join(INSTALLED_APPS_FILE), "[]")
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
load_installed_apps_if_recorded(tmp.path()).await,
|
||||
Some(std::collections::HashSet::new())
|
||||
);
|
||||
mark_installed(tmp.path(), "bitcoin-knots").await;
|
||||
assert!(
|
||||
load_installed_apps_if_recorded(tmp.path())
|
||||
.await
|
||||
.unwrap()
|
||||
.contains("bitcoin-knots")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn installed_record_survives_and_forgets_on_uninstall() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
|
||||
Reference in New Issue
Block a user