diff --git a/internal/store/export.go b/internal/store/export.go index 8aa4a398..4fe6f594 100644 --- a/internal/store/export.go +++ b/internal/store/export.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "log/slog" + "strings" "time" "unicode/utf8" @@ -851,59 +852,36 @@ func remapFieldIDs(fieldsJSON string, itemMap, collMap map[string]string) string } // 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 + // This was `strings.ReplaceAll` over the RAW id, 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 existed on neither side. 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. + // It matters 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. // - // 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. + // Matching the QUOTED JSON token is what makes it whole-value: `"c-1"` + // does not occur inside `"c-10"`, and the form still reaches ids inside + // arrays, which a multi-valued relation needs. + // + // DELIBERATELY NOT an unmarshal/marshal walk. That was this fix's first + // version and the full suite caught it: re-encoding rewrites the WHOLE + // blob, so a stored `\ufffd` escape came back as the literal replacement + // character and broke the NUL-repair import test, which asserts on the + // stored bytes. Every byte this function does not deliberately change has + // to survive it, and only a textual substitution guarantees that. // // 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 + result := fieldsJSON + for oldID, newID := range itemMap { + if oldID == "" || newID == "" { + continue } - return t - case []any: - out := make([]any, len(t)) - for i, e := range t { - out[i] = remapFieldValue(e, itemMap) - } - return out - default: - return v + result = strings.ReplaceAll(result, `"`+oldID+`"`, `"`+newID+`"`) } + return result } diff --git a/web/src/lib/components/items/CopyItemDialog.svelte b/web/src/lib/components/items/CopyItemDialog.svelte index 66459d9c..75e42ebe 100644 --- a/web/src/lib/components/items/CopyItemDialog.svelte +++ b/web/src/lib/components/items/CopyItemDialog.svelte @@ -947,8 +947,18 @@ user hunting for an item that provably does not exist. // so it rendered through the fallback as the raw enum string. Rare // while only `github_pr` produced it; routine since TASK-2878, which // emits it for every carried relation value on a cross-workspace copy. + // + // NEUTRAL WORDING, for the same reason `not_found` below has it, + // and it took a second reviewer to see that the same rule applied + // here (codex round 18). This reason is emitted for EVERY carried + // cross-workspace relation WITHOUT resolving the target, so the + // value may name a live item, a deleted one, one the caller cannot + // see, or nothing at all — and `github_pr` reaches it too, where + // the referent is not in any workspace. "It points at something in + // the source workspace" asserted both existence and location, and + // the response says neither. case 'referent_not_portable': - return 'it points at something in the source workspace'; + return 'this reference cannot be carried to the destination'; // The three same-workspace referent failures (TASK-2878). Worth // separate sentences: "no such item" and "wrong collection" send the // reader to different fixes, and a missing target is a schema problem diff --git a/web/src/lib/types/index.ts b/web/src/lib/types/index.ts index 502cd181..85a9d247 100644 --- a/web/src/lib/types/index.ts +++ b/web/src/lib/types/index.ts @@ -696,6 +696,36 @@ export interface Item { convention?: ItemConventionMetadata; implementation_notes?: ItemImplementationNote[]; decision_log?: ItemDecisionLogEntry[]; + /** + * Advisory notes about a write that SUCCEEDED. Present on create and + * update responses only — never on a read, and never stored — so + * `undefined` is the normal case and says nothing went unreported. + * + * The Go side has carried this since BUG-2850 and TASK-2878 added two + * more members; nothing here mirrored it, so typed code could not reach + * a warning the server was already sending (codex round 18). + */ + warnings?: ItemWriteWarnings; +} + +/** + * Mirrors `models.ItemWriteWarnings`. Every member is optional because the Go + * struct marks each `omitempty`: an absent key means "nothing to report for + * this kind", not "unknown". + */ +export interface ItemWriteWarnings { + /** Field keys stored in the blob that the collection's schema does not declare (BUG-2850). */ + undeclared_fields?: string[]; + /** Keys the write DISCARDED — a relation whose referent did not resolve, a value the destination schema has no home for (TASK-2878). */ + dropped_fields?: string[]; + /** + * Keys KEPT with a value that does not resolve to a live item. Import only: + * an import must carry junk relation values rather than refuse them, or no + * artifact written before referent validation could be imported. Distinct + * from `dropped_fields` — the value survives — and from `undeclared_fields`, + * which is about the key rather than what it points at. + */ + unresolved_relations?: string[]; } // ─── Items index (skinny projection) ─────────────────────────────────────────