Skip to content

fix: make review_reply wait for the next submit - #66

Merged
handlename merged 2 commits into
mainfrom
fix/certain-submit
Aug 29, 2026
Merged

fix: make review_reply wait for the next submit#66
handlename merged 2 commits into
mainfrom
fix/certain-submit

Conversation

@handlename

Copy link
Copy Markdown
Owner

Why this change is necessary

A review would sometimes stop dead the moment it should have continued. The human clicked Submit, the page went quiet, and nothing happened until they typed "I submitted" into the chat by hand — at which point the agent picked the round up instantly, as if it had been there all along.

Session transcripts say exactly where it went. The agent replied, called review_progress(idle), summarised the round to its user and ended its turn — and from that moment nothing was subscribed to the next submit. MCP is pull-only: a submit nobody is waiting for reaches no one.

Everything else was doing its job. The wait that followed a manual "I submitted" returned the round in two seconds, so the sidecar, the sequence counters and the notifier had never lost anything; and a wait that ran past two minutes came back to the agent as a task notification and the round continued, so the long stdio call — the part this repository had flagged as "documented but unverified" — is not the fragile piece either. Retiring the MCP server, which was on the table when this started, would have changed nothing.

Big picture

big picture

図のソース / Diagram source (mermaid)
sequenceDiagram
    autonumber
    participant H as Human (page)
    participant S as Server (session)
    participant A as Agent

    A->>S: review_start
    S-->>H: serve the review page
    A->>S: review_wait
    Note over S,A: the call blocks until a submit arrives
    H->>S: Submit (comments)
    S-->>A: review_wait returns the comments
    A->>A: edit the document
    A->>S: review_reply (inline replies)

    alt before: reply returns at once
        S-->>A: returns immediately
        A-->>H: summarise the round, then end the turn
        H->>S: Submit again
        Note over S,A: no waiter: the submit reaches no one
        H->>A: the human says "I submitted" by hand
    else after (this PR): reply also waits
        S->>S: record the replies, keep waiting
        H->>S: Submit again
        S-->>A: the same review_reply call returns the next round
        Note over S,A: the loop continues on its own
    end
Loading

Approach

review_reply now writes the replies and then waits for the next submit, returning what review_wait returns. Closing a round and entering the next wait are the same call, so the gap cannot open — the fix is structural rather than a matter of instructing the agent more firmly, which is what a pull-only protocol needs. review_wait stays for the first wait after review_start and for resuming after a timeout.

Alternatives considered and rejected: replacing MCP with another transport (the evidence clears MCP); a Stop hook that refuses to end the turn while a review is open (client-specific, and unnecessary once the tool surface closes the gap); strengthening the skill wording alone (leaves compliance to the model, which is the thing that failed).

One assumption is baked in: review_reply always waits, with no escape hatch, because mid-round reporting is what review_progress is for. If a case for replying without waiting shows up, a flag can be added later.

Design Documents

Review Points

  • Is "a reply always waits" the right invariant, or should the escape hatch exist from the start?
  • The waiting call must not hold the feedback sidecar lock, or the human's Submit would hang against the very call it is meant to release. A test pins this.
  • The embedded skill now calls review_progress(idle) before review_reply, since the reply no longer returns until the next round.
  • Verified beyond the unit tests: a live review over the real MCP surface, three rounds of human Submit and agent reply, with zero manual prompting, ending on session_ended from End Review.

handlename and others added 2 commits August 29, 2026 21:58
review_reply returned as soon as it had written, which left the agent
free to summarise the round to its user and end its turn with nothing
subscribed to the next submit. MCP is pull-only, so a submit nobody is
waiting for reached no one: the review stalled until the human said "I
submitted" by hand.

Nothing else was at fault, and the session transcripts say so. The wait
that followed such a prompt returned the round in two seconds, so the
sidecar, the sequence counters and the notifier had all done their job;
and a wait that ran past two minutes came back to the agent as a task
notification and the round continued, so the long stdio call is not the
fragile part of this loop either.

Closing the round and entering the next wait are now the same call, so
the gap cannot open. review_wait remains for the first wait after
review_start and for resuming after a timeout, and the tool description
says not to end the turn on any other outcome.

The embedded skill text moves review_progress(idle) ahead of the reply,
since the reply no longer returns until the next round.

User request: レビュー画面から submit してもエージェントが反応しないこと
がある。原因を突き止め、submit が確実に伝わるようにしたい。原因が MCP に
あるなら MCP サーバー機能の廃止も選択肢に入れる、という依頼。調査の結果
MCP は原因ではなく、返信後にループへ戻り損ねる一点だったため、ツール設計
で構造的に塞ぐ方針を選んだ。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
README and DESIGN carried the old loop -- reply, then back to
review_wait -- which is exactly the shape that let a round close with
nobody waiting. They now describe review_reply as the call that replies
and waits, and DESIGN keeps the reasoning: the signal was never lost,
there was simply often no next wait to deliver it to.

GLOSSARY's Feedback entry named review_wait as the only agent call a
submit releases; it now names both.

User request: 変更に合わせてドキュメントを追随させる。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@handlename
handlename merged commit 655759f into main Aug 29, 2026
2 checks passed
@handlename
handlename deleted the fix/certain-submit branch August 29, 2026 13:19
@handlename handlename mentioned this pull request Aug 29, 2026
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