Adjusting for less duplication in Skills and MCP

[Ellerbach]

Description

Adjusting for less duplication in Skills and MCP

Motivation and Context

• Adjusting for less duplication in Skills and MCP
• Creating also direct improvements in MCP

How Has This Been Tested?

Screenshots

Types of changes

• Improvement (non-breaking change that improves a feature, code or algorithm)
• Bug fix (non-breaking change which fixes an issue with code or algorithm)
• New feature (non-breaking change which adds functionality to code)
• Breaking change (fix or feature that would cause existing functionality to change)
• Config and build (change in the configuration and build system, has no impact on code or features)
• Dependencies (update dependencies and changes associated, has no impact on code or features)
• Unit Tests (add new Unit Test(s) or improved existing one(s), has no impact on code or features)
• Documentation (changes or updates in the documentation, has no impact on code or features)

Checklist:

• My code follows the code style of this project (only if there are changes in source code).
• My changes require an update to the documentation (there are changes that require the docs website to be updated).
• I have updated the documentation accordingly (the changes require an update on the docs in this repo).
• I have read the CONTRIBUTING document.
• I have tested everything locally and all new and existing tests passed (only if there are changes in source code).
• I have added new tests to cover my changes.

[coderabbitai]

Warning

Review limit reached

@Ellerbach, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 30 minutes and 21 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: nanoframework/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0410af91-dc01-467d-b863-80a1d24526ca

📥 Commits

Reviewing files that changed from the base of the PR and between e08a6c6 and c97727d.

📒 Files selected for processing (9)

• tests/McpEndToEndTest/McpEndToEndTest.nfproj
• tests/McpEndToEndTest/packages.config
• tests/McpEndToEndTest/packages.lock.json
• tests/SkillsEndToEndTest/SkillsEndToEndTest.nfproj
• tests/SkillsEndToEndTest/packages.config
• tests/SkillsEndToEndTest/packages.lock.json
• tests/WebServerE2ETests/WebServerE2ETests.nfproj
• tests/WebServerE2ETests/packages.config
• tests/WebServerE2ETests/packages.lock.json

📝 Walkthrough

Walkthrough

This PR consolidates shared utility types across two namespaced variants of the WebServer library. DescriptionAttribute, JsonHelper, and RegistryBase are extracted into single conditional-compilation sources that branch on WEBSERVER_MCP flag. Deserialization validation is tightened to reject malformed inputs rather than silently defaulting. Both MCP and Skills projects switch to link these unified sources instead of maintaining separate copies.

Changes

Shared utility consolidation with dual-namespace support

Layer / File(s)	Summary
Shared type namespace conditionals Shared/DescriptionAttribute.cs, Shared/JsonHelper.cs, Shared/RegistryBase.cs	DescriptionAttribute, JsonHelper, and RegistryBase gain #if WEBSERVER_MCP conditional compilation to select between nanoFramework.WebServer.Mcp and nanoFramework.WebServer.Skills namespaces. JsonHelper documentation generalized to neutral "action" wording.
Deserialization validation and exception handling in shared RegistryBase Shared/RegistryBase.cs	ConvertToPrimitiveType now strictly validates boolean inputs, accepting case-insensitive "true"/"false" or 1-char byte conversion and throwing InvalidCastException on unrecognized values. DeserializeFromHashtable stops silently skipping failed property assignments and instead propagates exceptions, including explicit InvalidCastException for unsupported nested types. CreateInstance throws parameterless Exception on failure. All three methods update exception contracts in XML documentation.
MCP project consolidation to shared sources nanoFramework.WebServer.Mcp/nanoFramework.WebServer.Mcp.nfproj	Defines WEBSERVER_MCP compilation constant, links shared DescriptionAttribute.cs and RegistryBase.cs from ..\Shared\, adds linked JsonHelper.cs, and removes local McpToolsJsonHelper.cs and redundant local RegistryBase.cs from compile list.
Skills project exception handling and shared source linking nanoFramework.WebServer.Skills/RegistryBase.cs, nanoFramework.WebServer.Skills/nanoFramework.WebServer.Skills.nfproj	RegistryBase wraps property deserialization loop in try/catch that rethrows exceptions unchanged. Project links shared DescriptionAttribute.cs, RegistryBase.cs, and JsonHelper.cs from ..\Shared\ instead of local copies.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• nanoframework/nanoFramework.WebServer#350: Earlier PR that introduced skills-discovery and shared types, directly impacted by this PR's unification and conditional-namespace refactoring of DescriptionAttribute, JsonHelper, and RegistryBase.

Suggested labels

Type: Unit Tests, Type: documentation, Area: Config-and-Build

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Adjusting for less duplication in Skills and MCP' is concise (48 characters), descriptive, clearly summarizes the main change of reducing code duplication, and does not end with a full stop.
Description check	✅ Passed	The pull request description relates to the changeset by explaining the motivation for reducing duplication in Skills and MCP, which aligns with the shared component restructuring and conditional compilation changes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@nanoFramework.WebServer.Skills/RegistryBase.cs`:
- Around line 175-182: The nested-Hashtable deserialization branch in
RegistryBase.cs is malformed; fix the DeserializeFromHashtable call and
surrounding control flow by replacing the corrupted fragment so it reads: if
(value is Hashtable nestedHashtable) { object nestedObject =
DeserializeFromHashtable(nestedHashtable, propertyType); method.Invoke(instance,
new object[] { nestedObject }); } else { throw new InvalidCastException(); } —
ensure you use the correct parameter name/propertyType used elsewhere in
RegistryBase (and keep method.Invoke(...) as shown) or, alternatively, remove
this duplicate Skills-local RegistryBase.cs file if the shared
..\Shared\RegistryBase.cs is the canonical source to avoid future build/tooling
issues.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: nanoframework/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b8b42dea-c4a4-42b5-ac56-c66870b75eef

📥 Commits

Reviewing files that changed from the base of the PR and between 0f9ab3c and e08a6c6.

📒 Files selected for processing (8)

• Shared/DescriptionAttribute.cs
• Shared/JsonHelper.cs
• Shared/RegistryBase.cs
• nanoFramework.WebServer.Mcp/DescriptionAttribute.cs
• nanoFramework.WebServer.Mcp/McpToolsJsonHelper.cs
• nanoFramework.WebServer.Mcp/nanoFramework.WebServer.Mcp.nfproj
• nanoFramework.WebServer.Skills/RegistryBase.cs
• nanoFramework.WebServer.Skills/nanoFramework.WebServer.Skills.nfproj

💤 Files with no reviewable changes (2)

• nanoFramework.WebServer.Mcp/DescriptionAttribute.cs
• nanoFramework.WebServer.Mcp/McpToolsJsonHelper.cs

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud