fix(schema): a grandfathered reserved-key FieldDef never reaches an item-field path (BUG-2685)

`validateNoReservedFieldKeys` (BUG-2674) stops a collection schema NEWLY
declaring `implementation_notes` / `decision_log` / `github_pr` / `convention`,
but deliberately grandfathers an existing declaration — so the shape survives in
any workspace that had it before that gate landed, and every path that reasons
about fields BY CONSULTING A SCHEMA then meets a FieldDef it should not honour.

Such a declaration is wrong in both directions at once, which is what rules out
refusing the write:

  - it REFUSES the system's own value. `pad item note` / `pad item decide`
    append an entry and PATCH the whole fields blob back; a legacy
    `implementation_notes: text` fails that ARRAY on validateFieldType, so the
    collection's items cannot be annotated at all.
  - it ACCEPTS a caller's junk. The same declaration waves a bare string
    through, where the append helpers then refuse to touch it (BUG-2627 part 3).

And a legacy `required` declaration refuses ordinary creates over a field no
client surface offers.

ONE PREDICATE, AT THE DECODE. A type-resolved pass over the tree counted 76
sites that consult a `CollectionSchema.Fields` and only 38 that DECODE one. A
rule applied at the consultations is 76 patches that every future consumer has
to remember, which is the shape of the four review rounds this bug was filed out
of — each found another site. Every consultation inherits whatever the decode
produced, so `models.UnmarshalItemFieldSchema` strips reserved FieldDefs there,
and 23 item-field decode sites now call it. 9 move/copy decodes already strip at
the point of use via `items.SchemaForMigratedFields` (whose implementation now
delegates to the same strip, so there is one). 6 keep the raw schema because the
collection DEFINITION is the subject — notably the grandfather test's own
`prevSchema`, which stripped would reclassify every existing declaration as
newly introduced and refuse every update to such a collection.

AND AN ENFORCEMENT, TYPE-RESOLVED. A predicate nothing enforces is a convention
the next consumer has to remember. `TestReservedFieldDecodeSitesAreClassified`
walks the tree with go/packages and requires every raw decode into a
SCHEMA-SHAPED type to be on an allow-list WITH A REASON; a sibling test rejects
dead entries, and a third pins the site COUNT so the guard cannot go blind while
passing. A destination counts by SHAPE — a struct with a `json:"fields"` member
that is a slice of structs — not by name, which is what catches `bootstrapSchema`
for what it IS rather than through a registry of mirrors.

The type checker was earned, not chosen. The first guard approximated types by
reading declarations with go/ast, and FIVE consecutive review rounds each named
more DESTINATION SPELLINGS it could not see: a bare pointer parameter; an
inferred-type var; new(T); an aliased models import, `var x = new(T)`,
`dst := &s`, an inline element type; a type alias, `dst := helper()`, a closure
parameter. Fixing them one at a time was measuring how many spellings a reviewer
could think of rather than whether the guard was sound (CONVE-35). go/packages
answers the question the approximation was approximating, including for the
spellings nobody has thought of yet.

Cost, measured rather than predicted: `go mod tidy` moves golang.org/x/tools
from go.mod's indirect block to its direct one and leaves go.sum BYTE-IDENTICAL
— the module set does not change, so nix/package.nix's vendorHash is unaffected.
x/tools was already in the graph with its full module zip hash in go.sum.

The 17-spelling fixture under internal/models/testdata/decodeshapes outlived the
guard it was written for and is kept as a REGRESSION TEST on the resolver: it
fails if a future change to the shape test, the callee test, or the
destination-argument rule narrows what can be resolved. It earned its keep
immediately — it caught that a Go type ALIAS materialises as *types.Alias and
walked straight past an unwrap loop that handled only Pointer and Named.

The guard skips VENDORED paths, and that is not a tidiness rule. It passed
locally and FAILED the Nix job, which builds from a `vendor/` tree that `./...`
then walks: 57 sites instead of 20, every extra one a dependency's own decode
caught by the interface arm. A guard whose verdict depends on whether the
checkout happens to be vendored is not a guard. Reproduced locally with
`go mod vendor` before the fix and re-run after, rather than inferred from the
CI log.

Out of scope, stated rather than implied: a full `fields` blob reaching item
CREATE is still a mint site for a hand-written reserved key (undeclared keys are
ACCEPTED — BUG-2850 chose accept-and-name after a census found 168 live values
under 14 such keys). That is a different class, whose remedy is refusal and
whose risk is the system writers; a test pins that this change does not silently
alter it. Collapsing the now-idempotent `SchemaForMigratedFields` calls is left
alone too — the copy preflight distinguishes the raw source schema from the
stripped one in its reporting, so that is a second change with its own surface.

Claude-Session: https://claude.ai/code/session_019EmDd3Wy8b3cPG1TZBSzBu

cmd/pad/cmd_item.go

@@ -253,7 +253,7 @@ Run with --help-collections to see available collections and their status values
 			if coll, err := cli.WithCollectionAliasFallback(rawSlug, nil, func(slug string) (*models.Collection, error) {
 				return client.GetCollection(ws, slug)
 			}); err == nil {
-				_ = json.Unmarshal([]byte(coll.Schema), &collSchema)
+				_ = models.UnmarshalItemFieldSchema([]byte(coll.Schema), &collSchema)
 			}
 			for _, kv := range fieldFlags {
 				key, val, ferr := items.SplitFieldEntry(kv)
@@ -1197,7 +1197,7 @@ Examples:
 				var collSchema models.CollectionSchema
 				if item.CollectionSlug != "" {
 					if coll, err := client.GetCollection(ws, item.CollectionSlug); err == nil {
-						_ = json.Unmarshal([]byte(coll.Schema), &collSchema)
+						_ = models.UnmarshalItemFieldSchema([]byte(coll.Schema), &collSchema)
 					}
 				}
 				for _, kv := range fieldFlags {
@@ -1860,7 +1860,7 @@ Examples:
 				Schema: func(wsSlug, collSlug string) models.CollectionSchema {
 					var schema models.CollectionSchema
 					if coll, err := client.GetCollection(wsSlug, collSlug); err == nil {
-						_ = json.Unmarshal([]byte(coll.Schema), &schema)
+						_ = models.UnmarshalItemFieldSchema([]byte(coll.Schema), &schema)
 					}
 					return schema
 				},

go.mod

@@ -47,6 +47,7 @@ require (
 	golang.org/x/term v0.45.0
 	golang.org/x/text v0.41.0
 	golang.org/x/time v0.16.0
+	golang.org/x/tools v0.48.0
 	modernc.org/sqlite v1.58.0
 )
 
@@ -134,7 +135,6 @@ require (
 	golang.org/x/net v0.58.0 // indirect
 	golang.org/x/oauth2 v0.36.0 // indirect
 	golang.org/x/sync v0.22.0 // indirect
-	golang.org/x/tools v0.48.0 // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20260803160001-6ac0973c030d // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20260803160001-6ac0973c030d // indirect
 	google.golang.org/grpc v1.83.2 // indirect

internal/items/migrate.go

@@ -257,6 +257,12 @@ func StillDropped(dropped []string, finalFields map[string]any) []string {
 // every move and copy for a collection whose only sin is a field name someone
 // was once allowed to pick.
 //
+// The strip itself now lives in models.SchemaWithoutReservedFields, which
+// BUG-2685 made the one predicate for every consumer that reasons about an
+// ITEM's fields rather than only for migration's output. This name stays
+// because its CALLERS are about migration and its doc is where the
+// migration-specific reasoning above belongs; there is one implementation.
+//
 // Returns schema unchanged (no copy) when it declares no reserved key, which is
 // every collection that has not been grandfathered — so the returned Fields
 // slice may ALIAS the caller's. That is deliberate: this is called on a hot
@@ -265,24 +271,5 @@ func StillDropped(dropped []string, finalFields map[string]any) []string {
 // read-only — treat the result as immutable. Every current caller passes it
 // straight to a validator or an enumeration and none mutates it.
 func SchemaForMigratedFields(schema models.CollectionSchema) models.CollectionSchema {
-	var reserved bool
-	for _, f := range schema.Fields {
-		if models.IsReservedItemField(f.Key) {
-			reserved = true
-			break
-		}
-	}
-	if !reserved {
-		return schema
-	}
-
-	out := schema
-	out.Fields = make([]models.FieldDef, 0, len(schema.Fields))
-	for _, f := range schema.Fields {
-		if models.IsReservedItemField(f.Key) {
-			continue
-		}
-		out.Fields = append(out.Fields, f)
-	}
-	return out
+	return models.SchemaWithoutReservedFields(schema)
 }

internal/mcp/dispatch_http_advanced.go

@@ -645,7 +645,7 @@ func (d *HTTPHandlerDispatcher) collectionSchemaShadowsParent(
 		// failed GetCollection there leaves collSchema at its zero value and
 		// --field values just stay untyped strings instead of blocking the
 		// update).
-		_ = json.Unmarshal([]byte(coll.Schema), &schema)
+		_ = models.UnmarshalItemFieldSchema([]byte(coll.Schema), &schema)
 	}
 	for _, f := range schema.Fields {
 		if f.Key == "parent" || f.Key == "plan" {

internal/models/collection_schema_item_fields.go

@@ -0,0 +1,89 @@
+package models
+
+import "encoding/json"
+
+// SchemaWithoutReservedFields returns schema with any reserved-metadata
+// FieldDef removed — the schema as a consumer that reasons about an ITEM's
+// field values must see it (BUG-2685).
+//
+// WHY A GRANDFATHERED DECLARATION IS IGNORED RATHER THAN REFUSED.
+// `validateNoReservedFieldKeys` (BUG-2674) stops a schema NEWLY declaring one
+// of these keys but deliberately grandfathers an existing declaration, so the
+// shape survives wherever it predates that gate. Such a declaration is wrong in
+// BOTH directions at once, which is what rules out refusing the write:
+//
+//   - it REFUSES the system's own value — `pad item note` / `pad item decide`
+//     append an entry and PATCH the whole fields blob back, and a legacy
+//     `implementation_notes: text` declaration fails that ARRAY on
+//     validateFieldType, so the collection's items cannot be annotated at all;
+//   - it ACCEPTS a caller's junk — the same declaration waves a bare string
+//     through into the key, where the append helpers then refuse to touch it
+//     (BUG-2627 part 3).
+//
+// Refusing would also break every system writer on such a collection, and a
+// required legacy declaration already refuses ordinary creates over a field no
+// client surface offers. Ignoring it restores the invariant these keys were
+// always supposed to have: they are system-owned, and no collection schema
+// speaks for them.
+//
+// Returns schema unchanged (no copy) when it declares no reserved key, which is
+// every collection that has not been grandfathered — so the returned Fields
+// slice may ALIAS the caller's. The contract is therefore read-only: treat the
+// result as immutable.
+func SchemaWithoutReservedFields(schema CollectionSchema) CollectionSchema {
+	var reserved bool
+	for _, f := range schema.Fields {
+		if IsReservedItemField(f.Key) {
+			reserved = true
+			break
+		}
+	}
+	if !reserved {
+		return schema
+	}
+
+	out := schema
+	out.Fields = make([]FieldDef, 0, len(schema.Fields))
+	for _, f := range schema.Fields {
+		if IsReservedItemField(f.Key) {
+			continue
+		}
+		out.Fields = append(out.Fields, f)
+	}
+	return out
+}
+
+// UnmarshalItemFieldSchema decodes a collection's stored schema JSON for a
+// consumer that reasons about an ITEM's field values, applying
+// SchemaWithoutReservedFields to the result.
+//
+// IT IS THE ONE PREDICATE, and it sits at the DECODE rather than at each use on
+// purpose. BUG-2685's instrument counted 76 sites that consult a
+// `CollectionSchema.Fields` and only 38 that decode one: a rule applied at the
+// consultations is 76 patches that every future consumer has to remember, which
+// is the shape of the last four review rounds this bug was filed out of. Every
+// consultation inherits whatever the decode produced, so the decode is where
+// one rule covers all of them.
+//
+// Signature mirrors json.Unmarshal's deliberately — same argument order, same
+// error semantics — so converting a call site is a one-token edit and the
+// remaining raw decodes stay greppable. TestReservedFieldDecodeSitesAreClassified
+// enforces that: a NEW raw decode into a CollectionSchema fails the build unless
+// it is added to that test's allow-list of collection-definition sites, with a
+// reason.
+//
+// NOT for the sites where the collection DEFINITION is the subject — schema
+// create/update input, the grandfather test's own `prevSchema`, and the CLI /
+// MCP paths that render or accept a schema. There the declaration is the thing
+// being talked about, and stripping it would be wrong rather than merely
+// unnecessary: feeding a stripped `prevSchema` to validateNoReservedFieldKeys
+// would reclassify every existing declaration as newly introduced and refuse
+// every update to such a collection.
+func UnmarshalItemFieldSchema(data []byte, dst *CollectionSchema) error {
+	var schema CollectionSchema
+	if err := json.Unmarshal(data, &schema); err != nil {
+		return err
+	}
+	*dst = SchemaWithoutReservedFields(schema)
+	return nil
+}

internal/models/reserved_field_decode_guard_test.go

@@ -0,0 +1,610 @@
+package models_test
+
+import (
+	"fmt"
+	"go/ast"
+	"go/types"
+	"os"
+	"path/filepath"
+	"reflect"
+	"sort"
+	"strings"
+	"sync"
+	"testing"
+
+	"golang.org/x/tools/go/packages"
+)
+
+// BUG-2685's enforcement half.
+//
+// The fix is one predicate — models.UnmarshalItemFieldSchema — applied at every
+// site that decodes a collection schema FOR ITEM-FIELD PURPOSES. A predicate
+// nothing enforces is a convention the next consumer has to remember, and the
+// last four review rounds this bug was filed out of are what remembering looks
+// like in practice: each round found another site.
+//
+// So this test enumerates every raw `json.Unmarshal` into a models.
+// CollectionSchema in the tree and requires each to be listed below WITH A
+// REASON. A new item-field consumer that decodes raw fails here and is told
+// which call to use; a genuine collection-definition site is added to the map
+// and the reason is the thing review argues about.
+//
+// The allow-list is deliberately positional (file + the identifier written to)
+// rather than a whole-file exemption: a file may hold both kinds, and
+// handlers_collections.go does.
+//
+// IT IS TYPE-RESOLVED, and that was earned rather than chosen. The first
+// version approximated types by reading declarations with go/ast, and FIVE
+// consecutive codex rounds each named more DESTINATION SPELLINGS it could not
+// see — a bare pointer parameter; an inferred-type var; new(T); an aliased
+// models import, `var x = new(T)`, `dst := &s`, an inline element type; a type
+// alias, `dst := helper()`, a closure parameter. That loop was measuring how
+// many spellings a reviewer could think of, not whether the guard was sound
+// (CONVE-35). go/packages answers the question the approximation was
+// approximating: what is the TYPE of this expression. No spelling escapes it,
+// including the ones nobody has thought of yet.
+//
+// It also removes the need for a registry of parallel schema structs. A
+// destination counts when its type is SCHEMA-SHAPED — a struct with a
+// `json:"fields"` member that is a slice of structs — so `bootstrapSchema`,
+// which is a hand-kept mirror of CollectionSchema under a different name and
+// was invisible to a pass keyed on the canonical type, is caught by what it IS.
+
+// rawDecodeAllowed maps "path::destination" to why that site decodes the schema
+// RAW. Every entry is a site where the collection DEFINITION is the subject, so
+// stripping the declaration would be wrong rather than merely unnecessary.
+var rawDecodeAllowed = map[string]string{
+	"internal/models/collection_schema_item_fields.go::schema": "UnmarshalItemFieldSchema IS the predicate — it decodes raw and then strips, which is the whole point of it",
+
+	// INTERFACE destinations. The type checker cannot see what these decode
+	// into, so they are reported rather than assumed harmless; each is a
+	// GENERIC decode helper whose callers choose the concrete type, and none
+	// of them can be converted without breaking every other thing they decode.
+	// A new one shows up here and has to be reasoned about, which is the point.
+	"internal/cli/client.go::result":           "generic HTTP response decoder: the caller supplies the destination, so this call has no schema to strip — a caller that wants an item-field schema must decode one itself",
+	"internal/oauth/storage.go::session":       "fosite session hydration: decodes an OAuth session blob, never a collection schema",
+	"internal/server/server.go::v":             "the shared request-body decoder (BUG-2803): decodes a handler's own input struct, and a handler taking a schema goes through the collection-definition doors above",
+	"internal/server/handlers_bootstrap.go::s": "trimRedundantSchemaLabels decodes into the parallel bootstrapSchema struct and strips reserved keys in the same loop — the parallel-struct twin of UnmarshalItemFieldSchema, and it cannot call it because the whole point of that struct is a different FieldDef shape",
+
+	"internal/server/handlers_collections.go::schema":     "collection create/update INPUT — the declaration is what is being validated, by validateNoReservedFieldKeys",
+	"internal/server/handlers_collections.go::prevSchema": "the grandfather test's own baseline: a stripped prevSchema would reclassify every EXISTING declaration as newly introduced and refuse every update to such a collection",
+	"cmd/pad/cmd_collection.go::schema":                   "CLI --schema parse and the collection list's own status-option rendering — the collection's definition, not any item's fields",
+	"internal/mcp/dispatch_http_routes.go::schema":        "MCP collection schema INPUT, the remote twin of handlers_collections.go",
+
+	// The move / copy family. These decode raw and strip at the point of use
+	// via items.SchemaForMigratedFields (BUG-2674), which is the same
+	// predicate; they are listed rather than converted because the preflight
+	// distinguishes the raw source schema from the stripped one in its
+	// reporting, and collapsing the two is a separate change with its own
+	// regression surface. See the BUG-2685 trail.
+	"internal/server/handlers_items.go::sourceSchema":                "move: stripped at use by items.SchemaForMigratedFields",
+	"internal/server/handlers_items.go::targetSchema":                "move: stripped at use by items.SchemaForMigratedFields",
+	"internal/server/handlers_items_bulk.go::sourceSchema":           "bulk move: stripped at use by items.SchemaForMigratedFields",
+	"internal/server/handlers_items_bulk.go::targetSchema":           "bulk move: stripped at use by items.SchemaForMigratedFields",
+	"internal/server/handlers_items_copy.go::targetSchema":           "copy: stripped at use by items.SchemaForMigratedFields",
+	"internal/server/handlers_items_copy_preflight.go::sourceSchema": "copy preflight: the RAW source schema is the subject of its dropped-key reporting",
+	"internal/server/handlers_items_copy_preflight.go::targetSchema": "copy preflight: stripped at use by items.SchemaForMigratedFields",
+	"internal/store/items_cross_workspace_copy.go::sourceSchema":     "store copy: the RAW source schema is the subject of its dropped-key reporting",
+	"internal/store/items_cross_workspace_copy.go::targetSchema":     "store copy: stripped at use by items.SchemaForMigratedFields",
+}
+
+func repoRoot(t *testing.T) string {
+	t.Helper()
+	dir, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("getwd: %v", err)
+	}
+	for {
+		if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil {
+			return dir
+		}
+		parent := filepath.Dir(dir)
+		if parent == dir {
+			t.Fatal("no go.mod above the test's working directory")
+		}
+		dir = parent
+	}
+}
+
+type decodeSite struct {
+	path string
+	line int
+	dest string
+}
+
+func (s decodeSite) key() string { return s.path + "::" + s.dest }
+
+// schemaShaped reports whether t is, under its pointers and named types, a
+// struct carrying a `json:"fields"` member that is a slice of structs — the
+// shape of a collection schema.
+//
+// BY SHAPE, NOT BY NAME, for the reason the bootstrap hole taught: a mirror
+// struct under a different name is the same contract, and a guard keyed on the
+// canonical type cannot see one. An ITEM's `fields` is its VALUES — a string, a
+// json.RawMessage, a map, a named struct — and there are seven of those in the
+// tree, all excluded by the slice-of-structs test.
+func schemaShaped(t types.Type) bool {
+	for i := 0; i < 8; i++ {
+		// types.Unalias FIRST: since Go 1.24 a type alias materialises as
+		// *types.Alias rather than resolving to the aliased Named, so a loop
+		// that only unwraps Pointer and Named stops dead on
+		// `type S = models.CollectionSchema`. Found by the fixture, which is
+		// exactly the job it was kept for.
+		t = types.Unalias(t)
+		switch u := t.(type) {
+		case *types.Pointer:
+			t = u.Elem()
+			continue
+		case *types.Named:
+			t = u.Underlying()
+			continue
+		}
+		break
+	}
+	st, ok := t.(*types.Struct)
+	if !ok {
+		return false
+	}
+	for i := 0; i < st.NumFields(); i++ {
+		// An EMBEDDED schema carries the same JSON members at the same level,
+		// so a struct embedding models.CollectionSchema decodes a schema
+		// declaration exactly as the schema itself does.
+		if st.Field(i).Embedded() && schemaShaped(st.Field(i).Type()) {
+			return true
+		}
+		if !jsonNameIsFields(st.Field(i).Name(), st.Tag(i)) {
+			continue
+		}
+		ft := st.Field(i).Type()
+		for j := 0; j < 8; j++ {
+			ft = types.Unalias(ft)
+			if n, ok := ft.(*types.Named); ok {
+				ft = n.Underlying()
+				continue
+			}
+			break
+		}
+		sl, ok := ft.(*types.Slice)
+		if !ok {
+			continue
+		}
+		el := sl.Elem()
+		for j := 0; j < 8; j++ {
+			el = types.Unalias(el)
+			// `[]*models.FieldDef` decodes the same JSON as `[]models.FieldDef`.
+			if pt, ok := el.(*types.Pointer); ok {
+				el = pt.Elem()
+				continue
+			}
+			if n, ok := el.(*types.Named); ok {
+				el = n.Underlying()
+				continue
+			}
+			break
+		}
+		if _, ok := el.(*types.Struct); ok {
+			return true
+		}
+	}
+	return false
+}
+
+// jsonNameIsFields reports whether a struct field is serialised as `fields`.
+//
+// encoding/json matches an UNTAGGED field by NAME, case-insensitively, so
+// `struct{ Fields []models.FieldDef }` decodes the same JSON as the tagged
+// version and is the same schema (codex round 12). Requiring the tag was a
+// property of how the shape test was written rather than of what JSON does.
+func jsonNameIsFields(name, tag string) bool {
+	if v, ok := reflect.StructTag(tag).Lookup("json"); ok {
+		jsonName := v
+		if idx := strings.Index(v, ","); idx >= 0 {
+			jsonName = v[:idx]
+		}
+		if jsonName == "-" {
+			return false
+		}
+		if jsonName != "" {
+			return jsonName == "fields"
+		}
+		// `json:",omitempty"` keeps the field NAME; fall through.
+	}
+	return strings.EqualFold(name, "fields")
+}
+
+// isVendored reports whether a repo-relative path lies under any vendor/
+// directory. Checked per SEGMENT rather than as a prefix, so a nested vendor
+// tree is excluded too and a legitimate path like `internal/vendorimport.go` is
+// not.
+func isVendored(rel string) bool {
+	for _, seg := range strings.Split(filepath.ToSlash(rel), "/") {
+		if seg == "vendor" {
+			return true
+		}
+	}
+	return false
+}
+
+// destName is the allow-list key's second half: the name a reader would use for
+// the destination. Derived from the expression rather than from its type, so the
+// key stays the one a person would write down.
+func destName(e ast.Expr) string {
+	switch x := e.(type) {
+	case *ast.UnaryExpr:
+		return destName(x.X)
+	case *ast.StarExpr:
+		return destName(x.X)
+	case *ast.Ident:
+		return x.Name
+	case *ast.SelectorExpr:
+		return x.Sel.Name
+	case *ast.IndexExpr:
+		return destName(x.X)
+	case *ast.CallExpr:
+		return destName(x.Fun)
+	case *ast.CompositeLit:
+		if x.Type != nil {
+			return destName(x.Type)
+		}
+	}
+	return "<expr>"
+}
+
+// collectSchemaDecodes returns every RAW decode whose destination is
+// schema-shaped: a call to encoding/json.Unmarshal, or to a Decode method on an
+// encoding/json Decoder, whose last argument has such a type.
+//
+// Calls to models.UnmarshalItemFieldSchema are not decodes for this purpose —
+// that IS the predicate — and are excluded by callee identity rather than by
+// name matching.
+// loadCache memoises the type-checked walk. Four tests ask the same question of
+// the same tree and a full go/packages load is ~5s each; without this the guard
+// alone is most of the package's runtime.
+var (
+	loadMu    sync.Mutex
+	loadCache = map[string][]decodeSite{}
+)
+
+func collectSchemaDecodes(t *testing.T, dir string, patterns ...string) []decodeSite {
+	t.Helper()
+	if len(patterns) == 0 {
+		patterns = []string{"./..."}
+	}
+	cacheKey := dir + "|" + strings.Join(patterns, ",")
+	loadMu.Lock()
+	if cached, ok := loadCache[cacheKey]; ok {
+		loadMu.Unlock()
+		return cached
+	}
+	loadMu.Unlock()
+	cfg := &packages.Config{
+		Mode: packages.NeedName | packages.NeedFiles | packages.NeedSyntax |
+			packages.NeedTypes | packages.NeedTypesInfo | packages.NeedDeps |
+			packages.NeedImports,
+		Dir:   dir,
+		Tests: false,
+	}
+	pkgs, err := packages.Load(cfg, patterns...)
+	if err != nil {
+		t.Fatalf("load: %v", err)
+	}
+	var loadErrs []string
+	packages.Visit(pkgs, nil, func(p *packages.Package) {
+		for _, e := range p.Errors {
+			loadErrs = append(loadErrs, p.PkgPath+": "+e.Error())
+		}
+	})
+	if len(loadErrs) > 0 {
+		// A guard that certifies a tree it could not type-check certifies
+		// nothing — this is the "went blind while passing" failure with the
+		// type checker's own voice.
+		t.Fatalf("the tree did not type-check, so this guard proves nothing:\n  %s",
+			strings.Join(loadErrs[:min(len(loadErrs), 5)], "\n  "))
+	}
+
+	root := dir
+	var out []decodeSite
+	seen := map[string]bool{}
+	packages.Visit(pkgs, nil, func(p *packages.Package) {
+		if p.TypesInfo == nil {
+			return
+		}
+		for _, f := range p.Syntax {
+			pos := p.Fset.Position(f.Pos())
+			if strings.HasSuffix(pos.Filename, "_test.go") {
+				continue
+			}
+			rel, err := filepath.Rel(root, pos.Filename)
+			if err != nil || strings.HasPrefix(rel, "..") {
+				continue // outside the tree under test (a dependency)
+			}
+			// VENDORED code is not ours to classify, and it is not hypothetical:
+			// this guard passed locally and FAILED the Nix job, which builds
+			// from a `vendor/` tree that `./...` then walks. 57 sites instead of
+			// 20, every extra one a dependency's own decode caught by the
+			// interface-destination arm. A guard whose verdict depends on
+			// whether the checkout happens to be vendored is not a guard.
+			if isVendored(rel) {
+				continue
+			}
+			ast.Inspect(f, func(n ast.Node) bool {
+				call, ok := n.(*ast.CallExpr)
+				if !ok || len(call.Args) == 0 {
+					return true
+				}
+				if !isDecodeCall(call, p.TypesInfo) {
+					return true
+				}
+				// The DESTINATION is the last argument for both
+				// json.Unmarshal(data, dst) and dec.Decode(dst). The source
+				// argument routinely mentions a schema too.
+				arg := call.Args[len(call.Args)-1]
+				tv, ok := p.TypesInfo.Types[arg]
+				if !ok {
+					return true
+				}
+				// An INTERFACE destination hides its concrete type from the
+				// type checker — `var dst any = &schema` decodes a schema and
+				// resolves as `any`. Tracking it properly is dataflow analysis;
+				// instead every such decode is REPORTED for classification.
+				// There are three in the tree, so the cost is three allow-list
+				// entries and the hole is closed rather than documented.
+				//
+				// A POINTER to an interface (`json.Unmarshal(b, &v)` where v is
+				// `any`) is DELIBERATELY NOT included, and the number is the
+				// reason: including it takes the population from 20 to 38, and
+				// the 18 additions are all the ordinary decode-arbitrary-JSON
+				// idiom. Eighteen entries whose reason is the same sentence is
+				// an allow-list nobody reads, which is a worse guard than the
+				// hole. The residual needs someone to place a schema POINTER
+				// inside an `any` and then pass its address — a shape no site
+				// in this tree uses and no converted site could become without
+				// the conversion being visible in the same diff.
+				if !schemaShaped(tv.Type) && !types.IsInterface(tv.Type) {
+					return true
+				}
+				cpos := p.Fset.Position(call.Pos())
+				s := decodeSite{
+					path: filepath.ToSlash(rel),
+					line: cpos.Line,
+					dest: destName(arg),
+				}
+				k := fmt.Sprintf("%s:%d:%s", s.path, s.line, s.dest)
+				if seen[k] {
+					return true
+				}
+				seen[k] = true
+				out = append(out, s)
+				return true
+			})
+		}
+	})
+	sort.Slice(out, func(i, j int) bool {
+		if out[i].path != out[j].path {
+			return out[i].path < out[j].path
+		}
+		if out[i].line != out[j].line {
+			return out[i].line < out[j].line
+		}
+		return out[i].dest < out[j].dest
+	})
+
+	loadMu.Lock()
+	loadCache[cacheKey] = out
+	loadMu.Unlock()
+	return out
+}
+
+// isDecodeCall reports whether call is a JSON decode — a call to
+// encoding/json.Unmarshal or to a Decode method on its Decoder, DIRECTLY or
+// through a function VALUE holding one.
+//
+// The function-value arm exists because `decode := json.Unmarshal` followed by
+// `decode(b, &schema)` resolves to no *types.Func at all. It is matched by
+// SIGNATURE, which would be far too wide on its own — the tree has 1710 two-arg
+// calls through function values (measured) — and is only ever reached for a
+// call whose LAST ARGUMENT is already schema-shaped, which is what makes it
+// narrow.
+func isDecodeCall(call *ast.CallExpr, info *types.Info) bool {
+	if sel, ok := call.Fun.(*ast.SelectorExpr); ok {
+		if fn, ok := info.Uses[sel.Sel].(*types.Func); ok {
+			return isJSONDecodeFunc(fn)
+		}
+	}
+	if id, ok := call.Fun.(*ast.Ident); ok {
+		if fn, ok := info.Uses[id].(*types.Func); ok {
+			return isJSONDecodeFunc(fn)
+		}
+	}
+	tv, ok := info.Types[call.Fun]
+	if !ok {
+		return false
+	}
+	sig, ok := types.Unalias(tv.Type).(*types.Signature)
+	if !ok || sig.Recv() != nil {
+		return false
+	}
+	// json.Unmarshal: func([]byte, any) error. Decoder.Decode: func(any) error.
+	if sig.Results().Len() != 1 || sig.Results().At(0).Type().String() != "error" {
+		return false
+	}
+	switch sig.Params().Len() {
+	case 1:
+		return types.IsInterface(sig.Params().At(0).Type())
+	case 2:
+		p0, ok := sig.Params().At(0).Type().(*types.Slice)
+		return ok && p0.Elem().String() == "byte" && types.IsInterface(sig.Params().At(1).Type())
+	}
+	return false
+}
+
+// isJSONDecodeFunc reports whether fn is encoding/json's Unmarshal, or the
+// Decode method on its Decoder.
+//
+// By CALLEE IDENTITY rather than by the receiver's spelling: an aliased import
+// and a `json.NewDecoder(r).Decode(...)` chain both resolve to the same object,
+// which is what makes the alias hole disappear rather than get another arm.
+func isJSONDecodeFunc(fn *types.Func) bool {
+	sig, _ := fn.Type().(*types.Signature)
+	if sig == nil {
+		return false
+	}
+	if recv := sig.Recv(); recv != nil {
+		if fn.Name() != "Decode" {
+			return false
+		}
+		t := recv.Type()
+		if p, ok := t.(*types.Pointer); ok {
+			t = p.Elem()
+		}
+		named, ok := t.(*types.Named)
+		if !ok || named.Obj().Pkg() == nil {
+			return false
+		}
+		return named.Obj().Pkg().Path() == "encoding/json" && named.Obj().Name() == "Decoder"
+	}
+	return fn.Pkg() != nil && fn.Pkg().Path() == "encoding/json" && fn.Name() == "Unmarshal"
+}
+
+func min(a, b int) int {
+	if a < b {
+		return a
+	}
+	return b
+}
+
+func TestReservedFieldDecodeSitesAreClassified(t *testing.T) {
+	root := repoRoot(t)
+	sites := collectSchemaDecodes(t, root)
+	if len(sites) == 0 {
+		t.Fatal("the guard found no decode sites at all — it has gone blind, " +
+			"which would let every future site through silently")
+	}
+	for _, s := range sites {
+		if _, ok := rawDecodeAllowed[s.key()]; !ok {
+			t.Errorf("%s:%d decodes a collection schema RAW into %q.\n"+
+				"  If it reasons about an ITEM's fields, call models.UnmarshalItemFieldSchema instead — a\n"+
+				"  grandfathered reserved-key FieldDef must not reach it (BUG-2685).\n"+
+				"  If the collection DEFINITION is the subject, add %q to rawDecodeAllowed with the reason.",
+				s.path, s.line, s.dest, s.key())
+		}
+	}
+}
+
+// TestGuardAllowListHasNoDeadEntries keeps the allow-list honest: an entry whose
+// site was converted or deleted is a standing permission for a site that no
+// longer asks for it, and the next raw decode with that name inherits it.
+func TestGuardAllowListHasNoDeadEntries(t *testing.T) {
+	live := map[string]bool{}
+	for _, s := range collectSchemaDecodes(t, repoRoot(t)) {
+		live[s.key()] = true
+	}
+	for k := range rawDecodeAllowed {
+		if !live[k] {
+			t.Errorf("rawDecodeAllowed has no live site for %q — remove the entry", k)
+		}
+	}
+}
+
+// rawDecodeSiteCount is how many raw decode sites the guard must find.
+//
+// A RECEIPT, not a magic number: 20 on the BUG-2685 tree — 16 decoding a
+// models.CollectionSchema, plus the one decoding the parallel `bootstrapSchema`
+// (which a pass keyed on the canonical type could not see, and which is why
+// this guard matches on SHAPE), plus the three whose destination is an
+// INTERFACE and so cannot be resolved at all. The last three are reported
+// rather than assumed harmless; the allow-list says what each actually decodes.
+//
+// It is a count and not just a key set because two sites can share a
+// `path::dest` key — cmd_collection.go and handlers_collections.go each have
+// two — so key-set equality alone would pass if the guard went blind to one of
+// a pair. A DROP with no conversion in the same change means the guard stopped
+// seeing sites, not that the sites stopped existing.
+const rawDecodeSiteCount = 20
+
+func TestGuardSeesEveryKnownDecodeSite(t *testing.T) {
+	sites := collectSchemaDecodes(t, repoRoot(t))
+	if len(sites) != rawDecodeSiteCount {
+		var got []string
+		for _, s := range sites {
+			got = append(got, fmt.Sprintf("%s:%d(%s)", s.path, s.line, s.dest))
+		}
+		t.Errorf("guard found %d raw decode sites, expected %d.\n  found: %s",
+			len(sites), rawDecodeSiteCount, strings.Join(got, "\n         "))
+	}
+}
+
+// TestGuardResolvesEveryDestinationShape runs the resolver over the fixture in
+// testdata/decodeshapes and requires it to see every destination spelling
+// written down there.
+//
+// THE FIXTURE OUTLIVED THE GUARD IT WAS WRITTEN FOR, which is why it is still
+// here. It was built for the go/ast approximation, where five review rounds
+// each named another spelling that approximation could not see; the type
+// checker answers all of them by construction, so this is no longer a list of
+// holes to plug but a REGRESSION TEST on the resolver — it fails if a future
+// change to schemaShaped, isJSONDecodeFunc, or the destination-argument rule
+// narrows what the guard can resolve.
+//
+// It also pins the NEGATIVE half: the fixture's last function decodes through
+// the correct helper with a SOURCE argument that mentions a schema, and must
+// NOT be reported.
+func TestGuardResolvesEveryDestinationShape(t *testing.T) {
+	root := repoRoot(t)
+	dir := filepath.Join(root, "internal", "models", "testdata", "decodeshapes")
+	if _, err := os.Stat(dir); err != nil {
+		t.Fatalf("fixture missing: %v", err)
+	}
+
+	sites := collectSchemaDecodes(t, dir, ".")
+	got := map[string]bool{}
+	for _, s := range sites {
+		got[s.dest] = true
+	}
+
+	want := []string{
+		"localDest",        // &local
+		"paramDest",        // bare pointer param
+		"fieldDest",        // &struct.field
+		"new",              // new(T) — the call, which is the destination itself
+		"CollectionSchema", // &T{} names the TYPE, not a variable
+		"sliceDest",        // &container[i]
+		"makeDest",         // the result of a helper returning a schema
+		"decoderDest",      // json.Decoder.Decode, aliased import
+		"shortDest",        // x := T{}
+		"inferredDest",     // var x = T{}
+		"newDest",          // var x = new(T)
+		"aliasDest",        // x := &alreadyASchema
+		"aliasedPkgDest",   // models imported under an alias
+		"typeAliasDest",    // a type alias for the schema
+		"helperResultDest", // x := helperReturningASchemaPointer()
+		"closureDest",      // a closure parameter
+		"inlineDest",       // a schema-shaped type that is not CollectionSchema
+		"namedSliceDest",   // the fields member is a NAMED slice type
+		"funcValueDest",    // called through a function value holding json.Unmarshal
+		"ifaceDest",        // an interface destination, reported not resolved
+		"embeddedDest",     // a struct embedding a schema
+		"ptrElemDest",      // fields whose element type is a pointer
+		"untaggedDest",     // an UNTAGGED fields member, matched by name
+	}
+	// `excludedDest` is deliberately absent: its fields member is `json:"-"`,
+	// so it is not a schema on the wire and must NOT be reported.
+	for _, w := range want {
+		if !got[w] {
+			t.Errorf("the resolver cannot see the destination shape named by %q in testdata/decodeshapes", w)
+		}
+	}
+
+	// Twenty-three decodes are destinations; the fixture's last call is the
+	// negative case and goes through the helper, so it is not a raw decode at
+	// all. BELOW twenty-three means a shape went missing; ABOVE means the source
+	// argument is matching again.
+	if len(sites) != 23 {
+		var lines []string
+		for _, s := range sites {
+			lines = append(lines, fmt.Sprintf("%s:%d(%s)", s.path, s.line, s.dest))
+		}
+		t.Errorf("fixture yielded %d raw decode sites, expected 23:\n  %s",
+			len(sites), strings.Join(lines, "\n  "))
+	}
+}

internal/models/terminal.go

@@ -142,7 +142,7 @@ func CollectionCompletedWorkValues(schemaJSON, settingsJSON string) (fieldKey st
 	var schema CollectionSchema
 	var settings CollectionSettings
 	if schemaJSON != "" {
-		_ = json.Unmarshal([]byte(schemaJSON), &schema)
+		_ = UnmarshalItemFieldSchema([]byte(schemaJSON), &schema)
 	}
 	if settingsJSON != "" {
 		_ = json.Unmarshal([]byte(settingsJSON), &settings)

internal/models/testdata/decodeshapes/aliased.go

@@ -0,0 +1,29 @@
+package decodeshapes
+
+import (
+	jsonx "encoding/json"
+
+	m "github.com/PerpetualSoftware/pad/internal/models"
+)
+
+// 13. the models package under a LOCAL ALIAS.
+func s13(b []byte) {
+	var aliasedPkgDest m.CollectionSchema
+	_ = jsonx.Unmarshal(b, &aliasedPkgDest)
+}
+
+// 14. a schema shape whose element type is declared INLINE.
+type inlineShaped struct {
+	Fields []struct {
+		Key  string `json:"key"`
+		Type string `json:"type"`
+	} `json:"fields"`
+}
+
+// 18. a decode into a schema-shaped type whose ELEMENT type is declared inline
+// and which is not models.CollectionSchema at all — the bootstrapSchema case,
+// generalised. A guard keyed on the canonical type cannot see this one.
+func s18(b []byte) {
+	var inlineDest inlineShaped
+	_ = jsonx.Unmarshal(b, &inlineDest)
+}

internal/models/testdata/decodeshapes/round8.go

@@ -0,0 +1,108 @@
+package decodeshapes
+
+import (
+	jsonx "encoding/json"
+
+	"github.com/PerpetualSoftware/pad/internal/models"
+)
+
+// 15. a TYPE ALIAS for a schema.
+type aliasType = models.CollectionSchema
+
+func s15(b []byte) {
+	var typeAliasDest aliasType
+	_ = jsonx.Unmarshal(b, &typeAliasDest)
+}
+
+// 16. a variable holding the result of a helper that returns a schema pointer.
+func s16(b []byte) {
+	helperResultDest := makeDest()
+	_ = jsonx.Unmarshal(b, helperResultDest)
+}
+
+// 17. a CLOSURE parameter.
+func s17(b []byte) {
+	f := func(closureDest *models.CollectionSchema) {
+		_ = jsonx.Unmarshal(b, closureDest)
+	}
+	f(nil)
+}
+
+// 19. a NAMED slice type for the fields member — the shape test must unwrap it.
+type fieldList []models.FieldDef
+
+type namedSliceSchema struct {
+	Fields fieldList `json:"fields"`
+}
+
+func s19(b []byte) {
+	var namedSliceDest namedSliceSchema
+	_ = jsonx.Unmarshal(b, &namedSliceDest)
+}
+
+// 20. a call through a FUNCTION VALUE holding json.Unmarshal.
+func s20(b []byte) {
+	decode := jsonx.Unmarshal
+	var funcValueDest models.CollectionSchema
+	_ = decode(b, &funcValueDest)
+}
+
+// 21. an INTERFACE destination, which hides the concrete type. Reported for
+// classification rather than resolved.
+func s21(b []byte) {
+	var schema models.CollectionSchema
+	var ifaceDest any = &schema
+	_ = jsonx.Unmarshal(b, ifaceDest)
+}
+
+// NOT resolved, and deliberately: a POINTER to an interface. Including it would
+// pull in every decode-arbitrary-JSON call in the tree (measured: 20 sites
+// become 38). See the comment beside the interface arm in the guard.
+func s22(b []byte) {
+	var schema models.CollectionSchema
+	var ptrIfaceDest any = &schema
+	_ = jsonx.Unmarshal(b, &ptrIfaceDest)
+}
+
+// 23. a struct EMBEDDING a schema carries the same JSON members.
+type embeddedSchema struct {
+	models.CollectionSchema
+	Extra string `json:"extra"`
+}
+
+func s23(b []byte) {
+	var embeddedDest embeddedSchema
+	_ = jsonx.Unmarshal(b, &embeddedDest)
+}
+
+// 24. a fields member whose ELEMENT is a pointer.
+type ptrElemSchema struct {
+	Fields []*models.FieldDef `json:"fields"`
+}
+
+func s24(b []byte) {
+	var ptrElemDest ptrElemSchema
+	_ = jsonx.Unmarshal(b, &ptrElemDest)
+}
+
+// 25. an UNTAGGED fields member. encoding/json matches it by name,
+// case-insensitively, so this decodes the same JSON as the tagged form.
+type untaggedSchema struct {
+	Fields []models.FieldDef
+}
+
+func s25(b []byte) {
+	var untaggedDest untaggedSchema
+	_ = jsonx.Unmarshal(b, &untaggedDest)
+}
+
+// 26. a fields member EXCLUDED from JSON is not a schema member at all.
+type excludedFieldsSchema struct {
+	Fields []models.FieldDef `json:"-"`
+	Real   string            `json:"real"`
+}
+
+func s26(b []byte) {
+	var excludedDest excludedFieldsSchema
+	_ = jsonx.Unmarshal(b, &excludedDest)
+}

internal/models/testdata/decodeshapes/shapes.go

@@ -0,0 +1,104 @@
+// Package decodeshapes is a FIXTURE, not compiled code — it lives under
+// testdata so the go tool ignores it, and exists to be parsed by
+// TestGuardCatchesEveryDestinationShape.
+//
+// It is the population table for BUG-2685's guard. Three consecutive review
+// rounds each named another destination spelling the guard did not handle, one
+// at a time; CONVE-35 says a loop that keeps finding new members of a class is
+// measuring the enumeration rather than the code. So the class is written down
+// HERE, once, and the guard is tested against the whole of it.
+//
+// Adding a spelling to this file is how you extend that table. A spelling the
+// guard cannot see fails the test, which is the point.
+package decodeshapes
+
+import (
+	jsonx "encoding/json"
+	"io"
+
+	"github.com/PerpetualSoftware/pad/internal/models"
+)
+
+type holder struct {
+	fieldDest models.CollectionSchema
+}
+
+func makeDest() *models.CollectionSchema { return &models.CollectionSchema{} }
+
+// 1. address of a local value
+func s1(b []byte) {
+	var localDest models.CollectionSchema
+	_ = jsonx.Unmarshal(b, &localDest)
+}
+
+// 2. a parameter that is already a pointer, passed bare
+func s2(b []byte, paramDest *models.CollectionSchema) {
+	_ = jsonx.Unmarshal(b, paramDest)
+}
+
+// 3. address of a struct field whose struct is declared elsewhere in the package
+func s3(b []byte) {
+	var h holder
+	_ = jsonx.Unmarshal(b, &h.fieldDest)
+}
+
+// 4. new(T)
+func s4(b []byte) {
+	_ = jsonx.Unmarshal(b, new(models.CollectionSchema))
+}
+
+// 5. address of a composite literal
+func s5(b []byte) {
+	_ = jsonx.Unmarshal(b, &models.CollectionSchema{})
+}
+
+// 6. address of an element of a container of schemas
+func s6(b []byte) {
+	sliceDest := make([]models.CollectionSchema, 1)
+	_ = jsonx.Unmarshal(b, &sliceDest[0])
+}
+
+// 7. the result of a helper that returns a schema pointer
+func s7(b []byte) {
+	_ = jsonx.Unmarshal(b, makeDest())
+}
+
+// 8. a json.Decoder, through an ALIASED import of encoding/json
+func s8(r io.Reader) {
+	var decoderDest models.CollectionSchema
+	_ = jsonx.NewDecoder(r).Decode(&decoderDest)
+}
+
+// 9. a short declaration with an inferred type
+func s9(b []byte) {
+	shortDest := models.CollectionSchema{}
+	_ = jsonx.Unmarshal(b, &shortDest)
+}
+
+// 10. a var declaration with an inferred type
+func s10(b []byte) {
+	var inferredDest = models.CollectionSchema{}
+	_ = jsonx.Unmarshal(b, &inferredDest)
+}
+
+// 11. a var whose inferred type comes from new(T)
+func s11(b []byte) {
+	var newDest = new(models.CollectionSchema)
+	_ = jsonx.Unmarshal(b, newDest)
+}
+
+// 12. a pointer taken from a schema already named
+func s12(b []byte) {
+	var base models.CollectionSchema
+	aliasDest := &base
+	_ = jsonx.Unmarshal(b, aliasDest)
+}
+
+// 13. models imported under an ALIAS (see aliased.go in this directory)
+
+// NOT a destination: the SOURCE argument mentioning a schema must not match, or
+// every correctly-converted site in the tree reports itself as raw.
+func notADestination(c models.Collection) {
+	var out models.CollectionSchema
+	_ = models.UnmarshalItemFieldSchema([]byte(c.Schema), &out)
+}

internal/server/bug2685_grandfathered_reserved_key_test.go

@@ -0,0 +1,312 @@
+package server
+
+import (
+	"encoding/json"
+	"net/http"
+	"testing"
+
+	"github.com/PerpetualSoftware/pad/internal/models"
+)
+
+// BUG-2685 — a collection schema that DECLARES a reserved metadata key.
+//
+// `validateNoReservedFieldKeys` (BUG-2674) stops a schema NEWLY declaring one,
+// but GRANDFATHERS an existing declaration, so the shape survives in any
+// workspace that had it before that gate landed. These tests reproduce that
+// shape the only way it can exist — written through the store, which is the
+// door a pre-gate row came through — and pin what the declaration does to
+// item-field paths that consult a schema.
+//
+// The declaration is wrong in BOTH directions at once, which is why the class
+// remedy is to ignore it rather than to refuse the write: a `text` declaration
+// REFUSES the system's own value (a notes ARRAY) and ACCEPTS a caller's junk
+// string.
+
+// grandfatherReservedKey rewrites a collection's STORED schema to declare def,
+// bypassing the HTTP gate exactly as a legacy row does. Returns the collection.
+func grandfatherReservedKey(t *testing.T, srv *Server, ws, collSlug string, def models.FieldDef) models.Collection {
+	t.Helper()
+	rr := doRequest(srv, "GET", "/api/v1/workspaces/"+ws+"/collections/"+collSlug, nil)
+	if rr.Code != http.StatusOK {
+		t.Fatalf("load collection %q: %d %s", collSlug, rr.Code, rr.Body.String())
+	}
+	var coll models.Collection
+	parseJSON(t, rr, &coll)
+
+	var schema models.CollectionSchema
+	if err := json.Unmarshal([]byte(coll.Schema), &schema); err != nil {
+		t.Fatalf("parse stored schema: %v (%s)", err, coll.Schema)
+	}
+	schema.Fields = append(schema.Fields, def)
+	raw, err := json.Marshal(schema)
+	if err != nil {
+		t.Fatalf("marshal schema: %v", err)
+	}
+	s := string(raw)
+	updated, err := srv.store.UpdateCollection(coll.ID, models.CollectionUpdate{Schema: &s})
+	if err != nil {
+		t.Fatalf("store-level schema write: %v", err)
+	}
+
+	// PREMISE, asserted rather than assumed: the HTTP door would have REFUSED
+	// this declaration, so the store write is genuinely reproducing a shape no
+	// current client can create — not just taking a shortcut to one it could.
+	rr = doRequest(srv, "POST", "/api/v1/workspaces/"+ws+"/collections",
+		map[string]interface{}{
+			"name":   "Gate Check " + def.Key,
+			"schema": `{"fields":[{"key":"` + def.Key + `","type":"text"}]}`,
+		})
+	if rr.Code != http.StatusBadRequest {
+		t.Fatalf("the reserved-key gate is not in force: creating a collection declaring %q returned %d %s",
+			def.Key, rr.Code, rr.Body.String())
+	}
+	return *updated
+}
+
+func bug2685CreateItem(t *testing.T, srv *Server, ws, coll string, body map[string]interface{}) *httpResult {
+	t.Helper()
+	rr := doRequest(srv, "POST", "/api/v1/workspaces/"+ws+"/collections/"+coll+"/items", body)
+	return &httpResult{code: rr.Code, body: rr.Body.String()}
+}
+
+type httpResult struct {
+	code int
+	body string
+}
+
+// Leg A — the system's OWN value is refused.
+//
+// `pad item note` / `pad item decide` read the item's fields, append an entry
+// with models.AppendImplementationNote, and PATCH the whole blob back. Against
+// a legacy `implementation_notes: text` declaration that blob fails
+// validateFieldType, so the collection's items cannot be annotated at all.
+func TestBUG2685_LegacyDeclarationRefusesTheSystemsOwnWrite(t *testing.T) {
+	srv := testServer(t)
+	ws := createWSWithCollections(t, srv)
+	grandfatherReservedKey(t, srv, ws, "tasks", models.FieldDef{
+		Key:  models.ItemFieldImplementationNotes,
+		Type: "text",
+	})
+
+	got := bug2685CreateItem(t, srv, ws, "tasks", map[string]interface{}{"title": "Annotate me"})
+	if got.code != http.StatusCreated {
+		t.Fatalf("create: %d %s", got.code, got.body)
+	}
+	var created models.Item
+	if err := json.Unmarshal([]byte(got.body), &created); err != nil {
+		t.Fatalf("decode created item: %v (%s)", err, got.body)
+	}
+
+	// Exactly what `pad item note` sends.
+	fields, err := models.AppendImplementationNote(created.Fields, models.ItemImplementationNote{
+		Summary:   "a note",
+		Details:   "the details",
+		CreatedAt: "2026-09-14T00:00:00Z",
+		CreatedBy: "agent",
+	})
+	if err != nil {
+		t.Fatalf("AppendImplementationNote: %v", err)
+	}
+
+	rr := doRequest(srv, "PATCH", "/api/v1/workspaces/"+ws+"/items/"+created.Slug,
+		map[string]interface{}{"fields": fields})
+	if rr.Code != http.StatusOK {
+		t.Errorf("the system's own note write was refused by a legacy declaration: %d %s",
+			rr.Code, rr.Body.String())
+	}
+}
+
+// Leg B — the declaration must not DECIDE the answer.
+//
+// The class rule stated as something a test can refute: two collections, one
+// carrying a legacy declaration and one not, must answer the same bytes the
+// same way. Unfixed they do not — a legacy `implementation_notes: number`
+// refuses the string that the undeclared collection stores, so whether a write
+// succeeds turns on a FieldDef nobody may create any more.
+//
+// The control leg is what makes this evidence rather than an assertion about
+// one path: it also pins the direction of the disagreement, so a future change
+// that closes the create-mint hole (undeclared reserved keys are ACCEPTED at
+// create — BUG-2850 chose accept-and-name, and `items/overrides.go` names the
+// hole) fails this test loudly instead of silently inverting it.
+//
+// Note what is NOT asserted: that the junk is refused. Refusing is the other
+// class and is out of this unit — see the BUG-2685 trail.
+func TestBUG2685_ADeclarationDoesNotDecideWhetherAWriteSucceeds(t *testing.T) {
+	srv := testServer(t)
+	ws := createWSWithCollections(t, srv)
+
+	body := map[string]interface{}{
+		"title":  "Same bytes",
+		"fields": `{"implementation_notes":"not a number and not a notes array"}`,
+	}
+
+	// CONTROL first, before the declaration exists anywhere.
+	control := bug2685CreateItem(t, srv, ws, "ideas", body)
+
+	grandfatherReservedKey(t, srv, ws, "tasks", models.FieldDef{
+		Key:  models.ItemFieldImplementationNotes,
+		Type: "number",
+	})
+	declared := bug2685CreateItem(t, srv, ws, "tasks", body)
+
+	if control.code != declared.code {
+		t.Errorf("a grandfathered declaration decided the outcome: undeclared collection answered %d, "+
+			"declared collection answered %d\n  undeclared: %s\n  declared:   %s",
+			control.code, declared.code, control.body, declared.body)
+	}
+	if control.code != http.StatusCreated {
+		t.Fatalf("premise broken: the control write did not succeed (%d %s), so the comparison above "+
+			"could pass by both paths failing", control.code, control.body)
+	}
+}
+
+// Leg C — a legacy REQUIRED declaration makes the collection uncreatable.
+//
+// Nothing in a create request mentions implementation_notes, and no client
+// surface offers it, so the 400 names a field the caller cannot supply.
+func TestBUG2685_LegacyRequiredDeclarationBlocksCreate(t *testing.T) {
+	srv := testServer(t)
+	ws := createWSWithCollections(t, srv)
+	grandfatherReservedKey(t, srv, ws, "tasks", models.FieldDef{
+		Key:      models.ItemFieldDecisionLog,
+		Type:     "text",
+		Required: true,
+	})
+
+	got := bug2685CreateItem(t, srv, ws, "tasks", map[string]interface{}{"title": "Ordinary task"})
+	if got.code != http.StatusCreated {
+		t.Errorf("an ordinary create was refused over a reserved key nobody typed: %d %s", got.code, got.body)
+	}
+}
+
+// Leg D — the two PRESENTATION doors must not publish the declaration either.
+//
+// These are the "bootstrap / share presentation" row of the filed table, and
+// the bootstrap one is why a sweep owes its vantage point: that decode does NOT
+// land in a models.CollectionSchema — `bootstrapSchema` is a parallel struct —
+// so the type-resolved pass that found the other 23 sites could not see it. It
+// surfaced only by chasing a review finding about the OTHER door here.
+//
+// Bootstrap is the sharper of the two: the blob goes to every agent at session
+// start and the skill treats `collections[].schema` as the write contract, so a
+// surviving declaration is one every agent is told to write into.
+//
+// Two tests rather than two subtests on one server, deliberately: the share leg
+// has to CREATE A USER for the link's created_by FK, and doing so closes the
+// fresh-install no-auth window the bootstrap leg's unauthenticated GET depends
+// on. Sharing a server made the second leg 401 — a failure about the harness,
+// inside a test whose whole job is to be about the payload.
+func bug2685PresentationWorkspace(t *testing.T) (*Server, string) {
+	t.Helper()
+	srv := testServer(t)
+	ws := createWSWithCollections(t, srv)
+	grandfatherReservedKey(t, srv, ws, "tasks", models.FieldDef{
+		Key:   models.ItemFieldImplementationNotes,
+		Label: "Implementation Notes",
+		Type:  "text",
+	})
+	return srv, ws
+}
+
+// Codex round 2 raised the public payload as a response-shape change and
+// proposed preserving the raw schema. The change IS the fix, and the reason is
+// in the handler's own comment two lines above the decode: the schema is
+// emitted "so the public viewer can render" the items. That is an item-field
+// purpose, so an anonymous viewer would otherwise be handed a declaration
+// telling it to render system metadata as an ordinary field. Pinned here so the
+// contract change stays deliberate.
+func TestBUG2685_PublicSharePayloadDoesNotPublishAGrandfatheredDeclaration(t *testing.T) {
+	srv, ws := bug2685PresentationWorkspace(t)
+
+	// Minted through the store rather than the HTTP door: that door records
+	// created_by and the FK needs a real user, which this workspace (made by
+	// the no-auth fresh-install path) has none of. Nothing about the READ under
+	// test depends on how the link was minted.
+	wsRow, err := srv.store.GetWorkspaceBySlug(ws)
+	if err != nil || wsRow == nil {
+		t.Fatalf("load workspace: %v", err)
+	}
+	coll, err := srv.store.GetCollectionBySlug(wsRow.ID, "tasks")
+	if err != nil || coll == nil {
+		t.Fatalf("load tasks collection: %v", err)
+	}
+	owner, err := srv.store.CreateUser(models.UserCreate{
+		Email: "bug2685-share@example.com", Name: "Share", Username: "bug2685-share",
+		Password: "pw-test-12345",
+	})
+	if err != nil {
+		t.Fatalf("create owner: %v", err)
+	}
+	link, err := srv.store.CreateShareLink(wsRow.ID, "collection", coll.ID, "view", owner.ID, nil)
+	if err != nil {
+		t.Fatalf("mint share link: %v", err)
+	}
+
+	rr := doRequest(srv, "GET", "/api/v1/s/"+link.Token, nil)
+	if rr.Code != http.StatusOK {
+		t.Fatalf("resolve share link: %d %s", rr.Code, rr.Body.String())
+	}
+	var resolved struct {
+		Collection struct {
+			Schema struct {
+				Fields []struct {
+					Key string `json:"key"`
+				} `json:"fields"`
+			} `json:"schema"`
+		} `json:"collection"`
+	}
+	if err := json.Unmarshal(rr.Body.Bytes(), &resolved); err != nil {
+		t.Fatalf("decode resolved share: %v (%s)", err, rr.Body.String())
+	}
+	if len(resolved.Collection.Schema.Fields) == 0 {
+		t.Fatal("premise broken: the public payload carried no schema fields at all, " +
+			"so the assertion below cannot distinguish a strip from an absent schema")
+	}
+	for _, f := range resolved.Collection.Schema.Fields {
+		if models.IsReservedItemField(f.Key) {
+			t.Errorf("the public share payload published a grandfathered reserved-key declaration: %q", f.Key)
+		}
+	}
+}
+
+func TestBUG2685_BootstrapDoesNotPublishAGrandfatheredDeclarationToEveryAgent(t *testing.T) {
+	srv, ws := bug2685PresentationWorkspace(t)
+
+	rr := doRequest(srv, "GET", "/api/v1/workspaces/"+ws+"/agent/bootstrap", nil)
+	if rr.Code != http.StatusOK {
+		t.Fatalf("bootstrap: %d %s", rr.Code, rr.Body.String())
+	}
+	var resp struct {
+		Collections []struct {
+			Slug   string `json:"slug"`
+			Schema struct {
+				Fields []struct {
+					Key string `json:"key"`
+				} `json:"fields"`
+			} `json:"schema"`
+		} `json:"collections"`
+	}
+	if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("decode bootstrap: %v", err)
+	}
+	var sawTasks bool
+	for _, c := range resp.Collections {
+		if c.Slug != "tasks" {
+			continue
+		}
+		sawTasks = true
+		for _, f := range c.Schema.Fields {
+			if models.IsReservedItemField(f.Key) {
+				t.Errorf("bootstrap published a grandfathered reserved-key declaration to every agent: %q", f.Key)
+			}
+		}
+		if len(c.Schema.Fields) == 0 {
+			t.Error("premise broken: the tasks schema came back with no fields at all, " +
+				"so the assertion above cannot distinguish a strip from an empty projection")
+		}
+	}
+	if !sawTasks {
+		t.Fatal("premise broken: bootstrap returned no tasks collection")
+	}
+}

internal/server/handlers_artifact_import.go

@@ -138,7 +138,7 @@ func (s *Server) handleImportArtifact(w http.ResponseWriter, r *http.Request) {
 	}
 
 	var schema models.CollectionSchema
-	if err := json.Unmarshal([]byte(coll.Schema), &schema); err != nil {
+	if err := models.UnmarshalItemFieldSchema([]byte(coll.Schema), &schema); err != nil {
 		writeError(w, http.StatusInternalServerError, "internal_error", "Failed to parse collection schema")
 		return
 	}

internal/server/handlers_bootstrap.go

@@ -189,11 +189,24 @@ func trimRedundantSchemaLabels(raw []byte) json.RawMessage {
 	if err := json.Unmarshal(raw, &s); err != nil {
 		return raw
 	}
+	// BUG-2685: a grandfathered reserved-key FieldDef is dropped here as well.
+	// This is the one schema decode in the tree that does NOT land in a
+	// models.CollectionSchema — bootstrapSchema is a parallel struct — which is
+	// exactly why it was missed by the pass that converted the other 23 sites,
+	// and why the guard now enumerates schema-SHAPED types rather than one
+	// named type. The bootstrap blob goes to every agent at session start, so a
+	// declaration surviving here is a declaration every agent is told to write.
+	kept := s.Fields[:0]
 	for i := range s.Fields {
+		if models.IsReservedItemField(s.Fields[i].Key) {
+			continue
+		}
 		if s.Fields[i].Label == titleCaseLabel(s.Fields[i].Key) {
 			s.Fields[i].Label = ""
 		}
+		kept = append(kept, s.Fields[i])
 	}
+	s.Fields = kept
 	out, err := json.Marshal(s)
 	if err != nil {
 		return raw

internal/server/handlers_dashboard.go

@@ -263,7 +263,7 @@ func buildDoneContextMap(collections []models.Collection) map[string]doneContext
 		// Best-effort parse: a malformed schema or settings just leaves
 		// the corresponding sub-struct zero-valued — the dashboard
 		// continues to function with reduced done-detection fidelity.
-		_ = json.Unmarshal([]byte(c.Schema), &ctx.schema)
+		_ = models.UnmarshalItemFieldSchema([]byte(c.Schema), &ctx.schema)
 		if c.Settings != "" {
 			_ = json.Unmarshal([]byte(c.Settings), &ctx.settings)
 		}

internal/server/handlers_items.go

@@ -571,7 +571,7 @@ func (s *Server) handleListCollectionItems(w http.ResponseWriter, r *http.Reques
 
 	var collSchema models.CollectionSchema
 	if coll.Schema != "" {
-		_ = json.Unmarshal([]byte(coll.Schema), &collSchema)
+		_ = models.UnmarshalItemFieldSchema([]byte(coll.Schema), &collSchema)
 	}
 	if err := s.resolveParentFilter(r, workspaceID, &params, collSchema); err != nil {
 		writeError(w, http.StatusBadRequest, "bad_request", err.Error())
@@ -675,7 +675,7 @@ func (s *Server) handleCreateItem(w http.ResponseWriter, r *http.Request) {
 
 	// Parse collection schema
 	var schema models.CollectionSchema
-	if err := json.Unmarshal([]byte(coll.Schema), &schema); err != nil {
+	if err := models.UnmarshalItemFieldSchema([]byte(coll.Schema), &schema); err != nil {
 		writeError(w, http.StatusInternalServerError, "internal_error", "Failed to parse collection schema")
 		return
 	}
@@ -1268,7 +1268,7 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) {
 		}
 
 		var schema models.CollectionSchema
-		if err := json.Unmarshal([]byte(coll.Schema), &schema); err != nil {
+		if err := models.UnmarshalItemFieldSchema([]byte(coll.Schema), &schema); err != nil {
 			writeError(w, http.StatusInternalServerError, "internal_error", "Failed to parse collection schema")
 			return
 		}
@@ -1448,7 +1448,7 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 		var schema models.CollectionSchema
-		if err := json.Unmarshal([]byte(coll.Schema), &schema); err != nil {
+		if err := models.UnmarshalItemFieldSchema([]byte(coll.Schema), &schema); err != nil {
 			writeError(w, http.StatusInternalServerError, "internal_error", "Failed to parse collection schema")
 			return
 		}
@@ -3134,7 +3134,7 @@ func (s *Server) handleGetItemProgress(w http.ResponseWriter, r *http.Request) {
 			ctx, cached := ctxCache[child.CollectionID]
 			if !cached {
 				if coll, cerr := s.store.GetCollection(child.CollectionID); cerr == nil && coll != nil {
-					_ = json.Unmarshal([]byte(coll.Schema), &ctx.schema)
+					_ = models.UnmarshalItemFieldSchema([]byte(coll.Schema), &ctx.schema)
 					if coll.Settings != "" {
 						_ = json.Unmarshal([]byte(coll.Settings), &ctx.settings)
 					}

internal/server/handlers_items_bulk.go

@@ -510,7 +510,7 @@ func (s *Server) bulkFieldUpdate(r *http.Request, workspaceID string, item *mode
 		return nil, &bulkOpError{message: "failed to load collection"}
 	}
 	var schema models.CollectionSchema
-	if err := json.Unmarshal([]byte(coll.Schema), &schema); err != nil {
+	if err := models.UnmarshalItemFieldSchema([]byte(coll.Schema), &schema); err != nil {
 		return nil, &bulkOpError{message: "failed to parse collection schema"}
 	}
 

internal/server/handlers_items_open_children_guard.go

@@ -212,7 +212,7 @@ func (s *Server) runOpenChildrenGuard(tx *sql.Tx, ctx openChildrenGuardContext)
 			// inside the item-update tx, and a pool read from here needs a
 			// second connection while that tx holds the first.
 			if coll, cerr := s.store.GetCollectionAnyStateTx(tx, child.CollectionID); cerr == nil && coll != nil {
-				_ = json.Unmarshal([]byte(coll.Schema), &dc.schema)
+				_ = models.UnmarshalItemFieldSchema([]byte(coll.Schema), &dc.schema)
 				if coll.Settings != "" {
 					_ = json.Unmarshal([]byte(coll.Settings), &dc.settings)
 				}

internal/server/handlers_share_links.go

@@ -624,7 +624,7 @@ func (s *Server) handleResolveShareLink(w http.ResponseWriter, r *http.Request)
 		}
 		if s := strings.TrimSpace(coll.Schema); s != "" {
 			var schema models.CollectionSchema
-			if err := json.Unmarshal([]byte(s), &schema); err == nil {
+			if err := models.UnmarshalItemFieldSchema([]byte(s), &schema); err == nil {
 				publicCollection["schema"] = schema
 			}
 		}

internal/server/relation_hydration.go

@@ -1,7 +1,6 @@
 package server
 
 import (
-	"encoding/json"
 	"net/http"
 
 	"github.com/PerpetualSoftware/pad/internal/models"
@@ -137,7 +136,7 @@ func (s *Server) relationSchemasForWorkspace(workspaceID string) (map[string]mod
 			continue
 		}
 		var schema models.CollectionSchema
-		if err := json.Unmarshal([]byte(colls[i].Schema), &schema); err != nil {
+		if err := models.UnmarshalItemFieldSchema([]byte(colls[i].Schema), &schema); err != nil {
 			// A schema that will not parse is a different defect, reported
 			// elsewhere; hydration declines to be the second voice.
 			continue

internal/store/collections.go

@@ -414,7 +414,7 @@ func (s *Store) ListCollections(workspaceID string) ([]models.Collection, error)
 	for idx := range result {
 		c := &result[idx]
 		var schema models.CollectionSchema
-		if err := json.Unmarshal([]byte(c.Schema), &schema); err != nil {
+		if err := models.UnmarshalItemFieldSchema([]byte(c.Schema), &schema); err != nil {
 			schema = models.CollectionSchema{}
 		}
 		var settings models.CollectionSettings

internal/store/item_stars.go

@@ -187,7 +187,7 @@ func (s *Store) buildCollectionDoneContextMap(workspaceID string) (map[string]co
 			return nil, err
 		}
 		var ctx collectionDoneContext
-		if err := json.Unmarshal([]byte(rawSchema), &ctx.schema); err != nil {
+		if err := models.UnmarshalItemFieldSchema([]byte(rawSchema), &ctx.schema); err != nil {
 			// Don't drop the entry entirely — we still want an empty
 			// context so the membership test falls back to defaults
 			// rather than silently treating the item as non-terminal.

internal/store/items.go

@@ -4334,7 +4334,7 @@ func scanCollectionDoneFilters(rows *sql.Rows) []collectionDoneFilter {
 			continue
 		}
 		var schema models.CollectionSchema
-		if err := json.Unmarshal([]byte(schemaJSON), &schema); err != nil {
+		if err := models.UnmarshalItemFieldSchema([]byte(schemaJSON), &schema); err != nil {
 			// Malformed schema → emit a default-fallback filter so the
 			// collection's items still get evaluated against the status
 			// column + global default terminals. This matches pre-TASK-604

internal/store/relation_links.go

@@ -167,7 +167,7 @@ func relationKeysFromSchemaJSON(schemaJSON string) map[string]struct{} {
 		return nil
 	}
 	var schema models.CollectionSchema
-	if err := json.Unmarshal([]byte(schemaJSON), &schema); err != nil {
+	if err := models.UnmarshalItemFieldSchema([]byte(schemaJSON), &schema); err != nil {
 		return nil
 	}
 	keys := map[string]struct{}{}
@@ -351,7 +351,7 @@ func (s *Store) GetRelationBacklinks(targetItemID, workspaceID string, limit, of
 		if !known {
 			byKey = map[string]string{}
 			var schema models.CollectionSchema
-			if json.Unmarshal([]byte(schemaJSON), &schema) == nil {
+			if models.UnmarshalItemFieldSchema([]byte(schemaJSON), &schema) == nil {
 				for _, def := range schema.Fields {
 					if def.Label != "" {
 						byKey[def.Key] = def.Label

internal/store/reports.go

@@ -666,7 +666,7 @@ func (s *Store) resolveReportCollections(workspaceID string, opts ReportOptions)
 		// Parse failures fall back to status + default terminals, matching
 		// scanCollectionDoneFilters' tolerance.
 		if c.Schema != "" {
-			_ = json.Unmarshal([]byte(c.Schema), &schema)
+			_ = models.UnmarshalItemFieldSchema([]byte(c.Schema), &schema)
 		}
 		if c.Settings != "" {
 			_ = json.Unmarshal([]byte(c.Settings), &settings)

internal/store/status_transitions_backfill.go

@@ -93,7 +93,7 @@ func doneFieldKeyFromCollection(col *models.Collection) string {
 // (e.g. an in-tx read) resolve the key without a models.Collection.
 func doneFieldKeyFromSchemaJSON(schemaJSON, settingsJSON string) string {
 	var schema models.CollectionSchema
-	if err := json.Unmarshal([]byte(schemaJSON), &schema); err != nil {
+	if err := models.UnmarshalItemFieldSchema([]byte(schemaJSON), &schema); err != nil {
 		return "status"
 	}
 	var settings models.CollectionSettings