fix(engine): harden merge finalize SQLite bind and recover bare merge subjects
Two compounding bugs surfaced as `feat(FN-XXXX): merge fusion/fn-XXXX`
commits landing on main:
1. The verification-fix finalize path could bind `undefined` to SQLite
parameter 4 (`commitSha`) of `upsertTaskCommitAssociation` under the
parallel-attempt race, failing the merge over a denormalization
write after the commit had already landed. Centralized both
duplicated callsites into a helper that validates each git output
before binding.
2. Four self-healing/aiMergeTask recovery sites copied
`classification.commit.subject` verbatim into
`mergeDetails.mergeCommitMessage`, persisting the tier-3
`merge ${branch}` fallback when it ended up on the landed commit.
New `regenerateBareMergeSubject` helper detects the bare pattern
and rebuilds a descriptive subject via the AI summarizer. Cosmetic
only — the git commit is not amended.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
@@ -0,0 +1,8 @@
|
||||
---
|
||||
"@fusion/engine": patch
|
||||
---
|
||||
|
||||
Fix two bugs that compounded to produce bare `feat(FN-XXXX): merge fusion/fn-XXXX` merge commits in the dashboard:
|
||||
|
||||
- **`Provided value cannot be bound to SQLite parameter 4` (TypeError) mid-merge**: the verification-fix finalize path called `upsertTaskCommitAssociation` with `commitSha` derived from a `git rev-parse HEAD` whose surrounding exec could reject under the parallel-attempt race, leaving `commitSha` undefined when bound to positional parameter 4. Extracted both duplicated callsites into a `recordCommitAssociationFromHead` helper that catches exec failures and validates each git output is non-empty before binding. The merge no longer fails over a denormalized lookup write when the commit itself landed cleanly.
|
||||
- **Bare-fallback subjects persisted into `mergeDetails.mergeCommitMessage`**: when `buildDeterministicMergeMessage`'s tier-3 fallback (`merge ${branch}`) made it onto a landed commit, the four `classification.commit.subject` / `landedCommit.subject` recovery sites in `self-healing.ts` and `aiMergeTask` copied that bare subject verbatim into `mergeDetails`. Added `regenerateBareMergeSubject` (in a new `merger-bare-subject.ts` module to keep self-healing's import graph narrow) which detects the bare pattern via `BARE_MERGE_SUBJECT_RE` and regenerates a descriptive subject from the landed commit's diff stat via the existing AI commit-subject summarizer. Cosmetic only — the git commit is never amended; the regenerated subject only populates the persisted `mergeDetails` and the in-process `MergeResult`. Gated by `settings.useAiMergeCommitSummary`.
|
||||
74
packages/engine/src/merger-bare-subject.ts
Normal file
74
packages/engine/src/merger-bare-subject.ts
Normal file
@@ -0,0 +1,74 @@
|
||||
import { exec } from "node:child_process";
|
||||
import { promisify } from "node:util";
|
||||
import {
|
||||
resolveTitleSummarizerSettingsModel,
|
||||
summarizeCommitSubject,
|
||||
type Settings,
|
||||
} from "@fusion/core";
|
||||
import { mergerLog } from "./logger.js";
|
||||
|
||||
const execAsync = promisify(exec);
|
||||
|
||||
function quoteArg(value: string): string {
|
||||
return `'${value.replace(/'/g, "'\\''")}'`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Bare-fallback merge subject pattern from `buildDeterministicMergeMessage`'s
|
||||
* tier-3 branch (e.g. `feat(FN-5592): merge fusion/fn-5592`). Used to detect
|
||||
* commits whose subject is the last-resort `merge ${branch}` form so callers
|
||||
* can regenerate a descriptive subject for `mergeDetails.mergeCommitMessage`.
|
||||
*/
|
||||
export const BARE_MERGE_SUBJECT_RE = /^[a-z]+(?:\([^)]+\))?!?:\s*merge\s+\S+\s*$/i;
|
||||
|
||||
/**
|
||||
* If `subject` is the bare tier-3 fallback (`feat(TASK): merge <branch>`),
|
||||
* regenerate a descriptive subject from the landed commit's diff stat via
|
||||
* the AI commit-subject summarizer. Returns the original subject when the
|
||||
* pattern doesn't match, AI summaries are disabled, or regeneration fails.
|
||||
*
|
||||
* Cosmetic only: never amends the git commit. Callers use the result to
|
||||
* populate `mergeDetails.mergeCommitMessage` so dashboards display a
|
||||
* meaningful subject even when the on-commit subject is the bare fallback.
|
||||
*
|
||||
* Lives in its own module (not `merger.ts`) so importers like `self-healing.ts`
|
||||
* don't pay the cost of merger's transitive graph — tests that mock
|
||||
* `node:child_process` narrowly would otherwise break.
|
||||
*/
|
||||
export async function regenerateBareMergeSubject(params: {
|
||||
subject: string | undefined;
|
||||
commitSha: string;
|
||||
branch: string;
|
||||
taskId: string;
|
||||
rootDir: string;
|
||||
settings: Settings;
|
||||
}): Promise<string | undefined> {
|
||||
const { subject, commitSha, branch, taskId, rootDir, settings } = params;
|
||||
if (!subject || !BARE_MERGE_SUBJECT_RE.test(subject)) return subject;
|
||||
if (!settings.useAiMergeCommitSummary) return subject;
|
||||
if (!commitSha) return subject;
|
||||
try {
|
||||
const { stdout: diffStat } = await execAsync(
|
||||
`git show ${quoteArg(commitSha)} --stat --format=`,
|
||||
{ cwd: rootDir, encoding: "utf-8" },
|
||||
);
|
||||
const trimmedStat = diffStat.trim();
|
||||
if (trimmedStat.length === 0) return subject;
|
||||
const resolved = resolveTitleSummarizerSettingsModel(settings);
|
||||
const regenerated = await summarizeCommitSubject(
|
||||
trimmedStat,
|
||||
rootDir,
|
||||
resolved.provider,
|
||||
resolved.modelId,
|
||||
{ branch, taskId },
|
||||
);
|
||||
if (!regenerated) return subject;
|
||||
const prefixMatch = subject.match(/^([a-z]+(?:\([^)]+\))?!?:)/i);
|
||||
const prefix = prefixMatch ? prefixMatch[1] : `feat(${taskId}):`;
|
||||
return `${prefix} ${regenerated}`;
|
||||
} catch (err: unknown) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
mergerLog.warn(`${taskId}: bare-merge-subject regeneration failed (${message})`);
|
||||
return subject;
|
||||
}
|
||||
}
|
||||
@@ -94,6 +94,8 @@ import { classifyTaskWorktree, getRegisteredWorktreeBranches, RemovalReason, rem
|
||||
import { activeSessionRegistry } from "./active-session-registry.js";
|
||||
import { AgentLogger } from "./agent-logger.js";
|
||||
import { mergerLog } from "./logger.js";
|
||||
import { regenerateBareMergeSubject } from "./merger-bare-subject.js";
|
||||
export { regenerateBareMergeSubject, BARE_MERGE_SUBJECT_RE } from "./merger-bare-subject.js";
|
||||
import { isUsageLimitError, checkSessionError, type UsageLimitPauser } from "./usage-limit-detector.js";
|
||||
import { isContextLimitError } from "./context-limit-detector.js";
|
||||
import { withRateLimitRetry } from "./rate-limit-retry.js";
|
||||
@@ -3777,6 +3779,54 @@ async function generateAiMergeSubject(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Capture HEAD's sha, subject, and authored timestamp via git, then persist a
|
||||
* canonical lineage-trailer association. Validates each git output is a
|
||||
* non-empty string before binding to SQLite — `upsertTaskCommitAssociation`
|
||||
* binds `commitSha` to positional parameter 4, and any `undefined` or empty
|
||||
* value would otherwise raise `TypeError: Provided value cannot be bound to
|
||||
* SQLite parameter 4` mid-merge (observed under parallel finalize-attempt
|
||||
* races where one of the three `git` calls aborted before the others).
|
||||
*
|
||||
* Returns silently and logs a warning when validation fails — the association
|
||||
* is a denormalized convenience for lineage lookups, not a correctness
|
||||
* invariant, so a missed write must not block the merge.
|
||||
*/
|
||||
async function recordCommitAssociationFromHead(
|
||||
store: TaskStore,
|
||||
rootDir: string,
|
||||
taskId: string,
|
||||
lineageId: string,
|
||||
): Promise<void> {
|
||||
let sha = "";
|
||||
let subject = "";
|
||||
let authoredAt = "";
|
||||
try {
|
||||
sha = (await execAsync("git rev-parse HEAD", { cwd: rootDir })).stdout.trim();
|
||||
subject = (await execAsync("git log -1 --format=%s HEAD", { cwd: rootDir })).stdout.trim();
|
||||
authoredAt = (await execAsync("git log -1 --format=%aI HEAD", { cwd: rootDir })).stdout.trim();
|
||||
} catch (err: unknown) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
mergerLog.warn(`${taskId}: skipped commit-association write — git inspection failed (${message})`);
|
||||
return;
|
||||
}
|
||||
if (!sha || !subject || !authoredAt) {
|
||||
mergerLog.warn(
|
||||
`${taskId}: skipped commit-association write — empty git output (sha=${sha.length}, subject=${subject.length}, authoredAt=${authoredAt.length})`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
await store.upsertTaskCommitAssociation({
|
||||
taskLineageId: lineageId,
|
||||
taskIdSnapshot: taskId,
|
||||
commitSha: sha,
|
||||
commitSubject: subject,
|
||||
authoredAt,
|
||||
matchedBy: "canonical-lineage-trailer",
|
||||
confidence: "canonical",
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Derive a non-AI subject summary from the branch's step commit log. The log
|
||||
* is `- subj1\n- subj2\n…` (most recent first). The naive "use lines[0]" choice
|
||||
@@ -4435,18 +4485,7 @@ export async function commitOrAmendMergeWithFixes(
|
||||
{ cwd: rootDir, env: mergerCommitEnv() },
|
||||
);
|
||||
if (store && lineageId) {
|
||||
const sha = (await execAsync("git rev-parse HEAD", { cwd: rootDir })).stdout.trim();
|
||||
const subject = (await execAsync("git log -1 --format=%s HEAD", { cwd: rootDir })).stdout.trim();
|
||||
const authoredAt = (await execAsync("git log -1 --format=%aI HEAD", { cwd: rootDir })).stdout.trim();
|
||||
await store.upsertTaskCommitAssociation({
|
||||
taskLineageId: lineageId,
|
||||
taskIdSnapshot: taskId,
|
||||
commitSha: sha,
|
||||
commitSubject: subject,
|
||||
authoredAt,
|
||||
matchedBy: "canonical-lineage-trailer",
|
||||
confidence: "canonical",
|
||||
});
|
||||
await recordCommitAssociationFromHead(store, rootDir, taskId, lineageId);
|
||||
}
|
||||
mergerLog.log(`${taskId}: created fresh merge commit after verification fix (no prior commit to amend)`);
|
||||
return { ok: true, reason: "committed" };
|
||||
@@ -4478,18 +4517,7 @@ export async function commitOrAmendMergeWithFixes(
|
||||
{ cwd: rootDir, env: mergerCommitEnv() },
|
||||
);
|
||||
if (store && lineageId) {
|
||||
const sha = (await execAsync("git rev-parse HEAD", { cwd: rootDir })).stdout.trim();
|
||||
const subject = (await execAsync("git log -1 --format=%s HEAD", { cwd: rootDir })).stdout.trim();
|
||||
const authoredAt = (await execAsync("git log -1 --format=%aI HEAD", { cwd: rootDir })).stdout.trim();
|
||||
await store.upsertTaskCommitAssociation({
|
||||
taskLineageId: lineageId,
|
||||
taskIdSnapshot: taskId,
|
||||
commitSha: sha,
|
||||
commitSubject: subject,
|
||||
authoredAt,
|
||||
matchedBy: "canonical-lineage-trailer",
|
||||
confidence: "canonical",
|
||||
});
|
||||
await recordCommitAssociationFromHead(store, rootDir, taskId, lineageId);
|
||||
}
|
||||
mergerLog.log(`${taskId}: amended merge commit with verification fixes (deterministic message)`);
|
||||
return { ok: true, reason: "committed" };
|
||||
@@ -7871,13 +7899,21 @@ export async function aiMergeTask(
|
||||
if (aheadInfo?.aheadCount === 0) {
|
||||
const classification = await classifyOwnedLandedEvidence(rootDir, task, { mergeTargetBranch: aheadInfo.baseRef });
|
||||
if (classification.kind === "owned-commit") {
|
||||
const mergeCommitMessage = await regenerateBareMergeSubject({
|
||||
subject: classification.commit.subject,
|
||||
commitSha: classification.commit.sha,
|
||||
branch,
|
||||
taskId,
|
||||
rootDir,
|
||||
settings,
|
||||
});
|
||||
const mergeDetails: MergeDetails = {
|
||||
...(task.mergeDetails || {}),
|
||||
commitSha: classification.commit.sha,
|
||||
filesChanged: classification.commit.filesChanged,
|
||||
insertions: classification.commit.insertions,
|
||||
deletions: classification.commit.deletions,
|
||||
mergeCommitMessage: classification.commit.subject,
|
||||
mergeCommitMessage,
|
||||
mergeConfirmed: true,
|
||||
mergedAt: new Date().toISOString(),
|
||||
prNumber: task.prInfo?.number,
|
||||
@@ -8132,13 +8168,21 @@ export async function aiMergeTask(
|
||||
|
||||
if (classification.kind === "owned-commit") {
|
||||
const mergedAt = new Date().toISOString();
|
||||
const mergeCommitMessage = await regenerateBareMergeSubject({
|
||||
subject: classification.commit.subject,
|
||||
commitSha: classification.commit.sha,
|
||||
branch,
|
||||
taskId,
|
||||
rootDir,
|
||||
settings,
|
||||
});
|
||||
await store.updateTask(taskId, {
|
||||
mergeDetails: {
|
||||
commitSha: classification.commit.sha,
|
||||
filesChanged: classification.commit.filesChanged,
|
||||
insertions: classification.commit.insertions,
|
||||
deletions: classification.commit.deletions,
|
||||
mergeCommitMessage: classification.commit.subject,
|
||||
mergeCommitMessage,
|
||||
mergedAt,
|
||||
mergeConfirmed: true,
|
||||
prNumber: task.prInfo?.number,
|
||||
@@ -8152,7 +8196,7 @@ export async function aiMergeTask(
|
||||
result.filesChanged = classification.commit.filesChanged;
|
||||
result.insertions = classification.commit.insertions;
|
||||
result.deletions = classification.commit.deletions;
|
||||
result.mergeCommitMessage = classification.commit.subject;
|
||||
result.mergeCommitMessage = mergeCommitMessage;
|
||||
result.mergedAt = mergedAt;
|
||||
result.mergeTargetBranch = mergeTarget.branch;
|
||||
result.mergeTargetSource = mergeTarget.source;
|
||||
|
||||
@@ -48,6 +48,7 @@ import { resolveWorktreesDir } from "./worktree-paths.js";
|
||||
import { canonicalFusionBranchName } from "./worktree-names.js";
|
||||
import { resolveIntegrationBranch } from "./integration-branch.js";
|
||||
import type { OwnedLandedClassification } from "./merger.js";
|
||||
import { regenerateBareMergeSubject } from "./merger-bare-subject.js";
|
||||
import { recoverForeignOnlyContamination } from "./recovery/foreign-only-contamination.js";
|
||||
import {
|
||||
buildNtfyClickUrl,
|
||||
@@ -4198,13 +4199,21 @@ export class SelfHealingManager {
|
||||
|
||||
const mergedAt = new Date().toISOString();
|
||||
if (classification.kind === "owned-commit") {
|
||||
const mergeCommitMessage = await regenerateBareMergeSubject({
|
||||
subject: classification.commit.subject,
|
||||
commitSha: classification.commit.sha,
|
||||
branch: task.branch ?? "",
|
||||
taskId: task.id,
|
||||
rootDir: this.options.rootDir,
|
||||
settings,
|
||||
});
|
||||
const mergeDetails: MergeDetails = {
|
||||
...(task.mergeDetails || {}),
|
||||
commitSha: classification.commit.sha,
|
||||
filesChanged: classification.commit.filesChanged,
|
||||
insertions: classification.commit.insertions,
|
||||
deletions: classification.commit.deletions,
|
||||
mergeCommitMessage: classification.commit.subject,
|
||||
mergeCommitMessage,
|
||||
mergeConfirmed: true,
|
||||
mergedAt,
|
||||
mergeTargetBranch: ahead.baseRef,
|
||||
@@ -4292,6 +4301,14 @@ export class SelfHealingManager {
|
||||
for (const task of candidates) {
|
||||
const classification = await classifyOwnedLandedEvidenceForSelfHealing(this.options.rootDir, task, mergeTargetBranch);
|
||||
if (classification.kind === "owned-commit") {
|
||||
const mergeCommitMessage = await regenerateBareMergeSubject({
|
||||
subject: classification.commit.subject,
|
||||
commitSha: classification.commit.sha,
|
||||
branch: task.branch ?? "",
|
||||
taskId: task.id,
|
||||
rootDir: this.options.rootDir,
|
||||
settings,
|
||||
});
|
||||
await this.store.updateTask(task.id, {
|
||||
mergeDetails: {
|
||||
...(task.mergeDetails || {}),
|
||||
@@ -4299,7 +4316,7 @@ export class SelfHealingManager {
|
||||
filesChanged: classification.commit.filesChanged,
|
||||
insertions: classification.commit.insertions,
|
||||
deletions: classification.commit.deletions,
|
||||
mergeCommitMessage: classification.commit.subject,
|
||||
mergeCommitMessage,
|
||||
},
|
||||
});
|
||||
await this.recordIntegrityAudit(task.id, "task:integrity-reconcile-modified-files", {
|
||||
@@ -5110,13 +5127,21 @@ export class SelfHealingManager {
|
||||
const landedCommit = await this.findLandedTaskCommit(task);
|
||||
|
||||
if (landedCommit) {
|
||||
const mergeCommitMessage = await regenerateBareMergeSubject({
|
||||
subject: landedCommit.subject,
|
||||
commitSha: landedCommit.sha,
|
||||
branch: task.branch ?? "",
|
||||
taskId: task.id,
|
||||
rootDir: this.options.rootDir,
|
||||
settings,
|
||||
});
|
||||
const mergeDetails: MergeDetails = {
|
||||
commitSha: landedCommit.sha,
|
||||
rebaseBaseSha: landedCommit.rebaseBaseSha,
|
||||
filesChanged: landedCommit.filesChanged,
|
||||
insertions: landedCommit.insertions,
|
||||
deletions: landedCommit.deletions,
|
||||
mergeCommitMessage: landedCommit.subject,
|
||||
mergeCommitMessage,
|
||||
mergedAt: new Date().toISOString(),
|
||||
mergeConfirmed: true,
|
||||
prNumber: getPrimaryPrInfo(task)?.number,
|
||||
@@ -5520,13 +5545,21 @@ export class SelfHealingManager {
|
||||
try {
|
||||
const landedCommit = await this.findLandedTaskCommit(task);
|
||||
if (landedCommit) {
|
||||
const mergeCommitMessage = await regenerateBareMergeSubject({
|
||||
subject: landedCommit.subject,
|
||||
commitSha: landedCommit.sha,
|
||||
branch: task.branch ?? "",
|
||||
taskId: task.id,
|
||||
rootDir: this.options.rootDir,
|
||||
settings,
|
||||
});
|
||||
const mergeDetails: MergeDetails = {
|
||||
commitSha: landedCommit.sha,
|
||||
rebaseBaseSha: landedCommit.rebaseBaseSha,
|
||||
filesChanged: landedCommit.filesChanged,
|
||||
insertions: landedCommit.insertions,
|
||||
deletions: landedCommit.deletions,
|
||||
mergeCommitMessage: landedCommit.subject,
|
||||
mergeCommitMessage,
|
||||
mergedAt: new Date().toISOString(),
|
||||
mergeConfirmed: true,
|
||||
prNumber: getPrimaryPrInfo(task)?.number,
|
||||
|
||||
Reference in New Issue
Block a user