From 9d8d97ca528811f0322f7e5e7ae6710866003a2a Mon Sep 17 00:00:00 2001 From: MK Date: Tue, 14 Jul 2026 14:42:57 -0400 Subject: [PATCH 1/2] =?UTF-8?q?feat(skills,mcp):=20platform=20tool-registr?= =?UTF-8?q?y=20contract=20=E2=80=94=20uses=20block=20+=20tool=20annotation?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two additive pieces the PLATFORM tool registry needs from forge (the registry itself lives on the platform; forge stays the runtime): - metadata.forge.uses — a skill's governed tool dependencies: references into the org's tool-registry (binary|mcp, entry key, selected operations). DECLARATIVE from forge's perspective: the platform resolves/validates refs at authoring/build time and materializes the runtime wiring (mcp servers + tool allowlists); forge parses the block so the declaration travels with the skill and never gates on it. Skills without the block parse byte-identically. - MCPToolDescriptor.Annotations — the MCP spec's readOnlyHint / destructiveHint / idempotentHint tool hints, surfaced from tools/list so platform-side discovery can seed side-effect classifications. Pointers keep absent vs explicit-false distinguishable; the runtime does not act on them. Co-Authored-By: Claude Fable 5 --- forge-core/mcp/annotations_test.go | 30 +++++++++++++++ forge-core/mcp/types.go | 13 +++++++ forge-skills/contract/types.go | 21 ++++++++++- forge-skills/parser/parser.go | 12 ++++++ forge-skills/parser/uses_test.go | 59 ++++++++++++++++++++++++++++++ 5 files changed, 133 insertions(+), 2 deletions(-) create mode 100644 forge-core/mcp/annotations_test.go create mode 100644 forge-skills/parser/uses_test.go 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..5dddb02e 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"` @@ -173,6 +181,15 @@ func (b *BinRequirement) UnmarshalYAML(value *yaml.Node) error { // SkillRequirements declares CLI binaries, environment variables, and runtime // capabilities a skill needs. +// 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"` +} + type SkillRequirements struct { Bins []BinRequirement `yaml:"bins,omitempty" json:"bins,omitempty"` Env *EnvRequirements `yaml:"env,omitempty" json:"env,omitempty"` 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) + } +} From aa4713159b0f9d6d9c8dc0fe5c6091bc4d4652a8 Mon Sep 17 00:00:00 2001 From: MK Date: Tue, 14 Jul 2026 17:42:35 -0400 Subject: [PATCH 2/2] fix(contract): relocate SkillToolDependency above SkillRequirements' godoc The insertion sat between SkillRequirements' doc comment and its type keyword, merging the comments onto the wrong type and leaving SkillRequirements undocumented (review #309). Co-Authored-By: Claude Fable 5 --- forge-skills/contract/types.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/forge-skills/contract/types.go b/forge-skills/contract/types.go index 5dddb02e..65e434ee 100644 --- a/forge-skills/contract/types.go +++ b/forge-skills/contract/types.go @@ -179,8 +179,6 @@ func (b *BinRequirement) UnmarshalYAML(value *yaml.Node) error { } } -// SkillRequirements declares CLI binaries, environment variables, and runtime -// capabilities a skill needs. // 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. @@ -190,6 +188,8 @@ type SkillToolDependency struct { Operations []string `yaml:"operations,omitempty" json:"operations,omitempty"` } +// SkillRequirements declares CLI binaries, environment variables, and runtime +// capabilities a skill needs. type SkillRequirements struct { Bins []BinRequirement `yaml:"bins,omitempty" json:"bins,omitempty"` Env *EnvRequirements `yaml:"env,omitempty" json:"env,omitempty"`