feat(labels): add label_list + issue_label tools (#52)

Closes #52 — unblocks parallax#3 dispatch labeling, which needs to both
discover a repo's label set and attach labels to an issue by name (the
CAD pipeline doesn't track Gitea's internal numeric label IDs).

- gitea.Client: ListLabels, AddIssueLabels (additive POST, matches
  Gitea's own semantics — no delete-then-post needed); extend the
  existing Label struct with Color for label_list's output.
- tools.LabelList (read-only, allowlisted): lists a repo's labels.
- tools.IssueLabel (allowlisted): resolves label names to IDs via
  ListLabels, so callers pass names (the primary interface) instead of
  hunting for numeric IDs; also accepts label_ids for callers that
  already have them. An unknown name fails closed, naming exactly which
  label wasn't found.
- Bump TestRegisteredToolCount 39 -> 41 in the same commit (this
  project was bitten today by a locked count going stale silently).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

.gitea/workflows/cd.yml

@@ -60,7 +60,11 @@ jobs:
         run: |
           REGISTRY="localhost:5000"
           REF="${REGISTRY}/${{ env.IMAGE }}:${{ steps.meta.outputs.sha-tag }}"
+          # Stamp the real build version: the git tag on a v* build, else the short sha.
+          VERSION="${{ steps.meta.outputs.version-tag }}"
+          [ -z "$VERSION" ] && VERSION="${{ steps.meta.outputs.sha-tag }}"
           buildah build \
+            --build-arg VERSION="${VERSION}" \
             --label "org.opencontainers.image.revision=${{ github.sha }}" \
             --label "org.opencontainers.image.source=${{ github.repositoryUrl }}" \
             -t ${REF} \

Dockerfile

@@ -14,7 +14,10 @@ ENV GOSUMDB=off
 COPY go.mod go.sum ./
 RUN go mod download
 COPY . .
-RUN CGO_ENABLED=0 go build -trimpath -ldflags='-s -w' -o /out/gitea-mcp ./cmd/gitea-mcp
+# Build version stamped in by CI (--build-arg VERSION=<tag|sha>); defaults to
+# "dev" for a plain `docker build` (#47).
+ARG VERSION=dev
+RUN CGO_ENABLED=0 go build -trimpath -ldflags="-s -w -X main.version=${VERSION}" -o /out/gitea-mcp ./cmd/gitea-mcp
 
 FROM gcr.io/distroless/static-debian12:nonroot
 COPY --from=build /out/gitea-mcp /gitea-mcp

internal/gitea/issues.go

@@ -24,6 +24,7 @@ type Issue struct {
 type Label struct {
 	ID    int64  `json:"id"`
 	Name  string `json:"name"`
+	Color string `json:"color,omitempty"`
 }
 
 type User struct {

internal/gitea/labels.go

@@ -0,0 +1,48 @@
+package gitea
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+)
+
+// ListLabels fetches all labels defined on a repo.
+func (c *Client) ListLabels(ctx context.Context, owner, repo string) ([]Label, error) {
+	p := fmt.Sprintf("/api/v1/repos/%s/%s/labels", owner, repo)
+	body, status, err := c.GetJSON(ctx, p)
+	if err != nil {
+		return nil, err
+	}
+	if err := MapStatus(status, body); err != nil {
+		return nil, err
+	}
+	var labels []Label
+	if err := json.Unmarshal(body, &labels); err != nil {
+		return nil, err
+	}
+	return labels, nil
+}
+
+// AddIssueLabels adds labelIDs to an issue or pull request (PRs share index
+// space with issues, per Gitea). This is additive per Gitea's own POST
+// semantics — existing labels are left in place, no replace/delete needed.
+// Returns the issue's full label set after the add.
+func (c *Client) AddIssueLabels(ctx context.Context, owner, repo string, number int, labelIDs []int64) ([]Label, error) {
+	p := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/labels", owner, repo, number)
+	payload, err := json.Marshal(map[string][]int64{"labels": labelIDs})
+	if err != nil {
+		return nil, err
+	}
+	body, status, err := c.PostJSON(ctx, p, payload)
+	if err != nil {
+		return nil, err
+	}
+	if err := MapStatus(status, body); err != nil {
+		return nil, err
+	}
+	var labels []Label
+	if err := json.Unmarshal(body, &labels); err != nil {
+		return nil, err
+	}
+	return labels, nil
+}

internal/gitea/labels_test.go

@@ -0,0 +1,107 @@
+package gitea_test
+
+import (
+	"context"
+	"encoding/json"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"git.d-ma.be/mathias/gitea-mcp/internal/gitea"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestListLabels(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		assert.Equal(t, http.MethodGet, r.Method)
+		assert.Equal(t, "/api/v1/repos/o/r/labels", r.URL.Path)
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`[
+			{"id":1,"name":"bug","color":"ee0701"},
+			{"id":2,"name":"enhancement","color":"84b6eb"}
+		]`))
+	}))
+	defer srv.Close()
+
+	c := gitea.NewClient(srv.URL, "tok")
+	labels, err := c.ListLabels(context.Background(), "o", "r")
+	require.NoError(t, err)
+	require.Len(t, labels, 2)
+	assert.Equal(t, int64(1), labels[0].ID)
+	assert.Equal(t, "bug", labels[0].Name)
+	assert.Equal(t, "ee0701", labels[0].Color)
+	assert.Equal(t, "enhancement", labels[1].Name)
+}
+
+func TestListLabels_Empty(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(`[]`))
+	}))
+	defer srv.Close()
+
+	c := gitea.NewClient(srv.URL, "tok")
+	labels, err := c.ListLabels(context.Background(), "o", "r")
+	require.NoError(t, err)
+	assert.Empty(t, labels)
+}
+
+func TestListLabels_NotFound(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusNotFound)
+		_, _ = w.Write([]byte(`{"message":"repo not found"}`))
+	}))
+	defer srv.Close()
+
+	c := gitea.NewClient(srv.URL, "tok")
+	_, err := c.ListLabels(context.Background(), "o", "r")
+	require.Error(t, err)
+	assert.ErrorIs(t, err, gitea.ErrNotFound)
+}
+
+func TestAddIssueLabels(t *testing.T) {
+	var captured []byte
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		assert.Equal(t, http.MethodPost, r.Method)
+		assert.Equal(t, "/api/v1/repos/o/r/issues/42/labels", r.URL.Path)
+		var err error
+		captured, err = io.ReadAll(r.Body)
+		require.NoError(t, err)
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`[
+			{"id":1,"name":"bug","color":"ee0701"},
+			{"id":3,"name":"priority","color":"00ff00"}
+		]`))
+	}))
+	defer srv.Close()
+
+	c := gitea.NewClient(srv.URL, "tok")
+	labels, err := c.AddIssueLabels(context.Background(), "o", "r", 42, []int64{3})
+	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)
+	require.Len(t, ids, 1)
+	assert.Equal(t, float64(3), ids[0])
+
+	require.Len(t, labels, 2)
+	assert.Equal(t, "bug", labels[0].Name)
+	assert.Equal(t, "priority", labels[1].Name)
+}
+
+func TestAddIssueLabels_NotFound(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusNotFound)
+		_, _ = w.Write([]byte(`{"message":"issue not found"}`))
+	}))
+	defer srv.Close()
+
+	c := gitea.NewClient(srv.URL, "tok")
+	_, err := c.AddIssueLabels(context.Background(), "o", "r", 999, []int64{1})
+	require.Error(t, err)
+	assert.ErrorIs(t, err, gitea.ErrNotFound)
+}

internal/tools/issue_label.go

@@ -0,0 +1,87 @@
+package tools
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+
+	"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/registry"
+)
+
+type IssueLabel struct {
+	c *gitea.Client
+	a *allowlist.Allowlist
+}
+
+func NewIssueLabel(c *gitea.Client, a *allowlist.Allowlist) *IssueLabel {
+	return &IssueLabel{c: c, a: a}
+}
+
+func (t *IssueLabel) Descriptor() registry.ToolDescriptor {
+	return registry.ToolDescriptor{
+		Name:        "issue_label",
+		Description: "Add labels to an issue or pull request. Resolves label names to IDs via the repo's label list. Additive — existing labels are left in place.",
+		InputSchema: json.RawMessage(`{
+			"type":"object",
+			"properties":{
+				"owner":{"type":"string"},
+				"repo":{"type":"string"},
+				"number":{"type":"integer","minimum":1},
+				"labels":{"type":"array","items":{"type":"string"}},
+				"label_ids":{"type":"array","items":{"type":"integer"}}
+			},
+			"required":["owner","repo","number","labels"]
+		}`),
+	}
+}
+
+type issueLabelArgs struct {
+	Owner    string   `json:"owner"`
+	Repo     string   `json:"repo"`
+	Number   int      `json:"number"`
+	Labels   []string `json:"labels,omitempty"`
+	LabelIDs []int64  `json:"label_ids,omitempty"`
+}
+
+func (t *IssueLabel) Call(ctx context.Context, raw json.RawMessage) (json.RawMessage, error) {
+	var args issueLabelArgs
+	if err := parseArgs(raw, &args); err != nil {
+		return nil, err
+	}
+	if err := t.a.Check(args.Owner); err != nil {
+		return nil, err
+	}
+	if args.Number < 1 {
+		return nil, fmt.Errorf("number must be >= 1: %w", gitea.ErrValidation)
+	}
+	if len(args.Labels) == 0 && len(args.LabelIDs) == 0 {
+		return nil, fmt.Errorf("labels is required: %w", gitea.ErrValidation)
+	}
+
+	ids := append([]int64{}, args.LabelIDs...)
+	if len(args.Labels) > 0 {
+		existing, err := t.c.ListLabels(ctx, args.Owner, args.Repo)
+		if err != nil {
+			return nil, err
+		}
+		byName := make(map[string]int64, len(existing))
+		for _, l := range existing {
+			byName[l.Name] = l.ID
+		}
+		for _, name := range args.Labels {
+			id, ok := byName[name]
+			if !ok {
+				return nil, fmt.Errorf("label %q not found in %s/%s: %w", name, args.Owner, args.Repo, gitea.ErrValidation)
+			}
+			ids = append(ids, id)
+		}
+	}
+
+	labels, err := t.c.AddIssueLabels(ctx, args.Owner, args.Repo, args.Number, ids)
+	if err != nil {
+		return nil, err
+	}
+	return textOK(labels)
+}

internal/tools/issue_label_test.go

@@ -0,0 +1,82 @@
+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"`)
+}
+
+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)
+}

internal/tools/label_list.go

@@ -0,0 +1,54 @@
+package tools
+
+import (
+	"context"
+	"encoding/json"
+
+	"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/registry"
+)
+
+type LabelList struct {
+	c *gitea.Client
+	a *allowlist.Allowlist
+}
+
+func NewLabelList(c *gitea.Client, a *allowlist.Allowlist) *LabelList {
+	return &LabelList{c: c, a: a}
+}
+
+func (t *LabelList) Descriptor() registry.ToolDescriptor {
+	return registry.ToolDescriptor{
+		Name:        "label_list",
+		Description: "List all labels defined on a repo. Returns id, name, and color for each label.",
+		InputSchema: json.RawMessage(`{
+			"type":"object",
+			"properties":{
+				"owner":{"type":"string"},
+				"repo":{"type":"string"}
+			},
+			"required":["owner","repo"]
+		}`),
+	}
+}
+
+type labelListArgs struct {
+	Owner string `json:"owner"`
+	Repo  string `json:"repo"`
+}
+
+func (t *LabelList) Call(ctx context.Context, raw json.RawMessage) (json.RawMessage, error) {
+	var args labelListArgs
+	if err := parseArgs(raw, &args); err != nil {
+		return nil, err
+	}
+	if err := t.a.Check(args.Owner); err != nil {
+		return nil, err
+	}
+	labels, err := t.c.ListLabels(ctx, args.Owner, args.Repo)
+	if err != nil {
+		return nil, err
+	}
+	return textOK(labels)
+}

internal/tools/label_list_test.go

@@ -0,0 +1,42 @@
+package tools_test
+
+import (
+	"context"
+	"encoding/json"
+	"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"
+)
+
+func TestLabelListTool(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		assert.Equal(t, http.MethodGet, r.Method)
+		assert.Equal(t, "/api/v1/repos/mathias/infra/labels", r.URL.Path)
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`[
+			{"id":1,"name":"bug","color":"ee0701"},
+			{"id":2,"name":"enhancement","color":"84b6eb"}
+		]`))
+	}))
+	defer srv.Close()
+
+	tool := tools.NewLabelList(gitea.NewClient(srv.URL, "tok"), allowlist.New([]string{"mathias"}))
+	out, err := tool.Call(context.Background(), json.RawMessage(`{"owner":"mathias","repo":"infra"}`))
+	require.NoError(t, err)
+	assert.Contains(t, string(out), `"id":1`)
+	assert.Contains(t, string(out), `"name":"bug"`)
+	assert.Contains(t, string(out), `"color":"ee0701"`)
+	assert.Contains(t, string(out), `"name":"enhancement"`)
+}
+
+func TestLabelListAllowlistRejects(t *testing.T) {
+	tool := tools.NewLabelList(gitea.NewClient("http://unused", ""), allowlist.New([]string{"mathias"}))
+	_, err := tool.Call(context.Background(), json.RawMessage(`{"owner":"evil","repo":"x"}`))
+	require.Error(t, err)
+}

internal/tools/registry.go

@@ -54,6 +54,8 @@ func RegisterAll(
 	reg.Register(NewIssueListComments(c, a))
 	reg.Register(NewIssueClose(c, a))
 	reg.Register(NewIssueReopen(c, a))
+	reg.Register(NewLabelList(c, a))
+	reg.Register(NewIssueLabel(c, a))
 	reg.Register(NewWorkflowRunList(c, a))
 	reg.Register(NewReleaseCreate(c, a))
 	reg.Register(NewRepoDelete(c, a))

internal/tools/registry_test.go

@@ -54,5 +54,5 @@ func TestEveryRegisteredToolIsDispatchable(t *testing.T) {
 // 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)
+	assert.Len(t, buildRegistry().Tools(), 41)
 }

internal/tools/repo_mirror_push.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"os"
 
 	"git.d-ma.be/mathias/gitea-mcp/internal/allowlist"
 	"git.d-ma.be/mathias/gitea-mcp/internal/gitea"
@@ -22,7 +23,7 @@ func NewRepoMirrorPush(c *gitea.Client, a *allowlist.Allowlist) *RepoMirrorPush
 func (t *RepoMirrorPush) Descriptor() registry.ToolDescriptor {
 	return registry.ToolDescriptor{
 		Name:        "repo_mirror_push",
-		Description: "Manage push mirrors for a repository: add, list, or delete.",
+		Description: "Manage push mirrors for a repository: add, list, or delete. For the mirror credential, PREFER remote_password_env (the name of an env var the server reads) so the secret never rides the tool-call payload/transcript; remote_password (raw) is discouraged and will be persisted in logs.",
 		InputSchema: json.RawMessage(`{
 			"type":"object",
 			"properties":{
@@ -31,7 +32,8 @@ func (t *RepoMirrorPush) Descriptor() registry.ToolDescriptor {
 				"action":{"type":"string","enum":["add","list","delete"]},
 				"remote_address":{"type":"string","description":"Mirror target URL (required for add)."},
 				"remote_username":{"type":"string"},
-				"remote_password":{"type":"string","description":"Never logged or returned."},
+				"remote_password_env":{"type":"string","description":"PREFERRED: name of a server-side env var holding the mirror credential; the server resolves it, so the secret is never in this call. Errors if the var is unset."},
+				"remote_password":{"type":"string","description":"DISCOURAGED: raw credential — lands in the tool-call transcript/logs. Use remote_password_env instead."},
 				"interval":{"type":"string","description":"Sync interval, e.g. '8h0m0s'."},
 				"sync_on_commit":{"type":"boolean"},
 				"mirror_name":{"type":"string","description":"Remote name to delete (required for delete)."}
@@ -48,6 +50,7 @@ type repoMirrorPushArgs struct {
 	RemoteAddress     string `json:"remote_address"`
 	RemoteUsername    string `json:"remote_username"`
 	RemotePassword    string `json:"remote_password"`
+	RemotePasswordEnv string `json:"remote_password_env"`
 	Interval          string `json:"interval"`
 	SyncOnCommit      bool   `json:"sync_on_commit"`
 	MirrorName        string `json:"mirror_name"`
@@ -72,6 +75,22 @@ func toSafeMirror(m *gitea.PushMirror) safeMirror {
 	}
 }
 
+// resolveMirrorPassword prefers remote_password_env — the name of a server-side
+// env var — so the credential never appears in the tool-call payload (#49). It
+// falls back to the raw (discouraged) remote_password. An env name that resolves
+// to empty is a loud error, not a silent empty password.
+func resolveMirrorPassword(args repoMirrorPushArgs) (string, error) {
+	if args.RemotePasswordEnv != "" {
+		pw := os.Getenv(args.RemotePasswordEnv)
+		if pw == "" {
+			return "", fmt.Errorf("remote_password_env %q is unset or empty in the server environment: %w",
+				args.RemotePasswordEnv, gitea.ErrValidation)
+		}
+		return pw, nil
+	}
+	return args.RemotePassword, nil
+}
+
 func (t *RepoMirrorPush) Call(ctx context.Context, raw json.RawMessage) (json.RawMessage, error) {
 	var args repoMirrorPushArgs
 	if err := parseArgs(raw, &args); err != nil {
@@ -82,10 +101,14 @@ func (t *RepoMirrorPush) Call(ctx context.Context, raw json.RawMessage) (json.Ra
 	}
 	switch args.Action {
 	case "add":
+		password, err := resolveMirrorPassword(args)
+		if err != nil {
+			return nil, err
+		}
 		m, err := t.c.AddPushMirror(ctx, args.Owner, args.Repo, gitea.AddPushMirrorArgs{
 			RemoteAddress:  args.RemoteAddress,
 			RemoteUsername: args.RemoteUsername,
-			RemotePassword: args.RemotePassword,
+			RemotePassword: password,
 			Interval:       args.Interval,
 			SyncOnCommit:   args.SyncOnCommit,
 		})

internal/tools/repo_mirror_push_test.go

@@ -3,6 +3,7 @@ package tools_test
 import (
 	"context"
 	"encoding/json"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -14,6 +15,45 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// #49: remote_password_env names a server-side env var; the secret is resolved
+// from the server environment and never rides the tool-call payload.
+func TestRepoMirrorPushTool_PasswordFromEnv(t *testing.T) {
+	t.Setenv("TEST_MIRROR_PW", "env-secret")
+	var gotPw string
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		body, _ := io.ReadAll(r.Body)
+		var m map[string]any
+		_ = json.Unmarshal(body, &m)
+		gotPw, _ = m["remote_password"].(string)
+		w.WriteHeader(http.StatusCreated)
+		_, _ = w.Write([]byte(`{"id":1,"remote_name":"m","remote_address":"a"}`))
+	}))
+	defer srv.Close()
+
+	tool := tools.NewRepoMirrorPush(gitea.NewClient(srv.URL, "tok"), allowlist.New([]string{"mathias"}))
+	out, err := tool.Call(context.Background(), json.RawMessage(`{
+		"owner":"mathias","name":"infra","action":"add",
+		"remote_address":"https://github.com/mathias/infra.git",
+		"remote_username":"mathias","remote_password_env":"TEST_MIRROR_PW"
+	}`))
+	require.NoError(t, err)
+	assert.Equal(t, "env-secret", gotPw, "password must be resolved from the server env var")
+	assert.NotContains(t, string(out), "env-secret")
+}
+
+// remote_password_env pointing at an unset var must fail loudly, not silently
+// send an empty password.
+func TestRepoMirrorPushTool_EnvUnsetErrors(t *testing.T) {
+	tool := tools.NewRepoMirrorPush(gitea.NewClient("http://unused", ""), allowlist.New([]string{"mathias"}))
+	_, err := tool.Call(context.Background(), json.RawMessage(`{
+		"owner":"mathias","name":"infra","action":"add",
+		"remote_address":"https://github.com/x/y.git","remote_username":"u",
+		"remote_password_env":"DEFINITELY_UNSET_MIRROR_VAR_XYZ"
+	}`))
+	require.Error(t, err)
+	assert.ErrorIs(t, err, gitea.ErrValidation)
+}
+
 func TestRepoMirrorPushTool_Add(t *testing.T) {
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		assert.Equal(t, http.MethodPost, r.Method)

internal/tools/tbd_ship.go

@@ -171,18 +171,26 @@ func (t *TBDShip) Call(ctx context.Context, raw json.RawMessage) (json.RawMessag
 		return nil, fmt.Errorf("branch protection probe: %w", err)
 	}
 
-	// Branch from base, write the change, open the PR.
+	// Branch from base. A conflict means the branch already exists — resume on it
-	if err := t.c.CreateBranch(ctx, args.Owner, args.Repo, branch, base); err != nil {
+	// (idempotent re-invoke, #48) rather than failing.
+	if err := t.c.CreateBranch(ctx, args.Owner, args.Repo, branch, base); err != nil && !errors.Is(err, gitea.ErrConflict) {
 		return nil, fmt.Errorf("create branch %s: %w", branch, err)
 	}
 
-	// A pre-existing file at path needs its blob sha to update; a new file does not.
+	// A pre-existing file at path needs its blob sha to update; a new file does
+	// not. If the content already on the branch is identical, skip the write so a
+	// resume doesn't produce a redundant empty-diff commit.
 	sha := ""
+	needWrite := true
 	if fc, ferr := t.c.GetFileContents(ctx, args.Owner, args.Repo, args.Path, branch); ferr == nil {
 		sha = fc.Sha
+		if decoded, derr := base64.StdEncoding.DecodeString(fc.Content); derr == nil && string(decoded) == args.Content {
+			needWrite = false
+		}
 	} else if !errors.Is(ferr, gitea.ErrNotFound) {
 		return nil, fmt.Errorf("read %s on %s: %w", args.Path, branch, ferr)
 	}
+	if needWrite {
 		if _, err := t.c.UpsertFile(ctx, args.Owner, args.Repo, args.Path, gitea.UpsertFileArgs{
 			Branch:  branch,
 			Content: base64.StdEncoding.EncodeToString([]byte(args.Content)),
@@ -191,6 +199,7 @@ func (t *TBDShip) Call(ctx context.Context, raw json.RawMessage) (json.RawMessag
 		}); err != nil {
 			return nil, fmt.Errorf("write %s on %s: %w", args.Path, branch, err)
 		}
+	}
 
 	prTitle := args.PRTitle
 	if prTitle == "" {
@@ -202,9 +211,15 @@ func (t *TBDShip) Call(ctx context.Context, raw json.RawMessage) (json.RawMessag
 		Head:  branch,
 		Base:  base,
 	})
+	if err != nil {
+		// An open PR for this head already exists → resume it (#48).
+		if errors.Is(err, gitea.ErrConflict) || errors.Is(err, gitea.ErrValidation) {
+			pr, err = t.findOpenPR(ctx, args.Owner, args.Repo, branch)
+		}
 		if err != nil {
 			return nil, fmt.Errorf("open PR: %w", err)
 		}
+	}
 
 	// CI gate on the PR head commit.
 	runs := t.pollRuns(ctx, args.Owner, args.Repo, pr.Head.Sha, args.CITimeoutSec)
@@ -267,6 +282,21 @@ func (t *TBDShip) pollRuns(ctx context.Context, owner, repo, headSHA string, tim
 	}
 }
 
+// findOpenPR returns the open PR whose head is the given branch — used to resume
+// an existing PR when CreatePullRequest reports one already exists (#48).
+func (t *TBDShip) findOpenPR(ctx context.Context, owner, repo, branch string) (*gitea.PullRequest, error) {
+	prs, err := t.c.ListPullRequests(ctx, owner, repo, "open", branch, 1, 50)
+	if err != nil {
+		return nil, err
+	}
+	for i := range prs {
+		if prs[i].Head.Ref == branch {
+			return &prs[i], nil
+		}
+	}
+	return nil, fmt.Errorf("no open PR found for head %s: %w", branch, gitea.ErrNotFound)
+}
+
 func (t *TBDShip) listRuns(ctx context.Context, owner, repo, headSHA string) []gitea.WorkflowRun {
 	resp, err := t.c.ListWorkflowRuns(ctx, owner, repo, gitea.ListWorkflowRunsArgs{HeadSHA: headSHA, Limit: 50})
 	if err != nil || resp == nil {

internal/tools/tbd_ship_test.go

@@ -19,12 +19,23 @@ import (
 // shipFake serves the whole tbd_ship flow; runsJSON is the workflow_runs array
 // for the head commit, letting each test drive the CI gate.
 func shipFake(t *testing.T, runsJSON string, merged, deleted *atomic.Bool) *httptest.Server {
+	t.Helper()
+	return shipFakeOpts(t, runsJSON, 0, http.StatusOK, merged, deleted)
+}
+
+// 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 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, "/branch_protections/"):
+			if requiredApprovals > 0 {
+				_, _ = w.Write([]byte(`{"required_approvals":` + itoa(requiredApprovals) + `}`))
+				return
+			}
 			w.WriteHeader(http.StatusNotFound) // unprotected
 			_, _ = w.Write([]byte(`{"message":"not found"}`))
 		case r.Method == http.MethodPost && strings.HasSuffix(p, "/branches"):
@@ -43,7 +54,7 @@ func shipFake(t *testing.T, runsJSON string, merged, deleted *atomic.Bool) *http
 			_, _ = w.Write([]byte(`{"total_count":0,"workflow_runs":` + runsJSON + `}`))
 		case r.Method == http.MethodPost && strings.HasSuffix(p, "/pulls/7/merge"):
 			merged.Store(true)
-			w.WriteHeader(http.StatusOK)
+			w.WriteHeader(mergeStatus)
 			_, _ = w.Write([]byte(`{}`))
 		case r.Method == http.MethodDelete && strings.Contains(p, "/branches/"):
 			deleted.Store(true)
@@ -109,6 +120,90 @@ func TestTBDShip_RedCI_FailsClosed(t *testing.T) {
 	assert.False(t, merged.Load(), "merge must NOT be called when CI is red")
 }
 
+func itoa(n int) string { b, _ := json.Marshal(n); return string(b) }
+
+// Green CI but the base branch requires review → fail closed (no auto-merge).
+func TestTBDShip_ReviewProtected_FailsClosed(t *testing.T) {
+	var merged, deleted atomic.Bool
+	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"}`)
+
+	assert.Equal(t, false, res["merged"])
+	assert.NotEmpty(t, res["reason"])
+	assert.Contains(t, res["reason"], "review")
+	assert.False(t, merged.Load(), "must NOT merge a review-protected base")
+}
+
+// Green CI but the merge is unclean (409) → fail closed, PR left open.
+func TestTBDShip_MergeConflict_FailsClosed(t *testing.T) {
+	var merged, deleted atomic.Bool
+	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"}`)
+
+	assert.Equal(t, false, res["merged"])
+	assert.NotEmpty(t, res["reason"])
+	assert.True(t, merged.Load(), "merge is attempted")
+	assert.False(t, deleted.Load(), "branch is NOT deleted on a failed merge")
+}
+
+// Re-invoking with the same change must resume the existing branch/PR (not
+// error on "branch exists"), skip the redundant write when content is
+// unchanged, and merge once CI is green (#48).
+func TestTBDShip_Resume_ExistingBranchAndPR(t *testing.T) {
+	var merged, deleted, wrote atomic.Bool
+	branch := "tbd/resume-me"
+	srv := 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, "/branch_protections/"):
+			w.WriteHeader(http.StatusNotFound)
+			_, _ = w.Write([]byte(`{"message":"not found"}`))
+		case r.Method == http.MethodPost && strings.HasSuffix(p, "/branches"):
+			w.WriteHeader(http.StatusConflict) // branch already exists
+			_, _ = w.Write([]byte(`{"message":"branch already exists"}`))
+		case r.Method == http.MethodGet && strings.Contains(p, "/contents/"):
+			// existing file with identical content ("hi") → write should be skipped
+			w.WriteHeader(http.StatusOK)
+			_, _ = w.Write([]byte(`{"path":"docs/x.md","sha":"s","content":"aGk=","encoding":"base64"}`))
+		case (r.Method == http.MethodPost || r.Method == http.MethodPut) && strings.Contains(p, "/contents/"):
+			wrote.Store(true)
+			w.WriteHeader(http.StatusCreated)
+			_, _ = w.Write([]byte(`{"content":{"path":"x","sha":"s2"},"commit":{"sha":"c"}}`))
+		case r.Method == http.MethodPost && strings.HasSuffix(p, "/pulls"):
+			w.WriteHeader(http.StatusConflict) // PR already exists for this head
+			_, _ = w.Write([]byte(`{"message":"pull request already exists"}`))
+		case r.Method == http.MethodGet && strings.HasSuffix(p, "/pulls"):
+			_, _ = w.Write([]byte(`[{"number":7,"html_url":"http://x/pulls/7","state":"open","head":{"ref":"` + branch + `","sha":"abc"},"base":{"ref":"main"}}]`))
+		case r.Method == http.MethodGet && strings.Contains(p, "/actions/runs"):
+			_, _ = w.Write([]byte(`{"total_count":1,"workflow_runs":[{"id":1,"status":"completed","conclusion":"success","head_sha":"abc"}]}`))
+		case r.Method == http.MethodPost && strings.HasSuffix(p, "/pulls/7/merge"):
+			merged.Store(true)
+			w.WriteHeader(http.StatusOK)
+			_, _ = w.Write([]byte(`{}`))
+		case r.Method == http.MethodDelete && strings.Contains(p, "/branches/"):
+			deleted.Store(true)
+			w.WriteHeader(http.StatusOK)
+			_, _ = w.Write([]byte(`{}`))
+		default:
+			t.Errorf("unexpected request: %s %s", r.Method, p)
+			w.WriteHeader(http.StatusNotFound)
+		}
+	}))
+	defer srv.Close()
+
+	res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x","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")
+	assert.False(t, wrote.Load(), "identical content must not trigger a redundant write")
+	assert.True(t, merged.Load())
+}
+
 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"}`))