Show goal-started threads in resume lists#21489
Conversation
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ 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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ 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". |
jif-oai
left a comment
jif-oai
left a comment
There was a problem hiding this comment.
Stepping back, I’m still not convinced this is the right shape for the fix. The bug is real, but this makes ThreadGoalUpdated behave like a synthetic first user message across persistence, metadata extraction, and listing, which feels like we’re solving a preview/discoverability problem by widening the meaning of first_user_message.
We should probably model the user-facing preview/discoverability bit directly instead of making goal lifecycle events stand in for user input.
What do you think?
| | EventMsg::AgentReasoningRawContent(_) | ||
| | EventMsg::PatchApplyEnd(_) | ||
| | EventMsg::TokenCount(_) | ||
| | EventMsg::ThreadGoalUpdated(_) |
There was a problem hiding this comment.
For the record, this won't backfill previous sessions but it's fine I guess
| } | ||
| } | ||
| } | ||
| EventMsg::ThreadGoalUpdated(event) => { |
There was a problem hiding this comment.
Won't this have an impact on the title? If a reaul user message arrives after the goal, it will now look like a distinct title and start shwoing up as thread name
Maybe I'm wrong though, I can't repro
2 participants
Fixes #20792
Why
Threads that start with
/goalcan have useful user-facing starting metadata even before a normalUserMessageappears. Resume listing currently filters out rollouts unless it sees both session metadata and a user-facing start event, and the state metadata path only setsfirst_user_messagefromUserMessage. That made/goal-first sessions resumable by id but invisible incodex resumeand Desktop recents.The earlier approach in #20800 attempted to fix this by materializing a synthetic
/goaluser message. This PR takes a narrower route: treat the already-emittedThreadGoalUpdatedevent as metadata-bearing rollout state and derive the preview from the goal objective.What changed
ThreadGoalUpdatedevents in the limited rollout stream.first_user_messagefrom the first non-empty goal objective.Verification
first_user_messagefromThreadGoalUpdated.cargo test -p codex-statecargo test -p codex-rolloutManually confirmed the bug (before) and fix (after).