diff --git a/apps/aflow-executor-host/src/__tests__/checkoutGitDir.test.ts b/apps/aflow-executor-host/src/__tests__/checkoutGitDir.test.ts new file mode 100644 index 00000000..86ad0036 --- /dev/null +++ b/apps/aflow-executor-host/src/__tests__/checkoutGitDir.test.ts @@ -0,0 +1,208 @@ +/** + * Contract: the git the executor runs in a run's checkout after the run reads + * the git directory the executor made for it, whatever the coding agent left in + * the checkout — and runs no program the agent chose. + * + * The agent, confined, plants what would otherwise reach that git: a write of a + * `commondir` and a filter into the clone's git directory, the checkout's `.git` + * rewritten to a git directory of its own whose `commondir` names a repository + * it built in its scratch with a `filter..clean` defined, and a + * `.gitattributes` naming that filter. Staging and the apply check then run + * outside the sandbox, with this process's PATH and HOME. + * + * The proof is a filter that leaves evidence where the sandbox cannot write: if + * it ever runs, it ran unconfined. And the same plant is shown to bite a git + * that resolves the checkout through its `.git`, with the safety arguments the + * executor's own git carries — so the test passing means the executor looked + * elsewhere, not that the plant was inert. + */ +import { execFile } from 'node:child_process'; +import { existsSync } from 'node:fs'; +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; +import { promisify } from 'node:util'; + +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +import { createHostHarnessHandler } from '../handlers/harnessHandlers.js'; +import { sandboxReadiness } from '../sandboxedRun.js'; +import { removeWorktree } from '../worktree.js'; +import { CONFINEMENT_LISTENERS, requires } from './fixtures/capabilities.js'; + +const run = promisify(execFile); +const confined = requires(...CONFINEMENT_LISTENERS); + +let base: string; +let repo: string; +let evidence: string; +let policyPath: string; + +async function git(cwd: string, ...args: string[]): Promise { + return (await run('git', ['-C', cwd, ...args])).stdout.trim(); +} + +/** The agent's whole turn: plant, record what the sandbox allowed, and leave. */ +function plantingScript(evidencePath: string): string { + return [ + 'gd=$(sed -n "s/^gitdir: //p" .git)', + 'agent="$TMPDIR/agent"', + 'git init -q "$agent"', + `printf '#!/bin/sh\\ntouch %s\\ncat\\n' '${evidencePath}' > "$TMPDIR/filter.sh"`, + 'chmod +x "$TMPDIR/filter.sh"', + 'git -C "$agent" config filter.pwn.clean "$TMPDIR/filter.sh"', + 'git -C "$agent" config filter.pwn.smudge "$TMPDIR/filter.sh"', + '{ echo "checkout=$PWD"', + ' printf "%s\\n" "$agent/.git" > "$gd/commondir" && echo commondir-written || echo commondir-refused', + ' printf "[filter \\"pwn\\"]\\n\\tclean = %s\\n" "$TMPDIR/filter.sh" >> "$gd/config" ' + + '&& echo config-written || echo config-refused', + '} > planted.txt 2>/dev/null', + 'mkdir -p "$TMPDIR/moved"', + 'echo "ref: refs/heads/main" > "$TMPDIR/moved/HEAD"', + 'printf "%s\\n" "$agent/.git" > "$TMPDIR/moved/commondir"', + 'printf "gitdir: %s\\n" "$TMPDIR/moved" > .git', + "echo '* filter=pwn' > .gitattributes", + "echo 'added by the agent' > added.txt", + ].join('\n'); +} + +beforeAll(async () => { + base = await mkdtemp(join(tmpdir(), 'aflow-checkout-gitdir-')); + repo = join(base, 'project'); + evidence = join(base, 'THE-FILTER-RAN'); + await mkdir(repo, { recursive: true }); + await git(repo, 'init', '-q', '-b', 'main'); + await git(repo, 'config', 'user.email', 'test@example.com'); + await git(repo, 'config', 'user.name', 'Test'); + await writeFile(join(repo, 'README.md'), '# project\n', 'utf8'); + await git(repo, 'add', '-A'); + await git(repo, 'commit', '-q', '-m', 'initial'); + + policyPath = join(base, 'host-policy.json'); + await writeFile( + policyPath, + JSON.stringify({ + version: 1, + bindings: [ + { + id: 'hb', + root: repo, + mode: 'readwrite', + allowsExecution: true, + singleFile: false, + spaceId: 'space-test', + }, + ], + harnesses: [ + { + id: 'plants', + executable: '/bin/sh', + args: ['-c', plantingScript(evidence)], + // A session keeps the checkout as the agent left it, so the plant can + // be shown to bite afterwards. + sessionArgs: ['{session}'], + resumeArgs: ['{session}'], + }, + ], + }), + ); +}); + +afterAll(async () => { + await rm(base, { recursive: true, force: true }); +}); + +describe.runIf(sandboxReadiness().ready)("the executor's git in a run's checkout", () => { + it.skipIf(confined.skip)( + confined.title( + 'stages and judges the run through the git directory it made, running no program the agent planted', + ), + { tags: ['listener'], timeout: 120_000 }, + async () => { + const written: Record = {}; + const outcome = await createHostHarnessHandler(policyPath).execute({ + operationId: 'host.harness.run', + spaceId: 'space-test', + runId: 'run-checkout-gitdir', + job: { inputRef: 'inline:x' }, + signal: new AbortController().signal, + log: { error: () => undefined, warn: () => undefined, info: () => undefined }, + readPayload: () => + Promise.resolve({ + bindingId: 'hb', + harness: 'plants', + task: 'Plant.', + timeoutMs: 60_000, + // A named base, so the apply check runs in the clone too. + base: 'main', + }), + emitLiveDelta: () => Promise.resolve(), + writePayload: (kind: string, data: unknown) => { + written[kind] = data; + return Promise.resolve(`inline:${kind}`); + }, + } as never); + expect(outcome.status).toBe('SUCCEEDED'); + const output = written['output'] as Record; + const patch = String(output['patch']); + const checkout = /^\+checkout=(.+)$/m.exec(patch)?.[1]; + expect(checkout).toBeDefined(); + try { + // The sandbox refused both writes into the clone's git directory. + expect(patch).toContain('+commondir-refused\n'); + expect(patch).toContain('+config-refused\n'); + + // Nothing ran: collection staged `.gitattributes` and the files it + // selects, and the apply check read them, and neither found the filter. + expect(existsSync(evidence)).toBe(false); + + // Read through the executor's git directory: the diff is against the + // base the clone checked out, holding only what the agent wrote — read + // through the agent's repository, whose index is empty, README.md would + // be an addition too — and the base commit the apply check reads + // exists only there, so a check through the agent's repository could + // not have answered `clean`. + expect(output['baseSha']).toBe(await git(repo, 'rev-parse', 'main')); + expect(output['filesChanged']).toBe(3); + for (const file of ['.gitattributes', 'added.txt', 'planted.txt']) { + expect(patch).toContain(`+++ b/${file}\n`); + } + expect(patch).not.toContain('b/README.md'); + expect(output['applies']).toBe('clean'); + + // The plant was real: a git resolving the checkout through its `.git`, + // carrying the same safety arguments and no global or system config, + // runs the agent's program. + await run( + 'git', + [ + '-C', + checkout ?? '', + '-c', + 'core.hooksPath=/dev/null', + '-c', + 'core.fsmonitor=', + '-c', + 'protocol.ext.allow=never', + 'add', + '-A', + ], + { + env: { + PATH: process.env['PATH'] ?? '', + HOME: process.env['HOME'] ?? '', + GIT_CONFIG_NOSYSTEM: '1', + GIT_CONFIG_GLOBAL: '/dev/null', + }, + }, + ); + expect(existsSync(evidence)).toBe(true); + } finally { + if (checkout !== undefined) { + await removeWorktree(checkout); + await rm(dirname(checkout), { recursive: true, force: true }); + } + } + }, + ); +}); diff --git a/apps/aflow-executor-host/src/__tests__/checkoutReplacement.test.ts b/apps/aflow-executor-host/src/__tests__/checkoutReplacement.test.ts new file mode 100644 index 00000000..31cfd753 --- /dev/null +++ b/apps/aflow-executor-host/src/__tests__/checkoutReplacement.test.ts @@ -0,0 +1,177 @@ +/** + * Contract: a run cannot put something else in place of its checkout. + * + * The executor runs git on a run's checkout outside the sandbox, naming the + * working tree outright, and git takes a link there as readily as a directory. + * A checkout replaced by a link to `~/.ssh` would have its contents staged into + * the run's diff and a merge written into it, as the operator. Two things + * stand in the way: the sandbox is never handed the directory holding the + * checkout, so a job cannot replace it; and every git the executor runs on a + * checkout first finds it the real directory that was made, so a replacement + * made any other way is refused rather than followed. + */ +import { execFile } from 'node:child_process'; +import { + lstat, + mkdir, + mkdtemp, + readdir, + readFile, + realpath, + rm, + symlink, + writeFile, +} from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { promisify } from 'node:util'; + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { mergeIntoCheckout } from '../baseMerge.js'; +import { HostBindingSchema } from '../bindings.js'; +import { makeAgentScratch, makeScratch } from '../executorScratch.js'; +import { runSandboxed, sandboxAvailable } from '../sandboxedRun.js'; +import { collectChanges, isRealDirectory, prepareWorktree } from '../worktree.js'; +import { CONFINEMENT_LISTENERS, requires } from './fixtures/capabilities.js'; + +const confined = requires(...CONFINEMENT_LISTENERS); + +const run = promisify(execFile); + +/** What every git the executor would have run there says instead. */ +const REFUSAL = /is no longer the directory made for it, so git was not run in it/; + +const IDENTITY = ['-c', 'user.name=Test', '-c', 'user.email=test@example.com']; + +let base: string; +let repo: string; +let scratch: string; +/** What a link would lead the executor's git into: the operator's keys, say. */ +let keys: string; + +async function git(cwd: string, ...args: string[]): Promise { + return (await run('git', ['-C', cwd, ...args])).stdout.trim(); +} + +async function contentsOf(dir: string): Promise> { + const contents: Record = {}; + for (const name of (await readdir(dir)).sort()) { + contents[name] = await readFile(join(dir, name), 'utf8'); + } + return contents; +} + +beforeEach(async () => { + base = await mkdtemp(join(tmpdir(), 'aflow-checkout-replaced-')); + repo = join(base, 'repo'); + keys = join(base, 'keys'); + await mkdir(join(repo, 'packages', 'app'), { recursive: true }); + await mkdir(keys); + await writeFile(join(keys, 'id_ed25519'), 'PRIVATE KEY\n'); + await git(repo, 'init', '-q', '--initial-branch=main'); + await git(repo, 'config', 'user.email', 'test@example.com'); + await git(repo, 'config', 'user.name', 'Test'); + await writeFile(join(repo, 'README.md'), 'base\n'); + await writeFile(join(repo, 'packages', 'app', 'index.txt'), 'app\n'); + await git(repo, 'add', '-A'); + await git(repo, 'commit', '-qm', 'base'); + scratch = await makeScratch('aflow-harness-', base); +}); + +afterEach(async () => { + await rm(base, { recursive: true, force: true }); +}); + +describe("a run's checkout replaced by a link", () => { + it.skipIf(!sandboxAvailable() || confined.skip)( + confined.title('cannot be made from inside the sandbox a run is handed'), + { tags: ['listener'], timeout: 120_000 }, + async () => { + const wt = await prepareWorktree(repo, scratch, 'work', { dependencies: 'none' }); + const agentScratch = await makeAgentScratch(scratch); + const result = await runSandboxed({ + binding: HostBindingSchema.parse({ + id: 'hb', + root: repo, + mode: 'readwrite', + allowsExecution: true, + }), + argv: [ + '/bin/sh', + '-c', + String.raw`rm -rf "$1"; mv "$1" "$1.moved"; ln -s "$2" "$1" && echo REPLACED; rm -rf "$3"; ln -s "$2" "$3" && echo GIT_DIR_REPLACED; exit 0`, + 'sh', + wt.path, + keys, + wt.gitDir, + ], + cwd: agentScratch, + env: {}, + timeoutMs: 60_000, + // As a commission is handed them: its own scratch, and the checkout. + scratchDir: agentScratch, + widening: { + authPaths: [], + allowedDomains: [], + writableRoot: wt.workDir, + gitDir: wt.gitDir, + withholdBindingWrite: true, + }, + idPrefix: 'rp', + ownerRunId: 'probe', + closeStdin: true, + signal: new AbortController().signal, + onDelta: () => undefined, + }); + + // It ran, and every attempt was refused. + expect(result.exitCode).toBe(0); + expect(result.stdout).not.toContain('REPLACED'); + expect(await isRealDirectory(wt.path)).toBe(true); + expect(await realpath(wt.path)).toBe(wt.path); + expect(await isRealDirectory(wt.gitDir)).toBe(true); + await expect(lstat(`${wt.path}.moved`)).rejects.toThrow(); + // Its files were the run's to delete, and the executor's git still reads it. + await expect(collectChanges(wt)).resolves.toMatchObject({ overCeiling: false }); + expect(await contentsOf(keys)).toEqual({ id_ed25519: 'PRIVATE KEY\n' }); + }, + ); + + it('made from outside the sandbox, is refused by collection and by a merge', async () => { + await git(repo, 'checkout', '-qb', 'moved'); + await writeFile(join(repo, 'README.md'), 'moved\n'); + await writeFile(join(repo, 'planted'), 'written by the merge\n'); + await git(repo, 'add', '-A'); + await git(repo, 'commit', '-qm', 'moved'); + const moved = await git(repo, 'rev-parse', 'HEAD'); + await git(repo, 'checkout', '-q', 'main'); + + const wt = await prepareWorktree(repo, scratch, 'work', { dependencies: 'none' }); + await expect(collectChanges(wt)).resolves.toMatchObject({ filesChanged: 0 }); + + await rm(wt.path, { recursive: true, force: true }); + await symlink(keys, wt.path); + + await expect(collectChanges(wt)).rejects.toThrow(REFUSAL); + await expect(mergeIntoCheckout(wt, moved, IDENTITY)).rejects.toMatchObject({ + name: 'WorktreeError', + kind: 'checkout_replaced', + }); + expect(await contentsOf(keys)).toEqual({ id_ed25519: 'PRIVATE KEY\n' }); + expect(await git(wt.gitDir, 'rev-parse', 'HEAD')).not.toBe(moved); + }); + + it("made at the folder's place inside the clone, is refused the same way", async () => { + const wt = await prepareWorktree(join(repo, 'packages', 'app'), scratch, 'work', { + dependencies: 'none', + }); + expect(wt.workDir).toBe(join(wt.path, 'packages', 'app')); + + await rm(wt.workDir, { recursive: true, force: true }); + await symlink(keys, wt.workDir); + + await expect(collectChanges(wt)).rejects.toThrow(REFUSAL); + expect(await contentsOf(keys)).toEqual({ id_ed25519: 'PRIVATE KEY\n' }); + }); +}); diff --git a/apps/aflow-executor-host/src/__tests__/commitCheck.test.ts b/apps/aflow-executor-host/src/__tests__/commitCheck.test.ts index 669bf625..533ef95a 100644 --- a/apps/aflow-executor-host/src/__tests__/commitCheck.test.ts +++ b/apps/aflow-executor-host/src/__tests__/commitCheck.test.ts @@ -29,6 +29,7 @@ import { import type { SandboxedRunInput, SandboxedRunResult } from '../sandboxedRun.js'; import { compileSandboxPolicy } from '../sandboxPolicy.js'; +import { checkoutGitDir } from '../worktree.js'; import { CONFINEMENT_LISTENERS, requires } from './fixtures/capabilities.js'; const confined = requires(...CONFINEMENT_LISTENERS); @@ -296,6 +297,9 @@ describe('host.commit.check — a passing check', () => { authPaths: [], allowedDomains: [], writableRoot: input?.cwd, + gitDir: checkoutGitDir(input?.cwd ?? ''), + // A repository's top level borrows nothing from outside the folder. + borrowedReads: [], withholdBindingWrite: true, }); expect(input?.trustedEnv).toMatchObject({ diff --git a/apps/aflow-executor-host/src/__tests__/gitFilterEscape.test.ts b/apps/aflow-executor-host/src/__tests__/gitFilterEscape.test.ts index 1d9fe114..0741ec9c 100644 --- a/apps/aflow-executor-host/src/__tests__/gitFilterEscape.test.ts +++ b/apps/aflow-executor-host/src/__tests__/gitFilterEscape.test.ts @@ -20,7 +20,7 @@ import { promisify } from 'node:util'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { collectChanges, prepareWorktree, removeWorktree } from '../worktree.js'; +import { checkApplies, collectChanges, prepareWorktree, removeWorktree } from '../worktree.js'; const run = promisify(execFile); @@ -88,11 +88,48 @@ describe('a filter the repository selects', () => { const worktree = await prepareWorktree(repo, scratch, 'run-filter'); await writeFile(join(worktree.path, 'b.txt'), 'added by the harness\n'); - await collectChanges(worktree.path); - await removeWorktree(repo, worktree.path); + await collectChanges(worktree); + await removeWorktree(worktree.path); // `worktree add` checked out every file and `git add -A` staged them. If // the filter had been reachable it would have run several times over. expect(existsSync(evidence)).toBe(false); }, 30_000); + + it("does not run when the checkout's .git is rewritten to a repository defining it", async () => { + // Committing the fixture under the operator's config ran theirs already. + await rm(evidence, { force: true }); + + // What a confined agent can leave (checkoutGitDir.test.ts plants it from + // inside the sandbox): a repository of its own defining a filter, a git + // directory whose `commondir` names it, the checkout's gitfile pointed + // there, and `.gitattributes` selecting the filter. + const worktree = await prepareWorktree(repo, scratch, 'run-gitfile'); + const agent = join(base, 'agent'); + const moved = join(base, 'moved'); + await git(base, 'init', '-q', agent); + await git(agent, 'config', 'filter.agent.clean', join(base, 'evil-filter.sh')); + await mkdir(moved, { recursive: true }); + await writeFile(join(moved, 'HEAD'), 'ref: refs/heads/main\n'); + await writeFile(join(moved, 'commondir'), `${join(agent, '.git')}\n`); + await writeFile(join(worktree.path, '.git'), `gitdir: ${moved}\n`); + await writeFile(join(worktree.path, '.gitattributes'), '* filter=agent\n'); + await writeFile(join(worktree.path, 'b.txt'), 'added by the harness\n'); + + const changes = await collectChanges(worktree); + // Against the clone's own index: through the agent's, whose index is empty, + // a.txt would read as an addition too. + expect(changes.filesChanged).toBe(2); + expect(changes.patch).not.toContain('b/a.txt'); + expect((await checkApplies(worktree, changes.patch, worktree.baseSha)).state).toBe('clean'); + expect(existsSync(evidence)).toBe(false); + + // The plant is real: git resolving the checkout through its `.git`, with + // no global or system config, runs it. + await run('git', ['-C', worktree.path, '-c', 'core.hooksPath=/dev/null', 'add', '-A'], { + env: { ...process.env, GIT_CONFIG_NOSYSTEM: '1', GIT_CONFIG_GLOBAL: '/dev/null' }, + }); + expect(existsSync(evidence)).toBe(true); + await removeWorktree(worktree.path); + }, 30_000); }); diff --git a/apps/aflow-executor-host/src/__tests__/gitReadiness.test.ts b/apps/aflow-executor-host/src/__tests__/gitReadiness.test.ts new file mode 100644 index 00000000..b345d841 --- /dev/null +++ b/apps/aflow-executor-host/src/__tests__/gitReadiness.test.ts @@ -0,0 +1,52 @@ +/** + * Contract: a publication runs only where the executor's git can bring the + * commit it makes in its clone into the folder — a fetch with + * `--no-write-fetch-head`, git 2.29 or later — and the refusal says what is + * missing. + */ +import { describe, expect, it } from 'vitest'; + +import { + publicationGitMissing, + publicationGitReadiness, + noPublicationGitMessage, + parseGitVersion, +} from '../gitReadiness.js'; + +describe('the git a publication needs', () => { + it('reads the version however the build spells it', () => { + expect(parseGitVersion('git version 2.52.0\n')).toEqual({ major: 2, minor: 52, patch: 0 }); + expect(parseGitVersion('git version 2.39.5 (Apple Git-154)')).toEqual({ + major: 2, + minor: 39, + patch: 5, + }); + expect(parseGitVersion('git version 2.45.1.windows.1')).toEqual({ + major: 2, + minor: 45, + patch: 1, + }); + expect(parseGitVersion('not git')).toBeUndefined(); + }); + + it('names the version found, the version needed and the fetch flag below the floor', () => { + expect(publicationGitMissing('git version 2.29.0')).toEqual([]); + expect(publicationGitMissing('git version 3.0.0')).toEqual([]); + const [missing] = publicationGitMissing('git version 2.28.1'); + expect(missing).toContain('git 2.29 or later'); + expect(missing).toContain('this machine has 2.28.1'); + expect(missing).toContain('`--no-write-fetch-head`'); + expect(publicationGitMissing(undefined)).toHaveLength(1); + expect(publicationGitMissing('something else')[0]).toContain('`something else`'); + }); + + it('refuses in the shape of the sandbox refusal, carrying what is missing', () => { + const message = noPublicationGitMessage(publicationGitMissing('git version 2.20.1')); + expect(message).toContain('nothing can be published here'); + expect(message).toContain('Missing: git 2.29 or later — this machine has 2.20.1'); + }); + + it('finds this machine ready', async () => { + expect(await publicationGitReadiness()).toEqual({ ready: true, missing: [] }); + }); +}); diff --git a/apps/aflow-executor-host/src/__tests__/harnessBrowserRun.test.ts b/apps/aflow-executor-host/src/__tests__/harnessBrowserRun.test.ts index 92f4b818..a66454fc 100644 --- a/apps/aflow-executor-host/src/__tests__/harnessBrowserRun.test.ts +++ b/apps/aflow-executor-host/src/__tests__/harnessBrowserRun.test.ts @@ -9,9 +9,10 @@ * for a run with a browser and one without, and compared. */ import { execFile } from 'node:child_process'; +import { realpathSync } from 'node:fs'; import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { dirname, join } from 'node:path'; import { promisify } from 'node:util'; import { @@ -177,6 +178,18 @@ async function runHarness( return { status: outcome.status, written, activity, chromeLaunches: h.launches.length }; } +/** + * `text` with this run's scratch written as one name: the directory around the + * one the sandbox is handed, which holds the checkout, in both of its spellings + * — the checkout's path is the resolved one. Resolved first, since the other + * is a suffix of it where the temp root is reached through a link. + */ +function scratchNamed(text: string, input: SandboxedRunInput): string { + const run = dirname(input.scratchDir); + const resolved = run.replace(tmpdir(), realpathSync(tmpdir())); + return [resolved, run].reduce((named, spelling) => named.split(spelling).join(''), text); +} + /** The policy the spawn compiles, with this run's scratch directory written as one name. */ function compiledPolicy(input: SandboxedRunInput): string { const policy = compileSandboxPolicy(input.binding, { @@ -184,7 +197,7 @@ function compiledPolicy(input: SandboxedRunInput): string { ...(input.widening !== undefined ? { widening: input.widening } : {}), ...(input.toolPaths !== undefined ? { toolPaths: input.toolPaths } : {}), }); - return JSON.stringify(policy).split(input.scratchDir).join(''); + return scratchNamed(JSON.stringify(policy), input); } describe('a harness run with a browser', () => { @@ -212,15 +225,16 @@ describe('a harness run with a browser', () => { // Everything else it was handed is the same too; only the command line gained the configuration. const rest = (input: SandboxedRunInput): string => - JSON.stringify({ - env: input.env, - trustedEnv: input.trustedEnv, - widening: input.widening, - toolPaths: input.toolPaths, - inheritEnv: input.inheritEnv, - }) - .split(input.scratchDir) - .join(''); + scratchNamed( + JSON.stringify({ + env: input.env, + trustedEnv: input.trustedEnv, + widening: input.widening, + toolPaths: input.toolPaths, + inheritEnv: input.inheritEnv, + }), + input, + ); expect(rest(browsing)).toBe(rest(plain)); const at = browsing.argv.indexOf('--mcp-config'); const config = browsing.argv[at + 1] ?? ''; diff --git a/apps/aflow-executor-host/src/__tests__/harnessResult.test.ts b/apps/aflow-executor-host/src/__tests__/harnessResult.test.ts index e6967272..0f80af6c 100644 --- a/apps/aflow-executor-host/src/__tests__/harnessResult.test.ts +++ b/apps/aflow-executor-host/src/__tests__/harnessResult.test.ts @@ -209,7 +209,6 @@ describe('the wire contract', () => { truncated: false, applies: 'empty', headMoved: false, - refChanges: [], blockedDomains: [], }); expect(output.result).toEqual({ verdict: 'sound' }); @@ -232,44 +231,10 @@ describe('the wire contract', () => { truncated: false, applies: 'empty', headMoved: false, - refChanges: [], blockedDomains: [], }), ).toThrow(); }); - - it('reports what changed among the local branches and tags, and requires the field', () => { - const output = { - runId: 'hr_1', - harness: { id: 'claude' }, - continued: false, - baseSha: 'abc', - filesChanged: 0, - patchTruncated: false, - exitCode: 0, - timedOut: false, - durationMs: 10, - truncated: false, - applies: 'empty', - headMoved: false, - blockedDomains: [], - }; - const refChanges = [ - { ref: 'refs/heads/mine', change: 'created', to: 'def' }, - { ref: 'refs/heads/old', change: 'deleted', from: 'abc' }, - { ref: 'refs/tags/v1', change: 'moved', from: 'abc', to: 'def' }, - ]; - expect(HostHarnessRunOutputSchema.parse({ ...output, refChanges }).refChanges).toEqual( - refChanges, - ); - expect(HostHarnessRunOutputSchema.safeParse(output).success).toBe(false); - expect( - HostHarnessRunOutputSchema.safeParse({ - ...output, - refChanges: [{ ref: 'refs/heads/x', change: 'renamed' }], - }).success, - ).toBe(false); - }); }); describe('a harness is never waiting on input nobody is sending', () => { @@ -407,9 +372,10 @@ describe.runIf(sandboxReadiness().ready)('a commission starts from a named ref', let repo: string; let feature: string; let other: string; - let operatorDone: string; const vcs = async (...args: string[]): Promise => (await promisify(execFile)('git', args, { cwd: repo })).stdout.trim(); + const folderRefs = async (): Promise => + await vcs('for-each-ref', '--format=%(refname) %(objectname)'); beforeAll(async () => { const base = await mkdtemp(join(tmpdir(), 'host-harness-base-')); @@ -428,17 +394,10 @@ describe.runIf(sandboxReadiness().ready)('a commission starts from a named ref', await writeFile(join(repo, 'README.md'), '# project, moved on\n', 'utf8'); await vcs('commit', '-am', 'main moves on'); - // Inside the connected folder, which a harness may read: the harness waits - // on it, so the operator's branch is made while the run is in flight. - operatorDone = join(repo, '.operator-done'); const harness = (id: string, script: string): Record => ({ id, executable: '/bin/sh', args: ['-c', script], - // The shared refs live under the repository's `.git`, which a harness - // cannot write unless its profile opens it. Opened here so only the hook - // stands between the harness and the operator's branches. - writePaths: [join(repo, '.git')], }); policyPath = join(base, 'host-policy.json'); await writeFile( @@ -463,16 +422,26 @@ describe.runIf(sandboxReadiness().ready)('a commission starts from a named ref', sessionArgs: ['{session}'], resumeArgs: ['{session}'], }, - harness('deletes', 'printf changed > touched.txt; git branch -D other'), - harness( - 'opts-out', - 'printf changed > touched.txt; git -c core.hooksPath=/dev/null branch -D other', - ), - harness('pushes', 'printf changed > touched.txt; git push -q . HEAD:refs/heads/pushed'), + // Every way around a hook the run's git used to be given, and none is + // given now: a hooks path of its own, a branch deleted and moved, and + // pushes into the folder's path, then a record of where it all landed. harness( - 'waits', - 'printf changed > touched.txt; i=0; ' + - `while [ ! -e '${operatorDone}' ] && [ $i -lt 600 ]; do sleep 0.1; i=$((i+1)); done`, + 'goes-around', + [ + 'printf changed > touched.txt', + 'git branch -D other', + 'git -c core.hooksPath=/dev/null update-ref refs/heads/main HEAD', + 'git -c core.hooksPath=/dev/null tag planted', + 'git push -q . HEAD:refs/heads/pushed', + `git push -q '${repo}' HEAD:refs/heads/pushed-into-folder`, + `git push -q -f '${repo}' HEAD:refs/heads/main`, + `git -c core.hooksPath=/dev/null push -q '${repo}' :refs/heads/feat/reviewed`, + '{ git rev-parse --absolute-git-dir; ' + + 'echo "config-env=${GIT_CONFIG_COUNT:-none}"; ' + + 'git config --local --get core.hooksPath || echo no-hooks-path; ' + + 'git remote | wc -l | tr -d " "; ' + + "git for-each-ref --format='%(refname)' refs/heads/ refs/tags/; } > clone.txt", + ].join('; '), ), ], }), @@ -569,104 +538,81 @@ describe.runIf(sandboxReadiness().ready)('a commission starts from a named ref', }, 120_000); it.skipIf(confined.skip)( - confined.title("stops the agent's own git from deleting a branch, and the run still succeeds"), - { tags: ['listener'], timeout: 120_000 }, + confined.title( + "leaves every ref of the folder as it was whatever git the run uses, and its publication lands exactly the run's commit", + ), + { tags: ['listener'], timeout: 180_000 }, async () => { - await vcs('branch', '-f', 'other', other); - const { outcome, written } = await runWith({ harness: 'deletes' }); + const before = await folderRefs(); + const { outcome, written } = await runWith({ + harness: 'goes-around', + base: 'feat/reviewed', + }); expect(outcome.status).toBe('SUCCEEDED'); const output = written['output'] as Record; + expect(await folderRefs()).toBe(before); expect(await vcs('rev-parse', 'other')).toBe(other); - expect(output['refChanges']).toEqual([]); - expect(String(output['stderr'])).toContain( - 'Refused `refs/heads/other`: a commission may not move branches or tags', - ); - expect(String(output['patch'])).toContain('touched.txt'); - }, - ); - - it.skipIf(confined.skip)( - confined.title( - 'reports a branch moved by a git call that names its own hooks path, which the guard does not stop', - ), - { tags: ['listener'], timeout: 120_000 }, - async () => { - await vcs('branch', '-f', 'other', other); - try { - const { outcome, written } = await runWith({ harness: 'opts-out' }); - expect(outcome.status).toBe('SUCCEEDED'); - const output = written['output'] as Record; - expect(output['refChanges']).toEqual([ - { ref: 'refs/heads/other', change: 'deleted', from: other }, - ]); - } finally { - await vcs('branch', '-f', 'other', other); - } - }, - ); - - it.skipIf(confined.skip)( - confined.title( - 'reports a branch made by a local push from the checkout, which the guard does not stop', - ), - { tags: ['listener'], timeout: 120_000 }, - async () => { + expect(await vcs('rev-parse', 'feat/reviewed')).toBe(feature); + expect(output).not.toHaveProperty('refChanges'); + + // Where the run's git landed: the clone's git directory beside the + // checkout in the run's scratch, given no hooks path and no config through + // the environment, naming no remote — and written by none of it, so its + // refs are the folder's as the clone copied them. + const patch = String(output['patch']); + expect(patch).toContain('+changed'); + const gitDir = /^\+(\/.*)$/m.exec(patch.slice(patch.indexOf('+++ b/clone.txt')))?.[1]; + expect(gitDir).toMatch(/\/git-work$/); + expect(gitDir).not.toBe(join(repo, '.git')); + expect(patch).toContain('+config-env=none\n'); + expect(patch).toContain('+no-hooks-path\n'); + expect(patch).toContain('+0\n'); + expect(patch).toContain('+refs/heads/other\n'); + expect(patch).not.toContain('+refs/heads/pushed\n'); + expect(patch).not.toContain('+refs/tags/planted\n'); + + const published: Record = {}; + const publication = await createHostPatchHandler(policyPath).execute({ + operationId: 'host.file.patch', + spaceId: 'space-test', + runId: 'run-base', + job: { inputRef: 'inline:publication' }, + signal: new AbortController().signal, + log: { error: () => undefined, warn: () => undefined, info: () => undefined }, + readPayload: () => + Promise.resolve({ + bindingId: 'hb', + patch, + commit: { + branch: 'aflow/around', + message: 'What the run changed', + baseSha: output['baseSha'], + }, + }), + emitLiveDelta: () => Promise.resolve(), + writePayload: (kind: string, data: unknown) => { + published[kind] = data; + return Promise.resolve(`inline:${kind}`); + }, + } as never); try { - const { outcome, written } = await runWith({ harness: 'pushes' }); - expect(outcome.status).toBe('SUCCEEDED'); - const output = written['output'] as Record; - expect(output['refChanges']).toEqual([ - { ref: 'refs/heads/pushed', change: 'created', to: output['baseSha'] }, - ]); + expect(publication.status).toBe('SUCCEEDED'); + const commit = (published['output'] as { commit: { sha: string } }).commit; + expect(await vcs('rev-parse', 'refs/heads/aflow/around')).toBe(commit.sha); + expect(await vcs('rev-parse', `${commit.sha}^`)).toBe(feature); + expect(await vcs('show', `${commit.sha}:touched.txt`)).toBe('changed'); + // The one write to the folder's refs is the branch the publication names. + expect(await folderRefs()).toBe( + [...before.split('\n'), `refs/heads/aflow/around ${commit.sha}`] + .sort((a, b) => (a < b ? -1 : 1)) + .join('\n'), + ); } finally { - await vcs('branch', '-D', 'pushed').catch(() => undefined); + await vcs('branch', '-D', 'aflow/around').catch(() => undefined); } }, ); - it.skipIf(confined.skip)( - confined.title( - 'reports a branch the operator made while the run was in flight, and refuses nothing', - ), - { tags: ['listener'], timeout: 120_000 }, - async () => { - await rm(operatorDone, { force: true }); - const running = runWith({ harness: 'waits' }); - // The harness has started once its edit is in its checkout, which is after - // the refs were first read. - let started = false; - for (let i = 0; i < 600 && !started; i += 1) { - const listing = await vcs('worktree', 'list', '--porcelain'); - const checkouts = listing - .split('\n') - .filter((line) => line.startsWith('worktree ')) - .map((line) => line.slice('worktree '.length)); - for (const path of checkouts) { - if ( - await stat(join(path, 'touched.txt')).then( - () => true, - () => false, - ) - ) - started = true; - } - if (!started) await new Promise((resolve) => setTimeout(resolve, 100)); - } - expect(started).toBe(true); - await vcs('branch', 'operator-work'); - const made = await vcs('rev-parse', 'operator-work'); - await writeFile(operatorDone, '', 'utf8'); - - const { outcome, written } = await running; - expect(outcome.status).toBe('SUCCEEDED'); - const output = written['output'] as Record; - expect(output['refChanges']).toEqual([ - { ref: 'refs/heads/operator-work', change: 'created', to: made }, - ]); - expect(String(output['patch'])).toContain('touched.txt'); - }, - ); - it("starts from the remote's branch as it is now when the base names a remote", async () => { const upstream = await mkdtemp(join(tmpdir(), 'host-harness-upstream-')); const elsewhere = await mkdtemp(join(tmpdir(), 'host-harness-elsewhere-')); diff --git a/apps/aflow-executor-host/src/__tests__/harnessRetry.test.ts b/apps/aflow-executor-host/src/__tests__/harnessRetry.test.ts index 4ae4e05b..8fff1b17 100644 --- a/apps/aflow-executor-host/src/__tests__/harnessRetry.test.ts +++ b/apps/aflow-executor-host/src/__tests__/harnessRetry.test.ts @@ -104,7 +104,7 @@ beforeEach(async () => { afterEach(async () => { for (const session of allSessions()) { forgetSession(session.id); - await removeWorktree(session.bindingRoot, session.worktreePath).catch(() => {}); + await removeWorktree(session.worktreePath).catch(() => {}); await discardScratch(session); } await rm(base, { recursive: true, force: true }); diff --git a/apps/aflow-executor-host/src/__tests__/mergeFrom.test.ts b/apps/aflow-executor-host/src/__tests__/mergeFrom.test.ts index e116946a..0a8e0303 100644 --- a/apps/aflow-executor-host/src/__tests__/mergeFrom.test.ts +++ b/apps/aflow-executor-host/src/__tests__/mergeFrom.test.ts @@ -198,7 +198,7 @@ beforeEach(async () => { afterEach(async () => { for (const session of allSessions()) { forgetSession(session.id); - await removeWorktree(session.bindingRoot, session.worktreePath).catch(() => {}); + await removeWorktree(session.worktreePath).catch(() => {}); await discardScratch(session); } await rm(base, { recursive: true, force: true }); @@ -415,16 +415,16 @@ describe('a commission refuses a merge no turn could finish', () => { }); try { const refused = await mergeIntoCheckout( - checkout.path, + checkout, mainHead, - await commitIdentityArgs(checkout.path), + await commitIdentityArgs(project), ).catch((error: unknown) => error); expect(refused).toBeInstanceOf(WorktreeError); expect((refused as InstanceType).kind).toBe('binary_conflict'); expect(await head(checkout.path, 'HEAD')).toBe(branchHead); expect((await git(checkout.path, 'status', '--porcelain')).trim()).toBe(''); } finally { - await removeWorktree(project, checkout.path); + await removeWorktree(checkout.path); await rm(scratch, { recursive: true, force: true }); } }, 60_000); @@ -532,10 +532,10 @@ async function mergedTree(at: string, from: string): Promise { const scratch = await mkdtemp(join(tmpdir(), 'aflow-merge-tree-')); const checkout = await prepareWorktree(project, scratch, 'merge', { dependencies: 'none', at }); try { - await mergeIntoCheckout(checkout.path, from, await commitIdentityArgs(checkout.path)); + await mergeIntoCheckout(checkout, from, await commitIdentityArgs(project)); return await head(checkout.path, 'HEAD^{tree}'); } finally { - await removeWorktree(project, checkout.path); + await removeWorktree(checkout.path); await rm(scratch, { recursive: true, force: true }); } } @@ -877,15 +877,15 @@ describe('the same two parents merge to the same tree', () => { }); try { const merge = await mergeIntoCheckout( - checkout.path, + checkout, mainHead, - await commitIdentityArgs(checkout.path), + await commitIdentityArgs(project), ); expect(merge?.conflicts).toEqual([{ path: 'a.txt', kind: 'content' }]); expect(await readFile(join(checkout.path, 'a.txt'), 'utf8')).toMatch(/^<<<<<<< HEAD$/m); trees.push(await head(checkout.path, 'HEAD^{tree}')); } finally { - await removeWorktree(project, checkout.path); + await removeWorktree(checkout.path); await rm(scratch, { recursive: true, force: true }); } } @@ -921,14 +921,14 @@ describe('the same two parents merge to the same tree', () => { }); try { const merge = await mergeIntoCheckout( - checkout.path, + checkout, mainHead, - await commitIdentityArgs(checkout.path), + await commitIdentityArgs(project), ); expect(merge?.conflicts).toEqual([]); trees.push(await head(checkout.path, 'HEAD^{tree}')); } finally { - await removeWorktree(project, checkout.path); + await removeWorktree(checkout.path); await rm(scratch, { recursive: true, force: true }); } } diff --git a/apps/aflow-executor-host/src/__tests__/patchApply.test.ts b/apps/aflow-executor-host/src/__tests__/patchApply.test.ts index f36c01b4..737a9688 100644 --- a/apps/aflow-executor-host/src/__tests__/patchApply.test.ts +++ b/apps/aflow-executor-host/src/__tests__/patchApply.test.ts @@ -369,6 +369,34 @@ describe('a conflicted merge says what it actually did', () => { }); describe('landing a patch as a commit on a new branch', () => { + it('fetches the commit from its own clone into the folder, writing the named branch and no other ref', async () => { + const patch = await diffFor(async (d) => { + await writeFile(join(d, 'a.txt'), 'one\nEDITED\nthree\n'); + }); + await git(root, 'branch', 'other'); + await git(root, 'tag', 'v1'); + const refsBefore = await git(root, 'for-each-ref', '--format=%(refname) %(objectname)'); + + const captured: Captured = {}; + const result = await createHostPatchHandler(policyPath).execute( + contextFor( + { bindingId: 'hb', patch, commit: { branch: 'aflow/x', message: 'the change' } }, + captured, + ), + ); + + expect(result.status).toBe('SUCCEEDED'); + const sha = (captured.output?.['commit'] as Record)['sha'] ?? ''; + expect((await git(root, 'cat-file', '-t', sha)).trim()).toBe('commit'); + expect(await git(root, 'for-each-ref', '--format=%(refname) %(objectname)')).toBe( + [...refsBefore.trim().split('\n'), `refs/heads/aflow/x ${sha}`] + .sort((a, b) => (a < b ? -1 : 1)) + .join('\n') + '\n', + ); + // The fetch wrote no FETCH_HEAD another fetch of the folder's could be reading. + await expect(readFile(join(root, '.git', 'FETCH_HEAD'), 'utf8')).rejects.toThrow(); + }, 30_000); + it('leaves the operator checkout exactly as it was', async () => { // The whole reason this mode exists: the operator is a second writer, and // a publication must not need their working tree, index or branch. diff --git a/apps/aflow-executor-host/src/__tests__/phase2Acceptance.test.ts b/apps/aflow-executor-host/src/__tests__/phase2Acceptance.test.ts index fe8ce0e3..e0ea58a6 100644 --- a/apps/aflow-executor-host/src/__tests__/phase2Acceptance.test.ts +++ b/apps/aflow-executor-host/src/__tests__/phase2Acceptance.test.ts @@ -3,7 +3,7 @@ * Each test names the criterion it stands for. */ import { execFile } from 'node:child_process'; -import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; +import { mkdtemp, readFile, rm, stat, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { promisify } from 'node:util'; @@ -28,6 +28,13 @@ async function git(cwd: string, ...args: string[]): Promise { return stdout; } +async function exists(path: string): Promise { + return await stat(path).then( + () => true, + () => false, + ); +} + beforeEach(async () => { repo = await mkdtemp(join(tmpdir(), 'aflow-p2-repo-')); scratch = await mkdtemp(join(tmpdir(), 'aflow-p2-scratch-')); @@ -69,7 +76,7 @@ describe('Phase 2 acceptance', () => { writeFile(join(a.path, 'a.txt'), 'A rewrote this\n'), writeFile(join(b.path, 'b.txt'), 'B rewrote this\n'), ]); - const [ca, cb] = await Promise.all([collectChanges(a.path), collectChanges(b.path)]); + const [ca, cb] = await Promise.all([collectChanges(a), collectChanges(b)]); expect(ca.patch).toContain('A rewrote this'); expect(ca.patch).not.toContain('B rewrote this'); @@ -77,8 +84,8 @@ describe('Phase 2 acceptance', () => { expect(cb.patch).not.toContain('A rewrote this'); expect(a.baseSha).toBe(b.baseSha); - await removeWorktree(repo, a.path); - await removeWorktree(repo, b.path); + await removeWorktree(a.path); + await removeWorktree(b.path); }, GIT_HEAVY_MS, ); @@ -90,8 +97,8 @@ describe('Phase 2 acceptance', () => { await writeFile(join(repo, 'a.txt'), 'MY EDIT\n'); const wt = await prepareWorktree(repo, scratch, 'run'); await writeFile(join(wt.path, 'a.txt'), 'harness edit\n'); - await collectChanges(wt.path); - await removeWorktree(repo, wt.path); + await collectChanges(wt); + await removeWorktree(wt.path); expect(await readFile(join(repo, 'a.txt'), 'utf8')).toBe('MY EDIT\n'); }, GIT_HEAVY_MS, @@ -106,13 +113,13 @@ describe('Phase 2 acceptance', () => { const b = await prepareWorktree(repo, scratch, 'run-b'); await writeFile(join(b.path, 'a.txt'), 'B kept working\n'); - await removeWorktree(repo, a.path); + await removeWorktree(a.path); - const cb = await collectChanges(b.path); + const cb = await collectChanges(b); expect(cb.patch).toContain('B kept working'); - expect(await git(repo, 'worktree', 'list')).not.toContain(a.path); - expect(await git(repo, 'worktree', 'list')).toContain(b.path); - await removeWorktree(repo, b.path); + expect(await exists(a.path)).toBe(false); + expect(await exists(join(b.path, 'a.txt'))).toBe(true); + await removeWorktree(b.path); }, GIT_HEAVY_MS, ); @@ -123,10 +130,10 @@ describe('Phase 2 acceptance', () => { // "collect partial work" const wt = await prepareWorktree(repo, scratch, 'run'); await writeFile(join(wt.path, 'a.txt'), 'one\nEDITED\nthree\n'); - const changes = await collectChanges(wt.path); + const changes = await collectChanges(wt); expect(changes.filesChanged).toBe(1); expect(changes.patch).toContain('EDITED'); - await removeWorktree(repo, wt.path); + await removeWorktree(wt.path); }, GIT_HEAVY_MS, ); @@ -137,7 +144,7 @@ describe('Phase 2 acceptance', () => { // "detect an apply conflict" const wt = await prepareWorktree(repo, scratch, 'run'); await writeFile(join(wt.path, 'a.txt'), 'one\nHARNESS\nthree\n'); - const changes = await collectChanges(wt.path); + const changes = await collectChanges(wt); // The operator moves the same lines while the run was going. await writeFile(join(repo, 'a.txt'), 'one\nOPERATOR\nthree\n'); @@ -147,7 +154,7 @@ describe('Phase 2 acceptance', () => { expect(applies.state).toBe('conflict'); if (applies.state === 'conflict') expect(applies.detail).toContain('a.txt'); expect(await currentHead(repo)).not.toBe(wt.baseSha); - await removeWorktree(repo, wt.path); + await removeWorktree(wt.path); }, GIT_HEAVY_MS, ); @@ -158,14 +165,14 @@ describe('Phase 2 acceptance', () => { // A moved HEAD is not a conflict; only overlapping edits are. const wt = await prepareWorktree(repo, scratch, 'run'); await writeFile(join(wt.path, 'a.txt'), 'one\nHARNESS\nthree\n'); - const changes = await collectChanges(wt.path); + const changes = await collectChanges(wt); await writeFile(join(repo, 'b.txt'), 'operator touched another file\n'); await git(repo, 'commit', '-am', 'unrelated edit'); expect((await checkApplies(repo, changes.patch)).state).toBe('clean'); expect(await currentHead(repo)).not.toBe(wt.baseSha); - await removeWorktree(repo, wt.path); + await removeWorktree(wt.path); }, GIT_HEAVY_MS, ); diff --git a/apps/aflow-executor-host/src/__tests__/refGuard.test.ts b/apps/aflow-executor-host/src/__tests__/refGuard.test.ts deleted file mode 100644 index 96c364d9..00000000 --- a/apps/aflow-executor-host/src/__tests__/refGuard.test.ts +++ /dev/null @@ -1,326 +0,0 @@ -/** - * Contract: a coding agent's ordinary git, run with the environment the - * executor gives it, cannot create, delete or move a local branch or tag of the repository its - * checkout shares refs with — and everything else a coding run does with git - * still works. - */ -import { execFile } from 'node:child_process'; -import { cp, mkdir, mkdtemp, realpath, rm, symlink, writeFile } from 'node:fs/promises'; -import { tmpdir } from 'node:os'; -import { basename, dirname, join } from 'node:path'; -import { promisify } from 'node:util'; - -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; - -import { buildBaseEnv, createWorkloadDirs } from '../baseEnv.js'; -import { - installRefGuard, - noRefGuardMessage, - parseGitVersion, - refGuardMissing, - refGuardReadiness, -} from '../refGuard.js'; -import { prepareWorktree, removeWorktree, snapshotRefs } from '../worktree.js'; - -const run = promisify(execFile); - -let repo: string; -let scratch: string; -let checkout: string; -let harnessEnv: Record; - -async function operatorGit(...args: string[]): Promise { - return (await run('git', ['-C', repo, ...args])).stdout.trim(); -} - -interface AgentOutcome { - ok: boolean; - stderr: string; -} - -async function agentGitIn( - cwd: string, - env: Record, - ...args: string[] -): Promise { - try { - const { stderr } = await run('git', args, { cwd, env: { ...harnessEnv, ...env } }); - return { ok: true, stderr }; - } catch (error) { - return { ok: false, stderr: String((error as { stderr?: unknown }).stderr ?? '') }; - } -} - -async function agentGit(...args: string[]): Promise { - return await agentGitIn(checkout, {}, ...args); -} - -beforeEach(async () => { - repo = await mkdtemp(join(tmpdir(), 'aflow-guard-repo-')); - scratch = await mkdtemp(join(tmpdir(), 'aflow-guard-scratch-')); - await operatorGit('init', '--initial-branch=main'); - await operatorGit('config', 'user.email', 'test@example.com'); - await operatorGit('config', 'user.name', 'Test'); - await writeFile(join(repo, 'app.txt'), 'committed\n'); - await operatorGit('add', '-A'); - await operatorGit('commit', '-m', 'initial'); - await operatorGit('branch', 'other'); - checkout = (await prepareWorktree(repo, scratch, 'work', { dependencies: 'none' })).path; - await createWorkloadDirs(scratch); - // What the harness process is given: the confined base environment, the - // same global- and system-config withholding the lane's own git uses, and - // the guard on top. - harnessEnv = { - ...buildBaseEnv(scratch), - GIT_CONFIG_GLOBAL: '/dev/null', - GIT_CONFIG_NOSYSTEM: '1', - ...(await installRefGuard(scratch, repo)), - }; -}); - -afterEach(async () => { - await removeWorktree(repo, checkout); - await rm(repo, { recursive: true, force: true }); - await rm(scratch, { recursive: true, force: true }); -}); - -const REFUSAL = 'a commission may not move branches or tags'; - -describe("the agent's git cannot move a branch or a tag", () => { - it.each([ - ['branch planted', 'refs/heads/planted'], - ['tag planted', 'refs/tags/planted'], - ['update-ref refs/heads/planted HEAD', 'refs/heads/planted'], - ['branch -D other', 'refs/heads/other'], - ])('refuses `git %s`, naming the ref', async (command, ref) => { - const before = await snapshotRefs(repo); - const outcome = await agentGit(...command.split(' ')); - expect(outcome.ok).toBe(false); - expect(outcome.stderr).toContain(`Refused \`${ref}\`: ${REFUSAL}`); - expect(await snapshotRefs(repo)).toEqual(before); - }); - - it('holds when the repository names a hooks path of its own', async () => { - await operatorGit('config', 'core.hooksPath', '.githooks'); - const outcome = await agentGit('branch', 'planted'); - expect(outcome.ok).toBe(false); - expect(outcome.stderr).toContain(REFUSAL); - }); - - it("refuses in the folder's own checkout as well as the run's", async () => { - const outcome = await agentGit('-C', repo, 'branch', 'planted'); - expect(outcome.ok).toBe(false); - expect(outcome.stderr).toContain(`Refused \`refs/heads/planted\`: ${REFUSAL}`); - }); - - describe('however the path to the repository is spelled', () => { - let elsewhere: string; - - beforeEach(async () => { - elsewhere = await mkdtemp(join(tmpdir(), 'aflow-guard-elsewhere-')); - }); - - afterEach(async () => { - await rm(elsewhere, { recursive: true, force: true }); - }); - - interface Invocation { - cwd: string; - env?: Record; - args?: string[]; - } - - const spellings: [string, () => Promise][] = [ - ["the run's checkout", () => Promise.resolve({ cwd: checkout })], - [ - "a subdirectory of the run's checkout", - async () => { - await mkdir(join(checkout, 'nested', 'deeper'), { recursive: true }); - return { cwd: join(checkout, 'nested', 'deeper') }; - }, - ], - ["the folder's own checkout", () => Promise.resolve({ cwd: repo })], - [ - 'a relative GIT_DIR of ../.git', - async () => { - await mkdir(join(repo, 'nested'), { recursive: true }); - return { cwd: join(repo, 'nested'), env: { GIT_DIR: '../.git' } }; - }, - ], - [ - '--git-dir', - () => Promise.resolve({ cwd: elsewhere, args: [`--git-dir=${join(repo, '.git')}`] }), - ], - [ - 'a symlink to the repository', - async () => { - await symlink(repo, join(elsewhere, 'linked')); - return { cwd: join(elsewhere, 'linked') }; - }, - ], - [ - "a GIT_DIR naming the checkout's entry under .git/worktrees", - async () => { - const gitDir = (await run('git', ['-C', checkout, 'rev-parse', '--absolute-git-dir'])) - .stdout; - return { cwd: elsewhere, env: { GIT_DIR: gitDir.trim() } }; - }, - ], - [ - 'a relative GIT_DIR that begins with a dash', - async () => { - await symlink(join(repo, '.git'), join(elsewhere, '-P')); - return { cwd: elsewhere, env: { GIT_DIR: '-P' } }; - }, - ], - [ - 'a CDPATH that holds another .git', - async () => { - await mkdir(join(elsewhere, '.git')); - return { cwd: repo, env: { CDPATH: elsewhere } }; - }, - ], - [ - "a git-new-workdir layout whose refs are the folder's", - async () => { - const gitDir = join(elsewhere, '.git'); - await mkdir(gitDir); - for (const shared of ['refs', 'objects', 'config']) { - await symlink(join(repo, '.git', shared), join(gitDir, shared)); - } - await writeFile(join(gitDir, 'HEAD'), 'ref: refs/heads/main\n'); - return { cwd: elsewhere }; - }, - ], - ]; - - // A macOS volume keeps the letter case a path was typed with and answers - // to any other, and the data volume is reachable under its firmlink too. - const macSpellings: [string, () => Promise][] = [ - [ - 'a GIT_DIR in a different letter case than the folder', - async () => { - const folder = await realpath(repo); - const recased = join(dirname(folder), basename(folder).toUpperCase(), '.git'); - return { cwd: elsewhere, env: { GIT_DIR: recased } }; - }, - ], - [ - 'a GIT_DIR under the data volume firmlink', - async () => ({ - cwd: elsewhere, - env: { GIT_DIR: join('/System/Volumes/Data', await realpath(repo), '.git') }, - }), - ], - ]; - - async function refusesThrough(arrange: () => Promise): Promise { - const { cwd, env = {}, args = [] } = await arrange(); - const before = await snapshotRefs(repo); - const outcome = await agentGitIn(cwd, env, ...args, 'branch', 'planted'); - expect(outcome.ok).toBe(false); - expect(outcome.stderr).toContain(`Refused \`refs/heads/planted\`: ${REFUSAL}`); - expect(await snapshotRefs(repo)).toEqual(before); - } - - it.each(spellings)('refuses through %s', async (_spelling, arrange) => { - await refusesThrough(arrange); - }); - - it.runIf(process.platform === 'darwin').each(macSpellings)( - 'refuses through %s', - async (_spelling, arrange) => { - await refusesThrough(arrange); - }, - ); - }); - - it('leaves the same command to the operator, whose git carries no guard', async () => { - await operatorGit('branch', 'planted'); - expect((await snapshotRefs(repo)).has('refs/heads/planted')).toBe(true); - }); -}); - -describe('what a coding run does with git still works', () => { - it('commits on the detached HEAD', async () => { - await writeFile(join(checkout, 'app.txt'), 'edited\n'); - expect((await agentGit('commit', '-am', 'work')).ok).toBe(true); - }); - - it('stashes', async () => { - await writeFile(join(checkout, 'app.txt'), 'in progress\n'); - expect((await agentGit('stash')).ok).toBe(true); - }); - - it('moves a remote-tracking ref', async () => { - expect((await agentGit('update-ref', 'refs/remotes/origin/x', 'HEAD')).ok).toBe(true); - }); - - it('creates and commits in a repository of its own, as a test suite does', async () => { - const own = await mkdtemp(join(tmpdir(), 'aflow-guard-own-')); - try { - await writeFile(join(own, 'fixture.txt'), 'fixture\n'); - for (const args of [ - ['init', '--initial-branch=main'], - ['add', '-A'], - ['-c', 'user.email=test@example.com', '-c', 'user.name=Test', 'commit', '-m', 'fixture'], - ['branch', 'feature'], - ['tag', 'v1'], - ]) { - const outcome = await agentGit('-C', own, ...args); - expect(outcome, `git ${args.join(' ')}`).toMatchObject({ ok: true }); - } - } finally { - await rm(own, { recursive: true, force: true }); - } - }); - - it("moves branches in a copy of the folder's .git, which has refs of its own", async () => { - const copy = await mkdtemp(join(tmpdir(), 'aflow-guard-copy-')); - try { - await cp(join(repo, '.git'), join(copy, '.git'), { recursive: true }); - const before = await snapshotRefs(repo); - expect(await agentGitIn(copy, {}, 'branch', 'planted')).toMatchObject({ ok: true }); - expect(await snapshotRefs(repo)).toEqual(before); - } finally { - await rm(copy, { recursive: true, force: true }); - } - }); -}); - -describe('the git a commission needs', () => { - it('reads the version however the build spells it', () => { - expect(parseGitVersion('git version 2.52.0\n')).toEqual({ major: 2, minor: 52, patch: 0 }); - expect(parseGitVersion('git version 2.39.5 (Apple Git-154)')).toEqual({ - major: 2, - minor: 39, - patch: 5, - }); - expect(parseGitVersion('git version 2.45.1.windows.1')).toEqual({ - major: 2, - minor: 45, - patch: 1, - }); - expect(parseGitVersion('not git')).toBeUndefined(); - }); - - it('names the version found and the version needed below the floor', () => { - expect(refGuardMissing('git version 2.28.0')).toEqual([]); - expect(refGuardMissing('git version 3.0.0')).toEqual([]); - const [missing] = refGuardMissing('git version 2.27.1'); - expect(missing).toContain('git 2.28 or later'); - expect(missing).toContain('this machine has 2.27.1'); - expect(refGuardMissing(undefined)).toHaveLength(1); - expect(refGuardMissing('something else')[0]).toContain('`something else`'); - }); - - it('refuses in the shape of the sandbox refusal, carrying what is missing', () => { - const message = noRefGuardMessage(refGuardMissing('git version 2.20.1')); - expect(message).toContain('cannot stop a coding agent'); - expect(message).toContain('Missing: git 2.28 or later — this machine has 2.20.1'); - }); - - it('finds this machine ready', async () => { - expect(await refGuardReadiness()).toEqual({ ready: true, missing: [] }); - }); -}); diff --git a/apps/aflow-executor-host/src/__tests__/sandboxPolicy.test.ts b/apps/aflow-executor-host/src/__tests__/sandboxPolicy.test.ts index 44b599dd..cae16940 100644 --- a/apps/aflow-executor-host/src/__tests__/sandboxPolicy.test.ts +++ b/apps/aflow-executor-host/src/__tests__/sandboxPolicy.test.ts @@ -5,11 +5,17 @@ * The adapter's own schema validates the shape, so its version bump surfaces * here rather than at the first sandboxed command. */ +import { join } from 'node:path'; + import { SandboxRuntimeConfigSchema } from '@anthropic-ai/sandbox-runtime'; import { describe, expect, it } from 'vitest'; import type { HostBinding } from '../bindings.js'; -import { FORBIDDEN_SANDBOX_OPTIONS, compileSandboxPolicy } from '../sandboxPolicy.js'; +import { + FORBIDDEN_SANDBOX_OPTIONS, + checkoutWidening, + compileSandboxPolicy, +} from '../sandboxPolicy.js'; const HOME = '/Users/probe'; const binding: HostBinding = { @@ -55,6 +61,25 @@ describe('what a repository exposes to a confined command', () => { expect(policy.filesystem.denyWrite).toContain('/Users/probe/projects/demo/**/.git/config'); }); + it("reads a checkout's git directory in the writable scratch and never writes it", () => { + // Its config is what the executor's own git reads when it stages the run. + const gitDir = '/tmp/aflow-host/git-work'; + const policy = compileSandboxPolicy(binding, { + home: HOME, + scratchDir: '/tmp/aflow-host', + widening: { + authPaths: [], + allowedDomains: [], + writableRoot: '/tmp/aflow-host/work', + gitDir, + withholdBindingWrite: true, + }, + }); + expect(policy.filesystem.allowRead).toContain(gitDir); + expect(policy.filesystem.allowWrite).toContain('/tmp/aflow-host'); + expect(policy.filesystem.denyWrite).toContain(gitDir); + }); + it('still refuses a read-only binding any write at all', () => { expect(compile({ mode: 'read' }).filesystem.allowWrite).not.toContain(binding.root); }); @@ -108,3 +133,83 @@ describe('sandbox policy compilation', () => { expect(() => SandboxRuntimeConfigSchema.parse(compile())).not.toThrow(); }); }); + +describe('the boundary of a run in a checkout, whole', () => { + // Every path either policy names, stated once, so a path gained or lost on + // either side of the boundary fails here rather than being found in a run. + const HOST_DIR = join(HOME, '.aflow'); + const SCRATCH = '/tmp/aflow-harness-x'; + const AGENT = join(SCRATCH, 'agent'); + const CLONE = join(SCRATCH, 'work'); + const GIT_DIR = join(SCRATCH, 'git-work'); + const TOOLCHAINS = ['.nvm', '.pyenv', '.rustup', '.cargo'].map((dir) => join(HOME, dir)); + const CARGO_CREDENTIALS = [ + join(HOME, '.cargo', 'credentials.toml'), + join(HOME, '.cargo', 'credentials'), + ]; + const hooks = (root: string): string[] => [ + join(root, '.git', 'hooks'), + join(root, '**', '.git', 'hooks'), + ]; + const config = (root: string): string[] => [ + join(root, '.git', 'config'), + join(root, '**', '.git', 'config'), + ]; + + function boundaryOf(root: string, workDir: string, borrowed: readonly string[]) { + return compileSandboxPolicy( + { ...binding, root }, + { + home: HOME, + hostDir: HOST_DIR, + scratchDir: AGENT, + widening: { + authPaths: [], + allowedDomains: [], + ...checkoutWidening({ workDir, gitDir: GIT_DIR }, borrowed), + }, + }, + ).filesystem; + } + + it("for a repository's top level: the folder, the run's own and the clone's", () => { + const root = join(HOME, 'projects', 'demo'); + expect(boundaryOf(root, CLONE, [])).toEqual({ + denyRead: [HOME, HOST_DIR, ...CARGO_CREDENTIALS, ...hooks(root), ...hooks(CLONE)], + allowRead: [root, AGENT, CLONE, GIT_DIR, ...TOOLCHAINS], + allowWrite: [AGENT, CLONE], + denyWrite: [ + HOST_DIR, + ...hooks(root), + ...hooks(CLONE), + ...config(root), + ...config(CLONE), + GIT_DIR, + ], + }); + }); + + it('for a folder inside a repository: the same, and what the clone borrows, read only', () => { + const top = join(HOME, 'projects', 'mono'); + const root = join(top, 'packages', 'app'); + const workDir = join(CLONE, 'packages', 'app'); + const borrowed = [ + join(top, '.git', 'objects'), + join(top, 'node_modules'), + join(top, 'packages', 'node_modules'), + ]; + expect(boundaryOf(root, workDir, borrowed)).toEqual({ + denyRead: [HOME, HOST_DIR, ...CARGO_CREDENTIALS, ...hooks(root), ...hooks(workDir)], + allowRead: [root, AGENT, workDir, GIT_DIR, ...borrowed, ...TOOLCHAINS], + allowWrite: [AGENT, workDir], + denyWrite: [ + HOST_DIR, + ...hooks(root), + ...hooks(workDir), + ...config(root), + ...config(workDir), + GIT_DIR, + ], + }); + }); +}); diff --git a/apps/aflow-executor-host/src/__tests__/sandboxPosture.test.ts b/apps/aflow-executor-host/src/__tests__/sandboxPosture.test.ts index 8f2e9bf2..b52bd2c6 100644 --- a/apps/aflow-executor-host/src/__tests__/sandboxPosture.test.ts +++ b/apps/aflow-executor-host/src/__tests__/sandboxPosture.test.ts @@ -15,11 +15,11 @@ * can confine one. */ import { execFile } from 'node:child_process'; -import { existsSync } from 'node:fs'; +import { existsSync, realpathSync } from 'node:fs'; import { mkdir, mkdtemp, readFile, writeFile } from 'node:fs/promises'; import { type AddressInfo, createServer } from 'node:net'; import { tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { basename, dirname, join } from 'node:path'; import { promisify } from 'node:util'; import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -30,6 +30,7 @@ import { type HostSandboxPosture, } from '@aflow/schemas'; +import { AGENT_SCRATCH_DIR } from '../executorScratch.js'; import type { SandboxedRunInput, SandboxedRunResult } from '../sandboxedRun.js'; import { CONFINEMENT_LISTENERS, requires } from './fixtures/capabilities.js'; @@ -82,6 +83,7 @@ const { const actual = await vi.importActual('../sandboxedRun.js'); const { compileSandboxPolicy, FORBIDDEN_SANDBOX_OPTIONS } = await import('../sandboxPolicy.js'); const { workloadTemp } = await import('../baseEnv.js'); +const { checkoutGitDir } = await import('../worktree.js'); const { MACHINE_LOOPBACK_REACH, runOpenPostureSelfTest } = await import('../openPostureSelfTest.js'); @@ -283,18 +285,32 @@ describe('a harness run under the folder’s posture', () => { expect(sandboxed).toHaveLength(1); const input = sandboxed[0]; expect(input?.posture).toBe('open'); - expect(input?.cwd.startsWith(tmpdir())).toBe(true); + expect(input?.cwd.startsWith(realpathSync(tmpdir()))).toBe(true); expect(input?.cwd).not.toBe(repo); + // The scratch it may write is one of its own, beside the checkout and its + // git directory: the directory holding those two is not handed to it. + const scratch = input?.scratchDir ?? ''; + expect(basename(dirname(scratch))).toBe(basename(dirname(input?.cwd ?? ''))); + expect(basename(scratch)).toBe(AGENT_SCRATCH_DIR); + // What a crash leaves is the scratch holding the checkout, so the journal + // takes the clone with its run rather than only the agent's part of it. + expect(input?.journalScratchDir).toBe(dirname(scratch)); + expect(basename(dirname(input?.cwd ?? ''))).toBe(basename(input?.journalScratchDir ?? '')); expect(input?.env).toEqual({}); expect(input?.trustedEnv).toMatchObject({ SRT_DEBUG: '1', AGENT_TOKEN: 'agent-credential', AGENT_CONFIG_DIR: expect.stringContaining('harness-config'), GIT_NO_REPLACE_OBJECTS: '1', - GIT_CONFIG_KEY_0: 'core.hooksPath', }); + // No hook is handed to the agent's git: the checkout's git directory is + // beside it, readable and never written. + expect( + Object.keys(input?.trustedEnv ?? {}).filter((key) => key.startsWith('GIT_CONFIG')), + ).toEqual([]); expect(input?.widening).toMatchObject({ writableRoot: input?.cwd, + gitDir: checkoutGitDir(input?.cwd ?? ''), withholdBindingWrite: true, }); }); @@ -311,7 +327,6 @@ describe('a harness run under the folder’s posture', () => { SRT_DEBUG: '1', AGENT_TOKEN: 'agent-credential', GIT_NO_REPLACE_OBJECTS: '1', - GIT_CONFIG_KEY_0: 'core.hooksPath', }); }); diff --git a/apps/aflow-executor-host/src/__tests__/subdirectoryBoundary.test.ts b/apps/aflow-executor-host/src/__tests__/subdirectoryBoundary.test.ts new file mode 100644 index 00000000..8fff2d0d --- /dev/null +++ b/apps/aflow-executor-host/src/__tests__/subdirectoryBoundary.test.ts @@ -0,0 +1,203 @@ +/** + * Contract: a folder connected inside a repository under the operator's home + * works inside the run, not only from outside it. + * + * Home is denied as a region and re-allowed a path at a time. The clone of + * such a folder owns no object — it borrows every one from the repository's + * store, at `/.git/objects` — and its mirrored top-level installation + * leads into `/node_modules`. Both are outside the folder and under home, + * so the run reads them only because the sandbox is told to. A linked worktree + * is a top level whose store is the main repository's, outside it, and is told + * the same. + */ +import { execFile } from 'node:child_process'; +import { mkdir, mkdtemp, realpath, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { promisify } from 'node:util'; + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { HostBindingSchema, type HostBinding } from '../bindings.js'; +import { runFolderChecks } from '../commitCheck.js'; +import { makeAgentScratch, makeScratch } from '../executorScratch.js'; +import { runSandboxed, sandboxAvailable } from '../sandboxedRun.js'; +import { checkoutWidening } from '../sandboxPolicy.js'; +import { borrowedReads, prepareWorktree, removeWorktree } from '../worktree.js'; +import { CONFINEMENT_LISTENERS, requires } from './fixtures/capabilities.js'; + +const confined = requires(...CONFINEMENT_LISTENERS); + +const run = promisify(execFile); + +const HOISTED = 'hoisted from the top level'; + +/** Reads HEAD's commit through the clone, then resolves a hoisted package. */ +const PROBE = [ + '/bin/sh', + '-c', + `git log -1 --format=%s; "${process.execPath}" -e 'console.log(require("hoisted"))'`, +]; + +let base: string; +let home: string; +let top: string; +let folder: string; +let previousHome: string | undefined; + +async function git(cwd: string, ...args: string[]): Promise { + return (await run('git', ['-C', cwd, ...args])).stdout.trim(); +} + +function bindingOf(root: string): HostBinding { + return HostBindingSchema.parse({ + id: 'hb', + root, + mode: 'readwrite', + allowsExecution: true, + sandbox: 'confined', + }); +} + +beforeEach(async () => { + base = await realpath(await mkdtemp(join(tmpdir(), 'aflow-subdir-boundary-'))); + home = join(base, 'home'); + top = join(home, 'projects', 'mono'); + folder = join(top, 'packages', 'app'); + await mkdir(folder, { recursive: true }); + await git(top, 'init', '-q', '--initial-branch=main'); + await git(top, 'config', 'user.email', 'test@example.com'); + await git(top, 'config', 'user.name', 'Test'); + await writeFile(join(top, '.gitignore'), 'node_modules\n'); + await writeFile(join(folder, 'index.js'), 'module.exports = 1;\n'); + await git(top, 'add', '-A'); + await git(top, 'commit', '-qm', 'the base commit'); + // Installed at the top level, as a workspace hoists it. + await mkdir(join(top, 'node_modules', 'hoisted'), { recursive: true }); + await writeFile( + join(top, 'node_modules', 'hoisted', 'package.json'), + '{"name":"hoisted","main":"index.js"}\n', + ); + await writeFile( + join(top, 'node_modules', 'hoisted', 'index.js'), + `module.exports = ${JSON.stringify(HOISTED)};\n`, + ); + // The sandbox denies the home it is compiled with, which is read from here. + previousHome = process.env['HOME']; + process.env['HOME'] = home; +}); + +afterEach(async () => { + if (previousHome === undefined) delete process.env['HOME']; + else process.env['HOME'] = previousHome; + await rm(base, { recursive: true, force: true }); +}); + +describe('a folder connected inside a repository under a denied home', () => { + it('borrows the repository’s object store and the installation above it', async () => { + expect(await borrowedReads(folder)).toEqual([ + join(top, '.git', 'objects'), + join(top, 'node_modules'), + ]); + expect(await borrowedReads(top)).toEqual([]); + }); + + it('borrows each installation on the way up from a folder below the scan’s depth', async () => { + const deep = join(top, 'packages', 'group', 'pkg', 'lib'); + await mkdir(deep, { recursive: true }); + await mkdir(join(top, 'packages', 'group', 'pkg', 'node_modules'), { recursive: true }); + expect(await borrowedReads(deep)).toEqual([ + join(top, '.git', 'objects'), + join(top, 'node_modules'), + join(top, 'packages', 'group', 'pkg', 'node_modules'), + ]); + }); + + it.skipIf(!sandboxAvailable() || confined.skip)( + confined.title('reads its history and resolves a hoisted package inside the run'), + { tags: ['listener'], timeout: 120_000 }, + async () => { + const sha = await git(top, 'rev-parse', 'HEAD'); + const checked = await runFolderChecks({ + binding: bindingOf(folder), + argv: PROBE, + timeoutMs: 60_000, + sha, + base: sha, + toolPaths: [], + ownerRunId: 'probe', + signal: new AbortController().signal, + }); + + expect(checked.output).toContain('the base commit'); + expect(checked.output).toContain(HOISTED); + expect(checked.result.exitCode).toBe(0); + }, + ); + + it.skipIf(!sandboxAvailable() || confined.skip)( + confined.title('reads neither without them, so the home is denied indeed'), + { tags: ['listener'], timeout: 120_000 }, + async () => { + const scratch = await makeScratch('aflow-check-', base); + const agentScratch = await makeAgentScratch(scratch); + const wt = await prepareWorktree(folder, scratch, 'check'); + try { + const result = await runSandboxed({ + binding: bindingOf(folder), + argv: PROBE, + cwd: wt.workDir, + env: {}, + timeoutMs: 60_000, + scratchDir: agentScratch, + widening: { authPaths: [], allowedDomains: [], ...checkoutWidening(wt, []) }, + idPrefix: 'sb', + ownerRunId: 'probe', + closeStdin: true, + signal: new AbortController().signal, + onDelta: () => undefined, + }); + + expect(result.stdout).not.toContain('the base commit'); + expect(result.stdout).not.toContain(HOISTED); + } finally { + await removeWorktree(wt.path); + } + }, + ); +}); + +describe('a linked worktree connected under a denied home', () => { + // A top level whose object store is the main repository's, outside it. + let linked: string; + + beforeEach(async () => { + linked = join(home, 'worktrees', 'mono'); + await git(top, 'worktree', 'add', '-q', '--detach', linked); + }); + + it('borrows the main repository’s object store', async () => { + expect(await borrowedReads(linked)).toEqual([join(top, '.git', 'objects')]); + }); + + it.skipIf(!sandboxAvailable() || confined.skip)( + confined.title('reads its history inside the run'), + { tags: ['listener'], timeout: 120_000 }, + async () => { + const sha = await git(linked, 'rev-parse', 'HEAD'); + const checked = await runFolderChecks({ + binding: bindingOf(linked), + argv: ['/bin/sh', '-c', 'git log -1 --format=%s'], + timeoutMs: 60_000, + sha, + base: sha, + toolPaths: [], + ownerRunId: 'probe', + signal: new AbortController().signal, + }); + + expect(checked.output).toContain('the base commit'); + expect(checked.result.exitCode).toBe(0); + }, + ); +}); diff --git a/apps/aflow-executor-host/src/__tests__/withdrawalAndLifetime.test.ts b/apps/aflow-executor-host/src/__tests__/withdrawalAndLifetime.test.ts index d32b89f8..37c8980b 100644 --- a/apps/aflow-executor-host/src/__tests__/withdrawalAndLifetime.test.ts +++ b/apps/aflow-executor-host/src/__tests__/withdrawalAndLifetime.test.ts @@ -30,7 +30,6 @@ function makeSession(overrides: Partial = {}): HarnessSession { ownerRunId: 'run-a', logicalExecutionId: `step:${id}`, bindingId: 'hb', - bindingRoot: '/repo', harnessId: 'claude', worktreePath: `/tmp/${id}/work`, scratchDir: `/tmp/${id}`, @@ -75,16 +74,6 @@ describe('a turn in progress is not swept away underneath itself', () => { releaseSession(session); forgetSession(session.id); }); - - it('carries the repository its worktree belongs to', () => { - // Cleanup happens on whichever run notices a session is stale, and that run - // may be in a different binding — removing a worktree against the wrong - // repository leaves a permanent stale entry in that one instead. - const session = makeSession({ bindingRoot: '/repos/theirs' }); - recordSession(session); - expect(ownedSession(session.id, 'run-a')?.bindingRoot).toBe('/repos/theirs'); - forgetSession(session.id); - }); }); describe('withdrawing a binding reaches sessions under it', () => { diff --git a/apps/aflow-executor-host/src/__tests__/workspaceLinks.test.ts b/apps/aflow-executor-host/src/__tests__/workspaceLinks.test.ts index be18318e..fb2afc8e 100644 --- a/apps/aflow-executor-host/src/__tests__/workspaceLinks.test.ts +++ b/apps/aflow-executor-host/src/__tests__/workspaceLinks.test.ts @@ -284,7 +284,7 @@ describe('a workspace package in the isolated checkout', () => { expect(await readlink(link)).toBe(join('..', '..', 'packages', 'a')); expect(await realpath(link)).toBe(join(await realpath(wt.path), 'packages', 'a')); } finally { - await removeWorktree(repo, wt.path); + await removeWorktree(wt.path); } }); @@ -298,7 +298,7 @@ describe('a workspace package in the isolated checkout', () => { join(await realpath(repo), 'node_modules', '.store', 'right-pad'), ); } finally { - await removeWorktree(repo, wt.path); + await removeWorktree(wt.path); } }); }); diff --git a/apps/aflow-executor-host/src/__tests__/worktree.test.ts b/apps/aflow-executor-host/src/__tests__/worktree.test.ts index a5a9ffca..6d87a873 100644 --- a/apps/aflow-executor-host/src/__tests__/worktree.test.ts +++ b/apps/aflow-executor-host/src/__tests__/worktree.test.ts @@ -1,11 +1,22 @@ import { execFile } from 'node:child_process'; -import { mkdir, mkdtemp, readFile, rm, stat, writeFile } from 'node:fs/promises'; +import { + lstat, + mkdir, + mkdtemp, + readFile, + realpath, + rm, + stat, + symlink, + writeFile, +} from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { basename, join } from 'node:path'; import { promisify } from 'node:util'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { HostBindingSchema } from '../bindings.js'; import { removeOrphanedCheckouts } from '../handlers/harnessHandlers.js'; import { allSessions, @@ -14,18 +25,23 @@ import { recordSession, } from '../harnessSessions.js'; import { PUBLICATION_SCRATCH_PREFIX } from '../branchCommit.js'; +import { makeAgentScratch, makeScratch } from '../executorScratch.js'; +import { openOrphanJournal, readJournal, reapOrphans, recordSpawn } from '../orphans.js'; +import { runSandboxed, sandboxAvailable } from '../sandboxedRun.js'; import { - changedRefs, checkApplies, + checkoutGitDir, collectChanges, fetchRemoteBase, isGitRepository, prepareWorktree, removeWorktree, resolveCommit, - snapshotRefs, WorktreeError, } from '../worktree.js'; +import { CONFINEMENT_LISTENERS, requires } from './fixtures/capabilities.js'; + +const confined = requires(...CONFINEMENT_LISTENERS); const run = promisify(execFile); @@ -75,7 +91,7 @@ describe('worktree per run', () => { // The worktree started from the commit, not from the dirty state. expect(wt.baseSha).toBe((await git(repo, 'rev-parse', 'HEAD')).trim()); - await removeWorktree(repo, wt.path); + await removeWorktree(wt.path); expect(await readFile(join(repo, 'app.txt'), 'utf8')).toBe('MY UNSAVED EDIT\n'); }); @@ -85,25 +101,25 @@ describe('worktree per run', () => { await writeFile(join(a.path, 'app.txt'), 'from A\n'); await writeFile(join(b.path, 'app.txt'), 'from B\n'); - const changesA = await collectChanges(a.path); - const changesB = await collectChanges(b.path); + const changesA = await collectChanges(a); + const changesB = await collectChanges(b); expect(changesA.patch).toContain('from A'); expect(changesA.patch).not.toContain('from B'); expect(changesB.patch).toContain('from B'); - await removeWorktree(repo, a.path); - await removeWorktree(repo, b.path); + await removeWorktree(a.path); + await removeWorktree(b.path); }); it('reports added files, which a plain diff would omit entirely', async () => { const wt = await prepareWorktree(repo, scratch, 'work'); await writeFile(join(wt.path, 'brand-new.ts'), 'export const x = 1;\n'); - const changes = await collectChanges(wt.path); + const changes = await collectChanges(wt); expect(changes.filesChanged).toBe(1); expect(changes.patch).toContain('brand-new.ts'); expect(changes.patch).toContain('export const x = 1;'); - await removeWorktree(repo, wt.path); + await removeWorktree(wt.path); }); it('checks out the commit as it is stored, never what a replace ref stands in for it', async () => { @@ -124,8 +140,8 @@ describe('worktree per run', () => { ]) { expect(wt.baseSha).toBe(stored); expect(await readFile(join(wt.path, 'app.txt'), 'utf8')).toBe('stored\n'); - expect((await collectChanges(wt.path)).filesChanged).toBe(0); - await removeWorktree(repo, wt.path); + expect((await collectChanges(wt)).filesChanged).toBe(0); + await removeWorktree(wt.path); } }); @@ -135,12 +151,12 @@ describe('worktree per run', () => { await writeFile(join(wt.path, '.aflow', 'change.patch'), 'a copy of the work\n'); await writeFile(join(wt.path, 'app.txt'), 'edited\n'); - const changes = await collectChanges(wt.path); + const changes = await collectChanges(wt); expect(changes.filesChanged).toBe(1); expect(changes.patch).toContain('app.txt'); expect(changes.patch).not.toContain('.aflow'); expect(changes.patch).not.toContain('a copy of the work'); - await removeWorktree(repo, wt.path); + await removeWorktree(wt.path); }); it('leaves out a tracked file under the scratch directory too', async () => { @@ -151,16 +167,16 @@ describe('worktree per run', () => { const wt = await prepareWorktree(repo, scratch, 'work'); await writeFile(join(wt.path, '.aflow', 'notes.md'), 'rewritten by the agent\n'); - const changes = await collectChanges(wt.path); + const changes = await collectChanges(wt); expect(changes).toEqual({ patch: '', filesChanged: 0, overCeiling: false }); - await removeWorktree(repo, wt.path); + await removeWorktree(wt.path); }); it('reports no change as no change, not as an empty diff of something', async () => { const wt = await prepareWorktree(repo, scratch, 'work'); - const changes = await collectChanges(wt.path); + const changes = await collectChanges(wt); expect(changes).toEqual({ patch: '', filesChanged: 0, overCeiling: false }); - await removeWorktree(repo, wt.path); + await removeWorktree(wt.path); }); it('collects work from a run that was stopped part-way', async () => { @@ -168,9 +184,9 @@ describe('worktree per run', () => { // from the worktree regardless of how the process ended. const wt = await prepareWorktree(repo, scratch, 'work'); await writeFile(join(wt.path, 'app.txt'), 'half-finished\n'); - const changes = await collectChanges(wt.path); + const changes = await collectChanges(wt); expect(changes.filesChanged).toBe(1); - await removeWorktree(repo, wt.path); + await removeWorktree(wt.path); }); it('refuses a folder that is not a repository, naming why', async () => { @@ -187,11 +203,49 @@ describe('worktree per run', () => { await rm(unborn, { recursive: true, force: true }); }); - it('leaves no worktree registered after cleanup', async () => { + it('registers nothing in the folder, and leaves nothing once removed', async () => { + const listed = await git(repo, 'worktree', 'list', '--porcelain'); const wt = await prepareWorktree(repo, scratch, 'work'); - expect(await git(repo, 'worktree', 'list')).toContain(wt.path); - await removeWorktree(repo, wt.path); - expect(await git(repo, 'worktree', 'list')).not.toContain(wt.path); + expect(await git(repo, 'worktree', 'list', '--porcelain')).toBe(listed); + await removeWorktree(wt.path); + expect(await exists(wt.path)).toBe(false); + expect(await git(repo, 'worktree', 'list', '--porcelain')).toBe(listed); + }); + + it('gives a folder connected inside a repository a checkout that works where the folder is', async () => { + // A package of a monorepo: a repository to `rev-parse`, and no top level + // `git clone` would take. + const folder = join(repo, 'packages', 'app'); + await mkdir(folder, { recursive: true }); + await writeFile(join(folder, 'index.ts'), 'export const app = 1;\n'); + await git(repo, 'add', '-A'); + await git(repo, 'commit', '-m', 'a package'); + expect(await isGitRepository(folder)).toBe(true); + + const wt = await prepareWorktree(folder, scratch, 'work'); + try { + expect(wt.workDir).toBe(join(wt.path, 'packages', 'app')); + expect(wt.baseSha).toBe((await git(repo, 'rev-parse', 'HEAD')).trim()); + expect(await readFile(join(wt.workDir, 'index.ts'), 'utf8')).toBe('export const app = 1;\n'); + + // The run's work is the folder's: an edit beside it in the repository and + // the run's own files are not part of the change. + await writeFile(join(wt.workDir, 'index.ts'), 'export const app = 2;\n'); + await writeFile(join(wt.workDir, 'added.ts'), 'export const added = true;\n'); + await mkdir(join(wt.workDir, '.aflow'), { recursive: true }); + await writeFile(join(wt.workDir, '.aflow', 'result.json'), '{}\n'); + await writeFile(join(wt.path, 'app.txt'), 'outside the folder\n'); + const changes = await collectChanges(wt); + expect(changes.filesChanged).toBe(2); + expect(changes.patch).toContain('packages/app/index.ts'); + expect(changes.patch).toContain('packages/app/added.ts'); + expect(changes.patch).not.toContain('app.txt'); + expect(changes.patch).not.toContain('result.json'); + expect(await checkApplies(folder, changes.patch)).toEqual({ state: 'clean' }); + } finally { + await removeWorktree(wt.path); + } + expect(await exists(wt.path)).toBe(false); }); }); @@ -207,7 +261,7 @@ describe('a run started from a named ref', () => { const wt = await prepareWorktree(repo, scratch, 'work', { at }); expect(wt.baseSha).toBe(branchHead); expect(await readFile(join(wt.path, 'app.txt'), 'utf8')).toBe('committed\n'); - await removeWorktree(repo, wt.path); + await removeWorktree(wt.path); }); it('refuses a ref the folder does not have, naming it', async () => { @@ -300,64 +354,75 @@ describe('a run started from a ref on a remote', () => { }); }); -describe('the refs reported for a run', () => { - it('sees nothing when the run only committed in its own checkout', async () => { - const wt = await prepareWorktree(repo, scratch, 'work'); - const before = await snapshotRefs(repo); - await writeFile(join(wt.path, 'app.txt'), 'committed in the checkout\n'); - await git(wt.path, 'commit', '-am', 'detached commit'); - expect(changedRefs(before, await snapshotRefs(repo))).toEqual([]); - await removeWorktree(repo, wt.path); - }); +describe("a run's checkout is a clone whose refs are its own", () => { + const refsOf = async (cwd: string): Promise => + await git(cwd, 'for-each-ref', '--format=%(refname) %(objectname)'); - it('sees a branch deleted through the shared refs', async () => { + beforeEach(async () => { await git(repo, 'branch', 'other'); - const other = (await git(repo, 'rev-parse', 'other')).trim(); - const wt = await prepareWorktree(repo, scratch, 'work'); - const before = await snapshotRefs(repo); - await git(wt.path, 'branch', '-D', 'other'); - - expect(changedRefs(before, await snapshotRefs(repo))).toEqual([ - { ref: 'refs/heads/other', change: 'deleted', from: other }, - ]); - await removeWorktree(repo, wt.path); + await git(repo, 'tag', 'v1'); + await git(repo, 'update-ref', 'refs/remotes/origin/main', 'HEAD'); }); - it('sees a ref moved and a ref created from inside the checkout', async () => { - const wt = await prepareWorktree(repo, scratch, 'work'); - const before = await snapshotRefs(repo); - const main = before.get('refs/heads/main'); - await writeFile(join(wt.path, 'app.txt'), 'rewritten\n'); - await git(wt.path, 'commit', '-am', 'rewrite'); - const rewritten = (await git(wt.path, 'rev-parse', 'HEAD')).trim(); - await git(wt.path, 'update-ref', 'refs/heads/main', rewritten); - await git(wt.path, 'update-ref', 'refs/tags/planted', rewritten); - - expect(changedRefs(before, await snapshotRefs(repo))).toEqual([ - { ref: 'refs/heads/main', change: 'moved', from: main, to: rewritten }, - { ref: 'refs/tags/planted', change: 'created', to: rewritten }, - ]); - await removeWorktree(repo, wt.path); - }); - - it('watches local branches and tags, and not what a fetch or a stash moves', async () => { - await git(repo, 'branch', 'other'); - const wt = await prepareWorktree(repo, scratch, 'work'); - const head = (await git(repo, 'rev-parse', 'HEAD')).trim(); - const before = await snapshotRefs(repo); + it('reads every ref as the folder names it, borrows its objects, and names it as no remote', async () => { + const wt = await prepareWorktree(repo, scratch, 'work', { dependencies: 'none' }); + expect(await refsOf(wt.path)).toBe(await refsOf(repo)); + // Its git directory sits beside it, and the checkout holds only a gitfile. + expect(wt.gitDir).toBe(join(await realpath(scratch), 'git-work')); + expect(await readFile(join(wt.path, '.git'), 'utf8')).toBe(`gitdir: ${wt.gitDir}\n`); + expect((await git(wt.path, 'rev-parse', '--absolute-git-dir')).trim()).toBe( + await realpath(wt.gitDir), + ); + expect(await readFile(join(wt.gitDir, 'objects', 'info', 'alternates'), 'utf8')).toContain( + join(basename(repo), '.git', 'objects'), + ); + expect((await git(wt.path, 'remote')).trim()).toBe(''); + expect((await git(wt.path, 'rev-parse', 'HEAD')).trim()).toBe(wt.baseSha); + expect(await readFile(join(wt.gitDir, 'HEAD'), 'utf8')).toBe(`${wt.baseSha}\n`); + await removeWorktree(wt.path); + await expect(stat(wt.gitDir)).rejects.toThrow(); + }); - await git(repo, 'update-ref', 'refs/remotes/origin/x', head); - await writeFile(join(repo, 'app.txt'), 'work in progress\n'); - await git(repo, 'stash'); - expect(changedRefs(before, await snapshotRefs(repo))).toEqual([]); + it("moves only the clone's refs, whatever git the run uses, with no hook anywhere", async () => { + const wt = await prepareWorktree(repo, scratch, 'work', { dependencies: 'none' }); + const before = await refsOf(repo); + await writeFile(join(wt.path, 'app.txt'), 'committed in the clone\n'); + await git( + wt.path, + '-c', + 'user.name=Agent', + '-c', + 'user.email=a@example.com', + 'commit', + '-qam', + 'w', + ); + const made = (await git(wt.path, 'rev-parse', 'HEAD')).trim(); await git(wt.path, 'branch', '-D', 'other'); - await git(wt.path, 'tag', 'planted'); - expect(changedRefs(before, await snapshotRefs(repo)).map((c) => [c.ref, c.change])).toEqual([ - ['refs/heads/other', 'deleted'], - ['refs/tags/planted', 'created'], - ]); - await removeWorktree(repo, wt.path); + await git(wt.path, '-c', 'core.hooksPath=/dev/null', 'update-ref', 'refs/heads/main', made); + await git(wt.path, '-c', 'core.hooksPath=/dev/null', 'tag', '-f', 'v1', made); + await git(wt.path, 'update-ref', '-d', 'refs/remotes/origin/main'); + await git(wt.path, 'push', '-q', '.', 'HEAD:refs/heads/pushed'); + + expect(await refsOf(repo)).toBe(before); + expect(await git(repo, 'cat-file', '-t', made).catch(() => 'absent')).toBe('absent'); + const clone = await refsOf(wt.path); + expect(clone).not.toContain('refs/heads/other '); + expect(clone).toContain(`refs/heads/main ${made}`); + expect(clone).toContain(`refs/heads/pushed ${made}`); + expect(await exists(join(repo, '.git', 'hooks', 'reference-transaction'))).toBe(false); + expect(await exists(join(scratch, 'git-hooks'))).toBe(false); + await removeWorktree(wt.path); + }); + + it('reads a commit made in the folder after the clone, through the objects it borrows', async () => { + const wt = await prepareWorktree(repo, scratch, 'work', { dependencies: 'none' }); + await writeFile(join(repo, 'app.txt'), 'later\n'); + await git(repo, 'commit', '-qam', 'later'); + const later = (await git(repo, 'rev-parse', 'HEAD')).trim(); + expect(await git(wt.path, 'show', `${later}:app.txt`)).toBe('later\n'); + await removeWorktree(wt.path); }); }); @@ -374,8 +439,8 @@ describe('whether a diff made from a named base applies', () => { const at = await resolveCommit(repo, 'feat/fix'); const wt = await prepareWorktree(repo, scratch, 'work', { at }); await writeFile(join(wt.path, 'app.txt'), 'fixed on the branch\n'); - const { patch } = await collectChanges(wt.path); - await removeWorktree(repo, wt.path); + const { patch } = await collectChanges(wt); + await removeWorktree(wt.path); expect((await checkApplies(repo, patch)).state).toBe('conflict'); expect((await checkApplies(repo, patch, at)).state).toBe('clean'); @@ -400,23 +465,18 @@ describe('checkouts a restart left behind', () => { }); it('removes what no session owns under the temp root, and nothing of the operator', async () => { - const orphanScratch = await mkdtemp(join(tempRoot, HARNESS_SCRATCH_PREFIX)); + const orphanScratch = await makeScratch(HARNESS_SCRATCH_PREFIX, tempRoot); await prepareWorktree(repo, orphanScratch, 'work'); - const orphanPublication = await mkdtemp(join(tempRoot, PUBLICATION_SCRATCH_PREFIX)); + const orphanPublication = await makeScratch(PUBLICATION_SCRATCH_PREFIX, tempRoot); await prepareWorktree(repo, orphanPublication, 'commit', { dependencies: 'none' }); - // A checkout whose directory is already gone is still registered. - const vanishedScratch = await mkdtemp(join(tempRoot, HARNESS_SCRATCH_PREFIX)); - await prepareWorktree(repo, vanishedScratch, 'work'); - await rm(vanishedScratch, { recursive: true, force: true }); - const stray = await mkdtemp(join(tempRoot, HARNESS_SCRATCH_PREFIX)); - const liveScratch = await mkdtemp(join(tempRoot, HARNESS_SCRATCH_PREFIX)); + const stray = await makeScratch(HARNESS_SCRATCH_PREFIX, tempRoot); + const liveScratch = await makeScratch(HARNESS_SCRATCH_PREFIX, tempRoot); const live = await prepareWorktree(repo, liveScratch, 'work'); recordSession({ id: 'hs_live', ownerRunId: 'run-live', logicalExecutionId: 'step:hs_live', bindingId: 'hb', - bindingRoot: repo, harnessId: 'edits', worktreePath: live.path, scratchDir: liveScratch, @@ -427,80 +487,152 @@ describe('checkouts a restart left behind', () => { lastUsedAt: Date.now(), busy: false, }); - // The operator's own checkouts in the same temp root: one on a branch with - // work in progress, in a directory named the way this executor names its - // scratch, and one detached, in a directory it never makes. + // The operator's own checkouts in the same temp root, in directories named + // the way this executor names its scratch but never marked by it: one added + // to the folder on a branch with work in progress, and a clone of their own + // on a branch. const operatorsScratch = await mkdtemp(join(tempRoot, HARNESS_SCRATCH_PREFIX)); const onBranch = join(operatorsScratch, 'mine'); await git(repo, 'worktree', 'add', '-b', 'operator/review', onBranch); await writeFile(join(onBranch, 'app.txt'), 'uncommitted review notes\n'); + const operatorsClone = await mkdtemp(join(tempRoot, HARNESS_SCRATCH_PREFIX)); + await git(operatorsClone, 'clone', '-q', repo, 'mine'); + // And one of this lane's shape, in a directory it never makes. const operatorsDetached = await mkdtemp(join(tempRoot, 'aflow-review-')); const detached = await prepareWorktree(repo, operatorsDetached, 'mine'); const unrelated = await mkdtemp(join(tempRoot, 'not-ours-')); + const refs = await git(repo, 'for-each-ref'); + const listed = await git(repo, 'worktree', 'list', '--porcelain'); - const outcome = await removeOrphanedCheckouts([repo, repo, scratch], tempRoot); + const outcome = await removeOrphanedCheckouts(tempRoot); - expect(outcome.removed).toEqual(new Map([[repo, 3]])); - expect(outcome.scratchDirs).toBe(3); - const listed = await git(repo, 'worktree', 'list', '--porcelain'); - // By directory name: git records a checkout under its resolved path. - expect(listed).not.toContain(basename(orphanScratch)); - expect(listed).not.toContain(basename(orphanPublication)); - expect(listed).not.toContain(basename(vanishedScratch)); - expect(listed).toContain(basename(liveScratch)); - expect(listed).toContain(basename(operatorsScratch)); - expect(listed).toContain('branch refs/heads/operator/review'); - expect(listed).toContain(basename(operatorsDetached)); + expect(outcome).toEqual({ checkouts: 2, scratchDirs: 3 }); expect(await exists(orphanScratch)).toBe(false); expect(await exists(orphanPublication)).toBe(false); expect(await exists(stray)).toBe(false); expect(await exists(live.path)).toBe(true); expect(await readFile(join(onBranch, 'app.txt'), 'utf8')).toBe('uncommitted review notes\n'); + expect(await exists(join(operatorsClone, 'mine', 'app.txt'))).toBe(true); expect(await exists(detached.path)).toBe(true); expect(await exists(unrelated)).toBe(true); + // The folder was never a party to any of it. + expect(await git(repo, 'for-each-ref')).toBe(refs); + expect(await git(repo, 'worktree', 'list', '--porcelain')).toBe(listed); - await removeWorktree(repo, live.path); - await removeWorktree(repo, onBranch); - await removeWorktree(repo, detached.path); + await removeWorktree(live.path); + await git(repo, 'worktree', 'remove', '--force', onBranch); + await removeWorktree(detached.path); }); - it('takes nothing when the connected folders could not be read', async () => { - const orphanScratch = await mkdtemp(join(tempRoot, HARNESS_SCRATCH_PREFIX)); - const orphan = await prepareWorktree(repo, orphanScratch, 'work'); + it('removes its own scratch whatever shape the checkouts in it were left in', async () => { + const scratchDir = await makeScratch(HARNESS_SCRATCH_PREFIX, tempRoot); + // A run killed between the gitfile and the detached checkout: the mirror's + // HEAD still names a branch, and the working tree holds only the gitfile. + const halfMade = join(scratchDir, 'work'); + const halfMadeGitDir = join(scratchDir, 'git-work'); + await git(repo, 'clone', '-q', '--mirror', '--shared', '--', repo, halfMadeGitDir); + await mkdir(halfMade); + await writeFile(join(halfMade, '.git'), `gitdir: ${halfMadeGitDir}\n`); + expect(await readFile(join(halfMadeGitDir, 'HEAD'), 'utf8')).toMatch(/^ref: /); + const refs = await git(repo, 'for-each-ref'); + const listed = await git(repo, 'worktree', 'list', '--porcelain'); - const outcome = await removeOrphanedCheckouts(undefined, tempRoot); + expect(await removeOrphanedCheckouts(tempRoot)).toEqual({ checkouts: 1, scratchDirs: 1 }); - expect(outcome.removed.size).toBe(0); - expect(outcome.scratchDirs).toBe(0); - expect(await exists(orphan.path)).toBe(true); - expect(await git(repo, 'worktree', 'list', '--porcelain')).toContain(basename(orphanScratch)); + expect(await exists(scratchDir)).toBe(false); + expect(await git(repo, 'for-each-ref')).toBe(refs); + expect(await git(repo, 'worktree', 'list', '--porcelain')).toBe(listed); + }); - await removeWorktree(repo, orphan.path); + it('refuses a link planted in its scratch, and leaves what the link leads to', async () => { + // The operator's project, with a workspace installation: a link to a + // downloaded package and one to a scoped package. + const project = await mkdtemp(join(tempRoot, 'project-')); + await mkdir(join(project, '.git')); + await mkdir(join(project, 'vendor', 'left-pad'), { recursive: true }); + await mkdir(join(project, 'node_modules', '@acme'), { recursive: true }); + await symlink(join(project, 'vendor', 'left-pad'), join(project, 'node_modules', 'left-pad')); + await symlink(join(project, 'vendor'), join(project, 'node_modules', '@acme', 'vendor')); + // What an agent writing the scratch can plant: a checkout that is a link to + // it, and a git directory named for one that is a link too. + const scratchDir = await makeScratch(HARNESS_SCRATCH_PREFIX, tempRoot); + await symlink(project, join(scratchDir, 'work')); + await symlink(project, join(scratchDir, 'git-other')); + + await removeWorktree(join(scratchDir, 'work')); + expect(await removeOrphanedCheckouts(tempRoot)).toEqual({ checkouts: 0, scratchDirs: 1 }); + + expect(await exists(scratchDir)).toBe(false); + expect((await lstat(join(project, 'node_modules', 'left-pad'))).isSymbolicLink()).toBe(true); + expect((await lstat(join(project, 'node_modules', '@acme', 'vendor'))).isSymbolicLink()).toBe( + true, + ); + expect(await exists(join(project, '.git'))).toBe(true); + expect(await exists(join(project, 'vendor', 'left-pad'))).toBe(true); }); - it('leaves scratch whose checkout a folder it could not read still registers', async () => { - const elsewhere = await mkdtemp(join(tmpdir(), 'wt-unbound-')); - await git(elsewhere, 'init', '--initial-branch=main'); - await git(elsewhere, 'config', 'user.email', 'test@example.com'); - await git(elsewhere, 'config', 'user.name', 'Test'); - await git(elsewhere, 'commit', '--allow-empty', '-m', 'initial'); + it("is what a crashed run's journal entry removes too, leaving nothing in the folder", async () => { + const journal = openOrphanJournal(join(tempRoot, 'host')); const orphanScratch = await mkdtemp(join(tempRoot, HARNESS_SCRATCH_PREFIX)); - const orphan = await prepareWorktree(elsewhere, orphanScratch, 'work', { - dependencies: 'none', - }); - - const outcome = await removeOrphanedCheckouts([repo], tempRoot); + await prepareWorktree(repo, orphanScratch, 'work'); + const listed = await git(repo, 'worktree', 'list', '--porcelain'); + recordSpawn(999_999, orphanScratch); - expect(outcome.scratchDirs).toBe(0); - expect(await exists(orphan.path)).toBe(true); + reapOrphans(journal, () => undefined); - await removeWorktree(elsewhere, orphan.path); - await rm(elsewhere, { recursive: true, force: true }); + expect(await exists(orphanScratch)).toBe(false); + expect(await git(repo, 'worktree', 'list', '--porcelain')).toBe(listed); + expect((await git(repo, 'worktree', 'list', '--porcelain')).includes('prunable')).toBe(false); }); + it.skipIf(!sandboxAvailable() || confined.skip)( + confined.title( + "journals a run handed `agent/` with the scratch holding its clone, so a crash's clone goes with it", + ), + { tags: ['listener'], timeout: 120_000 }, + async () => { + const journal = openOrphanJournal(join(tempRoot, 'host')); + const runScratch = await makeScratch(HARNESS_SCRATCH_PREFIX, tempRoot); + const wt = await prepareWorktree(repo, runScratch, 'work'); + const agentScratch = await makeAgentScratch(runScratch); + await runSandboxed({ + binding: HostBindingSchema.parse({ + id: 'hb', + root: repo, + mode: 'readwrite', + allowsExecution: true, + }), + argv: ['/bin/sh', '-c', 'sleep 60'], + cwd: wt.path, + env: {}, + timeoutMs: 60_000, + scratchDir: agentScratch, + journalScratchDir: runScratch, + widening: { + authPaths: [], + allowedDomains: [], + writableRoot: wt.path, + withholdBindingWrite: true, + }, + idPrefix: 'jr', + ownerRunId: 'crashed', + detach: true, + signal: new AbortController().signal, + onDelta: () => undefined, + }); + + // The executor dies here: what the next boot reads is the journal. + expect(readJournal(journal).map((record) => record.scratchDir)).toEqual([runScratch]); + expect(reapOrphans(journal)).toBe(1); + + expect(await exists(runScratch)).toBe(false); + expect(await exists(wt.path)).toBe(false); + expect(await exists(checkoutGitDir(wt.path))).toBe(false); + expect(await git(repo, 'status', '--porcelain')).toBe(''); + }, + ); + it('reports nothing when nothing was left behind', async () => { - const outcome = await removeOrphanedCheckouts([repo], tempRoot); - expect(outcome.removed.size).toBe(0); - expect(outcome.scratchDirs).toBe(0); + expect(await removeOrphanedCheckouts(tempRoot)).toEqual({ checkouts: 0, scratchDirs: 0 }); }); }); diff --git a/apps/aflow-executor-host/src/__tests__/worktreeDependencies.test.ts b/apps/aflow-executor-host/src/__tests__/worktreeDependencies.test.ts index 1fa58ee3..119d8b61 100644 --- a/apps/aflow-executor-host/src/__tests__/worktreeDependencies.test.ts +++ b/apps/aflow-executor-host/src/__tests__/worktreeDependencies.test.ts @@ -18,6 +18,7 @@ import { symlink, writeFile, } from 'node:fs/promises'; +import { createRequire } from 'node:module'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { promisify } from 'node:util'; @@ -92,7 +93,7 @@ describe('dependencies in the isolated checkout', () => { await readFile(join(wt.path, 'packages', 'a', 'node_modules', 'marker.txt'), 'utf8'), ).toBe('package dep\n'); - await removeWorktree(repo, wt.path); + await removeWorktree(wt.path); }); it('resolves a workspace package to the checkout and a real dependency to the folder', async () => { @@ -107,7 +108,94 @@ describe('dependencies in the isolated checkout', () => { join(repoReal, 'node_modules', 'left-pad'), ); - await removeWorktree(repo, wt.path); + await removeWorktree(wt.path); + }); + + it('carries the top level installation too, for a folder connected as a package', async () => { + // Resolution from inside the package walks up to the repository's top + // level, where a workspace hoists what the package needs. + const folder = join(repo, 'packages', 'a'); + const wt = await prepareWorktree(folder, scratch, 'work'); + const worktreeReal = await realpath(wt.path); + const repoReal = await realpath(repo); + + for (const installation of [join(wt.path, 'node_modules'), join(wt.workDir, 'node_modules')]) { + const mirrored = await lstat(installation); + expect(mirrored.isDirectory()).toBe(true); + expect(mirrored.isSymbolicLink()).toBe(false); + } + expect(await readFile(join(wt.workDir, 'node_modules', 'marker.txt'), 'utf8')).toBe( + 'package dep\n', + ); + expect(await realpath(join(wt.path, 'node_modules', 'left-pad'))).toBe( + join(repoReal, 'node_modules', 'left-pad'), + ); + // A workspace package resolves into the clone, the folder's own included, + // not into what the operator has open. + expect(await realpath(join(wt.path, 'node_modules', '@acme', 'a'))).toBe( + join(worktreeReal, 'packages', 'a'), + ); + expect(await collectChanges(wt)).toEqual({ patch: '', filesChanged: 0, overCeiling: false }); + + await removeWorktree(wt.path); + expect((await lstat(join(repo, 'node_modules', '@acme', 'a'))).isSymbolicLink()).toBe(true); + expect(await readdir(join(folder, 'node_modules'))).toEqual(['marker.txt']); + }); + + it('resolves a sibling package’s own dependency inside the clone, for a folder connected as a package', async () => { + // What the sibling did not hoist is in its own installation, which the + // workspace link from the folder leads into. + const sibling = join(repo, 'packages', 'b'); + await mkdir(sibling, { recursive: true }); + await writeFile(join(sibling, 'package.json'), '{"name":"@acme/b","main":"index.js"}\n'); + await writeFile(join(sibling, 'index.js'), 'module.exports = require("only-b");\n'); + await git(repo, 'add', '-A'); + await git(repo, 'commit', '-m', 'sibling'); + await mkdir(join(sibling, 'node_modules', 'only-b'), { recursive: true }); + await writeFile(join(sibling, 'node_modules', 'only-b', 'index.js'), 'module.exports = 1;\n'); + await symlink(join('..', '..', 'packages', 'b'), join(repo, 'node_modules', '@acme', 'b')); + + const wt = await prepareWorktree(join(repo, 'packages', 'a'), scratch, 'work'); + const worktreeReal = await realpath(wt.path); + const repoReal = await realpath(repo); + + const fromFolder = createRequire(join(wt.workDir, 'index.js')); + const siblingEntry = fromFolder.resolve('@acme/b'); + expect(siblingEntry).toBe(join(worktreeReal, 'packages', 'b', 'index.js')); + expect(await realpath(createRequire(siblingEntry).resolve('only-b'))).toBe( + join(repoReal, 'packages', 'b', 'node_modules', 'only-b', 'index.js'), + ); + expect(await collectChanges(wt)).toEqual({ patch: '', filesChanged: 0, overCeiling: false }); + + await removeWorktree(wt.path); + expect(await readdir(join(sibling, 'node_modules'))).toEqual(['only-b']); + }); + + it('resolves the folder’s own dependency inside the clone, for a folder three levels down', async () => { + // Below the depth the scan from the top level reaches: the walk up from + // the folder is what finds its installation. + const folder = join(repo, 'packages', 'group', 'pkg'); + await mkdir(folder, { recursive: true }); + await writeFile(join(folder, 'index.js'), 'module.exports = require("only-pkg");\n'); + await git(repo, 'add', '-A'); + await git(repo, 'commit', '-m', 'nested package'); + await mkdir(join(folder, 'node_modules', 'only-pkg'), { recursive: true }); + await writeFile(join(folder, 'node_modules', 'only-pkg', 'index.js'), 'module.exports = 1;\n'); + + const wt = await prepareWorktree(folder, scratch, 'work'); + const repoReal = await realpath(repo); + + const fromFolder = createRequire(join(wt.workDir, 'index.js')); + expect(await realpath(fromFolder.resolve('only-pkg'))).toBe( + join(repoReal, 'packages', 'group', 'pkg', 'node_modules', 'only-pkg', 'index.js'), + ); + expect(await realpath(fromFolder.resolve('left-pad'))).toBe( + join(repoReal, 'node_modules', 'left-pad', 'index.js'), + ); + expect(await collectChanges(wt)).toEqual({ patch: '', filesChanged: 0, overCeiling: false }); + + await removeWorktree(wt.path); + expect(await readdir(join(folder, 'node_modules'))).toEqual(['only-pkg']); }); it('shows a workspace package as committed, not as the operator is editing it', async () => { @@ -120,7 +208,7 @@ describe('dependencies in the isolated checkout', () => { 'package\n', ); - await removeWorktree(repo, wt.path); + await removeWorktree(wt.path); }); it('carries an installation the folder keeps somewhere else', async () => { @@ -143,12 +231,12 @@ describe('dependencies in the isolated checkout', () => { expect(await realpath(join(wt.path, 'node_modules', '@acme', 'a'))).toBe( join(await realpath(wt.path), 'packages', 'a'), ); - expect(await collectChanges(wt.path)).toEqual({ + expect(await collectChanges(wt)).toEqual({ patch: '', filesChanged: 0, overCeiling: false, }); - await removeWorktree(repo, wt.path); + await removeWorktree(wt.path); expect(await readdir(elsewhere)).toEqual(expect.arrayContaining(['@acme', 'left-pad'])); expect(await readFile(join(elsewhere, 'left-pad', 'index.js'), 'utf8')).toBe('stored dep\n'); } finally { @@ -158,26 +246,26 @@ describe('dependencies in the isolated checkout', () => { it('reports nothing changed, because a link is this lane doing rather than the harness', async () => { const wt = await prepareWorktree(repo, scratch, 'work'); - const changes = await collectChanges(wt.path); + const changes = await collectChanges(wt); expect(changes).toEqual({ patch: '', filesChanged: 0, overCeiling: false }); // Not merely absent from the diff: absent from what git was asked to stage. expect(await git(wt.path, 'diff', '--cached', '--name-only')).toBe(''); - await removeWorktree(repo, wt.path); + await removeWorktree(wt.path); }); it('still reports the work the harness did, beside the links', async () => { const wt = await prepareWorktree(repo, scratch, 'work'); await writeFile(join(wt.path, 'app.txt'), 'harness rewrote this\n'); - const changes = await collectChanges(wt.path); + const changes = await collectChanges(wt); expect(changes.filesChanged).toBe(1); expect(changes.patch).toContain('harness rewrote this'); expect(changes.patch).not.toContain('node_modules'); - await removeWorktree(repo, wt.path); + await removeWorktree(wt.path); }); it('takes the mirror away and leaves what the operator installed and wrote', async () => { const wt = await prepareWorktree(repo, scratch, 'work'); - await removeWorktree(repo, wt.path); + await removeWorktree(wt.path); expect(await readFile(join(repo, 'node_modules', 'left-pad', 'index.js'), 'utf8')).toBe( 'root dep\n', @@ -195,12 +283,12 @@ describe('dependencies in the isolated checkout', () => { const wt = await prepareWorktree(repo, scratch, 'work'); await expect(lstat(join(wt.path, 'node_modules'))).rejects.toThrow(); - expect(await collectChanges(wt.path)).toEqual({ + expect(await collectChanges(wt)).toEqual({ patch: '', filesChanged: 0, overCeiling: false, }); - await removeWorktree(repo, wt.path); + await removeWorktree(wt.path); }); it.skipIf(!sandboxAvailable() || confined.skip)( @@ -249,7 +337,7 @@ describe('dependencies in the isolated checkout', () => { 'root dep\n', ); - await removeWorktree(repo, wt.path); + await removeWorktree(wt.path); }, ); }); diff --git a/apps/aflow-executor-host/src/baseMerge.ts b/apps/aflow-executor-host/src/baseMerge.ts index d45a7b9a..d3a150f1 100644 --- a/apps/aflow-executor-host/src/baseMerge.ts +++ b/apps/aflow-executor-host/src/baseMerge.ts @@ -21,6 +21,7 @@ import { isPlainRefName, remoteNames, WorktreeError, + type Checkout, } from './worktree.js'; /** @@ -176,7 +177,7 @@ export async function mergeIdentityArgs(root: string): Promise { * Every unmerged path with the stages git left for it: 1 the common ancestor's * version, 2 the checkout's, 3 the merged commit's. */ -async function unmergedStages(checkout: string): Promise>> { +async function unmergedStages(checkout: Checkout): Promise>> { const listing = await git(checkout, ['ls-files', '-u', '-z'], APPLY_OUTPUT_CAP_BYTES); const stages = new Map>(); for (const record of listing.split('\0')) { @@ -212,7 +213,10 @@ function conflictsOf(stages: ReadonlyMap>): MergeCon * its text to resolve. A path one side deleted is not asked: whatever its * bytes, resolving it is keeping or deleting it. */ -async function binaryConflicts(checkout: string, conflicts: readonly string[]): Promise { +async function binaryConflicts( + checkout: Checkout, + conflicts: readonly string[], +): Promise { const binary = new Set(); // Between the two sides, since the conflicted working tree's own diff says nothing. const numstat = await git( @@ -259,7 +263,7 @@ async function binaryConflicts(checkout: string, conflicts: readonly string[]): * hold is refused, since no turn could resolve it. */ export async function mergeIntoCheckout( - checkout: string, + checkout: Checkout, from: string, identity: readonly string[], ): Promise { @@ -333,7 +337,7 @@ export async function mergeIntoCheckout( * in one is refused before it is committed, by `mergeIntoCheckout`. */ async function filesWithConflictMarkers( - checkout: string, + checkout: Checkout, paths: readonly string[], ): Promise { if (paths.length === 0) return []; @@ -379,7 +383,7 @@ async function filesWithConflictMarkers( * so accepts the deletion, and one that brings it back keeps the file. */ export async function undecidedConflicts( - checkout: string, + checkout: Checkout, conflicts: readonly MergeConflict[], ): Promise { const markable = conflicts.filter(({ kind }) => kind === 'content' || kind === 'add-add'); diff --git a/apps/aflow-executor-host/src/branchCommit.ts b/apps/aflow-executor-host/src/branchCommit.ts index d0a86ea3..2d22d1df 100644 --- a/apps/aflow-executor-host/src/branchCommit.ts +++ b/apps/aflow-executor-host/src/branchCommit.ts @@ -1,12 +1,12 @@ /** - * A diff landed as a commit on a branch, in a checkout of the executor's own. + * A diff landed as a commit on a branch, made in a clone of the executor's own + * and fetched from it into the folder. * * The publication half of the host lane: a new branch at the commission's * base, or one commit appended to the branch the diff was made on — on its * head, or on the merge of its base into it that the commission made. */ -import { mkdtemp, rm, writeFile } from 'node:fs/promises'; -import { tmpdir } from 'node:os'; +import { rm, writeFile } from 'node:fs/promises'; import { join } from 'node:path'; import { @@ -15,6 +15,8 @@ import { undecidedConflicts, type BaseMerge, } from './baseMerge.js'; +import { makeScratch } from './executorScratch.js'; +import { noPublicationGitMessage, publicationGitReadiness } from './gitReadiness.js'; import { APPLY_OUTPUT_CAP_BYTES, applyPatch, @@ -27,6 +29,7 @@ import { resolveCommit, WorktreeError, type ApplyOutcome, + type Checkout, type PreparedWorktree, } from './worktree.js'; @@ -196,7 +199,7 @@ async function mergeSource( return await reachMergeSource(root, mergeFrom, target.at); } -async function recordedMessage(root: string, sha: string): Promise { +async function recordedMessage(root: string | Checkout, sha: string): Promise { const message = await git(root, ['show', '-s', '--format=%B', sha], APPLY_OUTPUT_CAP_BYTES); return message.replace(/\n+$/, ''); } @@ -208,8 +211,12 @@ export function messageBody(message: string): string | undefined { return body === '' ? undefined : body; } -async function commitStaged(checkout: string, messagePath: string, amend: boolean): Promise { - const identity = await commitIdentityArgs(checkout); +async function commitStaged( + checkout: Checkout, + identity: readonly string[], + messagePath: string, + amend: boolean, +): Promise { try { await git( checkout, @@ -242,14 +249,16 @@ async function commitStaged(checkout: string, messagePath: string, amend: boolea * the merge of that commit into its head, made as the commission made it, with * the patch folded in, or the merge alone for an empty patch — so the diff * lands on the tree it was made against. The - * apply happens in a checkout made for this call alone, so the operator's + * apply happens in a private clone made for this call alone, so the operator's * working tree, index and current branch are never a party to it — they are a - * second writer this lane does not get to interrupt. What remains afterwards is - * one ref, created or advanced by one commit, which is the thing a publication - * can push and the thing an operator can reset if they disagree. + * second writer this lane does not get to interrupt. The commit is then fetched + * from the clone into the folder and the branch set to it: the one write to the + * folder's refs the loop makes, made by the executor's git. What remains + * afterwards is one ref, created or advanced by one commit, which is the thing a + * publication can push and the thing an operator can reset if they disagree. * - * The identity is the operator's own, resolved from the repository's config and - * then from their global one. Inventing an author for a commit that will carry + * The identity is the operator's own, resolved from the folder's config — the + * clone's holds none — and then from their global one. Inventing an author for a commit that will carry * the operator's name is not this lane's to do, so a folder where neither names * one is refused rather than signed. */ @@ -266,10 +275,16 @@ export async function commitPatchOnBranch( } = {}, ): Promise { const { pushBaseSha } = options; + // `fetchFromClone` below is what needs the floor; asked first, so a git + // without the flag is refused before a clone and a commit are made for nothing. + const gitReadiness = await publicationGitReadiness(); + if (!gitReadiness.ready) { + throw new WorktreeError(noPublicationGitMessage(gitReadiness.missing), 'git_failed'); + } const target = await commitTarget(root, branch, options.baseSha); const mergeFrom = await mergeSource(root, branch, target, options.mergeFrom); - const scratch = await mkdtemp(join(tmpdir(), PUBLICATION_SCRATCH_PREFIX)); + const scratch = await makeScratch(PUBLICATION_SCRATCH_PREFIX); let worktree: PreparedWorktree | undefined; try { worktree = await prepareWorktree(root, scratch, 'commit', { @@ -278,22 +293,18 @@ export async function commitPatchOnBranch( }); let merge: BaseMerge | undefined; if (mergeFrom !== undefined) { - merge = await mergeIntoCheckout( - worktree.path, - mergeFrom, - await commitIdentityArgs(worktree.path), - ); + merge = await mergeIntoCheckout(worktree, mergeFrom, await commitIdentityArgs(root)); } const apply: ApplyOutcome = merge !== undefined && patch.trim() === '' ? { state: 'applied', conflicts: [] } - : await applyPatch(worktree.path, patch, mode); + : await applyPatch(worktree, patch, mode); if (apply.state === 'conflict') return { apply: merge === undefined ? apply : filesNamedIn(apply) }; - await git(worktree.path, ['add', '-A', '--', '.'], APPLY_OUTPUT_CAP_BYTES); - const staged = (await git(worktree.path, ['diff', '--cached', '--name-only', '-z'])) + await git(worktree, ['add', '-A', '--', '.'], APPLY_OUTPUT_CAP_BYTES); + const staged = (await git(worktree, ['diff', '--cached', '--name-only', '-z'])) .split('\0') .filter((path) => path !== ''); // With a merge the merge commit is itself the change, so a patch that adds @@ -306,7 +317,7 @@ export async function commitPatchOnBranch( ); } if (merge !== undefined) { - const undecided = await undecidedConflicts(worktree.path, merge.conflicts); + const undecided = await undecidedConflicts(worktree, merge.conflicts); if (undecided.length > 0) { const named = undecided.map(({ path, kind }) => `\`${path}\` (${kind} conflict)`); throw new WorktreeError( @@ -321,18 +332,19 @@ export async function commitPatchOnBranch( const messagePath = join(scratch, 'commit-message'); await writeFile(messagePath, message, 'utf8'); - await commitStaged(worktree.path, messagePath, merge !== undefined); + await commitStaged(worktree, await commitIdentityArgs(root), messagePath, merge !== undefined); - const sha = (await git(worktree.path, ['rev-parse', 'HEAD'])).trim(); + const sha = (await git(worktree, ['rev-parse', 'HEAD'])).trim(); // Read back rather than echoed: git's cleanup trims what it was handed, and // the approval shows the commit as it will be pushed. - const recorded = await recordedMessage(worktree.path, sha); + const recorded = await recordedMessage(worktree, sha); const body = messageBody(recorded); + await fetchFromClone(root, worktree, sha); try { // Compare-and-swap on the old head: a branch that moved between the check // above and this line is refused here rather than overwritten. await git( - worktree.path, + root, target.appended ? ['update-ref', '--end-of-options', `refs/heads/${branch}`, sha, worktree.baseSha] : ['branch', '--end-of-options', branch, sha], @@ -361,11 +373,38 @@ export async function commitPatchOnBranch( }, }; } finally { - if (worktree !== undefined) await removeWorktree(root, worktree.path); + if (worktree !== undefined) await removeWorktree(worktree.path); await rm(scratch, { recursive: true, force: true }); } } +/** + * Bring the commit made in the clone into the folder's object store, and only + * that: no destination ref and no FETCH_HEAD, so the branch is then set by a + * compare-and-swap on its old head rather than by fetch's fast-forward rule, + * and the FETCH_HEAD another fetch is about to read is never overwritten. + */ +async function fetchFromClone(root: string, clone: Checkout, sha: string): Promise { + try { + await git(root, [ + 'fetch', + '--quiet', + '--no-tags', + '--no-write-fetch-head', + '--end-of-options', + clone.gitDir, + sha, + ]); + } catch (error) { + throw new WorktreeError( + `The commit was made but could not be brought into the folder: ${ + error instanceof Error ? (error.message.split('\n')[0] ?? 'git failed') : 'git failed' + }`, + 'git_failed', + ); + } +} + /** * An existing branch, published as it stands: its head reported as the * commit a publication checks, scans, reviews and pushes, with nothing made diff --git a/apps/aflow-executor-host/src/browser/harnessBrowser.ts b/apps/aflow-executor-host/src/browser/harnessBrowser.ts index cc43ee86..5f6f0dd5 100644 --- a/apps/aflow-executor-host/src/browser/harnessBrowser.ts +++ b/apps/aflow-executor-host/src/browser/harnessBrowser.ts @@ -23,7 +23,7 @@ */ import { execFile } from 'node:child_process'; import { constants, openSync } from 'node:fs'; -import { copyFile, mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { copyFile, mkdir, rm, writeFile } from 'node:fs/promises'; import { Socket } from 'node:net'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -51,6 +51,7 @@ import { toJsonSchemaSync, } from '@aflow/schemas'; import { z } from 'zod'; +import { makeScratch } from '../executorScratch.js'; import { browserFailure, type BrowserCall, @@ -286,8 +287,9 @@ export async function openHarnessBrowser(options: HarnessBrowserOptions): Promis let profileId: string; let ephemeralDir: string | undefined; if (ephemeral) { - ephemeralDir = await mkdtemp( - join(options.ephemeralRoot ?? tmpdir(), EPHEMERAL_PROFILE_SCRATCH_PREFIX), + ephemeralDir = await makeScratch( + EPHEMERAL_PROFILE_SCRATCH_PREFIX, + options.ephemeralRoot ?? tmpdir(), ); profileId = driver.startEphemeral(scope, ephemeralDir, options.reach); } else { diff --git a/apps/aflow-executor-host/src/commitCheck.ts b/apps/aflow-executor-host/src/commitCheck.ts index 788a1bd0..ec6e62c0 100644 --- a/apps/aflow-executor-host/src/commitCheck.ts +++ b/apps/aflow-executor-host/src/commitCheck.ts @@ -10,8 +10,7 @@ * printed is kept in the order it came, and from both ends where there is too * much of it: its start says what ran, and a failing check says why last. */ -import { mkdtemp, readFile, rm } from 'node:fs/promises'; -import { tmpdir } from 'node:os'; +import { readFile, rm } from 'node:fs/promises'; import { join } from 'node:path'; import { @@ -27,11 +26,18 @@ import { z } from 'zod'; import type { HostBinding } from './bindings.js'; import { SHORT_SHA_LENGTH } from './checkReceipt.js'; import { createChatterStripper } from './egressRefusals.js'; +import { makeAgentScratch, makeScratch } from './executorScratch.js'; import { formatMinutes } from './folderChecks.js'; import { runUnderFolderPosture } from './folderRun.js'; import type { SandboxedRunResult } from './sandboxedRun.js'; import { sandboxPostureOf } from './sandboxPosture.js'; -import { NO_REPLACE_OBJECTS_ENV, prepareWorktree, removeWorktree } from './worktree.js'; +import { checkoutWidening } from './sandboxPolicy.js'; +import { + borrowedReads, + NO_REPLACE_OBJECTS_ENV, + prepareWorktree, + removeWorktree, +} from './worktree.js'; type HostCommitCheckOutput = z.infer; @@ -206,8 +212,9 @@ export interface FolderCheckInput { } export async function runFolderChecks(input: FolderCheckInput): Promise { - const scratch = await mkdtemp(join(tmpdir(), CHECK_SCRATCH_PREFIX)); - const reportPath = join(scratch, CHECK_REPORT_FILE); + const scratch = await makeScratch(CHECK_SCRATCH_PREFIX); + const agentScratch = await makeAgentScratch(scratch); + const reportPath = join(agentScratch, CHECK_REPORT_FILE); let worktreePath: string | undefined; try { const worktree = await prepareWorktree(input.binding.root, scratch, 'check', { @@ -224,7 +231,7 @@ export async function runFolderChecks(input: FolderCheckInput): Promise { + const dir = join(scratch, AGENT_SCRATCH_DIR); + await mkdir(dir, { recursive: true }); + return dir; +} + +/** A new scratch directory named `…` under `root`, marked as this executor's. */ +export async function makeScratch(prefix: string, root: string = tmpdir()): Promise { + const scratch = await mkdtemp(join(root, prefix)); + await writeFile(join(scratch, SCRATCH_MARKER), '', 'utf8'); + return scratch; +} + +/** Whether `scratch` carries the marker `makeScratch` leaves. */ +export async function isMarkedScratch(scratch: string): Promise { + return (await lstat(join(scratch, SCRATCH_MARKER)).catch(() => undefined))?.isFile() === true; +} diff --git a/apps/aflow-executor-host/src/gitReadiness.ts b/apps/aflow-executor-host/src/gitReadiness.ts new file mode 100644 index 00000000..ac96380d --- /dev/null +++ b/apps/aflow-executor-host/src/gitReadiness.ts @@ -0,0 +1,70 @@ +/** + * The git a publication needs on this machine. + * + * A publication's commit reaches the folder by the executor's git fetching it + * from the clone it was made in with `--no-write-fetch-head` + * (`commitPatchOnBranch`), so the FETCH_HEAD another fetch is about to read is + * never overwritten. Git 2.29 is the first release with that flag; below it the + * fetch fails, so the publication is refused before it makes anything. Nothing + * else the lane runs needs the flag, so nothing else is refused for it. + */ +import { gitVersionText } from './worktree.js'; + +const GIT_VERSION_TIMEOUT_MS = 10_000; + +export const PUBLICATION_MIN_GIT_VERSION = { major: 2, minor: 29 } as const; + +export interface GitVersion { + readonly major: number; + readonly minor: number; + readonly patch: number; +} + +/** `git version 2.39.5 (Apple Git-154)` and `git version 2.52.0.windows.1` alike. */ +export function parseGitVersion(text: string): GitVersion | undefined { + const match = /\bgit version (\d+)\.(\d+)(?:\.(\d+))?/.exec(text); + if (match === null) return undefined; + return { + major: Number(match[1]), + minor: Number(match[2]), + patch: match[3] === undefined ? 0 : Number(match[3]), + }; +} + +export function fetchesWithoutFetchHead(version: GitVersion): boolean { + const floor = PUBLICATION_MIN_GIT_VERSION; + return ( + version.major > floor.major || (version.major === floor.major && version.minor >= floor.minor) + ); +} + +export function publicationGitMissing(versionText: string | undefined): string[] { + const needed = `${String(PUBLICATION_MIN_GIT_VERSION.major)}.${String(PUBLICATION_MIN_GIT_VERSION.minor)}`; + if (versionText === undefined) { + return [`git ${needed} or later — this machine's git did not report a version`]; + } + const version = parseGitVersion(versionText); + if (version === undefined) { + return [`git ${needed} or later — this machine's git reported \`${versionText.trim()}\``]; + } + if (fetchesWithoutFetchHead(version)) return []; + const found = `${String(version.major)}.${String(version.minor)}.${String(version.patch)}`; + return [ + `git ${needed} or later — this machine has ${found}, whose \`git fetch\` has no ` + + '`--no-write-fetch-head`, so a commit made in a clone could not be brought into the folder', + ]; +} + +/** The git the executor runs on its own `PATH`, which is the one that publishes. */ +export async function publicationGitReadiness(): Promise<{ ready: boolean; missing: string[] }> { + const missing = publicationGitMissing(await gitVersionText(GIT_VERSION_TIMEOUT_MS)); + return { ready: missing.length === 0, missing }; +} + +export function noPublicationGitMessage(missing: readonly string[]): string { + return ( + "This machine's git cannot bring a commit made in a clone into the folder, so nothing " + + 'can be published here.' + + (missing.length > 0 ? ` Missing: ${missing.join('; ')}.` : '') + ); +} diff --git a/apps/aflow-executor-host/src/handlers/harnessHandlers.ts b/apps/aflow-executor-host/src/handlers/harnessHandlers.ts index 347b1bb1..539a7cb2 100644 --- a/apps/aflow-executor-host/src/handlers/harnessHandlers.ts +++ b/apps/aflow-executor-host/src/handlers/harnessHandlers.ts @@ -9,15 +9,16 @@ * the boundary blocks. Its profile — in the machine's own policy file — is what * opens exactly those and nothing else. * - * The run happens in a detached worktree, never the operator's checkout. That - * is what lets a run start while they have uncommitted work, and what makes the - * result a diff to review rather than an edit already made. Nothing is - * committed, the agent's ordinary git is refused a branch or tag move - * (`refGuard.ts` names the calls that are not), and the worktree is removed once its changes have been collected. + * The run happens in a private clone of the folder, detached at its base, never + * the operator's checkout. That is what lets a run start while they have + * uncommitted work, and what makes the result a diff to review rather than an + * edit already made. The clone's refs are nobody else's, so whatever git the + * agent runs moves only those (Plan 315 F56). Nothing is committed, and the + * clone is removed once its changes have been collected. */ -import { mkdir, mkdtemp, readdir, readFile, realpath, rm, stat } from 'node:fs/promises'; +import { lstat, mkdir, readdir, readFile, rm, stat } from 'node:fs/promises'; import { tmpdir } from 'node:os'; -import { basename, join, sep } from 'node:path'; +import { basename, join } from 'node:path'; import AjvModule from 'ajv'; import type { z } from 'zod'; @@ -102,27 +103,30 @@ import { import { createChatterStripper } from '../egressRefusals.js'; import { createHarnessEventReader } from '../harnessEvents.js'; import { - changedRefs, + borrowedReads, checkApplies, + checkoutAt, + checkoutOfGitDir, collectChanges, currentHead, DIFF_CEILING_BYTES, fetchRemoteBase, + folderPrefix, INLINE_DIFF_CAP_BYTES, - linkedWorktrees, + isRealDirectory, NO_REPLACE_OBJECTS_ENV, prepareWorktree, removeWorktree, resolveCommit, RUN_SCRATCH_DIR, - snapshotRefs, utf8Prefix, WorktreeError, - type LinkedWorktree, + type PreparedWorktree, } from '../worktree.js'; import { PUBLICATION_SCRATCH_PREFIX } from '../branchCommit.js'; import { CHECK_SCRATCH_PREFIX } from '../commitCheck.js'; -import { installRefGuard, noRefGuardMessage, refGuardReadiness } from '../refGuard.js'; +import { isMarkedScratch, makeAgentScratch, makeScratch } from '../executorScratch.js'; +import { checkoutWidening } from '../sandboxPolicy.js'; import { browserFailure, jobScopeOf } from './browserHandler.js'; /** @@ -496,7 +500,6 @@ async function runHarness( let scratch: string | undefined; let worktreePath: string | undefined; - let bindingRoot: string | undefined; let credential: string | undefined; let claimed: HarnessSession | undefined; let browser: HarnessBrowser | undefined; @@ -531,9 +534,9 @@ async function runHarness( // A harness edits files and runs commands. It needs the grant that had to // be typed, not the one a folder gets by being connected. requireExecution(binding); - // Creating a worktree records it under the repository's own `.git`, so the - // binding has to permit writes even though the harness never uses that - // permission — its policy withholds it below. + // A base or a merge source that names a remote is fetched into the + // repository's own `.git`, so the binding has to permit writes even though + // the harness never uses that permission — its policy withholds it below. requireWritable(binding); const selection = selectHarness(policy.harnesses, input.harness); if (selection.kind === 'ambiguous') { @@ -552,22 +555,16 @@ async function runHarness( ); } } - bindingRoot = binding.root; - const readiness = sandboxReadiness(); if (!readiness.ready) { return await failureWithError(ctx, permissionError(noSandboxMessage(readiness.missing))); } - const refGuard = await refGuardReadiness(); - if (!refGuard.ready) { - return await failureWithError(ctx, permissionError(noRefGuardMessage(refGuard.missing))); - } // An abandoned session holds a checkout on disk, so expiry is collected // here rather than on a timer — the work is bounded by what is expired, and // arrives with a run that is already doing filesystem work. for (const stale of expiredSessions(Date.now())) { - await removeWorktree(stale.bindingRoot, stale.worktreePath).catch(() => {}); + await removeWorktree(stale.worktreePath).catch(() => {}); await discardScratch(stale); } @@ -657,10 +654,13 @@ async function runHarness( // is judged against that base as well. const base = namedRef ?? session?.base; - let worktree: { path: string; baseSha: string }; + let worktree: PreparedWorktree; if (session !== undefined && namedBase === undefined) { scratch = session.scratchDir; - worktree = { path: session.worktreePath, baseSha: session.baseSha }; + worktree = { + ...checkoutAt(session.worktreePath, await folderPrefix(binding.root)), + baseSha: session.baseSha, + }; keepScratch = true; } else if (session !== undefined && namedBase !== undefined) { // The conversation continues in a fresh checkout at the named base: the @@ -668,7 +668,7 @@ async function runHarness( // starting point. scratch = session.scratchDir; keepScratch = true; - await removeWorktree(session.bindingRoot, session.worktreePath); + await removeWorktree(session.worktreePath); try { worktree = await prepareWorktree(binding.root, scratch, 'work', { at: namedBase }); } catch (error) { @@ -687,7 +687,7 @@ async function runHarness( session = moved; claimed = moved; } else { - scratch = await mkdtemp(join(tmpdir(), HARNESS_SCRATCH_PREFIX)); + scratch = await makeScratch(HARNESS_SCRATCH_PREFIX); worktree = await prepareWorktree( binding.root, scratch, @@ -696,11 +696,12 @@ async function runHarness( ); } worktreePath = worktree.path; + const borrowed = await borrowedReads(binding.root); const keptCheckout = session !== undefined && namedBase === undefined; let merge: BaseMerge | undefined = keptCheckout ? session?.merge : undefined; if (mergeSource !== undefined && mergeIdentity !== undefined) { - merge = await mergeIntoCheckout(worktree.path, mergeSource, mergeIdentity); + merge = await mergeIntoCheckout(worktree, mergeSource, mergeIdentity); if (merge !== undefined && session !== undefined) { session = { ...session, merge }; recordSession(session); @@ -708,22 +709,23 @@ async function runHarness( } } - const refsBefore = await snapshotRefs(binding.root); - // Fetched before the run and held only for its duration. credential = await fetchCredential(profile); + // Everything the harness writes outside its checkout goes here, and the + // sandbox is granted this rather than the scratch holding the checkout. + const agentScratch = await makeAgentScratch(scratch); + // A configuration directory of the run's own, so a harness that keeps state // has somewhere to keep it that is not the operator's home. A session keeps // the same one across turns, which is where the conversation lives. let configDir: string | undefined; if (profile.configDirEnv !== undefined) { - configDir = session?.configDir ?? join(scratch, 'harness-config'); + configDir = session?.configDir ?? join(agentScratch, 'harness-config'); await mkdir(configDir, { recursive: true }); } const scratchDir = scratch; - const refGuardEnv = await installRefGuard(scratchDir, binding.root); // What the harness said, as opposed to what it printed. Set per turn, so // the last turn's answer is the one that comes back — the same rule the // run result itself follows. @@ -747,7 +749,7 @@ async function runHarness( scope: { ...jobScopeOf(ctx), spaceId }, profile: input.browser.profile, reach: { allowedDomains: profile.allowedDomains, localPorts: profile.browserLocalPorts }, - scratchDir, + scratchDir: agentScratch, stepExecutionId: ctx.stepExecutionId, storeScreenshot: async (image) => await browserService.storeScreenshot(tenantId, image), onActivity, @@ -785,13 +787,12 @@ async function runHarness( continued, browserTurn?.mcpConfigPath, ), - cwd: worktree.path, + cwd: worktree.workDir, // The sandbox's proxy names the host it refused only when asked to, and // that name is the whole diagnosis for a harness that reached nothing. env: {}, trustedEnv: { SRT_DEBUG: '1', - ...refGuardEnv, // Commissions see the stored objects too, as the checkout they run // in does: a review reads the range with its own git, and the // verdict that can stand in for the operator's approval has to be @@ -807,13 +808,13 @@ async function runHarness( : {}), }, timeoutMs: input.timeoutMs, - scratchDir, + scratchDir: agentScratch, + journalScratchDir: scratchDir, widening: { authPaths: profile.authPaths, writePaths: profile.writePaths, allowedDomains: profile.allowedDomains, - writableRoot: worktree.path, - withholdBindingWrite: true, + ...checkoutWidening(worktree, borrowed), }, idPrefix: 'hr', ownerRunId: ctx.runId, @@ -858,11 +859,10 @@ async function runHarness( ownerRunId: ctx.runId, logicalExecutionId: ctx.logicalExecutionId, bindingId: binding.id, - bindingRoot: binding.root, harnessId: profile.id, worktreePath: worktree.path, scratchDir, - configDir: configDir ?? join(scratchDir, 'harness-config'), + configDir: configDir ?? join(agentScratch, 'harness-config'), conversationId, baseSha: worktree.baseSha, ...(base !== undefined ? { base } : {}), @@ -879,7 +879,7 @@ async function runHarness( const brief = composeHarnessTask( input.task, input.inputs, - await hasRepositoryRules(worktree.path), + await hasRepositoryRules(worktree.workDir), keptCheckout ? undefined : merge, ); const task = continuesAttempt ? continuedAttemptTask(brief) : brief; @@ -889,7 +889,7 @@ async function runHarness( result = await runTurn(task, resuming, conversationId); } else { const { schema, validate } = expected; - const resultPath = join(worktree.path, RESULT_RELATIVE_PATH); + const resultPath = join(worktree.workDir, RESULT_RELATIVE_PATH); // A session's checkout still holds the answer to its last turn, and a // stale one reads exactly like a fresh one. await rm(resultPath, { force: true }); @@ -935,7 +935,7 @@ async function runHarness( }; let collectFailure: string | undefined; try { - changes = await collectChanges(worktree.path); + changes = await collectChanges(worktree); } catch (error) { collectFailure = error instanceof Error ? error.message : String(error); } @@ -969,13 +969,12 @@ async function runHarness( : [collectFailure, refusalNote].filter(Boolean).join(' '), credential, ); - const refChanges = changedRefs(refsBefore, await snapshotRefs(binding.root)); - // Asked of the binding, not the worktree: the question is whether what the + // Asked of the binding, not the clone: the question is whether what the // harness produced can still be taken into the repository the operator has. // A named base is where a publication would append the diff, so the tree - // that answers is that ref's head as it stands now, not the folder's working - // tree. A base ref gone since the run started is refused below as a ref - // change, and the commit the checkout started at stands in until then. + // that answers is that ref's head in the folder as it stands now, not the + // folder's working tree; a base ref gone since the run started leaves the + // commit the checkout started at to stand in. // A merged checkout is judged against its merge, which is where the // publication applies the diff once it has made that merge again; a branch // that moved since is the publication's `stale_base`, not this answer's. @@ -985,7 +984,15 @@ async function runHarness( : base === undefined ? undefined : await resolveCommit(binding.root, base).catch(() => worktree.baseSha); - const applies = await checkApplies(binding.root, changes.patch, judgedAt); + // A commit is judged through an index of its own, in the clone: the merge + // commit exists only there, and the clone reads every object the folder has. + // Reached through the git directory this lane made, never the checkout's + // `.git`, which the harness could have pointed at a repository of its own. + const applies = await checkApplies( + judgedAt === undefined ? binding.root : worktree, + changes.patch, + judgedAt, + ); // A named base is judged against itself; the folder's HEAD was never the // starting point, so its distance from it says nothing about the run. const headMoved = base === undefined && (await currentHead(binding.root)) !== worktree.baseSha; @@ -1029,7 +1036,6 @@ async function runHarness( applies: applies.state, ...(applies.state === 'conflict' ? { applyConflict: applies.detail } : {}), headMoved, - refChanges, exitCode: result.exitCode, timedOut: result.timedOut, durationMs: result.durationMs, @@ -1087,9 +1093,7 @@ async function runHarness( await browser?.close(); if (claimed !== undefined) releaseSession(claimed); if (!keepScratch) { - if (worktreePath !== undefined && bindingRoot !== undefined) { - await removeWorktree(bindingRoot, worktreePath); - } + if (worktreePath !== undefined) await removeWorktree(worktreePath); if (scratch !== undefined) await rm(scratch, { recursive: true, force: true }); } } @@ -1112,8 +1116,8 @@ export function createHostHarnessHandler( } export interface OrphanedCheckouts { - /** Checkouts taken off each connected folder, by the folder's root. */ - readonly removed: ReadonlyMap; + /** Clones removed from the scratch directories below. */ + readonly checkouts: number; /** Scratch directories removed from the temp root. */ readonly scratchDirs: number; } @@ -1122,91 +1126,71 @@ export interface OrphanedCheckouts { * Take away the checkouts no session owns. * * Sessions live in this executor's memory, so a restart forgets every one of - * them while their checkouts stay registered in the operator's repository and - * their scratch stays in the temp root, with nothing left that would ever - * expire them. Run at boot, before any job, when every checkout this lane made - * is one of those. - * - * Only a detached checkout under the temp root, in a directory named the way - * this executor names its own, is taken: a checkout with a branch in it, or - * anywhere else, is the operator's. + * them while their scratch, and the clone in it, stays in the temp root with + * nothing left that would ever expire it. Run at boot, before any job, when + * every checkout this lane made is one of those. * - * `roots` undefined means the connected folders could not be read, and then - * nothing is taken. A scratch directory holds a checkout registered in one of - * those folders, and removing it first leaves that registration behind in the - * operator's repository as a stale entry. + * What is swept is marked scratch and nothing else: each directory under the + * temp root named the way this executor names its scratch, owned by no session + * and carrying the marker `makeScratch` leaves, whole. Whatever a run left in + * it is the executor's, a clone killed before its checkout was detached + * included, so what decides is the directory and never the shape of a checkout + * in it. A directory without the marker is left, whatever it holds, and no + * connected folder is read or written. Inside one, only a real directory is + * treated as a checkout; a link is removed with the scratch and nothing it + * leads to is walked. */ export async function removeOrphanedCheckouts( - roots: readonly string[] | undefined, tempRoot: string = tmpdir(), ): Promise { - if (roots === undefined) return { removed: new Map(), scratchDirs: 0 }; - const owned = new Set(allSessions().map((session) => basename(session.scratchDir))); const unowned = (name: string): boolean => CHECKOUT_SCRATCH_PREFIXES.some((prefix) => name.startsWith(prefix)) && !owned.has(name); - // git records a checkout by its resolved path, and the temp root is commonly - // reached through a link (`/var` on macOS), so both spellings are asked. - const tempRoots = new Set([tempRoot, await realpath(tempRoot).catch(() => tempRoot)]); - const underUnownedScratch = (path: string): boolean => { - for (const under of tempRoots) { - if (!path.startsWith(under + sep)) continue; - return unowned(path.slice(under.length + 1).split(sep)[0] ?? ''); - } - return false; - }; - - const removed = new Map(); - for (const root of new Set(roots)) { - let checkouts: LinkedWorktree[]; - let rootReal: string; - try { - checkouts = await linkedWorktrees(root); - rootReal = await realpath(root); - } catch { - // Not a repository, or not there: nothing of this lane's is registered. - continue; - } - for (const checkout of checkouts) { - // A folder connected from inside a checkout lists itself among them. - if (checkout.path === rootReal || !checkout.detached) continue; - if (!underUnownedScratch(checkout.path)) continue; - await removeWorktree(root, checkout.path); - removed.set(root, (removed.get(root) ?? 0) + 1); - } - } + let checkouts = 0; let scratchDirs = 0; for (const name of await readdir(tempRoot).catch(() => [] as string[])) { if (!unowned(name)) continue; const scratchDir = join(tempRoot, name); - // A checkout still in it is registered in a repository the loop above did - // not take it from, and is left for that repository to account for. - if (await holdsCheckout(scratchDir)) continue; + if (!(await isRealDirectory(scratchDir)) || !(await isMarkedScratch(scratchDir))) continue; + for (const checkout of await checkoutsIn(scratchDir)) { + await removeWorktree(checkout); + checkouts += 1; + } await discardScratch({ scratchDir }); scratchDirs += 1; } - return { removed, scratchDirs }; + return { checkouts, scratchDirs }; } -async function holdsCheckout(scratchDir: string): Promise { +/** + * The checkouts in a scratch directory, made whole or in part: a directory + * holding a `.git` of any kind, and one whose git directory alone was made. + * Only a real directory counts, as the entry and as the checkout it names: the + * sweep decides nothing by a scratch's shape, and a link standing in a + * checkout's place would lead the removal into whatever the link points at. + */ +async function checkoutsIn(scratchDir: string): Promise { + const found = new Set(); for (const entry of await readdir(scratchDir).catch(() => [] as string[])) { - if ( - await stat(join(scratchDir, entry, '.git')).then( - () => true, - () => false, - ) - ) { - return true; + const path = join(scratchDir, entry); + if (!(await isRealDirectory(path))) continue; + if ((await lstat(join(path, '.git')).catch(() => undefined)) !== undefined) { + found.add(path); + continue; } + const checkout = checkoutOfGitDir(path); + if (checkout === undefined) continue; + const info = await lstat(checkout).catch(() => undefined); + if (info === undefined || info.isDirectory()) found.add(checkout); } - return false; + return [...found]; } /** * Drop every session under a binding, removing what they left on disk. Used - * when the binding is gone, so `removeWorktree` has no repository to prune - * against and the directory itself is what gets taken away. + * when the binding is gone; the scratch holds the session's clone, so taking + * the directory away takes the checkout with it. */ async function discardSessionsForBinding(bindingId: string): Promise { for (const session of sessionsForBinding(bindingId)) { diff --git a/apps/aflow-executor-host/src/harnessSessions.ts b/apps/aflow-executor-host/src/harnessSessions.ts index e285edf8..ccce67cf 100644 --- a/apps/aflow-executor-host/src/harnessSessions.ts +++ b/apps/aflow-executor-host/src/harnessSessions.ts @@ -41,17 +41,10 @@ export interface HarnessSession { */ readonly logicalExecutionId: string; readonly bindingId: string; - /** - * The repository this session's worktree belongs to. Held here because - * cleanup happens on whatever run notices the session is stale, and that run - * may be working in a different binding — removing a worktree against the - * wrong repository leaves a permanent stale entry in that repository instead. - */ - readonly bindingRoot: string; readonly harnessId: string; - /** The checkout the conversation has been reasoning about. */ + /** The private clone the conversation has been reasoning about. */ readonly worktreePath: string; - /** Scratch holding the worktree and the harness's state directory. */ + /** Scratch holding the clone and the harness's state directory. */ readonly scratchDir: string; readonly configDir: string; /** What the harness calls this conversation. Minted here, so it need not be parsed back out. */ @@ -184,8 +177,8 @@ export function forgetSession(sessionRef: string): void { /** * Sessions whose scratch should be removed: idle past the ttl, or every one of * them when a binding is withdrawn. Returned rather than deleted here, because - * removing a worktree needs the repository it belongs to and this module does - * not know about git. + * removing a checkout unlinks the installation mirrored into it first, and this + * module does not know about checkouts. */ export function expiredSessions(now: number): HarnessSession[] { const expired = [...sessions.values()].filter( diff --git a/apps/aflow-executor-host/src/index.ts b/apps/aflow-executor-host/src/index.ts index 812123b6..2a67493b 100644 --- a/apps/aflow-executor-host/src/index.ts +++ b/apps/aflow-executor-host/src/index.ts @@ -212,18 +212,12 @@ async function main(): Promise { log, }); - // Checkouts too: every session died with the previous executor, so what they - // held on disk and in the operator's repositories has nothing left that would - // ever expire it. - const bound = await loadHostPolicy(policyPath) - .then((policy) => [...policy.bindings.values()].map((binding) => binding.root)) - .catch(() => undefined); - const orphaned = await removeOrphanedCheckouts(bound); - const checkouts = [...orphaned.removed.values()].reduce((sum, count) => sum + count, 0); - if (checkouts > 0 || orphaned.scratchDirs > 0) { + // Checkouts too: every session died with the previous executor, so the clones + // they held on disk have nothing left that would ever expire them. + const orphaned = await removeOrphanedCheckouts(); + if (orphaned.checkouts > 0 || orphaned.scratchDirs > 0) { log.info('Removed checkouts left behind by a previous run of this executor', { - checkouts, - folders: [...orphaned.removed.keys()], + checkouts: orphaned.checkouts, scratchDirs: orphaned.scratchDirs, }); } @@ -451,9 +445,7 @@ async function main(): Promise { // another harness request or a shutdown. const dropped = withdrawnSessions(permitted.bindings); for (const session of dropped) { - await removeWorktree(session.bindingRoot, session.worktreePath).catch( - () => undefined, - ); + await removeWorktree(session.worktreePath).catch(() => undefined); await discardScratch(session); } if (killed.length > 0 || dropped.length > 0) { @@ -660,7 +652,7 @@ async function main(): Promise { const killed = killProcessesForBinding(notice.hostBindingId); const dropped = dropSessionsForBinding(notice.hostBindingId); for (const session of dropped) { - await removeWorktree(session.bindingRoot, session.worktreePath).catch(() => undefined); + await removeWorktree(session.worktreePath).catch(() => undefined); await discardScratch(session); } if (killed.length > 0 || dropped.length > 0) { diff --git a/apps/aflow-executor-host/src/refGuard.ts b/apps/aflow-executor-host/src/refGuard.ts deleted file mode 100644 index ba8fd146..00000000 --- a/apps/aflow-executor-host/src/refGuard.ts +++ /dev/null @@ -1,179 +0,0 @@ -/** - * The coding agent's ordinary git cannot move the operator's branches or tags. - * - * A worktree shares its refs with the repository it was added to, so a harness - * running `git branch -D` or `git update-ref` in its checkout rewrites the - * operator's branches. Comparing the refs before and after a run sees that, but - * it cannot say who did it: the operator creating a branch in the folder while - * a commission runs looks exactly the same. So the stop happens inside the - * agent's own git, where the actor is known — a `reference-transaction` hook - * the executor owns, reached through git's environment config, which aborts any - * transaction in the folder's repository naming `refs/heads/*` or - * `refs/tags/*`. The worktree's detached `HEAD`, remote-tracking refs and the - * stash are outside both prefixes and stay free. - */ -import { mkdir, writeFile } from 'node:fs/promises'; -import { join } from 'node:path'; - -import { commonGitDir, gitVersionText } from './worktree.js'; - -const GIT_VERSION_TIMEOUT_MS = 10_000; - -/** - * Git 2.28 is the first release that runs a `reference-transaction` hook. Below - * it the hook is never invoked, so a commission's git could move a branch and - * nothing would stop it. - */ -export const REF_GUARD_MIN_GIT_VERSION = { major: 2, minor: 28 } as const; - -function shellQuoted(text: string): string { - return `'${text.replaceAll("'", `'\\''`)}'`; -} - -/** - * The hook for a run in the folder whose repository directory is - * `guardedCommonDir`. - * - * The environment config reaches every git the run starts, including those in - * repositories the run creates for itself — a test suite that commits in a - * directory under the temp root is the common case. Those refs are nobody's but - * the run's, so the hook refuses only in the folder's own repository, which the - * run's checkout shares. A repository the hook cannot identify is treated as - * the folder's, and so is one whose `refs` directory is the folder's: a - * git-new-workdir layout has a `.git` of its own with `refs` symlinked into the - * folder's, so its branches are the operator's. A plain copy of `.git` has refs - * of its own and stays another repository. - * - * "The folder's own" is decided by identity, not by spelling. The repository's - * path is whatever the run's git was told, and one directory has many names: a - * symlink, a relative path, a firmlink prefix such as `/System/Volumes/Data`, - * and on a case-insensitive volume every mix of letter case. Resolving the path - * and comparing text misses the last two, so the hook asks `test -ef` whether - * both names are the same directory. The path reaches `test` only as - * `./`-prefixed or absolute, so a name like `-P` or `!` is never read as an - * operator. `IFS` is set to the single space git separates each update's fields - * with, because `read` splits on it and an inherited one could keep a ref name - * from matching. - * - * Read from stdin whole before deciding: git writes every update of the - * transaction to the hook, and a hook that stops reading part-way can leave it - * writing into a closed pipe. - */ -export function referenceTransactionHook(guardedCommonDir: string): string { - return `#!/bin/sh -[ "$1" = prepared ] || { cat >/dev/null; exit 0; } -IFS=' ' -common=$(git rev-parse --git-common-dir 2>/dev/null) || common= -case $common in - '' | /*) ;; - *) common=./$common ;; -esac -if [ -n "$common" ] && [ -d "$common" ] && ! [ "$common" -ef ${shellQuoted(guardedCommonDir)} ] && - ! [ "$common/refs" -ef ${shellQuoted(join(guardedCommonDir, 'refs'))} ]; then - cat >/dev/null - exit 0 -fi -refused= -while read -r old new ref; do - case "$ref" in - refs/heads/*|refs/tags/*) [ -n "$refused" ] || refused=$ref ;; - esac -done -[ -z "$refused" ] && exit 0 -echo "Refused \\\`$refused\\\`: a commission may not move branches or tags. Its work stays in this checkout, and what becomes of it is decided after the run." >&2 -exit 1 -`; -} - -export function refGuardHooksDir(scratchDir: string): string { - return join(scratchDir, 'git-hooks'); -} - -/** - * Writes the hook guarding the folder at `root` into the run's scratch and - * returns the environment that points the harness's git at it. - * - * Environment config sits above every config file git reads — system, global, - * the repository's and the worktree's — so a `core.hooksPath` in any of them, - * including a home directory the harness can write, does not displace it. - * - * Two calls run without it. One names its own hooks path on the command line - * (`git -c core.hooksPath=/elsewhere …`). The other pushes into the folder - * locally (`git push . HEAD:refs/heads/x`, or to the folder's path): the - * receiving git is started with the environment config cleared. The guard stops - * a commission's git from moving a branch or tag by accident, not a call that - * goes around it. Every move is still recorded by the run's before-and-after - * snapshot of the folder's refs, reported as `refChanges` on the result. - */ -export async function installRefGuard( - scratchDir: string, - root: string, -): Promise> { - const dir = refGuardHooksDir(scratchDir); - await mkdir(dir, { recursive: true }); - await writeFile( - join(dir, 'reference-transaction'), - referenceTransactionHook(await commonGitDir(root)), - { mode: 0o755 }, - ); - return { - GIT_CONFIG_COUNT: '1', - GIT_CONFIG_KEY_0: 'core.hooksPath', - GIT_CONFIG_VALUE_0: dir, - }; -} - -export interface GitVersion { - readonly major: number; - readonly minor: number; - readonly patch: number; -} - -/** `git version 2.39.5 (Apple Git-154)` and `git version 2.52.0.windows.1` alike. */ -export function parseGitVersion(text: string): GitVersion | undefined { - const match = /\bgit version (\d+)\.(\d+)(?:\.(\d+))?/.exec(text); - if (match === null) return undefined; - return { - major: Number(match[1]), - minor: Number(match[2]), - patch: match[3] === undefined ? 0 : Number(match[3]), - }; -} - -export function runsReferenceTransactionHook(version: GitVersion): boolean { - const floor = REF_GUARD_MIN_GIT_VERSION; - return ( - version.major > floor.major || (version.major === floor.major && version.minor >= floor.minor) - ); -} - -export function refGuardMissing(versionText: string | undefined): string[] { - const needed = `${String(REF_GUARD_MIN_GIT_VERSION.major)}.${String(REF_GUARD_MIN_GIT_VERSION.minor)}`; - if (versionText === undefined) { - return [`git ${needed} or later — this machine's git did not report a version`]; - } - const version = parseGitVersion(versionText); - if (version === undefined) { - return [`git ${needed} or later — this machine's git reported \`${versionText.trim()}\``]; - } - if (runsReferenceTransactionHook(version)) return []; - const found = `${String(version.major)}.${String(version.minor)}.${String(version.patch)}`; - return [ - `git ${needed} or later — this machine has ${found}, which runs no reference-transaction ` + - 'hook, so nothing would stop a commission from moving a branch', - ]; -} - -/** The git a harness finds on its `PATH`, which is the executor's own. */ -export async function refGuardReadiness(): Promise<{ ready: boolean; missing: string[] }> { - const missing = refGuardMissing(await gitVersionText(GIT_VERSION_TIMEOUT_MS)); - return { ready: missing.length === 0, missing }; -} - -export function noRefGuardMessage(missing: readonly string[]): string { - return ( - "This machine's git cannot stop a coding agent from moving the operator's branches and " + - 'tags, so a commission cannot run here.' + - (missing.length > 0 ? ` Missing: ${missing.join('; ')}.` : '') - ); -} diff --git a/apps/aflow-executor-host/src/sandboxPolicy.ts b/apps/aflow-executor-host/src/sandboxPolicy.ts index 76507ec6..39face20 100644 --- a/apps/aflow-executor-host/src/sandboxPolicy.ts +++ b/apps/aflow-executor-host/src/sandboxPolicy.ts @@ -120,6 +120,21 @@ export interface SandboxWidening { * isolated worktree while the connected folder stays untouched. */ readonly writableRoot?: string; + /** + * The writable root's git directory, kept outside it in the job's scratch. + * Read, so the job's own git works, and never written: its config is what + * the executor's git reads when it stages and judges the job's work, outside + * this boundary. The scratch holding both is not the job's to write — it is + * handed a directory of its own inside it (`makeAgentScratch`) — and the + * denial holds whatever a caller hands as `scratchDir`. + */ + readonly gitDir?: string; + /** + * What the writable root reads outside the binding's root and never writes: + * the repository's object store and its installations, each where it lies + * outside the folder (`borrowedReads`, `worktree.ts`). + */ + readonly borrowedReads?: readonly string[]; /** * Withhold the binding's own write grant even when it has one. A coding * harness works in its worktree and never needs the connected folder, so a @@ -129,6 +144,48 @@ export interface SandboxWidening { readonly withholdBindingWrite?: boolean; } +/** + * The boundary of a run in a private clone of the folder — a commission's and a + * folder check's alike, which is why both take their widening here. + * + * Reads are denied under home and open elsewhere, so what is enumerated under + * "reads" is what is re-allowed inside home; the clone, kept in the executor's + * scratch outside home, is readable whole either way. For a folder that holds + * its repository's object store and installations, the run may: + * + * - read the folder; its agent scratch; the folder's place in the clone; the + * clone's git directory; the toolchains (`toolchainReadPaths`), the machine's + * tool paths and the harness profile's auth and write paths; + * - write its agent scratch, the folder's place in the clone, and the profile's + * write paths; + * - neither read nor write home otherwise, the host directory, cargo's + * credential files, or `.git/hooks` at any depth under the folder and the + * writable root; + * - not write the folder, the scratch around the clone (only `agent/` inside it + * is handed over), the clone's git directory, or `.git/config` at any depth + * under the folder and the writable root. + * + * For any other folder the same, and it may also read, never write, what the + * clone borrows from outside the folder (`borrowedReads`, `worktree.ts`): the + * repository's object store, which holds every object the clone has, wherever + * it lies outside the folder — a folder inside a repository, a linked + * worktree, a submodule, a separate git directory — and each installation in + * the repository outside the folder, where the clone's mirrored dependencies + * lead. Without them, inside the run the clone's git cannot read HEAD's tree + * and a dependency installed outside the folder does not resolve. + */ +export function checkoutWidening( + checkout: { readonly workDir: string; readonly gitDir: string }, + borrowed: readonly string[], +): Pick { + return { + writableRoot: checkout.workDir, + gitDir: checkout.gitDir, + borrowedReads: [...borrowed], + withholdBindingWrite: true, + }; +} + /** * Repository metadata anywhere beneath a root, not only at its top. * @@ -177,6 +234,7 @@ export function compileSandboxPolicy( const scratch = options.scratchDir ? [options.scratchDir] : []; const widening = options.widening; const writableRoot = widening?.writableRoot ? [widening.writableRoot] : []; + const gitDir = widening?.gitDir ? [widening.gitDir] : []; // The host directory holds the policy every gate reads — push approval, the // checks, the posture itself — and pairing's credential. Nothing a job is // granted reaches into it, under either posture, whatever a profile or the @@ -205,6 +263,8 @@ export function compileSandboxPolicy( binding.root, ...scratch, ...writableRoot, + ...gitDir, + ...(widening?.borrowedReads ?? []), ...toolchainReadPaths(home), ...(options.toolPaths ?? []), ...(widening?.authPaths ?? []), @@ -241,6 +301,7 @@ export function compileSandboxPolicy( ...writableRoot.flatMap((r) => repositoryExecutableSurfaces(r)), ...repositoryConfigUnder(binding.root), ...writableRoot.flatMap((r) => repositoryConfigUnder(r)), + ...gitDir, ], }, }; diff --git a/apps/aflow-executor-host/src/sandboxedRun.ts b/apps/aflow-executor-host/src/sandboxedRun.ts index 1504a437..b557fbc8 100644 --- a/apps/aflow-executor-host/src/sandboxedRun.ts +++ b/apps/aflow-executor-host/src/sandboxedRun.ts @@ -408,6 +408,14 @@ export interface SandboxedRunInput extends SupervisedRun { readonly trustedEnv?: Record; /** Where the compiled policy is written — outside the binding, always. */ readonly scratchDir: string; + /** + * What the orphan journal removes if this executor dies during the run, where + * that is more than `scratchDir`: the scratch holding the run's checkout, of + * which `scratchDir` is the part the sandbox writes (`makeAgentScratch`). Only + * journalled — a run that ends removes `scratchDir` alone, and its caller + * still collects the checkout. + */ + readonly journalScratchDir?: string; readonly widening?: SandboxWidening; /** Extra names the machine's own policy permits a workload to inherit. */ readonly inheritEnv?: readonly string[]; @@ -439,6 +447,8 @@ interface SupervisedSpawn extends SupervisedRun { * that nothing hands the orphan sweep a path it must not delete. */ readonly scratchDir?: string; + /** Journalled in place of `scratchDir` (`SandboxedRunInput.journalScratchDir`). */ + readonly journalScratchDir?: string; /** * Where the wrapper around a confined command records the workload's own * status. Absent for the one command that runs unconfined: it has no launcher @@ -704,7 +714,7 @@ async function superviseSpawn(input: SupervisedSpawn): Promise.smudge`, `.clean` or `.process` — and those are programs git * runs too. A filter is *selected* by `.gitattributes`, which lives in the - * working tree and which a harness may write, and *defined* in config. Checking - * a worktree out and staging with `git add -A` both apply them, here, outside + * working tree and which a harness may write, and *defined* in config. Making + * a checkout and staging with `git add -A` both apply them, here, outside * the sandbox, with the operator's PATH. * * Naming filters to disable does not work: the set is whatever the operator has * configured. Removing where they are defined does. System config is already * off; global is pointed at nothing; what remains is the repository's own - * `.git/config`, which this lane refuses to let anything write. + * config. A run's clone starts with one that defines none, in a git directory + * kept beside the checkout rather than in it (`checkoutGitDir`), which the + * sandbox reads and never writes. The checkout holds only a gitfile naming + * that directory, and the coding agent can rewrite the gitfile, or put a + * `.git` directory of its own with a `commondir` in its place — which moves + * nothing but the agent's own git. So the executor's git never resolves a + * checkout through what it holds: it names the directory itself (`Checkout`). */ const GIT_SAFETY_ARGS = [ '-c', @@ -183,13 +196,93 @@ export async function gitVersionText(timeoutMs: number): Promise { + for (const path of new Set([checkout.path, checkout.workDir])) { + const resolved = (await isRealDirectory(path)) + ? await realpath(path).catch(() => undefined) + : undefined; + if (resolved !== path) { + throw new WorktreeError( + `The run's checkout at ${path} is no longer the directory made for it, so git was not run in it.`, + 'checkout_replaced', + ); + } + } +} + +/** Where git runs: a folder by its path, a checkout by its own git directory. */ +async function gitLocation(at: string | Checkout): Promise { + if (typeof at === 'string') return ['-C', at]; + await assertCheckoutInPlace(at); + return ['-C', at.path, `--git-dir=${at.gitDir}`, `--work-tree=${at.path}`]; +} + export async function git( - cwd: string, + cwd: string | Checkout, args: string[], maxBuffer = 1024 * 1024, env: Record = {}, ): Promise { - const { stdout } = await run('git', ['-C', cwd, ...GIT_SAFETY_ARGS, ...args], { + const { stdout } = await run('git', [...(await gitLocation(cwd)), ...GIT_SAFETY_ARGS, ...args], { timeout: GIT_TIMEOUT_MS, maxBuffer, env: { ...gitEnv(), ...env }, @@ -351,7 +444,7 @@ export async function countCommits(root: string, base: string, head: string): Pr /** * The directory a checkout inherits rather than fetches again. * - * A worktree is created at a commit, so it carries what the repository tracks + * A checkout is created at a commit, so it carries what the repository tracks * and nothing else — and installed dependencies are, correctly, not tracked. A * harness asked to run the tests or the typecheck therefore reaches for the * package registry, which the boundary denies, and reports a network failure @@ -406,9 +499,36 @@ async function dependencyDirs(base: string): Promise { return found; } +/** + * The installations of the repository at `top` that a checkout of its folder at + * `prefix` carries, as paths from the top level: the union of every one the + * scan from the top level reaches and every one on the walk from the folder up + * to the top level, the folder's own included. The scan stops at + * `DEPENDENCY_SCAN_DEPTH`; the walk does not, so a folder three directories + * down or more still has its own installation and each above it. + */ +async function repositoryInstallations(top: string, prefix: string): Promise { + const installations = new Set(await dependencyDirs(top)); + const steps = prefix === '' ? [] : prefix.split(sep); + for (let depth = steps.length; depth >= 0; depth -= 1) { + const relative = join(...steps.slice(0, depth), DEPENDENCY_DIR); + if (installations.has(relative)) continue; + const info = await lstat(join(top, relative)).catch(() => undefined); + if (info?.isDirectory() === true || info?.isSymbolicLink() === true) { + installations.add(relative); + } + } + return [...installations]; +} + interface MirrorContext { - /** The connected folder, resolved, so containment is decided on real paths. */ + /** + * The repository's top level, resolved, so containment is decided on real + * paths: a workspace link from a connected package may lead to a sibling + * package, which the clone carries as much as the folder itself. + */ readonly rootReal: string; + /** The clone's top level, where a path inside the repository lands. */ readonly worktreePath: string; } @@ -493,17 +613,26 @@ async function mirrorInstallation( * sandbox grants writes to the worktree by path, and a write through a link * lands on the folder's own directory, which the harness's policy withholds. So * a harness can run the tests and cannot corrupt what the operator installed. + * + * Every installation `repositoryInstallations` names is linked, at the same + * place in the clone, whichever folder of the repository is connected and + * however deep it lies: the folder's own and each on its way up to the top + * level, since resolution walks up from the folder into the top level's + * installation, where a workspace hoists most of what a package needs; and + * every other the scan from the top level reaches, since a workspace link leads + * into a sibling package, whose own installation holds what was not hoisted. */ -async function linkDependencies(root: string, worktreePath: string): Promise { +async function linkDependencies(top: string, checkout: Checkout): Promise { let rootReal: string; try { - rootReal = await realpath(root); + rootReal = await realpath(top); } catch { - rootReal = root; + rootReal = top; } - const context: MirrorContext = { rootReal, worktreePath }; - for (const relative of await dependencyDirs(root)) { - const destination = join(worktreePath, relative); + const context: MirrorContext = { rootReal, worktreePath: checkout.path }; + const prefix = relativePath(checkout.path, checkout.workDir); + for (const relative of await repositoryInstallations(top, prefix)) { + const destination = join(checkout.path, relative); try { // Only where the checkout has the same place to put it, and has nothing // there already — a tracked directory of that name is the repository's. @@ -517,7 +646,7 @@ async function linkDependencies(root: string, worktreePath: string): Promise { await rmdir(directory).catch(() => {}); } +/** Whether `path` is itself a directory, never one a link leads to. */ +export async function isRealDirectory(path: string): Promise { + return (await lstat(path).catch(() => undefined))?.isDirectory() === true; +} + +/** + * Take the mirrors out of the checkout at `worktreePath`, and out of nothing + * else: a checkout path that is a link is not walked, since what it leads to is + * someone's real `node_modules` and its links are their installation. + */ async function unlinkDependencies(worktreePath: string): Promise { + if (!(await isRealDirectory(worktreePath))) return; for (const relative of await dependencyDirs(worktreePath)) { await removeMirror(join(worktreePath, relative)); } } -export interface PreparedWorktree { - /** Absolute path the harness runs in. */ - readonly path: string; +export interface PreparedWorktree extends Checkout { /** Commit the run started from, so the diff has a stated base. */ readonly baseSha: string; } @@ -587,13 +725,41 @@ export async function isGitRepository(root: string): Promise { } /** - * The folder's repository directory, shared by every worktree added to it, with - * symbolic links resolved — the one path that names this repository whichever - * of its checkouts a git runs in. + * Where a folder is in its repository, as a relative path with no trailing + * slash: empty for the top level, `packages/app` for a package of a monorepo. */ -export async function commonGitDir(root: string): Promise { - const reported = (await git(root, ['rev-parse', '--git-common-dir'])).trim(); - return await realpath(isAbsolute(reported) ? reported : join(root, reported)); +export async function folderPrefix(root: string): Promise { + return (await git(root, ['rev-parse', '--show-prefix'])).trim().replace(/\/$/, ''); +} + +/** + * What a run in a checkout of the folder at `root` reads outside that folder, + * for the sandbox to grant read of (`SandboxWidening.borrowedReads`). + * + * Decided by where each lies, never by where the folder is in its repository. + * The repository's object store, which the clone owns no object of and borrows + * every one from (`clonePrivately`), resolved, whenever it lies outside the + * folder: for a folder inside a repository, and for a top level whose store is + * elsewhere too — a linked worktree, a submodule, a separate git directory. + * And each installation outside the folder of the same set the clone's + * mirrored links lead into (`repositoryInstallations`, `linkDependencies`), so + * those on the folder's way up are readable however deep the folder lies. Read + * from the folder, never from the checkout, which the run writes. + */ +export async function borrowedReads(root: string): Promise { + const rootReal = await realpath(root).catch(() => root); + const outsideRoot = (path: string): boolean => + path !== rootReal && !path.startsWith(rootReal + sep); + const reads: string[] = []; + const objects = await gitPath(root, 'objects'); + const objectsReal = await realpath(objects).catch(() => objects); + if (outsideRoot(objectsReal)) reads.push(objectsReal); + const top = (await git(root, ['rev-parse', '--show-toplevel'])).trim(); + for (const relative of await repositoryInstallations(top, await folderPrefix(root))) { + const installation = join(top, relative); + if (outsideRoot(installation)) reads.push(installation); + } + return reads; } export interface PrepareWorktreeOptions { @@ -781,7 +947,7 @@ export async function remotePushUrls(root: string, remote: string): Promise { @@ -829,17 +995,62 @@ export async function prepareWorktree( } } - const path = join(scratchDir, runKey); + // Recorded resolved, which is what every git in it checks it against + // (`assertCheckoutInPlace`). + await mkdir(scratchDir, { recursive: true }); + let checkout = checkoutAt(join(await realpath(scratchDir), runKey)); + let top: string; try { - await git(root, ['worktree', 'add', '--detach', path, baseSha]); + top = (await git(root, ['rev-parse', '--show-toplevel'])).trim(); + checkout = checkoutAt(checkout.path, await folderPrefix(root)); + await clonePrivately(top, checkout, baseSha); } catch (error) { + await rm(checkout.path, { recursive: true, force: true }).catch(() => {}); + await rm(checkout.gitDir, { recursive: true, force: true }).catch(() => {}); throw new WorktreeError( - `Could not create a worktree for this run: ${error instanceof Error ? error.message : String(error)}`, + `Could not create a checkout for this run: ${error instanceof Error ? error.message : String(error)}`, 'git_failed', ); } - if ((options.dependencies ?? 'linked') === 'linked') await linkDependencies(root, path); - return { path, baseSha }; + if ((options.dependencies ?? 'linked') === 'linked') { + await linkDependencies(top, checkout); + } + return { ...checkout, baseSha }; +} + +/** + * A clone at `path` of the repository whose top level is `top`, detached at + * `baseSha`, whose refs are its own. + * + * Cloned from the top level because `git clone` takes nothing else, while a + * connected folder may be any directory of a working tree; a run then works in + * the folder's place in the clone (`workDir`). + * + * `--mirror` copies every ref as the folder names it, so `main`, `origin/main` + * and a tag read in the clone as they read in the folder when it was made — a + * review's range and a check's base resolve to the same commits. `--shared` + * borrows the folder's objects through `objects/info/alternates` rather than + * copying them, so the clone costs its refs and its files; borrowing objects is + * reading, and nothing written in the clone reaches the folder's store. The + * mirror's remote is removed so no `git push` or `git fetch` in the clone names + * the folder by default. + * + * Mirrored as a bare repository into the checkout's git directory, beside the + * checkout rather than in it, and then given a working tree whose `.git` is a + * gitfile naming that directory — what `clone --separate-git-dir` makes, which + * git refuses alongside `--mirror` since a mirror is bare. `--force` checks out + * over the index the bare clone never had. + */ +async function clonePrivately(top: string, checkout: Checkout, baseSha: string): Promise { + await git(top, ['clone', '--quiet', '--mirror', '--shared', '--', top, checkout.gitDir]); + await git(checkout.gitDir, ['config', 'core.bare', 'false']); + await git(checkout.gitDir, ['config', '--remove-section', 'remote.origin']); + await mkdir(checkout.path, { recursive: true }); + await writeFile(join(checkout.path, '.git'), `gitdir: ${checkout.gitDir}\n`, 'utf8'); + // At the top alone: the folder's place in the clone exists only once checked out. + await git(checkoutAt(checkout.path), ['checkout', '--quiet', '--force', '--detach', baseSha]); + // A folder that tracks nothing at this commit is not in what was checked out. + await mkdir(checkout.workDir, { recursive: true }); } export interface WorktreeChanges { @@ -865,7 +1076,12 @@ export const RUN_SCRATCH_DIR = '.aflow'; * that adds a file has changed the tree, and a diff that silently omitted new * files would be the most misleading possible answer. */ -export async function collectChanges(worktreePath: string): Promise { +export async function collectChanges(checkout: Checkout): Promise { + // Only the connected folder's part of the clone is the run's work, and git + // runs at the clone's top, so every path below is anchored there. + const within = relativePath(checkout.path, checkout.workDir).split(sep).join('/'); + const anchor = within === '' ? '' : `${within}/`; + const scope = within === '' ? '.' : `:(top,literal)${within}`; // The mirrored installations are this lane's own doing, not the harness's // work, and git sees a directory of untracked symlinks that `add -A` would // stage. They cannot be removed instead: a session keeps its checkout across @@ -874,15 +1090,15 @@ export async function collectChanges(worktreePath: string): Promise `/${relative}\n`).join(''), + [...mirrors, `${RUN_SCRATCH_DIR}/`].map((relative) => `/${anchor}${relative}\n`).join(''), 'utf8', ); try { - await git(worktreePath, ['-c', `core.excludesFile=${excludes}`, 'add', '-A', '--', '.']); + await git(checkout, ['-c', `core.excludesFile=${excludes}`, 'add', '-A', '--', scope]); } catch (error) { throw new WorktreeError( `Could not read what the harness changed: ${error instanceof Error ? error.message.split('\n')[0] : 'git failed'}`, @@ -894,8 +1110,8 @@ export async function collectChanges(worktreePath: string): Promise line.length > 0).length; if (filesChanged === 0) { return { patch: '', filesChanged: 0, overCeiling: false }; @@ -907,7 +1123,7 @@ export async function collectChanges(worktreePath: string): Promise { - await unlinkDependencies(worktreePath); - try { - await git(root, ['worktree', 'remove', '--force', worktreePath]); - } catch { +export async function removeWorktree(worktreePath: string): Promise { + if (await isRealDirectory(worktreePath)) { + await unlinkDependencies(worktreePath); await rm(worktreePath, { recursive: true, force: true }).catch(() => {}); - await git(root, ['worktree', 'prune']).catch(() => {}); } -} - -export interface LinkedWorktree { - readonly path: string; - /** No branch is checked out in it, which is how every checkout this lane makes is added. */ - readonly detached: boolean; -} - -/** - * Every checkout added to the repository, without its main one. - * - * Read from git rather than from disk: a checkout whose directory is gone is - * still registered, and the registration is what the operator sees. - */ -export async function linkedWorktrees(root: string): Promise { - const listing = await git(root, ['worktree', 'list', '--porcelain'], APPLY_OUTPUT_CAP_BYTES); - return listing - .split(/\n\n+/) - .map((block) => block.split('\n')) - .filter((lines) => lines[0]?.startsWith('worktree ') === true) - .map((lines) => ({ - path: (lines[0] ?? '').slice('worktree '.length), - detached: lines.includes('detached'), - })) - .slice(1); + const gitDir = checkoutGitDir(worktreePath); + if (await isRealDirectory(gitDir)) { + await rm(gitDir, { recursive: true, force: true }).catch(() => {}); + } } /** @@ -984,7 +1181,7 @@ export type ApplyCheck = | { readonly state: 'empty' }; export async function checkApplies( - root: string, + root: string | Checkout, patch: string, against?: string, ): Promise { @@ -1029,59 +1226,6 @@ export async function currentHead(root: string): Promise { } } -/** Every ref the repository holds, by name, with the object it points at. */ -export type RefSnapshot = ReadonlyMap; - -/** - * The repository's local branches and tags, read before and after a run. - * - * Read from the connected folder, the listing holds the refs a worktree shares - * with it and not the run's own detached HEAD. A difference between two - * readings says a ref changed while the run was in flight, not who changed it: - * the operator's own work in the folder moves them too. What stops the agent's - * git is the hook in `refGuard.ts`; this is what the result reports. - */ -export async function snapshotRefs(root: string): Promise { - // A remote-tracking ref changes when a fetch runs — an editor's or a - // background one — and moves nothing the operator wrote; `refs/stash` is the - // operator's scratch. - const listing = await git( - root, - ['for-each-ref', '--format=%(refname) %(objectname)', 'refs/heads/', 'refs/tags/'], - APPLY_OUTPUT_CAP_BYTES, - ); - const refs = new Map(); - for (const line of listing.split('\n')) { - const space = line.lastIndexOf(' '); - if (space <= 0) continue; - refs.set(line.slice(0, space), line.slice(space + 1)); - } - return refs; -} - -export type RefChange = - | { readonly ref: string; readonly change: 'created'; readonly to: string } - | { readonly ref: string; readonly change: 'deleted'; readonly from: string } - | { - readonly ref: string; - readonly change: 'moved'; - readonly from: string; - readonly to: string; - }; - -export function changedRefs(before: RefSnapshot, after: RefSnapshot): RefChange[] { - const changes: RefChange[] = []; - for (const [ref, was] of before) { - const now = after.get(ref); - if (now === undefined) changes.push({ ref, change: 'deleted', from: was }); - else if (now !== was) changes.push({ ref, change: 'moved', from: was, to: now }); - } - for (const [ref, now] of after) { - if (!before.has(ref)) changes.push({ ref, change: 'created', to: now }); - } - return changes.sort((a, b) => a.ref.localeCompare(b.ref)); -} - /** * Every path a diff will touch, taken from git rather than parsed by hand. * @@ -1159,7 +1303,7 @@ export interface ApplyOutcome { * files it could not settle are named rather than left to be discovered. */ export async function applyPatch( - root: string, + root: string | Checkout, patch: string, mode: 'clean' | 'merge', ): Promise { diff --git a/docs/dev/local-appliance.md b/docs/dev/local-appliance.md index f5ff0e1b..1227e681 100644 --- a/docs/dev/local-appliance.md +++ b/docs/dev/local-appliance.md @@ -419,10 +419,11 @@ names which are absent, because that list is the whole of the fix. A coding agent already installed on the machine — the operator's own, already signed in — can be handed a task against a connected repository. The run happens -in a **worktree of its own**, detached at the repository's current commit, so +in a **private clone of its own**, detached at the repository's current commit, so uncommitted work is untouched, two runs cannot collide, and the result is a diff -to review rather than an edit already made. Nothing is committed, pushed or -applied. +to review rather than an edit already made. The clone's branches and tags are its +own, so nothing the agent does with git moves the repository's. Nothing is +committed, pushed or applied. ```bash # What is installed here, and what is allowed to run: diff --git a/docs/plans/315-local-first-run-ergonomics.md b/docs/plans/315-local-first-run-ergonomics.md index 9b38c86d..32897027 100644 --- a/docs/plans/315-local-first-run-ergonomics.md +++ b/docs/plans/315-local-first-run-ergonomics.md @@ -50,7 +50,7 @@ The loop delivered by the local operating model works. Reaching it from a first **D17 — A restart of the host executor drains instead of killing.** The dev stack restarts the host executor with a signal whenever a source it imports changes, and the executor answered every signal by ending each process it had started and discarding each session's checkout at once, so a merge into the stack's checkout ended every commission, check and review in flight on the machine. Two streams of work share one machine, and the rule that kept them apart — merge only when no harness runs — was kept by a person. The drain is not what SIGTERM means. Every supervisor that stops the executor with SIGTERM or SIGINT sends SIGKILL after a short grace — the dev runner after ten seconds, `tsx watch` after five, launchd after its default — and SIGKILL skips the exit handler and reaches no process in a group of its own, so a drain begun on SIGTERM would leave a harness running unaddressed, holding its provider credential, with its checkout on disk. SIGTERM and SIGINT therefore keep their meaning in full: end every workload, discard every checkout, exit within the supervisor's grace. The drain is SIGUSR2, which only a supervisor that has promised to wait for the exit without a kill sends, and only the dev stack's watcher sends it. On SIGUSR2 the executor stops reading its job stream and lets what it has claimed finish: every in-flight step keeps its heartbeat, the executor keeps its own, and every child process keeps running; it shuts down once the last step has ended. Jobs that a read already under way returns after the signal are run, and the drain waits for them: they were delivered to this consumer, the reclaim skips a consumer's own name, and the host executor's name is the machine's, so left pending they would outlive every restart with nothing to take them back. The executor listens for every signal before either runtime starts, so none lands in a window where it means the default action — which would skip the exit handler and leave a claimed harness's process group running. A step still waiting for its operation's slot is not in flight: the drain gives it back to the job stream unworked, for the next executor, so no coding agent starts here once claiming has stopped and the drain never waits on a queue. The step is already `STARTED`, so the executor keeps its in-flight record alive until it stops, and a restart inside that record's lifetime loses nothing. The drain has a deadline, and it is the started steps' own: the latest instant at which a step's timeout ends it, read again when it falls due, because a step that starts later brings its own — the executor's default stands in from the moment it starts until its own is resolved — and a progress-aware timeout slides its own. A drain that begins with nothing in flight while a read is still under way takes its deadline from the first step that read hands it. Nothing here chooses a length. Past the deadline, on a second SIGUSR2, or on a SIGTERM or SIGINT during the drain, each the word to stop now, the executor does what a signal did before: ends every process it started, discards every session's checkout, and shuts down. What the drain holds open is a harness run, a check and a review; it does not hold a session between its turns, whose checkout the exit discards with every other kept session's, so a merge between the turns of a continued commission still ends it (F74, deferred). The browser runtime stops as the drain begins, so its pages are not held. One line is logged as the drain begins, naming each step in flight and the deadline, and one as it ends, saying how: `finished`, `deadline` or `stopped`. The other executors keep their behaviour, because their steps are short. `tsx watch` does not wait for a drain: it kills five seconds after its signal. So the dev stack runs the host executor under a watcher of its own, which sends SIGUSR2 once per restart, starts the executor again when the drained process exits, and passes on its own SIGTERM or SIGINT as itself. Stopping the stack and `tsx watch` behave as they did before the drain. -**D18 — A commission is a skill, started without waiting, and the machine bounds how many run.** Review and publication are catalog skills and a commission was not: Helmsman called `host.harness.run` itself, a step its conversation waits on for the whole of the coding agent's run — thirty to forty minutes for a slice — so a stream that commissioned did nothing else until the change came back. A commission is therefore a skill, Commission Change, in a bundle of its own beside Local Code Review and Local Publish, started with `wait: 'none'` like any run that takes long: the start returns the run id, the conversation stays free, and the run's end wakes it with the result (D13). The skill is one operation task on `host.harness.run` whose run inputs are everything the operation takes that a brief names — the folder, the task text, `base`, `mergeFrom`, `continueFrom`, `harness`, `model`, `maxTurns`, `timeoutMs` — and its result promotes what the next step takes: `patchRef`, the primary, with `baseSha` and `merge`, which a publication takes as they stand, and `sessionRef`, which a further turn takes as `continueFrom`, beside `applies`, `filesChanged`, `continued` and `refChanges`. Its budgets are sized for the work, not guessed: the timeout defaults to the two hours a coding agent may run, because a slice runs thirty to forty minutes and the operation's half-hour default ended one, and the turn budget to several hundred, because every tool call is a turn and a cap sized to a guess ends a run while it is still reading. A run input's `default` is never applied, so the timeout's is made in the task's template. The turn budget's is not, because whether a run can carry a budget is the machine's to know: a harness whose profile has no turn argument — the built-in OpenCode profile has none — refuses any `maxTurns`, so a template that always sent one failed every commission on such a machine and left a brief no way to leave the budget out. The template sends `maxTurns` only when the run names it, and `host.harness.run` gives a run that names none its own default, several hundred turns, on a harness that takes a budget, and no budget on one that does not. `harness` is a run input because a machine offering more than one coding agent refuses a run that names none. The skill runs any number at once and queues none, because each commission has its own checkout. What bounds them is the machine, which runs a coding agent per commission and has only so many cores and so much memory; that limit belongs to the host executor, not to the skill. **D18b — the machine's limit.** The machine's policy holds `maxConcurrentHarnessRuns`, written only when the operator chooses a number (`aflow harness concurrency `, `--clear` for the default), and two otherwise: the machine is usually a laptop that also runs the stack, and two coding agents beside it, each running the project's checks in its own checkout, is what one carries without the stack slowing to a crawl. The host executor admits `host.harness.run` through the executor runtime's concurrency limiter with that number, for that operation alone, and follows a change of the policy without a restart. A run past the limit waits, claimed, for a slot and is never refused; its timeout counts from when it starts. The number is published in the machine's inventory and shown by `host.binding.inspect` beside the folder's push posture and checks. The checks a brief asks for are the folder's own — its tests, type check, linter and formatter, run by the coding agent in its checkout — and the folder's declared checks run again when the patch is published (D16). **D18b's completion** (#62's minor): a run waiting for its slot is scheduled, not started. A step is marked `STARTED`, with `startedAt` and its `StepStarted` event, once it is admitted, so its duration and its timeout count from then and `queueWaitMs` includes the wait; and the stall watchdog reads the live in-flight record of a `SCHEDULED` step as it does a `STARTED` one's, so an executor holding a queued step, or one a drain gave back (D17), vouches for it. +**D18 — A commission is a skill, started without waiting, and the machine bounds how many run.** Review and publication are catalog skills and a commission was not: Helmsman called `host.harness.run` itself, a step its conversation waits on for the whole of the coding agent's run — thirty to forty minutes for a slice — so a stream that commissioned did nothing else until the change came back. A commission is therefore a skill, Commission Change, in a bundle of its own beside Local Code Review and Local Publish, started with `wait: 'none'` like any run that takes long: the start returns the run id, the conversation stays free, and the run's end wakes it with the result (D13). The skill is one operation task on `host.harness.run` whose run inputs are everything the operation takes that a brief names — the folder, the task text, `base`, `mergeFrom`, `continueFrom`, `harness`, `model`, `maxTurns`, `timeoutMs` — and its result promotes what the next step takes: `patchRef`, the primary, with `baseSha` and `merge`, which a publication takes as they stand, and `sessionRef`, which a further turn takes as `continueFrom`, beside `applies`, `filesChanged` and `continued`. Its budgets are sized for the work, not guessed: the timeout defaults to the two hours a coding agent may run, because a slice runs thirty to forty minutes and the operation's half-hour default ended one, and the turn budget to several hundred, because every tool call is a turn and a cap sized to a guess ends a run while it is still reading. A run input's `default` is never applied, so the timeout's is made in the task's template. The turn budget's is not, because whether a run can carry a budget is the machine's to know: a harness whose profile has no turn argument — the built-in OpenCode profile has none — refuses any `maxTurns`, so a template that always sent one failed every commission on such a machine and left a brief no way to leave the budget out. The template sends `maxTurns` only when the run names it, and `host.harness.run` gives a run that names none its own default, several hundred turns, on a harness that takes a budget, and no budget on one that does not. `harness` is a run input because a machine offering more than one coding agent refuses a run that names none. The skill runs any number at once and queues none, because each commission has its own checkout. What bounds them is the machine, which runs a coding agent per commission and has only so many cores and so much memory; that limit belongs to the host executor, not to the skill. **D18b — the machine's limit.** The machine's policy holds `maxConcurrentHarnessRuns`, written only when the operator chooses a number (`aflow harness concurrency `, `--clear` for the default), and two otherwise: the machine is usually a laptop that also runs the stack, and two coding agents beside it, each running the project's checks in its own checkout, is what one carries without the stack slowing to a crawl. The host executor admits `host.harness.run` through the executor runtime's concurrency limiter with that number, for that operation alone, and follows a change of the policy without a restart. A run past the limit waits, claimed, for a slot and is never refused; its timeout counts from when it starts. The number is published in the machine's inventory and shown by `host.binding.inspect` beside the folder's push posture and checks. The checks a brief asks for are the folder's own — its tests, type check, linter and formatter, run by the coding agent in its checkout — and the folder's declared checks run again when the patch is published (D16). **D18b's completion** (#62's minor): a run waiting for its slot is scheduled, not started. A step is marked `STARTED`, with `startedAt` and its `StepStarted` event, once it is admitted, so its duration and its timeout count from then and `queueWaitMs` includes the wait; and the stall watchdog reads the live in-flight record of a `SCHEDULED` step as it does a `STARTED` one's, so an executor holding a queued step, or one a drain gave back (D17), vouches for it. **D19 — The sandbox posture is what the network is, not whether there is a sandbox.** The coding agent and the folder's checks ran under the sandbox a command runs in, and most of the executor's findings in the loop were that sandbox meeting ordinary work: a test that serves itself on loopback (F82), a dependency the agent could not add (F83), a shell whose every command read as failed (F84), a `yarn` Corepack could not fetch (F85), an agent configuration file the agent could not write (F86). Most of it was the network and one temporary file. Taking the sandbox away instead, as this decision first did, also took away what the gates stand on. The ref guard, the scan, the check, the review and the push gate each read their configuration from the machine's host directory (`~/.aflow/`, `hostDir.ts`): push approval, the checks and the posture itself are in `host-policy.json`, and pairing's credential is in `host.env`. An unconfined agent could rewrite that policy, read the credential, write the operator's working tree and its `.git/hooks`, and move a branch by writing `.git/refs` past the ref guard's hook. Gates whose configuration the agent can rewrite protect nothing. So the connected folder's policy gains `sandbox`, `open` or `confined`, set with `aflow harness sandbox open|confined` beside `push-approval` and `checks`. The policy file holds it only when the operator chose it. `host.binding.inspect` shows it, and so does the machine's inventory beside each folder's push posture. A folder that chose none takes `HOST_SANDBOX_POSTURE_DEFAULT` each time it is read. In the local edition that is `open`, with its reason written where it is defined. The posture is held to two reference harnesses, Claude Code and Codex CLI: `open` is no stricter than the one, and `confined` no looser than the other. `open` is the sandbox the operator's own coding tool runs under with every domain allowed — Claude Code's sandbox mode with every domain allowed: the network on, the machine's loopback closed. `confined` is the workspace-write default of the other — Codex CLI's `workspace-write`: the network off. Loopback to the machine is never a job's under either. `host.harness.run` and `host.commit.check` run inside the sandbox under both postures, under one compiled policy, and the posture decides which launcher reads it: under `confined` the adapter's own command line, which refuses every host the policy does not name; under `open` a launcher that admits every host but this machine, so the agent's own search and fetch work and Corepack and the registry are reachable. Nobody enumerates loopback ports. A list of the stack's own services was tried first (F98), and no such list can be complete: the stack's MCP server on 3100 hands the owner's API key to any local session that brings no credential, and `yarn infra:tools` publishes Redis Commander and pgAdmin without a login, so every port the list did not name was a door. Under both postures the job's loopback is the sandbox's: on Linux the network namespace's own, where it binds, accepts and connects to its own listeners and the machine's are not there; on macOS none, because the profile's only loopback grant opens every localhost port, so the policy never sets `allowLocalBinding` and a test that serves itself cannot listen there. The `open` launcher's proxy dials from outside the sandbox, so it refuses, on every port, a host that resolves to loopback, the unspecified address or any address of the machine's interfaces, and one that does not resolve. Neither posture opens anything shared: the system's `/tmp` is writable under neither, because on Linux it is where every job's scratch lives — its checkout, its check worktree, the sandbox settings and status files — and a job able to write it could write into a concurrent job's checkout or a `confined` folder's job. Each job is handed a temporary directory of its own inside its own scratch, as `TMPDIR`, `TMP`, `TEMP` and `CLAUDE_CODE_TMPDIR` (Claude Code writes its shell's working-directory file where that one says, and in `/tmp` otherwise, whatever `TMPDIR` says), and writes it under both. Everything else is the same under both. The job writes its checkout and the run's scratch, where its temporary directory is, and nothing else of the system's `/tmp`. The operator's folder is readable, so the checkout can mirror its installation, but it is not writable, and neither is its `.git`. The host directory can be neither read nor written, whatever a harness profile or the machine's tool paths name, and the machine's loopback cannot be reached, so the gates' state is reachable only by the executor. A test runs a command under each posture that tries each of those writes and the credential read, and one under each that reaches for a listener the test holds on the machine's loopback — directly and through the proxy, by address and by name — and is refused, has a loopback server of its own on Linux and none on macOS, and reaches a host off the machine under `open` alone. A test that needs a listener of its own therefore cannot run under the check on macOS: it declares the Vitest tag `listener`, and the check leaves it out there, names it, counts it in the check's receipt and says CI runs it. A check's result and its receipt carry the posture it ran under, signed with the rest. The push gate requires it to equal the folder's posture as the push runs and refuses it otherwise as `check_other_posture`, so a receipt from a check run `open` clears no push once the operator has made the folder `confined`; it answers the posture from the receipt, so a push from a folder that declares checks records it as `checkedUnder`. `host.process.exec` stays `confined` under both postures, because it runs a command an agent wrote, not the operator's tool. The ref guard is still a hook the run's git is told about. A git call that names its own hooks, or a push into the folder, now meets a `.git` it cannot write, so neither moves a branch there; the guard's limits as a hook stay recorded under F56. The lane's boot self-test runs three probes under `open` on the machine: a shell command exits 0, `yarn --version` runs, and a listener the lane holds on the machine's loopback is out of reach. F84 and F85 are retired under `open` by that measurement. F82 is answered rather than retired: a test that serves itself runs under the check on Linux, and on macOS is left out by name and runs in CI. F83 and F86 are not retired: under `open` the registry is reachable, but the installation is still the folder's, which no commission writes, and the sandbox keeps its own protection of agent configuration files. @@ -132,7 +132,7 @@ After D13 lands: the chat renders a run's wakeup as a card that says which run s **Then the drain, D17 (F72)**: `createShutdownController` (`@aflow/lib`, its shutdown code now in `shutdown.ts`) takes `drain: { work, endInFlight }`, where `work` stops claiming, lists what is in flight as `{ name, deadlineAt }`, resolves `whenInFlight()` once something is and `idle()` when nothing is. `drainOnce()` stops claiming, logs `draining` with the steps and the deadline, and waits for `idle()`, a timer set to the latest deadline and re-read when it fires, or a stop; with nothing in flight the timer waits on `whenInFlight()` instead, so a step that a read under way returns after the drain began arms it from its own timeout; `endInFlight` runs on the last two, `drain ended` is logged with the outcome, and `onShutdown` runs as before. `shutdownOnce()` is the stop: outside a drain it runs `endInFlight` at once, logging `stopping now` with the steps it ended, then `onShutdown`; during one it ends the drain as `stopped`. A second `drainOnce()` is the same stop. `attachSignalHandlers` takes `onDrain`, bound to SIGUSR2 (`DRAIN_SIGNAL`), and hands every signal to its handler rather than only the first, so a later one turns a drain into a stop; the process still exits once, when the first one's shutdown settles, which is why `onShutdown` must be safe to call again — every executor passes `shutdownOnce`. `ExecutorRuntime` gives the drain its work: `inFlightMessageIds` became `inFlightSteps`, keyed by message id, each entry holding the step's operation, its step execution, and the `deadlineRef` that `processJob` already kept current for the stall watchdog. `stopClaiming()` ends the consume loop without touching the heartbeats, and a read that returns after it is run, as it was before the drain; `idle()` waits for the consume loop to end before it waits for the steps, so a drain cannot end while that read is under way. A stop or a `stopClaiming()` that arrives while `start()` awaits stands: the runtime starts without reading its stream, and `stop()` waits for the start before stopping. `inFlight()` names each started step as ` `, `whenInFlight()` settles as a step starts, and `idle()` settles as the last step leaves. The host executor's own `SIGINT` and `SIGTERM` listeners are gone; the ending they ran is the controller's `endInFlight`, which SIGTERM and SIGINT run at once through `shutdownOnce`, a drain runs when it is cut short, and `exit` runs regardless. The browser runtime is stopped as a drain begins. Tests: `packages/lib/src/__tests__/shutdownDrain.test.ts` covers SIGTERM and SIGINT with a step in flight ending it and exiting at once, a fake in-flight step holding the process past SIGUSR2 until it finishes, a SIGTERM or a second SIGUSR2 during the drain ending it and exiting at once, a service without a drain shutting down once however many signals arrive, the deadline as the latest step timeout and following one that slides, a drain begun with nothing in flight while a read is under way taking its deadline from the first step that read returns and exiting when it returns nothing, and the two log lines; `packages/executor-runtime/src/__tests__/drain.test.ts` covers a real runtime with a step that has a thirty-minute timeout, which reports that deadline, stops reading its stream when claiming stops, runs and acknowledges a late read's job before it goes idle, settles `whenInFlight()` when a late read hands it a step after the last one ended, reads nothing when claiming stops or `stop()` is called while it starts, and goes idle when the step ends. The host executor attaches its handlers through `startUnderSignals` (`apps/aflow-executor-host/src/startUnderSignals.ts`) before either runtime starts and starts nothing more once a signal has begun a shutdown; `startUnderSignals.test.ts` delivers SIGTERM, SIGINT and SIGUSR2 to its own process right after a start resolves, which would end the test process were they unhandled. Measured on `tsx` 4.23.15, the version this repository installs, by reading the watcher's own code: `node -e 'const s=require("fs").readFileSync("node_modules/tsx/dist/cli.mjs","utf8");const i=s.indexOf("\"killProcess\"");console.log(s.slice(i-260,i+14))'` prints the minified `killProcess`, `async(m,S="SIGTERM",N=5e3)=>{…m.kill(S),setTimeout(()=>{L||(…m.kill("SIGKILL"))},N)…}`, which a restart calls with its defaults: it sends SIGTERM and, if the child has not exited 5 s later, logs "Process didn't exit in 5s. Force killing..." and sends `SIGKILL`. A change that arrives during those 5 s kills at once ("Process hasn't exited. Killing process..."). The watcher's own SIGINT or SIGTERM is relayed the same way, and no flag sets the grace. It was read rather than timed because the commission starts no process. `tsx watch` therefore kills, so `scripts/dev.mjs` runs `executor-host` as `node scripts/watch-and-drain.mjs apps/aflow-executor-host/src/index.ts apps/aflow-executor-host packages`. The wrapper runs the entry under `node --conditions=ts-source --import tsx` and watches the `.ts` files under the `src` of those trees, which is what `tsx watch --include 'packages/*/src/**/*.ts'` covered. A burst of writes counts as one change after the 100 ms `tsx watch` itself waits, and the wrapper sends one SIGUSR2 per restart, never a second. It restarts the executor when that process exits and passes on its own signals, a SIGINT included whether or not a terminal has delivered it too: the wrapper cannot tell, a `kill -INT` to it reaches it alone, and a second SIGINT only repeats a stop the executor has begun. `yarn executor:host` keeps `tsx watch` for a foreground run. Stopping the stack is the runner's own grace, ten seconds, as before: the runner sends SIGTERM, the wrapper passes it on as SIGTERM, and nothing drains on that path. `dev.mjs` has no tests, so the wrapper's choice of signal is `DRAIN_SIGNAL` in `scripts/drainSignal.mjs`, with its reason. Both it and `@aflow/lib`'s `DRAIN_SIGNAL` read `packages/lib/src/drainSignal.json`, because the wrapper runs under plain Node and cannot import the lib's source. `scripts/drainSignal.test.mjs` covers `signalToSend`, the function that picks it: the executor's drain signal for a restart, and the wrapper's own SIGTERM and SIGINT each passed on as itself. -**Then the commission skill, D18a**: Commission Change (`commission-change`, `packages/platform-artifacts/src/skillCatalog/commissionChange.ts`) ships in the Local Commission bundle (`local-commission`), beside Local Code Review and Local Publish. It is one operation task on `host.harness.run` with no `outputSchema`: what the coding agent reports is the brief's to ask for, and the change is the operation's own `patchRef`. Its run inputs are `bindingId` and `task`, required, and `base`, `mergeFrom`, `continueFrom`, `harness`, `model`, `maxTurns` and `timeoutMs`, each bound to the operation as it stands. `timeoutMs` reaches it through `$firstOf` with the skill's default, `HOST_HARNESS_TIMEOUT_MAX_MS`, two hours, because a run input's schema `default` is documentation that nothing applies; without the template's fallback an absent `timeoutMs` would be the operation's own `HOST_HARNESS_TIMEOUT_DEFAULT_MS`, the half hour that ended a slice. `maxTurns` is a plain `$bind`, so a brief that names none sends none, and its run input carries no `default`. The default belongs to the operation: `effectiveMaxTurns` in `harnessHandlers.ts` gives a run that names no budget `HOST_HARNESS_MAX_TURNS_DEFAULT` (600) where the profile has `turnsArgs` and none where it has not, and a budget a run does name on a harness without `turnsArgs` is still refused by name, as before. The `maxTurns` description in `host.ts` says so. The operation's limits are named in `host.ts` — `HOST_HARNESS_TASK_MAX_LENGTH`, `HOST_HARNESS_TIMEOUT_MIN_MS`, `HOST_HARNESS_TIMEOUT_DEFAULT_MS`, `HOST_HARNESS_TIMEOUT_MAX_MS`, which `HOST_CHECKS_TIMEOUT_MAX_MS` now is, `HOST_HARNESS_MAX_TURNS_DEFAULT` and `HOST_HARNESS_CONCURRENCY_DEFAULT` — and the skill reads them. The run promotes `patchRef` (the primary), `baseSha`, `merge`, `applies`, `filesChanged`, `sessionRef`, `continued` and `refChanges`, and its guidance says a publication takes `patchRef`, `baseSha` and `merge.from` as they stand and a further turn takes `sessionRef`. Its concurrency is unlimited runs of one task each. `commissionChange.test.ts` holds the contract valid, the run inputs and their bindings, the timeout the template makes when the brief names none and the `maxTurns` it leaves out, `harness` bound through, the values it passes through when the brief names them, each parsed by `HostHarnessRunInputSchema`, the promoted outputs and the guidance, the description's claims — the machine's number and a commission past it waiting — and the concurrency. `harnessTurnBudget.test.ts` holds the operation's default per profile: the default on a profile with `turnsArgs`, a run's own budget wherever it names one, no budget and no refusal on a profile without `turnsArgs` shaped like the built-in OpenCode one, and a named budget there still refused. The skill and its bundle are at version 2. +**Then the commission skill, D18a**: Commission Change (`commission-change`, `packages/platform-artifacts/src/skillCatalog/commissionChange.ts`) ships in the Local Commission bundle (`local-commission`), beside Local Code Review and Local Publish. It is one operation task on `host.harness.run` with no `outputSchema`: what the coding agent reports is the brief's to ask for, and the change is the operation's own `patchRef`. Its run inputs are `bindingId` and `task`, required, and `base`, `mergeFrom`, `continueFrom`, `harness`, `model`, `maxTurns` and `timeoutMs`, each bound to the operation as it stands. `timeoutMs` reaches it through `$firstOf` with the skill's default, `HOST_HARNESS_TIMEOUT_MAX_MS`, two hours, because a run input's schema `default` is documentation that nothing applies; without the template's fallback an absent `timeoutMs` would be the operation's own `HOST_HARNESS_TIMEOUT_DEFAULT_MS`, the half hour that ended a slice. `maxTurns` is a plain `$bind`, so a brief that names none sends none, and its run input carries no `default`. The default belongs to the operation: `effectiveMaxTurns` in `harnessHandlers.ts` gives a run that names no budget `HOST_HARNESS_MAX_TURNS_DEFAULT` (600) where the profile has `turnsArgs` and none where it has not, and a budget a run does name on a harness without `turnsArgs` is still refused by name, as before. The `maxTurns` description in `host.ts` says so. The operation's limits are named in `host.ts` — `HOST_HARNESS_TASK_MAX_LENGTH`, `HOST_HARNESS_TIMEOUT_MIN_MS`, `HOST_HARNESS_TIMEOUT_DEFAULT_MS`, `HOST_HARNESS_TIMEOUT_MAX_MS`, which `HOST_CHECKS_TIMEOUT_MAX_MS` now is, `HOST_HARNESS_MAX_TURNS_DEFAULT` and `HOST_HARNESS_CONCURRENCY_DEFAULT` — and the skill reads them. The run promotes `patchRef` (the primary), `baseSha`, `merge`, `applies`, `filesChanged`, `sessionRef` and `continued`, and its guidance says a publication takes `patchRef`, `baseSha` and `merge.from` as they stand and a further turn takes `sessionRef`. Its concurrency is unlimited runs of one task each. `commissionChange.test.ts` holds the contract valid, the run inputs and their bindings, the timeout the template makes when the brief names none and the `maxTurns` it leaves out, `harness` bound through, the values it passes through when the brief names them, each parsed by `HostHarnessRunInputSchema`, the promoted outputs and the guidance, the description's claims — the machine's number and a commission past it waiting — and the concurrency. `harnessTurnBudget.test.ts` holds the operation's default per profile: the default on a profile with `turnsArgs`, a run's own budget wherever it names one, no budget and no refusal on a profile without `turnsArgs` shaped like the built-in OpenCode one, and a named budget there still refused. The skill and its bundle are at version 2. **Then the machine's limit, D18b**: `HostPolicySchema` takes an optional `maxConcurrentHarnessRuns`, a whole number of at least one; `loadHostPolicy` reads it with `HOST_HARNESS_CONCURRENCY_DEFAULT` (`@aflow/schemas`, two, its reason written beside it) where the file holds none. `aflow harness concurrency ` writes it and `--clear` removes it, through `withHarnessConcurrency` in `harnessConcurrency.ts`, which refuses anything but a whole number of at least one, naming what it was given; `harness list` prints the number and whether it is the default. The executor runtime's `ConcurrencyLimiter` gained `setLimit`, which admits waiters at once on a raise and, on a cut, ends nothing and admits no one until enough has ended, and `release` respects a cut limit. `ExecutorRuntime.limitOperation(operationId, n)` creates or adjusts a limiter for one operation, held in `operationLimiters`, and `processJob` admits a step of such an operation through `admitOperation` (`operationAdmission.ts`) after its `StepStarted` record and in-flight registration and before its context is built. A step past the limit gives back its executor-wide slot while it waits, so operations without a limit of their own are not held up, refreshes its in-flight record with no deadline every heartbeat, so neither the session stall watchdog (it is `STARTED`, its executor alive) nor the operation-task sweeper (its in-flight record alive) reaps it, and takes its slot back once admitted; its timeout starts then. That refresh is a second site of `executor.step_inflight_refresh` in the background-work registry. The host executor calls `limitOperation('host.harness.run', …)` with the policy's number before it starts and again on every policy change, so a new number takes effect without a restart. The inventory carries `maxConcurrentHarnessRuns`, required: an executor from before the limit enforces none, so its inventory is refused rather than read as running the default, and `host.binding.inspect` returns it beside the folder's branch policy. Tests: `packages/executor-runtime/src/__tests__/operationConcurrency.test.ts` runs a real runtime with a fake step that blocks until released and four harness jobs and one file read claimed at once, under a limit of two: two run, the other two wait claimed with nothing refused or acknowledged, the file read runs past them, ending one admits the next, the peak never passes two, a raise admits a waiter at once, a cut ends nothing and admits no one until the runs are under it, and a waiting step's in-flight record is refreshed with no deadline. `harnessConcurrency.test.ts` holds the number written only when chosen, read back with the default otherwise, forgotten on `--clear`, refused when it is not a whole number of at least one, described in one line, and shown by `host.binding.inspect` both ways; `hostInventory.test.ts` carries it through and refuses an inventory without it. Review then found a queued run still started its coding agent once admitted after a cancel, and a drain waiting on every queued run. The step's abort controller is now registered before the wait and `ConcurrencyLimiter.acquire` takes a signal, so a cancel ends the wait and gives up the place; a step that waited runs the stale-attempt fence and the cancellation record again once admitted, session steps included, and is dropped without a result when either says so. `stopClaiming()` ends every wait still under way, and such a step is handed back by `releaseStepJob` (`@aflow/redis`), which re-adds the job and acknowledges the claimed message in one script that acknowledges nothing when the re-add fails and removes the re-added entry when the acknowledgement fails, since a message left in this consumer's pending list would wait on its heartbeat lapsing; the in-flight record of a step handed back is written just before and then only extended (`extendStepInFlight`, an EXPIRE that writes nothing and recreates nothing) until the runtime stops, because on a machine whose one executor is draining nothing claims the step until that executor exits, and an extension never overwrites the deadline the executor that picks the step up records; a step that could not be handed back is refreshed until the runtime stops; both are a second site of `executor.step_inflight_refresh`. `inFlight()` and `whenInFlight()` count only steps that have started. Tests: `operationConcurrency.test.ts` covers a cancel ending a wait, a recorded cancel, a failed and a superseded session step found once admitted, and only started runs named in flight; `givenBackStep.test.ts` the job handed back with its record written before and only extended after, until the runtime stops, and one that could not be handed back refreshed until the runtime stops; `releaseStepJob.test.ts` the script, a failed re-add leaving the job pending and a failed acknowledgement adding nothing; `apps/aflow-executor-host/src/__tests__/drainQueuedHarnessRuns.test.ts` a drain with two running and three queued runs that ends when the two finish, the three run by the next consumer, and no coding agent started after the drain began. @@ -162,6 +162,10 @@ After D13 lands: the chat renders a run's wakeup as a card that says which run s **Then F118 — the start rebuilds the packages the web application reads**: `scripts/dev.mjs`, which `yarn start` and `yarn dev:local` reach through the `local` profile, no longer runs the advisory staleness check that called a stale `dist` safe to ignore. Before any service starts it takes the Next applications among the services it is about to run (a workspace whose `dependencies` name `next`, found through the service's command as `whyUnavailable` finds it) and their dist closure — every workspace package their `dependencies`, `peerDependencies` and `optionalDependencies` reach, transitively, each after what it depends on — from the manifests alone (`scripts/webDists.mjs`); no list of packages is written anywhere. A package in it is stale where a file the `import` condition names is missing or older than the newest file of its `src/`, tests and `.tsbuildinfo` aside; measuring the files the bundler loads rather than all of `dist/` is deliberate, since a type-check writes declarations there without touching the JavaScript and would make a stale build look current. Each stale package is built with `yarn workspace build`, dependencies first, under one line that names them and says which application reads them from `dist/` where every other service reads `src/`, then one line per package with what made it stale and how long it took; a package already current is not built and a start with nothing changed prints nothing. A package the start runs a watcher for — `@aflow/web-product` — is named as stale and left to that watcher's first build. A build that fails does not stop the start, since the Node services read source: its output is printed with what it costs the web app and the command that rebuilds it. A profile with no web application — `core`, `engine`, `mcp` — selects nothing. `yarn check:packages` (`check-package-staleness.mjs`) shares the rule and now names each stale package and which consumer reads `dist/`. Tests: `scripts/webDists.test.mjs` measures the repository's own manifests with injected times — a touched `packages/schemas/src` file selects `@aflow/schemas` alone, a touched orchestrator file selects nothing, a current tree selects nothing, a watched package is left to its watcher, a fresh clone selects the whole closure dependencies first — and two packages on disk, one whose declarations a type-check rewrote after its source changed (stale) and one whose only newer file is a test (current). Not taken: the dev health check that requests the web app's first page, which would add a probe and its failure path to the runner; it stays open on this row, as F58's watch of the imported packages during a session does. +**Then F56 — the private clone is the boundary**: every checkout the host lane makes — a commission's, a check's, a publication's — is a private clone of the folder rather than a worktree added to it. `prepareWorktree` (`worktree.ts`) clones the folder's repository from its top level (`rev-parse --show-toplevel`, since `git clone` takes nothing else) with `--mirror --shared` into a git directory of its own beside the checkout (`git-` in the run's scratch, `checkoutGitDir`), which the sandbox reads and never writes, gives it a working tree whose `.git` is only a gitfile naming that directory — the executor's own git names the directory and the working tree outright (`Checkout`) and never resolves the checkout through what it holds — removes its remote and checks out the base commit detached: the clone reads every ref as the folder names it when it is made, so `origin/main`, a branch and a tag resolve as they do there, and borrows the folder's objects through `objects/info/alternates`, while its refs and config are its own. A `git update-ref`, a `git branch -D`, a call that names its own `core.hooksPath` or a push to `.` in the run moves the clone's refs and nobody else's, and a push to the folder's path meets a `.git` the sandbox does not let it write (D19). So the guard and the record are gone rather than kept beside it: `refGuard.ts` and its `reference-transaction` hook, with the git 2.28 floor a commission was refused under, and `snapshotRefs`/`changedRefs` with the result's `refChanges`, which leaves `HostHarnessRunOutputSchema`, `host.harness.run`'s description and Commission Change's promoted outputs. The publication reads its commit from its clone: `commitPatchOnBranch` (`branchCommit.ts`) applies and commits there, then the executor's git fetches that commit into the folder by sha (`--no-write-fetch-head`, so no FETCH_HEAD another fetch is reading is overwritten; git 2.29 or later) and creates the named branch, or advances it by compare-and-swap on the head it was made on — the one write to the folder's refs the loop makes. A commit's identity is read from the folder's config, since the clone's holds none; a commission's `applies` is judged against a merge commit in the clone, where it exists. `removeWorktree` takes the clone's path and removes the directory, which is the whole of it, and `HarnessSession` no longer carries `bindingRoot`. The boot sweep (`removeOrphanedCheckouts`) takes a scratch directory named as this executor names its own and carrying the marker `makeScratch` (`executorScratch.ts`) writes into every scratch it makes — a commission's, a check's, a publication's and an ephemeral browser profile's — and takes it whole, whatever its checkouts are: a clone killed before its detached checkout, whose HEAD still names a branch, goes with it. The shape of a HEAD decides nothing; a directory without the marker is left whatever it holds; a crashed run's journal entry (`orphans.ts`) removes its scratch, clone and all, with nothing left registered in the folder — for a commission and a check, which hand the sandbox only `agent/` inside it, the journal records the scratch holding the clone (`journalScratchDir`, passed to `recordSpawn` apart from the sandbox's `scratchDir`, `sandboxedRun.ts`), while a run that ends removes `agent/` alone and its caller the rest. Tests: a run whose agent deletes a branch, moves `main` and tags under its own hooks path, pushes to `.` and pushes and deletes into the folder's path leaves every ref of the folder as it was, is handed no `GIT_CONFIG_*` and names no remote, and its publication lands exactly its commit on the named branch with no other ref moved (`harnessResult.test.ts`, sandboxed); the clone's refs, alternates and detached HEAD, every move made inside it reaching only it, and a folder commit made after the clone read through what it borrows (`worktree.test.ts`); the publication's fetch writing the named branch and no other ref and no FETCH_HEAD (`patchApply.test.ts`); a clone removed when its run ends and swept when it is orphaned, by the boot sweep and by the journal, with the operator's own checkouts left (`worktree.test.ts`, `phase2Acceptance.test.ts`). Inside a marked scratch only a real directory is treated as a checkout, read with `lstat`: a link planted there, followed, led the removal's unlinking of mirrored installations into the `node_modules` of whatever the link pointed at, the operator's own project included. A link is now removed with the scratch and nothing it leads to is walked, and `removeWorktree` acts on its checkout and git directory only as real directories. The sweep handles marked scratch only and reads no connected folder, so the detached worktrees the executor before the private clone registered in an operator's repository, in scratch it never marked, are the operator's to prune once by hand (`git worktree remove --force` on each that `git worktree list` shows under the temp root). A folder connected as a subdirectory of a repository — a package of a monorepo, which `rev-parse --is-inside-work-tree` accepts — is cloned from that top level, and the checkout's `workDir` is the folder's path relative to it inside the clone: the harness and a check run there with it as their writable root, the installations of the repository are linked at the same place in the clone — the union of those the scan from the top level reaches, exactly as for a folder that is the top level, so a sibling package's own installation is there too, and those on the walk from the folder up to the top level, its own included, which the scan's depth (`DEPENDENCY_SCAN_DEPTH`) cannot cut short however deep the folder lies (`repositoryInstallations`) — and a workspace link resolving anywhere in the repository — a sibling package included — points at that path in the clone, so resolution in the clone sees what it sees in the folder, and `collectChanges` stages and diffs that directory alone, as paths from the top level, which is what `git apply` reads in the folder and what the publication applies at the clone's top. A session's checkout is found again from its binding's prefix (`folderPrefix`). Inside the run, a clone reads outside the folder wherever what it borrows lies outside it: it owns no object and borrows every one from the repository's store (`rev-parse --git-path objects`, resolved), which is outside the folder for a subdirectory and equally for a top level that is a linked worktree, a submodule or a repository with a separate git directory, and its mirrored installations outside the folder lead into `/node_modules` and the like, all under a home the sandbox denies. So the sandbox is handed exactly those that lie outside the folder, decided by location and never by the folder's prefix, read and never written, for a commission and a check alike (`borrowedReads`, `worktree.ts`, read from the folder rather than the checkout the run writes), and `checkoutWidening` (`sandboxPolicy.ts`) states the whole boundary of a run in a checkout in one place — what it reads and writes for a folder holding its store and installations and for any other — which a test holds the compiled policy to, path for path (`sandboxPolicy.test.ts`). Test: a subdirectory of a repository under a denied home, connected, runs `git log -1` and resolves a hoisted package inside a folder check's sandbox, and reaches neither without the borrowed reads; a linked worktree under a denied home, connected, runs `git log -1` there (`subdirectoryBoundary.test.ts`, sandboxed). Tests: a sweep of a marked scratch holding a half-made clone removes it with no ref or registration of the folder touched, and unmarked directories named like scratch are left; a marked scratch holding links to a project with a linked `node_modules`, as a checkout and as a git directory, is swept with every link in that project left; a folder connected inside a repository gets a checkout whose change is the folder's alone and applies to it (`worktree.test.ts`); a package connected as the folder gets the top level's installation and its own linked, a workspace package resolving into the clone, and a sibling package's own dependency resolving inside the clone, and a folder three levels down resolving its own dependency inside the clone (`worktreeDependencies.test.ts`); `borrowedReads` reads the same set, a folder below the scan's depth borrowing each installation on its way up (`subdirectoryBoundary.test.ts`); a sandboxed run handed `agent/` and killed with the executor is journalled with the scratch holding its clone, which the reap removes clone and all (`worktree.test.ts`, sandboxed), and a commission hands the sandbox that scratch as `journalScratchDir` (`sandboxPosture.test.ts`). The directory holding a checkout and its git directory is never the sandbox's to write. A commission and a check are handed `agent/` inside their scratch (`makeAgentScratch`, `executorScratch.ts`) — their home, temporary directory, harness configuration, browser pipes and check report — with the folder's place in the checkout as their writable root, so a job cannot remove its checkout and put a link to a directory of the operator's in its place. And because git, told `--work-tree`, takes a link there as readily as a directory, every git the executor runs on a checkout first finds the checkout and the folder's place in it real directories whose resolved path is the one `prepareWorktree` recorded, resolved, and refuses otherwise (`assertCheckoutInPlace`, in `gitLocation`) — staging (`collectChanges`), the base merge (`mergeIntoCheckout`) and `checkApplies` among them. Tests: a sandboxed job's attempt to remove or move its checkout or git directory and put a link in its place is refused (sandboxed), and a link put from outside in place of the checkout, or of the folder's place in it, is refused by collection and by the merge with the directory it leads to untouched (`checkoutReplacement.test.ts`); a commission is handed `agent/` beside its checkout (`sandboxPosture.test.ts`). + +**Then F56's git directory — out of the sandbox's reach**: a clone whose git directory was `/.git` sat inside the root the sandbox writes, where the policy closed only `config` and `hooks`. An agent could write `.git/commondir` naming a repository it built in its scratch with a `filter..clean` defined, add a `.gitattributes` selecting it, and the executor's own unconfined git — `collectChanges`' `git add -A` and `checkApplies` — read that config and ran its program with the executor's PATH and HOME. So the clone's git directory lives beside the checkout in the run's scratch (`checkoutGitDir`: `/git-`), mirrored there and given a working tree whose `.git` is only a gitfile naming it — what `clone --separate-git-dir` makes, which git refuses beside `--mirror`. The scratch around it is writable, so the sandbox is handed it by name (`SandboxWidening.gitDir`) and reads it without writing it, for a commission and for a check. Every git the executor runs in a checkout names that directory itself — `--git-dir` and `--work-tree`, through a `Checkout` that `git()` takes beside a folder path — and never resolves through the gitfile, which the agent may rewrite and which then moves only the agent's own git; the publication fetches its commit from the git directory, not the checkout. The agent's git therefore moves no ref at all, the clone's included. `removeWorktree` takes the git directory with the checkout, and the boot sweep counts a checkout as a directory holding a `.git` or a git directory named for one. A publication is refused below git 2.29, the first whose `git fetch` takes `--no-write-fetch-head`, by `commitPatchOnBranch` before it makes anything (`gitReadiness.ts`); a commission, which never runs that fetch, is not. Commission Change goes to version 4, since its outputs changed. Tests: a sandboxed agent writes a `commondir` and a filter into the clone's git directory (both refused), points the checkout's gitfile at a git directory whose `commondir` names a repository it built with a filter defined, and adds a `.gitattributes` selecting it; collection and the apply check run nothing and read the executor's git directory, while a git resolving through the gitfile with the same safety arguments runs the filter (`checkoutGitDir.test.ts`; unconfined, the executor half in `gitFilterEscape.test.ts`). + ### P5 — Measure, then trim (F17) Production-build heap and mount time for a long conversation with the Workbench open; if the reducer's retained feeds are the cost, completed feeds fold to counts and a stored reference, the card reading the store on open (the path the run page already uses). @@ -304,7 +308,7 @@ D20: the first `yarn start`, `yarn dev:local` or `yarn redis:password` on a mach ## 8. Test plan -A copy test over the product package for the withheld words on local surfaces; edition-gating tests for Repos, code-repository and the hosted-only tabs; a connection-notice test for the fresh-chat case; the machine-card reader tested against a space context with and without a machine; a ref-snapshot test with a planted `git branch -D` inside a commission; a Local Publish test for the existing-branch case; a golden transcript for the first question in a fresh local workspace; memory tests for write-without-provider marking, lexical-only search reporting, and a provider switch re-indexing while search stays available. +A copy test over the product package for the withheld words on local surfaces; edition-gating tests for Repos, code-repository and the hosted-only tabs; a connection-notice test for the fresh-chat case; the machine-card reader tested against a space context with and without a machine; a test that a commission's planted `git branch -D`, hooks-path override and push into the folder leave every ref of the folder as it was (F56); a Local Publish test for the existing-branch case; a golden transcript for the first question in a fresh local workspace; memory tests for write-without-provider marking, lexical-only search reporting, and a provider switch re-indexing while search stays available. ## 9. Open questions diff --git a/packages/platform-artifacts/src/skillCatalog/commissionChange.test.ts b/packages/platform-artifacts/src/skillCatalog/commissionChange.test.ts index 8bd83705..1534d1a3 100644 --- a/packages/platform-artifacts/src/skillCatalog/commissionChange.test.ts +++ b/packages/platform-artifacts/src/skillCatalog/commissionChange.test.ts @@ -138,7 +138,6 @@ describe('Commission Change — a brief to the machine’s coding agent', () => 'filesChanged', 'sessionRef', 'continued', - 'refChanges', ]; expect(promoted).toEqual(expected); expect((wf.stateVariables ?? []).map((v) => v.variableId)).toEqual(expected); diff --git a/packages/platform-artifacts/src/skillCatalog/commissionChange.ts b/packages/platform-artifacts/src/skillCatalog/commissionChange.ts index 307ca33a..9007f6f2 100644 --- a/packages/platform-artifacts/src/skillCatalog/commissionChange.ts +++ b/packages/platform-artifacts/src/skillCatalog/commissionChange.ts @@ -24,7 +24,7 @@ const MINUTES = 60_000; const COMMISSION_CHANGE: SkillCatalogEntry = { catalogId: 'commission-change', - version: 3, + version: 4, name: 'Commission Change', tagline: "Have the machine's own coding agent make a change in a connected repository, and get the change back as a patch.", @@ -38,7 +38,7 @@ const COMMISSION_CHANGE: SkillCatalogEntry = { **The checks a brief asks for are the folder's own** — its test runner on the touched tests, its type check, its linter, its formatter — run by the coding agent in the commission's checkout, with their output in the result the brief asks for. Nothing else runs them before publication, where the folder's declared checks run again on the commit. -**With the result**: report what changed — \`filesChanged\`, whether the patch \`applies\` where it would be published, and any branch or tag in \`refChanges\`. To publish, start Publish Local Changes with the commission's \`patchRef\`, \`baseSha\` and, where it merged, \`merge.from\`, as they stand — never the \`patch\` text, a copy for reading that is cut short on a large change. To ask for more on the same work, start another commission with \`continueFrom\` set to its \`sessionRef\`.`, +**With the result**: report what changed — \`filesChanged\` and whether the patch \`applies\` where it would be published. To publish, start Publish Local Changes with the commission's \`patchRef\`, \`baseSha\` and, where it merged, \`merge.from\`, as they stand — never the \`patch\` text, a copy for reading that is cut short on a large change. To ask for more on the same work, start another commission with \`continueFrom\` set to its \`sessionRef\`.`, tags: ['coding', 'commission', 'local', 'developer-tools'], bundle: { workflow: { @@ -176,7 +176,6 @@ const COMMISSION_CHANGE: SkillCatalogEntry = { { kind: 'output_path' as const, path: 'filesChanged', toState: 'filesChanged' }, { kind: 'output_path' as const, path: 'sessionRef', toState: 'sessionRef' }, { kind: 'output_path' as const, path: 'continued', toState: 'continued' }, - { kind: 'output_path' as const, path: 'refChanges', toState: 'refChanges' }, ], }, ], @@ -242,20 +241,11 @@ const COMMISSION_CHANGE: SkillCatalogEntry = { sensitive: false, immutable: false, }, - { - variableId: 'refChanges', - name: 'Ref changes', - description: - 'Every local branch and tag of the folder that moved while the commission ran, whoever moved it.', - required: false, - sensitive: false, - immutable: false, - }, ], output: { primary: 'patchRef', guidance: - 'patchRef is the change. A publication takes patchRef, baseSha and merge.from as they stand — never the patch text — and a further turn on the same work takes sessionRef as continueFrom. Report filesChanged and applies; a conflict means the base moved under the change. refChanges names any branch or tag that moved in the folder while it ran.', + 'patchRef is the change. A publication takes patchRef, baseSha and merge.from as they stand — never the patch text — and a further turn on the same work takes sessionRef as continueFrom. Report filesChanged and applies; a conflict means the base moved under the change.', }, iteration: { auto: false, maxConsecutiveRuns: 1, stopOnOutcomesMet: true, cooldownMs: 0 }, }, diff --git a/packages/platform-artifacts/src/storeCatalogHashes.json b/packages/platform-artifacts/src/storeCatalogHashes.json index ab3f3328..0dc48e08 100644 --- a/packages/platform-artifacts/src/storeCatalogHashes.json +++ b/packages/platform-artifacts/src/storeCatalogHashes.json @@ -100,7 +100,7 @@ "contentHash": "cd6f27ade9b8f319bd43cafed7e48afffb8412148560638f5181fabb505f8815" }, "local-commission": { - "version": 3, + "version": 4, "contentHash": "35998b7e73fb024809520f7738c4f169e18858a800d924baf195f43e6648a90a" }, "local-publish": { diff --git a/packages/schemas/src/operations/__tests__/hostHarnessRunInput.test.ts b/packages/schemas/src/operations/__tests__/hostHarnessRunInput.test.ts index 0684377d..694b91e0 100644 --- a/packages/schemas/src/operations/__tests__/hostHarnessRunInput.test.ts +++ b/packages/schemas/src/operations/__tests__/hostHarnessRunInput.test.ts @@ -200,7 +200,6 @@ describe('a fix to a branch its base moved past names the merge at both layers', truncated: false, applies: 'clean', headMoved: false, - refChanges: [], blockedDomains: [], }; const merge = { @@ -292,7 +291,6 @@ describe("a commission's diff travels by reference", () => { truncated: false, applies: 'clean', headMoved: false, - refChanges: [], blockedDomains: [], }; expect(HostHarnessRunOutputSchema.parse(run).patchRef).toBe(ref); diff --git a/packages/schemas/src/operations/host.ts b/packages/schemas/src/operations/host.ts index 1f7e1908..59288e21 100644 --- a/packages/schemas/src/operations/host.ts +++ b/packages/schemas/src/operations/host.ts @@ -1018,7 +1018,7 @@ export const HostHarnessRunOutputSchema = z.object({ baseSha: z .string() .describe( - "Commit the isolated worktree started from — `base` resolved, or the folder's HEAD — " + + "Commit the run's private clone started from — `base` resolved, or the folder's HEAD — " + 'before any merge. A publication that appends to a branch passes this as its ' + '`commit.baseSha`.', ), @@ -1131,8 +1131,7 @@ export const HostHarnessRunOutputSchema = z.object({ 'tree. A run that took a while can be overtaken by the operator committing or editing ' + 'the same lines. A run that merged is judged against its merge commit instead, which ' + 'is where the publication applies the diff, so a branch that moved during the run is ' + - 'not noticed here: the publication refuses it as `stale_base`, and `refChanges` ' + - 'already reports a move in the folder.', + 'not noticed here: the publication refuses it as `stale_base`.', ), applyConflict: z .string() @@ -1145,21 +1144,6 @@ export const HostHarnessRunOutputSchema = z.object({ 'given a `base`, or continuing a session that was, is judged against that base, not ' + 'the HEAD, and reports false.', ), - refChanges: z - .array( - z.object({ - ref: z.string(), - change: z.enum(['created', 'deleted', 'moved']), - from: z.string().optional().describe('What it pointed at before; absent when created.'), - to: z.string().optional().describe('What it points at now; absent when deleted.'), - }), - ) - .describe( - 'Every local branch and tag that moved while the run was in flight, whoever moved it: ' + - "the operator's own work in the folder shows here too. A hook refuses the commission's " + - 'ordinary git in its checkout; a git call that names its own `core.hooksPath`, or ' + - 'pushes locally into the folder, is not refused, and what it moved is recorded here.', - ), blockedDomains: z .array(z.string()) .describe( diff --git a/packages/schemas/src/operations/hostRegistrations.ts b/packages/schemas/src/operations/hostRegistrations.ts index e5a50806..071bee24 100644 --- a/packages/schemas/src/operations/hostRegistrations.ts +++ b/packages/schemas/src/operations/hostRegistrations.ts @@ -306,12 +306,10 @@ export const HostOperationRegistrations: OperationRegistration[] = [ 'Delegate a whole task over the files in a connected folder to a harness installed on ' + "the operator's machine — the same tool they use themselves, already authenticated, " + 'with their toolchain around it. It executes any work over those files: analysis, ' + - 'documents, data and code alike. The run happens in an isolated checkout at the ' + - 'current commit, or at the branch or commit it names, so their uncommitted work is ' + - 'untouched. A hook refuses ordinary git in that checkout when it would move the ' + - "repository's branches or tags; a git call that names its own `core.hooksPath`, or " + - 'pushes locally into the folder, is not refused, and `refChanges` records every branch ' + - 'or tag that moved during the run, whoever moved it. Given an `outputSchema` it ' + + 'documents, data and code alike. The run happens in a private clone of the folder at ' + + 'the current commit, or at the branch or commit it names, so their uncommitted work is ' + + "untouched. The clone's branches and tags are its own: whatever git the harness runs " + + "there moves none of the folder's. Given an `outputSchema` it " + 'returns a validated `result`; where it changed files it returns a diff to review, ' + 'committed, pushed and merged nowhere.', tags: ['host', 'harness', 'files', 'local'],