[codex] Fix stale app-server protocol paths#24606
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: 6cc8cdb2b7
ℹ️ 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".
| `getConversationSummary` remains as a compatibility helper for clients that still need a summary lookup by `conversationId` or `rolloutPath`. Lookups by `conversationId` are preferred; lookups by `rolloutPath` won't work with non-local thread stores. | ||
|
|
||
| For complete request and response shapes, see the app-server README and the protocol definitions in `app-server-protocol/src/protocol/v2.rs`. | ||
| For complete request and response shapes, see the app-server README and the protocol definitions in `app-server-protocol/src/protocol/v2/`. |
There was a problem hiding this comment.
This file still points readers to app-server-protocol/src/protocol/{common,v1,v2}.rs in the overview/authority text at lines 11, 33, and 144; since app-server-protocol/src/protocol/v2.rs no longer exists (v2 is now a directory), those brace-expanded references still send MCP integrators to the dead path even though this paragraph was corrected. I checked with rg '\{common,v1,v2\}\.rs|protocol/v2\.rs|v2\.rs' AGENTS.md codex-rs/app-server codex-rs/docs .codex, which only reports these remaining occurrences.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, I agree. The brace-expanded references still implied the removed v2.rs file. I pushed follow-ups to split those paths and, since app-server v1 is no longer supported, removed the v1 framing from the affected docs as well. The docs now point at common.rs and v2/, and the stale-reference scan is clean.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 116ddfbb7a
ℹ️ 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".
| - `applyPatchApproval`, `execCommandApproval` | ||
|
|
||
| See code for full type definitions and exact shapes: `app-server-protocol/src/protocol/{common,v1,v2}.rs`. | ||
| See code for full type definitions and exact shapes: `app-server-protocol/src/protocol/common.rs` and `app-server-protocol/src/protocol/v2/`. |
There was a problem hiding this comment.
This now claims the linked files contain the full request/response shapes, but the deprecated methods listed just above still get their params/responses from app-server-protocol/src/protocol/v1.rs (common.rs wires getConversationSummary, getAuthStatus, and gitDiffToRemote to v1::*). For clients maintaining those compatibility RPCs, the doc now points them away from the only file that defines several of the shapes, so the stale v2.rs path should be fixed without dropping the valid v1.rs reference.
Useful? React with 👍 / 👎.
| ### Core Rules | ||
|
|
||
| - All active API development should happen in app-server v2. Do not add new API surface area to v1. | ||
| - All active API development should happen in app-server v2. |
There was a problem hiding this comment.
For future app-server protocol edits, this removes the explicit instruction not to add API surface to v1 even though app-server-protocol/src/protocol/v1.rs still exists and is still wired for compatibility requests. The remaining wording says active development happens in v2, but it no longer tells agents and contributors that v1 must stay frozen, which weakens the guardrail that prevents new compatibility-only methods from being added there.
Useful? React with 👍 / 👎.
1 participant
Fixes #24604.
Why
The app-server v2 protocol was split into the
app-server-protocol/src/protocol/v2/module directory, but several docs still pointed contributors at paths that implied the removedapp-server-protocol/src/protocol/v2.rsfile. Those stale paths made the app-server API guidance harder to follow.What changed
AGENTS.mdto referenceapp-server-protocol/src/protocol/v2/and keep active API guidance focused on v2.common.rsand thev2/directory are listed accurately.Verification
rg -n "app-server v1|v1 compatibility|protocol/v1|\\{common,v1,v2\\}\\.rs|protocol/v2\\.rs|v2\\.rs" AGENTS.md codex-rs/app-server codex-rs/docs .codexand confirmed no stale references remain.