Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pkg/codingcontext/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ func main() {

// Access MCP server configurations
mcpServers := result.MCPServers()
for i, config := range mcpServers {
fmt.Printf("MCP Server %d: %s\n", i, config.Command)
for id, config := range mcpServers {
fmt.Printf("MCP Server %s: %s\n", id, config.Command)
}
}
```
Expand All @@ -139,7 +139,7 @@ Result holds the assembled context from running a task:
- `Agent Agent` - The agent used (from task frontmatter or option)

**Methods:**
- `MCPServers() []MCPServerConfig` - Returns all MCP server configurations from rules as a slice
- `MCPServers() map[string]MCPServerConfig` - Returns all MCP server configurations from rules as a map from rule ID to configuration

#### `Markdown[T]`

Expand Down
23 changes: 23 additions & 0 deletions pkg/codingcontext/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ func New(opts ...Option) *Context {
return c
}

// generateIDFromPath generates an ID from a file path by extracting the filename without extension.
// Used to auto-set ID fields in frontmatter when not explicitly provided.
func generateIDFromPath(path string) string {
baseName := filepath.Base(path)
ext := filepath.Ext(baseName)
return strings.TrimSuffix(baseName, ext)
Comment on lines +63 to +64
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generateIDFromPath function does not handle edge cases such as files without extensions or multiple extensions. For example, "file.tar.gz" would result in ID "file.tar" instead of "file". Consider using filepath.Base followed by removing all extensions, or documenting this behavior if it's intentional. Also consider handling empty filenames or paths with only an extension (e.g., ".gitignore").

Suggested change
ext := filepath.Ext(baseName)
return strings.TrimSuffix(baseName, ext)
if baseName == "" || baseName == "." {
return ""
}
name := baseName
for {
ext := filepath.Ext(name)
if ext == "" {
break
}
// If the entire name is just an extension (e.g., ".gitignore"),
// avoid returning an empty string and instead strip the leading dot.
if len(name) == len(ext) {
return strings.TrimPrefix(name, ".")
}
name = strings.TrimSuffix(name, ext)
}
return name

Copilot uses AI. Check for mistakes.
}

type markdownVisitor func(path string, fm *markdown.BaseFrontMatter) error

// findMarkdownFile searches for a markdown file by name in the given directories.
Expand Down Expand Up @@ -129,6 +137,11 @@ func (cc *Context) findTask(taskName string) error {
return fmt.Errorf("failed to parse task file %s: %w", path, err)
}

// Automatically set ID to filename (without extension) if not set in frontmatter
if frontMatter.ID == "" {
frontMatter.ID = generateIDFromPath(path)
}

// Extract selector labels from task frontmatter and add them to cc.includes.
// This combines CLI selectors (from -s flag) with task selectors using OR logic:
// rules match if their frontmatter value matches ANY selector value for a given key.
Expand Down Expand Up @@ -229,6 +242,11 @@ func (cc *Context) findCommand(commandName string, params taskparser.Params) (st
return fmt.Errorf("failed to parse command file %s: %w", path, err)
}

// Automatically set ID to filename (without extension) if not set in frontmatter
if frontMatter.ID == "" {
frontMatter.ID = generateIDFromPath(path)
}

// Extract selector labels from command frontmatter and add them to cc.includes.
// This combines CLI selectors, task selectors, and command selectors using OR logic:
// rules match if their frontmatter value matches ANY selector value for a given key.
Expand Down Expand Up @@ -516,6 +534,11 @@ func (cc *Context) findExecuteRuleFiles(ctx context.Context, homeDir string) err
return fmt.Errorf("failed to parse markdown file %s: %w", path, err)
}

// Automatically set ID to filename (without extension) if not set in frontmatter
if frontmatter.ID == "" {
frontmatter.ID = generateIDFromPath(path)
}

// Expand parameters only if expand is not explicitly set to false
var processedContent string
if shouldExpandParams(frontmatter.ExpandParams) {
Expand Down
66 changes: 66 additions & 0 deletions pkg/codingcontext/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,32 @@ func TestContext_Run_Basic(t *testing.T) {
}
},
},
{
name: "task ID automatically set from filename",
setup: func(t *testing.T, dir string) {
createTask(t, dir, "my-task", "", "Task content")
},
taskName: "my-task",
wantErr: false,
check: func(t *testing.T, result *Result) {
if result.Task.FrontMatter.ID != "my-task" {
t.Errorf("expected task ID 'my-task', got %q", result.Task.FrontMatter.ID)
}
},
},
{
name: "task with explicit ID in frontmatter",
setup: func(t *testing.T, dir string) {
createTask(t, dir, "file-name", "id: explicit-task-id", "Task content")
},
taskName: "file-name",
wantErr: false,
check: func(t *testing.T, result *Result) {
if result.Task.FrontMatter.ID != "explicit-task-id" {
t.Errorf("expected task ID 'explicit-task-id', got %q", result.Task.FrontMatter.ID)
}
},
},
}

for _, tt := range tests {
Expand Down Expand Up @@ -699,6 +725,46 @@ func TestContext_Run_Rules(t *testing.T) {
}
},
},
{
name: "rule IDs automatically set from filename",
setup: func(t *testing.T, dir string) {
createTask(t, dir, "id-task", "", "Task")
createRule(t, dir, ".agents/rules/my-rule.md", "", "Rule without ID in frontmatter")
createRule(t, dir, ".agents/rules/another-rule.md", "id: explicit-id", "Rule with explicit ID")
},
taskName: "id-task",
wantErr: false,
check: func(t *testing.T, result *Result) {
if len(result.Rules) != 2 {
t.Fatalf("expected 2 rules, got %d", len(result.Rules))
}

// Check that one rule has auto-generated ID from filename
foundMyRule := false
foundAnotherRule := false
for _, rule := range result.Rules {
if rule.FrontMatter.ID == "my-rule" {
foundMyRule = true
if !strings.Contains(rule.Content, "Rule without ID") {
t.Error("my-rule should contain 'Rule without ID'")
}
}
if rule.FrontMatter.ID == "explicit-id" {
foundAnotherRule = true
if !strings.Contains(rule.Content, "Rule with explicit ID") {
t.Error("explicit-id should contain 'Rule with explicit ID'")
}
}
}

if !foundMyRule {
t.Error("expected to find rule with auto-generated ID 'my-rule'")
}
if !foundAnotherRule {
t.Error("expected to find rule with explicit ID 'explicit-id'")
}
},
},
Comment on lines +728 to +767
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test coverage for duplicate IDs scenario. Consider adding a test case that verifies behavior when two rules have the same ID (either from same filenames in different directories, or explicit duplicate IDs in frontmatter). This should validate whether duplicates are detected, logged, or result in silent data loss.

Copilot uses AI. Check for mistakes.
Comment on lines +728 to +767
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing integration test coverage for the MCPServers functionality. Consider adding an integration test that creates rule files with MCP server configurations, runs the context, and validates that MCPServers returns the correct map with the expected IDs and configurations. This would validate the full flow from file loading through ID generation to the MCPServers method.

Copilot uses AI. Check for mistakes.
}

for _, tt := range tests {
Expand Down
12 changes: 7 additions & 5 deletions pkg/codingcontext/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,21 @@ type Result struct {
Prompt string // Combined prompt: all rules and task content
}

// MCPServers returns all MCP server configurations from rules.
// MCPServers returns all MCP server configurations from rules as a map.
// Each rule can specify one MCP server configuration.
// Returns a slice of all configured MCP servers from rules only.
// Returns a map from rule ID to MCP server configuration.
// Empty/zero-value MCP server configurations are filtered out.
func (r *Result) MCPServers() []mcp.MCPServerConfig {
var servers []mcp.MCPServerConfig
// The rule ID is automatically set to the filename (without extension) if not
// explicitly provided in the frontmatter.
func (r *Result) MCPServers() map[string]mcp.MCPServerConfig {
servers := make(map[string]mcp.MCPServerConfig)

// Add server from each rule, filtering out empty configs
for _, rule := range r.Rules {
server := rule.FrontMatter.MCPServer
// Skip empty MCP server configs (no command and no URL means empty)
if server.Command != "" || server.URL != "" {
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a rule has an empty ID (neither auto-generated nor explicitly set), it will be used as a map key with an empty string. This could cause multiple rules with empty IDs to overwrite each other. Consider adding validation to ensure IDs are non-empty before inserting into the map, or add a test case to validate behavior with empty IDs.

Suggested change
if server.Command != "" || server.URL != "" {
if (server.Command != "" || server.URL != "") && rule.FrontMatter.ID != "" {

Copilot uses AI. Check for mistakes.
servers = append(servers, server)
servers[rule.FrontMatter.ID] = server
}
}

Expand Down
116 changes: 99 additions & 17 deletions pkg/codingcontext/result_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func TestResult_MCPServers(t *testing.T) {
tests := []struct {
name string
result Result
want []mcp.MCPServerConfig
want map[string]mcp.MCPServerConfig
}{
{
name: "no MCP servers",
Expand All @@ -82,19 +82,21 @@ func TestResult_MCPServers(t *testing.T) {
FrontMatter: markdown.TaskFrontMatter{},
},
},
want: []mcp.MCPServerConfig{},
want: map[string]mcp.MCPServerConfig{},
},
{
name: "MCP servers from rules only",
name: "MCP servers from rules with IDs",
result: Result{
Rules: []markdown.Markdown[markdown.RuleFrontMatter]{
{
FrontMatter: markdown.RuleFrontMatter{
ID: "jira-server",
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeStdio, Command: "jira"},
},
},
{
FrontMatter: markdown.RuleFrontMatter{
ID: "api-server",
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeHTTP, URL: "https://api.example.com"},
},
},
Expand All @@ -103,9 +105,35 @@ func TestResult_MCPServers(t *testing.T) {
FrontMatter: markdown.TaskFrontMatter{},
},
},
want: []mcp.MCPServerConfig{
{Type: mcp.TransportTypeStdio, Command: "jira"},
{Type: mcp.TransportTypeHTTP, URL: "https://api.example.com"},
want: map[string]mcp.MCPServerConfig{
"jira-server": {Type: mcp.TransportTypeStdio, Command: "jira"},
"api-server": {Type: mcp.TransportTypeHTTP, URL: "https://api.example.com"},
},
},
{
name: "MCP servers from rules without explicit IDs in frontmatter",
result: Result{
Rules: []markdown.Markdown[markdown.RuleFrontMatter]{
{
FrontMatter: markdown.RuleFrontMatter{
ID: "rule-file-1", // ID is auto-set to filename during loading
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeStdio, Command: "server1"},
},
},
{
FrontMatter: markdown.RuleFrontMatter{
ID: "rule-file-2", // ID is auto-set to filename during loading
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeStdio, Command: "server2"},
},
},
},
Task: markdown.Markdown[markdown.TaskFrontMatter]{
FrontMatter: markdown.TaskFrontMatter{},
},
},
want: map[string]mcp.MCPServerConfig{
"rule-file-1": {Type: mcp.TransportTypeStdio, Command: "server1"},
"rule-file-2": {Type: mcp.TransportTypeStdio, Command: "server2"},
},
},
{
Expand All @@ -114,25 +142,29 @@ func TestResult_MCPServers(t *testing.T) {
Rules: []markdown.Markdown[markdown.RuleFrontMatter]{
{
FrontMatter: markdown.RuleFrontMatter{
ID: "server1-id",
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeStdio, Command: "server1"},
},
},
{
FrontMatter: markdown.RuleFrontMatter{
ID: "server2-id",
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeStdio, Command: "server2"},
},
},
{
FrontMatter: markdown.RuleFrontMatter{},
FrontMatter: markdown.RuleFrontMatter{
ID: "empty-rule",
},
},
},
Task: markdown.Markdown[markdown.TaskFrontMatter]{
FrontMatter: markdown.TaskFrontMatter{},
},
},
want: []mcp.MCPServerConfig{
{Type: mcp.TransportTypeStdio, Command: "server1"},
{Type: mcp.TransportTypeStdio, Command: "server2"},
want: map[string]mcp.MCPServerConfig{
"server1-id": {Type: mcp.TransportTypeStdio, Command: "server1"},
"server2-id": {Type: mcp.TransportTypeStdio, Command: "server2"},
// Empty rule MCP server is filtered out
},
},
Expand All @@ -141,17 +173,52 @@ func TestResult_MCPServers(t *testing.T) {
result: Result{
Rules: []markdown.Markdown[markdown.RuleFrontMatter]{
{
FrontMatter: markdown.RuleFrontMatter{},
FrontMatter: markdown.RuleFrontMatter{
ID: "no-server-rule",
},
},
},
Task: markdown.Markdown[markdown.TaskFrontMatter]{
FrontMatter: markdown.TaskFrontMatter{},
},
},
want: []mcp.MCPServerConfig{
want: map[string]mcp.MCPServerConfig{
// Empty rule MCP server is filtered out
},
},
{
name: "mixed rules with explicit and auto-generated IDs",
result: Result{
Rules: []markdown.Markdown[markdown.RuleFrontMatter]{
{
FrontMatter: markdown.RuleFrontMatter{
ID: "explicit-id", // Explicit ID in frontmatter
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeStdio, Command: "server1"},
},
},
{
FrontMatter: markdown.RuleFrontMatter{
ID: "some-rule", // ID auto-set to filename during loading
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeStdio, Command: "server2"},
},
},
{
FrontMatter: markdown.RuleFrontMatter{
ID: "another-id", // Explicit ID in frontmatter
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeHTTP, URL: "https://example.com"},
},
},
},
Task: markdown.Markdown[markdown.TaskFrontMatter]{
FrontMatter: markdown.TaskFrontMatter{},
},
},
want: map[string]mcp.MCPServerConfig{
"explicit-id": {Type: mcp.TransportTypeStdio, Command: "server1"},
"some-rule": {Type: mcp.TransportTypeStdio, Command: "server2"},
"another-id": {Type: mcp.TransportTypeHTTP, URL: "https://example.com"},
},
},
}

for _, tt := range tests {
Expand All @@ -160,22 +227,37 @@ func TestResult_MCPServers(t *testing.T) {

if len(got) != len(tt.want) {
t.Errorf("MCPServers() returned %d servers, want %d", len(got), len(tt.want))
t.Logf("Got keys: %v", mapKeys(got))
t.Logf("Want keys: %v", mapKeys(tt.want))
return
}

for i, wantServer := range tt.want {
gotServer := got[i]
for key, wantServer := range tt.want {
gotServer, ok := got[key]
if !ok {
t.Errorf("MCPServers() missing key %q", key)
continue
}

if gotServer.Type != wantServer.Type {
t.Errorf("MCPServers()[%d].Type = %v, want %v", i, gotServer.Type, wantServer.Type)
t.Errorf("MCPServers()[%q].Type = %v, want %v", key, gotServer.Type, wantServer.Type)
}
if gotServer.Command != wantServer.Command {
t.Errorf("MCPServers()[%d].Command = %q, want %q", i, gotServer.Command, wantServer.Command)
t.Errorf("MCPServers()[%q].Command = %q, want %q", key, gotServer.Command, wantServer.Command)
}
if gotServer.URL != wantServer.URL {
t.Errorf("MCPServers()[%d].URL = %q, want %q", i, gotServer.URL, wantServer.URL)
t.Errorf("MCPServers()[%q].URL = %q, want %q", key, gotServer.URL, wantServer.URL)
}
}
})
}
}

// Helper function to get map keys for debugging
func mapKeys(m map[string]mcp.MCPServerConfig) []string {
keys := make([]string, 0, len(m))
for k := range m {
keys = append(keys, k)
}
return keys
}
Loading