Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions codex-rs/app-server/src/request_processors/thread_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1308,7 +1308,7 @@ impl ThreadRequestProcessor {
params: ThreadArchiveParams,
) -> Result<(ThreadArchiveResponse, Vec<String>), JSONRPCErrorError> {
let thread_id = ThreadId::from_string(&params.thread_id)
.map_err(|err| invalid_request(format!("invalid thread id: {err}")))?;
.map_err(|err| invalid_request(format!("invalid session id: {err}")))?;

let mut thread_ids = vec![thread_id];
if let Some(state_db_ctx) = self.state_db.as_ref() {
Expand All @@ -1317,7 +1317,7 @@ impl ThreadRequestProcessor {
.await
.map_err(|err| {
internal_error(format!(
"failed to list spawned descendants for thread id {thread_id}: {err}"
"failed to list spawned descendants for session {thread_id}: {err}"
))
})?;
let mut seen = HashSet::from([thread_id]);
Expand Down Expand Up @@ -1629,7 +1629,7 @@ impl ThreadRequestProcessor {
params: ThreadUnarchiveParams,
) -> Result<(ThreadUnarchiveResponse, String), JSONRPCErrorError> {
let thread_id = ThreadId::from_string(&params.thread_id)
.map_err(|err| invalid_request(format!("invalid thread id: {err}")))?;
.map_err(|err| invalid_request(format!("invalid session id: {err}")))?;

let fallback_provider = self.config.model_provider_id.clone();
let stored_thread = self
Expand Down Expand Up @@ -2968,7 +2968,7 @@ impl ThreadRequestProcessor {
let existing_thread_id = match ThreadId::from_string(thread_id) {
Ok(id) => id,
Err(err) => {
return Err(invalid_request(format!("invalid thread id: {err}")));
return Err(invalid_request(format!("invalid session id: {err}")));
}
};
let params = StoreReadThreadParams {
Expand All @@ -2979,7 +2979,15 @@ impl ThreadRequestProcessor {
self.thread_store.read_thread(params).await
};

result.map_err(thread_store_resume_read_error)
let stored_thread = result.map_err(thread_store_resume_read_error)?;
if stored_thread.archived_at.is_some() {
let thread_id = stored_thread.thread_id;
return Err(invalid_request(format!(
"session {thread_id} is archived. Run `codex unarchive {thread_id}` to unarchive it first."
)));
Comment thread
etraut-openai marked this conversation as resolved.
Comment on lines +2983 to +2987
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Allow explicit forks from archived sessions

When a user explicitly forks an archived session by id or rollout path, this new guard rejects the request before thread_fork_inner can read the stored history, so codex fork <id> and thread/fork require an unnecessary unarchive/re-archive round trip even though forking only needs the archived transcript as a source. Fresh evidence after the prior comment: in this revision thread_fork_inner still calls this same read_stored_thread_for_resume helper, so the new archived_at check applies to fork as well as resume.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intended.

}

Ok(stored_thread)
}

async fn stored_thread_to_initial_history(
Expand Down Expand Up @@ -3964,7 +3972,7 @@ fn thread_store_archive_error(operation: &str, err: ThreadStoreError) -> JSONRPC
ThreadStoreError::Unsupported {
operation: unsupported_operation,
} => unsupported_thread_store_operation(unsupported_operation),
err => internal_error(format!("failed to {operation} thread: {err}")),
err => internal_error(format!("failed to {operation} session: {err}")),
}
}

Expand Down
52 changes: 52 additions & 0 deletions codex-rs/app-server/tests/suite/v2/thread_resume.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ use codex_app_server_protocol::TurnStartResponse;
use codex_app_server_protocol::TurnStatus;
use codex_app_server_protocol::UserInput;
use codex_config::types::AuthCredentialsStoreMode;
use codex_core::ARCHIVED_SESSIONS_SUBDIR;
use codex_login::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR;
use codex_protocol::ThreadId;
use codex_protocol::config_types::Personality;
Expand Down Expand Up @@ -780,6 +781,57 @@ async fn thread_resume_can_skip_turns_for_metadata_only_resume() -> Result<()> {
Ok(())
}

#[tokio::test]
async fn thread_resume_rejects_archived_session_by_id() -> Result<()> {
let server = create_mock_responses_server_repeating_assistant("Done").await;
let codex_home = TempDir::new()?;
create_config_toml(codex_home.path(), &server.uri())?;

let filename_ts = "2025-01-05T12-00-00";
let conversation_id = create_fake_rollout_with_text_elements(
codex_home.path(),
filename_ts,
"2025-01-05T12:00:00Z",
"Archived saved user message",
Vec::new(),
Some("mock_provider"),
/*git_info*/ None,
)?;
let active_rollout_path = rollout_path(codex_home.path(), filename_ts, &conversation_id);
let archived_dir = codex_home.path().join(ARCHIVED_SESSIONS_SUBDIR);
std::fs::create_dir_all(&archived_dir)?;
std::fs::rename(
&active_rollout_path,
archived_dir.join(active_rollout_path.file_name().expect("rollout file name")),
)?;

let mut mcp = McpProcess::new(codex_home.path()).await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;

let resume_id = mcp
.send_thread_resume_request(ThreadResumeParams {
thread_id: conversation_id.clone(),
..Default::default()
})
.await?;
let resume_err: JSONRPCError = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_error_message(RequestId::Integer(resume_id)),
)
.await??;

let message = resume_err.error.message;
assert!(
message.contains(&format!("session {conversation_id} is archived"))
&& message.contains(&format!(
"codex unarchive {conversation_id}` to unarchive it first"
)),
"unexpected resume error: {message}"
);

Ok(())
}

#[tokio::test]
async fn thread_resume_keeps_paused_goal_paused() -> Result<()> {
let server = create_mock_responses_server_repeating_assistant("Done").await;
Expand Down
Loading
Loading