Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
3a68c1dd2f | ||
|
|
fde1954eac | ||
|
|
25435c34b2 | ||
|
|
4a956e9c38 | ||
|
|
2a49814572 | ||
|
|
d1aa56bab3 | ||
|
|
cf322746a7 | ||
|
|
b25e733539 | ||
|
|
5a861976d7 | ||
|
|
bfe4965081 | ||
|
|
9d7ed8c071 | ||
|
|
88a40c1ea7 |
Generated
+1
@@ -1558,6 +1558,7 @@ version = "0.1.0"
|
||||
dependencies = [
|
||||
"chrono",
|
||||
"futures-util",
|
||||
"libc",
|
||||
"maestro-protocol",
|
||||
"maestro-testing",
|
||||
"maestro-tui",
|
||||
|
||||
@@ -158,6 +158,7 @@ enum Command {
|
||||
|
||||
enum ChildCommand {
|
||||
Signal(libc::c_int),
|
||||
Reap(oneshot::Sender<bool>),
|
||||
}
|
||||
|
||||
/// Why the handshake did not produce a session id.
|
||||
@@ -206,6 +207,18 @@ impl AcpSession {
|
||||
self.exit.borrow().is_some()
|
||||
}
|
||||
|
||||
/// Reap the child if it has already exited.
|
||||
pub async fn reap_if_exited(&self) -> bool {
|
||||
if self.has_exited() {
|
||||
return true;
|
||||
}
|
||||
let (sent, received) = oneshot::channel();
|
||||
if self.child_commands.send(ChildCommand::Reap(sent)).is_err() {
|
||||
return self.has_exited();
|
||||
}
|
||||
received.await.unwrap_or_else(|_| self.has_exited())
|
||||
}
|
||||
|
||||
/// Wait for the child to exit.
|
||||
pub async fn wait(&self) -> Option<ExitStatus> {
|
||||
wait_exit(&self.exit).await
|
||||
@@ -392,6 +405,21 @@ async fn supervise_child(
|
||||
// so its PID cannot be reused in that window.
|
||||
unsafe { libc::kill(pid as libc::pid_t, sig) };
|
||||
}
|
||||
Some(ChildCommand::Reap(done)) => match child.try_wait() {
|
||||
Ok(Some(status)) => {
|
||||
let _ = done.send(true);
|
||||
tracing::debug!(pid, ?status, "{ADAPTER_BIN} exited");
|
||||
let _ = exit_tx.send(Some(status));
|
||||
return;
|
||||
}
|
||||
Ok(None) => {
|
||||
let _ = done.send(false);
|
||||
}
|
||||
Err(error) => {
|
||||
tracing::warn!(pid, %error, "could not reap {ADAPTER_BIN}");
|
||||
let _ = done.send(false);
|
||||
}
|
||||
},
|
||||
None => {
|
||||
// The session vanished without a command receiver. This
|
||||
// task still owns the unreaped child, so this cannot hit
|
||||
|
||||
@@ -17,6 +17,8 @@ Environment knobs, all optional:
|
||||
signal SIGTERM
|
||||
FAKE_ACP_ECHO_ENV name of one env var to log at start
|
||||
FAKE_ACP_INIT_FAIL=1 answer `initialize` with a JSON-RPC error
|
||||
FAKE_ACP_DELAY_SESSION_NEW_MS=n wait n milliseconds before answering
|
||||
`session/new`
|
||||
FAKE_ACP_EXIT_AFTER_NEW=1 exit 0 right after answering `session/new`
|
||||
FAKE_ACP_EXIT_ON_CANCEL=1 exit 0 on `session/cancel` (default: keep running
|
||||
so the SIGTERM path is exercised)
|
||||
@@ -36,6 +38,7 @@ import json
|
||||
import os
|
||||
import signal
|
||||
import sys
|
||||
import time
|
||||
|
||||
LOG = os.environ.get("FAKE_ACP_LOG")
|
||||
|
||||
@@ -113,6 +116,7 @@ for raw in sys.stdin:
|
||||
"authMethods": [],
|
||||
}})
|
||||
elif method == "session/new":
|
||||
time.sleep(int(os.environ.get("FAKE_ACP_DELAY_SESSION_NEW_MS", "0")) / 1000)
|
||||
send({"jsonrpc": "2.0", "id": rid, "result": {"sessionId": session_id}})
|
||||
send(chunk("hello from fake"))
|
||||
if os.environ.get("FAKE_ACP_EXIT_AFTER_NEW") == "1":
|
||||
|
||||
@@ -122,21 +122,23 @@ async fn run(config_override: Option<PathBuf>) -> Result<(), Box<dyn Error>> {
|
||||
// shutdown, and both must reach the adapter children below (#134). The
|
||||
// unit's `KillMode=process` signals only the daemon, so nothing else
|
||||
// would end them.
|
||||
let shutdown = async {
|
||||
let shutdown_manager = manager.clone();
|
||||
let shutdown = async move {
|
||||
let mut sigterm = tokio::signal::unix::signal(tokio::signal::unix::SignalKind::terminate())
|
||||
.expect("install SIGTERM handler");
|
||||
tokio::select! {
|
||||
_ = tokio::signal::ctrl_c() => tracing::info!("ctrl-c received — shutting down"),
|
||||
_ = sigterm.recv() => tracing::info!("SIGTERM received — shutting down"),
|
||||
}
|
||||
// Axum waits for upgraded WebSockets to drain. Structured children
|
||||
// must be settled before that wait, while the signal path still owns
|
||||
// the shutdown order (#179).
|
||||
shutdown_manager.shutdown().await;
|
||||
};
|
||||
|
||||
axum::serve(listener, router)
|
||||
.tcp_nodelay(true)
|
||||
.with_graceful_shutdown(shutdown)
|
||||
.await?;
|
||||
// tmux sessions outlive the daemon by design; structured sessions'
|
||||
// adapter children do not (#134).
|
||||
manager.shutdown().await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -170,6 +170,7 @@ fn session_error_status(code: &str) -> StatusCode {
|
||||
// The session outlived its pane: nothing to type into, and nothing
|
||||
// the daemon can do about it.
|
||||
"not_running" => StatusCode::CONFLICT,
|
||||
"shutting_down" => StatusCode::SERVICE_UNAVAILABLE,
|
||||
_ => StatusCode::BAD_GATEWAY,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7,7 +7,7 @@
|
||||
|
||||
use std::collections::{HashMap, HashSet};
|
||||
use std::path::PathBuf;
|
||||
use std::sync::atomic::{AtomicU64, Ordering};
|
||||
use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
|
||||
@@ -127,6 +127,8 @@ pub enum SessionError {
|
||||
/// a stray keystroke on an attach socket, which is dropped.
|
||||
#[error("session is no longer running: {0}")]
|
||||
NotRunning(Uuid),
|
||||
#[error("daemon is shutting down")]
|
||||
ShuttingDown,
|
||||
/// The kill was refused (#79): the session's worktree holds unsaved work.
|
||||
/// Nothing happened — the session keeps running and the tree is untouched,
|
||||
/// so it stays the operator's handle on their own unfinished work.
|
||||
@@ -144,6 +146,7 @@ impl SessionError {
|
||||
SessionError::NotFound(_) => "not_found",
|
||||
SessionError::InvalidEnv(_) => "invalid_env",
|
||||
SessionError::NotRunning(_) => "not_running",
|
||||
SessionError::ShuttingDown => "shutting_down",
|
||||
SessionError::KillGuarded(_) => "kill_guarded",
|
||||
};
|
||||
let unsaved_work = match self {
|
||||
@@ -195,6 +198,12 @@ struct Inner {
|
||||
/// client that reconnects never collides with the registration its
|
||||
/// previous socket has not finished releasing.
|
||||
next_viewport: AtomicU64,
|
||||
/// Once daemon shutdown starts, it owns structured-session finalization.
|
||||
/// Background passes must not classify those deliberate exits as natural.
|
||||
shutting_down: AtomicBool,
|
||||
/// Serializes structured creation with shutdown so every adapter child is
|
||||
/// either rejected or included in shutdown's final drain.
|
||||
structured_lifecycle: Mutex<()>,
|
||||
}
|
||||
|
||||
impl SessionManager {
|
||||
@@ -237,6 +246,8 @@ impl SessionManager {
|
||||
events,
|
||||
viewports: Mutex::new(Viewports::new()),
|
||||
next_viewport: AtomicU64::new(0),
|
||||
shutting_down: AtomicBool::new(false),
|
||||
structured_lifecycle: Mutex::new(()),
|
||||
}),
|
||||
}
|
||||
}
|
||||
@@ -349,13 +360,18 @@ impl SessionManager {
|
||||
/// is on disk after the restart is truthful. tmux sessions are left
|
||||
/// alone — surviving a restart is what they are for.
|
||||
pub async fn shutdown(&self) {
|
||||
if self.inner.shutting_down.swap(true, Ordering::AcqRel) {
|
||||
return;
|
||||
}
|
||||
let _lifecycle = self.inner.structured_lifecycle.lock().await;
|
||||
let children: Vec<(Uuid, Arc<AcpSession>, Option<watch::Receiver<bool>>)> = {
|
||||
let sessions = self.inner.sessions.lock().await;
|
||||
sessions
|
||||
.iter()
|
||||
.filter_map(|(id, r)| {
|
||||
r.acp
|
||||
.clone()
|
||||
(r.session.status == SessionStatus::Running)
|
||||
.then_some(())
|
||||
.and_then(|()| r.acp.clone())
|
||||
.map(|acp| (*id, acp, r.acp_event_pump.clone()))
|
||||
})
|
||||
.collect()
|
||||
@@ -363,8 +379,26 @@ impl SessionManager {
|
||||
if children.is_empty() {
|
||||
return;
|
||||
}
|
||||
tracing::info!(count = children.len(), "shutdown: ending adapter children");
|
||||
let terminations = children
|
||||
let reaped =
|
||||
futures_util::future::join_all(children.iter().map(|(_, acp, _)| acp.reap_if_exited()))
|
||||
.await;
|
||||
let mut natural_exits = Vec::new();
|
||||
let mut live_children = Vec::new();
|
||||
for (child, exited) in children.iter().cloned().zip(reaped) {
|
||||
if exited {
|
||||
natural_exits.push(child.0);
|
||||
} else {
|
||||
live_children.push(child);
|
||||
}
|
||||
}
|
||||
for id in natural_exits {
|
||||
mark_session_dead_with_worktree_settlement(&self.inner, id, true).await;
|
||||
}
|
||||
tracing::info!(
|
||||
count = live_children.len(),
|
||||
"shutdown: ending adapter children"
|
||||
);
|
||||
let terminations = live_children
|
||||
.iter()
|
||||
.map(|(_, acp, _)| acp.terminate(maestro_acp::CANCEL_GRACE, maestro_acp::TERM_GRACE));
|
||||
futures_util::future::join_all(terminations).await;
|
||||
@@ -373,7 +407,7 @@ impl SessionManager {
|
||||
.filter_map(|(_, _, done)| done.clone())
|
||||
.map(wait_for_event_pump);
|
||||
futures_util::future::join_all(drains).await;
|
||||
for (id, _, _) in children {
|
||||
for (id, _, _) in live_children {
|
||||
mark_session_dead_after_shutdown(&self.inner, id).await;
|
||||
}
|
||||
}
|
||||
@@ -457,6 +491,17 @@ impl SessionManager {
|
||||
|
||||
/// Spawn a new session. On success returns the freshly-created `Session`.
|
||||
pub async fn create(&self, req: CreateSessionRequest) -> Result<Session, SessionError> {
|
||||
if req.view == SessionView::Structured {
|
||||
let _lifecycle = self.inner.structured_lifecycle.lock().await;
|
||||
if self.inner.shutting_down.load(Ordering::Acquire) {
|
||||
return Err(SessionError::ShuttingDown);
|
||||
}
|
||||
return self.create_inner(req).await;
|
||||
}
|
||||
self.create_inner(req).await
|
||||
}
|
||||
|
||||
async fn create_inner(&self, req: CreateSessionRequest) -> Result<Session, SessionError> {
|
||||
let (effective_cwd, worktree_path, worktree_base_repo) = match &req.worktree {
|
||||
Some(spec) => {
|
||||
let WorktreeSpec {
|
||||
@@ -903,9 +948,9 @@ impl SessionManager {
|
||||
adopted += 1;
|
||||
}
|
||||
|
||||
// Structured sessions have no tmux process to discover. Their
|
||||
// sidecars are the recovery inventory: when an ACP id exists, start a
|
||||
// fresh adapter process and load that conversation into it.
|
||||
// Structured sessions have no tmux process to discover. A durable
|
||||
// marker written during graceful shutdown is the only evidence that a
|
||||
// sidecar may be resumed; older and naturally-dead sidecars stay dead.
|
||||
for mut session in self
|
||||
.inner
|
||||
.sidecar
|
||||
@@ -918,6 +963,7 @@ impl SessionManager {
|
||||
}
|
||||
|
||||
let id = session.id;
|
||||
let recoverable = self.inner.sidecar.take_recovery_marker(id)?;
|
||||
let mut session_env = sanitize_env(&session.env)?;
|
||||
let fifo_path = self.inner.fifo_dir.join(format!("{id}.fifo"));
|
||||
let bridge =
|
||||
@@ -933,40 +979,50 @@ impl SessionManager {
|
||||
session_env.insert("MAESTRO_URL".into(), url);
|
||||
}
|
||||
|
||||
let (acp, failure) = match session.acp_session_id.clone() {
|
||||
Some(acp_session_id) => {
|
||||
let config = SpawnConfig::new(
|
||||
&session.cwd,
|
||||
session_env
|
||||
.iter()
|
||||
.map(|(key, value)| (key.clone(), value.clone()))
|
||||
.collect(),
|
||||
)
|
||||
.load_session(acp_session_id);
|
||||
match maestro_acp::spawn(config).await {
|
||||
Ok(acp) => (Some(Arc::new(acp)), None),
|
||||
Err(error) => {
|
||||
tracing::warn!(%id, %error, "structured session: adapter failed to resume");
|
||||
(None, Some(error.to_string()))
|
||||
let (acp, failure) = if recoverable {
|
||||
match session.acp_session_id.clone() {
|
||||
Some(acp_session_id) => {
|
||||
let config = SpawnConfig::new(
|
||||
&session.cwd,
|
||||
session_env
|
||||
.iter()
|
||||
.map(|(key, value)| (key.clone(), value.clone()))
|
||||
.collect(),
|
||||
)
|
||||
.load_session(acp_session_id);
|
||||
match maestro_acp::spawn(config).await {
|
||||
Ok(acp) => (Some(Arc::new(acp)), None),
|
||||
Err(error) => {
|
||||
tracing::warn!(%id, %error, "structured session: adapter failed to resume");
|
||||
(None, Some(error.to_string()))
|
||||
}
|
||||
}
|
||||
}
|
||||
None => (
|
||||
None,
|
||||
Some("structured session has no stored ACP session id".into()),
|
||||
),
|
||||
}
|
||||
None => (
|
||||
None,
|
||||
Some("structured session has no stored ACP session id".into()),
|
||||
),
|
||||
} else {
|
||||
(None, None)
|
||||
};
|
||||
session.status = if failure.is_some() {
|
||||
session.status = if recoverable {
|
||||
if failure.is_some() {
|
||||
SessionStatus::Dead
|
||||
} else {
|
||||
SessionStatus::Running
|
||||
}
|
||||
} else {
|
||||
SessionStatus::Dead
|
||||
} else {
|
||||
SessionStatus::Running
|
||||
};
|
||||
session.activity = if failure.is_some() {
|
||||
ActivityState::Error
|
||||
} else {
|
||||
ActivityState::Active
|
||||
};
|
||||
session.activity_detail = failure;
|
||||
if recoverable {
|
||||
session.activity = if failure.is_some() {
|
||||
ActivityState::Error
|
||||
} else {
|
||||
ActivityState::Active
|
||||
};
|
||||
session.activity_detail = failure;
|
||||
}
|
||||
|
||||
let acp_events = if let Some(acp) = &acp {
|
||||
match self.inner.sidecar.open_events(id) {
|
||||
@@ -1438,6 +1494,9 @@ async fn probe_worktree(
|
||||
/// One pass of the liveness watcher: diff the in-memory session set against
|
||||
/// the tmux-reported one, and reap anything we still track but tmux has lost.
|
||||
async fn run_liveness_pass(inner: &Arc<Inner>) {
|
||||
if inner.shutting_down.load(Ordering::Acquire) {
|
||||
return;
|
||||
}
|
||||
// Snapshot the in-memory ids first; if we own no sessions there is
|
||||
// nothing to reap and no reason to spawn a tmux subprocess (this also
|
||||
// avoids racing with the very first `new-session` that would otherwise
|
||||
@@ -1497,6 +1556,9 @@ async fn run_liveness_pass(inner: &Arc<Inner>) {
|
||||
/// `SessionUpdated` only when `activity` or `foreground` actually changes — so a
|
||||
/// steady state produces no diffs. Runs on the existing liveness tick.
|
||||
async fn run_activity_pass(inner: &Arc<Inner>) {
|
||||
if inner.shutting_down.load(Ordering::Acquire) {
|
||||
return;
|
||||
}
|
||||
let ids: Vec<Uuid> = {
|
||||
let sessions = inner.sessions.lock().await;
|
||||
if sessions.is_empty() {
|
||||
@@ -1539,6 +1601,9 @@ async fn run_activity_pass(inner: &Arc<Inner>) {
|
||||
|
||||
let updated = {
|
||||
let mut sessions = inner.sessions.lock().await;
|
||||
if inner.shutting_down.load(Ordering::Acquire) {
|
||||
return;
|
||||
}
|
||||
let Some(rec) = sessions.get_mut(&id) else {
|
||||
continue;
|
||||
};
|
||||
@@ -1613,6 +1678,9 @@ async fn run_activity_pass(inner: &Arc<Inner>) {
|
||||
/// error must not read as "nothing blocks". Like the activity pass, this only
|
||||
/// emits `SessionUpdated` when the value actually changed.
|
||||
async fn run_blocked_work_pass(inner: &Arc<Inner>) {
|
||||
if inner.shutting_down.load(Ordering::Acquire) {
|
||||
return;
|
||||
}
|
||||
let targets: Vec<(Uuid, (PathBuf, PathBuf))> = {
|
||||
let sessions = inner.sessions.lock().await;
|
||||
sessions
|
||||
@@ -1640,6 +1708,9 @@ async fn run_blocked_work_pass(inner: &Arc<Inner>) {
|
||||
};
|
||||
let updated = {
|
||||
let mut sessions = inner.sessions.lock().await;
|
||||
if inner.shutting_down.load(Ordering::Acquire) {
|
||||
return;
|
||||
}
|
||||
let Some(rec) = sessions.get_mut(&id) else {
|
||||
continue;
|
||||
};
|
||||
@@ -1672,20 +1743,25 @@ async fn run_blocked_work_pass(inner: &Arc<Inner>) {
|
||||
/// [`SessionManager::recover`] adopts only what tmux still holds. Structured
|
||||
/// sessions stopped for daemon shutdown retain their sidecars for ACP recovery.
|
||||
async fn mark_session_dead(inner: &Arc<Inner>, id: Uuid) {
|
||||
if inner.shutting_down.load(Ordering::Acquire) {
|
||||
return;
|
||||
}
|
||||
mark_session_dead_with_worktree_settlement(inner, id, true).await;
|
||||
}
|
||||
|
||||
/// A daemon restart terminates ACP children, but it is not a natural adapter
|
||||
/// exit: the sidecar is the recovery inventory, including its worktree.
|
||||
async fn mark_session_dead_after_shutdown(inner: &Arc<Inner>, id: Uuid) {
|
||||
mark_session_dead_with_worktree_settlement(inner, id, false).await;
|
||||
if mark_session_dead_with_worktree_settlement(inner, id, false).await {
|
||||
let _ = inner.sidecar.mark_for_recovery(id);
|
||||
}
|
||||
}
|
||||
|
||||
async fn mark_session_dead_with_worktree_settlement(
|
||||
inner: &Arc<Inner>,
|
||||
id: Uuid,
|
||||
settle_worktree: bool,
|
||||
) {
|
||||
) -> bool {
|
||||
let flipped = {
|
||||
let mut sessions = inner.sessions.lock().await;
|
||||
match sessions.get_mut(&id) {
|
||||
@@ -1705,7 +1781,7 @@ async fn mark_session_dead_with_worktree_settlement(
|
||||
}
|
||||
};
|
||||
let Some((mut session, worktree)) = flipped else {
|
||||
return;
|
||||
return false;
|
||||
};
|
||||
tracing::info!(%id, "session process gone — marking dead");
|
||||
|
||||
@@ -1736,6 +1812,7 @@ async fn mark_session_dead_with_worktree_settlement(
|
||||
|
||||
let _ = inner.sidecar.write(&session);
|
||||
let _ = inner.events.send(CatalogEvent::SessionUpdated { session });
|
||||
true
|
||||
}
|
||||
|
||||
/// Apply the exit rule (#82) to a vanished session's worktree: probe what
|
||||
|
||||
@@ -56,6 +56,28 @@ impl SidecarStore {
|
||||
self.dir.join(format!("{id}.events.jsonl"))
|
||||
}
|
||||
|
||||
/// Durable evidence that graceful daemon shutdown ended this structured
|
||||
/// session and the next daemon may resume it.
|
||||
pub fn recovery_path_for(&self, id: Uuid) -> PathBuf {
|
||||
self.dir.join(format!("{id}.recover"))
|
||||
}
|
||||
|
||||
/// Mark `id` as eligible for one structured-session recovery attempt.
|
||||
pub fn mark_for_recovery(&self, id: Uuid) -> Result<(), SidecarError> {
|
||||
std::fs::create_dir_all(&self.dir)?;
|
||||
std::fs::write(self.recovery_path_for(id), [])?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Consume the graceful-shutdown recovery marker for `id`.
|
||||
pub fn take_recovery_marker(&self, id: Uuid) -> Result<bool, SidecarError> {
|
||||
match std::fs::remove_file(self.recovery_path_for(id)) {
|
||||
Ok(()) => Ok(true),
|
||||
Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(false),
|
||||
Err(error) => Err(error.into()),
|
||||
}
|
||||
}
|
||||
|
||||
/// Open the event log for `id` for appending, creating it (and the
|
||||
/// directory) when missing. One handle per live session: the caller
|
||||
/// keeps it for the session's lifetime rather than reopening per
|
||||
@@ -164,10 +186,11 @@ impl SidecarStore {
|
||||
}
|
||||
}
|
||||
|
||||
/// Delete the sidecar and the event log for `id`. No-op for whichever
|
||||
/// does not exist.
|
||||
/// Delete the sidecar, event log, and recovery marker for `id`. No-op for
|
||||
/// whichever does not exist.
|
||||
pub fn delete(&self, id: Uuid) -> Result<(), SidecarError> {
|
||||
remove_if_present(&self.events_path_for(id))?;
|
||||
remove_if_present(&self.recovery_path_for(id))?;
|
||||
remove_if_present(&self.path_for(id))
|
||||
}
|
||||
|
||||
|
||||
@@ -232,6 +232,12 @@ async fn recovery_loads_the_acp_session_without_duplicating_history() {
|
||||
let old_pid = fake.pid();
|
||||
fx.manager.shutdown().await;
|
||||
wait_for_exit(old_pid).await;
|
||||
assert!(
|
||||
fx.sidecar_dir()
|
||||
.join(format!("{}.recover", session.id))
|
||||
.exists(),
|
||||
"graceful shutdown must record recovery provenance"
|
||||
);
|
||||
|
||||
let revived = SessionManager::new(
|
||||
Arc::new(fx.tmux.tmux.clone()),
|
||||
@@ -498,6 +504,144 @@ async fn shutdown_reaps_every_adapter_child() {
|
||||
assert_eq!(sidecar.status, SessionStatus::Dead);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn dismissal_during_shutdown_does_not_restore_recovery_artifacts() {
|
||||
let fx = DaemonFixture::new();
|
||||
let dismissed = Fake::new();
|
||||
let blocker = Fake::new();
|
||||
let cwd = TempDir::new().unwrap();
|
||||
let mut env = dismissed.env();
|
||||
env.insert("FAKE_ACP_EXIT_ON_CANCEL".into(), "1".into());
|
||||
|
||||
let session = fx
|
||||
.manager
|
||||
.create(structured(cwd.path(), env))
|
||||
.await
|
||||
.expect("create");
|
||||
fx.manager
|
||||
.create(structured(cwd.path(), blocker.env()))
|
||||
.await
|
||||
.expect("create blocker");
|
||||
|
||||
let manager = fx.manager.clone();
|
||||
let shutdown = tokio::spawn(async move { manager.shutdown().await });
|
||||
wait_for_async(Duration::from_secs(5), || async {
|
||||
dismissed.log().contains("recv session/cancel")
|
||||
})
|
||||
.await;
|
||||
|
||||
fx.manager.kill(session.id, false).await.expect("dismiss");
|
||||
assert!(
|
||||
!fx.sidecar_dir()
|
||||
.join(format!("{}.json", session.id))
|
||||
.exists(),
|
||||
"dismissal must finish before shutdown settles its stale snapshot"
|
||||
);
|
||||
shutdown.await.expect("shutdown task");
|
||||
|
||||
assert!(!fx.manager.has(session.id).await);
|
||||
assert!(
|
||||
!fx.sidecar_dir()
|
||||
.join(format!("{}.recover", session.id))
|
||||
.exists(),
|
||||
"dismissal must leave no recovery marker"
|
||||
);
|
||||
assert!(
|
||||
!fx.sidecar_dir()
|
||||
.join(format!("{}.json", session.id))
|
||||
.exists(),
|
||||
"dismissal must leave no sidecar"
|
||||
);
|
||||
assert!(
|
||||
!fx.sidecar_dir()
|
||||
.join(format!("{}.events.jsonl", session.id))
|
||||
.exists(),
|
||||
"dismissal must leave no event log"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn shutdown_does_not_recover_an_adapter_that_already_exited() {
|
||||
let fx = DaemonFixture::new();
|
||||
let fake = Fake::new();
|
||||
let cwd = TempDir::new().unwrap();
|
||||
let mut env = fake.env();
|
||||
env.insert("FAKE_ACP_EXIT_AFTER_NEW".into(), "1".into());
|
||||
|
||||
let session = fx
|
||||
.manager
|
||||
.create(structured(cwd.path(), env))
|
||||
.await
|
||||
.expect("create");
|
||||
wait_for_exit(fake.pid()).await;
|
||||
|
||||
fx.manager.shutdown().await;
|
||||
assert!(
|
||||
!fx.sidecar_dir()
|
||||
.join(format!("{}.recover", session.id))
|
||||
.exists(),
|
||||
"a naturally exited adapter is not recoverable"
|
||||
);
|
||||
|
||||
let revived = SessionManager::new(
|
||||
Arc::new(fx.tmux.tmux.clone()),
|
||||
SidecarStore::new(fx.sidecar_dir()),
|
||||
WorktreeManager::new(None),
|
||||
fx.fifo_dir(),
|
||||
);
|
||||
assert_eq!(revived.recover().await.expect("recover"), 1);
|
||||
assert_eq!(revived.status(session.id).await, Some(SessionStatus::Dead));
|
||||
assert!(
|
||||
!fake.log().contains("recv session/load"),
|
||||
"a naturally dead adapter must not be resumed: {}",
|
||||
fake.log()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn shutdown_waits_for_structured_creation_and_rejects_new_creates() {
|
||||
let fx = DaemonFixture::new();
|
||||
let fake = Fake::new();
|
||||
let cwd = TempDir::new().unwrap();
|
||||
let mut env = fake.env();
|
||||
env.insert("FAKE_ACP_DELAY_SESSION_NEW_MS".into(), "500".into());
|
||||
|
||||
let create_cwd = cwd.path().to_path_buf();
|
||||
let manager = fx.manager.clone();
|
||||
let create = tokio::spawn(async move { manager.create(structured(&create_cwd, env)).await });
|
||||
wait_for_async(Duration::from_secs(5), || async {
|
||||
fake.log().contains("recv session/new")
|
||||
})
|
||||
.await;
|
||||
|
||||
let manager = fx.manager.clone();
|
||||
let shutdown = tokio::spawn(async move { manager.shutdown().await });
|
||||
tokio::time::sleep(Duration::from_millis(50)).await;
|
||||
assert!(
|
||||
!shutdown.is_finished(),
|
||||
"shutdown returned before create finished"
|
||||
);
|
||||
|
||||
let session = create.await.expect("create task").expect("create");
|
||||
let pid = fake.pid();
|
||||
shutdown.await.expect("shutdown task");
|
||||
|
||||
wait_for_exit(pid).await;
|
||||
assert!(fake.log().contains("signal SIGTERM"), "{}", fake.log());
|
||||
assert_eq!(
|
||||
fx.manager.status(session.id).await,
|
||||
Some(SessionStatus::Dead)
|
||||
);
|
||||
assert!(matches!(
|
||||
fx.manager.create(structured(cwd.path(), fake.env())).await,
|
||||
Err(maestro_daemon::SessionError::ShuttingDown)
|
||||
));
|
||||
|
||||
// A repeated signal must not attempt to terminate the child again.
|
||||
fx.manager.shutdown().await;
|
||||
assert_eq!(fake.log().matches("signal SIGTERM").count(), 1);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn shutdown_drains_final_notifications_to_the_log() {
|
||||
let fx = DaemonFixture::new();
|
||||
@@ -556,7 +700,27 @@ async fn adapter_exit_marks_the_session_dead_and_keeps_it_listed() {
|
||||
})
|
||||
.await;
|
||||
assert!(fx.manager.has(session.id).await);
|
||||
assert!(
|
||||
!fx.sidecar_dir()
|
||||
.join(format!("{}.recover", session.id))
|
||||
.exists(),
|
||||
"a naturally exited adapter is never marked for recovery"
|
||||
);
|
||||
|
||||
fx.manager.kill(session.id, false).await.expect("dismiss");
|
||||
assert!(!fx.manager.has(session.id).await);
|
||||
let revived = SessionManager::new(
|
||||
Arc::new(fx.tmux.tmux.clone()),
|
||||
SidecarStore::new(fx.sidecar_dir()),
|
||||
WorktreeManager::new(None),
|
||||
fx.fifo_dir(),
|
||||
);
|
||||
assert_eq!(revived.recover().await.expect("recover"), 1);
|
||||
assert_eq!(revived.status(session.id).await, Some(SessionStatus::Dead));
|
||||
assert!(
|
||||
!fake.log().contains("recv session/load"),
|
||||
"a naturally dead adapter must not be resumed: {}",
|
||||
fake.log()
|
||||
);
|
||||
|
||||
revived.kill(session.id, false).await.expect("dismiss");
|
||||
assert!(!revived.has(session.id).await);
|
||||
}
|
||||
|
||||
@@ -68,15 +68,30 @@ fn delete_removes_file_and_is_idempotent() {
|
||||
let s = sample(id);
|
||||
|
||||
store.write(&s).unwrap();
|
||||
store.mark_for_recovery(id).unwrap();
|
||||
assert!(store.path_for(id).exists());
|
||||
assert!(store.recovery_path_for(id).exists());
|
||||
|
||||
store.delete(id).unwrap();
|
||||
assert!(!store.path_for(id).exists());
|
||||
assert!(!store.recovery_path_for(id).exists());
|
||||
|
||||
// second delete is a no-op
|
||||
store.delete(id).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn recovery_marker_is_consumed_once() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
let store = SidecarStore::new(dir.path());
|
||||
let id = Uuid::new_v4();
|
||||
|
||||
assert!(!store.take_recovery_marker(id).unwrap());
|
||||
store.mark_for_recovery(id).unwrap();
|
||||
assert!(store.take_recovery_marker(id).unwrap());
|
||||
assert!(!store.take_recovery_marker(id).unwrap());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn load_all_recovers_every_sidecar_and_skips_garbage() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
|
||||
@@ -13,6 +13,7 @@ path = "src/lib.rs"
|
||||
[dependencies]
|
||||
chrono.workspace = true
|
||||
futures-util.workspace = true
|
||||
libc.workspace = true
|
||||
maestro-protocol = { workspace = true, features = ["client"] }
|
||||
maestro-testing.workspace = true
|
||||
serde.workspace = true
|
||||
|
||||
@@ -87,6 +87,14 @@ impl Daemon {
|
||||
}
|
||||
}
|
||||
|
||||
/// Send the daemon the same SIGTERM systemd uses, without waiting for its
|
||||
/// graceful HTTP shutdown to finish.
|
||||
pub fn send_sigterm(&self) {
|
||||
let child = self.child.as_ref().expect("daemon is running");
|
||||
let result = unsafe { libc::kill(child.id() as libc::pid_t, libc::SIGTERM) };
|
||||
assert_eq!(result, 0, "send SIGTERM to daemon");
|
||||
}
|
||||
|
||||
/// Re-spawn the daemon on the same port + state. The previous tmux
|
||||
/// server (under `TMUX_TMPDIR`) is left running so `recover()` can adopt
|
||||
/// its sessions.
|
||||
|
||||
@@ -15,7 +15,7 @@ use std::time::{Duration, Instant};
|
||||
use futures_util::{SinkExt, StreamExt};
|
||||
use maestro_e2e::util::wait_for_async;
|
||||
use maestro_e2e::{Daemon, TestClient};
|
||||
use maestro_protocol::{CreateSessionRequest, WorktreeSpec};
|
||||
use maestro_protocol::{CreateSessionRequest, SessionStatus, SessionView, WorktreeSpec};
|
||||
use tempfile::TempDir;
|
||||
use tokio_tungstenite::tungstenite::Message as WsMessage;
|
||||
|
||||
@@ -36,6 +36,61 @@ fn create_req(cwd: &std::path::Path, command: &str) -> CreateSessionRequest {
|
||||
}
|
||||
}
|
||||
|
||||
struct FakeAcp {
|
||||
_dir: TempDir,
|
||||
env: HashMap<String, String>,
|
||||
}
|
||||
|
||||
fn fake_acp(log: &std::path::Path) -> FakeAcp {
|
||||
let dir = tempfile::tempdir().expect("fake adapter dir");
|
||||
let adapter = dir.path().join("claude-code-acp");
|
||||
std::fs::copy(
|
||||
concat!(
|
||||
env!("CARGO_MANIFEST_DIR"),
|
||||
"/../maestro-acp/tests/fake_acp_agent.py"
|
||||
),
|
||||
&adapter,
|
||||
)
|
||||
.expect("copy fake adapter");
|
||||
let mut permissions = std::fs::metadata(&adapter)
|
||||
.expect("adapter metadata")
|
||||
.permissions();
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
permissions.set_mode(0o755);
|
||||
}
|
||||
std::fs::set_permissions(&adapter, permissions).expect("make fake adapter executable");
|
||||
let env = HashMap::from([
|
||||
(
|
||||
"PATH".into(),
|
||||
format!(
|
||||
"{}:{}",
|
||||
dir.path().display(),
|
||||
std::env::var("PATH").unwrap_or_default()
|
||||
),
|
||||
),
|
||||
("FAKE_ACP_LOG".into(), log.to_string_lossy().into_owned()),
|
||||
]);
|
||||
FakeAcp { _dir: dir, env }
|
||||
}
|
||||
|
||||
fn fake_acp_pid(log: &std::path::Path) -> Option<u32> {
|
||||
std::fs::read_to_string(log).ok()?.lines().find_map(|line| {
|
||||
line.strip_prefix("start pid=")?
|
||||
.split(' ')
|
||||
.next()?
|
||||
.parse()
|
||||
.ok()
|
||||
})
|
||||
}
|
||||
|
||||
fn pid_alive(pid: u32) -> bool {
|
||||
std::fs::read_to_string(format!("/proc/{pid}/stat"))
|
||||
.map(|stat| !stat.contains(") Z "))
|
||||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
/// A worktree spec built through its wire shape. The daemon and clients ship
|
||||
/// together (#87 dropped `cleanup_on_kill` from the struct), so the JSON
|
||||
/// object carries only the fields the lifecycle actually honours.
|
||||
@@ -995,6 +1050,66 @@ async fn daemon_restart_recovers_session() {
|
||||
client.kill_session(s.id).await.expect("kill");
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn sigterm_stops_structured_child_before_events_ws_drains() {
|
||||
let (mut daemon, client) = bring_up().await;
|
||||
let cwd = TempDir::new().unwrap();
|
||||
let log = cwd.path().join("fake-acp.log");
|
||||
let fake = fake_acp(&log);
|
||||
let session = client
|
||||
.spawn_session(&CreateSessionRequest {
|
||||
view: SessionView::Structured,
|
||||
cwd: cwd.path().to_path_buf(),
|
||||
command: "claude".into(),
|
||||
env: fake.env.clone(),
|
||||
metadata: HashMap::new(),
|
||||
worktree: None,
|
||||
})
|
||||
.await
|
||||
.expect("spawn structured session");
|
||||
let ws_url = daemon.client().events_url().expect("events URL");
|
||||
let (_events_ws, _) = tokio_tungstenite::connect_async(ws_url)
|
||||
.await
|
||||
.expect("events WS handshake");
|
||||
wait_for_async("fake ACP starts", Duration::from_secs(5), || {
|
||||
let log = log.clone();
|
||||
async move { fake_acp_pid(&log).is_some() }
|
||||
})
|
||||
.await;
|
||||
let pid = fake_acp_pid(&log).expect("fake ACP pid");
|
||||
|
||||
daemon.send_sigterm();
|
||||
|
||||
wait_for_async(
|
||||
"fake ACP exits after SIGTERM",
|
||||
Duration::from_secs(5),
|
||||
|| async { !pid_alive(pid) },
|
||||
)
|
||||
.await;
|
||||
let sidecar_path = daemon
|
||||
.state_dir()
|
||||
.join("sessions")
|
||||
.join(format!("{}.json", session.id));
|
||||
wait_for_async(
|
||||
"dead structured session sidecar",
|
||||
Duration::from_secs(5),
|
||||
|| {
|
||||
let sidecar_path = sidecar_path.clone();
|
||||
async move {
|
||||
std::fs::read(&sidecar_path)
|
||||
.ok()
|
||||
.and_then(|json| {
|
||||
serde_json::from_slice::<maestro_protocol::Session>(&json).ok()
|
||||
})
|
||||
.is_some_and(|saved| saved.status == SessionStatus::Dead)
|
||||
}
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
daemon.stop();
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn concurrent_spawns_each_get_unique_ids() {
|
||||
let (_daemon, client) = bring_up().await;
|
||||
|
||||
Reference in New Issue
Block a user