docs(32): reconcile review disposition with the fix report
This commit is contained in:
@@ -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: <summary> (<ID>)`, 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_
|
||||
|
||||
Reference in New Issue
Block a user