From 0ff928d6cf7ee6f43b2457c99ef401cb1855f535 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Tue, 6 Oct 2026 20:37:37 +0200 Subject: [PATCH] docs(quick-261006-seq): fix nullable pointer scalars in cabana ML fields --- .planning/STATE.md | 3 +- .../261006-seq-PLAN.md | 167 ++++++++++++++++++ .../261006-seq-SUMMARY.md | 78 ++++++++ 3 files changed, 247 insertions(+), 1 deletion(-) create mode 100644 .planning/quick/261006-seq-fix-nullable-pointer-scalars-rendering-a/261006-seq-PLAN.md create mode 100644 .planning/quick/261006-seq-fix-nullable-pointer-scalars-rendering-a/261006-seq-SUMMARY.md diff --git a/.planning/STATE.md b/.planning/STATE.md index 28bebbc..3ecbb6c 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -31,7 +31,7 @@ See: .planning/PROJECT.md (updated 2026-09-16) Phase: 15 (Journal plugin) — EXECUTING Plan: 4 of 4 Status: Ready for phase verification -Last activity: 2026-10-06 — Completed quick task 261006-s0v: Remove copy-from locale UI from ML text and markdown admin fields +Last activity: 2026-10-06 — Completed quick task 261006-seq: Fix nullable pointer scalars rendering as /address in cabana ML form fields Progress: [██████████] 100% @@ -645,6 +645,7 @@ Recent decisions affecting current work: | 261005-qvk | Map Winter icon-* navigation names onto lucide so BM Studies and Quizzes icons render | 2026-10-05 | a00dafa | [261005-qvk-fix-icons-sm-bm-plugin-tried-to-apply-ic](./quick/261005-qvk-fix-icons-sm-bm-plugin-tried-to-apply-ic/) | | 261006-eyj | Widget form field payload request and structured data response channel | 2026-10-06 | e723c39 | [261006-eyj-widget-field-payload-and-data-channel](./quick/261006-eyj-widget-field-payload-and-data-channel/) | | 261006-s0v | Remove copy-from locale UI from ML text and markdown admin fields | 2026-10-06 | 2f03128 | [261006-s0v-remove-copy-from-locale-ui-from-ml-text-](./quick/261006-s0v-remove-copy-from-locale-ui-from-ml-text-/) | +| 261006-seq | Fix nullable pointer scalars rendering as /address in cabana ML form fields | 2026-10-06 | 90b87d6 | [261006-seq-fix-nullable-pointer-scalars-rendering-a](./quick/261006-seq-fix-nullable-pointer-scalars-rendering-a/) | ## Deferred Items diff --git a/.planning/quick/261006-seq-fix-nullable-pointer-scalars-rendering-a/261006-seq-PLAN.md b/.planning/quick/261006-seq-fix-nullable-pointer-scalars-rendering-a/261006-seq-PLAN.md new file mode 100644 index 0000000..9e6e163 --- /dev/null +++ b/.planning/quick/261006-seq-fix-nullable-pointer-scalars-rendering-a/261006-seq-PLAN.md @@ -0,0 +1,167 @@ +--- +phase: quick-261006-seq +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - modules/cabana/field_ml.go + - modules/cabana/ml_nullable_test.go +autonomous: true +requirements: [QUICK-261006-seq] + +estimate: + tokens: 30000 + raw_tokens: 30000 + tasks: 2 + confidence: low + +must_haves: + truths: + - "An mltext/mlmarkdown field backed by a nil *string host column hydrates every enabled locale to the empty string, never the text " + - "Saving an empty default-locale value into a *string mltext field and reading it back (save response and ShowRecord) yields the empty string, never a 0x pointer address" + - "Saving non-empty text into a *string mltext field stores that text in the host column, shows it on ShowRecord, and still sends non-default locales through TranslationWriter" + - "Any pointer-to-scalar host value (*int, *int64, *uint, *float64, *bool, **string, pointer to a named string type, *[]byte) hydrates to the dereferenced value's text; a nil pointer at any depth hydrates to the empty string" + - "Plain string, []byte and non-pointer scalar host values hydrate exactly as before (existing ML tests stay green)" + artifacts: + - path: modules/cabana/field_ml.go + provides: "hostScalarString dereferences pointers via reflect before stringifying" + - path: modules/cabana/ml_nullable_test.go + provides: "Postgres round-trip regression for a *string mltext field plus table-driven hostScalarString and DB-free hydrateMLRecord unit tests" + key_links: + - from: "modules/cabana/crud.go projectRecord" + to: "modules/cabana/field_ml.go hydrateMLRecord -> hostScalarString" + via: "projectRecord stores f.Interface() (a raw *string for nullable columns) in RecordResult.Data; hydrateMLRecord reads data[name] and passes it to hostScalarString for the default locale" + - from: "modules/cabana/field_ml.go liftMLValues" + to: "modules/lagoon/fill.go setField" + via: "the default-locale string is written back onto the body and lagoon.Fill allocates a new *string for pointer fields (reflect.Ptr branch) - save path audited, no change needed" +--- + + +Fix cabana ML host hydration so nullable pointer scalars render as their value instead of the literal "" or a pointer address like "0x4f685100150". + +Bug (Journal UAT, Categories -> Description, an `mltext` field on a `Description *string` GORM column): `projectRecord` (modules/cabana/crud.go) stores the raw `*string` in `RecordResult.Data`; `hydrateMLRecord` passes it to `hostScalarString` (modules/cabana/field_ml.go), whose `v == nil` check misses a typed nil pointer and whose default branch formats the pointer itself — "" for a nil pointer, the address for a live one (after clearing and saving, Fill stores a pointer to ""). + +Purpose: the framework must hydrate any pointer-to-scalar host column correctly; the fix belongs in cabana, not in plugin YAML. +Output: a reflect-based `hostScalarString`, a Postgres round-trip regression test, table-driven unit tests, two code commits. + + + +@~/.claude/gsd-core/workflows/execute-plan.md +@~/.claude/gsd-core/templates/summary.md + + + +@.planning/STATE.md +@CLAUDE.md +@modules/cabana/field_ml.go +@modules/cabana/ml_smoke_test.go +@modules/cabana/ml_test.go + +Facts gathered during planning (do not re-derive): + +- `hostScalarString(v any) string` (field_ml.go, bottom of file) is unexported and has exactly one caller: `hydrateMLRecord`, for the default locale. It is the ONLY `fmt.Sprint`-on-an-arbitrary-value site in non-test cabana code (the other `%v`/Sprintf uses in list_schema.go, relation.go, commands.go, http.go, query.go format YAML keys, ints or log text — not record values). No other cabana helper shares the pattern. +- `projectRecord` (crud.go ~line 1187) does `out[field.Name] = f.Interface()`, so nullable columns arrive as `*string`, `*int`, etc. JSON encoding of those for non-ML fields is already correct (null / value); only the ML hydration stringifies them. +- Save path audited: `liftMLValues` writes `values[defaultLocale]` (a `string`) back onto the body; `lagoon.Fill` -> `setField` (modules/lagoon/fill.go ~line 113) has a `field.Kind() == reflect.Ptr` branch that converts into the element type and stores a fresh pointer. So writing a string into a `*string` host field already works; an empty default-locale value stores a pointer to "" (WinterCMS also stores "" unless a model opts into nullable conversion). No save-path change is in scope — the round-trip test only pins this behavior. +- Test fixtures to reuse (ml_smoke_test.go): `recordingWriter` (DefaultLocale/EnabledLocales ignore tx; `attrs[locale][field]` records non-default writes), `mlFS()`, `mlListConfig`, `mlFormConfig`, `mlColumns`, `mlCompiled(t)`, `formPlugin{fsys: ...}`, `controllerRef{plugin, ctl}`, `compileRegistry`. `recordingWriter.TranslatedExact` type-asserts `*mlPost` only for the default locale, which `hydrateMLRecord` never requests — safe with another model. +- `newListService(t)` (query_test.go) boots a shared testcontainers Postgres; existing ML tests (`TestMLNestedSave`, `TestMLHydration`) use it with `DropTable` + `AutoMigrate`. +- cabana's writable compile requires the model to implement `lagoon.HasFillable` (`Fillable() []string`) listing every form column. +- Go is 1.27, so `new(expr)` (Go 1.26+) is available for pointer literals in tests; no test helper named `ptr`/`strPtr` exists in the package — do not add a generic helper with those names. +- `hostScalarString` is internal: `modules/cabana/README.md` and `docs/` do NOT change (no exported API, config key or CLI change). +- Commit rules: one logical change per commit, planning docs never in a code commit, no co-author trailers (user's global CLAUDE.md overrides any attribution reminder). + + + + + + Task 1: Round-trip regression for a *string mltext field, then dereference pointers in hostScalarString + modules/cabana/ml_nullable_test.go, modules/cabana/field_ml.go + modules/cabana/field_ml.go (hydrateMLRecord and hostScalarString), modules/cabana/ml_smoke_test.go (fixtures), modules/cabana/ml_test.go (TestMLHydration shape) + A Docker daemon is reachable so newListService can start the testcontainers Postgres used by the existing ML tests. + + - Seeded row with Title "Hello" and Description nil: ShowRecord returns Data["description"] as map[string]string{"en": "", "pl": ""}. + - UpdateRecord with description {"en": "", "pl": ""}: the returned Data["description"]["en"] is "" and a following ShowRecord also gives ""; the stored row's Description is a non-nil pointer to "" (pins lagoon.Fill pointer semantics). + - UpdateRecord with description {"en": "Opis kategorii", "pl": "Opis"}: ShowRecord gives en "Opis kategorii" and pl "Opis"; the stored row's Description dereferences to "Opis kategorii"; recordingWriter.attrs["pl"]["description"] is "Opis". + - No hydrated locale value anywhere in the test contains "" or starts with "0x". + + +RED first. Create modules/cabana/ml_nullable_test.go (package cabana). Add a model mlNullablePost with fields ID uint (gorm column id, primaryKey), Title string (column title) and Description *string (column description); TableName "cabana_ml_nullable_posts"; Fillable returns title and description; Rules requires only title. Add mlNullableController mirroring mlController (same ID "acme.demo.posts", ModelName "Post", ConfigDir "controllers/posts", pass-through FormExtendQuery) whose NewRecord returns a new mlNullablePost pointer, with the same pact interface assertions mlController has. Add mlNullableFS returning the mlFS() map with models/post/fields.yaml replaced by YAML declaring title (label Title, type mltext, required true) and description (label Description, type mltext). Add mlNullableCompiled(t) built like mlCompiled but with formPlugin{fsys: mlNullableFS()} and mlNullableController{}. + +Write TestMLNullablePointerHost implementing the behavior block: newListService, DropTable then AutoMigrate mlNullablePost, recordingWriter with defaultLocale "en" and enabled en/pl, CRUDService{DB: db, writer: writer}. Seed the row through db.Create with Title "Hello" and Description nil, then exercise ShowRecord / UpdateRecord with the row ID (always sending title as an en/pl map too, since title is required) and read the row back with db.First for the stored-value assertions. Add a small file-local assertion helper that fails when a hydrated value contains "" or has prefix "0x". Run it and confirm it FAILS on the current code (the nil case yields "", the cleared case an address). + +GREEN. In field_ml.go rewrite hostScalarString using reflect (add the reflect import): a nil interface returns ""; take reflect.ValueOf(v) and while its Kind is reflect.Pointer return "" if IsNil, otherwise step to Elem — so any pointer depth works and nil at any depth is ""; then a String kind returns the value's String() (raw stored text, also for named string types, bypassing any String method); a slice whose element kind is Uint8 returns string(Bytes()); every other kind returns fmt.Sprint of the dereferenced value's Interface() — never of the original pointer — which keeps existing formatting for plain ints, floats and bools. Update the doc comment above hostScalarString to say nullable pointer columns dereference and nil becomes the empty string. Do not touch liftMLValues, applyMLTranslations, lagoon.Fill, or projectRecord (save path audited, see context). Do not edit modules/cabana/README.md or docs/ — the helper is unexported. + +Run go vet ./modules/cabana and the targeted tests; all must pass, including the existing TestMLHydration and TestMLNestedSave. + +Commit the fix and the regression test together: fix(cabana): dereference nullable pointer scalars in ML host hydration (or equivalent). No .planning files, no co-author trailers. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go vet ./modules/cabana && go test ./modules/cabana -run 'TestMLNullablePointerHost|TestMLHydration|TestMLNestedSave|TestML$' -count=1 + + TestMLNullablePointerHost failed before the fix and passes after it; a nil *string mltext host shows "" for every locale, a cleared-then-saved one shows "" (not 0x...), a filled one shows its text; existing ML tests still pass; one code commit. + + + + Task 2: Unit tests for hostScalarString and DB-free hydrateMLRecord over pointer scalars + modules/cabana/ml_nullable_test.go + modules/cabana/ml_nullable_test.go (as left by Task 1), modules/cabana/field_ml.go (new hostScalarString) + + - nil interface, (*string)(nil), (*int)(nil), a **string pointing at a nil *string, and (*[]byte)(nil) all give "". + - Pointer to "" gives ""; pointer to "Opis" gives "Opis"; a **string pointing at a pointer to "x" gives "x". + - Pointer to int 42 gives "42"; pointer to int64 -7 gives "-7"; pointer to uint 9 gives "9"; pointer to float64 1.5 gives "1.5"; pointer to bool true gives "true". + - A file-local named string type (for example nullableSlug) gives its raw text both as a value and through a pointer. + - []byte("abc") and a pointer to it give "abc". + - Regression: plain string "plain" gives "plain", plain int 7 gives "7". + - Every case's result is checked to not contain "" and not start with "0x". + - hydrateMLRecord called directly (context.Background(), a nil *gorm.DB, mlCompiled(t), a recordingWriter with default en and enabled en/pl, a nil model, op "update") turns data["title"] = (*string)(nil) into map[string]string{"en": "", "pl": ""}, a pointer to "" into en "", and a pointer to "Hello" into en "Hello" with pl "". + + +Append to modules/cabana/ml_nullable_test.go: TestHostScalarString as a table-driven test (name, input any, want string) covering every case in the behavior block, using new(value) for pointer literals and declaring the named string type and any double-pointer variables locally; each row also runs the shared no-"" / no-"0x" assertion from Task 1. Add TestMLHydrationPointerHost that calls hydrateMLRecord directly with a nil tx (recordingWriter ignores it, so no Postgres is needed) for the three title cases in the behavior block, asserting the resulting map with reflect.DeepEqual. Keep tests as plain func TestX(t *testing.T) with t.Run subtests, consistent with the package. + +These tests target the Task 1 implementation, so they pass immediately; if any row fails, fix hostScalarString in field_ml.go rather than weakening the expectation, and include that file in the commit. + +Run gofmt, go vet ./... and go test ./... from the repo root; both must be green per CLAUDE.md. + +Commit: test(cabana): cover pointer scalars in ML host hydration (or equivalent). No .planning files, no co-author trailers. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && test -z "$(gofmt -l modules/cabana)" && go test ./modules/cabana -run 'TestHostScalarString|TestMLHydrationPointerHost|TestMLNullablePointerHost' -count=1 -v 2>&1 | tail -40 + + Table-driven and DB-free hydration tests cover nil and non-nil pointers to string, int, int64, uint, float64, bool, []byte, a named string type and a double pointer, plus plain-value regressions; go vet ./... and go test ./... are green; one test commit. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| cabana admin API -> admin SPA | Record data hydrated server-side is rendered in authenticated admin forms | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan | +|-----------|----------|-----------|----------|-------------|-----------------| +| T-quick-261006-seq-01 | Information disclosure | modules/cabana/field_ml.go hostScalarString | low | mitigate | Pointer dereference stops Go heap addresses (0x...) reaching admin responses; TestHostScalarString and TestMLNullablePointerHost assert no hydrated value starts with 0x | +| T-quick-261006-seq-02 | Tampering | ML save into *string host fields | low | accept | Save path unchanged; lagoon.Fill still only writes allowed fill keys and converts into the pointer element type; the round-trip test pins the stored value | +| T-quick-261006-seq-SC | Tampering | go module installs | low | accept | No new dependencies; only stdlib reflect is added to an existing file | + + + +- go vet ./... (repo root) is clean +- go test ./... (repo root) is green, including the testcontainers-backed cabana ML tests +- gofmt -l modules/cabana prints nothing +- git show --stat of the two new commits touches only modules/cabana/field_ml.go and modules/cabana/ml_nullable_test.go (no README, docs or .planning files) + + + +- Journal Categories -> Description (mltext on *string) shows an empty editor when the column is NULL, not "". +- Clearing it and saving shows an empty editor, not a 0x address; typing text and saving shows that text. +- Every pointer-to-scalar host column hydrates by value; nil at any depth is "". +- No exported API change, so cabana README and docs stay as they are. + + + +Create `.planning/quick/261006-seq-fix-nullable-pointer-scalars-rendering-a/261006-seq-SUMMARY.md` when done + diff --git a/.planning/quick/261006-seq-fix-nullable-pointer-scalars-rendering-a/261006-seq-SUMMARY.md b/.planning/quick/261006-seq-fix-nullable-pointer-scalars-rendering-a/261006-seq-SUMMARY.md new file mode 100644 index 0000000..9704448 --- /dev/null +++ b/.planning/quick/261006-seq-fix-nullable-pointer-scalars-rendering-a/261006-seq-SUMMARY.md @@ -0,0 +1,78 @@ +--- +phase: quick-261006-seq +plan: 01 +subsystem: cabana +tags: [cabana, ml, hydration, nullable, reflect] +status: complete +requires: [] +provides: + - "hostScalarString dereferences pointer host columns at any depth; nil becomes empty string" +affects: + - "Any mltext/mlmarkdown field backed by a nullable pointer GORM column (Journal Categories -> Description)" +tech-stack: + added: [] + patterns: + - "reflect pointer walk before stringifying projected host values" +key-files: + created: + - modules/cabana/ml_nullable_test.go + modified: + - modules/cabana/field_ml.go +decisions: + - "Fix lives in cabana hostScalarString, not plugin YAML; save path (liftMLValues -> lagoon.Fill) left unchanged and only pinned by the round-trip test" + - "String kinds return raw text via reflect.Value.String(), bypassing any String() method on named string types" +metrics: + duration: "~5 min" + completed: 2026-10-06 +actuals: + tokens: 2400 + tasks: 2 + commits: 2 +plan_head_before: 8e22e5f25b67f24759d36e6befdfd6c20433b9ea +plan_head_after: 4f7e69fd2b25bd55d7c571b31ccfaca2e8ae9ec7 +--- + +# Quick 261006-seq Plan 01: Fix nullable pointer scalars rendering in ML hydration Summary + +`hostScalarString` now walks pointers through reflect, so an mltext field on a nullable `*string` column hydrates to `""` or to its text. Before the fix it showed `""` or a heap address such as `0x36b66873bb0`. + +## Tasks + +| Task | Name | Commit | Files | +| ---- | ---- | ------ | ----- | +| 1 | Round-trip regression for a *string mltext field, then dereference pointers in hostScalarString | 90b87d6 | modules/cabana/field_ml.go, modules/cabana/ml_nullable_test.go | +| 2 | Unit tests for hostScalarString and DB-free hydrateMLRecord over pointer scalars | 4f7e69f | modules/cabana/ml_nullable_test.go | + +## What changed + +- `modules/cabana/field_ml.go`: `hostScalarString` returns `""` for a nil interface. It then dereferences pointers at any depth and returns `""` for a nil pointer at any depth. String kinds return their raw text, including named string types and bypassing their `String()` methods. Byte slices are decoded as text. Every other kind goes through `fmt.Sprint` of the dereferenced value, never of the pointer. A doc comment was added and `reflect` is now imported. +- `modules/cabana/ml_nullable_test.go`: + - `TestMLNullablePointerHost` is a Postgres round trip with `mlNullablePost.Description *string`. It covers a nil host, a cleared value (stored as a pointer to `""`) and a filled value (the host text is stored and Polish goes through `TranslationWriter`). + - `TestHostScalarString` has 19 table rows. + - `TestMLHydrationPointerHost` calls `hydrateMLRecord` directly with a nil tx. + +## TDD evidence + +RED: before the fix, `TestMLNullablePointerHost` failed with `show description.en = ""` and `save description.en = "0x36b66873bb0"`. GREEN: it passes after the fix, and `TestMLHydration`, `TestMLNestedSave` and `TestML` still pass. + +## Verification + +- `go vet ./...` is clean at both commits. +- `go test ./... -count=1` is green at both commits, including the testcontainers cabana tests. +- `gofmt -l modules/cabana` prints nothing. +- The two commits touch only `modules/cabana/field_ml.go` and `modules/cabana/ml_nullable_test.go`. They change no README, docs or .planning files and carry no co-author trailers. + +## Deviations from Plan + +None. The plan was executed as written. One small addition: the file-local `nullableSlug` type has a `String()` method that returns `"decorated"`. The named-string rows therefore show that the raw stored text is returned, not the Stringer output. + +## Threat Flags + +None. T-quick-261006-seq-01 is mitigated: tests assert that no hydrated value starts with `0x` or contains ``. + +## Self-Check: PASSED + +- FOUND: modules/cabana/field_ml.go +- FOUND: modules/cabana/ml_nullable_test.go +- FOUND: 90b87d6 +- FOUND: 4f7e69f