Skip to content

Migrate archive/unarchive to local ThreadStore#17892

Merged
wiltzius-openai merged 1 commit into
mainfrom
wiltzius/codex/thread-store-archive-unarchive
Apr 15, 2026
Merged

Migrate archive/unarchive to local ThreadStore#17892
wiltzius-openai merged 1 commit into
mainfrom
wiltzius/codex/thread-store-archive-unarchive

Conversation

@wiltzius-openai
Copy link
Copy Markdown
Contributor

@wiltzius-openai wiltzius-openai commented Apr 15, 2026 •
edited
Loading

Summary

  • implement local ThreadStore archive/unarchive operations
  • implement local ThreadStore read_thread operation
  • break up the various ThreadStore local method implementations into separate files
  • migrate app-server archive/unarchive and core archive fixture to use ThreadStore (but not all read operations yet!)
  • use the ThreadStore's read operation as a proxy check for thread persistence/existence in the app server code
  • move all other filesystem operations related to archive (path validation etc) into the local thread store.

Tests

  • add dedicated local store archive/unarchive tests
@wiltzius-openai wiltzius-openai force-pushed the wiltzius/codex/thread-store-archive-unarchive branch from dc9e0b4 to 0842a24 Compare April 15, 2026 04:43
pakrym-oai
pakrym-oai reviewed Apr 15, 2026
View reviewed changes
Comment thread codex-rs/app-server/src/codex_message_processor.rs Outdated

let thread_id_str = thread_id.to_string();
match self.archive_thread_common(thread_id, &rollout_path).await {
let store = LocalThreadStore::new(codex_rollout::RolloutConfig::from_view(&self.config));
Copy link
Copy Markdown
Collaborator

@pakrym-oai pakrym-oai Apr 15, 2026

Choose a reason for hiding this comment

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

let's stash the store on self so we don't have to re-create it every time.
Copy link
Copy Markdown
Contributor Author

@wiltzius-openai wiltzius-openai Apr 15, 2026

Choose a reason for hiding this comment

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

done
pakrym-oai
pakrym-oai reviewed Apr 15, 2026
View reviewed changes
Comment thread codex-rs/thread-store/src/local/mod.rs Outdated
}

#[tokio::test]
async fn archive_thread_moves_rollout_to_archived_collection() {
Copy link
Copy Markdown
Collaborator

@pakrym-oai pakrym-oai Apr 15, 2026

Choose a reason for hiding this comment

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

It's very-very good to have tests for these but let's move them out into their own file (or files, per op?) otherwise this mod will get huge.
Copy link
Copy Markdown
Contributor Author

@wiltzius-openai wiltzius-openai Apr 15, 2026

Choose a reason for hiding this comment

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

split into multiple files, one per ThreadStore method plus some helpers
pakrym-oai
pakrym-oai reviewed Apr 15, 2026
View reviewed changes
Comment thread codex-rs/thread-store/src/local/mod.rs Outdated

impl PreparedLocalThreadArchive {
/// Move the prepared rollout into the archived collection.
pub async fn complete(self) -> ThreadStoreResult<()> {
Copy link
Copy Markdown
Collaborator

@pakrym-oai pakrym-oai Apr 15, 2026

Choose a reason for hiding this comment

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

this isn't abstractable.
Copy link
Copy Markdown
Contributor Author

@wiltzius-openai wiltzius-openai Apr 15, 2026

Choose a reason for hiding this comment

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

now replaced by the reworked thread lookup followed by app-server-internal prepare followed by ThreadStore archive call
pakrym-oai
pakrym-oai reviewed Apr 15, 2026
View reviewed changes
Comment thread codex-rs/thread-store/src/local/mod.rs Outdated
}

/// Prepare to archive a local thread by resolving and validating its active rollout.
pub async fn prepare_archive_thread(
Copy link
Copy Markdown
Collaborator

@pakrym-oai pakrym-oai Apr 15, 2026

Choose a reason for hiding this comment

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

it's a bit strange to have the two stage archive. Can we shutdown the thread first and have a single step archive call?
Copy link
Copy Markdown
Contributor Author

@wiltzius-openai wiltzius-openai Apr 15, 2026

Choose a reason for hiding this comment

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

now replaced by the reworked thread lookup followed by app-server-internal prepare followed by ThreadStore archive call
@wiltzius-openai wiltzius-openai force-pushed the wiltzius/codex/thread-store-listing branch from 874e83c to 6676bbd Compare April 15, 2026 17:02
@wiltzius-openai wiltzius-openai force-pushed the wiltzius/codex/thread-store-archive-unarchive branch 3 times, most recently from f585746 to 71b0c1b Compare April 15, 2026 18:18
Base automatically changed from wiltzius/codex/thread-store-listing to main April 15, 2026 18:34
@wiltzius-openai wiltzius-openai force-pushed the wiltzius/codex/thread-store-archive-unarchive branch 3 times, most recently from a5c5e0c to d77c2af Compare April 15, 2026 19:05
@wiltzius-openai wiltzius-openai force-pushed the wiltzius/codex/thread-store-archive-unarchive branch from d77c2af to 379acd5 Compare April 15, 2026 20:25
@wiltzius-openai wiltzius-openai enabled auto-merge (squash) April 15, 2026 20:39
@wiltzius-openai wiltzius-openai merged commit 50d3128 into main Apr 15, 2026
35 of 36 checks passed
@wiltzius-openai wiltzius-openai deleted the wiltzius/codex/thread-store-archive-unarchive branch April 15, 2026 20:48
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 15, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

2 participants

@wiltzius-openai @pakrym-oai