add branch name w/ sync function for git info #2142
Conversation
pap-openai
commented
Aug 10, 2025
pap-openai
commented
codex-rs/tui/src/history_cell.rs
Outdated
| @@ -212,14 +212,27 @@ impl HistoryCell { | |||
| None => config.cwd.display().to_string(), | |||
| }; | |||
|
|
|||
| // Determine the current Git branch (if any) for display using | |||
| // the shared Git info collector in a way that is safe in sync contexts. | |||
| let branch_suffix = codex_core::git_info::collect_git_info_blocking(&config.cwd) | |||
There was a problem hiding this comment.
Can we make this part of the SessionConfiguredEvent so we can do all this work async?
There was a problem hiding this comment.
thanks! should be better now
| // Gather history metadata for SessionConfiguredEvent. | ||
| let (history_log_id, history_entry_count) = | ||
| crate::message_history::history_metadata(&config).await; | ||
| // Gather history metadata for SessionConfiguredEvent and await git info. |
There was a problem hiding this comment.
I feel like we should be specifying the timeout somehow at this callsite (either updating collect_git_info() to take a timeout, or wrapping these calls with tokio::time::timeout) to ensure that this runs in a bounded amount of time.
One challenge is that here, we are latency-sensitive and I agre with the short timeout.
Though the other callsite in rollout.rs is less latency-sensitive, and arguably should be able to specify a more generous timeout.
That said, from the callsite, one would expect to specify the timeout of the overall operation, whereas internally inside collect_git_info(), run_git_command_with_timeout() is run twice in series, so if collect_git_info() took its own timeout argument, arguably we should divide it by two and use the halved version for each internal call to run_git_command_with_timeout()?
I admit there's already the existing issue here that crate::message_history::history_metadata() should also be bound by a timeout, though I don't know what value to use (0?) if the timeout expires before it completes.
| @@ -883,6 +896,8 @@ impl HistoryCell { | |||
| } | |||
| } | |||
|
|
|||
| // | |||
