From b6da8aaa0ca1a7a5547aa7d60b9b6a48cec2f65a Mon Sep 17 00:00:00 2001 From: kuddev Date: Mon, 21 Sep 2026 15:08:25 +0800 Subject: [PATCH] fix(session): retain saved Codex IDs with legacy transcript names Keep a validated saved conversation ID when the optional native filename does not provide a rollout UUID. Cover normalization, command generation, unsafe IDs and the queued GPUI restore path. --- .../2026-09-21-native-resume-identity.md | 5 ++ .../gpui_shell/terminal/view/startup_tests.rs | 31 ++++++++ nebula_app/src/session.rs | 4 +- nebula_app/src/session/identity.rs | 74 ++++++++++++++++--- 4 files changed, 101 insertions(+), 13 deletions(-) diff --git a/architecture/notes/nebula_app/session/2026-09-21-native-resume-identity.md b/architecture/notes/nebula_app/session/2026-09-21-native-resume-identity.md index b089bd1..e81ac73 100644 --- a/architecture/notes/nebula_app/session/2026-09-21-native-resume-identity.md +++ b/architecture/notes/nebula_app/session/2026-09-21-native-resume-identity.md @@ -30,6 +30,11 @@ Retain that path in the existing optional snapshot field; the format version stays unchanged. Older payloads without `transcript_path` keep their compatibility behavior. Explicit null/invalid transcripts do not manufacture a resumable ID. +For a saved snapshot, a recognized rollout path takes precedence over its old +ID. An unrecognized filename retains the existing ID, which still passes through +the ordinary resume-command validator. Optional file metadata must not erase a +previously usable target or prevent an older snapshot from resuming. + Share rollout UUID parsing between Hook normalization, native-file discovery, the active-process probe and saved-target normalization. Parse Windows and POSIX paths lexically: a guest path must not be searched on the host filesystem. diff --git a/nebula_app/src/gpui_shell/terminal/view/startup_tests.rs b/nebula_app/src/gpui_shell/terminal/view/startup_tests.rs index c14265a..f411647 100644 --- a/nebula_app/src/gpui_shell/terminal/view/startup_tests.rs +++ b/nebula_app/src/gpui_shell/terminal/view/startup_tests.rs @@ -514,3 +514,34 @@ fn file_only_recovery_requires_the_provider_to_confirm_that_file() { ..saved })); } + +#[gpui::test] +fn cold_resume_preserves_codex_id_with_a_legacy_native_file(cx: &mut TestAppContext) { + let (view, window, receiver) = open(cx); + view.update(window, |view, cx| { + let event = crate::ai_hook::parse_remote_envelope( + b"nebula-hook/1 source=codex codex_hooks=full\n{\"hook_event_name\":\"SessionStart\",\"session_id\":\"saved-thread\",\"session_file\":\"/sessions/conversation.jsonl\"}", + Some(view.pane_id), + ).unwrap(); + let reported = crate::session::AgentSession { + source: event.source, + session_id: event.session_id, + session_file: event.session_file, + }; + let saved = serde_json::from_str::( + &serde_json::to_string(&reported).unwrap(), + ).unwrap(); + view.restore_agent(saved.clone(), cx); + assert_eq!(view.session_agent(), Some(saved.clone())); + assert!(view.pending_shell_command.is_some()); + assert!(!view.can_retry_recovery(), "a known ID must not become a failed restore"); + assert!(!receiver.try_iter().any(|message| matches!(message, Msg::Input(_)))); + feed(view, b"\x1b]133;A\x07user@host:~$ "); + view.flush_pending_shell_command(cx); + assert!(receiver.try_iter().any(|message| { + matches!(message, Msg::Input(bytes) if bytes.as_ref() == b"codex resume saved-thread") + })); + assert_eq!(view.session_agent(), Some(saved)); + assert!(view.recovery_pending(), "submission still waits for provider confirmation"); + }); +} diff --git a/nebula_app/src/session.rs b/nebula_app/src/session.rs index c225f51..e58b6f6 100644 --- a/nebula_app/src/session.rs +++ b/nebula_app/src/session.rs @@ -107,9 +107,9 @@ impl AgentSession { pub fn resume_command(&self) -> Option { let agent = crate::ai_agents::AgentKind::parse(&self.source)?; if agent == crate::ai_agents::AgentKind::Codex - && let Some(path) = &self.session_file + && let Some(id) = self.session_file.as_deref().and_then(codex_rollout_id) { - return agent.resume_command(codex_rollout_id(path)?); + return agent.resume_command(id); } match self.session_id.as_deref() { Some(id) if agent == crate::ai_agents::AgentKind::Pi && id.starts_with("pid-") => None, diff --git a/nebula_app/src/session/identity.rs b/nebula_app/src/session/identity.rs index b9a554f..bafe635 100644 --- a/nebula_app/src/session/identity.rs +++ b/nebula_app/src/session/identity.rs @@ -26,12 +26,13 @@ pub(crate) fn codex_rollout_id(path: &str) -> Option<&str> { impl super::AgentSession { /// Upgrade snapshots carrying a native file before comparing hook - /// acknowledgements or generating a command. This does not guess by cwd. + /// acknowledgements or generating a command. Unrecognized file names keep + /// the saved ID; missing filename evidence cannot erase an existing target. pub(crate) fn normalize_identity(&mut self) { if self.source == "codex" - && let Some(path) = &self.session_file + && let Some(id) = self.session_file.as_deref().and_then(codex_rollout_id) { - self.session_id = codex_rollout_id(path).map(str::to_owned); + self.session_id = Some(id.to_owned()); } } } @@ -65,14 +66,65 @@ mod tests { } #[test] - fn snapshot_resume_uses_the_rollout_thread_instead_of_the_hook_group() { - let mut saved = super::super::AgentSession { - source: "codex".into(), - session_id: Some("different-hook-group".into()), - session_file: Some(format!("/home/user/.codex/sessions/rollout-date-{THREAD}.jsonl")), - }; + fn unrecognized_native_file_keeps_the_saved_resume_command() { + for path in [ + "/sessions/conversation.jsonl", + r"C:\sessions\conversation.jsonl", + "/sessions/rollout-date-not-a-uuid.jsonl", + ] { + let saved = super::super::AgentSession { + source: "codex".into(), + session_id: Some("saved-thread".into()), + session_file: Some(path.into()), + }; + assert_eq!(saved.resume_command().as_deref(), Some("codex resume saved-thread")); + } + } + + #[test] + fn unrecognized_native_file_cannot_erase_an_existing_snapshot_identity() { + let mut saved: super::super::AgentSession = serde_json::from_value(serde_json::json!({ + "source": "codex", + "session_id": "saved-thread", + "session_file": "/sessions/conversation.jsonl", + })) + .unwrap(); + let original = saved.clone(); saved.normalize_identity(); - assert_eq!(saved.session_id.as_deref(), Some(THREAD)); - assert_eq!(saved.resume_command(), Some(format!("codex resume {THREAD}"))); + saved.normalize_identity(); + assert_eq!(saved, original, "normalization keeps the ID and native file intact"); + assert_eq!(saved.resume_command().as_deref(), Some("codex resume saved-thread")); + } + + #[test] + fn an_unrecognized_file_still_needs_a_safe_existing_id_to_resume() { + for id in [None, Some("".into()), Some("bad; echo injected".into()), Some("x".repeat(65))] { + let mut saved = super::super::AgentSession { + source: "codex".into(), + session_id: id.clone(), + session_file: Some("/sessions/conversation.jsonl".into()), + }; + assert!(saved.resume_command().is_none()); + saved.normalize_identity(); + assert_eq!(saved.session_id, id); + assert!(saved.resume_command().is_none()); + } + } + + #[test] + fn snapshot_resume_uses_the_rollout_thread_instead_of_the_hook_group() { + for id in [Some("different-hook-group".into()), None] { + let mut saved = super::super::AgentSession { + source: "codex".into(), + session_id: id, + session_file: Some(format!( + "/home/user/.codex/sessions/rollout-date-{THREAD}.jsonl" + )), + }; + assert_eq!(saved.resume_command(), Some(format!("codex resume {THREAD}"))); + saved.normalize_identity(); + assert_eq!(saved.session_id.as_deref(), Some(THREAD)); + assert_eq!(saved.resume_command(), Some(format!("codex resume {THREAD}"))); + } } }