Expose Astro's trailingSlash config to plugins via ctx.site#2122
Conversation
Plugins that build absolute URLs (sitemap, canonical, hreflang) need to match the site's URL convention, but ctx.site only carried name/url/locale, so plugins had to hardcode a trailing slash. That's wrong for a site configured with trailingSlash: 'never' -- notably a headless front-end that serves bare URLs, where the plugin's sitemap/canonical then advertise URLs the site doesn't serve. Capture astroConfig.trailingSlash at astro:config:setup, carry it on virtual:emdash/config, and surface it as ctx.site.trailingSlash -- the same path the i18n config already rides. Optional on SiteInfo; createSiteInfo defaults it to 'ignore' (Astro's default) so existing construction is unaffected.
🦋 Changeset detectedLatest commit: 9251b2e The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
All contributors have signed the CLA ✍️ ✅ |
github-actions
Bot
added
review/needs-review
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the right change for the stated problem: Astro-native sites need a way to expose trailingSlash to plugins that build absolute URLs, and threading it through virtual:emdash/config → runtime site info → ctx.site matches how i18n already travels. The implementation is small and additive, and the new test correctly covers createSiteInfo.
I found two issues that should be addressed before merge:
- Missing changeset. The PR changes the published
emdashpackage, so AGENTS.md/CONTRIBUTING.md require a changeset. - The sandbox payload is inadvertently widened. The PR description says the sandbox boundary is not widened, so sandboxed plugins should keep seeing the default
'ignore'. ButcreateSandboxRunnerOptionspasses the fullSiteInfoobject (now includingtrailingSlash) into the sandbox runner; the Cloudflare wrapper then JSON-stringifies that whole object into the generated worker code. That means out-of-process plugins do receive the realtrailingSlashvalue, contrary to the stated scope and to the unchangedSandboxOptions/WrapperOptionstypes.
I did not find any SQL, authorization, i18n/Tailwind, or runtime-security issues in this diff.
Findings
-
[needs fixing]
packages/core/src/astro/integration/index.ts:389This line changes the published
emdashpackage, but the PR does not include a changeset. AGENTS.md and CONTRIBUTING.md require a changeset for any published package change.Add a changeset such as
.changeset/expose-trailing-slash.md:--- "emdash": minor --- Exposes the host Astro project's `trailingSlash` config to plugins via `ctx.site.trailingSlash`, so plugins that build absolute URLs (sitemaps, canonical, hreflang) can match the site's routing policy.
-
[needs fixing]
packages/core/src/plugins/sandbox/runner-options.ts:14The PR description explicitly says the sandbox boundary is not widened here, so sandboxed plugins should continue seeing the default
'ignore'. However,createSiteInfo(siteInfo ?? {})now always emits atrailingSlashproperty, and that fullSiteInfoobject is passed into the sandbox runner. The Cloudflare wrapper thenJSON.stringifys the whole object into the generated worker code, so out-of-process plugins actually receive the host's realtrailingSlashvalue. This both contradicts the stated scope and silently changes the sandbox wire format without updatingSandboxOptions/WrapperOptions.To honor the out-of-scope note, build the sandbox
siteInfofrom onlyname/url/locale:export function createSandboxRunnerOptions( options: Omit<SandboxOptions, "siteInfo">, siteInfo?: SiteInfoOptions, ): SandboxOptions { const normalized = createSiteInfo(siteInfo ?? {}); return { ...options, siteInfo: { name: normalized.name, url: normalized.url, locale: normalized.locale }, }; }
|
I have read the CLA Document and I hereby sign the CLA |
Overlapping PRsThis PR modifies files that are also changed by other open PRs: This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
github-actions
Bot
added
cla: signed
review/awaiting-author
Review follow-up (emdash-cms#2122): createSiteInfo already flows the full SiteInfo — including trailingSlash — to sandboxed plugins via the runner options, matching the documented 'same normalized site context used by trusted plugin hooks and routes' parity. Make that explicit by adding trailingSlash to the sandbox site-shape types (core + the workerd/cloudflare wrappers) so the wire format is typed, not silently widened. Add the changeset for the published emdash change.
github-actions
Bot
added
area/cloudflare
size/M
review/needs-rereview
There was a problem hiding this comment.
This is the right change for the stated problem: Astro-native sites need a way to expose trailingSlash to plugins that build absolute URLs, and threading it through virtual:emdash/config → runtime site info → ctx.site matches how i18n already travels. It is additive, backward-compatible (defaults to Astro's 'ignore'), and the implementation is small.
Re-review status: the two issues from the previous review are addressed.
- Changeset added —
.changeset/expose-trailing-slash.mdis now present. - Sandbox scope clarified — the PR description and changeset now explicitly state that sandboxed plugins receive
trailingSlash, and the core / Cloudflare / workerd wrapper site payloads have been typed consistently. The earlier contradiction is resolved.
What I checked: read the diff, the changed source files, the changeset, and the AGENTS.md/CONTRIBUTING.md changeset requirements. I traced the data flow from astro:config:setup → serialized config → readSiteInfo() → createSiteInfo() → trusted plugin context factory and sandbox-runner options → Cloudflare/Workerd wrapper generation. I did not find SQL, authorization, i18n/Tailwind, or runtime-security issues.
Headline concern: the changeset only covers the emdash package, but this PR also changes the public behavior of two other published packages — @emdash-cms/cloudflare and @emdash-cms/sandbox-workerd — because their sandbox runners/wrappers now forward trailingSlash into generated plugin wrappers. CONTRIBUTING.md requires a changeset for every published package whose behavior changes. That should be fixed before merge.
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| "emdash": minor | |||
There was a problem hiding this comment.
[needs fixing] This changeset only lists emdash, but the PR also changes two other published packages: @emdash-cms/cloudflare and @emdash-cms/sandbox-workerd. Their sandbox runners/wrappers now forward trailingSlash into generated worker code (e.g. WrapperOptions.site and the runner siteInfo fields are widened), which is a real public-package behavior change.
CONTRIBUTING.md says a changeset is required for every published package whose behavior changes; otherwise the change won't trigger a release.
Add the other affected packages to this changeset so the coordinated release is captured:
| "emdash": minor | |
| --- | |
| "emdash": minor | |
| "@emdash-cms/cloudflare": minor | |
| "@emdash-cms/sandbox-workerd": minor | |
| --- | |
| Expose the host Astro project's `trailingSlash` config to plugins via `ctx.site.trailingSlash`, so plugins that build absolute URLs (sitemaps, canonical, hreflang) can match the site's routing policy. Available to in-process and sandboxed plugins alike. |
The sandbox runners/wrappers forward trailingSlash into generated worker code, so both published packages change behavior and must be released alongside emdash core.
There was a problem hiding this comment.
This is the right change for the stated problem: exposing Astro’s trailingSlash to plugins through the existing virtual:emdash/config → runtime site info → ctx.site pipeline is additive, backward-compatible (it defaults to Astro’s 'ignore'), and matches how i18n already travels. It’s now also correctly typed through the sandbox runners/wrappers in all three affected packages.
Re-review status: the two issues from the previous review are resolved.
- Changeset added —
.changeset/expose-trailing-slash.mdnow includesemdash,@emdash-cms/cloudflare, and@emdash-cms/sandbox-workerd. - Sandbox scope clarified — the PR description and changeset explicitly state that sandboxed plugins receive
trailingSlash, and the core / Cloudflare / workerd wrapper site payloads have been typed consistently.
New issue: I found one test regression caused by the new field. createSiteInfo now always populates trailingSlash (defaulting to "ignore"), but packages/core/tests/unit/plugins/sandbox-runner-options.test.ts uses strict toEqual assertions on the normalized siteInfo object without the new field, so those tests will fail once the change runs. The fix is to add trailingSlash: "ignore" to the expected objects.
What I checked: diff, all changed source files, the changeset, createSandboxRunnerOptions, PluginContextFactory, the Cloudflare/workerd wrapper generators, and existing test expectations around siteInfo. I traced the data flow from astro:config:setup → serialized config → readSiteInfo() → createSiteInfo() → trusted plugin context factory and sandbox-runner options → wrapper JSON injection. I did not run the test suite, so failure claims are static (exact toEqual mismatches).
Findings
-
[needs fixing]
packages/core/tests/unit/plugins/sandbox-runner-options.test.ts:17-25createSiteInfonow always addstrailingSlash(defaulting to"ignore"), so the normalizedsiteInforeturned bycreateSandboxRunnerOptionswill include it. This stricttoEqualassertion will fail.expect(options).toEqual({ db, siteInfo: { name: "Example Site", url: "https://example.com", locale: "nl", trailingSlash: "ignore", }, }); -
[needs fixing]
packages/core/tests/unit/plugins/sandbox-runner-options.test.ts:36The default
siteInfofromcreateSiteInfonow includestrailingSlash: "ignore", so this exact match will fail.expect(options.siteInfo).toEqual({ name: "", url: "", locale: "en", trailingSlash: "ignore" }); -
[suggestion]
packages/core/tests/integration/runtime/plugin-route-site-info.test.ts:82-86Since
trailingSlashnow flows into trusted plugin routes viactx.site, consider asserting it here to cover the end-to-end site-context propagation rather than only the unit-levelcreateSiteInfotests. ThistoMatchObjectassertion is a subset, so adding the field is safe.site: { name: "Example Site", url: "https://example.com", locale: "nl", trailingSlash: "ignore", },
github-actions
Bot
added
review/awaiting-author
createSiteInfo now always populates trailingSlash (default "ignore"), so the strict siteInfo expectations must include it: sandbox-runner-options (toEqual x2), create.test.ts (objectContaining on the sandbox-runner siteInfo), and the plugin-route-site-info integration test (end-to-end). Verified by running the full core suite locally (4753 passing).
547d82d to
675f531
Compare
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
There was a problem hiding this comment.
This is the right change for the stated problem: exposing Astro's trailingSlash routing policy to plugins through the existing virtual:emdash/config → runtime site info → ctx.site pipeline is additive, backward-compatible (it defaults to Astro's 'ignore'), and mirrors how i18n already travels. It's now also correctly typed through the sandbox runners/wrappers in all three affected packages.
Re-review status: the finding from the previous review is resolved.
packages/core/tests/unit/plugins/sandbox-runner-options.test.tsnow expectstrailingSlash: "ignore"in bothtoEqualassertions, andpackages/core/tests/integration/runtime/plugin-route-site-info.test.tsasserts the field in the end-to-end site-context output. No other strict site-info assertions remain in the test files I traced.
What I checked: the unified diff, all changed source files (astro/integration/index.ts, emdash-runtime.ts, plugins/context.ts, plugins/types.ts, plugins/sandbox/types.ts, virtual-modules.d.ts, plus the Cloudflare and workerd sandbox runner/wrapper modules), the changeset, and the updated tests. I traced the data flow from astro:config:setup → serialized config → readSiteInfo() → createSiteInfo() → trusted plugin context factory and sandbox-runner options → Cloudflare/workerd wrapper JSON injection. The runtime, type declarations, wrapper payloads, and tests are all consistent.
I did not run the test suite, lint, or typecheck, so CI remains the source of truth for those. I also could not verify from the repo that Discussion #2124 is approved (no GitHub access from this environment).
No remaining blocking or suggestion findings.
…ms#2122) * Expose Astro's trailingSlash config to plugins via ctx.site Plugins that build absolute URLs (sitemap, canonical, hreflang) need to match the site's URL convention, but ctx.site only carried name/url/locale, so plugins had to hardcode a trailing slash. That's wrong for a site configured with trailingSlash: 'never' -- notably a headless front-end that serves bare URLs, where the plugin's sitemap/canonical then advertise URLs the site doesn't serve. Capture astroConfig.trailingSlash at astro:config:setup, carry it on virtual:emdash/config, and surface it as ctx.site.trailingSlash -- the same path the i18n config already rides. Optional on SiteInfo; createSiteInfo defaults it to 'ignore' (Astro's default) so existing construction is unaffected. * Type trailingSlash on the sandbox site payload + add changeset Review follow-up (emdash-cms#2122): createSiteInfo already flows the full SiteInfo — including trailingSlash — to sandboxed plugins via the runner options, matching the documented 'same normalized site context used by trusted plugin hooks and routes' parity. Make that explicit by adding trailingSlash to the sandbox site-shape types (core + the workerd/cloudflare wrappers) so the wire format is typed, not silently widened. Add the changeset for the published emdash change. * changeset: declare @emdash-cms/cloudflare + @emdash-cms/sandbox-workerd The sandbox runners/wrappers forward trailingSlash into generated worker code, so both published packages change behavior and must be released alongside emdash core. * test: expect trailingSlash in normalized siteInfo createSiteInfo now always populates trailingSlash (default "ignore"), so the strict siteInfo expectations must include it: sandbox-runner-options (toEqual x2), create.test.ts (objectContaining on the sandbox-runner siteInfo), and the plugin-route-site-info integration test (end-to-end). Verified by running the full core suite locally (4753 passing). * style: format --------- Co-authored-by: Matt Kane <mkane@cloudflare.com> Co-authored-by: emdashbot[bot] <emdashbot[bot]@users.noreply.github.com>
3 participants
What does this PR do?
Exposes the host Astro project's
trailingSlashconfig to plugins viactx.site.trailingSlash, so plugins that build absolute URLs (sitemaps, canonical, hreflang) can match the site's routing policy. Without it, plugins hardcode a trailing slash — correct for Astro's default, but wrong for a site configuredtrailingSlash: 'never', notably a headless front-end that serves bare URLs, where a plugin's sitemap/canonical then advertise URLs the site doesn't serve.It rides the same path the
i18nconfig already uses:astro:config:setupcapturesastroConfig.trailingSlash→virtual:emdash/config→readSiteInfo()→SiteInfo. It's optional onSiteInfo, andcreateSiteInfodefaults it to'ignore'(Astro's default), so existing construction is unaffected. Sandboxed plugins receive it too: the sandbox already forwards the full normalizedSiteInfo("the same normalized site context used by trusted plugin hooks and routes"), and this PR makes that explicit by typingtrailingSlashon the sandbox/wrapper site payloads (core + workerd + cloudflare).Motivating consumer:
jdevalk/emdash-plugin-seo#2(itsschema/mapsitemap feed).Type of change
Discussion: #2124
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runAI-generated code disclosure
Screenshots / test output
N/A