fix(models): refuse an append that would destroy an unreadable structured field (BUG-2627, part 3)

[xarmian]

Part 3 of BUG-2627. Ships first by design — see Ordering below.

The defect

AppendImplementationNote / AppendDecisionLogEntry rebuilt their entry slice from Extract*, then assigned it over the field key unconditionally. When the stored value was something Extract* could not decode, Extract* returned nil and the assign overwrote that value with a one-element slice — reporting success.

Observed live, not hypothesised. A scratch item corrupted to the real-world shape, carrying two notes:

pad item note TASK-2668 "third note appended via the remedy path"
→ "Added implementation note to TASK-2668"     ← success

read back → one note. note-A and note-B are GONE.

The design point

The guard cannot key on Extract* returning nil — the obvious shape, and the wrong one. Extract* returns nil for four different reasons and only one is a defect:

	stored value	correct behaviour
1	key absent	proceed — the first append on an item
2	empty array	proceed — well-formed, just empty
3	explicit JSON null	proceed — carries no entries
4	undecodable value	refuse

if Extract(...) == nil { refuse } passes every refusal test and breaks every first append. So assertStructuredFieldAppendable tests decodability against the raw value in the fields map, which is the only check separating (4) from (1)–(3).

The error message

Deliberately does not name an append path. Per PATTE-135 a suggested remedy has to work in the state where the message appears, and every append path is precisely what's being refused. pad item show --format json is read-only and does surface the raw value, so it's the one action safe to suggest. A test asserts the message never names pad item note.

That requirement came from running the trace rather than reasoning about it — which is also what produced the ordering below.

Ordering

3 → 1 → 2. Load-bearing, not stylistic: part 2's error message names a remedy that destroys affected rows until this guard exists. Shipping part 2 first would weaponize its own error message.

Verification

Every mutant was run, and the killing assertion recorded:

mutant	killed by
guard removed	the refusal legs
present && Extract(...) == nil — the plausible-wrong guard	the empty_list / explicit_null control legs (it passes all refusal tests)
returns mutated fields alongside the error	the fields != "" assertion (an errors.Is check alone passes it)
message names pad item note	the message assertion
prepend instead of append	the ordering assertion
rebuild fieldsMap fresh	the sibling-field (github_pr) test
wrong-but-compiling type parameter, both directions	the type-discriminating cases

All mutants removed afterwards, verified by grep.

End-to-end against a binary built from this branch: the trace that destroyed two notes now exits non-zero and both notes read back byte-identical; a healthy item still takes a first note and appends onto an existing one.

Gates

• make lint — 0 issues
• go test ./... — exit 0, 25 packages
• make test-pg — exit 0, 3276 tests. Run, not waived: no SQL changed, but this is a shared writer the server handlers reach
• web checks — not applicable, zero web files
• Codex CLI — CLEAN at round 3 (3 rounds, findings below)

Codex rounds

• R1 — P1: item moves drop the reserved arrays entirely. Independently verified and reproduced live, then filed as BUG-2674 rather than fixed here: different mechanism, different door, and it destroys valid data. P2: generic --field writers remain a bypass — that is part 2. Nit: test coverage didn't support the claimed matrix → fixed.
• R2 — a real hole, not a coverage complaint: the guard is generic, so a wrong-but-compiling type parameter passes every test while permitting an append the extractor reads as empty. Both directions now pinned.
• R3 — CLEAN. Three nit-level accuracy notes on my commit messages; the one that mattered (an undercount that also sat in the code comment) is fixed where a maintainer reads it.

Known, deliberate non-goals

--field implementation_notes=... still reaches the fields patch and overwrites — that's part 2. Item moves still drop these fields — that's BUG-2674.

https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V