fix: bound runner disconnect recovery (#10667)

* fix: bound runner disconnect recovery

* fix: expose local runner recovery

* docs: sync runner command reference

Author: chubes4

crates/homeboy-cli/src/commands/runner/cli.rs

@@ -285,6 +285,10 @@ pub(super) enum RunnerCommand {
     Disconnect {
         /// Runner ID
         id: String,
+
+        /// Remove only this controller's matching local tunnel/session state without contacting the remote runner
+        #[arg(long)]
+        local_recovery: bool,
     },
     /// Build or select the Homeboy binary used for runner/Lab jobs
     RefreshHomeboy {

crates/homeboy-cli/src/commands/runner/dispatch.rs

@@ -170,7 +170,9 @@ pub fn run(args: RunnerArgs) -> CmdResult<RunnerCommandOutput> {
             generations,
             full,
         } => map_registry(status_mod::status(id.as_deref(), generations, full)),
-        RunnerCommand::Disconnect { id } => map_registry(registry::disconnect(&id)),
+        RunnerCommand::Disconnect { id, local_recovery } => {
+            map_registry(registry::disconnect(&id, local_recovery))
+        }
         RunnerCommand::RefreshHomeboy {
             runner_id,
             select,

crates/homeboy-cli/src/commands/runner/registry.rs

@@ -412,13 +412,17 @@ pub(super) fn connect(id: &str, input: RunnerConnectInput) -> CmdResult<RunnerOu
     ))
 }
 
-pub(super) fn disconnect(id: &str) -> CmdResult<RunnerOutput> {
+pub(super) fn disconnect(id: &str, local_recovery: bool) -> CmdResult<RunnerOutput> {
     Ok((
         RunnerOutput {
             command: "runner.disconnect".to_string(),
             id: Some(id.to_string()),
             extra: RunnerExtra {
-                connection: Some(RunnerConnectionOutput::Disconnect(runner::disconnect(id)?)),
+                connection: Some(RunnerConnectionOutput::Disconnect(if local_recovery {
+                    runner::disconnect_local_recovery(id)?
+                } else {
+                    runner::disconnect(id)?
+                })),
                 ..Default::default()
             },
             ..Default::default()

crates/homeboy-lab-runner/src/connection.rs

@@ -2734,6 +2734,10 @@ pub fn disconnect(runner_id: &str) -> Result<RunnerDisconnectReport> {
     stop_transport_recovery::disconnect_with_force(runner_id, false)
 }
 
+pub fn disconnect_local_recovery(runner_id: &str) -> Result<RunnerDisconnectReport> {
+    stop_transport_recovery::disconnect_local_recovery(runner_id)
+}
+
 #[cfg(test)]
 mod indexed_inspection_tests {
     #[test]

crates/homeboy-lab-runner/src/connection/session_store.rs

@@ -293,6 +293,13 @@ pub(super) fn remove_session(runner_id: &str) -> Result<()> {
     Ok(())
 }
 
+/// Delete only the snapshot this controller inspected. A changed session is a
+/// new connection and must be left for its owner to manage.
+pub(super) fn remove_session_if_matches(runner_id: &str, expected: &RunnerSession) -> Result<bool> {
+    let path = session_path(runner_id)?;
+    remove_session_at_if_matches(&path, expected)
+}
+
 pub(super) fn remove_ownership(runner_id: &str) -> Result<()> {
     let path = ownership_path(runner_id)?;
     if path.exists() {
@@ -303,6 +310,24 @@ pub(super) fn remove_ownership(runner_id: &str) -> Result<()> {
     Ok(())
 }
 
+pub(super) fn remove_ownership_if_matches(
+    runner_id: &str,
+    expected: &RunnerSession,
+) -> Result<bool> {
+    let path = ownership_path(runner_id)?;
+    remove_session_at_if_matches(&path, expected)
+}
+
+fn remove_session_at_if_matches(path: &PathBuf, expected: &RunnerSession) -> Result<bool> {
+    if read_session_at(path)? != Some(expected.clone()) {
+        return Ok(false);
+    }
+    std::fs::remove_file(path).map_err(|err| {
+        Error::internal_io(err.to_string(), Some(format!("delete {}", path.display())))
+    })?;
+    Ok(true)
+}
+
 pub(super) fn has_live_peer_session(session: &RunnerSession) -> Result<bool> {
     let directory = paths::runner_sessions_dir()?.join(&session.runner_id);
     has_live_peer_session_in(&directory, session, session_is_live)
@@ -445,6 +470,21 @@ mod tests {
         }
     }
 
+    #[test]
+    fn compare_and_delete_retains_a_changed_controller_session() {
+        let root = TempDir::new().expect("session directory");
+        let path = root.path().join("controller.json");
+        let recorded = session("controller", "lease-recorded");
+        let replacement = session("controller", "lease-reconnected");
+        write_session_at(&path, &replacement).expect("write replacement session");
+
+        assert!(!remove_session_at_if_matches(&path, &recorded).expect("compare session"));
+        assert_eq!(
+            read_session_at(&path).expect("read replacement session"),
+            Some(replacement)
+        );
+    }
+
     fn serve_health(
         lease_id: &str,
         pid: u32,

crates/homeboy-lab-runner/src/connection_stop_transport_recovery.rs

@@ -22,6 +22,66 @@ pub(crate) fn disconnect_with_force(
     disconnect_with_session(runner_id, None, force)
 }
 
+/// Recover the controller-owned side of a wedged tunnel without observing or
+/// mutating the remote daemon. Remote jobs intentionally remain ambiguous.
+pub(crate) fn disconnect_local_recovery(runner_id: &str) -> Result<RunnerDisconnectReport> {
+    let session = read_session(runner_id)?;
+    let session_path = session_path(runner_id)?.display().to_string();
+    if let Some(session) = session.as_ref() {
+        if session.mode == RunnerTunnelMode::DirectSsh {
+            if let Some(pid) = session.tunnel_pid {
+                terminate_pid(pid);
+            }
+        }
+        let removed = remove_session_if_matches(runner_id, session)?;
+        if removed {
+            let _ = remove_ownership_if_matches(runner_id, session)?;
+        }
+        return Ok(RunnerDisconnectReport {
+            runner_id: runner_id.to_string(),
+            disconnected: removed,
+            partial: true,
+            remote_error: Some(
+                "remote daemon was not contacted; its jobs and lifecycle remain ambiguous"
+                    .to_string(),
+            ),
+            local_recovery_command: None,
+            session: (!removed).then(|| session.clone()),
+            session_path,
+        });
+    }
+    Ok(RunnerDisconnectReport {
+        runner_id: runner_id.to_string(),
+        disconnected: false,
+        partial: true,
+        remote_error: Some(
+            "no controller-local session was present; remote daemon was not contacted".to_string(),
+        ),
+        local_recovery_command: None,
+        session: None,
+        session_path,
+    })
+}
+
+fn partial_disconnect_report(
+    runner_id: &str,
+    session: Option<RunnerSession>,
+    remote_error: impl Into<String>,
+) -> Result<RunnerDisconnectReport> {
+    Ok(RunnerDisconnectReport {
+        runner_id: runner_id.to_string(),
+        disconnected: false,
+        partial: true,
+        remote_error: Some(remote_error.into()),
+        local_recovery_command: Some(format!(
+            "homeboy runner disconnect {} --local-recovery",
+            shell::quote_arg(runner_id)
+        )),
+        session,
+        session_path: session_path(runner_id)?.display().to_string(),
+    })
+}
+
 /// Stop the daemon through the current live session after confirming it still
 /// owns the remote daemon observed by a caller's promotion transaction.
 pub(crate) fn disconnect_with_session(
@@ -66,8 +126,14 @@ pub(crate) fn disconnect_with_session(
         // SSH and clean up stale local tunnel processes only after its stop.
         let retained_generations =
             super::super::generation_store::live_sessions(runner_id, Some(session))?;
+        let authoritative_status = match probe_authoritative_daemon_status(runner_id) {
+            Ok(status) => status,
+            Err(error) => {
+                return partial_disconnect_report(runner_id, session.clone().into(), error.message)
+            }
+        };
         if remote_daemon::authoritative_stale_generations_are_dead(
-            &probe_authoritative_daemon_status(runner_id)?,
+            &authoritative_status,
             &eligible_stale_generation_leases(&retained_generations).unwrap_or_default(),
         ) {
             let leases =
@@ -78,13 +144,23 @@ pub(crate) fn disconnect_with_session(
             return Ok(RunnerDisconnectReport {
                 runner_id: runner_id.to_string(),
                 disconnected: true,
+                partial: false,
+                remote_error: None,
+                local_recovery_command: None,
                 session: None,
                 session_path: session_path(runner_id)?.display().to_string(),
             });
         }
-        if let Some(authoritative_session) =
-            reconcile_authoritative_idle_stale_generations(runner_id, &retained_generations)?
-        {
+        let authoritative_session = match reconcile_authoritative_idle_stale_generations(
+            runner_id,
+            &retained_generations,
+        ) {
+            Ok(session) => session,
+            Err(error) => {
+                return partial_disconnect_report(runner_id, session.clone().into(), error.message)
+            }
+        };
+        if let Some(authoritative_session) = authoritative_session {
             *session = authoritative_session.clone();
         }
         let mut reconciled_tunnel_pids = retained_generations
@@ -115,12 +191,18 @@ pub(crate) fn disconnect_with_session(
             }
         }
         if !unresolved.is_empty() {
-            return Err(Error::validation_invalid_argument(
-                "disconnect",
-                format!("runner `{runner_id}` has unresolved daemon generations; sessions and ledger were retained"),
-                Some(runner_id.to_string()),
-                Some(unresolved.into_iter().map(|entry| entry.to_string()).collect()),
-            ));
+            return partial_disconnect_report(
+                runner_id,
+                session.clone().into(),
+                format!(
+                    "remote daemon stop was not proven; sessions and ledger were retained: {}",
+                    unresolved
+                        .into_iter()
+                        .map(|entry| entry.to_string())
+                        .collect::<Vec<_>>()
+                        .join(", ")
+                ),
+            );
         }
         for pid in reconciled_tunnel_pids {
             terminate_pid(pid);
@@ -136,12 +218,17 @@ pub(crate) fn disconnect_with_session(
         // new generation merely to clean that already-dead inventory.
         let retained_generations = super::super::generation_store::live_sessions(runner_id, None)?;
         let leases = eligible_stale_generation_leases(&retained_generations).unwrap_or_default();
-        if !leases.is_empty()
-            && remote_daemon::authoritative_stale_generations_are_dead(
-                &probe_authoritative_daemon_status(runner_id)?,
-                &leases,
-            )
-        {
+        let authoritative_status = if leases.is_empty() {
+            None
+        } else {
+            match probe_authoritative_daemon_status(runner_id) {
+                Ok(status) => Some(status),
+                Err(error) => return partial_disconnect_report(runner_id, None, error.message),
+            }
+        };
+        if authoritative_status.as_ref().is_some_and(|status| {
+            remote_daemon::authoritative_stale_generations_are_dead(status, &leases)
+        }) {
             super::super::generation_store::tombstone_dead_direct_generations(runner_id, &leases)?;
             remove_ownership(runner_id)?;
         }
@@ -150,6 +237,9 @@ pub(crate) fn disconnect_with_session(
     Ok(RunnerDisconnectReport {
         runner_id: runner_id.to_string(),
         disconnected: session.is_some(),
+        partial: false,
+        remote_error: None,
+        local_recovery_command: None,
         session,
         session_path: session_path(runner_id)?.display().to_string(),
     })
@@ -880,6 +970,29 @@ mod tests {
         );
     }
 
+    #[test]
+    fn local_recovery_removes_only_the_controller_session_without_remote_access() {
+        homeboy_core::test_support::with_isolated_home(|_| {
+            let mut session = direct_ssh_session("lease-wedged");
+            session.tunnel_pid = None;
+            write_session(&session).expect("record controller session");
+            write_ownership(&session).expect("record ownership");
+
+            let report = disconnect_local_recovery("homeboy-lab").expect("local recovery");
+
+            assert!(report.disconnected);
+            assert!(report.partial);
+            assert!(report
+                .remote_error
+                .expect("ambiguity is explicit")
+                .contains("not contacted"));
+            assert!(read_session("homeboy-lab").expect("read session").is_none());
+            assert!(read_ownership("homeboy-lab")
+                .expect("read ownership")
+                .is_none());
+        });
+    }
+
     #[test]
     fn foreign_loopback_html_is_identity_mismatch_and_never_receives_stop() {
         let listener = TcpListener::bind("127.0.0.1:0").expect("foreign listener");

crates/homeboy-lab-runner/src/lib.rs

@@ -137,8 +137,8 @@ pub(crate) use connection::daemon_endpoint_identity;
 pub(crate) use connection::disconnect_with_force;
 pub use connection::{
     close_reconnected_job_log_owner, connect, connect_reverse, connect_with_live_lease_adoption,
-    connect_with_orphan_adoption, disconnect, persisted_status, persisted_statuses,
-    reconcile_terminal_jobs, reconnect_job_log_owner, reverse_broker_artifact,
+    connect_with_orphan_adoption, disconnect, disconnect_local_recovery, persisted_status,
+    persisted_statuses, reconcile_terminal_jobs, reconnect_job_log_owner, reverse_broker_artifact,
     reverse_broker_artifact_content, reverse_broker_reconcile, runner_artifact_content, status,
     statuses, statuses_indexed, submit_reverse_broker_job,
 };

crates/homeboy-lab-runner/src/runners.rs

@@ -26,7 +26,7 @@ pub use crate::{
     BROKER_TOKEN_ENV, BROKER_TOKEN_HEADER,
 };
 pub use crate::{
-    connect_reverse, disconnect, download_remote_artifact,
+    connect_reverse, disconnect, disconnect_local_recovery, download_remote_artifact,
     evaluate_lab_runner_capabilities_for_runner, exec, execute_lab_offload,
     hydrate_prepared_workspace_source_snapshot, is_remote_runner_artifact_path,
     is_reportable_artifact_evidence_path, is_retrievable_runner_artifact,

crates/homeboy-lab-runner/src/session.rs

@@ -1141,6 +1141,13 @@ fn same_homeboy_version(left: &str, right: &str) -> bool {
 pub struct RunnerDisconnectReport {
     pub runner_id: String,
     pub disconnected: bool,
+    /// The remote daemon could not be authoritatively stopped. Its jobs and
+    /// lifecycle remain ambiguous, while the controller session is retained.
+    pub partial: bool,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub remote_error: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub local_recovery_command: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub session: Option<RunnerSession>,
     pub session_path: String,

docs/reference/cli/commands/runner.md

@@ -253,7 +253,7 @@ Show persisted runner tunnel status
 ## `homeboy runner disconnect`
 
 ```sh
-homeboy runner disconnect <ID>
+homeboy runner disconnect [OPTIONS] <ID>
 ```
 
 Close a runner tunnel and remove its persisted session state
@@ -262,6 +262,10 @@ Close a runner tunnel and remove its persisted session state
 | --- | --- | --- |
 | `<ID>` | yes | Runner ID |
 
+| Option | Value | Description |
+| --- | --- | --- |
+| `--local-recovery` | flag | Remove only this controller's matching local tunnel/session state without contacting the remote runner |
+
 ## `homeboy runner refresh-homeboy`
 
 ```sh