fix(transactions): bound the batch patch's import-id identity at the spec's 36

CHANGELOG.md

@@ -8,6 +8,14 @@ All notable changes to this module are documented here, in
 
 ### Changed
 
+- `TransactionsService.UpdateBatch` now pre-flights the import-id identity
+  of a `PatchByImportID` patch against the spec's 36-character bound and
+  answers `*ArgumentError` naming the patch index, exactly as it already
+  did for `payee_name` and `memo`. Previously an over-long key reached the
+  wire and came back a server 400. Only the import-id identity is affected:
+  keys of 36 characters or fewer, and every `PatchByID` patch, behave as
+  before — the spec declares no bound on transaction ids, so none is
+  enforced.
 - Re-vendored `openapi.yaml` from the live YNAB spec. Upstream amended one
   description — `SaveTransactionWithOptionalFields.subtransactions`, the
   shared save-transaction schema — without bumping the spec version, which

contract_wire_test.go

@@ -30,6 +30,12 @@ import (
 // transaction, subtransaction and scheduled-transaction payloads bound
 // payee_name and memo identically, and ScheduledTransactionSpec.validate
 // reuses the transaction constants rather than declaring its own.
+//
+// A row here claims enforcement but does not prove it — the table pins the
+// spec side only. Each row's teeth are a behavior test rejecting at the
+// bound (mutation-verified: moving a row to the waiver table with a bogus
+// reason passes this file; deleting the enforcement code does not pass the
+// behavior test). A new row needs its rejection test in the same commit.
 var wireBounds = map[[2]string]int{
 	{"NewTransaction", "import_id"}:                     ynab.ImportIDMax,
 	{"NewTransaction", "payee_name"}:                    ynab.TransactionPayeeNameMax,
@@ -38,6 +44,7 @@ var wireBounds = map[[2]string]int{
 	{"ExistingTransaction", "memo"}:                     ynab.MemoMax,
 	{"SaveTransactionWithOptionalFields", "payee_name"}: ynab.TransactionPayeeNameMax,
 	{"SaveTransactionWithOptionalFields", "memo"}:       ynab.MemoMax,
+	{"SaveTransactionWithIdOrImportId", "import_id"}:    ynab.ImportIDMax,
 	{"SaveTransactionWithIdOrImportId", "payee_name"}:   ynab.TransactionPayeeNameMax,
 	{"SaveTransactionWithIdOrImportId", "memo"}:         ynab.MemoMax,
 	{"SaveSubTransaction", "payee_name"}:                ynab.TransactionPayeeNameMax,
@@ -53,14 +60,10 @@ var wireBounds = map[[2]string]int{
 // with the reason. A bound belongs here rather than in wireBounds only when
 // leaving it unchecked is a decision someone made; the completeness
 // assertion accepts either table, so the spec can never declare a bound
-// that goes entirely unremarked.
-var wireBoundsUnenforced = map[[2]string]string{
-	{"SaveTransactionWithIdOrImportId", "import_id"}: "PatchByImportID's key reaches the wire " +
-		"unchecked: TransactionPatch embeds TransactionUpdate, which has no ImportID field, so " +
-		"UpdateBatch validates everything except this. A >36-character key comes back a server " +
-		"400 instead of an *ArgumentError. Tracked as a follow-up; enforcing it is a behavior " +
-		"change and belongs in its own commit.",
-}
+// that goes entirely unremarked. Currently empty: the last waiver —
+// SaveTransactionWithIdOrImportId.import_id, PatchByImportID's identity —
+// was lifted when TransactionPatch.validate started bounding the key.
+var wireBoundsUnenforced = map[[2]string]string{}
 
 // wireEnums maps every enum the spec declares on a NAMED schema to the Go
 // enum type that mirrors it. The Go members come from the same AST scan

transactions.go

@@ -536,15 +536,28 @@ func PatchByID(id string, update TransactionUpdate) TransactionPatch {
 }
 
 // PatchByImportID addresses a batch update by import id (lookup only —
-// changing an import id is not allowed by the API). Live caveat: the
-// server resolves the lookup only for transactions that entered through
-// the import pipeline (linked accounts); an API-created transaction
-// carrying the same import_id answers 400 "transaction does not exist"
-// (probed live 2026-07-20 — see API_NOTES.md).
+// changing an import id is not allowed by the API). The key is bounded at
+// 36 characters, the same spec bound Create enforces on
+// [TransactionSpec.ImportID]. Live caveat: the server resolves the lookup
+// only for transactions that entered through the import pipeline (linked
+// accounts); an API-created transaction carrying the same import_id
+// answers 400 "transaction does not exist" (probed live 2026-07-20 — see
+// API_NOTES.md).
 func PatchByImportID(importID string, update TransactionUpdate) TransactionPatch {
 	return TransactionPatch{importID: importID, TransactionUpdate: update}
 }
 
+// validate applies the embedded update's bounds plus the one the identity
+// carries: the spec bounds import_id at 36 characters on batch patches
+// exactly as it does at creation. The id identity is deliberately not
+// bounded — the spec declares no maxLength on it.
+func (p TransactionPatch) validate(op string) error {
+	if err := checkRuneMax(op, "import_id", p.importID, importIDMax); err != nil {
+		return err
+	}
+	return p.TransactionUpdate.validate(op)
+}
+
 // MarshalJSON emits the update fields plus exactly one identity key.
 func (p TransactionPatch) MarshalJSON() ([]byte, error) {
 	raw, err := json.Marshal(p.TransactionUpdate)

transactions_test.go

@@ -7,6 +7,7 @@ package ynab_test
 import (
 	"bytes"
 	"encoding/json"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -233,3 +234,53 @@ func TestUpdateBatchAnnotatesPatchIndex(t *testing.T) {
 	require.ErrorAs(t, err, &argErr)
 	require.Contains(t, argErr.Reason, "(patch 1)", "the failing element must be named, like CreateBatch's (spec N)")
 }
+
+func TestUpdateBatchBoundsImportIDIdentity(t *testing.T) {
+	t.Parallel()
+
+	srv := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
+		t.Error("no request must be sent on a pre-flight failure")
+	}))
+	t.Cleanup(srv.Close)
+	client := ynab.New("t", ynab.WithBaseURL(srv.URL), ynab.WithRetryDisabled())
+
+	// 37 code points: one past the spec's 36. The bound counts runes, not
+	// bytes — ééé… would be 74 bytes at 37 runes and must fail identically.
+	_, err := client.Plan("p-1").Transactions.UpdateBatch(t.Context(), []ynab.TransactionPatch{
+		ynab.PatchByID("tr1", ynab.TransactionUpdate{Memo: ynab.Set("ok")}),
+		ynab.PatchByImportID(strings.Repeat("é", 37), ynab.TransactionUpdate{}),
+	})
+	var argErr *ynab.ArgumentError
+	require.ErrorAs(t, err, &argErr)
+	require.Equal(t, "import_id", argErr.Field)
+	require.Contains(t, argErr.Reason, "(patch 1)")
+}
+
+func TestUpdateBatchIDIdentityIsUnbounded(t *testing.T) {
+	t.Parallel()
+
+	// The spec declares maxLength on import_id only; a long transaction id
+	// must reach the wire. The fake captures the body, because "unbounded"
+	// means sent intact — a request merely going out would also pass if a
+	// refactor silently dropped or truncated the identity.
+	bodyCh := make(chan []byte, 1)
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		b, _ := io.ReadAll(r.Body)
+		bodyCh <- b
+		_, _ = w.Write([]byte(`{"data":{"transactions":[],"transaction_ids":[],` +
+			`"duplicate_import_ids":[],"server_knowledge":1}}`))
+	}))
+	t.Cleanup(srv.Close)
+	client := ynab.New("t", ynab.WithBaseURL(srv.URL), ynab.WithRetryDisabled())
+
+	_, err := client.Plan("p-1").Transactions.UpdateBatch(t.Context(), []ynab.TransactionPatch{
+		ynab.PatchByID(strings.Repeat("x", 100), ynab.TransactionUpdate{}),
+		// And the boundary itself: exactly 36 runes of import_id pass.
+		ynab.PatchByImportID(strings.Repeat("é", 36), ynab.TransactionUpdate{}),
+	})
+	require.NoError(t, err)
+
+	body := string(<-bodyCh)
+	require.Contains(t, body, strings.Repeat("x", 100), "the unbounded id must reach the wire intact")
+	require.Contains(t, body, strings.Repeat("é", 36), "the boundary import_id must reach the wire intact")
+}