MCP: Icons type as per SEP-973, spec types test improvements, ImplementationSchema use IconsSchema#947
Conversation
…onSchema use IconsSchema
src/spec.types.test.ts
Outdated
| it.each(typesToCheck)('%s should have a compatibility test', (type) => { | ||
| expect(sdkTypeChecks[type as keyof typeof sdkTypeChecks]).toBeDefined(); | ||
| }); |
There was a problem hiding this comment.
This should be indented one level. (why don't we have prettier on this repo??? - rhetorical)
There was a problem hiding this comment.
Have ran prettier manually on the file, will commit that once the .strip topic is closed above
There was a problem hiding this comment.
I don't see a resolve conversation button on the strip topic, so I'll dismiss my request. If you can push your prettier changes, will review again.
There was a problem hiding this comment.
This should be indented one level. (why don't we have prettier on this repo??? - rhetorical)
@cliffhall since you mentioned this... #953 😁
src/spec.types.test.ts
Outdated
| const specTypes = extractExportedTypes(fs.readFileSync(SPEC_TYPES_FILE, 'utf-8')); | ||
| const sdkTypes = extractExportedTypes(fs.readFileSync(SDK_TYPES_FILE, 'utf-8')); | ||
| const testSource = fs.readFileSync(__filename, 'utf-8'); | ||
| const typesToCheck = specTypes.filter(type => !MISSING_SDK_TYPES.includes(type)); |
…onstantinov/typescript-sdk into feature/SEP-973-icons-type-spec-tests
1d475bb to
0845a57
Compare
0845a57 to
c94ba4b
Compare
|
Tests on the main branch are now failing because of the "sizes" property change in the spec. This PR now sorts that too. cc @ihrpr @cliffhall |
cliffhall
left a comment
There was a problem hiding this comment.
This lines up with the spec change that was just merged.
|
Missed this PR while reviewing this quick CI fix: #981 Introduces a small conflict as that PR already replaces |
hehe! Was just merging it. Merge pushed. |
ihrpr
left a comment
There was a problem hiding this comment.
thanks so much for working on this. left some comments on comments but otherwise looks good!
src/types.ts
Outdated
| /** | ||
| * An optional list of icons for this implementation. | ||
| * This can be used by clients to display the implementation in a user interface. | ||
| * Each icon should have a `kind` property that specifies whether it is a data representation or a URL source, a `src` property that points to the icon file or data representation, and may also include a `mimeType` and `sizes` property. |
There was a problem hiding this comment.
Where is the kind coming from? I think it's fine not explain too much what IconSchema should look like as we have a definition right above
There was a problem hiding this comment.
Updated the comment with the latest from the spec.
src/types.ts
Outdated
| * This can be used by clients to display the implementation in a user interface. | ||
| * Each icon should have a `kind` property that specifies whether it is a data representation or a URL source, a `src` property that points to the icon file or data representation, and may also include a `mimeType` and `sizes` property. | ||
| * The `mimeType` property should be a valid MIME type for the icon file, such as "image/png" or "image/svg+xml". | ||
| * The `sizes` property should be a string that specifies one or more sizes at which the icon file can be used, such as "48x48" or "any" for scalable formats like SVG. |
There was a problem hiding this comment.
Updated with the same comments from the main spec
|
@KKonstantinov if it's not too much to ask, please can we also add this: modelcontextprotocol/modelcontextprotocol#1565 🙏 |
Done! |
…ntationSchema use IconsSchema (modelcontextprotocol#947)
This PR adds
Iconstype support for modelcontextprotocol/modelcontextprotocol#973 (PR), which exposed additional metadata for Implementations, Resources, Tools and Prompts.Motivation and Context
This PR adds the missing
Iconstype and respectiveIconsSchemazod schema.Further, a few improvements have been made to
spec.types.test.ts:How Has This Been Tested?
Tests and type checks are passing, there are no changes to actual implementation.
Breaking Changes
No breaking changes - just adding the Icons type and test improvements.
Types of changes
Checklist
Additional context