diff --git a/forge-core/mcp/annotations_test.go b/forge-core/mcp/annotations_test.go new file mode 100644 index 00000000..7145b3ba --- /dev/null +++ b/forge-core/mcp/annotations_test.go @@ -0,0 +1,30 @@ +package mcp + +import ( + "encoding/json" + "testing" +) + +// tools/list responses may carry the MCP annotations object; decode must +// surface it (platform discovery seeds side-effect classes from these hints) +// and its absence must stay distinguishable from explicit false. +func TestMCPToolDescriptor_AnnotationsDecode(t *testing.T) { + raw := []byte(`[ + {"name":"list_issues","inputSchema":{},"annotations":{"readOnlyHint":true}}, + {"name":"delete_issue","inputSchema":{},"annotations":{"destructiveHint":true,"readOnlyHint":false}}, + {"name":"create_issue","inputSchema":{}} + ]`) + var descs []MCPToolDescriptor + if err := json.Unmarshal(raw, &descs); err != nil { + t.Fatalf("decode: %v", err) + } + if descs[0].Annotations == nil || descs[0].Annotations.ReadOnlyHint == nil || !*descs[0].Annotations.ReadOnlyHint { + t.Fatalf("readOnlyHint lost: %+v", descs[0].Annotations) + } + if descs[1].Annotations == nil || descs[1].Annotations.DestructiveHint == nil || !*descs[1].Annotations.DestructiveHint { + t.Fatalf("destructiveHint lost: %+v", descs[1].Annotations) + } + if descs[2].Annotations != nil { + t.Fatalf("absent annotations must stay nil, got %+v", descs[2].Annotations) + } +} diff --git a/forge-core/mcp/types.go b/forge-core/mcp/types.go index 56169cc3..02d0e7d9 100644 --- a/forge-core/mcp/types.go +++ b/forge-core/mcp/types.go @@ -106,4 +106,17 @@ type MCPToolDescriptor struct { Name string `json:"name"` Description string `json:"description,omitempty"` InputSchema json.RawMessage `json:"inputSchema"` + // Annotations are the MCP spec's optional tool behavior hints + // (readOnlyHint / destructiveHint / idempotentHint). Advisory metadata + // from the server — surfaced so platform-side discovery can seed + // side-effect classifications; the runtime does not act on them. + Annotations *MCPToolAnnotations `json:"annotations,omitempty"` +} + +// MCPToolAnnotations mirrors the MCP tool annotations object. Pointers so +// "absent" and "false" stay distinguishable. +type MCPToolAnnotations struct { + ReadOnlyHint *bool `json:"readOnlyHint,omitempty"` + DestructiveHint *bool `json:"destructiveHint,omitempty"` + IdempotentHint *bool `json:"idempotentHint,omitempty"` } diff --git a/forge-skills/contract/types.go b/forge-skills/contract/types.go index 855bcac9..65e434ee 100644 --- a/forge-skills/contract/types.go +++ b/forge-skills/contract/types.go @@ -70,8 +70,16 @@ type ForgeSkillMeta struct { // parent agent's `tool.` span via TRACEPARENT env propagation // (issue #182). Wrapping the binary in a bash skill works too, but // adds a fork and a wrapper to maintain. - Runtime string `yaml:"runtime,omitempty" json:"runtime,omitempty"` - Requires *SkillRequirements `yaml:"requires,omitempty" json:"requires,omitempty"` + Runtime string `yaml:"runtime,omitempty" json:"runtime,omitempty"` + Requires *SkillRequirements `yaml:"requires,omitempty" json:"requires,omitempty"` + // Uses declares the skill's governed tool dependencies — references into + // the PLATFORM tool registry (the org-scoped catalog of admitted tools), + // chosen as binary or MCP with a selected operation subset. Additive and + // DECLARATIVE from forge's perspective: the platform resolves and + // validates refs at authoring/build time and materializes the runtime + // wiring (mcp servers, allowlists); forge parses the block so the + // declaration travels with the skill, and does not gate on it. + Uses []SkillToolDependency `yaml:"uses,omitempty" json:"uses,omitempty"` EgressDomains []string `yaml:"egress_domains,omitempty" json:"egress_domains,omitempty"` DeniedTools []string `yaml:"denied_tools,omitempty" json:"denied_tools,omitempty"` WorkflowPhase string `yaml:"workflow_phase,omitempty" json:"workflow_phase,omitempty"` @@ -171,6 +179,15 @@ func (b *BinRequirement) UnmarshalYAML(value *yaml.Node) error { } } +// SkillToolDependency is one governed tool dependency: a reference to a +// platform tool-registry entry (by its stable key) with the operations this +// skill selects. Type distinguishes binary vs MCP registry entries. +type SkillToolDependency struct { + Type string `yaml:"type" json:"type"` // binary | mcp + Ref string `yaml:"ref" json:"ref"` // registry entry key, e.g. "mcp.linear" + Operations []string `yaml:"operations,omitempty" json:"operations,omitempty"` +} + // SkillRequirements declares CLI binaries, environment variables, and runtime // capabilities a skill needs. type SkillRequirements struct { diff --git a/forge-skills/parser/parser.go b/forge-skills/parser/parser.go index 4d02638a..d2650873 100644 --- a/forge-skills/parser/parser.go +++ b/forge-skills/parser/parser.go @@ -262,6 +262,18 @@ func ExtractForgeMeta(meta *contract.SkillMetadata) *contract.ForgeSkillMeta { return &forgeMeta } +// ExtractForgeUses extracts the skill's governed tool dependencies +// (metadata.forge.uses — references into the platform tool registry). Thin +// wrapper over ExtractForgeMeta, mirroring ExtractForgeReqs. Returns nil when +// the skill declares none — the pre-registry common case. +func ExtractForgeUses(meta *contract.SkillMetadata) []contract.SkillToolDependency { + fm := ExtractForgeMeta(meta) + if fm == nil { + return nil + } + return fm.Uses +} + // ExtractForgeReqs extracts SkillRequirements, egress_domains, and guardrails from the generic // metadata map. Thin wrapper over ExtractForgeMeta for existing call sites. func ExtractForgeReqs(meta *contract.SkillMetadata) (*contract.SkillRequirements, []string, *contract.SkillGuardrailConfig) { diff --git a/forge-skills/parser/uses_test.go b/forge-skills/parser/uses_test.go new file mode 100644 index 00000000..edf4d212 --- /dev/null +++ b/forge-skills/parser/uses_test.go @@ -0,0 +1,59 @@ +package parser + +import ( + "strings" + "testing" +) + +const skillWithUses = `--- +name: linear-triage +description: Triage Linear issues +metadata: + forge: + requires: + bins: [jq] + uses: + - type: mcp + ref: mcp.linear + operations: [create_issue, list_issues] + - type: binary + ref: bin.jq + operations: [exec] +--- +# Linear triage +` + +func TestExtractForgeUses(t *testing.T) { + _, meta, err := ParseWithMetadata(strings.NewReader(skillWithUses)) + if err != nil { + t.Fatalf("parse: %v", err) + } + uses := ExtractForgeUses(meta) + if len(uses) != 2 { + t.Fatalf("want 2 deps, got %d: %+v", len(uses), uses) + } + if uses[0].Type != "mcp" || uses[0].Ref != "mcp.linear" || len(uses[0].Operations) != 2 { + t.Fatalf("mcp dep mismatch: %+v", uses[0]) + } + if uses[1].Type != "binary" || uses[1].Ref != "bin.jq" { + t.Fatalf("binary dep mismatch: %+v", uses[1]) + } + // requires.bins still parses beside it — the legacy surface is untouched. + reqs, _, _ := ExtractForgeReqs(meta) + if reqs == nil || len(reqs.Bins) != 1 || reqs.Bins[0].Name != "jq" { + t.Fatalf("requires.bins lost: %+v", reqs) + } +} + +// A skill with no uses block — the universal pre-registry case — must parse +// exactly as before, with ExtractForgeUses returning nil. +func TestExtractForgeUses_AbsentIsNil(t *testing.T) { + const legacy = "---\nname: x\nmetadata:\n forge:\n requires:\n bins: [curl]\n---\nbody\n" + _, meta, err := ParseWithMetadata(strings.NewReader(legacy)) + if err != nil { + t.Fatalf("parse: %v", err) + } + if uses := ExtractForgeUses(meta); uses != nil { + t.Fatalf("want nil, got %+v", uses) + } +}