From e2594f45f2ead6a013c60d4c9a66ae72fc0423a9 Mon Sep 17 00:00:00 2001 From: Mathias Date: Wed, 10 Jun 2026 09:52:29 +0200 Subject: [PATCH] refactor(tools): extract RegisterAll shared tool registration main.go and tests now share one registration list so a tool wired in one place cannot silently go missing from the other. Adds a dispatch round-trip test asserting every registered tool resolves and ships a parseable schema. Co-Authored-By: Claude Opus 4.8 (1M context) --- cmd/gitea-mcp/main.go | 39 +--------------------- internal/tools/registry.go | 59 +++++++++++++++++++++++++++++++++ internal/tools/registry_test.go | 58 ++++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+), 38 deletions(-) create mode 100644 internal/tools/registry.go create mode 100644 internal/tools/registry_test.go diff --git a/cmd/gitea-mcp/main.go b/cmd/gitea-mcp/main.go index fb42eb5..38586b0 100644 --- a/cmd/gitea-mcp/main.go +++ b/cmd/gitea-mcp/main.go @@ -38,44 +38,7 @@ func main() { ownerAllow := allowlist.New(cfg.AllowedOwners) reg := registry.New() - reg.Register(tools.NewRepoList(giteaClient, ownerAllow)) - reg.Register(tools.NewRepoGet(giteaClient, ownerAllow)) - reg.Register(tools.NewRepoSearch(giteaClient, ownerAllow)) - reg.Register(tools.NewRepoStatus(giteaClient, ownerAllow)) - reg.Register(tools.NewFileRead(giteaClient, ownerAllow)) - reg.Register(tools.NewFileWriteBranch(giteaClient, ownerAllow)) - reg.Register(tools.NewFileDelete(giteaClient, ownerAllow)) - reg.Register(tools.NewDirList(giteaClient, ownerAllow)) - reg.Register(tools.NewBranchList(giteaClient, ownerAllow)) - reg.Register(tools.NewBranchDelete(giteaClient, ownerAllow)) - reg.Register(tools.NewBranchProtectionGet(giteaClient, ownerAllow)) - reg.Register(tools.NewPRCreate(giteaClient, ownerAllow)) - reg.Register(tools.NewPRGet(giteaClient, ownerAllow)) - reg.Register(tools.NewPRList(giteaClient, ownerAllow)) - reg.Register(tools.NewPRMerge(giteaClient, ownerAllow)) - reg.Register(tools.NewPRComment(giteaClient, ownerAllow)) - reg.Register(tools.NewPRFilesDiff(giteaClient, ownerAllow)) - reg.Register(tools.NewWorkflowRunTrigger(giteaClient, ownerAllow, cfg.GiteaBaseURL)) - reg.Register(tools.NewWorkflowRunStatus(giteaClient, ownerAllow)) - reg.Register(tools.NewCodeSearch(giteaClient, ownerAllow)) - reg.Register(tools.NewIssueCreate(giteaClient, ownerAllow)) - reg.Register(tools.NewIssueEdit(giteaClient, ownerAllow)) - reg.Register(tools.NewIssueComment(giteaClient, ownerAllow)) - reg.Register(tools.NewCreateProjectFromTemplate(giteaClient, ownerAllow, "mathias", "template-go-web")) - reg.Register(tools.NewTagCreate(giteaClient, ownerAllow)) - reg.Register(tools.NewRepoCreate(giteaClient, ownerAllow)) - reg.Register(tools.NewRepoUpdate(giteaClient, ownerAllow)) - reg.Register(tools.NewRepoMirrorPush(giteaClient, ownerAllow)) - reg.Register(tools.NewRepoTree(giteaClient, ownerAllow)) - reg.Register(tools.NewRepoTopicsUpdate(giteaClient, ownerAllow)) - reg.Register(tools.NewIssueGet(giteaClient, ownerAllow)) - reg.Register(tools.NewIssueList(giteaClient, ownerAllow)) - reg.Register(tools.NewIssueListComments(giteaClient, ownerAllow)) - reg.Register(tools.NewIssueClose(giteaClient, ownerAllow)) - reg.Register(tools.NewIssueReopen(giteaClient, ownerAllow)) - reg.Register(tools.NewWorkflowRunList(giteaClient, ownerAllow)) - reg.Register(tools.NewReleaseCreate(giteaClient, ownerAllow)) - reg.Register(tools.NewRepoDelete(giteaClient, ownerAllow)) + tools.RegisterAll(reg, giteaClient, ownerAllow, cfg.GiteaBaseURL, "mathias", "template-go-web") mcpSrv := mcp.NewServer(mcp.ServerOptions{ Registry: reg, diff --git a/internal/tools/registry.go b/internal/tools/registry.go new file mode 100644 index 0000000..a2c667d --- /dev/null +++ b/internal/tools/registry.go @@ -0,0 +1,59 @@ +package tools + +import ( + "gitea.d-ma.be/mathias/gitea-mcp/internal/allowlist" + "gitea.d-ma.be/mathias/gitea-mcp/internal/gitea" + "gitea.d-ma.be/mathias/gitea-mcp/internal/registry" +) + +// RegisterAll registers every gitea-mcp tool on reg. main.go and the +// dispatch round-trip test share this single list so a newly added tool +// cannot be wired in one place but missing from the other. +// +// giteaBaseURL is needed by workflow_run_trigger; tmplOwner/tmplRepo seed +// create_project_from_template's default source template. +func RegisterAll( + reg *registry.Registry, + c *gitea.Client, + a *allowlist.Allowlist, + giteaBaseURL, tmplOwner, tmplRepo string, +) { + reg.Register(NewRepoList(c, a)) + reg.Register(NewRepoGet(c, a)) + reg.Register(NewRepoSearch(c, a)) + reg.Register(NewRepoStatus(c, a)) + reg.Register(NewFileRead(c, a)) + reg.Register(NewFileWriteBranch(c, a)) + reg.Register(NewFileDelete(c, a)) + reg.Register(NewDirList(c, a)) + reg.Register(NewBranchList(c, a)) + reg.Register(NewBranchDelete(c, a)) + reg.Register(NewBranchProtectionGet(c, a)) + reg.Register(NewPRCreate(c, a)) + reg.Register(NewPRGet(c, a)) + reg.Register(NewPRList(c, a)) + reg.Register(NewPRMerge(c, a)) + reg.Register(NewPRComment(c, a)) + reg.Register(NewPRFilesDiff(c, a)) + reg.Register(NewWorkflowRunTrigger(c, a, giteaBaseURL)) + reg.Register(NewWorkflowRunStatus(c, a)) + reg.Register(NewCodeSearch(c, a)) + reg.Register(NewIssueCreate(c, a)) + reg.Register(NewIssueEdit(c, a)) + reg.Register(NewIssueComment(c, a)) + reg.Register(NewCreateProjectFromTemplate(c, a, tmplOwner, tmplRepo)) + reg.Register(NewTagCreate(c, a)) + reg.Register(NewRepoCreate(c, a)) + reg.Register(NewRepoUpdate(c, a)) + reg.Register(NewRepoMirrorPush(c, a)) + reg.Register(NewRepoTree(c, a)) + reg.Register(NewRepoTopicsUpdate(c, a)) + reg.Register(NewIssueGet(c, a)) + reg.Register(NewIssueList(c, a)) + reg.Register(NewIssueListComments(c, a)) + reg.Register(NewIssueClose(c, a)) + reg.Register(NewIssueReopen(c, a)) + reg.Register(NewWorkflowRunList(c, a)) + reg.Register(NewReleaseCreate(c, a)) + reg.Register(NewRepoDelete(c, a)) +} diff --git a/internal/tools/registry_test.go b/internal/tools/registry_test.go new file mode 100644 index 0000000..e387dea --- /dev/null +++ b/internal/tools/registry_test.go @@ -0,0 +1,58 @@ +package tools_test + +import ( + "context" + "encoding/json" + "errors" + "testing" + + "gitea.d-ma.be/mathias/gitea-mcp/internal/allowlist" + "gitea.d-ma.be/mathias/gitea-mcp/internal/gitea" + "gitea.d-ma.be/mathias/gitea-mcp/internal/registry" + "gitea.d-ma.be/mathias/gitea-mcp/internal/tools" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// buildRegistry wires the full tool set exactly as main.go does, against an +// unroutable gitea base URL so any handler that reaches the network fails fast +// (connection refused) rather than hanging. +func buildRegistry() *registry.Registry { + reg := registry.New() + c := gitea.NewClient("http://127.0.0.1:1", "") + a := allowlist.New([]string{"mathias"}) + tools.RegisterAll(reg, c, a, "http://127.0.0.1:1", "mathias", "template-go-web") + return reg +} + +// Every registered tool must be dispatchable and advertise a parseable input +// schema. This is the regression guard for #36's whole class: a tool that is +// wired up but unroutable (or ships a malformed schema) fails CI here instead +// of 404ing a live caller. +func TestEveryRegisteredToolIsDispatchable(t *testing.T) { + reg := buildRegistry() + descs := reg.Tools() + require.NotEmpty(t, descs) + + for _, d := range descs { + t.Run(d.Name, func(t *testing.T) { + require.NotEmpty(t, d.Name, "tool has empty name") + assert.True(t, json.Valid(d.InputSchema), + "tool %q ships invalid JSON input schema", d.Name) + + // Dispatch with empty args. We do not care whether the call + // succeeds (most fail allowlist/validation/network) — only that + // the name resolves to a handler. ErrToolNotFound here means the + // tool advertised a name the dispatcher cannot route. + _, err := reg.Dispatch(context.Background(), d.Name, json.RawMessage(`{}`)) + assert.False(t, errors.Is(err, registry.ErrToolNotFound), + "registered tool %q does not dispatch", d.Name) + }) + } +} + +// Lock the tool count so an accidental drop of a registration in RegisterAll +// (the single source main.go and this test share) fails loudly. +func TestRegisteredToolCount(t *testing.T) { + assert.Len(t, buildRegistry().Tools(), 38) +}