fix(merger): build commit messages from actual content, not branch range
The merge commit message was built from `commitLog`/`diffStat` computed against `merge-base(branch, main)`. Under squash-merge workflows, when an earlier task is squash-merged onto main first, branches that forked off the pre-squash main no longer share ancestry with it — `merge-base` resolves to a point before the earlier task, and the message describes work already merged via the prior squash. FN-2952's commit body claimed 11 files / 557 insertions when the actual diff was 2 files / 55 lines. Subject was also a generic `merge <branch>` regardless of content. - packages/engine/src/merger.ts: new `computeActualMergeCommitContext` helper that derives commitLog/diffStat from the actual integration delta (`git diff --cached <integrationTarget> --stat`), filtering branch commits by patch-id against the target's recent history to drop already-squashed siblings. Wired into both commit-finalization sites (`commitOrAmendMergeWithFixes` uses `preAttemptHeadSha`; the final amend in `runMergeAttempt` uses `HEAD~1`). Agent-context use of the wide range is unchanged. - packages/engine/src/merger.ts: `buildDeterministicMergeMessage` now generates subject and body in parallel via `Promise.all`. Subject is composed as `feat(taskId): <ai summary>`, capped at 72 chars, with fallback to the legacy `merge <branch>` form on any AI failure. - packages/core/src/ai-summarize.ts: new `summarizeCommitSubject` and `sanitizeCommitSubject` mirroring the body summarizer's structure. Same title-summarizer lane, 15s timeout. Sanitizer strips quotes, bullets, re-added conventional-commit prefixes, and trailing periods; hard-caps at 60 chars. - packages/core/src/__tests__/ai-summarize.test.ts: 9 tests covering the sanitizer's behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -2,6 +2,8 @@ import { describe, it, expect, beforeEach, vi } from "vitest";
|
||||
import {
|
||||
summarizeTitle,
|
||||
summarizeCommitBody,
|
||||
sanitizeCommitSubject,
|
||||
MAX_COMMIT_SUBJECT_LENGTH,
|
||||
checkRateLimit,
|
||||
getRateLimitResetTime,
|
||||
validateDescription,
|
||||
@@ -266,6 +268,63 @@ describe("ai-summarize", () => {
|
||||
|
||||
// ── State Reset ───────────────────────────────────────────────────────────
|
||||
|
||||
describe("sanitizeCommitSubject", () => {
|
||||
it("returns null for empty / whitespace input", () => {
|
||||
expect(sanitizeCommitSubject("")).toBeNull();
|
||||
expect(sanitizeCommitSubject(" \n ")).toBeNull();
|
||||
});
|
||||
|
||||
it("keeps a clean subject as-is", () => {
|
||||
expect(sanitizeCommitSubject("add unavailable-node validation")).toBe(
|
||||
"add unavailable-node validation",
|
||||
);
|
||||
});
|
||||
|
||||
it("uses only the first non-empty line", () => {
|
||||
expect(sanitizeCommitSubject("add validation\n\nbody text here")).toBe(
|
||||
"add validation",
|
||||
);
|
||||
expect(sanitizeCommitSubject("\n\n refactor merger\nignored second line")).toBe(
|
||||
"refactor merger",
|
||||
);
|
||||
});
|
||||
|
||||
it("strips surrounding quotes and backticks", () => {
|
||||
expect(sanitizeCommitSubject('"add tests for store"')).toBe("add tests for store");
|
||||
expect(sanitizeCommitSubject("'fix race in heartbeat'")).toBe("fix race in heartbeat");
|
||||
expect(sanitizeCommitSubject("`add caching layer`")).toBe("add caching layer");
|
||||
});
|
||||
|
||||
it("strips a leading bullet marker", () => {
|
||||
expect(sanitizeCommitSubject("- add caching layer")).toBe("add caching layer");
|
||||
expect(sanitizeCommitSubject("* fix bug")).toBe("fix bug");
|
||||
});
|
||||
|
||||
it("drops a leading conventional-commit prefix the model adds back", () => {
|
||||
expect(sanitizeCommitSubject("feat: add validation")).toBe("add validation");
|
||||
expect(sanitizeCommitSubject("feat(FN-123): add validation")).toBe("add validation");
|
||||
expect(sanitizeCommitSubject("fix(scope): something")).toBe("something");
|
||||
expect(sanitizeCommitSubject("FEAT: shouty")).toBe("shouty");
|
||||
});
|
||||
|
||||
it("drops a trailing period", () => {
|
||||
expect(sanitizeCommitSubject("add validation.")).toBe("add validation");
|
||||
expect(sanitizeCommitSubject("add validation...")).toBe("add validation");
|
||||
});
|
||||
|
||||
it("hard-caps at MAX_COMMIT_SUBJECT_LENGTH", () => {
|
||||
const long = "a".repeat(MAX_COMMIT_SUBJECT_LENGTH + 20);
|
||||
const result = sanitizeCommitSubject(long);
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.length).toBeLessThanOrEqual(MAX_COMMIT_SUBJECT_LENGTH);
|
||||
});
|
||||
|
||||
it("returns null when stripping leaves nothing", () => {
|
||||
expect(sanitizeCommitSubject('""')).toBeNull();
|
||||
expect(sanitizeCommitSubject("feat: ")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("__resetSummarizeState", () => {
|
||||
it("should clear all rate limit entries", () => {
|
||||
const ip = "192.168.1.1";
|
||||
|
||||
Reference in New Issue
Block a user