diff --git a/.github/workflows/_verify.yaml b/.github/workflows/_verify.yaml index 3ef8632..7331682 100644 --- a/.github/workflows/_verify.yaml +++ b/.github/workflows/_verify.yaml @@ -6,7 +6,7 @@ # - pull-request.yaml, where it gates every pull request (including forks; # see the security notes there - this workflow uses no secrets and only # ever needs `contents: read`) -# - publish-ghcr-platform.yaml, where it gates image builds on `next` +# - publish-ghcr-platform.yaml, where it gates image publication on `next` # pushes, which land directly without a pull request name: Verify diff --git a/.github/workflows/publish-ghcr-platform.yaml b/.github/workflows/publish-ghcr-platform.yaml index 773f249..4c70153 100644 --- a/.github/workflows/publish-ghcr-platform.yaml +++ b/.github/workflows/publish-ghcr-platform.yaml @@ -71,11 +71,14 @@ jobs: fi # @note `next` receives direct pushes, so nothing has vetted the code yet - - # run the quality gate before spending build minutes on it. It runs on - # every next push, not just build-relevant ones, so the promotion pull - # request always finds a verify check on the head SHA. `main` only moves - # through pull requests that already passed the same gate, so the job is - # skipped there and the build starts immediately. + # the quality gate runs alongside the image build and blocks publication, + # not the build itself: build only pushes untagged per-architecture digests, + # so nothing a consumer can reach exists until publish tags them. Running + # both in parallel halves the wall-clock at the cost of build minutes spent + # on a red gate. It runs on every next push, not just build-relevant ones, + # so the promotion pull request always finds a verify check on the head + # SHA. `main` only moves through pull requests that already passed the same + # gate, so the job is skipped there. verify: if: >- (github.repository == 'chatbotkit/platform' || startsWith(github.repository, 'chatbotkit/platform-')) && @@ -86,14 +89,12 @@ jobs: build: needs: - changes - - verify - # @note !cancelled() + accepting the skipped verify keeps main builds - # running while a failed gate on next still blocks them + # @note deliberately not gated on verify: the two run in parallel and the + # gate is applied at publish, the only job that makes an image reachable if: >- !cancelled() && (github.repository == 'chatbotkit/platform' || startsWith(github.repository, 'chatbotkit/platform-')) && needs.changes.outputs.build == 'true' && - (needs.verify.result == 'success' || needs.verify.result == 'skipped') && github.actor != 'github-actions[bot]' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/next') name: Build ${{ matrix.flavor.name }} (${{ matrix.architecture.name }}) @@ -278,16 +279,22 @@ jobs: retention-days: 1 publish: - # @note !cancelled() is required: the implicit success() looks at the - # whole needs chain, and verify is skipped on main + # @note this is where the quality gate bites: a failed verify on next + # leaves the build's untagged digests unreachable in the registry and + # publishes nothing. !cancelled() + accepting the skipped verify is + # required: the implicit success() looks at the whole needs chain, and + # verify is skipped on main if: >- !cancelled() && (github.repository == 'chatbotkit/platform' || startsWith(github.repository, 'chatbotkit/platform-')) && needs.build.result == 'success' && + (needs.verify.result == 'success' || needs.verify.result == 'skipped') && github.actor != 'github-actions[bot]' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/next') name: Publish ${{ matrix.flavor.name }} - needs: build + needs: + - build + - verify runs-on: ${{ matrix.flavor.runner }} strategy: fail-fast: false diff --git a/docker/Dockerfile b/docker/Dockerfile index 8753c3e..1c98176 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -173,10 +173,24 @@ RUN pnpm --filter @chatbotkit-dev/db deploy --legacy /initializer FROM node:24.20.0-bookworm-slim AS initializer +# @note Prisma picks its schema-engine binary by the libssl it detects. The +# deployer stage has openssl (a ca-certificates dependency) and so bundles the +# openssl-3.0.x engine; without libssl here the CLI assumes 1.1.x and tries to +# download it at first boot, which fails on hosts without outbound DNS +RUN apt-get update \ + && apt-get install -y --no-install-recommends openssl \ + && rm -rf /var/lib/apt/lists/* + WORKDIR /app COPY --from=initializer-deployer /initializer ./ +# @note the engine must come from the image, never from the network: resolve +# it with the download mirror unreachable so a bundled/detected engine mismatch +# fails the build here rather than at first boot. Provider-agnostic on purpose +# - the db module behind this stage need not be SQLite +RUN PRISMA_ENGINES_MIRROR=http://127.0.0.1:9 npx prisma --version + # @note the Garage provisioning script rides along so the distribution stack # (docker/distro/*) can run it from this image instead of bind-mounting the # checkout - a published Compose OCI artifact cannot carry bind mounts diff --git a/platform/components/Theme.jsx b/platform/components/Theme.jsx index 9111821..6c2e9ab 100644 --- a/platform/components/Theme.jsx +++ b/platform/components/Theme.jsx @@ -21,10 +21,9 @@ export default function Theme({ themes = availableThemes, theme, children }) { diff --git a/platform/hooks/useTheme.ts b/platform/hooks/useTheme.ts index 5f1f4f9..2ff3d17 100644 --- a/platform/hooks/useTheme.ts +++ b/platform/hooks/useTheme.ts @@ -1,12 +1,16 @@ import { useTheme as useThemeBase } from 'next-themes' export default function useTheme() { - const { theme, forcedTheme, ...rest } = useThemeBase() + const { theme, forcedTheme, resolvedTheme, ...rest } = useThemeBase() return { ...rest, - theme: forcedTheme || theme, + // @note resolvedTheme maps "system" to light or dark; consumers compare + // against those two values only + theme: forcedTheme || resolvedTheme || theme, + + resolvedTheme, forcedTheme, } diff --git a/platform/hooks/useTheme.utest.js b/platform/hooks/useTheme.utest.js index 084b347..870ea7e 100644 --- a/platform/hooks/useTheme.utest.js +++ b/platform/hooks/useTheme.utest.js @@ -91,7 +91,7 @@ describe('useTheme', () => { expect(result.current.forcedTheme).toBeNull() }) - it('should handle system theme correctly', () => { + it('should resolve system theme to the system preference', () => { useThemeBase.mockReturnValue({ theme: 'system', forcedTheme: undefined, @@ -101,9 +101,35 @@ describe('useTheme', () => { const { result } = renderHook(() => useTheme()) - expect(result.current.theme).toBe('system') + expect(result.current.theme).toBe('dark') expect(result.current.resolvedTheme).toBe('dark') }) + + it('should prefer forcedTheme over resolvedTheme', () => { + useThemeBase.mockReturnValue({ + theme: 'system', + forcedTheme: 'light', + setTheme: jest.fn(), + resolvedTheme: 'dark', + }) + + const { result } = renderHook(() => useTheme()) + + expect(result.current.theme).toBe('light') + }) + + it('should fall back to theme before hydration when resolvedTheme is undefined', () => { + useThemeBase.mockReturnValue({ + theme: 'dark', + forcedTheme: undefined, + setTheme: jest.fn(), + resolvedTheme: undefined, + }) + + const { result } = renderHook(() => useTheme()) + + expect(result.current.theme).toBe('dark') + }) }) describe('edge cases', () => { diff --git a/platform/lib/defer.ts b/platform/lib/defer.ts index 2e98e97..61e4139 100644 --- a/platform/lib/defer.ts +++ b/platform/lib/defer.ts @@ -22,7 +22,7 @@ import { AsyncLocalStorage } from 'async_hooks' * in-tree so a deployment that never runs on Vercel does not carry the * vendor package for one optional chained call. */ -function waitUntil(promise: Promise): void { +function getWaitUntil(): ((p: Promise) => void) | undefined { const context = ( globalThis as { [key: symbol]: { @@ -31,7 +31,11 @@ function waitUntil(promise: Promise): void { } )[Symbol.for('@vercel/request-context')] - context?.get?.()?.waitUntil?.(promise) + return context?.get?.()?.waitUntil +} + +function waitUntil(promise: Promise): void { + getWaitUntil()?.(promise) } interface Store { @@ -183,7 +187,6 @@ export async function defer( if (store) { // @note add the promise to the list of deferred promises - // eslint-disable-next-line @typescript-eslint/no-floating-promises store.deferred = store.deferred || [] store.deferred.push(promise.catch(captureError)) @@ -194,4 +197,38 @@ export async function defer( } } +/** + * Runs work after the response is sent rather than before it. `defer` holds a + * plain (non-streaming) response until its promises settle - see `beforeClose` + * - so a webhook that must ack within a deadline cannot put its publish there. + * When the runtime cannot keep the function alive past the response the work + * is awaited in place, exactly as `defer` would do. + */ +export async function deferPastResponse( + fn: () => Promise +): Promise { + const keepAlive = getWaitUntil() + + if (!keepAlive) { + await defer(async () => { + await fn() + }) + + return + } + + // @note a fresh store, so anything `fn` itself defers settles on this promise + // and not on the request store, whose `beforeClose` would hold the response + + keepAlive( + als + .run({ deferred: [] }, async () => { + await fn() + + await awaitDeferred() + }) + .catch(captureError) + ) +} + export default defer diff --git a/platform/lib/defer.utest.js b/platform/lib/defer.utest.js index d3c14a8..96d8aa7 100644 --- a/platform/lib/defer.utest.js +++ b/platform/lib/defer.utest.js @@ -1,5 +1,10 @@ import debug, { createSpan, warn } from '@/lib/debug' -import { awaitDeferred, defer, runInDeferred } from '@/lib/defer' +import { + awaitDeferred, + defer, + deferPastResponse, + runInDeferred, +} from '@/lib/defer' import { captureError, captureException } from '@/lib/error' // @note the runtime hook `defer` uses to keep work alive past the response: @@ -779,3 +784,85 @@ describe('defer module', () => { }) }) }) + +describe('deferPastResponse', () => { + beforeEach(() => { + jest.clearAllMocks() + }) + + it('hands the work to waitUntil and resolves before the work settles', async () => { + let finish + const work = jest.fn( + () => + new Promise((resolve) => { + finish = resolve + }) + ) + + await deferPastResponse(work) + + expect(work).toHaveBeenCalledTimes(1) + expect(waitUntil).toHaveBeenCalledTimes(1) + + finish() + + await waitUntil.mock.calls[0][0] + }) + + it('does not hold a runInDeferred response for work the handed function defers', async () => { + let finish + const inner = new Promise((resolve) => { + finish = resolve + }) + + const wrapped = runInDeferred(async () => { + await deferPastResponse(async () => { + await defer(inner) + }) + + return new Response('ok', { status: 200 }) + }) + + // @note would hang if `inner` had landed on the request store + const result = await wrapped() + + expect(result.status).toBe(200) + + finish() + + await Promise.all(waitUntil.mock.calls.map(([promise]) => promise)) + }) + + it('captures a failure of the handed work', async () => { + const error = new Error('publish failed') + + await deferPastResponse(() => Promise.reject(error)) + + await waitUntil.mock.calls[0][0] + + expect(captureError).toHaveBeenCalledWith(error) + }) + + it('awaits the work in place when the runtime has no waitUntil', async () => { + const context = global[Symbol.for('@vercel/request-context')] + + delete global[Symbol.for('@vercel/request-context')] + + try { + const order = [] + + await deferPastResponse(async () => { + await new Promise((resolve) => setTimeout(resolve, 5)) + + order.push('work') + }) + + order.push('returned') + + expect(order).toEqual(['work', 'returned']) + expect(waitUntil).not.toHaveBeenCalled() + } finally { + global[Symbol.for('@vercel/request-context')] = context + } + }) +}) diff --git a/platform/lib/host.ts b/platform/lib/host.ts index 3708c4f..31b33e8 100644 --- a/platform/lib/host.ts +++ b/platform/lib/host.ts @@ -2,6 +2,7 @@ import { apiHostname, apiUrl, siteHostname, + siteUrl, staticHostname, staticUrl, widgetHostname, @@ -12,10 +13,12 @@ import { getContextAPIHost, getContextFrontendHost, getContextRequestHost, + getContextRequestProtocol, getContextStaticHost, getContextWidgetHost, } from '@/lib/context.store' import { isDevelopment, isTest } from '@/lib/env' +import { isLocalhost } from '@/lib/localhost' import { z } from 'zod' @@ -43,6 +46,30 @@ const env = z _ITEST_CHATBOTKIT_BASE_URL: process.env._ITEST_CHATBOTKIT_BASE_URL, }) +/** + * Builds a URL on a host, picking the scheme the host actually answers on. + * Loopback and `*.localhost` hosts are plain http (the community stack has + * no TLS), the site host follows SITE_URL, the request host follows the + * request scheme. Anything else is https. + */ +function buildHostURL(path: string, base: string): string { + const url = new URL(path, base) + + if ( + isLocalhost(url.hostname) || + url.hostname === '[::1]' || + url.hostname.endsWith('.localhost') + ) { + url.protocol = 'http:' + } else if (url.hostname === siteHostname) { + url.protocol = new URL(siteUrl).protocol + } else if (url.host === getContextRequestHost()) { + url.protocol = `${getContextRequestProtocol() || 'https'}:` + } + + return url.toString() +} + /** * Gets the local host based on the environment. When in development, it will * use the NGROK_HOST or LOCAL_HOST environment variables. Otherwise, it will @@ -79,13 +106,7 @@ export function getLocalHostURL( path: string = '/', host: string = getLocalHost() ): string { - const url = new URL(path, `https://${host}`) - - if (url.hostname === 'localhost') { - url.protocol = 'http:' - } - - return url.toString() + return buildHostURL(path, `https://${host}`) } /** @@ -128,13 +149,7 @@ export function getExternalHostURL( path: string = '/', host: string = getExternalHost() ): string { - const url = new URL(path, `https://${host}`) - - if (url.hostname === 'localhost') { - url.protocol = 'http:' - } - - return url.toString() + return buildHostURL(path, `https://${host}`) } /** @@ -155,13 +170,7 @@ export function getExternalFrontendHostURL( path: string = '/', host: string = getExternalFrontendHost() ): string { - const url = new URL(path, `https://${host}`) - - if (url.hostname === 'localhost') { - url.protocol = 'http:' - } - - return url.toString() + return buildHostURL(path, `https://${host}`) } /** @@ -223,13 +232,7 @@ export function getLocalAPIHostURL( path = `/api${path.startsWith('/') ? '' : '/'}${path}` } - const url = new URL(path, `https://${host}`) - - if (url.hostname === 'localhost') { - url.protocol = 'http:' - } - - return url.toString() + return buildHostURL(path, `https://${host}`) } /** @@ -290,11 +293,5 @@ export function getExternalAPIHostURL( path = `/api${path.startsWith('/') ? '' : '/'}${path}` } - const url = new URL(path, host === apiHostname ? apiUrl : `https://${host}`) - - if (url.hostname === 'localhost') { - url.protocol = 'http:' - } - - return url.toString() + return buildHostURL(path, host === apiHostname ? apiUrl : `https://${host}`) } diff --git a/platform/lib/host.utest.js b/platform/lib/host.utest.js index 63fb0b9..d7a529b 100644 --- a/platform/lib/host.utest.js +++ b/platform/lib/host.utest.js @@ -164,6 +164,7 @@ function loadHostScenario({ targetEnv = 'production', testSiteUrl = siteUrl, requestHost = null, + requestProtocol = null, frontendHost = null, contextAPIHost, contextStaticHost, @@ -204,6 +205,7 @@ function loadHostScenario({ ...jest.requireActual('@/lib/context.store'), getContextFrontendHost: jest.fn(() => frontendHost), getContextRequestHost: jest.fn(() => requestHost), + getContextRequestProtocol: jest.fn(() => requestProtocol), getContextAPIHost: jest.fn(() => contextAPIHost), getContextStaticHost: jest.fn(() => contextStaticHost), getContextWidgetHost: jest.fn(() => contextWidgetHost), @@ -336,6 +338,84 @@ describe('basic URL construction', () => { 'http://localhost:3000/api/v1/test' ) }) + + // @note the community stack serves plain http on loopback and *.localhost + // hosts; dialling them over https fails the TLS handshake + + it.each(['127.0.0.1:3000', '[::1]:3000', 'cbk.localhost:3000'])( + 'switches to http for the loopback request host %s', + (requestHost) => { + const host = loadHostScenario({ + testSiteUrl: 'https://platform.example.com', + requestHost, + }) + + expect(host.getLocalHostURL()).toBe(`http://${requestHost}/`) + expect(host.getExternalHostURL('/api/test')).toBe( + `http://${requestHost}/api/test` + ) + expect(host.getExternalFrontendHostURL('/dashboard')).toBe( + `http://${requestHost}/dashboard` + ) + expect(host.getLocalAPIHostURL('/v1/graphql')).toBe( + `http://${requestHost}/api/v1/graphql` + ) + expect(host.getExternalAPIHostURL('/v1/graphql')).toBe( + `http://${requestHost}/api/v1/graphql` + ) + } + ) + + it('follows the site scheme for the site host', () => { + const host = loadHostScenario({ + testSiteUrl: 'http://platform.internal', + }) + + expect(host.getLocalAPIHostURL('/v1/graphql')).toBe( + 'http://platform.internal/api/v1/graphql' + ) + expect(host.getExternalAPIHostURL('/v1/graphql')).toBe( + 'http://platform.internal/api/v1/graphql' + ) + }) + + it('follows the request scheme for a plain http request host', () => { + const host = loadHostScenario({ + testSiteUrl: 'https://platform.example.com', + requestHost: 'customer.example.org', + requestProtocol: 'http', + }) + + expect(host.getLocalAPIHostURL('/v1/graphql')).toBe( + 'http://customer.example.org/api/v1/graphql' + ) + expect(host.getExternalHostURL('/docs')).toBe( + 'http://customer.example.org/docs' + ) + }) + + it('keeps an https site host on https whatever the request scheme', () => { + const host = loadHostScenario({ + testSiteUrl: 'https://platform.example.com', + requestHost: 'platform.example.com', + requestProtocol: 'http', + }) + + expect(host.getLocalAPIHostURL('/v1/graphql')).toBe( + 'https://platform.example.com/api/v1/graphql' + ) + }) + + it('defaults to https for a request host without a request scheme', () => { + const host = loadHostScenario({ + testSiteUrl: 'https://platform.example.com', + requestHost: 'customer.example.org', + }) + + expect(host.getLocalAPIHostURL('/v1/graphql')).toBe( + 'https://customer.example.org/api/v1/graphql' + ) + }) }) describe('external static host', () => { diff --git a/platform/next.config.d/bundling.config.js b/platform/next.config.d/bundling.config.js index 1b2c1bb..99c1cbf 100644 --- a/platform/next.config.d/bundling.config.js +++ b/platform/next.config.d/bundling.config.js @@ -61,6 +61,15 @@ export default { webpack(config, options) { if (options.isServer && options.nextRuntime !== 'edge') { config.externals.push({ 'better-sqlite3': 'commonjs better-sqlite3' }) + // @note ws probes its optional native addons inside a try/catch and + // falls back to pure JS only when the require throws. Webpack replaces + // the uninstalled packages with empty modules instead, so ws wires + // `{}.unmask` / `{}(buf)` and every masked frame over 32 bytes (audio + // streaming) crashes the process. Externalizing keeps the require real. + config.externals.push({ + bufferutil: 'commonjs bufferutil', + 'utf-8-validate': 'commonjs utf-8-validate', + }) // @note no mirror for @rivet-dev/agentos-core: it is ESM-only, so the // list above externalizes it as an import() the sandbox module awaits // lazily - allowed by name in scripts/verify-bundle-modules.js diff --git a/platform/next.config.d/bundling.config.utest.js b/platform/next.config.d/bundling.config.utest.js new file mode 100644 index 0000000..e4e3ceb --- /dev/null +++ b/platform/next.config.d/bundling.config.utest.js @@ -0,0 +1,34 @@ +/* eslint-disable @typescript-eslint/no-require-imports */ + +const config = require('./bundling.config').default + +function compile(options) { + return config.webpack({ externals: [] }, options) +} + +function externalFor(externals, request) { + return externals.find((entry) => entry?.[request])?.[request] +} + +describe('bundling.config', () => { + it('externalizes the optional ws native addons on the node server build', () => { + const { externals } = compile({ isServer: true, nextRuntime: 'nodejs' }) + + // @note the request must reach Node's require so it throws when the + // addon is absent and ws falls back to JS; an empty webpack module + // would satisfy the require and leave ws calling undefined functions + expect(externalFor(externals, 'bufferutil')).toBe('commonjs bufferutil') + expect(externalFor(externals, 'utf-8-validate')).toBe( + 'commonjs utf-8-validate' + ) + }) + + it.each([ + ['client', { isServer: false }], + ['edge', { isServer: true, nextRuntime: 'edge' }], + ])('leaves the %s build untouched', (_, options) => { + const { externals } = compile(options) + + expect(externals).toEqual([]) + }) +})