-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: add per-agent skills support #995
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
5a47cfa
95c12b1
31b139c
3bd57a1
6f3083b
556d2a1
276df2f
70ec032
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1638,6 +1638,16 @@ public class CustomAgentConfig | |
| /// </summary> | ||
| [JsonPropertyName("infer")] | ||
| public bool? Infer { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// List of skill names to preload into this agent's context. | ||
| /// When set, the full content of each listed skill is eagerly injected into | ||
| /// the agent's context at startup. Skills are resolved by name from the | ||
| /// session's configured skill directories (skillDirectories). | ||
| /// When omitted, no skills are injected (opt-in model). | ||
| /// </summary> | ||
| [JsonPropertyName("skills")] | ||
| public List<string>? Skills { get; set; } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: Use Every other nullable public IList<string>? SkillDirectories { get; set; } // line ~1865
public IList<string>? DisabledSkills { get; set; } // line ~1870
public IList<string>? Tools { get; set; } // line ~1622Suggestion: public IList<string>? Skills { get; set; }This keeps the public API surface consistent with the rest of the SDK's collection-typed properties.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cross-SDK consistency / .NET type convention: This property uses the concrete Suggestion: change to match the established pattern: public IList<string>? Skills { get; set; }
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The property type should be public IList<string>? Skills { get; set; }
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The // Suggested change
public IList<string>? Skills { get; set; }For reference, the public IList<string>? Tools { get; set; }And similarly, |
||
| } | ||
|
|
||
| /// <summary> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cross-SDK consistency / .NET convention: This property uses the concrete
List<string>?type, but the established convention throughoutTypes.csis to use the interface typeIList<string>?for public collection properties (seeToolsat line 1622,SkillDirectoriesat line 1865,AvailableToolsat line 1792, etc.).Suggestion:
This keeps the public API surface consistent with every other nullable list property in the file.