[codex] Add MCP progress notifications#36
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5d5c9734d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| tool_name = op.get("tool", "") | ||
| op_args = op.get("arguments", {}) | ||
| await progress.report( | ||
| i, |
There was a problem hiding this comment.
When batch is called with _meta.progressToken, this emits 0 at batch start and then immediately emits 0 again for the first operation (progress=i), and later repeats values at operation boundaries. MCP progress requires the progress value to increase with each notification, so strict clients may treat these updates as invalid or drop them, which breaks interoperability for progress-aware tooling.
Useful? React with 👍 / 👎.
| ], | ||
| } | ||
|
|
||
| await progress.report(len(queries), len(queries), "Batch search response ready") |
There was a problem hiding this comment.
search_batch sends two consecutive terminal progress notifications with the same value (len(queries)): one for merge completion and one for response readiness. Under MCP, progress notifications must be strictly increasing, so this final duplicate can be considered protocol-invalid by strict clients when a progressToken is provided.
Useful? React with 👍 / 👎.
1 participant
Summary
notifications/progressfor long-running tool calls when clients provide_meta.progressToken.batch,search_batch, and FTS rebuild.search_batchbefore the final merged response.Validation
Research Notes