fix(mesh): configure step after a successful flash could race and fail

Traced a real "Operation failed" on the post-flash configure step: the
flash job's own post-completion code unconditionally called
MeshService::start() after configure(), regardless of the persisted
enabled flag. In this incident, the user had toggled mesh off shortly
before the flash (config.enabled saved as false), but the job's forced
start() ran anyway — leaving the listener actually running while
self.config.enabled stayed false. When the user then clicked "Keep As Is"
on the fresh post-flash probe, their mesh.configure call correctly saw a
false→true transition and tried to start the listener itself, hitting
"Mesh listener already running" — the listener had already been
force-started behind the config's back.

Two fixes:
1. The flash job now only restarts the listener if the freshly-loaded
   config still says enabled — respecting a user's own concurrent choice
   to disable mesh instead of silently overriding it.
2. MeshService::start() is now idempotent: finding the listener already
   running is treated as success, not an error. Ensuring the listener is
   running is what every caller actually wants; whichever caller's start()
   wins a race, the other finding it already satisfied is the correct
   outcome, not a failure to surface to the user.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
ssmithx 2026-07-23 16:59:41 +00:00
parent d6019e47a5
commit 7547d03166
2 changed files with 32 additions and 8 deletions

View File

@ -427,15 +427,31 @@ pub async fn start_flash_job(
let mut svc = bg_service.write().await;
if let Some(s) = svc.as_mut() {
if let Ok(config) = super::load_config(&data_dir).await {
match super::load_config(&data_dir).await {
Ok(config) => {
// Only resume if mesh is actually still enabled per the
// CURRENT persisted config — confirmed live 2026-07-23:
// unconditionally forcing a restart here, regardless of
// `enabled`, overrode a user's own concurrent "disable
// mesh" toggle and left the listener running while
// config said disabled. That inconsistent state is what
// made a later legitimate "Keep As Is" click (which
// correctly tries to start on a false→true transition)
// fail with "already running" — the listener had already
// been force-started behind the config's back.
let should_run = config.enabled;
if let Err(e) = s.configure(config).await {
warn!(error = %e, "Failed to resume mesh listener after flash");
}
}
if should_run {
if let Err(e) = s.start() {
warn!(error = %e, "Failed to restart mesh listener after flash");
}
}
}
Err(e) => warn!(error = %e, "Failed to load mesh config after flash"),
}
}
});
*job.abort_handle.write().await = Some(task.abort_handle());

View File

@ -735,10 +735,18 @@ impl MeshService {
self.server_name = name;
}
/// Start the background mesh listener.
/// Start the background mesh listener. Idempotent: if the listener is
/// already running, this is a harmless no-op rather than an error —
/// confirmed live 2026-07-23, a real race between the flash job's own
/// post-flash restart and a concurrent user "Keep As Is" click (both
/// legitimately trying to ensure the listener is running) surfaced this
/// as a user-facing "Mesh listener already running" RPC error. Ensuring
/// the listener is running is the intent every caller actually has;
/// whichever caller's start() happens to win the race, the other
/// finding it already satisfied is success, not failure.
pub fn start(&mut self) -> Result<()> {
if self.listener_handle.is_some() {
anyhow::bail!("Mesh listener already running");
return Ok(());
}
let (shutdown_tx, shutdown_rx) = watch::channel(false);