Repository navigation
th-b30a6a: send_message grows an optional skill — the engine resolves it, not the client - #338
Merged
Merged
Conversation
…es it, not the client
Every client used to resolve a skill itself and prepend the markdown body to
the message text. That put prose on the wire and persisted the skill body into
conversation history, where it was replayed as context on every later turn.
The wire now carries intent (`skill: "code-review"`); the server resolves the
name and composes the body into THAT turn's system prompt, so the persisted
user message stays exactly what the user typed.
- `skills::SkillResolver` — the host seam (`AppState::with_skill_resolver` /
`LocalServerBuilder::skill_resolver`).
- `skills::DirSkillResolver` — the default: `<root>/<name>/SKILL.md` over the
`:`-separated roots in `SMOOTH_SKILLS_DIR`, first match wins, frontmatter
stripped. Unset ⇒ no resolver installed, so a multi-tenant deploy never
serves host skills by accident.
- Fail-closed, unlike `images`: an unresolvable skill is `SKILL_NOT_FOUND` and
the turn does not run — answering unskilled is indistinguishable to the
caller from answering skilled. Names are `[A-Za-z0-9_-]{1,128}`, which makes
path traversal unrepresentable rather than filtered.
Absent `skill` is byte-for-byte the previous behavior. Rust is the reference
implementation; the TS/Python/Go/.NET servers ignore the field for now (the
same staging `images` is in).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YSE8xAs9aWN5VhrnnSyLKq
🦋 Changeset detectedLatest commit: b0970d4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
This was referenced Aug 11, 2026
brentrager
added a commit
that referenced
this pull request
Aug 14, 2026
…ver-side, Rust #338 parity (#352) 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. Claude-Session: https://claude.ai/code/session_012iM1Q8JC1H83H2FXQVQNs9 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merged
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>
Merged
brentrager
added a commit
that referenced
this pull request
Aug 30, 2026
) * th-ebe27d: resolve send_message.skill in the Python and Go servers Rust #338 moved skills from prose-on-the-wire to intent-on-the-wire: the client sends `skill: "code-review"`, the SERVER resolves the name to its markdown body and composes it into the turn's system prompt, so the persisted user message stays exactly what the user typed instead of carrying a skill body that gets replayed as context on every later turn. Rust, C# and TypeScript carried it. Python and Go ignored the field outright — worse than not supporting it, because a client that asked for a skill got a confident UNSKILLED answer with no signal anything was dropped. Both now mirror rust/smooth-operator-server/src/skills.rs: a SkillResolver host seam (ServerState.skill_resolver / WithSkillResolver) plus a DirSkillResolver default over <root>/<name>/SKILL.md across the ':'-separated SMOOTH_SKILLS_DIR roots, first root wins. Unset => no resolver installed => any skill field is a clean SKILL_NOT_FOUND, so a multi-tenant deploy never serves host skills by accident. Two properties are load-bearing, and tested as such: - Fail closed, BEFORE the ack. An unresolvable skill emits SKILL_NOT_FOUND instead of the 202 and never starts a turn. (Go needed the resolution lifted above assembleSystemPrompt, not merely above the ack, since the prompt is assembled first there.) A blank/whitespace skill is "no skill", not an unknown one, so a client that always sends the field still works. - Traversal is unrepresentable, not filtered. [A-Za-z0-9_-]{1,128} — the pattern spec/actions/send-message.schema.json already declared — is enforced before the name is ever joined onto a filesystem root. New conformance scenario skill-unknown-error pins the fail-closed contract across all five servers. It needs no filesystem setup (the default installs no resolver), so the corpus is the oracle for this seam rather than five per-language opinions about it. Go's env install checks the concrete *DirSkillResolver for nil before assigning into the interface: a typed nil would make ResolveSkillSection's `resolver == nil` guard miss and panic instead of reporting SKILL_NOT_FOUND. Pinned by test. Verified: 13 new Python tests + 404 total green; 12 new Go tests + full suite green; parity oracle 19/19 in Python, Go and TypeScript. Both new suites mutation-checked — disabling the prompt append fails them. No wire-protocol change; the schema already carried the field. * th-ebe27d: fix ruff import order in the server package exports The lint lane sorts `from .session_store` before `from .skills`; my new export block landed between server and session_store.
Merged
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
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
Every client resolved a skill itself and prepended the skill's markdown body to the message text. Two costs:
Solution
send_messagetakes an optionalskillname. The server resolves it and composes the body into that turn's system prompt.{ "action": "send_message", "requestId": "…", "sessionId": "…", "message": "review my diff", "skill": "code-review" }The persisted user message stays exactly what the user typed.
The resolver seam
skills::SkillResolverasync fn resolve(&self, name) -> Option<String>. Installed viaAppState::with_skill_resolverorLocalServerBuilder::skill_resolver. Big Smooth plugs its own discovery in here.skills::DirSkillResolver<root>/<name>/SKILL.mdover the:-separated roots inSMOOTH_SKILLS_DIR, first match wins, YAML frontmatter stripped (it's discovery metadata, not instructions).SMOOTH_SKILLS_DIRunset ⇒ no resolver is installed, so a multi-tenant deploy never serves host skills by accident.Fail-closed, unlike
imagesAn unresolvable skill returns
error { code: "SKILL_NOT_FOUND" }and the turn does not run. Answering without the requested recipe is indistinguishable to the caller from answering with it, so degrading silently is worse than erroring.Names are
[A-Za-z0-9_-]{1,128}— path traversal is unrepresentable rather than filtered, since the name is joined onto a filesystem root.Backward compatibility
Absent
skillis byte-for-byte the previous behavior.TurnRequest::skill_sectiondefaults toNone, which appends no system-prompt section.Tests
rust/smooth-operator-server/tests/skill_field.rsdrives the realhandler::handle_frameagainst in-memory storage andMockLlmClient— fully offline, and it is the contract the polyglot servers will mirror:SKILL_NOT_FOUND,mock.call_count() == 0(the turn never ran)../greet) resolves nothingskill⇒ ordinary turn, no## Skill:in the promptskill⇒ treated as absentPlus unit tests in
skills.rsfor name validation, frontmatter stripping (including a---markdown rule mid-body and unterminated frontmatter), root precedence, and path-list parsing.Local gates (this repo's
rust/**PR CI is compile-gate only, so these are the real gate):cargo test -p smooai-smooth-operator-server --lib --tests→ 276 passed, 33 suitescargo clippy -p smooai-smooth-operator-server --all-targets→ 0 errors (10 warnings, all pre-existing on main)cargo fmt --allcleanDocs + spec
spec/actions/send-message.schema.json— the field, itspattern, and the fail-closed contractdocs/Reference/Protocol Reference.md— new "Skills onsend_message" sectionDeferred: polyglot parity
The TS / Python / Go / .NET servers ignore
skill(it degrades to an ordinary turn) — the same stagingimages[]is in today, where Rust is the reference implementation and the ports follow separately. Worth a follow-up pearl once the seam shape has settled here.🤖 Generated with Claude Code
https://claude.ai/code/session_01YSE8xAs9aWN5VhrnnSyLKq