Skip to content

fix(acp): centralize session activity lifecycle - #107

Merged
danielkov merged 5 commits into
mainfrom
fix/acp-session-activity
Sep 5, 2026
Merged

fix(acp): centralize session activity lifecycle#107
danielkov merged 5 commits into
mainfrom
fix/acp-session-activity

Conversation

@danielkov

@danielkov danielkov commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Make one session-owned instrument the source of lifecycle state for ACP v1 and v2, and make the TUI consume ACP state directly. Background completion wakeups that find no work no longer produce synthetic turns or repeated took 0ms rows.

Technical details

Shared state and execution finalization

The instrument owns activity identity, execution origin, Running/Settling/Idle state, and terminal settlement. Actual loop activity starts an interval; steering and background-result synthesis continue within it. Both protocols use shared domain outcomes and a common cancellation, structured-work cleanup, content-drain, and diagnostic finalization policy.

Ordered protocol projections

Protocol adapters translate the instrument鈥檚 transitions rather than maintain their own lifecycle state. ACP v1 preserves standard prompt responses and its autonomous-turn extension; ACP v2 emits native state updates. Lifecycle ownership is separate from response replacement.

V1 lifecycle notifications enter the SDK outbound queue synchronously, and terminal settlement follows the content-drain barrier, including on error paths. Flush failures are no longer silently treated as successful completion.

Reactive client state

The TUI retains ACP state and terminal reasons directly and uses one timer across Running and RequiresAction states.

Retiring cancelled driver continuations

Cancellation at a tool-result boundary retires the driver's logical turn before session activity settles, so a subsequent prompt starts a fresh activity interval without resuming cancelled work. The existing AgentKit Git pins advance to 513e92b on feat/acp-v2-session-inject for the retirement API; the audited source-only change introduces no package-version, feature, or transitive dependency changes.

@kit-code-agent kit-code-agent Bot 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.

Cancellation at an AfterToolResult boundary can leave the driver's logical turn active after observable activity is settled, causing the next prompt to execute without Running or terminal Idle notifications.

Comment thread src/protocols/acp/v2.rs Outdated

@kit-code-agent kit-code-agent Bot 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.

The previously reported cancellation issue remains: cancelling at AfterToolResult settles observable activity without retiring the driver's logical turn, allowing the next prompt to stream without Running or terminal Idle notifications.

@kit-code-agent kit-code-agent Bot 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.

Cancellation at AfterToolResult still leaves the driver's logical turn and pending continuation active while session activity settles to Idle. A subsequent prompt resumes that continuation without a new TurnStarted, so it can stream a response without Running or terminal Idle notifications. Retire the interrupted logical turn without executing additional cancelled work before settling activity.

@kit-code-agent kit-code-agent Bot 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.

No issues found. The changes look good to merge.

@danielkov
danielkov merged commit 821256b into main Sep 5, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant