feat(FN-4333): emergency hot-fix for post-merge audit blocks
Stop the post-merge audit from parking tasks as `failed` when deterministic merge verification already proved the merged tree. Adds `postMergeAuditMode` project setting (`block` | `warn` | `off`, default `block`) and a verified-tree short-circuit for rebase-strategy overlap-only findings. - packages/engine/src/merger.ts: new `resolvePostMergeAuditAction` helper + audit call site gated on `postMergeAuditMode` and verification-cache lookup. Rebase + overlap-only + verified-tree → pass through with a short-circuit log entry instead of throwing SquashAuditError. - packages/core/src/types.ts, settings-schema.ts, index.ts: new `PostMergeAuditMode` type, `postMergeAuditMode` field on Settings, default `block`, exports `normalizePostMergeAuditMode`. - packages/engine/src/__tests__/post-merge-audit-action.test.ts: unit tests for the decision helper. - packages/dashboard/app/components/SettingsModal.tsx: settings UI row. - .changeset: minor bump for @runfusion/fusion. The FN-3936 silent-drop guard is preserved: duplicate-subject findings still block in `block` mode and squash-strategy audits still block (no equivalent deterministic guarantee). Fusion-Task-Id: FN-4333
This commit is contained in:
gsxdsm
149
packages/engine/src/__tests__/post-merge-audit-action.test.ts
Normal file
149
packages/engine/src/__tests__/post-merge-audit-action.test.ts
Normal file
@@ -0,0 +1,149 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
resolvePostMergeAuditAction,
|
||||
type PostMergeAuditAction,
|
||||
} from "../merger.js";
|
||||
import type {
|
||||
SquashAuditFindings,
|
||||
SquashAuditTouchedFileOverlapFinding,
|
||||
SquashAuditDuplicateSubjectFinding,
|
||||
} from "../merger-squash-audit.js";
|
||||
|
||||
/**
|
||||
* FN-4333 — unit tests for the post-merge audit decision helper.
|
||||
*
|
||||
* `resolvePostMergeAuditAction` decides whether a dirty audit should block
|
||||
* the merge or be passed through. The merger uses this to apply the
|
||||
* deterministic-verification short-circuit + the `postMergeAuditMode`
|
||||
* setting without spinning up real git/store state.
|
||||
*/
|
||||
|
||||
function overlap(file: string): SquashAuditTouchedFileOverlapFinding {
|
||||
return {
|
||||
type: "touched-file-overlap",
|
||||
file,
|
||||
recentMainCommits: [{ sha: "abcdef12", subject: "chore: recent main edit" }],
|
||||
};
|
||||
}
|
||||
|
||||
function duplicateSubject(subject: string): SquashAuditDuplicateSubjectFinding {
|
||||
return { type: "duplicate-subject", subject };
|
||||
}
|
||||
|
||||
function findings(opts: {
|
||||
strategy: "squash" | "rebase";
|
||||
duplicates?: SquashAuditDuplicateSubjectFinding[];
|
||||
overlaps?: SquashAuditTouchedFileOverlapFinding[];
|
||||
}): SquashAuditFindings {
|
||||
const duplicates = opts.duplicates ?? [];
|
||||
const overlaps = opts.overlaps ?? [];
|
||||
const list = [...duplicates, ...overlaps];
|
||||
const base = {
|
||||
parentSha: "0000000000000000000000000000000000000000",
|
||||
lookback: 30,
|
||||
branchSubjects: [],
|
||||
recentMainSubjects: [],
|
||||
duplicateSubjects: duplicates,
|
||||
touchedFiles: overlaps.map((o) => o.file),
|
||||
touchedFileOverlaps: overlaps,
|
||||
findings: list,
|
||||
issueCount: list.length,
|
||||
clean: list.length === 0,
|
||||
};
|
||||
if (opts.strategy === "rebase") {
|
||||
return {
|
||||
...base,
|
||||
strategy: "rebase",
|
||||
rangeBaseSha: "1".repeat(40),
|
||||
rangeHeadSha: "2".repeat(40),
|
||||
auditTargetLabel: "11111111..22222222",
|
||||
};
|
||||
}
|
||||
return {
|
||||
...base,
|
||||
strategy: "squash",
|
||||
squashSha: "3".repeat(40),
|
||||
squashSubject: "feat: squash",
|
||||
auditTargetLabel: "33333333",
|
||||
};
|
||||
}
|
||||
|
||||
describe("resolvePostMergeAuditAction (FN-4333)", () => {
|
||||
it("passes through when audit is clean (defensive default)", () => {
|
||||
const result = resolvePostMergeAuditAction({
|
||||
mode: "block",
|
||||
strategy: "rebase",
|
||||
findings: findings({ strategy: "rebase" }),
|
||||
isTreeVerified: false,
|
||||
});
|
||||
expect(result.action).toBe("pass");
|
||||
});
|
||||
|
||||
it("blocks duplicate-subject findings in block mode regardless of verification", () => {
|
||||
const result = resolvePostMergeAuditAction({
|
||||
mode: "block",
|
||||
strategy: "rebase",
|
||||
findings: findings({
|
||||
strategy: "rebase",
|
||||
duplicates: [duplicateSubject("feat: collide")],
|
||||
overlaps: [overlap("docs/README.md")],
|
||||
}),
|
||||
isTreeVerified: true,
|
||||
});
|
||||
expect(result).toEqual<PostMergeAuditAction>({ action: "block", reason: "mode-block" });
|
||||
});
|
||||
|
||||
it("short-circuits rebase-strategy overlap-only findings when the tree is verified", () => {
|
||||
const result = resolvePostMergeAuditAction({
|
||||
mode: "block",
|
||||
strategy: "rebase",
|
||||
findings: findings({ strategy: "rebase", overlaps: [overlap("docs/README.md")] }),
|
||||
isTreeVerified: true,
|
||||
});
|
||||
expect(result).toEqual<PostMergeAuditAction>({ action: "pass", reason: "verified-short-circuit" });
|
||||
});
|
||||
|
||||
it("does NOT short-circuit overlap-only findings when the tree was not verified", () => {
|
||||
const result = resolvePostMergeAuditAction({
|
||||
mode: "block",
|
||||
strategy: "rebase",
|
||||
findings: findings({ strategy: "rebase", overlaps: [overlap("docs/README.md")] }),
|
||||
isTreeVerified: false,
|
||||
});
|
||||
expect(result).toEqual<PostMergeAuditAction>({ action: "block", reason: "mode-block" });
|
||||
});
|
||||
|
||||
it("does NOT short-circuit squash-strategy overlap-only findings even when verified (no deterministic guarantee)", () => {
|
||||
const result = resolvePostMergeAuditAction({
|
||||
mode: "block",
|
||||
strategy: "squash",
|
||||
findings: findings({ strategy: "squash", overlaps: [overlap("docs/README.md")] }),
|
||||
isTreeVerified: true,
|
||||
});
|
||||
expect(result).toEqual<PostMergeAuditAction>({ action: "block", reason: "mode-block" });
|
||||
});
|
||||
|
||||
it("passes any dirty audit in warn mode", () => {
|
||||
const result = resolvePostMergeAuditAction({
|
||||
mode: "warn",
|
||||
strategy: "squash",
|
||||
findings: findings({
|
||||
strategy: "squash",
|
||||
duplicates: [duplicateSubject("feat: collide")],
|
||||
overlaps: [overlap("packages/dashboard/app/x.tsx")],
|
||||
}),
|
||||
isTreeVerified: false,
|
||||
});
|
||||
expect(result).toEqual<PostMergeAuditAction>({ action: "pass", reason: "mode-warn" });
|
||||
});
|
||||
|
||||
it("prefers verified-short-circuit reason over mode-warn when both would pass", () => {
|
||||
const result = resolvePostMergeAuditAction({
|
||||
mode: "warn",
|
||||
strategy: "rebase",
|
||||
findings: findings({ strategy: "rebase", overlaps: [overlap("a.txt")] }),
|
||||
isTreeVerified: true,
|
||||
});
|
||||
expect(result).toEqual<PostMergeAuditAction>({ action: "pass", reason: "verified-short-circuit" });
|
||||
});
|
||||
});
|
||||
@@ -36,6 +36,7 @@ import {
|
||||
getTaskMergeBlocker,
|
||||
normalizeMergeConflictStrategy,
|
||||
normalizeMergeStrategyOverlapBehavior,
|
||||
normalizePostMergeAuditMode,
|
||||
resolveTaskMergeTarget,
|
||||
resolveTitleSummarizerSettingsModel,
|
||||
resolveAgentPrompt,
|
||||
@@ -53,6 +54,7 @@ import {
|
||||
type AgentPromptsConfig,
|
||||
type CanonicalMergeConflictStrategy,
|
||||
type DirectMergeCommitStrategy,
|
||||
type PostMergeAuditMode,
|
||||
type TaskSourceIssue,
|
||||
type Task,
|
||||
type AutostashOrphanRecord,
|
||||
@@ -4599,6 +4601,58 @@ function shouldRunPostMergeAudit(
|
||||
return (result.autoResolvedCount ?? 0) > 0 || result.attemptsMade === 3;
|
||||
}
|
||||
|
||||
/**
|
||||
* Decide what to do with a dirty post-merge audit (FN-4333 hot-fix).
|
||||
*
|
||||
* Three modes (`postMergeAuditMode` setting):
|
||||
* - `"off"` — caller should skip auditing entirely (handled before this fn).
|
||||
* - `"warn"` — log findings on the agent log but proceed; never throws.
|
||||
* - `"block"` — today's behavior: throw `SquashAuditError` and park the task,
|
||||
* EXCEPT for the deterministic-verification short-circuit:
|
||||
* rebase-strategy + overlap-only findings + a verified merged tree
|
||||
* cannot have produced silent drops (the tree is provably the
|
||||
* rebase output by construction), so we pass through clean.
|
||||
*
|
||||
* Pure / side-effect-free so it is unit-testable without spinning up a real
|
||||
* merger flow. The merger call site uses the returned action to decide whether
|
||||
* to throw or fall through.
|
||||
*/
|
||||
export type PostMergeAuditAction =
|
||||
| { action: "pass"; reason: "verified-short-circuit" | "mode-warn" }
|
||||
| { action: "block"; reason: "mode-block" };
|
||||
|
||||
export function resolvePostMergeAuditAction(opts: {
|
||||
mode: PostMergeAuditMode;
|
||||
strategy: PostMergeAuditStrategy;
|
||||
findings: SquashAuditFindings;
|
||||
isTreeVerified: boolean;
|
||||
}): PostMergeAuditAction {
|
||||
if (opts.findings.clean) {
|
||||
// Caller should never invoke this on a clean audit, but be defensive.
|
||||
return { action: "pass", reason: "mode-warn" };
|
||||
}
|
||||
|
||||
const overlapOnly =
|
||||
opts.findings.duplicateSubjects.length === 0
|
||||
&& opts.findings.touchedFileOverlaps.length > 0;
|
||||
|
||||
// Stage 1 short-circuit: a verified rebase tree with overlap-only findings
|
||||
// cannot have produced silent drops. Pass regardless of mode (warn/block).
|
||||
if (
|
||||
opts.strategy === "rebase"
|
||||
&& overlapOnly
|
||||
&& opts.isTreeVerified
|
||||
) {
|
||||
return { action: "pass", reason: "verified-short-circuit" };
|
||||
}
|
||||
|
||||
if (opts.mode === "warn") {
|
||||
return { action: "pass", reason: "mode-warn" };
|
||||
}
|
||||
|
||||
return { action: "block", reason: "mode-block" };
|
||||
}
|
||||
|
||||
function buildPostMergeAuditBlockingMessage(taskId: string, findings: SquashAuditFindings): string {
|
||||
const riskParts: string[] = [];
|
||||
if (findings.duplicateSubjects.length > 0) {
|
||||
@@ -6665,7 +6719,12 @@ export async function aiMergeTask(
|
||||
const recordedSha = (isEmptyCommit || mergeWasEmpty) ? undefined : commitSha;
|
||||
|
||||
const auditSha = recordedSha;
|
||||
if (auditSha && shouldRunPostMergeAudit(selectedPostMergeAuditStrategy, result, mergeWasEmpty, isEmptyCommit, auditSha)) {
|
||||
const postMergeAuditMode = normalizePostMergeAuditMode(settings.postMergeAuditMode);
|
||||
if (
|
||||
auditSha
|
||||
&& postMergeAuditMode !== "off"
|
||||
&& shouldRunPostMergeAudit(selectedPostMergeAuditStrategy, result, mergeWasEmpty, isEmptyCommit, auditSha)
|
||||
) {
|
||||
const auditFindings = selectedPostMergeAuditStrategy === "rebase" && rebaseMergeBaseSha
|
||||
? await auditSquashMerge({
|
||||
rootDir,
|
||||
@@ -6679,24 +6738,72 @@ export async function aiMergeTask(
|
||||
squashSha: auditSha,
|
||||
});
|
||||
if (!auditFindings.clean) {
|
||||
const auditError = new SquashAuditError(taskId, auditSha, auditFindings);
|
||||
// FN-4333: a verified rebase tree with overlap-only findings cannot have
|
||||
// produced silent drops by construction. Check the verification cache to
|
||||
// detect that case and pass through. `warn` mode passes any dirty audit.
|
||||
let isTreeVerified = false;
|
||||
try {
|
||||
const { stdout: treeShaOut } = await execAsync(
|
||||
`git rev-parse ${auditSha}^{tree}`,
|
||||
{ cwd: rootDir, encoding: "utf-8" },
|
||||
);
|
||||
const treeSha = treeShaOut.trim();
|
||||
if (treeSha) {
|
||||
const cacheHit = store.getVerificationCacheHit(
|
||||
treeSha,
|
||||
effectiveTestCommand ?? "",
|
||||
effectiveBuildCommand ?? "",
|
||||
);
|
||||
isTreeVerified = Boolean(cacheHit);
|
||||
}
|
||||
} catch (err) {
|
||||
mergerLog.warn(
|
||||
`${taskId}: failed to resolve tree sha for audit verification short-circuit: ${err instanceof Error ? err.message : String(err)}`,
|
||||
);
|
||||
}
|
||||
|
||||
const decision = resolvePostMergeAuditAction({
|
||||
mode: postMergeAuditMode,
|
||||
strategy: selectedPostMergeAuditStrategy,
|
||||
findings: auditFindings,
|
||||
isTreeVerified,
|
||||
});
|
||||
|
||||
if (decision.action === "block") {
|
||||
const auditError = new SquashAuditError(taskId, auditSha, auditFindings);
|
||||
await store.appendAgentLog(
|
||||
taskId,
|
||||
auditError.message,
|
||||
"tool_error",
|
||||
formatSquashAuditAgentLog(auditFindings),
|
||||
"merger",
|
||||
);
|
||||
await store.updateTask(taskId, { status: null });
|
||||
throw auditError;
|
||||
}
|
||||
|
||||
const passLabel = decision.reason === "verified-short-circuit"
|
||||
? `${selectedPostMergeAuditStrategy === "rebase" ? "post-rebase" : "post-squash"} audit overlap cleared by deterministic verification`
|
||||
: `${selectedPostMergeAuditStrategy === "rebase" ? "post-rebase" : "post-squash"} audit found ${auditFindings.issueCount} risk(s) — continuing (postMergeAuditMode=warn)`;
|
||||
await store.appendAgentLog(
|
||||
taskId,
|
||||
auditError.message,
|
||||
"tool_error",
|
||||
passLabel,
|
||||
"text",
|
||||
formatSquashAuditAgentLog(auditFindings),
|
||||
"merger",
|
||||
);
|
||||
await store.updateTask(taskId, { status: null });
|
||||
throw auditError;
|
||||
mergerLog.log(`${taskId}: ${passLabel}`);
|
||||
} else {
|
||||
await store.appendAgentLog(
|
||||
taskId,
|
||||
selectedPostMergeAuditStrategy === "rebase" ? "post-rebase range audit clean" : "post-squash audit clean",
|
||||
"text",
|
||||
undefined,
|
||||
"merger",
|
||||
);
|
||||
}
|
||||
await store.appendAgentLog(
|
||||
taskId,
|
||||
selectedPostMergeAuditStrategy === "rebase" ? "post-rebase range audit clean" : "post-squash audit clean",
|
||||
"text",
|
||||
undefined,
|
||||
"merger",
|
||||
);
|
||||
} else if (auditSha && postMergeAuditMode === "off") {
|
||||
mergerLog.log(`${taskId}: post-merge audit skipped (postMergeAuditMode=off)`);
|
||||
}
|
||||
if (isEmptyCommit) {
|
||||
mergerLog.warn(
|
||||
|
||||
Reference in New Issue
Block a user