From d334f444ac463934581c283ec0aebaf8fa9c7bc2 Mon Sep 17 00:00:00 2001 From: rory Date: Thu, 22 Oct 2026 13:46:35 -0700 Subject: [PATCH 1/9] chore: migrate the PR-testing Worker to App --- .github/workflows/cloudflareWorkerTests.yml | 1 + .github/workflows/deployPRTestingWorker.yml | 69 ++++++ web/cloudflare-worker/README.md | 28 +++ web/cloudflare-worker/package-lock.json | 1 + web/cloudflare-worker/package.json | 1 + web/cloudflare-worker/src/csp.ts | 13 +- web/cloudflare-worker/src/index.ts | 19 +- web/cloudflare-worker/src/prTesting.ts | 45 ++++ web/cloudflare-worker/src/security.ts | 22 ++ web/cloudflare-worker/tests/prTesting.test.ts | 216 ++++++++++++++++++ .../wrangler.pr-testing.jsonc | 12 + 11 files changed, 408 insertions(+), 19 deletions(-) create mode 100644 .github/workflows/deployPRTestingWorker.yml create mode 100644 web/cloudflare-worker/src/prTesting.ts create mode 100644 web/cloudflare-worker/src/security.ts create mode 100644 web/cloudflare-worker/tests/prTesting.test.ts create mode 100644 web/cloudflare-worker/wrangler.pr-testing.jsonc diff --git a/.github/workflows/cloudflareWorkerTests.yml b/.github/workflows/cloudflareWorkerTests.yml index aff7d9d745ed..41718e588f1c 100644 --- a/.github/workflows/cloudflareWorkerTests.yml +++ b/.github/workflows/cloudflareWorkerTests.yml @@ -10,6 +10,7 @@ on: - 'web/index.html' - '.nvmrc' - '.github/workflows/cloudflareWorkerTests.yml' + - '.github/workflows/deployPRTestingWorker.yml' concurrency: group: ${{ github.ref }}-cloudflare-worker-tests diff --git a/.github/workflows/deployPRTestingWorker.yml b/.github/workflows/deployPRTestingWorker.yml new file mode 100644 index 000000000000..d7350ce98ef4 --- /dev/null +++ b/.github/workflows/deployPRTestingWorker.yml @@ -0,0 +1,69 @@ +name: Deploy PR-testing Cloudflare Worker + +on: + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: deploy-pr-testing-worker + cancel-in-progress: false + +jobs: + validateActor: + if: ${{ github.ref == 'refs/heads/main' }} + runs-on: blacksmith-2vcpu-ubuntu-2404 + outputs: + IS_DEPLOYER: ${{ steps.isUserDeployer.outputs.IS_DEPLOYER }} + steps: + - id: isUserDeployer + run: | + if gh api "/orgs/Expensify/teams/web-deployers/memberships/$GITHUB_ACTOR" --silent; then + echo "IS_DEPLOYER=true" >> "$GITHUB_OUTPUT" + elif gh api "/orgs/Expensify/teams/keyholders/memberships/$GITHUB_ACTOR" --silent; then + echo "IS_DEPLOYER=true" >> "$GITHUB_OUTPUT" + elif gh api "/orgs/Expensify/teams/mobile-deployers/memberships/$GITHUB_ACTOR" --silent; then + echo "IS_DEPLOYER=true" >> "$GITHUB_OUTPUT" + else + echo "IS_DEPLOYER=false" >> "$GITHUB_OUTPUT" + exit 1 + fi + env: + GITHUB_TOKEN: ${{ secrets.OS_BOTIFY_COMMIT_TOKEN }} + + deploy: + needs: validateActor + if: ${{ needs.validateActor.outputs.IS_DEPLOYER == 'true' }} + runs-on: blacksmith-2vcpu-ubuntu-2404 + defaults: + run: + working-directory: web/cloudflare-worker + env: + WRANGLER_SEND_METRICS: false + steps: + - name: Checkout + uses: useblacksmith/checkout@0647fdbab2614a5eb86d2971070e325060d91056 # v1.7.0 + + - name: Setup Node + # v6.1.0 + uses: actions/setup-node@395ad3262231945c25e8478fd5baf05154b1d79f + with: + node-version-file: '.nvmrc' + cache: npm + cache-dependency-path: web/cloudflare-worker/package-lock.json + + - name: Install dependencies + run: npm ci + + - name: Type check + run: npm run typecheck + + - name: Run tests + run: npm test + + - name: Deploy the existing PR-testing Worker + run: npx wrangler deploy --config wrangler.pr-testing.jsonc + env: + CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_WORKERS_DEPLOY_TOKEN }} + CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} diff --git a/web/cloudflare-worker/README.md b/web/cloudflare-worker/README.md index ed81dfe0e73f..3f17dcf20e86 100644 --- a/web/cloudflare-worker/README.md +++ b/web/cloudflare-worker/README.md @@ -140,3 +140,31 @@ npx wrangler deploy --dry-run --env test-staging ``` Never commit credentials, account IDs or `.dev.vars` files here. This is a public repository. + +## PR-testing Worker + +[`src/prTesting.ts`](src/prTesting.ts) serves `https://.pr-testing.expensify.com` from the existing public S3 prefix `https://ad-hoc-expensify-cash.s3.amazonaws.com/web//`. It shares the security headers and CSP definitions with the static-assets Worker, with PR-testing font and frame origins. It also allows the App template's inline script hashes. + +This is one wildcard Worker, not a Worker per PR. Ad-hoc builds still upload to S3 in `.github/workflows/buildAdHoc.yml`; no build or hosting changes are needed. The existing non-200-to-`index.html` fallback is preserved. `/`, `/index.html`, `/service-worker.js` and fallback HTML bypass forced edge caching. HTML also drops validators and gets `Cache-Control: no-cache` so each rewritten nonce matches its CSP. + +[`wrangler.pr-testing.jsonc`](wrangler.pr-testing.jsonc) is deliberately separate from the static-assets config: PR testing needs neither `dist/` nor an `ASSETS` or `PREVIOUS_BUILDS` binding. It retains the existing Worker name (`pr-testing-expensify`) and compatibility date. Its existing `*.pr-testing.expensify.com/*` route stays managed in Cloudflare, so deploys need no zone access. + +### Migration and deployment + +1. Grant `CLOUDFLARE_WORKERS_DEPLOY_TOKEN` access to edit the existing `pr-testing-expensify` Worker, in addition to the staging and production Workers. Keep `CLOUDFLARE_ACCOUNT_ID` in GitHub secrets. +2. Merge the App PR, then manually run **Deploy PR-testing Cloudflare Worker** (`.github/workflows/deployPRTestingWorker.yml`) on `main`. It requires membership in the web deployer, key holder or mobile deployer team, matching the old repository's deploy authorization. It runs the package's type check and tests before deploying. +3. Verify an existing PR hostname, a deep link, its service worker and two different PR builds. Rebuild one PR and check that the new build appears without stale HTML or CSP errors. +4. Only after that deployment is verified, merge the companion Cloudflare-Workers cleanup PR. Do not delete the live Worker or its wildcard route. If Workers Builds is connected to the old repository, disconnect that integration during the handoff so it cannot deploy the old code again. + +The workflow is manual, like the old repository's deploy workflow; it does not redeploy this shared Worker for every ad-hoc build. A rollback before cleanup can redeploy the previous commit from Cloudflare-Workers. After cleanup, use a previous Worker version in Cloudflare to roll back. + +Local verification, without credentials or uploading anything: + +```bash +npm ci +npm run typecheck +npm test +npx wrangler deploy --dry-run --config wrangler.pr-testing.jsonc +``` + +The PR-testing tests bundle the real configuration and run it in workerd with all requests to S3 intercepted. They verify PR isolation, edge-cache options, SPA fallbacks, nonce matching, shared security headers and service-worker caching without reading live builds. diff --git a/web/cloudflare-worker/package-lock.json b/web/cloudflare-worker/package-lock.json index fb1610bf4aca..f307bb24dce9 100644 --- a/web/cloudflare-worker/package-lock.json +++ b/web/cloudflare-worker/package-lock.json @@ -9,6 +9,7 @@ "version": "0.0.0", "devDependencies": { "@types/node": "24.19.0", + "miniflare": "5.20260926.1-alpha", "typescript": "7.0.2", "wrangler": "4.144.0" } diff --git a/web/cloudflare-worker/package.json b/web/cloudflare-worker/package.json index 9fc3c6cece9b..9aad7fa9a908 100644 --- a/web/cloudflare-worker/package.json +++ b/web/cloudflare-worker/package.json @@ -15,6 +15,7 @@ }, "devDependencies": { "@types/node": "24.19.0", + "miniflare": "5.20260926.1-alpha", "typescript": "7.0.2", "wrangler": "4.144.0" } diff --git a/web/cloudflare-worker/src/csp.ts b/web/cloudflare-worker/src/csp.ts index 32866baf0d55..8af4d0859f2f 100644 --- a/web/cloudflare-worker/src/csp.ts +++ b/web/cloudflare-worker/src/csp.ts @@ -167,9 +167,18 @@ const PRODUCTION_CSP = buildCSP({ scriptSrcHashes: INLINE_SCRIPT_HASHES, }); -function getCSP(environment: Env['ENVIRONMENT'], nonce: string): string { - const csp = environment === 'staging' ? STAGING_CSP : PRODUCTION_CSP; +const PR_TESTING_CSP = buildCSP({ + fontSrcHosts: ['https://*.pr-testing.expensify.com'], + frameSrcHosts: ['https://staging.expensify.com'], + scriptSrcHashes: INLINE_SCRIPT_HASHES, +}); + +type Environment = 'staging' | 'production' | 'pr-testing'; + +function getCSP(environment: Environment, nonce: string): string { + const csp = environment === 'pr-testing' ? PR_TESTING_CSP : environment === 'staging' ? STAGING_CSP : PRODUCTION_CSP; return csp.replace(NONCE_PLACEHOLDER, `nonce-${nonce}`); } export {NONCE_PLACEHOLDER, getCSP}; +export type {Environment}; diff --git a/web/cloudflare-worker/src/index.ts b/web/cloudflare-worker/src/index.ts index e501b582d93b..c309637108cb 100644 --- a/web/cloudflare-worker/src/index.ts +++ b/web/cloudflare-worker/src/index.ts @@ -1,4 +1,5 @@ -import {getCSP, NONCE_PLACEHOLDER} from './csp'; +import {NONCE_PLACEHOLDER} from './csp'; +import {generateNonce, setSecurityHeaders} from './security'; /** Content-hashed build output (`main-<16 hex>.bundle.js`, `main.<10 hex>.css`, ...), emitted at the root of `dist/`. */ const HASHED_ASSET_PATH = /^\/[^/]+(?:-[0-9a-f]{16}\.bundle\.js|\.[0-9a-f]{10}\.[a-z0-9]+)$/; @@ -9,12 +10,6 @@ const STATIC_FILE_PATH = /\.(?:js|mjs|css|map|json|wasm|png|jpe?g|gif|webp|avif| const AASA_PATH = '/.well-known/apple-app-site-association'; const LEGACY_AASA_PATH = '/apple-app-site-association'; -function generateNonce(): string { - const bytes = new Uint8Array(16); - crypto.getRandomValues(bytes); - return btoa(String.fromCharCode(...bytes)); -} - function isHTMLResponse(response: Response): boolean { return response.headers.get('Content-Type')?.startsWith('text/html') ?? false; } @@ -42,16 +37,6 @@ function buildAssetRequest(request: Request, url: URL): Request { return new Request(assetURL, request); } -function setSecurityHeaders(headers: Headers, environment: Env['ENVIRONMENT'], nonce: string) { - headers.set('Strict-Transport-Security', 'max-age=31536000; includeSubDomains; preload'); - headers.set('Content-Security-Policy', getCSP(environment, nonce)); - headers.set('X-Frame-Options', 'SAMEORIGIN'); - headers.set('X-Content-Type-Options', 'nosniff'); - headers.set('Referrer-Policy', 'origin-when-cross-origin'); - // Used by react-native-release-profiler - headers.set('Document-Policy', 'js-profiling'); -} - function getPreviousBuildFileHeaders(object: R2Object): Headers { const headers = new Headers({'Cache-Control': 'public, max-age=0, must-revalidate', ETag: object.httpEtag}); object.writeHttpMetadata(headers); diff --git a/web/cloudflare-worker/src/prTesting.ts b/web/cloudflare-worker/src/prTesting.ts new file mode 100644 index 000000000000..61e8bfdd93ac --- /dev/null +++ b/web/cloudflare-worker/src/prTesting.ts @@ -0,0 +1,45 @@ +/** Serves each PR's existing S3 build through the shared wildcard PR-testing Worker. */ +import {NONCE_PLACEHOLDER} from './csp'; +import {generateNonce, setSecurityHeaders} from './security'; + +// These URLs are overwritten by every ad-hoc build, unlike the content-hashed assets. +const UNVERSIONED_PATHS = new Set(['/', '/index.html', '/service-worker.js']); + +export default { + async fetch(request): Promise { + const url = new URL(request.url); + const prNumber = url.hostname.split('.')[0]; + const bucketURL = `https://ad-hoc-expensify-cash.s3.amazonaws.com/web/${prNumber}`; + const assetURL = bucketURL + (url.pathname === '/' ? '/index.html' : url.pathname); + let assetResponse = await fetch(assetURL, { + cf: {cacheEverything: !UNVERSIONED_PATHS.has(url.pathname)}, + }); + + // Preserve the existing SPA fallback, including for missing assets, during the repository migration. + if (assetResponse.status !== 200) { + assetResponse = await fetch(`${bucketURL}/index.html`, { + cf: {cacheEverything: false}, + }); + } + + const nonce = generateNonce(); + const response = new Response(assetResponse.body, assetResponse); + setSecurityHeaders(response.headers, 'pr-testing', nonce); + + if (!response.headers.get('Content-Type')?.startsWith('text/html')) { + return response; + } + + // A cached HTML body must not be paired with a freshly generated CSP nonce. + response.headers.set('Cache-Control', 'no-cache'); + response.headers.delete('ETag'); + response.headers.delete('Last-Modified'); + return new HTMLRewriter() + .on(`script[nonce="${NONCE_PLACEHOLDER}"]`, { + element(element) { + element.setAttribute('nonce', nonce); + }, + }) + .transform(response); + }, +} satisfies ExportedHandler; diff --git a/web/cloudflare-worker/src/security.ts b/web/cloudflare-worker/src/security.ts new file mode 100644 index 000000000000..21d75e5fcbc1 --- /dev/null +++ b/web/cloudflare-worker/src/security.ts @@ -0,0 +1,22 @@ +import type {Environment} from './csp'; + +/** Security headers and nonce generation shared by the static-assets and PR-testing Workers. */ +import {getCSP} from './csp'; + +function generateNonce(): string { + const bytes = new Uint8Array(16); + crypto.getRandomValues(bytes); + return btoa(String.fromCharCode(...bytes)); +} + +function setSecurityHeaders(headers: Headers, environment: Environment, nonce: string) { + headers.set('Strict-Transport-Security', 'max-age=31536000; includeSubDomains; preload'); + headers.set('Content-Security-Policy', getCSP(environment, nonce)); + headers.set('X-Frame-Options', 'SAMEORIGIN'); + headers.set('X-Content-Type-Options', 'nosniff'); + headers.set('Referrer-Policy', 'origin-when-cross-origin'); + // Used by react-native-release-profiler + headers.set('Document-Policy', 'js-profiling'); +} + +export {generateNonce, setSecurityHeaders}; diff --git a/web/cloudflare-worker/tests/prTesting.test.ts b/web/cloudflare-worker/tests/prTesting.test.ts new file mode 100644 index 000000000000..9612242203ba --- /dev/null +++ b/web/cloudflare-worker/tests/prTesting.test.ts @@ -0,0 +1,216 @@ +import {Miniflare, Request as WorkerRequest, Response as WorkerResponse} from 'miniflare'; +/** Runs the PR-testing Worker in workerd with S3 requests mocked, never reaching a real PR build. */ +import assert from 'node:assert/strict'; +import {execFileSync} from 'node:child_process'; +import {createHash} from 'node:crypto'; +import {mkdtempSync, readFileSync, rmSync} from 'node:fs'; +import {tmpdir} from 'node:os'; +import path from 'node:path'; +import {after, before, describe, test} from 'node:test'; +import {experimental_readRawConfig} from 'wrangler'; + +const WORKER_DIR = path.resolve(import.meta.dirname, '..'); +const HTML = ''; + +describe('PR-testing worker', () => { + const tempDir = mkdtempSync(path.join(tmpdir(), 'pr-testing-worker-')); + const pending: Array<{ + pathname: string; + status: number; + body: string; + headers: Record; + }> = []; + const requests: Array<{pathname: string; cacheEverything: unknown}> = []; + let worker: Miniflare; + + function mockS3(pathname: string, status: number, body: string, headers: Record = {}) { + pending.push({pathname, status, body, headers}); + } + + before(async () => { + const configPath = path.join(WORKER_DIR, 'wrangler.pr-testing.jsonc'); + execFileSync(process.execPath, [path.join(WORKER_DIR, 'node_modules/wrangler/bin/wrangler.js'), 'deploy', '--dry-run', '--config', configPath, '--outdir', tempDir], { + env: {...process.env, WRANGLER_SEND_METRICS: 'false'}, + }); + const {rawConfig} = experimental_readRawConfig({config: configPath}); + worker = new Miniflare({ + workers: [ + { + config: { + name: rawConfig.name ?? '', + compatibilityDate: rawConfig.compatibility_date ?? '', + manifest: { + mainModule: 'prTesting.js', + modules: { + 'prTesting.js': { + type: 'esm', + contents: readFileSync(path.join(tempDir, 'prTesting.js'), 'utf8'), + }, + }, + }, + }, + dev: { + outboundService: { + type: 'fetcher', + handler(request: WorkerRequest) { + const url = new URL(request.url); + assert.equal(url.origin, 'https://ad-hoc-expensify-cash.s3.amazonaws.com'); + const expected = pending.shift(); + assert.ok(expected, `Unexpected S3 request: ${url.pathname}`); + assert.equal(url.pathname, expected.pathname); + requests.push({ + pathname: url.pathname, + cacheEverything: request.cf?.cacheEverything, + }); + return new WorkerResponse(expected.body, { + status: expected.status, + headers: expected.headers, + }); + }, + }, + }, + }, + ], + }); + await worker.ready; + }); + + after(async () => { + await worker?.dispose(); + rmSync(tempDir, {recursive: true, force: true}); + assert.equal(pending.length, 0, 'Every expected S3 request must be made'); + }); + + function mockHTML(prNumber: string) { + mockS3(`/web/${prNumber}/index.html`, 200, HTML, { + 'Content-Type': 'text/html', + ETag: '"old-shell"', + 'Last-Modified': 'Wed, 01 Jan 2025 00:00:00 GMT', + }); + } + + test('serves the S3 directory selected by the PR hostname', async () => { + // Given two PR builds with different assets at the same path + for (const prNumber of ['12345', '67890']) { + mockS3(`/web/${prNumber}/bundle.js`, 200, `// PR ${prNumber}`, { + 'Content-Type': 'text/javascript', + 'Cache-Control': 'public,max-age=31536000,immutable', + }); + + // When each PR's hostname requests that asset + const response = await worker.dispatchFetch(`https://${prNumber}.pr-testing.expensify.com/bundle.js`); + + // Then it gets its own build and retains the origin's asset caching policy + assert.equal(response.status, 200); + assert.equal(await response.text(), `// PR ${prNumber}`); + assert.equal(response.headers.get('Cache-Control'), 'public,max-age=31536000,immutable'); + assert.equal(response.headers.get('X-Content-Type-Options'), 'nosniff'); + assert.equal(requests.at(-1)?.cacheEverything, true); + } + }); + + test('serves the root and index.html with matching fresh CSP nonces', async () => { + // Given two loads of the same PR's app shell + const nonces = new Set(); + for (const pathname of ['/', '/index.html']) { + mockHTML('12345'); + + // When the shell is requested at either URL + const response = await worker.dispatchFetch(`https://12345.pr-testing.expensify.com${pathname}`); + const html = await response.text(); + const nonce = /'; -describe('per-PR Worker deployment', () => { +describe('Worker Preview runtime packaging', () => { const temp = mkdtempSync(path.join(tmpdir(), 'pr-preview-integration-')); const directories = new Map(); const identity = getIdentity('a'.repeat(40), '100', '1'); @@ -43,7 +43,7 @@ describe('per-PR Worker deployment', () => { ); const archive = path.join(branch, 'webBuild.tar.gz'); execFileSync('tar', ['-czf', archive, '-C', branch, 'dist'], {env: {...process.env, COPYFILE_DISABLE: '1'}}); - const config = createConfig(getTarget(prNumber), runtime, 'b'.repeat(32)); + const config = createConfig(runtime); // Run the same Bun-native extractor directly; this standalone package does not install the App CLI dependencies. const directory = await prepareDirectory(path.join(branch, 'bundle'), archive, config, identity, (archivePath: string, destination: string) => { execFileSync('bun', [ @@ -54,17 +54,19 @@ describe('per-PR Worker deployment', () => { ]); }); directories.set(prNumber, directory); - const dryRun = execFileSync( - process.execPath, - [path.join(WORKER_DIR, 'node_modules/wrangler/bin/wrangler.js'), 'deploy', '--dry-run', '--config', path.join(directory, 'wrangler.json')], - { - encoding: 'utf8', - env: {...process.env, WRANGLER_SEND_METRICS: 'false'}, - }, - ); + // workerd has no native Preview API, so each Preview uses a separate local Worker and test-only route. + const localConfigPath = path.join(directory, 'wrangler.local.json'); + writeFileSync(localConfigPath, JSON.stringify({...config, name: `preview-${prNumber}`, vars: config.previews.vars, routes: [`${getTarget(prNumber).hostname}/*`]})); + const {rawConfig: deploymentConfig} = experimental_readRawConfig({config: path.join(directory, 'wrangler.json')}); + assert.equal(deploymentConfig.name, 'pr-testing-expensify'); + assert.equal(deploymentConfig.routes, undefined); + const dryRun = execFileSync(process.execPath, [path.join(WORKER_DIR, 'node_modules/wrangler/bin/wrangler.js'), 'deploy', '--dry-run', '--config', localConfigPath], { + encoding: 'utf8', + env: {...process.env, WRANGLER_SEND_METRICS: 'false'}, + }); assert.doesNotMatch(dryRun, /dist\/index\.html/, 'Static Assets must not also be uploaded as Worker modules'); } - server = createTestHarness({workers: [...directories.values()].map((directory) => ({configPath: path.join(directory, 'wrangler.json')}))}); + server = createTestHarness({workers: [...directories.values()].map((directory) => ({configPath: path.join(directory, 'wrangler.local.json')}))}); await server.listen(); }); @@ -108,7 +110,7 @@ describe('per-PR Worker deployment', () => { writeFileSync(path.join(directory, 'worker/__preview_entry.mjs'), createWrapper(nextIdentity)); writeFileSync(path.join(directory, 'dist/preview-build.json'), JSON.stringify(nextIdentity)); // When workerd loads the updated version - await server.update({workers: [...directories.values()].map((root) => ({configPath: path.join(root, 'wrangler.json')}))}); + await server.update({workers: [...directories.values()].map((root) => ({configPath: path.join(root, 'wrangler.local.json')}))}); // Then PR 123 serves the new code/assets while PR 456 remains unchanged assert.equal((await server.fetch('https://123.pr-testing.expensify.com/')).headers.get('X-Preview-Branch'), 'branch-123-v2'); assert.deepEqual(await (await server.fetch('https://123.pr-testing.expensify.com/preview-build.json')).json(), nextIdentity); From d9de2e83cde9d4a05409a1eeb304f7c1f4c223e3 Mon Sep 17 00:00:00 2001 From: rory Date: Tue, 3 Nov 2026 17:28:07 -0800 Subject: [PATCH 7/9] ci: remove GitHub Deployment bookkeeping --- .github/workflows/buildAdHoc.yml | 46 +--- .github/workflows/buildWeb.yml | 5 + .github/workflows/cloudflareWorkerTests.yml | 2 +- scripts/adhocCloudflareWorkerDeployer.ts | 256 ------------------ scripts/deployAdhocCloudflareWorker.ts | 89 ++++++ tests/tooling/prTestingWorker.test.mjs | 213 +++------------ web/cloudflare-worker/README.md | 13 +- .../scripts/prTestingConfig.ts | 13 +- 8 files changed, 147 insertions(+), 490 deletions(-) delete mode 100644 scripts/adhocCloudflareWorkerDeployer.ts create mode 100644 scripts/deployAdhocCloudflareWorker.ts diff --git a/.github/workflows/buildAdHoc.yml b/.github/workflows/buildAdHoc.yml index cd8a71a0909e..5fcd395ec5ef 100644 --- a/.github/workflows/buildAdHoc.yml +++ b/.github/workflows/buildAdHoc.yml @@ -101,39 +101,12 @@ jobs: download-dsyms: true secrets: inherit - prepareWeb: - name: Register the PR preview build - if: ${{ fromJSON(inputs.BUILD_WEB) && inputs.APP_PR_NUMBER != '' }} - runs-on: blacksmith-2vcpu-ubuntu-2404 - outputs: - PR_NUMBER: ${{ steps.register.outputs.PR_NUMBER }} - SHA: ${{ steps.register.outputs.SHA }} - DEPLOYMENT_ID: ${{ steps.register.outputs.DEPLOYMENT_ID }} - REQUEST_ATTEMPT: ${{ steps.register.outputs.REQUEST_ATTEMPT }} - steps: - - name: Checkout trusted preview tooling - uses: useblacksmith/checkout@0647fdbab2614a5eb86d2971070e325060d91056 # v1.7.0 - with: - ref: ${{ github.workflow_sha }} - persist-credentials: false - - - name: Setup Node - uses: ./.github/actions/composite/setupNode - - - name: Resolve the build SHA and register preview intent - id: register - # Both callers resolve the PR number and immutable App SHA before invoking this workflow. - run: bun scripts/adhocCloudflareWorkerDeployer.ts register --pr-number ${{ inputs.APP_PR_NUMBER }} --ref ${{ inputs.APP_REF }} - env: - GITHUB_TOKEN: ${{ github.token }} - buildWeb: name: Build Web - needs: [prepareWeb] if: ${{ fromJSON(inputs.BUILD_WEB) && inputs.APP_PR_NUMBER != '' }} uses: ./.github/workflows/buildWeb.yml with: - ref: ${{ needs.prepareWeb.outputs.SHA }} + ref: ${{ inputs.APP_REF }} environment: adhoc pull-request-number: ${{ inputs.APP_PR_NUMBER }} build-preview-worker: true @@ -141,8 +114,7 @@ jobs: deployWebAdHoc: name: Deploy the PR Worker Preview and web assets - needs: [prepareWeb, buildWeb] - if: ${{ needs.prepareWeb.outputs.DEPLOYMENT_ID != '' }} + needs: [buildWeb] runs-on: blacksmith-2vcpu-ubuntu-2404 env: WRANGLER_SEND_METRICS: false @@ -170,12 +142,10 @@ jobs: - name: Deploy and verify the PR Preview and assets together run: | - bun scripts/adhocCloudflareWorkerDeployer.ts deploy \ - --pr-number ${{ needs.prepareWeb.outputs.PR_NUMBER }} --sha ${{ needs.prepareWeb.outputs.SHA }} \ - --deployment-id ${{ needs.prepareWeb.outputs.DEPLOYMENT_ID }} --request-attempt ${{ needs.prepareWeb.outputs.REQUEST_ATTEMPT }} \ + bun scripts/deployAdhocCloudflareWorker.ts \ + --pr-number ${{ inputs.APP_PR_NUMBER }} --sha ${{ needs.buildWeb.outputs.BUILD_SHA }} \ --artifact-directory "${{ runner.temp }}/pr-testing-artifacts" env: - GITHUB_TOKEN: ${{ github.token }} CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_WORKERS_DEPLOY_TOKEN }} CLOUDFLARE_ACCOUNT_ID: 3eb3df6f8c190fa13c3fc0a53c30af1a @@ -183,7 +153,7 @@ jobs: runs-on: blacksmith-4vcpu-ubuntu-2404 if: ${{ always() && (inputs.APP_PR_NUMBER != '' || inputs.MOBILE_EXPENSIFY_PR != '') }} name: Post a GitHub comment with app download links for testing - needs: [prepareWeb, buildWeb, deployWebAdHoc, buildAndroid, buildIOS] + needs: [buildWeb, deployWebAdHoc, buildAndroid, buildIOS] steps: - name: Checkout uses: useblacksmith/checkout@0647fdbab2614a5eb86d2971070e325060d91056 # v1.7.0 @@ -202,7 +172,7 @@ jobs: COMMENT_PREFIX: ':test_tube::test_tube: Use the links below to test this adhoc build' ANDROID: ${{ needs.buildAndroid.result }} IOS: ${{ needs.buildIOS.result }} - WEB: ${{ (needs.prepareWeb.result == 'failure' || needs.buildWeb.result == 'failure') && 'failure' || needs.deployWebAdHoc.result }} + WEB: ${{ needs.buildWeb.result == 'failure' && 'failure' || needs.deployWebAdHoc.result }} ANDROID_LINK: ${{ needs.buildAndroid.outputs.ROCK_ARTIFACT_URL }} IOS_LINK: ${{ needs.buildIOS.outputs.ROCK_ARTIFACT_URL }} WEB_LINK: https://${{ inputs.APP_PR_NUMBER }}.pr-testing.expensify.com @@ -225,7 +195,7 @@ jobs: runs-on: blacksmith-4vcpu-ubuntu-2404 if: ${{ always() && (inputs.APP_PR_NUMBER != '' || needs.buildAndroid.outputs.ROCK_ARTIFACT_URL != '' || needs.buildIOS.outputs.ROCK_ARTIFACT_URL != '') }} name: Build Summary - needs: [prepareWeb, buildWeb, deployWebAdHoc, buildAndroid, buildIOS] + needs: [buildWeb, deployWebAdHoc, buildAndroid, buildIOS] steps: - name: Checkout uses: useblacksmith/checkout@0647fdbab2614a5eb86d2971070e325060d91056 # v1.7.0 @@ -255,7 +225,7 @@ jobs: const iosLink = '${{ needs.buildIOS.outputs.ROCK_ARTIFACT_URL }}' || ''; const mobileExpensifySHA = '${{ steps.getMobileExpensifySHA.outputs.SHA }}' || ''; - const webFailed = '${{ needs.prepareWeb.result }}' === 'failure' || '${{ needs.buildWeb.result }}' === 'failure' || webResult === 'failure'; + const webFailed = '${{ needs.buildWeb.result }}' === 'failure' || webResult === 'failure'; const webStatus = webLink ? '✅ Success' : webFailed ? '❌ Failed' : '⏭️ Skipped'; const androidStatus = androidLink ? '✅ Success' : '${{ needs.buildAndroid.result }}' === 'failure' ? '❌ Failed' : '⏭️ Skipped'; const iosStatus = iosLink ? '✅ Success' : '${{ needs.buildIOS.result }}' === 'failure' ? '❌ Failed' : '⏭️ Skipped'; diff --git a/.github/workflows/buildWeb.yml b/.github/workflows/buildWeb.yml index 176370928903..6833c7e06706 100644 --- a/.github/workflows/buildWeb.yml +++ b/.github/workflows/buildWeb.yml @@ -21,6 +21,9 @@ on: default: false outputs: + BUILD_SHA: + description: Immutable App commit used for the web build and Worker bundle + value: ${{ jobs.build.outputs.BUILD_SHA }} TAR_FILENAME: description: Filename of the .tar.gz web build artifact value: ${{ jobs.build.outputs.TAR_FILENAME }} @@ -36,6 +39,7 @@ jobs: name: Build Web runs-on: blacksmith-8vcpu-ubuntu-2404 outputs: + BUILD_SHA: ${{ steps.checkout.outputs.commit }} TAR_FILENAME: webBuild.tar.gz ZIP_FILENAME: webBuild.zip SOURCEMAP_FILENAME: merged-source-map.js.map @@ -43,6 +47,7 @@ jobs: PULL_REQUEST_NUMBER: ${{ inputs.pull-request-number }} steps: - name: Checkout + id: checkout uses: useblacksmith/checkout@0647fdbab2614a5eb86d2971070e325060d91056 # v1.7.0 with: ref: ${{ inputs.ref }} diff --git a/.github/workflows/cloudflareWorkerTests.yml b/.github/workflows/cloudflareWorkerTests.yml index f7e7ef274034..4297d68170c8 100644 --- a/.github/workflows/cloudflareWorkerTests.yml +++ b/.github/workflows/cloudflareWorkerTests.yml @@ -12,7 +12,7 @@ on: - '.github/workflows/cloudflareWorkerTests.yml' - '.github/workflows/buildWeb.yml' - '.github/workflows/buildAdHoc.yml' - - 'scripts/adhocCloudflareWorkerDeployer.ts' + - 'scripts/deployAdhocCloudflareWorker.ts' - 'scripts/extractPRTestingAssets.ts' - '.bun-version' diff --git a/scripts/adhocCloudflareWorkerDeployer.ts b/scripts/adhocCloudflareWorkerDeployer.ts deleted file mode 100644 index 959602b7d9ec..000000000000 --- a/scripts/adhocCloudflareWorkerDeployer.ts +++ /dev/null @@ -1,256 +0,0 @@ -#!/usr/bin/env bun - -/** - * Deploy ad-hoc Cloudflare previews from the same App revision as their web assets. - * This CLI records the requested revision in GitHub, publishes the compiled Worker and assets, and verifies the result. - * Application requests use the same Worker entry point as staging and production. - */ - -import CONST from '@github/libs/CONST'; -import GithubUtils from '@github/libs/GithubUtils'; - -import type {BuildIdentity} from '@expensify/new-expensify-cloudflare-worker/pr-testing-config'; - -import {createConfig, getIdentity, getPreviewCommand, getTarget, isNewerRequest, prepareDirectory, STATUS_PATH} from '@expensify/new-expensify-cloudflare-worker/pr-testing-config'; -import CLI from 'expensify-common/CLI'; -import {appendFile, lstat} from 'node:fs/promises'; -import path from 'node:path'; - -import extractPRTestingAssets from './extractPRTestingAssets'; - -type GitHubClient = { - pulls: Pick; - repos: Pick; -}; -type PreviewCommand = 'register' | 'deploy'; -type DeploymentState = 'in_progress' | 'success' | 'failure'; -type BuiltPreviewOptions = {prNumber: number; sha: string; deploymentID: number; requestAttempt: string}; -type PreviewOptions = {command: 'register'; prNumber: number; ref: string} | ({command: 'deploy'; artifactDirectory: string} & BuiltPreviewOptions); - -const REPOSITORY = {owner: CONST.GITHUB_OWNER, repo: CONST.APP_REPO}; -const VERIFY_ATTEMPTS = 12; -const VERIFY_REQUEST_TIMEOUT_MS = 10000; -const VERIFY_RETRY_DELAY_MS = 5000; - -function getPayloadIdentity(value: unknown): BuildIdentity { - if (typeof value !== 'object' || value === null || !('sha' in value) || !('runID' in value) || !('runAttempt' in value)) { - throw new Error('Deployment identity does not match this build'); - } - return getIdentity(value.sha, value.runID, value.runAttempt); -} - -/** - * Workflow cancellation stops superseded builds. These identity checks also reject artifacts from a retried older job, - * and prevent a working response from another deployment from being reported as this build's success. - */ -class AdhocCloudflareWorkerDeployer { - private readonly github: GitHubClient; - - private readonly fetch: typeof fetch; - - constructor({github, fetchImpl = fetch}: {github: GitHubClient; fetchImpl?: typeof fetch}) { - this.github = github; - this.fetch = fetchImpl; - } - - // GitHub REST request properties use their API's snake_case spelling. - /* eslint-disable @typescript-eslint/naming-convention */ - async register(prNumber: number, ref: string, runID: string, runAttempt: string, isPushBuild = false) { - const target = getTarget(prNumber); - // Closed PRs can retain and rebuild their previews, but the PR must belong to App. - await this.github.pulls.get({...REPOSITORY, pull_number: prNumber}); - const {data: commit} = await this.github.repos.getCommit({...REPOSITORY, ref}); - const identity = getIdentity(commit.sha, runID, runAttempt); - // A late registration must not replace a request with a newer workflow run or attempt. - const {data: latest} = await this.github.repos.listDeployments({...REPOSITORY, environment: target.environment, per_page: 1}); - const previousDeployment = latest.at(0); - if (previousDeployment && isNewerRequest(getPayloadIdentity(previousDeployment.payload), identity)) { - // Queued main builds still need artifacts for bisection, even if a newer manual preview already exists. - if (isPushBuild) { - return {sha: identity.sha, deploymentID: ''}; - } - throw new Error('A newer ad-hoc build was already requested'); - } - const {data: deployment} = await this.github.repos.createDeployment({ - ...REPOSITORY, - ref: identity.sha, - environment: target.environment, - auto_merge: false, - required_contexts: [], - transient_environment: true, - production_environment: false, - payload: identity, - description: `PR ${prNumber} ad-hoc web build`, - }); - if (!('id' in deployment)) { - throw new Error('GitHub did not create a deployment record'); - } - return {sha: identity.sha, deploymentID: String(deployment.id)}; - } - - async assertCurrent(prNumber: number, deploymentID: number, identity: BuildIdentity): Promise { - const target = getTarget(prNumber); - const {data: deployment} = await this.github.repos.getDeployment({...REPOSITORY, deployment_id: deploymentID}); - const payload = getPayloadIdentity(deployment.payload); - if ( - deployment.environment !== target.environment || - deployment.ref !== identity.sha || - payload.sha !== identity.sha || - payload.runID !== identity.runID || - payload.runAttempt !== identity.runAttempt - ) { - throw new Error('Deployment identity does not match this build'); - } - const {data: recent} = await this.github.repos.listDeployments({...REPOSITORY, environment: target.environment, per_page: 1}); - if (String(recent.at(0)?.id) !== String(deploymentID)) { - throw new Error('A newer ad-hoc build was requested; refusing to overwrite it'); - } - } - - async status(deploymentID: number, state: DeploymentState, hostname: string): Promise { - await this.github.repos.createDeploymentStatus({...REPOSITORY, deployment_id: deploymentID, state, environment_url: `https://${hostname}`, auto_inactive: state === 'success'}); - } - /* eslint-enable @typescript-eslint/naming-convention */ - - async verify(hostname: string, identity: BuildIdentity, maxAttempts = VERIFY_ATTEMPTS): Promise { - for (let attempt = 0; attempt < maxAttempts; attempt++) { - const responses = await Promise.all( - [STATUS_PATH, '/preview-build.json'].map((pathname) => - this.fetch(`https://${hostname}${pathname}?run=${identity.runID}-${identity.runAttempt}`, { - redirect: 'error', - signal: AbortSignal.timeout(VERIFY_REQUEST_TIMEOUT_MS), - }).catch(() => null), - ), - ); - const data: unknown[] = await Promise.all(responses.map(async (response) => (response?.ok ? response.json().catch(() => null) : null))); - const matches = data.every((item) => { - try { - const payload = getPayloadIdentity(item); - return payload.sha === identity.sha && payload.runID === identity.runID && payload.runAttempt === identity.runAttempt; - } catch { - return false; - } - }); - if (matches) { - return; - } - if (attempt < maxAttempts - 1) { - await Bun.sleep(VERIFY_RETRY_DELAY_MS); - } - } - throw new Error('The preview hostname does not serve this Worker and asset build'); - } - - async deploy(prNumber: number, deploymentID: number, identity: BuildIdentity, publish: () => void | Promise): Promise { - const target = getTarget(prNumber); - await this.assertCurrent(prNumber, deploymentID, identity); - await this.status(deploymentID, 'in_progress', target.hostname); - try { - await publish(); - await this.verify(target.hostname, identity); - await this.assertCurrent(prNumber, deploymentID, identity); - await this.status(deploymentID, 'success', target.hostname); - } catch (error) { - await this.status(deploymentID, 'failure', target.hostname); - throw error; - } - } -} - -function parseCommand(value: string): PreviewCommand { - if (value === 'register' || value === 'deploy') { - return value; - } - throw new Error('Expected register or deploy'); -} - -function parseNumericID(value: string): number { - if (!/^[1-9]\d*$/.test(value) || !Number.isSafeInteger(Number(value))) { - throw new Error('Expected a positive numeric ID'); - } - return Number(value); -} - -function requireString(value: string | undefined, name: string): string { - if (!value) { - throw new Error(`${name} is required`); - } - return value; -} - -function parseCLI(): PreviewOptions { - // The shared CLI uses kebab-case option names. - /* eslint-disable @typescript-eslint/naming-convention */ - const cli = new CLI({ - positionalArgs: [{name: 'command', description: 'register a request or deploy and verify a preview', parse: parseCommand}], - namedArgs: { - 'pr-number': {description: 'Expensify/App PR number', parse: parseNumericID}, - ref: {description: 'App source ref (required for register)', required: false}, - sha: {description: 'Immutable built App SHA (required for deploy)', required: false}, - 'deployment-id': {description: 'Registered GitHub deployment ID (required for deploy)', required: false, parse: parseNumericID}, - 'request-attempt': {description: 'Original request attempt, including on failed-job retries (required for deploy)', required: false}, - 'artifact-directory': {description: 'Downloaded Worker and web artifacts directory (required for deploy)', required: false}, - }, - } as const); - /* eslint-enable @typescript-eslint/naming-convention */ - const command = cli.positionalArgs.command; - const prNumber = cli.namedArgs['pr-number']; - if (command === 'register') { - return {command, prNumber, ref: requireString(cli.namedArgs.ref, '--ref')}; - } - const deploymentID = cli.namedArgs['deployment-id']; - if (deploymentID === undefined) { - throw new Error('--deployment-id is required'); - } - const built = { - prNumber, - deploymentID, - sha: requireString(cli.namedArgs.sha, '--sha'), - requestAttempt: requireString(cli.namedArgs['request-attempt'], '--request-attempt'), - }; - return {command, ...built, artifactDirectory: requireString(cli.namedArgs['artifact-directory'], '--artifact-directory')}; -} - -async function main(): Promise { - // Parse first so --help and invalid arguments work without CI credentials. - const options = parseCLI(); - const deployer = new AdhocCloudflareWorkerDeployer({github: GithubUtils.octokit}); - const prNumber = options.prNumber; - const runID = requireString(process.env.GITHUB_RUN_ID, 'GITHUB_RUN_ID'); - if (options.command === 'register') { - const runAttempt = requireString(process.env.GITHUB_RUN_ATTEMPT, 'GITHUB_RUN_ATTEMPT'); - const result = await deployer.register(prNumber, options.ref, runID, runAttempt, process.env.GITHUB_EVENT_NAME === 'push'); - await appendFile( - requireString(process.env.GITHUB_OUTPUT, 'GITHUB_OUTPUT'), - `PR_NUMBER=${prNumber}\nSHA=${result.sha}\nDEPLOYMENT_ID=${result.deploymentID}\nREQUEST_ATTEMPT=${runAttempt}\n`, - ); - return; - } - const identity = getIdentity(options.sha, runID, options.requestAttempt); - const deploymentID = options.deploymentID; - const artifactDirectory = path.resolve(options.artifactDirectory); - await deployer.deploy(prNumber, deploymentID, identity, async () => { - const runtimePath = path.join(artifactDirectory, 'worker/runtime.json'); - if (!(await lstat(runtimePath)).isFile()) { - throw new Error('Worker runtime settings must be a regular JSON file'); - } - const runtime: unknown = await Bun.file(runtimePath).json(); - const config = createConfig(runtime); - const root = await prepareDirectory(path.join(artifactDirectory, 'worker'), path.join(artifactDirectory, 'assets/webBuild.tar.gz'), config, identity, extractPRTestingAssets); - // Resolve from the workspace so both hoisted and package-local tool installs work. - const wranglerPackage = Bun.resolveSync('wrangler/package.json', path.resolve('web/cloudflare-worker')); - const wrangler = path.join(path.dirname(wranglerPackage), 'bin/wrangler.js'); - const command = getPreviewCommand(prNumber, path.join(root, 'wrangler.json'), process.env); - const result = Bun.spawn(['node', wrangler, ...command.args], {cwd: root, env: command.env, stdin: 'inherit', stdout: 'inherit', stderr: 'inherit'}); - if ((await result.exited) !== 0) { - throw new Error('Cloudflare preview deployment failed'); - } - }); -} - -if (import.meta.main) { - await main(); -} - -export {AdhocCloudflareWorkerDeployer}; -export type {GitHubClient}; diff --git a/scripts/deployAdhocCloudflareWorker.ts b/scripts/deployAdhocCloudflareWorker.ts new file mode 100644 index 000000000000..980526232e87 --- /dev/null +++ b/scripts/deployAdhocCloudflareWorker.ts @@ -0,0 +1,89 @@ +#!/usr/bin/env bun + +/** Publish branch Worker code and Static Assets as one Preview, then verify the requested build is served. */ +import type {BuildIdentity} from '@expensify/new-expensify-cloudflare-worker/pr-testing-config'; + +import {createConfig, getIdentity, getPreviewCommand, getTarget, prepareDirectory, STATUS_PATH} from '@expensify/new-expensify-cloudflare-worker/pr-testing-config'; +import CLI from 'expensify-common/CLI'; +import {lstat} from 'node:fs/promises'; +import path from 'node:path'; + +import extractPRTestingAssets from './extractPRTestingAssets'; + +const VERIFY_ATTEMPTS = 12; +const VERIFY_REQUEST_TIMEOUT_MS = 10000; +const VERIFY_RETRY_DELAY_MS = 5000; + +async function verifyPreview(hostname: string, identity: BuildIdentity, fetchImpl: typeof fetch = fetch, maxAttempts = VERIFY_ATTEMPTS): Promise { + for (let attempt = 0; attempt < maxAttempts; attempt++) { + const responses = await Promise.all( + [STATUS_PATH, '/preview-build.json'].map((pathname) => + fetchImpl(`https://${hostname}${pathname}?run=${identity.runID}-${identity.runAttempt}`, { + redirect: 'error', + signal: AbortSignal.timeout(VERIFY_REQUEST_TIMEOUT_MS), + }).catch(() => null), + ), + ); + const data: unknown[] = await Promise.all(responses.map(async (response) => (response?.ok ? response.json().catch(() => null) : null))); + const matches = data.every((item) => { + if (typeof item !== 'object' || item === null) { + return false; + } + return 'sha' in item && item.sha === identity.sha && 'runID' in item && item.runID === identity.runID && 'runAttempt' in item && item.runAttempt === identity.runAttempt; + }); + if (matches) { + return; + } + if (attempt < maxAttempts - 1) { + await Bun.sleep(VERIFY_RETRY_DELAY_MS); + } + } + throw new Error('The preview hostname does not serve this Worker and asset build'); +} + +async function deployAdhocCloudflareWorker(prNumber: number, identity: BuildIdentity, artifactDirectory: string): Promise { + const target = getTarget(prNumber); + const runtimePath = path.join(artifactDirectory, 'worker/runtime.json'); + if (!(await lstat(runtimePath)).isFile()) { + throw new Error('Worker runtime settings must be a regular JSON file'); + } + const runtime: unknown = await Bun.file(runtimePath).json(); + const root = await prepareDirectory( + path.join(artifactDirectory, 'worker'), + path.join(artifactDirectory, 'assets/webBuild.tar.gz'), + createConfig(runtime), + identity, + extractPRTestingAssets, + ); + // Resolve from the workspace so both hoisted and package-local tool installs work. + const wranglerPackage = Bun.resolveSync('wrangler/package.json', path.resolve('web/cloudflare-worker')); + const wrangler = path.join(path.dirname(wranglerPackage), 'bin/wrangler.js'); + const command = getPreviewCommand(prNumber, path.join(root, 'wrangler.json'), process.env); + const result = Bun.spawn(['node', wrangler, ...command.args], {cwd: root, env: command.env, stdin: 'inherit', stdout: 'inherit', stderr: 'inherit'}); + if ((await result.exited) !== 0) { + throw new Error('Cloudflare preview deployment failed'); + } + await verifyPreview(target.hostname, identity); +} + +async function main(): Promise { + // The shared CLI uses kebab-case option names. + /* eslint-disable @typescript-eslint/naming-convention */ + const cli = new CLI({ + namedArgs: { + 'pr-number': {description: 'Expensify/App PR number', parse: (value: string) => Number(getTarget(value).name)}, + sha: {description: 'Immutable App SHA used for the Worker and web build'}, + 'artifact-directory': {description: 'Downloaded Worker and web artifacts directory'}, + }, + } as const); + /* eslint-enable @typescript-eslint/naming-convention */ + const identity = getIdentity(cli.namedArgs.sha, process.env.GITHUB_RUN_ID, process.env.GITHUB_RUN_ATTEMPT); + await deployAdhocCloudflareWorker(cli.namedArgs['pr-number'], identity, path.resolve(cli.namedArgs['artifact-directory'])); +} + +if (import.meta.main) { + await main(); +} + +export default deployAdhocCloudflareWorker; +export {verifyPreview}; diff --git a/tests/tooling/prTestingWorker.test.mjs b/tests/tooling/prTestingWorker.test.mjs index cb7c0bdb2d71..e5f0b71a9c20 100644 --- a/tests/tooling/prTestingWorker.test.mjs +++ b/tests/tooling/prTestingWorker.test.mjs @@ -1,19 +1,18 @@ -/** Preview deployment behavior and artifact security, using a stateful fake service rather than SDK call-order assertions. */ +/** Preview identity verification and artifact security without live deployments. */ import {afterEach, describe, expect, test} from 'bun:test'; -import {createConfig, getIdentity, getPreviewCommand, getTarget, prepareDirectory} from '@expensify/new-expensify-cloudflare-worker/pr-testing-config'; +import {createConfig, getIdentity, getPreviewCommand, getTarget, prepareDirectory, STATUS_PATH} from '@expensify/new-expensify-cloudflare-worker/pr-testing-config'; import {spawnSync} from 'node:child_process'; import {existsSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync} from 'node:fs'; import {tmpdir} from 'node:os'; import path from 'node:path'; -import {AdhocCloudflareWorkerDeployer} from '../../scripts/adhocCloudflareWorkerDeployer.ts'; +import {verifyPreview} from '../../scripts/deployAdhocCloudflareWorker.ts'; import extractPRTestingAssets from '../../scripts/extractPRTestingAssets.ts'; describe('PR-testing Worker deployment', () => { const {Bun} = globalThis; const SHA = 'a'.repeat(40); - const OTHER_SHA = 'b'.repeat(40); const IDENTITY = getIdentity(SHA, '100', '1'); const RUNTIME = {compatibility_date: '2026-09-26', compatibility_flags: []}; const ROOT = path.resolve(import.meta.dirname, '../..'); @@ -31,200 +30,56 @@ describe('PR-testing Worker deployment', () => { return directory; } - function previewService(prState = 'open') { - const state = { - refs: new Map([ - ['feature/worker', SHA], - ['main', OTHER_SHA], - ]), - deployments: [], - statuses: new Map(), - worker: null, - assets: null, - }; - const github = { - pulls: {get: async () => ({data: {state: prState}})}, - repos: { - getCommit: async ({ref}) => { - if (!state.refs.has(ref)) { - throw new Error('Unknown source ref'); - } - return {data: {sha: state.refs.get(ref)}}; - }, - listDeployments: async ({environment, per_page: limit = 30}) => ({ - data: state.deployments - .filter((item) => item.environment === environment) - .toReversed() - .slice(0, limit), - }), - getDeployment: async ({deployment_id: id}) => { - const item = state.deployments.find((deployment) => deployment.id === id); - if (!item) { - throw new Error('Unknown deployment'); - } - return {data: structuredClone(item)}; - }, - createDeployment: async (input) => { - const deployment = {id: state.deployments.length + 1, ...structuredClone(input)}; - state.deployments.push(deployment); - return {data: deployment}; - }, - createDeploymentStatus: async ({deployment_id: id, ...status}) => { - state.statuses.set(id, status); - return {data: status}; - }, - }, - }; - const controller = new AdhocCloudflareWorkerDeployer({ - github, - fetchImpl: async (url) => { - const pathname = new URL(url).pathname; - if (pathname === '/__expensify_preview') { - return Response.json(state.worker); - } - if (pathname === '/preview-build.json') { - return Response.json(state.assets); - } - throw new Error('Unknown preview endpoint'); - }, - }); - const publish = (identity) => { - state.worker = identity; - state.assets = identity; - }; - return {controller, state, publish}; - } - - describe('preview deployment behavior', () => { - test('pins the requested revision and allows an authorized rebuild after the PR closes', async () => { - // Given a closed PR whose branch can move while a requested build is running - const {controller, state, publish} = previewService('closed'); - const request = await controller.register(123, 'feature/worker', '100', '1'); - state.refs.set('feature/worker', OTHER_SHA); - const identity = getIdentity(request.sha, '100', '1'); - // When that build publishes its original snapshot - await controller.deploy(123, Number(request.deploymentID), identity, () => publish(identity)); - // Then the preview and its reported URL refer to the pinned build, not the newer branch or main - expect(state.worker).toEqual(IDENTITY); - expect(state.assets).toEqual(IDENTITY); - expect(state.statuses.get(Number(request.deploymentID))).toMatchObject({state: 'success', environment_url: 'https://123.pr-testing.expensify.com'}); - }); - - test('only the newest requested build can publish, regardless of completion order or reruns', async () => { - // Given overlapping builds, including reruns and workflow IDs beyond safe integer precision - for (const [oldRun, oldAttempt, newRun, newAttempt] of [ - ['100', '1', '101', '1'], - ['100', '1', '100', '2'], - ['9007199254740992', '1', '9007199254740993', '1'], - ]) { - const {controller, state, publish} = previewService(); - const older = await controller.register(123, 'feature/worker', oldRun, oldAttempt); - const newer = await controller.register(123, 'feature/worker', newRun, newAttempt); - let oldPublished = false; - // When the older build finishes late or tries to register again - await expect( - controller.deploy(123, Number(older.deploymentID), getIdentity(SHA, oldRun, oldAttempt), () => { - oldPublished = true; - }), - ).rejects.toThrow(); - await expect(controller.register(123, 'feature/worker', oldRun, oldAttempt)).rejects.toThrow(); - const identity = getIdentity(SHA, newRun, newAttempt); - await controller.deploy(123, Number(newer.deploymentID), identity, () => publish(identity)); - // Then the newest build remains served and the old build never publishes or registers again - expect(oldPublished).toBe(false); - expect(state.deployments).toHaveLength(2); - expect(state.worker).toEqual(identity); - expect(state.statuses.get(Number(newer.deploymentID)).state).toBe('success'); + test('verification requires both Worker code and assets to identify the requested build', async () => { + // Given responses from another SHA, run or retry, plus unavailable or malformed endpoints + const invalidResponses = [ + () => Response.json({...IDENTITY, sha: 'b'.repeat(40)}), + () => Response.json({...IDENTITY, runID: '101'}), + () => Response.json({...IDENTITY, runAttempt: '2'}), + () => Response.json(null), + () => new Response('not JSON'), + () => new Response('unavailable', {status: 503}), + () => Promise.reject(new Error('offline')), + ]; + for (const invalidResponse of invalidResponses) { + for (const invalidPath of [STATUS_PATH, '/preview-build.json']) { + // When either endpoint fails to identify this build + const fetchImpl = async (url) => (new URL(url).pathname === invalidPath ? invalidResponse() : Response.json(IDENTITY)); + // Then an older healthy response from the other endpoint cannot make verification succeed + await expect(verifyPreview('123.pr-testing.expensify.com', IDENTITY, fetchImpl, 1)).rejects.toThrow(); } - }); - - test('preserves an older queued main build without replacing a newer preview request', async () => { - // Given a manual preview registered while an earlier push-to-main build was still queued - const {controller, state} = previewService(); - await controller.register(123, 'feature/worker', '101', '1'); - state.refs.set(SHA, SHA); - // When the queued push is finally prepared - const request = await controller.register(123, SHA, '100', '1', true); - // Then its web build can use the pinned SHA without deploying over the newer preview - expect(request).toEqual({sha: SHA, deploymentID: ''}); - expect(state.deployments).toHaveLength(1); - expect(state.deployments[0].payload.runID).toBe('101'); - }); - - test('cannot publish using another PR deployment record', async () => { - // Given a legitimate build record belonging to PR 123 - const {controller, state} = previewService(); - const request = await controller.register(123, 'feature/worker', '100', '1'); - let published = false; - // When that record is supplied for PR 456 - await expect( - controller.deploy(456, Number(request.deploymentID), IDENTITY, () => { - published = true; - }), - ).rejects.toThrow(); - // Then no preview changes and no success is reported - expect(published).toBe(false); - expect(state.worker).toBeNull(); - expect(state.statuses.size).toBe(0); - }); - - test('a failed upload preserves the previous preview without reporting the new build as successful', async () => { - // Given a working older preview and a newly requested build - const {controller, state, publish} = previewService(); - const previous = getIdentity(OTHER_SHA, '99', '1'); - publish(previous); - const request = await controller.register(123, 'feature/worker', '100', '1'); - // When uploading fails before the new version is activated - await expect( - controller.deploy(123, Number(request.deploymentID), IDENTITY, () => { - throw new Error('Upload failed'); - }), - ).rejects.toThrow('Upload failed'); - // Then the old URL still works, but the new build has a failure status rather than a false success - expect(state.worker).toEqual(previous); - expect(state.assets).toEqual(previous); - expect(state.statuses.get(Number(request.deploymentID)).state).toBe('failure'); - }); - - test('verification requires both Worker code and assets to belong to the requested build', async () => { - // Given new Worker code paired with assets from an older build - const {controller, state} = previewService(); - state.worker = IDENTITY; - state.assets = getIdentity(OTHER_SHA, '99', '1'); - // When the deployment is checked and then the matching assets become available - await expect(controller.verify('123.pr-testing.expensify.com', IDENTITY, 1)).rejects.toThrow(); - state.assets = IDENTITY; - // Then only the matching pair passes verification - await expect(controller.verify('123.pr-testing.expensify.com', IDENTITY, 1)).resolves.toBeUndefined(); - }); + } + // When both endpoints return the requested identity + // Then matching code and asset identities pass verification + await expect(verifyPreview('123.pr-testing.expensify.com', IDENTITY, async () => Response.json(IDENTITY), 1)).resolves.toBeUndefined(); }); - test('the CLI provides usage and rejects invalid publishing requests without credentials', () => { + test('the CLI provides usage and rejects invalid publishing requests without GitHub credentials', () => { // Given a user invoking the CLI outside CI - const script = path.join(ROOT, 'scripts/adhocCloudflareWorkerDeployer.ts'); - // When they ask for help or supply unsupported/incomplete commands + const script = path.join(ROOT, 'scripts/deployAdhocCloudflareWorker.ts'); + // When they ask for help or supply invalid or incomplete options expect(spawnSync(process.execPath, [script, '--help']).status).toBe(0); for (const args of [ - ['delete', '--pr-number', '123'], - ['deploy', '--pr-number', '123', '--sha', SHA, '--deployment-id', '42', '--request-attempt', '1'], + ['--pr-number', 'not-a-number', '--sha', SHA, '--artifact-directory', ROOT], + ['--pr-number', '123', '--sha', SHA], ]) { const result = spawnSync(process.execPath, [script, ...args]); // Then invalid requests fail before publishing anything expect(result.status).not.toBe(0); } - // When a branch workflow invokes registration without a run ID - const branchWorkflow = spawnSync(process.execPath, [script, 'register', '--pr-number', '123', '--ref', SHA], { + // When a branch workflow supplies valid options but no run ID + const branchWorkflow = spawnSync(process.execPath, [script, '--pr-number', '123', '--sha', SHA, '--artifact-directory', ROOT], { encoding: 'utf8', env: { ...process.env, - GITHUB_TOKEN: 'test-token', + GITHUB_TOKEN: '', GITHUB_WORKFLOW_REF: 'Expensify/App/.github/workflows/testBuild.yml@refs/heads/preview-experiment', GITHUB_RUN_ID: '', }, }); - // Then it accepts the workflow branch and fails on the missing run ID before making any API request + // Then it accepts the branch and needs only build identity, not GitHub authentication expect(branchWorkflow.status).not.toBe(0); - expect(branchWorkflow.stderr).toContain('GITHUB_RUN_ID is required'); + expect(branchWorkflow.stderr).toContain('An immutable SHA, run ID and run attempt are required'); }); describe('artifact security and compatibility', () => { diff --git a/web/cloudflare-worker/README.md b/web/cloudflare-worker/README.md index 92a33f70419f..5c92bc80808e 100644 --- a/web/cloudflare-worker/README.md +++ b/web/cloudflare-worker/README.md @@ -196,19 +196,18 @@ There are two different kinds of code here: ```text GitHub Actions (deployment tooling, not a Worker): - prepareWeb -> adhocCloudflareWorkerDeployer.ts register -> pin SHA + record build request - buildWeb -> compile that SHA's src/index.ts + build dist/ - deployWebAdHoc -> adhocCloudflareWorkerDeployer.ts deploy -> publish code/assets together and verify + buildWeb -> build web assets and compile the branch Worker, output the checked-out SHA + deployWebAdHoc -> deployAdhocCloudflareWorker.ts -> publish the Preview and verify code/assets identity Cloudflare (request handling): PR hostname -> named Worker Preview -> build-status wrapper -> compiled src/index.ts -> this PR's Static Assets ``` -**The runtime is shared with the main app Worker.** `buildWeb` bundles the existing `web/cloudflare-worker/src/index.ts` from the same App SHA as the web build. CSP, nonces, security headers, Brotli, caching and SPA routing are not reimplemented in the CI tooling. The preview wrapper answers the reserved build-status endpoint and delegates every application request unchanged. `scripts/adhocCloudflareWorkerDeployer.ts` runs only in CI. +**The runtime is shared with the main app Worker.** `buildWeb` bundles the existing `web/cloudflare-worker/src/index.ts` from the same App SHA as the web build. CSP, nonces, security headers, Brotli, caching and SPA routing are not reimplemented in the CI tooling. The preview wrapper answers the reserved build-status endpoint and delegates every application request unchanged. `scripts/deployAdhocCloudflareWorker.ts` runs only in CI. -The deployment CLI runs in Bun with TypeScript, uses the shared `expensify-common/CLI` parser for typed commands/options, and uses `GithubUtils.octokit` for GitHub authentication and REST calls. It uses `Bun.sleep`, `Bun.file` and `Bun.spawn` for local orchestration; Wrangler remains the trusted Node CLI. Pure typed packaging helpers in `web/cloudflare-worker/scripts/prTestingConfig.ts` are shared with the standalone workerd tests without pulling the App CLI/GitHub dependencies into that package. +The deployment CLI runs in Bun with TypeScript and the shared `expensify-common/CLI` parser. It publishes and verifies the Preview without GitHub API calls. Pure typed packaging helpers in `web/cloudflare-worker/scripts/prTestingConfig.ts` are shared with the standalone workerd tests without pulling the App CLI dependencies into that package. -The existing test-build authorization and reviewed-code confirmation still apply. `.github/workflows/buildAdHoc.yml` resolves one immutable App SHA before building and registers the request as a GitHub deployment in `pr-testing-`. `buildWeb.yml` compiles the web app and bundles the Worker from that same SHA without Cloudflare credentials. +The existing test-build authorization and reviewed-code confirmation still apply. Callers resolve the build ref, and `buildWeb.yml` outputs the actual checked-out commit SHA after compiling the web app and bundling the Worker without Cloudflare credentials. Publication stamps the code and assets with that SHA and the current workflow run/attempt. The `deployWebAdHoc` job runs tooling from the caller workflow's SHA, never from `APP_REF`. Authorized users can select a workflow branch for testing without merging it first. Downloaded artifacts cannot supply deployment targets, hooks or privileged bindings. Only compatibility date and flags are accepted from the branch's runtime manifest. `extractPRTestingAssets` copies validated regular files without following links or exposing source maps. Dependencies and build hooks run without Cloudflare credentials. @@ -218,4 +217,4 @@ The generated `previews` configuration supplies staging variables, while Static The trusted wrapper forwards application responses unchanged, including Brotli encoding and CSP nonces. Its reserved `/__expensify_preview` endpoint reports the SHA/run/attempt; `/preview-build.json` in Static Assets carries the same identity. Both must match before the build is reported as deployed. A failed new build never gets a success status just because an old version still serves that hostname. -Manual ad-hoc builds cancel an earlier run for the same App/mobile target, using each repository's PR number or ref. Push-to-main builds use a separate concurrency group and queue every push without cancellation, preserving builds for regression bisection. Deployment identity checks also reject stale artifacts from retries. +Manual ad-hoc builds cancel an earlier run for the same App/mobile target, using each repository's PR number or ref. Push-to-main builds use a separate concurrency group and queue every push without cancellation, preserving builds for regression bisection. Explicitly rerunning an older workflow republishes that run's build, not the latest PR head. diff --git a/web/cloudflare-worker/scripts/prTestingConfig.ts b/web/cloudflare-worker/scripts/prTestingConfig.ts index 030ff51fb695..31eb167b0725 100644 --- a/web/cloudflare-worker/scripts/prTestingConfig.ts +++ b/web/cloudflare-worker/scripts/prTestingConfig.ts @@ -4,18 +4,17 @@ import {tmpdir} from 'node:os'; import path from 'node:path'; type BuildIdentity = {sha: string; runID: string; runAttempt: string}; -type PreviewTarget = {name: string; hostname: string; environment: string}; type WorkerRuntime = {compatibility_date: string; compatibility_flags: string[]}; type ExtractAssets = (archive: string, destination: string) => void | Promise; const STATUS_PATH = '/__expensify_preview'; const WORKER_NAME = 'pr-testing-expensify'; -function getTarget(prNumber: unknown): PreviewTarget { +function getTarget(prNumber: unknown) { if ((typeof prNumber !== 'string' && typeof prNumber !== 'number') || !/^[1-9]\d*$/.test(String(prNumber)) || !Number.isSafeInteger(Number(prNumber))) { throw new Error('A positive numeric PR number is required'); } - return {name: String(prNumber), hostname: `${prNumber}.pr-testing.expensify.com`, environment: `pr-testing-${prNumber}`}; + return {name: String(prNumber), hostname: `${prNumber}.pr-testing.expensify.com`}; } function getPreviewCommand(prNumber: unknown, configPath: string, environment: Record) { @@ -35,10 +34,6 @@ function getIdentity(sha: unknown, runID: unknown, runAttempt: unknown): BuildId return {sha, runID, runAttempt}; } -function isNewerRequest(candidate: BuildIdentity, current: BuildIdentity): boolean { - return BigInt(candidate.runID) > BigInt(current.runID) || (candidate.runID === current.runID && BigInt(candidate.runAttempt) > BigInt(current.runAttempt)); -} - function isWorkerRuntime(runtime: unknown): runtime is WorkerRuntime { return ( typeof runtime === 'object' && @@ -124,5 +119,5 @@ async function prepareDirectory(bundleDirectory: string, assetsArchive: string, return root; } -export {createConfig, createWrapper, getIdentity, getPreviewCommand, getTarget, isNewerRequest, prepareDirectory, STATUS_PATH}; -export type {BuildIdentity, PreviewTarget}; +export {createConfig, createWrapper, getIdentity, getPreviewCommand, getTarget, prepareDirectory, STATUS_PATH}; +export type {BuildIdentity}; From b9a265b88cb6bf5fef2716108eea798c157c3830 Mon Sep 17 00:00:00 2001 From: rory Date: Wed, 4 Nov 2026 11:58:25 -0800 Subject: [PATCH 8/9] docs: trim preview credential documentation --- .github/workflows/README.md | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 712961fdc18b..77fcef190e73 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -96,11 +96,7 @@ The GitHub workflows require a large list of secrets to deploy, notify and test 1. `APPLE_ID` - Required for notarizing desktop code in `desktop/notarize.js` 1. `APPLE_TEAM_ID` - Required for notarizing desktop code in `desktop/notarize.js` 1. `APPLE_ID_PASSWORD` - Required for notarizing desktop code in `desktop/notarize.js` -1. `CLOUDFLARE_TOKEN` - Required for deploying the help site -1. `CLOUDFLARE_WORKERS_DEPLOY_TOKEN` - Deploys the staging/production Workers and named PR Previews under `pr-testing-expensify` -1. `CLOUDFLARE_ACCOUNT_ID` - Cloudflare account used for web deployments and R2 asset storage -1. `CLOUDFLARE_R2_WEB_ASSETS_ACCESS_KEY_ID` and `CLOUDFLARE_R2_WEB_ASSETS_SECRET_ACCESS_KEY` - Store previous web builds in R2 for stale-tab asset recovery -1. `OP_SERVICE_ACCOUNT_TOKEN` - Reads CI credentials from the 1Password vault configured by `OP_VAULT` +1. `CLOUDFLARE_TOKEN` - Purges the help-site Cloudflare cache 1. `APPLE_CONTACT_EMAIL` - Email used for contact between Expensify and Apple for https://appstoreconnect.apple.com/ 1. `APPLE_CONTACT_PHONE` - Phone number used for contact between Expensify and Apple for https://appstoreconnect.apple.com/ 1. `APPLE_DEMO_EMAIL` - Demo account email used for https://appstoreconnect.apple.com/ @@ -112,8 +108,6 @@ We use 1Password for many secrets and in general use two different actions from 1. `1password/install-cli-action` - This action is used to install 1Password cli `op` and is used to grab **files** using the `op read` command. 1. `1password/load-secrets-action` - This action is used to fetch **strings** from 1Password. -PR Previews use the existing `CLOUDFLARE_WORKERS_DEPLOY_TOKEN` repository secret. Their account ID is nonsecret configuration in `buildAdHoc.yml`; ad-hoc jobs do not create Workers or zone routes per PR. - ### Important note about Secrets Secrets are available by default in most workflows. The exception to the rule is callable workflows. If a workflow is triggered by the `workflow_call` event, it will only have access to repo secrets if the workflow that called it passed in the secrets explicitly (for example, using `secrets: inherit`). From d5b56e9d34fe5e9004bbc9bdc2a375d3ef41cfc3 Mon Sep 17 00:00:00 2001 From: rory Date: Thu, 5 Nov 2026 16:09:25 -0800 Subject: [PATCH 9/9] ci: opt into workspace dependencies per job --- .../actions/composite/setupNode/action.yml | 31 ++- .github/workflows/buildAdHoc.yml | 2 + .../workflows/buildVictoryChartRenderer.yml | 2 + .github/workflows/buildWeb.yml | 2 + .github/workflows/bunTests.yml | 2 + .github/workflows/claude-review.yml | 2 + .github/workflows/deployExpensifyHelp.yml | 2 + .github/workflows/knip.yml | 2 + .github/workflows/lint.yml | 2 + .github/workflows/seedCache.yml | 10 +- .github/workflows/typecheck.yml | 2 + tests/tooling/setupNode.test.ts | 208 ++++++++++++++++++ 12 files changed, 263 insertions(+), 4 deletions(-) create mode 100644 tests/tooling/setupNode.test.ts diff --git a/.github/actions/composite/setupNode/action.yml b/.github/actions/composite/setupNode/action.yml index c034613ec498..8cd872411dff 100644 --- a/.github/actions/composite/setupNode/action.yml +++ b/.github/actions/composite/setupNode/action.yml @@ -7,6 +7,10 @@ inputs: description: 'Indicates if node is set up for hybrid app' required: false default: 'false' + WORKSPACES: + description: 'JSON array of npm workspace names or paths to install alongside the root project. Defaults to root dependencies only.' + required: false + default: '[]' SEED_STICKY_DISKS: description: | 'true' only in the seedCache workflow, which installs dependencies and nothing else. @@ -37,11 +41,21 @@ runs: shell: bash run: echo "use-sticky-disk=${{ runner.os == 'Linux' && contains(runner.name, 'blacksmith') }}" >> "$GITHUB_OUTPUT" + - id: workspace-config + shell: bash + env: + WORKSPACES: ${{ inputs.WORKSPACES }} + run: | + workspaces=$(jq -ce 'if type == "array" and all(.[]; type == "string" and length > 0) then sort | unique else error("WORKSPACES must be an array of nonempty workspace names") end' <<< "$WORKSPACES") + echo "workspaces=$workspaces" >> "$GITHUB_OUTPUT" + # Equivalent selections must share a cache even if callers list them in a different order. + echo "cache-key=$(printf '%s' "$workspaces" | shasum -a 256 | cut -d ' ' -f 1)" >> "$GITHUB_OUTPUT" + - id: stickydisk-node-modules if: steps.runner-info.outputs.use-sticky-disk == 'true' uses: useblacksmith/stickydisk@74f3f01ab1392726dd6ee06904f0452b0ec1e151 # v1.6.0 with: - key: ${{ format('{0}-{1}-{2}-node-modules-{3}', github.repository, runner.os, runner.arch, inputs.IS_HYBRID_BUILD == 'true' && hashFiles('normalized-package-lock.json', 'patches/**', 'Mobile-Expensify/patches/**') || hashFiles('normalized-package-lock.json', 'patches/**')) }} + key: ${{ format('{0}-{1}-{2}-node-modules-workspaces-{3}-{4}', github.repository, runner.os, runner.arch, steps.workspace-config.outputs.cache-key, inputs.IS_HYBRID_BUILD == 'true' && hashFiles('normalized-package-lock.json', 'patches/**', 'Mobile-Expensify/patches/**') || hashFiles('normalized-package-lock.json', 'patches/**')) }} path: node_modules commit: ${{ inputs.SEED_STICKY_DISKS == 'true' && 'if-missing' || 'false' }} @@ -67,7 +81,7 @@ runs: uses: actions/cache@9255dc7a253b0ccc959486e2bca901246202afeb with: path: node_modules - key: ${{ format('{0}-node-modules-{1}', runner.os, inputs.IS_HYBRID_BUILD == 'true' && hashFiles('package-lock.json', 'patches/**', 'Mobile-Expensify/patches/**') || hashFiles('package-lock.json', 'patches/**')) }} + key: ${{ format('{0}-node-modules-workspaces-{1}-{2}', runner.os, steps.workspace-config.outputs.cache-key, inputs.IS_HYBRID_BUILD == 'true' && hashFiles('package-lock.json', 'patches/**', 'Mobile-Expensify/patches/**') || hashFiles('package-lock.json', 'patches/**')) }} - id: cache-old-dot-node-modules if: steps.runner-info.outputs.use-sticky-disk != 'true' && inputs.IS_HYBRID_BUILD == 'true' @@ -123,10 +137,21 @@ runs: - name: Install root project node packages if: steps.check-node-modules.outputs.cache-hit != 'true' || (inputs.IS_HYBRID_BUILD == 'true' && steps.check-old-dot-node-modules.outputs.cache-hit != 'true') uses: nick-fields/retry@ad984534de44a9489a53aefd81eb77f87c70dc60 # v4 + env: + WORKSPACES: ${{ steps.workspace-config.outputs.workspaces }} with: timeout_minutes: 30 max_attempts: 3 - command: npm ci + command: | + args=(--workspaces=false) + if [[ "$WORKSPACES" != '[]' ]]; then + # Selecting workspaces excludes the root project unless it is explicitly included. + args=(--include-workspace-root) + while IFS= read -r workspace; do + args+=(--workspace="$workspace") + done < <(jq -r '.[]' <<< "$WORKSPACES") + fi + npm ci "${args[@]}" - name: Add project binaries to PATH shell: bash diff --git a/.github/workflows/buildAdHoc.yml b/.github/workflows/buildAdHoc.yml index 5fcd395ec5ef..78e46d45c4a0 100644 --- a/.github/workflows/buildAdHoc.yml +++ b/.github/workflows/buildAdHoc.yml @@ -127,6 +127,8 @@ jobs: - name: Setup Node uses: ./.github/actions/composite/setupNode + with: + WORKSPACES: '["web/cloudflare-worker"]' - name: Download the matching web assets uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7 diff --git a/.github/workflows/buildVictoryChartRenderer.yml b/.github/workflows/buildVictoryChartRenderer.yml index 868e8988d17b..2ee933f1a66c 100644 --- a/.github/workflows/buildVictoryChartRenderer.yml +++ b/.github/workflows/buildVictoryChartRenderer.yml @@ -31,6 +31,8 @@ jobs: - name: Setup Node uses: ./.github/actions/composite/setupNode + with: + WORKSPACES: '["server/victory-chart-renderer"]' - name: Build victory-chart-renderer Linux x64 binary run: npm run server:vcr:build:linux diff --git a/.github/workflows/buildWeb.yml b/.github/workflows/buildWeb.yml index 6833c7e06706..6a27675dd027 100644 --- a/.github/workflows/buildWeb.yml +++ b/.github/workflows/buildWeb.yml @@ -64,6 +64,8 @@ jobs: - name: Setup Node uses: ./.github/actions/composite/setupNode + with: + WORKSPACES: ${{ inputs.build-preview-worker && '["web/cloudflare-worker"]' || '[]' }} - name: Build web run: | diff --git a/.github/workflows/bunTests.yml b/.github/workflows/bunTests.yml index b9aec8318a6f..d728af5ff7e6 100644 --- a/.github/workflows/bunTests.yml +++ b/.github/workflows/bunTests.yml @@ -38,6 +38,8 @@ jobs: - name: Setup Node uses: ./.github/actions/composite/setupNode + with: + WORKSPACES: '["server/victory-chart-renderer", "web/cloudflare-worker"]' - name: Run Bun tests run: npm run test:bun diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index 0b09c8f85a77..cf711e152011 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -86,6 +86,8 @@ jobs: - name: Setup Node if: steps.skip.outputs.skip != 'true' && (steps.filter.outputs.code == 'true' || steps.filter.outputs.docs == 'true') uses: ./.github/actions/composite/setupNode + with: + WORKSPACES: '["server/victory-chart-renderer", "web/cloudflare-worker"]' - name: Run Claude Code (code) id: code-review diff --git a/.github/workflows/deployExpensifyHelp.yml b/.github/workflows/deployExpensifyHelp.yml index 8979988505b0..3e1c7367b99b 100644 --- a/.github/workflows/deployExpensifyHelp.yml +++ b/.github/workflows/deployExpensifyHelp.yml @@ -42,6 +42,8 @@ jobs: - name: Setup NodeJS uses: ./.github/actions/composite/setupNode + with: + WORKSPACES: '["web/cloudflare-worker"]' - name: Create docs routes file run: ./.github/scripts/createDocsRoutes.sh diff --git a/.github/workflows/knip.yml b/.github/workflows/knip.yml index e876ff860e91..cd608afa4733 100644 --- a/.github/workflows/knip.yml +++ b/.github/workflows/knip.yml @@ -35,6 +35,8 @@ jobs: - name: Setup Node uses: ./.github/actions/composite/setupNode + with: + WORKSPACES: '["server/victory-chart-renderer", "web/cloudflare-worker"]' - name: Run knip on PR run: KNIP=true npx knip --reporter json --no-exit-code > /tmp/knip-pr.json diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 280c1be8e80a..b1b2fbe01337 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -44,6 +44,8 @@ jobs: - name: Setup Node uses: ./.github/actions/composite/setupNode + with: + WORKSPACES: '["server/victory-chart-renderer", "web/cloudflare-worker"]' # oxlint runs JS plugins on one thread (oxc#26621), so OxlintLinter splits them into 4 shards on this 8 vCPU runner. - name: Lint JavaScript and TypeScript with Oxlint diff --git a/.github/workflows/seedCache.yml b/.github/workflows/seedCache.yml index 02b96bd04b20..2159d1718380 100644 --- a/.github/workflows/seedCache.yml +++ b/.github/workflows/seedCache.yml @@ -11,8 +11,15 @@ concurrency: jobs: seedStandalone: - name: Seed standalone node_modules + name: Seed standalone node_modules (${{ matrix.workspaces }}) runs-on: blacksmith-2vcpu-ubuntu-2404 + strategy: + matrix: + workspaces: + - '[]' + - '["web/cloudflare-worker"]' + - '["server/victory-chart-renderer"]' + - '["server/victory-chart-renderer", "web/cloudflare-worker"]' steps: - name: Checkout uses: useblacksmith/checkout@0647fdbab2614a5eb86d2971070e325060d91056 # v1.7.0 @@ -21,6 +28,7 @@ jobs: uses: ./.github/actions/composite/setupNode with: SEED_STICKY_DISKS: 'true' + WORKSPACES: ${{ matrix.workspaces }} seedHybrid: name: Seed hybrid node_modules diff --git a/.github/workflows/typecheck.yml b/.github/workflows/typecheck.yml index c113ac5c96af..2f34ccc53334 100644 --- a/.github/workflows/typecheck.yml +++ b/.github/workflows/typecheck.yml @@ -23,6 +23,8 @@ jobs: - uses: useblacksmith/checkout@0647fdbab2614a5eb86d2971070e325060d91056 # v1.7.0 - uses: ./.github/actions/composite/setupNode + with: + WORKSPACES: '["server/victory-chart-renderer", "web/cloudflare-worker"]' - name: Type check with TypeScript run: npm run typecheck diff --git a/tests/tooling/setupNode.test.ts b/tests/tooling/setupNode.test.ts new file mode 100644 index 000000000000..a3305d781017 --- /dev/null +++ b/tests/tooling/setupNode.test.ts @@ -0,0 +1,208 @@ +/** Keep CI's workspace selections consistent with its installed-dependency caches and consumers. */ +/* eslint-disable no-template-curly-in-string -- These strings assert GitHub Actions expressions, not JavaScript interpolation. */ +// cspell:ignore stickydisk pipefail +import {afterEach, beforeEach, describe, expect, it} from 'bun:test'; + +import {existsSync, mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync} from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +type Step = { + id?: string; + name?: string; + uses?: string; + run?: string; + env?: Record; + with?: Record; +}; + +type Action = { + inputs: Record; + runs: {steps: Step[]}; +}; + +type Workflow = { + jobs: Record}}>; +}; + +const SETUP_NODE = './.github/actions/composite/setupNode'; +const WORKER = 'web/cloudflare-worker'; +const RENDERER = 'server/victory-chart-renderer'; +const WORKSPACE_SELECTIONS = [[], ['worker'], ['renderer'], ['worker', 'renderer']]; + +// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- This fixture is the action YAML checked into this repo. +const action = Bun.YAML.parse(readFileSync('.github/actions/composite/setupNode/action.yml', 'utf8')) as Action; + +function findActionStep(idOrName: string): Step { + const step = action.runs.steps.find((item) => item.id === idOrName || item.name === idOrName); + if (!step) { + throw new Error(`Missing setupNode step: ${idOrName}`); + } + return step; +} + +function readWorkflow(name: string): Workflow { + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- These fixtures are the workflow YAML checked into this repo. + return Bun.YAML.parse(readFileSync(`.github/workflows/${name}.yml`, 'utf8')) as Workflow; +} + +describe('setupNode workspace installs', () => { + let directory: string; + + beforeEach(() => { + directory = mkdtempSync(path.join(os.tmpdir(), 'setup-node-workspaces-')); + }); + + afterEach(() => { + rmSync(directory, {recursive: true, force: true}); + }); + + function configureWorkspaces(input: string) { + const output = path.join(directory, 'github-output'); + writeFileSync(output, ''); + const result = Bun.spawnSync(['/bin/bash', '-e', '-o', 'pipefail', '-c', findActionStep('workspace-config').run ?? ''], { + cwd: directory, + env: {...process.env, WORKSPACES: input, GITHUB_OUTPUT: output}, + }); + const outputs = Object.fromEntries( + readFileSync(output, 'utf8') + .trim() + .split('\n') + .map((line) => { + const separator = line.indexOf('='); + return [line.slice(0, separator), line.slice(separator + 1)]; + }), + ); + return {result, outputs}; + } + + it('defaults to root dependencies without workspace packages', () => { + // Given that most CI jobs do not consume either workspace + // When a caller omits the selection + // Then it must install only the root project's dependencies. + expect(action.inputs.WORKSPACES.default).toBe('[]'); + expect(findActionStep('workspace-config').env?.WORKSPACES).toBe('${{ inputs.WORKSPACES }}'); + expect(findActionStep('Install root project node packages').env?.WORKSPACES).toBe('${{ steps.workspace-config.outputs.workspaces }}'); + }); + + it('normalizes workspace order and duplicates before computing cache keys', () => { + // Given equivalent workspace selections written in different orders + const first = configureWorkspaces('["worker", "renderer", "worker"]'); + + // When the action normalizes them before restoring installed dependencies + const second = configureWorkspaces('["renderer", "worker"]'); + + // Then both must select the same packages and use the same cache. + expect(first.result.exitCode).toBe(0); + expect(second.result.exitCode).toBe(0); + expect(first.outputs.workspaces).toBe('["renderer","worker"]'); + expect(first.outputs['cache-key']).toBe(second.outputs['cache-key']); + }); + + it('uses a distinct cache key for each workspace selection', () => { + // Given root-only, Worker-only, renderer-only and combined installs + // When the action computes their keys + const keys = WORKSPACE_SELECTIONS.map((selection) => { + const {result, outputs} = configureWorkspaces(JSON.stringify(selection)); + expect(result.exitCode).toBe(0); + expect(outputs['cache-key']).toMatch(/^[a-f0-9]{64}$/); + return outputs['cache-key']; + }); + + // Then no selection can restore another selection's incomplete install. + expect(new Set(keys).size).toBe(WORKSPACE_SELECTIONS.length); + }); + + it.each(['not json', 'false', '{}', '[false]', '[""]'])('rejects invalid workspace selections: %s', (input) => { + // Given a malformed selection that must not silently drop required packages + // When the configuration step runs before any cache is restored + const {result} = configureWorkspaces(input); + + // Then the action must fail rather than guess an install mode. + expect(result.exitCode).not.toBe(0); + }); + + it.each(['stickydisk-node-modules', 'cache-node-modules'])('keys %s by the normalized workspace selection', (id) => { + // Given that both cache strategies must distinguish subsets even for hybrid jobs + // When their installed-dependency caches are restored + const step = findActionStep(id); + + // Then the selection hash must be in each key, without a fallback to another selection. + expect(step.with?.key).toContain('steps.workspace-config.outputs.cache-key'); + expect(step.with?.key).toContain('inputs.IS_HYBRID_BUILD'); + expect(step.with?.['restore-keys']).toBeUndefined(); + }); + + it.each(WORKSPACE_SELECTIONS.map((selection) => [selection]))('installs root dependencies and only the selected workspaces: %j', (selection) => { + // Given an offline root project and two workspaces with dependencies that the root does not own + const writePackage = (relativePath: string, contents: Record) => { + const target = path.join(directory, relativePath); + mkdirSync(target, {recursive: true}); + writeFileSync(path.join(target, 'package.json'), JSON.stringify(contents)); + }; + writePackage('.', {name: 'setup-node-fixture', private: true, workspaces: ['worker', 'renderer'], devDependencies: {root: 'file:./root-dependency'}}); + writePackage('root-dependency', {name: 'root', version: '1.0.0'}); + writePackage('worker', {name: 'fixture-worker', version: '1.0.0', devDependencies: {tool: 'file:../worker-dependency'}}); + writePackage('worker-dependency', {name: 'tool', version: '1.0.0'}); + writePackage('renderer', {name: 'fixture-renderer', version: '1.0.0', devDependencies: {chart: 'file:../renderer-dependency'}}); + writePackage('renderer-dependency', {name: 'chart', version: '1.0.0'}); + const env = {...process.env, NPM_CONFIG_OFFLINE: 'true', NPM_CONFIG_CACHE: path.join(directory, '.npm')}; + const lockfile = Bun.spawnSync(['npm', 'install', '--package-lock-only', '--ignore-scripts'], {cwd: directory, env}); + expect(lockfile.exitCode).toBe(0); + + // When the actual action scripts configure and install the requested subset + const {result: config, outputs} = configureWorkspaces(JSON.stringify(selection)); + expect(config.exitCode).toBe(0); + const command = findActionStep('Install root project node packages').with?.command; + expect(command).toBeDefined(); + const install = Bun.spawnSync(['/bin/bash', '-e', '-o', 'pipefail', '-c', command ?? ''], {cwd: directory, env: {...env, WORKSPACES: outputs.workspaces}}); + expect(install.stderr.toString()).not.toContain('npm error'); + expect(install.exitCode).toBe(0); + + // Then root tooling must always remain available, without dependencies from unselected workspaces. + expect(existsSync(path.join(directory, 'node_modules/root'))).toBe(true); + expect(existsSync(path.join(directory, 'node_modules/fixture-worker'))).toBe(selection.includes('worker')); + expect(existsSync(path.join(directory, 'node_modules/tool'))).toBe(selection.includes('worker')); + expect(existsSync(path.join(directory, 'node_modules/fixture-renderer'))).toBe(selection.includes('renderer')); + expect(existsSync(path.join(directory, 'node_modules/chart'))).toBe(selection.includes('renderer')); + }); + + it.each([ + ['buildAdHoc', 'deployWebAdHoc', [WORKER]], + ['buildVictoryChartRenderer', 'build', [RENDERER]], + ['deployExpensifyHelp', 'build', [WORKER]], + ['bunTests', 'test', [RENDERER, WORKER]], + ['typecheck', 'typecheck', [RENDERER, WORKER]], + ['knip', 'knip-compare', [RENDERER, WORKER]], + ['lint', 'lint', [RENDERER, WORKER]], + ['claude-review', 'review', [RENDERER, WORKER]], + ])('selects the required workspaces for %s / %s', (workflowName, job, selection) => { + // Given a job that imports, builds, tests or analyzes workspace code + // When its setupNode call is read from the actual workflow + const step = readWorkflow(workflowName).jobs[job].steps.find((item) => item.uses === SETUP_NODE); + + // Then it must explicitly select the packages needed for that work. + expect(step).toBeDefined(); + expect(JSON.parse(step?.with?.WORKSPACES ?? action.inputs.WORKSPACES.default)).toEqual(selection); + }); + + it('selects the Worker only when the web build also bundles a Preview Worker', () => { + // Given that ordinary web bundling does not use Wrangler, but Preview Worker bundling does + // When the reusable web workflow selects dependencies + const step = readWorkflow('buildWeb').jobs.build.steps.find((item) => item.uses === SETUP_NODE); + + // Then it must request only the Worker workspace when that bundling is enabled. + expect(step?.with?.WORKSPACES).toBe("${{ inputs.build-preview-worker && '[\"web/cloudflare-worker\"]' || '[]' }}"); + }); + + it('seeds every standalone selection and the root-only hybrid cache', () => { + // Given that ordinary CI jobs read sticky disks without persisting their installs + // When the cache-seeding workflow runs + const {jobs} = readWorkflow('seedCache'); + + // Then each subset needs a matching seed, while native hybrid builds need no workspace packages. + expect(jobs.seedStandalone.strategy?.matrix.workspaces.map((value) => JSON.parse(value) as unknown)).toEqual([[], [WORKER], [RENDERER], [RENDERER, WORKER]]); + expect(jobs.seedStandalone.steps.find((item) => item.uses === SETUP_NODE)?.with?.WORKSPACES).toBe('${{ matrix.workspaces }}'); + expect(jobs.seedHybrid.steps.find((item) => item.uses === SETUP_NODE)?.with?.WORKSPACES ?? action.inputs.WORKSPACES.default).toBe('[]'); + }); +});