Skip to content

Add thread archive CLI commands#25021

Merged
etraut-openai merged 13 commits into
mainfrom
etraut/archive-thread-commands
May 30, 2026
Merged

Add thread archive CLI commands#25021
etraut-openai merged 13 commits into
mainfrom
etraut/archive-thread-commands

Conversation

@etraut-openai
Copy link
Copy Markdown
Collaborator

@etraut-openai etraut-openai commented May 29, 2026

Problem

Saved threads can already be archived through app-server RPCs, but the command line did not expose direct archive or unarchive commands.

Solution

Add codex archive <thread> and codex unarchive <thread>, resolving UUIDs or exact thread names before calling the existing thread/archive and thread/unarchive RPCs. The commands support scoped remote flags so callers can target remote app-server endpoints when archiving or unarchiving threads.

This also fixes a long-standing bug in codex resume <thread id> and codex fork <thread id> that I found when testing the new commands. These operations shouldn't be allowed on archived sessions. They now fail with an error that tells the user to run codex unarchive <thread id> first.

Verification

Added app-server coverage for rejecting archived thread resume by id and checking that the error includes the matching codex unarchive <thread id> command.

@etraut-openai etraut-openai force-pushed the etraut/archive-thread-commands branch from 74ee516 to 933e434 Compare May 29, 2026 05:57
@etraut-openai etraut-openai requested a review from a team as a code owner May 29, 2026 05:57
@etraut-openai etraut-openai changed the base branch from etraut/extract-goal-extension to main May 29, 2026 05:57
@etraut-openai etraut-openai force-pushed the etraut/archive-thread-commands branch from 933e434 to 629f003 Compare May 29, 2026 06:02
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 629f003310

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codex-rs/tui/src/session_archive_commands.rs
Comment thread codex-rs/cli/src/main.rs
@etraut-openai etraut-openai force-pushed the etraut/archive-thread-commands branch 3 times, most recently from 1aa6890 to f1049e1 Compare May 29, 2026 06:40
Problem: Saved threads can be archived through app-server RPCs, but the command line has no direct archive or unarchive entrypoint.

Solution: Add codex archive and codex unarchive commands that resolve thread UUIDs or exact names and call the existing app-server archive RPCs.
@etraut-openai etraut-openai force-pushed the etraut/archive-thread-commands branch from f1049e1 to 5e63b78 Compare May 29, 2026 06:44
@etraut-openai etraut-openai changed the title Add thread archive commands Add thread archive CLI commands May 29, 2026
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5e63b786ca

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codex-rs/cli/src/main.rs Outdated
Comment thread codex-rs/tui/src/thread_archive_command.rs Outdated
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 34a90a0c77

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codex-rs/app-server/src/request_processors/thread_processor.rs
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: efc80b1022

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

archived: Some(matches!(action, SessionArchiveAction::Unarchive)),
cwd: None,
use_state_db_only: false,
search_term: None,
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 Filter archive name lookups server-side

When codex archive <name> or codex unarchive <name> resolves a name, this omits search_term, so the loop pages through every active/archived session until it finds an exact match or exhausts history. For users with large or repaired rollout histories—especially a missing archived name—this can force a full scan/repair of the session store instead of using the app-server title filter already used by resume name lookup; pass search_term: Some(name.to_string()) here and keep the exact-name check on the filtered results.

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 a good catch. It should be consistent with codex resume behavior. Fixed.

Copy link
Copy Markdown
Contributor

@fcoury-oai fcoury-oai left a comment

Choose a reason for hiding this comment

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

Exercised the happy path and it worked as expected. The code looks good as well.

Codex found 3 issues, 2 are non-blocking in my opinion. You may want to address the duplicate session name one, since this mutates the session as explained before.

I will approve this and leave the decision of which issues to address up to you.

limit: Some(100),
sort_key: Some(ThreadSortKey::UpdatedAt),
sort_direction: None,
model_providers: None,
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.

Issue found by Codex below. In my opinion this can either be addressed here or as a separate PR. Non-blocking.

model_providers: None is interpreted by app-server as the current configured provider only. The archive command accepts and merges --oss / --local-provider, but its config bootstrap never applies those provider overrides. Name-based archive and unarchive therefore cannot find sessions from another provider, even when the user explicitly supplies the matching OSS flags. UUID lookup still works.

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.

I'm going to leave this as is because it's consistent with how codex resume works. If we decide to change this, we should do so everywhere.

.await
.wrap_err("failed to list sessions while resolving session name")?;

if let Some(thread) = response
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.

Session names are not unique, but lookup returns the first exact match sorted by updatedAt. Resume has similar lookup behavior, but archive is a storage mutation. Picking one silently can archive or unarchive the wrong session.

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.

Yeah, I thought about that. If /archive was non-reversible, I'd feel more strongly about this. Since it's easily reversible, I think this is reasonable. The alternative is to 1) not allow name lookup, 2) do a full (potentially slow) scan for duplicate names and ask the user for confirmation.

archived: Some(matches!(action, SessionArchiveAction::Unarchive)),
cwd: None,
use_state_db_only: false,
search_term: None,
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.

Another non-blocking issue: archive commands can become slow for users with large histories. The reason I think this can be handled as a separate PR is because fork and resume exhibit the same issue. We should eventually fix this behavior.

Unlike the existing resume-by-name path, this sends search_term: None and pages through the complete active or archived history. That creates unnecessary RPCs and read-repair work, especially for missing names.

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.

Yes, I agree this should not be tackled in this PR.

etraut-openai added a commit that referenced this pull request May 29, 2026
## Why

TUI users can archive saved sessions from other surfaces, but there is
no in-session command for archiving the active session. Since archiving
the active session also exits the TUI, the command should ask for
explicit confirmation instead of firing immediately.

I'm also working on [a companion
PR](#25021) that adds `codex
archive` and `codex unarchive` top-level CLI commands.

## What changed

- Adds a new `/archive` slash command described as `archive this session
and exit`.
- Shows a confirmation dialog with `No, don't archive` selected first
and `Yes, archive and exit` as the explicit action.
- On confirmation, calls the existing `thread/archive` app-server RPC
for the active main session and exits after success.
- Keeps `/archive` disabled while a task is running and unavailable in
side conversations.

## Verification

Added focused TUI coverage for the `/archive` confirmation flow,
disabled-while-task-running behavior, and the `/ar` slash-command popup
snapshot.
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 17f9eab872

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codex-rs/tui/src/session_archive_commands.rs
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d6a1109595

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +2983 to +2987
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."
)));
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.

@etraut-openai etraut-openai merged commit 3e7baa0 into main May 30, 2026
31 checks passed
@etraut-openai etraut-openai deleted the etraut/archive-thread-commands branch May 30, 2026 06:37
@github-actions github-actions Bot locked and limited conversation to collaborators May 30, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants