fix(app-server, core): defer initial context write to rollout file until first turn #9950
Conversation
…des on thread/resume
owenlin0
force-pushed
the
owen/resume_with_no_overrides
branch
from
68d3b8d to
ed26d86
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
owenlin0
changed the title
owenlin0
changed the title
owenlin0
changed the title
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31590809ee
ℹ️ 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".
| let initial_context_seeded = sess.state.lock().await.initial_context_seeded; | ||
| if !initial_context_seeded { | ||
| return; |
There was a problem hiding this comment.
Because initial_context_seeded is set to false on resumed sessions, this early return skips recording any OverrideTurnContext update items. If a user resumes a thread, tweaks settings (e.g., cwd/approval/collaboration/personality), and then closes without starting a turn, those changes stay in memory only and are never written to the rollout, so they’re lost on the next resume and updated_at doesn’t reflect the modification. Previously overrides were persisted immediately, so this is a behavior regression for resumed threads.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
that's fine for now
Overview
Currently calling
thread/resumewill always bump the thread'supdated_attimestamp. This PR makes it theupdated_attimestamp changes only if a turn is triggered.Additonal context
What we typically do on resuming a thread is always writing “initial context” to the rollout file immediately. This initial context includes:
This PR defers writing the “initial context” to the rollout file until the first
turn/start, so we don't inadvertently bump the thread'supdated_attimestamp until a turn is actually triggered.This works even though both
thread/resumeandturn/startaccept overrides (such asmodel,cwd, etc.) because the initial context is seeded from the effectiveTurnContextin memory, computed atturn/starttime, after both sets of overrides have been applied.NOTE: This is a very short-lived solution until we introduce sqlite. Then we can remove this.