fix(merger): prefer step headline subject + add autostash race-rescue
- deriveDeterministicSubjectSummary now picks the lowest-numbered `complete Step N` headline (or the oldest commit) instead of the most recent commit, so trailing quality-gate revisions stop hijacking the squash-merge subject (FN-3617 landed as "align mailbox modal css..." when 4 of 5 commits were the actual Claude OAuth fix). - AI subject + body system prompts in ai-summarize.ts now weight by commit theme rather than file size, so a small token cleanup that touches a large CSS file no longer dominates the summary. - stashUnrelatedRootDirChanges adds a bounded re-snapshot loop after the primary stash is persisted but before \`git reset --hard\`. Any late-dirty paths (concurrent dev edits during a long merger run, parallel merger runs racing on rootDir, late test/build artifacts) get captured in labeled \`race-rescue-N\` stashes recoverable from \`git stash list\`, instead of being wiped by the destructive reset. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
43
packages/engine/src/__tests__/derive-subject-summary.test.ts
Normal file
43
packages/engine/src/__tests__/derive-subject-summary.test.ts
Normal file
@@ -0,0 +1,43 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { deriveDeterministicSubjectSummary } from "../merger.js";
|
||||
|
||||
describe("deriveDeterministicSubjectSummary", () => {
|
||||
it("returns null on empty input", () => {
|
||||
expect(deriveDeterministicSubjectSummary("")).toBeNull();
|
||||
expect(deriveDeterministicSubjectSummary(" \n ")).toBeNull();
|
||||
});
|
||||
|
||||
it("uses the only line as the summary", () => {
|
||||
expect(deriveDeterministicSubjectSummary("- feat(X): add widget")).toBe("add widget");
|
||||
});
|
||||
|
||||
it("prefers the lowest-numbered Step commit over the most recent revision", () => {
|
||||
// Order is most-recent-first, mimicking what the merger feeds in.
|
||||
const log = [
|
||||
"- fix(FN-3617): align mailbox modal css with design tokens",
|
||||
"- feat(FN-3617): complete Step 4 — document Claude manual OAuth flow",
|
||||
"- test(FN-3617): complete Step 2 — cover Anthropic manual-code UX",
|
||||
"- feat(FN-3617): complete Step 1 — switch Anthropic OAuth to manual code",
|
||||
].join("\n");
|
||||
expect(deriveDeterministicSubjectSummary(log)).toBe(
|
||||
"switch Anthropic OAuth to manual code (+3 more)",
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to the oldest commit when no Step headlines are present", () => {
|
||||
const log = [
|
||||
"- chore: lint",
|
||||
"- fix: handle null",
|
||||
"- feat: add foo",
|
||||
].join("\n");
|
||||
expect(deriveDeterministicSubjectSummary(log)).toBe("add foo (+2 more)");
|
||||
});
|
||||
|
||||
it("handles plain hyphen separator in Step lines", () => {
|
||||
const log = [
|
||||
"- feat: complete Step 2 - second thing",
|
||||
"- feat: complete Step 1 - first thing",
|
||||
].join("\n");
|
||||
expect(deriveDeterministicSubjectSummary(log)).toBe("first thing (+1 more)");
|
||||
});
|
||||
});
|
||||
@@ -1144,6 +1144,37 @@ async function stashUnrelatedRootDirChanges(
|
||||
{ cwd: rootDir },
|
||||
);
|
||||
|
||||
// Race-rescue: re-snapshot AFTER the stash is persisted but BEFORE the
|
||||
// destructive `git reset --hard` below. If any new dirty paths showed up
|
||||
// between our initial `git add -A` and now — concurrent dev edits, a
|
||||
// parallel merger run interleaving its own ops, or test/build artifacts
|
||||
// landing late — capture them in a SEPARATE rescue stash so they survive
|
||||
// the wipe. Without this loop, late writers lose their work because the
|
||||
// primary stash already snapshotted the earlier state. We loop a few
|
||||
// times because each rescue stash creation itself races with new writes;
|
||||
// bounded so a runaway writer can't pin us forever.
|
||||
const rescueShas: string[] = [];
|
||||
for (let attempt = 0; attempt < 3; attempt++) {
|
||||
const stillDirty = await snapshotDirtyFiles(rootDir);
|
||||
if (stillDirty.size === 0) break;
|
||||
const rescueLabel = `${AUTOSTASH_LABEL_PREFIX}${taskId}:race-rescue-${attempt}:${Date.now()}`;
|
||||
await execAsync("git add -A", { cwd: rootDir });
|
||||
const { stdout: rescueOut } = await execAsync("git stash create", {
|
||||
cwd: rootDir,
|
||||
encoding: "utf-8",
|
||||
});
|
||||
const rescueSha = String(rescueOut).trim();
|
||||
if (!rescueSha) break;
|
||||
await execAsync(
|
||||
`git stash store -m ${quoteArg(rescueLabel)} ${rescueSha}`,
|
||||
{ cwd: rootDir },
|
||||
);
|
||||
rescueShas.push(rescueSha);
|
||||
mergerLog.warn(
|
||||
`${taskId}: race-rescue stash ${rescueSha.slice(0, 7)} captured ${stillDirty.size} late-dirty path(s) (${rescueLabel}) — recover with: cd ${rootDir} && git stash apply ${rescueSha}`,
|
||||
);
|
||||
}
|
||||
|
||||
// Bring working tree back to HEAD so the merge can proceed. Reset
|
||||
// un-stages everything we just staged AND drops tracked-file
|
||||
// modifications. `git clean -fd` removes any untracked files / dirs
|
||||
@@ -1151,8 +1182,11 @@ async function stashUnrelatedRootDirChanges(
|
||||
await execAsync("git reset --hard HEAD", { cwd: rootDir });
|
||||
await execAsync("git clean -fd", { cwd: rootDir });
|
||||
|
||||
const rescueSuffix = rescueShas.length > 0
|
||||
? ` + ${rescueShas.length} race-rescue stash(es): ${rescueShas.map((s) => s.slice(0, 7)).join(", ")}`
|
||||
: "";
|
||||
mergerLog.log(
|
||||
`${taskId}: stashed ${dirty.size} unrelated dirty path(s) in rootDir as ${sha.slice(0, 7)} (${label})`,
|
||||
`${taskId}: stashed ${dirty.size} unrelated dirty path(s) in rootDir as ${sha.slice(0, 7)} (${label})${rescueSuffix}`,
|
||||
);
|
||||
return { sha, label };
|
||||
} catch (err: unknown) {
|
||||
@@ -1583,13 +1617,18 @@ async function generateAiMergeSubject(
|
||||
|
||||
/**
|
||||
* Derive a non-AI subject summary from the branch's step commit log. The log
|
||||
* is `- subj1\n- subj2\n…` (most recent first). We use the first subject with
|
||||
* its conventional-commit prefix stripped (to avoid `feat: feat(...): …`),
|
||||
* and tack on `(+N more)` when the branch has multiple step commits. This is
|
||||
* the fallback used when `summarizeCommitSubject` returns null — it conveys
|
||||
* what landed instead of the bare `merge <branch>` template.
|
||||
* is `- subj1\n- subj2\n…` (most recent first). The naive "use lines[0]" choice
|
||||
* is wrong in practice: when a quality-gate revision lands as the final commit
|
||||
* (e.g. a token-cleanup fixup after Step 4), the most-recent subject describes
|
||||
* the *fixup*, not the task. So we prefer, in order:
|
||||
* 1. The lowest-numbered `complete Step N — …` commit (the headline step)
|
||||
* 2. The oldest commit (lines[last]) — typically Step 1 / the first feat
|
||||
* commit on the branch
|
||||
*
|
||||
* Conventional-commit prefix is stripped to avoid `feat: feat(...): …`, and we
|
||||
* tack on `(+N more)` when the branch has multiple step commits.
|
||||
*/
|
||||
function deriveDeterministicSubjectSummary(commitLog: string): string | null {
|
||||
export function deriveDeterministicSubjectSummary(commitLog: string): string | null {
|
||||
const lines = commitLog
|
||||
.split(/\r?\n/)
|
||||
.map((l) => l.trim())
|
||||
@@ -1599,12 +1638,24 @@ function deriveDeterministicSubjectSummary(commitLog: string): string | null {
|
||||
const stripBullet = (l: string) => l.replace(/^[-*]\s+/, "").trim();
|
||||
const stripConventional = (l: string) =>
|
||||
l.replace(/^[a-z]+(?:\([^)]+\))?!?:\s*/i, "").trim();
|
||||
const cleaned = lines.map((l) => stripConventional(stripBullet(l)));
|
||||
|
||||
const first = stripConventional(stripBullet(lines[0]));
|
||||
if (!first) return null;
|
||||
const stepRe = /^complete Step (\d+)\s*[—\-:]\s*(.+)$/i;
|
||||
let bestStep: { n: number; summary: string } | null = null;
|
||||
for (const c of cleaned) {
|
||||
const m = c.match(stepRe);
|
||||
if (!m) continue;
|
||||
const n = Number(m[1]);
|
||||
const summary = m[2].trim();
|
||||
if (!summary) continue;
|
||||
if (!bestStep || n < bestStep.n) bestStep = { n, summary };
|
||||
}
|
||||
|
||||
const headline = bestStep?.summary ?? cleaned[cleaned.length - 1];
|
||||
if (!headline) return null;
|
||||
|
||||
const extras = lines.length - 1;
|
||||
const summary = extras > 0 ? `${first} (+${extras} more)` : first;
|
||||
const summary = extras > 0 ? `${headline} (+${extras} more)` : headline;
|
||||
return summary;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user