feat(config): add configurable prompts support #559
Conversation
nader-ziada
changed the title
nader-ziada
force-pushed
the
1-configurable-prompts
branch
2 times, most recently
from
aaac007 to
7994bf0
Compare
|
Spec Proposed: What Was Actually Implemented: why: Config prompts: PromptLoader.createHandler() generates a handler that performs {{argument}} substitution on the Templates field at runtime (For Later): Toolset prompts: Implementers provide a custom handler function directly in ServerPrompt.Handler |
|
Thanks for taking care of this. I still find that the API is getting far too complicated and complex for something that should be simpler and more intuitive. Let me create a PR to your branch with some proposed changes so that we can better discuss tradeoffs. |
Enable administrators to define custom MCP prompts in config.toml
using template substitution with {{argument}} syntax.
- PromptLoader for loading prompts from TOML config
- Core prompt types (ServerPrompt, Prompt, PromptArgument, PromptMessage)
- Template argument substitution with {{variable}} syntax
- Required argument validation
- Integration with MCP server to register and serve config prompts
Signed-off-by: Nader Ziada <[email protected]>
nader-ziada
force-pushed
the
1-configurable-prompts
branch
from
99858f9 to
7dcf0a7
Compare
|
@manusa ready for review |
| // mergePrompts merges two slices of prompts, with prompts in override taking precedence | ||
| // over prompts in base when they have the same name | ||
| func mergePrompts(base, override []api.ServerPrompt) []api.ServerPrompt { | ||
| // Create a map of override prompts by name for quick lookup | ||
| overrideMap := make(map[string]api.ServerPrompt) | ||
| for _, prompt := range override { | ||
| overrideMap[prompt.Prompt.Name] = prompt | ||
| } | ||
|
|
||
| // Build result: start with base prompts, skipping any that are overridden | ||
| result := make([]api.ServerPrompt, 0, len(base)+len(override)) | ||
| for _, prompt := range base { | ||
| if _, exists := overrideMap[prompt.Prompt.Name]; !exists { | ||
| result = append(result, prompt) | ||
| } | ||
| } | ||
|
|
||
| // Add all override prompts | ||
| result = append(result, override...) | ||
|
|
||
| return result | ||
| } |
There was a problem hiding this comment.
I believe this is something you'll be using in the follow-up for #556
| Name string `yaml:"name" json:"name" toml:"name"` | ||
| Title string `yaml:"title,omitempty" json:"title,omitempty" toml:"title,omitempty"` | ||
| Description string `yaml:"description,omitempty" json:"description,omitempty" toml:"description,omitempty"` | ||
| Arguments []PromptArgument `yaml:"arguments,omitempty" json:"arguments,omitempty" toml:"arguments,omitempty"` | ||
| Templates []PromptTemplate `yaml:"messages,omitempty" json:"messages,omitempty" toml:"messages,omitempty"` |
There was a problem hiding this comment.
I'm not sure we need the YAML tags or the yaml serialization tests.
Maybe this is a misunderstanding because the issue (#226) explicitly mentions yaml.
I believe I included this because the former PR had the yaml tags.
From my perspective there's no need for yaml since the prompts will be provided by the toml configuration and I don't believe they'll be marshalled to yaml from the MCP layer.
If you don't see any reason to include those, maybe we can simply remove the yaml layer.
There was a problem hiding this comment.
i can remove this in a follow up PR
| // Prompt configuration | ||
| // Raw TOML primitive for prompt definitions, parsed later | ||
| // Note: Uses toml:"-" because Primitive can't be encoded, only decoded | ||
| Prompts toml.Primitive `toml:"-"` | ||
| promptsDefined bool // Internal: tracks if prompts were defined in config | ||
| promptsMetadata toml.MetaData // Internal: metadata for prompts decoding |
There was a problem hiding this comment.
I don't quite grasp the limitation here.
I think it would be much clearer if StaticConfig only defined a Prompts []api.Prompt field.
I'll look into this because I'm sure I might be missing something, but the end goal would be that config-wise we didn't have to iterate over the prompts.
There was a problem hiding this comment.
OK; I see we're still having problems with the cyclic dependencies and the kubernetes package. I'll try to properly address this, creating an issue now for tracking.
manusa
changed the title
manusa
left a comment
manusa
left a comment
There was a problem hiding this comment.
LGTM, thx!
Thx for your patience too 😅
2 participants
This PR replaces: #510
Fixes: #226
Enable administrators to define custom MCP prompts in config.toml using template substitution with {{argument}} syntax.
Features: