app-server: preserve thread git info across live snapshots#1
Draft
Fuiste wants to merge 2 commits into
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements a small server-side correctness fix for thread git identity in the app-server.
Several live-thread lifecycle paths rebuilt
Threadvalues from config snapshots and ended up emittinggit_info: None, even when canonical git metadata was already available from persisted thread metadata or rollout summaries. That made downstream UI state vulnerable to branch/repo identity loss during thread reads, forks, and listener reattachment.This patch keeps the API shape unchanged and narrows the change to app-server state assembly.
Root Cause
build_thread_from_snapshot(...)constructs aThreadfrom in-memory snapshot/config state, but it does not hydrate git metadata. A few lifecycle paths were using that snapshot as the final truth for the thread returned to clients.That meant
thread.gitInfocould be dropped on:thread/startthread/readthread/forkMaterialized fork responses also needed a small fallback because the newly created fork summary could still be blank for git metadata even though the source rollout had the branch/origin information.
What Changed
Commit 1:
fix(app-server): preserve thread git info in live snapshotsbest_available_thread_git_info(...)incodex-rs/app-server/src/codex_message_processor.rsNoneonly if neither source has git metadatabuild_thread_from_snapshot_with_git_info(...)thread.git_infofrom canonical sources after the snapshot is builtsummary_to_thread(...)for consistencyCommit 2:
test(app-server): cover git info hydration on read and forkAdded focused regression coverage for the failure modes this patch targets:
thread/readpreserves persisted git metadatathread/forkpreserves branch/origin identitythread/forkpreserves branch/origin identityThere is already existing resume coverage in
thread_resume_prefers_persisted_git_metadata_for_local_threads, which still passes and helps confirm the patch remains aligned with the existing persisted-metadata preference.Scope / Non-Goals
This PR intentionally does not:
Threadfieldsthread/metadata/updatesemanticsThe goal is only to stop dropping already-known branch/repo identity in server responses.
Validation
Targeted tests run:
cargo test -p codex-app-server --test all thread_read_loaded_thread_preserves_persisted_git_infocargo test -p codex-app-server --test all thread_fork_preserves_git_infocargo test -p codex-app-server --test all thread_resume_prefers_persisted_git_metadata_for_local_threadsRepo-mandated cleanup run:
just fix -p codex-app-serverjust fmtAdditional note:
cargo test -p codex-app-serverrun surfaced three unrelated failures incommand_execandrealtime_conversation. I did not touch those areas, and I did not rerun the full suite afterjust fixbecause the repo instructions explicitly say not to rerun tests after the fix/fmt pass.Why This Helps
This does not solve stale PR attachment state by itself, but it leaves the architecture in a better place by making
thread.gitInfoa more trustworthy representation of thread branch/repo identity across the lifecycle paths that were previously lossy.That gives downstream clients a cleaner foundation for any future PR-state revalidation behavior.