types: rename ResourceReponsePart to ResourceResponsePart - #375
Merged
Conversation
…tle) Three small, independent corrections to the protocol surface. Each is source- or wire-breaking and so has to land before the 1.0 freeze, but none involves a design choice -- they align the declared types with what the docs already promise and what both reference implementations already do. 1. Rename `ResourceReponsePart` -> `ResourceResponsePart` The interface name misspelled "Response". The Rust and Go generators already carried per-language rename entries to correct it on output, while the TypeScript, Kotlin and Swift clients shipped the typo verbatim -- so a single protocol type had two different names across the five clients. Fix the canonical spelling and delete the two now redundant generator renames. 2. Narrow `SessionToolClientExecutionRequest.toolCall` Declared as the full `ToolCallState` union while its own doc comment said the host "only ever populates this with a ToolCallRunningState". Its two sibling variants are already narrowed (`SessionToolConfirmationRequest` -> `ToolCallConfirmationState`, `SessionToolAuthenticationRequest` -> `ToolCallAuthRequiredState`), so this was the odd one out. Narrow it to `ToolCallRunningState` and state the invariant in the type rather than in prose. 3. Make `ChatState.title` / `ChatSummary.title` optional Both were required `string`, but a session's default chat generally has no identity separate from its session. VS Code was papering over this by writing an empty string as an internal sentinel meaning "show the session title instead" -- a convention the protocol never defined. Make the field optional, specify that absence means "inherit the session's title", and explicitly forbid the empty-string sentinel so a deliberately blank title stays distinguishable from an absent one. No behavioural or reducer changes; `npm run generate` output is limited to these three shapes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Follow-up to the optional-title change, plus a revert. Add `CreateChatParams.title`. Previously `createChat` had no way to pass a title, so *every* chat necessarily had a titleless window between creation and the host assigning one -- not just a session's default chat. A client that already knows the name (a fork, a side chat, a tool-spawned worker) can now supply it up front instead of creating the chat untitled and immediately following up with `session/chatUpdated`. Chats whose title is derived from the conversation still start untitled and inherit the session's title until one is assigned. Revert the `SessionToolClientExecutionRequest.toolCall` narrowing from the previous commit. It looked like a free precision win, but `ToolCallRunningState` is a *variant* of the `ToolCallState` tagged union and the per-language generators emit variant structs with `omitDiscriminants`, because the `status` tag is written by the enum wrapper. Referencing the variant directly therefore dropped `"status": "running"` from the wire in Rust/Kotlin/Swift/Go while TypeScript -- whose variant interface still declares `status` -- would have kept emitting it. That is both a wire change and a cross-language inconsistency; the round-trip fixtures caught it. The field keeps its documented invariant in prose, with a note explaining why it is not narrowed. Doing this properly needs generator support for retaining discriminants on directly-referenced variants. Also fix the Go reducers example, which set `ChatState.Title` as a bare string. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
roblourens
changed the title
connor4312
previously approved these changes
Jul 31, 2026
connor4312
marked this pull request as ready for review
July 31, 2026 19:39
connor4312
reviewed
Jul 31, 2026
Comment on lines
+42
to
+51
| /** | ||
| * Chat title. | ||
| * | ||
| * Absent means the chat has no title of its own and consumers SHOULD fall | ||
| * back to the owning {@link SessionState.title | session's title}. This is | ||
| * the normal case for a session's default chat, which typically has no | ||
| * identity separate from the session itself. Producers MUST NOT use an empty | ||
| * string to mean "inherit" — omit the field instead. | ||
| */ | ||
| title?: string; |
Member
There was a problem hiding this comment.
Do we actually want this to be optional? A client will want to display chat tabs or navigation, and a title is pretty much needed for that, even if it's just a placeholder like "Subagent Session" issued by the agent host
Member
Author
There was a problem hiding this comment.
As I understand it, the special case here is the default chat. It wants to use the session title, today we set title: '' and fall back to the session title by convention. We can do that, or duplicate the title onto the default chat, or make title optional but implicitly required for other chats, or remove the title from the session, idk.
Member
Author
There was a problem hiding this comment.
I'd left this as a draft because I wasn't sure what I wanted
Member
Author
There was a problem hiding this comment.
We can definitely just leave it
Member
There was a problem hiding this comment.
I'd probably leave it for now 🤷
connor4312
marked this pull request as draft
July 31, 2026 19:40
roblourens
changed the title
Two hand-written references the generators don't reach: the AHPApp sample app (which imports AgentHostProtocol and so would no longer compile against the renamed type) and the state-model guide. Neither is covered by CI, which is why the swift job stayed green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
connor4312
approved these changes
Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rename
ResourceReponsePart→ResourceResponsePartThe interface name misspelled "Response". The Rust and Go generators already carried per-language rename entries to correct it on output:
…while TypeScript, Kotlin and Swift emitted the typo verbatim. So a single protocol type had two different names across the five clients, and consumers of the TS/Kotlin/Swift clients had to spell it wrong to name the type.
This corrects the canonical spelling in
types/channels-chat/state.tsand deletes both now-redundant generator renames, so the name is derived consistently everywhere instead of being patched per-language. It also updates two hand-written references the generators don't reach — theAHPAppSwift sample app (which importsAgentHostProtocol, so it would otherwise no longer compile) and the state-model guide. Neither is covered by CI.No wire change — the discriminant value stays
contentRefand the type is structurally identical. Source-breaking only for TypeScript, Kotlin and Swift consumers that reference the type by name; Rust and Go consumers already saw the corrected spelling.Worth doing before the 1.0 freeze, since renaming a published type afterwards is a major-version change.
Verification
npm run generate— no drift; zero remaining occurrences of the typo acrosstypes/, all five clients, the JSON Schemas and the docsnpm test— 378 pass, reducer branch coverage at 100%cargo test --workspaceandgo build ./... && go test ./...— both green(Written by Copilot)