diff --git a/.planning/phases/33-seo-output-audit-and-content/33-REVIEW-GAPFIX.md b/.planning/phases/33-seo-output-audit-and-content/33-REVIEW-GAPFIX.md new file mode 100644 index 00000000..e7a17dae --- /dev/null +++ b/.planning/phases/33-seo-output-audit-and-content/33-REVIEW-GAPFIX.md @@ -0,0 +1,197 @@ +--- +phase: 33-seo-output-audit-and-content +scope: gap-closure re-review (33-15, 33-16) +reviewed: 2026-11-04T23:30:00Z +depth: deep (targeted) +files_reviewed: 52 +files_reviewed_list: + - sm-sitemap-plugin (1048c01..HEAD): contract/contract.go, plugin.go, classes/rebuild.go, classes/lastmod.go, classes/listing.go, models/legacy.go, models/url.go, extensions_settings.go, updates/202610280002_update_golem15_sitemap_settings.go, updates/202611040001_add_golem15_sitemap_urls_indexnow_pending.go + - sm-seo-plugin (075cf17..HEAD): classes/crawl/job.go, classes/crawl/store.go, classes/crawl/actions.go, classes/crawl/crawler.go, classes/crawl/fetch.go, classes/crawl/render.go, classes/crawl/render_chrome.go, classes/crawl/compare.go, classes/crawl/checks.go, classes/crawl/fix.go, classes/linkgraph.go, classes/registry.go, classes/sitemap_filter.go, controllers/api/registry.go, controllers/api/crawls.go, controllers/crawls.go, models/crawl.go, models/noindex_path.go, jobs.go, mcp.go, updates/202611040001_create_golem15_seo_noindex_paths.go, updates/202611040002_add_golem15_seo_crawls_heartbeat.go + - sm-journal-plugin (311c28a..HEAD): classes/headings.go, classes/image_alt.go, classes/image_variants.go, classes/content_descriptors.go, classes/menu_item_types.go, console/fix_headings.go, controllers/api/maintenance.go, mcp.go + - vue-seo-layer (b3a0078..HEAD): lib/seo-module.ts, lib/sitemapPages.ts, lib/routes.ts, lib/options.ts, server/lib/routeReport.ts, server/lib/site.ts, server/middleware/seo-root.ts, server/plugins/seo-route-report.ts, server/seo-routes.d.ts, app/components/Seo/SitemapLink.vue, app/components/global/SeoHtmlSitemapPage.vue, app/utils/pageHref.ts, package.json + - vue-journal-layer (e8f307b..HEAD): app/journal-pages/PostPage.vue, app/utils/seo.ts, app/utils/routes.ts, lib/journal-module.ts, scripts/check-git-install.sh, package.json + - vue-menu-layer (a7602c9), vue-faq-layer (5e5950a): package.json, README.md +findings: + critical: 0 + warning: 4 + info: 0 + total: 4 +status: issues_found +--- + +# Phase 33 gap-closure re-review (33-15, 33-16) + +Paths are relative to `/media/nvme/dev/golem15/summercms.io/summercms/`. + +## Summary + +Most fixes do what their findings asked. Three are only partly done: + +- **L1 CR-02.** A markdown body that starts with `<` is still rewritten by the tokenizer. I confirmed this by running the code. +- **L1 WR-05.** Bodies stored as content_html only now get a 422 where they used to succeed (a regression), and the `<`-prefixed markdown subset still goes down the HTML path. +- **L4 CR-01.** The guard, the version fields and the install pins are done. The `link:` swap was deliberately left to 33-14 Task 4, so the layers still cannot be tagged from HEAD. + +I found no critical defects in the new code. There are four new warnings (below). + +Things I checked that hold up: +- **Advisory lock.** It does not deadlock. The pool is unbounded (lagoon sets no `MaxOpenConns`), the lock order is the same on every path (advisory lock, then the settings row), and the lock wait is bounded by the job context (conga's default 300 s). +- **Crawl heartbeat.** The heartbeat goroutine stops cleanly (closed channel, then wait), and it keeps writing during `finish` because `stop` is deferred after it. +- **River rescuer.** It honours the worker's 6 h timeout, so it does not rescue a crawl that is still running. +- **Cancel.** `Cancel` keeps the guard: the row stays `running` until the job ends it, and `ActiveCrawl`, `DeleteCrawls` and `Prune` all see it as active. +- **Migrations.** The new migrations only add a table or nullable columns and have rollbacks. `202610280002` keeps its ID, and no production database has run it (33-13 deployed nothing; the rehearsal databases are to be recreated, as the 33-15 SUMMARY says). +- **Gap-2 contract.** Both directions work. When `noindex` is absent (or `null`), the stored set stays unchanged. The layer keeps the body at or under 4,113,024 bytes and drops `noindex` entries first. The new plugin accepts twice that. +- **Concurrent route reports.** Two at once end up with the union of both sets (`ON CONFLICT DO NOTHING`), not with an error. +- **noindex input.** It is validated: + - same bearer token, compared in constant time; + - at most 2000 entries, checked before the transaction (422 and nothing stored); + - each entry passes `ValidPagePath` and the 2048-byte cap; + - patterns (`*`, `:param`) are refused; + - the body is capped with `MaxBytesReader`. +- **i18n hook.** Registering `pages:resolved` from `ready` puts it after i18n's `modules:done` hook, and the function strips its own routes again on dev regenerations. +- **sm-journal-plugin compatibility.** Exported signatures are unchanged. The behaviour changes (descriptor URL `""` for locales without content, the 422 for markdown images the tool cannot reach) are deliberate and in the v0.4.0 notes. + +## Verdicts + +| Finding | Verdict | Evidence | +|---|---|---| +| L1 CR-01 tall original | Resolved | `sm-journal-plugin/classes/image_variants.go:195-202` skips a width whose height is over 4096 and continues. `MaxImageWidth` (4096) equals `attach` `maxThumbEdge` (`summercms.go/modules/lagoon/attach/thumb.go:29`) only by coincidence. Other `ThumbKey` errors still abort the batch (`:203-206`); that was outside the plan | +| L1 CR-02 H1 rewrite of markdown | **Partially resolved** | `classes/headings.go:79-82` skips `content` only when `!LooksLikeHTML(content)`, and `LooksLikeHTML` only checks for a leading `<` (`classes/blog_editor.go:129-132`). The API always renders `content` as markdown (`controllers/api/posts_helpers.go:537,593`), so a markdown body that starts with an HTML block still has `

` rewritten inside its code fences. See new WR-01 | +| L1 WR-01 fix-headings CLI | Resolved | `console/fix_headings.go:51-55` uses `lookupDB` | +| L1 WR-03 tokenizer tail | Resolved | `classes/headings.go:26-36`, `classes/image_alt.go:31-40` write `html[consumed:]` on `ErrorToken`; `Raw()` slices are contiguous, so the offset is exact | +| L1 WR-05 alt on markdown | **Partially resolved** | `controllers/api/maintenance.go:172-188`: an inline `![..](src)` image or a raw img outside code is written into the markdown. But a body with empty `content` (a content_html-only translation) now always gets a 422, and markdown that starts with `<` goes down the HTML branch. See new WR-02 | +| L2 WR-01 IndexNow deletes/edits | Resolved | `classes/lastmod.go:135,158-162` set `indexnow_pending_at`; `classes/rebuild.go:516-524,539-546` add the row to Changed/Removed; `:631-637` clear only marks that are not newer than the value read | +| L2 WR-02 concurrent rebuilds | Resolved | `classes/rebuild.go:144-170`: `pg_advisory_xact_lock` is taken before the settings, type settings and `collect` | +| L2 WR-03 lastmod revert | Resolved | `classes/rebuild.go:621-626`: `lastmod` is written only for `ExtraTypeKey` rows | +| L2 WR-04 migration drops items | Resolved (as scoped in P33-66) | `models/legacy.go:202-278` and the migration at `updates/202610280002_…go:84-92`: leading slash added, spaces and unsafe bytes encoded, items parsed one by one, one warn log per dropped item. Absolute URLs with `&` are still dropped, but now logged | +| L2 WR-05 pinned headings | Resolved | `extensions_settings.go:128-135` loads stored titles only; `pickTitle` ignores empty strings (`classes/htmlgroups.go:193-201`) | +| L2 IN-01 nil event | Resolved | `classes/lastmod.go:114-116` | +| L3 CR-01 cancel/timeout loses results | Resolved (one gap, new WR-03) | `classes/crawl/job.go:222-228`: detached `FinishTimeout` context; `:342-356` maps a deadline to failed; `:365-368` falls back to default limits; `crawler.go:250-253` marks cut-short fetches "canceled"; `store.go:199-239` adds `RecoverStale`. The terminal write shares the store's deadline (WR-03) | +| L3 WR-01 cancel clears guard | Resolved | `job.go:541-556` sets `cancel_requested_at` and leaves the status `running`; `finish` writes `cancelled`; `controllers/crawls.go:84` hides the action while cancelling | +| L3 WR-03 any port | Resolved | `fetch.go:150-152` (`Allowed`), `render.go:184` (`allowRequest`), `compare.go:29-33` uses `Fetcher.Key` | +| L3 WR-04 two path forms | Resolved | `classes/linkgraph.go:261-287` (`PathKey`, `EscapeKeyPath`), `fetch.go:176-196`, `sitePath` handles both inputs. Residual edge case: `%2F` decodes to `/` | +| L3 WR-06 tab-open timeout | Resolved | `render_chrome.go:126-160`: one deadline (`TabOpenTimeout+RenderTimeout`), a 5 s open timer, the tab closed on timeout; the result channel is buffered, so the goroutine does not leak | +| G1 CMS side | Resolved | `classes/registry.go:821-925` (`ReportNoindex`, `Rebuilder` called after commit), `classes/sitemap_filter.go:174-176`, `controllers/api/registry.go:19-29,78-97`, `crawl/checks.go:338-354` (sitemap tool for extras) | +| G3 descriptors per locale | Resolved | `classes/content_descriptors.go:177-182,497-500,664`, `classes/menu_item_types.go:1046,1108-1228`; the sitemap skips `URL ""` (`sm-sitemap-plugin/classes/rebuild.go:250-252`), so older rows for those locales drop out as holes | +| L4 CR-01 link: dependency | **Partially resolved (deferred by plan)** | `check-git-install.sh --check-links` and the tag-mode guard are in place, versions are 0.2.0/0.8.0/0.5.0/0.5.0 and the README pins match. All three content layers still have `"vue-seo-layer": "link:../vue-seo-layer"` (checked with `package.json` grep). Do not tag before the 33-14 Task 4 swap | +| L4 WR-01 page-1 self-redirect | Resolved | `app/utils/pageHref.ts:38-41` (`target === fullPath` gives `null`), `SeoHtmlSitemapPage.vue:71-83`. Note: the CMS segment wins over the segment the route was registered with; if they differ, the page is served without a redirect (no loop) | +| L4 WR-02 og:type article | Resolved | `vue-journal-layer/app/journal-pages/PostPage.vue:56-68`, `app/utils/seo.ts:78-83` | +| L4 WR-03 path-form segments | Resolved | `vue-journal-layer/lib/journal-module.ts:58-64`, `app/utils/routes.ts:101-107` read `runtimeConfig.public.seo.pagination`, which the SEO module's `setup` writes (`vue-seo-layer/lib/seo-module.ts:206-221`) | +| L4 WR-04 catch-all shadowing | Resolved | `lib/sitemapPages.ts:362-373`; the `overCatchAll` opt-in is documented in all four READMEs | +| L4 WR-05 i18n copies | Resolved | `lib/seo-module.ts:259-269` (`ready`, then `pages:resolved`), `lib/sitemapPages.ts:348-381` (``), `SitemapLink.vue:39-46` checks the route | +| L4 WR-08 root files while unknown | Resolved for the reported case (new WR-04) | `server/lib/site.ts:96-99`: FIXED names are proxied in state `unknown`; a failing fetch gives 503. Side effect in WR-04 | +| G1 layer side | Resolved | `lib/routes.ts:161-171`, `lib/seo-module.ts:283-287`, `server/lib/routeReport.ts:428-478` (bounded body, `noindex` dropped first, `[]` always sent) | + +Counts: 26 verdict rows. 19 resolved (L3 CR-01 and L4 WR-08 each with a new warning attached), 3 partially resolved (L1 CR-02, L1 WR-05, L4 CR-01), 0 not resolved. + +## Narrative Findings (AI reviewer) + +## Warnings + +### WR-01: Markdown that starts with `<` still gets `

` rewritten inside code samples (CR-02 residual) + +**File:** `sm-journal-plugin/classes/headings.go:79-87` (with `classes/blog_editor.go:129-132`, `controllers/api/posts_helpers.go:537,593`, and the call sites in `posts_helpers.go:865`, `console/import_posts.go`, `classes/heading_fix.go`) + +**Issue:** `ConvertContentH1` treats `content` as HTML whenever its first non-space byte is `<`. The API, however, always renders `content` as markdown (`FormatHTML`), so a markdown post that opens with an HTML block (`

…

`, ``, ``, an autolink ``) is still run through the `x/net/html` tokenizer. Every `

` token in its fenced or inline code samples becomes `

`, silently and with the default-on setting, on every API or MCP save, on import and on `journal:fix-headings --apply`. It is worse than before: the count now comes from `content`, so the corruption is counted as a conversion. This is the same defect class as CR-02, with a narrower trigger. + +**Evidence:** I ran the shipped `headings.go` (copied into the scratchpad, `GOWORK=off go run`): +``` +in : "

Intro

\n\nExample:\n\n```html\n

Hello

\n```\n" +out: "

Intro

\n\nExample:\n\n```html\n

Hello

\n```\n" n=1, content_html unchanged +``` + +**Fix:** Decide by the render path, not by the first byte. Where the body is rendered as markdown (the API, import, fix-headings of an API post), rewrite only outside goldmark's code ranges. The helper for that already exists (`markdownCodeRanges` in `classes/image_alt.go`): +```go +func ConvertContentH1(content, contentHTML string) (string, string, int) { + contentHTML, nHTML := ConvertH1(contentHTML) + var nContent int + content, nContent = convertH1OutsideCode(content) // split on markdownCodeRanges(content), ConvertH1 on the parts + if nHTML == 0 { nHTML = nContent } + return content, contentHTML, nHTML +} +``` +For a real HTML body, `markdownCodeRanges` finds no fences, so the result is the same as today. Add a regression case with a leading `

` to `headings_test.go`. + +### WR-02: `images/alt` now refuses images in bodies stored as content_html only (regression), and misses markdown that starts with `<` + +**File:** `sm-journal-plugin/controllers/api/maintenance.go:172-188` (with `loadImageAltText` at `:263-282`) + +**Issue:** The markdown branch runs whenever `!LooksLikeHTML(text.content)`, and that includes `content == ""`. A translation that carries only `content_html` is a supported shape: +- `classes/format_html.go:101-102`; +- `classes/blog_editor.go:292`; +- `controllers/api/posts_helpers.go:802-804`. + +For such a body, `SetMarkdownImageAlt("")` and `SetImageAltOutsideCode("")` both return 0, so every change for it is refused with 422 `changes.N.src` ("The image is not an inline image of the post's markdown"). The batch is all or nothing, so one such image fails the whole MCP `set_image_alt` call. Before this change it succeeded and stuck, because no markdown re-derives that `content_html`. + +Markdown that starts with `<` takes the HTML branch instead. There, `SetImageAlt` finds no raw `` for a `![..](src)` image, so `m == 0`, yet the change is answered `changed: true` and is reverted on the next save (the original WR-05). A raw `` shown inside a code fence is also rewritten. + +**Fix:** Treat an empty `content` as "content_html is the body", and route markdown by the same rule as WR-01: +```go +switch { +case strings.TrimSpace(text.content) == "": + // content_html-only body: nothing to keep in sync +case classes.LooksLikeHTML(text.content) && !hasMarkdownImage(text.content, c.src): + if content, m, _ := classes.SetImageAlt(text.content, c.src, c.alt); m > 0 { text.content = content } +default: + content, m, _ := classes.SetMarkdownImageAlt(text.content, c.src, c.alt) + content, r, _ := classes.SetImageAltOutsideCode(content, c.src, c.alt) + if m+r == 0 { /* 422 */ } + text.content = content +} +``` +Add a test with a translation that has only `content_html`. + +### WR-03: The crawl's terminal status write shares the store's 10-minute deadline + +**File:** `sm-seo-plugin/classes/crawl/job.go:225-228, 383-397` + +**Issue:** `finish` runs the store transaction and then the terminal `UPDATE … status/finished_at` on the same `fctx` (`FinishTimeout` = 10 min). A large crawl can run out that budget inside `SavePages`/`SaveIssues`: up to 50 000 pages and 1000 links per page in one transaction (L3 WR-02 left it unbounded). The store then fails with `context deadline exceeded`, and the terminal update fails on the same expired context, so `finish` returns at `:397`. Three things follow: +- the row stays `running`, and the real error is lost; +- the conga record is not ended by `finish`; +- the guard blocks new crawls until `RecoverStale` runs another 10 minutes later (heartbeat plus `StaleAfter`) and records a misleading "no heartbeat since …" error. + +**Fix:** Give the final state its own fresh, short bound, separate from the store's: +```go +if err := lagoon.Transaction(ctx, db, store); err != nil { status, errText = models.CrawlFailed, err.Error() } +wctx, wcancel := context.WithTimeout(context.WithoutCancel(ctx), 30*time.Second) +defer wcancel() +wdb := db.WithContext(wctx) +if err := wdb.Model(&models.Crawl{}).Where("id = ?", row.ID).Updates(updates).Error; err != nil { return err } +// FailJob/StopJob/CompleteJob and Prune on wctx too +``` +Add a test with a store that blocks past a small `FinishTimeout`, which checks for `failed` with the store error. + +### WR-04: The root-file 503 also catches sites whose backend has no SEO plugin, after any outage longer than the cache TTL + +**File:** `vue-seo-layer/server/lib/site.ts:47-63, 96-99` + +**Issue:** `lastOk` remembers only an `ok` answer. An `absent` answer (a backend without SEO v0.2.0, for which the layer promises v0.1.1 behaviour) is cached for 5 minutes only. Once it expires, any moment the backend is unreachable or answers something other than 200 or 404 is `unknown` with `lastOk == null`. The new branch then proxies `/robots.txt`, `/sitemap.xml` and `/llms.txt` to a backend that never served them: +- if the fetch fails or answers 5xx, the layer serves 503; +- if the backend answers 404, the layer passes that 404 on. + +Before 33-16 these requests reached the app. That covers robots.txt or sitemaps produced by a Nuxt server route or module (`@nuxtjs/robots`, `@nuxtjs/sitemap`), or the app's own 404. Files in Nuxt's `public/` are still served first by Nitro's static handler. + +A 5xx robots.txt is read by Google as a site-wide disallow for up to about 12 hours. So on an SEO-less site, every short backend outage now stops crawling of the whole site. The same happens to a backend whose `/site` answers 200 with HTML (a catch-all) or 401/403 from an auth gateway: it is `unknown` for good, and its root files get hijacked. + +**Fix:** Remember the last definite answer, not only `ok`, and narrow `unknown` to "never answered": +```ts +let lastKnown: SeoSiteState | null = null +... +if (got.state !== 'unknown') lastKnown = got +if (got.state === 'unknown' && lastKnown) return lastKnown +``` +`rootFileTargets` then returns null for a remembered `absent`. Add a `rootFiles.test.ts` case: absent, then unknown, should still pass through. + +## Notes below the warning bar (not counted) + +- **Shutdown during a crawl.** A graceful worker shutdown (`conga.Worker.Stop` falls back to `StopAndCancel` with 5 s, `summercms.go/modules/conga/worker.go:219-226`) cancels the job context. `finalStatus` maps that to `cancelled` with no actor (`job.go:345-346`), so a deploy during a nightly crawl can leave a partial "cancelled" crawl as the current audit. The `Canceled → cancelled` mapping also covers `Cancel` calling `CancelJob` before its own transaction commits (`job.go:557-562`). Moving `CancelJob` to `AfterCommit` would allow a separate "interrupted" status. +- **DeleteCrawls stale rows.** `DeleteCrawls` calls `RecoverStale` but not `endStaleJobs` (`actions.go:81-83`), so the conga records of those crawls are never ended. +- **Open transaction during collect.** `Rebuild` keeps its transaction open (idle) for all of `collect` (`rebuild.go:144-170`). A database with `idle_in_transaction_session_timeout` set would kill long rebuilds. +- **Parameter limit.** `postContentLocales` passes every published post id of a page of tags or authors as one `IN ?` list (`menu_item_types.go:1150-1162`, `786-805`). Past about 65k posts, Postgres's parameter limit fails the descriptor call and with it the rebuild. +- **Repeat IndexNow pings.** `RecordSaved` marks rows that are already excluded (an unpublished record's old rows), and `apply` then sends them as removed again on every edit (`rebuild.go:539-545`). +- **Version pin.** sm-seo-plugin's go.mod still requires sm-sitemap-plugin v0.1.2, which has no `contract` package (`GOWORK=off go build` fails). The v0.2.0 pin is a 33-14 step, as the SUMMARY records. + +--- + +_Reviewed: 2026-11-04_ +_Reviewer: Claude (msd-code-reviewer)_ +_Depth: deep (targeted re-review of the 33-15 and 33-16 ranges)_ diff --git a/.planning/quick/261104-xae-phase-33-gap-fix-follow-ups/261104-xae-PLAN.md b/.planning/quick/261104-xae-phase-33-gap-fix-follow-ups/261104-xae-PLAN.md new file mode 100644 index 00000000..654cd584 --- /dev/null +++ b/.planning/quick/261104-xae-phase-33-gap-fix-follow-ups/261104-xae-PLAN.md @@ -0,0 +1,361 @@ +--- +phase: quick-261104-xae +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - /media/nvme/dev/golem15/summercms.io/summercms/sm-journal-plugin/classes/headings.go + - /media/nvme/dev/golem15/summercms.io/summercms/sm-journal-plugin/classes/headings_test.go + - /media/nvme/dev/golem15/summercms.io/summercms/sm-journal-plugin/headings_test.go + - /media/nvme/dev/golem15/summercms.io/summercms/sm-journal-plugin/controllers/api/maintenance.go + - /media/nvme/dev/golem15/summercms.io/summercms/sm-journal-plugin/README.md + - /media/nvme/dev/golem15/summercms.io/summercms/sm-seo-plugin/classes/crawl/job.go + - /media/nvme/dev/golem15/summercms.io/summercms/sm-seo-plugin/crawl_lifecycle_test.go + - /media/nvme/dev/golem15/summercms.io/summercms/sm-seo-plugin/README.md + - /media/nvme/dev/golem15/summercms.io/summercms/vue-seo-layer/server/lib/site.ts + - /media/nvme/dev/golem15/summercms.io/summercms/vue-seo-layer/server/middleware/seo-root.ts + - /media/nvme/dev/golem15/summercms.io/summercms/vue-seo-layer/tests/rootFiles.test.ts + - /media/nvme/dev/golem15/summercms.io/summercms/vue-seo-layer/README.md + - /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/.planning/phases/33-seo-output-audit-and-content/33-14-notes-sm-journal-plugin-v0.4.0.md + - /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/.planning/phases/33-seo-output-audit-and-content/33-14-notes-sm-seo-plugin-v0.2.0.md + - /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/.planning/phases/33-seo-output-audit-and-content/33-14-notes-vue-seo-layer-v0.2.0.md +autonomous: true +requirements: + - quick-261104-xae + +estimate: + tokens: 100000 + raw_tokens: 100000 + tasks: 4 + confidence: low + +must_haves: + truths: + - "ConvertContentH1 rewrites

in content only outside goldmark's code ranges (fenced and indented code blocks, code spans), whatever the body's first byte, on every caller (API and MCP saves, journal:import-posts, journal:fix-headings, translations): the reviewer's probe \"

Intro

\\n\\n```html\\n

Hello

\\n```\" comes back byte-identical with n=0, a raw

outside code becomes

, and an HTML body (no code ranges) is converted as before" + - "POST /_journal/api/v1/images/alt on a locale whose content is blank (a translation that holds only content_html) answers 200 with changed true and writes the alt into content_html alone, so a batch holding such a change is no longer refused with 422" + - "images/alt reads every non-blank content as markdown: a markdown body that starts with < gets its inline image label set, its code samples untouched, and keeps the alt after the next save of the same content; an image it does not hold as an inline image or a raw outside code is refused with 422 changes.N.src (never answered changed true and reverted later)" + - "A crawl whose result store runs past its bound ends at once as failed, with finished_at and the error \"storing the crawl results took longer than : \"; the final status write and the end of the conga record run on their own 30-second bound, the job returns nil, and the next Start is accepted" + - "vue-seo-layer's siteAnswer remembers the last definite site answer (ok or absent): after the cache expires while the backend is unreachable or answers neither 200-object nor 404, the remembered answer is used, so on a site without the SEO plugin /robots.txt, /sitemap.xml and /llms.txt keep passing through to the app; a backend that has not answered since start (or since the last purge) is still unknown and those three files still get 503" + - "No exported Go signature of sm-journal-plugin changes; sm-seo-plugin only adds crawl.WithFinishTimeout; each changed behaviour is in its README and in the three 33-14 release-note drafts" + - "Every new test failed on the code before its fix, for the reason the finding describes" + artifacts: + - path: /media/nvme/dev/golem15/summercms.io/summercms/sm-journal-plugin/classes/headings.go + provides: "unexported convertH1OutsideCode on markdownCodeRanges; ConvertContentH1 without the first-byte gate" + - path: /media/nvme/dev/golem15/summercms.io/summercms/sm-journal-plugin/controllers/api/maintenance.go + provides: "images/alt body rule: blank content means content_html is the body, any other content is markdown" + - path: /media/nvme/dev/golem15/summercms.io/summercms/sm-seo-plugin/classes/crawl/job.go + provides: "finish with a store bound, a 30 s final-write bound and a prune bound; WithFinishTimeout test seam" + - path: /media/nvme/dev/golem15/summercms.io/summercms/vue-seo-layer/server/lib/site.ts + provides: "lastKnown memory of the last ok or absent answer; the cache clock read at call time" + - path: /media/nvme/dev/golem15/summercms.io/summercms/sm-seo-plugin/crawl_lifecycle_test.go + provides: "TestCrawlStoreTimeout (Docker)" + - path: /media/nvme/dev/golem15/summercms.io/summercms/vue-seo-layer/tests/rootFiles.test.ts + provides: "siteAnswer memory cases with a stubbed $fetch and a mocked Date.now" + key_links: + - from: "controllers/api/posts_helpers.go:865,875, console/import_posts.go:229,367, classes/heading_fix.go:155,180" + to: "classes.ConvertContentH1 / ConvertTranslationH1 -> convertH1OutsideCode -> markdownCodeRanges (classes/image_alt.go)" + via: "every content H1 conversion except the admin rich editor's decode (ConvertH1 on HTML) and category host content" + - from: "controllers/api/maintenance.go SetImageAlt" + to: "classes.SetMarkdownImageAlt + classes.SetImageAltOutsideCode" + via: "the switch on strings.TrimSpace(text.content)" + - from: "crawl.Run" + to: "finish(detached ctx, ..., finishTimeoutFrom(ctx))" + via: "store on its own bound, then terminal UPDATE + conga FailJob/StopJob/CompleteJob on finalWriteTimeout, then Prune" + - from: "server/middleware/seo-root.ts" + to: "rootFileTargets(path, siteAnswer(...))" + via: "a remembered absent returns null (pass-through); unknown without memory proxies the FIXED names (503 on failure)" +--- + + +Close the four new warnings of the Phase 33 gap-closure re-review (`.planning/phases/33-seo-output-audit-and-content/33-REVIEW-GAPFIX.md`, WR-01 to WR-04), one task per fix, in three sibling repositories, before the 33-14 release: + +1. sm-journal-plugin WR-01: `ConvertContentH1` rewrites `

` inside code samples of a markdown body that starts with an HTML block. +2. sm-journal-plugin WR-02: `images/alt` refuses images of content_html-only translations (a 422 regression from 33-15), and sends markdown starting with `<` down the HTML branch (answered `changed: true`, then reverted). +3. sm-seo-plugin WR-03: the crawl's final status write shares the store's `FinishTimeout` deadline, so a store that times out leaves the crawl `running` and loses the error. +4. vue-seo-layer WR-04: only an `ok` site answer is remembered, so after any outage longer than the cache TTL an SEO-less site's robots.txt, sitemap.xml and llms.txt answer 503 instead of reaching the app. + +Purpose: the drafts for sm-journal-plugin v0.4.0, sm-seo-plugin v0.2.0 and vue-seo-layer v0.2.0 describe behaviour these defects contradict; 33-14 must not tag them in this state. + +Output: one code commit per task on each repository's default branch (sm-journal-plugin `main` x2, sm-seo-plugin `main`, vue-seo-layer `main`), one planning-docs commit in summercms.go (`master`) with the three release-note drafts, and the full Phase 33 gate (plugins, layers, docker) green. No push, no tag. + + + +@~/.claude/msd-core/workflows/execute-plan.md +@~/.claude/msd-core/templates/summary.md + + + +@/media/nvme/dev/golem15/summercms.io/summercms/summercms.go/CLAUDE.md +@/media/nvme/dev/golem15/summercms.io/summercms/summercms.go/.planning/phases/33-seo-output-audit-and-content/33-REVIEW-GAPFIX.md + +Repositories (siblings under /media/nvme/dev/golem15/summercms.io/summercms/), with BASE recorded at planning time; record the actual BASE in the SUMMARY before Task 1 and stop if a repo is not on its default branch: +- sm-journal-plugin: branch `main`, BASE 0a62090; untracked `go.work.sum` belongs to nobody in this task, never stage it. +- sm-seo-plugin: branch `main`, BASE 2a288a9, clean. +- vue-seo-layer: branch `main`, BASE 973142a, clean. +- summercms.go: branch `master`, BASE 5ce50be. Other sessions have uncommitted edits (STATE.md, state.json, G15OFFICE.md, .msd/, phase 19.x/20/32.1 files, 33-REVIEW-GAPFIX.md): leave them alone, stage only the three release-note drafts by path. + +Rules for every task (user and project rules): +- Commits: no `Co-Authored-By` and no `Claude-Session` trailer (the user's rule overrides any attribution reminder). One logical change per commit; stage files explicitly by path, never `git add -A`; run `git status --short` before staging. No push, no tag. +- sm-journal-plugin is a core plugin: additive only. No exported function, method, type or constant changes its signature or disappears; new helpers stay unexported. +- Commit shas used by the acceptance checks: T1 and T2 (sm-journal-plugin, Tasks 1 and 2), T3 (sm-seo-plugin), T4 (vue-seo-layer), TN (summercms.go notes commit); record each in the SUMMARY. If 0a62090, 2a288a9 or 973142a is not the BASE recorded before Task 1, use the recorded BASE in their place. +- Every new test is run against the code before its fix and must fail there for the reason the finding names; record the observed failure line in the SUMMARY (the 33-15 practice: "tests that pin a fix are checked to fail on the old code"). +- Docker tests use testcontainers. If the Docker socket is denied although the user is in the docker group, run the same command through `sudo -u golem` (CLAUDE.md) and say so in the SUMMARY. + +Facts gathered during planning (do not re-derive): + +Task 1 (sm-journal-plugin, WR-01): +- classes/headings.go (112 lines): `ConvertH1(html) (string, int)` lines 18-58 (fast path: no ` 0, else content's. `ConvertTranslationH1(attrs)` 95-112 calls ConvertContentH1 in place. +- classes/image_alt.go: `markdownCodeRanges(md) [][2]int` line 335 returns the sorted, merged byte ranges of fenced code block lines and info string, indented code blocks and code spans (backticks included), offsets into the untrimmed md; HTML blocks are NOT ranges there (html=false), so an HTML body has no ranges. `SetImageAltOutsideCode` lines 162-190 is the pattern to copy: walk the ranges, apply the rewrite to each gap, copy each range verbatim. +- `LooksLikeHTML` (classes/blog_editor.go:129) stays exported and in use elsewhere (blog editor load); only headings.go stops calling it. +- Callers of ConvertContentH1/ConvertTranslationH1: controllers/api/posts_helpers.go:865,875 (API and MCP saves, after FormatHTML), console/import_posts.go:229,367 (import, after FormatHTML), classes/heading_fix.go:155,180 (journal:fix-headings). classes/blog_editor.go:181 (admin rich editor decode, HTML) and heading_fix.go:128 (category host content, HTML) call ConvertH1 directly and are not touched. +- At v0.3.1 (the last tag) ConvertContentH1 ran ConvertH1 on content unconditionally (code samples included); 33-15 (untagged) stopped rewriting content whenever its first byte is not `<`. The new rule (convert outside code, no first-byte gate) keeps v0.3.1's conversion of real headings and drops only the code-sample corruption. +- classes/headings_test.go: TestConvertContentH1 lines 48-82, TestConvertTranslationH1 88-107. Line 79 pins `ConvertContentH1("plain

x

text", "")` as unchanged with n=0; under the new rule that raw `

` is outside code, so the pin flips to `"plain

x

text"`, n=1 (deliberate; say so in the commit body and SUMMARY). +- headings_test.go (root, package journal_test, Docker): TestHeadingConversion line 192; its 33-15 subtest "markdown with a fenced h1 sample keeps its source" (around lines 295-322) posts markdown through the API with `posts` and `token` and imports it with `console.ImportPosts(env.app)`; copy its shape. Helpers: apiPostID, storedPost, setH1ToH2, cmdInput, bonfire.NewOutput. +- README.md: line 181 (the "On." bullet, the parenthetical after "markdown content"), line 183 (the "Stored content." bullet, parenthetical "default and translated; of a markdown post only `content_html`"), line 495 (API row for ConvertContentH1/ConvertTranslationH1). + +Task 2 (sm-journal-plugin, WR-02): +- controllers/api/maintenance.go: handler doc comment lines 96-113; the branch at lines 172-188 (`if classes.LooksLikeHTML(text.content) { SetImageAlt on content, accepted with m == 0 } else { SetMarkdownImageAlt, then SetImageAltOutsideCode; m+r == 0 -> 422 changes.N.src errMarkdownImage }`); `loadImageAltText` 263-283 reads content and content_html per locale (translations through translate.TranslatedExact; an absent translated content is ""); `writeImageAltTexts` writes only attributes that changed. +- A translation that carries only content_html is a supported API shape: `validateTranslations` (posts_helpers.go:665) does not require content, and `FormatTranslation` (classes/format_html.go:304-320) sanitizes a supplied content_html when content is blank. +- The admin rich editor stores HTML in content and the same HTML in content_html (controllers/posts.go preparePostWrite). goldmark reads such HTML as HTML blocks, which markdownCodeRanges does not list, so `SetImageAltOutsideCode` finds its `` as a raw img outside code: the markdown branch covers HTML bodies too. TestSetImageAlt (headings_test.go:582) already posts an HTML-shaped default body `

old

` and a pl translation `

` and asserts the alt lands in both content and content_html; it must keep passing unchanged and is the proof for HTML bodies. +- Residual edge accepted (same as Task 1): an admin HTML body in which a blank line is followed by a line indented 4 or more spaces holding the `` is an indented code block to goldmark, so that one image would get 422. Note it in the SUMMARY; do not special-case it. +- Test helpers (root package): newPostsEnv, env.admin, env.expect, apiPostID, storedPost, translatedAttrs (posts_admin_smoke_test.go:304), listenRecords; the endpoint path is journalAPIPrefix + "/images/alt"; editor permissions `controllers.PermissionAccessPosts, controllers.PermissionAccessOtherPosts`. +- README.md line 187 (the "Image alt." bullet). The MCP tool description in mcp.go:232 stays as it is (still accurate; tests may pin it). + +Task 3 (sm-seo-plugin, WR-03): +- classes/crawl/job.go: const block lines 46-55 (JobTimeout 6h, FinishTimeout 10m, HeartbeatEvery 1m, StaleAfter 10m, exported); `heartbeatWrite = 10 * time.Second` line 58 (unexported, the style for the new bound); Run lines 200-228 builds `fctx := context.WithTimeout(context.WithoutCancel(ctx), FinishTimeout)` and calls `finish(fctx, svc, gdb.WithContext(fctx), row, res, runErr, ctx.Err())`; `finish` lines 361-420 reads cancelRequested and LoadLimits on db, runs `lagoon.Transaction(ctx, db, store)` (SavePages, Keyphrases, Check with Lookup, svc.Issues, SaveIssues), then the terminal `db.Model(&models.Crawl{}).Where("id = ?", row.ID).Updates(updates)` (returns its error), then conga FailJob/StopJob/CompleteJob on ctx, the failed-crawl warn log, then `Prune(db, limits.Keep)` (warn on error). `errors` is already imported (finalStatus). +- classes/crawl/render.go lines 38-55: `rendererKey`, `WithRenderer(ctx, r)` and `rendererFrom(ctx)` are the precedent for a code-only test seam carried by the job context ("settable only from Go code: no config key, setting, request or MCP call reaches it"). jobs.go:28 passes the job context straight to crawl.Run, so a value set by a test on the context given to `job.Work` reaches Run. +- classes/crawl/actions.go:97-103 Prune is one DELETE on golem15_seo_crawls beyond Keep (cascading to pages, links and issues of old crawls); it can be large, so it keeps a store-sized bound of its own after the final state is written (the reviewer suggested the 30 s bound; a large cascade would then never finish). +- classes/crawl/store.go:19 SavePages builds the rows in memory and then inserts the crawl's pages into golem15_seo_crawl_pages before its links; its first write is the one the test blocks. +- crawl_lifecycle_test.go (root, package seo_test, Docker): helpers newBlockingSite, lifecycleSite, env.crawlRow, env.runCrawlAsync(t, ctx, id), waitJob (30 s), env.storedPages; TestCrawlTimeout lines 225-255 is the model. crawl_site_test.go: bootCrawlEnv(t) line 310, env.startCrawl(t, token) 333, newFakeSite 48, site.sitemap 97, site.plant 176 with doc{path, title}; env.admin(t, true) gives a token; env.db is the *gorm.DB on the test database. lagoon sets no MaxOpenConns, so a test transaction holding a lock does not starve the job. +- README.md line 250 (the "Limits." paragraph: "bounded by `crawl.FinishTimeout` (10 min)") and the API reference rows 341-346 (row 345 lists `crawl.WithRenderer`). + +Task 4 (vue-seo-layer, WR-04): +- server/lib/site.ts (110 lines): header comment 1-17 (the three states); `cache = createLru({ max: 1, ttlMs: SITE_CACHE_TTL_MS })` line 27 (createLru's default clock `now = Date.now` is captured when the cache is created, so a test cannot move it); `let lastOk` line 28; `clearSiteCache()` 31-34 clears the cache and lastOk; `siteState(status, body)` 41-45 (200 object ok, 404 absent, anything else unknown); `siteAnswer(cfg, headers)` 48-64 fetches with `$fetch.raw('/site', ...)` (Nitro global, resolved at call time, so a test can stub `globalThis.$fetch`), caches only ok/absent, sets lastOk on ok and returns lastOk for unknown; `FIXED` line 72; `rootFileTargets` 95-110 with its doc comment 80-94 (absent returns null; FIXED names are targeted in any other state; everything else needs ok). +- server/lib/lru.ts: `createLru({ max, ttlMs, now })`, `now` is the documented test clock option; `clear()` bumps a generation so no request after a purge sees an answer fetched before it. +- Consumers of siteAnswer: server/middleware/seo-root.ts (rootFileTargets; comment lines 10-19 describes the states), seo-verification.ts and seo-drift.ts and api/__seo__/site.get.ts (act only on ok; absent and unknown behave the same there), api/__seo__/sitemap.get.ts (unknown 503, absent 404: with a remembered absent the HTML sitemap API answers 404 during an outage, as it does while the backend is up; acceptable, note it in the SUMMARY). +- Purge semantics (planner's choice): `clearSiteCache` keeps clearing the memory, as it cleared lastOk, so a purge still drops every answer fetched before it (lru.ts contract); the backend that sent the purge is up, and the next request learns its state again. "Never answered" therefore means since the server started or since the last purge. +- A backend whose /site always answers 200 with HTML or 401/403 never gives a definite answer and stays unknown, so its fixed root files are still proxied (reviewer's last WR-04 paragraph). Out of scope (the brief keeps 503 for a site that has never answered); list it as a residual in the SUMMARY with the operator workaround (`seo.rootFiles: false` or `seo.cms: false`). +- tests/rootFiles.test.ts (86 lines) imports rootFileTargets, siteState and the SeoSiteState type from '../server/lib/site.ts' and seoBackendBase, seoServerConfig from '../server/lib/config.ts'; constants B (backend base) and A (api base). `seoServerConfig({})` yields apiBase A and timeoutMs 1500. `npm test` runs `node --test --test-reporter=tap tests/*.test.ts` (Node 24 here, type stripping; one process per file). +- README.md line 32 (the "Root files." bullet: "while the backend is down the last one seen is used. While the backend has not answered since the server started, ..."). + +Release notes (Task 4, summercms.go): .planning/phases/33-seo-output-audit-and-content/33-14-notes-sm-journal-plugin-v0.4.0.md lines 20-21 (Changed: the H1 bullet and the image-alt bullet), 33-14-notes-sm-seo-plugin-v0.2.0.md line 16 (Added: crawl cancel and time limits), 33-14-notes-vue-seo-layer-v0.2.0.md line 25 (Fixed: the root-file 503s). User-facing markdown, no planning IDs, no H1. + +Gate: `scripts/check-phase33.sh` with several stages prints `PASS plugins`, `PASS layers`, `PASS docker` (or `FAIL ` and exit 1). --plugins runs vet and short tests of the three Go plugins plus JOURNAL_UNIT_TESTS (includes ./classes:TestConvertContentH1) and the plugins' vitest; --layers runs the four layers' node --test, playground builds and smoke runs (vue-seo-layer smoke:ssr includes expect-seo-down.txt, the never-answered 503 case); --docker runs each Go plugin's full `go test ./...` with Postgres. + + + + + + Task 1: H1 conversion rewrites markdown content only outside code (sm-journal-plugin, WR-01) + /media/nvme/dev/golem15/summercms.io/summercms/sm-journal-plugin/classes/headings.go, /media/nvme/dev/golem15/summercms.io/summercms/sm-journal-plugin/classes/headings_test.go, /media/nvme/dev/golem15/summercms.io/summercms/sm-journal-plugin/headings_test.go, /media/nvme/dev/golem15/summercms.io/summercms/sm-journal-plugin/README.md + sm-journal-plugin is on branch main at 0a62090 (or a descendant with no change to classes/headings.go), and `docker info` succeeds (else use sudo -u golem for the Docker tests). + classes/headings.go (whole), classes/image_alt.go lines 158-190 and 320-418, classes/headings_test.go (whole), headings_test.go lines 186-330 (TestHeadingConversion and its 33-15 markdown subtest), README.md lines 178-190 and 495 + + - Probe (the reviewer's input): `ConvertContentH1(probe, FormatHTML(probe))` and `ConvertContentH1(probe, "")`, with probe = "

Intro

\n\n```html\n

Hello

\n```", both return content identical to probe, content_html unchanged and n=0. On the current code they return "

Hello

" inside the fence and n=1 (RED). + - A raw heading outside code in a body that starts with an HTML block is still converted: "

Intro

\n\n

Real

\n\n```html\n

Hello

\n```" with "" gives the same text with "

Real

" and the fenced "

Hello

" kept, n=1. + - A code span is kept: "

x

\n\nUse `

` here" with "" is unchanged, n=0. + - ConvertTranslationH1 on {content: probe, content_html: FormatHTML(probe)} returns 0 and leaves content identical to probe. + - Existing rows keep their results: "# Title\n\ntext" with "

Title

text

"; "

a

b

" with "" (both converted, n=2); "

x

" pair (n=0); the 33-15 fenced/inline markdown sample; every TestConvertH1 and TestConvertTranslationH1 row. + - Deliberate flip of the pin at classes/headings_test.go:79: "plain

x

text" with "" now gives "plain

x

text", n=1 (a raw h1 outside code is a heading in the rendered page; v0.3.1 converted it too). Update the comment above that assertion accordingly. + - Docker, TestHeadingConversion, new subtest "markdown that starts with an HTML block keeps its h1 samples": with the H1 setting on, POST md = "

Intro

\n\n# Title\n\n```html\n

Hello

\n```" through the API: the stored content equals md byte for byte, content_html has no "Hello

" (RED). + + +Record BASE (`git -C ../sm-journal-plugin rev-parse HEAD`) and run `git status --short` there. + +Tests first (RED): in classes/headings_test.go add the behavior rows to TestConvertContentH1 and TestConvertTranslationH1 (or a new TestConvertContentH1HTMLLead next to them; keep TestConvertContentH1 as the name the gate requires), flip the line-79 pin as described, and add the Docker subtest to TestHeadingConversion in headings_test.go next to the 33-15 markdown subtest, using its helpers. Run `go test -short -count=1 -run 'TestConvertContentH1|TestConvertTranslationH1' ./classes` and the subtest, and record the failure lines (the probe comes back with "

Hello

" and n=1; the flipped pin fails the other way). + +Implementation (GREEN), classes/headings.go only: +- Add the unexported helper `convertH1OutsideCode(md string) (string, int)` after ConvertContentH1 (grep classes/ for the name first; pick another if it exists). It returns md and 0 at once when md contains neither " 0, else content's). Convert contentHTML with ConvertH1 and content with convertH1OutsideCode, with no first-byte (LooksLikeHTML) gate. Rewrite its doc comment: content is read as markdown, as every render path reads it (FormatHTML passes raw HTML through, so an HTML body is HTML blocks and is converted as before); an `

` inside a code block or code span keeps its tag whatever the body's first character, so a markdown post that opens with an HTML block keeps its code samples (33-REVIEW-GAPFIX WR-01); a raw `

` outside code becomes `

`, as in the rendered HTML. +- ConvertTranslationH1 doc comment: replace its sentence about markdown content with "content is rewritten outside code only (ConvertContentH1)". +- No other file in classes/ changes; LooksLikeHTML stays where it is. + +README.md (same commit): in the "On." bullet (line 181) replace the parenthetical after "markdown content" with: its source is converted outside code only (`classes.ConvertContentH1` reads `content` as markdown, as the renderer does): an `

` in a code block or code span keeps its tag, also when the body starts with an HTML block, and a raw `

` elsewhere becomes `

`, as in the rendered HTML. In the "Stored content." bullet (line 183) replace "of a markdown post only `content_html`" with "in a markdown source outside code only". In the API row at line 495 say `ConvertH1` over a post's `content` (outside markdown code) and `content_html`, and over one locale's translated attributes. + +After the commit below, check additivity in two steps so a failing git is not read as clean: `git diff 0a62090..$T1 -- . ':!*_test.go' > /xae-journal-api.diff` must exit 0, then `grep -E '^-func [A-Z]|^-func \([^)]*\) [A-Z]|^-type [A-Z]' /xae-journal-api.diff` must find nothing ( is the session scratchpad directory). + +Run `git status --short`, stage exactly classes/headings.go, classes/headings_test.go, headings_test.go and README.md by path, and commit: `fix: H1 conversion leaves code alone in markdown that starts with HTML`, body: ConvertContentH1 reads content as markdown on every path and converts it outside goldmark's code ranges only; a body opening with an HTML block keeps the h1 tags of its code samples, a raw h1 outside code is converted as in v0.3.1 (the 33-15 test pin for that case is reversed). quick-261104-xae, review WR-01. No trailer. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/sm-journal-plugin && go vet ./... && go test -short -count=1 ./... && go test -count=1 -run 'TestConvertH1|TestConvertContentH1|TestConvertTranslationH1' ./classes && go test -count=1 -run 'TestHeadingConversion|TestFixHeadingsCommand' . + The probe is rewritten or counted (n != 0) by ConvertContentH1 or ConvertTranslationH1; a raw h1 outside code stays h1; the API or import subtest stores a content different from the submitted markdown; any existing heading test regresses; go vet or a short test fails. + + + - `grep -n "func convertH1OutsideCode" classes/headings.go` and `grep -n "convertH1OutsideCode(content)" classes/headings.go` each match once. + - `grep -c 'lead' classes/headings_test.go` is at least 1 and the probe assertion expects n == 0; the line-79 assertion expects "plain

x

text" and 1. + - The SUMMARY quotes the RED failure line of the probe case on the old code and of the Docker subtest. + - The additivity diff check exits 0 and its grep finds nothing; `git show --stat $T1` lists exactly the four files (record T1 in the SUMMARY). + - README.md line 181, 183 and 495 describe the outside-code rule (`grep -c "outside code" README.md` grew by at least 2). + + Markdown content is converted outside code only, whatever its first byte; the probe is untouched with n=0; HTML bodies convert as before; the README matches; one commit on sm-journal-plugin main. + + + + Task 2: images/alt on content_html-only translations and on markdown that starts with a tag (sm-journal-plugin, WR-02) + /media/nvme/dev/golem15/summercms.io/summercms/sm-journal-plugin/controllers/api/maintenance.go, /media/nvme/dev/golem15/summercms.io/summercms/sm-journal-plugin/headings_test.go, /media/nvme/dev/golem15/summercms.io/summercms/sm-journal-plugin/README.md + Task 1 is committed on sm-journal-plugin main; `docker info` succeeds (else sudo -u golem). + controllers/api/maintenance.go lines 80-283, classes/format_html.go lines 300-320, headings_test.go lines 576-700 (TestSetImageAlt), posts_admin_smoke_test.go lines 300-320 (translatedAttrs), README.md line 187 + + New Docker test TestSetImageAltBodyShapes in headings_test.go (package journal_test), with a = "https://cdn.example.test/a.jpg", the editor token of TestSetImageAlt and POST journalAPIPrefix + "/images/alt": + - (a) content_html-only translation: create a post with content "Intro\n\n![](" + a + ")" and translations {"pl": {"title": "Obrazy", "content_html": "

"}}. One batch [{post_id, src: a, alt: "Grzyb"}, {post_id, locale: "pl", src: a, alt: "Lew"}] answers 200; the pl change has changed true and old_alt nil; the pl content_html holds alt="Lew"; the pl content is still blank (absent or whitespace); the default content became "Intro\n\n![Grzyb](" + a + ")". On the current code the batch answers 422 with changes.1.src and "not an inline image" (RED). + - (b) markdown starting with a tag: content "

Intro

\n\n![](" + a + ")\n\n```html\n\n```\n" gets alt "Grzyb": 200; the stored content equals "

Intro

\n\n![Grzyb](" + a + ")\n\n```html\n\n```\n" (label set, code sample untouched); content_html holds alt="Grzyb"; after PUT /posts/{id} with that same content the content_html still holds alt="Grzyb". On the current code the code sample's img is rewritten and the label stays empty (RED). + - (c) markdown starting with a tag that holds the image only by reference: content "

Lead

\n\n![old][r]\n\n[r]: " + a + "\n" answers 422 with changes.0.src and "not an inline image"; content and content_html are unchanged. On the current code it answers 200 with changed true (RED). + - TestSetImageAlt passes unchanged (its HTML-shaped default body and pl translation still get the alt in content and content_html). + + +Tests first (RED): add TestSetImageAltBodyShapes after TestSetImageAlt in headings_test.go with the three cases, reusing TestSetImageAlt's env setup, decode helper shape and assertions style (copy the small decode closure). Run `go test -count=1 -run 'TestSetImageAltBodyShapes' .` and record the three RED failures. + +Implementation (GREEN) in controllers/api/maintenance.go: replace the two-way branch on the first byte of text.content (lines 172-188) with a switch on the body shape, per 33-REVIEW-GAPFIX WR-02: +- `strings.TrimSpace(text.content) == ""`: content_html is the body (a translation that holds only content_html, or a host row without content); nothing else to keep in sync, so only content_html gets the alt and the change is answered as usual. +- default: the existing markdown rule, unchanged: SetMarkdownImageAlt, then SetImageAltOutsideCode on its result; m+r == 0 refuses the change with 422 changes.N.src and errMarkdownImage; otherwise text.content takes the result. Comment: content is read as markdown, as FormatHTML renders it on every API path; an HTML body (the rich editor's) is HTML blocks to goldmark, so its img is a raw img outside code and gets the alt here too; the first character no longer picks the branch, so markdown that opens with a tag keeps its code samples and is never answered changed and reverted later. +Keep everything else (the content_html SetImageAlt before the switch, the answer map, the transaction) as it is. Rewrite the handler doc comment (lines 96-113) to say: the post's content gets the alt too, read as markdown (an HTML body included): its inline image or a raw img outside code; a locale whose content is blank (a translation that carries only content_html) gets it in content_html alone. Run `go vet ./...`; if the classes import or any helper became unused, the build says so. + +README.md line 187 ("Image alt." bullet): replace the part from "and in `content`" to "so the next content save keeps the alt" with: and in `content`, read as markdown as the renderer reads it (an HTML body included, whose `` is a raw image outside code): its inline image `![alt](src)` (with or without a title, outside code; `classes.SetMarkdownImageAlt`) or a raw `` outside code (`classes.SetImageAltOutsideCode`), also when the markdown starts with an HTML tag, so the next content save keeps the alt; a locale whose `content` is blank (a translation that carries only `content_html`) gets the alt in `content_html` alone. Keep the rest of the bullet. + +Run `git status --short`, stage exactly controllers/api/maintenance.go, headings_test.go and README.md, and commit: `fix: image alt on HTML-only translations and on markdown that starts with a tag`, body: blank content means content_html is the body (no 422 for content_html-only translations, a regression from the markdown alt fix); any other content is handled as markdown, so a body opening with a tag gets its inline image label and keeps its code samples. quick-261104-xae, review WR-02. No trailer. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/sm-journal-plugin && go vet ./... && go test -short -count=1 ./... && go test -count=1 -run 'TestSetImageAlt' . && go test -count=1 -run 'TestSetImageAlt|TestSetMarkdownImageAlt' ./classes + A content_html-only translation's change is refused (422) or writes a content value; a markdown body starting with a tag keeps an empty label, has its code-sample img rewritten, or loses the alt after the next save; a reference-only image is answered 200; TestSetImageAlt's HTML-shaped bodies stop getting the alt in content; go vet or a short test fails. + + + - `grep -n 'strings.TrimSpace(text.content) == ""' controllers/api/maintenance.go` matches once. + - `grep -n "func TestSetImageAltBodyShapes" headings_test.go` matches once; the SUMMARY quotes its three RED failures on the Task 1 commit. + - TestSetImageAlt's body is unchanged: `git diff $T1..$T2 -- headings_test.go > /xae-t2-tests.diff` exits 0 and the file has no removed line inside TestSetImageAlt; the test passes. + - No exported Go identifier changed: `git diff $T1..$T2 -- controllers/api/maintenance.go > /xae-t2-api.diff` exits 0 and `grep -E '^-func [A-Z]' /xae-t2-api.diff` finds nothing; `git show --stat $T2` lists exactly the three files. + - README.md line 187 names the blank-content case (`grep -c "is blank (a translation that carries only" README.md` is at least 1). + + images/alt accepts content_html-only translations again, handles every non-blank body as markdown, and refuses what the markdown cannot hold; one commit on sm-journal-plugin main. + + + + Task 3: the crawl's final status gets its own bound (sm-seo-plugin, WR-03) + /media/nvme/dev/golem15/summercms.io/summercms/sm-seo-plugin/classes/crawl/job.go, /media/nvme/dev/golem15/summercms.io/summercms/sm-seo-plugin/crawl_lifecycle_test.go, /media/nvme/dev/golem15/summercms.io/summercms/sm-seo-plugin/README.md + sm-seo-plugin is on branch main at 2a288a9 (or a descendant with no change to classes/crawl/job.go); `docker info` succeeds (else sudo -u golem). + classes/crawl/job.go lines 40-60, 190-230 and 355-420, classes/crawl/render.go lines 36-55, classes/crawl/store.go lines 19-85, classes/crawl/actions.go lines 95-103, crawl_lifecycle_test.go (whole), crawl_site_test.go lines 40-120, 170-200 and 305-360, README.md line 250 and lines 340-347 + + New Docker test TestCrawlStoreTimeout in crawl_lifecycle_test.go, after TestCrawlTimeout: + - A fake site with "/" (title Home) and "/a" in its sitemap; a crawl started through the API (env.startCrawl with env.admin(t, true)). + - A separate transaction on env.db runs `LOCK TABLE golem15_seo_crawl_pages IN SHARE MODE` before the job starts, so the store's first write (SavePages' insert of the pages) waits; it is rolled back as soon as the row has left running, and in t.Cleanup in any case. + - The job runs through env.runCrawlAsync with ctx = crawl.WithFinishTimeout(fetchguard.WithTransport(t.Context(), site), 2*time.Second). + - Within 20 s the row leaves running: status failed, error containing "storing the crawl results took longer than 2s", error not containing "time limit", finished_at set. After the lock is released the job returns nil (waitJob), no page of the crawl is stored (the store rolled back), and a second env.startCrawl succeeds. + - RED on the old finish: the row stays running (the terminal UPDATE runs on the expired store context) and the job returns a deadline error. + - TestCrawlTimeout, TestCrawlCancelRunning, TestCrawlStaleRecovery and TestCrawlerTracer pass unchanged. + + +Step A, seam only (no behaviour change yet): in classes/crawl/job.go add `type finishTimeoutKey struct{}`, the exported `WithFinishTimeout(ctx context.Context, d time.Duration) context.Context` and the unexported `finishTimeoutFrom(ctx context.Context) time.Duration` (the ctx value when it is a positive duration, else FinishTimeout), modelled on WithRenderer/rendererFrom with the same doc wording: the test seam of the store bound, settable only from Go code, no config key, setting, request or MCP call reaches it. Make Run use `finishTimeoutFrom(ctx)` in place of FinishTimeout for its existing fctx. Write TestCrawlStoreTimeout as in behavior and run `go test -count=1 -run TestCrawlStoreTimeout .`: it must fail because the row stays running (record the line). + +Step B, the fix (per 33-REVIEW-GAPFIX WR-03): +- Next to heartbeatWrite add the unexported `finalWriteTimeout = 30 * time.Second`, documented as the bound of a crawl's final status write and the end of its conga record, on a context of their own, so a store that used up its bound still ends the crawl. +- Run: stop building fctx; call `finish(context.WithoutCancel(ctx), svc, gdb, row, res, runErr, ctx.Err(), finishTimeoutFrom(ctx))`. finish (unexported) takes the detached parent context, the undecorated *gorm.DB and the store bound. +- finish: create the store context `context.WithTimeout(parent, storeBound)` (deferred cancel) and use `gdb.WithContext` of it for cancelRequested, LoadLimits (with its existing default fallback) and `lagoon.Transaction(storeCtx, storeDB, store)`; the store closure is unchanged. When the transaction fails, status is failed and errText is the error; when the store context's error is context.DeadlineExceeded, errText becomes "storing the crawl results took longer than " + storeBound.String() + ": " + the error. Then create a fresh `context.WithTimeout(parent, finalWriteTimeout)` (deferred cancel) and run the terminal Updates on `gdb.WithContext` of it (still returning its error), and FailJob/StopJob/CompleteJob on that context. The failed-crawl warn log stays. Then Prune on its own `context.WithTimeout(parent, storeBound)` (deferred cancel) with the existing warn on error; comment that Prune deletes old crawls with their pages and links, keeps a store-sized bound and runs after the final state is written, so it never holds a crawl running. +- Update the doc comments: the const block (FinishTimeout bounds storing what a crawl found; the final status is then written on a 30-second bound of its own), Run's paragraph about the detached context, and finish's (its contexts and bounds). + +README.md: in the "Limits." paragraph (line 250), after "bounded by `crawl.FinishTimeout` (10 min)" add: the crawl's final status is then written on a 30-second bound of its own, so a crawl whose results take longer to store still ends at once, failed with "storing the crawl results took longer than 10m0s: …". Add an API row after the `crawl.WithRenderer` row: `crawl.WithFinishTimeout` | A code-only test override of `crawl.FinishTimeout`, like `crawl.WithRenderer`. + +Run `go vet ./...` and the verify command, then `git status --short`, stage exactly classes/crawl/job.go, crawl_lifecycle_test.go and README.md, and commit: `fix: a crawl whose store runs out of time still ends failed`, body: the store keeps FinishTimeout, the final status write and the conga job end get their own 30 s bound and prune its own store-sized bound, so the row no longer stays running with the error lost until RecoverStale; adds the code-only crawl.WithFinishTimeout seam. quick-261104-xae, review WR-03. No trailer. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/sm-seo-plugin && go vet ./... && go test -short -count=1 ./... && go test -count=1 -run 'TestCrawlStoreTimeout|TestCrawlTimeout|TestCrawlCancelRunning|TestCrawlStaleRecovery|TestCrawlerTracer' . + The crawl row stays running after its store ran past the bound, its error lacks "storing the crawl results took longer than 2s", finished_at is unset, the job returns an error, the next Start is refused, or an existing crawl lifecycle test regresses; go vet or a short test fails. + + + - `grep -n "func WithFinishTimeout" classes/crawl/job.go` matches once; `grep -c "finalWriteTimeout" classes/crawl/job.go` is at least 2; `grep -n "took longer than" classes/crawl/job.go` matches once. + - `grep -n "func TestCrawlStoreTimeout" crawl_lifecycle_test.go` matches once; the SUMMARY quotes its Step A RED line. + - The only new exported identifier is WithFinishTimeout: `git diff 2a288a9..$T3 -- classes/crawl/job.go > /xae-t3.diff` exits 0, `grep -E '^\+func [A-Z]' /xae-t3.diff` shows only WithFinishTimeout and `grep -E '^-func [A-Z]' /xae-t3.diff` finds nothing. + - README.md mentions `crawl.WithFinishTimeout` and the 30-second final write; `git show --stat $T3` lists exactly the three files. + + A store that times out no longer strands a running crawl: the crawl ends failed with the real error right away; one commit on sm-seo-plugin main. + + + + Task 4: remember the last definite site answer, absent included (vue-seo-layer, WR-04); release notes; full gate + /media/nvme/dev/golem15/summercms.io/summercms/vue-seo-layer/server/lib/site.ts, /media/nvme/dev/golem15/summercms.io/summercms/vue-seo-layer/server/middleware/seo-root.ts, /media/nvme/dev/golem15/summercms.io/summercms/vue-seo-layer/tests/rootFiles.test.ts, /media/nvme/dev/golem15/summercms.io/summercms/vue-seo-layer/README.md, /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/.planning/phases/33-seo-output-audit-and-content/33-14-notes-sm-journal-plugin-v0.4.0.md, /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/.planning/phases/33-seo-output-audit-and-content/33-14-notes-sm-seo-plugin-v0.2.0.md, /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/.planning/phases/33-seo-output-audit-and-content/33-14-notes-vue-seo-layer-v0.2.0.md + vue-seo-layer is on branch main at 973142a (or a descendant with no change to server/lib/site.ts); Tasks 1-3 are committed; Docker is reachable for the gate's docker stage (else sudo -u golem). + server/lib/site.ts (whole), server/lib/lru.ts lines 1-40, server/middleware/seo-root.ts lines 1-40, server/api/__seo__/sitemap.get.ts lines 1-45, tests/rootFiles.test.ts (whole), README.md line 32, the three 33-14 release-note drafts (whole, 21-33 lines each) + + New node --test cases in tests/rootFiles.test.ts driving siteAnswer(seoServerConfig({}), {}) with a stubbed `globalThis.$fetch = { raw }` (records calls; answers {status, _data} objects or throws) and `t.mock.method(Date, 'now', () => clock)`; each case calls clearSiteCache() first and restores the previous $fetch at its end: + - "a remembered absent outlives the cache while the backend is down (WR-04)": 404 gives absent; clock moves past SITE_CACHE_TTL_MS; then a thrown error, 500 {}, 200 "" and 401 {} each give state absent, each call fetches again (the call count grows by one per call), and rootFileTargets for /robots.txt, /sitemap.xml and /llms.txt with that state is null. On the old code (with only the clock change below) the state is unknown (RED). + - "the latest definite answer wins": ok, expire, 404, expire, failure gives absent; expire, ok, expire, failure gives ok and rootFileTargets('/robots.txt') is the root target. On the old code the first failure gives ok (RED). + - Guard (passes before and after): never answered, a failure gives unknown, rootFileTargets('/robots.txt', ...) is [{kind: 'root', url: B + '/robots.txt'}] (the middleware's failing fetch then answers 503), and a second failure fetches again (unknown is never cached). + - Guard (passes before and after): absent, then clearSiteCache(), then a failure gives unknown (a purge forgets the remembered answer). + - Every existing rootFiles.test.ts case passes unchanged. + + +Step A (no behaviour change): in server/lib/site.ts pass `now: () => Date.now()` to the site cache's createLru options, so the clock is read at call time and tests can move it. Write the four cases as in behavior (import siteAnswer, clearSiteCache and SITE_CACHE_TTL_MS from '../server/lib/site.ts'). Run `npm test` in vue-seo-layer: the two WR-04 cases fail with state unknown / ok (record the lines), the guards pass. + +Step B, the fix (per 33-REVIEW-GAPFIX WR-04): rename lastOk to lastKnown, store every answer that is not unknown in it (ok or absent), and return it for an unknown answer when set; clearSiteCache still clears it (a purge drops every answer fetched before it, as the cache does). Rewrite the header comment's state list: unknown means the backend did not answer or answered something other than a 200 object or a 404; the last definite answer (ok or absent) seen since the server started or the last purge is then used; with none, robots.txt, sitemap.xml and llms.txt still go to the backend (a failure is a 503 with Retry-After) and every other path behaves as with absent. Update the rootFileTargets doc comment (a site answer or an absent one may be the remembered one; unknown here means never answered) and the seo-root.ts comment lines 10-19 (the last answer, ok or absent, is kept while the backend is down, so on a backend without the SEO plugin the files keep reaching the app during an outage; only a backend that has not answered since start or the last purge gets the fixed names proxied). No other server file changes. + +README.md "Root files." bullet (line 32): say the site answer is cached for 300 seconds and cleared by the purge, and while the backend is down the last answer it gave is used, a 404 included, so on a site whose backend has no SEO plugin robots.txt and the sitemaps keep coming from the app during an outage; the 503 sentence then applies while the backend has not answered since the server started or since the last purge. + +Run `npm test` (all files, `# fail 0`), then `git status --short`, stage exactly server/lib/site.ts, server/middleware/seo-root.ts, tests/rootFiles.test.ts and README.md, and commit in vue-seo-layer: `fix: root files follow the last site answer, a 404 included, while the backend is down`, body: an SEO-less site's robots.txt, sitemap.xml and llms.txt keep reaching the app during an outage longer than the cache TTL; a backend that never answered still gets the 503. quick-261104-xae, review WR-04. No trailer. + +Release notes in summercms.go (user-facing wording, no planning IDs): +- 33-14-notes-sm-journal-plugin-v0.4.0.md, Changed: replace the H1 bullet (line 20) with: The H1 to H2 conversion leaves code samples alone: in a markdown body an `

` inside a code block or code span keeps its tag, also when the body starts with an HTML block, and a raw `

` elsewhere is converted as in the rendered page (API saves, imports, `journal:fix-headings` and translations). Replace the alt bullet (line 21) with: Setting an image's alt text through the API or MCP on a markdown post writes it into the markdown (`![alt](src)`, or a raw `` outside code), also when the markdown starts with an HTML tag, so the next save keeps it; an image the markdown does not hold that way is refused with a validation error. A translation that holds only HTML gets the alt in its HTML. +- 33-14-notes-sm-seo-plugin-v0.2.0.md, Added, the crawl cancel and time limits bullet (line 16): append: When storing a crawl's results takes longer than 10 minutes, the crawl still ends at once, failed with that error, instead of staying "running" until the next start. +- 33-14-notes-vue-seo-layer-v0.2.0.md, Fixed (line 25): keep the sentence and append: Once the backend has answered, its last answer is used while it is down, so on a site whose backend has no SEO plugin these files keep coming from the app during an outage. +Run `git -C status --short`, stage exactly the three files by path, and commit: `docs(33): release notes for the gap-fix follow-ups (quick-261104-xae)`. No trailer. + +Full gate from summercms.go: `set -o pipefail; PHASE33_REQUIRE_DOCKER=1 bash scripts/check-phase33.sh --plugins --layers --docker 2>&1 | tee /gate-xae.log`, where is the session scratchpad directory; it must exit 0 and the log must hold the lines `PASS plugins`, `PASS layers` and `PASS docker`. A failure caused by these changes is fixed in the task's own files and committed as a follow-up fix commit in the affected repo (same quick id); a pre-existing, unrelated failure is reported in the SUMMARY with its log line, not fixed. + + + set -o pipefail && cd /media/nvme/dev/golem15/summercms.io/summercms/vue-seo-layer && npm test 2>&1 | tee /tmp/claude-1000/-media-nvme-dev-golem15-summercms-io-summercms-summercms-go/cc746d7e-76ce-49f4-ae43-0e1debb2bbea/scratchpad/xae-node-test.log | tail -5 && grep -q '^# fail 0$' /tmp/claude-1000/-media-nvme-dev-golem15-summercms-io-summercms-summercms-go/cc746d7e-76ce-49f4-ae43-0e1debb2bbea/scratchpad/xae-node-test.log && cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && PHASE33_REQUIRE_DOCKER=1 bash scripts/check-phase33.sh --plugins --layers --docker 2>&1 | tee /tmp/claude-1000/-media-nvme-dev-golem15-summercms-io-summercms-summercms-go/cc746d7e-76ce-49f4-ae43-0e1debb2bbea/scratchpad/gate-xae.log && grep -qx 'PASS plugins' /tmp/claude-1000/-media-nvme-dev-golem15-summercms-io-summercms-summercms-go/cc746d7e-76ce-49f4-ae43-0e1debb2bbea/scratchpad/gate-xae.log && grep -qx 'PASS layers' /tmp/claude-1000/-media-nvme-dev-golem15-summercms-io-summercms-summercms-go/cc746d7e-76ce-49f4-ae43-0e1debb2bbea/scratchpad/gate-xae.log && grep -qx 'PASS docker' /tmp/claude-1000/-media-nvme-dev-golem15-summercms-io-summercms-summercms-go/cc746d7e-76ce-49f4-ae43-0e1debb2bbea/scratchpad/gate-xae.log + After an absent answer and an expired cache a failing backend yields unknown or targets /robots.txt, /sitemap.xml or /llms.txt; an ok answer outlives a later absent one; a never-answered backend stops targeting the fixed names; any node --test case fails; the gate exits non-zero or lacks a PASS line for plugins, layers or docker. + + + - `grep -c "lastKnown" server/lib/site.ts` is at least 3 and `grep -c "now: () => Date.now()" server/lib/site.ts` is 1; `grep -c "siteAnswer" tests/rootFiles.test.ts` is at least 4. + - The SUMMARY quotes the two WR-04 RED lines from Step A. + - vue-seo-layer `git show --stat $T4` lists exactly the four files; `git diff --quiet 973142a..$T4 -- package.json pnpm-lock.yaml` exits 0. + - The drafts hold the new text: `grep -c "code block or code span" 33-14-notes-sm-journal-plugin-v0.4.0.md` is 1, `grep -c "holds only HTML" 33-14-notes-sm-journal-plugin-v0.4.0.md` is 1, `grep -c "longer than 10 minutes" 33-14-notes-sm-seo-plugin-v0.2.0.md` is 1, `grep -c "no SEO plugin these files keep coming from the app" 33-14-notes-vue-seo-layer-v0.2.0.md` is 1; summercms.go `git show --stat $TN` lists exactly those three files. + - gate-xae.log holds `PASS plugins`, `PASS layers` and `PASS docker`; the command exited 0. + + The layer keeps an SEO-less site's root files with the app during outages and still answers 503 for a backend that never answered; the three drafts describe all four fixes; every repo's tests and the Phase 33 gate stages pass. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| API/MCP client -> Journal post writes and images/alt | Markdown content, translations and alt texts are client-controlled; conversions must not corrupt code samples and alt text must stay escaped | +| crawl job -> Postgres | The crawl's terminal state and the one-at-a-time guard depend on the final write landing | +| search-engine crawler -> Nuxt root files | robots.txt answers decide whether a whole site is crawled | +| test code -> crawl job context | WithFinishTimeout is a context value only Go code can set | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan | +|-----------|----------|-----------|----------|-------------|-----------------| +| T-261104xae-01 | Tampering | classes.ConvertContentH1 on markdown that starts with an HTML block | medium | mitigate | Task 1 converts content outside goldmark code ranges only; the probe test and the API/import subtest pin byte-identical code samples | +| T-261104xae-02 | Tampering | images/alt on markdown that starts with a tag | low | mitigate | Task 2 routes every non-blank content through SetMarkdownImageAlt and SetImageAltOutsideCode, whose labels are backslash-escaped and whose img attributes are re-encoded by the tokenizer (T-33-74 unchanged); code samples are not touched | +| T-261104xae-03 | Denial of service | images/alt batch with a content_html-only translation | low | mitigate | Blank content no longer refuses the whole batch; the alt still goes only through SetImageAlt's tokenizer encoding | +| T-261104xae-04 | Denial of service | crawl finish after a store timeout | medium | mitigate | Task 3 writes the final state on its own 30 s bound, so the guard is released at once instead of after RecoverStale; prune keeps a store-sized bound after the final write | +| T-261104xae-05 | Elevation of privilege | crawl.WithFinishTimeout seam | low | accept | A context value settable only from Go code, like WithRenderer and fetchguard.WithTransport; no config key, setting, request or MCP call reaches it | +| T-261104xae-06 | Denial of service | robots.txt 503 on a site without the SEO plugin | medium | mitigate | Task 4 remembers an absent answer, so a backend outage no longer turns the site's robots.txt into a 5xx that crawlers read as a site-wide disallow; never-answered backends keep the 503 that protects a staging Disallow | +| T-261104xae-07 | Information disclosure | remembered site answer across purges | low | accept | The purge still clears the memory; the remembered answer is only the backend's own public site answer and is used only while the backend is unreachable | +| T-261104xae-SC | Tampering | go modules / npm packages | low | accept | No dependency added or changed; go.mod, go.sum, package.json and pnpm-lock.yaml stay untouched in all repos | + + + +- sm-journal-plugin: go vet ./... clean; go test -short ./... green; Docker TestHeadingConversion, TestFixHeadingsCommand, TestSetImageAlt, TestSetImageAltBodyShapes green; no exported signature changed since 0a62090. +- sm-seo-plugin: go vet ./... clean; go test -short ./... green; Docker TestCrawlStoreTimeout and the crawl lifecycle tests green. +- vue-seo-layer: npm test `# fail 0`. +- summercms.go: `PHASE33_REQUIRE_DOCKER=1 bash scripts/check-phase33.sh --plugins --layers --docker` under pipefail exits 0 with PASS plugins, PASS layers, PASS docker. +- Every new test's RED failure on the pre-fix code is quoted in the SUMMARY. +- Commits: sm-journal-plugin 2, sm-seo-plugin 1, vue-seo-layer 1, summercms.go 1 (notes only), each listing only its task's files, no trailer; nothing pushed or tagged; other sessions' uncommitted files untouched. + + + +- WR-01: the reviewer's probe is left unchanged with n=0 on every conversion path; real raw H1s outside code are still converted. +- WR-02: content_html-only translations get their alt (200), markdown starting with a tag gets its label set and keeps code samples, unsupported images get 422. +- WR-03: a store timeout ends the crawl failed with the store error at once; the next crawl can start. +- WR-04: an SEO-less site's root files keep reaching the app during an outage; a never-answered backend still gets 503 for robots.txt, sitemap.xml and llms.txt. +- Release-note drafts updated in one planning-docs commit; the Phase 33 gate stages plugins, layers and docker pass. + + + +Create `/media/nvme/dev/golem15/summercms.io/summercms/summercms.go/.planning/quick/261104-xae-phase-33-gap-fix-follow-ups/261104-xae-SUMMARY.md` when done, with: BASE and HEAD per repo; the commit list; per finding, the test and its quoted RED line; the planner choices (no first-byte gate and the reversed line-79 pin; the markdown branch covering HTML bodies and its indented-code residual; the WithFinishTimeout seam, the error prefix and prune's own bound; the purge still clearing the remembered answer, and a remembered absent making /api/__seo__/sitemap answer 404 during an outage); the residuals (a backend whose /site answers 200 HTML or 401/403 stays unknown, workaround seo.rootFiles false or seo.cms false; the review's notes below the warning bar, untouched); the gate log path with its PASS lines. Do not edit STATE.md (the orchestrator handles state and the g15office comment). + diff --git a/.planning/quick/261104-xae-phase-33-gap-fix-follow-ups/261104-xae-SUMMARY.md b/.planning/quick/261104-xae-phase-33-gap-fix-follow-ups/261104-xae-SUMMARY.md new file mode 100644 index 00000000..e925e90c --- /dev/null +++ b/.planning/quick/261104-xae-phase-33-gap-fix-follow-ups/261104-xae-SUMMARY.md @@ -0,0 +1,149 @@ +--- +phase: quick-261104-xae +plan: 01 +subsystem: seo, journal +tags: [sm-journal-plugin, sm-seo-plugin, vue-seo-layer, goldmark, crawl, nitro] +status: complete +requires: + - phase: 33-seo-output-audit-and-content + provides: "33-15 / 33-16 gap closure, re-reviewed in 33-REVIEW-GAPFIX.md" +provides: + - "WR-01: ConvertContentH1 converts markdown content outside goldmark code ranges only, with no first-byte gate" + - "WR-02: images/alt treats blank content as content_html-only and every other content as markdown" + - "WR-03: the crawl's final status write and conga end run on their own 30 s bound; crawl.WithFinishTimeout seam" + - "WR-04: siteAnswer remembers the last definite answer (ok or absent)" + - "Release-note drafts for sm-journal-plugin v0.4.0, sm-seo-plugin v0.2.0, vue-seo-layer v0.2.0 updated" +affects: [33-14] +actuals: + tokens: 7598 + tasks: 4 + commits: 5 +tech-stack: + added: [] + patterns: + - "Code-only test seam carried by the job context (crawl.WithFinishTimeout, like WithRenderer)" + - "Split timeouts: store bound, final-write bound, prune bound, all on a detached parent" +key-files: + created: [] + modified: + - ../sm-journal-plugin/classes/headings.go + - ../sm-journal-plugin/classes/headings_test.go + - ../sm-journal-plugin/headings_test.go + - ../sm-journal-plugin/controllers/api/maintenance.go + - ../sm-journal-plugin/README.md + - ../sm-seo-plugin/classes/crawl/job.go + - ../sm-seo-plugin/crawl_lifecycle_test.go + - ../sm-seo-plugin/README.md + - ../vue-seo-layer/server/lib/site.ts + - ../vue-seo-layer/server/middleware/seo-root.ts + - ../vue-seo-layer/tests/rootFiles.test.ts + - ../vue-seo-layer/README.md + - .planning/phases/33-seo-output-audit-and-content/33-14-notes-sm-journal-plugin-v0.4.0.md + - .planning/phases/33-seo-output-audit-and-content/33-14-notes-sm-seo-plugin-v0.2.0.md + - .planning/phases/33-seo-output-audit-and-content/33-14-notes-vue-seo-layer-v0.2.0.md +key-decisions: + - "ConvertContentH1 has no first-byte gate: content is always read as markdown and converted outside code; the 33-15 pin 'plain

x

text' flips to converted (v0.3.1 behaviour for raw h1 outside code)" + - "images/alt markdown branch also covers rich-editor HTML bodies (HTML blocks are not code ranges)" + - "Prune keeps a store-sized bound of its own after the final write; error prefix 'storing the crawl results took longer than : '" + - "clearSiteCache (purge) still clears the remembered site answer" +requirements-completed: [quick-261104-xae] +duration: 16min +completed: 2026-11-05 +--- + +# Quick 261104-xae: Phase 33 gap-fix follow-ups Summary + +Closed the four warnings of the Phase 33 gap-closure re-review (33-REVIEW-GAPFIX WR-01 to WR-04). H1 conversion now skips markdown code wherever the body starts. Image alt works on content_html-only translations and on markdown that opens with a tag. A crawl whose store times out ends failed straight away. The SEO layer remembers an absent site answer through outages. + +## BASE and HEAD per repo + +| Repo | Branch | BASE | HEAD | +|------|--------|------|------| +| sm-journal-plugin | main | 0a62090 | cf13c60 | +| sm-seo-plugin | main | 2a288a9 | c71b58e | +| vue-seo-layer | main | 973142a | 3868f3d | +| summercms.go | master | 5ce50be | 9d299a4 | + +All four BASEs matched the planning-time values. Nothing pushed or tagged. The untracked `go.work.sum` in sm-journal-plugin and the other sessions' files in summercms.go were not staged. + +## Commits + +- T1 sm-journal-plugin `a41d537` fix: H1 conversion leaves code alone in markdown that starts with HTML (classes/headings.go, classes/headings_test.go, headings_test.go, README.md) +- T2 sm-journal-plugin `cf13c60` fix: image alt on HTML-only translations and on markdown that starts with a tag (controllers/api/maintenance.go, headings_test.go, README.md) +- T3 sm-seo-plugin `c71b58e` fix: a crawl whose store runs out of time still ends failed (classes/crawl/job.go, crawl_lifecycle_test.go, README.md) +- T4 vue-seo-layer `3868f3d` fix: root files follow the last site answer, a 404 included, while the backend is down (server/lib/site.ts, server/middleware/seo-root.ts, tests/rootFiles.test.ts, README.md) +- TN summercms.go `9d299a4` docs(33): release notes for the gap-fix follow-ups (quick-261104-xae) (the three 33-14 drafts) + +None of the commits has a trailer. + +## RED / GREEN per finding + +**WR-01 (Task 1).** The tests are TestConvertContentH1HTMLLead (new), new rows in TestConvertTranslationH1, the flipped pin in TestConvertContentH1, and the Docker subtest "markdown that starts with an HTML block keeps its h1 samples". +- RED (old code): + - `headings_test.go:99: probe with content_html "

Intro

\n
<h1>Hello</h1>\n
\n" = "

Intro

\n\n```html\n

Hello

\n```" ... n=1` + - `headings_test.go:141: markdown with an HTML lead was rewritten: ...

Hello

... n=1` + - The flipped pin failed the other way: `headings_test.go:82: markdown without HTML = "plain

x

text" "" 0`. + - Docker: `headings_test.go:330: API save: content = "

Intro

\n\n# Title\n\n```html\n

Hello

\n```", content_html = ...`. +- GREEN: TestConvertH1, TestConvertContentH1, TestConvertContentH1HTMLLead and TestConvertTranslationH1 pass. The Docker TestHeadingConversion and TestFixHeadingsCommand pass. go vet and the short tests are clean. +- Additivity: `git diff 0a62090..a41d537 -- . ':!*_test.go'` exited 0, and the grep for removed exported func/type lines found nothing. + +**WR-02 (Task 2).** The test is TestSetImageAltBodyShapes (Docker). RED on T1: +- (a) `headings_test.go:750: POST /_journal/api/v1/images/alt = 422, want 200: {"error":"Validation failed","errors":{"changes.1.src":["The image is not an inline image of the post's markdown; edit the post."]}}` +- (b) `headings_test.go:782: after the alt = "

Intro

\n\n![](https://cdn.example.test/a.jpg)\n\n```html\n\"Grzyb\"\n```\n" / ...`: the label stays empty and the code sample is rewritten. +- (c) `headings_test.go:794: POST /_journal/api/v1/images/alt = 200, want 422: {"changes":[{"alt":"new","changed":true,...,"old_alt":"old",...}]}` +- GREEN: TestSetImageAlt (body unchanged; the diff removes no line from it) and TestSetImageAltBodyShapes pass, as do the classes TestSetImageAlt and TestSetMarkdownImageAlt. The grep for removed exported functions in maintenance.go found nothing. + +**WR-03 (Task 3).** The test is TestCrawlStoreTimeout (Docker). It holds a `LOCK TABLE golem15_seo_crawl_pages IN SHARE MODE` transaction and runs with `crawl.WithFinishTimeout(..., 2s)`. +- RED (Step A, seam only): `crawl_lifecycle_test.go:286: the crawl is still running 20 s after its store bound: {ID:1 Status:running ... Error: ... FinishedAt: ...}`. +- GREEN: the row ends failed with "storing the crawl results took longer than 2s: ..." and finished_at set. The job returns nil, no pages are stored, and the next Start is accepted. TestCrawlTimeout, TestCrawlCancelRunning, TestCrawlStaleRecovery and TestCrawlerTracer pass. +- `git diff 2a288a9..c71b58e -- classes/crawl/job.go`: the only `+func [A-Z]` is WithFinishTimeout, and there is no `-func [A-Z]`. + +**WR-04 (Task 4).** These are new node --test cases in tests/rootFiles.test.ts, run with a stubbed `globalThis.$fetch` and `t.mock.method(Date, 'now')`. +- RED (Step A, clock read at call time only): + - `not ok 84 - siteAnswer: a remembered absent outlives the cache while the backend is down (WR-04)`: `+ 'unknown' - 'absent'` + - `not ok 85 - siteAnswer: the latest definite answer wins (WR-04)`: `+ 'ok' - 'absent'` +- The two guard cases (never answered stays unknown, a purge forgets) passed before and after the fix. +- GREEN: `npm test` reports `# pass 156`, `# fail 0` (log: /tmp/claude-1000/-media-nvme-dev-golem15-summercms-io-summercms-summercms-go/cc746d7e-76ce-49f4-ae43-0e1debb2bbea/scratchpad/xae-node-test.log). package.json and pnpm-lock.yaml are unchanged. + +## Gate + +`PHASE33_REQUIRE_DOCKER=1 bash scripts/check-phase33.sh --plugins --layers --docker` exited 0 under pipefail. Docker was reachable directly, so `sudo -u golem` was not needed. The log is /tmp/claude-1000/-media-nvme-dev-golem15-summercms-io-summercms-summercms-go/cc746d7e-76ce-49f4-ae43-0e1debb2bbea/scratchpad/gate-xae.log and holds: + +``` +PASS plugins +PASS layers +PASS docker +``` + +## Planner choices (as accepted) + +- **No first-byte gate.** ConvertContentH1 always reads content as markdown and converts outside goldmark code ranges, through the unexported `convertH1OutsideCode`. The 33-15 pin `ConvertContentH1("plain

x

text", "")` is deliberately reversed to `"plain

x

text"`, n=1, which matches v0.3.1 for a raw h1 outside code. +- **HTML bodies.** The images/alt markdown branch covers rich-editor HTML bodies too, because HTML blocks are not code ranges. TestSetImageAlt's HTML-shaped bodies keep passing unchanged. Residual: an admin HTML body in which a blank line is followed by a line indented 4 or more spaces holding the `` is an indented code block to goldmark, so that image gets 422. This is not special-cased. +- **Crawl bounds.** + - `crawl.WithFinishTimeout` is a code-only seam carried by the job context, like WithRenderer. + - The store runs on its own bound and the error prefix is "storing the crawl results took longer than : ". + - The final UPDATE and the conga FailJob/StopJob/CompleteJob run on `finalWriteTimeout` (30 s). + - Prune runs last, on its own store-sized bound. +- **Purge.** A purge (`clearSiteCache`) still clears the remembered answer, so "never answered" means since the server started or since the last purge. With a remembered absent, `/api/__seo__/sitemap` answers 404 during an outage, as it does while the backend is up. + +## Deviations from Plan + +1. **Doc comment reworded in Task 3 (acceptance wording).** The first finish doc comment quoted the literal "storing the crawl results took longer than", so `grep -n "took longer than" classes/crawl/job.go` matched twice where the plan asked for once. I reworded the comment ("an error that names the bound and the store error") and amended the unpushed local commit (0953294 became c71b58e). The code did not change. +2. **Test helper typing.** In tests/rootFiles.test.ts the stub helper takes `TestContext` (a type-only import from node:test), so the test needs no loose `Function` type. This is cosmetic. + +No other deviations. Pre-existing `gofmt -l` hits in sm-journal-plugin (classes/blog_editor_test.go, controllers/api/posts.go) are in files this task does not touch and were left alone. + +## Residuals + +- A backend whose `/site` always answers 200 HTML or 401/403 never gives a definite answer and stays unknown, so its fixed root files are still proxied and a failure is a 503. The workaround is `seo.rootFiles: false` or `seo.cms: false`. This is out of scope per the brief. +- The review's notes below the warning bar are untouched. + +## Known Stubs + +None. + +## Self-Check: PASSED + +- The commits exist: a41d537 and cf13c60 (sm-journal-plugin), c71b58e (sm-seo-plugin), 3868f3d (vue-seo-layer), 9d299a4 (summercms.go). +- Each commit's `git show --stat` lists exactly its task's files. +- The acceptance greps match: convertH1OutsideCode (definition and call), the TrimSpace switch, WithFinishTimeout, finalWriteTimeout (5 matches), lastKnown (4), siteAnswer in the test (7), `now: () => Date.now()` (1), and every release-note phrase (1 each).