diff --git a/README.md b/README.md index 01837646..b72854c7 100644 --- a/README.md +++ b/README.md @@ -178,7 +178,7 @@ CLAUDE_CODE_OAUTH_TOKEN=$(op read "op://Personal/Claude OAuth/credential") \ ```ts const task = await fetcher.fetch('123'); -const ctx = await prepareContext({ taskId: task.id, localRepoPath, sandbox }); +const ctx = await prepareContext({ taskId: task.id, localRepoPath, baseBranch: 'main', start: 'base', sandbox }); try { await runStages(ctx, implementReviewSimplifyStages(), { agent, variables: taskToVariables(task) }); const commit = await commitStage(ctx, { message: `feat: ${task.title}` }); diff --git a/examples/validate-phase3.ts b/examples/validate-phase3.ts index ec78396e..43909491 100644 --- a/examples/validate-phase3.ts +++ b/examples/validate-phase3.ts @@ -47,7 +47,7 @@ function sandbox(auth: AgentAuth): DockerSandboxProvider { async function scenarioAdversary(auth: AgentAuth): Promise { const repo = await freshRepo(); - const ctx = await prepareContext({ taskId: 'adv', localRepoPath: repo, sandbox: sandbox(auth) }); + const ctx = await prepareContext({ taskId: 'adv', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: sandbox(auth) }); try { const implementer: PipelineStage = { name: 'implementer', @@ -81,7 +81,7 @@ async function scenarioAdversary(auth: AgentAuth): Promise { async function scenarioHitl(auth: AgentAuth): Promise { const repo = await freshRepo(); - const ctx = await prepareContext({ taskId: 'hitl', localRepoPath: repo, sandbox: sandbox(auth) }); + const ctx = await prepareContext({ taskId: 'hitl', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: sandbox(auth) }); try { const generate: PipelineStage = { name: 'generator', @@ -114,7 +114,7 @@ async function scenarioHitl(auth: AgentAuth): Promise { async function scenarioBudget(auth: AgentAuth): Promise { const repo = await freshRepo(); - const ctx = await prepareContext({ taskId: 'budget', localRepoPath: repo, sandbox: sandbox(auth) }); + const ctx = await prepareContext({ taskId: 'budget', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: sandbox(auth) }); try { const step = (name: string, file: string): PipelineStage => ({ name, diff --git a/src/cli/research.ts b/src/cli/research.ts index f927218f..8cb1359d 100644 --- a/src/cli/research.ts +++ b/src/cli/research.ts @@ -6,6 +6,7 @@ import { startSandboxContext } from '../sandbox/sandbox-context.js'; import { agentAuthFromEnv, authSecrets } from '../agents/auth.js'; import { selectAgents } from '../agents/registry.js'; import { prepareContext, runAgent, disposeContext } from '../core/vanguard.js'; +import { DEFAULT_BASE_BRANCH } from '../core/base-branch.js'; import { literalPrompt } from '../context/prompt-engine.js'; import { runResearch } from '../runners/research.js'; import type { SandboxContext } from '../sandbox/sandbox-context.js'; @@ -87,7 +88,8 @@ async function runDefaultResearcher( ...(sandboxContext.network !== undefined ? { network: sandboxContext.network } : {}), }); const taskId = `research-${cmd.issueRef.replace(/[^a-zA-Z0-9]/g, '-')}`; - const ctx = await prepareContext({ taskId, localRepoPath: cmd.repoPath, sandbox, agentName: agents.agent.name }); + // research has no --base: the codebase is read at the default base as origin has it, like every other run. + const ctx = await prepareContext({ taskId, localRepoPath: cmd.repoPath, sandbox, agentName: agents.agent.name, baseBranch: DEFAULT_BASE_BRANCH, start: 'base' }); try { const result = await runAgent(ctx, { stageName: 'research', diff --git a/src/cli/review-mr.test.ts b/src/cli/review-mr.test.ts index 20160678..97859542 100644 --- a/src/cli/review-mr.test.ts +++ b/src/cli/review-mr.test.ts @@ -16,7 +16,7 @@ vi.mock('../core/vanguard.js', async (importOriginal) => ({ })); import { startSandboxContext } from '../sandbox/sandbox-context.js'; -import { runAgent } from '../core/vanguard.js'; +import { prepareContext, runAgent } from '../core/vanguard.js'; import { mergeRequestReviewMarker, reviewMergeRequest } from '../runners/mr-review.js'; import { reviewMrCommand } from './review-mr.js'; import type { GlabRunner } from '../tasks/gitlab.js'; @@ -88,6 +88,34 @@ describe('reviewMrCommand', () => { expect(calls.filter((c) => c[0] === 'mr' && c[1] === 'note')).toHaveLength(1); }); + it('prepares the worktree on the MR target branch as origin has it, and says so when the MR carries none (#446)', async () => { + process.env.CLAUDE_CODE_OAUTH_TOKEN = 'sk-ant-oat-test'; + vi.mocked(startSandboxContext).mockResolvedValue({ destroy: async () => undefined } as never); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + try { + vi.mocked(prepareContext).mockClear(); + const attempt = { taskId: 't', exitReason: 'completed', turns: 1, worktreePath: '/wt', worktreePreserved: false } as const; + vi.mocked(runAgent).mockResolvedValue({ ...attempt, completed: true, finalText: 'No blocking findings.' }); + for (const targetBranch of ['release', undefined]) { + const glab: GlabRunner = async (args) => { + if (args[0] === 'mr' && args[1] === 'view') return JSON.stringify({ iid: 5, sha: 'abc123def4567890', ...(targetBranch !== undefined ? { target_branch: targetBranch } : {}) }); + if (args[0] === 'api' && args[1] === 'user') return JSON.stringify({ username: 'vanguard-bot' }); + if (args[0] === 'api') return '[]'; + return ''; + }; + await reviewMrCommand( + { kind: 'review-mr', iid: 5, project: 'g/p', repoPath: '/repo', egress: true }, + { reviewMergeRequest: (ref, deps) => reviewMergeRequest(ref, { ...deps, glab }), log: () => undefined }, + ); + } + const prepared = vi.mocked(prepareContext).mock.calls.map(([opts]) => [opts.baseBranch, opts.start]); + expect(prepared).toEqual([['release', 'base'], ['main', 'base']]); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('MR g/p!5 carries no base branch')); + } finally { + warn.mockRestore(); + } + }); + it('--max-turns sets the first attempt cap and 1.5x for the retry', async () => { process.env.CLAUDE_CODE_OAUTH_TOKEN = 'sk-ant-oat-test'; vi.mocked(startSandboxContext).mockResolvedValue({ destroy: async () => undefined } as never); diff --git a/src/cli/review-mr.ts b/src/cli/review-mr.ts index d2dce762..ecab4f23 100644 --- a/src/cli/review-mr.ts +++ b/src/cli/review-mr.ts @@ -6,6 +6,7 @@ import { startSandboxContext } from '../sandbox/sandbox-context.js'; import { agentAuthFromEnv, authSecrets } from '../agents/auth.js'; import { selectAgents } from '../agents/registry.js'; import { prepareContext, runAgent, disposeContext } from '../core/vanguard.js'; +import { baseBranchOrDefault } from '../core/base-branch.js'; import { literalPrompt } from '../context/prompt-engine.js'; import { adversarySystemPrompt } from '../pipeline/pipeline.js'; import { buildMergeRequestReviewPrompt, reviewMergeRequest } from '../runners/mr-review.js'; @@ -102,7 +103,9 @@ async function runDefaultMrReviewer( ...(sandboxContext.network !== undefined ? { network: sandboxContext.network } : {}), }); const taskId = `mr-review-${mr.project.replace(/[^a-zA-Z0-9]/g, '-')}-${mr.iid}`; - const ctx = await prepareContext({ taskId, localRepoPath: cmd.repoPath, sandbox, agentName: agents.agent.name }); + // The reviewer reads the tree the change lands on: the MR's target branch as origin has it. + const baseBranch = baseBranchOrDefault(mr.targetBranch, `MR ${mr.project}!${mr.iid}`); + const ctx = await prepareContext({ taskId, localRepoPath: cmd.repoPath, sandbox, agentName: agents.agent.name, baseBranch, start: 'base' }); const baseTurns = cmd.maxTurns ?? DEFAULT_REVIEW_MAX_TURNS; try { const result = await runAgent(ctx, { diff --git a/src/cli/review-pr.test.ts b/src/cli/review-pr.test.ts index d85666a6..88783bf2 100644 --- a/src/cli/review-pr.test.ts +++ b/src/cli/review-pr.test.ts @@ -15,7 +15,7 @@ vi.mock('../core/vanguard.js', async (importOriginal) => ({ disposeContext: vi.fn(async () => undefined), })); -import { runAgent } from '../core/vanguard.js'; +import { prepareContext, runAgent } from '../core/vanguard.js'; import { readFile, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -104,6 +104,35 @@ describe('reviewPrCommand', () => { ]); }); + it('the default reviewer prepares the worktree on the PR base as origin has it, and says so when the PR carries none (#446)', async () => { + const prev = process.env.CLAUDE_CODE_OAUTH_TOKEN; + process.env.CLAUDE_CODE_OAUTH_TOKEN = 'sk-ant-oat-test'; + const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + try { + vi.mocked(prepareContext).mockClear(); + const attempt = { taskId: 't', exitReason: 'completed', turns: 1, worktreePath: '/wt', worktreePreserved: false } as const; + vi.mocked(runAgent).mockResolvedValue({ ...attempt, completed: true, finalText: 'No blocking findings.' }); + const cmd: Extract = { kind: 'review-pr', prRef: '12', repoSlug: 'o/r', repoPath: '/repo', egress: false }; + // The injected runner hands the default reviewer a PR against `release`, then one with no base at all. + for (const baseRefName of ['release', '']) { + await reviewPrCommand(cmd, { + reviewPullRequest: async (_ref, deps) => { + await deps.reviewer({ ...fakeResult().pr, baseRefName }, { isRetry: false }); + return fakeResult(); + }, + log: () => undefined, + }); + } + const prepared = vi.mocked(prepareContext).mock.calls.map(([opts]) => [opts.baseBranch, opts.start]); + expect(prepared).toEqual([['release', 'base'], ['main', 'base']]); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('PR o/r#12 carries no base branch')); + } finally { + warn.mockRestore(); + if (prev === undefined) delete process.env.CLAUDE_CODE_OAUTH_TOKEN; + else process.env.CLAUDE_CODE_OAUTH_TOKEN = prev; + } + }); + it('--out writes the review to a file and tells the runner not to publish (no PR comment)', async () => { const outPath = join(tmpdir(), `vanguard-review-out-${process.pid}-${Math.random().toString(36).slice(2)}.md`); let seenPublish: boolean | undefined; diff --git a/src/cli/review-pr.ts b/src/cli/review-pr.ts index fef39434..f27c91b2 100644 --- a/src/cli/review-pr.ts +++ b/src/cli/review-pr.ts @@ -8,6 +8,7 @@ import { startSandboxContext } from '../sandbox/sandbox-context.js'; import { agentAuthFromEnv, authSecrets } from '../agents/auth.js'; import { selectAgents } from '../agents/registry.js'; import { prepareContext, runAgent, disposeContext } from '../core/vanguard.js'; +import { baseBranchOrDefault } from '../core/base-branch.js'; import { literalPrompt } from '../context/prompt-engine.js'; import { adversarySystemPrompt } from '../pipeline/pipeline.js'; import { buildPullRequestReviewPrompt, PullRequestReviewIncompleteError, reviewPullRequest } from '../runners/pr-review.js'; @@ -112,7 +113,9 @@ async function runDefaultReviewer( ...(sandboxContext.network !== undefined ? { network: sandboxContext.network } : {}), }); const taskId = `pr-review-${pr.repoSlug.replace(/[^a-zA-Z0-9]/g, '-')}-${pr.number}`; - const ctx = await prepareContext({ taskId, localRepoPath: cmd.repoPath, sandbox, agentName: agents.agent.name }); + // The reviewer reads the tree the change lands on: the PR's base as origin has it. + const baseBranch = baseBranchOrDefault(pr.baseRefName, `PR ${pr.repoSlug}#${pr.number}`); + const ctx = await prepareContext({ taskId, localRepoPath: cmd.repoPath, sandbox, agentName: agents.agent.name, baseBranch, start: 'base' }); const baseTurns = cmd.maxTurns ?? DEFAULT_REVIEW_MAX_TURNS; try { const result = await runAgent(ctx, { diff --git a/src/core/base-branch.ts b/src/core/base-branch.ts index 5c1d73da..ebbbcc76 100644 --- a/src/core/base-branch.ts +++ b/src/core/base-branch.ts @@ -22,6 +22,19 @@ export function assertSafeBaseBranch(base: string): void { } } +/** The base a run targets when no `--base` is given. Every default goes through this name, never a bare literal. */ +export const DEFAULT_BASE_BRANCH = 'main'; + +/** + * A base taken from a PR/MR record, or the default when the API left it out. The fallback is announced: + * a review that silently reads the wrong branch is the failure this module exists to prevent. + */ +export function baseBranchOrDefault(base: string, what: string): string { + if (base !== '') return base; + console.warn(`vanguard: ${what} carries no base branch — reviewing against ${DEFAULT_BASE_BRANCH}`); + return DEFAULT_BASE_BRANCH; +} + export interface ResolveRemoteBaseRefOptions { logger?: VanguardLogger; /** Log-line prefix naming the pass ('spec', 'worktree'). */ diff --git a/src/core/vanguard.test.ts b/src/core/vanguard.test.ts index 3107669e..1336d350 100644 --- a/src/core/vanguard.test.ts +++ b/src/core/vanguard.test.ts @@ -171,7 +171,7 @@ describe('vanguard.run', () => { it('reuses one context across multiple agent stages (R11)', async () => { const wm = new WorktreeManager(repo); const { sandbox, wasDestroyed } = makeSandbox(); - const ctx = await prepareContext({ taskId: 't3', localRepoPath: repo, sandbox }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 't3', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox }, { worktrees: wm }); const a1 = await runAgent(ctx, { promptTemplate: 'a', agent: fakeAgent([{ text: 'one' }], { finalText: 'one', turns: 1, sessionId: 's' }), @@ -188,11 +188,11 @@ describe('vanguard.run', () => { expect(wasDestroyed()).toBe(true); }); - it('passes PrepareOptions.reuse to WorktreeManager.create', async () => { + it('a reuse start asks WorktreeManager.create for the existing branch', async () => { const wm = new WorktreeManager(repo); const spy = vi.spyOn(wm, 'create'); const { sandbox } = makeSandbox(); - const ctx = await prepareContext({ taskId: 'reuse-test', localRepoPath: repo, sandbox, reuse: true }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'reuse-test', localRepoPath: repo, baseBranch: 'main', start: 'reuse', sandbox }, { worktrees: wm }); await disposeContext(ctx); expect(spy).toHaveBeenCalledOnce(); expect(spy.mock.calls[0]?.[2]).toEqual({ reuse: true }); @@ -201,7 +201,7 @@ describe('vanguard.run', () => { it('keeps the sandbox alive when disposeContext is told to keep it', async () => { const wm = new WorktreeManager(repo); const { sandbox, wasDestroyed } = makeSandbox(); - const ctx = await prepareContext({ taskId: 'keep', localRepoPath: repo, sandbox }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'keep', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox }, { worktrees: wm }); await disposeContext(ctx, { keep: true }); expect(wasDestroyed()).toBe(false); }); @@ -241,7 +241,7 @@ describe('vanguard.run', () => { } as unknown as IsolatedSandboxProvider; const agent = fakeAgent([{ text: 'noop' }], { finalText: 'noop', turns: 1 }); - const ctx = await prepareContext({ taskId: 'cb-false', localRepoPath: repo, sandbox }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'cb-false', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox }, { worktrees: wm }); const result = await runAgent(ctx, { promptTemplate: 'p', agent, copyBack: false }); await disposeContext(ctx); @@ -278,7 +278,7 @@ describe('vanguard.run', () => { } as unknown as IsolatedSandboxProvider; const agent = fakeAgent([{ text: 'done' }], { finalText: 'done', turns: 1 }); - const ctx = await prepareContext({ taskId: 'cb-default', localRepoPath: repo, sandbox }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'cb-default', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox }, { worktrees: wm }); const result = await runAgent(ctx, { promptTemplate: 'p', agent }); await disposeContext(ctx); @@ -293,7 +293,7 @@ describe('vanguard.run', () => { await writeFile(join(hostPath, 'output.txt'), 'created'); }); const agent = fakeAgent([{ text: 'done' }], { finalText: 'done', turns: 1 }); - const ctx = await prepareContext({ taskId: 'cb-true', localRepoPath: repo, sandbox }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'cb-true', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox }, { worktrees: wm }); const result = await runAgent(ctx, { promptTemplate: 'p', agent, copyBack: true }); await disposeContext(ctx); @@ -304,7 +304,7 @@ describe('vanguard.run', () => { const wm = new WorktreeManager(repo); const { sandbox } = makeSandbox(); const agent = fakeAgent([], { finalText: 'done', turns: 1 }); - const ctx = await prepareContext({ taskId: 'model-fallback', localRepoPath: repo, sandbox }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'model-fallback', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox }, { worktrees: wm }); const result = await runAgent(ctx, { promptTemplate: 'p', agent, model: 'configured-model' }); await disposeContext(ctx); @@ -315,7 +315,7 @@ describe('vanguard.run', () => { const wm = new WorktreeManager(repo); const { sandbox } = makeSandbox(); const agent = fakeAgent([], { finalText: 'done', turns: 1, model: 'actual-model' }); - const ctx = await prepareContext({ taskId: 'model-reported', localRepoPath: repo, sandbox }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'model-reported', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox }, { worktrees: wm }); const result = await runAgent(ctx, { promptTemplate: 'p', agent, model: 'configured-model' }); await disposeContext(ctx); @@ -353,7 +353,7 @@ describe('vanguard.run', () => { return { finalText: 'reviewed', turns: 1, sessionId: 'codex-thread-1' }; }, }; - const ctx = await prepareContext({ taskId: 'cap-skip', localRepoPath: repo, sandbox }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'cap-skip', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox }, { worktrees: wm }); const result = await runAgent(ctx, { promptTemplate: 'p', agent }); await disposeContext(ctx); @@ -372,7 +372,7 @@ describe('vanguard.run', () => { return { finalText: 'done', turns: 1, sessionId: 's1' }; }, }; - const ctx = await prepareContext({ taskId: `cap-${name.replace(/[^a-z0-9]/gi, '-')}`, localRepoPath: repo, sandbox }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: `cap-${name.replace(/[^a-z0-9]/gi, '-')}`, localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox }, { worktrees: wm }); await runAgent(ctx, { promptTemplate: 'p', agent }); await disposeContext(ctx); expect(captureAttempted).toBe(true); @@ -389,7 +389,7 @@ describe('vanguard.run', () => { return { finalText: 'done', turns: 1, sessionId: 's1' }; }, }; - const ctx = await prepareContext({ taskId: 'cap-warn', localRepoPath: repo, sandbox }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'cap-warn', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox }, { worktrees: wm }); const result = await runAgent(ctx, { promptTemplate: 'p', agent }); await disposeContext(ctx); @@ -408,7 +408,7 @@ describe('vanguard.run', () => { costUsd: 0.01, }); const ctx = await prepareContext( - { taskId: 'log-1', localRepoPath: repo, sandbox, logger }, + { taskId: 'log-1', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox, logger }, { worktrees: wm }, ); await runAgent(ctx, { promptTemplate: 'p', agent, stageName: 'implementer' }); @@ -439,7 +439,7 @@ describe('vanguard.run', () => { await writeFile(join(hostPath, 'src', 'app.ts'), 'export const x = 1;\n'); }); const agent = fakeAgent([{ text: 'done' }], { finalText: 'done', turns: 1 }); - const ctx = await prepareContext({ taskId: 'wf-skip', localRepoPath: repo, sandbox }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'wf-skip', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox }, { worktrees: wm }); const result = await runAgent(ctx, { promptTemplate: 'p', agent }); await disposeContext(ctx); @@ -456,7 +456,7 @@ describe('vanguard.run', () => { await writeFile(join(hostPath, '.github', 'workflows', 'evil.yml'), 'on: push\njobs: {}\n'); }); const agent = fakeAgent([{ text: 'done' }], { finalText: 'done', turns: 1 }); - const ctx = await prepareContext({ taskId: 'wf-warn', localRepoPath: repo, sandbox, logger }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'wf-warn', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox, logger }, { worktrees: wm }); await runAgent(ctx, { promptTemplate: 'p', agent }); await disposeContext(ctx); @@ -477,7 +477,7 @@ describe('vanguard.run', () => { await writeFile(join(hostPath, '.gitlab', 'merge_request_templates', 'default.md'), 'template\n'); }); const agent = fakeAgent([{ text: 'done' }], { finalText: 'done', turns: 1 }); - const ctx = await prepareContext({ taskId: 'gl-skip', localRepoPath: repo, sandbox }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'gl-skip', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox }, { worktrees: wm }); const result = await runAgent(ctx, { promptTemplate: 'p', agent }); await disposeContext(ctx); @@ -496,7 +496,7 @@ describe('vanguard.run', () => { await writeFile(join(hostPath, '.github', 'dependabot.yml'), 'version: 2\n'); }); const agent = fakeAgent([{ text: 'done' }], { finalText: 'done', turns: 1 }); - const ctx = await prepareContext({ taskId: 'wf-sibling', localRepoPath: repo, sandbox }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'wf-sibling', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox }, { worktrees: wm }); const result = await runAgent(ctx, { promptTemplate: 'p', agent }); await disposeContext(ctx); @@ -531,7 +531,7 @@ describe('vanguard.run', () => { await symlink(outside, join(hostPath, '.gitlab', 'linked.yml')); }); const agent = fakeAgent([{ text: 'done' }], { finalText: 'done', turns: 1 }); - const ctx = await prepareContext({ taskId: 'ci-changed', localRepoPath: repo, sandbox, logger }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'ci-changed', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox, logger }, { worktrees: wm }); await runAgent(ctx, { promptTemplate: 'p', agent }); await disposeContext(ctx); await rm(outside, { force: true }); @@ -558,7 +558,7 @@ describe('vanguard.run', () => { await writeFile(join(hostPath, 'sub', '.gitlab'), 'not a directory\n'); }); const agent = fakeAgent([{ text: 'done' }], { finalText: 'done', turns: 1 }); - const ctx = await prepareContext({ taskId: 'ci-symlink', localRepoPath: repo, sandbox }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'ci-symlink', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox }, { worktrees: wm }); const result = await runAgent(ctx, { promptTemplate: 'p', agent }); const linked = await Promise.all(['.gitlab', '.github'].map((name) => lstat(join(ctx.worktreePath, name)).then(() => true, () => false))); await disposeContext(ctx); @@ -577,7 +577,7 @@ describe('vanguard.run', () => { for (let i = 0; i < 25; i++) await writeFile(join(hostPath, '.gitlab', 'ci', `job-${i}.yml`), `job${i}: {}\n`); }); const agent = fakeAgent([{ text: 'done' }], { finalText: 'done', turns: 1 }); - const ctx = await prepareContext({ taskId: 'ci-flood', localRepoPath: repo, sandbox, logger }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'ci-flood', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox, logger }, { worktrees: wm }); await runAgent(ctx, { promptTemplate: 'p', agent }); await disposeContext(ctx); @@ -633,7 +633,7 @@ describe('vanguard.run', () => { await execa('git', ['config', 'diff.srcPrefix', 'src/'], { cwd: repo }); await execa('git', ['config', 'diff.dstPrefix', 'dst/'], { cwd: repo }); const wm = new WorktreeManager(repo); - const ctx = await prepareContext({ taskId: 'wf-paths', localRepoPath: repo, sandbox: makeSandbox().sandbox }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'wf-paths', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox().sandbox }, { worktrees: wm }); await chmod(join(ctx.worktreePath, '.github', 'workflows', 'ci.yml'), 0o755); await writeFile(join(ctx.worktreePath, '.gitlab', 'ci', 'job.yml'), Buffer.from([0, 1, 2, 0])); @@ -665,7 +665,7 @@ describe('vanguard.run', () => { const wm = new WorktreeManager(repo); const { sandbox } = makeSandbox(); const agent = fakeAgent([{ text: 'done' }], { finalText: 'done', turns: 1 }); - const ctx = await prepareContext({ taskId: 'wf-guard-block', localRepoPath: repo, sandbox }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'wf-guard-block', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox }, { worktrees: wm }); // Simulate a bypass of the copy-back skip (e.g. a future regression) by placing a workflow // file directly on the worktree, as the guard's job is to catch exactly this. await mkdir(join(ctx.worktreePath, '.github', 'workflows'), { recursive: true }); @@ -683,7 +683,7 @@ describe('vanguard.run', () => { await writeFile(join(hostPath, 'src', 'app.ts'), 'export const x = 1;\n'); }); const agent = fakeAgent([{ text: 'done' }], { finalText: 'done', turns: 1 }); - const ctx = await prepareContext({ taskId: 'wf-normal', localRepoPath: repo, sandbox, logger }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'wf-normal', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox, logger }, { worktrees: wm }); const result = await runAgent(ctx, { promptTemplate: 'p', agent }); await disposeContext(ctx); @@ -724,4 +724,81 @@ describe('vanguard.run base resolution (#429)', () => { // The sandbox received origin's tree, not the stale local main. expect(copiedIn).toBe('v2'); }); + + /** An origin with a `main` and a `feature` branch, and a clone whose local main is one commit behind origin's. */ + const originAndStaleClone = async (): Promise<{ origin: string; clone: string }> => { + const origin = await mkdtemp(join(tmpdir(), 'vg-origin-')); + dirs.push(origin); + await execa('git', ['init', '-q', '-b', 'main'], { cwd: origin }); + await commitFile(origin, 'v1'); + const clone = await mkdtemp(join(tmpdir(), 'vg-clone-')); + dirs.push(clone); + await execa('git', ['clone', '-q', origin, clone]); + await execa('git', ['checkout', '-q', '-b', 'feature'], { cwd: origin }); + await commitFile(origin, 'feature-work'); + await execa('git', ['checkout', '-q', 'main'], { cwd: origin }); + await commitFile(origin, 'v2'); + return { origin, clone }; + }; + const sha = async (cwd: string, ref: string): Promise => (await execa('git', ['rev-parse', ref], { cwd })).stdout; + + it('records the start commit once: origin/ for a base start, the local base for reuse, the fetched head for a PR head', async () => { + const { origin, clone } = await originAndStaleClone(); + const { logger, entries } = captureLogger(); + const wm = new WorktreeManager(clone); + const spy = vi.spyOn(wm, 'create'); + + const base = await prepareContext({ taskId: 'start-base', localRepoPath: clone, baseBranch: 'main', start: 'base', sandbox: makeSandbox().sandbox, logger }, { worktrees: wm }); + await disposeContext(base); + expect(base.startRef).toBe(await sha(origin, 'main')); + expect(base.baseBranch).toBe('main'); + // Announced exactly once, and the worktree was cut from that very SHA. + expect(entries.filter((e) => e.msg === 'run start').map((e) => e.obj.startRef)).toEqual([base.startRef]); + expect(spy.mock.calls.at(-1)?.[1]).toBe(base.startRef); + + const reuse = await prepareContext({ taskId: 'start-reuse', localRepoPath: clone, baseBranch: 'main', start: 'reuse', sandbox: makeSandbox().sandbox }, { worktrees: wm }); + await disposeContext(reuse); + expect(reuse.startRef).toBe(await sha(clone, 'refs/heads/main')); // the local copy, never fetched + expect(reuse.startRef).not.toBe(base.startRef); + + const head = await prepareContext({ taskId: 'start-head', localRepoPath: clone, baseBranch: 'main', start: { prHead: 'feature' }, sandbox: makeSandbox().sandbox }, { worktrees: wm }); + await disposeContext(head); + expect(head.startRef).toBe(await sha(origin, 'feature')); + }); + + it('fails loudly when the start cannot be resolved instead of landing on a default', async () => { + const { clone } = await originAndStaleClone(); + const wm = new WorktreeManager(clone); + // A fresh reuse cut from a base this clone does not carry fails where it always did (the cut). + await expect( + prepareContext({ taskId: 'start-missing', localRepoPath: clone, baseBranch: 'no-such-branch', start: 'reuse', sandbox: makeSandbox().sandbox }, { worktrees: wm }), + ).rejects.toThrow(/Failed to create worktree for start-missing/); + await expect( + prepareContext({ taskId: 'start-nohead', localRepoPath: clone, baseBranch: 'main', start: { prHead: 'no-such-branch' }, sandbox: makeSandbox().sandbox }, { worktrees: wm }), + ).rejects.toThrow(/no-such-branch/); + await expect( + prepareContext({ taskId: 'start-unsafe', localRepoPath: clone, baseBranch: '--upload-pack=x', start: { prHead: 'feature' }, sandbox: makeSandbox().sandbox }, { worktrees: wm }), + ).rejects.toThrow(/cannot start with "-"/); + }); + + it('a reuse start still resumes a run branch when the base has no local branch (tracking ref, then the branch tip)', async () => { + const { origin, clone } = await originAndStaleClone(); + // `release` exists on origin and as a tracking ref here, but was never checked out locally. + await execa('git', ['branch', 'release', 'main'], { cwd: origin }); + await execa('git', ['fetch', '-q', 'origin', 'release'], { cwd: clone }); + const wm = new WorktreeManager(clone, undefined, () => 'r1'); + const { logger, entries } = captureLogger(); + + const first = await prepareContext({ taskId: 'resume', localRepoPath: clone, baseBranch: 'release', start: 'reuse', sandbox: makeSandbox().sandbox, logger }, { worktrees: wm }); + await disposeContext(first); + expect(first.startRef).toBe(await sha(clone, 'refs/remotes/origin/release')); + + // The base gone entirely: the existing run branch is still picked up, measured from its own tip, and says so. + await execa('git', ['branch', '-D', '-r', 'origin/release'], { cwd: clone }); + const resumed = await prepareContext({ taskId: 'resume', localRepoPath: clone, baseBranch: 'release', start: 'reuse', sandbox: makeSandbox().sandbox, logger }, { worktrees: wm }); + await disposeContext(resumed); + expect(resumed.branch).toBe(first.branch); + expect(resumed.startRef).toBe(await sha(clone, `refs/heads/${first.branch}`)); + expect(entries.some((e) => e.msg.includes('release is not a commit here'))).toBe(true); + }); }); diff --git a/src/core/vanguard.ts b/src/core/vanguard.ts index d5c5905b..003a3878 100644 --- a/src/core/vanguard.ts +++ b/src/core/vanguard.ts @@ -1,5 +1,6 @@ import { cp, lstat, mkdir, readFile, readdir, readlink, rm } from 'node:fs/promises'; import { join, relative } from 'node:path'; +import { execa } from 'execa'; import { WorktreeManager } from '../worktree/manager.js'; import { SkillRegistry } from '../context/skill-registry.js'; import { renderPrompt } from '../context/prompt-engine.js'; @@ -10,8 +11,8 @@ import { stageMetric } from './run-metric.js'; import { createLogger } from './logger.js'; import { installSignalCleanup, trackSandbox, untrackSandbox } from './cleanup.js'; import { acquireSandboxSlot, releaseSandboxSlot } from './concurrency.js'; -import { SandboxError, WorkflowGuardError } from './errors.js'; -import { resolveRemoteBaseRef } from './base-branch.js'; +import { SandboxError, WorkflowGuardError, WorktreeError } from './errors.js'; +import { DEFAULT_BASE_BRANCH, assertSafeBaseBranch, redactGitError, resolveRemoteBaseRef } from './base-branch.js'; import type { RunOptions, RunResult, ExitReason, ReasoningEffort } from './types.js'; import type { IsolatedSandboxProvider } from '../sandbox/provider.js'; import type { AgentProvider, AgentUsage } from '../agents/provider.js'; @@ -49,11 +50,23 @@ const WORKFLOW_PATH = /(^|[\\/])\.github[\\/]workflows([\\/]|$)|(^|[\\/])\.githu const CI_DIR_NAME = /(^|[\\/])\.git(hub|lab)$/i; const IN_CI_DIR = /(^|[\\/])\.git(hub|lab)[\\/]/i; +/** + * Which commit a task starts from. `'base'`: origin's copy of `baseBranch` when it is ahead of the + * local one (see resolveRemoteBaseRef; the local copy when it is kept). `'reuse'`: an existing run + * branch keeps its own history, and the local `baseBranch` is only the baseline a fresh branch is cut + * from and new commits are measured against. `{ prHead }`: that branch as it is on origin (revise-pr). + */ +export type TaskStart = 'base' | 'reuse' | { prHead: string }; + export interface PrepareOptions { taskId: string; localRepoPath: string; - baseBranch?: string; - reuse?: boolean; + /** The branch the task is for: the cut point of a 'base'/'reuse' start, and what the PR/MR targets. */ + baseBranch: string; + /** Which commit the task starts from. Resolved once, here, and recorded as RunContext.startRef. */ + start: TaskStart; + /** 'base' only: keep a local base that is ahead of/diverged from origin (default: outside CI only). See resolveRemoteBaseRef. */ + keepLocalIfAhead?: boolean; /** Override the run branch prefix (white-label mode: e.g. `feat/`). Default VANGUARD_BRANCH_PREFIX. */ branchPrefix?: string; /** Override the run branch/path id (white-label mode: bare issue number). Default taskId. */ @@ -76,6 +89,10 @@ export interface RunContext { branch: string; home: string; localRepoPath: string; + /** The branch the task is for (PrepareOptions.baseBranch): the pre-push rebase fetches it and the PR/MR targets it. */ + baseBranch: string; + /** The commit the task started from, as a SHA. The worktree cut, the closing-keyword scan and the pre-push rebase all measure from it. */ + startRef: string; wm: WorktreeManager; log: VanguardLogger; /** CI config files the agent added or edited in the sandbox, which copy-back dropped. The PR/MR description or revision summary lists them. */ @@ -123,16 +140,73 @@ async function resolveHome(sandbox: IsolatedSandboxProvider): Promise { return home; } +/** `git rev-parse` of `ref` as a commit SHA, or '' when it names no commit (or no such ref). */ +async function revParseCommit(cwd: string, ref: string): Promise { + return (await execa('git', ['rev-parse', '--verify', '--quiet', `${ref}^{commit}`], { cwd, reject: false })).stdout.trim(); +} + +/** + * Resolve PrepareOptions.start to the SHA the task starts from. Decided once, here, so the worktree + * cut, the closing-keyword scan and the pre-push rebase cannot disagree, and no caller lands on a + * default base by leaving it out. A SHA, not a ref: a concurrent run's fetch cannot move it and a + * same-named tag cannot shadow it. Returns undefined only for a 'reuse' start whose base names no + * commit here: the base is then unused when an existing run branch is picked up (prepareContext + * measures from that branch instead), and a fresh cut fails in WorktreeManager.create as it always did. + */ +async function resolveStartRef(opts: PrepareOptions, log: VanguardLogger): Promise { + const { localRepoPath: cwd, baseBranch, start } = opts; + // ctx.baseBranch feeds a fetch and the PR/MR target later; validate it here, whatever the start kind. + assertSafeBaseBranch(baseBranch); + let ref: string; + let what: string; + if (start === 'base') { + ref = await resolveRemoteBaseRef(cwd, baseBranch, { logger: log, ...(opts.keepLocalIfAhead !== undefined ? { keepLocalIfAhead: opts.keepLocalIfAhead } : {}) }); + what = baseBranch; + } else if (start === 'reuse') { + // The base as this clone has it, never fetched: a reused branch keeps its own history. The local + // branch first (the full ref, so a same-named tag cannot shadow it), then the tracking ref a + // single-branch clone may hold instead, then whatever git makes of the bare name. + for (const candidate of [`refs/heads/${baseBranch}`, `refs/remotes/origin/${baseBranch}`, baseBranch]) { + const sha = await revParseCommit(cwd, candidate); + if (sha !== '') return sha; + } + return undefined; + } else { + // The PR head as it is on origin, so a revision continues the PR, not the base. Same-repo PRs + // only: a fork PR's head is not on origin, so a same-named origin branch, or nothing, is fetched + // instead. The head ref name is author-controlled: --end-of-options keeps a "-"-led name from + // parsing as a git option, and the full refs/heads/ source keeps a "+"-led name (legal in git) + // from reading as a force refspec. + // No credential prompt from a daemon and a bounded wait, as in resolveRemoteBaseRef; the failure is + // reported masked, never as raw git stderr. + const fetch = await execa('git', ['fetch', '--end-of-options', 'origin', `refs/heads/${start.prHead}`], { cwd, reject: false, env: { GIT_TERMINAL_PROMPT: '0' }, timeout: 60_000 }); + if (fetch.exitCode !== 0) throw new WorktreeError(`Cannot fetch origin/${start.prHead} for ${opts.taskId}: ${redactGitError(fetch)}`); + ref = 'FETCH_HEAD'; + what = `origin/${start.prHead}`; + } + const sha = await revParseCommit(cwd, ref); + if (sha === '') throw new WorktreeError(`Cannot resolve the start commit for ${opts.taskId}: ${what} (${ref}) is not a commit in ${cwd}`); + return sha; +} + /** Provision worktree + sandbox + skills for one task. Caller owns disposeContext(). */ export async function prepareContext(opts: PrepareOptions, deps: RunDeps = {}): Promise { const log = opts.logger ?? createLogger(); const wm = deps.worktrees ?? new WorktreeManager(opts.localRepoPath); const skills = deps.skills ?? new SkillRegistry({}); - const wt = await wm.create(opts.taskId, opts.baseBranch ?? 'main', { - ...(opts.reuse !== undefined ? { reuse: opts.reuse } : {}), + let startRef = await resolveStartRef(opts, log); + const wt = await wm.create(opts.taskId, startRef ?? opts.baseBranch, { + ...(opts.start === 'reuse' ? { reuse: true } : {}), ...(opts.branchPrefix !== undefined ? { branchPrefix: opts.branchPrefix } : {}), ...(opts.branchId !== undefined ? { branchId: opts.branchId } : {}), }); + if (startRef === undefined) { + // A reused branch in a clone that does not carry the base at all: the branch's own tip is the only + // baseline there is (new commits are measured from it; nothing older is on the base either way). + startRef = await revParseCommit(wt.path, 'HEAD'); + if (startRef === '') throw new WorktreeError(`Cannot resolve the start commit for ${opts.taskId}: ${opts.baseBranch} is not a commit in ${opts.localRepoPath} and ${wt.branch} has no head`); + log.warn({ taskId: opts.taskId, baseBranch: opts.baseBranch, startRef }, `reuse: ${opts.baseBranch} is not a commit here — measuring from the tip of ${wt.branch}`); + } // Acquire a host concurrency slot before booting the sandbox, so a fan-out can't start more // sandboxes than the host can hold (blocks until a slot frees). await acquireSandboxSlot(opts.sandbox); @@ -152,10 +226,13 @@ export async function prepareContext(opts: PrepareOptions, deps: RunDeps = {}): branch: wt.branch, home, localRepoPath: opts.localRepoPath, + baseBranch: opts.baseBranch, + startRef, wm, log, }; - ctx.log.info({ taskId: opts.taskId }, 'run start'); + // The one place the start commit is announced: everything downstream reads ctx.startRef. + ctx.log.info({ taskId: opts.taskId, baseBranch: opts.baseBranch, startRef }, 'run start'); return ctx; } catch (error) { // Provisioning failed: untrack, free the slot, and tear down so nothing is orphaned. @@ -434,22 +511,17 @@ export async function disposeContext(ctx: RunContext, opts: { keep?: boolean } = /** Single-stage convenience: prepare -> one agent run -> dispose. */ export async function run(opts: RunOptions, deps: RunDeps = {}): Promise { - // Cut from origin's copy of the base when it is ahead of the local one, so the sandbox, verification - // and review see the tree the PR will land on (#423). A reused branch keeps its own history. - const baseBranch = opts.reuse === true - ? opts.baseBranch - : await resolveRemoteBaseRef(opts.localRepoPath, opts.baseBranch ?? 'main', { - ...(opts.logger !== undefined ? { logger: opts.logger } : {}), - ...(opts.keepLocalIfAhead !== undefined ? { keepLocalIfAhead: opts.keepLocalIfAhead } : {}), - }); const ctx = await prepareContext( { taskId: opts.taskId, localRepoPath: opts.localRepoPath, sandbox: opts.sandbox, agentName: opts.agent.name, - ...(baseBranch !== undefined ? { baseBranch } : {}), - ...(opts.reuse !== undefined ? { reuse: opts.reuse } : {}), + // Cut from origin's copy of the base when it is ahead of the local one, so the sandbox, verification + // and review see the tree the PR will land on (#423). A reused branch keeps its own history. + baseBranch: opts.baseBranch ?? DEFAULT_BASE_BRANCH, + start: opts.reuse === true ? 'reuse' : 'base', + ...(opts.keepLocalIfAhead !== undefined ? { keepLocalIfAhead: opts.keepLocalIfAhead } : {}), ...(opts.skills !== undefined ? { skills: opts.skills } : {}), ...(opts.logger !== undefined ? { logger: opts.logger } : {}), }, diff --git a/src/index.ts b/src/index.ts index f92d86d9..4286682b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,4 +1,4 @@ -export { run, prepareContext, runAgent, disposeContext, type RunDeps, type RunContext, type PrepareOptions, type StageInput } from './core/vanguard.js'; +export { run, prepareContext, runAgent, disposeContext, type RunDeps, type RunContext, type PrepareOptions, type TaskStart, type StageInput } from './core/vanguard.js'; export * as vanguard from './core/vanguard.js'; export type { RunOptions, RunResult, ReasoningEffort, ExitReason } from './core/types.js'; export type { diff --git a/src/pipeline/fork-select.test.ts b/src/pipeline/fork-select.test.ts index ba00fe28..849c552b 100644 --- a/src/pipeline/fork-select.test.ts +++ b/src/pipeline/fork-select.test.ts @@ -117,7 +117,7 @@ async function withCtx( fn: (ctx: RunContext) => Promise, ): Promise { const wm = new WorktreeManager(repo); - const ctx = await prepareContext({ taskId, localRepoPath: repo, sandbox }, { worktrees: wm }); + const ctx = await prepareContext({ taskId, localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox }, { worktrees: wm }); try { await fn(ctx); } finally { diff --git a/src/pipeline/judged-repair.test.ts b/src/pipeline/judged-repair.test.ts index ce573cfc..637b0637 100644 --- a/src/pipeline/judged-repair.test.ts +++ b/src/pipeline/judged-repair.test.ts @@ -73,7 +73,7 @@ function judgeReject(times: number): Judge { describe('runJudgedRepair', () => { it('freezes to needs_human after 3 consecutive rejects, leaving a shell command', async () => { const wm = new WorktreeManager(repo); - const ctx = await prepareContext({ taskId: 'h1', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'h1', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const result = await runJudgedRepair(ctx, { agent: agentReturning('done'), generate, repair, judge: judgeReject(99) }); expect(result.status).toBe('frozen'); if (result.status === 'frozen') { @@ -86,7 +86,7 @@ describe('runJudgedRepair', () => { it('completes when the judge passes within the reject budget', async () => { const wm = new WorktreeManager(repo); - const ctx = await prepareContext({ taskId: 'h2', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'h2', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const result = await runJudgedRepair(ctx, { agent: agentReturning('done'), generate, repair, judge: judgeReject(1) }); expect(result.status).toBe('completed'); await disposeContext(ctx); @@ -94,7 +94,7 @@ describe('runJudgedRepair', () => { it('freezes to budget_exceeded instead of completing when the judge still rejects', async () => { const wm = new WorktreeManager(repo); - const ctx = await prepareContext({ taskId: 'h-budget', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'h-budget', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const result = await runJudgedRepair(ctx, { agent: agentReturning('done', 0.5), generate, diff --git a/src/pipeline/pipeline.test.ts b/src/pipeline/pipeline.test.ts index 8d51c787..78f7fbc2 100644 --- a/src/pipeline/pipeline.test.ts +++ b/src/pipeline/pipeline.test.ts @@ -89,7 +89,7 @@ describe('runStages', () => { it('runs stages, chains the session, and feeds the previous diff forward', async () => { const wm = new WorktreeManager(repo); const received: AgentRunInput[] = []; - const ctx = await prepareContext({ taskId: 'p1', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'p1', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const outcomes = await runStages( ctx, [ @@ -111,7 +111,7 @@ describe('runStages', () => { const wm = new WorktreeManager(repo); const def: AgentRunInput[] = []; const review: AgentRunInput[] = []; - const ctx = await prepareContext({ taskId: 'xp', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'xp', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); await runStages( ctx, [ @@ -129,7 +129,7 @@ describe('runStages', () => { describe('sandboxComplete', () => { it('runs the agent one-shot in /tmp and returns its finalText', async () => { const wm = new WorktreeManager(repo); - const ctx = await prepareContext({ taskId: 'sc', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'sc', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); let seen: AgentRunInput | undefined; const agent: AgentProvider = { name: 'scorer', @@ -164,7 +164,7 @@ describe('runBudgetedStages', () => { it('freezes to budget_exceeded before the stage that would exceed the limit', async () => { const wm = new WorktreeManager(repo); - const ctx = await prepareContext({ taskId: 'bud', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'bud', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const result = await runBudgetedStages(ctx, threeStages, { agent: costingAgent(0.03), maxCostUsd: 0.05 }); expect(result.status).toBe('frozen'); if (result.status === 'frozen') { @@ -178,7 +178,7 @@ describe('runBudgetedStages', () => { it('completes when under budget', async () => { const wm = new WorktreeManager(repo); - const ctx = await prepareContext({ taskId: 'bud2', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'bud2', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const result = await runBudgetedStages(ctx, threeStages, { agent: costingAgent(0.01), maxCostUsd: 1 }); expect(result.status).toBe('completed'); if (result.status === 'completed') expect(result.outcomes).toHaveLength(3); @@ -197,7 +197,7 @@ describe('runBudgetedStages', () => { }, }; const fallback = recordingAgent(fallbackInputs); - const ctx = await prepareContext({ taskId: 'fallback', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'fallback', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const stages = withStageFallback( [{ name: 'reviewer', promptTemplate: 'review', provider: primary, model: 'gpt-5' }], { provider: fallback, model: 'sonnet' }, @@ -237,7 +237,7 @@ describe('runBudgetedStages', () => { }, }; const fallback = recordingAgent(fallbackInputs); - const ctx = await prepareContext({ taskId: 'impl-fallback', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'impl-fallback', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const stages = withStageFallback( [ { name: 'planner', promptTemplate: 'plan', provider: fallback }, @@ -285,7 +285,7 @@ describe('runBudgetedStages', () => { }, }; const fallback = recordingAgent(fallbackInputs); - const ctx = await prepareContext({ taskId: 'latch', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'latch', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const stages = withPrimaryFallback( [ { name: 'implementer', promptTemplate: 'implement', model: 'claude-sonnet-5' }, @@ -310,7 +310,7 @@ describe('runBudgetedStages', () => { it('emits ordered stage + cost events when onEvent is set', async () => { const wm = new WorktreeManager(repo); - const ctx = await prepareContext({ taskId: 'ev', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'ev', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const events: RunEvent[] = []; await runBudgetedStages(ctx, threeStages, { agent: costingAgent(0.01), @@ -332,7 +332,7 @@ describe('runBudgetedStages', () => { describe('commitStage', () => { it('commits dirty worktree work onto the branch', async () => { const wm = new WorktreeManager(repo, undefined, () => 'r1'); - const ctx = await prepareContext({ taskId: 'p2', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'p2', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); await writeFile(join(ctx.worktreePath, 'x.txt'), 'data'); const out = await commitStage(ctx, { message: 'feat: agent work' }); expect(out.committed).toBe(true); @@ -344,7 +344,7 @@ describe('commitStage', () => { it('reports nothing to commit on a clean worktree', async () => { const wm = new WorktreeManager(repo); - const ctx = await prepareContext({ taskId: 'p3', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'p3', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const out = await commitStage(ctx, { message: 'noop' }); expect(out.committed).toBe(false); await disposeContext(ctx); @@ -357,7 +357,7 @@ describe('commitStage', () => { await writeFile(hook, '#!/bin/sh\nexit 1\n', { mode: 0o755 }); try { const wm = new WorktreeManager(repo, undefined, () => 'r4'); - const ctx = await prepareContext({ taskId: 'p4', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'p4', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); await writeFile(join(ctx.worktreePath, 'y.txt'), 'data'); const out = await commitStage(ctx, { message: 'feat: work' }); expect(out.committed).toBe(true); @@ -387,7 +387,7 @@ describe('generateEvaluateRepairStages', () => { return { finalText: 'RAPORT-' + received.length, turns: 1 }; }, }; - const ctx = await prepareContext({ taskId: 'ger', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'ger', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); await runStages( ctx, [ @@ -590,7 +590,7 @@ describe('defaultSystemPrompt', () => { return { finalText: 'x', turns: 1 }; }, }; - const ctx = await prepareContext({ taskId: 'sp', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'sp', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); await runStages(ctx, [{ name: 's', promptTemplate: 'p', systemPrompt: 'SYS-XYZ' }], { agent }); expect(received[0]?.systemPrompt).toBe('SYS-XYZ'); await disposeContext(ctx); @@ -619,7 +619,7 @@ describe('runBudgetedStages copyBack', () => { shellCommand: (): string => 'docker exec -it vg-fake bash', } as unknown as IsolatedSandboxProvider; - const ctx = await prepareContext({ taskId: 'cb-pipe', localRepoPath: repo, sandbox }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'cb-pipe', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox }, { worktrees: wm }); const result = await runBudgetedStages( ctx, [{ name: 'spec', promptTemplate: 'describe', copyBack: false }], @@ -695,7 +695,7 @@ describe('techSpecStage', () => { shellCommand: (): string => 'docker exec -it vg-fake bash', } as unknown as IsolatedSandboxProvider; - const ctx = await prepareContext({ taskId: 'ts-ro', localRepoPath: repo, sandbox }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'ts-ro', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox }, { worktrees: wm }); await runBudgetedStages(ctx, techSpecStage(), { agent: recordingAgent([]), variables: { TITLE: 'T', DESCRIPTION: 'D' } }); await disposeContext(ctx); @@ -718,7 +718,7 @@ describe('techSpecStage', () => { }, }; - const ctx = await prepareContext({ taskId: 'ts-resume', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'ts-resume', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const outcomes = await runStages(ctx, techSpecStage(), { agent, variables: { TITLE: 'T', DESCRIPTION: 'D' } }); await disposeContext(ctx); @@ -780,7 +780,7 @@ describe('runBudgetedStages fork option', () => { }; const ctx = await prepareContext( - { taskId: 'fork-impl', localRepoPath: repo, sandbox: makeSandbox() }, + { taskId: 'fork-impl', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }, ); const result = await runBudgetedStages( @@ -816,7 +816,7 @@ describe('runBudgetedStages fork option', () => { const score = scoreCall++ === 0 ? 0.9 : 0.3; return `{"passed":true,"score":${score},"reason":"ok"}`; }; - const ctx = await prepareContext({ taskId: 'fork-merge', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'fork-merge', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const result = await runBudgetedStages(ctx, [{ name: 'implementer', promptTemplate: 'x' }], { agent, fork: { n: 2, complete } }); await disposeContext(ctx); expect(result.status).toBe('completed'); @@ -1061,7 +1061,7 @@ describe('per-stage budget cap', () => { it('effectiveCap uses fraction when set (no floor): fraction * maxCostUsd', async () => { const wm = new WorktreeManager(repo); const received: AgentRunInput[] = []; - const ctx = await prepareContext({ taskId: 'cap-frac', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'cap-frac', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); await runBudgetedStages( ctx, [{ name: 'a', promptTemplate: 'a', stageCostFraction: 0.4 }], @@ -1075,7 +1075,7 @@ describe('per-stage budget cap', () => { it('effectiveCap applies floor when floor exceeds fraction result', async () => { const wm = new WorktreeManager(repo); const received: AgentRunInput[] = []; - const ctx = await prepareContext({ taskId: 'cap-floor', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'cap-floor', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); await runBudgetedStages( ctx, [{ name: 'a', promptTemplate: 'a', stageCostFraction: 0.1, stageCostFloorUsd: 0.5 }], @@ -1089,7 +1089,7 @@ describe('per-stage budget cap', () => { it('global always wins: floor is capped by remainingGlobal when global is tiny', async () => { const wm = new WorktreeManager(repo); const received: AgentRunInput[] = []; - const ctx = await prepareContext({ taskId: 'cap-tiny-global', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'cap-tiny-global', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); await runBudgetedStages( ctx, [{ name: 'a', promptTemplate: 'a', stageCostFraction: 0.5, stageCostFloorUsd: 10.0 }], @@ -1103,7 +1103,7 @@ describe('per-stage budget cap', () => { it('stages without new fields get effectiveCap = remaining global (back-compat)', async () => { const wm = new WorktreeManager(repo); const received: AgentRunInput[] = []; - const ctx = await prepareContext({ taskId: 'cap-compat', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'cap-compat', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); await runBudgetedStages( ctx, [{ name: 'a', promptTemplate: 'a' }], @@ -1117,7 +1117,7 @@ describe('per-stage budget cap', () => { it('continue policy: implementer at cap, reviewer still runs, result completed', async () => { const wm = new WorktreeManager(repo); const received: AgentRunInput[] = []; - const ctx = await prepareContext({ taskId: 'cap-continue', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'cap-continue', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const stages: PipelineStage[] = [ { name: 'implementer', promptTemplate: 'impl', stageCostFraction: 0.5, onStageBudgetExceeded: 'continue' }, { name: 'reviewer', promptTemplate: 'review' }, @@ -1136,7 +1136,7 @@ describe('per-stage budget cap', () => { it('freeze policy: stage at cap returns frozen with outcomes so far', async () => { const wm = new WorktreeManager(repo); - const ctx = await prepareContext({ taskId: 'cap-freeze', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'cap-freeze', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const stages: PipelineStage[] = [ { name: 'a', promptTemplate: 'a', stageCostFraction: 0.3, onStageBudgetExceeded: 'freeze' }, { name: 'b', promptTemplate: 'b' }, @@ -1161,7 +1161,7 @@ describe('per-stage budget cap', () => { it('global backstop freezes regardless of per-stage continue policy', async () => { const wm = new WorktreeManager(repo); - const ctx = await prepareContext({ taskId: 'cap-global-wins', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'cap-global-wins', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const stages: PipelineStage[] = [ { name: 'a', promptTemplate: 'a', stageCostFraction: 0.5, onStageBudgetExceeded: 'continue' }, { name: 'b', promptTemplate: 'b', stageCostFraction: 0.5, onStageBudgetExceeded: 'continue' }, @@ -1189,7 +1189,7 @@ describe('per-stage budget cap', () => { it('skip policy: stage at cap proceeds like continue (next stage runs)', async () => { const wm = new WorktreeManager(repo); const received: AgentRunInput[] = []; - const ctx = await prepareContext({ taskId: 'cap-skip', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'cap-skip', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const stages: PipelineStage[] = [ { name: 'simplifier', promptTemplate: 'simplify', stageCostFraction: 0.1, onStageBudgetExceeded: 'skip' }, { name: 'reviewer', promptTemplate: 'review' }, @@ -1204,7 +1204,7 @@ describe('per-stage budget cap', () => { it('provider-ignored cap: orchestrator post-stage check still applies continue policy', async () => { // Simulates Codex/Cursor ignoring maxBudgetUsd: the agent returns costUsd above the cap. const wm = new WorktreeManager(repo); - const ctx = await prepareContext({ taskId: 'cap-advisory', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'cap-advisory', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const stages: PipelineStage[] = [ { name: 'a', promptTemplate: 'a', stageCostFraction: 0.2, onStageBudgetExceeded: 'continue' }, { name: 'b', promptTemplate: 'b' }, @@ -1224,7 +1224,7 @@ describe('per-stage budget cap', () => { it('resumeUntilComplete: per-stage cap stops resumes when aggregate cost exceeds cap', async () => { const wm = new WorktreeManager(repo); - const ctx = await prepareContext({ taskId: 'cap-resume', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'cap-resume', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); // effectiveCap = $1.0 * 0.3 = $0.3. Each call costs $0.15. // Primary: stageCost = $0.15. Resume 1: stageCost = $0.30. // Resume 2 condition: stageCost ($0.30) < effectiveCap ($0.30)? No → loop stops. @@ -1250,7 +1250,7 @@ describe('per-stage budget cap', () => { it('PREVIOUS_STAGE_TRUNCATED is true when prior stage exited non-completed', async () => { const wm = new WorktreeManager(repo); const received: AgentRunInput[] = []; - const ctx = await prepareContext({ taskId: 'cap-trunc', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'cap-trunc', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); // Stage 'a': maxTurns:1, agent returns turns=1 → exitReason='maxTurns' (not 'completed'). // Stage 'b': should see PREVIOUS_STAGE_TRUNCATED='true'. const agent: AgentProvider = { diff --git a/src/pipeline/remote-branch.test.ts b/src/pipeline/remote-branch.test.ts index 9d5ea197..f23cf501 100644 --- a/src/pipeline/remote-branch.test.ts +++ b/src/pipeline/remote-branch.test.ts @@ -48,7 +48,7 @@ function makeSandbox(): IsolatedSandboxProvider { describe('publishForReview', () => { it('pushes the branch and opens a PR via the injected runner', async () => { const wm = new WorktreeManager(repo, undefined, () => 'r1'); - const ctx = await prepareContext({ taskId: 'pub', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'pub', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const calls: Array<{ file: string; args: string[] }> = []; const runner = async (file: string, args: string[]): Promise => { calls.push({ file, args }); @@ -69,9 +69,25 @@ describe('publishForReview', () => { await disposeContext(ctx); }); + it('fetches and targets the base the context was prepared for, never a default (#446)', async () => { + await execa('git', ['branch', 'dev'], { cwd: repo }); + const wm = new WorktreeManager(repo, undefined, () => 'r-dev'); + const ctx = await prepareContext({ taskId: 'pub-dev', localRepoPath: repo, baseBranch: 'dev', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); + const calls: string[][] = []; + const runner = async (file: string, args: string[]): Promise => { + calls.push([file, ...args]); + if (args[0] === 'ls-remote') throw new Error('exit 2'); + return file === 'gh' ? 'https://github.com/o/r/pull/44' : ''; + }; + await publishForReview(ctx, { title: 'PR', runner }); + expect(calls).toContainEqual(['git', 'fetch', '--end-of-options', 'origin', 'dev']); + expect(calls.at(-1)).toEqual(expect.arrayContaining(['pr', 'create', '--base', 'dev'])); + await disposeContext(ctx); + }); + it('rebases onto the remote base, then pushes, when the base moved during the run (#423)', async () => { const wm = new WorktreeManager(repo, undefined, () => 'r2'); - const ctx = await prepareContext({ taskId: 'pub2', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'pub2', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const calls: string[][] = []; let ghArgs: string[] = []; const runner = async (file: string, args: string[]): Promise => { @@ -99,7 +115,7 @@ describe('publishForReview', () => { it('still pushes when the base cannot be fetched (no remote / offline)', async () => { const wm = new WorktreeManager(repo, undefined, () => 'r3'); - const ctx = await prepareContext({ taskId: 'pub3', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'pub3', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const calls: string[][] = []; const runner = async (file: string, args: string[]): Promise => { if (file === 'gh') return 'https://github.com/o/r/pull/44'; @@ -116,7 +132,7 @@ describe('publishForReview', () => { it('publishForReview with glab calls glab mr create with gitlab flags', async () => { const wm = new WorktreeManager(repo, undefined, () => 'r1'); - const ctx = await prepareContext({ taskId: 'gl-test', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'gl-test', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const calls: Array<{ file: string; args: string[]; cwd: string }> = []; const runner = async (file: string, args: string[], cwd: string): Promise => { calls.push({ file, args, cwd }); @@ -144,7 +160,7 @@ describe('publishForReview', () => { it('keeps a line of a GitLab MR description from running as a quick action', async () => { const wm = new WorktreeManager(repo, undefined, () => 'r1'); - const ctx = await prepareContext({ taskId: 'qa-desc', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'qa-desc', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const calls: string[][] = []; await publishForReview(ctx, { title: 'MR', @@ -162,7 +178,7 @@ describe('publishForReview', () => { it('lists the CI config copy-back dropped, with sandbox-chosen names reduced to plain characters', async () => { const wm = new WorktreeManager(repo, undefined, () => 'r1'); - const ctx = await prepareContext({ taskId: 'ci-note', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'ci-note', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const bodyOf = async (): Promise => { const calls: string[][] = []; await publishForReview(ctx, { @@ -208,7 +224,7 @@ describe('pushAuthConfigArgs', () => { describe('pushToExistingBranch', () => { it('with pushToken set, prepends the extraheader override before push', async () => { const wm = new WorktreeManager(repo, undefined, () => 'r1'); - const ctx = await prepareContext({ taskId: 'push-token', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'push-token', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const calls: Array<{ file: string; args: string[] }> = []; const runner = async (file: string, args: string[]): Promise => { calls.push({ file, args }); @@ -230,7 +246,7 @@ describe('pushToExistingBranch', () => { it('with pushToken absent, argv is exactly the baseline (no -c prefix)', async () => { const wm = new WorktreeManager(repo, undefined, () => 'r1'); - const ctx = await prepareContext({ taskId: 'push-notoken', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'push-notoken', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const calls: Array<{ file: string; args: string[] }> = []; const runner = async (file: string, args: string[]): Promise => { calls.push({ file, args }); @@ -243,7 +259,7 @@ describe('pushToExistingBranch', () => { it('redacts the base64 credential from a push failure when a token is in use', async () => { const wm = new WorktreeManager(repo, undefined, () => 'r1'); - const ctx = await prepareContext({ taskId: 'push-fail', localRepoPath: repo, sandbox: makeSandbox() }, { worktrees: wm }); + const ctx = await prepareContext({ taskId: 'push-fail', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox() }, { worktrees: wm }); const b64 = Buffer.from('x-access-token:TOK').toString('base64'); const runner = async (): Promise => { throw new Error(`git push failed: -c http.https://github.com/.extraheader=AUTHORIZATION: basic ${b64}`); diff --git a/src/pipeline/remote-branch.ts b/src/pipeline/remote-branch.ts index 5da839c6..ee3fa882 100644 --- a/src/pipeline/remote-branch.ts +++ b/src/pipeline/remote-branch.ts @@ -18,7 +18,6 @@ const defaultRunner: CommandRunner = async (file: string, args: string[], cwd: s export interface PublishOptions { title: string; body?: string; - baseBranch?: string; /** Git identity for the pre-push rebase (same default as commitStage). */ authorName?: string; authorEmail?: string; @@ -113,15 +112,18 @@ export interface RebaseOntoRemoteBaseOptions { } /** - * Rebase the task branch onto the remote base when the base moved during the run. The worktree is cut - * from the LOCAL base (on Actions: the event SHA), so a commit that lands on the remote base mid-run — - * typically a Dependabot workflow bump — leaves the branch behind. GitHub compares a NEW branch's - * workflow files against the default branch, so a stale `.github/workflows/*` is then rejected as a - * workflow update the token may not make (#423), even though the agent never touched those files. - * Never worse than pushing as-is: every git failure here is logged and the push proceeds unchanged - * (only the stale-workflow case is then still rejected by GitHub, exactly as before). Compares against - * FETCH_HEAD, which `git fetch ` always writes (a single-branch clone creates no - * `refs/remotes//` for another base). Returns true when the branch was rebased. + * Rebase the task branch onto the remote base when the branch is behind it. The worktree is cut from + * RunContext.startRef (the base as origin had it when the run began, or the local copy), so a commit + * that lands on the remote base mid-run — typically a Dependabot workflow bump — leaves the branch + * behind. GitHub compares a NEW branch's workflow files against the default branch, so a stale + * `.github/workflows/*` is then rejected as a workflow update the token may not make (#423), even + * though the agent never touched those files. Never worse than pushing as-is: every git failure here + * is logged and the push proceeds unchanged (only the stale-workflow case is then still rejected by + * GitHub, exactly as before). Compares HEAD, not startRef, against FETCH_HEAD: a reused branch was + * cut from an older base than the one its run resolved, and it is the branch's own distance that + * decides whether the push would be stale. FETCH_HEAD is what `git fetch ` always + * writes (a single-branch clone creates no `refs/remotes//` for another base). Returns + * true when the branch was rebased. */ export async function rebaseOntoRemoteBase(run: CommandRunner, cwd: string, opts: RebaseOntoRemoteBaseOptions): Promise { assertSafeBaseBranch(opts.base); // `main:refs/heads/x` or `+main` would make the fetch a writing refspec @@ -188,7 +190,8 @@ export async function publishForReview(ctx: RunContext, opts: PublishOptions): P // rejects Vanguard's `vanguard/…` branch prefix). The remote enforces no such rule; this is a local // husky gate, redundant with Vanguard's own review + the PR's CI. const remote = opts.remote ?? 'origin'; - const base = opts.baseBranch ?? 'main'; + // The base comes from the context: the same branch the worktree was prepared for, no default here. + const base = ctx.baseBranch; const rebased = await rebaseOntoRemoteBase(run, ctx.worktreePath, { remote, base, @@ -212,7 +215,7 @@ export async function publishForReview(ctx: RunContext, opts: PublishOptions): P args = [ 'mr', 'create', '--source-branch', ctx.branch, - '--target-branch', opts.baseBranch ?? 'main', + '--target-branch', base, '--title', opts.title, '--description', neutralizeQuickActions(body), ]; @@ -221,7 +224,7 @@ export async function publishForReview(ctx: RunContext, opts: PublishOptions): P args = [ 'pr', 'create', '--head', ctx.branch, - '--base', opts.baseBranch ?? 'main', + '--base', base, '--title', opts.title, '--body', body, ]; diff --git a/src/runners/deliver-change.test.ts b/src/runners/deliver-change.test.ts index 050c2ea1..5952dc69 100644 --- a/src/runners/deliver-change.test.ts +++ b/src/runners/deliver-change.test.ts @@ -43,7 +43,7 @@ function makeSandbox(content?: string): IsolatedSandboxProvider { } async function withWork(content: string | undefined, run: (ctx: Awaited>) => Promise): Promise { - const ctx = await prepareContext({ taskId: 'dc', localRepoPath: repo, sandbox: makeSandbox(content) }, { worktrees: new WorktreeManager(repo) }); + const ctx = await prepareContext({ taskId: 'dc', localRepoPath: repo, baseBranch: 'main', start: 'base', sandbox: makeSandbox(content) }, { worktrees: new WorktreeManager(repo) }); try { // Simulate the stage sync: write what the sandbox "produced" into the worktree. if (content !== undefined) await writeFile(join(ctx.worktreePath, 'feature.txt'), content); @@ -111,7 +111,7 @@ describe('deliverChange', () => { const r = await deliverChange(ctx, { taskId: 'o/r#7', commitMessage: 'feat: thing\n\nCloses #7', - closingKeywordBase: 'main', + closingKeywordScan: true, target: { kind: 'new-pr', title: 'thing (o/r#7)', diff --git a/src/runners/deliver-change.ts b/src/runners/deliver-change.ts index 09592e67..75aa7000 100644 --- a/src/runners/deliver-change.ts +++ b/src/runners/deliver-change.ts @@ -27,7 +27,6 @@ export type DeliveryTarget = /** Built after the commit so it can carry the closing-keyword leaks found in the new commits. */ body: (info: { commitLeaks: CommitClosingLeak[] }) => string; draft?: boolean; - baseBranch?: string; cli?: PublishOptions['cli']; /** Injected git/gh runner (tests). */ runner?: CommandRunner; @@ -47,8 +46,8 @@ export interface DeliverChangeOptions { /** White-label identity for the commit and the pre-push rebase; default Vanguard. */ commitAuthor?: CommitAuthor; target: DeliveryTarget; - /** Scan the new commits for `Closes #N` against this base (a partial delivery must not auto-close). */ - closingKeywordBase?: string; + /** Scan the new commits (`ctx.startRef..HEAD`) for `Closes #N` (a partial delivery must not auto-close). */ + closingKeywordScan?: boolean; } export type DeliverChangeResult = @@ -92,8 +91,8 @@ export async function deliverChange(ctx: RunContext, opts: DeliverChangeOptions) // A rebase merge closes the issue per commit message regardless of the PR body, so a partial // result surfaces any commit-level `Closes #N` as a blocking warning in the body. - const commitLeaks = opts.closingKeywordBase !== undefined - ? scanCommitClosingKeywords(await ctx.wm.commitMessages(ctx.worktreePath, opts.closingKeywordBase), opts.taskId) + const commitLeaks = opts.closingKeywordScan === true + ? scanCommitClosingKeywords(await ctx.wm.commitMessages(ctx.worktreePath, ctx.startRef), opts.taskId) : []; const { target } = opts; @@ -102,7 +101,6 @@ export async function deliverChange(ctx: RunContext, opts: DeliverChangeOptions) title: target.title, body: target.body({ commitLeaks }), ...(target.draft !== undefined ? { draft: target.draft } : {}), - ...(target.baseBranch !== undefined ? { baseBranch: target.baseBranch } : {}), ...(target.cli !== undefined ? { cli: target.cli } : {}), ...(target.runner !== undefined ? { runner: target.runner } : {}), // Same identity as the commit: the pre-push rebase replays the commits. diff --git a/src/runners/revise-pr.ts b/src/runners/revise-pr.ts index 2bf4592f..d85a4a06 100644 --- a/src/runners/revise-pr.ts +++ b/src/runners/revise-pr.ts @@ -1,6 +1,5 @@ import { mkdir, writeFile } from 'node:fs/promises'; import { dirname, resolve } from 'node:path'; -import { execa } from 'execa'; import { parsePullRequestRef, fetchPullRequestForReview, postPullRequestReview, commentPullRequest } from './pr-review.js'; import { fetchPullRequestFeedback, @@ -19,6 +18,7 @@ import { } from './pr-feedback.js'; import type { FeedbackItem } from './pr-feedback.js'; import { prepareContext, disposeContext } from '../core/vanguard.js'; +import { baseBranchOrDefault } from '../core/base-branch.js'; import { literalPrompt } from '../context/prompt-engine.js'; import { resolveVerifyCommand, runVerification, renderVerificationFeedback } from '../pipeline/verify.js'; import { reviewRequestBody } from './review-body.js'; @@ -108,8 +108,8 @@ export interface ReviseGithubPrDeps extends ProviderChoice { /** Injected CommandRunner for git push (pushToExistingBranch). */ _pushRunner?: CommandRunner; /** - * Override the baseBranch passed to prepareContext. When set, the git fetch step is skipped. - * Use in tests to point at a local branch instead of origin/. + * Start the worktree from this branch (a 'base' start that keeps a local copy that is ahead) instead + * of fetching the PR head. Use in tests, on a repo without an origin, to point at a local branch. */ _baseBranch?: string; } @@ -225,23 +225,22 @@ export async function runRevisePullRequest(prRef: string, deps: ReviseGithubPrDe ...(deps.network !== undefined ? { network: deps.network } : {}), }); - // Fetch the PR branch from origin so the worktree starts from the PR head, not main. This holds - // for a same-repo PR only: a fork PR's head is not on origin, so a same-named origin branch, or - // nothing, is fetched instead. The head ref name is author-controlled: --end-of-options keeps a - // "-"-led name from parsing as a git option, and the full refs/heads/ source keeps a "+"-led name - // (legal in git) from reading as a force refspec. - let baseBranch: string; - if (deps._baseBranch !== undefined) { - baseBranch = deps._baseBranch; - } else { - if (pr.headRefName === '') throw new VanguardError(`PR ${target.repoSlug}#${target.number} has no head ref to fetch`); - await execa('git', ['fetch', '--end-of-options', 'origin', `refs/heads/${pr.headRefName}`], { cwd: deps.repoPath }); - baseBranch = 'FETCH_HEAD'; + if (deps._baseBranch === undefined && pr.headRefName === '') { + throw new VanguardError(`PR ${target.repoSlug}#${target.number} has no head ref to fetch`); } - const taskId = `revise-pr-${target.repoSlug.replace(/[^a-zA-Z0-9]/g, '-')}-${target.number}`; const ctx = await prepareContext( - { taskId, localRepoPath: deps.repoPath, sandbox, baseBranch, agentName: agents.agent.name }, + { + taskId, + localRepoPath: deps.repoPath, + sandbox, + agentName: agents.agent.name, + // The worktree starts from the PR head as fetched from origin, not the base (see prepareContext + // for the same-repo caveat). Tests point it at a local branch instead, which has no origin. + ...(deps._baseBranch !== undefined + ? { baseBranch: deps._baseBranch, start: 'base' as const, keepLocalIfAhead: true } + : { baseBranch: baseBranchOrDefault(pr.baseRefName, `PR ${target.repoSlug}#${target.number}`), start: { prHead: pr.headRefName } }), + }, { ...(deps._worktrees !== undefined ? { worktrees: deps._worktrees } : {}) }, ); try { diff --git a/src/runners/source-adapter.test.ts b/src/runners/source-adapter.test.ts index ec7c3f6b..bafa1476 100644 --- a/src/runners/source-adapter.test.ts +++ b/src/runners/source-adapter.test.ts @@ -121,18 +121,15 @@ const { wmDiff, wmCommitMessages } = vi.hoisted(() => ({ const { runAgent } = vi.hoisted(() => ({ runAgent: vi.fn() })); // The agent pass resolves the base against origin before cutting the worktree; keep it a pure pass-through here. -vi.mock('../core/base-branch.js', async (importOriginal) => ({ - ...(await importOriginal()), - // Production returns the remote tip's SHA; a recognisable stand-in keeps the assertions honest. - resolveRemoteBaseRef: vi.fn(async (_repo: string, base: string) => `0000sha-${base}`), -})); import { prepareContext } from '../core/vanguard.js'; -import { resolveRemoteBaseRef } from '../core/base-branch.js'; vi.mock('../core/vanguard.js', () => ({ prepareContext: vi.fn(async () => ({ taskId: 'gl-1', sandbox: {}, worktreePath: '/wt', + baseBranch: 'main', + // Production records the SHA the worktree was cut from; a recognisable stand-in keeps the assertions honest. + startRef: '0000sha', wm: { diff: wmDiff, commitMessages: wmCommitMessages }, })), disposeContext: vi.fn(async () => {}), @@ -228,16 +225,15 @@ describe('runSourcedIssue', () => { scanForSecrets.mockImplementation(actual.scanForSecrets); }); - it('cuts the worktree from the resolved remote base (#429) and skips resolution on --reuse', async () => { + it('starts from the remote base (#429), or from the reused branch on --reuse, and says which (#446)', async () => { const adapter = fakeAdapter([], STAGES); - await runSourcedIssue('group/project#1', { repoPath: '/repo', baseBranch: 'develop' }, adapter); - expect(vi.mocked(resolveRemoteBaseRef)).toHaveBeenCalledWith('/repo', 'develop', {}); - expect(vi.mocked(prepareContext).mock.calls.at(-1)?.[0]).toEqual(expect.objectContaining({ baseBranch: '0000sha-develop' })); + await runSourcedIssue('group/project#1', { repoPath: '/repo', baseBranch: 'develop', keepLocalIfAhead: false }, adapter); + expect(vi.mocked(prepareContext).mock.calls.at(-1)?.[0]).toEqual(expect.objectContaining({ baseBranch: 'develop', start: 'base', keepLocalIfAhead: false })); - vi.mocked(resolveRemoteBaseRef).mockClear(); await runSourcedIssue('group/project#1', { repoPath: '/repo', reuse: true }, fakeAdapter([], STAGES)); - expect(vi.mocked(resolveRemoteBaseRef)).not.toHaveBeenCalled(); - expect(vi.mocked(prepareContext).mock.calls.at(-1)?.[0]).toEqual(expect.objectContaining({ baseBranch: 'main', reuse: true })); + const reused = vi.mocked(prepareContext).mock.calls.at(-1)?.[0]; + expect(reused).toEqual(expect.objectContaining({ baseBranch: 'main', start: 'reuse' })); + expect(reused).not.toHaveProperty('keepLocalIfAhead'); }); it('passes the pushed (possibly rebased) head to publishVerdict, not the SHA commitStage returned (#423)', async () => { @@ -586,12 +582,12 @@ describe('runSourcedIssue', () => { expect(msg.replace('feat: ', '')).not.toMatch(/[A-Z]/); // fully lower-case → passes commitlint subject-case }); - it('--base targets the PR at the given base branch', async () => { + it('--base is the branch the context is prepared for (the PR targets it via ctx.baseBranch)', async () => { const adapter = fakeAdapter([], STAGES); await runSourcedIssue('group/project#1', { repoPath: '/repo', baseBranch: 'dev' }, adapter); - const publishOpts = publishForReview.mock.calls[0]?.[1] as { baseBranch?: string }; - expect(publishOpts.baseBranch).toBe('dev'); + expect(vi.mocked(prepareContext).mock.calls[0]?.[0]).toEqual(expect.objectContaining({ baseBranch: 'dev' })); + expect(publishForReview.mock.calls[0]?.[1]).not.toHaveProperty('baseBranch'); }); it('persists stage outcomes WITHOUT a url and opens no PR on the no-commit early return', async () => { @@ -901,13 +897,13 @@ describe('runSourcedIssue', () => { expect(body).toContain(`\`Closes ${task.id}\``); }); - it('scans commit messages from the run base branch, not main', async () => { + it('scans commit messages from the start commit recorded on the context, not main', async () => { vi.mocked(resolveVerifyCommand).mockResolvedValueOnce('npm test'); vi.mocked(runVerification).mockResolvedValueOnce({ passed: false } as never); await runSourcedIssue('group/project#1', { repoPath: '/repo', baseBranch: 'master' }, fakeAdapter([], STAGES)); - expect(wmCommitMessages).toHaveBeenCalledWith('/wt', '0000sha-master'); // the resolved remote base, same commit the worktree was cut from + expect(wmCommitMessages).toHaveBeenCalledWith('/wt', '0000sha'); // ctx.startRef: the very commit the worktree was cut from }); it('omits the commit-leak warning on a full green pass', async () => { diff --git a/src/runners/source-adapter.ts b/src/runners/source-adapter.ts index bc3c9045..bcbf3836 100644 --- a/src/runners/source-adapter.ts +++ b/src/runners/source-adapter.ts @@ -3,7 +3,7 @@ import { taskToVariables } from '../tasks/fetcher.js'; import { DockerSandboxProvider, sandboxImage } from '../sandbox/docker.js'; import { sandboxResourceLimits } from '../sandbox/limits.js'; import { selectAgents, forcedProviderModel } from '../agents/registry.js'; -import { resolveRemoteBaseRef } from '../core/base-branch.js'; +import { DEFAULT_BASE_BRANCH } from '../core/base-branch.js'; import { prepareContext, disposeContext } from '../core/vanguard.js'; import { probeTaskDifficulty, decisionProbeConfig } from '../core/decision-probe.js'; import { decisionModelConfig, decisionEgressAllowed, decisionModelMissing, DECISION_MODEL_DEFAULT, type DecisionModelConfig } from '../core/decision-model.js'; @@ -379,12 +379,6 @@ export async function runSourcedIssue( config: decisionProbeConfig(process.env, { whiteLabel }), ...(deps.signal !== undefined ? { signal: deps.signal } : {}), }); - // Cut from origin's copy of the base when it is ahead of the local one (on Actions the checkout is - // the event SHA and main may already have moved), so the sandbox, verification and review see the - // tree the PR will land on (#423). A reused branch keeps its own history. - const baseRef = deps.reuse === true - ? (deps.baseBranch ?? 'main') - : await resolveRemoteBaseRef(deps.repoPath, deps.baseBranch ?? 'main', deps.keepLocalIfAhead !== undefined ? { keepLocalIfAhead: deps.keepLocalIfAhead } : {}); const ctx = await prepareContext( { taskId: adapter.taskId(task), @@ -393,8 +387,12 @@ export async function runSourcedIssue( agentName: agents.agent.name, ...(agents.reviewAgent !== undefined ? { reviewAgentName: agents.reviewAgent.name } : {}), ...(agents.fallbackAgent !== undefined ? { fallbackAgentName: agents.fallbackAgent.name } : {}), - ...(deps.reuse !== undefined ? { reuse: deps.reuse } : {}), - baseBranch: baseRef, + // Cut from origin's copy of the base when it is ahead of the local one (on Actions the checkout is + // the event SHA and main may already have moved), so the sandbox, verification and review see the + // tree the PR will land on (#423). A reused branch keeps its own history. + baseBranch: deps.baseBranch ?? DEFAULT_BASE_BRANCH, + start: deps.reuse === true ? 'reuse' : 'base', + ...(deps.keepLocalIfAhead !== undefined ? { keepLocalIfAhead: deps.keepLocalIfAhead } : {}), ...(whiteLabel ? { branchPrefix: 'feat/', branchId: branchIdFromTaskId(adapter.taskId(task)) } : {}), }, skills !== undefined ? { skills } : {}, @@ -562,12 +560,11 @@ export async function runSourcedIssue( ? conventionalCommitMessage(task.title, adapter.taskId(task)) : `feat: ${task.title} (${task.id})`, ...(deps.commitAuthor !== undefined ? { commitAuthor: deps.commitAuthor } : {}), - ...(partial ? { closingKeywordBase: baseRef } : {}), + ...(partial ? { closingKeywordScan: true } : {}), target: { kind: 'new-pr', title: `${task.title} (${task.id})`, draft: true, - ...(deps.baseBranch !== undefined ? { baseBranch: deps.baseBranch } : {}), ...(adapter.reviewCli !== undefined ? { cli: adapter.reviewCli } : {}), body: ({ commitLeaks }) => whiteLabel ? baseBody diff --git a/src/runners/spec.test.ts b/src/runners/spec.test.ts index 57f6bd61..cfb6f1a8 100644 --- a/src/runners/spec.test.ts +++ b/src/runners/spec.test.ts @@ -1,8 +1,9 @@ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { mkdtemp, rm, writeFile, readdir } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { execa } from 'execa'; +import { WorktreeManager } from '../worktree/manager.js'; import { specOnce, runLoopV1, @@ -11,7 +12,7 @@ import { linearWatchPrimitives, githubIssueWatchPrimitives, } from './watch.js'; -import { runSpecGenerator, resolveSpecBaseRef } from './spec.js'; +import { runSpecGenerator } from './spec.js'; import { parseSpecManifest } from '../pipeline/conformance-gate.js'; import { GITHUB_CLAIMED_LABEL, GITHUB_REVIEW_LABEL, GITHUB_SPEC_CLAIMED_LABEL } from '../github-labels.js'; import type { SpecWatchPrimitives, WatchPrimitives, GenerateSpec } from './watch.js'; @@ -586,72 +587,47 @@ function recordingSpecAgent(captured: { model?: string | undefined; maxTurns?: n }; } -describe('resolveSpecBaseRef', () => { +describe('runSpecGenerator base', () => { const dirs: string[] = []; const mk = async (prefix: string): Promise => { const d = await mkdtemp(join(tmpdir(), prefix)); dirs.push(d); return d; }; - const commit = (cwd: string, msg: string): Promise => - execa('git', ['-c', 'user.email=t@t', '-c', 'user.name=t', 'commit', '-m', msg], { cwd }); + const commit = async (cwd: string, msg: string): Promise => { + await writeFile(join(cwd, 'f.txt'), msg); + await execa('git', ['add', '.'], { cwd }); + await execa('git', ['-c', 'user.email=t@t', '-c', 'user.name=t', 'commit', '-m', msg], { cwd }); + }; afterEach(async () => { await Promise.all(dirs.splice(0).map((d) => rm(d, { recursive: true, force: true }))); }); - it('fetches origin and cuts from origin/ when the local base is behind', async () => { + it('researches against origin/ even when the local base is ahead (a spec never trusts a local copy)', async () => { const origin = await mk('vg-origin-'); await execa('git', ['init', '-b', 'main'], { cwd: origin }); - await writeFile(join(origin, 'f.txt'), 'v1'); - await execa('git', ['add', '.'], { cwd: origin }); await commit(origin, 'v1'); - const clone = await mk('vg-clone-'); - await execa('git', ['clone', origin, clone]); - - // Advance origin past the clone's local main — the stale-baseline scenario. - await writeFile(join(origin, 'f.txt'), 'v2'); - await execa('git', ['add', '.'], { cwd: origin }); - await commit(origin, 'v2'); - - const staleLocal = (await execa('git', ['rev-parse', 'main'], { cwd: clone })).stdout; - const originHeadNow = (await execa('git', ['rev-parse', 'main'], { cwd: origin })).stdout; - expect(await resolveSpecBaseRef(clone, 'main')).toBe(originHeadNow); - - const cut = (await execa('git', ['rev-parse', 'origin/main'], { cwd: clone })).stdout; - const originHead = (await execa('git', ['rev-parse', 'main'], { cwd: origin })).stdout; - expect(cut).toBe(originHead); // fetched the newer commit - expect(cut).not.toBe(staleLocal); // and it's ahead of the stale local main - }); + await execa('git', ['clone', '-q', origin, clone]); + await commit(clone, 'local-only'); // ahead of origin: a `run` outside CI would keep it, the spec must not - it('falls back to the local base when there is no origin remote', async () => { - const repo = await mk('vg-norem-'); - await execa('git', ['init', '-b', 'main'], { cwd: repo }); - await writeFile(join(repo, 'f.txt'), 'x'); - await execa('git', ['add', '.'], { cwd: repo }); - await commit(repo, 'init'); - expect(await resolveSpecBaseRef(repo, 'main')).toBe('refs/heads/main'); - }); - - it('rejects a base that git would parse as an option', async () => { - const repo = await mk('vg-dash-'); - await execa('git', ['init', '-b', 'main'], { cwd: repo }); - await expect(resolveSpecBaseRef(repo, '--upload-pack=false')).rejects.toThrow('cannot start with "-"'); - }); + const wm = new WorktreeManager(clone); + const create = vi.spyOn(wm, 'create'); + const { sandbox } = makeSandbox(); + const task = readyTask('ENG-11'); + const finalText = 'plan \n## Problem\nx\n COMPLETE'; + await runSpecGenerator(task.id, { + auth: { type: 'api', apiKey: 'x' } as never, + repoPath: clone, + fetcher: fakeFetcher({ [task.id]: task }, [task]), + sandboxFactory: () => sandbox, + agent: fakeAgent(finalText), + contextDeps: { worktrees: wm }, + }); - it('keeps a local base that is ahead of origin only when asked to (outside CI); on CI origin wins', async () => { - const origin = await mk('vg-origin-'); - await execa('git', ['init', '-b', 'main'], { cwd: origin }); - await writeFile(join(origin, 'f.txt'), 'v1'); - await execa('git', ['add', '.'], { cwd: origin }); - await commit(origin, 'v1'); - const clone = await mk('vg-clone-'); - await execa('git', ['clone', origin, clone]); - await writeFile(join(clone, 'f.txt'), 'local'); - await execa('git', ['add', '.'], { cwd: clone }); - await commit(clone, 'local-only'); - expect(await resolveSpecBaseRef(clone, 'main', undefined, true)).toBe('refs/heads/main'); - expect(await resolveSpecBaseRef(clone, 'main', undefined, false)).toBe((await execa('git', ['rev-parse', 'refs/remotes/origin/main'], { cwd: clone })).stdout); + const originTip = (await execa('git', ['rev-parse', 'main'], { cwd: origin })).stdout; + expect(create).toHaveBeenCalledOnce(); + expect(create.mock.calls[0]?.[1]).toBe(originTip); // the worktree was cut from origin's commit, not the local one }); }); diff --git a/src/runners/spec.ts b/src/runners/spec.ts index 1f8ad0c3..2449a360 100644 --- a/src/runners/spec.ts +++ b/src/runners/spec.ts @@ -22,7 +22,7 @@ import type { IsolatedSandboxProvider } from '../sandbox/provider.js'; import type { AgentProvider } from '../agents/provider.js'; import type { RunDeps } from '../core/vanguard.js'; import type { VanguardLogger } from '../core/logger.js'; -import { resolveRemoteBaseRef } from '../core/base-branch.js'; +import { DEFAULT_BASE_BRANCH } from '../core/base-branch.js'; /** * Everything needed to research one task and produce its technical specification. Mirrors the subset @@ -86,15 +86,6 @@ function defaultSandboxFactory( }); } -/** - * Resolve the ref the spec's research worktree is cut from: origin's copy of `base` when it is - * ahead of the local one, so the spec is written against the branch as it exists on the remote. - * See resolveRemoteBaseRef. - */ -export async function resolveSpecBaseRef(repoPath: string, base: string, logger?: VanguardLogger, keepLocalIfAhead?: boolean): Promise { - return resolveRemoteBaseRef(repoPath, base, { label: 'spec', ...(logger !== undefined ? { logger } : {}), ...(keepLocalIfAhead !== undefined ? { keepLocalIfAhead } : {}) }); -} - /** * Run the read-only SPEC pass for one task: fetch it, research the codebase in an isolated sandbox via * the tech-spec stage, and return the generated technical specification markdown. This is the front @@ -138,11 +129,6 @@ export async function runSpecGenerator(id: string, deps: RunSpecGeneratorDeps): try { const sandbox = (deps.sandboxFactory ?? ((s) => defaultSandboxFactory(deps, s, providerProxies.openai, injectAnthropicAuth)))(secrets); - // Fetch the base up front so the spec is researched against origin's view of the branch, not a - // stale local checkout (see resolveSpecBaseRef). Always set — defaults to a fetched `main`. - // The spec is written against the branch as it exists on the remote, never a local copy that is - // ahead of it (the very reason a spec diverges from a branch someone else is actively pushing to). - const baseBranch = await resolveSpecBaseRef(deps.repoPath, deps.baseBranch ?? 'main', deps.logger, false); const retrospectiveMemory = await loadRetrospectiveMemory(deps.repoPath); const ctx = await prepareContext( { @@ -150,7 +136,11 @@ export async function runSpecGenerator(id: string, deps: RunSpecGeneratorDeps): localRepoPath: deps.repoPath, sandbox, agentName: agent.name, - baseBranch, + // The spec is researched against origin's view of the base, never a local copy that is ahead of + // it (the very reason a spec diverges from a branch someone else is actively pushing to). + baseBranch: deps.baseBranch ?? DEFAULT_BASE_BRANCH, + start: 'base', + keepLocalIfAhead: false, ...(deps.logger !== undefined ? { logger: deps.logger } : {}), }, deps.contextDeps ?? {},