diff --git a/.github/workflows/content-sync.yml b/.github/workflows/content-sync.yml index 626710b..5108821 100644 --- a/.github/workflows/content-sync.yml +++ b/.github/workflows/content-sync.yml @@ -31,6 +31,17 @@ jobs: - run: pnpm install --frozen-lockfile + # Re-encode oversized images in place BEFORE upload, so the optimised bytes + # are what gets pushed to Blob (and committed back below). The local + # `/publish-content` flow also optimises, but contributors can bypass it + # (raw multi-MB images shipped straight to Blob in #73); this makes + # optimisation hold regardless of how the PR was authored. Scopes to assets + # changed vs the base ref because the CI working tree is clean. + - name: Optimize changed images + run: | + git fetch origin ${{ github.base_ref }} + node scripts/optimize-assets.js --write --base origin/${{ github.base_ref }} + - name: Upload new assets to Vercel Blob id: upload env: @@ -59,7 +70,7 @@ jobs: - name: Validate content run: pnpm validate - - name: Commit back rewritten URLs + - name: Commit back optimised images and rewritten URLs id: commit_back env: GH_PAT: ${{ secrets.OCOBO_POST_CLI }} @@ -68,7 +79,7 @@ jobs: git config user.email "github-actions[bot]@users.noreply.github.com" if ! git diff --quiet; then git add -A - git commit -m "chore(assets): rewrite local paths to Vercel Blob URLs [skip ci]" + git commit -m "chore(assets): optimise images and rewrite local paths to Vercel Blob URLs [skip ci]" git push https://x-access-token:${GH_PAT}@github.com/${{ github.repository }}.git HEAD:${{ github.head_ref }} echo "new_sha=$(git rev-parse HEAD)" >> $GITHUB_OUTPUT echo "pushed=true" >> $GITHUB_OUTPUT diff --git a/docs/asset-management.md b/docs/asset-management.md index 9d0f9d6..ddc3849 100644 --- a/docs/asset-management.md +++ b/docs/asset-management.md @@ -8,7 +8,7 @@ - **URLs**: markdown files are rewritten to Blob URLs: `https://[blob-id].vercel.app/content/posts/my-post/image.png`. - **Website**: the main website fetches markdown from this repo; images are served from the CDN. - **Performance**: no website rebuilds needed when adding or changing assets. -- **Optimisation**: `/publish-content` runs `pnpm optimize-assets:write` over the branch's new/changed images before upload, re-encoding oversized ones (> 400 KB) in place. This is local-only — there is no Vercel Blob CI step for re-encoding. +- **Optimisation**: oversized images (> 400 KB) are re-encoded in place. `/publish-content` runs `pnpm optimize-assets:write` locally before upload, **and** the `content-sync` CI workflow re-optimises the branch's changed images before pushing them to Blob — so optimisation holds even when a PR is authored outside the content skills. ## Setup diff --git a/scripts/__tests__/optimize-assets.test.js b/scripts/__tests__/optimize-assets.test.js index 76d9ed0..4aad078 100644 --- a/scripts/__tests__/optimize-assets.test.js +++ b/scripts/__tests__/optimize-assets.test.js @@ -11,6 +11,7 @@ import { optimizeBuffer, parseArgs, resolveChangedPaths, + resolveChangedPathsAgainstBase, resolvePaths, resolveScope, run, @@ -217,6 +218,55 @@ describe('resolveChangedPaths', () => { }); }); +describe('resolveChangedPathsAgainstBase', () => { + let rootDir; + + const git = (cmd) => + execSync(`git ${cmd}`, { cwd: rootDir, stdio: ['ignore', 'pipe', 'ignore'] }); + + beforeEach(async () => { + rootDir = await mkdtemp(join(tmpdir(), 'optimize-assets-base-')); + git('init -q -b main'); + git('config user.email "test@example.com"'); + git('config user.name "Test"'); + git('commit --allow-empty -q -m "init"'); + await mkdir(join(rootDir, 'assets', 'posts'), { recursive: true }); + await writeFile(join(rootDir, 'assets', 'posts', 'base.png'), 'base'); + git('add -A'); + git('commit -q -m "base asset"'); + }); + + afterEach(async () => { + await rm(rootDir, { recursive: true, force: true }); + }); + + it('returns image paths changed between base and HEAD (committed, not working tree)', async () => { + git('checkout -q -b feature'); + await writeFile(join(rootDir, 'assets', 'posts', 'fresh.jpg'), 'jpg'); + await writeFile(join(rootDir, 'assets', 'posts', 'notes.md'), 'md'); + git('add -A'); + git('commit -q -m "add fresh"'); + + const result = resolveChangedPathsAgainstBase(rootDir, 'main'); + + expect(result.paths).toContain('assets/posts/fresh.jpg'); + expect(result.paths).not.toContain('assets/posts/notes.md'); + expect(result.paths).not.toContain('assets/posts/base.png'); + expect(result.warning).toBeNull(); + }); + + it('returns no paths and a warning outside a git repository', async () => { + const nonRepo = await mkdtemp(join(tmpdir(), 'optimize-assets-nogit-base-')); + try { + const result = resolveChangedPathsAgainstBase(nonRepo, 'main'); + expect(result.paths).toEqual([]); + expect(result.warning).toMatch(/git/i); + } finally { + await rm(nonRepo, { recursive: true, force: true }); + } + }); +}); + describe('resolveScope', () => { it('explicit --paths win over --changed (git is never consulted)', () => { // rootDir is a non-existent, non-git path: if --changed were honoured it @@ -236,6 +286,29 @@ describe('resolveScope', () => { expect(paths).toBeUndefined(); }); + it('--base wins over --changed (CI mode scopes against the ref)', () => { + const { paths, warning } = resolveScope({ + changed: true, + explicitPaths: undefined, + base: 'origin/main', + rootDir: '/nonexistent', + }); + // /nonexistent is not a git repo, so against-base detection yields the warning. + expect(paths).toEqual([]); + expect(warning).toMatch(/git/i); + }); + + it('explicit --paths win over --base', () => { + const { paths, warning } = resolveScope({ + changed: false, + explicitPaths: ['assets/team/jane.jpg'], + base: 'origin/main', + rootDir: '/nonexistent', + }); + expect(paths).toEqual(['assets/team/jane.jpg']); + expect(warning).toBeNull(); + }); + it('--changed outside a git repo yields no paths and a warning (no mass rewrite)', () => { const { paths, warning } = resolveScope({ changed: true, @@ -257,6 +330,15 @@ describe('parseArgs', () => { it('sets changed when --changed is passed', () => { expect(parseArgs(argv('--changed', '--write')).changed).toBe(true); }); + + it('parses --base ', () => { + expect(parseArgs(argv('--write', '--base', 'origin/main')).base).toBe('origin/main'); + }); + + it('throws when --base has no ref', () => { + expect(() => parseArgs(argv('--base'))).toThrow(/--base requires a git ref/); + expect(() => parseArgs(argv('--base', '--write'))).toThrow(/--base requires a git ref/); + }); }); describe('optimizeBuffer', () => { diff --git a/scripts/optimize-assets.js b/scripts/optimize-assets.js index 0908d42..f78cc93 100644 --- a/scripts/optimize-assets.js +++ b/scripts/optimize-assets.js @@ -5,7 +5,7 @@ import { readFile, readdir, rename, stat, unlink, writeFile } from 'node:fs/prom import { dirname, extname, isAbsolute, join, relative, resolve, sep } from 'node:path'; import { fileURLToPath } from 'node:url'; import sharp from 'sharp'; -import { getChangedAssets } from './upload-assets.js'; +import { getChangedAssets, getChangedAssetsAgainstBase } from './upload-assets.js'; const JPEG_QUALITY = 80; const WEBP_QUALITY = 80; @@ -94,10 +94,25 @@ export const resolveChangedPaths = (rootDir) => { return { paths, warning: null }; }; -// Decide which files the CLI should process. Explicit --paths always wins over -// --changed; with neither, paths is undefined so run() walks all of assets/**. -export const resolveScope = ({ changed, explicitPaths, rootDir }) => { +// CI variant of resolveChangedPaths: working-tree diffs are empty in CI because +// content is already committed, so scope to assets changed between a base ref +// (e.g. origin/main) and HEAD instead. Mirrors upload-assets' --base mode so the +// optimizer and uploader see the same set of branch assets. +export const resolveChangedPathsAgainstBase = (rootDir, base) => { + const changed = getChangedAssetsAgainstBase(rootDir, base); + if (changed === null) { + return { paths: [], warning: 'Not in a git repository — no changed assets to optimise.' }; + } + const paths = changed.filter((p) => ALLOWED_EXTS.has(extname(p).toLowerCase())); + return { paths, warning: null }; +}; + +// Decide which files the CLI should process. Explicit --paths always wins, then +// --base (CI), then --changed (local working tree); with none, paths is undefined +// so run() walks all of assets/**. +export const resolveScope = ({ changed, explicitPaths, base, rootDir }) => { if (explicitPaths) return { paths: explicitPaths, warning: null }; + if (base) return resolveChangedPathsAgainstBase(rootDir, base); if (changed) return resolveChangedPaths(rootDir); return { paths: undefined, warning: null }; }; @@ -309,7 +324,15 @@ export const parseArgs = (argv) => { paths = raw.split(',').map((p) => p.trim()).filter(Boolean); } - return { write, changed, thresholdKb, paths }; + const baseIdx = args.indexOf('--base'); + let base; + if (baseIdx !== -1) { + const raw = args[baseIdx + 1]; + if (!raw || raw.startsWith('--')) throw new Error('--base requires a git ref (e.g. origin/main)'); + base = raw; + } + + return { write, changed, thresholdKb, paths, base }; }; const isMainModule = () => { @@ -319,16 +342,17 @@ const isMainModule = () => { }; if (isMainModule()) { - const { write, changed, thresholdKb, paths: explicitPaths } = parseArgs(process.argv); + const { write, changed, thresholdKb, paths: explicitPaths, base } = parseArgs(process.argv); const rootDir = join(dirname(fileURLToPath(import.meta.url)), '..'); - // --changed scopes to the branch's new/changed assets (publish flow). Explicit - // --paths wins if both are given; bare invocation still walks assets/**. - const { paths, warning: changedWarning } = resolveScope({ changed, explicitPaths, rootDir }); + // --base scopes to assets changed vs a ref (CI, where the working tree is clean). + // --changed scopes to the working tree (local publish flow). Explicit --paths wins + // over both; bare invocation still walks assets/**. + const { paths, warning: changedWarning } = resolveScope({ changed, explicitPaths, base, rootDir }); - const fromChanged = changed && !explicitPaths; + const scopedByGit = (changed || base) && !explicitPaths; const scopeLabel = paths - ? `, ${paths.length} ${fromChanged ? 'changed' : 'explicit'} path(s)` + ? `, ${paths.length} ${scopedByGit ? 'changed' : 'explicit'} path(s)` : ', walking assets/**'; console.log(`${write ? '✍️ Writing' : '🔍 Dry-run'} — threshold ${thresholdKb} KB${scopeLabel}`); console.log('');