Files
gitea-mcp/internal/tools/issue_label_test.go
T
mathiasandClaude Opus 4.8 ac337955e7
CD / Lint / Test / Vet (push) Successful in 7s
CD / Build & Import (push) Successful in 21s
CD / Deploy via GitOps (push) Has been skipped
fix(issue_label): schema wrongly required labels, blocking label_ids-only callers (#52 review finding)
Independent adversarial review of #52 (v0.8.0) caught a schema/implementation
mismatch: the advertised InputSchema marked "labels" as required, but Call
already treated labels/label_ids as either-or. An MCP client that validates
arguments against the advertised schema before dispatch would reject a
label_ids-only call as invalid even though the code was written to serve it —
and that path had zero test coverage either way.

Dropped "labels" from the required array (owner/repo/number remain required);
runtime validation already correctly requires at least one of labels/label_ids.
Added TestIssueLabelAppliesByIDOnly (asserts ListLabels is never called when
IDs are already known) and a schema-lock test for the fixed contract.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-04 13:57:05 +02:00

129 lines
5.1 KiB
Go

package tools_test
import (
"context"
"encoding/json"
"io"
"net/http"
"net/http/httptest"
"testing"
"git.d-ma.be/mathias/gitea-mcp/internal/allowlist"
"git.d-ma.be/mathias/gitea-mcp/internal/gitea"
"git.d-ma.be/mathias/gitea-mcp/internal/tools"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
const labelListFixture = `[
{"id":1,"name":"bug","color":"ee0701"},
{"id":2,"name":"enhancement","color":"84b6eb"}
]`
func TestIssueLabelAppliesByName(t *testing.T) {
var captured []byte
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch {
case r.Method == http.MethodGet && r.URL.Path == "/api/v1/repos/o/r/labels":
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(labelListFixture))
case r.Method == http.MethodPost && r.URL.Path == "/api/v1/repos/o/r/issues/42/labels":
var err error
captured, err = io.ReadAll(r.Body)
require.NoError(t, err)
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(labelListFixture))
default:
t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
}
}))
defer srv.Close()
tool := tools.NewIssueLabel(gitea.NewClient(srv.URL, "tok"), allowlist.New([]string{"o"}))
out, err := tool.Call(context.Background(), json.RawMessage(`{"owner":"o","repo":"r","number":42,"labels":["bug","enhancement"]}`))
require.NoError(t, err)
var payload map[string]any
require.NoError(t, json.Unmarshal(captured, &payload))
ids, ok := payload["labels"].([]any)
require.True(t, ok)
assert.ElementsMatch(t, []any{float64(1), float64(2)}, ids)
assert.Contains(t, string(out), `"name":"bug"`)
assert.Contains(t, string(out), `"name":"enhancement"`)
}
// label_ids alone (no labels) must work end-to-end without hitting ListLabels
// at all — this is the schema-level "either labels or label_ids" contract, and
// it must never require a GET to the label list when the caller already has IDs.
func TestIssueLabelAppliesByIDOnly(t *testing.T) {
var captured []byte
var listCalled bool
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch {
case r.Method == http.MethodGet && r.URL.Path == "/api/v1/repos/o/r/labels":
listCalled = true
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(labelListFixture))
case r.Method == http.MethodPost && r.URL.Path == "/api/v1/repos/o/r/issues/42/labels":
var err error
captured, err = io.ReadAll(r.Body)
require.NoError(t, err)
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(labelListFixture))
default:
t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
}
}))
defer srv.Close()
tool := tools.NewIssueLabel(gitea.NewClient(srv.URL, "tok"), allowlist.New([]string{"o"}))
out, err := tool.Call(context.Background(), json.RawMessage(`{"owner":"o","repo":"r","number":42,"label_ids":[1,2]}`))
require.NoError(t, err)
assert.False(t, listCalled, "label_ids-only must not call ListLabels")
var payload map[string]any
require.NoError(t, json.Unmarshal(captured, &payload))
ids, ok := payload["labels"].([]any)
require.True(t, ok)
assert.ElementsMatch(t, []any{float64(1), float64(2)}, ids)
assert.Contains(t, string(out), `"name":"bug"`)
}
// #52 review finding: the advertised schema wrongly required "labels", making
// label_ids-only calls fail JSON-Schema validation before reaching Call at all.
// Lock the fixed contract: neither is individually required.
func TestIssueLabelSchema_NeitherLabelsNorLabelIDsRequired(t *testing.T) {
sch := string(tools.NewIssueLabel(gitea.NewClient("http://unused", ""), allowlist.New([]string{"o"})).Descriptor().InputSchema)
assert.NotContains(t, sch, `"required":["owner","repo","number","labels"]`)
assert.Contains(t, sch, `"required":["owner","repo","number"]`)
}
func TestIssueLabelUnknownNameNamesTheMissingLabel(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(labelListFixture))
}))
defer srv.Close()
tool := tools.NewIssueLabel(gitea.NewClient(srv.URL, "tok"), allowlist.New([]string{"o"}))
_, err := tool.Call(context.Background(), json.RawMessage(`{"owner":"o","repo":"r","number":42,"labels":["bug","does-not-exist"]}`))
require.Error(t, err)
assert.ErrorIs(t, err, gitea.ErrValidation)
assert.Contains(t, err.Error(), `"does-not-exist"`)
assert.Contains(t, err.Error(), "o/r")
}
func TestIssueLabelAllowlistRejects(t *testing.T) {
tool := tools.NewIssueLabel(gitea.NewClient("http://unused", ""), allowlist.New([]string{"allowed"}))
_, err := tool.Call(context.Background(), json.RawMessage(`{"owner":"evil","repo":"r","number":1,"labels":["bug"]}`))
require.Error(t, err)
}
func TestIssueLabelRequiresValidNumber(t *testing.T) {
tool := tools.NewIssueLabel(gitea.NewClient("http://unused", ""), allowlist.New([]string{"o"}))
_, err := tool.Call(context.Background(), json.RawMessage(`{"owner":"o","repo":"r","number":0,"labels":["bug"]}`))
require.Error(t, err)
assert.ErrorIs(t, err, gitea.ErrValidation)
}