fix(session): increment turn_count in claude-code tool adapter (#1438)#1446
Conversation
The claude-code tool adapter (ACP transport) now emits TurnCompleted events that update turn_count in state.toml. Previously turn_count stayed at 0 for the entire session lifetime despite active execution. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements tracking for agent conversation turns across ACP and CLI transports by introducing a turn_count field. The feedback highlights a logic error where the count increments for every streaming chunk rather than per complete turn, potentially bypassing exit gates. The reviewer suggested modifying the increment logic to detect the start of message sequences and updated the associated tests to reflect correct turn counting.
| SessionEvent::AgentMessage(_) => { | ||
| self.turn_count = self.turn_count.saturating_add(1); | ||
| } |
There was a problem hiding this comment.
The turn_count is incremented for every AgentMessage event. In the ACP transport, AgentMessage events are generated from AgentMessageChunk updates (see line 312). This means turn_count will reflect the number of streaming chunks received rather than the number of conversation turns. This over-counting will cause the "no-op exit gate" in pipeline_post_exec.rs (which checks session.turn_count <= 1) to be bypassed incorrectly even when no real work was performed. Consider only incrementing the count when a new message sequence begins.
| SessionEvent::AgentMessage(_) => { | |
| self.turn_count = self.turn_count.saturating_add(1); | |
| } | |
| SessionEvent::AgentMessage(_) => { | |
| if !self.events.back().map_or(false, |e| matches!(e, SessionEvent::AgentMessage(_))) { | |
| self.turn_count = self.turn_count.saturating_add(1); | |
| } | |
| } |
| SessionEvent::AgentMessage(text) => { | ||
| result.metadata.message_text.push_str(text); | ||
| result.metadata.turn_count = result.metadata.turn_count.saturating_add(1); | ||
| } |
There was a problem hiding this comment.
Similar to the ACP transport, turn_count is incremented for every AgentMessage chunk, leading to an incorrect count when streaming is enabled. You should only increment the count at the start of a message sequence.
SessionEvent::AgentMessage(text) => {
result.metadata.message_text.push_str(text);
if !result.events.last().map_or(false, |e| matches!(e, SessionEvent::AgentMessage(_))) {
result.metadata.turn_count = result.metadata.turn_count.saturating_add(1);
}
}| fn parse_stream_json_counts_each_assistant_message_as_turn() { | ||
| let stream = concat!( | ||
| r#"{"type":"system","session_id":"sess-multi","subtype":"init"}"#, | ||
| "\n", | ||
| r#"{"type":"assistant","session_id":"sess-multi","message":{"content":[{"type":"text","text":"first"}]}}"#, | ||
| "\n", | ||
| r#"{"type":"tool_use","session_id":"sess-multi","tool_use_id":"tu-1","name":"Read"}"#, | ||
| "\n", | ||
| r#"{"type":"tool_result","session_id":"sess-multi","tool_use_id":"tu-1","status":"success"}"#, | ||
| "\n", | ||
| r#"{"type":"assistant","session_id":"sess-multi","message":{"content":[{"type":"text","text":"second"}]}}"#, | ||
| "\n", | ||
| r#"{"type":"assistant","session_id":"sess-multi","message":{"content":[{"type":"text","text":"third"}]}}"#, | ||
| "\n", | ||
| r#"{"type":"result","session_id":"sess-multi","subtype":"final"}"#, | ||
| "\n", | ||
| ); | ||
| let parsed = parse_stream_json(stream); | ||
| assert_eq!( | ||
| parsed.metadata.turn_count, 3, | ||
| "three assistant envelopes => three observed turns" | ||
| ); | ||
| assert_eq!(parsed.metadata.message_text, "firstsecondthird"); | ||
| } |
There was a problem hiding this comment.
This test asserts that each assistant envelope increments the turn count. If these envelopes represent streaming chunks of a single response, the expected turn_count should be 1 rather than 3. The current implementation and test appear to conflate streaming events with conversation turns.
1 participant
Summary
TurnCompletedevents that updateturn_countinstate.tomlturn_countstayed at 0 for entire session lifetime despite active executionTest plan
just pre-commitpasses (4868 unit + 32 e2e tests)csa review --check-verdictPASS (codex gpt-5.5)Closes #1438
🤖 Generated with Claude Code