From 0301a96aecc530370d900f10e63412039ba03e6a Mon Sep 17 00:00:00 2001 From: Christopher Tso Date: Fri, 3 Jul 2026 12:02:54 +0200 Subject: [PATCH] fix(core): simplify repo resolver output contract --- .../next/guides/workspace-architecture.mdx | 37 +++--- ...rtifact-and-workspace-resolver-contract.md | 5 +- .../src/evaluation/workspace/repo-manager.ts | 85 ++++++++++--- .../src/evaluation/workspace/repo-resolver.ts | 49 +++----- .../evaluation/workspace/repo-manager.test.ts | 119 ++++++++++++++++-- 5 files changed, 217 insertions(+), 78 deletions(-) diff --git a/apps/web/src/content/docs/docs/next/guides/workspace-architecture.mdx b/apps/web/src/content/docs/docs/next/guides/workspace-architecture.mdx index 4bcda0a0a..4fabfa5de 100644 --- a/apps/web/src/content/docs/docs/next/guides/workspace-architecture.mdx +++ b/apps/web/src/content/docs/docs/next/guides/workspace-architecture.mdx @@ -37,8 +37,8 @@ eval start +---------------------------+ | 3. Repo materialization | For each workspace.repos entry: | a. resolve acquisition | - registered project, configured mirror, -| b. git clone/fetch | AgentV cache, or remote fallback -| c. git checkout | - check out commit or HEAD +| b. materialize bytes | resolver source, AgentV cache, or remote fallback +| c. checkout if Git | - check out commit or HEAD for Git sources +---------------------------+ | v @@ -120,8 +120,8 @@ For each materialized repo, AgentV resolves acquisition in this order: | Order | Source | How it is used | |-------|--------|----------------| -| 1 | Pattern resolver | The first non-`default` `repo_resolvers[]` entry whose `repos` pattern matches the repo URL or identity. If it returns `handled:false`, AgentV continues to the default resolver. | -| 2 | Default resolver | The resolver named `default`, if configured. It must not declare `repos`; it is the unconditional project default. If it returns `handled:false`, AgentV continues to the built-in git resolver. | +| 1 | Pattern resolver | The first non-`default` `repo_resolvers[]` entry whose `repos` pattern matches the repo URL or identity. If it returns `status: "skip"`, AgentV continues to the default resolver. | +| 2 | Default resolver | The resolver named `default`, if configured. It must not declare `repos`; it is the unconditional project default. If it returns `status: "skip"`, AgentV continues to the built-in git resolver. | | 3 | Registered project | A project in `$AGENTV_HOME/projects.yaml` whose `origin` matches the repo identity. AgentV seeds its mirror cache from that local checkout, then clones the cache into the workspace and resets `origin` to the declared repo URL. | | 4 | Configured mirror | A path listed under `git_cache.mirrors`. AgentV seeds its mirror cache from that checkout or bare mirror, then clones the cache into the workspace. | | 5 | Mirror cache | An AgentV-owned bare cache under `$AGENTV_DATA_DIR/git-cache/`. Cache population is locked, cloned into a temporary path, verified, and atomically renamed before use. | @@ -136,8 +136,7 @@ local checkout is moved, deleted, or garbage-collected. Use `repo_resolvers` when repo bytes come from a project-specific source that AgentV core should not understand, such as an internal snapshot bundle. Put that -logic in a resolver script and return a local git source for AgentV to clone and -check out normally: +logic in a resolver script and return a local acquisition path: ```yaml # .agentv/config.yaml @@ -163,18 +162,26 @@ resolver writes JSON on stdout: ```json { - "handled": true, - "source": { - "type": "git", - "path": "/tmp/source.git", - "origin": "https://github.com/example/repo.git" - } + "status": "handled", + "path": "/tmp/source.git" } ``` -Only `source.type: "git"` is supported. Resolver scripts should prepare or -locate source directories independently from the final workspace; AgentV still -materializes the repo into every suite or attempt workspace it creates. +Return `{ "status": "skip" }` when the resolver does not own that repo. +`path` can point at a Git source or a plain directory snapshot. AgentV +classifies the path at runtime. + +For Git sources, AgentV clones from `path`, applies `sparse` when present, +resolves `commit`, walks `ancestor`, and checks out the resulting commit. +Resolver stdout does not set or override `origin`; the resolver chooses +acquisition bytes only. + +For plain directory snapshots, AgentV copies the directory contents into +`workspace.repos[].path` without requiring `.git`. Because the snapshot has no +Git history, `commit`, `ancestor`, and `sparse` are ignored for that repo. +Workspace resets restore the snapshot by copying the same resolver directory +source again. Targets and graders see a normal filesystem directory at the +configured workspace path. ### Configured mirrors diff --git a/docs/adr/0017-output-artifact-and-workspace-resolver-contract.md b/docs/adr/0017-output-artifact-and-workspace-resolver-contract.md index cc0b4b27f..3ee90effa 100644 --- a/docs/adr/0017-output-artifact-and-workspace-resolver-contract.md +++ b/docs/adr/0017-output-artifact-and-workspace-resolver-contract.md @@ -134,8 +134,9 @@ new acquisition technology plugs in without touching it. `commit` is an immutabl **before hooks** (ADR 0016 pt10). 2. **Acquisition = harness resolver in machine config (`$AGENTV_HOME/config.yaml`), keyed on `repo`**, ordered backends: (1) local checkout auto-adopt via origin-match - → `git clone --reference`; (2) bare mirror clone-cache (`--reference`, shared objects); - (3) snapshot artifact (WTG `download-release-deps` reframed); (4) remote clone; + → mirror cache; (2) configured local mirror; (3) custom command resolver returning + a flat `{status,path}` acquisition source, including Git sources or static directory + snapshots; (4) AgentV mirror cache and remote clone; (5) *future* Docker image (SWE-bench/margin/Inspect — same identity key, new backend; adopt Inspect's `image`/`build`/`x-local` distinction + per-config init caching). 3. **`--reference` (mirror cache) is the workhorse**: shallow-speed WITH full history, so diff --git a/packages/core/src/evaluation/workspace/repo-manager.ts b/packages/core/src/evaluation/workspace/repo-manager.ts index 5e0af3df2..74125466c 100644 --- a/packages/core/src/evaluation/workspace/repo-manager.ts +++ b/packages/core/src/evaluation/workspace/repo-manager.ts @@ -1,7 +1,7 @@ import { execFile, spawn } from 'node:child_process'; import { createHash, randomUUID } from 'node:crypto'; import { existsSync, readFileSync } from 'node:fs'; -import { mkdir, rename, rm, stat, writeFile } from 'node:fs/promises'; +import { cp, mkdir, rename, rm, stat, writeFile } from 'node:fs/promises'; import path from 'node:path'; import { promisify } from 'node:util'; @@ -47,7 +47,8 @@ interface AcquisitionSource { | 'registered-project' | 'mirror-cache' | 'remote' - | 'repo-resolver'; + | 'repo-resolver-git' + | 'repo-resolver-directory'; readonly sourceUrl: string; readonly originUrl: string; } @@ -133,6 +134,7 @@ export class RepoManager { private readonly heartbeatMs: number; private readonly timeoutMs: number; private readonly projectConfigDir?: string; + private readonly materializedAcquisitions = new Map(); constructor(verbose = false, options: RepoManagerOptions = {}) { this.verbose = verbose; @@ -650,12 +652,8 @@ export class RepoManager { workspacePath, this.timeoutMs, ); - if (result.handled) { - return { - kind: 'repo-resolver', - sourceUrl: result.source.path, - originUrl: result.source.origin ?? originUrl, - }; + if (result.status === 'handled') { + return this.repoResolverAcquisitionSource(result.path, originUrl); } if (selection.kind === 'pattern') { @@ -667,12 +665,8 @@ export class RepoManager { workspacePath, this.timeoutMs, ); - if (defaultResult.handled) { - return { - kind: 'repo-resolver', - sourceUrl: defaultResult.source.path, - originUrl: defaultResult.source.origin ?? originUrl, - }; + if (defaultResult.status === 'handled') { + return this.repoResolverAcquisitionSource(defaultResult.path, originUrl); } } } @@ -680,6 +674,37 @@ export class RepoManager { return undefined; } + private async repoResolverAcquisitionSource( + sourcePath: string, + originUrl: string, + ): Promise { + return { + kind: (await this.isGitCloneSource(sourcePath)) + ? 'repo-resolver-git' + : 'repo-resolver-directory', + sourceUrl: sourcePath, + originUrl, + }; + } + + private async isGitCloneSource(sourcePath: string): Promise { + try { + await stat(sourcePath); + } catch { + return false; + } + + try { + await this.runGit(['rev-parse', '--git-dir'], { + cwd: sourcePath, + timeout: 10_000, + }); + return true; + } catch { + return false; + } + } + private assertNoUserOwnedAlternates(targetDir: string, acquisition: AcquisitionSource): void { const alternatesPath = path.join(targetDir, '.git', 'objects', 'info', 'alternates'); if (!existsSync(alternatesPath)) return; @@ -757,6 +782,7 @@ export class RepoManager { const targetDir = path.join(workspacePath, repo.path); const acquisition = await this.resolveAcquisition(repo, workspacePath); + this.materializedAcquisitions.set(targetDir, acquisition); const startedAt = Date.now(); if (this.verbose) { @@ -765,6 +791,16 @@ export class RepoManager { ); } + if (acquisition.kind === 'repo-resolver-directory') { + await this.copyDirectorySnapshot(acquisition.sourceUrl, targetDir); + if (this.verbose) { + console.log( + `[repo] materialize done path=${repo.path} target=${targetDir} durationMs=${Date.now() - startedAt}`, + ); + } + return; + } + // Plain local clones hardlink objects on the same filesystem and copy // otherwise; unlike --reference, neither path leaves alternates behind. const cloneArgs = ['clone', '--progress', '--no-checkout']; @@ -776,7 +812,10 @@ export class RepoManager { }); this.assertNoUserOwnedAlternates(targetDir, acquisition); - if (acquisition.sourceUrl !== acquisition.originUrl) { + if ( + acquisition.kind !== 'repo-resolver-git' && + acquisition.sourceUrl !== acquisition.originUrl + ) { assertSafeGitOperand(acquisition.originUrl, 'repo origin URL'); await this.runGit(['remote', 'set-url', 'origin', acquisition.originUrl], { cwd: targetDir }); } @@ -829,9 +868,25 @@ export class RepoManager { for (const repo of repos) { if (!repo.path || !repo.repo) continue; const targetDir = path.join(workspacePath, repo.path); + const acquisition = this.materializedAcquisitions.get(targetDir); + if (acquisition?.kind === 'repo-resolver-directory') { + await this.copyDirectorySnapshot(acquisition.sourceUrl, targetDir); + continue; + } const resetSha = await this.resolveCheckoutCommit(repo, targetDir); await this.runGit(['reset', '--hard', resetSha], { cwd: targetDir }); await this.runGit(['clean', cleanFlag], { cwd: targetDir }); } } + + private async copyDirectorySnapshot(sourceDir: string, targetDir: string): Promise { + assertSafeGitOperand(sourceDir, 'repo resolver directory source'); + const sourceStat = await stat(sourceDir); + if (!sourceStat.isDirectory()) { + throw new Error(`Repo resolver directory source is not a directory: ${sourceDir}`); + } + await rm(targetDir, { recursive: true, force: true }); + await mkdir(path.dirname(targetDir), { recursive: true }); + await cp(sourceDir, targetDir, { recursive: true, force: true }); + } } diff --git a/packages/core/src/evaluation/workspace/repo-resolver.ts b/packages/core/src/evaluation/workspace/repo-resolver.ts index 5aca876e1..4c590b616 100644 --- a/packages/core/src/evaluation/workspace/repo-resolver.ts +++ b/packages/core/src/evaluation/workspace/repo-resolver.ts @@ -6,7 +6,7 @@ import path from 'node:path'; import micromatch from 'micromatch'; import { getAgentvDataDir } from '../../paths.js'; -import type { JsonObject, JsonValue, RepoConfig } from '../types.js'; +import type { JsonObject, RepoConfig } from '../types.js'; import { isJsonObject } from '../types.js'; import { getRepoCheckoutRef } from './repo-checkout.js'; import { normalizeRepoIdentity, resolveRepoCloneUrl } from './repo-identity.js'; @@ -34,19 +34,13 @@ export interface RepoResolverRequest { readonly config: JsonObject; } -export interface RepoResolverGitSource { - readonly type: 'git'; - readonly path: string; - readonly origin?: string; -} - export interface RepoResolverHandledResult { - readonly handled: true; - readonly source: RepoResolverGitSource; + readonly status: 'handled'; + readonly path: string; } export interface RepoResolverUnhandledResult { - readonly handled: false; + readonly status: 'skip'; } export type RepoResolverResult = RepoResolverHandledResult | RepoResolverUnhandledResult; @@ -308,35 +302,22 @@ function parseRepoResolverOutput(stdout: string, resolverName: string): RepoReso throw new Error(`Repo resolver '${resolverName}' stdout must be a JSON object.`); } - const output = parsed as Record; - if (output.handled === false) { - return { handled: false }; + const output = parsed as Record; + if (output.status === 'skip') { + return { status: 'skip' }; } - if (output.handled !== true) { - throw new Error(`Repo resolver '${resolverName}' stdout must set handled to true or false.`); + if (output.status !== 'handled') { + throw new Error( + `Repo resolver '${resolverName}' stdout must set status to 'handled' or 'skip'.`, + ); } - if (!isJsonObject(output.source)) { - throw new Error(`Repo resolver '${resolverName}' handled the repo but did not return source.`); - } - - const source = output.source as Record; - if (source.type !== 'git') { - throw new Error(`Repo resolver '${resolverName}' returned unsupported source.type.`); - } - if (typeof source.path !== 'string' || source.path.trim().length === 0) { - throw new Error(`Repo resolver '${resolverName}' source.path must be a non-empty string.`); - } - if (source.origin !== undefined && typeof source.origin !== 'string') { - throw new Error(`Repo resolver '${resolverName}' source.origin must be a string when set.`); + if (typeof output.path !== 'string' || output.path.trim().length === 0) { + throw new Error(`Repo resolver '${resolverName}' path must be a non-empty string.`); } return { - handled: true, - source: { - type: 'git', - path: source.path, - ...(typeof source.origin === 'string' && { origin: source.origin }), - }, + status: 'handled', + path: output.path, }; } diff --git a/packages/core/test/evaluation/workspace/repo-manager.test.ts b/packages/core/test/evaluation/workspace/repo-manager.test.ts index 7e710eb01..ba2890164 100644 --- a/packages/core/test/evaluation/workspace/repo-manager.test.ts +++ b/packages/core/test/evaluation/workspace/repo-manager.test.ts @@ -105,16 +105,15 @@ function writeResolverScript(scriptPath: string): void { 'if (request.config.request_log) {', ' appendFileSync(request.config.request_log, `${JSON.stringify(request)}\\n`);', '}', - 'if (request.config.handled === false) {', - ' process.stdout.write(JSON.stringify({ handled: false }));', + 'if (request.config.raw_stdout !== undefined) {', + ' process.stdout.write(String(request.config.raw_stdout));', + '} else ', + "if (request.config.status === 'skip') {", + " process.stdout.write(JSON.stringify({ status: 'skip' }));", '} else {', ' process.stdout.write(JSON.stringify({', - ' handled: true,', - ' source: {', - " type: 'git',", + " status: 'handled',", ' path: request.config.source_path,', - ' origin: request.config.origin,', - ' },', ' }));', '}', '', @@ -393,7 +392,7 @@ describe('RepoManager', () => { const targetDir = path.join(workspaceDir, 'resolved'); expect(readFileSync(path.join(targetDir, 'resolved.txt'), 'utf-8')).toBe('from resolver'); - expect(gitExec('git remote get-url origin', targetDir)).toBe( + expect(gitExec('git remote get-url origin', targetDir)).not.toBe( 'https://github.com/example/unreachable.git', ); }, 30_000); @@ -458,7 +457,7 @@ describe('RepoManager', () => { expect(existsSync(path.join(targetDir, 'src', 'main.ts'))).toBe(true); }, 30_000); - it('continues from handled:false pattern resolvers to the default resolver', async () => { + it('continues from status:skip pattern resolvers to the default resolver', async () => { const defaultSource = path.join(tmpDir, 'default-source'); createTestRepo(defaultSource, { 'default.txt': 'from default' }); const scriptPath = path.join(tmpDir, 'scripts', 'resolver.ts'); @@ -472,7 +471,7 @@ describe('RepoManager', () => { name: 'pattern', repos: ['https://github.com/example/*'], command: ['bun', scriptPath], - config: { handled: false }, + config: { status: 'skip' }, }, { name: 'default', @@ -498,7 +497,7 @@ describe('RepoManager', () => { ); }, 30_000); - it('falls back to built-in git acquisition when the default resolver returns handled:false', async () => { + it('falls back to built-in git acquisition when the default resolver returns status:skip', async () => { const sourceRepo = path.join(tmpDir, 'builtin-source'); createTestRepo(sourceRepo, { 'builtin.txt': 'from built-in' }); const scriptPath = path.join(tmpDir, 'scripts', 'resolver.ts'); @@ -511,7 +510,7 @@ describe('RepoManager', () => { { name: 'default', command: ['bun', scriptPath], - config: { handled: false }, + config: { status: 'skip' }, }, ]); @@ -532,6 +531,102 @@ describe('RepoManager', () => { ); }, 30_000); + it('rejects invalid resolver stdout without accepting the old handled/source shape', async () => { + const scriptPath = path.join(tmpDir, 'scripts', 'resolver.ts'); + writeResolverScript(scriptPath); + const projectDir = path.join(tmpDir, 'project-invalid-stdout'); + const evalDir = path.join(projectDir, 'evals'); + mkdirSync(path.join(projectDir, '.git'), { recursive: true }); + mkdirSync(evalDir, { recursive: true }); + writeRepoResolversConfig(path.join(projectDir, '.agentv', 'config.yaml'), [ + { + name: 'invalid_stdout', + repos: ['https://github.com/example/*'], + command: ['bun', scriptPath], + config: { + raw_stdout: JSON.stringify({ + handled: true, + source: { type: 'git', path: path.join(tmpDir, 'old-source') }, + }), + }, + }, + ]); + + const projectManager = new RepoManager(false, { + progress: false, + projectConfigDir: evalDir, + }); + await expect( + projectManager.materialize( + { + path: './invalid', + repo: 'https://github.com/example/invalid.git', + }, + workspaceDir, + ), + ).rejects.toThrow("stdout must set status to 'handled' or 'skip'"); + }, 30_000); + + it('copies non-git resolver directory snapshots and restores them on reset', async () => { + const snapshotDir = path.join(tmpDir, 'static-snapshot'); + mkdirSync(path.join(snapshotDir, 'src'), { recursive: true }); + writeFileSync(path.join(snapshotDir, 'src', 'main.ts'), 'snapshot'); + writeFileSync(path.join(snapshotDir, 'README.md'), 'static'); + const scriptPath = path.join(tmpDir, 'scripts', 'resolver.ts'); + writeResolverScript(scriptPath); + const projectDir = path.join(tmpDir, 'project-static'); + const evalDir = path.join(projectDir, 'evals'); + mkdirSync(path.join(projectDir, '.git'), { recursive: true }); + mkdirSync(evalDir, { recursive: true }); + writeRepoResolversConfig(path.join(projectDir, '.agentv', 'config.yaml'), [ + { + name: 'static', + repos: ['https://github.com/example/*'], + command: ['bun', scriptPath], + config: { source_path: snapshotDir }, + }, + ]); + + const projectManager = new RepoManager(false, { + progress: false, + projectConfigDir: evalDir, + }); + await projectManager.materialize( + { + path: './static', + repo: 'https://github.com/example/static.git', + commit: 'missing-commit', + ancestor: 3, + sparse: ['only-this-path'], + }, + workspaceDir, + ); + + const targetDir = path.join(workspaceDir, 'static'); + expect(readFileSync(path.join(targetDir, 'src', 'main.ts'), 'utf-8')).toBe('snapshot'); + expect(readFileSync(path.join(targetDir, 'README.md'), 'utf-8')).toBe('static'); + expect(existsSync(path.join(targetDir, '.git'))).toBe(false); + + writeFileSync(path.join(targetDir, 'src', 'main.ts'), 'mutated'); + writeFileSync(path.join(targetDir, 'generated.txt'), 'generated'); + await projectManager.reset( + [ + { + path: './static', + repo: 'https://github.com/example/static.git', + commit: 'missing-commit', + ancestor: 3, + sparse: ['only-this-path'], + }, + ], + workspaceDir, + 'strict', + ); + + expect(readFileSync(path.join(targetDir, 'src', 'main.ts'), 'utf-8')).toBe('snapshot'); + expect(existsSync(path.join(targetDir, 'generated.txt'))).toBe(false); + }, 30_000); + it('rejects duplicate resolver names and repos on the default resolver', async () => { const scriptPath = path.join(tmpDir, 'scripts', 'resolver.ts'); writeResolverScript(scriptPath);