fix(app-server-protocol): align v2 skills response nullability#166
Open
andrei-hasna wants to merge 1 commit into
Open
fix(app-server-protocol): align v2 skills response nullability#166andrei-hasna wants to merge 1 commit into
andrei-hasna wants to merge 1 commit into
Conversation
SkillMetadata and SkillToolDependency are nested v2 response payloads returned through SkillsListEntry.skills, but their optional fields used `#[serde(default, skip_serializing_if = "Option::is_none")]` and `#[ts(optional)]`, so absent values were omitted from the wire and generated TypeScript exposed them as optional (`shortDescription?: string`). That diverges from the v2 explicit-null response contract. Drop the response-side skip/default/ts(optional) attributes so these fields serialize as present `null` and render as `T | null` in TypeScript, matching the established `ThreadGoalUpdatedNotification.turn_id` pattern. Regenerate the TS schema fixtures (JSON schema already emitted nullable types) and add focused serialization coverage for the no-metadata and no-dependency-details cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
Summary
Align the v2 skills-list response metadata types with the v2 explicit-null response contract.
SkillMetadataandSkillToolDependencyare nested response payloads returned throughSkillsListEntry.skills, but several optional fields used#[serde(default, skip_serializing_if = "Option::is_none")]and#[ts(optional)]. That omitted absent values from the wire and generated TypeScript that exposed them as optional (shortDescription?: string,description?: string), diverging from the v2 contract that response-side nullable fields serialize as presentnull.This matches the already-established
ThreadGoalUpdatedNotification.turn_id: Option<String>pattern.Changes
plugin.rs: remove#[serde(default, skip_serializing_if = "Option::is_none")]+#[ts(optional)]fromSkillMetadata.{short_description, interface, dependencies}andSkillToolDependency.{description, transport, command, url}, leaving plainOption<T>.SkillMetadata.ts,SkillToolDependency.ts): fields flip from?: TtoT | null. JSON schema already emitted nullable types (matches the reference pattern), soSkillsListResponse.jsonis unchanged.nulloutput.Scoped strictly to skills-list response metadata; no request-param or MCP schema changes.
Validation
just write-app-server-schema(fixtures regenerated, in sync)just test-fast -p codex-app-server-protocol— 258 passed, incl. 2 new tests andgenerated_ts_optional_nullable_fields_only_in_paramsjust fmt,just fix -p codex-app-server-protocol— clean🤖 Generated with Claude Code