Add user command event types #6246
Conversation
zhao-oai
force-pushed
the
split/user-shell/part-1
branch
from
47c999a to
db43ab0
Compare
zhao-oai
force-pushed
the
split/user-shell/part-1
branch
4 times, most recently
from
c042acf to
db43ab0
Compare
codex-rs/core/src/rollout/policy.rs
Outdated
| | EventMsg::ExitedReviewMode(_) | ||
| | EventMsg::UndoCompleted(_) | ||
| | EventMsg::TurnAborted(_) => true, | ||
| EventMsg::ExecCommandBegin(ev) => ev.is_user_shell_command, |
There was a problem hiding this comment.
why do we want to persist the user ones?
There was a problem hiding this comment.
since the model will now be able to see the outputs of user shell commands, it wouldn't make sense if they are lost after resuming conversation. we're also persisting the associated response items since user shell input / outputs are represented as User Messages.
There was a problem hiding this comment.
I think we should persist only the response item user message, the event message is just for the UI. We decided to ignore execitems to make it easy to keep things backward comp.
| expression: rendered | ||
| --- | ||
| • Ran seq 1 10 1>&2 && false | ||
| • Ran: seq 1 10 1>&2 && false |
There was a problem hiding this comment.
looks like an unrelated change.
There was a problem hiding this comment.
i thought adding a colon would make it clearer since user commands will render as
You Ran: ls instead of You Ran ls for example
this is somewhat adjacent though so i'll get this in another PR
codex-rs/protocol/src/protocol.rs
Outdated
| /// True when this exec was initiated directly by the user (e.g. bang command), | ||
| /// not by the agent/model. Defaults to false for backwards compatibility. | ||
| #[serde(default)] | ||
| pub is_user_shell_command: bool, |
There was a problem hiding this comment.
Do we actually need this on the delta itself of can it be on start ?
pakrym-oai
left a comment
pakrym-oai
left a comment
There was a problem hiding this comment.
is it enought to have is_user_shell_command only on the start?
codex-rs/tui/src/exec_cell/render.rs
Outdated
| let title = if self.is_active() { | ||
| "Running" | ||
| } else if call.is_user_shell_command { | ||
| "You Ran" |
There was a problem hiding this comment.
yes i feel like this is a lot clearer vs. having it all be "Ran"
There was a problem hiding this comment.
I mean You ran ... lowercase r.
There was a problem hiding this comment.
@codex could you change "You Ran" to "You ran"?
There was a problem hiding this comment.
Summary
- Lowercased the user shell command header so completed commands now read “You ran …” per review feedback. codex-rs/tui/src/exec_cell/render.rsL312-L326
Testing
- ✅
just fmt(warnings aboutimports_granularity = Itembeing nightly-only)
zhao-oai
force-pushed
the
split/user-shell/part-1
branch
from
e28905c to
3f81883
Compare
This reverts commit 5f6983a.
zhao-oai
force-pushed
the
split/user-shell/part-1
branch
from
3f81883 to
056d13f
Compare
emit new events from user shell tasks, disable sandboxing for user shell commands, and present user shell commands to model
adding new user command event, logic in TUI to render user command events