From 4b8b534b8ce3095faf62d4edb8b77396e4d933db Mon Sep 17 00:00:00 2001 From: Chad Miller Date: Tue, 18 Aug 2026 22:51:08 -0700 Subject: [PATCH] feat(git-ci): fall back to the remote helper when smart HTTP cannot serve The HTTP endpoint builds a fetch response by concatenating the bundle chain's packfiles. That assumes the chain is disjoint, and a chain that repeats an object produces a pack git rejects: "the same object appears twice in the pack". This repository's own chain does it, so the runner could not check itself out. The clone now retries with git-remote-atproto, which unbundles each bundle on its own and does not care. The image carries the helper, a package manager for the repository's install step, and a toolchain for a dependency whose native addon has no prebuild for this platform. `.pdsjs/ci.json` runs the gate, minus the coverage floors that only the full `npm run ci` applies. Co-Authored-By: Claude Opus 5 (1M context) --- .pdsjs/ci.json | 11 ++++++ packages/git-ci/Dockerfile | 19 +++++++++-- packages/git-ci/src/checkout.js | 43 +++++++++++++++++++---- packages/git-ci/test/daemon.test.js | 53 ++++++++++++++++------------- 4 files changed, 95 insertions(+), 31 deletions(-) create mode 100644 .pdsjs/ci.json diff --git a/.pdsjs/ci.json b/.pdsjs/ci.json new file mode 100644 index 0000000..650f682 --- /dev/null +++ b/.pdsjs/ci.json @@ -0,0 +1,11 @@ +{ + "name": "ci", + "refs": ["refs/heads/main"], + "steps": [ + { "name": "install", "run": "pnpm install --frozen-lockfile" }, + { "name": "build:ui", "run": "npm run build:ui" }, + { "name": "check", "run": "npm run check" }, + { "name": "typecheck", "run": "npm run typecheck" }, + { "name": "test", "run": "npm run test:unit" } + ] +} diff --git a/packages/git-ci/Dockerfile b/packages/git-ci/Dockerfile index 889ad36..251dead 100644 --- a/packages/git-ci/Dockerfile +++ b/packages/git-ci/Dockerfile @@ -28,11 +28,26 @@ RUN pnpm install --frozen-lockfile --prod --ignore-scripts \ FROM node:22-slim AS runtime WORKDIR /app -# git clones the repository under test from the PDS over https. +# git clones the repository under test from the PDS over https. The +# toolchain is for the repository's own dependencies: a package with a +# native addon and no prebuild for this platform compiles on install, and a +# workflow that cannot install cannot run. RUN apt-get update \ - && apt-get install -y --no-install-recommends git ca-certificates gosu \ + && apt-get install -y --no-install-recommends \ + git ca-certificates gosu python3 make g++ \ && rm -rf /var/lib/apt/lists/* +# Workflows that install dependencies reach for the package manager their +# lockfile names, so corepack resolves it from the repository under test. +ENV COREPACK_ENABLE_DOWNLOAD_PROMPT=0 +RUN corepack enable + +# The remote helper, for a repository smart HTTP cannot serve. `git clone +# atproto://...` finds it here by name. +RUN printf '#!/bin/sh\nexec node /app/packages/git/src/cli.js "$@"\n' \ + > /usr/local/bin/git-remote-atproto \ + && chmod 755 /usr/local/bin/git-remote-atproto + # Runs unprivileged. node:22-slim ships a "node" user for exactly this. # The entrypoint starts as root only to take the state mount, then drops. RUN mkdir -p /data && chown node:node /data diff --git a/packages/git-ci/src/checkout.js b/packages/git-ci/src/checkout.js index 3b30a8d..89f3372 100644 --- a/packages/git-ci/src/checkout.js +++ b/packages/git-ci/src/checkout.js @@ -12,11 +12,15 @@ import { join } from 'node:path'; /** * @param {string[]} args * @param {(chunk: string) => void} onLog + * @param {Record} [env] - added to the environment * @returns {Promise} */ -function git(args, onLog) { +function git(args, onLog, env) { return new Promise((resolve, reject) => { - const child = spawn('git', args, { stdio: ['ignore', 'pipe', 'pipe'] }); + const child = spawn('git', args, { + stdio: ['ignore', 'pipe', 'pipe'], + env: env ? { ...process.env, ...env } : process.env, + }); let stderr = ''; child.stdout.setEncoding('utf8'); child.stderr.setEncoding('utf8'); @@ -44,9 +48,25 @@ export function cloneUrl(service, did, repoName) { return `${service.replace(/\/+$/, '')}/git/${did}/${encodeURIComponent(repoName)}`; } +/** + * The remote helper's URL for the same repository. + * @param {string} did + * @param {string} repoName + * @returns {string} + */ +export function helperUrl(did, repoName) { + return `atproto://${did}/${encodeURIComponent(repoName)}`; +} + /** * Clone the repository and detach at one commit. The caller owns the * directory and passes it to `discardCheckout` when the run finishes. + * + * Two ways in. Smart HTTP needs no helper on the PATH, so it goes first. It + * serves a repository by merging the bundle chain's packfiles, which git + * rejects when a chain repeats an object, so a chain long enough to repeat + * one falls back to the helper. The helper unbundles each bundle on its own + * and does not care. * @param {Object} options * @param {string} options.service * @param {string} options.did @@ -59,10 +79,21 @@ export async function checkout(options) { const { service, did, repoName, sha, onLog } = options; const dir = await mkdtemp(join(tmpdir(), 'pdsjs-git-ci-')); try { - await git( - ['clone', '--quiet', cloneUrl(service, did, repoName), dir], - onLog, - ); + try { + await git( + ['clone', '--quiet', cloneUrl(service, did, repoName), dir], + onLog, + ); + } catch (httpErr) { + onLog( + `\n[smart HTTP clone failed: ${httpErr instanceof Error ? httpErr.message : httpErr}]\n[retrying with git-remote-atproto]\n`, + ); + await rm(dir, { recursive: true, force: true }); + await git(['clone', '--quiet', helperUrl(did, repoName), dir], onLog, { + // The helper resolves the authority through PLC without this. + ATPROTO_GIT_SERVICE: service, + }); + } await git(['-C', dir, 'checkout', '--quiet', '--detach', sha], onLog); return dir; } catch (err) { diff --git a/packages/git-ci/test/daemon.test.js b/packages/git-ci/test/daemon.test.js index 819c7af..99ff61a 100644 --- a/packages/git-ci/test/daemon.test.js +++ b/packages/git-ci/test/daemon.test.js @@ -138,30 +138,37 @@ describe('createStateStore', () => { }); }); - it('keeps working in memory when the file cannot be written', () => { - // A bind mount the daemon does not own reads this way, and losing - // durability must not stop the events reaching the runner. - const readOnly = join(dir, 'locked'); - mkdirSync(readOnly); - chmodSync(readOnly, 0o500); - /** @type {string[]} */ - const warnings = []; - const store = createStateStore(join(readOnly, 'state.json'), (message) => - warnings.push(message), - ); - - expect(() => store.setCursor(SERVICE, 7)).not.toThrow(); - expect(() => store.setRefs('did:plc:abc/proj', { a: 'b' })).not.toThrow(); - expect(store.cursor(SERVICE)).toBe(7); - expect(store.refs('did:plc:abc/proj')).toEqual({ a: 'b' }); - // Warned once, not once per write. - expect(warnings).toHaveLength(1); - expect(warnings[0]).toMatch(/not writable/); - - chmodSync(readOnly, 0o700); - }); + // A directory mode cannot stop root, and a container often runs as root, + // so these two assert nothing there. + const unprivileged = (process.getuid?.() ?? 0) !== 0; + + it.runIf(unprivileged)( + 'keeps working in memory when the file cannot be written', + () => { + // A bind mount the daemon does not own reads this way, and losing + // durability must not stop the events reaching the runner. + const readOnly = join(dir, 'locked'); + mkdirSync(readOnly); + chmodSync(readOnly, 0o500); + /** @type {string[]} */ + const warnings = []; + const store = createStateStore(join(readOnly, 'state.json'), (message) => + warnings.push(message), + ); + + expect(() => store.setCursor(SERVICE, 7)).not.toThrow(); + expect(() => store.setRefs('did:plc:abc/proj', { a: 'b' })).not.toThrow(); + expect(store.cursor(SERVICE)).toBe(7); + expect(store.refs('did:plc:abc/proj')).toEqual({ a: 'b' }); + // Warned once, not once per write. + expect(warnings).toHaveLength(1); + expect(warnings[0]).toMatch(/not writable/); + + chmodSync(readOnly, 0o700); + }, + ); - it('carries on with no warning callback at all', () => { + it.runIf(unprivileged)('carries on with no warning callback at all', () => { const readOnly = join(dir, 'locked-quiet'); mkdirSync(readOnly); chmodSync(readOnly, 0o500); -- 2.51.2