feat(tbd_ship): gate auto-merge on the risk tier; unclassified is not low (#63) (#73)

Co-authored-by: Mathias <mthbqv@gmail.com>

.context/system-prompt.txt

@@ -304,7 +304,7 @@ entries that age well are about *why*, *how to avoid*, and *what to do when*.
 |---------|-------|-------|
 | **Claude Code, Claude Desktop** | `brain_query` (BM25), `brain_answer` (LLM-synth + sources) MCP tools | `brain_write` MCP tool |
 | **Crush, Pi, Antigravity, other MCP-capable** | same MCP server: `ingestion-brain` (via the `mcp__*_brain__*` namespace once authenticated) | same |
-| **Anything HTTP-only (curl, scripts)** | `POST https://brain-mcp.d-ma.be/query` with `{"query":"..."}` (auth via `BRAIN_MCP_TOKEN`) | `POST .../write` with `{"content":"...","filename":"..."}` |
+| **Anything HTTP-only (curl, scripts)** | Tailnet only: `POST https://koala.tail30472e.ts.net:8443/query` with `{"query":"..."}`, sending `BRAIN_MCP_TOKEN` (not yet enforced on REST, hyperguild#149). The public `brain-mcp.d-ma.be` serves `/mcp` only (infra#517). | `POST .../write` with `{"content":"...","wing":"...","hall":"..."}` |
 | **Browser / human inspection** | `https://git.d-ma.be/mathias/hyperguild` → `knowledge/` and `wiki/` markdown files |
 
 - **Scoping — the brain holds no confidential client content.** Nothing on the read path

.gitea/workflows/cd.yml

@@ -8,6 +8,12 @@ name: CD
     # (Only applies to branch pushes; tag pushes always trigger.)
     paths-ignore:
       - 'docs/**'
+  # gitea-mcp#68: without this a PR never got a CI run, so the risk/medium+
+  # PR lane merged on local `task check` alone. Only the check job runs for a
+  # PR -- build and deploy stay push/tag-only (no images, no infra writes,
+  # no secrets from a PR).
+  pull_request:
+    branches: [main]
 
 env:
   IMAGE: gitea-mcp
@@ -44,6 +50,7 @@ jobs:
   build:
     name: Build & Import
     needs: check
+    if: github.event_name == 'push'
     runs-on: self-hosted
     outputs:
       image-tag: ${{ steps.meta.outputs.sha-tag }}
@@ -52,10 +59,12 @@ jobs:
 
       - name: Derive image tags
         id: meta
+        env:
+          # signals#47: a ref name can carry $(...); pass it via env, not ${{ }}.
+          REF: ${{ github.ref }}
         run: |
           SHA=$(git rev-parse --short HEAD)
           echo "sha-tag=${SHA}" >> "$GITHUB_OUTPUT"
-          REF="${{ github.ref }}"
           if [[ "$REF" == refs/tags/v* ]]; then
             echo "version-tag=${REF#refs/tags/}" >> "$GITHUB_OUTPUT"
             echo "version=${REF#refs/tags/}" >> "$GITHUB_OUTPUT"

AGENTS.md

@@ -299,7 +299,7 @@ entries that age well are about *why*, *how to avoid*, and *what to do when*.
 |---------|-------|-------|
 | **Claude Code, Claude Desktop** | `brain_query` (BM25), `brain_answer` (LLM-synth + sources) MCP tools | `brain_write` MCP tool |
 | **Crush, Pi, Antigravity, other MCP-capable** | same MCP server: `ingestion-brain` (via the `mcp__*_brain__*` namespace once authenticated) | same |
-| **Anything HTTP-only (curl, scripts)** | `POST https://brain-mcp.d-ma.be/query` with `{"query":"..."}` (auth via `BRAIN_MCP_TOKEN`) | `POST .../write` with `{"content":"...","filename":"..."}` |
+| **Anything HTTP-only (curl, scripts)** | Tailnet only: `POST https://koala.tail30472e.ts.net:8443/query` with `{"query":"..."}`, sending `BRAIN_MCP_TOKEN` (not yet enforced on REST, hyperguild#149). The public `brain-mcp.d-ma.be` serves `/mcp` only (infra#517). | `POST .../write` with `{"content":"...","wing":"...","hall":"..."}` |
 | **Browser / human inspection** | `https://git.d-ma.be/mathias/hyperguild` → `knowledge/` and `wiki/` markdown files |
 
 - **Scoping — the brain holds no confidential client content.** Nothing on the read path

docs/HOW_GITEA_MCP_WORKS.md

@@ -125,6 +125,13 @@ by area:
 | Search / Templates | code_search, create_project_from_template |
 | Composite | tbd_ship (trunk-based ship helper) |
 
+**`tbd_ship` merge gate (gitea-mcp#63).** A PR is squash-merged only when ALL of: the head commit's CI is fully green,
+the base branch is not review-protected, and the issue named in `issue` carries **exactly one** `risk/` label, `risk/low`.
+No `issue`, no `risk/` label, several, an unknown value (`risk/lowish`), `risk/medium`, `risk/high`, or an issue that
+cannot be read all fail closed: the PR is opened, `merged:false`, with a `reason` and the `risk` tier in the result.
+Unclassified is not low. Limit: the gate enforces the tier of the issue the caller names; it cannot prove that issue is
+the one the change belongs to, so the link is the caller's claim (the convention in local-dev rule 5 still applies).
+
 Shared tool-layer hygiene (`internal/tools/tool.go`):
 - **Alias normalization** — `repo`↔`name` and `number`↔`index` are reconciled so
   a tool works whichever spelling a caller sends (an explicit canonical wins).

internal/tools/file_write_branch.go

@@ -63,6 +63,9 @@ func (t *FileWriteBranch) Call(ctx context.Context, raw json.RawMessage) (json.R
 	if args.Branch == "" {
 		return nil, fmt.Errorf("branch is required: %w", gitea.ErrValidation)
 	}
+	if err := ValidateBranchName(args.Branch); err != nil {
+		return nil, err
+	}
 	if args.Message == "" {
 		return nil, fmt.Errorf("message is required: %w", gitea.ErrValidation)
 	}

internal/tools/foldkey_internal_test.go

@@ -0,0 +1,42 @@
+package tools
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+	"time"
+)
+
+// foldKey must agree with strings.EqualFold, which is how encoding/json matches.
+func TestFoldKeyAgreesWithEqualFold(t *testing.T) {
+	pairs := [][2]string{{"branch", "Branch"}, {"sha", "ſha"}, {"k", "K"}, {"ss", "ß"}, {"a", "b"}, {"Σ", "ς"}}
+	for _, p := range pairs {
+		if got, want := foldKey(p[0]) == foldKey(p[1]), strings.EqualFold(p[0], p[1]); got != want {
+			t.Errorf("%q vs %q: foldKey equal=%v, EqualFold=%v", p[0], p[1], got, want)
+		}
+	}
+}
+
+// The duplicate scan is linear in the number of keys: 90k keys (about the
+// 1 MiB body cap) must scan well under a second.
+func TestRejectAmbiguousKeysScalesLinearly(t *testing.T) {
+	var b strings.Builder
+	b.WriteString("{")
+	for i := 0; i < 90_000; i++ {
+		if i > 0 {
+			b.WriteString(",")
+		}
+		fmt.Fprintf(&b, `"k%d":1`, i)
+	}
+	b.WriteString("}")
+	var dst struct {
+		Owner string `json:"owner"`
+	}
+	start := time.Now()
+	if err := rejectAmbiguousKeys([]byte(b.String()), &dst); err != nil {
+		t.Fatal(err)
+	}
+	if d := time.Since(start); d > 2*time.Second {
+		t.Errorf("scan of 90k keys took %v", d)
+	}
+}

internal/tools/strict_args_test.go

@@ -0,0 +1,116 @@
+package tools_test
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"sync/atomic"
+	"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"
+)
+
+// countingServer fails nothing by itself; it records how many requests reached
+// it so a test can assert that a rejected call never touched Gitea.
+func countingServer(t *testing.T) (*httptest.Server, *atomic.Int32) {
+	t.Helper()
+	var n atomic.Int32
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		n.Add(1)
+		w.WriteHeader(http.StatusTeapot)
+	}))
+	t.Cleanup(srv.Close)
+	return srv, &n
+}
+
+// #65 (1): encoding/json matches keys case-insensitively and the last one wins,
+// so a client-side check reading `branch` could see a different value from the
+// one the server acts on. Any two keys equal under strings.EqualFold, and any
+// key that differs from a schema field only in case, are rejected before a
+// request is made.
+func TestParseArgsRejectsCaseVariantKeys(t *testing.T) {
+	cases := map[string]string{
+		"issue repro: branch + Branch": `{"owner":"probe","repo":"t","name":"t","path":"case8.txt","content":"x","message":"m","branch":"feat/y8","Branch":"main"}`,
+		"exact duplicate key":          `{"owner":"probe","repo":"t","path":"p","content":"x","message":"m","branch":"feat/y8","branch":"main"}`,
+		"case-only schema mismatch":    `{"owner":"probe","repo":"t","path":"p","content":"x","message":"m","BRANCH":"main"}`,
+		"nested object duplicate":      `{"owner":"probe","repo":"t","path":"p","content":"x","message":"m","branch":"feat/y8","extra":{"a":1,"A":2}}`,
+		// PR #66 review: a number the token scanner cannot parse as float64 must
+		// not make the scan give up and let the duplicate through.
+		"1e999 decoy before duplicate":  `{"owner":"probe","repo":"t","name":"t","path":"case8.txt","content":"x","message":"m","z":1e999,"branch":"feat/y8","Branch":"main"}`,
+		"-1e309 decoy before exact dup": `{"owner":"probe","repo":"t","path":"p","content":"x","message":"m","z":-1e309,"branch":"feat/y8","branch":"main"}`,
+		// encoding/json folds like strings.EqualFold: long s (U+017F) == s.
+		"nested long-s duplicate": `{"owner":"probe","repo":"t","path":"p","content":"x","message":"m","branch":"feat/y8","extra":{"s":1,"\u017f":2}}`,
+	}
+	for name, raw := range cases {
+		t.Run(name, func(t *testing.T) {
+			srv, n := countingServer(t)
+			tool := tools.NewFileWriteBranch(gitea.NewClient(srv.URL, "tok"), allowlist.New([]string{"probe"}))
+			_, err := tool.Call(context.Background(), json.RawMessage(raw))
+			require.Error(t, err)
+			assert.ErrorIs(t, err, gitea.ErrValidation)
+			assert.Zero(t, n.Load(), "no request may reach gitea")
+		})
+	}
+}
+
+// #65 (2): `refs/heads/main` as a branch makes Gitea create
+// refs/heads/refs/heads/main, after which every API write to main 500s.
+// Branch names that can create such refs are rejected before any request.
+func TestBranchNameValidationRejectsBeforeRequest(t *testing.T) {
+	bad := []string{
+		"refs/heads/main", "refs/tags/v1", "a..b", "feat x", "feat\tx", "feat/",
+		"/feat", "-feat", ".feat", "feat//x", "feat/.x", "feat.lock", "feat.", "HEAD", "feat@{1}", "",
+	}
+	for _, b := range bad {
+		t.Run("file_write_branch "+b, func(t *testing.T) {
+			srv, n := countingServer(t)
+			tool := tools.NewFileWriteBranch(gitea.NewClient(srv.URL, "tok"), allowlist.New([]string{"probe"}))
+			args, _ := json.Marshal(map[string]string{
+				"owner": "probe", "repo": "t", "path": "p", "content": "x", "message": "m", "branch": b,
+			})
+			_, err := tool.Call(context.Background(), args)
+			require.Error(t, err)
+			assert.ErrorIs(t, err, gitea.ErrValidation)
+			assert.Zero(t, n.Load(), "no request may reach gitea")
+		})
+		if b == "" {
+			continue // tbd_ship derives a branch when empty
+		}
+		t.Run("tbd_ship "+b, func(t *testing.T) {
+			srv, n := countingServer(t)
+			tool := tools.NewTBDShip(gitea.NewClient(srv.URL, "tok"), allowlist.New([]string{"probe"}))
+			args, _ := json.Marshal(map[string]string{
+				"owner": "probe", "repo": "t", "path": "p", "content": "x", "message": "m", "branch": b,
+			})
+			_, err := tool.Call(context.Background(), args)
+			require.Error(t, err)
+			assert.ErrorIs(t, err, gitea.ErrValidation)
+			assert.Zero(t, n.Load(), "no request may reach gitea")
+		})
+	}
+}
+
+// tbd_ship's base is the branch-protection probe key: `refs/heads/main` would
+// read as unprotected, so it is held to the same rule as the head branch.
+func TestTBDShipRejectsRefsPrefixedBase(t *testing.T) {
+	srv, n := countingServer(t)
+	tool := tools.NewTBDShip(gitea.NewClient(srv.URL, "tok"), allowlist.New([]string{"probe"}))
+	_, err := tool.Call(context.Background(), json.RawMessage(
+		`{"owner":"probe","repo":"t","path":"p","content":"x","message":"m","base":"refs/heads/main"}`))
+	require.Error(t, err)
+	assert.ErrorIs(t, err, gitea.ErrValidation)
+	assert.Zero(t, n.Load(), "no request may reach gitea")
+}
+
+func TestBranchNameValidationAcceptsOrdinaryNames(t *testing.T) {
+	for _, b := range []string{"main", "feat/y8", "tbd/fix-abc-1234abcd", "hold/mail-chat-infra-21", "v1.2_x"} {
+		t.Run(b, func(t *testing.T) {
+			assert.NoError(t, tools.ValidateBranchName(b))
+		})
+	}
+}

internal/tools/tbd_ship.go

@@ -37,7 +37,7 @@ const (
 func (t *TBDShip) Descriptor() registry.ToolDescriptor {
 	return registry.ToolDescriptor{
 		Name:        "tbd_ship",
-		Description: "Trunk-based ship of a single-file change: branch from base, write the file, open a PR, and auto-merge (squash) to base ONLY when the head commit's CI is fully green. Fails closed to PR-only (never merges) when CI is pending, red, or absent, when the base branch requires reviews, or when the merge isn't clean — returning the PR for manual handling with a reason. Set ci_timeout_seconds to poll CI to completion (default 0 = snapshot, which returns pending right after opening the PR). Returns {merged, pr_url, pr_number, ci_status, reason, branch}.",
+		Description: "Trunk-based ship of a single-file change: branch from base, write the file, open a PR, and auto-merge (squash) to base ONLY when the head commit's CI is fully green AND the named issue carries exactly one risk/ label, risk/low. Fails closed to PR-only (never merges) when CI is pending, red, or absent, when no issue is named or its risk/ label is missing, ambiguous, unknown, risk/medium or risk/high, when the issue cannot be read, when the base branch requires reviews, or when the merge isn't clean — returning the PR for manual handling with a reason. Set ci_timeout_seconds to poll CI to completion (default 0 = snapshot, which returns pending right after opening the PR). Returns {merged, pr_url, pr_number, ci_status, risk, reason, branch}.",
 		InputSchema: json.RawMessage(`{
 			"type":"object",
 			"properties":{
@@ -50,6 +50,7 @@ func (t *TBDShip) Descriptor() registry.ToolDescriptor {
 				"branch":{"type":"string","description":"Short-lived branch name. Default derived: tbd/<slug>-<hash>."},
 				"pr_title":{"type":"string","description":"PR title. Default: the commit message."},
 				"pr_body":{"type":"string"},
+				"issue":{"type":"integer","minimum":1,"description":"Issue number in the same repo whose risk/ label classifies this change. Auto-merge happens only for exactly one label, risk/low; no issue, none, several, an unknown value, risk/medium or risk/high means PR only, never merged (unclassified is not low)."},
 				"delete_branch":{"type":"boolean","description":"Delete the branch after a successful merge. Default true."},
 				"ci_timeout_seconds":{"type":"integer","minimum":0,"maximum":600,"description":"Poll CI up to this long for the head commit's runs to complete. Default 0 (single snapshot)."}
 			},
@@ -68,10 +69,76 @@ type tbdShipArgs struct {
 	Branch       string `json:"branch"`
 	PRTitle      string `json:"pr_title"`
 	PRBody       string `json:"pr_body"`
+	Issue        int    `json:"issue"`
 	DeleteBranch *bool  `json:"delete_branch"`
 	CITimeoutSec int    `json:"ci_timeout_seconds"`
 }
 
+// riskTier is the change's classification under the lane model (local-dev .context/AGENT.md rule 5): only a change
+// whose issue carries exactly one risk/ label, risk/low, may merge on green; everything else is reviewed by a human
+// (gitea-mcp#63). The zero value is deliberately NOT riskLow: a forgotten field must never mean "low".
+type riskTier string
+
+const (
+	riskLow        riskTier = "low"
+	riskMedium     riskTier = "medium"
+	riskHigh       riskTier = "high"
+	riskNone       riskTier = "none"       // no issue named, or the issue has no risk/ label: unclassified is not low
+	riskAmbiguous  riskTier = "ambiguous"  // more than one risk/ label
+	riskUnknown    riskTier = "unknown"    // risk/<something else>
+	riskUnreadable riskTier = "unreadable" // the issue lookup failed
+)
+
+// classifyRisk reads the tier from an issue's labels. Exact, case-sensitive prefix "risk/"; exactly one such label
+// is required, so two labels (even identical ones) are ambiguous rather than "the lower one wins".
+func classifyRisk(labels []gitea.Label) riskTier {
+	var found []string
+	for _, l := range labels {
+		if strings.HasPrefix(l.Name, "risk/") {
+			found = append(found, strings.TrimPrefix(l.Name, "risk/"))
+		}
+	}
+	switch len(found) {
+	case 0:
+		return riskNone
+	case 1:
+	default:
+		return riskAmbiguous
+	}
+	switch found[0] {
+	case "low":
+		return riskLow
+	case "medium":
+		return riskMedium
+	case "high":
+		return riskHigh
+	default:
+		return riskUnknown
+	}
+}
+
+const unclassifiedReason = "no risk/ label on the named issue (or no issue named) — unclassified is not low; PR opened, not merged"
+
+// riskBlockReason explains why a tier other than riskLow blocks the merge.
+func riskBlockReason(r riskTier) string {
+	switch r {
+	case riskMedium, riskHigh:
+		return fmt.Sprintf("the issue is risk/%s — above risk/low a human reviews the change; PR opened, not merged", r)
+	case riskAmbiguous:
+		return "the issue carries more than one risk/ label — ambiguous classification; PR opened, not merged"
+	case riskUnknown:
+		return "the issue carries an unknown risk/ value — PR opened, not merged"
+	case riskUnreadable:
+		return "could not read the issue's labels — fail-closed; PR opened, not merged"
+	case riskLow:
+		return "" // never asked: only a non-low tier needs explaining
+	case riskNone:
+		return unclassifiedReason
+	default: // the zero value and any tier added later: unclassified is not low
+		return unclassifiedReason
+	}
+}
+
 // shipGate is the decision produced by evaluateShipGate.
 type shipGate struct {
 	merge    bool
@@ -101,9 +168,10 @@ func classifyCI(runs []gitea.WorkflowRun) string {
 }
 
 // evaluateShipGate is the load-bearing safety of tbd_ship. It returns merge=true
-// only when CI is fully green AND the base branch is not review-protected. Every
+// only when CI is fully green AND the base branch is not review-protected AND the
-// other state fails closed to PR-only with an explanatory reason (#40).
+// change is classified risk/low. Every other state fails closed to PR-only with an
-func evaluateShipGate(runs []gitea.WorkflowRun, bp *gitea.BranchProtection) shipGate {
+// explanatory reason (#40, gitea-mcp#63).
+func evaluateShipGate(runs []gitea.WorkflowRun, bp *gitea.BranchProtection, risk riskTier) shipGate {
 	ci := classifyCI(runs)
 
 	// Review-protected base can't be auto-merged regardless of CI.
@@ -112,6 +180,11 @@ func evaluateShipGate(runs []gitea.WorkflowRun, bp *gitea.BranchProtection) ship
 			"base branch requires %d approving review(s) — auto-merge disabled; PR opened for manual merge", bp.RequiredApprovals)}
 	}
 
+	// Only risk/low merges on green. Checked before CI so an unclassified change never reads as "waiting for CI".
+	if risk != riskLow {
+		return shipGate{false, ci, riskBlockReason(risk)}
+	}
+
 	switch ci {
 	case "success":
 		return shipGate{true, ci, ""}
@@ -160,6 +233,13 @@ func (t *TBDShip) Call(ctx context.Context, raw json.RawMessage) (json.RawMessag
 	if branch == "" {
 		branch = deriveBranch(args.Message, args.Path, args.Content)
 	}
+	// Both names reach a ref: base is also the protection-probe key, so a
+	// `refs/heads/main` base would read as unprotected (#65).
+	for _, b := range []string{base, branch} {
+		if err := ValidateBranchName(b); err != nil {
+			return nil, err
+		}
+	}
 	deleteBranch := true
 	if args.DeleteBranch != nil {
 		deleteBranch = *args.DeleteBranch
@@ -223,7 +303,8 @@ func (t *TBDShip) Call(ctx context.Context, raw json.RawMessage) (json.RawMessag
 
 	// CI gate on the PR head commit.
 	runs := t.pollRuns(ctx, args.Owner, args.Repo, pr.Head.Sha, args.CITimeoutSec)
-	gate := evaluateShipGate(runs, bp)
+	risk := t.riskTierOf(ctx, args.Owner, args.Repo, args.Issue)
+	gate := evaluateShipGate(runs, bp, risk)
 
 	result := map[string]any{
 		"merged":    false,
@@ -231,6 +312,7 @@ func (t *TBDShip) Call(ctx context.Context, raw json.RawMessage) (json.RawMessag
 		"pr_url":    pr.HTMLURL,
 		"branch":    branch,
 		"ci_status": gate.ciStatus,
+		"risk":      string(risk),
 	}
 	if !gate.merge {
 		result["reason"] = gate.reason
@@ -254,6 +336,20 @@ func (t *TBDShip) Call(ctx context.Context, raw json.RawMessage) (json.RawMessag
 	return textOK(result)
 }
 
+// riskTierOf classifies the change from the named issue. No issue, or an issue that cannot be read, never yields
+// riskLow. The link between the PR and the issue is the caller's claim: the gate enforces the tier of the issue it
+// is given, it cannot prove that issue is the one the change belongs to (stated limit, gitea-mcp#63).
+func (t *TBDShip) riskTierOf(ctx context.Context, owner, repo string, issue int) riskTier {
+	if issue <= 0 {
+		return riskNone
+	}
+	iss, err := t.c.GetIssue(ctx, owner, repo, issue)
+	if err != nil || iss == nil {
+		return riskUnreadable
+	}
+	return classifyRisk(iss.Labels)
+}
+
 // pollRuns lists the head commit's workflow runs, polling up to timeoutSec for
 // them to reach a terminal state. timeoutSec == 0 is a single snapshot (no
 // sleep). A listing error yields no runs → the gate fails closed.

internal/tools/tbd_ship_internal_test.go

@@ -20,22 +20,31 @@ func TestEvaluateShipGate(t *testing.T) {
 		name      string
 		runs      []gitea.WorkflowRun
 		bp        *gitea.BranchProtection
+		risk      riskTier
 		wantMerge bool
 		wantCI    string
 	}{
-		{"all green → merge", []gitea.WorkflowRun{run("completed", "success")}, open, true, "success"},
+		{"all green → merge", []gitea.WorkflowRun{run("completed", "success")}, open, riskLow, true, "success"},
-		{"in_progress → no merge", []gitea.WorkflowRun{run("in_progress", "")}, open, false, "pending"},
+		{"in_progress → no merge", []gitea.WorkflowRun{run("in_progress", "")}, open, riskLow, false, "pending"},
-		{"queued → no merge", []gitea.WorkflowRun{run("queued", "")}, open, false, "pending"},
+		{"queued → no merge", []gitea.WorkflowRun{run("queued", "")}, open, riskLow, false, "pending"},
-		{"failed → no merge", []gitea.WorkflowRun{run("completed", "failure")}, open, false, "failed"},
+		{"failed → no merge", []gitea.WorkflowRun{run("completed", "failure")}, open, riskLow, false, "failed"},
-		{"cancelled → no merge (fail-closed)", []gitea.WorkflowRun{run("completed", "cancelled")}, open, false, "failed"},
+		{"cancelled → no merge (fail-closed)", []gitea.WorkflowRun{run("completed", "cancelled")}, open, riskLow, false, "failed"},
-		{"no runs → no merge (no CI gate)", nil, open, false, "none"},
+		{"no runs → no merge (no CI gate)", nil, open, riskLow, false, "none"},
-		{"mixed success+queued → no merge", []gitea.WorkflowRun{run("completed", "success"), run("queued", "")}, open, false, "pending"},
+		{"mixed success+queued → no merge", []gitea.WorkflowRun{run("completed", "success"), run("queued", "")}, open, riskLow, false, "pending"},
-		{"green but protected → no merge", []gitea.WorkflowRun{run("completed", "success")}, protected, false, "success"},
+		{"green but protected → no merge", []gitea.WorkflowRun{run("completed", "success")}, protected, riskLow, false, "success"},
-		{"no runs + protected → no merge", nil, protected, false, "none"},
+		{"no runs + protected → no merge", nil, protected, riskLow, false, "none"},
+		// gitea-mcp#63: the tier is an input. Green CI never overrides it; unclassified is not low.
+		{"green + risk/medium → no merge", []gitea.WorkflowRun{run("completed", "success")}, open, riskMedium, false, "success"},
+		{"green + risk/high → no merge", []gitea.WorkflowRun{run("completed", "success")}, open, riskHigh, false, "success"},
+		{"green + no risk label → no merge", []gitea.WorkflowRun{run("completed", "success")}, open, riskNone, false, "success"},
+		{"green + ambiguous labels → no merge", []gitea.WorkflowRun{run("completed", "success")}, open, riskAmbiguous, false, "success"},
+		{"green + unknown value → no merge", []gitea.WorkflowRun{run("completed", "success")}, open, riskUnknown, false, "success"},
+		{"green + unreadable issue → no merge", []gitea.WorkflowRun{run("completed", "success")}, open, riskUnreadable, false, "success"},
+		{"green + the zero tier → no merge (a forgotten field must not mean low)", []gitea.WorkflowRun{run("completed", "success")}, open, riskTier(""), false, "success"},
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			g := evaluateShipGate(tc.runs, tc.bp)
+			g := evaluateShipGate(tc.runs, tc.bp, tc.risk)
 			if g.merge != tc.wantMerge {
 				t.Errorf("merge = %v, want %v (reason: %q)", g.merge, tc.wantMerge, g.reason)
 			}
@@ -51,3 +60,41 @@ func TestEvaluateShipGate(t *testing.T) {
 		})
 	}
 }
+
+func TestClassifyRisk(t *testing.T) {
+	l := func(names ...string) []gitea.Label {
+		out := make([]gitea.Label, len(names))
+		for i, n := range names {
+			out[i] = gitea.Label{Name: n}
+		}
+		return out
+	}
+	tests := []struct {
+		name   string
+		labels []gitea.Label
+		want   riskTier
+	}{
+		{"low", l("risk/low"), riskLow},
+		{"medium", l("risk/medium"), riskMedium},
+		{"high", l("risk/high"), riskHigh},
+		{"low among others", l("type/fix", "risk/low", "size/s"), riskLow},
+		{"none", l("type/fix"), riskNone},
+		{"no labels", nil, riskNone},
+		{"two different", l("risk/low", "risk/high"), riskAmbiguous},
+		{"two identical", l("risk/low", "risk/low"), riskAmbiguous},
+		{"unknown value", l("risk/extreme"), riskUnknown},
+		{"empty value", l("risk/"), riskUnknown},
+		{"wrong case", l("Risk/Low"), riskNone},
+		{"value case matters: risk/Low is unknown, not low", l("risk/Low"), riskUnknown},
+		{"trailing space in the value is unknown, not low", l("risk/low "), riskUnknown},
+		{"leading space in the value is unknown, not low", l("risk/ low"), riskUnknown},
+		{"prefix only inside a name", l("not-risk/low"), riskNone},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := classifyRisk(tc.labels); got != tc.want {
+				t.Errorf("classifyRisk(%v) = %q, want %q", tc.labels, got, tc.want)
+			}
+		})
+	}
+}

internal/tools/tbd_ship_test.go

@@ -26,11 +26,21 @@ func shipFake(t *testing.T, runsJSON string, merged, deleted *atomic.Bool) *http
 // shipFakeOpts adds protection (requiredApprovals>0 → protected) and a merge
 // status code, so tests can drive the review-protected and conflict paths.
 func shipFakeOpts(t *testing.T, runsJSON string, requiredApprovals, mergeStatus int, merged, deleted *atomic.Bool) *httptest.Server {
+	t.Helper()
+	return shipFakeFull(t, runsJSON, requiredApprovals, mergeStatus, `[{"id":1,"name":"risk/low"}]`, http.StatusOK, merged, deleted)
+}
+
+// shipFakeFull also serves the issue whose risk/ label classifies the change (gitea-mcp#63): issueLabels is the
+// labels JSON array, issueStatus the HTTP status of the issue lookup.
+func shipFakeFull(t *testing.T, runsJSON string, requiredApprovals, mergeStatus int, issueLabels string, issueStatus int, merged, deleted *atomic.Bool) *httptest.Server {
 	t.Helper()
 	return 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 && strings.Contains(p, "/issues/42"):
+			w.WriteHeader(issueStatus)
+			_, _ = w.Write([]byte(`{"number":42,"title":"t","state":"open","labels":` + issueLabels + `}`))
 		case r.Method == http.MethodGet && strings.Contains(p, "/branch_protections/"):
 			if requiredApprovals > 0 {
 				_, _ = w.Write([]byte(`{"required_approvals":` + itoa(requiredApprovals) + `}`))
@@ -83,7 +93,7 @@ func TestTBDShip_GreenCI_Merges(t *testing.T) {
 	srv := shipFake(t, `[{"id":1,"status":"completed","conclusion":"success","head_sha":"abc"}]`, &merged, &deleted)
 	defer srv.Close()
 
-	res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x"}`)
+	res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x","issue":42}`)
 
 	assert.Equal(t, true, res["merged"])
 	assert.Equal(t, "success", res["ci_status"])
@@ -98,7 +108,7 @@ func TestTBDShip_NoCI_FailsClosed(t *testing.T) {
 	srv := shipFake(t, `[]`, &merged, &deleted)
 	defer srv.Close()
 
-	res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x"}`)
+	res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x","issue":42}`)
 
 	assert.Equal(t, false, res["merged"])
 	assert.Equal(t, "none", res["ci_status"])
@@ -113,7 +123,7 @@ func TestTBDShip_RedCI_FailsClosed(t *testing.T) {
 	srv := shipFake(t, `[{"id":1,"status":"completed","conclusion":"failure","head_sha":"abc"}]`, &merged, &deleted)
 	defer srv.Close()
 
-	res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x"}`)
+	res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x","issue":42}`)
 
 	assert.Equal(t, false, res["merged"])
 	assert.Equal(t, "failed", res["ci_status"])
@@ -128,7 +138,7 @@ func TestTBDShip_ReviewProtected_FailsClosed(t *testing.T) {
 	srv := shipFakeOpts(t, `[{"id":1,"status":"completed","conclusion":"success","head_sha":"abc"}]`, 1, http.StatusOK, &merged, &deleted)
 	defer srv.Close()
 
-	res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x"}`)
+	res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x","issue":42}`)
 
 	assert.Equal(t, false, res["merged"])
 	assert.NotEmpty(t, res["reason"])
@@ -142,7 +152,7 @@ func TestTBDShip_MergeConflict_FailsClosed(t *testing.T) {
 	srv := shipFakeOpts(t, `[{"id":1,"status":"completed","conclusion":"success","head_sha":"abc"}]`, 0, http.StatusConflict, &merged, &deleted)
 	defer srv.Close()
 
-	res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x"}`)
+	res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x","issue":42}`)
 
 	assert.Equal(t, false, res["merged"])
 	assert.NotEmpty(t, res["reason"])
@@ -160,6 +170,8 @@ func TestTBDShip_Resume_ExistingBranchAndPR(t *testing.T) {
 		p := r.URL.Path
 		w.Header().Set("Content-Type", "application/json")
 		switch {
+		case r.Method == http.MethodGet && strings.Contains(p, "/issues/42"):
+			_, _ = w.Write([]byte(`{"number":42,"title":"t","state":"open","labels":[{"id":1,"name":"risk/low"}]}`))
 		case r.Method == http.MethodGet && strings.Contains(p, "/branch_protections/"):
 			w.WriteHeader(http.StatusNotFound)
 			_, _ = w.Write([]byte(`{"message":"not found"}`))
@@ -196,7 +208,7 @@ func TestTBDShip_Resume_ExistingBranchAndPR(t *testing.T) {
 	}))
 	defer srv.Close()
 
-	res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x","branch":"`+branch+`"}`)
+	res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x","issue":42,"branch":"`+branch+`"}`)
 
 	assert.Equal(t, true, res["merged"], "resume must merge when CI is green")
 	assert.Equal(t, float64(7), res["pr_number"], "must reuse the existing PR #7")
@@ -204,6 +216,103 @@ func TestTBDShip_Resume_ExistingBranchAndPR(t *testing.T) {
 	assert.True(t, merged.Load())
 }
 
+// ---- gitea-mcp#63: the risk tier is an input to the gate. Only exactly one risk/low label merges. ----
+const (
+	shipGreen = `[{"id":1,"status":"completed","conclusion":"success","head_sha":"abc"}]`
+	shipArgs  = `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x","issue":42}`
+)
+
+func TestTBDShip_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"},{"id":3,"name":"size/s"}]`, 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 risk value does not merge", `[{"id":1,"name":"risk/lowish"}]`, false, "unknown"},
+		{"an empty risk value does not merge", `[{"id":1,"name":"risk/"}]`, false, "unknown"},
+		{"labels are case-sensitive: RISK/LOW is not risk/low", `[{"id":1,"name":"RISK/LOW"}]`, false, "no risk/"},
+		{"a label that merely contains risk/low does not count", `[{"id":1,"name":"not-risk/low"}]`, false, "no risk/"},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			var merged, deleted atomic.Bool
+			srv := shipFakeFull(t, shipGreen, 0, http.StatusOK, tc.labels, http.StatusOK, &merged, &deleted)
+			defer srv.Close()
+
+			res := callShip(t, srv.URL, shipArgs)
+
+			assert.Equal(t, tc.wantMerged, res["merged"])
+			assert.Equal(t, tc.wantMerged, merged.Load(), "the merge endpoint is called iff the tier allows it")
+			assert.Equal(t, "success", res["ci_status"], "CI was green: the tier alone decided")
+			assert.Equal(t, float64(7), res["pr_number"], "the PR is opened either way")
+			if !tc.wantMerged {
+				assert.Contains(t, res["reason"], tc.wantInReason)
+			}
+		})
+	}
+}
+
+// No issue named → nothing classifies the change → no merge, even on green CI.
+func TestTBDShip_NoIssue_Unclassified_NoMerge(t *testing.T) {
+	var merged, deleted atomic.Bool
+	srv := shipFake(t, shipGreen, &merged, &deleted)
+	defer srv.Close()
+
+	res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x"}`)
+
+	assert.Equal(t, false, res["merged"])
+	assert.Contains(t, res["reason"], "issue")
+	assert.False(t, merged.Load(), "an unclassified change must not merge")
+	assert.Equal(t, float64(7), res["pr_number"])
+}
+
+// The issue lookup failing is not a reason to merge: fail closed, PR left open.
+func TestTBDShip_IssueLookupFails_FailsClosed(t *testing.T) {
+	for _, status := range []int{http.StatusInternalServerError, http.StatusNotFound, http.StatusForbidden} {
+		var merged, deleted atomic.Bool
+		srv := shipFakeFull(t, shipGreen, 0, http.StatusOK, `[{"id":1,"name":"risk/low"}]`, status, &merged, &deleted)
+
+		res := callShip(t, srv.URL, shipArgs)
+		srv.Close()
+
+		assert.Equal(t, false, res["merged"], "status %d", status)
+		assert.NotEmpty(t, res["reason"], "status %d", status)
+		assert.False(t, merged.Load(), "status %d: an unreadable issue must not merge", status)
+	}
+}
+
+// risk/low is necessary, not sufficient: red CI still blocks it, and a review-protected base still blocks it.
+func TestTBDShip_RiskLow_DoesNotOverrideTheOtherGates(t *testing.T) {
+	var merged, deleted atomic.Bool
+	red := shipFakeFull(t, `[{"id":1,"status":"completed","conclusion":"failure","head_sha":"abc"}]`, 0, http.StatusOK, `[{"id":1,"name":"risk/low"}]`, http.StatusOK, &merged, &deleted)
+	defer red.Close()
+	assert.Equal(t, false, callShip(t, red.URL, shipArgs)["merged"])
+	assert.False(t, merged.Load())
+
+	prot := shipFakeFull(t, shipGreen, 1, http.StatusOK, `[{"id":1,"name":"risk/low"}]`, http.StatusOK, &merged, &deleted)
+	defer prot.Close()
+	assert.Equal(t, false, callShip(t, prot.URL, shipArgs)["merged"])
+	assert.False(t, merged.Load())
+}
+
+// The decision is visible to the caller.
+func TestTBDShip_ReportsTheRiskTier(t *testing.T) {
+	var merged, deleted atomic.Bool
+	srv := shipFakeFull(t, shipGreen, 0, http.StatusOK, `[{"id":1,"name":"risk/medium"}]`, http.StatusOK, &merged, &deleted)
+	defer srv.Close()
+
+	assert.Equal(t, "medium", callShip(t, srv.URL, shipArgs)["risk"])
+}
+
 func TestTBDShip_AllowlistRejects(t *testing.T) {
 	tool := tools.NewTBDShip(gitea.NewClient("http://unused", ""), allowlist.New([]string{"mathias"}))
 	_, err := tool.Call(context.Background(), json.RawMessage(`{"owner":"evil","repo":"r","path":"p","content":"c","message":"m"}`))

internal/tools/tool.go

@@ -1,10 +1,14 @@
 package tools
 
 import (
+	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"reflect"
+	"regexp"
 	"strings"
+	"unicode"
 
 	"git.d-ma.be/mathias/gitea-mcp/internal/gitea"
 	"git.d-ma.be/mathias/gitea-mcp/internal/registry"
@@ -22,12 +26,154 @@ func parseArgs(raw json.RawMessage, dst any) error {
 		if err := json.Unmarshal([]byte("{}"), dst); err != nil {
 			return err
 		}
-	} else if err := json.Unmarshal(normalizeAliases(raw), dst); err != nil {
+	} else {
+		// Reject ambiguity BEFORE alias normalisation: re-marshalling through a
+		// map would otherwise silently collapse exact duplicates (#65).
+		if err := rejectAmbiguousKeys(raw, dst); err != nil {
 			return err
 		}
+		if err := json.Unmarshal(normalizeAliases(raw), dst); err != nil {
+			return err
+		}
+	}
 	return validateRequiredIdentifiers(dst)
 }
 
+// rejectAmbiguousKeys refuses input whose meaning depends on encoding/json's
+// case-insensitive, last-key-wins matching (#65). A client-side check that reads
+// `branch` must see the value the server acts on, so: no object at any depth may
+// carry two keys equal under strings.EqualFold (exact duplicates included), and
+// no top-level key may differ from a field of dst only in case.
+func rejectAmbiguousKeys(raw json.RawMessage, dst any) error {
+	dec := json.NewDecoder(bytes.NewReader(raw))
+	// UseNumber: without it Token() fails on a number outside float64 range,
+	// which json.Unmarshal ignores in an unknown key — a decoy that would end
+	// the scan before the duplicate (PR #66 review).
+	dec.UseNumber()
+	if err := scanJSONValue(dec); err != nil {
+		if errors.Is(err, gitea.ErrValidation) {
+			return err
+		}
+		// Fail closed: input the scanner cannot read is input it cannot vouch for.
+		return fmt.Errorf("arguments are not valid JSON: %v: %w", err, gitea.ErrValidation)
+	}
+	var top map[string]json.RawMessage
+	if err := json.Unmarshal(raw, &top); err != nil {
+		return nil // not an object; nothing to compare against the schema
+	}
+	fields := jsonFieldNames(dst)
+	for k := range top {
+		for _, f := range fields {
+			if k != f && strings.EqualFold(k, f) {
+				return fmt.Errorf("argument %q does not match field %q exactly (keys are case-sensitive): %w", k, f, gitea.ErrValidation)
+			}
+		}
+	}
+	return nil
+}
+
+// scanJSONValue consumes one JSON value from dec, failing with ErrValidation if
+// any object within it has two keys that are equal under strings.EqualFold.
+func scanJSONValue(dec *json.Decoder) error {
+	tok, err := dec.Token()
+	if err != nil {
+		return err
+	}
+	d, ok := tok.(json.Delim)
+	if !ok {
+		return nil
+	}
+	switch d {
+	case '{':
+		seen := map[string]string{}
+		for dec.More() {
+			kt, err := dec.Token()
+			if err != nil {
+				return err
+			}
+			k, _ := kt.(string)
+			fk := foldKey(k)
+			if prev, dup := seen[fk]; dup {
+				return fmt.Errorf("duplicate argument keys %q and %q (keys must be unique ignoring case): %w", prev, k, gitea.ErrValidation)
+			}
+			seen[fk] = k
+			if err := scanJSONValue(dec); err != nil {
+				return err
+			}
+		}
+	case '[':
+		for dec.More() {
+			if err := scanJSONValue(dec); err != nil {
+				return err
+			}
+		}
+	}
+	_, err = dec.Token() // closing delimiter
+	return err
+}
+
+// foldKey maps every rune to the smallest member of its unicode.SimpleFold
+// orbit, so foldKey(a) == foldKey(b) exactly when strings.EqualFold(a, b) —
+// the folding encoding/json uses (long s == s, Kelvin sign == k). A map keyed
+// on it keeps the duplicate scan linear (PR #66 review: a pairwise EqualFold
+// loop cost 7s of CPU on ~90k keys).
+func foldKey(s string) string {
+	var b strings.Builder
+	for _, r := range s {
+		min := r
+		for f := unicode.SimpleFold(r); f != r; f = unicode.SimpleFold(f) {
+			if f < min {
+				min = f
+			}
+		}
+		b.WriteRune(min)
+	}
+	return b.String()
+}
+
+// jsonFieldNames returns the json tag names of dst's top-level struct fields.
+func jsonFieldNames(dst any) []string {
+	v := reflect.ValueOf(dst)
+	if v.Kind() != reflect.Pointer || v.IsNil() || v.Elem().Kind() != reflect.Struct {
+		return nil
+	}
+	t := v.Elem().Type()
+	names := make([]string, 0, t.NumField())
+	for i := 0; i < t.NumField(); i++ {
+		name, _, _ := strings.Cut(t.Field(i).Tag.Get("json"), ",")
+		if name == "" || name == "-" {
+			name = t.Field(i).Name
+		}
+		names = append(names, name)
+	}
+	return names
+}
+
+// branchNameRe is the shape the skills mcp-destructive-guard hook enforces.
+var branchNameRe = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._/-]*$`)
+
+// ValidateBranchName rejects branch names that Gitea would turn into a ref
+// other than refs/heads/<name> as the caller meant it. `refs/heads/main` in
+// particular creates refs/heads/refs/heads/main, after which every API write to
+// main fails with "matches more than one" until the ref is deleted by hand
+// (#65). Beyond the shape regexp it applies git's own check-ref-format rules
+// that the regexp admits.
+func ValidateBranchName(b string) error {
+	bad := !branchNameRe.MatchString(b) ||
+		strings.HasPrefix(b, "refs/") ||
+		b == "HEAD" ||
+		strings.Contains(b, "..") ||
+		strings.Contains(b, "//") ||
+		strings.Contains(b, "/.") ||
+		strings.HasSuffix(b, "/") ||
+		strings.HasSuffix(b, ".") ||
+		strings.HasSuffix(b, ".lock")
+	if bad {
+		return fmt.Errorf("invalid branch name %q (want a short name like feat/x, no refs/ prefix): %w", b, gitea.ErrValidation)
+	}
+	return nil
+}
+
 // validateRequiredIdentifiers rejects an empty repo identifier at the tool layer
 // before it reaches the client, where it would build a trailing-empty path
 // segment (`/api/v1/repos/{owner}/`) and leak gitea's bare 404 (#37). `repo` and