Skip to content

feat(core): emit canonical command execution items - #31297

Merged
owenlin0 merged 2 commits into
mainfrom
owen/canonical-command-execution-items
Jul 7, 2026
Merged

feat(core): emit canonical command execution items#31297
owenlin0 merged 2 commits into
mainfrom
owen/canonical-command-execution-items

Conversation

@owenlin0

@owenlin0 owenlin0 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

This PR depends on #31296 for the canonical-to-legacy event mappings.

Description

This PR makes command execution emit canonical TurnItem::CommandExecution lifecycle from both the shell tool path and user /shell commands.

App-server v2 consumes the canonical command items directly and ignores the mapped ExecCommandBegin / ExecCommandEnd compatibility events, so clients still receive one command item lifecycle.

UnifiedExecInteraction stays on the legacy path because TerminalInteraction is still the v2 surface for stdin and poll events. Emitting a command item there would render the same wait twice.

Why

This is the first live producer migration after the compatibility mappings in #31296. Keeping command execution separate makes the unified exec exception reviewable without mixing in dynamic tools or multi-agent behavior.

What changed

  • Emit canonical command execution items from shell tool events and user shell commands.
  • Preserve the existing unified exec interaction carveout.
  • Move app-server command deduplication and completion bookkeeping onto canonical item events.
  • Update unified exec coverage to assert the completed command item.

ctx.session
.send_event(
ctx.turn,
EventMsg::ExecCommandBegin(ExecCommandBeginEvent {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be a legacy event?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is, yeah

.lock()
.await
.turn_summary
.command_execution_started

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we still need this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately we do, we synthesize a ThreadItem::CommandExecution whenever an approval is requested (user or guardian), so this helps us dedupe

@pakrym-oai pakrym-oai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is having matches!(exec_input.source, ExecCommandSource::UnifiedExecInteraction) cool?

Base automatically changed from owen/canonical-turn-item-legacy-fanout to main July 7, 2026 03:41
@owenlin0
owenlin0 force-pushed the owen/canonical-command-execution-items branch from 16ac27f to 1ec93f2 Compare July 7, 2026 03:54
@owenlin0
owenlin0 merged commit cca16a1 into main Jul 7, 2026
64 of 66 checks passed
@owenlin0
owenlin0 deleted the owen/canonical-command-execution-items branch July 7, 2026 04:22
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 7, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants