fix(FN-5627): always rebase behind branches before squash (safety fallback)

The FN-5627 default-threshold fix changed the prerebase threshold default
from 0 (never fire on commit-count) to 1 (fire on any divergence). But
that only affected projects WITHOUT an explicit threshold. Projects with
user-set values like 'prerebaseDivergenceThreshold: 50' continued to
skip prerebase for small divergences (e.g., 4 commits behind), so the
squash built against stale base and update-ref refused non-FF \u2014
producing the same-SHA spurious-concurrent-advance signature that
stranded FN-5626/FN-5628/FN-5633.

Root distinction missed earlier:
- prerebaseDivergenceThreshold is for USER-VISIBLE SEVERITY REPORTING
  (this branch is N commits behind, warn me).
- Engine correctness requires a SAFETY INVARIANT (any branch behind main
  MUST be rebased before squash or update-ref will fail).

These are independent concerns. The safety invariant must not be gated
on the user's threshold.

decideAutoPrerebase() now returns fire=true with reason
'safety-fallback-any-divergence' whenever commitsBehind > 0, after the
hot-file and threshold checks. The threshold path still wins the reason
label when its condition trips, so user-visible severity reporting is
unchanged for non-pathological cases.

Full opt-out remains prerebaseAutoEnabled=false (skips the safety
fallback; user accepts behind-branch merges will fail).
prerebaseDivergenceThreshold=0 is no longer a complete opt-out from the
commit-count gate \u2014 it only suppresses the threshold-based reason label.

Tests (4 updated/new):
- safety-fallback-any-divergence reason added to AutoPrerebaseDecision
- 4 commits behind with threshold=50 fires via safety fallback
- prerebaseAutoEnabled=false respects full opt-out
- threshold trip still wins reason label
- commitsBehind=0 returns no-divergence (unchanged)

Engine suite: 6163 tests pass.

In-flight: FN-5626, FN-5628, FN-5633 manually SQL-reset to mergeRetries=0,
status=null, error=null, transientRecoveryCount=0 so the next merger tick
(after engine restart picks up this code) auto-prerebases via safety
fallback and lands the work. Future occurrences self-heal automatically.

Fusion-Task-Id: FN-5627
This commit is contained in:
gsxdsm
2026-05-28 15:04:41 -07:00
parent b96b0bc79a
commit 694970b2f1
3 changed files with 104 additions and 14 deletions

View File

@@ -44,12 +44,58 @@ describe("decideAutoPrerebase", () => {
expect(decision.reason).toBe("divergence-threshold");
});
it("returns no-divergence when nothing triggers", () => {
const decision = decideAutoPrerebase({ settings, baseCommitSha: "abc", commitsBehind: 5, changedFiles: ["x.ts"], worktrunkEnabled: false });
it("returns no-divergence only when branch is fully up-to-date", () => {
// FN-5627 update: was 'returns no-divergence when nothing triggers'.
// Now safety-fallback fires for any commitsBehind > 0, so a true
// no-divergence requires commitsBehind === 0.
const decision = decideAutoPrerebase({ settings, baseCommitSha: "abc", commitsBehind: 0, changedFiles: [], worktrunkEnabled: false });
expect(decision.fire).toBe(false);
expect(decision.reason).toBe("no-divergence");
});
it("FN-5627: safety fallback fires for any commitsBehind > 0 even when threshold not tripped", () => {
// Root cause of FN-5626/FN-5633 strandings: user's project config had
// `prerebaseDivergenceThreshold: 50` for low-noise PR experience.
// FN-5626 was only 4 commits behind main, so the threshold path didn't
// fire, prerebase skipped, squash built against stale base, update-ref
// refused non-FF — task stranded. The safety fallback ensures any branch
// behind main rebases before squash, regardless of threshold.
const decision = decideAutoPrerebase({
settings, // threshold=50
baseCommitSha: "abc",
commitsBehind: 4,
changedFiles: ["x.ts"],
worktrunkEnabled: false,
});
expect(decision.fire).toBe(true);
expect(decision.reason).toBe("safety-fallback-any-divergence");
expect(decision.commitsBehind).toBe(4);
});
it("FN-5627: safety fallback respects prerebaseAutoEnabled=false opt-out", () => {
const decision = decideAutoPrerebase({
settings: { ...settings, prerebaseAutoEnabled: false },
baseCommitSha: "abc",
commitsBehind: 4,
changedFiles: ["x.ts"],
worktrunkEnabled: false,
});
expect(decision.fire).toBe(false);
expect(decision.reason).toBe("disabled");
});
it("FN-5627: divergence-threshold still wins over safety-fallback when configured threshold is tripped", () => {
const decision = decideAutoPrerebase({
settings, // threshold=50
baseCommitSha: "abc",
commitsBehind: 51,
changedFiles: ["x.ts"],
worktrunkEnabled: false,
});
expect(decision.fire).toBe(true);
expect(decision.reason).toBe("divergence-threshold");
});
it("FN-5627: fires on default threshold (1 commit) when prerebaseDivergenceThreshold is undefined", () => {
// Pre-FN-5627 default behavior: undefined threshold meant 'never fire on
// commit-count'. This left tasks that branched off older main tips with
@@ -74,21 +120,25 @@ describe("decideAutoPrerebase", () => {
expect(decision.commitsBehind).toBe(1);
});
it("FN-5627: respects explicit prerebaseDivergenceThreshold = 0 as opt-out (never fire on commit-count)", () => {
const settingsOptOut = {
it("FN-5627: explicit prerebaseDivergenceThreshold = 0 still fires via safety-fallback (full opt-out requires prerebaseAutoEnabled=false)", () => {
// FN-5627 semantics: `prerebaseDivergenceThreshold = 0` only opts out of
// the threshold-based trigger; the safety fallback still fires when the
// branch is behind, because the alternative is a guaranteed update-ref
// failure. Full opt-out requires `prerebaseAutoEnabled = false`.
const settingsThresholdZero = {
prerebaseAutoEnabled: true,
prerebaseHotFiles: ["AGENTS.md"],
prerebaseDivergenceThreshold: 0,
};
const decision = decideAutoPrerebase({
settings: settingsOptOut,
settings: settingsThresholdZero,
baseCommitSha: "abc",
commitsBehind: 100,
changedFiles: ["x.ts"],
worktrunkEnabled: false,
});
expect(decision.fire).toBe(false);
expect(decision.reason).toBe("no-divergence");
expect(decision.fire).toBe(true);
expect(decision.reason).toBe("safety-fallback-any-divergence");
});
it("FN-5627: default threshold doesn't fire when branch is up-to-date", () => {

View File

@@ -7,7 +7,7 @@ const execAsync = promisify(exec);
export interface AutoPrerebaseDecision {
fire: boolean;
reason: "disabled" | "no-base" | "no-divergence" | "worktrunk-deferred" | "hot-file" | "divergence-threshold";
reason: "disabled" | "no-base" | "no-divergence" | "worktrunk-deferred" | "hot-file" | "divergence-threshold" | "safety-fallback-any-divergence";
commitsBehind: number;
hotMatches: string[];
}
@@ -73,17 +73,33 @@ export function decideAutoPrerebase(input: {
// FN-5627 follow-up: default divergence threshold flipped from 0 ("never
// fire on commit-count") to 1 ("fire when branch is behind by at least 1
// commit"). The legacy default left tasks that branched off an older main
// tip with no recourse — the squash would build against the stale base,
// `git update-ref` would correctly refuse the non-fast-forward advance,
// and the merger would surface `IntegrationBranchConcurrentAdvanceError`
// with a misleading same-SHA pair. Users who want the legacy never-fire
// behavior can explicitly set `prerebaseDivergenceThreshold = 0`.
// commit"). Users who want the legacy never-fire behavior can explicitly
// set `prerebaseDivergenceThreshold = 0`. Threshold remains user-tunable
// for the user-visible severity label ("divergence-threshold" reason).
const threshold = input.settings.prerebaseDivergenceThreshold ?? 1;
if (threshold > 0 && commitsBehind >= threshold) {
return { fire: true, reason: "divergence-threshold", commitsBehind, hotMatches: [] };
}
// FN-5627 safety invariant: even when the user-configurable threshold
// hasn't tripped (e.g., user explicitly set threshold=50 for low-noise
// PR experience), ANY branch behind main MUST be rebased before squash.
// Otherwise the squash commit doesn't descend from current main, and
// `git update-ref refs/heads/<integration> <new> <old>` will refuse the
// non-fast-forward advance — the merger surfaces
// `IntegrationBranchConcurrentAdvanceError` with a misleading same-SHA
// pair (because `observedCurrentSha` was captured from the pre-update
// rev-parse, not post-failure), and the task is stranded at
// `mergeRetries=3`. This safety fallback is the engine-correctness path;
// the threshold above is for user-visible severity reporting only.
//
// The only way to opt out of the safety fallback is
// `prerebaseAutoEnabled = false`, which already short-circuits much earlier
// and means the user has accepted that behind-branch merges will fail.
if (commitsBehind > 0) {
return { fire: true, reason: "safety-fallback-any-divergence", commitsBehind, hotMatches: [] };
}
return { fire: false, reason: "no-divergence", commitsBehind, hotMatches: [] };
}