Skip to content

Remove URN support from frontmatter#208

Merged
alexec merged 2 commits intomainfrom
copilot/remove-urn-support-fontmatter
Feb 27, 2026
Merged

Remove URN support from frontmatter#208
alexec merged 2 commits intomainfrom
copilot/remove-urn-support-fontmatter

Conversation

Copy link
Contributor

Copilot AI commented Feb 27, 2026

The BaseFrontMatter struct carried a URN *urn.URN field (with id as a YAML alias) that added the github.com/leodido/go-urn dependency but provided no functional value — file path is sufficient as an identifier.

Changes

  • frontmatter.go: Remove URN field from BaseFrontMatter and baseFrontMatterRaw; remove id→URN alias parsing from UnmarshalYAML. The method itself is retained (simplified) to work around a yaml.v3 limitation with doubly-inlined structs not populating inline map[string]any fields.
  • go.mod / go.sum: Drop github.com/leodido/go-urn dependency.
  • Tests: Remove urn_test.go helper file and all URN-related test cases/assertions across frontmatter_*_test.go, result_test.go, and context_test.go. Files with id: urn:... in frontmatter continue to parse cleanly — id is now captured as a regular inline content key.

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Copilot AI changed the title [WIP] Remove support for URN in fontmatter Remove URN support from frontmatter Feb 27, 2026
@alexec alexec marked this pull request as ready for review February 27, 2026 15:57
Copilot AI review requested due to automatic review settings February 27, 2026 15:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Removes URN parsing/support from markdown frontmatter to drop the github.com/leodido/go-urn dependency and rely on existing identifiers (e.g., filename-derived Name / inline Content) instead.

Changes:

  • Removed URN field and id→URN alias parsing from BaseFrontMatter YAML unmarshalling.
  • Dropped github.com/leodido/go-urn from go.mod/go.sum and removed URN helper test file.
  • Updated tests to stop asserting URN behavior and to treat id as an inline Content key.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
pkg/codingcontext/markdown/frontmatter.go Removes URN fields/parsing; keeps custom UnmarshalYAML to reliably populate inline Content.
go.mod Drops github.com/leodido/go-urn requirement.
go.sum Removes go-urn checksums.
pkg/codingcontext/markdown/urn_test.go Deletes URN test helpers.
pkg/codingcontext/markdown/frontmatter_task_test.go Removes URN expectations; updates unmarshal expected Content to include id.
pkg/codingcontext/markdown/frontmatter_rule_test.go Removes URN expectations; updates unmarshal expected Content to include id.
pkg/codingcontext/markdown/frontmatter_command_test.go Removes URN expectations; updates unmarshal expected Content to include id.
pkg/codingcontext/result_test.go Removes URN usage in MCP server tests (now constructs rules without URNs).
pkg/codingcontext/context_test.go Removes URN-related integration assertions around tasks/rules.
Comments suppressed due to low confidence (2)

pkg/codingcontext/markdown/frontmatter_rule_test.go:162

  • This test now sets want.BaseFrontMatter.Content (including id), but the assertions never compare got.Content to want.Content. Add a check for the captured inline fields (at least id) so the new behavior is covered.
			// Compare fields individually
			if got.Name != tt.want.Name {
				t.Errorf("Name = %q, want %q", got.Name, tt.want.Name)
			}
			if got.Description != tt.want.Description {
				t.Errorf("Description = %q, want %q", got.Description, tt.want.Description)
			}
			if got.Agent != tt.want.Agent {
				t.Errorf("Agent = %q, want %q", got.Agent, tt.want.Agent)
			}

pkg/codingcontext/markdown/frontmatter_command_test.go:160

  • The test cases populate want.BaseFrontMatter.Content with id, but the assertions only compare typed fields and never verify got.Content. Add an assertion for got.Content["id"] (or compare the full Content map) to ensure the inline capture behavior is covered.
			// Compare fields individually
			if got.Name != tt.want.Name {
				t.Errorf("Name = %q, want %q", got.Name, tt.want.Name)
			}
			if got.Description != tt.want.Description {
				t.Errorf("Description = %q, want %q", got.Description, tt.want.Description)
			}
			if tt.want.ExpandParams != nil {
				if (got.ExpandParams == nil) != (tt.want.ExpandParams == nil) {
					t.Errorf("ExpandParams nil mismatch: got %v, want %v", got.ExpandParams == nil, tt.want.ExpandParams == nil)
				} else if got.ExpandParams != nil && tt.want.ExpandParams != nil {
					if *got.ExpandParams != *tt.want.ExpandParams {
						t.Errorf("ExpandParams = %v, want %v", *got.ExpandParams, *tt.want.ExpandParams)
					}
				}
			}
		})

Comment on lines 184 to 189
// Compare fields individually for better error messages
gotTaskName, _ := got.Content["task_name"].(string)
wantTaskName, _ := tt.want.Content["task_name"].(string)
if gotTaskName != wantTaskName {
t.Errorf("TaskName = %q, want %q", gotTaskName, wantTaskName)
}
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

The updated expectations add id into BaseFrontMatter.Content, but the test assertions only check task_name and typed fields. Add an assertion for got.Content["id"] (or compare the full Content map) so the behavior “id is captured as inline content” is actually validated.

Copilot generated this review using guidance from repository custom instructions.
Comment on lines 97 to 105
{
FrontMatter: markdown.RuleFrontMatter{
BaseFrontMatter: markdown.BaseFrontMatter{URN: mustParseURN("urn:agents:rule:jira-server")},
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeStdio, Command: "jira"},
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeStdio, Command: "jira"},
},
},
{
FrontMatter: markdown.RuleFrontMatter{
BaseFrontMatter: markdown.BaseFrontMatter{URN: mustParseURN("urn:agents:rule:api-server")},
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeHTTP, URL: "https://api.example.com"},
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeHTTP, URL: "https://api.example.com"},
},
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

These rules don’t set FrontMatter.Name, but Result.MCPServers() uses rule.FrontMatter.Name as the map key. With empty names, later entries overwrite earlier ones and the test won’t catch regressions around handling multiple servers; set distinct rule names here (or mirror Context’s filename->Name defaulting) and assert all expected entries are present.

Copilot generated this review using guidance from repository custom instructions.
Comment on lines 121 to 130
{
FrontMatter: markdown.RuleFrontMatter{
BaseFrontMatter: markdown.BaseFrontMatter{URN: mustParseURN("urn:agents:rule:server1")},
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeStdio, Command: "server1"},
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeStdio, Command: "server1"},
},
},
{
FrontMatter: markdown.RuleFrontMatter{
BaseFrontMatter: markdown.BaseFrontMatter{URN: mustParseURN("urn:agents:rule:server2")},
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeStdio, Command: "server2"},
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeStdio, Command: "server2"},
},
},
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

These rules are constructed without FrontMatter.Name, which causes key collisions in Result.MCPServers() (it indexes by FrontMatter.Name). Give each rule a distinct Name (or simulate Context’s defaulting) so this test actually validates behavior with multiple MCP servers instead of exercising overwrite-by-empty-key.

Copilot generated this review using guidance from repository custom instructions.
Comment on lines 165 to 177
FrontMatter: markdown.RuleFrontMatter{
BaseFrontMatter: markdown.BaseFrontMatter{URN: mustParseURN("urn:agents:rule:explicit")},
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeStdio, Command: "server1"},
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeStdio, Command: "server1"},
},
},
{
FrontMatter: markdown.RuleFrontMatter{
BaseFrontMatter: markdown.BaseFrontMatter{URN: mustParseURN("urn:agents:rule:some-rule")},
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeStdio, Command: "server2"},
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeStdio, Command: "server2"},
},
},
{
FrontMatter: markdown.RuleFrontMatter{
BaseFrontMatter: markdown.BaseFrontMatter{URN: mustParseURN("urn:agents:rule:another")},
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeHTTP, URL: "https://example.com"},
MCPServer: mcp.MCPServerConfig{Type: mcp.TransportTypeHTTP, URL: "https://example.com"},
},
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

All rules in this case omit FrontMatter.Name, but Result.MCPServers() uses Name as the map key; this makes the last MCP server silently win. Set distinct names on each RuleFrontMatter (or mirror the filename defaulting) and update the expected map so the test covers multiple returned servers.

Copilot generated this review using guidance from repository custom instructions.
@alexec alexec merged commit 3a9b6eb into main Feb 27, 2026
5 checks passed
@alexec alexec deleted the copilot/remove-urn-support-fontmatter branch February 27, 2026 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants