Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 68 additions & 4 deletions .blueframe/state.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"version": 1,
"project": "lords-dashboard-backend",
"lastSyncedCommit": "66f761add2143ef71722883fec34249a80ef6697",
"lastSyncedAt": "2026-08-05T19:45:00Z",
"notesForNextSession": "T-0301 is in REVIEW as PR #86, coupled to frontend PR 73. T-0297/T-0298 are merged into DEV (PR #84) but NOT on main.\n\n⚠️ NOTHING FROM THIS WORK IS IN PRODUCTION YET. main is the production branch; dev has to be promoted to main and the WEB image redeployed. The crawler routing now lives in the frontend's nginx.conf, so `deploy web <tag>` is what makes link previews start working — the API half has been correct and deployed since T-0293.\n\nVERIFY AFTER DEPLOY (the fast tell): a crawler-path response carrying `Vary: Accept-Encoding` + `Cache-Control: no-store` is the web nginx answering; the API sends `Vary: User-Agent` + `max-age=600`. As of the last check production still returns the former, i.e. the old build.\n\nTHE ROOT CAUSE nobody should re-diagnose: link previews were identical everywhere because production ran a Caddyfile predating even the T-0197 gallery unfurler. /api/seo/* was always correct. Cloudflare's certificate config is NOT involved — that was the first hypothesis and it was wrong (cf-cache-status was DYNAMIC throughout).\n\nTHE LESSON FROM THE REVIEW, worth keeping: the two bugs that mattered were both in code I had written careful comments about. /api/oembed declared `@Query('author') author: string | undefined` — Express does not honour that, `qs` yields string[] for a repeated key, and the global ValidationPipe coerces only Number and Boolean. That 500'd a @Public() route AND defeated a URL scheme check, because RegExp.test stringifies its argument. **Every other @Query typed `string` in this codebase has the same latent bug** and was NOT audited — that is the open regressionRisk on T-0301 and the most valuable thing to pick up next. Separately, describeEvents was changed without its seo-copy.ts twin, in the same PR whose comments warn three times about exactly that.\n\nSTILL NOT DONE. (a) Nothing pasted into a real Discord channel — the oEmbed author line is unit-tested only, and Discord caches an unfurl ~30min with no purge, so append a throwaway query string when re-testing. (b) The Caddyfile on the box is un-synced; it no longer blocks previews but still owns the www→apex 301, which nginx cannot do. (c) Search Console: the sitemap has never been submitted, and deploy/README.md is explicit that Authenticated Origin Pulls + TRUST_CF_CONNECTING_IP must land FIRST — until then every crawler shares one bucket per route, which is why the shell limit went 60→300 as a stopgap.",
"nextIdCounter": 302,
"lastSyncedCommit": "db548e722102dd359d00fcadd42b5226842666a4",
"lastSyncedAt": "2026-08-06T03:15:00Z",
"notesForNextSession": "T-0302 is in REVIEW as PR #87 against dev and is a COUPLED RELEASE with lords-regiment-dashboard:T-0311 — DEFAULT_CARD_IMAGE now names /assets/images/social-card.png, a file that only exists in the FRONTEND build. Deploy the web image first or ship both together.\n\nT-0301 is in REVIEW as PR #86, coupled to frontend PR 73. T-0297/T-0298 are merged into DEV (PR #84) but NOT on main.\n\n⚠️ NOTHING FROM THIS WORK IS IN PRODUCTION YET. main is the production branch; dev has to be promoted to main and the WEB image redeployed. The crawler routing now lives in the frontend's nginx.conf, so `deploy web <tag>` is what makes link previews start working — the API half has been correct and deployed since T-0293.\n\nVERIFY AFTER DEPLOY (the fast tell): a crawler-path response carrying `Vary: Accept-Encoding` + `Cache-Control: no-store` is the web nginx answering; the API sends `Vary: User-Agent` + `max-age=600`. As of the last check production still returns the former, i.e. the old build.\n\nTHE ROOT CAUSE nobody should re-diagnose: link previews were identical everywhere because production ran a Caddyfile predating even the T-0197 gallery unfurler. /api/seo/* was always correct. Cloudflare's certificate config is NOT involved — that was the first hypothesis and it was wrong (cf-cache-status was DYNAMIC throughout).\n\nTHE LESSON FROM THE REVIEW, worth keeping: the two bugs that mattered were both in code I had written careful comments about. /api/oembed declared `@Query('author') author: string | undefined` — Express does not honour that, `qs` yields string[] for a repeated key, and the global ValidationPipe coerces only Number and Boolean. That 500'd a @Public() route AND defeated a URL scheme check, because RegExp.test stringifies its argument. **Every other @Query typed `string` in this codebase has the same latent bug** and was NOT audited — that is the open regressionRisk on T-0301 and the most valuable thing to pick up next. Separately, describeEvents was changed without its seo-copy.ts twin, in the same PR whose comments warn three times about exactly that.\n\nSTILL NOT DONE. (a) Nothing pasted into a real Discord channel — the oEmbed author line is unit-tested only, and Discord caches an unfurl ~30min with no purge, so append a throwaway query string when re-testing. (b) The Caddyfile on the box is un-synced; it no longer blocks previews but still owns the www→apex 301, which nginx cannot do. (c) Search Console: the sitemap has never been submitted, and deploy/README.md is explicit that Authenticated Origin Pulls + TRUST_CF_CONNECTING_IP must land FIRST — until then every crawler shares one bucket per route, which is why the shell limit went 60→300 as a stopgap.",
"nextIdCounter": 303,
"connections": [],
"tasks": [
{
Expand Down Expand Up @@ -15917,6 +15917,65 @@
"note": "raised and fixed from the adversarial review; PR #86"
}
]
},
{
"id": "T-0302",
"title": "Point the crawler shell's default card at the crest, not the site banner",
"summary": "The owner pasted https://lordsofholdfast.com/ into Discord and got a frame of Holdfast gameplay under the regiment's name — banner.png, the default card image on every public URL that has nothing of its own to show. Replaced it with social-card.png: the regiment crest centred on --ink-900 at 1200x630, composed from regiment-logo.png. Wide-card size on purpose — the crest is square on its own and Discord demotes a square image to a thumbnail whatever twitter:card claims, so the ink field is what keeps the large embed the owner already had. The title and the tagline are unchanged; only the picture is. This half is DEFAULT_CARD_IMAGE and its two declared dimensions (853x480 -> 1200x630), which every shell-rendered page falls through to: the roster, an empty calendar, the generic card, and a member with neither an avatar nor a banner. The asset itself lives in the frontend build and is served from this origin, so the constant is a cross-repo reference and the deploy is coupled. The precedence above it is untouched — an owner-set hero or regiment banner still wins on /home, because the crest is the answer for a page nobody has spoken for, not an override of one.",
"status": "review",
"type": "fix",
"size": "S",
"sizeWeight": 1,
"area": "seo",
"feature": "crawler-shell",
"subfeature": "card-image",
"repo": "",
"paths": [
"src/seo/seo.service.ts",
"src/seo/seo.service.spec.ts"
],
"dependsOn": [
"lords-regiment-dashboard:T-0311"
],
"blocks": [],
"questions": [],
"testPlan": [
{
"desc": "the seo suite asserts the crest card for a member with no avatar and no banner, and for a missing regiment row",
"status": "passed"
},
{
"desc": "a crawler-UA fetch of /roster and /u/<slug> emits og:image = <site>/assets/images/social-card.png and that URL 200s",
"status": "pending"
},
{
"desc": "an event with its own bannerUrl still cards that banner, not the crest",
"status": "pending"
}
],
"regressionRisk": [
{
"desc": "DEFAULT_CARD_IMAGE names a file that ships in lords-regiment-dashboard, so this API cannot verify the URL resolves. Deployed ahead of the web image it emits a 404 card on every default-image page — the roster, an empty calendar, avatarless profiles. banner.png is deliberately still present in the frontend so the reverse order is safe.",
"status": "open",
"notedAt": "2026-08-06T03:15:00Z"
}
],
"skippedByClaude": [
"Did not change the /home precedence (presentation.heroBannerUrl -> bannerUrl -> default). An owner-set hero is a deliberate statement about that page's card; only the unclaimed pages change.",
"Did not reconcile the pre-existing disagreement between the shell (prefers presentation.heroBannerUrl) and the SPA landing page (uses cardBannerUrl only). It predates this task and is a separate fix."
],
"lastTouchedCommit": "",
"createdAt": "2026-08-06T03:15:00Z",
"updatedAt": "2026-08-06T03:15:00Z",
"history": [
{
"at": "2026-08-06T03:15:00Z",
"from": null,
"to": "review",
"by": "claude",
"note": "created and opened as a PR against dev, coupled to lords-regiment-dashboard:T-0311 (its PR #78)"
}
]
}
],
"history": [
Expand Down Expand Up @@ -15949,6 +16008,11 @@
"at": "2026-08-05T19:45:00Z",
"by": "claude",
"note": "T-0301 created from the adversarial review of #84; T-0297/T-0298 merged to dev"
},
{
"at": "2026-08-06T03:15:00Z",
"by": "claude",
"note": "T-0302 created (default crawler-shell card image -> composed crest card; coupled to frontend T-0311)"
}
]
}
6 changes: 3 additions & 3 deletions src/seo/seo.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,11 +338,11 @@ describe('SeoService — member-authored content on the profile shell (T-0216)',
expect(html).toContain('<meta name="twitter:card" content="summary_large_image" />');
});

it('still shows the site banner for a member with neither', async () => {
it('still shows the site crest card for a member with neither', async () => {
const html = await render(profile({ avatarUrl: null, bannerUrl: null }));

expect(html).toContain(
`<meta property="og:image" content="${SITE}/assets/images/banner.png" />`,
`<meta property="og:image" content="${SITE}/assets/images/social-card.png" />`,
);
});

Expand Down Expand Up @@ -426,7 +426,7 @@ describe('SeoService — member-authored content on the profile shell (T-0216)',

expect(html).toContain('<title>Lords Regiment</title>');
expect(html).toContain(
`<meta property="og:image" content="${SITE}/assets/images/banner.png" />`,
`<meta property="og:image" content="${SITE}/assets/images/social-card.png" />`,
);
});

Expand Down
29 changes: 24 additions & 5 deletions src/seo/seo.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,25 @@ const PROFILE_DESCRIPTION_LIMIT = 160;
* line of text — `twitter:card` degrades to `summary` when there is no image,
* and the frontend's `SeoService` applies the identical fallback so the two
* documents keep agreeing.
*
* THE CREST, NOT A SCREENSHOT (T-0302). This was `banner.png`, a frame of
* Holdfast gameplay. It filled the wide card, but a page with nothing of its own
* to show is exactly the page whose card has to identify the REGIMENT, and every
* one of them — the roster, an empty calendar, a member with no avatar — unfurled
* as the same anonymous battlefield. `social-card.png` is the crest centred on
* `--ink-900` at 1200x630, the wide card's canonical size, so the LAYOUT is
* unchanged and only the subject is. It is a composed asset rather than
* `regiment-logo.png` because the crest is square on its own and Discord demotes
* a square image to a thumbnail whatever the tag claims; the ink field is what
* buys the large card.
*
* ⚠️ Mirrored by `DEFAULT_IMAGE` in the frontend's `core/services/seo.service.ts`
* and by the static `og:image` in its `index.html`, and the file itself ships in
* that repo — so changing this is a coupled deploy in both directions.
*/
const DEFAULT_CARD_IMAGE = '/assets/images/banner.png';
const DEFAULT_CARD_WIDTH = 853;
const DEFAULT_CARD_HEIGHT = 480;
const DEFAULT_CARD_IMAGE = '/assets/images/social-card.png';
const DEFAULT_CARD_WIDTH = 1200;
const DEFAULT_CARD_HEIGHT = 630;

/**
* Renders the public pages as crawlable HTML (T-0215, widened in T-0293).
Expand Down Expand Up @@ -411,10 +426,14 @@ export class SeoService {
'Roster, events and gallery for a Holdfast: Nations at War regiment.';

// The admin-editable hero wins, then the regiment banner, then NULL — which
// `cardImageFor` turns into the shipped brand banner complete with its
// `cardImageFor` turns into the shipped crest card complete with its
// dimensions. Naming `DEFAULT_CARD_IMAGE` here instead would route it
// through the same branch as a CDN URL and silently drop the width/height
// an unfurler lays the card out from.
//
// Note the order survives T-0302: an owner who has uploaded a hero in
// Settings has said what THIS page's card should be, and the crest is the
// answer for a page nobody has spoken for — not an override of one.
const hero = profile?.presentation?.heroBannerUrl || profile?.bannerUrl || null;

const established = profile?.establishedYear ?? null;
Expand Down Expand Up @@ -852,7 +871,7 @@ export class SeoService {
// ── Card images ─────────────────────────────────────────────────────────────

/**
* The shipped brand banner, for a page with no image of its own.
* The shipped crest card, for a page with no image of its own.
*
* Without this the roster, an empty calendar and the generic card would all
* unfurl with `twitter:card: summary` and no picture — technically honest, and
Expand Down