Skip to content

Accept a caller-chosen thread id on thread start - #24

Merged
thedancingdeveloper merged 2 commits into
fork/mainfrom
fork/wi-1126-caller-thread-id
Oct 12, 2026
Merged

thedancingdeveloper merged 2 commits into
fork/mainfrom
fork/wi-1126-caller-thread-id

Conversation

@thedancingdeveloper

Copy link
Copy Markdown

Summary

  • thread/start accepts an experimental threadId (any UUID version). The server rejects a duplicate, an invalid id, and a caller id on an ephemeral thread, then passes the id through the existing reserved_thread_id path so the rollout is keyed by it. No core change.
  • The TUI and codex exec expose the same choice as public --session-id. It is refused with resume, fork, and ephemeral sessions.
  • A missing rollout is a store InvalidRequest ("no rollout found for thread id …"), not ThreadNotFound; that case is treated as absent. Resume of a brand-new thread is not asserted, because a fresh thread is not materialized until the first user message.

Test plan

  • cargo test -p codex-app-server --test all thread_start_ — 56 passed, including honor, invalid id, and ephemeral rejection
  • cargo check -p codex-tui -p codex-exec

Vogt needs to hibernate and resume a Codex session under the engine's own id. thread/start takes an experimental threadId, and the TUI and exec expose it as --session-id. A duplicate, an invalid UUID, or an ephemeral thread is rejected. Core already honours reserved_thread_id, so the rollout path is keyed by the caller's id without a core change.
@thedancingdeveloper

Copy link
Copy Markdown
Author

Review of head b76278c (no merge, no push).

Caller thread id is wired the way the brief asks. ThreadStartParams.thread_id is optional and experimental. It is parsed with ThreadId::from_string (any UUID version), rejected when live or when a rollout/archive file exists, and ThreadStoreError::InvalidRequest with the exact "no rollout found for thread id …" message (plus ThreadNotFound) is treated as absent. The id is passed through stage_pending_thread_metadata and start_options.reserved_thread_id; core is untouched and already honors that field on New/Cleared/Forked. Ephemeral rejects before parse. TUI and codex exec expose public --session-id and refuse it with resume, fork, or ephemeral; exec applies it only on the fresh-start path. thread/fork still passes None. v4 degrades: memory storage falls back when get_timestamp is None, and the honors test uses a v4 id and checks the rollout path. Resume-before-first-message is not asserted, which matches a rollout that does not exist until the first user message.

Non-blocking: the store-error check is an exact string match (same pattern already used in this file); no separate v4 unit test beyond the honors case; no generated schema artifact in the diff.

no blockers (head b76278c)

A global flag bound the positional SESSION_ID on resume and fork, so those
commands rejected a caller id they never received. Leave the flag on the root
exec command only, and refresh the experimental protocol export for threadId.
@thedancingdeveloper

Copy link
Copy Markdown
Author

Reviewed head 98c54e5 (12 files, against fork/main) from a detached worktree, not the shared checkout.

The latest commit does what it says. exec --session-id is no longer global = true, so resume/fork keep their positional SESSION_ID. The root command still rejects --session-id together with resume, fork, or --ephemeral before start_thread is called, and the new-thread path is the only one that forwards session_id into ThreadStartParams.thread_id. The TUI flag is not global and gets the same refusal in run_ratatui_app, then is written onto thread/start in StartupLaunchChoices::configure before the profile early return.

On the server, a caller id is parsed with ThreadId::from_string (any UUID version), refused for ephemeral threads, and refused when it is already live, already in the thread store (including archived, include_archived: true), or already on disk under sessions/ or archived_sessions/. The "no rollout found for thread id …" InvalidRequest is the exact string read_thread returns for a missing rollout, so a brand-new id is treated as absent rather than a 500. The id then replaces reserve_thread_id() and is set on StartThreadOptions.reserved_thread_id.

v4 degrades. The rollout directory and filename timestamp come from local time in the recorder, not from the UUID, and ThreadId::from_string accepts every version. The v7 filter in rollout_reconstruction.rs is on compaction window ids, not the thread id, so a v4 thread id does not fail reconstruction. thread_start_honors_caller_thread_id covers a v4 id, persistence under that id, and the duplicate rejection.

No defect that would break real use of a caller-chosen thread id.

no blockers (head 98c54e5)

@thedancingdeveloper
thedancingdeveloper merged commit 30d3201 into fork/main Oct 12, 2026
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant