From 6472f2861eca403344f882440e8f288dfbe3779f Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Tue, 27 Oct 2026 15:47:24 +0100 Subject: [PATCH] docs(32): reconcile review disposition with the fix report --- .../phases/32-seo-foundation/32-REVIEW-FIX.md | 254 +++++++++--------- 1 file changed, 130 insertions(+), 124 deletions(-) diff --git a/.planning/phases/32-seo-foundation/32-REVIEW-FIX.md b/.planning/phases/32-seo-foundation/32-REVIEW-FIX.md index 9b5ff24e..82d4f920 100644 --- a/.planning/phases/32-seo-foundation/32-REVIEW-FIX.md +++ b/.planning/phases/32-seo-foundation/32-REVIEW-FIX.md @@ -11,38 +11,13 @@ status: partial # Phase 32: Code Review Fix Report -Two fixer lanes ran in parallel on disjoint repositories. CR-04 was split: the framework half (WidgetField.vue element ordering) and the plugin half (element action queue with dropped-action retry); both are fixed. Skipped: WR-03 (needs a design decision: a way to mark a failing extension's fields unavailable so the admin does not post them back). +Two fixer lanes ran in parallel on disjoint repositories. CR-04 was split into a framework half (WidgetField.vue element ordering) and a plugin half (element action queue with dropped-action retry); both are fixed. ---- +## Fixed Issues -## Lane A: summercms.go and sm-journal-plugin +### CR-01: [CR-A01] `required: true` on a form extension field makes every save of the host form fail (also for administrators who cannot see the extension) - -### Phase 32: Code Review Fix Report (lane A + framework half of CR-04) - -**Fixed at:** 2026-10-27T15:55:00Z -**Source review:** .planning/phases/32-seo-foundation/32-REVIEW.md -**Iteration:** 1 - -The scope was CR-01, CR-02, WR-01..WR-07 (lane A), plus the framework half of CR-04 (WidgetField.vue). The sm-seo-plugin half of CR-04 was out of scope and not touched. - -**Summary:** -- Findings in scope: 10 -- Fixed: 9 (6 commits in summercms.go, 3 in sm-journal-plugin) -- Skipped: 1 (WR-03, needs a design decision) - -**Verification ran in the main checkout.** `workflow.use_worktrees` is false, so no worktree was used. -- summercms.go: `go vet ./...` clean; `go test -short ./...` green; Docker suites `go test ./modules/cabana ./modules/signpost` green; `examples/hello` green. -- admin: `npm --prefix admin run typecheck`, `npm --prefix admin run test` (1564 tests) and the build all pass. `scripts/check-admin-dist.sh` passes. -- docs: `summer docs:build --check` reports no problems. -- release gates: `scripts/check-phase32.sh --marker --additive` passes, with +0 exported API lines in cabana, pact and signpost against v0.5.1. -- sm-journal-plugin: `go vet ./...` clean; `go test ./...` (Docker) green. - -Nothing was tagged or pushed. - -### Fixed Issues - -### CR-01: `required: true` on a form extension field makes every save of the host form fail +_Lane: summercms.go / sm-journal-plugin_ **Status:** fixed **Repo:** summercms.go @@ -58,7 +33,9 @@ Nothing was tagged or pushed. - An administrator who does not see the extension can create and update freely. - Docs bullet added. -### CR-02: Plugin field-type upload route ignores the extension's Permissions/Visible +### CR-02: [CR-A02] Plugin field-type upload route ignores the extension's Permissions/Visible, so administrators without them can run its `Upload` + +_Lane: summercms.go / sm-journal-plugin_ **Status:** fixed **Repo:** summercms.go @@ -70,7 +47,9 @@ Nothing was tagged or pushed. The docs now say the extension's "field type uploads" answer 404 too. I found no other route that resolves an extension field by name without a visibility check. -### CR-04 (framework half): WidgetField assigns `element` after `append` +### CR-04: [CR-C01] The SERP preview and the robots.txt tester deadlock: their first action is dropped by the host and `inflight` is never cleared + +_Lane: summercms.go / sm-journal-plugin_ **Status:** fixed (framework half only; the sm-seo-plugin element changes belong to the other fixer) **Repo:** summercms.go @@ -88,7 +67,31 @@ The docs now say the extension's "field type uploads" answer 404 too. I found no - Dist rebuilt and committed; `check-admin-dist.sh` passes. - Docs note that an action dispatched from `connectedCallback` is taken, and that one dispatched while `busy` is set is dropped. -### WR-01: Admin writes that change or delete records fire no record event +_Lane: sm-seo-plugin_ + +**Status:** fixed (plugin half; `WidgetField.vue` was not touched, as instructed) +**Commit:** 3ecbd1f +**Files:** `assets/src/elements/actions.ts` (new), `serp.ts`, `robotstester.ts`, `labels.ts`, `assets/dist/seo.js`, `tests/elements/host.ts` (new), `tests/elements/serp.test.ts`, `tests/elements/robotstester.test.ts` (new) +**Applied fix:** Both elements now share a new `ActionQueue`: +- **Drop detection:** after `summer-action` is dispatched, a missing `busy` attribute means the host dropped the action. `inflight` is cleared and the payload is retried once on the next task. +- **Second drop:** reported as a failure. The SERP preview shows `contextFailed`; the tester shows a new `testFailed` label (en/pl). +- **First request:** `{op:'context'}` is sent on a microtask after `connectedCallback`. +- **Disconnect:** clears the request in flight and any pending retry. On reconnect the context is requested again, or the duplicate check is re-armed. + +**Tests:** +- `tests/elements/host.ts` is a fake of `WidgetField.onAction`. It sets `busy` synchronously, settles asynchronously, sets `state=error` on failure and removes `busy`. It supports both wiring orders (`append-first` is the v0.5.1 host), dropping N actions, and an unwired host. +- The SERP and tester suites cover: + - the append-first host; + - a dropped retry; + - a host that never takes the action; + - a queue behind `busy`; + - `state=error` unblocking the next request; + - a disconnect while a request is in flight. +- Against the old `serp.ts`, 6 of the 7 new SERP host tests fail. + +### WR-01: [WR-A01] Admin writes that change or delete records fire no record event (hasMany link/unlink, deferred child unlink, deferred child commit) + +_Lane: summercms.go / sm-journal-plugin_ **Status:** fixed **Repo:** summercms.go @@ -112,7 +115,9 @@ The docs now say the extension's "field type uploads" answer 404 too. I found no - New subtests `hasMany link and unlink` and `deferred children` in `TestRecordEvents`. - README and docs list the new event sources. -### WR-02: `RecordSaved.Locale` is the raw Accept-Language header / UI language +### WR-02: [WR-A02] `RecordSaved.Locale` is the admin's UI language or the raw `Accept-Language` header, not a content locale, and reorder reports something different + +_Lane: summercms.go / sm-journal-plugin_ **Status:** fixed (requires human verification of the chosen meaning) **Repo:** summercms.go @@ -133,7 +138,9 @@ The docs now say the extension's "field type uploads" answer 404 too. I found no I deliberately did not take the reviewer's alternative of switching `Locale` to the default content locale: that changes a field's meaning inside a patch release. No consumer reads `ev.Locale` today; I checked sm-seo-plugin, sm-sitemap-plugin and sm-journal-plugin. A human should confirm that "request locale, normalised" is the meaning we want. -### WR-04: Journal API writes can commit changes (including a slug) and fire no RecordSaved +### WR-04: [WR-A04] Journal API writes can commit changes (including a slug) and fire no RecordSaved + +_Lane: summercms.go / sm-journal-plugin_ **Status:** fixed **Repo:** sm-journal-plugin @@ -145,7 +152,9 @@ I deliberately did not take the reviewer's alternative of switching `Locale` to - New test step: a PUT with a new slug and an invalid `tags` value returns 500, fires no event and leaves the slug unchanged. It fails without the fix: the slug was `api-lost`. - README updated. -### WR-05: Journal `postSavedEvent` reports `Old: nil` for a translated `content_html` that existed +### WR-05: [WR-A05] Journal `postSavedEvent` reports `Old: nil` for a translated `content_html` that existed, because the body is mutated between the snapshot and the event + +_Lane: summercms.go / sm-journal-plugin_ **Status:** fixed **Repo:** sm-journal-plugin @@ -155,7 +164,9 @@ I deliberately did not take the reviewer's alternative of switching `Locale` to - `submittedTranslations` lists `content_html` whenever a translation sends a non-blank `content`. This mirrors `classes.FormatTranslation`, so the snapshot and the event cover the same fields whether or not the body has been mutated yet. - New test step: updating a pl `content` reports the pl `content_html` change with the old HTML as `Old`. It failed before the fix, with `Old` nil. -### WR-06: Tags auto-created by the Journal post API fire no RecordSaved +### WR-06: [WR-A06] Tags auto-created by the Journal post API fire no RecordSaved although `journal-tag` is now a content type + +_Lane: summercms.go / sm-journal-plugin_ **Status:** fixed **Repo:** sm-journal-plugin @@ -167,7 +178,9 @@ I deliberately did not take the reviewer's alternative of switching `Locale` to - New test step: a PUT with a new tag name fires two events, the post and a `journal-tag` `Created`; reusing that tag fires only the post event. - README updated. -### WR-07: `Hide` leaves the owner's relation-manager and partial endpoints of a hidden field reachable +### WR-07: [WR-A07] `Hide` leaves the owner's relation-manager and partial endpoints of a hidden field reachable + +_Lane: summercms.go / sm-journal-plugin_ **Status:** fixed **Repo:** summercms.go @@ -180,53 +193,9 @@ I deliberately did not take the reviewer's alternative of switching `Locale` to - New smoke test `TestFormExtensionHideEndpoints` checks that `relationOf` refuses the hidden relation-manager relation and that the hidden partial answers 404. With no `Hide`, both still work. It fails without the fix. - The pact `Hide` doc and the docs updated. -### Skipped Issues +### CR-03: [CR-B01] Exact redirects with a trailing slash or upper-case query characters never match at runtime -### WR-03: A failing extension `Load` (or plugin-type `Load`) after a successful save rolls back the whole save - -**Status:** skipped -**File:** `modules/cabana/crud.go:980`, `modules/cabana/form_extensions.go` (`hydrateFormExtensions`) -**Reason:** Both suggested fixes have problems that need a design decision: -- **Log and omit the extension's keys.** This risks data loss. The admin SPA's `editablePayload` always sends translatable fields (`localeRecord(undefined)`), so a response without them makes the next save post empty locale maps to the extension's `Save`. -- **Run hydration after the commit.** This stops the rollback, but the request still answers 500 after a committed write. Show still fails for the same `Load`, so editing stays blocked either way. - -A proper fix needs an "extension unavailable" state in the schema/record (for example, the failing extension's fields marked read-only or failed, so the SPA does not post them). That is beyond a v0.5.2 patch. - -**Original issue:** `hydrateFormExtensions` runs inside the save transaction, so one plugin's read-side failure turns a successful save into a 500 and rolls it back. `ShowRecord` likewise fails the whole record. - - -_Fixed: 2026-10-27T15:55:00Z_ -_Fixer: Claude (msd-code-fixer)_ -_Iteration: 1_ - ---- - -## Lanes B and C: sm-seo-plugin - - -### Phase 32: Code Review Fix Report (lanes B and C, sm-seo-plugin) - -**Fixed at:** 2026-10-27T14:45:18Z -**Source review:** /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/.planning/phases/32-seo-foundation/32-REVIEW.md -**Iteration:** 1 -**Repository:** /media/nvme/dev/golem15/summercms.io/summercms/sm-seo-plugin (branch `main`, on top of b19cd06; not pushed, not tagged) - -**Summary:** -- Findings in scope: 20 (CR-03, CR-04 plugin half, WR-08 to WR-25) -- Fixed: 20 (WR-15 is a documentation fix only; see its section) -- Skipped: 0 - -**Verification (run in the main checkout of sm-seo-plugin; `workflow.use_worktrees` is false):** -- `go vet ./...`: clean. -- `go test -short ./...`: ok. -- `go test -count=1 ./...` (Docker, testcontainers Postgres 16): ok. -- `npm run typecheck`, `npm test` (77 tests in 7 files), `bash scripts/check-dist.sh`: pass. -- `gofmt -l`: clean. -- Commits follow `fix: ()`, each staged by explicit path, with no co-author trailers. - -### Fixed Issues - -### CR-03: Exact redirects with a trailing slash or upper-case query characters never match at runtime +_Lane: sm-seo-plugin_ **Status:** fixed **Commit:** 2d81c26 @@ -240,36 +209,18 @@ _Iteration: 1_ With the old `lower(source)` fetch the test fails. -### CR-04: The SERP preview and the robots.txt tester deadlock (plugin half only) +### WR-08: [WR-B01] A query-specific exact redirect is silently shadowed by the path-only rule -**Status:** fixed (plugin half; `WidgetField.vue` was not touched, as instructed) -**Commit:** 3ecbd1f -**Files:** `assets/src/elements/actions.ts` (new), `serp.ts`, `robotstester.ts`, `labels.ts`, `assets/dist/seo.js`, `tests/elements/host.ts` (new), `tests/elements/serp.test.ts`, `tests/elements/robotstester.test.ts` (new) -**Applied fix:** Both elements now share a new `ActionQueue`: -- **Drop detection:** after `summer-action` is dispatched, a missing `busy` attribute means the host dropped the action. `inflight` is cleared and the payload is retried once on the next task. -- **Second drop:** reported as a failure. The SERP preview shows `contextFailed`; the tester shows a new `testFailed` label (en/pl). -- **First request:** `{op:'context'}` is sent on a microtask after `connectedCallback`. -- **Disconnect:** clears the request in flight and any pending retry. On reconnect the context is requested again, or the duplicate check is re-armed. - -**Tests:** -- `tests/elements/host.ts` is a fake of `WidgetField.onAction`. It sets `busy` synchronously, settles asynchronously, sets `state=error` on failure and removes `busy`. It supports both wiring orders (`append-first` is the v0.5.1 host), dropping N actions, and an unwired host. -- The SERP and tester suites cover: - - the append-first host; - - a dropped retry; - - a host that never takes the action; - - a queue behind `busy`; - - `state=error` unblocking the next request; - - a disconnect while a request is in flight. -- Against the old `serp.ts`, 6 of the 7 new SERP host tests fail. - -### WR-08: A query-specific exact redirect is silently shadowed by the path-only rule +_Lane: sm-seo-plugin_ **Status:** fixed (requires human verification: matching-order logic) **Commit:** 58b2504 **Files:** `classes/redirects.go`, `classes/redirect_graph_test.go`, `README.md` **Applied fix:** `exactKeys` now returns `path?query` before `path`, so the more specific exact rule wins. A unit test covers both insertion orders, and the README states the precedence. -### WR-09: `AddressChanged` writes redirects without validation or the loop simulation +### WR-09: [WR-B02] `AddressChanged` writes redirects without validation or the loop simulation + +_Lane: sm-seo-plugin_ **Status:** fixed (requires human verification: redirect-graph logic) **Commit:** cb614cb @@ -286,7 +237,9 @@ With the old `lower(source)` fetch the test fails. - an absolute old address; - a `//evil` new address. -### WR-10: The save-time loop check races with concurrent writes +### WR-10: [WR-B03] The save-time loop check races with concurrent writes + +_Lane: sm-seo-plugin_ **Status:** fixed **Commit:** c2f136a @@ -298,7 +251,9 @@ With the old `lower(source)` fetch the test fails. **Test:** `TestConcurrentRedirectWrites` holds A → B uncommitted and checks that a concurrent B → A waits, then is refused as a loop. With the lock removed the test fails. -### WR-11: Undo is not idempotent under concurrent requests +### WR-11: [WR-B04] Undo is not idempotent under concurrent requests + +_Lane: sm-seo-plugin_ **Status:** fixed **Commit:** d20f14f @@ -310,7 +265,9 @@ With the old `lower(source)` fetch the test fails. **Test:** `TestConcurrentUndo` checks that the second concurrent undo gets `already_undone`, the redirect is re-created once, and there is one undo history row. Before the fix the second undo hit a unique violation. -### WR-12: Public resolve and 404 report answer 500 for NUL or invalid UTF-8 paths +### WR-12: [WR-B05] Public resolve and 404 report answer 500 for NUL or invalid UTF-8 paths + +_Lane: sm-seo-plugin_ **Status:** fixed **Commit:** 98fbeb6 @@ -322,7 +279,9 @@ With the old `lower(source)` fetch the test fails. **Test:** in `TestNotFoundLog`, `/%00`, `/%FF` and query variants get 422 from resolve. The report refuses the path cases with 422 and never answers 5xx. -### WR-13: The runtime cycle log grows without bound and is keyed by attacker-controlled URLs +### WR-13: [WR-B06] The runtime cycle log grows without bound and is keyed by attacker-controlled URLs + +_Lane: sm-seo-plugin_ **Status:** fixed **Commit:** c2ba33e @@ -334,7 +293,9 @@ With the old `lower(source)` fetch the test fails. **Test:** `TestCycleLogBounded` sends 5000 URLs through one cycle, which leaves one key, and checks that the map never exceeds the cap. -### WR-14: Export with a large `page` panics the handler +### WR-14: [WR-B07] Export with a large `page` panics the handler + +_Lane: sm-seo-plugin_ **Status:** fixed **Commit:** 59c37a8 @@ -345,7 +306,9 @@ With the old `lower(source)` fetch the test fails. **Test:** `page=9223372036854777` on export and `page=MaxInt64` on a list both answer 200 with no rows. -### WR-15: Public throttle buckets key on the SSR server's IP, capping the whole site +### WR-15: [WR-B08] Public throttle buckets key on the SSR server's IP, capping the whole site + +_Lane: sm-seo-plugin_ **Status:** fixed (documentation only) **Commit:** cab1b9a @@ -353,7 +316,9 @@ With the old `lower(source)` fetch the test fails. **Applied fix:** The README has a new paragraph, "Behind the frontend server". It says the SSR host must be in `http.trusted_proxies` and must forward `X-Forwarded-For`. Without that, `seo-resolve` and `seo-notfound` become caps for the whole site, 600 requests a minute each. The `Buckets` doc comment says the same. **Not done (needs a decision):** an authenticated resolve call that skips the throttle or gets a higher limit. It needs a new token or config key and a framework way for a surf `Bucket` to skip or raise its limit per request; today `FixedWindowLimiter` has no such hook. This is a design and framework change, not a plugin patch. -### WR-16: Social image values bypass validation on every non-admin write path +### WR-16: [WR-B09] Social image values bypass validation on every non-admin write path + +_Lane: sm-seo-plugin_ **Status:** fixed **Commit:** c8e0961 @@ -365,7 +330,9 @@ With the old `lower(source)` fetch the test fails. **Test:** `javascript:`, `data:`, `//` and a value with spaces get 422 on all three MCP routes. -### WR-17: Invalid redirect status, match or query values are silently coerced on API, MCP and import +### WR-17: [WR-B10] Invalid redirect status, match or query values are silently coerced on API, MCP and import + +_Lane: sm-seo-plugin_ **Status:** fixed **Commit:** 24e6f38 @@ -377,7 +344,9 @@ With the old `lower(source)` fetch the test fails. **Tests:** MCP `status: 308`, `match: Regex` and `query: keep` get 422; a batch with 307 gets 422; an import with `glob` or `keep` gives 2 refused rows. -### WR-18: `set_url_seo`, batch `set_seo` and the records import accept any locale +### WR-18: [WR-B11] `set_url_seo`, batch `set_seo` and the records import accept any locale + +_Lane: sm-seo-plugin_ **Status:** fixed **Commit:** cff2d29 @@ -385,7 +354,9 @@ With the old `lower(source)` fetch the test fails. **Applied fix:** When a locale other than the matched one is given, `SetURLSEO` requires it to be one of the locales the record's descriptor describes (existing `recordLocales`). Otherwise it refuses with the new `api.locale_unknown` (en/pl). This covers the API, MCP, batch and the records import. **Test:** `xx` and `EN ` get 422 on `locale`; `pl` is written. -### WR-19: Entry generation overwrites `entry.URL`, which can swallow an address change and its 301 +### WR-19: [WR-B12] Entry generation overwrites `entry.URL`, which can swallow an address change and its 301 + +_Lane: sm-seo-plugin_ **Status:** fixed (requires human verification: address-change logic in the job) **Commit:** d8aa989 @@ -397,7 +368,9 @@ With the old `lower(source)` fetch the test fails. **Test:** `TestGenerateEntries` covers a slug changed by a direct DB write: the job now creates `/pages/one → /pages/one-renamed` and `/pl/pages/one → /pl/pages/one-renamed`. A third run adds nothing. -### WR-20: The social image preview accepts `/\host/...` +### WR-20: [WR-C01] The social image preview accepts `/\host/...`, so the admin can load an image from an attacker's host + +_Lane: sm-seo-plugin_ **Status:** fixed **Commit:** 882d4ee @@ -405,7 +378,9 @@ With the old `lower(source)` fetch the test fails. **Applied fix:** `safeImageURL` (`/^\/(?![\/\\])/`) and `ValidURLRef` both refuse a path that starts with `/\`. `ValidURLRef` is also the canonical validator, so canonicals are stricter in the same way. **Tests:** a Go table test and a TS test. -### WR-21: The CSV export/import round trip changes cells that already start with an apostrophe followed by a formula character +### WR-21: [WR-C02] The CSV export/import round trip changes cells that already start with an apostrophe followed by a formula character + +_Lane: sm-seo-plugin_ **Status:** fixed **Commit:** ed2d919 @@ -419,7 +394,9 @@ With the old `lower(source)` fetch the test fails. This also resolves IN-08 (IN-B02), which is out of scope. -### WR-22: The 404 "Open/Resolved" filter and the redirect "Chains" filter disagree with the runtime matcher +### WR-22: [WR-C03] The 404 "Open/Resolved" filter and the redirect "Chains" filter disagree with the runtime matcher + +_Lane: sm-seo-plugin_ **Status:** fixed (requires human verification: SQL matcher logic). Pattern and regex rules remain a documented gap. **Commits:** 7677d8d, ad419e9 (gofmt follow-up) @@ -435,7 +412,9 @@ This also resolves IN-08 (IN-B02), which is out of scope. - each row's badge agrees with the filter; - `/c1 → /c2/` is a chain next to `/c2`. -### WR-23: The URL rule address is uncapped but sits in a btree UNIQUE index +### WR-23: [WR-C04] The URL rule address is uncapped but sits in a btree UNIQUE index, so long values return 500 instead of 422 + +_Lane: sm-seo-plugin_ **Status:** fixed **Commit:** 4ca26e9 @@ -443,7 +422,9 @@ This also resolves IN-08 (IN-B02), which is out of scope. **Applied fix:** `models.MaxRuleURL = 2048`, and `Rule.Problems` refuses longer addresses with `rules.url_invalid`. The check covers the admin form, the API, MCP, batches and imports. **Test:** a 3001-byte rule URL gets 422 on `url`. -### WR-24: The robots element's grammar drifts from the Go grammar, and an unparseable value is silently wiped on the next edit +### WR-24: [WR-C05] The robots element's grammar drifts from the Go grammar, and an unparseable value is silently wiped on the next edit + +_Lane: sm-seo-plugin_ **Status:** fixed **Commit:** 923d0f9 @@ -455,7 +436,9 @@ This also resolves IN-08 (IN-B02), which is out of scope. **Tests:** the Feb-30, Apr-31 and day-00 dates are in the TS malformed list. A leap day and `+5` are accepted. An element test covers keep, disable and Reset. A Go test checks that `+5` normalises to `max-snippet:5`. -### WR-25: The element tests never exercise the host protocol; the tester and the image element have no tests +### WR-25: [WR-C06] The element tests never exercise the host protocol; the tester and the image element have no tests + +_Lane: sm-seo-plugin_ **Status:** fixed **Commits:** 3ecbd1f (fake host, SERP host-protocol tests, `robotstester.test.ts`), 882d4ee (`image.test.ts` `safeImageURL`), 5c2ad6a (transfer host tests, image element tests, README test gate) @@ -465,11 +448,11 @@ This also resolves IN-08 (IN-B02), which is out of scope. - `golem15-seo-image` is tested for safe and unsafe previews, upload id matching (a foreign id is ignored), the error and the `imageFailed` fallback, Remove, and the disabled state. - The README test gate now includes `npm test`. -### Skipped Issues + None. -### Notes for the orchestrator + - **No schema edits to released migrations:** CR-03 adds the new migration `202610280001_add_golem15_seo_redirects_source_key`. v0.1.0's migrations are untouched. - **Framework half of CR-04:** the `WidgetField.vue` change belongs to the other fixer. The plugin now works with either wiring order. @@ -484,7 +467,30 @@ None. These are worth a changelog line when the plugin is tagged. - **README sections updated:** redirects precedence, address changes, the loop lock, the runtime cycle log, "Behind the frontend server", entry generation, CSV escaping, and the Testing gate. +--- _Fixed: 2026-10-27T14:45:18Z_ _Fixer: Claude (msd-code-fixer)_ _Iteration: 1_ + +## Skipped Issues + +### WR-03: [WR-A03] A failing extension `Load` (or plugin-type `Load`) after a successful save rolls back the whole save + +_Lane: summercms.go / sm-journal-plugin_ + +**Status:** skipped +**File:** `modules/cabana/crud.go:980`, `modules/cabana/form_extensions.go` (`hydrateFormExtensions`) +**Reason:** Both suggested fixes have problems that need a design decision: +- **Log and omit the extension's keys.** This risks data loss. The admin SPA's `editablePayload` always sends translatable fields (`localeRecord(undefined)`), so a response without them makes the next save post empty locale maps to the extension's `Save`. +- **Run hydration after the commit.** This stops the rollback, but the request still answers 500 after a committed write. Show still fails for the same `Load`, so editing stays blocked either way. + +A proper fix needs an "extension unavailable" state in the schema/record (for example, the failing extension's fields marked read-only or failed, so the SPA does not post them). That is beyond a v0.5.2 patch. + +**Original issue:** `hydrateFormExtensions` runs inside the save transaction, so one plugin's read-side failure turns a successful save into a 500 and rolls it back. `ShowRecord` likewise fails the whole record. + +--- + +_Fixed: 2026-10-27T15:55:00Z_ +_Fixer: Claude (msd-code-fixer)_ +_Iteration: 1_