Skip to content

Commit 010df5d

Browse files
authored
fix(runner): retire rejected state-loss replacements (#11908)
Fixes #11901. AI assistance: OpenAI gpt-5.6-sol and gpt-5.6-terra via OpenCode implemented, tested, reviewed, and prepared this change for landing. Chris Huber remains responsible for every line.
2 parents e0613d7 + f5f6a5d commit 010df5d

3 files changed

Lines changed: 287 additions & 0 deletions

File tree

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ struct CliEnvelope {
6464
success: bool,
6565
data: Option<Value>,
6666
error: Option<Value>,
67+
diagnostics: Option<Value>,
6768
}
6869

6970
struct RemoteDaemonConnectOptions<'a> {
@@ -674,6 +675,11 @@ fn connect_with_orphan_adoption_and_live_lease(
674675
REMOTE_LEASELESS_RECOVERY_TIMEOUT,
675676
);
676677
if !output.success {
678+
if kind == "state-loss" && is_terminal_state_loss_refusal(&output) {
679+
super::generation_store::retire_rejected_state_loss_replacement(
680+
runner_id, &output,
681+
)?;
682+
}
677683
return Ok(failed_connect(
678684
runner_id,
679685
session_path,
@@ -691,6 +697,11 @@ fn connect_with_orphan_adoption_and_live_lease(
691697
)
692698
})?;
693699
if !envelope.success {
700+
if kind == "state-loss" && is_terminal_state_loss_refusal(&output) {
701+
super::generation_store::retire_rejected_state_loss_replacement(
702+
runner_id, &output,
703+
)?;
704+
}
694705
return Ok(failed_connect(
695706
runner_id,
696707
session_path,
@@ -800,6 +811,11 @@ fn connect_with_orphan_adoption_and_live_lease(
800811
REMOTE_LEASELESS_RECOVERY_TIMEOUT,
801812
);
802813
if !recovery.success {
814+
if is_terminal_state_loss_refusal(&recovery) {
815+
super::generation_store::retire_rejected_state_loss_replacement(
816+
runner_id, &recovery,
817+
)?;
818+
}
803819
return Ok(failed_connect(
804820
runner_id,
805821
session_path,
@@ -814,6 +830,11 @@ fn connect_with_orphan_adoption_and_live_lease(
814830
)
815831
})?;
816832
if !envelope.success {
833+
if is_terminal_state_loss_refusal(&recovery) {
834+
super::generation_store::retire_rejected_state_loss_replacement(
835+
runner_id, &recovery,
836+
)?;
837+
}
817838
return Ok(failed_connect(
818839
runner_id,
819840
session_path,
@@ -1489,6 +1510,25 @@ fn state_loss_recovery_failure_message(output: &homeboy_core::server::CommandOut
14891510
}
14901511
}
14911512

1513+
fn is_terminal_state_loss_refusal(output: &homeboy_core::server::CommandOutput) -> bool {
1514+
if output.timed_out {
1515+
return false;
1516+
}
1517+
let Ok(envelope) = parse_envelope(&output.stdout) else {
1518+
return false;
1519+
};
1520+
if envelope.success {
1521+
return false;
1522+
}
1523+
let code = envelope
1524+
.diagnostics
1525+
.as_ref()
1526+
.and_then(|diagnostics| diagnostics.get("code"))
1527+
.or_else(|| envelope.error.as_ref().and_then(|error| error.get("code")))
1528+
.and_then(Value::as_str);
1529+
matches!(code, Some(code) if code.starts_with("validation.") || code.starts_with("policy."))
1530+
}
1531+
14921532
fn remote_state_loss_recovery_command(
14931533
homeboy: &str,
14941534
lease_id: &str,

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

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1345,6 +1345,170 @@ esac
13451345
});
13461346
}
13471347

1348+
#[cfg(unix)]
1349+
fn rejected_state_loss_refusal_is_retired_before_plain_connect_retries(
1350+
legacy_replay: bool,
1351+
transport_success: bool,
1352+
) {
1353+
test_support::with_isolated_home(|home| {
1354+
let daemon = home.path().join("remote-homeboy");
1355+
let generation_count = home.path().join("daemon-generations");
1356+
let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("listener");
1357+
let address = listener.local_addr().expect("address");
1358+
let endpoint = std::thread::spawn(move || {
1359+
for _ in 0..16 {
1360+
let (mut stream, _) = listener.accept().expect("endpoint request");
1361+
let mut request = [0; 4096];
1362+
let length = stream.read(&mut request).expect("read endpoint request");
1363+
let request = String::from_utf8_lossy(&request[..length]);
1364+
let body = if request.starts_with("GET /health ") {
1365+
r#"{"freshness":{"fresh":true,"restartable":true,"lease_id":"lease-b","pid":4242,"active_jobs":0},"pid":4242}"#
1366+
} else {
1367+
r#"{"version":"0.284.0","build_identity":{"display":"homeboy 0.284.0+test"},"lease":{"lease_id":"lease-b"}}"#
1368+
};
1369+
stream
1370+
.write_all(format!("HTTP/1.1 200 OK\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", body.len()).as_bytes())
1371+
.expect("endpoint response");
1372+
}
1373+
});
1374+
std::fs::write(
1375+
&daemon,
1376+
format!(
1377+
r#"#!/bin/sh
1378+
case "$1 $2" in
1379+
"self identity")
1380+
printf '%s\n' '{{"success":true,"data":{{"version":"0.284.0","display":"homeboy 0.284.0+test"}}}}'
1381+
;;
1382+
"daemon status")
1383+
printf '%s\n' '{{"success":true,"data":{{"running":false,"fresh":false,"reachable":false,"freshness":{{"active_jobs":0}}}}}}'
1384+
;;
1385+
"daemon recover-missing-lease-state")
1386+
if [ "$3" = "--help" ]; then
1387+
printf '%s\n' 'OPTIONS:' ' --replacement-operation-id <ID>'
1388+
else
1389+
printf '%s\n' '{{"success":false,"diagnostics":{{"code":"validation.invalid_argument"}}}}'
1390+
{rejection_exit}
1391+
fi
1392+
;;
1393+
"daemon ensure-running")
1394+
if [ "$3" = "--help" ]; then
1395+
printf '%s\n' 'OPTIONS:' ' --replacement-operation-id <ID>'
1396+
else
1397+
printf '%s' '1' > "{generation_count}"
1398+
printf '%s\n' '{{"success":true,"data":{{"pid":4242,"address":"{address}","state_path":"/tmp/state-b.json","lease_id":"lease-b"}}}}'
1399+
fi
1400+
;;
1401+
esac
1402+
"#,
1403+
address = address,
1404+
generation_count = generation_count.display(),
1405+
rejection_exit = if transport_success { "" } else { " exit 2" },
1406+
),
1407+
)
1408+
.expect("write remote Homeboy shim");
1409+
let mut permissions = std::fs::metadata(&daemon).expect("metadata").permissions();
1410+
permissions.set_mode(0o755);
1411+
std::fs::set_permissions(&daemon, permissions).expect("make shim executable");
1412+
server::create(
1413+
&serde_json::json!({ "id": "local-runner", "host": "localhost", "user": "test" })
1414+
.to_string(),
1415+
false,
1416+
)
1417+
.expect("create local server");
1418+
crate::create(
1419+
&serde_json::json!({ "id": "local-runner", "kind": "ssh", "homeboy_path": daemon })
1420+
.to_string(),
1421+
false,
1422+
)
1423+
.expect("enable local runner");
1424+
1425+
if legacy_replay {
1426+
let operation_id = crate::generation_store::replacement_operation("local-runner")
1427+
.expect("create legacy operation");
1428+
crate::generation_store::record_replacement_operation_replay(
1429+
"local-runner",
1430+
"state-loss",
1431+
&remote_state_loss_recovery_command(
1432+
daemon.to_str().expect("daemon path"),
1433+
"lease-lost",
1434+
41,
1435+
"127.0.0.1:7419",
1436+
&operation_id,
1437+
),
1438+
)
1439+
.expect("write legacy state-loss replay");
1440+
}
1441+
1442+
let (_rejected, exit_code) = connect_with_orphan_adoption(
1443+
"local-runner",
1444+
None,
1445+
&[],
1446+
false,
1447+
(!legacy_replay).then_some("lease-lost"),
1448+
(!legacy_replay).then_some(41),
1449+
(!legacy_replay).then_some("127.0.0.1:7419"),
1450+
)
1451+
.expect("rejected state-loss result");
1452+
assert_eq!(exit_code, 20);
1453+
let journal_dir = homeboy_core::paths::runner_sessions_dir()
1454+
.expect("runner sessions")
1455+
.join("local-runner");
1456+
assert!(!journal_dir.join("pending-replacement.json").exists());
1457+
let operation: serde_json::Value = serde_json::from_str(
1458+
&std::fs::read_to_string(journal_dir.join("replacement-operation.json"))
1459+
.expect("replacement operation"),
1460+
)
1461+
.expect("replacement operation JSON");
1462+
assert!(operation["kind"].is_null());
1463+
assert!(operation["replay_command"].is_null());
1464+
let evidence = std::fs::read_dir(journal_dir.join("rejected-replacements"))
1465+
.expect("rejection evidence")
1466+
.next()
1467+
.expect("one rejection evidence")
1468+
.expect("evidence entry");
1469+
let evidence: serde_json::Value = serde_json::from_str(
1470+
&std::fs::read_to_string(evidence.path()).expect("read rejection evidence"),
1471+
)
1472+
.expect("rejection evidence JSON");
1473+
assert_eq!(evidence["kind"], "state-loss");
1474+
assert_eq!(evidence["exit_code"], if transport_success { 0 } else { 2 });
1475+
1476+
let (connected, exit_code) =
1477+
connect_with_orphan_adoption("local-runner", None, &[], false, None, None, None)
1478+
.expect("plain connect result");
1479+
assert_eq!(
1480+
exit_code, 0,
1481+
"plain connect: {:?}",
1482+
connected.failure_message
1483+
);
1484+
assert!(connected.connected);
1485+
assert_eq!(connected.remote_daemon_pid, Some(4242));
1486+
assert_eq!(
1487+
std::fs::read_to_string(generation_count).expect("start count"),
1488+
"1"
1489+
);
1490+
drop(endpoint);
1491+
});
1492+
}
1493+
1494+
/// #11901: a legacy replay journal can contain an already-refused exact
1495+
/// recovery. Its evidence is retained, but its pending authority must not block
1496+
/// a retry of plain connect.
1497+
#[cfg(unix)]
1498+
#[test]
1499+
fn rejected_legacy_state_loss_replay_is_retired_before_plain_connect_retries() {
1500+
rejected_state_loss_refusal_is_retired_before_plain_connect_retries(true, false);
1501+
}
1502+
1503+
/// #11901: the fresh exact state-loss command can receive a typed failure
1504+
/// envelope over a successful transport. That terminal refusal must retire its
1505+
/// replay authority before the next plain connect.
1506+
#[cfg(unix)]
1507+
#[test]
1508+
fn rejected_fresh_state_loss_envelope_is_retired_before_plain_connect_retries() {
1509+
rejected_state_loss_refusal_is_retired_before_plain_connect_retries(false, true);
1510+
}
1511+
13481512
/// #10430: once recovery creates B, losing the controller's bounded health
13491513
/// requests must leave enough durable evidence for the next invocation to
13501514
/// authenticate B. It must never create an unjournaled C.

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

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,13 @@ fn replacement_operation_path(runner_id: &str) -> Result<PathBuf> {
6767
.join("replacement-operation.json"))
6868
}
6969

70+
fn rejected_replacement_path(runner_id: &str) -> Result<PathBuf> {
71+
Ok(paths::runner_sessions_dir()?
72+
.join(runner_id)
73+
.join("rejected-replacements")
74+
.join(format!("{}.json", uuid::Uuid::new_v4())))
75+
}
76+
7077
fn admission_reservation_path(runner_id: &str) -> Result<PathBuf> {
7178
Ok(paths::runner_sessions_dir()?
7279
.join(runner_id)
@@ -94,6 +101,20 @@ struct ReplacementOperation {
94101
kind: Option<String>,
95102
}
96103

104+
#[derive(serde::Serialize)]
105+
struct RejectedReplacementEvidence<'a> {
106+
schema: &'static str,
107+
runner_id: &'a str,
108+
operation_id: &'a str,
109+
kind: &'a str,
110+
replay_command: Option<&'a str>,
111+
rejected_at: String,
112+
exit_code: i32,
113+
timed_out: bool,
114+
stdout: &'a str,
115+
stderr: &'a str,
116+
}
117+
97118
pub(crate) fn write_durable_json<T: serde::Serialize>(
98119
path: &std::path::Path,
99120
value: &T,
@@ -794,6 +815,68 @@ pub(crate) fn retire_pending_replacement(runner_id: &str) -> Result<String> {
794815
})
795816
}
796817

818+
/// Retire a remote policy or validation refusal after preserving the exact
819+
/// terminal response. Unlike an interrupted mutation, this operation cannot
820+
/// become publishable by replaying the same authority.
821+
pub(crate) fn retire_rejected_state_loss_replacement(
822+
runner_id: &str,
823+
output: &homeboy_core::server::CommandOutput,
824+
) -> Result<()> {
825+
with_registry_lock(runner_id, || {
826+
let operation_path = replacement_operation_path(runner_id)?;
827+
let operation: ReplacementOperation =
828+
serde_json::from_slice(&std::fs::read(&operation_path).map_err(|error| {
829+
Error::internal_io(
830+
error.to_string(),
831+
Some(format!("read {}", operation_path.display())),
832+
)
833+
})?)
834+
.map_err(|error| {
835+
Error::config_invalid_json(operation_path.display().to_string(), error)
836+
})?;
837+
if operation.runner_id != runner_id || operation.kind.as_deref() != Some("state-loss") {
838+
return Err(Error::internal_unexpected(
839+
"refusing to retire a replacement operation that is not the rejected state-loss recovery",
840+
));
841+
}
842+
let evidence_path = rejected_replacement_path(runner_id)?;
843+
write_durable_json(
844+
&evidence_path,
845+
&RejectedReplacementEvidence {
846+
schema: "homeboy/runner-rejected-replacement/v1",
847+
runner_id,
848+
operation_id: &operation.operation_id,
849+
kind: "state-loss",
850+
replay_command: operation.replay_command.as_deref(),
851+
rejected_at: Utc::now().to_rfc3339(),
852+
exit_code: output.exit_code,
853+
timed_out: output.timed_out,
854+
stdout: &output.stdout,
855+
stderr: &output.stderr,
856+
},
857+
)?;
858+
write_durable_json(
859+
&operation_path,
860+
&ReplacementOperation {
861+
runner_id: runner_id.to_string(),
862+
operation_id: uuid::Uuid::new_v4().to_string(),
863+
replay_command: None,
864+
kind: None,
865+
},
866+
)?;
867+
let pending_path = pending_replacement_path(runner_id)?;
868+
if pending_path.exists() {
869+
std::fs::remove_file(&pending_path).map_err(|error| {
870+
Error::internal_io(
871+
error.to_string(),
872+
Some(format!("remove {}", pending_path.display())),
873+
)
874+
})?;
875+
}
876+
Ok(())
877+
})
878+
}
879+
797880
/// Promote the direct session that status has just health-checked. The
798881
/// controller session is the admission authority for new work; older entries
799882
/// remain draining so their owned jobs keep their original endpoints.

0 commit comments

Comments
 (0)