feat(skills): invoke a user's own skill from a message - #553
Open
andrii-novikov wants to merge 9 commits into
Open
andrii-novikov wants to merge 9 commits into
andrii-novikov wants to merge 9 commits into
Conversation
andrii-novikov
force-pushed
the
feat/549-invoke-user-skills
branch
from
September 14, 2026 07:57
09e882a to
5d51c1f
Compare
Base automatically changed from
feat/418-skills-as-dial-resource
to
development
September 14, 2026 09:35
andrii-novikov
force-pushed
the
feat/549-invoke-user-skills
branch
2 times, most recently
from
September 14, 2026 12:51
56564f0 to
fd5da1e
Compare
A skill reaches a QuickApp agent only if the app author attached it and the model decides to call read_skill — the user has no deterministic way to say "use this skill for this message", and no way in at all for skills from their own catalog. This design specifies how QuickApps consumes the custom_content.skills wire contract agreed with DIAL Core (epam/ai-dial-core#1956): resolve each picked skill through DialSkillResolver, register it under a collision-free user:<name>:<hash> id, and inject a synthetic read_skill call and result after the user message so the manifest is in context before the model's turn. Phase 1 covers the user's own skills; picking the agent's own skills is deferred to #550. Claude-Session: https://claude.ai/code/session_0184dFYXFuKAjivbVvNxL2fa
Narrows the design to what the feature actually needs, and states the assumptions it now rests on instead of engineering around them. A picked skill is registered as an ordinary SkillsProvider entry under its own manifest name (order = -10), so it is listed, injected as a synthetic read_skill pair, and its bundled files stay readable on later turns through the existing registry — no new lookup path and no new collision machinery. Name clashes go through the merge SkillsRegistry already performs; the design adds no rule of its own and instead states the assumption that picked skills have non-clashing names. Shadowing of predefined and declared skills is recorded as a known gap, closed by 1b. Drops the user:<name>:<hash> scheme, the trimmed metadata copy, the stage-title parse and the per-message cap. Moves the custom_content.skills scrub into the never-gated SkillsModule so the field cannot reach the orchestrator deployment with preview off. Adds phase 1b as a follow-up section covering collision-free names. Refs #549
Corrects the one claim an implementer would have built against: clashes between two picks are settled by DialSkillResolver's name dedup, not by SkillsRegistry. Two picks live in the same provider, so the registry never sees the loser. Documents the real mechanism — oldest pick wins, reported with the resolver's duplicate-name wording every turn — and accepts the three consequences, all unreachable while picked skill names do not clash. Also removes two traps that would have produced silently broken code: only the tool lookup follows StagedToolSyntheticInjector, since its get_content forces stage_level=DEBUG and would suppress the "Reading Skill" stage; and the injector's trigger is stated once in terms of the context, because the SkillsModule scrub runs first and the messages no longer carry chips by then. Adds failure-modes rows for the two-URL same-name case, and records why reporting a duplicate distinctly and unique_names=False were rejected. Refs #549
Adds quickapp/skill_invocation/, a preview-gated module that reads custom_content.skills[*] off the user messages, resolves each pick through the existing DialSkillResolver, and registers it as an ordinary SkillsProvider entry (order -10) under its own manifest name — so the merge, generate_skills_xml, read_skill and bundled-file reads all work unchanged. A synthetic read_skill pair is injected right after the message that picked the skill, so the model always starts that turn with the manifest and its <skill_files> inventory in context; later turns restore the pair from state.tool_execution_history, which keeps the manifest stable and the skill's files readable. A chip that fails to resolve gets a fixed error result and the reason reaches the user in the initialization issues stage. Scrubbing custom_content.skills off the working messages lives in the never-preview-gated SkillsModule, so the field never reaches the orchestrator or a DIAL deployment tool regardless of the preview flag. quickapp/dial_skills/ is untouched. Phase 1a of the approved design, which accepts that a pick shadows an agent skill of the same name.
…ion #549 Replaces the bespoke injector with a StagedToolSyntheticInjector subclass: the tool lookup by function name, the not-found branch and the arun call all come from the mixin, leaving five small methods and one override. APPEND_IF_CHANGED puts the pair after the first user message, the slot the built-in file-transfer skill already uses, and should_inject limits the transformer to the turn the pick is made on. Later turns restore the pair from state.tool_execution_history like any other tool result, so read_skill is never re-run and the model keeps the manifest it first saw. StagedToolSyntheticInjector.stage_level becomes an overridable class attribute, defaulting to the DEBUG it hardcoded before, so every existing subclass is unaffected. Skill invocation raises it to INFO: the user asked for the skill, so the ordinary "Reading Skill" stage belongs in the response. get_content is overridden only to return a fixed error sentence for a skill that never reached the registry, rather than running the tool for the sake of its own "not found". Only one skill may be invoked per message. Extra chips on the message being answered are ignored and reported as a warning-severity initialization issue naming them, rather than dropped silently; extras on earlier turns are logged, so the stage does not repeat an issue the user can no longer act on.
The design predates the simplifications made while building it, so the approved doc no longer described what shipped. A message invokes at most one skill: only the first entry is loaded, and extras are ignored - reported as a warning on the message being answered, logged on earlier turns. Picks are keyed by user-message ordinal and deduped keeping the first occurrence, so a re-pick neither refreshes the skill's position under the cap nor injects a second pair. The context holds the resolved skills by URL and a single current-pick URL, not a list of this turn's chips. The injector is a StagedToolSyntheticInjector subclass on APPEND_IF_CHANGED, which puts the pair after the first user message rather than the last, gated by should_inject instead of a call-id prefix skip check. stage_level became an overridable class attribute, so the planned public helpers in synthetic_tool_call_injector.py were never needed and that file is untouched. Three failure-mode rows were wrong as a consequence and are corrected, together with the Alternatives row that rested on the removed skip check. Claude-Session: https://claude.ai/code/session_017gdYmTNUda8ZpR81aeahCB
andrii-novikov
force-pushed
the
feat/549-invoke-user-skills
branch
from
September 14, 2026 14:06
c646d5a to
3306b69
Compare
…549 A SkillInitializationException built without a `url` fell through the renderer's unhandled branch after the stage had already opened, so invoking more than one skill on a message produced an empty "Initialization issues" stage with no explanation. The exception class already formats for `url=None`; only the renderer did not. The same guard silently dropped the url-less resolver exceptions that `_SkillInvocationInitializer.__report` deliberately forwards. Alongside it, three conventions fixes and a correction to the docs: - Import the skill-reader tool name from `common.tool_names` rather than the private `skills._tool_configs`; it is a literal alias, and this was the only cross-package private import in the app tree. - Drop the per-turn skip log to DEBUG and name the skill, and drop the multi-chip log to DEBUG — both re-fired on every turn for the life of a conversation because each turn re-parses the whole message list. - Guard `_InvokedSkillsContext` mutators with a lock, as both sibling skill contexts already do. - State the real persistence rule in the injector docstring and docs/skills.md: only a pick made on the first user message is stored in `state.tool_execution_history`, because the orchestrator persists just what follows the last user message. A pick made later is in context for its own turn only; the skill stays listed and readable either way. Recorded as a known gap for phase 1a. The design doc gains the acknowledged stale-manifest gap: the shared `SyntheticToolCallInjector` appends rather than replaces when a pair's content changed, so editing a skill between turns can leave two manifests under one name.
andrii-novikov
marked this pull request as ready for review
September 14, 2026 14:35
Update docs/designs/skill_invocation.md to match the current repo state: - skills_as_dial_resource.md's dependency (feat/418) is merged to development (#524), not pending - document the known gap where a skill picked on a non-first user message doesn't survive the next turn, matching what docs/skills.md and the injector's docstring already say #549
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.
Applicable issues
Description of changes
Lets a user invoke their own skill from a message, in a conversation with an agent they don't
own. Phase 1a of
docs/designs/skill_invocation.md.The client puts the picked skill on the user message; DIAL Core auto-shares it to the app's
per-request key (already done in epam/ai-dial-core#1956):
{ "role": "user", "content": "/code-review focus on auth", "custom_content": { "skills": [{ "url": "skills/<bucket>/code-review" }] } }QuickApps resolves it, registers it as an ordinary skill, and injects a synthetic
read_skillcall/result pair so the manifest is in the model's context before it answers — the model doesn't
get to decide whether to load it.
New package
quickapp/skill_invocation/(preview-gated, registered afterDialSkillsModule):_skill_reference.pycustom_content.skills[*]off user messages — canonicalises the URL, drops malformed entries, keys picks by user-message ordinal_skill_invocation_initializer.pyDialSkillResolver_invoked_skills_context.pySkillsProvideratorder = -10,display_name = "user skills"_skill_invocation_injector.pyStagedToolSyntheticInjectorsubclass that injects the pair_settings.pySKILL_INVOCATION_MAX_SKILLS(default 10)Because a pick becomes an ordinary
SkillsProviderentry, everything downstream works unchanged:the
SkillsRegistrymerge,generate_skills_xml,read_skill, and bundled-file reads viaDialSkillReader.quickapp/dial_skills/is not touched.Behaviour
uses — and the user sees the normal "Reading Skill:
<name>" stage.state.tool_execution_historylike any other tool result, soread_skillis never re-run and themodel keeps the manifest it first saw.
<available_skills>and its bundled files still resolve on turn 4.reported by the existing
SkillsRegistrycollision path. Accepted for 1a — see Known gaps.the user. The request is still served.
custom_content.skillsis stripped from the working messages by a transformer in thenever-preview-gated
SkillsModule, so the field cannot reach the orchestrator deployment or aDIAL deployment tool even with the feature off.
Outside the feature package
StagedToolSyntheticInjector.stage_levelbecomes an overridable class attribute, defaulting tothe
StageDisplayLevel.DEBUGit previously hardcoded — every existing subclass is unaffected.Skill invocation raises it to
INFO, because the user asked for the skill explicitly.REQUEST_MESSAGESDI alias, so an initializer can read the raw request messages (they run beforesetup_messagespopulatescontext.messages).Deviations from the approved design — the design doc is a historical record and was not edited,
so recording them here:
synthetic_injectionhelpers made publicStagedToolSyntheticInjectoronAPPEND_IF_CHANGEDaruncall;common/synthetic_injection/synthetic_tool_call_injector.pyends up untouchedwarning-severity initialization issue naming themDocs:
docs/skills.mdgains an "Invoking a Skill from a Message" section;README.mddocumentsSKILL_INVOCATION_MAX_SKILLS;CLAUDE.mdnotes the new package.Known gaps (documented, deliberate for 1a)
collision-free names.
stays listed and
read_skill-able, just not re-injected. Turn-1 picks — the realistic"
/code-review …opens a task" flow — are fully stable.403, before the request reaches QuickApps. Fix belongs in Core._inject_append_if_changedmatches an existingpair on content as well as tool and arguments, so when the user edits the skill in Core between
turns the match fails and a second pair is appended instead of replacing the first. The flaw is in
the shared
SyntheticToolCallInjector—_InjectFileTransferInstructionTransformerhas the sameexposure, only a redeploy is needed to reach it — and skill invocation makes it reachable by an
ordinary user action. Either fix changes behaviour for every injector, so it is out of scope here.
Testing: 44 new unit tests (
skill_invocation_tests/, plus the scrub transformer and thestage_leveloverride).make lintclean,make test2052 passing. Integration tests not run.Not in this PR: the
ai-dial-chatside (the/palette,skillsonMessageCustomContentDto)is agreed but not started, so the feature is unreachable from the UI until that lands.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.