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.
This commit is contained in:
1 parent
97985b73d5
commit
b6da8aaa0c
4 files changed
+101
-13
No files matched your search
@@ -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.
|
||||
|
||||
@@ -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::<crate::session::AgentSession>(
|
||||
&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");
|
||||
});
|
||||
}
|
||||
@@ -107,9 +107,9 @@ impl AgentSession {
|
||||
pub fn resume_command(&self) -> Option<String> {
|
||||
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,
|
||||
|
||||
@@ -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}")));
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user