From d20cb69c48af2c7abb0651d02499025fe6899f5a Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Sat, 26 Sep 2026 02:53:37 -0700 Subject: [PATCH] Optimize CI follow-up workflows (#23190) Co-authored-by: m4air --- .github/workflows/mobile.yml | 23 +++ .github/workflows/pr.yml | 7 +- .github/workflows/release-cut.yml | 10 + .../ci-dependency-download-cache.test.mjs | 57 ++++++ .../mobile-recording-pin-checkout.test.mjs | 93 +++++++++ config/scripts/mobile-release-check-scope.mjs | 31 +++ .../mobile-release-check-scope.test.mjs | 191 ++++++++++++++++++ .../agent-status-store-in-place.test.ts | 7 +- 8 files changed, 413 insertions(+), 6 deletions(-) create mode 100644 config/scripts/mobile-recording-pin-checkout.test.mjs create mode 100644 config/scripts/mobile-release-check-scope.mjs create mode 100644 config/scripts/mobile-release-check-scope.test.mjs diff --git a/.github/workflows/mobile.yml b/.github/workflows/mobile.yml index 097ad813962..8382bb73b5d 100644 --- a/.github/workflows/mobile.yml +++ b/.github/workflows/mobile.yml @@ -37,6 +37,9 @@ on: - '.github/workflows/mobile.yml' - '.github/actions/install-node-dependencies/**' - '.github/workflows/mobile-ios-release.yml' + - 'config/scripts/mobile-release-check-scope*' + - 'config/scripts/pr-code-change-scope.mjs' + - 'config/scripts/mobile-recording-pin-checkout.test.mjs' # Why main too: a behaviour-change branch legitimately pins its own last fenced commit, and that # commit only stops being reachable when the branch squash-merges. The pull_request run cannot # see that; this one is where the pin guard finds it. @@ -72,6 +75,8 @@ jobs: steps: - name: Checkout uses: actions/checkout@v6 + with: + fetch-depth: 2 - uses: ./.github/actions/install-node-dependencies with: @@ -79,10 +84,23 @@ jobs: pnpm-lock.yaml mobile/pnpm-lock.yaml + - name: Detect Ruby release inputs + id: ruby-scope + shell: bash + working-directory: . + run: | + # Keep deletions when release files move into an application directory. + if ! git diff --name-only --no-renames -z HEAD^1 HEAD > "$RUNNER_TEMP/mobile-release-changes"; then + echo 'should_run=true' >> "$GITHUB_OUTPUT" + elif ! node config/scripts/mobile-release-check-scope.mjs "$RUNNER_TEMP/mobile-release-changes"; then + echo 'should_run=true' >> "$GITHUB_OUTPUT" + fi + # bundler-cache installs mobile/Gemfile.lock, so this job is also what # proves the pinned fastlane the release workflow depends on still # resolves — before a release run finds out. - name: Setup Ruby and fastlane + if: steps.ruby-scope.outputs.should_run != 'false' uses: ruby/setup-ruby@v1 with: ruby-version: '3.3' @@ -112,9 +130,11 @@ jobs: run: pnpm test - name: Test iOS release version resolution + if: steps.ruby-scope.outputs.should_run != 'false' run: ruby fastlane/ios_release_version_test.rb - name: Test TestFlight lane arguments + if: steps.ruby-scope.outputs.should_run != 'false' run: ruby fastlane/fastfile_testflight_arguments_test.rb # Why: nothing else in CI loads the Fastfile, so a syntax error, a broken @@ -123,6 +143,7 @@ jobs: # loads and lists, so it needs no App Store Connect credentials and makes # no network calls to Apple. - name: Smoke-check the Fastfile + if: steps.ruby-scope.outputs.should_run != 'false' env: FASTLANE_SKIP_UPDATE_CHECK: '1' FASTLANE_OPT_OUT_USAGE: '1' @@ -151,6 +172,8 @@ jobs: # answer at all rather than reporting a pass it has no evidence for -- and the pinned tree # below has to be checkable out. fetch-depth: 0 + # Ancestry needs commits; the pinned worktree fetches its historical blobs on demand. + filter: blob:none - uses: ./.github/actions/install-node-dependencies with: diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 0e987edcd43..26cafeab46c 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -899,14 +899,15 @@ jobs: persist-credentials: false - name: Cache electron-builder downloads - uses: actions/cache@v5 + uses: actions/cache/restore@v5 with: path: | ~\AppData\Local\electron\Cache ~\AppData\Local\electron-builder\Cache - key: electron-builder-windows-${{ hashFiles('pnpm-lock.yaml') }} + # Release builds seed this exact path set; PR-local copies cannot serve other PRs. + key: electron-builder-win-${{ hashFiles('pnpm-lock.yaml') }} restore-keys: | - electron-builder-windows- + electron-builder-win- # Why persist-native-cache false: this job later rebuilds the same path for # Electron. A post-job save would store the Electron ABI under the Node key. diff --git a/.github/workflows/release-cut.yml b/.github/workflows/release-cut.yml index b2ac03e2b08..2490d39af39 100644 --- a/.github/workflows/release-cut.yml +++ b/.github/workflows/release-cut.yml @@ -1314,6 +1314,16 @@ jobs: restore-keys: | electron-builder-${{ matrix.platform }}- + # PRs cache tools separately from Electron; identical paths preserve their cache version. + - name: Seed shared Linux packaging downloads + if: matrix.platform == 'linux-x64' && github.ref == 'refs/heads/main' + uses: actions/cache@v5 + with: + path: ~/.cache/electron-builder + key: electron-builder-linux-${{ hashFiles('pnpm-lock.yaml') }} + # The release cache above restores the same tools, so this only needs to save on a miss. + lookup-only: true + # Why: pnpm install triggers electron's postinstall, which downloads the # Electron binary from GitHub release assets. GitHub's download CDN # occasionally returns 504s that fail the whole release. Retry on diff --git a/config/scripts/ci-dependency-download-cache.test.mjs b/config/scripts/ci-dependency-download-cache.test.mjs index 5b516202be7..fad103d8fa6 100644 --- a/config/scripts/ci-dependency-download-cache.test.mjs +++ b/config/scripts/ci-dependency-download-cache.test.mjs @@ -50,6 +50,63 @@ describe('CI dependency download caches', () => { const saves = action.runs.steps.filter((step) => step.uses === 'actions/cache/save@v5') expect(saves).toEqual([]) }) + + it('restores Windows packaging downloads from the release cache without a PR upload', () => { + const packaging = workflow('pr').jobs.package_windows + const restore = packaging.steps.find((step) => step.name === 'Cache electron-builder downloads') + const release = workflow('release-cut').jobs.build + const windows = release.strategy.matrix.include.find((entry) => entry.platform === 'win') + const save = release.steps.find((step) => step.name === 'Cache electron-builder downloads') + + expect(packaging['runs-on']).toBe(windows.os) + expect(restore.uses).toBe('actions/cache/restore@v5') + // Cache versions include the path list, so matching key strings alone cannot prove reuse. + expect(restore.with.path).toBe(windows.eb_cache_path) + expect(restore.with.key).toBe(save.with.key.replace('${{ matrix.platform }}', 'win')) + expect(restore.with['restore-keys']).toBe( + save.with['restore-keys'].replace('${{ matrix.platform }}', 'win') + ) + expect(save.uses).toBe('actions/cache@v5') + expect(save.with.path).toBe('${{ matrix.eb_cache_path }}') + for (const name of ['dev-channel-win-build', 'windows-signing-rehearsal']) { + const writer = Object.values(workflow(name).jobs) + .flatMap((job) => job.steps ?? []) + .find((step) => step.name === 'Cache electron-builder downloads') + expect(writer.uses, name).toBe('actions/cache@v5') + expect(writer.with.path, name).toBe(restore.with.path) + expect(writer.with.key, name).toBe(restore.with.key) + expect(writer.with['restore-keys'], name).toBe(restore.with['restore-keys']) + } + }) + + it('seeds the existing Linux PR tool cache from successful main x64 release builds', () => { + const packaging = workflow('pr').jobs.package + const consumer = packaging.steps.find( + (step) => step.name === 'Cache electron-builder downloads' + ) + const release = workflow('release-cut').jobs.build + const combined = release.steps.find((step) => step.name === 'Cache electron-builder downloads') + const writer = release.steps.find( + (step) => step.name === 'Seed shared Linux packaging downloads' + ) + const linux = release.strategy.matrix.include.find((entry) => entry.platform === 'linux-x64') + + expect(packaging['runs-on']).toBe(linux.os) + expect(writer.if).toBe("matrix.platform == 'linux-x64' && github.ref == 'refs/heads/main'") + expect(writer.uses).toBe('actions/cache@v5') + expect(writer.with.path).toBe(consumer.with.path) + expect(writer.with.key).toBe(consumer.with.key) + expect(writer.with['restore-keys']).toBeUndefined() + expect(writer.with['lookup-only']).toBe(true) + expect(release.steps.indexOf(writer)).toBeGreaterThan(release.steps.indexOf(combined)) + // Retain PR fallback saves until a successful release seeds the default-branch entry. + expect(consumer.uses).toBe('actions/cache@v5') + expect(combined.uses).toBe('actions/cache@v5') + expect(linux.eb_cache_path.trim().split('\n')).toEqual([ + '~/.cache/electron', + '~/.cache/electron-builder' + ]) + }) }) describe('release install targets', () => { diff --git a/config/scripts/mobile-recording-pin-checkout.test.mjs b/config/scripts/mobile-recording-pin-checkout.test.mjs new file mode 100644 index 00000000000..f45db12bdbe --- /dev/null +++ b/config/scripts/mobile-recording-pin-checkout.test.mjs @@ -0,0 +1,93 @@ +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { pathToFileURL } from 'node:url' +import { expect, it } from 'vitest' +import { parse } from 'yaml' +import { runProcessSync } from './script-child-process.mjs' + +const workflow = parse( + readFileSync(new URL('../../.github/workflows/mobile.yml', import.meta.url), 'utf8') +) + +it('keeps full ancestry and credentials for lazy pinned-tree reads', () => { + const job = workflow.jobs['recording-pin'] + const checkout = job.steps.find((step) => step.uses?.startsWith('actions/checkout@')) + expect(checkout.with['fetch-depth']).toBe(0) + expect(checkout.with.filter).toBe('blob:none') + expect(checkout.with['persist-credentials']).not.toBe(false) + expect(job.if).toBeUndefined() + expect(workflow.on.push.branches).toEqual(['main']) + expect(workflow.concurrency.group).toContain('github.sha') + expect(job.steps.find((step) => step.name === 'Check the recording pin is reachable').run).toBe( + 'pnpm exec tsx scripts/rpc-recording-pin-guard.mts ancestry' + ) + const reproduce = job.steps.find( + (step) => step.name === 'Reproduce the corpus from the pinned tree' + ) + expect(reproduce.run).toContain('reproduce --if-changed-since "$PIN_GUARD_BASE"') + expect(reproduce.run).toContain( + 'else\n pnpm exec tsx scripts/rpc-recording-pin-guard.mts reproduce\nfi' + ) +}) + +it('retains ancestry while fetching a missing pinned blob for a detached worktree', () => { + const directory = mkdtempSync(join(tmpdir(), 'mobile-pin-checkout-')) + const source = join(directory, 'source') + const checkout = join(directory, 'checkout') + const pinnedTree = join(directory, 'pinned-tree') + const git = (cwd, ...args) => { + const result = runProcessSync({ program: 'git', args, cwd }) + expect(result.code, result.stderr).toBe(0) + return result.stdout.trim() + } + try { + git(directory, 'init', '--quiet', source) + git(source, 'symbolic-ref', 'HEAD', 'refs/heads/main') + git(source, 'config', 'user.name', 'Pin checkout fixture') + git(source, 'config', 'user.email', 'pin-checkout@example.invalid') + git(source, 'config', 'uploadpack.allowFilter', 'true') + git(source, 'config', 'uploadpack.allowAnySHA1InWant', 'true') + const corpus = 'mobile/rpc-foundation/goldens/fixture.json' + mkdirSync(join(source, 'mobile/rpc-foundation/goldens'), { recursive: true }) + const original = '{"baseline":"original historical recording"}\n' + const commit = () => { + git(source, 'add', '-A') + git(source, '-c', 'commit.gpgsign=false', 'commit', '--quiet', '-m', 'recording') + return git(source, 'rev-parse', 'HEAD') + } + writeFileSync(join(source, corpus), original) + const baseline = commit() + const oldBlob = git(source, 'rev-parse', `${baseline}:${corpus}`) + writeFileSync(join(source, corpus), '{"baseline":"current recording"}\n') + commit() + git( + directory, + 'clone', + '--filter=blob:none', + '--no-checkout', + '--single-branch', + '--no-tags', + pathToFileURL(source).href, + checkout + ) + git(checkout, 'checkout', '--quiet', '--force', 'main') + + expect(git(checkout, 'rev-parse', '--is-shallow-repository')).toBe('false') + expect(git(checkout, 'rev-list', '--count', 'HEAD')).toBe('2') + git(checkout, 'merge-base', '--is-ancestor', baseline, 'HEAD') + expect(git(checkout, 'rev-list', '--objects', '--missing=print', 'HEAD')).toContain( + `?${oldBlob}` + ) + + git(checkout, 'worktree', 'add', '--detach', pinnedTree, baseline) + + expect(readFileSync(join(pinnedTree, corpus), 'utf8')).toBe(original) + expect(git(checkout, 'rev-list', '--objects', '--missing=print', 'HEAD')).not.toContain( + `?${oldBlob}` + ) + git(checkout, 'worktree', 'remove', '--force', pinnedTree) + } finally { + rmSync(directory, { recursive: true, force: true }) + } +}) diff --git a/config/scripts/mobile-release-check-scope.mjs b/config/scripts/mobile-release-check-scope.mjs new file mode 100644 index 00000000000..435d3e6fccc --- /dev/null +++ b/config/scripts/mobile-release-check-scope.mjs @@ -0,0 +1,31 @@ +import { appendFileSync, readFileSync } from 'node:fs' +import { pathToFileURL } from 'node:url' +import { isDocsOnlyPath } from './pr-code-change-scope.mjs' + +const APPLICATION_PREFIXES = ['src/', 'mobile/app/', 'mobile/src/'] + +export function shouldRunMobileReleaseChecks(files) { + return ( + files.length === 0 || + files.some( + (file) => + !isDocsOnlyPath(file) && + file !== 'mobile/README.md' && + !file.startsWith('mobile/docs/') && + !( + APPLICATION_PREFIXES.some((prefix) => file.startsWith(prefix)) && + /\.(?:[cm]?[jt]sx?|css|md)$/.test(file) + ) + ) + ) +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + const input = readFileSync(process.argv[2], 'utf8') + const files = input.endsWith('\0') ? input.slice(0, -1).split('\0') : [] + const shouldRun = shouldRunMobileReleaseChecks(files) + console.log( + shouldRun ? 'Checking Ruby release inputs.' : 'Only application or documentation changed.' + ) + appendFileSync(process.env.GITHUB_OUTPUT, `should_run=${shouldRun}\n`) +} diff --git a/config/scripts/mobile-release-check-scope.test.mjs b/config/scripts/mobile-release-check-scope.test.mjs new file mode 100644 index 00000000000..faf90039feb --- /dev/null +++ b/config/scripts/mobile-release-check-scope.test.mjs @@ -0,0 +1,191 @@ +import { copyFileSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { dirname, join, matchesGlob, resolve } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { parse } from 'yaml' +import { runProcessSync } from './script-child-process.mjs' +import { shouldRunMobileReleaseChecks } from './mobile-release-check-scope.mjs' + +const root = resolve(import.meta.dirname, '../..') +const workflow = parse(readFileSync(join(root, '.github/workflows/mobile.yml'), 'utf8')) +const steps = workflow.jobs.verify.steps +const detector = steps.find((step) => step.id === 'ruby-scope') +const directories = [] + +afterEach(() => { + for (const directory of directories.splice(0)) { + rmSync(directory, { recursive: true, force: true }) + } +}) + +it.each([ + 'src/main/runtime/rpc/dispatcher.ts', + 'src/shared/rpc-contract/params.ts', + 'mobile/src/session/session.test.ts', + 'mobile/app/index.tsx', + 'mobile/src/theme.css', + 'mobile/src/session/README.md', + 'mobile/docs/release.md', + 'mobile/README.md', + 'docs/reference/mobile.md' +])('skips Ruby checks for application/documentation-only changes: %s', (file) => { + expect(shouldRunMobileReleaseChecks([file])).toBe(false) +}) + +it.each([ + 'mobile/fastlane/Fastfile', + 'mobile/fastlane/Appfile', + 'mobile/fastlane/ios_release_version.rb', + 'mobile/fastlane/ios_release_version_test.rb', + 'mobile/Gemfile', + 'mobile/Gemfile.lock', + 'mobile/.ruby-version', + 'mobile/.bundle/config', + 'mobile/app.json', + 'mobile/app.config.ts', + 'mobile/ios/Podfile', + 'mobile/package.json', + 'mobile/pnpm-lock.yaml', + 'mobile/scripts/release.mts', + 'mobile/src/release.rb', + 'mobile/src/release.json', + 'mobile/new-toolchain/input', + 'package.json', + 'pnpm-lock.yaml', + '.github/workflows/mobile.yml', + '.github/workflows/mobile-ios-release.yml', + '.github/actions/install-node-dependencies/action.yml', + 'config/scripts/mobile-release-check-scope.mjs', + 'config/scripts/mobile-release-check-scope.test.mjs' +])('retains Ruby release coverage for changed or unknown inputs: %s', (file) => { + expect(shouldRunMobileReleaseChecks([file])).toBe(true) + expect(shouldRunMobileReleaseChecks(['mobile/src/view.tsx', file])).toBe(true) +}) + +it('runs Ruby checks when the changed-file evidence is empty', () => { + expect(shouldRunMobileReleaseChecks([])).toBe(true) +}) + +it('gates only Ruby steps and keeps ordinary mobile validation unconditional', () => { + expect(steps[0].with['fetch-depth']).toBe(2) + expect(detector['working-directory']).toBe('.') + expect(steps.indexOf(detector)).toBeGreaterThan( + steps.findIndex((step) => step.uses === './.github/actions/install-node-dependencies') + ) + const gated = steps.filter((step) => step.if !== undefined) + expect(gated.map((step) => step.name)).toEqual([ + 'Setup Ruby and fastlane', + 'Test iOS release version resolution', + 'Test TestFlight lane arguments', + 'Smoke-check the Fastfile' + ]) + for (const step of gated) { + expect(step.if).toBe("steps.ruby-scope.outputs.should_run != 'false'") + } + for (const name of [ + 'Typecheck', + 'Typecheck tests (ratchet)', + 'Test', + 'Lint', + 'Check formatting' + ]) { + expect(steps.find((step) => step.name === name)?.if).toBeUndefined() + expect(steps.some((step) => step.name === name)).toBe(true) + } + for (const file of [ + 'config/scripts/mobile-release-check-scope.mjs', + 'config/scripts/mobile-release-check-scope.test.mjs', + 'config/scripts/pr-code-change-scope.mjs' + ]) { + expect(workflow.on.pull_request.paths.some((pattern) => matchesGlob(file, pattern))).toBe(true) + } + expect(workflow.jobs.verify.env.BUNDLE_FROZEN).toBe('true') + expect(steps.find((step) => step.name === 'Setup Ruby and fastlane').with['bundler-cache']).toBe( + true + ) + expect(steps.find((step) => step.name === 'Smoke-check the Fastfile').run).toBe( + 'bundle exec fastlane lanes' + ) +}) + +function fixture() { + const directory = mkdtempSync(join(tmpdir(), 'mobile-release-scope-')) + directories.push(directory) + const git = (...args) => { + const result = runProcessSync({ program: 'git', args, cwd: directory }) + expect(result.code, result.stderr).toBe(0) + return result.stdout.trim() + } + git('init', '--quiet') + git('symbolic-ref', 'HEAD', 'refs/heads/main') + git('config', 'user.name', 'Workflow fixture') + git('config', 'user.email', 'workflow@example.invalid') + const write = (file, content) => { + mkdirSync(dirname(join(directory, file)), { recursive: true }) + writeFileSync(join(directory, file), content) + } + write('mobile/src/view.tsx', 'export const view = 1\n') + write('mobile/fastlane/Fastfile', 'default_platform(:ios)\n') + mkdirSync(join(directory, 'config/scripts'), { recursive: true }) + for (const file of [ + 'package.json', + 'config/scripts/pr-code-change-scope.mjs', + 'config/scripts/mobile-release-check-scope.mjs' + ]) { + copyFileSync(join(root, file), join(directory, file)) + } + const commit = () => { + git('add', '-A') + git( + '-c', + 'commit.gpgsign=false', + '-c', + 'core.hooksPath=/dev/null', + 'commit', + '--quiet', + '-m', + 'fixture' + ) + } + commit() + const detect = () => { + const output = join(directory, 'github-output') + const result = runProcessSync({ + program: 'bash', + args: ['-e', '-c', detector.run], + cwd: directory, + env: { ...process.env, GITHUB_OUTPUT: output, RUNNER_TEMP: directory } + }) + expect(result.code, result.stderr).toBe(0) + return readFileSync(output, 'utf8') + } + return { directory, git, write, commit, detect } +} + +describe.skipIf(process.platform === 'win32')('the Linux workflow detector command', () => { + it('skips Ruby after a source-only commit', () => { + const repo = fixture() + repo.write('mobile/src/view.tsx', 'export const view = 2\n') + repo.commit() + expect(repo.detect()).toBe('should_run=false\n') + }) + + it('keeps a deleted release input when a rename moves it into an excluded directory', () => { + const repo = fixture() + repo.git('mv', 'mobile/fastlane/Fastfile', 'mobile/src/old-release.md') + repo.commit() + expect(repo.detect()).toBe('should_run=true\n') + }) + + it('runs Ruby when the merge parent is unavailable', () => { + expect(fixture().detect()).toBe('should_run=true\n') + }) + + it('runs Ruby when the classifier cannot execute', () => { + const repo = fixture() + repo.write('mobile/src/view.tsx', 'export const view = 2\n') + repo.commit() + rmSync(join(repo.directory, 'config/scripts/mobile-release-check-scope.mjs')) + expect(repo.detect()).toBe('should_run=true\n') + }) +}) diff --git a/src/shared/agent-status-store-in-place.test.ts b/src/shared/agent-status-store-in-place.test.ts index e67da48dfdf..efc999aef56 100644 --- a/src/shared/agent-status-store-in-place.test.ts +++ b/src/shared/agent-status-store-in-place.test.ts @@ -168,9 +168,10 @@ describe('AgentStatusStore applied in place', () => { for (const parent of parents) { expect(store.getChildren(parent)).toEqual(oracle.getChildren(parent)) } - for (const id of CHILD_IDS) { - expect(store.getAliasesForChild(id)).toEqual(oracle.getAliasesForChild(id)) - } + // Keep every child-id read in CHILD_IDS order while comparing one aggregate. + const aliasesByChildInIdOrder = CHILD_IDS.map((id) => store.getAliasesForChild(id)) + const oracleAliasesByChildInIdOrder = CHILD_IDS.map((id) => oracle.getAliasesForChild(id)) + expect(aliasesByChildInIdOrder).toEqual(oracleAliasesByChildInIdOrder) mostAliases = Math.max(mostAliases, oracle.state().aliases.size) const probe = mutation.aliases ?? [] expect(store.resolveChildAliases(probe)).toEqual(oracle.resolveChildAliases(probe))