Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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}` });
Expand Down
6 changes: 3 additions & 3 deletions examples/validate-phase3.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ function sandbox(auth: AgentAuth): DockerSandboxProvider {

async function scenarioAdversary(auth: AgentAuth): Promise<void> {
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',
Expand Down Expand Up @@ -81,7 +81,7 @@ async function scenarioAdversary(auth: AgentAuth): Promise<void> {

async function scenarioHitl(auth: AgentAuth): Promise<void> {
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',
Expand Down Expand Up @@ -114,7 +114,7 @@ async function scenarioHitl(auth: AgentAuth): Promise<void> {

async function scenarioBudget(auth: AgentAuth): Promise<void> {
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,
Expand Down
4 changes: 3 additions & 1 deletion src/cli/research.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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',
Expand Down
30 changes: 29 additions & 1 deletion src/cli/review-mr.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion src/cli/review-mr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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, {
Expand Down
31 changes: 30 additions & 1 deletion src/cli/review-pr.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<Command, { kind: 'review-pr' }> = { 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;
Expand Down
5 changes: 4 additions & 1 deletion src/cli/review-pr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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, {
Expand Down
13 changes: 13 additions & 0 deletions src/core/base-branch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'). */
Expand Down
Loading
Loading