Skip to content

Unify thread metadata updates above store#22236

Merged
wiltzius-openai merged 8 commits into
mainfrom
wiltzius/codex/thread-metadata-unified
May 13, 2026
Merged

Unify thread metadata updates above store#22236
wiltzius-openai merged 8 commits into
mainfrom
wiltzius/codex/thread-metadata-unified

Conversation

@wiltzius-openai
Copy link
Copy Markdown
Contributor

@wiltzius-openai wiltzius-openai commented May 12, 2026 •
edited
Loading

  • make ThreadStore::update_thread_metadata accept a broad range of metadata patches
  • keep ThreadStore::append_items as raw canonical history append (no metadata side effects)
  • in the local store, write these metadata updates to a combination of sqlite and rollout jsonl files for backwards-compat. It special cases which fields need to go into jsonl vs sqlite vs whatever, confining the awkwardness to just this implementation
  • in remote stores we can simply persist the metadata directly to a database, no special casing required.
  • move the "implicit metadata updates triggered by appending rollout items" from the RolloutRecorder (which is local-threadstore-specific) to the LiveThread layer above the ThreadStore, inside of a private helper utility called ThreadMetadataSync. LiveThread calls ThreadStore append_items and update_metadata separately.
  • Add a generic update metadata method to ThreadManager that works on both live threads and "cold" threads
  • Call that ThreadManager method from app server code, so app server doesn't need to worry about whether the thread is live or not

@wiltzius-openai wiltzius-openai force-pushed the wiltzius/codex/thread-metadata-unified branch 7 times, most recently from 5090dd5 to bb62b0e Compare May 12, 2026 01:30
@github-actions github-actions Bot mentioned this pull request May 12, 2026
Open
@wiltzius-openai wiltzius-openai force-pushed the wiltzius/codex/thread-metadata-unified branch 7 times, most recently from 74caa46 to 6095e6d Compare May 12, 2026 05:06
@wiltzius-openai
Copy link
Copy Markdown
Contributor Author

wiltzius-openai commented May 12, 2026

@codex review

chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed May 12, 2026
View reviewed changes
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: 6095e6da42

ℹ️ 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/thread-store/src/local/update_thread_metadata.rs
@wiltzius-openai
Copy link
Copy Markdown
Contributor Author

wiltzius-openai commented May 12, 2026

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown
Contributor

chatgpt-codex-connector Bot commented May 12, 2026

Codex Review: Didn't find any major issues. You're on a roll.

ℹ️ 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".

@wiltzius-openai
Copy link
Copy Markdown
Contributor Author

wiltzius-openai commented May 12, 2026

@codex review

chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed May 12, 2026
View reviewed changes
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: e173b815f8

ℹ️ 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/thread-store/src/live_thread.rs
@wiltzius-openai
Copy link
Copy Markdown
Contributor Author

wiltzius-openai commented May 12, 2026

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown
Contributor

chatgpt-codex-connector Bot commented May 12, 2026

Codex Review: Didn't find any major issues. Hooray!

ℹ️ 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".

@wiltzius-openai wiltzius-openai marked this pull request as ready for review May 12, 2026 16:49
@wiltzius-openai wiltzius-openai requested a review from a team as a code owner May 12, 2026 16:49
@wiltzius-openai
Defer token count until pending tools finish
c82cfe9 
The thread metadata rewrite made persisted TokenCount events part of the
observable storage path, which exposed an existing ordering issue in a few
core tests. We were emitting TokenCount as soon as token usage was computed,
even when the turn still had pending tool results or user-input work to
finish.

Split token usage bookkeeping from TokenCount event emission so the session
state can still be updated immediately, while the externally visible
TokenCount event is deferred until the turn is actually complete. This keeps
turn history ordered around tool continuations and avoids recording a token
count before the work it describes has settled.

The test updates in this commit are intentionally separate from the storage
rewrite: they document the corrected event ordering and remove assertions that
were depending on the old premature TokenCount timing.
@wiltzius-openai wiltzius-openai force-pushed the wiltzius/codex/thread-metadata-unified branch 2 times, most recently from ed2f967 to 03928b0 Compare May 12, 2026 17:03
owenlin0
owenlin0 reviewed May 12, 2026
View reviewed changes
Comment thread codex-rs/thread-store/src/types.rs Outdated
owenlin0
owenlin0 reviewed May 12, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@owenlin0 owenlin0 left a comment

Choose a reason for hiding this comment

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

A couple of thread-store metadata edge cases I think are worth tightening up.

Comment thread codex-rs/thread-store/src/local/update_thread_metadata.rs Outdated
Comment thread codex-rs/thread-store/src/local/update_thread_metadata.rs Outdated
@wiltzius-openai
Unify thread metadata updates above store
737f4fe 
Make the ThreadStore contract coherent: append_items is now a raw canonical
history append, and update_thread_metadata is the single metadata write API.
ThreadStore implementations no longer infer metadata by inspecting appended
rollout items.

LiveThread now owns the live append flow. It applies the rollout persistence
policy before writing JSONL history, observes the canonical items with a small
ThreadMetadataSync helper, and sends the derived facts back through
update_thread_metadata. ThreadManager becomes the single app-facing metadata
entrypoint, routing loaded threads through LiveThread and cold threads directly
to the store.

LocalThreadStore preserves the local compatibility requirements inside the
local implementation. SQLite, when available, receives explicit metadata
updates. JSONL and name-index compatibility are still maintained for explicit
metadata mutations such as rename, memory-mode, and git changes, so old rollout
files and SQLite-less local storage remain readable.

RolloutRecorder is reduced to local JSONL writing for thread-store appends.
The shared rollout persistence policy remains in codex-rollout, but recorder no
longer owns live metadata synthesis. The added thread-store README captures the
new ownership boundaries for future remote and local implementations.
@wiltzius-openai
Avoid git probes outside repositories
17fe70f 
The metadata rewrite added git metadata collection to live thread creation and
kept SessionMeta git collection in the rollout recorder. Those paths can also
run with temporary working directories that are not git repositories,
especially in app-server integration tests.

collect_git_info may spawn git and wait for its timeout before discovering that
a directory is not a repository. On slower CI platforms that extra subprocess
work made unrelated app-server startup tests miss their existing deadlines.

Guard the call sites with get_git_repo_root first. Non-repository directories
now skip git probing and record no git metadata, while real repository working
directories still collect the same commit, branch, and remote information as
before. This is a focused performance/behavior fix rather than a test timeout
increase.
@wiltzius-openai
Reject observed metadata for missing local threads
15b4a6e 
Observed metadata updates are facts derived from a live append stream. They
should refine metadata for a thread whose history already exists; they should
not create a new local thread on their own.

Tighten the local update path so an observed-only update for an unknown thread
returns ThreadNotFound instead of fabricating SQLite metadata from the observed
facts. User/API metadata mutations can still resolve the rollout first and use
the existing compatibility path for cold local threads.

This keeps accidental or reordered live metadata updates from creating phantom
threads, and it makes the local implementation match the intended store model:
history creates the thread, metadata updates describe it.
@wiltzius-openai
Use live rollout paths for observed metadata
991fe4c 
A live local thread can have a rollout path known to the live writer before the
JSONL file is materialized on disk. Observed metadata updates that arrive in
that window need to attach to the same path instead of trying to rediscover the
thread through filesystem scans.

Teach LocalThreadStore's metadata update path to consult the live writer for an
active rollout path and use that path when applying observed metadata. This
keeps SQLite metadata pointed at the eventual JSONL file even when the metadata
update runs before the file is visible to read/list fallback code.

Remote stores do not need rollout paths; this remains a local-store detail that
preserves JSONL and SQLite compatibility without leaking rollout-path handling
back into LiveThread.
@wiltzius-openai wiltzius-openai force-pushed the wiltzius/codex/thread-metadata-unified branch from 03928b0 to 991fe4c Compare May 12, 2026 20:19
owenlin0
owenlin0 reviewed May 12, 2026
View reviewed changes
Comment thread codex-rs/thread-store/src/local/mod.rs
Comment thread codex-rs/thread-store/src/types.rs Outdated
Comment thread codex-rs/thread-store/src/types.rs
owenlin0
owenlin0 reviewed May 12, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@owenlin0 owenlin0 left a comment

Choose a reason for hiding this comment

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

One small readability note on the metadata test.

Comment thread codex-rs/thread-store/src/local/update_thread_metadata.rs
owenlin0
owenlin0 reviewed May 12, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@owenlin0 owenlin0 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

.

@wiltzius-openai wiltzius-openai force-pushed the wiltzius/codex/thread-metadata-unified branch 2 times, most recently from a721bff to 6341d4b Compare May 12, 2026 22:20
owenlin0
owenlin0 reviewed May 12, 2026
View reviewed changes
Comment thread codex-rs/thread-store/src/types.rs
owenlin0
owenlin0 approved these changes May 12, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@owenlin0 owenlin0 left a comment

Choose a reason for hiding this comment

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

just one last comment but lgtm! heroic effort, thanks for cleaning a lot of the existing mess up while doing this

@wiltzius-openai wiltzius-openai enabled auto-merge (squash) May 13, 2026 00:11
@wiltzius-openai wiltzius-openai merged commit c51c65a into main May 13, 2026
27 checks passed
@wiltzius-openai wiltzius-openai deleted the wiltzius/codex/thread-metadata-unified branch May 13, 2026 00:28
@github-actions github-actions Bot locked and limited conversation to collaborators May 13, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

@owenlin0 owenlin0 owenlin0 approved these changes

Assignees

@owenlin0 owenlin0

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@wiltzius-openai @owenlin0