docs: add design for invoking a skill from a message - #552
Closed
andrii-novikov wants to merge 14 commits into
Closed
andrii-novikov wants to merge 14 commits into
andrii-novikov wants to merge 14 commits into
Conversation
Designs a minimal first cut at consuming DIAL Core's folder-shaped skill resources (epam/ai-dial-core#1633), scoped to ship without refactoring anything that already works. Phase 1: a new `dial-skill` config type over `skills/<bucket>/<path>`, resolved per request into a manifest plus an inventory of the skill's bundled text files, and `read_skill(skill_name, file_path?)` for progressive disclosure — one round-trip per file the model actually asks for. `dial_skills/` mirrors the existing `dial_prompt_skills/` package shape; `SkillsRegistry` gains one merge pass and one async method. Predefined skills and `dial-prompt` are untouched. Readability is enforced by inventory membership: a `file_path` is served only if it was advertised, which subsumes traversal, encoded-separator and dotfile handling without a bespoke validator. Deferred to phase 2 with reasons: binary/asset files, cross-request caching, `read_skill` offload exclusion, editor skill browsing. No QuickApps-side validator for `dial-skill` — Core validates `SKILL.md` on write — though `/skills/validate` must be narrowed to the prompt config so the published schema stops advertising a type it rejects. Records the two blockers verified against the source: the `client.skills` resource is written but unreleased in ai-dial-client-python, and ai-dial-core has no skills auto-share (no `getSkills`, `shareApplicationSkills`, or `ApiKeyData.attachedSkills`), which limits phase 1 to skills the caller's own key can already reach. Refs #418 Claude-Session: https://claude.ai/code/session_011mUSHykzn4b9y97fpjkskn
Phase 1 of the design in docs/designs/skills_as_dial_resource.md: an app config can reference a DIAL skill resource, and the agent can open the files bundled with it on demand. A new `dial-skill` config type over `skills/<bucket>/<path>` joins `SkillConfig`. At request initialization each configured skill costs one `SKILL.md` read plus one recursive file listing; the listing is appended to the manifest as a `<skill_files>` block, so the model learns what the skill contains the moment it calls `read_skill`. It then opens one with `read_skill(skill_name, file_path)` — one round-trip per file it actually asks for, memoized for the rest of the request. `dial_skills/` deliberately mirrors the existing `dial_prompt_skills/` package shape rather than factoring out a shared lifecycle: the two are identical in outline, and unifying them would mean rewriting the prompt path. `SkillsRegistry` gains one merge pass and one async method. Predefined skills and `dial-prompt` are untouched. Readability is inventory membership: a `file_path` is served only if it was advertised, and the inventory is a text-extension allowlist minus folders, dotfiles and `SKILL.md`. That single check subsumes traversal, encoded separators and hidden-file leakage without a bespoke validator — the model can only ask for what it was told exists. Binary files are not advertised and not readable in this phase. `/skills/validate` is narrowed to `DialPromptSkillConfig` rather than gaining a `dial-skill` branch: Core validates `SKILL.md` on write, so a stored skill is already valid, and keeping the union would have published an OpenAPI advertising a request the handler answers 400 to. The dead `isinstance` check goes with it. Also corrects a stale claim in docs/skills.md: the `skills` config field is a plain `Field`, not a `PreviewField`, so it never required ENABLE_PREVIEW_FEATURES. Known incomplete, both recorded in the design doc: - `aidial-client` is still pinned to 0.16.x, which has no `skills` resource — it lives on an unreleased branch. Access funnels through one `TODO(#418)` shim, so this cannot run against a real DIAL Core until that client ships. - DIAL Core does not auto-share config-declared skills to the app's per-request key, so a `dial-skill` resolves only where the caller's own key already has access. Verified: `make lint` clean, 1959 unit tests pass (54 new). Refs #418 Claude-Session: https://claude.ai/code/session_011mUSHykzn4b9y97fpjkskn
Offloading a large read_skill result into a DIAL-file pointer would strip the manifest's <skill_files> inventory from the tool result the model sees, breaking progressive disclosure. Add internal_skills_read_skill to _MANDATORY_EXCLUDED_TOOLS alongside the existing read-back guards.
DialSkillConfig and DialSkillsModule were unconditionally active regardless of ENABLE_PREVIEW_FEATURES, exposing a config type whose resolution depends on an aidial-client attribute that does not exist in any released client version yet. Mark DialSkillConfig with @preview_model (strips it from the discriminated union / schema when preview is off) and DialSkillsModule with @preview_module (drops it from the DI module list), matching the pattern used by FolderContextConfig / other preview-gated modules.
Replace the local editable path dependency on aidial-client (which only resolved on a machine with both repos checked out side by side) with a git dependency pinned to the feat/skills-read branch of epam/ai-dial-client-python. This carries the skills resource (/v2/skills reads) the feature needs but that has not been released yet, and is resolvable by CI and other machines. Still TEMPORARY - MUST NOT MERGE: must be swapped for a released version once aidial-client ships the skills resource.
Status: Draft -> Approved. The D-1 client-release gate remains open (aidial-client feat/skills-read is still unmerged/unreleased) and is tracked as an implementation/merge blocker, not a design defect.
- docs: clarify dynamic skill registration is Partial (DIAL-prompt and DIAL-skill sources resolve per request) - fix stage title formatting (remove spaces around /) - remove stale TODO comment about pinned aidial-client version
SkillsRegistry previously reached into dial_skills' private submodules for ResolvedDialSkill, _DialSkillsContext and MANIFEST_NAME, and did the bundled-file read itself. dial_skills likewise reached into skills' private _frontmatter/_skill_metadata modules. - skills/__init__.py now exports SkillMetadata, parse_frontmatter and SkillFileNotFoundError as its public surface - dial_skills/__init__.py exports DialSkillReader, DialSkillResolver, ResolvedDialSkill and _DialSkillsContext - New DialSkillReader owns the manifest/inventory check, path normalization and cache-then-fetch for one bundled file; the cache is its own private state (request-scoped, used by no one else) - _DialSkillsContext is pure state again (resolved_skills + exceptions), mirroring _DialPromptSkillsContext - SkillsRegistry.read_skill_file collapses to a merge lookup plus one delegated call - DialSkillResolver._fetch_one results now pair with their own URL via _fetch_labeled instead of correlating asyncio.gather output by index - rename build_skill_files_block to _build_skill_files_block (internal) Addresses PR #524 review comments.
Follows aidial-client's move to chained references: get_file(url, path) and list_files(url, ...) become skills(url=...).files(path=...).read() and .files.list(...). The files reference is built once per listing, so the url is parsed once rather than on every page. Replaces the MagicMock-based test doubles with hand-written fakes: skills.files.list() and skills.files(path=...).read() land on different auto-created mocks, so stubbing one silently failed to intercept the other. The fakes mirror the real signatures, so drift surfaces as a TypeError instead of a passing test. Refs #418
Replace the temporary git-branch override (feat/skills-read) with the released aidial-client 0.17.0, which ships the skills resource. Claude-Session: https://claude.ai/code/session_01TqzW8dqXH92sC28kX6WoxQ
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
Base automatically changed from
feat/418-skills-as-dial-resource
to
development
September 14, 2026 09:35
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.
Applicable issues
Description of changes
Why. A skill reaches a QuickApp agent in exactly one way today: the app author lists it in
ApplicationConfig.skills, QuickApps advertises it in<available_skills>, and the model decides whether to callread_skill. The user has no say. That leaves two gaps:/skill-namea guarantee.code-reviewin their own DIAL catalog cannot bring it into a conversation with an agent they don't own — under a per-request key the app can't readskills/<user-bucket>/…at all.DIAL Core and DIAL Chat have agreed a wire contract that closes the access half: a user message may carry
custom_content.skills[*], and Core auto-shares each referenced skill to the app's per-request key, exactly as it already does forcustom_content.attachments[*](done: epam/ai-dial-core#1956). This design specifies how QuickApps consumes that field.What the design proposes.
custom_content.skills[*]off user messages; skills are addressed by URL, never by name. Requests without the field behave exactly as today, system prompt included._SkillInvocationInitializerresolves each picked URL through the existingDialSkillResolver. A chip that fails to load is reported to both the user and the model, and the request is still served.user:<name>:<hash>, so every name in<available_skills>stays unique. A user skill can never shadow an agent skill, and there is no precedence rule to reason about. The user's bucket id is never shown to the model.read_skillcall and result are inserted after the user message, so the manifest is already in context when the model's turn starts. Re-picking a skill already present in history is skipped, and bundled files stay readable throughread_skillfor the rest of the conversation.The doc also records the use cases, failure modes, a known gap (a share revoked mid-conversation), alternatives considered, and the per-package summary of changes. It is marked Approved after four review rounds.
No code changes — implementation follows in #549.
Checklist
[ ] Documentation is updated/created (if applicable)(user-facing docs land with the implementation in Invoke user skills from a chat message (phase 1) #549)[ ] Changes are tested on review environment(docs-only, nothing deployable)[ ] App schema changes are backward compatible, or breaking changes are documented with a migration guide(no config model touched; the design's own Migration section notes the contract is additive)[ ] Integration tests pass(docs-only)By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
https://claude.ai/code/session_0184dFYXFuKAjivbVvNxL2fa