From 6770bbeef2dc7a48ad6cd3d6d0e37df080643e2d Mon Sep 17 00:00:00 2001 From: saravanakumardb1 Date: Mon, 1 Jun 2026 11:47:33 -0700 Subject: [PATCH] feat(fleet): honor job.autoMerge on ship and surface PR-merge failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit job.autoMerge was persisted but ignored — PR merging fired only when the host set FLEET_SHIP_MERGES_PR=1, and a failed merge was silent (PR left open, no signal). Now: - mergeRunPrOnShip merges when EITHER the job opted in (job.autoMerge) OR the global flag is set (new pure, unit-tested shouldMergePrOnShip gate). Existing global-flag behavior is preserved. - Merge outcomes are surfaced as job events: pr_merged on success (inline or via background retry) and pr_merge_failed when the inline attempt + 4 background retries all fail, so a stuck PR shows on the timeline instead of vanishing. Still fully best-effort and gated (no merge attempted unless opted in), so the real-world side effect only happens when explicitly requested. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../src/modules/fleet/automerge.test.ts | 28 +++++++++++ .../src/modules/fleet/coordinator.ts | 50 ++++++++++++++----- 2 files changed, 66 insertions(+), 12 deletions(-) create mode 100644 services/platform-service/src/modules/fleet/automerge.test.ts diff --git a/services/platform-service/src/modules/fleet/automerge.test.ts b/services/platform-service/src/modules/fleet/automerge.test.ts new file mode 100644 index 00000000..61e358e0 --- /dev/null +++ b/services/platform-service/src/modules/fleet/automerge.test.ts @@ -0,0 +1,28 @@ +/** + * autoMerge gating — `shouldMergePrOnShip` decides whether a shipped job's PR is + * auto-merged. Pure (job.autoMerge OR the FLEET_SHIP_MERGES_PR host flag), so we + * verify the gate without invoking the GitHub CLI. + */ + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { shouldMergePrOnShip } from './coordinator.js'; + +describe('shouldMergePrOnShip', () => { + beforeEach(() => delete process.env.FLEET_SHIP_MERGES_PR); + afterEach(() => delete process.env.FLEET_SHIP_MERGES_PR); + + it('merges when the job opted in via autoMerge', () => { + expect(shouldMergePrOnShip({ autoMerge: true })).toBe(true); + }); + + it('merges when the host enables it globally (FLEET_SHIP_MERGES_PR=1)', () => { + process.env.FLEET_SHIP_MERGES_PR = '1'; + expect(shouldMergePrOnShip({ autoMerge: false })).toBe(true); + expect(shouldMergePrOnShip({ autoMerge: undefined })).toBe(true); + }); + + it('does NOT merge by default (no opt-in, flag off)', () => { + expect(shouldMergePrOnShip({ autoMerge: false })).toBe(false); + expect(shouldMergePrOnShip({ autoMerge: undefined })).toBe(false); + }); +}); diff --git a/services/platform-service/src/modules/fleet/coordinator.ts b/services/platform-service/src/modules/fleet/coordinator.ts index d465b41c..6ea48421 100644 --- a/services/platform-service/src/modules/fleet/coordinator.ts +++ b/services/platform-service/src/modules/fleet/coordinator.ts @@ -737,29 +737,55 @@ async function ghMergePr(prUrl: string): Promise { } } -async function mergeRunPrOnShip(jobId: string, latest: FleetRunDoc | undefined): Promise { - if (process.env.FLEET_SHIP_MERGES_PR !== '1') return; +/** + * Whether a shipped job's PR should be auto-merged: when the job opted in + * (`job.autoMerge`) OR the host enables it globally (`FLEET_SHIP_MERGES_PR=1`). + * Pure (env + job only) so the gating is unit-testable without invoking `gh`. + */ +export function shouldMergePrOnShip(job: Pick): boolean { + return job.autoMerge === true || process.env.FLEET_SHIP_MERGES_PR === '1'; +} + +async function mergeRunPrOnShip(job: FleetJobDoc, latest: FleetRunDoc | undefined): Promise { + if (!shouldMergePrOnShip(job)) return; if (!latest?.prUrl || latest.prState === 'merged') return; const { prUrl, id: runId } = latest; + const { id: jobId, productId } = job; + const recordMerged = async () => { + try { + await repo.updateRun(runId, jobId, { prState: 'merged' }); + await repo.appendEvent({ jobId, productId, type: 'pr_merged', data: { prUrl } }); + } catch { + /* run gone — ignore */ + } + }; // Fast attempt inline (so a healthy proxy merges immediately without delaying ship). if (await ghMergePr(prUrl)) { - await repo.updateRun(runId, jobId, { prState: 'merged' }); + await recordMerged(); return; } // The corporate proxy intermittently 407s GitHub's API. Retry in the BACKGROUND - // with backoff so the ship response is never blocked; mark merged when one lands. + // with backoff so the ship response is never blocked; mark merged when one lands, + // and SURFACE a `pr_merge_failed` event if every attempt fails (so a stuck PR is + // visible on the job timeline instead of silently left open). void (async () => { for (const delayMs of [3_000, 8_000, 20_000, 45_000]) { await new Promise(r => setTimeout(r, delayMs)); if (await ghMergePr(prUrl)) { - try { - await repo.updateRun(runId, jobId, { prState: 'merged' }); - } catch { - /* run gone — ignore */ - } + await recordMerged(); return; } } + try { + await repo.appendEvent({ + jobId, + productId, + type: 'pr_merge_failed', + data: { prUrl, reason: 'gh pr merge failed after inline + 4 background retries' }, + }); + } catch { + /* job gone — ignore */ + } })().catch(() => { /* detached best-effort — never throws */ }); @@ -802,8 +828,8 @@ export async function patchJobFenced( try { // Run-level result mirrors the terminal stage (ungated). const latest = await markLatestRunShipped(jobId); - // Optionally merge the linked PR (flag-gated, best-effort). - await mergeRunPrOnShip(jobId, latest); + // Optionally merge the linked PR (per-job autoMerge or global flag, best-effort). + await mergeRunPrOnShip(job, latest); // Budgets (flag-gated): accrue the run's actual cost, idempotent per run. if (isBudgetsEnabled()) { await accrueSpend(productId, latest?.insights?.costUsd ?? 0, `${jobId}:${job.leaseEpoch}`); @@ -1272,7 +1298,7 @@ export async function operatorAction( if (stage === 'shipped') { try { const latest = await markLatestRunShipped(jobId); - await mergeRunPrOnShip(jobId, latest); + await mergeRunPrOnShip(job, latest); } catch { // best-effort — run bookkeeping never fails the operator action }