397 lines
14 KiB
Go
397 lines
14 KiB
Go
package tools_test
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"io"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"strings"
|
|
"sync"
|
|
"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"
|
|
)
|
|
|
|
// pr_merge is gated exactly like tbd_ship (gitea-mcp#74, #63): it merges only when the head commit's CI is fully
|
|
// green, the base is not review-protected AND the issue named in `issue` carries exactly one risk/ label, risk/low.
|
|
// The fake serves the whole read side the gate needs and records whether (and how) the merge endpoint was hit.
|
|
|
|
const (
|
|
greenRuns = `[{"id":1,"status":"completed","conclusion":"success","head_sha":"abc"}]`
|
|
lowLabels = `[{"id":1,"name":"risk/low"}]`
|
|
)
|
|
|
|
type prMergeFakeOpts struct {
|
|
runs string // workflow_runs array for the head commit
|
|
issueLabels string // labels array of issue 42
|
|
issueStatus int
|
|
prStatus int // GET pulls/7
|
|
protectionStatus int // 0 → 404 (unprotected); 200 with required approvals when requiredApprovals>0
|
|
requiredApprovals int
|
|
mergeStatus int
|
|
emptyHeadSha bool // the PR reports head.sha == ""
|
|
prState string // default "open"
|
|
prDraft bool
|
|
}
|
|
|
|
type prMergeFake struct {
|
|
*httptest.Server
|
|
mu sync.Mutex
|
|
mergeCalls int
|
|
mergeBody []byte
|
|
runsQuery string // raw query of the last workflow-runs request
|
|
}
|
|
|
|
func (f *prMergeFake) merged() bool { f.mu.Lock(); defer f.mu.Unlock(); return f.mergeCalls > 0 }
|
|
|
|
func newPRMergeFake(t *testing.T, o prMergeFakeOpts) *prMergeFake {
|
|
t.Helper()
|
|
if o.runs == "" {
|
|
o.runs = greenRuns
|
|
}
|
|
if o.issueLabels == "" {
|
|
o.issueLabels = lowLabels
|
|
}
|
|
if o.issueStatus == 0 {
|
|
o.issueStatus = http.StatusOK
|
|
}
|
|
if o.prStatus == 0 {
|
|
o.prStatus = http.StatusOK
|
|
}
|
|
if o.mergeStatus == 0 {
|
|
o.mergeStatus = http.StatusNoContent
|
|
}
|
|
if o.prState == "" {
|
|
o.prState = "open"
|
|
}
|
|
sha := "abc"
|
|
if o.emptyHeadSha {
|
|
sha = ""
|
|
}
|
|
f := &prMergeFake{}
|
|
f.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
p := r.URL.Path
|
|
w.Header().Set("Content-Type", "application/json")
|
|
switch {
|
|
case r.Method == http.MethodGet && p == "/api/v1/repos/owner/repo/pulls/7":
|
|
w.WriteHeader(o.prStatus)
|
|
_, _ = w.Write([]byte(`{"number":7,"title":"t","state":"` + o.prState + `","draft":` + boolJSON(o.prDraft) + `,"html_url":"http://x/pulls/7","head":{"ref":"feat","sha":"` + sha + `"},"base":{"ref":"main"}}`))
|
|
case r.Method == http.MethodGet && strings.Contains(p, "/branch_protections/"):
|
|
switch {
|
|
case o.protectionStatus != 0:
|
|
w.WriteHeader(o.protectionStatus)
|
|
_, _ = w.Write([]byte(`{"message":"boom"}`))
|
|
case o.requiredApprovals > 0:
|
|
_, _ = w.Write([]byte(`{"required_approvals":` + itoa(o.requiredApprovals) + `}`))
|
|
default:
|
|
w.WriteHeader(http.StatusNotFound)
|
|
_, _ = w.Write([]byte(`{"message":"not found"}`))
|
|
}
|
|
case r.Method == http.MethodGet && strings.Contains(p, "/actions/runs"):
|
|
f.mu.Lock()
|
|
f.runsQuery = r.URL.RawQuery
|
|
f.mu.Unlock()
|
|
_, _ = w.Write([]byte(`{"total_count":0,"workflow_runs":` + o.runs + `}`))
|
|
case r.Method == http.MethodGet && p == "/api/v1/repos/owner/repo/issues/42":
|
|
w.WriteHeader(o.issueStatus)
|
|
_, _ = w.Write([]byte(`{"number":42,"title":"t","state":"open","labels":` + o.issueLabels + `}`))
|
|
case r.Method == http.MethodPost && p == "/api/v1/repos/owner/repo/pulls/7/merge":
|
|
body, _ := io.ReadAll(r.Body)
|
|
f.mu.Lock()
|
|
f.mergeCalls++
|
|
f.mergeBody = body
|
|
f.mu.Unlock()
|
|
w.WriteHeader(o.mergeStatus)
|
|
_, _ = w.Write([]byte(`{"message":"merge conflict"}`))
|
|
default:
|
|
t.Errorf("unexpected request: %s %s", r.Method, p)
|
|
w.WriteHeader(http.StatusNotFound)
|
|
}
|
|
}))
|
|
t.Cleanup(f.Close)
|
|
return f
|
|
}
|
|
|
|
func boolJSON(b bool) string {
|
|
if b {
|
|
return "true"
|
|
}
|
|
return "false"
|
|
}
|
|
|
|
func callMerge(t *testing.T, f *prMergeFake, args string) (map[string]any, error) {
|
|
t.Helper()
|
|
tool := tools.NewPRMerge(gitea.NewClient(f.URL, "tok"), allowlist.New([]string{"owner"}))
|
|
out, err := tool.Call(context.Background(), json.RawMessage(args))
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
var res map[string]any
|
|
require.NoError(t, json.Unmarshal(out, &res))
|
|
return res, nil
|
|
}
|
|
|
|
const mergeArgs = `{"owner":"owner","repo":"repo","number":7,"issue":42}`
|
|
|
|
func TestPRMergeSuccess(t *testing.T) {
|
|
f := newPRMergeFake(t, prMergeFakeOpts{})
|
|
|
|
res, err := callMerge(t, f, mergeArgs)
|
|
|
|
require.NoError(t, err)
|
|
assert.Equal(t, true, res["merged"])
|
|
assert.Equal(t, "success", res["ci_status"])
|
|
assert.Equal(t, "low", res["risk"])
|
|
assert.Equal(t, float64(7), res["pr_number"])
|
|
assert.True(t, f.merged())
|
|
}
|
|
|
|
func TestPRMergeDefaultsToMergeStyleAndPassesStyleThrough(t *testing.T) {
|
|
f := newPRMergeFake(t, prMergeFakeOpts{})
|
|
_, err := callMerge(t, f, mergeArgs)
|
|
require.NoError(t, err)
|
|
var payload map[string]any
|
|
require.NoError(t, json.Unmarshal(f.mergeBody, &payload))
|
|
assert.Equal(t, "merge", payload["Do"])
|
|
|
|
f2 := newPRMergeFake(t, prMergeFakeOpts{})
|
|
_, err = callMerge(t, f2, `{"owner":"owner","repo":"repo","number":7,"issue":42,"style":"squash"}`)
|
|
require.NoError(t, err)
|
|
require.NoError(t, json.Unmarshal(f2.mergeBody, &payload))
|
|
assert.Equal(t, "squash", payload["Do"])
|
|
}
|
|
|
|
// TOCTOU: the gate read CI for ONE head commit. The merge must be bound to that commit, so a push to the PR branch
|
|
// after the check makes Gitea refuse the merge instead of merging code nobody checked.
|
|
func TestPRMerge_TheMergeIsBoundToTheCommitTheGateChecked(t *testing.T) {
|
|
f := newPRMergeFake(t, prMergeFakeOpts{})
|
|
|
|
_, err := callMerge(t, f, mergeArgs)
|
|
|
|
require.NoError(t, err)
|
|
var payload map[string]any
|
|
require.NoError(t, json.Unmarshal(f.mergeBody, &payload))
|
|
assert.Equal(t, "abc", payload["head_commit_id"], "the head sha the CI check used")
|
|
}
|
|
|
|
// A merge conflict is still an error, as before the gate existed.
|
|
func TestPRMergeConflictReturnsError(t *testing.T) {
|
|
f := newPRMergeFake(t, prMergeFakeOpts{mergeStatus: http.StatusConflict})
|
|
|
|
_, err := callMerge(t, f, mergeArgs)
|
|
|
|
require.Error(t, err)
|
|
assert.ErrorIs(t, err, gitea.ErrConflict)
|
|
assert.True(t, f.merged(), "the merge was attempted: the gate had passed")
|
|
}
|
|
|
|
// #45: pr_merge advertises the canonical `number` (was `index`); `index` stays an accepted alias via the shim.
|
|
func TestPRMergeNumberCanonical(t *testing.T) {
|
|
sch := string(tools.NewPRMerge(gitea.NewClient("http://unused", ""), allowlist.New([]string{"owner"})).Descriptor().InputSchema)
|
|
assert.Contains(t, sch, `"number":`, "pr_merge must advertise number")
|
|
assert.NotContains(t, sch, `"index":`, "pr_merge must not advertise index")
|
|
|
|
for _, args := range []string{
|
|
`{"owner":"owner","repo":"repo","number":7,"issue":42}`,
|
|
`{"owner":"owner","repo":"repo","index":7,"issue":42}`,
|
|
`{"owner":"owner","name":"repo","index":7,"issue":42}`,
|
|
} {
|
|
f := newPRMergeFake(t, prMergeFakeOpts{})
|
|
res, err := callMerge(t, f, args)
|
|
require.NoError(t, err, args)
|
|
assert.Equal(t, true, res["merged"], args)
|
|
}
|
|
}
|
|
|
|
// gitea-mcp#74: the descriptor tells the caller about the gate.
|
|
func TestPRMergeDescriptorAdvertisesTheGate(t *testing.T) {
|
|
d := tools.NewPRMerge(gitea.NewClient("http://unused", ""), allowlist.New([]string{"owner"})).Descriptor()
|
|
assert.Contains(t, string(d.InputSchema), `"issue":`)
|
|
assert.Contains(t, string(d.InputSchema), `"ci_timeout_seconds":`)
|
|
assert.Contains(t, d.Description, "risk/low")
|
|
assert.Contains(t, d.Description, "merged:false")
|
|
}
|
|
|
|
func TestPRMergeAllowlistRejects(t *testing.T) {
|
|
tool := tools.NewPRMerge(gitea.NewClient("http://unused", ""), allowlist.New([]string{"allowed"}))
|
|
_, err := tool.Call(context.Background(), json.RawMessage(`{"owner":"evil","name":"repo","index":1}`))
|
|
require.Error(t, err)
|
|
}
|
|
|
|
// ---- the tier: only exactly one risk/low merges ----
|
|
func TestPRMerge_RiskTier_OnlyLowMerges(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
labels string
|
|
wantMerged bool
|
|
wantInReason string
|
|
}{
|
|
{"risk/low merges", `[{"id":1,"name":"risk/low"}]`, true, ""},
|
|
{"risk/low among other labels merges", `[{"id":2,"name":"type/fix"},{"id":1,"name":"risk/low"}]`, true, ""},
|
|
{"risk/medium does not merge", `[{"id":1,"name":"risk/medium"}]`, false, "risk/medium"},
|
|
{"risk/high does not merge", `[{"id":1,"name":"risk/high"}]`, false, "risk/high"},
|
|
{"no risk label is unclassified, not low", `[{"id":2,"name":"type/fix"}]`, false, "no risk/"},
|
|
{"no labels at all", `[]`, false, "no risk/"},
|
|
{"two risk labels are ambiguous", `[{"id":1,"name":"risk/low"},{"id":2,"name":"risk/high"}]`, false, "more than one"},
|
|
{"two identical risk/low labels are still ambiguous", `[{"id":1,"name":"risk/low"},{"id":2,"name":"risk/low"}]`, false, "more than one"},
|
|
{"an unknown value does not merge", `[{"id":1,"name":"risk/lowish"}]`, false, "unknown"},
|
|
{"the value is case-sensitive: risk/Low", `[{"id":1,"name":"risk/Low"}]`, false, "unknown"},
|
|
{"trailing space in the value", `[{"id":1,"name":"risk/low "}]`, false, "unknown"},
|
|
{"RISK/LOW is not a risk/ label", `[{"id":1,"name":"RISK/LOW"}]`, false, "no risk/"},
|
|
}
|
|
for _, tc := range tests {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
f := newPRMergeFake(t, prMergeFakeOpts{issueLabels: tc.labels})
|
|
|
|
res, err := callMerge(t, f, mergeArgs)
|
|
|
|
require.NoError(t, err, "a refusal is a result, not an error")
|
|
assert.Equal(t, tc.wantMerged, res["merged"])
|
|
assert.Equal(t, tc.wantMerged, f.merged(), "the merge endpoint is called iff the tier allows it")
|
|
assert.Equal(t, "success", res["ci_status"], "CI was green: the tier alone decided")
|
|
if !tc.wantMerged {
|
|
assert.Contains(t, res["reason"], tc.wantInReason)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestPRMerge_NoIssue_Unclassified_NoMerge(t *testing.T) {
|
|
f := newPRMergeFake(t, prMergeFakeOpts{})
|
|
|
|
res, err := callMerge(t, f, `{"owner":"owner","repo":"repo","number":7}`)
|
|
|
|
require.NoError(t, err)
|
|
assert.Equal(t, false, res["merged"])
|
|
assert.Equal(t, "none", res["risk"])
|
|
assert.Contains(t, res["reason"], "issue")
|
|
assert.False(t, f.merged(), "an unclassified change must not merge")
|
|
}
|
|
|
|
func TestPRMerge_IssueLookupFails_FailsClosed(t *testing.T) {
|
|
for _, status := range []int{http.StatusInternalServerError, http.StatusNotFound, http.StatusForbidden} {
|
|
f := newPRMergeFake(t, prMergeFakeOpts{issueStatus: status})
|
|
|
|
res, err := callMerge(t, f, mergeArgs)
|
|
|
|
require.NoError(t, err, "status %d", status)
|
|
assert.Equal(t, false, res["merged"], "status %d", status)
|
|
assert.Equal(t, "unreadable", res["risk"], "status %d", status)
|
|
assert.False(t, f.merged(), "status %d: an unreadable issue must not merge", status)
|
|
}
|
|
}
|
|
|
|
// ---- risk/low is necessary, not sufficient: CI and branch protection still gate ----
|
|
func TestPRMerge_RiskLow_DoesNotOverrideCI(t *testing.T) {
|
|
for name, runs := range map[string]string{
|
|
"red": `[{"id":1,"status":"completed","conclusion":"failure","head_sha":"abc"}]`,
|
|
"cancelled": `[{"id":1,"status":"completed","conclusion":"cancelled","head_sha":"abc"}]`,
|
|
"in_progress": `[{"id":1,"status":"in_progress","conclusion":"","head_sha":"abc"}]`,
|
|
"no runs": `[]`,
|
|
"mixed": `[{"id":1,"status":"completed","conclusion":"success","head_sha":"abc"},{"id":2,"status":"queued","conclusion":"","head_sha":"abc"}]`,
|
|
} {
|
|
t.Run(name, func(t *testing.T) {
|
|
f := newPRMergeFake(t, prMergeFakeOpts{runs: runs})
|
|
|
|
res, err := callMerge(t, f, mergeArgs)
|
|
|
|
require.NoError(t, err)
|
|
assert.Equal(t, false, res["merged"])
|
|
assert.NotEmpty(t, res["reason"])
|
|
assert.False(t, f.merged(), "risk/low must not override CI that is not fully green")
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestPRMerge_RiskLow_DoesNotOverrideAReviewProtectedBase(t *testing.T) {
|
|
f := newPRMergeFake(t, prMergeFakeOpts{requiredApprovals: 1})
|
|
|
|
res, err := callMerge(t, f, mergeArgs)
|
|
|
|
require.NoError(t, err)
|
|
assert.Equal(t, false, res["merged"])
|
|
assert.Contains(t, res["reason"], "review")
|
|
assert.False(t, f.merged())
|
|
}
|
|
|
|
// An unreadable PR or an unreadable branch-protection probe is an error, never a merge.
|
|
func TestPRMerge_ReadSideFailures_AreErrorsAndNeverMerge(t *testing.T) {
|
|
for name, o := range map[string]prMergeFakeOpts{
|
|
"PR lookup 500": {prStatus: http.StatusInternalServerError},
|
|
"PR lookup 404": {prStatus: http.StatusNotFound},
|
|
"protection probe 500": {protectionStatus: http.StatusInternalServerError},
|
|
} {
|
|
t.Run(name, func(t *testing.T) {
|
|
f := newPRMergeFake(t, o)
|
|
|
|
_, err := callMerge(t, f, mergeArgs)
|
|
|
|
require.Error(t, err)
|
|
assert.False(t, f.merged(), "must not merge when the gate cannot be evaluated")
|
|
})
|
|
}
|
|
}
|
|
|
|
// The classification comes from the issue the caller names, not from `number`: a PR number that is also an issue
|
|
// must not classify itself by accident. (The gate trusts the named issue: documented limit.)
|
|
func TestPRMerge_TheTierIsReadFromTheNamedIssueOnly(t *testing.T) {
|
|
f := newPRMergeFake(t, prMergeFakeOpts{issueLabels: `[{"id":1,"name":"risk/high"}]`})
|
|
|
|
res, err := callMerge(t, f, `{"owner":"owner","repo":"repo","number":7,"issue":42}`)
|
|
|
|
require.NoError(t, err)
|
|
assert.Equal(t, "high", res["risk"])
|
|
assert.False(t, f.merged())
|
|
}
|
|
|
|
// ---- review of PR #76 ----
|
|
|
|
// An empty head sha used to disable BOTH the CI scoping (the runs query lost its head_sha filter, so the repo's most
|
|
// recent runs of any commit were classified) and the merge binding (head_commit_id is omitempty). Fail closed.
|
|
func TestPRMerge_AnEmptyHeadShaFailsClosed(t *testing.T) {
|
|
f := newPRMergeFake(t, prMergeFakeOpts{emptyHeadSha: true})
|
|
|
|
res, err := callMerge(t, f, mergeArgs)
|
|
|
|
require.NoError(t, err)
|
|
assert.Equal(t, false, res["merged"])
|
|
assert.Contains(t, res["reason"], "head commit")
|
|
assert.False(t, f.merged(), "an unknown head commit must never merge")
|
|
assert.Empty(t, f.runsQuery, "no unscoped CI query may be made when the head sha is unknown")
|
|
}
|
|
|
|
// CI is read for the PR's head commit only: the query names it.
|
|
func TestPRMerge_CIIsScopedToTheHeadCommit(t *testing.T) {
|
|
f := newPRMergeFake(t, prMergeFakeOpts{})
|
|
|
|
_, err := callMerge(t, f, mergeArgs)
|
|
|
|
require.NoError(t, err)
|
|
assert.Contains(t, f.runsQuery, "head_sha=abc")
|
|
}
|
|
|
|
// Defence in depth: Gitea refuses closed/merged PRs itself, but the gate should not depend on that.
|
|
func TestPRMerge_ClosedOrDraftPRsAreNotMerged(t *testing.T) {
|
|
for name, o := range map[string]prMergeFakeOpts{
|
|
"closed": {prState: "closed"},
|
|
"merged": {prState: "merged"},
|
|
"draft": {prDraft: true},
|
|
} {
|
|
t.Run(name, func(t *testing.T) {
|
|
f := newPRMergeFake(t, o)
|
|
|
|
res, err := callMerge(t, f, mergeArgs)
|
|
|
|
require.NoError(t, err)
|
|
assert.Equal(t, false, res["merged"])
|
|
assert.NotEmpty(t, res["reason"])
|
|
assert.False(t, f.merged())
|
|
})
|
|
}
|
|
}
|