Skip to content

review/start can disagree with turn/started turn.id during review lifecycle #18516

@yuhuan417

Description

@yuhuan417

What issue are you seeing?

The review lifecycle can report two different turn IDs for what appears to be the same review turn.

In particular, during review/start:

  • review/start returns response.turn.id = A
  • turn/started for the review can arrive with turn.id = B
  • but later lifecycle events such as item/completed for exitedReviewMode and turn/completed are still associated with A

That means clients cannot safely treat turn/started.turn.id as the sole authoritative review turn ID.

This also appears to affect persisted/history views of the review turn. If the review runs a command or tool call, the execution item can end up associated with the child review turn instead of the parent review turn that review/start returned.

The issue matters for both inline review and detached review:

  • inline review: the review/start turn ID and turn/started turn ID can diverge
  • detached review: the review thread is detached correctly, but clients still need a single authoritative turn ID within that detached thread

What steps can reproduce the bug?

Using app-server v2 APIs / notifications:

  1. Start a thread.
  2. Call review/start with delivery: inline.
  3. Record review/start.response.turn.id.
  4. Observe the subsequent turn/started notification for that review.
  5. In some cases, turn/started.turn.id differs from review/start.response.turn.id.
  6. Observe later review lifecycle notifications such as:
    • item/completed containing exitedReviewMode
    • turn/completed
  7. Those later events are still associated with the review/start turn ID, not the turn/started one.

A stronger repro is a review that performs a command/tool call:

  1. Start a thread with persisted extended history enabled.
  2. Call review/start with a custom review prompt that triggers a shell/tool call.
  3. Wait for the review to finish.
  4. Call thread/read(includeTurns=true).
  5. The command/tool execution item may not be attached to the same review turn returned by review/start.

What is the expected behavior?

A review should have one authoritative turn ID for its entire lifecycle.

For both inline and detached review, the following should all agree on the same turn ID:

  • review/start.response.turn.id
  • turn/started.turn.id
  • review item/* notifications such as enteredReviewMode / exitedReviewMode
  • turn/completed
  • persisted/history views such as thread/read(includeTurns=true)

Additional information

I searched for an existing public issue but did not find a clear duplicate.

From tracing the flow locally, the underlying problem seems to be that review has a parent review turn and a child review session, and child events can leak the child session turn ID even though the review lifecycle is completed on the parent turn.

I have a local analysis/fix branch based on current main (yuhuan417:fix/review-turn-id-consistency) if a maintainer wants a concrete patch reference, but I am intentionally opening the issue first because the contribution guide asks for discussion/invitation before opening an external PR.

Metadata

Metadata

Assignees

No one assigned

    Labels

    app-serverIssues involving app server protocol or interfacesbugSomething isn't working

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions