diff --git a/packages/fleet/package.json b/packages/fleet/package.json index 73d8050..720b99c 100644 --- a/packages/fleet/package.json +++ b/packages/fleet/package.json @@ -1,6 +1,6 @@ { "name": "@google/jules-fleet", - "version": "0.0.1-experimental.32", + "version": "0.0.1-experimental.33", "type": "module", "description": "Fleet orchestration tools for Jules — analyze, dispatch, merge, init, configure", "repository": { diff --git a/packages/fleet/src/__tests__/helpers/merge-test-harness.ts b/packages/fleet/src/__tests__/helpers/merge-test-harness.ts index c96468b..4e68451 100644 --- a/packages/fleet/src/__tests__/helpers/merge-test-harness.ts +++ b/packages/fleet/src/__tests__/helpers/merge-test-harness.ts @@ -54,6 +54,7 @@ export const BASE_INPUT: MergeInput = { baseBranch: 'main', admin: false, redispatch: false, + dryRun: false, maxCIWaitSeconds: 1, maxRetries: 2, pollTimeoutSeconds: 1, @@ -113,6 +114,7 @@ export class MergeTestHarness { mocks: { updateBranch: ReturnType; merge: ReturnType; + pullsList: ReturnType; pullsGet: ReturnType; pullsUpdate: ReturnType; julesSession: ReturnType; @@ -256,6 +258,7 @@ export class MergeTestHarness { mocks: { updateBranch: updateBranchMock, merge: mergeMock, + pullsList: octokit.rest.pulls.list, pullsGet: pullsGetMock, pullsUpdate: pullsUpdateMock, julesSession: this.julesSessionMock, diff --git a/packages/fleet/src/__tests__/merge-handler-scenarios.test.ts b/packages/fleet/src/__tests__/merge-handler-scenarios.test.ts index f66c897..bb89148 100644 --- a/packages/fleet/src/__tests__/merge-handler-scenarios.test.ts +++ b/packages/fleet/src/__tests__/merge-handler-scenarios.test.ts @@ -262,6 +262,51 @@ describe('Conflict group scenarios', () => { expect(result.data.skipped).toContain(3); } }); + + it('2.4: conflict group + existing reconcile PR → skipped, no new session', async () => { + const { handler, mocks } = new MergeTestHarness() + .withPRs([1, 2]) + .prFiles(1, ['shared.ts']) + .prFiles(2, ['shared.ts']) + .updateBranchResult(1, 'conflict') + .updateBranchResult(2, 'conflict') + .build(); + + // Simulate an existing open reconcile PR that covers this conflict group + mocks.pullsList.mockResolvedValue({ + data: [ + // The existing PRs from withPRs() + { + number: 1, head: { ref: 'branch-1', sha: 'sha-1' }, body: 'PR #1 body', + labels: [{ name: 'fleet-merge-ready' }], + }, + { + number: 2, head: { ref: 'branch-2', sha: 'sha-2' }, body: 'PR #2 body', + labels: [{ name: 'fleet-merge-ready' }], + }, + // An existing open reconcile PR — should trigger the dedup guard + { + number: 99, head: { ref: 'reconcile-fleet-1-2', sha: 'sha-99' }, + body: 'Batch conflict resolution for PRs #1, #2', + title: 'reconcile: merge fleet PRs', + labels: [{ name: 'fleet-merge-ready' }], + user: { login: 'jules-fleet[bot]' }, + }, + ], + }); + + const result = await handler.execute({ ...BASE_INPUT, redispatch: true }); + + expect(result.success).toBe(true); + if (result.success) { + // Should NOT have created a new Jules session for batch resolve + expect(mocks.julesSession).not.toHaveBeenCalled(); + // PRs should be skipped, not redispatched + expect(result.data.redispatched).toHaveLength(0); + expect(result.data.skipped).toContain(1); + expect(result.data.skipped).toContain(2); + } + }); }); // ═══════════════════════════════════════════════════════════════════ diff --git a/packages/fleet/src/merge/handler.ts b/packages/fleet/src/merge/handler.ts index 4b36e83..4978fe9 100644 --- a/packages/fleet/src/merge/handler.ts +++ b/packages/fleet/src/merge/handler.ts @@ -152,12 +152,19 @@ export class MergeHandler implements MergeSpec { ); if (resolveResult.success) { - for (const pr of failedPRs) { - redispatched.push({ - oldPr: pr.number, - sessionId: resolveResult.sessionId, - }); - skipped.push(pr.number); + if (!resolveResult.skipped) { + for (const pr of failedPRs) { + redispatched.push({ + oldPr: pr.number, + sessionId: resolveResult.sessionId, + }); + skipped.push(pr.number); + } + } else { + // Dedup guard fired — just skip, don't redispatch + for (const pr of failedPRs) { + skipped.push(pr.number); + } } } else { // Fall back to per-PR redispatch diff --git a/packages/fleet/src/merge/ops/resolve-conflicts.ts b/packages/fleet/src/merge/ops/resolve-conflicts.ts index f6c9f1b..b59ac20 100644 --- a/packages/fleet/src/merge/ops/resolve-conflicts.ts +++ b/packages/fleet/src/merge/ops/resolve-conflicts.ts @@ -37,6 +37,8 @@ export interface BatchResolveSuccess { success: true; sessionId: string; resolvedPRs: number[]; + /** True if skipped due to existing reconcile PR */ + skipped?: boolean; } export interface BatchResolveFailure { @@ -71,6 +73,18 @@ export async function batchResolveConflicts( sharedFiles, }); + // Dedup guard: skip if there's already an open reconcile PR covering these PRs + const existingReconcile = await findExistingReconcilePR(octokit, owner, repo, conflictingPRs); + if (existingReconcile) { + emit({ + type: 'merge:batch-resolve:skipped', + reason: 'existing-reconcile-pr', + existingPR: existingReconcile, + prNumbers: conflictingPRs.map((p) => p.number), + } as any); + return { success: true, sessionId: '', resolvedPRs: [], skipped: true }; + } + // 1. Fetch diffs for all conflicting PRs (non-fatal per-PR) const diffs = await fetchAllDiffs(octokit, owner, repo, conflictingPRs); @@ -137,11 +151,47 @@ export async function batchResolveConflicts( success: true, sessionId, resolvedPRs: conflictingPRs.map((p) => p.number), + skipped: false, }; } // ── Helpers ───────────────────────────────────────────────────────── +/** + * Checks whether an open reconcile PR already exists that covers + * at least one of the conflicting PRs. If so, returns the PR number; + * otherwise returns null. + */ +async function findExistingReconcilePR( + octokit: Octokit, + owner: string, + repo: string, + conflictingPRs: PR[], +): Promise { + try { + const { data: openPRs } = await octokit.rest.pulls.list({ + owner, + repo, + state: 'open', + }); + const conflictNumbers = new Set(conflictingPRs.map((p) => p.number)); + for (const pr of openPRs) { + const title = (pr as any).title ?? ''; + const body = pr.body ?? ''; + // Match PRs with 'reconcile' in title or 'Batch conflict resolution' in body + const isReconcile = /reconcile/i.test(title) || /batch conflict resolution/i.test(body); + if (!isReconcile) continue; + // Check if this reconcile PR mentions any of the conflicting PRs + const mentionedPRs = [...(body.matchAll(/#(\d+)/g))].map((m) => Number(m[1])); + const overlaps = mentionedPRs.some((n) => conflictNumbers.has(n)); + if (overlaps) return pr.number; + } + return null; + } catch { + return null; // Non-fatal — assume no existing reconcile + } +} + /** * Fetches the diff for each PR. Returns a map of PR number → diff string. * Non-fatal: returns empty string for PRs where the API call fails.