Conversation
There was a problem hiding this comment.
Pull request overview
Adds per-custom-agent skill scoping to the SDK surface area (types + docs) and introduces new cross-SDK E2E coverage + replay snapshots for the opt-in behavior.
Changes:
- Added
skillstoCustomAgentConfigacross Node.js, Python, Go, and .NET SDK types. - Updated docs to describe per-agent skill scoping and added
agentNametoskill.invokedevent docs. - Added E2E tests and new replay snapshots for “agent with skills” vs “agent without skills field”.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
nodejs/src/types.ts |
Adds skills?: string[] to CustomAgentConfig with JSdoc explaining opt-in semantics. |
python/copilot/session.py |
Adds skills to the Python CustomAgentConfig TypedDict. |
go/types.go |
Adds Skills []string to Go CustomAgentConfig for JSON serialization. |
dotnet/src/Types.cs |
Adds List<string>? Skills to .NET CustomAgentConfig with XML docs. |
docs/features/custom-agents.md |
Documents the new skills field and adds a dedicated per-agent skills section. |
docs/features/skills.md |
Updates “Skills + Custom Agents” section with per-agent skills example and opt-in note. |
docs/features/streaming-events.md |
Documents agentName on skill.invoked. |
nodejs/test/e2e/skills.test.ts |
Adds Node E2E tests for agent-scoped skills behavior. |
python/e2e/test_skills.py |
Adds Python E2E tests for agent-scoped skills behavior. |
go/internal/e2e/skills_test.go |
Adds Go E2E tests for agent-scoped skills behavior. |
dotnet/test/SkillsTests.cs |
Adds .NET E2E tests for agent-scoped skills behavior. |
test/snapshots/skills/should_allow_agent_with_skills_to_invoke_skill.yaml |
Replay snapshot for the “agent with skills can invoke” scenario. |
test/snapshots/skills/should_not_provide_skills_to_agent_without_skills_field.yaml |
Replay snapshot for the “no skills field => no skills” scenario. |
nodejs/package-lock.json |
Lockfile metadata updates. |
Files not reviewed (1)
- nodejs/package-lock.json: Language not supported
This comment has been minimized.
This comment has been minimized.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #995
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
- Add skills field to Python wire format converter - Explicitly select agents in all E2E tests for deterministic behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
- Add skills field to Python wire format converter - Explicitly select agents in all E2E tests for deterministic behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MackinnonBuck
force-pushed
the
mackinnonbuck/agent-skills-958
branch
from
b6eb0fa to
c1272a1
Compare
This comment has been minimized.
This comment has been minimized.
Add skills: Option<Vec<String>> to CustomAgentConfig, matching the new per-agent skills support in CLI 1.0.23 (SweCustomAgent.skills). Skills are referenced by name from the session's skillDirectories pool and eagerly preloaded into the agent's context at startup. Adds smoke_subagent_with_skills test that creates a skill directory with a SKILL.md containing a known passphrase, configures a subagent with skills: ["trivia-skill"], and verifies the model can access the skill content. Also fixes skill directory structure in smoke_agent_with_skill to use the proper subdirectory/SKILL.md convention. Upstream SDK PR: github/copilot-sdk#995 (draft, not yet merged) Runtime support: CLI 1.0.23 native binary Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a 'skills' field to CustomAgentConfig across all four SDK languages (Node.js, Python, Go, .NET) to support scoping skills to individual subagents. Skills are opt-in: agents get no skills by default. Changes: - Add skills?: string[] to CustomAgentConfig in all SDKs - Update custom-agents.md with skills in config table and new section - Update skills.md with per-agent skills example and opt-in note - Update streaming-events.md with agentName on skill.invoked event - Add E2E tests for agent-scoped skills in all four SDKs - Add snapshot YAML files for new test scenarios Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update type comments, docs, and test descriptions to reflect that per-agent skills are eagerly injected into the agent's context at startup rather than filtered for invocation. Sub-agents do not inherit skills from the parent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The runtime does not emit agentName on the skill.invoked event. The agent name is used only for internal logging during eager skill loading, not as event data. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add skills field to Python wire format converter - Explicitly select agents in all E2E tests for deterministic behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The generated_session_events.go on main changed from a flat Data struct to a SessionEventData interface with per-event typed structs. The agent skills test cases added in this PR were using the old message.Data.Content pattern instead of the type assertion pattern used elsewhere. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MackinnonBuck
force-pushed
the
mackinnonbuck/agent-skills-958
branch
from
c1272a1 to
6f3083b
Compare
MackinnonBuck
changed the title
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #995 · ● 510.2K
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #995 · ● 462.3K
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #995 · ● 468.7K
The runtime now eagerly injects skill content into <agent_instructions> in the user message instead of using a skill tool call. Update the replay proxy to strip <agent_instructions> during normalization, and simplify the snapshot for agent-with-skills to match the new flow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MackinnonBuck
force-pushed
the
mackinnonbuck/agent-skills-958
branch
from
305dd4e to
276df2f
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #995 · ● 543.4K
Add two regression tests validating that <agent_instructions> blocks are properly stripped during user message normalization, including the case where skill-context is nested inside agent_instructions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #995 · ● 599.7K
Match the established convention used by Tools, SkillDirectories, DisabledSkills, and other collection properties in the codebase. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Cross-SDK Consistency Review ✅This PR adds per-agent skills support consistently across all four SDK implementations. Here's a summary of the review: What was checked
FindingsConsistent across all SDKs ✅
All four implementations use proper language idioms, follow naming conventions, and include equivalent E2E tests covering both the "agent with skills" and "agent without skills (opt-in)" scenarios. One potential gap 🔍The PR description lists this change:
However, If this field is expected to be emitted by the runtime when a per-agent skill is invoked (to identify which agent triggered it), the docs and type definitions may need a follow-up update. If This is a minor observation — the core feature implementation is solid and consistent across all SDKs.
|
Summary
Adds SDK support for scoping skills to individual subagents, addressing #958.
Changes
Types (all 4 SDKs):
nodejs/src/types.ts): Addedskills?: string[]toCustomAgentConfigpython/copilot/session.py): Addedskillsfield to agent configgo/types.go): AddedSkills []stringto agent config structdotnet/src/Types.cs): AddedList<string>? Skillsto agent config classDocumentation:
docs/features/custom-agents.md: Addedskillsto config reference table and new section on per-agent skillsdocs/features/skills.md: Updated "Skills + Custom Agents" section with per-agent exampledocs/features/streaming-events.md: AddedagentNamefield toskill.invokedevent docsTests (all 4 SDKs):
skillscan invoke listed skillsskillsfield gets no skills (backward compatible)Design decisions
skillsis optional. Omitting it means the agent has no access to skills (not all skills).skillDirectoriespool.Closes #958