skill resolution (dotnet): resolve send_message.skill server-side (#338 parity) - #352
Merged
Conversation
…ver-side, Rust #338 parity The C# server carried the generated SendMessageRequest.Skill field but ignored it, exactly as the TS/Python/Go servers still do. It now resolves the skill and composes it into the turn. - Skills: IsValidSkillName / StripFrontmatter / SkillSection / ResolveSectionAsync - ISkillResolver host seam on the FrameDispatcher ctor (analog of Rust AppState::with_skill_resolver); DirSkillResolver over SMOOTH_SKILLS_DIR, with the ASP.NET host preferring a DI-registered resolver and falling back to FromEnv() like Rust's install_skill_resolver_from_env. - Fail-CLOSED: an unresolvable skill is SKILL_NOT_FOUND and the turn does not run — a caller who asked for a recipe and silently got a freeform answer has no way to tell. Blank skill is treated as absent (Rust trims then filters). - The body lands in the SYSTEM PROMPT, appended last, so the persisted user message stays what the user typed and skill prose never accumulates in history to be replayed every later turn. Name validation makes traversal unrepresentable rather than filtered, so "../../etc/passwd" can never reach a Path.Combine. Tests: all five Rust skills.rs unit tests ported under their Rust names, plus dispatcher-level fail-closed / placement / blank / absent coverage. RecordingChatClient promoted out of FileTransferTests into a shared TestChatClients.cs rather than duplicated. 311/311 server tests green, 0 warnings. Source-only — the engine stays the published NuGet, so no publish gate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012iM1Q8JC1H83H2FXQVQNs9
🦋 Changeset detectedLatest commit: 0b74537 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
brentrager
added a commit
that referenced
this pull request
Aug 14, 2026
+ #352 (#354) SmooAI.SmoothOperator.Server is stamped from the lockstep anchor (@smooai/smooth-operator, typescript/package.json) by sync-versions.mjs. #348's changeset named only @smooai/smooth-operator-server — which is the TYPESCRIPT server npm package — so release #349 bumped that to 1.8.0 and left the anchor at 1.39.0. The .NET NuGet was therefore never republished, and 1.39.0 consumers see no TurnContext, no directive sink and no images[]/files[] ingest despite main carrying all of it since 2026-08-11. #352 (skill resolution) inherited the same mistake by copying #348's changeset header. The sibling TS PR #346 named BOTH packages, which is why the TS lane shipped and the .NET lane silently did not. Reported by a downstream consumer who checked the published package rather than main — the report was right about the artifact and wrong about the code. Claude-Session: https://claude.ai/code/session_012iM1Q8JC1H83H2FXQVQNs9 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 14, 2026
Merged
brentrager
added a commit
that referenced
this pull request
Aug 14, 2026
…ree without naming the anchor (#356) Every artifact ships at one shared version held in @smooai/smooth-operator (typescript/package.json). Changesets versions only npm packages, so sync-versions.mjs stamps that number onto every other published manifest. A changeset that does not name the anchor therefore republishes nothing outside npm — silently, with a green release. That is how #348 (.NET file transfer) and #352 (.NET skill resolution) both landed on main and sat unpublished: their changesets named @smooai/smooth-operator-server, which is the TYPESCRIPT server package sitting one word away from the anchor. A downstream consumer found it two days later by reading the NuGet and filed a request to build what we had already shipped. Rule: touching a stamped tree WITH a changeset requires one changeset naming the anchor. Conditioned on having a changeset at all, so docs/test-only PRs stay quiet — a guard that cries wolf gets ignored, and then it protects nothing. sync-versions.mjs is now importable (stamping runs only when invoked directly) so the guard derives the stamped trees from the very list that does the stamping. A new target cannot be added without the guard learning about it. Verified by replay against real history: the guard FIRES on #348's actual commit and PASSES #346's (the TS sibling that correctly named both packages). Claude-Session: https://claude.ai/code/session_012iM1Q8JC1H83H2FXQVQNs9 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
brentrager
added a commit
that referenced
this pull request
Aug 14, 2026
…side, Rust #338 parity (#357) Second language in the fan-out after .NET (#352). The TS server had the field on the wire and ignored it, exactly as its own changelog admitted. - skills.ts: isValidSkillName / stripFrontmatter / skillSection / resolveSection - SkillResolver seam via serve({ skillResolver }); DirSkillResolver over SMOOTH_SKILLS_DIR with explicit-wins-then-env, mirroring Rust's install_skill_resolver_from_env - Fail-CLOSED, and resolved BEFORE the 202 ack so a client never gets "accepted" for a turn that will never run - Body appended LAST to the system prompt; the persisted user message stays exactly what the user typed Name validation makes traversal unrepresentable rather than filtered. Changeset names BOTH the TS package and the lockstep anchor, per #346 — the omission of the anchor is what stranded the .NET work in #348/#352. Tests: five Rust skills.rs tests ported under their Rust names + over-the-socket fail-closed / placement / blank-as-absent. 254 green (245 baseline + 9). Claude-Session: https://claude.ai/code/session_012iM1Q8JC1H83H2FXQVQNs9 Co-authored-by: Claude Opus 5 (1M context) <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.
Problem
send_message.skill(Rust #338) is resolved only in Rust. The TS changelog says it outright: "the TS / Python / Go / .NET servers ignore the field for now." The C# server had the generatedSendMessageRequest.Skillproperty and dropped it on the floor — a client asking for a skill got a freeform answer with no way to tell.This is the first landing of pearl th-ebe27d, the fan-out of the three seams that are still Rust-only. Audit:
skillresolution (#338)C# first, and
skillfirst within C#, because it is server-layer and ships source-only — the two engine seams land inSmooAI.SmoothOperator.Coreand hit the manual NuGet publish gate before the .NET server can consume them.Solution
Skills—IsValidSkillName,StripFrontmatter,SkillSection,ResolveSectionAsync.ISkillResolver— host seam on theFrameDispatcherctor (the analog of RustAppState::with_skill_resolver).DirSkillResolver—<root>/<name>/SKILL.mdover the:-separated roots inSMOOTH_SKILLS_DIR, first root wins. The ASP.NET host prefers a DI-registered resolver, elseFromEnv(), mirroringinstall_skill_resolver_from_env. Unset ⇒ nothing installed, so a multi-tenant deploy never serves host skills by accident.images: unresolvable ⇒error { code: "SKILL_NOT_FOUND" }and the turn does not run.Two security properties worth calling out, both inherited from the Rust reference: name validation makes traversal unrepresentable rather than filtered (
../../etc/passwdcan never reach aPath.Combine), and the resolver is off unless explicitly configured.Verification
All five Rust
skills.rsunit tests ported under their Rust names, per the repo's TDD parity rule — written first, watched fail (32 compile errors), then implemented. Plus dispatcher-level coverage: fail-closedSKILL_NOT_FOUNDwith the model never called, system-prompt-not-user-message placement, blank-skill-as-absent, and absent-skill unchanged.dotnet buildclean across server/aspnetcore/host/integration-tests/host-tests — 0 warnings (including the CS1574 cref I introduced and fixed).Notes
RecordingChatClientwas promoted out ofFileTransferTestsinto a sharedTestChatClients.csrather than copy-pasted — one double, two suites.ponytail:comment: the path separator is hardcoded':'to match Rust, which makes a drive-qualified Windows root unrepresentable. Changing it should change both lanes together or they diverge.skillis byte-for-byte the previous behavior. Source-only, no NuGet publish.🤖 Generated with Claude Code
https://claude.ai/code/session_012iM1Q8JC1H83H2FXQVQNs9