diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 10ecf602..8343e79c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -103,6 +103,33 @@ jobs: - run: pnpm lint - run: pnpm typecheck - run: pnpm test:unit + # The lock test verifies digests against a detached checkout of the + # authority, which is self-consistent however far main moves on, so it + # cannot see a content merge whose repin was never made. That is how + # three governed files drifted for four merges without a red build. + # + # A pull request is allowed to drift: the repin follows the merge, and + # cannot name a merge commit that does not exist yet. Main is not. + - name: Phase 1 authority is current + if: github.event_name == 'push' && github.ref == 'refs/heads/main' + shell: bash + run: | + set -euo pipefail + # Execute the authority's guard and imports, never the unpinned copy. + # Clear inherited Git settings before reading the lock or archive. + authority_git() { + env -i PATH="$PATH" HOME="$RUNNER_TEMP" \ + GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1 \ + GIT_NO_REPLACE_OBJECTS=1 GIT_NO_LAZY_FETCH=1 \ + GIT_TERMINAL_PROMPT=0 GIT_ALLOW_PROTOCOL= \ + git "$@" + } + authority=$(authority_git show HEAD:phase1-conformance.lock.json | jq -er '.harnessAuthority.revision') + [[ "$authority" =~ ^[0-9a-f]{40}$ ]] + pinned=$(mktemp -d) + trap 'rm -rf "$pinned"' EXIT + authority_git archive "$authority" scripts/ | tar -x -C "$pinned" + node "$pinned/scripts/phase1-authority-freshness.mjs" HEAD "$GITHUB_WORKSPACE" e2e: name: E2E diff --git a/docs/phase1-conformance.md b/docs/phase1-conformance.md index db962baf..588bba8f 100644 --- a/docs/phase1-conformance.md +++ b/docs/phase1-conformance.md @@ -2068,24 +2068,60 @@ not appear here. Editing ordinary CI config still dirties that entry and requires the same file-hash repin as touching the harness itself — this is not discoverable until a test fails on it. -A PR that repins `harnessAuthority.revision`/`.tree` to a commit within its -own branch (rather than to something already merged) must be merged with an -actual merge commit, never squash or rebase. Squashing silently breaks the -invariant: the pinned revision stops being an ancestor of `main`, and the -authority checkout keeps resolving only for as long as the now-orphaned -source branch survives. A follow-up repin to the real merge commit is the -only fix once that happens. - -The two-step converges in exactly two commits only when every digest update — -the harness digest, both workflow pin tables, every affected row in the table -above, and every affected `harnessAuthority.files` entry (the workflow's own -self-referential one included) — lands in the code commit itself. The repin -commit that follows must touch nothing but `harness.revision`, -`harnessAuthority.revision`/`.tree`, and the test's literal copy of them. Move -any digest into the repin commit instead and it invalidates the digests -recorded against the commit `harness.revision` still names, forcing a third -commit to advance the pin again — the exact shape of #102's first attempt and -#110's first attempt. +### Authority is pinned to merge commits on `main` + +`harnessAuthority.revision` names a merge commit that is already part of +`main`. It is never a commit on the branch that introduces the change, and a +content pull request never advances it. + +That ordering is forced rather than chosen. A merge commit's SHA does not +exist until the merge happens, so the lock inside the merged tree cannot name +it. Pinning a branch commit instead is what made every pair of conformance +branches collide: each wrote a different `harnessAuthority.revision` into the +same file, so whichever merged second had to be rebuilt, and whichever merged +last left a pin describing a tree that no longer matched what shipped. + +Landing a change to a governed file therefore takes two pull requests: + +1. **The content PR** changes governed files and leaves + `phase1-conformance.lock.json` alone. It lands as a merge commit so the + follow-up can name that commit. + The lock test stays green: it verifies digests against the previous + immutable authority. The main-only freshness guard fails until the repin + lands. +2. **The repin PR** follows, and is the only PR that touches the lock. It + points `harness.revision` and `harnessAuthority.revision`/`.tree` at the + content PR's merge commit on `main`, refreshes every digest to that + commit's tree in `harnessAuthority.files` and + `harnessAuthority.productionDeltas`, including the workflow entries, + and updates the test's literal copy and the prose records. Workflow source + pin tables belong in the content PR; editing a governed workflow during + the repin would create fresh drift. Digests and the revision move together, + against a commit that already exists, so there is no ordering to get wrong. + +Because only the repin PR writes the lock, and it runs serially on `main`, +two content branches can no longer conflict over it. + +### The repin is not optional + +Between the two merges the authority legitimately lags, and nothing in the +lock notices. The lock test compares digests against the authority's own +checkout, which stays self-consistent no matter how far `main` moves on. +#322 changed `src-tauri/Cargo.toml`, `Cargo.lock` and `keyring.rs` without a +repin and stayed green through three further merges; the drift only surfaced +when a later change advanced the authority and had to reconcile all three at +once. + +`scripts/phase1-authority-freshness.mjs` closes that gap. It asserts the pin +is on the first-parent history of the ref under test, that it is a merge +commit rather than a branch tip, and that no governed file or production +delta has moved since it was pinned. CI extracts and executes the guard and its imports from the +pinned authority, so changing the working-tree guard cannot bypass the check. The guard also +compares its own blob, even though it is outside the frozen harness file list. +The first content merge has no pinned guard yet and fails closed until its +repin lands. CI runs it on `main` pushes only, since a pull request is allowed +to lag by construction. A forgotten repin now turns `main` red instead of +staying silent. Before parsing or executing SDK authority, the harness queries the verified checkout with `git rev-parse --show-object-format`, accepts only `sha1` or diff --git a/scripts/phase1-authority-freshness.d.mts b/scripts/phase1-authority-freshness.d.mts new file mode 100644 index 00000000..329bab71 --- /dev/null +++ b/scripts/phase1-authority-freshness.d.mts @@ -0,0 +1,16 @@ +export type AuthorityDrift = { + group: 'files' | 'productionDeltas'; + path: string; + pinned: string | null; + shipped: string | null; +}; + +export type AuthorityFreshness = { + revision: string; + ref: string; + reachable: boolean; + adrift: AuthorityDrift[]; + failures: string[]; +}; + +export function checkAuthorityFreshness(ref?: string, repositoryRoot?: string): AuthorityFreshness; diff --git a/scripts/phase1-authority-freshness.mjs b/scripts/phase1-authority-freshness.mjs new file mode 100644 index 00000000..e37dbf26 --- /dev/null +++ b/scripts/phase1-authority-freshness.mjs @@ -0,0 +1,157 @@ +#!/usr/bin/env node +/** + * Guards the three properties the conformance lock cannot check itself. + * + * `phase1-conformance.lock.json` records digests for the governed harness + * files and production deltas, and the lock test verifies them against a + * detached checkout of `harnessAuthority.revision`. That is self-consistent by + * construction: the authority's tree is immutable, so the digests keep + * matching it no matter how far the branch moves on. Nothing there notices + * when the code that actually ships has left the authority behind. + * + * That is how #322 changed `keyring.rs`, `Cargo.toml` and `Cargo.lock` without + * a repin and stayed green across three further merges. + * + * Run against a ref, default `HEAD`: + * node scripts/phase1-authority-freshness.mjs [ref] + */ +import { execFileSync } from 'node:child_process'; +import { realpathSync } from 'node:fs'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { createGitEnvironment } from './phase1-conformance-lock.mjs'; + +const projectRoot = resolve(dirname(fileURLToPath(import.meta.url)), '..'); + +function runGit(repositoryRoot, args) { + return execFileSync('git', args, { + cwd: repositoryRoot, + env: createGitEnvironment(), + timeout: 30_000, + encoding: 'utf8', + maxBuffer: 64 * 1024 * 1024, + stdio: ['ignore', 'pipe', 'pipe'], + }).trim(); +} + +/** + * One tree listing per ref, rather than a `rev-parse` per file. The lock + * governs 35 paths, so the naive form spawns seventy processes and takes + * long enough to trip a test timeout. + */ +function blobsAt(git, ref) { + const blobs = new Map(); + let listing; + try { + listing = git(['ls-tree', '-r', '-z', ref]); + } catch { + return blobs; + } + for (const record of listing.split('\0')) { + if (record === '') { + continue; + } + const tab = record.indexOf('\t'); + if (tab < 0) { + continue; + } + const [, type, oid] = record.slice(0, tab).split(/\s+/u); + if (type === 'blob') { + blobs.set(record.slice(tab + 1), oid); + } + } + return blobs; +} + +export function checkAuthorityFreshness(ref = 'HEAD', repositoryRoot = projectRoot) { + const git = (args) => runGit(repositoryRoot, args); + ref = git(['rev-parse', '--verify', '--end-of-options', `${ref}^{commit}`]); + // Read the lock as it exists at `ref`, not from the working tree. Reading + // from disk would compare today's pin against a historical tree, which + // silently answers a different question than the one being asked. + const lock = JSON.parse(git(['show', `${ref}:phase1-conformance.lock.json`])); + const authority = lock.harnessAuthority; + const revision = authority.revision; + if (!/^[0-9a-f]{40}$/u.test(revision)) { + throw new Error('harnessAuthority.revision must be an immutable Git ID'); + } + const failures = []; + + // Orphaned pin: a squash or rebase leaves the authority reachable only while + // the source branch survives. + let reachable = true; + try { + git(['merge-base', '--is-ancestor', revision, ref]); + } catch { + reachable = false; + failures.push(`harnessAuthority.revision ${revision} is not an ancestor of ${ref}`); + } + + // A branch-tip pin is what makes two conformance branches collide. Authority + // belongs on a merge commit that is already part of the history. + if (reachable) { + if (!git(['rev-list', '--first-parent', ref]).split('\n').includes(revision)) { + failures.push( + 'harnessAuthority.revision must be on the first-parent history of the checked ref', + ); + } + const parents = git(['rev-list', '--parents', '-n', '1', revision]).split(/\s+/u).slice(1); + if (parents.length < 2) { + failures.push( + `harnessAuthority.revision ${revision} has ${parents.length} parent(s);` + + ' authority must be pinned to a merge commit', + ); + } + } + + // The pin must describe what ships, or the executed harness and the shipped + // tree have silently diverged. + const adrift = []; + if (reachable) { + const pinnedBlobs = blobsAt(git, revision); + const shippedBlobs = blobsAt(git, ref); + const guard = 'scripts/phase1-authority-freshness.mjs'; + if (!pinnedBlobs.has(guard)) { + failures.push('Pinned freshness guard is missing; a repin is due'); + } + const groups = { + files: [...authority.files, { path: guard }], + productionDeltas: authority.productionDeltas, + }; + for (const group of ['files', 'productionDeltas']) { + for (const entry of groups[group] ?? []) { + const pinned = pinnedBlobs.get(entry.path) ?? null; + const shipped = shippedBlobs.get(entry.path) ?? null; + if (pinned !== shipped) { + adrift.push({ group, path: entry.path, pinned, shipped }); + } + } + } + } + if (adrift.length > 0) { + failures.push( + `${adrift.length} governed file(s) have moved since the authority was` + + ' pinned; a repin is due', + ); + } + + return { revision, ref, reachable, adrift, failures }; +} + +if (process.argv[1] && realpathSync(process.argv[1]) === fileURLToPath(import.meta.url)) { + const ref = process.argv[2] ?? 'HEAD'; + const result = checkAuthorityFreshness(ref, process.argv[3] ?? projectRoot); + if (result.failures.length === 0) { + process.stdout.write( + `Phase 1 authority ${result.revision.slice(0, 8)} is current for ${ref}.\n`, + ); + process.exit(0); + } + for (const failure of result.failures) { + process.stderr.write(`${failure}\n`); + } + for (const entry of result.adrift) { + process.stderr.write(` ${entry.group}: ${entry.path}\n`); + } + process.exit(1); +} diff --git a/src/phase1-authority-freshness.test.ts b/src/phase1-authority-freshness.test.ts new file mode 100644 index 00000000..bc3d45a3 --- /dev/null +++ b/src/phase1-authority-freshness.test.ts @@ -0,0 +1,202 @@ +import { execFileSync, spawnSync } from 'node:child_process'; +import { copyFileSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { resolve } from 'node:path'; +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; +import { checkAuthorityFreshness } from '../scripts/phase1-authority-freshness.mjs'; + +let root: string; +const guard = 'scripts/phase1-authority-freshness.mjs'; +const git = (...args: string[]) => + execFileSync('git', args, { cwd: root, encoding: 'utf8' }).trim(); +const commit = (message: string) => { + git('add', '.'); + git( + '-c', + 'user.name=Fixture', + '-c', + 'user.email=fixture@example.test', + '-c', + 'commit.gpgsign=false', + 'commit', + '-qm', + message, + ); + return git('rev-parse', 'HEAD'); +}; +let authority: string; +function pin(revision = authority) { + writeFileSync( + resolve(root, 'phase1-conformance.lock.json'), + JSON.stringify({ + harnessAuthority: { + revision, + files: [{ path: 'governed.txt' }], + productionDeltas: [{ path: 'production.txt' }], + }, + }), + ); + commit('pin'); +} + +beforeEach(() => { + root = mkdtempSync(resolve(tmpdir(), 'chat-authority-')); + git('init', '-q', '-b', 'main'); + mkdirSync(resolve(root, 'scripts')); + for (const name of [ + 'phase1-authority-freshness', + 'phase1-conformance-lock', + 'supervised-exec', + 'executable-resolution', + 'owned-temp-directory', + 'supervisor-status', + ]) { + copyFileSync( + resolve(process.cwd(), `scripts/${name}.mjs`), + resolve(root, `scripts/${name}.mjs`), + ); + } + writeFileSync(resolve(root, 'governed.txt'), 'harness'); + writeFileSync(resolve(root, 'production.txt'), 'production'); + commit('initial'); + git('checkout', '-qb', 'content'); + writeFileSync(resolve(root, 'content.txt'), 'content'); + commit('content'); + git('checkout', '-q', 'main'); + git( + '-c', + 'user.name=Fixture', + '-c', + 'user.email=fixture@example.test', + '-c', + 'commit.gpgsign=false', + 'merge', + '--no-ff', + '-qm', + 'merge content', + 'content', + ); + authority = git('rev-parse', 'HEAD'); + pin(); +}); +afterEach(() => { + vi.unstubAllEnvs(); + rmSync(root, { recursive: true, force: true }); +}); +const check = () => checkAuthorityFreshness('HEAD', root); + +describe('phase 1 authority freshness', () => { + test('accepts a repin to an unchanged merged authority', () => { + expect(check().failures).toEqual([]); + }); + test.each(['governed.txt', 'production.txt', guard])('rejects drift in %s', (path) => { + writeFileSync(resolve(root, path), 'changed'); + commit('drift'); + expect(check().adrift.map((entry) => entry.path)).toContain(path); + }); + test('rejects a missing guard in the pinned authority', () => { + git('rm', guard); + commit('delete guard'); + pin(git('rev-parse', 'HEAD')); + expect(check().failures.join(' ')).toMatch(/freshness guard.*missing/u); + }); + test('rejects a branch tip pin', () => { + pin(git('rev-parse', 'HEAD')); + expect(check().failures.join(' ')).toMatch(/merge commit/u); + }); + test('rejects a merge reachable only through a feature branch', () => { + git('checkout', '-qb', 'feature'); + writeFileSync(resolve(root, 'feature.txt'), 'feature'); + commit('feature'); + git('checkout', '-qb', 'nested'); + writeFileSync(resolve(root, 'nested.txt'), 'nested'); + commit('nested'); + git('checkout', '-q', 'feature'); + git( + '-c', + 'user.name=Fixture', + '-c', + 'user.email=fixture@example.test', + '-c', + 'commit.gpgsign=false', + 'merge', + '--no-ff', + '-qm', + 'nested merge', + 'nested', + ); + const nested = git('rev-parse', 'HEAD'); + git('checkout', '-q', 'main'); + git( + '-c', + 'user.name=Fixture', + '-c', + 'user.email=fixture@example.test', + '-c', + 'commit.gpgsign=false', + 'merge', + '--no-ff', + '-qm', + 'feature merge', + 'feature', + ); + pin(nested); + expect(check().failures.join(' ')).toMatch(/first-parent/u); + }); + test('rejects an unreachable pin', () => { + git('checkout', '-qb', 'other'); + writeFileSync(resolve(root, 'other.txt'), 'other'); + const other = commit('other'); + git('checkout', '-q', 'main'); + pin(other); + expect(check().reachable).toBe(false); + }); + test('ignores inherited Git repository and configuration overrides', () => { + vi.stubEnv('GIT_DIR', resolve(root, 'nonexistent')); + vi.stubEnv('GIT_CONFIG_COUNT', '1'); + vi.stubEnv('GIT_CONFIG_KEY_0', 'core.bare'); + vi.stubEnv('GIT_CONFIG_VALUE_0', 'true'); + expect(check().failures).toEqual([]); + }); + test('does not let replacement objects conceal drift', () => { + const clean = git('rev-parse', 'HEAD'); + writeFileSync(resolve(root, 'governed.txt'), 'changed'); + const changed = commit('drift'); + git('replace', changed, clean); + expect(check().adrift.map((entry) => entry.path)).toContain('governed.txt'); + }); + test('the CI shell executes the pinned guard even when the current guard exits successfully', () => { + const workflow = readFileSync(resolve(process.cwd(), '.github/workflows/ci.yml'), 'utf8'); + const section = workflow.split(' - name: Phase 1 authority is current\n')[1]; + if (!section) throw new Error('Missing freshness step'); + const body = section.split('\n e2e:')[0]?.split(' run: |\n')[1]; + if (!body) throw new Error('Missing freshness shell body'); + const script = body + .split('\n') + .map((line) => line.slice(10)) + .join('\n'); + const run = () => + spawnSync('bash', ['-c', script], { + cwd: root, + encoding: 'utf8', + env: { ...process.env, RUNNER_TEMP: root, GITHUB_WORKSPACE: root, GIT_DIR: '/invalid' }, + }); + expect(run().status).toBe(0); + writeFileSync(resolve(root, guard), 'process.exit(0);'); + commit('neuter current guard'); + const failed = run(); + expect(failed.status).toBe(1); + expect(failed.stderr).toContain(guard); + }); + test('runs the main-only gate from the pinned scripts', () => { + const workflow = readFileSync(resolve(process.cwd(), '.github/workflows/ci.yml'), 'utf8'); + expect(workflow).toContain( + "if: github.event_name == 'push' && github.ref == 'refs/heads/main'", + ); + expect(workflow).toContain('archive "$authority" scripts/'); + expect(workflow).toContain( + 'node "$pinned/scripts/phase1-authority-freshness.mjs" HEAD "$GITHUB_WORKSPACE"', + ); + expect(workflow).toContain('GIT_CONFIG_NOSYSTEM=1'); + }); +});