[sideeffffect]

Stacked on #273 (T6) → #271 (T3). Base is 2026-07-28-protocol-support, so the diff also shows T3+T6 until they merge — review the T7 delta: sideeffffect/chimp@feat/t6-subscriptions...feat/t7-shared-types

T7 — #245: update types present in both schema versions

• new Icon and Annotations types
• icons on Implementation, ToolDefinition, Resource, ResourceTemplate, Prompt, ResourceLink
• annotations and _meta on content blocks (Text/Image/Audio/ResourceContent) and on Resource/ResourceTemplate
• Implementation.websiteUrl/description, PromptReference.title, ResourceLink.title/size
• server DSL builders (icons/annotations) on the resource, resource-template and prompt builders
• client doc snippets updated for the ToolContent.Text field additions (mdoc)
• schema conformance tests for the new fields

All additive (optional, default None) → 2025-11-25 wire unchanged. Verified: core + server unit suites green; all four conformance combos green (client/server × 2025-11-25 / 2026-07-28).

Note: tool-builder .icons(...) deferred (threads through the multi-step Tool[I,O] chain); ToolDefinition still carries icons at the protocol level.

Closes #245.

Stack (bottom → top, all on 2026-07-28-protocol-support)

#271 (T3) → #273 (T6) → #275 (T7, this PR) → #276 (T5) → #274 (T8)

🤖 Generated with Claude Code

[max-peroch]

Is plumbing .icons(...) on McpServer into initialize serverInfo already planned here or as a follow-up? Happy to send a small patch on this branch if not.

[sideeffffect]

Follow-up, not this PR. #275 only adds the shared metadata types — Implementation now carries title/icons/websiteUrl/description, but McpServer has no .icons(...) builder yet and McpHandler still builds serverInfo as Implementation(server.name, server.version) (McpHandler.scala:156). Wiring server-level icons into the initialize serverInfo needs a builder on McpServer plus threading it through serverImplementation. A small patch on this branch would be welcome — thanks for offering.

[sideeffffect]

@kubinio123 🙏

[kubinio123]

Automated review

Scope: the T7 delta only (c950ca4..pr-275), 15 files, +235/-59.

1. The tests contradict the PR's own title

All 9 new conformance cases went into Schema20260728ConformanceSpec. Schema20251125ConformanceSpec got nothing.

I diffed the two bundled schemas. Icon, Annotations, Implementation, Prompt, PromptReference, Resource, ResourceTemplate, ResourceLink and TextContent are identical across both revisions. Every field this PR adds already exists in 2025-11-25. So a regression that breaks the older revision is invisible.

AGENTS.md rule 2 covers exactly this. SchemaConformance is already a trait with an abstract schemaResourcePath and holds no cases. Move the 9 cases into it, or into a shared mixin, and both specs run them for free.

2. Icon.theme is a closed enum typed as String

core/src/main/scala/chimp/protocol/Metadata.scala:12

Both schemas declare "enum": ["dark", "light"]. The field is Option[String], so theme = Some("blue") compiles and puts invalid JSON on the wire.

Every other closed string set in chimp.protocol is a Scala enum with a hand-written codec: Role, LoggingLevel, ElicitAction, CacheScope, ResultType. This is also the one change here that is costly to undo, because widening String to an enum later breaks source compatibility on a public type.

Annotations.priority has the same shape of gap: the schema bounds it to 0..1, the type is a plain Double.

3. Three README snippets no longer compile

README.md:57, :99, :163 still use case ToolContent.Text(_, text). The arity is now 4.

mdocIn := file("docs") (build.sbt:346), so the README is not compile-checked. The PR fixed docs/ and examples/ and missed the most-read page.

Related: the fix style is split. The docs use case t: ToolContent.Text => t.text, which survives the next field addition. examples/ and the server tests use case ToolContent.Text(_, text, _, _), which does not. Pick the docs form everywhere.

4. ToolContent.ResourceLink breaks positional callers silently

core/src/main/scala/chimp/protocol/Tools.scala:57

title: Option[String] went in at position 4, between name and description. An existing call ResourceLink("resource_link", uri, name, desc, mime) still compiles, because all three are Option[String], and now writes desc into title and mime into description.

Every other insertion in this PR lands on a type-incompatible slot and fails to compile. This is the one exception. There are no positional callers inside the repo, so the risk is external only. Pre-1.0 breakage is fine; the silence is the problem.

While here: ResourceLink.name is Option[String], but both schemas list name as required. Making it a plain String fixes the required-field gap and removes the ambiguity above in one step.

Also worth fixing

• Server cannot set its own identity metadata. McpHandler.scala:154 hardcodes Implementation(server.name, server.version), so the new icons, websiteUrl and description are unreachable from the server side. Unlike the deferred tool .icons, this is not blocked by the Tool[I,O] chain.
• Codec duplication. The hand-written ToolContent codec grew from 59 to 100 lines, and each field is now named three times per case. A derived Codec.AsObject per enum case, with only the type dispatch kept by hand, cuts it to about 20 lines.
• Untested new fields: ResourceTemplate.icons/annotations, and annotations plus _meta on Image, Audio and ResourceContent. The _meta branch of all five codec cases is never exercised. The five new DSL builders have zero call sites anywhere.
• Scaladoc says "(2026-07-28)" on Icon and Annotations, which are in 2025-11-25 too. This tells readers the opposite of the PR's premise.
• docs/server/resources.md:4 lists the metadata builders by name and is now incomplete. AGENTS.md rule 4 applies, because these are new protocol features.
• // T7: comment at Schema20260728ConformanceSpec.scala:72 breaks AGENTS.md rule 6, and names an internal ticket in a public repo.
• SamplingMessage has _meta in both schemas and did not get it.
• Performance: nothing in this delta. The extra dropNullValues and downField calls are noise. One pre-existing issue did surface on the same path: McpHandler.scala:63 interpolates the full request and response trees into a string on every message, before slf4j checks the log level. Separate ticket.

[kubinio123]

Points 1, 2, 3 need addressing @sideeffffect

[sideeffffect]

Addressed points 1–3 from the automated review (head 9d87917):

1. Conformance now runs under both revisions — 9143cb5. Moved the nine shared-metadata cases (Icon, Annotations, Implementation+icons, Tool+icons, Resource, Prompt, PromptReference, ResourceLink, TextContent+annotations) out of Schema20260728ConformanceSpec into the shared SchemaConformance trait, so Schema20251125ConformanceSpec validates them too. Confirmed both bundled schemas carry every added field with no additionalProperties: false, so a 2025-11-25 regression is no longer invisible.

2. Closed domains are now typed — bf50fac. Icon.theme is an IconTheme enum (Dark/Light) with a hand-written codec, matching Role/LoggingLevel/CacheScope. Annotations.priority is an opaque Priority bounded to 0.0..1.0, rejected out of range on both construction and decode. Done pre-1.0 to avoid a later source-breaking String→enum / Double→refined widening.

3. README snippets compile — 9d87917. The three case ToolContent.Text(_, text) matches (README.md:57/99/163) now use case t: ToolContent.Text => t.text, the docs form that survives further field additions.

sbt scalafmtCheckAll "core/test" green locally (87 tests, 0 failed). Points 4 and the "Also worth fixing" items are tracked as follow-ups. Re-requesting review 🙏

[sideeffffect]

@kubinio123 all three points are addressed above — ready for another look when you have a moment. (A formal re-review request via the reviewers panel isn't available to me as a fork contributor, hence the ping.)