diff --git a/internal/server/handlers_items_copy_relation_test.go b/internal/server/handlers_items_copy_relation_test.go index 5634a185..e2254150 100644 --- a/internal/server/handlers_items_copy_relation_test.go +++ b/internal/server/handlers_items_copy_relation_test.go @@ -848,3 +848,60 @@ func TestCopyPreflight_RelationDroppedCarriedKeyDoesNotExemptTheDefault(t *testi v, carried, f.targetB.ID) } } + +// A malformed destination default gets the SAME answer whether or not the +// caller sent an unrelated null override (codex round 17). +// +// `MigrateRelationReferents` skipped non-string values on the general rule +// that shape is ValidateFields's to reject, so one defect makes one error. +// That rule is right for a supplied or carried value and wrong for a +// DESTINATION DEFAULT, because the two disagree about the outcome: +// ValidateFields REFUSES, and a default is not the caller's assertion, so +// this unit's posture is drop-and-report. +// +// The observable was worse than the inconsistency. A default MigrateFields +// injects sits in the map before validation and was REFUSED; the identical +// default that ValidateFields injects — which is what a `{"owner_ref": null}` +// override causes — reached the late pass and was DROPPED. So the same +// malformed schema either blocked the copy outright or dropped an optional +// field, chosen by a request detail with nothing to do with it. The existing +// non-string-default test sends the override and therefore only ever +// exercised the forgiving path. +func TestCopyEndpoint_NonStringDestinationDefaultDropsWithOrWithoutANullOverride(t *testing.T) { + for _, tc := range []struct { + name string + override bool + }{ + {"no override", false}, + {"explicit null override", true}, + } { + t.Run(tc.name, func(t *testing.T) { + f := newCopyRelationFixtureNonStringDefault(t) + body := f.baseBody() + if tc.override { + body["field_overrides"] = map[string]any{"owner_ref": nil} + } + + pre := f.ok(body) + reason, dropped := droppedReason(pre, "owner_ref") + if !dropped { + t.Fatalf("a non-reference default is neither carried nor dropped: %+v", pre.Fields) + } + if reason != "invalid_shape" { + t.Fatalf("dropped for reason %q, want invalid_shape — the value is not a "+ + "reference, so every other reason describes a lookup that never happened", + reason) + } + if !pre.Valid { + t.Fatalf("the preflight refuses over a malformed default in an OPTIONAL "+ + "field; a default is not the caller's assertion and drops: %+v", pre.Fields) + } + + res := assertPreflightMatchesCopy(t, f.copyPreflightFixture, + "non-string relation default ("+tc.name+")", body) + if v, present := f.persistedFields(res.Item.ID)["owner_ref"]; present { + t.Fatalf("the copy stored a non-reference in a relation field: %#v", v) + } + }) + } +} diff --git a/internal/store/export.go b/internal/store/export.go index c2a866fa..8aa4a398 100644 --- a/internal/store/export.go +++ b/internal/store/export.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "log/slog" - "strings" "time" "unicode/utf8" @@ -850,11 +849,61 @@ func remapFieldIDs(fieldsJSON string, itemMap, collMap map[string]string) string if fieldsJSON == "" { return "{}" } - result := fieldsJSON - for oldID, newID := range itemMap { - if oldID != "" && newID != "" { - result = strings.ReplaceAll(result, oldID, newID) - } + // WHOLE-VALUE matches only, never a substring (codex round 17). + // + // This was `strings.ReplaceAll` over the raw JSON text for every id in the + // bundle, which corrupts a value that merely CONTAINS one: with a bundle + // carrying ids `c-1` and a relation value `"c-10"`, the value came out as + // `0` — a string that references nothing and that the + // importer then stored as though it were the user's data. Bundle ids are + // whatever the exporting instance had and an import accepts a + // caller-supplied file, so this is not confined to well-formed UUIDs. + // + // It matters more since the carry posture: an unresolvable relation value + // is deliberately IMPORTED VERBATIM rather than dropped, and "verbatim" is + // the whole promise. A partial rewrite breaks it silently and produces a + // value that never existed on either side. + // + // The same text substitution also rewrote ids appearing inside ordinary + // text values, which was never the intent — the remap exists to repoint + // RELATIONS at their clones. Whole-value matching gets both. + // + // I engineered a fixture AROUND this hazard in the import carry test + // rather than asking whether the product had it. It did. + var decoded map[string]any + if err := json.Unmarshal([]byte(fieldsJSON), &decoded); err != nil { + // Not an object we can walk. Left exactly as-is rather than + // substring-rewritten: an unparseable blob is not a thing to guess at. + return fieldsJSON + } + for k, v := range decoded { + decoded[k] = remapFieldValue(v, itemMap) + } + out, err := json.Marshal(decoded) + if err != nil { + return fieldsJSON + } + return string(out) +} + +// remapFieldValue rewrites a value that IS an old item id, and recurses into +// arrays so a multi-valued relation is covered. Anything else — a number, a +// bool, a nested object, a string that merely contains an id — is returned +// unchanged. +func remapFieldValue(v any, itemMap map[string]string) any { + switch t := v.(type) { + case string: + if newID, ok := itemMap[t]; ok && t != "" && newID != "" { + return newID + } + return t + case []any: + out := make([]any, len(t)) + for i, e := range t { + out[i] = remapFieldValue(e, itemMap) + } + return out + default: + return v } - return result } diff --git a/internal/store/relation_referents.go b/internal/store/relation_referents.go index b2b7c713..53bb66ed 100644 --- a/internal/store/relation_referents.go +++ b/internal/store/relation_referents.go @@ -521,6 +521,42 @@ func (s *Store) MigrateRelationReferentsQ( // Destination defaults resolve against the DESTINATION in both modes — the // destination picked the value, so the mode says nothing about it. if defaults := byOrigin[RelationOriginDestinationDefault]; len(defaults) > 0 { + // A NON-STRING default is dropped and reported here, exactly as + // ResolveLateRelationDefaultsQ drops one, and NOT left for + // ValidateFields (codex round 17). + // + // The general rule a few lines up — shape is ValidateFields's to + // reject, so one defect produces one error — is right for a SUPPLIED + // or CARRIED value and wrong for a default, because the two disagree + // about what should happen: ValidateFields REFUSES the request, and a + // destination default is not the caller's assertion, so this unit's + // posture is drop-and-report. + // + // Leaving it produced an outcome that depended on WHEN the default + // arrived. A default MigrateFields injects is in the map before + // validation and was refused; the identical default that + // ValidateFields injects — which is what happens when the caller + // sends an unrelated `{"key": null}` override — reached the late pass + // and was dropped. Same malformed schema, opposite answers, selected + // by a request detail with nothing to do with it. Dropping here makes + // the two paths agree BY CONSTRUCTION rather than by argument. + for _, def := range schema.Fields { + if def.Type != "relation" { + continue + } + raw, exists := defaults[def.Key] + if !exists { + continue + } + if _, isStr := raw.(string); isStr { + continue + } + dropped = append(dropped, RelationIssue{ + Key: def.Key, Target: def.Collection, Reason: RelationTargetInvalidShape, + }) + delete(fieldMap, def.Key) + delete(defaults, def.Key) + } issues, resolveErr := s.ResolveRelationReferentsQ(q, workspaceID, schema, defaults) if resolveErr != nil { return nil, nil, resolveErr diff --git a/internal/store/relation_referents_test.go b/internal/store/relation_referents_test.go index 95b04fea..1479ef17 100644 --- a/internal/store/relation_referents_test.go +++ b/internal/store/relation_referents_test.go @@ -572,3 +572,104 @@ func TestImportWorkspace_CarriesUnresolvableRelationValues(t *testing.T) { }) } } + +// An unresolvable carried relation value survives the ID remap VERBATIM, even +// when it contains a resolvable id as a substring (codex round 17). +// +// `remapFieldIDs` rewrote relation values with strings.ReplaceAll over the raw +// JSON for every id in the bundle, so a value that merely CONTAINED another +// id was partially rewritten into a string that references nothing and existed +// on neither side. The carry posture makes this load-bearing: an unresolvable +// value is deliberately imported verbatim rather than dropped, and a partial +// rewrite breaks that promise silently. +// +// THE PREFIX RELATIONSHIP IS THE FIXTURE, not an accident of naming. The +// sibling test above deliberately chose ids sharing no prefix so its own +// assertion could not fail for this reason — which is how the defect went +// unexamined: the fixture was engineered around it instead of at it. +func TestImportWorkspace_UnresolvableRelationValueSurvivesAPrefixCollision(t *testing.T) { + s := testStore(t) + owner, err := s.CreateUser(models.UserCreate{ + Name: "Owner", Email: "prefix-collision-owner@example.com", Password: "passw0rd!", + }) + if err != nil { + t.Fatalf("CreateUser: %v", err) + } + + const ( + liveColorID = "old-color-1" // a real item in the bundle; gets remapped + danglingID = "old-color-10" // NOT an item; contains liveColorID as a prefix + ) + + export := &models.WorkspaceExport{ + Version: 1, + ExportedAt: "2026-09-05T00:00:00Z", + Workspace: models.WorkspaceExportMeta{Name: "Prefix Archive", Slug: "prefix-archive"}, + Collections: []models.CollectionExport{ + { + ID: "old-coll-colors", Name: "Colors", Slug: "colors", Prefix: "COLO", + Schema: `{"fields":[{"key":"status","type":"select","options":["open","done"],"default":"open","required":true}]}`, + CreatedAt: "2026-09-05T00:00:00Z", UpdatedAt: "2026-09-05T00:00:00Z", + }, + { + ID: "old-coll-cars", Name: "Cars", Slug: "cars", Prefix: "CAR", + Schema: `{"fields":[{"key":"status","type":"select","options":["open","done"],"default":"open","required":true},{"key":"color","type":"relation","collection":"colors"}]}`, + CreatedAt: "2026-09-05T00:00:00Z", UpdatedAt: "2026-09-05T00:00:00Z", + }, + }, + Items: []models.ItemExport{ + { + ID: liveColorID, CollectionID: "old-coll-colors", + Title: "Red", Slug: "red", Fields: `{}`, Tags: `[]`, + CreatedAt: "2026-09-05T00:00:00Z", UpdatedAt: "2026-09-05T00:00:00Z", + }, + { + ID: "old-car-dangling", CollectionID: "old-coll-cars", + Title: "Dangling", Slug: "dangling", + Fields: `{"color":"` + danglingID + `"}`, Tags: `[]`, + CreatedAt: "2026-09-05T00:00:00Z", UpdatedAt: "2026-09-05T00:00:00Z", + }, + { + // CONTROL: an EXACT match still gets remapped to the new id. + // Without this leg the test passes against a build that + // stopped remapping altogether. + ID: "old-car-resolvable", CollectionID: "old-coll-cars", + Title: "Resolvable", Slug: "resolvable", + Fields: `{"color":"` + liveColorID + `"}`, Tags: `[]`, + CreatedAt: "2026-09-05T00:00:00Z", UpdatedAt: "2026-09-05T00:00:00Z", + }, + }, + } + + ws, err := s.ImportWorkspace(export, "Prefix Archive Target", owner.ID) + if err != nil { + t.Fatalf("ImportWorkspace: %v", err) + } + + byTitle := map[string]string{} + items, err := s.ListItems(ws.ID, models.ItemListParams{}) + if err != nil { + t.Fatalf("ListItems: %v", err) + } + for _, it := range items { + var m map[string]any + if err := json.Unmarshal([]byte(it.Fields), &m); err != nil { + t.Fatalf("parse fields for %q (%q): %v", it.Title, it.Fields, err) + } + if v, ok := m["color"].(string); ok { + byTitle[it.Title] = v + } + if it.Title == "Red" { + byTitle["__red_id__"] = it.ID + } + } + + if got := byTitle["Dangling"]; got != danglingID { + t.Fatalf("the unresolvable value was rewritten to %q; it must carry VERBATIM as %q — "+ + "a partial rewrite produces a value that existed on neither side", got, danglingID) + } + if got, want := byTitle["Resolvable"], byTitle["__red_id__"]; got != want { + t.Fatalf("the EXACT-match value carried as %q, want the imported item's new id %q; "+ + "the remap has stopped working", got, want) + } +}