Merge pull request 'fix(tools): reject case-variant arg keys and ref-shaped branch names (#65)' (#66) from fix/65-strict-args-branch-names into main

PR #66 review round 2: the pairwise EqualFold loop was quadratic, 7s of
CPU for ~90k keys under the 1 MiB body cap. foldKey maps each rune to
the smallest member of its unicode.SimpleFold orbit, equal exactly when
strings.EqualFold is, so the scan is a map lookup again. The scaling
test fails on the previous code (7.1s).

Refs #65

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

.context/system-prompt.txt

@@ -78,6 +78,27 @@ These rules apply to every task across every project, regardless of harness.
    when a human reviewer outside the project is required. Document the reason in
    PROJECT.md.
 
+   **Exception — a GitOps repo where a branch cannot be reconciled.** In a repo whose
+   deployment watches one branch (`mathias/infra`: Flux watches `main`), a branch is
+   invisible to the thing that applies it. A `risk/medium` change to `alerts.yaml` or
+   `litellm-config.yaml` can be linted on a branch but never *exercised* there, so a
+   PR would buy a review of a state that was never deployed.
+
+   There, `risk/medium`+ work stays on `main` and instead requires a **recorded
+   adversarial review pass before the push** — `skills:pr-gatekeeper` or equivalent,
+   reviewing the staged diff. **The reviewer's verdict goes in the issue, not a PR
+   thread.** No verdict recorded means the lane was not used, and saying "the gate was
+   green" is not the same claim.
+
+   Be honest about what this is: a recorded *independent* pass, not four-eyes in the
+   sense `assessor-loop` §4.1 means for a regulated organisation, because the reviewer
+   is an agent. It is more than the repo had and less than the model it mirrors; do not
+   describe it as the stronger property. (infra#470, decided 2026-09-29.)
+
+   This lane depends on the `risk/` label being right. A label that triggers a heavier
+   lane is load-bearing, not cosmetic — if work that belongs in it is unlabelled, the
+   lane silently does not apply.
+
 6. **Close the loop — every substantive task ends with the same ritual.** Shipping
    the code is not the end of the task; capturing it is. Run this unprompted:
    - **Tag + bump SemVer** on the change (annotated tag; minor for a feature or
@@ -106,6 +127,43 @@ These rules apply to every task across every project, regardless of harness.
    revisit Y". Those are exactly the items that need a locator. If it is worth
    saying in a summary, it is worth writing where it can be found again without you.
 
+7. **Finish what's closable — only involve me for a genuine call.** If a
+   tracker issue, PR, or checklist item is demonstrably resolved — the fix
+   landed, the state changed, you verified it live — close/merge/check it off
+   in the same turn. Don't leave it open "for the record" or park it waiting
+   on me to confirm something that already happened; that's bookkeeping, not
+   a decision. Reserve asking me (`AskUserQuestion`, a `DECISION NEEDED`
+   comment, or otherwise pausing) for when it's genuinely mine to call:
+   irreversible or prod/host-impacting actions, a real judgment call with no
+   clearly-better option, ambiguous intent, or anything secret-writes/
+   destructive-git already gate. Before claiming something is closable,
+   verify it live (re-check the issue, re-run the command) rather than
+   trusting your own earlier summary (brain: `filed-issue-not-same-as-fixed`).
+
+8. **Control code needs an adversary, not a checklist.** Code whose job is to
+   refuse, deny, gate or contain is not adequately verified by a green gate.
+   Rule 5 makes CI the quality gate and that holds for ordinary work — but a
+   passing suite only asserts what its author already thought of, and for a
+   control the author is precisely the person whose imagination just failed.
+   Measured on `dispatch#75`: five review rounds, five real escapes verified by
+   execution, and **CI was green on every one of them** while `git rm
+   ././.dispatch-allow` deleted the file the control existed to protect. So
+   route this work through a review with authority to block, instructed to
+   verify by execution rather than by reading the implementation. Expect to be
+   blocked more than once; that is the control working, not the reviewer being
+   difficult. (brain: `a-green-gate-is-not-evidence-for-control-code`; the
+   practices live in the `pr-gatekeeper` agent.)
+
+9. **A test's name must not claim more than its assertions prove.** Name a test
+   after the mechanism it observes, not the goal it serves.
+   `DoesNotInheritEnv` is honest; `IsContained` is a claim the test cannot
+   support. This is not pedantry about naming — when name and assertion
+   diverge, the name wins in the reader's head and the gap becomes invisible,
+   which is what lets the defect survive review. Found three times in one PR,
+   twice in tests written to fix the previous instance of it; each passed while
+   the thing it was named for was exploitable. If you cannot name the mechanism,
+   you do not yet know what the test observes.
+
 ## CAD sprint patterns
 
 Two CAD sprint archetypes (defs: brain `wiki/homelab/decisions/continuous-agentic-development-cad-concept-2026-06-16`):
@@ -246,10 +304,16 @@ 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**: defaults to `public` collection; client projects filter to `{client}` + `public`.
+- **Scoping — the brain holds no confidential client content.** Nothing on the read path
+  filters by client: every token holder can read every wing, and `wing` on `brain_query`
+  is an optional argument, not an access control. Client confidentiality is enforced only
+  at write time (`capture`'s I1 gate refuses internal/confidential). So never `brain_write`
+  client material; generalise it first or keep it in the client's own repo. (hyperguild#144,
+  decided 2026-10-01. This line previously claimed client projects were filtered; they
+  were not.)
 - **Routing**: brain_answer's LLM uses berget.ai as primary, iguana ollama as
   fallback. Both are configurable in the `supervisor/ingestion-deployment.yaml`
   on the koala k3s cluster; don't hardcode local-only model names into the
@@ -319,8 +383,20 @@ Workflow:
    and missing-untracked adapters). A drift fails the check with a
    message telling you to stage the regenerated files.
 
-Behavior rules in this file and per-project rules in `PROJECT.md` apply
-unconditionally on every host, every harness.
+Behavior rules in this file and per-project rules in `PROJECT.md` apply on every
+host and every harness.
+
+**Where a project rule contradicts a rule here, the project rule governs — and it
+must state its reason.** Both halves matter: the project file wins because it knows
+something this file cannot (infra's Flux constraint is real and not visible from
+here), and it must say why because a bare override is indistinguishable from drift.
+
+This replaces the previous wording, which said rules here and in `PROJECT.md` both
+apply *"unconditionally"*. That was not a tiebreak but a contradiction, and it went
+unnoticed until rule 5 and infra's "never create feature branches" had to be obeyed
+at the same time (infra#470). A standard can hold two individually sensible clauses
+that are jointly unsatisfiable, and survive for as long as everyone reads it
+charitably — the same failure the Inception Sprint Oath's G6 had.
 
 ## Engineering Skills
 

AGENTS.md

@@ -73,6 +73,27 @@ These rules apply to every task across every project, regardless of harness.
    when a human reviewer outside the project is required. Document the reason in
    PROJECT.md.
 
+   **Exception — a GitOps repo where a branch cannot be reconciled.** In a repo whose
+   deployment watches one branch (`mathias/infra`: Flux watches `main`), a branch is
+   invisible to the thing that applies it. A `risk/medium` change to `alerts.yaml` or
+   `litellm-config.yaml` can be linted on a branch but never *exercised* there, so a
+   PR would buy a review of a state that was never deployed.
+
+   There, `risk/medium`+ work stays on `main` and instead requires a **recorded
+   adversarial review pass before the push** — `skills:pr-gatekeeper` or equivalent,
+   reviewing the staged diff. **The reviewer's verdict goes in the issue, not a PR
+   thread.** No verdict recorded means the lane was not used, and saying "the gate was
+   green" is not the same claim.
+
+   Be honest about what this is: a recorded *independent* pass, not four-eyes in the
+   sense `assessor-loop` §4.1 means for a regulated organisation, because the reviewer
+   is an agent. It is more than the repo had and less than the model it mirrors; do not
+   describe it as the stronger property. (infra#470, decided 2026-09-29.)
+
+   This lane depends on the `risk/` label being right. A label that triggers a heavier
+   lane is load-bearing, not cosmetic — if work that belongs in it is unlabelled, the
+   lane silently does not apply.
+
 6. **Close the loop — every substantive task ends with the same ritual.** Shipping
    the code is not the end of the task; capturing it is. Run this unprompted:
    - **Tag + bump SemVer** on the change (annotated tag; minor for a feature or
@@ -101,6 +122,43 @@ These rules apply to every task across every project, regardless of harness.
    revisit Y". Those are exactly the items that need a locator. If it is worth
    saying in a summary, it is worth writing where it can be found again without you.
 
+7. **Finish what's closable — only involve me for a genuine call.** If a
+   tracker issue, PR, or checklist item is demonstrably resolved — the fix
+   landed, the state changed, you verified it live — close/merge/check it off
+   in the same turn. Don't leave it open "for the record" or park it waiting
+   on me to confirm something that already happened; that's bookkeeping, not
+   a decision. Reserve asking me (`AskUserQuestion`, a `DECISION NEEDED`
+   comment, or otherwise pausing) for when it's genuinely mine to call:
+   irreversible or prod/host-impacting actions, a real judgment call with no
+   clearly-better option, ambiguous intent, or anything secret-writes/
+   destructive-git already gate. Before claiming something is closable,
+   verify it live (re-check the issue, re-run the command) rather than
+   trusting your own earlier summary (brain: `filed-issue-not-same-as-fixed`).
+
+8. **Control code needs an adversary, not a checklist.** Code whose job is to
+   refuse, deny, gate or contain is not adequately verified by a green gate.
+   Rule 5 makes CI the quality gate and that holds for ordinary work — but a
+   passing suite only asserts what its author already thought of, and for a
+   control the author is precisely the person whose imagination just failed.
+   Measured on `dispatch#75`: five review rounds, five real escapes verified by
+   execution, and **CI was green on every one of them** while `git rm
+   ././.dispatch-allow` deleted the file the control existed to protect. So
+   route this work through a review with authority to block, instructed to
+   verify by execution rather than by reading the implementation. Expect to be
+   blocked more than once; that is the control working, not the reviewer being
+   difficult. (brain: `a-green-gate-is-not-evidence-for-control-code`; the
+   practices live in the `pr-gatekeeper` agent.)
+
+9. **A test's name must not claim more than its assertions prove.** Name a test
+   after the mechanism it observes, not the goal it serves.
+   `DoesNotInheritEnv` is honest; `IsContained` is a claim the test cannot
+   support. This is not pedantry about naming — when name and assertion
+   diverge, the name wins in the reader's head and the gap becomes invisible,
+   which is what lets the defect survive review. Found three times in one PR,
+   twice in tests written to fix the previous instance of it; each passed while
+   the thing it was named for was exploitable. If you cannot name the mechanism,
+   you do not yet know what the test observes.
+
 ## CAD sprint patterns
 
 Two CAD sprint archetypes (defs: brain `wiki/homelab/decisions/continuous-agentic-development-cad-concept-2026-06-16`):
@@ -241,10 +299,16 @@ 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**: defaults to `public` collection; client projects filter to `{client}` + `public`.
+- **Scoping — the brain holds no confidential client content.** Nothing on the read path
+  filters by client: every token holder can read every wing, and `wing` on `brain_query`
+  is an optional argument, not an access control. Client confidentiality is enforced only
+  at write time (`capture`'s I1 gate refuses internal/confidential). So never `brain_write`
+  client material; generalise it first or keep it in the client's own repo. (hyperguild#144,
+  decided 2026-10-01. This line previously claimed client projects were filtered; they
+  were not.)
 - **Routing**: brain_answer's LLM uses berget.ai as primary, iguana ollama as
   fallback. Both are configurable in the `supervisor/ingestion-deployment.yaml`
   on the koala k3s cluster; don't hardcode local-only model names into the
@@ -314,8 +378,20 @@ Workflow:
    and missing-untracked adapters). A drift fails the check with a
    message telling you to stage the regenerated files.
 
-Behavior rules in this file and per-project rules in `PROJECT.md` apply
-unconditionally on every host, every harness.
+Behavior rules in this file and per-project rules in `PROJECT.md` apply on every
+host and every harness.
+
+**Where a project rule contradicts a rule here, the project rule governs — and it
+must state its reason.** Both halves matter: the project file wins because it knows
+something this file cannot (infra's Flux constraint is real and not visible from
+here), and it must say why because a bare override is indistinguishable from drift.
+
+This replaces the previous wording, which said rules here and in `PROJECT.md` both
+apply *"unconditionally"*. That was not a tiebreak but a contradiction, and it went
+unnoticed until rule 5 and infra's "never create feature branches" had to be obeyed
+at the same time (infra#470). A standard can hold two individually sensible clauses
+that are jointly unsatisfiable, and survive for as long as everyone reads it
+charitably — the same failure the Inception Sprint Oath's G6 had.
 
 ## Engineering Skills
 

internal/mcp/audit.go

@@ -0,0 +1,46 @@
+package mcp
+
+import (
+	"context"
+	"encoding/json"
+	"log/slog"
+	"time"
+
+	chassisauth "git.d-ma.be/mathias/mcp-chassis/auth"
+
+	"git.d-ma.be/mathias/gitea-mcp/internal/auth"
+	"git.d-ma.be/mathias/gitea-mcp/internal/gitea"
+)
+
+// auditTargetKeys are the only argument values copied into the audit record:
+// they identify what a call acted on without copying what it wrote. Every
+// other argument (file content, issue bodies) is reduced to args_bytes so the
+// audit trail never becomes a second copy of repo content (gitea-mcp#64).
+var auditTargetKeys = []string{"owner", "repo", "name", "number", "index", "branch", "path", "tag"}
+
+// logToolCall emits one structured record per tools/call (OWASP MCP08).
+func logToolCall(ctx context.Context, name string, args json.RawMessage, start time.Time, err error) {
+	attrs := []any{
+		"tool", name,
+		"caller", auth.Caller(ctx),
+		"args_bytes", len(args),
+		"duration_ms", time.Since(start).Milliseconds(),
+	}
+	if tok, ok := gitea.TokenFromContext(ctx); ok {
+		attrs = append(attrs, "token_fp", chassisauth.Fingerprint(tok))
+	}
+	var parsed map[string]any
+	if json.Unmarshal(args, &parsed) == nil {
+		for _, k := range auditTargetKeys {
+			if v, ok := parsed[k]; ok {
+				attrs = append(attrs, k, v)
+			}
+		}
+	}
+	outcome := "ok"
+	if err != nil {
+		outcome = err.Error()
+	}
+	attrs = append(attrs, "outcome", outcome)
+	slog.InfoContext(ctx, "mcp tool call", attrs...)
+}

internal/mcp/audit_test.go

@@ -0,0 +1,135 @@
+package mcp_test
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"errors"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"git.d-ma.be/mathias/gitea-mcp/internal/auth"
+	"git.d-ma.be/mathias/gitea-mcp/internal/mcp"
+	"git.d-ma.be/mathias/gitea-mcp/internal/registry"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+type auditStubTool struct {
+	name string
+	err  error
+}
+
+func (s auditStubTool) Descriptor() registry.ToolDescriptor {
+	return registry.ToolDescriptor{Name: s.name, InputSchema: json.RawMessage(`{"type":"object"}`)}
+}
+
+func (s auditStubTool) Call(context.Context, json.RawMessage) (json.RawMessage, error) {
+	if s.err != nil {
+		return nil, s.err
+	}
+	return json.RawMessage(`{"ok":true}`), nil
+}
+
+// captureAudit routes slog to a buffer and returns the decoded
+// "mcp tool call" records emitted while fn runs.
+func captureAudit(t *testing.T, fn func()) []map[string]any {
+	t.Helper()
+	var buf bytes.Buffer
+	prev := slog.Default()
+	slog.SetDefault(slog.New(slog.NewJSONHandler(&buf, nil)))
+	t.Cleanup(func() { slog.SetDefault(prev) })
+
+	fn()
+
+	var out []map[string]any
+	for _, l := range strings.Split(strings.TrimSpace(buf.String()), "\n") {
+		if l == "" {
+			continue
+		}
+		var rec map[string]any
+		require.NoError(t, json.Unmarshal([]byte(l), &rec))
+		if rec["msg"] == "mcp tool call" {
+			out = append(out, rec)
+		}
+	}
+	return out
+}
+
+func auditServer(tools ...registry.Tool) http.Handler {
+	reg := registry.New()
+	for _, tl := range tools {
+		reg.Register(tl)
+	}
+	srv := mcp.NewServer(mcp.ServerOptions{Registry: reg})
+	return auth.CallerMiddleware(slog.Default(), srv)
+}
+
+func callTool(t *testing.T, h http.Handler, name string, args any) {
+	t.Helper()
+	body := map[string]any{
+		"jsonrpc": "2.0", "id": 1, "method": "tools/call",
+		"params": map[string]any{"name": name, "arguments": args},
+	}
+	b, err := json.Marshal(body)
+	require.NoError(t, err)
+	req := httptest.NewRequest(http.MethodPost, "/", bytes.NewBuffer(b))
+	req.Header.Set("Content-Type", "application/json")
+	req.Header.Set("X-Auth-Request-User", "alice")
+	h.ServeHTTP(httptest.NewRecorder(), req)
+}
+
+// gitea-mcp#64: tools/call dispatched with no record at all.
+func TestToolsCallLogsOneAuditRecordWithToolCallerAndTarget(t *testing.T) {
+	recs := captureAudit(t, func() {
+		callTool(t, auditServer(auditStubTool{name: "repo_delete"}), "repo_delete",
+			map[string]any{"owner": "mathias", "repo": "scratch", "confirm": "scratch"})
+	})
+	require.Len(t, recs, 1)
+	r := recs[0]
+	assert.Equal(t, "repo_delete", r["tool"])
+	assert.Equal(t, "alice", r["caller"])
+	assert.Equal(t, "mathias", r["owner"])
+	assert.Equal(t, "scratch", r["repo"])
+	assert.Equal(t, "ok", r["outcome"])
+	assert.Contains(t, r, "duration_ms")
+}
+
+func TestToolsCallAuditRecordsErrorOutcome(t *testing.T) {
+	recs := captureAudit(t, func() {
+		callTool(t, auditServer(auditStubTool{name: "pr_merge", err: errors.New("boom")}),
+			"pr_merge", map[string]any{"owner": "mathias", "repo": "x", "number": 3})
+	})
+	require.Len(t, recs, 1)
+	assert.Equal(t, "boom", recs[0]["outcome"])
+	assert.EqualValues(t, 3, recs[0]["number"])
+}
+
+// The audit line must not become a second copy of repo content.
+func TestToolsCallAuditOmitsArgumentValuesOtherThanTarget(t *testing.T) {
+	const sentinel = "SENTINEL-FILE-CONTENT-7f3a"
+	recs := captureAudit(t, func() {
+		callTool(t, auditServer(auditStubTool{name: "file_write_branch"}), "file_write_branch",
+			map[string]any{"owner": "mathias", "repo": "x", "path": "a.txt", "content": sentinel})
+	})
+	require.Len(t, recs, 1)
+	raw, err := json.Marshal(recs[0])
+	require.NoError(t, err)
+	assert.NotContains(t, string(raw), sentinel)
+	assert.Contains(t, recs[0], "args_bytes")
+}
+
+// #38 keeps `name` (for repo) and `index` (for number) as legacy aliases; a
+// caller using them must still leave a target in the audit record.
+func TestToolsCallAuditRecordsLegacyAliasTargets(t *testing.T) {
+	recs := captureAudit(t, func() {
+		callTool(t, auditServer(auditStubTool{name: "issue_close"}), "issue_close",
+			map[string]any{"owner": "mathias", "name": "infra", "index": 7})
+	})
+	require.Len(t, recs, 1)
+	assert.Equal(t, "infra", recs[0]["name"])
+	assert.EqualValues(t, 7, recs[0]["index"])
+}

internal/mcp/server.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"errors"
 	"net/http"
+	"time"
 
 	"git.d-ma.be/mathias/gitea-mcp/internal/registry"
 )
@@ -90,7 +91,9 @@ func (s *Server) handlePOST(w http.ResponseWriter, r *http.Request) {
 				NewErrorResponse(req.ID, -32602, "invalid params", nil))
 			return
 		}
+		start := time.Now()
 		out, err := s.opts.Registry.Dispatch(r.Context(), p.Name, p.Arguments)
+		logToolCall(r.Context(), p.Name, p.Arguments, start, err)
 		if err != nil {
 			code := -32000
 			if errors.Is(err, registry.ErrToolNotFound) {

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

@@ -160,6 +160,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

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 {
-		return err
+	} 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

scripts/context-sync.sh

@@ -58,7 +58,7 @@ if [ "$FORCE" = false ]; then
     while IFS= read -r _match; do
       _lineno="${_match%%:*}"
       _content="${_match#*:}"
-      _token=$(printf '%s' "$_content" | grep -o '{{[^}]*}}' | head -1)
+      _token=$(head -1 < <(grep -o '{{[^}]*}}' <<<"$_content"))
       echo "  $PROJECT_FILE:$_lineno: unfilled placeholder $_token" >&2
     done <<< "$_placeholders"
     echo "" >&2