fix(maestro): recover() re-advertises sessions under current daemon_id (NAP-264)
ci / rust (push) Successful in 6m46s
ci / rust (push) Successful in 6m46s
`SessionManager::recover()` re-inserted each sidecar's `Session` verbatim, keeping the persisted `daemon_id`. If the operator renamed the daemon between starts, recovered sessions were re-advertised under the old id and became unroutable. Overwrite `daemon_id` with the daemon's current id on recover, warning when the sidecar's id differs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -324,7 +324,7 @@ impl SessionManager {
|
||||
let live = self.inner.tmux.list_sessions()?;
|
||||
let mut adopted = 0usize;
|
||||
for id in live {
|
||||
let session = match self.inner.sidecar.read(id) {
|
||||
let mut session = match self.inner.sidecar.read(id) {
|
||||
Ok(Some(s)) => s,
|
||||
_ => Session {
|
||||
id,
|
||||
@@ -337,6 +337,19 @@ impl SessionManager {
|
||||
created_at: Utc::now(),
|
||||
},
|
||||
};
|
||||
// The sidecar persists whatever daemon_id was current when the
|
||||
// session was created. If the operator renamed the daemon between
|
||||
// starts, that id is stale — re-advertise under our own id so the
|
||||
// session stays routable (NAP-264).
|
||||
if session.daemon_id != self.inner.daemon_id {
|
||||
tracing::warn!(
|
||||
%id,
|
||||
sidecar_daemon_id = %session.daemon_id,
|
||||
current_daemon_id = %self.inner.daemon_id,
|
||||
"recovered session had a stale daemon_id; re-advertising under the current id"
|
||||
);
|
||||
session.daemon_id = self.inner.daemon_id.clone();
|
||||
}
|
||||
let fifo_path = self.inner.fifo_dir.join(format!("{id}.fifo"));
|
||||
let bridge =
|
||||
PtyBridge::start_with_replay(fifo_path.clone(), self.inner.replay_bytes).await?;
|
||||
|
||||
@@ -108,6 +108,49 @@ async fn recover_adopts_existing_tmux_sessions() {
|
||||
assert_eq!(snap[0].command, "sleep 30");
|
||||
}
|
||||
|
||||
/// NAP-264: if the operator renamed the daemon between starts (config edit),
|
||||
/// `recover()` must re-advertise adopted sessions under the CURRENT daemon_id,
|
||||
/// not the stale one baked into the sidecar — otherwise nothing routes to them.
|
||||
#[tokio::test]
|
||||
async fn recover_overwrites_stale_daemon_id_after_rename() {
|
||||
let fx = DaemonFixture::new("laptop");
|
||||
let cwd = TempDir::new().unwrap();
|
||||
|
||||
let s = fx
|
||||
.manager
|
||||
.create(CreateSessionRequest {
|
||||
daemon_id: "laptop".into(),
|
||||
cwd: cwd.path().to_path_buf(),
|
||||
command: "sleep 30".into(),
|
||||
env: HashMap::new(),
|
||||
metadata: HashMap::new(),
|
||||
worktree: None,
|
||||
})
|
||||
.await
|
||||
.expect("create");
|
||||
|
||||
// Revive against the same dirs + tmux server but under a NEW daemon_id,
|
||||
// as if the operator edited `daemon_id` and restarted.
|
||||
let sidecar = maestro_daemon::sidecar::SidecarStore::new(fx.sidecar_dir());
|
||||
let worktree_mgr =
|
||||
maestro_daemon::worktree::WorktreeManager::new(fx.worktree_root.path().to_path_buf());
|
||||
let revived = maestro_daemon::SessionManager::new(
|
||||
"laptop-v2".to_string(),
|
||||
fx.tmux.tmux.clone(),
|
||||
sidecar,
|
||||
worktree_mgr,
|
||||
fx.fifo_dir(),
|
||||
);
|
||||
assert_eq!(revived.recover().await.expect("recover"), 1);
|
||||
|
||||
let snap = revived.snapshot().await;
|
||||
assert_eq!(snap[0].id, s.id);
|
||||
assert_eq!(
|
||||
snap[0].daemon_id, "laptop-v2",
|
||||
"recovered session kept the stale daemon_id from the sidecar"
|
||||
);
|
||||
}
|
||||
|
||||
/// Regression for NAP-253: the very first bytes a freshly-spawned command
|
||||
/// writes must reach the replay buffer. Before the fix, `pipe-pane` was a
|
||||
/// separate tmux invocation that ran after `new-session` + an async FIFO
|
||||
|
||||
Reference in New Issue
Block a user